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

Change layout of local library search path in order to be able to move Round_Spec.enso back to Tests #7634

Merged
merged 30 commits into from
Sep 1, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
7bdda0f
Move Round_Spec.enso back to `Tests`, try linking Tests from Table_Tests
radeusgd Aug 22, 2023
420b311
revert to default edition, but allow local lib resolution
radeusgd Aug 24, 2023
f4f1d9c
flatten library search structure and use package.yaml instead
radeusgd Aug 24, 2023
e0ff029
WIP: passing project root to LibraryLocations so that we can resolve …
radeusgd Aug 24, 2023
e3c51a9
pt2. allowing resolving projects next to current project as libraries
radeusgd Aug 28, 2023
cf2cbd0
Add tests for new semantics of local lib search
radeusgd Aug 28, 2023
83728ee
fix tests
radeusgd Aug 28, 2023
7771d4a
remove debug(?) log
radeusgd Aug 28, 2023
dc64dbf
add (failing) test for integration between LocalLibraryProvider and L…
radeusgd Aug 28, 2023
c297c83
fix that test
radeusgd Aug 28, 2023
25ddd88
formatting
radeusgd Aug 28, 2023
e810382
changelog
radeusgd Aug 28, 2023
b5bf679
update docs
radeusgd Aug 28, 2023
24ba65e
cleanup
radeusgd Aug 28, 2023
385dd4d
Merge branch 'develop' into wip/radeusgd/test-suite-dependencies
radeusgd Aug 31, 2023
f2407da
Merge branch 'develop' into wip/radeusgd/test-suite-dependencies
radeusgd Aug 31, 2023
f472f7b
prettier: changelog
radeusgd Aug 31, 2023
5488afd
prettier: docs
radeusgd Aug 31, 2023
dffd99c
Merge branch 'develop' into wip/radeusgd/test-suite-dependencies
radeusgd Aug 31, 2023
eb3933c
fixing LibrariesTests; avoid NoSuchFile exception
radeusgd Aug 31, 2023
ab524be
try aligning local library discovery
radeusgd Aug 31, 2023
93695fc
Merge branch 'develop' into wip/radeusgd/test-suite-dependencies
radeusgd Aug 31, 2023
becb9ec
fix libraries test
radeusgd Aug 31, 2023
a44aadb
make test more robust to side effects (other tests were creating othe…
radeusgd Sep 1, 2023
5868b2e
Merge branch 'develop' into wip/radeusgd/test-suite-dependencies
radeusgd Sep 1, 2023
40af6a1
fix other test (missing implementation)
radeusgd Sep 1, 2023
05bf37d
better diagnostic info on conflicts
radeusgd Sep 1, 2023
fe427bf
align package name with its true name, avoiding a naming conflict
radeusgd Sep 1, 2023
a56b744
warn only once about missing path
radeusgd Sep 1, 2023
06be852
not fail tests on cleanup
radeusgd Sep 1, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -936,6 +936,8 @@
- [Allow Java Enums in case of branches][7607]
- [Notification about the project rename action][7613]
- [Use `numpy` & co. from Enso!][7678]
- [Changed layout of local libraries directory, making it easier to reference
projects next to each other][7634]

[3227]: https://github.com/enso-org/enso/pull/3227
[3248]: https://github.com/enso-org/enso/pull/3248
Expand Down Expand Up @@ -1071,6 +1073,7 @@
[7607]: https://github.com/enso-org/enso/pull/7607
[7613]: https://github.com/enso-org/enso/pull/7613
[7678]: https://github.com/enso-org/enso/pull/7678
[7634]: https://github.com/enso-org/enso/pull/7634

# Enso 2.0.0-alpha.18 (2021-10-12)

Expand Down
54 changes: 33 additions & 21 deletions docs/libraries/editions.md
Original file line number Diff line number Diff line change
Expand Up @@ -189,36 +189,48 @@ so edition files that already exist on disk are not redownloaded.
Below are listed the steps that are taken when resolving an import of library
`Foo.Bar`:

1. If and only if the project has `prefer-local-libraries` set to `true` and if
any directory on the library path contains `Foo/Bar`, that local instance is
chosen as the library that should be used, regardless of the version that is
there;
1. If and only if the project has `prefer-local-libraries` set to `true`, the
library path is searched for sub-directories containing Enso packages. If any
of such packages has a `package.yaml` that defines `namespace:Foo` and
`name: Bar`, that local instance of the library is chosen. In this particular
scenario the version check is skipped - whatever version is present in the
local library path is used.
2. Otherwise, the list of libraries defined directly in the `edition` section of
`package.yaml` of the current project is checked, and if the library is
defined there, it is selected.
3. Otherwise, any parent editions are consulted; if they too do not contain the
library that we are searching for, an error is reported.
4. Once we know the library version to be used:
1. If the repository associated with the library is `local`, the library path
is searched for the first directory to contain `Foo/Bar` and this path is
loaded. If the library is not present on the library path, an error is
reported.
1. If the repository associated with the library is `local`, the local
library path is searched for the first directory to contain the requested
library and this path is loaded. If the library is not present on the
library path, an error is reported.
2. Otherwise, the edition must have defined an exact `<version>` of the
library that is supposed to be used.
3. If the library is already downloaded in the local repository cache, that
is the directory `$ENSO_DATA_DIRECTORY/lib/Foo/Bar/<version>` exists, that
3. If the library is already downloaded in the local repository cache (the
directory `$ENSO_DATA_DIRECTORY/lib/Foo/Bar/<version>` exists), that
package is loaded.
4. Otherwise, the library is missing and must be downloaded from its
associated repository (and placed in the cache as above).

By default, the library path is `<ENSO_HOME>/libraries/` but it can be
overridden by setting the `ENSO_LIBRARY_PATH` environment variable. It may
include a list of directories (separated by the system specific path separator);
the first directory on the list has the highest precedence.

In particular, if `prefer-local-libraries` is `false`, and the edition does not
define a library at all, when trying to resolve such a library, it is reported
as not found even if a local version of it exists. That is because
auto-discovery of local libraries is only done with `prefer-local-libraries` set
to `true`. In all other cases, the `local` repository overrides should be set
explicitly.
By default, the local library path consists of two directories:

- `<ENSO_HOME>/libraries/`,
- the parent directory of the currently opened project.

This allows the user to access libraries that are placed next to the current
project (although ones located in the Enso home still take precedence). Still,
to access local libraries they either have to be defined in the edition, or the
`prefer-local-libraries` flag must be set to `true`.

The local library search path can be overridden by setting the
`ENSO_LIBRARY_PATH` environment variable. It may include a list of directories
(separated by the system specific path separator); the first directory on the
list has the highest precedence. If the environment variable is defined, it
overrides the default paths.

If `prefer-local-libraries` is `false`, and the edition does not define a
library at all, when trying to resolve such a library, it is reported as not
found even if a local version of it exists. That is because auto-discovery of
local libraries is only done with `prefer-local-libraries` set to `true`. In all
other cases, the `local` repository overrides should be set explicitly.
12 changes: 2 additions & 10 deletions docs/libraries/sharing.md
Original file line number Diff line number Diff line change
Expand Up @@ -23,19 +23,11 @@ To prepare the project for sharing, make sure that it has a proper `namespace`
field set in `package.yaml`. It should be set to something unique, like your
username.

> **NOTE**: The field `namespace` is a temporary workaround and in the near
> future it will be deprecated and handled mostly automatically. For now however
> you need to set it properly.

To share an Enso library, all you need to do is to package the project into an
archive (for example ZIP) and share it (through e-mail, cloud drive services
etc.) with your peers. Now to be able to use the library that was shared with
you, you need to extract it to the directory
`~/enso/libraries/<namespace>/<Project_Name>` (where on Windows `~` should be
interpreted as your user home directory). To make sure that the library is
extracted correctly, make sure that under the path
`~/enso/libraries/<namespace>/<Project_Name>/package.yaml` and that its
`namespace` field has the same value as the name of the `<namespace>` directory.
you, you need to extract it to the directory `~/enso/libraries/<Project_Name>`
(where on Windows `~` should be interpreted as your user home directory).

Now you need to set up your project properly to be able to use this unpublished
library. The simplest way to do that is to set `prefer-local-libraries` in your
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -356,14 +356,18 @@ class MainModule(serverConfig: LanguageServerConfig, logLevel: LogLevel) {
"project-settings-manager"
)

val libraryLocations =
LibraryLocations.resolve(
distributionManager,
Some(languageHome),
Some(contentRoot.file.toPath)
)

val localLibraryManager = system.actorOf(
LocalLibraryManager.props(contentRoot.file, distributionManager),
LocalLibraryManager.props(contentRoot.file, libraryLocations),
"local-library-manager"
)

val libraryLocations =
LibraryLocations.resolve(distributionManager, Some(languageHome))

val libraryConfig = LibraryConfig(
localLibraryManager = localLibraryManager,
editionReferenceResolver = editionReferenceResolver,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import scala.util.Try

/** Resolves [[EditionReference]] to a raw or resolved edition. */
class EditionReferenceResolver(
projectRoot: File,
val projectRoot: File,
editionProvider: EditionProvider,
editionResolver: EditionResolver
) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,31 +2,28 @@ package org.enso.languageserver.libraries

import akka.actor.Props
import com.typesafe.scalalogging.LazyLogging
import org.enso.distribution.{DistributionManager, FileSystem}
import org.enso.distribution.FileSystem
import org.enso.editions.{Editions, LibraryName}
import org.enso.languageserver.libraries.LocalLibraryManagerProtocol._
import org.enso.librarymanager.local.{
DefaultLocalLibraryProvider,
LocalLibraryProvider
}
import org.enso.librarymanager.LibraryLocations
import org.enso.librarymanager.local.DefaultLocalLibraryProvider
import org.enso.librarymanager.published.repository.LibraryManifest
import org.enso.pkg.validation.NameValidation
import org.enso.pkg.{Config, Contact, Package, PackageManager}
import org.enso.yaml.YamlHelper

import java.io.File
import java.nio.file.{Files, Path}

import scala.util.{Success, Try}

/** An Actor that manages local libraries. */
class LocalLibraryManager(
currentProjectRoot: File,
distributionManager: DistributionManager
libraryLocations: LibraryLocations
) extends BlockingSynchronizedRequestHandler
with LazyLogging {
val localLibraryProvider = new DefaultLocalLibraryProvider(
distributionManager.paths.localLibrariesSearchPaths.toList
libraryLocations.localLibrarySearchPaths
)

override def requestStage: Receive = { case request: Request =>
Expand Down Expand Up @@ -77,7 +74,7 @@ class LocalLibraryManager(

// TODO [RW] make the exceptions more relevant
val possibleRoots = LazyList
.from(distributionManager.paths.localLibrariesSearchPaths)
.from(libraryLocations.localLibrarySearchPaths)
.filter { path =>
Try { if (Files.notExists(path)) Files.createDirectories(path) }
Files.isWritable(path)
Expand All @@ -88,9 +85,10 @@ class LocalLibraryManager(
)
}

val libraryPath =
LocalLibraryProvider.resolveLibraryPath(librariesRoot, libraryName)
val libraryPath = librariesRoot.resolve(libraryName.name)
if (Files.exists(libraryPath)) {
// TODO [RW] we could try finding alternative names (as directory name does not matter for local libraries), to find a free name
// This can be done as part of #1877
throw new RuntimeException("Local library already exists")
}
Comment on lines 89 to 93
Copy link
Member Author

Choose a reason for hiding this comment

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

The issue #1877 was closed some time ago as part of a cleanup, but it indeed remains unsolved - there are a few TODOs referencing it and indeed the error handling in that part is not fully developed but a shortcut.

It may be worth to re-open the issue. However, this part of the API is not yet being used by the IDE. I guess we may wait until it starts being used to re-visit these improvements. But let's keep in mind there is some missing functionality there that should be filled-in once this is actually used.


Expand Down Expand Up @@ -121,17 +119,7 @@ class LocalLibraryManager(
} yield ListLocalLibrariesResponse(libraryEntries)

private def findLocalLibraries(): Try[Seq[LibraryName]] = Try {
for {
searchPathRoot <- distributionManager.paths.localLibrariesSearchPaths
namespaceDir <- FileSystem
.listDirectory(searchPathRoot)
.filter(Files.isDirectory(_))
nameDir <- FileSystem
.listDirectory(namespaceDir)
.filter(Files.isDirectory(_))
namespace = namespaceDir.getFileName.toString
name = nameDir.getFileName.toString
} yield LibraryName(namespace, name)
localLibraryProvider.findAvailableLocalLibraries()
}

/** Finds the path on the filesystem to a local library. */
Expand Down Expand Up @@ -252,8 +240,8 @@ class LocalLibraryManager(
object LocalLibraryManager {
def props(
currentProjectRoot: File,
distributionManager: DistributionManager
libraryLocations: LibraryLocations
): Props = Props(
new LocalLibraryManager(currentProjectRoot, distributionManager)
new LocalLibraryManager(currentProjectRoot, libraryLocations)
)
}
Original file line number Diff line number Diff line change
Expand Up @@ -168,15 +168,15 @@ class LibraryPreinstallHandler(
case Left(error) =>
val errorMessage = error match {
case InternalError(throwable) =>
FileSystemError(s"Internal error: ${throwable.getMessage}")
FileSystemError(s"Internal error: ${throwable.toString}")
case DependencyGatheringError(throwable) =>
DependencyDiscoveryError(throwable.getMessage)
DependencyDiscoveryError(throwable.toString)
case InstallerError(Error.NotResolved(_)) =>
LibraryNotResolved(libraryName)
case InstallerError(Error.RequestedLocalLibraryDoesNotExist) =>
LocalLibraryNotFound(libraryName)
case InstallerError(Error.DownloadFailed(version, reason)) =>
LibraryDownloadError(libraryName, version, reason.getMessage)
LibraryDownloadError(libraryName, version, reason.toString)
}
replyTo ! ResponseError(
Some(requestId),
Expand Down Expand Up @@ -216,7 +216,8 @@ class LibraryPreinstallHandler(
progressReporter = notificationReporter,
languageHome = config.installerConfig.languageHome,
edition = edition,
preferLocalLibraries = preferLocalLibraries
preferLocalLibraries = preferLocalLibraries,
projectRoot = Some(editionReferenceResolver.projectRoot.toPath)
)
dependencyResolver = new DependencyResolver(
localLibraryProvider = config.localLibraryProvider,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,74 @@
package org.enso.languageserver.libraries

import akka.actor.ActorSystem
import akka.testkit._
import org.enso.distribution.FileSystem.PathSyntax
import org.enso.editions.LibraryName
import org.enso.librarymanager.LibraryLocations
import org.enso.pkg.PackageManager
import org.enso.testkit.WithTemporaryDirectory
import org.scalatest.matchers.should.Matchers
import org.scalatest.wordspec.AnyWordSpecLike
import org.scalatest.BeforeAndAfterAll
import org.scalatest.time.SpanSugar.convertIntToGrainOfTime

import scala.concurrent.duration.FiniteDuration

class LocalLibraryManagerSpec
extends TestKit(ActorSystem("TestSystem"))
with ImplicitSender
with AnyWordSpecLike
with Matchers
with BeforeAndAfterAll
with WithTemporaryDirectory {

val Timeout: FiniteDuration = 10.seconds

override def afterAll(): Unit = {
TestKit.shutdownActorSystem(system)
}

"LocalLibraryManager" should {
"find the libraries it has itself created" in {
Copy link
Member Author

Choose a reason for hiding this comment

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

This test is partially redundant with

"create a library project and include it on the list of local projects" in {
but it checks some more cases and is more a 'unit test' whereas the other is more an 'integration test'.

So IMO it may make sense to keep both - the role of the newly added one is to ensure the logic for creating a library is kept in-sync with the logic for resolving them. The other test is just an integration test checking library management.

val projectRoot = getTestDirectory / "project-root"
PackageManager.Default.create(projectRoot.toFile, "Test_Project_123")
val localLibraryRoot = getTestDirectory / "local-library-root"
val libraryLocations = LibraryLocations(
List(localLibraryRoot),
getTestDirectory / "library-cache-root",
List()
)
val manager =
system.actorOf(
LocalLibraryManager.props(projectRoot.toFile, libraryLocations)
)

val myLibraryName = LibraryName("user456", "My_Library")

manager ! LocalLibraryManagerProtocol.Create(
myLibraryName,
Seq(),
Seq(),
"CC0"
)
expectMsg(Timeout, LocalLibraryManagerProtocol.EmptyResponse())

manager ! LocalLibraryManagerProtocol.FindLibrary(myLibraryName)
expectMsgPF(Timeout, "FindLibraryResponse") {
case LocalLibraryManagerProtocol.FindLibraryResponse(Some(root)) =>
assert(root.location.startsWith(localLibraryRoot))
}

manager ! LocalLibraryManagerProtocol.ListLocalLibraries
val foundLibraries = expectMsgPF(Timeout, "ListLocalLibrariesResponse") {
case LocalLibraryManagerProtocol.ListLocalLibrariesResponse(
libraries
) =>
libraries
}
foundLibraries.map(entry =>
LibraryName(entry.namespace, entry.name)
) should contain theSameElementsAs Seq(myLibraryName)
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -297,16 +297,20 @@ class BaseServerTest
)
)

val libraryLocations =
LibraryLocations.resolve(
distributionManager,
Some(languageHome),
Some(config.projectContentRoot.file.toPath)
)

val localLibraryManager = system.actorOf(
LocalLibraryManager.props(
config.projectContentRoot.file,
distributionManager
libraryLocations
)
)

val libraryLocations =
LibraryLocations.resolve(distributionManager, Some(languageHome))

val libraryConfig = LibraryConfig(
localLibraryManager = localLibraryManager,
editionReferenceResolver = editionReferenceResolver,
Expand Down
Loading