snac @2.18: use the right compiler and flags

[ryandesign (Ryan Carsten Schmidt)]

snac isn't UsingTheRightCompiler nor flags:

​https://build.macports.org/builders/ports-10.9_x86_64-builder/builds/217714/steps/install-port/logs/stdio

cc -g -Wall -D st_mtim=st_mtimespec  -I/opt/local/include -c main.c
cc -g -Wall -D st_mtim=st_mtimespec  -I/opt/local/include -c data.c
cc -g -Wall -D st_mtim=st_mtimespec  -I/opt/local/include -c http.c
cc -g -Wall -D st_mtim=st_mtimespec  -I/opt/local/include -c httpd.c
cc -g -Wall -D st_mtim=st_mtimespec  -I/opt/local/include -c webfinger.c
cc -g -Wall -D st_mtim=st_mtimespec  -I/opt/local/include -c activitypub.c
cc -g -Wall -D st_mtim=st_mtimespec  -I/opt/local/include -c html.c

The port is manually setting use_configure no and not doing any of the manual work needed to support this properly.

The makefile portgroup could help.

[artkiver (グレェ)]

Ah, thanks. I will look into this further. I figured there was probably a better way to do this earlier, but I admit I am not super up to speed on portgroups. I will do additional research and experimentation and hopefully get this smoothed out a bit!

[artkiver (グレェ)]

So, snac 2.19 was released, which reminded me to look into this in more detail.

​https://guide.macports.org/chunked/reference.portgroup.html is a bit sparse as far as information on the makefile PortGroup.

​https://github.com/macports/macports-ports/blob/master/_resources/port1.0/group/makefile-1.0.tcl

Is I guess a useful reference insomuch as it is the TCL being invoked, though I am not sure how well I am parsing it as a human.

Regardless, doing some minor edits to the Portfile

e.g. PortGroup makefile 1.0

Still seems to install OK after I do a port -v clean.

In reading, I thought about adding:

build.args-append   CFLAGS=-g -Wall -D st_mtim=st_mtimespec

And it doesn't seem to cause harm?

I even explored removing the

patchfiles          Makefile.patch
post-patch          {reinplace "s|/usr/local|${prefix}|g" ${worksrcpath}/Makefile}

As well as attempting to add some other things to build.args-append

Such as:

PREFIX=/opt/local
and PREFIX_MAN=/opt/local/share/man

But I may have been a bit too ambitious with some of that and seemed to get some errors back when running lint --nitpick for example.

So, at the moment, I have a rather more conservative Portfile with some changes, which seems to function OK; but I am not sure if it addresses all the issues raised here.

I'm attaching it for reference though. Doubtlessly, it can be improved upon somewhat, but I figure best to update the Trac ticket for the time being as no one has replied on IRC to my inquiry yet.

[artkiver (グレェ)]

Portfile for net/snac which adds the makefile PortGroup, updates the version and checksums for 2.19, updates the long_description and tests OK locally.

[artkiver (グレェ)]

Well, what "worksforme" on my local system apparently made the build bots unhappy when I submitted a PR:

e.g.

​https://github.com/macports/macports-ports/actions/runs/4021961917/jobs/6911272754
​https://github.com/macports/macports-ports/actions/runs/4021961917/jobs/6911272700

I can, presumably, revert to an even more conservative Portfile version bump and I am guessing it would pass the CI checks OK, but probably not adequately address the concerns raised here.

[artkiver (グレェ)]

A more conservative PR was submitted here: ​https://github.com/macports/macports-ports/pull/17799

To update snac to 2.23 to be in alignment with the upstream project.

Thankfully that PR passed the CI build bot checks at least unlike my previous documented attempt, but omits the recommendations and other efforts described in this ticket, even if it does at least address the concerns raised in this one:

#66989

So, for the time being I am updating this ticket and leaving it open as a reminder that I have additional effort to put in so as to refactor the Portfile to utilize the makefile PortGroup.

[ryandesign (Ryan Carsten Schmidt)]

