Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

support fetching dependencies with the file:// protocol scheme #17364

Closed
andrewrk opened this issue Oct 2, 2023 · 2 comments
Closed

support fetching dependencies with the file:// protocol scheme #17364

andrewrk opened this issue Oct 2, 2023 · 2 comments
Assignees
@andrewrk
Labels
accepted This proposal is planned. contributor friendly This issue is limited in scope and/or knowledge of Zig internals. enhancement Solving this issue will likely involve adding new logic or components to the codebase. proposal This issue suggests modifications. If it also has the "accepted" label then it is planned. zig build system
Milestone
0.12.0

Comments

@andrewrk
Copy link
Member

andrewrk commented Oct 2, 2023 •
edited

This should work:

zig fetch file://C:/Code/myproject/

and of course the equivalent build.zig.zon snippet:

    .url = "file://C:/Code/myproject/"

Support file:// just like any other URL scheme.

URLs already can be private or require a proxy, or blocked in some countries. File paths have the exact same properties. There is no reason to not support this kind of URL.

If someone has a security concern about this and wants to be taken seriously then please explain the threat model and let's analyze it.

The previous issue for this feature was #14339 however, that was two separate use cases, and the resulting enhancement that came out of that was paths relative to the build root which are exempt from hashing. The use case for supporting file:/// still remains and is still planned.
@andrewrk andrewrk added enhancement Solving this issue will likely involve adding new logic or components to the codebase. contributor friendly This issue is limited in scope and/or knowledge of Zig internals. proposal This issue suggests modifications. If it also has the "accepted" label then it is planned. accepted This proposal is planned. zig build system labels Oct 2, 2023
@andrewrk andrewrk added this to the 0.13.0 milestone Oct 2, 2023
This was referenced Oct 2, 2023
Closed
Merged
@nektro
Copy link
Contributor

nektro commented Oct 2, 2023

small nit: it should be the file:// protocol, it commonly has 3 slashes because on non-windows systems absolute paths start with /

@andrewrk andrewrk self-assigned this Oct 4, 2023
@andrewrk andrewrk modified the milestones: 0.13.0, 0.12.0 Oct 4, 2023
@andrewrk andrewrk changed the title support fetching dependencies with the file:/// protocol scheme support fetching dependencies with the file:// protocol scheme Oct 4, 2023
andrewrk added a commit that referenced this issue Oct 4, 2023
@andrewrk
rework package manager
0bc416c 
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.
@andrewrk andrewrk mentioned this issue Oct 4, 2023
Merged
11 tasks
andrewrk added a commit that referenced this issue Oct 7, 2023
@andrewrk
rework package manager
187ffb8 
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.
andrewrk added a commit that referenced this issue Oct 8, 2023
@andrewrk
rework package manager
88bbec8 
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.
@andrewrk
Copy link
Member Author

andrewrk commented Oct 9, 2023

Landed in f7bc55c.

@andrewrk andrewrk closed this as completed Oct 9, 2023
@iacore iacore mentioned this issue Oct 12, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@andrewrk andrewrk

Labels
accepted This proposal is planned. contributor friendly This issue is limited in scope and/or knowledge of Zig internals. enhancement Solving this issue will likely involve adding new logic or components to the codebase. proposal This issue suggests modifications. If it also has the "accepted" label then it is planned. zig build system
Projects
None yet
Milestone
0.12.0
Development

No branches or pull requests
2 participants
@andrewrk @nektro