Re: [PATCH] xdiff: reduce indent heuristic overhead

2018-07-01 Thread Michael Haggerty
On 06/29/2018 10:28 PM, Stefan Beller wrote:
> [...]
> Adds some threshold to avoid expensive cases, like:
> 
> ```
> #!python
> open('a', 'w').write(" \n" * 100)
> open('b', 'w').write(" \n" * 101)
> ```
> 
> The indent heuristic is O(N * 20) (N = 100) for the above case, and
> makes diff much slower.
> [...]
> +/*
> + * For indentation heuristic, skip searching for better slide position after
> + * checking MAX_BORING lines without finding an improvement. This defends the
> + * indentation heuristic logic against pathological cases. The value is not
> + * picked scientifically but should be good enough.
> + */
> +#define MAX_BORING 100

This is an interesting case, and a speed difference of almost a factor
of five seems impressive. But this is a pretty pathological case, isn't
it? And I'm pretty sure that the algorithm is `O(N)` both before and
after this change. Remember that to find `earliest_end` and `g.end`,
there has already been a scan through all 100 lines. In other words,
you're not improving how the overall algorithm scales with `N`; you're
only changing the constant factor in front. So it's a little bit
questionable whether it is worth complicating the code for this unusual
case.

But *if* we want to improve this case, I think that we could be smarter
about it.

By the time we get to this point in the code, we already know that there
is a "slider" hunk of length `M` (`groupsize`) that can be slid up or
down within a range of `N` (`g.end - earliest_end + 1`) possible
positions. The interesting case here is `N ≫ M`, because then naively
the number of positions to try out is a lot bigger than the size of the
hunk itself. (In the case described above, `N` is 100 and `M` is 1.)

But how can that situation even arise? Remember, a hunk can only be slid
down by a line if the first line *after* the hunk is identical to the
first line *of* the hunk. It follows that if you shift a hunk down `M`
lines, then it has the same contents as when you started—you've just
rotated all of the hunk lines around once.

So if `N ≫ M`, there is necessarily a lot of repetition among the `N +
M` lines that the hunk could possibly overlay. Specifically, it must
consist of `floor((N + M)/M)` identical copies of the hunk, plus
possibly a few leftover lines constituting the start of another repetition.

Given this large amount of repetition, it seems to me that there is
never a need to scan more than the bottom `M + 1` possible positions [1]
plus the highest possible position [2] to be sure of finding the very
best one. In the pathological case that you described above, where `M`
is 1, only three positions have to be evaluated, not 100.

In fact, it *could* be that there is even more repetition, namely if the
hunk itself contains multiple copies of an even shorter block of `K`
lines. In that case, you would only have to scan `K + 1` positions at
the bottom plus one at the top to be sure to find the best hunk
position. This would be an interesting optimization for a case like

> open('a', 'w').write(" \n" * 100)
> open('b', 'w').write(" \n" * 110)

(`N = 100`, `M = 10`, `K = 1`) or

> open('a', 'w').write("\nMISSING\n\n" * 100)
> open('b', 'w').write("\nMISSING\n\n" * 110)

(`N = 300`, `M = 30`, `K = 3`). On the other hand, it's not
entirely trivial to find periodicity in a group of lines (i.e., to find
`K`), and I don't know offhand how that task scales with `M`.

Michael

[1] Actually, to be rigorously correct it might be necessary to check
even a bit more than `M + 1` positions at the bottom because the
heuristic looks a bit beyond the lines of the hunk.

[2] The position at the top has different predecessor lines than the
other positions, so it could have a lower score than all of the others.
It's worth checking it. Here too, to be rigorously correct it might be
necessary to check more than one position at the top because the
heuristic looks a bit beyond the lines of the hunk.


Feature request : "git fsck" should show the percentage of completeness in step "Checking connectivity:"

2018-07-01 Thread Toralf Förster
as "git fsck" does it already for "Checking objects:"

Is this a valid feature request?

-- 
Toralf
PGP C4EACDDE 0076E94E


Re: Use of new .gitattributes working-tree-encoding attribute across different platform types

2018-07-01 Thread brian m. carlson
On Thu, Jun 28, 2018 at 01:27:07PM -0400, Jeff King wrote:
> Yeah, that was along the lines that I was thinking. I wonder if anybody
> would ever need two such auto-encodings, though. Probably not. But
> another way to think about it would be to allow something like:
> 
>   working-tree-encoding=foo
> 
> and then in your config "foo" to map to some encoding.
> 
> But that may be over-engineering, I dunno. utf8 has always been enough
> for me. :)

I had a thought the other day about why this solution might be valuable.
Different platforms encode different values for iconv character sets.
So, for example, one may have platforms supporting some disjoint sets of
the following:

* LATIN-1
* LATIN1
* ISO8859-1
* ISO-8859-1
* ISO_8859-1
* ISO_8859-1:1987
* some lowercase variants of these

Therefore, specifying a working-tree-encoding value that works across a
wide variety of system may be non-trivial.  This is less of a problem
with UTF-8, but having the ability to pick an encoding and remap it to a
supported value may be useful nevertheless.
-- 
brian m. carlson: Houston, Texas, US
OpenPGP: https://keybase.io/bk2204


signature.asc
Description: PGP signature


Re: send-email: change the default value of sendmail.validate

2018-07-01 Thread brian m. carlson
On Fri, Jun 29, 2018 at 03:07:51PM -0400, Drew DeVault wrote:
> The purpose of this configuration option is to prevent your emails from
> blowing up on SMTP servers (rather than Extended SMTP servers). However,
> I find it often confuses people whose patches are otherwise correct, and
> they don't know how to solve the issue.
> 
> I haven't seen an SMTP server in a very long time which doesn't support
> extended SMTP. The default behavior should probably change. If not, the
> error message should be more clear about action items to address the
> issue.
> 
> I'll send a patch around to change this shortly.

Can you say a bit more about the exact error message you're seeing?

Are you suggesting that we not limit lines to 998 octets?  I've seen
lots of mail servers that do reject mail over 998 octets.  I've
configured Postfix to do so because being strict on mail standards is a
great way to stop spam.

If that's the issue you're seeing, it might be better to either
automatically encode those patches as binary patches or teach git
send-email and git am how to automatically handle quoted-printable.
-- 
brian m. carlson: Houston, Texas, US
OpenPGP: https://keybase.io/bk2204


signature.asc
Description: PGP signature


Re: send-email: change the default value of sendmail.validate

2018-07-01 Thread Drew DeVault
On 2018-07-01  6:15 PM, brian m. carlson wrote:
> Can you say a bit more about the exact error message you're seeing?

"patch contains a line longer than 998 characters"

A recent occasion when this came up was when someone attempted to send
me a patch which included a base64-encoded data URI, which cannot be
wrapped.

> Are you suggesting that we not limit lines to 998 octets?  I've seen
> lots of mail servers that do reject mail over 998 octets.  I've
> configured Postfix to do so because being strict on mail standards is a
> great way to stop spam.

Yes, that's what I'm suggesting. We should error out later when the SMTP
server rejects the mail.

Also, Extended SMTP is a standard. RFC 1869.

> If that's the issue you're seeing, it might be better to either
> automatically encode those patches as binary patches or teach git
> send-email and git am how to automatically handle quoted-printable.

I'm really not fond of quoted-printable. Extended SMTP has been
standardized for over 20 years. Assuming people don't blithly disable it
on their servers, we can expect it to be present on almost all mail
servers.

I don't think I've ever seen a spam email that would have been stopped
because it contained 998 lines. Approaches like SpamAssassin,
greylisting, DNSBLs, SPF/DKIM, these are much more effective.


git@vger.kernel.org

2018-07-01 Thread Eric Sunshine
Signed-off-by: Eric Sunshine 
---
 t/t-basic.sh  |  2 +-
 t/t0003-attributes.sh | 24 
 t/t0021-conversion.sh |  4 ++--
 3 files changed, 15 insertions(+), 15 deletions(-)

diff --git a/t/t-basic.sh b/t/t-basic.sh
index af61d083b4..34859fe4a5 100755
--- a/t/t-basic.sh
+++ b/t/t-basic.sh
@@ -1081,7 +1081,7 @@ test_expect_success 'very long name in the index handled 
sanely' '
(
git ls-files -s path4 |
sed -e "s/  .*/ /" |
-   tr -d "\012"
+   tr -d "\012" &&
echo "$a"
) | git update-index --index-info &&
len=$(git ls-files "a*" | wc -c) &&
diff --git a/t/t0003-attributes.sh b/t/t0003-attributes.sh
index f19ae4f8cc..5c37c2e1f8 100755
--- a/t/t0003-attributes.sh
+++ b/t/t0003-attributes.sh
@@ -34,15 +34,15 @@ test_expect_success 'open-quoted pathname' '
 test_expect_success 'setup' '
mkdir -p a/b/d a/c b &&
(
-   echo "[attr]notest !test"
-   echo "\" d \"   test=d"
-   echo " etest=e"
-   echo " e\"  test=e"
-   echo "f test=f"
-   echo "a/i test=a/i"
-   echo "onoff test -test"
-   echo "offon -test test"
-   echo "no notest"
+   echo "[attr]notest !test" &&
+   echo "\" d \"   test=d" &&
+   echo " etest=e" &&
+   echo " e\"  test=e" &&
+   echo "f test=f" &&
+   echo "a/i test=a/i" &&
+   echo "onoff test -test" &&
+   echo "offon -test test" &&
+   echo "no notest" &&
echo "A/e/F test=A/e/F"
) >.gitattributes &&
(
@@ -51,7 +51,7 @@ test_expect_success 'setup' '
) >a/.gitattributes &&
(
echo "h test=a/b/h" &&
-   echo "d/* test=a/b/d/*"
+   echo "d/* test=a/b/d/*" &&
echo "d/yes notest"
) >a/b/.gitattributes &&
(
@@ -287,7 +287,7 @@ test_expect_success 'bare repository: check that 
.gitattribute is ignored' '
(
cd bare.git &&
(
-   echo "f test=f"
+   echo "f test=f" &&
echo "a/i test=a/i"
) >.gitattributes &&
attr_check f unspecified &&
@@ -312,7 +312,7 @@ test_expect_success 'bare repository: test info/attributes' 
'
(
cd bare.git &&
(
-   echo "f test=f"
+   echo "f test=f" &&
echo "a/i test=a/i"
) >info/attributes &&
attr_check f f &&
diff --git a/t/t0021-conversion.sh b/t/t0021-conversion.sh
index 9479a4aaab..6a213608cc 100755
--- a/t/t0021-conversion.sh
+++ b/t/t0021-conversion.sh
@@ -785,7 +785,7 @@ test_expect_success PERL 'missing file in delayed checkout' 
'
cd repo &&
git init &&
echo "*.a filter=bug" >.gitattributes &&
-   cp "$TEST_ROOT/test.o" missing-delay.a
+   cp "$TEST_ROOT/test.o" missing-delay.a &&
git add . &&
git commit -m "test commit"
) &&
@@ -807,7 +807,7 @@ test_expect_success PERL 'invalid file in delayed checkout' 
'
git init &&
echo "*.a filter=bug" >.gitattributes &&
cp "$TEST_ROOT/test.o" invalid-delay.a &&
-   cp "$TEST_ROOT/test.o" unfiltered
+   cp "$TEST_ROOT/test.o" unfiltered &&
git add . &&
git commit -m "test commit"
) &&
-- 
2.18.0.203.gfac676dfb9



[PATCH 07/25] t5406: use write_script() instead of birthing shell script manually

2018-07-01 Thread Eric Sunshine
Take advantage of write_script() to abstract-away details of shell
script creation, thus allowing the reader to focus on script content.
Readability benefits, particularly in this case, since the script body
was buried in a noisy one-liner subshell responsible for emitting
boilerplate and body.

Signed-off-by: Eric Sunshine 
---
 t/t5406-remote-rejects.sh | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/t/t5406-remote-rejects.sh b/t/t5406-remote-rejects.sh
index 59e80a5ea2..ff06f99649 100755
--- a/t/t5406-remote-rejects.sh
+++ b/t/t5406-remote-rejects.sh
@@ -6,8 +6,9 @@ test_description='remote push rejects are reported by client'
 
 test_expect_success 'setup' '
mkdir .git/hooks &&
-   (echo "#!/bin/sh" ; echo "exit 1") >.git/hooks/update &&
-   chmod +x .git/hooks/update &&
+   write_script .git/hooks/update <<-\EOF &&
+   exit 1
+   EOF
echo 1 >file &&
git add file &&
git commit -m 1 &&
-- 
2.18.0.203.gfac676dfb9



[PATCH 10/25] t7201: drop pointless "exit 0" at end of subshell

2018-07-01 Thread Eric Sunshine
This test employs a for-loop inside a subshell and correctly aborts the
loop and fails the test overall (via "exit 1") if any iteration of the
for-loop fails. Otherwise, it exits the subshell with an explicit but
entirely unnecessary "exit 0", presumably to indicate that all
iterations of the loop succeeded. The &&-chain is broken between the
for-loop and the "exit 0". Rather than fixing the &&-chain, just drop
the pointless "exit 0".

Signed-off-by: Eric Sunshine 
---
 t/t7201-co.sh | 1 -
 1 file changed, 1 deletion(-)

diff --git a/t/t7201-co.sh b/t/t7201-co.sh
index ab9da61da3..8d8a63a24b 100755
--- a/t/t7201-co.sh
+++ b/t/t7201-co.sh
@@ -673,7 +673,6 @@ test_expect_success 'custom merge driver with checkout -m' '
do
grep $t arm || exit 1
done
-   exit 0
) &&
 
