* [PATCH] git-credential-gnome-keyring: Remove die
From: Matt Kraai @ 2012-11-11 0:08 UTC (permalink / raw)
To: git; +Cc: Matt Kraai
git-credential-gnome-keyring defines die, but it is unused and
incorrect (it passes a va_list to error, which does not expect one),
so remove it.
Signed-off-by: Matt Kraai <kraai@ftbfs.org>
---
.../credential/gnome-keyring/git-credential-gnome-keyring.c | 10 ----------
1 file changed, 10 deletions(-)
diff --git a/contrib/credential/gnome-keyring/git-credential-gnome-keyring.c b/contrib/credential/gnome-keyring/git-credential-gnome-keyring.c
index 41f61c5..3eaafba 100644
--- a/contrib/credential/gnome-keyring/git-credential-gnome-keyring.c
+++ b/contrib/credential/gnome-keyring/git-credential-gnome-keyring.c
@@ -104,16 +104,6 @@ static inline void error(const char *fmt, ...)
va_end(ap);
}
-static inline void die(const char *fmt, ...)
-{
- va_list ap;
-
- va_start(ap,fmt);
- error(fmt, ap);
- va_end(ap);
- exit(EXIT_FAILURE);
-}
-
static inline void die_errno(int err)
{
error("%s", strerror(err));
--
1.7.10.4
^ permalink raw reply related
* Re: Reviews on mailing-list
From: Felipe Contreras @ 2012-11-10 23:40 UTC (permalink / raw)
To: Deniz Türkoglu; +Cc: git, Junio C Hamano, Shawn Pearce
In-Reply-To: <CA+ZXwZOnZZyk_KWmLLLNChWb1R_Av1s5jM4PsLFWKaG8BB+uXQ@mail.gmail.com>
On Sun, Nov 11, 2012 at 12:19 AM, Deniz Türkoglu <deniz@spotify.com> wrote:
> This is my first mail to the git mailing list. I have been following
> the list for some time now and I would like to suggest moving the
> reviews out of the mailing list, for example to a gerrit instance, I
> believe it would improve the commits and the mailing list. I have a
> filter on 'PATCH', but I feel I miss some of the discussion, and
> things that I would be interested in.
>
> I have spoken to Shawn Pearce (gerrit project lead, google) and he
> said he is OK with hosting the gerrit instance.
>
> I would like to hear your thoughts on this.
Personally I think reviews on the mailing list is far superior than
any other review methods. I've even blogged about it and all the
reasons[1]. Gerrit is better than bugzilla, but it still requires a
web browser, and logging in.
I love to be able to just hit 'reply' with my favorite MUA, comment
inline, and hit send.
Cheers.
[1] http://felipec.wordpress.com/2010/01/19/why-bugzilla-sucks-for-handling-patches/
--
Felipe Contreras
^ permalink raw reply
* Reviews on mailing-list
From: Deniz Türkoglu @ 2012-11-10 23:19 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Shawn Pearce
Hi all,
This is my first mail to the git mailing list. I have been following
the list for some time now and I would like to suggest moving the
reviews out of the mailing list, for example to a gerrit instance, I
believe it would improve the commits and the mailing list. I have a
filter on 'PATCH', but I feel I miss some of the discussion, and
things that I would be interested in.
I have spoken to Shawn Pearce (gerrit project lead, google) and he
said he is OK with hosting the gerrit instance.
I would like to hear your thoughts on this.
cheers,
-deniz
^ permalink raw reply
* Re: t9350-fast-export.sh broken on peff/pu under Mac OS X
From: Felipe Contreras @ 2012-11-10 23:11 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Torsten Bögershausen, Git Mailing List
In-Reply-To: <7v4nkxozep.fsf@alter.siamese.dyndns.org>
On Sat, Nov 10, 2012 at 11:39 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Felipe Contreras <felipe.contreras@gmail.com> writes:
>
>> On Sat, Nov 10, 2012 at 3:37 PM, Torsten Bögershausen <tboegi@web.de> wrote:
>>> The short version:
>>> echo -n doesn't seem to be portable.
>>> The following works for me:
>>
>> Right, I was supposed to change that to:
>>
>> true > marks-cur &&
>
> Please make it like so:
>
> >marks-cur &&
>
> No command is necessary when creating an empty file or truncating an
> existing file to empty, and no SP between redirection and its target.
That hangs on zsh (presumably waiting for stdin).
--
Felipe Contreras
^ permalink raw reply
* Re: t9350-fast-export.sh broken on peff/pu under Mac OS X
From: Junio C Hamano @ 2012-11-10 22:39 UTC (permalink / raw)
To: Felipe Contreras; +Cc: Torsten Bögershausen, Git Mailing List
In-Reply-To: <CAMP44s2sKL4CJ_wcM7U9mYPX6bkaQA8hgDM6Z0QEj2TACh-8DQ@mail.gmail.com>
Felipe Contreras <felipe.contreras@gmail.com> writes:
> On Sat, Nov 10, 2012 at 3:37 PM, Torsten Bögershausen <tboegi@web.de> wrote:
>> The short version:
>> echo -n doesn't seem to be portable.
>> The following works for me:
>
> Right, I was supposed to change that to:
>
> true > marks-cur &&
Please make it like so:
>marks-cur &&
No command is necessary when creating an empty file or truncating an
existing file to empty, and no SP between redirection and its target.
^ permalink raw reply
* Re: What's cooking in git.git (Nov 2012, #02; Fri, 9)
From: Paul Fox @ 2012-11-10 22:08 UTC (permalink / raw)
To: Kalle Olavi Niemitalo; +Cc: git
In-Reply-To: <871ug15k5c.fsf@Niukka.kon.iki.fi>
kalle olavi niemitalo wrote:
> Paul Fox <pgf@foxharp.boston.ma.us> writes:
>
> > when i implemented the change, i wondered if some twisted emacs
> > workflow would be an issue. ;-) and i almost blocked SIGQUIT as
> > well -- the two programs i looked at for precedent (CVS and MH) both
> > block both SIGQUIT and SIGINT when spawning an editor.
> >
> > but since emacs users must have dealt with CVS for a long time before
> > dealing with git, how might they have done so?
>
> I think I usually ran CVS via vc.el, which prompts for a commit
> message in Emacs before it runs cvs commit. So CVS did not need
> to run $EDITOR.
>
> I just tried emacsclient with CVS 1.12.13-MirDebian-9, and it
> behaves somewhat differently from Git with pf/editor-ignore-sigint.
> When I tell Emacs to send SIGINT to the *Shell* buffer, CVS prompts:
>
> cvs commit: warning: editor session failed
>
> Log message unchanged or not specified
> a)bort, c)ontinue, e)dit, !)reuse this message unchanged for remaining dirs
> Action: (continue)
you're sending SIGINT to the cvs commit command, and that causes the
editor to die right away? that's surprising. i can replicate your
described behavior by setting $VISUAL to a script that just sleeps, and
sending SIGTERM to the cvs commit process. but not by sending SIGINT.
well, i'm not sure what to say. there's a real problem when using the
current code and traditional editors. i thought that the patch in
pf/editor-ignore-sigint reflected standard practice, and indeed it
accomplishes exactly the right thing with those editors. you've shown
a particular work flow involving emacsclient that won't work anymore
with the change made, though there are workarounds. perhaps there's
something the other editors themselves should be doing differently,
but i don't know what that might be.
paul
>
> and then I can choose to abort.
>
> With strace, it looks like CVS sets SIG_IGN as the handler of
> SIGINT and SIGQUIT only in the parent process after forking, not
> in the child process that executes the editor.
>
> CVS also temporarily blocks signals by calling sigprocmask, but
> it undoes that before it forks or waits for the child process.
=---------------------
paul fox, pgf@foxharp.boston.ma.us (arlington, ma, where it's 45.5 degrees)
^ permalink raw reply
* Re: t5801-remote-helpers.sh fails
From: Felipe Contreras @ 2012-11-10 22:05 UTC (permalink / raw)
To: Torsten Bögershausen; +Cc: Git Mailing List
In-Reply-To: <509EA8EE.1030908@web.de>
On Sat, Nov 10, 2012 at 8:20 PM, Torsten Bögershausen <tboegi@web.de> wrote:
> On 11/10/2012 08:15 PM, Felipe Contreras wrote:
>>
>> Hi,
>>
>> On Sat, Nov 10, 2012 at 2:48 PM, Torsten Bögershausen <tboegi@web.de>
>> wrote:
>>
>>> on peff/pu t5801 fails, the error is in git-remote-testgit, please see
>>> below.
>>>
>>> That's on my Mac OS X box.
>>>
>>> I haven't digged further into the test case, but it looks as if
>>> "[-+]A make NAMEs associative arrays"
>>> is not supported on this version of bash.
>>> /Torsten
>>>
>>>
>>> /Users/tb/projects/git/git.peff/git-remote-testgit: line 64: declare: -A:
>>> invalid option
>>> declare: usage: declare [-afFirtx] [-p] [name[=value] ...]
>>> /Users/tb/projects/git/git.peff/git-remote-testgit: line 66:
>>> refs/heads/master: division by 0 (error token is "/master")
>>> error: fast-export died of signal 13
>>> fatal: Error while running fast-export
>>
>>
>> What is your bash --version?
>>
> bash --version
> GNU bash, version 3.2.48(1)-release (x86_64-apple-darwin10.0)
> Copyright (C) 2007 Free Software Foundation, Inc.
I see, that version indeed doesn't have associative arrays.
> On the other hand, Documentation/CodingGuidelines says:
> - No shell arrays.
Yeah, for shell code I guess, but this is bash code.
> Could we use perl to have arrays?
I think the code in perl would be harder to follow, and this is meant
not only as a test, but also as a reference.
I'm not exactly sure what we should do here:
a) remove the code (would not be so good as a reference)
b) enable the code conditionally based on the version of bash (harder to read)
c) replace the code without associative arrays (will be much more
complicated and ugly)
d) add a check for the bash version to the top of the test in t/
I'm leaning towards d), followed by b).
If only there was a clean way to do this, so c) would not be so ugly.
After investigating different optins this seems to be the best:
join -e empty -o '0 1.2 2.2' -a 2 <(echo "$before") <(echo "$after")
| while read e a b; do
test "$a" == "$b" && continue
echo "changed $e"
done
But to me seems a bit harder to grasp. Not sure.
Cheers.
--
Felipe Contreras
^ permalink raw reply
* Re: [PATCH] git tag --contains : avoid stack overflow
From: Pat Thoyts @ 2012-11-10 21:33 UTC (permalink / raw)
To: Jean-Jacques Lafay; +Cc: msysgit, Git List, Philip Oakley
In-Reply-To: <d9843c09-3ca9-406e-9df8-78603235d985@googlegroups.com>
On 10 November 2012 21:13, Jean-Jacques Lafay
<jeanjacques.lafay@gmail.com> wrote:
> Le samedi 10 novembre 2012 21:00:10 UTC+1, Philip Oakley a écrit :
>>
>> From: "Jean-Jacques Lafay" <jeanjacq...@gmail.com> Sent: Saturday,
>> November 10, 2012 5:36 PM
>> > In large repos, the recursion implementation of contains(commit,
>> > commit_list)
>> > may result in a stack overflow. Replace the recursion with a loop to
>> > fix it.
>> >
>> > Signed-off-by: Jean-Jacques Lafay <jeanjacq...@gmail.com>
>>
>> This is a change to the main git code so it is better to submit it to
>> the main git list at g...@vger.kernel.org (plain text, no HTML, first
>> post usually has a delay ;-)
>>
>> It sounds reasonable but others may have opinions and comments.
>>
>> Have you actually suffered from the stack overflow issue? You only
>> suggest it as a possibility, rather than a real problem.
>>
>> Philip
>
>
> Yes, it actually crashed on me, and since the call is made by GitExtension
> while browsing the commit history, it was quickly annoying to have windows
> popping a "git.exe stopped working" message box at my face every time I
> clicked on a line of the history ;-) (well, you can sort of work around it
> by having the "file tree" or "diff" tab active)
>
> Coincidentally, as I was having a glance at the recent topics, I saw that
> someone else experienced it.
>
> However, I couldn't reproduce it on Linux : where the windows
> implementations crashes at a ~32000 depth (*not* exactly 32768, mind you),
> on linux it happily went through 100000 commits. I didn't take time to look
> much further, but maybe on my 64 bit Linux VM, the process can afford to
> reserve a much bigger address range for the stack of each thread than the
> 1Mb given to 32 bit processes on windows.
>
> Jean-Jacques.
I checked this on msysGit - the test causes a crash on Windows when
using the 1.8.0 release and recompiling with this patch applied fixes
this. As mentioned, its best to have this reviewed upstream too as its
likely that windows just has a smaller stack so causes the crash
earlier.
--
*** Please reply-to-all at all times ***
*** (do not pretend to know who is subscribed and who is not) ***
*** Please avoid top-posting. ***
The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.
You received this message because you are subscribed to the Google
Groups "msysGit" group.
To post to this group, send email to msysgit@googlegroups.com
To unsubscribe from this group, send email to
msysgit+unsubscribe@googlegroups.com
For more options, and view previous threads, visit this group at
http://groups.google.com/group/msysgit?hl=en_US?hl=en
^ permalink raw reply
* Re: [PATCH] git tag --contains : avoid stack overflow
From: Jean-Jacques Lafay @ 2012-11-10 21:13 UTC (permalink / raw)
To: msysgit; +Cc: Jean-Jacques Lafay, Git List, Philip Oakley
In-Reply-To: <03ED9D50D1F64467863C051959CFC433@PhilipOakley>
[-- Attachment #1: Type: text/plain, Size: 2385 bytes --]
Le samedi 10 novembre 2012 21:00:10 UTC+1, Philip Oakley a écrit :
> From: "Jean-Jacques Lafay" <jeanjacq...@gmail.com <javascript:>> Sent:
> Saturday,
> November 10, 2012 5:36 PM
> > In large repos, the recursion implementation of contains(commit,
> > commit_list)
> > may result in a stack overflow. Replace the recursion with a loop to
> > fix it.
> >
> > Signed-off-by: Jean-Jacques Lafay <jeanjacq...@gmail.com <javascript:>>
>
> This is a change to the main git code so it is better to submit it to
> the main git list at g...@vger.kernel.org <javascript:> (plain text, no
> HTML, first
> post usually has a delay ;-)
>
> It sounds reasonable but others may have opinions and comments.
>
> Have you actually suffered from the stack overflow issue? You only
> suggest it as a possibility, rather than a real problem.
>
> Philip
>
Yes, it actually crashed on me, and since the call is made by GitExtension
while browsing the commit history, it was quickly annoying to have windows
popping a "git.exe stopped working" message box at my face every time I
clicked on a line of the history ;-) (well, you can sort of work around it
by having the "file tree" or "diff" tab active)
Coincidentally, as I was having a glance at the recent topics, I saw that
someone else experienced it.
However, I couldn't reproduce it on Linux : where the windows
implementations crashes at a ~32000 depth (*not* exactly 32768, mind you),
on linux it happily went through 100000 commits. I didn't take time to look
much further, but maybe on my 64 bit Linux VM, the process can afford to
reserve a much bigger address range for the stack of each thread than the
1Mb given to 32 bit processes on windows.
Jean-Jacques.
--
*** Please reply-to-all at all times ***
*** (do not pretend to know who is subscribed and who is not) ***
*** Please avoid top-posting. ***
The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.
You received this message because you are subscribed to the Google
Groups "msysGit" group.
To post to this group, send email to msysgit@googlegroups.com
To unsubscribe from this group, send email to
msysgit+unsubscribe@googlegroups.com
For more options, and view previous threads, visit this group at
http://groups.google.com/group/msysgit?hl=en_US?hl=en
[-- Attachment #2: Type: text/html, Size: 3089 bytes --]
^ permalink raw reply
* Re: What's cooking in git.git (Nov 2012, #02; Fri, 9)
From: Andreas Schwab @ 2012-11-10 21:12 UTC (permalink / raw)
To: Kalle Olavi Niemitalo; +Cc: Paul Fox, git
In-Reply-To: <871ug15k5c.fsf@Niukka.kon.iki.fi>
Kalle Olavi Niemitalo <kon@iki.fi> writes:
> With strace, it looks like CVS sets SIG_IGN as the handler of
> SIGINT and SIGQUIT only in the parent process after forking, not
> in the child process that executes the editor.
>
> CVS also temporarily blocks signals by calling sigprocmask, but
> it undoes that before it forks or waits for the child process.
This emulates what system(3) does.
Andreas.
--
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5
"And now for something completely different."
^ permalink raw reply
* Re: git-reset man page
From: Krzysztof Mazur @ 2012-11-10 20:46 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Angelo Borsotti, git, peff
In-Reply-To: <7v8va9p6pn.fsf@alter.siamese.dyndns.org>
On Sat, Nov 10, 2012 at 12:02:12PM -0800, Junio C Hamano wrote:
> Krzysztof Mazur <krzysiek@podlesie.net> writes:
>
> > Maybe we should just add that <paths> is an shortcut for <pathspec>
> > and fix places where paths and pathspec are mixed or <path> is used as
> > <pathspec>.
>
> We should unify uses of <paths> and <path> (the former should be
> <path>... or something).
Currently in most cases "<paths>..." is used ;)
>
> Some places you need to give exact path (iow, these places you
> cannot use pathspec), while most other places pathspec
> (i.e. matching pattern) is accepted.
I know, thats why I added a note that "<path> is not always used for pathspec".
>
> The manual correctly updated will most likely to use both <path> and
> <pathspec> appropriately.
So we should always use "<path>" for exact path, and "<pathspec>" for
pathspecs patterns as defined in gitglossary. I think it's better
to avoid "<paths>" and always use "<path>..." or "<pathspec>...".
Krzysiek
^ permalink raw reply
* Re: git-reset man page
From: Junio C Hamano @ 2012-11-10 20:02 UTC (permalink / raw)
To: Krzysztof Mazur; +Cc: Angelo Borsotti, git, peff
In-Reply-To: <20121110155718.GA29321@shrek.podlesie.net>
Krzysztof Mazur <krzysiek@podlesie.net> writes:
> Maybe we should just add that <paths> is an shortcut for <pathspec>
> and fix places where paths and pathspec are mixed or <path> is used as
> <pathspec>.
We should unify uses of <paths> and <path> (the former should be
<path>... or something).
Some places you need to give exact path (iow, these places you
cannot use pathspec), while most other places pathspec
(i.e. matching pattern) is accepted.
The manual correctly updated will most likely to use both <path> and
<pathspec> appropriately.
^ permalink raw reply
* Re: [PATCH] git tag --contains : avoid stack overflow
From: Philip Oakley @ 2012-11-10 20:00 UTC (permalink / raw)
To: Jean-Jacques Lafay, msysgit; +Cc: Git List
In-Reply-To: <1352568970-4669-1-git-send-email-jeanjacques.lafay@gmail.com>
From: "Jean-Jacques Lafay" <jeanjacques.lafay@gmail.com> Sent: Saturday,
November 10, 2012 5:36 PM
> In large repos, the recursion implementation of contains(commit,
> commit_list)
> may result in a stack overflow. Replace the recursion with a loop to
> fix it.
>
> Signed-off-by: Jean-Jacques Lafay <jeanjacques.lafay@gmail.com>
This is a change to the main git code so it is better to submit it to
the main git list at git@vger.kernel.org (plain text, no HTML, first
post usually has a delay ;-)
It sounds reasonable but others may have opinions and comments.
Have you actually suffered from the stack overflow issue? You only
suggest it as a possibility, rather than a real problem.
Philip
> ---
> builtin/tag.c | 83
> ++++++++++++++++++++++++++++++++++++++++------------------
> t/t7004-tag.sh | 21 +++++++++++++++
> 2 files changed, 78 insertions(+), 26 deletions(-)
>
> diff --git a/builtin/tag.c b/builtin/tag.c
> index 9c3e067..4be67dd 100644
> --- a/builtin/tag.c
> +++ b/builtin/tag.c
> @@ -73,40 +73,71 @@ static int in_commit_list(const struct commit_list
> *want, struct commit *c)
> return 0;
> }
>
> -static int contains_recurse(struct commit *candidate,
> - const struct commit_list *want)
> -{
> - struct commit_list *p;
> -
> - /* was it previously marked as containing a want commit? */
> - if (candidate->object.flags & TMP_MARK)
> - return 1;
> - /* or marked as not possibly containing a want commit? */
> - if (candidate->object.flags & UNINTERESTING)
> - return 0;
> - /* or are we it? */
> - if (in_commit_list(want, candidate))
> - return 1;
> +struct commit_list_list {
> + struct commit *item;
> + struct commit_list *next_item;
> + struct commit_list_list *next;
> +};
>
> - if (parse_commit(candidate) < 0)
> - return 0;
> +static void mark_path(struct commit_list_list *path, int status)
> +{
> + struct commit_list_list *temp;
> + while (path) {
> + path->item->object.flags |= status;
> + temp = path;
> + path = temp->next;
> + free(temp);
> + }
> +}
>
> - /* Otherwise recurse and mark ourselves for future traversals. */
> - for (p = candidate->parents; p; p = p->next) {
> - if (contains_recurse(p->item, want)) {
> - candidate->object.flags |= TMP_MARK;
> +static int contains(struct commit *candidate,
> + const struct commit_list *want)
> +{
> + /* previously implemented with a recursion, but could stack overflow
> in large repos */
> + struct commit_list_list *p, *tmp;
> + p = xmalloc(sizeof(struct commit_list_list));
> + p->item = candidate;
> + p->next_item = NULL;
> + p->next = NULL;
> +
> + while (p) {
> + candidate = p->item;
> +
> + /* is it ok : */
> + /* was it previously marked as containing a want commit? */
> + /* or are we it? */
> + if (candidate->object.flags & TMP_MARK || in_commit_list(want,
> candidate)) {
> + mark_path(p, TMP_MARK);
> return 1;
> }
> + /* is it not ok : */
> + /* was it previously marked as not possibly containing a want
> commit? */
> + /* do we have parents? */
> + if (candidate->object.flags & UNINTERESTING ||
> parse_commit(candidate) < 0 || !candidate->parents) {
> + candidate->object.flags |= UNINTERESTING;
> + /* then backtrack, marking as UNINTERESTING along the way */
> + while (p && !p->next_item) {
> + p->item->object.flags |= UNINTERESTING;
> + tmp = p;
> + p = tmp->next;
> + free(tmp);
> + }
> + if (p) {
> + p->item = p->next_item->item;
> + p->next_item = p->next_item->next;
> + }
> + } else {
> + /* let's check the parents */
> + tmp = xmalloc(sizeof(struct commit_list_list));
> + tmp->item = candidate->parents->item;
> + tmp->next_item = candidate->parents->next;
> + tmp->next = p;
> + p = tmp;
> + }
> }
> - candidate->object.flags |= UNINTERESTING;
> return 0;
> }
>
> -static int contains(struct commit *candidate, const struct
> commit_list *want)
> -{
> - return contains_recurse(candidate, want);
> -}
> -
> static void show_tag_lines(const unsigned char *sha1, int lines)
> {
> int i;
> diff --git a/t/t7004-tag.sh b/t/t7004-tag.sh
> index 5189446..196ac54 100755
> --- a/t/t7004-tag.sh
> +++ b/t/t7004-tag.sh
> @@ -1365,4 +1365,25 @@ test_expect_success 'multiple --points-at are
> OR-ed together' '
> test_cmp expect actual
> '
>
> +# what about a deep repo ?
> +
> +>expect
> +test_expect_success '--contains works in a deep repo' '
> + i=1
> + while test $i -lt 40000
> + do
> + echo "commit refs/heads/master
> +committer A U Thor <author@example.com> $((1000000000 + $i * 100))
> +0200
> +data <<EOF
> +commit #$i
> +EOF"
> + test $i == 1 && echo "from refs/heads/master^0"
> + i=$(($i + 1))
> + done | git fast-import
> + git checkout master
> + git tag far-far-away HEAD^
> + git tag --contains HEAD >actual &&
> + test_cmp expect actual
> +'
> +
> test_done
> --
> 1.8.0.msysgit.0.1.geed93bd.dirty
>
> --
--
*** Please reply-to-all at all times ***
*** (do not pretend to know who is subscribed and who is not) ***
*** Please avoid top-posting. ***
The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.
You received this message because you are subscribed to the Google
Groups "msysGit" group.
To post to this group, send email to msysgit@googlegroups.com
To unsubscribe from this group, send email to
msysgit+unsubscribe@googlegroups.com
For more options, and view previous threads, visit this group at
http://groups.google.com/group/msysgit?hl=en_US?hl=en
^ permalink raw reply
* Re: What's cooking in git.git (Nov 2012, #02; Fri, 9)
From: Junio C Hamano @ 2012-11-10 19:56 UTC (permalink / raw)
To: SZEDER Gábor; +Cc: Jeff King, Felipe Contreras, git, Sverre Rabbelier
In-Reply-To: <20121110123250.GR12052@goldbirke>
SZEDER Gábor <szeder@ira.uka.de> writes:
>> I think it is better to keep the tests simple and maintainable.
>
> Maintainable? There is nothing to maintain here....
> ...
> OTOH, this series has some serious drawbacks.
>
> It makes debugging more difficult....
Are these referring to the same aspect of the series? The concern
you described about debuggability matches my impression IIRC back
when I took a look at the series, which I would count as a large
part of keeping tests maintainable.
But you may be referring to something different (sorry, not on my
primary machine yet).
^ permalink raw reply
* Re: What's cooking in git.git (Nov 2012, #02; Fri, 9)
From: Kalle Olavi Niemitalo @ 2012-11-10 19:32 UTC (permalink / raw)
To: Paul Fox; +Cc: git
In-Reply-To: <20121110155209.75EFC2E8B68@grass.foxharp.boston.ma.us>
Paul Fox <pgf@foxharp.boston.ma.us> writes:
> when i implemented the change, i wondered if some twisted emacs
> workflow would be an issue. ;-) and i almost blocked SIGQUIT as
> well -- the two programs i looked at for precedent (CVS and MH) both
> block both SIGQUIT and SIGINT when spawning an editor.
>
> but since emacs users must have dealt with CVS for a long time before
> dealing with git, how might they have done so?
I think I usually ran CVS via vc.el, which prompts for a commit
message in Emacs before it runs cvs commit. So CVS did not need
to run $EDITOR.
I just tried emacsclient with CVS 1.12.13-MirDebian-9, and it
behaves somewhat differently from Git with pf/editor-ignore-sigint.
When I tell Emacs to send SIGINT to the *Shell* buffer, CVS prompts:
cvs commit: warning: editor session failed
Log message unchanged or not specified
a)bort, c)ontinue, e)dit, !)reuse this message unchanged for remaining dirs
Action: (continue)
and then I can choose to abort.
With strace, it looks like CVS sets SIG_IGN as the handler of
SIGINT and SIGQUIT only in the parent process after forking, not
in the child process that executes the editor.
CVS also temporarily blocks signals by calling sigprocmask, but
it undoes that before it forks or waits for the child process.
^ permalink raw reply
* Re: RFD: fast-import is picky with author names (and maybe it should - but how much so?)
From: A Large Angry SCM @ 2012-11-10 19:25 UTC (permalink / raw)
To: Felipe Contreras, Michael J Gruber, Git Mailing List; +Cc: Jeff King
In-Reply-To: <CAMP44s219Zi2NPt2vA+6Od_sVstFK85OXZK-9K1OCFpVh220+A@mail.gmail.com>
On 11/10/2012 01:43 PM, Felipe Contreras wrote:
> On Sat, Nov 10, 2012 at 6:28 PM, Michael J Gruber
> <git@drmicha.warpmail.net> wrote:
>> Felipe Contreras venit, vidit, dixit 09.11.2012 15:34:
>>> On Fri, Nov 9, 2012 at 10:28 AM, Michael J Gruber
>>> <git@drmicha.warpmail.net> wrote:
>>>
>>>> Hg seems to store just anything in the author field ("committer"). The
>>>> various interfaces that are floating around do some behind-the-back
>>>> conversion to git format. The more conversions they do, the better they
>>>> seem to work (no erroring out) but I'm wondering whether it's really a
>>>> good thing, or whether we should encourage a more diligent approach
>>>> which requires a user to map non-conforming author names wilfully.
>>>
>>> So you propose that when somebody does 'git clone hg::hg hg-git' the
>>> thing should fail. I hope you don't think it's too unbecoming for me
>>> to say that I disagree.
>>
>> There is no need to disagree with a proposal I haven't made. I would
>> disagree with the proposal that I haven't made, too.
>
> All right, we shouldn't encourage a more diligent approach which
> requires a user to map author names then.
>
>>> IMO it should be git fast-import the one that converts these bad
>>> authors, not every single tool out there. Maybe throw a warning, but
>>> that's all. Or maybe generate a list of bad authors ready to be filled
>>> out. That way when a project is doing a real conversion, say, when
>>> moving to git, they can run the conversion once and see which authors
>>> are bad and not multiple times, each try taking longer than the next.
>>
>> As Jeff pointed out, git-fast-import expects output conforming to a
>> certain standard, and that's not going to change. import is agnostic to
>> where its import stream is coming from. Only the producer of that stream
>> can have additional information about the provenience of the stream's
>> data which may aid (possibly together with user input or choices) in
>> transforming that into something conforming.
>
> We already know where the import of those streams come from:
> mercurial, bazaar, etc. There's absolutely nothing the tools exporting
> data from those repositories can do, except try to convert all kind of
> weird names--and many tools do it poorly.
>
> So, the options are:
>
> a) Leave the name conversion to the export tools, and when they miss
> some weird corner case, like 'Author<email', let the user face the
> consequences, perhaps after an hour of the process.
>
> We know there are sources of data that don't have git-formatted author
> names, so we know every tool out there must do this checking.
>
> In addition to that, let the export tool decide what to do when one of
> these bad names appear, which in many cases probably means do nothing,
> so the user would not even see that such a bad name was there, which
> might not be what they want.
>
> b) Do the name conversion in fast-import itself, perhaps optionally,
> so if a tool missed some weird corner case, the user does not have to
> face the consequences.
>
> The tool writers don't have to worry about this, so we would not have
> tools out there doing a half-assed job of this.
>
> And what happens when such bad names end up being consistent: warning,
> a scaffold mapping of bad names, etc.
>
>
> One is bad for the users, and the tools writers, only disadvantages,
> the other is good for the users and the tools writers, only
> advantages.
>
c) Do the name conversion, and whatever other cleanup and manipulations
you're interesting in, in a filter between the exporter and git-fast-import.
^ permalink raw reply
* Re: Help requested - trying to build a tool doing whole-tree commits
From: Felipe Contreras @ 2012-11-10 19:24 UTC (permalink / raw)
To: Andreas Schwab; +Cc: Eric S. Raymond, git
In-Reply-To: <m2txsyqwdi.fsf@igel.home>
On Fri, Nov 9, 2012 at 10:50 PM, Andreas Schwab <schwab@linux-m68k.org> wrote:
> Unknown <unknown@unknown.invalid> writes:
Unknown Invalid? Please don't change the original email, makes it
harder for other people to reply.
>> I need a command or command sequence that will commit an entire file
>> tree to a repository...
>>
>> (a) Allowing me to specify committer and author metadata, and
>>
>> (b) deleting paths not present in the previous commit on the current
>> branch, and
>>
>> (c) allowing me to specify merge links from other previous commits.
>>
>> git commit -a passes (a) and (b) but not (c).
>
> git commit -a won't add new files, so you probably want to use git add
> -A && git commit. I'm not quite sure what you mean with "merge links",
> but if you want to create merge commits the you'll need to resort to
> plumbing: git add -A && git write-tree && git commit-tree && git
> update-ref.
I've done something similar, except that I used git hash-object
instead of git commit-tree so that I can specify every single detail
about the commit object.
Cheers.
--
Felipe Contreras
^ permalink raw reply
* Re: [PATCH v3 2/3] git-submodule foreach: export .gitmodules settings as variables
From: W. Trevor King @ 2012-11-10 19:21 UTC (permalink / raw)
To: Heiko Voigt; +Cc: Git, Jeff King, Phil Hord, Shawn Pearce, Jens Lehmann, Nahor
In-Reply-To: <20121109164516.GB922@book.hvoigt.net>
On Fri, Nov 09, 2012 at 05:45:22PM +0100, Heiko Voigt wrote:
> > can now be reduced to
> >
> > git submodule foreach 'git checkout $submodule_branch && git pull'
>
> What other use cases are there? Would the need for this maybe go away
> once you had floating submodules following branches?
None that I can think of, but I don't use submodules very much. The
idea of easily-accessible per-submodule configuration variables
strikes me as pretty useful, but I agree the code is a bit ugly.
Actually, I think exporting environment variables and calling the
foreach command in a subshell would be better than the current local
variables and eval. The subshell would also make variable cleanup
irrelevant, which would make for a cleaner patch.
> For completeness you should make the variables possible to override by
> repository from the local repository configuration like all other
> submodule options that are read directly from .gitmodules.
Good idea (I wasn't aware of the override before). Will do in v4.
--
This email may be signed or encrypted with GnuPG (http://www.gnupg.org).
For more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy
^ permalink raw reply
* Re: t5801-remote-helpers.sh fails
From: Torsten Bögershausen @ 2012-11-10 19:20 UTC (permalink / raw)
To: Felipe Contreras; +Cc: Torsten Bögershausen, Git Mailing List
In-Reply-To: <CAMP44s0HtMRaQ91z95cZS73W7tnqYwyKV_2guWJ6u6UP4p5JNw@mail.gmail.com>
On 11/10/2012 08:15 PM, Felipe Contreras wrote:
> Hi,
>
> On Sat, Nov 10, 2012 at 2:48 PM, Torsten Bögershausen <tboegi@web.de> wrote:
>
>> on peff/pu t5801 fails, the error is in git-remote-testgit, please see below.
>>
>> That's on my Mac OS X box.
>>
>> I haven't digged further into the test case, but it looks as if
>> "[-+]A make NAMEs associative arrays"
>> is not supported on this version of bash.
>> /Torsten
>>
>>
>> /Users/tb/projects/git/git.peff/git-remote-testgit: line 64: declare: -A: invalid option
>> declare: usage: declare [-afFirtx] [-p] [name[=value] ...]
>> /Users/tb/projects/git/git.peff/git-remote-testgit: line 66: refs/heads/master: division by 0 (error token is "/master")
>> error: fast-export died of signal 13
>> fatal: Error while running fast-export
>
> What is your bash --version?
>
bash --version
GNU bash, version 3.2.48(1)-release (x86_64-apple-darwin10.0)
Copyright (C) 2007 Free Software Foundation, Inc.
On the other hand, Documentation/CodingGuidelines says:
- No shell arrays.
Could we use perl to have arrays?
/Torsten
^ permalink raw reply
* Re: t9350-fast-export.sh broken on peff/pu under Mac OS X
From: Felipe Contreras @ 2012-11-10 19:17 UTC (permalink / raw)
To: Torsten Bögershausen; +Cc: Git Mailing List
In-Reply-To: <509E66B0.3040308@web.de>
On Sat, Nov 10, 2012 at 3:37 PM, Torsten Bögershausen <tboegi@web.de> wrote:
> The short version:
> echo -n doesn't seem to be portable.
> The following works for me:
Right, I was supposed to change that to:
true > marks-cur &&
I don't think the code below works everywhere.
> (And is this a typo: test_expect_success 'test biridectionality' ')
Right, thanks.
> diff ../../git.peff/t/t9350-fast-export.sh t9350-fast-export.sh
Please use diff -u, I think everybody is more familiar with that.
Cheers.
--
Felipe Contreras
^ permalink raw reply
* Re: t5801-remote-helpers.sh fails
From: Felipe Contreras @ 2012-11-10 19:15 UTC (permalink / raw)
To: Torsten Bögershausen; +Cc: Git Mailing List
In-Reply-To: <509E5B27.5080808@web.de>
Hi,
On Sat, Nov 10, 2012 at 2:48 PM, Torsten Bögershausen <tboegi@web.de> wrote:
> on peff/pu t5801 fails, the error is in git-remote-testgit, please see below.
>
> That's on my Mac OS X box.
>
> I haven't digged further into the test case, but it looks as if
> "[-+]A make NAMEs associative arrays"
> is not supported on this version of bash.
> /Torsten
>
>
> /Users/tb/projects/git/git.peff/git-remote-testgit: line 64: declare: -A: invalid option
> declare: usage: declare [-afFirtx] [-p] [name[=value] ...]
> /Users/tb/projects/git/git.peff/git-remote-testgit: line 66: refs/heads/master: division by 0 (error token is "/master")
> error: fast-export died of signal 13
> fatal: Error while running fast-export
What is your bash --version?
--
Felipe Contreras
^ permalink raw reply
* Re: [PATCH v3 1/3] git-submodule add: Add -r/--record option
From: W. Trevor King @ 2012-11-10 19:02 UTC (permalink / raw)
To: Heiko Voigt
Cc: Junio C Hamano, Git, Jeff King, Phil Hord, Shawn Pearce,
Jens Lehmann, Nahor
In-Reply-To: <20121109162919.GA922@book.hvoigt.net>
On Fri, Nov 09, 2012 at 05:29:27PM +0100, Heiko Voigt wrote:
> I think we should agree on a behavior for this option and implement it
> the same time when add learns about it. When we were discussing floating
> submodules as an important option for the gerrit people I already started
> to implement a proof of concept. Please have a look here:
>
> https://github.com/hvoigt/git/commits/hv/floating_submodules
After skimming through this, something like
$ git submodule update --pull
would probably be better than introducing a new command:
On Sat, Nov 10, 2012 at 01:44:37PM -0500, W. Trevor King wrote:
> $ git submodule pull-branch
I think "floating submodules" is a misleading name for this feature
though, since the checkout SHA is explicitly specified. We're just
making it more convenient to explicitly update the SHA. How about
"tracking submodules"?
--
This email may be signed or encrypted with GnuPG (http://www.gnupg.org).
For more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy
^ permalink raw reply
* Re: What's cooking in git.git (Nov 2012, #02; Fri, 9)
From: Felipe Contreras @ 2012-11-10 19:13 UTC (permalink / raw)
To: SZEDER Gábor; +Cc: Jeff King, git, Sverre Rabbelier
In-Reply-To: <20121110123250.GR12052@goldbirke>
On Sat, Nov 10, 2012 at 1:32 PM, SZEDER Gábor <szeder@ira.uka.de> wrote:
> Hi,
>
> On Fri, Nov 09, 2012 at 07:33:31PM -0500, Jeff King wrote:
>> On Sat, Nov 10, 2012 at 12:21:48AM +0100, Felipe Contreras wrote:
>> > > * fc/completion-test-simplification (2012-10-29) 2 commits
>> > > - completion: simplify __gitcomp test helper
>> > > - completion: refactor __gitcomp related tests
>> > >
>> > > Clean up completion tests.
>> > >
>> > > There were some comments on the list.
>> > >
>> > > Expecting a re-roll.
>> >
>> > The second patch I can re-roll, but the first patch needs some
>> > external input. My preference is that tests should also be simple and
>> > maintainable, SZEDER's preference is that tests are better being
>> > explicit and verbose (even if harder to maintain) to minimize possible
>> > issues in the tests.
>>
>> I think it is better to keep the tests simple and maintainable.
>
> Maintainable? There is nothing to maintain here. Users' completion
> scripts depend on __gitcomp(), so its behavior shouldn't be changed.
> It can only be extended by a fifth parameter or by quoting words when
> necessary, but these future changes must not alter the current
> behavior checked by these tests, therefore even then these tests must
> be left intact.
I disagree. If we add a new parameter to __gitcomp(), and we need to
add a new parameter to test_gitcomp(), so be it. Yes, we might change
the behavior of the other tests, but that's what reviews are for: to
make sure we don't alter other behavior by mistake. That's what we do
for code, and that what we should do for tests.
But in this particular case nothing would need to change because
test_gitcomp() would pass whatever arguments it receives, being four,
or five, or twenty. So this is not a concern, maybe some other kind of
change, but not this.
Compare this:
http://article.gmane.org/gmane.comp.version-control.git/208168
To this:
http://article.gmane.org/gmane.comp.version-control.git/207927
If we ever need to make changes to the __gitcomp tests, a small change
is better than a big change; IOW: the test code would be more
maintainable.
Even more, the end result is much less code: less code = more maintainability.
> Simple? Currently you only need to look at __gitcomp() and the test
> itself to understand what's going on. With this series you'll also
> need to look at test_gitcomp(), figure out what its parameters are
> supposed to mean, and possibly get puzzled on the way why __gitcomp()
> is now seemingly called with only one parameter.
Maybe it's easier for you to understand, but certainly not for other
people: 'declare -a COMPREPLY'? cur? print_comp? What does that even
mean? Chances are most people don't even know what __gitcomp is.
Fortunately they don't have to dig too deep to find all those things
out, but they can do the same for test_gitcomp().
It might make sense to add some comment on top of test_gitcomp to
explain the arguments and the input to make things easier, but that
would only make it more readable, not less, than the current
situation, because right now you would have to add a similar comment
to each and every block of code that calls __gitcomp.
Functions make our life easier.
> So, I don't see much benefit in this series (except the part to use
> print_comp instead of "change IFS && echo", but that's already done in
> this patch:
> http://article.gmane.org/gmane.comp.version-control.git/207927).
Yes, and that version is much bigger than this:
http://article.gmane.org/gmane.comp.version-control.git/208168
Code that allows the later patch is more maintainable.
> OTOH, this series has some serious drawbacks.
>
> It makes debugging more difficult. While working on the quoting
> issues I managed to break completion tests many-many times lately. In
> normal tests I could add a few debugging instructions to the failed
> test to find out where the breakage lies, without affecting other
> tests. However, if the failed test uses the test_completion() helper,
> then I have to add debugging instructions to test_completion() itself,
> too. This is bad, because many tests use this helper function and are
> therefore affected by the debugging instructions, producing truckloads
> of output making it difficult to dig out the relevant parts, or, worse
> yet, causing breakages in other tests. With this series the same
> difficulties will come to __gitcomp() tests, too.
What I do is copy the function to test_gitcomp2() and add the
debugging there, and only call it from the places where I want the
debugging. I don't think this is an issue.
In fact, I think it's an advantage; sometimes that's exactly what you
want, to add the debugging for everything.
> It can also encourage writing bad tests, similar to those that managed
> to cram many test_completion() lines into a single tests, giving me
> headaches to figure out what went wrong this time.
That's policy. We can decide what each test-case contains right here,
on the mailing list. There's no need to have verbose code to slightly
hint a policy.
Cheers.
--
Felipe Contreras
^ permalink raw reply
* Re: [PATCH v3 1/3] git-submodule add: Add -r/--record option
From: W. Trevor King @ 2012-11-10 19:11 UTC (permalink / raw)
To: Matt Kraai; +Cc: Git
In-Reply-To: <20121109104607.GC4406@ftbfs.org>
On Fri, Nov 09, 2012 at 02:46:07AM -0800, Matt Kraai wrote:
> On Thu, Nov 08, 2012 at 10:35:12PM -0500, W. Trevor King wrote:
> > @@ -366,6 +379,10 @@ Use -f if you really want to add it." >&2
> >
> > git config -f .gitmodules submodule."$sm_path".path "$sm_path" &&
> > git config -f .gitmodules submodule."$sm_path".url "$repo" &&
> > + if test -n "$branch"
> > + then
> > + git config -f .gitmodules submodule."$sm_path".branch "$record_branch"
> > + fi &&
> > git add --force .gitmodules ||
> > die "$(eval_gettext "Failed to register submodule '\$sm_path'")"
> > }
>
> Should the if condition test that $record_branch is not the empty
> string instead of testing that $branch is not the empty string? It
> seems like this will set submodule."$sm_path".branch to the empty
> string if -b is specified and no -r option is specified.
Oops, thanks for catching that. Will fix with v4, once we figure out
what to do about the semantic-pull situation.
--
This email may be signed or encrypted with GnuPG (http://www.gnupg.org).
For more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy
^ permalink raw reply
* Re: [PATCH v3 1/3] git-submodule add: Add -r/--record option
From: W. Trevor King @ 2012-11-10 18:44 UTC (permalink / raw)
To: Junio C Hamano
Cc: Git, Jeff King, Phil Hord, Shawn Pearce, Jens Lehmann, Nahor
In-Reply-To: <7v390jqlep.fsf@alter.siamese.dyndns.org>
On Thu, Nov 08, 2012 at 11:34:54PM -0800, Junio C Hamano wrote:
> "W. Trevor King" <wking@tremily.us> writes:
>
> > By remaining agnostic on the variable usage, this patch makes
> > submodule setup more convenient for all parties.
>
> I personally do not think "remaining agnostic on the usage" is a
> good thing, at least for any option to commands at the higher level
> on the stack, such as "git submodule". I am afraid that giving an
> easier way to set up a variable with undefined semantics may make
> setup more confusing for all parties. One party gives one specific
> meaning to the field, while another party uses it for something
> slightly different.
>
> I would not object to "git config submodule.$name.branch $value", on
> the other hand. "git config" can be used to set a piece of data
> that has specific meaning, but as a low-level tool, it is not
> _limited_ to variables that have defined meaning.
This is what I'm doing now:
$ git submodule add -b <branch> <repo> <path>
$ git config --file .gitmodules submodule.<path>.branch <branch>
$ git submodule foreach 'git checkout $(git config --file $toplevel/.gitmodules submodule.$name.branch) && git pull'
With my second patch (Phil's config export), that becomes
$ git submodule add -b <branch> <repo> <path>
$ git config --file .gitmodules submodule.<path>.branch <branch>
$ git submodule foreach 'git checkout $submodule_branch && git pull'
With my first patch, that becomes
$ git submodule add -rb <branch> <repo> <path>
$ git submodule foreach 'git checkout $submodule_branch && git pull'
This seems pretty useful to me, but I'm still using
submodule.<name>.branch explicitly as a user, and Git is not
interpreting the option directly. Users are free to store whatever
they like in that option, and use it however they wish:
$ git submodule foreach 'do-crazy-stuff.sh $submodule_branch'
If we need a semantic interpretation to justify -r/--record, everyone
that's chimed in so far has agreed on the same interpretation. I
wouldn't be averse to
$ git submodule add -rb <branch> <repo> <path>
$ git submodule pull-branch
which makes the foreach pull logic internal. However, there has been
a reasonable amount of resistance to this workflow in the past, so I
thought that a patch series that avoided a semantic interpretation
would be more acceptable.
If neither an agnostic -r/--record or a semantic pull-branch command
are acceptable, I suppose we'll have to drop my first and third
patches and only keep the second.
Trevor
--
This email may be signed or encrypted with GnuPG (http://www.gnupg.org).
For more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy
^ 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