Register services in `package.yaml` instead via `ServiceLoader` #11868

JaroslavTulach · 2024-12-13T16:42:59Z

Pull Request Description

Designs a fix for #11724. package.yaml is extended with

- services
  - provides: SPI_Type_Name
    with: Implementation_Type_Name

and a new method lookup_services will read all such info and load the implementations just like demonstrated in MetaServicesTest.

Important Notes

Checklist

Please ensure that the following checklist has been satisfied before submitting the PR:

The documentation has been updated, if necessary.
Screenshots/screencasts have been attached, if there are any visual changes. For interactive or animated visual changes, a screencast is preferred.
All code follows the
Scala,
Java,
TypeScript,
and
Rust
style guides. In case you are using a language not listed above, follow the Rust style guide.
Unit tests have been written where possible.
If meaningful changes were made to logic or tests affecting Enso Cloud integration in the libraries,
or the Snowflake database integration, a run of the Extra Tests has been scheduled.
- If applicable, it is suggested to paste a link to a successful run of the Extra Tests.

JaroslavTulach

@radeusgd, @jdunkerley & others, can you please review this concept for service registration? If it is found sound, I provide proper implementation of the lookup_services builtin.

JaroslavTulach · 2024-12-13T16:46:07Z

distribution/lib/Standard/Base/0.0.0-dev/src/System/File.enso

