Bug 259669 - comms/xastir: Update to 2.1.8
Summary: comms/xastir: Update to 2.1.8
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: Fernando Apesteguía
URL: https://github.com/Xastir/Xastir/rele...
Keywords: needs-qa
Depends on:
Blocks:
 
Reported: 2021-11-05 23:41 UTC by russo
Modified: 2021-12-29 17:43 UTC (History)
2 users (show)

See Also:
bugzilla: maintainer-feedback? (carl)


Attachments
Patch to update comms/xastir to version 2.1.8 (1.61 KB, patch)
2021-11-05 23:41 UTC, russo
no flags Details | Diff
Fixed patch that also fixes "gv" error described earlier (1.61 KB, patch)
2021-11-05 23:53 UTC, russo
no flags Details | Diff
Patch to update the port to 2.1.8 and fix an issue in probing for gv (1.67 KB, patch)
2021-11-05 23:55 UTC, russo
no flags Details | Diff
Additional patch to add extract dependencies so that port will build even if autotools not already installed (903 bytes, patch)
2021-11-06 01:08 UTC, russo
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description russo 2021-11-05 23:41:13 UTC
Created attachment 229305 [details]
Patch to update comms/xastir to version 2.1.8

The Xastir group released version 2.1.8 a couple of weeks ago.  It is mostly a maintenance release, and is easy to update.

The main "gotcha" is that the Github releases no longer come "pre-bootstrapped" --- they contain no "configure" script or any other autotools-produced files, nor are some of the other files produced by the "bootstrap.sh" present.

The attached patch should completely prepare the port for the new version.  I have tested it out and it works seamlessly with all the existing patch files as before.

Unfortunately, this is the first time I have ever tried to build the Xastir port in FreeBSD (I am an Xastir developer, and always build it from source myself), and I find that Xastir's probe for "gv" actually tries to *run* gv in order to get its version number, and this does not work when building through the port for reasons I don't understand (it gets a strange error when trying to run "gv --version", and interprets this as gv not being usable, so it skips using it).  But that was already the case before my changes, and means one can't print map screens  --- but apparently one never could with the port-built Xastir.

At any rate, my patch updates the port to 2.1.8 and the build is just as functional as it was with 2.1.6.

The patch was created from a private git branch using "git format-patch".
Comment 1 russo 2021-11-05 23:53:09 UTC
Created attachment 229306 [details]
Fixed patch that also fixes "gv" error described earlier

This replacement patch not only updates to version 2.1.8, but also fixes the boneheaded error in acinclude.m4 that tries to redirect standard error using the Bourne-shell syntax "2>&1" that is not appropriate for the cshell.  This new patch patches acinclude.m4 to remove that junk before "bootstrap.sh" is run.

With this change, port-built Xastir should now use "gv" properly in order to print maps, if gv happens to be installed.

I did not make "gv" a dependency, because it is not required, and is merely used if configure is able to detect it at build time.  One might consider making it a build dependency, but clearly nobody was using it so far since it wasn't working.

I'll check with the Xastir team to see if anyone objects to my removing that "2>&1" from the gv check in the upstream repo --- no other probe in all of Xastir's configure tries to do that.
Comment 2 russo 2021-11-05 23:55:00 UTC
Created attachment 229307 [details]
Patch to update the port to 2.1.8 and fix an issue in probing for gv

Fixed patch that also fixes "gv" error described earlier.

I had not committed my fixed fixes to my local git branch, and the obsolete diff was wrong.
Comment 3 russo 2021-11-06 00:53:04 UTC
Alas, while my patch does work on my system, that's because I have autoconf and automake installed, both of which are needed by "bootstrap.sh".  Unfortunately, as I have done it that introduces a problem.

Bootstrap.sh must be run *after* extraction and *before* patching, because two of the patches are to files that automake creates (Makefile.in and src/Makefile.in) and which do not exist in the tarball anymore.  That's why I put the replace_cmd and bootstrap.sh in post-extract.

