Bug 192714 - [stage] mail/milter-greylist-devel - update to 4.5.11, add staging, take maintainership
Summary: [stage] mail/milter-greylist-devel - update to 4.5.11, add staging, take main...
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: John Marino
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-08-16 20:51 UTC by Daniel Austin
Modified: 2014-08-17 13:08 UTC (History)
1 user (show)

See Also:


Attachments
update to 4.5.11, stage (6.75 KB, patch)
2014-08-16 20:51 UTC, Daniel Austin
no flags Details | Diff
reworked version (8.39 KB, patch)
2014-08-17 10:35 UTC, John Marino
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Daniel Austin 2014-08-16 20:51:38 UTC
Created attachment 145882 [details]
update to 4.5.11, stage

Update mail/milter-greylist-devel to 4.5.11
Take maintainer
Add staging


Deleted files:
 - files/patch-configure


poudriere testport logs at:

http://poudriere.dan.tm/latest-per-pkg/milter-greylist-devel/4.5.11/
Comment 1 John Marino freebsd_committer 2014-08-16 22:33:17 UTC
move tested staging PR to patch-ready status
Comment 2 John Marino freebsd_committer 2014-08-17 09:42:58 UTC
boom!

man pages are *always* installed.
Anything that has conditional installation like we see here (conditional per option MANPAGES) is flat-out wrong.
Comment 3 John Marino freebsd_committer 2014-08-17 09:43:46 UTC
actually it looks like you added that yourself.
Comment 4 Daniel Austin 2014-08-17 09:48:13 UTC
Ah, I added that to keep it in-line with the behaviour of the non-devel version of the port (mail/milter-greylist)
Comment 5 John Marino freebsd_committer 2014-08-17 09:55:24 UTC
okay, that maintainer is known for that.  it's wrong.  It would be helpful to submit a PR on it.

secondly, RUN_DEPENDS+= BUILD_DEPENDS is also wrong.  The handbook specifically says not to do that.  With the options it's really confusion what the run depends is supposed to be because the options are also assigning run depends.

Can you explicitly list what the run depends is supposed to be?  Should I assume it the "no options set" value of build depends?
Comment 6 John Marino freebsd_committer 2014-08-17 09:58:11 UTC
tip: Never have "post-install:" target when "do-install:" target is defined.  The post-install steps are then added to install target.
Comment 7 John Marino freebsd_committer 2014-08-17 10:00:43 UTC
very advanced obscure tip:

".include "${PORTSDIR}/mail/sendmail/..." is wrong

It should be:

".include "${.CURDIR}/../../mail/sendmail" or
".include "${.CURDIR}/../sendmail"

PORTSDIR is absolute path, we want relative paths.
Comment 8 Daniel Austin 2014-08-17 10:04:31 UTC
(In reply to John Marino from comment #5)
> okay, that maintainer is known for that.  it's wrong.  It would be helpful
> to submit a PR on it.
> 
> secondly, RUN_DEPENDS+= BUILD_DEPENDS is also wrong.  The handbook
> specifically says not to do that.  With the options it's really confusion
> what the run depends is supposed to be because the options are also
> assigning run depends.
> 
> Can you explicitly list what the run depends is supposed to be?  Should I
> assume it the "no options set" value of build depends?

That should be a safe assumption - without options set, it only requires sendmail to be installed (base or port) for access to libmilter.so
Comment 9 John Marino freebsd_committer 2014-08-17 10:04:47 UTC
Do we really need an 800-column line in pkg-plist?
1) is the warning still needed?
2) why can't this information be in pkg-message or UPDATING?
Comment 10 Daniel Austin 2014-08-17 10:06:31 UTC
(In reply to John Marino from comment #9)
> Do we really need an 800-column line in pkg-plist?
> 1) is the warning still needed?
> 2) why can't this information be in pkg-message or UPDATING?

I can't see any reason not to put it in pkg-message.
The only difference is that it's currently only shown if the old database location is detected, but i'm sure people are capable of checking for themselves!
Comment 11 Daniel Austin 2014-08-17 10:08:13 UTC
The warning can probably be removed altogether now I guess... I don't imagine many people will still have an old version installed with the old path in use.
Comment 12 John Marino freebsd_committer 2014-08-17 10:08:50 UTC
okay.
Can you tell me why you are saving this port?
I generally despite -devel ports.

