Skip to content

Commit

Permalink
Add requires-python = ">=3.10" to pyproject.toml (pytorch#4131)
Browse files Browse the repository at this point in the history
Summary:
We only support python 3.10/3.11/3.12 right now, so make that explicit.

The last time I tried to do this, the linter tried to force us to use
new-in-3.10 syntax. I was able to avoid that this time by telling the
`black` formatter to use a larger range of python versions.

Also modify setup.py to be 3.8 and 3.13 compatible so that it can run
and complain about the version mismatch instead of just failing to run.

Then, to save users some time, check the version in
install_requirements.sh before doing any real work.

And `cd` to the script's directory before starting so that users can
invoke it like `/path/to/executorch/install_requirements.sh`.

Pull Request resolved: pytorch#4131

Test Plan:
Built wheels in CI by adding the `ciflow/binaries/all` label.

Linted all python files:
```
find . -type f -name "*.py" | grep -vE 'third-party|cmake-out|pip-out' > /tmp/f
lintrunner --paths-cmd='cat /tmp/f'
```

Also created a conda environment with python 3.8 and ran
`install_requirements.sh` (without the fail-fast check). It used to
fail on syntax errors, but now it fails because the version can't be
satisfied.

To test the fail-fast logic, ran install_requirements.sh in 3.8 and 3.10
conda environments, and saw that it failed/passed as expected.

To test the failure cases, made some local modifications and saw that it
continued to build the wheel after printing a warning:
- Remove/rename pyproject.toml
- Remove the `requires-python` line from pyproject.toml
- Modify the `requires-python` line to contain an invalid range
- Import a missing module in the `python_is_compatible()` code

To test the new `cd` logic:
```
cd build
../install_requirements.sh  # This worked
```

Reviewed By: mergennachin

Differential Revision: D59291599

Pulled By: dbort

fbshipit-source-id: 5bfc97346180b65ad8719753c4126af025a41ae0
  • Loading branch information
dbort authored and facebook-github-bot committed Jul 3, 2024
1 parent 38d67db commit f32d707
Show file tree
Hide file tree
Showing 3 changed files with 83 additions and 23 deletions.
61 changes: 59 additions & 2 deletions install_requirements.sh
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,10 @@
# This source code is licensed under the BSD-style license found in the
# LICENSE file in the root directory of this source tree.

# Install required python dependencies for developing
# Dependencies are defined in .pyproject.toml
# Before doing anything, cd to the directory containing this script.
cd -- "$( dirname -- "${BASH_SOURCE[0]}" )" &> /dev/null || /bin/true

# Find the names of the python tools to use.
if [[ -z $PYTHON_EXECUTABLE ]];
then
if [[ -z $CONDA_DEFAULT_ENV ]] || [[ $CONDA_DEFAULT_ENV == "base" ]] || [[ ! -x "$(command -v python)" ]];
Expand All @@ -24,6 +26,61 @@ else
PIP_EXECUTABLE=pip3
fi

# Returns 0 if the current python version is compatible with the version range
# in pyprojects.toml, or returns 1 if it is not compatible. If the check logic
# itself fails, prints a warning and returns 0.
python_is_compatible() {
# Scrape the version range from pyproject.toml, which should be
# in the current directory.
local version_specifier
version_specifier="$(
grep "^requires-python" pyproject.toml \
| head -1 \
| sed -e 's/[^"]*"//;s/".*//'
)"
if [[ -z ${version_specifier} ]]; then
echo "WARNING: Skipping python version check: version range not found" >& 2
return 0
fi

# Install the packaging module if necessary.
if ! python -c 'import packaging' 2> /dev/null ; then
${PIP_EXECUTABLE} install packaging
fi

# Compare the current python version to the range in version_specifier. Exits
# with status 1 if the version is not compatible, or with status 0 if the
# version is compatible or the logic itself fails.
${PYTHON_EXECUTABLE} <<EOF
import sys
try:
import packaging.version
import packaging.specifiers
import platform
python_version = packaging.version.parse(platform.python_version())
version_range = packaging.specifiers.SpecifierSet("${version_specifier}")
if python_version not in version_range:
print(
"ERROR: ExecuTorch does not support python version "
+ f"{python_version}: must satisfy \"${version_specifier}\"",
file=sys.stderr,
)
sys.exit(1)
except Exception as e:
print(f"WARNING: Skipping python version check: {e}", file=sys.stderr)
sys.exit(0)
EOF

return $?
}

# Fail fast if the wheel build will fail because the current python version
# isn't supported. But don't fail if the check logic itself has problems: the
# wheel build will do a final check before proceeding.
if ! python_is_compatible; then
exit 1
fi

# Parse options.
EXECUTORCH_BUILD_PYBIND=OFF
Expand Down
9 changes: 9 additions & 0 deletions pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -44,9 +44,12 @@ classifiers = [
"Programming Language :: Python :: 3",
# Update this as we support more versions of python.
"Programming Language :: Python :: 3.10",
"Programming Language :: Python :: 3.11",
"Programming Language :: Python :: 3.12",
]

# Python dependencies required for use.
requires-python = ">=3.10"
dependencies=[
"expecttest",
"flatbuffers",
Expand Down Expand Up @@ -93,3 +96,9 @@ flatc = "executorch.data.bin:flatc"
[tool.usort]
# Do not try to put "first-party" imports in their own section.
first_party_detection = false

[tool.black]
# Emit syntax compatible with older versions of python instead of only the range
# specified by `requires-python`. TODO: Remove this once we support these older
# versions of python and can expand the `requires-python` range.
target-version = ["py38", "py39", "py310", "py311", "py312"]
36 changes: 15 additions & 21 deletions setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@
from distutils import log
from distutils.sysconfig import get_python_lib
from pathlib import Path
from typing import Optional
from typing import List, Optional

from setuptools import Extension, setup
from setuptools.command.build import build
Expand All @@ -82,31 +82,27 @@ def _is_env_enabled(env_var: str, default: bool = False) -> bool:
return True

@classmethod
@property
def pybindings(cls) -> bool:
return cls._is_env_enabled("EXECUTORCH_BUILD_PYBIND", default=False)

@classmethod
@property
def llama_custom_ops(cls) -> bool:
return cls._is_env_enabled("EXECUTORCH_BUILD_KERNELS_CUSTOM_AOT", default=True)

@classmethod
@property
def flatc(cls) -> bool:
return cls._is_env_enabled("EXECUTORCH_BUILD_FLATC", default=True)


class Version:
"""Static properties that describe the version of the pip package."""
"""Static strings that describe the version of the pip package."""

# Cached values returned by the properties.
__root_dir_attr: Optional[str] = None
__string_attr: Optional[str] = None
__git_hash_attr: Optional[str] = None

@classmethod
@property
def _root_dir(cls) -> str:
"""The path to the root of the git repo."""
if cls.__root_dir_attr is None:
Expand All @@ -115,7 +111,6 @@ def _root_dir(cls) -> str:
return str(cls.__root_dir_attr)

@classmethod
@property
def git_hash(cls) -> Optional[str]:
"""The current git hash, if known."""
if cls.__git_hash_attr is None:
Expand All @@ -124,7 +119,7 @@ def git_hash(cls) -> Optional[str]:
try:
cls.__git_hash_attr = (
subprocess.check_output(
["git", "rev-parse", "HEAD"], cwd=cls._root_dir
["git", "rev-parse", "HEAD"], cwd=cls._root_dir()
)
.decode("ascii")
.strip()
Expand All @@ -135,7 +130,6 @@ def git_hash(cls) -> Optional[str]:
return cls.__git_hash_attr if cls.__git_hash_attr else None

@classmethod
@property
def string(cls) -> str:
"""The version string."""
if cls.__string_attr is None:
Expand All @@ -147,10 +141,10 @@ def string(cls) -> str:
# Otherwise, read the version from a local file and add the git
# commit if available.
version = (
open(os.path.join(cls._root_dir, "version.txt")).read().strip()
open(os.path.join(cls._root_dir(), "version.txt")).read().strip()
)
if cls.git_hash:
version += "+" + cls.git_hash[:7]
if cls.git_hash():
version += "+" + cls.git_hash()[:7]
cls.__string_attr = version
return cls.__string_attr

Expand All @@ -160,9 +154,9 @@ def write_to_python_file(cls, path: str) -> None:
lines = [
"from typing import Optional",
'__all__ = ["__version__", "git_version"]',
f'__version__ = "{cls.string}"',
f'__version__ = "{cls.string()}"',
# A string or None.
f"git_version: Optional[str] = {repr(cls.git_hash)}",
f"git_version: Optional[str] = {repr(cls.git_hash())}",
]
with open(path, "w") as fp:
fp.write("\n".join(lines) + "\n")
Expand Down Expand Up @@ -480,7 +474,7 @@ def run(self):
# extension entries themselves instead of hard-coding them here.
build_args += ["--target", "flatc"]

if ShouldBuild.pybindings:
if ShouldBuild.pybindings():
cmake_args += [
"-DEXECUTORCH_BUILD_PYBIND=ON",
"-DEXECUTORCH_BUILD_KERNELS_QUANTIZED=ON", # add quantized ops to pybindings.
Expand All @@ -491,7 +485,7 @@ def run(self):
# add entries like `-DEXECUTORCH_BUILD_XNNPACK=ON` to the CMAKE_ARGS
# environment variable.

if ShouldBuild.llama_custom_ops:
if ShouldBuild.llama_custom_ops():
cmake_args += [
"-DEXECUTORCH_BUILD_KERNELS_CUSTOM=ON", # add llama sdpa ops to pybindings.
"-DEXECUTORCH_BUILD_KERNELS_CUSTOM_AOT=ON",
Expand Down Expand Up @@ -553,16 +547,16 @@ def run(self):
build.run(self)


def get_ext_modules() -> list[Extension]:
def get_ext_modules() -> List[Extension]:
"""Returns the set of extension modules to build."""

ext_modules = []
if ShouldBuild.flatc:
if ShouldBuild.flatc():
ext_modules.append(
BuiltFile("third-party/flatbuffers/flatc", "executorch/data/bin/")
)

if ShouldBuild.pybindings:
if ShouldBuild.pybindings():
ext_modules.append(
# Install the prebuilt pybindings extension wrapper for the runtime,
# portable kernels, and a selection of backends. This lets users
Expand All @@ -571,7 +565,7 @@ def get_ext_modules() -> list[Extension]:
"_portable_lib.*", "executorch.extension.pybindings._portable_lib"
)
)
if ShouldBuild.llama_custom_ops:
if ShouldBuild.llama_custom_ops():
ext_modules.append(
# Install the prebuilt library for custom ops used in llama.
BuiltFile(
Expand All @@ -588,7 +582,7 @@ def get_ext_modules() -> list[Extension]:


setup(
version=Version.string,
version=Version.string(),
# TODO(dbort): Could use py_modules to restrict the set of modules we
# package, and package_data to restrict the set up non-python files we
# include. See also setuptools/discovery.py for custom finders.
Expand Down

0 comments on commit f32d707

Please sign in to comment.