rework package manager #17392

andrewrk · 2023-10-04T06:42:31Z

Organize everything around a Fetch task which does a bunch of stuff in a
worker thread without touching any shared state, and then queues up
Fetch tasks for its dependencies.

This isn't the theoretical optimal package fetching performance because
CPU cores don't necessarily map 1:1 with I/O tasks, and each fetch task
contains a mixture of computations and I/O. However, it is expected for
this to significantly outperform master branch, which fetches everything
recursively with only one thread.

The logic is now a lot more linear and easy to follow. Everything that
is embarassingly parallel is done on the thread pool, and then after
everything is fetched, the worker threads are joined and the main thread
does the finishing touches of stitching together the dependencies.zig
import files. There is only one tiny little critical section.

This also lays the groundwork for #14281 because in system mode, all
this fetching logic will be skipped, but the "finishing touches"
mentioned above still need to be done. With this branch, that logic is
separated out and no longer recursively tangled with fetching stuff.

Additionally, this branch:

Implements inclusion directives in build.zig.zon for deciding which
files belong the package (add inclusion/exclusion directives to build.zig.zon for deciding which files belong the package #14311).
Adds basic documentation for build.zig.zon files.
Adds support for fetching dependencies with the file:// protocol
scheme (support fetching dependencies with the file:// protocol scheme #17364).
Adds a workaround for a Linux/btrfs file system bug (package manager: Transitive dependencies have incorrect hash #17282).
Implements zig build --fetch (a zig subcommand that fetches remote dependencies so that subsequent zig build executions are guaranteed to not attempt network access #14280)
Adds unstable sorting to ArrayHashMap (std.ArrayHashMap: add unstable sorting capability #17426)

Merge Checklist:

Follow-up issues

xdBronch · 2023-10-04T07:10:42Z

#17094 should be closed in favor of this if it works around the bug

scheibo · 2023-10-04T16:37:23Z

doc/build.zig.zon.md

+
+[semver](https://semver.org/)
+
+### `dependenices`


Suggested change

### `dependenices`

### `dependencies`

wooster0 · 2023-10-07T16:22:47Z

lib/std/array_hash_map.zig

-            self.entries.sort(sort_ctx);
+        pub inline fn sortUnstable(self: *Self, sort_ctx: anytype) void {
+            if (@sizeOf(ByIndexContext) != 0)
+                @compileError("Cannot infer context " ++ @typeName(Context) ++ ", call sortContext instead.");


Suggested change

@compileError("Cannot infer context " ++ @typeName(Context) ++ ", call sortContext instead.");

@compileError("Cannot infer context " ++ @typeName(Context) ++ ", call sortUnstableContext instead.");

See #14311

Organize everything around a Fetch task which does a bunch of stuff in a worker thread without touching any shared state, and then queues up Fetch tasks for its dependencies. This isn't the theoretical optimal package fetching performance because CPU cores don't necessarily map 1:1 with I/O tasks, and each fetch task contains a mixture of computations and I/O. However, it is expected for this to significantly outperform master branch, which fetches everything recursively with only one thread. The logic is now a lot more linear and easy to follow. Everything that is embarassingly parallel is done on the thread pool, and then after everything is fetched, the worker threads are joined and the main thread does the finishing touches of stitching together the dependencies.zig import files. There is only one tiny little critical section and it does not even have any error handling in it. This also lays the groundwork for #14281 because in system mode, all this fetching logic will be skipped, but the "finishing touches" mentioned above still need to be done. With this branch, that logic is separated out and no longer recursively tangled with fetching stuff. Additionally, this branch: * Implements inclusion directives in `build.zig.zon` for deciding which files belong the package (#14311). * Adds basic documentation for `build.zig.zon` files. * Adds support for fetching dependencies with the `file://` protocol scheme (#17364). * Adds a workaround for a Linux/btrfs file system bug (#17282). This commit is a work-in-progress. Still todo: 1. Hook up the CLI to the new system. 2. Restore the module table creation logic after all the fetching is done. 3. Fix compilation errors, get the tests passing, and regression test against real world projects.

* start renaming "package" to "module" (see #14307) - build system gains `main_mod_path` and `main_pkg_path` is still there but it is deprecated. * eliminate the object-oriented memory management style of what was previously `*Package`. Now it is `*Package.Module` and all pointers point to externally managed memory. * fixes to get the new Fetch.zig code working. The previous commit was work-in-progress. There are still two commented out code paths, the one that leads to `Compilation.create` and the one for `zig build` that fetches the entire dependency tree and creates the required modules for the build runner.

see #14311

Finish the work started in 4c4fb839972f66f55aa44fc0aca5f80b0608c731. Now the compiler compiles again. Wire up dependency tree fetching code in the CLI for `zig build`. Everything is hooked up except for `createDependenciesModule` is not yet implemented.

For path-relative dependencies, they always need to be added to the hash table at the end. For remote dependencies, they never need to be added.

closes #17426

Only problem is that it looks like `has_build_zig` is being false when it should be true. After that is fixed then main.zig needs to create the `@dependencies` module from the generated source code.

* add Module instances for each package's build.zig and attach it to the dependencies.zig module with the hash digest hex string as the name. * fix incorrectly skipping the wrong packages for creating dependencies.zig * a couple more renaming of "package" to "module"

closes #14280

Since I'm about to rely on this behavior.

Instead of every file deletion being followed by a recursive attempt to remove the parent directory, while walking the directory, every directory that will have nonzero files deleted from it is tracked in an array hash map. After all the threads have finished deleting and hashing, the parent thread sorts the "suspicious" directories by length, descending, ensuring that children appear before parents, and then iterates over the array hash map, attempting a rmdir operation on each. Any rmdir that succeeds appends the parent directory to the map so that it will be removed if empty.

Related: ziglang/zig#17392 . Signed-off-by: Eric Joldasov <bratishkaerik@getgoogleoff.me>

Small updates for latest Zig updates: - ziglang/zig#17623 - ziglang/zig#17392 Tested with `zig build install` and `zig build test`

andrewrk mentioned this pull request Oct 4, 2023

fix: package hash bug #17094

Closed

scheibo reviewed Oct 4, 2023

View reviewed changes

andrewrk mentioned this pull request Oct 5, 2023

ability to fetch from zip files #17408

Closed

andrewrk force-pushed the fetch branch from 0bc416c to 833de10 Compare October 7, 2023 05:17

wooster0 reviewed Oct 7, 2023

View reviewed changes

andrewrk force-pushed the fetch branch from 2324d09 to 68127cd Compare October 7, 2023 18:37

andrewrk added 23 commits October 8, 2023 16:54

std.tar: add option for omitting empty directories

51e15a9

std.zig.ErrorBundle: add some explicit error sets

24c8adc

Manifest: add top-level paths field for inclusion rules

ef87729

See #14311

add basic build.zig.zon documentation

d06da95

require inclusion directives in root manifest but not deps

1c0d6f9

see #14311

fix Fetch.JobQueue.consolidateErrors

aed6adb

fix inverted logic for allowing/disallowing paths field

ddb7c40

fix recursive package fetching logic

1ca4428

For path-relative dependencies, they always need to be added to the hash table at the end. For remote dependencies, they never need to be added.

std: add unstable sorting to array hash maps

cbb9b5d

closes #17426

make Package.Path support string escape formatting

9eb2154

package fetching: generate dependencies.zig file

4eb7b61

Only problem is that it looks like `has_build_zig` is being false when it should be true. After that is fixed then main.zig needs to create the `@dependencies` module from the generated source code.

fix detection of build.zig file inside packages

e86b7fc

finish implementing the auto-generated dependencies.zig module

a232f7e

more fixes related to previous commits Package/Module API

35d81c9

std.tar: fix creation of symlinks with omit_empty_directories

7b25d05

dependencies.zig: omit modules without build.zig as deps

b7fc53c

dependencies.zig: omit deps without hashes

b2fefc8

zig build: add --fetch argument

a605bd9

closes #14280

CLI: fix only_core_functionality logic

29156de

andrewrk added 2 commits October 8, 2023 16:54

give build.zig modules access to their dependencies' build.zig modules

1ad33f5

use long-lived arena for @cImport-generated Module

f48ec4b

andrewrk force-pushed the fetch branch from 0cc8d6c to f48ec4b Compare October 8, 2023 23:54

andrewrk marked this pull request as ready for review October 8, 2023 23:55

andrewrk requested a review from kristoff-it as a code owner October 8, 2023 23:55

andrewrk added 11 commits October 8, 2023 17:21

Package.Fetch: fix not making directory paths for symlinks

f0cc6f1

zig fetch: require 'paths' field in the manifest

8f20ce7

fix dependencies.zig generation with no dependencies

7bae6d9

move some package management related source files around

6d84caf

Package.Fetch: tighten up check for path outside root

5e7c44a

fix using the wrong allocator for modules

1b372f1

std.fs.path.resolve: add test cases for empty string

efbfa8e

Since I'm about to rely on this behavior.

Package.Fetch: apply inclusion rules from build.zig.zon

4a2cf38

give modules friendly names for error reporting

5eb5d52

restore progress reporting for package fetching

95907cb

andrewrk mentioned this pull request Oct 9, 2023

update for newest build system API hexops/mach-core#55

Merged

1 task

andrewrk merged commit f7bc55c into master Oct 9, 2023
10 checks passed

andrewrk deleted the fetch branch October 9, 2023 18:47

andrewrk mentioned this pull request Oct 9, 2023

package fetching: elide errors from unpacking that relate to excluded file paths #17460

Closed

kassane mentioned this pull request Oct 9, 2023

Zig-pkg support dependabot/dependabot-core#8166

Open

1 task

This was referenced Oct 9, 2023

package fetching: detect and fail when symlinks point outside the package root #17461

Open

make packages fetched by zig set the executable bit based on ELF header and shebang line #17463

Closed

gentoo-bot pushed a commit to gentoo/guru that referenced this pull request Oct 10, 2023

dev-zig/zls: sync 9999 with latest Zig 9999 changes

5de3d47

Related: ziglang/zig#17392 . Signed-off-by: Eric Joldasov <bratishkaerik@getgoogleoff.me>

This was referenced Oct 11, 2023

Package fetching has regressed on Windows #17477

Closed

Incorrect file paths in DWARF debug info #17482

Closed

moderation mentioned this pull request Oct 24, 2023

Update for latest Zig mitchellh/libxev#72

Merged

mitchellh added a commit to mitchellh/libxev that referenced this pull request Oct 24, 2023

Update for latest Zig (#72)

5ecbc87

Small updates for latest Zig updates: - ziglang/zig#17623 - ziglang/zig#17392 Tested with `zig build install` and `zig build test`

kassane mentioned this pull request Nov 1, 2023

Add: Zig fetch support xmake-io/xmake#4348

Open

usdogu mentioned this pull request Nov 22, 2023

Zon package manager support extism/zig-pdk#18

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

rework package manager #17392

rework package manager #17392

andrewrk commented Oct 4, 2023 •

edited

Loading

xdBronch commented Oct 4, 2023

scheibo Oct 4, 2023

wooster0 Oct 7, 2023

andrewrk Oct 7, 2023

	@compileError("Cannot infer context " ++ @typeName(Context) ++ ", call sortContext instead.");
	@compileError("Cannot infer context " ++ @typeName(Context) ++ ", call sortUnstableContext instead.");

rework package manager #17392

rework package manager #17392

Conversation

andrewrk commented Oct 4, 2023 • edited Loading

Merge Checklist:

Follow-up issues

xdBronch commented Oct 4, 2023

scheibo Oct 4, 2023

Choose a reason for hiding this comment

wooster0 Oct 7, 2023

Choose a reason for hiding this comment

andrewrk Oct 7, 2023

Choose a reason for hiding this comment

andrewrk commented Oct 4, 2023 •

edited

Loading