[PATCH 3/3] git-add --intent-to-add (-N)

Subsystems: the rest

DORMANTno replies

6 messages, 4 authors, 2016-06-15 · open the first message on its own page

[PATCH 3/3] git-add --intent-to-add (-N)

From: Junio C Hamano <hidden>
Date: 2016-06-15 22:45:12

This adds "--intent-to-add" option to "git add".  This is to let the
system know that you will tell it the final contents to be staged later,
iow, just be aware of the presense of the path with the type of the blob
for now.

With this sequence:

    $ git reset --hard
    $ edit newfile
    $ git add -N newfile
    $ edit newfile oldfile
    $ git diff

the diff will show all changes relative to the current commit.  Then you
can do:

    $ git commit -a ;# commit everything

or

    $ git commit oldfile ;# only oldfile, newfile not yet added

to pretend you are working with an index-free system like CVS.

Signed-off-by: Junio C Hamano <redacted>
---
 builtin-add.c         |    4 +++-
 cache.h               |    2 ++
 read-cache.c          |   30 ++++++++++++++++++++----------
 t/t2203-add-intent.sh |   36 ++++++++++++++++++++++++++++++++++++
 4 files changed, 61 insertions(+), 11 deletions(-)
 create mode 100755 t/t2203-add-intent.sh
diff --git a/builtin-add.c b/builtin-add.c
index fc3f96e..a08d50d 100644
--- a/builtin-add.c
+++ b/builtin-add.c
@@ -191,7 +191,7 @@ static const char ignore_error[] =
 "The following paths are ignored by one of your .gitignore files:\n";
 
 static int verbose = 0, show_only = 0, ignored_too = 0, refresh_only = 0;
