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

add builder-config into tarball sources #128822

Merged
merged 2 commits into from
Aug 9, 2024

Conversation

onur-ozkan
Copy link
Member

@onur-ozkan onur-ozkan commented Aug 8, 2024

This will be useful for certain scenarios where developers want to know how the tarball sources were generated. We also want this to check for CI rustc incompatible options on bootstrap.

Blocker for #122709

r? Kobzol

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) labels Aug 8, 2024
@rustbot
Copy link
Collaborator

rustbot commented Aug 8, 2024

This PR modifies src/bootstrap/src/core/config.

If appropriate, please update CONFIG_CHANGE_HISTORY in src/bootstrap/src/utils/change_tracker.rs.

Copy link
Contributor

@Kobzol Kobzol left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that somehow comparing the config options from CI is a good idea. This is a start, although it won't take into account options configured using the --set flags, right?

src/bootstrap/src/utils/tarball.rs Outdated Show resolved Hide resolved
@onur-ozkan onur-ozkan force-pushed the add-build-config-in-tarballs branch from 79a925f to 9d21178 Compare August 8, 2024 12:52
@onur-ozkan
Copy link
Member Author

This is a start, although it won't take into account options configured using the --set flags, right?

--set flag is used with configure script which creates the config.toml.

@Kobzol
Copy link
Contributor

Kobzol commented Aug 8, 2024

I meant this.

@onur-ozkan
Copy link
Member Author

onur-ozkan commented Aug 8, 2024

I meant this.

Oh, yeah that is ignored. I don't think it's important tho. We could save the invoked command into some file but I don't think it's really needed.

@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

@onur-ozkan onur-ozkan force-pushed the add-build-config-in-tarballs branch from 4237e5d to b65d779 Compare August 8, 2024 14:51
Copy link
Contributor

@Kobzol Kobzol left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think that we should add the config file to all the tarballs, even those that we publicly distribute to users. It is an implementation detail, and more importantly it makes it harder to reproduce the archives, especially the source code archive - that should only contain things that are in git, so that it can be perfectly reproduced to have the same SHA.

Could we only add it to the internal component(s) that need it, so for now RustcDev?

src/bootstrap/src/core/config/config.rs Show resolved Hide resolved
@onur-ozkan onur-ozkan force-pushed the add-build-config-in-tarballs branch from b65d779 to e1d04ac Compare August 9, 2024 08:36
@onur-ozkan onur-ozkan changed the title add config.toml into tarball sources add builder-config into tarball sources Aug 9, 2024
@onur-ozkan onur-ozkan force-pushed the add-build-config-in-tarballs branch from e1d04ac to ff0d37c Compare August 9, 2024 08:38
This will be useful for certain scenarios where developers want to know
how the tarball sources were generated. We also want this to check for CI
rustc incompatible options on bootstrap.

Signed-off-by: onur-ozkan <work@onurozkan.dev>
Signed-off-by: onur-ozkan <work@onurozkan.dev>
@Kobzol
Copy link
Contributor

Kobzol commented Aug 9, 2024

Clarification: the config file isn't added to bare tarballs (such as the source code archive), so it should be fine to add it unconditionally.

@bors r+

@bors
Copy link
Contributor

bors commented Aug 9, 2024

📌 Commit ff0d37c has been approved by Kobzol

It is now in the queue for this repository.

@bors 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 Aug 9, 2024
GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this pull request Aug 9, 2024
…balls, r=Kobzol

add `builder-config` into tarball sources

This will be useful for certain scenarios where developers want to know how the tarball sources were generated. We also want this to check for CI rustc incompatible options on bootstrap.

Blocker for rust-lang#122709

r? Kobzol
bors added a commit to rust-lang-ci/rust that referenced this pull request Aug 9, 2024
…iaskrgr

Rollup of 9 pull requests

Successful merges:

 - rust-lang#128815 (Add `Steal::is_stolen()`)
 - rust-lang#128817 (VxWorks code refactored )
 - rust-lang#128822 (add `builder-config` into tarball sources)
 - rust-lang#128838 (rustdoc: do not run doctests with invalid langstrings)
 - rust-lang#128852 (use stable sort to sort multipart diagnostics)
 - rust-lang#128859 (Fix the name of signal 19 in library/std/src/sys/pal/unix/process/process_unix/tests.rs for mips/sparc linux)
 - rust-lang#128864 (Use `SourceMap::end_point` instead of `- BytePos(1)` in arg removal suggestion)
 - rust-lang#128865 (Ensure let stmt compound assignment removal suggestion respect codepoint boundaries)
 - rust-lang#128874 (Disable verbose bootstrap command failure logging by default)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 5f1e25e into rust-lang:master Aug 9, 2024
6 checks passed
@rustbot rustbot added this to the 1.82.0 milestone Aug 9, 2024
bors added a commit to rust-lang-ci/rust that referenced this pull request Aug 9, 2024
…llaumeGomez

Rollup of 8 pull requests

Successful merges:

 - rust-lang#128815 (Add `Steal::is_stolen()`)
 - rust-lang#128822 (add `builder-config` into tarball sources)
 - rust-lang#128838 (rustdoc: do not run doctests with invalid langstrings)
 - rust-lang#128852 (use stable sort to sort multipart diagnostics)
 - rust-lang#128859 (Fix the name of signal 19 in library/std/src/sys/pal/unix/process/process_unix/tests.rs for mips/sparc linux)
 - rust-lang#128864 (Use `SourceMap::end_point` instead of `- BytePos(1)` in arg removal suggestion)
 - rust-lang#128865 (Ensure let stmt compound assignment removal suggestion respect codepoint boundaries)
 - rust-lang#128874 (Disable verbose bootstrap command failure logging by default)

r? `@ghost`
`@rustbot` modify labels: rollup
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Aug 9, 2024
Rollup merge of rust-lang#128822 - onur-ozkan:add-build-config-in-tarballs, r=Kobzol

add `builder-config` into tarball sources

This will be useful for certain scenarios where developers want to know how the tarball sources were generated. We also want this to check for CI rustc incompatible options on bootstrap.

Blocker for rust-lang#122709

r? Kobzol
@onur-ozkan onur-ozkan deleted the add-build-config-in-tarballs branch August 11, 2024 05:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants