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

craft needs to be more flexible with github artifact names #552

Closed
joshuarli opened this issue Jul 31, 2024 · 4 comments
Closed

craft needs to be more flexible with github artifact names #552

joshuarli opened this issue Jul 31, 2024 · 4 comments
Assignees

Comments

@joshuarli
Copy link
Member

joshuarli commented Jul 31, 2024

github upload-artifact v3 is deprecated and v4 currently doesn't allow uploading different filepaths to the same artifact name

upstream issue: actions/upload-artifact#478

currently craft expects the artifact name to be github.sha

this blocks:

i propose a quick fix to check for this as a prefix if {sha} isn't found (so that we don't break things): craft-{sha}-

also, this is annoying but there are going to be multiple artifact ziparchives now with craft-{sha}- for example symbolicator will need the following, and current code assumes just one artifact per revision

  - /^symbolicator-Darwin-universal$/
  - /^symbolicator-Linux-x86_64$/
  - /^symbolicator-Linux-x86_64-debug.zip$/
  - /^symbolicator-aarch64-apple-darwin-debug.zip$/
  - /^symbolicator-x86_64-apple-darwin-debug.zip$/
  - /^symbolicli-Darwin-universal$/
  - /^symbolicli-Linux-x86_64$/
  - /^symbolicli-Windows-x86_64\.exe$/
  - /^symsorter-Darwin-universal$/
  - /^symsorter-Linux-x86_64$/
  - /^symsorter-Windows-x86_64\.exe$/
  - /^wasm-split-Darwin-universal$/
  - /^wasm-split-Linux-x86_64$/
  - /^wasm-split-Windows-x86_64\.exe$/
@BYK
Copy link
Member

BYK commented Aug 7, 2024

Although less than ideal, artifacts/merge action might be a temporary solution.

@joshuarli
Copy link
Member Author

joshuarli commented Sep 17, 2024

Although less than ideal, artifacts/merge action might be a temporary solution.

Thanks for showing me this, I'm trying it out here first: getsentry/sentry-python#3545

I think it's a good solution as it keeps craft simpler and I like the obvious idea of a unified artifact named sha. The additional overhead isn't bad: 7 second job here (2 MB artifact, 2 artifacts merged) https://github.com/getsentry/sentry-python/actions/runs/10909857400/job/30279025731?pr=3545

16s job here (400MB+ artifact, 6 artifacts merged) https://github.com/getsentry/relay/actions/runs/10910443909/job/30281749726?pr=4042

@joshuarli
Copy link
Member Author

Closing this as I've submitted multiple PRs to respective repos using actions/merge successfully.

joshuarli added a commit to getsentry/relay that referenced this issue Sep 18, 2024
Supersedes #3876 by using
artifacts/merge to merge together multiple artifacts from different jobs
into the single one named github.sha that craft expects.

Summarily, upload-artifact v3 is deprecated but v4 doesn't support
mutating an artifact with the name name by uploading different filepaths
to the same artifact. Because we need a single artifact "github.sha", we
have to use actions/merge to create it. Alternatively craft could be
modified but this is the easiest way forward and I like the idea of a
unified artifact, it makes craft simpler.

ref: getsentry/craft#552

#skip-changelog
@joshuarli
Copy link
Member Author

joshuarli commented Sep 23, 2024

todo (remaining ones that need upload-artifact merge):

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants