Default build dir in script matches the one in setup.py
Default build dir in setup.py https://github.com/llvm/torch-mlir/blob/5ecc1d5c0dbb8ac166c36a083c982eb27e6f33eb/setup.py#L65-L66 Keep them in line.
Yes, I initially followed the development.md to complete the build. When packaging torch_mlir as a dependency using the setup.py build command, I noticed some inconsistencies with the development.md behavior, including:
- Does not automatically execute the script(e.g. update_torch_ods.sh)
- The default build directory used is not consistent, and I need to specify it using TORCH_MLIR_BUILD_DIR.
Perhaps I can try modifying the setup.py to make them consistent. Including automatically executing source generation, to achieve a one-click package.
Best would be if we can make setuptools not steal the build directory name. While perhaps a minor thing, the mlir c++ developers are quite used to having build/ be for cmake. It would be nice if pip moved and they could both exist in their preferred way.
Possibility: https://stackoverflow.com/questions/66163088/how-to-change-build-directory-in-setuptools
Best would be if we can make setuptools not steal the build directory name. While perhaps a minor thing, the mlir c++ developers are quite used to having build/ be for cmake. It would be nice if pip moved and they could both exist in their preferred way.
Possibility: https://stackoverflow.com/questions/66163088/how-to-change-build-directory-in-setuptools I opened a new PR to implement this feature (and made some enhancements to setuptools), because it was inconsistent with the original intention of my PR.
Using a new build directory also introduces a new problem: the default build path in update_torch_ods.sh cannot matches the one in setup.py. This means that after building with setup.py, must specify TORCH_MLIR_BUILD_DIR to execute update_torch_ods.sh.
I think there are two ways to solve this:
- Setuptools uses a new directory for packaging the wheel, but still uses build/ when calling cmake. We can check the cmake cache config to determine if an update is needed.
- Setuptools is only used for wheel packaging, avoiding operations like
setup.py develop/buildand documenting this in the documentation.
Best would be if we can make setuptools not steal the build directory name. While perhaps a minor thing, the mlir c++ developers are quite used to having build/ be for cmake. It would be nice if pip moved and they could both exist in their preferred way. Possibility: https://stackoverflow.com/questions/66163088/how-to-change-build-directory-in-setuptools I opened a new PR to implement this feature (and made some enhancements to setuptools), because it was inconsistent with the original intention of my PR.
Using a new build directory also introduces a new problem: the default build path in update_torch_ods.sh cannot matches the one in setup.py. This means that after building with setup.py, must specify TORCH_MLIR_BUILD_DIR to execute update_torch_ods.sh.
I think there are two ways to solve this:
- Setuptools uses a new directory for packaging the wheel, but still uses build/ when calling cmake. We can check the cmake cache config to determine if an update is needed.
- Setuptools is only used for wheel packaging, avoiding operations like
setup.py develop/buildand documenting this in the documentation.
I think I prefer this second option, but I know it is annoying.
Best would be if we can make setuptools not steal the build directory name. While perhaps a minor thing, the mlir c++ developers are quite used to having build/ be for cmake. It would be nice if pip moved and they could both exist in their preferred way. Possibility: https://stackoverflow.com/questions/66163088/how-to-change-build-directory-in-setuptools I opened a new PR to implement this feature (and made some enhancements to setuptools), because it was inconsistent with the original intention of my PR.
Using a new build directory also introduces a new problem: the default build path in update_torch_ods.sh cannot matches the one in setup.py. This means that after building with setup.py, must specify TORCH_MLIR_BUILD_DIR to execute update_torch_ods.sh. I think there are two ways to solve this:
- Setuptools uses a new directory for packaging the wheel, but still uses build/ when calling cmake. We can check the cmake cache config to determine if an update is needed.
- Setuptools is only used for wheel packaging, avoiding operations like
setup.py develop/buildand documenting this in the documentation.I think I prefer this second option, but I know it is annoying.
If go with the second option, I will try to make the python setup.py develop/build command throws an error message, prompting the user that the setup.py is only used for packaging the wheel. I will also update the development doc. In addition, I plan to revise some outdated content in the development documentation, which may take a considerable amount of time.
Thank you. If you see a way to simplify while doing it, let me know.
In my opinion, we should simply delete development docs that are wrong. I'd rather have people have to ask than sending down the wrong path. Obviously, full/good docs would be better: I'm just saying that if you don't have the time to update them all, we can just delete for now.
Best would be if we can make setuptools not steal the build directory name. While perhaps a minor thing, the mlir c++ developers are quite used to having build/ be for cmake. It would be nice if pip moved and they could both exist in their preferred way. Possibility: https://stackoverflow.com/questions/66163088/how-to-change-build-directory-in-setuptools I opened a new PR to implement this feature (and made some enhancements to setuptools), because it was inconsistent with the original intention of my PR.
Using a new build directory also introduces a new problem: the default build path in update_torch_ods.sh cannot matches the one in setup.py. This means that after building with setup.py, must specify TORCH_MLIR_BUILD_DIR to execute update_torch_ods.sh. I think there are two ways to solve this:
- Setuptools uses a new directory for packaging the wheel, but still uses build/ when calling cmake. We can check the cmake cache config to determine if an update is needed.
- Setuptools is only used for wheel packaging, avoiding operations like
setup.py develop/buildand documenting this in the documentation.I think I prefer this second option, but I know it is annoying.
If go with the second option, I will try to make the
python setup.py develop/buildcommand throws an error message, prompting the user that thesetup.pyis only used for packaging the wheel. I will also update the development doc. In addition, I plan to revise some outdated content in the development documentation, which may take a considerable amount of time.
I completed these tasks in a new PR #3162 . @stellaraccident