From 3085c9d9daab611a2a20a1db44868a2e4c8f759e Mon Sep 17 00:00:00 2001 From: dundargoc Date: Tue, 27 Aug 2024 13:22:17 +0200 Subject: revert: "Makefile: use pattern rules for build/.deps (#10366)" This reverts commit 7f6ff829aa2347eb940d8e70a825ea335d8f15ed. The given reasoning and usecase is unsatisfactory. While it is true that it allows to run `make build/bin/nvim`, it can easily be recreated with `ninja -C build bin/nvim` which does the exact same thing. This minor convenience is not worth adding the extra code given how rare this usecase should be. --- Makefile | 10 ---------- 1 file changed, 10 deletions(-) (limited to 'Makefile') diff --git a/Makefile b/Makefile index c44be8f8c7..58640fd25d 100644 --- a/Makefile +++ b/Makefile @@ -155,14 +155,4 @@ appimage: appimage-%: bash scripts/genappimage.sh $* -# Generic pattern rules, allowing for `make build/bin/nvim` etc. -# Does not work with "Unix Makefiles". -ifeq ($(CMAKE_GENERATOR),Ninja) -build/%: phony_force - $(BUILD_TOOL) -C build $(patsubst build/%,%,$@) - -$(DEPS_BUILD_DIR)/%: phony_force - $(BUILD_TOOL) -C $(DEPS_BUILD_DIR) $(patsubst $(DEPS_BUILD_DIR)/%,%,$@) -endif - .PHONY: test clean distclean nvim libnvim cmake deps install appimage checkprefix benchmark $(FORMAT) $(LINT) $(TEST) -- cgit From 4ee65484b16da9c51e6e1fc3b0d31f74259894f4 Mon Sep 17 00:00:00 2001 From: dundargoc Date: Fri, 30 Aug 2024 13:04:20 +0200 Subject: build: make makefile work on windows Using powershell as the default windows shell as using cmd alters $PATH in a way that makes building neovim fail (powershell prioritizes visual studio tools which is arguably more correct). This was tested with gnu make for windows, which can be installed with e.g. scoop. It does not work with nmake and it is extremely unlikely we want to add nmake support as the makefile is merely supposed to be syntactic sugar for the most common case. For similar reasons, the only supported generator is ninja. --- Makefile | 69 ++++++++++++++++++++++++++++++++++++++-------------------------- 1 file changed, 41 insertions(+), 28 deletions(-) (limited to 'Makefile') diff --git a/Makefile b/Makefile index 58640fd25d..9317bea509 100644 --- a/Makefile +++ b/Makefile @@ -1,3 +1,25 @@ +ifeq ($(OS),Windows_NT) + SHELL := powershell.exe + .SHELLFLAGS := -NoProfile -NoLogo + MKDIR := @$$null = new-item -itemtype directory -force + TOUCH := @$$null = new-item -force + RM := remove-item -force + CMAKE := cmake + CMAKE_GENERATOR := Ninja + define rmdir + if (Test-Path $1) { remove-item -recurse $1 } + endef +else + MKDIR := mkdir -p + TOUCH := touch + RM := rm -rf + CMAKE := $(shell (command -v cmake3 || echo cmake)) + CMAKE_GENERATOR ?= $(shell (command -v ninja > /dev/null 2>&1 && echo "Ninja") || echo "Unix Makefiles") + define rmdir + rm -rf $1 + endef +endif + MAKEFILE_PATH := $(abspath $(lastword $(MAKEFILE_LIST))) MAKEFILE_DIR := $(dir $(MAKEFILE_PATH)) @@ -9,7 +31,6 @@ filter-true = $(strip $(filter-out 1 on ON true TRUE,$1)) all: nvim -CMAKE ?= $(shell (command -v cmake3 || echo cmake)) CMAKE_FLAGS := -DCMAKE_BUILD_TYPE=$(CMAKE_BUILD_TYPE) # Extra CMake flags which extend the default set CMAKE_EXTRA_FLAGS ?= @@ -37,21 +58,11 @@ else checkprefix: ; endif -CMAKE_GENERATOR ?= $(shell (command -v ninja > /dev/null 2>&1 && echo "Ninja") || \ - echo "Unix Makefiles") -DEPS_BUILD_DIR ?= .deps +DEPS_BUILD_DIR ?= ".deps" ifneq (1,$(words [$(DEPS_BUILD_DIR)])) $(error DEPS_BUILD_DIR must not contain whitespace) endif -ifeq (,$(BUILD_TOOL)) - ifeq (Ninja,$(CMAKE_GENERATOR)) - BUILD_TOOL = ninja - else - BUILD_TOOL = $(MAKE) - endif -endif - DEPS_CMAKE_FLAGS ?= USE_BUNDLED ?= @@ -61,7 +72,7 @@ endif ifneq (,$(findstring functionaltest-lua,$(MAKECMDGOALS))) BUNDLED_LUA_CMAKE_FLAG := -DUSE_BUNDLED_LUA=ON - $(shell [ -x $(DEPS_BUILD_DIR)/usr/bin/lua ] || rm build/.ran-*) + $(shell [ -x $(DEPS_BUILD_DIR)/usr/bin/lua ] || $(RM) build/.ran-*) endif # For use where we want to make sure only a single job is run. This does issue @@ -69,34 +80,33 @@ endif SINGLE_MAKE = export MAKEFLAGS= ; $(MAKE) nvim: build/.ran-cmake deps - $(BUILD_TOOL) -C build + $(CMAKE) --build build libnvim: build/.ran-cmake deps - $(BUILD_TOOL) -C build libnvim + $(CMAKE) --build build --target libnvim cmake: - touch CMakeLists.txt + $(TOUCH) CMakeLists.txt $(MAKE) build/.ran-cmake build/.ran-cmake: | deps - $(CMAKE) -B build -G '$(CMAKE_GENERATOR)' $(CMAKE_FLAGS) $(CMAKE_EXTRA_FLAGS) $(MAKEFILE_DIR) - touch $@ + $(CMAKE) -B build -G $(CMAKE_GENERATOR) $(CMAKE_FLAGS) $(CMAKE_EXTRA_FLAGS) $(MAKEFILE_DIR) + $(TOUCH) $@ deps: | build/.ran-deps-cmake ifeq ($(call filter-true,$(USE_BUNDLED)),) - $(BUILD_TOOL) -C $(DEPS_BUILD_DIR) + $(CMAKE) --build $(DEPS_BUILD_DIR) endif ifeq ($(call filter-true,$(USE_BUNDLED)),) $(DEPS_BUILD_DIR): - mkdir -p "$@" + $(MKDIR) $@ build/.ran-deps-cmake:: $(DEPS_BUILD_DIR) - $(CMAKE) -S $(MAKEFILE_DIR)/cmake.deps -B $(DEPS_BUILD_DIR) -G '$(CMAKE_GENERATOR)' \ - $(BUNDLED_CMAKE_FLAG) $(BUNDLED_LUA_CMAKE_FLAG) $(DEPS_CMAKE_FLAGS) + $(CMAKE) -S $(MAKEFILE_DIR)/cmake.deps -B $(DEPS_BUILD_DIR) -G $(CMAKE_GENERATOR) $(BUNDLED_CMAKE_FLAG) $(BUNDLED_LUA_CMAKE_FLAG) $(DEPS_CMAKE_FLAGS) endif build/.ran-deps-cmake:: - mkdir -p build - touch $@ + $(MKDIR) build + $(TOUCH) "$@" # TODO: cmake 3.2+ add_custom_target() has a USES_TERMINAL flag. oldtest: | nvim @@ -113,7 +123,7 @@ test/old/testdir/%.vim: phony_force nvim $(SINGLE_MAKE) -C test/old/testdir NVIM_PRG=$(NVIM_PRG) SCRIPTS= $(MAKEOVERRIDES) $(patsubst test/old/testdir/%.vim,%,$@) functionaltest-lua: | nvim - $(BUILD_TOOL) -C build functionaltest + $(CMAKE) --build build --target functionaltest FORMAT=formatc formatlua format LINT=lintlua lintsh lintc clang-analyzer lintcommit lintdoc lint @@ -135,16 +145,19 @@ iwyu: build/.ran-cmake $(CMAKE) --build build clean: - test -d build && $(BUILD_TOOL) -C build clean || true +ifneq ($(wildcard build),) + $(CMAKE) --build build --target clean +endif $(MAKE) -C test/old/testdir clean $(MAKE) -C runtime/indent clean distclean: - rm -rf $(DEPS_BUILD_DIR) build + $(call rmdir, $(DEPS_BUILD_DIR)) + $(call rmdir, build) $(MAKE) clean install: checkprefix nvim - $(BUILD_TOOL) -C build install + $(CMAKE) --install build appimage: bash scripts/genappimage.sh -- cgit From ef8067a19d981388a14407ea08245811cf5b3604 Mon Sep 17 00:00:00 2001 From: dundargoc Date: Mon, 2 Sep 2024 12:51:30 +0200 Subject: build: add quotes around `CMAKE_GENERATOR` variable This will fix the following error when using generators that have a space in them, e.g. "Unix Makefiles": "CMake Error: Could not create named generator Unix". Closes https://github.com/neovim/neovim/issues/30218. --- Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'Makefile') diff --git a/Makefile b/Makefile index 9317bea509..84d4859bb3 100644 --- a/Makefile +++ b/Makefile @@ -14,7 +14,7 @@ else TOUCH := touch RM := rm -rf CMAKE := $(shell (command -v cmake3 || echo cmake)) - CMAKE_GENERATOR ?= $(shell (command -v ninja > /dev/null 2>&1 && echo "Ninja") || echo "Unix Makefiles") + CMAKE_GENERATOR ?= "$(shell (command -v ninja > /dev/null 2>&1 && echo "Ninja") || echo "Unix Makefiles")" define rmdir rm -rf $1 endef -- cgit From e268fcbdaa1e0e0cee3b513e62581d35bb937d40 Mon Sep 17 00:00:00 2001 From: dundargoc Date: Fri, 20 Sep 2024 14:50:32 +0200 Subject: build: work around bug in make when PATH includes cmake as dir There appears to be a bug in `make` where if a rule asks `make` to invoke a command called `foo`, and `foo` exists somewhere in `$PATH` as a directory (not an executable file), `make` will attempt to `execve` that directory instead of continuing to search in later parts of the `$PATH` for `foo` as a true executable. The cause can be traced back to a bug in Make 4.3 which stems from their use of the findprog function in Gnulib. This was reported to the Make maintainers here: https://savannah.gnu.org/bugs/index.php?57962 and then forwarded to the Gnulib maintainers here: https://github.com/coreutils/gnulib/commit/7b1de4a Make 4.4 does not have this bug, and I can confirm that I'm able to run make in the Neovim repo with no further modifications to my system than upgrading the version of make I'm using to 4.4 or 4.4.1. As the change is small enough, and it's unlikely that make version around the world is going to be updated in a timely manner, it makes sense to just add a workaround for this. Using `command -v` to resolve the `cmake` command, similar to what is already being done with `cmake3`, makes it work correctly in all cases. Continuing to include `... || echo cmake` at the end means that if neither `cmake3` nor `cmake` are installed, the user will still see a message about CMake being missing. Co-authored-by: Jake Zimmerman --- Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'Makefile') diff --git a/Makefile b/Makefile index 84d4859bb3..fe83f302e8 100644 --- a/Makefile +++ b/Makefile @@ -13,7 +13,7 @@ else MKDIR := mkdir -p TOUCH := touch RM := rm -rf - CMAKE := $(shell (command -v cmake3 || echo cmake)) + CMAKE := $(shell (command -v cmake3 || command -v cmake || echo cmake)) CMAKE_GENERATOR ?= "$(shell (command -v ninja > /dev/null 2>&1 && echo "Ninja") || echo "Unix Makefiles")" define rmdir rm -rf $1 -- cgit