* Re: [PATCH] completion: Add PS1 configuration for submodules
From: Kevin Ballard @ 2010-12-07 20:41 UTC (permalink / raw)
To: Scott Kyle; +Cc: Ævar Arnfjörð Bjarmason, git
In-Reply-To: <AANLkTinuD2ze_mn5QMLqFaoftwZvTsz-DKv1ojAizg7P@mail.gmail.com>
On Dec 7, 2010, at 12:37 PM, Scott Kyle wrote:
> On Tue, Dec 7, 2010 at 4:15 AM, Ævar Arnfjörð Bjarmason
> <avarab@gmail.com> wrote:
>>
>> On Tue, Dec 7, 2010 at 00:22, Scott Kyle <scott@appden.com> wrote:
>>> For those who often work on repositories with submodules, the dirty
>>> indicator for unstaged changes will almost always show because development
>>> is simultaneously happening on those submodules. The config option
>>> diff.ignoreSubmodules is not appropriate for this use because it has larger
>>> implications.
>>
>> Wouldn't it be a lot better to instead add support for showing
>> submodule dirtyness as distinct from the main tree's dirtyness? Then
>> you could easily spot if you had either your tree / submodule tree
>> changes, without just ignoring them.
>
> I considered that, but thought it to be a rather disruptive change,
> and one that conceptually didn't work. The way I see it, either
> somebody thinks of their repo as dirty when the submodules are dirty,
> or not. And I think since this behavior has perpetuated for so long,
> most users are content with how it currently works. I, however, was
> not, and so that is why I added an option for people like me.
The big win for such a change, from my perspective, is it tells me if I need
to do a `git submodule update --recursive`, or if I actually have dirty changes.
Because of that, if nobody else picks this up, I'll probably write a patch
to introduce such a config at some point in the future. But as I said before,
that's something that can be done later and doesn't need to affect this patch.
-Kevin Ballard
^ permalink raw reply
* Re: [RFC/PATCH 2/1] bash: eliminate dependency on bash_completion lib
From: SZEDER Gábor @ 2010-12-07 20:41 UTC (permalink / raw)
To: Jonathan Nieder
Cc: Peter van der Does, Shawn O. Pearce, git, Marc Branchaud,
Brian Gernhardt, Kevin Ballard, Mathias Lafeldt
In-Reply-To: <20101207194923.GD22587@burratino>
Hi Jonathan,
On Tue, Dec 07, 2010 at 01:49:23PM -0600, Jonathan Nieder wrote:
> SZEDER Gábor wrote:
> > On Thu, Dec 02, 2010 at 03:02:07PM -0600, Jonathan Nieder wrote:
>
> >> Add a minimal implementation of _get_comp_words_by_ref,
> >> the routine used to work around bash 4.0's COMP_WORDS semantics.
> >>
> >> Based on bash-completion 2.x (commit bf763033, 2010-10-26) but
> >> tweaked for simplicity and to allow zsh to at least parse the
> >> code.
> >
> > What is this patch based on?
>
> Ah, sorry, applies to d93f4a297 (bash: work around bash 4.0 change in
> COMP_WORDS semantics, 2010-12-02).
In which repo? ;)
I don't have d93f4a297, but I have e0a9590 (Introduce functions from
bash-completion project., 2010-12-01) and c7e75bb (Use the new
functions to get the current cword., 2010-12-01) instead, which were
merged into pu at 0c30752 (Merge branch 'pd/bash-4-completion' into
pu, 2010-12-03).
Hm, waittaminit... Ah, OK, nevermind, got it. There was a patch in
http://article.gmane.org/gmane.comp.version-control.git/162686
I forgot to apply first... Now I have it all, seems to work so far.
Thanks,
Gábor
^ permalink raw reply
* Re: [PATCH] completion: Add PS1 configuration for submodules
From: Kevin Ballard @ 2010-12-07 20:31 UTC (permalink / raw)
To: Ævar Arnfjörð Bjarmason; +Cc: Scott Kyle, git
In-Reply-To: <AANLkTin8JstkjEWaCGZuqpEXZnLMyudFm24K7Y3iCgX6@mail.gmail.com>
On Dec 7, 2010, at 4:15 AM, Ævar Arnfjörð Bjarmason wrote:
> On Tue, Dec 7, 2010 at 00:22, Scott Kyle <scott@appden.com> wrote:
>> For those who often work on repositories with submodules, the dirty
>> indicator for unstaged changes will almost always show because development
>> is simultaneously happening on those submodules. The config option
>> diff.ignoreSubmodules is not appropriate for this use because it has larger
>> implications.
>
> Wouldn't it be a lot better to instead add support for showing
> submodule dirtyness as distinct from the main tree's dirtyness? Then
> you could easily spot if you had either your tree / submodule tree
> changes, without just ignoring them.
That sounds like a good idea, but it doesn't necessarily have to come with
this patch. Scott's use case here is he has a submodule that is _always_ dirty,
and he simply doesn't want to see that stuff in the PS1. Having an option to
show it separately would be very useful for me, but should perhaps be written
as a separate patch.
-Kevin Ballard
^ permalink raw reply
* Re: [PATCH] completion: Add PS1 configuration for submodules
From: Scott Kyle @ 2010-12-07 20:37 UTC (permalink / raw)
To: Ævar Arnfjörð Bjarmason; +Cc: git
In-Reply-To: <AANLkTin8JstkjEWaCGZuqpEXZnLMyudFm24K7Y3iCgX6@mail.gmail.com>
[sorry for the duplicate email, my original was rejected from the list]
On Tue, Dec 7, 2010 at 4:15 AM, Ævar Arnfjörð Bjarmason
<avarab@gmail.com> wrote:
>
> On Tue, Dec 7, 2010 at 00:22, Scott Kyle <scott@appden.com> wrote:
> > For those who often work on repositories with submodules, the dirty
> > indicator for unstaged changes will almost always show because development
> > is simultaneously happening on those submodules. The config option
> > diff.ignoreSubmodules is not appropriate for this use because it has larger
> > implications.
>
> Wouldn't it be a lot better to instead add support for showing
> submodule dirtyness as distinct from the main tree's dirtyness? Then
> you could easily spot if you had either your tree / submodule tree
> changes, without just ignoring them.
I considered that, but thought it to be a rather disruptive change,
and one that conceptually didn't work. The way I see it, either
somebody thinks of their repo as dirty when the submodules are dirty,
or not. And I think since this behavior has perpetuated for so long,
most users are content with how it currently works. I, however, was
not, and so that is why I added an option for people like me.
Scott Kyle
http://appden.com
http://github.com/appden
http://twitter.com/appden
^ permalink raw reply
* Re: cmd_cherry in builtin/log.c?
From: Junio C Hamano @ 2010-12-07 20:30 UTC (permalink / raw)
To: René Scharfe; +Cc: Jonathan Nieder, Thiago Farina, Git Mailing List
In-Reply-To: <4CFE83A2.8070004@lsrfire.ath.cx>
René Scharfe <rene.scharfe@lsrfire.ath.cx> writes:
> Am 07.12.2010 18:39, schrieb Jonathan Nieder:
>> while the core of the builtin version is
>>
>> get_patch_ids(&revs, &patch_id_opts, prefix);
>>
>> The latter function is static, introduced by v1.4.1~12^2~5
>> (format-patch: introduce "--ignore-if-in-upstream", 2006-06-25).
>>
>> So the answer is that "git cherry" is considered a variant on
>> "git log" (like format-patch, show, and whatchanged) and that it uses
>> "git log" internals.
>
> That's right. get_patch_ids() could be moved into patch-ids.c now and
> then cmd_cherry() could get its own file in builtin/, though.
Right, but the key word is "could". Is it hurting _anything_ to have it
in the current place? I doubt it.
^ permalink raw reply
* Re: [PATCH] logging branch deletion to help recovering from mistakes
From: Junio C Hamano @ 2010-12-07 20:25 UTC (permalink / raw)
To: Jonathan Nieder; +Cc: Casey Dahlin, Junio C Hamano, git
In-Reply-To: <20101207180236.GC21483@burratino>
Jonathan Nieder <jrnieder@gmail.com> writes:
> Casey Dahlin wrote:
>> On Tue, Dec 07, 2010 at 11:45:20AM -0600, Jonathan Nieder wrote:
>>> Casey Dahlin wrote:
>
>>>> Could commits made onto a detached head also show up here? Or is that
>>>> better thwarted with another mechanism?
>>>
>>> I think that's better thwarted with the HEAD reflog:
>>>
>>> $ git log -g HEAD
>>
>> I was more worried about changes that were made onto a detached head,
>> and then the head was reattached, leaving the new commits dangling.
>
> But isn't that exactly what a detached HEAD is for? If one wants
> the experiments one does on detached HEAD to be kept around "just
> in case", wouldn't it make more sense to give them a (branch) name so
> they can be separated from one another?
What are you arguing after giving a correct answer. "git log -g HEAD"
keeps track of what was at the tip of HEAD, be it pointing at a branch or
pointing diretly at a commit in a detached state, no?
^ permalink raw reply
* Re: [PATCH] Corrected return values in post-receive-email.prep_for_email
From: Kevin P. Fleming @ 2010-12-07 19:44 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Alan Raison, git
In-Reply-To: <7v1v5tqswl.fsf@alter.siamese.dyndns.org>
On 12/07/2010 01:34 PM, Junio C Hamano wrote:
> "Alan Raison"<alan@theraisons.me.uk> writes:
>
>> ---
>> contrib/hooks/post-receive-email | 6 +++---
>> 1 files changed, 3 insertions(+), 3 deletions(-)
>
> No sign-off, no description.
>
> This is a regression introduced by 53cad69 (post-receive-email: ensure
> sent messages are not empty, 2010-09-10), I think.
>
>> diff --git a/contrib/hooks/post-receive-email
>> b/contrib/hooks/post-receive-email
>> index 85724bf..020536d 100755
>> --- a/contrib/hooks/post-receive-email
>> +++ b/contrib/hooks/post-receive-email
>> @@ -150,7 +150,7 @@ prep_for_email()
>> # Anything else (is there anything else?)
>> echo>&2 "*** Unknown type of update to $refname
>> ($rev_type)"
>> echo>&2 "*** - no email generated"
>> - return 0
>> + return 1
>
> This used to "exit 1" before 53cad69 and I agree with the patch that
> signalling error with "return 1" is the right thing to do here.
>
>> ;;
>> esac
>>
>> @@ -166,10 +166,10 @@ prep_for_email()
>> esac
>> echo>&2 "*** $config_name is not set so no email will be
>> sent"
>> echo>&2 "*** for $refname update $oldrev->$newrev"
>> - return 0
>> + return 1
>
> This used to "exit 0" before 53cad69 to cause the program stop before
> sending mails. Again, I agree with the patch that signalling error is the
> right thing to do here.
>
>> fi
>>
>> - return 1
>> + return 0
>
> And this obviously is correct.
>
> Kevin, care to review and Ack? Alan, care to add a few lines of patch
> description and sign-off?
Acked-by: Kevin P. Fleming <kpfleming@digium.com>
Yeah, this is clearly my breakage; our internal version of this script
is so different that it has become hard to backport fixes to the
upstream version... or I just did a terrible job of it.
Alan, while you are in there fixing this, there is a remaining 'exit 0'
in prep_for_email (at line 147) that should be 'return 1' instead.
--
Kevin P. Fleming
Digium, Inc. | Director of Software Technologies
445 Jan Davis Drive NW - Huntsville, AL 35806 - USA
skype: kpfleming | jabber: kfleming@digium.com
Check us out at www.digium.com & www.asterisk.org
^ permalink raw reply
* Re: git fetch vs push, git am questions
From: Neal Kreitzinger @ 2010-12-07 20:14 UTC (permalink / raw)
To: git
In-Reply-To: <99351291667275@web152.yandex.ru>
"Konstantin Kivi" <kkivi@yandex.ru> wrote in message
news:99351291667275@web152.yandex.ru...
> HI all
> I honestly tried to find answers in various docs/books about git and
> failed, so I have to ask here.
>
> 1) Is it possible to achieve the same result with git fetch, as I have
> with git push. I have cloned a bare repository (2) from my repository (1)
> and
> I want fetch made in (2) get all info contained in (1) . I talk only about
> 'master' here. git push from (1) to (2) does the job, but git fetch in (1)
> updates only origin/master, and not master.
> I also found that there is a notions of current branch in bare repository
> (master in my case), that stops my experiments of deleting master and
> making new master out of origin/master.
> How can I change current branch in bare repositry?
>
> 2) I read that git rebase used 'git am' internally. How can force git
> am/apply to create conflicts like rebase does in form
>
> <<<<<<old
> dksjfd
> ========
>>>>>>jfkjdfkd
>
> instead of *.rej files
>
I think the --mirror technology was made for what you are trying to do. I
created my bare repo as a mirror, but I see you don't want to recreate your
bare repo. However, I think you can convert yours to a mirror by modding
your config for that repo. With a bare repo that is a mirror, a git fetch
will make the bare repo look exactly like the remote repo it is mirroring.
That is, the mirror will get everything that the remote has. If the mirror
has extra branches that the remote does not have then the mirror will also
keep the extra branches as is. However, I think if you push from the mirror
to the remote the remote will get those extra branches so watch out for that
(I haven't tried it yet because I don't need to do that.)
You could play around with it on a testcopy of your bare repo:
cp -rp barerepo.git testmirror.git
will make a totally functional copy of your barerepo.git. Then you could
mod the config of testmirror.git to try and convert it to a mirror. The
mirror technology is on the git-clone manpage. It doesn't explain it in
much detail, but this is what I've learned from playing around with it. You
can first create a bare repo mirror with git-clone to find out what the
config setting are supposed to be.
For the push you can create a test copy of your remote:
cp -rp remote testremote
and then alter the config of testmirror.git to point to testremote. If you
are pushing from testremote to testmirror.git then be sure to mod
testremote's config to point to testmirror.git. I don't push from my remote
to my mirror because I use git fetch to get stuff from the remote to the
mirror. You shouldn't need to git push from your remote to your mirror,
either. However, you may have valid reasons for doing so, but I'm not sure
what they would be.
I am using a mirror very heavily in productional work so I know it works
(for what I want). I recommend you play around with it for yourself to get
a feel for it and not just go by what I say (the manpage does not explain it
very much).
Hope this helps.
v/r,
Neal
^ permalink raw reply
* Re: [PATCH 1/2] describe: Use for_each_rawref
From: Junio C Hamano @ 2010-12-07 19:49 UTC (permalink / raw)
To: Anders Kaseorg
Cc: Junio C Hamano, SZEDER Gábor, Jonathan Nieder, git,
Kirill Smelkov, Thomas Rast
In-Reply-To: <alpine.DEB.2.02.1012071325100.23348@dr-wily.mit.edu>
Anders Kaseorg <andersk@ksplice.com> writes:
> On Tue, 7 Dec 2010, Anders Kaseorg wrote:
>> Signed-off-by: Anders Kaseorg <andersk@ksplice.com>
>>
>> diff --git a/builtin/describe.c b/builtin/describe.c
>
> (Gaaah, sorry, I accidentally used format-patch -p here. Won’t happen
> again. :-) )
The default will work just fine, no?
Also labelling them like "[PATCH v2 1/2]" would have been nicer to spot
the progress of the series.
Thanks.
^ permalink raw reply
* Re: [RFC/PATCH 2/1] bash: eliminate dependency on bash_completion lib
From: Jonathan Nieder @ 2010-12-07 19:49 UTC (permalink / raw)
To: SZEDER Gábor
Cc: Peter van der Does, Shawn O. Pearce, git, Marc Branchaud,
Brian Gernhardt, Kevin Ballard, Mathias Lafeldt
In-Reply-To: <20101207160747.GD1867@neumann>
SZEDER Gábor wrote:
> On Thu, Dec 02, 2010 at 03:02:07PM -0600, Jonathan Nieder wrote:
>> Add a minimal implementation of _get_comp_words_by_ref,
>> the routine used to work around bash 4.0's COMP_WORDS semantics.
>>
>> Based on bash-completion 2.x (commit bf763033, 2010-10-26) but
>> tweaked for simplicity and to allow zsh to at least parse the
>> code.
>
> What is this patch based on?
Ah, sorry, applies to d93f4a297 (bash: work around bash 4.0 change in
COMP_WORDS semantics, 2010-12-02).
Can re-send in a few moments based on maint if you'd like.
^ permalink raw reply
* Re: [PATCH] logging branch deletion to help recovering from mistakes
From: Jeff King @ 2010-12-07 19:38 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Nguyen Thai Ngoc Duy, git
In-Reply-To: <7v7hflqth1.fsf@alter.siamese.dyndns.org>
On Tue, Dec 07, 2010 at 11:21:46AM -0800, Junio C Hamano wrote:
> I am more worried about stuff in branch.<name>.* that are discarded upon
> "branch -d". Without the config items, you won't have a working:
>
> $ branch -d frotz
> $ branch --undelete frotz
> $ git checkout frotz
> $ git pull
Hmm, yeah, I didn't think about that. Two possible solutions:
1. Just leave it in .git/config. It is not hurting anything if the
branch does not exist, but it is cruft in a file the user might
look at.
2. Drop it into .git/config.dead/<branch_name>. When resurrecting a
branch, copy it back into .git/config.
In both cases, when the reflog for the deleted branch is pruned to
nothing, we delete the relevant config, too.
In the second case, I think you would have to take special care for
something like:
$ git branch frotz origin/master
$ git branch -d frotz
$ git remote rename origin foo
$ git branch --undelete frotz
In the non-deleted case, this transparently renames branch.frotz.remote
from "origin" to "foo". In the deleted case, we would need to make sure
the dead config is updated, too.
To be honest, I have never been that interested in a "branch --undelete"
feature. I much more care about leaving the reflogs of deleted branches
around, so I can "git checkout -b foo bar@{1}" later on[1]. That is, to
me branch undeletion is not about bringing a branch back wholesale, but
rather remembering commits so I can start a new branch there.
But I guess others might disagree.
-Peff
[1] Well, that and just piece of mind from knowing that "branch -d" is
not totally unrecoverable. Specifically, if we kept deleted reflogs
around, it would be safe(r) to turn on auto-prune on fetch, the lack
of which is something that seems to confuse new users.
^ permalink raw reply
* Re: [PATCH] Corrected return values in post-receive-email.prep_for_email
From: Junio C Hamano @ 2010-12-07 19:34 UTC (permalink / raw)
To: Alan Raison; +Cc: git, Kevin P. Fleming
In-Reply-To: <002501cb962c$5fa3aa40$1eeafec0$@me.uk>
"Alan Raison" <alan@theraisons.me.uk> writes:
> ---
> contrib/hooks/post-receive-email | 6 +++---
> 1 files changed, 3 insertions(+), 3 deletions(-)
No sign-off, no description.
This is a regression introduced by 53cad69 (post-receive-email: ensure
sent messages are not empty, 2010-09-10), I think.
> diff --git a/contrib/hooks/post-receive-email
> b/contrib/hooks/post-receive-email
> index 85724bf..020536d 100755
> --- a/contrib/hooks/post-receive-email
> +++ b/contrib/hooks/post-receive-email
> @@ -150,7 +150,7 @@ prep_for_email()
> # Anything else (is there anything else?)
> echo >&2 "*** Unknown type of update to $refname
> ($rev_type)"
> echo >&2 "*** - no email generated"
> - return 0
> + return 1
This used to "exit 1" before 53cad69 and I agree with the patch that
signalling error with "return 1" is the right thing to do here.
> ;;
> esac
>
> @@ -166,10 +166,10 @@ prep_for_email()
> esac
> echo >&2 "*** $config_name is not set so no email will be
> sent"
> echo >&2 "*** for $refname update $oldrev->$newrev"
> - return 0
> + return 1
This used to "exit 0" before 53cad69 to cause the program stop before
sending mails. Again, I agree with the patch that signalling error is the
right thing to do here.
> fi
>
> - return 1
> + return 0
And this obviously is correct.
Kevin, care to review and Ack? Alan, care to add a few lines of patch
description and sign-off?
Thanks.
^ permalink raw reply
* Re: [PATCH] logging branch deletion to help recovering from mistakes
From: Junio C Hamano @ 2010-12-07 19:21 UTC (permalink / raw)
To: Jeff King; +Cc: Nguyen Thai Ngoc Duy, git
In-Reply-To: <20101207170623.GB21749@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> On Mon, Dec 06, 2010 at 10:28:53PM -0800, Junio C Hamano wrote:
>
> Yeah, I think this is not _quite_ what people want in this area. A base
> requirement from past discussions, I think, is that the whole reflog of
> the deleted branch be saved rather than just the tip. And then "reflog
> show" would make a lot more sense on such saved reflogs.
I am more worried about stuff in branch.<name>.* that are discarded upon
"branch -d". Without the config items, you won't have a working:
$ branch -d frotz
$ branch --undelete frotz
$ git checkout frotz
$ git pull
I would say it is fine to discard old reflog for "frotz" branch and tell
users of --undelete that even though their branch is undeleted, the reflog
for it is already expired when they deleted it the first time, but it is
impossible to implement "branch --undelete" without stashing away stuff
other than reflog.
^ permalink raw reply
* Re: [PATCH] git-rm.txt: Fix quoting
From: Junio C Hamano @ 2010-12-07 19:12 UTC (permalink / raw)
To: Michael J Gruber; +Cc: git, Jeff King
In-Reply-To: <73d13b752212b557c0dc157edea9a62122840e93.1291712241.git.git@drmicha.warpmail.net>
Michael J Gruber <git@drmicha.warpmail.net> writes:
> -Using "git commit -a"
> +Using ``git commit -a''
> ~~~~~~~~~~~~~~~~~~~~~
Can you keep the length of the underline the same here, or do you need to
extend it?
^ permalink raw reply
* Re: [PATCH] Improved error messages when temporary file creation fails
From: Jonathan Nieder @ 2010-12-07 19:09 UTC (permalink / raw)
To: Arnout Engelen; +Cc: git
In-Reply-To: <20101207181633.GF25767@bzzt.net>
Arnout Engelen wrote:
> + die_errno("Unable to create temporary file '%s' at %s",
> + template, getcwd(NULL, 0));
This is a Linux libc/glibc-specific extension, alas. On other platforms
it would print "(null)" or segfault.
Here's some other assorted tweaks. I didn't bother to find your old
patch in the mailing list archive to take a fuller change description
from.
Hope that helps,
Jonathan
---
diff --git a/test-mktemp.c b/test-mktemp.c
index d392fa7..2e3b134 100644
--- a/test-mktemp.c
+++ b/test-mktemp.c
@@ -7,10 +7,9 @@
int main(int argc, char *argv[])
{
- if (argc != 2) {
+ if (argc != 2)
usage("Expected 1 parameter defining the temporary file template");
- }
- xmkstemp(strdup(argv[1]));
+ xmkstemp(xstrdup(argv[1]));
return 0;
}
diff --git a/wrapper.c b/wrapper.c
index 6640c87..cb9c9ad 100644
--- a/wrapper.c
+++ b/wrapper.c
@@ -197,19 +197,18 @@ FILE *xfdopen(int fd, const char *mode)
int xmkstemp(char *template)
{
int fd;
- char origtemplate[255];
- strlcpy(origtemplate, template, 255);
+ char origtemplate[256];
+ strlcpy(origtemplate, template, sizeof(origtemplate));
fd = mkstemp(template);
if (fd < 0) {
+ int saved_errno = errno;
+
if (!template[0])
template = origtemplate;
-
- if (is_absolute_path(template))
- die_errno("Unable to create temporary file '%s'", template);
- else
- die_errno("Unable to create temporary file '%s' at %s",
- template, getcwd(NULL, 0));
+ template = make_nonrelative_path(template);
+ errno = saved_errno;
+ die_errno("Unable to create temporary file '%s'", template);
}
return fd;
}
@@ -330,19 +329,18 @@ int gitmkstemps(char *pattern, int suffix_len)
int xmkstemp_mode(char *template, int mode)
{
int fd;
- char origtemplate[255];
- strlcpy(origtemplate, template, 255);
+ char origtemplate[256];
+ strlcpy(origtemplate, template, sizeof(origtemplate));
fd = git_mkstemp_mode(template, mode);
if (fd < 0) {
+ int saved_errno = errno;
+
if (!template[0])
template = origtemplate;
-
- if (is_absolute_path(template))
- die_errno("Unable to create temporary file '%s'", template);
- else
- die_errno("Unable to create temporary file '%s' at %s",
- template, getcwd(NULL, 0));
+ template = make_nonrelative_path(template);
+ errno = saved_errno;
+ die_errno("Unable to create temporary file '%s'", template);
}
return fd;
}
^ permalink raw reply related
* Re: cmd_cherry in builtin/log.c?
From: René Scharfe @ 2010-12-07 18:57 UTC (permalink / raw)
To: Jonathan Nieder; +Cc: Thiago Farina, Git Mailing List
In-Reply-To: <20101207173923.GA21483@burratino>
Am 07.12.2010 18:39, schrieb Jonathan Nieder:
> while the core of the builtin version is
>
> get_patch_ids(&revs, &patch_id_opts, prefix);
>
> The latter function is static, introduced by v1.4.1~12^2~5
> (format-patch: introduce "--ignore-if-in-upstream", 2006-06-25).
>
> So the answer is that "git cherry" is considered a variant on
> "git log" (like format-patch, show, and whatchanged) and that it uses
> "git log" internals.
That's right. get_patch_ids() could be moved into patch-ids.c now and
then cmd_cherry() could get its own file in builtin/, though.
René
^ permalink raw reply
* Re: [PATCH] logging branch deletion to help recovering from mistakes
From: Shawn O. Pearce @ 2010-12-07 18:37 UTC (permalink / raw)
To: Jeff King; +Cc: Junio C Hamano, Nguyen Thai Ngoc Duy, git
In-Reply-To: <20101207183507.GA27277@sigill.intra.peff.net>
Jeff King <peff@peff.net> wrote:
> On Tue, Dec 07, 2010 at 10:23:42AM -0800, Shawn O. Pearce wrote:
>
> > Yea, I'm leaning more towards the foo..deleted-n idea too, for the
> > same reasons. It also makes it easier to GC a deleted branch's
> > reflog, we can examine the last record's timestamp in a reasonable
> > time bound and unlink the log if its really freaking old.
>
> Do we need to actually do that? Shouldn't the entries in the reflog get
> expired as part of the regular reflog gc? In that case, we would just
> delete the file when it had zero entries.
Yes, you are right. We should instead let the normal reflog expire
action do its work here, and delete the empty log file when it is
finally empty.
I guess we also need repack and prune to enumerate these deleted
reflogs and retain the objects their records point to.
--
Shawn.
^ permalink raw reply
* Re: [PATCH] logging branch deletion to help recovering from mistakes
From: Shawn O. Pearce @ 2010-12-07 18:41 UTC (permalink / raw)
To: Jeff King; +Cc: Junio C Hamano, Nguyen Thai Ngoc Duy, git
In-Reply-To: <20101207183930.GA27340@sigill.intra.peff.net>
Jeff King <peff@peff.net> wrote:
> On Tue, Dec 07, 2010 at 10:37:39AM -0800, Shawn O. Pearce wrote:
>
> > Yes, you are right. We should instead let the normal reflog expire
> > action do its work here, and delete the empty log file when it is
> > finally empty.
> >
> > I guess we also need repack and prune to enumerate these deleted
> > reflogs and retain the objects their records point to.
>
> Definitely. I sort of assumed all of those things just traversed
> .git/logs blindly without regard to whether there was a ref, which would
> handle this automagically. But maybe that is not the case.
I think those enumerate the logs of refs that are also being
traversed. Which means we would need to add new logic to enumerate
the deleted reflogs.
> Is there a reason to require that each log is specifically tied to a
> ref?
Historical bad assumptions?
I mean, no, there really isn't a good reason that each log is
tied to a ref. Its probably reasonable to just enumerate the logs
directory separate from the refs directory enumeration. Its just
some more code. Right now we discover logs by just relying on the
ref directory traversal code.
--
Shawn.
^ permalink raw reply
* Re: [PATCH] describe: Don’t look up commits with --exact-match
From: Junio C Hamano @ 2010-12-07 18:39 UTC (permalink / raw)
To: Anders Kaseorg
Cc: Jonathan Nieder, git, SZEDER Gábor, Kirill Smelkov,
Thomas Rast
In-Reply-To: <alpine.DEB.2.02.1012061159500.23348@dr-wily.mit.edu>
Anders Kaseorg <andersk@ksplice.com> writes:
> From 2ad1e58b8f6e9c117c77748b6e8b85227d9d5412 Mon Sep 17 00:00:00 2001
> From: Anders Kaseorg <andersk@ksplice.com>
> Subject: [PATCH 1/2] describe: Use for_each_rawref
>
> Don’t waste time checking for dangling refs; they wouldn’t affect the
> output of ‘git describe’ anyway. Although this doesn’t gain much
> performance by itself, it does in conjunction with the next commit.
>
> Signed-off-by: Anders Kaseorg <andersk@ksplice.com>
> ---
> builtin/describe.c | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/builtin/describe.c b/builtin/describe.c
> index 43caff2..700f740 100644
> --- a/builtin/describe.c
> +++ b/builtin/describe.c
> @@ -418,7 +418,7 @@ int cmd_describe(int argc, const char **argv, const char *prefix)
> return cmd_name_rev(i + argc, args, prefix);
> }
>
> - for_each_ref(get_name, NULL);
> + for_each_rawref(get_name, NULL);
> if (!found_names && !always)
> die("No names found, cannot describe anything.");
It is Ok during a review cycle that is not meant as the final submission,
but please follow Documentation/SubmittingPatches for the final round
(i.e. not multiple-changes in a single mail), as mentioned by Jonathan.
> From ce8a2ab9cf80247c2834d21f36f63cedb794e62f Mon Sep 17 00:00:00 2001
> From: Anders Kaseorg <andersk@ksplice.com>
> Subject: describe: Don’t look up commits with --exact-match
>
> This makes ‘git describe --exact-match HEAD’ about 15 times faster on
> a cold cache (2.3s instead of 35s) in a linux-2.6 repository with many
> packed tags. That’s a huge win for the interactivity of the __git_ps1
> shell prompt helper.
Good description of the motivation. Care to describe how it is achieved?
I may be misreading your intention, but my understanding of the thinking
behind what the new code does is...
- When the object name given from the command line happens to match one
of the tags exactly (either a lightweight tag in which case we can just
compare the object names to detect the match, or an annotated tag in
which case we only need to parse the tag but not the underlying commit
to do the comparison), we do not have to parse any commit object.
- Other cases we would need to parse commit objects and traverse the
history.
- Hence, the code can be optimized for the case to describe an exact
match by trying exact object name match first and then loading commit
objects only when no exact matches found.
- When we are only looking for exact matches (which is _not_ a rare
corner case), we won't ever fall back to the "parse commit objects and
traverse the history" codepath. So it is important to lazily parse
commit objects in order to optimize for this special case.
I however think this may hurt when more than one objects are asked to be
described and there is no exact match. The fallback codepath that lazily
runs lookup_commit_reference_gently() and uses replace_name() to compute
the best name needs to run only once, but this part of the code seems to
run it for the elements on the names list once per describe() invocation
that needs to fall back to this codepath. Would it remove this regression
if we make it run only once?
I don't think this affects correctness, though.
> @@ -259,6 +255,12 @@ static void describe(const char *arg, int last_one)
> if (debug)
> fprintf(stderr, "searching to describe %s\n", arg);
>
> + for (e = names; e; e = e->next) {
> + struct commit *c = lookup_commit_reference_gently(e->peeled, 1);
> + if (c && replace_name(c->util, e->prio, e->sha1, &e->tag))
> + c->util = e;
> + }
> +
> list = NULL;
> cmit->object.flags = SEEN;
> commit_list_insert(cmit, &list);
As you can see from its "if (!e->tag)" codepath, replace_name() is not
designed to be called excessively (e.g. we will end up running the
codepath for an annotated tag that we should know we won't find a tag).
Also, "struct tag *tag" does not need to exist in add_to_known_names()
anymore; a NULL is assigned to it and its only use is to get assigned
to e->tag.
^ permalink raw reply
* Re: [PATCH] logging branch deletion to help recovering from mistakes
From: Jeff King @ 2010-12-07 18:39 UTC (permalink / raw)
To: Shawn O. Pearce; +Cc: Junio C Hamano, Nguyen Thai Ngoc Duy, git
In-Reply-To: <20101207183739.GB3725@spearce.org>
On Tue, Dec 07, 2010 at 10:37:39AM -0800, Shawn O. Pearce wrote:
> Yes, you are right. We should instead let the normal reflog expire
> action do its work here, and delete the empty log file when it is
> finally empty.
>
> I guess we also need repack and prune to enumerate these deleted
> reflogs and retain the objects their records point to.
Definitely. I sort of assumed all of those things just traversed
.git/logs blindly without regard to whether there was a ref, which would
handle this automagically. But maybe that is not the case.
Is there a reason to require that each log is specifically tied to a
ref?
-Peff
^ permalink raw reply
* Re: [PATCH] logging branch deletion to help recovering from mistakes
From: Jeff King @ 2010-12-07 18:35 UTC (permalink / raw)
To: Shawn O. Pearce; +Cc: Junio C Hamano, Nguyen Thai Ngoc Duy, git
In-Reply-To: <20101207182342.GA3725@spearce.org>
On Tue, Dec 07, 2010 at 10:23:42AM -0800, Shawn O. Pearce wrote:
> Yea, I'm leaning more towards the foo..deleted-n idea too, for the
> same reasons. It also makes it easier to GC a deleted branch's
> reflog, we can examine the last record's timestamp in a reasonable
> time bound and unlink the log if its really freaking old.
Do we need to actually do that? Shouldn't the entries in the reflog get
expired as part of the regular reflog gc? In that case, we would just
delete the file when it had zero entries.
-Peff
^ permalink raw reply
* Re: [PATCH 1/2] describe: Use for_each_rawref
From: Anders Kaseorg @ 2010-12-07 18:26 UTC (permalink / raw)
To: Junio C Hamano
Cc: SZEDER Gábor, Jonathan Nieder, git, Kirill Smelkov,
Thomas Rast
In-Reply-To: <alpine.DEB.2.02.1012071320240.23348@dr-wily.mit.edu>
On Tue, 7 Dec 2010, Anders Kaseorg wrote:
> Signed-off-by: Anders Kaseorg <andersk@ksplice.com>
>
> diff --git a/builtin/describe.c b/builtin/describe.c
(Gaaah, sorry, I accidentally used format-patch -p here. Won’t happen
again. :-) )
Anders
^ permalink raw reply
* Re: [PATCH] logging branch deletion to help recovering from mistakes
From: Casey Dahlin @ 2010-12-07 18:26 UTC (permalink / raw)
To: Jonathan Nieder; +Cc: Casey Dahlin, Junio C Hamano, git
In-Reply-To: <20101207180236.GC21483@burratino>
On Tue, Dec 07, 2010 at 12:02:36PM -0600, Jonathan Nieder wrote:
> Casey Dahlin wrote:
> > On Tue, Dec 07, 2010 at 11:45:20AM -0600, Jonathan Nieder wrote:
> >> Casey Dahlin wrote:
>
> >>> Could commits made onto a detached head also show up here? Or is that
> >>> better thwarted with another mechanism?
> >>
> >> I think that's better thwarted with the HEAD reflog:
> >>
> >> $ git log -g HEAD
> >
> > I was more worried about changes that were made onto a detached head,
> > and then the head was reattached, leaving the new commits dangling.
>
> But isn't that exactly what a detached HEAD is for? If one wants
> the experiments one does on detached HEAD to be kept around "just
> in case", wouldn't it make more sense to give them a (branch) name so
> they can be separated from one another?
>
An experienced git user who's paying attention to what he's doing would
do things that way, but detached heads and what happens to commits
thereupon are one of those concepts that tends to puzzle newbies, so
making it harder to make mistakes with them is probably a good idea.
Even still, there's always going to be those "Oh, I actually wanted to
keep some of that" moments, and that's what I'm talking about preparing
for.
--CJD
^ permalink raw reply
* Re: [PATCH] logging branch deletion to help recovering from mistakes
From: Shawn O. Pearce @ 2010-12-07 18:23 UTC (permalink / raw)
To: Jeff King; +Cc: Junio C Hamano, Nguyen Thai Ngoc Duy, git
In-Reply-To: <20101207182040.GA26770@sigill.intra.peff.net>
Jeff King <peff@peff.net> wrote:
> On Tue, Dec 07, 2010 at 10:14:19AM -0800, Shawn O. Pearce wrote:
>
> > Per check-ref-format, ref names cannot contain two dots. We could
> > archive ref logs by renaming them, $GIT_DIR/logs/refs/heads/foo
> > becomes $GIT_DIR/logs/refs/heads/foo..deleted-1. If foo is created
> > and deleted again, it becomes foo..deleted-2.
...
> > A different approach might be to have $GIT_DIR/logs/refs/REF_ATTIC,
>
> That seems a lot less efficient, as we have to linearly search all of
> REF_ATTIC to get:
>
> 1. the reflog for one deleted branch
>
> 2. the list of deleted branches
>
> Neither of those is probably particularly performance critical, but it
> just seems like keeping the logs in files indexed by the original ref
> names is a more natural fit.
Yea, I'm leaning more towards the foo..deleted-n idea too, for the
same reasons. It also makes it easier to GC a deleted branch's
reflog, we can examine the last record's timestamp in a reasonable
time bound and unlink the log if its really freaking old.
--
Shawn.
^ permalink raw reply
* [PATCH 2/2] describe: Don’t look up commits with --exact-match
From: Anders Kaseorg @ 2010-12-07 18:22 UTC (permalink / raw)
To: Junio C Hamano, SZEDER Gábor
Cc: Jonathan Nieder, git, Kirill Smelkov, Thomas Rast
In-Reply-To: <alpine.DEB.2.02.1012071320240.23348@dr-wily.mit.edu>
This makes ‘git describe --exact-match HEAD’ about 15 times faster on
a cold cache (2.3s instead of 35s) in a linux-2.6 repository with many
packed tags. That’s a huge win for the interactivity of the __git_ps1
shell prompt helper when on a detached head.
Signed-off-by: Anders Kaseorg <andersk@ksplice.com>
diff --git a/builtin/describe.c b/builtin/describe.c
index 700f740..0cddef1 100644
--- a/builtin/describe.c
+++ b/builtin/describe.c
@@ -22,7 +22,7 @@ static int tags; /* Allow lightweight tags */
static int longformat;
static int abbrev = DEFAULT_ABBREV;
static int max_candidates = 10;
-static int found_names;
+static struct commit_name *names;
static const char *pattern;
static int always;
static const char *dirty;
@@ -34,6 +34,8 @@ static const char *diff_index_args[] = {
struct commit_name {
+ struct commit_name *next;
+ unsigned char peeled[20];
struct tag *tag;
unsigned prio:2; /* annotated tag = 2, tag = 1, head = 0 */
unsigned name_checked:1;
@@ -78,31 +80,26 @@ static int replace_name(struct commit_name *e,
}
static void add_to_known_names(const char *path,
- struct commit *commit,
+ const unsigned char *peeled,
int prio,
const unsigned char *sha1)
{
- struct commit_name *e = commit->util;
struct tag *tag = NULL;
- if (replace_name(e, prio, sha1, &tag)) {
- size_t len = strlen(path)+1;
- free(e);
- e = xmalloc(sizeof(struct commit_name) + len);
- e->tag = tag;
- e->prio = prio;
- e->name_checked = 0;
- hashcpy(e->sha1, sha1);
- memcpy(e->path, path, len);
- commit->util = e;
- }
- found_names = 1;
+ size_t len = strlen(path)+1;
+ struct commit_name *e = xmalloc(sizeof(struct commit_name) + len);
+ hashcpy(e->peeled, peeled);
+ e->tag = tag;
+ e->prio = prio;
+ e->name_checked = 0;
+ hashcpy(e->sha1, sha1);
+ memcpy(e->path, path, len);
+ e->next = names;
+ names = e;
}
static int get_name(const char *path, const unsigned char *sha1, int flag, void *cb_data)
{
int might_be_tag = !prefixcmp(path, "refs/tags/");
- struct commit *commit;
- struct object *object;
unsigned char peeled[20];
int is_tag, prio;
@@ -110,16 +107,10 @@ static int get_name(const char *path, const unsigned char *sha1, int flag, void
return 0;
if (!peel_ref(path, peeled) && !is_null_sha1(peeled)) {
- commit = lookup_commit_reference_gently(peeled, 1);
- if (!commit)
- return 0;
- is_tag = !!hashcmp(sha1, commit->object.sha1);
+ is_tag = !!hashcmp(sha1, peeled);
} else {
- commit = lookup_commit_reference_gently(sha1, 1);
- object = parse_object(sha1);
- if (!commit || !object)
- return 0;
- is_tag = object->type == OBJ_TAG;
+ hashcpy(peeled, sha1);
+ is_tag = 0;
}
/* If --all, then any refs are used.
@@ -142,7 +133,7 @@ static int get_name(const char *path, const unsigned char *sha1, int flag, void
if (!prio)
return 0;
}
- add_to_known_names(all ? path + 5 : path + 10, commit, prio, sha1);
+ add_to_known_names(all ? path + 5 : path + 10, peeled, prio, sha1);
return 0;
}
@@ -228,7 +219,7 @@ static void describe(const char *arg, int last_one)
unsigned char sha1[20];
struct commit *cmit, *gave_up_on = NULL;
struct commit_list *list;
- struct commit_name *n;
+ struct commit_name *n, *e;
struct possible_tag all_matches[MAX_TAGS];
unsigned int match_cnt = 0, annotated_cnt = 0, cur_match;
unsigned long seen_commits = 0;
@@ -240,7 +231,12 @@ static void describe(const char *arg, int last_one)
if (!cmit)
die("%s is not a valid '%s' object", arg, commit_type);
- n = cmit->util;
+ n = NULL;
+ for (e = names; e; e = e->next) {
+ if (!hashcmp(e->peeled, cmit->object.sha1) &&
+ replace_name(n, e->prio, e->sha1, &e->tag))
+ n = e;
+ }
if (n && (tags || all || n->prio == 2)) {
/*
* Exact match to an existing ref.
@@ -259,6 +255,12 @@ static void describe(const char *arg, int last_one)
if (debug)
fprintf(stderr, "searching to describe %s\n", arg);
+ for (e = names; e; e = e->next) {
+ struct commit *c = lookup_commit_reference_gently(e->peeled, 1);
+ if (c && replace_name(c->util, e->prio, e->sha1, &e->tag))
+ c->util = e;
+ }
+
list = NULL;
cmit->object.flags = SEEN;
commit_list_insert(cmit, &list);
@@ -419,7 +421,7 @@ int cmd_describe(int argc, const char **argv, const char *prefix)
}
for_each_rawref(get_name, NULL);
- if (!found_names && !always)
+ if (!names && !always)
die("No names found, cannot describe anything.");
if (argc == 0) {
--
1.7.3.3
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox