* [PATCH v2 0/2] fix data corruption in fast-import
From: Dmitry Ivankov @ 2011-08-14 18:32 UTC (permalink / raw)
To: git; +Cc: Jonathan Nieder, Shawn O. Pearce, David Barr, Dmitry Ivankov
In-Reply-To: <1313145170-24471-1-git-send-email-divanorama@gmail.com>
It turns out the bug is older than "M 040000.." command.
Managed to reproduce with just "C .." command from tags/v1.5.3-rc2~6^2
b6f3481b.. Teach fast-import to recursively copy files/directories (Jul 15 2007)
And even better, there is no need to add a explicit check for sha1 mismatch (we
may still want to have this, but it can go separately).
Basically the test does:
Fill two distinct directories old/a, old/b
Commit them
(necessary to make trees have sha1 computed and thus become potential delta bases)
C old new
C old/a new/b
M ... new/b/new_file
new/b is stored as a delta against old/a, but with delta base pointing to old/b.
And so ls-tree new/b fails, fsck fails both with "failed to apply delta".
Dmitry Ivankov (2):
fast-import: add a test for tree delta base corruption
fast-import: prevent producing bad delta
fast-import.c | 35 ++++++++++++++++++++++++++++++-----
t/t9300-fast-import.sh | 41 +++++++++++++++++++++++++++++++++++++++++
2 files changed, 71 insertions(+), 5 deletions(-)
--
1.7.3.4
^ permalink raw reply
* [PATCH v2 1/2] fast-import: add a test for tree delta base corruption
From: Dmitry Ivankov @ 2011-08-14 18:32 UTC (permalink / raw)
To: git; +Cc: Jonathan Nieder, Shawn O. Pearce, David Barr, Dmitry Ivankov
In-Reply-To: <1313145170-24471-1-git-send-email-divanorama@gmail.com>
fast-import is able to write imported tree objects in delta format.
It holds a tree structure in memory where each tree entry may have
a delta base sha1 assigned. When delta base data is needed it is
reconstructed from this in-memory structure. Though sometimes the
delta base data doesn't match the delta base sha1 so wrong or even
corrupt pack is produced.
Add a small test that produces a corrupt pack. It uses just tree
copy and file modification commands aside from the very basic commit
and blob commands.
Signed-off-by: Dmitry Ivankov <divanorama@gmail.com>
---
t/t9300-fast-import.sh | 41 +++++++++++++++++++++++++++++++++++++++++
1 files changed, 41 insertions(+), 0 deletions(-)
diff --git a/t/t9300-fast-import.sh b/t/t9300-fast-import.sh
index f256475..e2b94b5 100755
--- a/t/t9300-fast-import.sh
+++ b/t/t9300-fast-import.sh
@@ -734,6 +734,47 @@ test_expect_success \
git diff-tree --abbrev --raw L^ L >output &&
test_cmp expect output'
+cat >input <<INPUT_END
+blob
+mark :1
+data <<EOF
+the data
+EOF
+
+commit refs/heads/L2
+committer C O Mitter <committer@example.com> 1112912473 -0700
+data <<COMMIT
+init L2
+COMMIT
+M 644 :1 a/b/c
+M 644 :1 a/b/d
+M 644 :1 a/e/f
+
+commit refs/heads/L2
+committer C O Mitter <committer@example.com> 1112912473 -0700
+data <<COMMIT
+update L2
+COMMIT
+C a g
+C a/e g/b
+M 644 :1 g/b/h
+INPUT_END
+
+cat <<EOF >expect
+g/b/f
+g/b/h
+EOF
+
+test_expect_failure \
+ 'L: nested tree copy does not corrupt deltas' \
+ 'git fast-import <input &&
+ git ls-tree L2 g/b/ >tmp &&
+ cat tmp | cut -f 2 >actual &&
+ test_cmp expect actual &&
+ git fsck `git rev-parse L2`'
+
+git update-ref -d refs/heads/L2
+
###
### series M
###
--
1.7.3.4
^ permalink raw reply related
* [PATCH v2 2/2] fast-import: prevent producing bad delta
From: Dmitry Ivankov @ 2011-08-14 18:32 UTC (permalink / raw)
To: git; +Cc: Jonathan Nieder, Shawn O. Pearce, David Barr, Dmitry Ivankov
In-Reply-To: <1313145170-24471-1-git-send-email-divanorama@gmail.com>
To produce deltas for tree objects fast-import tracks two versions
of tree's entries - base and current one. Base version stands both
for a delta base of this tree, and for a entry inside a delta base
of a parent tree. So care should be taken to keep it in sync.
tree_content_set cuts away a whole subtree and replaces it with a
new one (or NULL for lazy load of a tree with known sha1). It
keeps a base sha1 for this subtree (needed for parent tree). And
here is the problem, 'subtree' tree root doesn't have the implied
base version entries.
Adjusting the subtree to include them would mean a deep rewrite of
subtree. Invalidating the subtree base version would mean recursive
invalidation of parents' base versions. So just mark this tree as
do-not-delta me. Abuse setuid bit for this purpose.
tree_content_replace is the same as tree_content_set except that is
is used to replace the root, so just clearing base sha1 here (instead
of setting the bit) is fine.
[di: log message]
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Dmitry Ivankov <divanorama@gmail.com>
---
fast-import.c | 35 ++++++++++++++++++++++++++++++-----
t/t9300-fast-import.sh | 2 +-
2 files changed, 31 insertions(+), 6 deletions(-)
diff --git a/fast-import.c b/fast-import.c
index 7cc2262..0be7629 100644
--- a/fast-import.c
+++ b/fast-import.c
@@ -170,6 +170,11 @@ Format of STDIN stream:
#define DEPTH_BITS 13
#define MAX_DEPTH ((1<<DEPTH_BITS)-1)
+/*
+ * We abuse the setuid bit on directories to mean "do not delta".
+ */
+#define NO_DELTA S_ISUID
+
struct object_entry {
struct pack_idx_entry idx;
struct object_entry *next;
@@ -1416,8 +1421,9 @@ static void mktree(struct tree_content *t, int v, struct strbuf *b)
struct tree_entry *e = t->entries[i];
if (!e->versions[v].mode)
continue;
- strbuf_addf(b, "%o %s%c", (unsigned int)e->versions[v].mode,
- e->name->str_dat, '\0');
+ strbuf_addf(b, "%o %s%c",
+ (unsigned int)(e->versions[v].mode & ~NO_DELTA),
+ e->name->str_dat, '\0');
strbuf_add(b, e->versions[v].sha1, 20);
}
}
@@ -1427,7 +1433,7 @@ static void store_tree(struct tree_entry *root)
struct tree_content *t = root->tree;
unsigned int i, j, del;
struct last_object lo = { STRBUF_INIT, 0, 0, /* no_swap */ 1 };
- struct object_entry *le;
+ struct object_entry *le = NULL;
if (!is_null_sha1(root->versions[1].sha1))
return;
@@ -1437,7 +1443,8 @@ static void store_tree(struct tree_entry *root)
store_tree(t->entries[i]);
}
- le = find_object(root->versions[0].sha1);
+ if (!(root->versions[0].mode & NO_DELTA))
+ le = find_object(root->versions[0].sha1);
if (S_ISDIR(root->versions[0].mode) && le && le->pack_id == pack_id) {
mktree(t, 0, &old_tree);
lo.data = old_tree;
@@ -1471,6 +1478,7 @@ static void tree_content_replace(
{
if (!S_ISDIR(mode))
die("Root cannot be a non-directory");
+ hashclr(root->versions[0].sha1);
hashcpy(root->versions[1].sha1, sha1);
if (root->tree)
release_tree_content_recursive(root->tree);
@@ -1515,6 +1523,23 @@ static int tree_content_set(
if (e->tree)
release_tree_content_recursive(e->tree);
e->tree = subtree;
+
+ /*
+ * We need to leave e->versions[0].sha1 alone
+ * to avoid modifying the preimage tree used
+ * when writing out the parent directory.
+ * But after replacing the subdir with a
+ * completely different one, it's not a good
+ * delta base any more, and besides, we've
+ * thrown away the tree entries needed to
+ * make a delta against it.
+ *
+ * So let's just explicitly disable deltas
+ * for the subtree.
+ */
+ if (S_ISDIR(e->versions[0].mode))
+ e->versions[0].mode |= NO_DELTA;
+
hashclr(root->versions[1].sha1);
return 1;
}
@@ -2929,7 +2954,7 @@ static void print_ls(int mode, const unsigned char *sha1, const char *path)
/* mode SP type SP object_name TAB path LF */
strbuf_reset(&line);
strbuf_addf(&line, "%06o %s %s\t",
- mode, type, sha1_to_hex(sha1));
+ mode & ~NO_DELTA, type, sha1_to_hex(sha1));
quote_c_style(path, &line, NULL, 0);
strbuf_addch(&line, '\n');
}
diff --git a/t/t9300-fast-import.sh b/t/t9300-fast-import.sh
index e2b94b5..106e3f3 100755
--- a/t/t9300-fast-import.sh
+++ b/t/t9300-fast-import.sh
@@ -765,7 +765,7 @@ g/b/f
g/b/h
EOF
-test_expect_failure \
+test_expect_success \
'L: nested tree copy does not corrupt deltas' \
'git fast-import <input &&
git ls-tree L2 g/b/ >tmp &&
--
1.7.3.4
^ permalink raw reply related
* [PATCH resend] Makefile: Use computed header dependencies if the compiler supports it
From: Fredrik Kuivinen @ 2011-08-14 18:45 UTC (permalink / raw)
To: git; +Cc: Fredrik Kuivinen, Jonathan Nieder
Previously you had to manually define COMPUTE_HEADER_DEPENDENCIES to
enable this feature. It seemed a bit sad that such a useful feature
had to be enabled manually.
Signed-off-by: Fredrik Kuivinen <frekui@gmail.com>
---
This is a resend, it has been rebased on top of master but otherwise
the same patch was sent 2011-06-11. Jonathan Nieder has been added to
the Cc list as he implemented the computed header dependencies
feature.
Makefile | 13 +++++++++----
1 files changed, 9 insertions(+), 4 deletions(-)
diff --git a/Makefile b/Makefile
index 8dd782f..c289074 100644
--- a/Makefile
+++ b/Makefile
@@ -250,10 +250,6 @@ all::
# DEFAULT_EDITOR='$GIT_FALLBACK_EDITOR',
# DEFAULT_EDITOR='"C:\Program Files\Vim\gvim.exe" --nofork'
#
-# Define COMPUTE_HEADER_DEPENDENCIES if your compiler supports the -MMD option
-# and you want to avoid rebuilding objects when an unrelated header file
-# changes.
-#
# Define CHECK_HEADER_DEPENDENCIES to check for problems in the hard-coded
# dependency rules.
#
@@ -1236,6 +1232,15 @@ endif
ifdef CHECK_HEADER_DEPENDENCIES
COMPUTE_HEADER_DEPENDENCIES =
USE_COMPUTED_HEADER_DEPENDENCIES =
+else
+dep_check = $(shell sh -c \
+ ': > ++empty.c; \
+ $(CC) -c -MF /dev/null -MMD -MP ++empty.c -o /dev/null 2>&1; \
+ echo $$?; \
+ $(RM) ++empty.c')
+ifeq ($(dep_check),0)
+COMPUTE_HEADER_DEPENDENCIES=YesPlease
+endif
endif
ifdef COMPUTE_HEADER_DEPENDENCIES
--
1.7.5.3.368.g8b1b7.dirty
^ permalink raw reply related
* Re: [PATCH resend] Makefile: Use computed header dependencies if the compiler supports it
From: Jonathan Nieder @ 2011-08-14 19:00 UTC (permalink / raw)
To: Fredrik Kuivinen; +Cc: git
In-Reply-To: <1313347512-7815-1-git-send-email-frekui@gmail.com>
Hi,
Fredrik Kuivinen wrote:
> Previously you had to manually define COMPUTE_HEADER_DEPENDENCIES to
> enable this feature. It seemed a bit sad that such a useful feature
> had to be enabled manually.
Yes! Thanks for this.
I have a few thoughts about the implementation:
> --- a/Makefile
> +++ b/Makefile
[...]
> @@ -1236,6 +1232,15 @@ endif
> ifdef CHECK_HEADER_DEPENDENCIES
> COMPUTE_HEADER_DEPENDENCIES =
> USE_COMPUTED_HEADER_DEPENDENCIES =
> +else
> +dep_check = $(shell sh -c \
> + ': > ++empty.c; \
> + $(CC) -c -MF /dev/null -MMD -MP ++empty.c -o /dev/null 2>&1; \
> + echo $$?; \
> + $(RM) ++empty.c')
> +ifeq ($(dep_check),0)
> +COMPUTE_HEADER_DEPENDENCIES=YesPlease
> +endif
This causes "make foo" to run gcc and create a temporary file
unconditionally, regardless of what foo is. In an ideal world:
- the autodetection would only happen when building targets that
care about it
- the detection would happen once (creating some file to store the
result) and not be repeated with each invocation of "make"
- (maybe) there would be a way to override the detection with
either a "yes" or "no" result, for those who really care to
save a little time.
I was about to say that the GIT_VERSION variable has some of these
properties, but now that I check, from the point of view of the
Makefile it doesn't. ./GIT-VERSION-GEN is just very fast. :)
I wonder if we can make do with a faster check, like
$(CC) -c -MF /dev/null -MMD -MP git.c --help >/dev/null 2>&1
What do you think?
^ permalink raw reply
* [PATCH] shell: add base path and ~ expansion support.
From: Dan Charney @ 2011-08-14 19:25 UTC (permalink / raw)
To: git
By default, paths that are forwarded to git-shell when users try to
pull from an ssh:// URI are relative to the server's root folder (/).
Add support for an environment variable, GIT_SHELL_BASE_PATH, to
allow sysadmins to make these paths relative to somewhere else
(e.g. /srv/git). Expand a leading "~/" to the value of $HOME.
Allow an additional leading slash to specify a path relative to the
root folder (/).
This change does not improve security in the slightest, just makes
ssh:// URIs prettier.
Signed-off-by: Dan Charney <git@drmoose.net>
---
I should point out that I've not tested the the behavior with the "cvs emulator" mode, in large part because CVS remains thoroughly baffling to me. Despite many attempts to learn how to use it, I never did move beyond the "copy and paste somebody else's pre-written commands" phase.
This is my first attempt at contributing to a project this big, and this is the first time this year I've written any C. Free to tear me a new one if I've done something stupid :-)
Documentation/git-shell.txt | 25 ++++++++++++++++++++++++-
shell.c | 37 ++++++++++++++++++++++++++++++++++++-
2 files changed, 60 insertions(+), 2 deletions(-)
diff --git a/Documentation/git-shell.txt b/Documentation/git-shell.txt
index 9b92506..280282e 100644
--- a/Documentation/git-shell.txt
+++ b/Documentation/git-shell.txt
@@ -22,13 +22,36 @@ is started in interactive mode when no arguments are given; in this
case, COMMAND_DIR must exist, and any of the executables in it can be
invoked.
-'cvs server' is a special command which executes git-cvsserver.
+'cvs server' is a special command which executes git-cvsserver. If
+"$GIT_SHELL_BASE_PATH" is set, it will be passed to git-cvsserver as
+the --base-path parameter.
COMMAND_DIR is the path "$HOME/git-shell-commands". The user must have
read and execute permissions to the directory in order to execute the
programs in it. The programs are executed with a cwd of $HOME, and
<argument> is parsed as a command-line string.
+PATH MAPPING
+------------
+
+If the environment variable "$GIT_SHELL_BASE_PATH" is set and the shell
+is not in interactive mode, $GIT_SHELL_BASE_PATH will be prepended to
+the front the <argument> passed to <command>. This is analogous to
+the --base-path switch from link:git-daemon[1] - if your environment
+variables include GIT_SHELL_BASE_PATH=/srv/git on example.com, and later
+someone pulls ssh://example.com/my.git, it will be interpreted as
+/srv/git/my.git. This feature does not restrict clients to subfolders
+of the base path; path elements such as .. are allowed.
+
+If the environment variable "$HOME" is set, git-shell will replace a leading
+"~/" in the <argument> with the value of "$HOME".
+
+Example::
+ If GIT_SHELL_BASE_PATH=/srv/git and HOME=/home/joeuser, then::
+ ssh://example.com/some.git will map to /srv/git/some.git
+ ssh://example.com/~/my.git will map to /home/joeuser/my.git
+ ssh://example.com//var/git/x.git will map to /var/git/x.git
+
GIT
---
Part of the linkgit:git[1] suite
diff --git a/shell.c b/shell.c
index abb8622..e250b2a 100644
--- a/shell.c
+++ b/shell.c
@@ -6,6 +6,37 @@
#define COMMAND_DIR "git-shell-commands"
#define HELP_COMMAND COMMAND_DIR "/help"
+#define BASE_PATH_VARIABLE "GIT_SHELL_BASE_PATH"
+#define DEFAULT_PATH "/"
+
+static const char *adjust_argument_path(const char *arg)
+{
+ struct strbuf path = STRBUF_INIT;
+ const char *env;
+ const char *prefix_variable = NULL;
+
+ if (arg == NULL || arg[0] != '/')
+ prefix_variable = BASE_PATH_VARIABLE;
+ else if (arg[1] == '/')
+ ++arg;
+ else if (arg[1] == '~' && arg[2] == '/') {
+ prefix_variable = "HOME";
+ arg += 2;
+ } else
+ prefix_variable = BASE_PATH_VARIABLE;
+
+ if (NULL != prefix_variable) {
+ env = getenv(prefix_variable);
+ if (NULL != env)
+ strbuf_addstr(&path, env);
+ }
+
+ if (NULL != arg)
+ strbuf_addstr(&path, arg);
+ if (NULL == path.buf || path.buf[0] == '\0')
+ return DEFAULT_PATH;
+ return path.buf;
+}
static int do_generic_cmd(const char *me, char *arg)
{
@@ -18,7 +49,7 @@ static int do_generic_cmd(const char *me, char *arg)
die("bad command");
my_argv[0] = me + 4;
- my_argv[1] = arg;
+ my_argv[1] = adjust_argument_path(arg);
my_argv[2] = NULL;
return execv_git_cmd(my_argv);
@@ -30,6 +61,10 @@ static int do_cvs_cmd(const char *me, char *arg)
"cvsserver", "server", NULL
};
+ const char *base_path;
+ base_path = adjust_argument_path(NULL);
+ setenv("GIT_CVSSERVER_BASE_PATH", base_path, 1);
+
if (!arg || strcmp(arg, "server"))
die("git-cvsserver only handles server: %s", arg);
--
1.7.4
^ permalink raw reply related
* Re: [PATCH resend] Makefile: Use computed header dependencies if the compiler supports it
From: Fredrik Kuivinen @ 2011-08-14 19:53 UTC (permalink / raw)
To: Jonathan Nieder; +Cc: git
In-Reply-To: <20110814190050.GA16819@elie.gateway.2wire.net>
Hi Jonathan,
Thanks for your comments.
On Sun, Aug 14, 2011 at 21:00, Jonathan Nieder <jrnieder@gmail.com> wrote:
>> --- a/Makefile
>> +++ b/Makefile
> [...]
>> @@ -1236,6 +1232,15 @@ endif
>> ifdef CHECK_HEADER_DEPENDENCIES
>> COMPUTE_HEADER_DEPENDENCIES =
>> USE_COMPUTED_HEADER_DEPENDENCIES =
>> +else
>> +dep_check = $(shell sh -c \
>> + ': > ++empty.c; \
>> + $(CC) -c -MF /dev/null -MMD -MP ++empty.c -o /dev/null 2>&1; \
>> + echo $$?; \
>> + $(RM) ++empty.c')
>> +ifeq ($(dep_check),0)
>> +COMPUTE_HEADER_DEPENDENCIES=YesPlease
>> +endif
>
> This causes "make foo" to run gcc and create a temporary file
> unconditionally, regardless of what foo is. In an ideal world:
>
> - the autodetection would only happen when building targets that
> care about it
In an ideal world, yes. But I don't see a simple and maintainable
way to do it for only those targets.
> - the detection would happen once (creating some file to store the
> result) and not be repeated with each invocation of "make"
>
> - (maybe) there would be a way to override the detection with
> either a "yes" or "no" result, for those who really care to
> save a little time.
I have done some benchmarking (see below) and considering the
results I got, I think implementing any of these two is not worth it.
> I was about to say that the GIT_VERSION variable has some of these
> properties, but now that I check, from the point of view of the
> Makefile it doesn't. ./GIT-VERSION-GEN is just very fast. :)
>
> I wonder if we can make do with a faster check, like
>
> $(CC) -c -MF /dev/null -MMD -MP git.c --help >/dev/null 2>&1
>
> What do you think?
>
Here are some benchmarks done with 'perf stat'. Before each invocation
of perf stat I ran a plain 'make' to make sure that everything was compiled.
Note that a fully compiled source tree is the worst case when it comes to
the overhead of the auto-detection.
Without patch (with COMPUTE_HEADER_DEPENDENCIES=Yes):
Performance counter stats for 'make' (10 runs):
1,566,393 cache-misses # 1.557 M/sec ( +- 0.264% )
6,428,212 cache-references # 6.391 M/sec ( +- 0.168% )
21,245,775 branch-misses # 4.626 % ( +- 0.015% )
459,268,954 branches # 456.585 M/sec ( +- 0.031% )
2,594,717,999 instructions # 1.177 IPC ( +- 0.022% )
2,205,246,745 cycles # 2192.359 M/sec ( +- 0.136% )
43,532 page-faults # 0.043 M/sec ( +- 0.034% )
215 CPU-migrations # 0.000 M/sec ( +- 0.891% )
457 context-switches # 0.000 M/sec ( +- 0.654% )
1005.878305 task-clock-msecs # 1.022 CPUs ( +- 0.544% )
0.984526665 seconds time elapsed ( +- 0.591% )
With patch:
Performance counter stats for 'make' (10 runs):
1,796,342 cache-misses # 1.732 M/sec ( +- 0.702% )
6,929,739 cache-references # 6.682 M/sec ( +- 0.186% )
21,582,772 branch-misses # 4.575 % ( +- 0.032% )
471,783,920 branches # 454.934 M/sec ( +- 0.024% )
2,662,671,428 instructions # 1.166 IPC ( +- 0.017% )
2,282,907,087 cycles # 2201.372 M/sec ( +- 0.162% )
49,244 page-faults # 0.047 M/sec ( +- 0.031% )
233 CPU-migrations # 0.000 M/sec ( +- 0.823% )
489 context-switches # 0.000 M/sec ( +- 0.460% )
1037.038252 task-clock-msecs # 1.022 CPUs ( +- 0.579% )
1.014409177 seconds time elapsed ( +- 0.476% )
With patch, but changed to use git.c instead of ++empty.c:
Performance counter stats for 'make' (10 runs):
2,125,147 cache-misses # 1.737 M/sec ( +- 0.287% )
9,080,043 cache-references # 7.423 M/sec ( +- 0.185% )
24,573,023 branch-misses # 4.222 % ( +- 0.032% )
582,018,515 branches # 475.809 M/sec ( +- 0.012% )
3,185,328,930 instructions # 1.165 IPC ( +- 0.009% )
2,734,176,502 cycles # 2235.229 M/sec ( +- 0.122% )
51,032 page-faults # 0.042 M/sec ( +- 0.034% )
227 CPU-migrations # 0.000 M/sec ( +- 0.943% )
515 context-switches # 0.000 M/sec ( +- 0.351% )
1223.219930 task-clock-msecs # 1.019 CPUs ( +- 0.579% )
1.200869268 seconds time elapsed ( +- 0.555% )
So, on my machine the auto-detection logic adds a slight overhead
(0.03s, 3% in a fully compiled tree). Using git.c is slower than using
++empty.c. IMHO adding any extra complexity to lower the 0.03s
is not worth it.
- Fredrik
^ permalink raw reply
* Re: [PATCH resend] Makefile: Use computed header dependencies if the compiler supports it
From: Jonathan Nieder @ 2011-08-14 20:02 UTC (permalink / raw)
To: Fredrik Kuivinen; +Cc: git
In-Reply-To: <CALx8hKRBjXr44gM1JA+d=RU80pmruPV56s-G3JvViz87eJ=ajQ@mail.gmail.com>
Fredrik Kuivinen wrote:
> On Sun, Aug 14, 2011 at 21:00, Jonathan Nieder <jrnieder@gmail.com> wrote:
>> I wonder if we can make do with a faster check, like
>>
>> $(CC) -c -MF /dev/null -MMD -MP git.c --help >/dev/null 2>&1
>>
>> What do you think?
[...]
> Without patch (with COMPUTE_HEADER_DEPENDENCIES=Yes):
The case to compare against is when COMPUTE_HEADER_DEPENDENCIES is not
set, I'd think, since that is the status quo. And I was talking about
commands like "make clean" that do not care about that feature, not
"make all".
[...]
> With patch, but changed to use git.c instead of ++empty.c:
Did you try with "--help"?
^ permalink raw reply
* Re: [PATCH 7/7] sequencer: Remove sequencer state after final commit
From: Junio C Hamano @ 2011-08-14 21:13 UTC (permalink / raw)
To: Jonathan Nieder
Cc: Ramkumar Ramachandra, Git List, Christian Couder, Daniel Barkalow,
Jeff King
In-Reply-To: <20110814160440.GK18466@elie.gateway.2wire.net>
Jonathan Nieder <jrnieder@gmail.com> writes:
> Ramkumar Ramachandra wrote:
>
>> Since d3f4628e (revert: Remove sequencer state when no commits are
>> pending, 2011-07-06), the sequencer removes the sequencer state before
>> the final commit is actually completed. This design is inherently
>> flawed, as it will not allow the user to abort the sequencer operation
>> at that stage. Instead, make 'git commit' notify the sequencer after
>> every successful commit; the sequencer then removes the state if no
>> more instructions are pending.
>
> Sorry, I'm getting lost in all the words. I suspect you are saying
> “d3f4628e was trying to solve such-and-such problem, but its fix was
> problematic because it removes the data that a hypothetical "git
> cherry-pick --abort" command would need to work. Back out that
> change and adopt the following instead.”
It still is unclear why "removing the sequencer state when no more insns
are pending" is so necessary that the codebase needs to bend backwards to
support that in the first place. What problem is d3f4628e really trying to
solve?
If "who is responsible for removing a stale state" is the issue it tries
to address, Wouldn't it be much cleaner to make "sequencer continue"
notice that state and silently succeed, saying "I am done"?
^ permalink raw reply
* Re: [PATCH 7/7] sequencer: Remove sequencer state after final commit
From: Jonathan Nieder @ 2011-08-14 21:32 UTC (permalink / raw)
To: Junio C Hamano
Cc: Ramkumar Ramachandra, Git List, Christian Couder, Daniel Barkalow,
Jeff King
In-Reply-To: <7vei0nn1cn.fsf@alter.siamese.dyndns.org>
Hi Junio,
Junio C Hamano wrote:
> It still is unclear why "removing the sequencer state when no more insns
> are pending" is so necessary that the codebase needs to bend backwards to
> support that in the first place. What problem is d3f4628e really trying to
> solve?
I believe it is meant to support command sequences such as these:
1.
git cherry-pick foo; # has conflicts
... resolve conflicts and "git add" the resolved files ...
git commit
git cherry-pick bar
2.
git cherry-pick foo bar; # has conflicts applying "bar"
... resolve ...
git commit
git cherry-pick baz
Those were intuitive things to do before the sequencer existed, and if
I understand correctly, d3f4628e was intended to support people and
scripts (such as the test suite) that have these commands wired into
their fingers.
^ permalink raw reply
* Re: [PATCH v3 1/2] rev-parse: add option --is-well-formed-git-dir [path]
From: Junio C Hamano @ 2011-08-14 21:42 UTC (permalink / raw)
To: Heiko Voigt; +Cc: Nguyen Thai Ngoc Duy, Fredrik Gustafsson, git, jens.lehmann
In-Reply-To: <20110813061342.GA459@book.hvoigt.net>
Heiko Voigt <hvoigt@hvoigt.net> writes:
> While we are talking about names how about:
>
> --resolve-git-dir
>
> ? Since we had this information already the option prints out the found
> resolved git directory and could be used for that.
I think we have a winner in the bikeshedding contest.
^ permalink raw reply
* Re: [PATCH 2/6] Access reference caches only through new function get_cached_refs().
From: Junio C Hamano @ 2011-08-14 22:12 UTC (permalink / raw)
To: Michael Haggerty
Cc: git, Junio C Hamano, Jeff King, Drew Northup, Jakub Narebski
In-Reply-To: <1313188589-2330-3-git-send-email-mhagger@alum.mit.edu>
Michael Haggerty <mhagger@alum.mit.edu> writes:
> Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
> ---
> refs.c | 54 +++++++++++++++++++++++++++++++-----------------------
> 1 files changed, 31 insertions(+), 23 deletions(-)
>
> diff --git a/refs.c b/refs.c
> index b0c8308..89840d7 100644
> --- a/refs.c
> +++ b/refs.c
> @@ -181,9 +181,26 @@ static void clear_cached_refs(struct cached_refs *ca)
> ca->did_loose = ca->did_packed = 0;
> }
>
> +/*
> + * Return a pointer to a cached_refs for the specified submodule. For
> + * the main repository, use submodule==NULL. The returned structure
> + * will be allocated and initialized but not necessarily populated; it
> + * should not be freed.
> + */
> +static struct cached_refs *get_cached_refs(const char *submodule)
> +{
> + if (! submodule)
(style) lose the SP before "submodule".
> + return &cached_refs;
> + else {
> + /* For now, don't reuse the refs cache for submodules. */
> + clear_cached_refs(&submodule_refs);
> + return &submodule_refs;
> + }
> +}
> +
> static void invalidate_cached_refs(void)
> {
> - clear_cached_refs(&cached_refs);
> + clear_cached_refs(get_cached_refs(NULL));
> }
>
> static void read_packed_refs(FILE *f, struct cached_refs *cached_refs)
> @@ -234,19 +251,14 @@ void clear_extra_refs(void)
>
> static struct ref_list *get_packed_refs(const char *submodule)
> {
> - const char *packed_refs_file;
> - struct cached_refs *refs;
> + struct cached_refs *refs = get_cached_refs(submodule);
>
> - if (submodule) {
> - packed_refs_file = git_path_submodule(submodule, "packed-refs");
> - refs = &submodule_refs;
> - free_ref_list(refs->packed);
> - } else {
> - packed_refs_file = git_path("packed-refs");
> - refs = &cached_refs;
> - }
> -
> - if (!refs->did_packed || submodule) {
> + if (!refs->did_packed) {
> + const char *packed_refs_file;
> + if (submodule)
> + packed_refs_file = git_path_submodule(submodule, "packed-refs");
> + else
> + packed_refs_file = git_path("packed-refs");
> FILE *f = fopen(packed_refs_file, "r");
decl-after-statement.
> refs->packed = NULL;
> if (f) {
> @@ -361,17 +373,13 @@ void warn_dangling_symref(FILE *fp, const char *msg_fmt, const char *refname)
>
> static struct ref_list *get_loose_refs(const char *submodule)
> {
> - if (submodule) {
> - free_ref_list(submodule_refs.loose);
> - submodule_refs.loose = get_ref_dir(submodule, "refs", NULL);
> - return submodule_refs.loose;
> - }
> + struct cached_refs *refs = get_cached_refs(submodule);
>
> - if (!cached_refs.did_loose) {
> - cached_refs.loose = get_ref_dir(NULL, "refs", NULL);
> - cached_refs.did_loose = 1;
> + if (!refs->did_loose) {
> + refs->loose = get_ref_dir(submodule, "refs", NULL);
> + refs->did_loose = 1;
> }
> - return cached_refs.loose;
> + return refs->loose;
> }
>
> /* We allow "recursive" symbolic refs. Only within reason, though */
^ permalink raw reply
* Re: [PATCH 4/6] Allocate cached_refs objects dynamically
From: Junio C Hamano @ 2011-08-14 22:21 UTC (permalink / raw)
To: Michael Haggerty
Cc: git, Junio C Hamano, Jeff King, Drew Northup, Jakub Narebski
In-Reply-To: <1313188589-2330-5-git-send-email-mhagger@alum.mit.edu>
Michael Haggerty <mhagger@alum.mit.edu> writes:
> Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
> ---
> refs.c | 28 +++++++++++++++++++++-------
> 1 files changed, 21 insertions(+), 7 deletions(-)
>
> diff --git a/refs.c b/refs.c
> index e043555..102ed03 100644
> --- a/refs.c
> +++ b/refs.c
> @@ -157,7 +157,7 @@ static struct cached_refs {
> char did_packed;
> struct ref_list *loose;
> struct ref_list *packed;
> -} cached_refs, submodule_refs;
> +} *cached_refs, *submodule_refs;
> static struct ref_list *current_ref;
>
> static struct ref_list *extra_refs;
> @@ -181,6 +181,15 @@ static void clear_cached_refs(struct cached_refs *ca)
> ca->did_loose = ca->did_packed = 0;
> }
>
> +struct cached_refs *create_cached_refs()
struct cached_refs *create_cached_refs(void)
^ permalink raw reply
* Re: [PATCH 7/7] sequencer: Remove sequencer state after final commit
From: Junio C Hamano @ 2011-08-14 22:30 UTC (permalink / raw)
To: Jonathan Nieder
Cc: Ramkumar Ramachandra, Git List, Christian Couder, Daniel Barkalow,
Jeff King
In-Reply-To: <20110814213200.GA6555@elie.gateway.2wire.net>
Jonathan Nieder <jrnieder@gmail.com> writes:
> I believe it is meant to support command sequences such as these:
>
> 1.
> git cherry-pick foo; # has conflicts
> ... resolve conflicts and "git add" the resolved files ...
> git commit
> git cherry-pick bar
Why does a single commit "cherry-pick foo" leave any sequencer state that
may interfere with the latter to begin with? Isn't that already a bug?
> 2.
> git cherry-pick foo bar; # has conflicts applying "bar"
> ... resolve ...
> git commit
> git cherry-pick baz
>
> Those were intuitive things to do before the sequencer existed, and if
> I understand correctly, d3f4628e was intended to support people and
> scripts (such as the test suite) that have these commands wired into
> their fingers.
Given that the latter was broken when "foo" stopped with conficts (it lost
"bar" altogether anyway), I am not worried about it, and I do not care
much about anybody who had wired such multi-pick into scripts or fingers,
either.
IOW, I do not necessarily agree with your "those were intuitive"
assertion.
^ permalink raw reply
* shallow clone not very shallow due to tags
From: Shawn Pearce @ 2011-08-14 23:58 UTC (permalink / raw)
To: git
$ git clone --depth 1 git://repo.or.cz/alt-git.git tmp_cgit_5
Cloning into tmp_cgit_5...
remote: Counting objects: 20234, done.
remote: Compressing objects: 100% (9454/9454), done.
remote: Total 20234 (delta 16355), reused 13587 (delta 10421)
Receiving objects: 100% (20234/20234), 9.15 MiB | 1.17 MiB/s, done.
Resolving deltas: 100% (16355/16355), done.
$ cd tmp_cgit_5
$ git tag -l | grep 0.99
v0.99
Uhm. That is not a very shallow clone. The clone copied 20234 objects
at 9.15 MiB... so its ~20 MiB lighter than a full clone. But nearly
all of the tags exist, because the clone client is declaring want
lines for them, making the server generate up to 1 commit back from
the wanted tag. I know shallow support is the feature nobody wants to
think about, but this just seems broken to me. Clients performing a
shallow clone shouldn't be asking for tags... but they should be using
the include-tag protocol option so that if they do happen to receive a
tagged commit, the tag object will also be sent.
--
Shawn.
^ permalink raw reply
* Re: [feature wishlist] add commit subcommand to git add -i
From: Jim Cromie @ 2011-08-15 0:43 UTC (permalink / raw)
To: Conrad Irwin; +Cc: git
In-Reply-To: <CAOTq_pvMv6JN_jpQvDsmQTwmMgQK9JzuwXr+VF1T6X4=qf3GsQ@mail.gmail.com>
On Sun, Aug 14, 2011 at 2:48 AM, Conrad Irwin <conrad.irwin@gmail.com> wrote:
> On Sun, Aug 14, 2011 at 1:38 AM, Jim Cromie <jim.cromie@gmail.com> wrote:
>> going further, if git rebase -i had ability to "back" a fixup patch
>> back to where it should have been, and adjust the intervening patches
>> where conflict would normally happen, that would be awesome.
>> Simplistically, this would just shift the patch 1 step back iteratively,
>> until it wouldnt apply properly, and then --abort, stopping at the last
>> clean rebase.
>>
>> apologies if this is too hair-brained, or already done.
>
> It sounds like you're looking for several git commit
> (-p|--interactive) --fixup <commit>, followed by a git rebase -i
> --autosquash. It's not quite as automatic as you describe, but I think
> that automating it would be pretty hard to do correctly.
>
> Conrad
>
it is indeed similar. in the simple case, I know which patch needs the fixup,
and using editor to rearrange the todo-file is straightforward.
using --autosquash requires knowing the commit-log message to be fixed,
and using it to name the fixup commit, which is doable, but I havent trained
myself to do so. I'll give it a try next time..
thanks
Jim
^ permalink raw reply
* Re: [feature wishlist] add commit subcommand to git add -i
From: Jim Cromie @ 2011-08-15 0:44 UTC (permalink / raw)
To: Ramkumar Ramachandra; +Cc: git, Conrad Irwin
In-Reply-To: <CALkWK0=9sT6wDwoa3vDF1bt1e8AiubwW42-o=c++MpzV47LhQg@mail.gmail.com>
On Sun, Aug 14, 2011 at 2:56 AM, Ramkumar Ramachandra
<artagnon@gmail.com> wrote:
> Hi Jim,
>
> Jim Cromie writes:
>> when using git add -i, it would be handy to have a [c]ommit option.
>
> I can't personally comment on this because I use Magit for staging/
> unstaging and committing. It's quite an awesome application- do check
> it out if you use Emacs.
>
I;ll take a look, thanks.
>> going further, if git rebase -i had ability to "back" a fixup patch
>> back to where it should have been, and adjust the intervening patches
>> where conflict would normally happen, that would be awesome.
>> Simplistically, this would just shift the patch 1 step back iteratively,
>> until it wouldnt apply properly, and then --abort, stopping at the last
>> clean rebase.
>
> Hm, I'm not sure if I understand fully: is the idea about moving a
> commit backwards iteratively so we have to resolve several simpler and
> smaller conflicts?
yes - thats certainly part of it.
I added iteratively as a simplification.
I suspect there are more clever ways to do it.
the simplest case is to move a fixup patch backwards where no conflicts arise,
put the fixup right after the patch where the errant code was added.
the harder one is to recognize and resolve the interim conflicts.
To avoid handwaving, I reconstructed my particular case,
which is available on github:
https://github.com/jimc/linux-2.6/tree/rebase-back-usecase
1 - patches 1-11 on dynamic-debug, from Jason Baron, last Thurs.
(these are the ones that gave git am heartburn, fixed in git-next)
2 - Suggestion by Joe Perches to use C-99
I cut pasted his suggestion, made a commit.
d61db7e joes suggestion C-99
3 - 53929c5 drop enabled, check flags&bitmask
4 - 57a9be0 prefer CONFIG_DYNAMIC_DEBUG over DEBUG
5 - fixup, which needs to go back to just after 2.
specifically:
[jimc@groucho linux-2.6]$ git log --oneline -5
07d4a51 fixup C-99, move attribute before assignment
57a9be0 prefer CONFIG_DYNAMIC_DEBUG over DEBUG
53929c5 drop enabled, check flags&bitmask
d61db7e joes suggestion C-99
f06abb5 dynamic_debug: use a single printk() to emit msgs
> I have to admit that I work around this problem by
> running 'rebase -i' several times, moving the commit back in the
> sequence little-by-little.
when I did it manually,
there were a number of conficts to resolve.
pretty minor really, but tedious.
<<<<<<< HEAD
.enabled = false, \
} __used __aligned(8) __attribute__((section("__verbose")))
=======
}
>>>>>>> 07d4a51... fixup C-99, move attribute before assignment
if I could say:
[jimc@groucho linux-2.6]$ git rebase -i HEAD~5
pick f06abb5 dynamic_debug: use a single printk() to emit msgs
pick d61db7e joes suggestion C-99
pick 53929c5 drop enabled, check flags&bitmask
pick 57a9be0 prefer CONFIG_DYNAMIC_DEBUG over DEBUG
*back* 07d4a51 fixup C-99, move attribute before assignment
and get something like (but with diff commit-ids)
[jimc@groucho linux-2.6]$ git log --oneline -5
57a9be0 prefer CONFIG_DYNAMIC_DEBUG over DEBUG
53929c5 drop enabled, check flags&bitmask
07d4a51 fixup C-99, move attribute before assignment
d61db7e joes suggestion C-99
f06abb5 dynamic_debug: use a single printk() to emit msgs
that would be *slick*
I wonder if a 3-way merge is a partial answer to the conflicts that arise?
>
> -- Ram
>
thanks
Jim
^ permalink raw reply
* [PATCH 1/2] xdiff-interface: allow consume function to quit early by returning non-zero
From: Nguyễn Thái Ngọc Duy @ 2011-08-15 2:41 UTC (permalink / raw)
To: git, Junio C Hamano; +Cc: Nguyễn Thái Ngọc Duy
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
combine-diff.c | 9 +++++----
diff.c | 32 +++++++++++++++++++-------------
diffcore-pickaxe.c | 7 ++++---
xdiff-interface.c | 15 ++++++++++-----
xdiff-interface.h | 2 +-
5 files changed, 39 insertions(+), 26 deletions(-)
diff --git a/combine-diff.c b/combine-diff.c
index be67cfc..d99e1c6 100644
--- a/combine-diff.c
+++ b/combine-diff.c
@@ -163,14 +163,14 @@ struct combine_diff_state {
struct sline *lost_bucket;
};
-static void consume_line(void *state_, char *line, unsigned long len)
+static int consume_line(void *state_, char *line, unsigned long len)
{
struct combine_diff_state *state = state_;
if (5 < len && !memcmp("@@ -", line, 4)) {
if (parse_hunk_header(line, len,
&state->ob, &state->on,
&state->nb, &state->nn))
- return;
+ return 0;
state->lno = state->nb;
if (state->nn == 0) {
/* @@ -X,Y +N,0 @@ removed Y lines
@@ -194,10 +194,10 @@ static void consume_line(void *state_, char *line, unsigned long len)
sizeof(unsigned long));
state->sline[state->nb-1].p_lno[state->n] = state->ob;
state->lost_bucket->next_lost = state->lost_bucket->lost_head;
- return;
+ return 0;
}
if (!state->lost_bucket)
- return; /* not in any hunk yet */
+ return 0; /* not in any hunk yet */
switch (line[0]) {
case '-':
append_lost(state->lost_bucket, state->n, line+1, len-1);
@@ -207,6 +207,7 @@ static void consume_line(void *state_, char *line, unsigned long len)
state->lno++;
break;
}
+ return 0;
}
static void combine_diff(const unsigned char *parent, unsigned int mode,
diff --git a/diff.c b/diff.c
index 93ef9a2..2801204 100644
--- a/diff.c
+++ b/diff.c
@@ -788,7 +788,7 @@ static int color_words_output_graph_prefix(struct diff_words_data *diff_words)
}
}
-static void fn_out_diff_words_aux(void *priv, char *line, unsigned long len)
+static int fn_out_diff_words_aux(void *priv, char *line, unsigned long len)
{
struct diff_words_data *diff_words = priv;
struct diff_words_style *style = diff_words->style;
@@ -800,7 +800,7 @@ static void fn_out_diff_words_aux(void *priv, char *line, unsigned long len)
if (line[0] != '@' || parse_hunk_header(line, len,
&minus_first, &minus_len, &plus_first, &plus_len))
- return;
+ return 0;
assert(opt);
if (opt->output_prefix) {
@@ -849,6 +849,7 @@ static void fn_out_diff_words_aux(void *priv, char *line, unsigned long len)
diff_words->current_plus = plus_end;
diff_words->last_minus = minus_first;
+ return 0;
}
/* This function starts looking at *begin, and returns 0 iff a word was found. */
@@ -1042,7 +1043,7 @@ static void find_lno(const char *line, struct emit_callback *ecbdata)
ecbdata->lno_in_postimage = strtol(p + 1, NULL, 10);
}
-static void fn_out_consume(void *priv, char *line, unsigned long len)
+static int fn_out_consume(void *priv, char *line, unsigned long len)
{
struct emit_callback *ecbdata = priv;
const char *meta = diff_get_color(ecbdata->color_diff, DIFF_METAINFO);
@@ -1091,7 +1092,7 @@ static void fn_out_consume(void *priv, char *line, unsigned long len)
emit_hunk_header(ecbdata, line, len);
if (line[len-1] != '\n')
putc('\n', ecbdata->opt->file);
- return;
+ return 0;
}
if (len < 1) {
@@ -1099,18 +1100,18 @@ static void fn_out_consume(void *priv, char *line, unsigned long len)
if (ecbdata->diff_words
&& ecbdata->diff_words->type == DIFF_WORDS_PORCELAIN)
fputs("~\n", ecbdata->opt->file);
- return;
+ return 0;
}
if (ecbdata->diff_words) {
if (line[0] == '-') {
diff_words_append(line, len,
&ecbdata->diff_words->minus);
- return;
+ return 0;
} else if (line[0] == '+') {
diff_words_append(line, len,
&ecbdata->diff_words->plus);
- return;
+ return 0;
}
diff_words_flush(ecbdata);
if (ecbdata->diff_words->type == DIFF_WORDS_PORCELAIN) {
@@ -1128,7 +1129,7 @@ static void fn_out_consume(void *priv, char *line, unsigned long len)
}
emit_line(ecbdata->opt, plain, reset, line, len);
}
- return;
+ return 0;
}
if (line[0] != '+') {
@@ -1143,6 +1144,8 @@ static void fn_out_consume(void *priv, char *line, unsigned long len)
ecbdata->lno_in_postimage++;
emit_add_line(reset, ecbdata, line + 1, len - 1);
}
+
+ return 0;
}
static char *pprint_rename(const char *a, const char *b)
@@ -1250,7 +1253,7 @@ static struct diffstat_file *diffstat_add(struct diffstat_t *diffstat,
return x;
}
-static void diffstat_consume(void *priv, char *line, unsigned long len)
+static int diffstat_consume(void *priv, char *line, unsigned long len)
{
struct diffstat_t *diffstat = priv;
struct diffstat_file *x = diffstat->files[diffstat->nr - 1];
@@ -1259,6 +1262,7 @@ static void diffstat_consume(void *priv, char *line, unsigned long len)
x->added++;
else if (line[0] == '-')
x->deleted++;
+ return 0;
}
const char mime_boundary_leader[] = "------------";
@@ -1805,7 +1809,7 @@ static int is_conflict_marker(const char *line, int marker_size, unsigned long l
return 1;
}
-static void checkdiff_consume(void *priv, char *line, unsigned long len)
+static int checkdiff_consume(void *priv, char *line, unsigned long len)
{
struct checkdiff_t *data = priv;
int color_diff = DIFF_OPT_TST(data->o, COLOR_DIFF);
@@ -1835,7 +1839,7 @@ static void checkdiff_consume(void *priv, char *line, unsigned long len)
}
bad = ws_check(line + 1, len - 1, data->ws_rule);
if (!bad)
- return;
+ return 0;
data->status |= bad;
err = whitespace_error_string(bad);
fprintf(data->o->file, "%s%s:%d: %s.\n",
@@ -1853,6 +1857,7 @@ static void checkdiff_consume(void *priv, char *line, unsigned long len)
else
die("invalid diff");
}
+ return 0;
}
static unsigned char *deflate_it(char *data,
@@ -4008,19 +4013,20 @@ static int remove_space(char *line, int len)
return dst - line;
}
-static void patch_id_consume(void *priv, char *line, unsigned long len)
+static int patch_id_consume(void *priv, char *line, unsigned long len)
{
struct patch_id_t *data = priv;
int new_len;
/* Ignore line numbers when computing the SHA1 of the patch */
if (!prefixcmp(line, "@@ -"))
- return;
+ return 0;
new_len = remove_space(line, len);
git_SHA1_Update(data->ctx, line, new_len);
data->patchlen += new_len;
+ return 0;
}
/* returns 0 upon success, and writes result into sha1 */
diff --git a/diffcore-pickaxe.c b/diffcore-pickaxe.c
index ea03b91..12811b9 100644
--- a/diffcore-pickaxe.c
+++ b/diffcore-pickaxe.c
@@ -12,25 +12,26 @@ struct diffgrep_cb {
int hit;
};
-static void diffgrep_consume(void *priv, char *line, unsigned long len)
+static int diffgrep_consume(void *priv, char *line, unsigned long len)
{
struct diffgrep_cb *data = priv;
regmatch_t regmatch;
int hold;
if (line[0] != '+' && line[0] != '-')
- return;
+ return 0;
if (data->hit)
/*
* NEEDSWORK: we should have a way to terminate the
* caller early.
*/
- return;
+ return 0;
/* Yuck -- line ought to be "const char *"! */
hold = line[len];
line[len] = '\0';
data->hit = !regexec(data->regexp, line + 1, 1, ®match, 0);
line[len] = hold;
+ return 0;
}
static void fill_one(struct diff_filespec *one,
diff --git a/xdiff-interface.c b/xdiff-interface.c
index 0e2c169..c5684b4 100644
--- a/xdiff-interface.c
+++ b/xdiff-interface.c
@@ -56,7 +56,7 @@ int parse_hunk_header(char *line, int len,
return -!!memcmp(cp, " @@", 3);
}
-static void consume_one(void *priv_, char *s, unsigned long size)
+static int consume_one(void *priv_, char *s, unsigned long size)
{
struct xdiff_emit_state *priv = priv_;
char *ep;
@@ -64,10 +64,12 @@ static void consume_one(void *priv_, char *s, unsigned long size)
unsigned long this_size;
ep = memchr(s, '\n', size);
this_size = (ep == NULL) ? size : (ep - s + 1);
- priv->consume(priv->consume_callback_data, s, this_size);
+ if (priv->consume(priv->consume_callback_data, s, this_size))
+ return -1;
size -= this_size;
s += this_size;
}
+ return 0;
}
static int xdiff_outf(void *priv_, mmbuffer_t *mb, int nbuf)
@@ -84,15 +86,18 @@ static int xdiff_outf(void *priv_, mmbuffer_t *mb, int nbuf)
/* we have a complete line */
if (!priv->remainder.len) {
- consume_one(priv, mb[i].ptr, mb[i].size);
+ if (consume_one(priv, mb[i].ptr, mb[i].size))
+ return -1;
continue;
}
strbuf_add(&priv->remainder, mb[i].ptr, mb[i].size);
- consume_one(priv, priv->remainder.buf, priv->remainder.len);
+ if (consume_one(priv, priv->remainder.buf, priv->remainder.len))
+ return -1;
strbuf_reset(&priv->remainder);
}
if (priv->remainder.len) {
- consume_one(priv, priv->remainder.buf, priv->remainder.len);
+ if (consume_one(priv, priv->remainder.buf, priv->remainder.len))
+ return -1;
strbuf_reset(&priv->remainder);
}
return 0;
diff --git a/xdiff-interface.h b/xdiff-interface.h
index 49d1116..b7aaa0e 100644
--- a/xdiff-interface.h
+++ b/xdiff-interface.h
@@ -3,7 +3,7 @@
#include "xdiff/xdiff.h"
-typedef void (*xdiff_emit_consume_fn)(void *, char *, unsigned long);
+typedef int (*xdiff_emit_consume_fn)(void *, char *, unsigned long);
typedef void (*xdiff_emit_hunk_consume_fn)(void *, long, long, long);
int xdi_diff(mmfile_t *mf1, mmfile_t *mf2, xpparam_t const *xpp, xdemitconf_t const *xecfg, xdemitcb_t *ecb);
--
1.7.4.74.g639db
^ permalink raw reply related
* [PATCH 2/2] diffcore-pickaxe: terminate grepping as soon as it hits
From: Nguyễn Thái Ngọc Duy @ 2011-08-15 2:41 UTC (permalink / raw)
To: git, Junio C Hamano; +Cc: Nguyễn Thái Ngọc Duy
In-Reply-To: <1313376083-24713-1-git-send-email-pclouds@gmail.com>
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
diffcore-pickaxe.c | 6 +-----
1 files changed, 1 insertions(+), 5 deletions(-)
diff --git a/diffcore-pickaxe.c b/diffcore-pickaxe.c
index 12811b9..e028dd5 100644
--- a/diffcore-pickaxe.c
+++ b/diffcore-pickaxe.c
@@ -21,11 +21,7 @@ static int diffgrep_consume(void *priv, char *line, unsigned long len)
if (line[0] != '+' && line[0] != '-')
return 0;
if (data->hit)
- /*
- * NEEDSWORK: we should have a way to terminate the
- * caller early.
- */
- return 0;
+ return -1;
/* Yuck -- line ought to be "const char *"! */
hold = line[len];
line[len] = '\0';
--
1.7.4.74.g639db
^ permalink raw reply related
* [PATCH] Configurable diff context in merge-tree command
From: Erik van Zijst @ 2011-08-15 6:17 UTC (permalink / raw)
To: git
Added support for the -U/--unified command line arguments to the
git-merge-tree command to change the number of context lines that
are generated.
Signed-off-by: Erik van Zijst <erik.van.zijst@gmail.com>
---
Documentation/git-merge-tree.txt | 8 +++++++-
builtin/merge-tree.c | 26 +++++++++++++++++++-------
2 files changed, 26 insertions(+), 8 deletions(-)
diff --git a/Documentation/git-merge-tree.txt b/Documentation/git-merge-tree.txt
index c5f84b6..7a0d927 100644
--- a/Documentation/git-merge-tree.txt
+++ b/Documentation/git-merge-tree.txt
@@ -9,7 +9,7 @@ git-merge-tree - Show three-way merge without touching index
SYNOPSIS
--------
[verse]
-'git merge-tree' <base-tree> <branch1> <branch2>
+'git merge-tree' [-U<n> | --unified=<n>] <base-tree> <branch1> <branch2>
DESCRIPTION
-----------
@@ -24,6 +24,12 @@ merge results outside of the index, and stuff the
results back into the
index. For this reason, the output from the command omits
entries that match the <branch1> tree.
+OPTIONS
+-------
+-U<n>::
+--unified=<n>::
+ Generate diffs with <n> lines of context instead of the usual three.
+
GIT
---
Part of the linkgit:git[1] suite
diff --git a/builtin/merge-tree.c b/builtin/merge-tree.c
index 897a563..0a5c8c9 100644
--- a/builtin/merge-tree.c
+++ b/builtin/merge-tree.c
@@ -4,9 +4,15 @@
#include "blob.h"
#include "exec_cmd.h"
#include "merge-file.h"
+#include "parse-options.h"
+
+static const char * const merge_tree_usage[] = {
+ "git merge-tree [options] <base-tree> <branch1> <branch2>",
+ NULL
+};
-static const char merge_tree_usage[] = "git merge-tree <base-tree>
<branch1> <branch2>";
static int resolve_directories = 1;
+static long ctxlen = 3;
struct merge_list {
struct merge_list *next;
@@ -108,7 +114,7 @@ static void show_diff(struct merge_list *entry)
xpp.flags = 0;
memset(&xecfg, 0, sizeof(xecfg));
- xecfg.ctxlen = 3;
+ xecfg.ctxlen = ctxlen;
ecb.outf = show_outf;
ecb.priv = NULL;
@@ -341,13 +347,19 @@ int cmd_merge_tree(int argc, const char **argv,
const char *prefix)
{
struct tree_desc t[3];
void *buf1, *buf2, *buf3;
+ const struct option opts[] = {
+ OPT_INTEGER('U', "unified", &ctxlen, "number of diff context lines"),
+ OPT_END()
+ };
+
+ argc = parse_options(argc, argv, prefix, opts, merge_tree_usage, 0);
- if (argc != 4)
- usage(merge_tree_usage);
+ if (argc != 3)
+ usage(*merge_tree_usage);
- buf1 = get_tree_descriptor(t+0, argv[1]);
- buf2 = get_tree_descriptor(t+1, argv[2]);
- buf3 = get_tree_descriptor(t+2, argv[3]);
+ buf1 = get_tree_descriptor(t+0, argv[0]);
+ buf2 = get_tree_descriptor(t+1, argv[1]);
+ buf3 = get_tree_descriptor(t+2, argv[2]);
merge_trees(t, "");
free(buf1);
free(buf2);
--
1.7.1
^ permalink raw reply related
* Re: Bug report: git log --[num|short]stat sometimes counts lines wrong
From: Alexander Pepper @ 2011-08-15 8:24 UTC (permalink / raw)
To: git
In-Reply-To: <45CC44BC-03FF-4C5F-97B7-7ED03CB68BC2@inf.fu-berlin.de>
Am 12.08.2011 um 17:21 schrieb Alexander Pepper:
> Hi there.
> This is my first contribution to git (if you count a bug report as a contribution) and I'm not really familiar where to report bugs. In the irc channel #git at freenode somebody pointed me to this mailing list.
>
> First of: I'm running git version 1.7.6 on OS X 10.6.8.
>
> Let me describe what I observed.
> repository: https://github.com/voldemort/voldemort.git
> The command "git log --numstat c21ad764ea1bae7f7bd83b5e2cb015dcbc44d586" shows for the commit c21ad764 and file '.../readonly/mr/HadoopStoreBuilderReducer.java' 25 lines added and 22 lines removed. But the patch of HadoopStoreBuilderReducer.java that I get with "git show c21ad764ea1bae7f7bd83b5e2cb015dcbc44d586 -- contrib/hadoop-store-builder/src/java/voldemort/store/readonly/mr/HadoopStoreBuilderReducer.java" adds 30 lines and removes 27.
>
> Why does "git log --numstat" drops 5 added lines and 5 removed lines? This also holds true for "git log --stat" and "git log --shortstat".
>
> Is this a bug or am I missing an option to git log or git show?
>
> More commits where I observed this problem on the same repository:
> 7e00fb6d2cf131dfed59c180f2171952808cc336 src/java/voldemort/client/rebalance/MigratePartitions.java
> 78ad6f2a6ea327dbae2110f4530a5bd07e5deaac src/java/voldemort/client/rebalance/MigratePartitions.java (same commit on another branch)
> 7871933f0f0f056e2eeac03a01db1e9cf81f8bda src/java/voldemort/client/protocol/admin/AdminClient.java
> 2d6f68b09c3bdc23dcf3ae1f91c9285fbd668820 src/java/voldemort/store/readonly/ExternalSorter.java
> 6fcacee866307ec34eb32b268e2c2b885a949319 build.xml
>
> Greetings from Berlin
> Alex
Hello again.
I also observed this behavior with git version 1.7.4.1 on Gentoo.
Any ideas how to fix this?
Greetings from Berlin
Alex
^ permalink raw reply
* diff --stat when chmoding binary files
From: Martin Mares @ 2011-08-15 8:50 UTC (permalink / raw)
To: git; +Cc: Tomáš Maleček
Hello, world!\n
I have discovered somewhat surprising behavior of `git diff --stat'
on files, which are not modified except for their executable bits.
When the file is binary, it is listed in the diffstat, otherwise
it is not.
For example:
| mj@albireo:/tmp$ mkdir gt
| mj@albireo:/tmp$ cd gt
| mj@albireo:/tmp/gt$ git init
| Initialized empty Git repository in /tmp/gt/.git/
| mj@albireo:/tmp/gt$ echo abcdef >a
| mj@albireo:/tmp/gt$ dd if=/dev/zero of=b bs=1024 count=1
| 1+0 records in
| 1+0 records out
| 1024 bytes (1.0 kB) copied, 7.3243e-05 s, 14.0 MB/s
| mj@albireo:/tmp/gt$ git add *
| mj@albireo:/tmp/gt$ git commit -m 'adding'
| [master (root-commit) 295d5f9] adding
| 2 files changed, 1 insertions(+), 0 deletions(-)
| create mode 100644 a
| create mode 100644 b
| mj@albireo:/tmp/gt$ chmod 755 *
| mj@albireo:/tmp/gt$ git add *
| mj@albireo:/tmp/gt$ git commit -m 'chmoding'
| [master 61c251b] chmoding
| 1 files changed, 0 insertions(+), 0 deletions(-)
| mode change 100644 => 100755 a
| mode change 100644 => 100755 b
| mj@albireo:/tmp/gt$ git diff HEAD^
| diff --git a/a b/a
| old mode 100644
| new mode 100755
| diff --git a/b b/b
| old mode 100644
| new mode 100755
| mj@albireo:/tmp/gt$ git diff --stat HEAD^
| b | Bin 1024 -> 1024 bytes
| 1 files changed, 0 insertions(+), 0 deletions(-)
This happens in git-1.7.6 and in several older versions as well.
Am I missing something or is it a bug?
Have a nice fortnight
--
Martin `MJ' Mares <mj@ucw.cz> http://mj.ucw.cz/
Faculty of Math and Physics, Charles University, Prague, Czech Rep., Earth
Q: Who invented the first airplane that did not fly? A: The Wrong Brothers.
^ permalink raw reply
* Re: Suggestions to make git easier to understand
From: Philippe Vaucher @ 2011-08-15 10:15 UTC (permalink / raw)
To: Jeff King; +Cc: Jonathan Nieder, git, Rafael Magana
In-Reply-To: <20110812222626.GA7079@sigill.intra.peff.net>
> Didn't we fix this already in 8009d83 (Better "Changed but not updated"
> message in git-status, 2010-11-02)? Since v1.7.4, "git status" has
> "Changes not staged for commit".
Yes, and the new text is fine for me.
Philippe
^ permalink raw reply
* Re: [feature wishlist] add commit subcommand to git add -i
From: Thomas Rast @ 2011-08-15 12:54 UTC (permalink / raw)
To: Jim Cromie; +Cc: git
In-Reply-To: <CAJfuBxwW8Dyp8FTS13uPOBKZGL9JOEqaSOhGN+zBJ_8BHpJE3g@mail.gmail.com>
Jim Cromie wrote:
> [f]ragment would also be handy, which would break each chunk of a diff
> into a separate commit, with the summary line provided automatically
> <file> @@ -696,7 +692,7 @@ int foo ...
>
> This would help a bit with random cleanups, since rebase -i could then
> be used to
> reorder and recombine the fragments, and edit the commit messages afterwards.
>
> going further, if git rebase -i had ability to "back" a fixup patch
> back to where it should have been
This little script may be of interest to you:
http://article.gmane.org/gmane.comp.version-control.git/163621
--
Thomas Rast
trast@{inf,student}.ethz.ch
^ permalink raw reply
* Re: shallow clone not very shallow due to tags
From: Nguyen Thai Ngoc Duy @ 2011-08-15 14:03 UTC (permalink / raw)
To: Shawn Pearce; +Cc: git
In-Reply-To: <CAJo=hJuyZMj+qwFr_=stbQtGh2SCCpjfsBxm+2wbfJK=i_VTdw@mail.gmail.com>
On Mon, Aug 15, 2011 at 6:58 AM, Shawn Pearce <spearce@spearce.org> wrote:
> Uhm. That is not a very shallow clone. The clone copied 20234 objects
> at 9.15 MiB... so its ~20 MiB lighter than a full clone. But nearly
> all of the tags exist, because the clone client is declaring want
> lines for them, making the server generate up to 1 commit back from
> the wanted tag. I know shallow support is the feature nobody wants to
> think about, but this just seems broken to me. Clients performing a
> shallow clone shouldn't be asking for tags... but they should be using
> the include-tag protocol option so that if they do happen to receive a
> tagged commit, the tag object will also be sent.
The same would apply if the repo in question has many branches. Should
we fetch only master (or a user-specified set of refs) in shallow
clone?
--
Duy
^ 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