Opened 7 years ago

Closed 7 years ago

#55878 closed defect (fixed)

lowdown @0.3.1: Fix mandir; fix universal variant; fix build on Snow Leopard and earlier

Reported by: ryandesign (Ryan Carsten Schmidt) Owned by: janstary (Jan Starý)
Priority: Normal Milestone:
Component: ports Version:
Keywords: haspatch Cc:
Port: lowdown

Description

This patch fixes several issues in the lowdown port.

  • It moves the specification of PREFIX from configure.args to configure.pre_args, to match where MacPorts base's default configure argument for specifying the prefix is.
  • It specifies the MANDIR so that the manpages are installed in the correct directory. Since this changes where files are installed, the revision is increased.
  • It removes --disable-dependency-tracking from the default configure.universal_args because the configure script errors out when that flag is passed.
  • It adds a patch to the configure script so that LDFLAGS from the environment are used. Without this patch, it only uses CC and CFLAGS from the environment.
  • It adds a patch to the Makefile so that LDFLAGS are used when linking lowdown. This, combined with the previous patch, mean that lowdown is now linked for the correct architecture and using all LDFLAGS that MacPorts might specify, including -lsnowleopardfixes when added by the snowleopard_fixes portgroup, which fixes the build on Snow Leopard and earlier.

These two patches should probably be submitted to upstream.

Attachments (4)

Portfile-lowdown.diff (686 bytes) - added by ryandesign (Ryan Carsten Schmidt) 7 years ago.
Makefile.patch (388 bytes) - added by ryandesign (Ryan Carsten Schmidt) 7 years ago.
configure.patch (814 bytes) - added by ryandesign (Ryan Carsten Schmidt) 7 years ago.
lowdown.log (35.5 KB) - added by janstary (Jan Starý) 7 years ago.

Download all attachments as: .zip

Change History (8)

Changed 7 years ago by ryandesign (Ryan Carsten Schmidt)

Attachment: Portfile-lowdown.diff added

Changed 7 years ago by ryandesign (Ryan Carsten Schmidt)

Attachment: Makefile.patch added

Changed 7 years ago by ryandesign (Ryan Carsten Schmidt)

Attachment: configure.patch added

comment:1 Changed 7 years ago by janstary (Jan Starý)

I agree with the PREFIX and MANDIR, sorry for missing them earlier.

I agree that --disable-dependency-tracking should be dropped. Indeed, lowdown's ./configure only accepts certain KEY=VAL as its arguments. So I think configure.universal_args should be cleared completely in fact.

I attach a log of port -vsd install lowdown on my system (10.13): I don't see the --disable-dependency-tracking added, perhaps that's why it didn't fail for me. Why would it not added on my system?

Why does the LDFLAGS need to be recognized in ./configure? Is there something we need to pass here to the lowdown build? Without this patch, it gets compiled with

LDFLAGS='-L/opt/local/lib -Wl,-headerpad_max_install_names -arch x86_64'

Is there something else we need to pass? As of now, it seems the only application is to have -lsnowleopardfixes added to the build. I would like to get rid of that completely: lowdown goes out of its way to be very portable and provides compatibility implementations for missing functions in compats.c -- it's just that strndup(3) and strnlen(3) isn't there. I talked to upstream: it seems much cleaner to just add these two functions as opposed to tweaking the build and link to something else; they will add it soon. We might either add them to compats.c for no until upstream does, or just wait a bit for the next release which will have them (upstream is quite responsive).

At any rate, thank you very much for looking into this.

Changed 7 years ago by janstary (Jan Starý)

Attachment: lowdown.log added

comment:2 in reply to:  1 Changed 7 years ago by ryandesign (Ryan Carsten Schmidt)

Replying to janstary:

I agree that --disable-dependency-tracking should be dropped. Indeed, lowdown's ./configure only accepts certain KEY=VAL as its arguments. So I think configure.universal_args should be cleared completely in fact.

As you wish. --disable-dependency-tracking is the only thing in configure.universal_args at this time, so it makes no difference.

I attach a log of port -vsd install lowdown on my system (10.13): I don't see the --disable-dependency-tracking added, perhaps that's why it didn't fail for me. Why would it not added on my system?

It is only added if you use the universal variant.

Why does the LDFLAGS need to be recognized in ./configure? Is there something we need to pass here to the lowdown build? Without this patch, it gets compiled with

LDFLAGS='-L/opt/local/lib -Wl,-headerpad_max_install_names -arch x86_64'

No, it does not use those flags. MacPorts tells the build system to use those flags, and the build system ignores that request. My patches fix the build system so that it respects the flags MacPorts tells it to use. This allows the universal variant to work correctly, for example, since the LDFLAGS are one of the places where MacPorts tells the build system what architectures to use.

Is there something else we need to pass? As of now, it seems the only application is to have -lsnowleopardfixes added to the build. I would like to get rid of that completely: lowdown goes out of its way to be very portable and provides compatibility implementations for missing functions in compats.c -- it's just that strndup(3) and strnlen(3) isn't there. I talked to upstream: it seems much cleaner to just add these two functions as opposed to tweaking the build and link to something else; they will add it soon. We might either add them to compats.c for no until upstream does, or just wait a bit for the next release which will have them (upstream is quite responsive).

Absolutely, it is better for upstream to provide the compatibility implementations. Until they do, including the snowleopard_fixes portgroup allows the port to build on Snow Leopard and earlier.

comment:3 Changed 7 years ago by janstary (Jan Starý)

This works for me. I have prepared a PR of this at https://github.com/macports/macports-ports/pull/1381 , slightly tweeked as described above. Would you please look at it?

comment:4 Changed 7 years ago by janstary (Jan Starý)

Resolution: fixed
Status: newclosed

In a01d230728b460ebf8bc76819629f153c6981c73/macports-ports:

lowdown: LDFLAGS for universal and snowleopard_fixes

Closes #55878

Note: See TracTickets for help on using tickets.