Distinguish between compiling the build script and the rest

[hencrice]

Fixes #7921.

Even though unit tests passed. I would like some guidance on why for the same project, some times the build script is compiled but not in others.

One example:
In the rebuild_preserves_out_dir test, ln 2178 does not compile the build script but line 2188 does.

Another example:
In the path_dep_build_cmd test, ln 716 does not compile the build script but ln 737 does.

[rust-highfive]

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @ehuss (or someone else) soon.

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

Please see the contribution instructions for more information.

[ehuss]

I think the confusion lies with how it is choosing to display the "Compiling build script" message. The dependency graph for a package with a build script looks something like this:

foo (lib)
└─ run foo's build script
   └─ compile foo's build script

Since building starts at the leaves, Cargo starts with "Compiling" the build script. The is_run_custom_build function returns true for the Unit in the middle (running the build script). Status is only reported once per-package, so the second message is skipped.

From a high level, I'm not sure how this should behave. I'd be reluctant to report every time a build script is built or run. And in a sense, for cargo check, it is correct to report that it is "Compiling" something, because it is. Some random ideas:

• Just report Checking even when building build scripts. Does the user really care one way or the other?
• Report Compiling with a note that it is compiling a build script (maybe like Compiling libc v0.2.66 (build-script)).
• Report both Compiling build script and Checking for the same package (only with cargo check).

It would probably be good, if we change this, to do similar treatment for cargo doc. I lean towards not changing the behavior for other commands (build, test, etc.).

[hencrice]

Hey Eric. Thanks for the explanation. Just want to make sure I understand, in the example of rebuild_preserves_out_dir test, the first cargo build on ln2174:

1. compiles the build script, which is shown as "Compiling foo"
2. runs it to build foo. However, this message is skipped.

For the second cargo build on ln2185. The reason why it shows "Compiling build script" is because:

1. it does not recompile the build script since it hasn't been changed. This prints nothing.
2. the compiled build script is run to build foo, which causes the "Compiling build script" (or more accurately, should've been called "Running build script") to be printed.

Is that the correct understanding?

As for the 3 options you laid out, I think 2 makes the most sense.

Could you elaborate on the last point regarding cargo doc. Does it also behave similarly (i.e. prints "Compiling foo" when it actually only compiles a build script)? Is there an issue for it as well?

[ehuss]

Is that the correct understanding?

Correct!

Could you elaborate on the last point regarding cargo doc. Does it also behave similarly (i.e. prints "Compiling foo" when it actually only compiles a build script)? Is there an issue for it as well?

Yes. It isn't necessarily a separate issue, because Cargo does the equivalent of cargo check for all dependencies when doing cargo doc.

Another thing to keep in mind is that proc-macros are also "compiled" (both for check and doc). I'm not sure how those should be handled, either.

[hencrice]

@ehuss Does the latest commit look ok?

[alexcrichton]

FWIW this UI has been an issue for quite a long time with issues like #833. I'm not really sure if this will improve things though since it only prints out that build scrips are being compiled, and then this doesn't actually print anything else for the main build. Users might, for example, get confused when there's not another annotation saying that something other than the build script is being built.

Overall I feel like this is maybe something we don't necessarily need to fix. Astute users can find that Cargo isn't printing everything that's happening, but I'm wary to add more output since it runs the risk of unnecessarily drowning out what's actually happening. (already I feel Cargo is a bit too verbose nowadays).

[hencrice]

FWIW this UI has been an issue for quite a long time with issues like #833. I'm not really sure if this will improve things though since it only prints out that build scrips are being compiled, and then this doesn't actually print anything else for the main build. Users might, for example, get confused when there's not another annotation saying that something other than the build script is being built.

Overall I feel like this is maybe something we don't necessarily need to fix. Astute users can find that Cargo isn't printing everything that's happening, but I'm wary to add more output since it runs the risk of unnecessarily drowning out what's actually happening. (already I feel Cargo is a bit too verbose nowadays).

Agree. I took this issue assuming it's a minor problem we want to address. But I'm totally ok with dropping it. I'm gonna close it for now. Feel free to reopen and let me know :)