Why does the build have an action that updates files in the source tree?

[Mark Mentovai]

Neb and Alok-

We found a file in the source tree that’s both checked in AND
something that’s potentially touched during the build process:
webkit/plugins/ppapi/ppb_opengles_impl.cc. This file is generated by
the generate_ppapi_gles_implementation action in
webkit/glue/webkit_glue.gypi, which specifies:

{
'action_name': 'generate_ppapi_gles_implementation',
'variables': {
'gles_script':
'<(DEPTH)/gpu/command_buffer/build_gles2_cmd_buffer.py',
},
'inputs': [
'<(gles_script)',
],
'outputs': [
'<(DEPTH)/webkit/plugins/ppapi/ppb_opengles_impl.cc',
],
'action': [
'python',
'<(gles_script)',
'--alternate-mode=chrome_ppapi'
],
'message': 'Generating Pepper OpenGL ES implementation',
}

This is bad. ppb_opengles_impl.cc shouldn’t be in the source tree
(where you’ve placed it now). It should be somewhere in
<(INTERMEDIATE_DIR) or <(SHARED_INTERMEDIATE_DIR).

It looks like at some point (r73222) this file transitioned from being
checked-in to being generated during the build. That would have been
the right time to move the location of the file.

Having a generated file checked in is bad for all sorts of reasons.
It’ll add a nuisance file to developers’ changelists. It’ll interfere
with the “pristine state” of a source tree. It can confuse build
environments that need to track dependencies.

Please restructure your action to not dump its output into the source tree.

[Evan Martin]

On Mon, Feb 7, 2011 at 1:06 PM, Mark Mentovai <ma...@chromium.org> wrote:
>              'outputs': [
>                '<(DEPTH)/webkit/plugins/ppapi/ppb_opengles_impl.cc',
>              ],

Should we consider some sort of gyp lint check that prevents output
paths that don't use one of the intermediate dirs?
I think getting this right is kind of subtle.

[Gavin Peters (蓋文彼德斯)]

I have now reverted this change: http://codereview.chromium.org/6461001 , landed as r74161.

A few times a day, these automated build products (which were built during common build targets like "chrome" and "unit_tests") were causing me merge conflicts.

Happy merging, everyone!

- Gavin

--
Chromium Developers mailing list: chromi...@chromium.org
View archives, change email options, or unsubscribe:
   http://groups.google.com/a/chromium.org/group/chromium-dev

[Mark Mentovai]

Nebojsa Sabovic wrote:
> This CL was originally producing the generated sources in the intermediate
> dir, but then we've decided to keep the generated files checked in the
> source tree so that we can notice when they change, and to have a make
> target that can be triggered manually.
> I've added suppress_wildcard: 1, I thought that would do it. No?
> I'll re-land the .py part, at least. Is there a way of making a build target
> that won't run as part of any build?

suppress_wildcard only excludes * from depending on the target. It
doesn’t prevent anyone else from writing an explicit dependency on
your target.

If there’s a GYP target for it, someone might depend on it, and then
it becomes part of the build.

[Mark Mentovai]

Nebojsa Sabovic wrote:
> No rule explicitly depends on it. Why was it getting built then?

I suspect that this is a by-product of how make works. The makefiles
wind up with a rule to produce that file, so any time something
depends on that file (for example, to compile it), make will run the
rule if the file appears out of date. make doesn’t quite offer
target-level granularity, it’s allows file-to-file dependency
relationships, and effectively puts them into the same dependency
graph as target-to-target relationships.

[Elliot Glaysher (Chromium)]

On Tue, Feb 8, 2011 at 12:42 PM, Nebojsa Sabovic <n...@chromium.org> wrote:
> No rule explicitly depends on it. Why was it getting built then?

If you just type "make", it will implicitly build all targets. Could that be it?

-- Elliot