[curses] New meta port #22093

dg0yt · 2021-12-19T07:05:01Z

What does your PR fix?

This PR adds a curses meta-port. It pulls in a suitable implementation (pdcurses, ncurses) and makes it available via find_package(Curses).
Relates issues: How do I use PDCurses after having installed it? #11027, [ncurses] fix windows/uwp port #20828, [ncurses] Add new port #17226 (comment)
Which triplets are supported/not supported? Have you updated the CI baseline?

!uwp, 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

dg0yt · 2021-12-19T07:08:19Z

NB: Windows static triplets available for CI test after merging #22092.

cenit · 2021-12-20T16:39:45Z

what about making ports depending on pdcurses/ncurses depending on this one, instead?

BillyONeal · 2021-12-20T16:54:34Z

ports/curses/vcpkg-cmake-wrapper.cmake

@@ -0,0 +1,20 @@
+if(NOT DEFINED CURSES_NEED_NCURSES)


I think we should generate this file with the selection here rather than trying to derive it; I'm concerned that this form makes it path dependent since the supports for this port and the dependencies don't match exactly. Does that seem reasonable?

Generating from portfile is a reasonable request. However this introduces redundancy in implementing the platform condition. This is one reason why I didn't start with that complexity.

(The only way to carry variable dependency information to the portfile is through features. The ideal solution would be attaching platform expressions to members of the default-features array...)

It is unfortunate that it introduces redundancy; however, I'm worried about a case like:

A depends on curses

B depends on curses

The end user wants a program that links with A and B

vcpkg install pdcurses

vcpkg install A (which installs curses as a dependency, A was built with pdcurses)

vcpkg install ncurses

vcpkg install B (whoops, now B was built with ncurses because that's what this wrapper prefers)

End user program is doomed because the different curses provide different features and/or expect to track information about the console in different globals, etc.

If there is some reason that pdcurses and ncurses can never be installed at the same time, and thus this kind of phantom switch triggered by vcpkg install ncurses can't happen, then we can avoid that.

Suggested change

if(NOT DEFINED CURSES_NEED_NCURSES)

if("@CURSES_BACKEND@" STREQUAL "ncurses")

...

elseif("@CURSES_BACKEND@" STREQUAL "pdcurses")

...

else()

message(FATAL_ERROR "Unknown backend: '@CURSES_BACKEND@'")

endif()

ports/curses/vcpkg-cmake-wrapper.cmake

BillyONeal · 2021-12-20T16:55:26Z

Thanks for fixing this!

dg0yt · 2021-12-20T17:50:47Z

what about making ports depending on pdcurses/ncurses depending on this one, instead?

This is left as a separate exercise. Since depending ports can't rely on a simple find_package(Curses) right now, this needs more work.

dg0yt · 2021-12-20T17:54:49Z

For the record:

ncurses documentation proposes #include <ncurses.h>. But IIUC the current interaction of CMake and port ncurses requires #include <ncurses/ncurses.h>, and also #include <ncurses/curses.h>.

This port changes that behaviour if CURSES_NEED_NCURSES is not set, by making the ncurses directory part of the include search path.

BillyONeal · 2021-12-22T04:46:30Z

what about making ports depending on pdcurses/ncurses depending on this one, instead?

This is left as a separate exercise. Since depending ports can't rely on a simple find_package(Curses) right now, this needs more work.

Fair enough. :) What do you think about the phantom dependency switch concern I discussed in the review comment earlier?

dg0yt · 2021-12-31T21:02:48Z

What do you think about the phantom dependency switch concern I discussed in the review comment earlier?

The phantom dependency switching is a general problem, isn't it? It is also what happens with unguarded discovery like

find_package(Pkg)
if(Pkg_FOUND)
    enable_feature(...)

Normally, we need to turn this unguarded discovery into explicit control. With the curses wrapper, I see two layers of this problem: It must do nothing if there is no explicit dependency on curses, and otherwise it must select the implementation which matches curses' dependencies.

The only way I see to handle this situation here is using vcpkg-port-config.cmake: If and only if a port directly depends on port curses, the wrapper may make a choice representing the explicit dependendencies. curses is probably used only for tools, but if it is used for a lib, the choice must also be forwarded to downstream users... perhaps feasible with cmake targets, but starting to leave the trivial sector.
vcpkg-port-config.cmake also doesn't work with user projects. No problem if there is only a single implementation installed. But with alternatives, we need user input (a variable) or we must fail.

BillyONeal · 2022-01-07T02:10:58Z

The phantom dependency switching is a general problem, isn't it?

It's a more general problem if things end up getting selected from the system but not generally if someone stays within the catalog right now.

The only way I see to handle this situation here is using vcpkg-port-config.cmake: If and only if a port directly depends on port curses, the wrapper may make a choice representing the explicit dependendencies.

I don't understand why there would need to be a vcpkg-port-config.cmake; all the port has to do is have a separate vcpkg-cmake-wrapper.cmake for pdcurses and ncurses, and install the matching one.

BillyONeal · 2022-01-07T02:11:51Z

what about making ports depending on pdcurses/ncurses depending on this one, instead?

