Re: [PATCH] perf: work around the tested repo having an index.lock
On Mon, Jun 5, 2017 at 4:02 AM, Junio C Hamano wrote: > Christian Couder writes: > >> On Sun, Jun 4, 2017 at 2:00 AM, Junio C Hamano wrote: >>> Ævar Arnfjörð Bjarmason writes: >>> > My feeling exactly. Diagnosing and failing upfront saying "well you > made a copy but it is not suitable for testing" sounds more sensible > at lesat to me. This change makes the repo suitable for testing when it wasn't before. >>> >>> Perhaps "not suitable" was a bit too vague. >>> >>> The copy you made is not in a consistent state that is good for >>> testing. This change may declare that it is now in a consistent >>> state, but removal of a single *.lock file does not make it so. We >>> do not know what other transient inconsistency the resulting copy >>> has; it is inherent to git-unaware copy---that is why we discouraged >>> and removed rsync transport after all. >> >> If we don't like git-unaware copies, maybe we should go back to the >> reasons why we are making one here. > > We do need git-unaware bit-for-bit copy for testing, because you may > want to see the effect of unreachable objects, for example. I think there might be different kind of people interested in performance tests. Users with existing repositories might want to see how the different Git versions perform on their real life repos. Developers might want to test Git on different repos with different characteristics. For example some developers might want to test on repos with and without a lot of unreachable objects, to make sure that the latest changes they made improve perf in both cases. While some users might only be interested in testing on their actual repositories to see how the latest Git versions improve things (or not) in practice. In this example the needs of developers would perhaps be better suited if they could control the amount of unreachable objects in the tests, while the needs of the users would be better suited if the tests just used actual repos as is. So I wonder what changes would be needed to the perf framework and the perf tests to accomodate both of these kinds of needs. > It's just that git-unaware copies, because it cannot be an atomic > snapshot, can introduce inconsistencies the original repository did > not have, rendering the result ineffective.
Re: [PATCH] test-lib: add ability to cap the runtime of tests
On Mon, Jun 5, 2017 at 3:55 AM, Junio C Hamano wrote: > Ævar Arnfjörð Bjarmason writes: > >> Realistically I'm going to submit this patch, I'm not going to take >> the much bigger project of refactoring the entire test suite so that >> no test runs under N second, and of course any such refactoring can >> only aim for a fixed instead of dynamic N. > > I do not expect any single person to tackle the splitting. I just > wished that a patch inspired by this patch (or better yet, a new > version of this patch) made the tail end of "make test" output to > read like this: > >... >[18:32:44] t9400-git-cvsserver-server.sh .. ok18331 ms >[18:32:49] t9402-git-cvsserver-refs.sh ok22902 ms >[18:32:49] t9200-git-cvsexportcommit.sh ... ok25163 ms >[18:32:51] >All tests successful. >Files=785, Tests=16928, 122 wallclock secs ( ... >Result: PASS > >* The following tests took longer than 15 seconds to run. We > may want to look into splitting them into smaller files. > >t3404-rebase-interactive.sh ...19 secs >t9001-send-email.sh ...22 secs >t9402-git-cvsserver-refs.sh ...22 secs >t9200-git-cvsexportcommit.sh ..25 secs > > when the hidden feature is _not_ used, so that wider set of people > will be forced to see that some tests take inordinate amount of > time, and entice at least some of them to look into it. I wonder if splitting tests would make a good GSoC microproject for next year.
What's cooking in git.git (Jun 2017, #03; Mon, 5)
Here are the topics that have been cooking. Commits prefixed with '-' are only in 'pu' (proposed updates) while commits prefixed with '+' are in 'next'. The ones marked with '.' do not appear in any of the integration branches, but I am still holding onto them. With many fixes accumulated since v2.13.0, the first maintenance release v2.13.1 has been tagged. Thanks for all the help. I'll be mostly offline in the later part of this week, so please expect slow response. You can find the changes described here in the integration branches of the repositories listed at http://git-blame.blogspot.com/p/git-public-repositories.html -- [Graduated to "master"] * ab/c-translators-comment-style (2017-05-31) 1 commit (merged to 'next' on 2017-06-01 at df7fb9199a) + C style: use standard style for "TRANSLATORS" comments Update the C style recommendation for notes for translators, as recent versions of gettext tools can work with our style of multi-line comments. * ab/sha1dc-maint (2017-05-22) 1 commit (merged to 'next' on 2017-05-30 at 9eb40bf912) + sha1dc: update from upstream (this branch is used by ab/sha1dc.) The "collision detecting" SHA-1 implementation shipped with 2.13 was quite broken on some big-endian platforms and/or platforms that do not like unaligned fetches. Update to the upstream code which has already fixed these issues. * ab/t3070-test-dedup (2017-05-29) 1 commit (merged to 'next' on 2017-05-30 at 71eadec33b) + wildmatch test: remove redundant duplicate test Test cleanup. * ad/pull-remote-doc (2017-06-02) 1 commit (merged to 'next' on 2017-06-02 at 32915e88db) + docs: fix formatting and grammar Docfix. * ah/doc-filter-branch-export-env (2017-05-29) 1 commit (merged to 'next' on 2017-05-30 at a62168bf77) + doc: filter-branch does not require re-export of vars Docfix. * ah/doc-rev-parse-short-default (2017-06-01) 1 commit (merged to 'next' on 2017-06-01 at f64fcc2a26) + doc: rewrite description for rev-parse --short Doc update. * jh/close-index-before-stat (2017-04-28) 1 commit (merged to 'next' on 2017-05-16 at 0c0372eb02) + read-cache: close index.lock in do_write_index Originally merged to 'next' on 2017-04-30 The timestamp of the index file is now taken after the file is closed, to help Windows, on which a stale timestamp is reported by fstat() on a file that is opened for writing and data was written but not yet closed. * jk/connect-symref-info-leak-fix (2017-05-26) 1 commit (merged to 'next' on 2017-05-30 at d8b75d2dd9) + connect.c: fix leak in parse_one_symref_info() Leakfix. * jk/drop-free-refspecs (2017-06-01) 1 commit (merged to 'next' on 2017-06-01 at 8f455319fc) + remote: drop free_refspecs() function Code clean-up. * jk/unbreak-am-h (2017-05-30) 1 commit (merged to 'next' on 2017-06-01 at ee2233d409) + am: handle "-h" argument earlier (this branch is used by jk/consistent-h.) "git am -h" triggered a BUG(). * jk/url-insteadof-config (2017-06-01) 1 commit (merged to 'next' on 2017-06-01 at e7ea6032c2) + docs/config: mention protocol implications of url.insteadOf The interaction of "url.*.insteadOf" and custom URL scheme's whitelisting is now documented better. * js/blame-lib (2017-05-25) 29 commits (merged to 'next' on 2017-05-30 at b4678b36a7) + blame: move entry prepend to libgit + blame: move scoreboard setup to libgit + blame: move scoreboard-related methods to libgit + blame: move fake-commit-related methods to libgit + blame: move origin-related methods to libgit + blame: move core structures to header + blame: create entry prepend function + blame: create scoreboard setup function + blame: create scoreboard init function + blame: rework methods that determine 'final' commit + blame: wrap blame_sort and compare_blame_final + blame: move progress updates to a scoreboard callback + blame: make sanity_check use a callback in scoreboard + blame: move no_whole_file_rename flag to scoreboard + blame: move xdl_opts flags to scoreboard + blame: move show_root flag to scoreboard + blame: move reverse flag to scoreboard + blame: move contents_from to scoreboard + blame: move copy/move thresholds to scoreboard + blame: move stat counters to scoreboard + blame: rename nth_line function + blame: rename ent_score function + blame: rename coalesce function + blame: rename origin-related functions + blame: rename scoreboard structure to blame_scoreboard + blame: rename origin structure to blame_origin + blame: remove unused parameters + blame: move textconv_object with related functions + blame: remove unneeded dependency on blob.h The internal logic used in "git blame" has been libified to make it easier to use by cgit. * mb/diff-default-to-indent-heuristics (2017-05-09) 4 commits (merged to 'next' on 2017-05-29 at 7645575e21) + add--interactive: drop diff.indentHeuristic handling + diff: enable indent heuristic b
Re: [PATCH 1/2] add setup step to filter-branch
Andreas Heiduk writes:
> A specific `--setup` step in `git filter-branch` makes it much easier
> to define the initial values of variables used in the real filters.
> Also sourcing/defining utility functions here instead of
> `--env-filter` improves performance and minimizes clogging the output
> in case of errors.
>
> Signed-off-by: Andreas Heiduk
> ---
I was placed on To: line, but I do not have a strong opinion on this
change, either for or against.
"filter-branch" program itself may probably already be hard to port
to C, but I need to point out that this makes it even harder than it
currently is [*1*], and it is likely that it has to stay implemented
in shell forever, though. I do not mind that future myself, but
those on platforms with weaker implementation of shells might.
[Footnote]
*1* The issue is *not* that these individual filter commands expect
written as a shell scriptlet; it is that these
scriptlets expect to be evaled inside a single shell process,
making an update to a shell variable in one command visible to
the next command that runs.
> Documentation/git-filter-branch.txt | 16 +++-
> git-filter-branch.sh| 18 +-
> 2 files changed, 24 insertions(+), 10 deletions(-)
>
> diff --git a/Documentation/git-filter-branch.txt
> b/Documentation/git-filter-branch.txt
> index 6e4bb0220..45c849d8c 100644
> --- a/Documentation/git-filter-branch.txt
> +++ b/Documentation/git-filter-branch.txt
> @@ -8,11 +8,11 @@ git-filter-branch - Rewrite branches
> SYNOPSIS
>
> [verse]
> -'git filter-branch' [--env-filter ] [--tree-filter ]
> - [--index-filter ] [--parent-filter ]
> - [--msg-filter ] [--commit-filter ]
> - [--tag-name-filter ] [--subdirectory-filter ]
> - [--prune-empty]
> +'git filter-branch' [--setup ] [--env-filter ]
> + [--tree-filter ] [--index-filter ]
> + [--parent-filter ] [--msg-filter ]
> + [--commit-filter ] [--tag-name-filter ]
> + [--subdirectory-filter ] [--prune-empty]
> [--original ] [-d ] [-f | --force]
> [--] [...]
>
> @@ -82,6 +82,12 @@ multiple commits.
> OPTIONS
> ---
>
> +--setup ::
> + This is not a real filter executed for each commit but a one
> + time setup just before the loop. Therefore no commit-specific
> + variables are defined yet. Functions or variables defined here
> + can be used or modified in the following filter steps.
> +
> --env-filter ::
> This filter may be used if you only need to modify the environment
> in which the commit will be performed. Specifically, you might
> diff --git a/git-filter-branch.sh b/git-filter-branch.sh
> index aafaf708d..2758ae5eb 100755
> --- a/git-filter-branch.sh
> +++ b/git-filter-branch.sh
> @@ -81,11 +81,12 @@ set_ident () {
> finish_ident COMMITTER
> }
>
> -USAGE="[--env-filter ] [--tree-filter ]
> - [--index-filter ] [--parent-filter ]
> - [--msg-filter ] [--commit-filter ]
> - [--tag-name-filter ] [--subdirectory-filter ]
> - [--original ] [-d ] [-f | --force]
> +USAGE="[--setup ] [--env-filter ]
> + [--tree-filter ] [--index-filter ]
> + [--parent-filter ] [--msg-filter ]
> + [--commit-filter ] [--tag-name-filter ]
> + [--subdirectory-filter ] [--original ]
> + [-d ] [-f | --force]
> [...]"
>
> OPTIONS_SPEC=
> @@ -96,6 +97,7 @@ if [ "$(is_bare_repository)" = false ]; then
> fi
>
> tempdir=.git-rewrite
> +filter_setup=
> filter_env=
> filter_tree=
> filter_index=
> @@ -148,6 +150,9 @@ do
> -d)
> tempdir="$OPTARG"
> ;;
> + --setup)
> + filter_setup="$OPTARG"
> + ;;
> --env-filter)
> filter_env="$OPTARG"
> ;;
> @@ -317,6 +322,9 @@ else
> need_index=
> fi
>
> +eval "$filter_setup" < /dev/null ||
> + die "filter setup failed: $filter_setup"
> +
> while read commit parents; do
> git_filter_branch__commit_count=$(($git_filter_branch__commit_count+1))
Re: [PATCH] perf: work around the tested repo having an index.lock
Jeff King writes: > I don't really mind addressing this one case that much. I'm not sure > that we want to accrue a pile of band-aids here that causes a > maintenance burden and doesn't really solve the problem completely. One > way to do that is to say no to the first band-aid. Yup, that was where my objection came from. > But we could also > apply it and see what happens. At the very worst it's a few extra lines > of code, and we can start to get worried on the second or third > band-aid. That's OK as well. But we should resolve now that we will rip all of them out once we start seeing the second or third band-aid. I really do not want to see the "accring a pile of band aids" in our future.
Re: [PATCH] perf: work around the tested repo having an index.lock
Christian Couder writes: > On Sun, Jun 4, 2017 at 2:00 AM, Junio C Hamano wrote: >> Ævar Arnfjörð Bjarmason writes: >> My feeling exactly. Diagnosing and failing upfront saying "well you made a copy but it is not suitable for testing" sounds more sensible at lesat to me. >>> >>> This change makes the repo suitable for testing when it wasn't before. >> >> Perhaps "not suitable" was a bit too vague. >> >> The copy you made is not in a consistent state that is good for >> testing. This change may declare that it is now in a consistent >> state, but removal of a single *.lock file does not make it so. We >> do not know what other transient inconsistency the resulting copy >> has; it is inherent to git-unaware copy---that is why we discouraged >> and removed rsync transport after all. > > If we don't like git-unaware copies, maybe we should go back to the > reasons why we are making one here. We do need git-unaware bit-for-bit copy for testing, because you may want to see the effect of unreachable objects, for example. It's just that git-unaware copies, because it cannot be an atomic snapshot, can introduce inconsistencies the original repository did not have, rendering the result ineffective.
Re: [PATCH] test-lib: add ability to cap the runtime of tests
Ævar Arnfjörð Bjarmason writes:
>> I certainly understand that but in the longer term, I'd prefer the
>> approach to call out an overly large test. That will hopefully
>> motivate us to split it (or speed up the thing) to help folks on
>> many-core machines.
>
> The reason I didn't document this in t/README was because I thought it
> made sense to have this as a mostly hidden feature that end users
> wouldn't be tempted to fiddle with, but would be useful to someone
> doing git development.
>
> Realistically I'm going to submit this patch, I'm not going to take
> the much bigger project of refactoring the entire test suite so that
> no test runs under N second, and of course any such refactoring can
> only aim for a fixed instead of dynamic N.
I do not expect any single person to tackle the splitting. I just
wished that a patch inspired by this patch (or better yet, a new
version of this patch) made the tail end of "make test" output to
read like this:
...
[18:32:44] t9400-git-cvsserver-server.sh .. ok18331 ms
[18:32:49] t9402-git-cvsserver-refs.sh ok22902 ms
[18:32:49] t9200-git-cvsexportcommit.sh ... ok25163 ms
[18:32:51]
All tests successful.
Files=785, Tests=16928, 122 wallclock secs ( ...
Result: PASS
* The following tests took longer than 15 seconds to run. We
may want to look into splitting them into smaller files.
t3404-rebase-interactive.sh ...19 secs
t9001-send-email.sh ...22 secs
t9402-git-cvsserver-refs.sh ...22 secs
t9200-git-cvsexportcommit.sh ..25 secs
when the hidden feature is _not_ used, so that wider set of people
will be forced to see that some tests take inordinate amount of
time, and entice at least some of them to look into it.
> Collecting the skipped ones is easy enough to do with a grep + for
> loop, so I don't think it's worth making the implementation more
> complex to occasionally answer the question of how many tests were
> skipped due to running into the timeout
"Easy enough" and "made to stand out so _NO_ effort is needed to
see" are very different things.
> Or you can just run in a mode where you stream out however many tests
> you're going to run as you go along, and then print "1..NUM_TESTS" at
> the end.
>
> We use the latter, so we can abort the entire test suite at any time
> with test_done, that's what this change does.
cf. bf4b7219 ("test-lib.sh: Add check for invalid use of 'skip_all'
facility", 2012-09-01)
>> Oh, by the way, is "date +%s" even portable? I thought not.
>
> The lib-git-p4.sh lib says not, and shells out to python's time() is a
> workaround, I could replace this with perl -e 'print time', but
> thought it wasn't worth bothering with for an obscure optional feature
> like this.
Let's do the right thing, because doing so is easy.
I personally think that filter-branch being broken is not noticed
only because it is not very often used, as opposed to that we want
to encourage those who are following along with us, especially those
who are on minority platforms, to run our tests every day. Let's
not spread sloppyness unnecessarily.
[ANNOUNCE] Git v2.13.1
The latest maintenance release Git v2.13.1 is now available at the usual places. The tarballs are found at: https://www.kernel.org/pub/software/scm/git/ The following public repositories all have a copy of the 'v2.13.1' tag and the 'maint' branch that the tag points at: url = https://kernel.googlesource.com/pub/scm/git/git url = git://repo.or.cz/alt-git.git url = https://github.com/gitster/git Git v2.13.1 Release Notes = Fixes since v2.13 - * The Web interface to gmane news archive is long gone, even though the articles are still accessible via NTTP. Replace the links with ones to public-inbox.org. Because their message identification is based on the actual message-id, it is likely that it will be easier to migrate away from it if/when necessary. * Update tests to pass under GETTEXT_POISON (a mechanism to ensure that output strings that should not be translated are not translated by mistake), and tell TravisCI to run them. * Setting "log.decorate=false" in the configuration file did not take effect in v2.13, which has been corrected. * An earlier update to test 7400 needed to be skipped on CYGWIN. * Git sometimes gives an advice in a rhetorical question that does not require an answer, which can confuse new users and non native speakers. Attempt to rephrase them. * "git read-tree -m" (no tree-ish) gave a nonsense suggestion "use --empty if you want to clear the index". With "-m", such a request will still fail anyway, as you'd need to name at least one tree-ish to be merged. * The codepath in "git am" that is used when running "git rebase" leaked memory held for the log message of the commits being rebased. * "pack-objects" can stream a slice of an existing packfile out when the pack bitmap can tell that the reachable objects are all needed in the output, without inspecting individual objects. This strategy however would not work well when "--local" and other options are in use, and need to be disabled. * Clarify documentation for include.path and includeIf..path configuration variables. * Tag objects, which are not reachable from any ref, that point at missing objects were mishandled by "git gc" and friends (they should silently be ignored instead) * A few http:// links that are redirected to https:// in the documentation have been updated to https:// links. * Make sure our tests would pass when the sources are checked out with "platform native" line ending convention by default on Windows. Some "text" files out tests use and the test scripts themselves that are meant to be run with /bin/sh, ought to be checked out with eol=LF even on Windows. * Fix memory leaks pointed out by Coverity (and people). * The receive-pack program now makes sure that the push certificate records the same set of push options used for pushing. * "git cherry-pick" and other uses of the sequencer machinery mishandled a trailer block whose last line is an incomplete line. This has been fixed so that an additional sign-off etc. are added after completing the existing incomplete line. * The shell completion script (in contrib/) learned "git stash" has a new "push" subcommand. * Travis CI gained a task to format the documentation with both AsciiDoc and AsciiDoctor. * Update the C style recommendation for notes for translators, as recent versions of gettext tools can work with our style of multi-line comments. * "git clone --config var=val" is a way to populate the per-repository configuration file of the new repository, but it did not work well when val is an empty string. This has been fixed. * A few codepaths in "checkout" and "am" working on an unborn branch tried to access an uninitialized piece of memory. * "git for-each-ref --format=..." with %(HEAD) in the format used to resolve the HEAD symref as many times as it had processed refs, which was wasteful, and "git branch" shared the same problem. * "git interpret-trailers", when used as GIT_EDITOR for "git commit -v", looked for and appended to a trailer block at the very end, i.e. at the end of the "diff" output. The command has been corrected to pay attention to the cut-mark line "commit -v" adds to the buffer---the real trailer block should appear just before it. * A test allowed both "git push" and "git receive-pack" on the other end write their traces into the same file. This is OK on platforms that allows atomically appending to a file opened with O_APPEND, but on other platforms led to a mangled output, causing intermittent test failures. This has been fixed by disabling traces from "receive-pack" in the test. * "foo\bar\baz" in "git fetch foo\bar\baz", even though there is no slashes in it, cannot be a nickname for a remote on Windows, as that is like
Re: [PATCH v4 0/8] PCRE v2, PCRE v1 JIT, log -P & fixes
Ævar Arnfjörð Bjarmason writes: > On Fri, Jun 2, 2017 at 6:10 PM, Johannes Schindelin >> >> Will continue with testing Git for Windows using PCRE2 next week and keep >> you posted, > > Thanks a lot for testing it. Great to hear that it (definitely almost) works! > > If the grep tests it's very likely that all of them will pass, the > only tests I run when developing this series (outside of the full run > for list submission) are t[0-9]*grep*.sh t[0-9]*log*.sh tests, since > those are the only ones impacted by it. 'ab/pcre-v2' was marked for 'next' in "What's cookin" a few issues ago, but I'll keep it in 'pu' to wait for the above---please give me a go ahead when we all are happy with the topic. I expect to be offline in the later part of this week, by the way. Thanks.
Re: New Order
Dear Sir, Have a nice day! We are interested in purchasing your products.Send your latest catalog and also, inform me about the Minimum order quantity, Delivery time or FOB, and payment terms warranty. Thanks and Best Regards, Mr.Marco Plujm. Phone: (+1) 703-684-5885 Fax: (+1) 703-684-0644 Washington, DC 20052 United State
Re: Git Merge 2017 Videos
From: "Christian Couder" On Sun, Jun 4, 2017 at 7:36 PM, Kevin Daudt wrote: On Sun, Jun 04, 2017 at 11:24:17AM +0100, Philip Oakley wrote: While looking at the recent .gitignore issue (the need to use `**`) I came up against a comment in https://public-inbox.org/git/cagz79kzqsaubfotjyqm+-+ljyyec2ykj5exuy5kderezfh0...@mail.gmail.com/ noting that the Git Merge 2017 videos were not available at that time. Well, a search found them on Youtube on the GitHub channel : https://www.youtube.com/results?search_query=git+merge+2017+videos With a playlist : https://www.youtube.com/watch?v=tvymSWfvkjw&list=PL0lo9MOBetEGRAJzoTCdco_fOKDfhqaOY Enjoy the viewing. The first few have been good. Thanks for sharing this. This was in the last edition of Git Rev News: https://git.github.io/rev_news/2017/05/17/edition-27/ I'd completely missed that. It was in the Other News, various, section. (as an aside, your email is the first I've noticed that I think got swallowed by my ISP's spam filter, though Eric's PublicInbox came to the rescue with it's mbox export!)
Re: Git Merge 2017 Videos
On Sun, Jun 4, 2017 at 7:36 PM, Kevin Daudt wrote: > On Sun, Jun 04, 2017 at 11:24:17AM +0100, Philip Oakley wrote: >> While looking at the recent .gitignore issue (the need to use `**`) I came >> up against a comment in >> https://public-inbox.org/git/cagz79kzqsaubfotjyqm+-+ljyyec2ykj5exuy5kderezfh0...@mail.gmail.com/ >> noting that the Git Merge 2017 videos were not available at that time. >> >> Well, a search found them on Youtube on the GitHub channel : >> https://www.youtube.com/results?search_query=git+merge+2017+videos >> >> With a playlist : >> https://www.youtube.com/watch?v=tvymSWfvkjw&list=PL0lo9MOBetEGRAJzoTCdco_fOKDfhqaOY >> >> Enjoy the viewing. The first few have been good. > > Thanks for sharing this. This was in the last edition of Git Rev News: https://git.github.io/rev_news/2017/05/17/edition-27/
Re: [PATCH] git-p4: remove obsolete version check
On Sat, Jun 03, 2017 at 06:55:22PM +0200, Ævar Arnfjörð Bjarmason wrote:
> On Sat, Jun 3, 2017 at 3:31 PM, Jakub Wilk wrote:
> > The file is syntactically correct only in Python >= 2.6, so the
> > version check never does anything.
>
> [CC-ing Eric who added that check]
>
> Your commit message doesn't give an example of this, but with e.g.
> python 2.0 you get:
>
> File "git-p4.py", line 469
> yield pattern
> ^
> SyntaxError: invalid syntax
>
> I checked the various other python files that had similar warnings,
> they all work correctly with python 2.0.
The yield syntax was in Python 2.3, so this isn't indicative of the
problem. You'd actually need to test with 2.5 itself in order to know
what that version complains about.
> One workaround to keep this would be to make git-p4.py import some
> library to do all its work, and use some subset of python syntax to
> just load and defer to that library. That works for me when I change
> it like that locally. Alternatively, does Python have something like
> Perl's BEGIN {} blocks where you can execute code right there before
> the file has finished parsing?
TTBOMK, Python doesn't have such functionality.
> Or we could just remove this, just wanted to note the above since I
> dug into it, and the commit message light on details.
As a note, Python 2.6 has been around since at least 2009, so I think
it's fine to drop support for earlier versions at this point. But I
agree an explanation in the commit message of what exactly makes it
syntactically invalid would be beneficial.
--
brian m. carlson / brian with sandals: Houston, Texas, US
https://www.crustytoothpaste.net/~bmc | My opinion only
OpenPGP: https://keybase.io/bk2204
signature.asc
Description: PGP signature
Re: [PATCH 0/6] Some more git config completions
> As an aside from this series, has anyone ever proposed some method of > semi-automatically keeping this up-to-date? For configuration variables, not that I know of. For command line options, there was an attempt to enhance parse-options to dump all command line options and use this in the completion script on-demand to lazy-initialize command-specific variables holding the list of options: http://public-inbox.org/git/1334140165-24958-1-git-send-email-bebar...@gmail.com/T/#u > Seems we're in a continual > cycle of adding flags/config, forgetting to update this, then updating > it. At least the command-line flags should be easy to parse out in > some test, ditto config variables from config.txt maybe... A couple of thoughts concerning configuration variables: - config.txt includes other files listing configuration variables, too. - There are many config variables with subsections, e.g. 'branch..description'. That '' is not good for completion, of course. - Some config variables are not listed with their full names, see 'advice.*' (this is easy to fix). - Perhaps there are config variables that are only mentioned in the docs of the relevant command, but not in config.txt (or in the included files). - There are definitely config variables that are not mentioned in the docs at all, e.g. the 'bash.*' variables controlling __git_ps1(). - The completion script is currently self-contained and ready to be used as-is. I think that's quite nice. This wouldn't be the case if we want to include an automatically generated list of config variables extracted from config.txt during the build process.
Re: Git Merge 2017 Videos
On Sun, Jun 04, 2017 at 11:24:17AM +0100, Philip Oakley wrote: > While looking at the recent .gitignore issue (the need to use `**`) I came > up against a comment in > https://public-inbox.org/git/cagz79kzqsaubfotjyqm+-+ljyyec2ykj5exuy5kderezfh0...@mail.gmail.com/ > noting that the Git Merge 2017 videos were not available at that time. > > Well, a search found them on Youtube on the GitHub channel : > https://www.youtube.com/results?search_query=git+merge+2017+videos > > With a playlist : > https://www.youtube.com/watch?v=tvymSWfvkjw&list=PL0lo9MOBetEGRAJzoTCdco_fOKDfhqaOY > > Enjoy the viewing. The first few have been good. > Thanks for sharing this.
Re: [PATCH] submodule foreach: correct $sm_path in nested submodules from a dir
On 03/06/17 15:07, Ramsay Jones wrote: [snip] >> diff --git a/Documentation/git-submodule.txt >> b/Documentation/git-submodule.txt >> index 74bc6200d5..52e3ef1325 100644 >> --- a/Documentation/git-submodule.txt >> +++ b/Documentation/git-submodule.txt >> @@ -218,20 +218,24 @@ information too. >> >> foreach [--recursive] :: >> Evaluates an arbitrary shell command in each checked out submodule. >> -The command has access to the variables $name, $path, $sha1 and >> -$toplevel: >> -$name is the name of the relevant submodule section in .gitmodules, >> -$path is the name of the submodule directory relative to the >> -superproject, $sha1 is the commit as recorded in the superproject, >> -and $toplevel is the absolute path to the top-level of the superproject. >> -Any submodules defined in the superproject but not checked out are >> -ignored by this command. Unless given `--quiet`, foreach prints the name >> -of each submodule before evaluating the command. >> -If `--recursive` is given, submodules are traversed recursively (i.e. >> -the given shell command is evaluated in nested submodules as well). >> -A non-zero return from the command in any submodule causes >> -the processing to terminate. This can be overridden by adding '|| :' >> -to the end of the command. >> +The command has access to the following variables: >> ++ >> +* `$name` is the name of the relevant submodule section in .gitmodules, >> +* `$sha1` is the commit as recorded in the superproject. >> +* `$sm_path` is the path recorded in the superproject. > > Just to be sure, the 'path recorded in the superproject' means the > same thing as the 'name of the submodule directory relative to the > superproject'. Yes? > >> +* `$toplevel` is the absolute path to its superproject, such that >> + `$toplevel/$sm_path` is the absolute path of the submodule. >> +* `$dpath` contains the relative path from the current working directory >> + to the submodules root directory. BTW, I have not given any thought to what happens to these variables/paths if you run '--recursive'. I assume the new 'recursed' submodule takes on the role of the 'superproject' in the definitions above. Hmm, but what does $dpath show? (having 'recursed', is the cwd now at the top level of the 'new' superproject?) Hmm, you have changed several command to recurse into submodule(s) (I haven't followed that effort closely), so I suppose it should have some measure of consistency with those commands. (whatever that means ;-) ATB, Ramsay Jones
Re: Git p4 sync changelist interval
On 4 June 2017 at 10:56, Андрей Ефанов <1134t...@gmail.com> wrote: > Hello, > > My goal is to sync the repository from p4 using an interval of > changelists so that the first changelist version of the repository > would be considered as an initial commit. > So I used the following command: > > git p4 clone //depot@cl1,cl2 > > And when it finished, the files, that were created before the cl1 were > not in the HEAD. Do you mean that if foo.c was created at cl1+1, that after doing the clone, it wasn't there? If so, that doesn't sound right to me. I have just tried doing what I think you mean: 1. Create p4 depot 2. Add foo.c (at CL 2) 3. Add bar.c (at CL 3) 4. git-p4 clone //depot@2,3 I end up with both files. > > The problem, as I see it, is that before syncing changes in the given > range, p4 task does not sync to cl1 version of the repo, and applies > commits to the empty repository. > Is it a bug or my misunderstanding of how git p4 should work? Possibly I'm misunderstanding what you're doing! Can you give a sequence of steps to show the problem? Thanks! Luke
Re: [GSoC][PATCH v6 2/2] submodule: port subcommand foreach from shell to C
On Sat, Jun 3, 2017 at 7:43 AM, Stefan Beller wrote:
> On Fri, Jun 2, 2017 at 4:24 AM, Prathamesh Chavan wrote:
>> This aims to make git-submodule foreach a builtin. This is the very
>> first step taken in this direction. Hence, 'foreach' is ported to
>> submodule--helper, and submodule--helper is called from git-submodule.sh.
>> The code is split up to have one function to obtain all the list of
>> submodules. This function acts as the front-end of git-submodule foreach
>> subcommand. It calls the function for_each_submodule_list, which basically
>> loops through the list and calls function fn, which in this case is
>> runcommand_in_submodule. This third function is a calling function that
>> takes care of running the command in that submodule, and recursively
>> perform the same when --recursive is flagged.
>>
>> The first function module_foreach first parses the options present in
>> argv, and then with the help of module_list_compute, generates the list of
>> submodules present in the current working tree.
>>
>> The second function for_each_submodule_list traverses through the
>> list, and calls function fn (which in case of submodule subcommand
>> foreach is runcommand_in_submodule) is called for each entry.
>>
>> The third function runcommand_in_submodule, generates a submodule struct sub
>> for $name, value and then later prepends name=sub->name; and other
>> value assignment to the env argv_array structure of a child_process.
>> Also the of submodule-foreach is push to args argv_array
>> structure and finally, using run_command the commands are executed
>> using a shell.
>>
>> The third function also takes care of the recursive flag, by creating
>> a separate child_process structure and prepending "--super-prefix
>> displaypath",
>> to the args argv_array structure. Other required arguments and the
>> input of submodule-foreach is also appended to this argv_array.
>>
>
> Is the commit message still accurate?
> You describe the changes between the versions below the --- line,
> that is not recorded in the permanent commit history.
>
> In the commit message is less important to write "what" is happening,
> because that can easily be read from the patch/commit itself, but rather
> "why" things happen, such as design choices, maybe:
>
> This aims to make git-submodule foreach a builtin. This is the very
> first step taken in this direction. Hence, 'foreach' is ported to
> submodule--helper, and submodule--helper is called from git-submodule.sh.
>
> We'll introduce 3 functions, one that is exposed to the command line
> and handles command line arguments, one to iterate over a set of
> submodules, and finally one to execute an arbitrary shell command
> in the submodule.
>
> Attention must be paid to the 'path' variable, see 64394e3ae9
> (git-submodule.sh: Don't use $path variable in eval_gettext string,
> 2012-04-17) details. The path varialbe is not exposed into the environment
> of the invoked shell, but we just give "path=%s;" as the first argument.
>
> We do not need to condition on the number of arguments as in 1c4fb136db
> (submodule foreach: skip eval for more than one argument, 2013-09-27)
> as we will run exactly one shell in the submodules directory.
>
> Sign-off-...
>
>>
>> Other than that, additionally the case of no. of arugments in
>> being equal to 1 is also considered separetly.
>> THe reason of having this change in the shell script was given in the
>> commit 1c4fb136db.
>> According to my understanding, eval "$1" executes $1 in same shell,
>> whereas "$@" gets executed in a separate shell, which doesn't allow
>> "$@" to access the env variables $name, $path, etc.
>> Hence, to keep the ported function similar, this condition is also
>> added.
>
> This paragraph would be a good candidate for the commit message, too.
> However as we rewrite it in C, we will spawn exactly one shell no matter
> how many arguments we have (well for 0 we have no shell, but for 1 or more
> arguments we'll spawn exactly one shell?)
>
I was trying to explaing the condition of the code in git-submodule.sh,
before porting. To be more clear, I meant that when we run the command
eval "$1", it runs in the same shell in which the cmd_foreach has been running,
unlike in the case of "$@", in which case, the command in executed in a separate
shell.
>> + } else if (prefix) {
>> + struct strbuf sb = STRBUF_INIT;
>> + char *displaypath = xstrdup(relative_path(path, prefix,
>> &sb));
>> + strbuf_release(&sb);
>> + return displaypath;
>
> Note to self (or any other that is interested in long term clean code):
> I have seen this pattern a couple of times, a strbuf just to appease
> the argument list of relative_path.
> (init_submodule, prepare_to_clone_next_submodule,
> resolve_relative_path in submodule--helper
> cmd_rev_parse in builtin/rev-parse
> connect_work_tree_and_git
Git Merge 2017 Videos
While looking at the recent .gitignore issue (the need to use `**`) I came up against a comment in https://public-inbox.org/git/cagz79kzqsaubfotjyqm+-+ljyyec2ykj5exuy5kderezfh0...@mail.gmail.com/ noting that the Git Merge 2017 videos were not available at that time. Well, a search found them on Youtube on the GitHub channel : https://www.youtube.com/results?search_query=git+merge+2017+videos With a playlist : https://www.youtube.com/watch?v=tvymSWfvkjw&list=PL0lo9MOBetEGRAJzoTCdco_fOKDfhqaOY Enjoy the viewing. The first few have been good. -- Philip
Git p4 sync changelist interval
Hello, My goal is to sync the repository from p4 using an interval of changelists so that the first changelist version of the repository would be considered as an initial commit. So I used the following command: git p4 clone //depot@cl1,cl2 And when it finished, the files, that were created before the cl1 were not in the HEAD. The problem, as I see it, is that before syncing changes in the given range, p4 task does not sync to cl1 version of the repo, and applies commits to the empty repository. Is it a bug or my misunderstanding of how git p4 should work? Regards, Andrew Yefanov
Re: [PATCH] perf: work around the tested repo having an index.lock
On Sun, Jun 04, 2017 at 11:04:50AM +0900, Junio C Hamano wrote: > Jeff King writes: > > > But I think a more compelling case is that there may be an ongoing > > operation in the original repo (e.g., say you are in the middle of > > writing a commit message) when we do a blind copy of the filesystem > > contents. You might racily pick up a lockfile. > > > > Should we find and delete all *.lock files in the copied directory? That > > would get ref locks, etc. Half-formed object files are OK. Technically > > if you want to get an uncorrupted repository you'd also want to copy > > refs before objects (in case somebody makes a new object and updates a > > ref while you're copying). > > Or "git branch -m A B" is in progress. > > I think it all depends on what your "threat" model is ;-). Do we > assume that many users are "time-sharing" a box and a repository? > If not, i.e. if you are the sole user of a box and a repository on > it, such a concurrent access to make the result of git-unaware copy > problematic will not be in index.lock (after all you are now doing > the perf thing, not editing a commit log message in the repository > used for testing Git), but will be in ref locks (somebody else > pushing into the repository you are *not* currently using from > sideways). I was specifically thinking of the case where you run "git commit -a", it locks the index, and then while you are writing the commit message you need to collect some more perf results. The default perf repo is the current repository itself, so running any perf script will copy the index.lock and probably cause the script to misbehave. That doesn't seem like an implausible sequence of events (frankly, I'm surprised I haven't hit it myself, as I often run perf scripts while writing commit messages. I think I've been saved by generally using a separate linux.git clone as my test repo). So it may be reasonable to say that the index.lock is special. We hold it for a long time (compared to reflocks, which do a quick compare-and-swap). And then we can throw up our hands for any other races. People who run "git prune" on their test repository running the perf scripts get what they deserve. :) -Peff
Re: [PATCH] perf: work around the tested repo having an index.lock
On Sun, Jun 04, 2017 at 09:00:28AM +0900, Junio C Hamano wrote: > Ævar Arnfjörð Bjarmason writes: > > >> My feeling exactly. Diagnosing and failing upfront saying "well you > >> made a copy but it is not suitable for testing" sounds more sensible > >> at lesat to me. > > > > This change makes the repo suitable for testing when it wasn't before. > > Perhaps "not suitable" was a bit too vague. > > The copy you made is not in a consistent state that is good for > testing. This change may declare that it is now in a consistent > state, but removal of a single *.lock file does not make it so. We > do not know what other transient inconsistency the resulting copy > has; it is inherent to git-unaware copy---that is why we discouraged > and removed rsync transport after all. Right. What I was getting at in my original message was that this is the tip of the iceberg if we are worried about inconsistent states. And the right solution is probably to say "you are on your own for making sure the repo you point to is in a sane state". Because there are so many cases to catch, and they're so rare, it's not worth trying to catch them all. I don't really mind addressing this one case that much. I'm not sure that we want to accrue a pile of band-aids here that causes a maintenance burden and doesn't really solve the problem completely. One way to do that is to say no to the first band-aid. But we could also apply it and see what happens. At the very worst it's a few extra lines of code, and we can start to get worried on the second or third band-aid. -Peff
Re: [PATCH] perf: work around the tested repo having an index.lock
On Sun, Jun 04, 2017 at 09:55:15AM +0200, Ævar Arnfjörð Bjarmason wrote: > > Is a local clone really much slower these days? Wouldn't it is use > > hard links too? > > By the way the many properties that are preserved might not be worth > > preserving as they could make results depend a lot on the current > > state of the original repo. > > AFAICT from some quick testing it'll hardlink the objects/ dir, so > e.g. you preserve the loose objects. Making the results depend on the > state of the original repo is a feature, but perhaps it should be opt > in. It's very useful to be able to take a repo that's accrued e.g. a > month's worth of refs & loose objects and test that v.s. a fresh > clone. > > But there are other things that ever a hardlink local clone doesn't > preserve which might be worth preserving... Yes. Reflogs are one example. They aren't copied at all as part of a clone, but they impact pruning and repacking. -Peff
Re: [WIP/PATCH 7/6] perf: add a performance test for core.fsmonitor
On Sun, Jun 04, 2017 at 09:46:19AM +0200, Ævar Arnfjörð Bjarmason wrote: > What I'm referring to is not a limitation of git (we'll always be able > to turn off core.fsmonitor), but a limitation of the perf framework. > There's no way to run a test like this: > > ./run master next -- p*.sh > > And have some information passed to the test to apply different > runtime options to the test depending on master or next, or be to test > master twice, once with the fsmonitor, once without, which this > hypothetical feature would do: > > ./run master:"GIT_PERF_7519_NO_FSMONITOR=Y" master -- p*.sh Yeah, the perf framework was originally designed to find regressions between versions of Git. It's really bad at comparing results between any other dimensions. You can test different repository sizes by tweaking the environment, but it can't aggregate or compare results between those runs. Likewise, you can have two tests in a script which time Git with and without certain options set, but there's no way to compare the results of those tests. It would be nice if the perf framework was aware of all of these dimensions, stored each result as a tuple of all of the properties (rather than just the version), and then let you group the results along any dimension (I suspect there are cases where multi-dimensional summaries would be useful, but that could come later). For some dimensions you'd probably want support in the perf scripts themselves to run a test with two variants. E.g., something like: test_perf_group 'fsmonitor' \ 'false' 'git config core.fsmonitor false' \ 'true' 'git config core.fsmonitor true' test_perf 'status' 'git status' test_perf_group_end where that would run the "git status" test for each of the named setups, and store the timing results under "($VERSION, fsmonitor=false)", and so on. That's less flexible than specifying it to "./run" (which would let you run just one tuple if you chose). But it also relieves the burden from the user of figuring out which dimensions are interesting to tweak. -Peff
Re: [PATCH] perf: work around the tested repo having an index.lock
On Sun, Jun 4, 2017 at 9:37 AM, Christian Couder wrote: > On Sun, Jun 4, 2017 at 2:00 AM, Junio C Hamano wrote: >> Ævar Arnfjörð Bjarmason writes: >> My feeling exactly. Diagnosing and failing upfront saying "well you made a copy but it is not suitable for testing" sounds more sensible at lesat to me. >>> >>> This change makes the repo suitable for testing when it wasn't before. >> >> Perhaps "not suitable" was a bit too vague. >> >> The copy you made is not in a consistent state that is good for >> testing. This change may declare that it is now in a consistent >> state, but removal of a single *.lock file does not make it so. We >> do not know what other transient inconsistency the resulting copy >> has; it is inherent to git-unaware copy---that is why we discouraged >> and removed rsync transport after all. > > If we don't like git-unaware copies, maybe we should go back to the > reasons why we are making one here. > In 342e9ef2d9 (Introduce a performance testing framework, 2012-02-17), > Thomas wrote: > > 3. Creating test repos from scratch in every test is extremely >time-consuming, and shipping or downloading such large/weird repos >is out of the question. > >We leave this decision to the user. Two different sizes of test >repos can be configured, and the scripts just copy one or more of >those (using hardlinks for the object store). By default it tries >to use the build tree's git.git repository. > >This is fairly fast and versatile. Using a copy instead of a clone >preserves many properties that the user may want to test for, such >as lots of loose objects, unpacked refs, etc. > > Is a local clone really much slower these days? Wouldn't it is use > hard links too? > By the way the many properties that are preserved might not be worth > preserving as they could make results depend a lot on the current > state of the original repo. AFAICT from some quick testing it'll hardlink the objects/ dir, so e.g. you preserve the loose objects. Making the results depend on the state of the original repo is a feature, but perhaps it should be opt in. It's very useful to be able to take a repo that's accrued e.g. a month's worth of refs & loose objects and test that v.s. a fresh clone. But there are other things that ever a hardlink local clone doesn't preserve which might be worth preserving...
Re: [WIP/PATCH 7/6] perf: add a performance test for core.fsmonitor
On Sun, Jun 4, 2017 at 3:59 AM, Junio C Hamano wrote: > Ævar Arnfjörð Bjarmason writes: > >> This is WIP code for the reasons explained in the setup comments, >> unfortunately the perf code doesn't easily allow you to run different >> setup code for different versions you're testing. This test will stop >> working if the fsmonitor is merged into the master branch. >> ... >> + >> + # Relies on core.fsmonitor not being merged into master. Needs >> + # better per-test ways to disable it if it gets merged. >> + git config core.fsmonitor true && > > Will stop working and relies on not merged can be read but I cannot > read "why" explained, and I cannot quite guess what the reason is. > > If the code to read the configuration is not there, setting this > would not have any effect. If the code is there, setting this would > have effect (either talking fsmonitor helps or it hurts). > > And I do not think we'd ever see a version of Git that always relies > on talking to fsmonitor, i.e. "git config core.fsmonitor false" is not > a way to disable it, so... > > Puzzled. Sorry about the unclear brevity. What I'm referring to is not a limitation of git (we'll always be able to turn off core.fsmonitor), but a limitation of the perf framework. There's no way to run a test like this: ./run master next -- p*.sh And have some information passed to the test to apply different runtime options to the test depending on master or next, or be to test master twice, once with the fsmonitor, once without, which this hypothetical feature would do: ./run master:"GIT_PERF_7519_NO_FSMONITOR=Y" master -- p*.sh So right now the test works because there's no core.fsmonitor in master, so turning it on all the time only impacts avar/fsmonitor, not master. I started trying to add this to the perf framework the other day but ran out of time, the option should also be passed down early enough to be intercepted by the GIT_PERF_MAKE_COMMAND, so you could do e.g.: GIT_PERF_MAKE_COMMAND='make CFLAGS="$F"' \ ./run v2.13.0:"F=-O0" v2.13.0:"F=-O1" v2.13.0:"F=-O2" v2.13.0:"F=-O3" -- p*.sh To test the same revision with different compilation flags. A change like this would break the ability to enact certain perf optimizations, right now we unpack the revision(s) you specify into , and e.g. make use of the fact that that directory is already unpacked so we don't need to unpack it again. If there was no way to pass a flag to specific revisions being tested, then perf could just optimize: ./run v2.12.0 v2.13.0 b06d364310 -- p*.sh To skip the b06d364310 run entirely since it's the same as v2.13.0. I think breaking this minor assumption in the framework is fine, but it's worth noting that it's an assumption we couldn't make anymore.
Re: [PATCH] perf: work around the tested repo having an index.lock
On Sun, Jun 4, 2017 at 2:00 AM, Junio C Hamano wrote: > Ævar Arnfjörð Bjarmason writes: > >>> My feeling exactly. Diagnosing and failing upfront saying "well you >>> made a copy but it is not suitable for testing" sounds more sensible >>> at lesat to me. >> >> This change makes the repo suitable for testing when it wasn't before. > > Perhaps "not suitable" was a bit too vague. > > The copy you made is not in a consistent state that is good for > testing. This change may declare that it is now in a consistent > state, but removal of a single *.lock file does not make it so. We > do not know what other transient inconsistency the resulting copy > has; it is inherent to git-unaware copy---that is why we discouraged > and removed rsync transport after all. If we don't like git-unaware copies, maybe we should go back to the reasons why we are making one here. In 342e9ef2d9 (Introduce a performance testing framework, 2012-02-17), Thomas wrote: 3. Creating test repos from scratch in every test is extremely time-consuming, and shipping or downloading such large/weird repos is out of the question. We leave this decision to the user. Two different sizes of test repos can be configured, and the scripts just copy one or more of those (using hardlinks for the object store). By default it tries to use the build tree's git.git repository. This is fairly fast and versatile. Using a copy instead of a clone preserves many properties that the user may want to test for, such as lots of loose objects, unpacked refs, etc. Is a local clone really much slower these days? Wouldn't it is use hard links too? By the way the many properties that are preserved might not be worth preserving as they could make results depend a lot on the current state of the original repo.
Re: [PATCH] test-lib: add ability to cap the runtime of tests
On Sun, Jun 4, 2017 at 2:31 AM, Junio C Hamano wrote:
> Ævar Arnfjörð Bjarmason writes:
>
>> Speeding up the test suite by simply cataloging and skipping tests
>> that take longer than N seconds is a hassle to maintain, and entirely
>> skips some tests which would be nice to at least partially run,
>> e.g. instead of entirely skipping t3404-rebase-interactive.sh we can
>> run it for N seconds and get at least some "git rebase -i" test
>> coverage in a fast test run.
>
> I'd be more supportive to the former approach in the longer run for
> two reasons.
>
> Is it even safe to stop a test in the middle? Won't we leave
> leftover server processes, for example?
>
> I see start_httpd at least sets up "trap" to call stop_httpd
> when the shell exits, so HTTP testing via lib-httpd.sh may be
> safe. I do not know about other network-y tests, though.
When this flag is in effect and you run into the timeout the code is
semantically equivalent to not running subsequent test_expect_*
blocks, things like the trap in lib-httpd.sh will still run, so will
test_when_finished.
Unless we have some test killing a daemon in a test_expect_success
block later in the test this'll work as intended.
> Granted, when a test fails, we already have the same problem, but
> then we'd go in and investigate, and the first thing we notice would
> be that the old leftover server instance is holding onto the port to
> prevent the attempt to re-run the test from running, which then we'd
> kill. But with this option, the user is not even made aware of
> tests being killed in the middle.
>
>> While running with a timeout of 10 seconds cuts the runtime in half,
>> over 92% of the tests are still run. The test coverage is higher than
>> that number indicates, just taking into account the many similar tests
>> t0027-auto-crlf.sh runs brings it up to 95%.
>
> I certainly understand that but in the longer term, I'd prefer the
> approach to call out an overly large test. That will hopefully
> motivate us to split it (or speed up the thing) to help folks on
> many-core machines.
The reason I didn't document this in t/README was because I thought it
made sense to have this as a mostly hidden feature that end users
wouldn't be tempted to fiddle with, but would be useful to someone
doing git development.
Realistically I'm going to submit this patch, I'm not going to take
the much bigger project of refactoring the entire test suite so that
no test runs under N second, and of course any such refactoring can
only aim for a fixed instead of dynamic N.
The point of this change is that I can replace running e.g. "prove
t[0-9]*{grep,log}*.sh" with just running the full test suite every
time, since 30s is noticeably slow during regular hacking but once
it's down to 15s it's perceptively fast enough.
Reading between the lines in your reply, I think you're afraid that
regular users just testing git out will start using this, as opposed
to power user developers who understand the trade-offs. I think that's
mostly mitigated by not documenting it in t/README, but I could amend
the patch to add some scary commend to test-lib.sh as well.
> I am afraid that the proposed change will disincentivize that by
> sweeping the problematic ones under the rug. Perhaps you can
> collect what tests are terminated in the middle because they run for
> too long and show the list of them at the end, or something?
This change incentivizes me to be regularly running a larger % of the
full test suite.
Collecting the skipped ones is easy enough to do with a grep + for
loop, so I don't think it's worth making the implementation more
complex to occasionally answer the question of how many tests were
skipped due to running into the timeout:
$ rm .prove; for t in 20 10 5 1; do printf "%s\t" $t && (time
GIT_TEST_TIMEOUT=$t prove -j$(parallel --number-of-cores)
--state=slow,save -v t[0-9]*.sh) 2>&1 | grep -c "Exceeded
GIT_TEST_TIMEOUT"; done
rm: cannot remove ‘.prove’: No such file or directory
20 4
10 36
5 80
1 509
Of course that gives you "how many tests had skipped tests", now how
many test_expect_* blocks were skipped. An earlier WIP version of this
did the former, but e.g. running the rest of t0027-auto-crlf.sh took
many seconds just do spew out hundreds/thousands of lines in a shell
loop emitting "skip" lines, so I went with the to_skip=all
implementation.
> Also, I thought that it was a no-no to say "to_skil=all" with
> skipped-reason in the middle of a test when the test is run under
> prove?
TAP has two main modes of operation, you can either declare that
you're going to run N tests in advance and then you must run N, this
makes prove report progress on your tests as they run.
Or you can just run in a mode where you stream out however many tests
you're going to run as you go along, and then print "1..NUM_TESTS" at
the end.
We use the latter, so we can abort the entire test suite at any time
with test_done, that's what this change
