Skip to content

rustbuild seems to deal badly with poor internet connections #32834

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

Closed
durka opened this issue Apr 8, 2016 · 14 comments
Closed

rustbuild seems to deal badly with poor internet connections #32834

durka opened this issue Apr 8, 2016 · 14 comments
Labels
E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap)

Comments

@durka
Copy link
Contributor

durka commented Apr 8, 2016

I tried to build rust with rustbuild.

./configure --enable-ccache --enable-rustbuild
make

But after a few seconds it fails downloading the nightly with errno 104, which is connection reset by peer. If I run it again it sees the file is there and fails saying the checksum is wrong. I guess it needs to retry the download.

@alexcrichton alexcrichton added E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) labels Apr 8, 2016
@alexcrichton
Copy link
Member

Ah we probably need to download to a temporary location and only when done move it over into where it's expected to be, that way we'll restart any failed download.

@cyplo
Copy link
Contributor

cyplo commented Apr 11, 2016

Hi ! Anyone interested in taking this ? If not I'd like to try.

@GuillaumeGomez
Copy link
Member

@cyplo: Sure. Take it.

@alexcrichton
Copy link
Member

If you have any questions, feel free to ask me @cyplo!

@cyplo
Copy link
Contributor

cyplo commented Apr 12, 2016

Hi @alexcrichton, using your invite to ask here ;)

I took a quick look there and it seems that the initial download is done via bootstrap.py and it seems that the fix can be done entirely within that file.

Do you maybe know if there any other downloads, not done during the bootstrap.py phase that would need this treatment or other reasons to touch something beyond bootstrap.py in the fix for this issue ?

Thanks a lot !

@alexcrichton
Copy link
Member

Yeah I believe that's the only location where we download anything. There's other downloads that Cargo does for the actual compilation of the build system itself but Cargo handles bad internet and things like that internally already.

@cyplo
Copy link
Contributor

cyplo commented Apr 16, 2016

Hi ! I see that #32926 is merged now, I'm thinking on how to integrate the download resume/retry with it. @alexcrichton / @caipre Let me know what do you think of the following:

  • try downloading to a temp location
  • check checksum there
  • if invalid - remove and retry max N times
  • if valid - move to destination

@GuillaumeGomez
Copy link
Member

GuillaumeGomez commented Apr 16, 2016

Isn't it possible to check checksum before downloading? It seems to be overkill otherwise (downloading something to just get "no, you're up to date finally" is really annoying). Or at least check the date or something equivalent which could prevent to download for nothing?

@cyplo
Copy link
Contributor

cyplo commented Apr 16, 2016

Yes, totally, it would probably be like this:

  • if there is no file - download
  • check checksum
  • invalid - retry from scratch
  • valid - proceed

thanks !

@caipre
Copy link
Contributor

caipre commented Apr 16, 2016

The logic to check whether the file exists and needs updated is already present:

if self.rustc().startswith(self.bin_root()) and \
(not os.path.exists(self.rustc()) or self.rustc_out_of_date()):

My PR added the checksumming within the get() method, so you shouldn't have any work on that point. I'm not sure we need a retry mechanism. I prefer to just report the problem and let the user decide what to do next.

For this issue, you need to change the path that the tarball is downloaded to to use some temporary path, then move that to the correct path after the download is complete.

An analogy to shell:

curl -o file.tmp https://... && mv file.tmp file

I suppose you might want to update the checksumming to be done on the temporary file, and only if that succeeds do the move.

@alexcrichton
Copy link
Member

Yeah @caipre's thoughts are exactly my own, let's download to a temp location, verify the checksum, and once both are done we move it to the final location.

@cyplo
Copy link
Contributor

cyplo commented Apr 24, 2016

Hey ! I need some more time to get to PR, as I'm quite tight on time lately. If it's not critical/needed quickly - I will come up with PR eventually. If it is - feel free to grab it.

bors added a commit that referenced this issue May 8, 2016
Get a file during bootstrap to a temp location first.

When downloading a file in the bootstrap phase - get it to a temp
location first. Verify it there and only if downloaded properly move it
to the `cache` directory.

This should prevent `make` being stuck if the download was interrupted
or otherwise corrupted, as per discussion in #32834

The temporary files are deleted in case of an exception.

I was looking for some unit/integration tests around this and couldn't find any - presumably because this is being tested by just Travis launching it ? Let me know if it would be good to try to write tests around this. Thanks !
@cyplo
Copy link
Contributor

cyplo commented May 9, 2016

Hi ! @durka: the fix should be merged now, could you see if this helps ? thanks !

@alexcrichton
Copy link
Member

Closing as #33288 has landed, but feel free to reopen if troubles still happen!

(thanks again @cyplo!)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. 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

5 participants