[pacman-dev] [PATCH 1/3] configure.ac: cleanup duplication in --enable-git-version

Dave Reisner

22 Sep 2012 22 Sep '12

4:17 p.m.

Avoid adding our own messaging, as autoconf will add this for us with the result of the AC_CHECK_FILE test. Reuse the cache variable from autoconf to set our local variable. Signed-off-by: Dave Reisner --- configure.ac | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/configure.ac b/configure.ac index 7fe696a..7c65a75 100644 --- a/configure.ac +++ b/configure.ac @@ -424,13 +424,9 @@ AC_MSG_CHECKING(whether to use git version if available) if test "x$wantgitver" = "xyes" ; then AC_CHECK_PROGS([GIT], [git]) AC_CHECK_FILE([.git/], hasgitdir=yes) + usegitver=$ac_cv_file__git if test $GIT -a "x$hasgitdir" = "xyes"; then - AC_MSG_RESULT([yes]) - usegitver=yes AC_DEFINE([USE_GIT_VERSION], , [Use GIT version in version string]) - else - AC_MSG_RESULT([no, git or .git dir missing]) - usegitver=no fi else AC_MSG_RESULT([no, disabled by configure]) -- 1.7.12.1

Show replies by date

Dave Reisner

22 Sep 22 Sep

4:17 p.m.

New subject: [pacman-dev] [PATCH 2/3] buildsys: eschew use of DEFS, prefer AM_CPPFLAGS

This is redundant, and any usage of -D should belong to CPPFLAGS. Signed-off-by: Dave Reisner --- configure.ac | 1 - lib/libalpm/Makefile.am | 5 ++--- src/pacman/Makefile.am | 18 ++++++++---------- src/util/Makefile.am | 14 ++++++-------- 4 files changed, 16 insertions(+), 22 deletions(-) diff --git a/configure.ac b/configure.ac index 7c65a75..5061078 100644 --- a/configure.ac +++ b/configure.ac @@ -486,7 +486,6 @@ ${PACKAGE_NAME}: compiler : ${CC} preprocessor flags : ${CPPFLAGS} compiler flags : ${WARNING_CFLAGS} ${CFLAGS} - defines : ${DEFS} library flags : ${LIBS} ${LIBSSL_LIBS} ${LIBARCHIVE_LIBS} ${LIBCURL_LIBS} ${GPGME_LIBS} linker flags : ${LDFLAGS} diff --git a/lib/libalpm/Makefile.am b/lib/libalpm/Makefile.am index 911e52b..c935e2d 100644 --- a/lib/libalpm/Makefile.am +++ b/lib/libalpm/Makefile.am @@ -5,10 +5,9 @@ SUBDIRS = po lib_LTLIBRARIES = libalpm.la include_HEADERS = alpm_list.h alpm.h -DEFS = -DLOCALEDIR=\"@localedir@\" @DEFS@ - AM_CPPFLAGS = \ - -imacros $(top_builddir)/config.h + -imacros $(top_builddir)/config.h \ + -DLOCALEDIR=\"@localedir@\" AM_CFLAGS = -pedantic -D_GNU_SOURCE $(WARNING_CFLAGS) diff --git a/src/pacman/Makefile.am b/src/pacman/Makefile.am index c8ce977..d296d48 100644 --- a/src/pacman/Makefile.am +++ b/src/pacman/Makefile.am @@ -9,23 +9,21 @@ logfile = ${localstatedir}/log/pacman.log bin_PROGRAMS = pacman -DEFS = -DLOCALEDIR=\"@localedir@\" \ - -DCONFFILE=\"$(conffile)\" \ - -DDBPATH=\"$(dbpath)\" \ - -DGPGDIR=\"$(gpgdir)\" \ - -DCACHEDIR=\"$(cachedir)\" \ - -DLOGFILE=\"$(logfile)\" \ - @DEFS@ - AM_CPPFLAGS = \ -imacros $(top_builddir)/config.h \ - -I$(top_srcdir)/lib/libalpm + -I$(top_srcdir)/lib/libalpm \ + -DLOCALEDIR=\"@localedir@\" \ + -DCONFFILE=\"$(conffile)\" \ + -DDBPATH=\"$(dbpath)\" \ + -DGPGDIR=\"$(gpgdir)\" \ + -DCACHEDIR=\"$(cachedir)\" \ + -DLOGFILE=\"$(logfile)\" AM_CFLAGS = -pedantic -D_GNU_SOURCE $(WARNING_CFLAGS) if USE_GIT_VERSION GIT_VERSION := $(shell sh -c 'git describe --abbrev=4 --dirty | sed s/^v//') -DEFS += -DGIT_VERSION=\"$(GIT_VERSION)\" +AM_CPPFLAGS += -DGIT_VERSION=\"$(GIT_VERSION)\" endif pacman_SOURCES = \ diff --git a/src/util/Makefile.am b/src/util/Makefile.am index 463abf7..d539fa2 100644 --- a/src/util/Makefile.am +++ b/src/util/Makefile.am @@ -6,16 +6,14 @@ cachedir = ${localstatedir}/cache/pacman/pkg/ bin_PROGRAMS = vercmp testpkg testdb cleanupdelta pacsort pactree -DEFS = -DLOCALEDIR=\"@localedir@\" \ - -DCONFFILE=\"$(conffile)\" \ - -DDBPATH=\"$(dbpath)\" \ - -DGPGDIR=\"$(gpgdir)\" \ - -DCACHEDIR=\"$(cachedir)\" \ - @DEFS@ - AM_CPPFLAGS = \ -imacros $(top_builddir)/config.h \ - -I$(top_srcdir)/lib/libalpm + -I$(top_srcdir)/lib/libalpm \ + -DLOCALEDIR=\"@localedir@\" \ + -DCONFFILE=\"$(conffile)\" \ + -DDBPATH=\"$(dbpath)\" \ + -DGPGDIR=\"$(gpgdir)\" \ + -DCACHEDIR=\"$(cachedir)\" AM_CFLAGS = -pedantic -D_GNU_SOURCE $(WARNING_CFLAGS) -- 1.7.12.1

