[PATCH] D37240: Fix crbug 759265 by suppressing llvm mt warnings.

[Rui Ueyama via Phabricator via llvm-commits]

ruiu added inline comments.


================
Comment at: lld/COFF/DriverUtils.cpp:431
+  std::string Msg = toString(InternalOutputBufferOrError.takeError());
+  if (Msg != "no libxml2")
+    fatal("error with internal manifest tool:" + Msg);
----------------
ecbeckmann wrote:
> hans wrote:
> > I suppose this will solve the immediate problem, but it still looks a bit strange.
> > 
> > I'm not very familiar with this code, but don't we know beforehand if the internal mt tool is available and expected to work?
> > 
> > As Rui pointed out, why can't we just do:
> > 
> > ```
> > If the internal mt is present:
> >   If merge fails for some reason:
> >     Report that error and abort
> > Otherwise:
> >   Invoke external mt command, and if it fails, abort
> > ```
> The only way we can tell if it will work is if we see if libxml2 is enabled, but as I said before that is an implementation detail internal to the library that users should not need to know about.  I suppose we can add a cmake variable that has the same value as the libxml2 flag.
I don't think whether a library is available or not is an internal detail of the library. It is in fact a very important information that all users of the library would want to know. Why don't you define something like `bool windows_manifest::WindowsManifestMerger::isAvailable()` which is true if it is available?


https://reviews.llvm.org/D37240