matrei commented on code in PR #15799:
URL: https://github.com/apache/grails-core/pull/15799#discussion_r3503549005
##########
grails-core/src/test/groovy/org/grails/compiler/injection/GlobalGrailsClassInjectorTransformationSpec.groovy:
##########
@@ -122,4 +123,104 @@ class FooGrailsPlugin {
xml.resources.resource.size() == 2
xml.resources.resource.text() == "FooBar"
}
+
+ void "isIsolatedBuild reflects the grails.isolated.build system
property"() {
+ given:
+ String original = System.getProperty('grails.isolated.build')
+
+ when:
+ System.setProperty('grails.isolated.build', value)
+
+ then:
+ GlobalGrailsClassInjectorTransformation.isIsolatedBuild() ==
expected
+
+ cleanup:
+ if (original != null) {
+ System.setProperty('grails.isolated.build', original)
+ } else {
+ System.clearProperty('grails.isolated.build')
+ }
+
+ where:
+ value || expected
+ 'true' || true
+ 'false' || false
+ 'TRUE' || true
+ 'yes' || false
+ }
Review Comment:
```suggestion
@RestoreSystemProperties
void "isIsolatedBuild reflects the grails.isolated.build system
property"() {
when:
System.setProperty('grails.isolated.build', value)
then:
GlobalGrailsClassInjectorTransformation.isIsolatedBuild() ==
expected
where:
value || expected
'true' || true
'false' || false
'TRUE' || true
'y' || true
'null' || false
'1' || true
'0' || false
'-1' || false
'' || false
}
```
##########
grails-core/src/test/groovy/org/grails/compiler/injection/GlobalGrailsClassInjectorTransformationSpec.groovy:
##########
@@ -122,4 +123,104 @@ class FooGrailsPlugin {
xml.resources.resource.size() == 2
xml.resources.resource.text() == "FooBar"
}
+
+ void "isIsolatedBuild reflects the grails.isolated.build system
property"() {
+ given:
+ String original = System.getProperty('grails.isolated.build')
+
+ when:
+ System.setProperty('grails.isolated.build', value)
+
+ then:
+ GlobalGrailsClassInjectorTransformation.isIsolatedBuild() ==
expected
+
+ cleanup:
+ if (original != null) {
+ System.setProperty('grails.isolated.build', original)
+ } else {
+ System.clearProperty('grails.isolated.build')
+ }
+
+ where:
+ value || expected
+ 'true' || true
+ 'false' || false
+ 'TRUE' || true
+ 'yes' || false
+ }
+
+ private SourceUnit sourceUnitWithTarget(File targetDirectory) {
+ def configuration = new CompilerConfiguration()
+ configuration.setTargetDirectory((File) targetDirectory)
+ Stub(SourceUnit) {
+ getConfiguration() >> configuration
+ getName() >> 'TestSource'
+ }
+ }
+
+ void "resolveCompilationTargetDirectory returns the configured target
directory"() {
+ given:
+ File target = new File(System.getProperty('java.io.tmpdir'),
'isolated-target/build/classes/groovy/main')
+ def source = sourceUnitWithTarget(target)
+
+ expect: "the configured directory is used regardless of build
isolation"
+
GlobalGrailsClassInjectorTransformation.resolveCompilationTargetDirectory(source,
false) == target
+
GlobalGrailsClassInjectorTransformation.resolveCompilationTargetDirectory(source,
true) == target
+ }
+
+ void "resolveCompilationTargetDirectory falls back to the shared relative
path for a non-isolated build"() {
+ given: "a source unit without a configured target directory"
+ def source = sourceUnitWithTarget(null)
+
+ when:
+ File resolved =
GlobalGrailsClassInjectorTransformation.resolveCompilationTargetDirectory(source,
false)
+
+ then: "the legacy relative fallback is used"
+ resolved == new File('build/classes/main')
+ }
+
+ void "resolveCompilationTargetDirectory fails fast instead of falling back
for an isolated build"() {
+ given: "a source unit without a configured target directory"
+ def source = sourceUnitWithTarget(null)
+
+ when: "the target directory cannot be resolved in an isolated build"
+
GlobalGrailsClassInjectorTransformation.resolveCompilationTargetDirectory(source,
true)
+
+ then: "the build fails loudly rather than writing to a shared location"
+ IllegalStateException e = thrown()
+ e.message.contains('GRAILS_ISOLATED_BUILD=true')
+ }
+
+ void "findSourceDirectory prefers the per-project base.dir system property
when set"() {
+ given: "base.dir points at an existing directory"
+ File baseDir = File.createTempDir()
+ System.setProperty('base.dir', baseDir.absolutePath)
+ File target = new File(baseDir, 'build/classes/groovy/main')
+
+ when:
+ File resolved =
GlobalGrailsClassInjectorTransformation.findSourceDirectory(target)
+
+ then: "the build-tool supplied base.dir wins"
+ resolved == baseDir
+
+ cleanup:
+ System.clearProperty('base.dir')
+ baseDir.deleteDir()
+ }
+
+ void "findSourceDirectory walks up to the project directory when base.dir
is not set"() {
+ given: "no base.dir and a standard per-project compile target"
+ System.clearProperty('base.dir')
Review Comment:
```suggestion
```
##########
grails-core/src/main/groovy/org/grails/compiler/injection/GlobalGrailsClassInjectorTransformation.groovy:
##########
@@ -176,14 +176,43 @@ class GlobalGrailsClassInjectorTransformation implements
ASTTransformation, Comp
generatePluginXml(pluginClassNode, pluginVersion, transformedClasses,
pluginXmlFile)
}
+ /**
+ * The system property signalling that each project compiles into its own
isolated output
+ * directory. When set, the transform must never fall back to a
shared/guessed location, which can
+ * leak one module's generated metadata into another.
+ */
+ public static final String ISOLATED_BUILD_PROPERTY =
'grails.isolated.build'
+
+ /**
+ * @return {@code true} when the {@code grails.isolated.build} system
property is {@code true}.
+ */
+ static boolean isIsolatedBuild() {
+ return
Boolean.parseBoolean(System.getProperty(ISOLATED_BUILD_PROPERTY))
Review Comment:
```suggestion
System.getProperty(ISOLATED_BUILD_PROPERTY, 'false').toBoolean()
```
##########
grails-core/src/test/groovy/org/grails/compiler/injection/GlobalGrailsClassInjectorTransformationSpec.groovy:
##########
@@ -122,4 +123,104 @@ class FooGrailsPlugin {
xml.resources.resource.size() == 2
xml.resources.resource.text() == "FooBar"
}
+
+ void "isIsolatedBuild reflects the grails.isolated.build system
property"() {
+ given:
+ String original = System.getProperty('grails.isolated.build')
+
+ when:
+ System.setProperty('grails.isolated.build', value)
+
+ then:
+ GlobalGrailsClassInjectorTransformation.isIsolatedBuild() ==
expected
+
+ cleanup:
+ if (original != null) {
+ System.setProperty('grails.isolated.build', original)
+ } else {
+ System.clearProperty('grails.isolated.build')
+ }
+
+ where:
+ value || expected
+ 'true' || true
+ 'false' || false
+ 'TRUE' || true
+ 'yes' || false
+ }
+
+ private SourceUnit sourceUnitWithTarget(File targetDirectory) {
+ def configuration = new CompilerConfiguration()
+ configuration.setTargetDirectory((File) targetDirectory)
+ Stub(SourceUnit) {
+ getConfiguration() >> configuration
+ getName() >> 'TestSource'
+ }
+ }
+
+ void "resolveCompilationTargetDirectory returns the configured target
directory"() {
+ given:
+ File target = new File(System.getProperty('java.io.tmpdir'),
'isolated-target/build/classes/groovy/main')
+ def source = sourceUnitWithTarget(target)
+
+ expect: "the configured directory is used regardless of build
isolation"
+
GlobalGrailsClassInjectorTransformation.resolveCompilationTargetDirectory(source,
false) == target
+
GlobalGrailsClassInjectorTransformation.resolveCompilationTargetDirectory(source,
true) == target
+ }
+
+ void "resolveCompilationTargetDirectory falls back to the shared relative
path for a non-isolated build"() {
+ given: "a source unit without a configured target directory"
+ def source = sourceUnitWithTarget(null)
+
+ when:
+ File resolved =
GlobalGrailsClassInjectorTransformation.resolveCompilationTargetDirectory(source,
false)
+
+ then: "the legacy relative fallback is used"
+ resolved == new File('build/classes/main')
+ }
+
+ void "resolveCompilationTargetDirectory fails fast instead of falling back
for an isolated build"() {
+ given: "a source unit without a configured target directory"
+ def source = sourceUnitWithTarget(null)
+
+ when: "the target directory cannot be resolved in an isolated build"
+
GlobalGrailsClassInjectorTransformation.resolveCompilationTargetDirectory(source,
true)
+
+ then: "the build fails loudly rather than writing to a shared location"
+ IllegalStateException e = thrown()
+ e.message.contains('GRAILS_ISOLATED_BUILD=true')
+ }
+
+ void "findSourceDirectory prefers the per-project base.dir system property
when set"() {
Review Comment:
```suggestion
@RestoreSystemProperties
void "findSourceDirectory prefers the per-project base.dir system
property when set"() {
```
##########
grails-core/src/test/groovy/org/grails/compiler/injection/GlobalGrailsClassInjectorTransformationSpec.groovy:
##########
@@ -122,4 +123,104 @@ class FooGrailsPlugin {
xml.resources.resource.size() == 2
xml.resources.resource.text() == "FooBar"
}
+
+ void "isIsolatedBuild reflects the grails.isolated.build system
property"() {
+ given:
+ String original = System.getProperty('grails.isolated.build')
+
+ when:
+ System.setProperty('grails.isolated.build', value)
+
+ then:
+ GlobalGrailsClassInjectorTransformation.isIsolatedBuild() ==
expected
+
+ cleanup:
+ if (original != null) {
+ System.setProperty('grails.isolated.build', original)
+ } else {
+ System.clearProperty('grails.isolated.build')
+ }
+
+ where:
+ value || expected
+ 'true' || true
+ 'false' || false
+ 'TRUE' || true
+ 'yes' || false
+ }
+
+ private SourceUnit sourceUnitWithTarget(File targetDirectory) {
+ def configuration = new CompilerConfiguration()
+ configuration.setTargetDirectory((File) targetDirectory)
+ Stub(SourceUnit) {
+ getConfiguration() >> configuration
+ getName() >> 'TestSource'
+ }
+ }
+
+ void "resolveCompilationTargetDirectory returns the configured target
directory"() {
+ given:
+ File target = new File(System.getProperty('java.io.tmpdir'),
'isolated-target/build/classes/groovy/main')
+ def source = sourceUnitWithTarget(target)
+
+ expect: "the configured directory is used regardless of build
isolation"
+
GlobalGrailsClassInjectorTransformation.resolveCompilationTargetDirectory(source,
false) == target
+
GlobalGrailsClassInjectorTransformation.resolveCompilationTargetDirectory(source,
true) == target
+ }
+
+ void "resolveCompilationTargetDirectory falls back to the shared relative
path for a non-isolated build"() {
+ given: "a source unit without a configured target directory"
+ def source = sourceUnitWithTarget(null)
+
+ when:
+ File resolved =
GlobalGrailsClassInjectorTransformation.resolveCompilationTargetDirectory(source,
false)
+
+ then: "the legacy relative fallback is used"
+ resolved == new File('build/classes/main')
+ }
+
+ void "resolveCompilationTargetDirectory fails fast instead of falling back
for an isolated build"() {
+ given: "a source unit without a configured target directory"
+ def source = sourceUnitWithTarget(null)
+
+ when: "the target directory cannot be resolved in an isolated build"
+
GlobalGrailsClassInjectorTransformation.resolveCompilationTargetDirectory(source,
true)
+
+ then: "the build fails loudly rather than writing to a shared location"
+ IllegalStateException e = thrown()
+ e.message.contains('GRAILS_ISOLATED_BUILD=true')
+ }
+
+ void "findSourceDirectory prefers the per-project base.dir system property
when set"() {
+ given: "base.dir points at an existing directory"
+ File baseDir = File.createTempDir()
+ System.setProperty('base.dir', baseDir.absolutePath)
+ File target = new File(baseDir, 'build/classes/groovy/main')
+
+ when:
+ File resolved =
GlobalGrailsClassInjectorTransformation.findSourceDirectory(target)
+
+ then: "the build-tool supplied base.dir wins"
+ resolved == baseDir
+
+ cleanup:
+ System.clearProperty('base.dir')
Review Comment:
```suggestion
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
