Stop assuming PATH env is defined when extra_paths is set, and then initialize to os.defpath

[consideRatio]

I'm not sure if this is sufficient to resolve issues with using extra_paths together with jupyterhub 5.2.0 where env_keep doesn't include PATH in the Spawner base class any more, but it should probably be an incremental improvement no matter what.

See debugging notes in the littlest jupyterhub distribution when trying to upgrade jupyterhub from 5.1.0 to 5.2.0.

I've not added a test for extra_paths in this project, but it gets tested via tljh though, and via jupyterhub/the-littlest-jupyterhub#1008 I can see that this made a positive difference making tests pass where they previously failed.

[consideRatio]

@minrk I'd like to go for a bugfix release with this PR to make tljh 5.2.0 compatible

[minrk]

Makes sense! One comment about using the standard default path, but go ahead once that's addressed.

[minrk]

If PATH is not defined, probably use os.defpath for the default value (env.get("PATH", os.defpath))

(Not a code suggestion because typed from my phone)

[consideRatio]

Suggested change

	current_path = env.get("PATH")
	current_path = env.get("PATH", os.defpath)

[consideRatio]

This should be done without being conditional on extra_paths being set non-empty then, right?

[consideRatio]

I've pushed 3d12e12 making us initialize PATH to os.defpath if unset, independently of if extra_paths has been set truthy or not.

[minrk]

I think this should only be for reading PATH. This is the same effective value if PATH is omitted, so we shouldn't need to add it if we're not setting PATH. It's only relevant when adding to PATH.

[consideRatio]

I missed the comment you made because GitHub UI didn't refresh properly =/

Okay deal! We'll then let this only influence the situation when extra_paths is set and we are reading PATH to prepend extra_paths!

[consideRatio]

Thank you for helping me with this @minrk!!!

[minrk]

Looking at the path resolution on L159, it uses os.getenv(PATH, os.defpath), i.e. the Hub's PATH, but only for looking up jupyterhub-singleuser itself, and only if $PATH is unspecified for the unit. it doesn't affect the default PATH of the process.

So systemdspawner will find JupyterHub-singleuser if:

• you specify PATH and jupyterhub-singleuser, or
• you don't specify PATH at all

But not if you specify PATH to something without singleuser, which seems inconsistent.

I think behavior might be more consistent if we either:

• never add Hub $PATH to the search for JupyterHub-singleuser (rely on correct PATH config of the systemd unit and/or absolute paths), or
• always do (singleuser should always be found, with current env as lowest priority fallback, no matter what unit $PATH is)

so behavior is more consistent across config situations.

We could also change the default for unit $PATH to hub $PATH, but this has higher risk of impacting real deployments. It should be quite rare and usually a mistake to leave $PATH undefined in a systemd unit, especially for user servers. Unless the launch command is an initialization script that is responsible for setting up $PATH.

[minrk]

Btw, I think there's an opportunity to clear a few things up and be more consistent, but that's its own task, not needed here. The surprising test failures here just raise the issue.

[consideRatio]

I've opened #146 to not loose track of this discussion.