canonical / jhack

Chock-full of Juju hackery.
Apache License 2.0
48 stars 23 forks source link

feat: better $HOME detection #170

Closed dimaqq closed 3 weeks ago

dimaqq commented 3 weeks ago

Specifically, trust $HOME if it's set

PietroPasotti commented 2 weeks ago

@dimaqq this apparently breaks jhack if snapped. also, TIL, Path("~").expanduser() internally uses the HOME envvar so the code I had before is functionally equivalent, only we swapped the priority of USER and HOME. I'm switching that back hoping it will solve the issue.

PietroPasotti commented 2 weeks ago

in the case of snapped jhack I'm afraid HOME will be set to the snap home dir, which we don't care about, so instead looking up USER first seems to give the desired behaviour. Was the priority swapping intentional, and if so, what were you trying to solve?

dimaqq commented 2 weeks ago

Oh, that's a problem...

I'm running a colima VM, and it's default is to create a user same as my host user, so dima, but the home dir is /home/dima.linux which means concatenating /home/ and username doesn't do the right thing.

I wonder if expanding ~ yields real home dir or snap home dir...

Or would it help to select a few candidate directories and test if they actually exist?

dimaqq commented 2 weeks ago

Looking at https://snapcraft.io/docs/home-interface if jhack snap declares the home plug, should it not just work?

PietroPasotti commented 2 weeks ago

we already have image and image

so I was expecting it would work out of the box

dimaqq commented 2 weeks ago

dot files are not part of the home interface

home interface is not the same as personal files interface.

dimaqq commented 2 weeks ago

See e.g. https://github.com/juju/juju/blob/3.6/snap/snapcraft.yaml#L58-L59

dimaqq commented 2 weeks ago

Anyway, if jhack doesn't need arbitrary files from user's home, then the personal files plug/interface should be enough. It's then only a matter of guessing the path to the config files consistently whether snapped or not... right?

PietroPasotti commented 2 weeks ago

correct. I'm not convinced requesting home is the solution here

PietroPasotti commented 2 weeks ago

also, turns out we already have the home plug too image

dimaqq commented 2 weeks ago

Should we check $SNAP_REAL_HOME first?

Here's the environment from snapped juju:

root@colima-ahh:/home/dima.linux# cat /proc/3015028/environ | tr \\0 \\n | grep -i home
HOME=/home/dima.linux/snap/juju/27226
OLDPWD=/home/dima.linux
SNAP_USER_DATA=/home/dima.linux/snap/juju/27226
SNAP_USER_COMMON=/home/dima.linux/snap/juju/common
PWD=/home/dima.linux
SNAP_REAL_HOME=/home/dima.linux
PietroPasotti commented 2 weeks ago

huh, if that's guaranteed to be set...