Skip to content

libfetchers: Fix lazy attribute comparisons - #16430

Draft
xokdvium wants to merge 1 commit into
masterfrom
fix-lazy-attr-comparison
Draft

libfetchers: Fix lazy attribute comparisons#16430
xokdvium wants to merge 1 commit into
masterfrom
fix-lazy-attr-comparison

Conversation

@xokdvium

@xokdvium xokdvium commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

Motivation

Comparison was erroneously shallow and comparing pointers without also considering LazyAttr producing the same value equivalent with another non-lazy attr, which lead to lock file comparison returning garbage on flake update commit message (and stderr output).

Also make code more concise (specifically operator<=> defaulting also defaults operator== and operator!=, though defining it explicitly doesn't - only = default does that).

Context

Reported in #15772 (comment). cc @SuperSandro2000


Add 👍 to pull requests you find important.

The Nix maintainer team uses a GitHub project board to schedule and track reviews.

@xokdvium
xokdvium requested a review from edolstra as a code owner September 5, 2026 22:52
@xokdvium xokdvium added the backport 2.35-maintenance Automatically creates a PR against the branch label Sep 5, 2026
@xokdvium
xokdvium requested review from roberth and removed request for edolstra September 5, 2026 22:52
@github-actions github-actions Bot added the fetching Networking with the outside (non-Nix) world, input locking label Sep 5, 2026
@xokdvium xokdvium mentioned this pull request Sep 5, 2026
@xokdvium
xokdvium force-pushed the fix-lazy-attr-comparison branch from f336db8 to 2ad624c Compare September 5, 2026 23:13
@xokdvium

xokdvium commented Sep 5, 2026

Copy link
Copy Markdown
Contributor Author

Ah amazing, revCount in shallow clones also doesn't work properly here. @roberth any thoughts?

nix-functional-tests> +(fetchGitShallow.sh:96) nix build --dry-run git+file:///build/nix-test/main/fetchGitShallow/shallow-build-clone
nix-functional-tests> warning: you don't have Internet access; disabling some network-dependent features
nix-functional-tests> Nix crashed. This is a bug. Please report this at https://github.com/NixOS/nix/issues with the following information included:
nix-functional-tests>
nix-functional-tests> Exception: nix::Error: error: '/build/nix-test/main/fetchGitShallow/shallow-build-clone' is a shallow Git repository, so 'revCount' is not available
nix-functional-tests> Stack trace:
nix-functional-tests>  0# 0x00000000001355DA in nix
nix-functional-tests>  1# 0x00000000000C539A in /nix/store/hngmi01i8wgi25a0byrxcn4ysz5j79mw-gcc-15.2.0-lib/lib/libstdc++.so.6
nix-functional-tests>  2# __cxa_call_terminate in /nix/store/hngmi01i8wgi25a0byrxcn4ysz5j79mw-gcc-15.2.0-lib/lib/libstdc++.so.6
nix-functional-tests>  3# __gxx_personality_v0 in /nix/store/hngmi01i8wgi25a0byrxcn4ysz5j79mw-gcc-15.2.0-lib/lib/libstdc++.so.6
nix-functional-tests>  4# 0x000000000002210B in /nix/store/hngmi01i8wgi25a0byrxcn4ysz5j79mw-gcc-15.2.0-lib/lib/libgcc_s.so.1
nix-functional-tests>  5# _Unwind_Resume in /nix/store/hngmi01i8wgi25a0byrxcn4ysz5j79mw-gcc-15.2.0-lib/lib/libgcc_s.so.1
nix-functional-tests>  6# 0x0000000000056A88 in /nix/store/wxhbhy9x7l7l5fh268jk390b2xhy3wcy-nix-fetchers-2.36.0pre/lib/libnixfetchers.so.2.36.0
nix-functional-tests>  7# nix::fetchers::Attr::operator==(nix::fetchers::Attr const&) const in /nix/store/wxhbhy9x7l7l5fh268jk390b2xhy3wcy-nix-fetchers-2.36.0pre/lib/libnixfetchers.so.2.36.0
nix-functional-tests>  8# nix::fetchers::Input::operator==(nix::fetchers::Input const&) const in /nix/store/wxhbhy9x7l7l5fh268jk390b2xhy3wcy-nix-fetchers-2.36.0pre/lib/libnixfetchers.so.2.36.0
nix-functional-tests>  9# 0x000000000003D8F7 in /nix/store/4xipffx3vzq4m2smdwcbvrrjj01ps353-nix-flake-2.36.0pre/lib/libnixflake.so.2.36.0
nix-functional-tests> 10# nix::flake::lockFlake(nix::flake::Settings const&, nix::EvalState&, nix::FlakeRef const&, nix::flake::LockFlags const&) in /nix/store/4xipffx3vzq4m2smdwcbvrrjj01ps353-nix-flake-2.36.0pre/lib/libnixflake.so.2.36.0
nix-functional-tests> 11# nix::InstallableFlake::getLockedFlake() const in /nix/store/xd94vh2nfs6ayqw5cmx52nwsr2lps5ij-nix-cmd-2.36.0pre/lib/libnixcmd.so.2.36.0
nix-functional-tests> 12# nix::InstallableFlake::getCursors(nix::EvalState&, nix::AutoCall) in /nix/store/xd94vh2nfs6ayqw5cmx52nwsr2lps5ij-nix-cmd-2.36.0pre/lib/libnixcmd.so.2.36.0
nix-functional-tests> 13# nix::InstallableValue::getCursor(nix::EvalState&, nix::AutoCall) in /nix/store/xd94vh2nfs6ayqw5cmx52nwsr2lps5ij-nix-cmd-2.36.0pre/lib/libnixcmd.so.2.36.0
nix-functional-tests> 14# nix::InstallableFlake::toDerivedPaths() in /nix/store/xd94vh2nfs6ayqw5cmx52nwsr2lps5ij-nix-cmd-2.36.0pre/lib/libnixcmd.so.2.36.0
nix-functional-tests> 15# 0x00000000001327F5 in nix
nix-functional-tests> 16# nix::InstallablesCommand::run(nix::ref<nix::Store>, std::vector<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::allocator<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > > >&&) in /nix/store/xd94vh2nfs6ayqw5cmx52nwsr2lps5ij-nix-cmd-2.36.0pre/lib/libnixcmd.so.2.36.0

So it was being compared after all (though just by pointer equaility - which is not good). I do think that deep comparison is the right approach here of course but something needs to be done about not relying on pointer equality. I'll do a quick hack for this particular issue but I hope you have better ideas.

@xokdvium

xokdvium commented Sep 5, 2026

Copy link
Copy Markdown
Contributor Author

Maybe we can indicate a lack of revCount by returning optional<ResolvedAttr> or something so that we can differentiate a "lack of revCount" case?

@xokdvium
xokdvium marked this pull request as draft September 5, 2026 23:30
@xokdvium
xokdvium force-pushed the fix-lazy-attr-comparison branch from 2ad624c to 56c9611 Compare September 5, 2026 23:49
@xokdvium

xokdvium commented Sep 5, 2026

Copy link
Copy Markdown
Contributor Author

Ok looks like I hacked up something, but I utterly hate it. Seems like the only way is to short-circuit Attr comparison of attribute is unavailable - i.e. missing like with revCount case. Maybe it's fine and I'm overthinking it.

Comparison was erroneously shallow, which lead to lock file comparison
returning garbage on `flake update` commit message.

Also make code more concise (specifically operator<=> defaulting also
defaults operator== and operator!=, though defining it explicitly doesn't -
only = default does that).
@xokdvium
xokdvium force-pushed the fix-lazy-attr-comparison branch from 56c9611 to ce0aa19 Compare September 6, 2026 09:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport 2.35-maintenance Automatically creates a PR against the branch fetching Networking with the outside (non-Nix) world, input locking

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant