Skip to content

Add Windows (CPU) build support #476

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.

Sign up for GitHub

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

Draft
doringeman wants to merge 3 commits into
base: main
Choose a base branch
from
+150 −34
Draft

Add Windows (CPU) build support #476

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
4457b59
build: add Windows support to CLI Makefile
doringeman Dec 2, 2025
0d65693
refactor: extract OS detection into shared Makefile.common
doringeman Dec 2, 2025
51f2799
build: add Windows CPU support to llamacpp Makefile
doringeman Dec 4, 2025
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
15 changes: 15 additions & 0 deletions Makefile.common
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
# Common Makefile definitions shared across subprojects
# This file should be included at the top of other Makefiles

# OS detection
ifeq ($(OS),Windows_NT)
DETECTED_OS := Windows
else
UNAME_S := $(shell uname -s)
ifeq ($(UNAME_S),Linux)
DETECTED_OS := Linux
endif
ifeq ($(UNAME_S),Darwin)
DETECTED_OS := macOS
endif
endif
Comment on lines +7 to +15
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot Dec 4, 2025

Choose a reason for hiding this comment

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

medium

The current OS detection logic doesn't handle cases where uname -s returns something other than Linux or Darwin (e.g., FreeBSD). In such cases, DETECTED_OS will be empty, which could lead to confusing "Unsupported OS: " messages later. It would be better to use an if-else-if structure and set a default value for DETECTED_OS to provide a more informative message for unsupported systems.

else
    UNAME_S := $(shell uname -s)
    ifeq ($(UNAME_S),Linux)
        DETECTED_OS := Linux
    else ifeq ($(UNAME_S),Darwin)
        DETECTED_OS := macOS
    else
        DETECTED_OS := $(UNAME_S)
    endif
endif

3 changes: 2 additions & 1 deletion cmd/cli/.gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
model-cli
model-cli.exe
.idea/
dist/
dist/
41 changes: 30 additions & 11 deletions cmd/cli/Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,32 +1,51 @@
include ../../Makefile.common

.PHONY: all build clean link mock unit-tests docs

BINARY_NAME=model-cli

PLUGIN_DIR=$(HOME)/.docker/cli-plugins
PLUGIN_NAME=docker-model

VERSION ?=

MACOS_MIN_VERSION := 14.0
MACOS_MIN_VERSION_LDFLAG := -mmacosx-version-min=$(MACOS_MIN_VERSION)

# OS-specific settings
ifeq ($(DETECTED_OS),Windows)
BINARY_EXT := .exe
PLUGIN_DIR := $(USERPROFILE)/.docker/cli-plugins
else
BINARY_EXT :=
PLUGIN_DIR := $(HOME)/.docker/cli-plugins
endif

BINARY_WITH_EXT := $(BINARY_NAME)$(BINARY_EXT)
PLUGIN_WITH_EXT := $(PLUGIN_NAME)$(BINARY_EXT)

all: build

build:
@echo "Building $(BINARY_NAME)..."
go build -ldflags="-s -w -X github.com/docker/model-runner/cmd/cli/desktop.Version=$(shell git describe --tags --always --dirty)" -o $(BINARY_NAME) .
@echo "Building $(BINARY_WITH_EXT)..."
go build -ldflags="-s -w -X github.com/docker/model-runner/cmd/cli/desktop.Version=$(shell git describe --tags --always --dirty)" -o $(BINARY_WITH_EXT) .

