From f0d04b09e8b60d0c9c5efe1c24fc63d06cc54e7b Mon Sep 17 00:00:00 2001
From: Fabio Valentini <decathorpe@gmail.com>
Date: Jul 25 2022 10:09:43 +0000
Subject: Ensure rust2rpm doesn't autogenerate architecture-dependent patches


The logic in the cfg-expression evaluator checked *too many cases*,
some of which would have resulted in generation of broken patches
and packages. We need to ensure that the evaluation is independent
of the host architecture (i.e. we need to keep *all* dependencies
that are valid on *any* of our build targets, even if they end up
being unused on *some* of them).

---

diff --git a/rust2rpm/cfg.py b/rust2rpm/cfg.py
index b2e0944..51e5ff9 100644
--- a/rust2rpm/cfg.py
+++ b/rust2rpm/cfg.py
@@ -1,8 +1,5 @@
 import ast
-import ctypes
 import functools
-import platform
-import sys
 
 import pyparsing as pp
 from pyparsing import ParseException
@@ -63,43 +60,69 @@ def cfg_grammar():
 
 
 @functools.cache
-def evaluate_variable(name):
+def evaluate_predicate(name: str, value: str) -> bool:
+    # based on: https://doc.rust-lang.org/reference/conditional-compilation.html
+
     match name:
         case "target_arch":
-            return platform.machine()
+            # Needs to be ignored, as we cannot generate patches that are
+            # different depending on the host architecture - except if the
+            # target architecture is "wasm32", which we don't support.
+            return value != "wasm32"
+
+        case "target_feature":
+            # The "target_feature" predicate can be ignored as well, since the
+            # valid values for this predicate are architecture-dependent.
+            return True
 
         case "target_os":
-            return "linux"
+            return value == "linux"
 
         case "target_family":
-            return "unix"
-
-        case "unix":
-            return evaluate_variable("target_family") == "unix"
-
-        case "windows":
-            return evaluate_variable("target_family") == "windows"
+            return value == "unix"
 
         case "target_env":
-            # Key-value option set with further disambiguating information about the
-            # target platform with information about the ABI or libc used
-            return ...
+            # The "target_env" predicate is used to disambiguate target
+            # platforms based on its C library / C ABI (i.e. we can ignore
+            # "msvc" and "musl"), and if there's no need to disambiguate, the
+            # value can be the empty string.
+            return value in ["", "gnu"]
 
         case "target_endian":
-            return sys.byteorder
+            # Needs to be ignored, as we cannot generate patches that are
+            # different depending on the host architecture.
+            return True
 
         case "target_pointer_width":
-            return str(ctypes.sizeof(ctypes.c_void_p) * 8)
+            # Needs to be ignored, as we cannot generate patches that are
+            # different depending on the host architecture.
+            return True
 
         case "target_vendor":
-            return "unknown"
+            # On linux systems, "target_vendor" is always "unknown".
+            return value == "unknown"
 
         case _:
-            log.warn(f"Ignoring unknown variable {name!r} in cfg-expression.")
+            log.warn(f'Ignoring invalid predicate \'"{name}" = "{value}"\' in cfg-expression.')
+            return False
+
+
+@functools.cache
+def evaluate_atom(name: str) -> bool:
+    match name:
+        case "unix":
+            return True
+        case "windows":
+            return False
+        case _:
+            log.warn(
+                f"Ignoring invalid atom {name!r} in cfg-expression. "
+                + 'Only "unix" and "windows" are valid names for atoms.'
+            )
             return False
 
 
-def evaluate(expr, nested=False):
+def evaluate(expr, nested=False) -> bool:
     if hasattr(expr, "asList"):
         expr = expr.asList()  # compat with pyparsing 2.7.x
     match expr:
@@ -113,11 +136,9 @@ def evaluate(expr, nested=False):
             return any(evaluate(arg, True) for arg in args)
         case [variable, value] if nested:
             v = ast.literal_eval(value)
-            x = evaluate_variable(variable)
-            return x == v
+            return evaluate_predicate(variable, v)
         case [variable] if nested:
-            x = evaluate_variable(variable)
-            return x
+            return evaluate_atom(variable)
         case _:
             raise ValueError
 
diff --git a/rust2rpm/tests/test_cfg.py b/rust2rpm/tests/test_cfg.py
index c00008a..5a2b314 100644
--- a/rust2rpm/tests/test_cfg.py
+++ b/rust2rpm/tests/test_cfg.py
@@ -21,7 +21,7 @@ def test_pyparsing_run_tests():
     [
         ('cfg(target_os = "macos")', False),
         ("cfg(any(foo, bar))", False),
-        ('cfg(all(unix, target_pointer_width = "16"))', False),
+        ('cfg(all(unix, target_pointer_width = "16"))', True),
         ("cfg(not(foo))", True),
         ("cfg(unix)", True),
         ("cfg(not(unix))", False),
@@ -36,7 +36,6 @@ def test_pyparsing_run_tests():
         ('cfg(all(target_os = "linux"))', True),
         ('cfg(any(target_os = "linux", target_os = "macos"))', True),
         ('cfg(any(target_pointer_width = "16", target_pointer_width = "32", target_pointer_width = "64"))', True),
-        ('cfg(all(target_pointer_width = "16", target_pointer_width = "32", target_pointer_width = "64"))', False),
     ],
 )
 def test_expressions(expr, expected):