Skip to content

Commit

Permalink
Bug 1840537 - Enable value checking in moz.build for CONFIG variables…
Browse files Browse the repository at this point in the history
… derived from target.*. r=firefox-build-system-reviewers,andi,sergesanspaille

This makes the types from mozbuild.configure.constants repr()-able, and
repr()-ed in config.status, which, when processing moz.build, translates
into value checking for comparison tests involving the variables.

To make them pickable, though, we replace the use of EnumString.subclass
with actual subclassing, which is a little less convenient, but avoids
having to figure out how to make the classes EnumString.subclass creates
pickable.

This caught some mismatches in media/libpng and third_party/libsrtp.

This also means we don't need to normalize the config before dumping it
in config.status, because the only types that this was actually useful
for are these (historically, we'd also turn byte strings into unicode
strings but that hasn't been a thing for 4 years ; the special treatment
of dicts and iterables was there to apply the normalization recursively,
not to normalize dicts and iterables themselves). We still normalization
before passing values to gyp, though.

Differential Revision: https://phabricator.services.mozilla.com/D182141
  • Loading branch information
glandium committed Jul 6, 2023
1 parent e40d788 commit 2fddbb7
Show file tree
Hide file tree
Showing 8 changed files with 81 additions and 72 deletions.
4 changes: 2 additions & 2 deletions build/moz.configure/init.configure
Original file line number Diff line number Diff line change
Expand Up @@ -811,8 +811,8 @@ def target_variables(target):
os_arch = target.kernel

return namespace(
OS_TARGET=os_target,
OS_ARCH=os_arch,
OS_TARGET=str(os_target),
OS_ARCH=str(os_arch),
INTEL_ARCHITECTURE=target.cpu in ("x86", "x86_64") or None,
)

Expand Down
18 changes: 1 addition & 17 deletions configure.py
Original file line number Diff line number Diff line change
Expand Up @@ -222,23 +222,6 @@ def sanitize_config(v):
print("Please file a bug for the above.", file=sys.stderr)
sys.exit(1)

# Some values in sanitized_config also have more complex types, such as
# EnumString, which using when calling config_status would currently
# break the build, as well as making it inconsistent with re-running
# config.status, for which they are normalized to plain strings via
# indented_repr. Likewise for non-dict non-string iterables being
# converted to lists.
def normalize(obj):
if isinstance(obj, dict):
return {k: normalize(v) for k, v in six.iteritems(obj)}
if isinstance(obj, six.text_type):
return six.text_type(obj)
if isinstance(obj, Iterable):
return [normalize(o) for o in obj]
return obj

sanitized_config = normalize(sanitized_config)

# Create config.status. Eventually, we'll want to just do the work it does
# here, when we're able to skip configure tests/use cached results/not rely
# on autoconf.
Expand All @@ -248,6 +231,7 @@ def normalize(obj):
"""\
#!%(python)s
# coding=utf-8
from mozbuild.configure.constants import *
"""
)
% {"python": config["PYTHON3"]}
Expand Down
2 changes: 1 addition & 1 deletion media/libpng/moz.build
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ if CONFIG['INTEL_ARCHITECTURE']:
'intel/intel_init.c'
]

if CONFIG['CPU_ARCH'] == 'mips':
if CONFIG['CPU_ARCH'] in ('mips32', 'mips64'):
DEFINES['MOZ_PNG_USE_MIPS_MSA'] = True
UNIFIED_SOURCES += [
'mips/filter_msa_intrinsics.c',
Expand Down
102 changes: 59 additions & 43 deletions python/mozbuild/mozbuild/configure/constants.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,38 +6,45 @@

from mozbuild.util import EnumString

CompilerType = EnumString.subclass(
"clang",
"clang-cl",
"gcc",
"msvc",
)

OS = EnumString.subclass(
"Android",
"DragonFly",
"FreeBSD",
"GNU",
"NetBSD",
"OpenBSD",
"OSX",
"SunOS",
"WINNT",
"WASI",
)
class CompilerType(EnumString):
POSSIBLE_VALUES = (
"clang",
"clang-cl",
"gcc",
"msvc",
)


class OS(EnumString):
POSSIBLE_VALUES = (
"Android",
"DragonFly",
"FreeBSD",
"GNU",
"NetBSD",
"OpenBSD",
"OSX",
"SunOS",
"WINNT",
"WASI",
)


class Kernel(EnumString):
POSSIBLE_VALUES = (
"Darwin",
"DragonFly",
"FreeBSD",
"kFreeBSD",
"Linux",
"NetBSD",
"OpenBSD",
"SunOS",
"WINNT",
"WASI",
)

Kernel = EnumString.subclass(
"Darwin",
"DragonFly",
"FreeBSD",
"kFreeBSD",
"Linux",
"NetBSD",
"OpenBSD",
"SunOS",
"WINNT",
"WASI",
)

CPU_bitness = {
"aarch64": 64,
Expand All @@ -62,22 +69,31 @@
"wasm32": 32,
}

CPU = EnumString.subclass(*CPU_bitness.keys())

Endianness = EnumString.subclass(
"big",
"little",
)
class CPU(EnumString):
POSSIBLE_VALUES = CPU_bitness.keys()

WindowsBinaryType = EnumString.subclass(
"win32",
"win64",
)

Abi = EnumString.subclass(
"msvc",
"mingw",
)
class Endianness(EnumString):
POSSIBLE_VALUES = (
"big",
"little",
)


class WindowsBinaryType(EnumString):
POSSIBLE_VALUES = (
"win32",
"win64",
)


class Abi(EnumString):
POSSIBLE_VALUES = (
"msvc",
"mingw",
)


# The order of those checks matter
CPU_preprocessor_checks = OrderedDict(
Expand Down
14 changes: 13 additions & 1 deletion python/mozbuild/mozbuild/frontend/gyp_reader.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
import os
import sys
import time
from collections.abc import Iterable

import gyp
import gyp.msvs_emulation
Expand Down Expand Up @@ -464,7 +465,18 @@ def __init__(
for name, _ in finder.find("*/supplement.gypi")
)

str_vars = dict(gyp_dir_attrs.variables)
# We need to normalize EnumStrings to strings because the gyp code is
# not guaranteed to like them.
def normalize(obj):
if isinstance(obj, dict):
return {k: normalize(v) for k, v in obj.items()}
if isinstance(obj, str): # includes EnumStrings
return str(obj)
if isinstance(obj, Iterable):
return [normalize(o) for o in obj]
return obj

str_vars = normalize(gyp_dir_attrs.variables)
str_vars["python"] = sys.executable
self._gyp_loader_future = executor.submit(
load_gyp, [path], "mozbuild", str_vars, includes, depth, params
Expand Down
3 changes: 2 additions & 1 deletion python/mozbuild/mozbuild/test/test_util.py
Original file line number Diff line number Diff line change
Expand Up @@ -824,7 +824,8 @@ def test_expand_variables(self):

class TestEnumString(unittest.TestCase):
def test_string(self):
CompilerType = EnumString.subclass("gcc", "clang", "clang-cl")
class CompilerType(EnumString):
POSSIBLE_VALUES = ("gcc", "clang", "clang-cl")

type = CompilerType("gcc")
self.assertEqual(type, "gcc")
Expand Down
8 changes: 2 additions & 6 deletions python/mozbuild/mozbuild/util.py
Original file line number Diff line number Diff line change
Expand Up @@ -1326,12 +1326,8 @@ def __ne__(self, other):
def __hash__(self):
return super(EnumString, self).__hash__()

@staticmethod
def subclass(*possible_values):
class EnumStringSubclass(EnumString):
POSSIBLE_VALUES = possible_values

return EnumStringSubclass
def __repr__(self):
return f"{self.__class__.__name__}({str(self)!r})"


def _escape_char(c):
Expand Down
2 changes: 1 addition & 1 deletion third_party/libsrtp/src/moz.build
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ DEFINES['NSS'] = 1
# falling back to the (presumably) slower-on-this-architecture but working
# code path. https://bugzilla.mozilla.org/show_bug.cgi?id=822380 has been filed
# to make the right and more performant fix and push it back upstream.
if CONFIG['CPU_ARCH'] in ('arm', 'x86', 'x86_64', 'mips', 'mips64'):
if CONFIG['CPU_ARCH'] in ('arm', 'x86', 'x86_64', 'mips32', 'mips64'):
DEFINES['CPU_CISC'] = 1
else:
# best guess
Expand Down

0 comments on commit 2fddbb7

Please sign in to comment.