Details
Diff Detail
- Repository
- rZORG LLVM Github Zorg
Event Timeline
@gkistanova Hi Galina, we are trying to set up a cross-targeting buildbot for AIX platform. And I sent you an email on Wed, Jan 20th, 2021 requesting access for the new build bot.
Can you please confirm that you received it. Thanks.
@Xiangling_L, sorry if this is adding extra respin test cycles. I've done as much review this time around as I can do for the patch as it is currently posted. Thanks.
buildbot/osuosl/master/config/builders.py | ||
---|---|---|
635 | Minor comment: Usage of the "ppc" tag seems to imply it is a generic tag for anything Power-related. It makes sense to add it here. | |
643 | Not sure if other people have a different opinion about this, but I think being explicit for some settings used by "nearby" entries make sense. | |
646 | Most configurations using involving Ninja appear to use getCmakeWithNinjaBuildFactory. The ones that don't mostly also have make='ninja'. | |
647 | My understanding is that "llvm" is not needed in the LLVM_ENABLE_PROJECTS list. I guess we could keep it, but I don't think we should keep the trailing semicolon. |
buildbot/osuosl/master/config/builders.py | ||
---|---|---|
646 | Generator options already seem to exist in the ClangBuilder implementation, so I don't think we should be passing them here. | |
656 | The binutils directory here is used for the gold plugin, so we don't need it for AIX. | |
657 | Looks like the jobs property on the worker will set the lit threads, so we don't need that part I think. | |
buildbot/osuosl/master/config/workers.py | ||
121 | Just noting we'll have to boost this when we add the native builder for example (we're still waiting on more machine resources though). |
LGTM with some last changes based on @daltenty's comments.
buildbot/osuosl/master/config/builders.py | ||
---|---|---|
641 | The make='ninja' line is not needed given @daltenty's comment. | |
646 | @daltenty, just confirming you are saying that https://reviews.llvm.org/source/zorg/browse/main/zorg/buildbot/builders/ClangBuilder.py gives us Ninja by default? |
buildbot/osuosl/master/config/builders.py | ||
---|---|---|
646 |
Certainly seems that way, every generator option is hard coded to Ninja |
Almost there. One more change it will be ready to land.
buildbot/osuosl/master/config/builders.py | ||
---|---|---|
649 | No need to specify the LLVM_ENABLE_PROJECTS. The build factory will take care of this for you. |
buildbot/osuosl/master/config/builders.py | ||
---|---|---|
649 | Thanks, I will update the patch accordingly. |
I think having the host platform described more clearly makes the non-native default target more obvious.