Makefile: remove build prerequisites from install targets (fixes #8971)#8976
Makefile: remove build prerequisites from install targets (fixes #8971)#8976ThomsenDrake wants to merge 1 commit intoElementsProject:masterfrom
Conversation
…entsProject#8971) make install should only copy pre-built files, never trigger builds. Previously, install-program listed $(BIN_PROGRAMS), $(PKGLIBEXEC_PROGRAMS), $(PLUGINS), and $(PY_PLUGINS) as prerequisites, causing make to rebuild binaries if they were missing or stale. Similarly, install-data listed man page prerequisites, triggering Python-based man page generation. This is problematic when running `sudo make install` because sudo has a different environment (missing per-user Rust, uv, Python modules like mako, etc.). After this change, `make install` only creates directories (via installdirs) and copies files. If expected files are missing, the install commands will fail with clear error messages indicating which files need to be built first. Fixes ElementsProject#8971
vincenzopalazzo
left a comment
vincenzopalazzo
left a comment
There was a problem hiding this comment.
Requesting changes for one regression: removing the install target prerequisites breaks make check / make installcheck (and likely make bin-tarball) on clean trees because those workflows no longer build generated manpages and Rust default targets before staging the install tree. If the goal is to keep make install side-effect free, those build dependencies need to move up to the higher-level targets that still expect installable artifacts.
| DOC_DATA = README.md LICENSE | ||
|
|
||
| install-data: installdirs $(MAN1PAGES) $(MAN5PAGES) $(MAN7PAGES) $(MAN8PAGES) $(DOC_DATA) | ||
| install-data: installdirs |
There was a problem hiding this comment.
This makes installcheck stop being self-contained. That target still only depends on all-programs, so on a clean tree it reaches make install without the manpages from doc-all; on Rust-enabled builds it can also miss the builtin plugins, since those live under DEFAULT_TARGETS, not ALL_PROGRAMS. Today the prerequisites removed in this PR are what make installcheck/check and bin-tarball work without a prior full make. Can we move the build deps up to those higher-level targets instead of dropping them here?
…rball (fixes ElementsProject#8971) install-data should only copy files, not trigger builds. This is important because `make install` is often run with `sudo` which has a different environment (missing Python deps, Rust toolchain, etc). Move doc-all and default-targets build dependencies from install-data to: - installcheck: ensure manpages and Rust plugins exist before staging - bin-tarball: ensure complete tarball with all artifacts `make install` itself remains pure copy, while higher-level targets that expect installable artifacts still trigger the necessary builds. Fixes ElementsProject#8971 Addresses review feedback on ElementsProject#8976
|
Closing in favor of #8996 which uses the correct approach (moving build prereqs to installcheck/bin-tarball instead of removing them), per maintainer feedback. |
2 participants
Fixes #8971
Problem
make installtriggers builds (wiregen, rust compilation, python scripts) because bothinstall-programandinstall-datalist build outputs ($(BIN_PROGRAMS),$(PKGLIBEXEC_PROGRAMS),$(PLUGINS),$(PY_PLUGINS),$(MAN1PAGES), etc.) as Makefile prerequisites.This means
make installcan fail if build dependencies are missing, even when all artifacts are already built. This is particularly problematic for:make installin a clean environmentuv run makebut install with plainmake installFix
Remove build output prerequisites from both
install-programandinstall-datatargets somake installonly copies pre-built files, as documented in the comment now added to the Makefile.Building remains available via
makeormake all.Changes
Makefile: 2-line change (remove$(BIN_PROGRAMS) $(PKGLIBEXEC_PROGRAMS) $(PLUGINS) $(PY_PLUGINS)from install-program, remove$(MAN1PAGES) $(MAN5PAGES) $(MAN7PAGES) $(MAN8PAGES) $(DOC_DATA)from install-data)make installshould only ever copy files #8971