Skip to content

Conversation

@buddly27
Copy link
Contributor

No description provided.

@buddly27 buddly27 requested a review from davvid July 30, 2025 19:49
@buddly27 buddly27 self-assigned this Jul 30, 2025
@buddly27 buddly27 force-pushed the fix-tests branch 2 times, most recently from 6ebef2c to 7cc03fc Compare July 30, 2025 20:03
@davvid
Copy link
Member

davvid commented Jul 30, 2025

Thanks @buddly27 this looks good. I'm not sure if windows-latest is a viable image tag. I've used that elsewhere, though, and it has helped slightly reduce maintenance of CI files elsewhere. I suspect that the compiler and related settings would need changing anyways so it may not be that big of a benefit for this project, but mentioning in case it's something we'd like to use.

@davvid
Copy link
Member

davvid commented Jul 30, 2025

https://github.com/actions/runner-images does mention windows-latest as an available tag.

@davvid
Copy link
Member

davvid commented Jul 30, 2025

https://github.com/PixarAnimationStudios/OpenUSD/blob/dev/build_scripts/build_usd.py#L385 is interesting because it is interesting because build_usd.py already tries to autodetet the generator (so that we don't have to specify a generator).

Not that big of a deal right now, but it'd be really nice if we could omit the --generator option and let build_usd.py select the correct generator automatically.

I do see that OpenUSD's CI workflows don't rely on the autodetection and they specify a generator as well, but seeing this did make me question whether it's strictly needed.

@buddly27
Copy link
Contributor Author

@davvid I kept windows-2022 instead of windows-latest to mirror OpenUSD practices:
PixarAnimationStudios/OpenUSD@f5f3bf9

Regarding the generator, I find it clearer to specify it explicitly for OpenUSD, especially since we already do so for UNF. I believe CMake tries to use Ninja if -G or CMAKE_GENERATOR aren't provided, but build_usd.py doesn't honor the CMAKE_GENERATOR environment variable, so it’s effectively equivalent to passing it directly!

@buddly27 buddly27 merged commit 5ea9cdd into main Jul 30, 2025
8 checks passed
@davvid davvid deleted the fix-tests branch October 3, 2025 20:16
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.

3 participants