Re: A git hook that does git cherry-pick and push automatically
> so it's a sever side hook that it's forced down to all developers. How does that work? /me is just curious -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
bug in bash completion for git-branch --set-upstream-to on OSX
There appears to be a bug in the bash completion for git-branch when attempting to complete the remote ref argument for --set-upstream-to= When: $ git branch --set-upstream-to=origin/mast I would expect it to complete to: $ git branch --set-upstream-to=origin/master However, the completion for --set-upstream-to= completes the ref correctly, but completely wipes the --set-upstream option; resulting in: $ git branch origin/master I'm running on OS X 10.9.5 with git from homebrew: $ bash --version GNU bash, version 4.3.33(1)-release (x86_64-apple-darwin13.4.0) $ git --version git version 2.3.3 The same behavior does *not* manifest (it works as expected) on CentOS 6.5, bash 4.1.2.1 (GNU bash, version 4.1.2(1)-release (x86_64-redhat-linux-gnu)). I'm running git 2.0.3 on CentOs but sourcing the shell completion script from latest source: 9ab698f I also cloned down latest git source on OS X and the bug still manifests when sourcing the completion script at 9ab698f. ~ Jason -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] t1700: make test pass with index-v4
The different index versions have different sha-1 checksums. Those checksums are checked in t1700, which makes it fail when run with index v4. Fix it. Signed-off-by: Thomas Gummerer --- t/t1700-split-index.sh | 15 --- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/t/t1700-split-index.sh b/t/t1700-split-index.sh index 94fb473..92f7298 100755 --- a/t/t1700-split-index.sh +++ b/t/t1700-split-index.sh @@ -10,9 +10,18 @@ sane_unset GIT_TEST_SPLIT_INDEX test_expect_success 'enable split index' ' git update-index --split-index && test-dump-split-index .git/index >actual && + indexversion=$(test-index-version <.git/index) && + if test "$indexversion" = "4" + then + own=432ef4b63f32193984f339431fd50ca796493569 + base=508851a7f0dfa8691e9f69c7f055865389012491 + else + own=8299b0bcd1ac364e5f1d7768efb62fa2da79a339 + base=39d890139ee5356c7ef572216cebcd27aa41f9df + fi && cat >expectexpect
[PATCH 27/27] t9104: fix test for following larger parents
This test is special for several reasons: It ends with a "true" statement, which should be a no-op. It is not because the &&-chain is broken right before it. Also, looking at what the test intended to test according to 7f578c5 (git-svn: --follow-parent now works on sub-directories of larger branches, 2007-01-24) it is not clear how it would achieve that with the given steps. Amend the test to include the second svn id to be tested for, and change the tested refs to the ones which are to be expected, and which make the test pass. Signed-off-by: Michael J Gruber --- In fact, these merge-base tests look kind of ugly, too. But they appear in many more places here and should best be rewritten separately. t/t9104-git-svn-follow-parent.sh | 10 ++ 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/t/t9104-git-svn-follow-parent.sh b/t/t9104-git-svn-follow-parent.sh index 13b179e..1bd1022 100755 --- a/t/t9104-git-svn-follow-parent.sh +++ b/t/t9104-git-svn-follow-parent.sh @@ -72,16 +72,18 @@ test_expect_success 'follow larger parent' ' svn import -m "import a larger parent" import "$svnrepo"/larger-parent && svn cp -m "hi" "$svnrepo"/larger-parent "$svnrepo"/another-larger && git svn init --minimize-url -i larger \ - "$svnrepo"/another-larger/trunk/thunk/bump/thud && + "$svnrepo"/larger-parent/trunk/thunk/bump/thud && git svn fetch -i larger && +git svn init --minimize-url -i larger-parent \ + "$svnrepo"/another-larger/trunk/thunk/bump/thud && +git svn fetch -i larger-parent && git rev-parse --verify refs/remotes/larger && git rev-parse --verify \ - refs/remotes/larger-parent/trunk/thunk/bump/thud && + refs/remotes/larger-parent && test "`git merge-base \ - refs/remotes/larger-parent/trunk/thunk/bump/thud \ + refs/remotes/larger-parent \ refs/remotes/larger`" = \ "`git rev-parse refs/remotes/larger`" -true ' test_expect_success 'follow higher-level parent' ' -- 2.3.3.438.g7254779 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 26/27] t/*svn*: fix moderate &&-chain breakage
All of these cases are moderate since they would most probably not lead to missed failing tests: Either they would fail otherwise, or fail a rm in test_when_finished only. Signed-off-by: Michael J Gruber --- t/t2026-prune-linked-checkouts.sh | 4 ++-- t/t9158-git-svn-mergeinfo.sh | 4 ++-- t/t9161-git-svn-mergeinfo-push.sh | 10 +- 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/t/t2026-prune-linked-checkouts.sh b/t/t2026-prune-linked-checkouts.sh index 2936d52..e885baf 100755 --- a/t/t2026-prune-linked-checkouts.sh +++ b/t/t2026-prune-linked-checkouts.sh @@ -65,7 +65,7 @@ test_expect_success 'prune directories with gitdir pointing to nowhere' ' ' test_expect_success 'not prune locked checkout' ' - test_when_finished rm -r .git/worktrees + test_when_finished rm -r .git/worktrees && mkdir -p .git/worktrees/ghi && : >.git/worktrees/ghi/locked && git prune --worktrees && @@ -73,7 +73,7 @@ test_expect_success 'not prune locked checkout' ' ' test_expect_success 'not prune recent checkouts' ' - test_when_finished rm -r .git/worktrees + test_when_finished rm -r .git/worktrees && mkdir zz && mkdir -p .git/worktrees/jlm && echo "$(pwd)"/zz >.git/worktrees/jlm/gitdir && diff --git a/t/t9158-git-svn-mergeinfo.sh b/t/t9158-git-svn-mergeinfo.sh index 8c9539e..13f78f2 100755 --- a/t/t9158-git-svn-mergeinfo.sh +++ b/t/t9158-git-svn-mergeinfo.sh @@ -34,7 +34,7 @@ test_expect_success 'change svn:mergeinfo' ' ' test_expect_success 'verify svn:mergeinfo' ' - mergeinfo=$(svn_cmd propget svn:mergeinfo "$svnrepo"/trunk) + mergeinfo=$(svn_cmd propget svn:mergeinfo "$svnrepo"/trunk) && test "$mergeinfo" = "/branches/foo:1-10" ' @@ -46,7 +46,7 @@ test_expect_success 'change svn:mergeinfo multiline' ' ' test_expect_success 'verify svn:mergeinfo multiline' ' - mergeinfo=$(svn_cmd propget svn:mergeinfo "$svnrepo"/trunk) + mergeinfo=$(svn_cmd propget svn:mergeinfo "$svnrepo"/trunk) && test "$mergeinfo" = "/branches/bar:1-10 /branches/other:3-5,8,10-11" ' diff --git a/t/t9161-git-svn-mergeinfo-push.sh b/t/t9161-git-svn-mergeinfo-push.sh index 6cb0909..f113aca 100755 --- a/t/t9161-git-svn-mergeinfo-push.sh +++ b/t/t9161-git-svn-mergeinfo-push.sh @@ -24,7 +24,7 @@ test_expect_success 'propagate merge information' ' ' test_expect_success 'check svn:mergeinfo' ' - mergeinfo=$(svn_cmd propget svn:mergeinfo "$svnrepo"/branches/svnb1) + mergeinfo=$(svn_cmd propget svn:mergeinfo "$svnrepo"/branches/svnb1) && test "$mergeinfo" = "/branches/svnb2:3,8" ' @@ -34,7 +34,7 @@ test_expect_success 'merge another branch' ' ' test_expect_success 'check primary parent mergeinfo respected' ' - mergeinfo=$(svn_cmd propget svn:mergeinfo "$svnrepo"/branches/svnb1) + mergeinfo=$(svn_cmd propget svn:mergeinfo "$svnrepo"/branches/svnb1) && test "$mergeinfo" = "/branches/svnb2:3,8 /branches/svnb3:4,9" ' @@ -45,7 +45,7 @@ test_expect_success 'merge existing merge' ' ' test_expect_success "check both parents' mergeinfo respected" ' - mergeinfo=$(svn_cmd propget svn:mergeinfo "$svnrepo"/branches/svnb1) + mergeinfo=$(svn_cmd propget svn:mergeinfo "$svnrepo"/branches/svnb1) && test "$mergeinfo" = "/branches/svnb2:3,8 /branches/svnb3:4,9 /branches/svnb4:5-6,10-12 @@ -70,7 +70,7 @@ test_expect_success 'second forward merge' ' ' test_expect_success 'check new mergeinfo added' ' - mergeinfo=$(svn_cmd propget svn:mergeinfo "$svnrepo"/branches/svnb1) + mergeinfo=$(svn_cmd propget svn:mergeinfo "$svnrepo"/branches/svnb1) && test "$mergeinfo" = "/branches/svnb2:3,8,16-17 /branches/svnb3:4,9 /branches/svnb4:5-6,10-12 @@ -84,7 +84,7 @@ test_expect_success 'reintegration merge' ' ' test_expect_success 'check reintegration mergeinfo' ' - mergeinfo=$(svn_cmd propget svn:mergeinfo "$svnrepo"/branches/svnb4) + mergeinfo=$(svn_cmd propget svn:mergeinfo "$svnrepo"/branches/svnb4) && test "$mergeinfo" = "/branches/svnb1:2-4,7-9,13-18 /branches/svnb2:3,8,16-17 /branches/svnb3:4,9 -- 2.3.3.438.g7254779 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/25] detecting &&-chain breakage
Jeff King venit, vidit, dixit 20.03.2015 11:04: > This is a cleanup of the &&-chain lint I posted earlier: > > http://thread.gmane.org/gmane.comp.version-control.git/265613/focus=265859 > > I don't know who came up with the idea for it originally, but the > concept certainly was floating in the back of my mind from Jonathan's > earlier version that is referenced in that thread. > > The general idea is to detect &&-chain breakage that can lead to our > tests yielding false success. The first patch implements and discusses > the lint-check itself, which is quite simple. The bulk of the work was > fixing all of the issues in the existing tests. :) > > That didn't all need to happen immediately. I mainly wanted to start on > it to answer two questions: > > 1. Are &&-chain breakages actually preventing us from seeing any test > failures? Or is it mostly just pedantry, and we miss out only on > knowing whether "cat >expect <<-\EOF" failed (which presumably it > never does). > > 2. How bad are the false positives? Both how common, and how bad to > work around. > > But after a few hours, I reached a zen state and just kept going. So at > the end of this series, the whole test suite is --chain-lint clean > (modulo any tests that are skipped on my platform). We could even switch > the checks on by default at the end of the series, but I didn't do that > here. I think it would be sane to run them all the time, though; in the > normal success case, they don't add any forks (the shell just runs > "(exit) && ...", and realizes that the whole thing is one big &&-chain). > I couldn't measure any time difference running the suite with and > without it. > > Anyway, to answer the questions: Yes, there were definitely tests whose > values were being thrown away, and we would not have noticed if they > failed. The good news is that all of them did pass once we started > checking their results. Hooray. > > There were a number of false positives, though as a percentage of the > test suite, probably not many (it's just that we have quite a lot of > tests). Most of them were in rather old tests, and IMHO the fixes I did > actually improved the readability of the result. So overall I think this > is a very positive change; I doubt it will get in people's way very > often, and I look forward to having one less thing to worry about > handling manually in review. The biggest downside is that I may have > automated Eric Sunshine out of a job. :) > > The patches are: > > [01/25]: t/test-lib: introduce --chain-lint option > [02/25]: t: fix severe &&-chain breakage > [03/25]: t: fix moderate &&-chain breakage > [04/25]: t: fix trivial &&-chain breakage > [05/25]: t: assume test_cmp produces verbose output > [06/25]: t: use verbose instead of hand-rolled errors > [07/25]: t: use test_must_fail instead of hand-rolled blocks > [08/25]: t: fix &&-chaining issues around setup which might fail > [09/25]: t: use test_might_fail for diff and grep > [10/25]: t: use test_expect_code instead of hand-rolled comparison > [11/25]: t: wrap complicated expect_code users in a block > [12/25]: t: avoid using ":" for comments > [13/25]: t3600: fix &&-chain breakage for setup commands > [14/25]: t7201: fix &&-chain breakage > [15/25]: t9502: fix &&-chain breakage > [16/25]: t6030: use modern test_* helpers > [17/25]: t0020: use modern test_* helpers > [18/25]: t1301: use modern test_* helpers > [19/25]: t6034: use modern test_* helpers > [20/25]: t4117: use modern test_* helpers > [21/25]: t9001: use test_when_finished > [22/25]: t0050: appease --chain-lint > [23/25]: t7004: fix embedded single-quotes > [24/25]: t0005: fix broken &&-chains > [25/25]: t4104: drop hand-rolled error reporting > > It's a lot of patches, and a few of them are long. I tried to group > them by functionality rather than file (though as you can see, some of > the tests were unique enough snowflakes that it made sense to discuss > their issues separately). I'm hoping it should be an easy review, if > not a short one. > > I'll spare you the full per-file diffstat, but the total is a very > satisfying: > >84 files changed, 397 insertions(+), 647 deletions(-) > > That's a lot of changes in a lot of hunks, but most of them are in files > that haven't been touched in a while. The series merges cleanly with > "pu", so I don't think I've stepped on anyone's topics in flight. > > -Peff Nice series! In fact, it's a great place to learn many of the test suite features that we have nowadays. With 1/25 only, I get 163 dubious or failed on current next. With 1/25 and only chain-lint without running the actual test loads, I get 213. So, just as Jeff explained, we don't want a "chain-lint-only mode" because it does not give correct results. I reviewed the first 15 and they all look good. I skimmed through the rest and it appears good to (though this is no review). You run without git svn tests, obvi
possible minor bug: 'git apply --reject' unnecessarily touches files
Hi I'm doing some splitting of a large patch and I noticed that when I do $ git apply --reject large_patch.diff and it generates a lot of .rej files, it seems to be touching the files that produced the .rej output, even if nothing in that particular file was actually updated. Not a major issue, but it would help my build times, which are considerable when working on LibreOffice :-) Thanks, Noel Grandin Disclaimer: http://www.peralex.com/disclaimer.html -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4] rev-list: refuse --first-parent combined with --bisect
On Mon, Mar 16, 2015 at 11:53:08AM -0700, Junio C Hamano wrote: > Because the history is not linear in Git, bisection works by > shrinking a subgraph of the history DAG that contains "yet to be > tested, suspected to have introduced a badness" commits. The > subgraph is defined as anything reachable from _the_ "bad" commit > (initially, the one you give to the command when you start) that are > not reachable from any of the "good" commits. > > Suppose you started from this graph. Time flows left to right as > usual. > > ---0---2---4---6---8---9 > \ / >1---3---5---7 > > Then mark the initial good and bad commits as G and B. > > ---G---2---4---6---8---B > \ / >1---3---5---7 > > And imagine that you are asked to check 4, which turns out to be > good. We do not _move_ G to 4; we mark 4 as good, while keeping > 0 also as good. > > ---G---2---G---6---8---B > \ / >1---3---5---7 > > And if you are next asked to check 5, and mark it as good, the graph > will become like this: > > ---G---2---G---6---8---B > \ / >1---3---G---7 > > Of course, at this point, the subgraph of suspects are 6, 7, 8 and > 9, and the subgraph no longer is affected by the fact that 0 is > good. But it is crucial to keep 0 marked as good in the step before > this one, before you tested 5, as that is what allows us not having > to test any ancestors of 0 at all. > > Now, one may wonder why we need multiple "good" commits but we do > not need multiple "bad" commits. This comes from the nature of > "bisection", which is a tool to find a _single_ breakage [*1*], and > a fundamental assumption is that a breakage does not fix itself. > > Hence, if you have a history that looks like this: > > >G...1---2---3---4---6---8---B > \ > 5---7---B > > it follows that 4 must also be "bad". It used to be good long time > ago somewhere before 1, and somewhere along way on the history, > there was a single breakage event that we are hunting for. That > single event cannot be 5, 6, 7 or 8 because breakage at say 5 would > not explain why the tip of the upper branch is broken---its breakage > has no way to propagate there. The breakage must have happened at 4 > or before that commit. But what if 7 & 8 are the same patch, cherry-picked? Or nearly the same patch, but with some conflict resolution? Couldn't that lead to the case that 4, 5, and 6 are good, while 7 & 8 are bad? Or does that violate the "single breakage" rule in a way that might be too subtle for some users? -- Scott Schmit smime.p7s Description: S/MIME cryptographic signature
[PATCH] Documentation: Add target to build PDF manpages
Signed-off-by: Thomas Schneider --- dblatex does print some warnings, but they seem to be irrelevant. Besides, first patch I submit to git or even to any project using a mailing list … let’s hope I did everything right :) Documentation/Makefile | 9 - 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/Documentation/Makefile b/Documentation/Makefile index 3e39e28..158cbd9 100644 --- a/Documentation/Makefile +++ b/Documentation/Makefile @@ -38,6 +38,7 @@ MAN7_TXT += gitworkflows.txt MAN_TXT = $(MAN1_TXT) $(MAN5_TXT) $(MAN7_TXT) MAN_XML = $(patsubst %.txt,%.xml,$(MAN_TXT)) MAN_HTML = $(patsubst %.txt,%.html,$(MAN_TXT)) +MAN_PDF = $(patsubst %.txt,%.pdf,$(MAN_TXT)) OBSOLETE_HTML += everyday.html OBSOLETE_HTML += git-remote-helpers.html @@ -223,7 +224,7 @@ man7: $(DOC_MAN7) info: git.info gitman.info -pdf: user-manual.pdf +pdf: user-manual.pdf $(MAN_PDF) install: install-man @@ -248,6 +249,7 @@ install-info: info install-pdf: pdf $(INSTALL) -d -m 755 $(DESTDIR)$(pdfdir) $(INSTALL) -m 644 user-manual.pdf $(DESTDIR)$(pdfdir) + $(INSTALL) -m 644 $(MAN_PDF) $(DESTDIR)$(pdfdir) install-html: html '$(SHELL_PATH_SQ)' ./install-webdoc.sh $(DESTDIR)$(htmldir) @@ -328,6 +330,11 @@ manpage-base-url.xsl: manpage-base-url.xsl.in $(QUIET_XMLTO)$(RM) $@ && \ $(XMLTO) -m $(MANPAGE_XSL) $(XMLTO_EXTRA) man $< +%.pdf: %.xml + $(QUIET_DBLATEX)$(RM) $@+ $@ && \ + $(DBLATEX) -o $@+ -p $(ASCIIDOC_DBLATEX_DIR)/asciidoc-dblatex.xsl -s $(ASCIIDOC_DBLATEX_DIR)/asciidoc-dblatex.sty $< && \ + mv $@+ $@ + %.xml : %.txt asciidoc.conf $(QUIET_ASCIIDOC)$(RM) $@+ $@ && \ $(TXT_TO_XML) -d manpage -o $@+ $< && \ -- 2.1.0 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Git ignore help
Hello, I am trying to setup my git ignore (resp. .git/info/exclude) so that I exclude all directories and files except the content of directories that I specifically include (incl. anything within them recursively). I set the .git/info/exclude with the following content: # Exclude everything /* # Except the below that we include !/db/data/load/base/bootstraponly !/db/data/load/base/safetoload !/db/ddl !/labels !/reports/usrint !/scripts !/src/cmdsrc/usrint However it does not do what I anticipated. It indeed excludes everything but the include part does not work - it only works for !/labels and !/scripts directories (i.e. the first level directories). All other are still ignored - so when I create file /db/data/load/base/bootstraponly/somefile.txt git still ignores it... Any idea what I am doing wrong? I tried all of the following combinations but still with the same result: !/db/data/load/base/bootstraponly !/db/data/load/base/bootstraponly/ !/db/data/load/base/bootstraponly/* !/db/data/load/base/bootstraponly/** When I add just "!/db" to exclude then it works. But that's not what I want - I really want to include just the /db/data/load/base/bootstraponly and its all content... Thank you, Martin -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/25] detecting &&-chain breakage
On Fri, Mar 20, 2015 at 06:04:30AM -0400, Jeff King wrote: > It's a lot of patches, and a few of them are long. I tried to group > them by functionality rather than file (though as you can see, some of > the tests were unique enough snowflakes that it made sense to discuss > their issues separately). I'm hoping it should be an easy review, if > not a short one. I hoped that would make it easier to review, but I was also curious about the patterns I would see. Here are a few things I noticed: - the single most common place to forget an "&&" was at the start of a here-doc I complained earlier that I would prefer a way to put it at the end. And indeed, I found somebody who agreed with me and was more clever than I. They wrote in one test: (cat >expect < foo" redirects when I was touching those particular lines :) ). Which is all to say, please don't mention ugly style issues you see in the context lines during review, unless it is to point me to your patch series that comes on top of mine and cleans them up. :) -Peff -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 18/25] t1301: use modern test_* helpers
This shortens the code and fixes some &&-chaining. Signed-off-by: Jeff King --- t/t1301-shared-repo.sh | 20 +++- 1 file changed, 7 insertions(+), 13 deletions(-) diff --git a/t/t1301-shared-repo.sh b/t/t1301-shared-repo.sh index 7eecfb8..ac10875 100755 --- a/t/t1301-shared-repo.sh +++ b/t/t1301-shared-repo.sh @@ -12,12 +12,11 @@ setfacl -k . 2>/dev/null # User must have read permissions to the repo -> failure on --shared=0400 test_expect_success 'shared = 0400 (faulty permission u-w)' ' + test_when_finished "rm -rf sub" && mkdir sub && ( - cd sub && git init --shared=0400 + cd sub && + test_must_fail git init --shared=0400 ) - ret="$?" - rm -rf sub - test $ret != "0" ' modebits () { @@ -33,7 +32,7 @@ do git init --shared=1 && test 1 = "$(git config core.sharedrepository)" ) && - actual=$(ls -l sub/.git/HEAD) + actual=$(ls -l sub/.git/HEAD) && case "$actual" in -rw-rw-r--*) : happy @@ -90,10 +89,8 @@ do rm -f .git/info/refs && git update-server-info && actual="$(modebits .git/info/refs)" && - test "x$actual" = "x-$y" || { - ls -lt .git/info - false - } + verbose test "x$actual" = "x-$y" + ' umask 077 && @@ -102,10 +99,7 @@ do rm -f .git/info/refs && git update-server-info && actual="$(modebits .git/info/refs)" && - test "x$actual" = "x-$x" || { - ls -lt .git/info - false - } + verbose test "x$actual" = "x-$x" ' -- 2.3.3.520.g3cfbb5d -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 14/25] t7201: fix &&-chain breakage
One of these breakages is in setup, but one is more severe and may miss a real test failure. These are pulled out from the rest, though, because we also clean up a few other anachronisms. The most interesting is the use of this here-doc construct: (cat >... < --- t/t7201-co.sh | 19 ++- 1 file changed, 6 insertions(+), 13 deletions(-) diff --git a/t/t7201-co.sh b/t/t7201-co.sh index a7fe4e6..8859236 100755 --- a/t/t7201-co.sh +++ b/t/t7201-co.sh @@ -88,14 +88,10 @@ test_expect_success "checkout with unrelated dirty tree without -m" ' git checkout -f master && fill 0 1 2 3 4 5 6 7 8 >same && - cp same kept + cp same kept && git checkout side >messages && - test_cmp same kept - (cat > messages.expectexpect.messages expect.master && @@ -409,12 +402,12 @@ test_expect_success \ test_expect_success \ 'checkout w/autosetupmerge=always sets up tracking' ' +test_when_finished git config branch.autosetupmerge false && git config branch.autosetupmerge always && git checkout master && git checkout -b track2 && test "$(git config branch.track2.remote)" && -test "$(git config branch.track2.merge)" -git config branch.autosetupmerge false' +test "$(git config branch.track2.merge)"' test_expect_success 'checkout w/--track from non-branch HEAD fails' ' git checkout master^0 && -- 2.3.3.520.g3cfbb5d -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
git@vger.kernel.org
The ":" noop command always returns true, so it is fine to include these lines in an &&-chain (and it appeases --chain-lint). Signed-off-by: Jeff King --- t/t0005-signals.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/t/t0005-signals.sh b/t/t0005-signals.sh index 5c5707d..e7f27eb 100755 --- a/t/t0005-signals.sh +++ b/t/t0005-signals.sh @@ -40,12 +40,12 @@ test_expect_success 'create blob' ' ' test_expect_success !MINGW 'a constipated git dies with SIGPIPE' ' - OUT=$( ((large_git; echo $? 1>&3) | :) 3>&1 ) + OUT=$( ((large_git; echo $? 1>&3) | :) 3>&1 ) && test "$OUT" -eq 141 ' test_expect_success !MINGW 'a constipated git dies with SIGPIPE even if parent ignores it' ' - OUT=$( ((trap "" PIPE; large_git; echo $? 1>&3) | :) 3>&1 ) + OUT=$( ((trap "" PIPE; large_git; echo $? 1>&3) | :) 3>&1 ) && test "$OUT" -eq 141 ' -- 2.3.3.520.g3cfbb5d -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 23/25] t7004: fix embedded single-quotes
This test uses single quotes inside the single-quoted test snippet, which effectively makes the contents unquoted. Since they don't need quoted anyway, this isn't a problem, but let's switch them to double-quotes to make it more obviously correct. Signed-off-by: Jeff King --- t/t7004-tag.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/t/t7004-tag.sh b/t/t7004-tag.sh index 347d3be..efb08c3 100755 --- a/t/t7004-tag.sh +++ b/t/t7004-tag.sh @@ -1181,7 +1181,7 @@ test_expect_success \ 'message in editor has initial comment: remainder' ' # remove commented lines from the remainder -- should be empty >rest.expect && - sed -e 1d -e '/^#/d' rest.actual && + sed -e 1d -e "/^#/d" rest.actual && test_cmp rest.expect rest.actual ' -- 2.3.3.520.g3cfbb5d -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 21/25] t9001: use test_when_finished
The confirmation tests in t9001 all save the value of sendemail.confirm, do something to it, then restore it at the end, in a way that breaks the &&-chain (they are not wrong, because they save the $? value, but it fools --chain-lint). Instead, they can all use test_when_finished, and we can even make the code simpler by factoring out the shared lines. Note that we can _almost_ use test_config here, except that: 1. We do not restore the config with test_unconfig, but by setting it back to some prior value. 2. We are not always setting a config variable. Sometimes the change to be undone is unsetting it entirely. We could teach test_config to handle these cases, but it's not worth the complexity for a single call-site. Signed-off-by: Jeff King --- t/t9001-send-email.sh | 30 ++ 1 file changed, 10 insertions(+), 20 deletions(-) diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh index 37caa18..c9f54d5 100755 --- a/t/t9001-send-email.sh +++ b/t/t9001-send-email.sh @@ -817,26 +817,25 @@ test_expect_success $PREREQ '--confirm=compose' ' test_confirm --confirm=compose --compose ' -test_expect_success $PREREQ 'confirm by default (due to cc)' ' +save_confirm () { CONFIRM=$(git config --get sendemail.confirm) && + test_when_finished "git config sendemail.confirm ${CONFIRM:-never}" +} + +test_expect_success $PREREQ 'confirm by default (due to cc)' ' + save_confirm && git config --unset sendemail.confirm && test_confirm - ret="$?" - git config sendemail.confirm ${CONFIRM:-never} - test $ret = "0" ' test_expect_success $PREREQ 'confirm by default (due to --compose)' ' - CONFIRM=$(git config --get sendemail.confirm) && + save_confirm && git config --unset sendemail.confirm && test_confirm --suppress-cc=all --compose - ret="$?" - git config sendemail.confirm ${CONFIRM:-never} - test $ret = "0" ' test_expect_success $PREREQ 'confirm detects EOF (inform assumes y)' ' - CONFIRM=$(git config --get sendemail.confirm) && + save_confirm && git config --unset sendemail.confirm && rm -fr outdir && git format-patch -2 -o outdir && @@ -846,13 +845,10 @@ test_expect_success $PREREQ 'confirm detects EOF (inform assumes y)' ' --to=nob...@example.com \ --smtp-server="$(pwd)/fake.sendmail" \ outdir/*.patch http://vger.kernel.org/majordomo-info.html
[PATCH 22/25] t0050: appease --chain-lint
Some of the symlink tests check an either-or case using the "||". This is not wrong, but fools --chain-lint into thinking the &&-chain is broken (in fact, there is no && chain here). We can solve this by wrapping the "||" inside a {} block. This is a bit more verbose, but this construct is rare, and the {} block helps call attention to it. Signed-off-by: Jeff King --- t/t0050-filesystem.sh | 12 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/t/t0050-filesystem.sh b/t/t0050-filesystem.sh index 988c392..b29d749 100755 --- a/t/t0050-filesystem.sh +++ b/t/t0050-filesystem.sh @@ -33,16 +33,20 @@ test_expect_success "detection of case insensitive filesystem during repo init" ' else test_expect_success "detection of case insensitive filesystem during repo init" ' - test_must_fail git config --bool core.ignorecase >/dev/null || - test $(git config --bool core.ignorecase) = false + { + test_must_fail git config --bool core.ignorecase >/dev/null || + test $(git config --bool core.ignorecase) = false + } ' fi if test_have_prereq SYMLINKS then test_expect_success "detection of filesystem w/o symlink support during repo init" ' - test_must_fail git config --bool core.symlinks || - test "$(git config --bool core.symlinks)" = true + { + test_must_fail git config --bool core.symlinks || + test "$(git config --bool core.symlinks)" = true + } ' else test_expect_success "detection of filesystem w/o symlink support during repo init" ' -- 2.3.3.520.g3cfbb5d -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 15/25] t9502: fix &&-chain breakage
This script misses a trivial &&-chain in one of its tests, but it also has a weird reverse: it includes an &&-chain outside of any test_expect block! This "cat" should never fail, but if it did, we would not notice, as it would cause us to skip the follow-on test entirely (which does not appear intentional; there are many later tests which rely on this cat). Let's instead move the setup into its own test_expect_success block, which is the standard practice nowadays. Signed-off-by: Jeff King --- t/t9502-gitweb-standalone-parse-output.sh | 10 ++ 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/t/t9502-gitweb-standalone-parse-output.sh b/t/t9502-gitweb-standalone-parse-output.sh index 86dfee2..a93e159 100755 --- a/t/t9502-gitweb-standalone-parse-output.sh +++ b/t/t9502-gitweb-standalone-parse-output.sh @@ -145,9 +145,11 @@ test_expect_success 'forks: not skipped unless "forks" feature enabled' ' grep -q ">fork of .*<" gitweb.body ' -cat >>gitweb_config.perl <<\EOF && -$feature{'forks'}{'default'} = [1]; -EOF +test_expect_success 'enable forks feature' ' + cat >>gitweb_config.perl <<-\EOF + $feature{'forks'}{'default'} = [1]; + EOF +' test_expect_success 'forks: forks skipped if "forks" feature enabled' ' gitweb_run "a=project_list" && @@ -173,7 +175,7 @@ test_expect_success 'forks: can access forked repository' ' ' test_expect_success 'forks: project_index lists all projects (incl. forks)' ' - cat >expected <<-\EOF + cat >expected <<-\EOF && .git foo.bar.git foo.git -- 2.3.3.520.g3cfbb5d -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 19/25] t6034: use modern test_* helpers
These say roughly the same thing as the hand-rolled messages. We do lose the "merge did not complete" debug message, but merge and write-tree are prefectly capable of writing useful error messages when they fail. Signed-off-by: Jeff King --- t/t6034-merge-rename-nocruft.sh | 66 - 1 file changed, 13 insertions(+), 53 deletions(-) diff --git a/t/t6034-merge-rename-nocruft.sh b/t/t6034-merge-rename-nocruft.sh index 65be95f..89871aa 100755 --- a/t/t6034-merge-rename-nocruft.sh +++ b/t/t6034-merge-rename-nocruft.sh @@ -73,33 +73,12 @@ test_expect_success 'merge white into red (A->B,M->N)' \ ' git checkout -b red-white red && git merge white && - git write-tree >/dev/null || { - echo "BAD: merge did not complete" - return 1 - } - - test -f B || { - echo "BAD: B does not exist in working directory" - return 1 - } - test -f N || { - echo "BAD: N does not exist in working directory" - return 1 - } - test -f R || { - echo "BAD: R does not exist in working directory" - return 1 - } - - test -f A && { - echo "BAD: A still exists in working directory" - return 1 - } - test -f M && { - echo "BAD: M still exists in working directory" - return 1 - } - return 0 + git write-tree && + test_path_is_file B && + test_path_is_file N && + test_path_is_file R && + test_path_is_missing A && + test_path_is_missing M ' # This test broke in 8371234ecaaf6e14fe3f2082a855eff1bbd79ae9 @@ -108,32 +87,13 @@ test_expect_success 'merge blue into white (A->B, mod A, A untracked)' \ git checkout -b white-blue white && echo dirty >A && git merge blue && - git write-tree >/dev/null || { - echo "BAD: merge did not complete" - return 1 - } - - test -f A || { - echo "BAD: A does not exist in working directory" - return 1 - } - test `cat A` = dirty || { - echo "BAD: A content is wrong" - return 1 - } - test -f B || { - echo "BAD: B does not exist in working directory" - return 1 - } - test -f N || { - echo "BAD: N does not exist in working directory" - return 1 - } - test -f M && { - echo "BAD: M still exists in working directory" - return 1 - } - return 0 + git write-tree && + test_path_is_file A && + echo dirty >expect && + test_cmp expect A && + test_path_is_file B && + test_path_is_file N && + test_path_is_missing M ' test_done -- 2.3.3.520.g3cfbb5d -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 25/25] t4104: drop hand-rolled error reporting
This use of "||" fools --chain-lint into thinking the &&-chain is broken (and indeed, it is somewhat broken; a failure of update-index in these tests would show the patch file, even if we never got to the part of the test where we fed the patch to git-apply). The extra blocks were there to include more debugging output, but it hardly seems worth it; the user should know which command failed (because git-apply will produce error messages) and can look in the trash directory themselves. Signed-off-by: Jeff King --- t/t4104-apply-boundary.sh | 10 ++ 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/t/t4104-apply-boundary.sh b/t/t4104-apply-boundary.sh index 497afdc..32e3b0e 100755 --- a/t/t4104-apply-boundary.sh +++ b/t/t4104-apply-boundary.sh @@ -95,10 +95,7 @@ do test_expect_success "apply $kind-patch $with context" ' cat original >victim && git update-index victim && - git apply --index '"$u$kind-patch.$with"' || { - cat '"$kind-patch.$with"' - (exit 1) - } && + git apply --index '"$u$kind-patch.$with"' && test_cmp '"$kind"'-expect victim ' done @@ -113,10 +110,7 @@ do test_expect_success "apply non-git $kind-patch without context" ' cat original >victim && git update-index victim && - git apply --unidiff-zero --index '"$kind-ng.without"' || { - cat '"$kind-ng.without"' - (exit 1) - } && + git apply --unidiff-zero --index '"$kind-ng.without"' && test_cmp '"$kind"'-expect victim ' done -- 2.3.3.520.g3cfbb5d -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 16/25] t6030: use modern test_* helpers
We can get rid of a lot of hand-rolled error messages by using test_must_fail and test_expect_code. The existing code was careful to use "|| return 1" when breaking the &&-chain, but it did fool --chain-lint; the new code is more idiomatic. We also add some uses of test_when_finished, which is less cryptic and more robust than putting code at the end of a test. In two cases we run "git bisect reset" from a subshell, which is a problem for test_when_finished (it would not run). However, in both of these cases, we are performing the tests in one-off sub-repos, so we do not need to clean up at all (and in fact it is nicer not to if the user wants to inspect the trash directory after a failure). Signed-off-by: Jeff King --- t/t6030-bisect-porcelain.sh | 91 +++-- 1 file changed, 31 insertions(+), 60 deletions(-) diff --git a/t/t6030-bisect-porcelain.sh b/t/t6030-bisect-porcelain.sh index e6abe65..06b4868 100755 --- a/t/t6030-bisect-porcelain.sh +++ b/t/t6030-bisect-porcelain.sh @@ -52,15 +52,8 @@ test_expect_success 'bisect starts with only one bad' ' test_expect_success 'bisect does not start with only one good' ' git bisect reset && git bisect start && - git bisect good $HASH1 || return 1 - - if git bisect next - then - echo Oops, should have failed. - false - else - : - fi + git bisect good $HASH1 && + test_must_fail git bisect next ' test_expect_success 'bisect start with one bad and good' ' @@ -191,34 +184,27 @@ test_expect_success 'bisect start: no ".git/BISECT_START" if checkout error' ' # but $HASH2 is bad, # so we should find $HASH2 as the first bad commit test_expect_success 'bisect skip: successful result' ' + test_when_finished git bisect reset && git bisect reset && git bisect start $HASH4 $HASH1 && git bisect skip && git bisect bad > my_bisect_log.txt && - grep "$HASH2 is the first bad commit" my_bisect_log.txt && - git bisect reset + grep "$HASH2 is the first bad commit" my_bisect_log.txt ' # $HASH1 is good, $HASH4 is bad, we skip $HASH3 and $HASH2 # so we should not be able to tell the first bad commit # among $HASH2, $HASH3 and $HASH4 test_expect_success 'bisect skip: cannot tell between 3 commits' ' + test_when_finished git bisect reset && git bisect start $HASH4 $HASH1 && - git bisect skip || return 1 - - if git bisect skip > my_bisect_log.txt - then - echo Oops, should have failed. - false - else - test $? -eq 2 && - grep "first bad commit could be any of" my_bisect_log.txt && - ! grep $HASH1 my_bisect_log.txt && - grep $HASH2 my_bisect_log.txt && - grep $HASH3 my_bisect_log.txt && - grep $HASH4 my_bisect_log.txt && - git bisect reset - fi + git bisect skip && + test_expect_code 2 git bisect skip >my_bisect_log.txt && + grep "first bad commit could be any of" my_bisect_log.txt && + ! grep $HASH1 my_bisect_log.txt && + grep $HASH2 my_bisect_log.txt && + grep $HASH3 my_bisect_log.txt && + grep $HASH4 my_bisect_log.txt ' # $HASH1 is good, $HASH4 is bad, we skip $HASH3 @@ -226,22 +212,15 @@ test_expect_success 'bisect skip: cannot tell between 3 commits' ' # so we should not be able to tell the first bad commit # among $HASH3 and $HASH4 test_expect_success 'bisect skip: cannot tell between 2 commits' ' + test_when_finished git bisect reset && git bisect start $HASH4 $HASH1 && - git bisect skip || return 1 - - if git bisect good > my_bisect_log.txt - then - echo Oops, should have failed. - false - else - test $? -eq 2 && - grep "first bad commit could be any of" my_bisect_log.txt && - ! grep $HASH1 my_bisect_log.txt && - ! grep $HASH2 my_bisect_log.txt && - grep $HASH3 my_bisect_log.txt && - grep $HASH4 my_bisect_log.txt && - git bisect reset - fi + git bisect skip && + test_expect_code 2 git bisect good >my_bisect_log.txt && + grep "first bad commit could be any of" my_bisect_log.txt && + ! grep $HASH1 my_bisect_log.txt && + ! grep $HASH2 my_bisect_log.txt && + grep $HASH3 my_bisect_log.txt && + grep $HASH4 my_bisect_log.txt ' # $HASH1 is good, $HASH4 is both skipped and bad, we skip $HASH3 @@ -249,24 +228,18 @@ test_expect_success 'bisect skip: cannot tell between 2 commits' ' # so we should not be able to tell the first bad commit # among $HASH3 and $HASH4 test_expect_success 'bisect skip: with commit both bad and skipped' ' + test_when_finished git bisect reset && git bisect start && git bisect skip && git bisect bad
[PATCH 20/25] t4117: use modern test_* helpers
We can use test_must_fail and test_path_* to avoid some hand-rolled if statements. This makes the code shorter, and makes it more obvious when we are breaking the &&-chain. Signed-off-by: Jeff King --- t/t4117-apply-reject.sh | 66 - 1 file changed, 10 insertions(+), 56 deletions(-) diff --git a/t/t4117-apply-reject.sh b/t/t4117-apply-reject.sh index 8e15ecb..d80187d 100755 --- a/t/t4117-apply-reject.sh +++ b/t/t4117-apply-reject.sh @@ -56,23 +56,13 @@ test_expect_success 'apply --reject is incompatible with --3way' ' test_expect_success 'apply without --reject should fail' ' - if git apply patch.1 - then - echo "Eh? Why?" - exit 1 - fi - + test_must_fail git apply patch.1 && test_cmp file1 saved.file1 ' test_expect_success 'apply without --reject should fail' ' - if git apply --verbose patch.1 - then - echo "Eh? Why?" - exit 1 - fi - + test_must_fail git apply --verbose patch.1 && test_cmp file1 saved.file1 ' @@ -81,21 +71,11 @@ test_expect_success 'apply with --reject should fail but update the file' ' cat saved.file1 >file1 && rm -f file1.rej file2.rej && - if git apply --reject patch.1 - then - echo "succeeds with --reject?" - exit 1 - fi - + test_must_fail git apply --reject patch.1 && test_cmp file1 expected && cat file1.rej && - - if test -f file2.rej - then - echo "file2 should not have been touched" - exit 1 - fi + test_path_is_missing file2.rej ' test_expect_success 'apply with --reject should fail but update the file' ' @@ -103,25 +83,12 @@ test_expect_success 'apply with --reject should fail but update the file' ' cat saved.file1 >file1 && rm -f file1.rej file2.rej file2 && - if git apply --reject patch.2 >rejects - then - echo "succeeds with --reject?" - exit 1 - fi - - test -f file1 && { - echo "file1 still exists?" - exit 1 - } + test_must_fail git apply --reject patch.2 >rejects && + test_path_is_missing file1 && test_cmp file2 expected && cat file2.rej && - - if test -f file1.rej - then - echo "file2 should not have been touched" - exit 1 - fi + test_path_is_missing file1.rej ' @@ -130,25 +97,12 @@ test_expect_success 'the same test with --verbose' ' cat saved.file1 >file1 && rm -f file1.rej file2.rej file2 && - if git apply --reject --verbose patch.2 >rejects - then - echo "succeeds with --reject?" - exit 1 - fi - - test -f file1 && { - echo "file1 still exists?" - exit 1 - } + test_must_fail git apply --reject --verbose patch.2 >rejects && + test_path_is_missing file1 && test_cmp file2 expected && cat file2.rej && - - if test -f file1.rej - then - echo "file2 should not have been touched" - exit 1 - fi + test_path_is_missing file1.rej ' -- 2.3.3.520.g3cfbb5d -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 17/25] t0020: use modern test_* helpers
This test contains a lot of hand-rolled messages to show when the test fails. We can omit most of these by using "verbose" and "test_must_fail". A few of them are for update-index, but we can assume it produces reasonable error messages when it fails. Signed-off-by: Jeff King --- t/t0020-crlf.sh | 144 +++- 1 file changed, 28 insertions(+), 116 deletions(-) diff --git a/t/t0020-crlf.sh b/t/t0020-crlf.sh index d2e51a8..9fa26df 100755 --- a/t/t0020-crlf.sh +++ b/t/t0020-crlf.sh @@ -104,18 +104,12 @@ test_expect_success 'update with autocrlf=input' ' for f in one dir/two do append_cr <$f >tmp && mv -f tmp $f && - git update-index -- $f || { - echo Oops - false - break - } + git update-index -- $f || + break done && differs=$(git diff-index --cached HEAD) && - test -z "$differs" || { - echo Oops "$differs" - false - } + verbose test -z "$differs" ' @@ -128,18 +122,12 @@ test_expect_success 'update with autocrlf=true' ' for f in one dir/two do append_cr <$f >tmp && mv -f tmp $f && - git update-index -- $f || { - echo "Oops $f" - false - break - } + git update-index -- $f || + break done && differs=$(git diff-index --cached HEAD) && - test -z "$differs" || { - echo Oops "$differs" - false - } + verbose test -z "$differs" ' @@ -152,19 +140,13 @@ test_expect_success 'checkout with autocrlf=true' ' for f in one dir/two do remove_cr <"$f" >tmp && mv -f tmp $f && - git update-index -- $f || { - echo "Eh? $f" - false - break - } + verbose git update-index -- $f || + break done && test "$one" = $(git hash-object --stdin .gitattributes && git read-tree --reset -u HEAD && - if has_cr dir/two - then - : happy - else - echo "Huh?" - false - fi && - - if has_cr three - then - : happy - else - echo "Huh?" - false - fi + verbose has_cr dir/two && + verbose has_cr three ' test_expect_success 'in-tree .gitattributes (1)' ' @@ -352,17 +300,8 @@ test_expect_success 'in-tree .gitattributes (1)' ' rm -rf tmp one dir .gitattributes patch.file three && git read-tree --reset -u HEAD && - if has_cr one - then - echo "Eh? one should not have CRLF" - false - else - : happy - fi && - has_cr three || { - echo "Eh? three should still have CRLF" - false - } + test_must_fail has_cr one && + verbose has_cr three ' test_expect_success 'in-tree .gitattributes (2)' ' @@ -371,17 +310,8 @@ test_expect_success 'in-tree .gitattributes (2)' ' git read-tree --reset HEAD && git checkout-index -f -q -u -a && - if has_cr one - then - echo "Eh? one should not have CRLF" - false - else - : happy - fi && - has_cr three || { - echo "Eh? three should still have CRLF" - false - } + test_must_fail has_cr one && + verbose has_cr three ' test_expect_success 'in-tree .gitattributes (3)' ' @@ -391,17 +321,8 @@ test_expect_success 'in-tree .gitattributes (3)' ' git checkout-index -u .gitattributes && git checkout-index -u one dir/two three && - if has_cr one - then - echo "Eh? one should not have CRLF" - false - else - : happy - fi && - has_cr three || { - echo "Eh? three should still have CRLF" - false - } + test_must_fail has_cr one && + verbose has_cr three ' test_expect_success 'in-tree .gitattributes (4)' ' @@ -411,17 +332,8 @@ test_expect_success 'in-tree .gitattributes (4)' ' git checkout-index -u one dir/two three && git checkout-index -u .gitattributes && - if has_cr one - then - echo "Eh? one should not have CRLF" - false - else - : happy - fi && - has_cr three || { - echo "Eh? three should still have CRLF" - false - } + test_must_fail has_cr one && + verbose has_cr three ' test_expect_success 'checkout with existing .gitattributes' ' -- 2.3.3.520.g3cfbb5d -- To unsubscribe from this list: se
[PATCH 11/25] t: wrap complicated expect_code users in a block
If we are expecting a command to produce a particular exit code, we can use test_expect_code. However, some cases are more complicated, and want to accept one of a range of exit codes. For these, we end up with something like: cmd; case "$?" in ... That unfortunately breaks the &&-chain and fools --chain-lint. Since these special cases are so few, we can wrap them in a block, like this: { cmd; ret=$?; } && case "$ret" in ... This accomplishes the same thing, and retains the &&-chain (the exit status fed to the && is that of the assignment, which should always be true). It's technically longer, but it is probably a good thing for unusual code like this to stand out. Signed-off-by: Jeff King --- t/t0005-signals.sh | 4 ++-- t/t4026-color.sh| 6 +++--- t/t5004-archive-corner-cases.sh | 6 -- t/t5512-ls-remote.sh| 6 -- 4 files changed, 13 insertions(+), 9 deletions(-) diff --git a/t/t0005-signals.sh b/t/t0005-signals.sh index aeea50c..5c5707d 100755 --- a/t/t0005-signals.sh +++ b/t/t0005-signals.sh @@ -10,8 +10,8 @@ one EOF test_expect_success 'sigchain works' ' - test-sigchain >actual - case "$?" in + { test-sigchain >actual; ret=$?; } && + case "$ret" in 143) true ;; # POSIX w/ SIGTERM=15 271) true ;; # ksh w/ SIGTERM=15 3) true ;; # Windows diff --git a/t/t4026-color.sh b/t/t4026-color.sh index 4d20fea..2b32c4f 100755 --- a/t/t4026-color.sh +++ b/t/t4026-color.sh @@ -111,9 +111,9 @@ test_expect_success 'unknown color slots are ignored (branch)' ' ' test_expect_success 'unknown color slots are ignored (status)' ' - git config color.status.nosuchslotwilleverbedefined white || exit - git status - case $? in 0|1) : ok ;; *) false ;; esac + git config color.status.nosuchslotwilleverbedefined white && + { git status; ret=$?; } && + case $ret in 0|1) : ok ;; *) false ;; esac ' test_done diff --git a/t/t5004-archive-corner-cases.sh b/t/t5004-archive-corner-cases.sh index 305bcac..654adda 100755 --- a/t/t5004-archive-corner-cases.sh +++ b/t/t5004-archive-corner-cases.sh @@ -66,8 +66,10 @@ test_expect_success UNZIP 'zip archive of empty tree is empty' ' # handle the empty repo at all, making our later check of its exit code # a no-op). But we cannot do anything reasonable except skip the test # on such platforms anyway, and this is the moral equivalent. - "$GIT_UNZIP" "$TEST_DIRECTORY"/t5004/empty.zip - expect_code=$? + { + "$GIT_UNZIP" "$TEST_DIRECTORY"/t5004/empty.zip + expect_code=$? + } && git archive --format=zip HEAD >empty.zip && make_dir extract && diff --git a/t/t5512-ls-remote.sh b/t/t5512-ls-remote.sh index 321c3e5..3bd9759 100755 --- a/t/t5512-ls-remote.sh +++ b/t/t5512-ls-remote.sh @@ -103,8 +103,10 @@ test_expect_success 'confuses pattern as remote when no remote specified' ' ' test_expect_success 'die with non-2 for wrong repository even with --exit-code' ' - git ls-remote --exit-code ./no-such-repository ;# not && - status=$? && + { + git ls-remote --exit-code ./no-such-repository + status=$? + } && test $status != 2 && test $status != 0 ' -- 2.3.3.520.g3cfbb5d -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 12/25] t: avoid using ":" for comments
The ":" is not a comment marker, but rather a noop command. Using it as a comment like: : do something cmd1 && : something else cmd2 breaks the &&-chain, and we would fail to notice if "cmd1" failed in this instance. We can just use regular "#" comments instead. Signed-off-by: Jeff King --- t/t4104-apply-boundary.sh | 18 +- t/t5533-push-cas.sh | 2 +- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/t/t4104-apply-boundary.sh b/t/t4104-apply-boundary.sh index c97aad1..497afdc 100755 --- a/t/t4104-apply-boundary.sh +++ b/t/t4104-apply-boundary.sh @@ -18,7 +18,7 @@ test_expect_success setup ' cat victim >original && git update-index --add victim && - : add to the head + # add to the head for i in a b '"$L"' y do echo $i @@ -27,7 +27,7 @@ test_expect_success setup ' git diff victim >add-a-patch.with && git diff --unified=0 >add-a-patch.without && - : insert at line two + # insert at line two for i in b a '"$L"' y do echo $i @@ -36,7 +36,7 @@ test_expect_success setup ' git diff victim >insert-a-patch.with && git diff --unified=0 >insert-a-patch.without && - : modify at the head + # modify at the head for i in a '"$L"' y do echo $i @@ -45,16 +45,16 @@ test_expect_success setup ' git diff victim >mod-a-patch.with && git diff --unified=0 >mod-a-patch.without && - : remove from the head + # remove from the head for i in '"$L"' y do echo $i done >victim && cat victim >del-a-expect && - git diff victim >del-a-patch.with + git diff victim >del-a-patch.with && git diff --unified=0 >del-a-patch.without && - : add to the tail + # add to the tail for i in b '"$L"' y z do echo $i @@ -63,7 +63,7 @@ test_expect_success setup ' git diff victim >add-z-patch.with && git diff --unified=0 >add-z-patch.without && - : modify at the tail + # modify at the tail for i in b '"$L"' z do echo $i @@ -72,7 +72,7 @@ test_expect_success setup ' git diff victim >mod-z-patch.with && git diff --unified=0 >mod-z-patch.without && - : remove from the tail + # remove from the tail for i in b '"$L"' do echo $i @@ -81,7 +81,7 @@ test_expect_success setup ' git diff victim >del-z-patch.with && git diff --unified=0 >del-z-patch.without - : done + # done ' for with in with without diff --git a/t/t5533-push-cas.sh b/t/t5533-push-cas.sh index dccf8a6..c402d8d 100755 --- a/t/t5533-push-cas.sh +++ b/t/t5533-push-cas.sh @@ -14,7 +14,7 @@ setup_srcdst_basic () { } test_expect_success setup ' - : create template repository + # create template repository test_commit A && test_commit B && test_commit C -- 2.3.3.520.g3cfbb5d -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 13/25] t3600: fix &&-chain breakage for setup commands
As with the earlier patch to fix "trivial" &&-chain breakage, these missing "&&" operators are not a serious problem (e.g., we do not expect "echo" to fail). Ironically, however, inserting them shows that some of the commands _do_ fail. Specifically, some of the tests start by making sure we are at a commit with the string "content" in the file "foo". However, running "git commit" may fail because the previous test left us in that state already, and there is nothing to commit. We could remove these commands entirely, but they serve to document the test's assumptions, as well as make it robust when an earlier test has failed. We could use test_might_fail to handle all cases, but that would miss an unrelated failure to make the commit. Instead, we can just pass the --allow-empty flag to git-commit, which means that it will not complain if our setup is a noop. Signed-off-by: Jeff King --- t/t3600-rm.sh | 36 ++-- 1 file changed, 18 insertions(+), 18 deletions(-) diff --git a/t/t3600-rm.sh b/t/t3600-rm.sh index 1962989..9d90d2c 100755 --- a/t/t3600-rm.sh +++ b/t/t3600-rm.sh @@ -38,37 +38,37 @@ test_expect_success \ test_expect_success \ 'Test that git rm --cached foo succeeds if the index matches the file' \ -'echo content > foo - git add foo +'echo content >foo && + git add foo && git rm --cached foo' test_expect_success \ 'Test that git rm --cached foo succeeds if the index matches the file' \ -'echo content > foo - git add foo - git commit -m foo - echo "other content" > foo +'echo content >foo && + git add foo && + git commit -m foo && + echo "other content" >foo && git rm --cached foo' test_expect_success \ 'Test that git rm --cached foo fails if the index matches neither the file nor HEAD' ' - echo content > foo - git add foo - git commit -m foo - echo "other content" > foo - git add foo - echo "yet another content" > foo + echo content >foo && + git add foo && + git commit -m foo --allow-empty && + echo "other content" >foo && + git add foo && + echo "yet another content" >foo && test_must_fail git rm --cached foo ' test_expect_success \ 'Test that git rm --cached -f foo works in case where --cached only did not' \ -'echo content > foo - git add foo - git commit -m foo - echo "other content" > foo - git add foo - echo "yet another content" > foo +'echo content >foo && + git add foo && + git commit -m foo --allow-empty && + echo "other content" >foo && + git add foo && + echo "yet another content" >foo && git rm --cached -f foo' test_expect_success \ -- 2.3.3.520.g3cfbb5d -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 10/25] t: use test_expect_code instead of hand-rolled comparison
This makes our output in the event of a failure slightly nicer, and it means that we do not break the &&-chain. Signed-off-by: Jeff King --- t/t0040-parse-options.sh | 12 +++-- t/t4035-diff-quiet.sh| 66 +++- t/t4053-diff-no-index.sh | 4 +-- 3 files changed, 38 insertions(+), 44 deletions(-) diff --git a/t/t0040-parse-options.sh b/t/t0040-parse-options.sh index a90c86b..b044785 100755 --- a/t/t0040-parse-options.sh +++ b/t/t0040-parse-options.sh @@ -172,12 +172,9 @@ test_expect_success 'long options' ' ' test_expect_success 'missing required value' ' - test-parse-options -s; - test $? = 129 && - test-parse-options --string; - test $? = 129 && - test-parse-options --file; - test $? = 129 + test_expect_code 129 test-parse-options -s && + test_expect_code 129 test-parse-options --string && + test_expect_code 129 test-parse-options --file ' cat > expect << EOF @@ -227,8 +224,7 @@ test_expect_success 'unambiguously abbreviated option with "="' ' ' test_expect_success 'ambiguously abbreviated option' ' - test-parse-options --strin 123; - test $? = 129 + test_expect_code 129 test-parse-options --strin 123 ' cat > expect << EOF diff --git a/t/t4035-diff-quiet.sh b/t/t4035-diff-quiet.sh index e8ae2a0..461f4bb 100755 --- a/t/t4035-diff-quiet.sh +++ b/t/t4035-diff-quiet.sh @@ -29,67 +29,65 @@ test_expect_success 'setup' ' ' test_expect_success 'git diff-tree HEAD^ HEAD' ' - git diff-tree --quiet HEAD^ HEAD >cnt - test $? = 1 && test_line_count = 0 cnt + test_expect_code 1 git diff-tree --quiet HEAD^ HEAD >cnt && + test_line_count = 0 cnt ' test_expect_success 'git diff-tree HEAD^ HEAD -- a' ' - git diff-tree --quiet HEAD^ HEAD -- a >cnt - test $? = 0 && test_line_count = 0 cnt + test_expect_code 0 git diff-tree --quiet HEAD^ HEAD -- a >cnt && + test_line_count = 0 cnt ' test_expect_success 'git diff-tree HEAD^ HEAD -- b' ' - git diff-tree --quiet HEAD^ HEAD -- b >cnt - test $? = 1 && test_line_count = 0 cnt + test_expect_code 1 git diff-tree --quiet HEAD^ HEAD -- b >cnt && + test_line_count = 0 cnt ' # this diff outputs one line: sha1 of the given head test_expect_success 'echo HEAD | git diff-tree --stdin' ' - echo $(git rev-parse HEAD) | git diff-tree --quiet --stdin >cnt - test $? = 1 && test_line_count = 1 cnt + echo $(git rev-parse HEAD) | + test_expect_code 1 git diff-tree --quiet --stdin >cnt && + test_line_count = 1 cnt ' test_expect_success 'git diff-tree HEAD HEAD' ' - git diff-tree --quiet HEAD HEAD >cnt - test $? = 0 && test_line_count = 0 cnt + test_expect_code 0 git diff-tree --quiet HEAD HEAD >cnt && + test_line_count = 0 cnt ' test_expect_success 'git diff-files' ' - git diff-files --quiet >cnt - test $? = 0 && test_line_count = 0 cnt + test_expect_code 0 git diff-files --quiet >cnt && + test_line_count = 0 cnt ' test_expect_success 'git diff-index --cached HEAD' ' - git diff-index --quiet --cached HEAD >cnt - test $? = 0 && test_line_count = 0 cnt + test_expect_code 0 git diff-index --quiet --cached HEAD >cnt && + test_line_count = 0 cnt ' test_expect_success 'git diff-index --cached HEAD^' ' - git diff-index --quiet --cached HEAD^ >cnt - test $? = 1 && test_line_count = 0 cnt + test_expect_code 1 git diff-index --quiet --cached HEAD^ >cnt && + test_line_count = 0 cnt ' test_expect_success 'git diff-index --cached HEAD^' ' echo text >>b && echo 3 >c && - git add . && { - git diff-index --quiet --cached HEAD^ >cnt - test $? = 1 && test_line_count = 0 cnt - } + git add . && + test_expect_code 1 git diff-index --quiet --cached HEAD^ >cnt && + test_line_count = 0 cnt ' test_expect_success 'git diff-tree -Stext HEAD^ HEAD -- b' ' - git commit -m "text in b" && { - git diff-tree --quiet -Stext HEAD^ HEAD -- b >cnt - test $? = 1 && test_line_count = 0 cnt - } + git commit -m "text in b" && + test_expect_code 1 git diff-tree --quiet -Stext HEAD^ HEAD -- b >cnt && + test_line_count = 0 cnt ' test_expect_success 'git diff-tree -Snot-found HEAD^ HEAD -- b' ' - git diff-tree --quiet -Snot-found HEAD^ HEAD -- b >cnt - test $? = 0 && test_line_count = 0 cnt + test_expect_code 0 git diff-tree --quiet -Snot-found HEAD^ HEAD -- b >cnt && + test_line_count = 0 cnt ' test_expect_success 'git diff-files' ' - echo 3 >>c && { - git diff-files --quiet >cnt - test $? = 1 && test_line_count = 0 cnt - } + echo 3 >>c && + test_expect_code 1 git diff-files --quiet >cnt && + test_line_count = 0 cnt ' + test_expect_success 'git diff-index --cached HEA
[PATCH 09/25] t: use test_might_fail for diff and grep
Some tests run diff or grep to produce an output, and then compare the output to an expected value. We know the exit code we expect these processes to have (e.g., grep yields 0 if it produced output and 1 otherwise), so it would not make the test wrong to look for it. But the difference between their output and the expected output (e.g., shown by test_cmp) is much more useful to somebody debugging the test than the test just bailing out. These tests break the &&-chain to skip the exit-code check of the process. However, we can get the same effect by using test_might_fail. Note that in some cases the test did use "|| return 1", which meant the test was not wrong, but it did fool --chain-lint. Signed-off-by: Jeff King --- t/t1002-read-tree-m-u-2way.sh | 8 t/t5550-http-fetch-dumb.sh| 4 ++-- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/t/t1002-read-tree-m-u-2way.sh b/t/t1002-read-tree-m-u-2way.sh index fed877b..e3bf821 100755 --- a/t/t1002-read-tree-m-u-2way.sh +++ b/t/t1002-read-tree-m-u-2way.sh @@ -75,8 +75,8 @@ test_expect_success \ echo yomin >yomin && git update-index --add yomin && read_tree_u_must_succeed -m -u $treeH $treeM && - git ls-files --stage >4.out || return 1 - git diff -U0 --no-index M.out 4.out >4diff.out + git ls-files --stage >4.out && + test_might_fail git diff -U0 --no-index M.out 4.out >4diff.out && compare_change 4diff.out expected && check_cache_at yomin clean && sum bozbar frotz nitfol >actual4.sum && @@ -94,8 +94,8 @@ test_expect_success \ git update-index --add yomin && echo yomin yomin >yomin && read_tree_u_must_succeed -m -u $treeH $treeM && - git ls-files --stage >5.out || return 1 - git diff -U0 --no-index M.out 5.out >5diff.out + git ls-files --stage >5.out && + test_might_fail git diff -U0 --no-index M.out 5.out >5diff.out && compare_change 5diff.out expected && check_cache_at yomin dirty && sum bozbar frotz nitfol >actual5.sum && diff --git a/t/t5550-http-fetch-dumb.sh b/t/t5550-http-fetch-dumb.sh index 61e087e..3d11b7a 100755 --- a/t/t5550-http-fetch-dumb.sh +++ b/t/t5550-http-fetch-dumb.sh @@ -184,8 +184,8 @@ test_expect_success 'fetch can handle previously-fetched .idx files' ' ' test_expect_success 'did not use upload-pack service' ' - grep '/git-upload-pack' <"$HTTPD_ROOT_PATH"/access.log >act - : >exp + test_might_fail grep '/git-upload-pack' <"$HTTPD_ROOT_PATH"/access.log >act && + : >exp && test_cmp exp act ' -- 2.3.3.520.g3cfbb5d -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 08/25] t: fix &&-chaining issues around setup which might fail
Many tests have an initial setup step that might fail based on whether earlier tests in the script have succeeded or not. Using a trick like "|| true" breaks the &&-chain, missing earlier failures (and fooling --chain-lint). We can use test_might_fail in some cases, which is correct and makes the intent more obvious. We can also use test_unconfig for unsetting config (and which is more robust, as well). The case in t9500 is an oddball. It wants to run cmd1 _or_ cmd2, and does it like: cmd1 || cmd2 && other_stuff It's not wrong in this case, but it's a bad habit to get into, because it breaks the &&-chain if used anywhere except at the beginning of the test (and we use the correct solution here, putting it inside a block for precedence). Signed-off-by: Jeff King --- t/t5503-tagfollow.sh | 4 ++-- t/t6032-merge-large-rename.sh | 6 +++--- t/t7201-co.sh | 2 +- t/t7508-status.sh | 2 +- t/t9500-gitweb-standalone-no-errors.sh | 5 - 5 files changed, 11 insertions(+), 8 deletions(-) diff --git a/t/t5503-tagfollow.sh b/t/t5503-tagfollow.sh index f30c038..4ca48f0 100755 --- a/t/t5503-tagfollow.sh +++ b/t/t5503-tagfollow.sh @@ -139,8 +139,8 @@ EOF ' test_expect_success 'new clone fetch master and tags' ' - git branch -D cat - rm -f $U + test_might_fail git branch -D cat && + rm -f $U && ( mkdir clone2 && cd clone2 && diff --git a/t/t6032-merge-large-rename.sh b/t/t6032-merge-large-rename.sh index 15beecc..0f79268 100755 --- a/t/t6032-merge-large-rename.sh +++ b/t/t6032-merge-large-rename.sh @@ -28,10 +28,10 @@ make_text() { test_rename() { test_expect_success "rename ($1, $2)" ' - n='$1' - expect='$2' + n='$1' && + expect='$2' && git checkout -f master && - git branch -D test$n || true && + test_might_fail git branch -D test$n && git reset --hard initial && for i in $(count $n); do make_text $i initial initial >$i diff --git a/t/t7201-co.sh b/t/t7201-co.sh index eae9e5a..a7fe4e6 100755 --- a/t/t7201-co.sh +++ b/t/t7201-co.sh @@ -591,7 +591,7 @@ test_expect_success 'checkout --conflict=merge, overriding config' ' ' test_expect_success 'checkout --conflict=diff3' ' - git config --unset merge.conflictstyle + test_unconfig merge.conflictstyle && setup_conflicting_index && echo "none of the above" >sample && echo ourside >expect && diff --git a/t/t7508-status.sh b/t/t7508-status.sh index 0f9ad4c..c3ed7cb 100755 --- a/t/t7508-status.sh +++ b/t/t7508-status.sh @@ -538,7 +538,7 @@ A dir2/added ?? untracked EOF test_expect_success 'status -s -uall' ' - git config --unset status.showuntrackedfiles + test_unconfig status.showuntrackedfiles && git status -s -uall >output && test_cmp expect output ' diff --git a/t/t9500-gitweb-standalone-no-errors.sh b/t/t9500-gitweb-standalone-no-errors.sh index f9f078e..e94b2f1 100755 --- a/t/t9500-gitweb-standalone-no-errors.sh +++ b/t/t9500-gitweb-standalone-no-errors.sh @@ -779,7 +779,10 @@ test_expect_success \ test_expect_success \ 'unborn HEAD: "summary" page (with "heads" subview)' \ - 'git checkout orphan_branch || git checkout --orphan orphan_branch && + '{ + git checkout orphan_branch || + git checkout --orphan orphan_branch +} && test_when_finished "git checkout master" && gitweb_run "p=.git;a=summary"' -- 2.3.3.520.g3cfbb5d -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 07/25] t: use test_must_fail instead of hand-rolled blocks
These test scripts likely predate test_must_fail, and can be made simpler by using it (in addition to making them pass --chain-lint). The case in t6036 loses some verbosity in the failure case, but it is so tied to a specific failure mode that it is not worth keeping around (and the outcome of the test is not affected at all). Signed-off-by: Jeff King --- t/t4124-apply-ws-rule.sh | 5 ++--- t/t6036-recursive-corner-cases.sh | 7 +-- t/t9300-fast-import.sh| 7 ++- 3 files changed, 5 insertions(+), 14 deletions(-) diff --git a/t/t4124-apply-ws-rule.sh b/t/t4124-apply-ws-rule.sh index c6474de..d350065 100755 --- a/t/t4124-apply-ws-rule.sh +++ b/t/t4124-apply-ws-rule.sh @@ -99,9 +99,8 @@ test_expect_success 'whitespace=warn, default rule' ' test_expect_success 'whitespace=error-all, default rule' ' - apply_patch --whitespace=error-all && return 1 - test -s target && return 1 - : happy + test_must_fail apply_patch --whitespace=error-all && + ! test -s target ' diff --git a/t/t6036-recursive-corner-cases.sh b/t/t6036-recursive-corner-cases.sh index 8923b04..9d6621c 100755 --- a/t/t6036-recursive-corner-cases.sh +++ b/t/t6036-recursive-corner-cases.sh @@ -195,12 +195,7 @@ test_expect_success 'git detects differently handled merges conflict' ' git reset --hard && git checkout D^0 && - git merge -s recursive E^0 && { - echo "BAD: should have conflicted" - test "Incorrectly merged content" = "$(cat new_a)" && - echo "BAD: Silently accepted wrong content" - return 1 - } + test_must_fail git merge -s recursive E^0 && test 3 = $(git ls-files -s | wc -l) && test 3 = $(git ls-files -u | wc -l) && diff --git a/t/t9300-fast-import.sh b/t/t9300-fast-import.sh index 584b538..aac126f 100755 --- a/t/t9300-fast-import.sh +++ b/t/t9300-fast-import.sh @@ -2853,8 +2853,8 @@ test_expect_success 'S: notemodify with garbage after mark commit-ish must fail' # from # test_expect_success 'S: from with garbage after mark must fail' ' - # no && - git fast-import --import-marks=marks --export-marks=marks <<-EOF 2>err + test_must_fail \ + git fast-import --import-marks=marks --export-marks=marks <<-EOF 2>err && commit refs/heads/S2 mark :303 committer $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> $GIT_COMMITTER_DATE @@ -2865,9 +2865,6 @@ test_expect_success 'S: from with garbage after mark must fail' ' M 100644 :403 hello.c EOF - ret=$? && - echo returned $ret && - test $ret -ne 0 && # failed, but it created the commit # go create the commit, need it for merge test git fast-import --import-marks=marks --export-marks=marks <<-EOF && -- 2.3.3.520.g3cfbb5d -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 06/25] t: use verbose instead of hand-rolled errors
Many tests that predate the "verbose" helper function use a pattern like: test ... || { echo ... false } to give more verbose output. Using the helper, we can do this with a single line, and avoid a || which interacts badly with &&-chaining (besides fooling --chain-lint, we hit the error block no matter which command in the chain failed, so we may often show useless results). In most cases, the messages printed by "verbose" are equally good (in some cases better; t6006 accidentally redirects the message to a file!). The exception is t7001, whose output suffers slightly. However, it's still enough to show the user which part failed, given that we will have just printed the test script to stderr. Signed-off-by: Jeff King --- t/t4022-diff-rewrite.sh| 5 + t/t4202-log.sh | 30 +- t/t6006-rev-list-format.sh | 5 + t/t7001-mv.sh | 5 + t/t7300-clean.sh | 10 ++ 5 files changed, 10 insertions(+), 45 deletions(-) diff --git a/t/t4022-diff-rewrite.sh b/t/t4022-diff-rewrite.sh index 2d030a4..cb51d9f 100755 --- a/t/t4022-diff-rewrite.sh +++ b/t/t4022-diff-rewrite.sh @@ -20,10 +20,7 @@ test_expect_success setup ' test_expect_success 'detect rewrite' ' actual=$(git diff-files -B --summary test) && - expr "$actual" : " rewrite test ([0-9]*%)$" || { - echo "Eh? <<$actual>>" - false - } + verbose expr "$actual" : " rewrite test ([0-9]*%)$" ' diff --git a/t/t4202-log.sh b/t/t4202-log.sh index a22ac7c..85230a8 100755 --- a/t/t4202-log.sh +++ b/t/t4202-log.sh @@ -113,11 +113,7 @@ test_expect_success 'diff-filter=M' ' actual=$(git log --pretty="format:%s" --diff-filter=M HEAD) && expect=$(echo second) && - test "$actual" = "$expect" || { - echo Oops - echo "Actual: $actual" - false - } + verbose test "$actual" = "$expect" ' @@ -125,11 +121,7 @@ test_expect_success 'diff-filter=D' ' actual=$(git log --pretty="format:%s" --diff-filter=D HEAD) && expect=$(echo sixth ; echo third) && - test "$actual" = "$expect" || { - echo Oops - echo "Actual: $actual" - false - } + verbose test "$actual" = "$expect" ' @@ -137,11 +129,7 @@ test_expect_success 'diff-filter=R' ' actual=$(git log -M --pretty="format:%s" --diff-filter=R HEAD) && expect=$(echo third) && - test "$actual" = "$expect" || { - echo Oops - echo "Actual: $actual" - false - } + verbose test "$actual" = "$expect" ' @@ -149,11 +137,7 @@ test_expect_success 'diff-filter=C' ' actual=$(git log -C -C --pretty="format:%s" --diff-filter=C HEAD) && expect=$(echo fourth) && - test "$actual" = "$expect" || { - echo Oops - echo "Actual: $actual" - false - } + verbose test "$actual" = "$expect" ' @@ -161,11 +145,7 @@ test_expect_success 'git log --follow' ' actual=$(git log --follow --pretty="format:%s" ichi) && expect=$(echo third ; echo second ; echo initial) && - test "$actual" = "$expect" || { - echo Oops - echo "Actual: $actual" - false - } + verbose test "$actual" = "$expect" ' diff --git a/t/t6006-rev-list-format.sh b/t/t6006-rev-list-format.sh index 2b7c0f0..b77d4c9 100755 --- a/t/t6006-rev-list-format.sh +++ b/t/t6006-rev-list-format.sh @@ -358,10 +358,7 @@ test_expect_success 'empty email' ' test_tick && C=$(GIT_AUTHOR_EMAIL= git commit-tree HEAD^{tree} failure - false - } + verbose test "$A" = "A U Thor,,Thu Apr 7 15:14:13 2005 -0700" ' test_expect_success 'del LF before empty (1)' ' diff --git a/t/t7001-mv.sh b/t/t7001-mv.sh index 69f11bd..7b56081 100755 --- a/t/t7001-mv.sh +++ b/t/t7001-mv.sh @@ -161,10 +161,7 @@ test_expect_success "Michael Cassar's test case" ' git mv papers/unsorted/Thesis.pdf papers/all-papers/moo-blah.pdf && T=`git write-tree` && - git ls-tree -r $T | grep partA/outline.txt || { - git ls-tree -r $T - (exit 1) - } + git ls-tree -r $T | verbose grep partA/outline.txt ' rm -fr papers partA path? diff --git a/t/t7300-clean.sh b/t/t7300-clean.sh index 04118ad..99be5d9 100755 --- a/t/t7300-clean.sh +++ b/t/t7300-clean.sh @@ -119,10 +119,7 @@ test_expect_success C_LOCALE_OUTPUT 'git clean with relative prefix' ' git clean -n ../src | sed -n -e "s|^Would remove ||p" ) && - test "$would_clean" = ../src/part3.c || { - echo "OOps <$would_clean>" - false - } + verbose test "$would_clean" = ../src/part3.c ' test_expect_success C_LOCALE_OUTPUT 'git clean with absolute path' ' @@
[PATCH 05/25] t: assume test_cmp produces verbose output
Some tests call test_cmp, and if it fails show the actual output generated. This is mostly pointless, as test_cmp will already show a diff between the expected and actual output. It also fools --chain-lint by putting an "||" in the middle of the chain, so we'd rather not use this construct. Note that these cases actually show a pre-processed version of the data, rather than exactly what test_cmp would show. However, test_cmp's output is generally good for pointing the user in the right direction, and they can then dig in the trash directory themselves if they want to see more details. Signed-off-by: Jeff King --- t/t6012-rev-list-simplify.sh | 10 ++ t/t6111-rev-list-treesame.sh | 5 + 2 files changed, 3 insertions(+), 12 deletions(-) diff --git a/t/t6012-rev-list-simplify.sh b/t/t6012-rev-list-simplify.sh index fde5e71..b89cd6b 100755 --- a/t/t6012-rev-list-simplify.sh +++ b/t/t6012-rev-list-simplify.sh @@ -95,10 +95,7 @@ check_outcome () { git log --pretty="$FMT" --parents $param | unnote >actual && sed -e "s/^.* \([^ ]*\) .*/\1/" >check actual && sed -e "s/^.* \([^ ]*\) .*/\1/" >check actual && sed -e "$munge_actual" check && - test_cmp expect check || { - cat actual - false - } + test_cmp expect check ' } -- 2.3.3.520.g3cfbb5d -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 04/25] t: fix trivial &&-chain breakage
These are tests which are missing a link in their &&-chain, but during a setup phase. We may fail to notice failure in commands that build the test environment, but these are typically not expected to fail at all (but it's still good to double-check that our test environment is what we expect). Signed-off-by: Jeff King --- t/annotate-tests.sh| 2 +- t/t-basic.sh | 4 ++-- t/t0011-hashmap.sh | 2 +- t/t0201-gettext-fallbacks.sh | 4 ++-- t/t1006-cat-file.sh| 2 +- t/t3010-ls-files-killed-modified.sh| 2 +- t/t3404-rebase-interactive.sh | 2 +- t/t3405-rebase-malformed.sh| 4 ++-- t/t3425-rebase-topology-merges.sh | 4 ++-- t/t3600-rm.sh | 4 ++-- t/t4014-format-patch.sh| 2 +- t/t4041-diff-submodule-option.sh | 10 ++ t/t4049-diff-stat-count.sh | 6 +++--- t/t4054-diff-bogus-tree.sh | 2 +- t/t5500-fetch-pack.sh | 2 +- t/t5510-fetch.sh | 2 +- t/t5526-fetch-submodules.sh| 10 +- t/t5531-deep-submodule-push.sh | 2 +- t/t5533-push-cas.sh| 4 ++-- t/t5541-http-push-smart.sh | 2 +- t/t5550-http-fetch-dumb.sh | 2 +- t/t5551-http-fetch-smart.sh| 2 +- t/t6000-rev-list-misc.sh | 4 ++-- t/t6006-rev-list-format.sh | 2 +- t/t6022-merge-rename.sh| 6 +++--- t/t6036-recursive-corner-cases.sh | 2 +- t/t6111-rev-list-treesame.sh | 2 +- t/t6200-fmt-merge-msg.sh | 2 +- t/t7004-tag.sh | 2 +- t/t7006-pager.sh | 2 +- t/t7009-filter-branch-null-sha1.sh | 2 +- t/t7400-submodule-basic.sh | 4 ++-- t/t7406-submodule-update.sh| 2 +- t/t7508-status.sh | 8 t/t7510-signed-commit.sh | 2 +- t/t7600-merge.sh | 2 +- t/t7612-merge-verify-signatures.sh | 2 +- t/t8003-blame-corner-cases.sh | 8 t/t8008-blame-formats.sh | 2 +- t/t9001-send-email.sh | 2 +- t/t9300-fast-import.sh | 2 +- t/t9500-gitweb-standalone-no-errors.sh | 2 +- t/t9903-bash-prompt.sh | 2 +- 43 files changed, 70 insertions(+), 68 deletions(-) diff --git a/t/annotate-tests.sh b/t/annotate-tests.sh index 071e4d7..f5c0175 100644 --- a/t/annotate-tests.sh +++ b/t/annotate-tests.sh @@ -405,7 +405,7 @@ test_expect_success 'setup -L :regex' ' mv hello.c hello.orig && echo "#include " >hello.c && cat hello.orig >>hello.c && - tr Q "\\t" >>hello.c <<-\EOF + tr Q "\\t" >>hello.c <<-\EOF && void mail() { Qputs("mail"); diff --git a/t/t-basic.sh b/t/t-basic.sh index f10ba4a..79b9074 100755 --- a/t/t-basic.sh +++ b/t/t-basic.sh @@ -253,7 +253,7 @@ test_expect_success 'test --verbose' ' test_expect_success "failing test" false test_done EOF - mv test-verbose/out test-verbose/out+ + mv test-verbose/out test-verbose/out+ && grep -v "^Initialized empty" test-verbose/out+ >test-verbose/out && check_sub_test_lib_test test-verbose <<-\EOF > expecting success: true @@ -974,7 +974,7 @@ test_expect_success 'writing this tree with --missing-ok' ' test_expect_success 'git read-tree followed by write-tree should be idempotent' ' - rm -f .git/index + rm -f .git/index && git read-tree $tree && test -f .git/index && newtree=$(git write-tree) && diff --git a/t/t0011-hashmap.sh b/t/t0011-hashmap.sh index f97c805..9c217d9 100755 --- a/t/t0011-hashmap.sh +++ b/t/t0011-hashmap.sh @@ -218,7 +218,7 @@ test_expect_success 'grow / shrink' ' echo size >> in && echo 64 51 >> expect && echo put key52 value52 >> in && - echo NULL >> expect + echo NULL >> expect && echo size >> in && echo 256 52 >> expect && for n in $(test_seq 12) diff --git a/t/t0201-gettext-fallbacks.sh b/t/t0201-gettext-fallbacks.sh index 00c6d3d..90da1c7 100755 --- a/t/t0201-gettext-fallbacks.sh +++ b/t/t0201-gettext-fallbacks.sh @@ -50,7 +50,7 @@ test_expect_success 'eval_gettext: our eval_gettext() fallback can interpolate v test_expect_success 'eval_gettext: our eval_gettext() fallback can interpolate variables with spaces' ' cmdline="git am" && -export cmdline; +export cmdline && printf "When you have resolved this problem, run git am --resolved." >expect && eval_gettext "When you have resolved this problem, run \$cmdline --resolved." >actual && test_i18ncmp expect actual @@ -58,7 +58,7 @@ test_expect_success 'eval_gettext: our eval
[PATCH 02/25] t: fix severe &&-chain breakage
These are tests which are missing a link in their &&-chain, in a location which causes a significant portion of the test to be missed (e.g., the test effectively does nothing, or consists of a long string of actions and output comparisons, and we throw away the exit code of at least one part of the string). Signed-off-by: Jeff King --- t/t1300-repo-config.sh| 2 +- t/t1403-show-ref.sh | 10 +- t/t1700-split-index.sh| 2 +- t/t3600-rm.sh | 2 +- t/t4047-diff-dirstat.sh | 2 +- t/t4104-apply-boundary.sh | 4 ++-- t/t4202-log.sh| 2 +- t/t5100-mailinfo.sh | 2 +- t/t5510-fetch.sh | 2 +- t/t5540-http-push-webdav.sh | 2 +- t/t5551-http-fetch-smart.sh | 2 +- t/t6036-recursive-corner-cases.sh | 2 +- t/t9300-fast-import.sh| 2 +- t/t9902-completion.sh | 2 +- 14 files changed, 19 insertions(+), 19 deletions(-) diff --git a/t/t1300-repo-config.sh b/t/t1300-repo-config.sh index 938fc8b..bc0b392 100755 --- a/t/t1300-repo-config.sh +++ b/t/t1300-repo-config.sh @@ -677,7 +677,7 @@ test_expect_success 'invalid unit' ' echo 1auto >expect && git config aninvalid.unit >actual && test_cmp expect actual && - cat >expect <<-\EOF + cat >expect <<-\EOF && fatal: bad numeric config value '\''1auto'\'' for '\''aninvalid.unit'\'' in .git/config: invalid unit EOF test_must_fail git config --int --get aninvalid.unit 2>actual && diff --git a/t/t1403-show-ref.sh b/t/t1403-show-ref.sh index 3e500ed..7e10bcf 100755 --- a/t/t1403-show-ref.sh +++ b/t/t1403-show-ref.sh @@ -28,7 +28,7 @@ test_expect_success 'show-ref' ' >expect && - test_must_fail git show-ref D >actual + test_must_fail git show-ref D >actual && test_cmp expect actual ' @@ -62,7 +62,7 @@ test_expect_success 'show-ref --verify' ' test_must_fail git show-ref --verify tags/A >actual && test_cmp expect actual && - test_must_fail git show-ref --verify D >actual + test_must_fail git show-ref --verify D >actual && test_cmp expect actual ' @@ -78,7 +78,7 @@ test_expect_success 'show-ref --verify -q' ' test_must_fail git show-ref --verify -q tags/A >actual && test_cmp expect actual && - test_must_fail git show-ref --verify -q D >actual + test_must_fail git show-ref --verify -q D >actual && test_cmp expect actual ' @@ -105,10 +105,10 @@ test_expect_success 'show-ref -d' ' test_cmp expect actual && git show-ref -d refs/heads/master >actual && - test_cmp expect actual + test_cmp expect actual && git show-ref -d --verify refs/heads/master >actual && - test_cmp expect actual + test_cmp expect actual && >expect && diff --git a/t/t1700-split-index.sh b/t/t1700-split-index.sh index 94fb473..a55f5bc 100755 --- a/t/t1700-split-index.sh +++ b/t/t1700-split-index.sh @@ -182,7 +182,7 @@ test_expect_success 'unify index, two files remain' ' 100644 e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 0 one 100644 e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 0 two EOF - test_cmp ls-files.expect ls-files.actual + test_cmp ls-files.expect ls-files.actual && test-dump-split-index .git/index | sed "/^own/d" >actual && cat >expectactual_diff_dirstat_CC && - test_cmp expect_diff_dirstat_CC actual_diff_dirstat_CC + test_cmp expect_diff_dirstat_CC actual_diff_dirstat_CC && git diff --dirstat=files --dirstat=10 --dirstat=cumulative --dirstat=changes --dirstat=noncumulative -X3 HEAD^..HEAD >actual_diff_dirstat && test_cmp expect_diff_dirstat actual_diff_dirstat && git diff --dirstat=files --dirstat=10 --dirstat=cumulative --dirstat=changes --dirstat=noncumulative -X3 -M HEAD^..HEAD >actual_diff_dirstat_M && diff --git a/t/t4104-apply-boundary.sh b/t/t4104-apply-boundary.sh index c617c2a..c97aad1 100755 --- a/t/t4104-apply-boundary.sh +++ b/t/t4104-apply-boundary.sh @@ -78,8 +78,8 @@ test_expect_success setup ' echo $i done >victim && cat victim >del-z-expect && - git diff victim >del-z-patch.with - git diff --unified=0 >del-z-patch.without && + git diff victim >del-z-patch.with && + git diff --unified=0 >del-z-patch.without : done ' diff --git a/t/t4202-log.sh b/t/t4202-log.sh index 5f2b290..a22ac7c 100755 --- a/t/t4202-log.sh +++ b/t/t4202-log.sh @@ -481,7 +481,7 @@ test_expect_success 'log.decorate configuration' ' git log --oneline --no-decorate >actual && test_cmp expect.none actual && git log --oneline --decorate >actual && - test_cmp exp
[PATCH 03/25] t: fix moderate &&-chain breakage
These are tests which are missing a link in their &&-chain, but in a way that probably does not effect the outcome of the test. Most of these are of the form: some_cmd >actual test_cmp expect actual The main point of the test is to verify the output, and a failure in some_cmd would probably be noticed by bogus output. But it is good for the tests to also confirm that "some_cmd" does not die unexpectedly after producing its output. Signed-off-by: Jeff King --- t/t0062-revision-walking.sh | 2 +- t/t0201-gettext-fallbacks.sh| 4 ++-- t/t1300-repo-config.sh | 2 +- t/t3001-ls-files-others-exclude.sh | 2 +- t/t3010-ls-files-killed-modified.sh | 4 ++-- t/t3404-rebase-interactive.sh | 2 +- t/t4041-diff-submodule-option.sh| 2 +- t/t4052-stat-output.sh | 34 +- t/t4212-log-corrupt.sh | 2 +- t/t5551-http-fetch-smart.sh | 2 +- t/t5709-clone-refspec.sh| 2 +- t/t6028-merge-up-to-date.sh | 2 +- t/t6132-pathspec-exclude.sh | 28 ++-- 13 files changed, 44 insertions(+), 44 deletions(-) diff --git a/t/t0062-revision-walking.sh b/t/t0062-revision-walking.sh index 3d98eb8..113c728 100755 --- a/t/t0062-revision-walking.sh +++ b/t/t0062-revision-walking.sh @@ -26,7 +26,7 @@ test_expect_success 'setup' ' ' test_expect_success 'revision walking can be done twice' ' - test-revision-walking run-twice > run_twice_actual + test-revision-walking run-twice >run_twice_actual && test_cmp run_twice_expected run_twice_actual ' diff --git a/t/t0201-gettext-fallbacks.sh b/t/t0201-gettext-fallbacks.sh index 5d80a98..00c6d3d 100755 --- a/t/t0201-gettext-fallbacks.sh +++ b/t/t0201-gettext-fallbacks.sh @@ -52,7 +52,7 @@ test_expect_success 'eval_gettext: our eval_gettext() fallback can interpolate v cmdline="git am" && export cmdline; printf "When you have resolved this problem, run git am --resolved." >expect && -eval_gettext "When you have resolved this problem, run \$cmdline --resolved." >actual +eval_gettext "When you have resolved this problem, run \$cmdline --resolved." >actual && test_i18ncmp expect actual ' @@ -60,7 +60,7 @@ test_expect_success 'eval_gettext: our eval_gettext() fallback can interpolate v cmdline="git am" && export cmdline; printf "When you have resolved this problem, run \"git am --resolved\"." >expect && -eval_gettext "When you have resolved this problem, run \"\$cmdline --resolved\"." >actual +eval_gettext "When you have resolved this problem, run \"\$cmdline --resolved\"." >actual && test_i18ncmp expect actual ' diff --git a/t/t1300-repo-config.sh b/t/t1300-repo-config.sh index bc0b392..66dd286 100755 --- a/t/t1300-repo-config.sh +++ b/t/t1300-repo-config.sh @@ -1165,7 +1165,7 @@ test_expect_failure 'adding a key into an empty section reuses header' ' Qkey = value EOF - git config section.key value + git config section.key value && test_cmp expect .git/config ' diff --git a/t/t3001-ls-files-others-exclude.sh b/t/t3001-ls-files-others-exclude.sh index b2798fe..3fc484e 100755 --- a/t/t3001-ls-files-others-exclude.sh +++ b/t/t3001-ls-files-others-exclude.sh @@ -294,7 +294,7 @@ one/a.1 one/two/a.1 three/a.1 EOF - git ls-files -o -i --exclude "**/a.1" >actual + git ls-files -o -i --exclude "**/a.1" >actual && test_cmp expect actual ' diff --git a/t/t3010-ls-files-killed-modified.sh b/t/t3010-ls-files-killed-modified.sh index 6d3b828..ea6a678 100755 --- a/t/t3010-ls-files-killed-modified.sh +++ b/t/t3010-ls-files-killed-modified.sh @@ -99,12 +99,12 @@ test_expect_success 'git ls-files -k to show killed files.' ' ' test_expect_success 'git ls-files -k output (w/o icase)' ' - git ls-files -k >.output + git ls-files -k >.output && test_cmp .expected .output ' test_expect_success 'git ls-files -k output (w/ icase)' ' - git -c core.ignorecase=true ls-files -k >.output + git -c core.ignorecase=true ls-files -k >.output && test_cmp .expected .output ' diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh index 034eb35..edad20d 100755 --- a/t/t3404-rebase-interactive.sh +++ b/t/t3404-rebase-interactive.sh @@ -950,7 +950,7 @@ test_expect_success 'rebase --edit-todo can be used to modify todo' ' set_fake_editor && FAKE_LINES="edit 1 2 3" git rebase -i HEAD~3 && FAKE_LINES="2 1" git rebase --edit-todo && - git rebase --continue + git rebase --continue && test M = $(git cat-file commit HEAD^ | sed -ne \$p) && test L = $(git cat-file commit HEAD | sed -ne \$p) ' diff --git a/t/t4041-diff-submodule-option.sh b/t/t4041-diff-submodule-option.sh index e432896..ff61341 100755 --- a/t/t4041-diff-submodule-option.sh +++ b/t/t4041-diff-submodule-option.sh @@ -539,7 +539,
[PATCH 01/25] t/test-lib: introduce --chain-lint option
It's easy to miss an "&&"-chain in a test script, like: test_expect_success 'check something important' ' cmd1 && cmd2 cmd3 ' The test harness will notice if cmd3 fails, but a failure of cmd1 or cmd2 will go unnoticed, as their exit status is lost after cmd3 runs. The toy example above is easy to spot because the "cmds" are all the same length, but real code is much more complicated. It's also difficult to detect these situations by statically analyzing the shell code with regexps (like the check-non-portable-shell script does); there's too much context required to know whether a &&-chain is appropriate on a given line or not. This patch instead lets the shell check each test by sticking a command with a specific and unusual return code at the top of each test, like: (exit 117) && cmd1 && cmd2 cmd3 In a well-formed test, the non-zero exit from the first command prevents any of the rest from being run, and the test's exit code is 117. In a bad test (like the one above), the 117 is lost, and cmd3 is run. When we encounter a failure of this check, we abort the test script entirely. For one thing, we have no clue which subset of the commands in the test snippet were actually run. Running further tests would be pointless, because we're now in an unknown state. And two, this is not a "test failure" in the traditional sense. The test script is buggy, not the code it is testing. We should be able to fix these problems in the script once, and not have them come back later as a regression in git's code. After checking a test snippet for --chain-lint, we do still run the test itself. We could actually have a pure-lint mode which just checks each test, but there are a few reasons not to. One, because the tests are executing arbitrary code, which could impact the later environment (e.g., that could impact which set of tests we run at all). And two, because a pure-lint mode would still be expensive to run, because a significant amount of code runs outside of the test_expect_* blocks. Instead, this option is designed to be used as part of a normal test suite run, where it adds very little overhead. Turning on this option detects quite a few problems in existing tests, which will be fixed in subsequent patches. However, there are a number of places it cannot reach: - it cannot find a failure to break out of loops on error, like: cmd1 && for i in a b c; do cmd2 $i done && cmd3 which will not notice failures of "cmd2 a" or "cmd b" - it cannot find a missing &&-chain inside a block or subfunction, like: foo () { cmd1 cmd2 } foo && bar which will not notice a failure of cmd1. - it only checks tests that you run; every platform will have some tests skipped due to missing prequisites, so it's impossible to say from one run that the test suite is free of broken &&-chains. However, all tests get run by _somebody_, so eventually we will notice problems. - it does not operate on test_when_finished or prerequisite blocks. It could, but these tends to be much shorter and less of a problem, so I punted on them in this patch. This patch was inspired by an earlier patch by Jonathan Nieder: http://article.gmane.org/gmane.comp.version-control.git/235913 This implementation and all bugs are mine. Signed-off-by: Jeff King --- t/README | 10 ++ t/test-lib.sh | 16 2 files changed, 26 insertions(+) diff --git a/t/README b/t/README index d5bb0c9..35438bc 100644 --- a/t/README +++ b/t/README @@ -168,6 +168,16 @@ appropriately before running "make". Using this option with a RAM-based filesystem (such as tmpfs) can massively speed up the test suite. +--chain-lint:: +--no-chain-lint:: + If --chain-lint is enabled, the test harness will check each + test to make sure that it properly "&&-chains" all commands (so + that a failure in the middle does not go unnoticed by the final + exit code of the test). This check is performed in addition to + running the tests themselves. You may also enable or disable + this feature by setting the GIT_TEST_CHAIN_LINT environment + variable to "1" or "0", respectively. + You can also set the GIT_TEST_INSTALLED environment variable to the bindir of an existing git installation to test that installation. You still need to have built this git sandbox, from which various diff --git a/t/test-lib.sh b/t/test-lib.sh index c096778..50b3d3f 100644 --- a/t/test-lib.sh +++ b/t/test-lib.sh @@ -232,6 +232,12 @@ do --root=*) root=$(expr "z$1" : 'z[^=]*=\(.*\)') shift ;; + --chain-lint) + GIT_TEST_CHAIN_LINT=1 + shift ;; + --no-chain-lint) + GIT_TEST_CHAIN_LINT=0 + shift ;; -x) trace=t verbose=t @@ -524,6 +530,16 @@ te
[PATCH 0/25] detecting &&-chain breakage
This is a cleanup of the &&-chain lint I posted earlier: http://thread.gmane.org/gmane.comp.version-control.git/265613/focus=265859 I don't know who came up with the idea for it originally, but the concept certainly was floating in the back of my mind from Jonathan's earlier version that is referenced in that thread. The general idea is to detect &&-chain breakage that can lead to our tests yielding false success. The first patch implements and discusses the lint-check itself, which is quite simple. The bulk of the work was fixing all of the issues in the existing tests. :) That didn't all need to happen immediately. I mainly wanted to start on it to answer two questions: 1. Are &&-chain breakages actually preventing us from seeing any test failures? Or is it mostly just pedantry, and we miss out only on knowing whether "cat >expect <<-\EOF" failed (which presumably it never does). 2. How bad are the false positives? Both how common, and how bad to work around. But after a few hours, I reached a zen state and just kept going. So at the end of this series, the whole test suite is --chain-lint clean (modulo any tests that are skipped on my platform). We could even switch the checks on by default at the end of the series, but I didn't do that here. I think it would be sane to run them all the time, though; in the normal success case, they don't add any forks (the shell just runs "(exit) && ...", and realizes that the whole thing is one big &&-chain). I couldn't measure any time difference running the suite with and without it. Anyway, to answer the questions: Yes, there were definitely tests whose values were being thrown away, and we would not have noticed if they failed. The good news is that all of them did pass once we started checking their results. Hooray. There were a number of false positives, though as a percentage of the test suite, probably not many (it's just that we have quite a lot of tests). Most of them were in rather old tests, and IMHO the fixes I did actually improved the readability of the result. So overall I think this is a very positive change; I doubt it will get in people's way very often, and I look forward to having one less thing to worry about handling manually in review. The biggest downside is that I may have automated Eric Sunshine out of a job. :) The patches are: [01/25]: t/test-lib: introduce --chain-lint option [02/25]: t: fix severe &&-chain breakage [03/25]: t: fix moderate &&-chain breakage [04/25]: t: fix trivial &&-chain breakage [05/25]: t: assume test_cmp produces verbose output [06/25]: t: use verbose instead of hand-rolled errors [07/25]: t: use test_must_fail instead of hand-rolled blocks [08/25]: t: fix &&-chaining issues around setup which might fail [09/25]: t: use test_might_fail for diff and grep [10/25]: t: use test_expect_code instead of hand-rolled comparison [11/25]: t: wrap complicated expect_code users in a block [12/25]: t: avoid using ":" for comments [13/25]: t3600: fix &&-chain breakage for setup commands [14/25]: t7201: fix &&-chain breakage [15/25]: t9502: fix &&-chain breakage [16/25]: t6030: use modern test_* helpers [17/25]: t0020: use modern test_* helpers [18/25]: t1301: use modern test_* helpers [19/25]: t6034: use modern test_* helpers [20/25]: t4117: use modern test_* helpers [21/25]: t9001: use test_when_finished [22/25]: t0050: appease --chain-lint [23/25]: t7004: fix embedded single-quotes [24/25]: t0005: fix broken &&-chains [25/25]: t4104: drop hand-rolled error reporting It's a lot of patches, and a few of them are long. I tried to group them by functionality rather than file (though as you can see, some of the tests were unique enough snowflakes that it made sense to discuss their issues separately). I'm hoping it should be an easy review, if not a short one. I'll spare you the full per-file diffstat, but the total is a very satisfying: 84 files changed, 397 insertions(+), 647 deletions(-) That's a lot of changes in a lot of hunks, but most of them are in files that haven't been touched in a while. The series merges cleanly with "pu", so I don't think I've stepped on anyone's topics in flight. -Peff -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Fwd: Seems to be pushing more than necessary
That all seems quite reasonable, and is what I would expect to happen. However at the moment, if I create a branch from master and edit one line in one file, with no other changes on the remote, it takes me over an hour to push the new branch. On 19 March 2015 at 18:36, Junio C Hamano wrote: > Graham Hay writes: > >> We have a fairly large repo (~2.4GB), mainly due to binary resources >> (for an ios app). I know this can generally be a problem, but I have a >> specific question. >> >> If I cut a branch, and edit a few (non-binary) files, and push, what >> should be uploaded? I assumed it was just the diff (I know whole >> compressed files are used, I mean the differences between my branch >> and where I cut it from). Is that correct? > > If you start from this state: > > (the 'origin')(you) > ---Z---A clone ->---Z---A > > and edit a few files, say, a/b, a/c and d/e/f, and committed to make > the history look like this: > > (the 'origin')(you) > ---Z---A ---Z---A---B > > i.e. "git diff --name-only A B" would show these three files, then > the next push from you to the origin, i.e. > > (the 'origin')(you) > ---Z---A---B<- push ---Z---A---B > > would involve transferring from you to the origin of the following: > > * The commit object that holds the message, authorship, etc. for B > * The top-level tree object of commit B (as that is different from >that of A) > * The tree object for 'a', 'd', 'd/e' and the blob object for >'a/b', 'a/c', and 'd/e/f'. > > However, that assumes that nothing is happening on the 'origin' > side. > > If the 'origin', for example, rewound its head to Z before you > attempt to push your B, then you may end up sending objects that do > not exist in Z that are reachable from B. Just like the above > bullet points enumerated what is different between A and B, you > can enumerate what is different between Z and A and add that to the > above set. That would be what will be sent. > > If the 'origin' updated its tip to a commit you do not even know > about, normally you will be prevented from pushing B because we > would not want you to lose somebody else's work. If you forced such > push, then you may end up sending a lot more. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: A git hook that does git cherry-pick and push automatically
David, Thanks for the suggestions, I left out the most unfortunate part of the problem I am facing, the one that add this hook is the git admin, aka our release engineer, so it's a sever side hook that it's forced down to all developers. I sure will try to persuade them from a different perspective more calmly this time. Regards, Ray >> On Mar 20, 2015, at 12:16 AM, David Aguilar wrote: >> >> On Thu, Mar 19, 2015 at 09:07:28PM -0700, Ray Xie wrote: >> So today I had a shocking moment while I was doing my cherry-pick, >> after I performed all the pre-checkin duties (the usual build the >> code, run the test to make sure the cherry-pick infact works), I found >> out that my original commit was already cherry-picked, then I found >> out someone in engineering make the decision to do an automate >> cherry-pick from that branch to another so he added a git hook to run >> do cherry-pick and push on every commit, yes, a simple cherry-pick >> then push; mind you, these are not feature / dev branch, these are >> release branches, both of them. >> >> Then after I came back from the shock, made a big protest about how >> this is the worst thing I have seen and I will never live with it, and >> that's why "git cherry-pick" and "git push" are 2 separate commands, >> as any reasonable developer, you do your very best to ensure you are >> not pushing something that is fundamentally broken; however for the >> life of me and talk these few people into senses. >> >> So, I am sending this to seek for some expert advice how I can drive >> some sense into these people so they don't screw up my life as an >> developer. >> >> Regards, >> Desperate developer, Ray. > > It's interesting that your clone picked up the hooks that they > added. Do you have a centrally managed git installation, where > perhaps they are modifying the default .git/hooks/ template that > is used to seed the initial repo when cloning? > > One way to check that is to look at your git installation. > If your git is installed as /usr/bin/git then check > /usr/share/git-core/templates and see if the hook is present > there. > > Assuming that you have the ability to create your own clones, > and that you are not all working in a common shared sandbox, > then the simplest solution would be to simply delete your > .git/hooks/* when cloning a new repo. > > That way you can at least have your sandboxes work the way > you want them to work. Hooks are local to your sandbox so you > have full control over them. > > Is this hook resilient to rebasing? Does it cause havoc if you > a feature onto this branch? Those are the kinds of things that > sound scary based on your description of the hook. > > I don't have any tips on how to convince people. One thing that > might help is to remember that people are self-motivated ~ try > to focus on what *they* stand to benefit from not having this > hook. If you can convince them that they are better-off without > the hook, from their POV, not from yours, then you might have a > better chance of persuading them. > > Good luck, > -- > David -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
So far we have 39 Special Sessions/ Workshops. You can propose your own Special Session
Dear Authors, We would like to invite you to organize a Special Session or a Workshop in CSCC 2015: Circuits, Systems, Communications and Computers www.cscc.co in conjuction with several parallel conferences of INASE.org So far we have 39 Special Sessions/ Workshops. You can propose your own Special Session - Workshop until April 30, 2015. See a full report and many wonderful photos from the CSCC.co and INASE.org conferences here: inase-news.blogspot.com/2014/12/post-conference-report-inase_15.html www.cscc14.org We would like to invite you also to present an Invited Lecture (you know the privileges from my former invitations) in CSCC.org and/org in INASE.org Conferences * Pure Mathematics, Applied Mathematics and Computational Methods * Continuum Mechanics, * Mechatronics and Robotics, Structural Analysis * Energy, Environment, Development and Economics * Civil Engineering, * Water Resources, Hydraulics & Hydrology * Geology and Seismology, * Education and Modern Educational Technologies * Materials, * Industrial Engineering, * Maritime and Naval Science and Engineering CSCC.org and INASE.org received more than 1400 papers in July of 2014. >From them after evaluation about 600 papers were accepted. See photos from 2014: inase-news.blogspot.com/2014/12/post-conference-report-inase_15.html We would like to invite you to organize a Special Session or Workshop. For successful Special Session / Workshop organizers we give financial support (honorarium) and free publication, or even air tickets and hotel expenses in case of great sucess in their special session / workshop. Contact us for more details. Successful special session organizers will be automatically included in the committee of CSCC 2016, as well as we will jointly examine their participation in the Editorial Board in one of our 36 collaborating ISI/SCI Journals. Special Session organizers are also appointed Associate Editors in the conference Proceedings. We will also give several mailing lists and web sites to the Session Organizers to send their own Call For Papers. So, far we have 39 Special Sessions and we believe that we will reach more than 100 until July. Send the proposal of your Special Session to us now following the instructions of cscc.co The list of the reviewers that will undertake the peer review process and their emails is included in the proceedings and is also sent to ISI, SCOPUS and other indexes. The CSCC.co and INASE.org conferences Conference are organized in the wonderful and historical island of Zakynthos in Greece (July 16,17,18,19,20,21 2015 / Five days). The conferences are indexed in ISI, SCOPUS, Engineering Villeage, EI Compendex, IET etc and has collaboration with important scientific societies and 3 universities: More Details: www.cscc.co Last year the Proceedings of CSCC.co and INASE.org conferences were published in 12 Volumes (hard-copy) and one CD (containing the material of these 12 volumes) and this material was distributed to our authors. CSCC has always famous plenary speakers from prestigious universities, IEEE Fellows (www.cscc.co / last year: www.cscc14.org) and this year we have arranged post-conference publication of all the extended version of all the accepted papers in 36 ISI/SCI Indexed Journals. We have also 4 Tutorials by well-known colleagues. See you in Zakynthos, Greece in July Dr. Z. Michel ** ** Should you wish not to receive further invitations, please send an email to cscc2...@gmail.com with the exact phrase " REMOVE git@vger.kernel.org " in the subject line. Do not include this exact phrase in the body of your email please, but only in the Subject Line of your Email! -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 01/14] numparse: new module for parsing integral numbers
On Wed, Mar 18, 2015 at 6:47 PM, Michael Haggerty wrote: > On 03/18/2015 07:27 PM, Eric Sunshine wrote: >> On Tuesday, March 17, 2015, Michael Haggerty wrote: >>> Implement wrappers for strtol() and strtoul() that are safer and more >>> convenient to use. >>> + * The lowest 6 bits of flags hold the numerical base that should be >>> + * used to parse the number, 2 <= base <= 36. If base is set to 0, >>> + * then NUM_BASE_SPECIFIER must be set too; in this case, the base is >>> + * detected automatically from the string's prefix. >> >> Does this restriction go against the goal of making these functions >> convenient, even while remaining strict? Is there a strong reason for >> not merely inferring NUM_BASE_SPECIFIER when base is 0? Doing so would >> make it consistent with strto*l() without (I think) introducing any >> ambiguity. > > I thought about doing this. If it were possible to eliminate > NUM_BASE_SPECIFIER altogether, then there is no doubt that it would be a > good change. But NUM_BASE_SPECIFIER also has an effect when base==16; > namely, that an "0x" prefix, if present, is consumed. So > > parse_i("0xff", 16 | NUM_BASE_SPECIFIER, &result, &endptr) > > gives result==255 and endptr==s+4, whereas > > parse_i("0xff", 16, &result, &endptr) > > gives result==0 and entptr==s+1 (it treats the "x" as the end of the > string). > > We could forgo that feature, effectively allowing a base specifier if > and only if base==0. But I didn't want to rule out allowing an optional > base specifier for base==16, in which case NUM_BASE_SPECIFIER can't be > dispensed with entirely. Making base==0 a special case doesn't have to mean ruling out or eliminating NUM_BASE_SPECIFIER for the base==16 case. However, a special case base==0 would make the API a bit non-orthogonal and require extra documentation. But for those familiar with how strto*l() treats base==0 specially, the rule of least surprise may apply. > If you agree with that, then the remaining question is: which policy is > less error-prone? My thinking was that forcing the caller to specify > NUM_BASE_SPECIFIER explicitly when they select base==0 will serve as a > reminder that the two features are intertwined. Since base==0 would unambiguously imply NUM_BASE_SPECIFIER, being able to tersely say convert_i(s, 0, &result) would be a win from a convenience perspective. However... > Because another > imaginable policy (arguably more consistent with the policy for base!=0) > would be that > > convert_i(s, 0, &result) > > , because it *doesn't* specify NUM_BASE_SPECIFIER, doesn't allow a base > prefix, and therefore indirectly only allows base-10 numbers. > > But I don't feel strongly about this. I don't feel strongly about it either, and can formulate arguments either way. Assuming you stick with your current design, if the strictness of having to specify NUM_BASE_SPECIFIER for base==0 proves too burdensome, it can be loosened later by making base==0 a special case. On the other hand, if you go with Junio's suggestion of choosing names for these functions which more closely mirror strto*l() names, then the rule of least surprise might suggest that a special case for base==0 has merit. >>> + * Return 0 on success or a negative value if there was an error. On >>> + * failure, *result and *entptr are left unchanged. >>> + * >>> + * Please note that if NUM_TRAILING is not set, then it is >>> + * nevertheless an error if there are any characters between the end >>> + * of the number and the end of the string. >> >> Again, on the subject of convenience, why this restriction? The stated >> purpose of the parse_*() functions is to parse the number from the >> front of the string and return a pointer to the first non-numeric >> character following. As a reader of this API, I interpret that as >> meaning that NUM_TRAILING is implied. Is there a strong reason for not >> inferring NUM_TRAILING for the parse_*() functions at the API level? >> (I realize that the convert_*() functions are built atop parse_*(), >> but that's an implementation detail.) > > Yes, I'd also thought about that change: > > * Make NUM_TRAILING private. > * Make the current parse_*() functions private. > * Add new parse_*(s, flags, result, endptr) functions that imply > NUM_TRAILING (and maybe they should *require* a non-null endptr argument). > * Change the convert_*() to not allow the NUM_TRAILING flag. > > This would add a little bit of code, so I didn't do it originally. But > since you seem to like the idea too, I guess I will make the change. The other option (as Junio also suggested) is to collapse this API into just the four parse_*() functions. All of the call-sites you converted in this series invoked convert_*(), but it's not clear that having two sets of functions which do almost the same thing is much of a win. If the default is for NUM_TRAILING to be off, and if NULL 'endptr' is allowed, then: parse_ul(s, 10, &result, NULL); doesn't seem particularly burdensome comp
Re: [msysGit] Git for Windows 1.9.5.msysgit.1
On Freitag, 20. März 2015 02:15:31 CEST, Mike Hommey wrote: On Fri, Mar 20, 2015 at 12:03:43AM +0100, Thomas Braun wrote: Hi, the Git for Windows team just released the first maintenance release of the Windows-specific installers for git 1.9.5. is it expected that there is no corresponding release on https://github.com/git-for-windows/git/releases ? https://github.com/msysgit/git/releases/tag/v1.9.5.msysgit.1 is the corresponding source code release. Binaries are available from https://github.com/msysgit/msysgit/releases/tag/Git-1.9.5-preview20150319. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] git=p4.py rebase now honor's client spec
On 19/03/15 21:58, brian m. carlson wrote: On Thu, Mar 19, 2015 at 12:28:09PM +, Sam Bishop wrote: When using the git-p4.py script, I found that if I used a client spec when cloning out a perforce repository, and then using a git-p4.py rebase, that the rebase command would be using the current perforce client spec, instead of the one used when doing the initial clone. My proposed patch causes the rebase command to switch to the perforce client spec used when doing the initial git-p4.py clone. That's very useful, thanks. I've noticed that behaviour in the past and always thought it ought to be fixed. As Brian notes, it needs a "Signed-off" line in the patch. A very small nit: could you prefix the subject with "git p4:" so that it's consistent with other recent git-p4 changes - it makes it easier to pick them out when reviewing changes. Could you run the git-p4 unit tests on it please (t/t98*)? I could be wrong about this, but it looks to me like t9806-git-p4-options.sh doesn't pass now with this change (*) Thanks! Luke (*) There's at least one test that doesn't pass anyway, but this seems to be new. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: test &&-chain lint (was: [PATCH 1/5] t5312: test object deletion code paths in a corrupted repository)
On Fri, Mar 20, 2015 at 1:10 AM, Jeff King wrote: > On Thu, Mar 19, 2015 at 10:25:32PM -0400, Jeff King wrote: >> > diff --git a/t/test-lib.sh b/t/test-lib.sh >> > index c096778..02a03d5 100644 >> > --- a/t/test-lib.sh >> > +++ b/t/test-lib.sh >> > @@ -524,6 +524,21 @@ test_eval_ () { >> > test_run_ () { >> > + if test -n "$GIT_TEST_CHAIN_LINT"; then >> > + # 117 is unlikely to match the exit code of >> > + # another part of the chain >> > + test_eval_ "(exit 117) && $1" >> > + if test "$?" != 117; then >> > + # all bets are off for continuing with other tests; >> > + # we expected none of the rest of the test commands to >> > + # run, but at least some did. Who knows what weird >> > + # state we're in? Just bail, and the user can diagnose >> > + # by running in --verbose mode >> > + error "bug in the test script: broken &&-chain" >> > + fi >> > + fi Clever (Jonathan's too); much nicer than trying to special case only here-doc. >> > This turns up an appalling number of failures, but AFAICT they are all >> > "real" in the sense that the &&-chains are broken. In some cases these >> > are real, but in others the tests are of an older style where they did >> > not expect some early commands to fail (and we would catch their bogus >> > output if they did). E.g., in the patch below, I think the first one is >> > a real potential bug, and the other two are mostly noise. I do not mind >> > setting a rule and fixing all of them, though. > > FWIW, I have spent about a few hours wading through the errors, and am > about 75% done. There are definitely some broken chains that were > causing test results to be ignored (as opposed to just minor setup steps > that we would not expect to fail). In most cases, the tests do passed. I > have a few that I still need to examine more closely, but there may be > some where there are actual test failures (but it's possible that I just > screwed it up while fixing the &&-chaining). > > I hope to post something tonight, but I wanted to drop a note on the off > chance that you were actively looking at it at the same time. Thanks for working on this. It looks like this technique should be a valuable addition to test-lint. (I had intended, but haven't yet found time to dig into it, so I'm happy to hear of your progress.) -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: A git hook that does git cherry-pick and push automatically
On Thu, Mar 19, 2015 at 09:07:28PM -0700, Ray Xie wrote: > So today I had a shocking moment while I was doing my cherry-pick, > after I performed all the pre-checkin duties (the usual build the > code, run the test to make sure the cherry-pick infact works), I found > out that my original commit was already cherry-picked, then I found > out someone in engineering make the decision to do an automate > cherry-pick from that branch to another so he added a git hook to run > do cherry-pick and push on every commit, yes, a simple cherry-pick > then push; mind you, these are not feature / dev branch, these are > release branches, both of them. > > Then after I came back from the shock, made a big protest about how > this is the worst thing I have seen and I will never live with it, and > that's why "git cherry-pick" and "git push" are 2 separate commands, > as any reasonable developer, you do your very best to ensure you are > not pushing something that is fundamentally broken; however for the > life of me and talk these few people into senses. > > So, I am sending this to seek for some expert advice how I can drive > some sense into these people so they don't screw up my life as an > developer. > > Regards, > Desperate developer, Ray. It's interesting that your clone picked up the hooks that they added. Do you have a centrally managed git installation, where perhaps they are modifying the default .git/hooks/ template that is used to seed the initial repo when cloning? One way to check that is to look at your git installation. If your git is installed as /usr/bin/git then check /usr/share/git-core/templates and see if the hook is present there. Assuming that you have the ability to create your own clones, and that you are not all working in a common shared sandbox, then the simplest solution would be to simply delete your .git/hooks/* when cloning a new repo. That way you can at least have your sandboxes work the way you want them to work. Hooks are local to your sandbox so you have full control over them. Is this hook resilient to rebasing? Does it cause havoc if you a feature onto this branch? Those are the kinds of things that sound scary based on your description of the hook. I don't have any tips on how to convince people. One thing that might help is to remember that people are self-motivated ~ try to focus on what *they* stand to benefit from not having this hook. If you can convince them that they are better-off without the hook, from their POV, not from yours, then you might have a better chance of persuading them. Good luck, -- David -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Why is "git fetch --prune" so much slower than "git remote prune"?
Michael Haggerty writes: > The lock conflict scenario is the only one that really worries me. Actually, I'd feel worried if this were on the receive-pack side, as it is entirely valid that two or more people make uncoordinated push into a single meeting point, but much less for something like "fetch". I do not see a valid reason why somebody cannot avoid fetching into the same repository in a racy way. > But I don't think it is *so* hard to keep the current "best-effort" > behavior. I agree that transaction is a wrong way to look at this. I view "We know packed refs is a lot more efficient if written once" as in the same category as "We know we will register many new objects with this operation, so instead of creating them into separate loose object files, declare that the object store is 'plugged' and send them into a single new packfile" (aka "bulk checkin"). Both are uninterested in all-or-none execution, but merely want to declare a boundary between a group of operations to help Git optimize them. > I am working on a function delete_refs() for the reference API > that deletes a bunch of references on a "best effort" basis, reporting > per-reference errors for any deletions that fail. I think it would make sense, but I suspect you would want to make it clear that the function is best-effort-bulk somewhere in its name (e.g. delete-refs-if-able or something) and make it not to automatically fail the surrounding transaction if any. That way an application could do things like this: transaction-begin; create-ref; update-ref; ... delete-refs-if-able; if trying to be as atomic as possible then if previous delete-refs-if-able found undeletable refs then transaction-abort; fi fi transaction-commit; Of course, supplying delete-refs that is not best-effort separately would make it even easier to code the above (the last if/then/fi will become unnecessary). -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html