* 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
* Re: Drastic jump in the time required for the test suite
From: Jeff King @ 2016-10-20 12:31 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: Junio C Hamano, git
In-Reply-To: <alpine.DEB.2.20.1610201154070.3264@virtualbox>
On Thu, Oct 20, 2016 at 12:17:33PM +0200, Johannes Schindelin wrote:
> If you want to know just how harmful this reliance on shell scripting is
> to our goal of keeping Git portable: already moving from Linux to MacOSX
> costs you roughly 3x as long to run the build & test (~12mins vs ~36mins
> for GCC, according to https://travis-ci.org/git/git/builds/159125647).
Wait, shell scripts are slow on MacOS now?
Perhaps, but it seems more likely that one or more of the following is
true:
- setup of the OS X VM takes longer (it does; if you click-through to
the test results, you'll see that the "make test" step goes from
647s on Linux to 1108s on MacOS. That's much worse, but not even
twice as slow, let alone 3x).
- Travis Linux and OSX VMs do not have identical hardware. Looking at
https://docs.travis-ci.com/user/ci-environment/, it appears that
Linux containers get twice as many cores.
- Git performance on Linux may be better than MacOS. The test suite is
very filesystem-heavy because it creates and destroys a lot of files
and repositories. If the kernel vfs performance is worse, it's
likely to show up in the test suite (especially if the issue is
latency and you aren't doing it massively in parallel).
I don't have a real way to measure that, but it seems like a
plausible factor.
So that sucks that the MacOS Travis build takes a half hour to run. But
I don't think that shell scripting is the culprit.
> So the only thing that would really count as an improvement would be to
> change the test suite in such a manner that it relies more on helpers in
> t/helper/ and less on heavy-duty shell scripting.
>
> 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.
I'm not sure what you mean by "resist". The tests suite has been a set
of shell scripts for over a decade. As far as I know there is not
currently a viable alternative. If you have patches that make it faster
without negatively impact the ease of writing tests, I'd be happy to see
them. If you have more t/helper programs that can eliminate expensive
bits of the shell scripts and speed up the test run, great. If you have
some other proposal entirely, I'd love to hear it. But I do not see
that there is any proposal to "resist" at this point.
I'm also not entirely convinced that the test suite being a shell script
is the main culprit for its slowness. We run git a lot of times, and
that's inherent in testing it. I ran the whole test suite under
"strace -f -e execve". There are ~335K execs. Here's the breakdown of
the top ones:
$ 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
Almost half are running git itself. Let's assume that can't be changed.
That leaves ~180K of shell-related overhead (versus the optimal case,
that the entire test suite becomes one monolithic program ;) ).
Close to 1/3 of those processes are just invoking the bin-wrapper
script to set up the EXEC_PATH, etc. I imagine it would not be too hard
to just do that in the test script. In fact, it looks like:
make prefix=/wherever install
GIT_TEST_INSTALLED=/wherever/bin make test
might give you an immediate speedup by skipping bin-wrappers entirely.
The rest of it is harder. I think you'd have to move the test suite to a
language like perl that can do more of that as builtins (I'm sure you'd
enjoy the portability implications of _that_). It would almost be
easier to build a variant of the shell that has sed, rm, cat, and a few
others compiled in.
-Peff
PS I haven't kept up with all of this POSIX-layer stuff that's been
announced in Windows the past few months. Is it a viable path forward
that would have better performance (obviously not in the short term,
but where we may arrive in a few years)?
^ permalink raw reply
* Re: [PATCH v4 05/25] sequencer: eventually release memory allocated for the option values
From: Johannes Schindelin @ 2016-10-20 12:16 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Jakub Narębski, Johannes Sixt, Ramsay Jones
In-Reply-To: <xmqq1szdnnvc.fsf@gitster.mtv.corp.google.com>
Hi Junio,
On Tue, 18 Oct 2016, Junio C Hamano wrote:
> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>
> >> > To remedy that, we now take custody of the option values in question,
> >> > requiring those values to be malloc()ed or strdup()ed
> >>
> >> That is the approach this patch takes, so "eventually release" in
> >> the title is no longer accurate, I would think.
> >
> > To the contrary, we now free() things in remove_state(), so we still
> > "eventually release" the memory.
>
> OK. We call a change to teach remove_state() to free the resource
> does more commonly as "plug leaks"; the word "eventually" gave me an
> impression that we are emphasizing the fact that we do not free(3)
> immediately but lazily do so at the end, hence my response.
I changed the commit subject, hopefully to your liking,
Dscho
^ permalink raw reply
* Re: [PATCH v3 14/25] sequencer: introduce a helper to read files written by scripts
From: Johannes Schindelin @ 2016-10-20 12:07 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Jakub Narębski, Johannes Sixt
In-Reply-To: <xmqqpomxr6t6.fsf@gitster.mtv.corp.google.com>
Hi Junio,
On Tue, 18 Oct 2016, Junio C Hamano wrote:
> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>
> > In the meantime, I'd be happy to just add a comment that this function is
> > intended for oneliners, but that it will also read multi-line files and
> > only strip off the EOL marker from the last line.
> >
> > Would that work for you?
>
> That would be ideal, I would think.
Done,
Dscho
^ permalink raw reply
* [REQUEST PULL] git-gui 0.20.0 to 0.21.0
From: Pat Thoyts @ 2016-10-20 11:39 UTC (permalink / raw)
To: Junio C Hamano; +Cc: GitList
The following changes since commit 4498b3a50a0e839788682f672df267cbc1ba9292:
git-gui: set version 0.20 (2015-04-18 12:15:32 +0100)
are available in the git repository at:
git://repo.or.cz/git-gui.git tags/gitgui-0.21.0
for you to fetch changes up to ccc985126f23ff5d9ac610cb820bca48405ff5ef:
git-gui: set version 0.21 (2016-10-20 11:19:43 +0100)
----------------------------------------------------------------
git-gui 0.21.0
----------------------------------------------------------------
Alex Riesen (2):
git-gui: support for $FILENAMES in tool definitions
git-gui: ensure the file in the diff pane is in the list of selected files
Alexander Shopov (2):
git-gui i18n: Updated Bulgarian translation (565,0f,0u)
git-gui: Mark 'All' in remote.tcl for translation
Dimitriy Ryazantcev (1):
git-gui: Update Russian translation
Elia Pinto (1):
git-gui/po/glossary/txt-to-pot.sh: use the $( ... ) construct for command substitution
Johannes Schindelin (1):
git-gui: respect commit.gpgsign again
Karsten Blees (2):
git-gui: unicode file name support on windows
git-gui: handle the encoding of Git's output correctly
Olaf Hering (1):
git-gui: sort entries in tclIndex
Orgad Shaneh (1):
git-gui: Do not reset author details on amend
Pat Thoyts (19):
Allow keyboard control to work in the staging widgets.
Amend tab ordering and text widget border and highlighting.
git-gui: fix detection of Cygwin
git-gui (Windows): use git-gui.exe in `Create Desktop Shortcut`
Merge branch 'sy/i18n' into pu
Merge branch 'js/commit-gpgsign' into pu
Merge branch 'rs/use-modern-git-merge-syntax' into pu
Merge branch 'va/i18n' into pu
Merge branch 'patches' into pu
Merge branch 'pt/git4win-mods' into pu
Merge branch 'pt/non-mouse-usage' into pu
Merge branch 'va/i18n_2' into pu
git-gui: maintain backwards compatibility for merge syntax
Merge branch 'dr/ru' into pu
git-gui: avoid persisting modified author identity
Merge branch 'kb/unicode' into pu
Merge branch 'os/preserve-author' into pu
Merge branch 'as/bulgarian' into pu
git-gui: set version 0.21
René Scharfe (1):
git-gui: stop using deprecated merge syntax
Satoshi Yasushima (6):
git-gui: consistently use the same word for "remote" in Japanese
git-gui: consistently use the same word for "blame" in Japanese
git-gui: apply po template to Japanese translation
git-gui: add Japanese language code
git-gui: update Japanese translation
git-gui: update Japanese information
Vasco Almeida (6):
git-gui i18n: mark strings for translation
git-gui: l10n: add Portuguese translation
git-gui i18n: internationalize use of colon punctuation
git-gui i18n: mark "usage:" strings for translation
git-gui: fix incorrect use of Tcl append command
git-gui i18n: mark string in lib/error.tcl for translation
yaras (1):
git-gui: fix initial git gui message encoding
GIT-VERSION-GEN | 2 +-
Makefile | 2 +-
git-gui.sh | 154 +-
lib/blame.tcl | 2 +-
lib/branch_checkout.tcl | 2 +-
lib/branch_create.tcl | 2 +-
lib/branch_delete.tcl | 4 +-
lib/branch_rename.tcl | 2 +-
lib/browser.tcl | 6 +-
lib/commit.tcl | 39 +-
lib/database.tcl | 4 +-
lib/diff.tcl | 14 +-
lib/error.tcl | 8 +-
lib/index.tcl | 12 +-
lib/merge.tcl | 18 +-
lib/option.tcl | 8 +-
lib/remote.tcl | 8 +-
lib/remote_add.tcl | 2 +-
lib/remote_branch_delete.tcl | 2 +-
lib/shortcut.tcl | 17 +-
lib/themed.tcl | 87 +-
lib/tools.tcl | 3 +
lib/tools_dlg.tcl | 6 +-
lib/transport.tcl | 2 +-
po/bg.po | 3543 ++++++++++++++++++++++--------------------
po/glossary/pt_pt.po | 293 ++++
po/glossary/txt-to-pot.sh | 4 +-
po/ja.po | 3077 ++++++++++++++++++------------------
po/pt_pt.po | 2716 ++++++++++++++++++++++++++++++++
po/ru.po | 680 +++-----
30 files changed, 6957 insertions(+), 3762 deletions(-)
create mode 100644 po/glossary/pt_pt.po
create mode 100644 po/pt_pt.po
^ permalink raw reply
* Re: Drastic jump in the time required for the test suite
From: Jeff King @ 2016-10-20 11:39 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: Junio C Hamano, git
In-Reply-To: <alpine.DEB.2.20.1610201218060.3264@virtualbox>
On Thu, Oct 20, 2016 at 12:50:32PM +0200, Johannes Schindelin wrote:
> That reflects my findings, too. I want to add that I found preciously
> little difference between running slow-to-fast and running in numeric
> order, so I gave up on optimizing on that front.
Interesting. It makes a 10-15% difference here.
I also point "--root" at a ram disk. The tests are very I/O heavy and
sometimes fsync; even on a system with an SSD, this saves another ~10%.
I know that's small potatoes compared to the Windows vs Linux times, but
it might be worth exploring.
> Further, I found that the Subversion tests (which run at the end) are so
> close in their running time that running the tests in parallel with -j5
> does not result in any noticeable improvement when reordered.
I normally don't run the Subversion tests at all. Installing cvs, cvsps,
subversion, and libsvn-perl nearly doubles the runtime of the test suite
for me (I imagine adding p4 to the mix would bump it further). While
it's certainly possible to break them with a change in core git, it
doesn't seem like a good tradeoff if I'm not touching them often.
As the GfW maintainer, you probably should be running them, at least
before a release. But cutting them might be a good way to speed up your
day-to-day runs.
I also use -j16 on a quad-core (+hyperthreads) machine, which I arrived
at experimentally. At least on Linux, it's definitely worth having more
threads than processors, to keep the processors busy.
> I guess I will have to bite into the sour apple and try to profile, say,
> t3404 somehow, including all the shell scripting stuff, to identify where
> exactly all that time is lost. My guess is that it boils down to
> gazillions of calls to programs like expr.exe or merely subshells.
I'm not so sure it isn't gazillions of calls to git. It is testing
rebase, after all, which is itself a shell script. GIT_TRACE_PERFORMANCE
gives sort of a crude measure; it reports only builtins (so it will
underestimate the total time spent in git), but it also doesn't make
clear which programs call which, so some times are double-counted (if a
builtin shells out to another builtin). But:
$ export GIT_TRACE_PERFORMANCE=/tmp/foo.out
$ rm /tmp/foo.out
$ time ./t3404-rebase-interactive.sh
real 0m29.755s
user 0m1.444s
sys 0m2.268s
$ perl -lne '
/performance: ([0-9.]+)/ and $total += $1;
END { print $total }
' /tmp/foo.out
32.851352624
Clearly that's not 100% accurate, as it claims we spent longer in git
than the script actually took to run. Given the caveats above, I'm not
even sure if it is in the right ballpark. But there are 11,000 git
builtins run as part of that script. Even at 2ms each, that's still most
of the time going to git.
And obviously the fix involves converting git-rebase, which you're
already working on. But it's not clear to me that the test
infrastructure or shell scripts are the primary cause of the slowness in
this particular case.
-Peff
^ permalink raw reply
* Re: Drastic jump in the time required for the test suite
From: Duy Nguyen @ 2016-10-20 11:02 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: Junio C Hamano, Git Mailing List
In-Reply-To: <alpine.DEB.2.20.1610191049040.3847@virtualbox>
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
--
Duy
^ permalink raw reply
* Re: Drastic jump in the time required for the test suite
From: Johannes Schindelin @ 2016-10-20 10:50 UTC (permalink / raw)
To: Jeff King; +Cc: Junio C Hamano, git
In-Reply-To: <20161019205638.m3ytxozzmeh47ml2@sigill.intra.peff.net>
Hi peff,
On Wed, 19 Oct 2016, Jeff King wrote:
> On Wed, Oct 19, 2016 at 10:32:12AM -0700, Junio C Hamano wrote:
>
> > > Maybe we should start optimizing the tests...
> >
> > Yup, two things that come to mind are to identify long ones and see if
> > each of them can be split into two halves that can be run in parallel,
> > and to go through the tests with fine toothed comb and remove the ones
> > that test exactly the same thing as another test. The latter would be
> > very time consuming, though.
>
> FWIW, I have made attempts at "split long ones into two" before, and
> didn't come up with much. There _are_ some tests that are much longer
> than others[1], but they are not longer than the whole suite takes to
> run. So running in slow-to-fast order means they start first, are run in
> parallel with the other tests, and the CPUs stay relatively full through
> the whole run.
That reflects my findings, too. I want to add that I found preciously
little difference between running slow-to-fast and running in numeric
order, so I gave up on optimizing on that front.
> 43.216765165329 t3404-rebase-interactive.sh
> 30.6568658351898 t3421-rebase-topology-linear.sh
> 27.92564702034 t9001-send-email.sh
> 15.5906939506531 t9500-gitweb-standalone-no-errors.sh
> 15.4882569313049 t6030-bisect-porcelain.sh
> 14.487174987793 t7610-mergetool.sh
> 13.8276169300079 t3425-rebase-topology-merges.sh
> 12.7450480461121 t3426-rebase-submodule.sh
> 12.4915001392365 t3415-rebase-autosquash.sh
> 11.8122401237488 t5572-pull-submodule.sh
That looks very similar to what I found: t3404 on top, followed by t3421.
Further, I found that the Subversion tests (which run at the end) are so
close in their running time that running the tests in parallel with -j5
does not result in any noticeable improvement when reordered.
I guess I will have to bite into the sour apple and try to profile, say,
t3404 somehow, including all the shell scripting stuff, to identify where
exactly all that time is lost. My guess is that it boils down to
gazillions of calls to programs like expr.exe or merely subshells.
Ciao,
Dscho
^ permalink raw reply
* Re: Drastic jump in the time required for the test suite
From: Johannes Schindelin @ 2016-10-20 10:17 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <xmqqbmygmehv.fsf@gitster.mtv.corp.google.com>
Hi Junio,
On Wed, 19 Oct 2016, Junio C Hamano wrote:
> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>
> > What I can also say for certain is that the version from yesterday (commit
> > 4ef44ce) was the first one in a week that built successfully and hence
> > reached the test phase, and it was the first version of `pu` ever to time
> > out after running for 3h.
>
> I am sympathetic, but I'd be lying if I said I can feel your pain.
Obviously.
> Admittedly I do not run _all_ the tests (I certainly know that I
> exclude the ones behind EXPENSIVE prerequisite), but after every
> rebuilding of 'jch' and 'pu', I run the testsuite (and also rebuild
> docs) before pushing them out, and "make test && make doc && make
> install install-doc" run sequentially for the four integration
> branches finishes within 15 minutes, even when I run them after
> "make clean".
You have the luxury of a system that runs shell scripts fast.
Or maybe I should put it differently: you set up the test suite in such a
way that it runs fast on your system, exploiting the fact that shell
scripts run fast there.
If you want to know just how harmful this reliance on shell scripting is
to our goal of keeping Git portable: already moving from Linux to MacOSX
costs you roughly 3x as long to run the build & test (~12mins vs ~36mins
for GCC, according to https://travis-ci.org/git/git/builds/159125647).
Again, I have to repeat myself: this is not good.
> Perhaps the recent change to run the tests in parallel from slower
> to faster under prove may be helping my case.
No, you misunderstand. The tests are *already* run in parallel. And
running them from slower to faster would only make a difference if the
last tests were not requiring roughly the same time to run.
Also, I cannot use prove(1) because it proved to be unreliable in my setup
(it does hang from time to time, for no good reason whatsoever, which
would only make my situation worse, of course).
> > Maybe we should start optimizing the tests...
>
> Yup, two things that come to mind are to identify long ones and see
> if each of them can be split into two halves that can be run in
> parallel, and to go through the tests with fine toothed comb and
> remove the ones that test exactly the same thing as another test.
> The latter would be very time consuming, though.
Again, you misunderstand.
The problem is not whether or not to run the tests in parallel.
The problem is that our tests take an insanely long time to run. That is a
big problem, it is no good, tests are only useful if they are cheap enough
to run all the time.
So the only thing that would really count as an improvement would be to
change the test suite in such a manner that it relies more on helpers in
t/helper/ and less on heavy-duty shell scripting.
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.
Ciao,
Dscho
P.S.: After increasing the time-out to 240 minutes, the test suite still
times out. I investigated a little bit and it appears that
t6030-bisect-porcelain.sh now hangs in the `git bisect next` call of its
"2 - bisect starts with only one bad" test case. This is specific to
Windows, I cannot reproduce that problem on Linux, and I will keep you
posted on my progress.
^ permalink raw reply
* Re: [PATCH] rev-list: restore the NUL commit separator in --header mode
From: Torsten Bögershausen @ 2016-10-20 6:51 UTC (permalink / raw)
To: Dennis Kaarsemaker, git
Cc: jacob.e.keller, stefanbeller, peff, j6t, jacob.keller
In-Reply-To: <20161019210448.aupphybw5qar6mqe@hurricane>
diff --git a/t/t6000-rev-list-misc.sh b/t/t6000-rev-list-misc.sh
> index 3e752ce..a2acff3 100755
> --- a/t/t6000-rev-list-misc.sh
> +++ b/t/t6000-rev-list-misc.sh
> @@ -100,4 +100,11 @@ test_expect_success '--bisect and --first-parent can not be combined' '
> test_must_fail git rev-list --bisect --first-parent HEAD
> '
>
> +test_expect_success '--header shows a NUL after each commit' '
> + touch expect &&
> + printf "\0" > expect &&
Micronit: No ' ' after '>'
(And why do we need the touch ?)
+ printf "\0" >expect &&
> + git rev-list --header --max-count=1 HEAD | tail -n1 >actual &&
> + test_cmp_bin expect actual
> +'
> +
> test_done
^ permalink raw reply
* Re: [PATCH 6/7] diff: handle sha1 abbreviations outside of repository
From: Jeff King @ 2016-10-20 6:31 UTC (permalink / raw)
To: git
In-Reply-To: <20161020062125.3iqej3bpdoitr3fz@sigill.intra.peff.net>
On Thu, Oct 20, 2016 at 02:21:25AM -0400, Jeff King wrote:
> diff --git a/diff.c b/diff.c
> index 8f0f309..ef11001 100644
> --- a/diff.c
> +++ b/diff.c
> @@ -3049,6 +3049,19 @@ static int similarity_index(struct diff_filepair *p)
> return p->score * 100 / MAX_SCORE;
> }
>
> +static const char *diff_abbrev_oid(const struct object_id *oid, int abbrev)
> +{
> + if (startup_info->have_repository)
> + return find_unique_abbrev(oid->hash, abbrev);
> + else {
> + char *hex = oid_to_hex(oid);
> + if (abbrev < 0 || abbrev > GIT_SHA1_HEXSZ)
> + die("BUG: oid abbreviation out of range: %d", abbrev);
> + hex[abbrev] = '\0';
> + return hex;
> + }
> +}
Note that this function has a semantic (but not textual) conflict with
lt/auto-abbrev in 'next', as it sets DEFAULT_ABBREV to -1.
The resolution is:
diff --git a/diff.c b/diff.c
index cab811e..4c09314 100644
--- a/diff.c
+++ b/diff.c
@@ -3102,7 +3102,9 @@ static const char *diff_abbrev_oid(const struct object_id *oid, int abbrev)
return find_unique_abbrev(oid->hash, abbrev);
else {
char *hex = oid_to_hex(oid);
- if (abbrev < 0 || abbrev > GIT_SHA1_HEXSZ)
+ if (abbrev < 0)
+ abbrev = FALLBACK_DEFAULT_ABBREV;
+ if (abbrev > GIT_SHA1_HEXSZ)
die("BUG: oid abbreviation out of range: %d", abbrev);
hex[abbrev] = '\0';
return hex;
This logic could be pushed down into the find_unique_abbrev() code
(where it _would_ just cause a textual conflict). I preferred to keep it
up here because other callers could conceivably want to handle the
non-repo case in some different way (e.g., by not abbreviating at all).
-Peff
^ permalink raw reply related
* [PATCH 7/7] setup_git_env: avoid blind fall-back to ".git"
From: Jeff King @ 2016-10-20 6:24 UTC (permalink / raw)
To: git
In-Reply-To: <20161020061536.6fqh23xb2nhxodpa@sigill.intra.peff.net>
When we default to ".git" without having done any kind of
repository setup, the results quite often do what the user
expects. But this has also historically been the cause of
some poorly behaved corner cases. These cases can be hard to
find, because they happen at the conjunction of two
relatively rare circumstances:
1. We are running some code which assumes there's a
repository present, but there isn't necessarily one
(e.g., low-level diff code triggered by "git diff
--no-index" might try to look at some repository data).
2. We have an unusual setup, like being in a subdirectory
of the working tree, or we have a .git file (rather
than a directory), or we are running a tool like "init"
or "clone" which may operate on a repository in a
different directory.
Our test scripts often cover (1), but miss doing (2) at the
same time, and so the fallback appears to work but has
lurking bugs. We can flush these bugs out by refusing to do
the fallback entirely., This makes potential problems a lot
more obvious by complaining even for "usual" setups.
This passes the test suite (after the adjustments in the
previous patches), but there's a risk of regression for any
cases where the fallback usually works fine but the code
isn't exercised by the test suite. So by itself, this
commit is a potential step backward, but lets us take two
steps forward once we've identified and fixed any such
instances.
Signed-off-by: Jeff King <peff@peff.net>
---
environment.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/environment.c b/environment.c
index cd5aa57..b1743e6 100644
--- a/environment.c
+++ b/environment.c
@@ -164,8 +164,11 @@ static void setup_git_env(void)
const char *replace_ref_base;
git_dir = getenv(GIT_DIR_ENVIRONMENT);
- if (!git_dir)
+ if (!git_dir) {
+ if (!startup_info->have_repository)
+ die("BUG: setup_git_env called without repository");
git_dir = DEFAULT_GIT_DIR_ENVIRONMENT;
+ }
gitfile = read_gitfile(git_dir);
git_dir = xstrdup(gitfile ? gitfile : git_dir);
if (get_common_dir(&sb, git_dir))
--
2.10.1.619.g16351a7
^ 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