-
Notifications
You must be signed in to change notification settings - Fork 63
releng - don't assume the "helios" checkout origin destination #9281
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
base: main
Are you sure you want to change the base?
Conversation
"cargo xtask releng" will by default use a helios checkout found in the same directory as the omicron checkout from which it is run. It should not assume that such a checkout has its origin remote set to point to oxidecomputer when verifying if it is up to date, but rather explicitly test oxidecomputer/helios
dev-tools/releng/src/main.rs
Outdated
| // HEAD in a remote repository refers to the default | ||
| // branch, even if the default branch is renamed. | ||
| // `--no-write-fetch-head` avoids modifying FETCH_HEAD. | ||
| .args(["fetch", "--no-write-fetch-head", "origin", "HEAD"]) | ||
| .args(["fetch", HELIOS_REPO, "HEAD"]) | ||
| .ensure_success(&logger) | ||
| .await?; | ||
| let upstream_commit = git_resolve_commit( | ||
| &args.git_bin, | ||
| &args.helios_dir, | ||
| "origin/HEAD", | ||
| "FETCH_HEAD", | ||
| &logger, | ||
| ) | ||
| .await?; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The reason we used --no-write-fetch-head is to avoid overwriting FETCH_HEAD in the user's repository without telling them. Maybe we really want git ls-remote here?
$ git ls-remote --exit-code https://github.com/oxidecomputer/helios.git HEAD
25b5bf1a445524e698a7a1d095184d6a7ced16e4 HEAD
More of a pain to parse but avoids modifying the repo without the user knowing. I can implement this if you'd like.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm kind of used to FETCH_HEAD being fairly transient, but that could be because it gets updated whenever we pull a change in from gerrit to look at it etc.
Yes, please feel free to take this and implement it with ls-remote, which does look like a better option.
I just realised that quite a few of us don't have origin pointing to where you might expect in our $HOME/helios checkouts, and so we weren't benefitting from the out-of-date check.
"cargo xtask releng" will by default use a helios checkout found in the
same directory as the omicron checkout from which it is run. It should
not assume that such a checkout has its origin remote set to point to
oxidecomputer when verifying if it is up to date, but rather explicitly
test oxidecomputer/helios