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.

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

SPICE-0005: Scheme-agnostic Projects #6

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

bioball
Copy link
Contributor

@bioball bioball commented May 10, 2024

Implementation: apple/pkl#486

Thanks to @HT154 for the groundwork here!

)
----

=== Local Dependencies

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can I have non-file local dependency in my PklProject? Is not clear for me:

["fruit"] = import("modulepath:/fruit/PklProject")

Or as these changes just usable in the evaluator API, but not in the CLI?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will add some details here; local dependencies must be resolvable as relative path. This means same scheme, and same authority.

Currently, paths that are prefixed with `@` _only_ have special meaning if declared within file-based, or package-based modules.
In these modules, this prefix is the marker of link:https://pkl-lang.org/main/current/language-reference/index.html#dependency-notation-uris[dependency notation].
In all other modules, this symbol has no special meaning.
For example, in module `modulepath:/foo.pkl`, the import `@bar/bar.pkl` is resolved as `modulepath:/@bar/bar.pkl`.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it would be good to make an addendum here for modulepath:, as it's a special case because of multiple roots. Show an example using modulepath X, project root Y (with multiple roots) and import @Z, how would that be resolved.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(summary from offline discussion)

This SPICE doesn't provide any changes to how modulepath works. With modulepath, local dependency, Z at path modulepath:/foo/bar, import "@Z/baz.pkl resolves to import modulepath:/foo/bar/baz.pkl. The meaning of modulepath:/foo/bar/baz.pkl is up to the modulepath resolver.

1. Instead of a "project directory", a project is identified by a base URI.
2. Relative paths starting with `@` are _always_ treated as link:https://pkl-lang.org/main/current/language-reference/index.html#dependency-notation-uris[dependency notation], in all modules.

== Detailed design

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm missing a table/list/etc detailing for a project defined using module key X what are the allowed module keys inside the dependencies.
I think right now if you import @foo/bar.pkl then bar.pkl can only import other projects (using @ or package://) or relative imports inside the project. If I now have a project defined with modulepath can I have modulepath: imports inside its dependencies?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Summary from offline discussion: Projects/packages don't remove things that can be imported, they can only add to what can be imported. Even normal projects whose project dir is file: based can have an import modulepath:/ within its sources, and it still works today.

bioball added a commit to apple/pkl that referenced this pull request Jun 4, 2024
This adds changes to support loading project dependencies in non-file based projects.

The design for this feature can be found in SPICE-0005: apple/pkl-evolution#6

Changes:
* Consider all imports prefixed with `@` as dependency notation.
* Bugfix: fix resolution of glob expressions in a local dependency.
* Adjust pkl.Project:
  - Allow local dependencies from a scheme-local paths.
  - Disallow certain evaluator settings if not loaded as a file-based module.
* Breaking API change: `ProjectDependenciesManager` constructor now requires `ModuleResolver` and `SecurityManager`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants