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

flake to "input" invoking git references in addition to revisions #3838

Closed
blaggacao opened this issue Jul 19, 2020 · 13 comments
Closed

flake to "input" invoking git references in addition to revisions #3838

blaggacao opened this issue Jul 19, 2020 · 13 comments

Comments

@blaggacao
Copy link
Contributor

blaggacao commented Jul 19, 2020

Coming from: https://discourse.nixos.org/t/flakes-dont-discover-git-references/8214

I insist for the vast majority of distribution (as opposed to development/CI) use cases, making the invoking git ref (if provided) part of the input lowers maintainability costs of the most trivial distribution oriented flakes out in the wild for upstream maintainers of any sort.

This comes at the cost of potential cache mismatch in cases where version information is not relevant by itself (use cases other than distribution use cases, such as development/CI). Strictly, also in the case a reference was updated, which shouldn't happen too often for release tags, at least.

Since the intention to differnetiate between the two use cases is entirely in the hands of the caller, anyway (
.../v1.2.3 vs .../87123gg198...), why shouldn't we be able to expect the develoment/CI caller to craft calls intelligently to minimize the problem?

@blaggacao blaggacao changed the title flake to support git references flake to "input" invoking git references in addition to revisions Jul 19, 2020
@nixos-discourse
Copy link

This issue has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/flakes-dont-discover-git-references/8214/7

@edolstra
Copy link
Member

Quoting myself: attributes like the ref or repository URL are not passed to the outputs function. This is intentional - for ensuring hermetic evaluation, and for evaluation caching, it would be bad if the evaluation result could be different depending on what branch/tag name you used to fetch the input. (E.g. the same commit on the release-20.03 and nixos-20.03 branches of the nixpkgs repository should produce the same evaluation result.)

@blaggacao
Copy link
Contributor Author

blaggacao commented Jul 20, 2020

@edolstra Either I don't understand this answer or it's not an ad rem answer.

  • Hermetic evaluation is not violated, since there is always self.rev present. self.ref is part of the input anyway, even if it's a string literal in the flake itself.

  • Cache misses are an arguable problem and hugely dependent on the predictable behaviour of the caller. (Or do I miss something with this argument?)

  • No acknowledgment of the benefits: git + caller mediated package actualization to lower maintenance overhead. (aka --version)

  • self.url supersedes self.rev with no additional semantic value, but self.ref while also superseding self.rev - in fact - does due to the widespread use of semver.

Shouldn't we reopen?

@blaggacao
Copy link
Contributor Author

Is my argument really so wrong/bad? 😸

@edolstra
Copy link
Member

It's not that I don't see the benefits, but we have to be very careful about adding attributes, since we can't remove them later.

@blaggacao
Copy link
Contributor Author

blaggacao commented Jul 29, 2020

Thanks for your answer! I'd agree, that it might be good advise to gather ample intelligence on the field from real use cases before wanting to push this further.

@blaggacao
Copy link
Contributor Author

divnix/digga#135
Here is a one of the predicted use cases popping up.

@nrdxp
Copy link

nrdxp commented Feb 23, 2021

I didn't realize this was already an issue, as I was about to open one for it, but as @blaggacao pointed out, I would like to use this feature in the above mentioned PR to help generate a version string automatically. Basically the logic would be as follows:

If: ref exists, use that as the version
Else: parse lastModified and shortRev for a version string

Had I known this issue was here, I might have chimed in earlier. There are some places I am already pulling in the flake.lock with builtins.fromJSON to get this information. Would probably be better to just include it in the source since the lock isn't guaranteed to be there.

I totally understand the desire to be super careful and super deliberate about what to include in the source information, and I appreciate this careful attention to detail. But I really think the ref is a useful piece of information, and if it is safe enough for the flake.lock file, I can't imagine why it wouldn't be so for the inputs themselves.

@blaggacao
Copy link
Contributor Author

blaggacao commented Mar 1, 2021

Maybe we (as an interested group of users) can propose a concrete draft implementation? Anyone sufficiently familiar with C would be willing to step up?

I consider this might be a pretty forward-looking feature nuance. Taking from the discourse discussion here is another use case:

{

                    buildFlagsArray = ''
                      -ldflags=
                        -w -s
                        -X main.version=${self.?????????????}
                        -X main.commit=${self.rev}
                        -X main.date=${self.lastModifiedDate}
                        -X main.builtBy=nix-flakes
                    '';

}

@domenkozar
Copy link
Member

Quoting myself: attributes like the ref or repository URL are not passed to the outputs function. This is intentional - for ensuring hermetic evaluation, and for evaluation caching, it would be bad if the evaluation result could be different depending on what branch/tag name you used to fetch the input. (E.g. the same commit on the release-20.03 and nixos-20.03 branches of the nixpkgs repository should produce the same evaluation result.)

I'm not sure I understand this, the output would be different because inputs differ. That seems to be hermetic to me.

@jmgilman
Copy link

Can this be reopened since it appears the original intent is still valid and doesn't break hermeticism, as shown in previous comments?

@nrdxp
Copy link

nrdxp commented Dec 16, 2022

For my part, I still think that if the information is available in the lock already anyway, why make us parse it?

@blaggacao
Copy link
Contributor Author

One thing that currently hampers progress on the SDLC with Nix: the lack of self.ref to make nix code branch sensitive and thereby better support release conditions.

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

No branches or pull requests

6 participants