build: address code-review findings (low-risk cleanups)

From an independent review of the branch:

* Makefile.in: `tcltk` is in both INSTALL_CAD_DIRS and PROGRAMS (Tcl build),
  so install@tcltk / install-tcl@tcltk were defined twice -> "given more than
  once" warnings on every command.  Wrap both install lists in $(sort ...) to
  dedup (as CLEAN_TARGETS already does).  Also fix a stale comment (module@ now
  waits for ${DATABASE_H}, not a `depend` phase).

* toolchains/emscripten/defs.mak: the scmos display-file embed globbed
  `mos.*.dstyle`, which misses mos.7bit.mraster_dstyle (an underscore, not a
  dot) -- so the mono-raster style was not embedded in WASM, and the comment
  wrongly called that file non-existent.  Glob `mos.*dstyle` to cover both.

* npm/build.sh: sed_strip_cr wrote the file back unconditionally, bumping the
  mtimes of configure and every scripts/ file on each WASM build (contradicting
  the "source never mutated" guarantee and able to force a coexisting native
  rebuild).  Only write back when a CR was actually stripped.

* graphics/Makefile.in: remove the dead DEPSRCS block left behind by retiring
  the Depend machinery (it also referenced an undefined ${THREE_D_SRCS}).

* scripts/defs.mak.in: clarify that only the SOURCE_DATE_EPOCH branch is UTC.

Verified: 0 duplicate-target warnings, wildcard now lists mos.7bit.mraster_dstyle,
full Tcl build+install rc=0 (375 files), graphics lib builds, build.sh syntax OK.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
Darryl L. Miles 2026-07-23 15:19:22 +00:00 committed by R. Timothy Edwards
parent eaf42fab01
commit 67d6a2ff5a
5 changed files with 18 additions and 31 deletions

View File

@ -61,8 +61,9 @@ config:
# -f path). A phase aggregate (depend, modules, libs, ...) is then just a list
# of its per-dir targets as prerequisites -- no shell `for` loops, no FORCE
# pattern-rule hack. Ordering between phases is expressed as prerequisites on the
# per-dir targets (e.g. every module@ waits for `depend`), so the graph is correct
# under `make -jN`, not just left-to-right in a serial build. '@' never appears
# per-dir targets (e.g. every module@ waits for the generated header ${DATABASE_H}),
# so the graph is correct under `make -jN`, not just left-to-right in a serial
# build. '@' never appears
# in a real target name, so it is a safe namespace separator.
# ---------------------------------------------------------------------------
@ -169,7 +170,7 @@ install-magic:
@${MAKE} install-real
# install@<dir>: install one subdir's artifacts (CAD dirs, then the programs).
INSTALL_TARGETS := $(addprefix install@,${INSTALL_CAD_DIRS} ${PROGRAMS})
INSTALL_TARGETS := $(addprefix install@,$(sort ${INSTALL_CAD_DIRS} ${PROGRAMS}))
.PHONY: $(INSTALL_TARGETS)
$(INSTALL_TARGETS): install@%: install-dirs
@$(call submake,$*,install)
@ -192,7 +193,7 @@ install-tcl:
@${MAKE} install-tcl-real
# install-tcl@<dir>: install one subdir's Tcl-flavoured artifacts.
INSTALLTCL_TARGETS := $(addprefix install-tcl@,${INSTALL_CAD_DIRS} ${PROGRAMS})
INSTALLTCL_TARGETS := $(addprefix install-tcl@,$(sort ${INSTALL_CAD_DIRS} ${PROGRAMS}))
.PHONY: $(INSTALLTCL_TARGETS)
$(INSTALLTCL_TARGETS): install-tcl@%: install-tcl-dirs
@$(call submake,$*,install-tcl)

View File