This is left as a separate exercise. Since depending ports can't rely on a simple find_package(Curses) right now, this needs more work.

Wait, now I'm confused. Is the entire purpose of this port not to make that work?

dg0yt · 2022-01-07T06:20:55Z

It's a more general problem if things end up getting selected from the system but not generally if someone stays within the catalog right now.

It is a problem within the catalog: if a port build randomly picks a dependency already installed at build time, but doesn't make it part of its ABI hash and doesn't forward the usage requirements. In the best case, it explodes right at consuming the port in a build. In the worst case, it explodes randomly at run-time.

All the port has to do is have a separate vcpkg-cmake-wrapper.cmake for pdcurses and ncurses, and install the matching one.

For now this should work.

strega-nil-ms · 2022-01-07T18:31:33Z

ports/curses/vcpkg-cmake-wrapper.cmake

@@ -0,0 +1,20 @@
+if(NOT DEFINED CURSES_NEED_NCURSES)


Suggested change

if(NOT DEFINED CURSES_NEED_NCURSES)

if("@CURSES_BACKEND@" STREQUAL "ncurses")

...

elseif("@CURSES_BACKEND@" STREQUAL "pdcurses")

...

else()

message(FATAL_ERROR "Unknown backend: '@CURSES_BACKEND@'")

endif()

strega-nil-ms · 2022-01-07T18:34:33Z

ports/curses/portfile.cmake

@@ -0,0 +1,3 @@
+set(VCPKG_POLICY_EMPTY_PACKAGE enabled)
+file(INSTALL "${CMAKE_CURRENT_LIST_DIR}/vcpkg-cmake-wrapper.cmake" DESTINATION "${CURRENT_PACKAGES_DIR}/share/${PORT}")


Suggested change

file(INSTALL "${CMAKE_CURRENT_LIST_DIR}/vcpkg-cmake-wrapper.cmake" DESTINATION "${CURRENT_PACKAGES_DIR}/share/${PORT}")

if(VCPKG_TARGET_IS_WINDOWS AND NOT VCPKG_TARGET_IS_MINGW)

set(CURSES_BACKEND "pdcurses")

else()

set(CURSES_BACKEND "ncurses")

endif()

configure_file(

"${CMAKE_CURRENT_LIST_DIR}/vcpkg-cmake-wrapper.cmake"

"${CURRENT_PACKAGES_DIR}/share/${PORT}/vcpkg-cmake-wrapper.cmake"

@ONLY

)

I understand you don't want to re-write the logic, but I think that doing this in the build is necessary.

dg0yt · 2022-01-09T14:18:51Z

Updated.

I tried to find some test ports, but as expected, vcpkg's focus on libs doesn't help.
libmysql needs (n)curses on Linux but tells the user to install it from the system package manager.
behaviortree-cpp would use ncurses (not curses) but this is disabled in vcpkg.

BillyONeal · 2022-01-11T21:55:56Z

If someone can't be expected to just say they need curses and be able to not care then I'm not sure what this change is doing?

ports/curses/vcpkg-cmake-wrapper.cmake

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/curses/vcpkg.json
scripts/test_ports/cmake-user/vcpkg.json

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

BillyONeal · 2022-03-17T18:17:49Z

The reason I'm still in 'waiting' state here is that as far as I understand it there's still no practical use of this metaport, because every consumer described still needs to have the check for which particular curses that they got. Put another way, what would a consumer of this metaport look like?

Note that the "customer" doesn't need to be a port :)

The requested configuration bit I asked for is fixed so I'm dropping 'request changes' status but I want the consumer question answered first.

LilyWangLL · 2022-04-12T09:17:46Z

The reason I'm still in 'waiting' state here is that as far as I understand it there's still no practical use of this metaport, because every consumer described still needs to have the check for which particular curses that they got. Put another way, what would a consumer of this metaport look like?

Note that the "customer" doesn't need to be a port :)

Ping @dg0yt for response. And could you please resolve the conflicts?

LilyWangLL · 2022-05-05T07:35:33Z

Closing this PR since it seems that no progress is being made. Please reopen if work is still being done.

Add curses meta-port

04abe98

github-actions bot approved these changes Dec 19, 2021

View reviewed changes

Update versions

16806b4

dg0yt force-pushed the curses branch from 34ebb59 to 16806b4 Compare December 19, 2021 07:09

github-actions bot approved these changes Dec 19, 2021

View reviewed changes

PhoebeHui assigned LilyWangLL Dec 20, 2021

LilyWangLL added the category:new-port The issue is requesting a new library to be added; consider making a PR! label Dec 20, 2021

LilyWangLL approved these changes Dec 20, 2021

View reviewed changes

LilyWangLL added the info:reviewed Pull Request changes follow basic guidelines label Dec 20, 2021

BillyONeal previously requested changes Dec 20, 2021

View reviewed changes

BillyONeal added requires:author-response and removed info:reviewed Pull Request changes follow basic guidelines labels Dec 20, 2021

strega-nil-ms suggested changes Jan 7, 2022

View reviewed changes

dg0yt added 2 commits January 9, 2022 15:01

Burn dependency into wrapper

