* What's cooking in git.git (Aug 2008, #05; Tue, 19)
@ 2008-08-19 9:05 Junio C Hamano
2008-08-19 11:02 ` Johannes Sixt
2008-08-19 12:54 ` Miklos Vajna
0 siblings, 2 replies; 35+ messages in thread
From: Junio C Hamano @ 2008-08-19 9:05 UTC (permalink / raw)
To: git
Here are the topics that have been cooking. Commits prefixed
with '-' are only in 'pu' while commits prefixed with '+' are
in 'next'.
The topics list the commits in reverse chronological order. The topics
meant to be merged to the maintenance series have "maint-" in their names.
Tonight's 'pu' does not pass tests because test vectors have not been
adjusted for the changes brought in by the jc/diff-prefix topic.
----------------------------------------------------------------
[New Topics]
* js/mingw-stat (Mon Aug 18 22:01:06 2008 +0200) 2 commits
- Revert "Windows: Use a customized struct stat that also has the
st_blocks member."
- compat: introduce on_disk_bytes()
This gets rid of use of st_blocks member (which is XSI but not POSIX
proper), which was originally prompted by recent Haiku port but it turns
out MinGW has the same issue as well. Queued on 'pu' just to have a
chance to make sure I munged the version j6t sent me correctly before
merging it upwards.
* ml/submodule-foreach (Sun Aug 10 19:10:04 2008 -0400) 1 commit
+ git-submodule - Add 'foreach' subcommand
* jc/stripspace (Sun Mar 9 00:30:35 2008 -0800) 6 commits
- git-am --forge: add Signed-off-by: line for the author
- git-am: clean-up Signed-off-by: lines
- stripspace: add --log-clean option to clean up signed-off-by:
lines
- stripspace: use parse_options()
- Add "git am -s" test
- git-am: refactor code to add signed-off-by line for the committer
* pm/log-exit-code (Mon Aug 11 08:46:25 2008 +0200) 2 commits
+ Teach git log --exit-code to return an appropriate exit code
+ Teach git log --check to return an appropriate exit code
* sb/commit-tree-minileak (Tue Aug 12 00:35:11 2008 +0200) 1 commit
+ Fix commit_tree() buffer leak
* pb/reflog-dwim (Sun Aug 10 22:22:21 2008 +0200) 1 commit
+ builtin-reflog: Allow reflog expire to name partial ref
* jc/send-pack-tell-me-more (Thu Mar 20 00:44:11 2008 -0700) 1 commit
- "git push": tellme-more protocol extension
* jc/merge-whitespace (Sun Feb 24 23:29:36 2008 -0800) 1 commit
- WIP: start teaching the --whitespace=fix to merge machinery
* lw/gitweb (Mon Aug 18 21:39:49 2008 +0200) 3 commits
- gitweb: use new Git::Repo API, and add optional caching
- add new Perl API: Git::Repo, Git::Commit, Git::Tag, and
Git::RepoRoot
- gitweb: add test suite with Test::WWW::Mechanize::CGI
* jc/diff-prefix (Mon Aug 18 20:08:09 2008 -0700) 1 commit
- diff: vary default prefix depending on what are compared
* jc/blame (Wed Jun 4 22:58:40 2008 -0700) 2 commits
- blame: show "previous" information in --porcelain/--incremental
format
- git-blame: refactor code to emit "porcelain format" output
----------------------------------------------------------------
[Graduated to "master"]
* ak/p4 (Thu Aug 14 23:40:39 2008 +0100) 14 commits
+ Utilise our new p4_read_pipe and p4_write_pipe wrappers
+ Add p4 read_pipe and write_pipe wrappers
+ Put in the two other configuration elements found in the source
+ Put some documentation in about the parameters that have been
added
+ Move git-p4.syncFromOrigin into a configuration parameters section
+ Consistently use 'git-p4' for the configuration entries
+ If the user has configured various parameters, use them.
+ Switch to using 'p4_build_cmd'
+ If we are in verbose mode, output what we are about to run (or
return)
+ Add a single command that will be used to construct the 'p4'
command
+ Utilise the new 'p4_system' function.
+ Have a command that specifically invokes 'p4' (via system)
+ Utilise the new 'p4_read_pipe_lines' command
+ Create a specific version of the read_pipe_lines command for p4
invocations
Warmly received by the primary contributors of git-p4; this was merged as
part of 1.6.0.
----------------------------------------------------------------
[Will merge to master soon]
* js/checkout-dwim-local (Sat Aug 9 16:00:12 2008 +0200) 1 commit
+ checkout --track: make up a sensible branch name if '-b' was
omitted
* bd/diff-strbuf (Wed Aug 13 23:18:22 2008 -0700) 3 commits
+ xdiff-interface: hide the whole "xdiff_emit_state" business from
the caller
+ Use strbuf for struct xdiff_emit_state's remainder
+ Make xdi_diff_outf interface for running xdiff_outf diffs
Gives measurable performance improvement to textual diff generation. For
improving "blame" performance, it might be more effective to hook directly
to lower level of xdiff machinery so that we do not even have to generate
patch only to discard after reading "@@ -l,k +m,n @@" lines, but that
would be a separate topic.
* jc/add-stop-at-symlink (Mon Aug 4 00:52:37 2008 -0700) 2 commits
+ add: refuse to add working tree items beyond symlinks
+ update-index: refuse to add working tree items beyond symlinks
Fix for a longstanding bug that allows "git add" and "git update-index" to
add a path "a/b" to the index when "a" is a symbolic link. We would need
a similar fix for the case where "a" is a submodule.
* dp/hash-literally (Sun Aug 3 18:36:22 2008 +0400) 6 commits
+ add --no-filters option to git hash-object
+ add --path option to git hash-object
+ use parse_options() in git hash-object
+ correct usage help string for git-hash-object
+ correct argument checking test for git hash-object
+ teach index_fd to work with pipes
Gives a bit more flexibility to hash-objects by allowing us to lie about
the path the contents comes from.
* mv/merge-custom (Wed Aug 13 23:32:43 2008 +0200) 7 commits
+ Update .gitignore to ignore git-help
+ Builtin git-help.
+ builtin-help: always load_command_list() in cmd_help()
+ Add a second testcase for handling invalid strategies in git-merge
+ Add a new test for using a custom merge strategy
+ builtin-merge: allow using a custom strategy
+ builtin-help: make some internal functions available to other
builtins
* kh/diff-tree (Sun Aug 10 18:13:04 2008 +0200) 4 commits
+ Add test for diff-tree --stdin with two trees
+ Teach git diff-tree --stdin to diff trees
+ diff-tree: Note that the commit ID is printed with --stdin
+ Refactoring: Split up diff_tree_stdin
* mg/count-objects (Fri Aug 15 00:20:20 2008 -0400) 1 commit
+ count-objects: Add total pack size to verbose output
This one is without the human readable bits.
* mz/push-verbose (Sat Aug 16 19:58:32 2008 +0200) 1 commit
+ Make push more verbose about illegal combination of options
* jc/index-extended-flags (Sat Aug 16 23:02:08 2008 -0700) 1 commit
+ index: future proof for "extended" index entries
* cc/merge-base-many (Sun Jul 27 13:47:22 2008 -0700) 4 commits
+ git-merge-octopus: use (merge-base A (merge B C D E...)) for
stepwise merge
+ merge-base-many: add trivial tests based on the documentation
+ documentation: merge-base: explain "git merge-base" with more than
2 args
+ merge-base: teach "git merge-base" to drive underlying
merge_bases_many()
* rs/imap (Wed Jul 9 22:29:02 2008 +0100) 5 commits
+ Documentation: Improve documentation for git-imap-send(1)
+ imap-send.c: more style fixes
+ imap-send.c: style fixes
+ git-imap-send: Support SSL
+ git-imap-send: Allow the program to be run from subdirectories of
a git tree
Some people seem to prefer having this feature available also with gnutls.
Such an enhancement can be done in-tree on top of this series if they are
so inclined.
* jc/add-addremove (Tue Jul 22 22:30:40 2008 -0700) 2 commits
+ builtin-add.c: optimize -A option and "git add ."
+ builtin-add.c: restructure the code for maintainability
* jk/pager-swap (Tue Jul 22 03:14:12 2008 -0400) 2 commits
+ spawn pager via run_command interface
+ run-command: add pre-exec callback
This changes the parent-child relationship between the pager and the git
process. We used to make pager the parent which meant that the exit
status from git is lost from the caller.
* ph/enable-threaded (Mon Jul 21 11:23:43 2008 +0200) 1 commit
+ Enable threaded delta search on *BSD and Linux.
* am/cherry-pick-rerere (Sun Aug 10 17:18:55 2008 +0530) 1 commit
+ Make cherry-pick use rerere for conflict resolution.
* js/parallel-test (Mon Aug 18 12:25:40 2008 -0400) 4 commits
+ Update t/.gitignore to ignore all trash directories
+ Enable parallel tests
+ tests: Clarify dependencies between tests, 'aggregate-results' and
'clean'
+ t9700: remove useless check
* jc/test-deeper (Fri Aug 8 02:26:28 2008 -0700) 1 commit
+ tests: use $TEST_DIRECTORY to refer to the t/ directory
This does not actually move "t/test directory" any deeper, but fixes test
scripts that assume they run immediately below "t/" to use TEST_DIRECTORY
variable.
----------------------------------------------------------------
[Actively Cooking]
* sp/missing-thin-base (Tue Aug 12 11:31:06 2008 -0700) 1 commit
+ pack-objects: Allow missing base objects when creating thin packs
* tr/filter-branch (Tue Aug 12 10:45:59 2008 +0200) 3 commits
+ filter-branch: use --simplify-merges
+ filter-branch: fix ref rewriting with --subdirectory-filter
+ filter-branch: Extend test to show rewriting bug
Fixes a longstanding filter branch bug.
* jc/post-simplify (Fri Aug 15 01:34:51 2008 -0700) 8 commits
- revision --simplify-merges: incremental simplification
- revision --simplify-merges: prepare for incremental simplification
- revision --simplify-merges: make it a no-op without pathspec
+ revision --simplify-merges: do not leave commits unprocessed
+ revision --simplify-merges: use decoration instead of commit->util
field
+ Topo-sort before --simplify-merges
+ revision traversal: show full history with merge simplification
+ revision.c: whitespace fix
"log --full-history" is with too much clutter, "log" itself is too cleverer
than some people, and here is the middle level of merge simplification.
I started making this incremental but the progress is not so great.
* tr/rev-list-docs (Tue Aug 12 01:55:37 2008 +0200) 1 commit
+ Documentation: rev-list-options: move --simplify-merges
documentation
----------------------------------------------------------------
[On Hold]
* lt/time-reject-fractional-seconds (Sat Aug 16 21:25:40 2008 -0700) 1 commit
- date/time: do not get confused by fractional seconds
Linus hints further enhancements as "the right way", so let's see if
somebody else steps up and tries it before merging this to 'next'.
* jc/cc-ld-dynpath (Sat Aug 16 15:01:23 2008 +0200) 2 commits
- configure: auto detect dynamic library path switches
- Makefile: Allow CC_LD_DYNPATH to be overriden
Needs success reports from people who do use user-defined dynamic library
path when they build their "git" before this series can go anywhere.
* sb/daemon (Thu Aug 14 20:02:20 2008 +0200) 4 commits
- git-daemon: rewrite kindergarden, new option --max-connections
- git-daemon: Simplify dead-children reaping logic
- git-daemon: use LOG_PID, simplify logging code
- git-daemon: call logerror() instead of error()
Can somebody who actually runs the daemon standalone comment on this one?
* mv/merge-recursive (Tue Aug 12 22:14:00 2008 +0200) 3 commits
- Make builtin-revert.c use merge_recursive_generic()
- merge-recursive.c: Add more generic merge_recursive_generic()
- Split out merge_recursive() to merge-recursive.c
I do not think builtlin-revert should use "recursive", but these patches
give a good starting point to separate the bulk of the "rename-aware
three-way merge" into library form.
* sp/smart-http (Sun Aug 3 00:25:17 2008 -0700) 2 commits
- [do not merge -- original version] Add Git-aware CGI for Git-aware
smart HTTP transport
- Add backdoor options to receive-pack for use in Git-aware CGI
The "magic" detection protocol was revised to use POST to info/refs; the
top one queued is from before that discussion.
* cc/bisect (Fri Jul 25 05:36:37 2008 +0200) 2 commits
- bisect: only check merge bases when needed
- bisect: test merge base if good rev is not an ancestor of bad rev
The first one alone does not pass its self-test but combined together they
seem to. It does not build confidence as the latter one is supposed to be
an optimization only. Resend of fixed-up series is needed.
* sg/merge-options (Sun Apr 6 03:23:47 2008 +0200) 1 commit
+ merge: remove deprecated summary and diffstat options and config
variables
This was previously in "will be in master soon" category, but it turns out
that the synonyms to the ones this one deletes are fairly new invention
that happend in 1.5.6 timeframe, and we cannot do this just yet. Perhaps
in 1.7.0.
* jc/dashless (Wed Jun 25 15:55:11 2008 -0700) 1 commit
- Make clients ask for "git program" over ssh and local transport
This is the "botched" one. Will be resurrected during 1.7.0 or 1.8.0
timeframe.
* jk/renamelimit (Sat May 3 13:58:42 2008 -0700) 1 commit
- diff: enable "too large a rename" warning when -M/-C is explicitly
asked for
This would be the right thing to do for command line use, but gitk will be
hit due to tcl/tk's limitation, so I am holding this back for now.
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: What's cooking in git.git (Aug 2008, #05; Tue, 19)
2008-08-19 9:05 What's cooking in git.git (Aug 2008, #05; Tue, 19) Junio C Hamano
@ 2008-08-19 11:02 ` Johannes Sixt
2008-08-19 12:35 ` Andreas Färber
2008-08-19 12:54 ` Miklos Vajna
1 sibling, 1 reply; 35+ messages in thread
From: Johannes Sixt @ 2008-08-19 11:02 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
Junio C Hamano schrieb:
> * js/mingw-stat (Mon Aug 18 22:01:06 2008 +0200) 2 commits
> - Revert "Windows: Use a customized struct stat that also has the
> st_blocks member."
> - compat: introduce on_disk_bytes()
>
> This gets rid of use of st_blocks member (which is XSI but not POSIX
> proper), which was originally prompted by recent Haiku port but it turns
> out MinGW has the same issue as well. Queued on 'pu' just to have a
> chance to make sure I munged the version j6t sent me correctly before
> merging it upwards.
I tested this again, and it works as expected.
-- Hannes
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: What's cooking in git.git (Aug 2008, #05; Tue, 19)
2008-08-19 11:02 ` Johannes Sixt
@ 2008-08-19 12:35 ` Andreas Färber
0 siblings, 0 replies; 35+ messages in thread
From: Andreas Färber @ 2008-08-19 12:35 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Johannes Sixt
Am 19.08.2008 um 13:02 schrieb Johannes Sixt:
> Junio C Hamano schrieb:
>> * js/mingw-stat (Mon Aug 18 22:01:06 2008 +0200) 2 commits
>> - Revert "Windows: Use a customized struct stat that also has the
>> st_blocks member."
>> - compat: introduce on_disk_bytes()
>>
>> This gets rid of use of st_blocks member (which is XSI but not POSIX
>> proper), which was originally prompted by recent Haiku port but it
>> turns
>> out MinGW has the same issue as well. Queued on 'pu' just to have a
>> chance to make sure I munged the version j6t sent me correctly before
>> merging it upwards.
>
> I tested this again, and it works as expected.
So did I for the latter, on Haiku. Together with the hardlink patch
not yet queued, this allows to build and install (*) via `make` with
suitable arguments.
Andreas
(*) Haiku's `read` (bash) seems to be broken - using `ls -1` in
templates/Makefile works around that, but it should be fixed at the
source. http://dev.haiku-os.org/ticket/2646
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: What's cooking in git.git (Aug 2008, #05; Tue, 19)
2008-08-19 9:05 What's cooking in git.git (Aug 2008, #05; Tue, 19) Junio C Hamano
2008-08-19 11:02 ` Johannes Sixt
@ 2008-08-19 12:54 ` Miklos Vajna
2008-08-19 19:19 ` Junio C Hamano
1 sibling, 1 reply; 35+ messages in thread
From: Miklos Vajna @ 2008-08-19 12:54 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
[-- Attachment #1: Type: text/plain, Size: 895 bytes --]
On Tue, Aug 19, 2008 at 02:05:42AM -0700, Junio C Hamano <gitster@pobox.com> wrote:
> [On Hold]
> (...)
> * mv/merge-recursive (Tue Aug 12 22:14:00 2008 +0200) 3 commits
> - Make builtin-revert.c use merge_recursive_generic()
> - merge-recursive.c: Add more generic merge_recursive_generic()
> - Split out merge_recursive() to merge-recursive.c
>
> I do not think builtlin-revert should use "recursive", but these patches
> give a good starting point to separate the bulk of the "rename-aware
> three-way merge" into library form.
I wanted to send a patch that makes builtin-merge use the new
merge_recursive_setup(), but then I was not able to decide to use
merge_recursive_generic() or not.
What is your preference here? I just want to avoid a "this could be
merged, but it uses merge_recursive(), not merge_recursive_generic()" or
the opposite of this. :)
Thanks.
[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: What's cooking in git.git (Aug 2008, #05; Tue, 19)
2008-08-19 12:54 ` Miklos Vajna
@ 2008-08-19 19:19 ` Junio C Hamano
2008-08-19 20:59 ` Miklos Vajna
0 siblings, 1 reply; 35+ messages in thread
From: Junio C Hamano @ 2008-08-19 19:19 UTC (permalink / raw)
To: Miklos Vajna; +Cc: git, Stephan Beyer
Miklos Vajna <vmiklos@frugalware.org> writes:
> On Tue, Aug 19, 2008 at 02:05:42AM -0700, Junio C Hamano <gitster@pobox.com> wrote:
>> [On Hold]
>> (...)
>> * mv/merge-recursive (Tue Aug 12 22:14:00 2008 +0200) 3 commits
>> - Make builtin-revert.c use merge_recursive_generic()
>> - merge-recursive.c: Add more generic merge_recursive_generic()
>> - Split out merge_recursive() to merge-recursive.c
>>
>> I do not think builtlin-revert should use "recursive", but these patches
>> give a good starting point to separate the bulk of the "rename-aware
>> three-way merge" into library form.
>
> I wanted to send a patch that makes builtin-merge use the new
> merge_recursive_setup(), but then I was not able to decide to use
> merge_recursive_generic() or not.
I think git-merge and git-merge-recursive should be the only two that
actually trigger the "recursive" behaviour. Everybody else should be
using non-recursive one, and that non-recursive one can be shared by the
one that is recursive.
Here is how the callchain looks like with your variant.
cmd_merge_recursive()
-> merge_recursive_setup()
-> merge_recursive_generic()
-> merge_recursive()
-> merge_recursive()
-> merge_trees()
The merge_recursive() is the "recursive" one. The workhorse that is not
recursive is merge_trees().
Since the latter is what everybody else ("checkout -m", "revert",
"cherry-pick", "am -3", "stash apply") should be using, I think it is
pretty much up to "git-merge" and "git-merge-recursive" implementations
how the caller of merge_recursive() function is structured. I suspect
that you would not need two separate functions, _setup() and _generic(),
for these two codepaths, but I didn't look closely.
And make_virtual_commit() should become static inside merge_recursive.c;
use of these fake commits is strictly an internal implementation issue of
how merge_recursive() function works and does not concern the caller, does
it?
By the way, the calling convention of merge_recursive_generic() looks
confusing (even though by the above reasoning it does not matter very much
outside "git-merge" and "git-merge-recursive"). Why does it take textual
object names for bases but binary object names for head and next?
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: What's cooking in git.git (Aug 2008, #05; Tue, 19)
2008-08-19 19:19 ` Junio C Hamano
@ 2008-08-19 20:59 ` Miklos Vajna
2008-08-19 22:00 ` Junio C Hamano
0 siblings, 1 reply; 35+ messages in thread
From: Miklos Vajna @ 2008-08-19 20:59 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Stephan Beyer
[-- Attachment #1: Type: text/plain, Size: 1148 bytes --]
On Tue, Aug 19, 2008 at 12:19:09PM -0700, Junio C Hamano <gitster@pobox.com> wrote:
> Since the latter is what everybody else ("checkout -m", "revert",
> "cherry-pick", "am -3", "stash apply") should be using, I think it is
> pretty much up to "git-merge" and "git-merge-recursive" implementations
> how the caller of merge_recursive() function is structured. I suspect
> that you would not need two separate functions, _setup() and _generic(),
> for these two codepaths, but I didn't look closely.
Sure, I can avoid _generic() and use merge_recursive() directly, that's
why I asked.
> And make_virtual_commit() should become static inside merge_recursive.c;
> use of these fake commits is strictly an internal implementation issue of
> how merge_recursive() function works and does not concern the caller, does
> it?
Not exactly. builtin-merge-recursive uses get_ref() - which should not
be in merge-recursive.c IMHO - and get_ref() uses make_virtual_commit().
merge_recursive() itself takes commits, so it can be only static if we
copy it builtin-merge-recursive as well, causing a code duplication. Or
have I missed something here?
Thanks.
[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: What's cooking in git.git (Aug 2008, #05; Tue, 19)
2008-08-19 20:59 ` Miklos Vajna
@ 2008-08-19 22:00 ` Junio C Hamano
2008-08-20 22:42 ` Miklos Vajna
` (2 more replies)
0 siblings, 3 replies; 35+ messages in thread
From: Junio C Hamano @ 2008-08-19 22:00 UTC (permalink / raw)
To: Miklos Vajna; +Cc: git, Stephan Beyer
Miklos Vajna <vmiklos@frugalware.org> writes:
> On Tue, Aug 19, 2008 at 12:19:09PM -0700, Junio C Hamano <gitster@pobox.com> wrote:
>> Since the latter is what everybody else ("checkout -m", "revert",
>> "cherry-pick", "am -3", "stash apply") should be using, I think it is
>> pretty much up to "git-merge" and "git-merge-recursive" implementations
>> how the caller of merge_recursive() function is structured. I suspect
>> that you would not need two separate functions, _setup() and _generic(),
>> for these two codepaths, but I didn't look closely.
>
> Sure, I can avoid _generic() and use merge_recursive() directly, that's
> why I asked.
>
>> And make_virtual_commit() should become static inside merge_recursive.c;
>> use of these fake commits is strictly an internal implementation issue of
>> how merge_recursive() function works and does not concern the caller, does
>> it?
>
> Not exactly. builtin-merge-recursive uses get_ref() - which should not
> be in merge-recursive.c IMHO - and get_ref() uses make_virtual_commit().
> merge_recursive() itself takes commits, so it can be only static if we
> copy it builtin-merge-recursive as well, causing a code duplication. Or
> have I missed something here?
I think you have.
Let's look at the call chain from cmd_merge_recursive() and think again.
cmd_merge_recursive()
-> merge_recursive_setup()
-> merge_recursive_generic()
-> merge_recursive()
-> merge_recursive()
-> merge_trees()
cmd_merge_recursive() takes subtree option and set of object names (two
commits and zero or more base commits), massages them and calls
merge_recursive(). merge_recursive_setup() and merge_recursive_generic()
are involved in this massaging process.
merge_recursive() computes the bases itself when given no base, and in a
multi-base situation, does its thing recursively to come up with a
consolidated base, using virtual commits. After coming up with the three
(virtual or real) commits to use, it gives them to merge_trees(), which
operate solely on tree objects.
In addition, merge_recursive() currently *requires* the caller to wrap
bare tree objects in virtual commits, if the caller wants to do a simple
three-way merge of trees (in which case because there is no ancestry
information available you would naturally not do any recursive behaviour).
This "input must be commit" requirement is why you think you need to have
get_ref() that uses make_virtual_commit() in the caller.
But it does not have to be that way. It is merely an artifact of the
current refactoring that kept the interface into merge_recursive() based
on commit objects. You could further refine the refactoring so that:
- merge_trees(), in addition to the three tree objects, takes options
such as use of the subtree behaviour, descriptive names for heads to be
used for conflict markers, verbosity level, and other future options
(such as "use this lower rename detaction threshold"). Introduce
"struct merge_options" for that and pass it around. These show() and
output() calls could even become callbacks, but I didn't look very
carefully.
- merge_recursive(), in addition to that "merge_options" structure, will
take heads, and list of common ancestors.
- merge_recursive_generic() can be a layer on top of merge_recursive() to
allow the caller to feed tree objects. Use of "const unsigned char *"
to give raw object names (or even "const char *" to feed texual object
names) would be easier for the callers. Wrapping a tree into virtual
commit can and should be done at this layer, hidden away inside
merge-recursive.c from the callers.
Alternatively, you can do away without such preparation step, and move
the "wrap a tree into a virtual commit" inside merge_recursive()
itself. If you take that route, merge_recursive() will take heads and
list of common ancestors all in "const unsigned char *" object names,
in addition to the "merge_options" structure.
When you rewrite cmd_merge() to make direct call to bypass a subprocess,
your callchain would look like:
cmd_merge()
-> merge_recursive()
-> merge_recursive()
-> merge_trees()
cmd_merge() needs to do the same arrangement for "subtree" and any
possible future options, and feed the same set of object names to
merge_recursive(). You cannot give a bare tree to "git merge", so you do
not have to worry about having to wrap it in a virtual commit.
So my gut feeling is that the interface may look something like:
struct merge_options {
const char *branch1_label;
const char *branch2_label;
unsigned subtree_merge : 1;
int verbosity;
/* other options here ... */
};
/* rename-detecting three-way merge, no recursion */
int merge_trees(struct merge_options *,
struct tree *head,
struct tree *merge,
struct tree *common,
struct tree **result);
/* merge_trees() but with recursive ancestor consolidation */
int merge_recursive(struct merge_options *,
struct commit *h1,
struct commit *h2,
struct commit_list *ca,
struct commit **result);
/*
* "git-merge-recursive" can be fed trees; wrap them into
* virtual commits and call merge_recursive() proper.
*/
int merge_recursive_generic(struct merge_options *,
const unsigned char *head,
const unsigned char *merge,
int num_ca,
const unsigned char **ca,
struct commit **result);
and the call chain would become:
cmd_merge_recursive()
-> merge_recursive_generic()
-> merge_recursive()
-> merge_recursive()
-> merge_trees()
cmd_merge()
-> merge_recursive()
-> merge_recursive()
-> merge_trees()
cmd_revert(), cmd_am(), cmd_checkout(), cmd_stash(), ...
-> merge_trees()
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: What's cooking in git.git (Aug 2008, #05; Tue, 19)
2008-08-19 22:00 ` Junio C Hamano
@ 2008-08-20 22:42 ` Miklos Vajna
2008-08-25 1:44 ` [PATCH] merge-recursive: introduce merge_options Miklos Vajna
2008-09-01 1:06 ` What's cooking in git.git (Aug 2008, #05; Tue, 19) Miklos Vajna
2 siblings, 0 replies; 35+ messages in thread
From: Miklos Vajna @ 2008-08-20 22:42 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Stephan Beyer
[-- Attachment #1: Type: text/plain, Size: 686 bytes --]
On Tue, Aug 19, 2008 at 03:00:27PM -0700, Junio C Hamano <gitster@pobox.com> wrote:
> > Not exactly. builtin-merge-recursive uses get_ref() - which should not
> > be in merge-recursive.c IMHO - and get_ref() uses make_virtual_commit().
> > merge_recursive() itself takes commits, so it can be only static if we
> > copy it builtin-merge-recursive as well, causing a code duplication. Or
> > have I missed something here?
>
> I think you have.
>
> Let's look at the call chain from cmd_merge_recursive() and think again.
Thanks for the detailed answer. I just wanted to say that probably I
won't have time to implement this before the weekend; but I plan to do
so then.
[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]
^ permalink raw reply [flat|nested] 35+ messages in thread
* [PATCH] merge-recursive: introduce merge_options
2008-08-19 22:00 ` Junio C Hamano
2008-08-20 22:42 ` Miklos Vajna
@ 2008-08-25 1:44 ` Miklos Vajna
2008-08-25 6:06 ` Junio C Hamano
2008-09-01 1:06 ` What's cooking in git.git (Aug 2008, #05; Tue, 19) Miklos Vajna
2 siblings, 1 reply; 35+ messages in thread
From: Miklos Vajna @ 2008-08-25 1:44 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Stephan Beyer
This makes it possible to avoid passing the labels of branches as
arguments to merge_recursive(), merge_trees() and
merge_recursive_generic().
It also takes care of subtree merge, output buffering, verbosity, and
rename limits - these were global variables till now in
merge-recursive.c.
A new function, named init_merge_options(), is introduced as well, it
clears the struct merge_info, then initializes with default values,
finally updates the default values based on the config and environment
variables.
Signed-off-by: Miklos Vajna <vmiklos@frugalware.org>
---
On Tue, Aug 19, 2008 at 03:00:27PM -0700, Junio C Hamano <gitster@pobox.com> wrote:
> and the call chain would become:
>
> cmd_merge_recursive()
> -> merge_recursive_generic()
> -> merge_recursive()
> -> merge_recursive()
> -> merge_trees()
Actually I still left some extra function, as the merge_options
structure should be initialized, and that was done by just initializing
global variables before, but now we have to have a new function for it.
Other than that, I hope the patch looks like the way you imagined. ;-)
Notes:
1) This applies on top of 1c868d4 (merge-recursive.c: Add more generic
merge_recursive_generic()). I can rebase this (along with 1c868d4 and
1c868d4^) on top of current master, if this is a problem.
2) I know that this patch is huge, but we want to have the verbosity
flag in merge_options, so it has to be passed as an argument in many
places.
> cmd_merge()
> -> merge_recursive()
> -> merge_recursive()
> -> merge_trees()
>
> cmd_revert(), cmd_am(), cmd_checkout(), cmd_stash(), ...
> -> merge_trees()
builtin-checkout already used merge_trees(), so I modified it to use
merge_options, otherwise the patch would break the build. 'am' and
'stash' is not (yet) a builtin, so that is not interesting here. If this
patch looks OK, then I want to do the builtin-merge and builtin-revert
parts as well.
builtin-checkout.c | 11 ++-
builtin-merge-recursive.c | 43 ++++----
merge-recursive.c | 242 ++++++++++++++++++++++----------------------
merge-recursive.h | 42 ++++++---
4 files changed, 180 insertions(+), 158 deletions(-)
diff --git a/builtin-checkout.c b/builtin-checkout.c
index 411cc51..3627996 100644
--- a/builtin-checkout.c
+++ b/builtin-checkout.c
@@ -264,6 +264,7 @@ static int merge_working_tree(struct checkout_opts *opts,
*/
struct tree *result;
struct tree *work;
+ struct merge_options o;
if (!opts->merge)
return 1;
parse_commit(old->commit);
@@ -282,13 +283,17 @@ static int merge_working_tree(struct checkout_opts *opts,
*/
add_files_to_cache(NULL, NULL, 0);
- work = write_tree_from_memory();
+ init_merge_options(&o);
+ o.verbosity = 0;
+ work = write_tree_from_memory(&o);
ret = reset_tree(new->commit->tree, opts, 1);
if (ret)
return ret;
- merge_trees(new->commit->tree, work, old->commit->tree,
- new->name, "local", &result);
+ o.branch1 = new->name;
+ o.branch2 = "local";
+ merge_trees(&o, new->commit->tree, work,
+ old->commit->tree, &result);
ret = reset_tree(new->commit->tree, opts, 0);
if (ret)
return ret;
diff --git a/builtin-merge-recursive.c b/builtin-merge-recursive.c
index 25f540b..6b534c1 100644
--- a/builtin-merge-recursive.c
+++ b/builtin-merge-recursive.c
@@ -17,32 +17,33 @@ static const char *better_branch_name(const char *branch)
int cmd_merge_recursive(int argc, const char **argv, const char *prefix)
{
- const char *bases[21];
+ const unsigned char *bases[21];
unsigned bases_count = 0;
int i, failed;
- const char *branch1, *branch2;
unsigned char h1[20], h2[20];
- int subtree_merge = 0;
+ struct merge_options o;
+ struct commit *result;
+ init_merge_options(&o);
if (argv[0]) {
int namelen = strlen(argv[0]);
if (8 < namelen &&
!strcmp(argv[0] + namelen - 8, "-subtree"))
- subtree_merge = 1;
+ o.subtree_merge = 1;
}
- git_config(merge_recursive_config, NULL);
- merge_recursive_setup(subtree_merge);
if (argc < 4)
die("Usage: %s <base>... -- <head> <remote> ...\n", argv[0]);
for (i = 1; i < argc; ++i) {
- if (!strcmp(argv[i], "--")) {
- bases[bases_count] = NULL;
+ if (!strcmp(argv[i], "--"))
break;
+ if (bases_count < ARRAY_SIZE(bases)-1) {
+ unsigned char *sha = xmalloc(20);
+ if (get_sha1(argv[i], sha))
+ die("Could not parse object '%s'", argv[i]);
+ bases[bases_count++] = sha;
}
- if (bases_count < ARRAY_SIZE(bases)-1)
- bases[bases_count++] = argv[i];
else
warning("Cannot handle more than %zu bases. "
"Ignoring %s.", ARRAY_SIZE(bases)-1, argv[i]);
@@ -50,21 +51,21 @@ int cmd_merge_recursive(int argc, const char **argv, const char *prefix)
if (argc - i != 3) /* "--" "<head>" "<remote>" */
die("Not handling anything other than two heads merge.");
- branch1 = argv[++i];
- branch2 = argv[++i];
+ o.branch1 = argv[++i];
+ o.branch2 = argv[++i];
- if (get_sha1(branch1, h1))
- die("Could not resolve ref '%s'", branch1);
- if (get_sha1(branch2, h2))
- die("Could not resolve ref '%s'", branch2);
+ if (get_sha1(o.branch1, h1))
+ die("Could not resolve ref '%s'", o.branch1);
+ if (get_sha1(o.branch2, h2))
+ die("Could not resolve ref '%s'", o.branch2);
- branch1 = better_branch_name(branch1);
- branch2 = better_branch_name(branch2);
+ o.branch1 = better_branch_name(o.branch1);
+ o.branch2 = better_branch_name(o.branch2);
- if (merge_recursive_verbosity >= 3)
- printf("Merging %s with %s\n", branch1, branch2);
+ if (o.verbosity >= 3)
+ printf("Merging %s with %s\n", o.branch1, o.branch2);
- failed = merge_recursive_generic(bases, h1, branch1, h2, branch2);
+ failed = merge_recursive_generic(&o, h1, h2, bases_count, bases, &result);
if (failed < 0)
return 128; /* die() error code */
return failed;
diff --git a/merge-recursive.c b/merge-recursive.c
index 74a9fdc..ee23396 100644
--- a/merge-recursive.c
+++ b/merge-recursive.c
@@ -83,16 +83,11 @@ static struct string_list current_file_set = {NULL, 0, 0, 1};
static struct string_list current_directory_set = {NULL, 0, 0, 1};
static int call_depth = 0;
-int merge_recursive_verbosity = 2;
-static int diff_rename_limit = -1;
-static int merge_rename_limit = -1;
-static int buffer_output = 1;
static struct strbuf obuf = STRBUF_INIT;
-static int show(int v)
+static int show(int v, struct merge_options *o)
{
- return (!call_depth && merge_recursive_verbosity >= v) ||
- merge_recursive_verbosity >= 5;
+ return (!call_depth && o->verbosity >= v) || o->verbosity >= 5;
}
static void flush_output(void)
@@ -103,12 +98,12 @@ static void flush_output(void)
}
}
-static void output(int v, const char *fmt, ...)
+static void output(int v, struct merge_options *o, const char *fmt, ...)
{
int len;
va_list ap;
- if (!show(v))
+ if (!show(v, o))
return;
strbuf_grow(&obuf, call_depth * 2 + 2);
@@ -132,7 +127,7 @@ static void output(int v, const char *fmt, ...)
}
strbuf_setlen(&obuf, obuf.len + len);
strbuf_add(&obuf, "\n", 1);
- if (!buffer_output)
+ if (!o->buffer_output)
flush_output();
}
@@ -219,17 +214,17 @@ static int git_merge_trees(int index_only,
return rc;
}
-struct tree *write_tree_from_memory(void)
+struct tree *write_tree_from_memory(struct merge_options *o)
{
struct tree *result = NULL;
if (unmerged_cache()) {
int i;
- output(0, "There are unmerged index entries:");
+ output(0, o, "There are unmerged index entries:");
for (i = 0; i < active_nr; i++) {
struct cache_entry *ce = active_cache[i];
if (ce_stage(ce))
- output(0, "%d %.*s", ce_stage(ce), ce_namelen(ce), ce->name);
+ output(0, o, "%d %.*s", ce_stage(ce), ce_namelen(ce), ce->name);
}
return NULL;
}
@@ -345,7 +340,8 @@ static struct string_list *get_renames(struct tree *tree,
struct tree *o_tree,
struct tree *a_tree,
struct tree *b_tree,
- struct string_list *entries)
+ struct string_list *entries,
+ struct merge_options *o)
{
int i;
struct string_list *renames;
@@ -355,8 +351,8 @@ static struct string_list *get_renames(struct tree *tree,
diff_setup(&opts);
DIFF_OPT_SET(&opts, RECURSIVE);
opts.detect_rename = DIFF_DETECT_RENAME;
- opts.rename_limit = merge_rename_limit >= 0 ? merge_rename_limit :
- diff_rename_limit >= 0 ? diff_rename_limit :
+ opts.rename_limit = o->merge_rename_limit >= 0 ? o->merge_rename_limit :
+ o->diff_rename_limit >= 0 ? o->diff_rename_limit :
500;
opts.warn_on_too_large_rename = 1;
opts.output_format = DIFF_FORMAT_NO_OUTPUT;
@@ -720,7 +716,8 @@ static struct merge_file_info merge_file(struct diff_filespec *o,
static void conflict_rename_rename(struct rename *ren1,
const char *branch1,
struct rename *ren2,
- const char *branch2)
+ const char *branch2,
+ struct merge_options *o)
{
char *del[2];
int delp = 0;
@@ -730,13 +727,13 @@ static void conflict_rename_rename(struct rename *ren1,
const char *dst_name2 = ren2_dst;
if (string_list_has_string(¤t_directory_set, ren1_dst)) {
dst_name1 = del[delp++] = unique_path(ren1_dst, branch1);
- output(1, "%s is a directory in %s added as %s instead",
+ output(1, o, "%s is a directory in %s added as %s instead",
ren1_dst, branch2, dst_name1);
remove_file(0, ren1_dst, 0);
}
if (string_list_has_string(¤t_directory_set, ren2_dst)) {
dst_name2 = del[delp++] = unique_path(ren2_dst, branch2);
- output(1, "%s is a directory in %s added as %s instead",
+ output(1, o, "%s is a directory in %s added as %s instead",
ren2_dst, branch1, dst_name2);
remove_file(0, ren2_dst, 0);
}
@@ -758,10 +755,11 @@ static void conflict_rename_rename(struct rename *ren1,
}
static void conflict_rename_dir(struct rename *ren1,
- const char *branch1)
+ const char *branch1,
+ struct merge_options *o)
{
char *new_path = unique_path(ren1->pair->two->path, branch1);
- output(1, "Renamed %s to %s instead", ren1->pair->one->path, new_path);
+ output(1, o, "Renamed %s to %s instead", ren1->pair->one->path, new_path);
remove_file(0, ren1->pair->two->path, 0);
update_file(0, ren1->pair->two->sha1, ren1->pair->two->mode, new_path);
free(new_path);
@@ -770,11 +768,12 @@ static void conflict_rename_dir(struct rename *ren1,
static void conflict_rename_rename_2(struct rename *ren1,
const char *branch1,
struct rename *ren2,
- const char *branch2)
+ const char *branch2,
+ struct merge_options *o)
{
char *new_path1 = unique_path(ren1->pair->two->path, branch1);
char *new_path2 = unique_path(ren2->pair->two->path, branch2);
- output(1, "Renamed %s to %s and %s to %s instead",
+ output(1, o, "Renamed %s to %s and %s to %s instead",
ren1->pair->one->path, new_path1,
ren2->pair->one->path, new_path2);
remove_file(0, ren1->pair->two->path, 0);
@@ -786,8 +785,7 @@ static void conflict_rename_rename_2(struct rename *ren1,
static int process_renames(struct string_list *a_renames,
struct string_list *b_renames,
- const char *a_branch,
- const char *b_branch)
+ struct merge_options *o)
{
int clean_merge = 1, i, j;
struct string_list a_by_dst = {NULL, 0, 0, 0}, b_by_dst = {NULL, 0, 0, 0};
@@ -832,15 +830,15 @@ static int process_renames(struct string_list *a_renames,
renames1 = a_renames;
renames2 = b_renames;
renames2Dst = &b_by_dst;
- branch1 = a_branch;
- branch2 = b_branch;
+ branch1 = o->branch1;
+ branch2 = o->branch2;
} else {
struct rename *tmp;
renames1 = b_renames;
renames2 = a_renames;
renames2Dst = &a_by_dst;
- branch1 = b_branch;
- branch2 = a_branch;
+ branch1 = o->branch2;
+ branch2 = o->branch1;
tmp = ren2;
ren2 = ren1;
ren1 = tmp;
@@ -867,7 +865,7 @@ static int process_renames(struct string_list *a_renames,
ren2->processed = 1;
if (strcmp(ren1_dst, ren2_dst) != 0) {
clean_merge = 0;
- output(1, "CONFLICT (rename/rename): "
+ output(1, o, "CONFLICT (rename/rename): "
"Rename \"%s\"->\"%s\" in branch \"%s\" "
"rename \"%s\"->\"%s\" in \"%s\"%s",
src, ren1_dst, branch1,
@@ -878,7 +876,7 @@ static int process_renames(struct string_list *a_renames,
update_file(0, ren1->pair->one->sha1,
ren1->pair->one->mode, src);
}
- conflict_rename_rename(ren1, branch1, ren2, branch2);
+ conflict_rename_rename(ren1, branch1, ren2, branch2, o);
} else {
struct merge_file_info mfi;
remove_file(1, ren1_src, 1);
@@ -888,13 +886,13 @@ static int process_renames(struct string_list *a_renames,
branch1,
branch2);
if (mfi.merge || !mfi.clean)
- output(1, "Renamed %s->%s", src, ren1_dst);
+ output(1, o, "Renamed %s->%s", src, ren1_dst);
if (mfi.merge)
- output(2, "Auto-merged %s", ren1_dst);
+ output(2, o, "Auto-merged %s", ren1_dst);
if (!mfi.clean) {
- output(1, "CONFLICT (content): merge conflict in %s",
+ output(1, o, "CONFLICT (content): merge conflict in %s",
ren1_dst);
clean_merge = 0;
@@ -925,14 +923,14 @@ static int process_renames(struct string_list *a_renames,
if (string_list_has_string(¤t_directory_set, ren1_dst)) {
clean_merge = 0;
- output(1, "CONFLICT (rename/directory): Renamed %s->%s in %s "
+ output(1, o, "CONFLICT (rename/directory): Renamed %s->%s in %s "
" directory %s added in %s",
ren1_src, ren1_dst, branch1,
ren1_dst, branch2);
- conflict_rename_dir(ren1, branch1);
+ conflict_rename_dir(ren1, branch1, o);
} else if (sha_eq(src_other.sha1, null_sha1)) {
clean_merge = 0;
- output(1, "CONFLICT (rename/delete): Renamed %s->%s in %s "
+ output(1, o, "CONFLICT (rename/delete): Renamed %s->%s in %s "
"and deleted in %s",
ren1_src, ren1_dst, branch1,
branch2);
@@ -941,31 +939,31 @@ static int process_renames(struct string_list *a_renames,
const char *new_path;
clean_merge = 0;
try_merge = 1;
- output(1, "CONFLICT (rename/add): Renamed %s->%s in %s. "
+ output(1, o, "CONFLICT (rename/add): Renamed %s->%s in %s. "
"%s added in %s",
ren1_src, ren1_dst, branch1,
ren1_dst, branch2);
new_path = unique_path(ren1_dst, branch2);
- output(1, "Added as %s instead", new_path);
+ output(1, o, "Added as %s instead", new_path);
update_file(0, dst_other.sha1, dst_other.mode, new_path);
} else if ((item = string_list_lookup(ren1_dst, renames2Dst))) {
ren2 = item->util;
clean_merge = 0;
ren2->processed = 1;
- output(1, "CONFLICT (rename/rename): Renamed %s->%s in %s. "
+ output(1, o, "CONFLICT (rename/rename): Renamed %s->%s in %s. "
"Renamed %s->%s in %s",
ren1_src, ren1_dst, branch1,
ren2->pair->one->path, ren2->pair->two->path, branch2);
- conflict_rename_rename_2(ren1, branch1, ren2, branch2);
+ conflict_rename_rename_2(ren1, branch1, ren2, branch2, o);
} else
try_merge = 1;
if (try_merge) {
- struct diff_filespec *o, *a, *b;
+ struct diff_filespec *one, *a, *b;
struct merge_file_info mfi;
src_other.path = (char *)ren1_src;
- o = ren1->pair->one;
+ one = ren1->pair->one;
if (a_renames == renames1) {
a = ren1->pair->two;
b = &src_other;
@@ -973,8 +971,8 @@ static int process_renames(struct string_list *a_renames,
b = ren1->pair->two;
a = &src_other;
}
- mfi = merge_file(o, a, b,
- a_branch, b_branch);
+ mfi = merge_file(one, a, b,
+ o->branch1, o->branch2);
if (mfi.clean &&
sha_eq(mfi.sha, ren1->pair->two->sha1) &&
@@ -984,20 +982,20 @@ static int process_renames(struct string_list *a_renames,
* t6022 test. If you change
* it update the test too.
*/
- output(3, "Skipped %s (merged same as existing)", ren1_dst);
+ output(3, o, "Skipped %s (merged same as existing)", ren1_dst);
else {
if (mfi.merge || !mfi.clean)
- output(1, "Renamed %s => %s", ren1_src, ren1_dst);
+ output(1, o, "Renamed %s => %s", ren1_src, ren1_dst);
if (mfi.merge)
- output(2, "Auto-merged %s", ren1_dst);
+ output(2, o, "Auto-merged %s", ren1_dst);
if (!mfi.clean) {
- output(1, "CONFLICT (rename/modify): Merge conflict in %s",
+ output(1, o, "CONFLICT (rename/modify): Merge conflict in %s",
ren1_dst);
clean_merge = 0;
if (!index_only)
update_stages(ren1_dst,
- o, a, b, 1);
+ one, a, b, 1);
}
update_file(mfi.clean, mfi.sha, mfi.mode, ren1_dst);
}
@@ -1017,8 +1015,7 @@ static unsigned char *stage_sha(const unsigned char *sha, unsigned mode)
/* Per entry merge function */
static int process_entry(const char *path, struct stage_data *entry,
- const char *branch1,
- const char *branch2)
+ struct merge_options *o)
{
/*
printf("processing entry, clean cache: %s\n", index_only ? "yes": "no");
@@ -1040,23 +1037,23 @@ static int process_entry(const char *path, struct stage_data *entry,
/* Deleted in both or deleted in one and
* unchanged in the other */
if (a_sha)
- output(2, "Removed %s", path);
+ output(2, o, "Removed %s", path);
/* do not touch working file if it did not exist */
remove_file(1, path, !a_sha);
} else {
/* Deleted in one and changed in the other */
clean_merge = 0;
if (!a_sha) {
- output(1, "CONFLICT (delete/modify): %s deleted in %s "
+ output(1, o, "CONFLICT (delete/modify): %s deleted in %s "
"and modified in %s. Version %s of %s left in tree.",
- path, branch1,
- branch2, branch2, path);
+ path, o->branch1,
+ o->branch2, o->branch2, path);
update_file(0, b_sha, b_mode, path);
} else {
- output(1, "CONFLICT (delete/modify): %s deleted in %s "
+ output(1, o, "CONFLICT (delete/modify): %s deleted in %s "
"and modified in %s. Version %s of %s left in tree.",
- path, branch2,
- branch1, branch1, path);
+ path, o->branch2,
+ o->branch1, o->branch1, path);
update_file(0, a_sha, a_mode, path);
}
}
@@ -1071,14 +1068,14 @@ static int process_entry(const char *path, struct stage_data *entry,
const char *conf;
if (a_sha) {
- add_branch = branch1;
- other_branch = branch2;
+ add_branch = o->branch1;
+ other_branch = o->branch2;
mode = a_mode;
sha = a_sha;
conf = "file/directory";
} else {
- add_branch = branch2;
- other_branch = branch1;
+ add_branch = o->branch2;
+ other_branch = o->branch1;
mode = b_mode;
sha = b_sha;
conf = "directory/file";
@@ -1086,13 +1083,13 @@ static int process_entry(const char *path, struct stage_data *entry,
if (string_list_has_string(¤t_directory_set, path)) {
const char *new_path = unique_path(path, add_branch);
clean_merge = 0;
- output(1, "CONFLICT (%s): There is a directory with name %s in %s. "
+ output(1, o, "CONFLICT (%s): There is a directory with name %s in %s. "
"Added %s as %s",
conf, path, other_branch, path, new_path);
remove_file(0, path, 0);
update_file(0, sha, mode, new_path);
} else {
- output(2, "Added %s", path);
+ output(2, o, "Added %s", path);
update_file(1, sha, mode, path);
}
} else if (a_sha && b_sha) {
@@ -1100,32 +1097,32 @@ static int process_entry(const char *path, struct stage_data *entry,
/* case D: Modified in both, but differently. */
const char *reason = "content";
struct merge_file_info mfi;
- struct diff_filespec o, a, b;
+ struct diff_filespec one, a, b;
if (!o_sha) {
reason = "add/add";
o_sha = (unsigned char *)null_sha1;
}
- output(2, "Auto-merged %s", path);
- o.path = a.path = b.path = (char *)path;
- hashcpy(o.sha1, o_sha);
- o.mode = o_mode;
+ output(2, o, "Auto-merged %s", path);
+ one.path = a.path = b.path = (char *)path;
+ hashcpy(one.sha1, o_sha);
+ one.mode = o_mode;
hashcpy(a.sha1, a_sha);
a.mode = a_mode;
hashcpy(b.sha1, b_sha);
b.mode = b_mode;
- mfi = merge_file(&o, &a, &b,
- branch1, branch2);
+ mfi = merge_file(&one, &a, &b,
+ o->branch1, o->branch2);
clean_merge = mfi.clean;
if (mfi.clean)
update_file(1, mfi.sha, mfi.mode, path);
else if (S_ISGITLINK(mfi.mode))
- output(1, "CONFLICT (submodule): Merge conflict in %s "
+ output(1, o, "CONFLICT (submodule): Merge conflict in %s "
"- needs %s", path, sha1_to_hex(b.sha1));
else {
- output(1, "CONFLICT (%s): Merge conflict in %s",
+ output(1, o, "CONFLICT (%s): Merge conflict in %s",
reason, path);
if (index_only)
@@ -1146,11 +1143,10 @@ static int process_entry(const char *path, struct stage_data *entry,
return clean_merge;
}
-int merge_trees(struct tree *head,
+int merge_trees(struct merge_options *o,
+ struct tree *head,
struct tree *merge,
struct tree *common,
- const char *branch1,
- const char *branch2,
struct tree **result)
{
int code, clean;
@@ -1161,7 +1157,7 @@ int merge_trees(struct tree *head,
}
if (sha_eq(common->object.sha1, merge->object.sha1)) {
- output(0, "Already uptodate!");
+ output(0, o, "Already uptodate!");
*result = head;
return 1;
}
@@ -1182,15 +1178,14 @@ int merge_trees(struct tree *head,
get_files_dirs(merge);
entries = get_unmerged();
- re_head = get_renames(head, common, head, merge, entries);
- re_merge = get_renames(merge, common, head, merge, entries);
- clean = process_renames(re_head, re_merge,
- branch1, branch2);
+ re_head = get_renames(head, common, head, merge, entries, o);
+ re_merge = get_renames(merge, common, head, merge, entries, o);
+ clean = process_renames(re_head, re_merge, o);
for (i = 0; i < entries->nr; i++) {
const char *path = entries->items[i].string;
struct stage_data *e = entries->items[i].util;
if (!e->processed
- && !process_entry(path, e, branch1, branch2))
+ && !process_entry(path, e, o))
clean = 0;
}
@@ -1203,7 +1198,7 @@ int merge_trees(struct tree *head,
clean = 1;
if (index_only)
- *result = write_tree_from_memory();
+ *result = write_tree_from_memory(o);
return clean;
}
@@ -1223,10 +1218,9 @@ static struct commit_list *reverse_commit_list(struct commit_list *list)
* Merge the commits h1 and h2, return the resulting virtual
* commit object and a flag indicating the cleanness of the merge.
*/
-int merge_recursive(struct commit *h1,
+int merge_recursive(struct merge_options *o,
+ struct commit *h1,
struct commit *h2,
- const char *branch1,
- const char *branch2,
struct commit_list *ca,
struct commit **result)
{
@@ -1235,8 +1229,8 @@ int merge_recursive(struct commit *h1,
struct tree *mrtree = mrtree;
int clean;
- if (show(4)) {
- output(4, "Merging:");
+ if (show(4, o)) {
+ output(4, o, "Merging:");
output_commit_title(h1);
output_commit_title(h2);
}
@@ -1246,8 +1240,8 @@ int merge_recursive(struct commit *h1,
ca = reverse_commit_list(ca);
}
- if (show(5)) {
- output(5, "found %u common ancestor(s):", commit_list_count(ca));
+ if (show(5, o)) {
+ output(5, o, "found %u common ancestor(s):", commit_list_count(ca));
for (iter = ca; iter; iter = iter->next)
output_commit_title(iter->item);
}
@@ -1264,6 +1258,7 @@ int merge_recursive(struct commit *h1,
}
for (iter = ca; iter; iter = iter->next) {
+ struct merge_options opts;
call_depth++;
/*
* When the merge fails, the result contains files
@@ -1273,10 +1268,11 @@ int merge_recursive(struct commit *h1,
* "conflicts" were already resolved.
*/
discard_cache();
- merge_recursive(merged_common_ancestors, iter->item,
- "Temporary merge branch 1",
- "Temporary merge branch 2",
- NULL,
+ memcpy(&opts, o, sizeof(struct merge_options));
+ opts.branch1 = "Temporary merge branch 1";
+ opts.branch2 = "Temporary merge branch 2";
+ merge_recursive(&opts, merged_common_ancestors,
+ iter->item, NULL,
&merged_common_ancestors);
call_depth--;
@@ -1291,8 +1287,8 @@ int merge_recursive(struct commit *h1,
} else
index_only = 1;
- clean = merge_trees(h1->tree, h2->tree, merged_common_ancestors->tree,
- branch1, branch2, &mrtree);
+ clean = merge_trees(o, h1->tree, h2->tree, merged_common_ancestors->tree,
+ &mrtree);
if (index_only) {
*result = make_virtual_commit(mrtree, "merged tree");
@@ -1319,35 +1315,33 @@ static struct commit *get_ref(const unsigned char *sha1, const char *name)
return (struct commit *)object;
}
-int merge_recursive_generic(const char **base_list,
- const unsigned char *head_sha1, const char *head_name,
- const unsigned char *next_sha1, const char *next_name)
+int merge_recursive_generic(struct merge_options *o,
+ const unsigned char *head,
+ const unsigned char *merge,
+ int num_base_list,
+ const unsigned char **base_list,
+ struct commit **result)
{
int clean, index_fd;
struct lock_file *lock = xcalloc(1, sizeof(struct lock_file));
- struct commit *result;
- struct commit *head_commit = get_ref(head_sha1, head_name);
- struct commit *next_commit = get_ref(next_sha1, next_name);
+ struct commit *head_commit = get_ref(head, o->branch1);
+ struct commit *next_commit = get_ref(merge, o->branch2);
struct commit_list *ca = NULL;
if (base_list) {
int i;
- for (i = 0; base_list[i]; ++i) {
- unsigned char sha[20];
+ for (i = 0; i < num_base_list; ++i) {
struct commit *base;
- if (get_sha1(base_list[i], sha))
- return error("Could not resolve ref '%s'",
- base_list[i]);
- if (!(base = get_ref(sha, base_list[i])))
+ if (!(base = get_ref(base_list[i], sha1_to_hex(base_list[i]))))
return error("Could not parse object '%s'",
- base_list[i]);
+ sha1_to_hex(base_list[i]));
commit_list_insert(base, &ca);
}
}
index_fd = hold_locked_index(lock, 1);
- clean = merge_recursive(head_commit, next_commit,
- head_name, next_name, ca, &result);
+ clean = merge_recursive(o, head_commit, next_commit, ca,
+ result);
if (active_cache_changed &&
(write_cache(index_fd, active_cache, active_nr) ||
commit_locked_index(lock)))
@@ -1356,29 +1350,35 @@ int merge_recursive_generic(const char **base_list,
return clean ? 0 : 1;
}
-int merge_recursive_config(const char *var, const char *value, void *cb)
+static int merge_recursive_config(const char *var, const char *value, void *cb)
{
+ struct merge_options *o = cb;
if (!strcasecmp(var, "merge.verbosity")) {
- merge_recursive_verbosity = git_config_int(var, value);
+ o->verbosity = git_config_int(var, value);
return 0;
}
if (!strcasecmp(var, "diff.renamelimit")) {
- diff_rename_limit = git_config_int(var, value);
+ o->diff_rename_limit = git_config_int(var, value);
return 0;
}
if (!strcasecmp(var, "merge.renamelimit")) {
- merge_rename_limit = git_config_int(var, value);
+ o->merge_rename_limit = git_config_int(var, value);
return 0;
}
return git_default_config(var, value, cb);
}
-void merge_recursive_setup(int is_subtree_merge)
+void init_merge_options(struct merge_options *o)
{
+ memset(o, 0, sizeof(struct merge_options));
+ o->verbosity = 2;
+ o->buffer_output = 1;
+ o->diff_rename_limit = -1;
+ o->merge_rename_limit = -1;
+ git_config(merge_recursive_config, o);
if (getenv("GIT_MERGE_VERBOSITY"))
- merge_recursive_verbosity =
+ o->verbosity =
strtol(getenv("GIT_MERGE_VERBOSITY"), NULL, 10);
- if (merge_recursive_verbosity >= 5)
- buffer_output = 0;
- subtree_merge = is_subtree_merge;
+ if (o->verbosity >= 5)
+ o->buffer_output = 0;
}
diff --git a/merge-recursive.h b/merge-recursive.h
index 4dd6476..72f0a28 100644
--- a/merge-recursive.h
+++ b/merge-recursive.h
@@ -1,26 +1,42 @@
#ifndef MERGE_RECURSIVE_H
#define MERGE_RECURSIVE_H
-int merge_recursive(struct commit *h1,
+struct merge_options {
+ const char *branch1;
+ const char *branch2;
+ unsigned subtree_merge : 1;
+ unsigned buffer_output : 1;
+ int verbosity;
+ int diff_rename_limit;
+ int merge_rename_limit;
+};
+
+/* merge_trees() but with recursive ancestor consolidation */
+int merge_recursive(struct merge_options *o,
+ struct commit *h1,
struct commit *h2,
- const char *branch1,
- const char *branch2,
struct commit_list *ancestors,
struct commit **result);
-int merge_trees(struct tree *head,
+/* rename-detecting three-way merge, no recursion */
+int merge_trees(struct merge_options *o,
+ struct tree *head,
struct tree *merge,
struct tree *common,
- const char *branch1,
- const char *branch2,
struct tree **result);
-extern int merge_recursive_generic(const char **base_list,
- const unsigned char *head_sha1, const char *head_name,
- const unsigned char *next_sha1, const char *next_name);
-int merge_recursive_config(const char *var, const char *value, void *cb);
-void merge_recursive_setup(int is_subtree_merge);
-struct tree *write_tree_from_memory(void);
-extern int merge_recursive_verbosity;
+/*
+ * "git-merge-recursive" can be fed trees; wrap them into
+ * virtual commits and call merge_recursive() proper.
+ */
+int merge_recursive_generic(struct merge_options *o,
+ const unsigned char *head,
+ const unsigned char *merge,
+ int num_ca,
+ const unsigned char **ca,
+ struct commit **result);
+
+void init_merge_options(struct merge_options *o);
+struct tree *write_tree_from_memory(struct merge_options *o);
#endif
--
1.6.0.rc3.17.gc14c8.dirty
^ permalink raw reply related [flat|nested] 35+ messages in thread
* Re: [PATCH] merge-recursive: introduce merge_options
2008-08-25 1:44 ` [PATCH] merge-recursive: introduce merge_options Miklos Vajna
@ 2008-08-25 6:06 ` Junio C Hamano
2008-08-25 14:25 ` Miklos Vajna
0 siblings, 1 reply; 35+ messages in thread
From: Junio C Hamano @ 2008-08-25 6:06 UTC (permalink / raw)
To: Miklos Vajna; +Cc: git, Stephan Beyer
Miklos Vajna <vmiklos@frugalware.org> writes:
> 1) This applies on top of 1c868d4 (merge-recursive.c: Add more generic
> merge_recursive_generic()). I can rebase this (along with 1c868d4 and
> 1c868d4^) on top of current master, if this is a problem.
It probably is cleaner to treat this as a fresh topic from scratch on top
of 'master', as we do not have anything outstanding in 'next' around this
area.
> 2) I know that this patch is huge, but we want to have the verbosity
> flag in merge_options, so it has to be passed as an argument in many
> places.
Size of the patch that results purely from addition of the merge_options
parameter from top to bottom does not bother me too much. The look quite
straightforward conversions, and getting rid of these many global
variables is a major step in the right direction.
It might however be a good idea to consistently have this at the same
place (either the beginning or at the end) of the parameter list of
functions that take one.
> @@ -1273,10 +1268,11 @@ int merge_recursive(struct commit *h1,
> * "conflicts" were already resolved.
> */
> discard_cache();
> - merge_recursive(merged_common_ancestors, iter->item,
> - "Temporary merge branch 1",
> - "Temporary merge branch 2",
> - NULL,
> + memcpy(&opts, o, sizeof(struct merge_options));
> + opts.branch1 = "Temporary merge branch 1";
> + opts.branch2 = "Temporary merge branch 2";
> + merge_recursive(&opts, merged_common_ancestors,
> + iter->item, NULL,
> &merged_common_ancestors);
> call_depth--;
After suggesting to keep label in merge_options, I was wondering how this
part should be handled the best. An alternative would be not to do copy
the structure but stash away only branch1 and branch2 members before
making the recursive call and restore them after it returns, like this:
const char *saved_b1, *saved_b2;
...
saved_b1 = o->branch1;
saved_b2 = o->branch2;
o->branch1 = "Temporary merge branch 1";
o->branch2 = "Temporary merge branch 2";
merge_recursive(o, ...);
o->branch1 = saved_b1;
o->branch2 = saved_b2;
call_depth--;
...
This might be better in the longer run, as we may want to pass *back*
status from merge_recursive() to the caller in fields of merge_options in
the future.
^ permalink raw reply [flat|nested] 35+ messages in thread
* [PATCH] merge-recursive: introduce merge_options
2008-08-25 6:06 ` Junio C Hamano
@ 2008-08-25 14:25 ` Miklos Vajna
2008-08-28 4:50 ` Junio C Hamano
2008-08-30 15:42 ` [PATCH] merge-recursive: fix subtree merge Miklos Vajna
0 siblings, 2 replies; 35+ messages in thread
From: Miklos Vajna @ 2008-08-25 14:25 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Stephan Beyer
This makes it possible to avoid passing the labels of branches as
arguments to merge_recursive(), merge_trees() and
merge_recursive_generic().
It also takes care of subtree merge, output buffering, verbosity, and
rename limits - these were global variables till now in
merge-recursive.c.
A new function, named init_merge_options(), is introduced as well, it
clears the struct merge_info, then initializes with default values,
finally updates the default values based on the config and environment
variables.
Signed-off-by: Miklos Vajna <vmiklos@frugalware.org>
---
On Sun, Aug 24, 2008 at 11:06:06PM -0700, Junio C Hamano <gitster@pobox.com> wrote:
> Miklos Vajna <vmiklos@frugalware.org> writes:
>
> > 1) This applies on top of 1c868d4 (merge-recursive.c: Add more
> > generic
> > merge_recursive_generic()). I can rebase this (along with 1c868d4
> > and
> > 1c868d4^) on top of current master, if this is a problem.
>
> It probably is cleaner to treat this as a fresh topic from scratch on
> top
> of 'master', as we do not have anything outstanding in 'next' around
> this
> area.
I'm now confused about what should I do:
1) Nothing. (That's what I did for now.)
2) Rebase against master and resend.
3) Rebase, squash and resend.
> > 2) I know that this patch is huge, but we want to have the verbosity
> > flag in merge_options, so it has to be passed as an argument in many
> > places.
>
> Size of the patch that results purely from addition of the
> merge_options
> parameter from top to bottom does not bother me too much. The look
> quite
> straightforward conversions, and getting rid of these many global
> variables is a major step in the right direction.
>
> It might however be a good idea to consistently have this at the same
> place (either the beginning or at the end) of the parameter list of
> functions that take one.
OK, I'll move them to the beginning, since that's what we have for the
public functions already.
> > @@ -1273,10 +1268,11 @@ int merge_recursive(struct commit *h1,
> > * "conflicts" were already resolved.
> > */
> > discard_cache();
> > - merge_recursive(merged_common_ancestors, iter->item,
> > - "Temporary merge branch 1",
> > - "Temporary merge branch 2",
> > - NULL,
> > + memcpy(&opts, o, sizeof(struct merge_options));
> > + opts.branch1 = "Temporary merge branch 1";
> > + opts.branch2 = "Temporary merge branch 2";
> > + merge_recursive(&opts, merged_common_ancestors,
> > + iter->item, NULL,
> > &merged_common_ancestors);
> > call_depth--;
>
> After suggesting to keep label in merge_options, I was wondering how
> this
> part should be handled the best. An alternative would be not to do
> copy
> the structure but stash away only branch1 and branch2 members before
> making the recursive call and restore them after it returns, like
> this:
>
> const char *saved_b1, *saved_b2;
> ...
> saved_b1 = o->branch1;
> saved_b2 = o->branch2;
> o->branch1 = "Temporary merge branch 1";
> o->branch2 = "Temporary merge branch 2";
> merge_recursive(o, ...);
> o->branch1 = saved_b1;
> o->branch2 = saved_b2;
> call_depth--;
> ...
>
> This might be better in the longer run, as we may want to pass *back*
> status from merge_recursive() to the caller in fields of merge_options
> in
> the future.
True, changed.
Interdiff:
git fetch git://repo.or.cz/git/vmiklos.git
git diff 2ff6553..8faf3ac
builtin-checkout.c | 11 ++-
builtin-merge-recursive.c | 43 ++++----
merge-recursive.c | 260 +++++++++++++++++++++++----------------------
merge-recursive.h | 42 +++++---
4 files changed, 190 insertions(+), 166 deletions(-)
diff --git a/builtin-checkout.c b/builtin-checkout.c
index 411cc51..3627996 100644
--- a/builtin-checkout.c
+++ b/builtin-checkout.c
@@ -264,6 +264,7 @@ static int merge_working_tree(struct checkout_opts *opts,
*/
struct tree *result;
struct tree *work;
+ struct merge_options o;
if (!opts->merge)
return 1;
parse_commit(old->commit);
@@ -282,13 +283,17 @@ static int merge_working_tree(struct checkout_opts *opts,
*/
add_files_to_cache(NULL, NULL, 0);
- work = write_tree_from_memory();
+ init_merge_options(&o);
+ o.verbosity = 0;
+ work = write_tree_from_memory(&o);
ret = reset_tree(new->commit->tree, opts, 1);
if (ret)
return ret;
- merge_trees(new->commit->tree, work, old->commit->tree,
- new->name, "local", &result);
+ o.branch1 = new->name;
+ o.branch2 = "local";
+ merge_trees(&o, new->commit->tree, work,
+ old->commit->tree, &result);
ret = reset_tree(new->commit->tree, opts, 0);
if (ret)
return ret;
diff --git a/builtin-merge-recursive.c b/builtin-merge-recursive.c
index 25f540b..6b534c1 100644
--- a/builtin-merge-recursive.c
+++ b/builtin-merge-recursive.c
@@ -17,32 +17,33 @@ static const char *better_branch_name(const char *branch)
int cmd_merge_recursive(int argc, const char **argv, const char *prefix)
{
- const char *bases[21];
+ const unsigned char *bases[21];
unsigned bases_count = 0;
int i, failed;
- const char *branch1, *branch2;
unsigned char h1[20], h2[20];
- int subtree_merge = 0;
+ struct merge_options o;
+ struct commit *result;
+ init_merge_options(&o);
if (argv[0]) {
int namelen = strlen(argv[0]);
if (8 < namelen &&
!strcmp(argv[0] + namelen - 8, "-subtree"))
- subtree_merge = 1;
+ o.subtree_merge = 1;
}
- git_config(merge_recursive_config, NULL);
- merge_recursive_setup(subtree_merge);
if (argc < 4)
die("Usage: %s <base>... -- <head> <remote> ...\n", argv[0]);
for (i = 1; i < argc; ++i) {
- if (!strcmp(argv[i], "--")) {
- bases[bases_count] = NULL;
+ if (!strcmp(argv[i], "--"))
break;
+ if (bases_count < ARRAY_SIZE(bases)-1) {
+ unsigned char *sha = xmalloc(20);
+ if (get_sha1(argv[i], sha))
+ die("Could not parse object '%s'", argv[i]);
+ bases[bases_count++] = sha;
}
- if (bases_count < ARRAY_SIZE(bases)-1)
- bases[bases_count++] = argv[i];
else
warning("Cannot handle more than %zu bases. "
"Ignoring %s.", ARRAY_SIZE(bases)-1, argv[i]);
@@ -50,21 +51,21 @@ int cmd_merge_recursive(int argc, const char **argv, const char *prefix)
if (argc - i != 3) /* "--" "<head>" "<remote>" */
die("Not handling anything other than two heads merge.");
- branch1 = argv[++i];
- branch2 = argv[++i];
+ o.branch1 = argv[++i];
+ o.branch2 = argv[++i];
- if (get_sha1(branch1, h1))
- die("Could not resolve ref '%s'", branch1);
- if (get_sha1(branch2, h2))
- die("Could not resolve ref '%s'", branch2);
+ if (get_sha1(o.branch1, h1))
+ die("Could not resolve ref '%s'", o.branch1);
+ if (get_sha1(o.branch2, h2))
+ die("Could not resolve ref '%s'", o.branch2);
- branch1 = better_branch_name(branch1);
- branch2 = better_branch_name(branch2);
+ o.branch1 = better_branch_name(o.branch1);
+ o.branch2 = better_branch_name(o.branch2);
- if (merge_recursive_verbosity >= 3)
- printf("Merging %s with %s\n", branch1, branch2);
+ if (o.verbosity >= 3)
+ printf("Merging %s with %s\n", o.branch1, o.branch2);
- failed = merge_recursive_generic(bases, h1, branch1, h2, branch2);
+ failed = merge_recursive_generic(&o, h1, h2, bases_count, bases, &result);
if (failed < 0)
return 128; /* die() error code */
return failed;
diff --git a/merge-recursive.c b/merge-recursive.c
index 74a9fdc..5fab301 100644
--- a/merge-recursive.c
+++ b/merge-recursive.c
@@ -83,16 +83,11 @@ static struct string_list current_file_set = {NULL, 0, 0, 1};
static struct string_list current_directory_set = {NULL, 0, 0, 1};
static int call_depth = 0;
-int merge_recursive_verbosity = 2;
-static int diff_rename_limit = -1;
-static int merge_rename_limit = -1;
-static int buffer_output = 1;
static struct strbuf obuf = STRBUF_INIT;
-static int show(int v)
+static int show(struct merge_options *o, int v)
{
- return (!call_depth && merge_recursive_verbosity >= v) ||
- merge_recursive_verbosity >= 5;
+ return (!call_depth && o->verbosity >= v) || o->verbosity >= 5;
}
static void flush_output(void)
@@ -103,12 +98,12 @@ static void flush_output(void)
}
}
-static void output(int v, const char *fmt, ...)
+static void output(struct merge_options *o, int v, const char *fmt, ...)
{
int len;
va_list ap;
- if (!show(v))
+ if (!show(o, v))
return;
strbuf_grow(&obuf, call_depth * 2 + 2);
@@ -132,7 +127,7 @@ static void output(int v, const char *fmt, ...)
}
strbuf_setlen(&obuf, obuf.len + len);
strbuf_add(&obuf, "\n", 1);
- if (!buffer_output)
+ if (!o->buffer_output)
flush_output();
}
@@ -219,17 +214,17 @@ static int git_merge_trees(int index_only,
return rc;
}
-struct tree *write_tree_from_memory(void)
+struct tree *write_tree_from_memory(struct merge_options *o)
{
struct tree *result = NULL;
if (unmerged_cache()) {
int i;
- output(0, "There are unmerged index entries:");
+ output(o, 0, "There are unmerged index entries:");
for (i = 0; i < active_nr; i++) {
struct cache_entry *ce = active_cache[i];
if (ce_stage(ce))
- output(0, "%d %.*s", ce_stage(ce), ce_namelen(ce), ce->name);
+ output(o, 0, "%d %.*s", ce_stage(ce), ce_namelen(ce), ce->name);
}
return NULL;
}
@@ -341,11 +336,12 @@ struct rename
* 'b_tree') to be able to associate the correct cache entries with
* the rename information. 'tree' is always equal to either a_tree or b_tree.
*/
-static struct string_list *get_renames(struct tree *tree,
- struct tree *o_tree,
- struct tree *a_tree,
- struct tree *b_tree,
- struct string_list *entries)
+static struct string_list *get_renames(struct merge_options *o,
+ struct tree *tree,
+ struct tree *o_tree,
+ struct tree *a_tree,
+ struct tree *b_tree,
+ struct string_list *entries)
{
int i;
struct string_list *renames;
@@ -355,8 +351,8 @@ static struct string_list *get_renames(struct tree *tree,
diff_setup(&opts);
DIFF_OPT_SET(&opts, RECURSIVE);
opts.detect_rename = DIFF_DETECT_RENAME;
- opts.rename_limit = merge_rename_limit >= 0 ? merge_rename_limit :
- diff_rename_limit >= 0 ? diff_rename_limit :
+ opts.rename_limit = o->merge_rename_limit >= 0 ? o->merge_rename_limit :
+ o->diff_rename_limit >= 0 ? o->diff_rename_limit :
500;
opts.warn_on_too_large_rename = 1;
opts.output_format = DIFF_FORMAT_NO_OUTPUT;
@@ -717,7 +713,8 @@ static struct merge_file_info merge_file(struct diff_filespec *o,
return result;
}
-static void conflict_rename_rename(struct rename *ren1,
+static void conflict_rename_rename(struct merge_options *o,
+ struct rename *ren1,
const char *branch1,
struct rename *ren2,
const char *branch2)
@@ -730,13 +727,13 @@ static void conflict_rename_rename(struct rename *ren1,
const char *dst_name2 = ren2_dst;
if (string_list_has_string(¤t_directory_set, ren1_dst)) {
dst_name1 = del[delp++] = unique_path(ren1_dst, branch1);
- output(1, "%s is a directory in %s added as %s instead",
+ output(o, 1, "%s is a directory in %s added as %s instead",
ren1_dst, branch2, dst_name1);
remove_file(0, ren1_dst, 0);
}
if (string_list_has_string(¤t_directory_set, ren2_dst)) {
dst_name2 = del[delp++] = unique_path(ren2_dst, branch2);
- output(1, "%s is a directory in %s added as %s instead",
+ output(o, 1, "%s is a directory in %s added as %s instead",
ren2_dst, branch1, dst_name2);
remove_file(0, ren2_dst, 0);
}
@@ -757,24 +754,26 @@ static void conflict_rename_rename(struct rename *ren1,
free(del[delp]);
}
-static void conflict_rename_dir(struct rename *ren1,
+static void conflict_rename_dir(struct merge_options *o,
+ struct rename *ren1,
const char *branch1)
{
char *new_path = unique_path(ren1->pair->two->path, branch1);
- output(1, "Renamed %s to %s instead", ren1->pair->one->path, new_path);
+ output(o, 1, "Renamed %s to %s instead", ren1->pair->one->path, new_path);
remove_file(0, ren1->pair->two->path, 0);
update_file(0, ren1->pair->two->sha1, ren1->pair->two->mode, new_path);
free(new_path);
}
-static void conflict_rename_rename_2(struct rename *ren1,
+static void conflict_rename_rename_2(struct merge_options *o,
+ struct rename *ren1,
const char *branch1,
struct rename *ren2,
const char *branch2)
{
char *new_path1 = unique_path(ren1->pair->two->path, branch1);
char *new_path2 = unique_path(ren2->pair->two->path, branch2);
- output(1, "Renamed %s to %s and %s to %s instead",
+ output(o, 1, "Renamed %s to %s and %s to %s instead",
ren1->pair->one->path, new_path1,
ren2->pair->one->path, new_path2);
remove_file(0, ren1->pair->two->path, 0);
@@ -784,10 +783,9 @@ static void conflict_rename_rename_2(struct rename *ren1,
free(new_path1);
}
-static int process_renames(struct string_list *a_renames,
- struct string_list *b_renames,
- const char *a_branch,
- const char *b_branch)
+static int process_renames(struct merge_options *o,
+ struct string_list *a_renames,
+ struct string_list *b_renames)
{
int clean_merge = 1, i, j;
struct string_list a_by_dst = {NULL, 0, 0, 0}, b_by_dst = {NULL, 0, 0, 0};
@@ -832,15 +830,15 @@ static int process_renames(struct string_list *a_renames,
renames1 = a_renames;
renames2 = b_renames;
renames2Dst = &b_by_dst;
- branch1 = a_branch;
- branch2 = b_branch;
+ branch1 = o->branch1;
+ branch2 = o->branch2;
} else {
struct rename *tmp;
renames1 = b_renames;
renames2 = a_renames;
renames2Dst = &a_by_dst;
- branch1 = b_branch;
- branch2 = a_branch;
+ branch1 = o->branch2;
+ branch2 = o->branch1;
tmp = ren2;
ren2 = ren1;
ren1 = tmp;
@@ -867,7 +865,7 @@ static int process_renames(struct string_list *a_renames,
ren2->processed = 1;
if (strcmp(ren1_dst, ren2_dst) != 0) {
clean_merge = 0;
- output(1, "CONFLICT (rename/rename): "
+ output(o, 1, "CONFLICT (rename/rename): "
"Rename \"%s\"->\"%s\" in branch \"%s\" "
"rename \"%s\"->\"%s\" in \"%s\"%s",
src, ren1_dst, branch1,
@@ -878,7 +876,7 @@ static int process_renames(struct string_list *a_renames,
update_file(0, ren1->pair->one->sha1,
ren1->pair->one->mode, src);
}
- conflict_rename_rename(ren1, branch1, ren2, branch2);
+ conflict_rename_rename(o, ren1, branch1, ren2, branch2);
} else {
struct merge_file_info mfi;
remove_file(1, ren1_src, 1);
@@ -888,13 +886,13 @@ static int process_renames(struct string_list *a_renames,
branch1,
branch2);
if (mfi.merge || !mfi.clean)
- output(1, "Renamed %s->%s", src, ren1_dst);
+ output(o, 1, "Renamed %s->%s", src, ren1_dst);
if (mfi.merge)
- output(2, "Auto-merged %s", ren1_dst);
+ output(o, 2, "Auto-merged %s", ren1_dst);
if (!mfi.clean) {
- output(1, "CONFLICT (content): merge conflict in %s",
+ output(o, 1, "CONFLICT (content): merge conflict in %s",
ren1_dst);
clean_merge = 0;
@@ -925,14 +923,14 @@ static int process_renames(struct string_list *a_renames,
if (string_list_has_string(¤t_directory_set, ren1_dst)) {
clean_merge = 0;
- output(1, "CONFLICT (rename/directory): Renamed %s->%s in %s "
+ output(o, 1, "CONFLICT (rename/directory): Renamed %s->%s in %s "
" directory %s added in %s",
ren1_src, ren1_dst, branch1,
ren1_dst, branch2);
- conflict_rename_dir(ren1, branch1);
+ conflict_rename_dir(o, ren1, branch1);
} else if (sha_eq(src_other.sha1, null_sha1)) {
clean_merge = 0;
- output(1, "CONFLICT (rename/delete): Renamed %s->%s in %s "
+ output(o, 1, "CONFLICT (rename/delete): Renamed %s->%s in %s "
"and deleted in %s",
ren1_src, ren1_dst, branch1,
branch2);
@@ -941,31 +939,31 @@ static int process_renames(struct string_list *a_renames,
const char *new_path;
clean_merge = 0;
try_merge = 1;
- output(1, "CONFLICT (rename/add): Renamed %s->%s in %s. "
+ output(o, 1, "CONFLICT (rename/add): Renamed %s->%s in %s. "
"%s added in %s",
ren1_src, ren1_dst, branch1,
ren1_dst, branch2);
new_path = unique_path(ren1_dst, branch2);
- output(1, "Added as %s instead", new_path);
+ output(o, 1, "Added as %s instead", new_path);
update_file(0, dst_other.sha1, dst_other.mode, new_path);
} else if ((item = string_list_lookup(ren1_dst, renames2Dst))) {
ren2 = item->util;
clean_merge = 0;
ren2->processed = 1;
- output(1, "CONFLICT (rename/rename): Renamed %s->%s in %s. "
+ output(o, 1, "CONFLICT (rename/rename): Renamed %s->%s in %s. "
"Renamed %s->%s in %s",
ren1_src, ren1_dst, branch1,
ren2->pair->one->path, ren2->pair->two->path, branch2);
- conflict_rename_rename_2(ren1, branch1, ren2, branch2);
+ conflict_rename_rename_2(o, ren1, branch1, ren2, branch2);
} else
try_merge = 1;
if (try_merge) {
- struct diff_filespec *o, *a, *b;
+ struct diff_filespec *one, *a, *b;
struct merge_file_info mfi;
src_other.path = (char *)ren1_src;
- o = ren1->pair->one;
+ one = ren1->pair->one;
if (a_renames == renames1) {
a = ren1->pair->two;
b = &src_other;
@@ -973,8 +971,8 @@ static int process_renames(struct string_list *a_renames,
b = ren1->pair->two;
a = &src_other;
}
- mfi = merge_file(o, a, b,
- a_branch, b_branch);
+ mfi = merge_file(one, a, b,
+ o->branch1, o->branch2);
if (mfi.clean &&
sha_eq(mfi.sha, ren1->pair->two->sha1) &&
@@ -984,20 +982,20 @@ static int process_renames(struct string_list *a_renames,
* t6022 test. If you change
* it update the test too.
*/
- output(3, "Skipped %s (merged same as existing)", ren1_dst);
+ output(o, 3, "Skipped %s (merged same as existing)", ren1_dst);
else {
if (mfi.merge || !mfi.clean)
- output(1, "Renamed %s => %s", ren1_src, ren1_dst);
+ output(o, 1, "Renamed %s => %s", ren1_src, ren1_dst);
if (mfi.merge)
- output(2, "Auto-merged %s", ren1_dst);
+ output(o, 2, "Auto-merged %s", ren1_dst);
if (!mfi.clean) {
- output(1, "CONFLICT (rename/modify): Merge conflict in %s",
+ output(o, 1, "CONFLICT (rename/modify): Merge conflict in %s",
ren1_dst);
clean_merge = 0;
if (!index_only)
update_stages(ren1_dst,
- o, a, b, 1);
+ one, a, b, 1);
}
update_file(mfi.clean, mfi.sha, mfi.mode, ren1_dst);
}
@@ -1016,9 +1014,8 @@ static unsigned char *stage_sha(const unsigned char *sha, unsigned mode)
}
/* Per entry merge function */
-static int process_entry(const char *path, struct stage_data *entry,
- const char *branch1,
- const char *branch2)
+static int process_entry(struct merge_options *o,
+ const char *path, struct stage_data *entry)
{
/*
printf("processing entry, clean cache: %s\n", index_only ? "yes": "no");
@@ -1040,23 +1037,23 @@ static int process_entry(const char *path, struct stage_data *entry,
/* Deleted in both or deleted in one and
* unchanged in the other */
if (a_sha)
- output(2, "Removed %s", path);
+ output(o, 2, "Removed %s", path);
/* do not touch working file if it did not exist */
remove_file(1, path, !a_sha);
} else {
/* Deleted in one and changed in the other */
clean_merge = 0;
if (!a_sha) {
- output(1, "CONFLICT (delete/modify): %s deleted in %s "
+ output(o, 1, "CONFLICT (delete/modify): %s deleted in %s "
"and modified in %s. Version %s of %s left in tree.",
- path, branch1,
- branch2, branch2, path);
+ path, o->branch1,
+ o->branch2, o->branch2, path);
update_file(0, b_sha, b_mode, path);
} else {
- output(1, "CONFLICT (delete/modify): %s deleted in %s "
+ output(o, 1, "CONFLICT (delete/modify): %s deleted in %s "
"and modified in %s. Version %s of %s left in tree.",
- path, branch2,
- branch1, branch1, path);
+ path, o->branch2,
+ o->branch1, o->branch1, path);
update_file(0, a_sha, a_mode, path);
}
}
@@ -1071,14 +1068,14 @@ static int process_entry(const char *path, struct stage_data *entry,
const char *conf;
if (a_sha) {
- add_branch = branch1;
- other_branch = branch2;
+ add_branch = o->branch1;
+ other_branch = o->branch2;
mode = a_mode;
sha = a_sha;
conf = "file/directory";
} else {
- add_branch = branch2;
- other_branch = branch1;
+ add_branch = o->branch2;
+ other_branch = o->branch1;
mode = b_mode;
sha = b_sha;
conf = "directory/file";
@@ -1086,13 +1083,13 @@ static int process_entry(const char *path, struct stage_data *entry,
if (string_list_has_string(¤t_directory_set, path)) {
const char *new_path = unique_path(path, add_branch);
clean_merge = 0;
- output(1, "CONFLICT (%s): There is a directory with name %s in %s. "
+ output(o, 1, "CONFLICT (%s): There is a directory with name %s in %s. "
"Added %s as %s",
conf, path, other_branch, path, new_path);
remove_file(0, path, 0);
update_file(0, sha, mode, new_path);
} else {
- output(2, "Added %s", path);
+ output(o, 2, "Added %s", path);
update_file(1, sha, mode, path);
}
} else if (a_sha && b_sha) {
@@ -1100,32 +1097,32 @@ static int process_entry(const char *path, struct stage_data *entry,
/* case D: Modified in both, but differently. */
const char *reason = "content";
struct merge_file_info mfi;
- struct diff_filespec o, a, b;
+ struct diff_filespec one, a, b;
if (!o_sha) {
reason = "add/add";
o_sha = (unsigned char *)null_sha1;
}
- output(2, "Auto-merged %s", path);
- o.path = a.path = b.path = (char *)path;
- hashcpy(o.sha1, o_sha);
- o.mode = o_mode;
+ output(o, 2, "Auto-merged %s", path);
+ one.path = a.path = b.path = (char *)path;
+ hashcpy(one.sha1, o_sha);
+ one.mode = o_mode;
hashcpy(a.sha1, a_sha);
a.mode = a_mode;
hashcpy(b.sha1, b_sha);
b.mode = b_mode;
- mfi = merge_file(&o, &a, &b,
- branch1, branch2);
+ mfi = merge_file(&one, &a, &b,
+ o->branch1, o->branch2);
clean_merge = mfi.clean;
if (mfi.clean)
update_file(1, mfi.sha, mfi.mode, path);
else if (S_ISGITLINK(mfi.mode))
- output(1, "CONFLICT (submodule): Merge conflict in %s "
+ output(o, 1, "CONFLICT (submodule): Merge conflict in %s "
"- needs %s", path, sha1_to_hex(b.sha1));
else {
- output(1, "CONFLICT (%s): Merge conflict in %s",
+ output(o, 1, "CONFLICT (%s): Merge conflict in %s",
reason, path);
if (index_only)
@@ -1146,11 +1143,10 @@ static int process_entry(const char *path, struct stage_data *entry,
return clean_merge;
}
-int merge_trees(struct tree *head,
+int merge_trees(struct merge_options *o,
+ struct tree *head,
struct tree *merge,
struct tree *common,
- const char *branch1,
- const char *branch2,
struct tree **result)
{
int code, clean;
@@ -1161,7 +1157,7 @@ int merge_trees(struct tree *head,
}
if (sha_eq(common->object.sha1, merge->object.sha1)) {
- output(0, "Already uptodate!");
+ output(o, 0, "Already uptodate!");
*result = head;
return 1;
}
@@ -1182,15 +1178,14 @@ int merge_trees(struct tree *head,
get_files_dirs(merge);
entries = get_unmerged();
- re_head = get_renames(head, common, head, merge, entries);
- re_merge = get_renames(merge, common, head, merge, entries);
- clean = process_renames(re_head, re_merge,
- branch1, branch2);
+ re_head = get_renames(o, head, common, head, merge, entries);
+ re_merge = get_renames(o, merge, common, head, merge, entries);
+ clean = process_renames(o, re_head, re_merge);
for (i = 0; i < entries->nr; i++) {
const char *path = entries->items[i].string;
struct stage_data *e = entries->items[i].util;
if (!e->processed
- && !process_entry(path, e, branch1, branch2))
+ && !process_entry(o, path, e))
clean = 0;
}
@@ -1203,7 +1198,7 @@ int merge_trees(struct tree *head,
clean = 1;
if (index_only)
- *result = write_tree_from_memory();
+ *result = write_tree_from_memory(o);
return clean;
}
@@ -1223,10 +1218,9 @@ static struct commit_list *reverse_commit_list(struct commit_list *list)
* Merge the commits h1 and h2, return the resulting virtual
* commit object and a flag indicating the cleanness of the merge.
*/
-int merge_recursive(struct commit *h1,
+int merge_recursive(struct merge_options *o,
+ struct commit *h1,
struct commit *h2,
- const char *branch1,
- const char *branch2,
struct commit_list *ca,
struct commit **result)
{
@@ -1235,8 +1229,8 @@ int merge_recursive(struct commit *h1,
struct tree *mrtree = mrtree;
int clean;
- if (show(4)) {
- output(4, "Merging:");
+ if (show(o, 4)) {
+ output(o, 4, "Merging:");
output_commit_title(h1);
output_commit_title(h2);
}
@@ -1246,8 +1240,8 @@ int merge_recursive(struct commit *h1,
ca = reverse_commit_list(ca);
}
- if (show(5)) {
- output(5, "found %u common ancestor(s):", commit_list_count(ca));
+ if (show(o, 5)) {
+ output(o, 5, "found %u common ancestor(s):", commit_list_count(ca));
for (iter = ca; iter; iter = iter->next)
output_commit_title(iter->item);
}
@@ -1264,6 +1258,7 @@ int merge_recursive(struct commit *h1,
}
for (iter = ca; iter; iter = iter->next) {
+ const char *saved_b1, *saved_b2;
call_depth++;
/*
* When the merge fails, the result contains files
@@ -1273,11 +1268,14 @@ int merge_recursive(struct commit *h1,
* "conflicts" were already resolved.
*/
discard_cache();
- merge_recursive(merged_common_ancestors, iter->item,
- "Temporary merge branch 1",
- "Temporary merge branch 2",
- NULL,
- &merged_common_ancestors);
+ saved_b1 = o->branch1;
+ saved_b2 = o->branch2;
+ o->branch1 = "Temporary merge branch 1";
+ o->branch2 = "Temporary merge branch 2";
+ merge_recursive(o, merged_common_ancestors, iter->item,
+ NULL, &merged_common_ancestors);
+ o->branch1 = saved_b1;
+ o->branch2 = saved_b2;
call_depth--;
if (!merged_common_ancestors)
@@ -1291,8 +1289,8 @@ int merge_recursive(struct commit *h1,
} else
index_only = 1;
- clean = merge_trees(h1->tree, h2->tree, merged_common_ancestors->tree,
- branch1, branch2, &mrtree);
+ clean = merge_trees(o, h1->tree, h2->tree, merged_common_ancestors->tree,
+ &mrtree);
if (index_only) {
*result = make_virtual_commit(mrtree, "merged tree");
@@ -1319,35 +1317,33 @@ static struct commit *get_ref(const unsigned char *sha1, const char *name)
return (struct commit *)object;
}
-int merge_recursive_generic(const char **base_list,
- const unsigned char *head_sha1, const char *head_name,
- const unsigned char *next_sha1, const char *next_name)
+int merge_recursive_generic(struct merge_options *o,
+ const unsigned char *head,
+ const unsigned char *merge,
+ int num_base_list,
+ const unsigned char **base_list,
+ struct commit **result)
{
int clean, index_fd;
struct lock_file *lock = xcalloc(1, sizeof(struct lock_file));
- struct commit *result;
- struct commit *head_commit = get_ref(head_sha1, head_name);
- struct commit *next_commit = get_ref(next_sha1, next_name);
+ struct commit *head_commit = get_ref(head, o->branch1);
+ struct commit *next_commit = get_ref(merge, o->branch2);
struct commit_list *ca = NULL;
if (base_list) {
int i;
- for (i = 0; base_list[i]; ++i) {
- unsigned char sha[20];
+ for (i = 0; i < num_base_list; ++i) {
struct commit *base;
- if (get_sha1(base_list[i], sha))
- return error("Could not resolve ref '%s'",
- base_list[i]);
- if (!(base = get_ref(sha, base_list[i])))
+ if (!(base = get_ref(base_list[i], sha1_to_hex(base_list[i]))))
return error("Could not parse object '%s'",
- base_list[i]);
+ sha1_to_hex(base_list[i]));
commit_list_insert(base, &ca);
}
}
index_fd = hold_locked_index(lock, 1);
- clean = merge_recursive(head_commit, next_commit,
- head_name, next_name, ca, &result);
+ clean = merge_recursive(o, head_commit, next_commit, ca,
+ result);
if (active_cache_changed &&
(write_cache(index_fd, active_cache, active_nr) ||
commit_locked_index(lock)))
@@ -1356,29 +1352,35 @@ int merge_recursive_generic(const char **base_list,
return clean ? 0 : 1;
}
-int merge_recursive_config(const char *var, const char *value, void *cb)
+static int merge_recursive_config(const char *var, const char *value, void *cb)
{
+ struct merge_options *o = cb;
if (!strcasecmp(var, "merge.verbosity")) {
- merge_recursive_verbosity = git_config_int(var, value);
+ o->verbosity = git_config_int(var, value);
return 0;
}
if (!strcasecmp(var, "diff.renamelimit")) {
- diff_rename_limit = git_config_int(var, value);
+ o->diff_rename_limit = git_config_int(var, value);
return 0;
}
if (!strcasecmp(var, "merge.renamelimit")) {
- merge_rename_limit = git_config_int(var, value);
+ o->merge_rename_limit = git_config_int(var, value);
return 0;
}
return git_default_config(var, value, cb);
}
-void merge_recursive_setup(int is_subtree_merge)
+void init_merge_options(struct merge_options *o)
{
+ memset(o, 0, sizeof(struct merge_options));
+ o->verbosity = 2;
+ o->buffer_output = 1;
+ o->diff_rename_limit = -1;
+ o->merge_rename_limit = -1;
+ git_config(merge_recursive_config, o);
if (getenv("GIT_MERGE_VERBOSITY"))
- merge_recursive_verbosity =
+ o->verbosity =
strtol(getenv("GIT_MERGE_VERBOSITY"), NULL, 10);
- if (merge_recursive_verbosity >= 5)
- buffer_output = 0;
- subtree_merge = is_subtree_merge;
+ if (o->verbosity >= 5)
+ o->buffer_output = 0;
}
diff --git a/merge-recursive.h b/merge-recursive.h
index 4dd6476..72f0a28 100644
--- a/merge-recursive.h
+++ b/merge-recursive.h
@@ -1,26 +1,42 @@
#ifndef MERGE_RECURSIVE_H
#define MERGE_RECURSIVE_H
-int merge_recursive(struct commit *h1,
+struct merge_options {
+ const char *branch1;
+ const char *branch2;
+ unsigned subtree_merge : 1;
+ unsigned buffer_output : 1;
+ int verbosity;
+ int diff_rename_limit;
+ int merge_rename_limit;
+};
+
+/* merge_trees() but with recursive ancestor consolidation */
+int merge_recursive(struct merge_options *o,
+ struct commit *h1,
struct commit *h2,
- const char *branch1,
- const char *branch2,
struct commit_list *ancestors,
struct commit **result);
-int merge_trees(struct tree *head,
+/* rename-detecting three-way merge, no recursion */
+int merge_trees(struct merge_options *o,
+ struct tree *head,
struct tree *merge,
struct tree *common,
- const char *branch1,
- const char *branch2,
struct tree **result);
-extern int merge_recursive_generic(const char **base_list,
- const unsigned char *head_sha1, const char *head_name,
- const unsigned char *next_sha1, const char *next_name);
-int merge_recursive_config(const char *var, const char *value, void *cb);
-void merge_recursive_setup(int is_subtree_merge);
-struct tree *write_tree_from_memory(void);
-extern int merge_recursive_verbosity;
+/*
+ * "git-merge-recursive" can be fed trees; wrap them into
+ * virtual commits and call merge_recursive() proper.
+ */
+int merge_recursive_generic(struct merge_options *o,
+ const unsigned char *head,
+ const unsigned char *merge,
+ int num_ca,
+ const unsigned char **ca,
+ struct commit **result);
+
+void init_merge_options(struct merge_options *o);
+struct tree *write_tree_from_memory(struct merge_options *o);
#endif
--
1.6.0.rc3.17.gc14c8.dirty
^ permalink raw reply related [flat|nested] 35+ messages in thread
* Re: [PATCH] merge-recursive: introduce merge_options
2008-08-25 14:25 ` Miklos Vajna
@ 2008-08-28 4:50 ` Junio C Hamano
2008-08-30 15:42 ` [PATCH] merge-recursive: fix subtree merge Miklos Vajna
1 sibling, 0 replies; 35+ messages in thread
From: Junio C Hamano @ 2008-08-28 4:50 UTC (permalink / raw)
To: Miklos Vajna; +Cc: git, Stephan Beyer
Miklos Vajna <vmiklos@frugalware.org> writes:
> On Sun, Aug 24, 2008 at 11:06:06PM -0700, Junio C Hamano <gitster@pobox.com> wrote:
>> Miklos Vajna <vmiklos@frugalware.org> writes:
>>
>> > 1) This applies on top of 1c868d4 (merge-recursive.c: Add more
>> > generic merge_recursive_generic()). I can rebase this (along with
>> > 1c868d4 and 1c868d4^) on top of current master, if this is a problem.
>>
>> It probably is cleaner to treat this as a fresh topic from scratch on
>> top of 'master', as we do not have anything outstanding in 'next'
>> around this area.
>
> I'm now confused about what should I do:
>
> 1) Nothing. (That's what I did for now.)
>
> 2) Rebase against master and resend.
>
> 3) Rebase, squash and resend.
What I meant was that the final state after applying this patch may make
what "git log master..1c868d4" currently shows (there are two patches if I
recall correctly) an incomplete failed experiment, in which case squashing
and possibly refactoring (if the result of squashing is too messy) would
make the history easier to review.
But I looked at the series again after rebasing them myself.
If you want to clean-it-up, you could replace them by sending in updates
to refactor them. I think they are still ugly, even though the end result
is tolerable ;-)
^ permalink raw reply [flat|nested] 35+ messages in thread
* [PATCH] merge-recursive: fix subtree merge
2008-08-25 14:25 ` Miklos Vajna
2008-08-28 4:50 ` Junio C Hamano
@ 2008-08-30 15:42 ` Miklos Vajna
2008-08-30 16:39 ` Junio C Hamano
2008-08-30 17:55 ` Junio C Hamano
1 sibling, 2 replies; 35+ messages in thread
From: Miklos Vajna @ 2008-08-30 15:42 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Stephan Beyer
We should decide if we do a subtree merge based on the merge_options
struct, not based on the global variable, which should not even exist
at all.
Signed-off-by: Miklos Vajna <vmiklos@frugalware.org>
---
On Mon, Aug 25, 2008 at 04:25:57PM +0200, Miklos Vajna <vmiklos@frugalware.org> wrote:
> It also takes care of subtree merge, output buffering, verbosity, and
> rename limits - these were global variables till now in
> merge-recursive.c.
Actually subtree_merge was not used from the struct merge_options, here
is the fix.
merge-recursive.c | 4 +---
1 files changed, 1 insertions(+), 3 deletions(-)
diff --git a/merge-recursive.c b/merge-recursive.c
index 3a38cc6..457ad84 100644
--- a/merge-recursive.c
+++ b/merge-recursive.c
@@ -20,8 +20,6 @@
#include "attr.h"
#include "merge-recursive.h"
-static int subtree_merge;
-
static struct tree *shift_tree_object(struct tree *one, struct tree *two)
{
unsigned char shifted[20];
@@ -1152,7 +1150,7 @@ int merge_trees(struct merge_options *o,
{
int code, clean;
- if (subtree_merge) {
+ if (o->subtree_merge) {
merge = shift_tree_object(head, merge);
common = shift_tree_object(head, common);
}
--
1.6.0
^ permalink raw reply related [flat|nested] 35+ messages in thread
* Re: [PATCH] merge-recursive: fix subtree merge
2008-08-30 15:42 ` [PATCH] merge-recursive: fix subtree merge Miklos Vajna
@ 2008-08-30 16:39 ` Junio C Hamano
2008-08-30 17:55 ` Junio C Hamano
1 sibling, 0 replies; 35+ messages in thread
From: Junio C Hamano @ 2008-08-30 16:39 UTC (permalink / raw)
To: Miklos Vajna; +Cc: git, Stephan Beyer
Miklos Vajna <vmiklos@frugalware.org> writes:
> On Mon, Aug 25, 2008 at 04:25:57PM +0200, Miklos Vajna <vmiklos@frugalware.org> wrote:
>> It also takes care of subtree merge, output buffering, verbosity, and
>> rename limits - these were global variables till now in
>> merge-recursive.c.
>
> Actually subtree_merge was not used from the struct merge_options, here
> is the fix.
As bd1e8fe (merge-recursive: introduce merge_options, 2008-08-25) is not
part of any solid integration branch yet, I'll squash this into it.
In the longer term, I suspect that other file scope static variables in
merge-recursive.c, such as call_depth, may want to move to merge_options,
not as "option" but as "current state". But that is quite minor, as it
only affects reentrancy, and I do not see a reason for merge_recursive()
to be reentrant (yet).
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH] merge-recursive: fix subtree merge
2008-08-30 15:42 ` [PATCH] merge-recursive: fix subtree merge Miklos Vajna
2008-08-30 16:39 ` Junio C Hamano
@ 2008-08-30 17:55 ` Junio C Hamano
2008-08-31 23:49 ` Miklos Vajna
1 sibling, 1 reply; 35+ messages in thread
From: Junio C Hamano @ 2008-08-30 17:55 UTC (permalink / raw)
To: Miklos Vajna; +Cc: git, Stephan Beyer
Miklos Vajna <vmiklos@frugalware.org> writes:
> On Mon, Aug 25, 2008 at 04:25:57PM +0200, Miklos Vajna <vmiklos@frugalware.org> wrote:
>> It also takes care of subtree merge, output buffering, verbosity, and
>> rename limits - these were global variables till now in
>> merge-recursive.c.
>
> Actually subtree_merge was not used from the struct merge_options, here
> is the fix.
This makes me wonder why we didn't see any breakages in the existing
tests.
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH] merge-recursive: fix subtree merge
2008-08-30 17:55 ` Junio C Hamano
@ 2008-08-31 23:49 ` Miklos Vajna
0 siblings, 0 replies; 35+ messages in thread
From: Miklos Vajna @ 2008-08-31 23:49 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Stephan Beyer
[-- Attachment #1: Type: text/plain, Size: 360 bytes --]
On Sat, Aug 30, 2008 at 10:55:24AM -0700, Junio C Hamano <gitster@pobox.com> wrote:
> This makes me wonder why we didn't see any breakages in the existing
> tests.
That is a really interesting question. I have no idea how t6029 can pass
without this fix, but actually it does. (Yes, I read that part of the
code and the testsuite as well, but still no clue.)
[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: What's cooking in git.git (Aug 2008, #05; Tue, 19)
2008-08-19 22:00 ` Junio C Hamano
2008-08-20 22:42 ` Miklos Vajna
2008-08-25 1:44 ` [PATCH] merge-recursive: introduce merge_options Miklos Vajna
@ 2008-09-01 1:06 ` Miklos Vajna
2008-09-01 1:09 ` [PATCH] builtin-revert: use merge_recursive_generic() Miklos Vajna
2008-09-02 20:02 ` What's cooking in git.git (Aug 2008, #05; Tue, 19) Junio C Hamano
2 siblings, 2 replies; 35+ messages in thread
From: Miklos Vajna @ 2008-09-01 1:06 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Stephan Beyer
[-- Attachment #1: Type: text/plain, Size: 3467 bytes --]
On Tue, Aug 19, 2008 at 03:00:27PM -0700, Junio C Hamano <gitster@pobox.com> wrote:
> and the call chain would become:
> (...)
> cmd_revert(), cmd_am(), cmd_checkout(), cmd_stash(), ...
> -> merge_trees()
I tried to let cmd_revert() use merge_trees() only and not
merge_recursive_generic(), but something is fishy with it.
t3501-revert-cherry-pick passes fine, but t3404-rebase-interactive fail,
becase once we have a conflict, git diff --name-status says 'M' for the
given file and not 'U', which is obviously wrong.
I'm sending both patches: the one that works properly using
merge_recursive_generic() - and which one passes the testsuite here, and
the one that is elegant as it uses merge_trees(), but actually not
complete, at least regarding not adding the unmerged entries to the
index.
So, first here is my failed attempt to use merge_trees() inside
builtin-revert:
diff --git a/builtin-revert.c b/builtin-revert.c
index 3667705..3071518 100644
--- a/builtin-revert.c
+++ b/builtin-revert.c
@@ -12,6 +12,7 @@
#include "diff.h"
#include "revision.h"
#include "rerere.h"
+#include "merge-recursive.h"
/*
* This implements the builtins revert and cherry-pick.
@@ -201,36 +202,6 @@ static void set_author_ident_env(const char *message)
sha1_to_hex(commit->object.sha1));
}
-static int merge_recursive(const char *base_sha1,
- const char *head_sha1, const char *head_name,
- const char *next_sha1, const char *next_name)
-{
- char buffer[256];
- const char *argv[6];
- int i = 0;
-
- sprintf(buffer, "GITHEAD_%s", head_sha1);
- setenv(buffer, head_name, 1);
- sprintf(buffer, "GITHEAD_%s", next_sha1);
- setenv(buffer, next_name, 1);
-
- /*
- * This three way merge is an interesting one. We are at
- * $head, and would want to apply the change between $commit
- * and $prev on top of us (when reverting), or the change between
- * $prev and $commit on top of us (when cherry-picking or replaying).
- */
- argv[i++] = "merge-recursive";
- if (base_sha1)
- argv[i++] = base_sha1;
- argv[i++] = "--";
- argv[i++] = head_sha1;
- argv[i++] = next_sha1;
- argv[i++] = NULL;
-
- return run_command_v_opt(argv, RUN_COMMAND_NO_STDIN | RUN_GIT_CMD);
-}
-
static char *help_msg(const unsigned char *sha1)
{
static char helpbuf[1024];
@@ -271,6 +242,8 @@ static int revert_or_cherry_pick(int argc, const char **argv)
char *oneline, *reencoded_message = NULL;
const char *message, *encoding;
const char *defmsg = xstrdup(git_path("MERGE_MSG"));
+ struct merge_options o;
+ struct tree *result;
git_config(git_default_config, NULL);
me = action == REVERT ? "revert" : "cherry-pick";
@@ -374,13 +347,16 @@ static int revert_or_cherry_pick(int argc, const char **argv)
}
}
- if (merge_recursive(base == NULL ?
- NULL : sha1_to_hex(base->object.sha1),
- sha1_to_hex(head), "HEAD",
- sha1_to_hex(next->object.sha1), oneline) ||
- write_cache_as_tree(head, 0, NULL)) {
+ read_cache();
+ init_merge_options(&o);
+ o.branch1 = "HEAD";
+ o.branch2 = oneline;
+ parse_commit(next);
+ parse_commit(base);
+ if (!merge_trees(&o, lookup_commit_reference_gently(head, 0)->tree,
+ next->tree, base->tree, &result) ||
+ write_cache_as_tree(head, 0, NULL)) {
add_to_msg("\nConflicts:\n\n");
- read_cache();
for (i = 0; i < active_nr;) {
struct cache_entry *ce = active_cache[i++];
if (ce_stage(ce)) {
[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]
^ permalink raw reply related [flat|nested] 35+ messages in thread
* [PATCH] builtin-revert: use merge_recursive_generic()
2008-09-01 1:06 ` What's cooking in git.git (Aug 2008, #05; Tue, 19) Miklos Vajna
@ 2008-09-01 1:09 ` Miklos Vajna
2008-09-02 20:02 ` What's cooking in git.git (Aug 2008, #05; Tue, 19) Junio C Hamano
1 sibling, 0 replies; 35+ messages in thread
From: Miklos Vajna @ 2008-09-01 1:09 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Stephan Beyer
We had a separate function to run merge-recursive in a separate process,
but this is not really necessary, since we have
merge_recursive_generic() to do the same, without wasting resources.
Signed-off-by: Miklos Vajna <vmiklos@frugalware.org>
---
On Mon, Sep 01, 2008 at 03:06:12AM +0200, Miklos Vajna <vmiklos@frugalware.org> wrote:
> I'm sending both patches: the one that works properly using
> merge_recursive_generic() - and which one passes the testsuite here
Here it is.
builtin-revert.c | 55 ++++++++++++++++++-----------------------------------
1 files changed, 19 insertions(+), 36 deletions(-)
diff --git a/builtin-revert.c b/builtin-revert.c
index 3667705..8d4ebcf 100644
--- a/builtin-revert.c
+++ b/builtin-revert.c
@@ -12,6 +12,7 @@
#include "diff.h"
#include "revision.h"
#include "rerere.h"
+#include "merge-recursive.h"
/*
* This implements the builtins revert and cherry-pick.
@@ -201,36 +202,6 @@ static void set_author_ident_env(const char *message)
sha1_to_hex(commit->object.sha1));
}
-static int merge_recursive(const char *base_sha1,
- const char *head_sha1, const char *head_name,
- const char *next_sha1, const char *next_name)
-{
- char buffer[256];
- const char *argv[6];
- int i = 0;
-
- sprintf(buffer, "GITHEAD_%s", head_sha1);
- setenv(buffer, head_name, 1);
- sprintf(buffer, "GITHEAD_%s", next_sha1);
- setenv(buffer, next_name, 1);
-
- /*
- * This three way merge is an interesting one. We are at
- * $head, and would want to apply the change between $commit
- * and $prev on top of us (when reverting), or the change between
- * $prev and $commit on top of us (when cherry-picking or replaying).
- */
- argv[i++] = "merge-recursive";
- if (base_sha1)
- argv[i++] = base_sha1;
- argv[i++] = "--";
- argv[i++] = head_sha1;
- argv[i++] = next_sha1;
- argv[i++] = NULL;
-
- return run_command_v_opt(argv, RUN_COMMAND_NO_STDIN | RUN_GIT_CMD);
-}
-
static char *help_msg(const unsigned char *sha1)
{
static char helpbuf[1024];
@@ -266,12 +237,16 @@ static int index_is_dirty(void)
static int revert_or_cherry_pick(int argc, const char **argv)
{
unsigned char head[20];
+ unsigned const char *ca = NULL;
struct commit *base, *next, *parent;
- int i;
+ int i, fail;
char *oneline, *reencoded_message = NULL;
const char *message, *encoding;
const char *defmsg = xstrdup(git_path("MERGE_MSG"));
+ struct merge_options o;
+ struct commit *result;
+ init_merge_options(&o);
git_config(git_default_config, NULL);
me = action == REVERT ? "revert" : "cherry-pick";
setenv(GIT_REFLOG_ACTION, me, 0);
@@ -374,11 +349,19 @@ static int revert_or_cherry_pick(int argc, const char **argv)
}
}
- if (merge_recursive(base == NULL ?
- NULL : sha1_to_hex(base->object.sha1),
- sha1_to_hex(head), "HEAD",
- sha1_to_hex(next->object.sha1), oneline) ||
- write_cache_as_tree(head, 0, NULL)) {
+ if (base)
+ ca = base->object.sha1;
+ o.branch1 = "HEAD";
+ o.branch2 = oneline;
+ fail = merge_recursive_generic(&o,
+ head,
+ next->object.sha1,
+ ca ? 1 : 0,
+ &ca,
+ &result);
+ if (fail < 0)
+ exit(1);
+ if (fail || write_cache_as_tree(head, 0, NULL)) {
add_to_msg("\nConflicts:\n\n");
read_cache();
for (i = 0; i < active_nr;) {
--
1.6.0
^ permalink raw reply related [flat|nested] 35+ messages in thread
* Re: What's cooking in git.git (Aug 2008, #05; Tue, 19)
2008-09-01 1:06 ` What's cooking in git.git (Aug 2008, #05; Tue, 19) Miklos Vajna
2008-09-01 1:09 ` [PATCH] builtin-revert: use merge_recursive_generic() Miklos Vajna
@ 2008-09-02 20:02 ` Junio C Hamano
2008-09-02 20:43 ` Junio C Hamano
2008-09-02 22:05 ` [PATCH 0/2] Move call_depth and index_only to struct merge_options Miklos Vajna
1 sibling, 2 replies; 35+ messages in thread
From: Junio C Hamano @ 2008-09-02 20:02 UTC (permalink / raw)
To: Miklos Vajna; +Cc: git, Stephan Beyer
Miklos Vajna <vmiklos@frugalware.org> writes:
> I tried to let cmd_revert() use merge_trees() only and not
> merge_recursive_generic(), but something is fishy with it.
> t3501-revert-cherry-pick passes fine, but t3404-rebase-interactive fail,
> becase once we have a conflict, git diff --name-status says 'M' for the
> given file and not 'U', which is obviously wrong.
I think this is because you are forgotting to write the index file out.
You are calling "git commit -n" at the end if a cherry-pick/revert is
clean, or giving the user back an unmerged state in the index otherwise.
In either case, after merge_trees() gives you a potentially unmerged index
back in-core, you need to write it to the $GIT_INDEX_FILE for later users.
Also notice that your output is awfully silent. You are forgetting to
flush the buffered output that is kept in "obuf".
Please check what other extra things merge_recursive() function does that
merge_trees() doesn't; the above two are what I've spotted by code
inspection, but there may be others. It could be an indication of
incorrect interface layering when the builtin was refactored.
I found it a bit disturbing that "index_only" and "call_depth" were not
part of merge_options structure. The machinery is not yet meant to be
reentrant, so we may rely the previous call to the function (either
merge_recursive() or merge_trees() revert them to a sane initial value,
but it is a bit unnerving, having to depend on that assumption.
I think your code happily dereferences NULL when picking the root commit
is involved, although I do not think the failure in the test suite you saw
is related to this omission.
Here is a partial fix to address the above issues I noticed on
top of your version; untested.
builtin-revert.c | 46 ++++++++++++++++++++++++++++++++++++----------
1 files changed, 36 insertions(+), 10 deletions(-)
diff --git i/builtin-revert.c w/builtin-revert.c
index 3071518..41f3ca2 100644
--- i/builtin-revert.c
+++ w/builtin-revert.c
@@ -234,16 +234,27 @@ static int index_is_dirty(void)
return !!DIFF_OPT_TST(&rev.diffopt, HAS_CHANGES);
}
+static struct tree *empty_tree(void)
+{
+ struct tree *tree = xcalloc(1, sizeof(struct tree));
+
+ tree->object.parsed = 1;
+ tree->object.type = OBJ_TREE;
+ pretend_sha1_file(NULL, 0, OBJ_TREE, tree->object.sha1);
+ return tree;
+}
+
static int revert_or_cherry_pick(int argc, const char **argv)
{
unsigned char head[20];
struct commit *base, *next, *parent;
- int i;
+ int i, index_fd, clean;
char *oneline, *reencoded_message = NULL;
const char *message, *encoding;
const char *defmsg = xstrdup(git_path("MERGE_MSG"));
struct merge_options o;
- struct tree *result;
+ struct tree *result, *next_tree, *base_tree, *head_tree;
+ static struct lock_file index_lock;
git_config(git_default_config, NULL);
me = action == REVERT ? "revert" : "cherry-pick";
@@ -254,6 +265,10 @@ static int revert_or_cherry_pick(int argc, const char **argv)
if (action == REVERT && !no_replay)
die("revert is incompatible with replay");
+ index_fd = hold_locked_index(&index_lock, 1);
+
+ if (read_cache() < 0)
+ die("git %s: failed to read the index", me);
if (no_commit) {
/*
* We do not intend to commit immediately. We just want to
@@ -266,12 +281,10 @@ static int revert_or_cherry_pick(int argc, const char **argv)
} else {
if (get_sha1("HEAD", head))
die ("You do not have a valid HEAD");
- if (read_cache() < 0)
- die("could not read the index");
if (index_is_dirty())
die ("Dirty index: cannot %s", me);
- discard_cache();
}
+ discard_cache();
if (!commit->parents) {
if (action == REVERT)
@@ -305,6 +318,10 @@ static int revert_or_cherry_pick(int argc, const char **argv)
die ("Cannot get commit message for %s",
sha1_to_hex(commit->object.sha1));
+ if (parent && parse_commit(parent) < 0)
+ die("%s: cannot parse parent commit %s",
+ me, sha1_to_hex(parent->object.sha1));
+
/*
* "commit" is an existing commit. We would want to apply
* the difference it introduces since its first parent "prev"
@@ -351,11 +368,20 @@ static int revert_or_cherry_pick(int argc, const char **argv)
init_merge_options(&o);
o.branch1 = "HEAD";
o.branch2 = oneline;
- parse_commit(next);
- parse_commit(base);
- if (!merge_trees(&o, lookup_commit_reference_gently(head, 0)->tree,
- next->tree, base->tree, &result) ||
- write_cache_as_tree(head, 0, NULL)) {
+
+ head_tree = parse_tree_indirect(head);
+ next_tree = next ? next->tree : empty_tree();
+ base_tree = base ? base->tree : empty_tree();
+
+ clean = merge_trees(&o,
+ head_tree,
+ next_tree, base_tree, &result);
+
+ if (write_cache(index_fd, active_cache, active_nr) ||
+ commit_locked_index(&index_lock))
+ die("%s: Unable to write new index file", me);
+
+ if (!clean) {
add_to_msg("\nConflicts:\n\n");
for (i = 0; i < active_nr;) {
struct cache_entry *ce = active_cache[i++];
^ permalink raw reply related [flat|nested] 35+ messages in thread
* Re: What's cooking in git.git (Aug 2008, #05; Tue, 19)
2008-09-02 20:02 ` What's cooking in git.git (Aug 2008, #05; Tue, 19) Junio C Hamano
@ 2008-09-02 20:43 ` Junio C Hamano
2008-09-02 22:05 ` [PATCH 0/2] Move call_depth and index_only to struct merge_options Miklos Vajna
1 sibling, 0 replies; 35+ messages in thread
From: Junio C Hamano @ 2008-09-02 20:43 UTC (permalink / raw)
To: Miklos Vajna; +Cc: git, Stephan Beyer
Junio C Hamano <gitster@pobox.com> writes:
> Miklos Vajna <vmiklos@frugalware.org> writes:
>
>> I tried to let cmd_revert() use merge_trees() only and not
>> merge_recursive_generic(), but something is fishy with it.
> Here is a partial fix to address the above issues I noticed on
> top of your version; untested.
This has a few fix-ups in addition to the one I sent earlier (not
incremental, this applies directly on top of yours, bypassing the earlier
one), and has passed the self tests.
builtin-revert.c | 47 +++++++++++++++++++++++++++++++++++++----------
1 files changed, 37 insertions(+), 10 deletions(-)
diff --git i/builtin-revert.c w/builtin-revert.c
index 3071518..8486539 100644
--- i/builtin-revert.c
+++ w/builtin-revert.c
@@ -234,16 +234,27 @@ static int index_is_dirty(void)
return !!DIFF_OPT_TST(&rev.diffopt, HAS_CHANGES);
}
+static struct tree *empty_tree(void)
+{
+ struct tree *tree = xcalloc(1, sizeof(struct tree));
+
+ tree->object.parsed = 1;
+ tree->object.type = OBJ_TREE;
+ pretend_sha1_file(NULL, 0, OBJ_TREE, tree->object.sha1);
+ return tree;
+}
+
static int revert_or_cherry_pick(int argc, const char **argv)
{
unsigned char head[20];
struct commit *base, *next, *parent;
- int i;
+ int i, index_fd, clean;
char *oneline, *reencoded_message = NULL;
const char *message, *encoding;
const char *defmsg = xstrdup(git_path("MERGE_MSG"));
struct merge_options o;
- struct tree *result;
+ struct tree *result, *next_tree, *base_tree, *head_tree;
+ static struct lock_file index_lock;
git_config(git_default_config, NULL);
me = action == REVERT ? "revert" : "cherry-pick";
@@ -254,6 +265,8 @@ static int revert_or_cherry_pick(int argc, const char **argv)
if (action == REVERT && !no_replay)
die("revert is incompatible with replay");
+ if (read_cache() < 0)
+ die("git %s: failed to read the index", me);
if (no_commit) {
/*
* We do not intend to commit immediately. We just want to
@@ -266,12 +279,12 @@ static int revert_or_cherry_pick(int argc, const char **argv)
} else {
if (get_sha1("HEAD", head))
die ("You do not have a valid HEAD");
- if (read_cache() < 0)
- die("could not read the index");
if (index_is_dirty())
die ("Dirty index: cannot %s", me);
- discard_cache();
}
+ discard_cache();
+
+ index_fd = hold_locked_index(&index_lock, 1);
if (!commit->parents) {
if (action == REVERT)
@@ -305,6 +318,10 @@ static int revert_or_cherry_pick(int argc, const char **argv)
die ("Cannot get commit message for %s",
sha1_to_hex(commit->object.sha1));
+ if (parent && parse_commit(parent) < 0)
+ die("%s: cannot parse parent commit %s",
+ me, sha1_to_hex(parent->object.sha1));
+
/*
* "commit" is an existing commit. We would want to apply
* the difference it introduces since its first parent "prev"
@@ -351,11 +368,21 @@ static int revert_or_cherry_pick(int argc, const char **argv)
init_merge_options(&o);
o.branch1 = "HEAD";
o.branch2 = oneline;
- parse_commit(next);
- parse_commit(base);
- if (!merge_trees(&o, lookup_commit_reference_gently(head, 0)->tree,
- next->tree, base->tree, &result) ||
- write_cache_as_tree(head, 0, NULL)) {
+
+ head_tree = parse_tree_indirect(head);
+ next_tree = next ? next->tree : empty_tree();
+ base_tree = base ? base->tree : empty_tree();
+
+ clean = merge_trees(&o,
+ head_tree,
+ next_tree, base_tree, &result);
+
+ if (active_cache_changed &&
+ (write_cache(index_fd, active_cache, active_nr) ||
+ commit_locked_index(&index_lock)))
+ die("%s: Unable to write new index file", me);
+
+ if (!clean) {
add_to_msg("\nConflicts:\n\n");
for (i = 0; i < active_nr;) {
struct cache_entry *ce = active_cache[i++];
^ permalink raw reply related [flat|nested] 35+ messages in thread
* [PATCH 0/2] Move call_depth and index_only to struct merge_options
2008-09-02 20:02 ` What's cooking in git.git (Aug 2008, #05; Tue, 19) Junio C Hamano
2008-09-02 20:43 ` Junio C Hamano
@ 2008-09-02 22:05 ` Miklos Vajna
2008-09-02 22:05 ` [PATCH 1/2] merge-recursive: move call_depth " Miklos Vajna
2008-09-02 22:39 ` [PATCH 0/2] Move call_depth and index_only to struct merge_options Junio C Hamano
1 sibling, 2 replies; 35+ messages in thread
From: Miklos Vajna @ 2008-09-02 22:05 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Stephan Beyer
On Tue, Sep 02, 2008 at 01:02:31PM -0700, Junio C Hamano <gitster@pobox.com> wrote:
> I found it a bit disturbing that "index_only" and "call_depth" were
> not
> part of merge_options structure.
Here are two patches to do it, on top of current mv/merge-recursive.
Both patches are pretty trivial, I think, but I split them as the second
one is larger.
Now in some cases we have function calls like
merge_file(o, one, a, b, o->branch1, o->branch2);
but I don't think we can avoid it, as the last two parameters are not
always like this (two times are like this, and once conditionally
swapped). I'm not sure if it worth to introduce a new "int swap"
parameter, just to avoid the last two one.
Miklos Vajna (2):
merge-recursive: move call_depth to struct merge_options
merge-recursive: move index_only to struct merge_options
merge-recursive.c | 164 ++++++++++++++++++++++++++---------------------------
merge-recursive.h | 13 ++++
2 files changed, 93 insertions(+), 84 deletions(-)
^ permalink raw reply [flat|nested] 35+ messages in thread
* [PATCH 1/2] merge-recursive: move call_depth to struct merge_options
2008-09-02 22:05 ` [PATCH 0/2] Move call_depth and index_only to struct merge_options Miklos Vajna
@ 2008-09-02 22:05 ` Miklos Vajna
2008-09-02 22:05 ` [PATCH 2/2] merge-recursive: move index_only " Miklos Vajna
2008-09-02 22:13 ` [PATCH] Makefile: add merge_recursive.h to LIB_H Miklos Vajna
2008-09-02 22:39 ` [PATCH 0/2] Move call_depth and index_only to struct merge_options Junio C Hamano
1 sibling, 2 replies; 35+ messages in thread
From: Miklos Vajna @ 2008-09-02 22:05 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Stephan Beyer
Signed-off-by: Miklos Vajna <vmiklos@frugalware.org>
---
merge-recursive.c | 25 ++++++++++++-------------
merge-recursive.h | 1 +
2 files changed, 13 insertions(+), 13 deletions(-)
diff --git a/merge-recursive.c b/merge-recursive.c
index 457ad84..5bb20aa 100644
--- a/merge-recursive.c
+++ b/merge-recursive.c
@@ -80,12 +80,11 @@ struct stage_data
static struct string_list current_file_set = {NULL, 0, 0, 1};
static struct string_list current_directory_set = {NULL, 0, 0, 1};
-static int call_depth = 0;
static struct strbuf obuf = STRBUF_INIT;
static int show(struct merge_options *o, int v)
{
- return (!call_depth && o->verbosity >= v) || o->verbosity >= 5;
+ return (!o->call_depth && o->verbosity >= v) || o->verbosity >= 5;
}
static void flush_output(void)
@@ -104,9 +103,9 @@ static void output(struct merge_options *o, int v, const char *fmt, ...)
if (!show(o, v))
return;
- strbuf_grow(&obuf, call_depth * 2 + 2);
- memset(obuf.buf + obuf.len, ' ', call_depth * 2);
- strbuf_setlen(&obuf, obuf.len + call_depth * 2);
+ strbuf_grow(&obuf, o->call_depth * 2 + 2);
+ memset(obuf.buf + obuf.len, ' ', o->call_depth * 2);
+ strbuf_setlen(&obuf, obuf.len + o->call_depth * 2);
va_start(ap, fmt);
len = vsnprintf(obuf.buf + obuf.len, strbuf_avail(&obuf), fmt, ap);
@@ -129,11 +128,11 @@ static void output(struct merge_options *o, int v, const char *fmt, ...)
flush_output();
}
-static void output_commit_title(struct commit *commit)
+static void output_commit_title(struct merge_options *o, struct commit *commit)
{
int i;
flush_output();
- for (i = call_depth; i--;)
+ for (i = o->call_depth; i--;)
fputs(" ", stdout);
if (commit->util)
printf("virtual %s\n", (char *)commit->util);
@@ -1230,8 +1229,8 @@ int merge_recursive(struct merge_options *o,
if (show(o, 4)) {
output(o, 4, "Merging:");
- output_commit_title(h1);
- output_commit_title(h2);
+ output_commit_title(o, h1);
+ output_commit_title(o, h2);
}
if (!ca) {
@@ -1242,7 +1241,7 @@ int merge_recursive(struct merge_options *o,
if (show(o, 5)) {
output(o, 5, "found %u common ancestor(s):", commit_list_count(ca));
for (iter = ca; iter; iter = iter->next)
- output_commit_title(iter->item);
+ output_commit_title(o, iter->item);
}
merged_common_ancestors = pop_commit(&ca);
@@ -1258,7 +1257,7 @@ int merge_recursive(struct merge_options *o,
for (iter = ca; iter; iter = iter->next) {
const char *saved_b1, *saved_b2;
- call_depth++;
+ o->call_depth++;
/*
* When the merge fails, the result contains files
* with conflict markers. The cleanness flag is
@@ -1275,14 +1274,14 @@ int merge_recursive(struct merge_options *o,
NULL, &merged_common_ancestors);
o->branch1 = saved_b1;
o->branch2 = saved_b2;
- call_depth--;
+ o->call_depth--;
if (!merged_common_ancestors)
die("merge returned no commit");
}
discard_cache();
- if (!call_depth) {
+ if (!o->call_depth) {
read_cache();
index_only = 0;
} else
diff --git a/merge-recursive.h b/merge-recursive.h
index 72f0a28..4f55374 100644
--- a/merge-recursive.h
+++ b/merge-recursive.h
@@ -9,6 +9,7 @@ struct merge_options {
int verbosity;
int diff_rename_limit;
int merge_rename_limit;
+ int call_depth;
};
/* merge_trees() but with recursive ancestor consolidation */
--
1.6.0.1
^ permalink raw reply related [flat|nested] 35+ messages in thread
* [PATCH 2/2] merge-recursive: move index_only to struct merge_options
2008-09-02 22:05 ` [PATCH 1/2] merge-recursive: move call_depth " Miklos Vajna
@ 2008-09-02 22:05 ` Miklos Vajna
2008-09-02 22:13 ` [PATCH] Makefile: add merge_recursive.h to LIB_H Miklos Vajna
1 sibling, 0 replies; 35+ messages in thread
From: Miklos Vajna @ 2008-09-02 22:05 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Stephan Beyer
Signed-off-by: Miklos Vajna <vmiklos@frugalware.org>
---
merge-recursive.c | 139 ++++++++++++++++++++++++++---------------------------
merge-recursive.h | 12 +++++
2 files changed, 80 insertions(+), 71 deletions(-)
diff --git a/merge-recursive.c b/merge-recursive.c
index 5bb20aa..4af657c 100644
--- a/merge-recursive.c
+++ b/merge-recursive.c
@@ -165,17 +165,6 @@ static int add_cacheinfo(unsigned int mode, const unsigned char *sha1,
return add_cache_entry(ce, options);
}
-/*
- * This is a global variable which is used in a number of places but
- * only written to in the 'merge' function.
- *
- * index_only == 1 => Don't leave any non-stage 0 entries in the cache and
- * don't update the working directory.
- * 0 => Leave unmerged entries in the cache and update
- * the working directory.
- */
-static int index_only = 0;
-
static void init_tree_desc_from_tree(struct tree_desc *desc, struct tree *tree)
{
parse_tree(tree);
@@ -428,10 +417,11 @@ static int remove_path(const char *name)
return ret;
}
-static int remove_file(int clean, const char *path, int no_wd)
+static int remove_file(struct merge_options *o, int clean,
+ const char *path, int no_wd)
{
- int update_cache = index_only || clean;
- int update_working_directory = !index_only && !no_wd;
+ int update_cache = o->index_only || clean;
+ int update_working_directory = !o->index_only && !no_wd;
if (update_cache) {
if (remove_file_from_cache(path))
@@ -509,13 +499,14 @@ static int make_room_for_path(const char *path)
return error(msg, path, ": perhaps a D/F conflict?");
}
-static void update_file_flags(const unsigned char *sha,
+static void update_file_flags(struct merge_options *o,
+ const unsigned char *sha,
unsigned mode,
const char *path,
int update_cache,
int update_wd)
{
- if (index_only)
+ if (o->index_only)
update_wd = 0;
if (update_wd) {
@@ -574,12 +565,13 @@ static void update_file_flags(const unsigned char *sha,
add_cacheinfo(mode, sha, path, 0, update_wd, ADD_CACHE_OK_TO_ADD);
}
-static void update_file(int clean,
+static void update_file(struct merge_options *o,
+ int clean,
const unsigned char *sha,
unsigned mode,
const char *path)
{
- update_file_flags(sha, mode, path, index_only || clean, !index_only);
+ update_file_flags(o, sha, mode, path, o->index_only || clean, !o->index_only);
}
/* Low level file merging, update and removal */
@@ -609,8 +601,9 @@ static void fill_mm(const unsigned char *sha1, mmfile_t *mm)
mm->size = size;
}
-static int merge_3way(mmbuffer_t *result_buf,
- struct diff_filespec *o,
+static int merge_3way(struct merge_options *o,
+ mmbuffer_t *result_buf,
+ struct diff_filespec *one,
struct diff_filespec *a,
struct diff_filespec *b,
const char *branch1,
@@ -623,13 +616,13 @@ static int merge_3way(mmbuffer_t *result_buf,
name1 = xstrdup(mkpath("%s:%s", branch1, a->path));
name2 = xstrdup(mkpath("%s:%s", branch2, b->path));
- fill_mm(o->sha1, &orig);
+ fill_mm(one->sha1, &orig);
fill_mm(a->sha1, &src1);
fill_mm(b->sha1, &src2);
merge_status = ll_merge(result_buf, a->path, &orig,
&src1, name1, &src2, name2,
- index_only);
+ o->index_only);
free(name1);
free(name2);
@@ -639,9 +632,12 @@ static int merge_3way(mmbuffer_t *result_buf,
return merge_status;
}
-static struct merge_file_info merge_file(struct diff_filespec *o,
- struct diff_filespec *a, struct diff_filespec *b,
- const char *branch1, const char *branch2)
+static struct merge_file_info merge_file(struct merge_options *o,
+ struct diff_filespec *one,
+ struct diff_filespec *a,
+ struct diff_filespec *b,
+ const char *branch1,
+ const char *branch2)
{
struct merge_file_info result;
result.merge = 0;
@@ -657,31 +653,31 @@ static struct merge_file_info merge_file(struct diff_filespec *o,
hashcpy(result.sha, b->sha1);
}
} else {
- if (!sha_eq(a->sha1, o->sha1) && !sha_eq(b->sha1, o->sha1))
+ if (!sha_eq(a->sha1, one->sha1) && !sha_eq(b->sha1, one->sha1))
result.merge = 1;
/*
* Merge modes
*/
- if (a->mode == b->mode || a->mode == o->mode)
+ if (a->mode == b->mode || a->mode == one->mode)
result.mode = b->mode;
else {
result.mode = a->mode;
- if (b->mode != o->mode) {
+ if (b->mode != one->mode) {
result.clean = 0;
result.merge = 1;
}
}
- if (sha_eq(a->sha1, b->sha1) || sha_eq(a->sha1, o->sha1))
+ if (sha_eq(a->sha1, b->sha1) || sha_eq(a->sha1, one->sha1))
hashcpy(result.sha, b->sha1);
- else if (sha_eq(b->sha1, o->sha1))
+ else if (sha_eq(b->sha1, one->sha1))
hashcpy(result.sha, a->sha1);
else if (S_ISREG(a->mode)) {
mmbuffer_t result_buf;
int merge_status;
- merge_status = merge_3way(&result_buf, o, a, b,
+ merge_status = merge_3way(o, &result_buf, one, a, b,
branch1, branch2);
if ((merge_status < 0) || !result_buf.ptr)
@@ -726,22 +722,22 @@ static void conflict_rename_rename(struct merge_options *o,
dst_name1 = del[delp++] = unique_path(ren1_dst, branch1);
output(o, 1, "%s is a directory in %s adding as %s instead",
ren1_dst, branch2, dst_name1);
- remove_file(0, ren1_dst, 0);
+ remove_file(o, 0, ren1_dst, 0);
}
if (string_list_has_string(¤t_directory_set, ren2_dst)) {
dst_name2 = del[delp++] = unique_path(ren2_dst, branch2);
output(o, 1, "%s is a directory in %s adding as %s instead",
ren2_dst, branch1, dst_name2);
- remove_file(0, ren2_dst, 0);
+ remove_file(o, 0, ren2_dst, 0);
}
- if (index_only) {
+ if (o->index_only) {
remove_file_from_cache(dst_name1);
remove_file_from_cache(dst_name2);
/*
* Uncomment to leave the conflicting names in the resulting tree
*
- * update_file(0, ren1->pair->two->sha1, ren1->pair->two->mode, dst_name1);
- * update_file(0, ren2->pair->two->sha1, ren2->pair->two->mode, dst_name2);
+ * update_file(o, 0, ren1->pair->two->sha1, ren1->pair->two->mode, dst_name1);
+ * update_file(o, 0, ren2->pair->two->sha1, ren2->pair->two->mode, dst_name2);
*/
} else {
update_stages(dst_name1, NULL, ren1->pair->two, NULL, 1);
@@ -757,8 +753,8 @@ static void conflict_rename_dir(struct merge_options *o,
{
char *new_path = unique_path(ren1->pair->two->path, branch1);
output(o, 1, "Renaming %s to %s instead", ren1->pair->one->path, new_path);
- remove_file(0, ren1->pair->two->path, 0);
- update_file(0, ren1->pair->two->sha1, ren1->pair->two->mode, new_path);
+ remove_file(o, 0, ren1->pair->two->path, 0);
+ update_file(o, 0, ren1->pair->two->sha1, ren1->pair->two->mode, new_path);
free(new_path);
}
@@ -773,9 +769,9 @@ static void conflict_rename_rename_2(struct merge_options *o,
output(o, 1, "Renaming %s to %s and %s to %s instead",
ren1->pair->one->path, new_path1,
ren2->pair->one->path, new_path2);
- remove_file(0, ren1->pair->two->path, 0);
- update_file(0, ren1->pair->two->sha1, ren1->pair->two->mode, new_path1);
- update_file(0, ren2->pair->two->sha1, ren2->pair->two->mode, new_path2);
+ remove_file(o, 0, ren1->pair->two->path, 0);
+ update_file(o, 0, ren1->pair->two->sha1, ren1->pair->two->mode, new_path1);
+ update_file(o, 0, ren2->pair->two->sha1, ren2->pair->two->mode, new_path2);
free(new_path2);
free(new_path1);
}
@@ -867,17 +863,18 @@ static int process_renames(struct merge_options *o,
"rename \"%s\"->\"%s\" in \"%s\"%s",
src, ren1_dst, branch1,
src, ren2_dst, branch2,
- index_only ? " (left unresolved)": "");
- if (index_only) {
+ o->index_only ? " (left unresolved)": "");
+ if (o->index_only) {
remove_file_from_cache(src);
- update_file(0, ren1->pair->one->sha1,
+ update_file(o, 0, ren1->pair->one->sha1,
ren1->pair->one->mode, src);
}
conflict_rename_rename(o, ren1, branch1, ren2, branch2);
} else {
struct merge_file_info mfi;
- remove_file(1, ren1_src, 1);
- mfi = merge_file(ren1->pair->one,
+ remove_file(o, 1, ren1_src, 1);
+ mfi = merge_file(o,
+ ren1->pair->one,
ren1->pair->two,
ren2->pair->two,
branch1,
@@ -893,14 +890,14 @@ static int process_renames(struct merge_options *o,
ren1_dst);
clean_merge = 0;
- if (!index_only)
+ if (!o->index_only)
update_stages(ren1_dst,
ren1->pair->one,
ren1->pair->two,
ren2->pair->two,
1 /* clear */);
}
- update_file(mfi.clean, mfi.sha, mfi.mode, ren1_dst);
+ update_file(o, mfi.clean, mfi.sha, mfi.mode, ren1_dst);
}
} else {
/* Renamed in 1, maybe changed in 2 */
@@ -909,7 +906,7 @@ static int process_renames(struct merge_options *o,
struct diff_filespec src_other, dst_other;
int try_merge, stage = a_renames == renames1 ? 3: 2;
- remove_file(1, ren1_src, index_only || stage == 3);
+ remove_file(o, 1, ren1_src, o->index_only || stage == 3);
hashcpy(src_other.sha1, ren1->src_entry->stages[stage].sha);
src_other.mode = ren1->src_entry->stages[stage].mode;
@@ -931,7 +928,7 @@ static int process_renames(struct merge_options *o,
"and deleted in %s",
ren1_src, ren1_dst, branch1,
branch2);
- update_file(0, ren1->pair->two->sha1, ren1->pair->two->mode, ren1_dst);
+ update_file(o, 0, ren1->pair->two->sha1, ren1->pair->two->mode, ren1_dst);
} else if (!sha_eq(dst_other.sha1, null_sha1)) {
const char *new_path;
clean_merge = 0;
@@ -942,7 +939,7 @@ static int process_renames(struct merge_options *o,
ren1_dst, branch2);
new_path = unique_path(ren1_dst, branch2);
output(o, 1, "Adding as %s instead", new_path);
- update_file(0, dst_other.sha1, dst_other.mode, new_path);
+ update_file(o, 0, dst_other.sha1, dst_other.mode, new_path);
} else if ((item = string_list_lookup(ren1_dst, renames2Dst))) {
ren2 = item->util;
clean_merge = 0;
@@ -969,7 +966,7 @@ static int process_renames(struct merge_options *o,
b = ren1->pair->two;
a = &src_other;
}
- mfi = merge_file(one, a, b,
+ mfi = merge_file(o, one, a, b,
o->branch1, o->branch2);
if (mfi.clean &&
@@ -991,11 +988,11 @@ static int process_renames(struct merge_options *o,
ren1_dst);
clean_merge = 0;
- if (!index_only)
+ if (!o->index_only)
update_stages(ren1_dst,
one, a, b, 1);
}
- update_file(mfi.clean, mfi.sha, mfi.mode, ren1_dst);
+ update_file(o, mfi.clean, mfi.sha, mfi.mode, ren1_dst);
}
}
}
@@ -1037,7 +1034,7 @@ static int process_entry(struct merge_options *o,
if (a_sha)
output(o, 2, "Removing %s", path);
/* do not touch working file if it did not exist */
- remove_file(1, path, !a_sha);
+ remove_file(o, 1, path, !a_sha);
} else {
/* Deleted in one and changed in the other */
clean_merge = 0;
@@ -1046,13 +1043,13 @@ static int process_entry(struct merge_options *o,
"and modified in %s. Version %s of %s left in tree.",
path, o->branch1,
o->branch2, o->branch2, path);
- update_file(0, b_sha, b_mode, path);
+ update_file(o, 0, b_sha, b_mode, path);
} else {
output(o, 1, "CONFLICT (delete/modify): %s deleted in %s "
"and modified in %s. Version %s of %s left in tree.",
path, o->branch2,
o->branch1, o->branch1, path);
- update_file(0, a_sha, a_mode, path);
+ update_file(o, 0, a_sha, a_mode, path);
}
}
@@ -1084,11 +1081,11 @@ static int process_entry(struct merge_options *o,
output(o, 1, "CONFLICT (%s): There is a directory with name %s in %s. "
"Adding %s as %s",
conf, path, other_branch, path, new_path);
- remove_file(0, path, 0);
- update_file(0, sha, mode, new_path);
+ remove_file(o, 0, path, 0);
+ update_file(o, 0, sha, mode, new_path);
} else {
output(o, 2, "Adding %s", path);
- update_file(1, sha, mode, path);
+ update_file(o, 1, sha, mode, path);
}
} else if (a_sha && b_sha) {
/* Case C: Added in both (check for same permissions) and */
@@ -1110,12 +1107,12 @@ static int process_entry(struct merge_options *o,
hashcpy(b.sha1, b_sha);
b.mode = b_mode;
- mfi = merge_file(&one, &a, &b,
+ mfi = merge_file(o, &one, &a, &b,
o->branch1, o->branch2);
clean_merge = mfi.clean;
if (mfi.clean)
- update_file(1, mfi.sha, mfi.mode, path);
+ update_file(o, 1, mfi.sha, mfi.mode, path);
else if (S_ISGITLINK(mfi.mode))
output(o, 1, "CONFLICT (submodule): Merge conflict in %s "
"- needs %s", path, sha1_to_hex(b.sha1));
@@ -1123,10 +1120,10 @@ static int process_entry(struct merge_options *o,
output(o, 1, "CONFLICT (%s): Merge conflict in %s",
reason, path);
- if (index_only)
- update_file(0, mfi.sha, mfi.mode, path);
+ if (o->index_only)
+ update_file(o, 0, mfi.sha, mfi.mode, path);
else
- update_file_flags(mfi.sha, mfi.mode, path,
+ update_file_flags(o, mfi.sha, mfi.mode, path,
0 /* update_cache */, 1 /* update_working_directory */);
}
} else if (!o_sha && !a_sha && !b_sha) {
@@ -1134,7 +1131,7 @@ static int process_entry(struct merge_options *o,
* this entry was deleted altogether. a_mode == 0 means
* we had that path and want to actively remove it.
*/
- remove_file(1, path, !a_mode);
+ remove_file(o, 1, path, !a_mode);
} else
die("Fatal merge failure, shouldn't happen.");
@@ -1160,7 +1157,7 @@ int merge_trees(struct merge_options *o,
return 1;
}
- code = git_merge_trees(index_only, common, head, merge);
+ code = git_merge_trees(o->index_only, common, head, merge);
if (code != 0)
die("merging of trees %s and %s failed",
@@ -1195,7 +1192,7 @@ int merge_trees(struct merge_options *o,
else
clean = 1;
- if (index_only)
+ if (o->index_only)
*result = write_tree_from_memory(o);
return clean;
@@ -1283,14 +1280,14 @@ int merge_recursive(struct merge_options *o,
discard_cache();
if (!o->call_depth) {
read_cache();
- index_only = 0;
+ o->index_only = 0;
} else
- index_only = 1;
+ o->index_only = 1;
clean = merge_trees(o, h1->tree, h2->tree, merged_common_ancestors->tree,
&mrtree);
- if (index_only) {
+ if (o->index_only) {
*result = make_virtual_commit(mrtree, "merged tree");
commit_list_insert(h1, &(*result)->parents);
commit_list_insert(h2, &(*result)->parents->next);
diff --git a/merge-recursive.h b/merge-recursive.h
index 4f55374..6622c3e 100644
--- a/merge-recursive.h
+++ b/merge-recursive.h
@@ -10,6 +10,18 @@ struct merge_options {
int diff_rename_limit;
int merge_rename_limit;
int call_depth;
+ /*
+ * This variable is used in a number of places but only written
+ * to in the 'merge_recursive' function.
+ *
+ * index_only == 1 => Don't leave any non-stage 0 entries in
+ * the cache and don't update the working
+ * directory.
+ * 0 => Leave unmerged entries in the cache and
+ * update the working directory.
+ */
+ int index_only;
+
};
/* merge_trees() but with recursive ancestor consolidation */
--
1.6.0.1
^ permalink raw reply related [flat|nested] 35+ messages in thread
* [PATCH] Makefile: add merge_recursive.h to LIB_H
2008-09-02 22:05 ` [PATCH 1/2] merge-recursive: move call_depth " Miklos Vajna
2008-09-02 22:05 ` [PATCH 2/2] merge-recursive: move index_only " Miklos Vajna
@ 2008-09-02 22:13 ` Miklos Vajna
2008-09-02 22:39 ` Junio C Hamano
1 sibling, 1 reply; 35+ messages in thread
From: Miklos Vajna @ 2008-09-02 22:13 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Stephan Beyer
When modifying merge-recursive.h, for example builtin-merge-recursive.c
have to be recompiled which was not true till now, causing various
runtime errors using an incremental build.
Signed-off-by: Miklos Vajna <vmiklos@frugalware.org>
---
On Wed, Sep 03, 2008 at 12:05:32AM +0200, Miklos Vajna <vmiklos@frugalware.org> wrote:
> Subject: Re: [PATCH 1/2] merge-recursive: move call_depth to struct
> merge_options
While testing this one, I got some weird errors, finally they gone after
a 'touch builtin-merge-recursive.c', I guess this is the right fix.
Makefile | 1 +
1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/Makefile b/Makefile
index f697618..bec6a84 100644
--- a/Makefile
+++ b/Makefile
@@ -362,6 +362,7 @@ LIB_H += list-objects.h
LIB_H += ll-merge.h
LIB_H += log-tree.h
LIB_H += mailmap.h
+LIB_H += merge_recursive.h
LIB_H += object.h
LIB_H += pack.h
LIB_H += pack-refs.h
--
1.6.0.1
^ permalink raw reply related [flat|nested] 35+ messages in thread
* Re: [PATCH 0/2] Move call_depth and index_only to struct merge_options
2008-09-02 22:05 ` [PATCH 0/2] Move call_depth and index_only to struct merge_options Miklos Vajna
2008-09-02 22:05 ` [PATCH 1/2] merge-recursive: move call_depth " Miklos Vajna
@ 2008-09-02 22:39 ` Junio C Hamano
2008-09-03 0:16 ` [PATCH] merge-recursive: get rid of the index_only global variable Miklos Vajna
2008-09-03 0:39 ` [PATCH] merge-recursive: move the global obuf to struct merge_options Miklos Vajna
1 sibling, 2 replies; 35+ messages in thread
From: Junio C Hamano @ 2008-09-02 22:39 UTC (permalink / raw)
To: Miklos Vajna; +Cc: git, Stephan Beyer
Miklos Vajna <vmiklos@frugalware.org> writes:
> On Tue, Sep 02, 2008 at 01:02:31PM -0700, Junio C Hamano <gitster@pobox.com> wrote:
>> I found it a bit disturbing that "index_only" and "call_depth" were
>> not
>> part of merge_options structure.
>
> Here are two patches to do it, on top of current mv/merge-recursive.
I suspected that it always holds that "index_only === !!call_depth".
Shouldn't strbuf obuf be part of the merge_options structure that
describes the current call status?
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH] Makefile: add merge_recursive.h to LIB_H
2008-09-02 22:13 ` [PATCH] Makefile: add merge_recursive.h to LIB_H Miklos Vajna
@ 2008-09-02 22:39 ` Junio C Hamano
2008-09-02 23:49 ` Miklos Vajna
0 siblings, 1 reply; 35+ messages in thread
From: Junio C Hamano @ 2008-09-02 22:39 UTC (permalink / raw)
To: Miklos Vajna; +Cc: git, Stephan Beyer
Miklos Vajna <vmiklos@frugalware.org> writes:
> When modifying merge-recursive.h, for example builtin-merge-recursive.c
> have to be recompiled which was not true till now, causing various
> runtime errors using an incremental build.
>
> Signed-off-by: Miklos Vajna <vmiklos@frugalware.org>
> ---
>
> On Wed, Sep 03, 2008 at 12:05:32AM +0200, Miklos Vajna <vmiklos@frugalware.org> wrote:
>> Subject: Re: [PATCH 1/2] merge-recursive: move call_depth to struct
>> merge_options
>
> While testing this one, I got some weird errors, finally they gone after
> a 'touch builtin-merge-recursive.c', I guess this is the right fix.
Thanks.
^ permalink raw reply [flat|nested] 35+ messages in thread
* [PATCH] Makefile: add merge_recursive.h to LIB_H
2008-09-02 22:39 ` Junio C Hamano
@ 2008-09-02 23:49 ` Miklos Vajna
0 siblings, 0 replies; 35+ messages in thread
From: Miklos Vajna @ 2008-09-02 23:49 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Stephan Beyer
When modifying merge-recursive.h, for example builtin-merge-recursive.c
have to be recompiled which was not true till now, causing various
runtime errors using an incremental build.
Signed-off-by: Miklos Vajna <vmiklos@frugalware.org>
---
n Tue, Sep 02, 2008 at 03:39:57PM -0700, Junio C Hamano <gitster@pobox.com> wrote:
> > While testing this one, I got some weird errors, finally they gone
> > after
> > a 'touch builtin-merge-recursive.c', I guess this is the right fix.
>
> Thanks.
Ugh, typo. It is called 'merge-recursive.h'.
Sorry.
Makefile | 1 +
1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/Makefile b/Makefile
index f697618..1647831 100644
--- a/Makefile
+++ b/Makefile
@@ -362,6 +362,7 @@ LIB_H += list-objects.h
LIB_H += ll-merge.h
LIB_H += log-tree.h
LIB_H += mailmap.h
+LIB_H += merge-recursive.h
LIB_H += object.h
LIB_H += pack.h
LIB_H += pack-refs.h
--
1.6.0.1
^ permalink raw reply related [flat|nested] 35+ messages in thread
* [PATCH] merge-recursive: get rid of the index_only global variable
2008-09-02 22:39 ` [PATCH 0/2] Move call_depth and index_only to struct merge_options Junio C Hamano
@ 2008-09-03 0:16 ` Miklos Vajna
2008-09-03 0:39 ` [PATCH] merge-recursive: move the global obuf to struct merge_options Miklos Vajna
1 sibling, 0 replies; 35+ messages in thread
From: Miklos Vajna @ 2008-09-03 0:16 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Stephan Beyer
struct merge_options as already a call_depth member, where index_only ==
!!call_depth. We always use index_only as a condition, so we can just
use call_depth instead of index_only.
Signed-off-by: Miklos Vajna <vmiklos@frugalware.org>
---
On Tue, Sep 02, 2008 at 03:39:33PM -0700, Junio C Hamano <gitster@pobox.com> wrote:
> I suspected that it always holds that "index_only === !!call_depth".
Uhm, yes. Here is an updated version that just removes the global
index_only and does not touch the header.
The patch is still large, as I still needed to introduce struct
merge_options as the first parameter in several functions.
merge-recursive.c | 140 +++++++++++++++++++++++++---------------------------
1 files changed, 67 insertions(+), 73 deletions(-)
diff --git a/merge-recursive.c b/merge-recursive.c
index 5bb20aa..c426589 100644
--- a/merge-recursive.c
+++ b/merge-recursive.c
@@ -165,17 +165,6 @@ static int add_cacheinfo(unsigned int mode, const unsigned char *sha1,
return add_cache_entry(ce, options);
}
-/*
- * This is a global variable which is used in a number of places but
- * only written to in the 'merge' function.
- *
- * index_only == 1 => Don't leave any non-stage 0 entries in the cache and
- * don't update the working directory.
- * 0 => Leave unmerged entries in the cache and update
- * the working directory.
- */
-static int index_only = 0;
-
static void init_tree_desc_from_tree(struct tree_desc *desc, struct tree *tree)
{
parse_tree(tree);
@@ -428,10 +417,11 @@ static int remove_path(const char *name)
return ret;
}
-static int remove_file(int clean, const char *path, int no_wd)
+static int remove_file(struct merge_options *o, int clean,
+ const char *path, int no_wd)
{
- int update_cache = index_only || clean;
- int update_working_directory = !index_only && !no_wd;
+ int update_cache = o->call_depth || clean;
+ int update_working_directory = !o->call_depth && !no_wd;
if (update_cache) {
if (remove_file_from_cache(path))
@@ -509,13 +499,14 @@ static int make_room_for_path(const char *path)
return error(msg, path, ": perhaps a D/F conflict?");
}
-static void update_file_flags(const unsigned char *sha,
+static void update_file_flags(struct merge_options *o,
+ const unsigned char *sha,
unsigned mode,
const char *path,
int update_cache,
int update_wd)
{
- if (index_only)
+ if (o->call_depth)
update_wd = 0;
if (update_wd) {
@@ -574,12 +565,13 @@ static void update_file_flags(const unsigned char *sha,
add_cacheinfo(mode, sha, path, 0, update_wd, ADD_CACHE_OK_TO_ADD);
}
-static void update_file(int clean,
+static void update_file(struct merge_options *o,
+ int clean,
const unsigned char *sha,
unsigned mode,
const char *path)
{
- update_file_flags(sha, mode, path, index_only || clean, !index_only);
+ update_file_flags(o, sha, mode, path, o->call_depth || clean, !o->call_depth);
}
/* Low level file merging, update and removal */
@@ -609,8 +601,9 @@ static void fill_mm(const unsigned char *sha1, mmfile_t *mm)
mm->size = size;
}
-static int merge_3way(mmbuffer_t *result_buf,
- struct diff_filespec *o,
+static int merge_3way(struct merge_options *o,
+ mmbuffer_t *result_buf,
+ struct diff_filespec *one,
struct diff_filespec *a,
struct diff_filespec *b,
const char *branch1,
@@ -623,13 +616,13 @@ static int merge_3way(mmbuffer_t *result_buf,
name1 = xstrdup(mkpath("%s:%s", branch1, a->path));
name2 = xstrdup(mkpath("%s:%s", branch2, b->path));
- fill_mm(o->sha1, &orig);
+ fill_mm(one->sha1, &orig);
fill_mm(a->sha1, &src1);
fill_mm(b->sha1, &src2);
merge_status = ll_merge(result_buf, a->path, &orig,
&src1, name1, &src2, name2,
- index_only);
+ o->call_depth);
free(name1);
free(name2);
@@ -639,9 +632,12 @@ static int merge_3way(mmbuffer_t *result_buf,
return merge_status;
}
-static struct merge_file_info merge_file(struct diff_filespec *o,
- struct diff_filespec *a, struct diff_filespec *b,
- const char *branch1, const char *branch2)
+static struct merge_file_info merge_file(struct merge_options *o,
+ struct diff_filespec *one,
+ struct diff_filespec *a,
+ struct diff_filespec *b,
+ const char *branch1,
+ const char *branch2)
{
struct merge_file_info result;
result.merge = 0;
@@ -657,31 +653,31 @@ static struct merge_file_info merge_file(struct diff_filespec *o,
hashcpy(result.sha, b->sha1);
}
} else {
- if (!sha_eq(a->sha1, o->sha1) && !sha_eq(b->sha1, o->sha1))
+ if (!sha_eq(a->sha1, one->sha1) && !sha_eq(b->sha1, one->sha1))
result.merge = 1;
/*
* Merge modes
*/
- if (a->mode == b->mode || a->mode == o->mode)
+ if (a->mode == b->mode || a->mode == one->mode)
result.mode = b->mode;
else {
result.mode = a->mode;
- if (b->mode != o->mode) {
+ if (b->mode != one->mode) {
result.clean = 0;
result.merge = 1;
}
}
- if (sha_eq(a->sha1, b->sha1) || sha_eq(a->sha1, o->sha1))
+ if (sha_eq(a->sha1, b->sha1) || sha_eq(a->sha1, one->sha1))
hashcpy(result.sha, b->sha1);
- else if (sha_eq(b->sha1, o->sha1))
+ else if (sha_eq(b->sha1, one->sha1))
hashcpy(result.sha, a->sha1);
else if (S_ISREG(a->mode)) {
mmbuffer_t result_buf;
int merge_status;
- merge_status = merge_3way(&result_buf, o, a, b,
+ merge_status = merge_3way(o, &result_buf, one, a, b,
branch1, branch2);
if ((merge_status < 0) || !result_buf.ptr)
@@ -726,22 +722,22 @@ static void conflict_rename_rename(struct merge_options *o,
dst_name1 = del[delp++] = unique_path(ren1_dst, branch1);
output(o, 1, "%s is a directory in %s adding as %s instead",
ren1_dst, branch2, dst_name1);
- remove_file(0, ren1_dst, 0);
+ remove_file(o, 0, ren1_dst, 0);
}
if (string_list_has_string(¤t_directory_set, ren2_dst)) {
dst_name2 = del[delp++] = unique_path(ren2_dst, branch2);
output(o, 1, "%s is a directory in %s adding as %s instead",
ren2_dst, branch1, dst_name2);
- remove_file(0, ren2_dst, 0);
+ remove_file(o, 0, ren2_dst, 0);
}
- if (index_only) {
+ if (o->call_depth) {
remove_file_from_cache(dst_name1);
remove_file_from_cache(dst_name2);
/*
* Uncomment to leave the conflicting names in the resulting tree
*
- * update_file(0, ren1->pair->two->sha1, ren1->pair->two->mode, dst_name1);
- * update_file(0, ren2->pair->two->sha1, ren2->pair->two->mode, dst_name2);
+ * update_file(o, 0, ren1->pair->two->sha1, ren1->pair->two->mode, dst_name1);
+ * update_file(o, 0, ren2->pair->two->sha1, ren2->pair->two->mode, dst_name2);
*/
} else {
update_stages(dst_name1, NULL, ren1->pair->two, NULL, 1);
@@ -757,8 +753,8 @@ static void conflict_rename_dir(struct merge_options *o,
{
char *new_path = unique_path(ren1->pair->two->path, branch1);
output(o, 1, "Renaming %s to %s instead", ren1->pair->one->path, new_path);
- remove_file(0, ren1->pair->two->path, 0);
- update_file(0, ren1->pair->two->sha1, ren1->pair->two->mode, new_path);
+ remove_file(o, 0, ren1->pair->two->path, 0);
+ update_file(o, 0, ren1->pair->two->sha1, ren1->pair->two->mode, new_path);
free(new_path);
}
@@ -773,9 +769,9 @@ static void conflict_rename_rename_2(struct merge_options *o,
output(o, 1, "Renaming %s to %s and %s to %s instead",
ren1->pair->one->path, new_path1,
ren2->pair->one->path, new_path2);
- remove_file(0, ren1->pair->two->path, 0);
- update_file(0, ren1->pair->two->sha1, ren1->pair->two->mode, new_path1);
- update_file(0, ren2->pair->two->sha1, ren2->pair->two->mode, new_path2);
+ remove_file(o, 0, ren1->pair->two->path, 0);
+ update_file(o, 0, ren1->pair->two->sha1, ren1->pair->two->mode, new_path1);
+ update_file(o, 0, ren2->pair->two->sha1, ren2->pair->two->mode, new_path2);
free(new_path2);
free(new_path1);
}
@@ -867,17 +863,18 @@ static int process_renames(struct merge_options *o,
"rename \"%s\"->\"%s\" in \"%s\"%s",
src, ren1_dst, branch1,
src, ren2_dst, branch2,
- index_only ? " (left unresolved)": "");
- if (index_only) {
+ o->call_depth ? " (left unresolved)": "");
+ if (o->call_depth) {
remove_file_from_cache(src);
- update_file(0, ren1->pair->one->sha1,
+ update_file(o, 0, ren1->pair->one->sha1,
ren1->pair->one->mode, src);
}
conflict_rename_rename(o, ren1, branch1, ren2, branch2);
} else {
struct merge_file_info mfi;
- remove_file(1, ren1_src, 1);
- mfi = merge_file(ren1->pair->one,
+ remove_file(o, 1, ren1_src, 1);
+ mfi = merge_file(o,
+ ren1->pair->one,
ren1->pair->two,
ren2->pair->two,
branch1,
@@ -893,14 +890,14 @@ static int process_renames(struct merge_options *o,
ren1_dst);
clean_merge = 0;
- if (!index_only)
+ if (!o->call_depth)
update_stages(ren1_dst,
ren1->pair->one,
ren1->pair->two,
ren2->pair->two,
1 /* clear */);
}
- update_file(mfi.clean, mfi.sha, mfi.mode, ren1_dst);
+ update_file(o, mfi.clean, mfi.sha, mfi.mode, ren1_dst);
}
} else {
/* Renamed in 1, maybe changed in 2 */
@@ -909,7 +906,7 @@ static int process_renames(struct merge_options *o,
struct diff_filespec src_other, dst_other;
int try_merge, stage = a_renames == renames1 ? 3: 2;
- remove_file(1, ren1_src, index_only || stage == 3);
+ remove_file(o, 1, ren1_src, o->call_depth || stage == 3);
hashcpy(src_other.sha1, ren1->src_entry->stages[stage].sha);
src_other.mode = ren1->src_entry->stages[stage].mode;
@@ -931,7 +928,7 @@ static int process_renames(struct merge_options *o,
"and deleted in %s",
ren1_src, ren1_dst, branch1,
branch2);
- update_file(0, ren1->pair->two->sha1, ren1->pair->two->mode, ren1_dst);
+ update_file(o, 0, ren1->pair->two->sha1, ren1->pair->two->mode, ren1_dst);
} else if (!sha_eq(dst_other.sha1, null_sha1)) {
const char *new_path;
clean_merge = 0;
@@ -942,7 +939,7 @@ static int process_renames(struct merge_options *o,
ren1_dst, branch2);
new_path = unique_path(ren1_dst, branch2);
output(o, 1, "Adding as %s instead", new_path);
- update_file(0, dst_other.sha1, dst_other.mode, new_path);
+ update_file(o, 0, dst_other.sha1, dst_other.mode, new_path);
} else if ((item = string_list_lookup(ren1_dst, renames2Dst))) {
ren2 = item->util;
clean_merge = 0;
@@ -969,7 +966,7 @@ static int process_renames(struct merge_options *o,
b = ren1->pair->two;
a = &src_other;
}
- mfi = merge_file(one, a, b,
+ mfi = merge_file(o, one, a, b,
o->branch1, o->branch2);
if (mfi.clean &&
@@ -991,11 +988,11 @@ static int process_renames(struct merge_options *o,
ren1_dst);
clean_merge = 0;
- if (!index_only)
+ if (!o->call_depth)
update_stages(ren1_dst,
one, a, b, 1);
}
- update_file(mfi.clean, mfi.sha, mfi.mode, ren1_dst);
+ update_file(o, mfi.clean, mfi.sha, mfi.mode, ren1_dst);
}
}
}
@@ -1037,7 +1034,7 @@ static int process_entry(struct merge_options *o,
if (a_sha)
output(o, 2, "Removing %s", path);
/* do not touch working file if it did not exist */
- remove_file(1, path, !a_sha);
+ remove_file(o, 1, path, !a_sha);
} else {
/* Deleted in one and changed in the other */
clean_merge = 0;
@@ -1046,13 +1043,13 @@ static int process_entry(struct merge_options *o,
"and modified in %s. Version %s of %s left in tree.",
path, o->branch1,
o->branch2, o->branch2, path);
- update_file(0, b_sha, b_mode, path);
+ update_file(o, 0, b_sha, b_mode, path);
} else {
output(o, 1, "CONFLICT (delete/modify): %s deleted in %s "
"and modified in %s. Version %s of %s left in tree.",
path, o->branch2,
o->branch1, o->branch1, path);
- update_file(0, a_sha, a_mode, path);
+ update_file(o, 0, a_sha, a_mode, path);
}
}
@@ -1084,11 +1081,11 @@ static int process_entry(struct merge_options *o,
output(o, 1, "CONFLICT (%s): There is a directory with name %s in %s. "
"Adding %s as %s",
conf, path, other_branch, path, new_path);
- remove_file(0, path, 0);
- update_file(0, sha, mode, new_path);
+ remove_file(o, 0, path, 0);
+ update_file(o, 0, sha, mode, new_path);
} else {
output(o, 2, "Adding %s", path);
- update_file(1, sha, mode, path);
+ update_file(o, 1, sha, mode, path);
}
} else if (a_sha && b_sha) {
/* Case C: Added in both (check for same permissions) and */
@@ -1110,12 +1107,12 @@ static int process_entry(struct merge_options *o,
hashcpy(b.sha1, b_sha);
b.mode = b_mode;
- mfi = merge_file(&one, &a, &b,
+ mfi = merge_file(o, &one, &a, &b,
o->branch1, o->branch2);
clean_merge = mfi.clean;
if (mfi.clean)
- update_file(1, mfi.sha, mfi.mode, path);
+ update_file(o, 1, mfi.sha, mfi.mode, path);
else if (S_ISGITLINK(mfi.mode))
output(o, 1, "CONFLICT (submodule): Merge conflict in %s "
"- needs %s", path, sha1_to_hex(b.sha1));
@@ -1123,10 +1120,10 @@ static int process_entry(struct merge_options *o,
output(o, 1, "CONFLICT (%s): Merge conflict in %s",
reason, path);
- if (index_only)
- update_file(0, mfi.sha, mfi.mode, path);
+ if (o->call_depth)
+ update_file(o, 0, mfi.sha, mfi.mode, path);
else
- update_file_flags(mfi.sha, mfi.mode, path,
+ update_file_flags(o, mfi.sha, mfi.mode, path,
0 /* update_cache */, 1 /* update_working_directory */);
}
} else if (!o_sha && !a_sha && !b_sha) {
@@ -1134,7 +1131,7 @@ static int process_entry(struct merge_options *o,
* this entry was deleted altogether. a_mode == 0 means
* we had that path and want to actively remove it.
*/
- remove_file(1, path, !a_mode);
+ remove_file(o, 1, path, !a_mode);
} else
die("Fatal merge failure, shouldn't happen.");
@@ -1160,7 +1157,7 @@ int merge_trees(struct merge_options *o,
return 1;
}
- code = git_merge_trees(index_only, common, head, merge);
+ code = git_merge_trees(o->call_depth, common, head, merge);
if (code != 0)
die("merging of trees %s and %s failed",
@@ -1195,7 +1192,7 @@ int merge_trees(struct merge_options *o,
else
clean = 1;
- if (index_only)
+ if (o->call_depth)
*result = write_tree_from_memory(o);
return clean;
@@ -1281,16 +1278,13 @@ int merge_recursive(struct merge_options *o,
}
discard_cache();
- if (!o->call_depth) {
+ if (!o->call_depth)
read_cache();
- index_only = 0;
- } else
- index_only = 1;
clean = merge_trees(o, h1->tree, h2->tree, merged_common_ancestors->tree,
&mrtree);
- if (index_only) {
+ if (o->call_depth) {
*result = make_virtual_commit(mrtree, "merged tree");
commit_list_insert(h1, &(*result)->parents);
commit_list_insert(h2, &(*result)->parents->next);
--
1.6.0.1
^ permalink raw reply related [flat|nested] 35+ messages in thread
* [PATCH] merge-recursive: move the global obuf to struct merge_options
2008-09-02 22:39 ` [PATCH 0/2] Move call_depth and index_only to struct merge_options Junio C Hamano
2008-09-03 0:16 ` [PATCH] merge-recursive: get rid of the index_only global variable Miklos Vajna
@ 2008-09-03 0:39 ` Miklos Vajna
2008-09-03 17:34 ` Miklos Vajna
1 sibling, 1 reply; 35+ messages in thread
From: Miklos Vajna @ 2008-09-03 0:39 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Stephan Beyer
Signed-off-by: Miklos Vajna <vmiklos@frugalware.org>
---
On Tue, Sep 02, 2008 at 03:39:33PM -0700, Junio C Hamano <gitster@pobox.com> wrote:
> Shouldn't strbuf obuf be part of the merge_options structure that
> describes the current call status?
It can be done, see below. :-)
BTW, there are 3 leftovers: make_virtual_commit()'s virtual_id and the
global current_file_set/current_directory_set. I guess all of them could
be moved to merge_options as well. (I don't have more time today to do
so, but I can do it tomorrow, unless you see some fundamental problem
with it.)
merge-recursive.c | 37 ++++++++++++++++++-------------------
merge-recursive.h | 1 +
2 files changed, 19 insertions(+), 19 deletions(-)
diff --git a/merge-recursive.c b/merge-recursive.c
index c426589..d4f12d0 100644
--- a/merge-recursive.c
+++ b/merge-recursive.c
@@ -80,18 +80,16 @@ struct stage_data
static struct string_list current_file_set = {NULL, 0, 0, 1};
static struct string_list current_directory_set = {NULL, 0, 0, 1};
-static struct strbuf obuf = STRBUF_INIT;
-
static int show(struct merge_options *o, int v)
{
return (!o->call_depth && o->verbosity >= v) || o->verbosity >= 5;
}
-static void flush_output(void)
+static void flush_output(struct merge_options *o)
{
- if (obuf.len) {
- fputs(obuf.buf, stdout);
- strbuf_reset(&obuf);
+ if (o->obuf.len) {
+ fputs(o->obuf.buf, stdout);
+ strbuf_reset(&o->obuf);
}
}
@@ -103,35 +101,35 @@ static void output(struct merge_options *o, int v, const char *fmt, ...)
if (!show(o, v))
return;
- strbuf_grow(&obuf, o->call_depth * 2 + 2);
- memset(obuf.buf + obuf.len, ' ', o->call_depth * 2);
- strbuf_setlen(&obuf, obuf.len + o->call_depth * 2);
+ strbuf_grow(&o->obuf, o->call_depth * 2 + 2);
+ memset(o->obuf.buf + o->obuf.len, ' ', o->call_depth * 2);
+ strbuf_setlen(&o->obuf, o->obuf.len + o->call_depth * 2);
va_start(ap, fmt);
- len = vsnprintf(obuf.buf + obuf.len, strbuf_avail(&obuf), fmt, ap);
+ len = vsnprintf(o->obuf.buf + o->obuf.len, strbuf_avail(&o->obuf), fmt, ap);
va_end(ap);
if (len < 0)
len = 0;
- if (len >= strbuf_avail(&obuf)) {
- strbuf_grow(&obuf, len + 2);
+ if (len >= strbuf_avail(&o->obuf)) {
+ strbuf_grow(&o->obuf, len + 2);
va_start(ap, fmt);
- len = vsnprintf(obuf.buf + obuf.len, strbuf_avail(&obuf), fmt, ap);
+ len = vsnprintf(o->obuf.buf + o->obuf.len, strbuf_avail(&o->obuf), fmt, ap);
va_end(ap);
- if (len >= strbuf_avail(&obuf)) {
+ if (len >= strbuf_avail(&o->obuf)) {
die("this should not happen, your snprintf is broken");
}
}
- strbuf_setlen(&obuf, obuf.len + len);
- strbuf_add(&obuf, "\n", 1);
+ strbuf_setlen(&o->obuf, o->obuf.len + len);
+ strbuf_add(&o->obuf, "\n", 1);
if (!o->buffer_output)
- flush_output();
+ flush_output(o);
}
static void output_commit_title(struct merge_options *o, struct commit *commit)
{
int i;
- flush_output();
+ flush_output(o);
for (i = o->call_depth; i--;)
fputs(" ", stdout);
if (commit->util)
@@ -1289,7 +1287,7 @@ int merge_recursive(struct merge_options *o,
commit_list_insert(h1, &(*result)->parents);
commit_list_insert(h2, &(*result)->parents->next);
}
- flush_output();
+ flush_output(o);
return clean;
}
@@ -1375,4 +1373,5 @@ void init_merge_options(struct merge_options *o)
strtol(getenv("GIT_MERGE_VERBOSITY"), NULL, 10);
if (o->verbosity >= 5)
o->buffer_output = 0;
+ strbuf_init(&o->obuf, 0);
}
diff --git a/merge-recursive.h b/merge-recursive.h
index 4f55374..be84d9b 100644
--- a/merge-recursive.h
+++ b/merge-recursive.h
@@ -10,6 +10,7 @@ struct merge_options {
int diff_rename_limit;
int merge_rename_limit;
int call_depth;
+ struct strbuf obuf;
};
/* merge_trees() but with recursive ancestor consolidation */
--
1.6.0.1
^ permalink raw reply related [flat|nested] 35+ messages in thread
* Re: [PATCH] merge-recursive: move the global obuf to struct merge_options
2008-09-03 0:39 ` [PATCH] merge-recursive: move the global obuf to struct merge_options Miklos Vajna
@ 2008-09-03 17:34 ` Miklos Vajna
2008-09-03 17:34 ` [PATCH 1/2] merge-recursive: move current_{file,directory}_set " Miklos Vajna
2008-09-04 19:05 ` [PATCH] merge-recursive: move the global obuf to struct merge_options Junio C Hamano
0 siblings, 2 replies; 35+ messages in thread
From: Miklos Vajna @ 2008-09-03 17:34 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Stephan Beyer
On Wed, Sep 03, 2008 at 02:39:09AM +0200, Miklos Vajna <vmiklos@frugalware.org> wrote:
> BTW, there are 3 leftovers: make_virtual_commit()'s virtual_id and the
> global current_file_set/current_directory_set. I guess all of them
> could be moved to merge_options as well. (I don't have more time today
> to do so, but I can do it tomorrow, unless you see some fundamental
> problem with it.)
Here are the two patches, that do this.
These, and the 4 other related patches which are not yet in git.git at
the moment are also available in the 'merge-recursive' branch of
git://repo.or.cz/git/vmiklos.git.
Miklos Vajna (2):
merge-recursive: move current_{file,directory}_set to struct
merge_options
merge-recursive: move make_virtual_commit()'s virtual_id to
merge_options
merge-recursive.c | 80 ++++++++++++++++++++++++++++------------------------
merge-recursive.h | 5 +++
2 files changed, 48 insertions(+), 37 deletions(-)
^ permalink raw reply [flat|nested] 35+ messages in thread
* [PATCH 1/2] merge-recursive: move current_{file,directory}_set to struct merge_options
2008-09-03 17:34 ` Miklos Vajna
@ 2008-09-03 17:34 ` Miklos Vajna
2008-09-03 17:34 ` [PATCH 2/2] merge-recursive: move make_virtual_commit()'s virtual_id to merge_options Miklos Vajna
2008-09-04 19:05 ` [PATCH] merge-recursive: move the global obuf to struct merge_options Junio C Hamano
1 sibling, 1 reply; 35+ messages in thread
From: Miklos Vajna @ 2008-09-03 17:34 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Stephan Beyer
Signed-off-by: Miklos Vajna <vmiklos@frugalware.org>
---
merge-recursive.c | 56 +++++++++++++++++++++++++++-------------------------
merge-recursive.h | 4 +++
2 files changed, 33 insertions(+), 27 deletions(-)
diff --git a/merge-recursive.c b/merge-recursive.c
index d4f12d0..964b8f3 100644
--- a/merge-recursive.c
+++ b/merge-recursive.c
@@ -77,9 +77,6 @@ struct stage_data
unsigned processed:1;
};
-static struct string_list current_file_set = {NULL, 0, 0, 1};
-static struct string_list current_directory_set = {NULL, 0, 0, 1};
-
static int show(struct merge_options *o, int v)
{
return (!o->call_depth && o->verbosity >= v) || o->verbosity >= 5;
@@ -235,22 +232,23 @@ static int save_files_dirs(const unsigned char *sha1,
memcpy(newpath, base, baselen);
memcpy(newpath + baselen, path, len);
newpath[baselen + len] = '\0';
+ struct merge_options *o = context;
if (S_ISDIR(mode))
- string_list_insert(newpath, ¤t_directory_set);
+ string_list_insert(newpath, &o->current_directory_set);
else
- string_list_insert(newpath, ¤t_file_set);
+ string_list_insert(newpath, &o->current_file_set);
free(newpath);
return READ_TREE_RECURSIVE;
}
-static int get_files_dirs(struct tree *tree)
+static int get_files_dirs(struct merge_options *o, struct tree *tree)
{
int n;
- if (read_tree_recursive(tree, "", 0, 0, NULL, save_files_dirs, NULL))
+ if (read_tree_recursive(tree, "", 0, 0, NULL, save_files_dirs, o))
return 0;
- n = current_file_set.nr + current_directory_set.nr;
+ n = o->current_file_set.nr + o->current_directory_set.nr;
return n;
}
@@ -434,7 +432,7 @@ static int remove_file(struct merge_options *o, int clean,
return 0;
}
-static char *unique_path(const char *path, const char *branch)
+static char *unique_path(struct merge_options *o, const char *path, const char *branch)
{
char *newpath = xmalloc(strlen(path) + 1 + strlen(branch) + 8 + 1);
int suffix = 0;
@@ -446,12 +444,12 @@ static char *unique_path(const char *path, const char *branch)
for (; *p; ++p)
if ('/' == *p)
*p = '_';
- while (string_list_has_string(¤t_file_set, newpath) ||
- string_list_has_string(¤t_directory_set, newpath) ||
+ while (string_list_has_string(&o->current_file_set, newpath) ||
+ string_list_has_string(&o->current_directory_set, newpath) ||
lstat(newpath, &st) == 0)
sprintf(p, "_%d", suffix++);
- string_list_insert(newpath, ¤t_file_set);
+ string_list_insert(newpath, &o->current_file_set);
return newpath;
}
@@ -716,14 +714,14 @@ static void conflict_rename_rename(struct merge_options *o,
const char *ren2_dst = ren2->pair->two->path;
const char *dst_name1 = ren1_dst;
const char *dst_name2 = ren2_dst;
- if (string_list_has_string(¤t_directory_set, ren1_dst)) {
- dst_name1 = del[delp++] = unique_path(ren1_dst, branch1);
+ if (string_list_has_string(&o->current_directory_set, ren1_dst)) {
+ dst_name1 = del[delp++] = unique_path(o, ren1_dst, branch1);
output(o, 1, "%s is a directory in %s adding as %s instead",
ren1_dst, branch2, dst_name1);
remove_file(o, 0, ren1_dst, 0);
}
- if (string_list_has_string(¤t_directory_set, ren2_dst)) {
- dst_name2 = del[delp++] = unique_path(ren2_dst, branch2);
+ if (string_list_has_string(&o->current_directory_set, ren2_dst)) {
+ dst_name2 = del[delp++] = unique_path(o, ren2_dst, branch2);
output(o, 1, "%s is a directory in %s adding as %s instead",
ren2_dst, branch1, dst_name2);
remove_file(o, 0, ren2_dst, 0);
@@ -749,7 +747,7 @@ static void conflict_rename_dir(struct merge_options *o,
struct rename *ren1,
const char *branch1)
{
- char *new_path = unique_path(ren1->pair->two->path, branch1);
+ char *new_path = unique_path(o, ren1->pair->two->path, branch1);
output(o, 1, "Renaming %s to %s instead", ren1->pair->one->path, new_path);
remove_file(o, 0, ren1->pair->two->path, 0);
update_file(o, 0, ren1->pair->two->sha1, ren1->pair->two->mode, new_path);
@@ -762,8 +760,8 @@ static void conflict_rename_rename_2(struct merge_options *o,
struct rename *ren2,
const char *branch2)
{
- char *new_path1 = unique_path(ren1->pair->two->path, branch1);
- char *new_path2 = unique_path(ren2->pair->two->path, branch2);
+ char *new_path1 = unique_path(o, ren1->pair->two->path, branch1);
+ char *new_path2 = unique_path(o, ren2->pair->two->path, branch2);
output(o, 1, "Renaming %s to %s and %s to %s instead",
ren1->pair->one->path, new_path1,
ren2->pair->one->path, new_path2);
@@ -913,7 +911,7 @@ static int process_renames(struct merge_options *o,
try_merge = 0;
- if (string_list_has_string(¤t_directory_set, ren1_dst)) {
+ if (string_list_has_string(&o->current_directory_set, ren1_dst)) {
clean_merge = 0;
output(o, 1, "CONFLICT (rename/directory): Rename %s->%s in %s "
" directory %s added in %s",
@@ -935,7 +933,7 @@ static int process_renames(struct merge_options *o,
"%s added in %s",
ren1_src, ren1_dst, branch1,
ren1_dst, branch2);
- new_path = unique_path(ren1_dst, branch2);
+ new_path = unique_path(o, ren1_dst, branch2);
output(o, 1, "Adding as %s instead", new_path);
update_file(o, 0, dst_other.sha1, dst_other.mode, new_path);
} else if ((item = string_list_lookup(ren1_dst, renames2Dst))) {
@@ -1073,8 +1071,8 @@ static int process_entry(struct merge_options *o,
sha = b_sha;
conf = "directory/file";
}
- if (string_list_has_string(¤t_directory_set, path)) {
- const char *new_path = unique_path(path, add_branch);
+ if (string_list_has_string(&o->current_directory_set, path)) {
+ const char *new_path = unique_path(o, path, add_branch);
clean_merge = 0;
output(o, 1, "CONFLICT (%s): There is a directory with name %s in %s. "
"Adding %s as %s",
@@ -1165,10 +1163,10 @@ int merge_trees(struct merge_options *o,
if (unmerged_cache()) {
struct string_list *entries, *re_head, *re_merge;
int i;
- string_list_clear(¤t_file_set, 1);
- string_list_clear(¤t_directory_set, 1);
- get_files_dirs(head);
- get_files_dirs(merge);
+ string_list_clear(&o->current_file_set, 1);
+ string_list_clear(&o->current_directory_set, 1);
+ get_files_dirs(o, head);
+ get_files_dirs(o, merge);
entries = get_unmerged();
re_head = get_renames(o, head, common, head, merge, entries);
@@ -1374,4 +1372,8 @@ void init_merge_options(struct merge_options *o)
if (o->verbosity >= 5)
o->buffer_output = 0;
strbuf_init(&o->obuf, 0);
+ memset(&o->current_file_set, 0, sizeof(struct string_list));
+ o->current_file_set.strdup_strings = 1;
+ memset(&o->current_directory_set, 0, sizeof(struct string_list));
+ o->current_directory_set.strdup_strings = 1;
}
diff --git a/merge-recursive.h b/merge-recursive.h
index be84d9b..fd138ca 100644
--- a/merge-recursive.h
+++ b/merge-recursive.h
@@ -1,6 +1,8 @@
#ifndef MERGE_RECURSIVE_H
#define MERGE_RECURSIVE_H
+#include "string-list.h"
+
struct merge_options {
const char *branch1;
const char *branch2;
@@ -11,6 +13,8 @@ struct merge_options {
int merge_rename_limit;
int call_depth;
struct strbuf obuf;
+ struct string_list current_file_set;
+ struct string_list current_directory_set;
};
/* merge_trees() but with recursive ancestor consolidation */
--
1.6.0.1
^ permalink raw reply related [flat|nested] 35+ messages in thread
* [PATCH 2/2] merge-recursive: move make_virtual_commit()'s virtual_id to merge_options
2008-09-03 17:34 ` [PATCH 1/2] merge-recursive: move current_{file,directory}_set " Miklos Vajna
@ 2008-09-03 17:34 ` Miklos Vajna
2008-09-04 19:03 ` Junio C Hamano
0 siblings, 1 reply; 35+ messages in thread
From: Miklos Vajna @ 2008-09-03 17:34 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Stephan Beyer
This is the last patch in this series, now all static variables in
merge-recursive.c are moved to struct merge_options.
Signed-off-by: Miklos Vajna <vmiklos@frugalware.org>
---
merge-recursive.c | 24 ++++++++++++++----------
merge-recursive.h | 1 +
2 files changed, 15 insertions(+), 10 deletions(-)
diff --git a/merge-recursive.c b/merge-recursive.c
index 964b8f3..4fa3308 100644
--- a/merge-recursive.c
+++ b/merge-recursive.c
@@ -40,13 +40,14 @@ static struct tree *shift_tree_object(struct tree *one, struct tree *two)
* - *(int *)commit->object.sha1 set to the virtual id.
*/
-struct commit *make_virtual_commit(struct tree *tree, const char *comment)
+struct commit *make_virtual_commit(struct merge_options *o,
+ struct tree *tree,
+ const char *comment)
{
struct commit *commit = xcalloc(1, sizeof(struct commit));
- static unsigned virtual_id = 1;
commit->tree = tree;
commit->util = (void*)comment;
- *(int*)commit->object.sha1 = virtual_id++;
+ *(int*)commit->object.sha1 = o->virtual_id++;
/* avoid warnings */
commit->object.parsed = 1;
return commit;
@@ -1245,7 +1246,7 @@ int merge_recursive(struct merge_options *o,
tree->object.parsed = 1;
tree->object.type = OBJ_TREE;
pretend_sha1_file(NULL, 0, OBJ_TREE, tree->object.sha1);
- merged_common_ancestors = make_virtual_commit(tree, "ancestor");
+ merged_common_ancestors = make_virtual_commit(o, tree, "ancestor");
}
for (iter = ca; iter; iter = iter->next) {
@@ -1281,7 +1282,7 @@ int merge_recursive(struct merge_options *o,
&mrtree);
if (o->call_depth) {
- *result = make_virtual_commit(mrtree, "merged tree");
+ *result = make_virtual_commit(o, mrtree, "merged tree");
commit_list_insert(h1, &(*result)->parents);
commit_list_insert(h2, &(*result)->parents->next);
}
@@ -1289,7 +1290,9 @@ int merge_recursive(struct merge_options *o,
return clean;
}
-static struct commit *get_ref(const unsigned char *sha1, const char *name)
+static struct commit *get_ref(struct merge_options *o,
+ const unsigned char *sha1,
+ const char *name)
{
struct object *object;
@@ -1297,7 +1300,7 @@ static struct commit *get_ref(const unsigned char *sha1, const char *name)
if (!object)
return NULL;
if (object->type == OBJ_TREE)
- return make_virtual_commit((struct tree*)object, name);
+ return make_virtual_commit(o, (struct tree*)object, name);
if (object->type != OBJ_COMMIT)
return NULL;
if (parse_commit((struct commit *)object))
@@ -1314,15 +1317,15 @@ int merge_recursive_generic(struct merge_options *o,
{
int clean, index_fd;
struct lock_file *lock = xcalloc(1, sizeof(struct lock_file));
- struct commit *head_commit = get_ref(head, o->branch1);
- struct commit *next_commit = get_ref(merge, o->branch2);
+ struct commit *head_commit = get_ref(o, head, o->branch1);
+ struct commit *next_commit = get_ref(o, merge, o->branch2);
struct commit_list *ca = NULL;
if (base_list) {
int i;
for (i = 0; i < num_base_list; ++i) {
struct commit *base;
- if (!(base = get_ref(base_list[i], sha1_to_hex(base_list[i]))))
+ if (!(base = get_ref(o, base_list[i], sha1_to_hex(base_list[i]))))
return error("Could not parse object '%s'",
sha1_to_hex(base_list[i]));
commit_list_insert(base, &ca);
@@ -1376,4 +1379,5 @@ void init_merge_options(struct merge_options *o)
o->current_file_set.strdup_strings = 1;
memset(&o->current_directory_set, 0, sizeof(struct string_list));
o->current_directory_set.strdup_strings = 1;
+ o->virtual_id = 1;
}
diff --git a/merge-recursive.h b/merge-recursive.h
index fd138ca..bc23fe0 100644
--- a/merge-recursive.h
+++ b/merge-recursive.h
@@ -15,6 +15,7 @@ struct merge_options {
struct strbuf obuf;
struct string_list current_file_set;
struct string_list current_directory_set;
+ unsigned virtual_id;
};
/* merge_trees() but with recursive ancestor consolidation */
--
1.6.0.1
^ permalink raw reply related [flat|nested] 35+ messages in thread
* Re: [PATCH 2/2] merge-recursive: move make_virtual_commit()'s virtual_id to merge_options
2008-09-03 17:34 ` [PATCH 2/2] merge-recursive: move make_virtual_commit()'s virtual_id to merge_options Miklos Vajna
@ 2008-09-04 19:03 ` Junio C Hamano
2008-09-05 17:26 ` [PATCH] merge-recursive: get rid of virtual_id Miklos Vajna
0 siblings, 1 reply; 35+ messages in thread
From: Junio C Hamano @ 2008-09-04 19:03 UTC (permalink / raw)
To: Miklos Vajna; +Cc: git, Stephan Beyer, Alex Riesen, Johannes Schindelin
I do not think this one is a good idea.
What does it mean for a two "virtual commits" that are returned from
separate calls to make_virtual_commit() to have the same virtual_id?
I do not know offhand if the existing code does something like:
commit = lookup_commit(commit->object.sha1)
or
if (hashcmp(commit1->object.sha1, commit2->object.sha1))
...
but if there is such a code, I think this change makes the problem
"virtual id" has even worse. With the current single function local
static "virtual_id", at least during a program's lifetime it is guaranteed
that there won't be any two instances of virtual commits created for
different purposes that share the same (fake) sha1 value. If you call
merge_recursive more than once in your process, using a fresh "struct
merge_options" each time, that guarantee is lost with your change.
The only purpose of a "virtual commit", as I understand it, is to allow
you to pass a tree resulting from an internal merge to a function that
expects you to call with three commit objects to come up with a new tree
that is the result of the merge. The code stores a "virtual_id" as a
phoney sha1 value in the object, but I do not think the actual value is
used for anything but debugging purposes (Alex, Dscho, please correct me
as necessary).
Does it hurt if we get rid of virtual_id and always leave the
object->sha1 field of virtual commits 0{40} as it is initialized?
I further suspect we _could_ fix the API that requires you to pass three
commits to accept three trees instead and get rid of virtual commits
altogether, but then we would lose an easy access to the message of
commits that are being merged, and we would need to pass these strings as
separate parameters (or part of merge_options) --- which might be a good
clean-up in a longer run, but I do not think it is absolutely necessary
during this round.
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH] merge-recursive: move the global obuf to struct merge_options
2008-09-03 17:34 ` Miklos Vajna
2008-09-03 17:34 ` [PATCH 1/2] merge-recursive: move current_{file,directory}_set " Miklos Vajna
@ 2008-09-04 19:05 ` Junio C Hamano
1 sibling, 0 replies; 35+ messages in thread
From: Junio C Hamano @ 2008-09-04 19:05 UTC (permalink / raw)
To: Miklos Vajna; +Cc: git, Stephan Beyer
Miklos Vajna <vmiklos@frugalware.org> writes:
> These, and the 4 other related patches which are not yet in git.git at
> the moment are also available in the 'merge-recursive' branch of
> git://repo.or.cz/git/vmiklos.git.
Thanks.
I already expressed my doubts about "move make_virtual_commit()".
"add merge-recursive to LIB_H" should go to 'maint' as a build-fix, so it
does not have to be on this topic. If this is meant to be a pullable
topic branch, I'd prefer if you didn't have that commit.
I cherry picked call-depth, index-only removal, obuf and current_{f/d}
but haven't pushed the results out.
^ permalink raw reply [flat|nested] 35+ messages in thread
* [PATCH] merge-recursive: get rid of virtual_id
2008-09-04 19:03 ` Junio C Hamano
@ 2008-09-05 17:26 ` Miklos Vajna
0 siblings, 0 replies; 35+ messages in thread
From: Miklos Vajna @ 2008-09-05 17:26 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Stephan Beyer, Alex Riesen, Johannes Schindelin
We now just leave the object->sha1 field of virtual commits 0{40} as it
is initialized, as a unique hash is not necessary in case of virtual
commits.
Signed-off-by: Miklos Vajna <vmiklos@frugalware.org>
---
On Thu, Sep 04, 2008 at 12:03:08PM -0700, Junio C Hamano <gitster@pobox.com> wrote:
> Does it hurt if we get rid of virtual_id and always leave the
> object->sha1 field of virtual commits 0{40} as it is initialized?
I don't think so. Here is a patch that does it.
merge-recursive.c | 6 +-----
1 files changed, 1 insertions(+), 5 deletions(-)
diff --git a/merge-recursive.c b/merge-recursive.c
index 1c24c31..dbdb9ac 100644
--- a/merge-recursive.c
+++ b/merge-recursive.c
@@ -35,18 +35,14 @@ static struct tree *shift_tree_object(struct tree *one, struct tree *two)
}
/*
- * A virtual commit has
- * - (const char *)commit->util set to the name, and
- * - *(int *)commit->object.sha1 set to the virtual id.
+ * A virtual commit has (const char *)commit->util set to the name.
*/
struct commit *make_virtual_commit(struct tree *tree, const char *comment)
{
struct commit *commit = xcalloc(1, sizeof(struct commit));
- static unsigned virtual_id = 1;
commit->tree = tree;
commit->util = (void*)comment;
- *(int*)commit->object.sha1 = virtual_id++;
/* avoid warnings */
commit->object.parsed = 1;
return commit;
--
1.6.0.1
^ permalink raw reply related [flat|nested] 35+ messages in thread
end of thread, other threads:[~2008-09-05 17:27 UTC | newest]
Thread overview: 35+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2008-08-19 9:05 What's cooking in git.git (Aug 2008, #05; Tue, 19) Junio C Hamano
2008-08-19 11:02 ` Johannes Sixt
2008-08-19 12:35 ` Andreas Färber
2008-08-19 12:54 ` Miklos Vajna
2008-08-19 19:19 ` Junio C Hamano
2008-08-19 20:59 ` Miklos Vajna
2008-08-19 22:00 ` Junio C Hamano
2008-08-20 22:42 ` Miklos Vajna
2008-08-25 1:44 ` [PATCH] merge-recursive: introduce merge_options Miklos Vajna
2008-08-25 6:06 ` Junio C Hamano
2008-08-25 14:25 ` Miklos Vajna
2008-08-28 4:50 ` Junio C Hamano
2008-08-30 15:42 ` [PATCH] merge-recursive: fix subtree merge Miklos Vajna
2008-08-30 16:39 ` Junio C Hamano
2008-08-30 17:55 ` Junio C Hamano
2008-08-31 23:49 ` Miklos Vajna
2008-09-01 1:06 ` What's cooking in git.git (Aug 2008, #05; Tue, 19) Miklos Vajna
2008-09-01 1:09 ` [PATCH] builtin-revert: use merge_recursive_generic() Miklos Vajna
2008-09-02 20:02 ` What's cooking in git.git (Aug 2008, #05; Tue, 19) Junio C Hamano
2008-09-02 20:43 ` Junio C Hamano
2008-09-02 22:05 ` [PATCH 0/2] Move call_depth and index_only to struct merge_options Miklos Vajna
2008-09-02 22:05 ` [PATCH 1/2] merge-recursive: move call_depth " Miklos Vajna
2008-09-02 22:05 ` [PATCH 2/2] merge-recursive: move index_only " Miklos Vajna
2008-09-02 22:13 ` [PATCH] Makefile: add merge_recursive.h to LIB_H Miklos Vajna
2008-09-02 22:39 ` Junio C Hamano
2008-09-02 23:49 ` Miklos Vajna
2008-09-02 22:39 ` [PATCH 0/2] Move call_depth and index_only to struct merge_options Junio C Hamano
2008-09-03 0:16 ` [PATCH] merge-recursive: get rid of the index_only global variable Miklos Vajna
2008-09-03 0:39 ` [PATCH] merge-recursive: move the global obuf to struct merge_options Miklos Vajna
2008-09-03 17:34 ` Miklos Vajna
2008-09-03 17:34 ` [PATCH 1/2] merge-recursive: move current_{file,directory}_set " Miklos Vajna
2008-09-03 17:34 ` [PATCH 2/2] merge-recursive: move make_virtual_commit()'s virtual_id to merge_options Miklos Vajna
2008-09-04 19:03 ` Junio C Hamano
2008-09-05 17:26 ` [PATCH] merge-recursive: get rid of virtual_id Miklos Vajna
2008-09-04 19:05 ` [PATCH] merge-recursive: move the global obuf to struct merge_options Junio C Hamano
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).