-
Notifications
You must be signed in to change notification settings - Fork 203
nix: tiny cleanup #80
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: tiny cleanup #80
Conversation
|
@NotAShelf does this solve #26 (comment)? or one of #81 |
d7227a4 to
041160e
Compare
|
No, it doesn't. I had tested the build, but it didn't occur to me to test the built binary. Though I think it's a trivial fix, allow me a moment. Edit: Fixed. |
@NotAShelf great. I will check the nix issue as done. |
Signed-off-by: NotAShelf <raf@notashelf.dev> Change-Id: Ic8a9ddb1e3e3bc56b81888230706cdf96a6a6964
041160e to
ea216e2
Compare
|
I've squashed my changes. Should be fully fixed now. |
|
@NotAShelf Thanks for the detailed explanation and for tightening this up. I’m not much experienced with Nix, you're definitely the expert here. The changes make sense from your explaination, so I’ll go ahead and merge this. For formatting, I’ll add the standard formatter. Feel free to review that, and if you think any adjustments are needed, you’re welcome to open a follow-up PR. |
|
@NotAShelf I've merged this to main but still facing the build error. If possible could you please check this- error: Cannot build '/nix/store/71r7x23xccfgfpscm33rj900msj8cnxx-witr-git-32c9c28.drv'.
Reason: builder failed with exit code 1.
Output paths:
/nix/store/la80mrjzks1rnplvd86fn4p2rcpv5bij-witr-git-32c9c28
Last 19 log lines:
> Running phase: unpackPhase
> unpacking source archive /nix/store/rgdg9zfmwm6jxicsbg5j65wzwpi1ccdk-source
> source root is source
> Running phase: patchPhase
> Running phase: updateAutotoolsGnuConfigScriptsPhase
> Running phase: configurePhase
> Running phase: buildPhase
> Building subPackage ./cmd
> go: inconsistent vendoring in /build/source:
> github.com/spf13/cobra@v1.10.2: is explicitly required in go.mod, but not marked as explicit in vendor/modules.txt
> github.com/cpuguy83/go-md2man/v2@v2.0.7: is explicitly required in go.mod, but not marked as explicit in vendor/modules.txt
> github.com/inconshreveable/mousetrap@v1.1.0: is explicitly required in go.mod, but not marked as explicit in vendor/modules.txt
> github.com/russross/blackfriday/v2@v2.1.0: is explicitly required in go.mod, but not marked as explicit in vendor/modules.txt
> github.com/spf13/pflag@v1.0.10: is explicitly required in go.mod, but not marked as explicit in vendor/modules.txt
> go.yaml.in/yaml/v3@v3.0.4: is explicitly required in go.mod, but not marked as explicit in vendor/modules.txt
>
> To ignore the vendor directory, use -mod=readonly or -mod=mod.
> To sync the vendor directory, run:
> go mod vendor
For full logs, run:
nix log /nix/store/71r7x23xccfgfpscm33rj900msj8cnxx-witr-git-32c9c28.drv
|
|
Seems like you're missing the Remember that if you add a new directory critical for a build, it'll also need to be added to the filter so a quick note in the flake.nix might be nice. |
Thanks for the assist, it is working now. |
Hello!
While working on a small patch for witr I've noticed that the Nix flake is a bit inefficient, so I've cleaned it up a little. This might seem rather opitionated and drastic, but it's made with good intentions I assure you. A short summary of my changes:
I've changed
import nixpkgs { inherit system; };tonixpkgs.legacyPackages.${system}because the latter is a tiny bit more efficient. This post generally covers what the issue is, and the alternative pattern I've used. Long thing short, we aren't using overlays, so theimportpattern is unnecessary. And less efficient.Changes
pkgs.libinvocations tonixpkgs.lib, since former is always in scope due to the argset. We don't have to partially instantiatepkgsfor lib. Though the position of thelet inmight be bothersome. Let me know if you want this changed.Changed the source filter to a more aggressive one to prevent unnecessary rebuilds. In the previous version, you would rebuild the package even when an irrelevant package has changed (say, your README.md) because the source filter actually covers so little. See here for the things covered. Instead, we make a "static" filter that specifies the files needed for the build, and the source is changed only when one of the relevant files are changed. This is nice to avoid unnecessary rebuilds.
Add a
postInstallstep to install the manpages. This is not done automatically, so we have to use theinstallShellFileshook. This is trivial and cost-free, so might as well.Added the
metafield with description and license fields. Those are critical for various commands, i.e., the former is displayed innix flake shownicely and the latter avoids gettingunfreewarnings. Also went ahead and added the homepage because, well, documentation is nice.Lastly, I had to reformat the file with a conventional formatter because I could not figure out what you were using, and my formatter nuked the file after I have made all of my changes. If you'd like me to reformat, I can do that or I can add
pkgs.nixfmtas the standard formatter for the flake so it works withnix fmtautomatically.Change-Id: Ic8a9ddb1e3e3bc56b81888230706cdf96a6a6964