From 02333e15cc36563b5478e3fb9adf7eed06db5403 Mon Sep 17 00:00:00 2001 From: Carl Gay Date: Wed, 21 Feb 2024 17:28:51 -0500 Subject: [PATCH 1/3] catalog: Don't check dev dependencies in validate-catalog See comment in diffs for details. Fixes #40 --- sources/pacman/catalog.dylan | 6 +++++- sources/pacman/deps-test.dylan | 16 +++++++++------- sources/pacman/deps.dylan | 34 +++++++++++++++------------------- 3 files changed, 29 insertions(+), 27 deletions(-) diff --git a/sources/pacman/catalog.dylan b/sources/pacman/catalog.dylan index b477689..73b390c 100644 --- a/sources/pacman/catalog.dylan +++ b/sources/pacman/catalog.dylan @@ -217,7 +217,11 @@ define function validate-catalog end; for (package in packages) for (release in package.package-releases) - resolve-release-deps(cat, release, dev?: #t, cache: cache); + // We pass `dev?: #f` here because it is valid for a package to have a dependency + // on itself if it is an active package _during development_. Since that catalog + // can't know what the active packages are we can't validate the dev dependencies + // and expect no errors. https://github.com/dylan-lang/pacman-catalog/issues/40 + resolve-release-deps(cat, release, dev?: #f, cache: cache); end; end; end function; diff --git a/sources/pacman/deps-test.dylan b/sources/pacman/deps-test.dylan index 8873815..b7ba4a5 100644 --- a/sources/pacman/deps-test.dylan +++ b/sources/pacman/deps-test.dylan @@ -102,13 +102,15 @@ define test test-dev-dependencies-not-transitive () end test; define test test-dev-dependency-conflict () - // Dev dependency on an incompatible version of another package. - assert-signals(, - make-test-catalog(#("testworks@1.0", #("strings@1.0"), #()), - // ok - #("strings@1.0", #(), #("testworks@1.0")), - // Should err because testworks wants strings@1.0. - #("strings@2.0", #(), #("testworks@1.0")))); + assert-no-errors + (make-test-catalog(#("testworks@1.0", #("strings@1.0"), #()), + #("strings@1.0", #(), #("testworks@1.0")), + // For main dependencies, strings@2.0 requiring testworks@1.0 + // would cause a dependency conflict because testworks@1.0 depends + // on strings@1.0. However, dev dependencies for the strings + // package are only used during ... development! So strings is one + // of the active packages, and validate-catalog doesn't check it. + #("strings@2.0", #(), #("testworks@1.0")))); // The two dev dependencies, bbb and ddd, have conflicting dependencies. assert-signals(, make-test-catalog(#("aaa@1.0", #(), #("bbb@2.0", "ddd@4.0")), diff --git a/sources/pacman/deps.dylan b/sources/pacman/deps.dylan index bbd3930..cc86346 100644 --- a/sources/pacman/deps.dylan +++ b/sources/pacman/deps.dylan @@ -97,13 +97,12 @@ define function resolve-release-deps resolve-deps(cat, deps, dev-deps, actives, cache: cache) end function; -// Resolve `release` to a set of releases it depends on, using `cat` as the world of -// potential releases. `release` itself is not included in the result. `active` maps -// package names to releases that are "active" in the current workspace and therefore -// should be treated specially. If any package encountered during resolution has a -// dependency on one of the active packages, that dependency is ignored since the active -// package will be used during the build process anyway. The returned deps do not include -// the active releases. +// Resolve a set of dependencies to a set of specific releases, using `cat` as the world +// of potential releases. `actives` maps package names to releases that are active in the +// current workspace and therefore should be treated specially. If any package +// encountered during resolution has a dependency on one of the active packages, that +// dependency is ignored since the active package will be used during the build process +// anyway. The set of returned releases does not include the active releases. // // Signal if dependencies can't be resolved due to circularities, // conflicting constraints, or if they are simply missing from the catalog. @@ -113,13 +112,13 @@ end function; // "providing repeatable builds by preferring the lowest possible specified version of // any package". // -// We are given a release that needs to be built. This is the root of a graph. Its deps -// form the second layer of a graph. The releases that match those deps form the third -// level of the graph, and the deps of those releases form the fourth, etc. So we have a -// graph in which the layers alternate between potential releases and their deps. This -// tree gets big fast. The result for any given release is memoized. +// We are given the deps of a release that needs to be built. That release is the root of +// a graph. Its deps form the second layer of a graph. The releases that match those deps +// form the third level of the graph, and the deps of those releases form the fourth, +// etc. So we have a graph in which the layers alternate between potential releases and +// their deps. This tree gets big fast. The result for any given release is memoized. // -// Each dep specifies only its minimum required version, e.g., P 1.2.3. These are +// Each dep specifies only its minimum required version, e.g., P@1.2.3. These are // semantic versions so if two dependencies on P specify different major versions it is // an error. // @@ -127,12 +126,9 @@ end function; // they are combined with other results to keep only the maximum minimum version for each // package. // -// (Note: We could support per-library dependencies (i.e., build deps). Test dependencies -// should not be included in the graph for the main library. For example, to build pacman -// testworks should not be a dependency. It's also possible to want to require D 1.0 for -// one library in a package and D 2.0 for a different library in the same package. I'm -// ignoring these issues for now to avoid unnecessary complexity. For now deps only work -// at the package level.) +// Note: The assumption is that deps are per package, not per library. That is, it's not +// possible for two independent libraries in one package to depend on different versions +// of a package. For that case, make two packages. define function resolve-deps (cat :: , deps :: , dev-deps :: , actives :: false-or(), #key cache) From 71fbc0328020eed605016b988b51242209067a95 Mon Sep 17 00:00:00 2001 From: Carl Gay Date: Sat, 23 Mar 2024 17:24:44 -0400 Subject: [PATCH 2/3] dylan-package.json: dev dep "testworks" without a specific version --- dylan-package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dylan-package.json b/dylan-package.json index 0e55b04..0b414c6 100644 --- a/dylan-package.json +++ b/dylan-package.json @@ -14,7 +14,7 @@ ], "dev-dependencies": [ "sphinx-extensions", - "testworks@2.0" + "testworks" ], "url": "https://github.com/dylan-lang/dylan-tool" } From 4d81e799bcdc4b8af963328b4b336d225367882f Mon Sep 17 00:00:00 2001 From: Carl Gay Date: Sat, 23 Mar 2024 17:25:55 -0400 Subject: [PATCH 3/3] workspaces: choose a reasonable default library name Choose a default library to build, test, or use as the project for lsp-dylan if no default is specified in workspace.json. --- sources/workspaces/registry.dylan | 18 +++++++++++++----- sources/workspaces/workspaces.dylan | 17 +++++++++++++---- 2 files changed, 26 insertions(+), 9 deletions(-) diff --git a/sources/workspaces/registry.dylan b/sources/workspaces/registry.dylan index c204b86..7458970 100644 --- a/sources/workspaces/registry.dylan +++ b/sources/workspaces/registry.dylan @@ -8,7 +8,6 @@ end; //// REGISTRY // Keys used to lookup values in a parsed LID file. -// TODO: use 'define enum' in uncommon-dylan define constant $platforms-key = #"platforms"; define constant $files-key = #"files"; define constant $library-key = #"library"; @@ -425,14 +424,23 @@ define function parse-lid-file lid end function; -define function find-library-names + +// Find the names of all libraries defined in `dir`, a directory or registry. +define generic find-library-names (dir) => (names :: ); + +define method find-library-names (dir :: ) => (names :: ) - let registry = make(, root-directory: dir); + find-library-names(make(, root-directory: dir)) +end method; + +define method find-library-names + (registry :: ) => (names :: ) // It's possible for a LID included via the LID: keyword to not have a library. remove(map(rcurry(lid-value, $library-key), - find-lids(registry, dir)), + find-lids(registry, registry.root-directory)), #f) -end function; +end method; + // Build a map from source file names (absolute pathname strings) to the names // of libraries they belong to (a sequence of strings). For now we only look at diff --git a/sources/workspaces/workspaces.dylan b/sources/workspaces/workspaces.dylan index 4f3c2a5..9d218d7 100644 --- a/sources/workspaces/workspaces.dylan +++ b/sources/workspaces/workspaces.dylan @@ -142,10 +142,19 @@ define function load-workspace let ws-json = ws-file & load-json-file(ws-file); let default-library = ws-json & element(ws-json, $default-library-key, default: #f); - if (~default-library & active-packages.size = 1) - // TODO: this isn't right. Should find the actual libraries, from the LID - // files, and if there's only one "*-test*" library, choose that. - default-library := pm/package-name(active-packages[0]); + if (~default-library) + let libs = find-library-names(registry); + if (~empty?(libs)) + local method match (suffix, lib) + ends-with?(lib, suffix) & lib + end; + // The assumption here is that (for small projects) there's usually one + // test library that you want to run. + default-library := (any?(curry(match, "-test-suite-app"), libs) + | any?(curry(match, "-test-suite"), libs) + | any?(curry(match, "-tests"), libs) + | libs[0]); + end; end; make(, active-packages: active-packages,