WebAssembly / wasi-sdk

WASI-enabled WebAssembly C/C++ toolchain
Apache License 2.0
1.27k stars 190 forks source link

Running `make` in a configured wasi-sdk tree installs things into `/usr/local` #442

Closed whitequark closed 3 months ago

whitequark commented 3 months ago

I did not expect this behavior (normally only make install does that, which I was going to run with DESTDIR=, as usual) and it is fairly unwelcome in that I don't know how or if I can uninstall those files.

whitequark commented 3 months ago

Double-checking my assumptions, the README suggests that the install target would install files, so my conclusion that the default target won't seems to be supported in that. I guess this is probably an oversight, but I still think it's an issue--just running make isn't supposed to modify the host system!

sbc100 commented 3 months ago

I agree, only the install target(s) should put things in /usr/local. Is this something that change in the recent cmake transition? It not unlikely that there will be a few things like this to iron out.

BTW, unless you run a root copying stuff to /usr/local/ should give you permission denied (at least on linux systems I'm familiar with.. I don't know how this works on macOS). i.e. sudo should be required the install phase by default.

The problem of not being able to cleanup file installed by make install is fairly common. I think both cmake and autoconf suffer from this problem, sadly. They don't have make uninstall targets. I don't know the history of that be its always been that way I think. I guess package managers are they way that got solved.

whitequark commented 3 months ago

Is this something that change in the recent cmake transition? It not unlikely that there will be a few things like this to iron out.

I think so.

BTW, unless you run a root copying stuff to /usr/local/ should give you permission denied (at least on linux systems I'm familiar with.. I don't know how this works on macOS). i.e. sudo should be required the install phase by default.

I have /usr/local configured to be owned by my $USER because it's essentially only used for software that's fussy about running from the build tree (no rpath, hardcoded resource paths, etc) and I'm the only one using this workstation.

On macOS this is done by default by Homebrew so this setup is actually quite common.

The problem of not being able to cleanup file installed by make install is fairly common. I think both cmake and autoconf suffer from this problem, sadly. They don't have make uninstall targets. I don't know the history of that be its always been that way I think. I guess package managers are they way that got solved.

Yes, which is why I put such an emphasis on not installing them there in the first place.

alexcrichton commented 3 months ago

This is sort of intended sort of not. With the previous Makefile you couldn't change the installation directory at all, it was simply always build/install. I wanted to add the option of changing things.

This is made difficult due to the dependencies between projects. For example if you want to build libcxx you have to previously build and install wasi-libc. It's not sufficient to simply build wasi-libc, it's got to get installed so clang knows how to pick it up. This all worked before because the installation directory was fixed to be inside of the build directory.

I'm happy to help implement other changes here, but the best I can think of is to either (1) error or warn on a missing -DCMAKE_INSTALL_PREFIX argument or (2) default the installation prefix to inside the build directory. Either that or documenting the current behavior.

I don't know how to architect things here such that it's a classic "run make to do all the build stuff and run make install to run all the copy stuff". Due to the usage of multiple projects I just don't know how to do that. If others know how though I'd be also happy to implement that.

sbc100 commented 3 months ago

Its a little confusing here since we want to do installation-like things as part of our build.

The building and installing of things into the sysroot should probably all be done locally inside the build directory. (i.e. llvm and clang should be configure to install into a location that is inside of the wasi-sdk build directory).

Actualy running make install at the wasi-sdk level should either do nothing (error out) or it should install into subdirectory such as /usr/local/share/wasi-sdk/... it should not be installing its sysroot stuff like libs and header into the system header/library directories.

I think just having make install be an error or do nothing makes the most sense for now.

sbc100 commented 3 months ago

Certainly typing make should not try to write to /usr/local/..

whitequark commented 3 months ago

Either that or documenting the current behavior.

I think this is the only option that's really not okay as that's very easy to miss and basically no well-behaved software installs stuff into /usr when you type make.

alexcrichton commented 3 months ago

I'll note I'm not defending the current state of affairs. My testing didn't surface this and I'm not a day-in-day-out C/C++ guru so this wasn't the first thing I looked to design when writing the CMake integration. I wanted to mostly outline the various options I thought were possible.

Regardless https://github.com/WebAssembly/wasi-sdk/pull/446 should implement this.

whitequark commented 3 months ago

Ah I see. #446 is perfect, thank you!

whitequark commented 3 months ago

Thanks for fixing this again!