xilun / cbwin

Launch Windows programs from "Bash on Ubuntu on Windows" (WSL)
Other
327 stars 25 forks source link

Added simple path resolving for the rest of the WSL filesystem.… #33

Closed aHooder closed 7 years ago

aHooder commented 7 years ago

… Relative paths only work to some extent due to how folders are organized differently in the windows filesystem and the linux filesystem. A more robust implementation is required for supporting all relative path-jumping fully. (Have a look at %localappdata%\lxss and the /root, /home, /data and /rootfs directories to get a gist of it...) May result in undefined behaviour if the lxss directory isn't stored and organized where it is by default. The change on line 359 of outbash/outbash.cpp (setting the current directory) also solves an issue some programs have when not specifying the full path to the executable.

aHooder commented 7 years ago

Sorry for not doing this with the latest commit.

aHooder commented 7 years ago

After reading some of your responses here I've realized that this was something you specifically intended not to do. As I have just gotten to setting up WSL for myself I haven't actually read up on all the dos and don'ts yet. The code, however, seems to work fine and grants even more power to the cbwin tool.

xilun commented 7 years ago

Thanks for this patch!

You correctly noticed that I consider that dangerous (because of the technical descriptions provided by MS about how VolFs is implemented, and how the behavior is basically undefined if Win32 write there, and potentially only slightly less undefined if Win32 only read there)

However, several people want to do it anyway, so I might consider adding it, but subject to a flag like --dangerous-translations

This might interact with other features I plan, so I'll further think about it.

aHooder commented 7 years ago

Sounds good.

xilun commented 7 years ago

Also, do you have an example about what makes the SetCurrentDirectoryW(wdir); necessary?

xilun commented 7 years ago

I guess SetCurrentDirectoryW(wdir); helps launching with wrun a program not in the PATH using a relative filename, including in ".", however SetCurrentDirectoryW() has process wide effect so this is not a proper fix.

aHooder commented 7 years ago

I spent a good 30 minutes wondering why the working dir didn't do the job. If there's another solution, go for it.

aHooder commented 7 years ago

It can obviously be done with a couple additional commands, like pushd and popd or just a cd.

xilun commented 7 years ago

The only thing I can think of would be to try to resolve relative programs filenames ourselves... Or use SetCurrentDirectoryW() anyway and put it + CreateProcess in a critical section, but I prefer to avoid excessive locking.

aHooder commented 7 years ago

Implementing actual path resolution would be a better solution. I'll see if I can get it working. Do you know if there's another way to resolve the paths: /home, /data, /root and /mnt (other than knowing where they are and matching strings)? As far as I can tell they aren't even mounted separately. Are there any interfaces we can use to get information about this from WSL possibly?

xilun commented 7 years ago

I'm not sure there is a way to get those infos. Note that on 14393 regular fs mount points are not listed, but on 14915 they are:

rootfs on / type lxfs (rw,noatime)
data on /data type lxfs (rw,noatime)
cache on /cache type lxfs (rw,noatime)
mnt on /mnt type lxfs (rw,noatime)
sysfs on /sys type sysfs (rw,noatime)
proc on /proc type proc (rw,noatime)
cgroup on /acct type cgroup (rw,noatime)
none on /dev type tmpfs (rw,noatime,mode=755)
devpts on /dev/pts type devpts (rw,noatime)
none on /run type tmpfs (rw,noatime,mode=755)
none on /run/lock type tmpfs (rw,noatime)
none on /run/shm type tmpfs (rw,noatime)
none on /run/user type tmpfs (rw,noatime,mode=755)
none on /mnt/c type drvfs (rw,noatime)
binfmt_misc on /proc/sys/fs/binfmt_misc type binfmt_misc (rw,nosuid,nodev,noexec,relatime)
aHooder commented 7 years ago

We could just rely on bash resolving the relative paths itself, then doing the custom replace on recognized prefixes. This could mess up some commands in some very special cases, so adding an argument to bypass the additional path resolving would be smart. I'll rearrange the loop to parse all arguments and have a look at working and current dir solutions.

aHooder commented 7 years ago

Uhm... it appears I was compiling an older version of the code. It was my previous code that resolved the paths; not bash. I'll have a fixed commit up soon.

xilun commented 7 years ago

I'm not sure resolving under WSL is the best solution. I'll take a look at the Win32 rules...

aHooder commented 7 years ago

Right. I don't really see why evaluating absolute path in bash and replacing the beginning with the windows location is a problem (pastebin). It works quite well now. The only thing I'm a bit uncertain with is paths in quotes for use with multiple commands in one wstart command. I'd say such deep path resolving wouldn't be good to deal with at the bash level. Have you got any good way to do it with windows calls?

aHooder commented 7 years ago

Moving to PR using the latest source.