New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 839924 link

Starred by 6 users

Issue metadata

Status: Fixed
Owner:
Closed: May 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: iOS
Pri: 1
Type: Bug



Sign in to add a comment

ChromeWebView generate_license step fails

Project Member Reported by michaeldo@chromium.org, May 4 2018

Issue description

Running ios/web_view:ios_web_view_generate_license fails with the following error:

[1/2] ACTION //ios/web_view:ios_web_view_generate_license(//build/toolchain/mac:ios_clang_x64)
FAILED: ios_web_view/LICENSE 
python ../../tools/licenses.py license_file ios_web_view/LICENSE --gn-target //ios/web_view --gn-out-dir .
Traceback (most recent call last):
  File "../../tools/licenses.py", line 756, in <module>
    sys.exit(main())
  File "../../tools/licenses.py", line 748, in main
    args.output_file, args.gn_out_dir, args.gn_target):
  File "../../tools/licenses.py", line 701, in GenerateLicenseFile
    directory, _REPOSITORY_ROOT, require_license_file=True)
  File "../../tools/licenses.py", line 368, in ParseDir
    "SPECIAL_CASES entry in %s\n" % path)
__main__.LicenseError: missing README.chromium or licenses.py SPECIAL_CASES entry in third_party/skia/third_party/skcms

ninja: build stopped: subcommand failed.
 
Cc: dpranke@chromium.org michaeldo@chromium.org ichikawa@chromium.org
Owner: ----
Status: Available (was: Started)
ichikawa@ is OOO, dpranke@ do you have any idea about this error?
Issue 839985 has been merged into this issue.
Components: Internals>Skia
Labels: -Pri-3 Pri-1
Owner: mtklein@chromium.org
Status: Assigned (was: Available)
This looks like a problem in Skia, with the way they added this dependency. I'm not sure why we're only noticing this now, given that we added it in January.

@mtklein - can you take a look and add a README.chromium or something as appropriate to fix this? I'm not actually seeing much in the Skia repo around this so I'm not sure how you've handled it in the past for similar dependencies.
Issue 840137 has been merged into this issue.
Hey, is third_party/skia/third_party/skcms not covered by third_party/skcms/README.chromium?
Am I missing something? AFAICT, there is no README.chromium.
Oops, typo.  I mean third_party/skia/README.chromium.  Is third_party/skia/third_party/skcms not covered by third_party/skia/README.chromium?
No, it wouldn't be, at least not by default. Anything under a third_party directory needs its own README.chromium to clarify what the relationship is.

It looks like skcms is actually part of Skia (maybe you wrote it)? Why is it in a separate repo?
Yep, we copy its source (skipping test code and data) into Skia with an autoroller, which then rolls out as normal to people who use it.
Okay. Can you add a stub README explaining that? 

I can double-check what we might need to do to make sure the license script can handle it.
Just curious... if you don't have a cooperative upstream, does this mean you can't DEPS in third_party targets?  You have to copy them in and add a README.chromium?
Often we'll create a placeholder directory with the needed file(s) and then put the actual repo in a subdirectory, e.g., //third_party/junit/README.chromium and //third_party/junit/src.
So the problem is just that we sync it into Skia under a directory named third_party?  If we sync it into third_party/skia/skcms, it's fine?  but third_party/skia/third_party/skcms is not?
Correct.

I'm still a bit unclear why it's a separate repo at all, though. I suppose there are other users of just that directory?
Cc: linds...@chromium.org
Yep, it's standalone, and an optional dependency of Skia.  Chromium uses it both directly and indirectly via Skia, and most other clients work the same way, though some just use skcms, and some use Skia without skcms.

Why would we care what repo it's in?
I don't really care, just curious.
Cc: brianosman@chromium.org pmarko@chromium.org hendrich@chromium.org mtklein@chromium.org
 Issue 840650  has been merged into this issue.
Project Member

Comment 19 by bugdroid1@chromium.org, May 8 2018

The following revision refers to this bug:
  https://skia.googlesource.com/skcms/+/ba83c975ac9eff93b8c840fa516ec578aca112dc

commit ba83c975ac9eff93b8c840fa516ec578aca112dc
Author: Mike Klein <mtklein@chromium.org>
Date: Tue May 08 13:22:47 2018

add README.chromium

  ¯\_(ツ)_/¯

Bug:  chromium:839924 

Change-Id: I0f97d121bcbb1c0491bdf043361b6815edb61683
Reviewed-on: https://skia-review.googlesource.com/126701
Auto-Submit: Mike Klein <mtklein@chromium.org>
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>

[add] https://crrev.com/ba83c975ac9eff93b8c840fa516ec578aca112dc/README.chromium

Comment 20 by pkl@chromium.org, May 8 2018

mtklein: Just wondering what's the current plan on this bug. According to issue 840137, this blocks Chrome for iOS' nightly canary build (which impacts our Dev channel) as well as another trybot for our new UI development/testing.

Our last successful build was last Wednesday (May 2) evening.

I think we just gotta let that CL up there in #19 roll into Chromium?

Comment 22 by pkl@chromium.org, May 8 2018

Autoroll should work. Thank you for the quick feedback.

Sorry that I seemed naggy, our comments 19/20 must have crossed in the ether.

We've just noticed it's going to be an hour or two more than we thought... autorollers configured in funny ways.  I definitely don't think it should be a problem past today.
Project Member

Comment 24 by bugdroid1@chromium.org, May 8 2018

The following revision refers to this bug:
  https://skia.googlesource.com/buildbot/+/48ce2ac432b0b2308b3bb1cc63264c960077e515

commit 48ce2ac432b0b2308b3bb1cc63264c960077e515
Author: Mike Klein <mtklein@chromium.org>
Date: Tue May 08 15:16:04 2018

add README.chromium to skcms->skia whitelist

Bug:  chromium:839924 

Change-Id: Ieccdc33744a5258618d4693b0ea37960b46d6329
Reviewed-on: https://skia-review.googlesource.com/126800
Commit-Queue: Mike Klein <mtklein@chromium.org>
Commit-Queue: Ravi Mistry <rmistry@google.com>
Auto-Submit: Mike Klein <mtklein@chromium.org>
Reviewed-by: Ravi Mistry <rmistry@google.com>

[modify] https://crrev.com/48ce2ac432b0b2308b3bb1cc63264c960077e515/autoroll/config/skcms-skia.json

Project Member

Comment 25 by bugdroid1@chromium.org, May 8 2018

Status: Fixed (was: Assigned)
This is fixed right?

Sign in to add a comment