Skip to content

[docs] Add Authoring-script-ports.md#22396

Merged
BillyONeal merged 3 commits intomicrosoft:masterfrom
ras0219-msft:dev/roschuma/docs
Feb 3, 2022
Merged

[docs] Add Authoring-script-ports.md#22396
BillyONeal merged 3 commits intomicrosoft:masterfrom
ras0219-msft:dev/roschuma/docs

Conversation

@ras0219-msft
Copy link
Contributor

Add documentation covering vcpkg-port-config.cmake and how to author script ports

@PhoebeHui PhoebeHui self-assigned this Jan 7, 2022
@PhoebeHui PhoebeHui added category:documentation To resolve the issue, documentation will need to be updated info:internal labels Jan 7, 2022
@PhoebeHui PhoebeHui added the info:needs-maintainer-attention Lets the current 'on rotation' vcpkg maintainer know they need to look at this. label Jan 7, 2022
Copy link
Contributor

@dg0yt dg0yt left a comment

Choose a reason for hiding this comment

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

It seems that this PR lacks the addition of docs/maintainers/authoring-script-ports.md.

@PhoebeHui PhoebeHui added requires:author-response and removed info:needs-maintainer-attention Lets the current 'on rotation' vcpkg maintainer know they need to look at this. labels Jan 7, 2022
Copy link
Contributor

@strega-nil-ms strega-nil-ms left a comment

Choose a reason for hiding this comment

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

It'd also be good to talk about how to add docs to a script port; adding the port to docs/regenerate.ps1.

@@ -0,0 +1,28 @@
# Authoring Script Ports

Ports can expose functions for other ports to consume during their build, for example the `vcpkg-cmake` helper port exposes the `vcpkg_cmake_configure()` helper function. Packaging common scripts into a shared helper port makes maintenance easier because all consumers can be updated from a single place. Because the scripts come from a port, they can be versioned and depended upon via all the same mechanisms as any other port.
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you split this up into multiple lines? Having a single paragraph all on one line makes this difficult to review...

Also, s/, for example/: as an example,/


Script ports are technically implemented via the `vcpkg-port-config.cmake` extension mechanism. Before invoking the `portfile.cmake` of a port, vcpkg will first import `share/<port>/vcpkg-port-config.cmake` from each direct dependency. If the direct dependency is a host dependency, the import will be performed in the host installed tree (e.g. `${HOST_INSTALLED_DIR}/share/${DEP}/vcpkg-port-config.cmake`).

Note that only direct dependencies are checked. This means that if a script relies on another script transitively, it must explicitly import the relevant config.
Copy link
Contributor

Choose a reason for hiding this comment

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

  • s/direct dependencies/the `vcpkg-port-config.cmake` of direct dependencies/
  • s/are checked/are included in this way/
  • s/relevant config/relevant `vcpkg-port-config.cmake`/

Some ports are host-only: script ports and tool ports are common examples.
In this case, you can use the `"native"` supports expression to describe this.
This supports expression is true when `TARGET_TRIPLET == HOST_TRIPLET`.
This supports expression is true when `TARGET_TRIPLET == HOST_TRIPLET` / `VCPKG_CROSSCOMPILING` is true.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
This supports expression is true when `TARGET_TRIPLET == HOST_TRIPLET` / `VCPKG_CROSSCOMPILING` is true.
This supports expression is true when `VCPKG_CROSSCOMPILING` is false (when `TARGET_TRIPLET == HOST_TRIPLET`).

@dg0yt
Copy link
Contributor

dg0yt commented Jan 12, 2022

It'd also be good to talk about how to add docs to a script port; adding the port to docs/regenerate.ps1.

Actually this concept is controversial. It breaks documentation fixes without CI rebuild.
Related: Discussion at #21763 (comment)

@ras0219-msft
Copy link
Contributor Author

I have resolved the comments that were more-or-less directly applied but left open the ones where I chose to format at least part of the suggestion differently.

@PhoebeHui PhoebeHui added the info:needs-maintainer-attention Lets the current 'on rotation' vcpkg maintainer know they need to look at this. label Jan 28, 2022

Ports should never provide a `vcpkg-port-config.cmake` file under a different
`share/` subdirectory than the current port (`${PORT}`).

Copy link
Contributor

Choose a reason for hiding this comment

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

You should probably add some information on docs; there's a list in docs/regenerate.ps1 that one should update when adding a new script port.

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 would prefer to merge this PR as-is; then that content can be easily added separately.

Co-authored-by: nicole mazzuca <83086508+strega-nil-ms@users.noreply.github.com>
@BillyONeal BillyONeal merged commit c06dfc0 into microsoft:master Feb 3, 2022
ekilmer added a commit to ekilmer/vcpkg that referenced this pull request Feb 5, 2022
* master: (221 commits)
  [vcpkg-tool] update to 2022-02-03 (microsoft#22924)
  [opencv4] Disable building cpufeatures since it conflict with libwebp (microsoft#22844)
  [rhasheq] New port (microsoft#22905)
  [sfml] Add arm64 patch to allow SFML to compile on apple silicon (microsoft#22937)
  [popsift] Fix missing Thrust include, already merged upstream. (microsoft#22929)
  [python3][python2] Use MKDIR_P to create directories to avoid race conditions (microsoft#22902)
  Added libe57format port (microsoft#22909)
  update polyhook2 (microsoft#22906)
  [botan] Fix debug info (microsoft#22911)
  [opentelemetry-cpp] update version to v1.2.0 (microsoft#22925)
  [docs] document VCPKG_INSTALLED_DIR variable (microsoft#22695)
  [c89stringutils] New port (microsoft#22904)
  [randomstr] New port (microsoft#22921)
  [docs] Add Authoring-script-ports.md (microsoft#22396)
  [vcpkg_fail_port_install] Deprecate function (microsoft#21489)
  [vcpkg-cmake-config] add missing TOOLS_PATH (microsoft#22863)
  Ace Build Windows - Missing files in include package (microsoft#22880)
  [opendnp3] Disable FetchContent in favor of predownloading (microsoft#22894)
  libraqm update to 0.9.0 (microsoft#22907)
  [google-cloud-cpp] update to latest release (v1.36.0) (microsoft#22897)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

category:documentation To resolve the issue, documentation will need to be updated 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.

5 participants