* Re: [PATCH 0/3] On compresing large index
From: Thomas Rast @ 2012-02-05 21:22 UTC (permalink / raw)
To: Nguyễn Thái Ngọc Duy; +Cc: git, Joshua Redstone
In-Reply-To: <1328430605-4566-1-git-send-email-pclouds@gmail.com>
Nguyễn Thái Ngọc Duy <pclouds@gmail.com> writes:
> $ time ~/w/git/git ls-files | head >/dev/null
> real 0m4.635s
> user 0m4.258s
> sys 0m0.329s
>
> $ time ~/w/git/git update-index level-0-0000/foo
> real 0m4.593s
> user 0m4.264s
> sys 0m0.323s
[...]
> We need to figure out what git uses 4s user time for.
When I worked on the cache-tree stuff, my observation (based on
profiling, so I had actual data :-) was that computing SHA1s absolutely
dominates everything in such operations. It does that when writing the
index to write the trailing checksum, and also when loading it to verify
that the index is valid.
ls-files shouldn't be so slow though. A quick run with callgrind in a
linux-2.6.git tells me it spends about 45% of its time on SHA1s and a
whopping 25% in quote_c_style(). I wonder what's so hard about
quoting...
> This series may be useful on OSes that do not cache heavily. Though
> I'm not sure if there is any out there nowadays.
I think you could make a case that they should not be called "OS" ;-)
--
Thomas Rast
trast@{inf,student}.ethz.ch
^ permalink raw reply
* Re: git-svn: t9155 fails against subversion 1.7.0
From: Robin H. Johnson @ 2012-02-05 21:25 UTC (permalink / raw)
To: Git Mailing List
In-Reply-To: <op.v4pu1zcq0aolir@keputer>
On Thu, Nov 10, 2011 at 07:02:13AM +0100, Frans Klaver wrote:
> I missed $gmane/184644 in my search for this issue.
Did you make any progress in fixing this?
> On Tue, 08 Nov 2011 23:09:30 +0100, Frans Klaver <fransklaver@gmail.com>
> wrote:
>
> > For kicks I decided to run the tests and noticed that on master
> > t9155-git-svn-fetch-deleted-tag fails against svn 1.7.0. We hit an
> > assertion in subversion's dirent_uri.c, stating that we don't provide a
> > canonical url. I haven't tested against other subversion versions. I
> > dare assume that this issue doesn't arise on earlier versions. It
> > probably won't affect a lot of users right now, but it will in the
> > future.
--
Robin Hugh Johnson
Gentoo Linux: Developer, Trustee & Infrastructure Lead
E-Mail : robbat2@gentoo.org
GnuPG FP : 11ACBA4F 4778E3F6 E4EDF38E B27B944E 34884E85
^ permalink raw reply
* Re: Specifying revisions in the future
From: Andreas Schwab @ 2012-02-05 21:37 UTC (permalink / raw)
To: Matthieu Moy; +Cc: jpaugh, git
In-Reply-To: <vpqipjlf309.fsf@bauges.imag.fr>
Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:
> "the successor of <commit>", OTOH, is not well defined, since there can
> be several successors, and one can't order them reliably (you can't
> really know the set of successors, because they can exist in different
> repositories).
Yet it would be nice to have a concise notation for "the nth successor
of <commit> towards <commit>" (using --first-parent ordering when
ambiguous).
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: [bug] blame duplicates trailing ">" in mailmapped emails
From: Junio C Hamano @ 2012-02-05 21:38 UTC (permalink / raw)
To: Jeff King; +Cc: Felipe Contreras, Jonathan Nieder, git, SZEDER Gábor
In-Reply-To: <7v39aphw85.fsf@alter.siamese.dyndns.org>
Junio C Hamano <gitster@pobox.com> writes:
> Jeff King <peff@peff.net> writes:
> ...
>> but it feels like the fix should go into map_user.
>
> Thanks.
>
> The map_user() API takes an email address that is terminated by either NUL
> or '>' to allow the caller to learn the corresponding up-to-date email
> address that is NUL terminated, while indicating with its return value
> that if the caller even needs to replace what it already has. But the
> function does not properly terminate email when it only touched the name
> part. And I think that is the real bug.
And the gist of the patch to fix the bug would look like this two liner.
In the real fix, "p" should be renamed to "end_of_email" or something
descriptive like that.
I only made sure that this fixes the original case of the email address of
Shawn reported by Felipe, but other than that like everything else I send
here with "... would look like this", not tested beyond "it compiles".
But conceptually it looks correct (famous last words ;-).
Felipe, does it pass your test cases?
mailmap.c | 2 ++
1 files changed, 2 insertions(+), 0 deletions(-)
diff --git a/mailmap.c b/mailmap.c
index 8c3196c..ce805fa 100644
--- a/mailmap.c
+++ b/mailmap.c
@@ -236,6 +236,8 @@ int map_user(struct string_list *map,
}
if (maxlen_email && mi->email)
strlcpy(email, mi->email, maxlen_email);
+ else
+ *p = '\0';
if (maxlen_name && mi->name)
strlcpy(name, mi->name, maxlen_name);
debug_mm("map_user: to '%s' <%s>\n", name, mi->email ? mi->email : "");
^ permalink raw reply related
* Re: [PATCH] send-email: add extra safetly in address sanitazion
From: Thomas Rast @ 2012-02-05 21:51 UTC (permalink / raw)
To: Felipe Contreras
Cc: git, Brandon Casey, Uwe Kleine-König, Brian Gernhardt,
Robin H. Johnson, Ævar Arnfjörð
In-Reply-To: <CAMP44s1wqmT4mavsXrEhB-OquOtQrYnnoSoX9G7X4wzFoMD29A@mail.gmail.com>
Felipe Contreras <felipe.contreras@gmail.com> writes:
> 2012/2/5 Thomas Rast <trast@inf.ethz.ch>:
>> Felipe Contreras <felipe.contreras@gmail.com> writes:
>>>
>>> 'Foo Bar <foo@bar.com>' -> 'Foo Bar <foo@bar.com>'
>>> '"Foo Bar" <foo@bar.com>' -> '"Foo Bar" <foo@bar.com>'
>>> 'foo@bar.com' -> 'foo@bar.com'
>>> '<foo@bar.com>' -> 'foo@bar.com'
>>> 'Foo Bar' -> 'Foo Bar'
>>
>> Am I the only one who stared at this for ten seconds, only to then
>> realize that there is no sanitizing whatsoever going on here?
>
> There is: '<foo@bar.com>' -> 'foo@bar.com'
Indeed.
I still feel cheated as a reader though, you showed me four examples of
no change but let me figure that on my own.
>>> 'Foo Bar <foo@bar.com>>' -> 'Foo Bar <foo@bar.com>'
>>> '"Foo Bar" <foo@bar.com>>' -> '"Foo Bar" <foo@bar.com>'
>>> '<foo@bar.com>>' -> 'foo@bar.com'
>>
>> All of these are the same underlying issue. Does your patch fix any
>> other malformed addresses, or just this particular type?
>
> See above.
Ok, I see I am falling into the same communication trap as Jonathan, so
let's be more explicit.
Your commit message first tells me you are going to sanitize something,
but starts out with examples of leaving the string unchanged. Then it
continues with only the '>>' examples.
Today, and being someone who on average reads about half the mail that
comes through here, I know that this relates to the blame -e '>>' bug.
So today, I am wondering from the commit message why you narrowly focus
on that bug. But you don't! It's just that the commit message
insinuates it.
In a year, your reader (and bear in mind that this may very well be
yourself, at least if your memory is as good as mine) will wonder what
was so damn special about that '>>' string that it needs a specific fix
to send-email.
I see that you wrote in another thread:
> I have to write a peer-reviewed essay with an introduction for the
> people that are not familiar with the code in each of the patches
I'm not sure you meant it that literally, but the whole *point* is that
the message is for people who are not familiar with the code. After
all, if I knew that your code did the right thing in the right way, I
would not be bothering with reading the message. Today, I would just
send an Acked-by instead. In a year, I'd scroll down for another
potential culprit for the bug I'm hunting.
What's especially striking me about your proposed messages of late: they
leave me with more open questions than I started with. I tried to show
this above. I'm not sure whether other contributors are better at
answering questions, or just better at not touching any topics that
might raise them.
--
Thomas Rast
trast@{inf,student}.ethz.ch
^ permalink raw reply
* Re: [PATCH] send-email: add extra safetly in address sanitazion
From: Junio C Hamano @ 2012-02-05 21:52 UTC (permalink / raw)
To: Thomas Rast
Cc: Felipe Contreras, git, Brandon Casey, Uwe Kleine-König,
Brian Gernhardt, Robin H. Johnson,
Ævar Arnfjörð Bjarmason
In-Reply-To: <87sjipxe5u.fsf@thomas.inf.ethz.ch>
Thomas Rast <trast@inf.ethz.ch> writes:
> Am I the only one who stared at this for ten seconds, only to then
> realize that there is no sanitizing whatsoever going on here?
>
>> 'Foo Bar <foo@bar.com>>' -> 'Foo Bar <foo@bar.com>'
>> '"Foo Bar" <foo@bar.com>>' -> '"Foo Bar" <foo@bar.com>'
>> '<foo@bar.com>>' -> 'foo@bar.com'
>
> All of these are the same underlying issue. Does your patch fix any
> other malformed addresses, or just this particular type?
Just this particular type, as long as the code handles it correctly, would
be better than nothing.
On the recieving end in mailinfo, I think we also support
gitster@pobox.com (Junio C Hamano)
although it does not seem to be used by many people these days (the only
one I can remember seeing on this list was merlyn), so we may want to be a
bit more consistent between sending and receiving end, though.
^ permalink raw reply
* Re: Specifying revisions in the future
From: Jakub Narebski @ 2012-02-05 21:57 UTC (permalink / raw)
To: Andreas Schwab; +Cc: Matthieu Moy, jpaugh, git
In-Reply-To: <m21uq9x8q2.fsf@igel.home>
Andreas Schwab <schwab@linux-m68k.org> writes:
> Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:
>
> > "the successor of <commit>", OTOH, is not well defined, since there can
> > be several successors, and one can't order them reliably (you can't
> > really know the set of successors, because they can exist in different
> > repositories).
>
> Yet it would be nice to have a concise notation for "the nth successor
> of <commit> towards <commit>" (using --first-parent ordering when
> ambiguous).
First, "the nth successor"... from which refs? Commit objects have
pointers in one direction only, from commit to its ancestors (earlier
commits).
Second, `--first-parent' won't help here. Take for example the
following situation:
---X<---*<---.<---A
\
\--.<---B
X+3 is A or B? Note that pointers point _to_ '*' commit, so there is
not first or second here - no natural ordering like in the case of
commit parents.
--
Jakub Narebski
^ permalink raw reply
* Re: Specifying revisions in the future
From: Junio C Hamano @ 2012-02-05 21:59 UTC (permalink / raw)
To: Andreas Schwab; +Cc: Matthieu Moy, jpaugh, git
In-Reply-To: <m21uq9x8q2.fsf@igel.home>
Andreas Schwab <schwab@linux-m68k.org> writes:
> Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:
>
>> "the successor of <commit>", OTOH, is not well defined, since there can
>> be several successors, and one can't order them reliably (you can't
>> really know the set of successors, because they can exist in different
>> repositories).
>
> Yet it would be nice to have a concise notation for "the nth successor
> of <commit> towards <commit>" (using --first-parent ordering when
> ambiguous).
I thought that 47 different people can build on Linux v3.2 and when you
ask the children of v3.2^0, you would not know which ones to show in what
order, let alone "concise notation to pick one at random among them".
Did you mean --first-child?
^ permalink raw reply
* git-svn and subversion 1.7: testcase failures due to SVN 1.7 changes
From: Robin H. Johnson @ 2012-02-05 22:00 UTC (permalink / raw)
To: Git Mailing List
[-- Attachment #1: Type: text/plain, Size: 4737 bytes --]
Newer versions of SVN hate a whitespace in any of the URLs (repo, branches,
tags), and this sinks all of the git-svn tests. I've tried to trace and fix it,
changing the canonicalize_path and canonicalize_url calls, but this ends up
making Git-svn very unhappy when it converts back to filenames.
I've seen some prior mentions on this list, but nobody seems to have really
solved it.
I've got a testcase partial workaround we avoid that by replaced the space in
the testcase working dir with an underscore.
Example failure:
Initialized empty Git repository in /dev/shm/portage/dev-vcs/git-9999/work/git-9999/t/trash directory.t9155/git_project/.git/
svn: E235000: In file 'subversion/libsvn_subr/dirent_uri.c' line 2291: assertion failed (svn_uri_is_canonical(url, pool))
With this workaround the following tests still fail: t9100 t9118 t9120
Without it, MOST of t91* fails, due to the space tripping up the
svn_uri_is_canonical internal call: t9100 t9101 t9102 t9103 t9104 t9105 t9106
t9107 t9108 t9109 t9110 t9111 t9112 t9114 t9115 t9116 t9117 t9118 t9120 t9121
t9122 t9123 t9124 t9125 t9127 t9128 t9129 t9130 t9131 t9132 t9133 t9134 t9135
t9136 t9137 t9138 t9139 t9140 t9141 t9142 t9143 t9144 t9145 t9146 t9150 t9151
t9152 t9153 t9154 t9155 t9156 t9157 t9158 t9159 t9160 t9161 t9162
git-svn actually needs to be fixed here, but this change is useful for
testing it.
workaround:
sed -i \
-e 's/trash directory/trash_directory/g' \
test-lib.sh t0000-basic.sh Makefile
Failures with the workaround:
t9100-git-svn-basic.sh:
- not ok - 11 executable file becomes a symlink to bar/zzz (file)
===
r5 = a74fcb37aadb2afb532b100932220f7b59108ca4 (refs/remotes/git-svn)
Done committing 1 revisions to SVN
Updating '.git/svn/refs/remotes/git-svn/svn-tree':
svn: E235000: In file 'subversion/libsvn_wc/update_editor.c' line 1582: assertion failed (action == svn_wc_conflict_action_edit || action == svn_wc_conflict_action_delete || action == svn_wc_conflict_action_replace)
./lib-git-svn.sh: line 61: 1922 Aborted svn "$orig_svncmd" --config-dir "$svnconf" "$@"
not ok - 11 executable file becomes a symlink to bar/zzz (file)
===
- not ok - 12 new symlink is added to a file that was also just made executable
- not ok - 13 modify a symlink to become a file
Both of these fail as a side effect of the one above.
===
r6 = 880d9fc9b4ffda5e3851e86b587af36d6c3ddaf8 (refs/remotes/git-svn)
Done committing 1 revisions to SVN
svn: E155004: Working copy '/dev/shm/portage/dev-vcs/git-1.7.9/work/git-1.7.9/t/trash_directory.t9100-git-svn-basic/.git/svn/refs/remotes/git-svn/svn-tree' locked.
svn: E155004: '/dev/shm/portage/dev-vcs/git-1.7.9/work/git-1.7.9/t/trash_directory.t9100-git-svn-basic/.git/svn/refs/remotes/git-svn/svn-tree' is already locked.
svn: run 'svn cleanup' to remove locks (type 'svn help cleanup' for details)
not ok - 12 new symlink is added to a file that was also just made executable
===
All of t9118/t9120 are a direct result of not having it happy with the escaping.
t9118-git-svn-funky-branch-names.sh:
- not ok - 2 test clone with funky branch names
- not ok - 3 test dcommit to funky branch
- not ok - 4 test dcommit to scary branch
- not ok - 5 test dcommit to trailing_dotlock branch
t9120-git-svn-clone-with-percent-escapes.sh
- not ok - 7 test clone --stdlayout with percent escapes
===
r1 = 60417f2d87410b4775fdd2a04e85d8d468e05e04 (refs/remotes/trunk)
Found possible branch point: file:///dev/shm/portage/dev-vcs/git-1.7.9/work/git-1.7.9/t/trash_directory.t9120-git-svn-clone-with-percent-escapes/svnrepo/pr ject/trunk => file:///dev/shm/portage/dev-vcs/git-1.7.9/work/git-1.7.9/t/trash_directory.t9120-git-svn-clone-with-percent-escapes/svnrepo/pr ject/branches/b, 1
Found branch parent: (refs/remotes/b) 60417f2d87410b4775fdd2a04e85d8d468e05e04
Following parent with do_switch
perl: subversion/libsvn_subr/dirent_uri.c:1519: uri_skip_ancestor: Assertion `svn_uri_is_canonical(child_uri, ((void *)0))' failed.
error: git-svn died of signal 6
not ok - 7 test clone --stdlayout with percent escapes
===
- not ok - 8 test clone -s with unescaped space
===
Initialized empty Git repository in /dev/shm/portage/dev-vcs/git-1.7.9/work/git-1.7.9/t/trash_directory.t9120-git-svn-clone-with-percent-escapes/space/.git/
svn: E235000: In file 'subversion/libsvn_subr/dirent_uri.c' line 2291: assertion failed (svn_uri_is_canonical(url, pool))
error: git-svn died of signal 6
not ok - 8 test clone -s with unescaped space
===
--
Robin Hugh Johnson
Gentoo Linux: Developer, Trustee & Infrastructure Lead
E-Mail : robbat2@gentoo.org
GnuPG FP : 11ACBA4F 4778E3F6 E4EDF38E B27B944E 34884E85
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 330 bytes --]
^ permalink raw reply
* Re: [PATCH] Explicitly set X to avoid potential build breakage
From: Junio C Hamano @ 2012-02-05 22:14 UTC (permalink / raw)
To: Michael; +Cc: git
In-Reply-To: <9854382.GTBzd7D6AT@telegraph>
Michael <kensington@astralcloak.net> writes:
> $X is appended to binary names for Windows builds (ie. git.exe).
> Pollution from the environment can inadvertently trigger this behaviour,
> resulting in 'git' turning into 'gitwhatever' without warning.
>
> Signed-off-by: Michael <kensington@astralcloak.net>
Unless there is a compelling reason not to, could we have your name here,
not just half a name, please?
It is not particularly a good reason to say "I go by 'Michael' among my
friends". The output from "git shortlog -s --author=Michael" shows more
than 20 Michaels already, and it would be nice for us if we can easily
differenciate you among this group of people you are now joining with this
patch.
> ---
> Makefile | 3 +++
> 1 files changed, 3 insertions(+), 0 deletions(-)
>
> diff --git a/Makefile b/Makefile
> index c457c34..380d96f 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -388,6 +388,9 @@ SCRIPT_SH =
> SCRIPT_LIB =
> TEST_PROGRAMS_NEED_X =
>
> +# Binary suffix used for Windows builds
> +X =
> +
The patch looks good to me, although it might make sense to move it down
before this line
PROGRAMS += $(patsubst %.o,git-%$X,$(PROGRAM_OBJS))
where it matters most as the documentation of what $X is being used for.
^ permalink raw reply
* Re: Specifying revisions in the future
From: Andreas Schwab @ 2012-02-05 22:15 UTC (permalink / raw)
To: Jakub Narebski; +Cc: Matthieu Moy, jpaugh, git
In-Reply-To: <m3ehu9kknw.fsf@localhost.localdomain>
Jakub Narebski <jnareb@gmail.com> writes:
> Andreas Schwab <schwab@linux-m68k.org> writes:
>> Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:
>>
>> > "the successor of <commit>", OTOH, is not well defined, since there can
>> > be several successors, and one can't order them reliably (you can't
>> > really know the set of successors, because they can exist in different
>> > repositories).
>>
>> Yet it would be nice to have a concise notation for "the nth successor
>> of <commit> towards <commit>" (using --first-parent ordering when
>> ambiguous).
>
> First, "the nth successor"... from which refs?
>From the first given commit towards the other given commit (the latter
defaulting to HEAD).
> Second, `--first-parent' won't help here. Take for example the
> following situation:
>
> ---X<---*<---.<---A
> \
> \--.<---B
>
> X+3 is A or B?
If "towards A" then it is A, if "towards B", it is B. In other words,
to get the "nth successor of C1 towards C2" take the leftmost possible
parent when walking from C2 to C1, then walk back n commits along this
path. This way you should have an unambigous definition.
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: Specifying revisions in the future
From: Jakub Narebski @ 2012-02-05 22:24 UTC (permalink / raw)
To: Andreas Schwab; +Cc: Matthieu Moy, jpaugh, git
In-Reply-To: <m2wr81vsdv.fsf@igel.home>
On Sun, 5 Feb 2012, Andreas Schwab wrote:
> Jakub Narebski <jnareb@gmail.com> writes:
>> Andreas Schwab <schwab@linux-m68k.org> writes:
>>> Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:
>>>
>>>> "the successor of <commit>", OTOH, is not well defined, since there can
>>>> be several successors, and one can't order them reliably (you can't
>>>> really know the set of successors, because they can exist in different
>>>> repositories).
>>>
>>> Yet it would be nice to have a concise notation for "the nth successor
>>> of <commit> towards <commit>" (using --first-parent ordering when
>>> ambiguous).
>>
>> First, "the nth successor"... from which refs?
>
> From the first given commit towards the other given commit (the latter
> defaulting to HEAD).
That helps some, but not all situations, see below.
> > Second, `--first-parent' won't help here. Take for example the
> > following situation:
> >
> > ---X<---*<---.<---A
> > \
> > \--.<---B
> >
> > X+3 is A or B?
>
> If "towards A" then it is A, if "towards B", it is B. In other words,
> to get the "nth successor of C1 towards C2" take the leftmost possible
> parent when walking from C2 to C1, then walk back n commits along this
> path. This way you should have an unambigous definition.
Nope, still ambiguous:
---X<---*<---.<---A<---.<---M<---
\ /
\--.<---B<------/
Is X+3 A or B? Though '--first-parent + towards N' is I think unambiguous.
--
Jakub Narebski
Poland
^ permalink raw reply
* [RFC/PATCH] tag: add --points-at list option
From: Tom Grennan @ 2012-02-05 22:28 UTC (permalink / raw)
To: git; +Cc: gitster, krh, jasampler
This filters the list for annotated|signed tags of the given object.
Example,
john$ git tag -s v1.0-john v1.0
john$ git tag -l --points-at v1.0
v1.0-john
Signed-off-by: Tom Grennan <tmgrennan@gmail.com>
---
Documentation/git-tag.txt | 5 +++-
builtin/tag.c | 59 ++++++++++++++++++++++++++++++++++++++-------
2 files changed, 54 insertions(+), 10 deletions(-)
diff --git a/Documentation/git-tag.txt b/Documentation/git-tag.txt
index 53ff5f6..b9ec75c 100644
--- a/Documentation/git-tag.txt
+++ b/Documentation/git-tag.txt
@@ -12,7 +12,7 @@ SYNOPSIS
'git tag' [-a | -s | -u <key-id>] [-f] [-m <msg> | -F <file>]
<tagname> [<commit> | <object>]
'git tag' -d <tagname>...
-'git tag' [-n[<num>]] -l [--contains <commit>] [<pattern>...]
+'git tag' [-n[<num>]] -l [--contains <commit>] [--points-at <object>] [<pattern>...]
'git tag' -v <tagname>...
DESCRIPTION
@@ -86,6 +86,9 @@ OPTIONS
--contains <commit>::
Only list tags which contain the specified commit.
+--points-at <object>::
+ Only list annotated or signed tags of the given object.
+
-m <msg>::
--message=<msg>::
Use the given tag message (instead of prompting).
diff --git a/builtin/tag.c b/builtin/tag.c
index 31f02e8..7568d6c 100644
--- a/builtin/tag.c
+++ b/builtin/tag.c
@@ -19,7 +19,8 @@
static const char * const git_tag_usage[] = {
"git tag [-a|-s|-u <key-id>] [-f] [-m <msg>|-F <file>] <tagname> [<head>]",
"git tag -d <tagname>...",
- "git tag -l [-n[<num>]] [<pattern>...]",
+ "git tag -l [-n[<num>]] [<pattern>...] \\\n\t\t"
+ "[--contains <commit>] [--points-at <object>]",
"git tag -v <tagname>...",
NULL
};
@@ -28,6 +29,7 @@ struct tag_filter {
const char **patterns;
int lines;
struct commit_list *with_commit;
+ const unsigned char *points_at;
};
static int match_pattern(const char **patterns, const char *ref)
@@ -105,16 +107,28 @@ static int show_reference(const char *refname, const unsigned char *sha1,
return 0;
}
+ buf = read_sha1_file(sha1, &type, &size);
+ if (!buf || !size)
+ return 0;
+
+ if (filter->points_at) {
+ unsigned char tagged_sha1[20];
+ if (memcmp("object ", buf, 7) \
+ || buf[47] != '\n' \
+ || get_sha1_hex(buf + 7, tagged_sha1) \
+ || memcmp(filter->points_at, tagged_sha1, 20)) {
+ free(buf);
+ return 0;
+ }
+ }
+
if (!filter->lines) {
printf("%s\n", refname);
+ free(buf);
return 0;
}
printf("%-15s ", refname);
- buf = read_sha1_file(sha1, &type, &size);
- if (!buf || !size)
- return 0;
-
/* skip header */
sp = strstr(buf, "\n\n");
if (!sp) {
@@ -143,16 +157,20 @@ static int show_reference(const char *refname, const unsigned char *sha1,
}
static int list_tags(const char **patterns, int lines,
- struct commit_list *with_commit)
+ struct commit_list *with_commit,
+ unsigned char *points_at)
{
struct tag_filter filter;
filter.patterns = patterns;
filter.lines = lines;
filter.with_commit = with_commit;
+ filter.points_at = points_at;
for_each_tag_ref(show_reference, (void *) &filter);
+ if (points_at)
+ free(points_at);
return 0;
}
@@ -375,12 +393,28 @@ static int strbuf_check_tag_ref(struct strbuf *sb, const char *name)
return check_refname_format(sb->buf, 0);
}
+int parse_opt_points_at(const struct option *opt, const char *arg, int unset)
+{
+ unsigned char *sha1;
+
+ if (!arg)
+ return -1;
+ sha1 = xmalloc(20);
+ if (get_sha1(arg, sha1)) {
+ free(sha1);
+ return error("malformed object name %s", arg);
+ }
+ *(unsigned char **)opt->value = sha1;
+ return 0;
+}
+
int cmd_tag(int argc, const char **argv, const char *prefix)
{
struct strbuf buf = STRBUF_INIT;
struct strbuf ref = STRBUF_INIT;
unsigned char object[20], prev[20];
const char *object_ref, *tag;
+ unsigned char *points_at;
struct ref_lock *lock;
struct create_tag_options opt;
char *cleanup_arg = NULL;
@@ -417,6 +451,12 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
PARSE_OPT_LASTARG_DEFAULT,
parse_opt_with_commit, (intptr_t)"HEAD",
},
+ {
+ OPTION_CALLBACK, 0, "points-at", &points_at, "object",
+ "print only annotated|signed tags of the object",
+ PARSE_OPT_LASTARG_DEFAULT,
+ parse_opt_points_at, (intptr_t)"HEAD",
+ },
OPT_END()
};
@@ -443,11 +483,12 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
usage_with_options(git_tag_usage, options);
if (list)
return list_tags(argv, lines == -1 ? 0 : lines,
- with_commit);
+ with_commit, points_at);
if (lines != -1)
die(_("-n option is only allowed with -l."));
- if (with_commit)
- die(_("--contains option is only allowed with -l."));
+ if (with_commit || points_at)
+ die(_("--contains and --points-at options "
+ "are only allowed with -l."));
if (delete)
return for_each_tag_name(argv, delete_tag);
if (verify)
--
1.7.8
^ permalink raw reply related
* (unknown),
From: Ben Walton @ 2012-02-05 22:32 UTC (permalink / raw)
To: git, gitster
In-Reply-To: <7vr4y9ggij.fsf@alter.siamese.dyndns.org>
Hi Junio,
> > diff --git a/compat/inet_ntop.c b/compat/inet_ntop.c
> > index 60b5a1d..f1bf81c 100644
> > --- a/compat/inet_ntop.c
> > +++ b/compat/inet_ntop.c
> > @@ -15,11 +15,9 @@
> > * SOFTWARE.
> > */
> >
> > +#include "../git-compat-util.h"
> > #include <errno.h>
> > #include <sys/types.h>
> > -
> > -#include "../git-compat-util.h"
> > -
> > #include <stdio.h>
> > #include <string.h>
>
> I actually have to wonder if any of these four inclusion of the
> system headers are warranted. Wouldn't they be included as part of
> git-compat-util.h anyway?
Yes, you're correct. Dropping the system includes is correct as
opposed to rearranging the order. Updated patch to follow.
Thanks
-Ben
^ permalink raw reply
* [PATCH] Drop system includes from inet_pton/inet_ntop compatibility wrappers
From: Ben Walton @ 2012-02-05 22:32 UTC (permalink / raw)
To: git, gitster; +Cc: Ben Walton
In-Reply-To: <1328481141-21037-1-git-send-email-bwalton@artsci.utoronto.ca>
As both of these compatibility wrappers include git-compat-utils.h,
all of the system includes were redundant.
Dropping these system includes also makes git-compat-utils.h the first
include which avoids a compiler warning on Solaris due to the
redefinition of _FILE_OFFSET_BITS.
Signed-off-by: Ben Walton <bwalton@artsci.utoronto.ca>
---
compat/inet_ntop.c | 6 ------
compat/inet_pton.c | 6 ------
2 files changed, 0 insertions(+), 12 deletions(-)
diff --git a/compat/inet_ntop.c b/compat/inet_ntop.c
index 60b5a1d..90b7cc4 100644
--- a/compat/inet_ntop.c
+++ b/compat/inet_ntop.c
@@ -15,14 +15,8 @@
* SOFTWARE.
*/
-#include <errno.h>
-#include <sys/types.h>
-
#include "../git-compat-util.h"
-#include <stdio.h>
-#include <string.h>
-
#ifndef NS_INADDRSZ
#define NS_INADDRSZ 4
#endif
diff --git a/compat/inet_pton.c b/compat/inet_pton.c
index 2ec995e..2b9a0a4 100644
--- a/compat/inet_pton.c
+++ b/compat/inet_pton.c
@@ -15,14 +15,8 @@
* WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
*/
-#include <errno.h>
-#include <sys/types.h>
-
#include "../git-compat-util.h"
-#include <stdio.h>
-#include <string.h>
-
#ifndef NS_INT16SZ
#define NS_INT16SZ 2
#endif
--
1.7.5.4
^ permalink raw reply related
* Re: [RFD] Rewriting safety - warn before/when rewriting published history
From: Johan Herland @ 2012-02-05 22:49 UTC (permalink / raw)
To: Jakub Narebski; +Cc: Philip Oakley, git
In-Reply-To: <201202052146.56458.jnareb@gmail.com>
On Sun, Feb 5, 2012 at 21:46, Jakub Narebski <jnareb@gmail.com> wrote:
> On Sun, 5 Feb 2012, Johan Herland wrote:
>> 2012/2/5 Jakub Narebski <jnareb@gmail.com>:
>> > > Being able to mark temporary, out of sequence or other hacks as Secret could
>> > > be useful, as would recording where Public commits had been sent.
>> >
>> > Marking as 'secret' must I think be explicit, but I think 'public' phase
>> > should be inferred from remote-tracking branches. The idea of phases is
>> > to allow UI to ask about status of commits: can we amend / rebase it or
>> > not, can we push it or not.
>>
>> I agree that the 'public' state should (by default) be automatically
>> inferred from remote-tracking branches. As it stands, we can do this
>> with current git, by writing a pre-rebase hook that checks if any of
>> the commits to-be-rebased are reachable from any remote-tracking
>> branch.
>
> It is nice that we can achieve a large part of this feature with existing
> infrastructure. It would be nice if we ship such pre-rebase hook with
> git, so people can just enable it if they want to use this functionality,
> like the default pre-commit hook that checks for whitespace errors.
Yeah. As it is, the pre-rebase hook shipped with v1.7.9 (when
activated) does something similar (i.e. prevent rewriting 'public'
commits). However, it's highly workflow-specific, since it determines
whether the branch being rebased has been merged into "next" or
"master". IMHO, a hook that tested for reachability from
remote-tracking refs would be more generally useful. Obviously, the
two can be combined, and even further combinations may be desirable
(e.g. also checking for reachability from commits annotated in
refs/notes/public).
>> Unfortunately, the pre-rebase hook only affects 'git rebase', and in
>> order to do the same check on 'git commit --amend' you'd have to write
>> a similar pre-commit hook (don't know how easy it is to find the
>> amended commit from within the hook). Maybe we should add a
>> pre-rewrite hook that trigger in the same situations as the
>> post-rewrite hook.
>
> pre-rewrite hook would be a really nice to have, especially that it would
> (I hope) cover third party tools like various GUIs for git; and also
> git-filter-branch.
>
> Note however that the safety net, i.e. refusing or warning against attempted
> rewrite of published history is only part of issue. Another important part
> is querying and showing "phase" of a commit. What I'd like to see is
> ability to show among others in "git log" and "git show" output if commit
> was already published or not (and if it is marked 'secret').
Today, you can use --decorate to display remote-tracking refs in the
log/show output. However, only the tip commits are decorated, so if
the commits shown are not at the tip, you're out of luck. I believe
teaching log/show to decorate _all_ commits that are reachable from
some given ref(s) should be fairly straightforward.
If you use 'git notes' to annotate 'public' and 'secret' states, then
you can also use the --show-notes=<ref> option to let show/log display
the annotations on 'public'/'secret' commits.
>> This should take care of the simplest 'public' use case in a
>> push-based workflow. If you publish commits by other means
>> (send-email, bundles, pulling directly from your repo), you need some
>> other way to mark the 'public' commits. One solution would be using
>> 'git notes' to annotate the 'public' commits on a 'refs/notes/public'
>> notes ref. Your pre-rebase/pre-rewrite hook should then check if any
>> of the commits to-be-rewritten are reachable from any commit annotated
>> as 'public'.
>
> Another solution would be to create "fake" remote-tracking branches
> by git-bundle and git-send-email.
Good point. Creating such "fake" remote-tracking branches might be a
good idea in those workflows anyway, simply to keep track of what has
been shared, and where.
>> Also, if you want to record where 'public' commits have been sent
>> (other than what can be inferred from the remote-tracking branches),
>> you could write this into the refs/notes/public annotation.
>
> I wonder if this too can be done by hook...
You're looking for someting like a post-push hook that runs on the
_client_ after a successful push. AFAIK, that doesn't exist yet. (Not
to be confused with the receive/update hooks that run on the
_server_.)
>> As for 'secret' commits, you could annotate these on a
>> refs/notes/secret notes ref, and then teach 'git push' (or whatever
>> other method for publishing commits you use) to refuse to publish
>> commits annotated on this notes ref. Possibly we would want to add a
>> "pre-push" or "pre-publish" hook.
>
> Well, addition of pre-push / pre-publish was resisted on the grounds
> that all it does is something that can be as easy done by hand before
> push. Perhaps this new use case would help bring it forward, don't
> you think?
Maybe. I didn't follow the original discussion. From my POV, you could
argue that instead of another hook, you could always write a script
that does the 'secret' check before invoking 'git push', and then
you'd use that script instead of 'git push'. But you could argue the
same point for pretty much all of the other existing hooks (e.g.
instead of a pre-commit hook you could have your own commit wrapper
script). So I don't think that's a sufficient argument to refuse the
existence of a pre-push/publish hook.
Have fun! :)
...Johan
--
Johan Herland, <johan@herland.net>
www.herland.net
^ permalink raw reply
* Re: Specifying revisions in the future
From: Philip Oakley @ 2012-02-05 22:58 UTC (permalink / raw)
To: Jakub Narebski, Andreas Schwab; +Cc: Matthieu Moy, jpaugh, git
In-Reply-To: <201202052324.59941.jnareb@gmail.com>
From: "Jakub Narebski" <jnareb@gmail.com>
To: "Andreas Schwab" <schwab@linux-m68k.org>
Cc: "Matthieu Moy" <Matthieu.Moy@grenoble-inp.fr>; <jpaugh@gmx.us>;
<git@vger.kernel.org>
Sent: Sunday, February 05, 2012 10:24 PM
> On Sun, 5 Feb 2012, Andreas Schwab wrote:
>> Jakub Narebski <jnareb@gmail.com> writes:
>>> Andreas Schwab <schwab@linux-m68k.org> writes:
>>>> Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:
>>>>
>>>>> "the successor of <commit>", OTOH, is not well defined, since there
>>>>> can
>>>>> be several successors, and one can't order them reliably (you can't
>>>>> really know the set of successors, because they can exist in different
>>>>> repositories).
>>>>
>>>> Yet it would be nice to have a concise notation for "the nth successor
>>>> of <commit> towards <commit>" (using --first-parent ordering when
>>>> ambiguous).
>>>
>>> First, "the nth successor"... from which refs?
>>
>> From the first given commit towards the other given commit (the latter
>> defaulting to HEAD).
>
> That helps some, but not all situations, see below.
>
>> > Second, `--first-parent' won't help here. Take for example the
>> > following situation:
>> >
>> > ---X<---*<---.<---A
>> > \
>> > \--.<---B
>> >
>> > X+3 is A or B?
>>
>> If "towards A" then it is A, if "towards B", it is B. In other words,
>> to get the "nth successor of C1 towards C2" take the leftmost possible
>> parent when walking from C2 to C1, then walk back n commits along this
>> path. This way you should have an unambigous definition.
>
> Nope, still ambiguous:
>
>
>
> ---X<---*<---.<---A<---.<---M<---
> \ /
> \--.<---B<------/
>
> Is X+3 A or B? Though '--first-parent + towards N' is I think
> unambiguous.
> --
Is there also a rule missing for X+2, viewed from D, in this example
X<---Y<---Z<---
\ \
A<----B<----C<----D
as to which order the first parent rule should _not_ be applied when D's
first parent chain doesn't reach X (it reaches A).
Using 'oldest' first for alternate parent testing would make X+2 = B, whilst
'newest' first would make X+2=Z. I have used the chain order for
'newest/oldest', rather than commit date.
(I'm sure that there already exists a natural rule in the dag walk order).
Philip
^ permalink raw reply
* Re: Specifying revisions in the future
From: Andreas Schwab @ 2012-02-05 22:58 UTC (permalink / raw)
To: Jakub Narebski; +Cc: Matthieu Moy, jpaugh, git
In-Reply-To: <201202052324.59941.jnareb@gmail.com>
Jakub Narebski <jnareb@gmail.com> writes:
> Nope, still ambiguous:
>
>
>
> ---X<---*<---.<---A<---.<---M<---
> \ /
> \--.<---B<------/
>
> Is X+3 A or B? Though '--first-parent + towards N' is I think unambiguous.
M
X+3->M is A, if A is the leftmost ancestor of M.
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: Specifying revisions in the future
From: Andreas Schwab @ 2012-02-05 23:08 UTC (permalink / raw)
To: Philip Oakley; +Cc: Jakub Narebski, Matthieu Moy, jpaugh, git
In-Reply-To: <178AA8FDB02246D9AA9416C0D54E51A8@PhilipOakley>
"Philip Oakley" <philipoakley@iee.org> writes:
> Is there also a rule missing for X+2, viewed from D, in this example
>
>X<---Y<---Z<---
> \ \
>A<----B<----C<----D
This is difficult to interpret since it has some extra indent, let's
assume that Z is the second parent of D and Y the second parent of B.
> as to which order the first parent rule should _not_ be applied when D's
> first parent chain doesn't reach X (it reaches A).
> Using 'oldest' first for alternate parent testing would make X+2 = B,
> whilst 'newest' first would make X+2=Z. I have used the chain order for
> newest/oldest', rather than commit date.
The rule should be to follow the leftmost parent as far as possible.
That means that X+2->D is B.
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: [RFC/PATCH] tag: add --points-at list option
From: Junio C Hamano @ 2012-02-05 23:31 UTC (permalink / raw)
To: Tom Grennan; +Cc: git, krh, jasampler
In-Reply-To: <1328480887-27463-1-git-send-email-tmgrennan@gmail.com>
Tom Grennan <tmgrennan@gmail.com> writes:
> @@ -105,16 +107,28 @@ static int show_reference(const char *refname, const unsigned char *sha1,
> return 0;
> }
>
> + buf = read_sha1_file(sha1, &type, &size);
> + if (!buf || !size)
> + return 0;
> +
> + if (filter->points_at) {
> + unsigned char tagged_sha1[20];
> + if (memcmp("object ", buf, 7) \
> + || buf[47] != '\n' \
> + || get_sha1_hex(buf + 7, tagged_sha1) \
> + || memcmp(filter->points_at, tagged_sha1, 20)) {
Do we need these backslashes at the end of these lines?
> @@ -143,16 +157,20 @@ static int show_reference(const char *refname, const unsigned char *sha1,
> }
>
> static int list_tags(const char **patterns, int lines,
> - struct commit_list *with_commit)
> + struct commit_list *with_commit,
> + unsigned char *points_at)
> {
It strikes me somewhat odd that you can give a list of commits to filter
when using "--contains" (e.g. "--contains v1.7.9 --contains 1.7.8.4"), but
you can only ask for a single object with "--points-at" from the UI point
of view.
> @@ -375,12 +393,28 @@ static int strbuf_check_tag_ref(struct strbuf *sb, const char *name)
> return check_refname_format(sb->buf, 0);
> }
>
> +int parse_opt_points_at(const struct option *opt, const char *arg, int unset)
> +{
> + unsigned char *sha1;
> +
> + if (!arg)
> + return -1;
> + sha1 = xmalloc(20);
> + if (get_sha1(arg, sha1)) {
> + free(sha1);
> + return error("malformed object name %s", arg);
> + }
> + *(unsigned char **)opt->value = sha1;
> + return 0;
> +}
We are ignoring earlier --points-at argument without telling the user that
we do not support more than one.
Would it become too much unnecessary addition of new code if you supported
multiple --points-at on the command line for the sake of consistency?
> @@ -417,6 +451,12 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
> PARSE_OPT_LASTARG_DEFAULT,
> parse_opt_with_commit, (intptr_t)"HEAD",
> },
> + {
> + OPTION_CALLBACK, 0, "points-at", &points_at, "object",
> + "print only annotated|signed tags of the object",
> + PARSE_OPT_LASTARG_DEFAULT,
> + parse_opt_points_at, (intptr_t)"HEAD",
> + },
I wonder if defaulting to HEAD even makes sense for --points-at. When you
are chasing a bug and checked out an old version that originally had
problem, "git tag --contains" that defaults to HEAD does have a value. It
tells us what releases are potentially contaminated with the buggy commit.
But does a similar use case support points-at that defaults to HEAD?
Other than that, thanks for a pleasant read.
^ permalink raw reply
* Re: [bug] blame duplicates trailing ">" in mailmapped emails
From: Jeff King @ 2012-02-05 23:47 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Felipe Contreras, Jonathan Nieder, git, SZEDER Gábor
In-Reply-To: <7vipjlezas.fsf@alter.siamese.dyndns.org>
On Sun, Feb 05, 2012 at 01:38:19PM -0800, Junio C Hamano wrote:
> > The map_user() API takes an email address that is terminated by either NUL
> > or '>' to allow the caller to learn the corresponding up-to-date email
> > address that is NUL terminated, while indicating with its return value
> > that if the caller even needs to replace what it already has. But the
> > function does not properly terminate email when it only touched the name
> > part. And I think that is the real bug.
>
> And the gist of the patch to fix the bug would look like this two liner.
> In the real fix, "p" should be renamed to "end_of_email" or something
> descriptive like that.
Exactly; this is much better.
We could also go as far as saying that map_user would _always_ terminate
in this way (i.e., the caller gets a munged result, whether we found
anything or not). Then internally, map_user could be simplified to stop
worrying about making a temporary copy in mailbuf. And callers could
simply call map_user without worrying about branching on whether it
found anything or not.
But maybe it is not worth that level of refactoring. From my reading,
your patch fixes the problem just fine.
-Peff
^ permalink raw reply
* Re: [bug] blame duplicates trailing ">" in mailmapped emails
From: Jeff King @ 2012-02-05 23:50 UTC (permalink / raw)
To: Felipe Contreras; +Cc: Junio C Hamano, Jonathan Nieder, git, SZEDER Gábor
In-Reply-To: <CAMP44s0xmvjxsE6AYrA5qTZuDfWq8vPDPOo69hiRS+xWbsbS7g@mail.gmail.com>
On Sun, Feb 05, 2012 at 11:11:20PM +0200, Felipe Contreras wrote:
> > Because of the calling convention of map_user, the buffer with the input
> > must also be writable (since it holds the result). So there should be no
> > loss of efficiency to convert the ">" into a "\0" (and in fact, the
> > simplest fix is probably to just have map_user "tie off" any ">" it
> > detects).
>
> Yes, but then the caller (git blame) would need to _always_ do that
> conversion before (">" -> "\0"), and after ("\0" -> ">"), as opposed
> to now, that it does the conversion only when map_user succeeds (or
> checks if it has to do it).
Yes, I'm talking about changing the calling and return conventions of
map_user. I think the efficiency change is negligible, though, as we
are talking about character assignments (and in fact, it would probably
end up more efficient, as we could eliminate some copying inside
map_user). But Junio's patch is simple, and fixes the problem without
creating any complexity for the callers. So I think it's a good fix.
-Peff
^ permalink raw reply
* Re: [RFC/PATCH] tag: add --points-at list option
From: Jeff King @ 2012-02-06 0:04 UTC (permalink / raw)
To: Tom Grennan; +Cc: git, gitster, krh, jasampler
In-Reply-To: <1328480887-27463-1-git-send-email-tmgrennan@gmail.com>
On Sun, Feb 05, 2012 at 02:28:07PM -0800, Tom Grennan wrote:
> This filters the list for annotated|signed tags of the given object.
> Example,
>
> john$ git tag -s v1.0-john v1.0
> john$ git tag -l --points-at v1.0
> v1.0-john
I really like this approach. One big question, and a few small comments:
> +--points-at <object>::
> + Only list annotated or signed tags of the given object.
> +
It is unclear to me from this documentation if we will only peel a
single level, or if we will peel indefinitely. E.g., what will this
show:
$ git tag one v1.0
$ git tag two one
$ git tag --points-at=v1.0
It will clearly show "one", but will it also show "two" (from reading
the code, I think the answer is "no")? If not, should it?
> + buf = read_sha1_file(sha1, &type, &size);
> + if (!buf || !size)
> + return 0;
Before your patch, a tag whose sha1 could not be read would get its name
printed, and then we would later return without printing anything more.
Now it won't get even the first bit printed.
However, I'm not sure the old behavior wasn't buggy; it would print part
of the line, but never actually print the newline.
> + if (filter->points_at) {
> + unsigned char tagged_sha1[20];
> + if (memcmp("object ", buf, 7) \
> + || buf[47] != '\n' \
> + || get_sha1_hex(buf + 7, tagged_sha1) \
> + || memcmp(filter->points_at, tagged_sha1, 20)) {
> + free(buf);
> + return 0;
> + }
> + }
Hmm, I would have expected to use parse_tag_buffer instead of doing it
by hand. This is probably a tiny bit more efficient, but I wonder if the
code complexity is worth it.
> static int list_tags(const char **patterns, int lines,
> - struct commit_list *with_commit)
> + struct commit_list *with_commit,
> + unsigned char *points_at)
Like Junio, I was surprised this did not allow a list.
-Peff
^ permalink raw reply
* merge: do not create a signed tag merge under --ff-only option
From: Junio C Hamano @ 2012-02-06 0:22 UTC (permalink / raw)
To: git; +Cc: Bart Trojanowski
In-Reply-To: <7vd39vlbgj.fsf@alter.siamese.dyndns.org>
Starting at release v1.7.9, if you ask to merge a signed tag, "git merge"
always creates a merge commit, even when the tag points at a commit that
happens to be a descendant of your current commit.
Unfortunately, this interacts rather badly for people who use --ff-only to
make sure that their branch is free of local developments. It used to be
possible to say:
$ git checkout -b frotz v1.7.9~30
$ git merge --ff-only v1.7.9
and expect that the resulting tip of frotz branch matches v1.7.9^0 (aka
the commit tagged as v1.7.9), but this fails with the updated Git with:
fatal: Not possible to fast-forward, aborting.
because a merge that merges v1.7.9 tag to v1.7.9~30 cannot be created by
fast forwarding.
We could teach users that now they have to do
$ git merge --ff-only v1.7.9^0
but it is far more pleasant for users if we DWIMmed this ourselves.
When an integrator pulls in a topic from a lieutenant via a signed tag,
even when the work done by the lieutenant happens to fast-forward, the
integrator wants to have a merge record, so the integrator will not be
asking for --ff-only when running "git pull" in such a case. Therefore,
this change should not regress the support for the use case v1.7.9 wanted
to add.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
Junio C Hamano <gitster@pobox.com> writes:
We could solve this in one of two ways. We could tell them to merge
v3.2.3^0 instead. Or we could just go ahead and do that for them
automatically ourselves. I am inclined to say that we should unwrap
the tag given from the command line when --ff-only was given, i.e. we
do the latter.
And it turns out that it is just a single-liner patch.
builtin/merge.c | 3 ++-
t/t7600-merge.sh | 13 +++++++++++++
2 files changed, 15 insertions(+), 1 deletions(-)
diff --git a/builtin/merge.c b/builtin/merge.c
index 3a45172..b4fbc60 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -1283,7 +1283,8 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
sha1_to_hex(commit->object.sha1));
setenv(buf.buf, argv[i], 1);
strbuf_reset(&buf);
- if (merge_remote_util(commit) &&
+ if (!fast_forward_only &&
+ merge_remote_util(commit) &&
merge_remote_util(commit)->obj &&
merge_remote_util(commit)->obj->type == OBJ_TAG) {
option_edit = 1;
diff --git a/t/t7600-merge.sh b/t/t7600-merge.sh
index 5d8c428..a598dfa 100755
--- a/t/t7600-merge.sh
+++ b/t/t7600-merge.sh
@@ -27,6 +27,7 @@ Testing basic merge operations/option parsing.
'
. ./test-lib.sh
+. "$TEST_DIRECTORY"/lib-gpg.sh
printf '%s\n' 1 2 3 4 5 6 7 8 9 >file
printf '%s\n' '1 X' 2 3 4 5 6 7 8 9 >file.1
@@ -670,4 +671,16 @@ test_expect_success 'merge --no-ff --edit' '
test_cmp actual expected
'
+test_expect_success GPG 'merge --ff-only tag' '
+ git reset --hard c0 &&
+ git commit --allow-empty -m "A newer commit" &&
+ git tag -s -m "A newer commit" signed &&
+ git reset --hard c0 &&
+
+ git merge --ff-only signed &&
+ git rev-parse signed^0 >expect &&
+ git rev-parse HEAD >actual &&
+ test_cmp actual expect
+'
+
test_done
^ permalink raw reply related
* Re: [bug] blame duplicates trailing ">" in mailmapped emails
From: Junio C Hamano @ 2012-02-06 0:39 UTC (permalink / raw)
To: Jeff King; +Cc: Felipe Contreras, Jonathan Nieder, git, SZEDER Gábor
In-Reply-To: <20120205234750.GA28735@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> We could also go as far as saying that map_user would _always_ terminate
> in this way (i.e., the caller gets a munged result, whether we found
> anything or not). Then internally, map_user could be simplified to stop
> worrying about making a temporary copy in mailbuf. And callers could
> simply call map_user without worrying about branching on whether it
> found anything or not.
I thought about it, but such a change needs to audit all the call sites
that assumes the promise original map_user() used to make before it was
broken. If we return 0 to the caller, the caller does not have to worry
about map_user() munging the buffer it lent to it.
It might be a worthwhile thing to do. I dunno; I didn't look into it.
^ 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