Skip to content

Add nix build file #110

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

Merged
merged 8 commits into from
Oct 27, 2019
Merged

Add nix build file #110

merged 8 commits into from
Oct 27, 2019

Conversation

kloenk
Copy link
Contributor

@kloenk kloenk commented Oct 24, 2019

Create a default.nix to enable the nix-build command. Also, enable git
tracking of Cargo.toml to build it with nix.

default.nix contains a Sha256Sum of the Cargo.lock file.

This commit can also be used to provide an entrance to get onefetch in the nixpkgs package repository, so every nix user can use it. If requested I can see that I get it into nixpkgs upstream

This pr resolves #109

Create a default.nix to enable the nix-build command. Also enable git
tracking of Cargo.toml to build it with nix.

default.nix contains a Sha256Sum of the Cargo.lock file
@spenserblack
Copy link
Collaborator

Just ran nix-build to test this out. Good job! 👍
I've messed around a little bit with NixOS, but I'm not too familiar with it. Should the Travis-CI config be updated to test for nix builds, in your opinion? Or can we trust that, as long as it builds in Linux via cargo build, it will build with nix-build (assuming no updates to default.nix)?

@kloenk
Copy link
Contributor Author

kloenk commented Oct 25, 2019

I think changing it would be a good way, because the default.nix has to be updated, because of the CargoSha256 attribute

@spenserblack
Copy link
Collaborator

Understood. Do you think you could change the Travis-CI config to test nix-build in your fork? Once you push the change up, I believe another build should be triggered with your config for this PR.

Removed notifications, because it is unknown to travis lint
The nix script should have been nix-build, but it wasn't
@kloenk
Copy link
Contributor Author

kloenk commented Oct 25, 2019

Added it to Travis-CI, but only with nix under linux.

How about nixpkgs, shall I make a PR into nixpkgs, after this PR is merged, or do you want to make it?

Sorry, pressed a wrong button
@spenserblack
Copy link
Collaborator

spenserblack commented Oct 25, 2019

How about nixpkgs, shall I make a PR into nixpkgs, after this PR is merged, or do you want to make it?

@o2sh, what do you think?

Also, @kloenk, the person who submits it would be the maintainer in default.nix, correct?

@kloenk
Copy link
Contributor Author

kloenk commented Oct 26, 2019

Yes, the submitter to nixpkgs should be the one in the default.nix. When we submit it to nixpkgs, we are just creating a default.nix in the nixpkgs repository, which basically will lock the same as the one I contributed to this repo, just the src attribute will be different

@o2sh
Copy link
Owner

o2sh commented Oct 26, 2019

Sure @kloenk, as long you can keep an eye on the project and update the package each time a new release is made, you can be the maintainer.

branches:
only:
- master
- cargo build --verbose
Copy link
Collaborator

Choose a reason for hiding this comment

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

Looking forward to merging your PR 👍 ! Just one quick question:
Why move cargo build below cargo test? Tests require a successful build. If the project has not been built, cargo test will build it, so building after tests have run successfully is a bit redundant.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No very good reason, personally I would first execute the tests, and then build, but that's the complete reason. But we maybe should change the cargo build to --release, and then there are different builds, which makes it irrelevant.

So, Should I change it, or make cargo build a release build?

@kloenk
Copy link
Contributor Author

kloenk commented Oct 27, 2019

Sure @kloenk, as long you can keep an eye on the project and update the package each time a new release is made, you can be the maintainer.

Ok, I will make a PR to nixpkgs, as soon as this PR is merged, and the version has been bumped, because I need a version I can refer as a tag, and the Cargo.lock inside this version.

buildRustPackage provides a default value, which includes windows, which
I forgot, so onefetch can also installed under windows via nix.
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.

nix derivation
3 participants