Fix cmake generators on Windows and enable in pre-commit. (#17619)
Progress on https://github.com/iree-org/iree/issues/17430
These generator scripts are writing cross platform build system files,
so they should not use platform-dependent paths (with the `/` separator
on Linux/macOS and the `\` separator on Windows). They maybe shouldn't
even use [pathlib](https://docs.python.org/3/library/pathlib.html) at
all.
diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml
index d23608f..132b2e4 100644
--- a/.github/workflows/lint.yml
+++ b/.github/workflows/lint.yml
@@ -4,8 +4,6 @@
# See https://llvm.org/LICENSE.txt for license information.
# SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
-# Keep this in sync with build_tools/scripts/lint.sh
-
name: Lint
on: [pull_request]
@@ -23,18 +21,3 @@
uses: actions/setup-python@d27e3f3d7c64b4bbf8e4abfb9b63b83e846e0435 # v4.5.0
- name: Running pre-commit
uses: pre-commit/action@2c7b3805fd2a0fd8c1884dcaebf91fc102a13ecd # v3.0.1
-
- generated_cmake_files:
- runs-on: ubuntu-20.04
- steps:
- - name: Checking out repository
- uses: actions/checkout@v4.1.7
- - name: Generating CMake files
- run: |
- ./build_tools/scripts/generate_cmake_files.sh
- - name: Checking Diff
- run: |
- # Make sure to run build_tools/scripts/generate_cmake_files.sh and
- # pick up new files that have been added.
- git add -A
- git diff HEAD --exit-code
diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml
index b673dc3..da192e1 100644
--- a/.pre-commit-config.yaml
+++ b/.pre-commit-config.yaml
@@ -107,23 +107,18 @@
# TODO(scotttodd): mypy type checking for Python (https://mypy-lang.org/)
- # TODO(scotttodd): enable these checks when they work on Windows
- # the generator scripts use \ on Windows instead of /
+ - id: generate_cmake_e2e_test_artifacts_suite
+ name: Run generate_cmake_e2e_test_artifacts_suite.py
+ language: python
+ entry: ./build_tools/testing/generate_cmake_e2e_test_artifacts_suite.py
+ args: ["--output_dir", "./tests/e2e/test_artifacts"]
+ files: '^(build_tools/python|build_tools/testing|tests/e2e/test_artifacts)/.*'
+ pass_filenames: false
- # - id: generate_cmake_e2e_test_artifacts_suite
- # name: Run generate_cmake_e2e_test_artifacts_suite.py
- # language: python
- # entry: ./build_tools/testing/generate_cmake_e2e_test_artifacts_suite.py
- # args: ["--output_dir", "./tests/e2e/test_artifacts"]
- # # TODO(scotttodd): run only on relevant files
- # always_run: true
- # pass_filenames: false
-
- # - id: generate_cmake_e2e_model_tests
- # name: Run generate_cmake_e2e_model_tests.py
- # language: python
- # entry: ./build_tools/testing/generate_cmake_e2e_model_tests.py
- # args: ["--output", "./tests/e2e/stablehlo_models/generated_e2e_model_tests.cmake"]
- # # TODO(scotttodd): run only on relevant files
- # always_run: true
- # pass_filenames: false
+ - id: generate_cmake_e2e_model_tests
+ name: Run generate_cmake_e2e_model_tests.py
+ language: python
+ entry: ./build_tools/testing/generate_cmake_e2e_model_tests.py
+ args: ["--output", "./tests/e2e/stablehlo_models/generated_e2e_model_tests.cmake"]
+ files: '^(build_tools/python|build_tools/testing|tests/e2e/stablehlo_models)/.*'
+ pass_filenames: false
diff --git a/build_tools/python/e2e_model_tests/cmake_generator.py b/build_tools/python/e2e_model_tests/cmake_generator.py
index 3a5e8b0..3274424 100644
--- a/build_tools/python/e2e_model_tests/cmake_generator.py
+++ b/build_tools/python/e2e_model_tests/cmake_generator.py
@@ -25,7 +25,7 @@
module_path = (
iree_artifacts.get_module_dir_path(gen_config)
/ iree_artifacts.MODULE_FILENAME
- )
+ ).as_posix()
all_module_path_map[
(gen_config.imported_model.composite_id, gen_config.compile_config.id)
] = module_path
diff --git a/build_tools/python/e2e_test_artifacts/cmake_generator/iree_rule_generator.py b/build_tools/python/e2e_test_artifacts/cmake_generator/iree_rule_generator.py
index 82e4b3b..fbf4c53 100644
--- a/build_tools/python/e2e_test_artifacts/cmake_generator/iree_rule_generator.py
+++ b/build_tools/python/e2e_test_artifacts/cmake_generator/iree_rule_generator.py
@@ -74,18 +74,18 @@
cmake_rules = [
cmake_builder.rules.build_iree_import_tflite_model(
target_path=self.build_target_path(target_name),
- source=str(source_model_rule.file_path),
+ source=str(source_model_rule.file_path.as_posix()),
import_flags=import_flags,
- output_mlir_file=str(output_file_path),
+ output_mlir_file=str(output_file_path.as_posix()),
)
]
elif import_config.tool == iree_definitions.ImportTool.TF_IMPORTER:
cmake_rules = [
cmake_builder.rules.build_iree_import_tf_model(
target_path=self.build_target_path(target_name),
- source=str(source_model_rule.file_path),
+ source=str(source_model_rule.file_path.as_posix()),
import_flags=import_flags,
- output_mlir_file=str(output_file_path),
+ output_mlir_file=str(output_file_path.as_posix()),
)
]
else:
@@ -117,8 +117,8 @@
cmake_rules = [
cmake_builder.rules.build_iree_bytecode_module(
target_name=target_name,
- src=str(model_import_rule.output_file_path),
- module_name=str(output_file_path),
+ src=str(model_import_rule.output_file_path.as_posix()),
+ module_name=str(output_file_path.as_posix()),
flags=compile_flags,
friendly_name=str(module_generation_config),
)
diff --git a/build_tools/python/e2e_test_artifacts/cmake_generator/model_rule_generator.py b/build_tools/python/e2e_test_artifacts/cmake_generator/model_rule_generator.py
index 37cc3d6..e121e09 100644
--- a/build_tools/python/e2e_test_artifacts/cmake_generator/model_rule_generator.py
+++ b/build_tools/python/e2e_test_artifacts/cmake_generator/model_rule_generator.py
@@ -39,7 +39,7 @@
cmake_builder.rules.build_iree_fetch_artifact(
target_name=target_name,
source_url=model.source_url,
- output=str(model_path),
+ output=str(model_path.as_posix()),
unpack=True,
)
]
diff --git a/build_tools/python/e2e_test_framework/definitions/iree_definitions.py b/build_tools/python/e2e_test_framework/definitions/iree_definitions.py
index e845bd5..ebf318c 100644
--- a/build_tools/python/e2e_test_framework/definitions/iree_definitions.py
+++ b/build_tools/python/e2e_test_framework/definitions/iree_definitions.py
@@ -271,9 +271,7 @@
"""Materialize flags with dependent values."""
def _replace_module_dir_placeholder(value: str) -> str:
- """Replaces ${MODULE_DIR} in a POSIX path and returns the
- platform-dependent path string.
- """
+ """Replaces ${MODULE_DIR} in a POSIX path and returns the new path."""
parts = pathlib.PurePosixPath(value).parts
if MODULE_DIR_VARIABLE not in parts:
return value
@@ -282,8 +280,7 @@
f"'{MODULE_DIR_VARIABLE}' needs to be the head of flag value"
f" if present, but got '{value}'."
)
- # Properly construct the platform-dependent path.
- return str(module_dir_path.joinpath(*parts[1:]))
+ return str(module_dir_path.joinpath(*parts[1:]).as_posix())
return utils.transform_flags(
flags=self.compile_flags, map_funcs=[_replace_module_dir_placeholder]
diff --git a/build_tools/python/e2e_test_framework/definitions/iree_definitions_test.py b/build_tools/python/e2e_test_framework/definitions/iree_definitions_test.py
index e497b34..bf4bdfb 100644
--- a/build_tools/python/e2e_test_framework/definitions/iree_definitions_test.py
+++ b/build_tools/python/e2e_test_framework/definitions/iree_definitions_test.py
@@ -192,7 +192,7 @@
module_dir_path=pathlib.Path("abc")
)
- expected_path = pathlib.Path("abc", "test.json")
+ expected_path = pathlib.Path("abc", "test.json").as_posix()
self.assertIn(f"--test={expected_path}", flags)
def test_materialize_compile_flags_invalid_module_dir_position(self):
diff --git a/build_tools/scripts/lint.sh b/build_tools/scripts/lint.sh
deleted file mode 100755
index 8881f65..0000000
--- a/build_tools/scripts/lint.sh
+++ /dev/null
@@ -1,65 +0,0 @@
-#!/bin/bash
-
-# Copyright 2021 The IREE Authors
-#
-# 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
-
-# Runs all the lint checks that we run on GitHub locally. Will fail if any tool
-# is missing.
-
-# Keep this in sync with .github/workflows/lint.yml
-
-# WARNING: this script *makes changes* to the working directory and the index.
-
-set -uo pipefail
-
-FINAL_RET=0
-PREV_CMD=""
-
-scripts_dir="$(dirname $0)"
-
-function update_ret() {
- local prev_ret="$?"
- local cur_cmd="${BASH_COMMAND}"
- if [[ "${prev_ret}" -gt "${FINAL_RET}" ]]; then
- FINAL_RET="${prev_ret}"
- FAILING_CMD="${PREV_CMD}"
- fi
- PREV_CMD="${cur_cmd}"
-}
-
-# Analyze the exit code of the previous command before every command
-trap update_ret DEBUG
-
-echo "***** Uncommitted changes *****"
-git add -A
-git diff HEAD --exit-code
-
-if [[ $? -ne 0 ]]; then
- echo "Found uncomitted changes in working directory. This script requires" \
- "all changes to be committed. All changes have been added to the" \
- "index. Please commit or clean all changes and try again."
- exit 1
-fi
-
-echo "***** pre-commit *****"
-pre-commit run
-
-echo "***** buildifier *****"
-${scripts_dir}/run_buildifier.sh
-git diff --exit-code
-
-echo "***** Generates CMake files *****"
-./build_tools/scripts/generate_cmake_files.sh
-git add -A
-git diff HEAD --exit-code
-
-if (( "${FINAL_RET}" != 0 )); then
- echo "Encountered failures when running: '${FAILING_CMD}'. Check error" \
- "messages and changes to the working directory and git index (which" \
- "may contain fixes) and try again."
-fi
-
-exit "${FINAL_RET}"