Replying to artkiver:

​https://guide.macports.org/chunked/reference.portgroup.html is a bit sparse as far as information on the makefile PortGroup.

​https://github.com/macports/macports-ports/blob/master/_resources/port1.0/group/makefile-1.0.tcl

Is I guess a useful reference insomuch as it is the TCL being invoked, though I am not sure how well I am parsing it as a human.

Yes, the portfile code, and the comments at the top of that file, are the documentation at present.

Regardless, doing some minor edits to the Portfile

e.g. PortGroup makefile 1.0

Still seems to install OK after I do a port -v clean.

Yes, but the entire purpose of the makefile portgroup is to assist you with projects that use custom Makefiles. As such, there is no one-size-fits-all solution and it is a virtual certainty that you will need to analyze the project's Makefile, and the settings available in the makefile portgroup, and adjust those settings to match what's needed for this Makefile and/or patch the Makefile to make it accept the level of customization we require.

In reading, I thought about adding:

build.args-append   CFLAGS=

And it doesn't seem to cause harm?

The correct way to augment the CFLAGS of a MacPorts port would be to -append to the configure.cflags variable:

configure.cflags-append -g -Wall -D st_mtim=st_mtimespec

The Makefile already puts -g -Wall into the CFLAGS but only if they weren't already set (and once we use the makefile portgroup, they will already be set). I think it's advisable to patch the Makefile so that it appends these flags to any existing value already present for CFLAGS.

I even explored removing the

patchfiles          Makefile.patch
post-patch          {reinplace "s|/usr/local|${prefix}|g" ${worksrcpath}/Makefile}

The makefile portgroup takes care of setting the PREFIX variable so if that were the only place where /usr/local occurred then you could remove the post-patch reinplace. But it isn't: the Makefile contains many other hardcoded references to /usr/local which also need to be replaced. I'll try to update the patch to fix these so the reinplace is no longer needed. This problem should be reported to the developers so that they can incorporate this patch or fix it another way.

The Makefile.patch shouldn't be removed because it currently does two things which are still needed (and will need to be expanded to fix additional problems):

1. It defines st_mtim=st_mtimespec which still needs to be done either there or by appending to the CFLAGS in the Portfile; the CI build logs you provided showed the build failed because this was not done. Since this should only be done on specific operating systems like Darwin, it's probably better to set it in a platform block in the Portfile. This problem should be reported to the developers so that they can modify their build system to set this on Darwin so we don't have to do it ourselves.
2. It teaches the Makefile how to support DESTDIR which is still desirable and which should be submitted to the developers of this software so they can incorporate it (although the makefile portgroup can be used whether or not a Makefile supports DESTDIR; just set makefile.has_destdir correctly).

As well as attempting to add some other things to build.args-append

Such as:

PREFIX=/opt/local
and PREFIX_MAN=/opt/local/share/man

The makefile portgroup sets PREFIX for you so you don't need to do it here again however the makefile portgroup sets PREFIX as an environment variable. Any variable set unconditionally in the Makefile will override an environment variable, and this Makefile does unconditionally set PREFIX. The solution is to either modify the Makefile so that it sets PREFIX only if it has not already been set (PREFIX?=/usr/local) or to use the makefile portgroup's option makefile.override-append PREFIX to tell it to set a build argument instead of an environment variable.

The Makefile defaults PREFIX_MAN to $(PREFIX)/man which is not the correct value today, so you're right that you'll need to set it to ${prefix}/share/man, either by specifying PREFIX_MAN in build.args or in the patchfile. In fact some work had already been done in the patchfile to correct the manpage installation directory but it was done wrong.

All of this should be fixed in this PR: ​https://github.com/macports/macports-ports/pull/18212

[artkiver (グレェ)]

Awesome, thank you for the informative and detailed reply! I have only skimmed it for the time being and am about to embark on a road trip, but I wanted to at least acknowledge the effort you put into this now before I am mostly away from a keyboard. I look forward to revisiting this as I am able!

Replying to ryandesign:

