This is an archive of the discontinued LLVM Phabricator instance.

COFF: Give manifest resource file a name.
ClosedPublic

Authored by pcc on Oct 16 2017, 2:05 PM.

Details

Summary

Without this, /linkrepro would create an invalid tar file.

No tests because this requires Windows and the linkrepro tests
require not-Windows.

Diff Detail

Repository
rL LLVM

Event Timeline

pcc created this revision.Oct 16 2017, 2:05 PM
ruiu accepted this revision.Oct 16 2017, 3:03 PM

LGTM

This revision is now accepted and ready to land.Oct 16 2017, 3:03 PM
This revision was automatically updated to reflect the committed changes.
thakis added a subscriber: thakis.Oct 20 2017, 12:10 PM

because this requires Windows an

Why does this require Windows? lld does manifest stuff natively nowadays, so this should be testable on non-Windows I think.

pcc added a comment.Oct 20 2017, 12:42 PM

because this requires Windows an

Why does this require Windows? lld does manifest stuff natively nowadays, so this should be testable on non-Windows I think.

That's true. Our implementation of the Windows maniifest tool appears to require libxml, so I suppose we could gate the test on the libxml2 feature. We might also want to do something like D38977 to test this on Windows as well.

pcc added a comment.Oct 24 2017, 10:02 PM
In D38973#902432, @pcc wrote:

because this requires Windows an

Why does this require Windows? lld does manifest stuff natively nowadays, so this should be testable on non-Windows I think.

That's true. Our implementation of the Windows maniifest tool appears to require libxml, so I suppose we could gate the test on the libxml2 feature. We might also want to do something like D38977 to test this on Windows as well.

I ended up adding a test for this commit in rL316547.