-
Notifications
You must be signed in to change notification settings - Fork 815
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
[nix] Add nix derivation for static builds #932
Conversation
@saschagrunert anyway I couldn't make the
Any idea? |
Thanks for the PR. What is the end-user benefit for shipping this in the Skopeo repository? I know basically nothing about Nix, but from a first look I’d expect Nix users to benefit far more from https://github.com/NixOS/nixpkgs/blob/master/pkgs/development/tools/skopeo/default.nix , where they can use a hopefully fully-maintained package using the default setup. Having a separate upstream packaging (with maintainers who, AFAIK, don’t use Nix), mostly just confuses the situation and adds overhead to any packaging work, it seems to me. What benefit am I missing? (Also, as discussed elsewhere in this repo, static builds are mostly a bad idea — the PR description already mentions the compromises that had to be made — and now that there is an automatically built container image (per #930 and the like), having more kinds of static builds adds the number of ways users can make difficult for themselves.) |
As an end user (Ansible Role developer for skopeo installation, support Ubuntu 18.04+ / CentOS 7+ / openSUSE 15.1 / Debian 10 / Fedora 32, https://github.com/alvistack/ansible-role-skopeo) I am looking for:
Today my answer for above is: install and compile skopeo manually on all of my supported OS (https://github.com/alvistack/ansible-role-skopeo/blob/5a89c9721665f79eb9bd28423af0b43f133058d4/tasks/main.yml#L64-L103). Yes this looks really silly and time consuming :-( What if we have a static binary? Case study from crun could be a good example:
Other else benefit with nix could be:
End user direct benefit could also be:
|
…
OK, that one is hard. Other than that, I would say “install the distribution’s package”, but that only works if the distribution did package that particular version.
If the “precompile” step is out of scope, that could plausibly use some other tools. E.g. use
That would only work if the newbies never need help with Nix or any Nix-specific aspect, because the CI and testing does not use it, and the primary development happens without Nix. In my personal experience, using “full” native installations and IDEs (which can use Go-specific tools like finding uses/references of a symbol, not just dumb editors) makes the programming part of development so much easier that even builds in isolated containers are not worth it if they make using IDEs harder. |
With nix derivation we "pin" to specific git rev, which result as all dependency with a specific version, therefore result binary will ALWAYS identical, even in sha256 checksum (!!) across different base OS when compile (@saschagrunert please correct me). With Podman based if the base image updated with newer dependency library the result binary will surely become different, which is difficult to reproduce the same binary for debug, if we don't cache/upload/backup previous static binary to somewhere else. Moreover, for Podman toolchain (crun/conmon/skopeo/buildah/podman/cri-o):
As a member of Podman toolchain, IMHO we should avoid the chicken or the egg issue (i.e. install podman from repo in order to compile a component of podman by running a container with podman)...
Again this is just a chicken or the egg issue; BTW after we support nix derivation inside our source code, newbie could start by:
Now skopeo newbie contributor could create PR and go; reviewer could build it quickly in local for debug; CI could build and always testing with the same binary with same sha256. P.S. I am a nix newbie for only 2 weeks experience, but now I could give a hand for PR with @saschagrunert kindly help ;-) |
That’s nice (and reproducibility is certainly useful for auditing), but most users just should not be compiling software to install it; they should download a built version. That amortizes the compilation overhead, removes the need to install large build dependencies, and gives them just as stable a binary for running and debugging — after all your own workflow is to compile the binary once and distribute it. People who want a reproducible binary can use the https://github.com/NixOS/nixpkgs/blob/master/pkgs/development/tools/skopeo/default.nix one, and that will benefit from maintenance by Nix experts, instead of, probably, “benevolent neglect” in this repository. Typically, it’s by far most convenient to use distribution-packaged software, because it is tracked automatically, updated without any extra care.
That’s more or less the same thing as “not currently happening”. Along with the IDE aspects and things like $GOPATH (although that might all be rapidly improving, I don’t know), I don’t expect Skopeo development to be primarily Nix-focused in the foreseeable future. So, I guess we are still at the fairly special case of “I want exactly the same version of Skopeo, including possibly development versions that have not been packaged, to run on many different systems”. I hope that’s a rare need, but I may well be naive WRT how buggy Skopeo is in production. Reading back in containers/podman#5566 , the reaction was similarly reluctant, with “make it clear that it is unsupported” as a primary concern. Happy to hear from @saschagrunert , of course. |
Generally the nix-based dynamic linked compile will be handled by nix's upstream (just similar as kubic package supported by kubic team); the nix derivation in this PR is just focus on static binary. And yes we should recommend people to kubic as first-class support, where nix-based static binary with trimmed functionality for quick deploy and debug, and compile from source for PR. There is always no single solution that could fit for all cases ;-) |
(I'll give it a look next week with respect to the build failures and the concerns. Just wanted to show-up that I noticed the ongoing discussion) |
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.
Alright I would leave the decision up to the maintainers if we really want to have this in skopeo. It really looks like that static binaries are wanted (at least for CRI-O for example) by the community, but I can’t prove that for sure.
When speaking about building static binaries then I would always prefer nix than an alpine image for example, because the pinned dependencies provide a much more stable build environment.
d2e76a3
to
1a93463
Compare
Compile now successful: https://github.com/alvistack/skopeo/releases/download/v1.0.0/skopeo-v1.0.0-linux-amd64
Static binaries:
Therefore could focus on CI/CD fixes ;-) |
1a93463
to
4cb7aa1
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.
LGTM
@saschagrunert core logic should be good enough, but need some input for making CI/CD happy, hope you may share some hints for me ;-) |
@hswong3i Still working on this? |
4cb7aa1
to
3b00167
Compare
@rhatdan No changes required since #932 (review), just rebase with master and updated with
|
@hswong3i Tests are failing |
a6a1259
to
6bb0196
Compare
2380: [nix] Add nix derivation for static builds r=rhatdan a=hswong3i #### What type of PR is this? <!-- Please label this pull request according to what type of issue you are addressing, especially if this is a release targeted pull request. Uncomment only one `/kind <>` line, hit enter to put that in a new line, and remove leading whitespace from that line: --> > /kind feature #### What this PR does / why we need it: Similar PR goes for crun/conmon/libpod/cri-o/etc, too. Also see: - ~~containers/crun#372 - ~~containers/conmon#161 - containers/skopeo#932 - #2380 - containers/podman#6402 - cri-o/cri-o#3804 Static binaries: - [crun-0.13-linux-amd64](https://github.com/alvistack/crun/releases/download/0.13/crun-0.13-linux-amd64) - [conmon-v2.0.17-linux-amd64](https://github.com/alvistack/conmon/releases/download/v2.0.17/conmon-v2.0.17-linux-amd64) - [skopeo-v1.0.0-linux-amd64](https://github.com/alvistack/skopeo/releases/download/v1.0.0/skopeo-v1.0.0-linux-amd64) - [buildah-v1.14.9-linux-amd64](https://github.com/alvistack/buildah/releases/download/v1.14.9/buildah-v1.14.9-linux-amd64) - [podman-v1.9.3-linux-amd64](https://github.com/alvistack/libpod/releases/download/v1.9.3/podman-v1.9.3-linux-amd64) - [cri-o-v1.17.4-linux-amd64.tar.gz](https://github.com/alvistack/cri-o/releases/download/v1.17.4/cri-o-v1.17.4-linux-amd64.tar.gz) - [cri-o-v1.18.1-linux-amd64.tar.gz](https://github.com/alvistack/cri-o/releases/download/v1.18.1/cri-o-v1.18.1-linux-amd64.tar.gz) Ansible Roles: - https://github.com/alvistack/ansible-role-crun - https://github.com/alvistack/ansible-role-conmon - https://github.com/alvistack/ansible-role-skopeo - https://github.com/alvistack/ansible-role-buildah - https://github.com/alvistack/ansible-role-podman - https://github.com/alvistack/ansible-role-cri_o #### How to verify it ``` nix build -f nix/ ``` #### Which issue(s) this PR fixes: <!-- Automatically closes linked issue when PR is merged. Uncomment the following comment block and include the issue number or None on one line. Usage: `Fixes #<issue number>`, or `Fixes (paste link of issue)`, or `None`. --> <!-- Fixes # or None --> #### Special notes for your reviewer: Here I skip the btrfs and lvm2 support for static binary, because: 1. btrfs will not support in CentOS 8 2. With skopeo experience both btrfs and lvm2 are not easy for compile as static binary Also see: - containers/podman#6402 (comment) #### Does this PR introduce a user-facing change? <!-- If no, just write `None` in the release-note block below. If yes, a release note is required: Enter your extended release note in the block below. If the PR requires additional action from users switching to the new release, include the string "action required". For more information on release notes please follow the kubernetes model: https://git.k8s.io/community/contributors/guide/release-notes.md --> ```release-note ``` Co-authored-by: Wong Hoi Sing Edison <[email protected]>
Signed-off-by: Wong Hoi Sing Edison <[email protected]>
6bb0196
to
0f458ee
Compare
2380: [nix] Add nix derivation for static builds r=rhatdan a=hswong3i #### What type of PR is this? <!-- Please label this pull request according to what type of issue you are addressing, especially if this is a release targeted pull request. Uncomment only one `/kind <>` line, hit enter to put that in a new line, and remove leading whitespace from that line: --> > /kind feature #### What this PR does / why we need it: Similar PR goes for crun/conmon/libpod/cri-o/etc, too. Also see: - ~~containers/crun#372 - ~~containers/conmon#161 - containers/skopeo#932 - #2380 - containers/podman#6402 - cri-o/cri-o#3804 Static binaries: - [crun-0.13-linux-amd64](https://github.com/alvistack/crun/releases/download/0.13/crun-0.13-linux-amd64) - [conmon-v2.0.17-linux-amd64](https://github.com/alvistack/conmon/releases/download/v2.0.17/conmon-v2.0.17-linux-amd64) - [skopeo-v1.0.0-linux-amd64](https://github.com/alvistack/skopeo/releases/download/v1.0.0/skopeo-v1.0.0-linux-amd64) - [buildah-v1.14.9-linux-amd64](https://github.com/alvistack/buildah/releases/download/v1.14.9/buildah-v1.14.9-linux-amd64) - [podman-v1.9.3-linux-amd64](https://github.com/alvistack/libpod/releases/download/v1.9.3/podman-v1.9.3-linux-amd64) - [cri-o-v1.17.4-linux-amd64.tar.gz](https://github.com/alvistack/cri-o/releases/download/v1.17.4/cri-o-v1.17.4-linux-amd64.tar.gz) - [cri-o-v1.18.1-linux-amd64.tar.gz](https://github.com/alvistack/cri-o/releases/download/v1.18.1/cri-o-v1.18.1-linux-amd64.tar.gz) Ansible Roles: - https://github.com/alvistack/ansible-role-crun - https://github.com/alvistack/ansible-role-conmon - https://github.com/alvistack/ansible-role-skopeo - https://github.com/alvistack/ansible-role-buildah - https://github.com/alvistack/ansible-role-podman - https://github.com/alvistack/ansible-role-cri_o #### How to verify it ``` nix build -f nix/ ``` #### Which issue(s) this PR fixes: <!-- Automatically closes linked issue when PR is merged. Uncomment the following comment block and include the issue number or None on one line. Usage: `Fixes #<issue number>`, or `Fixes (paste link of issue)`, or `None`. --> <!-- Fixes # or None --> #### Special notes for your reviewer: Here I skip the btrfs and lvm2 support for static binary, because: 1. btrfs will not support in CentOS 8 2. With skopeo experience both btrfs and lvm2 are not easy for compile as static binary Also see: - containers/podman#6402 (comment) #### Does this PR introduce a user-facing change? <!-- If no, just write `None` in the release-note block below. If yes, a release note is required: Enter your extended release note in the block below. If the PR requires additional action from users switching to the new release, include the string "action required". For more information on release notes please follow the kubernetes model: https://git.k8s.io/community/contributors/guide/release-notes.md --> ```release-note ``` Co-authored-by: Wong Hoi Sing Edison <[email protected]>
Sigh, this broke c/image CI. |
... which changed Make targets. Signed-off-by: Miloslav Trmač <[email protected]>
... which no longer works after containers#932. This does not add documentation for the current static build approach, nor does it add any other place where DISABLE_CGO is documented; both are not tested by CI, and discouraged due to bad integration with the rest of the system. Signed-off-by: Miloslav Trmač <[email protected]>
... which no longer works after #932. This does not add documentation for the current static build approach, nor does it add any other place where DISABLE_CGO is documented; both are not tested by CI, and discouraged due to bad integration with the rest of the system. Signed-off-by: Miloslav Trmač <[email protected]>
* When we can't store signatures, point the user at the destination. * Update for containers/skopeo#932 * Refactor configPath API * Load the rootless registries.conf.d for override * docker config: clean up after test * blobinfocache: clean up after test * enable search using pagination * pkg/docker/config: correct default file mode when create auth.json file * Update to Go 1.13 * Coverity found potential nil dereference * Look for normalized paths in tarfile. * Move docker/tarfile.Destination to docker/internal/tarfile.Destination * Use the docker/internal/tarfile.Destination from docker/daemon and docker/archive * Remove deprecated non-SystemContext functions from docker/internal.tarfile * Introduce Destination.configPath and Destination.physicalLayerPath * Split docker/internal.tarfile.Writer from Destination * Move createRepositoriesFile to a bit better place * Split Writer.createManifest from Destination.PutManifest * Reorganize docker/internal/tarfile.Writer.createManifest a bit * Move the computation of layerPaths in docker-archive * Implement writing multiple images in the modern format. * Split createSingleLegacyLayer from writeLegacyLayerMetadata * Move legacy layer ID computation to a bit later * Merge writeLegacyMetadata and createRepositoriesFile * Implement writing multiple images in the legacy format * Separate tarfile.Writer creation from Destination creation * Lock docker/internal/tarfile.Writer to support concurrent uses * Split openArchiveForWriting from docker/archive/newImageDestination * Finally, introduce docker/archive.Writer * use container/storage/pkg/homedir * Fix an error message on docker-archive:path:name@sha256:$digest * Move docker/tarfile.Source to docker/internal/tarfile.Source * Use the docker/internal/tarfile.Source from docker/daemon and docker/archive * Remove deprecated non-SystemContext functions from docker/internal/tarfile * Split docker/internal/tarfile.Reader from Source * Separate tarfile.Reader creation from Source creation * Read the tarfile manifest already when initializing tarfile.Reader * Turn tarfile.Source.LoadTarManifest into a TarManifest * Allow choosing an image from tarfile.Reader by reference * Introduce docker-archive:path:@index syntax for reading untagged images * Introduce docker/archive.Reader * Finally, share a tarfile.Reader across archiveSource objects * Add docker/archive.NewReaderForReference * Add docker/archive.Reader.ManifestTagsForReference * Support per user registries.d * Move TestInvalidPolicyFormatError * Reduce duplication in policy_config_test.go * Eliminate more duplication in signature/policy_config_tests.go * Return error body if UnexpectedHTTPResponseError * Set NoLchown to true in untar opts Signed-off-by: Miloslav Trmač <[email protected]>
* When we can't store signatures, point the user at the destination. * Update for containers/skopeo#932 * Refactor configPath API * Load the rootless registries.conf.d for override * docker config: clean up after test * blobinfocache: clean up after test * enable search using pagination * pkg/docker/config: correct default file mode when create auth.json file * Update to Go 1.13 * Coverity found potential nil dereference * Look for normalized paths in tarfile. * Move docker/tarfile.Destination to docker/internal/tarfile.Destination * Use the docker/internal/tarfile.Destination from docker/daemon and docker/archive * Remove deprecated non-SystemContext functions from docker/internal.tarfile * Introduce Destination.configPath and Destination.physicalLayerPath * Split docker/internal.tarfile.Writer from Destination * Move createRepositoriesFile to a bit better place * Split Writer.createManifest from Destination.PutManifest * Reorganize docker/internal/tarfile.Writer.createManifest a bit * Move the computation of layerPaths in docker-archive * Implement writing multiple images in the modern format. * Split createSingleLegacyLayer from writeLegacyLayerMetadata * Move legacy layer ID computation to a bit later * Merge writeLegacyMetadata and createRepositoriesFile * Implement writing multiple images in the legacy format * Separate tarfile.Writer creation from Destination creation * Lock docker/internal/tarfile.Writer to support concurrent uses * Split openArchiveForWriting from docker/archive/newImageDestination * Finally, introduce docker/archive.Writer * use container/storage/pkg/homedir * Fix an error message on docker-archive:path:name@sha256:$digest * Move docker/tarfile.Source to docker/internal/tarfile.Source * Use the docker/internal/tarfile.Source from docker/daemon and docker/archive * Remove deprecated non-SystemContext functions from docker/internal/tarfile * Split docker/internal/tarfile.Reader from Source * Separate tarfile.Reader creation from Source creation * Read the tarfile manifest already when initializing tarfile.Reader * Turn tarfile.Source.LoadTarManifest into a TarManifest * Allow choosing an image from tarfile.Reader by reference * Introduce docker-archive:path:@index syntax for reading untagged images * Introduce docker/archive.Reader * Finally, share a tarfile.Reader across archiveSource objects * Add docker/archive.NewReaderForReference * Add docker/archive.Reader.ManifestTagsForReference * Support per user registries.d * Move TestInvalidPolicyFormatError * Reduce duplication in policy_config_test.go * Eliminate more duplication in signature/policy_config_tests.go * Return error body if UnexpectedHTTPResponseError * Set NoLchown to true in untar opts Signed-off-by: Miloslav Trmač <[email protected]>
* When we can't store signatures, point the user at the destination. * Update for containers/skopeo#932 * Refactor configPath API * Load the rootless registries.conf.d for override * docker config: clean up after test * blobinfocache: clean up after test * enable search using pagination * pkg/docker/config: correct default file mode when create auth.json file * Update to Go 1.13 * Coverity found potential nil dereference * Look for normalized paths in tarfile. * Move docker/tarfile.Destination to docker/internal/tarfile.Destination * Use the docker/internal/tarfile.Destination from docker/daemon and docker/archive * Remove deprecated non-SystemContext functions from docker/internal.tarfile * Introduce Destination.configPath and Destination.physicalLayerPath * Split docker/internal.tarfile.Writer from Destination * Move createRepositoriesFile to a bit better place * Split Writer.createManifest from Destination.PutManifest * Reorganize docker/internal/tarfile.Writer.createManifest a bit * Move the computation of layerPaths in docker-archive * Implement writing multiple images in the modern format. * Split createSingleLegacyLayer from writeLegacyLayerMetadata * Move legacy layer ID computation to a bit later * Merge writeLegacyMetadata and createRepositoriesFile * Implement writing multiple images in the legacy format * Separate tarfile.Writer creation from Destination creation * Lock docker/internal/tarfile.Writer to support concurrent uses * Split openArchiveForWriting from docker/archive/newImageDestination * Finally, introduce docker/archive.Writer * use container/storage/pkg/homedir * Fix an error message on docker-archive:path:name@sha256:$digest * Move docker/tarfile.Source to docker/internal/tarfile.Source * Use the docker/internal/tarfile.Source from docker/daemon and docker/archive * Remove deprecated non-SystemContext functions from docker/internal/tarfile * Split docker/internal/tarfile.Reader from Source * Separate tarfile.Reader creation from Source creation * Read the tarfile manifest already when initializing tarfile.Reader * Turn tarfile.Source.LoadTarManifest into a TarManifest * Allow choosing an image from tarfile.Reader by reference * Introduce docker-archive:path:@index syntax for reading untagged images * Introduce docker/archive.Reader * Finally, share a tarfile.Reader across archiveSource objects * Add docker/archive.NewReaderForReference * Add docker/archive.Reader.ManifestTagsForReference * Support per user registries.d * Move TestInvalidPolicyFormatError * Reduce duplication in policy_config_test.go * Eliminate more duplication in signature/policy_config_tests.go * Return error body if UnexpectedHTTPResponseError * Set NoLchown to true in untar opts Signed-off-by: Miloslav Trmač <[email protected]>
What type of PR is this?
What this PR does / why we need it:
Similar PR goes for crun/conmon/libpod/cri-o/etc, too.
Also see:
[nix] Cleanup nix derivation for static builds crun#372[nix] Add nix derivation for static builds conmon#161[nix] Add nix derivation for static builds #932[nix] Add nix derivation for static builds buildah#2380Static binaries:
Ansible Roles:
How to verify it
Which issue(s) this PR fixes:
Special notes for your reviewer:
Here I skip the btrfs and lvm2 support for static binary, because:
Also see:
Does this PR introduce a user-facing change?