Why do we need this -AND- the original?
Is the maintainer blocking the update?
Is this alpha/beta or just newer?
Comment 13 John Marino freebsd_committer 2014-08-17 10:14:17 UTC
oh crap, half my changes are on the original port.  another reason to hate -devel ports, they screw with tab completion.
Comment 14 Daniel Austin 2014-08-17 10:17:52 UTC
(In reply to John Marino from comment #12)
> okay.
> Can you tell me why you are saving this port?
> I generally despite -devel ports.
> 
> Why do we need this -AND- the original?
> Is the maintainer blocking the update?
> Is this alpha/beta or just newer?

4.5.x is listed as "development snapshot - not for production use" vs 4.4.x as "stable" so I guess we should keep them separate.

Whether that's just the author not committing to creating a new stable release i'm not sure.
Comment 15 John Marino freebsd_committer 2014-08-17 10:21:27 UTC
There's no release of 4.5 at all?
Comment 16 John Marino freebsd_committer 2014-08-17 10:21:55 UTC
aside: INSTALL_* commands are masked with "@", that's not allowed.
Comment 17 Daniel Austin 2014-08-17 10:23:17 UTC
(In reply to John Marino from comment #15)
> There's no release of 4.5 at all?

nope, no 4.5.x release :-(

http://hcpnet.free.fr/milter-greylist/#downloads

(4.4.3 latest stable, 4.5.11 latest devel)
Comment 18 John Marino freebsd_committer 2014-08-17 10:24:33 UTC
(In reply to Daniel Austin from comment #11)
> The warning can probably be removed altogether now I guess... I don't
> imagine many people will still have an old version installed with the old
> path in use.

Is 4.4 "the old version" or does this pertain to a much older release?
Comment 19 John Marino freebsd_committer 2014-08-17 10:27:56 UTC
(In reply to Daniel Austin from comment #8)
> (In reply to John Marino from comment #5)
> > Can you explicitly list what the run depends is supposed to be?  Should I
> > assume it the "no options set" value of build depends?
> 
> That should be a safe assumption - without options set, it only requires
> sendmail to be installed (base or port) for access to libmilter.so

Ironically, BUILD_DEPENDS is empty in the base case.  I ran remove the line completely.
Comment 20 Daniel Austin 2014-08-17 10:30:58 UTC
(In reply to John Marino from comment #18)
> (In reply to Daniel Austin from comment #11)
> > The warning can probably be removed altogether now I guess... I don't
> > imagine many people will still have an old version installed with the old
> > path in use.
> 
> Is 4.4 "the old version" or does this pertain to a much older release?

Release date for 4.4.3 is 7th March 2013.
4.5.1 build date is 20th May 2013.

I'm assuming the author doesn't want to commit to pushing 4.5 to stable.
11 builds over just under a year within 4.5.x branch.

But, i'm also weary that milter-greylist is quite a popular installation with FreeBSD users.
Comment 21 John Marino freebsd_committer 2014-08-17 10:32:58 UTC
I'm just trying to decide if that warning is still necessary.  I can't figure that out from what you've told me.

However, I think "pkg-message" is fine instead of trying to detect it.
Comment 22 John Marino freebsd_committer 2014-08-17 10:34:07 UTC
okay, it busted on config.  I'm going to upload the current diff and the partial build log, maybe you can spot the issue.
Comment 23 John Marino freebsd_committer 2014-08-17 10:35:43 UTC
Created attachment 145911 [details]
reworked version

breaks here: 

checking if compiler accepts -Wall... yes
checking if compiler accepts -Werror... no
checking if ld accepts -R... yes
checking if ld accepts --rpath... yes
checking for ldap_search in -lc -lldap_r-2.4 -llber-2.4... no
checking for ldap_search in -lc -lldap_r -llber... no
Error: no OpenLDAP library names located!
===>  Script "configure" failed unexpectedly.
Please report the problem to freebsd-ports@dan.me.uk [maintainer] and attach
the
"/wrkdirs/usr/ports/mail/milter-greylist-devel/work/milter-greylist-4.5.11/conf
including the output of the failure of your make command. Also, it might be
a good idea to provide an overview of all packages installed on your system
(e.g. a /usr/local/sbin/pkg-static info -g -Ea).
*** Error code 1
Comment 24 John Marino freebsd_committer 2014-08-17 10:38:36 UTC
it's explicitly configured without openldap support, so I don't know why it's checking for it.  I'll look at config.log now.
Comment 25 Daniel Austin 2014-08-17 10:44:35 UTC
(In reply to John Marino from comment #24)
> it's explicitly configured without openldap support, so I don't know why
> it's checking for it.  I'll look at config.log now.

interesting... if you don't put "--without-openldap" to configure, it configures fine (and without openldap)
Comment 26 John Marino freebsd_committer 2014-08-17 10:46:25 UTC
i was thinking it was a bug in configure, that confirms it.  I'll put it back to how it was with a comment.
Comment 27 John Marino freebsd_committer 2014-08-17 10:51:59 UTC
okay, it builds and passes poudriere like that.

We have one final thing to decide:

Should we maintain the warning?
If you say yes, I'll add it to pkg-message.in.
If you say no, I'll commit it as it is.
Comment 28 Daniel Austin 2014-08-17 10:55:32 UTC
(In reply to John Marino from comment #27)
> okay, it builds and passes poudriere like that.
> 
> We have one final thing to decide:
> 
> Should we maintain the warning?
> If you say yes, I'll add it to pkg-message.in.
> If you say no, I'll commit it as it is.

Scrolling through commits for Makefile, that warning has been there over 5 years.
Based on that, i'm very happy for it to be removed completely.
It shouldn't be affecting anyone now.
Comment 29 John Marino freebsd_committer 2014-08-17 10:59:13 UTC
(In reply to Daniel Austin from comment #28)
> Scrolling through commits for Makefile, that warning has been there over 5
> years.
> Based on that, i'm very happy for it to be removed completely.
> It shouldn't be affecting anyone now.

Hypothetically, if somebody stayed on milter-greylist (not devel) and moves to devel today, would they need to heed the warning?
Comment 30 Daniel Austin 2014-08-17 11:03:36 UTC
(In reply to John Marino from comment #29)
> (In reply to Daniel Austin from comment #28)
> > Scrolling through commits for Makefile, that warning has been there over 5
> > years.
> > Based on that, i'm very happy for it to be removed completely.
> > It shouldn't be affecting anyone now.
> 
> Hypothetically, if somebody stayed on milter-greylist (not devel) and moves
> to devel today, would they need to heed the warning?

No, both use the same path for the file.
And the warning on non-devel port has been there >5 years also.
Comment 31 commit-hook freebsd_committer 2014-08-17 11:08:40 UTC
A commit references this bug:

Author: marino
Date: Sun Aug 17 11:08:06 UTC 2014
New revision: 365171
URL: http://svnweb.freebsd.org/changeset/ports/365171

Log:
  Stage mail/milter-greylist-devel and upgrade version 4.5.7 => 4.5.11

  Also assign maintainership to submitter

  PR:		192714
  Submitted by:	Daniel Austin
  Add'l work by:	marino

Changes:
  head/mail/milter-greylist-devel/Makefile
  head/mail/milter-greylist-devel/distinfo
  head/mail/milter-greylist-devel/files/patch-configure
  head/mail/milter-greylist-devel/pkg-plist
Comment 32 John Marino freebsd_committer 2014-08-17 11:11:55 UTC
it would be really great if you could make an new PR for milter-greylist that makes all the same changes except don't touch the warnings.

As long as it behaves exactly the same (except for manpages) then I can commit it without getting approval first.
Comment 33 Daniel Austin 2014-08-17 13:08:39 UTC
(In reply to John Marino from comment #32)
> it would be really great if you could make an new PR for milter-greylist
> that makes all the same changes except don't touch the warnings.
> 
> As long as it behaves exactly the same (except for manpages) then I can
> commit it without getting approval first.

Done - #192740