* [PATCH] usage.c: detect recursion in die routines and bail out immediately
From: Brandon Casey @ 2012-11-15 1:45 UTC (permalink / raw)
To: gitster; +Cc: git, Brandon Casey, Brandon Casey
It is theoretically possible for a die handler to get into a state of
infinite recursion. For example, if a die handler called another function
which itself called die(). Let's at least detect this situation, inform the
user, and call exit.
Signed-off-by: Brandon Casey <bcasey@nvidia.com>
---
usage.c | 15 +++++++++++++++
1 file changed, 15 insertions(+)
diff --git a/usage.c b/usage.c
index a2a6678..8eab281 100644
--- a/usage.c
+++ b/usage.c
@@ -6,6 +6,8 @@
#include "git-compat-util.h"
#include "cache.h"
+static int dying;
+
void vreportf(const char *prefix, const char *err, va_list params)
{
char msg[4096];
@@ -82,6 +84,12 @@ void NORETURN die(const char *err, ...)
{
va_list params;
+ if (dying) {
+ fputs("fatal: recursion detected in die handler\n", stderr);
+ exit(128);
+ }
+ dying = 1;
+
va_start(params, err);
die_routine(err, params);
va_end(params);
@@ -94,6 +102,13 @@ void NORETURN die_errno(const char *fmt, ...)
char str_error[256], *err;
int i, j;
+ if (dying) {
+ fputs("fatal: recursion detected in die_errno handler\n",
+ stderr);
+ exit(128);
+ }
+ dying = 1;
+
err = strerror(errno);
for (i = j = 0; err[i] && j < sizeof(str_error) - 1; ) {
if ((str_error[j++] = err[i++]) != '%')
--
1.8.0
^ permalink raw reply related
* [PATCH 3/5] sequencer.c: handle rfc2822 continuation lines correctly
From: Brandon Casey @ 2012-11-15 1:37 UTC (permalink / raw)
To: git; +Cc: Brandon Casey, Brandon Casey
In-Reply-To: <1352943474-15573-1-git-send-email-drafnel@gmail.com>
ends_rfc2822_footer() was incorrectly checking whether the current line
was a continuation of the previous line. It was actually checking the
next line instead of the current line. Let's fix this and mark the test
as expect_success.
Signed-off-by: Brandon Casey <bcasey@nvidia.com>
---
sequencer.c | 2 +-
t/t3511-cherry-pick-x.sh | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/sequencer.c b/sequencer.c
index be0cb8b..01edec2 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -1040,7 +1040,7 @@ static int ends_rfc2822_footer(struct strbuf *sb, int ignore_footer)
; /* do nothing */
k++;
- if ((buf[k] == ' ' || buf[k] == '\t') && !first)
+ if ((buf[i] == ' ' || buf[i] == '\t') && !first)
continue;
first = 0;
diff --git a/t/t3511-cherry-pick-x.sh b/t/t3511-cherry-pick-x.sh
index b4e5c65..b2098e0 100755
--- a/t/t3511-cherry-pick-x.sh
+++ b/t/t3511-cherry-pick-x.sh
@@ -52,7 +52,7 @@ test_expect_success 'cherry-pick -s inserts blank line after non-rfc2822 footer'
test_cmp expect actual
'
-test_expect_failure 'cherry-pick -s not confused by rfc2822 continuation line' '
+test_expect_success 'cherry-pick -s not confused by rfc2822 continuation line' '
pristine_detach initial &&
git cherry-pick -s rfc2822-base &&
cat <<-EOF >expect &&
--
1.8.0
^ permalink raw reply related
* Re: [PATCH] send-email: add proper default sender
From: Felipe Contreras @ 2012-11-15 1:41 UTC (permalink / raw)
To: Jeff King; +Cc: git, Thomas Rast, Junio C Hamano, Jonathan Nieder
In-Reply-To: <20121115000726.GA16910@sigill.intra.peff.net>
On Thu, Nov 15, 2012 at 1:07 AM, Jeff King <peff@peff.net> wrote:
> On Tue, Nov 13, 2012 at 09:35:18PM +0100, Felipe Contreras wrote:
>
>> > Yes, dying would be a regression, in that you would have to configure
>> > your name via the environment and re-run rather than type it at the
>> > prompt. You raise a good point that for people who _could_ take the
>> > implicit default, hitting "enter" is working fine now, and we would lose
>> > that. I'd be fine with also just continuing to prompt in the implicit
>> > case.
>> >
>> > But that is a much smaller issue to me than having send-email fail to
>> > respect environment variables and silently use user.*, which is what
>> > started this whole discussion. And I agree it is worth considering as a
>> > regression we should avoid.
>>
>> It might be smaller, I don't think so. A hypothetical user that was
>> relying on GIT_AUTHOR for whatever reason can switch to 'git
>> send-email --from' (which is much easier) when they notice the
>> failure, the same way somebody relying on fqdn would. The difference
>> is that people with fqdn do exist, and they might be relying on this.
>>
>> Both are small issues, that I agree with.
>>
>> But the point is that you seem to be very adamant about _my_
>> regressions, and not pay much attention about yours.
>
> Really? I mentioned initially the possibility of dying instead of
> prompting. You raised the point that it would regress a certain use
> case. And then what happened? I said above "you raise a good point[...].
> I'd be fine with also just continuing to prompt[...]. I agree it is
> worth considering as a regression we should avoid". And then I sent out
> a patch series which does not have the regression.
>
> In other words, my suggestion was a bad one, and once it was pointed
> out, I did not pursue it. If you want to call that "not paying much
> attention", feel free. But I'd much rather you point out problems in my
> actual patch series.
But that I meant that when I introduce a regression it's like I'm
killing all that is good and sacred about git, and when you do it's
everything but that.
Yes, you sent a new patch. So did I.
>> The second patch doesn't have this issue. It does change the behavior
>> of 'git commit', yeah, but I think that's a benefit.
>
> Changing "git commit" is even something I would entertain. It would be a
> regression for some people, but at least it buys us something (increased
> safety against people making bogus commits and failing to notice the
> warning). I'm undecided on whether that is worth it or not.
>
> But when you presented it, as far as I could tell the change in behavior
> to "git commit" was accidental (which is why I pointed it out in
> response).
How could it be accidental if I said this: "Not only will this fix
'git send-email', but it will also fix 'git
commit'".
> And as it was in the middle of a discussion about whether
> regressions matter,
That was not the discussion at all.
You can't say that all regressions are the same, and if I say
"regression X doesn't matter", that means ALL regressions don't
matter. That's a hasty generalization.
> If you want to seriously propose changing the behavior of "git commit",
> I think the best thing would be to make a real patch, laying out the
> pros and cons in the commit message, and post it. I would not be
> surprised if the other list participants have stopped reading our thread
> at this point, and the idea is going otherwise unnoticed.
I would, if I saw any chance in it actually going through.
>> Or:
>>
>> 4. Just stop prompting
>>
>> I already sent a patch for 4. with all the details of why nobody (or
>> very few, if any) would be affected negatively.
>
> If doing (2) were really hard, that might be worth considering. But it's
> not. I already did it. So I don't see how this is an attractive option,
> unless my series is so unpalatable that we would rather accept a
> regression.
A matter of opinion. I think that series introduces way too much code
for a very very small gain that eventually would probably disappear.
>> > [1/6]: ident: make user_ident_explicitly_given private
>> > [2/6]: ident: keep separate "explicit" flags for author and committer
>> > [3/6]: var: accept multiple variables on the command line
>> > [4/6]: var: provide explicit/implicit ident information
>> > [5/6]: Git.pm: teach "ident" to query explicitness
>> > [6/6]: send-email: do not prompt for explicit repo ident
>>
>> I think this adds a lot of code that nobody would use.
>
> A lot of code? It is mostly refactoring,
Patch #1 and #3 are refactoring, the rest are not.
> which IMHO makes the resulting
> code cleaner, and it increases the utility of "git var", and our test
> coverage. If you have review comments, then by all means, respond to the
> series.
I don't have any comments, except that I don't think all that code is
needed. And why would I bother commenting there, if my opinion will be
ignored?
>> > I do not necessarily agree on "git commit". Moreover, I feel like it is
>> > a separate issue. My series above _just_ implements the "do not prompt
>> > when explicit" behavior. It does not deal with git-commit at all, nor
>> > does it address the author/committer fallback questions. Those can
>> > easily go on top.
>>
>> Yes, at the cost of adding a lot of code. If we end up agreeing that
>> the changes to 'git commit' are desirable (which I hope at some point
>> we will), then this code would be all for nothing.
>
> If we are going to change "git commit" immediately, then I agree there
> is not much point merging my series. But even if we do change it, will
> we do so immediately? Will there be a deprecation period? If so, then my
> series helps send-email in the meantime. And it's already written, so
> you do not even have to do anything.
Yes, but it still adds a lot of code.
>> I want clarify that this is merely a disagreement to at which level
>> should we worry about regressions. On one side of the spectrum you
>> have projects like GNOME, who don't have any problem breaking the
>> user-experience from one release to the next, I'm not proposing
>> anything like that. On the other side I think it's you, because I
>> don't recall encountering anybody with such an extreme position of
>> never introducing a regression ever if there's absolutely no evidence
>> that anybody is using certain feature.
>
> I don't think that's a fair characterization of my position. I am fine
> with introducing a regression if there is a large benefit to it, and
> especially if the regression is mutually exclusive with the benefit. For
> example, look at IDENT_STRICT. We used to allow broken email addresses
> in commits, and it was _me_ who pushed forward the change to disallow
> it. That potentially regressed people who would rather have junk in the
> commit objects than configure their identity (e.g., because they are
> creating commits on the backend of some automated process). But we
> discussed it, and the breakage was worth the increased safety for normal
> users. We could not have it both ways, since the safety came at the
> expense of switching the default.
>
> But with this topic, we had a too-safe default (a safety prompt that was
> sometimes overkill). We can have our cake and eat it, too: drop the
> prompt for the overkill cases, but leave the other cases untouched. And
> that is what I tried to do in my series. Note that this _still_
> regresses certain use cases. What if I have configured my user.email,
> but I am expecting send-email to prompt me so I can put in some other
> random value. But we can't improve the prompting and leave that case
> there; they are mutually exclusive. But IMHO, the benefit outweighs the
> possibility of breakage.
That's true, so we will be introducing a regression regardless.
Now, if we go with this patch:
http://article.gmane.org/gmane.comp.version-control.git/209660
Instead of your patch series, who will get hurt? Hint: I already
answered that question.
Cheers.
--
Felipe Contreras
^ permalink raw reply
* [PATCH/RFC 5/5] sequencer.c: always separate "(cherry picked from" from commit body
From: Brandon Casey @ 2012-11-15 1:37 UTC (permalink / raw)
To: git; +Cc: Brandon Casey, Brandon Casey
In-Reply-To: <1352943474-15573-1-git-send-email-drafnel@gmail.com>
Start treating the "(cherry picked from" line added by cherry-pick -x
the same way that the s-o-b lines are treated. Namely, separate them
from the main commit message body with an empty line.
Also, introduce tests to test this functionality.
Signed-off-by: Brandon Casey <bcasey@nvidia.com>
---
This seems like the right thing to do, but it's more of a change in
policy than the others, so I marked it as RFC. Any disagreement here?
-Brandon
sequencer.c | 110 ++++++++++++++++++++++++-----------------------
t/t3511-cherry-pick-x.sh | 77 +++++++++++++++++++++++++++++++++
2 files changed, 133 insertions(+), 54 deletions(-)
diff --git a/sequencer.c b/sequencer.c
index 27e684c..0da0538 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -20,6 +20,60 @@
const char sign_off_header[] = "Signed-off-by: ";
const char cherry_picked_prefix[] = "(cherry picked from commit ";
+static int is_rfc2822_line(const char *buf, int len)
+{
+ int i;
+
+ for (i = 0; i < len; i++) {
+ int ch = buf[i];
+ if (ch == ':')
+ break;
+ if (isalnum(ch) || (ch == '-'))
+ continue;
+ return 0;
+ }
+
+ return 1;
+}
+
+static int is_cherry_pick_from_line(const char *buf, int len)
+{
+ return (strlen(cherry_picked_prefix) + 41) <= len &&
+ !prefixcmp(buf, cherry_picked_prefix);
+}
+
+static int ends_rfc2822_footer(struct strbuf *sb, int ignore_footer)
+{
+ int hit = 0;
+ int i, k;
+ int len = sb->len - ignore_footer;
+ int last_was_rfc2822 = 0;
+ const char *buf = sb->buf;
+
+ for (i = len - 1; i > 0; i--) {
+ if (hit && buf[i] == '\n')
+ break;
+ hit = (buf[i] == '\n');
+ }
+
+ while (i < len - 1 && buf[i] == '\n')
+ i++;
+
+ for (; i < len; i = k) {
+ for (k = i; k < len && buf[k] != '\n'; k++)
+ ; /* do nothing */
+ k++;
+
+ if (last_was_rfc2822 && (buf[i] == ' ' || buf[i] == '\t'))
+ continue;
+
+ if (!((last_was_rfc2822 = is_rfc2822_line(buf+i, k-i)) ||
+ is_cherry_pick_from_line(buf+i, k-i)))
+ return 0;
+ }
+ return 1;
+}
+
static void remove_sequencer_state(void)
{
struct strbuf seq_dir = STRBUF_INIT;
@@ -493,6 +547,8 @@ static int do_pick_commit(struct commit *commit, struct replay_opts *opts)
}
if (opts->record_origin) {
+ if (!ends_rfc2822_footer(&msgbuf, 0))
+ strbuf_addch(&msgbuf, '\n');
strbuf_addstr(&msgbuf, cherry_picked_prefix);
strbuf_addstr(&msgbuf, sha1_to_hex(commit->object.sha1));
strbuf_addstr(&msgbuf, ")\n");
@@ -1018,60 +1074,6 @@ int sequencer_pick_revisions(struct replay_opts *opts)
return pick_commits(todo_list, opts);
}
-static int is_rfc2822_line(const char *buf, int len)
-{
- int i;
-
- for (i = 0; i < len; i++) {
- int ch = buf[i];
- if (ch == ':')
- break;
- if (isalnum(ch) || (ch == '-'))
- continue;
- return 0;
- }
-
- return 1;
-}
-
-static int is_cherry_pick_from_line(const char *buf, int len)
-{
- return (strlen(cherry_picked_prefix) + 41) <= len &&
- !prefixcmp(buf, cherry_picked_prefix);
-}
-
-static int ends_rfc2822_footer(struct strbuf *sb, int ignore_footer)
-{
- int hit = 0;
- int i, k;
- int len = sb->len - ignore_footer;
- int last_was_rfc2822 = 0;
- const char *buf = sb->buf;
-
- for (i = len - 1; i > 0; i--) {
- if (hit && buf[i] == '\n')
- break;
- hit = (buf[i] == '\n');
- }
-
- while (i < len - 1 && buf[i] == '\n')
- i++;
-
- for (; i < len; i = k) {
- for (k = i; k < len && buf[k] != '\n'; k++)
- ; /* do nothing */
- k++;
-
- if (last_was_rfc2822 && (buf[i] == ' ' || buf[i] == '\t'))
- continue;
-
- if (!((last_was_rfc2822 = is_rfc2822_line(buf+i, k-i)) ||
- is_cherry_pick_from_line(buf+i, k-i)))
- return 0;
- }
- return 1;
-}
-
void append_signoff(struct strbuf *msgbuf, int ignore_footer)
{
struct strbuf sob = STRBUF_INIT;
diff --git a/t/t3511-cherry-pick-x.sh b/t/t3511-cherry-pick-x.sh
index 785486e..af7a87c 100755
--- a/t/t3511-cherry-pick-x.sh
+++ b/t/t3511-cherry-pick-x.sh
@@ -40,6 +40,19 @@ test_expect_success setup '
test_commit conflicting unrelated
'
+test_expect_success 'cherry-pick -x inserts blank line after non-rfc2822 footer' '
+ pristine_detach initial &&
+ sha1=`git rev-parse non-rfc2822-base^0` &&
+ git cherry-pick -x non-rfc2822-base &&
+ cat <<-EOF >expect &&
+ $non_rfc2822_mesg
+
+ (cherry picked from commit $sha1)
+ EOF
+ git log -1 --pretty=format:%B >actual &&
+ test_cmp expect actual
+'
+
test_expect_success 'cherry-pick -s inserts blank line after non-rfc2822 footer' '
pristine_detach initial &&
git cherry-pick -s non-rfc2822-base &&
@@ -52,6 +65,32 @@ test_expect_success 'cherry-pick -s inserts blank line after non-rfc2822 footer'
test_cmp expect actual
'
+test_expect_success 'cherry-pick -x -s inserts blank line after non-rfc2822 footer' '
+ pristine_detach initial &&
+ sha1=`git rev-parse non-rfc2822-base^0` &&
+ git cherry-pick -x -s non-rfc2822-base &&
+ cat <<-EOF >expect &&
+ $non_rfc2822_mesg
+
+ (cherry picked from commit $sha1)
+ Signed-off-by: $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL>
+ EOF
+ git log -1 --pretty=format:%B >actual &&
+ test_cmp expect actual
+'
+
+test_expect_success 'cherry-pick -x not confused by rfc2822 continuation line' '
+ pristine_detach initial &&
+ sha1=`git rev-parse rfc2822-base^0` &&
+ git cherry-pick -x rfc2822-base &&
+ cat <<-EOF >expect &&
+ $rfc2822_mesg
+ (cherry picked from commit $sha1)
+ EOF
+ git log -1 --pretty=format:%B >actual &&
+ test_cmp expect actual
+'
+
test_expect_success 'cherry-pick -s not confused by rfc2822 continuation line' '
pristine_detach initial &&
git cherry-pick -s rfc2822-base &&
@@ -63,6 +102,31 @@ test_expect_success 'cherry-pick -s not confused by rfc2822 continuation line' '
test_cmp expect actual
'
+test_expect_success 'cherry-pick -x -s not confused by rfc2822 continuation line' '
+ pristine_detach initial &&
+ sha1=`git rev-parse rfc2822-base^0` &&
+ git cherry-pick -x -s rfc2822-base &&
+ cat <<-EOF >expect &&
+ $rfc2822_mesg
+ (cherry picked from commit $sha1)
+ Signed-off-by: $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL>
+ EOF
+ git log -1 --pretty=format:%B >actual &&
+ test_cmp expect actual
+'
+
+test_expect_success 'cherry-pick treats -x "(cherry picked from..." line as part of footer' '
+ pristine_detach initial &&
+ sha1=`git rev-parse rfc2822-cherry-base^0` &&
+ git cherry-pick -x rfc2822-cherry-base &&
+ cat <<-EOF >expect &&
+ $rfc2822_cherry_mesg
+ (cherry picked from commit $sha1)
+ EOF
+ git log -1 --pretty=format:%B >actual &&
+ test_cmp expect actual
+'
+
test_expect_success 'cherry-pick treats -s "(cherry picked from..." line as part of footer' '
pristine_detach initial &&
git cherry-pick -s rfc2822-cherry-base &&
@@ -74,4 +138,17 @@ test_expect_success 'cherry-pick treats -s "(cherry picked from..." line as part
test_cmp expect actual
'
+test_expect_success 'cherry-pick treats -x -s "(cherry picked from..." line as part of footer' '
+ pristine_detach initial &&
+ sha1=`git rev-parse rfc2822-cherry-base^0` &&
+ git cherry-pick -x -s rfc2822-cherry-base &&
+ cat <<-EOF >expect &&
+ $rfc2822_cherry_mesg
+ (cherry picked from commit $sha1)
+ Signed-off-by: $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL>
+ EOF
+ git log -1 --pretty=format:%B >actual &&
+ test_cmp expect actual
+'
+
test_done
--
1.8.0
^ permalink raw reply related
* [PATCH 4/5] sequencer.c: recognize "(cherry picked from ..." as part of s-o-b footer
From: Brandon Casey @ 2012-11-15 1:37 UTC (permalink / raw)
To: git; +Cc: Brandon Casey, Brandon Casey
In-Reply-To: <1352943474-15573-1-git-send-email-drafnel@gmail.com>
Currently, if the s-o-b footer of a commit message contains a
"(cherry picked from ..." line that was added by a previous cherry-pick -x,
it is not recognized as a s-o-b footer and will cause a newline to be
inserted before an additional s-o-b is added.
So, rework ends_rfc2822_footer to recognize the "(cherry picked from ..."
string as part of the footer. Plus mark the test in t3511 as fixed.
Signed-off-by: Brandon Casey <bcasey@nvidia.com>
---
sequencer.c | 44 +++++++++++++++++++++++++++++---------------
t/t3511-cherry-pick-x.sh | 2 +-
2 files changed, 30 insertions(+), 16 deletions(-)
diff --git a/sequencer.c b/sequencer.c
index 01edec2..27e684c 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -18,6 +18,7 @@
#define GIT_REFLOG_ACTION "GIT_REFLOG_ACTION"
const char sign_off_header[] = "Signed-off-by: ";
+const char cherry_picked_prefix[] = "(cherry picked from commit ";
static void remove_sequencer_state(void)
{
@@ -492,7 +493,7 @@ static int do_pick_commit(struct commit *commit, struct replay_opts *opts)
}
if (opts->record_origin) {
- strbuf_addstr(&msgbuf, "(cherry picked from commit ");
+ strbuf_addstr(&msgbuf, cherry_picked_prefix);
strbuf_addstr(&msgbuf, sha1_to_hex(commit->object.sha1));
strbuf_addstr(&msgbuf, ")\n");
}
@@ -1017,13 +1018,34 @@ int sequencer_pick_revisions(struct replay_opts *opts)
return pick_commits(todo_list, opts);
}
+static int is_rfc2822_line(const char *buf, int len)
+{
+ int i;
+
+ for (i = 0; i < len; i++) {
+ int ch = buf[i];
+ if (ch == ':')
+ break;
+ if (isalnum(ch) || (ch == '-'))
+ continue;
+ return 0;
+ }
+
+ return 1;
+}
+
+static int is_cherry_pick_from_line(const char *buf, int len)
+{
+ return (strlen(cherry_picked_prefix) + 41) <= len &&
+ !prefixcmp(buf, cherry_picked_prefix);
+}
+
static int ends_rfc2822_footer(struct strbuf *sb, int ignore_footer)
{
- int ch;
int hit = 0;
- int i, j, k;
+ int i, k;
int len = sb->len - ignore_footer;
- int first = 1;
+ int last_was_rfc2822 = 0;
const char *buf = sb->buf;
for (i = len - 1; i > 0; i--) {
@@ -1040,20 +1062,12 @@ static int ends_rfc2822_footer(struct strbuf *sb, int ignore_footer)
; /* do nothing */
k++;
- if ((buf[i] == ' ' || buf[i] == '\t') && !first)
+ if (last_was_rfc2822 && (buf[i] == ' ' || buf[i] == '\t'))
continue;
- first = 0;
-
- for (j = 0; i + j < len; j++) {
- ch = buf[i + j];
- if (ch == ':')
- break;
- if (isalnum(ch) ||
- (ch == '-'))
- continue;
+ if (!((last_was_rfc2822 = is_rfc2822_line(buf+i, k-i)) ||
+ is_cherry_pick_from_line(buf+i, k-i)))
return 0;
- }
}
return 1;
}
diff --git a/t/t3511-cherry-pick-x.sh b/t/t3511-cherry-pick-x.sh
index b2098e0..785486e 100755
--- a/t/t3511-cherry-pick-x.sh
+++ b/t/t3511-cherry-pick-x.sh
@@ -63,7 +63,7 @@ test_expect_success 'cherry-pick -s not confused by rfc2822 continuation line' '
test_cmp expect actual
'
-test_expect_failure 'cherry-pick treats -s "(cherry picked from..." line as part of footer' '
+test_expect_success 'cherry-pick treats -s "(cherry picked from..." line as part of footer' '
pristine_detach initial &&
git cherry-pick -s rfc2822-cherry-base &&
cat <<-EOF >expect &&
--
1.8.0
^ permalink raw reply related
* [PATCH 2/5] t/t3511: demonstrate breakage in cherry-pick -s
From: Brandon Casey @ 2012-11-15 1:37 UTC (permalink / raw)
To: git; +Cc: Brandon Casey, Brandon Casey
In-Reply-To: <1352943474-15573-1-git-send-email-drafnel@gmail.com>
The cherry-pick -s functionality is currently broken in two ways.
1. handling of rfc2822 continuation lines has a bug, and the
continuation lines are not handled correctly.
2. the "(cherry picked from ...)" lines are commonly appended to the
end of the s-o-b footer and should be detected as part of the footer.
They should not cause a new line to be inserted before the new s-o-b
is added.
i.e. we should produce this:
Signed-off-by: A.U. Thor <author@example.com>
(cherry picked from )
Signed-off-by: C O Mmitter <committer@example.com>
not
Signed-off-by: A.U. Thor <author@example.com>
(cherry picked from da39a3ee5e6b4b0d3255bfef95601890afd80709)
Signed-off-by: C O Mmitter <committer@example.com>
Signed-off-by: Brandon Casey <bcasey@nvidia.com>
---
t/t3511-cherry-pick-x.sh | 77 ++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 77 insertions(+)
create mode 100755 t/t3511-cherry-pick-x.sh
diff --git a/t/t3511-cherry-pick-x.sh b/t/t3511-cherry-pick-x.sh
new file mode 100755
index 0000000..b4e5c65
--- /dev/null
+++ b/t/t3511-cherry-pick-x.sh
@@ -0,0 +1,77 @@
+#!/bin/sh
+
+test_description='Test cherry-pick -x and -s'
+
+. ./test-lib.sh
+
+pristine_detach () {
+ git cherry-pick --quit &&
+ git checkout -f "$1^0" &&
+ git read-tree -u --reset HEAD &&
+ git clean -d -f -f -q -x
+}
+
+non_rfc2822_mesg='base with footer
+
+Commit message body is here.'
+
+rfc2822_mesg="$non_rfc2822_mesg
+
+Signed-off-by: A.U. Thor
+ <author@example.com>
+Signed-off-by: B.U. Thor <buthor@example.com>"
+
+rfc2822_cherry_mesg="$rfc2822_mesg
+(cherry picked from commit da39a3ee5e6b4b0d3255bfef95601890afd80709)
+Tested-by: C.U. Thor <cuthor@example.com>"
+
+
+test_expect_success setup '
+ git config advice.detachedhead false &&
+ echo unrelated >unrelated &&
+ git add unrelated &&
+ test_commit initial foo a &&
+ test_commit "$non_rfc2822_mesg" foo b non-rfc2822-base &&
+ git reset --hard initial &&
+ test_commit "$rfc2822_mesg" foo b rfc2822-base &&
+ git reset --hard initial &&
+ test_commit "$rfc2822_cherry_mesg" foo b rfc2822-cherry-base &&
+ pristine_detach initial &&
+ test_commit conflicting unrelated
+'
+
+test_expect_success 'cherry-pick -s inserts blank line after non-rfc2822 footer' '
+ pristine_detach initial &&
+ git cherry-pick -s non-rfc2822-base &&
+ cat <<-EOF >expect &&
+ $non_rfc2822_mesg
+
+ Signed-off-by: $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL>
+ EOF
+ git log -1 --pretty=format:%B >actual &&
+ test_cmp expect actual
+'
+
+test_expect_failure 'cherry-pick -s not confused by rfc2822 continuation line' '
+ pristine_detach initial &&
+ git cherry-pick -s rfc2822-base &&
+ cat <<-EOF >expect &&
+ $rfc2822_mesg
+ Signed-off-by: $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL>
+ EOF
+ git log -1 --pretty=format:%B >actual &&
+ test_cmp expect actual
+'
+
+test_expect_failure 'cherry-pick treats -s "(cherry picked from..." line as part of footer' '
+ pristine_detach initial &&
+ git cherry-pick -s rfc2822-cherry-base &&
+ cat <<-EOF >expect &&
+ $rfc2822_cherry_mesg
+ Signed-off-by: $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL>
+ EOF
+ git log -1 --pretty=format:%B >actual &&
+ test_cmp expect actual
+'
+
+test_done
--
1.8.0
^ permalink raw reply related
* [PATCH 1/5] t/test-lib-functions.sh: allow to specify the tag name to test_commit
From: Brandon Casey @ 2012-11-15 1:37 UTC (permalink / raw)
To: git; +Cc: Brandon Casey, Brandon Casey
The <message> part of test_commit() may not be appropriate for a tag name.
So let's allow test_commit to accept a fourth argument to specify the tag
name.
Signed-off-by: Brandon Casey <bcasey@nvidia.com>
---
t/test-lib-functions.sh | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh
index 8889ba5..4d6057e 100644
--- a/t/test-lib-functions.sh
+++ b/t/test-lib-functions.sh
@@ -135,12 +135,13 @@ test_pause () {
fi
}
-# Call test_commit with the arguments "<message> [<file> [<contents>]]"
+# Call test_commit with the arguments "<message> [<file> [<contents> [<tag>]]]"
#
# This will commit a file with the given contents and the given commit
-# message. It will also add a tag with <message> as name.
+# message. It will also add a tag with <message> as name unless <tag> is
+# given.
#
-# Both <file> and <contents> default to <message>.
+# Both <file> <contents> and <tag> default to <message>.
test_commit () {
notick= &&
@@ -168,7 +169,7 @@ test_commit () {
test_tick
fi &&
git commit $signoff -m "$1" &&
- git tag "$1"
+ git tag "${4:-$1}"
}
# Call test_merge with the arguments "<message> <commit>", where <commit>
--
1.8.0
^ permalink raw reply related
* Re: Local clones aka forks disk size optimization
From: Andrew Ardill @ 2012-11-15 1:34 UTC (permalink / raw)
To: Javier Domingo; +Cc: git@vger.kernel.org
In-Reply-To: <CALZVapmBM78UtjAiNm2VoeWuetCiyxN70mTxbG14SQh5a5RCeQ@mail.gmail.com>
On 15 November 2012 12:15, Javier Domingo <javierdo1@gmail.com> wrote:
> Hi Andrew,
>
> Doing this would require I got tracked which one comes from which. So
> it would imply some logic (and db) over it. With the hardlinking way,
> it wouldn't require anything. The idea is that you don't have to do
> anything else in the server.
>
> I understand that it would be imposible to do it for windows users
> (but using cygwin), but for *nix ones yes...
> Javier Domingo
Paraphrasing from git-clone(1):
When cloning a repository, if the source repository is specified with
/path/to/repo syntax, the default is to clone the repository by making
a copy of HEAD and everything under objects and refs directories. The
files under .git/objects/ directory are hardlinked to save space when
possible. To force copying instead of hardlinking (which may be
desirable if you are trying to make a back-up of your repository)
--no-hardlinks can be used.
So hardlinks should be used where possible, and if they are not try
upgrading Git.
I think that covers all the use cases you have?
Regards,
Andrew Ardill
^ permalink raw reply
* Re: What's cooking in git.git (Nov 2012, #04; Wed, 14)
From: Felipe Contreras @ 2012-11-15 1:22 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vhaorajrh.fsf@alter.siamese.dyndns.org>
On Wed, Nov 14, 2012 at 11:42 PM, Junio C Hamano <gitster@pobox.com> wrote:
> * fc/completion-test-simplification (2012-10-29) 2 commits
> - completion: simplify __gitcomp test helper
> - completion: refactor __gitcomp related tests
>
> Clean up completion tests.
>
> There were some comments on the list.
>
> Expecting a re-roll.
This was already re-rolled
http://article.gmane.org/gmane.comp.version-control.git/209382
--
Felipe Contreras
^ permalink raw reply
* Re: [PATCH 2/2] pickaxe: use textconv for -S counting
From: Jeff King @ 2012-11-15 1:21 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Peter Oberndorfer, git
In-Reply-To: <7vk3tpcd0w.fsf@alter.siamese.dyndns.org>
On Tue, Nov 13, 2012 at 03:13:19PM -0800, Junio C Hamano wrote:
> > static int has_changes(struct diff_filepair *p, struct diff_options *o,
> > regex_t *regexp, kwset_t kws)
> > {
> > + struct userdiff_driver *textconv_one = get_textconv(p->one);
> > + struct userdiff_driver *textconv_two = get_textconv(p->two);
> > + mmfile_t mf1, mf2;
> > + int ret;
> > +
> > if (!o->pickaxe[0])
> > return 0;
> >
> > - if (!DIFF_FILE_VALID(p->one)) {
> > - if (!DIFF_FILE_VALID(p->two))
> > - return 0; /* ignore unmerged */
>
> What happened to this part that avoids showing nonsense for unmerged
> paths?
It's moved down. fill_one will return an empty mmfile if
!DIFF_FILE_VALID, so we end up here:
fill_one(p->one, &mf1, &textconv_one);
fill_one(p->two, &mf2, &textconv_two);
if (!mf1.ptr) {
if (!mf2.ptr)
ret = 0; /* ignore unmerged */
Prior to this change, we didn't use fill_one, so we had to check manually.
> > + /*
> > + * If we have an unmodified pair, we know that the count will be the
> > + * same and don't even have to load the blobs. Unless textconv is in
> > + * play, _and_ we are using two different textconv filters (e.g.,
> > + * because a pair is an exact rename with different textconv attributes
> > + * for each side, which might generate different content).
> > + */
> > + if (textconv_one == textconv_two && diff_unmodified_pair(p))
> > + return 0;
>
> I am not sure about this part that cares about the textconv.
>
> Wouldn't the normal "git diff A B" skip the filepair that are
> unmodified in the first place at the object name level without even
> looking at the contents (see e.g. diff_flush_patch())?
Hmph. The point was to find the case when the paths are different (e.g.,
in a rename), and therefore the textconvs might be different. But I
think I missed the fact that diff_unmodified_pair will note the
difference in paths. So just calling diff_unmodified_pair would be
sufficient, as the code prior to my patch does.
I thought the point was an optimization to avoid comparing contains() on
the same data (which we can know will match without looking at it).
Exact renames are the obvious one, but they are not handled here. So I
am not sure of the point (to catch "git diff $blob1 $blob2" when the two
are identical? I am not sure at what layer we cull that from the diff
queue).
So there is room for optimization here on exact renames, but
diff_unmodified_pair is too forgiving of what is interesting (a rename
is interesting to diff_flush_patch, because it wants to mention the
rename, but it is not interesting to pickaxe, because we did not change
the content, and it could be culled here).
I don't know that it is that big a deal in general. Pure renames are
going to be the minority of blobs we look at, so it is probably not even
measurable. You could construct a pathological case (e.g., an otherwise
small repo with a 2G file, rename the 2G file without modification, then
running "git log -Sfoo" will unnecessarily load the giant blob while
examining the rename commit).
> Shouldn't this part of the code emulating that behaviour no matter
> what textconv filter(s) are configured for these paths?
Yeah, I just missed that it is checking the path already. It may still
make sense to tighten the optimization, but that is a separate issue. It
should just check diff_unmodified_pair as before; textconv only matters
if you are trying to optimize out exact renames.
-Peff
^ permalink raw reply
* Re: Local clones aka forks disk size optimization
From: Javier Domingo @ 2012-11-15 1:15 UTC (permalink / raw)
To: Andrew Ardill; +Cc: git@vger.kernel.org
In-Reply-To: <CAH5451m4saVa7-NLbVbXp7q8ca5_0N4FLk3wYaqxLT=AE5frbw@mail.gmail.com>
Hi Andrew,
Doing this would require I got tracked which one comes from which. So
it would imply some logic (and db) over it. With the hardlinking way,
it wouldn't require anything. The idea is that you don't have to do
anything else in the server.
I understand that it would be imposible to do it for windows users
(but using cygwin), but for *nix ones yes...
Javier Domingo
2012/11/15 Andrew Ardill <andrew.ardill@gmail.com>:
> On 15 November 2012 11:40, Javier Domingo <javierdo1@gmail.com> wrote:
>> Hi Andrew,
>>
>> The problem about that, is that if I want to delete the first repo, I
>> will loose objects... Or does that repack also hard-link the objects
>> in other repos? I don't want to accidentally loose data, so it would
>> be nice that althought avoided to repack things, it would also
>> hardlink them.
>
> Hi Javier, check out the section below the one I linked earlier:
>
>> How to stop sharing objects between repositories?
>>
>> To copy the shared objects into the local repository, repack without the -l flag
>>
>> git repack -a
>>
>> Then remove the pointer to the alternate object store
>>
>> rm .git/objects/info/alternates
>>
>> (If the repository is edited between the two steps, it could become corrupted
>> when the alternates file is removed. If you're unsure, you can use git fsck to
>> check for corruption. If things go wrong, you can always recover by replacing
>> the alternates file and starting over).
>
> Regards,
>
> Andrew Ardill
^ permalink raw reply
* Re: Local clones aka forks disk size optimization
From: Andrew Ardill @ 2012-11-15 0:53 UTC (permalink / raw)
To: Javier Domingo; +Cc: git@vger.kernel.org
In-Reply-To: <CALZVap=kOwOpxeu8+_+5uQYZz3GNC8Ep_JeK7WCQHtu+Hn3rUw@mail.gmail.com>
On 15 November 2012 11:40, Javier Domingo <javierdo1@gmail.com> wrote:
> Hi Andrew,
>
> The problem about that, is that if I want to delete the first repo, I
> will loose objects... Or does that repack also hard-link the objects
> in other repos? I don't want to accidentally loose data, so it would
> be nice that althought avoided to repack things, it would also
> hardlink them.
Hi Javier, check out the section below the one I linked earlier:
> How to stop sharing objects between repositories?
>
> To copy the shared objects into the local repository, repack without the -l flag
>
> git repack -a
>
> Then remove the pointer to the alternate object store
>
> rm .git/objects/info/alternates
>
> (If the repository is edited between the two steps, it could become corrupted
> when the alternates file is removed. If you're unsure, you can use git fsck to
> check for corruption. If things go wrong, you can always recover by replacing
> the alternates file and starting over).
Regards,
Andrew Ardill
^ permalink raw reply
* Re: Local clones aka forks disk size optimization
From: Javier Domingo @ 2012-11-15 0:40 UTC (permalink / raw)
To: Andrew Ardill; +Cc: git@vger.kernel.org
In-Reply-To: <CAH5451nW2esQR8XaAttT3tYJZEw1Nj3OEMgkHsMZrZDxhcRXHw@mail.gmail.com>
Hi Andrew,
The problem about that, is that if I want to delete the first repo, I
will loose objects... Or does that repack also hard-link the objects
in other repos? I don't want to accidentally loose data, so it would
be nice that althought avoided to repack things, it would also
hardlink them.
Javier Domingo
2012/11/15 Andrew Ardill <andrew.ardill@gmail.com>:
> On 15 November 2012 10:42, Javier Domingo <javierdo1@gmail.com> wrote:
>> Hi,
>>
>> I have come up with this while doing some local forks for work.
>> Currently, when you clone a repo using a path (not file:/// protocol)
>> you get all the common objects linked.
>>
>> But as you work, each one will continue growing on its way, although
>> they may have common objects.
>>
>> Is there any way to avoid this? I mean, can something be done in git,
>> that it checks for (when pulling) the same objects in the other forks?
>
> Have you seen alternates? From [1]:
>
>> How to share objects between existing repositories?
>> ---------------------------------------------------------------------------
>>
>> Do
>>
>> echo "/source/git/project/.git/objects/" > .git/objects/info/alternates
>>
>> and then follow it up with
>>
>> git repack -a -d -l
>>
>> where the '-l' means that it will only put local objects in the pack-file
>> (strictly speaking, it will put any loose objects from the alternate tree
>> too, so you'll have a fully packed archive, but it won't duplicate objects
>> that are already packed in the alternate tree).
>
> [1] https://git.wiki.kernel.org/index.php/GitFaq#How_to_share_objects_between_existing_repositories.3F
>
>
> Regards,
>
> Andrew Ardill
^ permalink raw reply
* [PATCHv2 8/8] send-email: do not prompt for explicit repo ident
From: Jeff King @ 2012-11-15 0:36 UTC (permalink / raw)
To: git; +Cc: Felipe Contreras, git, Thomas Rast, Junio C Hamano,
Jonathan Nieder
In-Reply-To: <20121115003029.GA17550@sigill.intra.peff.net>
If git-send-email is configured with sendemail.from, we will
not prompt the user for the "From" address of the emails.
If it is not configured, we prompt the user, but provide the
repo author or committer as a default. Even though we
probably have a sensible value for the default, the prompt
is a safety check in case git generated an incorrect
implicit ident string.
Now that Git.pm will tell us whether the ident is implicit or
explicit, we can stop prompting in the explicit case, saving
most users from having to see the prompt at all.
Signed-off-by: Jeff King <peff@peff.net>
---
git-send-email.perl | 22 +++++++++++++---------
t/t9001-send-email.sh | 36 ++++++++++++++++++++++++++++++++++--
2 files changed, 47 insertions(+), 11 deletions(-)
diff --git a/git-send-email.perl b/git-send-email.perl
index 5a7c29d..0c49b32 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -436,9 +436,8 @@ if (0) {
}
}
-my ($repoauthor, $repocommitter);
-($repoauthor) = Git::ident_person(@repo, 'author');
-($repocommitter) = Git::ident_person(@repo, 'committer');
+my ($repoauthor, $author_explicit) = Git::ident_person(@repo, 'author');
+my ($repocommitter, $committer_explicit) = Git::ident_person(@repo, 'committer');
# Verify the user input
@@ -755,12 +754,17 @@ if (!$force) {
my $prompting = 0;
if (!defined $sender) {
- $sender = $repoauthor || $repocommitter || '';
- $sender = ask("Who should the emails appear to be from? [$sender] ",
- default => $sender,
- valid_re => qr/\@.*\./, confirm_only => 1);
- print "Emails will be sent from: ", $sender, "\n";
- $prompting++;
+ ($sender, my $explicit) =
+ defined $repoauthor ? ($repoauthor, $author_explicit) :
+ defined $repocommitter ? ($repocommitter, $committer_explicit) :
+ ('', 0);
+ if (!$explicit) {
+ $sender = ask("Who should the emails appear to be from? [$sender] ",
+ default => $sender,
+ valid_re => qr/\@.*\./, confirm_only => 1);
+ print "Emails will be sent from: ", $sender, "\n";
+ $prompting++;
+ }
}
if (!@initial_to && !defined $to_cmd) {
diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh
index 6c6af7d..0fe0b8e 100755
--- a/t/t9001-send-email.sh
+++ b/t/t9001-send-email.sh
@@ -191,15 +191,47 @@ test_expect_success $PREREQ 'Show all headers' '
test_expect_success $PREREQ 'Prompting works' '
clean_fake_sendmail &&
- (echo "Example <from@example.com>"
- echo "to@example.com"
+ (echo "to@example.com"
echo ""
) | GIT_SEND_EMAIL_NOTTY=1 git send-email \
--smtp-server="$(pwd)/fake.sendmail" \
$patches \
2>errors &&
+ grep "^From: A U Thor <author@example.com>\$" msgtxt1 &&
+ grep "^To: to@example.com\$" msgtxt1
+'
+
+test_expect_success $PREREQ,AUTOIDENT 'implicit ident prompts for sender' '
+ clean_fake_sendmail &&
+ (echo "Example <from@example.com>" &&
+ echo "to@example.com" &&
+ echo ""
+ ) |
+ (sane_unset GIT_AUTHOR_NAME &&
+ sane_unset GIT_AUTHOR_EMAIL &&
+ sane_unset GIT_COMMITTER_NAME &&
+ sane_unset GIT_COMMITTER_EMAIL &&
+ GIT_SEND_EMAIL_NOTTY=1 git send-email \
+ --smtp-server="$(pwd)/fake.sendmail" \
+ $patches \
+ 2>errors &&
grep "^From: Example <from@example.com>\$" msgtxt1 &&
grep "^To: to@example.com\$" msgtxt1
+ )
+'
+
+test_expect_success $PREREQ,!AUTOIDENT 'broken implicit ident aborts send-email' '
+ clean_fake_sendmail &&
+ (sane_unset GIT_AUTHOR_NAME &&
+ sane_unset GIT_AUTHOR_EMAIL &&
+ sane_unset GIT_COMMITTER_NAME &&
+ sane_unset GIT_COMMITTER_EMAIL &&
+ GIT_SEND_EMAIL_NOTTY=1 && export GIT_SEND_EMAIL_NOTTY &&
+ test_must_fail git send-email \
+ --smtp-server="$(pwd)/fake.sendmail" \
+ $patches </dev/null 2>errors.out &&
+ test_i18ngrep "tell me who you are" errors.out
+ )
'
test_expect_success $PREREQ 'tocmd works' '
--
1.8.0.207.gdf2154c
^ permalink raw reply related
* [PATCHv2 7/8] Git.pm: teach "ident" to query explicitness
From: Jeff King @ 2012-11-15 0:36 UTC (permalink / raw)
To: git; +Cc: Felipe Contreras, git, Thomas Rast, Junio C Hamano,
Jonathan Nieder
In-Reply-To: <20121115003029.GA17550@sigill.intra.peff.net>
"git var" recently learned to report on whether an ident we
fetch from it was configured explicitly or implicitly. Let's
make that information available to callers of the ident
function.
Because evaluating "ident" in an array versus scalar context
already has a meaning, we cannot return our extra value in a
backwards compatible way. Instead, we require the caller to
add an extra "explicit" flag to request the information.
The ident_person function, on the other hand, always returns
a scalar, so we are free to overload it in an array context.
Signed-off-by: Jeff King <peff@peff.net>
---
perl/Git.pm | 32 ++++++++++++++++++++++++--------
1 file changed, 24 insertions(+), 8 deletions(-)
diff --git a/perl/Git.pm b/perl/Git.pm
index 497f420..28075a9 100644
--- a/perl/Git.pm
+++ b/perl/Git.pm
@@ -737,7 +737,7 @@ sub remote_refs {
}
-=item ident ( TYPE | IDENTSTR )
+=item ident ( TYPE | IDENTSTR [, options] )
=item ident_person ( TYPE | IDENTSTR | IDENTARRAY )
@@ -750,8 +750,16 @@ and either returns it as a scalar string or as an array with the fields parsed.
Alternatively, it can take a prepared ident string (e.g. from the commit
object) and just parse it.
+If the C<explicit> option is set to 1, we are in an array context, and we are
+using C<git var> to lookup the ident, the returned array will contain an
+additional boolean element specifying whether the ident was configured
+explicitly by the user. See the description of GIT_*_EXPLICIT in git-var(1) for
+details.
+
C<ident_person> returns the person part of the ident - name and email;
it can take the same arguments as C<ident> or the array returned by C<ident>.
+In an array context, C<ident_person> returns both the person string and the
+C<explicit> boolean.
The synopsis is like:
@@ -763,17 +771,22 @@ The synopsis is like:
=cut
sub ident {
- my ($self, $type) = _maybe_self(@_);
- my $identstr;
+ my ($self, $type, %options) = _maybe_self(@_);
+ my ($identstr, $explicit);
if (lc $type eq lc 'committer' or lc $type eq lc 'author') {
- my @cmd = ('var', 'GIT_'.uc($type).'_IDENT');
+ my $uc = uc($type);
+ my @cmd = ('var', "GIT_${uc}_IDENT", "GIT_${uc}_EXPLICIT");
unshift @cmd, $self if $self;
- $identstr = command_oneline(@cmd);
+ ($identstr, $explicit) = command(@cmd);
} else {
$identstr = $type;
}
if (wantarray) {
- return $identstr =~ /^(.*) <(.*)> (\d+ [+-]\d{4})$/;
+ my @ret = $identstr =~ /^(.*) <(.*)> (\d+ [+-]\d{4})$/;
+ if ($options{explicit} && defined $explicit) {
+ push @ret, $explicit;
+ }
+ return @ret;
} else {
return $identstr;
}
@@ -781,8 +794,11 @@ sub ident {
sub ident_person {
my ($self, @ident) = _maybe_self(@_);
- $#ident == 0 and @ident = $self ? $self->ident($ident[0]) : ident($ident[0]);
- return "$ident[0] <$ident[1]>";
+ $#ident == 0 and @ident = $self ?
+ $self->ident($ident[0], explicit => 1) :
+ ident($ident[0], explicit => 1);
+ my $ret = "$ident[0] <$ident[1]>";
+ return wantarray ? ($ret, @ident[3]) : $ret;
}
--
1.8.0.207.gdf2154c
^ permalink raw reply related
* [PATCHv2 6/8] var: provide explicit/implicit ident information
From: Jeff King @ 2012-11-15 0:35 UTC (permalink / raw)
To: git; +Cc: Felipe Contreras, git, Thomas Rast, Junio C Hamano,
Jonathan Nieder
In-Reply-To: <20121115003029.GA17550@sigill.intra.peff.net>
Internally, we keep track of whether the author or committer
ident information was provided by the user, or whether it
was implicitly determined by the system. However, there is
currently no way for external programs or scripts to get
this information without re-implementing the ident logic
themselves. Let's provide a way for them to do so.
Signed-off-by: Jeff King <peff@peff.net>
---
Documentation/git-var.txt | 12 ++++++++++++
builtin/var.c | 27 +++++++++++++++++++++++++++
t/t0007-git-var.sh | 36 ++++++++++++++++++++++++++++++++++++
3 files changed, 75 insertions(+)
diff --git a/Documentation/git-var.txt b/Documentation/git-var.txt
index 53abba5..bb2997d 100644
--- a/Documentation/git-var.txt
+++ b/Documentation/git-var.txt
@@ -39,9 +39,21 @@ VARIABLES
GIT_AUTHOR_IDENT::
The author of a piece of code.
+GIT_AUTHOR_EXPLICIT::
+ Outputs "1" if the author identity was provided explicitly by the
+ user, or "0" if it was determined implicitly from the system.
+ Identity is considered explicit if it comes from one of git's config
+ file (i.e., via the `user.*` variables), from the `$GIT_AUTHOR_*`
+ environment variables, or from the `$EMAIL` environment variable. It
+ is considered implicit if it was generated from system variables
+ like the username and hostname.
+
GIT_COMMITTER_IDENT::
The person who put a piece of code into git.
+GIT_COMMITTER_EXPLICIT::
+ Like GIT_AUTHOR_EXPLICIT, but for the committer ident.
+
GIT_EDITOR::
Text editor for use by git commands. The value is meant to be
interpreted by the shell when it is used. Examples: `~/bin/vi`,
diff --git a/builtin/var.c b/builtin/var.c
index 49ab300..9503df9 100644
--- a/builtin/var.c
+++ b/builtin/var.c
@@ -26,13 +26,40 @@ static const char *pager(int flag)
return pgm;
}
+static const char *explicit_ident(const char * (*get_ident)(int),
+ int (*check_ident)(void))
+{
+ /*
+ * Prime the "explicit" flag by getting the identity.
+ * We do not want to be strict here, because we would not want
+ * to die on bogus implicit values, but instead just report that they
+ * are not explicit.
+ */
+ get_ident(0);
+ return check_ident() ? "1" : "0";
+}
+
+static const char *committer_explicit(int flag)
+{
+ return explicit_ident(git_committer_info,
+ committer_ident_sufficiently_given);
+}
+
+static const char *author_explicit(int flag)
+{
+ return explicit_ident(git_author_info,
+ author_ident_sufficiently_given);
+}
+
struct git_var {
const char *name;
const char *(*read)(int);
};
static struct git_var git_vars[] = {
{ "GIT_COMMITTER_IDENT", git_committer_info },
+ { "GIT_COMMITTER_EXPLICIT", committer_explicit },
{ "GIT_AUTHOR_IDENT", git_author_info },
+ { "GIT_AUTHOR_EXPLICIT", author_explicit },
{ "GIT_EDITOR", editor },
{ "GIT_PAGER", pager },
{ "", NULL },
diff --git a/t/t0007-git-var.sh b/t/t0007-git-var.sh
index ec5d946..47907de 100755
--- a/t/t0007-git-var.sh
+++ b/t/t0007-git-var.sh
@@ -47,4 +47,40 @@ test_expect_success 'git var can show multiple values' '
test_cmp expect actual
'
+for type in AUTHOR COMMITTER; do
+ test_expect_success "$type ident can detect implicit values" "
+ echo 0 >expect &&
+ (
+ sane_unset GIT_${type}_NAME &&
+ sane_unset GIT_${type}_EMAIL &&
+ sane_unset EMAIL &&
+ git var GIT_${type}_EXPLICIT >actual
+ ) &&
+ test_cmp expect actual
+ "
+
+ test_expect_success "$type ident is explicit via environment" "
+ echo 1 >expect &&
+ (
+ GIT_${type}_NAME='A Name' &&
+ export GIT_${type}_NAME &&
+ GIT_${type}_EMAIL='name@example.com' &&
+ export GIT_${type}_EMAIL &&
+ git var GIT_${type}_EXPLICIT >actual
+ ) &&
+ test_cmp expect actual
+ "
+
+ test_expect_success "$type ident is explicit via config" "
+ echo 1 >expect &&
+ test_config user.name 'A Name' &&
+ test_config user.email 'name@example.com' &&
+ (
+ sane_unset GIT_${type}_NAME &&
+ sane_unset GIT_${type}_EMAIL &&
+ git var GIT_${type}_EXPLICIT >actual
+ )
+ "
+done
+
test_done
--
1.8.0.207.gdf2154c
^ permalink raw reply related
* [PATCHv2 5/8] var: accept multiple variables on the command line
From: Jeff King @ 2012-11-15 0:35 UTC (permalink / raw)
To: git; +Cc: Felipe Contreras, git, Thomas Rast, Junio C Hamano,
Jonathan Nieder
In-Reply-To: <20121115003029.GA17550@sigill.intra.peff.net>
Git-var currently only accepts a single value to print. This
is inefficient if the caller is interested in finding
multiple values, as they must invoke git-var multiple times.
This patch lets callers specify multiple variables, and
prints one per line.
While we're in the area, let's add some basic tests for "git
var", which until now was largely untested (and of course a
test for multiple values on top).
Signed-off-by: Jeff King <peff@peff.net>
---
Documentation/git-var.txt | 9 +++++++--
builtin/var.c | 17 +++++++++-------
t/t0007-git-var.sh | 50 +++++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 67 insertions(+), 9 deletions(-)
create mode 100755 t/t0007-git-var.sh
diff --git a/Documentation/git-var.txt b/Documentation/git-var.txt
index 67edf58..53abba5 100644
--- a/Documentation/git-var.txt
+++ b/Documentation/git-var.txt
@@ -9,11 +9,16 @@ git-var - Show a git logical variable
SYNOPSIS
--------
[verse]
-'git var' ( -l | <variable> )
+'git var' ( -l | <variable>... )
DESCRIPTION
-----------
-Prints a git logical variable.
+Prints one or more git logical variables, separated by newlines.
+
+Note that some variables may contain newlines themselves (e.g.,
+`GIT_EDITOR`), and it is therefore possible to receive ambiguous output
+when requesting multiple variables. This can be mitigated by putting any
+such variables at the end of the list.
OPTIONS
-------
diff --git a/builtin/var.c b/builtin/var.c
index aedbb53..49ab300 100644
--- a/builtin/var.c
+++ b/builtin/var.c
@@ -5,7 +5,7 @@
*/
#include "builtin.h"
-static const char var_usage[] = "git var (-l | <variable>)";
+static const char var_usage[] = "git var (-l | <variable>...)";
static const char *editor(int flag)
{
@@ -73,21 +73,24 @@ static int show_config(const char *var, const char *value, void *cb)
int cmd_var(int argc, const char **argv, const char *prefix)
{
- const char *val = NULL;
- if (argc != 2)
+ if (argc < 2)
usage(var_usage);
if (strcmp(argv[1], "-l") == 0) {
+ if (argc > 2)
+ usage(var_usage);
git_config(show_config, NULL);
list_vars();
return 0;
}
git_config(git_default_config, NULL);
- val = read_var(argv[1]);
- if (!val)
- usage(var_usage);
- printf("%s\n", val);
+ while (*++argv) {
+ const char *val = read_var(*argv);
+ if (!val)
+ usage(var_usage);
+ printf("%s\n", val);
+ }
return 0;
}
diff --git a/t/t0007-git-var.sh b/t/t0007-git-var.sh
new file mode 100755
index 0000000..ec5d946
--- /dev/null
+++ b/t/t0007-git-var.sh
@@ -0,0 +1,50 @@
+#!/bin/sh
+
+test_description='basic sanity checks for git var'
+. ./test-lib.sh
+
+test_expect_success 'get GIT_AUTHOR_IDENT' '
+ test_tick &&
+ echo "$GIT_AUTHOR_NAME <$GIT_AUTHOR_EMAIL> $GIT_AUTHOR_DATE" >expect &&
+ git var GIT_AUTHOR_IDENT >actual &&
+ test_cmp expect actual
+'
+
+test_expect_success 'get GIT_COMMITTER_IDENT' '
+ test_tick &&
+ echo "$GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> $GIT_COMMITTER_DATE" >expect &&
+ git var GIT_COMMITTER_IDENT >actual &&
+ test_cmp expect actual
+'
+
+# For git var -l, we check only a representative variable;
+# testing the whole output would make our test too brittle with
+# respect to unrelated changes in the test suite's environment.
+test_expect_success 'git var -l lists variables' '
+ git var -l >actual &&
+ echo "$GIT_AUTHOR_NAME <$GIT_AUTHOR_EMAIL> $GIT_AUTHOR_DATE" >expect &&
+ sed -n s/GIT_AUTHOR_IDENT=//p <actual >actual.author &&
+ test_cmp expect actual.author
+'
+
+test_expect_success 'git var -l lists config' '
+ git var -l >actual &&
+ echo false >expect &&
+ sed -n s/core\\.bare=//p <actual >actual.bare &&
+ test_cmp expect actual.bare
+'
+
+test_expect_success 'listing and asking for variables are exclusive' '
+ test_must_fail git var -l GIT_COMMITTER_IDENT
+'
+
+test_expect_success 'git var can show multiple values' '
+ cat >expect <<-EOF &&
+ $GIT_AUTHOR_NAME <$GIT_AUTHOR_EMAIL> $GIT_AUTHOR_DATE
+ $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> $GIT_COMMITTER_DATE
+ EOF
+ git var GIT_AUTHOR_IDENT GIT_COMMITTER_IDENT >actual &&
+ test_cmp expect actual
+'
+
+test_done
--
1.8.0.207.gdf2154c
^ permalink raw reply related
* [PATCHv2 4/8] ident: keep separate "explicit" flags for author and committer
From: Jeff King @ 2012-11-15 0:34 UTC (permalink / raw)
To: git; +Cc: Felipe Contreras, git, Thomas Rast, Junio C Hamano,
Jonathan Nieder
In-Reply-To: <20121115003029.GA17550@sigill.intra.peff.net>
We keep track of whether the user ident was given to us
explicitly, or if we guessed at it from system parameters
like username and hostname. However, we kept only a single
variable. This covers the common cases (because the author
and committer will usually come from the same explicit
source), but can miss two cases:
1. GIT_COMMITTER_* is set explicitly, but we fallback for
GIT_AUTHOR. We claim the ident is explicit, even though
the author is not.
2. GIT_AUTHOR_* is set and we ask for author ident, but
not committer ident. We will claim the ident is
implicit, even though it is explicit.
This patch uses two variables instead of one, updates both
when we set the "fallback" values, and updates them
individually when we read from the environment.
Rather than keep user_ident_sufficiently_given as a
compatibility wrapper, we update the only two callers to
check the committer_ident, which matches their intent and
what was happening already.
Signed-off-by: Jeff King <peff@peff.net>
---
builtin/commit.c | 4 ++--
cache.h | 3 ++-
ident.c | 32 +++++++++++++++++++++++++-------
3 files changed, 29 insertions(+), 10 deletions(-)
diff --git a/builtin/commit.c b/builtin/commit.c
index 1dd2ec5..d6dd3df 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -755,7 +755,7 @@ static int prepare_to_commit(const char *index_file, const char *prefix,
ident_shown++ ? "" : "\n",
author_ident->buf);
- if (!user_ident_sufficiently_given())
+ if (!committer_ident_sufficiently_given())
status_printf_ln(s, GIT_COLOR_NORMAL,
_("%s"
"Committer: %s"),
@@ -1265,7 +1265,7 @@ static void print_summary(const char *prefix, const unsigned char *sha1,
strbuf_addstr(&format, "\n Author: ");
strbuf_addbuf_percentquote(&format, &author_ident);
}
- if (!user_ident_sufficiently_given()) {
+ if (!committer_ident_sufficiently_given()) {
strbuf_addstr(&format, "\n Committer: ");
strbuf_addbuf_percentquote(&format, &committer_ident);
if (advice_implicit_identity) {
diff --git a/cache.h b/cache.h
index 50d9eea..18fdd18 100644
--- a/cache.h
+++ b/cache.h
@@ -1149,7 +1149,8 @@ struct config_include_data {
#define CONFIG_INCLUDE_INIT { 0 }
extern int git_config_include(const char *name, const char *value, void *data);
-extern int user_ident_sufficiently_given(void);
+extern int committer_ident_sufficiently_given(void);
+extern int author_ident_sufficiently_given(void);
extern const char *git_commit_encoding;
extern const char *git_log_output_encoding;
diff --git a/ident.c b/ident.c
index 733d69d..ac9672f 100644
--- a/ident.c
+++ b/ident.c
@@ -14,7 +14,8 @@ static char git_default_date[50];
#define IDENT_NAME_GIVEN 01
#define IDENT_MAIL_GIVEN 02
#define IDENT_ALL_GIVEN (IDENT_NAME_GIVEN|IDENT_MAIL_GIVEN)
-static int user_ident_explicitly_given;
+static int committer_ident_explicitly_given;
+static int author_ident_explicitly_given;
#ifdef NO_GECOS_IN_PWENT
#define get_gecos(ignored) "&"
@@ -113,7 +114,8 @@ const char *ident_default_email(void)
if (email && email[0]) {
strbuf_addstr(&git_default_email, email);
- user_ident_explicitly_given |= IDENT_MAIL_GIVEN;
+ committer_ident_explicitly_given |= IDENT_MAIL_GIVEN;
+ author_ident_explicitly_given |= IDENT_MAIL_GIVEN;
} else
copy_email(xgetpwuid_self(), &git_default_email);
strbuf_trim(&git_default_email);
@@ -331,6 +333,10 @@ const char *fmt_name(const char *name, const char *email)
const char *git_author_info(int flag)
{
+ if (getenv("GIT_AUTHOR_NAME"))
+ author_ident_explicitly_given |= IDENT_NAME_GIVEN;
+ if (getenv("GIT_AUTHOR_EMAIL"))
+ author_ident_explicitly_given |= IDENT_MAIL_GIVEN;
return fmt_ident(getenv("GIT_AUTHOR_NAME"),
getenv("GIT_AUTHOR_EMAIL"),
getenv("GIT_AUTHOR_DATE"),
@@ -340,16 +346,16 @@ const char *git_author_info(int flag)
const char *git_committer_info(int flag)
{
if (getenv("GIT_COMMITTER_NAME"))
- user_ident_explicitly_given |= IDENT_NAME_GIVEN;
+ committer_ident_explicitly_given |= IDENT_NAME_GIVEN;
if (getenv("GIT_COMMITTER_EMAIL"))
- user_ident_explicitly_given |= IDENT_MAIL_GIVEN;
+ committer_ident_explicitly_given |= IDENT_MAIL_GIVEN;
return fmt_ident(getenv("GIT_COMMITTER_NAME"),
getenv("GIT_COMMITTER_EMAIL"),
getenv("GIT_COMMITTER_DATE"),
flag);
}
-int user_ident_sufficiently_given(void)
+static int ident_is_sufficient(int user_ident_explicitly_given)
{
#ifndef WINDOWS
return (user_ident_explicitly_given & IDENT_MAIL_GIVEN);
@@ -358,6 +364,16 @@ int user_ident_sufficiently_given(void)
#endif
}
+int committer_ident_sufficiently_given(void)
+{
+ return ident_is_sufficient(committer_ident_explicitly_given);
+}
+
+int author_ident_sufficiently_given(void)
+{
+ return ident_is_sufficient(author_ident_explicitly_given);
+}
+
int git_ident_config(const char *var, const char *value, void *data)
{
if (!strcmp(var, "user.name")) {
@@ -365,7 +381,8 @@ int git_ident_config(const char *var, const char *value, void *data)
return config_error_nonbool(var);
strbuf_reset(&git_default_name);
strbuf_addstr(&git_default_name, value);
- user_ident_explicitly_given |= IDENT_NAME_GIVEN;
+ committer_ident_explicitly_given |= IDENT_NAME_GIVEN;
+ author_ident_explicitly_given |= IDENT_NAME_GIVEN;
return 0;
}
@@ -374,7 +391,8 @@ int git_ident_config(const char *var, const char *value, void *data)
return config_error_nonbool(var);
strbuf_reset(&git_default_email);
strbuf_addstr(&git_default_email, value);
- user_ident_explicitly_given |= IDENT_MAIL_GIVEN;
+ committer_ident_explicitly_given |= IDENT_MAIL_GIVEN;
+ author_ident_explicitly_given |= IDENT_MAIL_GIVEN;
return 0;
}
--
1.8.0.207.gdf2154c
^ permalink raw reply related
* [PATCHv2 3/8] ident: make user_ident_explicitly_given static
From: Jeff King @ 2012-11-15 0:34 UTC (permalink / raw)
To: git; +Cc: Felipe Contreras, git, Thomas Rast, Junio C Hamano,
Jonathan Nieder
In-Reply-To: <20121115003029.GA17550@sigill.intra.peff.net>
In v1.5.6-rc0~56^2 (2008-05-04) "user_ident_explicitly_given"
was introduced as a global for communication between config,
ident, and builtin-commit. In v1.7.0-rc0~72^2 (2010-01-07)
readers switched to using the common wrapper
user_ident_sufficiently_given(). After v1.7.11-rc1~15^2~18
(2012-05-21), the var is only written in ident.c.
Now we can make it static, which will enable further
refactoring without worrying about upsetting other code.
Signed-off-by: Jeff King <peff@peff.net>
Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>
---
cache.h | 4 ----
ident.c | 6 +++++-
2 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/cache.h b/cache.h
index dbd8018..50d9eea 100644
--- a/cache.h
+++ b/cache.h
@@ -1149,10 +1149,6 @@ struct config_include_data {
#define CONFIG_INCLUDE_INIT { 0 }
extern int git_config_include(const char *name, const char *value, void *data);
-#define IDENT_NAME_GIVEN 01
-#define IDENT_MAIL_GIVEN 02
-#define IDENT_ALL_GIVEN (IDENT_NAME_GIVEN|IDENT_MAIL_GIVEN)
-extern int user_ident_explicitly_given;
extern int user_ident_sufficiently_given(void);
extern const char *git_commit_encoding;
diff --git a/ident.c b/ident.c
index a4bf206..733d69d 100644
--- a/ident.c
+++ b/ident.c
@@ -10,7 +10,11 @@
static struct strbuf git_default_name = STRBUF_INIT;
static struct strbuf git_default_email = STRBUF_INIT;
static char git_default_date[50];
-int user_ident_explicitly_given;
+
+#define IDENT_NAME_GIVEN 01
+#define IDENT_MAIL_GIVEN 02
+#define IDENT_ALL_GIVEN (IDENT_NAME_GIVEN|IDENT_MAIL_GIVEN)
+static int user_ident_explicitly_given;
#ifdef NO_GECOS_IN_PWENT
#define get_gecos(ignored) "&"
--
1.8.0.207.gdf2154c
^ permalink raw reply related
* [PATCHv2 2/8] t7502: factor out autoident prerequisite
From: Jeff King @ 2012-11-15 0:33 UTC (permalink / raw)
To: git; +Cc: Felipe Contreras, git, Thomas Rast, Junio C Hamano,
Jonathan Nieder
In-Reply-To: <20121115003029.GA17550@sigill.intra.peff.net>
t7502 checks the behavior of commit when we can and cannot
determine a valid committer ident. Let's move that into
test-lib as a lazy prerequisite so other scripts can use it.
Signed-off-by: Jeff King <peff@peff.net>
---
t/t7502-commit.sh | 12 +-----------
t/test-lib.sh | 6 ++++++
2 files changed, 7 insertions(+), 11 deletions(-)
diff --git a/t/t7502-commit.sh b/t/t7502-commit.sh
index deb187e..1a5cb69 100755
--- a/t/t7502-commit.sh
+++ b/t/t7502-commit.sh
@@ -243,16 +243,6 @@ test_expect_success 'message shows author when it is not equal to committer' '
.git/COMMIT_EDITMSG
'
-test_expect_success 'setup auto-ident prerequisite' '
- if (sane_unset GIT_COMMITTER_EMAIL &&
- sane_unset GIT_COMMITTER_NAME &&
- git var GIT_COMMITTER_IDENT); then
- test_set_prereq AUTOIDENT
- else
- test_set_prereq NOAUTOIDENT
- fi
-'
-
test_expect_success AUTOIDENT 'message shows committer when it is automatic' '
echo >>negative &&
@@ -271,7 +261,7 @@ echo editor started > "$(pwd)/.git/result"
exit 0
EOF
-test_expect_success NOAUTOIDENT 'do not fire editor when committer is bogus' '
+test_expect_success !AUTOIDENT 'do not fire editor when committer is bogus' '
>.git/result
>expect &&
diff --git a/t/test-lib.sh b/t/test-lib.sh
index 489bc80..0334a9e 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -738,6 +738,12 @@ test_lazy_prereq UTF8_NFD_TO_NFC '
esac
'
+test_lazy_prereq AUTOIDENT '
+ sane_unset GIT_AUTHOR_NAME &&
+ sane_unset GIT_AUTHOR_EMAIL &&
+ git var GIT_AUTHOR_IDENT
+'
+
# When the tests are run as root, permission tests will report that
# things are writable when they shouldn't be.
test -w / || test_set_prereq SANITY
--
1.8.0.207.gdf2154c
^ permalink raw reply related
* [PATCHv2 1/8] test-lib: allow negation of prerequisites
From: Jeff King @ 2012-11-15 0:33 UTC (permalink / raw)
To: git; +Cc: Felipe Contreras, git, Thomas Rast, Junio C Hamano,
Jonathan Nieder
In-Reply-To: <20121115003029.GA17550@sigill.intra.peff.net>
You can set and test a prerequisite like this:
test_set_prereq FOO
test_have_prereq FOO && echo yes
You can negate the test in the shell like this:
! test_have_prereq && echo no
However, when you are using the automatic prerequisite
checking in test_expect_*, there is no opportunity to use
the shell negation. This patch introduces the syntax "!FOO"
to indicate that the test should only run if a prerequisite
is not meant.
One alternative is to set an explicit negative prerequisite,
like:
if system_has_foo; then
test_set_prereq FOO
else
test_set_prereq NO_FOO
fi
However, this doesn't work for lazy prerequisites, which
associate a single test with a single name. We could teach
the lazy prereq evaluator to set both forms, but the code
change ends up quite similar to this one (because we still
need to convert NO_FOO into FOO to find the correct lazy
script).
Signed-off-by: Jeff King <peff@peff.net>
---
I chose "!" as the negation prefix because it will never conflict with
another prerequisite. But we could also use "NO_" if that's more
aesthetically pleasing. I don't have a strong preference.
t/t0000-basic.sh | 32 ++++++++++++++++++++++++++++++++
t/test-lib-functions.sh | 21 ++++++++++++++++++++-
2 files changed, 52 insertions(+), 1 deletion(-)
diff --git a/t/t0000-basic.sh b/t/t0000-basic.sh
index 08677df..562cf41 100755
--- a/t/t0000-basic.sh
+++ b/t/t0000-basic.sh
@@ -115,6 +115,38 @@ then
exit 1
fi
+test_lazy_prereq LAZY_TRUE true
+havetrue=no
+test_expect_success LAZY_TRUE 'test runs if lazy prereq is satisfied' '
+ havetrue=yes
+'
+donthavetrue=yes
+test_expect_success !LAZY_TRUE 'missing lazy prereqs skip tests' '
+ donthavetrue=no
+'
+
+if test "$havetrue$donthavetrue" != yesyes
+then
+ say 'bug in test framework: lazy prerequisites do not work'
+ exit 1
+fi
+
+test_lazy_prereq LAZY_FALSE false
+nothavefalse=no
+test_expect_success !LAZY_FALSE 'negative lazy prereqs checked' '
+ nothavefalse=yes
+'
+havefalse=yes
+test_expect_success LAZY_FALSE 'missing negative lazy prereqs will skip' '
+ havefalse=no
+'
+
+if test "$nothavefalse$havefalse" != yesyes
+then
+ say 'bug in test framework: negative lazy prerequisites do not work'
+ exit 1
+fi
+
clean=no
test_expect_success 'tests clean up after themselves' '
test_when_finished clean=yes
diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh
index 8889ba5..22a4f8f 100644
--- a/t/test-lib-functions.sh
+++ b/t/test-lib-functions.sh
@@ -275,6 +275,15 @@ test_have_prereq () {
for prerequisite
do
+ case "$prerequisite" in
+ !*)
+ negative_prereq=t
+ prerequisite=${prerequisite#!}
+ ;;
+ *)
+ negative_prereq=
+ esac
+
case " $lazily_tested_prereq " in
*" $prerequisite "*)
;;
@@ -294,10 +303,20 @@ test_have_prereq () {
total_prereq=$(($total_prereq + 1))
case "$satisfied_prereq" in
*" $prerequisite "*)
+ satisfied_this_prereq=t
+ ;;
+ *)
+ satisfied_this_prereq=
+ esac
+
+ case "$satisfied_this_prereq,$negative_prereq" in
+ t,|,t)
ok_prereq=$(($ok_prereq + 1))
;;
*)
- # Keep a list of missing prerequisites
+ # Keep a list of missing prerequisites; restore
+ # the negative marker if necessary.
+ prerequisite=${negative_prereq:+!}$prerequisite
if test -z "$missing_prereq"
then
missing_prereq=$prerequisite
--
1.8.0.207.gdf2154c
^ permalink raw reply related
* [PATCHv2 0/8] loosening "sender" prompt in send-email
From: Jeff King @ 2012-11-15 0:30 UTC (permalink / raw)
To: git; +Cc: Felipe Contreras, git, Thomas Rast, Junio C Hamano,
Jonathan Nieder
In-Reply-To: <20121113164845.GD20361@sigill.intra.peff.net>
On Tue, Nov 13, 2012 at 11:48:45AM -0500, Jeff King wrote:
> [1/6]: ident: make user_ident_explicitly_given private
> [2/6]: ident: keep separate "explicit" flags for author and committer
> [3/6]: var: accept multiple variables on the command line
> [4/6]: var: provide explicit/implicit ident information
> [5/6]: Git.pm: teach "ident" to query explicitness
> [6/6]: send-email: do not prompt for explicit repo ident
Here is a re-roll of the series based on comments from Jonathan (changes
from the first version noted below):
[1/8]: test-lib: allow negation of prerequisites
[2/8]: t7502: factor out autoident prerequisite
These two are new, and allow us to use the same prereq from t7502 in
t9001.
[3/8]: ident: make user_ident_explicitly_given static
Expanded the commit message to explain history of the variable.
[4/8]: ident: keep separate "explicit" flags for author and committer
Same as before.
[5/8]: var: accept multiple variables on the command line
- Expanded tests to include "git var -l"
- Use $GIT_* variables to create test "expect" files, which should make
them less brittle.
- Fixed "git var -l foo" to print usage instead of ignoring foo (i.e.,
keeping the behavior the same).
- Update usage message to match new feature.
[6/8]: var: provide explicit/implicit ident information
Updated documentation to describe what "explicit" means. No functional
changes.
[7/8]: Git.pm: teach "ident" to query explicitness
Improved Git.pm documentation. No functional changes.
[8/8]: send-email: do not prompt for explicit repo ident
Test the implicit ident case, both when we have implicit ident and when
we don't. No changes to send-email.
-Peff
^ permalink raw reply
* Re: What's cooking in git.git (Nov 2012, #03; Tue, 13)
From: Junio C Hamano @ 2012-11-15 0:19 UTC (permalink / raw)
To: Jeff King; +Cc: Torsten Bögershausen, Mark Levedahl, git
In-Reply-To: <20121115001635.GA17370@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> On Wed, Nov 14, 2012 at 10:13:28PM +0100, Torsten Bögershausen wrote:
>
>> >>>> * ml/cygwin-mingw-headers (2012-11-12) 1 commit
>> >>>> - Update cygwin.c for new mingw-64 win32 api headers
>> >>>>
>> >>>> Make git work on newer cygwin.
>> >>>>
>> >>>> Will merge to 'next'.
>
> I'm cc-ing Junio in case he missed the discussion; my original plan had
> been to move this topic right to 'next' to get exposure from other
> cygwin people. But it seems we have already got that, and it might need
> re-rolling, so it probably makes sense to hold back until the discussion
> reaches a conclusion.
Thanks for a reminder; that is what I did.
^ permalink raw reply
* Re: Local clones aka forks disk size optimization
From: Andrew Ardill @ 2012-11-15 0:18 UTC (permalink / raw)
To: Javier Domingo; +Cc: git@vger.kernel.org
In-Reply-To: <CALZVapmO61d8yXfXXGx6Qc444ka+8n7HabuNRt0rJdE5qy_7aQ@mail.gmail.com>
On 15 November 2012 10:42, Javier Domingo <javierdo1@gmail.com> wrote:
> Hi,
>
> I have come up with this while doing some local forks for work.
> Currently, when you clone a repo using a path (not file:/// protocol)
> you get all the common objects linked.
>
> But as you work, each one will continue growing on its way, although
> they may have common objects.
>
> Is there any way to avoid this? I mean, can something be done in git,
> that it checks for (when pulling) the same objects in the other forks?
Have you seen alternates? From [1]:
> How to share objects between existing repositories?
> ---------------------------------------------------------------------------
>
> Do
>
> echo "/source/git/project/.git/objects/" > .git/objects/info/alternates
>
> and then follow it up with
>
> git repack -a -d -l
>
> where the '-l' means that it will only put local objects in the pack-file
> (strictly speaking, it will put any loose objects from the alternate tree
> too, so you'll have a fully packed archive, but it won't duplicate objects
> that are already packed in the alternate tree).
[1] https://git.wiki.kernel.org/index.php/GitFaq#How_to_share_objects_between_existing_repositories.3F
Regards,
Andrew Ardill
^ permalink raw reply
* Re: What's cooking in git.git (Nov 2012, #03; Tue, 13)
From: Jeff King @ 2012-11-15 0:16 UTC (permalink / raw)
To: Torsten Bögershausen; +Cc: Junio C Hamano, Mark Levedahl, git
In-Reply-To: <50A40978.2060504@web.de>
On Wed, Nov 14, 2012 at 10:13:28PM +0100, Torsten Bögershausen wrote:
> >>>> * ml/cygwin-mingw-headers (2012-11-12) 1 commit
> >>>> - Update cygwin.c for new mingw-64 win32 api headers
> >>>>
> >>>> Make git work on newer cygwin.
> >>>>
> >>>> Will merge to 'next'.
I'm cc-ing Junio in case he missed the discussion; my original plan had
been to move this topic right to 'next' to get exposure from other
cygwin people. But it seems we have already got that, and it might need
re-rolling, so it probably makes sense to hold back until the discussion
reaches a conclusion.
> There are a couple of things which we may want consider:
> a) the name V15_MINGW_HEADERS:
> It indicates that this is true for Version 1.5 (of what?)
> If I assume Cygwin version 1.5 , then this name is confusing.
> Even cygwin versions like 1.7.7 use the same (or similar) include files as 1.5
> A better name could be CYGWIN_USE_MINGW_HEADERS (or the like) and to revert the logic.
Regardless of flipping the logic, I agree that having CYGWIN in the name
makes a lot of sense.
> b) Autodetection:
> (Just loud thinking), running
> $grep mingw /usr/include/w32api/winsock2.h
> * This file is part of the mingw-w64 runtime package.
> #include <_mingw_unicode.h>
>
> on cygwin 1.7.17 indicates that we can use grep in the Makefile to
> autodetect the "mingw headers"
Hmm. Can we rely on the /usr/include bit, though?
I assume a test-compile would be sufficient, but currently we do not do
anything more magic than "uname" in the Makefile itself to determine
defaults. Maybe it would be better to do the detection in the configure
script? And then eventually flip the default in the Makefile once
sufficient time has passed for most people to want the new format (which
would not be necessary for people using autoconf, but would help people
who do not).
-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