Misleading "Unable to bump version" is logged when the version was already bumped #2906

alexklibisz · 2023-01-10T00:29:49Z

When there are two deps both referencing the same val for their versions, the second update will print a misleading warning. I would expect that there is no warning, since the second update was, practically speaking, applied.

I'll create a PR to reproduce this.

…nce the same val version

alexklibisz · 2023-02-12T16:55:46Z

I looked into this a bit more today, running scala-steward on a private repo. I noticed the following interesting behavior:

I created an empty project with a build.sbt like this:

ThisBuild / version := "0.1.0-SNAPSHOT"

ThisBuild / scalaVersion := "2.13.10"

lazy val circeVersion = "0.13.0"

lazy val root = (project in file("."))
  .settings(
    name := "scala-steward-2906",
    libraryDependencies ++= Seq(
      "io.circe" %% "circe-core" % circeVersion,
      "io.circe" %% "circe-parser" % circeVersion
    )
  )

When I run scala-steward with no .scala-steward.conf, it correctly updates the circeVersion and does not print a warning.

Then I added a .scala-steward.conf to group the updates:

pullRequests.grouping = [
  { name = "circe", "title" = "Circe updates", "filter" = [{"group" = "io.circe"}] },
]

In this case, it still correctly updates the version, but it does print the warning:

2023-02-12 08:37:14,230 INFO  Found 1 update:
  circe (group) {
    io.circe:(circe-core, circe-core_2.13) : 0.13.0 -> 0.14.4
    io.circe:(circe-parser, circe-parser_2.13) : 0.13.0 -> 0.14.4
  }
...
2023-02-12 08:37:15,043 WARN  Unable to bump version for update io.circe:(circe-parser, circe-parser_2.13) : 0.13.0 -> 0.14.4

I'll do some more debugging and see if I can make a PR to resolve this.

alexklibisz · 2023-02-12T17:28:53Z

I was able to replicate this behavior in a "real" project. For now I think I'm just going to avoid the pullRequests.grouping configuration.

alexklibisz · 2023-02-12T17:39:31Z

I added some println debugging in my fake circe project, on this commit: 39b618f. Specifically in the findUpdateReplacements and applyUpdateReplacements methods in EditAlg. Below are the respective logs. I pulled out the parts that looked relevant, but also attached the full log files (incl. many debug lines).

With no grouping and no warning:

2023-02-12 09:10:40,320 INFO  Found 2 updates:
  io.circe:(circe-core, circe-core_2.13) : 0.13.0 -> 0.14.4
  io.circe:(circe-parser, circe-parser_2.13) : 0.13.0 -> 0.14.4
2023-02-12 09:10:40,325 INFO  alexklibisz/scala-steward-2906 is outdated:
  new version: io.circe:(circe-core, circe-core_2.13) : 0.13.0 -> 0.14.4
  new version: io.circe:(circe-parser, circe-parser_2.13) : 0.13.0 -> 0.14.4
2023-02-12 09:10:40,326 INFO  Nurture alexklibisz/scala-steward-2906
2023-02-12 09:10:40,329 INFO  Found 1 update:
  io.circe:{(circe-core, circe-core_2.13), (circe-parser, circe-parser_2.13)} : 0.13.0 -> 0.14.4
2023-02-12 09:10:40,356 INFO  Process update io.circe:{(circe-core, circe-core_2.13), (circe-parser, circe-parser_2.13)} : 0.13.0 -> 0.14.4
findUpdateReplacements
  update = ForGroupId(NonEmptyList(ForArtifactId(CrossDependency(NonEmptyList(Dependency(io.circe,ArtifactId(circe-core,Some(circe-core_2.13)),0.13.0,None,None,None))),NonEmptyList(0.14.4),None,None), ForArtifactId(CrossDependency(NonEmptyList(Dependency(io.circe,ArtifactId(circe-parser,Some(circe-parser_2.13)),0.13.0,None,None,None))),NonEmptyList(0.14.4),None,None)))
  versionPositions = List(ScalaVal(Position(build.sbt,105,0.13.0),lazy ,circeVersion))
  modulePositions = List(ModulePosition(Position(build.sbt,240,io.circe),Position(build.sbt,254,circe-core),Position(build.sbt,268,circeVersion,)), ModulePosition(Position(build.sbt,289,io.circe),Position(build.sbt,303,circe-parser),Position(build.sbt,319,circeVersion)))
