Skip to content

[bullet3] Remove vcpkg_fail_port_install.#22729

Merged
BillyONeal merged 2 commits intomicrosoft:masterfrom
BillyONeal:remove_fail_port_install_bullet3
Jan 24, 2022
Merged

[bullet3] Remove vcpkg_fail_port_install.#22729
BillyONeal merged 2 commits intomicrosoft:masterfrom
BillyONeal:remove_fail_port_install_bullet3

Conversation

@BillyONeal
Copy link
Member

The supports expression and the portfile.cmake disagreed.

Supports: !((windows | linux) & (arm | uwp))
Portfile: osx | !(arm | arm64 | uwp)

If we demorgan the supports expression we get something closer to the portfile:
!(windows | linux) | !(arm | uwp)

If we take "!(windows | linux)" to mean that the author meant "osx", and assume arm64 implies arm, we end up with:

osx | !(arm | uwp)

In support of: #21502

The supports expression and the portfile.cmake disagreed.

Supports: !((windows | linux) & (arm | uwp))
Portfile: osx | !(arm | arm64 | uwp)

If we demorgan the supports expression we get something closer to the portfile:
!(windows | linux) | !(arm | uwp)

If we take "!(windows | linux)" to mean that the author meant "osx", and assume arm64 implies arm, we end up with:

osx | !(arm | uwp)

In support of: microsoft#21502
Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

You have modified or added at least one vcpkg.json where a "license" field is missing.
If you feel able to do so, please consider adding a "license" field to the following files:

  • ports/bullet3/vcpkg.json

Valid values for the license field are listed at https://spdx.org/licenses/

@FrankXie05 FrankXie05 added the info:needs-maintainer-attention Lets the current 'on rotation' vcpkg maintainer know they need to look at this. label Jan 24, 2022
@BillyONeal
Copy link
Member Author

Thanks for the review @FrankXie05 :D

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

You have modified or added at least one vcpkg.json where a "license" field is missing.
If you feel able to do so, please consider adding a "license" field to the following files:

  • ports/bullet3/vcpkg.json

Valid values for the license field are listed at https://spdx.org/licenses/

@FrankXie05
Copy link
Contributor

@BillyONeal After you delete the content of the baseline, whether to overwrite the content of the git tree of versions/b-/bullet3.json again? :)

@FrankXie05 FrankXie05 removed the info:needs-maintainer-attention Lets the current 'on rotation' vcpkg maintainer know they need to look at this. label Jan 24, 2022
@BillyONeal
Copy link
Member Author

@BillyONeal After you delete the content of the baseline, whether to overwrite the content of the git tree of versions/b-/bullet3.json again? :)

ci.baseline.txt isn't part of the version database.

@BillyONeal
Copy link
Member Author

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@FrankXie05 FrankXie05 added the info:needs-maintainer-attention Lets the current 'on rotation' vcpkg maintainer know they need to look at this. label Jan 24, 2022
@FrankXie05
Copy link
Contributor

@BillyONeal oh, my bad. I don't see you modifying the baseline halfway through。

@BillyONeal
Copy link
Member Author

@BillyONeal oh, my bad. I don't see you modifying the baseline halfway through。

Yeah, I didn't think about it until @JackBoosY's comment on another PR so I double checked before clicking the merge button here :)

@BillyONeal BillyONeal merged commit 485e459 into microsoft:master Jan 24, 2022
@BillyONeal BillyONeal deleted the remove_fail_port_install_bullet3 branch January 24, 2022 06:43
@bwrsandman
Copy link
Contributor

Sorry to comment on a closed PR but why is arm disabled? There shouldn't be any issues with building for Android as it is listed as supported on their github.

@BillyONeal
Copy link
Member Author

Sorry to comment on a closed PR but why is arm disabled? There shouldn't be any issues with building for Android as it is listed as supported on their github.

I don't think most of what we've done in terms of determining what ports support what has considered Android; I observe that MacOS also allows ARM which may have been done for iOS?

With this change I was trying to keep the behavior of the port the same; if someone shows that it works elsewhere a PR changing the supports expression to be consistent with what upstream does would be welcome.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

info:needs-maintainer-attention Lets the current 'on rotation' vcpkg maintainer know they need to look at this.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants