[MacPorts] #66751: snac @2.18: use the right compiler and flags

[MacPorts]

#66751: snac @2.18: use the right compiler and flags
-------------------------+----------------------
  Reporter:  ryandesign  |      Owner:  artkiver
      Type:  defect      |     Status:  assigned
  Priority:  Normal      |  Milestone:
 Component:  ports       |    Version:  2.8.0
Resolution:              |   Keywords:  haspatch
      Port:  snac        |
-------------------------+----------------------
Changes (by ryandesign):

 * keywords:   => haspatch


Comment:

 Replying to [comment:2 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

-- 
Ticket URL: <https://trac.macports.org/ticket/66751#comment:5>
MacPorts <https://www.macports.org/>
Ports system for macOS