diff --git a/CHANGELOG.md b/CHANGELOG.md index 2310ff86..27b7974a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -134,6 +134,97 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 is not a list of items (`include!("table.rs")` holding `[1, 2, 3]`), is noted on stderr and skipped as a missing `mod` target already was — never a failed scan. +- **A package that uses `@rust_crate` at top level can be precompiled** + ([#339](https://github.com/AtelierArith/RustCall.jl/issues/339)). The macro + evaluated the generated module into an anonymous `Module` under `Main`, so + `Pkg.precompile()` of such a package failed with ``Evaluation into the closed + module `##RustCallCrateRuntime#N` breaks incremental compilation``. The + module is now defined **inside the module that expands the macro** + (`load_crate_bindings(...; target_module = __module__)`), in a hidden, + per-call child namespace (`Caller.var"##RustCallCrateRuntime#N"`), so nothing + the caller did not name appears in its namespace and repeated calls never + collide (the #222 contract). The return value is unchanged, a + `RustCall.CrateBindings`; `load_crate_bindings` called without a + `target_module` keeps the anonymous module. Two consequences for the + generated module: `_LIB_PATH` of an + in-memory `@rust_crate` module is now the **durable** library — RustCall's + cache copy, or Cargo's output — instead of the per-process generation copy, + which is made in `__init__` (as the written file already did since format 6), + because after precompilation `__init__` runs in a later session than the one + that generated the module (visible only through `Bindings.module_ref._LIB_PATH`); + and the module declares that library **and the crate's own input files** — + the set its artifact identity is computed from: the crate directory, every + local `path` dependency, a workspace root's manifest and lockfile, an + out-of-directory `[lib] path` — with `Base.include_dependency`. So editing + `src/lib.rs`, or `RustCall.clear_cache()`, makes the package's precompile + cache stale and the next `using` re-precompiles it and builds the crate + again, instead of `__init__` opening a path that is gone or the package + going on calling a build that no longer matches its source. Tracking the + library alone would not do the second of those: the library is + content-addressed, so a new build lands at a *different* path and leaves the + old file untouched (found in review of + [#351](https://github.com/AtelierArith/RustCall.jl/pull/351)). The + crate is built when the package is precompiled, nothing is written into the + package, and the library is not opened during precompilation (`__init__` is + deferred to load time), so the bindings are callable after the package's + `__init__`, not from its own top level. The generated module imports `Libdl` + through RustCall (`import RustCall.Libdl`), so the package does not need + `Libdl` among its dependencies. + +### Added +- **`@rust_crate ... submodule="Bindings"`** + ([#339](https://github.com/AtelierArith/RustCall.jl/issues/339)) defines the + generated module in the calling module under that name, so a package can + `using .Bindings: f, T` from it — the idiom that pairs with the precompile + fix above, and the same shape as `include("generated/Bindings.jl")`. `name=` + is unchanged: it names the generated module and defines nothing, which is + what keeps the documented `const MyBindings = @rust_crate path name="MyBindings"` + working. The two are separate options on purpose: an earlier cut of this + change made `name=` define the module, and a package written that way + precompiled and then **segfaulted** on load, because the constant was bound + over the module binding the macro had just created (found in review of + [#351](https://github.com/AtelierArith/RustCall.jl/pull/351)). +- **A changed build environment is reported rather than ignored** + ([#339](https://github.com/AtelierArith/RustCall.jl/issues/339)). `RUSTFLAGS`, + `PYO3_PYTHON` and a `PYO3_CONFIG_FILE` pointing at another file decide the + artifact but are not files, so Julia — which invalidates a precompile image + from files — keeps the image and the package loads a library built under the + previous values. The generated module records the environment it was built + under (`artifact_build_env`) and `__init__` warns when it no longer matches, + naming the variables that changed and how to force a rebuild. + [#355](https://github.com/AtelierArith/RustCall.jl/issues/355) tracks + representing such inputs in the invalidation scheme itself. +- **A plain crate's cache key covers the build environment** + ([#339](https://github.com/AtelierArith/RustCall.jl/issues/339)). + `compute_crate_hash` was called without `build_env` on the non-PyO3 path, so + two `cargo build`s under different `RUSTFLAGS` — or a different `CC` a build + script reads, or anything else in the #282 allowlist — shared one cache entry + and the second was handed the first one's library. The PyO3 wrapper path + already folded `artifact_build_env()` in; the plain path does now too, and + like the wrapper it hashes the *contents* of `PYO3_CONFIG_FILE` on top — + the allowlist records the path, and a plain build of a crate that depends on + pyo3 reads the file, so an in-place edit of the configuration is a different + binary under the same key (`_plain_crate_build_env`). One + consequence is that the load-time warning above can be acted on: forcing the + package to be precompiled again really does rebuild the artifact, instead of + finding the stale one under the same key. +- **`@rust_crate cache=false` on a crate that RustCall has to wrap** + ([#339](https://github.com/AtelierArith/RustCall.jl/issues/339)). A crate + whose `[lib]` is not a `cdylib` is bound through a generated wrapper project + in a temporary directory, which is deleted as soon as the build returns. + With caching on, the library had already been copied into the cache; with + `cache = false` nothing copied it, so the generated module named a file that + no longer existed and loading it failed with `could not load library + ".../rustcall_wrapper_XXXXXX/target/release/..."`. The library is now taken + out of the wrapper project before the cleanup — into the cache, or into a + directory of its own under the Cargo cache that outlives the process (a + package precompiled with `cache = false` is loaded by a *later* process, and + a `mktempdir()` cleaned at exit would have taken the recorded `_LIB_PATH` + with it; the PyO3 wrapper path had the same `mktempdir()` and uses the same + home now). `cache = false` is still not the shape to use inside a + package: `docs/src/crate_bindings.md` says which path the module then carries + and what makes its precompile cache stale. + ## [0.3.0] - 2026-09-08 diff --git a/docs/src/crate_bindings.md b/docs/src/crate_bindings.md index bf2b8f5f..713b71b3 100644 --- a/docs/src/crate_bindings.md +++ b/docs/src/crate_bindings.md @@ -503,7 +503,92 @@ MyMath.fibonacci(UInt32(20)) # => 6765 ## Precompilation Support -For package development, you can generate bindings to a file that will be precompiled with your package, improving startup time. +A package can carry a crate's bindings in two ways, and both precompile: use +`@rust_crate` at the package's top level, or write the bindings to a file with +`write_bindings_to_file` and `include` it. + +### Using `@rust_crate` inside a package + +```julia +module MyPackage +using RustCall + +# Build the crate and generate its bindings; define them as `MyPackage.Bindings`. +@rust_crate joinpath(@__DIR__, "..", "deps", "my_rust_crate") submodule="Bindings" +using .Bindings: add, multiply, MyStruct + +export add, multiply, MyStruct +end +``` + +The generated module is evaluated **inside the module that expands the macro** +(#339), so it is part of `MyPackage` and is compiled into `MyPackage`'s +precompile cache like any other submodule. What happens, and when: + +- **When the package is precompiled** (the first `using`, or + `Pkg.precompile()`): `@rust_crate` scans the crate, builds it — the library + goes into RustCall's cache under the depot's scratch space — and generates the + module. Nothing is written into the package. The library is *not* opened at + this point: Julia defers the generated module's `__init__` to load time, so + the bindings are callable after `MyPackage.__init__`, not from the package's + own top level. +- **When the package is loaded** from its cache: the generated module's + `__init__` opens the cached library (through a private per-process copy, so + Cargo's output and the cache copy stay free to be rebuilt, #309). No + scanning, no Cargo. +- **After an edit to the crate, or `RustCall.clear_cache()`**: the module + declared the library *and the crate's own input files* — the set the artifact + identity is computed from, so a `path` dependency and a workspace root count + too — with `Base.include_dependency`, so the + package's precompile cache is stale and the next `using` re-precompiles the + package, building the crate again. Deterministic, and never a failed + `dlopen` of a path that is gone. + +One thing Julia's invalidation cannot see: **the environment**. `RUSTFLAGS`, +`PYO3_PYTHON`, a `PYO3_CONFIG_FILE` pointing at a *different* file — all of them +decide the artifact, and none of them is a file the image can track. Change one +and every tracked file is still what it was, so the image stays valid and the +package loads the library built under the previous values. The generated module +records the values it was built under and `@warn`s at load time when they no +longer match, naming the variables; the fix is to precompile the package again +(`Pkg.precompile(; force = true)`, or touch a source file of the crate). + +`cache=false` is not the shape to use in a package. The library is then not +the cache copy but whatever the build produced: Cargo's own output under the +crate's `target/` for a crate that is already a `cdylib`, and a copy in a +directory of its own under RustCall's Cargo cache — one the cache lookup never +returns, and that only `RustCall.clear_cache()` removes — for a crate RustCall +has to wrap. The copy outlives the process that made it on purpose: a package +precompiled with `cache=false` is loaded by another process, which must still +find the file. In the first case the next `cargo build` of the crate +invalidates the package's cache; in the second every precompilation leaves a +copy behind until the cache is cleared. + +The naming rule: **`submodule="Bindings"` is what defines** the module as +`MyPackage.Bindings`, which is what `using .Bindings: ...` needs. Without it +the module gets a hidden, per-call name (`MyPackage.var"##RustCallCrateRuntime#N"...`) +and is reached only through the value the macro returns — nothing the caller +did not name appears in its namespace. `name="Bindings"` chooses that hidden +module's name and still defines nothing, which is why +`const MyBindings = @rust_crate path name="MyBindings"` — the form the macro's +docstring has always shown — keeps working: the constant is the only binding +the caller gets. + +A second `@rust_crate ... submodule="Bindings"` in the same module replaces the +module (Julia warns `replacing module Bindings`); values obtained earlier keep +the module they hold. Do not write `const Bindings = @rust_crate path submodule="Bindings"`: +that binds the returned value over the module the macro just defined. The +return value is the same `CrateBindings` in every position — REPL, function +body, package — so a package may also keep it: `const B = @rust_crate path` +gives `B.add(...)`, world-age-safe, without any visible module. The generated +module needs only `RustCall` among the package's dependencies (it reaches +`Libdl` through RustCall). + +Prefer this shape when the machine that loads the package has a Rust toolchain +and building the crate on first use is acceptable; the bindings can never be +out of date with respect to the crate. Prefer the written file below when the +package must load without Rust installed, or when the bindings and the library +must be inspected, committed or shipped as files. ### Generating Bindings to a File @@ -650,6 +735,13 @@ If you encounter precompilation issues: - Ensure the library path is correct (use `relative_lib_path` for portable packages) - Check that the library was copied to the correct location - Verify the generated code compiles without errors +- With `@rust_crate` at a package's top level: the package must depend on + `RustCall`, the crate must build on the machine that precompiles the + package, and calls into the bindings belong after the package's `__init__` + (the library is not open while the package's top level runs during + precompilation). `Base.isprecompiled(Base.identify_package("MyPackage"))` + says whether the cache is currently valid; it turns `false` after + `RustCall.clear_cache()` until the next `using` rebuilds. ## Object lifetime diff --git a/docs/src/precompilation.md b/docs/src/precompilation.md index feac65ed..45f63a54 100644 --- a/docs/src/precompilation.md +++ b/docs/src/precompilation.md @@ -17,16 +17,24 @@ When you use `@rust_crate` in a Julia package, the bindings can be precompiled a ### Runtime vs Precompile Time -**Without precompilation** (using `@rust_crate` directly): -1. Julia loads your package -2. `@rust_crate` scans the Rust crate -3. Rust code is compiled (if not cached) -4. Bindings are generated and evaluated - -**With precompilation** (using `write_bindings_to_file`): +Two ways to put a crate's bindings in a package, both precompiled: + +**`@rust_crate` at the package's top level** (`@rust_crate path submodule="Bindings"` +followed by `using .Bindings: ...`; see "Using `@rust_crate` inside a package" +in [Crate Bindings](crate_bindings.md)): +1. During precompilation: `@rust_crate` scans the crate, builds it (into + RustCall's cache) and generates the module, which is compiled into the + package's cache; nothing is written into the package +2. At runtime: the module's `__init__` opens the cached library +3. After `RustCall.clear_cache()` or a rebuild of the crate: the package's + precompile cache is stale and the next `using` re-precompiles it + +**`write_bindings_to_file`** (a `deps/build.jl` writes the bindings and copies +the library into the package): 1. During development: Generate bindings file once 2. During precompilation: Julia compiles the bindings module -3. At runtime: Precompiled bindings load instantly +3. At runtime: Precompiled bindings load instantly, from the library the + package carries — no Rust toolchain needed on the machine that loads it ### The Generation Process diff --git a/examples/README.md b/examples/README.md index d0b20cf2..26a287be 100644 --- a/examples/README.md +++ b/examples/README.md @@ -305,13 +305,15 @@ clear_cache() ### Module name confusion -When using `@rust_crate`, the returned bindings object wraps a generated runtime module whose default name is the crate name converted to PascalCase. +When using `@rust_crate`, the returned bindings object wraps a generated module whose default name is the crate name converted to PascalCase. Example: `sample_crate` → `SampleCrate` -You can override that internal runtime module name with `name=`, while still using the value returned by `@rust_crate`: +By default that module is hidden inside the calling module and is reached only through the returned value; `name=` chooses its name, and `submodule=` is what defines it in the calling module so a package can `using` from it (#339): ```julia -const bindings = @rust_crate "/path/to/crate" name="MyCustomName" +const bindings = @rust_crate "/path/to/crate" name="MyCustomName" # nothing new is defined here +@rust_crate "/path/to/crate" submodule="MyCustomName" # defines MyCustomName here +using .MyCustomName: add ``` ## Additional Resources diff --git a/src/artifact_id.jl b/src/artifact_id.jl index a2e7d1ab..2bd97ecb 100644 --- a/src/artifact_id.jl +++ b/src/artifact_id.jl @@ -904,6 +904,38 @@ function crate_input_files(dir::AbstractString) return "walk", files end +""" + crate_input_dirs(dir::AbstractString) -> Vector{String} + +Every directory `crate_input_files` walks, `dir` itself included, as +paths relative to `dir` (`"."` for the root). Same walk, same exclusions — +Cargo's `target/` at the package root, VCS metadata at any depth — so the two +cannot drift. + +Separate from the file list because a *directory* is what tells a consumer that +a file appeared. `crate_content_digest` hashes the list of files, so creating +the first file in a directory that was empty changes the artifact; the parent's +own entry list does not move (the directory was already there), and neither +does any file. A caller that tracks directories therefore has to know about the +empty ones too (#339 review). +""" +function crate_input_dirs(dir::AbstractString) + dir = String(dir) + dirs = String[] + for (root, subdirs, _) in walkdir(dir) + at_root = _canonical_dir(root) == _canonical_dir(dir) + filter!(subdirs) do d + d in CRATE_INPUT_VCS_DIRS_ANY_LEVEL && return false + at_root && d == "target" && return false + return true + end + push!(dirs, replace(relpath(root, dir), '\\' => '/')) + end + unique!(dirs) + sort!(dirs) + return dirs +end + """ local_path_dependency_dirs(root::AbstractString) -> (strategy::String, dirs::Vector{String}) @@ -972,6 +1004,19 @@ function _local_path_dependency_dirs_uncached(root::String) end if !isempty(found) append!(dirs, found) + # `cargo tree` resolves the *default* build's graph, and this + # function is called with no feature set: an optional `path` + # dependency that only `features = [...]` activates is not in + # `found`. Every local crate any manifest in the graph declares is + # added, transitively, optional or not — a crate the build *can* + # pull in is an input of the artifact, and an edit to it must + # change the key whether the current feature set pulls it in or + # not. Over-approximating costs a rebuild; under-approximating + # handed a stale library back under an unchanged key (#339 review). + seen = Set{String}() + for dir in copy(dirs) + _collect_manifest_path_deps!(dirs, dir, seen) + end unique!(dirs) return "cargo-tree", dirs end @@ -1088,9 +1133,40 @@ function _declared_path_dependencies(manifest::AbstractString)::Vector{String} end end end + # `[patch.] name = { path = "../local" }` replaces a registry or + # git dependency with a local crate — the dependency table itself still + # says `version = "..."`, so harvesting it alone never sees the directory. + # When the patched dependency is optional and a feature activates it, the + # default `cargo tree` graph omits it too, and an edit to the local crate + # changed neither the key nor the declared inputs (#339 review). Cargo + # honours `[patch]` in the workspace root's manifest (or a crate's own + # when it is its own root), and the paths are relative to the manifest + # that declares them. + _harvest_patch_paths!(out, parsed, dir) + root = _workspace_root_dir(dir) + if root !== nothing && _canonical_dir(root) != _canonical_dir(dir) + root_manifest = _parse_manifest_or_nothing(joinpath(root, "Cargo.toml")) + root_manifest isa AbstractDict && _harvest_patch_paths!(out, root_manifest, root) + end return sort!(unique!(out)) end +# Every `path` a `[patch.]` table of `parsed` names, made absolute +# against `dir`, the directory of the manifest that declares it. +function _harvest_patch_paths!(out::Vector{String}, parsed::AbstractDict, dir::AbstractString) + patches = get(parsed, "patch", nothing) + patches isa AbstractDict || return nothing + for (_, per_source) in patches + per_source isa AbstractDict || continue + for (_, spec) in per_source + spec isa AbstractDict || continue + p = get(spec, "path", nothing) + p isa AbstractString && push!(out, abspath(joinpath(String(dir), String(p)))) + end + end + return nothing +end + # `[workspace.dependencies]` of this manifest, of the workspace its # `[package] workspace = "..."` names, else of the nearest ancestor manifest # that declares a workspace. Returns the table together with the directory of diff --git a/src/crate_bindings.jl b/src/crate_bindings.jl index 03cf48e3..25701b28 100644 --- a/src/crate_bindings.jl +++ b/src/crate_bindings.jl @@ -404,6 +404,484 @@ end # Julia Module Generation # ============================================================================ +""" + _crate_precompile_dependencies(crate_path) -> Vector{String} + +Every path on disk that the crate's artifact identity is computed from — files +**and the directories that hold them** — as absolute paths. + +A module generated in memory by `@rust_crate` may be compiled into a package's +precompile image (#339), and Julia decides that image is stale from what the +module declared with `Base.include_dependency`. Declaring only the built +library is not enough: the library is content-addressed, so editing +`src/lib.rs` produces a *different* cache path and leaves the old file +untouched — the image would still be valid and the package would go on calling +the previous build (#339 review). Declaring the inputs instead makes an edit to +the crate invalidate the image, which is what sends the next `using` back +through `@rust_crate`. + +The list is deliberately the same set `compute_crate_hash` reads: the crate +directory's own input files, every local `path` dependency's, the effective +Cargo configuration, the contents of `PYO3_CONFIG_FILE` when one is set (both +the wrapper path and a plain build of a crate that depends on pyo3 read it), +the workspace root's manifest and lockfile when the crate is a +workspace member, and a library root that lives outside the package directory +(`[lib] path = "../shared/lib.rs"`). + +**Directories are in the list because files alone cannot see an addition.** +`include_dependency` tracks a directory by `join(readdir(path))`, so declaring +each directory that holds an input catches a *new* file appearing beside the +ones that were there — a source file a build script globs, or a +`.cargo/config.toml` created where none existed — which changes +`crate_content_digest` and therefore the artifact, while touching no file the +image already knew (#339 review). Two gaps remain, both deliberate: a +`.cargo/` created in an *ancestor* of the crate is not seen, and neither is one +appearing in `CARGO_HOME`, because tracking those directories would mean +tracking directories whose contents churn for unrelated reasons and +re-precompiling the package for each. + +A path that is not on disk is dropped — `include_dependency` raises on an +unreadable path, and a missing input already changes the digest through +`crate_content_digest`. +""" +function _crate_precompile_dependencies(crate_path::AbstractString) + root = abspath(String(crate_path)) + isdir(root) || return String[] + deps = String[] + dirs = String[root] + try + _, found = local_path_dependency_dirs(root) + append!(dirs, found) + catch e + # Resolving the graph needs Cargo; without it the crate's own files are + # still worth declaring. + @debug "Could not resolve path dependencies for precompile tracking" crate_path exception = e + end + # `local_path_dependency_dirs` already unions every local crate any + # manifest in the graph declares — optional ones included, transitively — + # so an optional dependency that only `features = [...]` activates is in + # this list *and* in the artifact key it feeds. The two must agree: a + # tracked file that changes the image but not the key would rebuild the + # bindings around the same stale library (#339 review). + for dir in unique(abspath.(dirs)) + isdir(dir) || continue + try + # `crate_input_files` / `crate_input_dirs` report `/`-separated + # relative names on every platform; `normpath` makes the joined + # path a native one, so the list has one spelling per file and a + # caller comparing paths on Windows sees `\` throughout. + _, files = crate_input_files(dir) + for rel in files + f = normpath(joinpath(dir, rel)) + isfile(f) && push!(deps, f) + end + # The directories of that same walk, including the ones holding no + # file: creating the first file in an empty `assets/` changes + # `crate_content_digest`, moves no file, and does not change its + # parent's entry list either, because the directory was already + # there (#339 review). + for rel in crate_input_dirs(dir) + d = rel == "." ? dir : normpath(joinpath(dir, rel)) + isdir(d) && push!(deps, d) + end + catch e + @debug "Could not list crate input files for precompile tracking" dir exception = e + end + end + # Cargo's own configuration decides the flags a build runs under, and is + # in the artifact key through `_cargo_config_digest` — an edit to + # `.cargo/config.toml` changes the binary without touching a file of the + # crate, so it belongs here too (#339 review). + try + append!(deps, _cargo_config_files(ENV; dir = root)) + catch e + @debug "Could not list Cargo configuration files for precompile tracking" root exception = e + end + # A workspace member is decided by files outside its directory, and a + # library root may live outside it too — both are in the artifact key. + try + workspace = _cargo_root_dir(root) + if abspath(workspace) != root + for name in ("Cargo.toml", "Cargo.lock") + f = joinpath(workspace, name) + isfile(f) && push!(deps, f) + end + end + manifest_path = joinpath(root, "Cargo.toml") + if isfile(manifest_path) + lib_root = crate_lib_root(root, parse_cargo_toml(manifest_path)) + if lib_root !== nothing + lib_dir = dirname(abspath(lib_root)) + if !startswith(lib_dir * "/", root * "/") && isdir(lib_dir) + _, files = crate_input_files(lib_dir) + for rel in files + f = normpath(joinpath(lib_dir, rel)) + isfile(f) && push!(deps, f) + end + # And this tree's directories, for the same reason as the + # crate's own: `external_lib_tree_digest` hashes the file + # list, so a first file appearing in a directory that was + # already there moves nothing else (#339 review). + for rel in crate_input_dirs(lib_dir) + d = rel == "." ? lib_dir : normpath(joinpath(lib_dir, rel)) + isdir(d) && push!(deps, d) + end + end + end + end + catch e + @debug "Could not resolve out-of-directory crate inputs" crate_path exception = e + end + # The directories that hold those files, so a file *appearing* is seen too: + # `include_dependency` tracks a directory by its entry list. `CARGO_HOME` + # is left out on purpose — its top level holds the registry and git caches, + # and tracking it would re-precompile the package for reasons that have + # nothing to do with this crate. + # Only the holders of *files*: a directory in the list is an input in its + # own right and its parent is not — for the crate root that parent is the + # checkout, whose unrelated siblings must not invalidate the image (#339 + # review). + cargo_home = abspath(get(ENV, "CARGO_HOME", joinpath(homedir(), ".cargo"))) + for dir in unique(dirname.(filter(isfile, deps))) + isdir(dir) || continue + abspath(dir) == cargo_home && continue + push!(deps, dir) + end + # `PYO3_CONFIG_FILE` names a file whose *contents* decide the wrapper's + # Python version, ABI and library directory, and both build paths hash + # those contents into the artifact. It usually lives outside the crate + # tree, so nothing above would have caught an edit to it (#339 review). + # Added *after* the holder loop on purpose: the selected file is the input, + # not its directory — a sibling appearing next to it changes nothing the + # build reads, and must not invalidate the image (#339 review). + # A selected file that does not exist *yet* is tracked through its + # directory instead: a build script that tolerates the absence is built + # without it, and the file appearing is then the one event that changes + # the build — the entry list of the directory is what sees it. Once the + # file exists, it is the input and the directory is not (#339 review). + let config = get(ENV, "PYO3_CONFIG_FILE", "") + if !isempty(config) + if isfile(config) + push!(deps, abspath(config)) + elseif isdir(dirname(abspath(config))) + push!(deps, dirname(abspath(config))) + end + end + end + return unique!(map(normpath, deps)) +end + +""" + _recorded_build_env() -> Vector{Pair{String, String}} + +The environment a generated module records and compares at load time: the +`artifact_build_env` allowlist, plus RustCall's own selectors that decide the +artifact without being in that allowlist — `RUSTCALL_PYTHON_LIBDIR`, which +`python_link_source()` gives precedence and `pyo3_link_rustflags()` folds into +a wrapper's identity and rpath (#339 review). One function for both sides, so +what is recorded and what is compared cannot drift. +""" +function _recorded_build_env(; python::Bool = false) + env = Pair{String, String}[String(k) => String(v) for (k, v) in artifact_build_env()] + # The *contents* of `PYO3_CONFIG_FILE`, not only its path: the file is + # tracked when it exists, but one selected before it exists cannot be — + # its directory is — and a load after it appeared must still be told. + # "" when unset, `_file_content_digest`'s marker when absent (#339 review). + push!(env, "" => _pyo3_config_file_digest()) + if !python + # A plain build's pyo3 — a `cdylib` with `#[julia]` items that also + # depends on pyo3 — runs pyo3's build script, which selects an + # interpreter (`PYO3_PYTHON`, else `python3` on `PATH`) and configures + # the library for *that* Python's ABI. The `PYO3_*` values above see a + # changed `PYO3_PYTHON`, not a `PATH` that now finds another Python or + # a shim retargeted under the same name; what the interpreter *is* does + # (#339 review). Recorded the way `_plain_crate_build_env` keys it. + interpreter, fingerprint = _pyo3_build_interpreter() + push!(env, "" => interpreter) + push!(env, "" => fingerprint) + end + # Only for a module that binds a PyO3 wrapper: a plain crate's build never + # consults `python_link_source()`, so for it this selector is not an input + # and comparing it would warn about a library nothing changed (#339 + # review). + if python + for name in ("RUSTCALL_PYTHON_LIBDIR",) + value = get(ENV, name, nothing) + value === nothing || push!(env, name => String(value)) + end + selection = _python_selection() + push!(env, "" => selection) + # The plan itself, computed once: `(libdir, interpreter, fingerprint)` + # exactly as `python_link_source()` decides it for a build. + source = _python_link_source_or_empty() + # When pyo3's own configuration (`PYO3_CROSS_LIB_DIR`, the `lib_dir` + # of a `PYO3_CONFIG_FILE`) decides, pyo3 consults no interpreter: the + # plan's fingerprint is "" and `_pyo3_wrapper_build_env` keys nothing + # by what `PYO3_PYTHON` resolves to. Recording it anyway warned about + # a `PYTHONHOME` or shim change that selects the same artifact (#339 + # review). So the two interpreter records below are empty on that + # branch, the way the plan's are. + configured = !isempty(_pyo3_configured_lib_dir()) + # What that selection *is*: `PYO3_PYTHON` may be a bare `python3` or a + # pyenv/asdf shim whose target moves under the same name, and + # `python_link_source()` runs the command and hashes what it reports. + # The resolved `sys.executable` is recorded beside the raw selection + # (one short subprocess, only for a PyO3 wrapper module; #339 review). + push!(env, "" => (configured ? "" : _python_resolved(selection))) + # And what it *reports*: the same executable can describe a different + # Python after `PYTHONHOME` or its sysconfig metadata changes, and + # `_pyo3_wrapper_build_env` hashes exactly that description + # (`plan.interpreter_config`). Recorded as the plan records it — the + # plan's own value, "" on the configured branch (#339 review). + push!(env, "" => source[3]) + # The link directory is not the interpreter's alone: for the implicit + # case `python_link_source()` asks a bare `python3-config --ldflags`, + # falling back to `python-config`, and `PATH` may resolve either to + # another installation than the interpreter's. Both commands' + # identities are recorded, and their content tracked as files — but + # only on that implicit branch: with `PYO3_PYTHON`, a configured + # library directory, `RUSTCALL_PYTHON_LIBDIR` or CondaPkg deciding, + # neither command is consulted and neither is an input (#339 review). + # Nor on macOS when the implicit interpreter is a framework build: + # `python_link_source()` takes the framework prefix and never asks + # either command (`_python_config_consulted`). + if _python_config_consulted() + for (name, path) in _python_config_selections() + push!(env, "<$name selection>" => path) + end + end + # And the directory all of that *resolves to*: `pyo3_link_rustflags` + # builds the wrapper's `-L` and rpath from `python_link_source()[1]`, + # and `_pyo3_wrapper_build_env` keys the artifact by those flags. The + # selections above name the commands; an unchanged `python3-config` + # that is a shim can still answer with another directory once the + # environment or metadata it reads moves, and only the answer itself + # says so. Recorded the way the flags are computed (#339 review). + push!(env, "" => source[1]) + end + return env +end + +""" + _pyo3_build_interpreter() -> (interpreter::String, fingerprint::String) + +The interpreter pyo3's **build script** selects when a crate that depends on +pyo3 is built as it stands (the plain path, no wrapper), and what that +interpreter reports about itself (`_python_interpreter_fingerprint`): `("", "")` +when pyo3's own configuration (`PYO3_CROSS_LIB_DIR`, the `lib_dir` of a +`PYO3_CONFIG_FILE`) decides and no interpreter is consulted; else `PYO3_PYTHON` +as given; else the `sys.executable` of the first `python3` / `python` on +`PATH`. The same order pyo3 uses — RustCall's own selectors +(`RUSTCALL_PYTHON_LIBDIR`, CondaPkg) play no part in a build RustCall does not +wrap. Part of a plain build's key and of its module's load-time record, so a +`PATH` that finds another Python, or a shim retargeted under one name, is a +different artifact and a reported change rather than a library configured for +the previous ABI (#339 review). One short subprocess; "" for both when no +interpreter can be run. +""" +function _pyo3_build_interpreter() + isempty(_pyo3_configured_lib_dir()) || return ("", "") + pinned = get(ENV, "PYO3_PYTHON", "") + interpreter = isempty(pinned) ? _python_executable_on_path() : String(pinned) + isempty(interpreter) && return ("", "") + fingerprint = try + String(_python_interpreter_fingerprint(interpreter)) + catch + "" + end + return (interpreter, fingerprint) +end + +""" + _python_link_is_implicit() -> Bool + +Whether `python_link_source()` would reach its last step — the interpreter and +`python3-config` / `python-config` found on `PATH` — rather than be decided by +pyo3's own configuration, `PYO3_PYTHON`, `RUSTCALL_PYTHON_LIBDIR` or CondaPkg. +Only then are the config commands inputs of the wrapper (#339 review). +""" +function _python_link_is_implicit() + isempty(_pyo3_configured_lib_dir()) || return false + isempty(get(ENV, "PYO3_PYTHON", "")) || return false + isempty(get(ENV, "RUSTCALL_PYTHON_LIBDIR", "")) || return false + return _condapkg_link_source() === nothing +end + +""" + _python_config_consulted() -> Bool + +Whether `python_link_source()` actually asks `python3-config` / `python-config` +for the link directory: the implicit case (`_python_link_is_implicit`), minus +the one step it takes before either command — on macOS a framework build of +the interpreter answers with its framework prefix, and neither command is run. +For such a Python the commands are not inputs: recording them warned about a +changed `python3-config` on `PATH`, and tracking its file rebuilt the package, +while the wrapper's link directory and identity had not moved (#339 review). +Mirrors the `python3` / `python` loop of `python_link_source()` step for step, +and is `false` when no interpreter is found at all — then nothing is consulted. +""" +function _python_config_consulted() + _python_link_is_implicit() || return false + for exe in ("python3", "python") + isempty(_python_executable(exe)) && continue + return !(Sys.isapple() && !isempty(_python_framework_prefix(exe))) + end + return false +end + +""" + _python_config_selections() -> Vector{Pair{String, String}} + +The `python3-config` and `python-config` that `python_link_source()` would run +for the implicit link directory — the first of each on `PATH`, "" when there is +none — in the order `_python_config_libdir()` tries them. Both are recorded and +compared for a PyO3 wrapper module, and both files tracked, because `PATH` +resolving either to another installation changes the rpath the wrapper is +linked with while the interpreter, and everything else recorded, stays the +same. Recording the fallback even when the first command answers is +deliberate: which one *answers* is only known by running them, and a load +must not (#339 review). +""" +function _python_config_selections() + # A `Vector`, not a tuple: the wrapper path splices `last.(...)` of this + # into a `String[...]`, and a tuple there is one element that cannot be + # converted, not two strings. + map(["python3-config", "python-config"]) do name + found = Sys.which(name) + name => (found === nothing ? "" : String(found)) + end +end + +""" + _python_resolved(command) -> String + +The `sys.executable` that `command` reports, or `command` itself when it cannot +be run; "" for "". A bare `python3` or a shim is one path on `PATH` and another +underneath, and only the interpreter can say which (#339 review). +""" +function _python_resolved(command::AbstractString) + isempty(command) && return "" + resolved = _python_executable(command) + return isempty(resolved) ? String(command) : resolved +end + +""" + _python_selection() -> String + +Which interpreter `python_link_source()` would pin, decided the way it decides +it: `PYO3_PYTHON` when set, else CondaPkg's when that package is loaded, else +the `sys.executable` the first `python3` / `python` on `PATH` reports +(`_python_executable_on_path`). "" when there is none. + +Recorded for a PyO3 wrapper module so `__init__` can tell that the *selection* +moved — `PYO3_PYTHON` unset and `PATH` now finding a different interpreter — +which tracking the selected interpreter's files cannot see, because the old +one is still there, unchanged (#339 review). The implicit case asks the +interpreter rather than trusting `Sys.which`: a pyenv or asdf shim keeps one +path on `PATH` while its project selection moves the real interpreter, and +only `sys.executable` says which one that is. One short subprocess per load of +a PyO3 wrapper module; a plain module never runs it. +""" +function _python_selection() + # The same order as `python_link_source()`, step for step — a selector that + # disagrees with it records the wrong interpreter and then never notices + # the real one moving (#339 review). The contract test asserts the two + # agree in the running environment. + # + # 1. pyo3's own configuration (`PYO3_CROSS_LIB_DIR`, `PYO3_CONFIG_FILE`): + # the interpreter is `PYO3_PYTHON` if set, else none. + isempty(_pyo3_configured_lib_dir()) || return String(get(ENV, "PYO3_PYTHON", "")) + # 2. an explicit `PYO3_PYTHON`. + pinned = get(ENV, "PYO3_PYTHON", "") + isempty(pinned) || return String(pinned) + # 3. `RUSTCALL_PYTHON_LIBDIR` alone leaves the interpreter to `PATH`, and + # that comes *before* CondaPkg. + isempty(get(ENV, "RUSTCALL_PYTHON_LIBDIR", "")) || return _python_executable_on_path() + # 4. CondaPkg's environment, when the package is loaded and has one. + conda = _condapkg_link_source() + conda === nothing || return String(conda[2]) + # 5. the first `python3` / `python` on `PATH`, as it reports itself. + return _python_executable_on_path() +end + +""" + _warn_if_build_env_changed(recorded, crate_path, lib_name) + +Warn when the environment that decides this crate's artifact is not the one it +was built under. + +Julia invalidates a precompile image from *files*, and +`Base.include_dependency` is the only lever a generated module has. Part of the +artifact identity is not a file: `RUSTFLAGS`, `PYO3_PYTHON`, a +`PYO3_CONFIG_FILE` **pointing somewhere else**, and the rest of the allowlist +`artifact_build_env` captures. Change one of those and every file the image +tracks is still byte-for-byte what it was, so Julia keeps the image and the +module loads a library built for the other environment — silently, and with a +Python preload plan to match (#339 review). + +Nothing here can invalidate the image; what it can do is refuse to be silent. +The module records the values it was generated under and compares them at load +time, which is cheap — the allowlist is read from `ENV`, no probe, no build. +The fix it names is the one that works: force the package to be precompiled +again. +""" +function _warn_if_build_env_changed(recorded, crate_path::AbstractString, lib_name::AbstractString, + recorded_cargo_config::AbstractString = "", + recorded_toolchain::AbstractString = ""; + python::Bool = false) + current = try + _recorded_build_env(; python = python) + catch e + @debug "Could not read the build environment" exception = e + return nothing + end + was = Dict{String, String}(String(k) => String(v) for (k, v) in recorded) + now = Dict{String, String}(String(k) => String(v) for (k, v) in current) + changed = sort!(collect(union(keys(was), keys(now)))) + filter!(k -> get(was, k, nothing) != get(now, k, nothing), changed) + # The *effective* Cargo configuration is selected by `CARGO_HOME`, which + # the allowlist deliberately does not capture — the file's contents go into + # the artifact identity instead of its path. So pointing `CARGO_HOME` + # somewhere else changes the flags a build runs under while every variable + # above, and every file `_CRATE_INPUTS` names, stays exactly as it was + # (#339 review). Comparing the digest catches that, and any other way the + # effective configuration differs. + if !isempty(recorded_cargo_config) + now_config = try + _cargo_config_digest(ENV; dir = crate_path) + catch e + @debug "Could not read the Cargo configuration" exception = e + recorded_cargo_config + end + now_config == recorded_cargo_config || push!(changed, "") + end + # The toolchain is in the artifact identity too (`toolchain_fingerprint`: + # compiler identity, extractor, core sources) and is not a file the image + # tracks — `rustup update stable` replaces the binaries behind a proxy + # whose path and content do not move (#339 review). Memoized per session, + # so this is one `rustc -vV` per process at most. + if !isempty(recorded_toolchain) + now_toolchain = try + toolchain_fingerprint() + catch e + @debug "Could not fingerprint the toolchain" exception = e + recorded_toolchain + end + now_toolchain == recorded_toolchain || push!(changed, "") + end + isempty(changed) && return nothing + @warn """ + RustCall: the build environment changed since `$(lib_name)` was compiled into this \ + package's precompile image, and Julia cannot see that — it invalidates an image from \ + files, and these are not files. The library that is about to load was built under the \ + previous values. + + Force a rebuild with `Pkg.precompile(; force = true)`, or touch a source file of the \ + crate. + """ crate = crate_path variables = changed + return nothing +end + """ emit_crate_module(info::CrateInfo, lib_path::String; module_name::Union{String, Nothing}=nothing) -> Expr @@ -423,7 +901,9 @@ function emit_crate_module(info::CrateInfo, lib_path::String; module_name::Union{String, Nothing}=nothing, build_release::Bool = true, lib_name::Union{String, Nothing} = nothing, - preload::Vector{String} = String[]) + preload::Vector{String} = String[], + extra_inputs::Vector{String} = String[], + python::Bool = false) # Determine module name mod_name = if module_name !== nothing Symbol(module_name) @@ -446,14 +926,61 @@ function emit_crate_module(info::CrateInfo, lib_path::String; # the registry hold the same handle and the same liveness flag. lib_key = lib_name === nothing ? crate_library_name(info; release = build_release) : lib_name + # The files an edit to the crate would touch; see + # `_crate_precompile_dependencies`. + crate_inputs = _crate_precompile_dependencies(info.path) + # Inputs the caller knows about and the crate directory does not — the PyO3 + # wrapper's interpreter and the libraries it preloads. An interpreter + # upgraded in place keeps its path, so only its *content* says it changed, + # and `plan.interpreter_config` is in the wrapper's artifact identity + # (#339 review). + for extra in extra_inputs + (isfile(extra) || isdir(extra)) && push!(crate_inputs, abspath(extra)) + end + unique!(crate_inputs) + # The part of the artifact identity that is *not* a file, recorded so the + # module can say so at load time (`_warn_if_build_env_changed`). + build_env = try + _recorded_build_env(; python = python) + catch e + @debug "Could not record the build environment" exception = e + Pair{String, String}[] + end + recorded_env = Any[String(k) => String(v) for (k, v) in build_env] + toolchain = try + toolchain_fingerprint() + catch e + @debug "Could not record the toolchain fingerprint" exception = e + "" + end + crate_dir = abspath(String(info.path)) + # The effective Cargo configuration is chosen by `CARGO_HOME`, which is not + # an allowlisted variable: its digest is what says whether the same build + # would run under the same flags (#339 review). + cargo_config_digest = try + _cargo_config_digest(ENV; dir = crate_dir) + catch e + @debug "Could not record the Cargo configuration" exception = e + "" + end + # Build the module body as a block module_body = quote import RustCall import RustCall: call_rust_function, get_function_pointer_from_lib, RustResult, RustOption, _check_not_freed, _call_rust_owned_string_ptr, _call_rust_borrowed_string_ptr, convert_return, _result_payload, FFIByValue - import Libdl - + # Through RustCall, not `import Libdl`: this module is evaluated inside + # the caller, and `import Libdl` would be resolved in the *caller's* + # environment — a package that uses `@rust_crate` would then need + # `Libdl` among its own dependencies to precompile (#339). + import RustCall.Libdl + + # The *durable* library — RustCall's cache copy, or Cargo's output — + # never the per-process generation copy, which is swept once the + # process that made it is gone. This module may be precompiled as part + # of a package (`@rust_crate` at top level, #339): its `__init__` then + # runs in a later session, which must still find this file. const _LIB_PATH = $lib_path const _LIB_NAME = $lib_key # Libraries the image imports by name that the loader would not find on @@ -461,6 +988,34 @@ function emit_crate_module(info::CrateInfo, lib_path::String; # no rpath — opened before it (`PyO3LinkPlan.runtime_libraries`). const _PRELOAD_LIBRARIES = $preload + # What makes a package that contains this module re-precompile, and so + # rebuild the crate, when it should (#339). Outside precompilation + # these record nothing. + # + # The library: removed by `RustCall.clear_cache()`, after which Julia + # sees the image as stale rather than letting `__init__` open a path + # that is gone. + Base.include_dependency(_LIB_PATH) + # And the crate's own inputs — the very files its artifact identity is + # computed from. Without them an edit to `src/lib.rs` would leave the + # image valid: the new build lands at a *different* content-addressed + # cache path and the old file is still there, unchanged, so nothing + # Julia tracks would have moved and the package would go on calling the + # previous build (#339 review). + const _CRATE_INPUTS = $crate_inputs + for _input in _CRATE_INPUTS + Base.include_dependency(_input) + end + # The rest of the identity is environment, not files — + # `RUSTFLAGS`, `PYO3_PYTHON`, a `PYO3_CONFIG_FILE` pointing elsewhere. + # Julia cannot invalidate an image on those, so the values are recorded + # and `__init__` says when they no longer match (#339 review). + const _BUILD_ENV = $recorded_env + const _CRATE_DIR = $crate_dir + const _CARGO_CONFIG = $cargo_config_digest + const _TOOLCHAIN = $toolchain + const _RECORDS_PYTHON = $python + # Everything this module knows about the image it calls — handle, # liveness flag and generation number — as **one immutable value**, in # one `Ref`. @@ -483,8 +1038,17 @@ function emit_crate_module(info::CrateInfo, lib_path::String; # assignment after it would overwrite a newer generation that a # concurrent reload had already published, and calls through this # module would go back to entering the retired image (#277). + RustCall._warn_if_build_env_changed(_BUILD_ENV, _CRATE_DIR, _LIB_NAME, _CARGO_CONFIG, + _TOOLCHAIN; python = _RECORDS_PYTHON) RustCall.register_handle_mirror!(_LIB_NAME, _LIB_GEN) - RustCall.load_artifact!(RustCall.crate_direct_policy(), _LIB_PATH; + # A private generation copy, never `_LIB_PATH` itself: that file is + # Cargo's output or the cache copy, and an image mapped in place + # cannot be overwritten on Windows — the next `cargo build` of the + # crate would fail (#255, #277, #309). Copied *here*, not when the + # module was generated, because `__init__` may run in a later + # session than the one that generated this module (#339). + RustCall.load_artifact!(RustCall.crate_direct_policy(), + RustCall.loadable_library_copy(_LIB_PATH); lib_name = _LIB_NAME, preload = _PRELOAD_LIBRARIES) end @@ -1872,6 +2436,100 @@ end # Main API # ============================================================================ +""" + _uncached_library_home(built) -> String + +A copy of `built` in a directory that outlives the process that made it, for +a build that is not entered into the cache (`cache = false`, or a cache write +that failed). + +Not `mktempdir()`: that cleans up at process exit, and the process that +generates a module is not always the one that loads it. A package precompiled +with `cache = false` records this path as its `_LIB_PATH`; the precompile +worker then exits, the directory goes with it, and the session that triggered +the precompilation loads an image whose library is already gone (#339 +review). The copy lives under the Cargo cache directory instead, under a name +the cache lookup never returns, so `RustCall.clear_cache()` is what removes +it. +""" +function _uncached_library_home(built::AbstractString) + home = mktempdir(get_cargo_cache_dir(); prefix = "uncached_", cleanup = false) + kept = joinpath(home, basename(built)) + cp(built, kept; force = true) + return kept +end + +""" + _cache_built_library(cache_key, built, cache_enabled) -> String + +The path a generated module should name for a library that was just built: +the cache copy when caching is on, and `built` itself when it is off or the +cache could not be written. + +Caching is what makes the path *durable*. `built` is either Cargo's output +under the crate's own `target/` — rewritten by the next build of the crate — +or a file inside a wrapper project that is about to be deleted; the cache copy +is neither, which is what a module compiled into a package's precompile image +needs when its `__init__` runs in a later session (#339). + +Returning `built` unchanged is the caller's signal that nothing was copied, so +a caller whose `built` is about to disappear can keep a copy of its own. +""" +function _cache_built_library(cache_key::String, built::String, cache_enabled::Bool) + cache_enabled || return built + try + save_cargo_cached_library(cache_key, built) + cached = get_cargo_cached_library(cache_key) + cached === nothing || return cached + catch e + @debug "Failed to cache library: $e" + end + return built +end + +""" + _plain_crate_build_env() -> Vector{Pair{String, String}} + +The environment a **plain** `@rust_crate` build (no PyO3 wrapper) is keyed by: +`artifact_build_env()` — the #282 allowlist, `PYO3_*` included by prefix — plus +the *contents* of `PYO3_CONFIG_FILE` when it is set. The allowlist records that +variable's value, which is a path; a crate that depends on pyo3 and takes this +path (a `cdylib` exposing `#[julia]` items, say) reads the file itself at build +time, so an edit to it — another Python version, ABI or library directory — is +a different binary under the same path. The wrapper path already hashes the +contents (`_pyo3_wrapper_build_env`); without this the plain key did not, and +`get_cargo_cached_library` answered the edited configuration with the old +library (#339 review). + +**Neither depends on the dependency graph**, deliberately. Cargo hands every +ambient variable to every build script, and a crate's own `build.rs` may read +`PYO3_PYTHON`, or open the file `PYO3_CONFIG_FILE` names, without depending on +pyo3 — which crates are in the graph proves nothing about what a script reads. +So the value is an input of every build (the #282 contract) and so are the +contents of the file it names, as `.cargo/config.toml`'s are: the price is a +spare rebuild when Python is configured for another package while this +variable is set and its file edited, and the alternative — a gate on pyo3 +being in the graph — was a stale library for the crate that read the file +anyway (#339 review; an earlier round of this PR tried the gate and reverted +it). +""" +function _plain_crate_build_env() + build_env = artifact_build_env() + digest = _pyo3_config_file_digest() + isempty(digest) || push!(build_env, "pyo3-config-file-digest" => digest) + # And the interpreter pyo3's build script would configure the library for + # — under the names the wrapper's identity uses for its own + # (`_pyo3_wrapper_build_env`), since it is the same fact about the same + # Python. Empty, and absent, when pyo3's configuration names the library + # directory and no interpreter is consulted (#339 review). + interpreter, fingerprint = _pyo3_build_interpreter() + if !isempty(interpreter) + push!(build_env, "rustcall-pyo3-python" => interpreter) + push!(build_env, "rustcall-pyo3-python-config" => fingerprint) + end + return build_env +end + """ generate_bindings(crate_path::String; kwargs...) -> Expr @@ -1943,11 +2601,30 @@ function generate_bindings(crate_path::String; else @info "Wrapped $(length(wrapper.info.julia_functions)) functions and " * "$(length(wrapper.info.julia_structs)) types ($(wrapper.plan.mode))" - return emit_crate_module(wrapper.info, loadable_library_copy(wrapper.lib_path); + # `wrapper.lib_path` is the cache copy (or, with caching off, a copy + # of Cargo's output); the module copies it per process in + # `__init__`. + # Python is an input of this module only when the wrapper links + # libpython. A `:python_free` build has pyo3 out of the graph and + # consults no interpreter, so recording one would warn on a + # routine `PATH` change and tracking `python3-config` would + # rebuild for nothing (#339 review). + links_python = wrapper.plan.mode === :link_libpython + python_inputs = if links_python + String[wrapper.plan.interpreter; + _python_resolved(wrapper.plan.interpreter); + wrapper.plan.runtime_libraries; + (_python_config_consulted() ? last.(_python_config_selections()) : String[])] + else + String[] + end + return emit_crate_module(wrapper.info, wrapper.lib_path; module_name = output_module_name, build_release = build_release, lib_name = wrapper.lib_name, - preload = wrapper.plan.runtime_libraries) + preload = wrapper.plan.runtime_libraries, + extra_inputs = python_inputs, + python = links_python) end end info = _plain_scan_info(crate_path, info, features, default_features, build_release) @@ -1956,8 +2633,18 @@ function generate_bindings(crate_path::String; # a build the caller asked for with `features` / `default_features` is # not the default build, and must neither answer its lookup nor be built # as it (#307 review). + # `artifact_build_env()` is in the key here as it already is for a PyO3 + # wrapper build (`_pyo3_wrapper_build_env`): `RUSTFLAGS`, a build script's + # `CC`, and the rest of the #282 allowlist decide what `cargo build` + # produces, so two builds under different values are different binaries and + # must not share an entry. Without it a changed environment found the + # previous library in the cache and handed it back — which also made the + # load-time warning's advice wrong, since re-precompiling the package + # rebuilt the bindings around the same stale artifact (#339 review). + build_env_snapshot = _plain_crate_build_env() cache_key = compute_crate_hash(info; release = build_release, - features = features, default_features = default_features) + features = features, default_features = default_features, + build_env = build_env_snapshot) cached_lib = cache_enabled ? get_cargo_cached_library(cache_key) : nothing lib_path = if cached_lib !== nothing && isfile(cached_lib) @@ -1968,9 +2655,15 @@ function generate_bindings(crate_path::String; if crate_has_cdylib(crate_path) # Build the crate directly @info "Building crate directly (already has cdylib crate-type)..." - lib_path = build_crate_directly(info, build_release; - features = features, - default_features = default_features) + built = build_crate_directly(info, build_release; + features = features, + default_features = default_features) + # Cargo's own output under the crate's `target/`: durable, but the + # next `cargo build` of the crate rewrites it, so with caching on + # the module names the cache copy instead — which is what a module + # precompiled into a package needs when its `__init__` runs in a + # later session (#339). + _cache_built_library(cache_key, built, cache_enabled) else # Create wrapper crate and build @info "Creating wrapper crate..." @@ -1986,40 +2679,52 @@ function generate_bindings(crate_path::String; ) try - lib_path = build_cargo_project(wrapper_project, release=build_release, - policy=crate_wrapper_policy()) + built = build_cargo_project(wrapper_project, release=build_release, + policy=crate_wrapper_policy()) + # The library must leave the wrapper project *here*: the + # `finally` below removes the whole project, the build output + # included, so anything that names a path inside it afterwards + # — the cache write, the module's `_LIB_PATH`, the per-process + # copy `__init__` makes — is naming a file that no longer + # exists. With caching on that is the cache copy; with + # `cache = false` it is a copy in a directory of its own, as + # `_build_pyo3_wrapper_project` already does for the PyO3 + # wrapper. Before this, `@rust_crate cache=false` on a + # crate that needs a wrapper failed to open its own library. + kept = _cache_built_library(cache_key, built, cache_enabled) + if kept == built + kept = _uncached_library_home(built) + end + kept finally cleanup_cargo_project(wrapper_project) end end - - # Cache the result - if cache_enabled - try - save_cargo_cached_library(cache_key, lib_path) - catch e - @debug "Failed to cache library: $e" - end - end - - lib_path end # RustCall never maps the file Cargo writes: a later build of the same # crate rewrites its output in place, which on Windows *fails* against a # mapped DLL (`Access is denied`) and elsewhere silently hands the old - # image back to the next `dlopen`. Opening a private copy leaves Cargo's - # output free (#255, #277). - lib_path = loadable_library_copy(lib_path) + # image back to the next `dlopen`. The module's `__init__` opens a private + # generation copy of `_LIB_PATH` (#255, #277) — in `__init__`, not here, + # because that copy belongs to the process that loads the module, which + # after precompilation is not the one that generated it (#339). # Generate module. The registry name follows the key, feature set # included, so two feature sets of one crate are two entries. @info "Generating Julia module..." + # The **same** snapshot decides the registry name as decides the cache key. + # Passing it to one and not the other gave two builds under different + # environments distinct artifacts under one `_LIB_NAME`: loading the second + # replaced the entry and re-pointed the first module's mirror at it, so its + # wrappers called the other build — a wrong ABI or a missing symbol where + # the environment changed the cfg-selected exports (#339 review). return emit_crate_module(info, lib_path; module_name=output_module_name, build_release=build_release, lib_name=crate_library_name(info; release = build_release, features = features, - default_features = default_features)) + default_features = default_features, + build_env = build_env_snapshot)) end """ @@ -2394,13 +3099,59 @@ end Base.show(io::IO, proxy::CrateBindingObject) = _show_crate_binding_object(io, proxy) Base.show(io::IO, ::MIME"text/plain", proxy::CrateBindingObject) = _show_crate_binding_object(io, proxy) -function _instantiate_runtime_bindings(bindings_expr::Expr) - runtime_namespace = Module(gensym(:RustCallCrateRuntime)) +""" + _instantiate_runtime_bindings(bindings_expr; target_module, visible) -> Module + +Evaluate the generated module expression and return the module. + +Where it is evaluated decides whether the caller can be precompiled (#339): + +- `target_module === nothing` — the run-time API, `load_crate_bindings` called + from a function with no expanding module: a fresh anonymous `Module` under + `Main`, as before. Nothing rooted in `Main` can be part of a package's + precompile image, and nothing that calls this way is being precompiled. +- `target_module` given (the `@rust_crate` macro passes `__module__`): + `visible = true` defines the module directly as `target_module.` — + the `submodule=` form, for `using .Name: ...`. Otherwise, **while the caller + is being precompiled** (`Base.generating_output()`), it goes into a hidden + child namespace `target_module.var"##RustCallCrateRuntime#N"`, unique per + call, so it belongs to the module tree Julia is serializing and nothing the + caller did not name appears in its namespace (the #222 contract). Outside + precompilation the anonymous `Main`-rooted module is used exactly as + before: a child module defined in the caller can never be removed, and a + run-time `@rust_crate` may be evaluated any number of times. + +Only `submodule=` makes it visible, never `name=`, and that separation is not +cosmetic: `const B = @rust_crate path name="B"` is a documented form, and +defining a module `B` in the caller and *then* binding the returned value to +the same constant produced a package whose precompile image segfaults on load +(#339 review). `name=` therefore keeps naming the module without defining +anything the caller did not ask for. +""" +function _instantiate_runtime_bindings(bindings_expr::Expr; + target_module::Union{Module, Nothing} = nothing, + visible::Bool = false) + # The caller-owned namespace exists for one reason: a module rooted in + # `Main` cannot be part of a precompile image. Outside precompilation that + # reason is absent, and a hidden child module defined in the caller on + # every call can never be removed again — a function-scope `@rust_crate` + # called in a loop, or a REPL evaluated repeatedly, would grow the caller's + # binding table for the life of the session. So the anonymous module is + # kept for run-time calls, and only a caller that is *being precompiled* + # (`Base.generating_output()`) gets the child namespace (#339 review). + # `submodule=` is a name the caller asked for, and is defined either way. + if target_module === nothing || (!visible && !Base.generating_output()) + runtime_namespace = Module(gensym(:RustCallCrateRuntime)) + return Base.invokelatest(Core.eval, runtime_namespace, bindings_expr) + end + visible && return Base.invokelatest(Core.eval, target_module, bindings_expr) + namespace_expr = Expr(:module, true, gensym(:RustCallCrateRuntime), Expr(:block)) + runtime_namespace = Base.invokelatest(Core.eval, target_module, namespace_expr) return Base.invokelatest(Core.eval, runtime_namespace, bindings_expr) end """ - load_crate_bindings(crate_path::String; output_module_name=nothing, build_release=true, cache_enabled=true) -> CrateBindings + load_crate_bindings(crate_path::String; output_module_name=nothing, submodule_name=nothing, build_release=true, cache_enabled=true, target_module=nothing) -> CrateBindings Generate, load, and return explicit bindings for a Rust crate. @@ -2413,26 +3164,50 @@ p = MyCrate.Point(3.0, 4.0) p isa MyCrate.Point ``` -`output_module_name` controls the generated runtime module name stored inside the -returned bindings object; it does not inject a caller-visible module. +`target_module` is where the generated module is defined. The `@rust_crate` +macro passes the module that expands it, which is what lets a package that +uses the macro at top level be precompiled (#339); called without it, the +module lives in an anonymous namespace under `Main` and the caller cannot be +precompiled. + +`output_module_name` names the generated module; it defines nothing in +`target_module`, so the bindings are reached through the returned value. +`submodule_name` is what defines it there — `target_module.Name`, so +`using .Name: f` works — and it also names it, so the two are not given +together. """ function load_crate_bindings(crate_path::String; output_module_name::Union{String, Nothing} = nothing, + submodule_name::Union{String, Nothing} = nothing, build_release::Bool = true, cache_enabled::Bool = true, features::Vector{String} = String[], default_features::Bool = true, + target_module::Union{Module, Nothing} = nothing, ) + if submodule_name !== nothing && output_module_name !== nothing && + submodule_name != output_module_name + throw(ArgumentError( + "load_crate_bindings: `submodule_name` ($(repr(submodule_name))) and " * + "`output_module_name` ($(repr(output_module_name))) name the same module " * + "and must agree; pass only `submodule_name` to define it in the caller")) + end + module_name = submodule_name === nothing ? output_module_name : submodule_name + bindings_expr = generate_bindings( crate_path; - output_module_name = output_module_name, + output_module_name = module_name, build_release = build_release, cache_enabled = cache_enabled, features = features, default_features = default_features, ) - crate_module = _instantiate_runtime_bindings(bindings_expr) + crate_module = _instantiate_runtime_bindings( + bindings_expr; + target_module = target_module, + visible = submodule_name !== nothing, + ) return CrateBindings(crate_module) end @@ -2450,10 +3225,32 @@ Generate and load bindings for an external Rust crate. - `path`: Path to the Rust crate (string literal) # Options -- `name="ModuleName"`: Override the generated runtime module name used inside the returned bindings object +- `name="ModuleName"`: name the generated module. It defines nothing in the + calling module; the bindings are reached through the returned value. +- `submodule="ModuleName"`: define the generated module under that name **in + the calling module**, so that `using .ModuleName: f, T` works — the shape a + package uses. Do not assign the result to the same name. - `release=true/false`: Build in release mode (default: true) - `cache=true/false`: Enable caching (default: true) +# Where the module lives + +The generated module is evaluated inside the module that expands the macro, so +a package that uses `@rust_crate` at top level can be precompiled (#339): the +crate is built and the bindings generated when the package is precompiled, and +the module's `__init__` opens the library — RustCall's cache copy — in the +session that loads the package. If that copy has been rebuilt or removed +(`RustCall.clear_cache()`), the package's precompile cache is stale and Julia +re-precompiles it, building the crate again. + +Without `submodule=` the module has a hidden, per-call name inside the caller, +so repeated calls never collide and nothing the caller did not name appears in +its namespace. With `submodule="X"`, a second `@rust_crate ... submodule="X"` +in the same module replaces `X` (Julia warns `replacing module X`); bindings +obtained earlier keep the module they hold. `submodule="X"` defines `X`, so do +not also write `const X = @rust_crate ... submodule="X"` — binding the returned +value over the module it just defined is what `name=` deliberately avoids. + # Example ```julia # Basic usage @@ -2466,10 +3263,19 @@ const MyBindings = @rust_crate "/path/to/my_crate" name="MyBindings" release=tru MyCrate.add(Int32(1), Int32(2)) p = MyCrate.Point(3.0, 4.0) MyCrate.distance(p) + +# In a package: define the module here and re-export from it +module MyPkg +using RustCall +@rust_crate joinpath(@__DIR__, "..", "deps", "my_crate") submodule="Bindings" +using .Bindings: add, Point +export add, Point +end ``` """ macro rust_crate(path, options...) module_name = nothing + submodule_name = nothing release = true cache = true features = :(String[]) @@ -2482,6 +3288,8 @@ macro rust_crate(path, options...) if key == :name module_name = value + elseif key == :submodule + submodule_name = value elseif key == :release release = value elseif key == :cache @@ -2494,14 +3302,19 @@ macro rust_crate(path, options...) end end + # `__module__` is the module the macro expands in. The generated module is + # placed inside it — hidden unless `submodule=` names it — which is what a + # package precompiling this call site needs (#339). quote load_crate_bindings( $(esc(path)); output_module_name = $module_name, + submodule_name = $submodule_name, build_release = $release, cache_enabled = $cache, features = String[$(esc(features))...], default_features = $(esc(default_features)), + target_module = $__module__, ) end end diff --git a/src/manifest.jl b/src/manifest.jl index a06ba202..513b1350 100644 --- a/src/manifest.jl +++ b/src/manifest.jl @@ -410,6 +410,42 @@ function _cargo_config_digest(env = ENV; dir::Union{Nothing, AbstractString} = n return bytes2hex(sha256(take!(io))) end +""" + _cargo_config_files(env = ENV; dir = nothing) -> Vector{String} + +The Cargo configuration files that are in effect for a build under `dir`, in +the order `_cargo_config_digest` reads them: the nearest `.cargo/config.toml` +(or `.cargo/config`) of `dir` and each ancestor, then `CARGO_HOME`'s. + +The same files, so a caller that must *track* them cannot drift from the +digest that decides the artifact key. A generated crate module declares them as +precompile dependencies (#339 review): they can change the compiler flags and +therefore the binary, without any file of the crate changing. +""" +function _cargo_config_files(env = ENV; dir::Union{Nothing, AbstractString} = nothing) + files = String[] + if dir !== nothing + for d in _cargo_config_search_dirs(dir) + for name in ("config.toml", "config") + path = joinpath(d, ".cargo", name) + if isfile(path) + push!(files, path) + break + end + end + end + end + home = get(env, "CARGO_HOME", joinpath(homedir(), ".cargo")) + for name in ("config.toml", "config") + path = joinpath(home, name) + if isfile(path) + push!(files, path) + break + end + end + return files +end + # `dir` and each of its ancestors, nearest first — where Cargo looks for # `.cargo/config.toml`. function _cargo_config_search_dirs(dir::AbstractString) diff --git a/src/pyo3.jl b/src/pyo3.jl index bb083b85..04ff4d8a 100644 --- a/src/pyo3.jl +++ b/src/pyo3.jl @@ -1501,10 +1501,10 @@ function _build_pyo3_wrapper_project(info::CrateInfo, plan::PyO3LinkPlan, @debug "Failed to cache PyO3 wrapper library: $e" end end - # No cache: keep the library somewhere the cleanup below does not reach. - keep = joinpath(mktempdir(prefix = "rustcall_pyo3_lib_"), basename(built)) - cp(built, keep; force = true) - return keep + # No cache: keep the library somewhere the cleanup below does not + # reach — and that outlives this process, since the module that + # records the path may be loaded by a later one (#339 review). + return _uncached_library_home(built) finally cleanup_cargo_project(project) end diff --git a/test/test_crate_bindings.jl b/test/test_crate_bindings.jl index 02a047a1..9de3e195 100644 --- a/test/test_crate_bindings.jl +++ b/test/test_crate_bindings.jl @@ -388,6 +388,8 @@ function _run_top_level_explicit_binding_contract() @test !occursin("RustCallCrateRuntime", point_display) @test SampleCrateContract.distance_from_origin(point) == 5.0 @test point.x == 3.0 + # `name=` names the generated module and defines nothing in the + # caller (#222); `submodule=` is what defines it (#339). @test !isdefined(Main, :SampleCrateInjected) """) end diff --git a/test/test_docs_examples.jl b/test/test_docs_examples.jl index 86d36be9..4e8f3052 100644 --- a/test/test_docs_examples.jl +++ b/test/test_docs_examples.jl @@ -496,7 +496,11 @@ const _DOCS_SAMPLE_CRATE_AVAILABLE = isdir(DOCS_SAMPLE_CRATE_PATH) @testset "crate_bindings.md - Explicit Binding" begin if _DOCS_SAMPLE_CRATE_AVAILABLE - # @rust_crate should return a local bindings value, not inject a module into Main. + # @rust_crate returns a local bindings value and adds nothing + # visible to the caller's namespace (#222) — `name=` only names + # the generated module. `submodule=` is the option that defines it + # in the caller (#339), and it is exercised in + # test_rust_crate_precompile.jl. let DocsSampleCrate = @rust_crate DOCS_SAMPLE_CRATE_PATH name="DocsSampleCrateInjected" @test DocsSampleCrate.add(Int32(1), Int32(2)) == Int32(3) @test DocsSampleCrate.Point isa DataType @@ -504,7 +508,23 @@ const _DOCS_SAMPLE_CRATE_AVAILABLE = isdir(DOCS_SAMPLE_CRATE_PATH) @test point isa DocsSampleCrate.Point @test DocsSampleCrate.distance_from_origin(point) == 5.0 @test Base.invokelatest(getproperty, point, :x) == 3.0 - @test !isdefined(Main, :DocsSampleCrateInjected) + @test !isdefined(@__MODULE__, :DocsSampleCrateInjected) + end + # `names` reads the binding table in the *current* world age, and + # this testset body runs in the world it started in — so a + # binding created inside it is only visible through `invokelatest`. + # At run time — not precompiling — nothing at all is added to the + # caller: the module lives under an anonymous `Main`-rooted module, + # so a repeated call leaves no trace (#222, #339 review). The + # caller-owned namespace is used only while precompiling, which + # test_rust_crate_precompile.jl exercises in a subprocess. + let before = Set(Base.invokelatest(names, @__MODULE__; all = true)), + DocsAnonymous = @rust_crate DOCS_SAMPLE_CRATE_PATH + @test DocsAnonymous.add(Int32(1), Int32(2)) == Int32(3) + added = setdiff(Set(Base.invokelatest(names, @__MODULE__; all = true)), before) + @test isempty(added) + @test !isdefined(@__MODULE__, :SampleCrate) + @test parentmodule(parentmodule(DocsAnonymous.module_ref)) === Main end else @test_skip "test/fixtures/sample_crate not available" diff --git a/test/test_rust_crate_precompile.jl b/test/test_rust_crate_precompile.jl new file mode 100644 index 00000000..75440cc4 --- /dev/null +++ b/test/test_rust_crate_precompile.jl @@ -0,0 +1,1129 @@ +#!/usr/bin/env julia +# `@rust_crate` at the top level of a package that is precompiled (#339). +# +# The generated module used to be evaluated into an anonymous `Module` under +# `Main`, which Julia refuses to serialize into a package's precompile image +# ("Evaluation into the closed module ... breaks incremental compilation"). It +# is now defined inside the module that expands the macro. Three things have +# to hold, and each needs a *separate* process, because a precompile image is +# only ever consumed by a session other than the one that produced it: +# +# 1. a package with `@rust_crate submodule="Bindings"` at top level, +# re-exporting through `using .Bindings: ...`, precompiles; +# 2. a fresh session loads it from the image: the module's `__init__` opens +# the library — RustCall's durable cache copy, not the generation copy +# the precompiling process made — and functions and structs work; +# 3. after `RustCall.clear_cache()` the image is stale (the library is a +# precompile dependency through `Base.include_dependency`), so the next +# `using` re-precompiles the package and builds the crate again, rather +# than `__init__` failing on a path that is gone. +# +# The subprocesses inherit this process's environment — `RUSTCALL_EXTRACT` +# included — but get a cache directory of their own (`RUSTCALL_CACHE_DIR`), so +# clearing it does not touch the cache the other test workers share. Cargo's +# own output under the fixture's `target/` is shared, so the second build in +# step 3 is a no-op for Cargo: one real crate build. + +using Test +using RustCall +using RustToolChain: cargo + +const PRECOMP_SAMPLE_CRATE = joinpath(@__DIR__, "fixtures", "sample_crate") +# A crate RustCall has to *wrap*: its `[lib]` is an `rlib`, so the binding goes +# through a generated wrapper project — a temporary directory — rather than +# through the crate's own `cdylib` output. +const PRECOMP_WRAPPED_CRATE = joinpath(@__DIR__, "fixtures", "sample_crate_pyo3_optional") + +function _precomp_cargo_available() + try + run(pipeline(`$(cargo()) --version`, devnull)) + return true + catch + return false + end +end + +@testset "@rust_crate in a precompiled package (#339)" begin + if !isdir(PRECOMP_SAMPLE_CRATE) || !_precomp_cargo_available() + @test_skip "cargo and test/fixtures/sample_crate are required" + else + root = mktempdir() + pkg_name = "RustCratePrecomp339" + pkg_uuid = "3d2f9a71-6b0e-4c2a-9f1d-5e8b7c6a4d39" + pkgdir_ = joinpath(root, pkg_name) + cache_dir = joinpath(root, "rustcall-cache") + mkpath(joinpath(pkgdir_, "src")) + mkpath(cache_dir) + write(joinpath(pkgdir_, "Project.toml"), """ + name = "$pkg_name" + uuid = "$pkg_uuid" + version = "0.1.0" + + [deps] + RustCall = "$(Base.PkgId(RustCall).uuid)" + """) + # No `Libdl` dependency on purpose: the generated module imports it + # through RustCall. `LOADED_AT_PRECOMPILE` records whether the library + # was open while the package's own top level ran — it is not: Julia + # defers the generated module's `__init__` to load time. + write(joinpath(pkgdir_, "src", "$pkg_name.jl"), """ + module $pkg_name + using RustCall + @rust_crate $(repr(abspath(PRECOMP_SAMPLE_CRATE))) submodule="Bindings" + using .Bindings: add, Point, distance_from_origin + export add, Point, distance_from_origin + const LOADED_AT_PRECOMPILE = Bindings._LIB_GEN[].handle != C_NULL + end + """) + pkgid = Base.PkgId(Base.UUID(pkg_uuid), pkg_name) + + project = pkgdir(RustCall) + sep = Sys.iswindows() ? ";" : ":" + function in_subprocess(script::AbstractString) + withenv("JULIA_LOAD_PATH" => join((project, root, "@stdlib"), sep), + "RUSTCALL_CACHE_DIR" => cache_dir, + "RUSTCALL_SUPPRESS_HELPERS_WARNING" => "1") do + # stderr carries the precompilation progress and RustCall's + # `@info` lines; only stdout is the answer. + readchomp(pipeline(`$(Base.julia_cmd()) --startup-file=no -e $script`; + stderr = devnull)) + end + end + + try + # 1. The first session precompiles the package (a `using` does, as + # `Pkg.precompile()` would), and can use it right away. + out1 = in_subprocess(""" + using $pkg_name + print(add(Int32(2), Int32(3)), " ", $pkg_name.LOADED_AT_PRECOMPILE, " ", + Base.isprecompiled(Base.identify_package("$pkg_name"))) + """) + @test out1 == "5 false true" + + # 2. A fresh session finds the image valid, loads the library from + # the durable cache path through a per-process copy, and both a + # function and a struct — a real type of the package, usable + # with `isa` — work. + out2 = in_subprocess(""" + using RustCall, Libdl + id = Base.identify_package("$pkg_name") + was_precompiled = Base.isprecompiled(id) + using $pkg_name + B = $pkg_name.Bindings + p = Point(3.0, 4.0) + loaded = Libdl.dlpath(B._LIB_GEN[].handle) + print(was_precompiled, " ", add(Int32(2), Int32(3)), " ", + distance_from_origin(p), " ", p isa Point, " ", + typeof(p) === B.Point, " ", + B isa Module, " ", parentmodule(B) === $pkg_name, " ", + startswith(B._LIB_PATH, $(repr(cache_dir))), " ", + isfile(B._LIB_PATH), " ", + realpath(loaded) != realpath(B._LIB_PATH), " ", + dirname(realpath(loaded)) == dirname(realpath(B._LIB_PATH)), " ", + occursin(".rustcall.", basename(loaded))) + """) + @test out2 == "true 5 5.0 true true true true true true true true true" + + # The library is recorded as a precompile dependency of the image, + # which is what makes step 3 deterministic. + cachefiles = Base.find_all_in_cache_path(pkgid) + @test !isempty(cachefiles) + if !isempty(cachefiles) + # `parse_cache_header` returns `(modules, (includes, srcfiles, + # requires), ...)`; the include_dependency records are in + # `includes`. + includes = Base.parse_cache_header(first(cachefiles))[2][1] + @test any(inc -> startswith(inc.filename, cache_dir), includes) + end + + # 3. Clearing RustCall's cache removes the library. The image is + # now stale — not broken — so the next `using` re-precompiles + # the package, `@rust_crate` builds the crate again, and calls + # work. + out3 = in_subprocess(""" + using RustCall + RustCall.clear_cache() + id = Base.identify_package("$pkg_name") + stale = !Base.isprecompiled(id) + using $pkg_name + B = $pkg_name.Bindings + print(stale, " ", add(Int32(4), Int32(5)), " ", isfile(B._LIB_PATH), " ", + Base.isprecompiled(id)) + """) + @test out3 == "true 9 true true" + finally + # The image lives in the shared depot, in a directory named after + # this package alone: remove it, then the subprocesses' cache + # directory together with the temporary package. + for dir in unique(dirname.(Base.find_all_in_cache_path(pkgid))) + rm(dir; recursive = true, force = true) + end + rm(root; recursive = true, force = true) + end + end +end + +# The path the generated module carries must be one that exists for as long as +# the module does. With caching on that is RustCall's cache copy; with +# `cache = false` on a crate that needs a wrapper crate it used to be a file +# inside the wrapper project, which `cleanup_cargo_project` deletes as soon as +# the build returns — so `__init__` opened a path that was already gone +# ("could not load library ... /T/rustcall_wrapper_XXXXXX/target/release/..."). +@testset "cache = false never names a deleted build tree (#339)" begin + if !isdir(PRECOMP_WRAPPED_CRATE) || !_precomp_cargo_available() + @test_skip "cargo and test/fixtures/sample_crate_pyo3_optional are required" + else + # Before the fix this line itself threw: the module's `__init__` ran + # `dlopen` on a file the wrapper project's cleanup had already taken + # with it. + bindings = @rust_crate PRECOMP_WRAPPED_CRATE cache=false + generated = bindings.module_ref + @test isfile(generated._LIB_PATH) + # And the copy outlives this process: not a `mktempdir()` that is + # cleaned at exit, but a directory under the Cargo cache that only + # `clear_cache()` removes — a package precompiled with `cache = false` + # is loaded by a process other than the one that made the copy. + @test startswith(generated._LIB_PATH, RustCall.get_cargo_cache_dir()) + @test occursin("uncached_", generated._LIB_PATH) + # The library is open, so the path named a real file at load time as + # well as now. + @test generated._LIB_GEN[].handle != C_NULL + # This crate exposes nothing without its `python` feature, so there is + # no binding to call: the library and its load are the whole claim. + @test isempty(RustCall.scan_crate(PRECOMP_WRAPPED_CRATE).julia_functions) + end +end + +# A run-time `@rust_crate` — a function called in a loop, a REPL line evaluated +# again — must not grow the caller: a child module defined in the caller can +# never be removed, so outside precompilation the module stays under an +# anonymous `Main`-rooted module, as it always did (#339 review). +@testset "A run-time @rust_crate leaves nothing in the caller (#339 review)" begin + if !isdir(PRECOMP_SAMPLE_CRATE) || !_precomp_cargo_available() + @test_skip "cargo and test/fixtures/sample_crate are required" + else + load_twice() = (@rust_crate PRECOMP_SAMPLE_CRATE), (@rust_crate PRECOMP_SAMPLE_CRATE) + before = Set(Base.invokelatest(names, @__MODULE__; all = true)) + a, b = load_twice() + @test a.add(Int32(1), Int32(1)) == 2 && b.add(Int32(2), Int32(2)) == 4 + @test Set(Base.invokelatest(names, @__MODULE__; all = true)) == before + @test parentmodule(parentmodule(a.module_ref)) === Main + @test a.module_ref !== b.module_ref + end +end + +# `const X = @rust_crate name="X"` is the form the macro's docstring has +# always shown. It must keep working, and that is why `name=` names the +# generated module without defining it in the caller: a version that defined +# `X` and then bound the returned `CrateBindings` over it produced a package +# whose precompile image **segfaulted** on load (signal 11), not merely a +# redefinition error (#339 review). +@testset "const X = @rust_crate ... name=\"X\" still loads (#339 review)" begin + if !isdir(PRECOMP_SAMPLE_CRATE) || !_precomp_cargo_available() + @test_skip "cargo and test/fixtures/sample_crate are required" + else + # In-process first: the name is not defined here, only the value. + bindings = @rust_crate PRECOMP_SAMPLE_CRATE name="PrecompSameName" + @test bindings isa RustCall.CrateBindings + @test !isdefined(@__MODULE__, :PrecompSameName) + @test nameof(bindings.module_ref) === :PrecompSameName + + # And in a package that is precompiled and then loaded in a fresh + # session, which is where the crash happened. + root = mktempdir() + pkg_name = "RustCrateSameName339" + pkg_uuid = "5c7e1b90-2d43-4f18-9a06-3b8e7d24c1af" + pkgdir_ = joinpath(root, pkg_name) + cache_dir = joinpath(root, "rustcall-cache") + mkpath(joinpath(pkgdir_, "src")) + mkpath(cache_dir) + write(joinpath(pkgdir_, "Project.toml"), """ + name = "$pkg_name" + uuid = "$pkg_uuid" + version = "0.1.0" + + [deps] + RustCall = "$(Base.PkgId(RustCall).uuid)" + """) + write(joinpath(pkgdir_, "src", "$pkg_name.jl"), """ + module $pkg_name + using RustCall + const MyBindings = @rust_crate $(repr(abspath(PRECOMP_SAMPLE_CRATE))) name="MyBindings" + end + """) + pkgid = Base.PkgId(Base.UUID(pkg_uuid), pkg_name) + sep = Sys.iswindows() ? ";" : ":" + try + out = withenv("JULIA_LOAD_PATH" => join((pkgdir(RustCall), root, "@stdlib"), sep), + "RUSTCALL_CACHE_DIR" => cache_dir, + "RUSTCALL_SUPPRESS_HELPERS_WARNING" => "1") do + readchomp(pipeline(`$(Base.julia_cmd()) --startup-file=no -e """ + using $pkg_name + print($pkg_name.MyBindings.add(Int32(1), Int32(2))) + """`; stderr = devnull)) + end + @test out == "3" + finally + for dir in unique(dirname.(Base.find_all_in_cache_path(pkgid))) + rm(dir; recursive = true, force = true) + end + rm(root; recursive = true, force = true) + end + end +end + +# Editing the crate must invalidate the package's precompile image. It does not +# follow from tracking the library: the library is content-addressed, so a new +# build lands at a *different* cache path and leaves the old file untouched — +# nothing Julia tracks would have moved, and the package would go on calling the +# previous build. The module therefore declares the crate's own input files, the +# very set its artifact identity is computed from (#339 review). +@testset "Editing the crate invalidates the package image (#339 review)" begin + if !RustCall.check_rustc_available() + @test_skip "rustc is required" + else + root = mktempdir() + crate = joinpath(root, "edited_crate") + mkpath(joinpath(crate, "src")) + macros = replace(joinpath(dirname(@__DIR__), "deps", "juliacall_macros"), "\\" => "/") + write(joinpath(crate, "Cargo.toml"), """ + [package] + name = "edited_crate" + version = "0.1.0" + edition = "2021" + + [lib] + crate-type = ["cdylib"] + + [dependencies] + juliacall_macros = { path = "$macros" } + """) + source(offset) = """ + use juliacall_macros::julia; + #[julia] + pub fn total(a: i32, b: i32) -> i32 { a + b + $offset } + """ + write(joinpath(crate, "src", "lib.rs"), source(0)) + + pkg_name = "RustCrateEdited339" + pkg_uuid = "9f3c1d70-4a52-4b86-9d13-7e2c5a8b6f04" + pkgdir_ = joinpath(root, pkg_name) + cache_dir = joinpath(root, "rustcall-cache") + mkpath(joinpath(pkgdir_, "src")) + mkpath(cache_dir) + write(joinpath(pkgdir_, "Project.toml"), """ + name = "$pkg_name" + uuid = "$pkg_uuid" + version = "0.1.0" + + [deps] + RustCall = "$(Base.PkgId(RustCall).uuid)" + """) + write(joinpath(pkgdir_, "src", "$pkg_name.jl"), """ + module $pkg_name + using RustCall + @rust_crate $(repr(abspath(crate))) submodule="Bindings" + using .Bindings: total + export total + end + """) + pkgid = Base.PkgId(Base.UUID(pkg_uuid), pkg_name) + sep = Sys.iswindows() ? ";" : ":" + run_pkg(script) = withenv("JULIA_LOAD_PATH" => join((pkgdir(RustCall), root, "@stdlib"), sep), + "RUSTCALL_CACHE_DIR" => cache_dir, + "RUSTCALL_SUPPRESS_HELPERS_WARNING" => "1") do + readchomp(pipeline(`$(Base.julia_cmd()) --startup-file=no -e $script`; stderr = devnull)) + end + + # `Base.isprecompiled` needs the package's *source* on the load path, + # which only the subprocesses have — asked here it raises "Cannot + # locate source". So the staleness question is asked inside a session + # that can see the package, before it loads it. + stale_then_call = """ + id = Base.identify_package("$pkg_name") + stale = !Base.isprecompiled(id) + using $pkg_name + print(stale, " ", total(Int32(2), Int32(3)), " ", Base.isprecompiled(id)) + """ + + try + @test run_pkg(stale_then_call) == "true 5 true" + + # The crate's source files are among the image's dependencies. + cachefiles = Base.find_all_in_cache_path(pkgid) + @test !isempty(cachefiles) + if !isempty(cachefiles) + includes = Base.parse_cache_header(first(cachefiles))[2][1] + @test any(inc -> inc.filename == joinpath(crate, "src", "lib.rs"), includes) + end + + # Nothing changed: the image stays valid and the crate is not + # rebuilt. + @test run_pkg(stale_then_call) == "false 5 true" + + # Edit the crate. `include_dependency` compares mtimes, and a build + # can be fast enough to land in the same second. + sleep(1.1) + write(joinpath(crate, "src", "lib.rs"), source(100)) + + # The next session finds the image stale, re-precompiles, rebuilds + # the crate, and calls the new code. + @test run_pkg(stale_then_call) == "true 105 true" + + # A file *appearing* changes the artifact — `crate_content_digest` + # hashes the file list — while touching none of the files the image + # already knew. The directories are in the dependency list for + # exactly this (#339 review). + sleep(1.1) + write(joinpath(crate, "src", "extra.rs"), "// not referenced\n") + @test run_pkg(stale_then_call) == "true 105 true" + + # And the file it added is itself tracked from then on. + sleep(1.1) + rm(joinpath(crate, "src", "extra.rs")) + @test run_pkg(stale_then_call) == "true 105 true" + + # A directory that held no input at all is tracked too: the first + # file appearing in an empty `assets/` changes the artifact, moves + # no file, and leaves the parent's entry list alone because the + # directory was already there (#339 review). + mkpath(joinpath(crate, "assets")) + sleep(1.1) + @test run_pkg(stale_then_call) == "true 105 true" # `assets/` itself is new + sleep(1.1) + write(joinpath(crate, "assets", "table.csv"), "1,2\n") + @test run_pkg(stale_then_call) == "true 105 true" # a file inside it is new + finally + for dir in unique(dirname.(Base.find_all_in_cache_path(pkgid))) + rm(dir; recursive = true, force = true) + end + rm(root; recursive = true, force = true) + end + end +end + +# `.cargo/config.toml` decides the flags a build runs under and is part of the +# artifact key (`_cargo_config_digest`), so an edit to it changes the binary +# without touching a file of the crate. It has to be tracked as well, or the +# package's image stays valid over a build it no longer describes (#339 review). +# A `features = [...]` build can activate an optional `path` dependency that the +# default `cargo tree` graph never lists; every local dependency a manifest +# declares is tracked, optional or not (#339 review). +@testset "Optional path dependencies are precompile dependencies (#339 review)" begin + mktempdir() do root + mkpath(joinpath(root, "main", "src")); mkpath(joinpath(root, "extra", "src")) + write(joinpath(root, "extra", "Cargo.toml"), """ + [package] + name = "extra" + version = "0.1.0" + edition = "2021" + """) + write(joinpath(root, "extra", "src", "lib.rs"), "pub fn e() -> i32 { 1 }\n") + write(joinpath(root, "main", "Cargo.toml"), """ + [package] + name = "main" + version = "0.1.0" + edition = "2021" + + [features] + with_extra = ["dep:extra"] + + [dependencies] + extra = { path = "../extra", optional = true } + """) + write(joinpath(root, "main", "src", "lib.rs"), "pub fn m() -> i32 { 1 }\n") + deps = RustCall._crate_precompile_dependencies(joinpath(root, "main")) + @test joinpath(root, "extra", "src", "lib.rs") in deps + @test joinpath(root, "extra", "Cargo.toml") in deps + # The crate's *parent* is not an input: an unrelated sibling appearing + # in the checkout must not invalidate the image (#339 review). + @test normpath(root) ∉ normpath.(deps) + + # The same crate is in the artifact key: an edit to it changes the + # digest, so a rebuild cannot find the old library under the old key. + _, dirs = RustCall.local_path_dependency_dirs(joinpath(root, "main")) + @test any(d -> RustCall._canonical_dir(d) == RustCall._canonical_dir(joinpath(root, "extra")), dirs) + before = RustCall.artifact_path_dependency_digest(joinpath(root, "main")) + write(joinpath(root, "extra", "src", "lib.rs"), "pub fn e() -> i32 { 2 }\n") + @test RustCall.artifact_path_dependency_digest(joinpath(root, "main")) != before + + # And a crate the optional one declares in turn is followed as well. + mkpath(joinpath(root, "deeper", "src")) + write(joinpath(root, "deeper", "Cargo.toml"), """ + [package] + name = "deeper" + version = "0.1.0" + edition = "2021" + """) + write(joinpath(root, "deeper", "src", "lib.rs"), "pub fn d() -> i32 { 1 }\n") + write(joinpath(root, "extra", "Cargo.toml"), """ + [package] + name = "extra" + version = "0.1.0" + edition = "2021" + + [dependencies] + deeper = { path = "../deeper", optional = true } + """) + @test joinpath(root, "deeper", "src", "lib.rs") in + RustCall._crate_precompile_dependencies(joinpath(root, "main")) + end +end + +# `[patch.crates-io] extra = { path = "../local" }` swaps a registry dependency +# for a local crate. The dependency table still says `version = "..."`, so +# harvesting it never names the directory, and when the dependency is optional +# the default `cargo tree` graph omits it as well: an edit to the local crate +# changed neither the artifact key nor the declared inputs, and a +# feature-enabled rebuild found the old library (#339 review). Patch tables are +# harvested too — the crate's own and, for a workspace member, the root's, +# which is the one Cargo honours. +@testset "Patched-in local crates are precompile dependencies (#339 review)" begin + mktempdir() do root + mkpath(joinpath(root, "main", "src")); mkpath(joinpath(root, "local_extra", "src")) + write(joinpath(root, "local_extra", "Cargo.toml"), """ + [package] + name = "extra" + version = "0.1.0" + edition = "2021" + """) + write(joinpath(root, "local_extra", "src", "lib.rs"), "pub fn e() -> i32 { 1 }\n") + write(joinpath(root, "main", "Cargo.toml"), """ + [package] + name = "main" + version = "0.1.0" + edition = "2021" + + [features] + with_extra = ["dep:extra"] + + [dependencies] + extra = { version = "0.1", optional = true } + + [patch.crates-io] + extra = { path = "../local_extra" } + """) + write(joinpath(root, "main", "src", "lib.rs"), "pub fn m() -> i32 { 1 }\n") + @test any(p -> RustCall._canonical_dir(p) == RustCall._canonical_dir(joinpath(root, "local_extra")), + RustCall._declared_path_dependencies(joinpath(root, "main", "Cargo.toml"))) + deps = RustCall._crate_precompile_dependencies(joinpath(root, "main")) + @test joinpath(root, "local_extra", "src", "lib.rs") in deps + @test joinpath(root, "local_extra", "Cargo.toml") in deps + _, dirs = RustCall.local_path_dependency_dirs(joinpath(root, "main")) + @test any(d -> RustCall._canonical_dir(d) == RustCall._canonical_dir(joinpath(root, "local_extra")), dirs) + before = RustCall.artifact_path_dependency_digest(joinpath(root, "main")) + write(joinpath(root, "local_extra", "src", "lib.rs"), "pub fn e() -> i32 { 2 }\n") + @test RustCall.artifact_path_dependency_digest(joinpath(root, "main")) != before + + # A workspace member's `[patch]` lives in the root manifest, and the + # path there is relative to the root, not to the member. + mkpath(joinpath(root, "ws", "member", "src")); mkpath(joinpath(root, "ws", "vendored", "src")) + write(joinpath(root, "ws", "Cargo.toml"), """ + [workspace] + members = ["member"] + + [patch.crates-io] + extra = { path = "vendored" } + """) + write(joinpath(root, "ws", "vendored", "Cargo.toml"), """ + [package] + name = "extra" + version = "0.1.0" + edition = "2021" + """) + write(joinpath(root, "ws", "vendored", "src", "lib.rs"), "pub fn v() -> i32 { 1 }\n") + write(joinpath(root, "ws", "member", "Cargo.toml"), """ + [package] + name = "member" + version = "0.1.0" + edition = "2021" + + [features] + with_extra = ["dep:extra"] + + [dependencies] + extra = { version = "0.1", optional = true } + """) + write(joinpath(root, "ws", "member", "src", "lib.rs"), "pub fn m() -> i32 { 1 }\n") + @test joinpath(root, "ws", "vendored", "src", "lib.rs") in + RustCall._crate_precompile_dependencies(joinpath(root, "ws", "member")) + before = RustCall.artifact_path_dependency_digest(joinpath(root, "ws", "member")) + write(joinpath(root, "ws", "vendored", "src", "lib.rs"), "pub fn v() -> i32 { 2 }\n") + @test RustCall.artifact_path_dependency_digest(joinpath(root, "ws", "member")) != before + end +end + +@testset "Cargo configuration files are precompile dependencies (#339 review)" begin + if !RustCall.check_rustc_available() + @test_skip "rustc is required" + else + root = mktempdir() + crate = joinpath(root, "configured_crate") + mkpath(joinpath(crate, "src", "")) + mkpath(joinpath(crate, ".cargo")) + config = joinpath(crate, ".cargo", "config.toml") + write(config, "# empty\n") + macros = replace(joinpath(dirname(@__DIR__), "deps", "juliacall_macros"), "\\" => "/") + write(joinpath(crate, "Cargo.toml"), """ + [package] + name = "configured_crate" + version = "0.1.0" + edition = "2021" + + [lib] + crate-type = ["cdylib"] + + [dependencies] + juliacall_macros = { path = "$macros" } + """) + write(joinpath(crate, "src", "lib.rs"), """ + use juliacall_macros::julia; + #[julia] + pub fn one() -> i32 { 1 } + """) + try + # The list the generated module declares, and the digest that + # decides the artifact key, must name the same file. + deps = RustCall._crate_precompile_dependencies(crate) + @test config in deps + @test joinpath(crate, "src", "lib.rs") in deps + @test config in RustCall._cargo_config_files(ENV; dir = crate) + finally + rm(root; recursive = true, force = true) + end + end +end + +# `PYO3_CONFIG_FILE` is an input by content, so the file is declared; its +# directory is not — an unrelated sibling appearing beside a configuration that +# lives outside the crate tree changes nothing the build reads, and tracking +# the directory would re-precompile the package for it (#339 review). +@testset "PYO3_CONFIG_FILE is tracked as a file, not with its directory (#339 review)" begin + if !isdir(PRECOMP_SAMPLE_CRATE) + @test_skip "test/fixtures/sample_crate is required" + else + mktempdir() do dir + config = joinpath(dir, "pyo3-build-config.txt") + write(config, "implementation=CPython\nversion=3.12\nshared=true\n") + deps = withenv("PYO3_CONFIG_FILE" => config) do + RustCall._crate_precompile_dependencies(PRECOMP_SAMPLE_CRATE) + end + @test normpath(config) in deps + @test normpath(dir) ∉ deps + @test normpath(config) ∉ withenv("PYO3_CONFIG_FILE" => nothing) do + RustCall._crate_precompile_dependencies(PRECOMP_SAMPLE_CRATE) + end + end + end +end + +# Part of the artifact identity is not a file — `RUSTFLAGS`, `PYO3_PYTHON`, a +# `PYO3_CONFIG_FILE` pointing somewhere else — and Julia invalidates a +# precompile image from files alone. The module cannot make the image stale, so +# it records the values it was built under and says so at load time rather than +# loading a library built for another environment in silence (#339 review). +@testset "A changed build environment is reported at load time (#339 review)" begin + # The recorded set is what `_recorded_build_env` captures at generation — + # the allowlist plus RustCall's own selectors — so it is taken from the same + # environment the comparison starts in. + withenv("RUSTFLAGS" => "-C target-cpu=native", "PYO3_PYTHON" => "/usr/bin/python3") do + recorded = Any[String(k) => String(v) for (k, v) in RustCall._recorded_build_env()] + @test any(p -> first(p) == "PYO3_PYTHON", recorded) + + # Unchanged: nothing to say. + @test_logs RustCall._warn_if_build_env_changed(recorded, "/crate", "lib") + + # Pointing elsewhere: warned, which is the case Julia's file-based + # invalidation cannot see. + withenv("PYO3_PYTHON" => "/opt/py/bin/python3") do + @test_logs (:warn,) match_mode = :any RustCall._warn_if_build_env_changed( + recorded, "/crate", "lib") + end + + # Gone away: warned too. + withenv("PYO3_PYTHON" => nothing) do + @test_logs (:warn,) match_mode = :any RustCall._warn_if_build_env_changed( + recorded, "/crate", "lib") + end + end + + # `CARGO_HOME` selects the effective Cargo configuration and is *not* an + # allowlisted variable — the file's contents go into the artifact identity + # instead of its path — so pointing it elsewhere changes the flags a build + # runs under while every recorded variable, and every tracked file, stays + # as it was. The recorded digest is what notices (#339 review). + mktempdir() do home + mkpath(joinpath(home, "a")) + mkpath(joinpath(home, "b")) + write(joinpath(home, "a", "config.toml"), "[build]\nrustflags = [\"-C\", \"opt-level=1\"]\n") + write(joinpath(home, "b", "config.toml"), "[build]\nrustflags = [\"-C\", \"opt-level=3\"]\n") + crate = joinpath(home, "crate"); mkpath(crate) + digest_a = withenv("CARGO_HOME" => joinpath(home, "a")) do + RustCall._cargo_config_digest(ENV; dir = crate) + end + digest_b = withenv("CARGO_HOME" => joinpath(home, "b")) do + RustCall._cargo_config_digest(ENV; dir = crate) + end + @test digest_a != digest_b + + # No allowlisted variable moves between the two, so only the digest + # can tell them apart. + env_a = withenv("CARGO_HOME" => joinpath(home, "a")) do + Any[String(k) => String(v) for (k, v) in RustCall._recorded_build_env()] + end + withenv("CARGO_HOME" => joinpath(home, "a")) do + @test_logs RustCall._warn_if_build_env_changed(env_a, crate, "lib", digest_a) + end + withenv("CARGO_HOME" => joinpath(home, "b")) do + @test_logs (:warn,) match_mode = :any RustCall._warn_if_build_env_changed( + env_a, crate, "lib", digest_a) + end + end + + # The config selections are spliced into the wrapper module's tracked + # inputs as `String[...; last.(selections)]`: a Vector of pairs, or that + # splice is one unconvertible tuple (it was, once). + let sel = RustCall._python_config_selections() + @test sel isa Vector + @test String[String[]; last.(sel)] isa Vector{String} + end + + # `RUSTCALL_PYTHON_LIBDIR` is RustCall's own selector, outside the + # allowlist, and it decides a PyO3 wrapper's rpath and identity: it is + # recorded and compared like the rest (#339 review). + withenv("RUSTCALL_PYTHON_LIBDIR" => "/opt/py-a/lib") do + recorded = Any[String(k) => String(v) for (k, v) in RustCall._recorded_build_env(; python = true)] + @test any(p -> first(p) == "RUSTCALL_PYTHON_LIBDIR", recorded) + @test_logs RustCall._warn_if_build_env_changed(recorded, "/crate", "lib"; python = true) + withenv("RUSTCALL_PYTHON_LIBDIR" => "/opt/py-b/lib") do + @test_logs (:warn,) match_mode = :any RustCall._warn_if_build_env_changed( + recorded, "/crate", "lib"; python = true) + end + # The implicit selection moves with `PATH` when nothing pins it: the + # old interpreter is still there and unchanged, so only the recorded + # selection can say so (#339 review). + # The fake interpreters and `python3-config`s below are shell scripts: + # on Windows `Sys.which` looks for `.exe`/PATHEXT and a script does + # not run, so this block — the precedence contract included, which + # puts a fake `python3` on `PATH` — is Unix-only. The recorded-set + # tests above run everywhere. + Sys.iswindows() || mktempdir() do fake + # A shim that reports itself as `sys.executable` would: the + # selection is what the interpreter *says* it is, not the command + # found on `PATH`, so a pyenv/asdf shim whose target moved is seen. + exe = joinpath(fake, "python3") + write(exe, "#!/bin/sh\necho \"$fake/python3\"\n"); chmod(exe, 0o755) + withenv("PYO3_PYTHON" => nothing) do + before = RustCall._python_selection() + withenv("PATH" => fake * (Sys.iswindows() ? ";" : ":") * get(ENV, "PATH", "")) do + @test RustCall._python_selection() == exe + @test RustCall._python_selection() != before + end + end + withenv("PYO3_PYTHON" => "/pinned/python3") do + @test RustCall._python_selection() == "/pinned/python3" + recorded = Any[String(k) => String(v) for (k, v) in RustCall._recorded_build_env(; python = true)] + @test any(p -> first(p) == "", recorded) + end + # `python3-config` decides the implicit link directory, and `PATH` + # may resolve it to another installation than the interpreter's: + # its selection is recorded and compared too (#339 review). + cfgexe = joinpath(fake, "python3-config") + write(cfgexe, "#!/bin/sh\necho -L$fake\n"); chmod(cfgexe, 0o755) + # The machine's own `python3` may be a framework build (macOS), + # for which the commands are never consulted — see below — so the + # implicit interpreter here is a fake that is not one: a directory + # holding only it, ahead of `PATH`, and answering with the same + # `sys.executable` as `exe` so that only the config command moves + # between the record and the comparison. + plain = mkpath(joinpath(fake, "plain")) + write(joinpath(plain, "python3"), "#!/bin/sh\necho \"$fake/python3\"\n") + chmod(joinpath(plain, "python3"), 0o755) + withenv("PYO3_PYTHON" => nothing, "RUSTCALL_PYTHON_LIBDIR" => nothing, + "PATH" => plain * (Sys.iswindows() ? ";" : ":") * get(ENV, "PATH", "")) do + recorded = Any[String(k) => String(v) for (k, v) in RustCall._recorded_build_env(; python = true)] + @test any(p -> first(p) == "", recorded) + @test any(p -> first(p) == "", recorded) + @test any(p -> first(p) == "", recorded) + # With `PYO3_PYTHON` deciding, neither config command is + # consulted, so neither is recorded (#339 review). + withenv("PYO3_PYTHON" => exe) do + pinned = Any[String(k) => String(v) for (k, v) in RustCall._recorded_build_env(; python = true)] + @test !any(p -> occursin("-config selection", first(p)), pinned) + @test !RustCall._python_link_is_implicit() + end + @test RustCall._python_link_is_implicit() + # The fake interpreter names a *file* as its framework prefix, + # so it is no framework build and the commands are consulted. + @test RustCall._python_config_consulted() + @test_logs RustCall._warn_if_build_env_changed(recorded, "/crate", "lib"; python = true) + withenv("PATH" => fake * (Sys.iswindows() ? ";" : ":") * get(ENV, "PATH", "")) do + @test Dict(RustCall._python_config_selections())["python3-config"] == cfgexe + @test_logs (:warn,) match_mode = :any RustCall._warn_if_build_env_changed( + recorded, "/crate", "lib"; python = true) + end + end + # The *answer* is recorded, not only the command: an unchanged + # `python3-config` that is a shim can name another directory once + # what it reads moves, and the wrapper's `-L`/rpath follow that + # answer (`pyo3_link_rustflags`). Same executable, same `PATH`, + # different `-L` → a warning; same answer → none (#339 review). + lib_a = mkpath(joinpath(fake, "lib-a")) + lib_b = mkpath(joinpath(fake, "lib-b")) + write(cfgexe, "#!/bin/sh\necho -L\$RUSTCALL_TEST_PY_LIBDIR\n"); chmod(cfgexe, 0o755) + withenv("PYO3_PYTHON" => nothing, "RUSTCALL_PYTHON_LIBDIR" => nothing, + "PATH" => fake * (Sys.iswindows() ? ";" : ":") * get(ENV, "PATH", "")) do + recorded = withenv("RUSTCALL_TEST_PY_LIBDIR" => lib_a) do + Any[String(k) => String(v) for (k, v) in RustCall._recorded_build_env(; python = true)] + end + @test Dict(recorded)[""] == lib_a + @test Dict(recorded)[""] == withenv("RUSTCALL_TEST_PY_LIBDIR" => lib_a) do + RustCall.python_link_source()[1] + end + withenv("RUSTCALL_TEST_PY_LIBDIR" => lib_a) do + @test_logs RustCall._warn_if_build_env_changed(recorded, "/crate", "lib"; python = true) + end + withenv("RUSTCALL_TEST_PY_LIBDIR" => lib_b) do + @test_logs (:warn,) match_mode = :any RustCall._warn_if_build_env_changed( + recorded, "/crate", "lib"; python = true) + end + end + # A framework build of Python (macOS) answers with its framework + # prefix, and `python_link_source()` never runs `python3-config` + # for it: the commands are not inputs, so neither is recorded nor + # tracked, and a changed `python3-config` on `PATH` cannot warn + # (#339 review). The fake prints an existing *directory* for every + # probe, which is what the framework-prefix question sees. + framework = joinpath(fake, "framework") + mkpath(joinpath(framework, "bin")) + write(joinpath(framework, "bin", "python3"), "#!/bin/sh\necho \"$framework\"\n") + chmod(joinpath(framework, "bin", "python3"), 0o755) + withenv("PYO3_PYTHON" => nothing, "RUSTCALL_PYTHON_LIBDIR" => nothing, + "PATH" => joinpath(framework, "bin") * (Sys.iswindows() ? ";" : ":") * fake * + (Sys.iswindows() ? ";" : ":") * get(ENV, "PATH", "")) do + @test RustCall._python_link_is_implicit() + @test RustCall._python_config_consulted() == !Sys.isapple() + recorded = Any[String(k) => String(v) for (k, v) in RustCall._recorded_build_env(; python = true)] + @test any(p -> occursin("-config selection", first(p)), recorded) == !Sys.isapple() + if Sys.isapple() + @test Dict(recorded)[""] == framework + end + end + # `RUSTCALL_PYTHON_LIBDIR` is the directory whatever else says, and + # the record follows the same precedence (#339 review). + withenv("RUSTCALL_PYTHON_LIBDIR" => lib_b, "PYO3_PYTHON" => nothing) do + recorded = Any[String(k) => String(v) for (k, v) in RustCall._recorded_build_env(; python = true)] + @test Dict(recorded)[""] == lib_b + end + write(cfgexe, "#!/bin/sh\necho -L$fake\n"); chmod(cfgexe, 0o755) + # The `python-config` fallback is a selector of its own: it is what + # answers when `python3-config` is absent or names no library + # directory, and `PATH` may move it alone (#339 review). + withenv("PYO3_PYTHON" => nothing, "RUSTCALL_PYTHON_LIBDIR" => nothing) do + recorded = Any[String(k) => String(v) for (k, v) in RustCall._recorded_build_env(; python = true)] + fallback = joinpath(fake, "python-config") + write(fallback, "#!/bin/sh\necho -L$fake\n"); chmod(fallback, 0o755) + withenv("PATH" => fake * (Sys.iswindows() ? ";" : ":") * get(ENV, "PATH", "")) do + @test Dict(RustCall._python_config_selections())["python-config"] == fallback + @test_logs (:warn,) match_mode = :any RustCall._warn_if_build_env_changed( + recorded, "/crate", "lib"; python = true) + end + rm(fallback) + end + # `PYO3_PYTHON` given as a bare command or a shim: the raw value is + # the selection (it is what `python_link_source()` pins), and what + # it *resolves to* is recorded beside it, so the same name pointing + # at another interpreter is seen (#339 review). + withenv("PYO3_PYTHON" => "python3", + "PATH" => fake * (Sys.iswindows() ? ";" : ":") * get(ENV, "PATH", "")) do + @test RustCall._python_selection() == "python3" + @test RustCall._python_resolved("python3") == exe + recorded = Any[String(k) => String(v) for (k, v) in RustCall._recorded_build_env(; python = true)] + @test ("" => exe) in recorded + @test_logs RustCall._warn_if_build_env_changed(recorded, "/crate", "lib"; python = true) + other = joinpath(fake, "other"); mkpath(other) + write(joinpath(other, "python3"), "#!/bin/sh\necho \"$other/python3\"\n") + chmod(joinpath(other, "python3"), 0o755) + withenv("PATH" => other * (Sys.iswindows() ? ";" : ":") * get(ENV, "PATH", "")) do + @test RustCall._python_selection() == "python3" # unchanged + @test_logs (:warn,) match_mode = :any RustCall._warn_if_build_env_changed( + recorded, "/crate", "lib"; python = true) # but resolved moved + end + end + + # The selector follows `python_link_source()` step for step, and + # the contract is that the two agree — here, and under the two + # precedences that differ from "pinned, else PATH": pyo3's own + # configuration leaves the interpreter to `PYO3_PYTHON` alone, and + # `RUSTCALL_PYTHON_LIBDIR` hands it to `PATH` before CondaPkg + # (#339 review). + agree() = RustCall._python_selection() == RustCall.python_link_source()[2] + withenv("PYO3_PYTHON" => nothing, "RUSTCALL_PYTHON_LIBDIR" => nothing, + "PYO3_CONFIG_FILE" => nothing, "PYO3_CROSS_LIB_DIR" => nothing) do + @test agree() + end + config = joinpath(fake, "pyo3-config.txt") + write(config, "implementation=CPython\nversion=3.12\nlib_dir=$fake\n") + withenv("PYO3_CONFIG_FILE" => config, "PYO3_PYTHON" => nothing) do + @test RustCall._python_selection() == "" + @test agree() + end + withenv("PYO3_CONFIG_FILE" => config, "PYO3_PYTHON" => "/pinned/python3") do + @test RustCall._python_selection() == "/pinned/python3" + @test agree() + end + withenv("RUSTCALL_PYTHON_LIBDIR" => fake, "PYO3_PYTHON" => nothing, + "PYO3_CONFIG_FILE" => nothing, + "PATH" => fake * (Sys.iswindows() ? ";" : ":") * get(ENV, "PATH", "")) do + @test RustCall._python_selection() == exe + @test agree() + end + end + + # A plain crate's build never consults it: not recorded, not compared, + # so configuring Python for another package warns about nothing here. + plain = Any[String(k) => String(v) for (k, v) in RustCall._recorded_build_env()] + @test !any(p -> first(p) == "RUSTCALL_PYTHON_LIBDIR", plain) + withenv("RUSTCALL_PYTHON_LIBDIR" => "/opt/py-b/lib") do + @test_logs RustCall._warn_if_build_env_changed(plain, "/crate", "lib") + end + end + + # And the toolchain: the fingerprint is in the artifact identity, and a + # `rustup update` moves no file the image tracks (#339 review). + let recorded = Any[String(k) => String(v) for (k, v) in RustCall._recorded_build_env()], + now = RustCall.toolchain_fingerprint() + @test_logs RustCall._warn_if_build_env_changed(recorded, "/crate", "lib", "", now) + @test_logs (:warn,) match_mode = :any RustCall._warn_if_build_env_changed( + recorded, "/crate", "lib", "", "not-" * now) + end +end + +# The plain-crate cache key covers the captured build environment, as the PyO3 +# wrapper's already did. Without it a changed `RUSTFLAGS` found the previous +# library in the cache and handed it back — and the load-time warning's advice +# was then wrong, because re-precompiling the package rebuilt the bindings +# around the same stale artifact (#339 review). +# When pyo3's own configuration decides the link directory, pyo3 consults no +# interpreter: the plan's fingerprint is "" and the wrapper's identity keys +# nothing by what `PYO3_PYTHON` resolves to. The record is empty there too — +# a `PYTHONHOME` change or a retargeted shim must not warn about a library the +# same environment would select again (#339 review). +@testset "Interpreter records follow the link plan (#339 review)" begin + mktempdir() do dir + config = joinpath(dir, "pyo3-build-config.txt") + write(config, "implementation=CPython\nversion=3.12\nshared=true\nlib_dir=$dir\n") + withenv("PYO3_CONFIG_FILE" => config, "PYO3_CROSS_LIB_DIR" => nothing, + "PYO3_PYTHON" => "/pinned/python3", "RUSTCALL_PYTHON_LIBDIR" => nothing) do + recorded = Dict(String(k) => String(v) for (k, v) in RustCall._recorded_build_env(; python = true)) + @test recorded[""] == "/pinned/python3" + @test recorded[""] == "" + @test recorded[""] == "" + @test recorded[""] == RustCall.python_link_source()[3] + @test recorded[""] == dir + end + # Off that branch the fingerprint is the plan's, whatever it is here. + withenv("PYO3_CONFIG_FILE" => nothing, "PYO3_CROSS_LIB_DIR" => nothing) do + recorded = Dict(String(k) => String(v) for (k, v) in RustCall._recorded_build_env(; python = true)) + @test recorded[""] == RustCall.python_link_source()[3] + end + end +end + +@testset "The plain crate key covers the build environment (#339 review)" begin + if !RustCall.check_rustc_available() + @test_skip "rustc is required" + else + info = RustCall.scan_crate(PRECOMP_SAMPLE_CRATE) + keys_of(flags) = withenv("RUSTFLAGS" => flags) do + RustCall.compute_crate_hash(info; release = true, + build_env = RustCall._plain_crate_build_env()) + end + a = keys_of("-C target-cpu=native") + b = keys_of("-C opt-level=1") + @test a != b + @test a == keys_of("-C target-cpu=native") + end +end + +# `PYO3_CONFIG_FILE` is on the allowlist by prefix, but what it *names* is a +# path, and a crate that depends on pyo3 reads the file's contents at build time. +# The wrapper path hashed those contents; the plain path keyed the path alone, +# so editing the configuration in place — another Python version, ABI or +# library directory — found the previous library in the cache (#339 review). +@testset "The plain crate key covers the contents of PYO3_CONFIG_FILE (#339 review)" begin + if !RustCall.check_rustc_available() + @test_skip "rustc is required" + else + info = RustCall.scan_crate(PRECOMP_SAMPLE_CRATE) + mktempdir() do dir + config = joinpath(dir, "pyo3-build-config.txt") + key_with(contents) = begin + write(config, contents) + withenv("PYO3_CONFIG_FILE" => config) do + RustCall.compute_crate_hash(info; release = true, + build_env = RustCall._plain_crate_build_env()) + end + end + a = key_with("implementation=CPython\nversion=3.12\nshared=true\n") + b = key_with("implementation=CPython\nversion=3.13\nshared=true\n") + @test a != b # same path, edited contents + @test a == key_with("implementation=CPython\nversion=3.12\nshared=true\n") + unset = withenv("PYO3_CONFIG_FILE" => nothing) do + RustCall.compute_crate_hash(info; release = true, + build_env = RustCall._plain_crate_build_env()) + end + @test unset != a + # Unset, the helper is the allowlist plus the interpreter pyo3's + # build script would use: no digest entry. + withenv("PYO3_CONFIG_FILE" => nothing) do + env = RustCall._plain_crate_build_env() + @test !any(p -> first(p) == "pyo3-config-file-digest", env) + @test filter(p -> !startswith(first(p), "rustcall-pyo3-"), env) == RustCall.artifact_build_env() + end + withenv("PYO3_CONFIG_FILE" => config) do + @test any(p -> first(p) == "pyo3-config-file-digest", RustCall._plain_crate_build_env()) + end + end + end +end + +# Neither the `PYO3_*` values nor the contents of `PYO3_CONFIG_FILE` are gated +# on pyo3 being in the graph: Cargo hands every ambient variable to every build +# script, and a crate's own `build.rs` may read the variable, or open the file +# it names, without depending on pyo3. The sample crate has no pyo3 anywhere in +# its graph, and both stay inputs of its build (#339 review). +@testset "PYO3_* and the config file are inputs of every plain build (#339 review)" begin + if !RustCall.check_rustc_available() + @test_skip "rustc is required" + else + info = RustCall.scan_crate(PRECOMP_SAMPLE_CRATE) + key_under(python) = withenv("PYO3_PYTHON" => python) do + RustCall.compute_crate_hash(info; release = true, + build_env = RustCall._plain_crate_build_env()) + end + @test key_under("/one/python3") != key_under("/two/python3") + withenv("PYO3_PYTHON" => "/one/python3") do + @test any(p -> first(p) == "PYO3_PYTHON", RustCall._plain_crate_build_env()) + @test any(p -> first(p) == "PYO3_PYTHON", RustCall._recorded_build_env()) + end + mktempdir() do dir + config = joinpath(dir, "pyo3-build-config.txt") + write(config, "implementation=CPython\nversion=3.12\nshared=true\n") + withenv("PYO3_CONFIG_FILE" => config) do + @test any(p -> first(p) == "pyo3-config-file-digest", RustCall._plain_crate_build_env()) + @test normpath(config) in RustCall._crate_precompile_dependencies(PRECOMP_SAMPLE_CRATE) + end + end + end +end + +# A plain build of a crate that depends on pyo3 runs pyo3's build script, which +# configures the library for the interpreter it selects — `PYO3_PYTHON`, else +# `python3` on `PATH`. The raw `PYO3_*` values see neither a `PATH` that now +# finds another Python nor a shim retargeted under one name; the interpreter's +# identity does, and it is in the key and in the load-time record (#339 +# review). Shell-script fakes: Unix only, as above. +@testset "A plain build is keyed by the interpreter pyo3 would configure for (#339 review)" begin + if !RustCall.check_rustc_available() + @test_skip "rustc is required" + else + Sys.iswindows() || mktempdir() do fake + sep = ":" + for which in ("a", "b") + dir = mkpath(joinpath(fake, which)) + exe = joinpath(dir, "python3") + write(exe, "#!/bin/sh\necho \"$dir/python3\"\n"); chmod(exe, 0o755) + end + info = RustCall.scan_crate(PRECOMP_SAMPLE_CRATE) + under(which) = withenv("PYO3_PYTHON" => nothing, "PYO3_CONFIG_FILE" => nothing, + "PYO3_CROSS_LIB_DIR" => nothing, + "PATH" => joinpath(fake, which) * sep * get(ENV, "PATH", "")) do + (RustCall.compute_crate_hash(info; release = true, + build_env = RustCall._plain_crate_build_env()), + Any[String(k) => String(v) for (k, v) in RustCall._recorded_build_env()], + RustCall._pyo3_build_interpreter()) + end + key_a, recorded_a, (interp_a, _) = under("a") + key_b, _, (interp_b, _) = under("b") + @test interp_a == joinpath(fake, "a", "python3") + @test interp_b == joinpath(fake, "b", "python3") + @test key_a != key_b # same PYO3_*, another Python on PATH + @test under("a")[1] == key_a + @test Dict(recorded_a)[""] == interp_a + withenv("PYO3_PYTHON" => nothing, "PYO3_CONFIG_FILE" => nothing, "PYO3_CROSS_LIB_DIR" => nothing, + "PATH" => joinpath(fake, "a") * sep * get(ENV, "PATH", "")) do + @test_logs RustCall._warn_if_build_env_changed(recorded_a, info.path, "lib") + end + withenv("PYO3_PYTHON" => nothing, "PYO3_CONFIG_FILE" => nothing, "PYO3_CROSS_LIB_DIR" => nothing, + "PATH" => joinpath(fake, "b") * sep * get(ENV, "PATH", "")) do + @test_logs (:warn,) match_mode = :any RustCall._warn_if_build_env_changed(recorded_a, info.path, "lib") + end + # `PYO3_PYTHON` is taken as given, the way pyo3's build script does. + withenv("PYO3_PYTHON" => joinpath(fake, "b", "python3"), "PYO3_CONFIG_FILE" => nothing, + "PYO3_CROSS_LIB_DIR" => nothing) do + @test RustCall._pyo3_build_interpreter()[1] == joinpath(fake, "b", "python3") + end + # pyo3's own configuration deciding: no interpreter is consulted, + # and none is keyed or recorded. + config = joinpath(fake, "pyo3-build-config.txt") + write(config, "implementation=CPython\nversion=3.12\nshared=true\nlib_dir=$fake\n") + withenv("PYO3_CONFIG_FILE" => config, "PYO3_PYTHON" => joinpath(fake, "a", "python3"), + "PYO3_CROSS_LIB_DIR" => nothing) do + @test RustCall._pyo3_build_interpreter() == ("", "") + @test !any(p -> startswith(first(p), "rustcall-pyo3-python"), RustCall._plain_crate_build_env()) + @test Dict(RustCall._recorded_build_env())[""] == "" + end + end + end +end + +# `PYO3_CONFIG_FILE` may name a file that does not exist yet; a build script that +# tolerates the absence is built without it, and the file appearing is then +# the one event that changes the build. Until it exists its directory is the +# declared input (the entry list sees the creation), afterwards the file is; +# and the record carries the contents' digest either way, so a load after the +# creation is told even when the image survived (#339 review). +@testset "A PYO3_CONFIG_FILE selected before it exists is seen appearing (#339 review)" begin + mktempdir() do dir + config = joinpath(dir, "pyo3-build-config.txt") + withenv("PYO3_CONFIG_FILE" => config) do + absent = RustCall._crate_precompile_dependencies(PRECOMP_SAMPLE_CRATE) + @test normpath(dir) in absent + @test normpath(config) ∉ absent + recorded = Any[String(k) => String(v) for (k, v) in RustCall._recorded_build_env()] + @test Dict(recorded)[""] == "unreadable" + @test_logs RustCall._warn_if_build_env_changed(recorded, PRECOMP_SAMPLE_CRATE, "lib") + write(config, "implementation=CPython\nversion=3.12\nshared=true\n") + present = RustCall._crate_precompile_dependencies(PRECOMP_SAMPLE_CRATE) + @test normpath(config) in present + @test normpath(dir) ∉ present + @test_logs (:warn,) match_mode = :any RustCall._warn_if_build_env_changed(recorded, PRECOMP_SAMPLE_CRATE, "lib") + end + withenv("PYO3_CONFIG_FILE" => nothing) do + @test Dict(RustCall._recorded_build_env())[""] == "" + end + end +end + +# The registry name and the cache key must be decided by the *same* environment +# snapshot. Keying only the cache gave two builds under different environments +# distinct artifacts under one `_LIB_NAME`, and loading the second replaced the +# first module's entry and mirror (#339 review). +@testset "The registry name follows the build environment too (#339 review)" begin + if !RustCall.check_rustc_available() + @test_skip "rustc is required" + else + info = RustCall.scan_crate(PRECOMP_SAMPLE_CRATE) + pair(flags) = withenv("RUSTFLAGS" => flags) do + env = RustCall._plain_crate_build_env() + (RustCall.compute_crate_hash(info; release = true, build_env = env), + RustCall.crate_library_name(info; release = true, build_env = env)) + end + a_key, a_name = pair("-C target-cpu=native") + b_key, b_name = pair("-C opt-level=1") + @test a_key != b_key + @test a_name != b_name # the name moves with the key, not apart from it + @test pair("-C target-cpu=native") == (a_key, a_name) + end +end