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

Merged
merged 3 commits into from
Jun 14, 2024

Conversation

bioball
Copy link
Contributor

@bioball bioball commented May 10, 2024

Implementation: apple/pkl#486

Thanks to @HT154 for the groundwork here!

bioball and others added 2 commits May 9, 2024 17:20
Co-authored-by: Josh B <421772+HT154@users.noreply.github.com>
)
----

=== 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`.
@bioball bioball merged commit b20d4a7 into apple:main Jun 14, 2024
@bioball bioball deleted the scheme-agnostic-projects branch June 18, 2024 18:44
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