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

Cargo skips build script directives containing filenames unrepresentable in UTF-8 #6537

Open
alercah opened this issue Jan 10, 2019 · 7 comments
Labels
A-build-scripts Area: build.rs scripts C-bug Category: bug S-needs-mentor Status: Issue or feature is accepted, but needs a team member to commit to helping and reviewing.

Comments

@alercah
Copy link

alercah commented Jan 10, 2019

When processing build scripts for directives, Cargo requires the line to be valid UTF-8. This poses a problem on Windows, where filenames are not all valid UTF-8 and may contain unpaired surrogates.

I do not have any good ideas for handling this. Putting WTF-8 on the wire is the only thing that seems close to workable, but it's explicitly not supposed to be used for that purpose, and there are no APIs that can be used (Rust very intentionally does not expose the wtf8 encoding).

@alercah alercah added the C-bug Category: bug label Jan 10, 2019
@toothbrush7777777
Copy link
Contributor

I may be wrong but this seems like a very obscure corner case. Despite Windows allowing invalid UTF-16 surrogates, nothing should normally generate them. It should never occur under any normal or abnormal usage. The build script should just fail if it is invalid UTF-8.

@ehuss ehuss added the A-build-scripts Area: build.rs scripts label Jan 31, 2019
@epage epage added the S-needs-team-input Status: Needs input from team on whether/how to proceed. label Sep 20, 2023
@epage
Copy link
Contributor

epage commented Sep 20, 2023

I don't see us handling invalid UTF8 (not really supported in cargo generally). I don't think we should silently ignore them. The question for me is whether this
needs to start as a warning (#12235) or move straight to an error.

@ChrisDenton
Copy link
Member

It can go straight to being an error, no? It's not supported atm (either by cargo or other tools cargo relies on) and in any case means something has gone badly wrong. At best it's a programming error somewhere and at worst it's something malicious.

@weihanglo
Copy link
Member

Even we decide to emit a hard error for this, we still need a transition period starting from a warning, in case not to break somebody.

@ChrisDenton
Copy link
Member

I was under the impression it's already broken for any practical purposes. Hence the support request.

@epage
Copy link
Contributor

epage commented Sep 21, 2023

Depends on the build directive and what the impact is. A "rerun if changed" would only mean you get brittle rebuilds but it you can still get things done and people can still depend on you.

@ChrisDenton
Copy link
Member

I'm definitely not against emitting a warning for awhile first and I don't mean to argue against moving forward with it, especially as you know Cargo far better than I.

I am just struggling to see a realistic scenario in which emitting an error would have an impact that either wasn't already felt elsewhere in the build (perhaps with a more cryptic message) or where it would cause more harm than continuing regardless. No build tools I'm aware of support non-Unicode Windows paths (or dev tools like git and vscode). And while a directive unexpectedly having no effect may not cause an immediate error, presumably the expectation would be for that file to be used for something.

@epage epage added S-needs-mentor Status: Issue or feature is accepted, but needs a team member to commit to helping and reviewing. and removed S-needs-team-input Status: Needs input from team on whether/how to proceed. labels Oct 31, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-build-scripts Area: build.rs scripts C-bug Category: bug S-needs-mentor Status: Issue or feature is accepted, but needs a team member to commit to helping and reviewing.
Projects
None yet
Development

No branches or pull requests

6 participants