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

Bootstrap shows unexpected warning: unable to check if origin/master is old due to error #131296

Closed
RalfJung opened this issue Oct 5, 2024 · 6 comments · Fixed by #131331
Closed
Labels
C-bug Category: This is a bug. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap)

Comments

@RalfJung
Copy link
Member

RalfJung commented Oct 5, 2024

I am getting this on each x.py invocation now, since very recently:

warning: unable to check if origin/master is old due to error: No such file or directory (os error 2)
warning: origin/master is used to determine if files have been modified
warning: if it is not updated, this may cause files to be needlessly reformatted

This is in my main checkout, not in a worktree, so I am not quite sure what could even be so unusual about my setup that causes an error here.

Cc @rust-lang/bootstrap

@rustbot rustbot added the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Oct 5, 2024
@RalfJung
Copy link
Member Author

RalfJung commented Oct 5, 2024

A bisect points at

cea707d96033d5e14d3c2b287b0bedd2847db680 is the first bad commit
commit cea707d96033d5e14d3c2b287b0bedd2847db680
Author: binarycat <binarycat@envs.net>
Date:   Tue Aug 27 16:31:40 2024 -0400

    emit old upstream warning no matter the build step

 src/bootstrap/src/core/build_steps/format.rs | 4 +---
 src/bootstrap/src/core/sanity.rs             | 6 ++++++
 2 files changed, 7 insertions(+), 3 deletions(-)

That's #129584. I am surprised about the date of this PR, I think I would have noticed this earlier if this happened for a month... so probably some other factor is also involved.

Cc @lolbinarycat

@RalfJung
Copy link
Member Author

RalfJung commented Oct 5, 2024

It seems to try to read .git/refs/remotes/origin/master which indeed is not a file that exists here.

$ tree .git/refs/
.git/refs/
├── bisect
├── heads
│   ├── immediate-offset-sanity-check
│   ├── master
│   └── master.2
├── remotes
│   └── origin
│       └── HEAD
└── tags

I also noticed this line in .git/packed-refs

f559d6188828b738ce7e7c2e4d99bf03111336d6 refs/remotes/origin/master

So maybe checking these files is just futile since git doesn't always use a file to track these refs?

@RalfJung
Copy link
Member Author

RalfJung commented Oct 5, 2024

I think what happened is that git gc has been run, and that indeed cleans up most of the files from .git/refs/remotes.

So the current approach in bootstrap of looking at these files is not reliable. IMO we should instead look at the commit date of the most recent commit in that branch. That also avoids having to directly mess with git's internal files.

@lolbinarycat
Copy link
Contributor

yeah, i'm gonna accept defeat on this one. git log is kinda slow on my device, but only if the relevant files aren't in the i/o cache, and building rust takes a good while anyways.

@saethlin saethlin added T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) C-bug Category: This is a bug. and removed needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. labels Oct 5, 2024
@onur-ozkan
Copy link
Member

#129528 is not the case anymore since refactor merge base logic and fix x fmt PR. I am planning to revert #130121 and #129584 as they are not really required.

@onur-ozkan
Copy link
Member

#129528 is not the case anymore since #130161.

Seems like it will still be the case if upstream was configured and too old. But that could be fixed in a much better way (e.g., we could ignore upstream commit and use merge commit in current branch) compare to #130121 and #129584.

@bors bors closed this as completed in 3a5a816 Oct 7, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this issue Oct 7, 2024
Rollup merge of rust-lang#131331 - onur-ozkan:131296, r=Kobzol

Revert "warn_old_master_branch" check

See rust-lang#131296 (comment).

Reverts rust-lang#130121 and rust-lang#129584.

Fixes rust-lang#131296 and rust-lang#131324.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: This is a bug. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants