[Bazel] Use `LLVM_VERSION` from `llvm/CMakeLists.txt`

[chapuni]

Summary

• Generate //:vars.bzl from llvm/CMakeLists.txt

_extract_cmake_settings() generates //:vars.bzl in llvm_configure().
It would be easier to use external commands like sed(1) and python.
For portability, I think the parser should run on Starlark.

@llvm-project//:vars.bzl may be loaded from both WORKSPACE and BUILD.
At the moment, vars.bzl provides some values as string.

• CMAKE_CXX_STANDARD = "17"
• LLVM_VERSION_MAJOR = "16"
• LLVM_VERSION_MINOR = "0"
• LLVM_VERSION_PATCH = "0"
• LLVM_VERSION = "16.0.0"
• llvm_vars = (dict of these values)

CMAKE_CXX_STANDARD may be used to configure toolchain.

• Use //vars.bzl for each BUILD files

It would be smarter if the BUILD phase could generate llvm-config.h.
Since I am afraid of the discussion in D126581, I just remove
LLVM_VERSION stuff out of the static llvm-config.h.

• Eliminate Bazel stuff in 'bump-version.py'

Current version of bump-version.py tries to substitute CLANG_VERSION.
It is the reason why I modify bump-version in this change rather than
upcoming patches.

[MaskRay]

Thanks for improving the Bazel build. I unfortunately don't know enough about Bazel... but I can try finding some folks who can help.

For one thing, there are some style issues, especially whether an operator has adjacent space like if setfoo[1]!="(": => if setfoo[1] != "(":

[MaskRay]

utils/bazel/llvm-project-overlay/clang/BUILD.bazel

392

Let me try removing this cmake variable in D136660

[GMNGeoffrey]

Are there practical portability concerns about doing this in Python? I'm not aware of a platform where Bazel runs that Python doesn't. I do like pulling this directly from the CMake in the cases where the values are set there. I think there was discussion at some point of whether it would be easier if these values were encoded somewhere in a simple text file. Here we go: https://discourse.llvm.org/t/rfc-centralized-location-for-version-information/65295. Might be good to follow-up on where that went

[chapuni]

• Caught up recent changes
• Reformat
• Use string.format() if possible
• Eliminate Bazel stuff in 'bump-version.py'

[chapuni]

Thank you @GMNGeoffrey for the comment and I am sorry for the delay.

Are there practical portability concerns about doing this in Python? I'm not aware of a platform where Bazel runs that Python doesn't.

It is my preference. Still I think that external tools may be avoided as possible in repository rules.
At the moment I don't use other platforms like Windows. I haven't met any portability issues but I would like to prevent them.
In my experiences, Python has not been easy for portability, configurability, version and platforms.

(I could use platform-specific tools to implement platform-specific functionality. For example, to import repositories from *.deb or installed deb packages)

FYI, the prototype was using sed(1). It was simpler but I gave it up due to portability concern.

I know my implementation by Starlark is not smart but I expect it may be removed in the near future when we could introduce the centralized version database file.

[chapuni]

@thieta Could you confirm my changes in bump-version.py?

[thieta]

Thanks for making this change. I think the changes to the bump script looks fine if no processing is needed in the future.

[rupprecht]

This looks like a nice improvement! Just one thing I noticed.

utils/bazel/configure.bzl

146

I see a warning that vars is unused here and should be removed. Did you mean to have _extract_cmake_settings() only parse the information, and have a separate helper method that actually writes out the file, i.e. split the previous method at # Write out individual vars?

[chapuni]

• [Bazel] Split out _write_dict_to_file

[chapuni]

utils/bazel/configure.bzl

146

I just thought vars might be used later after _extract_cmake_settings in _llvm_configure_impl.

Did you mean to have _extract_cmake_settings() only parse the information, and have a separate helper method that actually writes out the file, i.e. split the previous method at # Write out individual vars?

I could just remove an assignment but it really makes sense.
I have introduced _write_dict_to_file.

[rupprecht]

LGTM

[MaskRay]

utils/bazel/configure.bzl

75

Nit: We don't add a blank line after def:

131

Nit: We don't add a blank line after def:

llvm/utils/release/bump-version.py

def process_line(self, line: str) -> str:
                     line,
                 )
                 if nline != line:
                     break
 
         return nline
 
 
-# Process the many bazel files.
-class BazelProcessor(Processor):
-    def process_line(self, line: str) -> str:
-        # This matches the CLANG_VERSION line of clang/Config/config.h
-        nline = line
-        if "CLANG_VERSION " in line:
-            nline = re.sub(
-                r"#define CLANG_VERSION (.*)'",
-                f"#define CLANG_VERSION {self.version_str(include_suffix=False)}'",
-                line,
-            )
-        # Match version strings of LLVM, Clang and LLD overlay headers
-        elif "LLVM_VERSION_STRING" in line or "CLANG_VERSION_STRING" in line or "LLD_VERSION_STRING" in line:
-            nline = re.sub(
-                r"#define (LLVM|CLANG|LLD)_VERSION_STRING ([\\\"]+)[0-9\.rcgit-]+([\\\"]+)",
-                rf"#define \g<1>_VERSION_STRING \g<2>{self.version_str()}\g<3>",
-                line,
-            )
-        # Match the split out MAJOR/MINOR/PATCH versions of LLVM and Clang overlay headers
-        # in LLVM the define is called _PATCH and in clang it's called _PATCHLEVEL
-        elif "LLVM_VERSION_" in line or "CLANG_VERSION_" in line:
-            for c, cver in (
-                ("(MAJOR)", self.major),
-                ("(MINOR)", self.minor),
-                ("(PATCH|PATCHLEVEL)", self.patch),
-            ):
-                nline = re.sub(
-                    fr"(LLVM|CLANG)_VERSION_{c} \d+",
-                    rf"\g<1>_VERSION_\g<2> {cver}",
-                    line,
-                )
-                if nline != line:
-                    break
-
-        return nline
-
-
 # GN build system
 class GNIProcessor(Processor):
     def process_line(self, line: str) -> str:
         if "llvm_version_" in line:
             for c, cver in (
                 ("major", self.major),
                 ("minor", self.minor),
                 ("patch", self.patch),
files_to_update = (
             "llvm/utils/lit/lit/__init__.py",
             LitProcessor(),
         ),
         # GN build system
         (
             "llvm/utils/gn/secondary/llvm/version.gni",
             GNIProcessor(),
         ),
-        # Bazel build system
-        (
-            "utils/bazel/llvm-project-overlay/llvm/include/llvm/Config/llvm-config.h",
-            BazelProcessor(),
-        ),
-        (
-            "utils/bazel/llvm-project-overlay/clang/BUILD.bazel",
-            BazelProcessor(),
-        ),
-        (
-            "utils/bazel/llvm-project-overlay/clang/include/clang/Config/config.h",
-            BazelProcessor(),
-        ),
-        (
-            "utils/bazel/llvm-project-overlay/lld/BUILD.bazel",
-            BazelProcessor(),
-        ),
         (
             "libcxx/include/__config",
             LibCXXProcessor(),
         ),
     )
 
     for f, processor in files_to_update:
         processor.process_file(source_root / Path(f), version)

utils/bazel/configure.bzl

if exec_result.return_code != 0:
               "stdout:\n{stdout}\n" +
               "stderr:\n{stderr}\n").format(
             cmd = " ".join([str(arg) for arg in cmd]),
             return_code = exec_result.return_code,
             stdout = exec_result.stdout,
             stderr = exec_result.stderr,
         ))
 
+def _extract_cmake_settings(repository_ctx, llvm_cmake):
+
+    # The list to be written to vars.bzl
+    # `CMAKE_CXX_STANDARD` may be used from WORKSPACE for the toolchain.
+    c = {
+        "CMAKE_CXX_STANDARD": None,
+        "LLVM_VERSION_MAJOR": None,
+        "LLVM_VERSION_MINOR": None,
+        "LLVM_VERSION_PATCH": None,
+    }
+
+    # It would be easier to use external commands like sed(1) and python.
+    # For portability, the parser should run on Starlark.
+    llvm_cmake_path = repository_ctx.path(Label("//:" + llvm_cmake))
+    for line in repository_ctx.read(llvm_cmake_path).splitlines():
+        # Extract "set ( FOO bar ... "
+        setfoo = line.partition("(")
+        if setfoo[1] != "(":
+            continue
+        if setfoo[0].strip().lower() != "set":
+            continue
+        # `kv` is assumed as \s*KEY\s+VAL\s*\).*
+        # Typical case is like
+        #   LLVM_REQUIRED_CXX_STANDARD 17)
+        # Possible case -- It should be ignored.
+        #   CMAKE_CXX_STANDARD ${...} CACHE STRING "...")
+        kv = setfoo[2].strip()
+        i = kv.find(" ")
+        if i < 0:
+            continue
+        k = kv[:i]
+        # Prefer LLVM_REQUIRED_CXX_STANDARD instead of CMAKE_CXX_STANDARD
+        if k == "LLVM_REQUIRED_CXX_STANDARD":
+            k = "CMAKE_CXX_STANDARD"
+            c[k] = None
+        if k not in c:
+            continue
+        # Skip if `CMAKE_CXX_STANDARD` is set with
+        # `LLVM_REQUIRED_CXX_STANDARD`.
+        # Then `v` will not be desired form, like "${...} CACHE"
+        if c[k] != None:
+            continue
+        # Pick up 1st word as the value.
+        # Note: It assumes unquoted word.
+        v = kv[i:].strip().partition(")")[0].partition(" ")[0]
+        c[k] = v
+
+    # Synthesize `LLVM_VERSION` for convenience.
+    c["LLVM_VERSION"] = "{}.{}.{}".format(
+        c["LLVM_VERSION_MAJOR"],
+        c["LLVM_VERSION_MINOR"],
+        c["LLVM_VERSION_PATCH"],
+    )
+
+    return c
+
+def _write_dict_to_file(repository_ctx, filepath, header, vars):
+
+    # (fci + individual vars) + (fcd + dict items) + (fct)
+    fci = header
+    fcd = "\nllvm_vars={\n"
+    fct = "}\n"
+
+    for k, v in vars.items():
+        fci += '{} = "{}"\n'.format(k, v)
+        fcd += '    "{}": "{}",\n'.format(k, v)
+
+    repository_ctx.file(filepath, content=fci + fcd + fct)
+
 def _llvm_configure_impl(repository_ctx):
     _overlay_directories(repository_ctx)
 
+    llvm_cmake = "llvm/CMakeLists.txt"
+    vars = _extract_cmake_settings(
+        repository_ctx,
+        llvm_cmake,
+    )
+
+    _write_dict_to_file(
+        repository_ctx,
+        filepath="vars.bzl",
+        header="# Generated from {}\n\n".format(llvm_cmake),
+        vars=vars,
+    )
+
     # Create a starlark file with the requested LLVM targets.
     targets = repository_ctx.attr.targets
     repository_ctx.file(
         "llvm/targets.bzl",
         content = "llvm_targets = " + str(targets),
         executable = False,
     )
 

utils/bazel/llvm-project-overlay/clang/BUILD.bazel

 # This file is licensed under the Apache License v2.0 with LLVM Exceptions.
 # See https://llvm.org/LICENSE.txt for license information.
 # SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 
 load("@bazel_skylib//rules:expand_template.bzl", "expand_template")
 load("//llvm:tblgen.bzl", "gentbl")
 load("//llvm:binary_alias.bzl", "binary_alias")
 load("//llvm:cc_plugin_library.bzl", "cc_plugin_library")
 
+load(
+    "//:vars.bzl",
+    "LLVM_VERSION",
+    "LLVM_VERSION_MAJOR",
+    "LLVM_VERSION_MINOR",
+    "LLVM_VERSION_PATCH",
+)
+
 package(
     default_visibility = ["//visibility:public"],
     features = ["layering_check"],
 )
 
 licenses(["notice"])
 
 exports_files([

 exports_files(
     glob(["include/**/*.td"]),
 )
 
 genrule(
     name = "basic_version_gen",
     outs = ["include/clang/Basic/Version.inc"],
     cmd = (
-        "echo '#define CLANG_VERSION 16.0.0' >> $@\n" +
-        "echo '#define CLANG_VERSION_MAJOR 16' >> $@\n" +
-        "echo '#define CLANG_VERSION_MAJOR_STRING \"16\"' >> $@\n" +
-        "echo '#define CLANG_VERSION_MINOR 0' >> $@\n" +
-        "echo '#define CLANG_VERSION_PATCHLEVEL 0' >> $@\n" +
-        "echo '#define CLANG_VERSION_STRING \"16.0.0\"' >> $@\n"
+        "echo '#define CLANG_VERSION {vers}' >> $@\n" +
+        "echo '#define CLANG_VERSION_MAJOR {major}' >> $@\n" +
+        "echo '#define CLANG_VERSION_MAJOR_STRING \"{major}\"' >> $@\n" +
+        "echo '#define CLANG_VERSION_MINOR {minor}' >> $@\n" +
+        "echo '#define CLANG_VERSION_PATCHLEVEL {patch}' >> $@\n" +
+        "echo '#define CLANG_VERSION_STRING \"{vers}\"' >> $@\n"
+    ).format(
+        vers=LLVM_VERSION,
+        major=LLVM_VERSION_MAJOR,
+        minor=LLVM_VERSION_MINOR,
+        patch=LLVM_VERSION_PATCH,
     ),
 )
 
 cc_library(
     name = "config",
     hdrs = [
         "include/clang/Basic/Version.inc",
         "include/clang/Config/config.h",
     ],
     includes = ["include"],
     deps = [
         # We rely on the LLVM config library to provide configuration defines.
         "//llvm:config",
     ],
 )
 
 # TODO: This should get replaced with something that actually generates the
 # correct version number.
 genrule(

utils/bazel/llvm-project-overlay/lld/BUILD.bazel

 # This file is licensed under the Apache License v2.0 with LLVM Exceptions.
 # See https://llvm.org/LICENSE.txt for license information.
 # SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 
 load("@bazel_skylib//rules:expand_template.bzl", "expand_template")
 load("//llvm:tblgen.bzl", "gentbl")
 
+load(
+    "//:vars.bzl",
+    "LLVM_VERSION",
+)
+
 package(
     default_visibility = ["//visibility:public"],
 )
 
 licenses(["notice"])
 
 # TODO: Actually compute version info
 genrule(
     name = "config_version_gen",
     outs = ["include/lld/Common/Version.inc"],
-    cmd = "echo '#define LLD_VERSION_STRING \"16.0.0\"' > $@",
+    cmd = "echo '#define LLD_VERSION_STRING \"{}\"' > $@".format(LLVM_VERSION),
 )
 
 genrule(
     name = "vcs_version_gen",
     outs = ["Common/VCSVersion.inc"],
     cmd = "echo '#undef LLD_REVISION' >> $@\n" +
           "echo '#undef LLD_REPOSITORY' >> $@\n",
 )

utils/bazel/llvm-project-overlay/llvm/config.bzl

 # This file is licensed under the Apache License v2.0 with LLVM Exceptions.
 # See https://llvm.org/LICENSE.txt for license information.
 # SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 
 """Defines variables that use selects to configure LLVM based on platform."""
 
+load(
+    "//:vars.bzl",
+    "LLVM_VERSION",
+    "LLVM_VERSION_MAJOR",
+    "LLVM_VERSION_MINOR",
+    "LLVM_VERSION_PATCH",
+)
+
 def native_arch_defines(arch, triple):
     return [
         r'LLVM_NATIVE_ARCH=\"{}\"'.format(arch),
         "LLVM_NATIVE_ASMPARSER=LLVMInitialize{}AsmParser".format(arch),
         "LLVM_NATIVE_ASMPRINTER=LLVMInitialize{}AsmPrinter".format(arch),
         "LLVM_NATIVE_DISASSEMBLER=LLVMInitialize{}Disassembler".format(arch),
         "LLVM_NATIVE_TARGET=LLVMInitialize{}Target".format(arch),
         "LLVM_NATIVE_TARGETINFO=LLVMInitialize{}TargetInfo".format(arch),
llvm_config_defines = os_defines + select({
     "@bazel_tools//src/conditions:windows": native_arch_defines("X86", "x86_64-pc-win32"),
     "@bazel_tools//src/conditions:darwin_arm64": native_arch_defines("AArch64", "arm64-apple-darwin"),
     "@bazel_tools//src/conditions:darwin_x86_64": native_arch_defines("X86", "x86_64-unknown-darwin"),
     "@bazel_tools//src/conditions:linux_aarch64": native_arch_defines("AArch64", "aarch64-unknown-linux-gnu"),
     "@bazel_tools//src/conditions:linux_ppc64le": native_arch_defines("PowerPC", "powerpc64le-unknown-linux-gnu"),
     "@bazel_tools//src/conditions:linux_s390x": native_arch_defines("SystemZ", "systemz-unknown-linux_gnu"),
     "//conditions:default": native_arch_defines("X86", "x86_64-unknown-linux-gnu"),
 }) + [
+    "LLVM_VERSION_MAJOR={}".format(LLVM_VERSION_MAJOR),
+    "LLVM_VERSION_MINOR={}".format(LLVM_VERSION_MINOR),
+    "LLVM_VERSION_PATCH={}".format(LLVM_VERSION_PATCH),
+    r'LLVM_VERSION_STRING=\"{}git\"'.format(LLVM_VERSION),
     # These shouldn't be needed by the C++11 standard, but are for some
     # platforms (e.g. glibc < 2.18. See
     # https://sourceware.org/bugzilla/show_bug.cgi?id=15366). These are also
     # included unconditionally in the CMake build:
     # https://github.com/llvm/llvm-project/blob/cd0dd8ece8e/llvm/cmake/modules/HandleLLVMOptions.cmake#L907-L909
     "__STDC_LIMIT_MACROS",
     "__STDC_CONSTANT_MACROS",
     "__STDC_FORMAT_MACROS",
 ]

utils/bazel/llvm-project-overlay/llvm/include/llvm/Config/llvm-config.h

 /*===------- llvm/Config/llvm-config.h - llvm configuration -------*- C -*-===*/
 /*                                                                            */
 /* Part of the LLVM Project, under the Apache License v2.0 with LLVM          */
 /* Exceptions.                                                                */
 /* See https://llvm.org/LICENSE.txt for license information.                  */
 /* SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception                    */
 /*                                                                            */
 /*===----------------------------------------------------------------------===*/

 
 /* Define if we have the oprofile JIT-support library */
 #define LLVM_USE_OPROFILE 0
 
 /* Define if we have the perf JIT-support library */
 #define LLVM_USE_PERF 0
 
 /* Major version of the LLVM API */
-#define LLVM_VERSION_MAJOR 16
+/* #undef LLVM_VERSION_MAJOR */
 
 /* Minor version of the LLVM API */
-#define LLVM_VERSION_MINOR 0
+/* #undef LLVM_VERSION_MINOR */
 
 /* Patch version of the LLVM API */
-#define LLVM_VERSION_PATCH 0
+/* #undef LLVM_VERSION_PATCH */
 
 /* LLVM version string */
-#define LLVM_VERSION_STRING "16.0.0git"
+/* #undef LLVM_VERSION_STRING */
 
 /* Whether LLVM records statistics for use with GetStatistics(),
  * PrintStatistics() or PrintStatisticsJSON()
  */
 #define LLVM_FORCE_ENABLE_STATS 0
 
 /* Define if we have z3 and want to build it */
 /* #undef LLVM_WITH_Z3 */