Re: [PATCH] automake: avoid harmful directory change before invoking valac

[Stefano Lattarini]

On 12/22/2014 01:36 PM, Yanko Kaneti wrote:

On Mon, 2014-12-22 at 12:05 +0100, Stefano Lattarini wrote:

On 12/22/2014 11:32 AM, Yanko Kaneti wrote:

On Fri, 2014-12-19 at 22:01 +0100, Stefano Lattarini wrote:

Hi, thanks for the patch, and sorry for the delay.

On 12/02/2014 02:22 PM, Yanko Kaneti wrote:

The current am__cd right before invoking valac invalidates
any relative flags setup done before that.

https://bugzilla.gnome.org/show_bug.cgi?id=740825

   
BTW, this was already reported as http://debbugs.gnu.org/13002
So finally fixing it would likely be a good idea.


---
bin/automake.in | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)


Such a fix requires at least a new test case and a NEWS entry.
Care to take at stab at writing them?


Sorry,
I looked a bit in the t/ directory but my eyes glazed over.
Perhaps I am not the best person to attempt those.


It's OK if you can just post the snippets of relevant vala sources
and build options exposing the problem, and describe the steps to
reproduce such a problem.  I can synthesize a test case from there
myself then.


It was exposed by a builddir != srcdir problem in gsound in gnome
git https://git.gnome.org/browse/gsound/ (linked bugreport above)
where a test program (gsound-play.vala) needs the gsound vapis which
are also a build product.

The way to reproduce it is:

- autogen gsound with the workaround (4f8a78c) removed
- mkdir _build  cd _build
- ../configure
- notice how the gsound-play compile fails  in the _srcdir_ (because
of the the am__cd)  but its arguments are trying to use ../gsound
instead of the builddir gsound

Hope that helps.
Yanko


Since I know almost nothing about Vala, and I'm short of time these
days, I don't think I will go perusing into a third-party project
to find a test case; a minimal reproducer in form of Vala sources,
a Makefile.am fragment, and an high-level description of the expected
behavior) would be appreciated, and would help in speeding up the
fix.

Thanks,
  Stefano

Re: [PATCH] automake: avoid harmful directory change before invoking valac

[Yanko Kaneti]

On Fri, 2014-12-19 at 22:01 +0100, Stefano Lattarini wrote:
 Hi, thanks for the patch, and sorry for the delay.
 
 On 12/02/2014 02:22 PM, Yanko Kaneti wrote:
  The current am__cd right before invoking valac invalidates
  any relative flags setup done before that.
  
  https://bugzilla.gnome.org/show_bug.cgi?id=740825
  
 BTW, this was already reported as http://debbugs.gnu.org/13002 So 
 finally fixing it would likely be a good idea.
 
  ---
bin/automake.in | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
  
 Such a fix requires at least a new test case and a NEWS entry. Care 
 to take at stab at writing them?

Sorry,
I looked a bit in the t/ directory but my eyes glazed over.
Perhaps I am not the best person to attempt those.

  diff --git a/bin/automake.in b/bin/automake.in
  index 4cee0d0..3082207 100644
  --- a/bin/automake.in
  +++ b/bin/automake.in
  @@ -5476,6 +5476,7 @@ sub lang_vala_finish_target
  my $silent = silent_flag ();
  my $stampfile = \$(srcdir)/${derived}_vala.stamp;
  
  +  map { !/^\//  ($_ = \$(srcdir)\/ . $_) } @vala_sources;
  
 What is the point of the !/^\// conditional?

The idea was to protect from mangling any vala_sources that might 
already be absolute paths. If that's not expected or supported then it 
can be dropped.

\$(srcdir)/${derived}_vala.stamp: @vala_sources\n.
# Since the C files generated from the vala sources depend on the
  @@ -5485,7 +5486,7 @@ sub lang_vala_finish_target
# Thus we need to create the stamp file *before* invoking valac, 
  and to
# move it to its final location only after valac has been 
  invoked.
\t${silent}rm -f \$\@  echo stamp  \$\@-t\n.
  -\t${verbose}\$(am__cd) \$(srcdir)  $compile 
  @vala_sources\n.
  +\t${verbose}$compile -d \$(srcdir) @vala_sources\n.
\t${silent}mv -f \$\@-t \$\@\n;
  
  push_dist_common ($stampfile);
  
 
 Thanks,
Stefano

Regards and sorry for the dalay
Yanko

Re: [PATCH] automake: avoid harmful directory change before invoking valac

[Stefano Lattarini]

On 12/22/2014 11:32 AM, Yanko Kaneti wrote:

On Fri, 2014-12-19 at 22:01 +0100, Stefano Lattarini wrote:

Hi, thanks for the patch, and sorry for the delay.

On 12/02/2014 02:22 PM, Yanko Kaneti wrote:

The current am__cd right before invoking valac invalidates
any relative flags setup done before that.

https://bugzilla.gnome.org/show_bug.cgi?id=740825

  
BTW, this was already reported as http://debbugs.gnu.org/13002 So
finally fixing it would likely be a good idea.


---
   bin/automake.in | 3 ++-
   1 file changed, 2 insertions(+), 1 deletion(-)


Such a fix requires at least a new test case and a NEWS entry. Care
to take at stab at writing them?


Sorry,
I looked a bit in the t/ directory but my eyes glazed over.
Perhaps I am not the best person to attempt those.


It's OK if you can just post the snippets of relevant vala sources
and build options exposing the problem, and describe the steps to
reproduce such a problem.  I can synthesize a test case from there
myself then.


diff --git a/bin/automake.in b/bin/automake.in
index 4cee0d0..3082207 100644
--- a/bin/automake.in
+++ b/bin/automake.in
@@ -5476,6 +5476,7 @@ sub lang_vala_finish_target
 my $silent = silent_flag ();
 my $stampfile = \$(srcdir)/${derived}_vala.stamp;

+  map { !/^\//  ($_ = \$(srcdir)\/ . $_) } @vala_sources;

  
What is the point of the !/^\// conditional?


The idea was to protect from mangling any vala_sources that might
already be absolute paths. If that's not expected or supported then it
can be dropped.


Indeed it is not expected to be supported.

Thanks,
  Stefano

Re: [PATCH] automake: avoid harmful directory change before invoking valac

[Yanko Kaneti]

On Mon, 2014-12-22 at 12:05 +0100, Stefano Lattarini wrote:
 On 12/22/2014 11:32 AM, Yanko Kaneti wrote:
  On Fri, 2014-12-19 at 22:01 +0100, Stefano Lattarini wrote:
   Hi, thanks for the patch, and sorry for the delay.
   
   On 12/02/2014 02:22 PM, Yanko Kaneti wrote:
The current am__cd right before invoking valac invalidates
any relative flags setup done before that.

https://bugzilla.gnome.org/show_bug.cgi?id=740825
 
   BTW, this was already reported as http://debbugs.gnu.org/13002 
   So finally fixing it would likely be a good idea.
   
---
   bin/automake.in | 3 ++-
   1 file changed, 2 insertions(+), 1 deletion(-)

   Such a fix requires at least a new test case and a NEWS entry. 
   Care to take at stab at writing them?
  
  Sorry,
  I looked a bit in the t/ directory but my eyes glazed over.
  Perhaps I am not the best person to attempt those.
  
 It's OK if you can just post the snippets of relevant vala sources 
 and build options exposing the problem, and describe the steps to 
 reproduce such a problem.  I can synthesize a test case from there 
 myself then.

It was exposed by a builddir != srcdir problem in gsound in gnome 
git https://git.gnome.org/browse/gsound/ (linked bugreport above) 
where a test program (gsound-play.vala) needs the gsound vapis which 
are also a build product.

The way to reproduce it is:

- autogen gsound with the workaround (4f8a78c) removed
- mkdir _build  cd _build
- ../configure
- notice how the gsound-play compile fails  in the _srcdir_ (because 
of the the am__cd)  but its arguments are trying to use ../gsound 
instead of the builddir gsound

Hope that helps.
Yanko

Re: [PATCH] automake: avoid harmful directory change before invoking valac

[Stefano Lattarini]

Hi, thanks for the patch, and sorry for the delay.

On 12/02/2014 02:22 PM, Yanko Kaneti wrote:

The current am__cd right before invoking valac invalidates
any relative flags setup done before that.

https://bugzilla.gnome.org/show_bug.cgi?id=740825


BTW, this was already reported as http://debbugs.gnu.org/13002
So finally fixing it would likely be a good idea.


---
  bin/automake.in | 3 ++-
  1 file changed, 2 insertions(+), 1 deletion(-)


Such a fix requires at least a new test case and a NEWS entry.
Care to take at stab at writing them?


diff --git a/bin/automake.in b/bin/automake.in
index 4cee0d0..3082207 100644
--- a/bin/automake.in
+++ b/bin/automake.in
@@ -5476,6 +5476,7 @@ sub lang_vala_finish_target
my $silent = silent_flag ();
my $stampfile = \$(srcdir)/${derived}_vala.stamp;

+  map { !/^\//  ($_ = \$(srcdir)\/ . $_) } @vala_sources;


What is the point of the !/^\// conditional?


$output_rules .=
  \$(srcdir)/${derived}_vala.stamp: @vala_sources\n.
  # Since the C files generated from the vala sources depend on the
@@ -5485,7 +5486,7 @@ sub lang_vala_finish_target
  # Thus we need to create the stamp file *before* invoking valac, and to
  # move it to its final location only after valac has been invoked.
  \t${silent}rm -f \$\@  echo stamp  \$\@-t\n.
-\t${verbose}\$(am__cd) \$(srcdir)  $compile @vala_sources\n.
+\t${verbose}$compile -d \$(srcdir) @vala_sources\n.
  \t${silent}mv -f \$\@-t \$\@\n;

push_dist_common ($stampfile);



Thanks,
  Stefano