b2 icon indicating copy to clipboard operation
b2 copied to clipboard

Automatically use `embed-manifest-via=linker` for clang-win

Open Flamefire opened this issue 3 years ago • 9 comments

Environment and version details

Github Actions "Windows 2022" image.

Describe your use case

Building Boost with toolset=clang-win results in a failure:

'"C:\Program Files (x86)\Windows Kits\10\bin\x64\mt.exe"' is not recognized as an internal or external command, operable program or batch file.

Adding embed-manifest-via=linker solves this.

Describe the solution you'd like

Seemingly the b2 toolset uses a path to the non-existing file. For usability it would be good if B2 automatically uses embed-manifest-via=linker at least when mt.exe isn't found.

Flamefire avatar Jun 08 '22 08:06 Flamefire

I can't think of any downside to making embed-manifest-via=linker the default unconditionally across the board, so that the manifest tool is no longer needed.

pdimov avatar Jun 08 '22 11:06 pdimov

I can't think of any downside to making embed-manifest-via=linker the default unconditionally across the board, so that the manifest tool is no longer needed.

It requires VC11+

Kojoley avatar Jun 08 '22 16:06 Kojoley

Interesting. I use it unconditionally: https://github.com/boostorg/container_hash/blob/87c9eefe6ea8ed3cc0a7381e2580c1ce562e6acd/.appveyor.yml#L72

and the msvc-9.0,msvc-10.0 jobs pass: https://ci.appveyor.com/project/pdimov/container-hash/build/job/tbqpop4ap9qveye6

pdimov avatar Jun 08 '22 17:06 pdimov

Interesting. I use it unconditionally: boostorg/container_hash@87c9eef/.appveyor.yml#L72

and the msvc-9.0,msvc-10.0 jobs pass: ci.appveyor.com/project/pdimov/container-hash/build/job/tbqpop4ap9qveye6

I believe your input is saliently overridden by B2 for VC<11.

Kojoley avatar Jun 08 '22 20:06 Kojoley

I believe your input is saliently overridden by B2 for VC<11.

Why do you think that and how is it overridden? I mean there is compile-c-c++ bin.v2\libs\container_hash\test\hash_number_test2.test\msvc-9.0\debug\address-model-32\threading-multi\hash_number_test2.obj

Anyway the current solution is a clear Bug: The manifest-tool doesn't exist, the linker is capable of embedding the manifest but B2 still tries to run a non-existing tool.

So 2 solutions:

  1. B2 tries the linker first to see if it is capable of embedding the manifest (likely succeeds in almost all cases nowadays) and only if not searches for the mt.exe and errors out if it doesn't exist
  2. B2 searches for mt.exe and if that is not found enables embed-manifest-via=linker

I'd prefer the first solution as it is likely the most efficient one and provides a clean error in case of failure. The second is likely easier to implement.

Flamefire avatar Jun 09 '22 07:06 Flamefire

I believe Nikita means https://github.com/bfgroup/b2/blob/0c499449ae4e271a5eb4dcd81daf97422f9bf05e/src/tools/msvc.jam#L602-L617. Not sure what the intended meaning of "saliently" here was. :-)

The above does seem to set linker as the default though. Is this a recent change? Seems not:

https://github.com/bfgroup/b2/commit/b613e6dbf3bd84fed90b8a812a5fdf3e8f1d96fe

So we only need to make this default for clang-win, as it's already default for msvc.

pdimov avatar Jun 09 '22 13:06 pdimov

So we only need to make this default for clang-win, as it's already default for msvc.

Hm but there seemingly was an issue:

Unfortunately, it cannot be enabled on clang-cl with MSVC linker at the moment because it because of some path issues:

However given that we need it now, I'd say it no longer is an issue.

Flamefire avatar Jun 09 '22 14:06 Flamefire

Recent fixes to clang-win.jam may have made this unnecessary, but I think embed-manifest-via=linker should still be the default.

pdimov avatar May 01 '24 10:05 pdimov

embed-manifest-via=linker is broken on clang-win, it disables manifest embedding completely (disables mt invocation, but doesn't add /manifest:embed to link flags). Fixing that will expose all uses of embed-manifest-via=linker to a clang bug: it doesn't put WinSDK in PATH before invoking link.exe /manifest:embed, so you get LINK : fatal error LNK1158: cannot run 'rc.exe', which can be workaround but then there will still be no point in defaulting to embed-manifest-via=linker.

lld-link from recent (17?) release doesn't use WinSDK anymore, so clang-cl.exe /link /manifest:embed no longer fails with lld-link: error: unable to find mt.exe in PATH: no such file or directory. Switching to lld-link on clang-win is also needed to use LTO, so it might be a good move now.

Kojoley avatar May 04 '24 04:05 Kojoley