Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update config.guess and config.sub and enable ./autogen.sh to do so #40

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

binki
Copy link
Contributor

@binki binki commented Sep 3, 2016

IMO, config.guess and config.sub and ./configure and other things I’m forgetting about should be put in .gitignore and only included in dist tarballs. But since atheme seems to want to include them, this is what I had to do to get ./configure to run cleanly on my system. See #39.

At least with this PR ./autogen.sh is fixed to be smart enough to update config.sub and config.guess (which may be patched to the user’s environment, for example), so if these don’t work for others they can at least fix it themselves now instead of having to manually look for them.

To do this, it has a small automake project in autogen-automake-trap
and uses automake --add-missing --copy --force to get the updated files.
Then it copies them to the main source directory which uses
AC_CANONICAL_HOST and AC_CANONICAL_TARGET but does not use automake.
Fixes atheme#39.

Honestly, I have no idea why these files are tracked at all in the VCS.
They should only exist in dist tarballs. People altering source code when
working with the VCS will know to run ./autogen.sh, especially as they
might need to alter configure.ac anyway.
@grawity
Copy link
Contributor

grawity commented Sep 4, 2016

I have no experience with autotools, but I wonder if it wouldn't be better to fix the buildsystem so that autoreconf -f -i would work like in other projects...

@binki
Copy link
Contributor Author

binki commented Sep 6, 2016

@grawity idk why it is this way, but libmowgli2 uses this “buildsys.mk” thing because it’s not libtool+automake. libmowgli2’s use of AC_CANONICAL_TARGET and AC_CANONICAL_HOST rely on config.guess and config.sub which is provided by Automake. Enabling autoreconf -fi to work for libmowgli2 like it does for other autotools projects would mean switching to Automake and dropping buildsys.mk. If libmowgli2 really wants to do this, I would be willing to submit a PR, but it might take me a bit of time. Or another way of fixing #39 would just be to grab updated config.guess and config.sub files from the most recent Automake relase manually and commit them to the repo (that’d be a continuation of the status quo).

As I stated before, I personally don’t like how ./configure (a generated file), config.sub, config.guess, install-sh, etc., and even buildsys.mk itself (buildsys is its own project and including it in the VCS is like committing the source tree of a dependency, e.g., openssl to your own VCS). I do recognize that bundling this stuff doing this makes it easier for people to start contributing, especially if they’re only modifying a few lines of C. But it’s kind of noise in the VCS history…

As an aside, I actually haven’t yet managed to compile libmowgli2 under MSYS2 yet. And, at the very moment, my MSYS2 dev environment is tied to a broken keyboard xD. But if I do make any headway I will try to at least post any changes I made as I suppose others might like to see it working under MSYS2.

So, let me know which option you want:

  1. This patch (stay with buildsys.mk and use an ugly hack to get ./autogen.sh to be able to update config.guess and config.sub).
  2. A PR that just does a one-off update of config.guess and config.sub from a contemporary Automake release (continuation of the status quo).
  3. Switching to Automake+libtool/getting autoreconf -fi to work (I’d be surprised if this is the consensus :-p).

@maxteufel
Copy link
Member

I think switching to automake+libtool would be a good idea (same applies to atheme itself) since the current build system looks quite messy to me. But please let others comment on this as well.

Cc: @ilbelkyr @siniStar7boy

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants