* Re: [PATCH] MSVC: fix build warnings
From: Junio C Hamano @ 2009-10-05 2:41 UTC (permalink / raw)
To: Michael Wookey; +Cc: Junio C Hamano, git
In-Reply-To: <d2e97e800910021628t13bba313he119ba59babdecee@mail.gmail.com>
Michael Wookey <michaelwookey@gmail.com> writes:
> I can't build with -Werror on Ubuntu 9.04 (gcc 4.3.3) because of the following:
>
> http://article.gmane.org/gmane.comp.version-control.git/127477
I think that has been fixed already while I was away ;-)
^ permalink raw reply
* Re: [PATCH 2/6 (v4)] basic revision cache system, no integration or features
From: Junio C Hamano @ 2009-10-05 2:15 UTC (permalink / raw)
To: Nick Edelen
Cc: Junio C Hamano, Nicolas Pitre, Johannes Schindelin, Sam Vilain,
Michael J Gruber, Jeff King, Shawn O. Pearce, Andreas Ericsson,
Christian Couder, git@vger.kernel.org
In-Reply-To: <op.u061bavktdk399@sirnot.ed.ac.uk>
The entire series is unusable due to whitespace breakage.
^ permalink raw reply
* Re: [PATCH] Documentation - pt-BR.
From: Junio C Hamano @ 2009-10-05 1:53 UTC (permalink / raw)
To: Thiago Farina; +Cc: git
In-Reply-To: <1253730339-11146-1-git-send-email-tfransosi@gmail.com>
Thiago Farina <tfransosi@gmail.com> writes:
> diff --git a/Documentation/pt_BR/gittutorial.txt b/Documentation/pt_BR/gittutorial.txt
> index 81e7ad7..beba065 100644
> --- a/Documentation/pt_BR/gittutorial.txt
> +++ b/Documentation/pt_BR/gittutorial.txt
> @@ -1,15 +1,15 @@
> gittutorial(7)
> ==============
>
> -NAME
> +NOME
> ----
> gittutorial - Um tutorial de introdução ao git (para versão 1.5.1 ou mais nova)
>
> -SYNOPSIS
> +SINOPSE
> --------
> git *
>
> -DESCRIPTION
> +DESCRIÇÃO
> -----------
Not reading Portuguese, I have two comments.
- How well does AsciiDoc and its manpage backend work with these standard
section names localized?
- The length of the underline must match the section header word it
underlines.
Has anybody actually tried to format this document, either before or after
your patch?
^ permalink raw reply
* Re: [PATCH RESEND] git submodule add: make the <path> parameter optional
From: Junio C Hamano @ 2009-10-05 1:31 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: Jens Lehmann, Lars Hjemli, git
In-Reply-To: <alpine.DEB.1.00.0910042304060.4985@pacific.mpi-cbg.de>
Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> So far, I started submodules by cloning them, doing everything in the
> other files needed to integrate, and then actually wondered why "git
> submodule add" could not simply take the (relative) path to the
> checked-out submodule and deduce the URL from the corresponding config?
Let me try to rephrase the above to see if I understand what you are
doing. When building a top-level superproject that uses two submodules
from other places, you would do:
$ git init toplevel
$ cd toplevel
$ git clone $overthere submodule1
$ git clone $elsewhere submodule2
$ edit Makefile common.h
$ git add Makefile common.h submodule1 submodule2
and by "the corresponding config", you mean "submodule1/.git/config
already records that it came from $overthere, and there is no reason to
say it again in 'git submodule add $overthere submodule1'".
If that is the case, I think it is a very sane argument. It also makes me
wonder why we need "git submodule add" as a separate step to begin with
(i.e. "git add" Porcelain could do that behind the scene), but that is
probably a separate topic.
> So I would actually vote for making the <repository> parameter optional...
In your "git submodule add submodule1", it would be quite clear that it is
a local path and <repository> is being omitted. On the other hand, if you
said "git submodule add $overthere" without submodule1, because $overthere
is not likely to be an in-tree path, it also would be clear that it is
omitting the path. IOW, these two typesavers are not mutually exclusive.
In real life, it is very likely $overthere does _not_ end with submodule1,
and your suggestion would probably be more useful than the patch under
discussion, I think.
^ permalink raw reply
* Re: [PATCH] tests: make all test files executable
From: Mark Rada @ 2009-10-05 1:25 UTC (permalink / raw)
To: Jeff King; +Cc: Mark Rada, git
In-Reply-To: <20091004134022.GA14209@sigill.intra.peff.net>
[-- Attachment #1: Type: text/plain, Size: 2086 bytes --]
On 09-10-04 9:40 AM, Jeff King wrote:
> On Sun, Oct 04, 2009 at 09:18:20AM -0400, Mark Rada wrote:
>
>>> Ah, nevermind. The problem is that your patch was word-wrapped, making
>>> the second "diff --git" line bogus. It would have been nice to have it
>>> print a warning instead of silently ignoring that bit of the patch.
>>>
>> I didn't have format=flowed buggering things up this time, so I don't
>> quite understand the problem; could you please explain with more
>> details?
>
> Sure. The patch is perfect except for one line. What should have been:
>
> diff --git a/t/t9501-gitweb-standalone-http-status.sh b/t/t9501-gitweb-standalone-http-status.sh
>
> was wrapped to:
>
> diff --git a/t/t9501-gitweb-standalone-http-status.sh
> b/t/t9501-gitweb-standalone-http-status.sh
>
> I have no idea how you did that, though. :)
>
> It looks like you send with Thunderbird. How do you get the diff content
> into the email? Is it possible that it wraps the content after you have
> gotten it there?
I don't think so, I have plug-in that disables wrapping and has worked
just fine for the last couple of patches I sent, so I'm not sure what
was going on there.
>> When I try to apply the patch from a saved copy of the e-mail, I get
>> the following error:
>>
>> # git am ~/Downloads/\[PATCH\]\ tests_\ make\ all\ test\ files\
>> executable.eml
>> Patch format detection failed.
>> zsh: exit 1 git am
>>
>> The difference between the patch created by format-patch and the saved
>> e-mail is just some e-mail header information. Is that a different error
>> than what you were getting? I'm not sure what I'm doing wrong here, help
>> would be appreciated.
>
> Yeah, that's totally different than the problem I was having. I save to
> an mbox from mutt, which "git am" understands just fine. I'd have to see
> what was in your .eml file to know why "git am" couldn't figure it out
> (and it might be a good test case, as "git am" has recently learned to
> accept more mailbox formats).
I've attached a copy of the .eml file.
--
Mark Rada (ferrous26)
marada@uwaterloo.ca
[-- Attachment #2: [PATCH] tests_ make all test files executable.eml --]
[-- Type: message/rfc822, Size: 1142 bytes --]
Subject:
From:
Mark Rada <marada@mailservices.uwaterloo.ca>
Date:
Thu, 01 Oct 2009 21:59:20 -0400
To:
Junio C Hamano <gitster@pobox.com>
CC:
git@vger.kernel.org
Rm9yIGNvbnNpc3RlbmN5IHdpdGggdGhlIHJlc3Qgb2YgdGhlIHRlc3QgZmlsZXMuCgpTaWdu
ZWQtb2ZmLWJ5OiBNYXJrIFJhZGEgPG1hcmFkYUB1d2F0ZXJsb28uY2E+Ci0tLQoKT24gMjAw
OS0xMC0wMSwgYXQgNDoxMyBBTSwgSmFrdWIgTmFyZWJza2kgd3JvdGU6Cj4+ID4+IGRpZmYg
LS1naXQgYS90L3Q5NTAxLWdpdHdlYi1zdGFuZGFsb25lLWh0dHAtc3RhdHVzLnNoIGIvdC90
OTUwMS1naXR3ZWItc3RhbmRhbG9uZS1odHRwLXN0YXR1cy5zaAo+PiA+PiBpbmRleCBkMGZm
MjFkLi4wNjg4YTU3IDEwMDY0NAo+PiA+PiAtLS0gYS90L3Q5NTAxLWdpdHdlYi1zdGFuZGFs
b25lLWh0dHAtc3RhdHVzLnNoCj4+ID4+ICsrKyBiL3QvdDk1MDEtZ2l0d2ViLXN0YW5kYWxv
bmUtaHR0cC1zdGF0dXMuc2gKPiA+IAo+ID4gQlRXLiB0aGUgcmVzdCBvZiB0ZXN0IHNjcmlw
dHMgYXJlIGV4ZWN1dGFibGUsIGJ1dCBub3QgdGhpcyBvbmU/IFdoeT8KPiA+IChCdXQgY29y
cmVjdGluZyB0aGlzIHNob3VsZCBiZSBkb25lLCBpZiBuZWVkZWQsIGluIHNlcGFyYXRlIGNv
bW1pdCkuCgpJIG5vdGljZWQgb25lIG90aGVyIHRlc3Qgc2NyaXB0IHRoYXQgd2FzIG5vdCBz
ZXQgdG8gYmUgZXhlY3V0YWJsZS4KCgogMCBmaWxlcyBjaGFuZ2VkLCAwIGluc2VydGlvbnMo
KyksIDAgZGVsZXRpb25zKC0pCiBtb2RlIGNoYW5nZSAxMDA2NDQgPT4gMTAwNzU1IHQvdDU1
MzEtZGVlcC1zdWJtb2R1bGUtcHVzaC5zaAogbW9kZSBjaGFuZ2UgMTAwNjQ0ID0+IDEwMDc1
NSB0L3Q5NTAxLWdpdHdlYi1zdGFuZGFsb25lLWh0dHAtc3RhdHVzLnNoCgpkaWZmIC0tZ2l0
IGEvdC90NTUzMS1kZWVwLXN1Ym1vZHVsZS1wdXNoLnNoIGIvdC90NTUzMS1kZWVwLXN1Ym1v
ZHVsZS1wdXNoLnNoCm9sZCBtb2RlIDEwMDY0NApuZXcgbW9kZSAxMDA3NTUKZGlmZiAtLWdp
dCBhL3QvdDk1MDEtZ2l0d2ViLXN0YW5kYWxvbmUtaHR0cC1zdGF0dXMuc2gKYi90L3Q5NTAx
LWdpdHdlYi1zdGFuZGFsb25lLWh0dHAtc3RhdHVzLnNoCm9sZCBtb2RlIDEwMDY0NApuZXcg
bW9kZSAxMDA3NTUKLS0gMS42LjUucmMyIA==
^ permalink raw reply
* Re: [PATCH] Reserve a slot for argv[0] in default_arg.
From: Junio C Hamano @ 2009-10-04 22:20 UTC (permalink / raw)
To: Jeff King; +Cc: Petter Urkedal, git, Stephen Boyd
In-Reply-To: <20091004182746.GA22995@coredump.intra.peff.net>
Stephen Boyd <bebarino@gmail.com> writes:
> On Sun, 2009-10-04 at 14:27 -0400, Jeff King wrote:
>>
>> Ah, thanks, for some reason I wasn't able to produce it before, but I
>> can easily replicate it here. I think it's a regression from converting
>> show-branch to use parse_options, which happened in May, but I didn't
>> actually bisect it. I'm not sure showbranch.default has worked at all
>> since then (which I guess goes to show how many people are actually
>> using it).
It is a command specific aliasing mechanism; not even I use the feature
these days, since "alias.*" is much easier to use. But there is no strong
need to remove it either; it is not too much hassle to keep it for people
who do use it. Perhaps deprecate it and remove it in the long run?
> Correct. Junio sent a patch to fix this problem in June[1]. I guess he
> must have dropped his own patch, or he wasn't satisfied with how parse
> options clobbers things.
>
> [1] http://article.gmane.org/gmane.comp.version-control.git/121142
I had it kept still in my Inbox; thanks for noticing. Petter's patch does
essentially the same thing, but the old patch had a better log message
that described where in the history the fix should apply, so I'd probably
use that with your test squashed in.
Thanks.
^ permalink raw reply
* Re: [PATCH] Add the --submodule-summary option to the diff option family
From: Junio C Hamano @ 2009-10-04 22:19 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: git, Jens Lehmann
In-Reply-To: <67a884457aeaead275612be10902a80726b2a7db.1254668669u.git.johannes.schindelin@gmx.de>
Johannes Schindelin <johannes.schindelin@gmx.de> writes:
> Now you can see the submodule summaries inlined in the diff, instead of
> not-quite-helpful SHA-1 pairs.
This looks useful, but I do not think this is --summary. It is not part
of the "summary" output but is about making output for Submoodules more
verbose. I'd suggest naming it --verbose-submodule or something.
> The format imitates what "git submodule summary" shows.
The output format needs to be described better here and also in
Documentation/diff-format.txt.
> To do that, <path>/.git/objects/ is added to the alternate object
> databases (if that directory exists).
Is it always true that <path>/.git is the GIT_DIR for that submodule? Not
complaining but checking sanity.
> diff --git a/diff.c b/diff.c
> index 9e00131..cdef322 100644
> --- a/diff.c
> +++ b/diff.c
> @@ -1557,6 +1558,17 @@ static void builtin_diff(const char *name_a,
> const char *a_prefix, *b_prefix;
> const char *textconv_one = NULL, *textconv_two = NULL;
>
> + if (DIFF_OPT_TST(o, SUMMARIZE_SUBMODULES) &&
> + (!one->mode || S_ISGITLINK(one->mode)) &&
> + (!two->mode || S_ISGITLINK(two->mode))) {
> + const char *del = diff_get_color_opt(o, DIFF_FILE_OLD);
> + const char *add = diff_get_color_opt(o, DIFF_FILE_NEW);
> + show_submodule_summary(o->file, one ? one->path : two->path,
> + one->sha1, two->sha1,
> + del, add, reset);
> + return;
> + }
> +
Isn't this "textual diff" codepath?
I would have expected --submodule-summary output to be near the --summary
output and to be generated in the same codepath to generate it; if this
were named --verbose-submodule, I would certainly understand it, and I
think the placement is saner in the textual diff.
> diff --git a/submodule.c b/submodule.c
> new file mode 100644
> index 0000000..3f2590d
> --- /dev/null
> +++ b/submodule.c
> @@ -0,0 +1,108 @@
> +#include "cache.h"
> +#include "submodule.h"
> +#include "dir.h"
> +#include "diff.h"
> +#include "commit.h"
> +#include "revision.h"
> +
> +int add_submodule_odb(const char *path)
> +{
> + struct strbuf objects_directory = STRBUF_INIT;
> + struct alternate_object_database *alt_odb;
> +
> + strbuf_addf(&objects_directory, "%s/.git/objects/", path);
> + if (!is_directory(objects_directory.buf))
> + return -1;
> +
> + /* avoid adding it twice */
> + for (alt_odb = alt_odb_list; alt_odb; alt_odb = alt_odb->next)
> + if (alt_odb->name - alt_odb->base == objects_directory.len &&
> + !strncmp(alt_odb->base, objects_directory.buf,
> + objects_directory.len))
> + return 0;
> + alt_odb = xmalloc(objects_directory.len + 42 + sizeof(*alt_odb));
> + alt_odb->next = alt_odb_list;
> + strcpy(alt_odb->base, objects_directory.buf);
> + alt_odb->name = alt_odb->base + objects_directory.len;
> + alt_odb->name[2] = '/';
> + alt_odb->name[40] = '\0';
> + alt_odb->name[41] = '\0';
> + alt_odb_list = alt_odb;
> + prepare_alt_odb();
The lines after "avoid adding it twice" look somewhat familiar.
Don't we already have other codepaths to add an alternate odb that need to
be careful in the same way (i.e. do not add twice, do not loop, etc.), and
if so don't you want to reuse it after refactoring?
> +void show_submodule_summary(FILE *f, const char *path,
> + unsigned char one[20], unsigned char two[20],
> + const char *del, const char *add, const char *reset)
> +{
> + struct rev_info rev;
> + struct commit *commit, *left, *right;
> + struct commit_list *merge_bases, *list;
> + const char *message = NULL;
> + struct strbuf sb = STRBUF_INIT;
> + static const char *format = " %m %s";
> + int fast_forward = 0, fast_backward = 0;
> +
> + if (add_submodule_odb(path))
> + message = "(not checked out)";
> + else if (is_null_sha1(one))
> + message = "(new submodule)";
> + else if (is_null_sha1(two))
> + message = "(submodule deleted)";
Are you sure about this? Wouldn't "git diff HEAD" (not looking at the
index) give you the 0{40} on the "two" side when the path is modified?
> + if (!message) {
> + init_revisions(&rev, NULL);
> + setup_revisions(0, NULL, &rev, NULL);
> + rev.left_right = 1;
> + left->object.flags |= SYMMETRIC_LEFT;
> + add_pending_object(&rev, &left->object, path);
> + add_pending_object(&rev, &right->object, path);
> + merge_bases = get_merge_bases(left, right, 1);
> + if (merge_bases) {
> + if (merge_bases->item == left)
> + fast_forward = 1;
> + else if (merge_bases->item == right)
> + fast_backward = 1;
> + }
> + for (list = merge_bases; list; list = list->next) {
> + list->item->object.flags |= UNINTERESTING;
> + add_pending_object(&rev, &list->item->object,
> + sha1_to_hex(list->item->object.sha1));
> + }
> + if (prepare_revision_walk(&rev))
> + message = "(revision walker failed)";
If prepare_revision_walk() failed for whatever reason, can we trust
fast_forward/fast_backward at this point?
> + }
> +
> + strbuf_addf(&sb, "Submodule %s %s..", path,
> + find_unique_abbrev(one, DEFAULT_ABBREV));
> + if (!fast_backward && !fast_forward)
> + strbuf_addch(&sb, '.');
> + strbuf_addf(&sb, "%s", find_unique_abbrev(two, DEFAULT_ABBREV));
> + if (message)
> + strbuf_addf(&sb, " %s\n", message);
> + else
> + strbuf_addf(&sb, "%s:\n", fast_backward ? " (rewind)" : "");
No corresponding "(fast-forward)" label?
> + fwrite(sb.buf, sb.len, 1, f);
> +
> + if (!message) {
> + while ((commit = get_revision(&rev))) {
> + strbuf_setlen(&sb, 0);
> + if (del)
> + strbuf_addstr(&sb, commit->object.flags &
> + SYMMETRIC_LEFT ? del : add);
> + format_commit_message(commit, format, &sb,
> + rev.date_mode);
> + if (del)
> + strbuf_addstr(&sb, reset);
Three points, two of which are minor.
- Checking "del" to decide if you want to say "reset" feels funny.
- In the "ANSI-terminal only" world view, adding colors to strbuf and
writing it out together with the actual strings is an easy thing to do.
Don't Windows folks have trouble converting this kind of code to their
color control call that is separate from writing strings out? If it is
not a problem, I do not have any objection to it, but otherwise I'd
suggest not to add any more code that stores color escape sequence in
strbuf, so that we would not make later conversion by Windows folks
harder than necessary.
- The output is similar but different from "submodule --summary" and
there is no justification described in the patch nor the proposed
commit log message.
o Why does "submodule --summary" use --first-parent and this patch
doesn't?
o "submodule --summary" allows users to limit the walk but this seems
to give the full list---is it useful (or even sane) to always give
a full list?
> + strbuf_addch(&sb, '\n');
> + fwrite(sb.buf, sb.len, 1, f);
> + }
> + clear_commit_marks(left, ~0);
> + clear_commit_marks(right, ~0);
> + }
Aren't object flags shared between the outer revision walker that walks
the log and this new walker? If the supermodule history and submodule
history share commits (e.g. when later git.git starts using submodules to
bind gitk and git-gui), wouldn't this break "git log -p" a big way?
In any case, thanks for an interesting patch. Looking forward to see how
this topic evolves.
^ permalink raw reply
* Re: [PATCH] Teach 'rebase -i' the command "amend"
From: Johannes Schindelin @ 2009-10-04 21:09 UTC (permalink / raw)
To: Björn Gustavsson; +Cc: git, gitster
In-Reply-To: <4AC8F22F.5070101@gmail.com>
[-- Attachment #1: Type: TEXT/PLAIN, Size: 337 bytes --]
Hi,
On Sun, 4 Oct 2009, Björn Gustavsson wrote:
> Make it easier to edit just the commit message for a commit
> using 'git rebase -i' by introducing the "amend" command.
I thought that we had a discussion about this and that the consensus was
that "amend" would be misleading. Maybe you can find that thread in
GMane?
Ciao,
Dscho
^ permalink raw reply
* Re: [PATCH RESEND] git submodule add: make the <path> parameter optional
From: Johannes Schindelin @ 2009-10-04 21:05 UTC (permalink / raw)
To: Jens Lehmann; +Cc: Junio C Hamano, Lars Hjemli, git
In-Reply-To: <4AC8E0A8.4000901@web.de>
Hi,
On Sun, 4 Oct 2009, Jens Lehmann wrote:
> Junio C Hamano schrieb:
> > Jens Lehmann <Jens.Lehmann@web.de> writes:
> >
> >> When <path> is not given, use the "humanish" part of the source repository
> >> instead.
> >>
> >> Signed-off-by: Jens Lehmann <Jens.Lehmann@web.de>
> >> ---
> >>
> >> With this patch, git submodule add behaves like git clone in this respect.
> >>
> >> Didn't get a response the last weeks, so here is a resend.
> >>
> >>
> >> Documentation/git-submodule.txt | 8 ++++++--
> >> git-submodule.sh | 7 ++++++-
> >> t/t7400-submodule-basic.sh | 16 ++++++++++++++++
> >> 3 files changed, 28 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt
> >> index 5ccdd18..4ef70c4 100644
> >> --- a/Documentation/git-submodule.txt
> >> +++ b/Documentation/git-submodule.txt
> >> @@ -10,7 +10,7 @@ SYNOPSIS
> >> --------
> >> [verse]
> >> 'git submodule' [--quiet] add [-b branch]
> >> - [--reference <repository>] [--] <repository> <path>
> >> + [--reference <repository>] [--] <repository> [<path>]
> >> 'git submodule' [--quiet] status [--cached] [--recursive] [--] [<path>...]
> >> 'git submodule' [--quiet] init [--] [<path>...]
> >> 'git submodule' [--quiet] update [--init] [-N|--no-fetch] [--rebase]
> >> @@ -69,7 +69,11 @@ add::
> >> to the changeset to be committed next to the current
> >> project: the current project is termed the "superproject".
> >> +
> >> -This requires two arguments: <repository> and <path>.
> >> +This requires at least one argument: <repository>. The optional
> >> +argument <path> is the relative location for the cloned submodule
> >> +to exist in the superproject. If <path> is not given, the
> >> +"humanish" part of the source repository is used ("repo" for
> >> +"/path/to/repo.git" and "foo" for "host.xz:foo/.git").
> >
> > I do not know if this is useful in practice nor even desired. Comments?
>
> As nobody commented until now, i'll explain my motivation for this patch.
>
> When adding submodules i was surprised to find that i had to explicitly
> provide the pathname even though it could be easily generated from the
> reponame as git clone does it. And i see git clone and git submodule add
> as related commands from a users perspective, they both connect a remote
> repo to a working directory.
>
> IMHO this patch makes the ui more consistent and doesn't break existing
> setups or scripts. And it is really useful because i don't do typos in
> the pathname anymore ;-)
So far, I started submodules by cloning them, doing everything in the
other files needed to integrate, and then actually wondered why "git
submodule add" could not simply take the (relative) path to the
checked-out submodule and deduce the URL from the corresponding config?
So I would actually vote for making the <repository> parameter optional...
Ciao,
Dscho
^ permalink raw reply
* Re: [PATCH] Reserve a slot for argv[0] in default_arg.
From: Stephen Boyd @ 2009-10-04 20:02 UTC (permalink / raw)
To: Jeff King; +Cc: Petter Urkedal, git, gitster
In-Reply-To: <20091004182746.GA22995@coredump.intra.peff.net>
On Sun, 2009-10-04 at 14:27 -0400, Jeff King wrote:
>
> Ah, thanks, for some reason I wasn't able to produce it before, but I
> can easily replicate it here. I think it's a regression from converting
> show-branch to use parse_options, which happened in May, but I didn't
> actually bisect it. I'm not sure showbranch.default has worked at all
> since then (which I guess goes to show how many people are actually
> using it).
Correct. Junio sent a patch to fix this problem in June[1]. I guess he
must have dropped his own patch, or he wasn't satisfied with how parse
options clobbers things.
[1] http://article.gmane.org/gmane.comp.version-control.git/121142
^ permalink raw reply
* [PATCH] Teach 'rebase -i' the command "amend"
From: Björn Gustavsson @ 2009-10-04 19:06 UTC (permalink / raw)
To: git; +Cc: gitster
Make it easier to edit just the commit message for a commit
using 'git rebase -i' by introducing the "amend" command.
Signed-off-by: Björn Gustavsson <bgustavsson@gmail.com>
---
Here is my first ever patch of git.
I find that I often use 'git rebase -i' just to correct typos
in my commit messages or to fill in more details, so I thought
it would be a good idea to add to add an "amend" command to
'git rebase -i'.
/Björn
Documentation/git-rebase.txt | 3 +++
git-rebase--interactive.sh | 9 +++++++++
t/lib-rebase.sh | 6 +++---
t/t3404-rebase-interactive.sh | 14 ++++++++++++++
4 files changed, 29 insertions(+), 3 deletions(-)
diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
index 0aefc34..28b90ec 100644
--- a/Documentation/git-rebase.txt
+++ b/Documentation/git-rebase.txt
@@ -368,6 +368,9 @@ By replacing the command "pick" with the command "edit", you can tell
the files and/or the commit message, amend the commit, and continue
rebasing.
+If you just want to edit the commit message for a commit, you can replace
+the command "pick" with the command "amend".
+
If you want to fold two or more commits into one, replace the command
"pick" with "squash" for the second and subsequent commit. If the
commits had different authors, it will attribute the squashed commit to
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 23ded48..3f6bcc0 100755
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -340,6 +340,14 @@ do_next () {
pick_one $sha1 ||
die_with_patch $sha1 "Could not apply $sha1... $rest"
;;
+ amend|a)
+ comment_for_reflog amend
+
+ mark_action_done
+ pick_one $sha1 ||
+ die_with_patch $sha1 "Could not apply $sha1... $rest"
+ output git commit --amend
+ ;;
edit|e)
comment_for_reflog edit
@@ -752,6 +760,7 @@ first and then run 'git rebase --continue' again."
#
# Commands:
# p, pick = use commit
+# a, amend = use commit, but allow editing of the commit message
# e, edit = use commit, but stop for amending
# s, squash = use commit, but meld into previous commit
#
diff --git a/t/lib-rebase.sh b/t/lib-rebase.sh
index 260a231..2ca0481 100644
--- a/t/lib-rebase.sh
+++ b/t/lib-rebase.sh
@@ -9,8 +9,8 @@
#
# "[<lineno1>] [<lineno2>]..."
#
-# If a line number is prefixed with "squash" or "edit", the respective line's
-# command will be replaced with the specified one.
+# If a line number is prefixed with "squash", "edit", or "amend", the
+# respective line's command will be replaced with the specified one.
set_fake_editor () {
echo "#!$SHELL_PATH" >fake-editor.sh
@@ -32,7 +32,7 @@ cat "$1".tmp
action=pick
for line in $FAKE_LINES; do
case $line in
- squash|edit)
+ squash|edit|amend)
action="$line";;
*)
echo sed -n "${line}s/^pick/$action/p"
diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
index 4cae019..3520480 100755
--- a/t/t3404-rebase-interactive.sh
+++ b/t/t3404-rebase-interactive.sh
@@ -470,4 +470,18 @@ test_expect_success 'avoid unnecessary reset' '
test 123456789 = $MTIME
'
+test_expect_success 'amend' '
+ git checkout -b amend-branch master &&
+ FAKE_LINES="1 2 3 amend 4" FAKE_COMMIT_MESSAGE="E changed" git rebase -i A &&
+ git show HEAD | grep "E changed" &&
+ test $(git rev-parse master) != $(git rev-parse HEAD) &&
+ test $(git rev-parse master^) = $(git rev-parse HEAD^) &&
+ FAKE_LINES="1 2 amend 3 4" FAKE_COMMIT_MESSAGE="D changed" git rebase -i A &&
+ git show HEAD^ | grep "D changed" &&
+ FAKE_LINES="amend 1 2 3 4" FAKE_COMMIT_MESSAGE="B changed" git rebase -i A &&
+ git show HEAD~3 | grep "B changed" &&
+ FAKE_LINES="1 amend 2 3 4" FAKE_COMMIT_MESSAGE="C changed" git rebase -i A &&
+ git show HEAD~2 | grep "C changed"
+'
+
test_done
--
1.6.5.rc2.18.g117a
^ permalink raw reply related
* Read and Reply me
From: Angel Sharon @ 2009-10-04 18:08 UTC (permalink / raw)
[-- Attachment #1: Type: text/plain, Size: 115 bytes --]
Open the attachment and I will be glad to hear from you soon.
Mrs Angel Sharon
Reply me at angel79@btinternet.com
[-- Attachment #2: angel2.txt --]
[-- Type: application/octet-stream, Size: 1276 bytes --]
^ permalink raw reply
* Re: reset doesn't reset a revert?
From: Adam Brewster @ 2009-10-04 18:37 UTC (permalink / raw)
To: Octavian Râşniţă; +Cc: git
In-Reply-To: <98300251CB1D46A0B635B4495138C3A7@teddy>
>
> E:\lucru\git\k>git revert HEAD~
> fatal: Cannot revert a root commit
>
> #Does anyone know what does this mean? So I've tried with HEAD^ instead.
>
Git revert isn't like svn revert. GIt revert creates a new commit off
of the HEAD that introduces a change that's the opposite of the change
introduced in the commit you specify. The first commit in a
repository doesn't introduce any change because there's no previous
commit.
Like it says, you can't revert the first commit in the history.
> E:\lucru\git\k>git revert HEAD^
> More?
> More?
> Finished one revert.
> [master 1beba20] Revert "Added baz to file.txt"
> 1 files changed, 0 insertions(+), 1 deletions(-)
>
> # What should I respond to the questions More?
> #I've seen that no matter what I type, it adds to the "HEAD" and tells that
> that commit can't be found, so I just pressed enter.
>
>
It seems the caret is special to the windows command prompt. I don't
know what it does, but a line of nonsense that ends in ^ causes the
More? More? response you described. Try quoting the argument so your
shell doesn't mangle it.
>
> #And it seems that not only the repository was changed, but the working
> directory also. Is it correct?
>
That's how revert usually works,
> #Well, now let's say I discovered that this new commit was an error and I
> want to reset it.
> #And I used HEAD^ because HEAD~ didn't work with revert.
>
> E:\lucru\git\k>git reset HEAD^
> More?
> More?
> E:\lucru\git\k>git log --pretty=format:"%s %h"
> WARNING: terminal is not fully functional
> Revert "Added baz to file.txt" 1beba20
> Added baz to file.txt fabd2f2
> First commit e969cd5
> (END)
>
> #Well, git reset didn't reset the latest commit.
> #Does anyone know why or what I am doing wrong?
>
I don't know what the ^ means, but I'm guessing it's not getting
passed to git. I'm guessing what you really did is `git reset HEAD`
which clears the index but doesn't undo any commits.
> E:\lucru\git\k>git status
> # On branch master
> nothing to commit (working directory clean)
> E:\lucru\git\k>git reset HEAD~
> file.txt: locally modified
> E:\lucru\git\k>git log --pretty=format:"%s %h"
> WARNING: terminal is not fully functional
> Added baz to file.txt fabd2f2
> First commit e969cd5
> (END)
>
> #This time git reset resetted the latest HEAD.
> #It seems that git reset wants the HEAD~ commit, while git revert wants the
> HEAD^ commit. Do you know why (or can I find an explanation for this
> somewhere)?
>
Windows' command shell sucks. I don't use it. I don't recommend it.
Try Cygwin. Or PuTTYcyg.
> E:\lucru\git\k>type file.txt
> foo
> bar
>
> #However, git reset modified just the repository and not the working
> directory.
>
git reset has three modes. --hard, --soft, and --mixed. If you
weren't using windows' shell you could type man git-reset to find out
about them. Try googling git reset. Basically git reset doesn't mess
with your working copy files unless you ask it to.
> I added the line baz in the file file.txt, commited this change and then
> reverted to the previous commit. This has also deleted the line "baz" from
> the file.
> Then I resetted the last commit (the revert), however the line "baz" didn't
> appear in the file.
>
Reset and revert are very different commands. Make sure you
understand the difference.
Adam
^ permalink raw reply
* Re: [PATCH] Reserve a slot for argv[0] in default_arg.
From: Jeff King @ 2009-10-04 18:27 UTC (permalink / raw)
To: Petter Urkedal; +Cc: git
In-Reply-To: <20091004141355.GA15783@eideticdew.org>
On Sun, Oct 04, 2009 at 04:13:55PM +0200, Petter Urkedal wrote:
> I was wondering myself. I tried to switch off optimisation, but that
> had no effect. I'm suspecting PIE, but it could be some other
> configuration implied by the Gentoo "hardened" use-flag.
Nope, it's just a plain old git bug...
> I can reproduce it on my machine with
>
> mkdir test-repo; cd test-repo
> /path/to/git init
> /path/to/git config showbranch.default --topo-order
> /path/to/git show-branch
Ah, thanks, for some reason I wasn't able to produce it before, but I
can easily replicate it here. I think it's a regression from converting
show-branch to use parse_options, which happened in May, but I didn't
actually bisect it. I'm not sure showbranch.default has worked at all
since then (which I guess goes to show how many people are actually
using it).
So your fix is definitely right, and the test case below (which can be
squashed in) fails reliably without it.
t3202 is maybe a bit of weird place to put it, but we don't seem to test
show-branch anywhere else. It could probably use a "check that
show-branch works at all" set of tests, but I am not volunteering to
write such a thing. I have always found its output to be one step above
line noise.
I also looked at putting it in t1200-tutorial.sh near the show-branch
call, but that script is an utter mess. Most of the tests don't actually
check the exit status of commands, and there is a random "test_done"
halfway through the script which skips all of the later tests (including
the show-branch test!). Removing that to enable the later tests reveals
that they are broken, with such obviously non-working crap as
git merge -s "Merge upstream changes." master
which is clearly bogus. I wonder if we should just remove that script
altogether; at best it just seems redundant with other tests, and it is
full of obvious errors.
> Comment's are treated as whitespace, but I'll adjust it for readability.
> Maybe worse: I missed the 8-column indentation. So, here is the patch
> again (attached, I hope Git can extract it).
Thanks, that looks better (I actually didn't even notice the indent
problem the first time, but yes, it should be 8 columns).
Squashable test case is below.
-Peff
---
diff --git a/t/t3202-show-branch-octopus.sh b/t/t3202-show-branch-octopus.sh
index 7fe4a6e..0a5d5e6 100755
--- a/t/t3202-show-branch-octopus.sh
+++ b/t/t3202-show-branch-octopus.sh
@@ -56,4 +56,12 @@ test_expect_success 'show-branch with more than 8 branches' '
'
+test_expect_success 'show-branch with showbranch.default' '
+ for i in $numbers; do
+ git config --add showbranch.default branch$i
+ done &&
+ git show-branch >out &&
+ test_cmp expect out
+'
+
test_done
^ permalink raw reply related
* Re: [PATCH RESEND] git submodule add: make the <path> parameter optional
From: Jens Lehmann @ 2009-10-04 17:51 UTC (permalink / raw)
To: Junio C Hamano, Lars Hjemli; +Cc: git
In-Reply-To: <7vbpl2srw9.fsf@alter.siamese.dyndns.org>
Junio C Hamano schrieb:
> Jens Lehmann <Jens.Lehmann@web.de> writes:
>
>> When <path> is not given, use the "humanish" part of the source repository
>> instead.
>>
>> Signed-off-by: Jens Lehmann <Jens.Lehmann@web.de>
>> ---
>>
>> With this patch, git submodule add behaves like git clone in this respect.
>>
>> Didn't get a response the last weeks, so here is a resend.
>>
>>
>> Documentation/git-submodule.txt | 8 ++++++--
>> git-submodule.sh | 7 ++++++-
>> t/t7400-submodule-basic.sh | 16 ++++++++++++++++
>> 3 files changed, 28 insertions(+), 3 deletions(-)
>>
>> diff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt
>> index 5ccdd18..4ef70c4 100644
>> --- a/Documentation/git-submodule.txt
>> +++ b/Documentation/git-submodule.txt
>> @@ -10,7 +10,7 @@ SYNOPSIS
>> --------
>> [verse]
>> 'git submodule' [--quiet] add [-b branch]
>> - [--reference <repository>] [--] <repository> <path>
>> + [--reference <repository>] [--] <repository> [<path>]
>> 'git submodule' [--quiet] status [--cached] [--recursive] [--] [<path>...]
>> 'git submodule' [--quiet] init [--] [<path>...]
>> 'git submodule' [--quiet] update [--init] [-N|--no-fetch] [--rebase]
>> @@ -69,7 +69,11 @@ add::
>> to the changeset to be committed next to the current
>> project: the current project is termed the "superproject".
>> +
>> -This requires two arguments: <repository> and <path>.
>> +This requires at least one argument: <repository>. The optional
>> +argument <path> is the relative location for the cloned submodule
>> +to exist in the superproject. If <path> is not given, the
>> +"humanish" part of the source repository is used ("repo" for
>> +"/path/to/repo.git" and "foo" for "host.xz:foo/.git").
>
> I do not know if this is useful in practice nor even desired. Comments?
As nobody commented until now, i'll explain my motivation for this patch.
When adding submodules i was surprised to find that i had to explicitly
provide the pathname even though it could be easily generated from the
reponame as git clone does it. And i see git clone and git submodule add
as related commands from a users perspective, they both connect a remote
repo to a working directory.
IMHO this patch makes the ui more consistent and doesn't break existing
setups or scripts. And it is really useful because i don't do typos in
the pathname anymore ;-)
Jens
^ permalink raw reply
* Re: Git push over git protocol for corporate environment
From: Matthieu Moy @ 2009-10-04 16:26 UTC (permalink / raw)
To: Jakub Narebski; +Cc: Eugene Sajine, git
In-Reply-To: <200910041725.39992.jnareb@gmail.com>
Jakub Narebski <jnareb@gmail.com> writes:
> On Thu, 1 Oct 2009, Eugene Sajine wrote:
>
>> Thanks to everybody for prompt answers!
>
> You are welcome!
>
>> There is one thing I'm still missing though. Do I understand correctly that
>> if a person has an ssh access (account) to the host in internal network,
>> then this won't be enough for him to be able to push to the repo? Should we
>> still go through the hassle of managing the ssh keys for each particular
>> user who is supposed to have push access?
>
> Yes, it is enough to push (and fetch) via SSH protocol.
To be a bit more precise: roughly, there are two ways to manage access
to a Git repo via SSH:
* One unix user (typically called "git") managing the repository, and
eveybody connecting to the repo via ssh://git@.... Then, if you want
any access control within the owned repositories for this user, you
need a key-based authentication to be able to distinguish who's
connecting. This is what gitorious does.
* Everyone has its own unix account, and the repository is shared (via
ACLs or simple group-based permissions, see git init --shared).
Then, each user can choose the way he prefers for authentication,
and if the user has an unrestricted account (i.e. can write
~/.ssh/authorized_keys), then it's the job of the users to manage
this, not the one of the sysadmin.
--
Matthieu Moy
http://www-verimag.imag.fr/~moy/
^ permalink raw reply
* Re: Git push over git protocol for corporate environment
From: Jakub Narebski @ 2009-10-04 15:25 UTC (permalink / raw)
To: Eugene Sajine; +Cc: git
In-Reply-To: <00163623ac5d75929b0474e66b96@google.com>
On Thu, 1 Oct 2009, Eugene Sajine wrote:
> Thanks to everybody for prompt answers!
You are welcome!
> There is one thing I'm still missing though. Do I understand correctly that
> if a person has an ssh access (account) to the host in internal network,
> then this won't be enough for him to be able to push to the repo? Should we
> still go through the hassle of managing the ssh keys for each particular
> user who is supposed to have push access?
Yes, it is enough to push (and fetch) via SSH protocol.
Nevertheless it is better to use key-based authentication, or to be more
exact passwordless authentication, so that you are asked for ssh key
password (if there is one set for given key) only once when adding it
to ssh agent (c.f. ssh-add, ssh-agent, and keychain). Otherwise you
would need to give password over and over (well, I think git uses one
or two connections for fetch / push, so it shouldn't be much of an
issue).
Also you can have SSH key shared via networked filesystem...
Conversely, on the other hand side many git management tool require to
setup only single SSH account, and distinguish between users based on
keys they use.
>
> I believe the answer is yes and that's why I'm leaning towards pulls and
> pushes over git protocol. There is no solution yet which would be as
> effective and simple to maintain. Using git protocol will not add security,
> but it won't be worse than existing CVS or any other centralized version
> control security model. As soon as security comes into play, then we will
> need some other solution, but currently i didn't see anything that would be
> easy to sell to the company.
Git protocol is anonymous and unauthenticated, so you can't (I think)
make any ACL with it. CVS pserver was not secure, but CVS tunnelled over
SSH, or used over SSH was.
Gitosis, Gitolite, SCuMD, repo are all git management tools.
GitHub:FI, Gitosis, Girocco (with github), InDefero are all git hosting
tools (with web interface both for repo browsing and for administration).
Gerrit is git based review board.
> Github is cool, but FI is way too expensive and very hard to sell.
>
> Gitorious is even better!! for corporate use, i think, because of its team
> oriented approach, but... man... I would kill for java implementation or
> anything as simple as that!!
Gitorious is roughly GitHub equivalent, as it is also git hosting
software (a web application).
Both SCuMD and (I think) Gerrit are in Java; SCuMD is SSH server and/or
git repository management tool in early stages of development, Gerrit
is multi-repo management web app and (mainly) review board.
> As i see It is impossible to install in
> network without internet access, and the amount of dependencies which you
> have install/pre-install is enormous. I read somewhere ruby on rails is fun
> to develop with, but is a nightmare to deploy and maintain, and it seems to
> be true. Come on, guys!! Look at the Hudson CI - one war file containing
> everything you need, application starts from command line "java -jar
> hudson.war" and runs on any port you specify. Time to start from download
> to having first build is less the 10 min!!! If there are gitorious guys -
> please, think about it and don't forget to share the profit;)!
I don't know what are tricks that Rubyists use to ease deployment, but
take a look at things such as Gems and Capistrano.
>
> I think Cgit can be something competitive - although i failed to run it
> yet, having some issues with build...and as all other web based stuff, you
> should implement something in order to create and set up bare repos on the
> server automatically (even probably edit the config file via script) to
> avoid a mess and to avoid one guy spending his time adding and configuring
> repos... Probably we will and up using gitweb as it at least knows to scan
> a folder for git repos...although it also gives me troubles installing...
> both with cgit and gitweb are conducted under cygwin, so probably this is
> the real problem with them;)
Both cgit (which is written in C) and gitweb (which is single Perl script,
plus CSS and two images) are git web interfaces. They do not have
features for managing repositories, nor for managing access to git
repositories.
Girocco, which is git hosting software that http://repo.or.cz uses to
manage git repositories, uses (enhanced) gitweb for web interface.
>
> I think that this is what is missing right now in order for git to get
> rocket start and spread inside companies: secure and easy to maintain
> mainline hosting.
Paraphrasing known quote: Git development community have no plans for
world domination; it would be purely accidental side-effect ;-))
Spread inside companies is not a goal; best possible tool for OSS
development is.
>
> Probably all my frustration comes from lack of experience - so, while i
> will continue to work on it, i would really appreciate any advice,
> especially about experience using git not for open source and not in 3
> person's team.
You didn't mention trying Gitosis or Gitolite, which are I think most
commonly used tool for managing git repositories (well, Gitosis is
anyway). Gitosis is even mentioned in "Pro Git" book (http://progit.org).
See e.g. "Hosting Git repositories, The Easy (and Secure) Way"
http://scie.nti.st/2007/11/14/hosting-git-repositories-the-easy-and-secure-way
(from http://git.or.cz/gitwiki/BlogPosts)
[cut rest of reply; please do not toppost, and remove parts of reply
you are not responding to].
--
Jakub Narebski
Poland
^ permalink raw reply
* [PATCH] Add the --submodule-summary option to the diff option family
From: Johannes Schindelin @ 2009-10-04 15:05 UTC (permalink / raw)
To: git, gitster; +Cc: Jens Lehmann
In-Reply-To: <cover.1254668669u.git.johannes.schindelin@gmx.de>
Now you can see the submodule summaries inlined in the diff, instead of
not-quite-helpful SHA-1 pairs.
The format imitates what "git submodule summary" shows.
To do that, <path>/.git/objects/ is added to the alternate object
databases (if that directory exists).
This option was requested by Jens Lehmann at the GitTogether in Berlin.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
Documentation/diff-options.txt | 4 ++
Makefile | 2 +
diff.c | 14 +++++
diff.h | 1 +
submodule.c | 108 ++++++++++++++++++++++++++++++++++++++++
submodule.h | 8 +++
6 files changed, 137 insertions(+), 0 deletions(-)
create mode 100644 submodule.c
create mode 100644 submodule.h
diff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt
index 9276fae..5fcc5a8 100644
--- a/Documentation/diff-options.txt
+++ b/Documentation/diff-options.txt
@@ -87,6 +87,10 @@ endif::git-format-patch[]
Show only names and status of changed files. See the description
of the `--diff-filter` option on what the status letters mean.
+--submodule-summary::
+ Instead of showing pairs of commit names, list the commits in that
+ commit range in the same style as linkgit:git-submodule[1].
+
--color::
Show colored diff.
diff --git a/Makefile b/Makefile
index 2c20922..56d4104 100644
--- a/Makefile
+++ b/Makefile
@@ -449,6 +449,7 @@ LIB_H += sideband.h
LIB_H += sigchain.h
LIB_H += strbuf.h
LIB_H += string-list.h
+LIB_H += submodule.h
LIB_H += tag.h
LIB_H += transport.h
LIB_H += tree.h
@@ -547,6 +548,7 @@ LIB_OBJS += sideband.o
LIB_OBJS += sigchain.o
LIB_OBJS += strbuf.o
LIB_OBJS += string-list.o
+LIB_OBJS += submodule.o
LIB_OBJS += symlinks.o
LIB_OBJS += tag.o
LIB_OBJS += trace.o
diff --git a/diff.c b/diff.c
index 9e00131..cdef322 100644
--- a/diff.c
+++ b/diff.c
@@ -13,6 +13,7 @@
#include "utf8.h"
#include "userdiff.h"
#include "sigchain.h"
+#include "submodule.h"
#ifdef NO_FAST_WORKING_DIRECTORY
#define FAST_WORKING_DIRECTORY 0
@@ -1557,6 +1558,17 @@ static void builtin_diff(const char *name_a,
const char *a_prefix, *b_prefix;
const char *textconv_one = NULL, *textconv_two = NULL;
+ if (DIFF_OPT_TST(o, SUMMARIZE_SUBMODULES) &&
+ (!one->mode || S_ISGITLINK(one->mode)) &&
+ (!two->mode || S_ISGITLINK(two->mode))) {
+ const char *del = diff_get_color_opt(o, DIFF_FILE_OLD);
+ const char *add = diff_get_color_opt(o, DIFF_FILE_NEW);
+ show_submodule_summary(o->file, one ? one->path : two->path,
+ one->sha1, two->sha1,
+ del, add, reset);
+ return;
+ }
+
if (DIFF_OPT_TST(o, ALLOW_TEXTCONV)) {
textconv_one = get_textconv(one);
textconv_two = get_textconv(two);
@@ -2771,6 +2783,8 @@ int diff_opt_parse(struct diff_options *options, const char **av, int ac)
DIFF_OPT_CLR(options, ALLOW_TEXTCONV);
else if (!strcmp(arg, "--ignore-submodules"))
DIFF_OPT_SET(options, IGNORE_SUBMODULES);
+ else if (!strcmp(arg, "--submodule-summary"))
+ DIFF_OPT_SET(options, SUMMARIZE_SUBMODULES);
/* misc options */
else if (!strcmp(arg, "-z"))
diff --git a/diff.h b/diff.h
index a7e7ccb..aa6976f 100644
--- a/diff.h
+++ b/diff.h
@@ -67,6 +67,7 @@ typedef void (*diff_format_fn_t)(struct diff_queue_struct *q,
#define DIFF_OPT_DIRSTAT_BY_FILE (1 << 20)
#define DIFF_OPT_ALLOW_TEXTCONV (1 << 21)
#define DIFF_OPT_DIFF_FROM_CONTENTS (1 << 22)
+#define DIFF_OPT_SUMMARIZE_SUBMODULES (1 << 23)
#define DIFF_OPT_TST(opts, flag) ((opts)->flags & DIFF_OPT_##flag)
#define DIFF_OPT_SET(opts, flag) ((opts)->flags |= DIFF_OPT_##flag)
#define DIFF_OPT_CLR(opts, flag) ((opts)->flags &= ~DIFF_OPT_##flag)
diff --git a/submodule.c b/submodule.c
new file mode 100644
index 0000000..3f2590d
--- /dev/null
+++ b/submodule.c
@@ -0,0 +1,108 @@
+#include "cache.h"
+#include "submodule.h"
+#include "dir.h"
+#include "diff.h"
+#include "commit.h"
+#include "revision.h"
+
+int add_submodule_odb(const char *path)
+{
+ struct strbuf objects_directory = STRBUF_INIT;
+ struct alternate_object_database *alt_odb;
+
+ strbuf_addf(&objects_directory, "%s/.git/objects/", path);
+ if (!is_directory(objects_directory.buf))
+ return -1;
+
+ /* avoid adding it twice */
+ for (alt_odb = alt_odb_list; alt_odb; alt_odb = alt_odb->next)
+ if (alt_odb->name - alt_odb->base == objects_directory.len &&
+ !strncmp(alt_odb->base, objects_directory.buf,
+ objects_directory.len))
+ return 0;
+
+ alt_odb = xmalloc(objects_directory.len + 42 + sizeof(*alt_odb));
+ alt_odb->next = alt_odb_list;
+ strcpy(alt_odb->base, objects_directory.buf);
+ alt_odb->name = alt_odb->base + objects_directory.len;
+ alt_odb->name[2] = '/';
+ alt_odb->name[40] = '\0';
+ alt_odb->name[41] = '\0';
+ alt_odb_list = alt_odb;
+ prepare_alt_odb();
+ return 0;
+}
+
+void show_submodule_summary(FILE *f, const char *path,
+ unsigned char one[20], unsigned char two[20],
+ const char *del, const char *add, const char *reset)
+{
+ struct rev_info rev;
+ struct commit *commit, *left, *right;
+ struct commit_list *merge_bases, *list;
+ const char *message = NULL;
+ struct strbuf sb = STRBUF_INIT;
+ static const char *format = " %m %s";
+ int fast_forward = 0, fast_backward = 0;
+
+ if (add_submodule_odb(path))
+ message = "(not checked out)";
+ else if (is_null_sha1(one))
+ message = "(new submodule)";
+ else if (is_null_sha1(two))
+ message = "(submodule deleted)";
+ else if (!(left = lookup_commit_reference(one)) ||
+ !(right = lookup_commit_reference(two)))
+ message = "(commits not present)";
+
+ if (!message) {
+ init_revisions(&rev, NULL);
+ setup_revisions(0, NULL, &rev, NULL);
+ rev.left_right = 1;
+ left->object.flags |= SYMMETRIC_LEFT;
+ add_pending_object(&rev, &left->object, path);
+ add_pending_object(&rev, &right->object, path);
+ merge_bases = get_merge_bases(left, right, 1);
+ if (merge_bases) {
+ if (merge_bases->item == left)
+ fast_forward = 1;
+ else if (merge_bases->item == right)
+ fast_backward = 1;
+ }
+ for (list = merge_bases; list; list = list->next) {
+ list->item->object.flags |= UNINTERESTING;
+ add_pending_object(&rev, &list->item->object,
+ sha1_to_hex(list->item->object.sha1));
+ }
+ if (prepare_revision_walk(&rev))
+ message = "(revision walker failed)";
+ }
+
+ strbuf_addf(&sb, "Submodule %s %s..", path,
+ find_unique_abbrev(one, DEFAULT_ABBREV));
+ if (!fast_backward && !fast_forward)
+ strbuf_addch(&sb, '.');
+ strbuf_addf(&sb, "%s", find_unique_abbrev(two, DEFAULT_ABBREV));
+ if (message)
+ strbuf_addf(&sb, " %s\n", message);
+ else
+ strbuf_addf(&sb, "%s:\n", fast_backward ? " (rewind)" : "");
+ fwrite(sb.buf, sb.len, 1, f);
+
+ if (!message) {
+ while ((commit = get_revision(&rev))) {
+ strbuf_setlen(&sb, 0);
+ if (del)
+ strbuf_addstr(&sb, commit->object.flags &
+ SYMMETRIC_LEFT ? del : add);
+ format_commit_message(commit, format, &sb,
+ rev.date_mode);
+ if (del)
+ strbuf_addstr(&sb, reset);
+ strbuf_addch(&sb, '\n');
+ fwrite(sb.buf, sb.len, 1, f);
+ }
+ clear_commit_marks(left, ~0);
+ clear_commit_marks(right, ~0);
+ }
+}
diff --git a/submodule.h b/submodule.h
new file mode 100644
index 0000000..4c0269d
--- /dev/null
+++ b/submodule.h
@@ -0,0 +1,8 @@
+#ifndef SUBMODULE_H
+#define SUBMODULE_H
+
+void show_submodule_summary(FILE *f, const char *path,
+ unsigned char one[20], unsigned char two[20],
+ const char *del, const char *add, const char *reset);
+
+#endif
--
1.6.4.313.g3d9e3
^ permalink raw reply related
* Re: [PATCH] Reserve a slot for argv[0] in default_arg.
From: Petter Urkedal @ 2009-10-04 14:51 UTC (permalink / raw)
To: Jeff King; +Cc: git, urkedal
In-Reply-To: <20091004133333.GA13894@sigill.intra.peff.net>
On 2009-10-04, Jeff King wrote:
> Thanks, your fix looks sane. But I am curious about whether we are
> triggering some glibc pickiness that is in your setup, or if we are
> somehow violating the assumption that we only ever look at
> default_arg[1] and beyond.
I had a look at parse-options.c. In parse_options_start, argv is
assigned to ctx->out, which is overwritten from index 0 in
parse_options_end. This will show the problem:
diff --git a/builtin-show-branch.c b/builtin-show-branch.c
index 3510a86..1c587ad 100644
--- a/builtin-show-branch.c
+++ b/builtin-show-branch.c
@@ -691,6 +691,7 @@ int cmd_show_branch(int ac, const char **av, const char *prefix)
showbranch_use_color = git_use_color_default;
/* If nothing is specified, try the default first */
+ printf("default_arg = %p\n", default_arg);
if (ac == 1 && default_num) {
ac = default_num + 1;
av = default_arg - 1; /* ick; we would not address av[0] */
diff --git a/parse-options.c b/parse-options.c
index f559411..267e752 100644
--- a/parse-options.c
+++ b/parse-options.c
@@ -435,6 +435,7 @@ unknown:
int parse_options_end(struct parse_opt_ctx_t *ctx)
{
+ printf("Assigning to %p\n", ctx->out + ctx->cpidx);
memmove(ctx->out + ctx->cpidx, ctx->argv, ctx->argc * sizeof(*ctx->out));
ctx->out[ctx->cpidx + ctx->argc] = NULL;
return ctx->cpidx + ctx->argc;
^ permalink raw reply related
* Re: [PATCH] Reserve a slot for argv[0] in default_arg.
From: Petter Urkedal @ 2009-10-04 14:13 UTC (permalink / raw)
To: Jeff King; +Cc: git, urkedal
In-Reply-To: <20091004133333.GA13894@sigill.intra.peff.net>
[-- Attachment #1: Type: text/plain, Size: 1505 bytes --]
On 2009-10-04, Jeff King wrote:
> On Sat, Oct 03, 2009 at 03:29:31PM +0200, Petter Urkedal wrote:
>
> > Setting "av" to one slot before the allocated "default_arg" array causes
> > glibc abort with "free(): invalid next size (normal)" in some
> > configurations (Gentoo, glibc-2.9_p20081201-r2, gcc-5.3.2 with PIE).
>
> Thanks, your fix looks sane. But I am curious about whether we are
> triggering some glibc pickiness that is in your setup, or if we are
> somehow violating the assumption that we only ever look at
> default_arg[1] and beyond.
I was wondering myself. I tried to switch off optimisation, but that
had no effect. I'm suspecting PIE, but it could be some other
configuration implied by the Gentoo "hardened" use-flag.
> What show-branch command did you issue to hit this? I was hoping to run
> it under valgrind.
I can reproduce it on my machine with
mkdir test-repo; cd test-repo
/path/to/git init
/path/to/git config showbranch.default --topo-order
/path/to/git show-branch
> Also:
>
> > + if (!default_num)
> > + /* One unused position for argv[0]. */
> > + default_arg[default_num++] = NULL;
>
> I don't know if we have a style rule for comments on single line
> conditionals, but I had to read this a few times to make sure it wasn't
> missing braces.
Comment's are treated as whitespace, but I'll adjust it for readability.
Maybe worse: I missed the 8-column indentation. So, here is the patch
again (attached, I hope Git can extract it).
[-- Attachment #2: showbranch-argv0.txt --]
[-- Type: text/plain, Size: 5885 bytes --]
1552 eideticdew /tmp/test-repo$ gdb /home/urkedal/proj-ext/git/git show-branch
GNU gdb 6.8
Copyright (C) 2008 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html>
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law. Type "show copying"
and "show warranty" for details.
This GDB was configured as "x86_64-pc-linux-gnu"...
/tmp/test-repo/show-branch: No such file or directory.
(gdb) run show-branch
[Thread debugging using libthread_db enabled]
*** glibc detected *** /home/urkedal/proj-ext/git/git: corrupted double-linked list: 0x0000000000734340 ***
[New Thread 0x7f937d4c36f0 (LWP 15739)]
======= Backtrace: =========
/lib/libc.so.6[0x7f937c84e1f2]
/lib/libc.so.6[0x7f937c84e4a4]
/lib/libc.so.6[0x7f937c84f7fc]
/lib/libc.so.6(cfree+0x75)[0x7f937c84fb13]
/lib/libc.so.6[0x7f937c8407bf]
/home/urkedal/proj-ext/git/git[0x48c921]
/home/urkedal/proj-ext/git/git[0x48e617]
/home/urkedal/proj-ext/git/git[0x44e1d2]
/home/urkedal/proj-ext/git/git[0x4042a3]
/home/urkedal/proj-ext/git/git[0x40444d]
/lib/libc.so.6(__libc_start_main+0xe6)[0x7f937c7fd56e]
/home/urkedal/proj-ext/git/git[0x403d59]
======= Memory map: ========
00400000-004e8000 r-xp 00000000 fd:04 12700046 /home/urkedal/proj-ext/git/git
006e7000-006e8000 r--p 000e7000 fd:04 12700046 /home/urkedal/proj-ext/git/git
006e8000-006ed000 rw-p 000e8000 fd:04 12700046 /home/urkedal/proj-ext/git/git
006ed000-00755000 rw-p 00000000 00:00 0 [heap]
7f9378000000-7f9378021000 rw-p 00000000 00:00 0
7f9378021000-7f937c000000 ---p 00000000 00:00 0
7f937c3c5000-7f937c3da000 r-xp 00000000 08:01 32689 /lib64/libgcc_s.so.1
7f937c3da000-7f937c5d9000 ---p 00015000 08:01 32689 /lib64/libgcc_s.so.1
7f937c5d9000-7f937c5da000 r--p 00014000 08:01 32689 /lib64/libgcc_s.so.1
7f937c5da000-7f937c5db000 rw-p 00015000 08:01 32689 /lib64/libgcc_s.so.1
7f937c5db000-7f937c5dd000 r-xp 00000000 08:01 33045 /lib64/libdl-2.9.so
7f937c5dd000-7f937c7dd000 ---p 00002000 08:01 33045 /lib64/libdl-2.9.so
7f937c7dd000-7f937c7de000 r--p 00002000 08:01 33045 /lib64/libdl-2.9.so
7f937c7de000-7f937c7df000 rw-p 00003000 08:01 33045 /lib64/libdl-2.9.so
7f937c7df000-7f937c91e000 r-xp 00000000 08:01 32884 /lib64/libc-2.9.so
7f937c91e000-7f937cb1e000 ---p 0013f000 08:01 32884 /lib64/libc-2.9.so
7f937cb1e000-7f937cb22000 r--p 0013f000 08:01 32884 /lib64/libc-2.9.so
7f937cb22000-7f937cb23000 rw-p 00143000 08:01 32884 /lib64/libc-2.9.so
7f937cb23000-7f937cb28000 rw-p 00000000 00:00 0
7f937cb28000-7f937cb3d000 r-xp 00000000 08:01 32902 /lib64/libpthread-2.9.so
7f937cb3d000-7f937cd3d000 ---p 00015000 08:01 32902 /lib64/libpthread-2.9.so
7f937cd3d000-7f937cd3e000 r--p 00015000 08:01 32902 /lib64/libpthread-2.9.so
7f937cd3e000-7f937cd3f000 rw-p 00016000 08:01 32902 /lib64/libpthread-2.9.so
7f937cd3f000-7f937cd43000 rw-p 00000000 00:00 0
7f937cd43000-7f937ce95000 r-xp 00000000 fd:00 1181123 /usr/lib64/libcrypto.so.0.9.8
7f937ce95000-7f937d095000 ---p 00152000 fd:00 1181123 /usr/lib64/libcrypto.so.0.9.8
7f937d095000-7f937d0a3000 r--p 00152000 fd:00 1181123 /usr/lib64/libcrypto.so.0.9.8
7f937d0a3000-7f937d0bb000 rw-p 00160000 fd:00 1181123 /usr/lib64/libcrypto.so.0.9.8
7f937d0bb000-7f937d0bf000 rw-p 00000000 00:00 0
7f937d0bf000-7f937d0d3000 r-xp 00000000 08:01 32655 /lib64/libz.so.1.2.3
7f937d0d3000-7f937d2d2000 ---p 00014000 08:01 32655 /lib64/libz.so.1.2.3
7f937d2d2000-7f937d2d3000 r--p 00013000 08:01 32655 /lib64/libz.so.1.2.3
7f937d2d3000-7f937d2d4000 rw-p 00014000 08:01 32655 /lib64/libz.so.1.2.3
7f937d2d4000-7f937d2f0000 r-xp 00000000 08:01 33042 /lib64/ld-2.9.so
7f937d4c3000-7f937d4c6000 rw-p 00000000 00:00 0
7f937d4ed000-7f937d4ef000 rw-p 00000000 00:00 0
7f937d4ef000-7f937d4f0000 r--p 0001b000 08:01 33042 /lib64/ld-2.9.so
7f937d4f0000-7f937d4f1000 rw-p 0001c000 08:01 33042 /lib64/ld-2.9.so
7fff94a38000-7fff94a4e000 rw-p 00000000 00:00 0 [stack]
7fff94bff000-7fff94c00000 r-xp 00000000 00:00 0 [vdso]
ffffffffff600000-ffffffffff601000 r-xp 00000000 00:00 0 [vsyscall]
Program received signal SIGABRT, Aborted.
[Switching to Thread 0x7f937d4c36f0 (LWP 15739)]
0x00007f937c810536 in raise () from /lib/libc.so.6
(gdb) bt
#0 0x00007f937c810536 in raise () from /lib/libc.so.6
#1 0x00007f937c811723 in abort () from /lib/libc.so.6
#2 0x00007f937c84953f in ?? () from /lib/libc.so.6
#3 0x00007f937c84e1f2 in ?? () from /lib/libc.so.6
#4 0x00007f937c84e4a4 in ?? () from /lib/libc.so.6
#5 0x00007f937c84f7fc in ?? () from /lib/libc.so.6
#6 0x00007f937c84fb13 in free () from /lib/libc.so.6
#7 0x00007f937c8407bf in ?? () from /lib/libc.so.6
#8 0x000000000048c921 in get_packed_refs () at refs.c:234
#9 0x000000000048e617 in do_for_each_ref (base=0x3d7b <Address 0x3d7b out of bounds>, fn=0x3d7b,
trim=6, flags=-1, cb_data=0x7f937c8e71e0) at refs.c:598
#10 0x000000000044e1d2 in cmd_show_branch (ac=<value optimized out>, av=0x734348, prefix=0x0)
at builtin-show-branch.c:478
#11 0x00000000004042a3 in handle_internal_command (argc=1, argv=0x7fff94a4b1e0) at git.c:249
#12 0x000000000040444d in main (argc=1, argv=0x7fff94a4b1e0) at git.c:436
(gdb) q
[-- Attachment #3: 0001-Reserve-a-slot-for-argv-0-in-default_arg.patch --]
[-- Type: text/plain, Size: 1512 bytes --]
From a6bea57b5f9e3ebca38afce7829922ccb8f7d24f Mon Sep 17 00:00:00 2001
From: Petter Urkedal <urkedal@nbi.dk>
Date: Sat, 3 Oct 2009 14:52:41 +0200
Subject: [PATCH] Reserve a slot for argv[0] in default_arg.
Setting "av" to one slot before the allocated "default_arg" array causes
glibc abort with "free(): invalid next size (normal)" in some
configurations (Gentoo, glibc-2.9_p20081201-r2, gcc-5.3.2 with PIE).
---
builtin-show-branch.c | 8 ++++++--
1 files changed, 6 insertions(+), 2 deletions(-)
diff --git a/builtin-show-branch.c b/builtin-show-branch.c
index 3510a86..81c477c 100644
--- a/builtin-show-branch.c
+++ b/builtin-show-branch.c
@@ -568,6 +568,10 @@ static int git_show_branch_config(const char *var, const char *value, void *cb)
if (default_alloc <= default_num + 1) {
default_alloc = default_alloc * 3 / 2 + 20;
default_arg = xrealloc(default_arg, sizeof *default_arg * default_alloc);
+ if (!default_num) {
+ /* One unused position for argv[0]. */
+ default_arg[default_num++] = NULL;
+ }
}
default_arg[default_num++] = xstrdup(value);
default_arg[default_num] = NULL;
@@ -692,8 +696,8 @@ int cmd_show_branch(int ac, const char **av, const char *prefix)
/* If nothing is specified, try the default first */
if (ac == 1 && default_num) {
- ac = default_num + 1;
- av = default_arg - 1; /* ick; we would not address av[0] */
+ ac = default_num;
+ av = default_arg;
}
ac = parse_options(ac, av, prefix, builtin_show_branch_options,
--
1.6.4.4
^ permalink raw reply related
* Re: A bug or a feature (git diff --author + --grep)
From: Pascal Obry @ 2009-10-04 13:44 UTC (permalink / raw)
To: Björn Steinbrink; +Cc: git list
In-Reply-To: <20091004131933.GA32000@atjola.homenet>
Björn,
> Feature. There's --all-match to switch on AND-mode.
Thanks for the quick answer, I missed the --all-macth option.
--
--|------------------------------------------------------
--| Pascal Obry Team-Ada Member
--| 45, rue Gabriel Peri - 78114 Magny Les Hameaux FRANCE
--|------------------------------------------------------
--| http://www.obry.net - http://v2p.fr.eu.org
--| "The best way to travel is by means of imagination"
--|
--| gpg --keyserver keys.gnupg.net --recv-key F949BD3B
^ permalink raw reply
* Re: [PATCH] Add the utterly important 'mispel' command
From: Jeff King @ 2009-10-04 13:41 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: git, spearce, gitster
In-Reply-To: <alpine.DEB.1.00.0910041250420.4985@pacific.mpi-cbg.de>
On Sun, Oct 04, 2009 at 12:52:09PM +0200, Johannes Schindelin wrote:
> Well, the command is called "mispel", not "autocorrect". So I think you
> misunderstood the purpose of the patch.
OK, well, then I can rail against the quality of your commit message. ;)
> Let this be a lesson, kids: don't review GitTogether patches before you
> had a drink.
Heh.
-Peff
^ permalink raw reply
* Re: [PATCH] tests: make all test files executable
From: Jeff King @ 2009-10-04 13:40 UTC (permalink / raw)
To: Mark Rada; +Cc: git
In-Reply-To: <1762B430-2DC0-48F5-8C34-8428F9145A1E@mailservices.uwaterloo.ca>
On Sun, Oct 04, 2009 at 09:18:20AM -0400, Mark Rada wrote:
> >Ah, nevermind. The problem is that your patch was word-wrapped, making
> >the second "diff --git" line bogus. It would have been nice to have it
> >print a warning instead of silently ignoring that bit of the patch.
> >
> I didn't have format=flowed buggering things up this time, so I don't
> quite understand the problem; could you please explain with more
> details?
Sure. The patch is perfect except for one line. What should have been:
diff --git a/t/t9501-gitweb-standalone-http-status.sh b/t/t9501-gitweb-standalone-http-status.sh
was wrapped to:
diff --git a/t/t9501-gitweb-standalone-http-status.sh
b/t/t9501-gitweb-standalone-http-status.sh
I have no idea how you did that, though. :)
It looks like you send with Thunderbird. How do you get the diff content
into the email? Is it possible that it wraps the content after you have
gotten it there?
> When I try to apply the patch from a saved copy of the e-mail, I get
> the following error:
>
> # git am ~/Downloads/\[PATCH\]\ tests_\ make\ all\ test\ files\
> executable.eml
> Patch format detection failed.
> zsh: exit 1 git am
>
> The difference between the patch created by format-patch and the saved
> e-mail is just some e-mail header information. Is that a different error
> than what you were getting? I'm not sure what I'm doing wrong here, help
> would be appreciated.
Yeah, that's totally different than the problem I was having. I save to
an mbox from mutt, which "git am" understands just fine. I'd have to see
what was in your .eml file to know why "git am" couldn't figure it out
(and it might be a good test case, as "git am" has recently learned to
accept more mailbox formats).
-Peff
^ permalink raw reply
* Re: [PATCH] Reserve a slot for argv[0] in default_arg.
From: Jeff King @ 2009-10-04 13:33 UTC (permalink / raw)
To: Petter Urkedal; +Cc: git
In-Reply-To: <1254576571-29274-1-git-send-email-urkedal@nbi.dk>
On Sat, Oct 03, 2009 at 03:29:31PM +0200, Petter Urkedal wrote:
> Setting "av" to one slot before the allocated "default_arg" array causes
> glibc abort with "free(): invalid next size (normal)" in some
> configurations (Gentoo, glibc-2.9_p20081201-r2, gcc-5.3.2 with PIE).
Thanks, your fix looks sane. But I am curious about whether we are
triggering some glibc pickiness that is in your setup, or if we are
somehow violating the assumption that we only ever look at
default_arg[1] and beyond.
What show-branch command did you issue to hit this? I was hoping to run
it under valgrind.
Also:
> + if (!default_num)
> + /* One unused position for argv[0]. */
> + default_arg[default_num++] = NULL;
I don't know if we have a style rule for comments on single line
conditionals, but I had to read this a few times to make sure it wasn't
missing braces.
> - ac = default_num + 1;
> - av = default_arg - 1; /* ick; we would not address av[0] */
> + ac = default_num;
> + av = default_arg;
Any time you can remove a comment with "ick" in it is probably a good
thing. ;)
-Peff
^ permalink raw reply
* Re: Interim maintainer tree
From: Jeff King @ 2009-10-04 13:29 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Shawn O. Pearce, git
In-Reply-To: <7viqevpjr3.fsf@alter.siamese.dyndns.org>
On Sun, Oct 04, 2009 at 02:54:08AM -0700, Junio C Hamano wrote:
> > - silence gcc warning:
> > http://article.gmane.org/gmane.comp.version-control.git/129485
> > The warning is overly cautious, I think, but it is a dubious enough
> > construct that it is probably worth fixing.
>
> I cannot reach gmane now, but if this is about -Wextra, I'd rather not
> touch it before the release. "comparison between signed and unsigned"
> tends to be excessive and IMNSHO it is crazy to use -Wextra and -Werror
> together.
Actually, I mislabeled this. It is about a glibc run-time complaint not
a gcc warning, which is more worrisome. It is triggered by an argv list
where we pass (argv - 1) to something expecting only to index it
starting from '1'. I'm not sure yet if it is glibc being picky or if we
violate that assumption somewhere. I'll post a followup in the thread.
-Peff
^ 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