Skip to content

Conversation

@sourcefrog
Copy link
Contributor

@sourcefrog sourcefrog commented Sep 7, 2021

This makes downloadable artifacts out of the whole build tree and install results for every Check run. The build tree is perhaps useful for understanding build failures. The install results gives us compiled binaries and libraries that can be directly installed, which is particularly useful for platforms like Windows.

@sourcefrog sourcefrog changed the title Upload artifacts Upload build artifacts from Github actions Sep 7, 2021
@sourcefrog sourcefrog requested a review from dbaarda September 7, 2021 15:03
@sourcefrog sourcefrog marked this pull request as ready for review September 7, 2021 15:03
Seems to include api fingerprints?
This avoids a warning for the no-rdiff matrix case
and lets us make it an error for other cases
Copy link
Member

@dbaarda dbaarda left a comment

Choose a reason for hiding this comment

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

I have a few thoughts on this...

with:
name: rdiff ${{ matrix.os }} ${{ matrix.build }}
path: |
${{github.workspace}}/build/rdiff
Copy link
Member

Choose a reason for hiding this comment

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

Note that unless its a static build, the rdiff executable will also need the librsync library to run, which is either the .so (and related symlinks) on linux or the .dll on windows.

But perhaps, rather than manually selecting the bits to put in the artifact, perhaps we should do a cmake --build '${{github.workspace}}/build' --target install and then build an artifact out of everything that is installed. That way we should get the executables, supporting libraries, required headers to use those libraries, and supporting man pages.

That also means we don't need to selectively exclude the no-rdiff case, as we probably also want to build/check the artifact you get from installing only the library without rdiff.

I'm not sure what happens on windows when you run a 'cmake --target install`, and it wouldn't surprise me if it's broken. However, this is probably just something else we need to fix.

The bad news is my experience with trying to make windows work for the github actions was kinda painful, but we are pretty close now to a working windows setup. At least github actions runs nice and fast for testing all this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I wonder if just building a self-contained static executable would be more helpful for people who want the executable?

Copy link
Member

Choose a reason for hiding this comment

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

We now have install artifacts for all builds that we test. Currently, we only test static linking on Linux, so we don't have a static executable for windows, only a dynamic one. We could add another static build on windows, but I don't know if we need one.

@dbaarda
Copy link
Member

dbaarda commented Sep 8, 2021

The artifacts thing is interesting... I'm finding out more and more about what can be done with github actions and it's pretty cool. I'm starting to think about maybe having actions that automate much of the release process too.

One thing that worries me a little bit is... this is all a bit TOO cool... building/keeping all these artefacts for every checkin is surely going to eat resources. What are the freebie limits on all this? Is there some kind of retention-horizon or quota limit on these?

It would be nice to extend the release instructions in CONTRIBUTING.md to include uploading the windows binary as another "asset", the same way we grab a snapshot of the release tarball. People on Windows are far more likely to need a precompiled binary. I can update the CONTRIBUTING.md after we get this working nicely.

@sourcefrog
Copy link
Contributor Author

One thing that worries me a little bit is... this is all a bit TOO cool... building/keeping all these artefacts for every checkin is surely going to eat resources. What are the freebie limits on all this? Is there some kind of retention-horizon or quota limit on these?

They're kept for 90 days by default: https://docs.github.com/en/actions/reference/usage-limits-billing-and-administration#artifact-and-log-retention-policy

@sourcefrog sourcefrog self-assigned this Sep 8, 2021
@sourcefrog
Copy link
Contributor Author

The artifacts thing is interesting... I'm finding out more and more about what can be done with github actions and it's pretty cool. I'm starting to think about maybe having actions that automate much of the release process too.

One thing that worries me a little bit is... this is all a bit TOO cool... building/keeping all these artefacts for every checkin is surely going to eat resources. What are the freebie limits on all this? Is there some kind of retention-horizon or quota limit on these?

It would be nice to extend the release instructions in CONTRIBUTING.md to include uploading the windows binary as another "asset", the same way we grab a snapshot of the release tarball. People on Windows are far more likely to need a precompiled binary. I can update the CONTRIBUTING.md after we get this working nicely.

Automating the release process sounds like a great idea, not only to reduce toil but also to move further towards https://security.googleblog.com/2021/06/introducing-slsa-end-to-end-framework.html

Adds a "Build install" step and changes "Upload rdiff" to "Upload install" instead. Removed the "no_rdiff" setting since we always upload the install, rdiff or not.
@dbaarda
Copy link
Member

dbaarda commented Sep 15, 2021

I changed this to include the "build install" step and made the rdiff artifact into an "install" artifact. I also added the "options" dimension to the artifact name, so we can see all the build and install results.

I'm happy with this now... do you want to check it and see?

How do you want to handle the merge? Normal merge, squash merge, rebase merge, or some kind of manual cleanup of the history? I'm not that fussed either way.

@dbaarda dbaarda changed the title Upload build artifacts from Github actions Upload build and install artifacts from Github actions Sep 15, 2021
Copy link
Member

@dbaarda dbaarda left a comment

Choose a reason for hiding this comment

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

I think this is ready to merge... happy to let you do this in whatever way you think is best.

@dbaarda
Copy link
Member

dbaarda commented Sep 16, 2021

If I don't hear otherwise, I'm going to do a "Squash and merge" of this PR tomorrow.

@dbaarda
Copy link
Member

dbaarda commented Sep 17, 2021

I'm going to squash and merge this now.

@dbaarda dbaarda merged commit 0f7199e into librsync:master Sep 17, 2021
@sourcefrog sourcefrog deleted the upload-artifacts branch September 18, 2021 20:27
@sourcefrog
Copy link
Contributor Author

Thanks!

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

Successfully merging this pull request may close these issues.

2 participants