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

Adding Nix building configuration #1222

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Conversation

mgrzybek
Copy link

This PR adds default.nix. This allows users to use the Nix building system to create the binary.
matchbox is then added to the nixstore.

$ git clone https://github.com/poseidon/matchbox/
$ cd matchbox
$ nix-build
this derivation will be built:
  /nix/store/96f9rm6xbbw8ggkd9v187h6s849y8lka-matchbox.drv
building '/nix/store/96f9rm6xbbw8ggkd9v187h6s849y8lka-matchbox.drv'...
unpacking sources
unpacking source archive /nix/store/a592kv07ff9yx9ly71nmwva591ky1www-source
source root is source
patching sources
updateAutotoolsGnuConfigScriptsPhase
configuring
building
Building subPackage ./cmd/bootcmd
Building subPackage ./cmd/matchbox
Building subPackage ./matchbox/cli
Building subPackage ./matchbox/client
Building subPackage ./matchbox/http
Building subPackage ./matchbox/rpc
Building subPackage ./matchbox/rpc/rpcpb
Building subPackage ./matchbox/server
Building subPackage ./matchbox/server/serverpb
Building subPackage ./matchbox/sign
Building subPackage ./matchbox/storage
Building subPackage ./matchbox/storage/storagepb
Building subPackage ./matchbox/storage/testfakes
Building subPackage ./matchbox/tlsutil
Building subPackage ./matchbox/version
running tests
ok      github.com/poseidon/matchbox/matchbox/client    0.005s
ok      github.com/poseidon/matchbox/matchbox/http      0.019s
ok      github.com/poseidon/matchbox/matchbox/rpc       0.005s
ok      github.com/poseidon/matchbox/matchbox/server    0.005s
ok      github.com/poseidon/matchbox/matchbox/sign      0.008s
ok      github.com/poseidon/matchbox/matchbox/storage   0.010s
ok      github.com/poseidon/matchbox/matchbox/storage/storagepb 0.005s
installing
post-installation fixup
shrinking RPATHs of ELF executables and libraries in /nix/store/yxpjkjwfc5m37av6xbp179bxbfzn3n2w-matchbox
shrinking /nix/store/yxpjkjwfc5m37av6xbp179bxbfzn3n2w-matchbox/bin/matchbox
shrinking /nix/store/yxpjkjwfc5m37av6xbp179bxbfzn3n2w-matchbox/bin/bootcmd
checking for references to /build/ in /nix/store/yxpjkjwfc5m37av6xbp179bxbfzn3n2w-matchbox...
patching script interpreter paths in /nix/store/yxpjkjwfc5m37av6xbp179bxbfzn3n2w-matchbox
stripping (with command strip and flags -S -p) in  /nix/store/yxpjkjwfc5m37av6xbp179bxbfzn3n2w-matchbox/bin
/nix/store/yxpjkjwfc5m37av6xbp179bxbfzn3n2w-matchbox
$

Copy link
Member

@dghubble dghubble left a comment

Choose a reason for hiding this comment

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

This dosn't build the binary correctly. It's supposed to be statically linked and to set the version when you run matchbox --version. See Makefile for reference.

./result/bin/matchbox -version
was not built properly

How are you imaginging this being used? Just a helper for those who prefer to use nix-build for local iteration instead of the Makefile?

default.nix Outdated Show resolved Hide resolved
default.nix Outdated
lib = pkgs.lib;
matchbox = pkgs.buildGoModule {
name = "matchbox";
src = lib.cleanSource ../matchbox;
Copy link
Member

Choose a reason for hiding this comment

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

can you explain cleanSource here?

Copy link
Author

@mgrzybek mgrzybek Mar 25, 2024

Choose a reason for hiding this comment

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

This is a recommended macro on development environments: https://gist.github.com/CMCDragonkai/8d91e90c47d810cffe7e65af15a6824c

This filters out some unwanted artifacts (temporary files, git data…).

matchbox = pkgs.buildGoModule {
name = "matchbox";
src = lib.cleanSource ../matchbox;
vendorHash = "sha256-sVC4xeQIcqAbKU4MOAtNicHcioYjdsleQwKWLstnjfk";
Copy link
Member

Choose a reason for hiding this comment

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

How are you imaginging this being updated? I don't expect to maintain or bump a hash. If the aim is for this to be an alternative to using the Makefile during local iteration/development, vendorHash seems counter to quick iteration

Copy link
Author

Choose a reason for hiding this comment

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

The vendor hash must be set per release tag because they are releases.
You can also set vendorHash to null in the main branch but nix-build won’t work out of the box because go mod vendor must be run first.

Copy link
Member

Choose a reason for hiding this comment

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

For an in-project development Nix config, I think the config should just build from the local source. As in, if you edit some Go files or dependencies locally, running nix-build should just build it without someone having to fiddle with hashes. I'd prefer real releases and vendor pinning be done in upstream nixpkgs.

Do you see a usage story that aligns with this or no?

Copy link
Author

Choose a reason for hiding this comment

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

Sounds fair.

@dghubble
Copy link
Member

Separately, I'm also aiming to get this packaged in nixpkgs, but I could possibly see value in having a local in-project build configuration if it matches the Makefile

@mgrzybek
Copy link
Author

Separately, I'm also aiming to get this packaged in nixpkgs, but I could possibly see value in having a local in-project build configuration if it matches the Makefile

I saw it and asked the maintainer I know to have a look so it's merged. Now let's write the systemd service.

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