2023-02-12 09:10:40,908 INFO  Create branch update/circe-core-0.14.4
applyUpdateReplacements
  data = ...
  update = ForGroupId(NonEmptyList(ForArtifactId(CrossDependency(NonEmptyList(Dependency(io.circe,ArtifactId(circe-core,Some(circe-core_2.13)),0.13.0,None,None,None))),NonEmptyList(0.14.4),None,None), ForArtifactId(CrossDependency(NonEmptyList(Dependency(io.circe,ArtifactId(circe-parser,Some(circe-parser_2.13)),0.13.0,None,None,None))),NonEmptyList(0.14.4),None,None)))
  updateReplacements = List(Replacement(Position(build.sbt,105,0.13.0),0.14.4))
2023-02-12 09:10:41,021 INFO  Push 1 commit(s)
2023-02-12 09:10:41,999 INFO  Create PR update/circe-core-0.14.4
2023-02-12 09:10:50,348 INFO  Created PR https://github.com/alexklibisz/scala-steward-2906/pull/5
2023-02-12 09:10:50,446 INFO  ──────────── Total time: Steward alexklibisz/scala-steward-2906: 23s 158ms ────────────
2023-02-12 09:10:50,447 INFO  ──────────── Total time: run: 23s 524ms ────────────

Full logs: no-warning.log

With grouping and with warning:

2023-02-12 08:59:30,741 INFO  Found 2 updates:
  io.circe:(circe-core, circe-core_2.13) : 0.13.0 -> 0.14.4
  io.circe:(circe-parser, circe-parser_2.13) : 0.13.0 -> 0.14.4
2023-02-12 08:59:30,745 INFO  alexklibisz/scala-steward-2906 is outdated:
  new version: io.circe:(circe-core, circe-core_2.13) : 0.13.0 -> 0.14.4
  new version: io.circe:(circe-parser, circe-parser_2.13) : 0.13.0 -> 0.14.4
2023-02-12 08:59:30,746 INFO  Nurture alexklibisz/scala-steward-2906
2023-02-12 08:59:30,753 INFO  Found 1 update:
  circe (group) {
    io.circe:(circe-core, circe-core_2.13) : 0.13.0 -> 0.14.4
    io.circe:(circe-parser, circe-parser_2.13) : 0.13.0 -> 0.14.4
  }
2023-02-12 08:59:30,771 INFO  Process update circeEAD
2023-02-12 08:59:31,016 INFO  Create branch update/circe
findUpdateReplacements
  update = ForArtifactId(CrossDependency(NonEmptyList(Dependency(io.circe,ArtifactId(circe-core,Some(circe-core_2.13)),0.13.0,None,None,None))),NonEmptyList(0.14.4),None,None)
  versionPositions = List(ScalaVal(Position(build.sbt,105,0.13.0),lazy ,circeVersion))
  modulePositions = List(ModulePosition(Position(build.sbt,240,io.circe),Position(build.sbt,254,circe-core),Position(build.sbt,268,circeVersion,)))
applyUpdateReplacements
  data = ...
  update = ForArtifactId(CrossDependency(NonEmptyList(Dependency(io.circe,ArtifactId(circe-core,Some(circe-core_2.13)),0.13.0,None,None,None))),NonEmptyList(0.14.4),None,None)
  updateReplacements = List(Replacement(Position(build.sbt,105,0.13.0),0.14.4))
findUpdateReplacements
  update = ForArtifactId(CrossDependency(NonEmptyList(Dependency(io.circe,ArtifactId(circe-parser,Some(circe-parser_2.13)),0.13.0,None,None,None))),NonEmptyList(0.14.4),None,None)
  versionPositions = List()
  modulePositions = List(ModulePosition(Position(build.sbt,289,io.circe),Position(build.sbt,303,circe-parser),Position(build.sbt,319,circeVersion)))
2023-02-12 08:59:31,493 WARN  Unable to bump version for update io.circe:(circe-parser, circe-parser_2.13) : 0.13.0 -> 0.14.4date/circe --
2023-02-12 08:59:31,508 INFO  Push 1 commit(s)eam origin update/circe
2023-02-12 08:59:32,694 INFO  Create PR update/circe--files-with-matches 0.13.0--files-with-matches 0.13.0
2023-02-12 08:59:38,993 INFO  Created PR https://github.com/alexklibisz/scala-steward-2906/pull/4
2023-02-12 08:59:39,087 INFO  ──────────── Total time: Steward alexklibisz/scala-steward-2906: 21s 638ms ────────────
2023-02-12 08:59:39,088 INFO  ──────────── Total time: run: 22s 13ms ────────────

Full logs: warning.log

alexklibisz · 2023-02-12T17:44:29Z

The main difference I see is in the update parameter passed to findUpdateReplacements:

without grouping: a single call o findUpdateReplacement w/ update = ForGroupId(NonEmptyList(ForArtifactId(CrossDependency(NonEmptyList(Dependency(io.circe,ArtifactId(circe-core,Some(circe-core_2.13)),0.13.0,None,None,None))),NonEmptyList(0.14.4),None,None), ForArtifactId(CrossDependency(NonEmptyList(Dependency(io.circe,ArtifactId(circe-parser,Some(circe-parser_2.13)),0.13.0,None,None,None))),NonEmptyList(0.14.4),None,None)))
with grouping: two calls to findUpdateReplacement w/ update = ForArtifactId(CrossDependency(NonEmptyList(Dependency(io.circe,ArtifactId(circe-core,Some(circe-core_2.13)),0.13.0,None,None,None))),NonEmptyList(0.14.4),None,None) and update = ForArtifactId(CrossDependency(NonEmptyList(Dependency(io.circe,ArtifactId(circe-parser,Some(circe-parser_2.13)),0.13.0,None,None,None))),NonEmptyList(0.14.4),None,None)

alexklibisz · 2023-02-12T17:47:58Z

So there might be two questions here:

Why does presence or absence of grouping produce different updates for the same dependency?
Why does a ForArtifactId(CrossDependency(...)) update produce this warning?

alexklibisz · 2023-02-12T18:37:39Z

One last though, and then I have to wrap up my investigation soon:

I added another println to applyNewUpdate in NurtureAlg, to print the data.update parameter.

Without grouping: data.update = ForGroupId(NonEmptyList(ForArtifactId(CrossDependency(NonEmptyList(Dependency(io.circe,ArtifactId(circe-core,Some(circe-core_2.13)),0.13.0,None,None,None))),NonEmptyList(0.14.4),None,None), ForArtifactId(CrossDependency(NonEmptyList(Dependency(io.circe,ArtifactId(circe-parser,Some(circe-parser_2.13)),0.13.0,None,None,None))),NonEmptyList(0.14.4),None,None)))
With grouping: data.update = Grouped(circe,Some(Circe updates),List(ForArtifactId(CrossDependency(NonEmptyList(Dependency(io.circe,ArtifactId(circe-core,Some(circe-core_2.13)),0.13.0,None,None,None))),NonEmptyList(0.14.4),None,None), ForArtifactId(CrossDependency(NonEmptyList(Dependency(io.circe,ArtifactId(circe-parser,Some(circe-parser_2.13)),0.13.0,None,None,None))),NonEmptyList(0.14.4),None,None)))

So, in this case, the Grouped update and the ForGroupId update are actually equivalent, but they are treated differently based on the grouping. That difference seems to originate in Update.groupByPullRequestGroup. Maybe we can update that method to identify cases like this one, and return a ForGroupId instead of a Grouped?

alejandrohdezma · 2023-02-13T16:29:12Z

So the reason this happens is because Grouped updates work with single ForArtifactId updates. So when two update dependencies use the same val for the version, the first one updates the val and the second one doesn't do anything and logs the warning. There is no point on making individual updates aware of the changes their predecessor updates have made. The reason why this was made this way is because there is no reason to have both ForArtifactId and ForGroupId inside Grouped updates since the latter were created to "join" together updates for the same groupId so it would only create one PR.

alejandrohdezma · 2023-02-13T16:31:08Z

So, in this case, the Grouped update and the ForGroupId update are actually equivalent, but they are treated differently based on the grouping. That difference seems to originate in Update.groupByPullRequestGroup. Maybe we can update that method to identify cases like this one, and return a ForGroupId instead of a Grouped?

Also for this, if we generate a ForGroupId instead of Grouped we are not doing what the user has stated, which could lead to automations not working since the PR created would not respect the branch/PR-title set in the grouping setting.

alexklibisz · 2023-02-13T16:34:51Z

Thanks for the insight. I'll have to re-read a few times to make sure I follow. In the meantime, do you see any way to avoid the incorrect warnings?

alejandrohdezma · 2023-02-13T16:43:40Z

🤔 Nothing that would require refactors here and there so Grouped supported both ForArtifactId and ForGroupId. Or maybe just remove the log or change it to be debug? cc @exoego

alexklibisz added a commit to alexklibisz/scala-steward that referenced this issue Jan 10, 2023

Issue scala-steward-org#2906: Misleading warning when two deps refere…

0ab2a67

…nce the same val version

alexklibisz mentioned this issue Jan 10, 2023

Issue #2906: Misleading warning when two deps reference the same val version #2907

Open

exoego added the enhancement New feature or request label Feb 9, 2023

exoego mentioned this issue Feb 9, 2023

Strict mode that fails if any version bump fails for any update #2905

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Misleading "Unable to bump version" is logged when the version was already bumped #2906

Misleading "Unable to bump version" is logged when the version was already bumped #2906

alexklibisz commented Jan 10, 2023 •

edited

Loading

alexklibisz commented Feb 12, 2023

alexklibisz commented Feb 12, 2023 •

edited

Loading

alexklibisz commented Feb 12, 2023 •

edited

Loading

alexklibisz commented Feb 12, 2023 •

edited

Loading

alexklibisz commented Feb 12, 2023 •

edited

Loading

alexklibisz commented Feb 12, 2023

alejandrohdezma commented Feb 13, 2023

alejandrohdezma commented Feb 13, 2023

alexklibisz commented Feb 13, 2023

alejandrohdezma commented Feb 13, 2023

Misleading "Unable to bump version" is logged when the version was already bumped #2906

Misleading "Unable to bump version" is logged when the version was already bumped #2906

Comments

alexklibisz commented Jan 10, 2023 • edited Loading

alexklibisz commented Feb 12, 2023

alexklibisz commented Feb 12, 2023 • edited Loading

alexklibisz commented Feb 12, 2023 • edited Loading

alexklibisz commented Feb 12, 2023 • edited Loading

alexklibisz commented Feb 12, 2023 • edited Loading

alexklibisz commented Feb 12, 2023

alejandrohdezma commented Feb 13, 2023

alejandrohdezma commented Feb 13, 2023

alexklibisz commented Feb 13, 2023

alejandrohdezma commented Feb 13, 2023

alexklibisz commented Jan 10, 2023 •

edited

Loading

alexklibisz commented Feb 12, 2023 •

edited

Loading

alexklibisz commented Feb 12, 2023 •

edited

Loading

alexklibisz commented Feb 12, 2023 •

edited

Loading

alexklibisz commented Feb 12, 2023 •

edited

Loading