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

chore: create and attach binaries on tag, release #91

Merged
merged 2 commits into from
Aug 10, 2023

Conversation

rebornplusplus
Copy link
Member

@rebornplusplus rebornplusplus commented Jul 31, 2023

Requires #90.
(This PR includes commit 3e4ec0b from #90.)

Create binaries on push, tag creation and when a release has been published. Create an archive with the binary, and attach it to the release if a release is published. Attach the archive in the workflow run as an artifact in all cases.

The actions/checkout@v3 action overwrites annotated tag with lightweight tags breaking git describe. See [1], [2]. This commit adds a workaround to restore the latest tag to it's original state.

References:

  1. checkout overwrites tags with lightweight tags actions/checkout#882
  2. Preserve tag annotations actions/checkout#290

  • Have you signed the CLA?

@rebornplusplus
Copy link
Member Author

/cc @cjdcordeiro @woky @niemeyer

@rebornplusplus
Copy link
Member Author

Note: due to the nature of how the version is generated in Chisel right now (parsing from git describe --always output), new tags should be annotated.

Copy link
Collaborator

@cjdcordeiro cjdcordeiro left a comment

Choose a reason for hiding this comment

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

nice. looks clean and aligned with Pebble's workflows.

Do you have an example of a successful run?

Wrt the lightweight tags: what if a release is created from a lightweight tag? wouldn't this try to attach the binaries to the wrong release?

.github/workflows/build.yml Show resolved Hide resolved
@cjdcordeiro cjdcordeiro added the Blocked Waiting for something external label Aug 2, 2023
@rebornplusplus
Copy link
Member Author

Do you have an example of a successful run?

This is the test release on my fork: https://github.com/rebornplusplus/chisel/releases/tag/v1.0.1 (Notice the attached tarballs)
And the relevant workflow run: https://github.com/rebornplusplus/chisel/actions/runs/5747732205

Wrt the lightweight tags: what if a release is created from a lightweight tag? wouldn't this try to attach the binaries to the wrong release?

So if a release is published with lightweight tags, mkversion.sh as it is right now, will not pick up on the lightweight tags.

        v="$(git describe --always | sed -e 's/-/+git/;y/-/./' )"

To allow lightweight tags to contribute to version determination, we will need to pass the --tags option to git describe above. It's a simple fix. Let me know if you want me to include it in this PR.

@cjdcordeiro
Copy link
Collaborator

Thanks for the details @rebornplusplus!

Alright, so wrt the lightweight tags, that seems to be what we also do in Pebble -> https://github.com/canonical/pebble/blob/f5938105cb2611c7753f63beff5c71373ff0612b/cmd/mkversion.sh#L46

Unless you see a good reason why we should avoid releases that are based on lightweight tags, I think it would be safer to just add --tags to the version determination.

@rebornplusplus
Copy link
Member Author

Unless you see a good reason why we should avoid releases that are based on lightweight tags, I think it would be safer to just add --tags to the version determination.

Sounds good. I have updated it in c272db4. Please take another look.

Copy link
Collaborator

@cjdcordeiro cjdcordeiro left a comment

Choose a reason for hiding this comment

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

great, thanks ;)

Copy link
Member

@jnsgruk jnsgruk left a comment

Choose a reason for hiding this comment

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

One comment that could make things a little easier to read. Let me know, this is pretty much ready to go.

.github/workflows/build.yml Show resolved Hide resolved
Create binaries on push, tag creation and when a release
has been published. Create an archive with the binary, and
attach it to the release if a release is published. In all
cases, attach the archive in the workflow run as an artifact.

The actions/checkout@v3 action overwrites annonated tag with
lightweight tags breaking ``git describe``. See [1], [2].
This commit adds a workaround to restore the latest tag to
it's original state.

References:
- [1] actions/checkout#882
- [2] actions/checkout#290
@rebornplusplus
Copy link
Member Author

I have rebased the commits of this PR on the updated main branch since #90 is merged already. The only change is that 3e4ec0b was dropped from this PR as the changes in that commit are merged already. Sorry for any inconveniences.

@jnsgruk jnsgruk merged commit 4e81c83 into canonical:main Aug 10, 2023
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Blocked Waiting for something external
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants