Extend the Curses dependency to use config tools and hand rolled search #7757

dcbaker · 2020-09-18T17:59:15Z

Like many other classic unix libraries curses is a portability pain, there's a bunch of slightly different implementations, that are found different ways. Meson already provides abstractions for making other widely available but not always easy to find libraries easier to use.

This adds all of the methods, and is tested on linux. I can test it on illumos/OpenIndiana and FreeBSD later if needed.

This doesn't resolve the fact that users will likely need to know which header they should #include. But that's a bigger change to dependencies.

Fixes #7693

dcbaker · 2020-09-18T18:00:13Z

There's also a zlib fix in here, I can split that out if others prefer.

dcbaker · 2020-09-18T21:51:05Z

Hmmmm, not sure why msys fails while everything else succeeds.

mesonbuild/dependencies/misc.py

test cases/frameworks/31 curses/meson.build

The-King-of-Toasters · 2020-09-19T04:23:24Z

I have access to a couple of systems, will mark off when tested:

OpenBSD - current and stable (uses system).
OmniOS (can use pkg-config, config-tool and system).
Arch Linux (can use pkg-config, config-tool and system).
FreeBSD (system with base ncurses, pkg-config with ncurses 6 from ports).
macOS Catalina KVM (can use pkg-config with homebrew, config-tool installed as ncurse5.4-config???).

nirbheek

LGTM, minus typos already pointed out.

lygstate · 2020-09-19T16:26:26Z

Also, hope msys2/mingw to be tested.

lygstate · 2020-09-23T12:51:51Z

@dcbaker do you have time to update the pull request?

dcbaker · 2020-09-23T13:52:36Z

Yes, I have the comments addressed locally, in just trying to get an msys environment set up to figure out why that's failing

lygstate · 2020-09-23T14:22:54Z

Yes, I have the comments addressed locally, in just trying to get an msys environment set up to figure out why that's failing

maybe I can help the msys environment, I've already steup it.

lygstate · 2020-09-23T15:15:29Z

