Skip to content

Conversation

@snnn
Copy link
Contributor

@snnn snnn commented Feb 9, 2021

Description:

  1. Add python 3.8/3.9 support for Windows GPU and Linux ARM64

  2. Delete jemalloc from cgmanifest.json.

  3. Add onnx node test to Nuphar pipeline.

  4. Change $ANDROID_HOME/ndk-bundle to $ANDROID_NDK_HOME. The later one is more accurate.

  5. Delete Java GPU packaging pipeline

  6. Remove test data download step in Nuget Mac OS pipeline. Because these machines are out of control and out of our network, it's hard to make it reliable and the data secure.

  7. Fix a doc problem in c-api-artifacts-package-and-publish-steps-windows.yml. It shouldn't copy C_API.md, because the file has been moved into a different branch.

  8. Delete the CI build docker file for Ubuntu cuda 9.x and Ubuntu x86 32 bits

  9. And, due to some internal restrictions, I need to rename some of the agent pools

Org Old name New Name
onnxruntime Linux-CPU Linux-CPU-2019
aiinfra Win-CPU-2019 Win-CPU-2021

Motivation and Context

  • Why is this change required? What problem does it solve?
    To have full support for python 3.8/3.9.

Note: there is a tiny issue for the GPU java packaging part. The package name is wrong. I'll address it separately.

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

@snnn snnn requested a review from a team as a code owner February 9, 2021 05:36
@snnn snnn force-pushed the snnn/python1 branch 2 times, most recently from 3d5fce8 to a356d38 Compare February 9, 2021 23:58
@snnn
Copy link
Contributor Author

snnn commented Feb 10, 2021

/azp run Windows CPU CI Pipeline

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@snnn snnn force-pushed the snnn/python1 branch 3 times, most recently from 0f59788 to a7365cb Compare February 10, 2021 22:47
inc="-I build/native/include"

if [ $IsMacOS = "true" ]; then
if [[ $IsMacOS == "True" || $IsMacOS == "true" ]]; then
Copy link
Contributor

Choose a reason for hiding this comment

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

looks like there is a way to convert to lower case: https://www.gnu.org/software/bash/manual/html_node/Shell-Parameter-Expansion.html#Shell-Parameter-Expansion (search for ${parameter,,pattern})
how about ${IsMacOs,,} == "true"?

# Licensed under the MIT License.

LocalNuGetRepo=$1
SourceRoot=$2
Copy link
Contributor

Choose a reason for hiding this comment

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

why not keep this parameter and pass the correct value? seems better to not depend on azure devops variable names like BUILD_SOURCESDIRECTORY outside of build definitions.

Copy link
Contributor

Choose a reason for hiding this comment

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

could add an additional parameter for the binaries directory

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, then I need to add another parameter, which requires modify more files(all the callers of this script). That's why I choose this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Indeed I would prefer to pass less parameters when possible. The more we have, the more likely we could make some errors during the passing, for example, missing one parameter or the order is wrong.

Copy link
Contributor

@edgchen1 edgchen1 Feb 12, 2021

Choose a reason for hiding this comment

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

that's what error handling is for :)
or named command line options
can handle it later

Copy link
Contributor Author

@snnn snnn Feb 12, 2021

Choose a reason for hiding this comment

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

Oh, I forgot to mention, sometimes the file is called from docker, sometimes it runs directly in the host environment. If we use the azure devops variable names directly, then in the first case we need to pass it, in the second case we don't and we can directly use the vars.

@snnn snnn merged commit 8378a45 into master Feb 12, 2021
@snnn snnn deleted the snnn/python1 branch February 12, 2021 00:43
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.

4 participants