Because any `LazyPath` might be resolved to a relative path, it's
incorrect to pass that directly to a child process whose cwd might
differ. Instead, if the child has an overriden cwd, we need to convert
such paths to be relative to the child cwd using `std.fs.path.relative`.
Previously, various doc comments heavily disagreed with the
implementation on both what lives where on the filesystem at what time,
and how that was represented in code. Notably, the combination of emit
paths outside the cache and `disable_lld_caching` created a kind of
ad-hoc "cache disable" mechanism -- which didn't actually *work* very
well, 'most everything still ended up in this cache. There was also a
long-standing issue where building using the LLVM backend would put a
random object file in your cwd.
This commit reworks how emit paths are specified in
`Compilation.CreateOptions`, how they are represented internally, and
how the cache usage is specified.
There are now 3 options for `Compilation.CacheMode`:
* `.none`: do not use the cache. The paths we have to emit to are
relative to the compiler cwd (they're either user-specified, or
defaults inferred from the root name). If we create any temporary
files (e.g. the ZCU object when using the LLVM backend) they are
emitted to a directory in `local_cache/tmp/`, which is deleted once
the update finishes.
* `.whole`: cache the compilation based on all inputs, including file
contents. All emit paths are computed by the compiler (and will be
stored as relative to the local cache directory); it is a CLI error to
specify an explicit emit path. Artifacts (including temporary files)
are written to a directory under `local_cache/tmp/`, which is later
renamed to an appropriate `local_cache/o/`. The caller (who is using
`--listen`; e.g. the build system) learns the name of this directory,
and can get the artifacts from it.
* `.incremental`: similar to `.whole`, but Zig source file contents, and
anything else which incremental compilation can handle changes for, is
not included in the cache manifest. We don't need to do the dance
where the output directory is initially in `tmp/`, because our digest
is computed entirely from CLI inputs.
To be clear, the difference between `CacheMode.whole` and
`CacheMode.incremental` is unchanged. `CacheMode.none` is new
(previously it was sort of poorly imitated with `CacheMode.whole`). The
defined behavior for temporary/intermediate files is new.
`.none` is used for direct CLI invocations like `zig build-exe foo.zig`.
The other cache modes are reserved for `--listen`, and the cache mode in
use is currently just based on the presence of the `-fincremental` flag.
There are two cases in which `CacheMode.whole` is used despite there
being no `--listen` flag: `zig test` and `zig run`. Unless an explicit
`-femit-bin=xxx` argument is passed on the CLI, these subcommands will
use `CacheMode.whole`, so that they can put the output somewhere without
polluting the cwd (plus, caching is potentially more useful for direct
usage of these subcommands).
Users of `--listen` (such as the build system) can now use
`std.zig.EmitArtifact.cacheName` to find out what an output will be
named. This avoids having to synchronize logic between the compiler and
all users of `--listen`.
For directory arguments that need both prefix and suffix strings
appended.
Needed to unbreak ffmpeg package after fe855691f6f742a14678cb617422977c2a55be39
We have no control over memory usage on arbitrary systems in the wild. But we
would still like to get the warnings so we can adjust the values based on
observations in the official ZSF CI.
Closes#23254.
Closes#23638.
In a compiler built with debug extensions, pass `--debug-incremental` to
spawn the "incremental debug server". This is a TCP server exposing a
REPL which allows querying a bunch of compiler state, some of which is
stored only when that flag is passed. Eventually, this will probably
move into `std.zig.Server`/`std.zig.Client`, but this is easier to work
with right now. The easiest way to interact with the server is `telnet`.
Right now, if you override the build root with `--build-root`, then
`Run` steps can fail to execute because of incorrect path handling in
the compiler: `std.process.Child` gets a cwd-relative path, but also has
its cwd set to the build root. The latter behavior is really weird; it
doesn't match my expectations, nor does it match how we spawn child
`zig` processes. So, this commit makes the child process inherit the
build runner's cwd, as `LazyPath.getPath2` *expects* it to.
After investigating, this behavior dates all the way back to 2017; it
was introduced in 4543413. So, there isn't any clear/documented reason
for this; it should be safe to revert, since under the modern `LazyPath`
system it is strictly a bug AFAICT.
This function was broken, because it took ownership of the buffer on
error *sometimes*, in a way which the caller could not tell. Rather than
trying to be clever, it's easier to just follow the same interface as
all other `addFilePost` methods, and not take ownership of the path.
This is a breaking change. The next commits will apply it to the
compiler, which is the only user of this function in the ziglang/zig
repository.
Aside from adding comments to document the logic in `Cache.Manifest.hit`
better, this commit fixes two serious bugs.
The first, spotted by Andrew, is that when upgrading from a shared to an
exclusive lock on the manifest file, we do not seek it back to the
start. This is a simple fix.
The second is more subtle, and has to do with the computation of file
digests. Broadly speaking, the goal of the main loop in `hit` is to
iterate the files listed in the manifest file, and check if they've
changed, based on stat and a file hash. While doing this, the
`bin_digest` field of `std.Build.Cache.File`, which is initially
`undefined`, is populated for all files, either straight from the
manifest (if the stat matches) or recomputed from the file on-disk. This
file digest is then used to update `man.hash.hasher`, which is building
the final hash used as, for instance, the output directory name when the
compiler emits into the cache directory. When `hit` returns a cache
miss, it is expected that `man.hash.hasher` includes the digests of all
"initial files"; that is, those which have been already added with e.g.
`addFilePath`, but not those which will later be added with
`addFilePost` (even though the manifest file has told us about some such
files). Previously, `hit` was using the `unhit` function to do this in a
few cases. However, this is incorrect, because `hit` assumes that all
files already have their `bin_digest` field populated; this function is
only valid to call *after* `hit` returns. Instead, we need to actually
compute the hashes which haven't yet been populated. Even if this logic
has been working, there was still a bug here, because we called `unhit`
when upgrading from a shared to an exclusive lock, writing the
(potentially `undefined`) file digests, but the loop itself writes the
file digests *again*! All in all, the hashing logic here was actually
incredibly broken.
I've taken the opportunity to restructure this section of the code into
what I think is a more readable format. A new function,
`hitWithCurrentLock`, uses the open manifest file to try and find a
cache hit. It returns a tagged union which, in the miss case, tells the
caller (`hit`) how many files already have their hash populated. This
avoids redundant work recomputing the same hash multiple times in
situations where the lock needs upgrading. This also eliminates the
outer loop from `hit`, which was a little confusing because it iterated
no more than twice!
The bugs fixed here could manifest in several different ways depending
on how contended file locks were satisfied. Most notably, on a cache
miss, the Zig compiler might have written the compilation output to the
incorrect directory (because it incorrectly constructed a hash using
`undefined` or repeated file digests), resulting in all future hits on
this manifest causing `error.FileNotFound`. This is #23110. I have been
able to reproduce #23110 on `master`, and have not been able to after
this commit, so I am relatively sure this commit resolves that issue.
Resolves: #23110
* Accept -fsanitize-c=trap|full in addition to the existing form.
* Accept -f(no-)sanitize-trap=undefined in zig cc.
* Change type of std.Build.Module.sanitize_c to std.zig.SanitizeC.
* Add some missing Compilation.Config fields to the cache.
Closes#23216.
This is fairly straightforward; the actual compiler changes are limited
to the CLI, since `Compilation` already supports this combination.
A new `std.Build` API is introduced to allow representing this. By
passing the `emit_object` option to `std.Build.addTest`, you get a
`Step.Compile` which emits an object file; you can then use that as you
would any other object, such as either installing it for external use,
or linking it into another step.
A standalone test is added to cover the build system API. It builds a
test into an object, and links it into a final executable, which it then
runs.
Using this build system mechanism prevents the build system from
noticing that you're running a `zig test`, so the build runner and test
runner do not communicate over stdio. However, that's okay, because the
real-world use cases for this feature don't want to do that anyway!
Resolves: #23374
This change fixes false-positive cache hits for run steps that get run
with different sets of environment variables due the the environment map
being excluded from the cache hash.
The code did one useless thing and two wrong things:
- ref counting was basically a noop
- last_dir_fd was chosen from the wrong index and also under the wrong
condition
This caused regular crashes on macOS which are now gone.
This reverts commit dea72d15da4fba909dc3ccb2e9dc5286372ac023, reversing
changes made to ab381933c87bcc744058d25a876cfdc0d23fc674.
The changeset does not work as advertised and does not have sufficient
test coverage.
Reopens#22822
Inheriting allow-deprecation from parent modules doesn't make too much
sense, so instead make them default to disallow unless otherwise
specified. This allows build system to avoid redundant
`-fno-allow-deprecated` args.
This makes the generated CLIs smaller, and makes zig1.wasm update not
needed.
Also represented `is_root` differently (moved to field of graph).
This can also be extended to ELF later as it means roughly the same thing there.
This addresses the main issue in #21721 but as I don't have a macOS machine to
do further testing on, I can't confirm whether zig cc is able to pass the entire
cgo test suite after this commit. It can, however, cross-compile a basic program
that uses cgo to x86_64-macos-none which previously failed due to lack of -x
support. Unlike previously, the resulting symbol table does not contain local
symbols (such as C static functions).
I believe this satisfies the related donor bounty: https://ziglang.org/news/second-donor-bounty
Functions like isMinGW() and isGnuLibC() have a good reason to exist: They look
at multiple components of the target. But functions like isWasm(), isDarwin(),
isGnu(), etc only exist to save 4-8 characters. I don't think this is a good
enough reason to keep them, especially given that:
* It's not immediately obvious to a reader whether target.isDarwin() means the
same thing as target.os.tag.isDarwin() precisely because isMinGW() and similar
functions *do* look at multiple components.
* It's not clear where we would draw the line. The logical conclusion before
this commit would be to also wrap Arch.isX86(), Os.Tag.isSolarish(),
Abi.isOpenHarmony(), etc... this obviously quickly gets out of hand.
* It's nice to just have a single correct way of doing something.
* fix merge conflicts
* rename the declarations
* reword documentation
* extract FixedBufferAllocator to separate file
* take advantage of locals
* remove the assertion about max alignment in Allocator API, leaving it
Allocator implementation defined
* fix non-inline function call in start logic
The GeneralPurposeAllocator implementation is totally broken because it
uses global state but I didn't address that in this commit.
