Explicitely link against tinfo. Needed for ncurses 6 by kripton · Pull Request #1621 · OpenLightingProject/ola

kripton · 2020-03-15T12:32:41Z

This fixes a compilation error when ncurses is installed in version 6 only:

libtool: link: x86_64-pc-linux-gnu-g++ -I./include -I./include -Wall -Wformat -W -fvisibility-inlines-hidden -pthread -march=native -O2 -pipe -fomit-frame-pointer -std=gnu++11 -Wno-error=deprecated-declarations -pthread -Wl,-O1 -o examples/.libs/ola_dmxmonitor examples/ola-dmxmonitor.o  -Wl,--as-needed common/.libs/libolacommon.so ola/.libs/libola.so /var/tmp/portage/app-misc/ola-9999/work/ola-9999/common/.libs/libolacommon.so -lresolv -lpthread -lprotobuf -lncurses -ldl -pthread
/usr/lib/gcc/x86_64-pc-linux-gnu/9.2.0/../../../../x86_64-pc-linux-gnu/bin/ld: examples/ola-dmxmonitor.o: undefined reference to symbol 'resetty'
/usr/lib/gcc/x86_64-pc-linux-gnu/9.2.0/../../../../x86_64-pc-linux-gnu/bin/ld: /lib64/libtinfo.so.6: error adding symbols: DSO missing from command line
collect2: error: ld returned 1 exit status

I'm unable to test if this also works with ncurses 5, so I'm sending this as a draft and Travis will tell us if it's okay for ncurses 5 as well.

kripton · 2020-03-15T14:36:41Z

Meh, fails on macOS :-/
Will see if I can make it conditional using Autotools

kripton · 2020-03-15T14:45:04Z

kripton@momo ~ $ pkg-config  --libs ncurses
-lncurses -ltinfo

seems to be the way to go

peternewman · 2020-03-15T20:45:51Z

kripton@momo ~ $ pkg-config  --libs ncurses
-lncurses -ltinfo

seems to be the way to go

That sounds perfect. I was going to suggest checking if tinfo existed as a package and doing some clever stuff, but just using pkg-config should be even easier and you can copy some existing Makefile stuff.

Can you target 0.10 branch with this please too.

…ything about ncurses

kripton · 2020-03-17T21:46:48Z

Implementation done, let's see how Travis (and you ;)) like that one.
I've kept the old logic and just made some additions mainly due to one thing:
The location of the header files seem to be in a different location on some systems:

ola/configure.ac

Line 442 in 4faf2c5

# Location is ncurses/curses.h on DragonFly

I know that pkg-config won't tell you the exact location of header files, only via CFLAGS and only indirect there. And since I don't have a system running Dragonfly to test, I better not touched that part.

kripton · 2020-03-17T21:47:21Z

And sure, when this is approved to be merged, I'll rebase the changes on 0.10 branch and send a new PR for that as well 👍

peternewman

LGTM

kripton · 2020-03-18T05:46:42Z

Mh, doesn't look good to Travis:
checking for libncurses... configure: error: Package requirements (ncurses >= 5) were not met:
It's a thing that I hoped does not happen: PKG_CHECK_MODULES fails the configure step if not found. I'll see how to make it not fail.

peternewman

That's even worse now. I think you still need the AM_CONDITIONAL for the variable to exist in the Makefiles, see the FTDI stuff for example (so using the lower case interim state probably makes sense).

peternewman · 2020-03-18T12:49:10Z

I've marked it ready for review to see if that fixes the odd Travis python-wip test which should be an allowed failure.

peternewman · 2020-03-18T16:11:39Z

And sure, when this is approved to be merged, I'll rebase the changes on 0.10 branch and send a new PR for that as well +1

You could target 0.10 directly if you want, then we can merge that into master from there, whichever is easier?

kripton · 2020-03-18T17:21:17Z

That's even worse now. I think you still need the AM_CONDITIONAL for the variable to exist in the Makefiles, see the FTDI stuff for example (so using the lower case interim state probably makes sense).

Yep, agreed. Those changes were just a quick shot this morning before I started my day-time-job. It just shows my inexperience with automake but I'm working on it ;)

You could target 0.10 directly if you want, then we can merge that into master from there, whichever is easier?

For development right now, master is easier. But as I said, I'll rebase on top of 0.10 once we have a solution we both like.

kripton · 2020-03-18T19:32:39Z

