Skip to content

Conversation

@griffio
Copy link
Contributor

@griffio griffio commented Oct 1, 2025

Currently sql-psi breaks SqlDelight - main reason is that localFileSystem, jarFileSystem were private.

Make localFileSystem, jarFileSystem protected as they are inherited properties in SqlDelight.
Unless there is some reason due to Isolation for these to be private ?

Fix SqlDelight fixture tests with file.toAbsolutePath as the relative path breaks all tests in SqlDelight.

Note:
SqlDelight will require some minor changes to take this sql-psi version as it is not a compatible version, that will be done once this new version is published and can be tried out.

This sql-psi was tested in SqlDelight by updating settings.gradle

includeBuild('../sql-psi') {
  dependencySubstitution {
    substitute(module("app.cash.sql-psi:core")).using(project(":core"))
    substitute(module("app.cash.sql-psi:environment")).using(project(":environment"))
  }
}

Integration tests are independent builds so need a different relative path for settings.gradle

includeBuild('../../../../../sql-psi') {
  dependencySubstitution {
    substitute(module("app.cash.sql-psi:core")).using(project(":core"))
    substitute(module("app.cash.sql-psi:environment")).using(project(":environment"))
  }
}

  • CHANGELOG.md's "Unreleased" section has been updated, if applicable.

Make localFileSystem, jarFileSystem protected as they are inherited properties in SqlDelight

Unless there is some reason due to Isolation for these to be private

Fix fixture tests with `file.toAbsolutePath` as the relative path breaks tests in SqlDelight
Copy link
Collaborator

@JakeWharton JakeWharton left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have no context on this, but if it unblocks upgrading I'm all for it.

@JakeWharton JakeWharton merged commit bc209cc into sqldelight:master Nov 12, 2025
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants