gemini-code-assist[bot] commented on code in PR #144: URL: https://github.com/apache/tvm-ffi/pull/144#discussion_r2434957111
########## docs/guides/install.md: ########## @@ -0,0 +1,98 @@ +<!--- Licensed to the Apache Software Foundation (ASF) under one --> +<!--- or more contributor license agreements. See the NOTICE file --> +<!--- distributed with this work for additional information --> +<!--- regarding copyright ownership. The ASF licenses this file --> +<!--- to you under the Apache License, Version 2.0 (the --> +<!--- "License"); you may not use this file except in compliance --> +<!--- with the License. You may obtain a copy of the License at --> + +<!--- http://www.apache.org/licenses/LICENSE-2.0 --> + +<!--- Unless required by applicable law or agreed to in writing, --> +<!--- software distributed under the License is distributed on an --> +<!--- "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY --> +<!--- KIND, either express or implied. See the License for the --> +<!--- specific language governing permissions and limitations --> +<!--- under the License. --> +# Build from Source + +This guide covers two common workflows: + +- Building and installing the Python package, which wraps the core C++ library; +- Building only the core C/C++ library only without Python. + +```{admonition} Prerequisite +:class: tip + +- Python: 3.9 or newer +- Compiler: C++17-capable toolchain + - Linux: GCC or Clang with C++17 support + - macOS: Apple Clang (via Xcode Command Line Tools) + - Windows: MSVC (Visual Studio 2019 or 2022, x64) +- Build tools: CMake 3.18+; Ninja (optional, for faster builds) +``` + +## Build the Python Package + +Download the source via: + +```bash +git clone --recursive https://github.com/apache/tvm-ffi +cd tvm-ffi +``` + +```{note} +Always clone with ``--recursive`` to pull submodules. If you already cloned without it, run: + git submodule update --init --recursive +``` + +The Python package is built with scikit-build-core, which drives CMake to compile the C++ core and Cython extension. + +```bash +pip install --reinstall --verbose -e . \ + --config-settings cmake.define.TVM_FFI_ATTACH_DEBUG_SYMBOLS=ON +``` + +The ``-e/--editable`` install reflects Python-only changes immediately, and the ``--reinstall`` flag forces a rebuild. + +```{warning} +Changes to C++/Cython require re-running the install command. +``` + +Verify the install: + +```bash +python -c "import tvm_ffi; print(tvm_ffi.__version__)" +tvm-ffi-config -h +``` + +```{tip} +Use ``tvm-ffi-config`` to query include and link flags when consuming TVM FFI from external C/C++ projects: + tvm-ffi-config --includedir + tvm-ffi-config --dlpack-includedir + tvm-ffi-config --libfiles # or --libs/--ldflags on Unix +``` + +## Build the C/C++ Library Only + +TVM FFI can be used as a standalone C/C++ library without Python. + +Single-config generators (Ninja/Unix Makefiles, typical on Linux/macOS): Review Comment:  This section only provides build instructions for single-config generators like Ninja and Makefiles, which are common on Linux and macOS. However, the prerequisites mention MSVC on Windows, which is a multi-config generator. The guide should include instructions for building with Visual Studio to be complete. A troubleshooting tip at the end mentions this, but the instructions should be in the main section. For example, you could add a section for multi-config generators: **Multi-config generators (Visual Studio):** ```bash cmake -S . -B build_cpp cmake --build build_cpp --config RelWithDebInfo --parallel --target tvm_ffi_shared cmake --install build_cpp --config RelWithDebInfo --prefix dist ``` ########## docs/guides/install.md: ########## @@ -0,0 +1,98 @@ +<!--- Licensed to the Apache Software Foundation (ASF) under one --> +<!--- or more contributor license agreements. See the NOTICE file --> +<!--- distributed with this work for additional information --> +<!--- regarding copyright ownership. The ASF licenses this file --> +<!--- to you under the Apache License, Version 2.0 (the --> +<!--- "License"); you may not use this file except in compliance --> +<!--- with the License. You may obtain a copy of the License at --> + +<!--- http://www.apache.org/licenses/LICENSE-2.0 --> + +<!--- Unless required by applicable law or agreed to in writing, --> +<!--- software distributed under the License is distributed on an --> +<!--- "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY --> +<!--- KIND, either express or implied. See the License for the --> +<!--- specific language governing permissions and limitations --> +<!--- under the License. --> +# Build from Source + +This guide covers two common workflows: + +- Building and installing the Python package, which wraps the core C++ library; +- Building only the core C/C++ library only without Python. + +```{admonition} Prerequisite +:class: tip + +- Python: 3.9 or newer +- Compiler: C++17-capable toolchain + - Linux: GCC or Clang with C++17 support + - macOS: Apple Clang (via Xcode Command Line Tools) + - Windows: MSVC (Visual Studio 2019 or 2022, x64) +- Build tools: CMake 3.18+; Ninja (optional, for faster builds) +``` + +## Build the Python Package + +Download the source via: + +```bash +git clone --recursive https://github.com/apache/tvm-ffi +cd tvm-ffi +``` + +```{note} +Always clone with ``--recursive`` to pull submodules. If you already cloned without it, run: + git submodule update --init --recursive +``` + +The Python package is built with scikit-build-core, which drives CMake to compile the C++ core and Cython extension. + +```bash +pip install --reinstall --verbose -e . \ + --config-settings cmake.define.TVM_FFI_ATTACH_DEBUG_SYMBOLS=ON +``` + +The ``-e/--editable`` install reflects Python-only changes immediately, and the ``--reinstall`` flag forces a rebuild. + +```{warning} +Changes to C++/Cython require re-running the install command. +``` + +Verify the install: + +```bash +python -c "import tvm_ffi; print(tvm_ffi.__version__)" +tvm-ffi-config -h +``` + +```{tip} +Use ``tvm-ffi-config`` to query include and link flags when consuming TVM FFI from external C/C++ projects: + tvm-ffi-config --includedir + tvm-ffi-config --dlpack-includedir + tvm-ffi-config --libfiles # or --libs/--ldflags on Unix +``` + +## Build the C/C++ Library Only + +TVM FFI can be used as a standalone C/C++ library without Python. + +Single-config generators (Ninja/Unix Makefiles, typical on Linux/macOS): + +```bash +cmake -S . -B build_cpp -DCMAKE_BUILD_TYPE=RelWithDebInfo +cmake --build build_cpp --parallel --target tvm_ffi_shared +cmake --install build_cpp --prefix $PWD/dist Review Comment:  The use of `$PWD` is not portable and will fail on Windows systems that don't use a bash-like shell (e.g., Command Prompt or PowerShell). It's better to use a relative path like `dist`, which CMake handles correctly across platforms. ```suggestion cmake --install build_cpp --prefix dist ``` ########## docs/guides/install.md: ########## @@ -0,0 +1,98 @@ +<!--- Licensed to the Apache Software Foundation (ASF) under one --> +<!--- or more contributor license agreements. See the NOTICE file --> +<!--- distributed with this work for additional information --> +<!--- regarding copyright ownership. The ASF licenses this file --> +<!--- to you under the Apache License, Version 2.0 (the --> +<!--- "License"); you may not use this file except in compliance --> +<!--- with the License. You may obtain a copy of the License at --> + +<!--- http://www.apache.org/licenses/LICENSE-2.0 --> + +<!--- Unless required by applicable law or agreed to in writing, --> +<!--- software distributed under the License is distributed on an --> +<!--- "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY --> +<!--- KIND, either express or implied. See the License for the --> +<!--- specific language governing permissions and limitations --> +<!--- under the License. --> +# Build from Source + +This guide covers two common workflows: + +- Building and installing the Python package, which wraps the core C++ library; +- Building only the core C/C++ library only without Python. + +```{admonition} Prerequisite +:class: tip Review Comment:  The `Prerequisite` section is styled as a `tip`. Prerequisites are essential requirements, not optional tips. Using `:class: note` or `:class: important` would be more appropriate and less confusing for users. ```suggestion :class: note ``` ########## docs/guides/install.md: ########## @@ -0,0 +1,98 @@ +<!--- Licensed to the Apache Software Foundation (ASF) under one --> +<!--- or more contributor license agreements. See the NOTICE file --> +<!--- distributed with this work for additional information --> +<!--- regarding copyright ownership. The ASF licenses this file --> +<!--- to you under the Apache License, Version 2.0 (the --> +<!--- "License"); you may not use this file except in compliance --> +<!--- with the License. You may obtain a copy of the License at --> + +<!--- http://www.apache.org/licenses/LICENSE-2.0 --> + +<!--- Unless required by applicable law or agreed to in writing, --> +<!--- software distributed under the License is distributed on an --> +<!--- "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY --> +<!--- KIND, either express or implied. See the License for the --> +<!--- specific language governing permissions and limitations --> +<!--- under the License. --> +# Build from Source + +This guide covers two common workflows: + +- Building and installing the Python package, which wraps the core C++ library; +- Building only the core C/C++ library only without Python. Review Comment:  There's a small grammatical error here with the repeated word 'only'. For better clarity, I suggest rephrasing this sentence. ```suggestion - Building only the core C/C++ library without Python. ``` ########## docs/guides/install.md: ########## @@ -0,0 +1,98 @@ +<!--- Licensed to the Apache Software Foundation (ASF) under one --> +<!--- or more contributor license agreements. See the NOTICE file --> +<!--- distributed with this work for additional information --> +<!--- regarding copyright ownership. The ASF licenses this file --> +<!--- to you under the Apache License, Version 2.0 (the --> +<!--- "License"); you may not use this file except in compliance --> +<!--- with the License. You may obtain a copy of the License at --> + +<!--- http://www.apache.org/licenses/LICENSE-2.0 --> + +<!--- Unless required by applicable law or agreed to in writing, --> +<!--- software distributed under the License is distributed on an --> +<!--- "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY --> +<!--- KIND, either express or implied. See the License for the --> +<!--- specific language governing permissions and limitations --> +<!--- under the License. --> +# Build from Source + +This guide covers two common workflows: + +- Building and installing the Python package, which wraps the core C++ library; +- Building only the core C/C++ library only without Python. + +```{admonition} Prerequisite +:class: tip + +- Python: 3.9 or newer +- Compiler: C++17-capable toolchain + - Linux: GCC or Clang with C++17 support + - macOS: Apple Clang (via Xcode Command Line Tools) + - Windows: MSVC (Visual Studio 2019 or 2022, x64) +- Build tools: CMake 3.18+; Ninja (optional, for faster builds) +``` + +## Build the Python Package + +Download the source via: + +```bash +git clone --recursive https://github.com/apache/tvm-ffi +cd tvm-ffi +``` + +```{note} +Always clone with ``--recursive`` to pull submodules. If you already cloned without it, run: + git submodule update --init --recursive +``` + +The Python package is built with scikit-build-core, which drives CMake to compile the C++ core and Cython extension. + +```bash +pip install --reinstall --verbose -e . \ + --config-settings cmake.define.TVM_FFI_ATTACH_DEBUG_SYMBOLS=ON +``` + +The ``-e/--editable`` install reflects Python-only changes immediately, and the ``--reinstall`` flag forces a rebuild. + +```{warning} +Changes to C++/Cython require re-running the install command. +``` + +Verify the install: + +```bash +python -c "import tvm_ffi; print(tvm_ffi.__version__)" +tvm-ffi-config -h +``` + +```{tip} +Use ``tvm-ffi-config`` to query include and link flags when consuming TVM FFI from external C/C++ projects: + tvm-ffi-config --includedir + tvm-ffi-config --dlpack-includedir + tvm-ffi-config --libfiles # or --libs/--ldflags on Unix +``` + +## Build the C/C++ Library Only + +TVM FFI can be used as a standalone C/C++ library without Python. + +Single-config generators (Ninja/Unix Makefiles, typical on Linux/macOS): + +```bash +cmake -S . -B build_cpp -DCMAKE_BUILD_TYPE=RelWithDebInfo +cmake --build build_cpp --parallel --target tvm_ffi_shared +cmake --install build_cpp --prefix $PWD/dist +``` + +After installation, you should see: + +- Headers are installed under ``dist/include/``; +- Libraries are installed under ``dist/lib/``. + +## Troubleshooting + +- Rebuilding after C++/Cython changes: re-run ``pip install --reinstall -e .``. Editable installs only auto-reflect pure Python changes. +- Submodules missing: run ``git submodule update --init --recursive`` from the repo root. +- Library not found at import time: ensure your dynamic loader can find the shared library. For source builds, add the ``lib`` directory to ``LD_LIBRARY_PATH`` (Linux), ``DYLD_LIBRARY_PATH`` (macOS), or ``PATH`` (Windows). Review Comment:  This troubleshooting tip is a bit ambiguous. It's not clear which 'source build' it refers to (the Python package build or the C/C++ library-only build). Also, the `lib` directory path is not specified (e.g., `build/lib` or `dist/lib`). Please clarify the context to make this tip more helpful. For instance, you could specify that this applies when using the library built via the 'Build the C/C++ Library Only' steps and that the path to add is `dist/lib` relative to the installation prefix. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
