This is an archive of the discontinued LLVM Phabricator instance.

Make standalone-build-x86_64 a standalone builder
ClosedPublic

Authored by kwk on May 5 2022, 5:10 AM.

Details

Summary

I've written an annotated bash script that will be used when running
builds of the standalone-build-x86_64 builder.

The bash script builds LLVM and installs it into a directory for later
consumption when building Clang independently.

Diff Detail

Event Timeline

kwk created this revision.May 5 2022, 5:10 AM
Herald added a project: Restricted Project. · View Herald Transcript
Herald added a subscriber: pengfei. · View Herald Transcript
kwk requested review of this revision.May 5 2022, 5:10 AM
kwk edited the summary of this revision. (Show Details)May 9 2022, 2:43 AM
tstellar added inline comments.May 9 2022, 9:30 AM
zorg/buildbot/builders/annotated/standalone-build.sh
47

We should wait until this patch is no longer needed before we commit this change.

72

Here are the minimal set of cmake args that should work:

-DCMAKE_BUILD_TYPE=Release \
-DLLVM_BUILD_LLVM_DYLIB=ON \
-DLLVM_LINK_LLVM_DYLIB=ON \
-DLLVM_INCLUDE_BENCHMARKS=OFF \
-DLLVM_INSTALL_UTILS=ON \
-DCMAKE_INSTALL_PREFIX=$install_prefix
158

Minimal CMake args for clang:

-DCMAKE_BUILD_TYPE=Release \
-DCLANG_LINK_CLANG_DYLIB=ON \
-DCLANG_INCLUDE_TESTS=ON \
-DLLVM_EXTERNAL_LIT=/usr/bin/lit \
-DCMAKE_INSTALL_PREFIX=$install_prefix \
-DLLVM_ROOT=$install_prefix \
kwk added a comment.May 9 2022, 12:35 PM

@tstellar I'm trying your minimal config now on my build machine. I thought you wanted more details which is why I've taken the settings from Copr builds.

zorg/buildbot/builders/annotated/standalone-build.sh
47

We should wait until this patch is no longer needed before we commit this change.

No, I disagree with waiting because the patch is from Feb 22nd. How long do you want to wait? Maybe by showing that we need it we can have it land more quickly.

kwk added inline comments.May 9 2022, 12:37 PM
zorg/buildbot/builders/annotated/standalone-build.sh
158

Don't you think we should use the just installed lit as in -DLLVM_EXTERNAL_LIT=${LLVM_INSTALL_DIR}/bin/lit?

kwk updated this revision to Diff 428179.May 9 2022, 1:00 PM
tstellar added inline comments.May 9 2022, 1:03 PM
zorg/buildbot/builders/annotated/standalone-build.sh
47

Is clang-tools-extra even being built?

158

@kwk Yes, that's fine.

kwk updated this revision to Diff 428184.May 9 2022, 1:08 PM
kwk marked 3 inline comments as done.
  • Remove no longer needed export
kwk added inline comments.May 9 2022, 1:08 PM
zorg/buildbot/builders/annotated/standalone-build.sh
47

Well, without the patch things won't build:

CMake Error at /home/fedora/llvm-build-standalone/llvm-install/lib64/cmake/llvm/AddLLVM.cmake:1820 (add_dependencies):
  The dependency target "LLVMHello" of target "check-all" does not exist.
Call Stack (most recent call first):
  CMakeLists.txt:596 (add_lit_target)
nikic added inline comments.May 9 2022, 1:12 PM
zorg/buildbot/builders/annotated/standalone-build.sh
47

Maybe specify LLVM_MAIN_SRC_DIR until this is resolved?

tstellar added inline comments.May 9 2022, 1:15 PM
zorg/buildbot/builders/annotated/standalone-build.sh
47

Is it automatically building clang-tools-extra, because the source directory is there? I think we should be removing directories that we don't need so that we can catch when new dependencies are introduced between directories. We can use sparse-checkouts for this to make it easier.

kwk updated this revision to Diff 428187.May 9 2022, 1:16 PM
  • Patch not needed with minimal config and LLVM_ROOT
kwk edited the summary of this revision. (Show Details)May 9 2022, 1:16 PM

@nikic @tstellar The patch is no longer needed because of the changes to the minimal config suggested by @tstellar . I hope we can get it in now.

zorg/buildbot/builders/annotated/standalone-build.sh
47

No please no. I've rebuilt everything and the minimal config you suggested no longer produced the error anymore. This can go in without any patch whatsoever ;)

tstellar added inline comments.May 9 2022, 1:20 PM
zorg/buildbot/builders/annotated/standalone-build.sh
47

I still think we should use sparse-checkouts to only checkout the directories we need, but if you want to do that as a follow on patch, that's fine.

zorg/buildbot/builders/annotated/standalone-build.sh
58

Do we want -DLLVM_INCLUDE_TOOLS:BOOL=ON -DLLVM_BUILD_TOOLS:BOOL=ON ?

92

Do we want to cleanup all build / install dir here to decrease pressure on filesystem or is it fine?

kwk added inline comments.May 10 2022, 12:50 AM
zorg/buildbot/builders/annotated/standalone-build.sh
47

I still think we should use sparse-checkouts to only checkout the directories we need, but if you want to do that as a follow on patch, that's fine.

I can put the sparse checkout in this patch, that's fine.

92

Do we want to cleanup all build / install dir here to decrease pressure on filesystem or is it fine?

Do you ask for why we clean up or why we don't clean up after the install?

We clean up before the install to make sure that whatever is installed contains no relics.

kwk updated this revision to Diff 428304.May 10 2022, 1:12 AM
kwk edited the summary of this revision. (Show Details)
  • Sparse checkout only the directories needed
kwk marked 5 inline comments as done.May 10 2022, 1:18 AM

@tstellar sparse checkout is implemented

Looks good to me. Please wait for confirmation by @tstellar though.

This revision is now accepted and ready to land.May 10 2022, 2:19 AM
kwk updated this revision to Diff 428371.May 10 2022, 7:23 AM
  • Organize build script with functions
tstellar accepted this revision.May 11 2022, 9:08 AM
This revision was automatically updated to reflect the committed changes.
nikic added inline comments.May 30 2022, 3:24 AM
zorg/buildbot/builders/annotated/standalone-build.sh
107

What does the LLVM_ROOT variable do? I couldn't find any uses of it in cmake files.