SBT fails on autoReplace

[walczakp]

What would you like help with?

I think I found a bug

How are you running Renovate?

Self-hosted

If you're self-hosting Renovate, tell us which platform (GitHub, GitLab, etc) and which version of Renovate.

GitHub

Please tell us more about your question or problem

Updates for SBT stopped working after in one of the renovate releases. I am running renovate using docker image. Seem to still work fine with docker image 37.215.0-slim, does not work from 37.221.1-slim onwards (also tested on 37.245.0-slim).

Minimal reproduction repo

Created a minimal reproduction repo here: https://github.com/walczakp/renovate-sbt-issue
The following workflow is how I trigger renovate in the original repo: https://github.com/walczakp/renovate-sbt-issue/actions/runs/8292942418/job/22695112052

Logs (if relevant)

Logs

renovate version: 37.245.0-slim

DEBUG: manager.getUpdatedPackageFiles() reuseExistingBranch=false (repository=my-cool-repo, branch=deps/testcontainers-java-monorepo)
TRACE: autoReplace replaceString (repository=my-cool-repo, branch=deps/testcontainers-java-monorepo)
       "depName": "org.testcontainers:testcontainers",
       "replaceString": "1.19.6"
DEBUG: Starting search at index 2226 (repository=my-cool-repo, packageFile=project/Dependencies.scala, branch=deps/testcontainers-java-monorepo)
       "depName": "org.testcontainers:testcontainers"
DEBUG: Found match at index 2226 (repository=my-cool-repo, packageFile=project/Dependencies.scala, branch=deps/testcontainers-java-monorepo)
       "depName": "org.testcontainers:testcontainers"
DEBUG: Could not extract project/Dependencies.scala (manager=sbt) after autoreplace. Did the autoreplace make the file unparseable? (repository=my-cool-repo, branch=deps/testcontainers-java-monorepo)
TRACE: packageFile content after autoreplace (repository=my-cool-repo, packageFile=project/Dependencies.scala, branch=deps/testcontainers-java-monorepo)
       "content": "[content]"
 WARN: Error updating branch: update failure (repository=my-cool-repo, branch=deps/testcontainers-java-monorepo)

renovate version: 37.215.0-slim

INFO: Branch created (repository=my-cool-repo, branch=deps/testcontainers-java-monorepo)
       "commitSha": "..."
 INFO: PR created (repository=my-cool-repo, branch=deps/testcontainers-java-monorepo)
       "pr": 392,
       "prTitle": "fix(deps): update dependency org.testcontainers:testcontainers to v1.19.7"

[github-actions[bot]]

Hi there,

Get your discussion fixed faster by creating a minimal reproduction. This means a repository dedicated to reproducing this issue with the minimal dependencies and config possible.

Before we start working on your issue we need to know exactly what's causing the current behavior. A minimal reproduction helps us with this. Discussions without reproductions are less likely to be converted to Issues.

To get started, please read our guide on creating a minimal reproduction.

Good luck,

The Renovate team

[walczakp]

Created a minimal reproduction repo here: https://github.com/walczakp/renovate-sbt-issue
The following workflow is how I trigger renovate in the original repo: https://github.com/walczakp/renovate-sbt-issue/actions/runs/8292942418/job/22695112052

[Shegox]

I did run into the same problem and tried to narrow it down.

During the run it seems like the main problem is that during the confirmation of the update it fails to extract the package at because extractPackageFile is not available for the sbt-manger:

renovate/lib/modules/manager/index.ts

Lines 73 to 76 in e0b2772

	return m.extractPackageFile
	? m.extractPackageFile(content, fileName, config)
	: null;

Digging a bit deeper it seems that the sbt-manager only exports extractAllPackageFiles and removed extractPackageFile from the export in in 5de12e7#diff-14828857daefc37b6914840eb83ef93765f7929ccb087c88edd55b9392b55c84
caused this problem.

Manually adding extractPackageFile back to the sbt/index.ts exports caused it to work again:

diff --git a/lib/modules/manager/sbt/index.ts b/lib/modules/manager/sbt/index.ts
index 671f3e729..d143d5ebf 100644
--- a/lib/modules/manager/sbt/index.ts
+++ b/lib/modules/manager/sbt/index.ts
@@ -5,7 +5,7 @@ import { SbtPackageDatasource } from '../../datasource/sbt-package';
 import { SbtPluginDatasource } from '../../datasource/sbt-plugin';
 import * as ivyVersioning from '../../versioning/ivy';
 
-export { extractAllPackageFiles } from './extract';
+export { extractAllPackageFiles, extractPackageFile } from './extract';
 export { bumpPackageVersion } from './update';
 
 export const supportedDatasources = [

The problem seems that the confirmIfDepUpdated function can only use extractPackageFile to confirm if an update took place, but not extractAllPackageFiles:

renovate/lib/workers/repository/update/branch/auto-replace.ts

Lines 13 to 25 in b69416c

	export async function confirmIfDepUpdated(
	upgrade: BranchUpgradeConfig,
	newContent: string,
	): Promise<boolean> {
	const { manager, packageFile, depIndex } = upgrade;
	let newUpgrade: PackageDependency;
	try {
	const newExtract = await extractPackageFile(
	manager,
	newContent,
	packageFile!,
	upgrade,
	);

[Shegox]

@rarkins do you think just adding extractPackageFile back to the exports is the right approach to fix this? If yes, I will open a PR.

I'm a bit confused because other managers (e.g. npm) are as well only exporting extractAllPackageFiles and there it works...
So not sure if there is a better approach to use it, because the sbt-extractAllPackageFiles does some registryUrl massaging on top of extracting the deps.

Edit: I found that the code is only executed when the manager doesn't implement updateDependency(), because then Renovate falls back to use the standard replacing mechanism and this uses confirmIfDepUpdated(), this again depends on extractPackageFile(). The other managers only exporting extractAllPackageFiles() do implement updateDependency() and thus don't run into this problem.

[Shegox]

Created:

• fix(sbt): export extractPackageFile() for confirmIfDepUpdated() #27953 to fix this for sbt
• test(manager): manager need to exports updateDependency or extractPackageFile when exporting extractAllPackageFiles #27954 to add a test that managers must implement either extractPackageFile or updateDependency .

[walczakp]

Confirmed, can't reproduce with 37.252.0. Thank you all for helping with that!