search

yuhangwang / gyp

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

Ninja generator fails when 'rules' only has conditional actions #253

Open GoogleCodeExporter opened 9 years ago

GoogleCodeExporter commented 9 years ago 
See the gyp targets in: 
http://code.google.com/codesearch#OAMlx_jo-ck/src/third_party/libjpeg_turbo/libj
peg.gyp and the rule "assemble"  on L249. In order to get ninja to run, I added:
'action': [],

As the action is currently only defined in a conditional, the ninja generator 
fails in WriteRules:

Traceback (most recent call last):
  File "/usr/local/google/code/trunk/external/chrome/build/gyp_chromium", line 171, in <module>
    sys.exit(gyp.main(args))
  File "/usr/local/google/code/trunk/external/chrome/tools/gyp/pylib/gyp/__init__.py", line 480, in main
    generator.GenerateOutput(flat_list, targets, data, params)
  File "/usr/local/google/code/trunk/external/chrome/tools/gyp/pylib/gyp/generator/ninja.py", line 1290, in GenerateOutput
    config_name)
  File "/usr/local/google/code/trunk/external/chrome/tools/gyp/pylib/gyp/generator/ninja.py", line 1268, in GenerateOutputForConfig
    target = writer.WriteSpec(spec, config_name)
  File "/usr/local/google/code/trunk/external/chrome/tools/gyp/pylib/gyp/generator/ninja.py", line 342, in WriteSpec
    spec, extra_sources, actions_depends, mac_bundle_depends)
  File "/usr/local/google/code/trunk/external/chrome/tools/gyp/pylib/gyp/generator/ninja.py", line 399, in WriteActionsRulesCopies
    extra_mac_bundle_resources)
  File "/usr/local/google/code/trunk/external/chrome/tools/gyp/pylib/gyp/generator/ninja.py", line 463, in WriteRules
    args = rule['action']
KeyError: 'action'

Original issue reported on code.google.com by yfriedman@chromium.org on 23 Feb 2012 at 7:05

GoogleCodeExporter commented 9 years ago 
Interesting; this doesn't bite chromeos builds b/c of l.13 of that .gyp file 
(chromeos==1 implies use system libjpeg).

ISTM the right thing to do is to add a /bin/false action for the arm case.
An empty rule only works b/c there are no .asm files in the sources list in the 
arm case, today.
If an .asm file is ever added, you'd want to make sure a proper assembler is 
added, too, so hopefully a /bin/false action would be a good indicator that 
something else needs to be done (probably with an explanatory comment).

I don't think there's a gyp bug here.
(this isn't so much a cross-compile ninja.py issue; the same would happen if 
building for arm on arm)

Original comment by fischman@chromium.org on 23 Feb 2012 at 7:42

GoogleCodeExporter commented 9 years ago 
I agree that it's not a cross-compile issue. However, the make generator 
doesn't have the same issue. So you don't think that there's a general problem 
where a 'rules' section can have no 'action' because the conditions don't match 
the build configuration? That seems like a technically valid gyp configuration.

Original comment by yfriedman@chromium.org on 23 Feb 2012 at 7:49

GoogleCodeExporter commented 9 years ago 
yfriedman: sorry I missed your question in #2 when you sent it.  Indeed, I 
think there's a difference between "do nothing" and "should never get here".  
I'm saying in the case you've uncovered you actually want "should never get 
here".  Whether gyp's ninja.py ought to support an _implicit_ "do nothing" is 
something I'll let gyp's owners decide (I would say no, considering how rare 
the need has been (de-facto), and that adding implicit magic to things makes 
understanding those things harder than necessary).

Original comment by fischman@chromium.org on 14 May 2012 at 9:08

GoogleCodeExporter commented 9 years ago 
Commit: 952c8b185cc8152c887e04a4af9e6f1f6a79f75f
 Email: thakis@chromium.org@78cadc50-ecff-11dd-a971-7dbc132099af

Fix bug in ninja/msvs/scons generator when there's no default action.

In some cases, an action is only evaluated under certain conditions. 
This works in the make generator, update the others to match.

BUG=gyp:253
TEST=python gyptest.py -a test/rules/gyptest-all.py passes 

Review URL: http://codereview.chromium.org/10382161/
Patch from Yaron Friedman <yfriedman@chromium.org>

git-svn-id: http://gyp.googlecode.com/svn/trunk@1410 
78cadc50-ecff-11dd-a971-7dbc132099af

M   pylib/gyp/generator/msvs.py
M   pylib/gyp/generator/ninja.py
M   pylib/gyp/generator/scons.py
M   test/rules/gyptest-all.py
M   test/rules/src/actions.gyp
A   test/rules/src/noaction/file1.in
A   test/rules/src/noaction/no_action_with_rules_fails.gyp
A   test/rules/src/subdir2/no_action.gyp

Original comment by bugdroid1@chromium.org on 31 May 2012 at 11:29

GoogleCodeExporter commented 9 years ago 
thakis: were you planning to roll gyp r1410 into chromium DEPS?  (or was there 
a reason you were holding off?)

Original comment by fischman@chromium.org on 6 Jun 2012 at 10:40