* Re: [PATCHv2 3/8] ident: make user_ident_explicitly_given static
From: Jonathan Nieder @ 2012-11-15 7:51 UTC (permalink / raw)
To: Jeff King; +Cc: git, Felipe Contreras, Thomas Rast, Junio C Hamano
In-Reply-To: <20121115003405.GC17819@sigill.intra.peff.net>
Jeff King wrote:
> 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>
For what it's worth I liked the old commit message more. But I don't
mind either.
Hope that helps,
Jonathan
^ permalink raw reply
* Re: [PATCHv2 2/8] t7502: factor out autoident prerequisite
From: Jonathan Nieder @ 2012-11-15 7:49 UTC (permalink / raw)
To: Jeff King; +Cc: git, Felipe Contreras, Thomas Rast, Junio C Hamano
In-Reply-To: <20121115003340.GB17819@sigill.intra.peff.net>
Jeff King wrote:
> --- 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
> +'
Lazy prereq scripts run in a subshell, so this should be safe. Ack.
Thanks,
Jonathan
^ permalink raw reply
* Re: [PATCH v3 0/5] push: update remote tags only with force
From: Angelo Borsotti @ 2012-11-15 7:48 UTC (permalink / raw)
To: Junio C Hamano
Cc: Chris Rorvick, git, Drew Northup, Michael Haggerty, Philip Oakley,
Johannes Sixt, Kacper Kornet, Jeff King, Felipe Contreras
In-Reply-To: <7v8va3afrt.fsf@alter.siamese.dyndns.org>
Hi Junio,
> I am *not* convinced that the "refs/tags/ is the only special
> hierarchy whose contents should not move" is a bad limitation we
> should avoid, but if it indeed is a bad limitation, the above is one
> possible way to think about avoiding it.
What other hierarchy besides branches and tags is there? Do you have
in mind some other that should not move?
-Angelo
On 15 November 2012 01:09, Junio C Hamano <gitster@pobox.com> wrote:
> Junio C Hamano <gitster@pobox.com> writes:
>
> Addendum.
>
>> In any case, I thought this series was about users who run "push"
>> voluntarily stopping themselves from pushing updates to tags that
>> may happen to fast-forward, so if we were to go with the
>> configuration route, the suggestion would be more like
>>
>> [push]
>> updateNeedsForce = refs/tags/:refs/frotz/
>>
>> or perhaps
>>
>> [remote "origin"]
>> updateNeedsForce = refs/tags/:refs/frotz/
>>
>> if we want to configure it per-remote, to specify that you would
>> need to say "--force" to update the refs in the listed hierarchies.
>>
>> Then your patch series could become just the matter of declaring
>> that the value of push.updateNeedsForce, when unspecified, defaults
>> to "refs/tags/".
>
> The above is not a "you should do it this way" suggestion, by the
> way.
>
> I was just explaining what I meant by "it may be a good feature, but
> may not necessarily be limited to refs/tags" in my earlier message
> in a different way "... and a possible design that lifts the
> limitation may go like this".
>
> I am *not* convinced that the "refs/tags/ is the only special
> hierarchy whose contents should not move" is a bad limitation we
> should avoid, but if it indeed is a bad limitation, the above is one
> possible way to think about avoiding it.
>
> Thanks.
^ permalink raw reply
* Re: [PATCHv2 1/8] test-lib: allow negation of prerequisites
From: Jonathan Nieder @ 2012-11-15 7:46 UTC (permalink / raw)
To: Jeff King; +Cc: git, Felipe Contreras, Thomas Rast, Junio C Hamano
In-Reply-To: <20121115003325.GA17819@sigill.intra.peff.net>
Jeff King wrote:
> +test_expect_success !LAZY_TRUE 'missing lazy prereqs skip tests' '
I have a visceral nervousness when reading this code, from too much
unpleasant experience of bash's csh-style !history expansion. Luckily
bash does not treat ! specially in the '-o sh' mode used by tests.
Does this feature work when running a test explicitly using
"bash <name of test>"? That's something I do from time to time to
figure out whether a weird behavior is shell-specific.
If it works everywhere, this patch would help me conquer my fear of
exclamation points in git's tests, which would be a comfort to me and
a very good thing.
^ permalink raw reply
* Re: What's cooking in git.git (Nov 2012, #03; Tue, 13)
From: Torsten Bögershausen @ 2012-11-15 5:54 UTC (permalink / raw)
To: Jeff King; +Cc: Mark Levedahl, Torsten Bögershausen, Junio C Hamano, git
In-Reply-To: <20121115015608.GB19131@sigill.intra.peff.net>
On 15.11.12 02:56, Jeff King wrote:
> On Wed, Nov 14, 2012 at 08:50:43PM -0500, Mark Levedahl wrote:
>
>> Cygwin changed the win32api implementation, and the old is not just
>> no longer supported for the current release series, but virtually
>> impossible to even install (several new packages are now installed,
>> the old package is in the "obsolete" category, i.e., not available).
>> The older cygwin 1.5 dll + utilities can be installed afresh, so that
>> is why I set up to switch based upon dll version - the proposed
>> test(s) and configuration would be to have git maintain compatibility
>> with an unsupported Cygwin configuration. I just don't think this is
>> worth the maintenance burden, but of course I am not the maintainer,
>> just expressing my opinion.
>
> OK. I don't have a strong opinion either, as I don't know what's normal
> in the Cygwin world, and that is probably the most important thing to
> follow for the default. I got the impression that "normal" is changing
> to the new way, but Torsten's message made me wonder if were there quite
> yet (if there was some issue with upgrades versus new fresh installs).
>
> But I have no real cygwin knowledge, so I'll bow out and let you guys
> discuss.
>
My understanding:
Either use people cygwin 1.5 or they use cygwin 1.7, and in this case
the installation is updated frequently.
Peff or Junio, please go ahead with the patch.
If it turns out that we want to support cygwin installations like 1.7.7
which could be upgraded, but are not upgraded since they are
"production machines we do not dare to touch" we can still improve
the autodetection.
Thanks for the responses.
/Torsten
^ permalink raw reply
* [PATCH 1/5 v2] t/test-lib-functions.sh: allow to specify the tag name to test_commit
From: Brandon Casey @ 2012-11-15 5:49 UTC (permalink / raw)
To: kraai; +Cc: git, Brandon Casey, Brandon Casey
In-Reply-To: <20121115032005.GA20677@ftbfs.org>
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..9e2b8b8 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>.
+# <file>, <contents>, and <tag> all 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: [PATCH 1/5] t/test-lib-functions.sh: allow to specify the tag name to test_commit
From: Brandon Casey @ 2012-11-15 5:43 UTC (permalink / raw)
To: Matt Kraai; +Cc: git@vger.kernel.org, Brandon Casey
In-Reply-To: <20121115032005.GA20677@ftbfs.org>
Good eye. Thanks.
On Wed, Nov 14, 2012 at 7:20 PM, Matt Kraai <kraai@ftbfs.org> wrote:
> On Wed, Nov 14, 2012 at 05:37:50PM -0800, Brandon Casey wrote:
>> -# Both <file> and <contents> default to <message>.
>> +# Both <file> <contents> and <tag> default to <message>.
>
> I think this line would be better as
>
> # <file>, <contents>, and <tag> all default to <message>.
>
> since there's now more than two arguments that default to message.
>
> --
> Matt
^ permalink raw reply
* Re: Local clones aka forks disk size optimization
From: Sitaram Chamarty @ 2012-11-15 3:44 UTC (permalink / raw)
To: Andrew Ardill; +Cc: Javier Domingo, git@vger.kernel.org
In-Reply-To: <CAH5451=Tk=zjkYbK0720VBkAA12VRCAE_Dx8bBkoXba60ho8AA@mail.gmail.com>
On Thu, Nov 15, 2012 at 7:04 AM, Andrew Ardill <andrew.ardill@gmail.com> wrote:
> 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?
I am not sure it does. My understanding is this:
'git clone -l' saves space on the initial clone, but subsequent pushes
end up with the same objects duplicated across all the "forks"
(assuming most of the forks keep up with some canonical repo).
The alternates mechanism can give you ongoing savings (as long as you
push to the "main" repo first), but it is dangerous, in the words of
the git-clone manpage. You have to be confident no one will delete a
ref from the "main" repo and then do a gc or let it auto-gc.
He's looking for something that addresses both these issues.
As an additional idea, I suspect this is what the namespaces feature
was created for, but I am not sure, and have never played with it till
now.
Maybe someone who knows namespaces very well will chip in...
^ permalink raw reply
* Re: [PATCH 1/5] t/test-lib-functions.sh: allow to specify the tag name to test_commit
From: Matt Kraai @ 2012-11-15 3:20 UTC (permalink / raw)
To: Brandon Casey; +Cc: git, Brandon Casey
In-Reply-To: <1352943474-15573-1-git-send-email-drafnel@gmail.com>
On Wed, Nov 14, 2012 at 05:37:50PM -0800, Brandon Casey wrote:
> -# Both <file> and <contents> default to <message>.
> +# Both <file> <contents> and <tag> default to <message>.
I think this line would be better as
# <file>, <contents>, and <tag> all default to <message>.
since there's now more than two arguments that default to message.
--
Matt
^ permalink raw reply
* Re: [PATCH] Add tcsh-completion support to contrib by using git-completion.bash
From: Felipe Contreras @ 2012-11-15 2:40 UTC (permalink / raw)
To: SZEDER Gábor; +Cc: Marc Khouzam, git
In-Reply-To: <20121114001159.GA14631@goldbirke>
On Wed, Nov 14, 2012 at 1:11 AM, SZEDER Gábor <szeder@ira.uka.de> wrote:
> On Tue, Nov 13, 2012 at 07:31:45PM +0100, Felipe Contreras wrote:
>> On Mon, Nov 12, 2012 at 9:07 PM, Marc Khouzam <marc.khouzam@gmail.com> wrote:
>> > + # Call _git() or _gitk() of the bash script, based on the first
>> > + # element of the command-line
>> > + _${COMP_WORDS[0]}
>>
>> You might want to use __${COMP_WORDS[0]}_main instead.
>
> That wouldn't work. __git_main() doesn't set up the
> command-line-specific variables, but the wrapper around it does.
Yeah, but you can set those command-line-specific variables manually,
like the zsh completion wrapper does.
The problem with the _git wrapper is that it will call the
bash-specific complete command.
>> > +# Make the script executable if it is not
>> > +if ( ! -x ${__git_tcsh_completion_script} ) then
>> > + chmod u+x ${__git_tcsh_completion_script}
>> > +endif
>>
>> Why not just source it?
>
> The goal is to re-use a Bash script to do completion in tcsh. They
> are two different breeds, tcsh doesn't grok bash. So sourcing the
> completion script is not an option, but we can still run it via Bash
> and use it's results.
I see, but the tcsh script can do something like this:
bash <<\EOF
echo $BASH
\EOF
Cheers.
--
Felipe Contreras
^ permalink raw reply
* Re: [PATCH] send-email: add proper default sender
From: Felipe Contreras @ 2012-11-15 2:14 UTC (permalink / raw)
To: Jeff King; +Cc: git, Thomas Rast, Junio C Hamano, Jonathan Nieder
In-Reply-To: <20121115015052.GA19131@sigill.intra.peff.net>
On Thu, Nov 15, 2012 at 2:50 AM, Jeff King <peff@peff.net> wrote:
> On Thu, Nov 15, 2012 at 02:41:50AM +0100, Felipe Contreras wrote:
>
>> 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.
>
> The rhetoric in this statement is a good indication that there is
> nothing productive to come from our discussing it anymore.
The point is still true.
>> > 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.
>
> Well, it certainly will not go through if you do not try.
At least I wouldn't be wasting my time.
Cheers.
--
Felipe Contreras
^ permalink raw reply
* Re: [PATCHv2 8/8] send-email: do not prompt for explicit repo ident
From: Felipe Contreras @ 2012-11-15 2:08 UTC (permalink / raw)
To: Jeff King; +Cc: git, Thomas Rast, Junio C Hamano, Jonathan Nieder
In-Reply-To: <20121115003640.GH17819@sigill.intra.peff.net>
On Thu, Nov 15, 2012 at 1:36 AM, Jeff King <peff@peff.net> wrote:
> 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) {
I don't think there's any need for all that, this does the trick:
diff --git a/git-send-email.perl b/git-send-email.perl
index aea66a0..503e551 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -748,16 +748,11 @@ 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++;
}
+my $prompting = 0;
This passes all the current tests and the ones you added.
Which kind of user will get the prompt with your patch, that would
miss it with mine?
Cheers.
--
Felipe Contreras
^ permalink raw reply related
* Re: What's cooking in git.git (Nov 2012, #03; Tue, 13)
From: Jeff King @ 2012-11-15 1:56 UTC (permalink / raw)
To: Mark Levedahl; +Cc: Torsten Bögershausen, Junio C Hamano, git
In-Reply-To: <50A44A73.8020007@gmail.com>
On Wed, Nov 14, 2012 at 08:50:43PM -0500, Mark Levedahl wrote:
> Cygwin changed the win32api implementation, and the old is not just
> no longer supported for the current release series, but virtually
> impossible to even install (several new packages are now installed,
> the old package is in the "obsolete" category, i.e., not available).
> The older cygwin 1.5 dll + utilities can be installed afresh, so that
> is why I set up to switch based upon dll version - the proposed
> test(s) and configuration would be to have git maintain compatibility
> with an unsupported Cygwin configuration. I just don't think this is
> worth the maintenance burden, but of course I am not the maintainer,
> just expressing my opinion.
OK. I don't have a strong opinion either, as I don't know what's normal
in the Cygwin world, and that is probably the most important thing to
follow for the default. I got the impression that "normal" is changing
to the new way, but Torsten's message made me wonder if were there quite
yet (if there was some issue with upgrades versus new fresh installs).
But I have no real cygwin knowledge, so I'll bow out and let you guys
discuss.
-Peff
^ permalink raw reply
* Re: What's cooking in git.git (Nov 2012, #03; Tue, 13)
From: Mark Levedahl @ 2012-11-15 1:50 UTC (permalink / raw)
To: Jeff King; +Cc: Torsten Bögershausen, Junio C Hamano, git
In-Reply-To: <20121115001635.GA17370@sigill.intra.peff.net>
On 11/14/2012 07:16 PM, Jeff King wrote:
> On Wed, Nov 14, 2012 at 10:13:28PM +0100, Torsten Bögershausen wrote:
> 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
>
Cygwin changed the win32api implementation, and the old is not just no
longer supported for the current release series, but virtually
impossible to even install (several new packages are now installed, the
old package is in the "obsolete" category, i.e., not available). The
older cygwin 1.5 dll + utilities can be installed afresh, so that is why
I set up to switch based upon dll version - the proposed test(s) and
configuration would be to have git maintain compatibility with an
unsupported Cygwin configuration. I just don't think this is worth the
maintenance burden, but of course I am not the maintainer, just
expressing my opinion.
I have no trouble renaming the macro to whatever seems to clarify things.
Mark
^ permalink raw reply
* Re: [PATCH] send-email: add proper default sender
From: Jeff King @ 2012-11-15 1:50 UTC (permalink / raw)
To: Felipe Contreras; +Cc: git, Thomas Rast, Junio C Hamano, Jonathan Nieder
In-Reply-To: <CAMP44s3vYP6zR6sKxgVSoZUySxcAuR7MioMeT1WDAnU0PtZKGA@mail.gmail.com>
On Thu, Nov 15, 2012 at 02:41:50AM +0100, Felipe Contreras wrote:
> 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.
The rhetoric in this statement is a good indication that there is
nothing productive to come from our discussing it anymore.
> > 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.
Well, it certainly will not go through if you do not try.
-Peff
^ permalink raw reply
* [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
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