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.

Download Raw Diff

Details

Reviewers

gulfem

Group Reviewers

Restricted Project

Commits

rGaa6e4cdd7300: [lld-macho] Fixed lld-version expectation in test so it works on Fuchsia.

Diff Detail

Repository: rG LLVM Github Monorepo

Unit TestsFailed

	Time	Test
	110 ms	x64 debian > lld.MachO::dependency-info.s

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

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)

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?

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)

@smeenai Thanks for the fix!

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

Revision Contents

Path

Size

lld/

test/

MachO/

dependency-info.s

2 lines

Diff 332797

lld/test/MachO/dependency-info.s

Show All 9 Lines

# 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-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: %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

# CHECK: lld-version: LLD {{.*}}

# CHECK: lld-version: {{.*}} LLD {{.*}}

smeenaiUnsubmitted

Not Done

# 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

(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: {{.*}}/libfoo.dylib

# CHECK-DAG: input-file: {{.*}}/libSystem.tbd

# CHECK-DAG: input-file: {{.*}}/main.o

# CHECK-DAG: bar.o

# CHECK-NEXT: not-found: {{.*}}/libdyld.dylib

## There could be more not-found here but we are not checking those because it's brittle.

Show All 20 Lines