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

ci: cross compile and publish #409

Closed
wants to merge 2 commits into from
Closed

Conversation

thesayyn
Copy link

@thesayyn thesayyn commented Jan 4, 2022

Addresses #332

@cyphar
Copy link
Member

cyphar commented Jan 4, 2022

You need to add a Signed-off-by: line to your commit(s) which indicates that you attest the Developer Certificate of Origin a statement about your contributions that you must read before signing (don't worry, it's quite short and easy-to-read). You can add it to your commits with git commit --amend -s, and then doing a git push --force.

NOTE: This is a saved reply. Sorry if it reads as a cookie-cutter response, it was written so that newcomers understand what a "DCO" is and make the process for contributing a little less scary.

@thesayyn
Copy link
Author

Hey @cyphar, just a friendly ping.

I have time to push this to get it landed. generated binaries seem to be working without any intervention. I have added platforms as I saw fit but I am afraid that we can’t support some of them due to the lack of a CI pipeline that tests for these platforms. Since you are the one who knows the codebase better could you take a look and point any errors that I am making? I would appreciate it a lot.

Signed-off-by: Sahin <[email protected]>
@thesayyn
Copy link
Author

thesayyn commented Feb 9, 2022

@cyphar or anyone. (sorry for pinging again). could you PTAL? now, this is the pure bash script and uses some of the bits from runc repository.

@thesayyn thesayyn changed the title ci: introduce goreleaser for cross compiling ci: cross compile and publish Feb 9, 2022
@thesayyn
Copy link
Author

thesayyn commented Feb 9, 2022

Remaining points:

  • Who is going to invoke scripts build and sign? Manually or by the CI.
  • If latter, CI has to have the gpg private key present on the machine (if we are going to use umoci.keyring) or one of its own.
  • If again the latter, what's the trigger for release workflow? the VERSION file or a push to refs/tag/x.x.x?

@codecov-commenter
Copy link

codecov-commenter commented Feb 23, 2022

Codecov Report

Merging #409 (16869a3) into main (30af8a3) will decrease coverage by 0.18%.
The diff coverage is 62.50%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #409      +/-   ##
==========================================
- Coverage   74.30%   74.12%   -0.19%     
==========================================
  Files          59       59              
  Lines        4783     4830      +47     
==========================================
+ Hits         3554     3580      +26     
- Misses        884      898      +14     
- Partials      345      352       +7     
Impacted Files Coverage Δ
oci/casext/mediatype/parse.go 74.73% <58.13%> (-14.74%) ⬇️
mutate/mutate.go 70.04% <100.00%> (+0.41%) ⬆️
oci/cas/dir/dir.go 59.28% <100.00%> (+0.74%) ⬆️
oci/layer/unpack.go 56.60% <0.00%> (-1.42%) ⬇️
cmd/umoci/new.go 91.78% <0.00%> (+0.11%) ⬆️

@cyphar
Copy link
Member

cyphar commented Feb 23, 2022

Sorry for not taking a look earlier, I was on leave for a while and there were other issues I had to work on. Thanks for reworking this to be a shell script. I will take a closer look at how this interacts with my release workflow.

At the moment I do the build and signing on my local machine. I did think about coming up with a more complicated setup for both runc and umoci (trigger a binary build in CI which then creates a release with the unsigned artifacts, then run a locally script which pulls the artefacts, signs them, then re-uploads). But I'm not sure the time spent getting such a thing to work would be worth it.

As an aside, in runc, I recently reworked the signing system so that the binaries are all built in a container (so we can cross-compile them) and a separate script does the signing. For runc we needed to use a container because we need to build libseccomp from scratch and only Debian lets you have a full cross-compiling environment for all the platforms we need for runc. Since umoci is pure-Go I suspect these problems don't affect us as much.

EDIT: Ah, I see you switched to my new scripts from runc. That should make this simpler to review. Thanks!

@thesayyn
Copy link
Author

thesayyn commented Feb 23, 2022

EDIT: Ah, I see you switched to my new scripts from runc. That should make this simpler to review. Thanks!

yep. mostly comes from there. just changed it in a way that could cross-compile for different os and arch pairs. also, naming of the binaries changed drastically so there might be people who are depending on linux/amd64 binary that is already part of the releases. so it might be a good idea to have two copies of linux/amd64 as umoci_linux_amd64 and umoci.amd64 so that we don't break any workflows out there downloading the binary with the old name.

But I'm not sure the time spent getting such a thing to work would be worth it.
these scripts would work for your local release process. I don't see a strong reason you should change your release process either.

Though, I believe it could be helpful to build unsigned (signed by CI) binaries for HEAD whenever HEAD moves forward. possibly as part of CI artifacts. this could come in handy for testing unstable features and fixes out there in the wild.
this could be a separate PR if you agree also.

Since umoci is pure-Go I suspect these problems don't affect us as much.

Yeap. we don't need a container at all. compilation happens on the host machine.

@thesayyn
Copy link
Author

thesayyn commented Apr 1, 2022

hey, @cyphar Sorry for pinging again. Did you have time to look at this?

@thesayyn thesayyn closed this Apr 2, 2023
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.

3 participants