We are currently migrating Bugzilla to GitHub issues.
Any changes made to the bug tracker now will be lost, so please do not post new bugs or make changes to them.
When we're done, all bug URLs will redirect to their equivalent location on the new bug tracker.

Bug 804

Summary: facelift of configure.in and other autotools stuff
Product: SDL Reporter: MatÄ›j TÃ½Ä <matej.tyc>
Component: buildAssignee: Ryan C. Gordon <icculus>
Status: RESOLVED WONTFIX QA Contact: Sam Lantinga <slouken>
Severity: enhancement    
Priority: P2    
Version: HG 2.0   
Hardware: All   
OS: All   
Attachments: facelifted configure.in
patch to the ac_have_define macro

Description MatÄ›j TÃ½Ä 2009-09-22 07:41:54 UTC
Hello,
I have took a look on your configure.in and I have to say that it looks pretty old.
I have run the "autoupdate" utility (an official tool shipped with autoconf) and there is quite a lot of diffs there already.
Next, there is quite a lot of if ... then ... fi shell constructs in the file, although there is an autoconf macro for it (it looks better with the macro :-)
I could take a look and send you eventually some patches, just let me know whether you are interested...

Maybe I should emphasize that those changes are not expected to affect backward compatibility.
Comment 1 Sam Lantinga 2009-09-26 03:41:50 UTC
Sure, I'd love to see a patch.

Thanks!
Comment 2 MatÄ›j TÃ½Ä 2009-09-27 05:13:44 UTC
OK, so I have begun the work.
Nevertheless, I have discovered that the current SDL doesn't actually use automake.
How is the developer's attitude to that tool? When I am at it, I may also write an automake support (the result would be much simpler and easier to maintain than the current hand-written Makefile.in).
Would you be interested in that? Some people (Allegro developers for instance :-) seem to suffer from a strange automake allergy...
Comment 3 Sam Lantinga 2009-09-27 10:20:32 UTC
No, SDL used to use automake and I spent literally months trying to get it working on all versions of automake in the wild.  In the end it was actually easier to rewrite it than continue to try to work around issues with various versions of automake.
Comment 4 MatÄ›j TÃ½Ä 2009-09-27 12:07:29 UTC
Well, that's pretty interesting. Deja vu :-)
Actually I think that automake is quite stable now. If one looks at the automake timeline,
http://www.gnu.org/software/libtool/manual/automake/Timeline.html#Timeline
one can guess that times when there had to be more versions of automake in the repos are gone for good.
Actually, what I suggest is to reconsider supporting automake in further major release that can be more relaxed regarding backward compatibility. I just see quite a few workarounds in the configure.in file that could be done in a more elegant way by autotools.

Hm, when I think about it, the problem is little bit weird. When you produce a tarball using your favourite version of automake, the others just run the configure script (without having to have autotools installed at all) on any supported platrorm and that's it. I was a happy MSW user when the automake hell was going on, so I was not touched by it, so don't be annoyed by this.
Comment 5 Sam Lantinga 2009-09-27 12:37:23 UTC
I would suggest doing it in two stages.  First, clean up configure.in with macros that are available on all shipping versions of autoconf.  Second, submit a patch converting back to automake that can be carefully evaluated.

Thanks!
Comment 6 MatÄ›j TÃ½Ä 2009-10-05 14:05:49 UTC
Created attachment 398 [details]
facelifted configure.in

Hello, what you see in the attachement is the rewritten configure.in reflecting the latest SVN status.
It does the same thing, but things like formatting and if/case statements are changed. I think that it is more practical to send a file instead of a patch.

So how to test it? 
1. Copy configure.ac next to configure.in
2. Patch acinclude.m4
3. Run 'aclocal' and 'autoreconf -f' (instead of autogen.sh)
4. Run ./configure with your parameters and 'make' - as you are used to

In the a case you are in doubts, it should be easy to check against the current configure.in, just rely on search patterns to get the location in the other file, not on line numbers :-)

What really needed to be changed: AC_DEFINE now requires all three parameters with the current autoconf. So acinclude.m4 needs to be patched as well.
Comment 7 MatÄ›j TÃ½Ä 2009-10-05 14:09:03 UTC
Created attachment 399 [details]
patch to the ac_have_define macro

patched to comply to the up-to-date autoconf
Comment 8 Sam Lantinga 2009-10-05 23:31:31 UTC
Thanks.  Out of curiosity, what is the benefit of this change?

I find the macro indentation really hard to follow.

Also, autoreconf is blowing away SDL_config.h.in, which has changes which I need.  Is there any way to prevent autoreconf from overwriting it?
Comment 9 MatÄ›j TÃ½Ä 2009-10-06 05:22:43 UTC
Hello,
I have corrected some potential quoting issues that can make things look like more cluttered and changed # to dnl (# is an m4 character which is the reason why the test programs that use preprocessor are double quoted).
Concerning indentation, this style allows you to see easily how are macros nested inside each other. I also like the "one large parameter per line" style since it makes more visible what is going on.

Concerning your SDL_config.h.in, the reason it gets blown away most likely is if you pass the "-f" argument to autoreconf that I have told you to do so.
So restore your SDL_config.h.in and when you need to regenerate configure script, just run 'autoreconf' with no arguments.
Comment 10 MatÄ›j TÃ½Ä 2009-10-08 14:31:41 UTC
Well, FYI, there were some errors introduced into the "old" configure.in.
You can't use the [ and ] brackers as you are used to in conditionals, because they are m4 characters. You should use 'test' everywhere. There are some 'if [' patterns in that file and AFAIK the configure script should complain when it reaches such points. I have not posted a bugreport since I have corrected it in the configure.ac file.

I see that the configure.in is in a state of evolution and it is apparent that it can drift apart from the configure.ac. The .ac file is supposed to do exactly the same as the old one, it should work with the autogen.sh in the same way as the old one did.

What am I trying to say? If you have some questions, objections or suggestions, tell me so we can move ahead. I won't be mad if you reject the stuff I have sent although it took me some effort, but I thing that things should get moving either direction since everything changes all the time.

I see quite a big potential of making the file much smaller and more correct from autoconf's point of view. The configure.ac I have sent to you is the basis you are actually used to and that can be improved. And by looking how huge and repetitive configure.in is, you probably agree that it could use some improvement.

Regards,
Matej
Comment 11 Sam Lantinga 2009-10-09 22:36:35 UTC
So I'm still running into problems with it.

I tried to run autoreconf on my Ubuntu system and it complained about missing libtool.  Then, when I installed libtool, libtool said this:
libtoolize: putting auxiliary files in `.'.
libtoolize: copying file `./ltmain.sh'
libtoolize: Consider adding `AC_CONFIG_MACRO_DIR([m4])' to configure.ac and
libtoolize: rerunning libtoolize, to keep the correct libtool macros in-tree.
libtoolize: Consider adding `-I m4' to ACLOCAL_AMFLAGS in Makefile.am.

And finally, it blew away SDL_config.h.in anyway.

Suggestions?
Comment 12 MatÄ›j TÃ½Ä 2009-10-10 06:16:12 UTC
Thanks,
just FYI what is going on here:
Why you suddenly need to install libtool? Since even the original configure.in contains the LT_INIT macro call, you really do need libtool. However, the original setup seems to bundle it, it is probably contained in the acinclude.m4 file (it is quite complicated one to read through :-).

Then, autoreconf tries to make the configure script as well as possible. So it calls various utilities like aclocal, autoheader, autoconf etc. 
The problem with blowing away SDL_config.h.in is due to the fact that autoreconf always calls the autoheader utility.
Autoheader does a pretty good thing, it ensures that the SDL_config.h.in is in sync with configure.ac However, it can't recognize that it was actually you who intentionally added some stuff there and that it is not some junk that comes from a previous out-of-date version of configure.ac

Finally, those suggestions you saw can be ignored for now.

SOLUTIONS:
1. Rename configure.ac to configure.in and use autogen.sh as you are used to. The behavior should be identical to one you are used to, too.
2. The current state is probably not optimal. I would suggest leaving SDL_config.h to autotools and make another header that would include SDL_config.h. You can call it eg. SDL_stuff.h and include it wherever you need SDL_config.h. This would result in separation of human-edited data from autogenerated data, that is IMO not a bad idea at all.
There are also other useful and nicely written autotools tips here, if you are interested:
http://www.linux.com/archive/articles/114061
Comment 13 Sam Lantinga 2009-10-10 11:18:07 UTC
So copying configure.ac to configure.in works pretty well.

You did a really good job converting it to macro format, thanks!

I'm still not sure whether I'm comfortable creating and maintaining this syntax though.  I know Bourne shell scripting pretty well, but I get lost following the macro indenting and parameters.

For example:
                      AS_CASE(["$host"],
                              [*-*-darwin*], dnl Latest Mac OS X actually ships with Xrandr/Xrender libs...
                              [x11_symbols_private=yes dnl TODO: Does this really have to bee here
                               x11_lib='/usr/X11R6/lib/libX11.6.dylib'
                               x11ext_lib='/usr/X11R6/lib/libXext.6.dylib'
                               xrender_lib='/usr/X11R6/lib/libXrender.1.dylib'
                               xrandr_lib='/usr/X11R6/lib/libXrandr.2.dylib'
                               xinput_lib='/usr/X11R6/lib/libXi.6.dylib'
                               xss_lib='/usr/X11R6/lib/libXss.6.dylib'],
                              [*-*-osf*],
                              [x11_lib='libX11.so'
                               x11ext_lib='libXext.so'],
                              [*-*-irix*], dnl IRIX 6.5 requires that we use /usr/lib32
                              [x11_lib='libX11.so'
                               x11ext_lib='libXext.so'],
                              [*],
                              [x11_lib_spec=[`echo $X_LIBS | sed 's/.*-L\([^ ]*\).*/\1/'`]
                               for path in $x11_lib_path /usr/$base_libdir /usr/X11/$base_libdir /usr/X11R6/$base_libdir; do
                               AS_IF([test "x$x11_lib" = "x"],
                                     [x11_lib=[`ls -- $path/libX11.so.[0-9] 2>/dev/null | sort -r | sed 's/.*\/\(.*\)/\1/; q'`]
                                      AS_IF([test "x$x11_lib" = "x"],
                                            [x11_lib=[`ls -- $path/libX11.so.[0-9]* 2>/dev/null | sort -r | sed 's/.*\/\(.*\)/\1/; q'`] ]) ])
Comment 14 Sam Lantinga 2009-10-10 11:45:11 UTC
Also, on FreeBSD, I get this output:
configure.in:1: error: possibly undefined macro: dnl
configure.in:5: error: possibly undefined macro: AC_USE_SYSTEM_EXTENSIONS
configure.in:50: error: possibly undefined macro: AS_IF
configure.in:51: error: possibly undefined macro: AC_DEFINE
configure.in:75: error: possibly undefined macro: AS_CASE
configure.in:109: error: possibly undefined macro: AC_ARG_ENABLE
...
Comment 15 Sam Lantinga 2009-10-10 11:52:40 UTC
I appreciate all your work, but I think we'll pass on this at this time.

If there are specific changes that fix bugs or other issues, please let me know!

Thanks!
Comment 16 MatÄ›j TÃ½Ä 2009-10-11 06:56:08 UTC
I will answer some of your questions, not sure how much are you interested now :-)
Hmm, the second error is pretty bad, difficult to say what's going on there. If
you swap to the original configure.in it is all OK? Errors like that usually
pop up when there are some unmatched parentheses, but that would cause problems
on Linux, too.
Could you check whether running autoreconf also results in those errors?

Regarding the indentation:
Using macros instead of pure shell constructs has some advantages.
It is more portable than just using shell constructs:
http://www.gnu.org/software/autoconf/manual/autoconf.html#Limitations-of-Builtins
The indentation helps you to see how macros are nested inside of each other.
And you can use editors bracket matching function to see easily what is in the
command block of if/case. 
Make sure you use an editor with tab width 8 and syntax highlighting :-) 

Now about the negative response. Although I understand that swapping a 2400
line file for another in the state when the old one works and the new one not
without issues might not look appealing, there are some things that should be
resolved.

1. Now AC_DEFINE has to have all 3 parameters. You can't use recent autoconf
(autoheader) with the old file now. And the newer autostuff is usually more
mature and faster.
2. There are depreceated macros used in there, but this is not an issue
compared to the point 1.
3. The macros in the old file are underquoted. This might not matter most of
the time, but you probably know how it is. If the original is underquoted, then
the contributions will be underquoted, too and one day a strange error may
occur.
4. Just a suggestion: I recommend to throw away platform checks wherever
possible and just to check for features. For instance if you check for
windows.h header on Linux, the check will fail. A check for X11 will fail on
Windows. I think that one doesn't have to check for the host platform to
perform these checks like it is done at the moment.
5. I believe that configure.in is unneccessarilly big. I have an impression that it can be made much smaller and cleaner with some effort. 

OK, that's it. If you don't change your mind, I would like to advise you to
post my configure.ac to your forums consider it as public domain. Maybe someone from SDL developers or trusted users could get interested in it and you will get some benefit from that.
Comment 17 Sam Lantinga 2009-10-11 08:18:27 UTC
Thanks.  I'll definitely post the configure.ac and let people play with it.

For future reference, configure.ac is current with SDL 1.3 revision 4928.

See ya!