2020-09-18T20:33:47.0655611Z Source dir: D:\a\1\s\test cases\frameworks\31 curses
2020-09-18T20:33:47.0655960Z Build dir: D:\a\1\s\b efdeae105d
2020-09-18T20:33:47.0656322Z Build type: native build
2020-09-18T20:33:47.0656605Z Project name: curses
2020-09-18T20:33:47.0656884Z Project version: undefined
2020-09-18T20:33:47.0657356Z C compiler for the host machine: cc (gcc 10.2.0 "cc (Rev1, Built by MSYS2 project) 10.2.0")
2020-09-18T20:33:47.0657807Z C linker for the host machine: cc ld.bfd 2.35
2020-09-18T20:33:47.0658199Z Host machine cpu family: x86
2020-09-18T20:33:47.0658494Z Host machine cpu: x86
2020-09-18T20:33:47.0658943Z ncursesw6-config found: YES (sh D:\a\msys64\mingw32\bin\ncursesw6-config) 6.2.20200212
2020-09-18T20:33:47.0659413Z Run-time dependency curses found: YES 6.2.20200212
2020-09-18T20:33:47.0659925Z Dependency curses found: NO found 6.2.20200212 but need: '> 1000000' (cached)
2020-09-18T20:33:47.0660557Z WARNING: Project targeting '>= 0.51.1' but tried to use feature introduced in '0.53.0': assert function without message argument.
2020-09-18T20:33:47.0661082Z Build targets in project: 1
2020-09-18T20:33:47.0661653Z WARNING: Project specifies a minimum meson_version '>= 0.51.1' but uses features which were added in newer versions:
2020-09-18T20:33:47.0662328Z  * 0.53.0: {'assert function without message argument'}
2020-09-18T20:33:47.0662753Z WARNING: Deprecated features used:
2020-09-18T20:33:47.0663072Z  * 0.48.0: {'python3 module'}
2020-09-18T20:33:47.0663451Z  * 0.55.0: {'cmake_options arg in subproject'}
2020-09-18T20:33:47.0663697Z 
2020-09-18T20:33:47.0663987Z Found ninja-1.10.1 at D:\a\msys64\mingw32\bin\ninja.EXE
2020-09-18T20:33:47.0664988Z [1/2] "cc" "-Ibasic.exe.p" "-I." "-I..\test cases\frameworks\31 curses" "-I/mingw32/include/ncursesw" "-I/mingw32/include" "-fdiagnostics-color=always" "-pipe" "-D_FILE_OFFSET_BITS=64" "-Wall" "-Winvalid-pch" "-g" "-D_XOPEN_SOURCE=600" "-D_POSIX_C_SOURCE=199506L" -MD -MQ basic.exe.p/main.c.obj -MF "basic.exe.p\main.c.obj.d" -o basic.exe.p/main.c.obj "-c" "../test cases/frameworks/31 curses/main.c"
2020-09-18T20:33:47.0666017Z FAILED: basic.exe.p/main.c.obj 
2020-09-18T20:33:47.0666917Z "cc" "-Ibasic.exe.p" "-I." "-I..\test cases\frameworks\31 curses" "-I/mingw32/include/ncursesw" "-I/mingw32/include" "-fdiagnostics-color=always" "-pipe" "-D_FILE_OFFSET_BITS=64" "-Wall" "-Winvalid-pch" "-g" "-D_XOPEN_SOURCE=600" "-D_POSIX_C_SOURCE=199506L" -MD -MQ basic.exe.p/main.c.obj -MF "basic.exe.p\main.c.obj.d" -o basic.exe.p/main.c.obj "-c" "../test cases/frameworks/31 curses/main.c"
2020-09-18T20:33:47.0668018Z ../test cases/frameworks/31 curses/main.c:1:10: fatal error: curses.h: No such file or directory
2020-09-18T20:33:47.0668507Z     1 | #include "curses.h"
2020-09-18T20:33:47.0668792Z       |          ^~~~~~~~~~
2020-09-18T20:33:47.0669129Z compilation terminated.
2020-09-18T20:33:47.0669439Z ninja: build stopped: subcommand failed.
2020-09-18T20:33:47.0670093Z  
2020-09-18T20:33:47.0670309Z 
2020-09-18T20:33:47.0671771Z CI COMMAND ci_include:
2020-09-18T20:33:47.0722277Z Included file D:\a\1\s\b efdeae105d\build.ninja:
2020-09-18T20:33:47.0722991Z # This is the build file for project "curses"
2020-09-18T20:33:47.0723956Z # It is autogenerated by the Meson build system.
2020-09-18T20:33:47.0724462Z # Do not edit by hand.
2020-09-18T20:33:47.0724869Z 
2020-09-18T20:33:47.0725252Z ninja_required_version = 1.7.1

didn't found this header

lygstate · 2020-09-23T15:28:15Z

Hi, I've found the cause of the msys2 error:


lygstate@DESKTOP-94PU0GB MINGW64 ~
$ pkg-config ncursesw --cflags
-D_XOPEN_SOURCE=600 -D_POSIX_C_SOURCE=199506L -IC:/CI-Tools/msys64/mingw64/include/ncursesw

lygstate@DESKTOP-94PU0GB MINGW64 ~
$ ncursesw6-config --cflags
-D_XOPEN_SOURCE=600 -D_POSIX_C_SOURCE=199506L -I/mingw64/include/ncursesw -I/mingw64/include

The ncursesw6-config are showing the msys2 style path, but pkg-config ncursesw --cflags showing the windows style path, and the mingw/gcc compiler just recoginize the windows sytle path, didn't recognize the msys2 style path, so we should return the windows style path instead.

lygstate · 2020-09-23T15:30:14Z

also, lack of pdcurses detection on msys2

lygstate · 2020-09-23T16:24:08Z

@dcbaker Hi, I've fixes the msys2 for you pull request and add pdcurses support on msys2, you can cherry pick my patches
at
#7773

lygstate · 2020-09-23T16:24:45Z

I didn't fixes the reviews for your pull reqeust, I create that just used to verify if the CI passed

dcbaker · 2020-09-23T19:56:23Z

@lazka I'm not sure what to do about the config-tool thing. Is there someway for us to patch up the path so that it works correctly?

dcbaker · 2020-09-23T20:13:39Z

@lazka: is there something we can do to fix the config tools (or should I open a bug against msys somewhere?)

@lygstate I pulled your patches in, but I split the adding of pdcurses and disabling of configtool on windows into two separate patches.

lygstate · 2020-09-23T20:47:01Z

@lazka: is there something we can do to fix the config tools (or should I open a bug against msys somewhere?)

@lygstate I pulled your patches in, but I split the adding of pdcurses and disabling of configtool on windows into two separate patches.

No problem, I found my pull request still have problem, and I know why, on msys2/mingw when the curses are detect by system
we need add the include path manually, Don't know if other system also need that.

dcbaker · 2020-09-23T21:09:37Z

Not that I've seen macos, linux, freebsd, and openindiana all just work.

lygstate · 2020-09-23T21:20:25Z

Not that I've seen macos, linux, freebsd, and openindiana all just work.

Yeap, mingw maybe a differen things, but from qemu, there must some platform need the include

lygstate · 2020-09-23T22:02:00Z

I see why things goes wrong
The mingw path convert procedure are not properly handled

    def _convert_mingw_paths(self, args: T.List[str]) -> T.List[str]:
        '''
        Both MSVC and native Python on Windows cannot handle MinGW-esque /c/foo
        paths so convert them to C:/foo. We cannot resolve other paths starting
        with / like /home/foo so leave them as-is so that the user gets an
        error/warning from the compiler/linker.
        '''
        if not mesonlib.is_windows():
            return args
        converted = []
        for arg in args:
            pargs = []
            # Library search path
            if arg.startswith('-L/'):
                pargs = PurePath(arg[2:]).parts
                tmpl = '-L{}:/{}'
            elif arg.startswith('-I/'):
                pargs = PurePath(arg[2:]).parts
                tmpl = '-I{}:/{}'
            # Full path to library or .la file
            elif arg.startswith('/'):
                pargs = PurePath(arg).parts
                tmpl = '{}:/{}'
            elif arg.startswith(('-L', '-I')) or (len(arg) > 2 and arg[1] == ':'):
                # clean out improper '\\ ' as comes from some Windows pkg-config files
                arg = arg.replace('\\ ', ' ')
            if len(pargs) > 1 and len(pargs[1]) == 1:
                arg = tmpl.format(pargs[1], '/'.join(pargs[2:]))
            converted.append(arg)
        return converted

So we have no need to disable CursesConfigToolDependency on windows
Just need to fixes _convert_mingw_paths, and this is also whey qemu building with meson failed.

lygstate · 2020-09-23T23:16:06Z

@dcbaker I've rebase and update with #7773 to fixes the CI, and from that we know the existing mingw path handling are error prone. we need use cygpath to do proper path convert.

So the pkg-config and custom config-tool both need use that.

lazka · 2020-09-24T05:15:25Z

@lazka: is there something we can do to fix the config tools (or should I open a bug against msys somewhere?)

I've filed msys2/MINGW-packages#7025

lygstate · 2020-09-24T05:29:06Z

@lazka: is there something we can do to fix the config tools (or should I open a bug against msys somewhere?)

I've filed msys2/MINGW-packages#7025

I've fixes it consistence with pkg-config in pull request #7773
So there is no need the mingw bug anymore

lazka · 2020-09-24T05:55:30Z

So there is no need the mingw bug anymore

I assume fixing it doesn't hurt though? I've done so in msys2/MINGW-packages#7026

lygstate · 2020-09-24T06:00:33Z

So there is no need the mingw bug anymore

