threads / patch / 55175

patch, 2 partsadd: add option --no-filters to disable attribute-based filtering

Subject: [PATCH 1/2] add: add option --no-filters to disable attribute-based filtering

## tl;dr

11 messages between Feb 19, 2021 and Feb 20, 2021. Diffs are folded; open one to read it.

replies: 10people: 3as markdown or json

Andrej Shadura via GitGitGadget· Feb 19, 2021, 18:12 UTC · lore

[PATCH 0/2] Add --no-filters option to git-add

It is possible for a user to disable attribute-based filtering when committing by doing one of the following:

 * Create .git/info/attributes unapplying all possible transforming
   attributes.
 * Use git hash-object and git update-index to stage files manually.

Doing the former requires keeping an up-to-date list of all attributes which can transform files when committing or checking out. Doing the latter is difficult, error-prone and slow when done from scripts.

Instead, similarly to git hash-object, --no-filter can be added to git add to enable temporarily disabling filtering in an easy to use way.

These patches:
 * Add new flag ADD_CACHE_RAW to add_to_index()
 * Add new flag HASH_RAW to index_fd()
 * Make git hash-object use the new HASH_RAW flag for consistency
 * Add tests for the new git-add option.
Andrej Shadura (2):
  add: add option --no-filters to disable attribute-based filtering
  hash-object: use the new HASH_RAW flag instead of setting path to NULL
 Documentation/git-add.txt |  7 +++++-
 builtin/add.c             |  3 +++
 builtin/hash-object.c     | 17 ++++++---------
 cache.h                   |  2 ++
 object-file.c             |  2 +-
 read-cache.c              |  3 +++
 t/t2205-add-no-filters.sh | 46 +++++++++++++++++++++++++++++++++++++++
 7 files changed, 68 insertions(+), 12 deletions(-)
 create mode 100755 t/t2205-add-no-filters.sh
base-commit: 2283e0e9af55689215afa39c03beb2315ce18e83
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-880%2Fandrewshadura%2Fgit-add-no-filters-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-880/andrewshadura/git-add-no-filters-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/880
-- 
gitgitgadget
Andrej Shadura via GitGitGadget· Feb 19, 2021, 18:12 UTC · re: Andrej Shadura via GitGitGadget · lore
From: Andrej Shadura <andrew.shadura@collabora.co.uk>

It is possible for a user to disable attribute-based filtering when committing by doing one of the following:

* Create .git/info/attributes unapplying all possible transforming
  attributes.
* Use git hash-object and git update-index to stage files manually.

Doing the former requires keeping an up-to-date list of all attributes which can transform files when committing or checking out. Doing the latter is difficult, error-prone and slow when done from scripts.

Instead, similarly to git hash-object, --no-filter can be added to git add to enable temporarily disabling filtering in an easy to use way:

* Add new flag ADD_CACHE_RAW to add_to_index()
* Add new flag HASH_RAW to index_fd()
Signed-off-by: Andrej Shadura <andrew.shadura@collabora.co.uk>
---
 Documentation/git-add.txt |  7 +++++-
 builtin/add.c             |  3 +++
 cache.h                   |  2 ++
 object-file.c             |  2 +-
 read-cache.c              |  3 +++
 t/t2205-add-no-filters.sh | 46 +++++++++++++++++++++++++++++++++++++++
 6 files changed, 61 insertions(+), 2 deletions(-)
 create mode 100755 t/t2205-add-no-filters.sh
Show changes to 6 files +61 −2

Documentation/git-add.txt, builtin/add.c, cache.h, object-file.c, read-cache.c, t/t2205-add-no-filters.sh

diff --git a/Documentation/git-add.txt b/Documentation/git-add.txt
index be5e3ac54b85..b3d7b13e83f4 100644
--- a/Documentation/git-add.txt
+++ b/Documentation/git-add.txt
@@ -9,7 +9,7 @@ SYNOPSIS
 --------
 [verse]
 'git add' [--verbose | -v] [--dry-run | -n] [--force | -f] [--interactive | -i] [--patch | -p]
-	  [--edit | -e] [--[no-]all | --[no-]ignore-removal | [--update | -u]]
+	  [--edit | -e] [--[no-]all | --[no-]ignore-removal | [--update | -u]] [--no-filters]
 	  [--intent-to-add | -N] [--refresh] [--ignore-errors] [--ignore-missing] [--renormalize]
 	  [--chmod=(+|-)x] [--pathspec-from-file=<file> [--pathspec-file-nul]]
 	  [--] [<pathspec>...]
@@ -183,6 +183,11 @@ for "git add --no-all <pathspec>...", i.e. ignored removed files.
 	in order to correct files added with wrong CRLF/LF line endings.
 	This option implies `-u`.
 
+--no-filters::
+	Add the contents as is, ignoring any input filter that would
+	have been chosen by the attributes mechanism, including the end-of-line
+	conversion.
+
 --chmod=(+|-)x::
 	Override the executable bit of the added files.  The executable
 	bit is only changed in the index, the files on disk are left