258b8e5

Update versions

add0498

github-actions bot approved these changes Jan 9, 2022

View reviewed changes

strega-nil-ms suggested changes Jan 11, 2022

View reviewed changes

ports/curses/vcpkg-cmake-wrapper.cmake Show resolved Hide resolved

mathisloge mentioned this pull request Jan 31, 2022

[behaviortree-cpp] update port #22877

Closed

1 task

LilyWangLL requested a review from strega-nil-ms February 14, 2022 06:45

LilyWangLL removed the requires:author-response label Feb 14, 2022

Merge branch 'master' into curses

76c1ec8

github-actions bot approved these changes Feb 22, 2022

View reviewed changes

LilyWangLL added the info:reviewed Pull Request changes follow basic guidelines label Feb 22, 2022

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

LilyWangLL closed this May 5, 2022

hihig2001 mentioned this pull request Jan 13, 2023

[New Port Request] curses (empty) to switch(link) between pdcurses (windows) and ncurses(linux) #28899

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[curses] New meta port #22093

[curses] New meta port #22093

dg0yt commented Dec 19, 2021

dg0yt commented Dec 19, 2021

cenit commented Dec 20, 2021

BillyONeal Dec 20, 2021

dg0yt Dec 20, 2021

BillyONeal Dec 22, 2021

strega-nil-ms Jan 7, 2022

BillyONeal commented Dec 20, 2021

dg0yt commented Dec 20, 2021

dg0yt commented Dec 20, 2021

BillyONeal commented Dec 22, 2021

dg0yt commented Dec 31, 2021

BillyONeal commented Jan 7, 2022

BillyONeal commented Jan 7, 2022

dg0yt commented Jan 7, 2022

strega-nil-ms Jan 7, 2022

strega-nil-ms Jan 7, 2022 •

edited

Loading

dg0yt commented Jan 9, 2022

BillyONeal commented Jan 11, 2022

github-actions bot left a comment

BillyONeal commented Mar 17, 2022 •

edited

Loading

LilyWangLL commented Apr 12, 2022 •

edited

Loading

LilyWangLL commented May 5, 2022

-if(NOT DEFINED CURSES_NEED_NCURSES)
+if("@CURSES_BACKEND@" STREQUAL "ncurses")
+    ...
+elseif("@CURSES_BACKEND@" STREQUAL "pdcurses")
+    ...
+else()
+    message(FATAL_ERROR "Unknown backend: '@CURSES_BACKEND@'")
+endif()

		@@ -0,0 +1,3 @@
		set(VCPKG_POLICY_EMPTY_PACKAGE enabled)
		file(INSTALL "${CMAKE_CURRENT_LIST_DIR}/vcpkg-cmake-wrapper.cmake" DESTINATION "${CURRENT_PACKAGES_DIR}/share/${PORT}")

-file(INSTALL "${CMAKE_CURRENT_LIST_DIR}/vcpkg-cmake-wrapper.cmake" DESTINATION "${CURRENT_PACKAGES_DIR}/share/${PORT}")
+if(VCPKG_TARGET_IS_WINDOWS AND NOT VCPKG_TARGET_IS_MINGW)
+    set(CURSES_BACKEND "pdcurses")
+else()
+    set(CURSES_BACKEND "ncurses")
+endif()
+configure_file(
+    "${CMAKE_CURRENT_LIST_DIR}/vcpkg-cmake-wrapper.cmake"
+    "${CURRENT_PACKAGES_DIR}/share/${PORT}/vcpkg-cmake-wrapper.cmake"
+    @ONLY
+)

[curses] New meta port #22093

[curses] New meta port #22093

Conversation

dg0yt commented Dec 19, 2021

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?

dg0yt commented Dec 19, 2021

cenit commented Dec 20, 2021

BillyONeal Dec 20, 2021

Choose a reason for hiding this comment

dg0yt Dec 20, 2021

Choose a reason for hiding this comment

BillyONeal Dec 22, 2021

Choose a reason for hiding this comment

strega-nil-ms Jan 7, 2022

Choose a reason for hiding this comment

BillyONeal commented Dec 20, 2021

dg0yt commented Dec 20, 2021

dg0yt commented Dec 20, 2021

BillyONeal commented Dec 22, 2021

dg0yt commented Dec 31, 2021

BillyONeal commented Jan 7, 2022

BillyONeal commented Jan 7, 2022

dg0yt commented Jan 7, 2022

strega-nil-ms Jan 7, 2022

Choose a reason for hiding this comment

strega-nil-ms Jan 7, 2022 • edited Loading

Choose a reason for hiding this comment

dg0yt commented Jan 9, 2022

BillyONeal commented Jan 11, 2022

github-actions bot left a comment

Choose a reason for hiding this comment

BillyONeal commented Mar 17, 2022 • edited Loading

LilyWangLL commented Apr 12, 2022 • edited Loading

LilyWangLL commented May 5, 2022

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

strega-nil-ms Jan 7, 2022 •

edited

Loading

BillyONeal commented Mar 17, 2022 •

edited

Loading

LilyWangLL commented Apr 12, 2022 •

edited

Loading