But because bootstrap.sh runs them, autoconf and autoconf are build dependencies.  I have tried to add them to BUILD_DEPENDS as:

BUILD_DEPENDS= xfontsel:x11-fonts/xfontsel \
               autoconf>=2.60:devel/autoconf \
               automake>=0:devel/automake

But apparently build dependencies don't even get checked until after patching. Thus, a user trying to build the package without having autoconf and automake installed already would have the port fail during post-extraction, and not cleanly.

I am not sure the best way to proceed, because this package must have bootstrap.sh run before the patches can work, and bootstrap.sh requires autoconf and automake.

It is possible that the patches to Makefile.in and src/Makefile.in could be better handled by patching Makefile.am and src/Makefile.am, the inputs to automake, and if so "bootstrap.sh" could be run as a pre-configure step --- that would solve the dependency problem, because build dependencies are checked before pre-configure.

Unfortunately, I don't immediately see how to make both of those changes work in the pre-autotools files.

The Makefile.in patch is easy and could just be done earlier by removing one line of Makefile.am (the line that unnecessarily sets mandir=${prefix}/share/man).  I don't even know why that line is there.

But I don't understand the sparc64-specific src/Makefile.in patch or how to place it correctly in Makefile.am so it is inserted properly when automake generates Makefile.am.  From what I can tell, it's trying to change optimization options on the compile line for exactly one source file, but only on sparc64.

If the two Makefile.in patches could be recrafted so that they could apply to Makefile.am instead, then running bootstrap.sh could be deferred until pre-configure.

Note that simply doing "USES = autoreconf" is not good enough, because Xastir's bootstrap.sh does more than just run all the autotools, it also generates a few other files outside of the autoconf system (mostly goofy language files that some developer thought were hilarious for some reason).
Comment 4 russo 2021-11-06 01:08:54 UTC
Created attachment 229308 [details]
Additional patch to add extract dependencies so that port will build even if autotools not already installed

Sorry for all the noise, I am not a port maintainer (and don't want to be) and am not familiar with all the nuances of creating and updating ports.  But I have just read through the porter's handbook and now understand that in addition to BUILD_DEPENDS, there's also an EXTRACT_DEPENDS.

This second patch, to be applied after the first one, will correctly set extract dependencies so that autoconf and automake are installed before trying to run bootstrap.sh.

The two patches here should now completely resolve the update from 2.1.6 to 2.1.8.
Comment 5 russo 2021-11-06 01:25:26 UTC
While I've babbled about how to make this update work, I haven't justified why it should be applied.

The release notes for the 2.1.8 version are here:

https://github.com/Xastir/Xastir/releases/tag/Release-2.1.8

Fixes here will prevent breakage with upcoming releases of the "proj" library and very new versions of automake (neither of which are in ports yet, but will be eventually), among other things.
Comment 6 Fernando Apesteguía freebsd_committer freebsd_triage 2021-11-19 08:52:39 UTC
^Triage: Simplifying title

^Triage: If there is a changelog or release notes URL available for this version, please add it to the URL field.

Q/A:  Makefile: [14]: use a tab (not space) after a variable name
 Makefile: Consider adding support for a NLS knob to conditionally disable gettext support.
 
^Triage: Please confirm this change passes QA (portlint, poudriere at least).
--
https://www.freebsd.org/doc/en/books/porters-handbook/testing.html


Thanks!
Comment 7 russo 2021-11-19 14:34:26 UTC
Comment on attachment 229307 [details]
Patch to update the port to 2.1.8 and fix an issue in probing for gv

It was not correct to mark attachment 229307 [details] as obsolete.

My fix involves two patches, attachment 229307 [details] and 229308, both of which must be applied.

Attachment 229307 [details] updates the port to 2.1.8.

Attachment 229308 [details] fixes the dependencies so the port will build even if autotools are not yet installed.
Comment 8 russo 2021-11-19 14:44:30 UTC
(In reply to Fernando Apesteguía from comment #6)
I have unmarked attachment 229307 [details] as obsolete, because it isn't.  Both patches must be applied to update the port to 2.1.8.  It was incorrect for you to have marked it as obsolete.

As for your other comments:
 - The tabs in the makefile were there prior to my updates, I simply copied the pattern.
 - The current version of Xastir probes for gettext and expects it to be present, even though it shouldn't (it doesn't even USE gettext, and this is a vestige in its configure.ac that will be removed in the next version).  So it would be inappropriate at this time to change the port to make gettext optional.  However, in the *next* version it will be OK to remove gettext altogether.  Thank you for pointing out that this port still expects gettext, because we thought we had removed that requirement from Xastir itself already.

I am an Xastir developer, not the ports maintainer.  My bug report is intended to provide the ports maintainer with a little help getting the port updated.
Comment 9 russo 2021-11-19 14:53:19 UTC
I was wrong about Xastir's configure expecting gettext.  It does probe for it unnecessarily, but will build fine if it is not present (gettext is not used at all even if it's found).

