[chromium-dev] Re: Clobbering
On Mon, Aug 10, 2009 at 10:45 PM, Aaron Boodman a...@chromium.org wrote: Such a system does not help when people sync your change. We should invest the effort we would expend building this system fixing the dependency issues. gclient could be made to obey it as well. But I agree, it's a hack. Barring that, we have a hack that we do where for resource files we make a whitespace change to the corresponding .gyp. We could automate this by having a presubmit check that enforces this. Better than nothing. We also have the hack for grd files that deletes all the associated .h files. Something like that might work here as well. How hard would it be to make the system actually understand the dependencies? Is visual studio really just too dumb to understand? --~--~-~--~~~---~--~~ Chromium Developers mailing list: chromium-dev@googlegroups.com View archives, change email options, or unsubscribe: http://groups.google.com/group/chromium-dev -~--~~~~--~~--~--~---
[chromium-dev] Re: Clobbering
We currently have a hack of sorts in mind (well an issue filed on gyp) that would cover a larger class of settings changes (the worst handled by vstudio directly). The idea is to have gyp generate a text file full of settings garp which would be an additional dependency of each project. There are of course other dependency flaws (sgk is working on a long running validation build that would look for missing links in the chain) that are just due to mistakes in the gyp files. I would be curious which kind of dependency issue these latest ones were (can someone point me at the CLs?, been on nacl/o3d buildbot stuff of late). -BradN On Mon, Aug 10, 2009 at 11:00 PM, Jeremy Orlow jor...@chromium.org wrote: On Mon, Aug 10, 2009 at 10:45 PM, Aaron Boodman a...@chromium.org wrote: Such a system does not help when people sync your change. We should invest the effort we would expend building this system fixing the dependency issues. gclient could be made to obey it as well. But I agree, it's a hack. Barring that, we have a hack that we do where for resource files we make a whitespace change to the corresponding .gyp. We could automate this by having a presubmit check that enforces this. Better than nothing. We also have the hack for grd files that deletes all the associated .h files. Something like that might work here as well. How hard would it be to make the system actually understand the dependencies? Is visual studio really just too dumb to understand? --~--~-~--~~~---~--~~ Chromium Developers mailing list: chromium-dev@googlegroups.com View archives, change email options, or unsubscribe: http://groups.google.com/group/chromium-dev -~--~~~~--~~--~--~---
[chromium-dev] Re: Clobbering
They were both WebKit deps rolls. The latter one was caused by an .idl file that changed. The former, I don't know off the top of my head. Here's the chromium review: http://codereview.chromium.org/165278 Here are the files that changed: http://trac.webkit.org/changeset?new=47...@trunkold=46...@trunk Here's the error visual studio gave: DerivedSourcesAllInOne.cpp C:\b\slave\win\build\src\chrome\Debug\obj\webcore\bindings/V8Document.cpp(1003) : error C2039: 'v8ElementEventHandlerAccessorGetter' : is not a member of 'WebCore::V8Custom' C:\b\slave\win\build\src\third_party\WebKit\WebCore\bindings\v8\custom\V8CustomBinding.h(94) : see declaration of 'WebCore::V8Custom' On Mon, Aug 10, 2009 at 11:17 PM, Bradley Nelson bradnel...@google.comwrote: We currently have a hack of sorts in mind (well an issue filed on gyp) that would cover a larger class of settings changes (the worst handled by vstudio directly). The idea is to have gyp generate a text file full of settings garp which would be an additional dependency of each project. There are of course other dependency flaws (sgk is working on a long running validation build that would look for missing links in the chain) that are just due to mistakes in the gyp files. I would be curious which kind of dependency issue these latest ones were (can someone point me at the CLs?, been on nacl/o3d buildbot stuff of late). -BradN On Mon, Aug 10, 2009 at 11:00 PM, Jeremy Orlow jor...@chromium.orgwrote: On Mon, Aug 10, 2009 at 10:45 PM, Aaron Boodman a...@chromium.org wrote: Such a system does not help when people sync your change. We should invest the effort we would expend building this system fixing the dependency issues. gclient could be made to obey it as well. But I agree, it's a hack. Barring that, we have a hack that we do where for resource files we make a whitespace change to the corresponding .gyp. We could automate this by having a presubmit check that enforces this. Better than nothing. We also have the hack for grd files that deletes all the associated .h files. Something like that might work here as well. How hard would it be to make the system actually understand the dependencies? Is visual studio really just too dumb to understand? --~--~-~--~~~---~--~~ Chromium Developers mailing list: chromium-dev@googlegroups.com View archives, change email options, or unsubscribe: http://groups.google.com/group/chromium-dev -~--~~~~--~~--~--~---
[chromium-dev] Re: Clobbering
I agree. Every clobber means we have a dependency bug. I would prefer that we track and fix those bugs than get desensitized to them. --Amanda On Tue, Aug 11, 2009 at 1:45 AM, Aaron Boodman a...@chromium.org wrote: Such a system does not help when people sync your change. We should invest the effort we would expend building this system fixing the dependency issues. Barring that, we have a hack that we do where for resource files we make a whitespace change to the corresponding .gyp. We could automate this by having a presubmit check that enforces this. - a On Mon, Aug 10, 2009 at 10:16 PM, Jeremy Orlowjor...@chromium.org wrote: We really need a better way to submit patches that we know require a clobber. Today alone, there were 2 WebKit deps rolls that we _knew_ would need a clobber. Both ended up closing the tree for a bit. What if we added an optional flag to the CL descriptions that tells the bots that a clobber is necessary? Maybe just CLOBBER=blah where blah is the platforms that require it split by commas and/or spaces? So, for example, my CL might look like: This is a really nice CL, but it touches files that confuse the dependency tracking system on Windows and linux. TEST=none BUG=none CLOBBER=win, linux Would this be terribly hard? Any major downsides? J -- Portability is generally the result of advance planning rather than trench warfare involving #ifdef -- Henry Spencer (1992) --~--~-~--~~~---~--~~ Chromium Developers mailing list: chromium-dev@googlegroups.com View archives, change email options, or unsubscribe: http://groups.google.com/group/chromium-dev -~--~~~~--~~--~--~---
[chromium-dev] Re: Clobbering
Knowing MS, it could be the forward slash (/) that is breaking the dependency scanner. M-A On Tue, Aug 11, 2009 at 2:24 AM, Jeremy Orlowjor...@chromium.org wrote: They were both WebKit deps rolls. The latter one was caused by an .idl file that changed. The former, I don't know off the top of my head. Here's the chromium review: http://codereview.chromium.org/165278 Here are the files that changed: http://trac.webkit.org/changeset?new=47...@trunkold=46...@trunk Here's the error visual studio gave: DerivedSourcesAllInOne.cpp C:\b\slave\win\build\src\chrome\Debug\obj\webcore\bindings/V8Document.cpp(1003) : error C2039: 'v8ElementEventHandlerAccessorGetter' : is not a member of 'WebCore::V8Custom' C:\b\slave\win\build\src\third_party\WebKit\WebCore\bindings\v8\custom\V8CustomBinding.h(94) : see declaration of 'WebCore::V8Custom' On Mon, Aug 10, 2009 at 11:17 PM, Bradley Nelson bradnel...@google.com wrote: We currently have a hack of sorts in mind (well an issue filed on gyp) that would cover a larger class of settings changes (the worst handled by vstudio directly). The idea is to have gyp generate a text file full of settings garp which would be an additional dependency of each project. There are of course other dependency flaws (sgk is working on a long running validation build that would look for missing links in the chain) that are just due to mistakes in the gyp files. I would be curious which kind of dependency issue these latest ones were (can someone point me at the CLs?, been on nacl/o3d buildbot stuff of late). -BradN On Mon, Aug 10, 2009 at 11:00 PM, Jeremy Orlow jor...@chromium.org wrote: On Mon, Aug 10, 2009 at 10:45 PM, Aaron Boodman a...@chromium.org wrote: Such a system does not help when people sync your change. We should invest the effort we would expend building this system fixing the dependency issues. gclient could be made to obey it as well. But I agree, it's a hack. Barring that, we have a hack that we do where for resource files we make a whitespace change to the corresponding .gyp. We could automate this by having a presubmit check that enforces this. Better than nothing. We also have the hack for grd files that deletes all the associated .h files. Something like that might work here as well. How hard would it be to make the system actually understand the dependencies? Is visual studio really just too dumb to understand? --~--~-~--~~~---~--~~ Chromium Developers mailing list: chromium-dev@googlegroups.com View archives, change email options, or unsubscribe: http://groups.google.com/group/chromium-dev -~--~~~~--~~--~--~---
[chromium-dev] Re: Clobbering
The culprit here was the change to the CodeGeneratorV8.pm. It looks like we should somehow trigger clobber of of rule_binding.py when that happens. :DG On Mon, Aug 10, 2009 at 11:24 PM, Jeremy Orlowjor...@chromium.org wrote: They were both WebKit deps rolls. The latter one was caused by an .idl file that changed. The former, I don't know off the top of my head. Here's the chromium review: http://codereview.chromium.org/165278 Here are the files that changed: http://trac.webkit.org/changeset?new=47...@trunkold=46...@trunk Here's the error visual studio gave: DerivedSourcesAllInOne.cpp C:\b\slave\win\build\src\chrome\Debug\obj\webcore\bindings/V8Document.cpp(1003) : error C2039: 'v8ElementEventHandlerAccessorGetter' : is not a member of 'WebCore::V8Custom' C:\b\slave\win\build\src\third_party\WebKit\WebCore\bindings\v8\custom\V8CustomBinding.h(94) : see declaration of 'WebCore::V8Custom' On Mon, Aug 10, 2009 at 11:17 PM, Bradley Nelson bradnel...@google.com wrote: We currently have a hack of sorts in mind (well an issue filed on gyp) that would cover a larger class of settings changes (the worst handled by vstudio directly). The idea is to have gyp generate a text file full of settings garp which would be an additional dependency of each project. There are of course other dependency flaws (sgk is working on a long running validation build that would look for missing links in the chain) that are just due to mistakes in the gyp files. I would be curious which kind of dependency issue these latest ones were (can someone point me at the CLs?, been on nacl/o3d buildbot stuff of late). -BradN On Mon, Aug 10, 2009 at 11:00 PM, Jeremy Orlow jor...@chromium.org wrote: On Mon, Aug 10, 2009 at 10:45 PM, Aaron Boodman a...@chromium.org wrote: Such a system does not help when people sync your change. We should invest the effort we would expend building this system fixing the dependency issues. gclient could be made to obey it as well. But I agree, it's a hack. Barring that, we have a hack that we do where for resource files we make a whitespace change to the corresponding .gyp. We could automate this by having a presubmit check that enforces this. Better than nothing. We also have the hack for grd files that deletes all the associated .h files. Something like that might work here as well. How hard would it be to make the system actually understand the dependencies? Is visual studio really just too dumb to understand? --~--~-~--~~~---~--~~ Chromium Developers mailing list: chromium-dev@googlegroups.com View archives, change email options, or unsubscribe: http://groups.google.com/group/chromium-dev -~--~~~~--~~--~--~---
[chromium-dev] Re: Clobbering
Never mind, that line is already complete! GENERATE_BINDINGS_SCRIPTS = \ bindings/scripts/CodeGenerator.pm \ bindings/scripts/IDLParser.pm \ bindings/scripts/IDLStructure.pm \ bindings/scripts/generate-bindings.pl \ On Tue, Aug 11, 2009 at 7:51 AM, Eric Seidel esei...@chromium.org wrote: IDL file tracking is part of why WebKit uses DerivedSources.make (which I assume Chromium does to? Or at least we will eventually need to if we're going to have an upstream WebKit build...) JS%.h : %.idl $(GENERATE_BINDINGS_SCRIPTS) bindings/scripts/CodeGeneratorJS.pm $(GENERATE_BINDINGS) --defines $(FEATURE_DEFINES) LANGUAGE_JAVASCRIPT --generator JS $ That line actually should depend on all of the .pm files in bindings/scripts, but generally CodeGeneratorJS.pm has been enough. -eric On Tue, Aug 11, 2009 at 7:25 AM, Dimitri Glazkov dglaz...@google.comwrote: The culprit here was the change to the CodeGeneratorV8.pm. It looks like we should somehow trigger clobber of of rule_binding.py when that happens. :DG On Mon, Aug 10, 2009 at 11:24 PM, Jeremy Orlowjor...@chromium.org wrote: They were both WebKit deps rolls. The latter one was caused by an .idl file that changed. The former, I don't know off the top of my head. Here's the chromium review: http://codereview.chromium.org/165278 Here are the files that changed: http://trac.webkit.org/changeset?new=47...@trunkold=46...@trunk Here's the error visual studio gave: DerivedSourcesAllInOne.cpp C:\b\slave\win\build\src\chrome\Debug\obj\webcore\bindings/V8Document.cpp(1003) : error C2039: 'v8ElementEventHandlerAccessorGetter' : is not a member of 'WebCore::V8Custom' C:\b\slave\win\build\src\third_party\WebKit\WebCore\bindings\v8\custom\V8CustomBinding.h(94) : see declaration of 'WebCore::V8Custom' On Mon, Aug 10, 2009 at 11:17 PM, Bradley Nelson bradnel...@google.com wrote: We currently have a hack of sorts in mind (well an issue filed on gyp) that would cover a larger class of settings changes (the worst handled by vstudio directly). The idea is to have gyp generate a text file full of settings garp which would be an additional dependency of each project. There are of course other dependency flaws (sgk is working on a long running validation build that would look for missing links in the chain) that are just due to mistakes in the gyp files. I would be curious which kind of dependency issue these latest ones were (can someone point me at the CLs?, been on nacl/o3d buildbot stuff of late). -BradN On Mon, Aug 10, 2009 at 11:00 PM, Jeremy Orlow jor...@chromium.org wrote: On Mon, Aug 10, 2009 at 10:45 PM, Aaron Boodman a...@chromium.org wrote: Such a system does not help when people sync your change. We should invest the effort we would expend building this system fixing the dependency issues. gclient could be made to obey it as well. But I agree, it's a hack. Barring that, we have a hack that we do where for resource files we make a whitespace change to the corresponding .gyp. We could automate this by having a presubmit check that enforces this. Better than nothing. We also have the hack for grd files that deletes all the associated .h files. Something like that might work here as well. How hard would it be to make the system actually understand the dependencies? Is visual studio really just too dumb to understand? --~--~-~--~~~---~--~~ Chromium Developers mailing list: chromium-dev@googlegroups.com View archives, change email options, or unsubscribe: http://groups.google.com/group/chromium-dev -~--~~~~--~~--~--~---
[chromium-dev] Re: Clobbering
Dimitri Glazkov wrote: The culprit here was the change to the CodeGeneratorV8.pm. It looks like we should somehow trigger clobber of of rule_binding.py when that happens. We already do. In the .idl rule in webkit.gyp, we have: 'inputs': [ '../third_party/WebKit/WebCore/bindings/scripts/generate-bindings.pl', '../third_party/WebKit/WebCore/bindings/scripts/CodeGenerator.pm', '../third_party/WebKit/WebCore/bindings/scripts/CodeGeneratorV8.pm', '../third_party/WebKit/WebCore/bindings/scripts/IDLParser.pm', '../third_party/WebKit/WebCore/bindings/scripts/IDLStructure.pm', ], meaning that if any of those files changes, or an .idl file changes, we'll run generate-bindings.pl again. Mark --~--~-~--~~~---~--~~ Chromium Developers mailing list: chromium-dev@googlegroups.com View archives, change email options, or unsubscribe: http://groups.google.com/group/chromium-dev -~--~~~~--~~--~--~---
[chromium-dev] Re: Clobbering
Ok I think I've found it.It looks like a bug in the rule - makefile emitter which is only used for webcore. (Most gyp rules turn into custom build rules, but we added a makefile emitter option for this one because the native rules were too slow). It appears that the custom build step for the makefile didn't get any of the dependencies on the inputs (oddly the makefile itself does have them). Filing bug against gyp. Working on fix... -BradN On Tue, Aug 11, 2009 at 8:03 AM, Mark Mentovai m...@chromium.org wrote: Dimitri Glazkov wrote: The culprit here was the change to the CodeGeneratorV8.pm. It looks like we should somehow trigger clobber of of rule_binding.py when that happens. We already do. In the .idl rule in webkit.gyp, we have: 'inputs': [ '../third_party/WebKit/WebCore/bindings/scripts/generate-bindings.pl', '../third_party/WebKit/WebCore/bindings/scripts/CodeGenerator.pm', '../third_party/WebKit/WebCore/bindings/scripts/CodeGeneratorV8.pm', '../third_party/WebKit/WebCore/bindings/scripts/IDLParser.pm', '../third_party/WebKit/WebCore/bindings/scripts/IDLStructure.pm', ], meaning that if any of those files changes, or an .idl file changes, we'll run generate-bindings.pl again. Mark --~--~-~--~~~---~--~~ Chromium Developers mailing list: chromium-dev@googlegroups.com View archives, change email options, or unsubscribe: http://groups.google.com/group/chromium-dev -~--~~~~--~~--~--~---
[chromium-dev] Re: Clobbering
Nice!!! On Tue, Aug 11, 2009 at 3:57 PM, Bradley Nelson bradnel...@google.comwrote: Ok I think I've found it.It looks like a bug in the rule - makefile emitter which is only used for webcore. (Most gyp rules turn into custom build rules, but we added a makefile emitter option for this one because the native rules were too slow). It appears that the custom build step for the makefile didn't get any of the dependencies on the inputs (oddly the makefile itself does have them). Filing bug against gyp. Working on fix... -BradN On Tue, Aug 11, 2009 at 8:03 AM, Mark Mentovai m...@chromium.org wrote: Dimitri Glazkov wrote: The culprit here was the change to the CodeGeneratorV8.pm. It looks like we should somehow trigger clobber of of rule_binding.py when that happens. We already do. In the .idl rule in webkit.gyp, we have: 'inputs': [ '../third_party/WebKit/WebCore/bindings/scripts/generate-bindings.pl', '../third_party/WebKit/WebCore/bindings/scripts/CodeGenerator.pm', '../third_party/WebKit/WebCore/bindings/scripts/CodeGeneratorV8.pm', '../third_party/WebKit/WebCore/bindings/scripts/IDLParser.pm', '../third_party/WebKit/WebCore/bindings/scripts/IDLStructure.pm', ], meaning that if any of those files changes, or an .idl file changes, we'll run generate-bindings.pl again. Mark --~--~-~--~~~---~--~~ Chromium Developers mailing list: chromium-dev@googlegroups.com View archives, change email options, or unsubscribe: http://groups.google.com/group/chromium-dev -~--~~~~--~~--~--~---
