#31 shell variable assignments generate invalid deps

v2.0
closed-fixed
nobody
None
5
2014-06-18
2012-12-09
No

In the following makefile, which is adapted from a similar fragment in something of mine:

---
x: y z
a=l; cat y z > x
y:
touch y
z:
touch z
l.c:
echo '#include "x"' > l.c
l.o: x
---

The result of "make" is correct:

---
touch y
touch z
a=l; cat y z > x
---

makepp generates an invalid dependency (x: l), which in turn creates a circular dependency (x:l; l:l.o; l.o: x). Behavior of makepp in the original makefile varied (sometimes built l.o first, and sometimes bult y first, then x (but not z). Behavior for this makefile:

---
makepp: Loading makefile `.../makefile'
makepp: Entering directory `...'
echo '#include "x"' > l.c
makepp: Scanning `.../l.c'
makepp: warning: could not read .../x to scan for includes--No such file or directory
gcc -g -Wall -c l.c -o l.o
l.c:1:13: fatal error: x: No such file or directory
compilation terminated.
makepp: Failed to build target `.../l.o' [1]
makepp: 1 file updated, 0 phony targets built and 1 target failed
---

To correct, I recommend skipping past value instead of leaving loop when scanning for var assignments (Lexer.pm:220):
$ix >= 0 or $ix = length $command; # Can't find the end

After this change, makepp does the right thing:
---
makepp: Loading makefile `.../makefile'
makepp: Entering directory `...'
touch y
touch z
a=l; cat y z > x
makepp: 3 files updated and 0 phony targets built
---

Discussion

  • Daniel Pfeiffer

    Daniel Pfeiffer - 2012-12-09

    Very strange, what platform and Perl are you on? On Linux both 2.0 and 2.0.98.2 give me the expected:

    makepp: Loading makefile `.../Makeppfile'
    makepp: Entering directory `...'
    touch y
    touch z
    a=l; cat y z > x
    makepp: 3 files updated and 0 phony targets built

    Can I have your .makepp/log or "mppl -c" output after this goes wrong (even though I don't expect it shed much light...)

    As for your fix, we're operating separately on each part returned by "split_commands $action", i.e last is only giving up on "a=l". That's correct, since there's nothing useful to be found out about that assignment. What this code is looking for is "a=l cmd-that-looks-at-env-a", which is a not so well known way of doing single command env vars.

     
  • Thomas Moore

    Thomas Moore - 2012-12-09

    I see. Do you have space between the variable assignment and the semicolon? I never do, and it appears that makepp doesn't trim whitespace, so having an explicit space following the variable assignment does not trigger the bug. That is, it works correctly if I use "a=l ; ...", but not if I do "a=l; ...".

    In any case, as you requested, I have attached the output of mppl -c. At least that should answer your questions about platform and perl version, although I first encountered this bug (but didn't realize what the root cause was) several years ago with an older version of perl and on AIX.

    As to my fix: it was a suggestion, only. Feel free to ignore and fix it correctly (I am not familiar with the code; I just don't like posting bug reports without having at least something like a working fix). I'm not sure what you mean by "not-so-well-known", since I use temporary env assignments all the time. I know what the code is trying to do, I'm just saying it's doing it wrong. As it stands, the code leaves the loop with the value of the last variable assignment as the command if it can't find any whitespace following the variable assignment. I think the correct behavior would be to abandon the entire parse and return no command if there is any kind of parse error at all, but I didn't look at the code long enough to know how to do that correctly.

     
  • Thomas Moore

    Thomas Moore - 2012-12-09

    Output from mppl -c after failed makepp

     
  • Daniel Pfeiffer

    Daniel Pfeiffer - 2013-02-11

    Hi Thomas,

    I'm feeling bad about this, because you did exactly what programmers dream of: reduce the problem to a small example. Alas it must be something about your environment, and I don't understand what. There shouldn't be a way to make the Lexer misbehave like you observed...

    The only solution I see, is for you to debug the Lexer and tell me exactly where it is choosing 'l' as a dependency, and then go back and see where it took the wrong branch to get there. Alternately if you can give me remote login to a machine where this is reproducible, I could do that myself.

    regards – Daniel

     
  • Daniel Pfeiffer

    Daniel Pfeiffer - 2013-03-10
    • status: open --> pending
     
  • Daniel Pfeiffer

    Daniel Pfeiffer - 2014-06-18
    • status: pending --> closed-fixed
    • Group: --> v1.0_(example)
     
  • Daniel Pfeiffer

    Daniel Pfeiffer - 2014-06-18

    Version 2.0 handles this fine.

     
  • Daniel Pfeiffer

    Daniel Pfeiffer - 2014-06-18
    • Group: v1.0_(example) --> v2.0
     

Log in to post a comment.