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

ci: Build gccjit from a git archive #122658

Merged
merged 3 commits into from
Mar 24, 2024
Merged

Conversation

cuviper
Copy link
Member

@cuviper cuviper commented Mar 17, 2024

A full git clone of GCC includes quite a lot of history, and it's
completely unnecessary for building it in CI. We can use a GitHub
archive URL to get a simple tarball that is much faster to download.

Also, the gcc-build directory can be removed after install to reduce
the image size even further.

A full `git clone` of GCC includes quite a lot of history, and it's
completely unnecessary for building it in CI. We can use a GitHub
archive URL to get a simple tarball that is much faster to download.

Also, the `gcc-build` directory can be removed after install to reduce
the image size even further.
@rustbot
Copy link
Collaborator

rustbot commented Mar 17, 2024

r? @Mark-Simulacrum

rustbot has assigned @Mark-Simulacrum.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@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 Mar 17, 2024
# This commit hash needs to be updated to use a more recent gcc fork version.
git checkout 78dc50f0e50e6cd1433149520bd512a4e0eaa1bc
curl -L "$GIT_REPO/archive/$GIT_COMMIT.tar.gz" |
tar -xz --transform "s/gcc-$GIT_COMMIT/gcc-src/"
Copy link
Member Author

Choose a reason for hiding this comment

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

If for some reason we do want git metadata on disk, AFAIK shallow clones are not possible with specific commit hashes, but you can get there with git init; git fetch --depth 1 $repo $commit; git checkout $commit.

cd gcc-src
# This commit hash needs to be updated to use a more recent gcc fork version.
git checkout 78dc50f0e50e6cd1433149520bd512a4e0eaa1bc
curl -L "$GIT_REPO/archive/$GIT_COMMIT.tar.gz" |
Copy link
Member

Choose a reason for hiding this comment

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

We should probably also retry the download if it fails for some reason.

Suggested change
curl -L "$GIT_REPO/archive/$GIT_COMMIT.tar.gz" |
retry curl -L "$GIT_REPO/archive/$GIT_COMMIT.tar.gz" |

Copy link
Member Author

Choose a reason for hiding this comment

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

I think it's not that simple -- if a partial download fails and restarts, the latter | tar is not likely to fare well. We could download to disk first, or wrap the pipeline in a bash function to be retried. But I wonder if this is useful, because if a GitHub CI runner can't download from github.com, we likely have bigger issues...

Copy link
Member

Choose a reason for hiding this comment

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

The issue here is not so much the download but the transient DNS failures, they happened in the past, even for github.com (can't find a example right now).

Sure, we may want to download first to disk; I didn't look to closely at the retry function.

Copy link
Member

Choose a reason for hiding this comment

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

I wouldn't worry about it. The whole docker build is already retried 5 times IIRC. This is maybe nice cleanup in the future but isn't worth it for now.

@Mark-Simulacrum
Copy link
Member

@bors r+ rollup

@bors
Copy link
Contributor

bors commented Mar 23, 2024

📌 Commit f512f9e has been approved by Mark-Simulacrum

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 Mar 23, 2024
@Mark-Simulacrum
Copy link
Member

@bors rollup=iffy (actually)

matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Mar 23, 2024
…ulacrum

ci: Build gccjit from a git archive

A full `git clone` of GCC includes quite a lot of history, and it's
completely unnecessary for building it in CI. We can use a GitHub
archive URL to get a simple tarball that is much faster to download.

Also, the `gcc-build` directory can be removed after install to reduce
the image size even further.
bors added a commit to rust-lang-ci/rust that referenced this pull request Mar 23, 2024
…iaskrgr

Rollup of 9 pull requests

Successful merges:

 - rust-lang#116016 (Soft-destabilize `RustcEncodable` & `RustcDecodable`, remove from prelude in next edition)
 - rust-lang#122460 (Rework rmake support library API)
 - rust-lang#122658 (ci: Build gccjit from a git archive)
 - rust-lang#122698 (Cancel `cargo update` job if there's no updates)
 - rust-lang#122878 (Use `arch::wasm::unreachable` instead of `arch::wasm32::unreachable`)
 - rust-lang#122915 (Delay a bug if no RPITITs were found)
 - rust-lang#122916 (docs(sync): normalize dot in fn summaries)
 - rust-lang#122921 (Enable more mir-opt tests in debug builds)
 - rust-lang#122922 (-Zprint-type-sizes: print the types of awaitees and unnamed coroutine locals.)

r? `@ghost`
`@rustbot` modify labels: rollup
bors added a commit to rust-lang-ci/rust that referenced this pull request Mar 24, 2024
…acrum

ci: Build gccjit from a git archive

A full `git clone` of GCC includes quite a lot of history, and it's
completely unnecessary for building it in CI. We can use a GitHub
archive URL to get a simple tarball that is much faster to download.

Also, the `gcc-build` directory can be removed after install to reduce
the image size even further.
@bors
Copy link
Contributor

bors commented Mar 24, 2024

⌛ Testing commit f512f9e with merge 7af7db4...

@bors
Copy link
Contributor

bors commented Mar 24, 2024

💥 Test timed out

@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 Mar 24, 2024
@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)

