* [PATCH] githooks.txt: put hooks into subsections
From: Bert Wesarg @ 2009-09-06 10:22 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Christian Couder, git, Bert Wesarg
All hooks are currently in its own section. Which may confuse users,
because the section name serves as the hook file name and sections are
all caps for man pages. Putting them into a new HOOKS section and each
hook into a subsection keeps the case to lower case.
Signed-off-by: Bert Wesarg <bert.wesarg@googlemail.com>
---
Documentation/githooks.txt | 33 ++++++++++++++++++---------------
1 files changed, 18 insertions(+), 15 deletions(-)
diff --git a/Documentation/githooks.txt b/Documentation/githooks.txt
index 1c73673..acc408d 100644
--- a/Documentation/githooks.txt
+++ b/Documentation/githooks.txt
@@ -26,8 +26,11 @@ executable by default.
This document describes the currently defined hooks.
+HOOKS
+-----
+
applypatch-msg
---------------
+~~~~~~~~~~~~~~
This hook is invoked by 'git-am' script. It takes a single
parameter, the name of the file that holds the proposed commit
@@ -43,7 +46,7 @@ The default 'applypatch-msg' hook, when enabled, runs the
'commit-msg' hook, if the latter is enabled.
pre-applypatch
---------------
+~~~~~~~~~~~~~~
This hook is invoked by 'git-am'. It takes no parameter, and is
invoked after the patch is applied, but before a commit is made.
@@ -58,7 +61,7 @@ The default 'pre-applypatch' hook, when enabled, runs the
'pre-commit' hook, if the latter is enabled.
post-applypatch
----------------
+~~~~~~~~~~~~~~~
This hook is invoked by 'git-am'. It takes no parameter,
and is invoked after the patch is applied and a commit is made.
@@ -67,7 +70,7 @@ This hook is meant primarily for notification, and cannot affect
the outcome of 'git-am'.
pre-commit
-----------
+~~~~~~~~~~
This hook is invoked by 'git-commit', and can be bypassed
with `\--no-verify` option. It takes no parameter, and is
@@ -84,7 +87,7 @@ variable `GIT_EDITOR=:` if the command will not bring up an editor
to modify the commit message.
prepare-commit-msg
-------------------
+~~~~~~~~~~~~~~~~~~
This hook is invoked by 'git-commit' right after preparing the
default log message, and before the editor is started.
@@ -109,7 +112,7 @@ The sample `prepare-commit-msg` hook that comes with git comments
out the `Conflicts:` part of a merge's commit message.
commit-msg
-----------
+~~~~~~~~~~
This hook is invoked by 'git-commit', and can be bypassed
with `\--no-verify` option. It takes a single parameter, the
@@ -126,7 +129,7 @@ The default 'commit-msg' hook, when enabled, detects duplicate
"Signed-off-by" lines, and aborts the commit if one is found.
post-commit
------------
+~~~~~~~~~~~
This hook is invoked by 'git-commit'. It takes no
parameter, and is invoked after a commit is made.
@@ -135,14 +138,14 @@ This hook is meant primarily for notification, and cannot affect
the outcome of 'git-commit'.
pre-rebase
-----------
+~~~~~~~~~~
This hook is called by 'git-rebase' and can be used to prevent a branch
from getting rebased.
post-checkout
------------
+~~~~~~~~~~~~~
This hook is invoked when a 'git-checkout' is run after having updated the
worktree. The hook is given three parameters: the ref of the previous HEAD,
@@ -160,7 +163,7 @@ differences from the previous HEAD if different, or set working dir metadata
properties.
post-merge
------------
+~~~~~~~~~~
This hook is invoked by 'git-merge', which happens when a 'git-pull'
is done on a local repository. The hook takes a single parameter, a status
@@ -175,7 +178,7 @@ for an example of how to do this.
[[pre-receive]]
pre-receive
------------
+~~~~~~~~~~~
This hook is invoked by 'git-receive-pack' on the remote repository,
which happens when a 'git-push' is done on a local repository.
@@ -204,7 +207,7 @@ for the user.
[[update]]
update
-------
+~~~~~~
This hook is invoked by 'git-receive-pack' on the remote repository,
which happens when a 'git-push' is done on a local repository.
@@ -247,7 +250,7 @@ unannotated tags to be pushed.
[[post-receive]]
post-receive
-------------
+~~~~~~~~~~~~
This hook is invoked by 'git-receive-pack' on the remote repository,
which happens when a 'git-push' is done on a local repository.
@@ -277,7 +280,7 @@ emails.
[[post-update]]
post-update
------------
+~~~~~~~~~~~
This hook is invoked by 'git-receive-pack' on the remote repository,
which happens when a 'git-push' is done on a local repository.
@@ -308,7 +311,7 @@ Both standard output and standard error output are forwarded to
for the user.
pre-auto-gc
------------
+~~~~~~~~~~~
This hook is invoked by 'git-gc --auto'. It takes no parameter, and
exiting with non-zero status from this script causes the 'git-gc --auto'
--
1.6.4.GIT
^ permalink raw reply related
* Re: [PATCH 3/4] push: make non-fast-forward help message configurable
From: Sverre Rabbelier @ 2009-09-06 11:30 UTC (permalink / raw)
To: Junio C Hamano
Cc: Jeff King, Nanako Shiraishi, Matthieu Moy, Teemu Likonen, git
In-Reply-To: <7vzl98fr22.fsf@alter.siamese.dyndns.org>
Heya,
On Sun, Sep 6, 2009 at 09:52, Junio C Hamano<gitster@pobox.com> wrote:
> Resetting is done because I want to build an alternate history starting
> from an earlier point, and when I am done, I may want to feed this to "git
> diff" or whatever to sanity check the result, without having to go through
> the reflog.
I agree, the 'you are now at ...' is not very helpful, I just TOLD you
where I want to go to! However, 'you were at ....' is also not
optimal, since what I really want to know is where I was at "plus any
old data I had" (going back to the 'git reset --hard' should create a
snapshot of the current content before doing it's resetting'). I would
definitely prefer 'you were at' over 'you are now at' though.
WRT the 'none of your business', it sounds like 'I [already] know
[that]' might be approriate?
--
Cheers,
Sverre Rabbelier
^ permalink raw reply
* Re: [RFC/PATCH 0/4] make helpful messages optional
From: Matthieu Moy @ 2009-09-06 11:53 UTC (permalink / raw)
To: Jeff King; +Cc: Nanako Shiraishi, Junio C Hamano, Teemu Likonen, git
In-Reply-To: <20090906064454.GA1643@coredump.intra.peff.net>
Jeff King <peff@peff.net> writes:
> [1/4]: push: fix english in non-fast-forward message
> [2/4]: push: re-flow non-fast-forward message
Sounds OK to me.
> [3/4]: push: make non-fast-forward help message configurable
> [4/4]: status: make "how to stage" messages optional
I didn't review the code in details, but I like the direction where
it's going. I'm not sure what's the best name for the configuration
option, but probably your proposal of "message" is good because it can
encompass many different cases (very detailed error messages, advices,
informative messages, ...).
And BTW, as the initial author of the push help message, I fully agree
that it is unnecessarily eating 3 lines of my terminal ;-). The
message is clearly targeted to newbies (but I do think it's very
helpfull to them).
--
Matthieu Moy
http://www-verimag.imag.fr/~moy/
^ permalink raw reply
* Re: [BUG] 'add -u' doesn't work from untracked subdir
From: Matthieu Moy @ 2009-09-06 12:32 UTC (permalink / raw)
To: Clemens Buchacher; +Cc: Junio C Hamano, Jeff King, SZEDER Gábor, git
In-Reply-To: <20090905084641.GA24865@darc.dnsalias.org>
Clemens Buchacher <drizzd@aon.at> writes:
> On Sat, Sep 05, 2009 at 12:02:35AM -0700, Junio C Hamano wrote:
>
>> I personally find "add -u" that defaults to the current directory more
>> natural than always going to the root; same preference for "grep".
>> Besides, "add -u subdir" must add subdir relative to the cwd, without
>> going to the root. Why should "add -u" sans argument behave drastically
>> differently?
>
> Sorry for stating the obvious here, but the following commands affect the
> entire repository, even though they limit themselves to the current
> directory, if passed a '.'.
>
> git commit
> git log
> git diff
> git checkout
> git reset
You have to add "git add -e", "git add -i" and "git add -p" here.
I completely agree that "git add -u" should have been a full-tree
oriented command, just like other "git add" variants and other Git
commands, from the beginning.
Now, I'm unconfortable with both a behavior change and a config
option. Someone used to the cwd-limited behavior typing "git add -u"
on a machine configured to git the full-tree behavior could be really
annoyed (not even mentionning scripts).
I think it has already been proposed to introduce "git add -a" doing
what "git add -u" do, but for the full tree. The "-a" option here
being analogous to the one of "git commit": roughly, "git add -a; git
commit" would be equivalent to "git commit -a". This would allow a
long deprecation period for "git add -u". I find the proposal
sensible, but IIRC it has already been rejected.
--
Matthieu Moy
http://www-verimag.imag.fr/~moy/
^ permalink raw reply
* Re: Use case I don't know how to address
From: Alan Chandler @ 2009-09-06 12:50 UTC (permalink / raw)
Cc: git
In-Reply-To: <200909051923.28831.j6t@kdbg.org>
Johannes Sixt wrote:
> On Samstag, 5. September 2009, Alan Chandler wrote:
>> The problem comes when I want to now merge back further work that I did
>> on the master branch (the 5-6 transition) to the fan club site
>>
>>
>> 2' - 2a - 3' - 4' ----------------- 6' SITE
>> / / / /
>> 1 - 2 ------ 3 - 4 ------------6'''- 6a TEST
>> \ /
>> 5 ------ 6 MASTER
>> \ \
>> 5''- 5a- 6'' DEMO
>>
>>
>> What will happen is the changes made in 4->5 will get applied to the
>> (now) Test branch as part of the 6->6'' merge, and I will be left having
>> to add a new commit, 6a, to undo them all again. Given this is likely
>> to be quite a substantial change I want to try and avoid it if possible.
>>
>> Is there any way I could use git to remember the 4->5 transition,
>> reverse it and apply it back to the Test branch before hand.
>
> Basically, your mistake was to rename master to test and continue development
> on the demo branch. So what you should do instead is this:
>
> 2' - 2a - 3' - 4' ------ 6' SITE
> / / / /
> 1 - 2 ------ 3 - 4 ------- 6 MASTER
> \ \
> 5 - 5a - 6'' DEMO
>
I understand what you are saying. I think I need to consider a slight
change to it however. Master is currently the branch that I push to my
public repository and that should ideally have my NON FAN CLUB skinning
So actually I think I am like this
2' - 2a - 3' - 4' ------ 6' SITE
/ / / /
1 - 2 ------ 3 - 4 ------- 6 MASTER
\ \
5 - -- - 6'' PUBLIC REPO
\ \
5'''-5a -6''' DEMO
--
Alan Chandler
http://www.chandlerfamily.org.uk
^ permalink raw reply
* [PATCH v7 0/6] fast-import: add new feature and option command
From: Sverre Rabbelier @ 2009-09-06 14:35 UTC (permalink / raw)
To: Junio C Hamano, Shawn O. Pearce, Johannes Schindelin, Git List,
Ian Clatworthy <ian.cla
Only changes are in 5/6 and 6/6, as suggested by Junio we now ignore
git options that we do not recognise. If desired we could add a
feature command for each option so that the frontend can make it
required with 'feature git-option-marks'.
Sverre Rabbelier (6):
fast-import: put option parsing code in separate functions
fast-import: put marks reading in it's own function
fast-import: add feature command
fast-import: test the new feature command
fast-import: add option command
fast-import: test the new option command
Documentation/git-fast-import.txt | 40 ++++++
fast-import.c | 264 ++++++++++++++++++++++++++-----------
t/t9300-fast-import.sh | 107 +++++++++++++++
3 files changed, 334 insertions(+), 77 deletions(-)
^ permalink raw reply
* [PATCH v7 2/6] fast-import: put marks reading in it's own function
From: Sverre Rabbelier @ 2009-09-06 14:35 UTC (permalink / raw)
To: Junio C Hamano, Shawn O. Pearce, Johannes Schindelin, Git List,
Ian Clatworthy <ian.cla
Cc: Sverre Rabbelier
In-Reply-To: <1252247748-14507-2-git-send-email-srabbelier@gmail.com>
All options do nothing but set settings, with the exception of the
--input-marks option. Delay the reading of the marks file till after
all options have been parsed.
Signed-off-by: Sverre Rabbelier <srabbelier@gmail.com>
---
Unchanged again.
fast-import.c | 73 ++++++++++++++++++++++++++++++++-------------------------
1 files changed, 41 insertions(+), 32 deletions(-)
diff --git a/fast-import.c b/fast-import.c
index b904f20..812fcf0 100644
--- a/fast-import.c
+++ b/fast-import.c
@@ -315,6 +315,7 @@ static struct object_entry_pool *blocks;
static struct object_entry *object_table[1 << 16];
static struct mark_set *marks;
static const char *mark_file;
+static const char *input_file;
/* Our last blob */
static struct last_object last_blob = { STRBUF_INIT, 0, 0, 0 };
@@ -1643,6 +1644,42 @@ static void dump_marks(void)
}
}
+static void read_marks(void)
+{
+ char line[512];
+ FILE *f = fopen(input_file, "r");
+ if (!f)
+ die_errno("cannot read '%s'", input_file);
+ while (fgets(line, sizeof(line), f)) {
+ uintmax_t mark;
+ char *end;
+ unsigned char sha1[20];
+ struct object_entry *e;
+
+ end = strchr(line, '\n');
+ if (line[0] != ':' || !end)
+ die("corrupt mark line: %s", line);
+ *end = 0;
+ mark = strtoumax(line + 1, &end, 10);
+ if (!mark || end == line + 1
+ || *end != ' ' || get_sha1(end + 1, sha1))
+ die("corrupt mark line: %s", line);
+ e = find_object(sha1);
+ if (!e) {
+ enum object_type type = sha1_object_info(sha1, NULL);
+ if (type < 0)
+ die("object not found: %s", sha1_to_hex(sha1));
+ e = insert_object(sha1);
+ e->type = type;
+ e->pack_id = MAX_PACK_ID;
+ e->offset = 1; /* just not zero! */
+ }
+ insert_mark(mark, e);
+ }
+ fclose(f);
+}
+
+
static int read_next_command(void)
{
static int stdin_eof = 0;
@@ -2338,39 +2375,9 @@ static void parse_progress(void)
skip_optional_lf();
}
-static void option_import_marks(const char *input_file)
+static void option_import_marks(const char *marks)
{
- char line[512];
- FILE *f = fopen(input_file, "r");
- if (!f)
- die_errno("cannot read '%s'", input_file);
- while (fgets(line, sizeof(line), f)) {
- uintmax_t mark;
- char *end;
- unsigned char sha1[20];
- struct object_entry *e;
-
- end = strchr(line, '\n');
- if (line[0] != ':' || !end)
- die("corrupt mark line: %s", line);
- *end = 0;
- mark = strtoumax(line + 1, &end, 10);
- if (!mark || end == line + 1
- || *end != ' ' || get_sha1(end + 1, sha1))
- die("corrupt mark line: %s", line);
- e = find_object(sha1);
- if (!e) {
- enum object_type type = sha1_object_info(sha1, NULL);
- if (type < 0)
- die("object not found: %s", sha1_to_hex(sha1));
- e = insert_object(sha1);
- e->type = type;
- e->pack_id = MAX_PACK_ID;
- e->offset = 1; /* just not zero! */
- }
- insert_mark(mark, e);
- }
- fclose(f);
+ input_file = xstrdup(marks);
}
static void option_date_format(const char *fmt)
@@ -2495,6 +2502,8 @@ int main(int argc, const char **argv)
}
if (i != argc)
usage(fast_import_usage);
+ if (input_file)
+ read_marks();
rc_free = pool_alloc(cmd_save * sizeof(*rc_free));
for (i = 0; i < (cmd_save - 1); i++)
--
1.6.4.16.g72c66.dirty
^ permalink raw reply related
* [PATCH v7 1/6] fast-import: put option parsing code in separate functions
From: Sverre Rabbelier @ 2009-09-06 14:35 UTC (permalink / raw)
To: Junio C Hamano, Shawn O. Pearce, Johannes Schindelin, Git List,
Ian Clatworthy <ian.cla
Cc: Sverre Rabbelier
In-Reply-To: <1252247748-14507-1-git-send-email-srabbelier@gmail.com>
Putting the options in their own functions increases readability of
the option parsing block and makes it easier to reuse the option
parsing code later on.
Signed-off-by: Sverre Rabbelier <srabbelier@gmail.com>
---
Unchanged again.
fast-import.c | 115 +++++++++++++++++++++++++++++++++++++--------------------
1 files changed, 75 insertions(+), 40 deletions(-)
diff --git a/fast-import.c b/fast-import.c
index 7ef9865..b904f20 100644
--- a/fast-import.c
+++ b/fast-import.c
@@ -291,6 +291,7 @@ static unsigned long branch_count;
static unsigned long branch_load_count;
static int failure;
static FILE *pack_edges;
+static unsigned int show_stats = 1;
/* Memory pools */
static size_t mem_pool_alloc = 2*1024*1024 - sizeof(struct mem_pool);
@@ -2337,7 +2338,7 @@ static void parse_progress(void)
skip_optional_lf();
}
-static void import_marks(const char *input_file)
+static void option_import_marks(const char *input_file)
{
char line[512];
FILE *f = fopen(input_file, "r");
@@ -2372,6 +2373,76 @@ static void import_marks(const char *input_file)
fclose(f);
}
+static void option_date_format(const char *fmt)
+{
+ if (!strcmp(fmt, "raw"))
+ whenspec = WHENSPEC_RAW;
+ else if (!strcmp(fmt, "rfc2822"))
+ whenspec = WHENSPEC_RFC2822;
+ else if (!strcmp(fmt, "now"))
+ whenspec = WHENSPEC_NOW;
+ else
+ die("unknown --date-format argument %s", fmt);
+}
+
+static void option_max_pack_size(const char *packsize)
+{
+ max_packsize = strtoumax(packsize, NULL, 0) * 1024 * 1024;
+}
+
+static void option_depth(const char *depth)
+{
+ max_depth = strtoul(depth, NULL, 0);
+ if (max_depth > MAX_DEPTH)
+ die("--depth cannot exceed %u", MAX_DEPTH);
+}
+
+static void option_active_branches(const char *branches)
+{
+ max_active_branches = strtoul(branches, NULL, 0);
+}
+
+static void option_export_marks(const char *marks)
+{
+ mark_file = xstrdup(marks);
+}
+
+static void option_export_pack_edges(const char *edges)
+{
+ if (pack_edges)
+ fclose(pack_edges);
+ pack_edges = fopen(edges, "a");
+ if (!pack_edges)
+ die_errno("Cannot open '%s'", edges);
+}
+
+static void parse_one_option(const char *option)
+{
+ if (!prefixcmp(option, "date-format=")) {
+ option_date_format(option + 12);
+ } else if (!prefixcmp(option, "max-pack-size=")) {
+ option_max_pack_size(option + 14);
+ } else if (!prefixcmp(option, "depth=")) {
+ option_depth(option + 6);
+ } else if (!prefixcmp(option, "active-branches=")) {
+ option_active_branches(option + 16);
+ } else if (!prefixcmp(option, "import-marks=")) {
+ option_import_marks(option + 13);
+ } else if (!prefixcmp(option, "export-marks=")) {
+ option_export_marks(option + 13);
+ } else if (!prefixcmp(option, "export-pack-edges=")) {
+ option_export_pack_edges(option + 18);
+ } else if (!prefixcmp(option, "force")) {
+ force_update = 1;
+ } else if (!prefixcmp(option, "quiet")) {
+ show_stats = 0;
+ } else if (!prefixcmp(option, "stats")) {
+ show_stats = 1;
+ } else {
+ die("Unsupported option: %s", option);
+ }
+}
+
static int git_pack_config(const char *k, const char *v, void *cb)
{
if (!strcmp(k, "pack.depth")) {
@@ -2398,7 +2469,7 @@ static const char fast_import_usage[] =
int main(int argc, const char **argv)
{
- unsigned int i, show_stats = 1;
+ unsigned int i;
git_extract_argv0_path(argv[0]);
@@ -2419,44 +2490,8 @@ int main(int argc, const char **argv)
if (*a != '-' || !strcmp(a, "--"))
break;
- else if (!prefixcmp(a, "--date-format=")) {
- const char *fmt = a + 14;
- if (!strcmp(fmt, "raw"))
- whenspec = WHENSPEC_RAW;
- else if (!strcmp(fmt, "rfc2822"))
- whenspec = WHENSPEC_RFC2822;
- else if (!strcmp(fmt, "now"))
- whenspec = WHENSPEC_NOW;
- else
- die("unknown --date-format argument %s", fmt);
- }
- else if (!prefixcmp(a, "--max-pack-size="))
- max_packsize = strtoumax(a + 16, NULL, 0) * 1024 * 1024;
- else if (!prefixcmp(a, "--depth=")) {
- max_depth = strtoul(a + 8, NULL, 0);
- if (max_depth > MAX_DEPTH)
- die("--depth cannot exceed %u", MAX_DEPTH);
- }
- else if (!prefixcmp(a, "--active-branches="))
- max_active_branches = strtoul(a + 18, NULL, 0);
- else if (!prefixcmp(a, "--import-marks="))
- import_marks(a + 15);
- else if (!prefixcmp(a, "--export-marks="))
- mark_file = a + 15;
- else if (!prefixcmp(a, "--export-pack-edges=")) {
- if (pack_edges)
- fclose(pack_edges);
- pack_edges = fopen(a + 20, "a");
- if (!pack_edges)
- die_errno("Cannot open '%s'", a + 20);
- } else if (!strcmp(a, "--force"))
- force_update = 1;
- else if (!strcmp(a, "--quiet"))
- show_stats = 0;
- else if (!strcmp(a, "--stats"))
- show_stats = 1;
- else
- die("unknown option %s", a);
+
+ parse_one_option(a + 2);
}
if (i != argc)
usage(fast_import_usage);
--
1.6.4.16.g72c66.dirty
^ permalink raw reply related
* [PATCH v7 4/6] fast-import: test the new feature command
From: Sverre Rabbelier @ 2009-09-06 14:35 UTC (permalink / raw)
To: Junio C Hamano, Shawn O. Pearce, Johannes Schindelin, Git List,
Ian Clatworthy <ian.cla
Cc: Sverre Rabbelier
In-Reply-To: <1252247748-14507-4-git-send-email-srabbelier@gmail.com>
Test that an unknown feature causes fast-import to abort, and that a
known feature is accepted.
Signed-off-by: Sverre Rabbelier <srabbelier@gmail.com>
---
Unchanged since v6.
t/t9300-fast-import.sh | 20 ++++++++++++++++++++
1 files changed, 20 insertions(+), 0 deletions(-)
diff --git a/t/t9300-fast-import.sh b/t/t9300-fast-import.sh
index 821be7c..564ed6b 100755
--- a/t/t9300-fast-import.sh
+++ b/t/t9300-fast-import.sh
@@ -1088,4 +1088,24 @@ INPUT_END
test_expect_success 'P: fail on blob mark in gitlink' '
test_must_fail git fast-import <input'
+###
+### series R (feature)
+###
+
+cat >input <<EOF
+feature no-such-feature-exists
+EOF
+
+test_expect_success 'R: abort on unsupported feature' '
+ test_must_fail git fast-import <input
+'
+
+cat >input <<EOF
+feature date-format=now
+EOF
+
+test_expect_success 'R: supported feature is accepted' '
+ git fast-import <input
+'
+
test_done
--
1.6.4.16.g72c66.dirty
^ permalink raw reply related
* [PATCH v7 3/6] fast-import: add feature command
From: Sverre Rabbelier @ 2009-09-06 14:35 UTC (permalink / raw)
To: Junio C Hamano, Shawn O. Pearce, Johannes Schindelin, Git List,
Ian Clatworthy <ian.cla
Cc: Sverre Rabbelier
In-Reply-To: <1252247748-14507-3-git-send-email-srabbelier@gmail.com>
This allows the fronted to require a specific feature to be supported
by the frontend, or abort.
Also add support for the first feature, date-format=.
Signed-off-by: Sverre Rabbelier <srabbelier@gmail.com>
---
Unchanged since v6.
Documentation/git-fast-import.txt | 16 ++++++++++++++++
fast-import.c | 13 +++++++++++++
2 files changed, 29 insertions(+), 0 deletions(-)
diff --git a/Documentation/git-fast-import.txt b/Documentation/git-fast-import.txt
index c2f483a..1e293f2 100644
--- a/Documentation/git-fast-import.txt
+++ b/Documentation/git-fast-import.txt
@@ -303,6 +303,10 @@ and control the current import process. More detailed discussion
standard output. This command is optional and is not needed
to perform an import.
+`feature`::
+ Require that fast-import supports the specified feature, or
+ abort if it does not.
+
`commit`
~~~~~~~~
Create or update a branch with a new commit, recording one logical
@@ -813,6 +817,18 @@ Placing a `progress` command immediately after a `checkpoint` will
inform the reader when the `checkpoint` has been completed and it
can safely access the refs that fast-import updated.
+`feature`
+~~~~~~~~~
+Require that fast-import supports the specified feature, or abort if
+it does not.
+
+....
+ 'feature' SP <feature> LF
+....
+
+The <feature> part of the command may be any string matching
+[a-zA-Z-] and should be understood by a version of fast-import.
+
Crash Reports
-------------
If fast-import is supplied invalid input it will terminate with a
diff --git a/fast-import.c b/fast-import.c
index 812fcf0..9bf06a4 100644
--- a/fast-import.c
+++ b/fast-import.c
@@ -2450,6 +2450,17 @@ static void parse_one_option(const char *option)
}
}
+static void parse_feature(void)
+{
+ char *feature = command_buf.buf + 8;
+
+ if (!prefixcmp(feature, "date-format=")) {
+ option_date_format(feature + 12);
+ } else {
+ die("This version of fast-import does not support feature %s.", feature);
+ }
+}
+
static int git_pack_config(const char *k, const char *v, void *cb)
{
if (!strcmp(k, "pack.depth")) {
@@ -2526,6 +2537,8 @@ int main(int argc, const char **argv)
parse_checkpoint();
else if (!prefixcmp(command_buf.buf, "progress "))
parse_progress();
+ else if (!prefixcmp(command_buf.buf, "feature "))
+ parse_feature();
else
die("Unsupported command: %s", command_buf.buf);
}
--
1.6.4.16.g72c66.dirty
^ permalink raw reply related
* [PATCH v7 5/6] fast-import: add option command
From: Sverre Rabbelier @ 2009-09-06 14:35 UTC (permalink / raw)
To: Junio C Hamano, Shawn O. Pearce, Johannes Schindelin, Git List,
Ian Clatworthy <ian.cla
Cc: Sverre Rabbelier
In-Reply-To: <1252247748-14507-5-git-send-email-srabbelier@gmail.com>
This allows the frontend to specify any of the supported options as
long as no non-option command has been given. This way the
user does not have to include any frontend-specific options, but
instead she can rely on the frontend to tell fast-import what it
needs.
Also factor out parsing of argv and have it execute when we reach the
first non-option command, or after all commands have been read and
no non-option command has been encountered.
Non-git options and unrecognised git options are ignored, although
unrecognised options on the commandline still result in an error.
Signed-off-by: Sverre Rabbelier <srabbelier@gmail.com>
---
Fixed a few style errors pointed out by Junio and ignore
unrecognised git options.
Documentation/git-fast-import.txt | 24 +++++++++++
fast-import.c | 81 ++++++++++++++++++++++++++++++------
2 files changed, 91 insertions(+), 14 deletions(-)
diff --git a/Documentation/git-fast-import.txt b/Documentation/git-fast-import.txt
index 1e293f2..f1c94b4 100644
--- a/Documentation/git-fast-import.txt
+++ b/Documentation/git-fast-import.txt
@@ -307,6 +307,11 @@ and control the current import process. More detailed discussion
Require that fast-import supports the specified feature, or
abort if it does not.
+`option`::
+ Specify any of the options listed under OPTIONS to change
+ fast-import's behavior to suit the frontend's needs. This command
+ is optional and is not needed to perform an import.
+
`commit`
~~~~~~~~
Create or update a branch with a new commit, recording one logical
@@ -829,6 +834,25 @@ it does not.
The <feature> part of the command may be any string matching
[a-zA-Z-] and should be understood by a version of fast-import.
+`option`
+~~~~~~~~
+Processes the specified option so that git fast-import behaves in a
+way that suits the frontend's needs.
+Note that options specified by the frontend are overridden by any
+options the user may specify to git fast-import itself.
+
+....
+ 'option' SP <option> LF
+....
+
+The `<option>` part of the command may contain any of the options
+listed in the OPTIONS section, without the leading '--' and is
+treated in the same way.
+
+Option commands must be the first commands on the input (not counting
+feature commands), to give an option command after any non-option
+command is an error.
+
Crash Reports
-------------
If fast-import is supplied invalid input it will terminate with a
diff --git a/fast-import.c b/fast-import.c
index 9bf06a4..dcfb8fa 100644
--- a/fast-import.c
+++ b/fast-import.c
@@ -292,6 +292,8 @@ static unsigned long branch_load_count;
static int failure;
static FILE *pack_edges;
static unsigned int show_stats = 1;
+static int global_argc;
+static const char **global_argv;
/* Memory pools */
static size_t mem_pool_alloc = 2*1024*1024 - sizeof(struct mem_pool);
@@ -349,6 +351,10 @@ static struct recent_command *rc_free;
static unsigned int cmd_save = 100;
static uintmax_t next_mark;
static struct strbuf new_data = STRBUF_INIT;
+static int options_enabled;
+static int seen_non_option_command;
+
+static void parse_argv(void);
static void write_branch_report(FILE *rpt, struct branch *b)
{
@@ -1700,6 +1706,12 @@ static int read_next_command(void)
if (stdin_eof)
return EOF;
+ if (!seen_non_option_command
+ && prefixcmp(command_buf.buf, "feature ")
+ && prefixcmp(command_buf.buf, "option ")) {
+ parse_argv();
+ }
+
rc = rc_free;
if (rc)
rc_free = rc->next;
@@ -2423,7 +2435,7 @@ static void option_export_pack_edges(const char *edges)
die_errno("Cannot open '%s'", edges);
}
-static void parse_one_option(const char *option)
+static void parse_one_option(const char *option, int optional)
{
if (!prefixcmp(option, "date-format=")) {
option_date_format(option + 12);
@@ -2445,7 +2457,7 @@ static void parse_one_option(const char *option)
show_stats = 0;
} else if (!prefixcmp(option, "stats")) {
show_stats = 1;
- } else {
+ } else if (!optional) {
die("Unsupported option: %s", option);
}
}
@@ -2456,11 +2468,32 @@ static void parse_feature(void)
if (!prefixcmp(feature, "date-format=")) {
option_date_format(feature + 12);
+ } else if (!strcmp("git-options", feature)) {
+ options_enabled = 1;
} else {
die("This version of fast-import does not support feature %s.", feature);
}
}
+static void parse_option(void)
+{
+ char *option = command_buf.buf + 11;
+
+ if (!options_enabled)
+ die("Got option command '%s' before options feature'", option);
+
+ if (seen_non_option_command)
+ die("Got option command '%s' after non-option command", option);
+
+ /* ignore unknown options, instead of erroring */
+ parse_one_option(option, 1);
+}
+
+static void parse_nongit_option(void)
+{
+ /* do nothing */
+}
+
static int git_pack_config(const char *k, const char *v, void *cb)
{
if (!strcmp(k, "pack.depth")) {
@@ -2485,6 +2518,27 @@ static int git_pack_config(const char *k, const char *v, void *cb)
static const char fast_import_usage[] =
"git fast-import [--date-format=f] [--max-pack-size=n] [--depth=n] [--active-branches=n] [--export-marks=marks.file]";
+static void parse_argv(void)
+{
+ unsigned int i;
+
+ for (i = 1; i < global_argc; i++) {
+ const char *a = global_argv[i];
+
+ if (*a != '-' || !strcmp(a, "--"))
+ break;
+
+ /* error on unknown options */
+ parse_one_option(a + 2, 0);
+ }
+ if (i != global_argc)
+ usage(fast_import_usage);
+
+ seen_non_option_command = 1;
+ if (input_file)
+ read_marks();
+}
+
int main(int argc, const char **argv)
{
unsigned int i;
@@ -2503,18 +2557,8 @@ int main(int argc, const char **argv)
avail_tree_table = xcalloc(avail_tree_table_sz, sizeof(struct avail_tree_content*));
marks = pool_calloc(1, sizeof(struct mark_set));
- for (i = 1; i < argc; i++) {
- const char *a = argv[i];
-
- if (*a != '-' || !strcmp(a, "--"))
- break;
-
- parse_one_option(a + 2);
- }
- if (i != argc)
- usage(fast_import_usage);
- if (input_file)
- read_marks();
+ global_argc = argc;
+ global_argv = argv;
rc_free = pool_alloc(cmd_save * sizeof(*rc_free));
for (i = 0; i < (cmd_save - 1); i++)
@@ -2539,9 +2583,18 @@ int main(int argc, const char **argv)
parse_progress();
else if (!prefixcmp(command_buf.buf, "feature "))
parse_feature();
+ else if (!prefixcmp(command_buf.buf, "option git "))
+ parse_option();
+ else if (!prefixcmp(command_buf.buf, "option "))
+ parse_nongit_option();
else
die("Unsupported command: %s", command_buf.buf);
}
+
+ /* argv hasn't been parsed yet, do so */
+ if (!seen_non_option_command)
+ parse_argv();
+
end_packfile();
dump_branches();
--
1.6.4.16.g72c66.dirty
^ permalink raw reply related
* [PATCH v7 6/6] fast-import: test the new option command
From: Sverre Rabbelier @ 2009-09-06 14:35 UTC (permalink / raw)
To: Junio C Hamano, Shawn O. Pearce, Johannes Schindelin, Git List,
Ian Clatworthy <ian.cla
Cc: Sverre Rabbelier
In-Reply-To: <1252247748-14507-6-git-send-email-srabbelier@gmail.com>
Test three options (quiet and import/export-marks) and verify that the
commandline options override these.
Also make sure that a option command without a preceeding feature
git-options command is rejected and that non-git options are ignored.
Lastly, make sure that git options that we do not recognise are
ignored as well, but that they are rejected when parsed on the
commandline.
Signed-off-by: Sverre Rabbelier <srabbelier@gmail.com>
---
Updated tests for the new behavior.
t/t9300-fast-import.sh | 89 +++++++++++++++++++++++++++++++++++++++++++++++-
1 files changed, 88 insertions(+), 1 deletions(-)
diff --git a/t/t9300-fast-import.sh b/t/t9300-fast-import.sh
index 564ed6b..d33fc55 100755
--- a/t/t9300-fast-import.sh
+++ b/t/t9300-fast-import.sh
@@ -1089,7 +1089,7 @@ test_expect_success 'P: fail on blob mark in gitlink' '
test_must_fail git fast-import <input'
###
-### series R (feature)
+### series R (feature and option)
###
cat >input <<EOF
@@ -1108,4 +1108,91 @@ test_expect_success 'R: supported feature is accepted' '
git fast-import <input
'
+cat >input << EOF
+feature git-options
+option git quiet
+blob
+data 3
+hi
+
+EOF
+
+touch empty
+
+test_expect_success 'R: quiet option results in no stats being output' '
+ cat input | git fast-import 2> output &&
+ test_cmp empty output
+'
+
+cat >input << EOF
+feature git-options
+option git export-marks=git.marks
+blob
+mark :1
+data 3
+hi
+
+EOF
+
+test_expect_success \
+ 'R: export-marks option results in a marks file being created' \
+ 'cat input | git fast-import &&
+ grep :1 git.marks'
+
+test_expect_success \
+ 'R: export-marks options can be overriden by commandline options' \
+ 'cat input | git fast-import --export-marks=other.marks &&
+ grep :1 other.marks'
+
+cat >input << EOF
+feature git-options
+option git import-marks=marks.out
+option git export-marks=marks.new
+EOF
+
+test_expect_success \
+ 'R: import to output marks works without any content' \
+ 'cat input | git fast-import &&
+ test_cmp marks.out marks.new'
+
+cat >input <<EOF
+feature git-options
+option git import-marks=nonexistant.marks
+option git export-marks=marks.new
+EOF
+
+test_expect_success \
+ 'R: import marks prefers commandline marks file over the stream' \
+ 'cat input | git fast-import --import-marks=marks.out &&
+ test_cmp marks.out marks.new'
+
+cat >input <<EOF
+feature git-options
+option git non-existing-option
+EOF
+
+test_expect_success \
+ 'R: feature option is accepted and ignores unknown options' \
+ 'git fast-import <input'
+
+cat >input <<EOF
+option git quiet
+EOF
+
+test_expect_success 'R: unknown commandline options are rejected' '\
+ test_must_fail git fast-import --non-existing-option < /dev/null
+'
+
+test_expect_success \
+ 'R: option without preceeding feature command is rejected' \
+ 'test_must_fail git fast-import <input'
+
+cat >input <<EOF
+option non-existing-vcs non-existing-option
+EOF
+
+test_expect_success 'R: ignore non-git options' '
+ git fast-import <input
+'
+
test_done
--
1.6.4.16.g72c66.dirty
^ permalink raw reply related
* Re: [BUG] 'add -u' doesn't work from untracked subdir
From: Clemens Buchacher @ 2009-09-06 18:16 UTC (permalink / raw)
To: Matthieu Moy; +Cc: Junio C Hamano, Jeff King, SZEDER Gábor, git
In-Reply-To: <vpqfxb0s177.fsf@bauges.imag.fr>
On Sun, Sep 06, 2009 at 02:32:44PM +0200, Matthieu Moy wrote:
> I think it has already been proposed to introduce "git add -a" doing
> what "git add -u" do, but for the full tree.
I like that, actually. AFAICT it's completely analogous to "git commit -a".
We also need something for "git add -A" though.
Do you feel the same way about changing the behavior of "git grep"? I don't
really want to change the command's name.
Clemens
^ permalink raw reply
* Re: Strange merge failure (would be overwritten by merge / cannot merge)
From: Linus Torvalds @ 2009-09-06 18:18 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Git Mailing List
In-Reply-To: <7vvdjwbhzv.fsf@alter.siamese.dyndns.org>
Sorry, I was off for the week with very limited computer time (my main
computer was a Suunto Gekko that I wore underwater), and spent yesterday
doing the last kernel -rc.
Anyway, this is a problem I knew about, and it comes directly from the
insane historical behavior of unpack_trees(), where it would mix up blobs
and trees. The problem was _very_ obvious when I rewrote the tree-walking
to be readable.
See commit 91e4f03604bd089e09154e95294d5d08c805ea49, and in particular the
change from using base_name_compare() to using the idiotic df_name_compare().
It's called 'df_name_compare()' for a reason. That 'df' is because of the
insane directory/file semantics that are simply not a compete ordering.
And because it's not a complete ordering, it's impossible to handle
certain cases, exactly because of the following situation:
a < a-b < a/
but at the same time
a == a/
ie you have a unsatisfiable situation.
And no, changing the ordering to "pure blob order" is _not_ the solution.
Because then you'll no longer traverse the trees in the same order as you
traverse the index, and you'll get into _much_ deeper trouble.
So the real solution was always to never use 'df_name_compare()': any user
of that function is broken by design. It's sadly just the case that the
original unpack_trees() always had those insane semantics, and when I
rewrote it, I was very careful to keep the old semantics. Trust me, I very
much wanted to change them.
So the solution is to change the 'df_name_compare()' (that fundamentally
has that impossible constraint of 'a' == 'a/') to 'base_name_compare()'.
That way name comparisons are always meaningful, and always follow the
rules. The df_name_compare() thing was always a broken hack - just one
that got the semantics we wanted for all the truly simple cases.
And then fix the fallout from that: callers never get mixed-up tree and
blob entries, and have to do their DF checking themselves.
IOW, the starting point is the following. And let me try to see what we
need to do in the callers (this really is just a starting point: we'll
need to clean up all the insane DF conflict marker code that is now
pointless)
Linus
---
From: Linus Torvalds <torvalds@linux-foundation.org>
Date: Sun, 6 Sep 2009 11:15:04 -0700
Subject: [PATCH] Get rid of the broken 'df_name_compare()'
This _will_ break the test-suite, but anything that depends on
df_name_compare() is fundamentally flawed. Because it is designed to
compare directory and blob names as equal, you have 'a' == 'a/', but at
the same time you also have 'a' < 'a-b' < 'a/'.
Out old unpack_trees() semantics depend on that impossible situation,
and we've kept this hack around for a long time. But now somebody has
finally hit the impossible case, and we need to bite the bullet and get
rid of the hack, and fix D/F conflict handling properly.
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
---
cache.h | 1 -
read-cache.c | 35 -----------------------------------
tree-walk.c | 2 +-
unpack-trees.c | 2 +-
4 files changed, 2 insertions(+), 38 deletions(-)
diff --git a/cache.h b/cache.h
index 5fad24c..fda9a49 100644
--- a/cache.h
+++ b/cache.h
@@ -709,7 +709,6 @@ extern int create_symref(const char *ref, const char *refs_heads_master, const c
extern int validate_headref(const char *ref);
extern int base_name_compare(const char *name1, int len1, int mode1, const char *name2, int len2, int mode2);
-extern int df_name_compare(const char *name1, int len1, int mode1, const char *name2, int len2, int mode2);
extern int cache_name_compare(const char *name1, int len1, const char *name2, int len2);
extern void *read_object_with_reference(const unsigned char *sha1,
diff --git a/read-cache.c b/read-cache.c
index 1bbaf1c..f3143d8 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -362,41 +362,6 @@ int base_name_compare(const char *name1, int len1, int mode1,
return (c1 < c2) ? -1 : (c1 > c2) ? 1 : 0;
}
-/*
- * df_name_compare() is identical to base_name_compare(), except it
- * compares conflicting directory/file entries as equal. Note that
- * while a directory name compares as equal to a regular file, they
- * then individually compare _differently_ to a filename that has
- * a dot after the basename (because '\0' < '.' < '/').
- *
- * This is used by routines that want to traverse the git namespace
- * but then handle conflicting entries together when possible.
- */
-int df_name_compare(const char *name1, int len1, int mode1,
- const char *name2, int len2, int mode2)
-{
- int len = len1 < len2 ? len1 : len2, cmp;
- unsigned char c1, c2;
-
- cmp = memcmp(name1, name2, len);
- if (cmp)
- return cmp;
- /* Directories and files compare equal (same length, same name) */
- if (len1 == len2)
- return 0;
- c1 = name1[len];
- if (!c1 && S_ISDIR(mode1))
- c1 = '/';
- c2 = name2[len];
- if (!c2 && S_ISDIR(mode2))
- c2 = '/';
- if (c1 == '/' && !c2)
- return 0;
- if (c2 == '/' && !c1)
- return 0;
- return c1 - c2;
-}
-
int cache_name_compare(const char *name1, int flags1, const char *name2, int flags2)
{
int len1 = flags1 & CE_NAMEMASK;
diff --git a/tree-walk.c b/tree-walk.c
index 02e2aed..8fc0ddc 100644
--- a/tree-walk.c
+++ b/tree-walk.c
@@ -62,7 +62,7 @@ void *fill_tree_descriptor(struct tree_desc *desc, const unsigned char *sha1)
static int entry_compare(struct name_entry *a, struct name_entry *b)
{
- return df_name_compare(
+ return base_name_compare(
a->path, tree_entry_len(a->path, a->sha1), a->mode,
b->path, tree_entry_len(b->path, b->sha1), b->mode);
}
diff --git a/unpack-trees.c b/unpack-trees.c
index 720f7a1..548fef4 100644
--- a/unpack-trees.c
+++ b/unpack-trees.c
@@ -196,7 +196,7 @@ static int do_compare_entry(const struct cache_entry *ce, const struct traverse_
ce_name = ce->name + pathlen;
len = tree_entry_len(n->path, n->sha1);
- return df_name_compare(ce_name, ce_len, S_IFREG, n->path, len, n->mode);
+ return base_name_compare(ce_name, ce_len, S_IFREG, n->path, len, n->mode);
}
static int compare_entry(const struct cache_entry *ce, const struct traverse_info *info, const struct name_entry *n)
^ permalink raw reply related
* Re: [BUG] 'add -u' doesn't work from untracked subdir
From: Clemens Buchacher @ 2009-09-06 18:28 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Jeff King, SZEDER Gábor, git
In-Reply-To: <7vy6ot4x61.fsf@alter.siamese.dyndns.org>
On Sat, Sep 05, 2009 at 01:23:50AM -0700, Junio C Hamano wrote:
> #1 What are the commands that will be affected, other than "add -u" and
> "grep"? Are there others?
There are "ls-tree", "ls-files" and "clean". I can't find anything else.
> #2 Do all the commands in the answer to #1 currently behave exactly the
> same when run without any path parameter and when run with a single
> '.'?
Yes. But "ls-tree" and "ls-files" do not accept superdirectories in their
path specs.
> #3 Do all the commands that are already relative to the root currently
> limit their action to the cwd when run with a single '.'?
I can't think of one that doesn't.
Clemens
^ permalink raw reply
* Re: Strange merge failure (would be overwritten by merge / cannot merge)
From: Junio C Hamano @ 2009-09-06 19:39 UTC (permalink / raw)
To: Linus Torvalds; +Cc: Git Mailing List
In-Reply-To: <alpine.LFD.2.01.0909061053330.8946@localhost.localdomain>
Linus Torvalds <torvalds@linux-foundation.org> writes:
> And then fix the fallout from that: callers never get mixed-up tree and
> blob entries, and have to do their DF checking themselves.
There are two levels of internal APIs involved, and I am getting confused
as to which level of callers you are referrring to in the above.
My understanding of the current situation is:
* unpack_trees() takes a callback from the caller in o->fn(). It
promises (but fails to keep the promise) that the callback is called
with entries with matching names, so that it gets to see D/F
conflicting entries in one go.
* traverse_trees() takes a callback from the caller in info->fn(). It
feeds the callback the entries with the same name most of the time, but
that is not a guarantee, and the bug we are seeing is coming from a
caller, unpack_trees_callback(), assuming it.
Do you mean we would still keep the promise unpack_trees() makes to its
callbacks, e.g. threeway_merge(), or do you mean these callbacks are to be
prepared to get DF-split input themselves and expected to coalesce them
as necessary?
There are only two callers of traverse_trees() interface. unpack_trees()
codepath is the primary one (the other being the merge-tree.c, which is in
disuse).
If you mean unpack_trees() by "callers, ... have to do their DF checking",
it may be a much isolated fix than what I had in mind in the last message
from me---the one that makes traverse_trees() pay attention to the caller
supplied "candiate 'earliest' name" and tries to push the "give entries
from all the trees with the same name to the callback in one go" promise
down to traverse_trees() layer.
If traverse_trees() did not do this name coalescing, on the other hand, I
wonder if it is doing only half of the job to be useful. I do not think
of a plausible scenario where a caller, who wants to walk multiple trees
in parallel, does not want to be fed the entries with the same name from
all the input trees in one invocation of the callback it gives.
If all the callers need to do the name coalescing in order to notice the
situation that led to this bug, I wonder if it would be nicer to do so in
traverse_trees()?
unpack_trees() would of course be helped if traverse_trees() gave that
promise, and I suspect merge-tree.c::threeway_callback() would expect that
behaviour, too.
^ permalink raw reply
* Re: Strange merge failure (would be overwritten by merge / cannot merge)
From: Linus Torvalds @ 2009-09-06 19:54 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Git Mailing List
In-Reply-To: <7v3a6z3lsg.fsf@alter.siamese.dyndns.org>
On Sun, 6 Sep 2009, Junio C Hamano wrote:
> Linus Torvalds <torvalds@linux-foundation.org> writes:
>
> > And then fix the fallout from that: callers never get mixed-up tree and
> > blob entries, and have to do their DF checking themselves.
>
> There are two levels of internal APIs involved, and I am getting confused
> as to which level of callers you are referrring to in the above.
It could be done at any level, but there's a few places where it's easier
than others.
> My understanding of the current situation is:
>
> * unpack_trees() takes a callback from the caller in o->fn(). It
> promises (but fails to keep the promise) that the callback is called
> with entries with matching names, so that it gets to see D/F
> conflicting entries in one go.
Indeed. I'd _like_ to do it at this level (or even at the o->fn() level),
but quite frankly, unpack_trees() is so horribly complicated, and you'd
have to remember state, that doing it at this level is not realy
maintainable.
> * traverse_trees() takes a callback from the caller in info->fn(). It
> feeds the callback the entries with the same name most of the time, but
> that is not a guarantee, and the bug we are seeing is coming from a
> caller, unpack_trees_callback(), assuming it.
This is the level I'm looking at. In fact, I'm going to cheat. I'm not
going to do it when we call info->fn(), I'm going to do it _before_ the
call, and have a special "find conflicts" phase inside traverse_trees()
itself.
That way, any traverse_trees() user will see the conflicts exactly like
they used to, because I'm just going to add a special "find conflicts"
phase there that does the right thing. It's a hack, but it's a "useful"
hack, and it at least avoids being the current "it can't work for the
special case" thing.
> Do you mean we would still keep the promise unpack_trees() makes to its
> callbacks, e.g. threeway_merge(), or do you mean these callbacks are to be
> prepared to get DF-split input themselves and expected to coalesce them
> as necessary?
Either would work, but changing unpack_trees() semantics would just be
very painful. There are just too many users of it, and they are too
ingrained in their expectations of getting conflict information in a
single pass.
I think I have a good solution, give me half an hour to actually get it to
work.
Linus
^ permalink raw reply
* Re: Strange merge failure (would be overwritten by merge / cannot merge)
From: Junio C Hamano @ 2009-09-06 20:36 UTC (permalink / raw)
To: Linus Torvalds; +Cc: Junio C Hamano, Git Mailing List
In-Reply-To: <alpine.LFD.2.01.0909061248470.8946@localhost.localdomain>
Linus Torvalds <torvalds@linux-foundation.org> writes:
> On Sun, 6 Sep 2009, Junio C Hamano wrote:
>...
>> * traverse_trees() takes a callback from the caller in info->fn(). It
>> feeds the callback the entries with the same name most of the time, but
>> that is not a guarantee, and the bug we are seeing is coming from a
>> caller, unpack_trees_callback(), assuming it.
>
> This is the level I'm looking at. In fact, I'm going to cheat. I'm not
> going to do it when we call info->fn(), I'm going to do it _before_ the
> call, and have a special "find conflicts" phase inside traverse_trees()
> itself.
>
> That way, any traverse_trees() user will see the conflicts exactly like
> they used to, because I'm just going to add a special "find conflicts"
> phase there that does the right thing. It's a hack, but it's a "useful"
> hack, and it at least avoids being the current "it can't work for the
> special case" thing.
> ...
> I think I have a good solution, give me half an hour to actually get it to
> work.
Thanks.
The reason I brought up adding the "candidate for the earliest name"
interface to the function was to avoid a case where the index has
blob "t"
blob "t-f"
and all the trees being merged have
blob "t-f"
tree "t"
in which case the "Are we supposed to look at the index too?" logic in
unpack_callback() may not catch the "t-f" entry from the index when the
first callback from traverse_trees() feeds it "t-f". It would notice that
the entry at o->pos is "t". When that happens, I did not think of a clean
way to avoid the codepath from emitting "t" as "only exists in the index".
With the "candidate" addition, traverse_trees() could say "You asked me
that I may have to return 't' to you, and here are the entries from all
the trees." before giving "t-f" back.
Other than that, I think find_conflicts() phase in the traverse_trees()
makes sense.
^ permalink raw reply
* Re: Strange merge failure (would be overwritten by merge / cannot merge)
From: Linus Torvalds @ 2009-09-06 20:42 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Git Mailing List
In-Reply-To: <7vws4bzu7w.fsf@alter.siamese.dyndns.org>
On Sun, 6 Sep 2009, Junio C Hamano wrote:
>
> The reason I brought up adding the "candidate for the earliest name"
> interface to the function was to avoid a case where the index has
>
> blob "t"
> blob "t-f"
>
> and all the trees being merged have
>
> blob "t-f"
> tree "t"
>
> in which case the "Are we supposed to look at the index too?" logic in
> unpack_callback() may not catch the "t-f" entry from the index when the
> first callback from traverse_trees() feeds it "t-f".
I agree. It's why I initially wanted to do it _all_ in the
unpack_callback() thing, but the more I tried, the more complex it got.
So now my plan is to do the conflict handling at a tree level in
traverse_trees(), and get rid of the use of 'df_name_compare()' just there
first.
The index case is slightly easier, as we can go back-and-forth in the
source index (we do try to avoid it right now, but that's a small
optimization rather than anything fundamental), so the index we can
traverse in a more flexible manner, and find the 't' conflict that way.
Linus
^ permalink raw reply
* Re: Strange merge failure (would be overwritten by merge / cannot merge)
From: Linus Torvalds @ 2009-09-06 20:58 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Git Mailing List
In-Reply-To: <alpine.LFD.2.01.0909061338350.8946@localhost.localdomain>
On Sun, 6 Sep 2009, Linus Torvalds wrote:
>
> I agree. It's why I initially wanted to do it _all_ in the
> unpack_callback() thing, but the more I tried, the more complex it got.
>
> So now my plan is to do the conflict handling at a tree level in
> traverse_trees(), and get rid of the use of 'df_name_compare()' just there
> first.
Grr. I need to go off and do some other things, and this still fails a few
tests. This is not my more exhaustive patch that actually tries to
remember the conflict entries we've used up, this is the most cut-down and
simplified "just remove df_name_compare() in tree-walk.c".
And it's not working, for some reason I'm not seeing, but I thought I'd
send it to you just as a way to show where I'm trying to take this. Maybe
you see what my thinko is.
[ In other words: in this version, we only do a single-entry lookahead,
exactly like we used to do before. So this is not meant to _fix_
anything, or change any semantics. It's an incremental "change the model
so that we can look ahead more in a future patch" patch.
But it's broken, and I have to run away for a while. ]
Linus
---
tree-walk.c | 69 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
1 files changed, 67 insertions(+), 2 deletions(-)
diff --git a/tree-walk.c b/tree-walk.c
index 02e2aed..dd563e9 100644
--- a/tree-walk.c
+++ b/tree-walk.c
@@ -62,7 +62,7 @@ void *fill_tree_descriptor(struct tree_desc *desc, const unsigned char *sha1)
static int entry_compare(struct name_entry *a, struct name_entry *b)
{
- return df_name_compare(
+ return base_name_compare(
a->path, tree_entry_len(a->path, a->sha1), a->mode,
b->path, tree_entry_len(b->path, b->sha1), b->mode);
}
@@ -138,6 +138,68 @@ char *make_traverse_path(char *path, const struct traverse_info *info, const str
return path;
}
+/*
+ * See if 'entry' may conflict with a later tree entry in 't': if so,
+ * fill in 'conflict' with the conflicting tree entry from 't'.
+ *
+ * NOTE! Right now we do _not_ create a create a private copy of the tree
+ * descriptor, so we can't actually walk it any further without losing
+ * our place. We should change it to a loop over a copy of the tree
+ * descriptor, but then we'd also have to remember the skipped entries,
+ * so this is a hacky simple case that only handles the case we used
+ * to handle historically (ie clash in the very first entry)
+ *
+ * Note that only a regular file 'entry' can conflict with a later
+ * directory, since a directory with the same name will sort later.
+ */
+static int find_df_conflict(struct tree_desc *t, struct name_entry *entry, struct name_entry *conflict)
+{
+ int len;
+
+ if (S_ISDIR(entry->mode))
+ return 0;
+ len = tree_entry_len(entry->path, entry->sha1);
+
+ while (t->size) {
+ int nlen;
+
+ entry_extract(t, conflict);
+ nlen = tree_entry_len(conflict->path, conflict->sha1);
+
+ /*
+ * We can only have a future conflict if the entry matches the
+ * beginning of the name exactly, and if the next character is
+ * smaller than '/'.
+ *
+ * Break out otherwise.
+ */
+ if (nlen < len)
+ break;
+ if (memcmp(conflict->path, entry->path, nlen))
+ break;
+
+ /*
+ * FIXME! This is the case where we'd like to mark the tree
+ * entry used in the original 't' rather than modify 't'!
+ */
+ if (nlen == len) {
+ update_tree_entry(t);
+ return 1;
+ }
+
+ if (conflict->path[len] > '/')
+ break;
+ /*
+ * FIXME! Here we'd really like to do 'update_tree_entry(©);'
+ * but that requires us to remember the conflict position specially
+ * so now we just punt and stop looking for conflicts
+ */
+ break;
+ }
+ entry_clear(conflict);
+ return 0;
+}
+
int traverse_trees(int n, struct tree_desc *t, struct traverse_info *info)
{
int ret = 0;
@@ -179,12 +241,15 @@ int traverse_trees(int n, struct tree_desc *t, struct traverse_info *info)
dirmask &= mask;
/*
- * Clear all the unused name-entries.
+ * Clear all the unused name-entries, and look for
+ * conflicts.
*/
for (i = 0; i < n; i++) {
if (mask & (1ul << i))
continue;
entry_clear(entry + i);
+ if (find_df_conflict(t+i, entry+last, entry+i))
+ dirmask |= (1ul << i);
}
ret = info->fn(n, mask, dirmask, entry, info);
if (ret < 0)
^ permalink raw reply related
* Re: Strange merge failure (would be overwritten by merge / cannot merge)
From: Junio C Hamano @ 2009-09-06 21:11 UTC (permalink / raw)
To: Linus Torvalds; +Cc: Git Mailing List
In-Reply-To: <alpine.LFD.2.01.0909061338350.8946@localhost.localdomain>
Linus Torvalds <torvalds@linux-foundation.org> writes:
> The index case is slightly easier, as we can go back-and-forth in the
> source index (we do try to avoid it right now, but that's a small
> optimization rather than anything fundamental),...
Almost true, but there is the final "Any left-over entries in the index?"
phase in unpack_trees() that relies on o->pos being correct.
^ permalink raw reply
* Re: Strange merge failure (would be overwritten by merge / cannot merge)
From: Junio C Hamano @ 2009-09-06 21:17 UTC (permalink / raw)
To: Linus Torvalds; +Cc: Git Mailing List
In-Reply-To: <alpine.LFD.2.01.0909061354010.8946@localhost.localdomain>
Linus Torvalds <torvalds@linux-foundation.org> writes:
> Grr. I need to go off and do some other things, and this still fails a few
> tests...
> [ In other words: in this version, we only do a single-entry lookahead,
> exactly like we used to do before. So this is not meant to _fix_
> anything, or change any semantics. It's an incremental "change the model
> so that we can look ahead more in a future patch" patch.
>
> But it's broken, and I have to run away for a while. ]
That's okay. I myself have to go and give a final review to the galley
proof of my book, before the publisher gives a go to the printing press
now.
I do not think we are in a hurry, as this issue, as far as I can tell, has
been with us from early days of git; I would say for the whole lifetime of
the unpack_trees() interface, but I didn't check.
^ permalink raw reply
* Re: Strange merge failure (would be overwritten by merge / cannot merge)
From: Linus Torvalds @ 2009-09-06 21:37 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Git Mailing List
In-Reply-To: <alpine.LFD.2.01.0909061354010.8946@localhost.localdomain>
On Sun, 6 Sep 2009, Linus Torvalds wrote:
>
> And it's not working, for some reason I'm not seeing, but I thought I'd
> send it to you just as a way to show where I'm trying to take this. Maybe
> you see what my thinko is.
Duh. My thinko was that I wasn't testing the right thing. The patch works
fine, my test failures came from the fact that I was working on a branch
with some other broken experimental features.
Now, the patch I sent out did have a buglet: when a conflict happens,
'mask' will not have the conflicting bits we just added to 'dirmask'. But
that buglet doesn't actually seem to affect any of the traverse_tree()
callers, and they are fine with 'dirmask' being separate from 'mask'.
So here's a slightly updated version, and it passes all the tests.
Again, an important note: this is _not_ meant to change semantics, it's
really only meant to re-organize the code so that we _can_ do look-ahead
in the trees for conflicts. But in its current form, the look-ahead is
limited to the next entry, the same as the old code.
So there should be no semantic changes from this patch. Just avoid using
'df_name_compare()' and create that 'find_df_conflict()' function that can
be expanded to actually look ahead more.
Linus
---
tree-walk.c | 72 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++--
1 files changed, 69 insertions(+), 3 deletions(-)
diff --git a/tree-walk.c b/tree-walk.c
index 02e2aed..fb5ca1e 100644
--- a/tree-walk.c
+++ b/tree-walk.c
@@ -62,7 +62,7 @@ void *fill_tree_descriptor(struct tree_desc *desc, const unsigned char *sha1)
static int entry_compare(struct name_entry *a, struct name_entry *b)
{
- return df_name_compare(
+ return base_name_compare(
a->path, tree_entry_len(a->path, a->sha1), a->mode,
b->path, tree_entry_len(b->path, b->sha1), b->mode);
}
@@ -138,6 +138,68 @@ char *make_traverse_path(char *path, const struct traverse_info *info, const str
return path;
}
+/*
+ * See if 'entry' may conflict with a later tree entry in 't': if so,
+ * fill in 'conflict' with the conflicting tree entry from 't'.
+ *
+ * NOTE! Right now we do _not_ create a create a private copy of the tree
+ * descriptor, so we can't actually walk it any further without losing
+ * our place. We should change it to a loop over a copy of the tree
+ * descriptor, but then we'd also have to remember the skipped entries,
+ * so this is a hacky simple case that only handles the case we used
+ * to handle historically (ie clash in the very first entry)
+ *
+ * Note that only a regular file 'entry' can conflict with a later
+ * directory, since a directory with the same name will sort later.
+ */
+static int find_df_conflict(struct tree_desc *t, struct name_entry *entry, struct name_entry *conflict)
+{
+ int len;
+
+ if (S_ISDIR(entry->mode))
+ return 0;
+ len = tree_entry_len(entry->path, entry->sha1);
+
+ while (t->size) {
+ int nlen;
+
+ entry_extract(t, conflict);
+ nlen = tree_entry_len(conflict->path, conflict->sha1);
+
+ /*
+ * We can only have a future conflict if the entry matches the
+ * beginning of the name exactly, and if the next character is
+ * smaller than '/'.
+ *
+ * Break out otherwise.
+ */
+ if (nlen < len)
+ break;
+ if (memcmp(conflict->path, entry->path, nlen))
+ break;
+
+ /*
+ * FIXME! This is the case where we'd like to mark the tree
+ * entry used in the original 't' rather than modify 't'!
+ */
+ if (nlen == len) {
+ update_tree_entry(t);
+ return 1;
+ }
+
+ if (conflict->path[len] > '/')
+ break;
+ /*
+ * FIXME! Here we'd really like to do 'update_tree_entry(©);'
+ * but that requires us to remember the conflict position specially
+ * so now we just punt and stop looking for conflicts
+ */
+ break;
+ }
+ entry_clear(conflict);
+ return 0;
+}
+
int traverse_trees(int n, struct tree_desc *t, struct traverse_info *info)
{
int ret = 0;
@@ -179,14 +241,18 @@ int traverse_trees(int n, struct tree_desc *t, struct traverse_info *info)
dirmask &= mask;
/*
- * Clear all the unused name-entries.
+ * Clear all the unused name-entries, and look for
+ * conflicts.
*/
for (i = 0; i < n; i++) {
if (mask & (1ul << i))
continue;
entry_clear(entry + i);
+ if (find_df_conflict(t+i, entry+last, entry+i))
+ dirmask |= (1ul << i);
}
- ret = info->fn(n, mask, dirmask, entry, info);
+
+ ret = info->fn(n, mask | dirmask, dirmask, entry, info);
if (ret < 0)
break;
if (ret)
^ permalink raw reply related
* Re: [git-svn] [FEATURE-REQ] track merges from git
From: Ximin Luo @ 2009-09-06 22:15 UTC (permalink / raw)
To: Eric Wong; +Cc: git
In-Reply-To: <20090905080337.GE22272@dcvr.yhbt.net>
Eric Wong wrote:
> You may want to try the "set-tree" function of git svn instead of
> dcommit, it was originally named "commit" back in the day set-tree does
> not rewrite any history.
>
> It fell out of favor because you could end up with a lot of non-linear
> history making it difficult for sharing diffs with SVN-using cow-orkers.
>
> It is useful if you don't want to share your individual changesets, but
> push your work upstream to the SVN repos as one big ugly change.
>
> But if you want a staircase effect in gitk, set-tree can be used to make
> individual commits where every change ends up as a merge (and you'll see
> two commits for every change you made)
My problem no longer requires using set-tree (see below), but just to let you
know that when I try to set-tree, it gives:
$ git svn set-tree HEAD
config --get svn-remote.svn.fetch :refs/remotes/git-svn$: command returned
error: 1
In my repo, "remotes/git-svn" doesn't exist; I have
$ git branch -a
* master
test1
remotes/test1
remotes/trunk
but the manual doesn't tell me how to select an svn-remote that's not "git-svn"..
>> (17:16:40) infinity0: i read a thread where it says those are different things
>> (17:16:41) offby1: infinity0: I suspect you're using git svn for something for
>> which it wasn't designed.
>> (17:17:17) infinity0: would it be possible, in theory, to have git-svn store
>> the git merge information in eg. the same way it stores the git-svn tag in the
>> svn commit message
>> (17:17:33) Grum: then just use svn?
>> (17:17:37) Grum: and a postit?
>
> I don't agree with having git-specific metadata on the SVN side itself.
> Often times that git-specific metadata has SHA1s unique to the user that
> committed it, so it wouldn't be useful to anyone else unless users are
> merging from each others git repos (which is not an easy/natural
> workflow if SVN is the mainline). Patch exchange is more
> reliable/easier...
>
> I've also worked in places where alternative tools are frowned upon, so
> sending git-specific metadata over to SVN should always be optional.
>
> The majority of folks I've worked with on SVN-hosted projects have never
> known about my git usage (that is changing as git popularity increases,
> however).
>
>> (17:18:01) infinity0: i'm trying to link two separate svn repos together via git
>> (17:18:17) Grum: and that is just what offby1 said
>> (17:18:30) infinity0: "what" is
>> (17:18:40) Grum: I suspect you're using git svn for something for which it
>> wasn't designed.
>> (17:18:42) infinity0: as you all are saying, git merges and svn "merges" are
>> different things
>> (17:18:58) infinity0: ok, but it would be possible to make git-svn have this
>> functionality? or not
>> (17:18:59) offby1: certainly
>> (17:19:16) offby1: I fear not, since Eric Wong seems like a smart fella; if it
>> were doable, I suspect he'd have done it already.
>> (17:19:21) offby1: But then ... who knows, maybe he's busy.
>
> I'm not smart but I am busy :)
>
> Summary of the git svn merge tracking situation:
>
> Mapping git <-> git merges to SVN:
>
> * already doable for the committing user with set-tree,
> but makes history ugly for:
>
> a) yourself (with every commit set-tree'd individually)
> b) SVN users (single set-tree with the newest commit)
> c) all of the above (varying granularity)
>
> * Pushing git metadata to SVN will annoy SVN-only users
>
> * Putting git metadata in SVN may not be useful since SHA1s
> may be specific to the user that made that commit.
>
> Mapping SVN <-> SVN merges to SVN via git svn:
>
> * most likely to be doable, they'll become plain SVN <-> SVN merges,
> see problems with getting SVN <-> SVN merges back to git, however...
>
> Mapping SVN <-> SVN merges to git:
>
> * SVN can represent merges that git can't, SVN can be/is extremely
> complicated when it comes to merges.
>
> * I don't see many projects (I care about) use SVN merge tracking,
> which projects actually use it? Maybe it's still too new and
> distros/users are behind the upgrade curve...
>
> I've probably missed some, I've been dozing off while replying to
> emails...
>
Actually, it turns out that my original problem is simpler than any of these
scenarios. What I was doing was git <-> git merges, where both of these git
branches were tracking *different* SVN branches (in my original case, from
different repos; in my simplified test case, from the same repo).
Consider this scenario:
----A0*-----A1---+
\ \
B0*----B1----B2
branchA: A1
branchB: B2
Where A0* and B0* have both been dcommited into their SVN branches, but A1, B1
and B2 are present in the git repo only. A0* and B0* have the "git-svn-id" tag,
and show my svn committer name; A1, B1 and B2 are still pure git commits, and
show my git commiter name.
Scenario 1:
If HEAD is at B2, and we try to "git-svn dcommit", then what happens currently
is, git-svn will dcommit B1 then B2, re-writing them in the process (adding
git-svn-id and using the svn credentials instead of git credentials); however,
it will *miss out* dcommitting A1. So we get this:
----A0*-----A1----+
\ \
B0*----B1*----B2*
branchA: A1
branchB: B2*
where B1* and B2* are the re-written versions of B1 and B2, with the added
git-svn-id and the svn committer name instead of the git committer name. There
isn't a problem yet; but when we switch to branchA and dcommit, we get this:
+-----A1*
/
----A0*-----A1----+
\ \
B0*----B1*----B2*
branchA: A1*
branchB: B2*
Where A1* is the re-written version of A1. But B2* still has A1 as a parent,
and now we have an extra "duplicate" commit in our git repo. What's even worse
is that A1* is **not a parent** of B2*, and so future merges on the branches
will potentially need to resolve conflicts that were resolved already when
merging (A1, B1) to B2.
Scenario 2:
If however, we dcommit A1, then B1, then B2, the commits will be re-written in
such a way that the proper merge history is preserved, including the correct
parents.
In one of the follow-up emails to my original posting, I supplied a test script
(gitsvntest.sh) which demonstrates the 2 scenarios. You can use a GUI to review
the history graphs. I can re-send it if you can't find that email.
I'm not sure how hard this is to fix; I guess it would involve making dcommit
detecting the case where a commit has more than 1 parent, and recursing down
all the parents to see if they are tracking an svn branch.
At the very least, I think this situation can be detected and the user warned.
In switching from svn to git, git-svn was very helpful to me, but this
behaviour confused me completely - sometimes things worked fine, depending on
the order in which I dcommited stuff, so it was incredibly hard to figure out,
especially since at that time I had no understanding of the concept of object
graphs and rewriting commits and that sort of thing.
X
^ permalink raw reply
* Re: Strange merge failure (would be overwritten by merge / cannot merge)
From: Linus Torvalds @ 2009-09-06 22:49 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Git Mailing List
In-Reply-To: <alpine.LFD.2.01.0909061424160.8946@localhost.localdomain>
On Sun, 6 Sep 2009, Linus Torvalds wrote:
>
> So here's a slightly updated version, and it passes all the tests.
.. and here's something with a bit more abstraction, a bit more cleanup,
and making more sure that there's no semantic changes. So that I can feel
happy signing off on it.
Linus
---
From: Linus Torvalds <torvalds@linux-foundation.org>
Date: Sun, 6 Sep 2009 14:37:21 -0700
Subject: [PATCH] Prepare 'traverse_trees()' for D/F conflict lookahead
traverse_trees() used to always walk the trees in order, and used the
special (and fundamentally broken) 'df_name_compare()' function to
compare directory and file entries as equal.
That works fine for all the common cases, when the D/F conflicts are
immediately adjacent, and there are no other entries that could confuse
the ordering. But if you have one tree with a file 'a', and another
tree with a file 'a-1' and a directory 'a/', then you would not see the
D/F conflict of 'a' and 'a/' without looking ahead past the 'a-1' file
in the other tree.
So this re-organizes the tree walking code so that we can start doing
look-ahead for those cases. It doesn't actually _do_ that lookahead
yet, because it requires marking the conflicts we've used, but the code
is now organized to do so.
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
---
tree-walk.c | 90 +++++++++++++++++++++++++++++++++++++++++++++++++++++++---
1 files changed, 85 insertions(+), 5 deletions(-)
diff --git a/tree-walk.c b/tree-walk.c
index 02e2aed..7251dd2 100644
--- a/tree-walk.c
+++ b/tree-walk.c
@@ -62,7 +62,7 @@ void *fill_tree_descriptor(struct tree_desc *desc, const unsigned char *sha1)
static int entry_compare(struct name_entry *a, struct name_entry *b)
{
- return df_name_compare(
+ return base_name_compare(
a->path, tree_entry_len(a->path, a->sha1), a->mode,
b->path, tree_entry_len(b->path, b->sha1), b->mode);
}
@@ -138,6 +138,80 @@ char *make_traverse_path(char *path, const struct traverse_info *info, const str
return path;
}
+/*
+ * See if 'entry' may conflict with a later tree entry in 't': if so,
+ * fill in 'conflict' with the conflicting tree entry from 't'.
+ *
+ * NOTE! Right now we do _not_ create a create a private copy of the tree
+ * descriptor, so we can't actually walk it any further without losing
+ * our place. We should change it to a loop over a copy of the tree
+ * descriptor, but then we'd also have to remember the skipped entries,
+ * so this is a hacky simple case that only handles the case we used
+ * to handle historically (ie clash in the very first entry)
+ *
+ * Note that only a regular file 'entry' can conflict with a later
+ * directory, since a directory with the same name will sort later.
+ */
+static int find_df_conflict(struct tree_desc *t, struct name_entry *entry, struct name_entry *conflict)
+{
+ int len;
+
+ if (S_ISDIR(entry->mode))
+ return 0;
+ len = tree_entry_len(entry->path, entry->sha1);
+
+ while (t->size) {
+ int nlen;
+
+ entry_extract(t, conflict);
+ nlen = tree_entry_len(conflict->path, conflict->sha1);
+
+ /*
+ * We can only have a future conflict if the entry matches the
+ * beginning of the name exactly, and if the next character is
+ * smaller than '/'.
+ *
+ * Break out otherwise.
+ */
+ if (nlen < len)
+ break;
+ if (memcmp(conflict->path, entry->path, nlen))
+ break;
+ if (nlen == len)
+ return 1;
+
+ if (conflict->path[len] > '/')
+ break;
+ /*
+ * FIXME! Here we'd really like to do 'update_tree_entry(©);'
+ * but that requires us to remember the conflict position specially
+ * so now we just punt and stop looking for conflicts
+ */
+ break;
+ }
+ entry_clear(conflict);
+ return 0;
+}
+
+/*
+ * For now, the used entries are always at the head of the tree_desc
+ * (no look-ahead), so marking an entry used is always just a matter
+ * of doing an 'update_tree_entry()'
+ */
+static void used_entry(struct tree_desc *t, struct name_entry *entry)
+{
+ update_tree_entry(t);
+}
+
+static int get_entry(struct tree_desc *t, struct name_entry *entry)
+{
+ if (t->size) {
+ entry_extract(t, entry);
+ return 1;
+ }
+ return 0;
+}
+
int traverse_trees(int n, struct tree_desc *t, struct traverse_info *info)
{
int ret = 0;
@@ -150,9 +224,8 @@ int traverse_trees(int n, struct tree_desc *t, struct traverse_info *info)
last = -1;
for (i = 0; i < n; i++) {
- if (!t[i].size)
+ if (!get_entry(t+i, entry+i))
continue;
- entry_extract(t+i, entry+i);
if (last >= 0) {
int cmp = entry_compare(entry+i, entry+last);
@@ -179,13 +252,20 @@ int traverse_trees(int n, struct tree_desc *t, struct traverse_info *info)
dirmask &= mask;
/*
- * Clear all the unused name-entries.
+ * Clear all the unused name-entries, and look for
+ * conflicts.
*/
for (i = 0; i < n; i++) {
if (mask & (1ul << i))
continue;
entry_clear(entry + i);
+ if (find_df_conflict(t+i, entry+last, entry+i))
+ dirmask |= (1ul << i);
}
+
+ /* Add in the DF conflict entries into the mask */
+ mask |= dirmask;
+
ret = info->fn(n, mask, dirmask, entry, info);
if (ret < 0)
break;
@@ -194,7 +274,7 @@ int traverse_trees(int n, struct tree_desc *t, struct traverse_info *info)
ret = 0;
for (i = 0; i < n; i++) {
if (mask & (1ul << i))
- update_tree_entry(t + i);
+ used_entry(t+i, entry+i);
}
}
free(entry);
^ permalink raw reply related
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