After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 609209 - Modernize autotools configuration
Modernize autotools configuration
Status: RESOLVED FIXED
Product: gtksourceview
Classification: Platform
Component: General
2.9.x
Other All
: Normal normal
: ---
Assigned To: GTK Sourceview maintainers
GTK Sourceview maintainers
Depends on: 609250
Blocks:
 
 
Reported: 2010-02-07 04:02 UTC by Javier Jardón (IRC: jjardon)
Modified: 2010-02-23 01:22 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Modernize autotools configuration (8.88 KB, patch)
2010-02-07 04:04 UTC, Javier Jardón (IRC: jjardon)
none Details | Review
Modernize autotools configuration.v2 (7.71 KB, patch)
2010-02-07 16:28 UTC, Javier Jardón (IRC: jjardon)
none Details | Review
Modernize autotools configuration.v3 (7.43 KB, patch)
2010-02-07 16:50 UTC, Javier Jardón (IRC: jjardon)
none Details | Review
Modernize autotools configuration.v4 (6.93 KB, patch)
2010-02-22 21:59 UTC, Javier Jardón (IRC: jjardon)
committed Details | Review

Description Javier Jardón (IRC: jjardon) 2010-02-07 04:02:43 UTC
See http://live.gnome.org/GnomeGoals/ModernAutotools
Comment 1 Javier Jardón (IRC: jjardon) 2010-02-07 04:04:19 UTC
Created attachment 153183 [details] [review]
Modernize autotools configuration

New requirements:

autoconf >= 2.63.2
automake >= 1.10
libtool >= 2.2.6
intltool >= 0.40.0
gtk-doc >= 1.11
Comment 2 Paolo Borelli 2010-02-07 08:59:05 UTC
looks like a libpeas patch... wrong attachment?
Comment 3 Javier Jardón (IRC: jjardon) 2010-02-07 16:28:26 UTC
Created attachment 153203 [details] [review]
Modernize autotools configuration.v2

Ooops, sorry. Here the correct patch
Comment 4 Paolo Borelli 2010-02-07 16:40:42 UTC
Review of attachment 153203 [details] [review]:

::: configure.ac
@@ +18,3 @@
 m4_ifdef([AM_SILENT_RULES],[AM_SILENT_RULES([yes])])
 
+# Checks for programs

(in all the comments) why "checks" instead of "check" like it was before? "check" is more correct english...

@@ -97,1 +85,5 @@
 
+# test-widget uses this to find lang files and gtksourcebuffer.c
+ABS_TOP_SRCDIR=`cd $srcdir && pwd`
+AC_SUBST(ABS_TOP_SRCDIR)
+

why did you change the order? I prefer to keep this defines that are just for tests at the end of the file

@@ +94,3 @@
+# Compiler warnings
+GNOME_COMPILE_WARNINGS([maximum])
+#GNOME_COMPILER_MODE_DEFINES

this is duplicated, you alerady added it before as far as I can see
Comment 5 Javier Jardón (IRC: jjardon) 2010-02-07 16:50:39 UTC
Created attachment 153210 [details] [review]
Modernize autotools configuration.v3
Comment 6 Javier Jardón (IRC: jjardon) 2010-02-07 16:52:59 UTC
Ok, third round.
Sorry for the incorrect english in "check": blame autoscan for it ;)
Comment 7 Paolo Borelli 2010-02-22 20:17:07 UTC
Review of attachment 153210 [details] [review]:

I was about to commit this for todays release, but after a closer look I do not understand some of the changes made to configure.ac

::: configure.ac
@@ -21,3 @@
 m4_ifdef([AM_SILENT_RULES],[AM_SILENT_RULES([yes])])
 
-AC_ISC_POSIX

why is this removed?

@@ -24,3 @@
 AC_PROG_CC
 AC_PROG_INSTALL
-AC_DISABLE_STATIC

why this is removed?

@@ +30,3 @@
+
+# Check for typedefs, structures, and compiler characteristics.
+AC_C_INLINE

why are you adding this check? I do not see it in the original

@@ +34,3 @@
+# Check for library functions.
+AC_FUNC_MALLOC
+AC_FUNC_REALLOC

why are you adding these checks? I do not see them in the original
Comment 8 Paolo Borelli 2010-02-22 20:17:35 UTC
Review of attachment 153210 [details] [review]:

I was about to commit this for todays release, but after a closer look I do not understand some of the changes made to configure.ac

::: configure.ac
@@ -21,3 @@
 m4_ifdef([AM_SILENT_RULES],[AM_SILENT_RULES([yes])])
 
-AC_ISC_POSIX

why is this removed?

@@ -24,3 @@
 AC_PROG_CC
 AC_PROG_INSTALL
-AC_DISABLE_STATIC

why this is removed?

@@ +30,3 @@
+
+# Check for typedefs, structures, and compiler characteristics.
+AC_C_INLINE

why are you adding this check? I do not see it in the original

@@ +34,3 @@
+# Check for library functions.
+AC_FUNC_MALLOC
+AC_FUNC_REALLOC

why are you adding these checks? I do not see them in the original
Comment 9 Paolo Borelli 2010-02-22 20:18:37 UTC
(no idea why it decided to post it twice...)
Comment 10 Javier Jardón (IRC: jjardon) 2010-02-22 21:17:26 UTC
(In reply to comment #7)
> I was about to commit this for todays release, but after a closer look I do not
> understand some of the changes made to configure.ac
> 
> ::: configure.ac
> @@ -21,3 @@
>  m4_ifdef([AM_SILENT_RULES],[AM_SILENT_RULES([yes])])
> 
> -AC_ISC_POSIX
> 
> why is this removed?

From autoconf manual:
"This macro adds -lcposix to output variable LIBS if necessary for Posix facilities. Sun dropped support for the obsolete interactive Systems Corporation Unix on 2006-07-23. New programs need not use this macro"

> @@ -24,3 @@
>  AC_PROG_CC
>  AC_PROG_INSTALL
> -AC_DISABLE_STATIC
> 
> why this is removed?

This is now: (new libtool syntax)
LT_INIT([disable-static])

 
> @@ +30,3 @@
> +
> +# Check for typedefs, structures, and compiler characteristics.
> +AC_C_INLINE
> 
> why are you adding this check? I do not see it in the original
> 
> @@ +34,3 @@
> +# Check for library functions.
> +AC_FUNC_MALLOC
> +AC_FUNC_REALLOC
> 
> why are you adding these checks? I do not see them in the original

These checks were added by autoscan tool.
You are using malloc an realloc functions in your code.

Anyway, I think these checks were not needed if you dont have a fallback for it in the code, so I'll do a new patch rigth now
Comment 11 Javier Jardón (IRC: jjardon) 2010-02-22 21:59:25 UTC
Created attachment 154448 [details] [review]
Modernize autotools configuration.v4

New requirements:
    autoconf >= 2.64
    automake >= 1.11.1
    libtool >= 2.2.6
    intltool >= 0.41.0
    gtk-doc >= 1.11

See the GnomeGoal page for info about the versions used
Comment 12 Paolo Borelli 2010-02-22 22:02:27 UTC
Review of attachment 154448 [details] [review]:

Thanks! please commit and sorry for being so picky in the previous reviews ;)
Comment 13 Javier Jardón (IRC: jjardon) 2010-02-23 01:15:14 UTC
Comment on attachment 154448 [details] [review]
Modernize autotools configuration.v4

commit 57b3de501d74725f756d077e810910ca98e30492
Comment 14 Javier Jardón (IRC: jjardon) 2010-02-23 01:22:32 UTC
(In reply to comment #12)
> Review of attachment 154448 [details] [review]:
> 
> Thanks! please commit and sorry for being so picky in the previous reviews ;)

No problem at all ;)

Indeed, thank you for the review