New issue
Advanced search Search tips

Issue 852620 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Jun 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug

Blocking:
issue 850728



Sign in to add a comment

Fix shaderc build

Project Member Reported by cblume@chromium.org, Jun 14 2018

Issue description

shaderc doesn't build because it has an interface with virtual functions which does not have a corresponding virtual destructor.

This appears to have been fixed upstream.
 

Comment 1 by cblume@chromium.org, Jun 14 2018

I began making a fix upstream before learning that they had already fixed it.
My pull request is no longer needed but I left it around in case they want to make things more explicit:
https://github.com/google/shaderc/pull/469

Additionally, it seems like the best option for us is to remove the Shaderc dependency. We currently only use it in unit tests and have no plans to compile GLSL at runtime (outside of ANGLE).

The CL for that is here:
https://chromium-review.googlesource.com/#/c/chromium/src/+/1100298
Project Member

Comment 2 by bugdroid1@chromium.org, Jun 14 2018

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

commit 7f7dd90b6e4a8db6eb2b05dacfe1d94b9a90370e
Author: Chris Blume <cblume@chromium.org>
Date: Thu Jun 14 02:17:37 2018

Remove Shaderc

Shaderc currently does not build and is used in a Vulkan test.
The upstream version of Shaderc has fixed the issue preventing the
build. However, we may not actually want Shaderc.

Currently, Shaderc is only being used in a unit test. It is possible
that in the future we will want to compiler GLSL at runtime. But there
are no plans for that right now.

BUG= 852620 

Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;luci.chromium.try:linux_optional_gpu_tests_rel;luci.chromium.try:mac_optional_gpu_tests_rel;luci.chromium.try:win_optional_gpu_tests_rel
Change-Id: I4edc119b6fe36851901a0b03924613c161db7c7e
Reviewed-on: https://chromium-review.googlesource.com/1100298
Reviewed-by: Antoine Labour <piman@chromium.org>
Reviewed-by: Chris Blume <cblume@chromium.org>
Commit-Queue: Chris Blume <cblume@chromium.org>
Cr-Commit-Position: refs/heads/master@{#567093}
[modify] https://crrev.com/7f7dd90b6e4a8db6eb2b05dacfe1d94b9a90370e/gpu/vulkan/BUILD.gn
[delete] https://crrev.com/33e367907d36133333914890249d090e2df6b0b8/gpu/vulkan/tests/shader_module_unittest.cc
[modify] https://crrev.com/7f7dd90b6e4a8db6eb2b05dacfe1d94b9a90370e/gpu/vulkan/vulkan_shader_module.cc
[modify] https://crrev.com/7f7dd90b6e4a8db6eb2b05dacfe1d94b9a90370e/gpu/vulkan/vulkan_shader_module.h
[delete] https://crrev.com/33e367907d36133333914890249d090e2df6b0b8/third_party/shaderc/BUILD.gn
[delete] https://crrev.com/33e367907d36133333914890249d090e2df6b0b8/third_party/shaderc/LICENSE
[delete] https://crrev.com/33e367907d36133333914890249d090e2df6b0b8/third_party/shaderc/OWNERS
[delete] https://crrev.com/33e367907d36133333914890249d090e2df6b0b8/third_party/shaderc/README.chromium
[delete] https://crrev.com/33e367907d36133333914890249d090e2df6b0b8/third_party/shaderc/shaderc_build_version_wrapper.py

Comment 3 by cblume@chromium.org, Jun 14 2018

Status: Fixed (was: Assigned)
Project Member

Comment 4 by bugdroid1@chromium.org, Sep 19

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

commit 490e363f23a54c896ec09e9e53982b23bb8bc22d
Author: Raphael Kubo da Costa <raphael.kubo.da.costa@intel.com>
Date: Wed Sep 19 17:45:37 2018

Stop checking out third_party/shaderc/src.

Shaderc was dropped altogether from the tree in commit 7f7dd90b6 ("Remove
Shaderc"), but the code was still being checked out for no reason.
Additionally, having third_party/shaderc/src with no accompanying
README.chromium causes tools/licenses.py to fail:

    licenses.LicenseError: missing README.chromium or licenses.py SPECIAL_CASES entry in third_party/shaderc

Bug: 39240,  852620 
Change-Id: Ic5e98407149ebccf320a976f2810c93f71ee6658
Reviewed-on: https://chromium-review.googlesource.com/1228053
Reviewed-by: Antoine Labour <piman@chromium.org>
Reviewed-by: Chris Blume <cblume@chromium.org>
Commit-Queue: Raphael Kubo da Costa (CET) <raphael.kubo.da.costa@intel.com>
Cr-Commit-Position: refs/heads/master@{#592461}
[modify] https://crrev.com/490e363f23a54c896ec09e9e53982b23bb8bc22d/DEPS

Sign in to add a comment