Bug 233662 - graphics/djvulibre: Level up port compliance, Take MAINTAINER'ship
Summary: graphics/djvulibre: Level up port compliance, Take MAINTAINER'ship
Status: Closed FIXED
Alias: None
Product: Ports & Packages
Classification: Unclassified
Component: Individual Port(s) (show other bugs)
Version: Latest
Hardware: Any Any
: --- Affects Only Me
Assignee: Steve Wills
URL:
Keywords: easy
Depends on:
Blocks:
 
Reported: 2018-11-30 11:58 UTC by Lorenzo Salvadore
Modified: 2019-01-06 02:34 UTC (History)
1 user (show)

See Also:
koobs: merge-quarterly?


Attachments
djvulibre adoption patch (3.94 KB, patch)
2018-11-30 11:58 UTC, Lorenzo Salvadore
salvadore: maintainer-approval+
Details | Diff
djvulibre adoption patch, correction 1 (3.88 KB, patch)
2018-11-30 12:46 UTC, Lorenzo Salvadore
salvadore: maintainer-approval+
Details | Diff
djvulibre adoption patch, without PORTREVISION bump (correction 2) (1.37 KB, patch)
2018-12-01 17:28 UTC, Lorenzo Salvadore
salvadore: maintainer-approval+
Details | Diff
djvulibre adoption patch, with PORTREVISION bump (correction 3) (1.44 KB, patch)
2018-12-03 11:29 UTC, Lorenzo Salvadore
salvadore: maintainer-approval+
Details | Diff
djvulibre, adoption patch (correction 4) (1.71 KB, patch)
2018-12-17 10:22 UTC, Lorenzo Salvadore
salvadore: maintainer-approval+
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Lorenzo Salvadore freebsd_committer freebsd_triage 2018-11-30 11:58:37 UTC
Created attachment 199683 [details]
djvulibre adoption patch

I would like to adopt the djvulibre port. My patch include the following changes:

- Sets new value for MAINTAINER to myself.
- Cleans the header of the Makefile from the deprecated line "Created by".
- Reorders variables consistently with the official order.
- Removes unneeded ${ICONV_LIB} from LD_FLAGS (the variable is empty for all supported FreeBSD versions).
- Changes a CONFIGURE_ARGS= --with... in a CONFIGURE_WITH= ...
- Sorts pkg-plist alphabetically.

Everything has been tested successfully with portlint and poudriere (11.2-RELEASE-p5, i386/amd64).
Comment 1 Tobias Kortkamp freebsd_committer freebsd_triage 2018-11-30 12:17:52 UTC
(In reply to Lorenzo Salvadore from comment #0)
> Cleans the header of the Makefile from the deprecated line "Created
> by".

It is not deprecated, adding new "Created by" lines is only
discouraged.  You cannot just change/remove "Created by" headers
that you yourself did not add without approval of the original
creator, so leave it in or ask mi@ for permission.

> Changes a CONFIGURE_ARGS= --with... in a CONFIGURE_WITH= ...

There is no CONFIGURE_WITH variable (there are opt_CONFIGURE_WITH helpers
for options only), so this is wrong...  How was this tested?  Compare
`make -V CONFIGURE_ARGS` before and after this change.

Using LOCALBASE in this context is wrong too.  LOCALBASE is where
dependencies come from, PREFIX is where the port installs files
into, so it should stay as PREFIX.  But passing --with-pkgconfigdir
to configure is probably no longer needed anyway due to recent
framework changes and fixup-lib-pkgconfig.
Comment 2 Lorenzo Salvadore freebsd_committer freebsd_triage 2018-11-30 12:46:39 UTC
Created attachment 199684 [details]
djvulibre adoption patch, correction 1

About the line I declared deprecated:
https://www.freebsd.org/doc/en/books/porters-handbook/quick-porting.html#porting-makefile
At this page, in the grey box, it is written "This additional information has been declared obsolete, and is being phased out." See also this wiki page where I put together such controversial cases (it is a discussion page, I am willing to modify it of course if I am wrong about anything):
https://wiki.freebsd.org/LorenzoSalvadore/Contradictions
Moreover, I do not think that keeping old ports signatures and discouraging new signatures at the same time makes sense: it is unfair to new users. For example, I tried myself to sign a new port I created and saw the committer removed the signature line: I think he did right for the reasons I wrote in the wiki page, but I also think all signature lines should be allowed or disallowed following a unique rule.
I would like to point out that I have no intention to be rude toward anyone, in particular I do not want to be rude toward the original creator of the port.

I fixed the rest by removing the CONFIGURE_WITH line as you suggestested. I tested with poudriere successfully (11.2-RELEASE-p5, i386/amd64).
Comment 3 Tobias Kortkamp freebsd_committer freebsd_triage 2018-11-30 17:49:39 UTC
(In reply to Lorenzo Salvadore from comment #2)
There is a difference on creating new ports and changing existing
ports.  Why are you insisting so strongly on removing the header?

(In reply to Lorenzo Salvadore from comment #0)
> Sorts pkg-plist alphabetically.

Did not see this before.  You should revert this.  Your list on the
Wiki says that the Porter's Handbook suggests doing this.  It does
not.  It suggests keeping the plist sorted by *filename* (this means
after the %%..%% have been expanded) not by the raw value.  If in
doubt, the order as output by `make makeplist` is the correct order.
Everything else is fighting the available tools, which is no fun
for anybody.
Comment 4 Lorenzo Salvadore freebsd_committer freebsd_triage 2018-12-01 17:28:56 UTC
Created attachment 199715 [details]
djvulibre adoption patch, without PORTREVISION bump (correction 2)

I have put the header back and restored the original pkg-plist.

About the header:
As I said, I had no intention to be rude against anyone.
If I insisted it is only because I sincerly thought that what I wrote in the wiki and in my comment was right, but if the official policy is to leave such lines, it is fine for me.
I will soon update my wiki page with the old ports/new ports distinction and propose a patch to the porter's handbook to have the policy officially documented.

About the plist:
I will fix the wiki about that point too. And I might also propose a patch to the handbook to make clearer the order (I guess others might make my mistake).
Comment 5 Lorenzo Salvadore freebsd_committer freebsd_triage 2018-12-03 11:29:30 UTC
Created attachment 199791 [details]
djvulibre adoption patch, with PORTREVISION bump (correction 3)

- Bumps PORTREVISION, as it is needed for the MAINTAINER change.
Comment 6 Lorenzo Salvadore freebsd_committer freebsd_triage 2018-12-06 11:36:48 UTC
It is unclear if PORTREVISION should be bumped or not.
On one hand, the package changes because of the MAINTAINER modification, but on the other hand the patch does not changes the functionality of the package while the bump enforces a rebuild (and also might enforce many rebuilds on poudriere).

Hence I propose both the patches with and without the PORTREVISION bump. The committer will be able to choose the right one.
Comment 7 Lorenzo Salvadore freebsd_committer freebsd_triage 2018-12-17 10:22:26 UTC
Created attachment 200189 [details]
djvulibre, adoption patch (correction 4)

- Removes CONFLICTS as the conflicting ports do not exist anymore since 2012.

- Adds USES=gnome when option DESKTOPFILES is on as it is requested by poudriere.

Now tested successfully with poudriere on 12.0-RELEASE i386/amd64 too.

(This version of the patch has the PORTREVISION bump. If it is not needed a committer will easily remove it: I think it is better to avoid having double versions of attachements each time a modification is necessary.)
Comment 8 commit-hook freebsd_committer freebsd_triage 2019-01-06 01:21:43 UTC
A commit references this bug:

Author: swills
Date: Sun Jan  6 01:20:50 UTC 2019
New revision: 489428
URL: https://svnweb.freebsd.org/changeset/ports/489428

Log:
  graphics/djvulibre: cleanup, assign maintainer

  PR:		233662
  Submitted by:	Lorenzo Salvadore <phascolarctos@protonmail.ch>

Changes:
  head/graphics/djvulibre/Makefile
Comment 9 Steve Wills freebsd_committer freebsd_triage 2019-01-06 01:22:48 UTC
Committed, thanks!