@peternewman: This looks good to me now. I've looked at how it is implemented for ftdi and adopted the lower-case syntax.
Travis looks good (python-wip still flaky but I didn't change anything related to Python, so I don't see a problem there.
Two of four macOS runners are green 👍 (EDIT: two others are not failed, still pending)

I'm currently setting up a DragonFly BSD VM to see how pkg-config behaves there now regarding to ncurses. But since macOS doesn't have pkg-config OR no pkg-config info for ncurses, I think I'll leave this as is

kripton · 2020-03-18T19:55:04Z

Can't get pkg-config to work at all on Dragonfly. But with the changes done here, it should not affect building on Dragonfly.

peternewman

Thanks for this, just one more comment to tweak it a bit if you don't mind.

peternewman · 2020-03-18T23:50:26Z

+# Defines libncurses_LIBS that we can use to link against ncurses
+# Needed since >=ncurses-6 where -ltinfo is needed in addition to -lncurses
+PKG_CHECK_MODULES(libncurses, [ncurses >= 5])
+AM_CONDITIONAL([HAVE_NCURSES_PKGCONFIG], [test "$libncurses_LIBS" != ""])


I think this would fail for some theoretical system which had ncurses support built in, so didn't require a flag, so you'd fall back to adding one which would upset it. See avahi for a better method, where we set a status if PKG_CHECK_MODULES finds the module, and then test that to set the conditional.

You shouldn't need to change the Makefile code, just configure to fix that.

Mh, not sure if I can follow. The current solution (before this PR), checks for libncurses and adds -lncurses here: https://github.com/OpenLightingProject/ola/pull/1621/files#diff-83cb084b35952ddc152db3cfbc85b9f9L94.
So wouldn't that fail in the current master as well? In that theoretical case where ncurses is "built-in"

peternewman · 2020-03-18T23:54:38Z

Yep, agreed. Those changes were just a quick shot this morning before I started my day-time-job. It just shows my inexperience with automake but I'm working on it ;)

No worries, I was just flagging it up in case you hadn't seen it. Most of my automake is still copy/paste existing stuff.

For development right now, master is easier. But as I said, I'll rebase on top of 0.10 once we have a solution we both like.

Fine, in general (until we drop 0.10 branch release (probably at the next release), bugfixes can be targeted at 0.10 and new features at master.

Can't get pkg-config to work at all on Dragonfly. But with the changes done here, it should not affect building on Dragonfly.

Fair enough, I've only used it briefly by proxy. Out of interest, did it build with make/make check okay? I don't suppose you fancy turning that VM into a Buildbot slave for the occasional run?

peternewman · 2020-03-19T00:51:46Z

Travis looks good (python-wip still flaky but I didn't change anything related to Python, so I don't see a problem there.

That pychecker-wip test should be in allowed failures, I don't really understand how it's escaped! Edit, after a bit more reading and guesswork I've pinned it down. Can you pull the contents of this PR into your PR too or make the equivalent change to .travis.yml:
https://github.com/OpenLightingProject/ola/pull/1622/files

kripton · 2020-03-19T21:42:16Z

No worries, I was just flagging it up in case you hadn't seen it. Most of my automake is still copy/paste existing stuff.

👍 I also hope to gain experience there to finish #1188 some time ...

Fine, in general (until we drop 0.10 branch release (probably at the next release), bugfixes can be targeted at 0.10 and new features at master.

Understood :) However, with current master not building for me (due to older protobuf being required), it's a lots of cherry-picking and rebasing stuff around. But we are working on it!

Fair enough, I've only used it briefly by proxy. Out of interest, did it build with make/make check okay? I don't suppose you fancy turning that VM into a Buildbot slave for the occasional run?

Hm, that VM was just a quick to check how pkg-config behaves there and where the ncurses headers are. I actually didn't try to build OLA there. The learning curve was quite steep and I just assumed that my changes didn't affect building on Dragonfly anyways. I might try it again ;)

kripton · 2020-03-19T21:47:03Z

That pychecker-wip test should be in allowed failures, I don't really understand how it's escaped! Edit, after a bit more reading and guesswork I've pinned it down. Can you pull the contents of this PR into your PR too or make the equivalent change to .travis.yml:
https://github.com/OpenLightingProject/ola/pull/1622/files

Erm, yes, I could add those changes to this PR. I could also create a new one with those changes. Why don't we just merge #1622 ?

peternewman

Just a few minor tweaks.

peternewman · 2020-03-19T23:45:28Z

That pychecker-wip test should be in allowed failures, I don't really understand how it's escaped! Edit, after a bit more reading and guesswork I've pinned it down. Can you pull the contents of this PR into your PR too or make the equivalent change to .travis.yml:
https://github.com/OpenLightingProject/ola/pull/1622/files

Erm, yes, I could add those changes to this PR. I could also create a new one with those changes. Why don't we just merge #1622 ?

Various fiddly reasons, but Travis came good so I've done those and merged it.

peternewman · 2020-03-19T23:50:15Z

No worries, I was just flagging it up in case you hadn't seen it. Most of my automake is still copy/paste existing stuff.

+1 I also hope to gain experience there to finish #1188 some time ...

Cool, let us know if we can hope at all.

Fine, in general (until we drop 0.10 branch release (probably at the next release), bugfixes can be targeted at 0.10 and new features at master.

Understood :) However, with current master not building for me (due to older protobuf being required), it's a lots of cherry-picking and rebasing stuff around. But we are working on it!

Okay I understand.

Fair enough, I've only used it briefly by proxy. Out of interest, did it build with make/make check okay? I don't suppose you fancy turning that VM into a Buildbot slave for the occasional run?

Hm, that VM was just a quick to check how pkg-config behaves there and where the ncurses headers are. I actually didn't try to build OLA there. The learning curve was quite steep and I just assumed that my changes didn't affect building on Dragonfly anyways. I might try it again ;)

Heh no worries, I don't think Dragonfly is particularly important, just different as someone brought it up once.

kripton · 2020-03-20T19:47:25Z

Indentation fixed and made sure that have_ncurses_pkgconfig has a value (yes/no) defined after the PKG-CONFIG check

peternewman

LGTM

kripton · 2020-03-21T18:04:03Z

LGTM

Great :) Can you merge it? I don't have write access.
I've rebased that branch on top of 0.10: #1623

