This is an archive of the discontinued LLVM Phabricator instance.

Differential D99217

[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.

Details

Reviewers
gulfem
Group Reviewers
Restricted Project
Commits
rGaa6e4cdd7300: [lld-macho] Fixed lld-version expectation in test so it works on Fuchsia.

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
Herald added a subscriber: llvm-commits. · View Herald Transcript
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
Closed by commit rGaa6e4cdd7300: [lld-macho] Fixed lld-version expectation in test so it works on Fuchsia. (authored by oontvoo). · Explain Why
This revision was automatically updated to reflect the committed changes.
oontvoo added a commit: rGaa6e4cdd7300: [lld-macho] Fixed lld-version expectation in test so it works on Fuchsia..
thakis added a subscriber: thakis.Mar 23 2021, 3:01 PM

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 a subscriber: smeenai.Mar 23 2021, 4:09 PM
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)

smeenai added a comment.Mar 23 2021, 4:10 PM
In D99217#2646114, @thakis wrote:

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?

smeenai added a comment.Mar 23 2021, 4:17 PM
In D99217#2646288, @smeenai wrote:
In D99217#2646114, @thakis wrote:

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)

oontvoo added a comment.Mar 23 2021, 4:44 PM

@smeenai Thanks for the fix!

Harbormaster completed remote builds in B95350: Diff 332797.Mar 23 2021, 7:38 PM

Revision Contents

PathSize
lld/
test/
MachO/
2 lines

Diff 332798

lld/test/MachO/dependency-info.s

Show All 9 Lines
# RUN: %lld -dylib -o %t/libfoo.dylib %t/foo.o # RUN: %lld -dylib -o %t/libfoo.dylib %t/foo.o
# RUN: llvm-mc -filetype=obj -triple=x86_64-apple-macos -o %t/bar.o %t/bar.s # RUN: llvm-mc -filetype=obj -triple=x86_64-apple-macos -o %t/bar.o %t/bar.s
# RUN: llvm-ar csr %t/bar.a %t/bar.o # RUN: llvm-ar csr %t/bar.a %t/bar.o
# RUN: llvm-mc -filetype=obj -triple=x86_64-apple-macos -o %t/main.o %t/main.s # RUN: llvm-mc -filetype=obj -triple=x86_64-apple-macos -o %t/main.o %t/main.s
# RUN: %lld %t/main.o %t/bar.a %t/libfoo.dylib -lSystem -dependency_info %t/deps_info.out # RUN: %lld %t/main.o %t/bar.a %t/libfoo.dylib -lSystem -dependency_info %t/deps_info.out
# RUN: %python %S/Inputs/DependencyDump.py %t/deps_info.out | FileCheck %s # RUN: %python %S/Inputs/DependencyDump.py %t/deps_info.out | FileCheck %s
# CHECK: lld-version: LLD {{.*}} # CHECK: lld-version: {{.*}} LLD {{.*}}
smeenaiUnsubmitted
Not Done
ReplyInline Actions
# RUN: %python %S/Inputs/DependencyDump.py %t/deps_info.out | FileCheck %s
- # CHECK: lld-version: {{.*}} LLD {{.*}}
+ # CHECK: lld-version: {{.*}}LLD {{.*}}
# CHECK-DAG: input-file: {{.*}}/bar.a

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)

smeenai: Tiny nit: I wouldn't have put an additional space here, to account for the case where there's…
smeenaiUnsubmitted
Not Done
ReplyInline Actions

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

smeenai: (ignore this; turns out non-strict whitespace doesn't kick in here)
# CHECK-DAG: input-file: {{.*}}/bar.a # CHECK-DAG: input-file: {{.*}}/bar.a
# CHECK-DAG: input-file: {{.*}}/libfoo.dylib # CHECK-DAG: input-file: {{.*}}/libfoo.dylib
# CHECK-DAG: input-file: {{.*}}/libSystem.tbd # CHECK-DAG: input-file: {{.*}}/libSystem.tbd
# CHECK-DAG: input-file: {{.*}}/main.o # CHECK-DAG: input-file: {{.*}}/main.o
# CHECK-DAG: bar.o # CHECK-DAG: bar.o
# CHECK-NEXT: not-found: {{.*}}/libdyld.dylib # CHECK-NEXT: not-found: {{.*}}/libdyld.dylib
## There could be more not-found here but we are not checking those because it's brittle. ## There could be more not-found here but we are not checking those because it's brittle.
Show All 20 Lines