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

Escape sysroot and manifest paths to remove whitespaces #221

Closed
wants to merge 1 commit into from
Closed

Escape sysroot and manifest paths to remove whitespaces #221

wants to merge 1 commit into from

Conversation

AxelMontini
Copy link

@AxelMontini AxelMontini commented May 30, 2018

Hopefully fixes #206

Fixes an error on windows where paths with whitespaces don't get escaped at all, causing the error error: multiple input filenames provided. In my case it happens because my Windows username is "Axel Montini" and xargo doesn't like the space in the middle.
This commit adds a function in utils to escape a string and replace spaces with %20 on windows or \ on other operative systems.
I haven't tested this on a Unix machine, but it seems to work on windows.

@AxelMontini AxelMontini changed the title Escape sysroot and manifest paths to remove blank spaces Escape sysroot and manifest paths to remove whitespaces May 30, 2018
@phil-opp
Copy link

phil-opp commented Jun 11, 2018

Hi @AxelMontini! Thank you for tackling #206!

It seems like xargo is no longer maintained, but we have an active fork at rust-osdev/cargo-xbuild. Your PR looks useful to us, so I opened rust-osdev#6 as a copy of this PR. Is this ok with you that we merge it?

@AxelMontini
Copy link
Author

Hi @phil-opp! Yes, you can merge it if you want, so we can finally resolve this issue.

@RalfJung
Copy link
Collaborator

These paths are used in RUSTFLAGS, right? Escaping won't help, unfortunately. Did you actually test this? I don't think this will work.

See rust-lang/cargo#6139 -- with RUSTFLAGS there just is no way to pass spaces to rustc.

@RalfJung
Copy link
Collaborator

Oh I see, I think this works on Windows because it actually replaces the space by a string that does not have a space. The unix version will definitely not work, there is still a space in the string and backslash does not help.

Can you update this PR to instead error on unix, pointing to rust-lang/cargo#6139 ? We just cannot support paths with spaces on that platform currently. We can do that on Windows though, which seems nice!

@phil-opp
Copy link

phil-opp commented Aug 26, 2019

If I remember correctly, I removed this patch from cargo-xbuild again after it turned out that it wasn't really working (on Windows).

@RalfJung
Copy link
Collaborator

Okay, closing this PR then.
In principle I am open to a Windows-only solution to this problem, but then we should really have Windows CI as well to make sure this works.

@RalfJung RalfJung closed this Sep 18, 2019
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.

Issues with spaces in sysroot path (e.g., Windows user accounts containing spaces)
3 participants