Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[CI/Build] setuptools-scm fixes #8900

Merged
merged 1 commit into from
Oct 14, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions .buildkite/release-pipeline.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ steps:
agents:
queue: cpu_queue
commands:
- "DOCKER_BUILDKIT=1 docker build --build-arg max_jobs=16 --build-arg buildkite_commit=$BUILDKITE_COMMIT --build-arg USE_SCCACHE=1 --build-arg CUDA_VERSION=12.1.0 --tag vllm-ci:build-image --target build --progress plain ."
- "DOCKER_BUILDKIT=1 docker build --build-arg max_jobs=16 --build-arg USE_SCCACHE=1 --build-arg CUDA_VERSION=12.1.0 --tag vllm-ci:build-image --target build --progress plain ."
- "mkdir artifacts"
- "docker run --rm -v $(pwd)/artifacts:/artifacts_host vllm-ci:build-image bash -c 'cp -r dist /artifacts_host && chmod -R a+rw /artifacts_host'"
# rename the files to change linux -> manylinux1
Expand All @@ -22,7 +22,7 @@ steps:
agents:
queue: cpu_queue
commands:
- "DOCKER_BUILDKIT=1 docker build --build-arg max_jobs=16 --build-arg buildkite_commit=$BUILDKITE_COMMIT --build-arg USE_SCCACHE=1 --build-arg CUDA_VERSION=11.8.0 --tag vllm-ci:build-image --target build --progress plain ."
- "DOCKER_BUILDKIT=1 docker build --build-arg max_jobs=16 --build-arg USE_SCCACHE=1 --build-arg CUDA_VERSION=11.8.0 --tag vllm-ci:build-image --target build --progress plain ."
- "mkdir artifacts"
- "docker run --rm -v $(pwd)/artifacts:/artifacts_host vllm-ci:build-image bash -c 'cp -r dist /artifacts_host && chmod -R a+rw /artifacts_host'"
# rename the files to change linux -> manylinux1
Expand Down
30 changes: 29 additions & 1 deletion .dockerignore
Original file line number Diff line number Diff line change
Expand Up @@ -2,5 +2,33 @@
/.venv
/build
dist
Dockerfile*
vllm/*.so

Copy link
Contributor Author

Choose a reason for hiding this comment

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

These additions should be safe and should make sure that COPY . . in the Dockerfiles does not copy any additional files that shouldn't go in a build.

I'm unsure on whether there's anything else we might need to add here to avoid copying temporary from the host into the container build (mostly thinking about vllm/vllm_flash_attn right now)

# Byte-compiled / optimized / DLL files
__pycache__/
*.py[cod]
*$py.class

.mypy_cache

# Distribution / packaging
.Python
/build/
cmake-build-*/
CMakeUserPresets.json
develop-eggs/
/dist/
downloads/
eggs/
.eggs/
lib/
lib64/
parts/
sdist/
var/
wheels/
share/python-wheels/
*.egg-info/
.installed.cfg
*.egg
MANIFEST
3 changes: 1 addition & 2 deletions .github/workflows/scripts/build.sh
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,7 @@ PATH=${cuda_home}/bin:$PATH
LD_LIBRARY_PATH=${cuda_home}/lib64:$LD_LIBRARY_PATH

# Install requirements
$python_executable -m pip install wheel packaging 'setuptools-scm>=8'
$python_executable -m pip install -r requirements-cuda.txt
$python_executable -m pip install -r requirements-build.txt -r requirements-cuda.txt

# Limit the number of parallel jobs to avoid OOM
export MAX_JOBS=1
Expand Down
10 changes: 1 addition & 9 deletions Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -71,15 +71,7 @@ RUN --mount=type=cache,target=/root/.cache/pip \
python3 -m pip install -r requirements-build.txt

# files and directories related to build wheels
COPY csrc csrc
COPY setup.py setup.py
COPY cmake cmake
COPY CMakeLists.txt CMakeLists.txt
COPY README.md README.md
COPY requirements-common.txt requirements-common.txt
COPY requirements-cuda.txt requirements-cuda.txt
COPY pyproject.toml pyproject.toml
COPY vllm vllm
COPY . .

# max jobs used by Ninja to build extensions
ARG max_jobs=2
Expand Down
11 changes: 1 addition & 10 deletions Dockerfile.openvino
Original file line number Diff line number Diff line change
Expand Up @@ -9,16 +9,7 @@ RUN apt-get update -y && \
ffmpeg libsm6 libxext6 libgl1
WORKDIR /workspace

# copy requirements
Copy link
Member

Choose a reason for hiding this comment

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

I think we copy files separately to avoid docker build cache invalidation.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is this a problem? The next step builds vllm, and that relies sccache anyway, which greatly speeds up the process.

COPY requirements-build.txt /workspace/vllm/
COPY requirements-common.txt /workspace/vllm/
COPY requirements-openvino.txt /workspace/vllm/

COPY vllm/ /workspace/vllm/vllm
COPY csrc/core /workspace/vllm/csrc/core
COPY cmake/utils.cmake /workspace/vllm/cmake/
COPY CMakeLists.txt /workspace/vllm/
COPY setup.py /workspace/vllm/
COPY . .

# install build requirements
RUN PIP_EXTRA_INDEX_URL="https://download.pytorch.org/whl/cpu" python3 -m pip install -r /workspace/vllm/requirements-build.txt
Expand Down
27 changes: 10 additions & 17 deletions collect_env.py
Original file line number Diff line number Diff line change
Expand Up @@ -267,23 +267,16 @@ def get_neuron_sdk_version(run_lambda):


def get_vllm_version():
version = ""
try:
import vllm
version = vllm.__version__
except Exception:
pass
commit = ""
try:
import vllm
commit = vllm.__commit__
except Exception:
pass
if version != "" and commit != "":
return f"{version}@{commit}"
if version == "" and commit == "":
return "N/A"
return version or commit
from vllm import __version__, __version_tuple__

if __version__ == "dev":
return "N/A (dev)"
Copy link
Collaborator

Choose a reason for hiding this comment

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

can we use 1.0.0-dev instead of N/A

Copy link
Contributor Author

Choose a reason for hiding this comment

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

__version__ will be set to dev only if setuptools-scm doesn't write vllm/_version.py (or something else bad happens with imports, see here, meaning we cannot infer what version this is (.git is not present ).

In this case, I think we have no reason for setting this to 1.0.0-dev, we can just acknowledge that this is an unknown development build.


if len(__version_tuple__) == 4: # dev build
Copy link
Collaborator

Choose a reason for hiding this comment

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

why is this 4 particularly?

Copy link
Collaborator

Choose a reason for hiding this comment

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

can I get some more context on __version_tuple__?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You can find the setuptools-scm docs here: https://setuptools-scm.readthedocs.io/en/v8.0.1/usage/

For dev builds, version tuple is going to look like this:

(0, 6, 3, 'dev76', 'g78a09a757')

for exact tags, version tuple is going to look like this:

(0, 6, 3)

i.e. you have an extra field for development builds which includes the git sha

git_sha = __version_tuple__[-1][1:] # type: ignore
return f"{__version__} (git sha: {git_sha}"

return __version__

def summarize_vllm_build_flags():
# This could be a static method if the flags are constant, or dynamic if you need to check environment variables, etc.
Expand Down
3 changes: 3 additions & 0 deletions pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,9 @@ requires = [
]
build-backend = "setuptools.build_meta"

[tool.setuptools_scm]
# version_file = "vllm/_version.py" # currently handled by `setup.py:get_version()`

[tool.ruff]
# Allow lines to be as long as 80.
line-length = 80
Expand Down
Loading