Git development
 help / color / mirror / Atom feed
* Re: [PATCH] t2203: fix wrong commit command
From: Nguyen Thai Ngoc Duy @ 2012-01-11  4:43 UTC (permalink / raw)
  To: Ramkumar Ramachandra; +Cc: git, Junio C Hamano
In-Reply-To: <CALkWK0kDBFvssyX2ZPJ9WNzfNXD=wEJoXTRVpFWm1TDKJrvNzA@mail.gmail.com>

2012/1/11 Ramkumar Ramachandra <artagnon@gmail.com>:
> Hi Duy,
>
> Nguyễn Thái Ngọc Duy wrote:
>> Add commit message to avoid commit's aborting due to the lack of
>> commit message, not because there are INTENT_TO_ADD entries in index.
>
> Is there any way to differentiate between the two kinds of errors, so
> that we can avoid this in the future?  Can we grep the error message
> for something, or inspect the exit status?

commit exits with 1 in both cases, so no.
-- 
Duy

^ permalink raw reply

* Re: [PATCH] t2203: fix wrong commit command
From: Nguyen Thai Ngoc Duy @ 2012-01-11  4:43 UTC (permalink / raw)
  To: Ramkumar Ramachandra; +Cc: git, Junio C Hamano
In-Reply-To: <CACsJy8CjWBRtj3x3guyu-iJeTYxcRriXKbE=Be-rG=Rmesgt=g@mail.gmail.com>

On Wed, Jan 11, 2012 at 11:43 AM, Nguyen Thai Ngoc Duy
<pclouds@gmail.com> wrote:
> 2012/1/11 Ramkumar Ramachandra <artagnon@gmail.com>:
>> Hi Duy,
>>
>> Nguyễn Thái Ngọc Duy wrote:
>>> Add commit message to avoid commit's aborting due to the lack of
>>> commit message, not because there are INTENT_TO_ADD entries in index.
>>
>> Is there any way to differentiate between the two kinds of errors, so
>> that we can avoid this in the future?  Can we grep the error message
>> for something, or inspect the exit status?
>
> commit exits with 1 in both cases, so no.

but grepping error message should work, sorry I hit send too fast
-- 
Duy

^ permalink raw reply

* Re: [PATCH] Fix incorrect ref namespace check
From: Michael Haggerty @ 2012-01-11  4:49 UTC (permalink / raw)
  To: Nguyen Thai Ngoc Duy; +Cc: git, Junio C Hamano
In-Reply-To: <CACsJy8Af=PjXE4sHqSk1v8qoraivOnT2w495W1c1dVQBW1s-cQ@mail.gmail.com>

On 01/11/2012 02:54 AM, Nguyen Thai Ngoc Duy wrote:
> Any comments Mike?

Looks fine to me.

Theoretically, we could add an API for iterating over references
*excluding* a particular namespace, which (after hierarchical refs is
accepted) could be implemented a bit more efficiently than iterating
over all references and then excluding those in the unwanted namespace.
 But I think it would be far more trouble than it's worth.

Michael

