Mk/Uses/ncurses.mk: Fix broken ports ncurses support

[bapt]

Summary

Background: DragonFly master has effectively removed ncurses from base
(it's still there, but in a private area dports won't find). As a result,
literally hundreds of ports broken. That's how many are missing
USES=ncurses. That's not a big problem because the infrastructure blanket
will allow me to fix those.

The problem is that ports ncurces handling is broken in two ways:

1. There is no library path (-L) set in LDFLAGS. For compilers that do no automatically search /usr/local/lib, that's broken, and of course if a non-default localbase is used, it's broken

2. The LDFLAGS that are set regarding rpath is wrong. It uses the -Wl, prefix which is for the compiler, not the linker.

The commit fixes both problem by:
A) adding -L${LOCALBASE}/lib to LDFLAGS

2. correcting -Wl,-rpath=${NCURSESPATH} to "-rpath ${NCURSESPATH}"

DragonFly will be extremely helpful to FreeBSD because we have to fix
all these ncurses ports, which will be very nice for FreeBSD if/when
they privatize ncurses library in a similar fashion.

[marino]

hmmm, hold on, it should be
LDFLAGS+= -L${NCURSESRPATH}

Let me update quickly.

[marino]

Update -L flag to use NCURSESPATH

[antoine]

This change breaks some ports (guile, cone, libcdio, probably others)

[marino]

which change?
the -L or the -rpath?

Secondly, put aside the ports for a minute and look at this in a vacuum. Is the statement that ncurses.mk is wrong correct or not?

I don't see how -L could be wrong, so I will assume you are talking about -rpath.

It is a fact that modern linkers do not accept -Wl, arguments. LDFLAGS is for the linker. So if a port is breaking, then it is using LDFLAGS for the compiler, meaning the port is wrong.

If you want, I can show you error messages from LD saying it does not recognize -Wl,

[marino]

to be fair, though, I've seen LDFLAGS passed to a compiler too.

[marino]

if the "-L" change by itself can be approved, that would still be an improvement. the -rpath change will clearly require an exp-run

[antoine]

Yes the -rpath breaks some ports.
And I agree USES=ncurses doesn't really work now.

[marino]

in any case, it should be "-rpath=${NCURSESRPATH}" not "-rpath ${NCURSESRPATH}"

some applications mishandle 2 arguments rather than one.

So far I have not identified any fallout with the "=" version. I'm effectively doing an exp-run right now because we're using this patch

[marino]

antoine, why is rpath even defined on the base ncurses case?

rtld searches there unconditionally.

It seems to me that -rpath should only be defined for the ports ncurses case in any event. Thoughts? Can you think of a base ncurses case that requires -rpath set?

it seems my -rpath change is not correct in any case:
"LDFLAGS: Extra flags to give to compilers when they are supposed to invoke the linker, ‘ld’, such as -L. Libraries (-lfoo) should be added to the LDLIBS variable instead."

However, there are definitely ports that send LDFLAGS to the linker, so those ports are wrong.

the "ports ncurses" probably needs CFLAGS set for -I (to point to ncurses ports headers) though

[antoine]

Ask bapt@ as he wrote the code?

[marino]

This is what I had in mind (will test in next few days)

Okay, based on Antoine's (confirmed) feedback, I'm modifying the
patch, which I will test under a few bulk run in a few days:

1. editorial, fix indents of conditional statements to match previous
2. Add -I to cflags for "ports" version. This will be required should base ncurses headers not be present
3. relocate -rpath switches to "ports" ncurses branch. Unless I am missing something, having -rpath on base ncurses has no impact (unless maybe there's a rtld switch to ignore base libs? but in that case, I would think the switch would override -rpath anyway)

obviously #3 is the biggest change, so maybe bapt can explain why this
was being applied to the base and why it has an effect (I don't think
it does)

[marino]

Fix typo (I transferred it by hand)

[marino]

Tidy up by combining LDFLAGS and tabbing CFLAGS (it is also more logical)

[antoine]

Mk/Uses/ncurses.mk

80

I think this should be CPPFLAGS, not CFLAGS