Bug 282880

Summary: textproc/amberfish: update to 1.7.1, take maintainership
Product: Ports & Packages Reporter: nrn
Component: Individual Port(s)Assignee: Robert Clausecker <fuz>
Status: Closed FIXED    
Severity: Affects Only Me CC: fuz
Priority: ---    
Version: Latest   
Hardware: Any   
OS: Any   
Attachments:
Description Flags
textproc/amberfish: update to 1.7.1
none
textproc/amberfish: update to 1.7.1 [resubmitted]
none
textproc/amberfish: update to 1.7.1 [resubmitted(2)] none

Description nrn 2024-11-21 04:09:36 UTC
Created attachment 255339 [details]
textproc/amberfish: update to 1.7.1

This updates textproc/amberfish from 1.6.4 to 1.7.1.

Apologies for any errors.  This is my first attempt to contribute.

Also I am unsure how to upload the .tar.gz file.

Many thanks.
Comment 1 Robert Clausecker freebsd_committer freebsd_triage 2024-11-22 13:03:41 UTC
Thank you for your submission.

Here are some things for you to check:

 - please use DISTVERSION over PORTVERSION if possible
 - you removed MASTER_SITES but didn't put any replacement in.  From where are
   the distfiles thus fetched?  Maybe you need USE_GITLAB=yes?
 - REINPLACE_CMD is only for dynamic patching.  For static patches, write patch
   files.  Please replace all uses of static patching that currently use
   REINPLACE_CMD with patch files.

Please check these items and resubmit.

I have not conducted a build test and can't say if the port builds with your update.  Make sure to run all these checkers and fix any errors they report:

 - make check-plist
 - make stage-qa
 - portlint

I also strongly recommend that you test the port with Poudriere, ideally on as many architectures as you can and on both the currently oldest FreeBSD 13 and FreeBSD 14 releases.  That is not required, but it'll make it easier for you to find any issues.
Comment 2 nrn 2024-11-24 00:52:21 UTC
Created attachment 255419 [details]
textproc/amberfish: update to 1.7.1 [resubmitted]

Resubmitted patch based on comments from Robert Clausecker.
Comment 3 nrn 2024-11-24 01:08:49 UTC
Thank you for your thorough comments.  I have resubmitted after making these changes:

- used DISTVERSION over PORTVERSION
- added USE_GITLAB=yes
- replaced REINPLACE_CMD with patch files
- ran: make check-plist, make stage-qa, and portlint
- tested the port (without Poudriere) in FreeBSD 13.3, 14.0, and 14.1 running in bhyve

I am still trying to understand Poudriere.  It seems not to fetch the distribution file, and if I place the file in /usr/local/poudriere/ports/default/distfiles/, it expects a different directory name than what it finds when it expands the file.

Thanks again.
Comment 4 Robert Clausecker freebsd_committer freebsd_triage 2024-11-24 07:43:30 UTC
(In reply to nrn from comment #3)

Thank you for improving your patch.

If I run portlint on the patch you submitted, I get a whole lot of errors and warnings:

WARN: Makefile: [5]: USE_* seen before USES.  According to the porters-handbook, USES must appear first.
WARN: Makefile: [33]: possible direct use of command "echo" found. use ${ECHO_CMD} or ${ECHO_MSG} instead.
FATAL: Makefile: extra item "USE_GITLAB" placed in the MAINTAINER section.
FATAL: Makefile: extra item "GL_COMMIT" placed in the MAINTAINER section.
WARN: Makefile: COMMENT is set externally to this port's Makefile, but this port is not configured as a slave port.
FATAL: Makefile: extra item "MAINTAINER" placed in the LICENSE section.
FATAL: Makefile: extra item "COMMENT" placed in the LICENSE section.
FATAL: Makefile: extra item "WWW" placed in the LICENSE section.
WARN: Makefile: "LICENSE_FILE" has to appear earlier.
WARN: Makefile: "LICENSE_FILE" has to appear earlier.
WARN: Makefile: "LICENSE" has to appear earlier.
WARN: Makefile: "LICENSE_FILE" has to appear earlier.
WARN: Makefile: "LIB_DEPENDS" has to appear earlier.
WARN: Makefile: "USES" has to appear earlier.
5 fatal errors and 9 warnings found.

The error is that USE_GITLAB=yes and GL_COMMIT go a bit further below in the Makefile, right after USES=...  Also, GL_TAGNAME should be used instead of GL_COMMIT.

It also warns you to use ${ECHO_CMD} instead of echo, which is something that also needs to be fixed.

Are you sure you ran the tool correctly?  It has to be run in the directory of the port with no arguments.  Please make sure you fix the errors and possibly warnings raised by portlint.

A build test passed, but there are stage-qa warnings you did not address:

====> Running Q/A tests (stage-qa)
Warning: 'bin/af' is not stripped consider trying INSTALL_TARGET=install-strip or using ${STRIP_CMD}
Warning: you might not need LIB_DEPENDS on libxerces-c.so

You can use ${STRIP_CMD} in post-install to fix that, or remove the patch that stops the program from being stripped.

And the plist is incorrect, as reported by "make check-plist":

====> Checking for pkg-plist issues (check-plist)
===> Parsing plist
===> Checking for items in STAGEDIR missing from pkg-plist
Error: Orphaned: share/man/man3/afclose.3.gz
Error: Orphaned: share/man/man3/afgetresultmd.3.gz
Error: Orphaned: share/man/man3/afopen.3.gz
Error: Orphaned: share/man/man3/afsearch.3.gz
Error: Orphaned: share/man/man3/afsortdocid.3.gz
Error: Orphaned: share/man/man3/afsortscore.3.gz

Are you sure you actually ran "make stage-qa" and "make check-plist"?

Poudriere should be able to fetch the distfile if the upstream site is configured correctly.  For your patch, it looks like it is; I was able to build the port just fine using Poudriere.  Maybe DISTFILES_CACHE is configured to something unexpected in /usr/local/etc/poudriere.conf?  That's where poudriere places the distfiles, not in the jails.
Comment 5 nrn 2024-11-24 16:02:23 UTC
I think these are now fixed, except for:

Warning: you might not need LIB_DEPENDS on libxerces-c.so

This warning may be because ./configure will allow building without xerces-c3 but with very reduced features.  Since xerces-c3 is available in ports, it is a good idea to keep the dependency.  However, I don't know if there might be a better way to express the dependency.  I will wait for comments before resubmitting the patch.

On portlint, I must have missed it somehow.  My apologies.

On GL_TAGNAME, note that the documentation at https://docs.freebsd.org/en/books/porters-handbook/makefiles/ does not mention GL_TAGNAME as an option, and I was not aware of it.

On stripping the program, I assume the previous maintainer probably had a good reason for patching the Makefile to remove strip and also not adding it to the port Makefile.  Also, when I tried it without that patch, the port would not build.  Maybe it is simply not in the $PATH.  (I will look into why the original strip does not work, but for now I have added ${STRIP_CMD}.)

On the plist being incorrect, since those man3 pages are only for developers, I wrongly thought the check-plist errors meant that the files were left out of the installation, and that the previous maintainer had intentionally left them out.  I see now that it means the files were installed but not listed in DOCS_PLIST_FILES.

I have made some progress with Poudriere, but it seems to want GL_COMMIT, among other things; so I will install a more recent version from source.

Thanks again.
Comment 6 Robert Clausecker freebsd_committer freebsd_triage 2024-11-25 00:16:46 UTC
(In reply to nrn from comment #5)

Thank you for the update.

There was a recent change in the ports tree, renaming GL_COMMIT to GL_TAGNAME.  Are you by chance trying to apply the patch to a very old copy of the ports tree?  Make sure you are always working on a current ports tree, even with Poudriere.  You can import the ports tree you develop your patches on into Poudriere using the -m null option to poudriere ports -c.  Try that!

I'll have a look at the update in detail tomorrow.
Comment 7 Robert Clausecker freebsd_committer freebsd_triage 2024-11-25 10:09:41 UTC
(In reply to nrn from comment #5)

It seems that you forgot to attach your updated patch.
Comment 8 nrn 2024-11-25 15:34:31 UTC
Created attachment 255445 [details]
textproc/amberfish: update to 1.7.1 [resubmitted(2)]

Thanks again.  I mentioned in the message above that I would wait for comments on LIB_DEPENDS before resubmitting the patch, because it seemed that all of the linter/checker errors needed to be addressed first.  But here is an updated patch, if it helps!
Comment 9 Robert Clausecker freebsd_committer freebsd_triage 2024-11-25 15:59:48 UTC
(In reply to nrn from comment #8)

Thank you for the patch, I'll go ahead and test it.

Warnings are not errors and don't necessarily indicate a problem.  The LIB_DEPENDS check is sometimes not able to find the library dependency despite you actually using it.  If you know that the library is actually used, ignore the warning.  When in doubt, double check that it is, e.g. using the ldd utility.

But yeah, all errors should definitely be fixed and warnings if they indicate a problem.
Comment 10 Robert Clausecker freebsd_committer freebsd_triage 2024-11-26 10:32:38 UTC
Changes I'll do on commit:

 - remove GL_TAGNAME and instead add DISTVERSIONPREFIX=v so
   ${DISTVERSIONPREFIX}${DISTVERSION} matches upstream's tag.  Then it fetches
   just fine without needing to give an explicit commit.
 - remove GNU_CONFIGURE_MANPREFIX, the value you set it to is the default
 - set PORTDOCS to amberfish.html as that is the only documentation file
   to be installed
 - man pages are changed to always be installed as per policy
   (see ยง 5.17.4 Porter's Handbook)

The port does not currently build when option DOCS is disabled as the "html"
target required asciidoctor, but that is only guaranteed to be present when DOCS
is enabled.  I'll change ALL_TARGET to "all" and add DOCS_ALL_TARGET=html to only
build documentation when requested.
Comment 11 commit-hook freebsd_committer freebsd_triage 2024-11-27 12:02:42 UTC
A commit in branch main references this bug:

URL: https://cgit.FreeBSD.org/ports/commit/?id=13e3d18eb6e7f2a7771b4facdb72bf388ffd38e5

commit 13e3d18eb6e7f2a7771b4facdb72bf388ffd38e5
Author:     Nassib Nassar <nrn@etymon.com>
AuthorDate: 2024-11-25 15:22:37 +0000
Commit:     Robert Clausecker <fuz@FreeBSD.org>
CommitDate: 2024-11-27 11:54:52 +0000

    textproc/amberfish: update to 1.7.1, take maintainership

     - chase to new upstream
     - always install man pages as per policy
     - license changed to MIT
     - submitter becomes maintainer
     - turn static REINPLACE_CMD use into patches

    Changelog: https://gitlab.com/amberfish/amberfish/-/releases

    PR:             282880

 textproc/amberfish/Makefile                        |  64 ++-
 textproc/amberfish/distinfo                        |   5 +-
 textproc/amberfish/files/patch-Makefile (new)      |  26 ++
 .../files/patch-src_backend_Makefile.in (new)      |  10 +
 textproc/amberfish/files/porter.cc (gone)          | 438 ---------------------
 textproc/amberfish/pkg-descr                       |  25 +-
 6 files changed, 73 insertions(+), 495 deletions(-)
Comment 12 Robert Clausecker freebsd_committer freebsd_triage 2024-11-27 12:04:59 UTC
Thank you for your contribution.