> 2012/1/5 Junio C Hamano <gitster@pobox.com>:
>> Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:
>>
>>> The reason why the trailing slash is needed is obvious. refs/stash and
>>> HEAD are not namespace, but complete refs. Do full string compare on them.
>>>
>>> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
>>> ---
>>>  I missed prefixcmp(..., "HEAD") right below prefixcmp(..., "refs/stash")
>>
>> As Michael has been actively showing interest in cleaning up the area, he
>> should have been CC'ed, I would think.
>>
>>>
>>>  builtin/fetch.c  |    2 +-
>>>  builtin/remote.c |    2 +-
>>>  log-tree.c       |    4 ++--
>>>  3 files changed, 4 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/builtin/fetch.c b/builtin/fetch.c
>>> index 33ad3aa..daa68d2 100644
>>> --- a/builtin/fetch.c
>>> +++ b/builtin/fetch.c
>>> @@ -573,7 +573,7 @@ static void find_non_local_tags(struct transport *transport,
>>>
>>>       for_each_ref(add_existing, &existing_refs);
>>>       for (ref = transport_get_remote_refs(transport); ref; ref = ref->next) {
>>> -             if (prefixcmp(ref->name, "refs/tags"))
>>> +             if (prefixcmp(ref->name, "refs/tags/"))
>>>                       continue;
>>>
>>>               /*
>>> diff --git a/builtin/remote.c b/builtin/remote.c
>>> index 583eec9..f54a89a 100644
>>> --- a/builtin/remote.c
>>> +++ b/builtin/remote.c
>>> @@ -534,7 +534,7 @@ static int add_branch_for_removal(const char *refname,
>>>       }
>>>
>>>       /* don't delete non-remote-tracking refs */
>>> -     if (prefixcmp(refname, "refs/remotes")) {
>>> +     if (prefixcmp(refname, "refs/remotes/")) {
>>>               /* advise user how to delete local branches */
>>>               if (!prefixcmp(refname, "refs/heads/"))
>>>                       string_list_append(branches->skipped,
>>> diff --git a/log-tree.c b/log-tree.c
>>> index 319bd31..535b905 100644
>>> --- a/log-tree.c
>>> +++ b/log-tree.c
>>> @@ -119,9 +119,9 @@ static int add_ref_decoration(const char *refname, const unsigned char *sha1, in
>>>               type = DECORATION_REF_REMOTE;
>>>       else if (!prefixcmp(refname, "refs/tags/"))
>>>               type = DECORATION_REF_TAG;
>>> -     else if (!prefixcmp(refname, "refs/stash"))
>>> +     else if (!strcmp(refname, "refs/stash"))
>>>               type = DECORATION_REF_STASH;
>>> -     else if (!prefixcmp(refname, "HEAD"))
>>> +     else if (!strcmp(refname, "HEAD"))
>>>               type = DECORATION_REF_HEAD;
>>>
>>>       if (!cb_data || *(int *)cb_data == DECORATE_SHORT_REFS)
> 
> 
> 


-- 
Michael Haggerty
mhagger@alum.mit.edu
http://softwareswirl.blogspot.com/

^ permalink raw reply

* Re: [PATCH 2/8] revert: decouple sequencer actions from builtin commands
From: Jonathan Nieder @ 2012-01-11  5:04 UTC (permalink / raw)
  To: Ramkumar Ramachandra; +Cc: Git List, Junio C Hamano
In-Reply-To: <CALkWK0=bEPPv4rtPrMrQnk3MK=JY4-wwAByWPmzg86NBm_56iQ@mail.gmail.com>

Ramkumar Ramachandra wrote:

> I wrote that too quickly.  I can't stand seeing so many strcmp() calls
> all over my codebase -- look at the number of instances of matching
> opts->command to REPLAY_CMD_*.

If the number of such in sequencer.c is greater than 0, it's probably
a bug.  Why would the sequencer change behavior based on its caller's
name?

It's very easy for me to be wrong, though --- this is just a first
reaction, and I haven't looked at this version of sequencer.c yet,
since it doesn't exist. :)

Is this what you're talking about?

	if (opts->action == CHERRY_PICK)
		error(_("Your local changes would be overwritten by cherry-pick."));
	else
		error(_("Your local changes would be overwritten by revert."));

Now imagine "git rebase" wants to hook into the same stuff. What
should the message be?

Maybe:

	error(_("your local changes would be overwritten - aborting"));

Or:

	error(_(opts->error_msgs[REPLAY_WOULD_CLOBBER_LOCAL_CHANGES]));

^ permalink raw reply

* Re: [PATCH 2/8] revert: decouple sequencer actions from builtin commands
From: Ramkumar Ramachandra @ 2012-01-11  5:14 UTC (permalink / raw)
  To: Jonathan Nieder; +Cc: Git List, Junio C Hamano
In-Reply-To: <20120111050404.GA13507@burratino>

Jonathan Nieder wrote:
> Ramkumar Ramachandra wrote:
>
>> I wrote that too quickly.  I can't stand seeing so many strcmp() calls
>> all over my codebase -- look at the number of instances of matching
>> opts->command to REPLAY_CMD_*.
>
> If the number of such in sequencer.c is greater than 0, it's probably
> a bug.  Why would the sequencer change behavior based on its caller's
> name?

In the super-final version, yes-  I agree.  However, we're not there
yet -- this patch is more of a transition, to make the life of
"revert: allow mixing "pick" and "revert" actions" easier.  Once the
painful move to sequencer.c is completed, we can think about all these
things.  Right now, I'm only focusing on the move, and everything that
should logically precede it (in my opinion).

> Is this what you're talking about?
>
>        if (opts->action == CHERRY_PICK)
>                error(_("Your local changes would be overwritten by cherry-pick."));
>        else
>                error(_("Your local changes would be overwritten by revert."));

1. Yes, error messages like this.
2. Since I haven't modified function prototypes to include a
"replay_action" at the "revert: decouple sequencer actions from
builtin commands" stage, functions like do_pick_commit() have to rely
on opts->command!  That's the entire point: I want to show how this
"opts->command" is changing to "action" in most places in "revert:
allow mixing "pick" and "revert" actions".

I'd rationalize that taking care of (1) is not urgent- we can do it
after we move to sequencer.c.  As for (2), we have little choice at
this point- either make it a string like it's supposed to be in the
super-final version, and struggle with the ugliness of strcmp() now,
or make it an enum now; I choose the latter.

Thanks for making me explain this.

-- Ram

^ permalink raw reply

* Re: [PATCH 2/8] revert: decouple sequencer actions from builtin commands
From: Ramkumar Ramachandra @ 2012-01-11  5:26 UTC (permalink / raw)
  To: Jonathan Nieder; +Cc: Git List, Junio C Hamano
In-Reply-To: <CALkWK0kJpEXvBMV=D7h91sz7U2sLvXdW1UzomW0kG2bbM+byYA@mail.gmail.com>

Hi Jonathan,

I wrote a new commit message for this patch.  Perhaps it can help clarify?

  revert: decouple sequencer actions from commands

  Currently, we have two actions "pick" and "revert" that directly
  correspond to the action of the commands 'git cherry-pick' and 'git
  revert' respectively.  Since we would like to permit mixing different
  actions in the same instruction sheet inspired by the way 'git rebase
  --interactive' works, a future instruction sheet would look like:

    pickle next~4
    revert rr/moo^2~34
    action3 b74fea

  where the actions "pickle", "revert" and "action3" need not directly
  correspond to the specific commands.  So, instead of using the same
  representation for both the command executed and the sequencer action
  (the "replay_action" enum), store the command separately as a
  "replay_command" enum.

  Since we don't allow mixing actions yet, the entire operation of 'git
  cherry-pick'/ 'git revert' relies on the command, with the
  exception of the instruction sheet parser which converts a
  "replay_action" to a "replay_command".  As we support mixing actions
  in future patches, more portions of the code will rely on the
  action, demoting the command to a string used in error reporting.

-- Ram

^ permalink raw reply

* Re: [PATCH 2/8] revert: decouple sequencer actions from builtin commands
From: Jonathan Nieder @ 2012-01-11  5:49 UTC (permalink / raw)
  To: Ramkumar Ramachandra; +Cc: Git List, Junio C Hamano
In-Reply-To: <CALkWK0muXXKu37_qQ8E+LEZiCxebWvWghkc8QjyfdBazjLOstw@mail.gmail.com>

Ramkumar Ramachandra wrote:

> Hi Jonathan,
>
> I wrote a new commit message for this patch.  Perhaps it can help clarify?

Thanks.

>   revert: decouple sequencer actions from commands
>
>   Currently, we have two actions "pick" and "revert" that directly
>   correspond to the action of the commands 'git cherry-pick' and 'git
>   revert' respectively.

Well, let's start here.  The two insns "pick" and "revert" and the
ability to mix them doesn't have much to do with the picture, does it?

I think the actual problem being solved is that insn types, as described
by the replay_action enum, are being abused to refer to top-level git
commands "revert" and "cherry-pick".  The sequencer isn't supposed to
care which top-level git command called it, except in some messages, so
we'd certainly like to stop pretending that has something to do with
insn types.

Based on what you've said, correcting this cleanly is complicated in
some places by the inconvenient fact that the sequencer _does_ care
which top-level git command called it.  (I haven't checked this; I'm
just taking it on faith from you.)  If we want to let other git
commands (like "git rebase" or "git sequence") call into the
sequencer, that sounds like a way bigger problem than any conflict of
terminology.

^ permalink raw reply

* [PATCH RFC] commit: allow to commit even if there are intent-to-add entries
From: Nguyễn Thái Ngọc Duy @ 2012-01-11  6:01 UTC (permalink / raw)
  To: git; +Cc: Nguyễn Thái Ngọc Duy

This patch replaces the approach in 331fcb5 (git add --intent-to-add:
do not let an empty blob be committed by accident) regarding i-t-a
entries: instead of forbidding i-t-a entries at commit time, we can
simply ignore them.

We already ignore CE_REMOVE entries while updating cache-tree. Putting
CE_INTENT_TO_ADD ones in the same category should not cause any negative
effects regarding cache-tree.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 On the few chances I have to use "git add -N" it does not fit well
 with "git add -p; git diff --cached; git commit -m foo" style. I
 think this may be a good thing to do.

 builtin/commit.c      |    2 +-
 builtin/write-tree.c  |    2 +-
 cache-tree.c          |   14 +++++---------
 t/t2203-add-intent.sh |   10 +++++++++-
 4 files changed, 16 insertions(+), 12 deletions(-)

diff --git a/builtin/commit.c b/builtin/commit.c
index eba1377..767b78a 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -871,7 +871,7 @@ static int prepare_to_commit(const char *index_file, const char *prefix,
 	discard_cache();
 	read_cache_from(index_file);
 	if (update_main_cache_tree(0)) {
-		error(_("Error building trees"));
+		error(_("Error building trees; the index is unmerged?"));
 		return 0;
 	}
 
diff --git a/builtin/write-tree.c b/builtin/write-tree.c
index b223af4..68baa24 100644
--- a/builtin/write-tree.c
+++ b/builtin/write-tree.c
@@ -46,7 +46,7 @@ int cmd_write_tree(int argc, const char **argv, const char *unused_prefix)
 		die("%s: error reading the index", me);
 		break;
 	case WRITE_TREE_UNMERGED_INDEX:
-		die("%s: error building trees", me);
+		die("%s: error building trees; the index is unmerged?", me);
 		break;
 	case WRITE_TREE_PREFIX_ERROR:
 		die("%s: prefix %s not found", me, prefix);
diff --git a/cache-tree.c b/cache-tree.c
index 8de3959..47defd1 100644
--- a/cache-tree.c
+++ b/cache-tree.c
@@ -158,19 +158,15 @@ static int verify_cache(struct cache_entry **cache,
 	funny = 0;
 	for (i = 0; i < entries; i++) {
 		struct cache_entry *ce = cache[i];
-		if (ce_stage(ce) || (ce->ce_flags & CE_INTENT_TO_ADD)) {
+		if (ce_stage(ce)) {
 			if (silent)
 				return -1;
 			if (10 < ++funny) {
 				fprintf(stderr, "...\n");
 				break;
 			}
-			if (ce_stage(ce))
-				fprintf(stderr, "%s: unmerged (%s)\n",
-					ce->name, sha1_to_hex(ce->sha1));
-			else
-				fprintf(stderr, "%s: not added yet\n",
-					ce->name);
+			fprintf(stderr, "%s: unmerged (%s)\n",
+				ce->name, sha1_to_hex(ce->sha1));
 		}
 	}
 	if (funny)
@@ -338,8 +334,8 @@ static int update_one(struct cache_tree *it,
 				mode, sha1_to_hex(sha1), entlen+baselen, path);
 		}
 
-		if (ce->ce_flags & CE_REMOVE)
-			continue; /* entry being removed */
+		if (ce->ce_flags & (CE_REMOVE | CE_INTENT_TO_ADD))
+			continue; /* entry being removed or just placeholder */
 
 		strbuf_grow(&buffer, entlen + 100);
 		strbuf_addf(&buffer, "%o %.*s%c", mode, entlen, path + baselen, '\0');
diff --git a/t/t2203-add-intent.sh b/t/t2203-add-intent.sh
index 2543529..65430e4 100755
--- a/t/t2203-add-intent.sh
+++ b/t/t2203-add-intent.sh
@@ -41,7 +41,15 @@ test_expect_success 'cannot commit with i-t-a entry' '
 	echo frotz >nitfol &&
 	git add rezrov &&
 	git add -N nitfol &&
-	test_must_fail git commit -m initial
+	git commit -m initial &&
+	git ls-tree -r HEAD >actual &&
+	cat >expected <<EOF &&
+100644 blob ce013625030ba8dba906f756967f9e9ca394464a	elif
+100644 blob ce013625030ba8dba906f756967f9e9ca394464a	file
+100644 blob cf7711b63209d0dbc2d030f7fe3513745a9880e4	rezrov
+EOF
+	test_cmp expected actual &&
+	git reset HEAD^
 '
 
 test_expect_success 'can commit with an unrelated i-t-a entry in index' '
-- 
1.7.3.1.256.g2539c.dirty

^ permalink raw reply related

* Re: [PATCH 3/3] diff-index: pass pathspec down to unpack-trees machinery
From: Jonathan Nieder @ 2012-01-11  6:31 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Linus Torvalds, Nguyen Thai Ngoc Duy
In-Reply-To: <1314653603-7533-4-git-send-email-gitster@pobox.com>

Hi,

Junio C Hamano wrote:

> And finally, pass the pathspec down through unpack_trees() to traverse_trees()
> callchain.
>
> Before and after applying this series, looking for changes in the kernel
> repository with a fairly narrow pathspec gets a moderate speeds up.

Check this out:

	mkdir -p test-repo/subdir
	cd test-repo
	git init
	echo hi >subdir/hello.h
	git add subdir/hello.h
	git commit -m 'say hi'
	git diff-index --abbrev HEAD -- '*.h'

In git versions which include the patch described above, the unchanged
subdir/hello.h shows up as a newly added file.  Reverting that patch
(v1.7.7.1~22^2, diff-index: pass pathspec down to unpack-trees
machinery, 2011-08-29) makes "git diff HEAD" with wildcards work
again.

It looks like the pruning of the preimage by pathspec is too
aggressive and is omiting whole directories that do not match the
pathspec without realizing that the path to a file contained within
might match.

We could just add a test for this to the testsuite and do that revert,
but I'd rather not yet, in case this is a symptom of some deeper
unpack_trees() confusion.

Hints?

Thanks,
Jonathan

>  diff-lib.c |    1 +
>  1 files changed, 1 insertions(+), 0 deletions(-)
>
> diff --git a/diff-lib.c b/diff-lib.c
> index f8454dd..ebe751e 100644
> --- a/diff-lib.c
> +++ b/diff-lib.c
> @@ -468,6 +468,7 @@ static int diff_cache(struct rev_info *revs,
>  	opts.unpack_data = revs;
>  	opts.src_index = &the_index;
>  	opts.dst_index = NULL;
> +	opts.pathspec = &revs->diffopt.pathspec;
>  
>  	init_tree_desc(&t, tree->buffer, tree->size);
>  	return unpack_trees(1, &t, &opts);
> -- 
> 1.7.7.rc0.70.g82660
>
>

^ permalink raw reply

* Re: Intriguing error with Git 1.6.3.1: Too many open files
From: Soham Mehta @ 2012-01-11  6:52 UTC (permalink / raw)
  To: Jonathan Nieder; +Cc: git
In-Reply-To: <20120111030931.GA12862@burratino>

On Tue, Jan 10, 2012 at 7:09 PM, Jonathan Nieder <jrnieder@gmail.com> wrote:
> Hi,
>
> Soham Mehta wrote:
>
>> *"error: unable to open object pack directory: ./objects/pack: Too many
>> open files"
>
> Do current git versions (v1.7.4.2 or later) behave the same way?
>
> Sorry for the trouble,
> Jonathan


I haven't played with versions yet. Are you alluding that #2 on this
list may have addressed it?
https://raw.github.com/gitster/git/master/Documentation/RelNotes/1.7.4.2.txt

 * We used to keep one file descriptor open for each and every packfile
   that we have a mmap window on it (read: "in use"), even when for very
   tiny packfiles.  We now close the file descriptor early when the entire
   packfile fits inside one mmap window.


The corrupt repo does have a 1000+ pack files, anywhere from 650M to 2.5K.

^ permalink raw reply

* Re* Regulator updates for 3.3
From: Junio C Hamano @ 2012-01-11  6:59 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: Mark Brown, Liam Girdwood, linux-kernel, Git Mailing List
In-Reply-To: <CA+55aFzuGtJkQFDooSGWQ2_NiJVHN2E7S5dmOnWTYn8_s8Gg3g@mail.gmail.com>

Linus Torvalds <torvalds@linux-foundation.org> writes:

> The (b) thing I suggested was for "work around for people who have
> legacy cases that they don't want to make explicit". I guess you could
> count that as some rule, but I really think it's more of a "ok, we had
> bad legacy behavior, and now we have scripts that depended on that bad
> legacy".

I would think that would solve the issue for scripts like the one used to
rebuild the linux-next tree.  I also have such a script to rebuild 'pu' I
use three or four times a day, but admittedly the standard input of "git
merge" in that script is connected to a here document that feeds the list
of branches to be merged to the loop that drives the "git merge", so your
heuristic (a) will kick in and I wouldn't need the GIT_MERGE_NO_EDIT
environment myself.

What makes me uneasy about the idea of running the editor by default is
that many people still use Git as a better CVS/SVN. Their workflow is to
build randomly on their 'master', attempt to push and get rejected, pull
only so that they can push out, and then push the merge result out. Such
merges are done without any consideration on the cohesiveness of the
branch (the segment of the history that records their work since they last
pulled from the central repository). Having to justify the backmerge is
nothing but a nuisance for them, as they do not have any justification
better than "I am done for the day, and my commuter shuttle will leave in
15 minutes, so I tried to push what I've done so far, but it was rejected
due to non fast-forward, and I am merging random things others did so that
I can push back". They won't be saying "This merges the great work I
completed and have been testing privately for a few days to the trunk", as
the direction of their merge is backwards.

With that caveat, the patch should look like this.

-- >8 --
Subject: [PATCH] merge: use editor by default in interactive sessions

Traditionally, a cleanly resolved merge was committed by "git merge" using
the auto-generated merge commit log message with invoking the editor.

After 5 years of use in the field, it turns out that many people perform
too many unjustified backmerges of the upstream history into their topic
branches. These merges are not just useless, but they are more often than
not explained and making the end result unreadable when it gets time for
merging their history back to their upstream.

Earlier we added the "--edit" option to the command, so that people can
edit the log message to explain and justify their merge commits. Let's
take it one step further and spawn the editor by default when we are in an
interactive session (i.e. the standard input and the standard output are
pointing at the same tty device).

There may be existing scripts that leave the standard input and the
standard output of the "git merge" connected to whatever environment the
scripts were started, and such invocation might trigger the above
"interactive session" heuristics. Such scripts can export GIT_MERGE_LEGACY
environment variable set to "yes" to force the traditional behaviour.

Suggested-by: Linus Torvalds
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 builtin/merge.c |   34 ++++++++++++++++++++++++++++++----
 t/test-lib.sh   |    3 ++-
 2 files changed, 32 insertions(+), 5 deletions(-)

diff --git a/builtin/merge.c b/builtin/merge.c
index 99f1429..6a80e1e 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -46,7 +46,7 @@ static const char * const builtin_merge_usage[] = {
 
 static int show_diffstat = 1, shortlog_len, squash;
 static int option_commit = 1, allow_fast_forward = 1;
-static int fast_forward_only, option_edit;
+static int fast_forward_only, option_edit = -1;
 static int allow_trivial = 1, have_message;
 static struct strbuf merge_msg;
 static struct commit_list *remoteheads;
@@ -189,7 +189,7 @@ static struct option builtin_merge_options[] = {
 		"create a single commit instead of doing a merge"),
 	OPT_BOOLEAN(0, "commit", &option_commit,
 		"perform a commit if the merge succeeds (default)"),
-	OPT_BOOLEAN('e', "edit", &option_edit,
+	OPT_BOOL('e', "edit", &option_edit,
 		"edit message before committing"),
 	OPT_BOOLEAN(0, "ff", &allow_fast_forward,
 		"allow fast-forward (default)"),
@@ -877,12 +877,12 @@ static void prepare_to_commit(void)
 	write_merge_msg(&msg);
 	run_hook(get_index_file(), "prepare-commit-msg",
 		 git_path("MERGE_MSG"), "merge", NULL, NULL);
-	if (option_edit) {
+	if (0 < option_edit) {
 		if (launch_editor(git_path("MERGE_MSG"), NULL, NULL))
 			abort_commit(NULL);
 	}
 	read_merge_msg(&msg);
-	stripspace(&msg, option_edit);
+	stripspace(&msg, 0 < option_edit);
 	if (!msg.len)
 		abort_commit(_("Empty commit message."));
 	strbuf_release(&merge_msg);
@@ -1076,6 +1076,29 @@ static void write_merge_state(void)
 	close(fd);
 }
 
+static int default_edit_option(void)
+{
+	static const char name[] = "GIT_MERGE_LEGACY";
+	const char *e = getenv(name);
+	struct stat st_stdin, st_stdout;
+
+	if (e) {
+		int v = git_config_maybe_bool(name, e);
+		if (v < 0)
+			die("Bad value '%s' in environment '%s'", e, name);
+		return !v;
+	}
+
+	/* Use editor if stdin and stdout are the same and is a tty */
+	return (!fstat(0, &st_stdin) &&
+		!fstat(1, &st_stdout) &&
+		isatty(0) &&
+		st_stdin.st_dev == st_stdout.st_dev &&
+		st_stdin.st_ino == st_stdout.st_ino &&
+		st_stdin.st_rdev == st_stdout.st_rdev);
+}
+
+
 int cmd_merge(int argc, const char **argv, const char *prefix)
 {
 	unsigned char result_tree[20];
@@ -1261,6 +1284,9 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
 		}
 	}
 
+	if (option_edit < 0)
+		option_edit = default_edit_option();
+
 	if (!use_strategies) {
 		if (!remoteheads->next)
 			add_strategies(pull_twohead, DEFAULT_TWOHEAD);
diff --git a/t/test-lib.sh b/t/test-lib.sh
index bdd9513..439f192 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -63,7 +63,8 @@ GIT_AUTHOR_NAME='A U Thor'
 GIT_COMMITTER_EMAIL=committer@example.com
 GIT_COMMITTER_NAME='C O Mitter'
 GIT_MERGE_VERBOSITY=5
-export GIT_MERGE_VERBOSITY
+GIT_MERGE_LEGACY=yes
+export GIT_MERGE_VERBOSITY GIT_MERGE_LEGACY
 export GIT_AUTHOR_EMAIL GIT_AUTHOR_NAME
 export GIT_COMMITTER_EMAIL GIT_COMMITTER_NAME
 export EDITOR
-- 
1.7.9.rc0.39.ged86b

^ permalink raw reply related

* Re: Intriguing error with Git 1.6.3.1: Too many open files
From: Jonathan Nieder @ 2012-01-11  7:18 UTC (permalink / raw)
  To: Soham Mehta; +Cc: git
In-Reply-To: <CALjyegVzchN==DEsARD65jEmz0BkkbYMQRhG=ujNiGCBHuB4SQ@mail.gmail.com>

Soham Mehta wrote:

> I haven't played with versions yet.

For almost all purposes, the latest stable version should work fine
(v1.7.8.3 at the moment).  The people on this list tend to be very
keen to hear about cases where that's not true --- if people are stuck
using old versions because of regressions, that means we're failing.

> Are you alluding that #2 on this
> list may have addressed it?
> https://raw.github.com/gitster/git/master/Documentation/RelNotes/1.7.4.2.txt

Not exactly.  I was thinking of commit c7934306 (Limit file
descriptors used by packs, 2011-02-28).

In git time, 1.6.3.1 is ancient history.  There are many little fixes
and performance improvements you are missing out on.

Just my two cents,
Jonathan

^ permalink raw reply

* Re: [PATCH 3/3] diff-index: pass pathspec down to unpack-trees machinery
From: Junio C Hamano @ 2012-01-11  8:05 UTC (permalink / raw)
  To: Jonathan Nieder; +Cc: git, Linus Torvalds, Nguyen Thai Ngoc Duy
In-Reply-To: <20120111063104.GA3153@burratino>

Jonathan Nieder <jrnieder@gmail.com> writes:

> Junio C Hamano wrote:
>
>> And finally, pass the pathspec down through unpack_trees() to traverse_trees()
>> callchain.
>
> In git versions which include the patch described above, the unchanged
> subdir/hello.h shows up as a newly added file.  Reverting that patch
> (v1.7.7.1~22^2, diff-index: pass pathspec down to unpack-trees
> machinery, 2011-08-29) makes "git diff HEAD" with wildcards work
> again.

I suspect that the particular change on the side branch predates Nguyen's
effort to unify the pathspec semantics to teach the wildcard (i.e. not the
traditional "prefix match") to the tree traversal code, but it is fairly
late here, so I didn't check.

I think the right fix is to update the logic that still assumes that a
pathspec used for tree traversal is always prefix match when leaving the
traversal early, and instead use the proper matching logic that knows that
a wildcard pathspec needs to dig deeper into the tree regardless (I think
the pathspec implementation used in "git grep" got this right, but please
double check), so that we do not dig unnecessary subtrees when pathspecs
are all prefixes, but still keep digging when there is a wildcard match.

I suspect that it would lead to a fairly complete unification of the three
implementations of pathspec matching logic and would allow us to also do
something like "git log -- '*.h'" for free.

^ permalink raw reply

* Re: [PATCH RFC] commit: allow to commit even if there are intent-to-add entries
From: Junio C Hamano @ 2012-01-11  8:08 UTC (permalink / raw)
  To: Nguyễn Thái Ngọc Duy; +Cc: git
In-Reply-To: <1326261707-11484-1-git-send-email-pclouds@gmail.com>

Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:

> This patch replaces the approach in 331fcb5 (git add --intent-to-add:
> do not let an empty blob be committed by accident) regarding i-t-a
> entries: instead of forbidding i-t-a entries at commit time, we can
> simply ignore them.

I have a mild suspicion that in earlier incarnation of the patch we used
to let empty blobs committed, and then we used to instead not commit
anything at all for such a path, and the real users were bitten by either
of these approaches, forgetting to add the contents to the final commit.

So I am not sure if this is such a good idea.

^ permalink raw reply

* Re: rsync a *bunch* of git repos
From: Andreas T.Auer @ 2012-01-11  8:01 UTC (permalink / raw)
  To: Jason; +Cc: git
In-Reply-To: <20120110211548.GD10255@titan.lakedaemon.net>

On 10.01.2012 22:15 Jason wrote:
> The nuts and bolts of this aren't difficult, the problem is I don't have
> a complete understanding of how git stores data.  I've heard in the
> past that it uses a lot of hardlinks and softlinks.  I need to make
> sure, that once I transfer the data, and reboot the machine with the new
> partition mounted under /home, that all my git repos will be okay.
>
>    
As far as I know git uses hardlinks to save diskspace, but it doesn't 
rely on hardlinks to work. It also works with filesystems that don't 
support hardlinks.
But without hardlinks you will probably waste disk space so I recommend 
the -H parameter to rsync, which preserve hardlinks, if possible. If you 
have hundreds of git repos that are cloned from each other, than this 
should make a difference.

^ permalink raw reply

* Re: [PATCH] t2203: fix wrong commit command
From: Junio C Hamano @ 2012-01-11  8:09 UTC (permalink / raw)
  To: Nguyễn Thái Ngọc Duy; +Cc: git
In-Reply-To: <1326252098-2891-1-git-send-email-pclouds@gmail.com>

Thanks.

^ permalink raw reply

* Re: leaky cherry-pick
From: Junio C Hamano @ 2012-01-11  8:16 UTC (permalink / raw)
  To: Ramkumar Ramachandra
  Cc: Jeff King, Pete Wyckoff, git, Nguyễn Thái Ngọc
In-Reply-To: <CALkWK0kDnxjtQ+ihH_dif_7yivHLd=pibao4KPs_PDXfc2UMOA@mail.gmail.com>

Ramkumar Ramachandra <artagnon@gmail.com> writes:

>> If you then do a lookup for "foo/bar/baz/file2", it can use the exact
>> same stack without looking for or reparsing the attribute files. If you
>> then do a lookup for "foo/bar/bleep/file", it pops only the entry for
>> "foo/bar/baz/.gitattributes", and pushes only the entry for
>> "foo/bar/bleep/.gitattributes".
>
> I see.  Thanks for the excellent explanation-  I'll try implementing
> this scheme.

I somehow have a feeling that you did not read the conclusion in Peff's
message correctly.  The code only keeps data from one active path of
per-directory .gitattributes files to the leaf of a working tree and
releases unneeded data (IOW, it "pops" the attr_stack elements) when it
goes on to look at the next path, so my understanding is that there is
nothing to "try implementing" here.

Unless attr.c::free_attr_elem() is leaky, that is. If that were the case,
such a leak will accumulate and would make a difference.

^ permalink raw reply

* Re: leaky cherry-pick
From: Ramkumar Ramachandra @ 2012-01-11  9:00 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Jeff King, Pete Wyckoff, git, Nguyễn Thái Ngọc
In-Reply-To: <7vipki7ix9.fsf@alter.siamese.dyndns.org>

Junio C Hamano wrote:
> Ramkumar Ramachandra <artagnon@gmail.com> writes:
>
>>> If you then do a lookup for "foo/bar/baz/file2", it can use the exact
>>> same stack without looking for or reparsing the attribute files. If you
>>> then do a lookup for "foo/bar/bleep/file", it pops only the entry for
>>> "foo/bar/baz/.gitattributes", and pushes only the entry for
>>> "foo/bar/bleep/.gitattributes".
>>
>> I see.  Thanks for the excellent explanation-  I'll try implementing
>> this scheme.
>
> I somehow have a feeling that you did not read the conclusion in Peff's
> message correctly.  The code only keeps data from one active path of
> per-directory .gitattributes files to the leaf of a working tree and
> releases unneeded data (IOW, it "pops" the attr_stack elements) when it
> goes on to look at the next path, so my understanding is that there is
> nothing to "try implementing" here.

My bad-  I thought the current implementation doesn't release the
unneeded data.  So, does the entire 7 KB of leaked data come from one
active path?

-- Ram

^ permalink raw reply

* [PATCH 0/2] Make t9200-git-cvsexportcommit.sh pass on MSYS
From: Sebastian Schuberth @ 2012-01-11  9:16 UTC (permalink / raw)
  To: msysgit; +Cc: git

This tiny series makes the long-pending t9200-git-cvsexportcommit.sh pass on MSYS.

Sebastian Schuberth (2):
  t9200: On MSYS, do not pass Windows-style paths to CVS
  git-cvsexportcommit: Fix calling Perl's rel2abs() on MSYS

 git-cvsexportcommit.perl       |    7 +++++++
 t/t9200-git-cvsexportcommit.sh |    6 +++---
 2 files changed, 10 insertions(+), 3 deletions(-)

-- 
1.7.9.rc0.5096.g30a61

^ permalink raw reply

* [PATCH 1/2] t9200: On MSYS, do not pass Windows-style paths to CVS
From: Sebastian Schuberth @ 2012-01-11  9:20 UTC (permalink / raw)
  Cc: msysgit, git
In-Reply-To: <4F0D5367.1000506@gmail.com>

For details, see the commit message of 4114156ae9. Note that while using
$PWD as part of GIT_DIR is not required here, it does no harm and it is
more consistent. In addition, on MSYS using an environment variable should
be slightly faster than spawning an external executable.

Signed-off-by: Sebastian Schuberth <sschuberth@gmail.com>
---
 t/t9200-git-cvsexportcommit.sh |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/t/t9200-git-cvsexportcommit.sh b/t/t9200-git-cvsexportcommit.sh
index 41db05c..518358a 100755
--- a/t/t9200-git-cvsexportcommit.sh
+++ b/t/t9200-git-cvsexportcommit.sh
@@ -19,9 +19,9 @@ then
     test_done
 fi
 
-CVSROOT=$(pwd)/cvsroot
-CVSWORK=$(pwd)/cvswork
-GIT_DIR=$(pwd)/.git
+CVSROOT=$PWD/cvsroot
+CVSWORK=$PWD/cvswork
+GIT_DIR=$PWD/.git
 export CVSROOT CVSWORK GIT_DIR
 
 rm -rf "$CVSROOT" "$CVSWORK"
-- 
1.7.9.rc0.5096.g30a61

^ permalink raw reply related

* Re: [PATCH 2/8] revert: decouple sequencer actions from builtin commands
From: Ramkumar Ramachandra @ 2012-01-11  9:19 UTC (permalink / raw)
  To: Jonathan Nieder; +Cc: Git List, Junio C Hamano
In-Reply-To: <20120111054954.GB13507@burratino>

Jonathan Nieder wrote:
> Well, let's start here.  The two insns "pick" and "revert" and the
> ability to mix them doesn't have much to do with the picture, does it?
>
> I think the actual problem being solved is that insn types, as described
> by the replay_action enum, are being abused to refer to top-level git
> commands "revert" and "cherry-pick".  The sequencer isn't supposed to
> care which top-level git command called it, except in some messages, so
> we'd certainly like to stop pretending that has something to do with
> insn types.

Exactly.  I'll update the commit message.

> Based on what you've said, correcting this cleanly is complicated in
> some places by the inconvenient fact that the sequencer _does_ care
> which top-level git command called it.  (I haven't checked this; I'm
> just taking it on faith from you.)  If we want to let other git
> commands (like "git rebase" or "git sequence") call into the
> sequencer, that sounds like a way bigger problem than any conflict of
> terminology.

There's no need to trust me: just think about the problem.  We fill
out a replay_opts structure to call into the sequencer with- how does
the sequencer know what to do with this hypothetical command string
(say "cherry-pick") on a fresh invocation?  It needs to translate this
into a replay_action at some point, right?  There are atleast three
places where this happens: prepare_revs(), walk_revs_populate_todo(),
and single_pick().

-- Ram

^ permalink raw reply

* [PATCH 2/2] git-cvsexportcommit: Fix calling Perl's rel2abs() on MSYS
From: Sebastian Schuberth @ 2012-01-11  9:21 UTC (permalink / raw)
  Cc: msysgit, git
In-Reply-To: <4F0D5367.1000506@gmail.com>

Due to MSYS path mangling GIT_DIR contains a Windows-style path when
checked inside a Perl script even if GIT_DIR was previously set to an
MSYS-style path in a shell script. So explicitly convert to an MSYS-style
path before calling Perl's rel2abs() to make it work.

This fix was inspired by a very similar patch in WebKit:

http://trac.webkit.org/changeset/76255/trunk/Tools/Scripts/commit-log-editor

Signed-off-by: Sebastian Schuberth <sschuberth@gmail.com>
---
 git-cvsexportcommit.perl |    7 +++++++
 1 files changed, 7 insertions(+), 0 deletions(-)

diff --git a/git-cvsexportcommit.perl b/git-cvsexportcommit.perl
index 39a426e..e6bf252 100755
--- a/git-cvsexportcommit.perl
+++ b/git-cvsexportcommit.perl
@@ -30,6 +30,13 @@ if ($opt_w || $opt_W) {
 		chomp($gd);
 		$ENV{GIT_DIR} = $gd;
 	}
+
+	# On MSYS, convert a Windows-style path to an MSYS-style path
+	# so that rel2abs() below works correctly.
+	if ($^O eq 'msys') {
+		$ENV{GIT_DIR} =~ s#^([[:alpha:]]):/#/$1/#;
+	}
+
 	# Make sure GIT_DIR is absolute
 	$ENV{GIT_DIR} = File::Spec->rel2abs($ENV{GIT_DIR});
 }
-- 
1.7.9.rc0.5096.g30a61

^ permalink raw reply related

* Re: [PATCH 2/8] revert: decouple sequencer actions from builtin commands
From: Jonathan Nieder @ 2012-01-11  9:52 UTC (permalink / raw)
  To: Ramkumar Ramachandra; +Cc: Git List, Junio C Hamano
In-Reply-To: <CALkWK0=o+KkV08G9JuTaG8Vtb-AzHQVMQPzMy8td_iBVnGX4Dg@mail.gmail.com>

Ramkumar Ramachandra wrote:

>                                                              how does
> the sequencer know what to do with this hypothetical command string
> (say "cherry-pick") on a fresh invocation?  It needs to translate this
> into a replay_action at some point, right?  There are atleast three
> places where this happens: prepare_revs(), walk_revs_populate_todo(),
> and single_pick().

I see.

Perhaps cherry-pick and revert should be different values for
replay_subcommand, to avoid conflating the mechanics and the command
name?  Resulting in something like this:

	enum replay_subcommand {
		REPLAY_PICK_REVISIONS,
		REPLAY_REVERT_REVISIONS,
		REPLAY_EDIT_SEQUENCE,
		REPLAY_REMOVE_STATE,
		REPLAY_CONTINUE,
		REPLAY_SKIP,
		REPLAY_ROLLBACK
	};

Though this dispatcher on an enum to perform many different actions
already felt a bit awkward, so an alternative could be

	extern int pick_revisions(struct replay_opts *opts);
	extern int revert_revisions(struct replay_opts *opts);

	extern int launch_sequence_editor(struct replay_opts *opts);
	extern void remove_sequencer_state(void);
	extern int sequencer_continue(struct replay_opts *opts);
	extern int sequencer_skip(struct replay_opts *opts);
	extern int sequencer_rollback(struct replay_opts *opts);

which would make it easier to add arguments specific to any one of the
routines as appropriate.

^ permalink raw reply

* Re: [PATCH RFC] commit: allow to commit even if there are intent-to-add entries
From: Nguyễn Thái Ngọc Duy @ 2012-01-11  9:59 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Nguyễn Thái Ngọc Duy
In-Reply-To: <1326261707-11484-1-git-send-email-pclouds@gmail.com>

2012/1/11 Junio C Hamano <gitster@pobox.com>:
> I have a mild suspicion that in earlier incarnation of the patch we used
> to let empty blobs committed, and then we used to instead not commit
> anything at all for such a path, and the real users were bitten by either
> of these approaches, forgetting to add the contents to the final commit.
>
> So I am not sure if this is such a good idea.

I found your elaborate writing [1] about it. These are the
interpretations listed in that post:

-- 8< --
When running "commit" and "status" with files marked with "intent to add",
I think there are three possible interpretations of what the user wants to
do.

 (1) I earlier said "I'll decide the exact contents to be committed for
     these paths and tell you by running 'git add' later." when I said
     'git add -N'.  But I forgot to do so before running "git commit".
     Thanks for catching this mistake and erroring out.

 (2) I said "I'll decide the exact content to be committed ... later."
     when I said 'git add -N'. I am running "git commit" now, but I still
     don't know what the final contents for this path should be.  I
     changed my mind, and I do not want to include the addition of these
     paths in the commit I am making.  Please do not error out, but just
     ignore the earlier 'add -N' for now.

 (3) I said "I'll decide the exact content to be committed ... later."
     when I said 'git add -N'. I am running "git commit" now, without
     explicitly telling you with 'git add' about the final contents for
     these paths.  Please take it as an implicit hint that I am happy with
     the contents in the working tree and commit them as if I said 'git
     add' on these paths, but do leave modifications to already tracked
     paths that I haven't added with 'git add'.
-- 8< --

So (1) may be the safe and sane interpretation and should be the
default. But perhaps we should allow (2) also, for example with
--skip-intent-to-add option? It's really frustrating to remove all
i-t-a, commit (I don't do "commit <path>"), then add them back.

[1] http://article.gmane.org/gmane.comp.version-control.git/170658

Nguyễn Thái Ngọc Duy (2):
  cache-tree: update API to take abitrary flags
  commit: add --skip-intent-to-add to allow commit with i-t-a entries
    in index

 builtin/commit.c       |   10 +++++++---
 cache-tree.c           |   35 +++++++++++++++++------------------
 cache-tree.h           |    5 ++++-
 merge-recursive.c      |    2 +-
 t/t2203-add-intent.sh  |   17 +++++++++++++++++
 test-dump-cache-tree.c |    2 +-
 6 files changed, 47 insertions(+), 24 deletions(-)

-- 
1.7.3.1.256.g2539c.dirty

^ permalink raw reply

* [PATCH 1/2] cache-tree: update API to take abitrary flags
From: Nguyễn Thái Ngọc Duy @ 2012-01-11  9:59 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Nguyễn Thái Ngọc Duy
In-Reply-To: <1326261707-11484-1-git-send-email-pclouds@gmail.com>

---
 builtin/commit.c       |    4 ++--
 cache-tree.c           |   27 ++++++++++++---------------
 cache-tree.h           |    4 +++-
 merge-recursive.c      |    2 +-
 test-dump-cache-tree.c |    2 +-
 5 files changed, 19 insertions(+), 20 deletions(-)

diff --git a/builtin/commit.c b/builtin/commit.c
index eba1377..bf42bb3 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -400,7 +400,7 @@ static char *prepare_index(int argc, const char **argv, const char *prefix,
 		fd = hold_locked_index(&index_lock, 1);
 		add_files_to_cache(also ? prefix : NULL, pathspec, 0);
 		refresh_cache_or_die(refresh_flags);
-		update_main_cache_tree(1);
+		update_main_cache_tree(WRITE_TREE_SILENT);
 		if (write_cache(fd, active_cache, active_nr) ||
 		    close_lock_file(&index_lock))
 			die(_("unable to write new_index file"));
@@ -421,7 +421,7 @@ static char *prepare_index(int argc, const char **argv, const char *prefix,
 		fd = hold_locked_index(&index_lock, 1);
 		refresh_cache_or_die(refresh_flags);
 		if (active_cache_changed) {
-			update_main_cache_tree(1);
+			update_main_cache_tree(WRITE_TREE_SILENT);
 			if (write_cache(fd, active_cache, active_nr) ||
 			    commit_locked_index(&index_lock))
 				die(_("unable to write new_index file"));
diff --git a/cache-tree.c b/cache-tree.c
index 8de3959..16355d6 100644
--- a/cache-tree.c
+++ b/cache-tree.c
@@ -150,9 +150,10 @@ void cache_tree_invalidate_path(struct cache_tree *it, const char *path)
 }
 
 static int verify_cache(struct cache_entry **cache,
-			int entries, int silent)
+			int entries, int flags)
 {
 	int i, funny;
+	int silent = flags & WRITE_TREE_SILENT;
 
 	/* Verify that the tree is merged */
 	funny = 0;
@@ -241,10 +242,11 @@ static int update_one(struct cache_tree *it,
 		      int entries,
 		      const char *base,
 		      int baselen,
-		      int missing_ok,
-		      int dryrun)
+		      int flags)
 {
 	struct strbuf buffer;
+	int missing_ok = flags & WRITE_TREE_MISSING_OK;
+	int dryrun = flags & WRITE_TREE_DRY_RUN;
 	int i;
 
 	if (0 <= it->entry_count && has_sha1_file(it->sha1))
@@ -288,8 +290,7 @@ static int update_one(struct cache_tree *it,
 				    cache + i, entries - i,
 				    path,
 				    baselen + sublen + 1,
-				    missing_ok,
-				    dryrun);
+				    flags);
 		if (subcnt < 0)
 			return subcnt;
 		i += subcnt - 1;
@@ -371,15 +372,13 @@ static int update_one(struct cache_tree *it,
 int cache_tree_update(struct cache_tree *it,
 		      struct cache_entry **cache,
 		      int entries,
-		      int missing_ok,
-		      int dryrun,
-		      int silent)
+		      int flags)
 {
 	int i;
-	i = verify_cache(cache, entries, silent);
+	i = verify_cache(cache, entries, flags);
 	if (i)
 		return i;
-	i = update_one(it, cache, entries, "", 0, missing_ok, dryrun);
+	i = update_one(it, cache, entries, "", 0, flags);
 	if (i < 0)
 		return i;
 	return 0;
@@ -572,11 +571,9 @@ int write_cache_as_tree(unsigned char *sha1, int flags, const char *prefix)
 
 	was_valid = cache_tree_fully_valid(active_cache_tree);
 	if (!was_valid) {
-		int missing_ok = flags & WRITE_TREE_MISSING_OK;
-
 		if (cache_tree_update(active_cache_tree,
 				      active_cache, active_nr,
-				      missing_ok, 0, 0) < 0)
+				      flags) < 0)
 			return WRITE_TREE_UNMERGED_INDEX;
 		if (0 <= newfd) {
 			if (!write_cache(newfd, active_cache, active_nr) &&
@@ -672,10 +669,10 @@ int cache_tree_matches_traversal(struct cache_tree *root,
 	return 0;
 }
 
-int update_main_cache_tree (int silent)
+int update_main_cache_tree (int flags)
 {
 	if (!the_index.cache_tree)
 		the_index.cache_tree = cache_tree();
 	return cache_tree_update(the_index.cache_tree,
-				 the_index.cache, the_index.cache_nr, 0, 0, silent);
+				 the_index.cache, the_index.cache_nr, flags);
 }
diff --git a/cache-tree.h b/cache-tree.h
index 0ec0b2a..d8cb2e9 100644
--- a/cache-tree.h
+++ b/cache-tree.h
@@ -29,13 +29,15 @@ void cache_tree_write(struct strbuf *, struct cache_tree *root);
 struct cache_tree *cache_tree_read(const char *buffer, unsigned long size);
 
 int cache_tree_fully_valid(struct cache_tree *);
-int cache_tree_update(struct cache_tree *, struct cache_entry **, int, int, int, int);
+int cache_tree_update(struct cache_tree *, struct cache_entry **, int, int);
 
 int update_main_cache_tree(int);
 
 /* bitmasks to write_cache_as_tree flags */
 #define WRITE_TREE_MISSING_OK 1
 #define WRITE_TREE_IGNORE_CACHE_TREE 2
+#define WRITE_TREE_DRY_RUN 4
+#define WRITE_TREE_SILENT 8
 
 /* error return codes */
 #define WRITE_TREE_UNREADABLE_INDEX (-1)
diff --git a/merge-recursive.c b/merge-recursive.c
index d83cd6c..6479a60 100644
--- a/merge-recursive.c
+++ b/merge-recursive.c
@@ -264,7 +264,7 @@ struct tree *write_tree_from_memory(struct merge_options *o)
 
 	if (!cache_tree_fully_valid(active_cache_tree) &&
 	    cache_tree_update(active_cache_tree,
-			      active_cache, active_nr, 0, 0, 0) < 0)
+			      active_cache, active_nr, 0) < 0)
 		die("error building trees");
 
 	result = lookup_tree(active_cache_tree->sha1);
diff --git a/test-dump-cache-tree.c b/test-dump-cache-tree.c
index e6c2923..a6ffdf3 100644
--- a/test-dump-cache-tree.c
+++ b/test-dump-cache-tree.c
@@ -59,6 +59,6 @@ int main(int ac, char **av)
 	struct cache_tree *another = cache_tree();
 	if (read_cache() < 0)
 		die("unable to read index file");
-	cache_tree_update(another, active_cache, active_nr, 0, 1, 0);
+	cache_tree_update(another, active_cache, active_nr, WRITE_TREE_DRY_RUN);
 	return dump_cache_tree(active_cache_tree, another, "");
 }
-- 
1.7.3.1.256.g2539c.dirty

^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox