Skip to content

add --remote-time flag to curl for bootstrap #111771

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
May 20, 2023
Merged

add --remote-time flag to curl for bootstrap #111771

merged 1 commit into from
May 20, 2023

Conversation

liushuyu
Copy link
Contributor

This pull request sets the timestamp of the downloaded stage0 binary according to the server-reported timestamp (if possible).
This allows make_orig-dl_tarball.sh to be more reproducible on the filesystem.

@rustbot
Copy link
Collaborator

rustbot commented May 19, 2023

r? @ozkanonur

(rustbot has picked a reviewer for you, use r? to override)

@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 May 19, 2023
@Mark-Simulacrum
Copy link
Member

make_orig-dl_tarball.sh

What is this script? Is there a reason for reproducible tarballs you're not just setting all times to the unix epoch or so?

@liushuyu
Copy link
Contributor Author

What is this script?

That's the script from Debian. I believe I need to re-phrase the commit message since I was just forwarding the potentially upstreamable patches there.

Is there a reason for reproducible tarballs you're not just setting all times to the unix epoch or so?

I think it's not setting the times to the Unix epoch because it differentiates different binaries used so that they will have a different timestamp. Debian uses make to build stuff; this could also be a hint to make where the binaries changed.

onur-ozkan
onur-ozkan approved these changes May 19, 2023
View reviewed changes
Copy link
Member

@onur-ozkan onur-ozkan left a comment

Choose a reason for hiding this comment

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

LGTM.

-R flag was added to curl back in 2001. I think oldest curl version we are using is 7.29(released at 2013), so shipping this PR shouldn't cause any trouble on CI runners.

I believe I need to re-phrase the commit message since I was just forwarding the potentially upstreamable patches there.

Yes, please. The current commit message and description are not providing us with much assistance.

@onur-ozkan onur-ozkan changed the title Set the timestamp of downloaded stage0 files add --remote-timeflag to curl for bootstrap May 19, 2023
@onur-ozkan onur-ozkan changed the title add --remote-timeflag to curl for bootstrap add --remote-time flag to curl for bootstrap May 19, 2023
@liushuyu liushuyu force-pushed the ubuntu/rep-stage0 branch 2 times, most recently from e37d148 to d646591 Compare May 19, 2023 22:39
@liushuyu
Copy link
Contributor Author

Yes, please. The current commit message and description are not providing us with much assistance.

Thank you for the feedback! I have modified the commit message to (hopefully) reflect the changes better.

@Mark-Simulacrum
Copy link
Member

I believe there are other curl commands we use, unless I'm forgetting, so we should try to update those too.

@infinity0 @liushuyu
Set the timestamp of downloaded stage0 files ...
0d7d2ca 
... using server-reported timestamp.
This allows us to track changes to the downloaded artifact more easily
and in a more reproducible manner.

Co-authored-by: Zixing Liu <[email protected]>
@liushuyu liushuyu force-pushed the ubuntu/rep-stage0 branch from d646591 to 0d7d2ca Compare May 20, 2023 00:50
@liushuyu
Copy link
Contributor Author

I believe there are other curl commands we use, unless I'm forgetting, so we should try to update those too.

I have run an rg '\bcurl\b' on the source tree and found other usages. I added the changes to src/bootstrap/download.rs in the commit, but some are inside src/ci/, which do not need updating, I suppose?

@onur-ozkan
Copy link
Member

I believe there are other curl commands we use, unless I'm forgetting, so we should try to update those too.

I have run an rg '\bcurl\b' on the source tree and found other usages. I added the changes to src/bootstrap/download.rs in the commit, but some are inside src/ci/, which do not need updating, I suppose?

src/ci doesn't need this update

@onur-ozkan
Copy link
Member

@bors r+ rollup

@bors
Copy link
Collaborator

bors commented May 20, 2023

📌 Commit 0d7d2ca has been approved by ozkanonur

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 May 20, 2023
@GuillaumeGomez GuillaumeGomez mentioned this pull request May 20, 2023
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request May 20, 2023
@bors
Auto merge of rust-lang#111798 - GuillaumeGomez:rollup-t3bus8o, r=Gui…
6bacf5a 
…llaumeGomez

Rollup of 5 pull requests

Successful merges:

 - rust-lang#111450 (Use `OpaqueTypeKey` in query response)
 - rust-lang#111726 (Migrate GUI colors test to original CSS color format)
 - rust-lang#111746 (Merge some query impl modules into one)
 - rust-lang#111765 (Migrate GUI colors test to original CSS color format)
 - rust-lang#111771 (add `--remote-time` flag to curl for bootstrap)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit a6ebaf8 into rust-lang:master May 20, 2023
@rustbot rustbot added this to the 1.71.0 milestone May 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@onur-ozkan onur-ozkan onur-ozkan approved these changes

Assignees

@onur-ozkan onur-ozkan

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
Milestone
1.71.0
Development

Successfully merging this pull request may close these issues.
6 participants
@liushuyu @rustbot @Mark-Simulacrum @onur-ozkan @bors @infinity0