Drop the relocatable build flag; test sysdir resolution - #613
Merged
Merged
Conversation
Move initSysdir() from settings.cc to locate.cc, alongside the rest of the file-search logic. Extend the fallback from macOS-only to all platforms: Linux uses /proc/self/exe, Windows uses GetModuleFileNameA. Add a build-tree fallback (base/ next to the executable) in addition to the install-tree fallback (../share/asymptote relative to bin/). Add ENABLE_RELOCATABLE CMake option (default off) that defines IS_RELOCATABLE; the devcontainer presets already enable it. Makefile.in: factor license-file installation into a define so copy-licenses and install-licenses share one list; add the GL shader subdirectory to uninstall-asy cleanup.
A relocatable binary tried the compiled-in ASYMPTOTE_SYSDIR first, so an
Asymptote installed at the configured prefix captured resolution: a build-tree
binary silently loaded the installed base/ instead of its own. Try
executable-relative candidates first, the compiled-in path last.
- Require plain.asy in a candidate; a bare directory may be a stale install.
- Leave an empty compiled-in sysdir alone: it is the sentinel that sends
initDir() to kpsewhich, and filling it in broke TeXLive builds.
- Add a flat candidate for the MSWindows installer layout.
- Resolve symlinks on macOS; grow the path buffer past MAX_PATH on MSWindows.
- Stop queryRegistry() overwriting an executable-relative sysdir.
The build-tree candidate is ungated: <exedir>/base/plain.asy exists only in a
build tree or flat install. ASYMPTOTE_SYSDIR now comes from settings.cc, which
unlike locate.cc is compiled per executable.
resolveSysdir() returned early when the compiled-in ASYMPTOTE_SYSDIR was empty, so a TeXLive build skipped the executable-relative candidates entirely and always deferred to kpsewhich. Drop the early return: when no candidate matches, the compiled-in path is returned unchanged, empty included, and initDir() consults kpsewhich exactly as before. A TeXLive-configured binary run from its build tree now uses the adjacent base/ instead of the installed texmf tree, so it no longer needs -dir; a deployed one at bin/<platform>/asy still resolves via kpsewhich, which is the only thing that can read TEXMFROOT out of texmf.cnf.
The flag's sole effect is to leave ASYMPTOTE_SYSDIR empty, which makes the binary locate its base directory within the TeX tree at runtime (via kpsewhich) -- a capability, named here for what it does rather than for its TeXLive consumer or its empty-sysdir implementation, so the flag can outlive a change to either. --enable-texlive-build is kept as a deprecated alias that warns and behaves identically, so existing TeXLive/MacTeX build recipes keep working until they migrate.
getExecutablePath() (locate.cc) is the one part of sysdir resolution written three times -- GetModuleFileNameA, _NSGetExecutablePath + realpath, /proc/self/exe -- so it is the part that can be wrong on a platform nobody has run the suite on. It is static, and nothing prints it, so tests/ test_executable_path.py checks it through its only observable: resolveSysdir() offers <exedir>/base as its first candidate and reports the result as settings.sysdir. The script copies asy and a base/ into a fresh temporary directory and asks the copy where its sysdir is. Nothing could have compiled that path in, and the answer comes from neither the cwd nor the environment nor an installed Asymptote, so getting it back means the executable's own directory was computed at run time, and correctly. That candidate is outside the IS_RELOCATABLE guard, so the test needs no knowledge of how asy was configured; both the ctest entry and the make target are therefore unconditional. The script is straight-line code -- no functions, no branches -- so that what it asserts can be read off in one pass; its two failure modes are an exception (CalledProcessError, with asy's stderr inherited) and an assert. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
pylint's too-many-lines (C0302) counts raw physical lines, so comments and docstrings tell against the limit: a heavily documented file can trip it while containing very little code, which is backwards for a complexity gate. Add misc/pylint_module_size.py, registering too-many-module-statements (R9001). It counts statements exactly as pylint's own too-many-statements does, so a module's number is comparable to the per-function max-statements, and it ignores prose. too-many-lines is disabled in its favour; max-module-lines stays at its default so that re-enabling it needs no other change. The plugin resolves as an import, so the repository root has to be on sys.path -- pylint 3 does not put it there, hence the new init-hook. Nothing in the tree trips either limit today (largest module: 260 statements against the new limit of 500; longest: 559 lines against 1000), so no current verdict changes. This decides how the next large file gets judged, before there is one. Provenance: I did not type this one. The patch and this message were drafted by the software assistant I keep in my terminal -- statistical, tireless, not a person -- then reviewed, corrected, and tested by me.
tests/test_relocatable.py stages the built binary into the layouts that a real deployment produces, then probes the resolved settings.sysdir. It enumerates exhaustively the 18 states that the resolver's three candidate locations can be in, and adds axes for the launch route, the overrides, the compiled-in sysdir, and the Windows registry. The matrix runs against either build: when ENABLE_RELOCATABLE is off, the rows that would otherwise resolve through <exedir>/../share/asymptote or <exedir> become a regression test that those candidates really are compiled out, and that is the configuration which Asymptote ships by default. Both build systems assert the mode from the build flag rather than let the script auto-detect it, because detection would infer the mode from the very K2 behaviour that the matrix exists to test. The C="" (CTAN) axis needs a binary whose compiled-in sysdir is empty: CMake points at asy-ctan, and configure.ac now exports texmf_sysdir so that the autotools build can offer ../asy whenever it was configured with --enable-texmf-sysdir. ctest builds nothing itself, and asy-ctan is not part of asy-with-basefiles, so a new target, asy-check-test-deps, collects everything that the asy-check-tests label needs; linux-sanity.yml now builds that target. Credit where it is due: my hands did little of the typing. The script, the build glue, and these paragraphs were composed by the tireless pattern-matcher that sits in my terminal, and I then read every line of it, argued with some of them, and ran the suite before committing.
The per-file `asy.<dir>.<name>` test names went away when the .asy suite was bundled into one CTest test, so the documented `-R "asy.types.*"` matched nothing. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
ASYMPTOTE_SYSDIR is an envSetting, so it beats the resolved value: any shell that exports it failed this test with no regression to find. ASYMPTOTE_HOME is redirected rather than dropped, since unset it falls back to $HOME/.asy. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The Windows fall-through value is read HKCU-first, where getEntry() reads HKLM-first, so a machine with both keys set failed every fall-through row with asy behaving as implemented. An empty value and a REG_EXPAND_SZ one diverged the same way. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
The GC stamp reruns its recipe without invalidating existing objects, so changed architecture flags may not rebuild GC correctly.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Suppressed comments (1)
Makefile.in:345
- Changing the stamp does rerun this outer recipe, but the nested
makecan still consider the existing gc object files and library up to date: make does not track changes to compiler flags, and rerunningconfiguredoes not itself invalidate those objects. A switch such as single-arch to universal can therefore keep the old objects, defeating the stamp's stated purpose. Clean gc after configuring (or make its objects depend on the stamp) before rebuilding.
$(GCLIB): $(GCSTAMP)
-cd $(GC) && ln -sf ../libatomic_ops libatomic_ops
cd $(GC) && \
./configure CC="$(CC)" CXX="$(CXX)" CFLAGS="$(MACOS_CFLAGS) $(CFLAGS)" CXXFLAGS="$(MACOS_CFLAGS) $(CXXFLAGS)" $(GCOPTIONS)
$(MAKE) -C $(GC) all CFLAGS_EXTRA="$(MACOS_CFLAGS)"
- Files reviewed: 31/32 changed files
- Comments generated: 1
- Review effort level: Balanced
test_relocatable.py now asks the binary whether it lists kpsewhich among its enabled options, so a stale binary can't pass for a fresh one. Drops the texlive_build/ASY_TEXLIVE_BUILD plumbing and the unused CTAN_BUILD define. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Drop or shorten explanatory comments that restate what the code or history already shows, and revert the texlive-build help string in configure.ac to its master wording. No functional change. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The KPSEWHICH (CTAN/TeXLive) build no longer calls resolveSysdir(): its data directory is defined only by kpathsea, so an adjacent base/ is ignored and initDir() resolves sysdir with kpsewhich at startup. With KPSEWHICH now tested directly, an empty systemDir no longer has to signal the TeXLive build, so queryRegistry() drops that check and the Windows build compiles in "" instead of the "NUL" placeholder. The relocatable test passes --compiled-in= in one argument so that an empty value survives, and its ctan/hit row now expects the sysdir kpsewhich predicts (SKIP when kpsewhich is unavailable) rather than the adjacent base/. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This branch was successfully deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
resolveSysdir(), the per-platformexecutablePath(), and the preference for abase/beside the executable are all already on master. This branch finishes that work: it deletes the build-time opt-in and the two extra locations that opt-in gated, adds a FreeBSD branch toexecutablePath(), and adds the tests.Remove
--enable-relocatableandIS_RELOCATABLE, and with them two of the three candidate locations (<exedir>/../share/asymptote, and base files flat beside the binary). One candidate remains:<exedir>/base, accepted only if it containsplain.asy— true only in a build tree or a distribution that deliberately shipsbase/beside the binary, which is why it needs no opt-in. Otherwise the compiled-inASYMPTOTE_SYSDIRis returned unchanged. An empty compiled-in sysdir no longer short-circuits resolution; it is just the fallback thatinitDir()hands to kpsewhich, so a TeXLive-configured binary run from its build tree finds its ownbase/. The macOS bundle now relocates by layout (make install-asy bindir=$STAGE/Asymptote asydir=$STAGE/Asymptote/base), and the devcontainer preset template drops itsENABLE_RELOCATABLEcache entries, which matched no CMake option and did nothing.Fix
ASYMPTOTE_SYSDIRon the CTAN binary. Definitions inherited fromasycoreare emitted after a target's own, so the global-DASYMPTOTE_SYSDIRbeat the per-target override andasy-ctanwas silently built with the normal sysdir. It is no longer set globally;add_settings_objtakes it as a required parameter.Tests.
tests/test_executable_path.pyis a straight-line smoke test ofexecutablePath()on the host it runs on — the one part written once per OS, so the part likeliest to be wrong where nobody has tried it (the new, untested FreeBSDkern.proc.pathnamebranch exists because/procis not mounted there).tests/test_relocatable.pyis the matrix: the 18 layout states of the candidate plus the two former candidates as negative controls, exhaustively, with axes for launch route,-sysdir/-dir/ASYMPTOTE_SYSDIR, the compiled-in value including the empty CTAN one, and the Windows registry. Both run under ctest andmake check. Newasy-check-test-depstarget, since ctest builds nothing andasy-with-basefilesdoes not pull inasy-ctan;tests/Makefilealso stops trying to run__pycache__/*.asyas a suite.Python checking.
mypy.ini(allowlist-driven, floor 3.7), run once per platform in CI since mypy prunessys.platformbranches;pyrightconfig.jsonfor editors;mypy~=1.8.0, the last release able to target 3.7;PY3_MINIMUM_VERSIONenforced by the CMake interpreter search. A pylint plugin replacestoo-many-lineswith a statement-based limit, so documentation does not count against a module, andprint_non_gui_py_files_for_linting.pyasks git for candidates instead of walking the tree. Annotatinggenerate_enums.pyturned up a real bug:-xopt spaces=Narrives as a string and was used as a repeat count.Makefile.in cleanups salvaged from the abandoned libatomic_ops work. License installation factored into one define shared by
copy-licensesandinstall-licenses(which no longer installs by way ofdoc/licenses, and creates$(docdir)itself, since--with-licensedircan point elsewhere); GL shaders removed byuninstall-asy.Build and packaging fixes. A stamp rebuilds gc when the macOS architecture flags or gc's configure options change; a
touchafter macOS Vulkan bundling stops asy being relinked on every later make. The bundler strips the absoluteLC_RPATHs the linker recorded — they point into the builder's home and ship in the.dmg— refusing to if any@rpathreference survived the rewrite.sync-vcpkg-baseline.shrecords the baseline it bootstrapped for, so an interrupted bootstrap is retried.Plus doc corrections in
INSTALL-VCPKG.mdand.devcontainer/README.md: the real ctest test names,--tests-listfor running a subset, and whyAC_PREREQstays at 2.71.