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

Correct linker-wrapper script under Msys2 and Cygwin. Fixes #58 #59

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

SquallATF
Copy link

Because rustc can not run shell script under windows bash, so make a hack in linker-wrapper.bat, if SEHLL variable is defind and find cygpath then try run shell script. Otherwise the default behavior will be used
Another fix is convert RUST_ANDROID_GRADLE_CC to unix path when run with native python under Msys2 and Cygwin. if run with native python under Msys2 and Cygwin the subprocess need executable file path in unix format.

@ncalexan
Copy link
Member

@SquallATF thanks for the patch. This is more complicated than I am comfortable, mostly because you choose to "do it right" and use ctypes to invoke Cygwin/MSYS path conversions, but I can live with that.

However, I'd like to understand why you bounce from .bat -> .sh -> python rather than go straight from .bat -> python. Is there a technical reason for that?

Finally, I have ticket #49 to stop using Python for the linker wrapper entirely. I don't think it's needed, since we're just invoking. If you were to instead fix #49, and just invoke in the .bat file (which I'm pretty sure I didn't know how to do when I wrote this thing), and then you were to handle the Cygwin/MSYS case, we would no longer need to do any ctypes stuff. Right? What do you think of that approach?

If there's some hard blocker to doing #49 and then this work, I'm inclined to accept this patch: I have only nits and style preferences. Thanks again!

@SquallATF
Copy link
Author

Update patch, remove python path convert and convert it from linker-wrapper.bat

if use .bat -> python, will break link.
when linker arguments too long, rustc will create a linker response file, but native Msys2/Cygwin application will convert response file back to argument when run in cmd, then will get argument too long error. so that need run python from .sh when use Msys2/Cygwin

@ncalexan
Copy link
Member

@SquallATF I'm sorry that I haven't responded -- I've been very busy with work and family. I expect to merge this fix but I haven't had time to work through the "don't use Python" details myself just yet. Thanks again, for the patch and for your patience.

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.

None yet

2 participants