@ -31,24 +31,6 @@ X11HELP_PROG = XHelper7
include ${MAGICDIR}/defs.mak
SRCS = ${BASE_SRCS} ${GR_SRCS}
DEPSRCS = ${BASE_SRCS}
ifeq (${MAKE_X11},1)
DEPSRCS += ${X11_SRCS} ${TK_SRCS}
endif
ifeq (${MAKE_GL},1)
DEPSRCS += ${OGL_GL_SRCS} ${TOGL_GL_SRCS}
ifeq (${MAKE_GLU},1)
DEPSRCS += ${OGL_GLU_SRCS} ${TOGL_GLU_SRCS}
endif
DEPSRCS += ${THREE_D_SRCS}
endif
ifeq (${MAKE_X11},1)
DEPSRCS += ${TKCOMMON_SRCS}
DEPSRCS += ${X11HELPER_SRCS} ${X11THREAD_SRCS}
endif
ifeq (${MAKE_CAIRO},1)
DEPSRCS += ${TCAIRO_SRCS}
endif
# Note GR_SRCS is from defs.mak
OBJS = ${BASE_SRCS:.c=.o} ${GR_SRCS:.c=.o}
CFLAGS += ${GR_CFLAGS}

View File

@ -90,7 +90,9 @@ ncpu() {
sed_strip_cr() {
local file=$1 tmp
tmp=$(mktemp)
sed 's/\r//' "$file" > "$tmp" && cat "$tmp" > "$file" && rm "$tmp"
# Only write back when a CR was actually stripped, so a clean LF checkout is
# left untouched (no mtime bump that could perturb a coexisting native build).
sed 's/\r//' "$file" > "$tmp" && { cmp -s "$tmp" "$file" || cat "$tmp" > "$file"; } && rm "$tmp"
}
if [ $OPT_RELEASE -eq 1 ]; then

View File

@ -131,7 +131,8 @@ MAGIC_BUILDDATE_DEFS = @MAGIC_BUILDDATE_DEFS@
# skipping it under --disable-magic-builddate avoids the parse-time date spawn.
ifeq (${MAGIC_BUILDDATE_DEFS},)
# Honor SOURCE_DATE_EPOCH (reproducible builds -- appimage/npm tarballs) when set,
# else the current time. UTC so the stamp does not depend on the builder's zone.
# else the current time. The SOURCE_DATE_EPOCH branch uses UTC so a reproducible
# stamp does not depend on the builder's zone; the plain-`date` fallback is local.
MAGIC_BUILDDATE ?= $(shell if [ -n "$$SOURCE_DATE_EPOCH" ]; then \
date -u -d "@$$SOURCE_DATE_EPOCH" 2>/dev/null \
|| date -u -r "$$SOURCE_DATE_EPOCH" 2>/dev/null; \

View File

@ -62,11 +62,12 @@ TOP_EXTRA_LIBS += \
# The ${MAGICDIR}/scmos dir embed above supplies the *generated* tech files, but a
# technology's "styles" section also needs the display styles + colour maps
# (scmos ${FILES}: mos.*.dstyle / mos.*.cmap). Those are *source* files, so out
# of source they are not in the build tree and the embed misses them -- init then
# fails with "Couldn't open color map file mos.7bit.std.cmap" / "Cannot load
# technology". Embed each from the source tree into the same VFS dir. $(wildcard)
# matches only files that exist, so a stale/typo'd name in ${FILES} (e.g. the
# non-existent mos.7bit.mraster_dstyle) can never break the link.
SCMOS_DISPLAY := $(wildcard ${MAGICSRC}/scmos/mos.*.dstyle ${MAGICSRC}/scmos/mos.*.cmap)
# (scmos ${FILES}). Those are *source* files, so out of source they are not in
# the build tree and the embed misses them -- init then fails with "Couldn't open
# color map file mos.7bit.std.cmap" / "Cannot load technology". Embed each from
# the source tree into the same VFS dir. Match `mos.*dstyle` (no dot) so both the
# `.dstyle` files and mos.7bit.mraster_dstyle (an underscore, not a dot) are
# covered, plus the `mos.*.cmap` colour maps. $(wildcard) matches only files that
# exist, so it never lists a name that would break the link.
SCMOS_DISPLAY := $(wildcard ${MAGICSRC}/scmos/mos.*dstyle ${MAGICSRC}/scmos/mos.*.cmap)
TOP_EXTRA_LIBS += $(foreach f,$(SCMOS_DISPLAY),--embed-file $(f)@/magic/sys/current/$(notdir $(f)))