fix: nox.session.run-ing commands with pathlib.Path arguments

[Julian]

Everything previously really worked fine in theory, but the debug string
which is logged (and calls shlex.quote) was blowing up trying
to quote a pathlib.Path object. I'm somewhat surprised upstream
shlex.quote doesn't support pathlib.Path objects (yet?) but for
now this just falls back to a cruder representation.

(Obviously there are other ways to handle this, like calling str() on all the args, let me know if you prefer something else).

This also apparently doesn't work on Windows, but that's because subprocess.Popen is apparently broken with WindowsPaths. The upstream bug is python/cpython#85815.

[henryiii]

I'd think os.fspath(x) if isinstance(x, os.PathLike) for x in args would be the right way to handle it (without introducing bugs if something non-pathlike gets passed in that has a str representation - which everything in Python does).

[Julian]

Happy to do that instead obviously -- do you mean just what I just pushed (I don't think one needs to do the isinstance check, unless you meant to filter out non-fspath args silently, which seems unlikely)?

[henryiii]

Actually, I guess it doesn't need a check, since os.fspath passes through str / bytes and only works on os.PathLike otherwise. It just seems a little odd to pass a non-path into os.fspath, and there will be non-paths coming in. It also might be a bit more extendable - if any other object protocols were ever added, you'd then need the check.

[henryiii]

FYI, I recently wrote a class that wrapped a commandline tool, and realized I could define __fspath__() instead of making up an arbitrary attribute holding the path to the wrapped exe. Things like this would cause it to just work as if it was the wrapped exe (this is exactly what I did with my own run function). Even without it, os.fspath(thing) is easier to remember than thing.some_name_I_made_up.

But the important thing is the str is different than __fspath__(), and the fspath one is the correct one to use.

[Julian]

To os.fspath you mean? That's why the way I did it originally seemed slightly better to me, since really what should work is "anything that subprocess is happy with should work here too", and it's nice if that automatically stays true without needing to make sure it does (considering there's some side use here of printing a debug string which indeed has different requirements). Especially so since I suspected from the comment that there's intention to remove _shlex_join if/when you stop supporting <3.8, so having it do something more than what the "real" shlex.join does is slightly awkward.

But obviously I'm happy to do whatever you'd like here, just lemme know what you suggest.

[Julian]

Right I've made CLIs that worked precisely as you describe (where they allow lots of fun objects to become filepaths). Certainly supporting more objects would indeed be nice.

[Julian]

Anything else you'd like to see changed from this one?

[henryiii]

I think this is great, and would avoid me having to write things like:

session.run("check-wheel-contents", str(*Path("dist").glob("*.whl")), "--ignore=W002")

That obviously only works if one wheel is found; if I could get rid of the str, it would handle any number of wheels (or I could use a comprehension, but that's already more complex then I want for something like this).

[FollowTheProcess]

I was initially concerned about using os.fspath on any argument, whether or not it is "PathLike". But it turns out that if os.fspath gets passed str or bytes they are returned unchanged so no worries there! Looks good to me @Julian thanks!

Link to os.fspath: https://docs.python.org/3/library/os.html#os.fspath

[Julian]

Thanks!