I assume fixing it doesn't hurt though? I've done so in msys2/MINGW-packages#7026

Not hurt:), just a bit boring, every package need consider that

lygstate · 2020-09-25T16:59:12Z

@dcbaker I've updated #7773 (comment) and fixes all compiling issues under win32, please have a look and cherry-pick it

jpakkane · 2020-09-29T16:54:34Z

How does this tie in with #7773 then? Should both of them be merged or only one? If one, which one?

dcbaker · 2020-09-29T21:57:16Z

What I think I'd like to do, is just work around the windows issue for now and merge this, then we'll take the msys issues @lygstate is trying to fix one at a time in a separate MR. does that sound reasonable?

The big difference between them is that 7773 is trying to fix a bunch of msys issues as well (that break using this dependency)

look for (in order): ncursesw, ncurses, curses.

These are mostly duplicated with pkg-config, but maybe someone has one but not another, and they're easy to turn on with the ConfigToolDependency.

has_header returns a tuple of (found: bool, cached: bool), so `if has_header` will always return true because the tuple is non-empty. We need to check if the found value is true or not.

That calls find_library and has_header in conjunction to look for curses implementations that are baked into the system without any other find method.

This is mostly important for the system dependency where we need to roll the version check ourselves.

On win32 there is pdcurses, so we detect it first, because python depends on ncursesw, so if we don't want to use ncursesw, we should make sure pdcurses detect before ncursesw

… windows with msys ncurses-config returns a unix style path (currently, though it's been fixed upstream), which the compilers don't understand, so we can't do that. Additionally, while the system search does work, there's missing include directories that need to be added.

lygstate · 2020-09-30T05:51:17Z

What I think I'd like to do, is just work around the windows issue for now and merge this, then we'll take the msys issues @lygstate is trying to fix one at a time in a separate MR. does that sound reasonable?

The big difference between them is that 7773 is trying to fix a bunch of msys issues as well (that break using this dependency)

reasonable

dcbaker · 2020-09-30T16:33:38Z

Okay, I've gone ahead and merged this, let me go take a closer look at 7773 now.

dcbaker requested a review from scivision September 18, 2020 17:59

dcbaker requested a review from jpakkane as a code owner September 18, 2020 17:59

dcbaker added the dependencies label Sep 18, 2020

dcbaker force-pushed the submit/curses-dependency branch from f9de21b to ef5cf66 Compare September 18, 2020 19:31

The-King-of-Toasters reviewed Sep 19, 2020

View reviewed changes

mesonbuild/dependencies/misc.py Outdated Show resolved Hide resolved

test cases/frameworks/31 curses/meson.build Outdated Show resolved Hide resolved

nirbheek approved these changes Sep 19, 2020

View reviewed changes

lygstate mentioned this pull request Sep 23, 2020

dependencies/curses: Fixes curses detection properly on msys2/mingw #7773

Open

dcbaker force-pushed the submit/curses-dependency branch from ef5cf66 to e8982e9 Compare September 23, 2020 20:12

lygstate mentioned this pull request Sep 23, 2020

_convert_mingw_paths should not only in PkgConfigDependency, but should also for system and config-tool #7777

Open

lazka mentioned this pull request Sep 24, 2020

[ncurses] ncursesw6-config returns unix style paths msys2/MINGW-packages#7025

Closed

dcbaker and others added 10 commits September 29, 2020 14:58

dependencies/misc: Fix typing of curses_factory

c411b29

docs/Dependencies Add missing curses documentation

16d3513

dependency/misc: change lookup order for curses pkg-config

354c1c1

look for (in order): ncursesw, ncurses, curses.

dependencies/curses: Add support for using the ncurses config tools

7d11d7c

These are mostly duplicated with pkg-config, but maybe someone has one but not another, and they're easy to turn on with the ConfigToolDependency.

dependencies/zlib: Fix header detection

cb6ccf2

has_header returns a tuple of (found: bool, cached: bool), so `if has_header` will always return true because the tuple is non-empty. We need to check if the found value is true or not.

dependencies/curses: Add a system dependency

5aee856

That calls find_library and has_header in conjunction to look for curses implementations that are baked into the system without any other find method.

tests/curses: Extend to test versions

19f2b3d

This is mostly important for the system dependency where we need to roll the version check ourselves.

dependencies/curses: Add way to testing curses on msys2.

e873e64

dependencies/curses: Add support for pdcurses

9a47509

On win32 there is pdcurses, so we detect it first, because python depends on ncursesw, so if we don't want to use ncursesw, we should make sure pdcurses detect before ncursesw

dcbaker force-pushed the submit/curses-dependency branch from e8982e9 to a310677 Compare September 29, 2020 22:01

dcbaker merged commit ce2d927 into mesonbuild:master Sep 30, 2020

dcbaker deleted the submit/curses-dependency branch September 30, 2020 16:33

Extend the Curses dependency to use config tools and hand rolled search #7757

Extend the Curses dependency to use config tools and hand rolled search #7757

dcbaker commented Sep 18, 2020

dcbaker commented Sep 18, 2020

dcbaker commented Sep 18, 2020 •

edited

The-King-of-Toasters commented Sep 19, 2020 •

edited

nirbheek left a comment

lygstate commented Sep 19, 2020

lygstate commented Sep 23, 2020

dcbaker commented Sep 23, 2020

lygstate commented Sep 23, 2020

lygstate commented Sep 23, 2020

lygstate commented Sep 23, 2020

lygstate commented Sep 23, 2020

lygstate commented Sep 23, 2020

lygstate commented Sep 23, 2020

dcbaker commented Sep 23, 2020

dcbaker commented Sep 23, 2020

lygstate commented Sep 23, 2020

dcbaker commented Sep 23, 2020

lygstate commented Sep 23, 2020

lygstate commented Sep 23, 2020

lygstate commented Sep 23, 2020

lazka commented Sep 24, 2020

lygstate commented Sep 24, 2020

lazka commented Sep 24, 2020

lygstate commented Sep 24, 2020 •

edited

lygstate commented Sep 25, 2020

jpakkane commented Sep 29, 2020

dcbaker commented Sep 29, 2020 •

edited

lygstate commented Sep 30, 2020

dcbaker commented Sep 30, 2020

Extend the Curses dependency to use config tools and hand rolled search #7757

Extend the Curses dependency to use config tools and hand rolled search #7757

Conversation

dcbaker commented Sep 18, 2020

dcbaker commented Sep 18, 2020

dcbaker commented Sep 18, 2020 • edited

The-King-of-Toasters commented Sep 19, 2020 • edited

nirbheek left a comment

Choose a reason for hiding this comment

lygstate commented Sep 19, 2020

lygstate commented Sep 23, 2020

dcbaker commented Sep 23, 2020

lygstate commented Sep 23, 2020

lygstate commented Sep 23, 2020

lygstate commented Sep 23, 2020

lygstate commented Sep 23, 2020

lygstate commented Sep 23, 2020

lygstate commented Sep 23, 2020

dcbaker commented Sep 23, 2020

dcbaker commented Sep 23, 2020

lygstate commented Sep 23, 2020

dcbaker commented Sep 23, 2020

lygstate commented Sep 23, 2020

lygstate commented Sep 23, 2020

lygstate commented Sep 23, 2020

lazka commented Sep 24, 2020

lygstate commented Sep 24, 2020

lazka commented Sep 24, 2020

lygstate commented Sep 24, 2020 • edited

lygstate commented Sep 25, 2020

jpakkane commented Sep 29, 2020

dcbaker commented Sep 29, 2020 • edited

lygstate commented Sep 30, 2020

dcbaker commented Sep 30, 2020

dcbaker commented Sep 18, 2020 •

edited

The-King-of-Toasters commented Sep 19, 2020 •

edited

lygstate commented Sep 24, 2020 •

edited

dcbaker commented Sep 29, 2020 •

edited