<div dir="ltr"><div dir="ltr"><br></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Wed, Sep 7, 2022 at 5:14 AM Rob Landley <<a href="mailto:rob@landley.net" target="_blank">rob@landley.net</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">On 9/6/22 19:09, enh wrote:<br>
>     I can force the time format with --full-time, although does the mac ls command<br>
>     have that...<br>
> <br>
>     <a href="https://apple.stackexchange.com/questions/15170/how-do-i-change-the-time-format-used-by-ls-command-line-on-osx" rel="noreferrer" target="_blank">https://apple.stackexchange.com/questions/15170/how-do-i-change-the-time-format-used-by-ls-command-line-on-osx</a><br>
>     <<a href="https://apple.stackexchange.com/questions/15170/how-do-i-change-the-time-format-used-by-ls-command-line-on-osx" rel="noreferrer" target="_blank">https://apple.stackexchange.com/questions/15170/how-do-i-change-the-time-format-used-by-ls-command-line-on-osx</a>><br>
> <br>
>     No it does  not. Nor does it provide an obvious alternative, they recommend<br>
>     "brew install coreutils". Right, I'm pretty happy saying mac only supports "make<br>
>     tests" all at once with macos_defconfig, and if you want more than that you can<br>
>     set up the $PATH to appropriate commands yourself. (Comparing toybox, busybox,<br>
>     and debian makes sense. Comparing with macosx is out of scope.)<br>
> <br>
> it's also useful for debugging specific mac issues... i currently have to `rm<br>
> tests/*.test` and then `git checkout -- tests/tar.test` to test tar on macos in<br>
> a reasonable amount of time.<br>
<br>
Um, _what_ is useful? (Happy to make that easier for you, but what specifically<br>
are you recommending I do? Lots of things that first "it" could apply to in the<br>
previous paragraph...)<br></blockquote><div><br></div><div>sorry, my intended referent wasn't even in your block --- i was grumbling about "mac only supports `make tests` all at once". specifically "it's annoying that we can't `make test_foo` on macOS [though i do have a workaround so it's not a priority, and i don't have a good alternative to realpath on macOS so it's not trivial]".</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
>     The problem is, if you're still seeing the OLD hash, I can't just change it to<br>
>     the one I'm seeing. If you are, could you send me the tarball? (You saw the<br>
>     "save all tarballs" hack a couple emails back, right? I should work the test<br>
>     name in there or something, although "sha1sum *" should let you know which one<br>
>     has the matching hash...)<br>
>  <br>
> i saw it, but didn't you see my reply saying that it didn't work?<br>
<br>
Not ringing a bell, but it's been a week... (dig dig dig...)<br>
<br>
Found it. Ah, I remember the _start_ of that message. I opened a reply window,<br>
got halfway through, my laptop battery died between sessions and lost my open<br>
reply windows (thanks thunderbird) and the original message was marked as read. :P<br>
<br>
Got it open in another window and responding to it again.<br>
<br>
>     I checked in the rest of it, lemme know what I broke?<br>
> <br>
> it doesn't seem to work at all?<br>
> <br>
> you got the test the wrong way round (it should have been `&& SKIP=1` not `||<br>
> SKIP=1`), but then it only seems to last for one test anyway?<br>
<br>
Sigh. I changed those tests manually to make sure the plumbing was skipping<br>
them, and the "check for darwin" version I reverted to after wasn't right...<br>
<br>
> oh, because you're supposed to count the number of tests to skip? i'm not sure i<br>
> have the words to express how terrible of an idea this seems to me? that's so<br>
> unreadable an idea even its own author got it wrong? :-P<br>
> <br>
> i still don't understand why there isn't just a "positive" version of skipnot?<br>
<br>
There can be, but skipnot increments skip by 1 so would still have to be<br>
repeated before each test instead of grouped.<br>
<br>
These test suite changes open new semantic possibilities, you can now skip<br>
GROUPS of tests. The interesting cases are probably just "skip next" and "skip<br>
until told to stop", but a generic mechanism was easier to implement.<br></blockquote><div><br></div><div>grouping as an idea is fine, but this implementation just seems hard to write _and_ hard to read (and non-obvious to know whether you're even within one).</div><div><br></div><div>my preference for "grouping" was just to factor out the condition and then mark each test. so instead of</div><div><br></div><div>skipnot some-complicated-condition-that-tells-us-whether-we-have-sparse-support-say</div><div>test ...</div><div>skipnot some-complicated-condition-that-tells-us-whether-we-have-sparse-support-say</div><div>test ...</div><div>skipnot some-complicated-condition-that-tells-us-whether-we-have-sparse-support-say</div><div>test ...</div><br><div>we'd have</div><div><br></div><div>HAS_SPARSE=some-complicated-condition-that-tells-us-whether-we-have-sparse-support-say</div><div>skipnot HAS_SPARSE</div><div>test ...</div><div>skipnot HAS_SPARSE</div><div>test ...</div><div>skipnot HAS_SPARSE</div><div>test ...</div><div><br></div><div>it's mildly repetitive still, but it's more intention revealing (modulo our ability to come up with a decent name), and it's super easy to read and you never need a lookbehind of more than one line to know what the line you're looking at will do.</div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
The new API for USING the new infrastructure was quick and dirty "get something<br>
checked in". It could definitely use more polishing. I got distracted by "huh,<br>
'optional' has almost entirely gone away hasn't it" and "the documentation at<br>
the start of runtest.sh is completely out of date" and testing every change I<br>
made under mksh to make sure it supports ((++blah)) and such...<br></blockquote><div><br></div><div>at the very least, if we do keep something like the current implementation of grouping, i'd recommend we start indenting the tests that are in a group for readability.</div><div><br></div><div>(but then at that part i start to wonder even more what exactly this is buying us? the .test files are already shell scripts, so isn't `if ... fi` the better way to do this --- everyone already needs to know that.)</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
> personally i think the only thing wrong with skipnot is that it's backwards,<br>
> which makes the tests hard to read.<br>
<br>
Semantics: it skips ONE test.<br>
<br>
skipnot()<br>
{<br>
...<br>
  [ $? -eq 0 ] || { ((++SKIP)); return 1; }<br>
}<br>
<br>
It does not start a group of skipped tests.<br></blockquote><div><br></div><div>(i still think that's a _feature_. see above.)</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
Also, "toyonly testcmd blah..." is one line, where:<br>
<br>
  skipnot condition<br>
  testcmd blah...<br>
<br>
Is two and they can even have arbitrary extra stuff _between_ them (comments or<br>
mkdir etc)... not consistent semantics there.<br></blockquote><div><br></div><div>but we could have them "stack" instead, no? if skipnot never _clears_ the "should i skip?" flag, you can have</div><div><br></div><div>skipnot condition</div><div>toyonly</div><div>testcmd blah...</div><div><br></div><div>anyway, right? or, if you really wanted consistency, you could replace toyonly with a variable that could be used with skipnot for something like</div><div><br></div><div>skipnot not-toy     # it's cases like this that really make me want a skip_if!</div><div>skipnot condition</div><div> <br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
I was reluctant to expand that without giving it more thought. Sure I can add<br>
four "skipif", "skipnot", "skipifmany", "skipnotmany" but I'm not sure that's<br>
the right way to go vs just "here's a flexible mechanism"...<br></blockquote><div><br></div><div>i think our biggest disagreement is that i don't think the *many forms are a good idea.</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
toyonly encapsulates a non-obvious test (are we running under toybox), and using<br>
((++SKIP)) instead of SKIP=1 is subtly important to have an individual test skip<br>
_inside_ a larger block that could be skipped for other reasons:<br>
<br>
  [ thingy ] || SKIP=1<br>
<br>
Is a sharp edge because it terminates the skip block you're in after this test...<br>
<br>
> i actually _like_ having a `skip_if` on<br>
> tests; each test is understandable in its own right, without you needing to read<br>
> any context. (that's actually part of why _i_ wasn't offended by the repetition<br>
> of all the uname checks in my version, but i'd much rather factor out a DARWIN=1<br>
> or something if you want something brief like `skip_if DARWIN` instead.)<br>
<br>
I agree it needs more design work. And documentation. And a walkthrough video.<br>
It's on the todo heap with making the test suite run under mkroot and the whole<br>
"running root tests" vs "running non-root tests" (which test different things,<br>
for example in tests/test.test where the access tests punch right through the<br>
permissions as root so aren't really _testing_ as much. This means I probably<br>
have to have mkroot run the test suite TWICE, both as root and as non-root...)<br>
<br>
> but expecting humans to _count_ the number of tests to skip (and to know, when<br>
> adding/removing/changing existing tests, that they might invisibly be in such a<br>
> "block") sounds like the kind of thing that even a FORTRAN IV programmer might<br>
> have said "dude, are you sure?" to...<br>
<br>
That's why the new "optional" sets it to 99999.<br>
<br>
Having it count down and expire means the same mechanism can handle "skip the<br>
next test" and "skip all tests until we switch this back off". That's basically<br>
an implementation detail. I exposed it in tar.test because I hadn't worked out<br>
what a wrapper should look like yet (or if there should be one).<br>
<br>
(The current answer to the question "ah but can different skip blocks nest?" is<br>
"not going there". Yes I _could_ verbose_has the SKIP plumbing but I really<br>
wouldn't want to DOCUMENT that...)<br></blockquote><div><br></div><div>we wouldn't even have to have this discussion if we just said "a little bit of repetition is helpfully intention-revealing, and there's always `if ... fi` if there's too much"...</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
> from this line:<br>
> <br>
> SKIP=0 # End of tests that don't work on MacOS X<br>
> <br>
> i don't think you even believed in it yourself :-) did you accidentally check in<br>
> something that was between two changes?<br>
<br>
The semantics changed a couple times while I was thinking it through and the<br>
LOCAL test I used to make sure skipping them worked wasn't "darwin", and then I<br>
used "undo" to put the darwin tests back and it reverted too far. :P<br></blockquote><div><br></div><div>btw, i spoke too soon when i said that it works on Android ... i hadn't run the chattr tests :-(</div><div><br></div><div>looks like there are still .test files with SKIPNEXT references but SKIPNEXT is broken now?</div><div><br></div><div>that causes trouble in chattr.test for the reason outlined in the existing comment there:</div><div><br></div><div># For the rest, just toggle the bits back and forth (where supported).<br># Note that some file system/kernel combinations do return success but<br># silently ignore your request: +T on 4.19 f2fs, or +F on 5.2 ext4,<br># for example, so we're deliberately a bit selective here.<br># f2fs in 5.6+ kernels supports compression, but you can only enable<br># compression on a file while it's still empty, so we skip +c too.</div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
Rob<br>
</blockquote></div></div>