Skip to content
This repository has been archived by the owner on May 11, 2023. It is now read-only.

Binary versions with build metadata #242

Merged
merged 2 commits into from
Sep 9, 2022

Conversation

erak
Copy link
Contributor

@erak erak commented Sep 1, 2022

Closes #239.

@slack-coder
Copy link
Contributor

How about removing the 'vergen' package when things are finalized? It works, but introducing a new dependency comes at a cost.

@erak
Copy link
Contributor Author

erak commented Sep 2, 2022

Indeed, vergen comes with the cost of additional dependencies. Rebuilding this mechanism from scratch also comes with some cost though. We'd need to call a git subcommand, get the latest revision and, I think that's the tricky part, consistently create the correct Cargo config, such that the environment variable ends up being available to rad.

@slack-coder
Copy link
Contributor

👍 I can help out with this once we're settled on the requirements

@cloudhead
Copy link
Contributor

See: https://github.com/radicle-dev/radicle-client-services/blob/master/build.rs

@slack-coder
Copy link
Contributor

Here's the patch for bumping the version to 0.7.0-dev

erak and others added 2 commits September 7, 2022 22:17
@erak
Copy link
Contributor Author

erak commented Sep 7, 2022

@slack-coder I picked your commit, thanks! @cloudhead Together with that and the snippet you posted, the version is now being reported as e.g. rad 0.7.0-dev+cb2c115 for development builds. If it's a release build (version does not contain -dev), the sha is not added.

use std::process::Command;

fn main() {
let hash = Command::new("git")
Copy link
Contributor

@pinkforest pinkforest Sep 8, 2022

Choose a reason for hiding this comment

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

fwiw - I have a feeling this is / maybe going to be flaky and will / may sometimes break things and can potentially cause friction there and there. it would be better to have it in automated builds that have the appropriate handling for it across all the different builds. build.rs is useful e.g. with protoc for code generation but not necessarily for managing build metadata.
I mean there is a risk and generally I try to avoid build.rs as much as I possible can 🤷‍♀️

Copy link
Contributor Author

@erak erak Sep 8, 2022

Choose a reason for hiding this comment

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

Could you maybe elaborate on what you expect to be flaky? I think I'm just to new to build scripts and might be missing something here. What would be a more appropriate solution to this problem other then build scripts?

Besides automated builds, I think it should be possible to have a proper version output for local builds.

@pinkforest
Copy link
Contributor

pinkforest commented Sep 8, 2022

fwiw -

It's probably worthwhile to start using CHANGELOG.md using "standard" keepchangelog https://keepachangelog.com/en/1.0.0/

e.g. see example use: https://github.com/khonsulabs/bonsaidb/blob/main/CHANGELOG.md

And align on https://semver.org/spec/v2.0.0.html use.

As to the -dev etc. suffix(es) it's best generated in -nightly, -pr etc. automated "tagged" release-builds rather than off manifest / build.rs which should reflect the "base version" at master / main or the expected future release.

e.g. look nightly release builder:

https://github.com/pinkforest/test-auditable-releases

It makes nightly releases based on matrix using git tags pushing to timestmaped nightly ver.

https://github.com/pinkforest/test-auditable-releases/releases/tag/cargo-auditable%40nightly-20220830-0324

I have a separate repo for radicle-nightly but I'm tuning some of the target triplets that I need for testing:

https://github.com/pinkforest/rad-release/releases/tag/radicle-cli%40nightly-20220908-0511

When the CHANGELOG.md is using standardised form typical release tooling has pattern to extract it out automatically.

@erak
Copy link
Contributor Author

erak commented Sep 8, 2022

fwiw -

It's probably worthwhile to start using CHANGELOG.md using "standard" keepchangelog https://keepachangelog.com/en/1.0.0/

e.g. see example use: https://github.com/khonsulabs/bonsaidb/blob/main/CHANGELOG.md

Oh, yes. We should add a changelog. I've filed an issue tracking this: #249

And align on https://semver.org/spec/v2.0.0.html use.

As to the -dev etc. suffix(es) it's best generated in -nightly, -pr etc. automated "tagged" release-builds rather than off manifest / build.rs which should reflect the "base version" at master / main or the expected future release.

I agree with the base version here, but was under the impression that people were leaning towards this solution.

e.g. look nightly release builder:

https://github.com/pinkforest/test-auditable-releases

It makes nightly releases based on matrix using git tags pushing to timestmaped nightly ver.

https://github.com/pinkforest/test-auditable-releases/releases/tag/cargo-auditable%40nightly-20220830-0324

I have a separate repo for radicle-nightly but I'm tuning some of the target triplets that I need for testing:

https://github.com/pinkforest/rad-release/releases/tag/radicle-cli%40nightly-20220908-0511

When the CHANGELOG.md is using standardised form typical release tooling has pattern to extract it out automatically.

Cool, that's very valuable! I'm in favor of providing nightly builds. But as mentioned above, it should be possible to distinguish local builds as well. What would be your proposal here?

I think overall, I'd be in favor of finding a simple solution that works for now and can be changed later. I fear that starting the conversation around nightly builds will block this from being done. And we should also keep in mind, that with the project slowly moving away from Github, we'll lose CI for now.

@slack-coder
Copy link
Contributor

@pinkforest check the issue for the PR, it may be a good place to follow up so as not to fragment the discussion.

@slack-coder
Copy link
Contributor

slack-coder commented Sep 8, 2022

rad --version
# rad 0.7.0-dev+10dfad2

I like it :) this will help a lot with bug reports

@cloudhead cloudhead merged commit 10dfad2 into radicle-dev:master Sep 9, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support issue tracking through versioning
4 participants