peternewman · 2020-03-21T18:29:05Z

LGTM

Great :) Can you merge it? I don't have write access.
I've rebased that branch on top of 0.10: #1623

Yeah waiting for Travis to play ball.

kripton added 2 commits March 17, 2020 22:37

Use pkg-info to detect library flags to link against ncurses

2022d7d

Add a fallback to the old behaviour in case pkg-config didn't know an…

548b62a

…ything about ncurses

kripton force-pushed the ncurses-6 branch from 4779ceb to 548b62a Compare March 17, 2020 21:40

peternewman approved these changes Mar 17, 2020

View reviewed changes

peternewman requested changes Mar 18, 2020

View reviewed changes

peternewman marked this pull request as ready for review March 18, 2020 12:48

Make configure not failing if PKG_CHECK_MODULES fails on ncurses

4a3516e

kripton force-pushed the ncurses-6 branch from 113ecc3 to 4a3516e Compare March 18, 2020 18:26

peternewman requested changes Mar 18, 2020

View reviewed changes

kripton mentioned this pull request Mar 19, 2020

app-misc/ola: Add to tree in version 0.10.7 gentoo/gentoo#15017

Closed

kripton mentioned this pull request Mar 19, 2020

Use Raspi's GPIO pins as secondary UART #1188

Open

peternewman requested changes Mar 19, 2020

View reviewed changes

Comment thread configure.ac Outdated

Comment thread configure.ac Outdated

Comment thread configure.ac Outdated

Merge branch 'master' into ncurses-6

7f5f9fd

Indentation fixes

eee16b5

Indentation fixes, part 2

80511ad

peternewman approved these changes Mar 21, 2020

View reviewed changes

kripton mentioned this pull request Mar 21, 2020

Ncurses 6 0.10 #1623

Merged

peternewman merged commit e9fcf75 into OpenLightingProject:master Mar 22, 2020

kripton deleted the ncurses-6 branch March 29, 2020 13:27

peternewman added this to the 0.11.0 milestone Mar 2, 2024

peternewman mentioned this pull request Jul 12, 2026

Add 0.11 news #2060

Draft

Uh oh!

Conversation

kripton commented Mar 15, 2020

Uh oh!

kripton commented Mar 15, 2020

Uh oh!

kripton commented Mar 15, 2020

Uh oh!

peternewman commented Mar 15, 2020

Uh oh!

kripton commented Mar 17, 2020

Uh oh!

kripton commented Mar 17, 2020

Uh oh!

peternewman left a comment

Choose a reason for hiding this comment

Uh oh!

kripton commented Mar 18, 2020

Uh oh!

peternewman left a comment

Choose a reason for hiding this comment

Uh oh!

peternewman commented Mar 18, 2020

Uh oh!

peternewman commented Mar 18, 2020

Uh oh!

kripton commented Mar 18, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kripton commented Mar 18, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kripton commented Mar 18, 2020

Uh oh!

peternewman left a comment

Choose a reason for hiding this comment

Uh oh!

peternewman Mar 18, 2020

Choose a reason for hiding this comment

Uh oh!

kripton Mar 19, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

peternewman commented Mar 18, 2020

Uh oh!

peternewman commented Mar 19, 2020

Uh oh!

kripton commented Mar 19, 2020

Uh oh!

kripton commented Mar 19, 2020

Uh oh!

peternewman left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

peternewman commented Mar 19, 2020

Uh oh!

peternewman commented Mar 19, 2020

Uh oh!

kripton commented Mar 20, 2020

Uh oh!

peternewman left a comment

Choose a reason for hiding this comment

Uh oh!

kripton commented Mar 21, 2020

Uh oh!

peternewman commented Mar 21, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

kripton commented Mar 18, 2020 •

edited

Loading

kripton commented Mar 18, 2020 •

edited

Loading

kripton Mar 19, 2020 •

edited

Loading