Replying to artkiver:

​https://guide.macports.org/chunked/reference.portgroup.html is a bit sparse as far as information on the makefile PortGroup.

​https://github.com/macports/macports-ports/blob/master/_resources/port1.0/group/makefile-1.0.tcl

Is I guess a useful reference insomuch as it is the TCL being invoked, though I am not sure how well I am parsing it as a human.

Yes, the portfile code, and the comments at the top of that file, are the documentation at present.

Regardless, doing some minor edits to the Portfile

e.g. PortGroup makefile 1.0

Still seems to install OK after I do a port -v clean.

Yes, but the entire purpose of the makefile portgroup is to assist you with projects that use custom Makefiles. As such, there is no one-size-fits-all solution and it is a virtual certainty that you will need to analyze the project's Makefile, and the settings available in the makefile portgroup, and adjust those settings to match what's needed for this Makefile and/or patch the Makefile to make it accept the level of customization we require.

In reading, I thought about adding:

build.args-append   CFLAGS=

And it doesn't seem to cause harm?

The correct way to augment the CFLAGS of a MacPorts port would be to -append to the configure.cflags variable:

configure.cflags-append -g -Wall -D st_mtim=st_mtimespec

The Makefile already puts -g -Wall into the CFLAGS but only if they weren't already set (and once we use the makefile portgroup, they will already be set). I think it's advisable to patch the Makefile so that it appends these flags to any existing value already present for CFLAGS.

I even explored removing the

patchfiles          Makefile.patch
post-patch          {reinplace "s|/usr/local|${prefix}|g" ${worksrcpath}/Makefile}

The makefile portgroup takes care of setting the PREFIX variable so if that were the only place where /usr/local occurred then you could remove the post-patch reinplace. But it isn't: the Makefile contains many other hardcoded references to /usr/local which also need to be replaced. I'll try to update the patch to fix these so the reinplace is no longer needed. This problem should be reported to the developers so that they can incorporate this patch or fix it another way.

The Makefile.patch shouldn't be removed because it currently does two things which are still needed (and will need to be expanded to fix additional problems):

1. It defines st_mtim=st_mtimespec which still needs to be done either there or by appending to the CFLAGS in the Portfile; the CI build logs you provided showed the build failed because this was not done. Since this should only be done on specific operating systems like Darwin, it's probably better to set it in a platform block in the Portfile. This problem should be reported to the developers so that they can modify their build system to set this on Darwin so we don't have to do it ourselves.
2. It teaches the Makefile how to support DESTDIR which is still desirable and which should be submitted to the developers of this software so they can incorporate it (although the makefile portgroup can be used whether or not a Makefile supports DESTDIR; just set makefile.has_destdir correctly).

As well as attempting to add some other things to build.args-append

Such as:

PREFIX=/opt/local
and PREFIX_MAN=/opt/local/share/man

The makefile portgroup sets PREFIX for you so you don't need to do it here again however the makefile portgroup sets PREFIX as an environment variable. Any variable set unconditionally in the Makefile will override an environment variable, and this Makefile does unconditionally set PREFIX. The solution is to either modify the Makefile so that it sets PREFIX only if it has not already been set (PREFIX?=/usr/local) or to use the makefile portgroup's option makefile.override-append PREFIX to tell it to set a build argument instead of an environment variable.

The Makefile defaults PREFIX_MAN to $(PREFIX)/man which is not the correct value today, so you're right that you'll need to set it to ${prefix}/share/man, either by specifying PREFIX_MAN in build.args or in the patchfile. In fact some work had already been done in the patchfile to correct the manpage installation directory but it was done wrong.

All of this should be fixed in this PR: ​https://github.com/macports/macports-ports/pull/18212

[ryandesign (Ryan Carsten Schmidt)]

In 38d40d9147df82e023c3a0434909d0451c276b39/macports-ports (master):

snac: Fix manpage location, compiler, flags

Put manpages in the right directory. Use the right compiler and flags so
that legacy support can work properly.

Closes: #66751