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

Investigate if stamp logic in bootstrap could be consistently handled #134962

Open
jieyouxu opened this issue Dec 31, 2024 · 2 comments
Open

Investigate if stamp logic in bootstrap could be consistently handled #134962

jieyouxu opened this issue Dec 31, 2024 · 2 comments
Labels
A-bootstrap-stamp Area: bootstrap stamp logic C-cleanup Category: PRs that clean code up or issues documenting cleanup. E-needs-investigation Call for partcipation: This issues needs some investigation to determine current status T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap)

Comments

@jieyouxu
Copy link
Member

jieyouxu commented Dec 31, 2024

We were looking to revisit the tool_check_step! macro in check logic in #134950, but found that stamp logic was scattered. Different check steps (and probably other step kinds) might not have their stamp logic be consistently handled if only one stamp logic site is changed but not the others.

Change of stamp looks sus: the same stamps build in

.join(format!(".{}-check.stamp", stringify!($name).to_lowercase()));
too, but now they desync. I think it's simpler to revert it in current pr (and optionally rework stamps in separate pr, so they can be produces somehow centralized).

Originally posted by @klensy in #134950 (comment)

Might be valuable to double-check how stamps are handled throughout bootstrap, and possibly unify the stamp handling logic so they don't risk diverging.

@rustbot rustbot added the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Dec 31, 2024
@jieyouxu jieyouxu added C-cleanup Category: PRs that clean code up or issues documenting cleanup. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) E-needs-investigation Call for partcipation: This issues needs some investigation to determine current status and removed needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. labels Dec 31, 2024
@jieyouxu jieyouxu added the A-bootstrap-stamp Area: bootstrap stamp logic label Dec 31, 2024
@Zalathar
Copy link
Contributor

I suspect that some of these “stamps” are actually just an overcomplicated way to tell cargo which build directory to use, with the actual stamp itself never actually being read anywhere.

But I'm not yet confident enough to take action on that basis.

@Zalathar
Copy link
Contributor

In particular, note that the stamp path is used to derive target_root_dir, so a stamp path is needed (and a stamp is written) regardless of whether the stamp is actually needed for anything.

// `target_root_dir` looks like $dir/$target/release
let target_root_dir = stamp.parent().unwrap();

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-bootstrap-stamp Area: bootstrap stamp logic C-cleanup Category: PRs that clean code up or issues documenting cleanup. E-needs-investigation Call for partcipation: This issues needs some investigation to determine current status T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap)
Projects
None yet
Development

No branches or pull requests

3 participants