@@ -924,3 +919,16 @@ local_file_move (source : File) (destination : File) (replace_existing : Boolean
    File_Error.handle_java_exceptions source <|
        copy_options = if replace_existing then [StandardCopyOption.REPLACE_EXISTING.to_text] else []
        source.move_builtin destination copy_options
+
+# PRIVATE
+type File_System_SPI


First step, @radeusgd, is to define an SPI type. It can contain any information needed to process the registered instances. In this case it is protocol name. For purposes of simplicity the other argument is typ - to keep the implementation compatible, but one could do better, probably.

JaroslavTulach · 2024-12-13T16:47:24Z

distribution/lib/Standard/Base/0.0.0-dev/src/System/File.enso

+        if vec . not_empty then vec.first else Nothing
+
+    private get_types _:Boolean =
+        Meta.lookup_services File_System_SPI


To look the implementations up one will be allowed to use lookup_services method and specify the SPI type one is interested in. The behavior of the lookup_services method will be similar to the operations shown in the MetaServicesTest.

JaroslavTulach · 2024-12-13T16:49:03Z

distribution/lib/Standard/Base/0.0.0-dev/package.yaml

@@ -29,3 +29,7 @@ component-groups:
    - Logical: { color: oklch(0.464 0.14 300) }
    - Operators: { color: oklch(0.464 0.14 275) }
    - Errors: { color: oklch(0.464 0.14 15) }
+
+services:


To register an implementation one adds services section that specifies pair of SPI type and implementation type.

@jdunkerley: If you want the IDE to show a drop box with all available providers sooner than the library is loaded, then we might need (localized) display name explaining why such a library should be imported. Engine+ls would deliver list of all possible implementations to the IDE via suggestion db or etc.

James would like Meta.lookup_services to be able to return even the not enabled services. Then one can create a drop down with a selection of all possible implementation. Question: what happens when a not enabled service is selected? How does one turn that library on?

Shall Meta provide something like Meta.is_library_imported? Then if not, we could give some feedback to the language server and ask it to insert a missing import.

feedback to the language server

CCing @4e6.

how exactly? widgets don't have a way to talk to the language server

there is Allow modification of source code from inside of Enso #8011

with such a feature widgets would have a way to modify the code

but it is a way bigger task than what Register services in package.yaml instead via ServiceLoader #11868 originally tend to achieve

E.g. if we continue to insist on solving the problem with not enabled services we effectively put this PR on hold.

JaroslavTulach · 2024-12-13T16:54:26Z

distribution/lib/Standard/Base/0.0.0-dev/src/Enso_Cloud/Enso_File_System_Impl.enso

+import project.System.File.File_System_SPI
+import project.Enso_Cloud.Enso_File
+
+type Enso_File_System_Impl


Registration of implementation can be done in a private module by defining a singleton type (which is its own type) and a conversion from such type to the SPI interface. In this case the conversion is using File_System_SPI.new factory method.

Akirathan

What is the reason to keep org.enso.base.file_system.FileSystemSPI? Why not migrate other service implementations in this PR as well?

Akirathan · 2024-12-16T10:24:57Z

lib/scala/pkg/src/main/scala/org/enso/pkg/Config.scala

+  * @param provides name of SPI type
+  * @param with name of implementation type
+  */
+case class ProvidesWith(val provides: String, val `with`: String) {}


Can we change String to org.enso.pkg.QualifiedName?

Akirathan · 2024-12-16T10:29:28Z

distribution/lib/Standard/Base/0.0.0-dev/src/Enso_Cloud/Enso_File_System_Impl.enso

+
+type Enso_File_System_Impl
+
+File_System_SPI.from (_:Enso_File_System_Impl) =


To ensure this works, you should remove EnsoPathFileSystemImpl.java?

lib/scala/pkg/src/main/scala/org/enso/pkg/Config.scala

Co-authored-by: Hubert Plociniczak <[email protected]>

distribution/lib/Standard/Base/0.0.0-dev/src/System/File.enso

Register services in package.yaml instead via ServiceLoader

0777b32

JaroslavTulach self-assigned this Dec 13, 2024

JaroslavTulach requested review from jdunkerley, radeusgd, GregoryTravis, AdRiley, marthasharkey, 4e6, hubertp and Akirathan as code owners December 13, 2024 16:43

GregoryTravis approved these changes Dec 13, 2024

View reviewed changes

Really search for ProvidesWith types

8e3b3e2

JaroslavTulach commented Dec 13, 2024

View reviewed changes

enso-bot bot mentioned this pull request Dec 14, 2024

Pattern matching a EnsoMultiValue ignores 2nd & co. types #11828

Closed

JaroslavTulach changed the title ~~Register services in package.yaml instead via ServiceLoader~~ Register services in package.yaml instead via ServiceLoader Dec 16, 2024

JaroslavTulach linked an issue Dec 16, 2024 that may be closed by this pull request

Enso facility for finding implementations of SPI across loaded/available libraries #11724

Open

This was referenced Dec 16, 2024

Encapsulation of polyglot/java Dependencies #7082

Open

Package libraries Java code as Java Modules #10714

Draft

Akirathan reviewed Dec 16, 2024

View reviewed changes

hubertp approved these changes Dec 16, 2024

View reviewed changes

lib/scala/pkg/src/main/scala/org/enso/pkg/Config.scala Outdated Show resolved Hide resolved

lib/scala/pkg/src/main/scala/org/enso/pkg/Config.scala Outdated Show resolved Hide resolved

JaroslavTulach and others added 2 commits December 17, 2024 08:09

Missing with field

5b7fd35

Co-authored-by: Hubert Plociniczak <[email protected]>

Missing provides field

f71de2e

Co-authored-by: Hubert Plociniczak <[email protected]>

radeusgd reviewed Dec 31, 2024

View reviewed changes

distribution/lib/Standard/Base/0.0.0-dev/src/System/File.enso Show resolved Hide resolved

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Register services in `package.yaml` instead via `ServiceLoader` #11868

Register services in `package.yaml` instead via `ServiceLoader` #11868

JaroslavTulach commented Dec 13, 2024

JaroslavTulach left a comment •

edited

Loading

JaroslavTulach Dec 13, 2024

JaroslavTulach Dec 13, 2024

JaroslavTulach Dec 13, 2024

JaroslavTulach Dec 17, 2024 •

edited

Loading

radeusgd Dec 31, 2024

JaroslavTulach Jan 4, 2025 •

edited

Loading

JaroslavTulach Dec 13, 2024

Akirathan left a comment

Akirathan Dec 16, 2024

Akirathan Dec 16, 2024


		type Enso_File_System_Impl

		File_System_SPI.from (_:Enso_File_System_Impl) =

Register services in package.yaml instead via ServiceLoader #11868

Are you sure you want to change the base?

Register services in package.yaml instead via ServiceLoader #11868

Conversation

JaroslavTulach commented Dec 13, 2024

Pull Request Description

Important Notes

Checklist

JaroslavTulach left a comment • edited Loading

Choose a reason for hiding this comment

JaroslavTulach Dec 13, 2024

Choose a reason for hiding this comment

JaroslavTulach Dec 13, 2024

Choose a reason for hiding this comment

JaroslavTulach Dec 13, 2024

Choose a reason for hiding this comment

JaroslavTulach Dec 17, 2024 • edited Loading

Choose a reason for hiding this comment

radeusgd Dec 31, 2024

Choose a reason for hiding this comment

JaroslavTulach Jan 4, 2025 • edited Loading

Choose a reason for hiding this comment

JaroslavTulach Dec 13, 2024

Choose a reason for hiding this comment

Akirathan left a comment

Choose a reason for hiding this comment

Akirathan Dec 16, 2024

Choose a reason for hiding this comment

Akirathan Dec 16, 2024

Choose a reason for hiding this comment

Register services in `package.yaml` instead via `ServiceLoader` #11868

Register services in `package.yaml` instead via `ServiceLoader` #11868

JaroslavTulach left a comment •

edited

Loading

JaroslavTulach Dec 17, 2024 •

edited

Loading

JaroslavTulach Jan 4, 2025 •

edited

Loading