From 2f0fea69b6ae56dba965ea8817750793862b3b2d Mon Sep 17 00:00:00 2001 From: sofisl <55454395+sofisl@users.noreply.github.com> Date: Tue, 28 Nov 2023 12:21:02 -0800 Subject: [PATCH] Revert "chore: remove compile protos from post processor" (#1904) Revert "chore: remove compile protos from post processor (#1899)" This reverts commit 73248044166b6ba2dd102862f8cd2c4829e868db. --- docker/owlbot/nodejs_mono_repo/Dockerfile | 5 ++-- synthtool/languages/node.py | 27 +++++++++++++++++ synthtool/languages/node_mono_repo.py | 36 +++++++++++++++++++++++ tests/test_node.py | 7 +++++ tests/test_node_mono_repo.py | 24 +++++++++++++++ 5 files changed, 97 insertions(+), 2 deletions(-) diff --git a/docker/owlbot/nodejs_mono_repo/Dockerfile b/docker/owlbot/nodejs_mono_repo/Dockerfile index 925f9de5e..1b674bf2b 100644 --- a/docker/owlbot/nodejs_mono_repo/Dockerfile +++ b/docker/owlbot/nodejs_mono_repo/Dockerfile @@ -51,7 +51,8 @@ RUN find /synthtool -type d -exec chmod a+x {} \; # Install dependencies used for post processing: # * gts/typescript are used for linting. -RUN cd /synthtool && mkdir node_modules && npm i gts@5.0.0 \ - typescript@5.1.6 @google-cloud/typeless-sample-bot@1.3.3 +# * google-gax and gapic-tools are used for compiling protos. +RUN cd /synthtool && mkdir node_modules && npm i gts@5.0.0 google-gax@4.0.3 \ + typescript@5.1.6 @google-cloud/typeless-sample-bot@1.3.3 gapic-tools@0.1.8 ENTRYPOINT [ "/bin/bash", "/synthtool/docker/owlbot/nodejs_mono_repo/entrypoint.sh" ] diff --git a/synthtool/languages/node.py b/synthtool/languages/node.py index ec3d9c02d..9f3091d2e 100644 --- a/synthtool/languages/node.py +++ b/synthtool/languages/node.py @@ -234,16 +234,43 @@ def fix_hermetic(hide_output=False): ) +def compile_protos(hide_output=False): + """ + Compiles protos into .json, .js, and .d.ts files using + compileProtos script from google-gax. + """ + logger.debug("Compiling protos...") + shell.run(["npx", "compileProtos", "src"], hide_output=hide_output) + + +# TODO: delete these functions if it turns out we no longer +# need them to be hermetic. +def compile_protos_hermetic(hide_output=False): + """ + Compiles protos into .json, .js, and .d.ts files using + compileProtos script from google-gax. Assumes that compileProtos + is already installed in a well known location on disk (node_modules/.bin). + """ + logger.debug("Compiling protos...") + shell.run( + ["node_modules/.bin/compileProtos", "src"], + check=True, + hide_output=hide_output, + ) + + def postprocess_gapic_library(hide_output=False): logger.debug("Post-processing GAPIC library...") install(hide_output=hide_output) fix(hide_output=hide_output) + compile_protos(hide_output=hide_output) logger.debug("Post-processing completed") def postprocess_gapic_library_hermetic(hide_output=False): logger.debug("Post-processing GAPIC library...") fix(hide_output=hide_output) + compile_protos(hide_output=hide_output) logger.debug("Post-processing completed") diff --git a/synthtool/languages/node_mono_repo.py b/synthtool/languages/node_mono_repo.py index 3ab018f01..74191d2c1 100644 --- a/synthtool/languages/node_mono_repo.py +++ b/synthtool/languages/node_mono_repo.py @@ -335,16 +335,52 @@ def fix_hermetic(relative_dir, hide_output=False): ) +def compile_protos(hide_output=False, is_esm=False): + """ + Compiles protos into .json, .js, and .d.ts files using + compileProtos script from google-gax. + """ + logger.debug("Compiling protos...") + command = ( + ["npx", "compileProtos", "src"] + if not is_esm + else ["npx", "compileProtos", "esm/src", "--esm"] + ) + shell.run(command, hide_output=hide_output) + + +def compile_protos_hermetic(relative_dir, is_esm=False, hide_output=False): + """ + Compiles protos into .json, .js, and .d.ts files using + compileProtos script from google-gax. Assumes that compileProtos + is already installed in a well known location on disk (node_modules/.bin). + """ + logger.debug("Compiling protos...") + command = ( + [f"{_TOOLS_DIRECTORY}/node_modules/.bin/compileProtos", "esm/src", "--esm"] + if not is_esm + else [f"{_TOOLS_DIRECTORY}/node_modules/.bin/compileProtos", "esm/src", "--esm"] + ) + shell.run( + command, + cwd=relative_dir, + check=True, + hide_output=hide_output, + ) + + def postprocess_gapic_library(hide_output=False, is_esm=False): logger.debug("Post-processing GAPIC library...") install(hide_output=hide_output) fix(hide_output=hide_output) + compile_protos(hide_output=hide_output, is_esm=is_esm) logger.debug("Post-processing completed") def postprocess_gapic_library_hermetic(relative_dir, hide_output=False, is_esm=False): logger.debug("Post-processing GAPIC library...") fix_hermetic(relative_dir, hide_output=hide_output) + compile_protos_hermetic(relative_dir, hide_output=hide_output, is_esm=is_esm) logger.debug("Post-processing completed") diff --git a/tests/test_node.py b/tests/test_node.py index 297fee1af..cd48d62f2 100644 --- a/tests/test_node.py +++ b/tests/test_node.py @@ -169,12 +169,19 @@ def test_fix(self, shell_run_mock): calls = shell_run_mock.call_args_list assert any(["npm run fix" in " ".join(call[0][0]) for call in calls]) + @patch("synthtool.shell.run") + def test_compile_protos(self, shell_run_mock): + node.compile_protos() + calls = shell_run_mock.call_args_list + assert any(["npx compileProtos src" in " ".join(call[0][0]) for call in calls]) + @patch("synthtool.shell.run") def test_postprocess_gapic_library(self, shell_run_mock): node.postprocess_gapic_library() calls = shell_run_mock.call_args_list assert any(["npm install" in " ".join(call[0][0]) for call in calls]) assert any(["npm run fix" in " ".join(call[0][0]) for call in calls]) + assert any(["npx compileProtos src" in " ".join(call[0][0]) for call in calls]) # postprocess_gapic_library_hermetic() must be mocked because it depends on node modules diff --git a/tests/test_node_mono_repo.py b/tests/test_node_mono_repo.py index ce414a070..c91373878 100644 --- a/tests/test_node_mono_repo.py +++ b/tests/test_node_mono_repo.py @@ -354,12 +354,30 @@ def test_fix(self, shell_run_mock): calls = shell_run_mock.call_args_list assert any(["npm run fix" in " ".join(call[0][0]) for call in calls]) + @patch("synthtool.shell.run") + def test_compile_protos(self, shell_run_mock): + node_mono_repo.compile_protos() + calls = shell_run_mock.call_args_list + assert any(["npx compileProtos src" in " ".join(call[0][0]) for call in calls]) + + @patch("synthtool.shell.run") + def test_compile_protos_esm(self, shell_run_mock): + node_mono_repo.compile_protos(is_esm=True) + calls = shell_run_mock.call_args_list + assert any( + [ + "npx compileProtos esm/src --esm" in " ".join(call[0][0]) + for call in calls + ] + ) + @patch("synthtool.shell.run") def test_postprocess_gapic_library(self, shell_run_mock): node_mono_repo.postprocess_gapic_library() calls = shell_run_mock.call_args_list assert any(["npm install" in " ".join(call[0][0]) for call in calls]) assert any(["npm run fix" in " ".join(call[0][0]) for call in calls]) + assert any(["npx compileProtos src" in " ".join(call[0][0]) for call in calls]) @patch("synthtool.shell.run") def test_postprocess_gapic_library_esm(self, shell_run_mock): @@ -367,6 +385,12 @@ def test_postprocess_gapic_library_esm(self, shell_run_mock): calls = shell_run_mock.call_args_list assert any(["npm install" in " ".join(call[0][0]) for call in calls]) assert any(["npm run fix" in " ".join(call[0][0]) for call in calls]) + assert any( + [ + "npx compileProtos esm/src --esm" in " ".join(call[0][0]) + for call in calls + ] + ) # postprocess_gapic_library_hermetic() must be mocked because it depends on node modules