[Spirv reflect] Add new port #22295

kleszcz · 2022-01-02T16:49:43Z

Describe the pull request

What does your PR fix?

Adds port for SPIRV-Reflect
Which triplets are supported/not supported? Have you updated the CI baseline?

linux, windows, No
Does your PR follow the maintainer guide?

Yes
If you have added/updated a port: Have you run ./vcpkg x-add-version --all and committed the result?

Yes

If you are still working on the PR, open it as a Draft: https://github.blog/2019-02-14-introducing-draft-pull-requests/

ghost · 2022-01-02T16:49:55Z

All CLA requirements met.

github-actions

You have modified or added at least one portfile where deprecated functions are used.

If you feel able to do so, please consider migrating them to the new functions:
vcpkg_install_cmake -> vcpkg_cmake_install (from port vcpkg-cmake)
vcpkg_build_cmake -> vcpkg_cmake_build (from port vcpkg-cmake)
vcpkg_configure_cmake -> vcpkg_cmake_configure (Please remove the option PREFER_NINJA) (from port vcpkg-cmake)
vcpkg_fixup_cmake_targets -> vcpkg_cmake_config_fixup (from port vcpkg-cmake-config)

In the ports that use the new function, you have to add the corresponding dependencies:

{
  "name": "vcpkg-cmake",
  "host": true
},
{
  "name": "vcpkg-cmake-config",
  "host": true
}

The following files are affected:

ports/spirv-reflect/portfile.cmake

ports/spirv-reflect/portfile.cmake

ports/spirv-reflect/vcpkg.json

versions/s-/spirv-reflect.json

Co-authored-by: JonLiu1993 <63675417+JonLiu1993@users.noreply.github.com>

github-actions

You have modified or added at least one portfile where deprecated functions are used.

If you feel able to do so, please consider migrating them to the new functions:
vcpkg_install_cmake -> vcpkg_cmake_install (from port vcpkg-cmake)
vcpkg_build_cmake -> vcpkg_cmake_build (from port vcpkg-cmake)
vcpkg_configure_cmake -> vcpkg_cmake_configure (Please remove the option PREFER_NINJA) (from port vcpkg-cmake)
vcpkg_fixup_cmake_targets -> vcpkg_cmake_config_fixup (from port vcpkg-cmake-config)

In the ports that use the new function, you have to add the corresponding dependencies:

{
  "name": "vcpkg-cmake",
  "host": true
},
{
  "name": "vcpkg-cmake-config",
  "host": true
}

The following files are affected:

ports/spirv-reflect/portfile.cmake

JonLiu1993 · 2022-01-04T07:28:59Z

@kleszcz ，Have you tested the usage locally?

kleszcz · 2022-01-04T07:32:49Z

Hi @JonLiu1993!
Yes, I've it tested locally. Also I use this port in my Vulkan project via vcpkg manifest and it works fine .

JonLiu1993 · 2022-01-11T03:08:31Z

@strega-nil-ms ,Could you please take a look?

BillyONeal

These are mostly just nitpicks, thanks for the port!

ports/spirv-reflect/CMakeLists.txt

BillyONeal · 2022-01-14T23:25:24Z

ports/spirv-reflect/portfile.cmake

+if(DEFINED ENV{VULKAN_SDK})
+    message(STATUS "VULKAN_SDK env var found: $ENV{VULKAN_SDK}")
+else()
+    message(FATAL_ERROR "VULKAN_SDK env var not found!")
+endif()


This seems to be duplicating output from the dependency vulcan port so I don't know how useful this reporting is; I would remove it. If it stays, I think you need to explain something about how the user can remedy the situation.

BillyONeal · 2022-01-14T23:27:36Z

ports/spirv-reflect/CMakeLists.txt

+)
+
+install(
+    EXPORT spirv-reflect-config


When vcpkg makes up these names we usually try to add unofficial- as a prefix so that we don't stomp on upstream should they choose to add their own CMake targets. Or is this intended to match upstream exactly? (I see in the history this PR used to not be a complete build replacement)

Should the EXPORT be named unofficial-spriv-reflect-config and should I rename the namespace too?

I believe the "config" name doesn't need the prefix because we can compose that if upstream ever adds official support. (That is, the foo-config can provide both foo and unofficial-foo targets to not break existing customers if we want)

ports/spirv-reflect/CMakeLists.txt

JonLiu1993 · 2022-01-30T03:42:10Z

@kleszcz ,Could you please consider @BillyONeal 's suggestion？

Co-authored-by: Billy O'Neal <bion@microsoft.com>

@BillyONeal

as suggested by @BillyONeal

github-actions

This is a new experimental fast check for PR issues. Please let us know if this bot is helpful!

PRs must add only one version and must not modify any published versions

When making any changes to a library, the version or port-version in vcpkg.json or CONTROL must be modified.

Error: Local changes detected for spirv-reflect but no changes to version or port version.
-- Version: 2021-12-31
-- Old SHA: ae1fa5320c5da60247371bad1cd79fa87fea274d
-- New SHA: 63be5702e26e0065c5e067d100b1a26a5240c249
-- Did you remember to update the version or port version?
-- Pass `--overwrite-version` to bypass this check.
***No files were updated.***

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/spirv-reflect/vcpkg.json

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

ports/spirv-reflect/CMakeLists.txt

Co-authored-by: JonLiu1993 <63675417+JonLiu1993@users.noreply.github.com>

github-actions

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/spirv-reflect/vcpkg.json

Valid values for the license field can be found in the documentation

BillyONeal · 2022-02-11T00:36:19Z

@strega-nil tells me my reply above was incorrect. In order to reduce confusion I've submitted #23041 to codify what we want. I will push a change consistent with what lands there here if you want me to.

github-actions

This is a new experimental fast check for PR issues. Please let us know if this bot is helpful!

PRs must add only one version and must not modify any published versions

When making any changes to a library, the version or port-version in vcpkg.json or CONTROL must be modified.

Error: Local changes detected for spirv-reflect but no changes to version or port version.
-- Version: 2021-12-31
-- Old SHA: 78347a377022009b403554dfcb56b8d6c19182b9
-- New SHA: bcebcfdd9e9332b68c4185b3127fc898fc91402a
-- Did you remember to update the version or port version?
-- Pass `--overwrite-version` to bypass this check.
***No files were updated.***

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/spirv-reflect/vcpkg.json

Valid values for the license field can be found in the documentation

github-actions

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/spirv-reflect/vcpkg.json

Valid values for the license field can be found in the documentation

* master: (54 commits) [imgui] Update to 1.87 [implot] Update to 0.13 (microsoft#22988) [nu-book-zxing-cpp] New port (microsoft#22657) [librabbitmq] Update to 0.11.0 (microsoft#23037) [doc] Add doc for `supports` expression `staticcrt` (microsoft#23079) [doctest] Update to 2.4.8 (microsoft#23081) [log4cplus] Remove unneeded catch dependency (microsoft#23066) [Azure SDK] Update vcpkg ports for Feb Release (microsoft#23080) [Freerdp] Update to 2.5.0 (microsoft#23095) Update vcpkg-tool to 2022-02-11 (microsoft#23059) Minor bugfixes to MacOS deployment readme. (microsoft#23062) [ci.baseline.txt] Skip colmap on osx due to metis conflict (microsoft#23047) [gtkmm] update to 4.6.0 (microsoft#23024) [faiss] Update to 1.7.2 (microsoft#22705) [ocilib] Disable warning C4191 (microsoft#23028) [polyhook2] Update to latest (microsoft#23044) Add notice about how to export unofficial CMake targets. (microsoft#23041) [Spirv reflect] Add new port (microsoft#22295) [easyhook] Update target .NET Framework version to 4.7.2. (microsoft#23040) [gh suggestions] change license link, make it details (microsoft#22946) [field3d] Remove port (microsoft#22463) ...

kleszcz added 2 commits January 2, 2022 17:39

port for spirv-reflect

8a116f7

commiting results of vcpkg x-add-version --all

81bd2a4

kleszcz added 10 commits January 2, 2022 17:54

fixing deprecated function calls

f95472f

removing unused messages

e806856

fixing version issue

eefb34b

fixing version issue wrong sha

35f3bbd

fixing version issue missing port version

38e2dc2

fixing version issue missing port version and wrong sha

4ecac88

update version db

5a0224a

update version db

a596e37

port version bump

8c6ceee

Update version database

1fcaae9

kleszcz marked this pull request as ready for review January 2, 2022 17:37

JonLiu1993 self-assigned this Jan 4, 2022

JonLiu1993 added the category:new-port The issue is requesting a new library to be added; consider making a PR! label Jan 4, 2022

github-actions bot approved these changes Jan 4, 2022

View reviewed changes

JonLiu1993 changed the title ~~Spirv reflect port~~ [Spirv reflect] Add new port Jan 4, 2022

JonLiu1993 requested changes Jan 4, 2022

View reviewed changes

ports/spirv-reflect/portfile.cmake Outdated Show resolved Hide resolved

ports/spirv-reflect/vcpkg.json Outdated Show resolved Hide resolved

versions/s-/spirv-reflect.json Outdated Show resolved Hide resolved

JonLiu1993 added the requires:author-response label Jan 4, 2022

kleszcz and others added 2 commits January 4, 2022 07:50

Apply suggestions from code review

f62bf64

Co-authored-by: JonLiu1993 <63675417+JonLiu1993@users.noreply.github.com>

Update versions db

7b8f99e

kleszcz requested a review from JonLiu1993 January 4, 2022 07:27

JonLiu1993 removed the requires:author-response label Jan 4, 2022

github-actions bot approved these changes Jan 4, 2022

View reviewed changes

JonLiu1993 approved these changes Jan 4, 2022

View reviewed changes

JonLiu1993 added the requires:author-response label Jan 4, 2022

JonLiu1993 removed the requires:author-response label Jan 4, 2022

JonLiu1993 approved these changes Jan 11, 2022

View reviewed changes

JonLiu1993 removed the requires:author-response label Jan 11, 2022

JonLiu1993 approved these changes Jan 14, 2022

View reviewed changes

JonLiu1993 added the info:reviewed Pull Request changes follow basic guidelines label Jan 14, 2022

BillyONeal requested changes Jan 14, 2022

View reviewed changes

BillyONeal added requires:author-response and removed info:reviewed Pull Request changes follow basic guidelines labels Jan 15, 2022

kleszcz and others added 3 commits February 1, 2022 17:18

Update ports/spirv-reflect/CMakeLists.txt

8f5cee1

Co-authored-by: Billy O'Neal <bion@microsoft.com>

Adding @BillyONeal suggestions

7558c86

Removing unneeded check

18530dc

as suggested by @BillyONeal

github-actions bot requested changes Feb 7, 2022

View reviewed changes

JonLiu1993 reviewed Feb 7, 2022

View reviewed changes

ports/spirv-reflect/CMakeLists.txt Outdated Show resolved Hide resolved

JonLiu1993 reviewed Feb 7, 2022

View reviewed changes

ports/spirv-reflect/CMakeLists.txt Outdated Show resolved Hide resolved

JonLiu1993 reviewed Feb 7, 2022

View reviewed changes

ports/spirv-reflect/CMakeLists.txt Outdated Show resolved Hide resolved

JonLiu1993 reviewed Feb 7, 2022

View reviewed changes

ports/spirv-reflect/CMakeLists.txt Outdated Show resolved Hide resolved

kleszcz and others added 3 commits February 10, 2022 22:23

Apply suggestions from code review

7fe91c2

Co-authored-by: JonLiu1993 <63675417+JonLiu1993@users.noreply.github.com>

update version db

0037423

Merge remote-tracking branch 'origin/master' into HEAD

1de2a2f

github-actions bot approved these changes Feb 11, 2022

View reviewed changes

Drop "unofficial" from the unnamespaced target name.

a738a16

github-actions bot requested changes Feb 11, 2022

View reviewed changes

Fix version database.

95ecfd6

github-actions bot approved these changes Feb 11, 2022

View reviewed changes

BillyONeal removed the requires:author-response label Feb 11, 2022

BillyONeal approved these changes Feb 11, 2022

View reviewed changes

BillyONeal merged commit 7003b95 into microsoft:master Feb 11, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Spirv reflect] Add new port #22295

[Spirv reflect] Add new port #22295

kleszcz commented Jan 2, 2022 •

edited

ghost commented Jan 2, 2022 •

edited by ghost

github-actions bot left a comment

github-actions bot left a comment

JonLiu1993 commented Jan 4, 2022

kleszcz commented Jan 4, 2022

JonLiu1993 commented Jan 11, 2022

BillyONeal left a comment

BillyONeal Jan 14, 2022

BillyONeal Jan 14, 2022

kleszcz Feb 1, 2022

BillyONeal Feb 11, 2022

JonLiu1993 commented Jan 30, 2022

github-actions bot left a comment

github-actions bot left a comment

BillyONeal commented Feb 11, 2022

github-actions bot left a comment

github-actions bot left a comment

[Spirv reflect] Add new port #22295

[Spirv reflect] Add new port #22295

Conversation

kleszcz commented Jan 2, 2022 • edited

What does your PR fix?

Which triplets are supported/not supported? Have you updated the CI baseline?

Does your PR follow the maintainer guide?

If you have added/updated a port: Have you run ./vcpkg x-add-version --all and committed the result?

ghost commented Jan 2, 2022 • edited by ghost

github-actions bot left a comment

Choose a reason for hiding this comment

github-actions bot left a comment

Choose a reason for hiding this comment

JonLiu1993 commented Jan 4, 2022

kleszcz commented Jan 4, 2022

JonLiu1993 commented Jan 11, 2022

BillyONeal left a comment

Choose a reason for hiding this comment

BillyONeal Jan 14, 2022

Choose a reason for hiding this comment

BillyONeal Jan 14, 2022

Choose a reason for hiding this comment

kleszcz Feb 1, 2022

Choose a reason for hiding this comment

BillyONeal Feb 11, 2022

Choose a reason for hiding this comment

JonLiu1993 commented Jan 30, 2022

github-actions bot left a comment

Choose a reason for hiding this comment

github-actions bot left a comment

Choose a reason for hiding this comment

BillyONeal commented Feb 11, 2022

github-actions bot left a comment

Choose a reason for hiding this comment

github-actions bot left a comment

Choose a reason for hiding this comment

kleszcz commented Jan 2, 2022 •

edited

If you have added/updated a port: Have you run `./vcpkg x-add-version --all` and committed the result?

ghost commented Jan 2, 2022 •

edited by ghost