Skip to content

Delete no longer needed artifact_linux dockerfiles#25542

Merged
jtattermusch merged 2 commits intogrpc:masterfrom
jtattermusch:remove_artifact_linux_jessie
Mar 4, 2021
Merged

Delete no longer needed artifact_linux dockerfiles#25542
jtattermusch merged 2 commits intogrpc:masterfrom
jtattermusch:remove_artifact_linux_jessie

Conversation

@jtattermusch
Copy link
Copy Markdown
Contributor

grpc_artifact_centos6 is now used for most of the artifact building and the "build_packages" script actually doesn't do much.

@jtattermusch jtattermusch added the release notes: no Indicates if PR should not be in release notes label Feb 24, 2021
@jtattermusch
Copy link
Copy Markdown
Contributor Author

@jtattermusch
Copy link
Copy Markdown
Contributor Author

@jtattermusch
Copy link
Copy Markdown
Contributor Author

This might be a problem:

+ apt-get install -y python-pip
tools/run_tests/artifacts/build_package_python.sh: line 26: apt-get: command not found

https://fusion2.corp.google.com/invocations/5afb294e-6579-41cc-b031-a350adcfb577/targets/github%2Fgrpc;config=default/tests

@jtattermusch jtattermusch force-pushed the remove_artifact_linux_jessie branch from a0c12d5 to 88fe51d Compare March 3, 2021 15:34
@jtattermusch jtattermusch requested a review from lidizheng March 3, 2021 15:35
@jtattermusch
Copy link
Copy Markdown
Contributor Author

CC @lidizheng please review, @apolcyn owners approval please

@jtattermusch jtattermusch assigned lidizheng and unassigned veblush Mar 3, 2021
@jtattermusch
Copy link
Copy Markdown
Contributor Author

Another invocation of the adhoc artifact - packages - distribtests build
https://fusion2.corp.google.com/invocations/3555b041-d1d1-4fa8-a215-d2b038eadc73/targets

Copy link
Copy Markdown
Contributor

@lidizheng lidizheng left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great improvement! Two Docker images less.

Copy link
Copy Markdown
Contributor Author

@jtattermusch jtattermusch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Forgot to send my clarification on some of the changes. Doing that now.

cp -r "${EXTERNAL_GIT_ROOT}"/input_artifacts/python_*/* artifacts/ || true

apt-get install -y python-pip
python -m pip install -U pip==19.3.1
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

note that pinning the pip version was needed only when running under python2 (since pip has removed python2 support). the manylinux2014 image has a new enough version of pip (and wheel) that we can use.
FTR #25277

python -m pip install -U wheel
export PYTHON=${PYTHON:-python}

strip_binary_wheel() {
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@lidizheng is there a reason why the symbol stripping is being done in the "build_packages" step (and it's the only step there) and not in the "build_artifacts" step (where a few other wheel transformations are already being performed). Should we move "strip_binary_wheel" to the build_artifacts step for consistency?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, I would in favor of that. The stripping step in done in "build_packages", because the name of this step.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sg, I will look into moving the stripping into the artifact build once I have some time (or free to do that yourself).

@jtattermusch
Copy link
Copy Markdown
Contributor Author

https://fusion2.corp.google.com/invocations/3555b041-d1d1-4fa8-a215-d2b038eadc73/targets

I checked the results and there seems to be an unrelated issue with the artifact build on windows (https://source.cloud.google.com/results/invocations/1e3c02cd-719c-46cd-9329-f6678810bc5f) but both the package build tasks and the distribtests are passing, so it should be safe to merge this.

@jtattermusch
Copy link
Copy Markdown
Contributor Author

Known failures: The C# failure is b/158826903 and unrelated to this PR.

@jtattermusch jtattermusch merged commit cb8114d into grpc:master Mar 4, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

release notes: no Indicates if PR should not be in release notes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants