Add aclocal.m4 to .gitignore

[ThomasHickman]

This is a generated file from automake, so should be included in the .gitignore file.

[daviesrob]

aclocal.m4 comes from aclocal, presumably called by automake which is not used by HTSlib. If you have an aclocal.m4 it's probably because you used autoreconf which tries to run automake even though it's not needed.

I'm not entirely sure we should hide aclocal.m4 as it's not supposed to be there and should really be deleted if it appears.

[ThomasHickman]

Hmm ok, I was just wondering why I had a aclocal.m4 in my htslib folder which wasn't ignored by git. I'll close the PR.

[jkbonfield]

It's there because autoreconf creates it. This is standard behaviour when checking out from git to get your configure script, so I think the PR is valid.

[ThomasHickman]

Just had a look at the README:

autoconf # ...and configure script (or use autoreconf to do both)

Surely we should ignore files that are generated by suggested methods?

[jkbonfield]

The INSTALL notes actually state to manually do autoheader followed by autoconf, but autoreconf is so ubiquitous as a synonym for running all the autotools programs that I still think it's valid to ignore this file.

Arguably we should be using aclocal (the extra bit that autoconf runs) anyway as this adds PKG_CONFIG path support to the configure script, which we do use in our configure script already.

I don't understand the insistence on promoting the harder route either.

[jmarshall]

“Insistence”? “harder”? 😄

I imagine README.md was originally written that way so as to act as documentation of which parts of the autotools the project is using. And aclocal.m4 would not have been originally listed as ignored because if it did appear (in a non-automake setup like htslib is) it should be consciously either git added or nuked, as @daviesrob commented.

I'm not really sure why autoreconf is insisting on running aclocal and creating this.
At the time that README.md text was written, autoreconf did not create aclocal.m4 as there was nothing to go in it. Since then there is and it does. Actually this might be a bug introduced when the PKG_CONFIG stuff was added to configure.ac — on current develop, autoheader; autoconf and autoreconf produce different configure files. Seems likely that in fact aclocal.m4 needs to be added, not ignored (or aclocal added to the instructions, but this is basically the same question as whether the m4/ directory should be checked in).

[jkbonfield]

Aclocal does nothing if it finds nothing that needs adding.

Here it is finding our use of PKG_CONFIG and so adding parameters to permit the user to adjust the bin and lib search paths, which isn't a bad thing. I'm not sure it actually works though as I've given it nothing more than a cursory glance.

Thus the configure script we get differs if we use the prescribed route vs autoreconf route.

[jkbonfield]

Crossed-lines there. No, we shouldn't add auto-generated files. We don't add configure, and this falls into the same case. We do state it is something to ignore though.

[jmarshall]

We're agreeing that there is a bug here; but aclocal.m4 is in a different category from configure.

configure is generated from other files within the HTSlib distribution. Indeed it should not be checked in (in 2018; in the old days there was an argument for checking it in to control for different versions of autoconf).

aclocal.m4 OTOH is a cache of m4 functions from outwith HTSlib, which at least in theory people might not have or might have borken versions of. (Compare samtools/samtools@30bc5e2's commit message.) So there are reasons to consider checking it in.

[jkbonfield]

Ok I finally get it. My mistake here was believing the m4 files in /usr/share/aclocal/ were bundled as part of aclocal, but they're not. That's /usr/share/aclocal-1.14 (for example). So yes, agreed the pkg.m4 script which gets put into aclocal.m4 should probably be checked in so it is included irrespective of whether someone has pkgconfig (although if not then frankly it won't work anyway, so we're down to the case of dealing with autoreconf on one system and ./configure on another).

Anyway, this issue highlighted something interesting and a bug still. Thanks @ThomasHickman

[sb10]

Having aclocal.m4 committed means that you end up with dirty builds. I sort of followed the above discussion, but didn't understand the conclusion. What benefit does it give anyone to provide a file that they will generate for themselves?