Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add missing input for CopyJsp and missing onlyIf config for WriteDependenciesFile #213

Merged
merged 4 commits into from
Jul 18, 2024
Merged

Add missing input for CopyJsp and missing onlyIf config for WriteDependenciesFile #213

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
2567fb3
Add missing InputDirectory property for `copyJsp` task
labkey-susanh Jul 17, 2024
e83afed
Add missing `onlyIf` condition for `WriteDependenciesFile`
labkey-susanh Jul 18, 2024
571e0c3
Move onlyIf config inside task class
labkey-susanh Jul 18, 2024
894bae6
Prepare for merge
labkey-susanh Jul 18, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,12 @@ on how to do that, including how to develop and test locally and the versioning

_Note: 1.28.0 and later require Gradle 7_

### TBD
*Released*: TBD
(Earliest compatible LabKey version: 24.8)
- Fix issue with JSP copying because of missing input directory
- Add missing `onlyIf` condition for `WriteDependenciesFile`

### 3.0.0
*Released*: 15 July 2024
(Earliest compatible LabKey version: 24.8)
Expand Down
2 changes: 1 addition & 1 deletion build.gradle
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ dependencies {
}

group 'org.labkey.build'
project.version = "3.1.0-SNAPSHOT"
project.version = "3.1.0-fixCopyJsp-SNAPSHOT"

gradlePlugin {
plugins {
Expand Down
5 changes: 4 additions & 1 deletion src/main/groovy/org/labkey/gradle/plugin/ModuleResources.groovy
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,10 +41,13 @@ class ModuleResources
Provider<Set<ResolvedArtifactResult>> artifacts = config.getIncoming().getArtifacts().getResolvedArtifacts();
task.getArtifactIds().set(artifacts.map(new WriteDependenciesFile.IdExtractor()))
}
task.externalDependencies.set(project.extensions.findByType(ModuleExtension.class).getExternalDependencies())
task.onlyIf {
return !task.externalDependencies.get().isEmpty()
}
} catch (UnknownDomainObjectException ignore) {

}
Copy link
Member

Choose a reason for hiding this comment

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

Would an UnknownDomainObjectException cause this task to run when it shouldn't since task.onlyIf {...} won't be reached?
Should the onlyIf just be baked into WriteDependenciesFile, similar to InstallRPackage?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Technically, that's two questions. 😄, but they are both good questions, so I'll let it slide.

Effectively, it won't happen anymore that the exception is thrown since the configuration is created more broadly, but still a good idea to check. I had assumed Gradle wouldn't like the onlyIf inside the class, but that's cleaner, so I'll try it.

Copy link
Member

Choose a reason for hiding this comment

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

There is no escape from off-by-one errors 😰

task.externalDependencies.set(project.extensions.findByType(ModuleExtension.class).getExternalDependencies())
}

project.tasks.named("processModuleResources").configure {dependsOn(project.tasks.named('writeDependenciesList'))}
Expand Down
6 changes: 5 additions & 1 deletion src/main/groovy/org/labkey/gradle/task/CopyJsp.groovy
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package org.labkey.gradle.task
import org.gradle.api.DefaultTask
import org.gradle.api.file.DirectoryProperty
import org.gradle.api.file.FileSystemOperations
import org.gradle.api.tasks.InputDirectory
import org.gradle.api.tasks.OutputDirectory
import org.gradle.api.tasks.TaskAction

Expand All @@ -14,6 +15,9 @@ abstract class CopyJsp extends DefaultTask

@Inject abstract FileSystemOperations getFs()

@InputDirectory
final abstract DirectoryProperty srcDir = project.objects.directoryProperty().convention(project.layout.projectDirectory.dir('src'))

@OutputDirectory
final abstract DirectoryProperty webappDir = project.objects.directoryProperty().convention(project.layout.buildDirectory.dir(WEBAPP_DIR).get())

Expand All @@ -23,7 +27,7 @@ abstract class CopyJsp extends DefaultTask
it.delete(webappDir.get().dir("org"))
}
fs.copy {
from 'src'
from srcDir.get()
into webappDir.get()
include '**/*.jsp'
}
Expand Down
6 changes: 3 additions & 3 deletions src/main/groovy/org/labkey/gradle/task/WriteDependenciesFile.groovy
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,9 +61,6 @@ abstract class WriteDependenciesFile extends DefaultTask

private void writeDependencies(OutputStreamWriter writer)
{
if (externalDependencies.get().isEmpty())
return

List<String> missing = []
List<String> licenseMissing = []
Map<String, ExternalDependency> dependencies = externalDependencies.get()
Expand Down Expand Up @@ -109,6 +106,9 @@ abstract class WriteDependenciesFile extends DefaultTask

void writeJarsTxt()
{
if (externalDependencies.get().isEmpty())
return

OutputStreamWriter writer = null
try {
writer = new OutputStreamWriter(new FileOutputStream(jarsTxtFile.get().asFile), StandardCharsets.UTF_8)
Expand Down