search

rogpeppe / go-internal

Selected Go-internal packages factored out from the standard library
BSD 3-Clause "New" or "Revised" License
825 stars 67 forks source link

Add 'unix' condition #169

Closed bitfield closed 1 year ago

bitfield commented 1 year ago

(see commit messages)

Fixes #166.

bitfield commented 1 year ago

Sorry, I'm not completely clear on what you're suggesting. Could you elaborate a little?

You could check that [unix] matches when the host is [linux] and not when it's [windows]

That was the aim of the two tests guarded by mutually exclusive build tags—I couldn't find a way to get testscript to "pretend" to be running on a particular GOOS. Setting env GOOS=linux in the script has no effect. Is there a neater way to do this?

mvdan commented 1 year ago

Just pushed a commit to show what I mean :) I ended up deciding against using nested testscript, because we really don't need it. The end result is very close to what you had done, but with far less test code, thanks to testscript itself. The test only checks linux/darwin/windows, but I reckon that is enough given how popular they are, and that they are the three OSes that we use CI on.

mvdan commented 1 year ago

Oh, and while there, I also rewrote the existing cond.txt to use the native mkdir and exists rather than an external shell, which means more portability - particularly necessary for Windows. So perhaps we should wait for an extra review from @myitcv or @rogpeppe since I can't really approve my own changes there.

bitfield commented 1 year ago

Great! Very good suggestions, thank you @mvdan.

mvdan commented 1 year ago

If it sounds good to you, then squash into your second commit - then we're ready for another review and a merge.