Dave Reisner

4:17 p.m.

New subject: [pacman-dev] [PATCH 3/3] etc/Makefile: simplify build rule for conf files

This lets us define the build rule and the dependency all at once, and additionally removes the need for an intermediate temp file. Signed-off-by: Dave Reisner --- I'm actually not sure I grok the need for the tmp file here. If anyone happens to know why this patch might be a bad idea, feel free to chime in. etc/Makefile.am | 17 +++++++---------- 1 file changed, 7 insertions(+), 10 deletions(-) diff --git a/etc/Makefile.am b/etc/Makefile.am index 58a80bd..7a1b91b 100644 --- a/etc/Makefile.am +++ b/etc/Makefile.am @@ -4,8 +4,9 @@ EXTRA_DIST = makepkg.conf.in pacman.conf.in # Files that should be removed, but which Automake does not know. MOSTLYCLEANFILES = $(dist_sysconf_DATA) -#### Taken from the autoconf scripts Makefile.am #### -edit = sed \ +SED_PROCESS = \ + $(AM_V_GEN)$(MKDIR_P) $(dir $@) && \ + $(SED) \ -e 's|@sysconfdir[@]|$(sysconfdir)|g' \ -e 's|@localstatedir[@]|$(localstatedir)|g' \ -e 's|@prefix[@]|$(prefix)|g' \ @@ -19,14 +20,10 @@ edit = sed \ -e 's|@CARCH[@]|$(CARCH)|g' \ -e 's|@CHOST[@]|$(CHOST)|g' \ -e 's|@ARCHSWITCH[@]|$(ARCHSWITCH)|g' \ - -e 's|@ROOTDIR[@]|$(ROOTDIR)|g' + -e 's|@ROOTDIR[@]|$(ROOTDIR)|g' \ + < $< > $@ -$(dist_sysconf_DATA): Makefile - $(AM_V_at)$(RM) $@ $@.tmp - $(AM_V_GEN)$(edit) `test -f ./$@.in || echo $(srcdir)/`$@.in >$@.tmp - $(AM_V_at)mv $@.tmp $@ - -makepkg.conf: $(srcdir)/makepkg.conf.in -pacman.conf: $(srcdir)/pacman.conf.in +%.conf: %.conf.in Makefile + $(SED_PROCESS) # vim:set ts=2 sw=2 noet: -- 1.7.12.1

Allan McRae

25 Sep 25 Sep

4:19 a.m.

New subject: [pacman-dev] [PATCH 1/3] configure.ac: cleanup duplication in --enable-git-version

On 23/09/12 02:17, Dave Reisner wrote:

...

Avoid adding our own messaging, as autoconf will add this for us with the result of the AC_CHECK_FILE test. Reuse the cache variable from autoconf to set our local variable.

Signed-off-by: Dave Reisner --- configure.ac | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/configure.ac b/configure.ac index 7fe696a..7c65a75 100644 --- a/configure.ac +++ b/configure.ac @@ -424,13 +424,9 @@ AC_MSG_CHECKING(whether to use git version if available) if test "x$wantgitver" = "xyes" ; then AC_CHECK_PROGS([GIT], [git]) AC_CHECK_FILE([.git/], hasgitdir=yes) + usegitver=$ac_cv_file__git

That should be: $ac_cv_file__git_

...

if test $GIT -a "x$hasgitdir" = "xyes"; then - AC_MSG_RESULT([yes]) - usegitver=yes AC_DEFINE([USE_GIT_VERSION], , [Use GIT version in version string]) - else - AC_MSG_RESULT([no, git or .git dir missing]) - usegitver=no fi else AC_MSG_RESULT([no, disabled by configure])

Dave Reisner

4:24 a.m.

New subject: [pacman-dev] [PATCH 1/3] configure.ac: cleanup duplication in --enable-git-version

On Tue, Sep 25, 2012 at 02:19:16PM +1000, Allan McRae wrote:

...

On 23/09/12 02:17, Dave Reisner wrote:

...
Avoid adding our own messaging, as autoconf will add this for us with the result of the AC_CHECK_FILE test. Reuse the cache variable from autoconf to set our local variable.

Signed-off-by: Dave Reisner --- configure.ac | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/configure.ac b/configure.ac index 7fe696a..7c65a75 100644 --- a/configure.ac +++ b/configure.ac @@ -424,13 +424,9 @@ AC_MSG_CHECKING(whether to use git version if available) if test "x$wantgitver" = "xyes" ; then AC_CHECK_PROGS([GIT], [git]) AC_CHECK_FILE([.git/], hasgitdir=yes) + usegitver=$ac_cv_file__git

That should be:

$ac_cv_file__git_

Interesting that it still appeared to work as intended. Anyways, you're right. Fixed.

...

...
if test $GIT -a "x$hasgitdir" = "xyes"; then - AC_MSG_RESULT([yes]) - usegitver=yes AC_DEFINE([USE_GIT_VERSION], , [Use GIT version in version string]) - else - AC_MSG_RESULT([no, git or .git dir missing]) - usegitver=no fi else AC_MSG_RESULT([no, disabled by configure])

Allan McRae

4:25 a.m.

New subject: [pacman-dev] [PATCH 1/3] configure.ac: cleanup duplication in --enable-git-version

On 25/09/12 14:24, Dave Reisner wrote:

...

On Tue, Sep 25, 2012 at 02:19:16PM +1000, Allan McRae wrote:

...
On 23/09/12 02:17, Dave Reisner wrote:

...
Avoid adding our own messaging, as autoconf will add this for us with the result of the AC_CHECK_FILE test. Reuse the cache variable from autoconf to set our local variable.

Signed-off-by: Dave Reisner --- configure.ac | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/configure.ac b/configure.ac index 7fe696a..7c65a75 100644 --- a/configure.ac +++ b/configure.ac @@ -424,13 +424,9 @@ AC_MSG_CHECKING(whether to use git version if available) if test "x$wantgitver" = "xyes" ; then AC_CHECK_PROGS([GIT], [git]) AC_CHECK_FILE([.git/], hasgitdir=yes) + usegitver=$ac_cv_file__git

That should be:

$ac_cv_file__git_

Interesting that it still appeared to work as intended. Anyways, you're right. Fixed.

Hmm... it did not work for me (which is how I noticed it). Which brings up the point, do we want to rely on autoconf naming of such variables? Allan

Dave Reisner

4:33 a.m.

New subject: [pacman-dev] [PATCH 1/3] configure.ac: cleanup duplication in --enable-git-version

On Tue, Sep 25, 2012 at 02:25:59PM +1000, Allan McRae wrote:

...

On 25/09/12 14:24, Dave Reisner wrote:

...
On Tue, Sep 25, 2012 at 02:19:16PM +1000, Allan McRae wrote:

...
On 23/09/12 02:17, Dave Reisner wrote:

...
Avoid adding our own messaging, as autoconf will add this for us with the result of the AC_CHECK_FILE test. Reuse the cache variable from autoconf to set our local variable.

Signed-off-by: Dave Reisner --- configure.ac | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/configure.ac b/configure.ac index 7fe696a..7c65a75 100644 --- a/configure.ac +++ b/configure.ac @@ -424,13 +424,9 @@ AC_MSG_CHECKING(whether to use git version if available) if test "x$wantgitver" = "xyes" ; then AC_CHECK_PROGS([GIT], [git]) AC_CHECK_FILE([.git/], hasgitdir=yes) + usegitver=$ac_cv_file__git

That should be:

$ac_cv_file__git_

Interesting that it still appeared to work as intended. Anyways, you're right. Fixed.

Hmm... it did not work for me (which is how I noticed it).

Which brings up the point, do we want to rely on autoconf naming of such variables?

Allan

I don't see why not. The variable naming scheme is documented and intended to be used. FWIW, we used to use a cache variable for the purposes of detecting libssl.

Allan McRae

5:42 a.m.

New subject: [pacman-dev] [PATCH 1/3] configure.ac: cleanup duplication in --enable-git-version

On 25/09/12 14:33, Dave Reisner wrote:

...

On Tue, Sep 25, 2012 at 02:25:59PM +1000, Allan McRae wrote:

...
On 25/09/12 14:24, Dave Reisner wrote:

...
On Tue, Sep 25, 2012 at 02:19:16PM +1000, Allan McRae wrote:

...
On 23/09/12 02:17, Dave Reisner wrote:

...
Avoid adding our own messaging, as autoconf will add this for us with the result of the AC_CHECK_FILE test. Reuse the cache variable from autoconf to set our local variable.

Signed-off-by: Dave Reisner --- configure.ac | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/configure.ac b/configure.ac index 7fe696a..7c65a75 100644 --- a/configure.ac +++ b/configure.ac @@ -424,13 +424,9 @@ AC_MSG_CHECKING(whether to use git version if available) if test "x$wantgitver" = "xyes" ; then AC_CHECK_PROGS([GIT], [git]) AC_CHECK_FILE([.git/], hasgitdir=yes) + usegitver=$ac_cv_file__git

That should be:

$ac_cv_file__git_

Interesting that it still appeared to work as intended. Anyways, you're right. Fixed.

Hmm... it did not work for me (which is how I noticed it).

Which brings up the point, do we want to rely on autoconf naming of such variables?

I don't see why not. The variable naming scheme is documented and intended to be used. FWIW, we used to use a cache variable for the purposes of detecting libssl.

Ah - I did not realise that scheme was documented. All is good then.

4224

Age (days ago)

4227

Last active (days ago)

List overview

Download

7 comments

3 participants

participants (3)

Allan McRae
Dave Reisner
Dave Reisner

[pacman-dev] [PATCH 1/3] configure.ac: cleanup duplication in --enable-git-version

tags

participants (3)