Skip to content

Fix release runtime package.#9118

Merged
stellaraccident merged 1 commit intoiree-org:mainfrom
stellaraccident:fixpyruntime
May 13, 2022
Merged

Fix release runtime package.#9118
stellaraccident merged 1 commit intoiree-org:mainfrom
stellaraccident:fixpyruntime

Conversation

@stellaraccident
Copy link
Copy Markdown
Collaborator

When the runtime path was flattened, the install locations were not
updated to line up. They were also going through a really old macro that
had outlived its usefulness. Just added corrected, explicit install()
calls. Also fixed a typo in the build_linux_packages.sh script noticed
when trying to build just one python version to test.

It is now possible to test/repo this exactly as the CI does:

override_python_versions=cp39-cp39 packages=iree-runtime
./build_tools/python_deploy/build_linux_packages.sh
pip install
build_tools/python_deploy/wheelhouse/iree_runtime-0.dev0+4020b408be725b54b0266a2a9c3388b5c704ced4-cp39-cp39-manylinux_2_17_x86_64.manylinux2014_x86_64.whl
python runtime/bindings/python/tests/array_interop_test.py
iree-run-module

Fixes #9108

When the runtime path was flattened, the install locations were not
updated to line up. They were also going through a really old macro that
had outlived its usefulness. Just added corrected, explicit install()
calls. Also fixed a typo in the build_linux_packages.sh script noticed
when trying to build just one python version to test.

It is now possible to test/repo this exactly as the CI does:

```
override_python_versions=cp39-cp39 packages=iree-runtime
./build_tools/python_deploy/build_linux_packages.sh
pip install
build_tools/python_deploy/wheelhouse/iree_runtime-0.dev0+4020b408be725b54b0266a2a9c3388b5c704ced4-cp39-cp39-manylinux_2_17_x86_64.manylinux2014_x86_64.whl
python runtime/bindings/python/tests/array_interop_test.py
iree-run-module
```

Fixes iree-org#9108
@ScottTodd ScottTodd added infrastructure Relating to build systems, CI, or testing bindings/python Python wrapping IREE's C API labels May 13, 2022
Copy link
Copy Markdown
Contributor

@GMNGeoffrey GMNGeoffrey left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

download

Comment on lines +193 to 199
# Install tools into python_packages/iree_runtime/iree/runtime
install(
TARGETS
iree_tools_iree-benchmark-module
iree_tools_iree-benchmark-trace
iree_tools_iree-run-module
iree_tools_iree-run-trace
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FYI I tried to shuffle around the directory include order here https://github.com/google/iree/blob/66e48077386d347cf9113d15f332c8b14fd526bd/CMakeLists.txt#L693-L701

to include runtime/ first, then tools/, but I ran into issues with this block of code (#9084 (comment))

CMake Error at runtime/bindings/python/CMakeLists.txt:196 (install):
  install TARGETS given target "iree_tools_iree-benchmark-module" which does
  not exist.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah: Most things in CMake don't have that problem, but unfortunately, install() targets must exist at the time of configure. Open to suggestions.

The nuclear option is: https://cmake.org/cmake/help/latest/command/cmake_language.html#deferring-calls

(could just defer these to the end of the project directory)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My first thought was to have a section at the bottom of tools/CMakeLists.txt that would either directly set up the python installs, or include() a runtime/bindings/python/tools/CMakeLists.txt (that isn't included transitively by including runtime/) - basically deferred calls, but with code sprinkled across a few different files

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, this might be an appropriate use of the nuclear option:

install(
  TARGETS
    iree_tools_iree-benchmark-module
    iree_tools_iree-benchmark-trace
    iree_tools_iree-run-module
    iree_tools_iree-run-trace
    ${_EXTRA_INSTALL_TOOL_TARGETS}
  DESTINATION "${_INSTALL_DIR}/iree/runtime"
  COMPONENT "${_INSTALL_COMPONENT}"
)

Becomes:

cmake_language(DEFER DIRECTORY "${IREE_SOURCE_DIR}"
  CALL install
  TARGETS
    iree_tools_iree-benchmark-module
    iree_tools_iree-benchmark-trace
    iree_tools_iree-run-module
    iree_tools_iree-run-trace
    ${_EXTRA_INSTALL_TOOL_TARGETS}
  DESTINATION "${_INSTALL_DIR}/iree/runtime"
  COMPONENT "${_INSTALL_COMPONENT}"
)

The cmake_language defer stuff is relatively terrifying. Someone seems to have been compelled to turn it into an RPC primitive... but this usage isn't too bad.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bindings/python Python wrapping IREE's C API infrastructure Relating to build systems, CI, or testing

Projects

None yet

Development

Successfully merging this pull request may close these issues.

build failures for python packages

3 participants