diff --git a/builtin/add.c b/builtin/add.c
index a825887c503d..609a0e6c0157 100644
--- a/builtin/add.c
+++ b/builtin/add.c
@@ -29,6 +29,7 @@ static const char * const builtin_add_usage[] = {
 static int patch_interactive, add_interactive, edit_interactive;
 static int take_worktree_changes;
 static int add_renormalize;
+static int no_filters;
 static int pathspec_file_nul;
 static const char *pathspec_from_file;
 static int legacy_stash_p; /* support for the scripted `git stash` */
@@ -334,6 +335,7 @@ static struct option builtin_add_options[] = {
 	OPT__FORCE(&ignored_too, N_("allow adding otherwise ignored files"), 0),
 	OPT_BOOL('u', "update", &take_worktree_changes, N_("update tracked files")),
 	OPT_BOOL(0, "renormalize", &add_renormalize, N_("renormalize EOL of tracked files (implies -u)")),
+	OPT_BOOL(0 , "no-filters", &no_filters, N_("store file as is without filters")),
 	OPT_BOOL('N', "intent-to-add", &intent_to_add, N_("record only the fact that the path will be added later")),
 	OPT_BOOL('A', "all", &addremove_explicit, N_("add changes from all tracked and untracked files")),
 	OPT_CALLBACK_F(0, "ignore-removal", &addremove_explicit,
@@ -531,6 +533,7 @@ int cmd_add(int argc, const char **argv, const char *prefix)
 		 (show_only ? ADD_CACHE_PRETEND : 0) |
 		 (intent_to_add ? ADD_CACHE_INTENT : 0) |
 		 (ignore_add_errors ? ADD_CACHE_IGNORE_ERRORS : 0) |
+		 (no_filters ? ADD_CACHE_RAW : 0) |
 		 (!(addremove || take_worktree_changes)
 		  ? ADD_CACHE_IGNORE_REMOVAL : 0));
 
diff --git a/cache.h b/cache.h
index d92814961405..df83b5709c32 100644
--- a/cache.h
+++ b/cache.h
@@ -820,6 +820,7 @@ int remove_file_from_index(struct index_state *, const char *path);
 #define ADD_CACHE_IGNORE_ERRORS	4
 #define ADD_CACHE_IGNORE_REMOVAL 8
 #define ADD_CACHE_INTENT 16
+#define ADD_CACHE_RAW 32
 /*
  * These two are used to add the contents of the file at path
  * to the index, marking the working tree up-to-date by storing
@@ -858,6 +859,7 @@ int ie_modified(struct index_state *, const struct cache_entry *, struct stat *,
 #define HASH_WRITE_OBJECT 1
 #define HASH_FORMAT_CHECK 2
 #define HASH_RENORMALIZE  4
+#define HASH_RAW          8
 int index_fd(struct index_state *istate, struct object_id *oid, int fd, struct stat *st, enum object_type type, const char *path, unsigned flags);
 int index_path(struct index_state *istate, struct object_id *oid, const char *path, struct stat *st, unsigned flags);
 
diff --git a/object-file.c b/object-file.c
index 5bcfde847188..74487c22e586 100644
--- a/object-file.c
+++ b/object-file.c
@@ -2067,7 +2067,7 @@ static int index_mem(struct index_state *istate,
 	/*
 	 * Convert blobs to git internal format
 	 */
-	if ((type == OBJ_BLOB) && path) {
+	if ((type == OBJ_BLOB) && path && !(flags & HASH_RAW)) {
 		struct strbuf nbuf = STRBUF_INIT;
 		if (convert_to_git(istate, path, buf, size, &nbuf,
 				   get_conv_flags(flags))) {
diff --git a/read-cache.c b/read-cache.c
index 29144cf879e7..0fb31201c705 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -716,6 +716,9 @@ int add_to_index(struct index_state *istate, const char *path, struct stat *st,
 	if (flags & ADD_CACHE_RENORMALIZE)
 		hash_flags |= HASH_RENORMALIZE;
 
+	if (flags & ADD_CACHE_RAW)
+		hash_flags |= HASH_RAW;
+
 	if (!S_ISREG(st_mode) && !S_ISLNK(st_mode) && !S_ISDIR(st_mode))
 		return error(_("%s: can only add regular files, symbolic links or git-directories"), path);
 
diff --git a/t/t2205-add-no-filters.sh b/t/t2205-add-no-filters.sh
new file mode 100755
index 000000000000..a897ed810827
--- /dev/null
+++ b/t/t2205-add-no-filters.sh
@@ -0,0 +1,46 @@
+#!/bin/sh
+
+test_description='git add --no-filters
+
+This test creates a file and a corresponding .gitattributes setup
+to ensure the file undergoes a conversion when committed or checked
+out.
+
+It then verifies that the conversion happens by default, but does not
+when --no-filters is used.'
+
+. ./test-lib.sh
+
+test_expect_success setup '
+	echo "* eol=crlf" > .gitattributes &&
+	git add .gitattributes &&
+	git commit -m initial &&
+	printf "test\r\ntest\r\n" > test
+'
+
+test_expect_success 'add without --no-filters' '
+	original="$(git hash-object --stdin < test)" &&
+	converted="$(git hash-object test)" &&
+	git add test &&
+	git ls-files -s > actual &&
+	cat > expected <<-EOF &&
+	100644 $(git hash-object .gitattributes) 0	.gitattributes
+	100644 $converted 0	test
+	EOF
+	test_cmp expected actual
+'
+
+test_expect_success 'add with --no-filters' '
+	git rm -f --cached test &&
+	original="$(git hash-object --stdin < test)" &&
+	converted="$(git hash-object test)" &&
+	git add --no-filters test &&
+	git ls-files -s > actual &&
+	cat > expected <<-EOF &&
+	100644 $(git hash-object .gitattributes) 0	.gitattributes
+	100644 $original 0	test
+	EOF
+	test_cmp expected actual
+'
+
+test_done
-- 
gitgitgadget
Andrej Shadura via GitGitGadget· Feb 19, 2021, 18:12 UTC · re: Andrej Shadura via GitGitGadget · lore

[PATCH 2/2] hash-object: use the new HASH_RAW flag instead of setting path to NULL

From: Andrej Shadura <andrew.shadura@collabora.co.uk>

While setting path to NULL works and flips the condition in the right branch inside index_mem(), doing so isn’t obvious for the reader of the code. Since index_mem() now has an additional flag to disable filtering, use that instead.

Signed-off-by: Andrej Shadura <andrew.shadura@collabora.co.uk>
---
 builtin/hash-object.c | 17 +++++++----------
 1 file changed, 7 insertions(+), 10 deletions(-)
Show changes to builtin/hash-object.c +7 −10
diff --git a/builtin/hash-object.c b/builtin/hash-object.c
index 640ef4ded595..8e0543b12bc5 100644
--- a/builtin/hash-object.c
+++ b/builtin/hash-object.c
@@ -59,8 +59,7 @@ static void hash_object(const char *path, const char *type, const char *vpath,
 	hash_fd(fd, type, vpath, flags, literally);
 }
 
-static void hash_stdin_paths(const char *type, int no_filters, unsigned flags,
-			     int literally)
+static void hash_stdin_paths(const char *type, unsigned flags, int literally)
 {
 	struct strbuf buf = STRBUF_INIT;
 	struct strbuf unquoted = STRBUF_INIT;
@@ -72,8 +71,7 @@ static void hash_stdin_paths(const char *type, int no_filters, unsigned flags,
 				die("line is badly quoted");
 			strbuf_swap(&buf, &unquoted);
 		}
-		hash_object(buf.buf, type, no_filters ? NULL : buf.buf, flags,
-			    literally);
+		hash_object(buf.buf, type, buf.buf, flags, literally);
 	}
 	strbuf_release(&buf);
 	strbuf_release(&unquoted);
@@ -89,7 +87,6 @@ int cmd_hash_object(int argc, const char **argv, const char *prefix)
 	const char *type = blob_type;
 	int hashstdin = 0;
 	int stdin_paths = 0;
-	int no_filters = 0;
 	int literally = 0;
 	int nongit = 0;
 	unsigned flags = HASH_FORMAT_CHECK;
@@ -100,7 +97,8 @@ int cmd_hash_object(int argc, const char **argv, const char *prefix)
 			HASH_WRITE_OBJECT),
 		OPT_COUNTUP( 0 , "stdin", &hashstdin, N_("read the object from stdin")),
 		OPT_BOOL( 0 , "stdin-paths", &stdin_paths, N_("read file names from stdin")),
-		OPT_BOOL( 0 , "no-filters", &no_filters, N_("store file as is without filters")),
+		OPT_BIT(0 , "no-filters", &flags, N_("store file as is without filters"),
+			HASH_RAW),
 		OPT_BOOL( 0, "literally", &literally, N_("just hash any random garbage to create corrupt objects for debugging Git")),
 		OPT_STRING( 0 , "path", &vpath, N_("file"), N_("process file as it were from this path")),
 		OPT_END()
@@ -132,7 +130,7 @@ int cmd_hash_object(int argc, const char **argv, const char *prefix)
 	else {
 		if (hashstdin > 1)
 			errstr = "Multiple --stdin arguments are not supported";
-		if (vpath && no_filters)
+		if (vpath && (flags & HASH_RAW))
 			errstr = "Can't use --path with --no-filters";
 	}
 
@@ -150,13 +148,12 @@ int cmd_hash_object(int argc, const char **argv, const char *prefix)
 
 		if (prefix)
 			arg = to_free = prefix_filename(prefix, arg);
-		hash_object(arg, type, no_filters ? NULL : vpath ? vpath : arg,
-			    flags, literally);
+		hash_object(arg, type, vpath ? vpath : arg, flags, literally);
 		free(to_free);
 	}
 
 	if (stdin_paths)
-		hash_stdin_paths(type, no_filters, flags, literally);
+		hash_stdin_paths(type, flags, literally);
 
 	return 0;
 }
-- 
gitgitgadget
brian m. carlson· Feb 19, 2021, 22:36 UTC · re: Andrej Shadura via GitGitGadget · lore

Re: [PATCH 0/2] Add --no-filters option to git-add

On 2021-02-19 at 18:12:11, Andrej Shadura via GitGitGadget wrote:
Show 20 quoted lines
> It is possible for a user to disable attribute-based filtering when
> committing by doing one of the following:
> 
>  * Create .git/info/attributes unapplying all possible transforming
>    attributes.
>  * Use git hash-object and git update-index to stage files manually.
> 
> Doing the former requires keeping an up-to-date list of all attributes which
> can transform files when committing or checking out. Doing the latter is
> difficult, error-prone and slow when done from scripts.
> 
> Instead, similarly to git hash-object, --no-filter can be added to git add
> to enable temporarily disabling filtering in an easy to use way.
> 
> These patches:
> 
>  * Add new flag ADD_CACHE_RAW to add_to_index()
>  * Add new flag HASH_RAW to index_fd()
>  * Make git hash-object use the new HASH_RAW flag for consistency
>  * Add tests for the new git-add option.

I'm interested in your use cases here. While I agree that this is an interesting feature, it also means that practically, a user who checks out a file that's added this way may find that git status marks it as perpetually modified until a properly cleaned version is committed. Moreover, even "git reset --hard" won't fix this situation.

We see this problem extremely frequently with Git LFS where people change the .gitattributes file but don't run "git add --renormalize ." and then end up with this problem. However, it's not limited to Git LFS in particular; anything that uses filters, working tree encodings, or end of line attributes can be affected.

So I think that while this might be a useful escape hatch for users, I definitely want to see a compelling rationale for it and a big warning in the documentation and an update to the relevant entry in the Git FAQ before we accept such a patch.

-- 
brian m. carlson (he/him or they/them)
Houston, Texas, US
Andrej Shadura· Feb 20, 2021, 08:06 UTC · re: brian m. carlson · lore

Re: [PATCH 0/2] Add --no-filters option to git-add

On 19/02/2021 23:36, brian m. carlson wrote:
Show 14 quoted lines
> On 2021-02-19 at 18:12:11, Andrej Shadura via GitGitGadget wrote:
>> It is possible for a user to disable attribute-based filtering when
>> committing by doing one of the following:
>>
>>  * Create .git/info/attributes unapplying all possible transforming
>>    attributes.
>>  * Use git hash-object and git update-index to stage files manually.
>>
>> Doing the former requires keeping an up-to-date list of all attributes which
>> can transform files when committing or checking out. Doing the latter is
>> difficult, error-prone and slow when done from scripts.
>>
>> Instead, similarly to git hash-object, --no-filter can be added to git add
>> to enable temporarily disabling filtering in an easy to use way.
Show 16 quoted lines
> I'm interested in your use cases here.  While I agree that this is an
> interesting feature, it also means that practically, a user who checks
> out a file that's added this way may find that git status marks it as
> perpetually modified until a properly cleaned version is committed.
> Moreover, even "git reset --hard" won't fix this situation.
> 
> We see this problem extremely frequently with Git LFS where people
> change the .gitattributes file but don't run "git add --renormalize ."
> and then end up with this problem.  However, it's not limited to Git LFS
> in particular; anything that uses filters, working tree encodings, or
> end of line attributes can be affected.
> 
> So I think that while this might be a useful escape hatch for users, I
> definitely want to see a compelling rationale for it and a big warning
> in the documentation and an update to the relevant entry in the Git FAQ
> before we accept such a patch.

My use case here is mostly non-interactive use in scripts creating Git trees that need to exactly correspond to a working directory regardless of whether or not they have any .gitattributes files.

For example, for git-buildpackage or dgit, which facilitate Git workflows with Debian packages, want to ensure the contents of the packages can be exactly reproduced, which is difficult if the upstream’s tarball has .gitattributes. It is possible to "defuse" the attributes as demonstrated above, but this will break if the user modifies the .git/i/a file *or* if Git adds more attribute-based conversions. This is what dgit currently does, and this is what git-buildpackage will soon do too.

Of course, this patch set only addresses staging files. While working on a patch to git-buildpackage to reproducibly import the contents of tarballs, I realised that the only realistic way seem to do that is to use hash-object + update-index manually, which is likely to come with a performance drop compared to git add (which is what gbp currently uses). A workaround might be to use dulwich (which would allow to do hash-object without fork/exec) or perhaps GitPython (which I haven’t really looked into), or maybe to use git fast-import, but all of these alternatives are quite complex and don’t guarantee the same performance.

Adding a new option to git add allows to keep the performance without having to ensure attributes are set to the right values. The attributes will likely have to be adjusted anyway for user’s convenience, but at least if they modify them afterwards, the tools won’t break.

-- 
Cheers,
  Andrej
Andrej Shadura· Feb 20, 2021, 09:30 UTC · re: Andrej Shadura · lore

Re: [PATCH 0/2] Add --no-filters option to git-add

On 20/02/2021 09:06, Andrej Shadura wrote:
Show 5 quoted lines
> On 19/02/2021 23:36, brian m. carlson wrote:
>> So I think that while this might be a useful escape hatch for users, I
>> definitely want to see a compelling rationale for it and a big warning
>> in the documentation and an update to the relevant entry in the Git FAQ
>> before we accept such a patch.
Here’s my proposal for the updated manpage description of the option:
--no-filters::

Add the contents as is, ignoring any input filter that would have been chosen by the attributes mechanism, including the end-of-line conversion. Note that this option is not intended for interactive use, since files added this way will always show up as modified if Git were to apply transformations to them, making the situation potentially very confusing.

And here the FAQ entry extended:

It is also possible for perpetually modified files to occur on any platform if a smudge or clean filter is in use on your system but a file was previously committed without running the smudge or clean filter. To fix this, run the following on an otherwise clean working tree: + ---- $ git add --renormalize . ---- + Another situation where perpetually modified may appear on any platform is when a file has been committed without running any filters (including the end-of-line conversion), but the `.gitattributes` file states that this file requires a conversion. In this case, you can either renormalize the files if this happened by mistake, or modify `.gitattributes` or `$GIT_DIR/info/attributes` as described above to exempt the file from the conversion if this was intentional.

(I will send an updated patch set when we agree on the wording.)
-- 
Cheers,
  Andrej
brian m. carlson· Feb 20, 2021, 14:10 UTC · re: Andrej Shadura · lore

Re: [PATCH 0/2] Add --no-filters option to git-add

On 2021-02-20 at 09:30:58, Andrej Shadura wrote:
Show 38 quoted lines
> On 20/02/2021 09:06, Andrej Shadura wrote:
> > On 19/02/2021 23:36, brian m. carlson wrote:
> >> So I think that while this might be a useful escape hatch for users, I
> >> definitely want to see a compelling rationale for it and a big warning
> >> in the documentation and an update to the relevant entry in the Git FAQ
> >> before we accept such a patch.
> 
> Here’s my proposal for the updated manpage description of the option:
> 
> --no-filters::
> 
> Add the contents as is, ignoring any input filter that would have been
> chosen by the attributes mechanism, including the end-of-line
> conversion. Note that this option is not intended for interactive use,
> since files added this way will always show up as modified if Git were
> to apply transformations to them, making the situation potentially very
> confusing.
> 
> And here the FAQ entry extended:
> 
> It is also possible for perpetually modified files to occur on any
> platform if a smudge or clean filter is in use on your system but a file
> was previously committed without running the smudge or clean filter.  To
> fix this, run the following on an otherwise clean working tree:
> +
> ----
> $ git add --renormalize .
> ----
> +
> Another situation where perpetually modified may appear on any platform
> is when a file has been committed without running any filters (including
> the end-of-line conversion), but the `.gitattributes` file states that
> this file requires a conversion.  In this case, you can either
> renormalize the files if this happened by mistake, or modify
> `.gitattributes` or `$GIT_DIR/info/attributes` as described above to
> exempt the file from the conversion if this was intentional.
> 
> (I will send an updated patch set when we agree on the wording.)

This seems fine. Thanks for being open to addressing my concerns, and I agree that your use case seems like a good one and that this is a valuable feature.

-- 
brian m. carlson (he/him or they/them)
Houston, Texas, US
Andrej Shadura via GitGitGadget· Feb 20, 2021, 17:07 UTC · re: Andrej Shadura via GitGitGadget · lore

[PATCH v2 0/2] Add --no-filters option to git-add

It is possible for a user to disable attribute-based filtering when committing by doing one of the following:

 * Create .git/info/attributes unapplying all possible transforming
   attributes.
 * Use git hash-object and git update-index to stage files manually.

Doing the former requires keeping an up-to-date list of all attributes which can transform files when committing or checking out. Doing the latter is difficult, error-prone and slow when done from scripts.

Instead, similarly to git hash-object, --no-filter can be added to git add to enable temporarily disabling filtering in an easy to use way.

The use case here is mostly non-interactive use in scripts creating Git trees that need to exactly correspond to a working directory regardless of whether or not they have any .gitattributes files.

For example, for git-buildpackage or dgit, which facilitate Git workflows with Debian packages, want to ensure the contents of the packages can be exactly reproduced, which is difficult if the upstream’s tarball has .gitattributes. It is possible to "defuse" the attributes as demonstrated above, but this will break if the user modifies the .git/i/a file or if Git adds more attribute-based conversions. This is what dgit currently does, and this is what git-buildpackage will soon do too.

Of course, this patch set only addresses staging files. While working on a patch to git-buildpackage to reproducibly import the contents of tarballs, I realised that the only realistic way seem to do that is to use hash-object + update-index manually, which is likely to come with a performance drop compared to git add (which is what gbp currently uses). A workaround might be to use Dulwich (which would allow to do hash-object without fork/exec) or perhaps GitPython (which I haven’t really looked into), or maybe to use git fast-import, but all of these alternatives are quite complex and don’t guarantee the same performance.

Adding a new option to git add allows to keep the performance without having to ensure attributes are set to the right values. The attributes will likely have to be adjusted anyway for user’s convenience, but at least if they modify them afterwards, the tools won’t break.

These patches:
 * Add new flag ADD_CACHE_RAW to add_to_index()
 * Add new flag HASH_RAW to index_fd()
 * Make git hash-object use the new HASH_RAW flag for consistency
 * Add tests for the new git-add option.
 * Update the git-add manpage describing the new option and pointing out
   it’s tricky to use.
 * Expand the relevant FAQ entry adding --no-filters as yet one more reason
   for perpetually modified files to appear.
Changes since v1:
 * Removed an extra space left in the option definition. Jessica Clarke on
   GitHub pointed out the inconsistent formatting, but contrary to the
   suggestion I updated the style to the more widespread variation: "(0,"
   occurs at least 689 times across the codebase, while "( 0" only at most
   23.
 * Expanded the option description to warn users about potentially confusing
   situations (pointed out by brian m. carlson)
 * Expanded the FAQ entry to mention the new option as one of the cause of
   perpetually modified files.
Andrej Shadura (2):
  add: add option --no-filters to disable attribute-based filtering
  hash-object: use the new HASH_RAW flag instead of setting path to NULL
 Documentation/git-add.txt | 10 ++++++++-
 Documentation/gitfaq.txt  |  7 ++++++
 builtin/add.c             |  3 +++
 builtin/hash-object.c     | 17 ++++++---------
 cache.h                   |  2 ++
 object-file.c             |  2 +-
 read-cache.c              |  3 +++
 t/t2205-add-no-filters.sh | 46 +++++++++++++++++++++++++++++++++++++++
 8 files changed, 78 insertions(+), 12 deletions(-)
 create mode 100755 t/t2205-add-no-filters.sh
base-commit: 2283e0e9af55689215afa39c03beb2315ce18e83
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-880%2Fandrewshadura%2Fgit-add-no-filters-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-880/andrewshadura/git-add-no-filters-v2
Pull-Request: https://github.com/gitgitgadget/git/pull/880
Range-diff vs v1:
 1:  cf8eb886a1e2 ! 1:  1d84503fff48 add: add option --no-filters to disable attribute-based filtering
     @@ Documentation/git-add.txt: for "git add --no-all <pathspec>...", i.e. ignored re
      +--no-filters::
      +	Add the contents as is, ignoring any input filter that would
      +	have been chosen by the attributes mechanism, including the end-of-line
     -+	conversion.
     ++	conversion. Note that this option is not intended for interactive use,
     ++	since files added this way will always show up as modified if Git were
     ++	to apply transformations to them, making the situation potentially
     ++	very confusing.
      +
       --chmod=(+|-)x::
       	Override the executable bit of the added files.  The executable
       	bit is only changed in the index, the files on disk are left
      
     + ## Documentation/gitfaq.txt ##
     +@@ Documentation/gitfaq.txt: following on an otherwise clean working tree:
     + ----
     + $ git add --renormalize .
     + ----
     +++
     ++Another situation where perpetually modified may appear on any platform is when
     ++a file has been committed without running any filters (including the end-of-line
     ++conversion), but the `.gitattributes` file states that this file requires a
     ++conversion.  In this case, you can either renormalize the files if this happened
     ++by mistake, or modify `.gitattributes` or `$GIT_DIR/info/attributes` as described
     ++above to exempt the file from the conversion if this was intentional.
     + 
     + [[recommended-storage-settings]]
     + What's the recommended way to store files in Git?::
     +
       ## builtin/add.c ##
      @@ builtin/add.c: static const char * const builtin_add_usage[] = {
       static int patch_interactive, add_interactive, edit_interactive;
 2:  0e113e958b35 ! 2:  810d4005fe8f hash-object: use the new HASH_RAW flag instead of setting path to NULL
     @@ builtin/hash-object.c: int cmd_hash_object(int argc, const char **argv, const ch
       		OPT_COUNTUP( 0 , "stdin", &hashstdin, N_("read the object from stdin")),
       		OPT_BOOL( 0 , "stdin-paths", &stdin_paths, N_("read file names from stdin")),
      -		OPT_BOOL( 0 , "no-filters", &no_filters, N_("store file as is without filters")),
     -+		OPT_BIT(0 , "no-filters", &flags, N_("store file as is without filters"),
     ++		OPT_BIT(0, "no-filters", &flags, N_("store file as is without filters"),
      +			HASH_RAW),
       		OPT_BOOL( 0, "literally", &literally, N_("just hash any random garbage to create corrupt objects for debugging Git")),
       		OPT_STRING( 0 , "path", &vpath, N_("file"), N_("process file as it were from this path")),
-- 
gitgitgadget
Andrej Shadura via GitGitGadget· Feb 20, 2021, 17:07 UTC · re: Andrej Shadura via GitGitGadget · lore

[PATCH v2 1/2] add: add option --no-filters to disable attribute-based filtering

From: Andrej Shadura <andrew.shadura@collabora.co.uk>

It is possible for a user to disable attribute-based filtering when committing by doing one of the following:

* Create .git/info/attributes unapplying all possible transforming
  attributes.
* Use git hash-object and git update-index to stage files manually.

Doing the former requires keeping an up-to-date list of all attributes which can transform files when committing or checking out. Doing the latter is difficult, error-prone and slow when done from scripts.

Instead, similarly to git hash-object, --no-filter can be added to git add to enable temporarily disabling filtering in an easy to use way:

* Add new flag ADD_CACHE_RAW to add_to_index()
* Add new flag HASH_RAW to index_fd()
Signed-off-by: Andrej Shadura <andrew.shadura@collabora.co.uk>
---
 Documentation/git-add.txt | 10 ++++++++-
 Documentation/gitfaq.txt  |  7 ++++++
 builtin/add.c             |  3 +++
 cache.h                   |  2 ++
 object-file.c             |  2 +-
 read-cache.c              |  3 +++
 t/t2205-add-no-filters.sh | 46 +++++++++++++++++++++++++++++++++++++++
 7 files changed, 71 insertions(+), 2 deletions(-)
 create mode 100755 t/t2205-add-no-filters.sh
Show changes to 7 files +71 −2

Documentation/git-add.txt, Documentation/gitfaq.txt, builtin/add.c, cache.h, object-file.c, read-cache.c, t/t2205-add-no-filters.sh

diff --git a/Documentation/git-add.txt b/Documentation/git-add.txt
index be5e3ac54b85..75b474a88fca 100644
--- a/Documentation/git-add.txt
+++ b/Documentation/git-add.txt
@@ -9,7 +9,7 @@ SYNOPSIS
 --------
 [verse]
 'git add' [--verbose | -v] [--dry-run | -n] [--force | -f] [--interactive | -i] [--patch | -p]
-	  [--edit | -e] [--[no-]all | --[no-]ignore-removal | [--update | -u]]
+	  [--edit | -e] [--[no-]all | --[no-]ignore-removal | [--update | -u]] [--no-filters]
 	  [--intent-to-add | -N] [--refresh] [--ignore-errors] [--ignore-missing] [--renormalize]
 	  [--chmod=(+|-)x] [--pathspec-from-file=<file> [--pathspec-file-nul]]
 	  [--] [<pathspec>...]
@@ -183,6 +183,14 @@ for "git add --no-all <pathspec>...", i.e. ignored removed files.
 	in order to correct files added with wrong CRLF/LF line endings.
 	This option implies `-u`.
 
+--no-filters::
+	Add the contents as is, ignoring any input filter that would
+	have been chosen by the attributes mechanism, including the end-of-line
+	conversion. Note that this option is not intended for interactive use,
+	since files added this way will always show up as modified if Git were
+	to apply transformations to them, making the situation potentially
+	very confusing.
+
 --chmod=(+|-)x::
 	Override the executable bit of the added files.  The executable
 	bit is only changed in the index, the files on disk are left
diff --git a/Documentation/gitfaq.txt b/Documentation/gitfaq.txt
index afdaeab8503c..6011abca0bea 100644
--- a/Documentation/gitfaq.txt
+++ b/Documentation/gitfaq.txt
@@ -395,6 +395,13 @@ following on an otherwise clean working tree:
 ----
 $ git add --renormalize .
 ----
++
+Another situation where perpetually modified may appear on any platform is when
+a file has been committed without running any filters (including the end-of-line
+conversion), but the `.gitattributes` file states that this file requires a
+conversion.  In this case, you can either renormalize the files if this happened
+by mistake, or modify `.gitattributes` or `$GIT_DIR/info/attributes` as described
+above to exempt the file from the conversion if this was intentional.
 
 [[recommended-storage-settings]]
 What's the recommended way to store files in Git?::
diff --git a/builtin/add.c b/builtin/add.c
index a825887c503d..609a0e6c0157 100644
--- a/builtin/add.c
+++ b/builtin/add.c
@@ -29,6 +29,7 @@ static const char * const builtin_add_usage[] = {
 static int patch_interactive, add_interactive, edit_interactive;
 static int take_worktree_changes;
 static int add_renormalize;
+static int no_filters;
 static int pathspec_file_nul;
 static const char *pathspec_from_file;
 static int legacy_stash_p; /* support for the scripted `git stash` */
@@ -334,6 +335,7 @@ static struct option builtin_add_options[] = {
 	OPT__FORCE(&ignored_too, N_("allow adding otherwise ignored files"), 0),
 	OPT_BOOL('u', "update", &take_worktree_changes, N_("update tracked files")),
 	OPT_BOOL(0, "renormalize", &add_renormalize, N_("renormalize EOL of tracked files (implies -u)")),
+	OPT_BOOL(0 , "no-filters", &no_filters, N_("store file as is without filters")),
 	OPT_BOOL('N', "intent-to-add", &intent_to_add, N_("record only the fact that the path will be added later")),
 	OPT_BOOL('A', "all", &addremove_explicit, N_("add changes from all tracked and untracked files")),
 	OPT_CALLBACK_F(0, "ignore-removal", &addremove_explicit,
@@ -531,6 +533,7 @@ int cmd_add(int argc, const char **argv, const char *prefix)
 		 (show_only ? ADD_CACHE_PRETEND : 0) |
 		 (intent_to_add ? ADD_CACHE_INTENT : 0) |
 		 (ignore_add_errors ? ADD_CACHE_IGNORE_ERRORS : 0) |
+		 (no_filters ? ADD_CACHE_RAW : 0) |
 		 (!(addremove || take_worktree_changes)
 		  ? ADD_CACHE_IGNORE_REMOVAL : 0));
 
diff --git a/cache.h b/cache.h
index d92814961405..df83b5709c32 100644
--- a/cache.h
+++ b/cache.h
@@ -820,6 +820,7 @@ int remove_file_from_index(struct index_state *, const char *path);
 #define ADD_CACHE_IGNORE_ERRORS	4
 #define ADD_CACHE_IGNORE_REMOVAL 8
 #define ADD_CACHE_INTENT 16
+#define ADD_CACHE_RAW 32
 /*
  * These two are used to add the contents of the file at path
  * to the index, marking the working tree up-to-date by storing
@@ -858,6 +859,7 @@ int ie_modified(struct index_state *, const struct cache_entry *, struct stat *,
 #define HASH_WRITE_OBJECT 1
 #define HASH_FORMAT_CHECK 2
 #define HASH_RENORMALIZE  4
+#define HASH_RAW          8
 int index_fd(struct index_state *istate, struct object_id *oid, int fd, struct stat *st, enum object_type type, const char *path, unsigned flags);
 int index_path(struct index_state *istate, struct object_id *oid, const char *path, struct stat *st, unsigned flags);
 
diff --git a/object-file.c b/object-file.c
index 5bcfde847188..74487c22e586 100644
--- a/object-file.c
+++ b/object-file.c
@@ -2067,7 +2067,7 @@ static int index_mem(struct index_state *istate,
 	/*
 	 * Convert blobs to git internal format
 	 */
-	if ((type == OBJ_BLOB) && path) {
+	if ((type == OBJ_BLOB) && path && !(flags & HASH_RAW)) {
 		struct strbuf nbuf = STRBUF_INIT;
 		if (convert_to_git(istate, path, buf, size, &nbuf,
 				   get_conv_flags(flags))) {
diff --git a/read-cache.c b/read-cache.c
index 29144cf879e7..0fb31201c705 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -716,6 +716,9 @@ int add_to_index(struct index_state *istate, const char *path, struct stat *st,
 	if (flags & ADD_CACHE_RENORMALIZE)
 		hash_flags |= HASH_RENORMALIZE;
 
+	if (flags & ADD_CACHE_RAW)
+		hash_flags |= HASH_RAW;
+
 	if (!S_ISREG(st_mode) && !S_ISLNK(st_mode) && !S_ISDIR(st_mode))
 		return error(_("%s: can only add regular files, symbolic links or git-directories"), path);
 
diff --git a/t/t2205-add-no-filters.sh b/t/t2205-add-no-filters.sh
new file mode 100755
index 000000000000..a897ed810827
--- /dev/null
+++ b/t/t2205-add-no-filters.sh
@@ -0,0 +1,46 @@
+#!/bin/sh
+
+test_description='git add --no-filters
+
+This test creates a file and a corresponding .gitattributes setup
+to ensure the file undergoes a conversion when committed or checked
+out.
+
+It then verifies that the conversion happens by default, but does not
+when --no-filters is used.'
+
+. ./test-lib.sh
+
+test_expect_success setup '
+	echo "* eol=crlf" > .gitattributes &&
+	git add .gitattributes &&
+	git commit -m initial &&
+	printf "test\r\ntest\r\n" > test
+'
+
+test_expect_success 'add without --no-filters' '
+	original="$(git hash-object --stdin < test)" &&
+	converted="$(git hash-object test)" &&
+	git add test &&
+	git ls-files -s > actual &&
+	cat > expected <<-EOF &&
+	100644 $(git hash-object .gitattributes) 0	.gitattributes
+	100644 $converted 0	test
+	EOF
+	test_cmp expected actual
+'
+
+test_expect_success 'add with --no-filters' '
+	git rm -f --cached test &&
+	original="$(git hash-object --stdin < test)" &&
+	converted="$(git hash-object test)" &&
+	git add --no-filters test &&
+	git ls-files -s > actual &&
+	cat > expected <<-EOF &&
+	100644 $(git hash-object .gitattributes) 0	.gitattributes
+	100644 $original 0	test
+	EOF
+	test_cmp expected actual
+'
+
+test_done
-- 
gitgitgadget
Andrej Shadura via GitGitGadget· Feb 20, 2021, 17:07 UTC · re: Andrej Shadura via GitGitGadget · lore

[PATCH v2 2/2] hash-object: use the new HASH_RAW flag instead of setting path to NULL

From: Andrej Shadura <andrew.shadura@collabora.co.uk>

While setting path to NULL works and flips the condition in the right branch inside index_mem(), doing so isn’t obvious for the reader of the code. Since index_mem() now has an additional flag to disable filtering, use that instead.

Signed-off-by: Andrej Shadura <andrew.shadura@collabora.co.uk>
---
 builtin/hash-object.c | 17 +++++++----------
 1 file changed, 7 insertions(+), 10 deletions(-)
Show changes to builtin/hash-object.c +7 −10
diff --git a/builtin/hash-object.c b/builtin/hash-object.c
index 640ef4ded595..6f261a1f00e3 100644
--- a/builtin/hash-object.c
+++ b/builtin/hash-object.c
@@ -59,8 +59,7 @@ static void hash_object(const char *path, const char *type, const char *vpath,
 	hash_fd(fd, type, vpath, flags, literally);
 }
 
-static void hash_stdin_paths(const char *type, int no_filters, unsigned flags,
-			     int literally)
+static void hash_stdin_paths(const char *type, unsigned flags, int literally)
 {
 	struct strbuf buf = STRBUF_INIT;
 	struct strbuf unquoted = STRBUF_INIT;
@@ -72,8 +71,7 @@ static void hash_stdin_paths(const char *type, int no_filters, unsigned flags,
 				die("line is badly quoted");
 			strbuf_swap(&buf, &unquoted);
 		}
-		hash_object(buf.buf, type, no_filters ? NULL : buf.buf, flags,
-			    literally);
+		hash_object(buf.buf, type, buf.buf, flags, literally);
 	}
 	strbuf_release(&buf);
 	strbuf_release(&unquoted);
@@ -89,7 +87,6 @@ int cmd_hash_object(int argc, const char **argv, const char *prefix)
 	const char *type = blob_type;
 	int hashstdin = 0;
 	int stdin_paths = 0;
-	int no_filters = 0;
 	int literally = 0;
 	int nongit = 0;
 	unsigned flags = HASH_FORMAT_CHECK;
@@ -100,7 +97,8 @@ int cmd_hash_object(int argc, const char **argv, const char *prefix)
 			HASH_WRITE_OBJECT),
 		OPT_COUNTUP( 0 , "stdin", &hashstdin, N_("read the object from stdin")),
 		OPT_BOOL( 0 , "stdin-paths", &stdin_paths, N_("read file names from stdin")),
-		OPT_BOOL( 0 , "no-filters", &no_filters, N_("store file as is without filters")),
+		OPT_BIT(0, "no-filters", &flags, N_("store file as is without filters"),
+			HASH_RAW),
 		OPT_BOOL( 0, "literally", &literally, N_("just hash any random garbage to create corrupt objects for debugging Git")),
 		OPT_STRING( 0 , "path", &vpath, N_("file"), N_("process file as it were from this path")),
 		OPT_END()
@@ -132,7 +130,7 @@ int cmd_hash_object(int argc, const char **argv, const char *prefix)
 	else {
 		if (hashstdin > 1)
 			errstr = "Multiple --stdin arguments are not supported";
-		if (vpath && no_filters)
+		if (vpath && (flags & HASH_RAW))
 			errstr = "Can't use --path with --no-filters";
 	}
 
@@ -150,13 +148,12 @@ int cmd_hash_object(int argc, const char **argv, const char *prefix)
 
 		if (prefix)
 			arg = to_free = prefix_filename(prefix, arg);
-		hash_object(arg, type, no_filters ? NULL : vpath ? vpath : arg,
-			    flags, literally);
+		hash_object(arg, type, vpath ? vpath : arg, flags, literally);
 		free(to_free);
 	}
 
 	if (stdin_paths)
-		hash_stdin_paths(type, no_filters, flags, literally);
+		hash_stdin_paths(type, flags, literally);
 
 	return 0;
 }
-- 
gitgitgadget
Junio C Hamano· Feb 20, 2021, 21:34 UTC · re: Andrej Shadura via GitGitGadget · lore

Re: [PATCH v2 0/2] Add --no-filters option to git-add

"Andrej Shadura via GitGitGadget" <gitgitgadget@gmail.com> writes:
Show 13 quoted lines
> It is possible for a user to disable attribute-based filtering when
> committing by doing one of the following:
>
>  * Create .git/info/attributes unapplying all possible transforming
>    attributes.
>  * Use git hash-object and git update-index to stage files manually.
>
> Doing the former requires keeping an up-to-date list of all attributes which
> can transform files when committing or checking out. Doing the latter is
> difficult, error-prone and slow when done from scripts.
>
> Instead, similarly to git hash-object, --no-filter can be added to git add
> to enable temporarily disabling filtering in an easy to use way.

I think brian's review covered if such a feature is desirable to sufficient level, and I do not have anything to add in that area, so I'll limit my comment to the general design and implementation.

In general, think three times before introducing --no-something option. It often is much cleaner and futureproof if you instead introduced --something option whose value defaults to true instead, so that the end-user can say --no-something from the command line.

>       -		OPT_BOOL( 0 , "no-filters", &no_filters, N_("store file as is without filters")),
>      -+		OPT_BIT(0 , "no-filters", &flags, N_("store file as is without filters"),
>      ++		OPT_BIT(0, "no-filters", &flags, N_("store file as is without filters"),
>       +			HASH_RAW),

In other words, these should give "filters" option, and the code should initialize the flags word with USE_CLEAN_FILTER bit on by default (the use of "clean" here comes from "clean vs smudge", one of the pair of filters end-user can customize the path the data takes going into Git from the outside world; and the "clean" and "smudge" datapaths also trigger non-custom standard ones like crlf munging).

That way when a configuration variable support is introduced to allow the users to say "I by default refuse to use the clean filters when running 'git add'" by setting say "[add] cleanfilter = false", the user can override that with "--filters" from the command line "for just this time". The same goes for an alias that hardcodes "--no-filters" on the command line, where allowing "--filters" lets the users override it.

Thanks.

← back to recent threads