bug#27188: lex/yacc with subdir-objects and --disable-dependency-tracking broken

classic Classic list List threaded Threaded
4 messages Options
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

bug#27188: lex/yacc with subdir-objects and --disable-dependency-tracking broken

Nick Brown
By default Debian dh passes --disable-dependacy-tracking when building.
If the package rules also adds --builddirectory to use out of source builds
(eg. "dh $@ --builddirectory=build --parallel --with autoreconf"), and the
package happens to be using subdir-objects automake option and generated source
files from lex/yacc then the build will fail.

Eg.

mkdir build
../configure --disable-dependency-tracking --disable-silent-rules
make

/bin/bash ../ylwrap ../src/scanner.ll lex.yy.c src/scanner.cc -- flex
../ylwrap: line 206: ../src/scanner.cc: No such file or directory
Makefile:422: recipe for target 'src/scanner.cc' failed
make: *** [src/scanner.cc] Error 1

A simple patch to lex.am and yacc.am fixes this.

0001-yacc-and-lex-built-objects-without-dependancy-tracking.patch (1K) Download Attachment
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

bug#27188: lex/yacc with subdir-objects and --disable-dependency-tracking broken

Mathieu Lirzin
Hello,

Nick Brown <[hidden email]> writes:

> By default Debian dh passes --disable-dependacy-tracking when building.
> If the package rules also adds --builddirectory to use out of source builds
> (eg. "dh $@ --builddirectory=build --parallel --with autoreconf"), and the
> package happens to be using subdir-objects automake option and generated source
> files from lex/yacc then the build will fail.
>
> Eg.
>
> mkdir build
> ../configure --disable-dependency-tracking --disable-silent-rules
> make
>
> /bin/bash ../ylwrap ../src/scanner.ll lex.yy.c src/scanner.cc -- flex
> ../ylwrap: line 206: ../src/scanner.cc: No such file or directory
> Makefile:422: recipe for target 'src/scanner.cc' failed
> make: *** [src/scanner.cc] Error 1

This seems like a known bug since 2011, that would be nice to fix.
There is already an "expected fail" test case for the issue you have
described.  The commit 40c34328d1e5d3ab6885f046ce27517332413c13 which
has intentionaly make the test fail has the following ChangeLog:

+2011-10-20  Stefano Lattarini  <[hidden email]>
+
+ deps: partially revert commit `v1.11-512-geeee551'
+ This change partly reverts commit "Create subdirs for generated
+ sources even when not dep tracking", of 2011-04-02.
+ That commit had caused the bugs #8485 and #8526.  Since we are
+ nearing the bug-fixing automake release 1.11.2, the safest policy
+ at the moment is to just revert the problematic hunks: an older,
+ known bug is better than a regression.
+ * automake.in (handle_single_transform): Don't add a dirstamp
+ dependency, even when $object is derived and lands in a subdir.
+ * tests/Makefile.am (XFAIL_TESTS): Add lex-subobj-nodep.test,
+ remove yacc-dist-nobuild-subdir.test.

This seems to imply that this has not been fixed because of time
constraints before a release.  Can you investigate more to see if your
patch doesn't reintroduce problems from bugs #8485 and #8526?

>
> A simple patch to lex.am and yacc.am fixes this.
>
>>From bd6971224f304c8f5951afca620f33c25248b446 Mon Sep 17 00:00:00 2001
> From: Nicholas Brown <[hidden email]>
> Date: Fri, 4 Dec 2015 10:49:18 +0000
> Subject: [PATCH] yacc and lex built objects without dependancy tracking
>
> ---
>  lib/am/lex.am  | 1 +
>  lib/am/yacc.am | 1 +
>  2 files changed, 2 insertions(+)
>
> diff --git a/lib/am/lex.am b/lib/am/lex.am
> index d7ddc77..6357507 100644
> --- a/lib/am/lex.am
> +++ b/lib/am/lex.am
> @@ -23,6 +23,7 @@ endif %?MAINTAINER-MODE%
>  
>  ?GENERIC?%EXT%%DERIVED-EXT%:
>  ?!GENERIC?%OBJ%: %SOURCE%
> +?SUBDIROBJ? %SILENT%test -d $(dir $@) || $(MKDIR_P) $(dir $@)

I suspect the '$(dir ..)' syntax is not portable.  Hopefully there
should be a alternative.  Can you look into it?

>  ?GENERIC? %VERBOSE%$(am__skiplex) $(SHELL) $(YLWRAP) %SOURCE% $(LEX_OUTPUT_ROOT).c %OBJ% -- %COMPILE%
>  ?!GENERIC? %VERBOSE% \
>  ?!GENERIC??DIST_SOURCE? $(am__skiplex) \
> diff --git a/lib/am/yacc.am b/lib/am/yacc.am
> index 2b3f92a..2c33023 100644
> --- a/lib/am/yacc.am
> +++ b/lib/am/yacc.am
> @@ -43,6 +43,7 @@ endif %?FIRST%
>  
>  ?GENERIC?%EXT%%DERIVED-EXT%:
>  ?!GENERIC?%OBJ%: %SOURCE%
> +?SUBDIROBJ? %SILENT%test -d $(dir $@) || $(MKDIR_P) $(dir $@)
>  ?GENERIC? %VERBOSE%$(am__skipyacc) $(SHELL) $(YLWRAP) %SOURCE% y.tab.c %OBJ% y.tab.h `echo %OBJ% | $(am__yacc_c2h)` y.output %BASE%.output -- %COMPILE%
>  ?!GENERIC? %VERBOSE% \
>  ?!GENERIC??DIST_SOURCE? $(am__skipyacc) \

Thank you for you patch.

--
Mathieu Lirzin
GPG: F2A3 8D7E EB2B 6640 5761  070D 0ADE E100 9460 4D37



Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

bug#27188: lex/yacc with subdir-objects and --disable-dependency-tracking broken

Nick Bowler
On 6/4/17, Mathieu Lirzin <[hidden email]> wrote:

> Nick Brown <[hidden email]> writes:
>> diff --git a/lib/am/lex.am b/lib/am/lex.am
>> index d7ddc77..6357507 100644
>> --- a/lib/am/lex.am
>> +++ b/lib/am/lex.am
>> @@ -23,6 +23,7 @@ endif %?MAINTAINER-MODE%
>>
>>  ?GENERIC?%EXT%%DERIVED-EXT%:
>>  ?!GENERIC?%OBJ%: %SOURCE%
>> +?SUBDIROBJ? %SILENT%test -d $(dir $@) || $(MKDIR_P) $(dir $@)
>
> I suspect the '$(dir ..)' syntax is not portable.  Hopefully there
> should be a alternative.  Can you look into it?

$(dir ...) is most definitely not portable.  One alternative is to use
$(@D), which is specified in POSIX and essentially works in every make
implementation that I know of.

However, there are still portability gotchas.  At least one implementation
(dmake) supports $(@D) in principle but expands it in a not-quite-POSIX-
compliant way.

POSIX says that the expansion of $(@D) (and similar variables) does not
include a trailing slash and expands to .  for the current directory
(i.e., when the target name does not contain a slash).

In dmake, the expansion of $(@D) (and similar variables) for the current
directory is the empty string, otherwise the expansion contains a trailing
slash.

In cases where the difference matters, this can be worked around in the
shell easily enough.  The difference matters if we adapt the above example
because mkdir with an empty string will fail.  Something like this should
be pretty portable (untested):

  test x"$(@D)" = x || $(MKDIR_P) "$(@D)"

Another way, perhaps even more portable, would be to do the splitting
entirely in the shell, e.g., by using expr.

Cheers,
  Nick



Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

bug#27188: lex/yacc with subdir-objects and --disable-dependency-tracking broken

Mathieu Lirzin
Hello Nick,

Nick Bowler <[hidden email]> writes:

> On 6/4/17, Mathieu Lirzin <[hidden email]> wrote:
>> Nick Brown <[hidden email]> writes:
>>> diff --git a/lib/am/lex.am b/lib/am/lex.am
>>> index d7ddc77..6357507 100644
>>> --- a/lib/am/lex.am
>>> +++ b/lib/am/lex.am
>>> @@ -23,6 +23,7 @@ endif %?MAINTAINER-MODE%
>>>
>>>  ?GENERIC?%EXT%%DERIVED-EXT%:
>>>  ?!GENERIC?%OBJ%: %SOURCE%
>>> +?SUBDIROBJ? %SILENT%test -d $(dir $@) || $(MKDIR_P) $(dir $@)
>>
>> I suspect the '$(dir ..)' syntax is not portable.  Hopefully there
>> should be a alternative.  Can you look into it?
>
> $(dir ...) is most definitely not portable.  One alternative is to use
> $(@D), which is specified in POSIX and essentially works in every make
> implementation that I know of.
>
> However, there are still portability gotchas.  At least one implementation
> (dmake) supports $(@D) in principle but expands it in a not-quite-POSIX-
> compliant way.
>
> POSIX says that the expansion of $(@D) (and similar variables) does not
> include a trailing slash and expands to .  for the current directory
> (i.e., when the target name does not contain a slash).
>
> In dmake, the expansion of $(@D) (and similar variables) for the current
> directory is the empty string, otherwise the expansion contains a trailing
> slash.
>
> In cases where the difference matters, this can be worked around in the
> shell easily enough.  The difference matters if we adapt the above example
> because mkdir with an empty string will fail.  Something like this should
> be pretty portable (untested):
>
>   test x"$(@D)" = x || $(MKDIR_P) "$(@D)"
>
> Another way, perhaps even more portable, would be to do the splitting
> entirely in the shell, e.g., by using expr.

I have searched for similar MKDIR_P constructs in the lib/am files to
know what is the current practice, but I haven't found any.  I think it
might be reasonable to use $(@D) fornow.  In fact Automake is using it
in its own Makefile (see "bin/Automake.inc").

Besides the portability issue, we need to figure out if this bug-fix
doesn't bring some regressions (as suggested by the commit log sample of
my previous email).  I have run the test suite with the changes you have
suggested, and the test suite doesn't complain except for the "XPASS"
concerning the "t/lex-subobj-nodep.sh" test (the one I was refering too
in my previous email).  So this seems to look good, but this isn't
enough to commit it confidently.

I have discovered that this bug is a duplicate of #9859 for which you
have already sent a patch 2 year ago.  :)

Thank you for investigating.

--
Mathieu Lirzin
GPG: F2A3 8D7E EB2B 6640 5761  070D 0ADE E100 9460 4D37



Loading...