[PATCH] D143295: [bazel] Move bazel configuration to a Python script

Aaron Siddhartha Mondal via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Feb 6 15:42:01 PST 2023


aaronmondal added a comment.

Can we use some kind of formatting/linting for this script, potentially at a later time, unrelated to this relocation commit?

If you run something like `wemake-python-styleguide` on this your eyes will start to bleed a little. I think for now the following edits would be nice for some basic PEP8 compliance:

  --- old.py        2023-02-07 00:29:01.552414476 +0100
  +++ new.py        2023-02-07 00:29:02.787444050 +0100
  @@ -65,7 +65,7 @@
           subprocess.run(cmd, check=True)
       except subprocess.CalledProcessError as e:
           cmd_str = " ".join([str(arg) for arg in cmd])
  -        print(f"Failed to execute overlay script: '{cmd}'\n" +
  +        print(f"Failed to execute overlay script: '{cmd_str}'\n" +
                 f"Exited with code {e.returncode}\n" +
                 f"stdout:\n{e.stdout}\n" +
                 f"stderr:\n{e.stderr}\n", file=sys.stderr)
  @@ -111,7 +111,7 @@
               # 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:
  +            if c[k] is not None:
                   continue
   
               # Pick up 1st word as the value.
  @@ -153,9 +153,9 @@
       vars = _extract_cmake_settings(llvm_cmake_path)
   
       _write_dict_to_file(
  -        filepath = os.path.join(args.dst, "vars.bzl"),
  -        header = "# Generated from {}\n\n".format(llvm_cmake),
  -        vars = vars,
  +        filepath=os.path.join(args.dst, "vars.bzl"),
  +        header="# Generated from {}\n\n".format(llvm_cmake),
  +        vars=vars,
       )
   
       # Create a starlark file with the requested LLVM targets.
  @@ -164,6 +164,7 @@
       with open(targets_bzl_path, "w") as tgt_file:
           print(f"llvm_targets = [{targets_list_str}]", end="", file=tgt_file)
   
  +
   if __name__ == "__main__":
  -  _check_python_version()
  -  main(parse_arguments())
  +    _check_python_version()
  +    main(parse_arguments())

Is there a particular reason behind `_overlay_directories` invoking a python subprocess instead of invoking the script in a starlark rule?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D143295/new/

https://reviews.llvm.org/D143295



More information about the llvm-commits mailing list