Skip to content

fix: use --retry-connrefused in curl invocations of bootstrap download - #110245

Closed
kayagokalp wants to merge 1 commit into
rust-lang:masterfrom
kayagokalp:kayagokalp/110178
Closed

fix: use --retry-connrefused in curl invocations of bootstrap download#110245
kayagokalp wants to merge 1 commit into
rust-lang:masterfrom
kayagokalp:kayagokalp/110178

Conversation

@kayagokalp

@kayagokalpkayagokalp commented Apr 12, 2023•
edited by onur-ozkan
Loading

Copy link
Copy Markdown

helps with #110178.

@rustbot

rustbot commented Apr 12, 2023

Copy link
Copy Markdown
Collaborator

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @ozkanonur (or someone else) soon.

Please see the contribution instructions for more information. Namely, in order to ensure the minimum review times lag, PR authors and assigned reviewers should ensure that the review label (S-waiting-on-review and S-waiting-on-author) stays updated, invoking these commands when appropriate:

  • @rustbot author: the review is finished, PR author should check the comments and take action accordingly
  • @rustbot review: the author is ready for a review, this PR will be queued again in the reviewer's queue

@rustbotrustbot 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 Apr 12, 2023
@kayagokalpkayagokalp changed the title fix: change curl args from --retry to --retry-all-errorsfix: use --retry-all-errors instead of --retry in curl invocations of bootstrap downloadApr 12, 2023
@kayagokalp
kayagokalp marked this pull request as ready for review April 12, 2023 17:29
onur-ozkan
onur-ozkan suggested changes Apr 12, 2023
View reviewed changes

@onur-ozkanonur-ozkan left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks for the PR!

--retry-all-errors should be an extra argument with keeping --retry as it is. Otherwise curl will hang the downloading processes until the timeout.

e.g:
2023-04-12_21-52

@onur-ozkan

onur-ozkan commented Apr 12, 2023

Copy link
Copy Markdown
Contributor

@rustbot author

@rustbotrustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Apr 12, 2023
@kayagokalp

kayagokalp commented Apr 12, 2023

Copy link
Copy Markdown
Author

@rustbot label -S-waiting-on-author +S-waiting-on-review

@rustbotrustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Apr 12, 2023
@onur-ozkan

onur-ozkan commented Apr 12, 2023

Copy link
Copy Markdown
Contributor

this lgtm, once you squash your commits, we can merge the PR

@onur-ozkan

onur-ozkan commented Apr 12, 2023

Copy link
Copy Markdown
Contributor

@bors r+ rollup

@bors

bors commented Apr 12, 2023

Copy link
Copy Markdown
Collaborator

📌 Commit b065eef has been approved by ozkanonur

It is now in the queue for this repository.

@borsbors 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 Apr 12, 2023
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Apr 13, 2023
@matthiaskrgr
Rollup merge ofrust-lang#110245- kayagokalp:kayagokalp/110178, r=oz…
2585de5
…kanonur
fix: use `--retry-all-errors` instead of `--retry` in curl invocations of bootstrap download
closesrust-lang#110178.
@matthiaskrgrmatthiaskrgr mentioned this pull request Apr 13, 2023
Closed
@matthiaskrgr

matthiaskrgr commented Apr 13, 2023

Copy link
Copy Markdown
Member

@bors r-
failed in a rollup
#110271

@borsbors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Apr 13, 2023
@klensy

klensy commented Apr 13, 2023•
edited
Loading

Copy link
Copy Markdown
Contributor

Looks like --retry-all-errors supported from curl 7.71.0 and some runners older than that?

ubuntu 20.04 image uses curl 7.68 https://packages.ubuntu.com/focal/curl

@onur-ozkan

onur-ozkan commented Apr 13, 2023

Copy link
Copy Markdown
Contributor

Looks like --retry-all-errors supported from curl 7.71.0 and some runners older than that?

ubuntu 20.04 image uses curl 7.68 https://packages.ubuntu.com/focal/curl

Yeah seems like --retry-all-errors flag shipped very recently.

@kayagokalp Can you replace the --retry-all-errors flag with --retry-connrefused and update commit message/PR description? --retry-connrefused added since v7.52.0, which should work on any CI/local machine.

@onur-ozkanonur-ozkan mentioned this pull request Apr 13, 2023
Closed
@kayagokalpkayagokalp changed the title fix: use --retry-all-errors instead of --retry in curl invocations of bootstrap downloadfix: use --retry-connrefused in curl invocations of bootstrap downloadApr 13, 2023
@kayagokalp

kayagokalp commented Apr 13, 2023

Copy link
Copy Markdown
Author

@rustbot ready

@rustbotrustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Apr 13, 2023
onur-ozkan
onur-ozkan reviewed Apr 13, 2023
View reviewed changes
Comment threadsrc/bootstrap/bootstrap.py Outdated
@onur-ozkan

onur-ozkan commented Apr 13, 2023

Copy link
Copy Markdown
Contributor

@bors r+ rollup

@bors

bors commented Apr 13, 2023

Copy link
Copy Markdown
Collaborator

📌 Commit fa28d22 has been approved by ozkanonur

It is now in the queue for this repository.

@borsbors 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 Apr 13, 2023
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Apr 14, 2023
@matthiaskrgr
Rollup merge ofrust-lang#110245- kayagokalp:kayagokalp/110178, r=oz…
9376441
…kanonur
fix: use `--retry-connrefused` in curl invocations of bootstrap download
helps with rust-lang#110178.
@matthiaskrgrmatthiaskrgr mentioned this pull request Apr 14, 2023
Closed
@matthiaskrgr

matthiaskrgr commented Apr 14, 2023

Copy link
Copy Markdown
Member

@bors r- rollup=iffy still failing

@borsbors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Apr 14, 2023
@klensy

klensy commented Apr 14, 2023

Copy link
Copy Markdown
Contributor

centos 7 image have curl 7.29 :-(. It's easier to download our own version and install/build it.

@onur-ozkan

onur-ozkan commented Apr 14, 2023

Copy link
Copy Markdown
Contributor

centos 7 image have curl 7.29 :-(.

7.52 released in 2016, I was pretty sure that we have more recent version...

"curl version 7.29. 0 was released on February 6 2013." :(

It's easier to download our own version and install/build it.

Doing custom build or installation just for couple new arguments, it doesn't really seem worth to me.

cc @jyn514

@onur-ozkan

onur-ozkan commented Apr 14, 2023

Copy link
Copy Markdown
Contributor

@rustbot blocked

@rustbotrustbot added S-blocked Status: Blocked on something else such as an RFC or other implementation work. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Apr 14, 2023
@jyn514

jyn514 commented Apr 14, 2023

Copy link
Copy Markdown
Member

Doing custom build or installation just for couple new arguments, it doesn't really seem worth to me.

i agree. instead we can put the retry logic in Rust (and make sure not to retry if it's a 404).

@jyn514jyn514 added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-blocked Status: Blocked on something else such as an RFC or other implementation work. labels Apr 15, 2023
@kayagokalp

kayagokalp commented Apr 20, 2023

Copy link
Copy Markdown
Author

Going to close this for now, and re ignite once I got some time again if no one beats mo to it!

@klensyklensy mentioned this pull request May 31, 2023
Closed
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@onur-ozkanonur-ozkanAwaiting requested review from onur-ozkan

Assignees

@onur-ozkanonur-ozkan

Labels

S-waiting-on-authorStatus: This is awaiting some action (such as code changes or more information) from the author.T-bootstrapRelevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap)

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

@kayagokalp@rustbot@onur-ozkan@bors@matthiaskrgr@klensy@jyn514