{"thread":{"id":"35723","subject":"[PATCH 0/2] solaris test fixups","startedAt":"2014-01-23T19:54:05Z","lastAt":"2014-01-23T21:28:56Z","messageCount":9,"participants":["Jeff King","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"233615","messageId":"20140123195404.GA31314@sigill.intra.peff.net","threadId":"35723","inReplyTo":null,"subject":"[PATCH 0/2] solaris test fixups","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2014-01-23T19:54:05Z","receivedAt":"2014-01-23T19:54:05Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"Due to the alignment bug in another thread, I had the pleasure of\nvisiting my old friend Solaris 9 today. The tests _almost_ all run out\nof the box.\n\nThis series features two minor fixes:\n\n [1/2]: t7501: fix \"empty commit\" test with NO_PERL\n [2/2]: t7700: do not use \"touch -r\"\n\nI had a few other failures related to encodings; I suspect the problem\nis simply that the machine in question doesn't have eucJP support at\nall.\n\nThe big one that I did not fix is in t7001-mv. We do this:\n\n test_must_fail git mv some-file no-such-dir/\n\nand assume that it will fail. It doesn't. Solaris happily renames\nsome-file to a regular file named \"no-such-dir\". So we fail later during\nthe index-update, complaining about adding the entry \"no-such-dir/\", but\nstill exit(0) at the end. I'm mostly willing to just call Solaris crazy\nfor allowing the rename (Linux returns ENOTDIR), but I do wonder if\nthe index codepath could be improved (and especially to return an\nerror).\n\n-Peff\n"},{"id":"233617","messageId":"20140123195456.GA31871@sigill.intra.peff.net","threadId":"35723","inReplyTo":"20140123195404.GA31314@sigill.intra.peff.net","subject":"[PATCH 1/2] t7501: fix \"empty commit\" test with NO_PERL","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2014-01-23T19:54:57Z","receivedAt":"2014-01-23T19:54:57Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"t7501.9 tries to check that \"git commit\" will fail when the\nindex is unchanged. It relies on previous tests not to have\nmodified the index. When it was originally written, this was\nalways the case. However, commit c65dc35 (t7501: test the\nright kind of breakage, 2012-03-30) changed earlier tests (4\nand 5) to leave a modification in the index.\n\nWe never noticed, however, because t7501.7, between the two,\nclears the index state as a side effect. However, that test\ndepends on the PERL prerequisite, and so it does not always\nrun. Therefore if NO_PERL is set, we do not run the\nintervening test, the index is left unclean, and t7501.9\nfails.\n\nWe could fix this by moving t7501.9 up in the script.\nHowever, this patch instead leaves it in place and adds a\n\"git reset\" before the commit. This makes the test more\nexplicit about its preconditions, and will future-proof it\nagainst any other changes in the test state.\n\nSigned-off-by: Jeff King \n---\n t/t7501-commit.sh | 1 +\n 1 file changed, 1 insertion(+)\n\ndiff --git a/t/t7501-commit.sh b/t/t7501-commit.sh\nindex f04798f..94eec83 100755\n--- a/t/t7501-commit.sh\n+++ b/t/t7501-commit.sh\n@@ -57,6 +57,7 @@ test_expect_success 'using invalid commit with -C' '\n '\n \n test_expect_success 'nothing to commit' '\n+\tgit reset --hard &&\n \ttest_must_fail git commit -m initial\n '\n \n-- \n1.8.5.2.500.g8060133\n"},{"id":"233616","messageId":"20140123195518.GB31871@sigill.intra.peff.net","threadId":"35723","inReplyTo":"20140123195404.GA31314@sigill.intra.peff.net","subject":"[PATCH 2/2] t7700: do not use \"touch -r\"","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2014-01-23T19:55:18Z","receivedAt":"2014-01-23T19:55:18Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"Some versions of touch (such as /usr/ucb/touch on Solaris)\ndo not know about the \"-r\" option. This would make sense as\na feature of test-chmtime, but fortunately this fix is even\neasier. The test does not care about the timestamp of the\n.keep file it creates at all, only that it exists. So we can\nsimply drop the use of \"-r\".\n\nSigned-off-by: Jeff King \n---\n t/t7700-repack.sh | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/t/t7700-repack.sh b/t/t7700-repack.sh\nindex d954b84..f9f9014 100755\n--- a/t/t7700-repack.sh\n+++ b/t/t7700-repack.sh\n@@ -17,7 +17,7 @@ test_expect_success 'objects in packs marked .keep are not repacked' '\n \t# The second pack will contain the excluded object\n \tpacksha1=$(git rev-list --objects --all | grep file2 |\n \t\tgit pack-objects pack) &&\n-\ttouch -r pack-$packsha1.pack pack-$packsha1.keep &&\n+\ttouch pack-$packsha1.keep &&\n \tobjsha1=$(git verify-pack -v pack-$packsha1.idx | head -n 1 |\n \t\tsed -e \"s/^\\([0-9a-f]\\{40\\}\\).*/\\1/\") &&\n \tmv pack-* .git/objects/pack/ &&\n-- \n1.8.5.2.500.g8060133\n"},{"id":"233633","messageId":"xmqqeh3yto8h.fsf@gitster.dls.corp.google.com","threadId":"35723","inReplyTo":"20140123195404.GA31314@sigill.intra.peff.net","subject":"Re: [PATCH 0/2] solaris test fixups","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-01-23T20:52:30Z","receivedAt":"2014-01-23T20:52:30Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> and assume that it will fail. It doesn't. Solaris happily renames\n> some-file to a regular file named \"no-such-dir\". So we fail later during\n> the index-update, complaining about adding the entry \"no-such-dir/\", but\n> still exit(0) at the end. I'm mostly willing to just call Solaris crazy\n> for allowing the rename (Linux returns ENOTDIR), but I do wonder if\n> the index codepath could be improved (and especially to return an\n> error).\n\nI think j6t has a patch for that, a8933469 (mv: let 'git mv file\nno-such-dir/' error out on Windows, too, 2014-01-08).\n"},{"id":"233634","messageId":"20140123205404.GA12757@sigill.intra.peff.net","threadId":"35723","inReplyTo":"xmqqeh3yto8h.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH 0/2] solaris test fixups","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2014-01-23T20:54:04Z","receivedAt":"2014-01-23T20:54:04Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Jan 23, 2014 at 12:52:30PM -0800, Junio C Hamano wrote:\n\n> Jeff King writes:\n> \n> > and assume that it will fail. It doesn't. Solaris happily renames\n> > some-file to a regular file named \"no-such-dir\". So we fail later during\n> > the index-update, complaining about adding the entry \"no-such-dir/\", but\n> > still exit(0) at the end. I'm mostly willing to just call Solaris crazy\n> > for allowing the rename (Linux returns ENOTDIR), but I do wonder if\n> > the index codepath could be improved (and especially to return an\n> > error).\n> \n> I think j6t has a patch for that, a8933469 (mv: let 'git mv file\n> no-such-dir/' error out on Windows, too, 2014-01-08).\n\nAh yeah, that looks like exactly the same issue (and the fix looks sane\nfrom my cursory investigation). Thanks for the pointer. I wasn't\nplanning to look further into it, but now I can do so without feeling\nguilty. :)\n\n-Peff\n"},{"id":"233637","messageId":"xmqq61patnag.fsf@gitster.dls.corp.google.com","threadId":"35723","inReplyTo":"20140123195518.GB31871@sigill.intra.peff.net","subject":"Re: [PATCH 2/2] t7700: do not use \"touch -r\"","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-01-23T21:12:55Z","receivedAt":"2014-01-23T21:12:55Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> ... The test does not care about the timestamp of the\n> .keep file it creates at all, only that it exists.\n\nPlease refrain from using \"touch\" for such use cases in the first\nplace. It appears that\n\n\t>pack-$packsha1.keep &&\n\nis what the test wants.\n"},{"id":"233638","messageId":"20140123211425.GA21377@sigill.intra.peff.net","threadId":"35723","inReplyTo":"xmqq61patnag.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH 2/2] t7700: do not use \"touch -r\"","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2014-01-23T21:14:25Z","receivedAt":"2014-01-23T21:14:25Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Jan 23, 2014 at 01:12:55PM -0800, Junio C Hamano wrote:\n\n> Jeff King writes:\n> \n> > ... The test does not care about the timestamp of the\n> > .keep file it creates at all, only that it exists.\n> \n> Please refrain from using \"touch\" for such use cases in the first\n> place. It appears that\n> \n> \t>pack-$packsha1.keep &&\n> \n> is what the test wants.\n\nAgreed. I was making the minimal change, but I think there is no reason\nnot to fix both while we are there. Do you want to just mark up the\npatch in transit?\n\n-Peff\n"},{"id":"233641","messageId":"xmqq1tzytmqr.fsf@gitster.dls.corp.google.com","threadId":"35723","inReplyTo":"20140123211425.GA21377@sigill.intra.peff.net","subject":"Re: [PATCH 2/2] t7700: do not use \"touch -r\"","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-01-23T21:24:44Z","receivedAt":"2014-01-23T21:24:44Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> Agreed. I was making the minimal change, but I think there is no reason\n> not to fix both while we are there. Do you want to just mark up the\n> patch in transit?\n\nLet's just queue this instead.\n\n-- >8 --\nFrom: Jeff King \nDate: Thu, 23 Jan 2014 14:55:18 -0500\nSubject: [PATCH] t7700: do not use \"touch\" unnecessarily\n\nSome versions of touch (such as /usr/ucb/touch on Solaris)\ndo not know about the \"-r\" option. This would make sense as\na feature of test-chmtime, but fortunately this fix is even\neasier.\n\nThe test does not care about the timestamp of the .keep file it\ncreates at all, only that it exists. For such a use case, with or\nwithout portability issues around \"-r\", \"touch\" should not be used\nin the first place.\n\nSigned-off-by: Jeff King \nSigned-off-by: Junio C Hamano \n---\n t/t7700-repack.sh | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/t/t7700-repack.sh b/t/t7700-repack.sh\nindex d954b84..b45bd1e 100755\n--- a/t/t7700-repack.sh\n+++ b/t/t7700-repack.sh\n@@ -17,7 +17,7 @@ test_expect_success 'objects in packs marked .keep are not repacked' '\n \t# The second pack will contain the excluded object\n \tpacksha1=$(git rev-list --objects --all | grep file2 |\n \t\tgit pack-objects pack) &&\n-\ttouch -r pack-$packsha1.pack pack-$packsha1.keep &&\n+\t>pack-$packsha1.keep &&\n \tobjsha1=$(git verify-pack -v pack-$packsha1.idx | head -n 1 |\n \t\tsed -e \"s/^\\([0-9a-f]\\{40\\}\\).*/\\1/\") &&\n \tmv pack-* .git/objects/pack/ &&\n-- \n1.9-rc0-234-g8e6341d\n"},{"id":"233645","messageId":"20140123212856.GA21838@sigill.intra.peff.net","threadId":"35723","inReplyTo":"xmqq1tzytmqr.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH 2/2] t7700: do not use \"touch -r\"","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2014-01-23T21:28:56Z","receivedAt":"2014-01-23T21:28:56Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Jan 23, 2014 at 01:24:44PM -0800, Junio C Hamano wrote:\n\n> Jeff King writes:\n> \n> > Agreed. I was making the minimal change, but I think there is no reason\n> > not to fix both while we are there. Do you want to just mark up the\n> > patch in transit?\n> \n> Let's just queue this instead.\n\nThat's what I meant. :)\n\nThanks, looks good to me.\n\n-Peff\n"}]}