* 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
* 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
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