feat: support managing submodules via inputs.self #7862

tomberek · 2023-02-19T11:16:02Z

Provides special handling for inputs.self. If submodule is set to true, will re-call getFlake as if the original reference was set. The intended behavior is for CLI management of the parameter overrides any direct flake.nix settings.

Thanks to: @x10an14, @aaronchall, @mrene

Motivation

Context

Provides a mechanism for inputs.self.submodules to control the availability of submodules in a local flake.

Checklist for maintainers

test with remote resources (@Kranzes)
provide check/error for that this logic won't work with "github:"

Maintainers: tick if completed or explain if not relevant

agreed on idea
agreed on implementation strategy
tests, as appropriate
- functional tests - tests/**.sh
- unit tests - src/*/tests
- integration tests - tests/nixos/*
documentation in the manual
code and comments are self-explanatory
commit message explains why the change was made
new feature or incompatible change: updated release notes

Provides special handling for inputs.self. If submodule is set to true, will re-call getFlake as if the original reference was set. The intended behavior is for CLI management of the parameter overrides any direct flake.nix settings.

roberth · 2023-02-19T22:02:20Z

src/libexpr/flake/flake.cc

+        if (originalRef.input.attrs.count("submodules") == 0) {
+            auto self = flake.inputs.find("self");
+            if (self != flake.inputs.end() && self->second.hasSubmodules) {


This comparison is a little ad hoc. It should be described in more general terms. My understanding of the process is:

Nix makes a guess what the self input is

Nix checks that its guess matches the declared self input

Adjust

None of this is specific to git or submodules, so it should be done through virtual methods.

Other than that, I think lazy trees #6530 should make this specific parameter conceptually redundant, and enable it by default, as submodules should be fetched lazily.
However, that's not a reason not to have this mechanism, as we'll need it for at least a git-crypt parameter.

A general mechanism (but also with submodules as the motivating case) is described and discussed in

Support self-fetching parameters in flake.nix #5312

roberth · 2023-02-19T22:03:15Z

src/nix/flake.md

@@ -114,6 +114,8 @@ reference types:

 * `ref`: A Git or Mercurial branch or tag name.

+* `submodules`: A boolean supporting Git submodules.


What's the default?

Suggested change

* `submodules`: A boolean supporting Git submodules.

* `submodules`: Whether to also include Git submodules. Defaults to `false`.

tomberek · 2023-02-20T00:44:27Z

src/libexpr/flake/flake.cc

@@ -405,6 +421,7 @@ LockedFlake lockFlake(
               necessary (i.e. if they're new or the flakeref changed
               from what's in the lock file). */
            for (auto & [id, input2] : flakeInputs) {
+                if (id == "self") continue;


Needs a note, and docs, about the “self” input being special.

edolstra · 2023-02-20T13:47:15Z

Hm, it seems pretty ugly to add a git-specific submodules field to inputs.self. IMHO the better solution would be to enable submodules by default (after we've implemented efficient submodules support on the lazy-trees branch).

Kranzes · 2023-02-20T14:40:58Z

Hm, it seems pretty ugly to add a git-specific submodules field to inputs.self. IMHO the better solution would be to enable submodules by default (after we've implemented efficient submodules support on the lazy-trees branch).

Yeah it isn't fancy. We just implemented it overnight in a call yesterday. We're not even sure if this is even a good idea to have. The thing is that the git-specific submodules inputs field already exists, it's just take effect in the CLI. That's what this PR does, it makes it function as you expect. This isn't an addition of a new feature, it is a fix of an already existing functionality.

fricklerhandwerk · 2023-02-24T12:20:18Z

Triaged in the Nix team meeting:

@edolstra: prefer to enable submodules by default
- right now they are extremly inefficient, but may want to make it work with lazy trees
@thufschmitt: wouldn't hurt to have a feature to forcibly enable them
- @edolstra: this would change the flake language, requires us to continue supporting this eventually
@tomberek: often in practice people use submodule for private dependencies
@fricklerhandwerk: how high on our global priority list is this?
to discuss

Kranzes · 2023-02-24T12:39:01Z

@edolstra: prefer to enable submodules by default

I believe that used to be the case but it was reverted because it was fetching when people didn't want it.

nixos-discourse · 2023-02-24T13:02:00Z

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

https://discourse.nixos.org/t/2023-02-24-nix-team-meeting-minutes-35/25757/1

Kha · 2023-02-24T15:11:13Z

@edolstra: prefer to enable submodules by default

I believe that used to be the case but it was reverted because it was fetching when people didn't want it.

To be clear it was because of their inefficiency #5280. But that sounds like a perfect application of lazy trees as mentioned.

JakeHillion · 2023-03-18T13:33:03Z

To chime in, I've seen C++ projects before where certain submodules are optional except for special cases like testing. While enabling by default would be helpful, I can also imagine a feature like .?submodules=extern/X,extern/Y being useful to keep clones efficient. This wouldn't fit neatly with always including submodules. I don't know how this fits in with flakes though as generally you'd want the flake to cover every case (incl. testing) so it may be very niche, or be on a per output level.

Either way, this feature as implemented in this PR or submodules by default would be incredibly useful for me.

roberth · 2023-03-18T13:58:13Z

I think we should consider closing this, because the non-libgit fetching has too many problems and a more elegant replacement is in progress in #6530.
We'd better make some progress on that, because we should neither develop a dead end, nor let fetcher development be blocked for a year.

fricklerhandwerk · 2023-03-20T12:45:56Z

Discussed in the Nix team meeting:

We recommend instead to build a Nix language function that re-fetches an input by adding ?submodules=true. This is dirty but avoids changing Nix itself.
We plan on delivering a principled solution eventually anyway, and the soft implementation is a good-enough stopgap.

Complete discussion

@Ericson2314: it would make sense if we implemented libgit first
@roberth: it's a good mechanism, but we don't have a need for it yet
@edolstra: the real problem is that it changes the interface by adding inputs.self, and we'd have to continue supporting that in the future
- especially if it hardcodes submodules default to false
- @thufschmitt: don't think the default is a problem
- @fricklerhandwerk: is anything speaking against treating that addition as "extra-experimental" in isolation, even if we currently have no means of expressing it in the code?
  - @thufschmitt: that's the problem, there is no way of enforcing it whatsoever
  - @edolstra: in principle since flakes are experimental, we can change it arbitrarily, but ideally we shouldn't have to change the interface if it can be avoided
@thufschmitt: since submodules is false by default, one would have to specify the parameter to build the top-level flakes every time
- would it make sense set it true for the top-level flake by default?
- @edolstra: no, that would be very expensive
@edolstra: it's convenient to the user, but architecturally questionable to do the double-fetching
- @roberth: with lazy trees this will become super cheap
@thufschmitt: couldn't we instead build a Nix language function that re-fetches an input by adding ?submodules=true?
- this is dirty but avoids changing Nix itself
- we will deliver a principled solution eventually, and this is good enough as a stopgap
decision: postponed

nixos-discourse · 2023-03-22T20:35:14Z

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

https://discourse.nixos.org/t/2023-03-20-nix-team-meeting-minutes-42/26615/1

NorfairKing · 2024-01-24T12:24:10Z

What's the status here? I'd really like not to have to specify ?submodules=1 on the command-line every time.

roberth · 2024-01-24T17:14:26Z

fetchTree will fetch submodules lazily in the upcoming release, 2.20.
We'll have very little reason to disable submodules by default...

EDIT: added to team discussion queue for Friday.

nixos-discourse · 2024-02-22T12:32:29Z

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

https://discourse.nixos.org/t/get-nix-flake-to-include-git-submodule/30324/13

amfaber · 2024-03-27T09:53:36Z

Is this available on nix version 2.20 onwards? I would very much like an example of how I can build a derivation using a local submodule

znd4 · 2024-03-28T14:26:29Z

@amfaber , not a contributor/maintainer, but it doesn't seem like it based on the release notes

roberth · 2024-03-28T16:16:38Z

Is this available on nix version 2.20 onwards?

It is not.

We'll have very little reason to disable submodules by default...

We're still exploring the solution space for efficiently fetching from a forge such as GitHub, which can return a tarball as a significantly quicker operation, so it's hard to tell whether changing the default setting would make sense.

I do think we'll need a configurable inputs.self regardless.

nixos-discourse · 2024-03-30T10:46:15Z

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

https://discourse.nixos.org/t/get-nix-flake-to-include-git-submodule/30324/14

jakubgs · 2024-03-30T11:12:32Z

Lack of support for submodules for current(self) repo makes packaging most of our software not viable using Nix Flakes.
I'm really hoping we can get support of submodules for self in Nix.

nixos-discourse · 2024-05-09T03:50:25Z

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

https://discourse.nixos.org/t/handling-git-submodules-in-flakes-from-nix-2-18-to-2-22-nar-hash-mismatch-issues/45118/1

Must be built with `nix build '.?submodules=1'` until this is merged: NixOS/nix#7862

AshleyYakeley · 2024-07-19T20:01:13Z

Any update on this?

tomberek force-pushed the self_submodules branch from 3636d49 to 6d4c2ce Compare February 19, 2023 12:17

feat: support managing submodules via inputs.self

6ebe06a

Provides special handling for inputs.self. If submodule is set to true, will re-call getFlake as if the original reference was set. The intended behavior is for CLI management of the parameter overrides any direct flake.nix settings.

tomberek force-pushed the self_submodules branch from 6d4c2ce to 6ebe06a Compare February 19, 2023 12:18

test: add tests for submodule handling in flake.nix and CLI

5fd671f

tomberek force-pushed the self_submodules branch from 406319c to 5fd671f Compare February 19, 2023 13:03

docs: add mention of submodule support for flake references

6a05394

tomberek force-pushed the self_submodules branch from bca2a7b to 6a05394 Compare February 19, 2023 13:48

tomberek marked this pull request as ready for review February 19, 2023 14:33

tomberek requested review from fricklerhandwerk, edolstra and thufschmitt as code owners February 19, 2023 14:33

roberth reviewed Feb 19, 2023

View reviewed changes

tomberek commented Feb 20, 2023

View reviewed changes

tomberek marked this pull request as draft February 21, 2023 01:38

n8henrie mentioned this pull request Feb 24, 2023

Submodules of flakes are not working #6633

Open

thufschmitt added feature Feature request or proposal flakes labels Feb 27, 2023

Kha mentioned this pull request Mar 18, 2023

Optimize submodules fetching #5399

Closed

Patryk27 mentioned this pull request May 7, 2023

Building crate that has a git submodule that is a crate that one depends on as a path dependency seems to fail? nix-community/naersk#287

Closed

wmertens mentioned this pull request Jun 18, 2023

flake: support git submodules direnv/direnv#1120

Closed

MrFoxPro mentioned this pull request Oct 29, 2023

chore(nix): initial flake edgedb/edgedb-cli#1159

Closed

sjmonson added a commit to sjmonson/tdr-inverse that referenced this pull request May 23, 2024

Add a nix package

83e3497

Must be built with `nix build '.?submodules=1'` until this is merged: NixOS/nix#7862

pedorich-n mentioned this pull request Aug 31, 2024

Partitions with dev files stored outside of the repository? hercules-ci/flake-parts#244

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: support managing submodules via inputs.self #7862

feat: support managing submodules via inputs.self #7862

tomberek commented Feb 19, 2023 •

edited

Loading

roberth Feb 19, 2023

roberth Apr 9, 2023

roberth Feb 19, 2023

Kranzes Feb 20, 2023

tomberek Feb 20, 2023

edolstra commented Feb 20, 2023

Kranzes commented Feb 20, 2023 •

edited

Loading

fricklerhandwerk commented Feb 24, 2023 •

edited

Loading

Kranzes commented Feb 24, 2023 •

edited

Loading

nixos-discourse commented Feb 24, 2023

Kha commented Feb 24, 2023

JakeHillion commented Mar 18, 2023 •

edited

Loading

roberth commented Mar 18, 2023

fricklerhandwerk commented Mar 20, 2023

nixos-discourse commented Mar 22, 2023

NorfairKing commented Jan 24, 2024 •

edited

Loading

roberth commented Jan 24, 2024 •

edited

Loading

nixos-discourse commented Feb 22, 2024

amfaber commented Mar 27, 2024

znd4 commented Mar 28, 2024

roberth commented Mar 28, 2024

nixos-discourse commented Mar 30, 2024

jakubgs commented Mar 30, 2024 •

edited

Loading

nixos-discourse commented May 9, 2024

AshleyYakeley commented Jul 19, 2024

		@@ -114,6 +114,8 @@ reference types:

		* `ref`: A Git or Mercurial branch or tag name.

		* `submodules`: A boolean supporting Git submodules.

	* `submodules`: A boolean supporting Git submodules.
	* `submodules`: Whether to also include Git submodules. Defaults to `false`.

feat: support managing submodules via inputs.self #7862

Are you sure you want to change the base?

feat: support managing submodules via inputs.self #7862

Conversation

tomberek commented Feb 19, 2023 • edited Loading

Motivation

Context

Checklist for maintainers

roberth Feb 19, 2023

Choose a reason for hiding this comment

roberth Apr 9, 2023

Choose a reason for hiding this comment

roberth Feb 19, 2023

Choose a reason for hiding this comment

Kranzes Feb 20, 2023

Choose a reason for hiding this comment

tomberek Feb 20, 2023

Choose a reason for hiding this comment

edolstra commented Feb 20, 2023

Kranzes commented Feb 20, 2023 • edited Loading

fricklerhandwerk commented Feb 24, 2023 • edited Loading

Kranzes commented Feb 24, 2023 • edited Loading

nixos-discourse commented Feb 24, 2023

Kha commented Feb 24, 2023

JakeHillion commented Mar 18, 2023 • edited Loading

roberth commented Mar 18, 2023

fricklerhandwerk commented Mar 20, 2023

nixos-discourse commented Mar 22, 2023

NorfairKing commented Jan 24, 2024 • edited Loading

roberth commented Jan 24, 2024 • edited Loading

nixos-discourse commented Feb 22, 2024

amfaber commented Mar 27, 2024

znd4 commented Mar 28, 2024

roberth commented Mar 28, 2024

nixos-discourse commented Mar 30, 2024

jakubgs commented Mar 30, 2024 • edited Loading

nixos-discourse commented May 9, 2024

AshleyYakeley commented Jul 19, 2024

tomberek commented Feb 19, 2023 •

edited

Loading

Kranzes commented Feb 20, 2023 •

edited

Loading

fricklerhandwerk commented Feb 24, 2023 •

edited

Loading

Kranzes commented Feb 24, 2023 •

edited

Loading

JakeHillion commented Mar 18, 2023 •

edited

Loading

NorfairKing commented Jan 24, 2024 •

edited

Loading

roberth commented Jan 24, 2024 •

edited

Loading

jakubgs commented Mar 30, 2024 •

edited

Loading