search

gaoyingie / gyp

Automatically exported from code.google.com/p/gyp
0 stars 0 forks source link

make -j > 1 non-deterministic when action output changes (unless it's the first output) #318

Open GoogleCodeExporter opened 9 years ago

GoogleCodeExporter commented 9 years ago 
When an action output changes, make (with -j > 1) does not trigger a 
recompilation of the output (unless it's the first output listed in the 
'outputs'). When you run make the second time, it sees the changed output and 
compiles. make -j1 works fine, and ninja doesn't have this problem. I have GNU 
Make 3.81.

One other interesting thing I noticed is that when the non-first action output 
changes once, every time afterwards make is run, it will spit out ACTION ...

I ran into this problem in chromium with webkit's 
inspector_protocol_sources/webcore_bindings targets.

====simple repro====

1) Run gyp on all.gyp listed below (with the make generator).
2) make -j2 test
3) edit gen.py to generate a different 'two.cc' (e.g., by modifying testme2)
4) make -j2 test

all.gyp:
{
  'targets': [
    {
      'target_name': 'test',
      'type': 'static_library',
      'sources': [
        'one.cc',
        'two.cc',
      ],
      'actions': [
        {
          'action_name': 'gen',
          'inputs': [
            'gen.py',
          ],
          'outputs': [
            'one.cc',
            'two.cc',
          ],
          'action': [
            'python',
            '<@(_inputs)',
          ],
          'message': 'Generating',
        },
      ],
    },
  ],
}

gen.py:
class SmartOutput(object):
    def __init__(self, file_name):
        self.file_name_ = file_name
        self.output_ = ""

    def write(self, text):
        self.output_ += text

    def close(self):
        read_file = open(self.file_name_, "r")
        old_text = read_file.read()
        read_file.close()
        text_changed = old_text != self.output_

        if text_changed:
            out_file = open(self.file_name_, "w")
            out_file.write(self.output_)
            out_file.close()

f = SmartOutput('one.cc')
f.write('int testme1() { return 2; }')
f.close()

f = SmartOutput('two.cc')
f.write('int testme2() { return 5; }')
f.close()

Original issue reported on code.google.com by kkania@chromium.org on 18 Jan 2013 at 5:25

GoogleCodeExporter commented 9 years ago

Original comment by kkania@chromium.org on 18 Jan 2013 at 5:32

GoogleCodeExporter commented 9 years ago 
The current make generator will generate a makefile similar to this for the 
multi-output action:

one.cc: gen.py
    python gen.py
two.cc: one.cc
one.o: one.cc
    g++ -c one.cc
two.o: two.cc
    g++ -c two.cc
.PHONY: test
test: one.o two.o

This doesn't work if two.cc changes from gen.py but one.cc doesn't. The 
simplest way to fix this is to change
two.cc: one.cc
  ->
two.cc: one.cc FORCE_DO_CMD ;

--- a/pylib/gyp/generator/make.py
+++ b/pylib/gyp/generator/make.py
@@ -1704,7 +1704,7 @@ $(obj).$(TOOLSET)/$(TARGET)/%%.o: $(obj)/%%%s FORCE_DO_CMD
       # think these outputs haven't (couldn't have?) changed, and thus doesn't
       # flag them as changed (i.e. include in '$?') when evaluating dependent
       # rules, which in turn causes do_cmd() to skip running dependent commands.
-      self.WriteLn('%s: ;' % (' '.join(outputs[1:])))
+      self.WriteLn('%s: FORCE_DO_CMD ;' % (' '.join(outputs[1:])))
     self.WriteLn()

Technically, it only needs to be an order-only prerequisite, so we could do:
two.cc: FORCE_DO_CMD | one.cc ;

Original comment by kkania@chromium.org on 21 Jan 2013 at 6:48

GoogleCodeExporter commented 9 years ago 
The above patch still doesn't fix the minor issue that the action is always 
performed if gen.py is newer than one.cc. To fix that, we could so something 
like:

action: gen.py
    python gen.py && touch action
one.cc: action ;
two.cc: action ;
one.o: one.cc
    g++ -c one.cc
two.o: two.cc
    g++ -c two.cc
.PHONY: test
test: one.o two.o

This is a bit more complicated but fixes both problems and doesn't rely on 
FORCE_DO_CMD.

Original comment by kkania@chromium.org on 21 Jan 2013 at 7:57

GoogleCodeExporter commented 9 years ago 
Actually, the FORCE_DO_CMD stuff doesn't work. We'll have to use a separate 
file or something like I mentioned above.

See 
http://www.gnu.org/savannah-checkouts/gnu/automake/manual/html_node/Multiple-Out
puts.html

Original comment by kkania@chromium.org on 23 Jan 2013 at 12:59

GoogleCodeExporter commented 9 years ago 
Here's a sample gyp test that repro's the problem: 
https://codereview.chromium.org/12052022

Original comment by kkania@chromium.org on 23 Jan 2013 at 1:10

GoogleCodeExporter commented 9 years ago

Original comment by kkania@chromium.org on 25 Jan 2013 at 12:50