Page MenuHomePhabricator

[lld-macho] Fixed lld-version expectation in test so it works on Fuchsia. On Fuchsia, it's called Fuchsia LLD
ClosedPublic

Authored by oontvoo on Mar 23 2021, 2:52 PM.

Diff Detail

Event Timeline

oontvoo created this revision.Mar 23 2021, 2:52 PM
Herald added a project: Restricted Project. · View Herald TranscriptMar 23 2021, 2:52 PM
Herald added a reviewer: Restricted Project. · View Herald Transcript
oontvoo requested review of this revision.Mar 23 2021, 2:52 PM
Herald added a project: Restricted Project. · View Herald TranscriptMar 23 2021, 2:52 PM
gulfem accepted this revision.Mar 23 2021, 2:54 PM
This revision is now accepted and ready to land.Mar 23 2021, 2:54 PM
This revision was landed with ongoing or failed builds.Mar 23 2021, 2:56 PM
This revision was automatically updated to reflect the committed changes.

This seems to break the test on linux: http://45.33.8.238/linux/42505/step_11.txt

Probably because this now needs two spaces between : and LLD

smeenai added inline comments.
lld/test/MachO/dependency-info.s
18

Tiny nit: I wouldn't have put an additional space here, to account for the case where there's no vendor in front of the "LLD". FileCheck uses non-strict whitespace by default though, so this also works. (See https://llvm.org/docs/CommandGuide/FileCheck.html#cmdoption-filecheck-strict-whitespace)

This seems to break the test on linux: http://45.33.8.238/linux/42505/step_11.txt

Probably because this now needs two spaces between : and LLD

Oops, I missed your comment before making mine. I thought not having strict whitespace would take care of this, but maybe not?

This seems to break the test on linux: http://45.33.8.238/linux/42505/step_11.txt

Probably because this now needs two spaces between : and LLD

Oops, I missed your comment before making mine. I thought not having strict whitespace would take care of this, but maybe not?

Okay, yeah, it was broken. Pushed rG48d9b2fd8ea3 to fix.

lld/test/MachO/dependency-info.s
18

(ignore this; turns out non-strict whitespace doesn't kick in here)

@smeenai Thanks for the fix!