Bug 248631 - math/units: Does not find or link against ports libreadline
Summary: math/units: Does not find or link against ports libreadline
Status: Closed FIXED
Alias: None
Product: Ports & Packages
Classification: Unclassified
Component: Individual Port(s) (show other bugs)
Version: Latest
Hardware: Any Any
: --- Affects Some People
Assignee: Li-Wen Hsu
URL:
Keywords: needs-qa
Depends on:
Blocks:
 
Reported: 2020-08-12 17:03 UTC by ports-units
Modified: 2020-08-19 11:21 UTC (History)
2 users (show)

See Also:
jharris: maintainer-feedback+
jharris: maintainer-feedback+
koobs: merge-quarterly?


Attachments
math/units Makefile patch (310 bytes, patch)
2020-08-12 17:03 UTC, ports-units
no flags Details | Diff
math/units Makefile patch (332 bytes, patch)
2020-08-13 07:47 UTC, ports-units
koobs: maintainer-approval+
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description ports-units 2020-08-12 17:03:57 UTC
Created attachment 217172 [details]
math/units Makefile patch

math/units port currently builds successfully, but does not pick up libreadline during configure, so the resulting port has no readline support.

Add correct configure options to fix this.
Comment 1 Kubilay Kocak freebsd_committer freebsd_triage 2020-08-13 04:35:29 UTC
@Reporter Could you:

 - test if USES+localbase is sufficient to make it work, as this construct is preferred over individual *FLAGS/LIBS
 - Since readline support doesn't appear compulsory, add a READLINE option (enabled by default) and scope USES=localbase to that OPTIONS with: READLINE_USES=localbase (and test that this works in both enabled and disabled cases)

Note: If the software supports an *explicit* readline configure option or flag, we should use that instead of just providing *FLAGS/LIBS overrides

^Triage: [tags] in issue Titles are deprecated
Comment 2 ports-units 2020-08-13 07:47:38 UTC
Created attachment 217184 [details]
math/units Makefile patch
Comment 3 ports-units 2020-08-13 07:49:50 UTC
New patch attached.

It now correctly adds the dependency on devel/readline, and appears to work correctly in both the enabled and disabled configuration.

Thanks.
Comment 4 ports-units 2020-08-13 07:59:47 UTC
> If the software supports an *explicit* readline configure option or flag, we should use that

This package actually doesn't accept a configure-time argument to enable/disable readline; it unconditionally adds support if it finds libreadline.  But the USES=readline:port ensures that libreadline is there and causes localbase CFLAGS/LDFLAGS to get set, so it works the way it's supposed to.
Comment 5 Kubilay Kocak freebsd_committer freebsd_triage 2020-08-13 09:52:51 UTC
(In reply to ports-units from comment #4)

Appreciate the update. The actual most common failure case is when some library is installed for an unrelated reason (via a different port), and the OPTION'al dependency is disabled, but the configure/build system finds it nevertheless and links against it.

Can you confirm whether this case is still ok with the latest patch?

Good way to test this is just to add an unconditional LIB_DEPENDS on readline, and disable the READLINE option, then check whether the build
Comment 6 ports-units 2020-08-13 11:14:19 UTC
Confirmed that OPTIONS READLINE still works as expected even when libreadline was already installed unrelatedly.

If the READLINE OPTION is enabled, readline support is included; if READLINE OPTION is disabled, readline support is not included even in the case where libreadline is installed.

Thanks.
Comment 7 Kubilay Kocak freebsd_committer freebsd_triage 2020-08-13 12:27:57 UTC
(In reply to ports-units from comment #6)

Much appreciated
Comment 8 Kubilay Kocak freebsd_committer freebsd_triage 2020-08-13 12:28:45 UTC
Comment on attachment 217184 [details]
math/units Makefile patch

Approved by: portmgr (blanket: build, run, dependency fix)
MFH: 2020Q3 (blanket: build, run, dependency fix)

Pending QA
Comment 9 jharris 2020-08-16 23:57:52 UTC
Comment on attachment 217184 [details]
math/units Makefile patch

Approved, thanks!

Built fine with readline on updated 12-RELEASE.
Comment 10 commit-hook freebsd_committer 2020-08-19 11:14:59 UTC
A commit references this bug:

Author: lwhsu
Date: Wed Aug 19 11:14:30 UTC 2020
New revision: 545313
URL: https://svnweb.freebsd.org/changeset/ports/545313

Log:
  math/units: Add a default option READLINE to tuggle readline support

  This makes sure the readline support is tuggled in the build time and not
  affected by the libreadline installed or not during build/run time.

  PR:		248631
  Submitted by:	ports-units@shalott.net
  Approved by:	jharris@widomaker.com (maintainer)
  MFH:		2020Q3 (blanket: build, run, dependency fix)

Changes:
  head/math/units/Makefile
Comment 11 commit-hook freebsd_committer 2020-08-19 11:21:02 UTC
A commit references this bug:

Author: lwhsu
Date: Wed Aug 19 11:20:03 UTC 2020
New revision: 545316
URL: https://svnweb.freebsd.org/changeset/ports/545316

Log:
  MFH: r545313

  math/units: Add a default option READLINE to tuggle readline support

  This makes sure the readline support is tuggled in the build time and not
  affected by the libreadline installed or not during build/run time.

  PR:		248631
  Submitted by:	ports-units@shalott.net
  Approved by:	jharris@widomaker.com (maintainer)

  Approved by:	portmgr (blanket: build, run, dependency fix)

Changes:
_U  branches/2020Q3/
  branches/2020Q3/math/units/Makefile
Comment 12 Li-Wen Hsu freebsd_committer 2020-08-19 11:21:15 UTC
Committed (with minor style fix), please also consider updating this port to 2.19. Thanks!