This is an archive of the discontinued LLVM Phabricator instance.

Restore missing runtimes-test-depends target that causes build failures when LLVM_INCLUDE_TESTS is ON
ClosedPublic

Authored by JamesNagurne on Jun 8 2022, 10:54 AM.

Details

Summary
Monorepo commit 7cc8377f, revision D121838 removed the 'runtimes-test-depends' target in runtimes builds that
is assumed to exist when using a bootstrapped runtime build.

For a full analysis, see:
https://discourse.llvm.org/t/looking-for-guidance-on-broken-downstream-bootstrapped-runtimes-builds/62934

The solution here is presented mostly as a stop-gap to get discussion
started, since contacting through Phabricator or Discourse have resulted
in no response.

A true solution would be to determine if runtimes-test-depends still makes
sense and, if not, remove it from the bootstrap runtimes CMakeLists.

Diff Detail

Event Timeline

JamesNagurne created this revision.Jun 8 2022, 10:54 AM
Herald added a project: Restricted Project. · View Herald TranscriptJun 8 2022, 10:54 AM
Herald added a subscriber: mgorny. · View Herald Transcript
JamesNagurne requested review of this revision.Jun 8 2022, 10:54 AM
Herald added projects: Restricted Project, Restricted Project. · View Herald TranscriptJun 8 2022, 10:54 AM
Herald added a reviewer: Restricted Project. · View Herald Transcript
Herald added a reviewer: Restricted Project. · View Herald Transcript
phosek accepted this revision.Jun 8 2022, 11:10 AM

LGTM

ldionne added a subscriber: ldionne.Jun 9 2022, 9:00 AM
ldionne added inline comments.
runtimes/CMakeLists.txt
230

I can't find any mention of LLVM_RUNTIMES_LIT_DEPENDS in the monorepo anymore, so this will always be an empty target. It may "fix" the build in that it won't complain about a missing target anymore, but it doesn't look correct to me. @phosek ?

JamesNagurne added inline comments.Jun 9 2022, 9:07 AM
runtimes/CMakeLists.txt
230

This target is guaranteed by umbrella_lit_testsuite_begin

https://github.com/llvm/llvm-project/blob/4110c2c657b329f91a5c4eef653a15c1a15b61ed/llvm/cmake/modules/AddLLVM.cmake#L1888

I'm not a huge fan of relying on a variable that can't be easily searched, but this is the best facsimile of the original variable that was used as runtimes-test-depends' dependencies

ldionne accepted this revision.Jun 9 2022, 12:52 PM
ldionne added inline comments.
runtimes/CMakeLists.txt
230

Oh, wow, okay then.

This revision is now accepted and ready to land.Jun 9 2022, 12:52 PM
This revision was landed with ongoing or failed builds.Jun 13 2022, 1:53 PM
This revision was automatically updated to reflect the committed changes.