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

build_helper::git uses the upstream/master branch to tell if a file has been modified, but this branch is never automatically updated. #129528

Closed
lolbinarycat opened this issue Aug 24, 2024 · 4 comments · Fixed by #129584
Assignees
Labels
T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap)

Comments

@lolbinarycat
Copy link
Contributor

lolbinarycat commented Aug 24, 2024

I have my master branch set up to track the master branch of my fork (mainly because i don't want to accidentally pull in every git tag into my shallow clone), so upstream/master doesn't updated when i run git pull (presumably the upstream remote was created by ./x setup? i don't remember creating it, and lots of code needs it (or another branch pointing to this repo) to work).

as it turns out, ./x fmt uses the state of upstream/master to determine if a file has been modified. so instead of it formatting every file that i modified, it formats every file anyone has modified in the last few months.

at no point did anything warn me that i need to be updating a remote i didn't add. it should probably do that.

@rustbot rustbot added the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Aug 24, 2024
@lolbinarycat
Copy link
Contributor Author

this could be addressed by checking the timestamp on .git/refs/remotes/upstream/master and warning if it is very old

@lolbinarycat
Copy link
Contributor Author

@rustbot claim

@nickrum
Copy link
Contributor

nickrum commented Aug 26, 2024

I ran into a similar issue where ./x build library was building llvm from source instead of using the prebuilt ci version due to an outdated upstream branch. Would it make sense to solve both issues in one go?

@lolbinarycat
Copy link
Contributor Author

@RickRum should be pretty easy, just call the same warning method for both build steps

maybe it should be used for all build steps?

@saethlin saethlin added T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) and removed needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. labels Sep 1, 2024
tgross35 added a commit to tgross35/rust that referenced this issue Sep 5, 2024
… r=albertlarsan68

warn the user if the upstream master branch is old

fixes rust-lang#129528
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Sep 5, 2024
… r=albertlarsan68

warn the user if the upstream master branch is old

fixes rust-lang#129528
@bors bors closed this as completed in 94e9c4c Sep 5, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this issue Sep 5, 2024
Rollup merge of rust-lang#129584 - lolbinarycat:old-upstream-warning, r=albertlarsan68

warn the user if the upstream master branch is old

fixes rust-lang#129528
github-actions bot pushed a commit to rust-lang/miri that referenced this issue Sep 6, 2024
…larsan68

warn the user if the upstream master branch is old

fixes rust-lang/rust#129528
tgross35 added a commit to tgross35/rust that referenced this issue Oct 12, 2024
force "HEAD" for non-CI and `git_upstream_merge_base` for CI environment

When rust-lang/rust is configured as remote, some of the git logic (for tracking changed files) that uses get_closest_merge_commit starts to produce annoying results as the upstream branch becomes outdated quickly (since it isn't updated with git pull). We can rely on HEAD for non-CI environments as we specifically treat bors commits as merge commits, which also exist on upstream. As for CI environments, we should use `git_upstream_merge_base` to correctly track modified files as bors commits may be in `HEAD` but not yet on the upstream remote.

This is also an alternative fix for rust-lang#129528 since rust-lang#131331 reverts the previous fix attempts.
tgross35 added a commit to tgross35/rust that referenced this issue Oct 13, 2024
force "HEAD" for non-CI and `git_upstream_merge_base` for CI environment

When rust-lang/rust is configured as remote, some of the git logic (for tracking changed files) that uses get_closest_merge_commit starts to produce annoying results as the upstream branch becomes outdated quickly (since it isn't updated with git pull). We can rely on HEAD for non-CI environments as we specifically treat bors commits as merge commits, which also exist on upstream. As for CI environments, we should use `git_upstream_merge_base` to correctly track modified files as bors commits may be in `HEAD` but not yet on the upstream remote.

This is also an alternative fix for rust-lang#129528 since rust-lang#131331 reverts the previous fix attempts.
rust-timer added a commit to rust-lang-ci/rust that referenced this issue Oct 13, 2024
Rollup merge of rust-lang#131358 - onur-ozkan:129528, r=Mark-Simulacrum

force "HEAD" for non-CI and `git_upstream_merge_base` for CI environment

When rust-lang/rust is configured as remote, some of the git logic (for tracking changed files) that uses get_closest_merge_commit starts to produce annoying results as the upstream branch becomes outdated quickly (since it isn't updated with git pull). We can rely on HEAD for non-CI environments as we specifically treat bors commits as merge commits, which also exist on upstream. As for CI environments, we should use `git_upstream_merge_base` to correctly track modified files as bors commits may be in `HEAD` but not yet on the upstream remote.

This is also an alternative fix for rust-lang#129528 since rust-lang#131331 reverts the previous fix attempts.
RalfJung pushed a commit to RalfJung/miri that referenced this issue Oct 14, 2024
force "HEAD" for non-CI and `git_upstream_merge_base` for CI environment

When rust-lang/rust is configured as remote, some of the git logic (for tracking changed files) that uses get_closest_merge_commit starts to produce annoying results as the upstream branch becomes outdated quickly (since it isn't updated with git pull). We can rely on HEAD for non-CI environments as we specifically treat bors commits as merge commits, which also exist on upstream. As for CI environments, we should use `git_upstream_merge_base` to correctly track modified files as bors commits may be in `HEAD` but not yet on the upstream remote.

This is also an alternative fix for rust-lang/rust#129528 since rust-lang/rust#131331 reverts the previous fix attempts.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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.

4 participants