mv arm expect &&
-- 
2.18.0.203.gfac676dfb9



git@vger.kernel.org

2018-07-01 Thread Eric Sunshine
Signed-off-by: Eric Sunshine 
---
 t/t1004-read-tree-m-u-wf.sh |  8 
 t/t1005-read-tree-reset.sh  | 10 +-
 t/t1020-subdirectory.sh |  2 +-
 t/t1050-large.sh|  6 +++---
 t/t1411-reflog-show.sh  |  6 +++---
 t/t1512-rev-parse-disambiguation.sh |  6 +++---
 6 files changed, 19 insertions(+), 19 deletions(-)

diff --git a/t/t1004-read-tree-m-u-wf.sh b/t/t1004-read-tree-m-u-wf.sh
index 826cd32e23..c13578a635 100755
--- a/t/t1004-read-tree-m-u-wf.sh
+++ b/t/t1004-read-tree-m-u-wf.sh
@@ -210,10 +210,10 @@ test_expect_success 'D/F' '
read_tree_u_must_succeed -m -u branch-point side-b side-a &&
git ls-files -u >actual &&
(
-   a=$(git rev-parse branch-point:subdir/file2)
-   b=$(git rev-parse side-a:subdir/file2/another)
-   echo "100644 $a 1   subdir/file2"
-   echo "100644 $a 2   subdir/file2"
+   a=$(git rev-parse branch-point:subdir/file2) &&
+   b=$(git rev-parse side-a:subdir/file2/another) &&
+   echo "100644 $a 1   subdir/file2" &&
+   echo "100644 $a 2   subdir/file2" &&
echo "100644 $b 3   subdir/file2/another"
) >expect &&
test_cmp expect actual
diff --git a/t/t1005-read-tree-reset.sh b/t/t1005-read-tree-reset.sh
index 074568500a..83b09e1310 100755
--- a/t/t1005-read-tree-reset.sh
+++ b/t/t1005-read-tree-reset.sh
@@ -33,7 +33,7 @@ test_expect_success 'reset should remove remnants from a 
failed merge' '
git ls-files -s >expect &&
sha1=$(git rev-parse :new) &&
(
-   echo "100644 $sha1 1old"
+   echo "100644 $sha1 1old" &&
echo "100644 $sha1 3old"
) | git update-index --index-info &&
>old &&
@@ -48,7 +48,7 @@ test_expect_success 'two-way reset should remove remnants 
too' '
git ls-files -s >expect &&
sha1=$(git rev-parse :new) &&
(
-   echo "100644 $sha1 1old"
+   echo "100644 $sha1 1old" &&
echo "100644 $sha1 3old"
) | git update-index --index-info &&
>old &&
@@ -63,7 +63,7 @@ test_expect_success 'Porcelain reset should remove remnants 
too' '
git ls-files -s >expect &&
sha1=$(git rev-parse :new) &&
(
-   echo "100644 $sha1 1old"
+   echo "100644 $sha1 1old" &&
echo "100644 $sha1 3old"
) | git update-index --index-info &&
>old &&
@@ -78,7 +78,7 @@ test_expect_success 'Porcelain checkout -f should remove 
remnants too' '
git ls-files -s >expect &&
sha1=$(git rev-parse :new) &&
(
-   echo "100644 $sha1 1old"
+   echo "100644 $sha1 1old" &&
echo "100644 $sha1 3old"
) | git update-index --index-info &&
>old &&
@@ -93,7 +93,7 @@ test_expect_success 'Porcelain checkout -f HEAD should remove 
remnants too' '
git ls-files -s >expect &&
sha1=$(git rev-parse :new) &&
(
-   echo "100644 $sha1 1old"
+   echo "100644 $sha1 1old" &&
echo "100644 $sha1 3old"
) | git update-index --index-info &&
>old &&
diff --git a/t/t1020-subdirectory.sh b/t/t1020-subdirectory.sh
index df3183ea1a..c2df75e495 100755
--- a/t/t1020-subdirectory.sh
+++ b/t/t1020-subdirectory.sh
@@ -148,7 +148,7 @@ test_expect_success 'GIT_PREFIX for built-ins' '
(
cd dir &&
echo "change" >two &&
-   GIT_EXTERNAL_DIFF=./diff git diff >../actual
+   GIT_EXTERNAL_DIFF=./diff git diff >../actual &&
git checkout -- two
) &&
test_cmp expect actual
diff --git a/t/t1050-large.sh b/t/t1050-large.sh
index f9eb143f43..1a9b21b293 100755
--- a/t/t1050-large.sh
+++ b/t/t1050-large.sh
@@ -108,7 +108,7 @@ test_expect_success 'packsize limit' '
test-tool genrandom "c" $(( 128 * 1024 )) >mid3 &&
git add mid1 mid2 mid3 &&
 
-   count=0
+   count=0 &&
for pi in .git/objects/pack/pack-*.idx
do
test -f "$pi" && count=$(( $count + 1 ))
@@ -116,8 +116,8 @@ test_expect_success 'packsize limit' '
test $count = 2 &&
 
(
-   git hash-object --stdin expect &&
diff --git a/t/t1411-reflog-show.sh b/t/t1411-reflog-show.sh
index 596907758d..4d62ceef9c 100755
--- a/t/t1411-reflog-show.sh
+++ b/t/t1411-reflog-show.sh
@@ -159,9 +159,9 @@ test_expect_success 'git log -g -p shows diffs vs. parents' 
'
git log -1 -p HEAD^ >log.one &&
git log -1 -p HEAD >log.two &&
(
-   cat log.one; echo
-   cat log.two; echo
-   cat log.one; echo
+   cat log.one && echo &&
+   c

[PATCH 08/25] t5505: modernize and simplify hard-to-digest test

2018-07-01 Thread Eric Sunshine
This test uses a subshell within a subshell but is formatted in such a
way as to suggests that the inner subshell is a sibling rather than a
child, which makes it difficult to digest the test's structure and
intent.

Worse, the inner subshell performs cleanup of actions from earlier in
the test, however, a failure between the initial actions and the cleanup
will prevent the cleanup from taking place.

Fix these problems by modernizing and simplifying the test and by using
test_when_finished() for the cleanup action.

Signed-off-by: Eric Sunshine 
---
 t/t5505-remote.sh | 8 ++--
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/t/t5505-remote.sh b/t/t5505-remote.sh
index a6c0178f3a..3552b51b4c 100755
--- a/t/t5505-remote.sh
+++ b/t/t5505-remote.sh
@@ -348,17 +348,13 @@ URL: $(pwd)/one
 EOF
 
 test_expect_success 'prune --dry-run' '
-   (
-   cd one &&
-   git branch -m side2 side) &&
+   git -C one branch -m side2 side &&
+   test_when_finished "git -C one branch -m side side2" &&
(
cd test &&
git remote prune --dry-run origin >output &&
git rev-parse refs/remotes/origin/side2 &&
test_must_fail git rev-parse refs/remotes/origin/side &&
-   (
-   cd ../one &&
-   git branch -m side side2) &&
test_i18ncmp expect output
)
 '
-- 
2.18.0.203.gfac676dfb9



[PATCH 11/25] t7400: fix broken "submodule add/reconfigure --force" test

2018-07-01 Thread Eric Sunshine
This test has been dysfunctional since it was added by 619acfc78c
(submodule add: extend force flag to add existing repos, 2016-10-06),
however, two problems early in the test went unnoticed due to a broken
&&-chain later in the test.

First, it tries configuring the submodule with repository "bogus-url",
however, "git submodule add" insists that the repository be either an
absolute URL or a relative pathname requiring prefix "./" or "../" (this
is true even with --force), but "bogus-url" does not meet those
criteria, thus the command fails.

Second, it then tries configuring a submodule with a path which is
.gitignore'd, which is disallowed. This restriction can be overridden
with --force, but the test neglects to use that option.

Fix both problems, as well as the broken &&-chain behind which they hid.

Reviewed-by: Stefan Beller 
Signed-off-by: Eric Sunshine 
---
 t/t7400-submodule-basic.sh | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh
index 812db137b8..401adaed32 100755
--- a/t/t7400-submodule-basic.sh
+++ b/t/t7400-submodule-basic.sh
@@ -171,12 +171,12 @@ test_expect_success 'submodule add to .gitignored path 
with --force' '
 test_expect_success 'submodule add to reconfigure existing submodule with 
--force' '
(
cd addtest-ignore &&
-   git submodule add --force bogus-url submod &&
-   git submodule add -b initial "$submodurl" submod-branch &&
-   test "bogus-url" = "$(git config -f .gitmodules 
submodule.submod.url)" &&
-   test "bogus-url" = "$(git config submodule.submod.url)" &&
+   git submodule add --force /bogus-url submod &&
+   git submodule add --force -b initial "$submodurl" submod-branch 
&&
+   test "/bogus-url" = "$(git config -f .gitmodules 
submodule.submod.url)" &&
+   test "/bogus-url" = "$(git config submodule.submod.url)" &&
# Restore the url
-   git submodule add --force "$submodurl" submod
+   git submodule add --force "$submodurl" submod &&
test "$submodurl" = "$(git config -f .gitmodules 
submodule.submod.url)" &&
test "$submodurl" = "$(git config submodule.submod.url)"
)
-- 
2.18.0.203.gfac676dfb9



git@vger.kernel.org

2018-07-01 Thread Eric Sunshine
Signed-off-by: Eric Sunshine 
---
 t/t7001-mv.sh  |  2 +-
 t/t7201-co.sh  | 40 +-
 t/t7400-submodule-basic.sh |  2 +-
 t/t7406-submodule-update.sh|  6 ++--
 t/t7408-submodule-reference.sh |  2 +-
 t/t7501-commit.sh  | 52 +-
 t/t7506-status-submodule.sh| 10 +++
 7 files changed, 57 insertions(+), 57 deletions(-)

