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.

Sign up for GitHub

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

Jump to bottom

[ogre] Disable share folder check #29830

Closed
wants to merge 5 commits into from
Closed

[ogre] Disable share folder check #29830

wants to merge 5 commits into from

Conversation

JackBoosY
Copy link
Contributor

Prepare for microsoft/vcpkg-tool#675 to avoid CI failure.

@JackBoosY
Copy link
Contributor Author

cc @ras0219-msft @BillyONeal @dg0yt

@Adela0814 Adela0814 added the category:port-bug The issue is with a library, which is something the port should already support label Feb 24, 2023
Adela0814
Adela0814 previously approved these changes Feb 27, 2023
View reviewed changes
@Adela0814 Adela0814 added the info:reviewed Pull Request changes follow basic guidelines label Feb 27, 2023
dg0yt
dg0yt reviewed Feb 27, 2023
View reviewed changes
ports/ogre/portfile.cmake
@@ -6,6 +6,8 @@ if(NOT VCPKG_TARGET_IS_WINDOWS AND NOT VCPKG_TARGET_IS_OSX)
message("${PORT} currently requires the following library from the system package manager:\n Xaw\n\nIt can be installed on Ubuntu systems via apt-get install libxaw7-dev")
endif()

set(VCPKG_POLICY_SKIP_SHARE_FOLDER_CHECK enabled)
Copy link
Contributor

@dg0yt dg0yt Feb 27, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Did you check if it can be renamed, or is it a quick fix?
If it is a quick fix, it would be worth noting this in a comment.
We have to check each affected ports, and we need to know which prots were already checked.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I didn't look at the generated files, the modification in this PR is just a workaround.

JavierMatosD
JavierMatosD reviewed Feb 27, 2023
View reviewed changes
Copy link
Contributor

@JavierMatosD JavierMatosD left a comment •
edited
Loading

Choose a reason for hiding this comment

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

@JackBoosY I think this is fine, but I want to wait to merge this until we are sure that microsoft/vcpkg-tool#675 is ready, just in case we want the policy name to change. I will tag vcpkg-team-review to solidify the policy name.

@JavierMatosD JavierMatosD added requires:vcpkg-team-review This PR or issue requires someone on the vcpkg team to take a further look. and removed info:reviewed Pull Request changes follow basic guidelines labels Feb 27, 2023
@JackBoosY JackBoosY mentioned this pull request Apr 19, 2023
Merged
@Adela0814 Adela0814 added depends:different-pr This PR or Issue depends on a PR which has been filed and removed requires:vcpkg-team-review This PR or issue requires someone on the vcpkg team to take a further look. labels Apr 21, 2023
@JackBoosY
Copy link
Contributor Author

@Adela0814 Different-PR can be removed.

@Adela0814 Adela0814 removed the depends:different-pr This PR or Issue depends on a PR which has been filed label Apr 24, 2023
@Adela0814 Adela0814 added the info:reviewed Pull Request changes follow basic guidelines label Apr 24, 2023
@dg0yt
Copy link
Contributor

dg0yt commented Apr 24, 2023 •
edited
Loading

This should not be merged before there is a binding commitment to enforce lower-case names in share.
Other mitigations:

  • [vcpkg-cmake-config] Fix package name quirks #31053 is to include lower-case adjustments for cmake config. It already reports the problem.
  • IMO cmake config could be in a different dir with less strict requirements. Then share would no longer need to carry most directories which are not port names.

@BillyONeal BillyONeal added requires:vcpkg-team-review This PR or issue requires someone on the vcpkg team to take a further look. and removed info:reviewed Pull Request changes follow basic guidelines labels Apr 24, 2023
@JackBoosY JackBoosY closed this Jun 19, 2023
@JackBoosY JackBoosY mentioned this pull request Jun 19, 2023
Closed
@JackBoosY JackBoosY deleted the dev/jack/disable-ogre-lint-check branch June 19, 2023 03:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dg0yt dg0yt dg0yt left review comments

@JavierMatosD JavierMatosD JavierMatosD left review comments

@Adela0814 Adela0814 Adela0814 approved these changes

Assignees

@Adela0814 Adela0814

Labels
category:port-bug The issue is with a library, which is something the port should already support requires:vcpkg-team-review This PR or issue requires someone on the vcpkg team to take a further look.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@JackBoosY @dg0yt @JavierMatosD @Adela0814 @BillyONeal