Skip to content

Remove git and python packages from the docker images used by Zip-Nuget-Java-Nodejs Packaging Pipeline#11651

Merged
snnn merged 11 commits intomasterfrom
snnn/update_git
Jun 4, 2022
Merged

Remove git and python packages from the docker images used by Zip-Nuget-Java-Nodejs Packaging Pipeline#11651
snnn merged 11 commits intomasterfrom
snnn/update_git

Conversation

@snnn
Copy link
Contributor

@snnn snnn commented May 27, 2022

Description:
Remove git from the docker images used by "Zip-Nuget-Java-Nodejs Packaging Pipeline". For unknown reason, the git224 package is gone. While I can upgrade it to git236, I think a better choice is to just remove it.

Remove python packages also. To reduce the number of OSS components we need to maintain for compliance purpose.

The other changes in this PR are just code refactoring.

Motivation and Context

  • Why is this change required? What problem does it solve?

To resolve the build error in https://aiinfra.visualstudio.com/Lotus/_build/results?buildId=209965&view=logs&j=714209ae-2e3b-5c94-7779-6bb56a181c3b&t=4dc3fda7-6978-52ea-4a7e-758a98a77dd1

  • If it fixes an open issue, please link to the issue here.

@snnn snnn changed the title Update git from 2.24 to 2.36 Remove git from the docker images used by Zip-Nuget-Java-Nodejs Packaging Pipeline May 27, 2022
@snnn snnn closed this May 28, 2022
@snnn
Copy link
Contributor Author

snnn commented May 28, 2022

Need more work.

@snnn snnn reopened this Jun 2, 2022
@snnn snnn marked this pull request as draft June 2, 2022 02:41
@snnn snnn force-pushed the snnn/update_git branch 3 times, most recently from b075cb2 to 01aae17 Compare June 2, 2022 21:51
@snnn snnn marked this pull request as ready for review June 2, 2022 22:04
@snnn snnn force-pushed the snnn/update_git branch from 72c6268 to 927c82c Compare June 3, 2022 07:47
@snnn snnn requested a review from a team as a code owner June 3, 2022 19:37
@snnn snnn force-pushed the snnn/update_git branch from 7fece7b to 927c82c Compare June 3, 2022 23:44
@snnn snnn changed the title Remove git from the docker images used by Zip-Nuget-Java-Nodejs Packaging Pipeline Remove git and python packages from the docker images used by Zip-Nuget-Java-Nodejs Packaging Pipeline Jun 3, 2022
@@ -0,0 +1,71 @@
#!/bin/bash
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This file is identical with the one in aarch64 folder.

Copy link
Contributor

Choose a reason for hiding this comment

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

do they need to be kept consistent? is there a way to avoid duplication?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Use a script to generate them.

Context: tools/ci_build/github/linux/docker
DockerBuildArgs: "--network=host --build-arg POLICY=manylinux2014 --build-arg PLATFORM=x86_64 --build-arg BASEIMAGE=nvidia/cuda:11.4.0-cudnn8-devel-centos7 --build-arg DEVTOOLSET_ROOTPATH=/opt/rh/devtoolset-10/root --build-arg PREPEND_PATH=/opt/rh/devtoolset-10/root/usr/bin: --build-arg LD_LIBRARY_PATH_ARG=/opt/rh/devtoolset-10/root/usr/lib64:/opt/rh/devtoolset-10/root/usr/lib:/opt/rh/devtoolset-10/root/usr/lib64/dyninst:/opt/rh/devtoolset-10/root/usr/lib/dyninst:/usr/local/lib64 --build-arg BUILD_UID=$( id -u )"
Repository: onnxruntimecuda11build
Dockerfile: tools/ci_build/github/linux/docker/inference/x64/default/cpu/Dockerfile
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This change is to remove manylinux related things from this C API pipeline, because we don't need these python stuffs here.

else:
my_env["LD_LIBRARY_PATH"] = dll_path
if cuda_home:
my_env["PATH"] += os.pathsep + os.path.join(cuda_home, "bin") + os.pathsep + my_env["PATH"]
Copy link
Contributor

Choose a reason for hiding this comment

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

so the final value is <original PATH>:<cuda_home>/bin:<original PATH>?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed.

--volume /data/models:/build/models:ro --volume $HOME/.onnx:/home/onnxruntimedev/.onnx -e NIGHTLY_BUILD onnxruntimecuda11build \
/opt/python/cp37-cp37m/bin/python3 /onnxruntime_src/tools/ci_build/build.py --build_java --build_dir /build --config Release \
--skip_submodule_sync --parallel --build_shared_lib --use_cuda --cuda_version=$(CUDA_VERSION) --cuda_home=/usr/local/cuda-$(CUDA_VERSION) --cudnn_home=/usr/local/cuda-$(CUDA_VERSION) --cmake_extra_defines CMAKE_CUDA_HOST_COMPILER=/opt/rh/devtoolset-10/root/usr/bin/cc 'CMAKE_CUDA_ARCHITECTURES=37;50;52;60;61;70;75;80'
docker run --gpus all -e CFLAGS="-Wp,-D_FORTIFY_SOURCE=2 -Wp,-D_GLIBCXX_ASSERTIONS -fstack-protector-strong -fstack-clash-protection -fcf-protection -O3 -Wl,--strip-all" -e CXXFLAGS="-Wp,-D_FORTIFY_SOURCE=2 -Wp,-D_GLIBCXX_ASSERTIONS -fstack-protector-strong -fstack-clash-protection -fcf-protection -O3 -Wl,--strip-all" -e NVIDIA_VISIBLE_DEVICES=all --rm --volume /data/onnx:/data/onnx:ro --volume $(Build.SourcesDirectory):/onnxruntime_src --volume $(Build.BinariesDirectory):/build \
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: can we make the lines shorter for readability?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated

my_env["LD_LIBRARY_PATH"] = dll_path
# Add nvcc's folder to PATH env so that our cmake file can find nvcc
if cuda_home:
my_env["PATH"] = os.path.join(cuda_home, "bin") + os.pathsep + my_env["PATH"]
Copy link
Contributor

Choose a reason for hiding this comment

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

should we also check if "PATH" in my_env as is done for dll_path?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

PATH is always in env vars.

@snnn snnn merged commit d5e34ac into master Jun 4, 2022
@snnn snnn deleted the snnn/update_git branch June 4, 2022 03:00
jchen351 added a commit that referenced this pull request Aug 5, 2022
… Zip-Nuget-Java-Nodejs Packaging Pipeline (#11651)"

This reverts commit d5e34ac
jchen351 added a commit that referenced this pull request Aug 5, 2022
… used by Zip-Nuget-Java-Nodejs Packaging Pipeline (#11651)""

This reverts commit 3c1a330.
Craigacp pushed a commit to Craigacp/onnxruntime that referenced this pull request Aug 8, 2022
* adding conditional variable again

* Adding split test cases in python

* Adding python cases for split

* Enable s8s8 split

* Optimize input

* Revert "Remove git and python packages from the docker images used by Zip-Nuget-Java-Nodejs Packaging Pipeline (microsoft#11651)"

This reverts commit d5e34ac

* Revert "Revert "Remove git and python packages from the docker images used by Zip-Nuget-Java-Nodejs Packaging Pipeline (microsoft#11651)""

This reverts commit 3c1a330.

* format file

* Update c-api-linux-cpu.yml

* Update c-api-linux-cpu.yml

* Update c-api-linux-cpu.yml

* Reformat file

* Reformat file

* format file

* Optimize input

* Remove unused import

* Remove useless init

* Format split.py with black
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants