Tools - makecorever.py explicit output argument (#9248)

* Tools - makecorever.py explicit output argument

Don't hide where the file is actually placed
Using pathlib instead of raw strings to simplify path ops
Plus, add stdout preview when output is missing

Clean-up git-describe <-> version availility checks

* typo

* fix unhandled exc in git cmd

* unused escape

* RELEASE using same values

cores/esp8266/Esp-version.cpp

@@ -20,10 +20,11 @@
 
 #include <Arduino.h>
 #include <user_interface.h>
-#include <core_version.h>
 #include <lwipopts.h>       // LWIP_HASH_STR (lwip2)
 #include <bearssl/bearssl_git.h>  // BEARSSL_GIT short hash
 
+#include <core_version.h>
+
 #define STRHELPER(x) #x
 #define STR(x) STRHELPER(x) // stringifier
 

cores/esp8266/Esp.cpp

@@ -254,7 +254,7 @@ String EspClass::getCoreVersion()
         return String(core_release);
     }
     char buf[12];
-    snprintf(buf, sizeof(buf), "%08x", core_version);
+    snprintf(buf, sizeof(buf), PSTR("%08x"), core_version);
     return String(buf);
 }
 

platform.txt

@@ -121,7 +121,7 @@ compiler.elf2hex.extra_flags=
 ## needs git
 recipe.hooks.sketch.prebuild.pattern="{runtime.tools.python3.path}/python3" -I "{runtime.tools.signing}" --mode header --publickey "{build.source.path}/public.key" --out "{build.path}/core/Updater_Signing.h"
 # This is quite a working hack. This form of prebuild hook, while intuitive, is not explicitly documented.
-recipe.hooks.prebuild.1.pattern="{runtime.tools.python3.path}/python3" -I "{runtime.tools.makecorever}" --build_path "{build.path}" --platform_path "{runtime.platform.path}" --version "{version}"
+recipe.hooks.prebuild.1.pattern="{runtime.tools.python3.path}/python3" -I "{runtime.tools.makecorever}" --git-root "{runtime.platform.path}" --version "{version}" "{build.path}/core/core_version.h"
 
 # Handle processing sketch global options
 recipe.hooks.prebuild.2.pattern="{runtime.tools.python3.path}/python3" -I "{runtime.tools.mkbuildoptglobals}" "{runtime.ide.path}" {runtime.ide.version} "{build.path}" "{build.opt.fqfn}" "{globals.h.source.fqfn}" "{commonhfile.fqfn}" {mkbuildoptglobals.extra_flags}

tools/makecorever.py

@@ -18,25 +18,59 @@
 # along with this program.  If not, see <https://www.gnu.org/licenses/>.
 
 import argparse
-import os
+import pathlib
 import subprocess
+import sys
+
+from typing import Optional, TextIO
 
 
-def generate(path, platform_path, version="unspecified", release = False):
+PWD = pathlib.Path(__file__).parent.absolute()
-    def git(*args):
-        cmd = ["git", "-C", platform_path]
-        cmd.extend(args)
-        proc = subprocess.Popen(cmd, stdout=subprocess.PIPE, universal_newlines=True, stderr=subprocess.DEVNULL)
-        return proc.stdout.readlines()[0].strip()
 
-    text = ""
+VERSION_UNSPECIFIED = "unspecified"
+VERSION_DEFAULT = ("0", "0", "0")
+
+
+def check_git(*args: str, cwd: Optional[str]):
+    cmd = ["git"]
+    if cwd:
+        cmd.extend(["-C", cwd])
+    cmd.extend(args)
 
-    git_ver = "00000000"
     try:
-        git_ver = git("rev-parse", "--short=8", "HEAD")
+        with subprocess.Popen(
-    except Exception:
+            cmd,
+            stdout=subprocess.PIPE,
+            universal_newlines=True,
+            stderr=subprocess.DEVNULL,
+        ) as proc:
+            if proc.stdout:
+                lines = proc.stdout.readlines()
+                return lines[0].strip()
+    except IndexError:
         pass
-    text = "#define ARDUINO_ESP8266_GIT_VER   0x{}\n".format(git_ver)
+    except FileNotFoundError:
+        pass
+
+    return ""
+
+
+def generate(
+    out: TextIO,
+    *,
+    git_root: pathlib.Path,
+    hash_length: int = 8,
+    release: bool,
+    version: str,
+):
+    git_root = git_root.absolute()
+    git_cwd = git_root.as_posix()
+
+    def git(*args):
+        return check_git(*args, cwd=git_cwd)
+
+    git_ver = "0" * hash_length
+    git_ver = git("rev-parse", f"--short={hash_length}", "HEAD") or git_ver
 
     # version is
     # - using Arduino-CLI:
@@ -45,75 +79,99 @@ def generate(path, platform_path, version="unspecified", release = False):
     # - using git:
     #   - 5.6.7            (from release script, official release)
     #   - 5.6.7-42-g00d1e5 (from release script, test release)
-    git_desc = version
-    try:
-        # in any case, get a better version when git is around
-        git_desc = git("describe", "--tags")
-    except Exception:
-        pass
 
-    text += "#define ARDUINO_ESP8266_GIT_DESC  {}\n".format(git_desc)
+    # in any case, get a better version when git is around
-    text += "#define ARDUINO_ESP8266_VERSION   {}\n".format(version)
+    git_desc = git("describe", "--tags") or version
-    text += "\n"
 
-    version_split = version.split(".")
+    if version == VERSION_UNSPECIFIED:
-    # major: if present, skip "unix-" in "unix-3"
+        version = git_desc
-    text += "#define ARDUINO_ESP8266_MAJOR     {}\n".format(version_split[0].split("-")[-1])
-    text += "#define ARDUINO_ESP8266_MINOR     {}\n".format(version_split[1])
-    # revision can be ".n" or ".n-dev" or ".n-42-g00d1e5"
-    revision = version_split[2].split("-")
-    text += "#define ARDUINO_ESP8266_REVISION  {}\n".format(revision[0])
-    text += "\n"
 
-    # release or dev ?
+    version_triple = list(VERSION_DEFAULT)
+
+    if version != VERSION_UNSPECIFIED:
+        try:
+            version_triple = version.split(".", 2)
+        except ValueError:
+            pass
+
+    major, minor, patch = version_triple
+
+    major = major.split("-")[-1]
+    revision = patch.split("-")[0]
+
+    text = rf"""// ! ! ! DO NOT EDIT, AUTOMATICALLY GENERATED ! ! !
+#define ARDUINO_ESP8266_GIT_VER   0x{git_ver}
+#define ARDUINO_ESP8266_GIT_DESC  {git_desc}
+#define ARDUINO_ESP8266_VERSION   {version}
+
+#define ARDUINO_ESP8266_MAJOR     {major}
+#define ARDUINO_ESP8266_MINOR     {minor}
+#define ARDUINO_ESP8266_REVISION  {revision}
+"""
     if release:
-        text += "#define ARDUINO_ESP8266_RELEASE   \"{}\"\n".format(git_desc)
+        text += rf"""
-        text += "#define ARDUINO_ESP8266_RELEASE_{}\n".format(git_desc.replace("-","_").replace(".","_"))
+#define ARDUINO_ESP8266_RELEASE   \"{major}.{minor}.{revision}\"
+#define ARDUINO_ESP8266_RELEASE_{major}_{minor}_{revision}
+"""
     else:
-        text += "#define ARDUINO_ESP8266_DEV       1 // development version\n"
+        text += """
+#define ARDUINO_ESP8266_DEV       1 // development version
+"""
 
-    try:
+    out.write(text)
-        with open(path, "r") as inp:
-            old_text = inp.read()
-        if old_text == text:
-            return
-    except Exception:
-        pass
-
-    with open(path, "w") as out:
-        out.write(text)
 
 
 if __name__ == "__main__":
     parser = argparse.ArgumentParser(description="Generate core_version.h")
 
     parser.add_argument(
-        "-b", "--build_path", action="store", required=True, help="build.path variable"
+        "--git-root",
-    )
-    parser.add_argument(
-        "-p",
-        "--platform_path",
         action="store",
-        required=True,
+        help="ESP8266 Core Git root. In platform.txt, this is {platform.path}",
-        help="platform.path variable",
+        type=pathlib.Path,
+        default=PWD / "..",
     )
     parser.add_argument(
-        "-v", "--version", action="store", required=True, help="version variable"
+        "--hash-length",
+        default=8,
+        type=int,
+        help="Used in git rev-parse --short=...",
+    )
+    parser.add_argument(
+        "--version",
+        action="store",
+        default=VERSION_UNSPECIFIED,
+        help="ESP8266 Core version string. In platform.txt, this is {version}",
+    )
+    parser.add_argument(
+        "--release",
+        action="store_true",
+        default=False,
+        help="In addition to numeric version, also provide ARDUINO_ESP8266_RELEASE{,_...} definitions",
+    )
+    parser.add_argument(
+        "output",
+        nargs="?",
+        type=str,
+        default="",
     )
-    parser.add_argument("-i", "--include_dir", default="core")
-    parser.add_argument("-r", "--release", action="store_true", default=False)
 
     args = parser.parse_args()
 
-    include_dir = os.path.join(args.build_path, args.include_dir)
+    def select_output(s: str) -> TextIO:
-    try:
+        if not s:
-        os.makedirs(include_dir)
+            return sys.stdout
-    except Exception:
-        pass
 
-    generate(
+        out = pathlib.Path(s)
-        os.path.join(include_dir, "core_version.h"),
+        out.parent.mkdir(parents=True, exist_ok=True)
-        args.platform_path,
+
-        version=args.version,
+        return out.open("w", encoding="utf-8")
-        release=args.release
+
-    )
+    with select_output(args.output) as out:
+        generate(
+            out,
+            git_root=args.git_root,
+            hash_length=args.hash_length,
+            release=args.release,
+            version=args.version,
+        )

tools/platformio-build.py

@@ -409,18 +409,20 @@ def platform_txt_version(default):
 
 
 if isdir(join(FRAMEWORK_DIR, ".git")):
-    cmd = '"$PYTHONEXE" "{script}" -b "$BUILD_DIR" -p "{framework_dir}" -v {version}'
+    out = join("$BUILD_DIR", "core", "core_version.h")
+
+    cmd = '"$PYTHONEXE" "{script}" --git-root "{framework_dir}" --version {version} "$TARGET"'
     fmt = {
         "script": join(FRAMEWORK_DIR, "tools", "makecorever.py"),
         "framework_dir": FRAMEWORK_DIR,
-        "version": platform_txt_version("unspecified")
+        "version": platform_txt_version("unspecified"),
     }
 
     env.Prepend(CPPPATH=[
         join("$BUILD_DIR", "core")
     ])
     core_version = env.Command(
-        join("$BUILD_DIR", "core", "core_version.h"),
+        out,
         join(FRAMEWORK_DIR, ".git"),
         env.VerboseAction(cmd.format(**fmt), "Generating $TARGET")
     )