[PATCH] t7610 test for mktemp existence
Subject: t7610: test for mktemp before test execution mktemp is not available on all platforms, so the test 'temporary filenames are used with mergetool.writeToTemp' fails there. This patch does not replace mktemp but just disables the test that otherwise would fail. mergetool checks itself before executing mktemp and reports an error. Signed-off-by: Armin Kunaschik --- diff --git a/t/t7610-mergetool.sh b/t/t7610-mergetool.sh index 76306cf..9279bf5 100755 --- a/t/t7610-mergetool.sh +++ b/t/t7610-mergetool.sh @@ -589,7 +589,12 @@ test_expect_success 'filenames seen by tools start with ./' ' git reset --hard master >/dev/null 2>&1 ' -test_expect_success 'temporary filenames are used with mergetool.writeToTemp' ' +test_lazy_prereq MKTEMP ' + tempdir=$(mktemp -d -t foo.XXX) && + test -d "$tempdir" +' + +test_expect_success MKTEMP 'temporary filenames are used with mergetool.writeToTemp' ' git checkout -b test16 branch1 && test_config mergetool.writeToTemp true && test_config mergetool.myecho.cmd "echo \"\$LOCAL\"" && -- 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: t7610-mergetool.sh test failure
Another thread I'm trying to revive... the discussion went away quite a bit from the suggested patch to conditionally run this one test only when mktemp is available. I'll create a patch when there are chances it is accepted. I could think of a way to replace mktemp with a perl one-liner (or small script)... conditionally, when mktemp is not available... maybe in the build process? As far as I can see, perl is absolutely necessary and can therefore be used to "solve" the mktemp problem... ...or maybe I should stop bringing this up again :-) Armin -- 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] t7800 readlink not found
On Tue, May 31, 2016 at 7:51 AM, Junio C Hamano wrote: > Torsten Bögershausen writes: > >>> diff --git a/t/t7800-difftool.sh b/t/t7800-difftool.sh >>> index 7ce4cd7..905035c 100755 >>> --- a/t/t7800-difftool.sh >>> +++ b/t/t7800-difftool.sh >>> @@ -446,7 +446,7 @@ write_script .git/CHECK_SYMLINKS <<\EOF >>> for f in file file2 sub/sub >>> do >>> echo "$f" >>> -readlink "$2/$f" >>> +ls -ld "$2/$f" | sed -e 's/.* -> //' >>> done >actual >>> EOF >>> >> I don't know how portable #ls -ld" really is. > > The parts with mode bits, nlinks, uid, gid, size, and date part do > have some variations. For example, we have been burned on ACL > enabled systems having some funny suffix after the usual mode bits > stuff. > > However, as far as this test is concerned, I do not think "how > portable is the output from ls -ld" is an especially relevant > question. None of the things we expect early in the output (the > fields I enumerated in the previous paragraph) would contain " -> ". > And we know that we do not use a filename that has " -> " (or "->") > as a substring in our tests. > > We don't have to use readlink, even on platforms where we do have > readlink. Building the conditional to be checked at runtime and > providing a shell function read_link that uses "ls -ld | sed" or > "readlink" depending on the runtime check is wasteful. > Just a short, curious question: Is this patch to be accepted/included some time? I didn't see it in 2.8.4 nor 2.9.0. Maybe it just fell off the table... Armin -- 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] t7800 readlink not found
On 05/27/2016 06:19 AM, David Aguilar wrote: > On Wed, May 25, 2016 at 11:33:33AM +0200, Armin Kunaschik wrote: > > Would you mind submitting a patch so that we can support these > tests when running on AIX/HP-UX? I don't feel comfortable to submit patches for tests I can't verify. I don't have valgrind and python/p4 here. Looking to the code I'd say, patching the p4 tests with "ls -ld | sed" looks quite save. But I'm not sure about the test-lib.sh. When you are really super paranoid, as written in the comment, you should probably use perl like perl -e 'print readlink $ARGV[0]' $name as a replacement. So, as suggested by Junio, here the readlink workaround for t7800 only. (hopefully whitespace clean this time) --- 8< --- 8< --- From: Armin Kunaschik Subject: t7800: readlink is not portable The readlink(1) command is not available on all platforms (notably not on AIX and HP-UX) and can be replaced in this test with the "workaround" ls -ld | sed -e 's/.* -> //' This is no universal readlink replacement but works in the controlled test environment good enough. Signed-off-by: Armin Kunaschik --- diff --git a/t/t7800-difftool.sh b/t/t7800-difftool.sh index 7ce4cd7..905035c 100755 --- a/t/t7800-difftool.sh +++ b/t/t7800-difftool.sh @@ -446,7 +446,7 @@ write_script .git/CHECK_SYMLINKS <<\EOF for f in file file2 sub/sub do echo "$f" - readlink "$2/$f" + ls -ld "$2/$f" | sed -e 's/.* -> //' done >actual EOF -- 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: t7800 test failure
On Tue, May 24, 2016 at 7:36 PM, Junio C Hamano wrote: > Armin Kunaschik writes: >> >> Ok, how can this be implemented within the test environment? > > I actually think an unconditional check like this is sufficient. Ah, good. My thoughts were a bit more complicated. Anyway, this works for me. Thanks! > t/t7800-difftool.sh | 17 + > 1 file changed, 9 insertions(+), 8 deletions(-) > > diff --git a/t/t7800-difftool.sh b/t/t7800-difftool.sh > index 7ce4cd7..f304228 100755 > --- a/t/t7800-difftool.sh > +++ b/t/t7800-difftool.sh > @@ -442,15 +442,16 @@ run_dir_diff_test 'difftool --dir-diff with unmerged > files' ' > test_cmp expect actual > ' > > -write_script .git/CHECK_SYMLINKS <<\EOF > -for f in file file2 sub/sub > -do > - echo "$f" > - readlink "$2/$f" > -done >actual > -EOF > - > test_expect_success PERL,SYMLINKS 'difftool --dir-diff --symlink without > unstaged changes' ' > + > + write_script .git/CHECK_SYMLINKS <<-\EOF && > + for f in file file2 sub/sub > + do > + echo "$f" > + ls -ld "$2/$f" | sed -e "s/.* -> //" > + done >actual > + EOF > + > cat >expect <<-EOF && > file > $PWD/file -- 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: t7800 test failure
On Tue, May 24, 2016 at 6:57 PM, Junio C Hamano wrote: > Matthieu Moy writes: > >> Armin Kunaschik writes: >> >>> t7800 fails on systems where readlink (GNUism?) is not available. >> >> I don't think it's POSIX, but it is present on all POSIX-like systems I >> know. On which system did you get the issue? It's not available in AIX or HP-UX. >>> +readlink() { ls -ld "$1" | sed 's/.* -> //'; } >> >> This is much less robust than the actual readlink. For example, if -> >> appears in the link name, it breaks. > > I wouldn't allow it in our scripted Porcelain, but the environment > of our test scripts are under our control, so I do not think it is a > problem ("ls piped to sed" has been an established idiom before > readlink(1) was widely accepted, by the way). I think so too. Maybe I can improve the sed expression a bit, but it will never be a universal readlink replacement. But it doesn't have to. It's defined locally for this one test only and it does the specific job. >> It would be acceptable as a fall-back if readlink is not present, but >> shouldn't activate the "ls" hack by default. > > Yup. Ok, how can this be implemented within the test environment? -- 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: t7610-mergetool.sh test failure
On Tue, May 24, 2016 at 6:45 PM, Junio C Hamano wrote: > 3. find and install mktemp? Sure, but which one? :-) mktemp is not mentioned in POSIX. http://stackoverflow.com/questions/2792675/how-portable-is-mktemp1 -- 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
t7610-mergetool.sh test failure
t7610-mergetool.sh fails on systems without mktemp. mktemp is used in git-mergetool.sh and throws an error when it's not available. error: mktemp is needed when 'mergetool.writeToTemp' is true I see 2 options: 1. code around it, write an own mktemp, maybe use the test-mktemp as a basis. 2. disable the test when mktemp is not available >From my point of view option 2 would be enough. Any opinions about that? Peff suggested something like this... works for me. This patch is probably whitespace damaged... I could not yet figure out a way to use and preserve tabs with Google mail. --- diff --git a/t/t7610-mergetool.sh b/t/t7610-mergetool.sh index 76306cf..9279bf5 100755 --- a/t/t7610-mergetool.sh +++ b/t/t7610-mergetool.sh @@ -589,7 +589,12 @@ test_expect_success 'filenames seen by tools start with ./' ' git reset --hard master >/dev/null 2>&1 ' -test_expect_success 'temporary filenames are used with mergetool.writeToTemp' ' +test_lazy_prereq MKTEMP ' + tempdir=$(mktemp -d -t foo.XXX) && + test -d "$tempdir" +' + +test_expect_success MKTEMP 'temporary filenames are used with mergetool.writeToTemp' ' git checkout -b test16 branch1 && test_config mergetool.writeToTemp true && test_config mergetool.myecho.cmd "echo \"\$LOCAL\"" && -- 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
t7800 test failure
t7800 fails on systems where readlink (GNUism?) is not available. An easy workaround for the very basic readlink usage of this test would be to use a shell function like this: --- diff --git a/t/t7800-difftool.sh b/t/t7800-difftool.sh index ff7a9e9..be3d19f 100755 --- a/t/t7800-difftool.sh +++ b/t/t7800-difftool.sh @@ -420,6 +420,7 @@ run_dir_diff_test 'difftool --dir-diff when worktree file is missing' ' ' write_script .git/CHECK_SYMLINKS <<\EOF +readlink() { ls -ld "$1" | sed 's/.* -> //'; } for f in file file2 sub/sub do echo "$f" -- 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
t1400 test failures
Hello, I have 2 failed tests here: large transaction creating branches does not burst open file limit large transaction deleting branches does not burst open file limit both tests fail because of the decreased file limit: git-2.8.3/t/../bin-wrappers/git: bad file unit number The tests run fine on AIX with ksh88 when I increase the ulimit to 63 or higher. bash and ksh93 test fine with the present ulimit of 32. Changing the shell interpreter of the git-wrapper script to /bin/bash or /bin/ksh93 is enough to make these 2 tests work with the lower ulimit. All other scripts can be run with any shell. I have no idea why the ksh88 consumes so much file descriptors, I also don't have an idea if or how this test can be "fixed". So I'll leave this here for information purposes :-) Armin -- 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: t4204-patch-id failures
On Tue, May 24, 2016 at 12:23 AM, Junio C Hamano wrote: > Junio C Hamano writes: > Having said all that, this illustrates the root cause of different > behaviours better, but it is harder to reason about than simply > changing the variable name used in this shell function. POSIX reads > a bit fuzzy to me around here: > > ... each command of a multi-command pipeline is in a subshell > environment; as an extension, however, any or all commands in a > pipeline may be executed in the current environment. All other > commands shall be executed in the current shell environment. > > That essentially says nothing useful; it does not guarantee that > each command on a pipeline runs in its own subshell environment, and > a portable script must be prepared to see some of them run in the > current shell environment. Writing portable shell scripts sometimes is quite a mess :-) > So let's do this instead: > > -- >8 -- > Subject: t4204: do not let $name variable clobbered > > test_patch_id_file_order shell function uses $name variable to hold > one filename, and calls another shell function calc_patch_id as a > downstream of one pipeline. The called function, however, also uses > the same $name variable. With a shell implementation that runs the > callee in the current shell environment, the caller's $name would > be clobbered by the callee's use of the same variable. > > This hasn't been an issue with dash and bash. ksh93 reveals the > breakage in the test script. Maybe add ksh88 too, just to be "feature" complete. > Fix it by using a distinct variable name in the callee. Agreed. > Reported-by: Armin Kunaschik > Signed-off-by: Junio C Hamano -- 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
t4204-patch-id failures
Hello, I see 3 test failures in t4202: expecting success: test_patch_id_file_order irrelevant --stable --stable Already on 'same' cmp: cannot open patch-id_ordered-ordered-order---stable-irrelevant not ok 7 - file order is irrelevant with --stable # # test_patch_id_file_order irrelevant --stable --stable # expecting success: test_patch_id_file_order relevant --unstable --unstable Already on 'same' cmp: cannot open patch-id_ordered-ordered-order---unstable-relevant [..] expecting success: test_config patchid.stable true && test_patch_id irrelevant patchid.stable=true Already on 'same' cmp: cannot open patch-id_ordered-ordered-order-patchid.stable=true-irrelevant not ok 10 - patchid.stable = true is stable # # test_config patchid.stable true && # test_patch_id irrelevant patchid.stable=true # [..] expecting success: test_config patchid.stable false && test_patch_id irrelevant patchid.stable=false--stable --stable Already on 'same' cmp: cannot open patch-id_ordered-ordered-order-patchid.stable=false--stable-irrelevant not ok 13 - --stable overrides patchid.stable = false # # test_config patchid.stable false && # test_patch_id irrelevant patchid.stable=false--stable --stable # Please notice the double "ordered"! >From my point of view there is a problem in the function calc_patch_id() which changes the variable $name and causes the test to fail. Essentially it's working like this: #!/bin/bash func1() { name=${1} echo "func1 name=$name" } func2() { name=${1} echo "func2 name=$name" func1 "ordered-$name" echo "func2 again name=$name" } func2 foo which prints in bash, ksh88 and ksh93 on Linux and AIX the same func2 name=foo func1 name=ordered-foo func2 again name=ordered-foo I wonder if those 3 tests ever worked... and why? :-) A suggested patch would be to rename $name inside calc_patch_id into something else except $name... e.g. $pname. Armin -- 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] t0008: 4 tests fail with ksh88
On Fri, May 20, 2016 at 6:10 PM, Junio C Hamano wrote: > Junio C Hamano writes: > >>> diff --git a/t/t0008-ignores.sh b/t/t0008-ignores.sh >>> index 89544dd..b425f3a 100755 >>> --- a/t/t0008-ignores.sh >>> +++ b/t/t0008-ignores.sh >>> @@ -605,7 +605,7 @@ cat <<-EOF >expected-verbose >>> a/b/.gitignore:8:!on* a/b/one >>> a/b/.gitignore:8:!on* a/b/one one > > The patch was whitespace-damaged and didn't apply, so I had to redo > it from scratch while updating the log message. If this looks good > to you, there is no need to resend. Thanks. Looks like even Google Mail interface in plain text mode eats white spaces :-( -- 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] t0008: 4 tests fail with ksh88
On Fri, May 20, 2016 at 5:16 PM, Junio C Hamano wrote: > Armin Kunaschik writes: > >> From: Armin Kunaschik >> >> \" in the test t0008 is not treated the same way in bash and in ksh. > > Could you refrain from singling out "bash"? We don't write for > "bash" specifically (and the test I ran are with "dash" before I > push things out). I can name it "other shells" if this is more comfortable. But I tested this only with bash and ksh88 on AIX. > Ideally, if you can try ksh93 and if you find out that ksh93 works, > then the above can be made in line with your "Subject" to mark ksh88 > as broken (as opposed to other POSIX shells)? That would help us by > reminding that running test fine with ksh93 is not a sufficient > check to make sure we didn't break ksh88 users. > >> In ksh the \ disappears and generates false expect data to >> compare with. >> Using \\" works portable, the same way in bash and in ksh and >> is less ambigous. > > All of the above would need s/ksh/&88/g; I'd think. I just tried > > make SHELL_PATH=/bin/ksh93 > cd t && /bin/ksh93 t0008-*.sh > > and this patch is not necessary for ksh93. Yes, the patch is not necessary with ksh93 on AIX, but it works :-) The patch is targeting "ksh" on AIX (which actually is a ksh88). In the discussion Jeff took a look into the POSIX specification and described the behavior like this: I think either is reasonable (there is no need to backslash-escape a double-quote inside a here-doc, but one assumes that backslash would generally have its usual behavior). I'm not quite sure how to interpret POSIX here (see below), but it seems clear that spelling it with two backslashes as you suggest is the best bet. I'd not declare ksh88 on AIX broken just because of this ambiguity since it is not 100% clear in the POSIX description. Armin -- 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] t0008: 4 tests fail with ksh88
From: Armin Kunaschik \" in the test t0008 is not treated the same way in bash and in ksh. In ksh the \ disappears and generates false expect data to compare with. Using \\" works portable, the same way in bash and in ksh and is less ambigous. Acked-by: Jeff King Signed-off-by: Armin Kunaschik --- diff --git a/t/t0008-ignores.sh b/t/t0008-ignores.sh index 89544dd..b425f3a 100755 --- a/t/t0008-ignores.sh +++ b/t/t0008-ignores.sh @@ -605,7 +605,7 @@ cat <<-EOF >expected-verbose a/b/.gitignore:8:!on* a/b/one a/b/.gitignore:8:!on* a/b/one one a/b/.gitignore:8:!on* a/b/one two - a/b/.gitignore:8:!on* "a/b/one\"three" + a/b/.gitignore:8:!on* "a/b/one\\"three" a/b/.gitignore:9:!two a/b/two a/.gitignore:1:two* a/b/twooo $global_excludes:2:!globaltwo globaltwo @@ -686,7 +686,7 @@ cat <<-EOF >expected-all a/b/.gitignore:8:!on* b/one a/b/.gitignore:8:!on* b/one one a/b/.gitignore:8:!on* b/one two - a/b/.gitignore:8:!on* "b/one\"three" + a/b/.gitignore:8:!on* "b/one\\"three" a/b/.gitignore:9:!two b/two :: b/not-ignored a/.gitignore:1:two* b/twooo -- 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
t3404 static check of bad SHA-1 failure
Hello, in t3404 test 91 - static check of bad SHA-1 fails (with ksh) with a syntax error in git-rebase. git-rebase[6]: test: argument expected Reason is, again, a test -z without quotes in git-rebase--interactive: *** ../git-rebase--interactive.orig Tue May 10 17:36:59 2016 --- ../git-rebase--interactive Fri May 13 17:57:27 2016 *** *** 870,876 badsha=1 else sha1_verif="$(git rev-parse --verify --quiet $1^{commit})" ! if test -z $sha1_verif then badsha=1 fi --- 870,876 badsha=1 else sha1_verif="$(git rev-parse --verify --quiet $1^{commit})" ! if test -z "$sha1_verif" then badsha=1 fi Maybe it would be a good idea, to quote the test -z $1 in the same function check_commit_sha too. The test completes successfully without it, but since it's an user option, I think it should be quoted. Then there would be no more unquoted test -z anymore :-) Armin -- 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
t0008 test fails with ksh
Hello, in t0008 I see tests fails with not ok 374 - --stdin -v # # expect_from_stdin expected-all .gitignore:1:one../one :: ../not-ignored .gitignore:1:oneone :: not-ignored a/b/.gitignore:8:!on* b/on a/b/.gitignore:8:!on* b/one a/b/.gitignore:8:!on* b/one one a/b/.gitignore:8:!on* b/one two a/b/.gitignore:8:!on* "b/one\"three" a/b/.gitignore:9:!two b/two :: b/not-ignored a/.gitignore:1:two* b/twooo $global_excludes:2:!globaltwo ../globaltwo $global_excludes:2:!globaltwo globaltwo $global_excludes:2:!globaltwo b/globaltwo $global_excludes:2:!globaltwo ../b/globaltwo :: c/not-ignored EOF behaves differently in bash and in ksh. a/b/.gitignore:8:!on* "b/one\"three" comes out unmodified with bash but with ksh it becomes a/b/.gitignore:8:!on* "b/one"three" I'm not sure what shell is "wrong" here, but when I modify the line to a/b/.gitignore:8:!on* "b/one\\"three" both shells generate the "right" output: a/b/.gitignore:8:!on* "b/one\"three" >From what I've learned I'd expect the double backslash to be the "right" way to escape one backslash. Do you agree or am I wrong? This snippet appears twice in this test, generates expected-all and expected-verbose. Armin -- 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
syntax error in git-rebase while running t34* tests
Hello, I noticed in my environment (AIX, ksh) that all rebase tests don't work. git-rebase terminates with git-rebase[6]: Syntax error at line 3 : `newline or ;' is not expected. Digging deeper I found the problem in git-rebase--interactive line 85 strategy_args=${strategy:+--strategy=$strategy} eval ' for strategy_opt in '"$strategy_opts"' do strategy_args="$strategy_args -X$(git rev-parse --sq-quote "${strategy_opt#--}")" done ' This snippet fails when $strategy_opts is empty or undefined... and my eval seems not to like this. The for loop runs fine, even with empty strategy_opts, but not inside eval. I fail to see why eval is really necessary here. Things can probably be done without eval, but I don't feel to see the big picture around this code. My quick and dirty hotfix is to place a test -n "$strategy_opts" && in front of the eval. The tests run fine after this change. What do you think? Armin -- 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: t4151 missing quotes
Sorry for any duplicate mails, the list blocked my html mail. Note to self: Don't use GMail on a tablet. On Mon, May 9, 2016 at 11:35 PM, Eric Sunshine wrote: >> >> Hmph, do we have a broken &&-chain? > > I don't know. Unfortunately, Armin didn't provide much information in > his initial email, saying only "skipping through some failed tests", > which doesn't necessarily indicate if those tests failed or if he > somehow manually skipped them. In t4151 there was only a problem with this test. All other tests inside t4151 were ok. Skipping through the tests referred to all git tests, not just t4151. >> If an earlier test fails and leaves an unmerged path, "ls-files -u" >> would give some output, so "test -z" would get one or more non-empty >> strings; if we feed multiple, this would fail. But we would not have >> even run "test -z" as long as we properly &&-chain these tests. >> >> I think the real issue is when the earlier step succeeds and does >> not leave any unmerged path. In that case, we would run "test -z" >> without anything else on the command line, which would lead to an >> syntax error. Yes. While debugging the test, I saw a syntax error. I did not try to find out why the test argument is empty. It seems not necessary.. the test logic is still the same. >> Side Note: /usr/bin/test and test (built into bash and dash) >> seem not to care about the lack of string in "test -z " >> and "test -n ". It appears to me that they just take >> "-z" and "-n" without "" as a special case of "test >> " that is fed "-z" or "-n" as . Apparently, the >> platform Armin is working on doesn't. > > I also tested on Mac OS X and BSD, and they happily accept bare "test > -n", as well (though, I don't doubt that there are old shells which > complain). I'm building on a quite current AIX 6.1 where /bin/sh defaults to /bin/ksh which is a posix shell (ksh88). Using /bin/bash doesn't work because SHELL_PATH is only used in git scripts but not in any t* test scripts. -- 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: t4151 missing quotes
I'm currently concentrating on finding problems with my setup... this is already a tough job :-) I'm a git beginner, and Documentation/SubmittingPatches would keep me busy for a week. So anybody feel free to submit this thingy. Armin -- 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] t6041: do not compress backup tar file
Hi Stefan, I'm currently in the process of skipping through the failed tests on my AIX box. There are more tests which require GNU tools like mktemp (t7610-mergetool.sh) or readlink (t7800-difftool.sh). But I don't have a solution or workaround for these two. But at least there are not more failing compressed tar tests :-) Thanks, Armin On Mon, May 9, 2016 at 7:09 PM, Stefan Beller wrote: > The test uses the 'z' option, i.e. "compress the output while at > it", which is GNUism and not portable. > > Reported-by: Armin Kunaschik > Signed-off-by: Stefan Beller > --- > > Thanks Armin for reporting these GNUism! > Are there any more? (So we can do these patches as a > series instead of one by one:) > > developed on top of origin/sb/z-is-gnutar-ism > > Thanks, > Stefan -- 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
t5601-clone.sh failures
Hi list, eight t5601 tests don't run with my version of sed. The reason is a trailing space in the sed expression. See below: #IPv6 for tuah in ::1 [::1] [::1]: user@::1 user@[::1] user@[::1]: [user@::1] [user@::1]: do ehost=$(echo $tuah | sed -e "s/1]:/1]/ "| tr -d "[]") test_expect_success "clone ssh://$tuah/home/user/repo" " test_clone_url ssh://$tuah/home/user/repo $ehost /home/user/repo " done I can not imagine a reason why there should be a space character... The tests run fine here without the space. When there isn't any reason, the trailing space should be removed. Armin -- 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
t6041-bisect-submodule.sh tar -z not portable
Hi, similar to t3513, in t6041 tar is used with the -z flag which is not portable and should be removed the same way as in t3513. Regards, Armin -- 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: t4151 missing quotes
Sorry, this was my first patch to the list. I'll do better :-) You are right about the "wc -l" parts. Maybe I was a bit over pessimistic. Throw away my last mail. In my case test 9 ran unsuccessful because of an empty "git ls-files -u" This reduces the diff to this one (hopefully the right way now): *** ./t4151-am-abort.sh.origFri Apr 29 23:37:00 2016 --- ./t4151-am-abort.sh Mon May 9 18:28:18 2016 *** *** 82,88 test 4 = "$(cat otherfile-4)" && git am --abort && test_cmp_rev initial HEAD && ! test -z $(git ls-files -u) && test_path_is_missing otherfile-4 ' --- 82,88 test 4 = "$(cat otherfile-4)" && git am --abort && test_cmp_rev initial HEAD && ! test -z "$(git ls-files -u)" && test_path_is_missing otherfile-4 ' All the other similar occurrences are correctly quoted. On Mon, May 9, 2016 at 6:22 PM, Eric Sunshine wrote: > On Mon, May 9, 2016 at 12:09 PM, Armin Kunaschik > wrote: >> skipping through some failed tests I found more (smaller) problems >> inside the test... when test arguments are empty they need to be >> quoted (quite a lot test in this sentence). >> >> Error is like >> t4151-am-abort.sh[5]: test: argument expected >> >> My patch: >> >> *** t4151-am-abort.sh Mon May 9 17:51:44 2016 >> --- t4151-am-abort.sh.orig Fri Apr 29 23:37:00 2016 >> *** >> *** 67,73 >> test_expect_success 'am -3 --skip removes otherfile-4' ' >> git reset --hard initial && >> test_must_fail git am -3 0003-*.patch && >> ! test 3 -eq "$(git ls-files -u | wc -l)" && >> test 4 = "$(cat otherfile-4)" && >> git am --skip && >> test_cmp_rev initial HEAD && >> --- 67,73 >> test_expect_success 'am -3 --skip removes otherfile-4' ' >> git reset --hard initial && >> test_must_fail git am -3 0003-*.patch && >> ! test 3 -eq $(git ls-files -u | wc -l) && >> test 4 = "$(cat otherfile-4)" && >> git am --skip && >> test_cmp_rev initial HEAD && >> *** > > Some comments: > > Quoting the output of 'wc -l' will break the tests on Mac OS X and BSD > since the output contains leading whitespace which won't match the "3" > on the other side of the '='. > > Your diff is backward, comparing 'current' against 'original', which > makes it difficult to read. Reviewers on this list expect to see > 'original' compared against 'current'. > > Use a unified format to make the diff easier to read; or just use > git-diff or git-format patch, which is even simpler. > > It's not clear how the output of 'wc -l' could ever be the empty > string. Perhaps git-ls-files is dying and causing the pipe to abort > before 'wc -l' ever outputs anything? Without additional information > about the problem you're experiencing, it's difficult to judge if this > change is a good idea. -- 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
t4151 missing quotes
Hi there, skipping through some failed tests I found more (smaller) problems inside the test... when test arguments are empty they need to be quoted (quite a lot test in this sentence). Error is like t4151-am-abort.sh[5]: test: argument expected My patch: *** t4151-am-abort.sh Mon May 9 17:51:44 2016 --- t4151-am-abort.sh.orig Fri Apr 29 23:37:00 2016 *** *** 67,73 test_expect_success 'am -3 --skip removes otherfile-4' ' git reset --hard initial && test_must_fail git am -3 0003-*.patch && ! test 3 -eq "$(git ls-files -u | wc -l)" && test 4 = "$(cat otherfile-4)" && git am --skip && test_cmp_rev initial HEAD && --- 67,73 test_expect_success 'am -3 --skip removes otherfile-4' ' git reset --hard initial && test_must_fail git am -3 0003-*.patch && ! test 3 -eq $(git ls-files -u | wc -l) && test 4 = "$(cat otherfile-4)" && git am --skip && test_cmp_rev initial HEAD && *** *** 78,88 test_expect_success 'am -3 --abort removes otherfile-4' ' git reset --hard initial && test_must_fail git am -3 0003-*.patch && ! test 3 -eq "$(git ls-files -u | wc -l)" && test 4 = "$(cat otherfile-4)" && git am --abort && test_cmp_rev initial HEAD && ! test -z "$(git ls-files -u)" && test_path_is_missing otherfile-4 ' --- 78,88 test_expect_success 'am -3 --abort removes otherfile-4' ' git reset --hard initial && test_must_fail git am -3 0003-*.patch && ! test 3 -eq $(git ls-files -u | wc -l) && test 4 = "$(cat otherfile-4)" && git am --abort && test_cmp_rev initial HEAD && ! test -z $(git ls-files -u) && test_path_is_missing otherfile-4 ' *** *** 146,152 git reset && rm -f otherfile-4 otherfile-2 file-1 file-2 && test_must_fail git am -3 initial.patch 0003-*.patch && ! test 3 -eq "$(git ls-files -u | wc -l)" && test 4 = "$(cat otherfile-4)" && git am --abort && test -z "$(git ls-files -u)" && --- 146,152 git reset && rm -f otherfile-4 otherfile-2 file-1 file-2 && test_must_fail git am -3 initial.patch 0003-*.patch && ! test 3 -eq $(git ls-files -u | wc -l) && test 4 = "$(cat otherfile-4)" && git am --abort && test -z "$(git ls-files -u)" && Regards, Armin -- 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: Portability of git shell scripts?
On Wed, May 4, 2016 at 11:20 PM, Jeff King wrote: > On Wed, May 04, 2016 at 08:17:38PM +0200, Armin Kunaschik wrote: > >> I'm trying to compile/test/use git 2.8.2 on AIX 6.1 with no bash available. >> /bin/sh is a hard link to /bin/ksh which is a ksh88, a posix shell. >> Is this supposed to work? > > We aim for a practical subset of Bourne shells, including bash, dash, > ash, ksh, etc. There's at least one Bourne-ish shell known not to work, > which is Solaris /bin/sh[1]. POSIX is usually a good guide, but we aim > for practical portability more than adhering strictly to the standards > document. > > I've tested with mksh in the past (though it's possible that we've > introduced a regression since then). But I think we've run into problems > with ksh93[2]. I don't know about ksh88, or what construct it doesn't > like. It may or may not be easy to work around. In general ksh (88 or 93) is posix compliant... and bash is moving away from posix. :-) But I know what you mean. >> As an example: make test fails on nearly every t34* test and on tests >> which contain rebase. >> The installation of bash (and manually changing the shebang to >> /bin/bash) "fixes" all rebase test failures. So obviously git-rebase >> is not portable at some point. > > Right. Any modern-ish Bourne shell will do, so moving to bash is one way > to fix it. My last compile of git 2.2.2 did far better than the current 2.8.2. So it looks like there were more recent changes that broke portability. >> Does it make any sense to put work into making these scripts portable, >> that is, work with posix shells? > > Maybe. :) If you can find what it is that ksh88 is unhappy with, we can > see how painful it is to adapt to. But given my looking into ksh93 in > [2], I suspect it will be easier to just use a more modern shell. Regarding [2] this was a bug which was fixed quite fast. To me this is no real showstopper. Modernity of ksh93 depends on the letter after the 93 :-) >> And, as last resort, is it possible to configure git use bash in some >> or all shell scripts? > > You can set SHELL_PATH in your config.mak file. I tried a build with SHELL_PATH=/bin/bash. Many problems "went away". Others appeared. I'll give it a few more days to look into it. First finding: make test does not make it through t3513-revert-submodule.sh anymore. The test is not portable since it uses the z-flags of GNU-tar. When -z is removed, (and extension is changed back to tar) everything runs and tests smoothly. Is this report enough to start the magic to change things? Regards, Armin -- 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
Portability of git shell scripts?
Hi list, I'm trying to compile/test/use git 2.8.2 on AIX 6.1 with no bash available. /bin/sh is a hard link to /bin/ksh which is a ksh88, a posix shell. Is this supposed to work? As an example: make test fails on nearly every t34* test and on tests which contain rebase. The installation of bash (and manually changing the shebang to /bin/bash) "fixes" all rebase test failures. So obviously git-rebase is not portable at some point. Does it make any sense to put work into making these scripts portable, that is, work with posix shells? And, as last resort, is it possible to configure git use bash in some or all shell scripts? Regards, Armin -- 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