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

Do not use env:NUMBER_OF_PROCESSORS for MinGW #2076

Merged
merged 1 commit into from
Mar 25, 2024

Conversation

vrabaud
Copy link
Collaborator

@vrabaud vrabaud commented Mar 25, 2024

Since CMake 3.29 (which is on MingW), we can omit the parameter. https://cmake.org/cmake/help/latest/manual/ctest.1.html "Otherwise, if the value is omitted, parallelism is limited by the number of processors, or 2, whichever is larger."

Since CMake 3.29 (which is on MingW), we can omit the parameter.
https://cmake.org/cmake/help/latest/manual/ctest.1.html
"Otherwise, if the value is omitted, parallelism is limited by the
number of processors, or 2, whichever is larger."
@vrabaud vrabaud requested a review from y-guyon March 25, 2024 13:17
@vrabaud vrabaud merged commit 3cc0772 into AOMediaCodec:main Mar 25, 2024
23 checks passed
@vrabaud vrabaud deleted the mingw2 branch March 25, 2024 13:23
@@ -82,4 +82,4 @@ jobs:
run: ninja
- name: Run AVIF Tests
working-directory: ./build
run: ctest -j $Env:NUMBER_OF_PROCESSORS --output-on-failure
Copy link
Collaborator

Choose a reason for hiding this comment

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

Thanks for the fix. Do you know why $Env:NUMBER_OF_PROCESSORS suddenly stopped working recently?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

No idea, maybe @kmilos has an idea? Thx

Copy link
Contributor

@kmilos kmilos Mar 26, 2024

Choose a reason for hiding this comment

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

Not really, sorry... Perhaps a syntax thing? $Env:NUMBER_OF_PROCESSORS is powershell, while the default shell for this job is msys2 (bash)...

Copy link
Contributor

Choose a reason for hiding this comment

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

Yup: https://github.com/AOMediaCodec/libavif/actions/runs/8411426945/job/23030997022#step:10:7

This should be just $NUMBER_OF_PROCESSORS in case you want to bring it back.

@wantehchang wantehchang mentioned this pull request Mar 26, 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
Development

Successfully merging this pull request may close these issues.

4 participants