Page MenuHomePhabricator

[zorg] Adjust libcxx buildbot config on AIX
ClosedPublic

Authored by Xiangling_L on Feb 23 2021, 12:16 PM.

Diff Detail

Event Timeline

Xiangling_L requested review of this revision.Feb 23 2021, 12:16 PM
Xiangling_L created this revision.
daltenty edited the summary of this revision. (Show Details)Feb 26 2021, 11:11 AM
daltenty added inline comments.Feb 26 2021, 11:17 AM
buildbot/osuosl/master/config/builders.py
1312

See comment below.

1313

I think this should be a binary option to select the new "Unified Standalone" build configuration. We can then just set the correct src_root inside the factory rather than having to pass this explicitly.

zorg/buildbot/builders/LibcxxAndAbiBuilder.py
77 ↗(On Diff #325871)

We could just append to LLVM_ENABLE_RUNTIMES instead here, if we are in the Unified Standalone build configuration

Xiangling_L marked 3 inline comments as done.

Use a boolean flag instead to control if building a unified standalone runtimes;

daltenty accepted this revision.Mar 1 2021, 11:30 AM

LGTM, thanks!

This revision is now accepted and ready to land.Mar 1 2021, 11:30 AM
gkistanova reopened this revision.Mar 2 2021, 6:14 PM

I'm not sure I follow what are you trying to achieve by changing the src_root.

Changing src_root would not give you ../llvm/runtimes, it simply changes where the source code gets checked out to. Like <buildbot-worker-directory>/<builder>/runtimes/llvm-project/<the rest of the monorepo tree>.

zorg/buildbot/builders/LibcxxAndAbiBuilder.py
15 ↗(On Diff #327283)

There is a mistype here. Should be a comma at the end.
I have fixed this for you with 4cd2689758e95b6811dae41ef9abec92f71e03a7.

81 ↗(On Diff #327283)

f.depends_on_projects could contain more than just runtimes.

This revision is now accepted and ready to land.Mar 2 2021, 6:14 PM
gkistanova requested changes to this revision.Mar 2 2021, 6:15 PM

It seems this patch needs more attention.
Please see my comments above.

This revision now requires changes to proceed.Mar 2 2021, 6:15 PM
Xiangling_L marked an inline comment as done.Mar 3 2021, 7:33 AM

I'm not sure I follow what are you trying to achieve by changing the src_root.

Changing src_root would not give you ../llvm/runtimes, it simply changes where the source code gets checked out to. Like <buildbot-worker-directory>/<builder>/runtimes/llvm-project/<the rest of the monorepo tree>.

Thanks for pointing this out. What we are trying to do is to specify runtimes project[https://github.com/llvm/llvm-project/tree/main/runtimes] as the source to cmake and enable the building of libcxx;libcxxabi;libunwind with -DLLVM_ENABLE_RUNTIMES="libcxx;libcxxabi;libunwind" to build a standalone runtimes.

The cmake command I am expecting is something similar to:

cmake ../llvm/runtimes ' -DCMAKE_BUILD_TYPE=Release -DCMAKE_C_COMPILER=gcc -DCMAKE_CXX_COMPILER=g++ -DCMAKE_AR=/usr/bin/ar -DLIBCXX_CXX_ABI=libcxxabi -DLLVM_ENABLE_RUNTIMES="libcxx;libcxxabi;libunwind"

Could you point to me what is the proper way to do this?

Adjust the build_standalone implementation.

I have added the support you need to UnifiedTreeBuilder.

You could pass src_to_build_dir='runtimes' to UnifiedTreeBuilder.getLLVMBuildFactoryAndSourcecodeSteps( to have what you are after.

Pass runtimes as src_bo_build_dir when building standalone runtimes

Almost there.
One problem to fix.

zorg/buildbot/builders/LibcxxAndAbiBuilder.py
35 ↗(On Diff #332288)

src_to_build_dir is not defined if the branch is not taken here.

41 ↗(On Diff #332288)

How about something like

src_to_build_dir='runtimes' if build_standalone else None,

and getting rid of that if above?

Or if you prefer having if to highlight the logic, then you need to initialize src_to_build_dir to None right before that if.

Xiangling_L marked 2 inline comments as done.

Addressed the comments;

This revision is now accepted and ready to land.Thu, Mar 25, 12:02 PM
Xiangling_L planned changes to this revision.Fri, Mar 26, 6:31 AM

LGTM

Thank you so much for the review. However we have some strategy adjustment at this moment and may need to further change our buildbot config. So I'd like to suspend this patch for the moment. Sorry for the inconvenience.

Adjust the libcxx buildbot config to enable libcxx project only;
But still keep adding build_standalone functionalities to LibcxxAndAbiBuilder;

This revision is now accepted and ready to land.Fri, Mar 26, 1:18 PM

Looks fine.

However, now the change to buildbot/osuosl/master/config/builders.py and the change to zorg/buildbot/builders/LibcxxAndAbiBuilder.py are not related.
Could you break them to 2 separate patches, please?

Looks fine.

However, now the change to buildbot/osuosl/master/config/builders.py and the change to zorg/buildbot/builders/LibcxxAndAbiBuilder.py are not related.
Could you break them to 2 separate patches, please?

Sure will do.

Xiangling_L edited the summary of this revision. (Show Details)

Split this patch into two;

This revision was landed with ongoing or failed builds.Wed, Apr 7, 6:59 AM
This revision was automatically updated to reflect the committed changes.