* Re: Fwd: New Defects reported by Coverity Scan for git
From: Junio C Hamano @ 2016-10-20 18:05 UTC (permalink / raw)
To: Stefan Beller; +Cc: git@vger.kernel.org, Jeff King
In-Reply-To: <CAGZ79kYTwsWy+oxnPKghWKGLEaO9qWZbrQOOejeEBOe0c-o0Pg@mail.gmail.com>
Stefan Beller <sbeller@google.com> writes:
> I do it most of the time, but I did not start managing it.
> And I have been pretty lax/liberal about handing out rights to do stuff,
> because I did not want to tip on anyone's toes giving to few rights
> and thereby annoying them.
Good to know that you have been managing it; I was mostly worried
about not having anybody managing it (i.e. imagining Coverity
nominated/volunteered me as manager with everybody else as viewers)
and the new viewer requests get totally ignored by the project as
the whole.
> I see that some of these emails may be inconvenient to you, I can
> change your role to defect viewer/contributor if you prefer.
It is not a huge inconvenience to me, because any piece of e-mail
that is addressed directly to gitster@ without CC'ing to git@vger
and is not a follow-up to any earlier message goes to a separate
lowest-priority mailbox that I rarely look at. But if it is easy
to recategorize me, please do so.
Thanks.
^ permalink raw reply
* Re: [PATCH] rev-list: restore the NUL commit separator in --header mode
From: Dennis Kaarsemaker @ 2016-10-20 18:04 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, jacob.e.keller, stefanbeller, peff, j6t, jacob.keller
In-Reply-To: <xmqq4m48j70o.fsf@gitster.mtv.corp.google.com>
On Wed, 2016-10-19 at 15:41 -0700, Junio C Hamano wrote:
> Dennis Kaarsemaker <dennis@kaarsemaker.net> writes:
>
> > + touch expect &&
> > + printf "\0" > expect &&
>
>
> What's the point of that "touch", especially if you are going to
> overwrite it immediately after?
Leftover debugging crud. I tried various ways of generating an
actual/expect to compare.
> > + git rev-list --header --max-count=1 HEAD | tail -n1 >actual &&
>
>
> As "tail" is a tool for text files, it is likely unportable to use
> "tail -n1" to grab the "last incomplete line that happens to contain
> a single NUL".
>
> > + test_cmp_bin expect actual
> > +'
Yeah, I was fearing that. I didn't find anything in the testsuite that
helps answering the question "does this file end with a NUL" and would
appreciate a hint :)
D.
^ permalink raw reply
* Re: [PATCH] rev-list: restore the NUL commit separator in --header mode
From: Dennis Kaarsemaker @ 2016-10-20 18:02 UTC (permalink / raw)
To: Junio C Hamano
Cc: git@vger.kernel.org, j6t@kdbg.org, peff@peff.net,
stefanbeller@gmail.com
In-Reply-To: <xmqq8ttkj740.fsf@gitster.mtv.corp.google.com>
On Wed, 2016-10-19 at 15:39 -0700, Junio C Hamano wrote:
> Jacob Keller <jacob.keller@gmail.com> writes:
>
> > Hi,
> >
> > On Wed, Oct 19, 2016 at 2:04 PM, Dennis Kaarsemaker
> > <dennis@kaarsemaker.net> wrote:
> > > Commit 660e113 (graph: add support for --line-prefix on all graph-aware
> > > output) changed the way commits were shown. Unfortunately this dropped
> > > the NUL between commits in --header mode. Restore the NUL and add a test
> > > for this feature.
> > >
> >
> >
> > Oops! Thanks for the bug fix.
> >
> > > Signed-off-by: Dennis Kaarsemaker <dennis@kaarsemaker.net>
> > > ---
> > > builtin/rev-list.c | 4 ++++
> > > t/t6000-rev-list-misc.sh | 7 +++++++
> > > 2 files changed, 11 insertions(+)
> > >
> > > diff --git a/builtin/rev-list.c b/builtin/rev-list.c
> > > index 8479f6e..cfa6a7d 100644
> > > --- a/builtin/rev-list.c
> > > +++ b/builtin/rev-list.c
> > > @@ -157,6 +157,10 @@ static void show_commit(struct commit *commit, void *data)
> > > if (revs->commit_format == CMIT_FMT_ONELINE)
> > > putchar('\n');
> > > }
> > > + if (revs->commit_format == CMIT_FMT_RAW) {
> > > + putchar(info->hdr_termination);
> > > + }
> > > +
> >
> >
> > This seems right to me. My one concern is that we make sure we restore
> > it for every case (in case it needs to be there for other formats?)
> > I'm not entirely sure about whether other non-raw modes need this or
> > not?
>
>
> Right. The original didn't do anything special for CMIT_FMT_RAW,
> and 660e113 did not remove anything special for CMIT_FMT_RAW, so it
> isn't immediately obvious why this patch is sufficient.
>
> Dennis, care to elaborate?
The original logic was (best seen with git show -w 660e113):
if(showing graphs) {
do pretty things
}
else {
just print the buffer and the header terminator
}
660e113 changed that to
do pretty things
Given that the 'do pretty things part' works for other uses of git rev-
list, it made sense that the \0 should only be added back in
CMIT_FMT_RAW mode. Changing the first putchar('\n') as Jacob proposes
(that mail arrived while I'm typing this) might work too, I haven't
tested it.
D.
^ permalink raw reply
* Re: Fwd: New Defects reported by Coverity Scan for git
From: Stefan Beller @ 2016-10-20 17:58 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git@vger.kernel.org, Jeff King
In-Reply-To: <xmqqshrqhpua.fsf@gitster.mtv.corp.google.com>
On Thu, Oct 20, 2016 at 10:50 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Stefan Beller <sbeller@google.com> writes:
>
>> Not sure what triggered the new finding of coverity as seen below as the
>> parse_commit() was not touched. Junios series regarding the merge base
>> optimization touches a bit of code nearby though.
>>
>> Do we want to replace the unchecked places of parse_commit with
>> parse_commit_or_die ?
>
> The reason parse_commit() would fail at this point would be because
> the repository is corrupt, I do not think it would hurt to do such a
> change.
>
> I agree that it is curious why it shows up as a "new defect",
> though.
>
> By the way, do you know who is managing the service on our end
> (e.g. approving new people to be "defect viewer")?
I do it most of the time, but I did not start managing it.
And I have been pretty lax/liberal about handing out rights to do stuff,
because I did not want to tip on anyone's toes giving to few rights
and thereby annoying them.
I see that some of these emails may be inconvenient to you, I can
change your role to defect viewer/contributor if you prefer.
Thanks,
Stefan
^ permalink raw reply
* Re: Fwd: New Defects reported by Coverity Scan for git
From: Junio C Hamano @ 2016-10-20 17:50 UTC (permalink / raw)
To: Stefan Beller; +Cc: git@vger.kernel.org, Jeff King
In-Reply-To: <CAGZ79kabVPhp0_z-e_4jJOFq+jzSE2SsgmFuY-2RUgrEviGKyA@mail.gmail.com>
Stefan Beller <sbeller@google.com> writes:
> Not sure what triggered the new finding of coverity as seen below as the
> parse_commit() was not touched. Junios series regarding the merge base
> optimization touches a bit of code nearby though.
>
> Do we want to replace the unchecked places of parse_commit with
> parse_commit_or_die ?
The reason parse_commit() would fail at this point would be because
the repository is corrupt, I do not think it would hurt to do such a
change.
I agree that it is curious why it shows up as a "new defect",
though.
By the way, do you know who is managing the service on our end
(e.g. approving new people to be "defect viewer")? The site seems
to think I have the power to manage others' subscription, which I do
not think I have (I do not go to the site myself). As it spewed
quite a many false positives into my mailbox in the past, I do not
pay very close attention to these reports these days, but I still
read the e-mailed reports every once in a while.
Thanks.
^ permalink raw reply
* Re: tools for easily "uncommitting" parts of a patch I just commited?
From: Junio C Hamano @ 2016-10-20 17:39 UTC (permalink / raw)
To: Jacob Keller; +Cc: Jeff King, Git mailing list
In-Reply-To: <CA+P7+xq25LcdmtzmBNChiGhGratcdp7m0EOsQuEh68=gJQ9HNQ@mail.gmail.com>
Jacob Keller <jacob.keller@gmail.com> writes:
> I still think we're misunderstanding. I want git commit to complain
> *only* under the following circumstance:
>
> I run "git add -p" and put a partial change into the index in <file>.
> There are still other parts which were not added to the index yet.
> Thus, the index version of the file and the actual file differ.
>
> Then, I (accidentally) run "git commit <file>"
I agree that this case is different.
Again, users are different, and I also often do
$ edit file; think; decide it is a good enough first cut
$ git add file
$ edit file; think; decide it is getting better
$ git add file
$ edit file; think; decide it is now perfect
$ git commit file
Because I do not think you can differentiate the above workflow from
the case where "git add -p" was used earlier, I think your updated
"git commit" needs to complain at this point.
I am not sure if that is OK. I think it is less not-OK than the use
case I mentioned in my earlier message, in that this is not a case
that "please don't do it" breaks. It however is an inconvenience
that the user has to say "git add file" before the "git commit" (or
"git commit file") to conclude the sequence.
So I dunno.
^ permalink raw reply
* Re: tools for easily "uncommitting" parts of a patch I just commited?
From: Jacob Keller @ 2016-10-20 17:27 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Jeff King, Git mailing list
In-Reply-To: <xmqqa8dzhtki.fsf@gitster.mtv.corp.google.com>
On Thu, Oct 20, 2016 at 9:30 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Jeff King <peff@peff.net> writes:
>
>>> I still think it's worth while to add a check for git-commit which
>>> does something like check when we say "git commit <files>" and if the
>>> index already has those files marked as being changed, compare them
>>> with the current contents of the file as in the checkout and quick
>>> saying "please don't do that" so as to avoid the problem in the first
>>> place.
>> ...
>> I suspect both of those would complain about legitimate workflows.
>>
>> I dunno. I do not ever use "git commit <file>" myself.
>
> Users are different. I do use this all the time, and it is not
> unusual at all to have changed contents on paths other than <file>
> already added to the index when I do so, i.e. an unrelated small
> typofix in <file> jumping ahead of the real changes I am working on
> in other parts of the tree.
>
> "Please don't do that" would break. Jacob says "avoid the problem",
> but I do not see a problem in allowing it (it could be that the
> problem Jacob has is in other parts of his workflow, but I do not
> know what it is offhand).
I still think we're misunderstanding. I want git commit to complain
*only* under the following circumstance:
I run "git add -p" and put a partial change into the index in <file>.
There are still other parts which were not added to the index yet.
Thus, the index version of the file and the actual file differ.
Then, I (accidentally) run "git commit <file>"
I want git commit to complain here that the index <file> and acutal
<file> being requested are different and it thinks there's an issue.
I do *NOT* want it to complain if I do "git add -p" and put parts of
<other-file> into the index, and then run
git commit <file>
Does that make sense?
Basically if the index and "git commit <file>" both say "add <file>"
but they conflict in what version of <file> I want it to go "hey..
uhhh.. that's a bad idea"
Thanks,
Jake
^ permalink raw reply
* Re: Drastic jump in the time required for the test suite
From: Matthieu Moy @ 2016-10-20 17:13 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Johannes Schindelin, git
In-Reply-To: <xmqqmvhzhu6f.fsf@gitster.mtv.corp.google.com>
Junio C Hamano <gitster@pobox.com> writes:
> Are you proposing to replace the tests written as shell scripts with
> scripts in another language or framework that run equivalent
> sequences of git commands that is as portable as, if not more,
> Bourne shell?
The language (/bin/sh) is probably not the biggest issue. The way we use
it may be.
I don't have benchmark to tell what slows down the testsuite, but for
example we often write
cat >expected <<EOF &&
content
EOF
git foo >actual &&
test_cmp expected actual
We create 2 files, fork one 'cat' and one 'diff'. Now, suppose we can
write instead
test_check_output git foo <<EOF
content
EOF
where test_check_output would be a compiled program running "git foo"
with popen(), reading the expected output on stdin, and comparing both
(and possibly creating files and forking 'diff', but only on the error
path). That would reduce the overhead of test a lot: on fork+exec to
create the "test_check_output" process + plain string comparison.
Obviously, I don't expect this change to drastically reduce the time
(Peff's numbers seem to suggest that there's not so much to gain), but
maybe getting a few C helpers and "optimized" idioms in our testsuite
could lead to measurable improvements.
--
Matthieu Moy
http://www-verimag.imag.fr/~moy/
^ permalink raw reply
* Re: [PATCH v4 05/14] i18n: add--interactive: mark plural strings
From: Junio C Hamano @ 2016-10-20 17:11 UTC (permalink / raw)
To: Vasco Almeida
Cc: git, Jiang Xin, Ævar Arnfjörð Bjarmason,
Jean-Noël AVILA, Jakub Narębski, David Aguilar
In-Reply-To: <1476981335.14459.19.camel@sapo.pt>
Vasco Almeida <vascomalmeida@sapo.pt> writes:
> A Seg, 10-10-2016 às 12:54 +0000, Vasco Almeida escreveu:
>> @@ -70,6 +72,8 @@ Git::I18N - Perl interface to Git's Gettext localizations
>>
>> printf __("The following error occurred: %s\n"), $error;
>>
>> + printf __n("commited %d file", "commited %d files", $files), $files;
>> +
>
> I forgot to add \n to this example as suggested in
> <xmqqoa2ymnb1.fsf@gitster.mtv.corp.google.com>
>
> What should I do? Should I wait for more reviews and then send a new
> re-roll fixing this?
You fix it up locally not to forget, in case you need a reroll, and
wait for more reviews. In the meantime, I'll also fix it up locally
not to forget ;-) That way, if it turns out that this round is good
enough to be the final version, people will see my fixup, and if what
I have needs to be replaced with your new version, your fixup will
be in there.
Thanks.
^ permalink raw reply
* Fwd: New Defects reported by Coverity Scan for git
From: Stefan Beller @ 2016-10-20 17:05 UTC (permalink / raw)
To: git@vger.kernel.org, Junio C Hamano, Jeff King
In-Reply-To: <580893d5a4736_4ed37b53181837@ss1435.mail>
Not sure what triggered the new finding of coverity as seen below as the
parse_commit() was not touched. Junios series regarding the merge base
optimization touches a bit of code nearby though.
Do we want to replace the unchecked places of parse_commit with
parse_commit_or_die ?
Thanks,
Stefan
_________________________________________________________
*** CID 1374088: Error handling issues (CHECKED_RETURN)
/commit.c: 913 in mark_redundant()
907
908 work = xcalloc(cnt, sizeof(*work));
909 redundant = xcalloc(cnt, 1);
910 ALLOC_ARRAY(filled_index, cnt - 1);
911
912 for (i = 0; i < cnt; i++)
>>> CID 1374088: Error handling issues (CHECKED_RETURN)
>>> Calling "parse_commit" without checking return value (as is done elsewhere 37 out of 45 times).
913 parse_commit(array[i]);
914 for (i = 0; i < cnt; i++) {
915 struct commit_list *common;
916
917 if (redundant[i])
918 continue;
^ permalink raw reply
* Re: [PATCH 2/2] tag: send fully qualified refnames to verify_tag_and_format
From: Santiago Torres @ 2016-10-20 16:57 UTC (permalink / raw)
To: Jeff King; +Cc: git, gitster, sunshine, walters, Lukas Puehringer
In-Reply-To: <20161019203943.epjxnfci7vcqg4xv@sigill.intra.peff.net>
[-- Attachment #1: Type: text/plain, Size: 1762 bytes --]
On Wed, Oct 19, 2016 at 04:39:44PM -0400, Jeff King wrote:
> The ref-filter code generally expects to see fully qualified
> refs, so that things like "%(refname)" and "%(refname:short)"
> work as expected. We can do so easily from git-tag, which
> always works with refnames in the refs/tags namespace. As a
> bonus, we can drop the "kind" parameter from
> pretty_print_ref() and just deduce it automatically.
>
> Unfortunately, things are not so simple for verify-tag,
> which takes an arbitrary sha1 expression. It has no clue if
> a refname as used or not, and whether it was in the
> refs/tags namespace.
>
> In an ideal world, get_sha1_with_context() would optionally
> tell us about any refs we resolved while it was working, and
> we could just feed that refname (and then in cases where we
> didn't use a ref at all, like a bare sha1, we could fallback
> to just showing the sha1 name the user gave us).
>
> Signed-off-by: Jeff King <peff@peff.net>
> ---
> I think you'd really just squash the various bits of this into your
> series at the right spots, though I don't mind it on top, either.
>
> The big question is to what degree we should care about the verify-tag
> case. I don't think it's any worse off with this change than it is with
> your series (its "kind" becomes "OTHER", but I don't think that is
> actually used for display at all; the name remains the same). I'd be OK
> with leaving it like this, as a known bug, until get_sha1_with_context()
> learns to tell us about the ref. It's an unhandled corner case in a
> brand-new feature, not a regression in an existing one.
I see now, I think I can sprinkle some of these changes on 2/7 then. The
rest should be doing 4/7 and 5/7. Does this sound ok?
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]
^ permalink raw reply
* Re: Drastic jump in the time required for the test suite
From: René Scharfe @ 2016-10-20 16:40 UTC (permalink / raw)
To: Duy Nguyen, Johannes Schindelin; +Cc: Junio C Hamano, Git Mailing List
In-Reply-To: <CACsJy8ALRjnvgM6sW5ioFHtquSR6zgQHkQ6-LUnKFnqXV9eDCQ@mail.gmail.com>
Am 20.10.2016 um 13:02 schrieb Duy Nguyen:
> On Wed, Oct 19, 2016 at 4:18 PM, Johannes Schindelin
> <Johannes.Schindelin@gmx.de> wrote:
>> Hi Junio,
>>
>> I know you are a fan of testing things thoroughly in the test suite, but I
>> have to say that it is getting out of hand, in particular due to our
>> over-use of shell script idioms (which really only run fast on Linux, not
>> a good idea for a portable software).
>>
>> My builds of `pu` now time out, after running for 3h straight in the VM
>> dedicated to perform the daily routine of building and testing the git.git
>> branches in Git for Windows' SDK. For comparison, `next` passes build &
>> tests in 2.6h. That is quite the jump.
>
> I'm just curious, will running git.exe from WSL [1] help speed things
> up a bit (or, hopefully, a lot)? I'm assuming that shell's speed in
> WSL is quite fast.
>
> I'm pretty sure the test suite would need some adaptation, but if the
> speedup is significant, maybe it's worth spending time on.
>
> [1] https://news.ycombinator.com/item?id=12748395
I get this on WSL with prove -j8:
Files=750, Tests=13657, 906 wallclock secs ( 8.51 usr 17.17 sys + 282.62 cusr 3731.85 csys = 4040.15 CPU)
And this for a run on Debian inside a Hyper-V VM on the same system:
Files=759, Tests=13895, 99 wallclock secs ( 4.81 usr 1.06 sys + 39.70 cusr 25.82 csys = 71.39 CPU)
All tests pass on master.
René
^ permalink raw reply
* Re: [PATCH v4 05/14] i18n: add--interactive: mark plural strings
From: Vasco Almeida @ 2016-10-20 16:35 UTC (permalink / raw)
To: git
Cc: Jiang Xin, Ævar Arnfjörð Bjarmason,
Jean-Noël AVILA, Jakub Narębski, David Aguilar,
Junio C Hamano
In-Reply-To: <20161010125449.7929-6-vascomalmeida@sapo.pt>
A Seg, 10-10-2016 às 12:54 +0000, Vasco Almeida escreveu:
> @@ -70,6 +72,8 @@ Git::I18N - Perl interface to Git's Gettext localizations
>
> printf __("The following error occurred: %s\n"), $error;
>
> + printf __n("commited %d file", "commited %d files", $files), $files;
> +
I forgot to add \n to this example as suggested in
<xmqqoa2ymnb1.fsf@gitster.mtv.corp.google.com>
What should I do? Should I wait for more reviews and then send a new
re-roll fixing this?
^ permalink raw reply
* Re: Drastic jump in the time required for the test suite
From: Stefan Beller @ 2016-10-20 16:30 UTC (permalink / raw)
To: Jeff King; +Cc: Johannes Schindelin, Junio C Hamano, git@vger.kernel.org
In-Reply-To: <20161020123111.qnbsainul2g54z4z@sigill.intra.peff.net>
On Thu, Oct 20, 2016 at 5:31 AM, Jeff King <peff@peff.net> wrote:
>
> $ perl -lne '/execve\("(.*?)"/ and print $1' /tmp/foo.out | sort | uniq -c | sort -rn | head
> 152271 /home/peff/compile/git/git
> 57340 /home/peff/compile/git/t/../bin-wrappers/git
> 16865 /bin/sed
> 12650 /bin/rm
> 11257 /bin/cat
> 9326 /home/peff/compile/git/git-sh-i18n--envsubst
> 9079 /usr/bin/diff
> 8013 /usr/bin/wc
> 5924 /bin/mv
> 4566 /bin/grep
>
I am not an expert on perl nor tracing, but is it feasible to find out
how many internal calls there are? i.e. either some shell script (rebase,
submodule) calling git itself a couple of times or even from compile/git/git
itself, e.g. some submodule operations use forking in there.
^ permalink raw reply
* Re: tools for easily "uncommitting" parts of a patch I just commited?
From: Junio C Hamano @ 2016-10-20 16:30 UTC (permalink / raw)
To: Jeff King; +Cc: Jacob Keller, Git mailing list
In-Reply-To: <20161020021323.tav5glu7xy4u7mtj@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
>> I still think it's worth while to add a check for git-commit which
>> does something like check when we say "git commit <files>" and if the
>> index already has those files marked as being changed, compare them
>> with the current contents of the file as in the checkout and quick
>> saying "please don't do that" so as to avoid the problem in the first
>> place.
> ...
> I suspect both of those would complain about legitimate workflows.
>
> I dunno. I do not ever use "git commit <file>" myself.
Users are different. I do use this all the time, and it is not
unusual at all to have changed contents on paths other than <file>
already added to the index when I do so, i.e. an unrelated small
typofix in <file> jumping ahead of the real changes I am working on
in other parts of the tree.
"Please don't do that" would break. Jacob says "avoid the problem",
but I do not see a problem in allowing it (it could be that the
problem Jacob has is in other parts of his workflow, but I do not
know what it is offhand).
^ permalink raw reply
* Re: [PATCH v4 05/14] i18n: add--interactive: mark plural strings
From: Vasco Almeida @ 2016-10-20 16:29 UTC (permalink / raw)
To: Junio C Hamano
Cc: git, Jiang Xin, Ævar Arnfjörð Bjarmason,
Jean-Noël AVILA, Jakub Narębski, David Aguilar
In-Reply-To: <xmqqtwc8kwqz.fsf@gitster.mtv.corp.google.com>
A Qua, 19-10-2016 às 11:40 -0700, Junio C Hamano escreveu:
> Vasco Almeida <vascomalmeida@sapo.pt> writes:
>
> >
> > @@ -669,12 +669,18 @@ sub status_cmd {
> > sub say_n_paths {
> > my $did = shift @_;
> > my $cnt = scalar @_;
> > - print "$did ";
> > - if (1 < $cnt) {
> > - print "$cnt paths\n";
> > - }
> > - else {
> > - print "one path\n";
> > + if ($did eq 'added') {
> > + printf(__n("added %d path\n", "added %d paths\n",
> > + $cnt), $cnt);
> > + } elsif ($did eq 'updated') {
> > + printf(__n("updated %d path\n", "updated %d
> > paths\n",
> > + $cnt), $cnt);
> > + } elsif ($did eq 'reverted') {
> > + printf(__n("reverted %d path\n", "reverted %d
> > paths\n",
> > + $cnt), $cnt);
> > + } else {
> > + printf(__n("touched %d path\n", "touched %d
> > paths\n",
> > + $cnt), $cnt);
> > }
> > }
>
> Nice to see you covered all verbs currently in use and then
> future-proofed by adding a fallback "touched" here.
>
> Thanks.
>
Thanks. Here I added %d to the singular sentences "added %d path\n" to
avoid a Perl warning about a redundant argument in printf.
^ permalink raw reply
* Re: [PATCH v4 01/14] i18n: add--interactive: mark strings for translation
From: Junio C Hamano @ 2016-10-20 16:25 UTC (permalink / raw)
To: Vasco Almeida
Cc: git, Jiang Xin, Ævar Arnfjörð Bjarmason,
Jean-Noël AVILA, Jakub Narębski, David Aguilar
In-Reply-To: <1476980461.14459.10.camel@sapo.pt>
Vasco Almeida <vascomalmeida@sapo.pt> writes:
>> Not a big deal, but this makes me wonder if we want to do this
>> instead ...
For future reference (for others as well), when I say "makes me
wonder" or "I wonder", I am never demanding to change what the
original author wrote. I just am trying to see that pros-and-cons
have been considered already.
> ... Also I think msgfmt checks if English source and translation
> both end with newline or not.
That is a good enough safety belt to me.
> I will leave this patch as is.
Yup. Thanks.
^ permalink raw reply
* Re: [regression] `make profile-install` fails in 2.10.1
From: Junio C Hamano @ 2016-10-20 16:23 UTC (permalink / raw)
To: Jeff King; +Cc: Jan Keromnes, git
In-Reply-To: <20161019223840.j3lvfswazuozxkjo@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> I usually just try to recreate the actual environment (e.g., run the
> tests as root, run them on a loopback case-insensitive fs, etc) as that
> gives a more realistic recreation.
True. I just do not want to run the tests as root myself ;-) I
wonder if fakeroot would give us good enough illusion for that--I
haven't used it for a long while.
^ permalink raw reply
* Re: [PATCH v4 01/14] i18n: add--interactive: mark strings for translation
From: Vasco Almeida @ 2016-10-20 16:21 UTC (permalink / raw)
To: Junio C Hamano
Cc: git, Jiang Xin, Ævar Arnfjörð Bjarmason,
Jean-Noël AVILA, Jakub Narębski, David Aguilar
In-Reply-To: <xmqqy41kkxzd.fsf@gitster.mtv.corp.google.com>
A Qua, 19-10-2016 às 11:14 -0700, Junio C Hamano escreveu:
> Vasco Almeida <vascomalmeida@sapo.pt> writes:
>
> >
> > } else {
> > - print "No untracked files.\n";
> > + print __("No untracked files.\n");
> > }
>
> Not a big deal, but this makes me wonder if we want to do this
> instead
>
> print __("No untracked files.") . "\n";
>
> so that translators do not have to remember to keep the final LF.
This can be a good idea. On the other hand, I think translators are
cautious to not forget the final LF since there is a lot of them from C
source. Also I think msgfmt checks if English source and translation
both end with newline or not. So if a translator forgets to put a \n
then msgfmt would return an error. If it is not the translator to find
the error herself, someone else will, like the Translation coordinator.
I will leave this patch as is.
https://www.gnu.org/software/gettext/FAQ.html#newline
^ permalink raw reply
* Re: Drastic jump in the time required for the test suite
From: Junio C Hamano @ 2016-10-20 16:16 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: git
In-Reply-To: <alpine.DEB.2.20.1610201154070.3264@virtualbox>
Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> Of course, if you continue to resist (because the problem is obviously not
> affecting you personally, so why would you care), I won't even try to find
> the time to start on that project.
Sorry, but I did not know I was resisting, as I didn't see any
proposal to resist against in the first place. I was trying to help
by mentioning two tricks that may be helping my test runtime that
may help you as well.
Are you proposing to replace the tests written as shell scripts with
scripts in another language or framework that run equivalent
sequences of git commands that is as portable as, if not more,
Bourne shell? If that is what you are proposing, well, I won't stop
you and I may even help you in there, but I fail to guess what
alternative you have in mind. I certainly do not have a suggestion
myself and I won't suggest migrate to tclsh or perl for that matter.
If that is not what you are trying to propose, and if parallelism
has already been employed, then there may or may not be other tricks
you are not yet using that helps to speed up your shell execution
that others are using---being confrontational is not an effective
way to ask others about them.
^ permalink raw reply
* Re: [PATCH] rev-list: restore the NUL commit separator in --header mode
From: Keller, Jacob E @ 2016-10-20 16:07 UTC (permalink / raw)
To: gitster@pobox.com, jacob.keller@gmail.com
Cc: git@vger.kernel.org, j6t@kdbg.org, peff@peff.net,
stefanbeller@gmail.com, dennis@kaarsemaker.net
In-Reply-To: <xmqq8ttkj740.fsf@gitster.mtv.corp.google.com>
On Wed, 2016-10-19 at 15:39 -0700, Junio C Hamano wrote:
> Jacob Keller <jacob.keller@gmail.com> writes:
>
> >
> > Hi,
> >
> > On Wed, Oct 19, 2016 at 2:04 PM, Dennis Kaarsemaker
> > <dennis@kaarsemaker.net> wrote:
> > >
> > > Commit 660e113 (graph: add support for --line-prefix on all
> > > graph-aware
> > > output) changed the way commits were shown. Unfortunately this
> > > dropped
> > > the NUL between commits in --header mode. Restore the NUL and add
> > > a test
> > > for this feature.
> > >
> >
> > Oops! Thanks for the bug fix.
> >
> > >
> > > Signed-off-by: Dennis Kaarsemaker <dennis@kaarsemaker.net>
> > > ---
> > > builtin/rev-list.c | 4 ++++
> > > t/t6000-rev-list-misc.sh | 7 +++++++
> > > 2 files changed, 11 insertions(+)
> > >
> > > diff --git a/builtin/rev-list.c b/builtin/rev-list.c
> > > index 8479f6e..cfa6a7d 100644
> > > --- a/builtin/rev-list.c
> > > +++ b/builtin/rev-list.c
> > > @@ -157,6 +157,10 @@ static void show_commit(struct commit
> > > *commit, void *data)
> > > if (revs->commit_format ==
> > > CMIT_FMT_ONELINE)
> > > putchar('\n');
> > > }
> > > + if (revs->commit_format == CMIT_FMT_RAW) {
> > > + putchar(info->hdr_termination);
> > > + }
> > > +
> >
> > This seems right to me. My one concern is that we make sure we
> > restore
> > it for every case (in case it needs to be there for other formats?)
> > I'm not entirely sure about whether other non-raw modes need this
> > or
> > not?
>
> Right. The original didn't do anything special for CMIT_FMT_RAW,
> and 660e113 did not remove anything special for CMIT_FMT_RAW, so it
> isn't immediately obvious why this patch is sufficient.
>
> Dennis, care to elaborate?
I believe all we need to do is change one of the places where we emit
"\n" with emiting info->hdr_termination instead.
I'm looking at the original code now.
Thanks,
Jake
^ permalink raw reply
* Re: How to rename remote branches if I only have "client access"?
From: Junio C Hamano @ 2016-10-20 15:55 UTC (permalink / raw)
To: Manuel Reimer; +Cc: git
In-Reply-To: <nuamc4$uvk$1@blaine.gmane.org>
Manuel Reimer <Manuel.Spam@nurfuerspam.de> writes:
> I have the following branches on my remote repo:
>
> new_version
> master
>
> I now want the "new_version" branch to be the "master" and the old
> "master" has to be renamed to the old version number (in my case
> 0.2.0).
>
> How to do this?
I assume you have "git push" access into this remote repository.
You are correct that there is no way for you to tell "git push" (or
any Git native method) to rename "master" to "maint-0.2.0".
What you can do is to push the commits at the tip of "master" and
"new_version" to "maint-0.2.0" and "master" respectively, and then
delete "new_version". If the 'master' and 'new_version' you locally
have are already the 'master' and 'new_version' you have over there
that you want to see sit at the tips of updated 'maint-0.2.0' and
'master' branches, then:
git push $remote master:refs/heads/maint-0.2.0 new_version:master
git push $remote :new_version
would do this.
Note that in the <source>:<dest> syntax in "git push", the <source>
side refers to the names in your local repository; if your local
'master' is different from what you want to see at the tip of
'maint-0.2.0' at the remote after this, replace it with whatever
name you give to that commit locally in the above example.
Also note that the this push may not succeed if your new_version is
not a descendant of the current 'master' at the remote. You'd need
to use +<source>:<dest> to force it if that is the case.
The second command that has an empty <source> is to delete <dest>.
Lastly, the remote side can be configured to forbid deletion of
branches, and/or to forbid forced pushes. If your remote is
configured that way (which is not default), then there is no way for
you to do any of the above (and that is by design---the server
operators use these configuration variables to forbid you from doing
something, so you shouldn't be able to override that choice).
> Currently this causes me much trouble as I can't
> delete the remote "master" repository as this is the "remote current
> repository"...
Sorry, but you lost me here. You were talking about two branches in
a single repository that is remote earlier. I do not know what this
"remote master repository" you bring up in this paragraph is, and
why you can't remove it. Not that I want to know the answers to
these questions myself. I just do not understand these as a reason
behind your wanting to rename branches at a remote repository.
^ permalink raw reply
* Re: [PATCH] doc: remove reference to the traditional layout in git-tag.txt
From: Junio C Hamano @ 2016-10-20 15:34 UTC (permalink / raw)
To: Younes Khoudli; +Cc: git
In-Reply-To: <20161020132144.6018-1-younes.khoudli@gmail.com>
Younes Khoudli <younes.khoudli@gmail.com> writes:
> This is the only place in the documentation that the traditional layout
> is mentioned, and it is confusing. Remove it.
Yeah, the information is not incorrect per-se, but certainly is out
of place and immaterial to what this part of the documentation tries
to teach.
Will queue; thanks.
>
> * Documentation/git-tag.txt: Here.
>
> Signed-off-by: Younes Khoudli <younes.khoudli@gmail.com>
> ---
> Documentation/git-tag.txt | 5 ++---
> 1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/Documentation/git-tag.txt b/Documentation/git-tag.txt
> index 7ecca8e..80019c5 100644
> --- a/Documentation/git-tag.txt
> +++ b/Documentation/git-tag.txt
> @@ -253,9 +253,8 @@ On Automatic following
> ~~~~~~~~~~~~~~~~~~~~~~
>
> If you are following somebody else's tree, you are most likely
> -using remote-tracking branches (`refs/heads/origin` in traditional
> -layout, or `refs/remotes/origin/master` in the separate-remote
> -layout). You usually want the tags from the other end.
> +using remote-tracking branches (eg. `refs/remotes/origin/master`).
> +You usually want the tags from the other end.
>
> On the other hand, if you are fetching because you would want a
> one-shot merge from somebody else, you typically do not want to
^ permalink raw reply
* How to rename remote branches if I only have "client access"?
From: Manuel Reimer @ 2016-10-20 15:03 UTC (permalink / raw)
To: git
Hello,
I have the following branches on my remote repo:
new_version
master
I now want the "new_version" branch to be the "master" and the old
"master" has to be renamed to the old version number (in my case 0.2.0).
How to do this? Currently this causes me much trouble as I can't delete
the remote "master" repository as this is the "remote current repository"...
Thanks in advance
Manuel
^ permalink raw reply
* [PATCH] doc: remove reference to the traditional layout in git-tag.txt
From: Younes Khoudli @ 2016-10-20 13:21 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Younes Khoudli
This is the only place in the documentation that the traditional layout
is mentioned, and it is confusing. Remove it.
* Documentation/git-tag.txt: Here.
Signed-off-by: Younes Khoudli <younes.khoudli@gmail.com>
---
Documentation/git-tag.txt | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/Documentation/git-tag.txt b/Documentation/git-tag.txt
index 7ecca8e..80019c5 100644
--- a/Documentation/git-tag.txt
+++ b/Documentation/git-tag.txt
@@ -253,9 +253,8 @@ On Automatic following
~~~~~~~~~~~~~~~~~~~~~~
If you are following somebody else's tree, you are most likely
-using remote-tracking branches (`refs/heads/origin` in traditional
-layout, or `refs/remotes/origin/master` in the separate-remote
-layout). You usually want the tags from the other end.
+using remote-tracking branches (eg. `refs/remotes/origin/master`).
+You usually want the tags from the other end.
On the other hand, if you are fetching because you would want a
one-shot merge from somebody else, you typically do not want to
--
2.10.0
^ 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