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

Update upload-artifact to v4 #122951

Merged
merged 1 commit into from Mar 25, 2024
Merged

Update upload-artifact to v4 #122951

merged 1 commit into from Mar 25, 2024

Conversation

Nilstrieb
Copy link
Member

This contains a breaking change around artifact merging no longer being done. This was not relied on, so it's fine.

This contains a breaking change around artifact merging no longer being
done. This was not relied on, so it's fine.
@rustbot
Copy link
Collaborator

rustbot commented Mar 23, 2024

r? @Kobzol

rustbot has assigned @Kobzol.
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 23, 2024
@Kobzol
Copy link
Contributor

Kobzol commented Mar 23, 2024

Hmm, it seems to me that name: "${{ env.DOC_ARTIFACT_NAME }}" might be the same across different jobs, which could cause an error in v4. Anyway, the easiest way to find out is probably to try to merge the change.

@bors r+ rollup=never

@bors
Copy link
Contributor

bors commented Mar 23, 2024

📌 Commit 24a0d7d has been approved by Kobzol

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
@Nilstrieb
Copy link
Member Author

I downloaded the artifact from the last master run and it only contained a single tarball (im the zip, haha).

@Kobzol
Copy link
Contributor

Kobzol commented Mar 23, 2024

Yeah, which looks like the jobs overwrite the files. Which is weird, given that they should be combined. But from the workflow it looks like all/most jobs should upload it (?).

@Nilstrieb
Copy link
Member Author

I looked through some job logs and the ones I checked didn't actually upload it as there weren't any files.

@Kobzol
Copy link
Contributor

Kobzol commented Mar 23, 2024

Maybe it is actually uploaded only by a single job. But in that case we should document that better, it's not at all obvious to me why that happens.

@Nilstrieb
Copy link
Member Author

yeah, it's a bit confusing.

@bors
Copy link
Contributor

bors commented Mar 25, 2024

⌛ Testing commit 24a0d7d with merge fa374a8...

@bors
Copy link
Contributor

bors commented Mar 25, 2024

☀️ Test successful - checks-actions
Approved by: Kobzol
Pushing fa374a8 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Mar 25, 2024
@bors bors merged commit fa374a8 into rust-lang:master Mar 25, 2024
12 checks passed
@rustbot rustbot added this to the 1.79.0 milestone Mar 25, 2024
@Nilstrieb Nilstrieb deleted the nodejs20 branch March 25, 2024 06:47
@rust-timer
Copy link
Collaborator

Finished benchmarking commit (fa374a8): comparison URL.

Overall result: no relevant changes - no action needed

@rustbot label: -perf-regression

Instruction count

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

Max RSS (memory usage)

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

Cycles

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)
- - 0
Regressions ❌
(secondary)
- - 0
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
-2.2% [-2.2%, -2.2%] 1
All ❌✅ (primary) - - 0

Binary size

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

Bootstrap: 670.312s -> 671.665s (0.20%)
Artifact size: 315.10 MiB -> 315.02 MiB (-0.03%)

@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 8416913615
shell: /usr/bin/bash --noprofile --norc -e -o pipefail {0}

@Nilstrieb
Copy link
Member Author

@clubby789

@ehuss
Copy link
Contributor

ehuss commented Mar 26, 2024

This is unfortunately causing a confusing message on every merged PR due to rust-lang/rust-log-analyzer#81.

@ehuss
Copy link
Contributor

ehuss commented Mar 26, 2024

Is the problem with the dependencies workflow that the download-artifact action also needs to be updated to v4?

@Kobzol
Copy link
Contributor

Kobzol commented Mar 26, 2024

I'm not sure if it's related, the Cargo update workflow started failing a few hours before this PR was merged (https://github.com/rust-lang/rust/actions/workflows/dependencies.yml).

matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Mar 26, 2024
…=Kobzol

Revert `cargo update` changes and bump `download-artifact` to v4

Revert rust-lang#122489 and rust-lang#122698

rust-lang#122951 (comment)
The failures + rust-lang/rust-log-analyzer#81 are causing some annoying spam. I don't think this is _that_ important for now and I don't know enough GHA to fix it 😓
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Mar 26, 2024
Rollup merge of rust-lang#123069 - clubby789:un-often-cargo-update, r=Kobzol

Revert `cargo update` changes and bump `download-artifact` to v4

Revert rust-lang#122489 and rust-lang#122698

rust-lang#122951 (comment)
The failures + rust-lang/rust-log-analyzer#81 are causing some annoying spam. I don't think this is _that_ important for now and I don't know enough GHA to fix it 😓
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.

None yet

7 participants