On Wed, Mar 29, 2023 at 3:58 AM Ævar Arnfjörð Bjarmason <avarab@xxxxxxxxx> wrote: > > On Tue, Mar 28 2023, Felipe Contreras wrote: > > > On Tue, Mar 28, 2023 at 6:57 PM brian m. carlson > > <sandals@xxxxxxxxxxxxxxxxxxxx> wrote: > >> > >> On 2023-03-28 at 17:39:26, Felipe Contreras wrote: > >> > It's not difficult to make the testing library work for zsh, so I did that in > >> > the first patch. > >> > > >> > The rest of the patches are basically to deal with some variables that are > >> > special in zsh, workaround a bug, and a minor discrepancy. > >> > >> There was a point at which the tests worked entirely in sh mode with > >> zsh. I know because I fixed a handful of tests there, ending with > >> c64368e3a2a47, and I patched zsh to run all commands in a pipeline in a > >> subshell in sh mode to fix the remaining tests. > >> > >> If I symlink zsh (zsh 5.9 (x86_64-debian-linux-gnu)) to sh in a > >> temporary directory and use it in SHELL_PATH, I get only the following > >> failures: > > > > That would defeat my motivation behind the patches, which is to be > > able to run one test file in zsh. > > "One" as in one specific file you have in mind, or a "one-off run"? One specific file. I just did a quick porting of the code in my fork, and this should give you a good idea (it's preliminary): https://github.com/felipec/git/blob/zsh/tests/t/t9904-completion-zsh.sh I had already sent versions of this test file that run in bash, but it's much better to fix test-lib.sh so that it works for zsh and they can be run directly. > The > 1/6 here looks like it fixes most of the issues, but e.g. the > test-lib.sh fix in 2/6 would be needed by any test that reached that > code, wouldn't it? Yes, but my test file doesn't reach that code. > Some details on all of this in an updated commit message would be most > welcome... Well, those details depended on the response. Since nobody seems to be interested in the tests running with vanilla zsh, I can just drop them and the explanation. > > Only the first patch is needed for that, the rest were in case anyone > > cared to run all the tests. > > > >> I don't care a lot of other folks want to make zsh run the testsuite in > >> zsh mode, but I'd think that using sh mode would be simpler and less > >> likely to break in general, and would avoid us needing to carry a lot of > >> patches to work around various variables that are special in zsh mode. > > > > We don't need to carry the patches if the patches are applied. > > But we do need to carry some hacks going forward, some of it seems > pretty isolated & easy to spot, but e.g. the 6/6 fix of: > > - if test "$c" = " " > + if test "$c" = " " || test -z "$c" > > Is quite subtle, you might look at that and be convinced that the RHS is > redundant, and be right, but only because you assume POSIX semantics. > > If we are going to include this I think the relevant t/README and > Documentation/CodingGuidelines parts should be updated to note that > we're not targeting POSIX shellscripts anymore, but the subset of it > that zsh is happy with. There's no point in that. I consider it a bug in zsh, along with 5/6, so presumably at some point it's going to be fixed. And if nobody cares about running these tests in zsh, it doesn't matter anyway. Cheers. -- Felipe Contreras
