-
Notifications
You must be signed in to change notification settings - Fork 107
Conversation
* github workflow which updates deps.nix
Update deps.nix
… feature/nix-derivation
68eabf6
to
656bc94
Compare
656bc94
to
3d82895
Compare
Update deps.nix
Update deps.nix
The last few commits and builds illustrate how the pull request functionality works. The corresponding pull requests are:
|
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.
I've never used Nix myself so I can't review those bits
Co-Authored-By: Javier Uruen Val <juruen@github.com>
I am currently not a 100% sure how flexible and open nixpkgs contributors are on merging pull request which basically just references a derivation stored in an other repository. It should not really matter because nix is side effect free but you never know. For easy maintenance of So if this is of any interest to the project, I would actually recommend the following for this pull request:
|
Hello, Thanks for the PR! I have plans for finally try and dig NixOS. I am really beginning to learn it to be able to switch to it as my main OS. Maybe I can review as soon as I have enough knowledge... In the meantime, if any other Nix user feels comfortable for reviewing this, I would not see any problems to merge to have it merged... Loric |
Related question on discourse: Is importing a derivation from another repository bad practice? |
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.
Hey, sorry this will feel more like questions than a proper review because I am really starting to dig the Nix world (which by the way is awesome!). If you can help me understanding this by answering my questions, that would be super helpful to me.
{stdenv, buildGoModule}: | ||
|
||
let | ||
in buildGoModule rec { |
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.
Does this pattern mean that someone using this derivation.nix
can easily inject custom attributes to the set?
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.
No, this is a custom builder provided by nixpkgs for builiding go programs managed with go modules. I tried to follow the official nixpkgs documentation, which suggest using buildGomodule
. But yes, you could create an overlay for this if you wanted to 'inject' things.
{ pkgs ? import <nixpkgs> {} }: | ||
with pkgs; | ||
|
||
let |
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.
What is the purpose of an empty let / in
statement? Is it just by convention for potential future values?
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.
Nah that's merely just a leftover. I used to have stuff in there which disappeared over time. I will remove this.
|
||
let | ||
in pkgs.callPackage ./derivation.nix { | ||
buildGoModule = super: buildGoModule (super // { |
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.
As I am quite new in functional languages and nix in general, I might need a little extra help to understand these two lines 5 and 6! So of what I understand, buildGoModule
is a function that should return a set that will represent the final derivation. So defining it like this woud allow the caller to provide a super
set argument that would be merged with the body of the function here.
But what is the difference with having maybe something a little bit simpler like the following:
derivation.nix
{ stdend, overrides }:
overrides // {
name = "rMAPI";
src = ./.;
goDeps = ./deps.nix
...
}
shell.nix
{ pkgs ? import <nixpkgs> {} }:
pkgs.callPackage ./derivation.nix {
overrides = {
name = "rMAPI-env";
buildInputs = ...;
};
}
Thanks for shedding light!
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.
I do think, technically this would work. But I can see some possible downsides of this approach:
- I tried to to make the inputs reflect dependencies only
- When used in
<nixpkgs>
,callPackage
will resolveoverrides
to whatever this is defined to in the scope of the calling function. - overriding describes the process of changing parameters of a function and therefore
overrides
is not a good name for this. See this discussion for some ideas around the topic.
Also I liked the readability and simplicity of the chosen approach. But that's just my opinion, maybe i am completely wrong about this.
@@ -0,0 +1,192 @@ | |||
# file generated from go.mod using vgo2nix (https://github.com/adisbladis/vgo2nix) |
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.
By using nix to maintain dependencies, I understand that it feels more nix-friendly and pure but do you think that it is more robust than just keeping dependencies with go.mod
?
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.
In order to ensure purity of the build, we have to use fetchers from <nixpkgs>
. To my knowledge there is no fetcher which does understand go.mod
directly. I am unsure about the exact reasons for this but I do think go.mod
was not considered sufficient to ensure a purity by nixpkgs. Maybe there is some loophole somewhere which could break purity.
This is why we have to convert go.mod
into something nixpkgs
understands.
Hi @Enteee, I have started to have a look and try to grasp all of these nix concepts! I have reviewed and added some questions. Maybe a last more global question here for you. Is it a common pattern to have the nix derivation building scripts inside the repository of the project and then only a simple Thanks, |
Thank you for your comments @lobre. I tried to answer your questions to my best knowledge.
I try to do it in all of my projects, mostly for the sake of having a simple and well defined build workflow, a standardized and easy to set up development environment and a CI/CD pipeline support for keeping build inputs automatically in sync (i.e. My initial plan was to keep all the build relevant files here and just reference it then in On a side note, I wrote a few words about my experience with reMarkable including this project on my blog. I hope this is ok for you guys. |
With NixOS/nixpkgs#85898 rmapi is now maintained in nixpkgs. My original pull request (NixOS/nixpkgs#74657) was never merged. Therefore, I don't think there is much value in keeping this pull request open. I can do small changes to the derivation as well, but the official maintainer of the rmapi derivation in nixpkgs is @NickHu. So please get in touch with him in case you do need something changed. |
Hi folks, NixOS support! 😄 🚀
The things that I added are:
default.nix
: a reproducible and pure builds withnix-build
shell.nix
: a managed development environment withnix-shell
README.md
: documentation on how to enter the development environmentdeps.nix
: freezes all build dependencies managed ingo.mod
.github/workflows/nix.yml
GitHub workflow which:deps.nix
ifgo.mod
changes (automatically creates a pull request)deps.nix
is not in sync withgo.mod
nix-build
which does build the binary & runs testsderivation.nix
: allows for easy integration into the nixpkgs repository with something like this:I did this mainly for myself, but thought maybe you would be interested in this as well. If you are interested in this PR and we merge this, i will then take it a step further an create a derivation in the nixpkgs repository so that all NixOS users can use this by just:
nix-shell -p rmapi
.If not I will probably have to create a very similar pull request in the nixpkgs repository.
No hard feelings if you decide to reject this 😄 🦆
To Do
If I do succeed in getting this to nixpkgs and you are ok with the changes suggested, we merge this PR.I will then create a new PR for nixpkgs which basically only changes the source repository from my fork to this repository.Related