Skip to contentSkip to conversation

Add --no-filters option to git-add

PATCH Superseded Andrej Shadura via GitGitGadget 4 participants 11 messages Feb 19, 2021 – Feb 20, 2021 Documentation

lore
Share & tools
lore
#0 lore raw

cover 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

Signature · 1 line
gitgitgadget
#1 lore raw

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

From: Andrej Shadura <[email protected]>

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

---
 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
Documentation/git-add.txt +6 -1
index be5e3ac54b85..b3d7b13e83f4 100644
@@ -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
builtin/add.c +3 -0
index a825887c503d..609a0e6c0157 100644
@@ -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));
 
cache.h +2 -0
index d92814961405..df83b5709c32 100644
@@ -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);
 
object-file.c +1 -1
index 5bcfde847188..74487c22e586 100644
@@ -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))) {
read-cache.c +3 -0
index 29144cf879e7..0fb31201c705 100644
@@ -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);
 
t/t2205-add-no-filters.sh new file +46 -0

46 lines not on this page · raw message

Signature · 1 line
gitgitgadget
#2 lore raw

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

In reply to Andrej Shadura via GitGitGadget

From: Andrej Shadura <[email protected]>

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.

---
 builtin/hash-object.c | 17 +++++++----------
 1 file changed, 7 insertions(+), 10 deletions(-)
builtin/hash-object.c +7 -10
index 640ef4ded595..8e0543b12bc5 100644
@@ -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;
 }
Signature · 1 line
gitgitgadget
#3 lore raw
In reply to Andrej Shadura via GitGitGadget
Quoted 14 lines · 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.

Quoted 6 lines

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.

Signature · 2 lines
brian m. carlson (he/him or they/them)
Houston, Texas, US
#4 lore raw
Quoted 27 lines · brian m. carlson
Quoted 7 lines · 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.

Quoted 6 lines

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.

Quoted 4 lines

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.

Signature · 2 lines
Cheers,
  Andrej
#5 lore raw
Quoted 5 lines · Andrej Shadura
Quoted 4 lines · brian m. carlson

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

Signature · 2 lines
Cheers,
  Andrej
#6 lore raw
Quoted 37 lines · Andrej Shadura
Quoted 5 lines · Andrej Shadura
Quoted 4 lines · brian m. carlson

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.
Quoted 1 line

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

Signature · 2 lines
brian m. carlson (he/him or they/them)
Houston, Texas, US
#7 lore raw

v2 cover Add --no-filters option to git-add

In reply to 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")),

Signature · 1 line
gitgitgadget
#8 lore raw

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

From: Andrej Shadura <[email protected]>

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

---
 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
Documentation/git-add.txt +9 -1
index be5e3ac54b85..75b474a88fca 100644
@@ -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
Documentation/gitfaq.txt +7 -0
index afdaeab8503c..6011abca0bea 100644
@@ -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?::
builtin/add.c +3 -0
index a825887c503d..609a0e6c0157 100644
@@ -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));
 
cache.h +2 -0
index d92814961405..df83b5709c32 100644
@@ -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);
 
object-file.c +1 -1
index 5bcfde847188..74487c22e586 100644
@@ -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))) {
read-cache.c +3 -0
index 29144cf879e7..0fb31201c705 100644
@@ -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);
 
t/t2205-add-no-filters.sh new file +46 -0

46 lines not on this page · raw message

Signature · 1 line
gitgitgadget
#9 lore raw

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

In reply to Andrej Shadura via GitGitGadget

From: Andrej Shadura <[email protected]>

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.

---
 builtin/hash-object.c | 17 +++++++----------
 1 file changed, 7 insertions(+), 10 deletions(-)
builtin/hash-object.c +7 -10
index 640ef4ded595..6f261a1f00e3 100644
@@ -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;
 }
Signature · 1 line
gitgitgadget
#10 lore raw
In reply to Andrej Shadura via GitGitGadget
Quoted 7 lines · 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.

Quoted 6 lines

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.

Quoted 4 lines
      -		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.