rustdoc: make --emit and --out-dir mimic rustc#153003
rustdoc: make --emit and --out-dir mimic rustc#153003notriddle wants to merge 1 commit intorust-lang:mainfrom
--emit and --out-dir mimic rustc#153003Conversation
|
r? @camelid rustbot has assigned @camelid. Use Why was this reviewer chosen?The reviewer was selected based on:
|
This comment has been minimized.
This comment has been minimized.
efa1dfa to
3136b6c
Compare
This comment has been minimized.
This comment has been minimized.
3136b6c to
ab734a6
Compare
|
r? rustdoc |
| input, | ||
| output_file: None, | ||
| output_dir: None, | ||
| output_dir: if render_options.output_to_stdout { |
There was a problem hiding this comment.
Hmm, rustc & rustdoc (except the json backend) actually interpret the - passed to --out-dir literally. So under "normal execution" both place the artifacts into a file called - (I can't say I like that; I think ideally we would reject such a 'weird' path). Consequently, rustc tries to write to -/$FILESTEM.d given --emit=depinfo --out-dir=-.
I fear it would be more consistent to ignore output_to_stdout here (which is currently only used by the json backend). What do you think?
There was a problem hiding this comment.
The json backend is usable with --emit=depinfo, though.
The problem is that --out is aliased to --out-dir in rustdoc, and it isn't in rustc. If #149365 was merged, there'd be a more principled way to handle this, but it isn't.
src/librustdoc/lib.rs
Outdated
| } | ||
|
|
||
| if render_opts.dep_info().is_some() { | ||
| if let Err(e) = fs::create_dir_all(&render_opts.output) { |
There was a problem hiding this comment.
For some unknown reason, rustc doesn't actually try to create the outdir for depinfo files, not sure if that's intentional. E.g., --out-dir=nonexistent --emit=dep-info fails. This might warrant a quick investigation. Unless it's a bug in rustc, we should probably be consistent with rustc and not attempt to create the outdir? I'm split.
I don't know why but write_dep_info specifically contains a check to prevent create_dir_all from being called if DepInfo is the only requested emission type. However that runs after write_out_deps which by that point would've already bailed out with an error if the directory didn't exist...
rust/compiler/rustc_interface/src/passes.rs
Lines 844 to 855 in 67aec36
There was a problem hiding this comment.
I can remove it.
I put it in there because it seemed more consistent with rustdoc's behavior (we implicitly create the directory, but we do it after emitting depinfo while emitting html), but it's easy for build systems to create the directory themselves (cargo does).
The behavior in the test case matches rustc's:
test-dingus % ls
main.rs
test-dingus % mkdir foobar
test-dingus % rustc --emit=dep-info main.rs --out-dir=foobar
test-dingus % ls
foobar main.rs
test-dingus % ls foobar
main.d
test-dingus % rustc --emit=dep-info=testfile.d main.rs --out-dir=foobar
test-dingus % ls
foobar main.rs testfile.d
test-dingus % ls foobar
main.d
ab734a6 to
22a0e06
Compare
The behavior in the test case matches rustc's:
CC #146220 (comment)