Git development
 help / color / mirror / Atom feed
* Re: Client-side mirroring patches (v0)
From: Shawn O. Pearce @ 2009-11-26  0:58 UTC (permalink / raw)
  To: Sam Vilain; +Cc: git
In-Reply-To: <1259143617-26580-1-git-send-email-sam@vilain.net>

Sam Vilain <sam@vilain.net> wrote:
> Hey folks, this is the first stage of git mirroring 
...
> Also there is the matter of falling over to the next mirror should one
> not be reachable, but then we're getting into C weaknesses really.
> Should I plan to do exception recovery using 'longjmp' ? 

Please don't use longjmp.

You'll have to change the code to not die() upon connection failure,
but instead return an error code to the higher level which can
locate another mirror and retry.

> Also the
> process should be interruptible and provide a user menu.  Again this
> seems like it would be very tedious and clumsy in C.  How do people
> manage?

With great pain.  :-)

To do a user menu you can do a simple interface like `git add -i`
does, which just dumps the choices to stdout and a prompt for the
user to enter their selection.  If you want something more complex
you need to link to curses or ncurses, which IIRC opens some issues
with portablity, but lets you do a bit nicer interface on the tty.

-- 
Shawn.

^ permalink raw reply

* Re: [PATCH/RFC 02/11] strbuf: add non-variadic function strbuf_vaddf()
From: Junio C Hamano @ 2009-11-26  0:59 UTC (permalink / raw)
  To: Erik Faye-Lund; +Cc: msysgit, git, dotzenlabs, Erik Faye-Lund
In-Reply-To: <1259196260-3064-3-git-send-email-kusmabite@gmail.com>

Erik Faye-Lund <kusmabite@googlemail.com> writes:

> +void strbuf_vaddf(struct strbuf *sb, const char *fmt, va_list ap)
>  {
>  	int len;
>  
>  	if (!strbuf_avail(sb))
>  		strbuf_grow(sb, 64);
>  	len = vsnprintf(sb->buf + sb->len, sb->alloc - sb->len, fmt, ap);
>  	if (len < 0)
>  		die("your vsnprintf is broken");
>  	if (len > strbuf_avail(sb)) {
>  		strbuf_grow(sb, len);
>  		len = vsnprintf(sb->buf + sb->len, sb->alloc - sb->len, fmt, ap);
>  		if (len > strbuf_avail(sb)) {
>  			die("this should not happen, your snprintf is broken");
>  		}

Hmm, I would have expected to see va_copy() somewhere in the patch text.
Is it safe to reuse ap like this in two separate invocations of
vsnprintf()?

^ permalink raw reply

* Re: [PATCH] gitweb.js: Harden setting blamed commit info in incremental blame
From: Jakub Narebski @ 2009-11-26  0:59 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Stephen Boyd, git
In-Reply-To: <7vpr76m8dx.fsf@alter.siamese.dyndns.org>

On Thu, 26 Nov 2009, Junio C Hamano wrote:
> Jakub Narebski <jnareb@gmail.com> writes:
> 
> > Below there is request-pull with reordered series, unless you want me
> > to resend this series as a set of patches, as reply to this email.
> 
> It is too late for that, as they are already in 'next'.  I do not think it
> is necessary nor desirable, either.
> 
> With the "Create links" commit, you are _adding_ a new link that leads to
> a new feature with known breakage, not _replacing_ a link that leads to an
> existing working implementation with one that does not work for some
> people, no?

No.  The "Create links" commits make existing 'blame' links, which till that
commit always lead to non-incremental 'blame' action, now lead to new
'blame_incremental' action (if JavaScript is enabled).  So the result is
that we _replace_ links to 'blame' view by links to 'blame_incremental' view.

> Merging everything else but not merging that commit does not make any
> sense.  It won't give any wider exposure to the feature, and is no better
> than carrying the entire thing in 'next' (or 'pu') post 1.6.6.
> 
> A follow-up patch to add a gitweb configuration switch that disables the
> non-working view by default but allows site owners to enable it in order
> to help improving the feature would be a sensible thing to do.  As long as
> that patch is solidly done we can merge the whole thing to 'master' in the
> upcoming release.

But if it is already in 'next', then I'll try to come up with patch which
makes JavaScript-ing links (replacing links with JavaScript to equivalent
actions utilizing JavaScript, currently only 'blame' -> 'blame_incremental')
configurable.

-- 
Jakub Narebski
Poland

^ permalink raw reply

* What's cooking in git.git (Nov 2009, #06; Wed, 25)
From: Junio C Hamano @ 2009-11-26  1:03 UTC (permalink / raw)
  To: git

I wanted to do a 1.6.6-rc1 but somehow ended up spending too much time in
mail compose buffer instead of in C-mode today.  There are a few topics
that are in "stalled" state that may be worthy of being in 1.6.6.

We should start updating the Release Notes to make the 1.7.0 warning a bit
more visible.

-- >8 --

Here are the topics that have been cooking.  Commits prefixed with '-' are
only in 'pu' while commits prefixed with '+' are in 'next'.  The ones
marked with '.' do not appear in any of the integration branches, but I am
still holding onto them.

In 1.7.0, we plan to correct handful of warts in the interfaces everybody
agrees that they were mistakes.  The resulting system may not be strictly
backward compatible.  Currently planned changes are:

 * refuse push to update the checked out branch in a non-bare repo by
   default

   Make "git push" into a repository to update the branch that is checked
   out fail by default.  You can countermand this default by setting a
   configuration variable in the receiving repository.

   http://thread.gmane.org/gmane.comp.version-control.git/107758/focus=108007

 * refuse push to delete the current branch by default

   Make "git push $there :$killed" to delete the branch that is pointed at
   by its HEAD fail by default.  You can countermand this default by
   setting a configuration variable in the receiving repository.

   http://thread.gmane.org/gmane.comp.version-control.git/108862/focus=108936

 * "git send-email" won't make deep threads by default

   Many people said that by default when sending more than 2 patches the
   threading git-send-email makes by default is hard to read, and they
   prefer the default be one cover letter and each patch as a direct
   follow-up to the cover letter.  You can countermand this by setting a
   configuration variable.

   http://article.gmane.org/gmane.comp.version-control.git/109790

 * "git status" won't be "git-commit --dry-run" anymore

   http://thread.gmane.org/gmane.comp.version-control.git/125989/focus=125993

 * "git diff -w --exit-code" will exit success if only differences it
   found are whitespace changes that are stripped away from the output.

   http://thread.gmane.org/gmane.comp.version-control.git/119731/focus=119751

 * "git diff -w/-b" won't even produce "diff --git" header when all changes
   are about whitespaces.

   http://thread.gmane.org/gmane.comp.version-control.git/133256

--------------------------------------------------
[Graduated to "master"]

* bg/fetch-multi (2009-11-10) 9 commits.
  (merged to 'next' on 2009-11-21 at 282f464)
 + Re-implement 'git remote update' using 'git fetch'
 + builtin-fetch: add --dry-run option
 + builtin-fetch: add --prune option
 + teach warn_dangling_symref to take a FILE argument
 + remote: refactor some logic into get_stale_heads()
 + Add missing test for 'git remote update --prune'
 + Add the configuration option skipFetchAll
 + Teach the --multiple option to 'git fetch'
 + Teach the --all option to 'git fetch'

* bg/apply-doc (2009-11-22) 4 commits
  (merged to 'next' on 2009-11-22 at b42fece)
 + apply: Use the term "working tree" consistently
 + apply: Format all options using back-quotes
 + apply: apply works outside a repository
 + Clarify and correct -z

* cc/replace (2009-11-19) 3 commits
  (merged to 'next' on 2009-11-21 at 2aaf84b)
 + Documentation: talk a little bit about GIT_NO_REPLACE_OBJECTS
 + Documentation: fix typos and spelling in replace documentation
 + replace: use a GIT_NO_REPLACE_OBJECTS env variable

* mm/maint-hint-failed-merge (2009-11-22) 2 commits.
  (merged to 'next' on 2009-11-22 at c0f64c2)
 + user-manual: Document that "git merge" doesn't like uncommited changes.
 + merge-recursive: point the user to commit when file would be overwritten.

* rj/maint-cygwin-count-objects (2009-11-19) 2 commits.
  (merged to 'next' on 2009-11-22 at 4ba5880)
 + ST_BLOCKS_COUNTS_IN_BLKSIZE to say on-disk size is (st_blksize * st_blocks)
 + git-count-objects: Fix a disk-space under-estimate on Cygwin

* rs/color-escape-has-zero-width (2009-11-23) 1 commit
 + Teach %w() that color escape codes have zero width

* jc/log-stdin (2009-11-23) 5 commits
  (merged to 'next' on 2009-11-23 at ea71363)
 + Add trivial tests for --stdin option to log family
  (merged to 'next' on 2009-11-21 at c3e2e52)
 + Make --stdin option to "log" family read also pathspecs
 + setup_revisions(): do not call get_pathspec() too early
 + Teach --stdin option to "log" family
 + read_revision_from_stdin(): use strbuf

* mr/gitweb-snapshot (2009-11-07) 4 commits.
  (merged to 'next' on 2009-11-21 at e825ad9)
 + gitweb: Smarter snapshot names
 + gitweb: Document current snapshot rules via new tests
 + t/gitweb-lib.sh: Split gitweb output into headers and body
  (merged to 'next' on 2009-10-11 at 22ba047)
 + gitweb: check given hash before trying to create snapshot

* rs/work-around-grep-opt-insanity (2009-11-23) 2 commits.
  (merged to 'next' on 2009-11-25 at bf972d8)
 + Protect scripted Porcelains from GREP_OPTIONS insanity
 + mergetool--lib: simplify guess_merge_tool()

--------------------------------------------------
[New Topics]

* jc/botched-maint-cygwin-count-objects (2009-11-24) 2 commits
  (merged to 'next' on 2009-11-25 at 8aa62a0)
 + Revert "ST_BLOCKS_COUNTS_IN_BLKSIZE to say on-disk size is (st_blksize * st_blocks)"
  (merged to 'next' on 2009-11-22 at 4ba5880)
 + ST_BLOCKS_COUNTS_IN_BLKSIZE to say on-disk size is (st_blksize * st_blocks)

This is a revert of the tip one I merged prematurely to 'next'.  The real
fix from Ramsay is already in 'master'.

* jc/grep-full-tree (2009-11-24) 1 commit.
 - grep: --full-tree

We probably would want test, doc and a configuration variable to make it
default (or non-default) before we can merge it to 'master'.

* uk/maint-shortlog-encoding (2009-11-25) 1 commit.
 - shortlog: respect commit encoding

The fix is a maint material but the patch was against next, so I
back-rebased it myself.  I tried to be careful but please double check the
result.

Perhaps merge it to 'master' before 1.6.6-rc1?

--------------------------------------------------
[Stalled]

* je/send-email-no-subject (2009-08-05) 1 commit.
  (merged to 'next' on 2009-10-11 at 1b99c56)
 + send-email: confirm on empty mail subjects

The existing tests cover the positive case (i.e. as long as the user says
"yes" to the "do you really want to send this message that lacks subject",
the message is sent) of this feature, but the feature itself needs its own
test to verify the negative case (i.e. does it correctly stop if the user
says "no"?)

* fc/send-email-envelope (2009-11-22) 1 commit.
 - t9001: test --envelope-sender option of send-email

The new feature itself looked promising; this is just an unrelated test
patch.

* jn/rfc-pull-rebase-error-message (2009-11-12) 1 commit
 - git-pull.sh --rebase: overhaul error handling when no candidates are found

I heard this needs at least retitling among other changes?

* sr/vcs-helper (2009-11-18) 12 commits
 - Add Python support library for remote helpers
 - Basic build infrastructure for Python scripts
 - Allow helpers to report in "list" command that the ref is unchanged
 - Fix various memory leaks in transport-helper.c
 - Allow helper to map private ref names into normal names
 - Add support for "import" helper command
 - Allow specifying the remote helper in the url
 - Add a config option for remotes to specify a foreign vcs
 - Allow fetch to modify refs
 - Use a function to determine whether a remote is valid
 - Allow programs to not depend on remotes having urls
 - Fix memory leak in helper method for disconnect

Replaced again, and looking good.  Perhaps Daniel has some comments?

* jh/notes (2009-11-20) 10 commits
 - Add more testcases to test fast-import of notes
 - Rename t9301 to t9350, to make room for more fast-import tests
 - fast-import: Proper notes tree manipulation using the notes API
 - Refactor notes concatenation into a flexible interface for combining notes
 - Notes API: Allow multiple concurrent notes trees with new struct notes_tree
 - Notes API: for_each_note(): Traverse the entire notes tree with a callback
 - Notes API: get_note(): Return the note annotating the given object
 - Notes API: add_note(): Add note objects to the internal notes tree structure
 - Notes API: init_notes(): Initialize the notes tree from the given notes ref
 - Notes API: get_commit_notes() -> format_note() + remove the commit restriction

Johan waits for an Ack from Shawn on "fast-import" one.

* tr/maint-merge-ours-clarification (2009-11-15) 1 commit
  (merged to 'next' on 2009-11-21 at fadaf7b)
 + rebase: refuse to rebase with -s ours

I do not think we reached a concensus for solving conflicts between "give
them rope" and "protect users from clearly meaningless combinations".  The
author obviously is for the latter (and I am inclined to agree); Dscho
seems to think otherwise.

* jc/fix-tree-walk (2009-10-22) 8 commits
  (merged to 'next' on 2009-10-22 at 10c0c8f)
 + Revert failed attempt since 353c5ee
 + read-tree --debug-unpack
  (merged to 'next' on 2009-10-11 at 0b058e2)
 + unpack-trees.c: look ahead in the index
 + unpack-trees.c: prepare for looking ahead in the index
 + Aggressive three-way merge: fix D/F case
 + traverse_trees(): handle D/F conflict case sanely
 + more D/F conflict tests
 + tests: move convenience regexp to match object names to test-lib.sh

This has some stupid bugs and reverted from 'next' until I can fix it, but
the "temporarily" turned out to be very loooong.  Sigh...

* sr/gfi-options (2009-09-06) 6 commits.
 - fast-import: test the new option command
 - fast-import: add option command
 - fast-import: test the new feature command
 - fast-import: add feature command
 - fast-import: put marks reading in it's own function
 - fast-import: put option parsing code in separate functions

Sverre is working on a re-roll to address comments from Shawn.

--------------------------------------------------
[Cooking]

* jc/mailinfo-remove-brackets (2009-07-15) 1 commit.
  (merged to 'next' on 2009-11-25 at 09d498f)
 + mailinfo: -b option keeps [bracketed] strings that is not a [PATCH] marker

Jim Meyering sent a patch to do a subset of what this does; to allow
keeping '[SECURITY]' when the subject says '[SECURITY][PATCH]', you need
to also teach "am" to pass the new -b option, but that is independent of
what Jim showed the need in real-world, so I think this can go in as-is.

Perhaps merge it to 'master' before 1.6.6-rc1?

* jc/checkout-merge-base (2009-11-20) 2 commits
 - "rebase --onto A...B" replays history on the merge base between A and B
 - "checkout A...B" switches to the merge base between A and B

I've been using the first one for a while myself but do not see many users
want this (yet); the new feature is not urgent anyway.

* tr/reset-checkout-patch (2009-11-19) 1 commit.
  (merged to 'next' on 2009-11-22 at b224950)
 + {checkout,reset} -p: make patch direction configurable

I do not particularly like a configuration like this that changes the
behaviour of a command in a drastic way---it will make helping others much
harder.

Perhaps merge it to 'master' before 1.6.6-rc1?

* jn/gitweb-blame (2009-11-24) 8 commits.
  (merged to 'next' on 2009-11-25 at 0a5b649)
 + gitweb.js: fix padLeftStr() and its usage
 + gitweb.js: Harden setting blamed commit info in incremental blame
 + gitweb.js: fix null object exception in initials calculation
 + gitweb: Minify gitweb.js if JSMIN is defined
 + gitweb: Create links leading to 'blame_incremental' using JavaScript
  (merged to 'next' on 2009-10-11 at 73c4a83)
 + gitweb: Colorize 'blame_incremental' view during processing
 + gitweb: Incremental blame (using JavaScript)
 + gitweb: Add optional "time to generate page" info in footer

Ajax-y blame, with further fixes.  As this does not seem to break existing
features, I am inclined to say that we push this out early, as a new
feature with known breakages, to give it wider audience.

* em/commit-claim (2009-11-04) 1 commit
  (merged to 'next' on 2009-11-23 at b5df6fd)
 + commit -c/-C/--amend: reset timestamp and authorship to committer with --reset-author

I am not sure if the option name does a good job at explaining it to the
end users, but I think the code and feature is solid.

Perhaps merge it to 'master' before 1.6.6-rc1?

* cc/bisect-doc (2009-11-08) 1 commit
 - Documentation: add "Fighting regressions with git bisect" article

Nobody seems to think this should go to Documentation/technical instead,
so unless I hear otherwise, we will have it as-is in 'next' shortly.

Perhaps merge it to 'master' before 1.6.6-rc1?

* nd/sparse (2009-11-25) 20 commits.
  (merged to 'next' on 2009-11-25 at 71380f5)
 + tests: rename duplicate t1009
  (merged to 'next' on 2009-11-23 at f712a41)
 + sparse checkout: inhibit empty worktree
 + Add tests for sparse checkout
 + read-tree: add --no-sparse-checkout to disable sparse checkout support
 + unpack-trees(): ignore worktree check outside checkout area
 + unpack_trees(): apply $GIT_DIR/info/sparse-checkout to the final index
 + unpack-trees(): "enable" sparse checkout and load $GIT_DIR/info/sparse-checkout
 + unpack-trees.c: generalize verify_* functions
 + unpack-trees(): add CE_WT_REMOVE to remove on worktree alone
 + Introduce "sparse checkout"
 + dir.c: export excluded_1() and add_excludes_from_file_1()
 + excluded_1(): support exclude files in index
 + unpack-trees(): carry skip-worktree bit over in merged_entry()
 + Read .gitignore from index if it is skip-worktree
 + Avoid writing to buffer in add_excludes_from_file_1()
 + Teach Git to respect skip-worktree bit (writing part)
 + Teach Git to respect skip-worktree bit (reading part)
 + Introduce "skip-worktree" bit in index, teach Git to get/set this bit
 + Add test-index-version
 + update-index: refactor mark_valid() in preparation for new options

* jc/pretty-lf (2009-10-04) 1 commit.
 - Pretty-format: %[+-]x to tweak inter-item newlines

Perhaps drop the "%-x" part and merge it to 'next' and to 'master' before
1.6.6?

--------------------------------------------------
[For 1.7.0]

* jk/1.7.0-status (2009-09-05) 5 commits.
  (merged to 'next' on 2009-11-21 at 884bb56)
 + docs: note that status configuration affects only long format
  (merged to 'next' on 2009-10-11 at 65c8513)
 + commit: support alternate status formats
 + status: add --porcelain output format
 + status: refactor format option parsing
 + status: refactor short-mode printing to its own function
 (this branch uses jc/1.7.0-status.)

Gives the --short output format to post 1.7.0 "git commit --dry-run" that
is similar to that of post 1.7.0 "git status".

* jc/1.7.0-status (2009-09-05) 4 commits.
  (merged to 'next' on 2009-10-11 at 9558627)
 + status: typo fix in usage
 + git status: not "commit --dry-run" anymore
 + git stat -s: short status output
 + git stat: the beginning of "status that is not a dry-run of commit"
 (this branch is used by jk/1.7.0-status.)

With this, "git status" is no longer "git commit --dry-run".

* jc/1.7.0-send-email-no-thread-default (2009-08-22) 1 commit.
  (merged to 'next' on 2009-10-11 at 043acdf)
 + send-email: make --no-chain-reply-to the default

* jc/1.7.0-diff-whitespace-only-status (2009-08-30) 4 commits.
  (merged to 'next' on 2009-10-11 at 546c74d)
 + diff.c: fix typoes in comments
 + Make test case number unique
 + diff: Rename QUIET internal option to QUICK
 + diff: change semantics of "ignore whitespace" options

This changes exit code from "git diff --ignore-whitespace" and friends
when there is no actual output.  It is a backward incompatible change, but
we could argue that it is a bugfix.

* gb/1.7.0-diff-whitespace-only-outout (2009-11-19) 1 commit
  (merged to 'next' on 2009-11-21 at 3375bf4)
 + No diff -b/-w output for all-whitespace changes

* jc/1.7.0-push-safety (2009-02-09) 2 commits.
  (merged to 'next' on 2009-10-11 at 81b8128)
 + Refuse deleting the current branch via push
 + Refuse updating the current branch in a non-bare repository via push

--------------------------------------------------
[I have been too busy to purge these]

* jc/log-tz (2009-03-03) 1 commit.
 - Allow --date=local --date=other-format to work as expected

Maybe some people care about this.  I dunno.

* jc/1.7.0-no-commit-no-ff-2 (2009-10-22) 1 commit.
 . git-merge: forbid fast-forward and up-to-date when --no-commit is given

This makes "git merge --no-commit" fail when it results in fast-forward or
up-to-date.  It appears nobody wants to have this, so I dropped it.

* ne/rev-cache (2009-10-19) 7 commits.
 . support for commit grafts, slight change to general mechanism
 . support for path name caching in rev-cache
 . full integration of rev-cache into git, completed test suite
 . administrative functions for rev-cache, start of integration into git
 . support for non-commit object caching in rev-cache
 . basic revision cache system, no integration or features
 . man page and technical discussion for rev-cache

The author indicated that there is another round coming.  Does not seem to
pass the tests when merged to 'pu', so it has been ejected for now.

* pb/gitweb-no-project-list (2009-11-06) 3 commits.
 . gitweb: Polish the content tags support
 . gitweb: Support for no project list on gitweb front page
 . gitweb: Refactor project list routines

I picked these up but didn't queue as Warthog9's comments made certain
amount of sense to me.

* ks/precompute-completion (2009-11-15) 4 commits.
  (merged to 'next' on 2009-11-15 at 23cdb96)
 + Revert ks/precompute-completion series
  (merged to 'next' on 2009-10-28 at cd5177f)
 + completion: ignore custom merge strategies when pre-generating
  (merged to 'next' on 2009-10-22 at f46a28a)
 + bug: precomputed completion includes scripts sources
  (merged to 'next' on 2009-10-14 at adf722a)
 + Speedup bash completion loading

Reverted out of 'next', to be replaced with jn/faster-completion-startup
topic.

^ permalink raw reply

* Re: [egit] Git repository with multiple eclipse projects ?
From: Douglas Campos @ 2009-11-26  1:12 UTC (permalink / raw)
  To: Shawn O. Pearce; +Cc: Yann Dirson, git
In-Reply-To: <20091126004817.GL11919@spearce.org>

(worksforme) egit 0.5.x

On Wed, Nov 25, 2009 at 10:48 PM, Shawn O. Pearce <spearce@spearce.org> wrote:
> Yann Dirson <ydirson@linagora.com> wrote:
>> I am investigating whether it is possible at all to have several
>> eclipse projects in a single git repo, and have those projects
>> correctly seen as managed by git.
>
> As Robin said, it should work.  EGit and JGit both use this layout.
>
>> When importing a git repo into eclipse, we get a list of projects to
>> import, but that list is empty.  What is expected by egit to get this
>> list filled ?
>
> There should be .project files in the repository.  I think we scan
> the entire checkout tree for .project files, but maybe we are doing
> something stupid and only looking at the top level directory of
> the checkout.
>
>> It also does not look like it would be possible to use the "share"
>> functionnality to setup such a repository from multiple projects (or
>> from a project set), right ?
>
> Nope, I don't think this is supported right now.  You need to
> initialize the git repository by hand in the higher level directory
> that holds the projects.
>
> --
> Shawn.
> --
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>



-- 
Douglas Campos (qmx)
+55 11 7626 5959
+55 11 6762 5959

^ permalink raw reply

* Re: What's cooking in git.git (Nov 2009, #06; Wed, 25)
From: Sverre Rabbelier @ 2009-11-26  1:15 UTC (permalink / raw)
  To: Junio C Hamano, Daniel Barkalow, Johan Herland; +Cc: git
In-Reply-To: <7v8wduksgq.fsf@alter.siamese.dyndns.org>

Heya,

On Thu, Nov 26, 2009 at 02:03, Junio C Hamano <gitster@pobox.com> wrote:
> * sr/vcs-helper (2009-11-18) 12 commits
>  - Add Python support library for remote helpers
>  - Basic build infrastructure for Python scripts
>  - Allow helpers to report in "list" command that the ref is unchanged
>  - Fix various memory leaks in transport-helper.c
>  - Allow helper to map private ref names into normal names
>  - Add support for "import" helper command
>  - Allow specifying the remote helper in the url
>  - Add a config option for remotes to specify a foreign vcs
>  - Allow fetch to modify refs
>  - Use a function to determine whether a remote is valid
>  - Allow programs to not depend on remotes having urls
>  - Fix memory leak in helper method for disconnect
>
> Replaced again, and looking good.  Perhaps Daniel has some comments?

Indeed, Johan, Daniel, is the current version good for next?

-- 
Cheers,

Sverre Rabbelier

^ permalink raw reply

* Re: [PATCH 2/4] fetch: try mirrors if selected
From: Shawn O. Pearce @ 2009-11-26  1:20 UTC (permalink / raw)
  To: Sam Vilain; +Cc: git
In-Reply-To: <1259143617-26580-3-git-send-email-sam@vilain.net>

Sam Vilain <sam@vilain.net> wrote:
> diff --git a/builtin-fetch.c b/builtin-fetch.c
> index 209f502..b3b8766 100644
> @@ -109,6 +109,109 @@ static void find_non_local_tags(struct transport *transport,
>  			struct ref **head,
>  			struct ref ***tail);
>  
> +char* get_url_hostname(const char *url)

Minor nit, but we mark any function not used outside of the module
as static.  Especially in a builtin-*.c since they all link into
the same namespace.  If this is meant to be reused, it belongs in
connect.c most likely, that's where we already have code like this
to get the SSH hostname out of a URL for SSH connections.

I don't have time right now to read the rest of this series, but
the general approach of fetching to a temporary mirror space before
checking if you really are current is a good one.

I'm not sure that storing the list of mirrors inside of the remote
makes much sense, I would think the user would want to store only
a handful of "fast" URLs.

And even then I wonder why this can't just be the url[1]..url[n-1]
entries in the configuration file.  push pushes to all of the
URLs at once, "seeding the mirrors".  Why can't fetch use the same
configuration?

-- 
Shawn.

^ permalink raw reply

* git-svn: dcommit --commiturl rebases against fetch url rather than against commit url
From: Ingmar Vanhassel @ 2009-11-26  1:15 UTC (permalink / raw)
  To: Eric Wong; +Cc: git

Hi Eric & list,

I'm using a git-svn setup where svn-remote.svn.url is set to use
anonymouse svn, and svn-remote.svn.commiturl uses my ssh+svn:// login.

I'm using anonymous svn for fetching because this git-svn import is
shared with a few other developers, who don't have commit access, but
who do prefer to use a git-svn import over SVN, for their work. Since I
use anonsvn for the import they can continue my import.

When I dcommit to svn, git-svn happily commits the first commit. When it
tries to rebase after that it fetches from the anonsvn url, which is
synced only ever 5 or 10 minutes. Thus, it doesn't find my commit, and
it aborts, meaning I need to wait until anonsvn catches up, before being
able to do another commit.

Any suggestions on how this could be solved without reimporting with
the rewriteRoot option set?

-Ingmar
-- 
Exherbo KDE, X.org maintainer

^ permalink raw reply

* Re: git-svn: dcommit --commiturl rebases against fetch url rather than against commit url
From: Eric Wong @ 2009-11-26  1:59 UTC (permalink / raw)
  To: Ingmar Vanhassel; +Cc: git
In-Reply-To: <1259198081-sup-3175@cannonball>

Ingmar Vanhassel <ingmar@exherbo.org> wrote:
> Hi Eric & list,
> 
> I'm using a git-svn setup where svn-remote.svn.url is set to use
> anonymouse svn, and svn-remote.svn.commiturl uses my ssh+svn:// login.
> 
> I'm using anonymous svn for fetching because this git-svn import is
> shared with a few other developers, who don't have commit access, but
> who do prefer to use a git-svn import over SVN, for their work. Since I
> use anonsvn for the import they can continue my import.
> 
> When I dcommit to svn, git-svn happily commits the first commit. When it
> tries to rebase after that it fetches from the anonsvn url, which is
> synced only ever 5 or 10 minutes. Thus, it doesn't find my commit, and
> it aborts, meaning I need to wait until anonsvn catches up, before being
> able to do another commit.

Hi Ingmar,

Wow, talk about weird corner cases.  Is this the monster KDE SVN repo?
I suppose the sync delay and multiple servers is necessary for that
thing.  I wrote commitUrl because I needed to work on a tiny repo :)
(and it was migrated fully to git shortly after)

> Any suggestions on how this could be solved without reimporting with
> the rewriteRoot option set?

Depending on how big your imported repo is, I might want to keep two
copies since rewriteRoot can be messy.  So keep using the anonymous one
shared with your developers, and make a full copy of $GIT_DIR for
yourself to run git filter-branch against.

That means don't write to the anonymous one at all here, just copy that
entire $GIT_DIR in case something goes wrong, you can recopy and tweak
the procedure (It's been a while since I've tried this :)

 * write a git filter-branch script that rewrites the git-svn-id:
   lines to the svn+ssh:// URL,  I haven't used filter-branch in
   a while but this should work...

 * remove all .rev_map.* files under $GIT_DIR/svn

 * change your $GIT_CONFIG file to point to the svn+ssh URL
   no need to use the commitUrl option anymore, either

 * remove $GIT_DIR/svn/.metadata to be safe, too

 * run "git svn fetch"
   This last step should rebuild all the .rev_map.* files under
   $GIT_DIR/svn.

Hopefully that works for you, let us know how it goes.

-- 
Eric Wong

^ permalink raw reply

* [PATCH 0/8] The return of -Xours, -Xtheirs, -Xsubtree=dir
From: Avery Pennarun @ 2009-11-26  2:23 UTC (permalink / raw)
  To: git; +Cc: gitster, Avery Pennarun

As discussed earlier today, this brings back Junio's earlier patch series
that introduced (and then used) a -X option for configuring merge
strategies.  My favourite use of this is -Xsubtree=<dir>, which lets you
provide the actual subdir prefix when using the subtree merge strategy.

Avery Pennarun (8):
  git-merge-file --ours, --theirs
  builtin-merge.c: call exclude_cmds() correctly.
  git-merge-recursive-{ours,theirs}
  Teach git-merge to pass -X<option> to the backend strategy module
  Teach git-pull to pass -X<option> to git-merge
  Make "subtree" part more orthogonal to the rest of merge-recursive.
  Extend merge-subtree tests to test -Xsubtree=dir.
  Document that merge strategies can now take their own options

 .gitignore                         |    2 +
 Documentation/git-merge-file.txt   |   12 +++++-
 Documentation/merge-options.txt    |    4 ++
 Documentation/merge-strategies.txt |   29 ++++++++++++++-
 builtin-checkout.c                 |    2 +-
 builtin-merge-file.c               |    5 ++-
 builtin-merge-recursive.c          |   24 ++++++++++---
 builtin-merge.c                    |   44 +++++++++++++++++++++--
 cache.h                            |    1 +
 contrib/examples/git-merge.sh      |    3 +-
 git-compat-util.h                  |    1 +
 git-pull.sh                        |   17 ++++++++-
 git.c                              |    2 +
 ll-merge.c                         |   20 +++++-----
 ll-merge.h                         |    2 +-
 match-trees.c                      |   69 +++++++++++++++++++++++++++++++++++-
 merge-recursive.c                  |   35 +++++++++++++++---
 merge-recursive.h                  |    7 +++-
 strbuf.c                           |    9 +++++
 t/t6029-merge-subtree.sh           |   47 ++++++++++++++++++++++++-
 t/t6034-merge-ours-theirs.sh       |   64 +++++++++++++++++++++++++++++++++
 xdiff/xdiff.h                      |    7 +++-
 xdiff/xmerge.c                     |   11 +++++-
 23 files changed, 377 insertions(+), 40 deletions(-)
 create mode 100755 t/t6034-merge-ours-theirs.sh

^ permalink raw reply

* [PATCH 1/8] git-merge-file --ours, --theirs
From: Avery Pennarun @ 2009-11-26  2:23 UTC (permalink / raw)
  To: git; +Cc: gitster, Avery Pennarun
In-Reply-To: <cover.1259201377.git.apenwarr@gmail.com>

Often people want their conflicting merges autoresolved by favouring
upstream changes (or their own --- it's the same thing), and hinted to run
"git diff --name-only | xargs git checkout MERGE_HEAD --".  This is
essentially to accept automerge results for the paths that are fully
resolved automatically while taking their version of the file in full for
paths that have conflicts.

This is problematic on two counts.

One problem is that this is not exactly what these people want.  They
usually want to salvage as much automerge result as possible.  In
particular, they want to keep autoresolved parts in conflicting paths, as
well as the paths that are fully autoresolved.

This patch teaches two new modes of operation to the lowest-lever merge
machinery, xdl_merge().  Instead of leaving the conflicted lines from both
sides enclosed in <<<, ===, and >>> markers, you can tell the conflicts to
be resolved favouring your side or their side of changes.

A larger problem is that this tends to encourage a bad workflow by
allowing them to record such a mixed up half-merge result as a full commit
without auditing.  This commit does not tackle this latter issue.  In git,
we usually give long enough rope to users with strange wishes as long as
the risky features is not on by default.

(Patch originally by Junio Hamano <gitster@pobox.com>.)

Signed-off-by: Avery Pennarun <apenwarr@gmail.com>
---
 Documentation/git-merge-file.txt |   12 ++++++++++--
 builtin-merge-file.c             |    5 ++++-
 xdiff/xdiff.h                    |    7 ++++++-
 xdiff/xmerge.c                   |   11 +++++++++--
 4 files changed, 29 insertions(+), 6 deletions(-)

diff --git a/Documentation/git-merge-file.txt b/Documentation/git-merge-file.txt
index 3035373..b9d2276 100644
--- a/Documentation/git-merge-file.txt
+++ b/Documentation/git-merge-file.txt
@@ -10,7 +10,8 @@ SYNOPSIS
 --------
 [verse]
 'git merge-file' [-L <current-name> [-L <base-name> [-L <other-name>]]]
-	[-p|--stdout] [-q|--quiet] <current-file> <base-file> <other-file>
+	[--ours|--theirs] [-p|--stdout] [-q|--quiet]
+	<current-file> <base-file> <other-file>
 
 
 DESCRIPTION
@@ -34,7 +35,9 @@ normally outputs a warning and brackets the conflict with lines containing
 	>>>>>>> B
 
 If there are conflicts, the user should edit the result and delete one of
-the alternatives.
+the alternatives.  When `--ours` or `--theirs` option is in effect, however,
+these conflicts are resolved favouring lines from `<current-file>` or
+lines from `<other-file>` respectively.
 
 The exit value of this program is negative on error, and the number of
 conflicts otherwise. If the merge was clean, the exit value is 0.
@@ -62,6 +65,11 @@ OPTIONS
 -q::
 	Quiet; do not warn about conflicts.
 
+--ours::
+--theirs::
+	Instead of leaving conflicts in the file, resolve conflicts
+	favouring our (or their) side of the lines.
+
 
 EXAMPLES
 --------
diff --git a/builtin-merge-file.c b/builtin-merge-file.c
index afd2ea7..8f22aa8 100644
--- a/builtin-merge-file.c
+++ b/builtin-merge-file.c
@@ -29,11 +29,14 @@ int cmd_merge_file(int argc, const char **argv, const char *prefix)
 	int ret = 0, i = 0, to_stdout = 0;
 	int merge_level = XDL_MERGE_ZEALOUS_ALNUM;
 	int merge_style = 0, quiet = 0;
+	int merge_favor = 0;
 	int nongit;
 
 	struct option options[] = {
 		OPT_BOOLEAN('p', "stdout", &to_stdout, "send results to standard output"),
 		OPT_SET_INT(0, "diff3", &merge_style, "use a diff3 based merge", XDL_MERGE_DIFF3),
+		OPT_SET_INT(0, "ours", &merge_favor, "for conflicts, use our version", XDL_MERGE_FAVOR_OURS),
+		OPT_SET_INT(0, "theirs", &merge_favor, "for conflicts, use their version", XDL_MERGE_FAVOR_THEIRS),
 		OPT__QUIET(&quiet),
 		OPT_CALLBACK('L', NULL, names, "name",
 			     "set labels for file1/orig_file/file2", &label_cb),
@@ -68,7 +71,7 @@ int cmd_merge_file(int argc, const char **argv, const char *prefix)
 	}
 
 	ret = xdl_merge(mmfs + 1, mmfs + 0, names[0], mmfs + 2, names[2],
-			&xpp, merge_level | merge_style, &result);
+			&xpp, merge_level | merge_style | merge_favor, &result);
 
 	for (i = 0; i < 3; i++)
 		free(mmfs[i].ptr);
diff --git a/xdiff/xdiff.h b/xdiff/xdiff.h
index 4da052a..2cce49d 100644
--- a/xdiff/xdiff.h
+++ b/xdiff/xdiff.h
@@ -58,6 +58,11 @@ extern "C" {
 #define XDL_MERGE_ZEALOUS_ALNUM 3
 #define XDL_MERGE_LEVEL_MASK 0x0f
 
+/* merge favor modes */
+#define XDL_MERGE_FAVOR_OURS 0x0010
+#define XDL_MERGE_FAVOR_THEIRS 0x0020
+#define XDL_MERGE_FAVOR(flags) (((flags)>>4) & 3)
+
 /* merge output styles */
 #define XDL_MERGE_DIFF3 0x8000
 #define XDL_MERGE_STYLE_MASK 0x8000
@@ -110,7 +115,7 @@ int xdl_diff(mmfile_t *mf1, mmfile_t *mf2, xpparam_t const *xpp,
 
 int xdl_merge(mmfile_t *orig, mmfile_t *mf1, const char *name1,
 		mmfile_t *mf2, const char *name2,
-		xpparam_t const *xpp, int level, mmbuffer_t *result);
+		xpparam_t const *xpp, int flags, mmbuffer_t *result);
 
 #ifdef __cplusplus
 }
diff --git a/xdiff/xmerge.c b/xdiff/xmerge.c
index 1cb65a9..2325f6d 100644
--- a/xdiff/xmerge.c
+++ b/xdiff/xmerge.c
@@ -214,11 +214,15 @@ static int fill_conflict_hunk(xdfenv_t *xe1, const char *name1,
 
 static int xdl_fill_merge_buffer(xdfenv_t *xe1, const char *name1,
 				 xdfenv_t *xe2, const char *name2,
+				 int favor,
 				 xdmerge_t *m, char *dest, int style)
 {
 	int size, i;
 
 	for (size = i = 0; m; m = m->next) {
+		if (favor && !m->mode)
+                	m->mode = favor;
+		
 		if (m->mode == 0)
 			size = fill_conflict_hunk(xe1, name1, xe2, name2,
 						  size, i, style, m, dest);
@@ -391,6 +395,7 @@ static int xdl_do_merge(xdfenv_t *xe1, xdchange_t *xscr1, const char *name1,
 	int i0, i1, i2, chg0, chg1, chg2;
 	int level = flags & XDL_MERGE_LEVEL_MASK;
 	int style = flags & XDL_MERGE_STYLE_MASK;
+	int favor = XDL_MERGE_FAVOR(flags);
 
 	if (style == XDL_MERGE_DIFF3) {
 		/*
@@ -523,14 +528,14 @@ static int xdl_do_merge(xdfenv_t *xe1, xdchange_t *xscr1, const char *name1,
 	/* output */
 	if (result) {
 		int size = xdl_fill_merge_buffer(xe1, name1, xe2, name2,
-			changes, NULL, style);
+			favor, changes, NULL, style);
 		result->ptr = xdl_malloc(size);
 		if (!result->ptr) {
 			xdl_cleanup_merge(changes);
 			return -1;
 		}
 		result->size = size;
-		xdl_fill_merge_buffer(xe1, name1, xe2, name2, changes,
+		xdl_fill_merge_buffer(xe1, name1, xe2, name2, favor, changes,
 				      result->ptr, style);
 	}
 	return xdl_cleanup_merge(changes);
@@ -542,6 +547,8 @@ int xdl_merge(mmfile_t *orig, mmfile_t *mf1, const char *name1,
 	xdchange_t *xscr1, *xscr2;
 	xdfenv_t xe1, xe2;
 	int status;
+	int level = flags & XDL_MERGE_LEVEL_MASK;
+	int favor = XDL_MERGE_FAVOR(flags);
 
 	result->ptr = NULL;
 	result->size = 0;
-- 
1.6.6.rc0.62.gaccf

^ permalink raw reply related

* [PATCH 6/8] Make "subtree" part more orthogonal to the rest of merge-recursive.
From: Avery Pennarun @ 2009-11-26  2:23 UTC (permalink / raw)
  To: git; +Cc: gitster, Avery Pennarun
In-Reply-To: <cover.1259201377.git.apenwarr@gmail.com>

This makes "subtree" more orthogonal to the rest of recursive merge, so
that you can use subtree and ours/theirs features at the same time.  For
example, you can now say:

	git merge -s subtree -Xtheirs other

to merge with "other" branch while shifting it up or down to match the
shape of the tree of the current branch, and resolving conflicts favoring
the changes "other" branch made over changes made in the current branch.

It also allows the prefix used to shift the trees to be specified using
the "-Xsubtree=$prefix" option.  Giving an empty prefix tells the command
to figure out how much to shift trees automatically as we have always
done.  "merge -s subtree" is the same as "merge -s recursive -Xsubtree="
(or "merge -s recursive -Xsubtree").

(Patch originally by Junio Hamano <gitster@pobox.com>.)

Signed-off-by: Avery Pennarun <apenwarr@gmail.com>
---
 builtin-merge-recursive.c |    6 ++-
 builtin-merge.c           |    6 ++-
 cache.h                   |    1 +
 match-trees.c             |   69 ++++++++++++++++++++++++++++++++++++++++++++-
 merge-recursive.c         |   16 +++++++---
 merge-recursive.h         |    3 +-
 6 files changed, 90 insertions(+), 11 deletions(-)

diff --git a/builtin-merge-recursive.c b/builtin-merge-recursive.c
index 53f8f05..d9404e1 100644
--- a/builtin-merge-recursive.c
+++ b/builtin-merge-recursive.c
@@ -28,7 +28,7 @@ int cmd_merge_recursive(int argc, const char **argv, const char *prefix)
 	if (argv[0]) {
 		int namelen = strlen(argv[0]);
 		if (!suffixcmp(argv[0], "-subtree"))
-			o.recursive_variant = MERGE_RECURSIVE_SUBTREE;
+			o.subtree_shift = "";
 	}
 
 	if (argc < 4)
@@ -45,7 +45,9 @@ int cmd_merge_recursive(int argc, const char **argv, const char *prefix)
 			else if (!strcmp(arg+2, "theirs"))
 				o.recursive_variant = MERGE_RECURSIVE_THEIRS;
 			else if (!strcmp(arg+2, "subtree"))
-				o.recursive_variant = MERGE_RECURSIVE_SUBTREE;
+				o.subtree_shift = "";
+			else if (!prefixcmp(arg+2, "subtree="))
+				o.subtree_shift = arg + 10;
 			else
 				die("Unknown option %s", arg);
 			continue;
diff --git a/builtin-merge.c b/builtin-merge.c
index 9a95bc8..a64b8f2 100644
--- a/builtin-merge.c
+++ b/builtin-merge.c
@@ -578,7 +578,7 @@ static int try_merge_strategy(const char *strategy, struct commit_list *common,
 
 		init_merge_options(&o);
 		if (!strcmp(strategy, "subtree"))
-			o.recursive_variant = MERGE_RECURSIVE_SUBTREE;
+			o.subtree_shift = "";
 			
 		for (x = 0; x < xopts_nr; x++) {
 			if (!strcmp(xopts[x], "ours"))
@@ -586,7 +586,9 @@ static int try_merge_strategy(const char *strategy, struct commit_list *common,
 			else if (!strcmp(xopts[x], "theirs"))
 				o.recursive_variant = MERGE_RECURSIVE_THEIRS;
 			else if (!strcmp(xopts[x], "subtree"))
-				o.recursive_variant = MERGE_RECURSIVE_SUBTREE;
+                        	o.subtree_shift = "";
+			else if (!prefixcmp(xopts[x], "subtree="))
+				o.subtree_shift = xopts[x]+8;
 			else
 				die("Unknown option for merge-recursive: -X%s", xopts[x]);
 		}
diff --git a/cache.h b/cache.h
index bf468e5..c6902d2 100644
--- a/cache.h
+++ b/cache.h
@@ -993,6 +993,7 @@ extern int diff_auto_refresh_index;
 
 /* match-trees.c */
 void shift_tree(const unsigned char *, const unsigned char *, unsigned char *, int);
+void shift_tree_by(const unsigned char *, const unsigned char *, unsigned char *, const char *);
 
 /*
  * whitespace rules.
diff --git a/match-trees.c b/match-trees.c
index 0fd6df7..26f7ed1 100644
--- a/match-trees.c
+++ b/match-trees.c
@@ -185,7 +185,7 @@ static void match_trees(const unsigned char *hash1,
  * tree object by replacing it with another tree "hash2".
  */
 static int splice_tree(const unsigned char *hash1,
-		       char *prefix,
+		       const char *prefix,
 		       const unsigned char *hash2,
 		       unsigned char *result)
 {
@@ -264,6 +264,13 @@ void shift_tree(const unsigned char *hash1,
 	char *del_prefix;
 	int add_score, del_score;
 
+	/*
+	 * NEEDSWORK: this limits the recursion depth to hardcoded
+	 * value '2' to avoid excessive overhead.
+	 */
+	if (!depth_limit)
+		depth_limit = 2;
+
 	add_score = del_score = score_trees(hash1, hash2);
 	add_prefix = xcalloc(1, 1);
 	del_prefix = xcalloc(1, 1);
@@ -301,3 +308,63 @@ void shift_tree(const unsigned char *hash1,
 
 	splice_tree(hash1, add_prefix, hash2, shifted);
 }
+
+/*
+ * The user says the trees will be shifted by this much.
+ * Unfortunately we cannot fundamentally tell which one to
+ * be prefixed, as recursive merge can work in either direction.
+ */
+void shift_tree_by(const unsigned char *hash1,
+		   const unsigned char *hash2,
+		   unsigned char *shifted,
+		   const char *shift_prefix)
+{
+	unsigned char sub1[20], sub2[20];
+	unsigned mode1, mode2;
+	unsigned candidate = 0;
+
+	/* Can hash2 be a tree at shift_prefix in tree hash1? */
+	if (!get_tree_entry(hash1, shift_prefix, sub1, &mode1) &&
+	    S_ISDIR(mode1))
+		candidate |= 1;
+
+	/* Can hash1 be a tree at shift_prefix in tree hash2? */
+	if (!get_tree_entry(hash2, shift_prefix, sub2, &mode2) &&
+	    S_ISDIR(mode2))
+		candidate |= 2;
+
+	if (candidate == 3) {
+		/* Both are plausible -- we need to evaluate the score */
+		int best_score = score_trees(hash1, hash2);
+		int score;
+
+		candidate = 0;
+		score = score_trees(sub1, hash2);
+		if (score > best_score) {
+			candidate = 1;
+			best_score = score;
+		}
+		score = score_trees(sub2, hash1);
+		if (score > best_score)
+			candidate = 2;
+	}
+
+	if (!candidate) {
+		/* Neither is plausible -- do not shift */
+		hashcpy(shifted, hash2);
+		return;
+	}
+
+	if (candidate == 1)
+		/*
+		 * shift tree2 down by adding shift_prefix above it
+		 * to match tree1.
+		 */
+		splice_tree(hash1, shift_prefix, hash2, shifted);
+	else
+		/*
+		 * shift tree2 up by removing shift_prefix from it
+		 * to match tree1.
+		 */
+		hashcpy(shifted, sub2);
+}
diff --git a/merge-recursive.c b/merge-recursive.c
index 257bf8f..79b45ed 100644
--- a/merge-recursive.c
+++ b/merge-recursive.c
@@ -21,7 +21,8 @@
 #include "merge-recursive.h"
 #include "dir.h"
 
-static struct tree *shift_tree_object(struct tree *one, struct tree *two)
+static struct tree *shift_tree_object(struct tree *one, struct tree *two,
+				      const char *subtree_shift)
 {
 	unsigned char shifted[20];
 
@@ -29,7 +30,12 @@ static struct tree *shift_tree_object(struct tree *one, struct tree *two)
 	 * NEEDSWORK: this limits the recursion depth to hardcoded
 	 * value '2' to avoid excessive overhead.
 	 */
-	shift_tree(one->object.sha1, two->object.sha1, shifted, 2);
+	if (!*subtree_shift) {
+		shift_tree(one->object.sha1, two->object.sha1, shifted, 0);
+	} else {
+		shift_tree_by(one->object.sha1, two->object.sha1, shifted,
+			      subtree_shift);
+	}
 	if (!hashcmp(two->object.sha1, shifted))
 		return two;
 	return lookup_tree(shifted);
@@ -1213,9 +1219,9 @@ int merge_trees(struct merge_options *o,
 {
 	int code, clean;
 
-	if (o->recursive_variant == MERGE_RECURSIVE_SUBTREE) {
-		merge = shift_tree_object(head, merge);
-		common = shift_tree_object(head, common);
+	if (o->subtree_shift) {
+		merge = shift_tree_object(head, merge, o->subtree_shift);
+		common = shift_tree_object(head, common, o->subtree_shift);
 	}
 
 	if (sha_eq(common->object.sha1, merge->object.sha1)) {
diff --git a/merge-recursive.h b/merge-recursive.h
index 9d54219..d9347ce 100644
--- a/merge-recursive.h
+++ b/merge-recursive.h
@@ -7,10 +7,11 @@ struct merge_options {
 	const char *branch1;
 	const char *branch2;
 	enum {
-        	MERGE_RECURSIVE_SUBTREE = 1,
+		MERGE_RECURSIVE_NORMAL = 0,
         	MERGE_RECURSIVE_OURS,
         	MERGE_RECURSIVE_THEIRS,
 	} recursive_variant;
+	const char *subtree_shift;
 	unsigned buffer_output : 1;
 	int verbosity;
 	int diff_rename_limit;
-- 
1.6.6.rc0.62.gaccf

^ permalink raw reply related

* [PATCH 4/8] Teach git-merge to pass -X<option> to the backend strategy module
From: Avery Pennarun @ 2009-11-26  2:23 UTC (permalink / raw)
  To: git; +Cc: gitster, Avery Pennarun
In-Reply-To: <cover.1259201377.git.apenwarr@gmail.com>

Distinguishing slight variation of modes of operation between the vanilla
merge-recursive and merge-recursive-ours by the command name may have been
an easy way to experiment, but we should bite the bullet and allow backend
specific options to be given by the end user.

(Patch originally by Junio Hamano <gitster@pobox.com>.)

Signed-off-by: Avery Pennarun <apenwarr@gmail.com>
---
 Makefile                     |    3 ---
 builtin-merge-recursive.c    |   21 +++++++++++++++------
 builtin-merge.c              |   40 ++++++++++++++++++++++++++++++++++++----
 t/t6034-merge-ours-theirs.sh |    4 ++--
 4 files changed, 53 insertions(+), 15 deletions(-)

diff --git a/Makefile b/Makefile
index f92b375..5a0b3d4 100644
--- a/Makefile
+++ b/Makefile
@@ -401,8 +401,6 @@ BUILT_INS += git-format-patch$X
 BUILT_INS += git-fsck-objects$X
 BUILT_INS += git-get-tar-commit-id$X
 BUILT_INS += git-init$X
-BUILT_INS += git-merge-recursive-ours$X
-BUILT_INS += git-merge-recursive-theirs$X
 BUILT_INS += git-merge-subtree$X
 BUILT_INS += git-peek-remote$X
 BUILT_INS += git-repo-config$X
@@ -1911,7 +1909,6 @@ check-docs::
 	do \
 		case "$$v" in \
 		git-merge-octopus | git-merge-ours | git-merge-recursive | \
-		git-merge-recursive-ours | git-merge-recursive-theirs | \
 		git-merge-resolve | git-merge-subtree | \
 		git-fsck-objects | git-init-db | \
 		git-?*--?* ) continue ;; \
diff --git a/builtin-merge-recursive.c b/builtin-merge-recursive.c
index f5082da..53f8f05 100644
--- a/builtin-merge-recursive.c
+++ b/builtin-merge-recursive.c
@@ -29,18 +29,27 @@ int cmd_merge_recursive(int argc, const char **argv, const char *prefix)
 		int namelen = strlen(argv[0]);
 		if (!suffixcmp(argv[0], "-subtree"))
 			o.recursive_variant = MERGE_RECURSIVE_SUBTREE;
-		else if (!suffixcmp(argv[0], "-ours"))
-			o.recursive_variant = MERGE_RECURSIVE_OURS;
-		else if (!suffixcmp(argv[0], "-theirs"))
-			o.recursive_variant = MERGE_RECURSIVE_THEIRS;
 	}
 
 	if (argc < 4)
 		usagef("%s <base>... -- <head> <remote> ...", argv[0]);
 
 	for (i = 1; i < argc; ++i) {
-		if (!strcmp(argv[i], "--"))
-			break;
+		const char *arg = argv[i];
+
+		if (!prefixcmp(arg, "--")) {
+			if (!arg[2])
+				break;
+			if (!strcmp(arg+2, "ours"))
+				o.recursive_variant = MERGE_RECURSIVE_OURS;
+			else if (!strcmp(arg+2, "theirs"))
+				o.recursive_variant = MERGE_RECURSIVE_THEIRS;
+			else if (!strcmp(arg+2, "subtree"))
+				o.recursive_variant = MERGE_RECURSIVE_SUBTREE;
+			else
+				die("Unknown option %s", arg);
+			continue;
+		}
 		if (bases_count < ARRAY_SIZE(bases)-1) {
 			unsigned char *sha = xmalloc(20);
 			if (get_sha1(argv[i], sha))
diff --git a/builtin-merge.c b/builtin-merge.c
index df089bb..9a95bc8 100644
--- a/builtin-merge.c
+++ b/builtin-merge.c
@@ -50,13 +50,13 @@ static struct commit_list *remoteheads;
 static unsigned char head[20], stash[20];
 static struct strategy **use_strategies;
 static size_t use_strategies_nr, use_strategies_alloc;
+static const char **xopts;
+static size_t xopts_nr, xopts_alloc;
 static const char *branch;
 static int verbosity;
 
 static struct strategy all_strategy[] = {
 	{ "recursive",  DEFAULT_TWOHEAD | NO_TRIVIAL },
-	{ "recursive-ours", DEFAULT_TWOHEAD | NO_TRIVIAL },
-	{ "recursive-theirs", DEFAULT_TWOHEAD | NO_TRIVIAL },
 	{ "octopus",    DEFAULT_OCTOPUS },
 	{ "resolve",    0 },
 	{ "ours",       NO_FAST_FORWARD | NO_TRIVIAL },
@@ -148,6 +148,17 @@ static int option_parse_strategy(const struct option *opt,
 	return 0;
 }
 
+static int option_parse_x(const struct option *opt,
+			  const char *arg, int unset)
+{
+	if (unset)
+		return 0;
+	
+	ALLOC_GROW(xopts, xopts_nr + 1, xopts_alloc);
+	xopts[xopts_nr++] = xstrdup(arg);
+	return 0;
+}
+
 static int option_parse_n(const struct option *opt,
 			  const char *arg, int unset)
 {
@@ -174,6 +185,8 @@ static struct option builtin_merge_options[] = {
 		"abort if fast-forward is not possible"),
 	OPT_CALLBACK('s', "strategy", &use_strategies, "strategy",
 		"merge strategy to use", option_parse_strategy),
+	OPT_CALLBACK('X', "extended", &xopts, "option=value",
+		"option for selected merge strategy", option_parse_x),
 	OPT_CALLBACK('m', "message", &merge_msg, "message",
 		"message to be used for the merge commit (if any)",
 		option_parse_message),
@@ -536,7 +549,7 @@ static int try_merge_strategy(const char *strategy, struct commit_list *common,
 			      const char *head_arg)
 {
 	const char **args;
-	int i = 0, ret;
+	int i = 0, x = 0, ret;
 	struct commit_list *j;
 	struct strbuf buf = STRBUF_INIT;
 	int index_fd;
@@ -566,6 +579,17 @@ static int try_merge_strategy(const char *strategy, struct commit_list *common,
 		init_merge_options(&o);
 		if (!strcmp(strategy, "subtree"))
 			o.recursive_variant = MERGE_RECURSIVE_SUBTREE;
+			
+		for (x = 0; x < xopts_nr; x++) {
+			if (!strcmp(xopts[x], "ours"))
+				o.recursive_variant = MERGE_RECURSIVE_OURS;
+			else if (!strcmp(xopts[x], "theirs"))
+				o.recursive_variant = MERGE_RECURSIVE_THEIRS;
+			else if (!strcmp(xopts[x], "subtree"))
+				o.recursive_variant = MERGE_RECURSIVE_SUBTREE;
+			else
+				die("Unknown option for merge-recursive: -X%s", xopts[x]);
+		}
 
 		o.branch1 = head_arg;
 		o.branch2 = remoteheads->item->util;
@@ -583,10 +607,16 @@ static int try_merge_strategy(const char *strategy, struct commit_list *common,
 		rollback_lock_file(lock);
 		return clean ? 0 : 1;
 	} else {
-		args = xmalloc((4 + commit_list_count(common) +
+		args = xmalloc((4 + xopts_nr + commit_list_count(common) +
 					commit_list_count(remoteheads)) * sizeof(char *));
 		strbuf_addf(&buf, "merge-%s", strategy);
 		args[i++] = buf.buf;
+		for (x = 0; x < xopts_nr; x++) {
+			char *s = xmalloc(strlen(xopts[x])+2+1);
+			strcpy(s, "--");
+			strcpy(s+2, xopts[x]);
+			args[i++] = s;
+		}
 		for (j = common; j; j = j->next)
 			args[i++] = xstrdup(sha1_to_hex(j->item->object.sha1));
 		args[i++] = "--";
@@ -597,6 +627,8 @@ static int try_merge_strategy(const char *strategy, struct commit_list *common,
 		ret = run_command_v_opt(args, RUN_GIT_CMD);
 		strbuf_release(&buf);
 		i = 1;
+		for (x = 0; x < xopts_nr; x++)
+			free((void *)args[i++]);
 		for (j = common; j; j = j->next)
 			free((void *)args[i++]);
 		i += 2;
diff --git a/t/t6034-merge-ours-theirs.sh b/t/t6034-merge-ours-theirs.sh
index 56a9247..08c9f79 100755
--- a/t/t6034-merge-ours-theirs.sh
+++ b/t/t6034-merge-ours-theirs.sh
@@ -35,7 +35,7 @@ test_expect_success 'plain recursive - should conflict' '
 
 test_expect_success 'recursive favouring theirs' '
 	git reset --hard master &&
-	git merge -s recursive-theirs side &&
+	git merge -s recursive -Xtheirs side &&
 	! grep nine file &&
 	grep nueve file &&
 	! grep 9 file &&
@@ -45,7 +45,7 @@ test_expect_success 'recursive favouring theirs' '
 
 test_expect_success 'recursive favouring ours' '
 	git reset --hard master &&
-	git merge -s recursive-ours side &&
+	git merge -s recursive -X ours side &&
 	grep nine file &&
 	! grep nueve file &&
 	! grep 9 file &&
-- 
1.6.6.rc0.62.gaccf

^ permalink raw reply related

* [PATCH 5/8] Teach git-pull to pass -X<option> to git-merge
From: Avery Pennarun @ 2009-11-26  2:23 UTC (permalink / raw)
  To: git; +Cc: gitster, Avery Pennarun
In-Reply-To: <cover.1259201377.git.apenwarr@gmail.com>

(Patch originally by Junio Hamano <gitster@pobox.com>.)

Signed-off-by: Avery Pennarun <apenwarr@gmail.com>
---
 git-pull.sh                  |   17 +++++++++++++++--
 t/t6034-merge-ours-theirs.sh |    8 ++++++++
 2 files changed, 23 insertions(+), 2 deletions(-)

diff --git a/git-pull.sh b/git-pull.sh
index bfeb4a0..6d961b6 100755
--- a/git-pull.sh
+++ b/git-pull.sh
@@ -18,6 +18,7 @@ test -z "$(git ls-files -u)" ||
 
 strategy_args= diffstat= no_commit= squash= no_ff= ff_only=
 log_arg= verbosity=
+merge_args=
 curr_branch=$(git symbolic-ref -q HEAD)
 curr_branch_short=$(echo "$curr_branch" | sed "s|refs/heads/||")
 rebase=$(git config --bool branch.$curr_branch_short.rebase)
@@ -62,6 +63,18 @@ do
 		esac
 		strategy_args="${strategy_args}-s $strategy "
 		;;
+	-X*)
+		case "$#,$1" in
+		1,-X)
+			usage ;;
+		*,-X)
+			xx="-X $2"
+			shift ;;
+		*,*)
+			xx="$1" ;;
+		esac
+		merge_args="$merge_args$xx "
+		;;
 	-r|--r|--re|--reb|--reba|--rebas|--rebase)
 		rebase=true
 		;;
@@ -216,7 +229,7 @@ fi
 
 merge_name=$(git fmt-merge-msg $log_arg <"$GIT_DIR/FETCH_HEAD") || exit
 test true = "$rebase" &&
-	exec git-rebase $diffstat $strategy_args --onto $merge_head \
+	exec git-rebase $diffstat $strategy_args $merge_args --onto $merge_head \
 	${oldremoteref:-$merge_head}
-exec git-merge $diffstat $no_commit $squash $no_ff $ff_only $log_arg $strategy_args \
+exec git-merge $diffstat $no_commit $squash $no_ff $ff_only $log_arg $strategy_args $merge_args \
 	"$merge_name" HEAD $merge_head $verbosity
diff --git a/t/t6034-merge-ours-theirs.sh b/t/t6034-merge-ours-theirs.sh
index 08c9f79..8ab3d61 100755
--- a/t/t6034-merge-ours-theirs.sh
+++ b/t/t6034-merge-ours-theirs.sh
@@ -53,4 +53,12 @@ test_expect_success 'recursive favouring ours' '
 	! grep 1 file
 '
 
+test_expect_success 'pull with -X' '
+	git reset --hard master && git pull -s recursive -Xours . side &&
+	git reset --hard master && git pull -s recursive -X ours . side &&
+	git reset --hard master && git pull -s recursive -Xtheirs . side &&
+	git reset --hard master && git pull -s recursive -X theirs . side &&
+	git reset --hard master && ! git pull -s recursive -X bork . side
+'
+
 test_done
-- 
1.6.6.rc0.62.gaccf

^ permalink raw reply related

* [PATCH 7/8] Extend merge-subtree tests to test -Xsubtree=dir.
From: Avery Pennarun @ 2009-11-26  2:23 UTC (permalink / raw)
  To: git; +Cc: gitster, Avery Pennarun
In-Reply-To: <cover.1259201377.git.apenwarr@gmail.com>

This tests the configurable -Xsubtree feature of merge-recursive.

Signed-off-by: Avery Pennarun <apenwarr@gmail.com>
---
 t/t6029-merge-subtree.sh |   47 +++++++++++++++++++++++++++++++++++++++++++++-
 1 files changed, 46 insertions(+), 1 deletions(-)

diff --git a/t/t6029-merge-subtree.sh b/t/t6029-merge-subtree.sh
index 5bbfa44..3900d9f 100755
--- a/t/t6029-merge-subtree.sh
+++ b/t/t6029-merge-subtree.sh
@@ -52,6 +52,7 @@ test_expect_success 'initial merge' '
 	git merge -s ours --no-commit gui/master &&
 	git read-tree --prefix=git-gui/ -u gui/master &&
 	git commit -m "Merge git-gui as our subdirectory" &&
+	git checkout -b work &&
 	git ls-files -s >actual &&
 	(
 		echo "100644 $o1 0	git-gui/git-gui.sh"
@@ -65,9 +66,10 @@ test_expect_success 'merge update' '
 	echo git-gui2 > git-gui.sh &&
 	o3=$(git hash-object git-gui.sh) &&
 	git add git-gui.sh &&
+	git checkout -b master2 &&
 	git commit -m "update git-gui" &&
 	cd ../git &&
-	git pull -s subtree gui master &&
+	git pull -s subtree gui master2 &&
 	git ls-files -s >actual &&
 	(
 		echo "100644 $o3 0	git-gui/git-gui.sh"
@@ -76,4 +78,47 @@ test_expect_success 'merge update' '
 	test_cmp expected actual
 '
 
+test_expect_success 'initial ambiguous subtree' '
+	cd ../git &&
+	git reset --hard master &&
+	git checkout -b master2 &&
+	git merge -s ours --no-commit gui/master &&
+	git read-tree --prefix=git-gui2/ -u gui/master &&
+	git commit -m "Merge git-gui2 as our subdirectory" &&
+	git checkout -b work2 &&
+	git ls-files -s >actual &&
+	(
+		echo "100644 $o1 0	git-gui/git-gui.sh"
+		echo "100644 $o1 0	git-gui2/git-gui.sh"
+		echo "100644 $o2 0	git.c"
+	) >expected &&
+	test_cmp expected actual
+'
+
+test_expect_success 'merge using explicit' '
+	cd ../git &&
+	git reset --hard master2 &&
+	git pull -Xsubtree=git-gui gui master2 &&
+	git ls-files -s >actual &&
+	(
+		echo "100644 $o3 0	git-gui/git-gui.sh"
+		echo "100644 $o1 0	git-gui2/git-gui.sh"
+		echo "100644 $o2 0	git.c"
+	) >expected &&
+	test_cmp expected actual
+'
+
+test_expect_success 'merge2 using explicit' '
+	cd ../git &&
+	git reset --hard master2 &&
+	git pull -Xsubtree=git-gui2 gui master2 &&
+	git ls-files -s >actual &&
+	(
+		echo "100644 $o1 0	git-gui/git-gui.sh"
+		echo "100644 $o3 0	git-gui2/git-gui.sh"
+		echo "100644 $o2 0	git.c"
+	) >expected &&
+	test_cmp expected actual
+'
+
 test_done
-- 
1.6.6.rc0.62.gaccf

^ permalink raw reply related

* [PATCH 2/8] builtin-merge.c: call exclude_cmds() correctly.
From: Avery Pennarun @ 2009-11-26  2:23 UTC (permalink / raw)
  To: git; +Cc: gitster, Avery Pennarun
In-Reply-To: <cover.1259201377.git.apenwarr@gmail.com>

We need to call exclude_cmds() after the loop, not during the loop, because
excluding a command from the array can change the indexes of objects in the
array.  The result is that, depending on file ordering, some commands
weren't excluded as they should have been.

Signed-off-by: Avery Pennarun <apenwarr@gmail.com>
---
 builtin-merge.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/builtin-merge.c b/builtin-merge.c
index 57eedd4..855cf65 100644
--- a/builtin-merge.c
+++ b/builtin-merge.c
@@ -107,8 +107,8 @@ static struct strategy *get_strategy(const char *name)
 					found = 1;
 			if (!found)
 				add_cmdname(&not_strategies, ent->name, ent->len);
-			exclude_cmds(&main_cmds, &not_strategies);
 		}
+		exclude_cmds(&main_cmds, &not_strategies);
 	}
 	if (!is_in_cmdlist(&main_cmds, name) && !is_in_cmdlist(&other_cmds, name)) {
 		fprintf(stderr, "Could not find merge strategy '%s'.\n", name);
-- 
1.6.6.rc0.62.gaccf

^ permalink raw reply related

* [PATCH 8/8] Document that merge strategies can now take their own options
From: Avery Pennarun @ 2009-11-26  2:24 UTC (permalink / raw)
  To: git; +Cc: gitster, Avery Pennarun
In-Reply-To: <cover.1259201377.git.apenwarr@gmail.com>

Also document the recently added -Xtheirs, -Xours and -Xsubtree[=path]
options to the merge-recursive strategy.

(Patch originally by Junio Hamano <gitster@pobox.com>.)

Signed-off-by: Avery Pennarun <apenwarr@gmail.com>
---
 Documentation/merge-options.txt    |    4 ++++
 Documentation/merge-strategies.txt |   29 ++++++++++++++++++++++++++++-
 2 files changed, 32 insertions(+), 1 deletions(-)

diff --git a/Documentation/merge-options.txt b/Documentation/merge-options.txt
index fec3394..95244d2 100644
--- a/Documentation/merge-options.txt
+++ b/Documentation/merge-options.txt
@@ -74,3 +74,7 @@ option can be used to override --squash.
 -v::
 --verbose::
 	Be verbose.
+
+-X<option>::
+	Pass merge strategy specific option through to the merge
+	strategy.
diff --git a/Documentation/merge-strategies.txt b/Documentation/merge-strategies.txt
index 42910a3..360dd6f 100644
--- a/Documentation/merge-strategies.txt
+++ b/Documentation/merge-strategies.txt
@@ -1,6 +1,11 @@
 MERGE STRATEGIES
 ----------------
 
+The merge mechanism ('git-merge' and 'git-pull' commands) allows the
+backend 'merge strategies' to be chosen with `-s` option.  Some strategies
+can also take their own options, which can be passed by giving `-X<option>`
+arguments to 'git-merge' and/or 'git-pull'.
+
 resolve::
 	This can only resolve two heads (i.e. the current branch
 	and another branch you pulled from) using a 3-way merge
@@ -20,6 +25,27 @@ recursive::
 	Additionally this can detect and handle merges involving
 	renames.  This is the default merge strategy when
 	pulling or merging one branch.
++
+The 'recursive' strategy can take the following options:
+
+ours;;
+	This option forces conflicting hunks to be auto-resolved cleanly by
+	favoring 'our' version.  Changes from the other tree that do not
+	conflict with our side are reflected to the merge result.
++
+This should not be confused with the 'ours' merge strategy, which does not
+even look at what the other tree contains at all.  That one discards everything
+the other tree did, declaring 'our' history contains all that happened in it.
+
+theirs;;
+	This is opposite of 'ours'.
+
+subtree[=path];;
+	This option is a more advanced form of 'subtree' strategy, where
+	the strategy makes a guess on how two trees must be shifted to
+	match with each other when merging.  Instead, the specified path
+	is prefixed (or stripped from the beginning) to make the shape of
+	two trees to match.
 
 octopus::
 	This resolves cases with more than two heads, but refuses to do
@@ -33,7 +59,8 @@ ours::
 	merge is always that of the current branch head, effectively
 	ignoring all changes from all other branches.  It is meant to
 	be used to supersede old development history of side
-	branches.
+	branches.  Note that this is different from the -Xours option to
+	the 'recursive' merge strategy.
 
 subtree::
 	This is a modified recursive strategy. When merging trees A and
-- 
1.6.6.rc0.62.gaccf

^ permalink raw reply related

* [PATCH 3/8] git-merge-recursive-{ours,theirs}
From: Avery Pennarun @ 2009-11-26  2:23 UTC (permalink / raw)
  To: git; +Cc: gitster, Avery Pennarun
In-Reply-To: <cover.1259201377.git.apenwarr@gmail.com>

This uses the low-level mechanism for "ours" and "theirs" autoresolution
introduced by the previous commit to introduce two additional merge
strategies, merge-recursive-ours and merge-recursive-theirs.

(Patch originally by Junio Hamano <gitster@pobox.com>.)

Signed-off-by: Avery Pennarun <apenwarr@gmail.com>
---
 .gitignore                    |    2 +
 Makefile                      |    3 ++
 builtin-checkout.c            |    2 +-
 builtin-merge-recursive.c     |    9 ++++--
 builtin-merge.c               |    4 ++-
 contrib/examples/git-merge.sh |    3 +-
 git-compat-util.h             |    1 +
 git.c                         |    2 +
 ll-merge.c                    |   20 +++++++-------
 ll-merge.h                    |    2 +-
 merge-recursive.c             |   21 ++++++++++++++-
 merge-recursive.h             |    6 +++-
 strbuf.c                      |    9 ++++++
 t/t6034-merge-ours-theirs.sh  |   56 +++++++++++++++++++++++++++++++++++++++++
 14 files changed, 120 insertions(+), 20 deletions(-)
 create mode 100755 t/t6034-merge-ours-theirs.sh

diff --git a/.gitignore b/.gitignore
index ac02a58..87467d6 100644
--- a/.gitignore
+++ b/.gitignore
@@ -79,6 +79,8 @@
 /git-merge-one-file
 /git-merge-ours
 /git-merge-recursive
+/git-merge-recursive-ours
+/git-merge-recursive-theirs
 /git-merge-resolve
 /git-merge-subtree
 /git-mergetool
diff --git a/Makefile b/Makefile
index 5a0b3d4..f92b375 100644
--- a/Makefile
+++ b/Makefile
@@ -401,6 +401,8 @@ BUILT_INS += git-format-patch$X
 BUILT_INS += git-fsck-objects$X
 BUILT_INS += git-get-tar-commit-id$X
 BUILT_INS += git-init$X
+BUILT_INS += git-merge-recursive-ours$X
+BUILT_INS += git-merge-recursive-theirs$X
 BUILT_INS += git-merge-subtree$X
 BUILT_INS += git-peek-remote$X
 BUILT_INS += git-repo-config$X
@@ -1909,6 +1911,7 @@ check-docs::
 	do \
 		case "$$v" in \
 		git-merge-octopus | git-merge-ours | git-merge-recursive | \
+		git-merge-recursive-ours | git-merge-recursive-theirs | \
 		git-merge-resolve | git-merge-subtree | \
 		git-fsck-objects | git-init-db | \
 		git-?*--?* ) continue ;; \
diff --git a/builtin-checkout.c b/builtin-checkout.c
index 64f3a11..b392d1b 100644
--- a/builtin-checkout.c
+++ b/builtin-checkout.c
@@ -167,7 +167,7 @@ static int checkout_merged(int pos, struct checkout *state)
 	fill_mm(active_cache[pos+2]->sha1, &theirs);
 
 	status = ll_merge(&result_buf, path, &ancestor,
-			  &ours, "ours", &theirs, "theirs", 1);
+			  &ours, "ours", &theirs, "theirs", 1, 0);
 	free(ancestor.ptr);
 	free(ours.ptr);
 	free(theirs.ptr);
diff --git a/builtin-merge-recursive.c b/builtin-merge-recursive.c
index 710674c..f5082da 100644
--- a/builtin-merge-recursive.c
+++ b/builtin-merge-recursive.c
@@ -27,9 +27,12 @@ int cmd_merge_recursive(int argc, const char **argv, const char *prefix)
 	init_merge_options(&o);
 	if (argv[0]) {
 		int namelen = strlen(argv[0]);
-		if (8 < namelen &&
-		    !strcmp(argv[0] + namelen - 8, "-subtree"))
-			o.subtree_merge = 1;
+		if (!suffixcmp(argv[0], "-subtree"))
+			o.recursive_variant = MERGE_RECURSIVE_SUBTREE;
+		else if (!suffixcmp(argv[0], "-ours"))
+			o.recursive_variant = MERGE_RECURSIVE_OURS;
+		else if (!suffixcmp(argv[0], "-theirs"))
+			o.recursive_variant = MERGE_RECURSIVE_THEIRS;
 	}
 
 	if (argc < 4)
diff --git a/builtin-merge.c b/builtin-merge.c
index 855cf65..df089bb 100644
--- a/builtin-merge.c
+++ b/builtin-merge.c
@@ -55,6 +55,8 @@ static int verbosity;
 
 static struct strategy all_strategy[] = {
 	{ "recursive",  DEFAULT_TWOHEAD | NO_TRIVIAL },
+	{ "recursive-ours", DEFAULT_TWOHEAD | NO_TRIVIAL },
+	{ "recursive-theirs", DEFAULT_TWOHEAD | NO_TRIVIAL },
 	{ "octopus",    DEFAULT_OCTOPUS },
 	{ "resolve",    0 },
 	{ "ours",       NO_FAST_FORWARD | NO_TRIVIAL },
@@ -563,7 +565,7 @@ static int try_merge_strategy(const char *strategy, struct commit_list *common,
 
 		init_merge_options(&o);
 		if (!strcmp(strategy, "subtree"))
-			o.subtree_merge = 1;
+			o.recursive_variant = MERGE_RECURSIVE_SUBTREE;
 
 		o.branch1 = head_arg;
 		o.branch2 = remoteheads->item->util;
diff --git a/contrib/examples/git-merge.sh b/contrib/examples/git-merge.sh
index 500635f..8f617fc 100755
--- a/contrib/examples/git-merge.sh
+++ b/contrib/examples/git-merge.sh
@@ -31,10 +31,11 @@ LF='
 '
 
 all_strategies='recur recursive octopus resolve stupid ours subtree'
+all_strategies="$all_strategies recursive-ours recursive-theirs"
 default_twohead_strategies='recursive'
 default_octopus_strategies='octopus'
 no_fast_forward_strategies='subtree ours'
-no_trivial_strategies='recursive recur subtree ours'
+no_trivial_strategies='recursive recur subtree ours recursive-ours recursive-theirs'
 use_strategies=
 
 allow_fast_forward=t
diff --git a/git-compat-util.h b/git-compat-util.h
index 5c59687..f64cc45 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -198,6 +198,7 @@ extern void warning(const char *err, ...) __attribute__((format (printf, 1, 2)))
 extern void set_die_routine(NORETURN_PTR void (*routine)(const char *err, va_list params));
 
 extern int prefixcmp(const char *str, const char *prefix);
+extern int suffixcmp(const char *str, const char *suffix);
 extern time_t tm_to_time_t(const struct tm *tm);
 
 static inline const char *skip_prefix(const char *str, const char *prefix)
diff --git a/git.c b/git.c
index 11544cd..4735f11 100644
--- a/git.c
+++ b/git.c
@@ -332,6 +332,8 @@ static void handle_internal_command(int argc, const char **argv)
 		{ "merge-file", cmd_merge_file },
 		{ "merge-ours", cmd_merge_ours, RUN_SETUP },
 		{ "merge-recursive", cmd_merge_recursive, RUN_SETUP | NEED_WORK_TREE },
+		{ "merge-recursive-ours", cmd_merge_recursive, RUN_SETUP | NEED_WORK_TREE },
+		{ "merge-recursive-theirs", cmd_merge_recursive, RUN_SETUP | NEED_WORK_TREE },
 		{ "merge-subtree", cmd_merge_recursive, RUN_SETUP | NEED_WORK_TREE },
 		{ "mktree", cmd_mktree, RUN_SETUP },
 		{ "mv", cmd_mv, RUN_SETUP | NEED_WORK_TREE },
diff --git a/ll-merge.c b/ll-merge.c
index 2d6b6d6..cc6814f 100644
--- a/ll-merge.c
+++ b/ll-merge.c
@@ -18,7 +18,7 @@ typedef int (*ll_merge_fn)(const struct ll_merge_driver *,
 			   mmfile_t *orig,
 			   mmfile_t *src1, const char *name1,
 			   mmfile_t *src2, const char *name2,
-			   int virtual_ancestor);
+			   int virtual_ancestor, int favor);
 
 struct ll_merge_driver {
 	const char *name;
@@ -38,7 +38,7 @@ static int ll_binary_merge(const struct ll_merge_driver *drv_unused,
 			   mmfile_t *orig,
 			   mmfile_t *src1, const char *name1,
 			   mmfile_t *src2, const char *name2,
-			   int virtual_ancestor)
+			   int virtual_ancestor, int favor)
 {
 	/*
 	 * The tentative merge result is "ours" for the final round,
@@ -59,7 +59,7 @@ static int ll_xdl_merge(const struct ll_merge_driver *drv_unused,
 			mmfile_t *orig,
 			mmfile_t *src1, const char *name1,
 			mmfile_t *src2, const char *name2,
-			int virtual_ancestor)
+			int virtual_ancestor, int favor)
 {
 	xpparam_t xpp;
 	int style = 0;
@@ -73,7 +73,7 @@ static int ll_xdl_merge(const struct ll_merge_driver *drv_unused,
 				       path,
 				       orig, src1, name1,
 				       src2, name2,
-				       virtual_ancestor);
+				       virtual_ancestor, favor);
 	}
 
 	memset(&xpp, 0, sizeof(xpp));
@@ -82,7 +82,7 @@ static int ll_xdl_merge(const struct ll_merge_driver *drv_unused,
 	return xdl_merge(orig,
 			 src1, name1,
 			 src2, name2,
-			 &xpp, XDL_MERGE_ZEALOUS | style,
+			 &xpp, XDL_MERGE_ZEALOUS | style | favor,
 			 result);
 }
 
@@ -92,7 +92,7 @@ static int ll_union_merge(const struct ll_merge_driver *drv_unused,
 			  mmfile_t *orig,
 			  mmfile_t *src1, const char *name1,
 			  mmfile_t *src2, const char *name2,
-			  int virtual_ancestor)
+			  int virtual_ancestor, int favor)
 {
 	char *src, *dst;
 	long size;
@@ -104,7 +104,7 @@ static int ll_union_merge(const struct ll_merge_driver *drv_unused,
 	git_xmerge_style = 0;
 	status = ll_xdl_merge(drv_unused, result, path_unused,
 			      orig, src1, NULL, src2, NULL,
-			      virtual_ancestor);
+			      virtual_ancestor, favor);
 	git_xmerge_style = saved_style;
 	if (status <= 0)
 		return status;
@@ -165,7 +165,7 @@ static int ll_ext_merge(const struct ll_merge_driver *fn,
 			mmfile_t *orig,
 			mmfile_t *src1, const char *name1,
 			mmfile_t *src2, const char *name2,
-			int virtual_ancestor)
+			int virtual_ancestor, int favor)
 {
 	char temp[3][50];
 	struct strbuf cmd = STRBUF_INIT;
@@ -356,7 +356,7 @@ int ll_merge(mmbuffer_t *result_buf,
 	     mmfile_t *ancestor,
 	     mmfile_t *ours, const char *our_label,
 	     mmfile_t *theirs, const char *their_label,
-	     int virtual_ancestor)
+	     int virtual_ancestor, int favor)
 {
 	const char *ll_driver_name;
 	const struct ll_merge_driver *driver;
@@ -369,5 +369,5 @@ int ll_merge(mmbuffer_t *result_buf,
 	return driver->fn(driver, result_buf, path,
 			  ancestor,
 			  ours, our_label,
-			  theirs, their_label, virtual_ancestor);
+			  theirs, their_label, virtual_ancestor, favor);
 }
diff --git a/ll-merge.h b/ll-merge.h
index 5388422..2c94fdb 100644
--- a/ll-merge.h
+++ b/ll-merge.h
@@ -10,6 +10,6 @@ int ll_merge(mmbuffer_t *result_buf,
 	     mmfile_t *ancestor,
 	     mmfile_t *ours, const char *our_label,
 	     mmfile_t *theirs, const char *their_label,
-	     int virtual_ancestor);
+	     int virtual_ancestor, int favor);
 
 #endif
diff --git a/merge-recursive.c b/merge-recursive.c
index a91208f..257bf8f 100644
--- a/merge-recursive.c
+++ b/merge-recursive.c
@@ -642,6 +642,23 @@ static int merge_3way(struct merge_options *o,
 	mmfile_t orig, src1, src2;
 	char *name1, *name2;
 	int merge_status;
+	int favor;
+	
+	if (o->call_depth)
+        	favor = 0;
+	else {
+		switch (o->recursive_variant) {
+		case MERGE_RECURSIVE_OURS:
+			favor = XDL_MERGE_FAVOR_OURS;
+			break;
+		case MERGE_RECURSIVE_THEIRS:
+			favor = XDL_MERGE_FAVOR_THEIRS;
+			break;
+		default:
+			favor = 0;
+			break;
+		}
+	}
 
 	if (strcmp(a->path, b->path)) {
 		name1 = xstrdup(mkpath("%s:%s", branch1, a->path));
@@ -657,7 +674,7 @@ static int merge_3way(struct merge_options *o,
 
 	merge_status = ll_merge(result_buf, a->path, &orig,
 				&src1, name1, &src2, name2,
-				o->call_depth);
+				o->call_depth, favor);
 
 	free(name1);
 	free(name2);
@@ -1196,7 +1213,7 @@ int merge_trees(struct merge_options *o,
 {
 	int code, clean;
 
-	if (o->subtree_merge) {
+	if (o->recursive_variant == MERGE_RECURSIVE_SUBTREE) {
 		merge = shift_tree_object(head, merge);
 		common = shift_tree_object(head, common);
 	}
diff --git a/merge-recursive.h b/merge-recursive.h
index fd138ca..9d54219 100644
--- a/merge-recursive.h
+++ b/merge-recursive.h
@@ -6,7 +6,11 @@
 struct merge_options {
 	const char *branch1;
 	const char *branch2;
-	unsigned subtree_merge : 1;
+	enum {
+        	MERGE_RECURSIVE_SUBTREE = 1,
+        	MERGE_RECURSIVE_OURS,
+        	MERGE_RECURSIVE_THEIRS,
+	} recursive_variant;
 	unsigned buffer_output : 1;
 	int verbosity;
 	int diff_rename_limit;
diff --git a/strbuf.c b/strbuf.c
index a6153dc..d71a623 100644
--- a/strbuf.c
+++ b/strbuf.c
@@ -10,6 +10,15 @@ int prefixcmp(const char *str, const char *prefix)
 			return (unsigned char)*prefix - (unsigned char)*str;
 }
 
+int suffixcmp(const char *str, const char *suffix)
+{
+	int len = strlen(str), suflen = strlen(suffix);
+	if (len < suflen)
+		return -1;
+	else
+		return strcmp(str + len - suflen, suffix);
+}
+
 /*
  * Used as the default ->buf value, so that people can always assume
  * buf is non NULL and ->buf is NUL terminated even for a freshly
diff --git a/t/t6034-merge-ours-theirs.sh b/t/t6034-merge-ours-theirs.sh
new file mode 100755
index 0000000..56a9247
--- /dev/null
+++ b/t/t6034-merge-ours-theirs.sh
@@ -0,0 +1,56 @@
+#!/bin/sh
+
+test_description='Merge-recursive ours and theirs variants'
+. ./test-lib.sh
+
+test_expect_success setup '
+	for i in 1 2 3 4 5 6 7 8 9
+	do
+		echo "$i"
+	done >file &&
+	git add file &&
+	cp file elif &&
+	git commit -m initial &&
+
+	sed -e "s/1/one/" -e "s/9/nine/" >file <elif &&
+	git commit -a -m ours &&
+
+	git checkout -b side HEAD^ &&
+
+	sed -e "s/9/nueve/" >file <elif &&
+	git commit -a -m theirs &&
+
+	git checkout master^0
+'
+
+test_expect_success 'plain recursive - should conflict' '
+	git reset --hard master &&
+	test_must_fail git merge -s recursive side &&
+	grep nine file &&
+	grep nueve file &&
+	! grep 9 file &&
+	grep one file &&
+	! grep 1 file
+'
+
+test_expect_success 'recursive favouring theirs' '
+	git reset --hard master &&
+	git merge -s recursive-theirs side &&
+	! grep nine file &&
+	grep nueve file &&
+	! grep 9 file &&
+	grep one file &&
+	! grep 1 file
+'
+
+test_expect_success 'recursive favouring ours' '
+	git reset --hard master &&
+	git merge -s recursive-ours side &&
+	grep nine file &&
+	! grep nueve file &&
+	! grep 9 file &&
+	grep one file &&
+	! grep 1 file
+'
+
+test_done
-- 
1.6.6.rc0.62.gaccf

^ permalink raw reply related

* Re: [RFC/PATCHv8 08/10] fast-import: Proper notes tree manipulation using the notes API
From: Shawn O. Pearce @ 2009-11-26  2:46 UTC (permalink / raw)
  To: Johan Herland; +Cc: git, gitster
In-Reply-To: <1258681154-2167-9-git-send-email-johan@herland.net>

Johan Herland <johan@herland.net> wrote:
> This patch teaches 'git fast-import' to use the notes API to organize
...
> This patch is substantially different from the previous iteration.
> Unloading (and reloading) the notes tree along with its corresponding
> branch was relatively straightforward to fix, but avoiding the
> destroying and re-adding of all the notes in every commit was much
> harder. After 3-4 attempts at a simpler (but fundamentally broken)
> approach, I finally landed on this. I'm not satisfied with the amount
> of code introduced by this patch, and would be happy if someone found
> a better/shorter/more elegant way to solve this problem.

Yea, I agree, I'm not happy with the amount of complex code added
to implement this.  But I can't say there's a better way to do it
and still reuse the notes code.  Maybe its just worth breaking away
from the notes code altogether?  fast-import also implements its
own pack formatting functions because reusing them from pack-objects
was just too ugly.

Aside from a few minor nits below though, I could ACK this, it at
least avoids the nasty corners that can arise when there are a lot
of branches and tries to minimize the cost when there are many notes.
 
> diff --git a/fast-import.c b/fast-import.c
> +
> +static void add_to_replace_list(
> +		struct tree_entry_replace **replace_list,
> +		const char *old_path, const char *new_path)
> +{
> +	struct tree_entry_replace *r = (struct tree_entry_replace *)
> +		xmalloc(sizeof(struct tree_entry_replace));
> +	r->next = (*replace_list)->next;
> +	r->old_path = xstrdup(old_path);
> +	r->new_path = xstrdup(new_path);
> +	(*replace_list)->next = r;
> +	*replace_list = r;

Really?  I don't get why you are replacing the head's next with r
only to then replace head itself with r.

> @@ -2265,6 +2540,18 @@ static void parse_new_commit(void)
>  			break;
>  	}
> 
> +	if (notes) {
> +		/* reconcile diffs between b->branch_tree and the notes tree */
> +		struct reconcile_notes_tree_helper_data d;
> +		struct tree_entry_replace *replace_list =
> +			xcalloc(1, sizeof(struct tree_entry_replace));

Oh, I see.  The issue I had with understanding add_to_replace_list()
is due to this spot allocating a blank header node.  Normally we do
this with a pointer to a pointer and initialize NULL:

	struct tree_entry_replace *list = NULL;
	struct tree_entry_replace **replace_list = &list;

Can we avoid this blank header node?  I think it comlicates the code,
e.g. in process_replace_list() you have to skip over the blank node
by testing for both paths being NULL.

-- 
Shawn.

^ permalink raw reply

* Re: [PATCH 2/8] builtin-merge.c: call exclude_cmds() correctly.
From: Junio C Hamano @ 2009-11-26  5:36 UTC (permalink / raw)
  To: Avery Pennarun; +Cc: git
In-Reply-To: <905749faf5ccb2c7c54d3318dbc662d69daf8d0e.1259201377.git.apenwarr@gmail.com>

"Avery Pennarun" <apenwarr@gmail.com> writes:

> We need to call exclude_cmds() after the loop, not during the loop, because
> excluding a command from the array can change the indexes of objects in the
> array.  The result is that, depending on file ordering, some commands
> weren't excluded as they should have been.

As an independent bugfix, I would prefer this to be made against 'maint'
and not as a part of this series.

How did you notice it?  Can you make a test case out of your experience of
noticing this bug in the first place, by the way (I am suspecting that you
saw some breakage and chased it in the debugger)?

^ permalink raw reply

* Re: [PATCH 3/8] git-merge-recursive-{ours,theirs}
From: Junio C Hamano @ 2009-11-26  6:15 UTC (permalink / raw)
  To: Avery Pennarun; +Cc: git, gitster
In-Reply-To: <7e1f1179fc5fe2f568e2c75f75366fa40d7bbbfb.1259201377.git.apenwarr@gmail.com>

"Avery Pennarun" <apenwarr@gmail.com> writes:

> This uses the low-level mechanism for "ours" and "theirs" autoresolution
> introduced by the previous commit to introduce two additional merge
> strategies, merge-recursive-ours and merge-recursive-theirs.
>
> (Patch originally by Junio Hamano <gitster@pobox.com>.)
>
> Signed-off-by: Avery Pennarun <apenwarr@gmail.com>

Two comments.

 - The original series was done over a few weeks in 'pu' and this
   intermediate step was done before a better alternative of not using
   these two extra merge strategies were discovered ("...may have been an
   easy way to experiment, but we should bite the bullet", in the next
   patch).

   As the second round to seriously polish the series for inclusion, it
   would make much more sense to squash this with the next patch to erase
   this failed approach that has already been shown as clearly inferiour.

 - I think we should avoid adding the extra argument to ll_merge_fn() by
   combining virtual_ancestor and favor into one "flags" parameter.  If
   you do so, we do not have to change the callsites again next time we
   need to add new optional features that needs only a few bits.

   I vaguely recall that I did the counterpart of this patch that way
   exactly for the above reason, but it is more than a year ago, so maybe
   I didn't do it that way.

^ permalink raw reply

* Re: [PATCH 4/8] Teach git-merge to pass -X<option> to the backend strategy module
From: Junio C Hamano @ 2009-11-26  6:16 UTC (permalink / raw)
  To: Avery Pennarun; +Cc: git, gitster
In-Reply-To: <73a42e99b4a083c74b017caf2970d1bbf5886b96.1259201377.git.apenwarr@gmail.com>

Avery Pennarun <apenwarr@gmail.com> writes:

> Distinguishing slight variation of modes of operation between the vanilla
> merge-recursive and merge-recursive-ours by the command name may have been
> an easy way to experiment, but we should bite the bullet and allow backend
> specific options to be given by the end user.
>
> (Patch originally by Junio Hamano <gitster@pobox.com>.)
>
> Signed-off-by: Avery Pennarun <apenwarr@gmail.com>
> ---
>  Makefile                     |    3 ---
>  builtin-merge-recursive.c    |   21 +++++++++++++++------
>  builtin-merge.c              |   40 ++++++++++++++++++++++++++++++++++++----
>  t/t6034-merge-ours-theirs.sh |    4 ++--
>  4 files changed, 53 insertions(+), 15 deletions(-)

You added .gitignore entries for the two programs previously, and are
removing them in this patch, but forgot to remove them from .gitignore in
this patch.

As I already suggested you to, if you squash this to the previous one, it
is not a big deal, though.

> diff --git a/builtin-merge.c b/builtin-merge.c
> index df089bb..9a95bc8 100644
> --- a/builtin-merge.c
> +++ b/builtin-merge.c
> @@ -148,6 +148,17 @@ static int option_parse_strategy(const struct option *opt,
>  	return 0;
>  }
>  
> +static int option_parse_x(const struct option *opt,
> +			  const char *arg, int unset)
> +{
> +	if (unset)
> +		return 0;

Should "merge --no-extended" silently be ignored, or be diagnosed as an
error?

> @@ -174,6 +185,8 @@ static struct option builtin_merge_options[] = {
>  		"abort if fast-forward is not possible"),
>  	OPT_CALLBACK('s', "strategy", &use_strategies, "strategy",
>  		"merge strategy to use", option_parse_strategy),
> +	OPT_CALLBACK('X', "extended", &xopts, "option=value",
> +		"option for selected merge strategy", option_parse_x),

I actually didn't name X for "extended" but more for "external" (to the
merge program proper).  "--strategy-option" perhaps?

^ permalink raw reply

* Re: [PATCH 5/8] Teach git-pull to pass -X<option> to git-merge
From: Junio C Hamano @ 2009-11-26  6:16 UTC (permalink / raw)
  To: Avery Pennarun; +Cc: git
In-Reply-To: <1ff0b2f7e3fae4cc6c7610c92711f33df9a3d07c.1259201377.git.apenwarr@gmail.com>

Avery Pennarun <apenwarr@gmail.com> writes:

> (Patch originally by Junio Hamano <gitster@pobox.com>.)
>
> Signed-off-by: Avery Pennarun <apenwarr@gmail.com>

You should take the full authorship of this patch without even mentioning
"originally by".  It has no code from me.

> @@ -216,7 +229,7 @@ fi
>  
>  merge_name=$(git fmt-merge-msg $log_arg <"$GIT_DIR/FETCH_HEAD") || exit
>  test true = "$rebase" &&
> -	exec git-rebase $diffstat $strategy_args --onto $merge_head \
> +	exec git-rebase $diffstat $strategy_args $merge_args --onto $merge_head \
>  	${oldremoteref:-$merge_head}
> -exec git-merge $diffstat $no_commit $squash $no_ff $ff_only $log_arg $strategy_args \
> +exec git-merge $diffstat $no_commit $squash $no_ff $ff_only $log_arg $strategy_args $merge_args \
>  	"$merge_name" HEAD $merge_head $verbosity

This part needs the usual "sq-then-eval" trick; -X subtree="My Programs"
on the command line will be split by the shell if you didn't do so.

^ permalink raw reply

* Re: [PATCH 6/8] Make "subtree" part more orthogonal to the rest of merge-recursive.
From: Junio C Hamano @ 2009-11-26  6:17 UTC (permalink / raw)
  To: Avery Pennarun; +Cc: git, gitster
In-Reply-To: <c78d4c177f470e0f2f64314321df12e1a59077bf.1259201377.git.apenwarr@gmail.com>

"Avery Pennarun" <apenwarr@gmail.com> writes:

> diff --git a/merge-recursive.c b/merge-recursive.c
> index 257bf8f..79b45ed 100644
> --- a/merge-recursive.c
> +++ b/merge-recursive.c
> @@ -21,7 +21,8 @@
>  #include "merge-recursive.h"
>  #include "dir.h"
>  
> -static struct tree *shift_tree_object(struct tree *one, struct tree *two)
> +static struct tree *shift_tree_object(struct tree *one, struct tree *two,
> +				      const char *subtree_shift)
>  {
>  	unsigned char shifted[20];
>  
> @@ -29,7 +30,12 @@ static struct tree *shift_tree_object(struct tree *one, struct tree *two)
>  	 * NEEDSWORK: this limits the recursion depth to hardcoded
>  	 * value '2' to avoid excessive overhead.
>  	 */
> -	shift_tree(one->object.sha1, two->object.sha1, shifted, 2);

The block comment with NEEDSWORK should be removed from here.  I may have
forgotten to in the original one, but that is not an excuse to replicate a
bad job ;-)

^ permalink raw reply

* Re: [PATCH 1/8] git-merge-file --ours, --theirs
From: Junio C Hamano @ 2009-11-26  6:17 UTC (permalink / raw)
  To: Avery Pennarun; +Cc: git
In-Reply-To: <d243a513ffb8da4272f7a0e13a711f9b65195c25.1259201377.git.apenwarr@gmail.com>

"Avery Pennarun" <apenwarr@gmail.com> writes:

> ...
> A larger problem is that this tends to encourage a bad workflow by
> allowing them to record such a mixed up half-merge result as a full commit
> without auditing.  This commit does not tackle this latter issue.  In git,
> we usually give long enough rope to users with strange wishes as long as
> the risky features is not on by default.

Typo/Grammo.  "risky features are not on by default".

> (Patch originally by Junio Hamano <gitster@pobox.com>.)
>
> Signed-off-by: Avery Pennarun <apenwarr@gmail.com>

Except for parse-optification, this one is more or less a verbatim copy of
my patch, and I think I probably deserve an in-body "From: " line for this
[PATCH 1/8], [PATCH 6/8] and [PATCH 8/8] to take the full authorship of
them.

> diff --git a/xdiff/xdiff.h b/xdiff/xdiff.h
> index 4da052a..2cce49d 100644
> --- a/xdiff/xdiff.h
> +++ b/xdiff/xdiff.h
> @@ -58,6 +58,11 @@ extern "C" {
>  #define XDL_MERGE_ZEALOUS_ALNUM 3
>  #define XDL_MERGE_LEVEL_MASK 0x0f
>  
> +/* merge favor modes */
> +#define XDL_MERGE_FAVOR_OURS 0x0010
> +#define XDL_MERGE_FAVOR_THEIRS 0x0020
> +#define XDL_MERGE_FAVOR(flags) (((flags)>>4) & 3)

This is a bad change.  It forces the high-level layer of the resulting
code to be aware that the favor bits are shifted by 4 and it is different
from what the low-level layer expects.  If I were porting it to
parse-options, I would have kept OURS = 1 and THEIRS = 2 as the original
patch, and instead did something like:

 	ret = xdl_merge(mmfs + 1, mmfs + 0, names[0], mmfs + 2, names[2],
-			&xpp, merge_level | merge_style, &result);
+			&xpp, XDL_MERGE_FLAGS(merge_level, merge_style, merge_favor), &result);

with an updated definition like this:

    #define XDL_MERGE_FLAGS(level, style, favor) ((level)|(style)|((favor)<<4)

^ permalink raw reply


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