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.

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

[ncurses] Add new port #17226

Merged
merged 3 commits into from
Apr 30, 2021
Merged

[ncurses] Add new port #17226

merged 3 commits into from
Apr 30, 2021

Conversation

Hoikas
Copy link
Contributor

@Hoikas Hoikas commented Apr 12, 2021

Adds the port ncurses.

@Hoikas Hoikas mentioned this pull request Apr 12, 2021
@JackBoosY JackBoosY added the category:new-port The issue is requesting a new library to be added; consider making a PR! label Apr 12, 2021
@Hoikas Hoikas marked this pull request as draft April 12, 2021 03:09
@Hoikas Hoikas force-pushed the ncurses branch 2 times, most recently from da50102 to 421ca38 Compare April 12, 2021 03:20
@Hoikas Hoikas marked this pull request as ready for review April 12, 2021 03:25
@JackBoosY JackBoosY added the info:reviewed Pull Request changes follow basic guidelines label Apr 12, 2021
@electron271
Copy link

Trying to install right now..

@Neumann-A
Copy link
Contributor

add testing via cmake test port in script/test_ports. The option for cmake is BUILD_CursesDialog

@electron271
Copy link

would it be possible to make it work with windows/uwp? or do I have to stick with pdcurses

@Hoikas
Copy link
Contributor Author

Hoikas commented Apr 12, 2021

add testing via cmake test port in script/test_ports. The option for cmake is BUILD_CursesDialog

I'm sorry, but I don't understand this statement.

would it be possible to make it work with windows/uwp? or do I have to stick with pdcurses

You will need to use MinGW for this to work on Windows. I do not have a functioning MinGW setup, so I cannot support that.

@Neumann-A
Copy link
Contributor

I'm sorry, but I don't understand this statement.

To test if ncurses can be correctly used it is helpful to build an application with it. Since CMake supports building with some sort of Curses dependency it would be appropriate to include it in the cmake build by settting BUILD_CursesDialog=ON after

-DBUILD_QtDialog=ON # Just to test Qt with CMake
and add a dependency on it in the manifest.

This way it can be checked if it actually builds a useable dialog and links correctly.

@electron271
Copy link

electron271 commented Apr 12, 2021

You will need to use MinGW for this to work on Windows. I do not have a functioning MinGW setup, so I cannot support that.

well then I guess i'm going to stick to pdcurses or some other lib

@Neumann-A
Copy link
Contributor

@Hoikas You probably need to comment out:

file(REMOVE_RECURSE ${CURRENT_PACKAGES_DIR}/bin)

for a CI run so that you can get the application name. Otherwise it will simply be deleted.

@Hoikas
Copy link
Contributor Author

Hoikas commented Apr 12, 2021

Will that be a permanent change, or just so we can test ccmake locally?

@Neumann-A
Copy link
Contributor

Will that be a permanent change, or just so we can test ccmake locally?

temporary but since you seem to already now the application name you can add it to the copy tools section of the port. Somewhere around here:

set(_tools cmake cmake-gui ctest cpack)
if(VCPKG_TARGET_IS_WINDOWS)
list(APPEND _tools cmcldeps)
endif()
vcpkg_copy_tools(TOOL_NAMES ${_tools} AUTO_CLEAN)

Do you know the deps of ncurses? Maybe I can make it somehow work for windows in #9966.

@Hoikas
Copy link
Contributor Author

Hoikas commented Apr 12, 2021

Very minimal deps. There's an optional pcre2 dependency (that we're not opting into) and getopt is needed to build the programs. I initially wrote the port on Windows but got nasty build errors about things like DWORD not being defined. It was more than I really wanted to investigate at the time 😅

@strega-nil-ms
Copy link
Contributor

Thanks @Hoikas, this is great :)

@strega-nil-ms strega-nil-ms merged commit cc726b6 into microsoft:master Apr 30, 2021
@Hoikas Hoikas deleted the ncurses branch April 30, 2021 18:28
@GitMensch
Copy link
Contributor

Does it "just not compile" with MSVC or were there other reasons to not integrate it?
Years ago this was doomed to work because of the missing termcap model but as ncurses 5.8 added "added a terminal driver for Windows console" and more recent MSVC versions are not as limited as old ones I've thought that this would work - wouldn't it?

@JackBoosY
Copy link
Contributor

@GitMensch Please open a new issue to request Windows support.

@Hoikas
Copy link
Contributor Author

Hoikas commented Dec 10, 2021

ncurses does not compile with MSVC; however, I suspect you may find some success with mingw or clang-cl.

@dg0yt
Copy link
Contributor

dg0yt commented Dec 10, 2021

Note that there is port pdcurses for windows. (However it seems to need some maintenance, in particular a port to vcpkg_build_nmake which might also fix issues with some triplets.)
Last not least there could be a curses meta-port with specific dependencies for each platform.

@GitMensch
Copy link
Contributor

ncurses does not compile with MSVC; ...

Can you point to build tests / documentation for this?

... however, I suspect you may find some success with mingw or clang-cl.

mingw should definitely work as this is what is used in MSYS2.
Are there compiler specific triplets?
Do you suggest compiling ncurses with mingw, then linking against the result with msvc?

Last not least there could be a curses meta-port with specific dependencies for each platform.

That would be very reasonable, and as long as each port installs a curses.h (and/or adds -DHAVE_PDCURSES_H and similar) would really help with portability of recipes - a central place for that is much better than conditionals in every package that wants to use curses to let it compile both on Win32 and on other triplets.

@dg0yt
Copy link
Contributor

dg0yt commented Dec 10, 2021

ncurses does not compile with MSVC; ...

Can you point to build tests / documentation for this?

The citation you have given, complete line:

add a terminal driver for Windows console, which supports a MinGW port to Windows.

Related:
https://invisible-island.net/ncurses/ncurses.html#download_mingw

Additionally:

It should port easily to any ANSI/POSIX-conforming UNIX.

Future plans
...
Ports to more systems, including DOS and Windows.

Briefly: Not enough POSIX with MSVC.

@dg0yt dg0yt mentioned this pull request Dec 19, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category:new-port The issue is requesting a new library to be added; consider making a PR! info:reviewed Pull Request changes follow basic guidelines
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[New Port Request] ncurses [New port Request] ncurses
8 participants