diff --git a/t/t7001-mv.sh b/t/t7001-mv.sh
index cc3fd2baf2..9e59e5a5dd 100755
--- a/t/t7001-mv.sh
+++ b/t/t7001-mv.sh
@@ -509,7 +509,7 @@ test_expect_success 'moving nested submodules' '
touch nested_level1 &&
git init &&
git add . &&
-   git commit -m "nested level 1"
+   git commit -m "nested level 1" &&
git submodule add ../sub_nested_nested &&
git commit -m "add nested level 2"
) &&
diff --git a/t/t7201-co.sh b/t/t7201-co.sh
index 8d8a63a24b..94cb039a03 100755
--- a/t/t7201-co.sh
+++ b/t/t7201-co.sh
@@ -528,10 +528,10 @@ test_expect_success 'checkout with --merge' '
cat sample >filf &&
git checkout -m -- fild file filf &&
(
-   echo "<<< ours"
-   echo ourside
-   echo "==="
-   echo theirside
+   echo "<<< ours" &&
+   echo ourside &&
+   echo "===" &&
+   echo theirside &&
echo ">>> theirs"
) >merged &&
test_cmp expect fild &&
@@ -549,12 +549,12 @@ test_expect_success 'checkout with --merge, in diff3 -m 
style' '
cat sample >filf &&
git checkout -m -- fild file filf &&
(
-   echo "<<< ours"
-   echo ourside
-   echo "||| base"
-   echo original
-   echo "==="
-   echo theirside
+   echo "<<< ours" &&
+   echo ourside &&
+   echo "||| base" &&
+   echo original &&
+   echo "===" &&
+   echo theirside &&
echo ">>> theirs"
) >merged &&
test_cmp expect fild &&
@@ -572,10 +572,10 @@ test_expect_success 'checkout --conflict=merge, 
overriding config' '
cat sample >filf &&
git checkout --conflict=merge -- fild file filf &&
(
-   echo "<<< ours"
-   echo ourside
-   echo "==="
-   echo theirside
+   echo "<<< ours" &&
+   echo ourside &&
+   echo "===" &&
+   echo theirside &&
echo ">>> theirs"
) >merged &&
test_cmp expect fild &&
@@ -593,12 +593,12 @@ test_expect_success 'checkout --conflict=diff3' '
cat sample >filf &&
git checkout --conflict=diff3 -- fild file filf &&
(
-   echo "<<< ours"
-   echo ourside
-   echo "||| base"
-   echo original
-   echo "==="
-   echo theirside
+   echo "<<< ours" &&
+   echo ourside &&
+   echo "||| base" &&
+   echo original &&
+   echo "===" &&
+   echo theirside &&
echo ">>> theirs"
) >merged &&
test_cmp expect fild &&
diff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh
index 401adaed32..76cf522a08 100755
--- a/t/t7400-submodule-basic.sh
+++ b/t/t7400-submodule-basic.sh
@@ -818,7 +818,7 @@ test_expect_success '../bar/a/b/c works with relative local 
path - ../foo/bar.gi
cp pristine-.git-config .git/config &&
cp pristine-.gitmodules .gitmodules &&
mkdir -p a/b/c &&
-   (cd a/b/c; git init) &&
+   (cd a/b/c && git init) &&
git config remote.origin.url ../foo/bar.git &&
git submodule add ../bar/a/b/c ./a/b/c &&
git submodule init &&
diff --git a/t/t7406-submodule-update.sh b/t/t7406-submodule-update.sh
index 9e0d31700e..f604ef7a72 100755
--- a/t/t7406-submodule-update.sh
+++ b/t/t7406-submodule-update.sh
@@ -865,9 +865,9 @@ test_expect_success 'submodule update places git-dir in 
superprojects git-dir re
 (cd submodule/subsubmodule &&
  git log > ../../expected
 ) &&
-(cd .git/modules/submodule/modules/subsubmodule
+(cd .git/modules/submodule/modules/subsubmodule &&
  git log > ../../../../../actual
-)
+) &&
 test_cmp actual expected
)
 '
@@ -886,7 +886,7 @@ test_expect_success 'submodule update properly revives a 
moved submodule' '
 git commit -am "pre move" &&
 H2=$(git rev-parse --short HEAD) &&
 git status | sed "s/$H/XXX/" >expect &&
-H=$(cd submodule2; git rev-parse HEAD) &&
+H=$(cd submodule2 &&

[PATCH 04/25] t: drop unnecessary terminating semicolon in subshell

2018-07-01 Thread Eric Sunshine
Signed-off-by: Eric Sunshine 
---
 t/t3102-ls-tree-wildcards.sh| 2 +-
 t/t4010-diff-pathspec.sh| 4 ++--
 t/t9400-git-cvsserver-server.sh | 2 +-
 3 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/t/t3102-ls-tree-wildcards.sh b/t/t3102-ls-tree-wildcards.sh
index e804377f1c..1e16c6b8ea 100755
--- a/t/t3102-ls-tree-wildcards.sh
+++ b/t/t3102-ls-tree-wildcards.sh
@@ -23,7 +23,7 @@ test_expect_success 'ls-tree outside prefix' '
cat >expect <<-EOF &&
100644 blob $EMPTY_BLOB ../a[a]/three
EOF
-   ( cd aa && git ls-tree -r HEAD "../a[a]"; ) >actual &&
+   ( cd aa && git ls-tree -r HEAD "../a[a]" ) >actual &&
test_cmp expect actual
 '
 
diff --git a/t/t4010-diff-pathspec.sh b/t/t4010-diff-pathspec.sh
index 35b35a81c8..b7f25071cf 100755
--- a/t/t4010-diff-pathspec.sh
+++ b/t/t4010-diff-pathspec.sh
@@ -111,10 +111,10 @@ test_expect_success 'diff-tree -r with wildcard' '
 test_expect_success 'setup submodules' '
test_tick &&
git init submod &&
-   ( cd submod && test_commit first; ) &&
+   ( cd submod && test_commit first ) &&
git add submod &&
git commit -m first &&
-   ( cd submod && test_commit second; ) &&
+   ( cd submod && test_commit second ) &&
git add submod &&
git commit -m second
 '
diff --git a/t/t9400-git-cvsserver-server.sh b/t/t9400-git-cvsserver-server.sh
index 06742748e9..6b09da79bf 100755
--- a/t/t9400-git-cvsserver-server.sh
+++ b/t/t9400-git-cvsserver-server.sh
@@ -328,7 +328,7 @@ test_expect_success 'cvs update (subdirectories)' \
   '(for dir in A A/B A/B/C A/D E; do
   mkdir $dir &&
   echo "test file in $dir" >"$dir/file_in_$(echo $dir|sed -e "s#/# #g")"  
&&
-  git add $dir;
+  git add $dir
done) &&
git commit -q -m "deep sub directory structure" &&
git push gitcvs.git >/dev/null &&
-- 
2.18.0.203.gfac676dfb9



git@vger.kernel.org

2018-07-01 Thread Eric Sunshine
This series fixes several buggy tests which went undetected due to
broken &&-chains in subshells, modernizes some tests to take advantage
of test functions (test_might_fail(), test_write_lines(), etc.), and
fixes a lot of broken &&-chains in subshells. It applies atop
'master'. Happily, there are no broken &&-chains in subshells in any
in-flight topic.

It is split out from an earlier series[1] which additionally extended
--chain-lint to work within subshells. I decided to make this an
independent series because these (hopefully) non-controversial changes
all stand on their own merit, and because I don't want to flood the list
repeatedly with this lengthy series as I re-roll the "extend
--chain-lint to work in subshells" functionality from [1].

To ease review burden, I largely avoided noisy modernizations and
cleanups, and limited changes to merely adding "&&" even when some other
transformation would have made the fix nicer overall. (For example, I
resisted the urge to replace a series of 'echo' statements with a simple
here-doc.)

Changes since [1]:

* Found and fixed more &&-chain breakage, and converted a couple more
  'unset' instances (which were hidden behind a MINGW prerequisite) to
  sane_unset().

* Rewrote commit messages to sell changes on their own merit rather than
  leaning on --chain-lint as a crutch. (junio, jrnieder)

* Changed a modernization/cleanup to use "! non-git-command" rather than
  test_must_fail(), moving it to its own patch in the process. (j6t)

* Changed "printf '%s\n'" idiom to test_write_lines(). (junio)

* Incorporated Stefan's fix[2] for a broken 't/lib-submodule-update'
  test since my interpretation of the problem was incorrect.

* Converted a subshell 'echo' sequence to write_script().

* Dropped patches which existed primarily to pacify --chain-lint; they
  are no longer needed since I re-wrote the "linter" to detect &&-chain
  breakage itself (by pure textual inspection) rather than by
  incorporating subshell bodies into the main &&-chain:

  t0001: use "{...}" block around "||" expression rather than subshell
  t3303: use standard here-doc tag "EOF" to avoid fooling --chain-lint
  t9104: use "{...}" block around "||" expression rather than subshell
  t9401: drop unnecessary nested subshell

* Dropped a patch which pretty much duplicated Junio's 037714252f
  (tests: clean after SANITY tests, 2018-06-15), which graduated to
  'master':

  t7508: use test_when_finished() instead of managing exit code manually

An interdiff against [1] is below, although I stripped out all the noisy
"printf '%s\n'" to test_write_lines() differences, of which there were a
lot, since they drowned out the other more significant changes.

Thanks to Elijah, Hannes, Jonathan Nieder, Jonathan Tan, Junio, Luke,
Peff, and Stefan for comments on [1].

[1]: https://public-inbox.org/git/20180626073001.6555-1-sunsh...@sunshineco.com/
[2]: https://public-inbox.org/git/20180627183057.254467-1-sbel...@google.com/

Eric Sunshine (25):
  t: use test_might_fail() instead of manipulating exit code manually
  t: use test_write_lines() instead of series of 'echo' commands
  t: use sane_unset() rather than 'unset' with broken &&-chain
  t: drop unnecessary terminating semicolon in subshell
  t/lib-submodule-update: fix "absorbing" test
  t5405: use test_must_fail() instead of checking exit code manually
  t5406: use write_script() instead of birthing shell script manually
  t5505: modernize and simplify hard-to-digest test
  t6036: fix broken "merge fails but has appropriate contents" tests
  t7201: drop pointless "exit 0" at end of subshell
  t7400: fix broken "submodule add/reconfigure --force" test
  t7810: use test_expect_code() instead of hand-rolled comparison
  t9001: fix broken "invoke hook" test
  t9814: simplify convoluted check that command correctly errors out
  t-t0999: fix broken &&-chains
  t1000-t1999: fix broken &&-chains
  t2000-t2999: fix broken &&-chains
  t3000-t3999: fix broken &&-chains
  t3030: fix broken &&-chains
  t4000-t4999: fix broken &&-chains
  t5000-t5999: fix broken &&-chains
  t6000-t6999: fix broken &&-chains
  t7000-t7999: fix broken &&-chains
  t9000-t: fix broken &&-chains
  t9119: fix broken &&-chains

 t/lib-submodule-update.sh |   5 +-
 t/t-basic.sh  |   2 +-
 t/t0001-init.sh   |   4 +-
 t/t0003-attributes.sh |  24 +-
 t/t0021-conversion.sh |   4 +-
 t/t0090-cache-tree.sh |   2 +-
 t/t1004-read-tree-m-u-wf.sh   |   8 +-
 t/t1005-read-tree-reset.sh|  10 +-
 t/t1008-read-tree-overlay.sh  |   2 +-
 t/t1020-subdirectory.sh   |   2 +-
 t/t1050-large.sh  |   6 +-
 t/t1300-config.sh |   2 +-
 t/t1411-reflog-show.sh|   6 +-
 t/t1507-rev-parse-upstream.sh 

[PATCH 01/25] t: use test_might_fail() instead of manipulating exit code manually

2018-07-01 Thread Eric Sunshine
These tests manually coerce the exit code of invoked commands to
"success" when they don't care if the command succeeds or fails since
failure of those commands should not cause the test to fail overall.
In doing so, they intentionally break the &&-chain. Modernize by
replacing manual exit code management with test_might_fail() and a
normal &&-chain.

Signed-off-by: Eric Sunshine 
---
 t/t1507-rev-parse-upstream.sh | 6 +++---
 t/t1700-split-index.sh| 2 +-
 t/t4012-diff-binary.sh| 6 ++
 t/t5400-send-pack.sh  | 4 ++--
 4 files changed, 8 insertions(+), 10 deletions(-)

