nix-gitignore: filter-out .git #94960
Conversation
| @@ -98,7 +98,7 @@ in rec { | |||
| let | |||
| onPath = f: a: if typeOf a == "path" then f a else a; | |||
| str_patterns = map (onPath readFile) (lib.toList file_str_patterns); | |||
| in concatStringsSep "\n" str_patterns; | |||
| in ".git\n" + (concatStringsSep "\n" str_patterns); | |||
There was a problem hiding this comment.
I think it indeed makes some sense to add .git to the default filters for gitignoreSource.
This doesn't seem to be the right place to put this default value to me, though:
The 2 documented entry points for nix-gitignore are gitignoreSource and gitignoreSourcePure. They both call gitignoreFilterSourcePure, but gitignoreSource adds .gitignore to the parameters (via withGitignoreFile), while gitignoreSourcePure does not.
To be consistent I think it the additional default .git ignore should only be added to gitignoreSource, and gitignoreSourcePure should remain 'pure'. So I think .git should be added to withGitignoreFile (perhaps renamed to withDefaults or something?) instead of gitignoreCompileIgnore
There was a problem hiding this comment.
My rationale was: "gitignoreSource works like git and gitignoreSourcePure like git in a repo where there is no .gitignore". What do you think?
There was a problem hiding this comment.
That's a bit strange when there actually is a .gitignore present in that directory, though, right?
To me the interpretation 'gitignoreSource has defaults to work like git and gitignoreSourcePure gives you complete control' makes more sense.
There was a problem hiding this comment.
Ok makes sense, I changed the PR to only affect the non pure variant.
|
cc @siers as original author ? |
|
Go for it. :) |
|
|
||
| withRecursiveGitignoreFile = patterns: root: | ||
| lib.toList patterns ++ [(compileRecursiveGitignore root)]; | ||
| lib.toList patterns ++ [ ".git" ] ++ [(compileRecursiveGitignore root)]; |
symphorien
force-pushed
the
gitignore
branch
from
c694782
to
a5a383a
Compare
|
How did you test it?
…On Wed, Nov 11, 2020 at 10:06 PM symphorien ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In pkgs/build-support/nix-gitignore/default.nix
<#94960 (comment)>:
> @@ -148,10 +148,10 @@ in rec {
'');
withGitignoreFile = patterns: root:
- lib.toList patterns ++ [(root + "/.gitignore")];
+ lib.toList patterns ++ [ ".git" ] ++ [(root + "/.gitignore")];
I undid this after testing:
error: reading from file: Is a directory
because ".git" is a gitignore patter, not a path to a file to be read.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#94960 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AABZNC3RRJFUEGPFNN4QJATSPLVEVANCNFSM4PYZRTEA>
.
--
Raitis Veinbahs, +371 2236 4708
|
|
I assume you know, but the nix-gitignore in siers/nix-gitignore has the
`.git` pattern added and it seems to be working judging by no issues filed
about this.
…On Thu, Nov 12, 2020 at 11:07 AM Raitis Veinbahs ***@***.***> wrote:
How did you test it?
On Wed, Nov 11, 2020 at 10:06 PM symphorien ***@***.***>
wrote:
> ***@***.**** commented on this pull request.
> ------------------------------
>
> In pkgs/build-support/nix-gitignore/default.nix
> <#94960 (comment)>:
>
> > @@ -148,10 +148,10 @@ in rec {
> '');
>
> withGitignoreFile = patterns: root:
> - lib.toList patterns ++ [(root + "/.gitignore")];
> + lib.toList patterns ++ [ ".git" ] ++ [(root + "/.gitignore")];
>
> I undid this after testing:
>
> error: reading from file: Is a directory
>
> because ".git" is a gitignore patter, not a path to a file to be read.
>
> —
> You are receiving this because you were mentioned.
> Reply to this email directly, view it on GitHub
> <#94960 (comment)>, or
> unsubscribe
> <https://github.com/notifications/unsubscribe-auth/AABZNC3RRJFUEGPFNN4QJATSPLVEVANCNFSM4PYZRTEA>
> .
>
--
Raitis Veinbahs, +371 2236 4708
--
Raitis Veinbahs, +371 2236 4708
|
See the reproduction steps of the linked issue
No, I did not know siers/nix-gitignore existed before opening this PR. Anyway I'd like to keep using the one in nixpkgs. |
|
@GrahamcOfBorg eval |
fixes #94959
Things done
sandboxinnix.confon non-NixOS linux)nix-shell -p nixpkgs-review --run "nixpkgs-review wip"./result/bin/)nix path-info -Sbefore and after)