Skip to content

Conversation

@jfroche
Copy link
Contributor

@jfroche jfroche commented Nov 13, 2023

It can be confusing (especially for new users) that the git and mercurial fetchers do not include untracked files. This commit adds an option for the fetchers to include untracked files.

The option is disabled by default and still take into account the repository ignored files.

Priorities

Add 👍 to pull requests you find important.

This subject has been referenced before in
#8389 #7107 #3231 #6858

@jfroche jfroche requested a review from edolstra as a code owner November 13, 2023 22:49
@github-actions github-actions bot added with-tests Issues related to testing. PRs with tests have some priority fetching Networking with the outside (non-Nix) world, input locking labels Nov 13, 2023
@sellout
Copy link
Contributor

sellout commented Nov 14, 2023

I agree that the current behavior is confusing and odd. If there is a line to be drawn, it should be between committed/uncommitted or maybe staged/unstaged. But the current split includes some changes that aren't expected to be committed, but not others. Untracked is odd on its own because there's no way to make it unstaged. The closest you can get is intent-to-add, which stages an empty file, leaving the staged changes in a state that you probably don't want to commit.

It's also odd because the current behavior will fetch the whole tree if there is no repo, but as soon as you git init, the fetcher fetches nothing.

This PR at least gives the option to avoid this oddness, without change the behavior for any existing usage.

@roberth
Copy link
Member

roberth commented Nov 17, 2023

I'm in favor of this, but unfortunately it seems that the git part of the implementation will have to be redone after #9240, which is a priority because it is a (long awaited) fix for reproducibility issues with git.

Mercurial might suffer from similar issues, but we haven't made a priority out of stabilizing that fetcher yet.

It can be confusing (especially for new users) that the git and
mercurial fetchers do not include untracked files. This commit adds an
option for the fetchers to include untracked files. The option is
disabled by default.
@jfroche jfroche force-pushed the feat/include-untracked-files branch from cc8d1ed to a5a2edc Compare November 20, 2023 20:01
@nixos-discourse
Copy link

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

https://discourse.nixos.org/t/the-garn-devlog/36297/2

@nixos-discourse
Copy link

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

https://discourse.nixos.org/t/use-nix-file-excluded-from-git/37196/7

@NobbZ
Copy link
Contributor

NobbZ commented Jan 17, 2024

What is the difference between this flag and using path-type explicitely?

@v-kat
Copy link

v-kat commented Jan 19, 2024

Is this still planned for release? It'd be helpful with some agenix issues I was facing

@jfroche
Copy link
Contributor Author

jfroche commented Jan 21, 2024

@roberth I realize I may not have been clear earlier. Three days after your comment, I've pushed an implementation that should align with the new approach based on libgit2. I'm curious about the likelihood of it being reviewed and eventually merged. Any insights on this?

@jfroche
Copy link
Contributor Author

jfroche commented Jan 21, 2024

What is the difference between this flag and using path-type explicitely?

New users are most of time unaware of the distinctions between running it within or outside a Git repository, often leading to confusion. This option explicitly clarifies its intended functionality.

This implementation also overlooks files that are specified in the .gitignore.

@Jonas-Sander
Copy link

This implementation also overlooks files that are specified in the .gitignore.

Just bumped into this, would love for this restriction to be removed.

@roberth roberth added this to Nix team Apr 9, 2025
@github-project-automation github-project-automation bot moved this to To triage in Nix team Apr 9, 2025
@NobbZ
Copy link
Contributor

NobbZ commented Apr 10, 2025

This implementation also overlooks files that are specified in the .gitignore.

Given the various ignoremechanisms in git (global [XDG_CONFIG_HOME/git/ignore], shared/in repo [REPO_ROOT/.gitignore], local [GITDIR/info/exclude]) we have to decide between confusing 2 groups of poeple…

  1. When I run the same command with the same manual changes, why does it produce a different result on my 2 computers
  2. Why is my entry in the global/local ignore file not respected and the not-meant-to-appear file copied into the store?

edit add missing `

@jfroche
Copy link
Contributor Author

jfroche commented Apr 10, 2025

This implementation also overlooks files that are specified in the .gitignore.

Given the various ignoremechanisms in git (global [XDG_CONFIG_HOME/git/ignore], shared/in repo [REPO_ROOT/.gitignore], local [GITDIR/info/exclude]) we have to decide between confusing 2 groups of poeple…

  1. When I run the same command with the same manual changes, why does it produce a different result on my 2 computers
  2. Why is my entry in the global/local ignore file not respected and the not-meant-to-appear file copied into the store?

edit add missing `

Ah maybe I wasn't clear in my last message, we keep using libgit2 so all the ignore files should be taken into account.

@NobbZ
Copy link
Contributor

NobbZ commented Apr 10, 2025

@jfroche As said, that confuses one group of people, as they might get different results on different computers, with seemingly same settings because there is something in the global ignore, that makes a file not being available to nix.

@sellout
Copy link
Contributor

sellout commented Apr 10, 2025

How about something more like

    Setting<std::string> includeAdditionalFiles{this, "none", "include-additional-files",
        "Whether to include untracked files in Git/Mercurial trees."};

and the values can be "none", "untracked", or "untracked and ignored"?

The "untracked" option still has the uncertainty around locally-ignored files, but I doubt libgit2 provides any finer distinction where one could include some ignored files.

@jfroche
Copy link
Contributor Author

jfroche commented Apr 11, 2025

Maybe #12884 is good enough ?

@tomberek
Copy link
Contributor

tomberek commented Jun 4, 2025

Partially addressed in 12884. Potential to improve the message. Not planning to change the default at the moment. Will need another PR to improve the message for files other than flake.nix. (maybe #12915)

@tomberek tomberek moved this from To triage to 🏁 Review in Nix team Jun 4, 2025
@nixos-discourse
Copy link

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

https://discourse.nixos.org/t/2025-05-04-nix-team-meeting-minutes-230/65206/1

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

Labels

fetching Networking with the outside (non-Nix) world, input locking with-tests Issues related to testing. PRs with tests have some priority

Projects

Status: 🏁 Review

Development

Successfully merging this pull request may close these issues.

8 participants