aclocal.m4

[jkbonfield]

A new issue as #733 cannot be reopened.

This debate is still raging.

@daviesrob claims the aclocal.m4 is not needed at all, in which case we should delete it.

Others seem to think it is needed, but it should not be checked in as we get dirty builds when following the build notes. Options here are to remove the file or to fix the build notes (they explicitly state autoreconf may be used).

@jmarshall persuaded me before, hence why I closed the issue, that the appropriate m4 file must be checked in because it's not a standard part of the autotools package (it only exists if you have the pkgconfig bits installed).

With hindsight I now disagree, but rather than revert it, I think I'll punt to Rob who understands the whole pkgconfig auto-detection stuff a lot more (as he wrote it I think).

We don't bundle a compiler, just incase it hasn't been installed. That would be lunacy! (Hello iRODS). We don't bundle autotools just incase they're not installed either. So I don't have an issue with needing pkgconfig installed if you want the aclocal.m4 part to be added either, especially if as Rob points out it does nothing whatsoever anyway.

[jmarshall]

The dirty trees are indeed annoying. They would be less dirty if we all had the same version of aclocal/etc installed (BTW the aclocal.m4 checked in was generated with a somewhat outdated one), but of course that's not very achievable.

What I said before was “there are reasons to consider checking it in” and I was a bit surprised when that actually happened. 😄

After PKG_CONFIG was added to configure.ac and before aclocal.m4 was checked in, there was a bug in the instructions as autoheader; autoconf and autoreconf produced different configure files (as noted in #733 (comment)). So if aclocal.m4 were to be deleted from the repo and git-ignored (which I would not disagree with), then the show-the-parts-of-the-autotools instructions need to be correspondingly changed to aclocal; autoheader; autoconf. (If that path is chosen, the release scripts would also have to ensure an up-to-date aclocal.m4 makes its way into the release tarballs and is used to make the tarball's configure.)

(There is also a third option of having aclocal.m4 checked in and also git-ignoring changes in it, but I'm not sure how well that works and it is probably not worth investigating.)

(CC @sb10.)

[daviesrob]

The problem here is that aclocal is updating pkg.m4 which is pretty much the only thing in our aclocal.m4. We currently have serial 1, the latest seems to be serial 12, which is about a year and a half old.

There are a few possible solutions:

• Check in the updated aclocal.m4.
• Copy it into the m4 directory and m4_include it. Remove aclocal.m4 as it's no longer needed.
• Just copy in the single function we use (PKG_PROG_PKG_CONFIG).
• Copy the single function and rename it.

With the first option we would eventually learn about newer versions of pkg.m4 as aclocal would modify aclocal.m4 again.

The second option seems to be enough to stop aclocal.m4 from coming back, even if a newer pkg.m4 is available.

The third means we only ship the bit we need, and the fourth would stop our possibly out of date version from interfering with the real pkg.m4.

I suspect the second option may be the best, but other opinions are welcome.

[zamaudio]

Sorry just saw this thread but already submitted a PR to fix it #1091

[jmarshall]

PR #1091 is an incomplete fix — choosing that route also requires adding aclocal to the instructions, at least.

I agree with @daviesrob that the second option is probably best: copy pkg.m4 into the m4 directory and m4_include it, and remove (but don't .gitignore) aclocal.m4.

[zamaudio]

Have updated my PR #1091 to reflect option 2.

[zamaudio]

Fixed in #1091