Fix build failure in case file doesn't exist #63869

GuillaumeGomez · 2019-08-24T21:47:48Z

It fixes the following issue:

$ ./x.py test src/tools/linkchecker ./build/x86_64-apple-darwin/doc/ --stage 1
Updating only changed submodules
Submodules updated in 0.05 seconds
    Finished dev [unoptimized] target(s) in 0.15s
thread 'main' panicked at 'source "/Users/imperio/rust/rust/build/x86_64-apple-darwin/doc/version_info.html" failed to get metadata: No such file or directory (os error 2)', src/build_helper/lib.rs:179:19
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace.
failed to run: /Users/imperio/rust/rust/build/bootstrap/debug/bootstrap test src/tools/linkchecker ./build/x86_64-apple-darwin/doc/ --stage 1
Build completed unsuccessfully in 0:00:01

If the file doesn't exist, it makes sense anyway to just run the command in order to generate it.

r? @Mark-Simulacrum

GuillaumeGomez · 2019-08-25T12:19:22Z

I removed the useless parens I added.

Mark-Simulacrum · 2019-08-25T13:48:38Z

Could we instead make sure the file is created in that same function? The code that creates it should probably have a || !version_info.exists()

GuillaumeGomez · 2019-08-27T20:11:11Z

Added the check (hope it's what you had in mind).

Mark-Simulacrum · 2019-08-27T22:13:01Z

src/bootstrap/doc.rs

@@ -375,6 +376,7 @@ impl Step for Standalone {
               up_to_date(&footer, &html) &&
               up_to_date(&favicon, &html) &&
               up_to_date(&full_toc, &html) &&
+               version_info.exists() &&


If this is false, something has gone horribly wrong; we should remove this check.

This can be false during the dry run. I think the correct fix would be to not run up_to_date(&version_info, &html) if builder.config.dry_run just like up_to_date(&rustdoc, &html) below.

Mark-Simulacrum · 2019-08-27T22:13:21Z

r=me with last comment fixed and commits squashed into one

ollie27 · 2019-08-30T09:40:26Z

src/bootstrap/doc.rs

@@ -353,7 +353,8 @@ impl Step for Standalone {
        let version_input = builder.src.join("src/doc/version_info.html.template");
        let version_info = out.join("version_info.html");

-        if !builder.config.dry_run && !up_to_date(&version_input, &version_info) {
+        if !builder.config.dry_run &&
+            (!version_info.exists() || !up_to_date(&version_input, &version_info)) {


This change doesn't have any effect as up_to_date already checks if the file exists:

rust/src/build_helper/lib.rs

Lines 173 to 175 in b7ad3f9

if !dst.exists() {

return false;

}

Indeed, the check should be on version_input. Good catch!

That's not what I meant. Checking version_input makes even less sense as that file has to exist because it's part of the source code.

Try changing the line:

up_to_date(&version_info, &html) &&

to

(builder.config.dry_run || up_to_date(&version_info, &html)) &&

I think that will fix this issue.

Mark-Simulacrum · 2019-08-30T13:05:06Z

src/bootstrap/doc.rs

@@ -353,7 +353,8 @@ impl Step for Standalone {
        let version_input = builder.src.join("src/doc/version_info.html.template");
        let version_info = out.join("version_info.html");

-        if !builder.config.dry_run && !up_to_date(&version_input, &version_info) {
+        if (builder.config.dry_run || up_to_date(&version_input, &version_info)) &&


It seems like this is wrong. We shouldn't need to generate this file during dry_run; if there's something that reads from it during dry_run then that should be gated, not the file creation.

GuillaumeGomez · 2019-08-30T13:15:12Z

I wonder if I shouldn't just go back to my initial fix at this point. 😆

ollie27 · 2019-08-31T20:14:56Z

I think I've caused some confusion. Anyway what I was trying to suggest was to just do:

--- a/src/bootstrap/doc.rs
+++ b/src/bootstrap/doc.rs
@@ -375,7 +375,7 @@ impl Step for Standalone {
                up_to_date(&footer, &html) &&
                up_to_date(&favicon, &html) &&
                up_to_date(&full_toc, &html) &&
-               up_to_date(&version_info, &html) &&
+               (builder.config.dry_run || up_to_date(&version_info, &html)) &&
                (builder.config.dry_run || up_to_date(&rustdoc, &html)) {
                 continue
             }

That will avoid the panic but I don't know if it's the best fix because I'm not 100% sure what should and shouldn't be run during the dry run.

Mark-Simulacrum · 2019-08-31T20:22:45Z

dry run as much as possible shouldn't be creating/reading files -- we can mostly avoid this via helper methods on builder which internally return empty strings and such, but not always, which is why we something see gating on dry_run in various code

GuillaumeGomez · 2019-09-02T12:14:20Z

Ok updated. Sorry about the confusion.

Mark-Simulacrum · 2019-09-02T12:30:13Z

I presume you've tested that this works?

GuillaumeGomez · 2019-09-02T12:33:54Z

Not yet. Just applied your suggestion. I'll try it once I'm done with dayjob.

Mark-Simulacrum · 2019-09-02T12:42:08Z

r=me if it works

GuillaumeGomez · 2019-09-02T20:16:09Z

I confirm: problem fixed. Thanks a lot @Mark-Simulacrum and @ollie27 !

@bors: r=Mark-Simulacrum

bors · 2019-09-02T20:16:10Z

📌 Commit a80ab3a has been approved by Mark-Simulacrum

tesuji · 2019-09-03T00:37:11Z

src/bootstrap/doc.rs

@@ -375,7 +375,7 @@ impl Step for Standalone {
               up_to_date(&footer, &html) &&
               up_to_date(&favicon, &html) &&
               up_to_date(&full_toc, &html) &&
-               up_to_date(&version_info, &html) &&
+               (builder.config.dry_run || up_to_date(&version_info, &html)) &&
               (builder.config.dry_run || up_to_date(&rustdoc, &html)) {


Suggested change

(builder.config.dry_run || up_to_date(&rustdoc, &html)) {

builder.config.dry_run ||

(up_to_date(&version_info, &html) && up_to_date(&rustdoc, &html)) {

This hurts rather than aids readability in my eyes.

bors · 2019-09-03T04:33:16Z

⌛ Testing commit a80ab3a with merge 815dec9...

@Mark-Simulacrum

…acrum Fix build failure in case file doesn't exist It fixes the following issue: ```bash $ ./x.py test src/tools/linkchecker ./build/x86_64-apple-darwin/doc/ --stage 1 Updating only changed submodules Submodules updated in 0.05 seconds Finished dev [unoptimized] target(s) in 0.15s thread 'main' panicked at 'source "/Users/imperio/rust/rust/build/x86_64-apple-darwin/doc/version_info.html" failed to get metadata: No such file or directory (os error 2)', src/build_helper/lib.rs:179:19 note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace. failed to run: /Users/imperio/rust/rust/build/bootstrap/debug/bootstrap test src/tools/linkchecker ./build/x86_64-apple-darwin/doc/ --stage 1 Build completed unsuccessfully in 0:00:01 ``` If the file doesn't exist, it makes sense anyway to just run the command in order to generate it. r? @Mark-Simulacrum

bors · 2019-09-03T08:24:45Z

☀️ Test successful - checks-azure
Approved by: Mark-Simulacrum
Pushing 815dec9 to master...

rust-highfive assigned Mark-Simulacrum Aug 24, 2019

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Aug 24, 2019

GuillaumeGomez force-pushed the fix-build-failure branch from e8569ba to 364f669 Compare August 25, 2019 12:18

Mark-Simulacrum reviewed Aug 27, 2019

View reviewed changes

GuillaumeGomez force-pushed the fix-build-failure branch from 60da4a9 to 7fdb312 Compare August 30, 2019 09:02

ollie27 reviewed Aug 30, 2019

View reviewed changes

GuillaumeGomez force-pushed the fix-build-failure branch 2 times, most recently from c3eee3a to 4b340b9 Compare August 30, 2019 11:08

Mark-Simulacrum reviewed Aug 30, 2019

View reviewed changes

Generate version file if it doesn't exist

a80ab3a

GuillaumeGomez force-pushed the fix-build-failure branch from 4b340b9 to a80ab3a Compare September 2, 2019 12:13

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Sep 2, 2019

tesuji reviewed Sep 3, 2019

View reviewed changes

bors added the merged-by-bors This PR was explicitly merged by bors. label Sep 3, 2019

bors merged commit a80ab3a into rust-lang:master Sep 3, 2019

GuillaumeGomez deleted the fix-build-failure branch September 4, 2019 08:27

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix build failure in case file doesn't exist #63869

Fix build failure in case file doesn't exist #63869

GuillaumeGomez commented Aug 24, 2019

GuillaumeGomez commented Aug 25, 2019

Mark-Simulacrum commented Aug 25, 2019

GuillaumeGomez commented Aug 27, 2019

Mark-Simulacrum Aug 27, 2019

ollie27 Aug 30, 2019

Mark-Simulacrum commented Aug 27, 2019

ollie27 Aug 30, 2019

GuillaumeGomez Aug 30, 2019

ollie27 Aug 30, 2019

Mark-Simulacrum Aug 30, 2019

GuillaumeGomez commented Aug 30, 2019

ollie27 commented Aug 31, 2019

Mark-Simulacrum commented Aug 31, 2019

GuillaumeGomez commented Sep 2, 2019

Mark-Simulacrum commented Sep 2, 2019

GuillaumeGomez commented Sep 2, 2019

Mark-Simulacrum commented Sep 2, 2019

GuillaumeGomez commented Sep 2, 2019

bors commented Sep 2, 2019

tesuji Sep 3, 2019

Mark-Simulacrum Sep 3, 2019

bors commented Sep 3, 2019

bors commented Sep 3, 2019

	(builder.config.dry_run \|\| up_to_date(&rustdoc, &html)) {
	builder.config.dry_run \|\|
	(up_to_date(&version_info, &html) && up_to_date(&rustdoc, &html)) {

Fix build failure in case file doesn't exist #63869

Fix build failure in case file doesn't exist #63869

Conversation

GuillaumeGomez commented Aug 24, 2019

GuillaumeGomez commented Aug 25, 2019

Mark-Simulacrum commented Aug 25, 2019

GuillaumeGomez commented Aug 27, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Mark-Simulacrum commented Aug 27, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

GuillaumeGomez commented Aug 30, 2019

ollie27 commented Aug 31, 2019

Mark-Simulacrum commented Aug 31, 2019

GuillaumeGomez commented Sep 2, 2019

Mark-Simulacrum commented Sep 2, 2019

GuillaumeGomez commented Sep 2, 2019

Mark-Simulacrum commented Sep 2, 2019

GuillaumeGomez commented Sep 2, 2019

bors commented Sep 2, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bors commented Sep 3, 2019

bors commented Sep 3, 2019