So gettext can simply be removed from the USES line.  All traces of gettext probing will be removed from Xastir's configure in the next release.

Xastir also used to contain a single python script, but that script was removed a couple of releases ago.  "python:run" can also be removed from the USES line.

My patches do neither of these things.
Comment 10 Fernando Apesteguía freebsd_committer freebsd_triage 2021-11-19 15:10:22 UTC
Thanks for your comments.

I think it is easier if all the changes come from a single patch. It helps with PR management and reduces the possibility of error.

But this will do too.

Cheers!
Comment 11 commit-hook freebsd_committer freebsd_triage 2021-12-29 13:43:06 UTC
A commit in branch main references this bug:

URL: https://cgit.FreeBSD.org/ports/commit/?id=27132d3fdab4de9ef9f976a24f14200e567028ef

commit 27132d3fdab4de9ef9f976a24f14200e567028ef
Author:     Fernando Apesteguía <fernape@FreeBSD.org>
AuthorDate: 2021-12-29 11:41:25 +0000
Commit:     Fernando Apesteguía <fernape@FreeBSD.org>
CommitDate: 2021-12-29 13:38:16 +0000

    comms/xastir: Update to 2.1.8

    ChangeLog: https://github.com/Xastir/Xastir/releases/tag/Release-2.1.8

    While here:

    * Remove gettext and python from USES. The port doesn't used them anymore.
    * Remove comment
    * Move patch to .am file instead of generated .in file
    * Remove sparc64 patch (architecture is UNSUPPORTED in 13 and above)
    * Add USES=autoreconf

    This last item allows us to bypass bootstrap.sh. The language preprocessing is
    already done in the config directory so there is no need to do it as a bootstrap
    step.

    PR:     259669
    Reported by:    russo@bogodyn.org (xastir developer)
    Approved by:    carl@stagecraft.cx (maintainer, timeout > 1 month)

 comms/xastir/Makefile                           |  9 ++++++---
 comms/xastir/distinfo                           |  6 +++---
 comms/xastir/files/patch-Makefile.am (new)      | 11 +++++++++++
 comms/xastir/files/patch-Makefile.in (gone)     | 11 -----------
 comms/xastir/files/patch-src_Makefile.in (gone) | 15 ---------------
 5 files changed, 20 insertions(+), 32 deletions(-)
Comment 12 Fernando Apesteguía freebsd_committer freebsd_triage 2021-12-29 13:43:43 UTC
Committed a new patch including most of your changes.

I used autoreconf instead of executing bootstrap.sh manually (which led to the autoconf/automake dependency problem).

Thanks!
Comment 13 russo 2021-12-29 17:43:58 UTC
Thank you for pointing out that the derived language files are created by the makefile in the config directory.

A developer added that in 2010 and didn't make a big deal about it, but then never bothered to remove the duplicated generation from bootstrap.sh that a *different* developer had added two years earlier.

I'm going to correct that oversight upstream now.