Bug 195426 - mail/pine-pgp-filters Makefile contains incomplete check for ${LOCALBASE}/bin/gpg2
Summary: mail/pine-pgp-filters Makefile contains incomplete check for ${LOCALBASE}/bin...
Status: Closed FIXED
Alias: None
Product: Ports & Packages
Classification: Unclassified
Component: Individual Port(s) (show other bugs)
Version: Latest
Hardware: Any Any
: --- Affects Many People
Assignee: freebsd-ports-bugs (Nobody)
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-11-26 20:38 UTC by Trond Endrestøl
Modified: 2015-02-06 23:06 UTC (History)
1 user (show)

See Also:


Attachments
Small patch to add inclusion of bsd.port.pre.mk just in time to expand ${LOCALBASE} (494 bytes, patch)
2014-11-26 20:38 UTC, Trond Endrestøl
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Trond Endrestøl 2014-11-26 20:38:59 UTC
Created attachment 149918 [details]
Small patch to add inclusion of bsd.port.pre.mk just in time to expand ${LOCALBASE}

${LOCALBASE} isn't defined in time to be evaluated.
Solved by including bsd.port.pre.mk just prior to the .if check.
Comment 1 John Marino freebsd_committer freebsd_triage 2015-02-06 16:13:27 UTC
"${LOCALBASE} isn't defined in time to be evaluated."

so what happens as a result?

You've stated a fact, but I don't know what the impact is.

What's the impact of not implementing this patch?
Comment 2 Trond Endrestøl 2015-02-06 18:25:27 UTC
(In reply to John Marino from comment #1)

If ${LOCALBASE} expands to the empty string, then ${LOCALBASE}/bin/gpg2 expands to /bin/gpg2. I gather this is not intended, nor expected.
Comment 3 John Marino freebsd_committer freebsd_triage 2015-02-06 18:32:56 UTC
I had to go look at the Makefile to understand what you meant (that was after the second explanation).

So this is a huge hack.
This is going to install gnupg1 100% of the time as well on the builders.  The if exists line will never be true (the expansion of LOCALBASE makes no difference, it will always be false).

AFAICT, gnupg and gnupg1 can co-exist, so what's the point of trying to avoid a single dependency anyway?

We should just pick the correct dependency (security/gnupg I presume) and remove this hack.

thoughts?
Comment 4 Trond Endrestøl 2015-02-06 19:44:22 UTC
(In reply to John Marino from comment #3)

Should we add OPTIONS* to the Makefile and let the user specify if mail/pine-pgp-filters should depend on gnupg2, gnupg1, or none of the above, with the latter being the default?

I'm just thinking of the case where a particular user would like to build their own packages, and simultaneously pick the correct dependency to automate the installation of said ports/packages. I.e., if security/gnupg is missing, then pkg would install that one as well while installing mail/pine-pgp-filters.

My initial motivation for the patch back in November, was that it's counterproductive to depend on security/gnupg1, since I'm more into security/gnupg than security/gnupg1. The OPTIONS* approach might be a flexible solution, suitable for everyone.
Comment 5 John Marino freebsd_committer freebsd_triage 2015-02-06 19:57:06 UTC
I don't understand what problem needs to be solved.

I am not familiar with pine filters.  Why would somebody care what very of gpg it's built against?  If both gpg can co-exist, what's the problem with just picking the best version for pine filters?
Comment 6 John Marino freebsd_committer freebsd_triage 2015-02-06 19:57:25 UTC
s/what very/what version/
Comment 7 Trond Endrestøl 2015-02-06 20:25:07 UTC
(In reply to John Marino from comment #5)

Maybe dougb@ should be consulted on the matter.

Regardless of the fact of whether security/gnupg and security/gnupg1 can coexist, why install security/gnupg1 if security/gnupg is already on the system? The same argument can be made for security/gnupg1 when security/gnupg is missing.

I just wanted to fix what was intended in the Makefile. ${LOCALBASE} would normally expand to /usr/local, thus ${LOCALBASE}/bin/gpg2 would become /usr/local/bin/gpg2. Without my patch from November, ${LOCALBASE} remains empty, and the condition in the Makefile can never be satisfied.
Comment 8 John Marino freebsd_committer freebsd_triage 2015-02-06 20:40:18 UTC
(In reply to Trond.Endrestol from comment #7)

> Maybe dougb@ should be consulted on the matter.

I see no reason to do that.

> Regardless of the fact of whether security/gnupg and security/gnupg1 can
> coexist, why install security/gnupg1 if security/gnupg is already on the
> system? 

You seem not to be aware of how the builders work.  By definition, nothing is installed on the system, ever, before the target port is built.  For binary packages, which are the primary concern these days, this is a case that can never happen unless somehow the dependencies pull in both of them

Secondly, who cares?

We don't want to stick with an inferior version just because it's already installed on the system.  You want the most appropriate version for pine.  They range from 1.0M-1.5M in package size anyway, it a trivial amount of resources to have both.


> The same argument can be made for security/gnupg1 when security/gnupg is
> missing.

The problem is I don't see any argument at all.

> I just wanted to fix what was intended in the Makefile. ${LOCALBASE} would
> normally expand to /usr/local, thus ${LOCALBASE}/bin/gpg2 would become
> /usr/local/bin/gpg2. Without my patch from November, ${LOCALBASE} remains
> empty, and the condition in the Makefile can never be satisfied.

What was intended is dubious at best.
I think it needs to be removed entirely.
As I said, for the package builders it doesn't even work.
It only comes into play for people building on live systems (something I recommend against doing) and even then, the logic becomes: If the inferior version is installed on your system, use that instead of what you should use.  I really don't see how this is a good thing.

The argument seems to be to avoid building or installing a single rather small dependency.  It's not even worth the complexity.

It's good you brought this up so it can be removed IMO.
Comment 9 Trond Endrestøl 2015-02-06 20:43:49 UTC
(In reply to John Marino from comment #8)

Agreed. Axe away.
Comment 10 commit-hook freebsd_committer freebsd_triage 2015-02-06 22:59:34 UTC
A commit references this bug:

Author: marino
Date: Fri Feb  6 22:59:21 UTC 2015
New revision: 378568
URL: https://svnweb.freebsd.org/changeset/ports/378568

Log:
  mail/pine-pgp-filters: use gnupg only, fix MASTER_SITES

  When dougb released the port, he also reset the MASTER_SITES.  It has
  been pulling off the cache ever since.  If the cache gets cleared, this
  port would break.  I've moved the distfiles to my LOCAL site so this
  never happens.

  The other big change is to remove this hacky dependency logic.  Someone
  tried to select between gnupg and gnupg1 depending on what was already
  installed.  For the standard packages builders, this meant the port always
  used the old gnupg1 because gnupg was never installed in the clean jail.

  As both gnupg packages are range between 1.0M and 1.5M in size and can
  coexist, the savings of resources in miniscule assuming they weren't both
  installed anyway.  Let's just pick the new gnupg as the unconditional
  dependency and remove the hack.  Inspired by discussion on PR which
  was trying to fix a bug in the hack.

  PR:		195426
  Submitted by:	Trond Endrestol

Changes:
  head/mail/pine-pgp-filters/Makefile
Comment 11 John Marino freebsd_committer freebsd_triage 2015-02-06 23:06:34 UTC
Maybe it wasn't fixed the way you wanted at first, but it is fixed. :)