Bug 236189 - mail/mutt: simplify - remove LDFLAGS, etc.
Summary: mail/mutt: simplify - remove LDFLAGS, etc.
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: Alex Kozlov
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2019-03-03 23:39 UTC by John Hein
Modified: 2019-04-04 04:09 UTC (History)
2 users (show)

See Also:
dereks: maintainer-feedback+


Attachments
[patch] remove unnecessary localbase flags (1.59 KB, patch)
2019-03-03 23:39 UTC, John Hein
no flags Details | Diff
[patch] fix non-default LOCALBASE for aspell (514 bytes, patch)
2019-03-03 23:59 UTC, John Hein
no flags Details | Diff
Updated patch (8.52 KB, patch)
2019-04-03 13:22 UTC, Alex Kozlov
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description John Hein 2019-03-03 23:39:28 UTC
Created attachment 202530 [details]
[patch] remove unnecessary localbase flags

This is a low priority change, but I removed the global LDFLAGS and -I${LOCALBASE}/include from mutt/Makefile and built various combinations of options in poudriere.

Attached is a patch to remove the global localbase flag settings.
Comment 1 John Hein 2019-03-03 23:46:23 UTC
(In reply to John Hein from comment #0)
QA:
 poudriere (ok with many option combinations)
 portlint (ok - warning about unnecessary PORTREVISION unrelated to this PR, but could be removed)
Comment 2 John Hein 2019-03-03 23:59:52 UTC
Created attachment 202531 [details]
[patch] fix non-default LOCALBASE for aspell

Also attached is a patch to fix the aspell invocation if LOCALBASE != /usr/local.  it just relies on PATH to find aspell rather than hard-coding the absolute path.
Comment 3 Derek Schrock 2019-03-09 02:01:03 UTC
This seems alright however I think aspell should be using ${LOCALBASE}.

Removing extra-patch-aspell and adding to the Make file

...
pre-configure-ASPELL-on:
	${ECHO_CMD} 'set ispell="${LOCALBASE}/bin/aspell --mode=email check"' >> \
	    ${WRKSRC}/doc/Muttrc.head
...


If you think that's alright do you want to combine the patches and bump the PORTREVISION?
Comment 4 John Hein 2019-03-13 18:32:51 UTC
(In reply to Derek Schrock from comment #3)
I generally prefer to not hard-code the path when possible.  This let's the user have more control.  It works fine for the default user since /usr/local/bin is in the PATH by default (and there's no ambiguity with a /usr/bin/aspell).  And it allows someone who might want to use a different aspell the flexibility to do so (e.g., env PATH=/path/to/alt-aspell:$PATH mutt).

I can definitely combine and bump.  Give my feedback a think and let me know - my preference would be no hard-coded path, but it's your call as maintainer.
Comment 5 Derek Schrock 2019-03-21 23:29:44 UTC
Ok.  Leave the full path off.
Comment 6 Alex Kozlov freebsd_committer freebsd_triage 2019-04-03 13:22:52 UTC
Created attachment 203346 [details]
Updated patch

It seems that original poster is a little busy with RL or there is some sort of miscommunication, so I've updated the patch as per latest suggestions. Maintainer, can you please review it and approve if it looks ok:

- Cleanup LDFLAGS, CONFIGURE_ENV [1]
- Fix non-default LOCALBASE for aspell [1] [2]
- Remove unused extra-patch-parent-child-match

PR: 236189

Submitted by: John Hein <jcfyecrayz@liamekaens.com> [1]
Approved by: Derek Schrock <dereks@lifeofadishwasher.com> (maintainer) [2]
Comment 7 Derek Schrock 2019-04-03 19:45:16 UTC
Looks good. Thanks.
Comment 8 commit-hook freebsd_committer freebsd_triage 2019-04-04 04:08:20 UTC
A commit references this bug:

Author: ak
Date: Thu Apr  4 04:07:18 UTC 2019
New revision: 497785
URL: https://svnweb.freebsd.org/changeset/ports/497785

Log:
  - Cleanup LDFLAGS, CONFIGURE_ENV [1]
  - Fix non-default LOCALBASE for aspell [1]
  - Remove unused extra-patch-parent-child-match

  PR:	236189

  Submitted by:	John Hein <jcfyecrayz@liamekaens.com> [1]
  Approved by:	Derek Schrock <dereks@lifeofadishwasher.com> (maintainer)

Changes:
  head/mail/mutt/Makefile
  head/mail/mutt/files/extra-patch-aspell
  head/mail/mutt/files/extra-patch-parent-child-match