Support for non-constraint versions (#73)

- also fixed some issues with local update/fetch
- more integration tests
- removed some dead testdata
- included corral-test-repo as a submodule

.gitmodules

@@ -0,0 +1,3 @@
+[submodule "corral/test/testdata/corral-test-repo"]
+	path = corral/test/testdata/corral-test-repo
+	url = https://github.com/ponylang/corral-test-repo

corral/bundle/bundle.pony

@@ -73,7 +73,7 @@ class Bundle
     ld.locator = locator
     ld.revision = revision
     let dep = Dep(this, dd, ld)
-    deps(dd.locator) = dep
+    deps(locator) = dep
     modified = true
     dep
 

corral/bundle/dep.pony

@@ -25,9 +25,8 @@ class val Locator is (su.ComparableMixin[Locator] & Hashable & Stringable)
         rp = parts(0)?
         vs = s
         let p = parts(1)?
-        bp = if (p.size() > 0) and (p(0)? == '/') then p.trim(1) else p end
-        // TODO: strip any leading scheme://
-        //Debug.out(" loc:: rp:" + rp + " vs:" + vs  + " bp:" + bp)
+        bp = if p.at("/") then p.trim(1) else p end
+        //Debug.out(" loc:: " + loc + " => rp:" + rp + " vs:" + vs  + " bp:" + bp)
         break
       end end
     end
@@ -59,7 +58,7 @@ class val Locator is (su.ComparableMixin[Locator] & Hashable & Stringable)
 
   fun is_vcs(): Bool => vcs_suffix != ""
 
-  fun is_local(): Bool => (repo_path == "") and (vcs_suffix == "")
+  fun is_local(): Bool => (repo_path == "") or repo_path.at(".") or repo_path.at("/")
 
   fun is_remote_vcs(): Bool => is_vcs() and not is_local()
 
@@ -83,28 +82,17 @@ class Dep
     lock = lock'
     locator = Locator(data.locator)
 
-  fun name(): String =>
-    locator.path()
+  fun name(): String => locator.path()
 
-  fun repo(): String =>
-    locator.repo_path + locator.vcs_suffix
+  fun repo(): String => locator.repo_path + locator.vcs_suffix
 
-  fun flat_repo(): String =>
-    _Flattened(repo())
+  fun flat_repo(): String => _Flattened(repo())
 
   fun version(): String => data.version
 
-  fun revision(): String =>
-    if lock.revision != "" then
-      lock.revision
-    elseif data.version != "" then
-      data.version
-    else
-      "master"
-    end
+  fun revision(): String => lock.revision
 
-  fun vcs(): String =>
-    locator.vcs_suffix.trim(1)
+  fun vcs(): String => locator.vcs_suffix.trim(1)
 
 primitive _Flattened
   fun apply(path: String): String val =>

corral/cmd/cmd_fetch.pony

@@ -10,7 +10,8 @@ class CmdFetch is CmdType
   new create(cmd: Command) => None
 
   fun apply(ctx: Context, project: Project) =>
-    ctx.uout.info("fetch:")
+    ctx.uout.info("fetch: fetching from " + project.dir.path)
+
     match project.load_bundle()
     | let base_bundle: Bundle iso =>
       _Fetcher(ctx, project, consume base_bundle)
@@ -52,20 +53,10 @@ actor _Fetcher
       let vcs = VCSForType(ctx.env, dep.vcs())?
       let repo = RepoForDep(ctx, project, dep)?
 
-      // Determine revision of the dep to fetch
-      // - If it's already resolved from a lock, use that.
-      // - Else, if the dep is a specific revision, use that.
-      // - Else, it is a constraint to solve but update should have done that,
-      //   so just use master for now.
-      // TODO https://github.com/ponylang/corral/issues/59
-
-      var revision = base_bundle.dep_revision(dep.locator.string())
-      if revision == "" then
-        revision = dep.revision()
-      end
-      if Constraints.parse(revision).size() > 0 then
-        revision = "master"  // TODO: hack until we can ensure revisions are all locked.
-      end
+      let revision = Constraints.best_revision(
+        base_bundle.dep_revision(dep.locator.string()),
+        dep.revision(),
+        dep.version())
 
       let self: _Fetcher tag = this
       let checkout_op = vcs.checkout_op(revision,
@@ -80,13 +71,17 @@ actor _Fetcher
 
   be fetch_transitive_dep(locator: Locator) =>
     ctx.log.info("Fetching transitive dep: " + locator.path())
+
+    let bundle_dir = try
+        project.dep_bundle_root(locator)?
+      else
+        ctx.log.err("Unexpected error making path for: " + locator.string())
+        return
+      end
     try
-      let bundle_dir = project.dep_bundle_root(locator)?
       ctx.log.fine("Fetching dep's bundle from: " + bundle_dir.path)
       let dep_bundle: Bundle val = Bundle.load(bundle_dir, ctx.log)?
-      ctx.log.fine("Fetched dep's bundle is: " + dep_bundle.name())
       fetch_bundle_deps(dep_bundle)
     else
-      ctx.uout.err("Error loading/fetching dep bundle: " + locator.flat_name())
-      ctx.env.exitcode(1)
+      ctx.uout.warn("No dep bundle for: " + locator.string())
     end

corral/cmd/cmd_info.pony

@@ -8,13 +8,10 @@ class CmdInfo is CmdType
   new create(cmd: Command) => None
 
   fun apply(ctx: Context, project: Project) =>
-    ctx.uout.info("info: from dir " + project.dir.path)
+    ctx.uout.info("info: from " + project.dir.path)
 
     match project.load_bundle()
     | let bundle: Bundle =>
-      ctx.uout.info(
-        "info: from " + Files.bundle_filename()
-          + " in " + bundle.name())
       ctx.uout.info("info: " + bundle.info.json().string())
     | let err: Error =>
       ctx.uout.err(err.message)

corral/cmd/cmd_init.pony

@@ -11,7 +11,7 @@ class CmdInit is CmdType
   fun requires_no_bundle(): Bool => true
 
   fun apply(ctx: Context, project: Project) =>
-    ctx.uout.info("init: from dir " + project.dir.path)
+    ctx.uout.info("init: in " + project.dir.path)
 
     // TODO: try to read first to convert/update existing file(s)
     // TODO: might want to fail if files exist.

corral/cmd/cmd_list.pony

@@ -8,13 +8,10 @@ class CmdList is CmdType
   new create(cmd: Command) => None
 
   fun apply(ctx: Context, project: Project) =>
-    ctx.uout.info("list: from dir " + project.dir.path)
+    ctx.uout.info("list: from " + project.dir.path)
 
     match project.load_bundle()
     | let bundle: Bundle =>
-      ctx.uout.info(
-        "list: listing " + Files.bundle_filename() + " in " + bundle.name())
-
       let iter = project.transitive_deps(bundle).values()
       for d in iter do
         ctx.uout.info("  dep: " + d.name())

corral/cmd/cmd_update.pony

@@ -10,7 +10,7 @@ class CmdUpdate is CmdType
   new create(cmd: Command) => None
 
   fun apply(ctx: Context, project: Project) =>
-    ctx.uout.info("update:")
+    ctx.uout.info("update: updating from " + project.dir.path)
     match project.load_bundle()
     | let bundle: Bundle iso =>
       _Updater(ctx, project, consume bundle)
@@ -70,7 +70,12 @@ actor _Updater
 
     ctx.log.info("Loading dep: " + locator.path())
 
-    let checkout_op = vcs.checkout_op("master", {
+    let revision = Constraints.best_revision(
+      base_bundle.dep_revision(dep.locator.string()),
+      dep.revision(),
+      dep.version())
+
+    let checkout_op = vcs.checkout_op(revision, {
         (repo: Repo) =>
           self.load_transitive_dep(locator)
       } val)
@@ -88,15 +93,15 @@ actor _Updater
     sync_op(repo)
 
   be load_transitive_dep(locator: Locator) =>
-    ctx.log.info("Fetching transitive dep: " + locator.path())
+    ctx.log.info("Loading transitive dep: " + locator.path())
     try
       let bundle_dir = project.dep_bundle_root(locator)?
-      ctx.log.fine("Fetching dep's bundle from: " + bundle_dir.path)
+      ctx.log.fine("Loading dep's bundle from: " + bundle_dir.path)
       let dep_bundle: Bundle ref = Bundle.load(bundle_dir, ctx.log)?
-      ctx.log.fine("Fetched dep's bundle is: " + dep_bundle.name())
+      ctx.log.fine("Loading dep's bundle is: " + dep_bundle.name())
       load_bundle_deps(dep_bundle)
     else
-      ctx.uout.err("Error loading/fetching dep bundle: " + locator.flat_name())
+      ctx.uout.err("Error loading dep bundle: " + locator.flat_name())
       ctx.env.exitcode(1)
     end
 
@@ -134,22 +139,15 @@ actor _Updater
     end
 
   fun ref update_dep(dep: Dep, tags: Array[String] val) =>
-    try
-      // TODO: consider parsing version much earlier, maybe in Bundle.
-      // TODO: also consider allowing literal tags and not just constraints
-      // expressions.
-      // https://github.com/ponylang/corral/issues/26
-      let constraints = Constraints.parse(dep.data.version)
-
-      let result = Constraints.solve(constraints, tags, ctx.log)
-
-      // TODO: probably OK to have multiple solutions: choose 'best' with a strategy like 'latest'.
-      // https://github.com/ponylang/corral/issues/63
-      if result.solution.size() == 1 then
-        let rev: String = result.solution(0)?.version.string()
-        ctx.log.fine("solution for " + dep.locator.string() + ": " + rev)
-        base_bundle.lock_revision(dep.locator.string(), rev)
-      else
-        ctx.log.fine("no single solution for " + dep.locator.string() + ": " + result.solution.size().string())
+    // TODO: consider parsing version much earlier, maybe in Bundle.
+    // https://github.com/ponylang/corral/issues/26
+
+    let revision =
+      match Constraints.resolve_version(dep.data.version, tags, ctx.log)
+      | "" => Constraints.best_revision(
+        base_bundle.dep_revision(dep.locator.string()),
+        dep.revision(),
+        dep.version())
+      | let rev: String => rev
       end
-    end
+    base_bundle.lock_revision(dep.locator.string(), revision)

corral/cmd/constraints.pony

@@ -5,19 +5,82 @@ use sv="../semver/version"
 use "../util"
 
 primitive Constraints
-  fun parse(constraint_str: String box): Array[ss.Constraint] =>
+
+  fun resolve_version(version: String, tags: Array[String] val, log: Log): String
+  =>
+    """
+    Returns the best revision given a version string and an array of tag
+    choices. If version is a tag, hash or other non-constraint, then return
+    that.
+    """
+    // Attempt to parse version as constraints.
+    let constraints =
+      try
+        _parse_constraints(version)?
+      else
+        return version // interpret version as a literal tag or hash
+      end
+
+    let result = _solve_constraints(constraints, tags, log)
+
+    // TODO: probably OK to have multiple solutions: choose 'best' with a strategy like 'latest'.
+    // https://github.com/ponylang/corral/issues/63
+
+    try
+      if (result.solution.size() == 0) or (result.is_err()) then
+        log.warn("no solution for " + version + ": " + result.err)
+        ""
+      elseif result.solution.size() == 1 then
+        let rev: String = result.solution(0)?.version.string()
+        log.fine("single solution for " + version + ": " + rev)
+        rev
+      else
+        log.fine("multiple solutions for " + version + ": " + result.solution.size().string())
+        let max_heap = MaxHeap[ss.Artifact box](result.solution.size())
+        max_heap.append(result.solution) // Could use itertools.map() to get a String iter
+        let rev_arti = max_heap.pop()?
+        log.fine("  selected: " + rev_arti.string())
+        rev_arti.string()
+      end
+    else
+      "" // Should not happen since we know collections accessed are not empty
+    end
+
+
+  fun best_revision(lrevision: String, drevision: String, version: String): String
+  =>
+    """
+    Returns the best choice of possible: a lock revision, a fallback dep revision, and a version.
+    TODO https://github.com/ponylang/corral/issues/59
+    """
+    if lrevision != "" then
+      lrevision  // Base lock revision is always best
+    elseif drevision != "" then
+      drevision  // Dep lock revision is second best
+    else
+      try
+        Constraints._parse_constraints(version)?
+        "master" // Is a constraint: use master until update.
+      else
+        if version != "" then
+          version  // Version is not a constraint, use that.
+        else
+          "master"  // Get the latest master if no constraints at all.
+        end
+      end
+    end
+
+  fun _parse_constraints(constraint_str: String box): Array[ss.Constraint] ? =>
     let constraints: Array[ss.Constraint] = constraints.create()
     for c in constraint_str.split_by(" ").values() do
       let cs = recover val c.clone().>strip() end
       if cs != "" then
-        try
-          constraints.push(_parse_constraint(cs)?)
-        else
-          // How should we report a bad constraint?
-          None // ctx.log.warn("Error parsing constraint " + cs)
-        end
+        constraints.push(_parse_constraint(cs)?)
       end
     end
+    if constraints.size() == 0 then
+      error
+    end
     constraints
 
   fun _parse_constraint(c: String box): ss.Constraint ? =>
@@ -42,16 +105,16 @@ primitive Constraints
     end
     error
 
-  fun solve(constraints: Array[ss.Constraint], tags: Array[String] val, log: Log): ss.Result
+  fun _solve_constraints(constraints: Array[ss.Constraint], tags: Array[String] val, log: Log): ss.Result
   =>
-      let source: ss.InMemArtifactSource = source.create()
-      for tg in tags.values() do
-        log.fine("  tag:" + tg)
-        let artifact = ss.Artifact("A", sv.ParseVersion(tg))
-        source.add(artifact)
-      end
-      let result = ss.Solver(source).solve(constraints.values())
-      if result.is_err() then
-        log.fine("result err: " + result.err)
-      end
-      result
+    let source: ss.InMemArtifactSource = source.create()
+    for tg in tags.values() do
+      log.fine("  tag:" + tg)
+      let artifact = ss.Artifact("A", sv.ParseVersion(tg))
+      source.add(artifact)
+    end
+    let result = ss.Solver(source).solve(constraints.values())
+    if result.is_err() then
+      log.fine("result err: " + result.err)
+    end
+    result

corral/test/integration/test_clean.pony

@@ -4,7 +4,7 @@ use ".."
 use "../../util"
 
 class TestClean is UnitTest
-  fun name(): String => "integration/clean-github-deep"
+  fun name(): String => "integration/clean/github-deep"
 
   fun apply(h: TestHelper) =>
     h.long_test(2_000_000_000)