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.

Sign up for GitHub

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

Jump to bottom

fetchGit: work-around bug in Nix 2.4 #424

Closed
wants to merge 2 commits into from
Closed

fetchGit: work-around bug in Nix 2.4 #424

wants to merge 2 commits into from

Conversation

vlaci
Copy link
Contributor

@vlaci vlaci commented Oct 20, 2021 •
edited
Loading

Another approach to make poetry2nix work with dependencies specified
by arbitrary rev=<hash> values on newer Nix revisions.

Relates-to: #358
Relates-to: NixOS/nix#5128

--
Interestingly I only see this error on 'nix-2.4pre20211001_4f49615' (nixpkgs-unstable channel) but not on 'nix-2.5pre20211007_844dd90' (nixos-unstable channel)
Update: there was a cached copy of that dependency in my /nix/store 🙃

@vlaci
Copy link
Contributor Author

vlaci commented Oct 20, 2021

I was mistaken, the issue persists on 2.5 as well, so the condition check need to be adjusted accordingly

bjornfor
bjornfor reviewed Oct 20, 2021
View reviewed changes
mk-poetry-dep.nix Outdated Show resolved Hide resolved
@vlaci vlaci force-pushed the fetchgit-nix24 branch 2 times, most recently from f3ea507 to 5525d1f Compare October 21, 2021 08:19
@vlaci
Copy link
Contributor Author

vlaci commented Oct 21, 2021 •
edited
Loading

Tested on nix-2.4pre20211001_4f49615 and nix-2.5pre20211007_844dd90

László Vaskó added 2 commits October 22, 2021 13:23
fetchGit: work-around bug in Nix 2.4
563522a 
Another approach to make `poetry2nix` work with dependencies specified
by arbitrary `rev=<hash>` values on newer Nix revisions.

Relates-to: nix-community#358
Relates-to: NixOS/nix#5128
fetchGit: use allRefs only when it is needed
fc4cdde 
If `branch` or `tag` is given in sourceSpec, then `fetchGit` would
work as before, the only problematic setup is `rev` when used with Nix 2.4+
@adisbladis
Copy link
Member

This bug should be fixed in Nix, not worked around in poetry2nix.

@adisbladis adisbladis closed this Nov 16, 2021
@vlaci
Copy link
Contributor Author

vlaci commented Nov 16, 2021

I'd like you to reconsider this decision because unless I am mistaken there is no alternative right now to use commit hashes with flakes.
There is a similar workaround for yarn2nix in nixpkgs proper https://github.com/NixOS/nixpkgs/pull/141005/files

@mou
Copy link
Contributor

mou commented Jan 17, 2022

I agree with @vlaci. But for me it's not a workaround, it's plain following to upstream API changes

@adisbladis adisbladis mentioned this pull request Jan 17, 2022
Merged
@adisbladis
Copy link
Member

I couldn't force-push to this branch so I made a new PR in #512.

@adisbladis
Copy link
Member

it's plain following to upstream API changes

The point is that we shouldn't have these types of regressions in Nix and force downstream projects to deal with them.

@mou
Copy link
Contributor

mou commented Jan 18, 2022

@adisbladis i agree, Nix team should communicate clearly and announce breaking changes in advance. Downstream projects should have time to react and adapt. In a week of learning Nix this is already second case when upstream make dependent projects broken. It's not ok ((

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bjornfor bjornfor bjornfor left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@vlaci @adisbladis @mou @bjornfor