-
Notifications
You must be signed in to change notification settings - Fork 2.4k
Build base-builder-clang-full on GCB #13225
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
First attempt at building base-builder-clang-full. We need a good amount of special handling for it.
/gcbrun trial_build.py skcms |
/gcbrun trial_build.py skcms |
/gcbrun trial_build.py skcms |
@@ -14,7 +14,8 @@ | |||
# | |||
################################################################################ | |||
|
|||
FROM gcr.io/oss-fuzz-base/base-clang | |||
ARG BASE_IMAGE=gcr.io/oss-fuzz-base/base-clang |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do we need this?
I think for now, most of our anticipated usages can just rely on base-clang-full directly?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe we can figure this out tomorrow, but based on my understanding I think we do need it, isn't the indexer going to be needed to run in the same image building the project? Maybe this assumption I made is wrong? I'll put up my next PR, it will become clearer what I'm trying to do.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IMO, we can just pull out the indexer binary and inject it into a regular oss-fuzz project build. IIRC the only runtime dependency it has a LLVM/C headers which are also there in the regular base-clang.
There should be no need to rebase the project image on top of these, so we can keep things simpler.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
WDYT I wait to use the indexer then before merging either approach? I just don't know if this is feasible. But if you've done it, we can go with the approach your recommending if you think it works.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah, let's try the static indexer binary approach -- if that fails we can come back to this. We can avoid some complexity this way
|
||
# Build base-clang-full and base-builder-clang-full, push the latter. Do this | ||
# here, because they need special handling due to their use of build args. | ||
clang_image_name = f'{tag_prefix}base-clang-full' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it's probably a lot cleaner to add support for build-args
in get_docker_build_step
instead of hardcoding this special case here, and change
BASE_IMAGES = [
'base-image',
'base-clang',
'base-builder',
'base-builder-go',
'base-builder-javascript',
'base-builder-jvm',
'base-builder-python',
'base-builder-ruby',
'base-builder-rust',
'base-builder-ruby',
'base-builder-swift',
'base-runner',
'base-runner-debug',
]
to something like:
BASE_IMAGES = [
('base-image', None)
('base-clang-full', {'FULL_LLVM_BUILD': '1'}),
]
oss-fuzz/infra/build/functions/build_lib.py
Line 415 in d3af01d
args.extend(['--build-arg', f'CACHE_IMAGE={cache_image}']) |
WDYT?
Build and push base-builder-clang-full without clobbering the regular base-clang or base-builder images.
This will be used for running the extra tools on fuzzing builds.