vcpkg icon indicating copy to clipboard operation
vcpkg copied to clipboard
verified publisher icon microsoft

[usd] Fixes for recent TBB versions and osx-arm64

Open michaelmigliore opened this issue 1 year ago • 9 comments

  • [x] Changes comply with the maintainer guide.
  • [ ] ~~SHA512s are updated for each updated download.~~
  • [x] The "supports" clause reflects platforms that may be fixed by this new version.
  • [x] Any fixed CI baseline entries are removed from that file.
  • [x] Any patches that are no longer applied are deleted from the port's directory.
  • [x] The version database is fixed by rerunning ./vcpkg x-add-version --all and committing the result.
  • [x] Only one version is added to each modified port's versions file.

michaelmigliore avatar Jun 14 '24 10:06 michaelmigliore

Has this patch been submitted upstream? This is much larger than we normally would want to maintain ourselves.

BillyONeal avatar Jun 14 '24 18:06 BillyONeal

Has this patch been submitted upstream? This is much larger than we normally would want to maintain ourselves.

Yes. Most of these changes are already merged in the dev branch but for some reason it takes time to make its way to the release.

michaelmigliore avatar Jun 14 '24 19:06 michaelmigliore

Is the failing x64_osx job related to my change? It's not clear to me.

michaelmigliore avatar Jun 21 '24 07:06 michaelmigliore

Is the failing x64_osx job related to my change? It's not clear to me.

No logs were generated so I've re-ran the test.

WangWeiLin-MV avatar Jun 21 '24 07:06 WangWeiLin-MV

Thank you @WangWeiLin-MV, the CI looks green now.

michaelmigliore avatar Jun 21 '24 14:06 michaelmigliore

Tagging vcpkg-team-review to ask if a patch this big is OK. If the patches do indeed exist in upstream you might be able to avoid this concern by downloading the patches directly from upstream, for example:

https://github.com/microsoft/vcpkg/blob/c4467cb686f92671f0172aa8299a77d908175b4e/ports/nifly/portfile.cmake#L3-L9

BillyONeal avatar Jun 22 '24 00:06 BillyONeal

Tagging vcpkg-team-review to ask if a patch this big is OK. If the patches do indeed exist in upstream you might be able to avoid this concern by downloading the patches directly from upstream, for example:

https://github.com/microsoft/vcpkg/blob/c4467cb686f92671f0172aa8299a77d908175b4e/ports/nifly/portfile.cmake#L3-L9

See https://github.com/microsoft/vcpkg/pull/39292#discussion_r1642496781 I tried that but the commits are conflicting with the release branch

michaelmigliore avatar Jun 23 '24 13:06 michaelmigliore

@AugP @ras0219-msft @data-queue and I discussed this today.

@BillyONeal I am weakly against taking this because I'm concerned about the licensing implications

@ras0219-msft Option 1: We close this and say 'anyone who wants these changes can use what's in this PR until upstream does a release' Option 2: We just ship the development snapshot with these changes already inside instead.

@ras0219-msft has opinions about how to do versioning if you choose option 2.

Please feel free to choose one or the other and either close it (option 1) or just use a version with it fixed and press 'ready for review' when that's done. Thanks for fixing the port!

BillyONeal avatar Jun 27 '24 22:06 BillyONeal

I'd rather wait for the next release in this case.
There's another fix in this PR related to macos ARM support, should I create another PR or remove TBB related patches in this one and force push? It's basically just changing !arm to !(windows & arm)

michaelmigliore avatar Jun 28 '24 06:06 michaelmigliore

Closing this since it's outdated and the new version is supporting oneTBB now according to the release note.
See https://github.com/microsoft/vcpkg/pull/40225 and https://github.com/microsoft/vcpkg/pull/41864

michaelmigliore avatar Nov 08 '24 08:11 michaelmigliore