Skip to content

Commit

Permalink
Merge pull request #84 from cgay/dev
Browse files Browse the repository at this point in the history
validate-catalog: Don't check dev dependencies
  • Loading branch information
cgay authored Mar 25, 2024
2 parents a56d40d + 4d81e79 commit f86bbe7
Show file tree
Hide file tree
Showing 6 changed files with 56 additions and 37 deletions.
2 changes: 1 addition & 1 deletion dylan-package.json
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
],
"dev-dependencies": [
"sphinx-extensions",
"testworks@2.0"
"testworks"
],
"url": "https://github.com/dylan-lang/dylan-tool"
}
6 changes: 5 additions & 1 deletion sources/pacman/catalog.dylan
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
16 changes: 9 additions & 7 deletions sources/pacman/deps-test.dylan
Original file line number Diff line number Diff line change
Expand Up @@ -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(<dep-error>,
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(<dep-error>,
make-test-catalog(#("aaa@1.0", #(), #("bbb@2.0", "ddd@4.0")),
Expand Down
34 changes: 15 additions & 19 deletions sources/pacman/deps.dylan
Original file line number Diff line number Diff line change
Expand Up @@ -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 <dep-error> if dependencies can't be resolved due to circularities,
// conflicting constraints, or if they are simply missing from the catalog.
Expand All @@ -113,26 +112,23 @@ 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.
//
// Releases with no deps terminate the recursion and as results percolate up the stack
// 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 :: <catalog>, deps :: <dep-vector>, dev-deps :: <dep-vector>,
actives :: false-or(<istring-table>), #key cache)
Expand Down
18 changes: 13 additions & 5 deletions sources/workspaces/registry.dylan
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down Expand Up @@ -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 :: <seq>);

define method find-library-names
(dir :: <directory-locator>) => (names :: <seq>)
let registry = make(<registry>, root-directory: dir);
find-library-names(make(<registry>, root-directory: dir))
end method;

define method find-library-names
(registry :: <registry>) => (names :: <seq>)
// 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
Expand Down
17 changes: 13 additions & 4 deletions sources/workspaces/workspaces.dylan
Original file line number Diff line number Diff line change
Expand Up @@ -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(<workspace>,
active-packages: active-packages,
Expand Down

0 comments on commit f86bbe7

Please sign in to comment.