Bug 74472 – [chromium] Remove redundant third_party entries from chromium DEPS

• Status: RESOLVED
• Resolution: FIXED
• Component: Tools / Tests

[Kenneth Russell]

Currently Source/WebKit/chromium/DEPS contains an entry for 'third_party' as well as several directories under there. The entry is currently needed in order to create the third_party directory, but the redundancy is causing two sets of problems. First, it is causing intermittent gclient failures on the Windows builders due to gclient's parallelization and the fact that the top-level directory is locked by Subversion while gclient is attempting to populate subdirectories. See http://build.webkit.org/builders/Chromium%20Win%20Release/builds/36771 for an example of such a failure. Second, all of Chromium's directories in third_party are being pulled in via this rule. This makes the explicit listing of the dependencies under that directory meaningless -- and, in fact, it seems that several additional dependencies have crept in unnoticed.

update-webkit-chromium should be updated to manually create the third_party directory before running gclient, and all of the needed third_party dependencies should be enumerated in the DEPS file.

[Tony Chang]

The entry isn't needed to create the directory.  I think gclient is smart enough to create the necessary directories.  It was added because every week or two, someone would add something to src/third_party that webkit needed.  It mades rolling a lot harder when you need to add an entry to DEPS.

I thought gclient was smart enough to not start on a subdir until directories above it were finished.

We could also just turn parallel syncs off on the bot.

[Marc-Antoine Ruel]

- gclient won't sync a subdirectory before the parent directory is done. e.g.
if you have dependencies src and src/third_party/foo, src/third_party/foo won't be synced until src is completely done.
- gclient will create the directories itself up to the parent directory of the solution. So if you have src but no src/third_party and sync src/third_party/foo, gclient will first mkdir src/third_party, once src is completely synced.

[Kenneth Russell]

I see. As I'm struggling to get this change to actually work and adding in the required dependencies one by one I see what you mean.

Another solution, I'm pretty sure, would be to just remove the explicit checkouts of the third_party directories coming from Var('chromium_svn')+'/third_party/...'. They're useless given that we're already checking out the parent directory. Any objection to my making this change?

[Kenneth Russell]

Created attachment 119138 [details]
Patch

[Kenneth Russell]

What do you think about this? It seems safe, correct and should clear up these intermittent Windows gclient failures.

[WebKit Review Bot]

Comment on attachment 119138 [details]
Patch

Attachment 119138 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/10873072

[Kenneth Russell]

Hmm. That built locallly.

[Tony Chang]

This patch looks correct to me.  I think gclient is getting confused because it thinks that third_party/libwebp is safe to delete.

Maybe there's some way to get it to update without having to clobber the bots.  I'll try a few things locally.

[Tony Chang]

Created attachment 119328 [details]
Patch

[Tony Chang]

After the bots cycle, we can revert the change to update-webkit-chromium.

[Kenneth Russell]

Comment on attachment 119328 [details]
Patch

Thanks, looks good; r=me. Do you want to file a follow-on bug about the cleanup and reference it in the comment?

[Tony Chang]

Created attachment 119336 [details]
Patch for landing

[Kenneth Russell]

Another instance of the same problem, this time on Linux:

http://build.webkit.org/builders/Chromium%20Linux%20Release%20%28Tests%29/builds/27157

[WebKit Review Bot]

Comment on attachment 119336 [details]
Patch for landing

Clearing flags on attachment: 119336

Committed r102891: <http://trac.webkit.org/changeset/102891>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.

With this patch, update-webkit --chromium asks you to remove those directories, and if you fall for this trap, your checkout is broken :-/

[Tony Chang]

(In reply to comment #16)
> With this patch, update-webkit --chromium asks you to remove those directories, and if you fall for this trap, your checkout is broken :-/

That's true, we should have sent a note to chromium-dev about this.