Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix: nintendoswitch compilation #21368

Merged
merged 4 commits into from
Feb 25, 2023
Merged

Fix: nintendoswitch compilation #21368

merged 4 commits into from
Feb 25, 2023

Conversation

dkgitdev
Copy link
Contributor

make nintendoswitch someGcc, remove symlink support for nintendoswitch, add getAppFilename for nintendoswitch.

Finally used correct branches :)

…switch, add getAppFilename for nintendoswitch
@dkgitdev dkgitdev mentioned this pull request Feb 14, 2023
lib/pure/os.nim Outdated
@@ -702,6 +702,8 @@ proc getAppFilename*(): string {.rtl, extern: "nos$1", tags: [ReadIOEffect], noW
result = getApplHaiku()
elif defined(openbsd):
result = getApplOpenBsd()
elif defined(nintendoswitch):
result = "nxNimApp"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why are you doing this? The path of the nro is passed in via argv[0] by the hbmenu.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you! I didn't know that.

I've tried to access the argv through nim:

    elif defined(nintendoswitch):
      result = paramStr(0)

I've found out that there are no headers for cmdLine or cmdCount available:

$ nimble build_device_test
  Executing task build_device_test in /home/dk/PycharmProjects/toggleNx/toggleNx.nimble
Building: /home/dk/PycharmProjects/toggleNx/src/toggleNx/deviceTest/c.nim...
Building static lib file...
clean ...
main.c
linking nimNxStatic.elf
/opt/devkitpro/devkitA64/bin/../lib/gcc/aarch64-none-elf/12.2.0/../../../../aarch64-none-elf/bin/ld: /opt/devkitpro/devkitA64/bin/../lib/gcc/aarch64-none-elf/12.2.0/../../../../aarch64-none-elf/bin/ld: DWARF error: can't find .debug_ranges section.
/home/dk/PycharmProjects/toggleNx/libs/nimNxStatic/libs/c/libc.a(@m..@s..@s..@s..@s..@sNim@slib@[email protected]): in function `paramStr__stdZcmdline_52':
@m..@s..@s..@s..@s..@sNim@slib@[email protected]:(.text.paramStr__stdZcmdline_52+0x6c): undefined reference to `cmdCount'
/opt/devkitpro/devkitA64/bin/../lib/gcc/aarch64-none-elf/12.2.0/../../../../aarch64-none-elf/bin/ld: @m..@s..@s..@s..@s..@sNim@slib@[email protected]:(.text.paramStr__stdZcmdline_52+0x70): undefined reference to `cmdCount'
/opt/devkitpro/devkitA64/bin/../lib/gcc/aarch64-none-elf/12.2.0/../../../../aarch64-none-elf/bin/ld: @m..@s..@s..@s..@s..@sNim@slib@[email protected]:(.text.paramStr__stdZcmdline_52+0x90): undefined reference to `cmdLine'
/opt/devkitpro/devkitA64/bin/../lib/gcc/aarch64-none-elf/12.2.0/../../../../aarch64-none-elf/bin/ld: @m..@s..@s..@s..@s..@sNim@slib@[email protected]:(.text.paramStr__stdZcmdline_52+0x94): undefined reference to `cmdLine'
collect2: error: ld returned 1 exit status
make[1]: *** [/opt/devkitpro/libnx/switch_rules:80: /home/dk/PycharmProjects/toggleNx/libs/nimNxStatic/nimNxStatic.elf] Ошибка 1
make: *** [Makefile:165: build] Ошибка 2
stack trace: (most recent call last)
/tmp/nimblecache-334211879/nimscriptapi_3780686481.nim(187, 16)
/home/dk/PycharmProjects/toggleNx/toggleNx.nimble(36, 3) build_device_testTask
/home/dk/Nim/lib/system/nimscript.nim(264, 7) exec
/home/dk/Nim/lib/system/nimscript.nim(264, 7) Error: unhandled exception: FAILED: make clean && make [OSError]
     Error: Exception raised during nimble script execution

So, I guess it's better to leave it this way unless someone can help me out with this one.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

also, devkitpro didn't mention cmdCount in any of its files :(

Copy link
Contributor

@RSDuck RSDuck Feb 14, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it used to work (I fixed it myself with a PR some time ago), though looks like it's broken now.

cmdCount and cmdLine are from Nim:

posixCmdLine.add "\tN_LIB_PRIVATE int cmdCount;\L"

Copy link
Contributor Author

@dkgitdev dkgitdev Feb 14, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So how about stubbing it for now in order to make compilation work and migrate onto working solution under different PR?

I don't think amount of work will increase much if we do it later :)

Copy link
Contributor

@RSDuck RSDuck Feb 15, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah but don't bring your temporary fix which you could also just leave local into Nim devel.

You can use GDB on Switch btw, https://gist.github.com/jam1garner/c9ba6c0cff150f1a2480d0c18ff05e33

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fair enough, I'll try to fix cmdLine / cmdCount or remove the temp fix completely.

I tried to pick a generic name: nx stands switch, Nim is surely being used, and App is, well, app.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A-ha, the issue goes far than I thought.

It's because I use --noMaim option, so these params are turned off.

Should I fix it in logging, so it would not depend on this functionality?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, so I used the getApplHeuristic that feels like it suits my case exactly

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@RSDuck, can you please review my changes when you have time and ether ask for more changes or resolve this conversation? :)

Comment on lines +567 to +568
when not defined(nintendoswitch):
proc readlink*(a1, a2: cstring, a3: int): int {.importc, header: "<unistd.h>".}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

symlink is defined in the Nintendo Switch SDK, but not readlink?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it's just newlib. For Switch specifically it shouldn't matter either way though, because the OS only supports SD cards formatted with FAT32 or exFAT and to my knowledge neither support symlinks.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, I've explicitly revoked readlink, as I cannot efficiently stub it. It must break the code that tries to use it, so the developer will be aware of it not working, as it may be hard to debug if stubbed.

But as for the symlink creation, the non-working part is easy to see (nothing gets into some dir), so I made it result in a "nothing done" behaviour:

when not defined(nintendoswitch):
  proc symlink*(a1, a2: cstring): cint {.importc, header: "<unistd.h>".}
else:
  proc symlink*(a1, a2: cstring): cint = -1

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So, could you please tell me what to fix in the way I used or resolve this conversation? :)

@Araq Araq merged commit b2edfe7 into nim-lang:devel Feb 25, 2023
@github-actions
Copy link
Contributor

Thanks for your hard work on this PR!
The lines below are statistics of the Nim compiler built from b2edfe7

Hint: mm: orc; opt: speed; options: -d:release
166274 lines; 8.139s; 610.73MiB peakmem

survivorm pushed a commit to survivorm/Nim that referenced this pull request Feb 28, 2023
* Fix: make nintendoswitch someGcc, remove symlink support for nintendoswitch, add getAppFilename for nintendoswitch

* Fix: use getApplHeuristic on nintendoswitch
capocasa pushed a commit to capocasa/Nim that referenced this pull request Mar 31, 2023
* Fix: make nintendoswitch someGcc, remove symlink support for nintendoswitch, add getAppFilename for nintendoswitch

* Fix: use getApplHeuristic on nintendoswitch
bung87 pushed a commit to bung87/Nim that referenced this pull request Jul 29, 2023
* Fix: make nintendoswitch someGcc, remove symlink support for nintendoswitch, add getAppFilename for nintendoswitch

* Fix: use getApplHeuristic on nintendoswitch
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants