Shorten Dep serialization format in common cases

.github/workflows/run-tests.yml

@@ -23,6 +23,8 @@ concurrency:
   cancel-in-progress: true
 
 jobs:
+  # Jobs are listed in rough order of priority: if multiple jobs fail, the first job
+  # in the list should be the one that's most worth looking into
   build-linux:
     uses: ./.github/workflows/run-mill-action.yml
     with:
@@ -46,38 +48,6 @@ jobs:
 
       - run: ./mill -i docs.githubPages
 
-  compiler-bridge:
-    needs: build-linux
-    uses: ./.github/workflows/run-mill-action.yml
-    with:
-      java-version: '11'
-      millargs: bridge.__.publishLocal
-      env-bridge-versions: 'essential'
-
-  lint-autofix:
-    needs: build-linux
-    uses: ./.github/workflows/run-mill-action.yml
-    with:
-      java-version: '11'
-      buildcmd: ./mill -i mill.scalalib.scalafmt.ScalafmtModule/checkFormatAll __.sources + __.mimaReportBinaryIssues + __.fix --check
-
-  itest:
-    needs: build-linux
-    strategy:
-      fail-fast: false
-      matrix:
-        include:
-          # bootstrap tests
-          - java-version: '11' # Have one job on oldest JVM
-            buildcmd: ci/test-mill-dev.sh && ci/test-mill-release.sh && ./mill -i -k __.ivyDepsTree && ./mill -i -k __.ivyDepsTree --withRuntime
-          - java-version: 17 # Have one job on default JVM
-            buildcmd: ci/test-mill-bootstrap.sh
-
-    uses: ./.github/workflows/run-mill-action.yml
-    with:
-      java-version: ${{ matrix.java-version }}
-      buildcmd: ${{ matrix.buildcmd }}
-
   linux:
     needs: build-linux
     strategy:
@@ -148,3 +118,38 @@ jobs:
       os: windows-latest
       java-version: ${{ matrix.java-version }}
       millargs: ${{ matrix.millargs }}
+
+  itest:
+    needs: build-linux
+    strategy:
+      fail-fast: false
+      matrix:
+        include:
+          # bootstrap tests
+          - java-version: '11' # Have one job on oldest JVM
+            buildcmd: ci/test-mill-dev.sh && ci/test-mill-release.sh && ./mill -i -k __.ivyDepsTree && ./mill -i -k __.ivyDepsTree --withRuntime
+          - java-version: 17 # Have one job on default JVM
+            buildcmd: ci/test-mill-bootstrap.sh
+
+    uses: ./.github/workflows/run-mill-action.yml
+    with:
+      java-version: ${{ matrix.java-version }}
+      buildcmd: ${{ matrix.buildcmd }}
+
+  # Rarely breaks so run it near the end
+  compiler-bridge:
+    needs: build-linux
+    uses: ./.github/workflows/run-mill-action.yml
+    with:
+      java-version: '11'
+      millargs: bridge.__.publishLocal
+      env-bridge-versions: 'essential'
+
+  # Scalafmt, Mima, and Scalafix job runs last because it's the least important:
+  # usually just a automated or mechanical manual fix to do before merging
+  lint-autofix:
+    needs: build-linux
+    uses: ./.github/workflows/run-mill-action.yml
+    with:
+      java-version: '11'
+      buildcmd: ./mill -i mill.scalalib.scalafmt.ScalafmtModule/checkFormatAll __.sources + __.mimaReportBinaryIssues + __.fix --check

scalalib/src/mill/scalalib/Dep.scala

@@ -124,6 +124,53 @@ object Dep {
       case _ => throw new Exception(s"Unable to parse signature: [$signature]")
     }).configure(attributes = attributes)
   }
+
+  @unused private implicit val depFormat: RW[Dependency] = mill.scalalib.JsonFormatters.depFormat
+
+  def unparse(dep: Dep): Option[String] = {
+    val org = dep.dep.module.organization.value
+    val mod = dep.dep.module.name.value
+    val ver = dep.dep.version
+
+    val classifierAttr = dep.dep.attributes.classifier.value match {
+      case "" => ""
+      case s => s";classifier=$s"
+    }
+
+    val typeAttr = dep.dep.attributes.`type`.value match {
+      case "" => ""
+      case s => s";type=$s"
+    }
+    val attrs = classifierAttr + typeAttr
+
+    val prospective = dep.cross match {
+      case CrossVersion.Constant("", false) => Some(s"$org:$mod:$ver$attrs")
+      case CrossVersion.Constant("", true) => Some(s"$org:$mod::$ver$attrs")
+      case CrossVersion.Binary(false) => Some(s"$org::$mod:$ver$attrs")
+      case CrossVersion.Binary(true) => Some(s"$org::$mod::$ver$attrs")
+      case CrossVersion.Full(false) => Some(s"$org:::$mod:$ver$attrs")
+      case CrossVersion.Full(true) => Some(s"$org:::$mod::$ver$attrs")
+      case CrossVersion.Constant(v, _) => None
+    }
+
+    prospective.filter(parse(_) == dep)
+  }
+  private val rw0: RW[Dep] = macroRW
+
+  // Use literal JSON strings for common cases so that files
+  // containing serialized dependencies can be easier to skim
+  implicit val rw: RW[Dep] = upickle.default.readwriter[ujson.Value].bimap[Dep](
+    (dep: Dep) =>
+      unparse(dep) match {
+        case Some(s) => ujson.Str(s)
+        case None => upickle.default.writeJs[Dep](dep)(rw0)
+      },
+    {
+      case s: ujson.Str => parse(s.value)
+      case v: ujson.Value => upickle.default.read[Dep](v)(rw0)
+    }
+  )
+
   def apply(
       org: String,
       name: String,
@@ -140,8 +187,7 @@ object Dep {
       force
     )
   }
-  @unused private implicit val depFormat: RW[Dependency] = mill.scalalib.JsonFormatters.depFormat
-  implicit def rw: RW[Dep] = macroRW
+
 }
 
 sealed trait CrossVersion {
@@ -213,5 +259,26 @@ case class BoundDep(
 
 object BoundDep {
   @unused private implicit val depFormat: RW[Dependency] = mill.scalalib.JsonFormatters.depFormat
-  implicit val jsonify: upickle.default.ReadWriter[BoundDep] = upickle.default.macroRW
+  private val jsonify0: upickle.default.ReadWriter[BoundDep] = upickle.default.macroRW
+
+  // Use literal JSON strings for common cases so that files
+  // containing serialized dependencies can be easier to skim
+  //
+  // `BoundDep` is basically a `Dep` with `cross=CrossVersion.Constant("", false)`,
+  // so we can re-use most of `Dep`'s serialization logic
+  implicit val jsonify: upickle.default.ReadWriter[BoundDep] =
+    upickle.default.readwriter[ujson.Value].bimap[BoundDep](
+      bdep => {
+        Dep.unparse(Dep(bdep.dep, CrossVersion.Constant("", false), bdep.force)) match {
+          case None => upickle.default.writeJs(bdep)(jsonify0)
+          case Some(s) => ujson.Str(s)
+        }
+      },
+      {
+        case ujson.Str(s) =>
+          val dep = Dep.parse(s)
+          BoundDep(dep.dep, dep.force)
+        case v => upickle.default.read[BoundDep](v)(jsonify0)
+      }
+    )
 }

scalalib/test/src/mill/scalalib/ResolveDepsTests.scala

@@ -16,6 +16,18 @@ object ResolveDepsTests extends TestSuite {
     deps.map(Lib.depToBoundDep(_, scala212Version, ""))
   )
 
+  def assertRoundTrip(deps: Agg[Dep], simplified: Boolean) = {
+    for (dep <- deps) {
+      val unparsed = Dep.unparse(dep)
+      if (simplified) {
+        assert(unparsed.nonEmpty)
+        assert(Dep.parse(unparsed.get) == dep)
+      } else {
+        assert(unparsed.isEmpty)
+      }
+      assert(upickle.default.read[Dep](upickle.default.write(dep)) == dep)
+    }
+  }
   val tests = Tests {
     test("resolveValidDeps") {
       val deps = Agg(ivy"com.lihaoyi::pprint:0.5.3")
@@ -25,13 +37,15 @@ object ResolveDepsTests extends TestSuite {
 
     test("resolveValidDepsWithClassifier") {
       val deps = Agg(ivy"org.lwjgl:lwjgl:3.1.1;classifier=natives-macos")
+      assertRoundTrip(deps, simplified = true)
       val Success(paths) = evalDeps(deps)
       assert(paths.nonEmpty)
       assert(paths.items.next().path.toString.contains("natives-macos"))
     }
 
     test("resolveTransitiveRuntimeDeps") {
       val deps = Agg(ivy"org.mockito:mockito-core:2.7.22")
+      assertRoundTrip(deps, simplified = true)
       val Success(paths) = evalDeps(deps)
       assert(paths.nonEmpty)
       assert(paths.exists(_.path.toString.contains("objenesis")))
@@ -40,12 +54,14 @@ object ResolveDepsTests extends TestSuite {
 
     test("excludeTransitiveDeps") {
       val deps = Agg(ivy"com.lihaoyi::pprint:0.5.3".exclude("com.lihaoyi" -> "fansi_2.12"))
+      assertRoundTrip(deps, simplified = false)
       val Success(paths) = evalDeps(deps)
       assert(!paths.exists(_.path.toString.contains("fansi_2.12")))
     }
 
     test("excludeTransitiveDepsByOrg") {
       val deps = Agg(ivy"com.lihaoyi::pprint:0.5.3".excludeOrg("com.lihaoyi"))
+      assertRoundTrip(deps, simplified = false)
       val Success(paths) = evalDeps(deps)
       assert(!paths.exists(path =>
         path.path.toString.contains("com/lihaoyi") && !path.path.toString.contains("pprint_2.12")
@@ -54,24 +70,28 @@ object ResolveDepsTests extends TestSuite {
 
     test("excludeTransitiveDepsByName") {
       val deps = Agg(ivy"com.lihaoyi::pprint:0.5.3".excludeName("fansi_2.12"))
+      assertRoundTrip(deps, simplified = false)
       val Success(paths) = evalDeps(deps)
       assert(!paths.exists(_.path.toString.contains("fansi_2.12")))
     }
 
     test("errOnInvalidOrgDeps") {
       val deps = Agg(ivy"xxx.yyy.invalid::pprint:0.5.3")
+      assertRoundTrip(deps, simplified = true)
       val Failure(errMsg, _) = evalDeps(deps)
       assert(errMsg.contains("xxx.yyy.invalid"))
     }
 
     test("errOnInvalidVersionDeps") {
       val deps = Agg(ivy"com.lihaoyi::pprint:invalid.version.num")
+      assertRoundTrip(deps, simplified = true)
       val Failure(errMsg, _) = evalDeps(deps)
       assert(errMsg.contains("invalid.version.num"))
     }
 
     test("errOnPartialSuccess") {
       val deps = Agg(ivy"com.lihaoyi::pprint:0.5.3", ivy"fake::fake:fake")
+      assertRoundTrip(deps, simplified = true)
       val Failure(errMsg, _) = evalDeps(deps)
       assert(errMsg.contains("fake"))
     }