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

Warn about untracked files in git repositories #7244

Closed

Conversation

rti
Copy link

@rti rti commented Oct 31, 2022

This adds a warning about untracked files in fetched git repositories. Addresses #7107 and hopefully helps to avoid some confusion. Maybe this can help as a simple fix before #6530 gets merged.

@SuperSandro2000
Copy link
Member

Would this be disabled with warn-dirty = false?

@rti
Copy link
Author

rti commented Nov 2, 2022

Would this be disabled with warn-dirty = false?

At the moment, not. Should it?

@edolstra
Copy link
Member

edolstra commented Nov 2, 2022

I'd like to postpone this until after #6530 is merged to avoid creating merge conflicts.

@Artturin
Copy link
Member

Artturin commented Nov 3, 2022

Would this be disabled with warn-dirty = false?

At the moment, not. Should it?

yeah that'd be nice, some users have warn-dirty = false because we don't need these warnings.

@rti
Copy link
Author

rti commented Nov 10, 2022

As I understand it, #6530 introduces a warning for files not under version control anyway, doesn't it?

@roberth roberth mentioned this pull request Feb 23, 2023
9 tasks
@roberth
Copy link
Member

roberth commented Feb 23, 2023

It'd be better to only warn when an untracked file would be accessed, such as in pathExists, readDir, builtins.path, "${./path}", etc. See also my suggestion about improving cases where it results in an error

@SuperSandro2000
Copy link
Member

yeah that'd be nice, some users have warn-dirty = false because we don't need these warnings.

I don't consider this nice to have but absolutely required to not spam even more warnings when building.

@roberth
Copy link
Member

roberth commented Feb 28, 2023

I'd consider dirtiness and having untracked files to be rather different scenarios.
If the user has presumably committed all their files, the outcome is a non-dirty build, as it has a valid commit id. Nonetheless, their evaluation may try to access untracked files.
I think warning about dirty repos might be useful to some, when referencing repositories that aren't the current working directory, although personally I'd like to silence them anyway, as they occur too often and I ignore them anyway.
Attempts to access untracked files on the other hand are indicative of a real problem that can be resolved by adding the file to git or to .gitignore.

I would avoid warning about files that aren't accessed though. Some people keep untracked files around and unnecessary warnings could again lead to warning fatigue, just like we have with the dirty warning.

@fricklerhandwerk fricklerhandwerk added error-messages Confusing messages and better diagnostics flakes labels Mar 3, 2023
@fricklerhandwerk
Copy link
Contributor

fricklerhandwerk commented Mar 22, 2023

Triaged in the Nix team meeting 2023-03-17:

This was discussed briefly: the approach is too eager, we should only warn on access. No concrete decision was made how to proceed. @rti are you still interested in pursuing this?

@nixos-discourse
Copy link

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

https://discourse.nixos.org/t/2023-03-17-nix-team-meeting-minutes-41/26614/1

@rti
Copy link
Author

rti commented Mar 29, 2023

@fricklerhandwerk Thanks a lot for the update. Unfortunately I don't have the resources for this any time soon. So I will just close this PR for now.

@rti rti closed this Mar 29, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
error-messages Confusing messages and better diagnostics flakes
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

None yet

7 participants