New issue
Advanced search Search tips

Issue 878300 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Aug 31
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 1
Type: Bug



Sign in to add a comment

V8 deps roll broken: https://crrev.com/c/1192237

Project Member Reported by machenb...@chromium.org, Aug 28

Issue description

V8 deps roll https://crrev.com/c/1192237 broken by https://crrev.com/c/1188601

The change includes a CIPD for proguard. The dilemma: V8 depends on proguard as a repo. Looks like gclient doesn't support both entries at the same time.

Here an attempt to add recursive deps which also fails:
https://crrev.com/c/1192924
 
Components: Blink>JavaScript Build
Description: Show this description
Cc: ehmaldonado@chromium.org agrieve@chromium.org
Components: Infra>Client>V8
Cc: bpastene@chromium.org jbudorick@chromium.org
If https://crrev.com/c/1192924 isn't going to work, what are the alternatives?

Could we have the proguard CIPD under a different directory? E.g. third_party/proguard_cipd? And just adjust some references? Then V8 could have both DEPS entries side-by-side in the toplevel DEPS file.
Here is a workaround like sketched in comment 5: https://crrev.com/c/1193928
Just realized that //third_party/r8 would also have this issue (as would any third-party library with prebuilts that we depend on.

wdyt about having a DEPS in //build that would pull all of these in under //build/third_party?
If it works, that'd be most desired. But recursing into build might lead to the same problem as my first CL.

In any way, we (V8) would like to get a solution fast, since this is fully blocking rolling deps into V8, and possibly stacking up other problems. E.g. the workaround https://crrev.com/c/1193928 until a better solution is found.
What do you mean with #7?
See also  http://crbug.com/800048#c4 
Here's the idea for #7:
//chromium-review.googlesource.com/c/chromium/src/+/1195827

It doesn't work though. I'm guessing it's because //build isn't a dep itself, so recursedeps ignores it?
Cc: dpranke@chromium.org yvesg@google.com
Yes.
+Yves is working on an include mechanism, so that we can let //build list its own DEPS, and do something like #7.
+Dirk on whether we can put things under //build/third_party
We should try to avoid a //build/third_party if at all possible.

First, in general the OSPO wants us to have as few third_party directories as possible per repo, to make their own processes work better. I realize you pull //build in as a separate repo, but it's not one in src.git, which is probably what matters.

Second, it's not clear that a given dependency is either only used by //build or only *not* used by //build (i.e., //build and non-build things don't share references). Unless we were sure that that was the case, and would never be the case, I would be leery of trying to move these dependencies from one to the other.

If someone wants to investigate whether that's true or not and it turns out that it is and seems like we can keep it true in the future, then I'm willing to go to the OSPO and see what they think about us creating another third_party dir.

Would it be possible to change v8 to use proguard as a CIPD package, and not a repo? 
re: your last question: the problem is that they are pulling in //src/proguard as a separate .git entry in order to pull the BUILD.gn file (which lives in src.git), but then also need to include //src/proguard as a CIPD entry.

To fix it so that v8 could pull //src/proguard/lib as a CIPD entry, we'd need to have the CIPD upload process be:
1. Copy the .yaml file into lib
2. Run the cipd create command
3. Delete the .yaml file in lib

Not that hardest thing in the world, but right now all the instructions say to put the .yaml file parent directory. 
Project Member

Comment 14 by bugdroid1@chromium.org, Aug 30

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/55b4289389438570c972a9583c4490eb558a1552

commit 55b4289389438570c972a9583c4490eb558a1552
Author: Andrew Grieve <agrieve@chromium.org>
Date: Thu Aug 30 01:59:59 2018

Android: Don't reference //third_party/proguard when !enable_java_templates

Should fix v8 auto-roller.

TBR=jbudorick

Bug:  878300 
Change-Id: Iec5a638f1eb1560c1fb6af0421d3df1e1bea21bc
Reviewed-on: https://chromium-review.googlesource.com/1196183
Reviewed-by: agrieve <agrieve@chromium.org>
Commit-Queue: agrieve <agrieve@chromium.org>
Cr-Commit-Position: refs/heads/master@{#587413}
[modify] https://crrev.com/55b4289389438570c972a9583c4490eb558a1552/build/android/BUILD.gn

Is the roller fixed?
It's stuck on something else now :( trying to push it forward and fix everything at once...
The other breakage is luckily shortly after your fix. So I update the roll now until your fix exactly and try again:
https://crrev.com/c/1192237
Status: Fixed (was: Assigned)
This is fixed now. Closing this bug, but hierarchical DEPS of some sort would still be desired. We just got the next breakage right after: https://chromium-review.googlesource.com/c/v8/v8/+/1199404

Sign in to add a comment