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

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

[Andrej Shadura via GitGitGadget]

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

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

[Andrej Shadura via GitGitGadget]

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

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

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

[Andrej Shadura via GitGitGadget]

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(-)

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

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

[brian m. carlson]

[-- Attachment #1: Type: text/plain, Size: 1939 bytes --]

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.
> 
> 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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 263 bytes --]

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

[Andrej Shadura]

On 19/02/2021 23:36, brian m. carlson wrote:
> 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.

> 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

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

[Andrej Shadura]

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.)

-- 
Cheers,
  Andrej

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

[brian m. carlson]

[-- Attachment #1: Type: text/plain, Size: 2074 bytes --]

On 2021-02-20 at 09:30:58, Andrej Shadura wrote:
> 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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 263 bytes --]

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

[Andrej Shadura via GitGitGadget]

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

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

[Andrej Shadura via GitGitGadget]

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

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

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

[Andrej Shadura via GitGitGadget]

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(-)

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

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

[Junio C Hamano]

"Andrej Shadura via GitGitGadget" <gitgitgadget@gmail.com> writes:

> 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.