Re: test -chain lint
Jeff King p...@peff.net writes:
[+cc Jonathan, whose patch I apparently subconsciously copied]
On Thu, Mar 19, 2015 at 10:08:51PM -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_ () {
test_cleanup=:
expecting_failure=$2
+
+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
+
setup_malloc_check
test_eval_ $1
eval_ret=$?
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.
I seem to recall people looked at doing this sort of lint a while ago,
but we never ended up committing anything. I wonder if it was because of
all of these false positives.
This turns out to be rather annoying to grep for in the list archives,
but I found at least one discussion:
http://article.gmane.org/gmane.comp.version-control.git/235913
I don't know why we didn't follow it up then. Perhaps because the patch
there (which is rather similar to what I have above) was not
conditional, so whole chunks of the test suite needed fixing. There are
enough problems that we would probably want to do this conditionally,
fix them over time, and then finally flip the feature on by default.
Hmmm, they do look similar and unfamiliar ;-) It happened while I
was offline, it seems.
Thanks for working on this---I think test-chain-lint should become
one of the pre-acceptance test on my end when it gets applied.
--
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: [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 p...@peff.net 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: Why is git fetch --prune so much slower than git remote prune?
Michael Haggerty mhag...@alum.mit.edu 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
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 gits...@pobox.com wrote: Graham Hay grahamr...@gmail.com 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
[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 p...@peff.net
---
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 @@ test_eval_ () {
test_run_ ()
[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 p...@peff.net
---
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 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 p...@peff.net
---
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 HEAD' '
- git update-index c {
- git diff-index --quiet --cached HEAD cnt
-
[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 p...@peff.net --- 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 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 p...@peff.net
---
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 p...@peff.net
---
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 p...@peff.net --- 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 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 p...@peff.net
---
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 /dev/null
- ret=$?
- git config sendemail.confirm ${CONFIRM:-never}
- test $ret = 0
'
test_expect_success $PREREQ 'confirm detects EOF (auto causes failure)' '
- CONFIRM=$(git config --get sendemail.confirm)
+ save_confirm
git config sendemail.confirm auto
GIT_SEND_EMAIL_NOTTY=1
export GIT_SEND_EMAIL_NOTTY
@@ -861,13 +857,10 @@ test_expect_success $PREREQ 'confirm detects EOF (auto
causes failure)' '
--to=nob...@example.com \
--smtp-server=$(pwd)/fake.sendmail \
$patches /dev/null
- ret=$?
- git config sendemail.confirm ${CONFIRM:-never}
- test $ret = 0
'
test_expect_success $PREREQ 'confirm does not loop forever' '
- CONFIRM=$(git config --get sendemail.confirm)
+ save_confirm
git config sendemail.confirm auto
GIT_SEND_EMAIL_NOTTY=1
export GIT_SEND_EMAIL_NOTTY
@@ -876,9 +869,6 @@ test_expect_success $PREREQ 'confirm does not loop forever'
'
--to=nob...@example.com \
--smtp-server=$(pwd)/fake.sendmail \
$patches
- ret=$?
- git config sendemail.confirm ${CONFIRM:-never}
- test $ret = 0
'
test_expect_success $PREREQ 'utf8 Cc is rfc2047 encoded' '
--
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 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 p...@peff.net
---
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 p...@peff.net
---
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 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 ... EOF ... EOF ) It looks like an attempt to make the -chaining more natural by letting it come at the end of the here-doc. But the extra sub-shell is so non-idiomatic (plus the lack of -) that it ends up confusing. Since these are just using a single line, we can accomplish the same thing with a single printf (which also makes the use of tab more obvious than the verbatim whitespace). Signed-off-by: Jeff King p...@peff.net --- 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.expect EOF -M same -EOF -) - touch messages.expect + test_cmp same kept + printf M\t%s\n same messages.expect test_cmp messages.expect messages ' @@ -109,10 +105,7 @@ test_expect_success checkout -m with dirty tree ' test $(git symbolic-ref HEAD) = refs/heads/side - (cat expect.messages EOF -M one -EOF -) + printf M\t%s\n one expect.messages test_cmp expect.messages messages fill M one A three D two 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
[PATCH 24/25] t0005: fix broken -chains
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 p...@peff.net --- 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 $? 13) | :) 31 ) + OUT=$( ((large_git; echo $? 13) | :) 31 ) 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 $? 13) | :) 31 ) + OUT=$( ((trap PIPE; large_git; echo $? 13) | :) 31 ) 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 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 p...@peff.net --- 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 git bisect good $HASH1 git bisect skip - if git bisect good
[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 p...@peff.net
---
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 p...@peff.net
---
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 one)
test $two = $(git hash-object --stdin dir/two)
differs=$(git diff-index --cached HEAD)
- test -z $differs || {
- echo Oops $differs
- false
- }
+ verbose test -z $differs
'
test_expect_success 'checkout with autocrlf=input' '
@@ -187,10 +169,7 @@ test_expect_success 'checkout with autocrlf=input' '
test $one = $(git hash-object --stdin one)
test $two = $(git hash-object --stdin dir/two)
differs=$(git diff-index --cached HEAD)
- test -z $differs || {
- echo Oops $differs
- false
- }
+ verbose test -z $differs
'
test_expect_success 'apply patch (autocrlf=input)' '
@@ -200,10 +179,7 @@ test_expect_success 'apply patch (autocrlf=input)' '
git read-tree --reset -u HEAD
git apply patch.file
- test $patched = $(git hash-object --stdin one) || {
- echo Eh? apply without index
- false
- }
+ verbose test $patched = $(git hash-object --stdin one)
'
test_expect_success 'apply patch --cached (autocrlf=input)' '
@@ -213,10 +189,7 @@ test_expect_success 'apply patch --cached
(autocrlf=input)' '
git read-tree --reset -u HEAD
git apply --cached patch.file
- test $patched = $(git rev-parse :one) || {
- echo Eh? apply with --cached
- false
- }
+ verbose test $patched = $(git rev-parse :one)
'
test_expect_success 'apply patch --index (autocrlf=input)' '
@@ -226,11 +199,8 @@ test_expect_success 'apply patch --index (autocrlf=input)'
'
git read-tree --reset -u HEAD
git apply --index patch.file
- test $patched = $(git rev-parse :one)
- test $patched = $(git hash-object --stdin one) || {
- echo Eh? apply with --index
- false
- }
+ verbose test $patched = $(git rev-parse :one)
+ verbose test $patched = $(git hash-object --stdin one)
'
test_expect_success 'apply patch (autocrlf=true)' '
@@ -240,10 +210,7 @@ test_expect_success 'apply patch (autocrlf=true)' '
git read-tree --reset -u HEAD
git apply patch.file
- test $patched = $(remove_cr one | git hash-object --stdin) || {
- echo Eh? apply without index
- false
- }
+ verbose test $patched = $(remove_cr one | git hash-object --stdin)
'
test_expect_success 'apply patch --cached (autocrlf=true)' '
@@ -253,10 +220,7 @@ test_expect_success 'apply patch --cached (autocrlf=true)'
'
git read-tree --reset -u HEAD
git apply --cached patch.file
- test $patched = $(git rev-parse :one) || {
- echo Eh? apply without index
- false
- }
+ verbose test $patched =
[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 p...@peff.net
---
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 p...@peff.net
---
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 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 p...@peff.net --- 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' actual rest.actual + sed -e 1d -e /^#/d actual 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 18/25] t1301: use modern test_* helpers
This shortens the code and fixes some -chaining.
Signed-off-by: Jeff King p...@peff.net
---
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
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 EOF ... whatever ... EOF ) which I suspect would be less error-prone, but is also quite ugly. I actually pulled that out in favor of a more conventional form (ironically, I found it because the author screwed up the -operator a few lines above). With automated -checking, I don't think there's any need to contort our syntax. The test suite will remind us when we forget. - the other common place I noticed was at the trailing ) of a sub-shell - Running through old tests, I saw a lot of opportunity for cleanups that I resisted. Places where we could use verbose for better output, where test_cmp would be nicer than using test X = Y, where people had indented here-docs badly (mostly failing to use -, places where people had some arcane indentation style for the test titles and snippets themselves, etc. The series was already bordering on intractable, so I tried to leave those be (though I did fix cat 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
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
[PATCH] Documentation: Add target to build PDF manpages
Signed-off-by: Thomas Schneider thosc...@gmail.com --- 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
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
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: 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 dav...@gmail.com 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
[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
[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 p...@peff.net
---
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 stdio.h 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_gettext()
fallback can interpolate v
test_expect_success
[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 p...@peff.net
---
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
- test_cmp expect check || {
- cat actual
- false
- }
+ test_cmp expect check
'
}
@@ -121,10 +118,7 @@ test_expect_success 'full history simplification without
parent' '
git log --pretty=$FMT --full-history E -- lost |
unnote actual
sed -e s/^.* \([^ ]*\) .*/\1/ check actual
- test_cmp expect check || {
- cat actual
- false
- }
+ test_cmp expect check
'
test_expect_success '--full-diff is not affected by --parents' '
diff --git a/t/t6111-rev-list-treesame.sh b/t/t6111-rev-list-treesame.sh
index 45e3673..32474c2 100755
--- a/t/t6111-rev-list-treesame.sh
+++ b/t/t6111-rev-list-treesame.sh
@@ -92,10 +92,7 @@ check_outcome () {
git log --format=$FMT $param |
unnote actual
sed -e $munge_actual 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 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 p...@peff.net
---
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} /dev/null)
A=$(git show --pretty=format:%an,%ae,%ad%n -s $C)
- test $A = A U Thor,,Thu Apr 7 15:14:13 2005 -0700 || {
- echo Eh? $A 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
[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 p...@peff.net
---
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 2err
+ test_must_fail \
+ git fast-import --import-marks=marks --export-marks=marks -EOF 2err
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
Re: What's cooking in git.git (Mar 2015, #07; Fri, 20)
On Mar 20, 2015, at 15:02, Junio C Hamano wrote: * bc/object-id (2015-03-13) 10 commits - apply: convert threeway_stage to object_id - patch-id: convert to use struct object_id - commit: convert parts to struct object_id - diff: convert struct combine_diff_path to object_id - bulk-checkin.c: convert to use struct object_id - zip: use GIT_SHA1_HEXSZ for trailers - archive.c: convert to use struct object_id - bisect.c: convert leaf functions to use struct object_id - define utility functions for object IDs - define a structure for object IDs Identify parts of the code that knows that we use SHA-1 hash to name our objects too much, and use (1) symbolic constants instead of hardcoded 20 as byte count and/or (2) use struct object_id instead of unsigned char [20] for object names. Will cook in 'next'. Has this been merged to 'next'? Even fetching github.com/gitster/ git.git I'm only seeing it in pu: $ git rev-parse bc/object-id d07d4ab401173a10173f2747cf5e0f074b6d2b39 $ git branch --contains d07d4ab401173a10173f2747cf5e0f074b6d2b39 --all bc/object-id jch pu remotes/github2/pu remotes/gob-private/pu remotes/gph/pu remotes/ko/pu remotes/repo/pu -Kyle -- 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 6:04 AM, Jeff King p...@peff.net wrote: [...] 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. :) Heh. I won't mind. Thanks for doing a thorough job. Ironically, one of the broken here-doc -links you detected with --chain-lint and fixed in 4/25 was from a patch from me: 5a9830cb (t8001/t8002 (blame): add blame -L :funcname tests, 2013-07-17). -- 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/15] http-push: Remove unneeded cleanup
preq is NULL as the condition the line before dictates. And the cleanup
function release_http_pack_request is not null pointer safe.
Signed-off-by: Stefan Beller sbel...@google.com
---
http-push.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/http-push.c b/http-push.c
index bfb1c96..c98dad2 100644
--- a/http-push.c
+++ b/http-push.c
@@ -316,7 +316,6 @@ static void start_fetch_packed(struct transfer_request
*request)
preq = new_http_pack_request(target, repo-url);
if (preq == NULL) {
- release_http_pack_request(preq);
repo-can_update_info_refs = 0;
return;
}
--
2.3.0.81.gc37f363
--
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 05/15] builtin/apply.c: fix a memleak
oldlines is allocated earlier in the function and also freed on the
successful code path.
Signed-off-by: Stefan Beller sbel...@google.com
---
builtin/apply.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/builtin/apply.c b/builtin/apply.c
index 65b97ee..e152c4d 100644
--- a/builtin/apply.c
+++ b/builtin/apply.c
@@ -2776,6 +2776,7 @@ static int apply_one_fragment(struct image *img, struct
fragment *frag,
default:
if (apply_verbosely)
error(_(invalid start of line: '%c'), first);
+ free(oldlines);
return -1;
}
if (added_blank_line) {
--
2.3.0.81.gc37f363
--
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 02/15] read-cache: Improve readability
Signed-off-by: Stefan Beller sbel...@google.com --- read-cache.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/read-cache.c b/read-cache.c index f72ea9f..769897e 100644 --- a/read-cache.c +++ b/read-cache.c @@ -703,9 +703,7 @@ int add_to_index(struct index_state *istate, const char *path, struct stat *st, !hashcmp(alias-sha1, ce-sha1) ce-ce_mode == alias-ce_mode); - if (pretend) - ; - else if (add_index_entry(istate, ce, add_option)) + if (!pretend add_index_entry(istate, ce, add_option)) return error(unable to add %s to index,path); if (verbose !was_same) printf(add '%s'\n, path); -- 2.3.0.81.gc37f363 -- 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/15] builtin/unpack-file: fix a memleak
Signed-off-by: Stefan Beller sbel...@google.com --- Notes: This is for discussion again, see 2nd previous patch. However why do we close the fd when we know the program ends soon? So let's also free the memory. builtin/unpack-file.c | 1 + 1 file changed, 1 insertion(+) diff --git a/builtin/unpack-file.c b/builtin/unpack-file.c index 1920029..ea5767e 100644 --- a/builtin/unpack-file.c +++ b/builtin/unpack-file.c @@ -17,6 +17,7 @@ static char *create_temp_file(unsigned char *sha1) if (write_in_full(fd, buf, size) != size) die_errno(unable to write temp-file); close(fd); + free(buf); return path; } -- 2.3.0.81.gc37f363 -- 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/15] builtin/merge-base: fix memleak
Signed-off-by: Stefan Beller sbel...@google.com --- Notes: see discussion on previous patch. builtin/merge-base.c | 1 + 1 file changed, 1 insertion(+) diff --git a/builtin/merge-base.c b/builtin/merge-base.c index 08a8217..4a8ff02 100644 --- a/builtin/merge-base.c +++ b/builtin/merge-base.c @@ -22,6 +22,7 @@ static int show_merge_base(struct commit **rev, int rev_nr, int show_all) result = result-next; } + free(rev); return 0; } -- 2.3.0.81.gc37f363 -- 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/15] commit.c: fix a memory leak
Signed-off-by: Stefan Beller sbel...@google.com
---
builtin/commit.c | 6 --
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/builtin/commit.c b/builtin/commit.c
index 961e467..da79ac4 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -229,7 +229,7 @@ static int commit_index_files(void)
static int list_paths(struct string_list *list, const char *with_tree,
const char *prefix, const struct pathspec *pattern)
{
- int i;
+ int i, ret;
char *m;
if (!pattern-nr)
@@ -256,7 +256,9 @@ static int list_paths(struct string_list *list, const char
*with_tree,
item-util = item; /* better a valid pointer than a
fake one */
}
- return report_path_error(m, pattern, prefix);
+ ret = report_path_error(m, pattern, prefix);
+ free(m);
+ return ret;
}
static void add_remove_files(struct string_list *list)
--
2.3.0.81.gc37f363
--
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 11/15] builtin/check-attr: fix a memleak
Signed-off-by: Stefan Beller sbel...@google.com --- Notes: I do not quite recall a discussion about such fixes when I started doing these fixes like 2 years ago. As this is a main function of a subcommand the freed memory is likely to have no impact as the process is done soon, so then it gets freed by the OS which is likey to be faster as the OS frees the whole pages of the process. Also there is no expected memory shortage as the process is going to be done soon as opposed to fixing mem leaks early in the process. An upside of fixes like this one is however to make code analysis tools produce less noise, so narrowing down the *real* issue may be easier. I wonder if we could have a 'weak' free which does nothing if git is compiled regularly and actually frees the memory if it is build with a flag to tell it to do so. This would help finding the real issues as the noise goes down and it would still be 'fast' as it could be when compiled for productive use. On the other hand I don't like to have another 'invented here again' systemcall-ish function as it would clutter the code and you'd have to remember to use that weak free. I dunno. builtin/check-attr.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/builtin/check-attr.c b/builtin/check-attr.c index 21d2bed..fa96356 100644 --- a/builtin/check-attr.c +++ b/builtin/check-attr.c @@ -182,5 +182,7 @@ int cmd_check_attr(int argc, const char **argv, const char *prefix) check_attr(prefix, cnt, check, argv[i]); maybe_flush_or_die(stdout, attribute to stdout); } + + free(check); return 0; } -- 2.3.0.81.gc37f363 -- 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 00/15] Fixing memory leaks
So here comes another bunch of mem leak fixes. While I consider the first 10 patches a pretty safe bet, the last 5 hopefully spark a discussion if we want patches which just clean up before the program ends. Stefan Beller (15): read-cache: fix memleak read-cache: Improve readability read-cache: free cache entry in add_to_index in case of early return update-index: fix a memleak builtin/apply.c: fix a memleak merge-blobs.c: Fix a memleak merge-recursive: fix memleaks http-push: Remove unneeded cleanup http: release the memory of a http pack request as well commit.c: fix a memory leak builtin/check-attr: fix a memleak builtin/merge-base: fix memleak builtin/unpack-file: fix a memleak builtin/cat-file: free memleak ls-files: fix a memleak builtin/apply.c| 1 + builtin/cat-file.c | 1 + builtin/check-attr.c | 2 ++ builtin/commit.c | 6 -- builtin/ls-files.c | 1 + builtin/merge-base.c | 1 + builtin/unpack-file.c | 1 + builtin/update-index.c | 1 + http-push.c| 1 - http.c | 1 + merge-blobs.c | 4 +++- merge-recursive.c | 3 +++ read-cache.c | 13 - 13 files changed, 27 insertions(+), 9 deletions(-) -- 2.3.0.81.gc37f363 -- 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/15] builtin/cat-file: free memleak
Signed-off-by: Stefan Beller sbel...@google.com --- Notes: see discussion builtin/cat-file.c | 1 + 1 file changed, 1 insertion(+) diff --git a/builtin/cat-file.c b/builtin/cat-file.c index df99df4..8de6b0d 100644 --- a/builtin/cat-file.c +++ b/builtin/cat-file.c @@ -104,6 +104,7 @@ static int cat_one_file(int opt, const char *exp_type, const char *obj_name) die(git cat-file %s: bad file, obj_name); write_or_die(1, buf, size); + free(buf); return 0; } -- 2.3.0.81.gc37f363 -- 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/15] ls-files: fix a memleak
Signed-off-by: Stefan Beller sbel...@google.com --- Notes: see discussion builtin/ls-files.c | 1 + 1 file changed, 1 insertion(+) diff --git a/builtin/ls-files.c b/builtin/ls-files.c index 914054d..306defa 100644 --- a/builtin/ls-files.c +++ b/builtin/ls-files.c @@ -590,6 +590,7 @@ int cmd_ls_files(int argc, const char **argv, const char *cmd_prefix) if (bad) fprintf(stderr, Did you forget to 'git add'?\n); + free(ps_matched); return bad ? 1 : 0; } -- 2.3.0.81.gc37f363 -- 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/15] merge-blobs.c: Fix a memleak
Signed-off-by: Stefan Beller sbel...@google.com
---
merge-blobs.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/merge-blobs.c b/merge-blobs.c
index 57211bc..7abb894 100644
--- a/merge-blobs.c
+++ b/merge-blobs.c
@@ -14,8 +14,10 @@ static int fill_mmfile_blob(mmfile_t *f, struct blob *obj)
buf = read_sha1_file(obj-object.sha1, type, size);
if (!buf)
return -1;
- if (type != OBJ_BLOB)
+ if (type != OBJ_BLOB) {
+ free(buf);
return -1;
+ }
f-ptr = buf;
f-size = size;
return 0;
--
2.3.0.81.gc37f363
--
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/15] update-index: fix a memleak
`old` is not used outside the loop and would get lost once we reach the goto. Signed-off-by: Stefan Beller sbel...@google.com --- builtin/update-index.c | 1 + 1 file changed, 1 insertion(+) diff --git a/builtin/update-index.c b/builtin/update-index.c index 5878986..6271b54 100644 --- a/builtin/update-index.c +++ b/builtin/update-index.c @@ -584,6 +584,7 @@ static int do_reupdate(int ac, const char **av, path = xstrdup(ce-name); update_one(path); free(path); + free(old); if (save_nr != active_nr) goto redo; } -- 2.3.0.81.gc37f363 -- 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 03/15] read-cache: free cache entry in add_to_index in case of early return
This frees `ce` would be leaking in the error path.
Additionally a free is moved towards the return. This helps code
readability as we often have this pattern of freeing resources just
before return/exit and not in between the code.
Signed-off-by: Stefan Beller sbel...@google.com
---
read-cache.c | 7 +--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/read-cache.c b/read-cache.c
index 769897e..935f91a 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -681,15 +681,18 @@ int add_to_index(struct index_state *istate, const char
*path, struct stat *st,
alias = index_file_exists(istate, ce-name, ce_namelen(ce),
ignore_case);
if (alias !ce_stage(alias) !ie_match_stat(istate, alias, st,
ce_option)) {
/* Nothing changed, really */
- free(ce);
if (!S_ISGITLINK(alias-ce_mode))
ce_mark_uptodate(alias);
alias-ce_flags |= CE_ADDED;
+
+ free(ce);
return 0;
}
if (!intent_only) {
- if (index_path(ce-sha1, path, st, HASH_WRITE_OBJECT))
+ if (index_path(ce-sha1, path, st, HASH_WRITE_OBJECT)) {
+ free(ce);
return error(unable to index file %s, path);
+ }
} else
set_object_name_for_intent_to_add_entry(ce);
--
2.3.0.81.gc37f363
--
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 01/15] read-cache: fix memleak
`ce` is allocated in make_cache_entry and should be freed if it is not
used any more. refresh_cache_entry as a wrapper around refresh_cache_ent
will either return `ce` or a new updated cache entry which is allocated
to new memory. In that case we need to free `ce` ourselfs.
Signed-off-by: Stefan Beller sbel...@google.com
---
read-cache.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/read-cache.c b/read-cache.c
index 8d71860..f72ea9f 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -747,6 +747,8 @@ struct cache_entry *make_cache_entry(unsigned int mode,
free(ce);
return NULL;
} else {
+ if (ret != ce)
+ free(ce);
return ret;
}
}
--
2.3.0.81.gc37f363
--
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/15] merge-recursive: fix memleaks
Usually when using string lists it looks like: struct string_list a = STRING_LIST_INIT_NODUP; // do stuff with a such as string_list_insert(a, test string); print_string_list(a, test prefix); // Cleaning up works on everything inside the struct, not on the // struct itself: string_list_clear(a); But as we deal with the pointers to the string lists directly, we also need to free the actual struct. Signed-off-by: Stefan Beller sbel...@google.com --- merge-recursive.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/merge-recursive.c b/merge-recursive.c index 771f5e2..1c9c30d 100644 --- a/merge-recursive.c +++ b/merge-recursive.c @@ -1858,6 +1858,9 @@ int merge_trees(struct merge_options *o, string_list_clear(re_head, 0); string_list_clear(entries, 1); + free(re_merge); + free(re_head); + free(entries); } else clean = 1; -- 2.3.0.81.gc37f363 -- 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 09/15] http: release the memory of a http pack request as well
The cleanup function is used in 4 places now and it's always safe to free up the memory as well. Signed-off-by: Stefan Beller sbel...@google.com --- http.c | 1 + 1 file changed, 1 insertion(+) diff --git a/http.c b/http.c index 9c825af..4b179f6 100644 --- a/http.c +++ b/http.c @@ -1462,6 +1462,7 @@ void release_http_pack_request(struct http_pack_request *preq) } preq-slot = NULL; free(preq-url); + free(preq); } int finish_http_pack_request(struct http_pack_request *preq) -- 2.3.0.81.gc37f363 -- 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: What's cooking in git.git (Mar 2015, #07; Fri, 20)
On Mar 20, 2015, at 16:29, Stefan Beller wrote: On Fri, Mar 20, 2015 at 4:24 PM, Kyle J. McKay mack...@gmail.com wrote: On Mar 20, 2015, at 15:02, Junio C Hamano wrote: * bc/object-id (2015-03-13) 10 commits [snip] Will cook in 'next'. Has this been merged to 'next'? Usually Junio writes the mail first and then does a git push all the branches just before being done for the day. At least that's my suspicion as an observer of the timing when git fetch returns new shiny stuff and when these emails are sent. I would expect that if it said, Will merge to 'next'. However the What's cooking in git.git (Mar 2015, #06; Tue, 17) also says Will cook in 'next' for this topic so I think that perhaps it's fallen through the cracks somehow. -Kyle -- 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: What's cooking in git.git (Mar 2015, #07; Fri, 20)
On Fri, Mar 20, 2015 at 4:24 PM, Kyle J. McKay mack...@gmail.com wrote: Has this been merged to 'next'? Usually Junio writes the mail first and then does a git push all the branches just before being done for the day. At least that's my suspicion as an observer of the timing when git fetch returns new shiny stuff and when these emails are sent. -- 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
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
[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 g...@drmicha.warpmail.net --- 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
[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 g...@drmicha.warpmail.net --- 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
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
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, obviously... I'll follow up with two patches for those. Michael -- To unsubscribe from this list: send the line unsubscribe git in
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/mastTAB 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.gumme...@gmail.com --- 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 expect EOF -own 8299b0bcd1ac364e5f1d7768efb62fa2da79a339 -base 39d890139ee5356c7ef572216cebcd27aa41f9df +own $own +base $base replacements: deletions: EOF @@ -30,7 +39,7 @@ EOF test-dump-split-index .git/index | sed /^own/d actual cat expect EOF -base 39d890139ee5356c7ef572216cebcd27aa41f9df +base $base 100644 e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 0 one replacements: deletions: -- 2.3.3.377.gdac1145 -- 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 03/15] read-cache: free cache entry in add_to_index in case of early return
Stefan Beller sbel...@google.com writes: This frees `ce` would be leaking in the error path. At this point ce is not yet added to the index, so it is clear it is safe to free it---otherwise we will leak it. Good. Additionally a free is moved towards the return. I am on the fence on this one between two schools and do not have a strong preference. One school is to free as soon as you know you do not need it, which is a valid stance to take. Another is, as you did, not to care about the minimum necessary lifetime of the storage and free them all at the end, which is also valid. Technically, the former could be more performant while the latter is easier on the eyes. -- 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 04/15] update-index: fix a memleak
Stefan Beller sbel...@google.com writes: `old` is not used outside the loop and would get lost once we reach the goto. Signed-off-by: Stefan Beller sbel...@google.com --- builtin/update-index.c | 1 + 1 file changed, 1 insertion(+) diff --git a/builtin/update-index.c b/builtin/update-index.c index 5878986..6271b54 100644 --- a/builtin/update-index.c +++ b/builtin/update-index.c @@ -584,6 +584,7 @@ static int do_reupdate(int ac, const char **av, path = xstrdup(ce-name); update_one(path); free(path); + free(old); The change looks trivially correct. A tangent; I wonder if we want to make path a strbuf that is declared in the function scope, reset (not released) at each iteration of the loop and released after the loop; keep allocating and freeing a small piece of string all the time feels somewhat wasteful. Also, we might want to add to the Be careful comment to describe why this cannot just be a call to update_one(ce-name) that does not use path. -- 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 3/4] docs/git-credential-store: document XDG file and precedence
Hi, On Thu, Mar 19, 2015 at 12:23 AM, Matthieu Moy matthieu@grenoble-inp.fr wrote: I would personnally prefer to see this squashed with PATCH 2/4: pushing the bisectable history principle a bit, the state between patches 2 and 3 could be considered broken because the code does not do what the documentation says. And as a reviewer, I like having pieces of docs linked to the patch they document. Yup, I can see what you mean. Will squash on the next version. Paul Tan pyoka...@gmail.com writes: +Credential storage will per default Not a native, but per default sounds weird and by default seems far more common. Ah right, that definitely sounds better. Thanks. -- 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 02/15] read-cache: Improve readability
On Fri, Mar 20, 2015 at 9:19 PM, Junio C Hamano gits...@pobox.com wrote:
Stefan Beller sbel...@google.com writes:
Signed-off-by: Stefan Beller sbel...@google.com
---
read-cache.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/read-cache.c b/read-cache.c
index f72ea9f..769897e 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -703,9 +703,7 @@ int add_to_index(struct index_state *istate, const char
*path, struct stat *st,
!hashcmp(alias-sha1, ce-sha1)
ce-ce_mode == alias-ce_mode);
- if (pretend)
- ;
- else if (add_index_entry(istate, ce, add_option))
+ if (!pretend add_index_entry(istate, ce, add_option))
return error(unable to add %s to index,path);
if (verbose !was_same)
printf(add '%s'\n, path);
I have a moderately strong feeling against this change, as the code
was done this way quite deliberately to keep it readable, namely, to
avoid using to concatenate two boolean expressions that are in
totally different class inside condition part of if/while, where A
is a precondition guard for B (i.e. you cannot evaluate B unless A
holds) and B is called primarily for its side effect. The problem
is that, once you start liberally doing
if (A B C D ...)
with booleans with mixed semantics (guards and actions), it will
quickly get harder to tell which one is which.
I could have written it as
if (!pretend) {
if (add_index_entry(...))
return error(...);
}
This makes sense to point out the different semantics to me.
Maybe I have read too much of the refs code lately as there we
have these long chains which combine precondition with error
checking. :/ That's why I thought it would be global to git to not
care much about this semantics distinction.
and that would have been just as readable as the original; it
clearly separates the guard (i.e. only do the add-index thing when
we are not pretending) and the operation that is done for the side
effect.
But I find the original tells you if pretend mode, do *nothing*
and otherwise, try add_index_entry() and act on its error very
clearly. Of course, I am biased as the original is my code from
38ed1d89 (git-add -n -u should not add but just report,
2008-05-21).
FYI, between preference and taste, I'd say this one is much closer
to the latter than the former.
By the way, aren't we leaking ce when we are merely pretending?
Yes we are, that's how I found this spot. (coverity pointed out ce was
leaking, so I was refactoring to actually make it easier to fix it, and then
heavily reordered the patch series afterwards. That spot was forgotten
apparently.
--
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 03/15] read-cache: free cache entry in add_to_index in case of early return
On Fri, Mar 20, 2015 at 8:31 PM, Junio C Hamano gits...@pobox.com wrote: Stefan Beller sbel...@google.com writes: This frees `ce` would be leaking in the error path. At this point ce is not yet added to the index, so it is clear it is safe to free it---otherwise we will leak it. Good. Additionally a free is moved towards the return. I am on the fence on this one between two schools and do not have a strong preference. One school is to free as soon as you know you do not need it, which is a valid stance to take. Another is, as you did, not to care about the minimum necessary lifetime of the storage and free them all at the end, which is also valid. Technically, the former could be more performant while the latter is easier on the eyes. I only recall to have seen the latter school so far, which is why I made the change in the first place assuming the school of freeing ASAP has no strong supporters inside the git community. I can resend the patch dropping the reordering, if you prefer. -- 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: What's cooking in git.git (Mar 2015, #07; Fri, 20)
Kyle J. McKay mack...@gmail.com writes: Will cook in 'next'. Has this been merged to 'next'? Even fetching github.com/gitster/git.git I'm only seeing it in pu: That was a short-hand for will merge and cook in 'next' ;-) I had an impression that the series may see at least one reroll to polish it further before it gets ready for 'next', as I only saw discussions on the tangent (e.g. possible futures) and didn't see serious reviews of the code that we will actually be using. -- 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 00/15] Fixing memory leaks
Stefan Beller sbel...@google.com writes: So here comes another bunch of mem leak fixes. Looked mostly sensible. -- 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 10/15] commit.c: fix a memory leak
Stefan Beller sbel...@google.com writes:
Signed-off-by: Stefan Beller sbel...@google.com
---
builtin/commit.c | 6 --
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/builtin/commit.c b/builtin/commit.c
index 961e467..da79ac4 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -229,7 +229,7 @@ static int commit_index_files(void)
static int list_paths(struct string_list *list, const char *with_tree,
const char *prefix, const struct pathspec *pattern)
{
- int i;
+ int i, ret;
char *m;
if (!pattern-nr)
@@ -256,7 +256,9 @@ static int list_paths(struct string_list *list, const
char *with_tree,
item-util = item; /* better a valid pointer than a
fake one */
}
- return report_path_error(m, pattern, prefix);
+ ret = report_path_error(m, pattern, prefix);
+ free(m);
+ return ret;
}
Looks correct.
A tangent. We may want to move report_path_error() to somewhere
more common-ish, like dir.c which is where we have bulk of
pathspec matching infrastructure. Seeing the function used from
builtin/ls-files.c makes me feel dirty.
A further tangent (Duy Cc'ed for this point). We might want to
rethink the interface to ce_path_match() and report_path_error()
so that we do not have to do a separate allocation of has this
pathspec been used? array. This was a remnant from the olden days
back when pathspec were mere const char ** where we did not have
any room to add per-item bit---these days pathspec is repreasented
as an array of struct pathspec and we can afford to add a bit
to the structure---which will make this kind of leak much less
likely to happen.
--
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 07/15] merge-recursive: fix memleaks
Stefan Beller sbel...@google.com writes: Usually when using string lists it looks like: struct string_list a = STRING_LIST_INIT_NODUP; // do stuff with a such as string_list_insert(a, test string); print_string_list(a, test prefix); // Cleaning up works on everything inside the struct, not on the // struct itself: string_list_clear(a); But as we deal with the pointers to the string lists directly, we also need to free the actual struct. In other words, these two were allocated for the sole use of this fuction by get_renames(), so this function is responsible for freeing it? Sounds sensible. Signed-off-by: Stefan Beller sbel...@google.com --- merge-recursive.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/merge-recursive.c b/merge-recursive.c index 771f5e2..1c9c30d 100644 --- a/merge-recursive.c +++ b/merge-recursive.c @@ -1858,6 +1858,9 @@ int merge_trees(struct merge_options *o, string_list_clear(re_head, 0); string_list_clear(entries, 1); + free(re_merge); + free(re_head); + free(entries); } else clean = 1; -- 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 02/15] read-cache: Improve readability
Stefan Beller sbel...@google.com writes:
Signed-off-by: Stefan Beller sbel...@google.com
---
read-cache.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/read-cache.c b/read-cache.c
index f72ea9f..769897e 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -703,9 +703,7 @@ int add_to_index(struct index_state *istate, const char
*path, struct stat *st,
!hashcmp(alias-sha1, ce-sha1)
ce-ce_mode == alias-ce_mode);
- if (pretend)
- ;
- else if (add_index_entry(istate, ce, add_option))
+ if (!pretend add_index_entry(istate, ce, add_option))
return error(unable to add %s to index,path);
if (verbose !was_same)
printf(add '%s'\n, path);
I have a moderately strong feeling against this change, as the code
was done this way quite deliberately to keep it readable, namely, to
avoid using to concatenate two boolean expressions that are in
totally different class inside condition part of if/while, where A
is a precondition guard for B (i.e. you cannot evaluate B unless A
holds) and B is called primarily for its side effect. The problem
is that, once you start liberally doing
if (A B C D ...)
with booleans with mixed semantics (guards and actions), it will
quickly get harder to tell which one is which.
I could have written it as
if (!pretend) {
if (add_index_entry(...))
return error(...);
}
and that would have been just as readable as the original; it
clearly separates the guard (i.e. only do the add-index thing when
we are not pretending) and the operation that is done for the side
effect.
But I find the original tells you if pretend mode, do *nothing*
and otherwise, try add_index_entry() and act on its error very
clearly. Of course, I am biased as the original is my code from
38ed1d89 (git-add -n -u should not add but just report,
2008-05-21).
FYI, between preference and taste, I'd say this one is much closer
to the latter than the former.
By the way, aren't we leaking ce when we are merely pretending?
--
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/15] read-cache: fix memleak
Stefan Beller sbel...@google.com writes:
`ce` is allocated in make_cache_entry and should be freed if it is not
used any more. refresh_cache_entry as a wrapper around refresh_cache_ent
will either return `ce` or a new updated cache entry which is allocated
to new memory. In that case we need to free `ce` ourselfs.
Signed-off-by: Stefan Beller sbel...@google.com
---
read-cache.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/read-cache.c b/read-cache.c
index 8d71860..f72ea9f 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -747,6 +747,8 @@ struct cache_entry *make_cache_entry(unsigned int mode,
free(ce);
return NULL;
} else {
+ if (ret != ce)
+ free(ce);
return ret;
}
}
Good, I vaguely recall that we did something similar in another
codepath that forgot the fact that refresh_cache_entry() may make
the incoming ce unnecessary.
As the rule is if ret is different from ce, then ce must be freed
in this codepath, I wonder if this is easier to read:
ret = refresh_cache_entry(ce, ...);
if (ret != ce)
free(ce);
return ret;
--
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 05/15] builtin/apply.c: fix a memleak
Stefan Beller sbel...@google.com writes:
oldlines is allocated earlier in the function and also freed on the
successful code path.
Signed-off-by: Stefan Beller sbel...@google.com
---
builtin/apply.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/builtin/apply.c b/builtin/apply.c
index 65b97ee..e152c4d 100644
--- a/builtin/apply.c
+++ b/builtin/apply.c
@@ -2776,6 +2776,7 @@ static int apply_one_fragment(struct image *img, struct
fragment *frag,
default:
if (apply_verbosely)
error(_(invalid start of line: '%c'), first);
+ free(oldlines);
return -1;
Good.
By the way, aren't the following also leaking here?
- the strbuf newlines that starts out as size
- line[] arrays of preimage and postimage
}
if (added_blank_line) {
--
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 4/4] t0302: test credential-store support for XDG_CONFIG_HOME
Hi, On Thu, Mar 19, 2015 at 3:26 AM, Eric Sunshine sunsh...@sunshineco.com wrote: On Wed, Mar 18, 2015 at 3:04 AM, Paul Tan pyoka...@gmail.com wrote: diff --git a/t/t0302-credential-store.sh b/t/t0302-credential-store.sh index f61b40c..5b0a666 100755 --- a/t/t0302-credential-store.sh +++ b/t/t0302-credential-store.sh @@ -6,4 +6,115 @@ test_description='credential-store tests' helper_test store +test_expect_success 'when xdg credentials file does not exist, only write to ~/.git-credentials; do not create xdg file' ' These test descriptions are quite long, often mirroring in prose what the test body already says more concisely. I generally try to keep descriptions short while still being descriptive enough to give a general idea about what is being tested. I've rewritten a few test descriptions (below) to be very short, in fact probably too terse; but perhaps better descriptions would lie somewhere between the two extremes. First example, for this test: !xdg: .git-credentials !xdg Key: Space-separated items. Items before : are the pre-conditions, and items after are the post-state. ! file does not exist; output goes here. I will make the test descriptions shorter. However, I don't think the test descriptions need to be so terse such that a separate key is required. e.g. I will shorten the above to when xdg file does not exist, it's not created., or even terser: when xdg file not exists, it's not created.. I don't think symbols should be used, as many other test descriptions do not use them. + test -s $HOME/.git-credentials + test_path_is_missing $HOME/.config/git/credentials +' + +test_expect_success 'create $XDG_CONFIG_HOME/git/credentials file' ' It's customary to call this setup rather than create. Will fix. Terse version: setup: -.git-redentials +xdg Key: - file removed; + file created. How about just setup xdg file (the fact that ~/.git-credentials is not created/deleted is implied in the next test) + rm -f $HOME/.git-credentials + mkdir -p $HOME/.config/git + $HOME/.config/git/credentials +' + +helper_test store + +test_expect_success 'when xdg credentials file exists, only write to xdg file; do not create ~/.git-credentials' ' Terse version: !.git-credentials xdg: !.git-credentials xdg How about when xdg file exists, home file not created + test -s $HOME/.config/git/credentials + test_path_is_missing $HOME/.git-credentials +' + +test_expect_success 'set up custom XDG_CONFIG_HOME credential file at ~/xdg/git/credentials' ' s/set up/setup/ Will fix. Thanks. Terse: setup custom-xdg It's a matter of taste, but I personally don't see the need for hyphenation. How about: setup custom xdg file + mkdir -p $HOME/xdg/git + rm -f $HOME/.config/git/credentials + $HOME/xdg/git/credentials It would be easier to read this if you placed the two lines together which refer to the custom xdg file. Also, for completeness and to be self-contained, don't you want to remove ~/.git-credentials? Ah yes, thanks for the suggestion. rm -f $HOME/.git-credentials rm -f $HOME/.config/git/credentials mkdir -p $HOME/xdg/git $HOME/xdg/git/credentials +' + +XDG_CONFIG_HOME=$HOME/xdg export XDG_CONFIG_HOME helper_test store +unset XDG_CONFIG_HOME It's hard to spot the helper_test store at the end of line. I'd place it on a line by itself so that it is easy to see that it is wrapped by the setting and unsetting of the environment variable. Thanks, will fix. Although now it looks weird that the export is the only one with a continuation on a single line, so I split all of them so that they each have their own line. +test_expect_success 'if custom XDG_CONFIG_HOME credentials file ~/xdg/git/credentials exists, it will only be written to; ~/.config/git/credentials and ~/.git-credentials will not be created' ' Terse: !.git-credentials !xdg custom-xdg: !.git-credentials !xdg custom-xdg + test_when_finished rm -f $HOME/xdg/git/credentials + test -s $HOME/xdg/git/credentials + test_path_is_missing $HOME/.config/git/credentials Matthieu already pointed out the broken -chain. Will fix. Thanks for catching it. + test_path_is_missing $HOME/.git-credentials +' + +test_expect_success 'get: return credentials from home file if matches are found in both home and xdg files' ' Terse: .git-credentials xdg: .git-credentials Key: taken from here. How about use home file if both home and xdg files have matches. I wish to make it explicit that which files are used depends on whether they have the matching credential, not if they exist. + mkdir -p $HOME/.config/git + echo https://xdg-user:xdg-p...@example.com; $HOME/.config/git/credentials + echo https://home-user:home-p...@example.com; $HOME/.git-credentials + check fill store -\EOF + protocol=https + host=example.com
[PATCH v2 3/5] prune: turn on ref_paranoia flag
Prune should know about broken objects at the tips of refs, so that we can feed them to our traversal rather than ignoring them. It's better for us to abort the operation on the broken object than it is to start deleting objects with an incomplete view of the reachability namespace. Note that for missing objects, aborting is the best we can do. For a badly-named ref, we technically could use its sha1 as a reachability tip. However, the iteration code just feeds us a null sha1, so there would be a reasonable amount of code involved to pass down our wishes. It's not really worth trying to do better, because this is a case that should happen extremely rarely, and the message we provide: fatal: unable to parse object: refs/heads/bogus:name is probably enough to point the user in the right direction. Signed-off-by: Jeff King p...@peff.net --- builtin/prune.c | 1 + t/t5312-prune-corruption.sh | 4 ++-- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/builtin/prune.c b/builtin/prune.c index 04d3b12..17094ad 100644 --- a/builtin/prune.c +++ b/builtin/prune.c @@ -115,6 +115,7 @@ int cmd_prune(int argc, const char **argv, const char *prefix) expire = ULONG_MAX; save_commit_buffer = 0; check_replace_refs = 0; + ref_paranoia = 1; init_revisions(revs, prefix); argc = parse_options(argc, argv, prefix, options, prune_usage, 0); diff --git a/t/t5312-prune-corruption.sh b/t/t5312-prune-corruption.sh index 496a9f5..5ffb817 100755 --- a/t/t5312-prune-corruption.sh +++ b/t/t5312-prune-corruption.sh @@ -25,7 +25,7 @@ test_expect_success 'create history reachable only from a bogus-named ref' ' git reset --hard HEAD^ ' -test_expect_failure 'pruning does not drop bogus object' ' +test_expect_success 'pruning does not drop bogus object' ' test_when_finished git hash-object -w -t commit saved test_might_fail git prune --expire=now verbose git cat-file -e $bogus @@ -72,7 +72,7 @@ test_expect_success 'create history with missing tip commit' ' test_must_fail git cat-file -e $missing ' -test_expect_failure 'pruning with a corrupted tip does not drop history' ' +test_expect_success 'pruning with a corrupted tip does not drop history' ' test_when_finished git hash-object -w -t commit saved test_might_fail git prune --expire=now verbose git cat-file -e $recoverable -- 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
Re: [PATCH 0/25] detecting -chain breakage
Jeff King p...@peff.net writes: I'm actually about to send out a re-roll of that, as I think all of the review comments have been addressed. Thanks. -- 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 v2 5/5] refs.c: drop curate_packed_refs
When we delete a ref, we have to rewrite the entire
packed-refs file. We take this opportunity to curate the
packed-refs file and drop any entries that are crufty or
broken.
Dropping broken entries (e.g., with bogus names, or ones
that point to missing objects) is actively a bad idea, as it
means that we lose any notion that the data was there in the
first place. Aside from the general hackiness that we might
lose any information about ref foo while deleting an
unrelated ref bar, this may seriously hamper any attempts
by the user at recovering from the corruption in foo.
They will lose the sha1 and name of foo; the exact pointer
may still be useful even if they recover missing objects
from a different copy of the repository. But worse, once the
ref is gone, there is no trace of the corruption. A
follow-up git prune may delete objects, even though it
would otherwise bail when seeing corruption.
We could just drop the broken bits from
curate_packed_refs, and continue to drop the crufty bits:
refs whose loose counterpart exists in the filesystem. This
is not wrong to do, and it does have the advantage that we
may write out a slightly smaller packed-refs file. But it
has two disadvantages:
1. It is a potential source of races or mistakes with
respect to these refs that are otherwise unrelated to
the operation. To my knowledge, there aren't any active
problems in this area, but it seems like an unnecessary
risk.
2. We have to spend time looking up the matching loose
refs for every item in the packed-refs file. If you
have a large number of packed refs that do not change,
that outweighs the benefit from writing out a smaller
packed-refs file (it doesn't get smaller, and you do a
bunch of directory traversal to find that out).
Signed-off-by: Jeff King p...@peff.net
---
refs.c | 67 +
t/t5312-prune-corruption.sh | 2 +-
2 files changed, 2 insertions(+), 67 deletions(-)
diff --git a/refs.c b/refs.c
index 7f0e7be..47e4e53 100644
--- a/refs.c
+++ b/refs.c
@@ -2621,68 +2621,10 @@ int pack_refs(unsigned int flags)
return 0;
}
-/*
- * If entry is no longer needed in packed-refs, add it to the string
- * list pointed to by cb_data. Reasons for deleting entries:
- *
- * - Entry is broken.
- * - Entry is overridden by a loose ref.
- * - Entry does not point at a valid object.
- *
- * In the first and third cases, also emit an error message because these
- * are indications of repository corruption.
- */
-static int curate_packed_ref_fn(struct ref_entry *entry, void *cb_data)
-{
- struct string_list *refs_to_delete = cb_data;
-
- if (entry-flag REF_ISBROKEN) {
- /* This shouldn't happen to packed refs. */
- error(%s is broken!, entry-name);
- string_list_append(refs_to_delete, entry-name);
- return 0;
- }
- if (!has_sha1_file(entry-u.value.sha1)) {
- unsigned char sha1[20];
- int flags;
-
- if (read_ref_full(entry-name, 0, sha1, flags))
- /* We should at least have found the packed ref. */
- die(Internal error);
- if ((flags REF_ISSYMREF) || !(flags REF_ISPACKED)) {
- /*
-* This packed reference is overridden by a
-* loose reference, so it is OK that its value
-* is no longer valid; for example, it might
-* refer to an object that has been garbage
-* collected. For this purpose we don't even
-* care whether the loose reference itself is
-* invalid, broken, symbolic, etc. Silently
-* remove the packed reference.
-*/
- string_list_append(refs_to_delete, entry-name);
- return 0;
- }
- /*
-* There is no overriding loose reference, so the fact
-* that this reference doesn't refer to a valid object
-* indicates some kind of repository corruption.
-* Report the problem, then omit the reference from
-* the output.
-*/
- error(%s does not point to a valid object!, entry-name);
- string_list_append(refs_to_delete, entry-name);
- return 0;
- }
-
- return 0;
-}
-
int repack_without_refs(struct string_list *refnames, struct strbuf *err)
{
struct ref_dir *packed;
- struct string_list refs_to_delete = STRING_LIST_INIT_DUP;
- struct string_list_item *refname, *ref_to_delete;
+ struct string_list_item *refname;
int ret, needs_repacking = 0, removed = 0;
assert(err);
@@ -2718,13 +2660,6 @@ int repack_without_refs(struct
[PATCH v2 4/5] repack: turn on ref paranoia when doing a destructive repack
If we are repacking with -ad, we will drop any unreachable
objects. Likewise, using -Ad --unpack-unreachable=time
will drop any old, unreachable objects. In these cases, we
want to make sure the reachability we compute with --all
is complete. We can do this by passing GIT_REF_PARANOIA=1 in
the environment to pack-objects.
Note that -Ad is safe already, because it only loosens
unreachable objects. It is up to git prune to avoid
deleting them.
Signed-off-by: Jeff King p...@peff.net
---
builtin/repack.c| 8 ++--
t/t5312-prune-corruption.sh | 2 +-
2 files changed, 7 insertions(+), 3 deletions(-)
diff --git a/builtin/repack.c b/builtin/repack.c
index 28fbc70..f2edeb0 100644
--- a/builtin/repack.c
+++ b/builtin/repack.c
@@ -228,13 +228,17 @@ int cmd_repack(int argc, const char **argv, const char
*prefix)
get_non_kept_pack_filenames(existing_packs);
if (existing_packs.nr delete_redundant) {
- if (unpack_unreachable)
+ if (unpack_unreachable) {
argv_array_pushf(cmd.args,
--unpack-unreachable=%s,
unpack_unreachable);
- else if (pack_everything LOOSEN_UNREACHABLE)
+ argv_array_push(cmd.env_array,
GIT_REF_PARANOIA=1);
+ } else if (pack_everything LOOSEN_UNREACHABLE) {
argv_array_push(cmd.args,
--unpack-unreachable);
+ } else {
+ argv_array_push(cmd.env_array,
GIT_REF_PARANOIA=1);
+ }
}
} else {
argv_array_push(cmd.args, --unpacked);
diff --git a/t/t5312-prune-corruption.sh b/t/t5312-prune-corruption.sh
index 5ffb817..e8d04ef 100755
--- a/t/t5312-prune-corruption.sh
+++ b/t/t5312-prune-corruption.sh
@@ -38,7 +38,7 @@ test_expect_success 'put bogus object into pack' '
verbose git cat-file -e $bogus
'
-test_expect_failure 'destructive repack keeps packed object' '
+test_expect_success 'destructive repack keeps packed object' '
test_might_fail git repack -Ad --unpack-unreachable=now
verbose git cat-file -e $bogus
test_might_fail git repack -ad
--
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 v2 0/5] not making corruption worse
This is a re-roll of the series to make git prune in a corrupted repository safer. There are only minor tweaks from v1, but I think all of the raised issues were addressed (there was discussion on some other points, but I think they are OK as-is; more discussion is of course welcome). The changes from v1 are: - use bogus..name as a bad refname instead of bogus:name, as the latter has problems on Windows - fix broken -chains in tests - better commenting of missing-commit setup in t5312 - typo-fixes in commit messages Patches: [1/5]: t5312: test object deletion code paths in a corrupted repository [2/5]: refs: introduce a ref paranoia flag [3/5]: prune: turn on ref_paranoia flag [4/5]: repack: turn on ref paranoia when doing a destructive repack [5/5]: refs.c: drop curate_packed_refs -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 v2] 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 the test suite is run with TEST_GIT_INDEX_VERSION=4. Fix it. Signed-off-by: Thomas Gummerer t.gumme...@gmail.com --- An updated patch to mention when run with TEST_GIT_INDEX_VERSION=4 in the log message was what I was asking for ;-) Sorry I didn't catch that. Here it is. 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 expect EOF -own 8299b0bcd1ac364e5f1d7768efb62fa2da79a339 -base 39d890139ee5356c7ef572216cebcd27aa41f9df +own $own +base $base replacements: deletions: EOF @@ -30,7 +39,7 @@ EOF test-dump-split-index .git/index | sed /^own/d actual cat expect EOF -base 39d890139ee5356c7ef572216cebcd27aa41f9df +base $base 100644 e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 0 one replacements: deletions: -- 2.3.3.377.gdac1145 -- 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 v2 1/5] t5312: test object deletion code paths in a corrupted repository
When we are doing a destructive operation like git prune, we want to be extra careful that the set of reachable tips we compute is valid. If there is any corruption or oddity, we are better off aborting the operation and letting the user figure things out rather than plowing ahead and possibly deleting some data that cannot be recovered. The tests here include: 1. Pruning objects mentioned only be refs with invalid names. This used to abort prior to d0f810f (refs.c: allow listing and deleting badly named refs, 2014-09-03), but since then we silently ignore the tip. Likewise, we test repacking that can drop objects (either -ad, which drops anything unreachable, or -Ad --unpack-unreachable=time, which tries to optimize out a loose object write that would be directly pruned). 2. Pruning objects when some refs point to missing objects. We don't know whether any dangling objects would have been reachable from the missing objects. We are better to keep them around, as they are better than nothing for helping the user recover history. 3. Packed refs that point to missing objects can sometimes be dropped. By itself, this is more of an annoyance (you do not have the object anyway; even if you can recover it from elsewhere, all you are losing is a placeholder for your state at the time of corruption). But coupled with (2), if we drop the ref and then go on to prune, we may lose unrecoverable objects. Note that we use test_might_fail for some of the operations. In some cases, it would be appropriate to abort the operation, and in others, it might be acceptable to continue but taking the information into account. The tests don't care either way, and check only for data loss. Signed-off-by: Jeff King p...@peff.net --- t/t5312-prune-corruption.sh | 114 1 file changed, 114 insertions(+) create mode 100755 t/t5312-prune-corruption.sh diff --git a/t/t5312-prune-corruption.sh b/t/t5312-prune-corruption.sh new file mode 100755 index 000..496a9f5 --- /dev/null +++ b/t/t5312-prune-corruption.sh @@ -0,0 +1,114 @@ +#!/bin/sh + +test_description=' +Test pruning of repositories with minor corruptions. The goal +here is that we should always be erring on the side of safety. So +if we see, for example, a ref with a bogus name, it is OK either to +bail out or to proceed using it as a reachable tip, but it is _not_ +OK to proceed as if it did not exist. Otherwise we might silently +delete objects that cannot be recovered. +' +. ./test-lib.sh + +test_expect_success 'disable reflogs' ' + git config core.logallrefupdates false + rm -rf .git/logs +' + +test_expect_success 'create history reachable only from a bogus-named ref' ' + test_tick git commit --allow-empty -m master + base=$(git rev-parse HEAD) + test_tick git commit --allow-empty -m bogus + bogus=$(git rev-parse HEAD) + git cat-file commit $bogus saved + echo $bogus .git/refs/heads/bogus..name + git reset --hard HEAD^ +' + +test_expect_failure 'pruning does not drop bogus object' ' + test_when_finished git hash-object -w -t commit saved + test_might_fail git prune --expire=now + verbose git cat-file -e $bogus +' + +test_expect_success 'put bogus object into pack' ' + git tag reachable $bogus + git repack -ad + git tag -d reachable + verbose git cat-file -e $bogus +' + +test_expect_failure 'destructive repack keeps packed object' ' + test_might_fail git repack -Ad --unpack-unreachable=now + verbose git cat-file -e $bogus + test_might_fail git repack -ad + verbose git cat-file -e $bogus +' + +# subsequent tests will have different corruptions +test_expect_success 'clean up bogus ref' ' + rm .git/refs/heads/bogus..name +' + +# We create two new objects here, one and two. Our +# master branch points to two, which is deleted, +# corrupting the repository. But we'd like to make sure +# that the otherwise unreachable one is not pruned +# (since it is the user's best bet for recovering +# from the corruption). +# +# Note that we also point HEAD somewhere besides two, +# as we want to make sure we test the case where we +# pick up the reference to two by iterating the refs, +# not by resolving HEAD. +test_expect_success 'create history with missing tip commit' ' + test_tick git commit --allow-empty -m one + recoverable=$(git rev-parse HEAD) + git cat-file commit $recoverable saved + test_tick git commit --allow-empty -m two + missing=$(git rev-parse HEAD) + git checkout --detach $base + rm .git/objects/$(echo $missing | sed s,..,/,) + test_must_fail git cat-file -e $missing +' + +test_expect_failure 'pruning with a corrupted tip does not drop history' ' + test_when_finished git hash-object -w -t commit saved + test_might_fail
[PATCH v2 2/5] refs: introduce a ref paranoia flag
Most operations that iterate over refs are happy to ignore broken cruft. However, some operations should be performed with knowledge of these broken refs, because it is better for the operation to choke on a missing object than it is to silently pretend that the ref did not exist (e.g., if we are computing the set of reachable tips in order to prune objects). These processes could just call for_each_rawref, except that ref iteration is often hidden behind other interfaces. For instance, for a destructive repack -ad, we would have to inform pack-objects that we are destructive, and then it would in turn have to tell the revision code that our --all should include broken refs. It's much simpler to just set a global for dangerous operations that includes broken refs in all iterations. Signed-off-by: Jeff King p...@peff.net --- Documentation/git.txt | 11 +++ cache.h | 8 environment.c | 1 + refs.c| 5 + 4 files changed, 25 insertions(+) diff --git a/Documentation/git.txt b/Documentation/git.txt index af30620..8da85a6 100644 --- a/Documentation/git.txt +++ b/Documentation/git.txt @@ -1026,6 +1026,17 @@ GIT_ICASE_PATHSPECS:: variable when it is invoked as the top level command by the end user, to be recorded in the body of the reflog. +`GIT_REF_PARANOIA`:: + If set to `1`, include broken or badly named refs when iterating + over lists of refs. In a normal, non-corrupted repository, this + does nothing. However, enabling it may help git to detect and + abort some operations in the presence of broken refs. Git sets + this variable automatically when performing destructive + operations like linkgit:git-prune[1]. You should not need to set + it yourself unless you want to be paranoid about making sure + an operation has touched every ref (e.g., because you are + cloning a repository to make a backup). + Discussion[[Discussion]] diff --git a/cache.h b/cache.h index 761c570..162ea6f 100644 --- a/cache.h +++ b/cache.h @@ -614,6 +614,14 @@ extern int protect_hfs; extern int protect_ntfs; /* + * Include broken refs in all ref iterations, which will + * generally choke dangerous operations rather than letting + * them silently proceed without taking the broken ref into + * account. + */ +extern int ref_paranoia; + +/* * The character that begins a commented line in user-editable file * that is subject to stripspace. */ diff --git a/environment.c b/environment.c index 1ade5c9..a40044c 100644 --- a/environment.c +++ b/environment.c @@ -24,6 +24,7 @@ int is_bare_repository_cfg = -1; /* unspecified */ int log_all_ref_updates = -1; /* unspecified */ int warn_ambiguous_refs = 1; int warn_on_object_refname_ambiguity = 1; +int ref_paranoia = -1; int repository_format_version; const char *git_commit_encoding; const char *git_log_output_encoding; diff --git a/refs.c b/refs.c index e23542b..7f0e7be 100644 --- a/refs.c +++ b/refs.c @@ -1934,6 +1934,11 @@ static int do_for_each_ref(struct ref_cache *refs, const char *base, data.fn = fn; data.cb_data = cb_data; + if (ref_paranoia 0) + ref_paranoia = git_env_bool(GIT_REF_PARANOIA, 0); + if (ref_paranoia) + data.flags |= DO_FOR_EACH_INCLUDE_BROKEN; + return do_for_each_entry(refs, base, do_one_ref, data); } -- 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
Re: [PATCH] Documentation: Add target to build PDF manpages
On Fri, Mar 20, 2015 at 4:23 AM, Thomas Schneider thosc...@gmail.com wrote: Signed-off-by: Thomas Schneider thosc...@gmail.com --- 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 :) Welcome to the git community! The patch itself applies cleanly and the generated pdfs look really great, though how would I read man pages in pdf format? I tried searching the web and all I can find is how to convert the a man page to pdf. So is there a conveniant way to tell `man` to prefer opening pdfs when available? 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 -- 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] t1700: make test pass with index-v4
Thomas Gummerer t.gumme...@gmail.com writes: 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. I am more interested to see how you managed to use index v4 in the tests be described next to when run with index v4. I thought we were controling these things fairly tightly (e.g. we disable hooks, move $HOME to avoid getting affected by your personal settings, etc.). Thanks. Signed-off-by: Thomas Gummerer t.gumme...@gmail.com --- 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 expect EOF -own 8299b0bcd1ac364e5f1d7768efb62fa2da79a339 -base 39d890139ee5356c7ef572216cebcd27aa41f9df +own $own +base $base replacements: deletions: EOF @@ -30,7 +39,7 @@ EOF test-dump-split-index .git/index | sed /^own/d actual cat expect EOF -base 39d890139ee5356c7ef572216cebcd27aa41f9df +base $base 100644 e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 0one replacements: deletions: -- 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
On Fri, Mar 20, 2015 at 10:04:43AM -0700, Junio C Hamano wrote:
One case where this might misdetect a good test would be this one:
test_expect_success 'either succeed or fail with status 1' '
git subcmd || case $? in 1) : happy ;; *) false failure ;; esac
'
Yes. Any use of || is going to cause problems. See the conversions in
the series I posted a few hours ago. The solution is to adapt the test
style to encase any || inside a block. I don't like having to modify
the tests to match our lint check, but it is a fairly rare case, and the
modification is not complex. And in fact I think it often highlights to
the reader that something funny is going on; a snippet like the one
above is OK to run at the start of a test, but:
foo
bar ||
case $?
...
is potentially dangerous. You do not know if you are checking the $? of
foo or bar in that case statement.
Your case above is actually better spelled as test_expect_code, but
there are more complex one-off cases that I solved using a {} block.
I wonder if another valid way to make it harder for us to commit
broken chain errors in our test may be to make it not an error
in the first place. Do we know how buggy various implementations of
shells are with respect to their handling of set -e?
Certainly that is a thought that occurred to me while writing the
series. At GitHub, we have a shell-based test harness for internal
projects that is somewhat like Git's t/test-lib.sh, but uses -e to
catch errors.
It's mostly OK, but there are some surprising bits. For instance, try
this sequence of snippets:
set -e
false
echo should not reach
That should print nothing. So far so good. How about in a subshell:
set -e
(
false
echo 1
)
echo 2
That also prints nothing. Good. But set -e is suppressed in a
conditional or in the presence of logic operators like ||. So you'd
expect this:
set -e
(
false
echo 1
) || {
echo outcome=$?
false
}
echo 2
to break out of the subshell, print the failed outcome, and then exit.
But it doesn't. It prints 1 and 2. The suppression of set -e
continues inside the subshell, even though the inner commands are not
part of their own conditionals.
OK, so when we open a subshell we have to re-iterate our desire for set
-e:
set -e
(
set -e
false
echo 1
) || {
echo outcome=$?
false
}
echo 2
Nope, does nothing, at least in bash and dash. The ||-operator
suppression is clearly not turn off set -e, but globally make -e
useless inside here.
So you end up having to use manual exits to jump out of the subshell,
like:
set -e
(
false || exit 1
echo 1
) || {
echo outcome=$?
false
}
echo 2
That snippet produces outcome=1, which is what I expect.
Of course most tests are not so complicated to have subshells with
conditional operators on the side. But I did not just type out that
example right now from scratch. I cut-and-pasted it from a real-world
commit message.
So I dunno. I think set -e is kind of a dangerous lure. It works so
well _most_ of the time that you start to rely on it, but it really does
have some funny corner cases (even on modern shells, and for all I know,
the behavior above is mandated by POSIX).
-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: [PATCH] t1700: make test pass with index-v4
On 03/20, Junio C Hamano wrote: Thomas Gummerer t.gumme...@gmail.com writes: 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. I am more interested to see how you managed to use index v4 in the tests be described next to when run with index v4. I thought we were controling these things fairly tightly (e.g. we disable hooks, move $HOME to avoid getting affected by your personal settings, etc.). The tests can be run with index-v4 by setting TEST_GIT_INDEX_VERSION in config.mak. This configuration was introduced in 5d9fc88 test-lib: allow setting the index format version. Thanks. Signed-off-by: Thomas Gummerer t.gumme...@gmail.com --- 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 expect EOF -own 8299b0bcd1ac364e5f1d7768efb62fa2da79a339 -base 39d890139ee5356c7ef572216cebcd27aa41f9df +own $own +base $base replacements: deletions: EOF @@ -30,7 +39,7 @@ EOF test-dump-split-index .git/index | sed /^own/d actual cat expect EOF -base 39d890139ee5356c7ef572216cebcd27aa41f9df +base $base 100644 e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 0 one replacements: deletions: -- Thomas Gummerer -- 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 15/25] t9502: fix -chain breakage
Am 20.03.2015 um 11:13 schrieb Jeff King:
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 p...@peff.net
---
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
+'
This loses the single-quotes in the generated perl script, doesn't it?
Most likely, it does not matter.
-- Hannes
--
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
Michael Haggerty mhag...@alum.mit.edu writes:
+static int parse_precheck(const char *s, unsigned int *flags)
+{
+ const char *number;
+
+ if (isspace(*s)) {
+ if (!(*flags NUM_LEADING_WHITESPACE))
+ return -NUM_LEADING_WHITESPACE;
+ do {
+ s++;
+ } while (isspace(*s));
+ }
+
+ if (*s == '+') {
+ if (!(*flags NUM_PLUS))
+ return -NUM_PLUS;
+ number = s + 1;
+ *flags = ~NUM_NEGATIVE;
+ } else if (*s == '-') {
+ if (!(*flags NUM_MINUS))
+ return -NUM_MINUS;
+ number = s + 1;
+ *flags |= NUM_NEGATIVE;
+ } else {
+ number = s;
+ *flags = ~NUM_NEGATIVE;
+ }
+
+ if (!(*flags NUM_BASE_SPECIFIER)) {
+ int base = *flags NUM_BASE_MASK;
+ if (base == 0) {
+ /* This is a pointless combination of options. */
+ die(BUG: base=0 specified without NUM_BASE_SPECIFIER);
+ } else if (base == 16 starts_with(number, 0x)) {
+ /*
+ * We want to treat this as zero terminated by
+ * an 'x', whereas strtol()/strtoul() would
+ * silently eat the 0x. We accomplish this
+ * by treating it as a base 10 number:
+ */
+ *flags = (*flags ~NUM_BASE_MASK) | 10;
+ }
+ }
+ return 0;
+}
I somehow feel that a pre-processing that only _inspects_ part of
the string, without munging that string (e.g. notice '-' but feed
that to underlying strtol(3)) somewhat a brittle approach. When I
read the above for the first time, I somehow expected that the code
would notice leading '-', strip that leading '-' and remember the
fact that it did so in the *flags, let the strtol(3) to parse the
remainder _and_ always make sure the returned result is not negative
(because that would imply that the original input had two leading
minuses and digits), and give the sign based on what this preprocess
found out in *flags, and then seeing that there is no sign of such
processing in the caller I scratched my head.
I still have not convinced myself that what I am seeing in the
base==16 part in the above is correct.
--
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
On Fri, Mar 20, 2015 at 10:34:51AM -0700, Junio C Hamano wrote:
Jeff King p...@peff.net writes:
Your case above is actually better spelled as test_expect_code, but
there are more complex one-off cases that I solved using a {} block.
Just for the record, test_expect_code expects only one possible good
exit status and it does not allow us to say 0 is OK and 1 is also
OK, everything else is bad, so it is not quite appropriate there.
Oh, sorry, I misread your test as expecting 1, but it is actually
expecting 0 or 1. Yeah, I agree this is the exact sort of case covered
in my patch 11/25 (and there are only 4 instances in the whole test
suite).
-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: [PATCH 0/25] detecting -chain breakage
Junio C Hamano gits...@pobox.com writes: I found 2026 and 5312 to be broken (there may be others that are excluded in my usual test set) in 'pu'. As to these topics in git log --first-parent master..pu, my preference is to queue fixups on the broken-out topics (available at http://github.com/gitster/git) independently. For example, this will go on top of nd/multiple-work-trees. ... and this goes on top of jk/prune-with-corrupt-refs until it is rerolled. -- 8 -- Subject: [PATCH] SQUASH??? t5312: fix broken -chain Signed-off-by: Junio C Hamano gits...@pobox.com --- t/t5312-prune-corruption.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/t/t5312-prune-corruption.sh b/t/t5312-prune-corruption.sh index 8b54d16..9633d97 100755 --- a/t/t5312-prune-corruption.sh +++ b/t/t5312-prune-corruption.sh @@ -80,7 +80,7 @@ test_expect_success 'pack-refs does not silently delete broken loose ref' ' # skip processing a broken ref test_expect_success 'create packed-refs file with broken ref' ' rm -f .git/refs/heads/master - cat .git/packed-refs -EOF + cat .git/packed-refs -EOF $missing refs/heads/master $recoverable refs/heads/other EOF -- 2.3.3-401-g402122f -- 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 15/25] t9502: fix -chain breakage
On Fri, Mar 20, 2015 at 06:48:35PM +0100, Johannes Sixt wrote:
-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
+'
This loses the single-quotes in the generated perl script, doesn't it? Most
likely, it does not matter.
Eek, you're right. Thanks for noticing.
I agree it should not matter (it is OK in perl to use barewords as hash
keys, but it would probably be more obvious to omit them entirely, or to
just use double-quotes. I'll squash this in:
diff --git a/t/t9502-gitweb-standalone-parse-output.sh
b/t/t9502-gitweb-standalone-parse-output.sh
index a93e159..0796a43 100755
--- a/t/t9502-gitweb-standalone-parse-output.sh
+++ b/t/t9502-gitweb-standalone-parse-output.sh
@@ -147,7 +147,7 @@ test_expect_success 'forks: not skipped unless forks
feature enabled' '
test_expect_success 'enable forks feature' '
cat gitweb_config.perl -\EOF
- $feature{'forks'}{'default'} = [1];
+ $feature{forks}{default} = [1];
EOF
'
-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: cover letter and cc list
Olaf Hering o...@aepfle.de writes: What does it take to send the cover letter to all people which are listed in the Cc: list of the following patches? Each patch has a different Cc: list. The git send-email --help command suggests that this cmdline should do it. But the cover letter goes just to the address listed in --to=: env TMPDIR=/dev/shm LC_ALL=C git send-email -M --stat --annotate \ --cover-letter --cc-cover --to=$address \ $base..$head First step is not to drive format-patch from within send-email I would think. Instead prepare them in files in a directory (with format-patch -o $dir). You can edit Cc: header in -*.patch message while you proof-read what you are going to send out. -- 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 03:28:11PM +0100, Michael J Gruber wrote: 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. Thanks for checking. I had assumed there would be some weirdness, but I didn't actually try a lint-only mode. I only ran the lint against master earlier. There are two trivial broken chains in pu. Patch is below (against nd/multiple-work-trees). Looks like that is in 'next', so we can't just squash it in. -- 8 -- Subject: t2026: fix broken -chains These are totally trivial (test_when_finished should never fail), but being complete with our -chaining makes the new --chain-lint feature more useful. Signed-off-by: Jeff King p...@peff.net --- t/t2026-prune-linked-checkouts.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 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 -- 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
cover letter and cc list
What does it take to send the cover letter to all people which are listed in the Cc: list of the following patches? Each patch has a different Cc: list. The git send-email --help command suggests that this cmdline should do it. But the cover letter goes just to the address listed in --to=: env TMPDIR=/dev/shm LC_ALL=C git send-email -M --stat --annotate \ --cover-letter --cc-cover --to=$address \ $base..$head Thanks, Olaf -- 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
Jeff King p...@peff.net writes:
Your case above is actually better spelled as test_expect_code, but
there are more complex one-off cases that I solved using a {} block.
Just for the record, test_expect_code expects only one possible good
exit status and it does not allow us to say 0 is OK and 1 is also
OK, everything else is bad, so it is not quite appropriate there.
...
So I dunno. I think set -e is kind of a dangerous lure.
Yes, I think we should stay away from it. -chaining is simpler to
see what is going on, even though it is a bit more to type.
Thanks.
--
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
Thanks. They applied cleanly on 'master' and all looked sensible. I found 2026 and 5312 to be broken (there may be others that are excluded in my usual test set) in 'pu'. As to these topics in git log --first-parent master..pu, my preference is to queue fixups on the broken-out topics (available at http://github.com/gitster/git) independently. For example, this will go on top of nd/multiple-work-trees. 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 -- 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 11:00:05AM -0700, Junio C Hamano wrote: Junio C Hamano gits...@pobox.com writes: I found 2026 and 5312 to be broken (there may be others that are excluded in my usual test set) in 'pu'. As to these topics in git log --first-parent master..pu, my preference is to queue fixups on the broken-out topics (available at http://github.com/gitster/git) independently. For example, this will go on top of nd/multiple-work-trees. Heh, I just crossed emails with you over the same patch. ... and this goes on top of jk/prune-with-corrupt-refs until it is rerolled. I'm actually about to send out a re-roll of that, as I think all of the review comments have been addressed. -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: [PATCH 26/27] t/*svn*: fix moderate -chain breakage
Michael J Gruber g...@drmicha.warpmail.net writes: 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 g...@drmicha.warpmail.net --- 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(-) Ahh, it seems that I should have read everything in my inbox before starting my day X-. I already queued an identical patch for 2026 on nd/multiple-work-trees, and its new tip is in 'next' now. Which branches are the git-svn ones meant to apply? Are they meant to fix an existing bug already in master, or are they new ones added by still-in-flight topics? Can you split if necessary and mark them for individual topios in flight if that is the case, so that we can apply them independently from GIT_TEST_CHAIN_LINT series? Thanks. -- 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
