[PATCH] D14866: [zorg] Add support for uploading artifacts to the 'llvmlab bisect' bucket and enable this for clang-cmake-mips.

Galina via llvm-commits llvm-commits at lists.llvm.org
Wed Dec 2 10:49:37 PST 2015


gkistanova added a comment.

Sorry for not being clear from the beginning.
Anyway, your interpretation is correct. I meant having a separate get<something>BuildFactory(...) function for the builders which would upload blobs.

> My only interpretation of Galina's suggestion was to create a new function called getClangGCSBuildFactory(...) that calls

>  getClangBuildFactory(...) and then adds the extra upload step.


Unfortunately, with the current getClangBuildFactory design this is not that simple. To do that right would require a relatively large refactoring of the getClangBuildFactory, which I don't want to force Daniel do just to get this patch in.

So, adding a new get<something>BuildFactory(...) which would simply call the changed getClangBuildFactory or copying and changing the actually needed code from the getClangBuildFactory would do for now.

> I don't see what benefit that would bring TBH. Hopefully Galina can clarify.


There are 2 issues here:

1. Having the getClangBuildFactory changed this way, we could all of a sudden get a lot of builders uploading or trying to upload blobs by just changing the default value of one of the many optional parameters. This might be something the buildslave owners didn't consider and have setup for in the first place.

2. The getClangBuildFactory already due for refactoring, and adding more parameters there is not a good idea in general. Though, this is acceptable in this particular case as a "till-the-next-refactoring" solution, but I'd keep the dependencies as small and clear as possible.

Hope this make sense.


http://reviews.llvm.org/D14866





More information about the llvm-commits mailing list