-
Notifications
You must be signed in to change notification settings - Fork 77
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
feat: extra build artifacts #613
Conversation
79a01bd
to
dd0890e
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems right, just a question of the scope of the config
.github/workflows/release.yml
Outdated
@@ -59,7 +59,7 @@ jobs: | |||
with: | |||
submodules: recursive | |||
- name: Install cargo-dist | |||
run: "curl --proto '=https' --tlsv1.2 -LsSf https://github.com/axodotdev/cargo-dist/releases/download/v0.5.0/cargo-dist-installer.sh | sh" | |||
run: "cargo install --git https://github.com/axodotdev/cargo-dist/ --branch=extra-artifacts cargo-dist" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oroboros....
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Haha, yeah, just for testing. I'm removing this commit before merging.
cargo-dist/src/config.rs
Outdated
if extra_artifacts.is_some() { | ||
warn!("package.metadata.dist.extra_artifacts is set, but this is only accepted in workspace.metadata (value is being ignored): {}", package_manifest_path); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oh interesting, i would have assumed this was package-specific (vaguely recalls some discussion we had about this but forgets the conclusion)
eprintln!(); | ||
eprintln!("stdout:"); | ||
stderr().write_all(&result.stdout).into_diagnostic()?; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is a weird juxtaposition... but ok i think i see why
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is for consistency with the cargo builds, where we have things which consume the underlying build's stdout. I'd run into cases where allowing the output to go to stdout caused other parts of our build process to fail. (This code is actually from the original generic builds code, just duplicated here.)
build: extra.build.to_owned(), | ||
artifact: target_path.to_owned(), | ||
}), | ||
checksum: None, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hmm i imagine we'll add a flag for that one day
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I figured we could talk about what we wanted here and maybe add it later.
dd0890e
to
a1dd84b
Compare
cfc695a
to
a267a0f
Compare
This allows building and packaging arbitrary extra artifacts alongside code tarballs. For example, cargo-dist itself uses it for its own schema metadata JSON.
cf61ea8
to
60f5d13
Compare
This adds support for specifying arbitrary extra artifacts alongside the primary builds. These artifacts are currently expected to be architecture-independent and are uploaded as their own build artifacts rather than bundled into release tarballs. For example, we can use this to replace our current
append-release
code in to generate and upload thedist-manifest-schema.json
.I realized that most of our generic build code is very well-suited to this, so I split the existing generic builds into half. The code that runs the builds is now shared between this and the generic build path, while another section is separate for implementation detail reasons. There are two main distinctions between these and generic builds:
The syntax looks like this:
There can be arbitrary numbers of extra artifacts per build, so this is repeatable within a
Cargo.toml
/dist.toml
as many times as needed.Closes #349.