-static int ignore_add_errors, addremove;
+static int ignore_add_errors, addremove, intent_to_add;
 
 static struct option builtin_add_options[] = {
 	OPT__DRY_RUN(&show_only),
@@ -201,6 +201,7 @@ static struct option builtin_add_options[] = {
 	OPT_BOOLEAN('p', "patch", &patch_interactive, "interactive patching"),
 	OPT_BOOLEAN('f', "force", &ignored_too, "allow adding otherwise ignored files"),
 	OPT_BOOLEAN('u', "update", &take_worktree_changes, "update tracked files"),
+	OPT_BOOLEAN('N', "intent-to-add", &intent_to_add, "record only the fact that the path will be added later"),
 	OPT_BOOLEAN('A', "all", &addremove, "add all, noticing removal of tracked files"),
 	OPT_BOOLEAN( 0 , "refresh", &refresh_only, "don't add, only refresh the index"),
 	OPT_BOOLEAN( 0 , "ignore-errors", &ignore_add_errors, "just skip files which cannot be added because of errors"),
@@ -271,6 +272,7 @@ int cmd_add(int argc, const char **argv, const char *prefix)
 
 	flags = ((verbose ? ADD_CACHE_VERBOSE : 0) |
 		 (show_only ? ADD_CACHE_PRETEND : 0) |
+		 (intent_to_add ? ADD_CACHE_INTENT : 0) |
 		 (ignore_add_errors ? ADD_CACHE_IGNORE_ERRORS : 0));
 
 	if (require_pathspec && argc == 0) {
diff --git a/cache.h b/cache.h
index 68ce6e6..5948bcc 100644
--- a/cache.h
+++ b/cache.h
@@ -369,6 +369,7 @@ extern int index_name_pos(const struct index_state *, const char *name, int name
 #define ADD_CACHE_OK_TO_REPLACE 2	/* Ok to replace file/directory */
 #define ADD_CACHE_SKIP_DFCHECK 4	/* Ok to skip DF conflict checks */
 #define ADD_CACHE_JUST_APPEND 8		/* Append only; tree.c::read_tree() */
+#define ADD_CACHE_NEW_ONLY 16		/* Do not replace existing ones */
 extern int add_index_entry(struct index_state *, struct cache_entry *ce, int option);
 extern struct cache_entry *refresh_cache_entry(struct cache_entry *ce, int really);
 extern void rename_index_entry_at(struct index_state *, int pos, const char *new_name);
@@ -377,6 +378,7 @@ extern int remove_file_from_index(struct index_state *, const char *path);
 #define ADD_CACHE_VERBOSE 1
 #define ADD_CACHE_PRETEND 2
 #define ADD_CACHE_IGNORE_ERRORS	4
+#define ADD_CACHE_INTENT 8
 extern int add_to_index(struct index_state *, const char *path, struct stat *, int flags);
 extern int add_file_to_index(struct index_state *, const char *path, int flags);
 extern struct cache_entry *make_cache_entry(unsigned int mode, const unsigned char *sha1, const char *path, int stage, int refresh);
diff --git a/read-cache.c b/read-cache.c
index 2c03ec3..1592045 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -154,13 +154,13 @@ static int ce_modified_check_fs(struct cache_entry *ce, struct stat *st)
 	return 0;
 }
 
+static const unsigned char empty_blob_sha1[20] = {
+	0xe6,0x9d,0xe2,0x9b,0xb2,0xd1,0xd6,0x43,0x4b,0x8b,
+	0x29,0xae,0x77,0x5a,0xd8,0xc2,0xe4,0x8c,0x53,0x91
+};
+
 static int is_empty_blob_sha1(const unsigned char *sha1)
 {
-	static const unsigned char empty_blob_sha1[20] = {
-		0xe6,0x9d,0xe2,0x9b,0xb2,0xd1,0xd6,0x43,0x4b,0x8b,
-		0x29,0xae,0x77,0x5a,0xd8,0xc2,0xe4,0x8c,0x53,0x91
-	};
-
 	return !hashcmp(sha1, empty_blob_sha1);
 }
 
@@ -514,6 +514,9 @@ int add_to_index(struct index_state *istate, const char *path, struct stat *st,
 	unsigned ce_option = CE_MATCH_IGNORE_VALID|CE_MATCH_RACY_IS_DIRTY;
 	int verbose = flags & (ADD_CACHE_VERBOSE | ADD_CACHE_PRETEND);
 	int pretend = flags & ADD_CACHE_PRETEND;
+	int intent_only = flags & ADD_CACHE_INTENT;
+	int add_option = (ADD_CACHE_OK_TO_ADD|ADD_CACHE_OK_TO_REPLACE|
+			  (intent_only ? ADD_CACHE_NEW_ONLY : 0));
 
 	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);
@@ -527,7 +530,8 @@ int add_to_index(struct index_state *istate, const char *path, struct stat *st,
 	ce = xcalloc(1, size);
 	memcpy(ce->name, path, namelen);
 	ce->ce_flags = namelen;
-	fill_stat_cache_info(ce, st);
+	if (!intent_only)
+		fill_stat_cache_info(ce, st);
 
 	if (trust_executable_bit && has_symlinks)
 		ce->ce_mode = create_ce_mode(st_mode);
@@ -550,8 +554,12 @@ int add_to_index(struct index_state *istate, const char *path, struct stat *st,
 		alias->ce_flags |= CE_ADDED;
 		return 0;
 	}
-	if (index_path(ce->sha1, path, st, 1))
-		return error("unable to index file %s", path);
+	if (!intent_only) {
+		if (index_path(ce->sha1, path, st, 1))
+			return error("unable to index file %s", path);
+	} else
+		hashcpy(ce->sha1, empty_blob_sha1);
+
 	if (ignore_case && alias && different_name(ce, alias))
 		ce = create_alias_ce(ce, alias);
 	ce->ce_flags |= CE_ADDED;
@@ -564,7 +572,7 @@ int add_to_index(struct index_state *istate, const char *path, struct stat *st,
 
 	if (pretend)
 		;
-	else if (add_index_entry(istate, ce, ADD_CACHE_OK_TO_ADD|ADD_CACHE_OK_TO_REPLACE))
+	else if (add_index_entry(istate, ce, add_option))
 		return error("unable to add %s to index",path);
 	if (verbose && !was_same)
 		printf("add '%s'\n", path);
@@ -843,13 +851,15 @@ static int add_index_entry_with_check(struct index_state *istate, struct cache_e
 	int ok_to_add = option & ADD_CACHE_OK_TO_ADD;
 	int ok_to_replace = option & ADD_CACHE_OK_TO_REPLACE;
 	int skip_df_check = option & ADD_CACHE_SKIP_DFCHECK;
+	int new_only = option & ADD_CACHE_NEW_ONLY;
 
 	cache_tree_invalidate_path(istate->cache_tree, ce->name);
 	pos = index_name_pos(istate, ce->name, ce->ce_flags);
 
 	/* existing match? Just replace it. */
 	if (pos >= 0) {
-		replace_index_entry(istate, pos, ce);
+		if (!new_only)
+			replace_index_entry(istate, pos, ce);
 		return 0;
 	}
 	pos = -pos-1;
diff --git a/t/t2203-add-intent.sh b/t/t2203-add-intent.sh
new file mode 100755
index 0000000..d4de35e
--- /dev/null
+++ b/t/t2203-add-intent.sh
@@ -0,0 +1,36 @@
+#!/bin/sh
+
+test_description='Intent to add'
+
+. ./test-lib.sh
+
+test_expect_success 'intent to add' '
+	echo hello >file &&
+	echo hello >elif &&
+	git add -N file &&
+	git add elif
+'
+
+test_expect_success 'check result of "add -N"' '
+	git ls-files -s file >actual &&
+	empty=$(git hash-object --stdin </dev/null) &&
+	echo "100644 $empty 0	file" >expect &&
+	test_cmp expect actual
+'
+
+test_expect_success 'intent to add is just an ordinary empty blob' '
+	git add -u &&
+	git ls-files -s file >actual &&
+	git ls-files -s elif | sed -e "s/elif/file/" >expect &&
+	test_cmp expect actual
+'
+
+test_expect_success 'intent to add does not clobber existing paths' '
+	git add -N file elif &&
+	empty=$(git hash-object --stdin </dev/null) &&
+	git ls-files -s >actual &&
+	! grep "$empty" actual
+'
+
+test_done
+
-- 
1.6.0.51.g078ae

Re: [PATCH 3/3] git-add --intent-to-add (-N)

From: Paolo Bonzini <hidden>
Date: 2016-06-15 22:45:12

Junio C Hamano wrote:
This adds "--intent-to-add" option to "git add".  This is to let the
system know that you will tell it the final contents to be staged later,
iow, just be aware of the presense of the path with the type of the blob
for now.
While I like intent_* in the variables, what about "git add --path FILE" 
for the user interface?  Also, I wonder if it would be good to restrict 
"git add --path" to paths not already in the index, and give an error 
otherwise.

As I said elsewhere in the thread, I wouldn't use this feature (I keep a 
"git citool" window open to review my own changes, when I have to deal 
with new files), but I applaud its introduction.
Then you can do:

    $ git commit -a ;# commit everything

or

    $ git commit oldfile ;# only oldfile, newfile not yet added
Did you mean "git commit" for the second use case?

Paolo

Re: [PATCH 3/3] git-add --intent-to-add (-N)

From: Jonathan Nieder <hidden>
Date: 2016-06-15 22:45:12

Hi,

Junio C Hamano wrote:
This adds "--intent-to-add" option to "git add".
I quite like the idea of this patch series.  When I try to test it with
"git merge jc/ita; make test", t0020-crlf setup fails with

	error: invalid object e69de29bb2d1d6434b8b29ae775ad8c2e48c5391
	error: Error building trees
	* FAIL 1: setup

This could be me doing something wrong, but I thought you'd like to
know, anyway.  I'll try to diagnose it tonight.

Regards,
Jonathan

Re: [PATCH 3/3] git-add --intent-to-add (-N)

From: Jonathan Nieder <hidden>
Date: 2016-06-15 22:45:12

Hi,

Jonathan Nieder wrote:
I quite like the idea of this patch series.  When I try to test it with
"git merge jc/ita; make test", t0020-crlf setup fails
[...]
This could be me doing something [stupid]
and it was.  In a sleepy daze, I resolved a conflict

<<<<<<<
#define ADD_CACHE_IGNORE_REMOVAL 8
=======
#define ADD_CACHE_INTENT 8
quoted
quoted
quoted
quoted
quoted
quoted
by using the same bit for both.  Sorry for the noise.

Others can experience that unpleasant error message for themselves
with next + jc/add-ita merged properly:

	$ mkdir test-repo && cd test-repo
	$ git init
	Initialized empty Git repository in /var/tmp/jrnieder/test-repo/.git/
	$ : >a
	$ git add -N a
	$ git commit
	error: invalid object e69de29bb2d1d6434b8b29ae775ad8c2e48c5391
	error: Error building trees

I think the first error comes from update_one, which creates a tree
object from the index.  It is complaining, because after all, that
object is not in any sha1 file.

If the empty blob happened to be in our object database, the user's
mistake would be hidden:

	$ git add a && git commit
	error: invalid object e69de29bb2d1d6434b8b29ae775ad8c2e48c5391
	error: Error building trees
	$ git rm -f --cached a
	rm 'a'
	$ git add a
	$ git commit -m initial
	$ echo hi >b
	$ git add -N b
	$ git commit && echo ok
	Created commit 91325db: some commit message
	 0 files changed, 0 insertions(+), 0 deletions(-)
	 create mode 100644 b
	ok

Maybe it would be better to use some other magic blob (or a bit
somewhere) to remember that the file has not been added yet.

Thoughts?

Regards,
Jonathan

Re: [PATCH 3/3] git-add --intent-to-add (-N)

From: Daniel Barkalow <hidden>
Date: 2016-06-15 22:45:12

On Thu, 21 Aug 2008, Jonathan Nieder wrote:
Hi,

Jonathan Nieder wrote:
quoted
I quite like the idea of this patch series.  When I try to test it with
"git merge jc/ita; make test", t0020-crlf setup fails
[...]
quoted
This could be me doing something [stupid]
and it was.  In a sleepy daze, I resolved a conflict

<<<<<<<
#define ADD_CACHE_IGNORE_REMOVAL 8
=======
#define ADD_CACHE_INTENT 8
quoted
quoted
quoted
quoted
quoted
quoted
quoted
by using the same bit for both.  Sorry for the noise.

Others can experience that unpleasant error message for themselves
with next + jc/add-ita merged properly:

	$ mkdir test-repo && cd test-repo
	$ git init
	Initialized empty Git repository in /var/tmp/jrnieder/test-repo/.git/
	$ : >a
	$ git add -N a
	$ git commit
	error: invalid object e69de29bb2d1d6434b8b29ae775ad8c2e48c5391
	error: Error building trees

I think the first error comes from update_one, which creates a tree
object from the index.  It is complaining, because after all, that
object is not in any sha1 file.
I think [1/3] was supposed to make this not an issue, with that particular 
object being implicitly in all objects databases.
If the empty blob happened to be in our object database, the user's
mistake would be hidden:

	$ git add a && git commit
	error: invalid object e69de29bb2d1d6434b8b29ae775ad8c2e48c5391
	error: Error building trees
	$ git rm -f --cached a
	rm 'a'
	$ git add a
	$ git commit -m initial
	$ echo hi >b
	$ git add -N b
	$ git commit && echo ok
	Created commit 91325db: some commit message
	 0 files changed, 0 insertions(+), 0 deletions(-)
	 create mode 100644 b
	ok

Maybe it would be better to use some other magic blob (or a bit
somewhere) to remember that the file has not been added yet.
An actual magic value (maybe the all-zeros hash) would make it an actual 
error for the file to not have been added; the current code behaves as if 
you did:

$ touch b
$ git add b

right before putting anything in b. Aside, perhaps, from retrieval bugs, 
it's just like you actually added an empty blob.

Last time I tried something along these lines, using the all-zeros hash 
actually came pretty close to working, except that diff uses this value 
for "look at the working tree" in its representation, and stuff gets 
confused by it; these are actually distinguishable, IIRC, by whether the 
mode bits are set or not, but current code doesn't check that.

	-Daniel
*This .sig left intentionally blank*

Re: [PATCH 3/3] git-add --intent-to-add (-N)

From: Jonathan Nieder <hidden>
Date: 2016-06-15 22:45:12

Daniel Barkalow wrote:
On Thu, 21 Aug 2008, Jonathan Nieder wrote:
[...]
quoted
	$ git add -N a
	$ git commit
	error: invalid object e69de29bb2d1d6434b8b29ae775ad8c2e48c5391
	error: Error building trees

I think the first error comes from update_one, which creates a tree
object from the index.  It is complaining, because after all, that
object is not in any sha1 file.
I think [1/3] was supposed to make this not an issue, with that particular 
object being implicitly in all objects databases.
Wait, is [1/3] meant to create that strong of an illusion?  That is,
should has_sha1_file pretend the object is present, too?

Jonathan
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help