[PATCH] D108628: [lld/COFF] Improve handling of the /manifestdependency: flag

[Nico Weber via Phabricator via llvm-commits]

thakis added a comment.

I didn't see the test failure because linkrepro-manifest.test requires gnutar, which on macOS apparently isn't present (only BSD tar is). The test runs fine without this patch on macOS though; maybe that REQUIRES is overly strict.

Anyways, that test failure is real and kind of annoying. The problem is that my patch moves `addBuffer(createManifestRes(), false, false);` after the response file has been written, and so the .res file that is internally created by that is added to `filePaths` only after `filePaths` has been copied to the response file.

However, `/manifestdependency:` in input files means we can't create the inputs to the .res file much earlier. So we have to call `createResponseFile()` later I suppose, but there are several early exits (for writing import .lib files, or for writing thin indexes only) that then also have to call it, which is a bit messy.

Another option, which seems maybe a bit nicer and more consistent with the change here, is to undo D38975 <https://reviews.llvm.org/D38975> and instead of putting the generated `.res` file in the response file, put the input flags in it instead. We have llvm-cvtres now, so we no longer need cvtres.exe to link repro files that bundle the inputs.

I like that last idea most.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D108628/new/

https://reviews.llvm.org/D108628