* [RFC 2/2] Make misuse of get_pathname() buffers detectable by valgrind
From: Michael Haggerty @ 2011-09-27 4:28 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Petr Baudis, Michael Haggerty
In-Reply-To: <1317097687-11098-1-git-send-email-mhagger@alum.mit.edu>
A temporary buffer produced by get_pathname() is recycled after a few
subsequent calls of get_pathname(). The use of such a buffer after it
has been recycled can result in the wrong file being accessed with
very strange effects. Moreover, such a bug can lie dormant until code
elsewhere is changed to use a temporary buffer, causing very
mysterious, nonlocal failures that are hard to analyze.
Add a second implementation of get_pathname() (activated if the
VALGRIND preprocessor macro is defined) that allocates and frees
buffers instead of recycling statically-allocated buffers. This does
not make the problem less serious, but it turns the errors into
access-after-free errors, making it possible to locate the guilty code
using valgrind.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
I believe that it is frowned upon to use #ifdefs in git code, but no
good alternative is obvious to me for this type of use. Suggestions
are welcome.
I would also welcome suggestions for a better name than "VALGRIND" for
the preprocessor macro. Are there standard names used elsewhere in
git for such purposes?
path.c | 40 ++++++++++++++++++++++++++++++++++++++--
1 files changed, 38 insertions(+), 2 deletions(-)
diff --git path.c path.c
index 6c4714d..3021207 100644
--- path.c
+++ path.c
@@ -9,6 +9,20 @@
* f = open(mkpath("%s/%s.git", base, name), O_RDONLY);
*
* which is what it's designed for.
+ *
+ * The temporary buffers returned by these functions will be clobbered
+ * by later calls to these functions. Therefore it is important not
+ * to expect such buffers to keep their values across calls to other
+ * git functions. Violations of this rule can cause the original
+ * buffer to be overwritten and lead to very confusing, nonlocal bugs,
+ * including data loss (you think you are writing to your file but are
+ * actually writing to a filename created by some other caller).
+ *
+ * If the VALGRIND preprocessor macro is defined, then buffers are
+ * created via xmalloc and old temporary buffers are recycled using
+ * free(). This changes the symptom of abuse of the buffers from
+ * mysterious, random errors into access-after-free errors that are
+ * detectable by valgrind.
*/
#include "cache.h"
#include "strbuf.h"
@@ -17,12 +31,34 @@
#define PATHNAME_BUFFER_COUNT (1 << 2)
static char bad_path[] = "/bad-path/";
+#ifdef VALGRIND
+static char buggy_path[] = "/git-internal-error/";
+#endif
static char *get_pathname(void)
{
- static char pathname_array[PATHNAME_BUFFER_COUNT][PATH_MAX];
static int index;
- return pathname_array[(PATHNAME_BUFFER_COUNT - 1) & ++index];
+#ifdef VALGRIND
+ static char *pathname_array[PATHNAME_BUFFER_COUNT];
+ index = (index + 1) & (PATHNAME_BUFFER_COUNT - 1);
+ if (pathname_array[index]) {
+ /*
+ * In a correct program, this will have no effect, but
+ * *if* somebody erroneously uses this buffer after it
+ * has been freed, it gives more of a chance that the
+ * error will be detected even if valgrind is not
+ * running:
+ */
+ strcpy(pathname_array[index], buggy_path);
+
+ free(pathname_array[index]);
+ }
+ pathname_array[index] = xmalloc(PATH_MAX);
+ return pathname_array[index];
+#else
+ static char pathname_array[PATHNAME_BUFFER_COUNT][PATH_MAX];
+ return pathname_array[(PATHNAME_BUFFER_COUNT - 1) & ++index];
+#endif
}
static char *cleanup_path(char *path)
--
1.7.7.rc2
^ permalink raw reply
* [RFC 1/2] Make the number of pathname buffers a compile-time constant
From: Michael Haggerty @ 2011-09-27 4:28 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Petr Baudis, Michael Haggerty
In-Reply-To: <1317097687-11098-1-git-send-email-mhagger@alum.mit.edu>
Changing the number of available temporary buffers for get_pathname()
can help diagnose abuse of such buffers. If a bug goes away when
PATHNAME_BUFFER_COUNT is increased, then it might be due to a buffer
being used after it has been recycled. Similarly, if a bug appears
when PATHNAME_BUFFER_COUNT is decreased, then there might be a latent
problem that could emerge after unrelated code changes.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
path.c | 7 +++++--
1 files changed, 5 insertions(+), 2 deletions(-)
diff --git path.c path.c
index 6f3f5d5..6c4714d 100644
--- path.c
+++ path.c
@@ -13,13 +13,16 @@
#include "cache.h"
#include "strbuf.h"
+/* This must be a power of 2: */
+#define PATHNAME_BUFFER_COUNT (1 << 2)
+
static char bad_path[] = "/bad-path/";
static char *get_pathname(void)
{
- static char pathname_array[4][PATH_MAX];
+ static char pathname_array[PATHNAME_BUFFER_COUNT][PATH_MAX];
static int index;
- return pathname_array[3 & ++index];
+ return pathname_array[(PATHNAME_BUFFER_COUNT - 1) & ++index];
}
static char *cleanup_path(char *path)
--
1.7.7.rc2
^ permalink raw reply
* [RFC 0/2] Debugging tools for get_pathname()
From: Michael Haggerty @ 2011-09-27 4:28 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Petr Baudis, Michael Haggerty
I just wasted a bunch of time diagnosing a problem that was caused by
other code's misuse of a temporary buffer created by get_pathname()
(patch forthcoming). The bug was triggered by totally unrelated (and
innocent) code changes that I had made. I found it quite difficult to
figure out the source of the problem.
It is probably obvious to everybody else, but here are the two tricks
that finally enabled me to diagnose the problem:
1. I increased the number of temporary buffers available to
get_pathname(). This made the test suite failure go away, which
was a good indication that the problem was related to the buffers.
2. I changed get_pathname() to allocate and free the buffers instead
of using statically-allocated buffers. This made it trivial to
find the offender using valgrind.
This patch series make it easier to apply these tricks. I hope it
helps somebody else avoid the pain that I just experienced.
Michael Haggerty (2):
Make the number of pathname buffers a compile-time constant
Make misuse of get_pathname() buffers detectable by valgrind
path.c | 43 +++++++++++++++++++++++++++++++++++++++++--
1 files changed, 41 insertions(+), 2 deletions(-)
--
1.7.7.rc2
^ permalink raw reply
* Re: [BUG?] git fetch -p -t prunes all non-tag refs
From: Jeff King @ 2011-09-27 3:31 UTC (permalink / raw)
To: Carlos Martín Nieto; +Cc: Junio C Hamano, mathstuf, git
In-Reply-To: <1317079692.5579.19.camel@centaur.lab.cmartin.tk>
On Tue, Sep 27, 2011 at 01:28:09AM +0200, Carlos Martín Nieto wrote:
> > term I think we should fix it properly. We are already learning "what are
> > the refs the remote side currently has" from the transport and the right
> > fix ought to be to use that original information, not the version filtered
> > for the use of the primary objective of fetch, which is to only fetch what
> > the user asked for.
>
> Do you mean that we should ignore the refspec? Or do you mean that we
> should look at the refspec if it exists, and only consider deleting
> those that meet the refspec, so that `--prune --tags` would only delete
> tags that don't exist in the remote?
The latter. If I say:
git fetch --prune origin refs/heads/master:refs/remotes/origin/master
and refs/heads/master doesn't exist on the remote, I would expect
refs/remotes/origin/master to be deleted locally. And that naturally
extends to:
git fetch --prune origin refs/heads/*:refs/remotes/origin/*
We do something similar with "git push --mirror", which does pruning
like this[1].
-Peff
[1] Actually, I'm not sure how correct "push --mirror" is. It would be
nice if the prune operation could be split from the mirror, too. In
the past, I have wanted to do both:
# backup to a repository where our objects will be shared
# with other related backups. So we must only use our slice of the
# ref namespace.
git push --mirror backup-repo +refs/*:refs/`hostname`/*
and:
# update topic branches we have already published (using the
# "matching" refspec), but remove any that we have deleted
# locally.
git push --mirror publish-point +:
and I don't think either works.
^ permalink raw reply
* Re: [PATCH] git-submodule: a small fix
From: Andrew Ardill @ 2011-09-27 3:22 UTC (permalink / raw)
To: Roy Liu; +Cc: git
In-Reply-To: <CAOOg04z0UjOQCBMTuii37y3ykNU17hDTcD9E81C+r1whFpMaCg@mail.gmail.com>
On 27 September 2011 08:00, Roy Liu <carsomyr@gmail.com> wrote:
> In git-submodule.sh, the "url" variable may contain a stale value from
> the previous loop iteration, so clear it.
>
> --- git-submodule.sh.orig 2011-09-26 17:50:45.000000000 -0400
> +++ git-submodule.sh 2011-09-26 17:51:18.000000000 -0400
> @@ -370,6 +370,8 @@
> esac
> git config submodule."$name".url "$url" ||
> die "Failed to register url for submodule path '$path'"
> + else
> + url=""
> fi
>
> # Copy "update" setting when it is not set yet
Perhaps a better commit description would be:
git-submodule: clear the url variable when not set to avoid using stale values
otherwise looks clean enough
Andrew
^ permalink raw reply
* Re: Git is not scalable with too many refs/*
From: Martin Fick @ 2011-09-27 2:34 UTC (permalink / raw)
To: Julian Phillips; +Cc: git
In-Reply-To: <97c45128ddeb8269273a4431b3941478@quantumfyre.co.uk>
> Julian Phillips <julian@quantumfyre.co.uk> wrote:
>On Mon, 26 Sep 2011 18:12:31 -0600, Martin Fick wrote:
>That sounds a lot better. Hopefully other commands should be faster
>now too.
Yeah, I will try this in a few other places to see.
>> Thanks way much!!!
>
>No problem. Thank you for all the time you've put in to help chase
>this down. Makes it so much easier when the person with original
>problem mucks in with the investigation.
>Just think how much time you've saved for anyone with a large number of
>
>those Gerrit change refs ;)
Perhaps this is a naive question, but why are all these refs being put into a list to be sorted, only to be discarded soon thereafter anyway? After all, git branch knows that it isn't going to print these, and the refs are stored precategorized, so why not only grab the refs which matter upfront?
-Martin
^ permalink raw reply
* Re: [PATCH] git-remote rename should match whole string when renaming remote ref directory
From: Benny Halevy @ 2011-09-27 2:45 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Martin von Zweigbergk, git
In-Reply-To: <7v62kf41ud.fsf@alter.siamese.dyndns.org>
On 2011-09-26 21:04, Junio C Hamano wrote:
> Benny Halevy <benny@tonian.com> writes:
>
>> From: Benny Halevy <bhalevy@tonian.com>
>>
>> Otherwise, with two remotes: test, test-2
>> git remote rename test test-
>> ends up with:
>> .git/refs/remotes/test-
>> .git/refs/remotes/test--2
>> ...
>> diff --git a/builtin/remote.c b/builtin/remote.c
>> index f2a9c26..5443e71 100644
>> --- a/builtin/remote.c
>> +++ b/builtin/remote.c
>> @@ -571,7 +571,7 @@ static int read_remote_branches(const char *refname,
>> const char *symref;
>>
>> strbuf_addf(&buf, "refs/remotes/%s", rename->old);
>> - if (!prefixcmp(refname, buf.buf)) {
>> + if (!strcmp(refname, buf.buf)) {
>
> At this point of the code, refname has "refs/remotes/test/foo" and it is
> queued to later rename it to "refs/remotes/test-/foo" (the next invocation
> of this function will see "refs/remotes/test/bar" in refname). And the
> strbuf buf.buf has "refs/remotes/test"; your !strcmp(refname, buf.buf)
> would never trigger, I suspect.
>
> Isn't 60e5eee (remote: "rename o foo" should not rename ref "origin/bar",
> 2011-09-01) the correct fix for this issue? It makes buf.buf properly
> terminated with a slash, to contain "refs/remotes/test/" so that prefixcmp
> properly matches it with "refs/remotes/test/foo" but not with refs under
> other hierarchies like "refs/remotes/test-2/anything".
OK, 60e5eee solves the problem too.
I wasn't aware of it as I was looking at the master branch.
FWIW, here's the test I used:
#!/bin/sh
git=git
cwd=$(pwd)
function fail ()
{
echo $0: $*
exit 1
}
for i in main test test-2; do
mkdir $i || fail $i exists;
$git init $i || fail git init $i failed
echo $i > $i/foo
( cd $i; git add foo; git commit -m $i )
done
cd main || fail cd main failed
for i in test test-2; do
$git remote add $i file://$cwd/$i || fail git remote add $i failed
done
$git remote update || fail git remote update fail
$git remote rename test test-
$git show test-2/master || fail FAILED
echo PASSED
>
> Thanks.
^ permalink raw reply
* Re: config-file includes
From: Jeff King @ 2011-09-27 2:38 UTC (permalink / raw)
To: Jay Soffian
Cc: David Aguilar, Nguyen Thai Ngoc Duy, git, Michael Haggerty,
Junio C Hamano, Jakub Narebski
In-Reply-To: <CAG+J_Dw3B0qReTevph725sPatpKDzikC=W0XTvKo4GsYLVcL4w@mail.gmail.com>
On Mon, Sep 26, 2011 at 10:13:44PM -0400, Jay Soffian wrote:
> > We could allow arbitrary shell code like:
> >
> > [include-if "test `uname -s` -eq Darwin"]
> >
> > Very flexible, though it makes me think we are getting a little
> > overboard. And it's an extra shell invocation whenever we read the
> > config, which is ugly.
>
> I would think git could just learn a few useful defines at the time of
> compile, such as e.g. OS_Darwin would be more than sufficient.
Yeah, I think that is probably more sane. We don't collect the
information now, but it probably wouldn't be that hard (at the very
worst, it would probably just involve running "uname" at build time).
> I can also give you another use use case. I keep all my work repos
> under ~Work/ and I want my user.email on all those to be my work email
> addy, and all other repos on my system I want to use my personal email
> address. So my ~/.gitconfig has my personal email address and then 99%
> of the time I forget to configure the repos under ~/Work correctly.
> That said, I'm not sure how the config include would help this...
You would need to provide git with some condition about which address
should be used. It sounds like the repo directory is the best bet. So
maybe something like:
[include-ifrepo "/home/jsoffian/Work/*"]
or something. Maybe that's too gross. I dunno.
FWIW, this hack would work even with current git:
cat >bin/git <<\EOF
#!/bin/sh
case "`git rev-parse --show-toplevel`" in
"$HOME/Work/*")
set -- -c user.email=whatever "$@"
;;
esac
exec /path/to/real/git "$@"
EOF
which would actually work with most of the conditionals that have been
mentioned in this thread. But it's kind of nasty.
-Peff
^ permalink raw reply
* Re: config-file includes
From: Jay Soffian @ 2011-09-27 2:13 UTC (permalink / raw)
To: Jeff King
Cc: David Aguilar, Nguyen Thai Ngoc Duy, git, Michael Haggerty,
Junio C Hamano, Jakub Narebski
In-Reply-To: <20110926200553.GA492@sigill.intra.peff.net>
On Mon, Sep 26, 2011 at 4:05 PM, Jeff King <peff@peff.net> wrote:
>> [include-ifdef "darwin"]
>> path = ~/.gitconfig-darwin
>
> Thanks for another concrete example.
>
> I'm not sure how that would be implemented, though. I don't think git
> knows that it is compiled for darwin. Would it just be running "uname
> -s" behind the scenes? Should it happen at runtime, or as part of the
> compile process?
>
> We could allow arbitrary shell code like:
>
> [include-if "test `uname -s` -eq Darwin"]
>
> Very flexible, though it makes me think we are getting a little
> overboard. And it's an extra shell invocation whenever we read the
> config, which is ugly.
I would think git could just learn a few useful defines at the time of
compile, such as e.g. OS_Darwin would be more than sufficient.
I can also give you another use use case. I keep all my work repos
under ~Work/ and I want my user.email on all those to be my work email
addy, and all other repos on my system I want to use my personal email
address. So my ~/.gitconfig has my personal email address and then 99%
of the time I forget to configure the repos under ~/Work correctly.
That said, I'm not sure how the config include would help this...
j.
^ permalink raw reply
* Re: What's cooking in git.git (Sep 2011, #06; Wed, 21)
From: Jay Soffian @ 2011-09-27 2:08 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Scott Chacon
In-Reply-To: <7vmxdr2l16.fsf@alter.siamese.dyndns.org>
On Mon, Sep 26, 2011 at 2:52 PM, Junio C Hamano <gitster@pobox.com> wrote:
> I haven't figured out how the preformatted documentation branches will be
> managed in the longer term, as it seems likely that I no longer would have
> the post-update hook access to update them upon pushing into my k.org
> repository. I might end up dropping these branches altogether if it gets
> too cumbersome for me to maintain, but I do not know yet.
Building the documentation on OS X is a huge pain. OS X users
definitely appreciate the preformatted documentation. I know that
Homebrew uses it, and I think the git-osx-installer recently discussed
on this list uses it as well.
So, uh, maybe a way can be found to make it less cumbersome for you?
j.
^ permalink raw reply
* Re: [PATCH] refs.c: Fix slowness with numerous loose refs
From: David Michael Barr @ 2011-09-27 2:04 UTC (permalink / raw)
To: Git Mailing List
Cc: Julian Phillips, Martin Fick, Junio C Hamano, David Barr,
Shawn O. Pearce
In-Reply-To: <1317085283-33943-1-git-send-email-davidbarr@google.com>
+cc Shawn O. Pearce
I used the following to generate a test repo shaped like
a gerrit mirror with unpacked refs (10k, because life is too short for
100k tests):
cd test.git
git init
touch empty
git add empty
git commit -m 'empty'
REV=`git rev-parse HEAD`
for ((d=0;d<100;++d)); do
for ((n=0;n<100;++n)); do
let r=n*100+d
mkdir -p .git/refs/changes/$d/$r
echo $REV > .git/refs/changes/$d/$r/1
done
done
time git branch xyz
With warm caches...
Git 1.7.6.4:
real 0m8.232s
user 0m7.842s
sys 0m0.385s
Git 1.7.6.4, with patch below:
real 0m0.394s
user 0m0.069s
sys 0m0.324s
On Tue, Sep 27, 2011 at 11:01 AM, David Barr <davidbarr@google.com> wrote:
> Martin Fick reported:
> OK, I have found what I believe is another performance
> regression for large ref counts (~100K).
>
> When I run git br on my repo which only has one branch, but
> has ~100K refs under ref/changes (a gerrit repo), it takes
> normally 3-6mins depending on whether my caches are fresh or
> not. After bisecting some older changes, I noticed that
> this ref seems to be where things start to get slow:
> v1.5.2-rc0~21^2 (refs.c: add a function to sort a ref list,
> rather then sorting on add) (Julian Phillips, Apr 17, 2007)
>
> Martin Fick observed that sort_refs_lists() was called almost
> as many times as there were loose refs.
>
> Julian Phillips commented:
> Back when I made that change, I failed to notice that get_ref_dir
> was recursive for subdirectories ... sorry ...
>
> Hopefully this should speed things up. My test repo went from
> ~17m user time, to ~2.5s.
> Packing still make things much faster of course.
>
> Martin Fick acked:
> Excellent! This works (almost, in my refs.c it is called
> sort_ref_list, not sort_refs_list). So, on the non garbage
> collected repo, git branch now takes ~.5s, and in the
> garbage collected one it takes only ~.05s!
>
> [db: summarised transcript, rewrote patch to fix callee not callers]
>
> [attn jch: patch applies to maint]
>
> Analyzed-by: Martin Fick <mfick@codeaurora.org>
> Inspired-by: Julian Phillips <julian@quantumfyre.co.uk>
> Acked-by: Martin Fick <mfick@codeaurora.org>
> Signed-off-by: David Barr <davidbarr@google.com>
> ---
> refs.c | 14 ++++++++++----
> 1 files changed, 10 insertions(+), 4 deletions(-)
>
> diff --git a/refs.c b/refs.c
> index 4c1fd47..e40a09c 100644
> --- a/refs.c
> +++ b/refs.c
> @@ -255,8 +255,8 @@ static struct ref_list *get_packed_refs(const char *submodule)
> return refs->packed;
> }
>
> -static struct ref_list *get_ref_dir(const char *submodule, const char *base,
> - struct ref_list *list)
> +static struct ref_list *walk_ref_dir(const char *submodule, const char *base,
> + struct ref_list *list)
> {
> DIR *dir;
> const char *path;
> @@ -299,7 +299,7 @@ static struct ref_list *get_ref_dir(const char *submodule, const char *base,
> if (stat(refdir, &st) < 0)
> continue;
> if (S_ISDIR(st.st_mode)) {
> - list = get_ref_dir(submodule, ref, list);
> + list = walk_ref_dir(submodule, ref, list);
> continue;
> }
> if (submodule) {
> @@ -319,7 +319,13 @@ static struct ref_list *get_ref_dir(const char *submodule, const char *base,
> free(ref);
> closedir(dir);
> }
> - return sort_ref_list(list);
> + return list;
> +}
> +
> +static struct ref_list *get_ref_dir(const char *submodule, const char *base,
> + struct ref_list *list)
> +{
> + return sort_ref_list(walk_ref_dir(submodule, base, list));
> }
>
> struct warn_if_dangling_data {
> --
> 1.7.5.75.g69330
>
>
--
David Barr | Software Engineer | davidbarr@google.com | 614-3438-8348
^ permalink raw reply
* [PATCH] refs.c: Fix slowness with numerous loose refs
From: David Barr @ 2011-09-27 1:01 UTC (permalink / raw)
To: Git Mailing List; +Cc: Julian Phillips, Martin Fick, Junio C Hamano, David Barr
In-Reply-To: <CAFfmPPNCCCo=40CVvjRebXvkR7H_wh9+cz=tGxHZ1LtarE+w+A@mail.gmail.com>
Martin Fick reported:
OK, I have found what I believe is another performance
regression for large ref counts (~100K).
When I run git br on my repo which only has one branch, but
has ~100K refs under ref/changes (a gerrit repo), it takes
normally 3-6mins depending on whether my caches are fresh or
not. After bisecting some older changes, I noticed that
this ref seems to be where things start to get slow:
v1.5.2-rc0~21^2 (refs.c: add a function to sort a ref list,
rather then sorting on add) (Julian Phillips, Apr 17, 2007)
Martin Fick observed that sort_refs_lists() was called almost
as many times as there were loose refs.
Julian Phillips commented:
Back when I made that change, I failed to notice that get_ref_dir
was recursive for subdirectories ... sorry ...
Hopefully this should speed things up. My test repo went from
~17m user time, to ~2.5s.
Packing still make things much faster of course.
Martin Fick acked:
Excellent! This works (almost, in my refs.c it is called
sort_ref_list, not sort_refs_list). So, on the non garbage
collected repo, git branch now takes ~.5s, and in the
garbage collected one it takes only ~.05s!
[db: summarised transcript, rewrote patch to fix callee not callers]
[attn jch: patch applies to maint]
Analyzed-by: Martin Fick <mfick@codeaurora.org>
Inspired-by: Julian Phillips <julian@quantumfyre.co.uk>
Acked-by: Martin Fick <mfick@codeaurora.org>
Signed-off-by: David Barr <davidbarr@google.com>
---
refs.c | 14 ++++++++++----
1 files changed, 10 insertions(+), 4 deletions(-)
diff --git a/refs.c b/refs.c
index 4c1fd47..e40a09c 100644
--- a/refs.c
+++ b/refs.c
@@ -255,8 +255,8 @@ static struct ref_list *get_packed_refs(const char *submodule)
return refs->packed;
}
-static struct ref_list *get_ref_dir(const char *submodule, const char *base,
- struct ref_list *list)
+static struct ref_list *walk_ref_dir(const char *submodule, const char *base,
+ struct ref_list *list)
{
DIR *dir;
const char *path;
@@ -299,7 +299,7 @@ static struct ref_list *get_ref_dir(const char *submodule, const char *base,
if (stat(refdir, &st) < 0)
continue;
if (S_ISDIR(st.st_mode)) {
- list = get_ref_dir(submodule, ref, list);
+ list = walk_ref_dir(submodule, ref, list);
continue;
}
if (submodule) {
@@ -319,7 +319,13 @@ static struct ref_list *get_ref_dir(const char *submodule, const char *base,
free(ref);
closedir(dir);
}
- return sort_ref_list(list);
+ return list;
+}
+
+static struct ref_list *get_ref_dir(const char *submodule, const char *base,
+ struct ref_list *list)
+{
+ return sort_ref_list(walk_ref_dir(submodule, base, list));
}
struct warn_if_dangling_data {
--
1.7.5.75.g69330
^ permalink raw reply related
* Re: Git is not scalable with too many refs/*
From: Julian Phillips @ 2011-09-27 0:22 UTC (permalink / raw)
To: Martin Fick; +Cc: git
In-Reply-To: <201109261812.31738.mfick@codeaurora.org>
On Mon, 26 Sep 2011 18:12:31 -0600, Martin Fick wrote:
> On Monday, September 26, 2011 05:26:55 pm Julian Phillips
> wrote:
-- snip --
>> Back when I made that change, I failed to notice that
>> get_ref_dir was recursive for subdirectories ... sorry
>> ...
>>
>> Hopefully this should speed things up. My test repo went
>> from ~17m user time, to ~2.5s.
>> Packing still make things much faster of course.
>
> Excellent! This works (almost, in my refs.c it is called
> sort_ref_list, not sort_refs_list).
Yeah, in mine too ;) It's late and I got the compile/send mail
sequence backwards. :$
It's fixed in the proper patch email.
> So, on the non garbage
> collected repo, git branch now takes ~.5s, and in the
> garbage collected one it takes only ~.05s!
That sounds a lot better. Hopefully other commands should be faster
now too.
> Thanks way much!!!
No problem. Thank you for all the time you've put in to help chase
this down. Makes it so much easier when the person with original
problem mucks in with the investigation.
Just think how much time you've saved for anyone with a large number of
those Gerrit change refs ;)
--
Julian
^ permalink raw reply
* Re: Git is not scalable with too many refs/*
From: Martin Fick @ 2011-09-27 0:12 UTC (permalink / raw)
To: Julian Phillips; +Cc: git
In-Reply-To: <ece30e6a1b74bcddde5634003408f61f@quantumfyre.co.uk>
On Monday, September 26, 2011 05:26:55 pm Julian Phillips
wrote:
> On Mon, 26 Sep 2011 15:52:04 -0600, Martin Fick wrote:
> > On Monday, September 26, 2011 03:39:33 pm Martin Fick
wrote:
> >> On Monday, September 26, 2011 02:28:53 pm Julian
> >> Phillips
> >> In my case it was called 18785 times! Any other tests
> >> I should run?
> >
> > Gerrit stores the changes in directories under
> > refs/changes named after the last 2 digits of the
> > change. Then under each change it stores each
> > patchset. So it looks like this:
> > refs/changes/dd/change_num/ps_num
> >
> > I noticed that:
> > ls refs/changes/* | wc -l
> > -> 18876
> >
> > somewhat close, but not super close to 18785, I am not
> > sure if that is a clue. It's almost like each change
> > is causing a re-sort,
>
> basically, it is ...
>
> Back when I made that change, I failed to notice that
> get_ref_dir was recursive for subdirectories ... sorry
> ...
>
> Hopefully this should speed things up. My test repo went
> from ~17m user time, to ~2.5s.
> Packing still make things much faster of course.
Excellent! This works (almost, in my refs.c it is called
sort_ref_list, not sort_refs_list). So, on the non garbage
collected repo, git branch now takes ~.5s, and in the
garbage collected one it takes only ~.05s!
Thanks way much!!!
-Martin
--
Employee of Qualcomm Innovation Center, Inc. which is a
member of Code Aurora Forum
^ permalink raw reply
* Re: [PATCH/RFC 0/2] Teach receive-pack not to run update hook for corrupt/non existent ref
From: Sitaram Chamarty @ 2011-09-27 0:05 UTC (permalink / raw)
To: Junio C Hamano
Cc: Pang Yan Han, git, Shawn O. Pearce, Jeff King,
Johannes Schindelin
In-Reply-To: <7voby6zwxg.fsf@alter.siamese.dyndns.org>
On Tue, Sep 27, 2011 at 5:19 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Sitaram Chamarty <sitaramc@gmail.com> writes:
>
>>> In that case (if "non-existent-ref" was indeed non-existent, and not just
>>> pointing at a dangling commit), I would say the post anything hook should
>>> not be called for that ref. These hooks of course need to run if there
>>> are _other_ refs that were updated, though, to handle these _other_ refs,
>>> but I do not think they should be told about the no-op.
>>
>> Question is what happens if none of them existed. It's a difference
>> between not calling the hook at all, versus calling it with no
>> arguments/empty stdin (as the case may be) -- which would you do?
>
> In case it was unclear, I was trying to say the hooks should not run with
> empty input.
I saw "should not be called for that ref" and I did get confused;
thanks for clarifying.
I perfectly fine with it for post-{update,receive}. My interest is in
making sure the *update* hook runs, which (in an earlier email in the
thread) I explained why and you agreed it made sense.
Thanks,
--
Sitaram
^ permalink raw reply
* [PATCH] Don't sort ref_list too early
From: Julian Phillips @ 2011-09-27 0:00 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Martin Fick, git
In-Reply-To: <7vsjnizxf5.fsf@alter.siamese.dyndns.org>
get_ref_dir is called recursively for subdirectories, which means that
we were calling sort_ref_list for each directory of refs instead of
once for all the refs. This is a massive wast of processing, so now
just call sort_ref_list on the result of the top-level get_ref_dir, so
that the sort is only done once.
In the common case of only a few different directories of refs the
difference isn't very noticable, but it becomes very noticeable when
you have a large number of direcotries containing refs (e.g. as
created by Gerrit).
Reported by Martin Fick.
Signed-off-by: Julian Phillips <julian@quantumfyre.co.uk>
---
This time the typos are fixed too ... perhaps I wrote the original commit at 1am
too ... :$
refs.c | 4 +++-
1 files changed, 3 insertions(+), 1 deletions(-)
diff --git a/refs.c b/refs.c
index a615043..a49ff74 100644
--- a/refs.c
+++ b/refs.c
@@ -319,7 +319,7 @@ static struct ref_list *get_ref_dir(const char *submodule, const char *base,
free(ref);
closedir(dir);
}
- return sort_ref_list(list);
+ return list;
}
struct warn_if_dangling_data {
@@ -361,11 +361,13 @@ 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);
+ submodule_refs.loose = sort_ref_list(submodule_refs.loose);
return submodule_refs.loose;
}
if (!cached_refs.did_loose) {
cached_refs.loose = get_ref_dir(NULL, "refs", NULL);
+ cached_refs.loose = sort_ref_list(cached_refs.loose);
cached_refs.did_loose = 1;
}
return cached_refs.loose;
--
1.7.6.1
^ permalink raw reply related
* Re: [PATCH/RFC 0/2] Teach receive-pack not to run update hook for corrupt/non existent ref
From: Junio C Hamano @ 2011-09-27 0:04 UTC (permalink / raw)
To: Sitaram Chamarty
Cc: Pang Yan Han, git, Shawn O. Pearce, Jeff King,
Johannes Schindelin
In-Reply-To: <7voby6zwxg.fsf@alter.siamese.dyndns.org>
Junio C Hamano <gitster@pobox.com> writes:
> Sitaram Chamarty <sitaramc@gmail.com> writes:
>
>>> In that case (if "non-existent-ref" was indeed non-existent, and not just
>>> pointing at a dangling commit), I would say the post anything hook should
>>> not be called for that ref. These hooks of course need to run if there
>>> are _other_ refs that were updated, though, to handle these _other_ refs,
>>> but I do not think they should be told about the no-op.
>>
>> Question is what happens if none of them existed. It's a difference
>> between not calling the hook at all, versus calling it with no
>> arguments/empty stdin (as the case may be) -- which would you do?
>
> In case it was unclear, I was trying to say the hooks should not run with
> empty input.
If the purpose of "post-update" (or "post-receive") hooks were to trigger
every time anybody attempted to push into the repository, then it would
make perfect sense for them to trigger when "push origin :no-such-branch"
were attempted. But if that were the purpose of these hooks, they should
also trigger when "push origin master" is run and "master" is already at
the right commit, as that is the same kind of no-op -- the pushed into
repository was already up-to-date with respect to the wish of the pusher.
I do not mind, and I do prefer, these hooks to run when somebody deleted
an existing ref that points at a corrupt or non-existent object, as that
is _not_ a no-op but is a meaningful event that has an effect that is
observable from the outside world (e.g. ls-remote).
^ permalink raw reply
* Re: [PATCH/RFC 0/2] Teach receive-pack not to run update hook for corrupt/non existent ref
From: Junio C Hamano @ 2011-09-26 23:49 UTC (permalink / raw)
To: Sitaram Chamarty
Cc: Junio C Hamano, Pang Yan Han, git, Shawn O. Pearce, Jeff King,
Johannes Schindelin
In-Reply-To: <CAMK1S_gR6U=OxzGsjTD8LbvZFS125=p1fQ8Af7aRD2XSsRur_Q@mail.gmail.com>
Sitaram Chamarty <sitaramc@gmail.com> writes:
>> In that case (if "non-existent-ref" was indeed non-existent, and not just
>> pointing at a dangling commit), I would say the post anything hook should
>> not be called for that ref. These hooks of course need to run if there
>> are _other_ refs that were updated, though, to handle these _other_ refs,
>> but I do not think they should be told about the no-op.
>
> Question is what happens if none of them existed. It's a difference
> between not calling the hook at all, versus calling it with no
> arguments/empty stdin (as the case may be) -- which would you do?
In case it was unclear, I was trying to say the hooks should not run with
empty input.
^ permalink raw reply
* Re: [PATCH/RFC 0/2] Teach receive-pack not to run update hook for corrupt/non existent ref
From: Sitaram Chamarty @ 2011-09-26 23:44 UTC (permalink / raw)
To: Junio C Hamano
Cc: Pang Yan Han, git, Shawn O. Pearce, Jeff King,
Johannes Schindelin
In-Reply-To: <7vwrcuzy44.fsf@alter.siamese.dyndns.org>
On Tue, Sep 27, 2011 at 4:53 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Sitaram Chamarty <sitaramc@gmail.com> writes:
>
>> From a philosophical point of view, update and pre-receive *check*
>> things to make sure everything is OK. IMO they should be allowed to
>> run even if the ref being deleted doesn't exist -- that could well be
>> an error condition that the guy who owns the repo wants to trap and
>> alert himself to in some special way. I would *not* like them
>> disabled.
>
> I think this is a sane thing to do.
>
>> Post-{update,receive} are for *after* a successful push. My
>> suggestion would be to make sure the inputs supplied to those hooks
>> (via STDIN for post-receive, and as arguments in case of post-update)
>> reflect this -- only successfully updated refs are sent in as args.
>
> Perhaps sane.
>
>> This might mean that in the case of 'git push origin
>> :refs/heads/non-existent-ref' the post-receive hook would run but
>> STDIN would be empty, and post-update would run but have no arguments.
>
> Hmm?
>
> In that case (if "non-existent-ref" was indeed non-existent, and not just
> pointing at a dangling commit), I would say the post anything hook should
> not be called for that ref. These hooks of course need to run if there
> are _other_ refs that were updated, though, to handle these _other_ refs,
> but I do not think they should be told about the no-op.
Question is what happens if none of them existed. It's a difference
between not calling the hook at all, versus calling it with no
arguments/empty stdin (as the case may be) -- which would you do? I
say the hook *should* always run, and the code inside the hook should
take care of the fact that no arguments/no input means nothing
actually happened.
--
Sitaram
^ permalink raw reply
* Re: Git is not scalable with too many refs/*
From: Junio C Hamano @ 2011-09-26 23:38 UTC (permalink / raw)
To: Julian Phillips; +Cc: Martin Fick, git
In-Reply-To: <ece30e6a1b74bcddde5634003408f61f@quantumfyre.co.uk>
Julian Phillips <julian@quantumfyre.co.uk> writes:
> Back when I made that change, I failed to notice that get_ref_dir was
> recursive for subdirectories ... sorry ...
Aha, I also was blind while I was watching this discussion from the
sideline, and I thought I re-read the codepath involved X-<. Indeed
we were sorting the list way too early and the patch looks correct.
Thanks.
> Hopefully this should speed things up. My test repo went from ~17m
> user time, to ~2.5s.
> Packing still make things much faster of course.
>
> diff --git a/refs.c b/refs.c
> index a615043..212e7ec 100644
> --- a/refs.c
> +++ b/refs.c
> @@ -319,7 +319,7 @@ static struct ref_list *get_ref_dir(const char
> *submodule, c
> free(ref);
> closedir(dir);
> }
> - return sort_ref_list(list);
> + return list;
> }
>
> struct warn_if_dangling_data {
> @@ -361,11 +361,13 @@ static struct ref_list *get_loose_refs(const
> char *submodu
> if (submodule) {
> free_ref_list(submodule_refs.loose);
> submodule_refs.loose = get_ref_dir(submodule, "refs",
> NULL);
> + submodule_refs.loose =
> sort_refs_list(submodule_refs.loose);
> return submodule_refs.loose;
> }
>
> if (!cached_refs.did_loose) {
> cached_refs.loose = get_ref_dir(NULL, "refs", NULL);
> + cached_refs.loose = sort_refs_list(cached_refs.loose);
> cached_refs.did_loose = 1;
> }
> return cached_refs.loose;
>
>
>
>>
>>
>> -Martin
^ permalink raw reply
* Re: Git is not scalable with too many refs/*
From: David Michael Barr @ 2011-09-26 23:37 UTC (permalink / raw)
To: Julian Phillips; +Cc: Martin Fick, git
In-Reply-To: <ece30e6a1b74bcddde5634003408f61f@quantumfyre.co.uk>
On Tue, Sep 27, 2011 at 9:26 AM, Julian Phillips
<julian@quantumfyre.co.uk> wrote:
>
> On Mon, 26 Sep 2011 15:52:04 -0600, Martin Fick wrote:
>>
>> On Monday, September 26, 2011 03:39:33 pm Martin Fick wrote:
>>>
>>> On Monday, September 26, 2011 02:28:53 pm Julian Phillips
>>> wrote:
>>> > >> Random thought. What happens to the with
>>> > >> compression case if you leave the commit in, but
>>> > >> add a sleep(15) to the end of sort_refs_list?
>>> > >
>>> > > Why, what are you thinking? Hmm, I am trying this on
>>> > > the non gced repo and it doesn't seem to be
>>> > > completing (no cpu usage)! It appears that perhaps
>>> > > it is being called many times (the sleeping would
>>> > > explain no cpu usage)?!? This could be a real
>>> > > problem, this should only get called once right?
>>> >
>>> > I was just wondering if the time taken to get the refs
>>> > was changing the interaction with something else. Not
>>> > very likely, but ...
>>> >
>>> > I added a print statement, and it was called four times
>>> > when I had unpacked refs, and once with packed. So,
>>> > maybe you are hitting some nasty case with unpacked
>>> > refs. If you use a print statement instead of a sleep,
>>> > how many times does sort_refs_lists get called in your
>>> > unpacked case? It may well also be worth calculating
>>> > the time taken to do the sort.
>>>
>>> In my case it was called 18785 times! Any other tests I
>>> should run?
>>
>> Gerrit stores the changes in directories under refs/changes
>> named after the last 2 digits of the change. Then under
>> each change it stores each patchset. So it looks like this:
>> refs/changes/dd/change_num/ps_num
>>
>> I noticed that:
>>
>> ls refs/changes/* | wc -l
>> -> 18876
>>
>> somewhat close, but not super close to 18785, I am not sure
>> if that is a clue. It's almost like each change is causing
>> a re-sort,
>
> basically, it is ...
>
> Back when I made that change, I failed to notice that get_ref_dir was recursive for subdirectories ... sorry ...
>
> Hopefully this should speed things up. My test repo went from ~17m user time, to ~2.5s.
> Packing still make things much faster of course.
>
> diff --git a/refs.c b/refs.c
> index a615043..212e7ec 100644
> --- a/refs.c
> +++ b/refs.c
> @@ -319,7 +319,7 @@ static struct ref_list *get_ref_dir(const char *submodule, c
> free(ref);
> closedir(dir);
> }
> - return sort_ref_list(list);
> + return list;
> }
>
> struct warn_if_dangling_data {
> @@ -361,11 +361,13 @@ static struct ref_list *get_loose_refs(const char *submodu
> if (submodule) {
> free_ref_list(submodule_refs.loose);
> submodule_refs.loose = get_ref_dir(submodule, "refs", NULL);
> + submodule_refs.loose = sort_refs_list(submodule_refs.loose);
> return submodule_refs.loose;
> }
>
> if (!cached_refs.did_loose) {
> cached_refs.loose = get_ref_dir(NULL, "refs", NULL);
> + cached_refs.loose = sort_refs_list(cached_refs.loose);
> cached_refs.did_loose = 1;
> }
> return cached_refs.loose;
>
>
>
>>
>>
>> -Martin
>
> --
> Julian
> --
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
Well done! I'll try to compose a patch attributed to Julian with the
information from this thread.
--
David Barr
^ permalink raw reply
* Re: [BUG?] git fetch -p -t prunes all non-tag refs
From: Carlos Martín Nieto @ 2011-09-26 23:28 UTC (permalink / raw)
To: Junio C Hamano; +Cc: mathstuf, git
In-Reply-To: <7v1uv228t4.fsf@alter.siamese.dyndns.org>
[-- Attachment #1: Type: text/plain, Size: 1724 bytes --]
On Mon, 2011-09-26 at 16:16 -0700, Junio C Hamano wrote:
> Carlos Martín Nieto <cmn@elego.de> writes:
>
> > On Mon, 2011-09-26 at 15:30 -0700, Junio C Hamano wrote:
> >> Ben Boeckel <mathstuf@gmail.com> writes:
> >>
> >> > When the --prune and --tags options are given to git fetch together, all
> >> > non-tag refs are pruned because only tags are looked at and when pruning
> >> > it appears as if the branches have disappeared and are therefore deleted
> >> > locally.
> >>
> >> I would call that a bug, and it is not limited to the use of "--tags". For
> >> example, I suspect that
> >>
> >> $ git fetch --prune origin refs/heads/master:refs/remotes/origin/master
> >>
> >> would prune remote tracking branches for "origin" other than "master".
> >
> > This should fix it (in a way). Let's agree that it's a bad idea and
> > complain to the user.
>
> That might be a reasonable short-term safety measure, but in the longer
Sure.
> term I think we should fix it properly. We are already learning "what are
> the refs the remote side currently has" from the transport and the right
> fix ought to be to use that original information, not the version filtered
> for the use of the primary objective of fetch, which is to only fetch what
> the user asked for.
Do you mean that we should ignore the refspec? Or do you mean that we
should look at the refspec if it exists, and only consider deleting
those that meet the refspec, so that `--prune --tags` would only delete
tags that don't exist in the remote?
Either way, it's a bit more complicated than a two-liner and it's too
late in my timezone for that. I'll try to see if I can do it in the next
few days.
cmn
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 490 bytes --]
^ permalink raw reply
* Re: Git is not scalable with too many refs/*
From: Julian Phillips @ 2011-09-26 23:26 UTC (permalink / raw)
To: Martin Fick; +Cc: git
In-Reply-To: <201109261552.04946.mfick@codeaurora.org>
On Mon, 26 Sep 2011 15:52:04 -0600, Martin Fick wrote:
> On Monday, September 26, 2011 03:39:33 pm Martin Fick wrote:
>> On Monday, September 26, 2011 02:28:53 pm Julian Phillips
>> wrote:
>> > >> Random thought. What happens to the with
>> > >> compression case if you leave the commit in, but
>> > >> add a sleep(15) to the end of sort_refs_list?
>> > >
>> > > Why, what are you thinking? Hmm, I am trying this on
>> > > the non gced repo and it doesn't seem to be
>> > > completing (no cpu usage)! It appears that perhaps
>> > > it is being called many times (the sleeping would
>> > > explain no cpu usage)?!? This could be a real
>> > > problem, this should only get called once right?
>> >
>> > I was just wondering if the time taken to get the refs
>> > was changing the interaction with something else. Not
>> > very likely, but ...
>> >
>> > I added a print statement, and it was called four times
>> > when I had unpacked refs, and once with packed. So,
>> > maybe you are hitting some nasty case with unpacked
>> > refs. If you use a print statement instead of a sleep,
>> > how many times does sort_refs_lists get called in your
>> > unpacked case? It may well also be worth calculating
>> > the time taken to do the sort.
>>
>> In my case it was called 18785 times! Any other tests I
>> should run?
>
> Gerrit stores the changes in directories under refs/changes
> named after the last 2 digits of the change. Then under
> each change it stores each patchset. So it looks like this:
> refs/changes/dd/change_num/ps_num
>
> I noticed that:
>
> ls refs/changes/* | wc -l
> -> 18876
>
> somewhat close, but not super close to 18785, I am not sure
> if that is a clue. It's almost like each change is causing
> a re-sort,
basically, it is ...
Back when I made that change, I failed to notice that get_ref_dir was
recursive for subdirectories ... sorry ...
Hopefully this should speed things up. My test repo went from ~17m
user time, to ~2.5s.
Packing still make things much faster of course.
diff --git a/refs.c b/refs.c
index a615043..212e7ec 100644
--- a/refs.c
+++ b/refs.c
@@ -319,7 +319,7 @@ static struct ref_list *get_ref_dir(const char
*submodule, c
free(ref);
closedir(dir);
}
- return sort_ref_list(list);
+ return list;
}
struct warn_if_dangling_data {
@@ -361,11 +361,13 @@ static struct ref_list *get_loose_refs(const char
*submodu
if (submodule) {
free_ref_list(submodule_refs.loose);
submodule_refs.loose = get_ref_dir(submodule, "refs",
NULL);
+ submodule_refs.loose =
sort_refs_list(submodule_refs.loose);
return submodule_refs.loose;
}
if (!cached_refs.did_loose) {
cached_refs.loose = get_ref_dir(NULL, "refs", NULL);
+ cached_refs.loose = sort_refs_list(cached_refs.loose);
cached_refs.did_loose = 1;
}
return cached_refs.loose;
>
>
> -Martin
--
Julian
^ permalink raw reply related
* Re: [PATCH/RFC 0/2] Teach receive-pack not to run update hook for corrupt/non existent ref
From: Junio C Hamano @ 2011-09-26 23:23 UTC (permalink / raw)
To: Sitaram Chamarty
Cc: Pang Yan Han, git, Shawn O. Pearce, Jeff King,
Johannes Schindelin
In-Reply-To: <CAMK1S_h3ufrK29_ajpcSSW7HV6ZA8z8ZVHvhHr2bx5Cga5FAKQ@mail.gmail.com>
Sitaram Chamarty <sitaramc@gmail.com> writes:
> From a philosophical point of view, update and pre-receive *check*
> things to make sure everything is OK. IMO they should be allowed to
> run even if the ref being deleted doesn't exist -- that could well be
> an error condition that the guy who owns the repo wants to trap and
> alert himself to in some special way. I would *not* like them
> disabled.
I think this is a sane thing to do.
> Post-{update,receive} are for *after* a successful push. My
> suggestion would be to make sure the inputs supplied to those hooks
> (via STDIN for post-receive, and as arguments in case of post-update)
> reflect this -- only successfully updated refs are sent in as args.
Perhaps sane.
> This might mean that in the case of 'git push origin
> :refs/heads/non-existent-ref' the post-receive hook would run but
> STDIN would be empty, and post-update would run but have no arguments.
Hmm?
In that case (if "non-existent-ref" was indeed non-existent, and not just
pointing at a dangling commit), I would say the post anything hook should
not be called for that ref. These hooks of course need to run if there
are _other_ refs that were updated, though, to handle these _other_ refs,
but I do not think they should be told about the no-op.
^ permalink raw reply
* Re: rerere with modified/deleted conflicts
From: Nicolas Pitre @ 2011-09-26 23:23 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7v62ke293f.fsf@alter.siamese.dyndns.org>
On Mon, 26 Sep 2011, Junio C Hamano wrote:
> Nicolas Pitre <nico@fluxnic.net> writes:
>
> > Is this supported already?
>
> I do not think so, as rerere is about applying previous change using 3-way
> merge. A modify/delete conflict would mean you have only two stages, and
> while I can see somebody might want to say "I want to ignore further
> modifications to this dead path ignored and resolve in favor of deletion",
> it felt a bit too aggressive to my taste when I last worked on rerere.
Well, that's exactly the use case we have here this would be nice for.
But I wouldn't design it as "ignore any further changes" but rather
"ignore _this_ particular change to the file I want you to delete".
Nicolas
^ 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