diff --git a/t/t1507-rev-parse-upstream.sh b/t/t1507-rev-parse-upstream.sh
index 93c77eac45..349f6e10af 100755
--- a/t/t1507-rev-parse-upstream.sh
+++ b/t/t1507-rev-parse-upstream.sh
@@ -123,9 +123,9 @@ test_expect_success 'checkout -b new my-side@{u} forks from 
the same' '
 
 test_expect_success 'merge my-side@{u} records the correct name' '
 (
-   cd clone || exit
-   git checkout master || exit
-   git branch -D new ;# can fail but is ok
+   cd clone &&
+   git checkout master &&
+   test_might_fail git branch -D new &&
git branch -t new my-side@{u} &&
git merge -s ours new@{u} &&
git show -s --pretty=tformat:%s >actual &&
diff --git a/t/t1700-split-index.sh b/t/t1700-split-index.sh
index 1e81b33b2e..39133bcbc8 100755
--- a/t/t1700-split-index.sh
+++ b/t/t1700-split-index.sh
@@ -435,7 +435,7 @@ test_expect_success 'writing split index with null sha1 
does not write cache tre
commit=$(git commit-tree $tree -p HEAD cache-tree.out || true) &&
+   test_might_fail test-tool dump-cache-tree >cache-tree.out &&
test_line_count = 0 cache-tree.out
 '
 
diff --git a/t/t4012-diff-binary.sh b/t/t4012-diff-binary.sh
index 0a8af76aab..6579c81216 100755
--- a/t/t4012-diff-binary.sh
+++ b/t/t4012-diff-binary.sh
@@ -102,10 +102,8 @@ test_expect_success 'apply binary patch' '
 
 test_expect_success 'diff --no-index with binary creation' '
echo Q | q_to_nul >binary &&
-   (: hide error code from diff, which just indicates differences
-git diff --binary --no-index /dev/null binary >current ||
-true
-   ) &&
+   # hide error code from diff, which just indicates differences
+   test_might_fail git diff --binary --no-index /dev/null binary >current 
&&
rm binary &&
git apply --binary expected &&
diff --git a/t/t5400-send-pack.sh b/t/t5400-send-pack.sh
index 911eae1bf7..f1932ea431 100755
--- a/t/t5400-send-pack.sh
+++ b/t/t5400-send-pack.sh
@@ -86,7 +86,7 @@ test_expect_success 'push can be used to delete a ref' '
 test_expect_success 'refuse deleting push with denyDeletes' '
(
cd victim &&
-   ( git branch -D extra || : ) &&
+   test_might_fail git branch -D extra &&
git config receive.denyDeletes true &&
git branch extra master
) &&
@@ -119,7 +119,7 @@ test_expect_success 'override denyDeletes with git -c 
receive-pack' '
 test_expect_success 'denyNonFastforwards trumps --force' '
(
cd victim &&
-   ( git branch -D extra || : ) &&
+   test_might_fail git branch -D extra &&
git config receive.denyNonFastforwards true
) &&
victim_orig=$(cd victim && git rev-parse --verify master) &&
-- 
2.18.0.203.gfac676dfb9



git@vger.kernel.org

2018-07-01 Thread Eric Sunshine
These tests intentionally break the &&-chain after using 'unset' since
they don't know if 'unset' will succeed or fail and don't want a local
'unset' failure to fail the test overall. We can do better by using
sane_unset(), which can be linked into the &&-chain as usual.

Signed-off-by: Eric Sunshine 
---
 t/t0001-init.sh   | 4 ++--
 t/t1300-config.sh | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/t/t0001-init.sh b/t/t0001-init.sh
index 4c865051e7..ca85aae51e 100755
--- a/t/t0001-init.sh
+++ b/t/t0001-init.sh
@@ -408,7 +408,7 @@ is_hidden () {
 test_expect_success MINGW '.git hidden' '
rm -rf newdir &&
(
-   unset GIT_DIR GIT_WORK_TREE
+   sane_unset GIT_DIR GIT_WORK_TREE &&
mkdir newdir &&
cd newdir &&
git init &&
@@ -420,7 +420,7 @@ test_expect_success MINGW '.git hidden' '
 test_expect_success MINGW 'bare git dir not hidden' '
rm -rf newdir &&
(
-   unset GIT_DIR GIT_WORK_TREE GIT_CONFIG
+   sane_unset GIT_DIR GIT_WORK_TREE GIT_CONFIG &&
mkdir newdir &&
cd newdir &&
git --bare init
diff --git a/t/t1300-config.sh b/t/t1300-config.sh
index 03c223708e..24706ba412 100755
--- a/t/t1300-config.sh
+++ b/t/t1300-config.sh
@@ -888,7 +888,7 @@ EOF
 
 test_expect_success !MINGW 'get --path copes with unset $HOME' '
(
-   unset HOME;
+   sane_unset HOME &&
test_must_fail git config --get --path path.home \
>result 2>msg &&
git config --get --path path.normal >>result &&
-- 
2.18.0.203.gfac676dfb9



[PATCH 12/25] t7810: use test_expect_code() instead of hand-rolled comparison

2018-07-01 Thread Eric Sunshine
This test manually checks the exit code of git-grep for a particular
value. In doing so, it intentionally breaks the &&-chain. Modernize the
test by taking advantage of test_expect_code() and a normal &&-chain.

Signed-off-by: Eric Sunshine 
---
 t/t7810-grep.sh | 7 +++
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/t/t7810-grep.sh b/t/t7810-grep.sh
index 1797f632a3..fecee602c1 100755
--- a/t/t7810-grep.sh
+++ b/t/t7810-grep.sh
@@ -845,10 +845,9 @@ test_expect_success 'grep from a subdirectory to search 
wider area (1)' '
 test_expect_success 'grep from a subdirectory to search wider area (2)' '
mkdir -p s &&
(
-   cd s || exit 1
-   ( git grep xxyyzz .. >out ; echo $? >status )
-   ! test -s out &&
-   test 1 = $(cat status)
+   cd s &&
+   test_expect_code 1 git grep xxyyzz .. >out &&
+   ! test -s out
)
 '
 
-- 
2.18.0.203.gfac676dfb9



git@vger.kernel.org

2018-07-01 Thread Eric Sunshine
Signed-off-by: Eric Sunshine 
---
 t/t9001-send-email.sh |  6 +++---
 t/t9100-git-svn-basic.sh  |  2 +-
 t/t9101-git-svn-props.sh  |  2 +-
 t/t9122-git-svn-author.sh |  6 +++---
 t/t9129-git-svn-i18n-commitencoding.sh|  2 +-
 t/t9130-git-svn-authors-file.sh   |  4 ++--
 t/t9134-git-svn-ignore-paths.sh   |  6 +++---
 t/t9137-git-svn-dcommit-clobber-series.sh |  2 +-
 t/t9138-git-svn-authors-prog.sh   |  6 +++---
 t/t9146-git-svn-empty-dirs.sh | 20 +--
 t/t9147-git-svn-include-paths.sh  |  6 +++---
 t/t9152-svn-empty-dirs-after-gc.sh|  2 +-
 t/t9164-git-svn-dcommit-concurrent.sh |  2 +-
 ...65-git-svn-fetch-merge-branch-of-branch.sh |  2 +-
 t/t9200-git-cvsexportcommit.sh|  6 +++---
 t/t9302-fast-import-unpack-limit.sh   |  2 +-
 t/t9400-git-cvsserver-server.sh   |  6 +++---
 t/t9600-cvsimport.sh  |  2 +-
 t/t9806-git-p4-options.sh |  2 +-
 t/t9810-git-p4-rcs.sh |  2 +-
 t/t9811-git-p4-label-import.sh|  2 +-
 t/t9815-git-p4-submit-fail.sh |  2 +-
 t/t9830-git-p4-symlink-dir.sh |  2 +-
 t/t9831-git-p4-triggers.sh|  2 +-
 t/t9902-completion.sh |  4 ++--
 t/t9903-bash-prompt.sh|  2 +-
 26 files changed, 51 insertions(+), 51 deletions(-)

diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh
index 776769fe0d..53314ff54e 100755
--- a/t/t9001-send-email.sh
+++ b/t/t9001-send-email.sh
@@ -330,7 +330,7 @@ test_expect_success $PREREQ 'Show all headers' '
 
 test_expect_success $PREREQ 'Prompting works' '
clean_fake_sendmail &&
-   (echo "t...@example.com"
+   (echo "t...@example.com" &&
 echo ""
) | GIT_SEND_EMAIL_NOTTY=1 git send-email \
--smtp-server="$(pwd)/fake.sendmail" \
@@ -470,8 +470,8 @@ test_expect_success $PREREQ 'Invalid In-Reply-To' '
 
 test_expect_success $PREREQ 'Valid In-Reply-To when prompting' '
clean_fake_sendmail &&
-   (echo "From Example "
-echo "To Example "
+   (echo "From Example " &&
+echo "To Example " &&
 echo ""
) | GIT_SEND_EMAIL_NOTTY=1 git send-email \
--smtp-server="$(pwd)/fake.sendmail" \
diff --git a/t/t9100-git-svn-basic.sh b/t/t9100-git-svn-basic.sh
index c937330a5f..9af6078844 100755
--- a/t/t9100-git-svn-basic.sh
+++ b/t/t9100-git-svn-basic.sh
@@ -31,7 +31,7 @@ test_expect_success \
(
cd import &&
echo foo >foo &&
-   ln -s foo foo.link
+   ln -s foo foo.link &&
mkdir -p dir/a/b/c/d/e &&
echo "deep dir" >dir/a/b/c/d/e/file &&
mkdir bar &&
diff --git a/t/t9101-git-svn-props.sh b/t/t9101-git-svn-props.sh
index 07bfb63777..8a5c8dc1aa 100755
--- a/t/t9101-git-svn-props.sh
+++ b/t/t9101-git-svn-props.sh
@@ -149,7 +149,7 @@ test_expect_success 'test show-ignore' "
svn_cmd up &&
svn_cmd propset -R svn:ignore '
 no-such-file*
-' .
+' . &&
svn_cmd commit -m 'propset svn:ignore'
) &&
git svn show-ignore > show-ignore.got &&
diff --git a/t/t9122-git-svn-author.sh b/t/t9122-git-svn-author.sh
index 30013b7bb9..9e8fe38e7e 100755
--- a/t/t9122-git-svn-author.sh
+++ b/t/t9122-git-svn-author.sh
@@ -7,8 +7,8 @@ test_expect_success 'setup svn repository' '
svn_cmd checkout "$svnrepo" work.svn &&
(
cd work.svn &&
-   echo >file
-   svn_cmd add file
+   echo >file &&
+   svn_cmd add file &&
svn_cmd commit -m "first commit" file
)
 '
@@ -17,7 +17,7 @@ test_expect_success 'interact with it via git svn' '
mkdir work.git &&
(
cd work.git &&
-   git svn init "$svnrepo"
+   git svn init "$svnrepo" &&
git svn fetch &&
 
echo modification >file &&
diff --git a/t/t9129-git-svn-i18n-commitencoding.sh 
b/t/t9129-git-svn-i18n-commitencoding.sh
index 8dbd6476fa..2c213ae654 100755
--- a/t/t9129-git-svn-i18n-commitencoding.sh
+++ b/t/t9129-git-svn-i18n-commitencoding.sh
@@ -51,7 +51,7 @@ do
git add F &&
git commit -a -F "$TEST_DIRECTORY"/t3900/$H.txt &&
E=$(git cat-file commit HEAD | sed -ne "s/^encoding //p") &&
-   test "z$E" = "z$H"
+   test "z$E" = "z$H" &&
compare_git_head_with "$TEST_DIRECTORY"/t3900/$H.txt
)
'
diff --git a/t/t9130-git-svn-authors-file.sh b/t/t9130-git-svn-authors-file.sh
index d8262854bb..cb764bcadc 100755
--- a/t/t9130-git-svn-authors-file.sh
+++ b/t/t9130-git-svn-authors-file.sh
@@ -25,7 +25,7 @@ test_

[PATCH 05/25] t/lib-submodule-update: fix "absorbing" test

2018-07-01 Thread Eric Sunshine
This test has been dysfunctional since it was added by 259f3ee296
(lib-submodule-update.sh: define tests for recursing into submodules,
2017-03-14), however, the problem went unnoticed due to a broken
&&-chain.

The test wants to verify that replacing a submodule containing a .git
directory will absorb the .git directory into the .git/modules/ of the
superproject, and then replace the working tree content appropriate to
the superproject. It is, therefore, incorrect to check if the
submodule content still exists since the submodule will have been
replaced by the content of the superproject.

Fix this by removing the submodule content check, which also happens
to be the line that broke the &&-chain.

While at it, fix broken &&-chains in a couple neighboring tests.

Helped-by: Stefan Beller 
Signed-off-by: Eric Sunshine 
Signed-off-by: Stefan Beller 
Signed-off-by: Eric Sunshine 
---
 t/lib-submodule-update.sh | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/t/lib-submodule-update.sh b/t/lib-submodule-update.sh
index 1f38a85371..e90ec79087 100755
--- a/t/lib-submodule-update.sh
+++ b/t/lib-submodule-update.sh
@@ -755,7 +755,7 @@ test_submodule_recursing_with_args_common() {
: >sub1/untrackedfile &&
test_must_fail $command replace_sub1_with_file &&
test_superproject_content origin/add_sub1 &&
-   test_submodule_content sub1 origin/add_sub1
+   test_submodule_content sub1 origin/add_sub1 &&
test -f sub1/untracked_file
)
'
@@ -842,7 +842,7 @@ test_submodule_switch_recursing_with_args () {
cd submodule_update &&
git branch -t add_sub1 origin/add_sub1 &&
: >sub1 &&
-   echo sub1 >.git/info/exclude
+   echo sub1 >.git/info/exclude &&
$command add_sub1 &&
test_superproject_content origin/add_sub1 &&
test_submodule_content sub1 origin/add_sub1
@@ -969,7 +969,6 @@ test_submodule_forced_switch_recursing_with_args () {
rm -rf .git/modules/sub1 &&
$command replace_sub1_with_directory &&
test_superproject_content 
origin/replace_sub1_with_directory &&
-   test_submodule_content sub1 origin/modify_sub1
test_git_directory_exists sub1
)
'
-- 
2.18.0.203.gfac676dfb9



git@vger.kernel.org

2018-07-01 Thread Eric Sunshine
Signed-off-by: Eric Sunshine 
---
 t/t9119-git-svn-info.sh | 120 
 1 file changed, 60 insertions(+), 60 deletions(-)

diff --git a/t/t9119-git-svn-info.sh b/t/t9119-git-svn-info.sh
index 88241baee3..8201c3e808 100755
--- a/t/t9119-git-svn-info.sh
+++ b/t/t9119-git-svn-info.sh
@@ -22,8 +22,8 @@ esac
 # same value as "svn info" (i.e. the commit timestamp that touched the
 # path most recently); do not expect that field to match.
 test_cmp_info () {
-   sed -e '/^Text Last Updated:/d' "$1" >tmp.expect
-   sed -e '/^Text Last Updated:/d' "$2" >tmp.actual
+   sed -e '/^Text Last Updated:/d' "$1" >tmp.expect &&
+   sed -e '/^Text Last Updated:/d' "$2" >tmp.actual &&
test_cmp tmp.expect tmp.actual &&
rm -f tmp.expect tmp.actual
 }
@@ -59,24 +59,24 @@ test_expect_success 'setup repository and import' '
'
 
 test_expect_success 'info' "
-   (cd svnwc; svn info) > expected.info &&
-   (cd gitwc; git svn info) > actual.info &&
+   (cd svnwc && svn info) > expected.info &&
+   (cd gitwc && git svn info) > actual.info &&
test_cmp_info expected.info actual.info
"
 
 test_expect_success 'info --url' '
-   test "$(cd gitwc; git svn info --url)" = "$quoted_svnrepo"
+   test "$(cd gitwc && git svn info --url)" = "$quoted_svnrepo"
'
 
 test_expect_success 'info .' "
-   (cd svnwc; svn info .) > expected.info-dot &&
-   (cd gitwc; git svn info .) > actual.info-dot &&
+   (cd svnwc && svn info .) > expected.info-dot &&
+   (cd gitwc && git svn info .) > actual.info-dot &&
test_cmp_info expected.info-dot actual.info-dot
"
 
 test_expect_success 'info $(pwd)' '
-   (cd svnwc; svn info "$(pwd)") >expected.info-pwd &&
-   (cd gitwc; git svn info "$(pwd)") >actual.info-pwd &&
+   (cd svnwc && svn info "$(pwd)") >expected.info-pwd &&
+   (cd gitwc && git svn info "$(pwd)") >actual.info-pwd &&
grep -v ^Path: expected.info-np &&
grep -v ^Path: actual.info-np &&
test_cmp_info expected.info-np actual.info-np &&
@@ -85,8 +85,8 @@ test_expect_success 'info $(pwd)' '
'
 
 test_expect_success 'info $(pwd)/../___wc' '
-   (cd svnwc; svn info "$(pwd)/../svnwc") >expected.info-pwd &&
-   (cd gitwc; git svn info "$(pwd)/../gitwc") >actual.info-pwd &&
+   (cd svnwc && svn info "$(pwd)/../svnwc") >expected.info-pwd &&
+   (cd gitwc && git svn info "$(pwd)/../gitwc") >actual.info-pwd &&
grep -v ^Path: expected.info-np &&
grep -v ^Path: actual.info-np &&
test_cmp_info expected.info-np actual.info-np &&
@@ -95,8 +95,8 @@ test_expect_success 'info $(pwd)/../___wc' '
'
 
 test_expect_success 'info $(pwd)/../___wc//file' '
-   (cd svnwc; svn info "$(pwd)/../svnwc//file") >expected.info-pwd &&
-   (cd gitwc; git svn info "$(pwd)/../gitwc//file") >actual.info-pwd &&
+   (cd svnwc && svn info "$(pwd)/../svnwc//file") >expected.info-pwd &&
+   (cd gitwc && git svn info "$(pwd)/../gitwc//file") >actual.info-pwd &&
grep -v ^Path: expected.info-np &&
grep -v ^Path: actual.info-np &&
test_cmp_info expected.info-np actual.info-np &&
@@ -105,56 +105,56 @@ test_expect_success 'info $(pwd)/../___wc//file' '
'
 
 test_expect_success 'info --url .' '
-   test "$(cd gitwc; git svn info --url .)" = "$quoted_svnrepo"
+   test "$(cd gitwc && git svn info --url .)" = "$quoted_svnrepo"
'
 
 test_expect_success 'info file' "
-   (cd svnwc; svn info file) > expected.info-file &&
-   (cd gitwc; git svn info file) > actual.info-file &&
+   (cd svnwc && svn info file) > expected.info-file &&
+   (cd gitwc && git svn info file) > actual.info-file &&
test_cmp_info expected.info-file actual.info-file
"
 
 test_expect_success 'info --url file' '
-   test "$(cd gitwc; git svn info --url file)" = "$quoted_svnrepo/file"
+   test "$(cd gitwc && git svn info --url file)" = "$quoted_svnrepo/file"
'
 
 test_expect_success 'info directory' "
-   (cd svnwc; svn info directory) > expected.info-directory &&
-   (cd gitwc; git svn info directory) > actual.info-directory &&
+   (cd svnwc && svn info directory) > expected.info-directory &&
+   (cd gitwc && git svn info directory) > actual.info-directory &&
test_cmp_info expected.info-directory actual.info-directory
"
 
 test_expect_success 'info inside directory' "
-   (cd svnwc/directory; svn info) > expected.info-inside-directory &&
-   (cd gitwc/directory; git svn info) > actual.info-inside-directory &&
+   (cd svnwc/directory && svn info) > expected.info-inside-directory &&
+   (cd gitwc/directory && git svn info) > actual.info-inside-directory &&
test_cmp_info expected.info-inside-directory 
actual.info-inside-directory
"
 
 test_expect_success 'info --url directory' '
-   test "$(cd gitwc; git svn info --url di

[PATCH 02/25] t: use test_write_lines() instead of series of 'echo' commands

2018-07-01 Thread Eric Sunshine
These tests employ a noisy subshell (with missing &&-chain) to feed
input into Git commands or files:

(echo a; echo b; echo c) | git some-command ...

Simplify by taking advantage of test_write_lines():

test_write_lines a b c | git some-command ...

Signed-off-by: Eric Sunshine 
---
 t/t0090-cache-tree.sh |  2 +-
 t/t1008-read-tree-overlay.sh  |  2 +-
 t/t2016-checkout-patch.sh | 24 ++--
 t/t3404-rebase-interactive.sh |  6 ++---
 t/t3701-add-interactive.sh| 16 +++---
 t/t3904-stash-patch.sh|  8 +++
 t/t7105-reset-patch.sh| 12 +-
 t/t7301-clean-interactive.sh  | 41 +--
 t/t7501-commit.sh |  4 ++--
 t/t7610-mergetool.sh  |  8 +++
 10 files changed, 61 insertions(+), 62 deletions(-)

diff --git a/t/t0090-cache-tree.sh b/t/t0090-cache-tree.sh
index 0c61268fd2..28ea93f509 100755
--- a/t/t0090-cache-tree.sh
+++ b/t/t0090-cache-tree.sh
@@ -156,7 +156,7 @@ test_expect_success PERL 'commit --interactive gives 
cache-tree on partial commi
return 44;
}
EOT
-   (echo p; echo 1; echo; echo s; echo n; echo y; echo q) |
+   test_write_lines p 1 "" s n y q |
git commit --interactive -m foo &&
test_cache_tree
 '
diff --git a/t/t1008-read-tree-overlay.sh b/t/t1008-read-tree-overlay.sh
index 4c50ed955e..cf96016844 100755
--- a/t/t1008-read-tree-overlay.sh
+++ b/t/t1008-read-tree-overlay.sh
@@ -23,7 +23,7 @@ test_expect_success setup '
 
 test_expect_success 'multi-read' '
read_tree_must_succeed initial master side &&
-   (echo a; echo b/c) >expect &&
+   test_write_lines a b/c >expect &&
git ls-files >actual &&
test_cmp expect actual
 '
diff --git a/t/t2016-checkout-patch.sh b/t/t2016-checkout-patch.sh
index 9cd0ac4ba3..47aeb0b167 100755
--- a/t/t2016-checkout-patch.sh
+++ b/t/t2016-checkout-patch.sh
@@ -20,33 +20,33 @@ test_expect_success PERL 'setup' '
 
 test_expect_success PERL 'saying "n" does nothing' '
set_and_save_state dir/foo work head &&
-   (echo n; echo n) | git checkout -p &&
+   test_write_lines n n | git checkout -p &&
verify_saved_state bar &&
verify_saved_state dir/foo
 '
 
 test_expect_success PERL 'git checkout -p' '
-   (echo n; echo y) | git checkout -p &&
+   test_write_lines n y | git checkout -p &&
verify_saved_state bar &&
verify_state dir/foo head head
 '
 
 test_expect_success PERL 'git checkout -p with staged changes' '
set_state dir/foo work index &&
-   (echo n; echo y) | git checkout -p &&
+   test_write_lines n y | git checkout -p &&
verify_saved_state bar &&
verify_state dir/foo index index
 '
 
 test_expect_success PERL 'git checkout -p HEAD with NO staged changes: abort' '
set_and_save_state dir/foo work head &&
-   (echo n; echo y; echo n) | git checkout -p HEAD &&
+   test_write_lines n y n | git checkout -p HEAD &&
verify_saved_state bar &&
verify_saved_state dir/foo
 '
 
 test_expect_success PERL 'git checkout -p HEAD with NO staged changes: apply' '
-   (echo n; echo y; echo y) | git checkout -p HEAD &&
+   test_write_lines n y y | git checkout -p HEAD &&
verify_saved_state bar &&
verify_state dir/foo head head
 '
@@ -54,14 +54,14 @@ test_expect_success PERL 'git checkout -p HEAD with NO 
staged changes: apply' '
 test_expect_success PERL 'git checkout -p HEAD with change already staged' '
set_state dir/foo index index &&
# the third n is to get out in case it mistakenly does not apply
-   (echo n; echo y; echo n) | git checkout -p HEAD &&
+   test_write_lines n y n | git checkout -p HEAD &&
verify_saved_state bar &&
verify_state dir/foo head head
 '
 
 test_expect_success PERL 'git checkout -p HEAD^' '
# the third n is to get out in case it mistakenly does not apply
-   (echo n; echo y; echo n) | git checkout -p HEAD^ &&
+   test_write_lines n y n | git checkout -p HEAD^ &&
verify_saved_state bar &&
verify_state dir/foo parent parent
 '
@@ -69,7 +69,7 @@ test_expect_success PERL 'git checkout -p HEAD^' '
 test_expect_success PERL 'git checkout -p handles deletion' '
set_state dir/foo work index &&
rm dir/foo &&
-   (echo n; echo y) | git checkout -p &&
+   test_write_lines n y | git checkout -p &&
verify_saved_state bar &&
verify_state dir/foo index index
 '
@@ -81,21 +81,21 @@ test_expect_success PERL 'git checkout -p handles deletion' 
'
 
 test_expect_success PERL 'path limiting works: dir' '
set_state dir/foo work head &&
-   (echo y; echo n) | git checkout -p dir &&
+   test_write_lines y n | git checkout -p dir &&
verify_saved_state bar &&
verify_state dir/foo head head
 '
 
 test_expect_success PERL 'path limiting works: -- dir' '
set_state dir/foo work head &&
-  

git@vger.kernel.org

2018-07-01 Thread Eric Sunshine
Signed-off-by: Eric Sunshine 
---
 t/t5300-pack-object.sh |  2 +-
 t/t5302-pack-index.sh  |  2 +-
 t/t5401-update-hooks.sh|  4 ++--
 t/t5500-fetch-pack.sh  |  2 +-
 t/t5505-remote.sh  |  2 +-
 t/t5512-ls-remote.sh   |  4 ++--
 t/t5516-fetch-push.sh  | 10 +-
 t/t5517-push-mirror.sh | 10 +-
 t/t5526-fetch-submodules.sh|  2 +-
 t/t5531-deep-submodule-push.sh |  2 +-
 t/t5543-atomic-push.sh |  2 +-
 t/t5601-clone.sh   |  2 +-
 t/t5605-clone-local.sh |  2 +-
 t/t5801-remote-helpers.sh  |  2 +-
 14 files changed, 24 insertions(+), 24 deletions(-)

diff --git a/t/t5300-pack-object.sh b/t/t5300-pack-object.sh
index 2336d09dcc..6c620cd540 100755
--- a/t/t5300-pack-object.sh
+++ b/t/t5300-pack-object.sh
@@ -191,7 +191,7 @@ test_expect_success 'survive missing objects/pack 
directory' '
mkdir missing-pack &&
cd missing-pack &&
git init &&
-   GOP=.git/objects/pack
+   GOP=.git/objects/pack &&
rm -fr $GOP &&
git index-pack --stdin --keep=test 
<../test-3-${packname_3}.pack &&
test -f $GOP/pack-${packname_3}.pack &&
diff --git a/t/t5302-pack-index.sh b/t/t5302-pack-index.sh
index bb9b8bb309..91d51b35f9 100755
--- a/t/t5302-pack-index.sh
+++ b/t/t5302-pack-index.sh
@@ -237,7 +237,7 @@ test_expect_success 'running index-pack in the object 
store' '
 rm -f .git/objects/pack/* &&
 cp test-1-${pack1}.pack .git/objects/pack/pack-${pack1}.pack &&
 (
-   cd .git/objects/pack
+   cd .git/objects/pack &&
git index-pack pack-${pack1}.pack
 ) &&
 test -f .git/objects/pack/pack-${pack1}.idx
diff --git a/t/t5401-update-hooks.sh b/t/t5401-update-hooks.sh
index 7f278d8ce9..b5f886a0e2 100755
--- a/t/t5401-update-hooks.sh
+++ b/t/t5401-update-hooks.sh
@@ -82,13 +82,13 @@ test_expect_success 'hooks ran' '
 '
 
 test_expect_success 'pre-receive hook input' '
-   (echo $commit0 $commit1 refs/heads/master;
+   (echo $commit0 $commit1 refs/heads/master &&
 echo $commit1 $commit0 refs/heads/tofail
) | test_cmp - victim.git/pre-receive.stdin
 '
 
 test_expect_success 'update hook arguments' '
-   (echo refs/heads/master $commit0 $commit1;
+   (echo refs/heads/master $commit0 $commit1 &&
 echo refs/heads/tofail $commit1 $commit0
) | test_cmp - victim.git/update.args
 '
diff --git a/t/t5500-fetch-pack.sh b/t/t5500-fetch-pack.sh
index ea6570e819..efe054 100755
--- a/t/t5500-fetch-pack.sh
+++ b/t/t5500-fetch-pack.sh
@@ -259,7 +259,7 @@ test_expect_success 'clone shallow object count' '
 test_expect_success 'pull in shallow repo with missing merge base' '
(
cd shallow &&
-   git fetch --depth 4 .. A
+   git fetch --depth 4 .. A &&
test_must_fail git merge --allow-unrelated-histories FETCH_HEAD
)
 '
diff --git a/t/t5505-remote.sh b/t/t5505-remote.sh
index 3552b51b4c..11e14a1e0f 100755
--- a/t/t5505-remote.sh
+++ b/t/t5505-remote.sh
@@ -844,7 +844,7 @@ test_expect_success 'migrate a remote from named file in 
$GIT_DIR/branches (2)'
git remote rename origin origin &&
test_path_is_missing .git/branches/origin &&
test "$(git config remote.origin.url)" = "quux" &&
-   test "$(git config remote.origin.fetch)" = 
"refs/heads/foom:refs/heads/origin"
+   test "$(git config remote.origin.fetch)" = 
"refs/heads/foom:refs/heads/origin" &&
test "$(git config remote.origin.push)" = "HEAD:refs/heads/foom"
)
 '
diff --git a/t/t5512-ls-remote.sh b/t/t5512-ls-remote.sh
index 6a949484d0..ea020040e8 100755
--- a/t/t5512-ls-remote.sh
+++ b/t/t5512-ls-remote.sh
@@ -15,7 +15,7 @@ test_expect_success setup '
git tag mark1.10 &&
git show-ref --tags -d | sed -e "s/ /   /" >expected.tag &&
(
-   echo "$(git rev-parse HEAD) HEAD"
+   echo "$(git rev-parse HEAD) HEAD" &&
git show-ref -d | sed -e "s/ /  /"
) >expected.all &&
 
@@ -105,7 +105,7 @@ test_expect_success 'use branch..remote if possible' '
git clone . other.git &&
(
cd other.git &&
-   echo "$(git rev-parse HEAD) HEAD"
+   echo "$(git rev-parse HEAD) HEAD" &&
git show-ref| sed -e "s/ /  /"
) >exp &&
 
diff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh
index a5077d8b7c..bd8f23e430 100755
--- a/t/t5516-fetch-push.sh
+++ b/t/t5516-fetch-push.sh
@@ -923,7 +923,7 @@ test_expect_success 'push into aliased refs (consistent)' '
(
cd child1 &&
git branch foo &&
-   git symbolic-ref refs/heads/bar refs/heads/foo
+   git symbolic-ref refs/heads/bar refs/heads/foo &&
git config receive.de

git@vger.kernel.org

2018-07-01 Thread Eric Sunshine
Signed-off-by: Eric Sunshine 
---
 t/t2103-update-index-ignore-missing.sh |  2 +-
 t/t2202-add-addremove.sh   | 14 +++---
 2 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/t/t2103-update-index-ignore-missing.sh 
b/t/t2103-update-index-ignore-missing.sh
index 332694e7d3..0114f05228 100755
--- a/t/t2103-update-index-ignore-missing.sh
+++ b/t/t2103-update-index-ignore-missing.sh
@@ -32,7 +32,7 @@ test_expect_success basics '
test_create_repo xyzzy &&
cd xyzzy &&
>file &&
-   git add file
+   git add file &&
git commit -m "sub initial"
) &&
git add xyzzy &&
diff --git a/t/t2202-add-addremove.sh b/t/t2202-add-addremove.sh
index 6a5a3166b1..17744e8c57 100755
--- a/t/t2202-add-addremove.sh
+++ b/t/t2202-add-addremove.sh
@@ -6,12 +6,12 @@ test_description='git add --all'
 
 test_expect_success setup '
(
-   echo .gitignore
+   echo .gitignore &&
echo will-remove
) >expect &&
(
-   echo actual
-   echo expect
+   echo actual &&
+   echo expect &&
echo ignored
) >.gitignore &&
git --literal-pathspecs add --all &&
@@ -25,10 +25,10 @@ test_expect_success setup '
 
 test_expect_success 'git add --all' '
(
-   echo .gitignore
-   echo not-ignored
-   echo "M .gitignore"
-   echo "A not-ignored"
+   echo .gitignore &&
+   echo not-ignored &&
+   echo "M .gitignore" &&
+   echo "A not-ignored" &&
echo "D will-remove"
) >expect &&
>ignored &&
-- 
2.18.0.203.gfac676dfb9



git@vger.kernel.org

2018-07-01 Thread Eric Sunshine
Signed-off-by: Eric Sunshine 
---
 t/t6010-merge-base.sh|  2 +-
 t/t6029-merge-subtree.sh | 16 
 t/t6036-recursive-corner-cases.sh|  6 +++---
 t/t6042-merge-rename-corner-cases.sh |  8 
 t/t6043-merge-rename-directories.sh  |  2 +-
 5 files changed, 17 insertions(+), 17 deletions(-)

diff --git a/t/t6010-merge-base.sh b/t/t6010-merge-base.sh
index aa2d360ce3..44c726ea39 100755
--- a/t/t6010-merge-base.sh
+++ b/t/t6010-merge-base.sh
@@ -245,7 +245,7 @@ test_expect_success 'using reflog to find the fork point' '
git commit --allow-empty -m "Derived #$count" &&
git rev-parse HEAD >derived$count &&
git checkout -B base $E || exit 1
-   done
+   done &&
 
for count in 1 2 3
do
diff --git a/t/t6029-merge-subtree.sh b/t/t6029-merge-subtree.sh
index 3e692454a7..7d5bc78472 100755
--- a/t/t6029-merge-subtree.sh
+++ b/t/t6029-merge-subtree.sh
@@ -55,7 +55,7 @@ test_expect_success 'initial merge' '
git checkout -b work &&
git ls-files -s >actual &&
(
-   echo "100644 $o1 0  git-gui/git-gui.sh"
+   echo "100644 $o1 0  git-gui/git-gui.sh" &&
echo "100644 $o2 0  git.c"
) >expected &&
test_cmp expected actual
@@ -72,7 +72,7 @@ test_expect_success 'merge update' '
git pull -s subtree gui master2 &&
git ls-files -s >actual &&
(
-   echo "100644 $o3 0  git-gui/git-gui.sh"
+   echo "100644 $o3 0  git-gui/git-gui.sh" &&
echo "100644 $o2 0  git.c"
) >expected &&
test_cmp expected actual
@@ -88,8 +88,8 @@ test_expect_success 'initial ambiguous subtree' '
git checkout -b work2 &&
git ls-files -s >actual &&
(
-   echo "100644 $o1 0  git-gui/git-gui.sh"
-   echo "100644 $o1 0  git-gui2/git-gui.sh"
+   echo "100644 $o1 0  git-gui/git-gui.sh" &&
+   echo "100644 $o1 0  git-gui2/git-gui.sh" &&
echo "100644 $o2 0  git.c"
) >expected &&
test_cmp expected actual
@@ -101,8 +101,8 @@ test_expect_success 'merge using explicit' '
git pull -Xsubtree=git-gui gui master2 &&
git ls-files -s >actual &&
(
-   echo "100644 $o3 0  git-gui/git-gui.sh"
-   echo "100644 $o1 0  git-gui2/git-gui.sh"
+   echo "100644 $o3 0  git-gui/git-gui.sh" &&
+   echo "100644 $o1 0  git-gui2/git-gui.sh" &&
echo "100644 $o2 0  git.c"
) >expected &&
test_cmp expected actual
@@ -114,8 +114,8 @@ test_expect_success 'merge2 using explicit' '
git pull -Xsubtree=git-gui2 gui master2 &&
git ls-files -s >actual &&
(
-   echo "100644 $o1 0  git-gui/git-gui.sh"
-   echo "100644 $o3 0  git-gui2/git-gui.sh"
+   echo "100644 $o1 0  git-gui/git-gui.sh" &&
+   echo "100644 $o3 0  git-gui2/git-gui.sh" &&
echo "100644 $o2 0  git.c"
) >expected &&
test_cmp expected actual
diff --git a/t/t6036-recursive-corner-cases.sh 
b/t/t6036-recursive-corner-cases.sh
index b32ff8e1db..892cf08743 100755
--- a/t/t6036-recursive-corner-cases.sh
+++ b/t/t6036-recursive-corner-cases.sh
@@ -72,7 +72,7 @@ test_expect_success 'merge simple rename+criss-cross with no 
modifications' '
git rev-parse   >actual \
:2:three   :3:three &&
git hash-object >>actual\
-   three~HEAD three~R2^0
+   three~HEAD three~R2^0 &&
test_cmp expect actual
)
 '
@@ -148,7 +148,7 @@ test_expect_success 'merge criss-cross + rename merges with 
basic modification'
git rev-parse   >actual \
:2:three   :3:three &&
git hash-object >>actual\
-   three~HEAD three~R2^0
+   three~HEAD three~R2^0 &&
test_cmp expect actual
)
 '
@@ -228,7 +228,7 @@ test_expect_success 'git detects differently handled merges 
conflict' '
D:new_a  E:new_a &&
git rev-parse   >actual \
:2:new_a :3:new_a &&
-   test_cmp expect actual
+   test_cmp expect actual &&
 
git cat-file -p B:new_a >ours &&
git cat-file -p C:new_a >theirs &&
diff --git a/t/t6042-merge-rename-corner-cases.sh 
b/t/t6042-merge-rename-corner-cases.sh
index 1cbd946fc2..661b633478 100755
--- a/t/t6042-merge-rename-corner-cases.sh
+++ b/t/t6042-merge-rename-corner-cases.sh
@@ -352,7 +352,7 @@ test_expect_success 'rename/directory conflict + content 
merge conflict' '
base:file

[PATCH 09/25] t6036: fix broken "merge fails but has appropriate contents" tests

2018-07-01 Thread Eric Sunshine
These tests reference non-existent object "c" when they really mean to
be referencing "C", however, these errors went unnoticed due to a broken
&&-chain later in the tests. Fix these errors, as well as the broken
&&-chains behind which they hid.

Reviewed-by: Elijah Newren 
Signed-off-by: Eric Sunshine 
---
 t/t6036-recursive-corner-cases.sh | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/t/t6036-recursive-corner-cases.sh 
b/t/t6036-recursive-corner-cases.sh
index b5621303d6..b32ff8e1db 100755
--- a/t/t6036-recursive-corner-cases.sh
+++ b/t/t6036-recursive-corner-cases.sh
@@ -506,10 +506,10 @@ test_expect_success 'merge of D & E2 fails but has 
appropriate contents' '
test_line_count = 2 out &&
 
git rev-parse >expect\
-   B:a   E2:a/file  c:a/file   A:ignore-me &&
+   B:a   E2:a/file  C:a/file   A:ignore-me &&
git rev-parse   >actual   \
:2:a  :3:a/file  :1:a/file  :0:ignore-me &&
-   test_cmp expect actual
+   test_cmp expect actual &&
 
test_path_is_file a~HEAD
)
@@ -533,10 +533,10 @@ test_expect_success 'merge of E2 & D fails but has 
appropriate contents' '
test_line_count = 2 out &&
 
git rev-parse >expect\
-   B:a   E2:a/file  c:a/file   A:ignore-me &&
+   B:a   E2:a/file  C:a/file   A:ignore-me &&
git rev-parse   >actual   \
:3:a  :2:a/file  :1:a/file  :0:ignore-me &&
-   test_cmp expect actual
+   test_cmp expect actual &&
 
test_path_is_file a~D^0
)
-- 
2.18.0.203.gfac676dfb9



[PATCH 13/25] t9001: fix broken "invoke hook" test

2018-07-01 Thread Eric Sunshine
This test has been dysfunctional since it was added by 6489660b4b
(send-email: support validate hook, 2017-05-12), however, the problem
went unnoticed due to a broken &&-chain late in the test.

The test wants to verify that a non-zero exit code from the
'sendemail-validate' hook causes git-send-email to abort with a
particular error message. A command which is expected to fail should be
run with 'test_must_fail', however, the test neglects to do so.

Fix this problem, as well as the broken &&-chain behind which the
problem hid.

Signed-off-by: Eric Sunshine 
---
 t/t9001-send-email.sh | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh
index e80eacbb1b..776769fe0d 100755
--- a/t/t9001-send-email.sh
+++ b/t/t9001-send-email.sh
@@ -1966,11 +1966,11 @@ test_expect_success $PREREQ 'invoke hook' '
 
# Verify error message when a patch is rejected by the hook
sed -e "s/add master/x/" ../0001-add-master.patch 
>../another.patch &&
-   git send-email \
+   test_must_fail git send-email \
--from="Example " \
--to=nob...@example.com \
--smtp-server="$(pwd)/../fake.sendmail" \
-   ../another.patch 2>err
+   ../another.patch 2>err &&
test_i18ngrep "rejected by sendemail-validate hook" err
)
 '
-- 
2.18.0.203.gfac676dfb9



[PATCH 06/25] t5405: use test_must_fail() instead of checking exit code manually

2018-07-01 Thread Eric Sunshine
This test expects "git push" to fail, thus it manually inverts that
local expected failure into a successful exit code for the test overall.
In doing so, it intentionally breaks the &&-chain. Modernize by
replacing manual exit code management with test_must_fail() and a normal
&&-chain.

Signed-off-by: Eric Sunshine 
---
 t/t5405-send-pack-rewind.sh | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/t/t5405-send-pack-rewind.sh b/t/t5405-send-pack-rewind.sh
index 4bda18a662..235fb7686a 100755
--- a/t/t5405-send-pack-rewind.sh
+++ b/t/t5405-send-pack-rewind.sh
@@ -25,8 +25,7 @@ test_expect_success 'non forced push should die not segfault' 
'
 
(
cd another &&
-   git push .. master:master
-   test $? = 1
+   test_must_fail git push .. master:master
)
 
 '
-- 
2.18.0.203.gfac676dfb9



git@vger.kernel.org

2018-07-01 Thread Eric Sunshine
Signed-off-by: Eric Sunshine 
---
 t/t4001-diff-rename.sh   |  2 +-
 t/t4024-diff-optimize-common.sh  | 16 
 t/t4025-hunk-header.sh   |  8 
 t/t4041-diff-submodule-option.sh |  4 ++--
 t/t4060-diff-submodule-option-diff-format.sh |  2 +-
 t/t4121-apply-diffs.sh   |  2 +-
 6 files changed, 17 insertions(+), 17 deletions(-)

diff --git a/t/t4001-diff-rename.sh b/t/t4001-diff-rename.sh
index bf4030371a..c16486a9d4 100755
--- a/t/t4001-diff-rename.sh
+++ b/t/t4001-diff-rename.sh
@@ -180,7 +180,7 @@ test_expect_success 'setup for many rename source 
candidates' '
git add "path??" &&
test_tick &&
git commit -m "hundred" &&
-   (cat path1; echo new) >new-path &&
+   (cat path1 && echo new) >new-path &&
echo old >>path1 &&
git add new-path path1 &&
git diff -l 4 -C -C --cached --name-status >actual 2>actual.err &&
diff --git a/t/t4024-diff-optimize-common.sh b/t/t4024-diff-optimize-common.sh
index 7e76018296..6b44ce1493 100755
--- a/t/t4024-diff-optimize-common.sh
+++ b/t/t4024-diff-optimize-common.sh
@@ -127,17 +127,17 @@ test_expect_success setup '
 
for n in $sample
do
-   ( zs $n ; echo a ) >file-a$n &&
-   ( echo b; zs $n; echo ) >file-b$n &&
-   ( printf c; zs $n ) >file-c$n &&
-   ( echo d; zs $n ) >file-d$n &&
+   ( zs $n && echo a ) >file-a$n &&
+   ( echo b && zs $n && echo ) >file-b$n &&
+   ( printf c && zs $n ) >file-c$n &&
+   ( echo d && zs $n ) >file-d$n &&
 
git add file-a$n file-b$n file-c$n file-d$n &&
 
-   ( zs $n ; echo A ) >file-a$n &&
-   ( echo B; zs $n; echo ) >file-b$n &&
-   ( printf C; zs $n ) >file-c$n &&
-   ( echo D; zs $n ) >file-d$n &&
+   ( zs $n && echo A ) >file-a$n &&
+   ( echo B && zs $n && echo ) >file-b$n &&
+   ( printf C && zs $n ) >file-c$n &&
+   ( echo D && zs $n ) >file-d$n &&
 
expect_pattern $n || return 1
 
diff --git a/t/t4025-hunk-header.sh b/t/t4025-hunk-header.sh
index 7a3dbc1ea2..fa44e78869 100755
--- a/t/t4025-hunk-header.sh
+++ b/t/t4025-hunk-header.sh
@@ -12,12 +12,12 @@ NS="$N$N$N$N$N$N$N$N$N$N$N$N$N"
 test_expect_success setup '
 
(
-   echo "A $NS"
+   echo "A $NS" &&
for c in B C D E F G H I J K
do
echo "  $c"
-   done
-   echo "L  $NS"
+   done &&
+   echo "L  $NS" &&
for c in M N O P Q R S T U V
do
echo "  $c"
@@ -34,7 +34,7 @@ test_expect_success 'hunk header truncation with an overly 
long line' '
 
git diff | sed -n -e "s/^.*@@//p" >actual &&
(
-   echo " A $N$N$N$N$N$N$N$N$N2"
+   echo " A $N$N$N$N$N$N$N$N$N2" &&
echo " L  $N$N$N$N$N$N$N$N$N1"
) >expected &&
test_cmp actual expected
diff --git a/t/t4041-diff-submodule-option.sh b/t/t4041-diff-submodule-option.sh
index 058ee0829d..4e3499ef84 100755
--- a/t/t4041-diff-submodule-option.sh
+++ b/t/t4041-diff-submodule-option.sh
@@ -498,7 +498,7 @@ test_expect_success 'given commit --submodule=short' '
 test_expect_success 'setup .git file for sm2' '
(cd sm2 &&
 REAL="$(pwd)/../.real" &&
-mv .git "$REAL"
+mv .git "$REAL" &&
 echo "gitdir: $REAL" >.git)
 '
 
@@ -527,7 +527,7 @@ test_expect_success 'diff --submodule with objects 
referenced by alternates' '
git commit -m "sub a"
) &&
(cd sub_alt &&
-   sha1_before=$(git rev-parse --short HEAD)
+   sha1_before=$(git rev-parse --short HEAD) &&
echo b >b &&
git add b &&
git commit -m b &&
diff --git a/t/t4060-diff-submodule-option-diff-format.sh 
b/t/t4060-diff-submodule-option-diff-format.sh
index 4b168d0ed7..0eba4620f0 100755
--- a/t/t4060-diff-submodule-option-diff-format.sh
+++ b/t/t4060-diff-submodule-option-diff-format.sh
@@ -721,7 +721,7 @@ test_expect_success 'given commit' '
 test_expect_success 'setup .git file for sm2' '
(cd sm2 &&
 REAL="$(pwd)/../.real" &&
-mv .git "$REAL"
+mv .git "$REAL" &&
 echo "gitdir: $REAL" >.git)
 '
 
diff --git a/t/t4121-apply-diffs.sh b/t/t4121-apply-diffs.sh
index aff551a1d7..66368effd5 100755
--- a/t/t4121-apply-diffs.sh
+++ b/t/t4121-apply-diffs.sh
@@ -27,6 +27,6 @@ test_expect_success 'setup' \
 
 test_expect_success \
'check if contextually independent diffs for the same file apply' \
-   '( git diff test~2 test~1; git diff test~1 test~0 )| git apply'
+   '( git diff test~2 test~1 && git diff test~1 test~0 )| git apply'
 
 test_done
-- 
2.18.0.203.gfac676dfb9


git@vger.kernel.org

2018-07-01 Thread Eric Sunshine
Signed-off-by: Eric Sunshine 
---
 t/t3030-merge-recursive.sh | 340 ++---
 1 file changed, 170 insertions(+), 170 deletions(-)

diff --git a/t/t3030-merge-recursive.sh b/t/t3030-merge-recursive.sh
index 3563e77b37..ff641b348a 100755
--- a/t/t3030-merge-recursive.sh
+++ b/t/t3030-merge-recursive.sh
@@ -36,15 +36,15 @@ test_expect_success 'setup 1' '
test_tick &&
git commit -m "master modifies a and d/e" &&
c1=$(git rev-parse --verify HEAD) &&
-   ( git ls-tree -r HEAD ; git ls-files -s ) >actual &&
+   ( git ls-tree -r HEAD && git ls-files -s ) >actual &&
(
-   echo "100644 blob $o1   a"
-   echo "100644 blob $o0   b"
-   echo "100644 blob $o0   c"
-   echo "100644 blob $o1   d/e"
-   echo "100644 $o1 0  a"
-   echo "100644 $o0 0  b"
-   echo "100644 $o0 0  c"
+   echo "100644 blob $o1   a" &&
+   echo "100644 blob $o0   b" &&
+   echo "100644 blob $o0   c" &&
+   echo "100644 blob $o1   d/e" &&
+   echo "100644 $o1 0  a" &&
+   echo "100644 $o0 0  b" &&
+   echo "100644 $o0 0  c" &&
echo "100644 $o1 0  d/e"
) >expected &&
test_cmp expected actual
@@ -54,15 +54,15 @@ test_expect_success 'setup 2' '
 
rm -rf [abcd] &&
git checkout side &&
-   ( git ls-tree -r HEAD ; git ls-files -s ) >actual &&
+   ( git ls-tree -r HEAD && git ls-files -s ) >actual &&
(
-   echo "100644 blob $o0   a"
-   echo "100644 blob $o0   b"
-   echo "100644 blob $o0   c"
-   echo "100644 blob $o0   d/e"
-   echo "100644 $o0 0  a"
-   echo "100644 $o0 0  b"
-   echo "100644 $o0 0  c"
+   echo "100644 blob $o0   a" &&
+   echo "100644 blob $o0   b" &&
+   echo "100644 blob $o0   c" &&
+   echo "100644 blob $o0   d/e" &&
+   echo "100644 $o0 0  a" &&
+   echo "100644 $o0 0  b" &&
+   echo "100644 $o0 0  c" &&
echo "100644 $o0 0  d/e"
) >expected &&
test_cmp expected actual &&
@@ -75,15 +75,15 @@ test_expect_success 'setup 2' '
test_tick &&
git commit -m "side modifies a" &&
c2=$(git rev-parse --verify HEAD) &&
-   ( git ls-tree -r HEAD ; git ls-files -s ) >actual &&
+   ( git ls-tree -r HEAD && git ls-files -s ) >actual &&
(
-   echo "100644 blob $o2   a"
-   echo "100644 blob $o0   b"
-   echo "100644 blob $o0   c"
-   echo "100644 blob $o0   d/e"
-   echo "100644 $o2 0  a"
-   echo "100644 $o0 0  b"
-   echo "100644 $o0 0  c"
+   echo "100644 blob $o2   a" &&
+   echo "100644 blob $o0   b" &&
+   echo "100644 blob $o0   c" &&
+   echo "100644 blob $o0   d/e" &&
+   echo "100644 $o2 0  a" &&
+   echo "100644 $o0 0  b" &&
+   echo "100644 $o0 0  c" &&
echo "100644 $o0 0  d/e"
) >expected &&
test_cmp expected actual
@@ -93,15 +93,15 @@ test_expect_success 'setup 3' '
 
rm -rf [abcd] &&
git checkout df-1 &&
-   ( git ls-tree -r HEAD ; git ls-files -s ) >actual &&
+   ( git ls-tree -r HEAD && git ls-files -s ) >actual &&
(
-   echo "100644 blob $o0   a"
-   echo "100644 blob $o0   b"
-   echo "100644 blob $o0   c"
-   echo "100644 blob $o0   d/e"
-   echo "100644 $o0 0  a"
-   echo "100644 $o0 0  b"
-   echo "100644 $o0 0  c"
+   echo "100644 blob $o0   a" &&
+   echo "100644 blob $o0   b" &&
+   echo "100644 blob $o0   c" &&
+   echo "100644 blob $o0   d/e" &&
+   echo "100644 $o0 0  a" &&
+   echo "100644 $o0 0  b" &&
+   echo "100644 $o0 0  c" &&
echo "100644 $o0 0  d/e"
) >expected &&
test_cmp expected actual &&
@@ -112,15 +112,15 @@ test_expect_success 'setup 3' '
test_tick &&
git commit -m "df-1 makes b/c" &&
c3=$(git rev-parse --verify HEAD) &&
-   ( git ls-tree -r HEAD ; git ls-files -s ) >actual &&
+   ( git ls-tree -r HEAD && git ls-files -s ) >actual &&
(
-   echo "100644 blob $o0   a"
-   echo "100644 blob $o3   b/c"
-   echo "100644 blob $o0   c"
-   echo "100644 blob $o0   d/e"
-   echo "100644 $o0 0  a"
-   echo "100644 $o3 0  b/c"
-   echo "100644 $o0 0  c"
+   echo "100644 blob $o0   a" &&
+   echo "

[PATCH 14/25] t9814: simplify convoluted check that command correctly errors out

2018-07-01 Thread Eric Sunshine
This test uses a convoluted method to verify that "p4 help" errors
out when asked for help about an unknown command. In doing so, it
intentionally breaks the &&-chain. Simplify by employing the typical
"! command" idiom and a normal &&-chain instead.

Signed-off-by: Eric Sunshine 
---
 t/t9814-git-p4-rename.sh | 18 +++---
 1 file changed, 3 insertions(+), 15 deletions(-)

diff --git a/t/t9814-git-p4-rename.sh b/t/t9814-git-p4-rename.sh
index e7e0268e98..60baa06e27 100755
--- a/t/t9814-git-p4-rename.sh
+++ b/t/t9814-git-p4-rename.sh
@@ -9,23 +9,11 @@ test_expect_success 'start p4d' '
 '
 
 # We rely on this behavior to detect for p4 move availability.
-test_expect_success 'p4 help unknown returns 1' '
+test_expect_success '"p4 help unknown" errors out' '
(
cd "$cli" &&
-   (
-   p4 help client >errs 2>&1
-   echo $? >retval
-   )
-   echo 0 >expected &&
-   test_cmp expected retval &&
-   rm retval &&
-   (
-   p4 help nosuchcommand >errs 2>&1
-   echo $? >retval
-   )
-   echo 1 >expected &&
-   test_cmp expected retval &&
-   rm retval
+   p4 help client &&
+   ! p4 help nosuchcommand
)
 '
 
-- 
2.18.0.203.gfac676dfb9



git@vger.kernel.org

2018-07-01 Thread Eric Sunshine
Signed-off-by: Eric Sunshine 
---
 t/t3000-ls-files-others.sh  | 2 +-
 t/t3006-ls-files-long.sh| 2 +-
 t/t3008-ls-files-lazy-init-name-hash.sh | 8 
 t/t3050-subprojects-fetch.sh| 8 
 t/t3210-pack-refs.sh| 4 ++--
 t/t3301-notes.sh| 8 
 t/t3400-rebase.sh   | 8 
 t/t3402-rebase-merge.sh | 4 ++--
 t/t3418-rebase-continue.sh  | 4 ++--
 t/t3700-add.sh  | 8 
 10 files changed, 28 insertions(+), 28 deletions(-)

diff --git a/t/t3000-ls-files-others.sh b/t/t3000-ls-files-others.sh
index c525656b2c..afd4756134 100755
--- a/t/t3000-ls-files-others.sh
+++ b/t/t3000-ls-files-others.sh
@@ -84,7 +84,7 @@ test_expect_success SYMLINKS 'ls-files --others with 
symlinked submodule' '
) &&
(
cd super &&
-   "$SHELL_PATH" 
"$TEST_DIRECTORY/../contrib/workdir/git-new-workdir" ../sub sub
+   "$SHELL_PATH" 
"$TEST_DIRECTORY/../contrib/workdir/git-new-workdir" ../sub sub &&
git ls-files --others --exclude-standard >../actual
) &&
echo sub/ >expect &&
diff --git a/t/t3006-ls-files-long.sh b/t/t3006-ls-files-long.sh
index 202ad658b8..e109c3fbfb 100755
--- a/t/t3006-ls-files-long.sh
+++ b/t/t3006-ls-files-long.sh
@@ -29,7 +29,7 @@ test_expect_success 'overly-long path does not replace 
another by mistake' '
printf "$pat" "$blob_a" "$path_a" "$blob_z" "$path_z" |
git update-index --add --index-info &&
(
-   echo "$path_a"
+   echo "$path_a" &&
echo "$path_z"
) >expect &&
git ls-files >actual &&
diff --git a/t/t3008-ls-files-lazy-init-name-hash.sh 
b/t/t3008-ls-files-lazy-init-name-hash.sh
index 08af596ba6..64f047332b 100755
--- a/t/t3008-ls-files-lazy-init-name-hash.sh
+++ b/t/t3008-ls-files-lazy-init-name-hash.sh
@@ -14,10 +14,10 @@ LAZY_THREAD_COST=2000
 
 test_expect_success 'no buffer overflow in lazy_init_name_hash' '
(
-   test_seq $LAZY_THREAD_COST | sed "s/^/a_/"
-   echo b/b/b
-   test_seq $LAZY_THREAD_COST | sed "s/^/c_/"
-   test_seq 50 | sed "s/^/d_/" | tr "\n" "/"; echo d
+   test_seq $LAZY_THREAD_COST | sed "s/^/a_/" &&
+   echo b/b/b &&
+   test_seq $LAZY_THREAD_COST | sed "s/^/c_/" &&
+   test_seq 50 | sed "s/^/d_/" | tr "\n" "/" && echo d
) |
sed "s/^/100644 $EMPTY_BLOB /" |
git update-index --index-info &&
diff --git a/t/t3050-subprojects-fetch.sh b/t/t3050-subprojects-fetch.sh
index 2f5f41a012..f1f09abdd9 100755
--- a/t/t3050-subprojects-fetch.sh
+++ b/t/t3050-subprojects-fetch.sh
@@ -21,10 +21,10 @@ test_expect_success setup '
 
 test_expect_success clone '
git clone "file://$(pwd)/.git" cloned &&
-   (git rev-parse HEAD; git ls-files -s) >expected &&
+   (git rev-parse HEAD && git ls-files -s) >expected &&
(
cd cloned &&
-   (git rev-parse HEAD; git ls-files -s) >../actual
+   (git rev-parse HEAD && git ls-files -s) >../actual
) &&
test_cmp expected actual
 '
@@ -40,11 +40,11 @@ test_expect_success advance '
 '
 
 test_expect_success fetch '
-   (git rev-parse HEAD; git ls-files -s) >expected &&
+   (git rev-parse HEAD && git ls-files -s) >expected &&
(
cd cloned &&
git pull &&
-   (git rev-parse HEAD; git ls-files -s) >../actual
+   (git rev-parse HEAD && git ls-files -s) >../actual
) &&
test_cmp expected actual
 '
diff --git a/t/t3210-pack-refs.sh b/t/t3210-pack-refs.sh
index afa27ffe2d..724b4b9bc0 100755
--- a/t/t3210-pack-refs.sh
+++ b/t/t3210-pack-refs.sh
@@ -231,9 +231,9 @@ test_expect_success 'timeout if packed-refs.lock exists' '
 test_expect_success 'retry acquiring packed-refs.lock' '
LOCK=.git/packed-refs.lock &&
>"$LOCK" &&
-   test_when_finished "wait; rm -f $LOCK" &&
+   test_when_finished "wait && rm -f $LOCK" &&
{
-   ( sleep 1 ; rm -f $LOCK ) &
+   ( sleep 1 && rm -f $LOCK ) &
} &&
git -c core.packedrefstimeout=3000 pack-refs --all --prune
 '
diff --git a/t/t3301-notes.sh b/t/t3301-notes.sh
index 2d200fdf36..ac62dc0e8f 100755
--- a/t/t3301-notes.sh
+++ b/t/t3301-notes.sh
@@ -914,7 +914,7 @@ test_expect_success 'git notes copy --stdin' '
${indent}
${indent}yet another note
EOF
-   (echo $(git rev-parse HEAD~3) $(git rev-parse HEAD^); \
+   (echo $(git rev-parse HEAD~3) $(git rev-parse HEAD^) &&
echo $(git rev-parse HEAD~2) $(git rev-parse HEAD)) |
git notes copy --stdin &&
git log -2 >actual &&
@@ -939,7 +939,7 @@ test_expect_success 'git notes copy --for-rewrite 
(unconfigured)' '
EOF
test_commit 14th &&
test_com

Re: send-email: change the default value of sendmail.validate

2018-07-01 Thread brian m. carlson
On Sun, Jul 01, 2018 at 08:17:53PM -0400, Drew DeVault wrote:
> On 2018-07-01  6:15 PM, brian m. carlson wrote:
> > Are you suggesting that we not limit lines to 998 octets?  I've seen
> > lots of mail servers that do reject mail over 998 octets.  I've
> > configured Postfix to do so because being strict on mail standards is a
> > great way to stop spam.
> 
> Yes, that's what I'm suggesting. We should error out later when the SMTP
> server rejects the mail.

I don't think it's a good idea to continue when we know we're going to
send invalid data.  If you really want to send the email and you know
it's safe in your environment, use --no-validate.

> Also, Extended SMTP is a standard. RFC 1869.

ESMTP doesn't lift the 998-octet limit.  RFC 5321 specifies ESMTP and is
silent about line length.  RFC 5322, which defines the email message
format, limits lines in an email message to 998 octets.  The limit is in
the email format, rather than the ESMTP protocol.

> > If that's the issue you're seeing, it might be better to either
> > automatically encode those patches as binary patches or teach git
> > send-email and git am how to automatically handle quoted-printable.
> 
> I'm really not fond of quoted-printable. Extended SMTP has been
> standardized for over 20 years. Assuming people don't blithly disable it
> on their servers, we can expect it to be present on almost all mail
> servers.

I'm not bothered if we don't support pre-ESMTP servers.  I do care
whether we produce properly formatted email messages.  Long lines
require wrapping, and that means either base64 or quoted-printable.  For
plain text data, quoted-printable is less likely to be filtered as spam
than base64.  It's also easier to read with plain text tools such as
less.

I'd be willing to implement quoted-printable formatting at some point if
that's the direction we want to go.

> I don't think I've ever seen a spam email that would have been stopped
> because it contained 998 lines. Approaches like SpamAssassin,
> greylisting, DNSBLs, SPF/DKIM, these are much more effective.

It is actually extremely effective.  Most spam-ware produces invalid
messages, and IME almost all malformed messages are spam.  I use a
variety of techniques, including this one and most of the others you've
mentioned.  Other people do so as well.

Regardless, a default mail server configuration on Debian rejects overly
long lines, and there are various other systems that do so as well, so
"just send it" isn't a viable solution.  Fixing Git so it produces valid
data in this case seems more robust.
-- 
brian m. carlson: Houston, Texas, US
OpenPGP: https://keybase.io/bk2204


signature.asc
Description: PGP signature


Re: send-email: change the default value of sendmail.validate

2018-07-01 Thread Drew DeVault
I seem to be mistaken about the degree to which this is standardized and
supported. The Debian argument cinches it for me. Quoted printable is
probably the right way to go, then.