link:
@if [ ! -f $(BINARY_NAME) ]; then \
@if [ ! -f $(BINARY_WITH_EXT) ]; then \
echo "Binary not found, building first..."; \
$(MAKE) build; \
else \
echo "Using existing binary $(BINARY_NAME)"; \
echo "Using existing binary $(BINARY_WITH_EXT)"; \
fi
@echo "Linking $(BINARY_NAME) to Docker CLI plugins directory..."
@mkdir -p $(PLUGIN_DIR)
@ln -sf $(shell pwd)/$(BINARY_NAME) $(PLUGIN_DIR)/$(PLUGIN_NAME)
@echo "Link created: $(PLUGIN_DIR)/$(PLUGIN_NAME)"
ifeq ($(DETECTED_OS),Windows)
@echo "Copying $(BINARY_WITH_EXT) to Docker CLI plugins directory..."
@mkdir -p "$(PLUGIN_DIR)"
@cp -f $(BINARY_WITH_EXT) "$(PLUGIN_DIR)/$(PLUGIN_WITH_EXT)"
@echo "Plugin installed: $(PLUGIN_DIR)/$(PLUGIN_WITH_EXT)"
else
@echo "Linking $(BINARY_WITH_EXT) to Docker CLI plugins directory..."
@mkdir -p "$(PLUGIN_DIR)"
@ln -sf $(shell pwd)/$(BINARY_WITH_EXT) "$(PLUGIN_DIR)/$(PLUGIN_WITH_EXT)"
@echo "Link created: $(PLUGIN_DIR)/$(PLUGIN_WITH_EXT)"
endif

install: build link

Expand Down Expand Up @@ -68,7 +87,7 @@ unit-tests:

clean:
@echo "Cleaning up..."
@rm -f $(BINARY_NAME)
@rm -f $(BINARY_WITH_EXT)
@echo "Cleaned!"

docs:
Expand Down
125 changes: 103 additions & 22 deletions llamacpp/Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,20 +1,27 @@
ifeq ($(OS),Windows_NT)
DETECTED_OS := Windows
else
UNAME_S := $(shell uname -s)
ifeq ($(UNAME_S),Linux)
DETECTED_OS := Linux
endif
ifeq ($(UNAME_S),Darwin)
DETECTED_OS := macOS
endif
endif
include ../Makefile.common

BUILD_DIR := build
INSTALL_DIR := install
NATIVE_DIR := native

.PHONY: build clean install-deps build-dir install-dir
# Vulkan SDK version for Windows
VULKAN_VERSION := 1.4.313.2

# Architecture detection for Windows
ifeq ($(DETECTED_OS),Windows)
UNAME_M := $(shell uname -m)
ifeq ($(UNAME_M),x86_64)
DETECTED_ARCH := amd64
else ifeq ($(UNAME_M),aarch64)
DETECTED_ARCH := arm64
else ifeq ($(UNAME_M),arm64)
DETECTED_ARCH := arm64
else
DETECTED_ARCH := amd64
endif
endif

.PHONY: build clean install-deps install-deps-cuda install-deps-opencl build-dir install-dir

build: install-deps
ifeq ($(DETECTED_OS),macOS)
Expand Down Expand Up @@ -48,8 +55,60 @@ else ifeq ($(DETECTED_OS),Linux)
@echo "Linux build not implemented yet"
@exit 1
else ifeq ($(DETECTED_OS),Windows)
@echo "Windows build not implemented yet"
@exit 1
ifeq ($(DETECTED_ARCH),amd64)
@echo "Building for Windows AMD64 (CPU with Vulkan)..."
@echo "Configuring CMake..."
PATH="C:/Program Files/LLVM/bin;$$PATH" cmake -B $(BUILD_DIR) \
-DCMAKE_BUILD_TYPE=Release \
-DGGML_NATIVE=OFF \
-DGGML_OPENMP=OFF \
-DDDLLAMA_BUILD_UTILS=ON \
-GNinja \
-DLLAMA_CURL=OFF \
-DBUILD_SHARED_LIBS=ON \
-DGGML_BACKEND_DL=ON \
-DGGML_CPU_ALL_VARIANTS=ON \
-DGGML_VULKAN=ON \
"-DCMAKE_C_COMPILER=C:/Program Files/LLVM/bin/clang.exe" \
"-DCMAKE_CXX_COMPILER=C:/Program Files/LLVM/bin/clang++.exe" \
"-DCMAKE_RC_COMPILER=C:/Program Files/LLVM/bin/llvm-rc.exe" \
-S $(NATIVE_DIR)
@echo "Building..."
PATH="C:/Program Files/LLVM/bin;$$PATH" cmake --build $(BUILD_DIR) --config Release
Comment on lines +61 to +77
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot Dec 4, 2025

Choose a reason for hiding this comment

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

high

The path to the LLVM toolchain is hardcoded. This can cause issues if LLVM is installed in a different location. It's better to define a variable for the LLVM path at the top of the file (e.g., LLVM_PATH ?= C:/Program Files/LLVM), which can be easily overridden if needed.

