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

Include extraction in the retrying for submodule download #106680

Conversation

pietroalbini
Copy link
Member

The code to download larger submodules previously used retries around the curl invocation to handle network failures, but we saw in recent build failures that failures can also happen during extraction, for example if a response got terminated early.

This commit moves the retry outwards, wrapping the whole download+extraction function in the retrying code. This means, if the extraction fails the tarball will be re-downloaded.

The code to download larger submodules previously used retries around
the `curl` invocation to handle network failures, but we saw in recent
build failures that failures can also happen during extraction, for
example if a response got terminated early.

This commit moves the retry outwards, wrapping the whole
download+extraction function in the retrying code. This means, if the
extraction fails the tarball will be re-downloaded.
@rustbot
Copy link
Collaborator

rustbot commented Jan 10, 2023

r? @jyn514

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

@rustbot rustbot added A-testsuite Area: The testsuite used to check the correctness of rustc S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-infra Relevant to the infrastructure team, which will review and decide on the PR/issue. labels Jan 10, 2023
@JohnTitor
Copy link
Member

@bors r+ p=51
To fix the CI failure

@bors
Copy link
Contributor

bors commented Jan 10, 2023

📌 Commit 48291f1 has been approved by JohnTitor

It is now in the queue for this repository.

@bors
Copy link
Contributor

bors commented Jan 10, 2023

🌲 The tree is currently closed for pull requests below priority 50. This pull request will be tested once the tree is reopened.

@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 Jan 10, 2023
@bors
Copy link
Contributor

bors commented Jan 10, 2023

⌛ Testing commit 48291f1 with merge b030ab677278e6cc1600e4cd54a87b55972e0a55...

@bors
Copy link
Contributor

bors commented Jan 10, 2023

💔 Test failed - checks-actions

@bors bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Jan 10, 2023
@pietroalbini
Copy link
Member Author

@bors retry

@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 Jan 10, 2023
@bors
Copy link
Contributor

bors commented Jan 10, 2023

⌛ Testing commit 48291f1 with merge 7b96e10b46410dd20cb6f8a39754a59ef62aa3f4...

@pietroalbini
Copy link
Member Author

@bors r-

Superseded by #106682

@bors bors 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 Jan 10, 2023
@pietroalbini pietroalbini deleted the pa-retry-submodule-checkout branch January 10, 2023 15:08
@rust-log-analyzer
Copy link
Collaborator

The job dist-mips64el-linux failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)
-- Performing Test CXX_SUPPORTS_FDATA_SECTIONS - Success
-- Looking for os_signpost_interval_begin
-- Looking for os_signpost_interval_begin - not found
-- Found Python3: /usr/bin/python3.10 (found suitable version "3.10.6", minimum required is "3.0") found components: Interpreter 
CMake Error: File /checkout/src/llvm-project/llvm/llvm.spec.in does not exist.
CMake Error at CMakeLists.txt:996 (configure_file):
  configure_file Problem configuring file

-- Linker detection: GNU ld
-- Performing Test HAS_WERROR_GLOBAL_CTORS
-- Performing Test HAS_WERROR_GLOBAL_CTORS - Failed
-- Performing Test HAS_WERROR_GLOBAL_CTORS - Failed
-- Looking for __x86_64__
-- Looking for __x86_64__ - found
CMake Error at CMakeLists.txt:1111 (add_subdirectory):
  add_subdirectory given source "utils/TableGen" which is not an existing


CMake Error at cmake/modules/TableGen.cmake:10 (message):
  LLVM_TABLEGEN_EXE not set
  LLVM_TABLEGEN_EXE not set
Call Stack (most recent call first):
  include/llvm/IR/CMakeLists.txt:2 (tablegen)


-- Configuring incomplete, errors occurred!
See also "/checkout/obj/build/x86_64-unknown-linux-gnu/llvm/build/CMakeFiles/CMakeOutput.log".
See also "/checkout/obj/build/x86_64-unknown-linux-gnu/llvm/build/CMakeFiles/CMakeError.log".
command did not execute successfully, got: exit status: 1


build script failed, must exit now', /cargo/registry/src/git.luolix.top-1ecc6299db9ec823/cmake-0.1.48/src/lib.rs:975:5
 finished in 6.994 seconds
Build completed unsuccessfully in 0:01:04

@rust-log-analyzer
Copy link
Collaborator

A job failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)

@fee1-dead
Copy link
Member

Stuck in queue?

@bors r-

@JohnTitor
Copy link
Member

IIRC bors commands on closed PRs don't work, I guess we have to synchronize the queue or reopen this to r- (it needs Pietro to restore the branch).

@fee1-dead
Copy link
Member

I've initiated the synchronization

@ehuss
Copy link
Contributor

ehuss commented Jan 11, 2023

@fee1-dead, when clicking the synchronize button, you need to go through the queue after it is finished and clean up things. Synchronize is fairly buggy. For example, if a PR was approved, but failed in CI, Synchronize will cause it to be re-approved. I think there are other issues like try builds getting converted to r+ in some cases. I went through and removed some erroneously approved PRs, but I did not check if any PRs got unapproved inadvertently since I didn't have the list of approved PRs before the synchronize.

@fee1-dead
Copy link
Member

thanks for the information, and sorry about not following up.. it was my first time doing a synchronize and I did not know the risks. It is probably best to change the confirmation message such that people will understand exactly what would happen after pushing the button.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-testsuite Area: The testsuite used to check the correctness of rustc S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. T-infra Relevant to the infrastructure team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants