all messages for Guix-related lists mirrored at yhetil.org
 help / color / mirror / code / Atom feed
* Re: 01/01: gnu: Add agg.
       [not found] ` <E1YYBc3-0000d6-3W@vcs.savannah.gnu.org>
@ 2015-03-18 13:09   ` Ludovic Courtès
  2015-03-18 13:47     ` Tomáš Čech
  0 siblings, 1 reply; 5+ messages in thread
From: Ludovic Courtès @ 2015-03-18 13:09 UTC (permalink / raw)
  To: Tomáš Čech; +Cc: guix-devel

Tomas Cech <sleep_walker@suse.cz> skribis:

> commit 87bafa07ae68084819bb2648bd1042c354d11317
> Author: Tomáš Čech <sleep_walker@gnu.org>
> Date:   Wed Mar 18 08:57:50 2015 +0100
>
>     gnu: Add agg.
>     
>     * gnu/packages/graphics.scm (agg): New variable.
>     * gnu/packages/patches/agg-am_c_prototype.patch: New file.
>     * gnu-system.am (dist_patch_DATA): Add it.

[...]

> +++ b/gnu/packages/patches/agg-am_c_prototype.patch
> @@ -0,0 +1,10 @@
> +--- a/configure.in	2015-03-15 18:59:12.557417149 +0100
> ++++ b/configure.in	2015-03-15 18:59:29.273416518 +0100

Patches must have a comment at the beginning to explain what they do,
why, and what the upstream status is.

Please pay more attention to these rules before pushing.  In general, I
would recommend that you wait for some review before pushing.  (This is
sometimes slow, but hopefully others will help reduce latency,
hint hint! ;-))

> +@@ -8,7 +8,6 @@
> + AC_PROG_CC
> + AC_PROG_CXX
> + AC_ISC_POSIX
> +-AM_C_PROTOTYPES
> + if test "x$U" != "x"; then
> +   AC_MSG_ERROR(Compiler not ANSI compliant)
> + fi

It is best to avoid modifying .ac or .am files because that requires
adding a dependency on the autotools.

What problem does this patch solve?  I’m confident this can be worked
around without modifying the .ac file, for instance by passing the right
ac_cv_ variable as a ‘configure’ argument.

Thanks,
Ludo’.

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: 01/01: gnu: Add agg.
  2015-03-18 13:09   ` 01/01: gnu: Add agg Ludovic Courtès
@ 2015-03-18 13:47     ` Tomáš Čech
  2015-03-18 20:40       ` Ludovic Courtès
  0 siblings, 1 reply; 5+ messages in thread
From: Tomáš Čech @ 2015-03-18 13:47 UTC (permalink / raw)
  To: guix-devel

[-- Attachment #1: Type: text/plain, Size: 2646 bytes --]

On Wed, Mar 18, 2015 at 02:09:49PM +0100, Ludovic Courtès wrote:
>Tomas Cech <sleep_walker@suse.cz> skribis:
>
>> commit 87bafa07ae68084819bb2648bd1042c354d11317
>> Author: Tomáš Čech <sleep_walker@gnu.org>
>> Date:   Wed Mar 18 08:57:50 2015 +0100
>>
>>     gnu: Add agg.
>>
>>     * gnu/packages/graphics.scm (agg): New variable.
>>     * gnu/packages/patches/agg-am_c_prototype.patch: New file.
>>     * gnu-system.am (dist_patch_DATA): Add it.
>
>[...]
>
>> +++ b/gnu/packages/patches/agg-am_c_prototype.patch
>> @@ -0,0 +1,10 @@
>> +--- a/configure.in	2015-03-15 18:59:12.557417149 +0100
>> ++++ b/configure.in	2015-03-15 18:59:29.273416518 +0100
>
>Patches must have a comment at the beginning to explain what they do,
>why, and what the upstream status is.

Will do.

>Please pay more attention to these rules before pushing.  In general, I
>would recommend that you wait for some review before pushing.  (This is
>sometimes slow, but hopefully others will help reduce latency,
>hint hint! ;-))

Yeah, I can see what I'm causing lately. I'll relax with pushing.

>
>> +@@ -8,7 +8,6 @@
>> + AC_PROG_CC
>> + AC_PROG_CXX
>> + AC_ISC_POSIX
>> +-AM_C_PROTOTYPES
>> + if test "x$U" != "x"; then
>> +   AC_MSG_ERROR(Compiler not ANSI compliant)
>> + fi
>
>It is best to avoid modifying .ac or .am files because that requires
>adding a dependency on the autotools.]

I will remember that but I'm afraid that in this case there is no
configure in tarball anyway and has to be generated.

>What problem does this patch solve?  I’m confident this can be worked
>around without modifying the .ac file, for instance by passing the right
>ac_cv_ variable as a ‘configure’ argument.


AM_C_PROTOTYPES has been removed from automake.

https://lists.gnu.org/archive/html/automake-patches/2011-06/msg00082.html

and the reconfigure leads to failure:

starting phase `autoreconf'
aclocal: warning: autoconf input should be named 'configure.ac', not 'configure.in'
configure.in:11: error: automatic de-ANSI-fication support has been removed
/gnu/store/2h4lnk22bj479fnr0l2cyx2rvisv9cg7-automake-1.15/share/aclocal-1.15/obsolete.m4:26: AM_C_PROTOTYPES is expanded from...
configure.in:11: the top level

I'd gladly take your recommendation about passing 'the right ac_cv_
variable' on this.

The agg package is old, last it's release was 8 years ago, sourceforge
marked that as '(historical)'. It is unlikely that anyone would accept
patch.

The only reason I spent time on it is because it is required
dependency for GNU Gnash.


Thanks for review.

Best regards,

S_W

[-- Attachment #2: Type: application/pgp-signature, Size: 181 bytes --]

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: 01/01: gnu: Add agg.
  2015-03-18 13:47     ` Tomáš Čech
@ 2015-03-18 20:40       ` Ludovic Courtès
  2015-03-18 22:08         ` [PATCH] gnu: agg: Add explanatory comment to the patch Tomáš Čech
  0 siblings, 1 reply; 5+ messages in thread
From: Ludovic Courtès @ 2015-03-18 20:40 UTC (permalink / raw)
  To: guix-devel

Tomáš Čech <sleep_walker@gnu.org> skribis:

> On Wed, Mar 18, 2015 at 02:09:49PM +0100, Ludovic Courtès wrote:
>>Tomas Cech <sleep_walker@suse.cz> skribis:

[...]

>>> +@@ -8,7 +8,6 @@
>>> + AC_PROG_CC
>>> + AC_PROG_CXX
>>> + AC_ISC_POSIX
>>> +-AM_C_PROTOTYPES
>>> + if test "x$U" != "x"; then
>>> +   AC_MSG_ERROR(Compiler not ANSI compliant)
>>> + fi
>>
>>It is best to avoid modifying .ac or .am files because that requires
>>adding a dependency on the autotools.]
>
> I will remember that but I'm afraid that in this case there is no
> configure in tarball anyway and has to be generated.

Oh, right, sorry for the confusion!  If that has to be generated anyway,
then fine, no problem.

>>What problem does this patch solve?  I’m confident this can be worked
>>around without modifying the .ac file, for instance by passing the right
>>ac_cv_ variable as a ‘configure’ argument.
>
>
> AM_C_PROTOTYPES has been removed from automake.
>
> https://lists.gnu.org/archive/html/automake-patches/2011-06/msg00082.html

Right, so you made the right decision.  Just mention it in the patch.

Thank you!

Ludo’.

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH] gnu: agg: Add explanatory comment to the patch.
  2015-03-18 20:40       ` Ludovic Courtès
@ 2015-03-18 22:08         ` Tomáš Čech
  2015-03-19 16:02           ` Ludovic Courtès
  0 siblings, 1 reply; 5+ messages in thread
From: Tomáš Čech @ 2015-03-18 22:08 UTC (permalink / raw)
  To: guix-devel

* gnu/packages/patches/agg-am_c_prototype.patch: Add comment.
---
 gnu/packages/patches/agg-am_c_prototype.patch | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/gnu/packages/patches/agg-am_c_prototype.patch b/gnu/packages/patches/agg-am_c_prototype.patch
index 3a09dc7..b31e86c 100644
--- a/gnu/packages/patches/agg-am_c_prototype.patch
+++ b/gnu/packages/patches/agg-am_c_prototype.patch
@@ -1,3 +1,17 @@
+AM_C_PROTOTYPES is no longer supported.
+
+https://lists.gnu.org/archive/html/automake-patches/2011-06/msg00082.html
+
+Failure without patch:
+
+starting phase `autoreconf'
+aclocal: warning: autoconf input should be named 'configure.ac', not 'configure.in'
+configure.in:11: error: automatic de-ANSI-fication support has been removed
+/gnu/store/…-automake-1.15/share/aclocal-1.15/obsolete.m4:26: AM_C_PROTOTYPES is expanded from...
+configure.in:11: the top level
+
+
+Upstream is dead.
 --- a/configure.in	2015-03-15 18:59:12.557417149 +0100
 +++ b/configure.in	2015-03-15 18:59:29.273416518 +0100
 @@ -8,7 +8,6 @@
-- 
2.2.1

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH] gnu: agg: Add explanatory comment to the patch.
  2015-03-18 22:08         ` [PATCH] gnu: agg: Add explanatory comment to the patch Tomáš Čech
@ 2015-03-19 16:02           ` Ludovic Courtès
  0 siblings, 0 replies; 5+ messages in thread
From: Ludovic Courtès @ 2015-03-19 16:02 UTC (permalink / raw)
  To: Tomáš Čech; +Cc: guix-devel

Tomáš Čech <sleep_walker@gnu.org> skribis:

> * gnu/packages/patches/agg-am_c_prototype.patch: Add comment.

Perfect, please push.  Thanks!

Ludo’.

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2015-03-19 16:02 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <20150318105410.2359.97929@vcs.savannah.gnu.org>
     [not found] ` <E1YYBc3-0000d6-3W@vcs.savannah.gnu.org>
2015-03-18 13:09   ` 01/01: gnu: Add agg Ludovic Courtès
2015-03-18 13:47     ` Tomáš Čech
2015-03-18 20:40       ` Ludovic Courtès
2015-03-18 22:08         ` [PATCH] gnu: agg: Add explanatory comment to the patch Tomáš Čech
2015-03-19 16:02           ` Ludovic Courtès

Code repositories for project(s) associated with this external index

	https://git.savannah.gnu.org/cgit/guix.git

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.