Skip to content
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

CSHARP-4962: Fix package build script on CI to produce 472 binaries #1264

Merged
merged 1 commit into from Feb 16, 2024

Conversation

sanych-sun
Copy link
Member

Run packages-pack on Windows host + set priority to 10 for all package-related tasks.

@sanych-sun sanych-sun requested a review from a team as a code owner February 9, 2024 22:07
@sanych-sun sanych-sun removed the request for review from a team February 9, 2024 22:07
@@ -944,7 +944,7 @@ functions:
params:
aws_key: ${aws_key}
aws_secret: ${aws_secret}
local_file: ${PROJECT_DIRECTORY}/artifacts/nuget/${PACKAGE_ID}.${PACKAGE_VERSION}.nupkg
local_file: mongo-csharp-driver/artifacts/nuget/${PACKAGE_ID}.${PACKAGE_VERSION}.nupkg
Copy link
Member Author

Choose a reason for hiding this comment

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

Windows hosts were not happy with the absolute path, it was threated it as relative.

Copy link
Contributor

Choose a reason for hiding this comment

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

We use ${PROJECT_DIRECTORY} throughout evergreen.yml and it runs successfully on Windows hosts. Any idea why it is problematic when used for the local_file argument?

Copy link
Member Author

Choose a reason for hiding this comment

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

As it was discussed offline: s3.put command somehow does not work very well with absolute path on Windows hosts. So will go with relative path as per proposed code.

@@ -2331,22 +2331,24 @@ buildvariants:
- matrix_name: build-packages
matrix_spec:
build-target: "release"
os: "windows-64" # should produce package on Windows to make sure full framework binaries created.
Copy link
Member Author

Choose a reason for hiding this comment

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

Setting host as a matrix_spec axis also set env variable, so platform detection works properly in sh-files.

Copy link
Contributor

Choose a reason for hiding this comment

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

matrix_name allows us to run all combinations of the specified matrix_spec. Given that the package step is a single action, we can simply use buildvariants/name with run_on similar to gssapi-auth-tests-windows or driver-performance-tests. No need to invoke the matrix_* machinery.

Copy link
Member Author

Choose a reason for hiding this comment

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

I was unable to properly set the OS expansion/env variable without axis, so the platform detection was not working properly. Also we already have a matrix there for "build-target" variable (it's a convenient way to pass this variable and ensure it's enum-like variable with limited number of allowed values).

tags: ["build-packages", "release-tag"]
tasks:
- name: build-packages
git_tag_only: true
priority: 10
Copy link
Member Author

Choose a reason for hiding this comment

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

When someone starting a release, he/she have to wait for it's completion to move forward with after release manual steps. So I've decided to prioritize package-related steps.

Copy link
Contributor

@JamesKovacs JamesKovacs left a comment

Choose a reason for hiding this comment

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

Some questions and suggested simplifications.

@@ -944,7 +944,7 @@ functions:
params:
aws_key: ${aws_key}
aws_secret: ${aws_secret}
local_file: ${PROJECT_DIRECTORY}/artifacts/nuget/${PACKAGE_ID}.${PACKAGE_VERSION}.nupkg
local_file: mongo-csharp-driver/artifacts/nuget/${PACKAGE_ID}.${PACKAGE_VERSION}.nupkg
Copy link
Contributor

Choose a reason for hiding this comment

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

We use ${PROJECT_DIRECTORY} throughout evergreen.yml and it runs successfully on Windows hosts. Any idea why it is problematic when used for the local_file argument?

@@ -2331,22 +2331,24 @@ buildvariants:
- matrix_name: build-packages
matrix_spec:
build-target: "release"
os: "windows-64" # should produce package on Windows to make sure full framework binaries created.
Copy link
Contributor

Choose a reason for hiding this comment

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

matrix_name allows us to run all combinations of the specified matrix_spec. Given that the package step is a single action, we can simply use buildvariants/name with run_on similar to gssapi-auth-tests-windows or driver-performance-tests. No need to invoke the matrix_* machinery.


- matrix_name: generate-apidocs
matrix_spec:
build-target: "release"
os: "ubuntu-2004"
Copy link
Contributor

Choose a reason for hiding this comment

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

Why are we moving from ubuntu2004-small to ubuntu2004 for these variants?

Copy link
Member Author

Choose a reason for hiding this comment

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

Basically I've removed explicit run_on by using the matrix_spec (so it would set OS env variable and for consistency). Setting OS axis to "ubuntu-2004" is to be run on the ubuntu2004-small.

      - id: "ubuntu-2004"
        display_name: "Ubuntu 20.04"
        variables:
          OS: "ubuntu-2004"
          python3_binary: "/opt/python/3.8/bin/python3"
        run_on: ubuntu2004-small

Copy link
Contributor

@JamesKovacs JamesKovacs left a comment

Choose a reason for hiding this comment

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

LGTM

@sanych-sun sanych-sun merged commit a6f939d into mongodb:master Feb 16, 2024
18 of 25 checks passed
@sanych-sun sanych-sun deleted the csharp4962 branch February 16, 2024 18:32
sanych-sun added a commit that referenced this pull request Feb 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants