-
Notifications
You must be signed in to change notification settings - Fork 69
feat: bake gapic-generator-java into the hermetic build docker image #3067
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
Changes from 13 commits
8de1c01
15b2fc5
0a95554
034edf9
9795e23
89fc562
1f2ea9e
8af8bec
b5e49dd
1649403
34c5355
651af1e
4ef977b
6e20135
a5c18b2
89e16c1
1cf63ea
74b36d3
e582817
82e6b8e
3f0dc7f
b599d6f
ddb4ff9
6d35e7f
d08e739
eea978d
cdb137e
9b50a73
4ac8caf
4a57a7d
1164694
5a4cfdc
a9257fe
ab7a684
ddaeaf1
62b281d
e0de3c9
1bc545a
9022ba0
6f1e200
bd71775
0e90ddc
25f2d7f
902f656
a06cd0b
7194148
41ab2da
1301b0f
c4933ea
6379c0d
16435c4
1b4a8cc
8e0ddcc
81f6748
35ad97e
32e9af3
d6844ce
d776b32
1de293f
1895792
7d805e2
1eec008
935534e
b01717a
5fb1be8
38b53a2
87b7425
e639d2c
7dd0f3e
6a4f446
b524768
01d8fa0
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -12,11 +12,25 @@ | |||||||||||||||||||||||||||||||||||
# See the License for the specific language governing permissions and | ||||||||||||||||||||||||||||||||||||
# limitations under the License. | ||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||
# install gapic-generator-java in a separate layer so we don't overload the image | ||||||||||||||||||||||||||||||||||||
# with the transferred source code and jars | ||||||||||||||||||||||||||||||||||||
FROM gcr.io/cloud-devrel-public-resources/java21 AS ggj-build | ||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||
WORKDIR /sdk-platform-java | ||||||||||||||||||||||||||||||||||||
COPY . . | ||||||||||||||||||||||||||||||||||||
# {x-version-update-start:gapic-generator-java:current} | ||||||||||||||||||||||||||||||||||||
ENV DOCKER_GAPIC_GENERATOR_VERSION="2.42.1-SNAPSHOT" | ||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||
RUN mvn install -DskipTests -Dclirr.skip -Dcheckstyle.skip | ||||||||||||||||||||||||||||||||||||
RUN ls "/root/.m2/repository/com/google/api/gapic-generator-java/" | ||||||||||||||||||||||||||||||||||||
RUN cp "/root/.m2/repository/com/google/api/gapic-generator-java/${DOCKER_GAPIC_GENERATOR_VERSION}/gapic-generator-java-${DOCKER_GAPIC_GENERATOR_VERSION}.jar" . | ||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Only copying the jar (as opposed to copying the entire There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Does this jar have all the info we need? Can you test running the image without internet? Sometimes the dependencies of the jar are still downloaded. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I ran This function from the transferred synthtool fetches data from Maven:
Our script pulls googleapis' proto definitions: sdk-platform-java/library_generation/utils/utilities.py Lines 175 to 182 in 0ef6619
I thought there was a commit in synthtool that fetches libraries_bom from our config yaml? sdk-platform-java/library_generation/owlbot/synthtool/languages/java.py Lines 280 to 287 in 0ef6619
For the googleapis download, I guess it's expected behavior There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I did a temporary bypass (not committed) to see if the generation is successful. It is except for the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sounds good! Yes we should definitely follow up but I don't think they should block this PR, since we already have all the jars in the docker image. |
||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||
# build from the root of this repo: | ||||||||||||||||||||||||||||||||||||
FROM gcr.io/cloud-devrel-public-resources/python | ||||||||||||||||||||||||||||||||||||
JoeWang1127 marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||
SHELL [ "/bin/bash", "-c" ] | ||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||
ARG OWLBOT_CLI_COMMITTISH=ac84fa5c423a0069bbce3d2d869c9730c8fdf550 | ||||||||||||||||||||||||||||||||||||
ARG PROTOC_VERSION=25.3 | ||||||||||||||||||||||||||||||||||||
ARG GRPC_VERSION=1.65.1 | ||||||||||||||||||||||||||||||||||||
|
@@ -47,7 +61,16 @@ RUN source /src/utils/utilities.sh \ | |||||||||||||||||||||||||||||||||||
ENV DOCKER_GRPC_LOCATION="/grpc/protoc-gen-grpc-java-${GRPC_VERSION}-${OS_ARCHITECTURE}.exe" | ||||||||||||||||||||||||||||||||||||
ENV DOCKER_GRPC_VERSION="${GRPC_VERSION}" | ||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||
# use python 3.11 (the base image has several python versions; here we define the default one) | ||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||
# we transfer gapic-generator-java from the previous stage. | ||||||||||||||||||||||||||||||||||||
# here we redeclare this env var since they are not preserved between stages | ||||||||||||||||||||||||||||||||||||
ENV DOCKER_GAPIC_GENERATOR_VERSION="2.42.1-SNAPSHOT" | ||||||||||||||||||||||||||||||||||||
# {x-version-update-end:gapic-generator-java:current} | ||||||||||||||||||||||||||||||||||||
ENV DOCKER_GAPIC_GENERATOR_LOCATION="/gapic-generator-java/gapic-generator-java-${DOCKER_GAPIC_GENERATOR_VERSION}.jar" | ||||||||||||||||||||||||||||||||||||
COPY --from=ggj-build "/root/.m2/repository/com/google/api/gapic-generator-java/${DOCKER_GAPIC_GENERATOR_VERSION}/gapic-generator-java-${DOCKER_GAPIC_GENERATOR_VERSION}.jar" "${DOCKER_GAPIC_GENERATOR_LOCATION}" | ||||||||||||||||||||||||||||||||||||
RUN chmod 755 "${DOCKER_GAPIC_GENERATOR_LOCATION}" | ||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||
# use python 3.11 (the base image has several python versions; here we define the default one) | ||||||||||||||||||||||||||||||||||||
RUN rm $(which python3) | ||||||||||||||||||||||||||||||||||||
RUN ln -s $(which python3.11) /usr/local/bin/python | ||||||||||||||||||||||||||||||||||||
RUN ln -s $(which python3.11) /usr/local/bin/python3 | ||||||||||||||||||||||||||||||||||||
|
Original file line number | Diff line number | Diff line change | ||
---|---|---|---|---|
|
@@ -158,7 +158,6 @@ download_tools() { | |||
local grpc_version=$3 | ||||
local os_architecture=$4 | ||||
pushd "${output_folder}" | ||||
download_generator_artifact "${gapic_generator_version}" "gapic-generator-java-${gapic_generator_version}.jar" | ||||
diegomarquezp marked this conversation as resolved.
Show resolved
Hide resolved
|
||||
|
||||
# the variable protoc_path is used in generate_library.sh. It is explicitly | ||||
# exported to make clear that it is used outside this utilities file. | ||||
|
@@ -179,6 +178,19 @@ download_tools() { | |||
export grpc_path=$(download_grpc_plugin "${grpc_version}" "${os_architecture}") | ||||
fi | ||||
|
||||
# similar case with gapic-generator-java | ||||
if [[ "${DOCKER_GAPIC_GENERATOR_VERSION}" == "${gapic_generator_version}" ]]; then | ||||
# if the specified gapic_generator_version matches the one baked in the docker | ||||
# container, we copy the generator jar into the output folder so it can be | ||||
# picked up by gapic-generator-java-wrapper | ||||
>&2 echo "Using gapic-generator-java version baked into the container: ${DOCKER_GAPIC_GENERATOR_VERSION}" | ||||
cp "${DOCKER_GAPIC_GENERATOR_LOCATION}" "${output_folder}" | ||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think this copy logic might already be covered here? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I decided to not follow the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I see. Yes it's definitely OK to put it into a different location, as long as we don't use maven to retrieve it. Let's see if we can get rid of There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can we just use the jar as it is without copying? Is it for There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The wrapper assumes the jar to be in CWD
I'm not sure if we can both avoid copying the jar and at the same time not have a Maybe we can copy the jar directly into the home |
||||
else | ||||
download_generator_artifact \ | ||||
"${gapic_generator_version}" \ | ||||
"gapic-generator-java-${gapic_generator_version}.jar" | ||||
fi | ||||
|
||||
popd | ||||
} | ||||
|
||||
|
Uh oh!
There was an error while loading. Please reload this page.