Re: [PATCH] Speedup recursive by flushing index only once for all entries

From: Sergey Vlasov <vsu@altlinux.ru>
Date: Jan 12, 2007, 15:48 UTC
Message-ID: <20070112184839.9431ddff.vsu@altlinux.ru>
In-Reply-To: <7vr6u2adgx.fsf@assigned-by-dhcp.cox.net>

On Wed, 10 Jan 2007 11:28:14 -0800 Junio C Hamano wrote:

Show 15 quoted lines

> From: Junio C Hamano <junkio@cox.net>
> Date: Wed, 10 Jan 2007 11:20:58 -0800
> Subject: [PATCH] merge-recursive: do not use on-file index when not needed.
>
> This revamps the merge-recursive implementation following the
> outline in:
>
> 	Message-ID: <7v8xgileza.fsf@assigned-by-dhcp.cox.net>
>
> There is no need to write out the index until the very end just
> once from merge-recursive.  Also there is no need to write out
> the resulting tree object for the simple case of merging with a
> single merge base.
>
> Signed-off-by: Junio C Hamano <junkio@cox.net>

This commit broke t3401-rebase-partial.sh:

...
*   ok 3: rebase topic branch against new master and check git-am did not get halted

* expecting success: git-checkout -f my-topic-branch-merge &&
         git-rebase --merge master-merge &&
         test ! -d .git/.dotest-merge
First, rewinding head to replay your work on top of it...
HEAD is now at 5f97179... Add C.
Merging master-merge with my-topic-branch-merge~1
Merging:
5f97179 Add C.
1be2c8e Add B.
found 1 common ancestor(s):
0e8cba9 Add A.
.../git-rebase: line 82: 11517 Segmentation fault      git-merge-$strategy "$cmt^" -- "$hd" "$cmt"
Unknown exit code (139) from command: git-merge-recursive 1be2c8e0eba8a7a383d0403facb1c72c622c0939^ -- HEAD 1be2c8e0eba8a7a383d0403facb1c72c622c0939
* FAIL 4: rebase --merge topic branch that was partially merged upstream
        git-checkout -f my-topic-branch-merge &&
                 git-rebase --merge master-merge &&
                 test ! -d .git/.dotest-merge

* failed 1 among 4 test(s)

Show 5 quoted lines

> @@ -1105,9 +1040,7 @@ static int merge_trees(struct tree *head,
>  		    sha1_to_hex(head->object.sha1),
>  		    sha1_to_hex(merge->object.sha1));
>
> -	*result = git_write_tree();

Previously *result was set here...

Show 27 quoted lines

> -
> -	if (!*result) {
> +	if (unmerged_index()) {
>  		struct path_list *entries, *re_head, *re_merge;
>  		int i;
>  		path_list_clear(&current_file_set, 1);
> @@ -1128,17 +1061,11 @@ static int merge_trees(struct tree *head,
>  			if (!process_entry(path, e, branch1, branch2))
>  				clean = 0;
>  		}
> -		if (cache_dirty)
> -			flush_cache();
>
>  		path_list_clear(re_merge, 0);
>  		path_list_clear(re_head, 0);
>  		path_list_clear(entries, 1);
>
> -		if (clean || index_only)
> -			*result = git_write_tree();
> -		else
> -			*result = NULL;
>  	} else {
>  		clean = 1;
>  		printf("merging of trees %s and %s resulted in %s\n",
> @@ -1146,6 +1073,8 @@ static int merge_trees(struct tree *head,
>  		       sha1_to_hex(merge->object.sha1),
>  		       sha1_to_hex((*result)->object.sha1));

...and it is still used here - however, after the patch *result is uninitialized at this point.

>  	}
> +	if (index_only)
> +		*result = git_write_tree();

Too late...

>
>  	return clean;
>  }