@Mark-Simulacrum
Copy link
Member

@bors retry apple-aarch64 timed out

@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 Mar 24, 2024
@bors
Copy link
Contributor

bors commented Mar 24, 2024

⌛ Testing commit f512f9e with merge 0824b30...

@bors
Copy link
Contributor

bors commented Mar 24, 2024

☀️ Test successful - checks-actions
Approved by: Mark-Simulacrum
Pushing 0824b30 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Mar 24, 2024
@bors bors merged commit 0824b30 into rust-lang:master Mar 24, 2024
12 checks passed
@rustbot rustbot added this to the 1.79.0 milestone Mar 24, 2024
@rust-timer
Copy link
Collaborator

Finished benchmarking commit (0824b30): comparison URL.

Overall result: ✅ improvements - no action needed

@rustbot label: -perf-regression

Instruction count

This is a highly reliable metric that was used to determine the overall result at the top of this comment.

mean range count
Regressions ❌
(primary)
- - 0
Regressions ❌
(secondary)
- - 0
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
-3.2% [-3.2%, -3.2%] 1
All ❌✅ (primary) - - 0

Max RSS (memory usage)

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

mean range count
Regressions ❌
(primary)
2.0% [1.5%, 2.3%] 3
Regressions ❌
(secondary)
- - 0
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
-4.5% [-5.9%, -1.7%] 3
All ❌✅ (primary) 2.0% [1.5%, 2.3%] 3

Cycles

This benchmark run did not return any relevant results for this metric.

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: 670.98s -> 670.209s (-0.11%)
Artifact size: 315.04 MiB -> 315.07 MiB (0.01%)

@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)
##[endgroup]
Secret source: Actions
Prepare workflow directory
Prepare all required actions
Complete job name: skip if S-waiting-on-bors
##[group]Run # Fetch state and labels of PR
# Fetch state and labels of PR
# Or exit successfully if PR does not exist
JSON=$(gh pr view cargo_update --repo $GITHUB_REPOSITORY --json labels,state || exit 0)
STATE=$(echo "$JSON" | jq -r '.state')
WAITING_ON_BORS=$(echo "$JSON" | jq '.labels[] | any(.name == "S-waiting-on-bors"; .)')

# Exit with error if open and S-waiting-on-bors
if [[ "$STATE" == "OPEN" && "$WAITING_ON_BORS" == "true" ]]; then
  gh run cancel 8413221908
shell: /usr/bin/bash --noprofile --norc -e -o pipefail {0}

@cuviper cuviper deleted the gccjit-archive branch April 11, 2024 16:05
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 merged-by-bors This PR was explicitly merged by bors. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. 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.

7 participants