Page MenuHomePhabricator

[libc builder] Use AnnotatedBuilder instead of LLVMBuilder.
Needs ReviewPublic

Authored by sivachandra on Nov 1 2019, 12:00 PM.

Details

Reviewers
gkistanova

Event Timeline

sivachandra created this revision.Nov 1 2019, 12:00 PM
sivachandra marked an inline comment as done.Nov 1 2019, 12:03 PM

This part of the fix you wanted me to do in https://reviews.llvm.org/D69655

zorg/buildbot/builders/AnnotatedBuilder.py
46 ↗(On Diff #227502)

I am not sure if this is correct, but the comment seems to indicate it has to under if clean.

gkistanova requested changes to this revision.Nov 4 2019, 8:55 PM

Hello Siva,

How AnnotatedBuilder handles clean arg doesn't seem related to adding libc builders. May I suggest removing this change from your patch and make a separate review item if you want to change that behavior, please?

You do not need LibcBuilder factory at all. Just use that AnnotatedBuilder.getAnnotatedBuildFactory in the builders.py for your build configurations. The same is for the z`org/buildbot/builders/libc/libc_vars.py` file, which is not used, unless I'm missing something in your patch.

To add your libc builders you just need to change buildbot/osuosl/master/config/builders.py using nnotatedBuilder.getAnnotatedBuildFactory there, buildbot/osuosl/master/config/slaves.py to add your computer, and buildbot/osuosl/master/config/status.py for notifier. Very close to what you had for these files in https://reviews.llvm.org/D69655.

I think I got the idea of what you are trying to do and can propose a patch if you would like me to. Unless you want to do that by yourself, of course.

This revision now requires changes to proceed.Nov 4 2019, 8:55 PM

Use the module libc_vars.

Hello Siva,

How AnnotatedBuilder handles clean arg doesn't seem related to adding libc builders. May I suggest removing this change from your patch and make a separate review item if you want to change that behavior, please?

I have removed that change from this patch now.

You do not need LibcBuilder factory at all. Just use that AnnotatedBuilder.getAnnotatedBuildFactory in the builders.py for your build configurations. The same is for the z`org/buildbot/builders/libc/libc_vars.py` file, which is not used, unless I'm missing something in your patch.

That was my mistake to not have used libc_vars. Since I want the ability to pass different params to the annotation script, I created this additional module for the names of the env vars that will be set. I created a new builder function for libc also for the same reason (which is to be able to parametrize the builder.)

I think I got the idea of what you are trying to do and can propose a patch if you would like me to. Unless you want to do that by yourself, of course.

If this latest update does not suit you, feel free to land a change for me.