Fix determination of wasi-sdk version when built in a subdirectory

[whitequark]

This is just a small wrinkle that happens when I build wasi-sdk as a subproject (where I run cmake -S wasi-sdk-src -B wasi-sdk-build, and so the working directory doesn't match the directory with the script).

[sbc100]

I think by "as a subproject" you mean "out-of-tree build"? If so can you update the title and description accordingly? "subproject" has specific meaning in CMake I believe which initially confused me when reading this PR.

Otherwise LGTM with a minor nit

[sbc100]

Can you instead remove the =None here and this to the exec call on line 63?

Can you also remove the ='.' from git_commit? All the callers the directory name explicitly.

[whitequark]

Done

[whitequark]

I think by "as a subproject" you mean "out-of-tree build"? If so can you update the title and description accordingly? "subproject" has specific meaning in CMake I believe which initially confused me when reading this PR.

Done, although I think it would probably break if you built it as a subproject too.

[whitequark]

By the way, I ended up deciding not to use wasi-sdk in the YoWASP Clang distribution because of these lines:

wasi-sdk/cmake/wasi-sdk-sysroot.cmake

Lines 70 to 86 in 297b6d0

	execute_process(
	COMMAND ${CMAKE_C_COMPILER} -print-runtime-dir
	OUTPUT_VARIABLE clang_runtime_dir
	OUTPUT_STRIP_TRAILING_WHITESPACE)
	cmake_path(GET clang_runtime_dir PARENT_PATH clang_runtime_libdir) # chop off `wasi`
	cmake_path(GET clang_runtime_libdir PARENT_PATH clang_sysroot_dir) # chop off `lib`
	add_custom_target(compiler-rt-post-build
	COMMAND ${CMAKE_COMMAND} -E copy_directory
	${clang_sysroot_dir} ${compiler_rt_dst}
	COMMAND ${CMAKE_COMMAND} -E copy_directory
	${compiler_rt_dst}/lib/wasi ${compiler_rt_dst}/lib/wasip1
	COMMAND ${CMAKE_COMMAND} -E copy_directory
	${compiler_rt_dst}/lib/wasi ${compiler_rt_dst}/lib/wasip2
	COMMAND ${CMAKE_COMMAND} -E copy_directory_if_different
	${compiler_rt_dst}/lib ${clang_runtime_libdir}
	COMMENT "finalizing compiler-rt installation"
	)

which assume that CMAKE_C_COMPILER is configured to use the target sysroot (i.e. that build sysroot physically== target sysroot), which is not the case for the build I'm doing. Is this an intended limitation?

[sbc100]

I'll wait for @abrown to give the final approval here since he wrote this script

[sbc100]

Also @alexcrichton do we not to out-of-tree builds by default with the new cmake build? Should we ?

[whitequark]

Also @alexcrichton do we not to out-of-tree builds by default with the new cmake build? Should we ?

I think this build is technically out-of-tree if you build it exactly as specified in README, i.e. first the compiler then the sysroot. If you have an existing compiler build and you want just the sysroot, building the sysroot mangles the compiler runtime dir.

[sbc100]

Also @alexcrichton do we not to out-of-tree builds by default with the new cmake build? Should we ?

I think this build is technically out-of-tree if you build it exactly as specified in README, i.e. first the compiler then the sysroot. If you have an existing compiler build and you want just the sysroot, building the sysroot mangles the compiler runtime dir.

Sounds like we should update the README so its clear how to just build the sysroot with an existing toolchain.

[whitequark]

Sounds like we should update the README so its clear how to just build the sysroot with an existing toolchain.

Is this actually something that's supported currently? As far as I can tell it will always mangle the sysroot of such a toolchain no matter what you do. Other than that it was clear to me--I feel like this is just a bug, or a design issue perhaps.

[sbc100]

Sounds like we should update the README so its clear how to just build the sysroot with an existing toolchain.

Is this actually something that's supported currently? As far as I can tell it will always mangle the sysroot of such a toolchain no matter what you do. Other than that it was clear to me--I feel like this is just a bug, or a design issue perhaps.

Its probably a missing feature yes. Or maybe a feature that was possible with the old build system but is missing from the new cmake system, I'm not sure which.

Also, to be clear, what you say "mangle the sysroot" are you talking about the installation of the wasm compiler-rt library and clang headers? If so, I believe those go in the resource directory of the compiler, not the sysroot.

[sbc100]

Perhaps you could open a bug, asking to add support for building wasi-sdk without building clang/llvm.

BTW, I think you do need to eventually install the compiler-rt file from wasi-sdk into your clang resource directory, so there will some pollution of the compiler you want to use. This is because compiler-rt is really part of the compiler and not part of the sysroot.

[whitequark]

Also, to be clear, what you say "mangle the sysroot" are you talking about the installation of the wasm compiler-rt library and clang headers? If so, I believe those go in the resource directory of the compiler, not the sysroot.

This is accurate. I am using the terms somewhat interchangeably in a way that created this confusion because in the build of Clang I'm using that targets Wasm it was by far easiest to merge the two (i.e. have --sysroot /x -resource-dir /x). I think the accurate term would be "toolchain installation prefix" in this case.

BTW, I think you do need to eventually install the compiler-rt file from wasi-sdk into your clang resource directory, so there will some pollution of the compiler you want to use. This is because compiler-rt is really part of the compiler and not part of the sysroot.

I could also use an explicit -resource-dir to avoid this. (This is I think unavoidable in my particular case because the default resource directory logic seems to result in the compiler driver using a relative path for some reason.)

In any case I do have to take care to match the version of all LLVM components used in the Clang build and in the wasi-sdk build, which in itself requires additional work. For now I decided to build compiler-rt, wasi-libc, and libcxxabi/libcxx on my own to avoid this integration work since it wasn't clear if there's a large benefit to it.

[whitequark]

Perhaps you could open a bug, asking to add support for building wasi-sdk without building clang/llvm.

I've thought about this but I'm unsure how much I want to drop TODO list items of uncertain relevance into inboxes of wasi-sdk developers (who I'm sure have plenty to work on), so I thought I'd bring it up here first.

[sbc100]

For now I decided to build compiler-rt, wasi-libc, and libcxxabi/libcxx on my own to avoid this integration work since it wasn't clear if there's a large benefit to it.

That sounds like a reasonable solution. The on the main design goals of wasi-sdk was that it should be as "plain llvm/clang" as possible. Building your own should be as easy as possible and wasi-sdk is really just a convenience for folks that want a quick and easy set of pre-builts. This is why, for example, we have stayed away from forking clang or any of its standard libraries here.

[sbc100]

For now I decided to build compiler-rt, wasi-libc, and libcxxabi/libcxx on my own to avoid this integration work since it wasn't clear if there's a large benefit to it.

That sounds like a reasonable solution. The on the main design goals of wasi-sdk was that it should be as "plain llvm/clang" as possible. Building your own should be as easy as possible and wasi-sdk is really just a convenience for folks that want a quick and easy set of pre-builts. This is why, for example, we have stayed away from forking clang or any of its standard libraries here.

BTW, if you do want just the sysroot and resource-dir (compiler-rt) you can download those two directly from the releases page (without downloading the compiler itself).

[whitequark]

The on the main design goals of wasi-sdk was that it should be as "plain llvm/clang" as possible. Building your own should be as easy as possible and wasi-sdk is really just a convenience for folks that want a quick and easy set of pre-builts. This is why, for example, we have stayed away from forking clang or any of its standard libraries here.

That makes sense! I think the only reservation I have is related to the way wasi-sdk provides support for multiple targets in the same prefix, since I don't fully understand the mechanism by which it does so, or whether there is anything users of the SDK would have to be aware of. But I should just study it more.

BTW, if you do want just the sysroot and resource-dir (compiler-rt) you can download those two directly from the releases page (without downloading the compiler itself).

I do know that but one of my goals (which is as much my goal as it is a desire of the LLVM community) is to provide daily builds of the SDK with LLVM from main, which of course isn't OK to mix with a resource directory from a different build of LLVM. Otherwise I would have just shipped the wasi-sdk sysroot.

[sbc100]

The on the main design goals of wasi-sdk was that it should be as "plain llvm/clang" as possible. Building your own should be as easy as possible and wasi-sdk is really just a convenience for folks that want a quick and easy set of pre-builts. This is why, for example, we have stayed away from forking clang or any of its standard libraries here.

That makes sense! I think the only reservation I have is related to the way wasi-sdk provides support for multiple targets in the same prefix, since I don't fully understand the mechanism by which it does so, or whether there is anything users of the SDK would have to be aware of. But I should just study it more.

BTW, if you do want just the sysroot and resource-dir (compiler-rt) you can download those two directly from the releases page (without downloading the compiler itself).

I do know that but one of my goals (which is as much my goal as it is a desire of the LLVM community) is to provide daily builds of the SDK with LLVM from main, which of course isn't OK to mix with a resource directory from a different build of LLVM. Otherwise I would have just shipped the wasi-sdk sysroot.

The sysroot part should be relatively independent the llvm/clang versions.

Even the libclang/compiler-rt part is very stable since the interface between clang and compiler-rt very rarely changes so you should be able to build llvm from main and use the sysroot and compiler-rt from the latest wasi-sdk release. I can imagine that could in theory break but it would be rare.

In emscripten, for example, we always provide llvm/clang from tip-of-tree and compiler-rt pinned to that last stable llvm release. I don't think we have ever had any issues with that.

[whitequark]

Thank you, this is very useful context to know! I'll have to consider which choice would be the best maintenance-wise for YoWASP Clang.

[alexcrichton]

By the way, I ended up deciding not to use wasi-sdk in the YoWASP Clang distribution because of these lines:

(also in repsonse to other things mentioning me here)

This is one of the major downsides of the "sysroot only build" right now. I couldn't figure out how to actually do that as it seemed that clang required that compiler-rt was in its own directory and clang didn't look in the --sysroot directory for compiler-rt. This means that if the host toolchain's directory wasn't mangled then the build would not succeed.

Now that being said I was unaware of the distinction of -resource-dir and --sysroot. It sounds like the reality of the situation is that clang looks inside of -resource-dir for compiler-rt. That means that the build system for the sysroot should place compiler-rt in a build-specific directory and pass -resource-dir to clang. Or... something like that. We don't want to lose headers like wasm_simd128.h or such so I'm not sure how to do that.

Basically this is something I'd like to fix, but I don't know enough how to fix it. It would be easy enough to add a CMake option to disable building compiler-rt though and that should fix the issue here I believe.

do we not to out-of-tree builds by default with the new cmake build

This is a function of how CMake is invoked, so it's not really a build-system-specific option codified with a default one way or another. Most CMake builds end up being out-of-tree-builds.

Sounds like we should update the README so its clear how to just build the sysroot with an existing toolchain.

Is this actually something that's supported currently?

Its probably a missing feature yes. Or maybe a feature that was possible with the old build system but is missing from the new cmake system, I'm not sure which.

This feature, AFAIK, has never been supported. The old Makefile was "you either want this build configuration or you're out of luck". With CMake my hope was to expose more possible configurations, such as building just a sysroot. The compiler-rt libraries are the wrinkle in this because I couldn't figure out a way to use the compiler-rt built from source without modifying the host toolchain's sysroot. If others know how to do this I'd love to add support.

---

Overall I think the easiest thing for now, if it works for you @whitequark, would be to add a -DWASI_SDK_BUILD_COMPILER_RT=OFF option which simply disables building compiler-rt entirely and would thus not modify the host sysroot. Doing that, however, would only work if you already have a compiler-rt in the -resource-dir of the host toolchain. That would require either downloading a release from this repository or something like that. Short of that though I believe it would require building a custom -resource-dir but I don't know how to do that.

[alexcrichton]

Also I'm going to go ahead and merge this PR as it looks like a good change to make and feedback has been addressed. I don't mean to stymie discussion of a sysroot-only build though. I've opened #444 to track sysroot-only builds.

[sbc100]

I guess if want to build a "resource-dir" outside of the toolchains actual/real/own resource dir when we would need/want to create a full resource dir containing both libclang (compiler-rt) and the toolchain headers such as wasm_simd128.h . I imagine there is a target to installing those clang headers alongside libclang.

[alexcrichton]

The problem though is that with a sysroot-only build where you're not using the host toolchain I don't know where to get the toolchain headers from. If there's a build target in LLVM to install just the headers and not actually build any object files that might be reasonable, but otherwise if we're not building LLVM I'm not sure how to get the headers out other than just copying them from the host toolchain.

[sbc100]

Yes, I was imagining there would be some kind of llvm build target. i.e.make install-clang-rt install-clang-header?

[alexcrichton]

I think #445 should fix the -resource-dir and modifying-the-host-compiler-sysroot behavior. I ended up handling headers by copying them from the host compiler rather than using the in-tree LLVM sources.

[whitequark]

If there's a build target in LLVM to install just the headers and not actually build any object files that might be reasonable

This target is called install-core-resource-headers and I'm using it in YoWASP Clang.

[whitequark]

Although I'm not 100% sure that this is the specifically right target and @sbc100 might know better than I do.