PATH="$(LLVM_PATH)/bin;$$PATH" cmake -B $(BUILD_DIR) \
		-DCMAKE_BUILD_TYPE=Release \
		-DGGML_NATIVE=OFF \
		-DGGML_OPENMP=OFF \
		-DDDLLAMA_BUILD_UTILS=ON \
		-GNinja \
		-DLLAMA_CURL=OFF \
		-DBUILD_SHARED_LIBS=ON \
		-DGGML_BACKEND_DL=ON \
		-DGGML_CPU_ALL_VARIANTS=ON \
		-DGGML_VULKAN=ON \
		"-DCMAKE_C_COMPILER=$(LLVM_PATH)/bin/clang.exe" \
		"-DCMAKE_CXX_COMPILER=$(LLVM_PATH)/bin/clang++.exe" \
		"-DCMAKE_RC_COMPILER=$(LLVM_PATH)/bin/llvm-rc.exe" \
		-S $(NATIVE_DIR)
	@echo "Building..."
	PATH="$(LLVM_PATH)/bin;$$PATH" cmake --build $(BUILD_DIR) --config Release

@echo "Installing..."
cmake --install $(BUILD_DIR) \
--config Release \
--prefix $(INSTALL_DIR)
@echo "Cleaning install directory..."
rm -f $(INSTALL_DIR)/bin/*.py
rm -rf $(INSTALL_DIR)/lib/cmake
rm -rf $(INSTALL_DIR)/lib/pkgconfig
rm -rf $(INSTALL_DIR)/include
@echo "Build complete! Binaries are in $(INSTALL_DIR)"
Comment on lines +82 to +87
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot Dec 4, 2025

Choose a reason for hiding this comment

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

medium

This block of code for cleaning the install directory is duplicated in the arm64 build section (lines 105-110). To improve maintainability and reduce redundancy, you could define a multi-line variable using define for these commands and call it from both places. For example:

define WINDOWS_POST_INSTALL_CLEAN
	@echo "Cleaning install directory..."
	rm -f $(INSTALL_DIR)/bin/*.py
	rm -rf $(INSTALL_DIR)/lib/cmake
	rm -rf $(INSTALL_DIR)/lib/pkgconfig
	rm -rf $(INSTALL_DIR)/include
	@echo "Build complete! Binaries are in $(INSTALL_DIR)"
endef

Then use $(call WINDOWS_POST_INSTALL_CLEAN) in both build sections.

else
@echo "Building for Windows ARM64 (CPU)..."
@echo "Configuring CMake..."
cmake -B $(BUILD_DIR) \
-G Ninja \
-DCMAKE_C_COMPILER="C:/Program Files/LLVM/bin/clang.exe" \
-DCMAKE_CXX_COMPILER="C:/Program Files/LLVM/bin/clang++.exe" \
-DCMAKE_BUILD_TYPE=Release \
-DGGML_NATIVE=OFF \
-DLLAMA_CURL=OFF \
-S $(NATIVE_DIR)
Comment on lines +91 to +98
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot Dec 4, 2025

Choose a reason for hiding this comment

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

high

Similar to the amd64 build, the path to the LLVM toolchain is hardcoded here. This should be replaced with a variable (e.g., LLVM_PATH) to make the build script more robust and configurable.

cmake -B $(BUILD_DIR) \
		-G Ninja \
		-DCMAKE_C_COMPILER="$(LLVM_PATH)/bin/clang.exe" \
		-DCMAKE_CXX_COMPILER="$(LLVM_PATH)/bin/clang++.exe" \
		-DCMAKE_BUILD_TYPE=Release \
		-DGGML_NATIVE=OFF \
		-DLLAMA_CURL=OFF \
		-S $(NATIVE_DIR)

@echo "Building..."
cmake --build $(BUILD_DIR) --config Release
@echo "Installing..."
cmake --install $(BUILD_DIR) \
--config Release \
--prefix $(INSTALL_DIR)
@echo "Cleaning install directory..."
rm -f $(INSTALL_DIR)/bin/*.py
rm -rf $(INSTALL_DIR)/lib/cmake
rm -rf $(INSTALL_DIR)/lib/pkgconfig
rm -rf $(INSTALL_DIR)/include
@echo "Build complete! Binaries are in $(INSTALL_DIR)"
Comment on lines +105 to +110
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot Dec 4, 2025

Choose a reason for hiding this comment

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

medium

This is a duplicate of the cleanup code in the amd64 build section (lines 82-87). Consider refactoring this into a reusable block to avoid code duplication, as suggested in the comment on that section.

endif
else
@echo "Unsupported OS: $(DETECTED_OS)"
@exit 1
Expand All @@ -68,8 +127,30 @@ else ifeq ($(DETECTED_OS),Linux)
@echo "Linux dependency installation not implemented yet"
@exit 1
else ifeq ($(DETECTED_OS),Windows)
@echo "Windows dependency installation not implemented yet"
@exit 1
ifeq ($(DETECTED_ARCH),amd64)
@echo "Installing build dependencies for Windows AMD64 (CPU with Vulkan)..."
@echo "Installing Ninja..."
choco install ninja -y
@echo "Installing CMake..."
choco install cmake -y --installargs 'ADD_CMAKE_TO_PATH=System'
@echo "Installing LLVM (clang)..."
choco install llvm -y
@echo "Adding LLVM to system PATH..."
@powershell -Command "[System.Environment]::SetEnvironmentVariable('PATH', 'C:\\Program Files\\LLVM\\bin;' + [System.Environment]::GetEnvironmentVariable('PATH', 'Machine'), 'Machine')" || echo "LLVM PATH update attempted"
@if [ -d "/c/VulkanSDK/$(VULKAN_VERSION)" ]; then \
echo "Vulkan SDK $(VULKAN_VERSION) already installed"; \
else \
echo "Installing Vulkan SDK $(VULKAN_VERSION)..."; \
curl.exe -o "$$TEMP/VulkanSDK-Installer.exe" -L "https://sdk.lunarg.com/sdk/download/$(VULKAN_VERSION)/windows/vulkansdk-windows-X64-$(VULKAN_VERSION).exe"; \
"$$TEMP/VulkanSDK-Installer.exe" --accept-licenses --default-answer --confirm-command install; \
powershell -Command "[System.Environment]::SetEnvironmentVariable('VULKAN_SDK', 'C:\\VulkanSDK\\$(VULKAN_VERSION)', 'User')"; \
powershell -Command "[System.Environment]::SetEnvironmentVariable('PATH', [System.Environment]::GetEnvironmentVariable('PATH', 'User') + ';C:\\VulkanSDK\\$(VULKAN_VERSION)\\bin', 'User')"; \
echo "Vulkan SDK $(VULKAN_VERSION) installed. Please restart your shell for changes to take effect."; \
fi
Comment on lines +139 to +149
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot Dec 4, 2025

Choose a reason for hiding this comment

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

high

The paths for LLVM and Vulkan SDK are hardcoded. This can lead to problems if the user has installed them in non-default locations. It would be more robust to use variables for these paths, with sensible defaults. For example:

LLVM_INSTALL_PATH ?= C:\\Program Files\\LLVM
VULKAN_INSTALL_PATH ?= C:\\VulkanSDK

Then use $(LLVM_INSTALL_PATH) and $(VULKAN_INSTALL_PATH) in these commands.

else
@echo "Installing build dependencies for Windows ARM64 (CPU)..."
@echo "Ninja and LLVM should already be installed on ARM64 runners"
endif
else
@echo "Unsupported OS: $(DETECTED_OS)"
@exit 1
Expand All @@ -87,9 +168,9 @@ install-dir:

help:
@echo "Available targets:"
@echo " build - Build llama.cpp (macOS only for now)"
@echo " install-deps - Install build dependencies"
@echo " build-dir - Print build directory path"
@echo " install-dir - Print install directory path"
@echo " clean - Clean build artifacts"
@echo " help - Show this help"
@echo " build - Build llama.cpp (macOS and Windows)"
@echo " install-deps - Install build dependencies (CPU with Vulkan for Windows AMD64)"
@echo " build-dir - Print build directory path"
@echo " install-dir - Print install directory path"
@echo " clean - Clean build artifacts"
@echo " help - Show this help"
Loading