Revert "zmake: Trim short version on full hashes"

This reverts commit 695b3c909c4846493eacd324e9c26933847372d1.

Reason for revert: Broke CQ

See https://ci.chromium.org/ui/p/chromeos/builders/postsubmit/trogdor-zephyr-postsubmit/314

Original change's description:
> zmake: Trim short version on full hashes
>
> The ec_version.h contains both a long build string and also a 31 char
> string. Change the short string to omit the commit count, and only
> include full hashes, i.e. hayato_v2.6-ec:5bd1aa,os:ade7b4 not
> hayato_v2.6.73670-ec:5bd1aa,os:
>
> Change test to be a real version string with hashes of the right length.
>
> BUG=None
> BRANCH=None
> TEST=./zephyr/zmake/run_tests.sh
>
> Change-Id: I18d73345036933594b8c74187796c3727f868629
> Signed-off-by: Jeremy Bettis <jbettis@google.com>
> Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/ec/+/3160464
> Tested-by: Jeremy Bettis <jbettis@chromium.org>
> Auto-Submit: Jeremy Bettis <jbettis@chromium.org>
> Commit-Queue: Jack Rosenthal <jrosenth@chromium.org>
> Reviewed-by: Jack Rosenthal <jrosenth@chromium.org>

Bug: None
Change-Id: Ib4282f761c514c0ca2da6249d38ccc9cba852f6d
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/ec/+/3172040
Auto-Submit: Jack Rosenthal <jrosenth@chromium.org>
Tested-by: Jack Rosenthal <jrosenth@chromium.org>
Commit-Queue: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>

4 files changed, 41 insertions, 104 deletions

diff --git a/zephyr/zmake/tests/test_version.py b/zephyr/zmake/tests/test_version.py
index b9d2ec0d4b..a238a8ac02 100644
--- a/zephyr/zmake/tests/test_version.py
+++ b/zephyr/zmake/tests/test_version.py
@@ -88,17 +88,17 @@ def _setup_example_repos(tmp_path):
 
 def test_version_string(tmp_path):
     project, zephyr_base, modules = _setup_example_repos(tmp_path)
-    assert version.get_version_string(project, zephyr_base, modules) == (
-        "prj_v2.6.4-ec:b5991f,os:377d26,mod1:02fd7a",
-        "prj_v2.6-ec:b5991f,os:377d26",
+    assert (
+        version.get_version_string(project, zephyr_base, modules)
+        == "prj_v2.6.4-ec:b5991f,os:377d26,mod1:02fd7a"
     )
 
 
 def test_version_string_static(tmp_path):
     project, zephyr_base, modules = _setup_example_repos(tmp_path)
-    assert version.get_version_string(project, zephyr_base, modules, static=True) == (
-        "prj_v2.6.0-STATIC",
-        "prj_v2.6.0-STATIC",
+    assert (
+        version.get_version_string(project, zephyr_base, modules, static=True)
+        == "prj_v2.6.0-STATIC"
     )
 
 
@@ -117,36 +117,17 @@ def fake_date():
         yield
 
 
-HEADER_VERSION_STR = (
-    "hayato_v2.6.73670-ec:5bd1aa,os:ade7b4,cmsis:c3bd20,hal_stm32:f8ff8d"
-)
-SHORT_HEADER_VERSION_STR = "hayato_v2.6-ec:5bd1aa,os:ade7b4"
+HEADER_VERSION_STR = "trogdor_v2.6.1004-cmsis:0dead0,hal_stm32:0beef0,os:ad00da"
 EXPECTED_HEADER = (
     "/* This file is automatically generated by zmake */\n"
-    '#define VERSION "hayato_v2.6.73670-ec:5bd1aa,os:ade7b4,cmsis:c3bd20,'
-    'hal_stm32:f8ff8d"\n'
-    '#define CROS_EC_VERSION32 "hayato_v2.6-ec:5bd1aa,os:ade7b4"\n'
-    '#define BUILDER "toukmond@pokey"\n'
-    '#define DATE "2021-06-28 03:18:53"\n'
-    '#define CROS_FWID_MISSING_STR "CROS_FWID_MISSING"\n'
-    "#define CROS_FWID32 CROS_FWID_MISSING_STR\n"
-)
-HEADER_VERSION_STR2 = (
-    "herobrine_npcx9_v2.6.73695-ec:7b0169,os:862339,cmsis:c3bd20,hal_stm32:f8ff8d"
-)
-SHORT_HEADER_VERSION_STR2 = "herobrine_npcx9_v2.6-ec:7b0169"
-EXPECTED_HEADER2 = (
-    "/* This file is automatically generated by zmake */\n"
-    '#define VERSION "herobrine_npcx9_v2.6.73695-ec:7b0169,os:862339,'
-    'cmsis:c3bd20,hal_stm32:f8ff8d"\n'
-    '#define CROS_EC_VERSION32 "herobrine_npcx9_v2.6-ec:7b0169"\n'
+    '#define VERSION "trogdor_v2.6.1004-cmsis:0dead0,hal_stm32:0beef0,os:ad00da"\n'
+    '#define CROS_EC_VERSION32 "trogdor_v2.6.1004-cmsis:0dead0,"\n'
     '#define BUILDER "toukmond@pokey"\n'
     '#define DATE "2021-06-28 03:18:53"\n'
     '#define CROS_FWID_MISSING_STR "CROS_FWID_MISSING"\n'
     "#define CROS_FWID32 CROS_FWID_MISSING_STR\n"
 )
 HEADER_VERSION_STR_STATIC = "trogdor_v2.6.0-STATIC"
-SHORT_HEADER_VERSION_STR_STATIC = "trogdor_v2.6.0-STATIC"
 EXPECTED_HEADER_STATIC = (
     "/* This file is automatically generated by zmake */\n"
     '#define VERSION "trogdor_v2.6.0-STATIC"\n'
@@ -158,32 +139,18 @@ EXPECTED_HEADER_STATIC = (
 )
 
 
-@pytest.mark.parametrize(
-    "version_str,short_version_str,expected_header,static",
-    [
-        (HEADER_VERSION_STR, SHORT_HEADER_VERSION_STR, EXPECTED_HEADER, False),
-        (HEADER_VERSION_STR2, SHORT_HEADER_VERSION_STR2, EXPECTED_HEADER2, False),
-        (
-            HEADER_VERSION_STR_STATIC,
-            SHORT_HEADER_VERSION_STR_STATIC,
-            EXPECTED_HEADER_STATIC,
-            True,
-        ),
-    ],
-)
-def test_header_gen(
-    fake_user_hostname,
-    fake_date,
-    tmp_path,
-    version_str,
-    short_version_str,
-    expected_header,
-    static,
-):
+def test_header_gen(fake_user_hostname, fake_date, tmp_path):
     # Test the simple case (static=False, no existing header).
     output_file = tmp_path / "ec_version.h"
-    version.write_version_header(version_str, short_version_str, output_file, static)
-    assert output_file.read_text() == expected_header
+    version.write_version_header(HEADER_VERSION_STR, output_file)
+    assert output_file.read_text() == EXPECTED_HEADER
+
+
+def test_header_gen_reproducible_build(tmp_path):
+    # With static=True this time.
+    output_file = tmp_path / "ec_version.h"
+    version.write_version_header(HEADER_VERSION_STR_STATIC, output_file, static=True)
+    assert output_file.read_text() == EXPECTED_HEADER_STATIC
 
 
 def test_header_gen_exists_not_changed(fake_user_hostname, fake_date, tmp_path):
@@ -191,15 +158,11 @@ def test_header_gen_exists_not_changed(fake_user_hostname, fake_date, tmp_path):
     output_file = tmp_path / "ec_version.h"
 
     # First time, write and record mtime.
-    version.write_version_header(
-        HEADER_VERSION_STR, SHORT_HEADER_VERSION_STR, output_file
-    )
+    version.write_version_header(HEADER_VERSION_STR, output_file)
     expected_mtime = output_file.stat().st_mtime
 
     # Do another write (contents should be unchanged).
-    version.write_version_header(
-        HEADER_VERSION_STR, SHORT_HEADER_VERSION_STR, output_file
-    )
+    version.write_version_header(HEADER_VERSION_STR, output_file)
 
     # Assert we didn't write again.
     assert output_file.stat().st_mtime == expected_mtime
@@ -210,18 +173,11 @@ def test_header_gen_exists_needs_changes(fake_user_hostname, fake_date, tmp_path
     output_file = tmp_path / "ec_version.h"
 
     # First time, write and save contents.
-    version.write_version_header(
-        HEADER_VERSION_STR, SHORT_HEADER_VERSION_STR, output_file
-    )
+    version.write_version_header(HEADER_VERSION_STR, output_file)
     original_contents = output_file.read_text()
 
     # Do another write (contents should be changed).
-    version.write_version_header(
-        HEADER_VERSION_STR_STATIC,
-        SHORT_HEADER_VERSION_STR_STATIC,
-        output_file,
-        static=True,
-    )
+    version.write_version_header(HEADER_VERSION_STR_STATIC, output_file, static=True)
 
     # Assert we overwrote.
     assert output_file.read_text() != original_contents
diff --git a/zephyr/zmake/tests/test_zmake.py b/zephyr/zmake/tests/test_zmake.py
index e57a4f5ed3..641f9f3db9 100644
--- a/zephyr/zmake/tests/test_zmake.py
+++ b/zephyr/zmake/tests/test_zmake.py
@@ -141,7 +141,7 @@ EXTRAVERSION =
 """
                 )
             zephyr_base.resolve = mock.Mock(return_value=pathlib.Path(tmpname))
-            with patch("zmake.version.get_version_string", return_value=("123", "123")):
+            with patch("zmake.version.get_version_string", return_value="123"):
                 with patch.object(zmake.project, "Project", return_value=FakeProject()):
                     if use_configure:
                         zmk.configure(
diff --git a/zephyr/zmake/zmake/version.py b/zephyr/zmake/zmake/version.py
index c5506e9921..47aba6d804 100644
--- a/zephyr/zmake/zmake/version.py
+++ b/zephyr/zmake/zmake/version.py
@@ -86,14 +86,12 @@ def get_version_string(project, zephyr_base, modules, static=False):
             commits.
 
     Returns:
-        A tuple of version string which can be placed in FRID, FWID, or used in
-        the build for the OS, and a shortened string which is 31 chars or less.
+        A version string which can be placed in FRID, FWID, or used in
+        the build for the OS.
     """
     major_version, minor_version, *_ = util.read_zephyr_version(zephyr_base)
     project_id = project.project_dir.parts[-1]
     num_commits = 0
-    # Omit num_commits from short version string.
-    short_version_string = "{}_v{}.{}".format(project_id, major_version, minor_version)
 
     if static:
         vcs_hashes = "STATIC"
@@ -106,38 +104,23 @@ def get_version_string(project, zephyr_base, modules, static=False):
         for repo in repos.values():
             num_commits += _get_num_commits(repo)
 
-        vcs_parts = []
-        for name, repo in sorted(
-            repos.items(),
-            # Put the EC module first, then Zephyr OS kernel, as
-            # these are probably the most important hashes to
-            # developers.
-            key=lambda p: (p[0] != "ec", p[0] != "os", p),
-        ):
-            # Add hashes to vcs_hashes and short_version_string if it is still
-            # short enough.
-            new_short_ver = short_version_string
-            val = "{}:{}".format(name, _get_revision(repo)[:6])
-            vcs_parts.append(val)
-            vcs_hashes = ",".join(vcs_parts)
-            new_short_ver = "{}_v{}.{}-{}".format(
-                project_id, major_version, minor_version, vcs_hashes
+        vcs_hashes = ",".join(
+            "{}:{}".format(name, _get_revision(repo)[:6])
+            for name, repo in sorted(
+                repos.items(),
+                # Put the EC module first, then Zephyr OS kernel, as
+                # these are probably the most important hashes to
+                # developers.
+                key=lambda p: (p[0] != "ec", p[0] != "os", p),
             )
+        )
 
-            if len(new_short_ver) <= 31:
-                short_version_string = new_short_ver
-
-    long_version_string = "{}_v{}.{}.{}-{}".format(
+    return "{}_v{}.{}.{}-{}".format(
         project_id, major_version, minor_version, num_commits, vcs_hashes
     )
-    # If the long version string is short enough use it.
-    if len(long_version_string) <= 31:
-        short_version_string = long_version_string
-    assert len(short_version_string) <= 31
-    return long_version_string, short_version_string
 
 
-def write_version_header(version_str, short_version_str, output_path, static=False):
+def write_version_header(version_str, output_path, static=False):
     """Generate a version header and write it to the specified path.
 
     Generate a version header in the format expected by the EC build
@@ -165,7 +148,7 @@ def write_version_header(version_str, short_version_str, output_path, static=Fal
         output.write("#define {} {}\n".format(name, value))
 
     add_def("VERSION", version_str)
-    add_def("CROS_EC_VERSION32", short_version_str)
+    add_def("CROS_EC_VERSION32", version_str[:31])
 
     if static:
         add_def("BUILDER", "reproducible@build")
diff --git a/zephyr/zmake/zmake/zmake.py b/zephyr/zmake/zmake/zmake.py
index 34e25cc365..1b60e66f66 100644
--- a/zephyr/zmake/zmake/zmake.py
+++ b/zephyr/zmake/zmake/zmake.py
@@ -376,7 +376,7 @@ class Zmake:
         project = zmake.project.Project(build_dir / "project")
 
         # Compute the version string.
-        version_string, short_version_string = zmake.version.get_version_string(
+        version_string = zmake.version.get_version_string(
             project,
             build_dir / "zephyr_base",
             zmake.modules.locate_from_directory(build_dir / "modules"),
@@ -387,7 +387,6 @@ class Zmake:
         # configure was run.
         zmake.version.write_version_header(
             version_string,
-            short_version_string,
             build_dir / "include" / "ec_version.h",
         )
 
@@ -443,7 +442,7 @@ class Zmake:
         if output_files_out is None:
             output_files_out = []
         for output_file, output_name in project.packer.pack_firmware(
-            packer_work_dir, self.jobserver, version_string=short_version_string, **dirs
+            packer_work_dir, self.jobserver, version_string=version_string, **dirs
         ):
             shutil.copy2(output_file, output_dir / output_name)
             self.logger.debug("Output file '%s' created.", output_file)
@@ -592,7 +591,7 @@ class Zmake:
             return rv
 
         # Compute the version string.
-        version_string, short_version_string = zmake.version.get_version_string(
+        version_string = zmake.version.get_version_string(
             project,
             build_dir / "zephyr_base",
             zmake.modules.locate_from_directory(build_dir / "modules"),
@@ -603,7 +602,6 @@ class Zmake:
         # configure was run.
         zmake.version.write_version_header(
             version_string,
-            short_version_string,
             build_dir / "include" / "ec_version.h",
         )