godot-cpp icon indicating copy to clipboard operation
godot-cpp copied to clipboard

Fix shell argument limit

Open ze2j opened this issue 10 months ago • 12 comments

Fix 1729

Tested on Linux only.

Note: I didn't manage to remove the ar command print in non verbose builds. output

To reproduce/test:

  • make sure the/path/to/godot-cpp is long enough (at least 47 characters)
  • use this extension_api.json
  • build godot-cpp as usual on a linux system

ze2j avatar Mar 06 '25 12:03 ze2j

aww linux only, i was hoping to test it for msys and mingw on windows see if it helps mingw link times at all(unlikely)

enetheru avatar Mar 06 '25 13:03 enetheru

Thanks for starting on a PR for this!

This should work on MSYS with mingw, since I think it uses GNU binutils. And, I'm not sure this will work on Linux if using ar from LLVM. So, I suspect specifically targeting this to Linux isn't the right way to go, I think we probably want to check for ar from GNU binutils.

Have you tried building on other platforms? I suspect this is a problem on Windows and MacOS too, and, in fact, my guess would be that Windows has a smaller limit than Linux.

Ideally, any solution to this would address all platforms and tool chains - unless this isn't a problem on other platforms and tool chains

dsnopek avatar Mar 06 '25 14:03 dsnopek

I'm not sure this will work on Linux if using ar from LLVM.

https://llvm.org/docs/CommandGuide/llvm-ar.html

@<FILE>

    Read command-line options and commands from response file <FILE>.

enetheru avatar Mar 06 '25 14:03 enetheru

https://llvm.org/docs/CommandGuide/llvm-ar.html

@enetheru Oh, awesome, thanks for looking that up!

I guess this means it'll work when building for Android too, because I think the Android tool chain uses clang

dsnopek avatar Mar 06 '25 14:03 dsnopek

I made the suggested fixes and included MinGW.

@dsnopek I’ve only ever built my project on Linux, so I’m not sure if this issue can be reproduced on other platforms. Windows seems to have an 8192 limit, so my guess is that there’s no issue.

I took a very pragmatic approach here, but if you prefer a more general one, I won’t be offended if you decide to close this PR. I won’t be able to spend time implementing a more general solution, but I can help test it on my project if someone comes up with one.

ze2j avatar Mar 07 '25 12:03 ze2j

I can't get the test integration compiling for mingw64 but everything else appears to work.

enetheru avatar Mar 08 '25 03:03 enetheru

@enetheru Should I drop the mingw inclusion or there is still some hope to make it work?

ze2j avatar Mar 09 '25 12:03 ze2j

I'm not very experienced on the scons side, perhaps someone else can test? but if it's going to block merging I would drop it as it's not required to fix your issue yeah?

enetheru avatar Mar 09 '25 21:03 enetheru

I’ve only ever built my project on Linux, so I’m not sure if this issue can be reproduced on other platforms. Windows seems to have an 8192 limit, so my guess is that there’s no issue. [...] Should I drop the mingw inclusion or there is still some hope to make it work?

I need to find some time to test this on a few platforms, and see if it's a problem elsewhere or just on Linux. If it's a problem elsewhere and this fix will work, I'd like to enable it everywhere that we can. If anyone else has time to test, that would be appreciated :-)

@ze2j: Is there any chance you could share the extension_api.json that you're using? Then folks will be able to easily reproduce the issue, without having to first make their own extension_api.json that will exhibit the problem

dsnopek avatar Mar 14 '25 14:03 dsnopek

@dsnopek sure, here it is. Don't forget to use a path to godot-cpp long enough.

ze2j avatar Mar 18 '25 20:03 ze2j

Hey, hope it's okay for me to comment on this!

Just wanted to mention I ran into the same issue in one of my own projects (see here) where the argument list was too long: scons: *** [/home/runner/work/godot-project-template/godot-project-template/engine/godot-cpp/bin/libgodot-cpp.linux.template_debug.dev.x86_64.a] sh: Argument list too long

From what I can see in my own project it only appeared to be an issue when compiling on a Linux OS, the other platforms that built on Mac or Windows passed (see here).

I've tried these changes in my own godot-cpp fork and I have changed it slightly so that instead of using: env["platform"] == "linux" it uses platform.system().lower() == "linux" and that takes into account compiling on Linux but for other env["platform"] targets. In my godot-cpp fork, everything passed with these changes apart from 🏁 Windows (x86_64, MinGW) (see here lots of undefined references) and I'm not sure why it's failing: c:/programdata/chocolatey/lib/mingw/tools/install/mingw64/bin/../lib/gcc/x86_64-w64-mingw32/12.2.0/../../../../x86_64-w64-mingw32/bin/ld.exe: src\example.o:example.cpp:(.text+0x336): undefined reference to godot::String::String(char const*)' I don't use MinGW myself so I don't understand what's going wrong with that part of the CI run.

These are my godot-cpp tool scripts in case anyone wanted to try them: godotcpp.py common_compiler_flags.py

And these are the diffs to highlight what the changes are: common_compiler_flags.py image

godotcpp.py image image

With these godot-cpp changes my project CI passes (see here) but as mentioned before the godot-cpp CI is failing for 🏁 Windows (x86_64, MinGW) so I'm not sure what the fix for that is.

Any ideas why the godot-cpp MinGW CI is failing with these changes?

comfyjase avatar Sep 19 '25 15:09 comfyjase

This issue happens / can be reproduced in MSYS2 mingw-w64-ucrt-x86_64-toolchain simply by cloning / cheking out godot-4.5-stable. Both MSYS and godot-cpp are in the root of a drive i.e. E:/msys64/... and E:/godot-cpp/... so there is no way to make the paths shorter. This PR seems to fix it at least for scons platform=windows.

h4ilst0rm avatar Oct 07 '25 09:10 h4ilst0rm