* Re: [PATCH/WIP 02/11] notes-merge: use opendir/readdir instead of using read_directory()
From: Junio C Hamano @ 2011-10-27 17:23 UTC (permalink / raw)
To: Nguyen Thai Ngoc Duy; +Cc: Junio C Hamano, git
In-Reply-To: <CACsJy8C4iHffr4UYP9SvCU0OPC-LocUORwAQ492LqaV_tyvFQA@mail.gmail.com>
Nguyen Thai Ngoc Duy <pclouds@gmail.com> writes:
>> When read_directory("where/ever") is called, what kind of paths does it
>> collect? Do the paths the function collects share "where/ever" as their
>> common prefix? I thought it collects the paths relative to whatever
>> top-level directory given to the function, so that "where/ever" could be
>> anything.
>
> Correct. But read_directory() takes pathspec now so naturally it does
> not treat "where/ever" a common prefix anymore. So it has to open(".")
> and starts from there.
That is a puzzling statement. The read_directory() function takes:
- dir: use this struct to pass traversal status and collected paths;
- path, len: this is the directory (not a pathspec) we start traversal
from; and
- pathspec: these are the patterns that specify which parts of the
directory hierarchy under <path,len> are traversed.
I do not see any good reason for <path,len> to become a match pattern. Are
you trying to get it prepended to elements in pathspec[] and match the path
collected including the <path> part?
Why?
I could see that "open . and start from there, treating as if <path,len>
is also pathspec" could be made to work, but I do not see why that is
desirable.
In other words, are there existing callers that abuse read_directory()
to feed a pattern in <path,len>? Maybe they should be the one that needs
fixing instead?
^ permalink raw reply
* Re: [PATCH] gitweb/Makefile: Remove static/gitweb.js in the clean target
From: Ramsay Jones @ 2011-10-26 21:30 UTC (permalink / raw)
To: Jakub Narebski; +Cc: Drew Northup, Junio C Hamano, GIT Mailing-list
In-Reply-To: <201110260236.59509.jnareb@gmail.com>
Jakub Narebski wrote:
> Drew Northup napisał:
>> On Tue, 2011-10-25 at 18:15 +0100, Ramsay Jones wrote:
>>> Signed-off-by: Ramsay Jones <ramsay@ramsay1.demon.co.uk>
>>> ---
>>> gitweb/Makefile | 4 +++-
>>> 1 files changed, 3 insertions(+), 1 deletions(-)
>>>
>>> diff --git a/gitweb/Makefile b/gitweb/Makefile
>>> index 1c85b5f..4191c6b 100644
>>> --- a/gitweb/Makefile
>>> +++ b/gitweb/Makefile
>>> @@ -185,7 +185,9 @@ install: all
>>> ### Cleaning rules
>>>
>>> clean:
>>> - $(RM) gitweb.cgi static/gitweb.min.js static/gitweb.min.css GITWEB-BUILD-OPTIONS
>>> + $(RM) gitweb.cgi static/gitweb.js \
>>> + static/gitweb.min.js static/gitweb.min.css \
>>> + GITWEB-BUILD-OPTIONS
>>>
>>> .PHONY: all clean install test test-installed .FORCE-GIT-VERSION-FILE FORCE
>>>
>> Forgive me for sounding a bit numb, but what does this fix? I don't see
>> it in the commit message.
>
> gitweb.js is nowadays a generated file. Though that bit should be
> in commit message...
Yep, will do ...
ATB,
Ramsay Jones
^ permalink raw reply
* Re: [PATCH 0/2] git-credential-cache--daemon on Cygwin
From: Ramsay Jones @ 2011-10-27 17:18 UTC (permalink / raw)
To: Jeff King; +Cc: GIT Mailing-list
In-Reply-To: <20111022191509.GB1785@sigill.intra.peff.net>
Jeff King wrote:
> On Sat, Oct 22, 2011 at 06:23:25PM +0100, Ramsay Jones wrote:
>
>> Assuming that a modified http-auth-keyring series will make a return to pu
>> sometime, could you please squash these patches into (the patch corresponding to)
>> commit 2d6874d (credentials: add "cache" helper, 18-07-2011). Thanks!
>
> I'm planning a reroll, so I'll squash them (or something similar) in.
Thanks!
> It's definitely coming back, so if you feel like working on it, please
> do. Also note that if it would be easier to have an alternate
> abstraction for inter-process communication on windows, I'm open to
> doing that in the cache daemon.
My initial reaction was to use a "named pipe" (aka fifo), but on reflection,
I don't think it would be any easier; the unix socket emulation should not be too
difficult (famous last words!). :-D
ATB,
Ramsay Jones
^ permalink raw reply
* Re: [PATCH 1/2] credential-cache--daemon.c: Don't leave stale socket on --exit
From: Ramsay Jones @ 2011-10-27 17:29 UTC (permalink / raw)
To: Jeff King; +Cc: GIT Mailing-list
In-Reply-To: <20111022191711.GC1785@sigill.intra.peff.net>
Jeff King wrote:
> On Sat, Oct 22, 2011 at 06:24:51PM +0100, Ramsay Jones wrote:
>
>> Signed-off-by: Ramsay Jones <ramsay@ramsay1.demon.co.uk>
>> ---
>> credential-cache--daemon.c | 23 ++++++++++++-----------
>> 1 files changed, 12 insertions(+), 11 deletions(-)
>
> Looks sane, and I'll probably squash it in. Alternatively, we could also
> set a signal/exit handler to clean up our socket when we die. That would
> also cover the error exit cases.
I considered this, *very* briefly, but decided it wasn't worth the effort
or complexity.
> In either case, I think we need to handle stale sockets better. They
> will happen eventually due to power loss or kill -9, anyway.
Indeed, hence patch #2. ;-)
I suspect that, given the current code, the *vast* majority of stale sockets
would be (somewhat gratuitously) created by the --exit action - hence this
patch. :-P
ATB,
Ramsay Jones
^ permalink raw reply
* Re: [PATCH] Fix 'Cloning into' message
From: Junio C Hamano @ 2011-10-27 17:39 UTC (permalink / raw)
To: Richard Hartmann; +Cc: git
In-Reply-To: <1319734013-8956-1-git-send-email-richih.mailinglist@gmail.com>
Richard Hartmann <richih.mailinglist@gmail.com> writes:
> Without this patch,
>
> git clone foo .
>
> results in this:
>
> Cloning into ....
> done.
>
> With it:
>
> Cloning into '.'...
> done.
>
> Signed-off-by: Richard Hartmann <richih.mailinglist@gmail.com>
We try to be consistent and many places do quote user supplied paths in
pair of single quotes in human readable messages, and this is in line with
that pattern.
$ git clone foo "joey's foo"
would result in
Cloning into 'joey's foo'...
but that is probably Ok ;-)
Thanks.
^ permalink raw reply
* Re: [PATCH/WIP 03/11] t5403: avoid doing "git add foo/bar" where foo/.git exists
From: Junio C Hamano @ 2011-10-27 17:41 UTC (permalink / raw)
To: Nguyen Thai Ngoc Duy; +Cc: git
In-Reply-To: <CACsJy8C2nUJkN5=E7p2u_wjHqWy7EC_BS3Sr4+_QgunWHDdtKg@mail.gmail.com>
Nguyen Thai Ngoc Duy <pclouds@gmail.com> writes:
> On Thu, Oct 27, 2011 at 4:26 AM, Junio C Hamano <gitster@pobox.com> wrote:
> ...
>> Where did you get this idea that submodule is somehow involved in this test?
>
> Because "clone2" looks like a submodule (it has ".git" inside with valid HEAD)
But there is a crucial difference between "it looks like" and "it is". If
it is not a submodule, and the established behaviour is not to treat it
like a submodule, then we shouldn't suddenly change our behaviour to start
treating as one.
> ... Should we stick with one way only?
Whatever we have been doing should not change, especially in corner cases.
^ permalink raw reply
* Re: [PATCH 0/2] git-credential-cache--daemon on Cygwin
From: Jeff King @ 2011-10-27 17:41 UTC (permalink / raw)
To: Ramsay Jones; +Cc: GIT Mailing-list
In-Reply-To: <4EA99266.7030002@ramsay1.demon.co.uk>
On Thu, Oct 27, 2011 at 06:18:30PM +0100, Ramsay Jones wrote:
> > It's definitely coming back, so if you feel like working on it, please
> > do. Also note that if it would be easier to have an alternate
> > abstraction for inter-process communication on windows, I'm open to
> > doing that in the cache daemon.
>
> My initial reaction was to use a "named pipe" (aka fifo), but on reflection,
> I don't think it would be any easier; the unix socket emulation should not be too
> difficult (famous last words!). :-D
The problem with named pipes is that all clients share the same pipe. So
if two clients connect, their writes will be intermingled at the server,
and they will both get the responses for each other. Assuming your
platform provides atomic write() over pipes, you could design a
packet-ized protocol, and just have clients ignore packets not meant for
them. But that's getting pretty complex.
-Peff
^ permalink raw reply
* Re: [PATCH 1/2] credential-cache--daemon.c: Don't leave stale socket on --exit
From: Jeff King @ 2011-10-27 17:42 UTC (permalink / raw)
To: Ramsay Jones; +Cc: GIT Mailing-list
In-Reply-To: <4EA9950A.9020208@ramsay1.demon.co.uk>
On Thu, Oct 27, 2011 at 06:29:46PM +0100, Ramsay Jones wrote:
> Jeff King wrote:
> > On Sat, Oct 22, 2011 at 06:24:51PM +0100, Ramsay Jones wrote:
> >
> >> Signed-off-by: Ramsay Jones <ramsay@ramsay1.demon.co.uk>
> >> ---
> >> credential-cache--daemon.c | 23 ++++++++++++-----------
> >> 1 files changed, 12 insertions(+), 11 deletions(-)
> >
> > Looks sane, and I'll probably squash it in. Alternatively, we could also
> > set a signal/exit handler to clean up our socket when we die. That would
> > also cover the error exit cases.
>
> I considered this, *very* briefly, but decided it wasn't worth the effort
> or complexity.
Actually, I think with the sigchain code, it would only end up as a few
lines. I'll probably take a look when I re-roll.
Thanks for your patches.
-Peff
^ permalink raw reply
* Re: Credentials and the Secrets API...
From: Jeff King @ 2011-10-27 17:48 UTC (permalink / raw)
To: John Szakmeister; +Cc: git
In-Reply-To: <CAEBDL5Udooim_3Za76Q1Rt_aGXtsSv76nxRegGWRBE=WJQzfZA@mail.gmail.com>
On Thu, Oct 27, 2011 at 12:05:03PM -0400, John Szakmeister wrote:
> Just wanted to keep folks in the loop. It turns out that the Secrets
> API is still to young. I asked about the format to store credentials
> in (as far as attributes), and got a response from a KDE developer
> that says it's still to young on their front. They hope to have
> support in the next release of KDE. But there's still the issue of
> what attributes to use.
Thanks for looking into this. That explains why I had such trouble
finding good documentation on it.
> With that information, I went ahead and created a
> gnome-credential-keyring that uses Gnome's Keyring facility. I still
> need to do a few more things (mainly run it against Jeff's tests), but
> it's generally working. Just wanted to keep folks in the loop.
> Hopefully, I can get patches out this weekend.
Great, I'm looking forward to reading it.
> Jeff: it would be really excellent to break out the various pieces. I
> think it would also be better to split the asking for passwords from
> the storing of passwords.
That's my current plan. I just need to stop dragging my feet on
re-rolling the series.
-Peff
^ permalink raw reply
* Re: [PATCH] gitweb/Makefile: Remove static/gitweb.js in the clean target
From: Junio C Hamano @ 2011-10-27 17:49 UTC (permalink / raw)
To: Ramsay Jones
Cc: Jakub Narebski, Drew Northup, Junio C Hamano, GIT Mailing-list
In-Reply-To: <4EA87C06.8080808@ramsay1.demon.co.uk>
Ramsay Jones <ramsay@ramsay1.demon.co.uk> writes:
>> gitweb.js is nowadays a generated file. Though that bit should be
>> in commit message...
>
> Yep, will do ...
Thanks; here is what I already queued.
-- >8 --
From: Ramsay Jones <ramsay@ramsay1.demon.co.uk>
Date: Tue, 25 Oct 2011 18:15:20 +0100
Subject: [PATCH] gitweb/Makefile: Remove static/gitweb.js in the clean target
Since 9a86dd5 (gitweb: Split JavaScript for maintability, combining on
build, 2011-04-28), static/gitweb.js has been a build product that should
be cleaned upon "make clean".
Signed-off-by: Ramsay Jones <ramsay@ramsay1.demon.co.uk>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
gitweb/Makefile | 4 +++-
1 files changed, 3 insertions(+), 1 deletions(-)
diff --git a/gitweb/Makefile b/gitweb/Makefile
index 5d20515..c360284 100644
--- a/gitweb/Makefile
+++ b/gitweb/Makefile
@@ -183,7 +183,9 @@ install: all
### Cleaning rules
clean:
- $(RM) gitweb.cgi static/gitweb.min.js static/gitweb.min.css GITWEB-BUILD-OPTIONS
+ $(RM) gitweb.cgi static/gitweb.js \
+ static/gitweb.min.js static/gitweb.min.css \
+ GITWEB-BUILD-OPTIONS
.PHONY: all clean install test test-installed .FORCE-GIT-VERSION-FILE FORCE
--
1.7.7.1.552.g2c3d8
^ permalink raw reply related
* Re: [PATCH] git grep: be careful to use mutices only when they are initialized
From: Jeff King @ 2011-10-27 18:02 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Johannes Schindelin, msysgit, git
In-Reply-To: <7v39ef34in.fsf@alter.siamese.dyndns.org>
On Wed, Oct 26, 2011 at 01:02:40PM -0700, Junio C Hamano wrote:
> - Could we lose "#ifndef NO_PTHREADS" inside grep_sha1(), grep_file(),
> and possibly cmd_grep() functions and let the compiler optimize things
> away under NO_PTHREADS compilation?
I don't think so. If NO_PTHREADS is set, we might not have pthread
functions at all. Sure, many compilers will optimize:
if (0)
pthread_mutex_lock(...);
to remove the call completely. But would a compiler be wrong to complain
that pthread_mutex_lock is not defined, or to include reference to it
for the linker? gcc, both with and without optimizations, will complain
about:
echo 'int main() { if (0) does_not_exist(); return 0; }' >foo.c
gcc -Wall -c foo.c
though it does actually remove the dead code and link properly. I
wouldn't be surprised if some other compilers don't work, though (and of
course the warning is ugly).
I think you would have to do something like this in thread-utils.h:
#ifndef NO_PTHREADS
#include <pthread.h>
#else
#define pthread_mutex_t int
#define pthread_mutex_init(m, a) do {} while(0)
#define pthread_mutex_lock(m) do {} while(0)
#define pthread_mutex_unlock(m) do {} while (0)
/* and so forth for every pthread function */
#endif
-Peff
^ permalink raw reply
* Re: [PATCH/WIP 01/11] Introduce "check-attr --excluded" as a replacement for "add --ignore-missing"
From: Junio C Hamano @ 2011-10-27 18:08 UTC (permalink / raw)
To: Nguyễn Thái Ngọc Duy; +Cc: git
In-Reply-To: <1319438176-7304-2-git-send-email-pclouds@gmail.com>
Nguyễn Thái Ngọc Duy <pclouds@gmail.com> writes:
> --ignore-missing is used by submodule to check if a path may be
> ignored by .gitignore files. It does not really fit in git-add (git
> add takes pathspec, but --ignore-missing takes only paths)
>
> Google reckons that --ignore-missing is not used anywhere but
> git-submodule.sh. Remove --ignore-missing and introduce "check-attr
> --excluded" as a replacement.
Hmm. "add --ignore-missing" somehow does not fit very well with other uses
of the option of the same name. In all other contexts, "ignore-missing"
means just that: ignore the fact that whatever _thing_ we were made to
expect to exist by the instruction from the user does not exist, which
usually results in an error or a report. "add --ignore-missing" does not
seem to be that (for one thing, it requires --dry-run).
It is unclear to me what is supposed to do to after reading the three-line
documentation in the manpage X-<.
So I am perfectly OK with the removal in the current form.
But I do not think "is this path ignored with the .gitignore rules" check
belongs to check-attr, either.
The pattern of having .scmignore files to list ignored paths were forced
upon us by historical version control systems, in the name of "the users
expect it". If there weren't such constraints, it would have been far
nicer---we could have just said "if you want to ignore paths, just use the
attributes mechanism and give them the 'ignored' attribute" without having
to have the exclude mechanism.
But we do not live in that ideal world.
Perhaps ls-files is a more suitable home for the feature?
^ permalink raw reply
* Re: [PATCH 00/22] Refactor to accept NUL in commit messages
From: Jeff King @ 2011-10-27 18:13 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Nguyen Thai Ngoc Duy, Ævar Arnfjörð
In-Reply-To: <7vvcrd411x.fsf@alter.siamese.dyndns.org>
On Tue, Oct 25, 2011 at 07:07:38AM -0700, Junio C Hamano wrote:
> Jeff King <peff@peff.net> writes:
>
> > I mean, besides the obvious that UTF-16 is ...
>
> Yes, you could, besides the obvious. But that obvious reason makes it
> sufficiently different that it may not be so outrageous to draw the line
> between it and all the others.
Yeah, and I'm OK with that. It's just not a satisfying answer to give
Windows people who think UTF-16 is a good idea. But at the very least,
it's still unicode. It should be lossless for them to convert to utf8
and back if they want.
Speaking of which, I've been looking at handling diffing of utf-16
files. Right now we generally just consider them binary, which sucks.
It's easy to identify them by BOM in the is_buffer_binary() code, but
that's only part of it. We do an OK job of diffing them, except that:
1. The BOM makes some diffs a little noisier.
2. We split lines on 0x0a. But this byte can appear in other code
points, like 0x010a (Ċ), or the entire entire 0x0a* code point (the
entire Gurmukhi charset).
I'm tempted to detect the UTF-{16,32}{LE,BE} by their BOM, reencode them
to utf8, and then display them in utf8. Is that too gross for us to
consider?
You can kind-of implement this outside of git using textconv. But you
have to manually mark each file as utf-16, as there's no way to trigger
an alternative diff driver on something like a BOM.
I'm really not clear on how people with utf-16 files work. Even if we
did treat utf-16 like text, the _rest_ of git is outputting ascii, so
it's not like their terminals are utf-16. But we do have projects on
github with utf-16 and utf-32 encodings.
-Peff
^ permalink raw reply
* Re: [PATCH] Fix 'Cloning into' message
From: Richard Hartmann @ 2011-10-27 18:34 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vvcraz64i.fsf@alter.siamese.dyndns.org>
On Thu, Oct 27, 2011 at 19:39, Junio C Hamano <gitster@pobox.com> wrote:
> We try to be consistent and many places do quote user supplied paths in
> pair of single quotes in human readable messages, and this is in line with
> that pattern.
Exactly. That is why I chose single quotes. Sorry for not including
the signed-off the first time.
> $ git clone foo "joey's foo"
In that case, the user has other problems ;)
Richard
^ permalink raw reply
* Re: [PATCH/WIP 05/11] symbolize return values of tree_entry_interesting()
From: Junio C Hamano @ 2011-10-27 18:36 UTC (permalink / raw)
To: Nguyễn Thái Ngọc Duy; +Cc: git
In-Reply-To: <1319438176-7304-6-git-send-email-pclouds@gmail.com>
Nguyễn Thái Ngọc Duy <pclouds@gmail.com> writes:
> This helps extending the value later on for "interesting, but cannot
> decide if the entry truely matches yet" (ie. prefix matches)
>
> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Good change; it is a basic code hygiene to avoid magic constants anyway.
> diff --git a/tree-diff.c b/tree-diff.c
> index 6782484..25cc981 100644
> --- a/tree-diff.c
> +++ b/tree-diff.c
> @@ -114,12 +114,13 @@ static void show_entry(struct diff_options *opt, const char *prefix,
> }
>
> static void skip_uninteresting(struct tree_desc *t, struct strbuf *base,
> - struct diff_options *opt, int *match)
> + struct diff_options *opt,
> + enum interesting *match)
> {
> while (t->size) {
> *match = tree_entry_interesting(&t->entry, base, 0, &opt->pathspec);
> if (*match) {
> - if (*match < 0)
> + if (*match == all_entries_not_interesting)
> t->size = 0;
> break;
> }
The caller of this function needs to be updated as well.
But I have to wonder why this skip_uninteresting() does not peek the
original value of *match and skip, which is the loop structure the other
caller of tree_entry_interesting() in this file has.
diff --git a/tree-diff.c b/tree-diff.c
index 25cc981..de4ba28 100644
--- a/tree-diff.c
+++ b/tree-diff.c
@@ -133,7 +133,7 @@ int diff_tree(struct tree_desc *t1, struct tree_desc *t2,
{
struct strbuf base;
int baselen = strlen(base_str);
- int t1_match = 0, t2_match = 0;
+ enum interesting t1_match, t2_match;
/* Enable recursion indefinitely */
opt->pathspec.recursive = DIFF_OPT_TST(opt, RECURSIVE);
@@ -142,6 +142,9 @@ int diff_tree(struct tree_desc *t1, struct tree_desc *t2,
strbuf_init(&base, PATH_MAX);
strbuf_add(&base, base_str, baselen);
+ /* Initialize to something other than all_entries_not_interesting */
+ t1_match = t2_match = entry_not_interesting;
+
for (;;) {
if (diff_can_quit_early(opt))
break;
^ permalink raw reply related
* Re: [PATCH 00/22] Refactor to accept NUL in commit messages
From: Junio C Hamano @ 2011-10-27 18:47 UTC (permalink / raw)
To: Jeff King; +Cc: git, Nguyen Thai Ngoc Duy, Ævar Arnfjörð
In-Reply-To: <20111027181303.GF1967@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> I'm tempted to detect the UTF-{16,32}{LE,BE} by their BOM, reencode them
> to utf8, and then display them in utf8. Is that too gross for us to
> consider?
I tend to think so; it is entirely a different matter if the user
instructed us to clean/smudge UTF-16 payload into/outof UTF-8.
^ permalink raw reply
* Re: [PATCH 00/22] Refactor to accept NUL in commit messages
From: Jeff King @ 2011-10-27 18:52 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Nguyen Thai Ngoc Duy, Ævar Arnfjörð
In-Reply-To: <7v7h3qz2yo.fsf@alter.siamese.dyndns.org>
On Thu, Oct 27, 2011 at 11:47:27AM -0700, Junio C Hamano wrote:
> Jeff King <peff@peff.net> writes:
>
> > I'm tempted to detect the UTF-{16,32}{LE,BE} by their BOM, reencode them
> > to utf8, and then display them in utf8. Is that too gross for us to
> > consider?
>
> I tend to think so; it is entirely a different matter if the user
> instructed us to clean/smudge UTF-16 payload into/outof UTF-8.
Minor nit, but this is just for diff, so it is not about clean/smudge
but rather about doing something like textconv.
The other option I mentioned would be something like detecting the BOM
and pretending as if the attribute "diff=utf-16" was set (which would do
nothing by default). Then people could set themselves up to handle
utf-16 if they wanted, but wouldn't have to go around marking each file
with .gitattributes.
But maybe that is too gross, too, and they should just use
.gitattributes.
-Peff
^ permalink raw reply
* Re: [PATCH 00/22] Refactor to accept NUL in commit messages
From: Junio C Hamano @ 2011-10-27 19:14 UTC (permalink / raw)
To: Jeff King; +Cc: git, Nguyen Thai Ngoc Duy, Ævar Arnfjörð
In-Reply-To: <20111027185220.GA26621@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> On Thu, Oct 27, 2011 at 11:47:27AM -0700, Junio C Hamano wrote:
>
>> Jeff King <peff@peff.net> writes:
>>
>> > I'm tempted to detect the UTF-{16,32}{LE,BE} by their BOM, reencode them
>> > to utf8, and then display them in utf8. Is that too gross for us to
>> > consider?
>>
>> I tend to think so; it is entirely a different matter if the user
>> instructed us to clean/smudge UTF-16 payload into/outof UTF-8.
>
> Minor nit, but this is just for diff, so it is not about clean/smudge
> but rather about doing something like textconv.
I can understand if some tools in the Windows land prefer to work with
these encodings, so clean/smudge to have the checkout in these encodings
would be a reasonable thing not just diff but things like grep. On the
other hand, I do doubt the sanity of these people if they want to have
in-repository representation also in these encodings.
So I do not think "it is just for diff" is any improvement.
^ permalink raw reply
* Repository data loss in fast-export with a merge of a deleted submodule
From: Joshua Jensen @ 2011-10-27 19:27 UTC (permalink / raw)
To: git@vger.kernel.org
Hello.
We had a submodule that we deleted and then added back into the
repository at the same location as the former submodule. When running
fast-export, the newly 'added' files for the merge commit are listed and
then are followed with a:
M ... path/to/submodule/file
D path/to/submodule
On fast-import, the resultant repository becomes corrupt due to the
Delete instruction above occurring AFTER the file adds/modifications.
The new repository does not match the old repository where the
fast-export was performed.
I have included a repro script below. I have not been able to test this
on Git 1.7.7.1, but I have tested on Git 1.7.7 (msysGit version).
Please compare the differences between the generated main.fe and
newmain.fe files. newmain.fe has data loss.
I am not familiar with the fast-export code. Can anyone help out?
Thanks.
Josh
---------
rm -rf main brokenmain sub main.fenewmain.fe
# Create the submodule.
mkdir sub
cd sub
git init
echo file > file
git add file
git commit -m file
cd ..
# Create the main repository.
mkdir main
cd main
git init
# Add the submodule.
git submodule add ../sub sub
git commit -m "Add submodule"
# Remove the submodule.
rm -rf sub
git rm sub .gitmodules
git commit -m "Remove submodule"
# Add sub/file to the master branch.
mkdir sub
echo file > sub/file
git add sub/file
git commit -m "Add sub/file"
if [ -f sub/file ]; then
echo "main: master branch: sub/file exists."
fi
# Delete the submodule directory manually, because we know that the
incoming merge will need it gone.
git checkout -B will-be-broken HEAD^^
rm -rf sub
git merge --no-ff master
# sub/file exists within the 'will-be-broken' branch.
if [ -f sub/file ]; then
echo "main: will-be-broken branch: sub/file exists."
fi
# Export out the main repository.
git fast-export --all > ../main.fe
# Create the brokenmain repository.
cd ..
mkdir brokenmain
cd brokenmain
git init
# Import in everything from the main repository.
git fast-import < ../main.fe
# sub/file exists within the master branch.
git checkout master
if [ -f sub/file ]; then
echo "brokenmain: master branch: sub/file exists."
fi
# sub/file SHOULD exist within the 'will-be-broken' branch but doesn't.
git checkout will-be-broken
if [ ! -f sub/file ]; then
echo "brokenmain: will-be-broken branch: sub/file SHOULD exist but
doesn't."
fi
# Export out the brokenmain repository.
git fast-export --all > ../brokenmain.fe
^ permalink raw reply
* Re: [PATCH 2/2] pack-objects: optimize "recency order"
From: Ævar Arnfjörð Bjarmason @ 2011-10-27 21:01 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Shawn O. Pearce
In-Reply-To: <1310084657-16790-3-git-send-email-gitster@pobox.com>
On Fri, Jul 8, 2011 at 02:24, Junio C Hamano <gitster@pobox.com> wrote:
> - Commits at the tip of tags are written together, in the hope that
> revision traversal done in incremental fetch (which starts by
> putting them in a revision queue marked as UNINTERESTING) will see a
> better locality of these objects;
We recently upgraded to 1.7.7 and I've traced a very large slowdown in
packing down to this commit.
On our repository packing used to take around 30 seconds, it now takes
about 4 minutes. Which means that cloning the repository went from
being slightly slow to pretty intolerable.
I haven't dug into why this is but I'm pretty sure it's because this
patch makes Git behave pathologically on repositories with a large
amount of tags. git.git itself has ~27k revs / and ~450 tags, or a tag
every ~60 revisions.
Try it on e.g. a repository with a couple of hundred thousand revs and
a tag every 10-20 revisions.
^ permalink raw reply
* Re: [PATCH 2/2] pack-objects: optimize "recency order"
From: Ævar Arnfjörð Bjarmason @ 2011-10-27 21:49 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Shawn O. Pearce
In-Reply-To: <CACBZZX6Ss4jLtdrDhLUNKCUjEHjHHKzfv-LMkOyTPDhRUXm4sQ@mail.gmail.com>
On Thu, Oct 27, 2011 at 23:01, Ævar Arnfjörð Bjarmason <avarab@gmail.com> wrote:
> I haven't dug into why this is but I'm pretty sure it's because this
> patch makes Git behave pathologically on repositories with a large
> amount of tags. git.git itself has ~27k revs / and ~450 tags, or a tag
> every ~60 revisions.
>
> Try it on e.g. a repository with a couple of hundred thousand revs and
> a tag every 10-20 revisions.
Actually it just seems slow in general, not just on repositories with
a lot of tags, on linux-2.6.git with this patch:
$ time ~/g/git/git-pack-objects --keep-true-parents
--honor-pack-keep --non-empty --all --reflog
--delta-base-offs</dev/null
/home/avar/g/linux-2.6/.git/objects/pack/.tmp-pack
Counting objects: 2184059, done.
Delta compression using up to 8 threads.
Compressing objects: 100% (338780/338780), done.
130605d5b82492a38452b9bc7bddfb4a6ef0e942
Writing objects: 100% (2184059/2184059), done.
Total 2184059 (delta 1825129), reused 2184059 (delta 1825129)
real 1m28.426s
user 1m19.661s
sys 0m3.008s
With it reverted:
$ time ~/g/git/git-pack-objects --keep-true-parents
--honor-pack-keep --non-empty --all --reflog --delta-base-offset
</dev/null /home/avar/g/linux-2.6/.git/objects/pack/.tmp-pack
Counting objects: 2184059, done.
Delta compression using up to 8 threads.
Compressing objects: 100% (338780/338780), done.
130605d5b82492a38452b9bc7bddfb4a6ef0e942
Writing objects: 100% (2184059/2184059), done.
Total 2184059 (delta 1825129), reused 2184059 (delta 1825129)
real 0m50.152s
user 0m45.367s
sys 0m2.920s
And if you create a lot of tags:
git log --pretty=%h | perl -lne 'chomp(my $sha = <>); print $sha
if $i++ % 10 == 0' | parallel "git tag test-tag-{} {}"
Resulting in:
$ git rev-list HEAD | wc -l
269514
$ git tag -l | wc -l
13733
This'll be the time with this patch reverted, i.e. almost exactly the
same as with just ~250 tags:
real 0m52.860s
user 0m44.907s
sys 0m3.172s
And with it applied, i.e. almost exactly the same as with just ~250
tags:
real 1m29.080s
user 1m21.825s
sys 0m3.096s
^ permalink raw reply
* Re: [PATCH 2/2] pack-objects: optimize "recency order"
From: Ævar Arnfjörð Bjarmason @ 2011-10-27 22:02 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Shawn O. Pearce
In-Reply-To: <CACBZZX6ZWOF=j-k8o-4NHmjS2HpyS+PmKjJh_QKevWurBf9pbA@mail.gmail.com>
On Thu, Oct 27, 2011 at 23:49, Ævar Arnfjörð Bjarmason <avarab@gmail.com> wrote:
> Actually it just seems slow in general, not just on repositories with
> a lot of tags, on linux-2.6.git with this patch:
Here's profiling with gprof for everything with >1% of execution time
with the patch applied:
Each sample counts as 0.01 seconds.
% cumulative self self total
time seconds seconds calls s/call s/call name
21.07 15.99 15.99 2184059 0.00 0.00
add_descendants_to_write_order
20.25 31.35 15.37 1146371554 0.00 0.00 add_to_write_order
11.94 40.41 9.06 142180385 0.00 0.00 hashcmp
5.55 44.62 4.21 90592818 0.00 0.00 lookup_object
4.64 48.14 3.52 72804470 0.00 0.00 hashcmp
3.87 51.08 2.94 90007452 0.00 0.00 get_mode
3.31 53.59 2.51 90007452 0.00 0.00 decode_tree_entry
1.90 55.03 1.44 2184059 0.00 0.00
add_family_to_write_order
1.79 56.39 1.36 43247856 0.00 0.00 hashcmp
1.29 57.37 0.98 pack_offset_sort
1.27 58.33 0.96 90007452 0.00 0.00 update_tree_entry
1.27 59.29 0.96 90592817 0.00 0.00 hashtable_index
1.20 60.20 0.91 4009188 0.00 0.00 find_pack_revindex
1.19 61.10 0.90 5899321 0.00 0.00 find_pack_entry_one
1.12 61.95 0.85 269514 0.00 0.00
commit_list_insert_by_date
1.08 62.77 0.82 5387773 0.00 0.00 patch_delta
And without:
Each sample counts as 0.01 seconds.
% cumulative self self total
time seconds seconds calls s/call s/call name
21.29 9.13 9.13 142180385 0.00 0.00 hashcmp
10.59 13.67 4.54 90592818 0.00 0.00 lookup_object
8.48 17.31 3.64 72638478 0.00 0.00 hashcmp
6.60 20.14 2.83 90007452 0.00 0.00 decode_tree_entry
6.15 22.77 2.64 90007452 0.00 0.00 get_mode
2.99 24.05 1.28 43247182 0.00 0.00 hashcmp
2.96 25.32 1.27 90592817 0.00 0.00 hashtable_index
2.47 26.38 1.06 90007452 0.00 0.00 update_tree_entry
2.26 27.35 0.97 4009188 0.00 0.00 find_pack_revindex
2.05 28.23 0.88 269245 0.00 0.00 process_tree
1.96 29.07 0.84 269514 0.00 0.00
commit_list_insert_by_date
1.94 29.90 0.83 pack_offset_sort
1.73 30.64 0.74 5389900 0.00 0.00 patch_delta
1.70 31.37 0.73 5885588 0.00 0.00 find_pack_entry_one
1.38 31.96 0.59 8692967 0.00 0.00 hashcmp
1.24 32.49 0.53 8175096 0.00 0.00
unpack_object_header_buffer
1.14 32.98 0.49 1 0.49 0.59 write_idx_file
1.12 33.46 0.48 5885588 0.00 0.00
nth_packed_object_offset
1.12 33.94 0.48 6051632 0.00 0.00
locate_object_entry_hash
^ permalink raw reply
* Re: [PATCH 2/2] pack-objects: optimize "recency order"
From: Junio C Hamano @ 2011-10-27 22:05 UTC (permalink / raw)
To: Ævar Arnfjörð Bjarmason; +Cc: git, Shawn O. Pearce
In-Reply-To: <CACBZZX6Ss4jLtdrDhLUNKCUjEHjHHKzfv-LMkOyTPDhRUXm4sQ@mail.gmail.com>
Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:
> We recently upgraded to 1.7.7 and I've traced a very large slowdown in
> packing down to this commit.
Does Dan McGee's series leading to 38d4deb (pack-objects: don't traverse
objects unnecessarily, 2011-10-18) help?
^ permalink raw reply
* Re: [PATCH 4/4] pack-objects: rewrite add_descendants_to_write_order() iteratively
From: Junio C Hamano @ 2011-10-27 22:13 UTC (permalink / raw)
To: Dan McGee; +Cc: git
In-Reply-To: <1318915284-6361-4-git-send-email-dpmcgee@gmail.com>
Dan McGee <dpmcgee@gmail.com> writes:
> This removes the need to call this function recursively, shinking the
> code size slightly and netting a small performance increase.
>
> Signed-off-by: Dan McGee <dpmcgee@gmail.com>
Tricky.
As long as this is done after compute_write_order() populates
delta_sibling vs delta link in a consistent way, the new logic should
produce exactly the same result as the original code.
Thanks.
^ permalink raw reply
* git alias and --help
From: Gelonida N @ 2011-10-27 22:20 UTC (permalink / raw)
To: git
I'm having a tiny question about git aliases.
in ~/.gitconfig I defined an alias
b = branch
This works well execp if I type
git b --help
Now I get the output
`git b' is aliased to `branch'
which is nice to know however what I reall wanted is to get the helptext
for the branch command.
Is there any trick to get the help text of git branch without
having to type
git branch --help
^ 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