Re: Automake!

[Bastien Nocera]

On Thu, 2007-11-29 at 19:52 +0100, Pawel Kot wrote:
> Hi,
> 
> I've got few questions/comments.
> 
> On Nov 29, 2007 5:18 PM, Bastien Nocera <address@hidden> wrote:
> > A big patch to automake gnokii. It adds a (maintainer-only) dependency
> 
> maintainer-only == for those who build from CVS, right?

Yep.

> > on glib-gettextize, and requires a libc with libintl (I don't think this
> > would cause any problems, even under MSVC).
> 
> What about solaris/*bsd/mac osx?

They can all run glib fine, so they all have libintl (recent versions of
glib have a hard requirement for those functions, so we don't need to
cater for 10-year OSes).

> +if GLIB
> +# FIXME
> +#SUBDIRS += smsd
> +endif
> 
> Perhaps not autodetect but with explicit --enable-smsd option?

Well, that's the plan, but it's disabled for now, as it requires a lot
of changes to configure.in.

> -autopoint -f
> 
> Why?

Because the gettext bits needed are installed by glib-gettextize.

> -dnl  Copyright (C) 1999 Hugh Blemings & Pavel Jan?k ml.
> -dnl                2000 Karel Zak, Pawe? Kot
> +dnl  Copyright (C) 1999 Hugh Blemings & Pavel Jan?k ml.
> +dnl                2000 Karel Zak, Pawe? Kot
> 
> These are probably encoding issues. I'd like to keep everything utf-8.

I'll change the strings to be UTF-8 (they weren't).

> +AC_INIT([gnokii],
> +      [0.6.22],
> 
> Would it be possible to read from VERSION?

Right now you change VERSION by hand for CVS/release versions. I think
it would be easier to have one version in configure.in (changing
depending on whether the configure is being run from CVS or not), and
the config.h with the VERSION defined, rather than rely on an external
file.

When the VERSION file is modified, configure needs to be re-run to
update the config.h, so you might as well do away with the separate
VERSION file (we should also move the libtool versioning to configure as
well).

> +AM_INIT_AUTOMAKE([1.9 dist-bzip2 no-dist-gzip])
> 
> I'd like to have dist-gzip as well.

I can make it spit out both, although usually bzip2 are generated by
make dist, and then the gzip one is generated by whatever release
scripts.

> -AC_CONFIG_HEADER(include/config.h)
> +AC_CONFIG_HEADERS([include/config.h])
> 
> Not that it matters much, but why?

Just that the autotools like complaining about unquoted strings.

> +    smsd support:       $found_glib
> More logical would be using $have_smsd or something.

As I mentioned, smsd support isn't finished. I see a big FIXME in the
smsd/Makefile saying it doesn't support conditional backends. Definitely
more work needed there.

> --- /dev/null 1 Jan 1970 00:00:00 -0000
> +++ common/localcharset.c     29 Nov 2007 16:00:10 -0000
> 
> What is that?

It's needed for locale_charset, which isn't a POSIX function, and only
exists in libintl. Copied there as it's the only place it's used. Direct
copy from gettext.

> -bool GTerminateThread;
> +extern bool GTerminateThread;
> 
> I don't think so.
> 
> -bool GTerminateThread;
> +extern bool GTerminateThread = false;
> 
> Ditto.
> 
> -bool GTerminateThread = false;
> +extern bool GTerminateThread;
> 
> Ditto.

Actually, this is correct. You want the global living in the library,
not in the program, otherwise you can end up with the global not being
defined in the program, and it would fail to link...

> And no, I haven't tried it yet. :)

I don't blame you ;)

Cheers