* [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
* Re: [PATCH 1/8] git-merge-file --ours, --theirs
From: Nanako Shiraishi @ 2009-11-26 6:37 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Avery Pennarun, git
In-Reply-To: <7vy6ltdd2l.fsf@alter.siamese.dyndns.org>
Quoting Junio C Hamano <gitster@pobox.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.
Could you give an guideline to decide when to take authorship and when to
give it to others? The above seems somewhat arbitrary to me.
--
Nanako Shiraishi
http://ivory.ap.teacup.com/nanako3/
^ permalink raw reply
* Re: [PATCH 1/8] git-merge-file --ours, --theirs
From: Junio C Hamano @ 2009-11-26 7:05 UTC (permalink / raw)
To: Nanako Shiraishi; +Cc: Avery Pennarun, git
In-Reply-To: <20091126153726.6117@nanako3.lavabit.com>
Nanako Shiraishi <nanako3@lavabit.com> writes:
> Quoting Junio C Hamano <gitster@pobox.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.
>
> Could you give an guideline to decide when to take authorship and when to
> give it to others? The above seems somewhat arbitrary to me.
It might seem that way to you, as you do not write much C, but I am
reasonably sure Avery understands after having worked on the series.
Imagine that Avery were an area expert (the subsystem maintainer) on "git
merge" and downwards, and somebody who did not know that "merge" has
already been rewritten in C, nor some parts of the system have been
rewritten to use parse-options, submitted a patch series for review and
Avery is helping to polish it up [*1*].
As the subsystem maintainer, Avery looks at the patches, updates parts of
the code that is based on obsolete infrastructure, adds lacking tests and
documentation as necessary, and forwards the tested result upwards for
inclusion. How would the messages from Avery to me look?
Patches that were majorly reworked should be attributed to Avery, and
obviously new patches that are added for missing tests should be, too.
For example, changes made to git-merge.sh by the original submitter must
be discarded and redone from scratch to builtin-merge.c, and if you look
at the changes, it would be quite obvious that the original patch wouldn't
have served as anything more than giving the specification.
But the ones with minor updates should retain the original authorship.
It unfortunately is not black-and-white, though.
In any case, where does Avery's credit go? Is there a point in helping to
polish others' patches?
It is recoded on the Signed-off-by line. When somebody passes a patch
from somebody else, an S-o-b is added for DCO purposes, but it also leaves
the "patch trail"---these people looked at the patch, spent effort to make
sure it is suitable for inclusion by reviewing, polishing, and enhancing.
A subsystem maintainer, or anybody who helps to polish others
contribution, may not necessarily have his name as the "author" of the
patch, and if the patch forwarding is done via e-mail, his name won't be
on the "committer" line either. But the contribution is still noted and
appreciated, and the hint to pay attention to is by counting non-author
S-o-b and Tested-by lines in the commit messages.
cf. http://lwn.net/SubscriberLink/363456/50efdecf49af77ba/ check the last
table.
[Footnote]
*1* That somebody happens to be me from 18 months ago, but the important
point here is that the person is not Avery as the subsystem maintainer (in
other words, it is immaterial that it happens to be the same person as the
toplevel maintainer).
^ permalink raw reply
* Re: cvsexportcommit dies when applying an (empty) merge commit
From: Robin Rosenberg @ 2009-11-26 6:51 UTC (permalink / raw)
To: Nick Woolley; +Cc: git
In-Reply-To: <4B0D1C1A.60707@yahoo.co.uk>
onsdag 25 november 2009 12:59:22 skrev Nick Woolley:
> Hi,
>
> I have a git repository with a merge point on the master branch. This
> merge commit is empty, and just contains a commit message:
>
> Merge commit 'otherbranch'
>
> I'm trying to export this branch into CVS using git-cvsexportcommit (the
> latest version from the master branch). It's actually done in a wrapper
> script [1] but the command that gets invoked is essentially:
>
> git cvsexportcommit -p -v -u -w 'cvscheckout/HEAD/my-cvs-module' -c \
> <parent commit> <commit>
>
> Where <commit> is the empty merge commit. However this invocation dies and
> aborts the process of exporting the branch half way.
>
> The fatal error I get is:
>
> Applying to CVS commit <commit> from parent <parent commit>
> Checking if patch will apply
> Applying
> error: No changes
> cannot patch at /usr/lib/git-core/git-cvsexportcommit line 324.
>
[....]
> Is the existing behaviour deliberately fatal, or is this worth supplying a
> patch for?
I'm not the only contributor, but I'd say its a omission. cvsexportcommit
doesn't export commits. It export deltas, that is the change relative to one
of the parents. It is reasonable that cvsexportcommit can "export" an
empty commit by doing nothing and exiting with 0. Printing some kind of
warning seems reasonable too.
- robin
^ permalink raw reply
* Re: [PATCH 1/8] git-merge-file --ours, --theirs
From: Nanako Shiraishi @ 2009-11-26 7:30 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Avery Pennarun, git
In-Reply-To: <7vvdgxbwav.fsf@alter.siamese.dyndns.org>
Quoting Junio C Hamano <gitster@pobox.com> writes:
> In any case, where does Avery's credit go? Is there a point in helping to
> polish others' patches?
>
> It is recoded on the Signed-off-by line. When somebody passes a patch
> from somebody else, an S-o-b is added for DCO purposes, but it also leaves
> the "patch trail"---these people looked at the patch, spent effort to make
> sure it is suitable for inclusion by reviewing, polishing, and enhancing.
> A subsystem maintainer, or anybody who helps to polish others
> contribution, may not necessarily have his name as the "author" of the
> patch, and if the patch forwarding is done via e-mail, his name won't be
> on the "committer" line either. But the contribution is still noted and
> appreciated, and the hint to pay attention to is by counting non-author
> S-o-b and Tested-by lines in the commit messages.
I understand. Thank you for a detailed explanation.
--
Nanako Shiraishi
http://ivory.ap.teacup.com/nanako3/
^ permalink raw reply
* Re: What's cooking in git.git (Nov 2009, #06; Wed, 25)
From: Johan Herland @ 2009-11-26 7:37 UTC (permalink / raw)
To: Sverre Rabbelier; +Cc: Junio C Hamano, Daniel Barkalow, git
In-Reply-To: <fabb9a1e0911251715u661ce0aem79a4d700d552e105@mail.gmail.com>
On Thursday 26 November 2009, Sverre Rabbelier wrote:
> 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?
As I replied to an earlier "What's cooking" email, it all looks good to me,
but I would really like Daniel's ACK on the transport layer parts.
...Johan
--
Johan Herland, <johan@herland.net>
www.herland.net
^ permalink raw reply
* Re: Please help with GIT install problem.
From: Tom G. Christensen @ 2009-11-26 7:44 UTC (permalink / raw)
To: Stephan T.; +Cc: git
In-Reply-To: <377205.65475.qm@web30806.mail.mud.yahoo.com>
Stephan T. wrote:
> Hello there,
>
> My system is a:
> % uname -a
> Linux naboo 2.4.21-50.ELsmp #1 SMP Tue May 8 17:18:29 EDT 2007 i686 i686 i386 GNU/Linux
>
Looks like you're on RHEL 3 or a derivative.
> Trying to install "git-1.6.5.3". Configure goes happily to the end but make gives me the following error:
>
>> make
> CC fast-import.o
> In file included from /usr/include/openssl/ssl.h:179,
> from git-compat-util.h:138,
> from builtin.h:4,
> from fast-import.c:143:
> /usr/include/openssl/kssl.h:72:18: krb5.h: No such file or directory
> In file included from /usr/include/openssl/ssl.h:179,
> from git-compat-util.h:138,
> from builtin.h:4,
> from fast-import.c:143:
> /usr/include/openssl/kssl.h:134: syntax error before "krb5_enctype"
>
>
> What is missing on my system?
>
You need to add "-I/usr/kerberos/include" to CFLAGS to build with
openssl on el3.
Instead of doing your own build you could install the packages I
maintain for my own use:
http://jupiterrise.com/tmp/git-el3
This is just a temporary location, in the future I hope so have this in
rpmforge.
Not that perl-Git will require perl-Error from rpmforge
(http://packages.sw.be/perl-Error)
-tgc
^ permalink raw reply
* Re: [PATCH/RFC 01/11] mingw: add network-wrappers for daemon
From: Martin Storsjö @ 2009-11-26 8:24 UTC (permalink / raw)
To: Erik Faye-Lund; +Cc: msysgit, git, dotzenlabs, Erik Faye-Lund
In-Reply-To: <1259196260-3064-2-git-send-email-kusmabite@gmail.com>
Hi,
First of all, great that you're working on adding daemon support for
windows!
On Thu, 26 Nov 2009, Erik Faye-Lund wrote:
> +static void wsa_init(void)
> {
> + static int initialized = 0;
> WSADATA wsa;
>
> + if (initialized)
> + return;
> +
> if (WSAStartup(MAKEWORD(2,2), &wsa))
> die("unable to initialize winsock subsystem, error %d",
> WSAGetLastError());
> atexit((void(*)(void)) WSACleanup);
> + initialized = 1;
> +}
Something similar to this was merged into master recently as part of my
mingw/ipv6 patches, so by rebasing your patch on top of that, this patch
will probably get a bit smaller.
Also, the getaddrinfo-compatibility wrappers perhaps may need some minor
updates to handle the use cases needed for setting up listening sockets.
// Martin
^ permalink raw reply
* Re: OS X and umlauts in file names
From: Thomas Singer @ 2009-11-26 8:28 UTC (permalink / raw)
To: Andreas Schwab; +Cc: Daniel Barkalow, git
In-Reply-To: <m21vjmkyxh.fsf@igel.home>
Hi Andreas,
Thank you for this hint. When trying
toms-mac-mini:git-umlauts tom$ git stage $'U\314\210berla\314\210nge.txt'
git shows me no error or other output, but invoking 'git status' again shows
no difference, the file is still showing up as new file.
I've also tried to use double backslashes, but I could not enter a backslash
in the OS X Terminal (works fine in other applications). :(
--
Tom
Andreas Schwab wrote:
> Thomas Singer <thomas.singer@syntevo.com> writes:
>
>> I've did following:
>>
>> toms-mac-mini:git-umlauts tom$ ls
>> Überlänge.txt
>> toms-mac-mini:git-umlauts tom$ git status
>> # On branch master
>> #
>> # Initial commit
>> #
>> # Changes to be committed:
>> # (use "git rm --cached <file>..." to unstage)
>> #
>> # new file: "U\314\210berla\314\210nge.txt"
>> #
>> toms-mac-mini:git-umlauts tom$ git stage "U\314\210berla\314\210nge.txt"
>> fatal: pathspec 'U\314\210berla\314\210nge.txt' did not match any files
>
> Try $'U\314\210berla\314\210nge.txt' instead.
> "U\314\210berla\314\210nge.txt" is the same as
> "U\\314\\210berla\\314\\210nge.txt" to the shell.
>
> Andreas.
>
^ permalink raw reply
* Re: [egit] Git repository with multiple eclipse projects ?
From: Yann Simon @ 2009-11-26 8:39 UTC (permalink / raw)
To: Shawn O. Pearce; +Cc: Yann Dirson, git
In-Reply-To: <20091126004817.GL11919@spearce.org>
2009/11/26 Shawn O. Pearce <spearce@spearce.org>:
> Yann Dirson <ydirson@linagora.com> wrote:
>> 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.
That was the question I initialy understood.
^ permalink raw reply
* [PATCH (resend)] Let core.excludesfile default to ~/.gitexcludes.
From: Matthieu Moy @ 2009-11-26 10:35 UTC (permalink / raw)
To: git, gitster; +Cc: Matthieu Moy
In-Reply-To: <1258840832-22130-1-git-send-email-Matthieu.Moy@imag.fr>
Most users will set it to ~/.gitsomething. ~/.gitignore would conflict
with per-directory ignore file if ~/ is managed by Git, so ~/.gitexcludes
is a sane default.
Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
---
(just resending since the patch seems to have got lost in the process)
Documentation/config.txt | 1 +
dir.c | 9 ++++++++-
2 files changed, 9 insertions(+), 1 deletions(-)
diff --git a/Documentation/config.txt b/Documentation/config.txt
index 7ff2d1d..9fc527c 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -408,6 +408,7 @@ core.excludesfile::
of files which are not meant to be tracked. "{tilde}/" is expanded
to the value of `$HOME` and "{tilde}user/" to the specified user's
home directory. See linkgit:gitignore[5].
+ Default: ~/.gitexcludes.
core.editor::
Commands such as `commit` and `tag` that lets you edit
diff --git a/dir.c b/dir.c
index 3a8d3e6..a6a6291 100644
--- a/dir.c
+++ b/dir.c
@@ -944,9 +944,16 @@ void setup_standard_excludes(struct dir_struct *dir)
dir->exclude_per_dir = ".gitignore";
path = git_path("info/exclude");
+ if (!excludes_file) {
+ const char *home = getenv("HOME");
+ char *user_gitignore = malloc(strlen(home) + strlen("/.gitexcludes") + 1);
+ strcpy(user_gitignore, home);
+ strcat(user_gitignore, "/.gitexcludes");
+ excludes_file = user_gitignore;
+ }
if (!access(path, R_OK))
add_excludes_from_file(dir, path);
- if (excludes_file && !access(excludes_file, R_OK))
+ if (!access(excludes_file, R_OK))
add_excludes_from_file(dir, excludes_file);
}
--
1.6.5.3.435.g5f2e3.dirty
^ permalink raw reply related
* Re: [PATCH/RFC 02/11] strbuf: add non-variadic function strbuf_vaddf()
From: Erik Faye-Lund @ 2009-11-26 10:38 UTC (permalink / raw)
To: Junio C Hamano; +Cc: msysgit, git, dotzenlabs, Alex Riesen
In-Reply-To: <7vskc2ksnn.fsf@alter.siamese.dyndns.org>
On Thu, Nov 26, 2009 at 1:59 AM, Junio C Hamano <gitster@pobox.com> wrote:
> 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()?
>
I think your expectation is well justified, this seems to be a
portability-bug waiting to happen. Sorry for missing this prior to
sending out - on Windows this is known to work, and this function is
currently only used from the Windows implementation of syslog.
How kosher is it to use va_copy in the git-core, considering that it's
C99? A quick grep reveals only one occurrence of va_copy in the
source, and that's in compat/winansi.c. Searching the history of next
reveals that Alex Riesen (CC'd) already removed one occurrence
(4bf5383), so I'm starting to get slightly scared it might not be OK.
In practice it seems that something like the following works
portably-enough for many applications, dunno if it's something we'll
be happy with:
#ifndef va_copy
#define va_copy(a,b) ((a) = (b))
#endif
--
Erik "kusma" Faye-Lund
^ permalink raw reply
* [PATCH] Improve the mingw getaddrinfo stub to handle more use cases
From: Martin Storsjö @ 2009-11-26 10:43 UTC (permalink / raw)
To: Erik Faye-Lund; +Cc: msysgit, git, dotzenlabs, Erik Faye-Lund
In-Reply-To: <alpine.DEB.2.00.0911261015140.14228@cone.home.martin.st>
Allow the node parameter to be null, which is used for getting
the default bind address.
Also allow the hints parameter to be null, to improve standard
conformance of the stub implementation a little.
Signed-off-by: Martin Storsjo <martin@martin.st>
---
This patch adds support for the getaddrinfo parameters used by git-daemon,
as mentioned earlier.
compat/mingw.c | 20 +++++++++++++-------
1 files changed, 13 insertions(+), 7 deletions(-)
diff --git a/compat/mingw.c b/compat/mingw.c
index 0653560..17d1314 100644
--- a/compat/mingw.c
+++ b/compat/mingw.c
@@ -913,19 +913,22 @@ static int WSAAPI getaddrinfo_stub(const char *node, const char *service,
const struct addrinfo *hints,
struct addrinfo **res)
{
- struct hostent *h = gethostbyname(node);
+ struct hostent *h = NULL;
struct addrinfo *ai;
struct sockaddr_in *sin;
- if (!h)
- return WSAGetLastError();
+ if (node) {
+ h = gethostbyname(node);
+ if (!h)
+ return WSAGetLastError();
+ }
ai = xmalloc(sizeof(struct addrinfo));
*res = ai;
ai->ai_flags = 0;
ai->ai_family = AF_INET;
- ai->ai_socktype = hints->ai_socktype;
- switch (hints->ai_socktype) {
+ ai->ai_socktype = hints ? hints->ai_socktype : 0;
+ switch (ai->ai_socktype) {
case SOCK_STREAM:
ai->ai_protocol = IPPROTO_TCP;
break;
@@ -937,14 +940,17 @@ static int WSAAPI getaddrinfo_stub(const char *node, const char *service,
break;
}
ai->ai_addrlen = sizeof(struct sockaddr_in);
- ai->ai_canonname = strdup(h->h_name);
+ ai->ai_canonname = h ? strdup(h->h_name) : NULL;
sin = xmalloc(ai->ai_addrlen);
memset(sin, 0, ai->ai_addrlen);
sin->sin_family = AF_INET;
if (service)
sin->sin_port = htons(atoi(service));
- sin->sin_addr = *(struct in_addr *)h->h_addr;
+ if (h)
+ sin->sin_addr = *(struct in_addr *)h->h_addr;
+ else
+ sin->sin_addr.s_addr = INADDR_ANY;
ai->ai_addr = (struct sockaddr *)sin;
ai->ai_next = 0;
return 0;
--
1.6.4.4
^ permalink raw reply related
* Re: [PATCH/RFC 01/11] mingw: add network-wrappers for daemon
From: Erik Faye-Lund @ 2009-11-26 10:46 UTC (permalink / raw)
To: Martin Storsjö; +Cc: msysgit, git, dotzenlabs
In-Reply-To: <alpine.DEB.2.00.0911261015140.14228@cone.home.martin.st>
On Thu, Nov 26, 2009 at 9:24 AM, Martin Storsjö <martin@martin.st> wrote:
> Hi,
>
> First of all, great that you're working on adding daemon support for
> windows!
Thanks. I meant to send this out a couple of weeks ago, but I wasn't
able to find the time until now.
Also, I wouldn't have come this far without going tired of it without
Mike's patches, so some credit should go to him for doing good initial
work!
> On Thu, 26 Nov 2009, Erik Faye-Lund wrote:
>
>> +static void wsa_init(void)
>> {
>> + static int initialized = 0;
>> WSADATA wsa;
>>
>> + if (initialized)
>> + return;
>> +
>> if (WSAStartup(MAKEWORD(2,2), &wsa))
>> die("unable to initialize winsock subsystem, error %d",
>> WSAGetLastError());
>> atexit((void(*)(void)) WSACleanup);
>> + initialized = 1;
>> +}
>
> Something similar to this was merged into master recently as part of my
> mingw/ipv6 patches, so by rebasing your patch on top of that, this patch
> will probably get a bit smaller.
Yeah, I saw your patches, and realized that I needed to rebase my work
at some point, but none of the repos I usually pull from seems to
contain the patches yet. Rebasing will be a requirement before this
can be applied for sure.
>
> Also, the getaddrinfo-compatibility wrappers perhaps may need some minor
> updates to handle the use cases needed for setting up listening sockets.
I expect you're referring to IPv6 support in the wrappers this patch
adds? Unfortunately IPv6 isn't something I'm very familiar with, but
I'll give it a go unless someone else provides some patches...
--
Erik "kusma" Faye-Lund
^ permalink raw reply
* Re: [PATCH/RFC 01/11] mingw: add network-wrappers for daemon
From: Martin Storsjö @ 2009-11-26 11:03 UTC (permalink / raw)
To: kusmabite; +Cc: msysgit, git, dotzenlabs
In-Reply-To: <40aa078e0911260246j47fa36d5t421de7c1d07d5cca@mail.gmail.com>
On Thu, 26 Nov 2009, Erik Faye-Lund wrote:
> Yeah, I saw your patches, and realized that I needed to rebase my work
> at some point, but none of the repos I usually pull from seems to
> contain the patches yet. Rebasing will be a requirement before this
> can be applied for sure.
Ok, great! I tried applying it on the latest master, and it wasn't too
much of work.
> > Also, the getaddrinfo-compatibility wrappers perhaps may need some minor
> > updates to handle the use cases needed for setting up listening sockets.
>
> I expect you're referring to IPv6 support in the wrappers this patch
> adds? Unfortunately IPv6 isn't something I'm very familiar with, but
> I'll give it a go unless someone else provides some patches...
No, sorry for being unclear.
When IPv6 is enabled, name lookups go through getaddrinfo instead of
gethostbyname. Since getaddrinfo isn't available on Win2k (and switching
between getaddrinfo/gethostbyname happens at compile time when IPv6 is
enabled), we have to provide a small getaddrinfo stub, implemented in
terms of gethostbyname. This currently implements only parts of the
getaddrinfo interface - enough for the way getaddrinfo was used this far.
git-daemon uses getaddrinfo in a slightly different way (for setting up
listening sockets), and thus uses parameters that our current getaddrinfo
stub doesn't support. The patch I sent to this thread a moment ago adds
support for the way git-daemon uses getaddrinfo.
I tested this patch series on top of the latest master, with IPv6 support,
and found a slight problem caused by the IPv6 support. If IPv6 isn't
enabled, git-daemon always listens on one single socket, otherwise it may
listen on two separate sockets, one for v4 and one for v6.
This causes problems with the mingw poll() replacement, which has a
special case for polling one single fd - otherwise it tries to use some
emulation that currently only works for pipes. I didn't try to make any
proper fix for this though. I tested git-daemon by hacking it to listen on
only one of the sockets, and that worked well for both v4 and v6.
So, in addition to the getaddrinfo patch I sent, the mingw poll()
replacement needs some updates to handle polling multiple sockets. Except
from that, things seem to work, at a quick glance.
// Martin
^ permalink raw reply
* Re: [RFC/PATCHv8 08/10] fast-import: Proper notes tree manipulation using the notes API
From: Johan Herland @ 2009-11-26 11:10 UTC (permalink / raw)
To: Shawn O. Pearce; +Cc: git, gitster
In-Reply-To: <20091126024655.GR11919@spearce.org>
On Thursday 26 November 2009, Shawn O. Pearce wrote:
> 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.
Ok, I will attempt to redo the patch without reusing the notes code. I
have couple of ideas on how to get it done, but probably won't have the
time to implement them until next week...
> 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.
The main problem is that I need to grow a linked list at the far end,
while keeping a reference to the first element, so that
process_replace_list() traverse the list in the correct order (FIFO).
However, I agree that my approach to doing so may have been somewhat
ham-fisted... Will redo (unless the alternative approach above renders
this code obsolete).
BTW, while we're on the topic, this whole code is only present because I
assume it's not possible to edit the fast-import tree structure _while_
traversing it. Is this assumption correct, or are there ways to get
around maintaining a separate edit list that is applied to the tree
structure afterwards?
Thanks for the review! :)
...Johan
--
Johan Herland, <johan@herland.net>
www.herland.net
^ permalink raw reply
* Re: [PATCH/RFC 02/11] strbuf: add non-variadic function strbuf_vaddf()
From: Paolo Bonzini @ 2009-11-26 11:13 UTC (permalink / raw)
To: git; +Cc: msysgit
In-Reply-To: <40aa078e0911260238rd0c90cag126709d1de5f50de@mail.gmail.com>
On 11/26/2009 11:38 AM, Erik Faye-Lund wrote:
> In practice it seems that something like the following works
> portably-enough for many applications, dunno if it's something we'll
> be happy with:
> #ifndef va_copy
> #define va_copy(a,b) ((a) = (b))
> #endif
Yes, this is correct.
Paolo
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox