Skip to content

Commit

Permalink
Fixed build failure
Browse files Browse the repository at this point in the history
Reland "[Jellybean] Ensure legacy mappings ignore token namespace"

This reverts commit aa90c73.

Reason for revert: Fixed the underlying issue

Original change's description:
> Revert "[Jellybean] Ensure legacy mappings ignore token namespace"
>
> This reverts commit 5f390a2.
>
> Reason for revert: Breaks compilation on ChromiumOS ASAN Release
>
> See https://ci.chromium.org/ui/p/chromium/builders/ci/ChromiumOS%20ASAN%20Release/288455/overview
>
> Original change's description:
> > [Jellybean] Ensure legacy mappings ignore token namespace
> >
> > Previously tokens specified in legacy_mappings would
> > pick up the namespace of the json5 file they were declared
> > in. Since these are legacy variables we want to map to new
> > tokens they should not be prefixed with any token
> > namespace.
> >
> > This CL fixes the model to implement the correct behaviour
> > and adds a test case.
> >
> > Change-Id: Ifa18991bb6a1df8b29c722cafa77f2beb3cf599d
> > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4426610
> > Reviewed-by: Christos Froussios <cfroussios@chromium.org>
> > Commit-Queue: Zain Afzal <zafzal@google.com>
> > Cr-Commit-Position: refs/heads/main@{#1131666}
>
> Change-Id: I3de82b0a4b10cfcd21e80bebeeacaaab8a93fa97
> No-Presubmit: true
> No-Tree-Checks: true
> No-Try: true
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4439229
> Reviewed-by: Christos Froussios <cfroussios@chromium.org>
> Commit-Queue: Christos Froussios <cfroussios@chromium.org>
> Owners-Override: Christos Froussios <cfroussios@chromium.org>
> Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
> Cr-Commit-Position: refs/heads/main@{#1131686}

Change-Id: Ia54854d34f38e46d94127cad6652803d07442b1b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4439230
Reviewed-by: Christos Froussios <cfroussios@chromium.org>
Commit-Queue: Zain Afzal <zafzal@google.com>
Cr-Commit-Position: refs/heads/main@{#1132282}
  • Loading branch information
Zain Afzal authored and Chromium LUCI CQ committed Apr 19, 2023
1 parent 2b054d0 commit 225b194
Show file tree
Hide file tree
Showing 4 changed files with 27 additions and 15 deletions.
35 changes: 21 additions & 14 deletions tools/style_variable_generator/model.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,13 @@
from abc import ABC, abstractmethod


def full_token_name(name, context):
namespace = context['token_namespace']
if namespace:
return f'{namespace}.{name}'
return name


class Modes:
LIGHT = 'light'
DARK = 'dark'
Expand Down Expand Up @@ -158,12 +165,12 @@ def __init__(self, opacity_model):
self.variable_type = VariableType.COLOR

def Add(self, name, value_obj, context):
name = full_token_name(name, context)
added = []
# If a color has generate_per_mode set, a separate variable will be
# created for each mode, suffixed by mode name.
# (e.g my_color_light, my_color_debug)
generate_per_mode = False

# If a color has generate_inverted set, a |color_name|_inverted will be
# generated which uses the dark color for light mode and vice versa.
generate_inverted = False
Expand Down Expand Up @@ -278,7 +285,6 @@ def _BlendColors(self, color_a, color_b, mode):
def _CreateValue(self, value):
return Color(value)


class SimpleModel(collections.OrderedDict, Submodel):
def __init__(self, variable_type, check_func=None):
self.variable_type = variable_type
Expand All @@ -291,6 +297,13 @@ def Add(self, name, value_obj, context):
return [StyleVariable(self.variable_type, name, value_obj, context)]


# A simple model where all variables are prefixed with the current namespace.
class NamespacedModel(SimpleModel):
def Add(self, name, value_obj, context):
name = full_token_name(name, context)
return super().Add(name, value_obj, context)


class Model(object):
def __init__(self):
# A map of all variables to their |StyleVariable| object.
Expand All @@ -305,7 +318,7 @@ def __init__(self):
self.colors = ColorModel(self.opacities)
self.submodels[VariableType.COLOR] = self.colors

self.untyped_css = SimpleModel(VariableType.UNTYPED_CSS)
self.untyped_css = NamespacedModel(VariableType.UNTYPED_CSS)
self.submodels[VariableType.UNTYPED_CSS] = self.untyped_css

self.legacy_mappings = SimpleModel(VariableType.LEGACY_MAPPING)
Expand All @@ -317,23 +330,22 @@ def CheckTypeFace(name, value_obj, context):
assert value_obj['font_weight']
assert value_obj['line_height']

self.typefaces = SimpleModel(VariableType.TYPEFACE, CheckTypeFace)
self.typefaces = NamespacedModel(VariableType.TYPEFACE, CheckTypeFace)
self.submodels[VariableType.TYPEFACE] = self.typefaces

def CheckFontFamily(name, value_obj, context):
assert name.startswith('font_family_')

self.font_families = SimpleModel(VariableType.FONT_FAMILY,
CheckFontFamily)
self.font_families = NamespacedModel(VariableType.FONT_FAMILY,
CheckFontFamily)
self.submodels[VariableType.FONT_FAMILY] = self.font_families

def Add(self, variable_type, name, value_obj, context):
'''Adds a new variable to the submodel for |variable_type|.
'''
try:
full_name = self.FullTokenName(name, context)
added = self.submodels[variable_type].Add(full_name, value_obj,
context)
submodel = self.submodels[variable_type]
added = self.submodels[variable_type].Add(name, value_obj, context)
except ValueError as err:
raise ValueError(
f'Error parsing {variable_type} "{full_name}": {value_obj}'
Expand All @@ -344,11 +356,6 @@ def Add(self, variable_type, name, value_obj, context):
raise ValueError('Variable name "%s" is reused' % name)
self.variable_map[var.name] = var

def FullTokenName(self, name, context):
namespace = context['token_namespace']
if namespace:
return f'{namespace}.{name}'
return name

def PostProcess(self, default_preblend=True):
'''Called after all variables have been added to perform operations that
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,9 @@
field_id: 2,
},
},
legacy_mappings: {
'cros-color-primary' : '$cros.sys.primary-container',
},
colors: {
/* Primary */
primary: {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,8 @@ html:not(body) {
--cros-sys-disabled_opacity: 0.38;

--cros-sys-reference_opacity: var(--cros-sys-disabled_opacity);

--cros-color-primary: var(--cros-sys-primary_container);
}

@media (prefers-color-scheme: dark) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -317,7 +317,7 @@ def testColorTestJSON(self):
! UPDATE_GOLDENS IS TRUE. EACH TIME THE TESTS ARE RUN ALL GOLDENS
WILL BE UPDATED TO MATCH CURRENT BEHAVIOUR.
! ALL TESTS WILL PASS WITHOUT BEING CHECKED.
! THE CODE SHOULD NOT BE SUBBMITTED AS IS.
! THE CODE SHOULD NOT BE SUBMITTED AS IS.
===================================================================\033[0m
""")
unittest.main()

0 comments on commit 225b194

Please sign in to comment.