rustc: implement argsfiles for command line #63175
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
(rust_highfive has picked a reviewer for you, use r? to override) |
|
I clearly remember I've seen an issue about this, but I can't find it right now. MSVC supports them as well, with the same syntax. |
This comment has been minimized.
This comment has been minimized.
cf1bc47
to
fbcaacf
Compare
|
cc @alexcrichton since I believe you added our support for calling linkers with this and are probably generally interested due to Cargo concerns |
src/librustc_driver/args/mod.rs
Outdated
| Some(FileArg::Arg(arg)) => return Some(arg), | ||
| Some(FileArg::Path(path)) => { | ||
| let file = | ||
| if self.stack.len() >= MAX_RECURSION { None } else { fs::read(&path).ok() }; |
There was a problem hiding this comment.
It feels a bit error-prone to simply ignore @files that are "too deep". Maybe we can at least eprintln or something here? There's probably no good way to give a good error message here...
One possible "solution" is to just panic and bump MAX_RECURSION to some very large number like 1000 which would be relatively unreasonable.
There was a problem hiding this comment.
The documented error handling is pretty awkward - it also just includes the literal @file string as an arg if it can't open/read the file. The recursion handling is at least consistent with that low standard.
I'm happy to scatter some eprintlns (if that's the best that's available here) around to make noise about things going wrong.
|
Though this matches the ld spec, it is worth noting that this is the first place in rustc/cargo/etc where we're splitting by whitespace and implementing quoting rules around that. I haven't reviewed the implementation here closely but maybe we could avoid doing that somehow? |
|
Hey, I was just wanting this! Wonderful :) |
|
@Mark-Simulacrum I was following this as a convention rather than novelty for the sake of it. I chose gnu-ld as a model because it was the first even slightly detailed description of the file syntax which made sense (tho there's still a fair amount of ambiguity). I could go with something simpler like "each arg is on its own line", though that raises questions like:
The this implementation:
This has some amount of superfluous complexity, but I think that's offset by making it compatible with typical shell quoting rules (so, for example, Python's shlex module can be used to quote args in this form - with some care). |
|
I completely agree that this is a viable and maybe best implementation; just wanted to raise the concern that we've elsewhere always tried to shy away from shlex or the like. One solution might be to use a null byte as a separator, since iirc they're not valid utf-8 (and rustc generally doesn't work with anything requiring them, anyway). Since these files are presumably mostly programmatically generated that doesn't seem too bad. |
|
@Mark-Simulacrum \0-separated is an option, but its a bit painful to generate from a shell script and would make the files be treated as binary and unmergable by a source control system (eg, tests in rustc itself). |
|
BTW, where does |
|
I think my personal preference is |
This comment has been minimized.
This comment has been minimized.
|
@Mark-Simulacrum Ah thanks, I had thought that it could be used in the .rs file as an expansion. Test fixed (I hope). |
This comment has been minimized.
This comment has been minimized.
746623c
to
f3620d5
Compare
|
OK:
|
|
The job Click to expand the log.I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
9f51b9e
to
b409b65
Compare
|
The job Click to expand the log.I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
|
The job Click to expand the log.I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
|
@bors: delegate+ Implementation looks good to me, feel free to r+ when tests are good |
|
✌️ @jsgf can now approve this pull request |
07fc983
to
44753eb
Compare
|
The job Click to expand the log.I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
This will make @path work with miri and other non-standard entrypoints. Also since this simplifies librustc_driver::args, move it into a simple source file. Also remove the tests since they're doing nothing more than checking `str::lines` has the right behaviour.
|
@bors r+ |
|
📌 Commit d949774 has been approved by |
bors
added a commit
that referenced
this pull request
Aug 22, 2019
rustc: implement argsfiles for command line Many tools, such as gcc and gnu-ld, support "args files" - that is, being able to specify @file on the command line. This causes `file` to be opened and parsed for command line options. They're separated with whitespace; whitespace can be quoted with double or single quotes, and everything can be \\-escaped. Args files may recursively include other args files via `@file2`. See https://sourceware.org/binutils/docs/ld/Options.html#Options for the documentation of gnu-ld's @file parameters. This is useful for very large command lines, or when command lines are being generated into files by other tooling.
|
💥 Test timed out |
|
@bors retry |
bors
added a commit
that referenced
this pull request
Aug 22, 2019
rustc: implement argsfiles for command line Many tools, such as gcc and gnu-ld, support "args files" - that is, being able to specify @file on the command line. This causes `file` to be opened and parsed for command line options. They're separated with whitespace; whitespace can be quoted with double or single quotes, and everything can be \\-escaped. Args files may recursively include other args files via `@file2`. See https://sourceware.org/binutils/docs/ld/Options.html#Options for the documentation of gnu-ld's @file parameters. This is useful for very large command lines, or when command lines are being generated into files by other tooling.
|
☀️ Test successful - checks-azure |
Closed
3 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Many tools, such as gcc and gnu-ld, support "args files" - that is, being able to specify @file on the command line. This causes
fileto be opened and parsed for command line options. They're separated with whitespace; whitespace can be quoted with double or single quotes, and everything can be \-escaped. Args files may recursively include other args files via@file2.See https://sourceware.org/binutils/docs/ld/Options.html#Options for the documentation of gnu-ld's @file parameters.
This is useful for very large command lines, or when command lines are being generated into files by other tooling.