[PATCH] D36255: Integrate manifest merging library into LLD.

Eric Beckmann via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Aug 21 16:45:02 PDT 2017


ecbeckmann added inline comments.


================
Comment at: lld/test/lit.cfg:270
+# libxml2 to merge manifests.
+if (lit.util.which('cvtres', config.environment['PATH'])) or (config.llvm_libxml2_enabled == "1"):
+    config.available_features.add('manifest_tool')
----------------
zturner wrote:
> Can you line-break here?  This goes past 80.  Also, why are we checking for `cvtres` instead of `mt`?  The comment makes sense, but why not just check for `mt` directly?
Checking for mt always seems to succeed, there are too many identically named tools I believe.  Note that this was the standard check for finding Microsoft utils even before I started making changes, you could search for rc and cvtres but not for mt.


https://reviews.llvm.org/D36255





More information about the llvm-commits mailing list