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

Jump to bottom

Upgrade Groovydoc #30066

Merged
merged 1 commit into from
Aug 22, 2024
Merged

Upgrade Groovydoc #30066

merged 1 commit into from
Aug 22, 2024

Conversation

asodja
Copy link
Member

@asodja asodja commented Jul 31, 2024 •
edited
Loading

Except classpath due to #30273

@asodja asodja added a:chore Minor issue without significant impact p:lazy-migration Issues covered by migration to an all-lazy API labels Jul 31, 2024
@asodja

This comment has been minimized.

Sign in to view
@bot-gradle

This comment has been minimized.

Sign in to view
@asodja asodja force-pushed the provider-api-migration/public-api-changes branch 2 times, most recently from c7c33b8 to d066169 Compare August 1, 2024 13:15
@asodja asodja force-pushed the provider-api-migration/public-api-changes branch from d066169 to 7b0a8b6 Compare August 2, 2024 09:20
@asodja

This comment has been minimized.

Sign in to view
@bot-gradle

This comment has been minimized.

Sign in to view
@asodja

This comment has been minimized.

Sign in to view
@bot-gradle

This comment has been minimized.

Sign in to view
@asodja
Copy link
Member Author

asodja commented Aug 2, 2024

There seems to be an issue with one test:

gradle/platforms/jvm/plugins-groovy/src/integTest/groovy/org/gradle/groovy/GroovyBasePluginIntegrationTest.groovy

Lines 60 to 94 in 983bea4

def "only resolves source class path feeding into inferred Groovy class path if/when the latter is actually used (but not during autowiring)"() {
file("build.gradle") << """
apply plugin: "groovy-base"
sourceSets {
custom
}
${mavenCentralRepository()}
dependencies {
customImplementation "org.codehaus.groovy:groovy-all:2.4.10"
}
task groovydoc(type: Groovydoc) {
classpath = sourceSets.custom.runtimeClasspath
}
task verify {
def customCompileClasspathState = provider {
configurations.customCompileClasspath.state.toString()
}
def customRuntimeClasspathState = provider {
configurations.customRuntimeClasspath.state.toString()
}
doLast {
assert customCompileClasspathState.get() == "UNRESOLVED"
assert customRuntimeClasspathState.get() == "UNRESOLVED"
}
}
"""
expect:
succeeds("verify")
}

That verifies that configuration passed to classpath is not resolved:

task groovydoc(type: Groovydoc) {
    classpath = sourceSets.custom.runtimeClasspath
}

This worked when we had a setter but once I migrated classpath to ConfigurableFileCollection it starts failing.

Interestingly, changing code to:

task groovydoc(type: Groovydoc) {
    classpath.setFrom(sourceSets.custom.runtimeClasspath)
}

fixes issue. This looks related to

gradle/platforms/core-configuration/file-collections/src/main/java/org/gradle/api/internal/file/collections/DefaultConfigurableFileCollection.java

Lines 193 to 218 in 99560e1

fileCollection.visitStructure(new FileCollectionStructureVisitor() {
@Override
public boolean startVisit(FileCollectionInternal.Source source, FileCollectionInternal fileCollection) {
if (DefaultConfigurableFileCollection.this == fileCollection) {
throw new UnsupportedOperationException("Self-referencing ConfigurableFileCollections are not supported. Use the from() method to add to a ConfigurableFileCollection.");
}
return true;
}
@Override
public VisitType prepareForVisit(Source source) {
return VisitType.NoContents;
}
@Override
public void visitCollection(Source source, Iterable<File> contents) {
}
@Override
public void visitFileTree(File root, PatternSet patterns, FileTreeInternal fileTree) {
}
@Override
public void visitFileTreeBackedByFile(File file, FileTreeInternal fileTree, FileSystemMirroringFileTree sourceTree) {
}
});

@lptr lptr force-pushed the provider-api-migration/public-api-changes branch 7 times, most recently from 6a9e20e to fd4f94e Compare August 14, 2024 08:19
@asodja asodja force-pushed the provider-api-migration/public-api-changes branch from fd4f94e to 65c996e Compare August 20, 2024 13:11
@asodja asodja force-pushed the asodja/upgrade-groovydoc branch 2 times, most recently from cba5228 to 78a3575 Compare August 20, 2024 14:11
@asodja asodja force-pushed the provider-api-migration/public-api-changes branch 3 times, most recently from 0682ac8 to 9c6458a Compare August 22, 2024 06:58
@asodja asodja force-pushed the asodja/upgrade-groovydoc branch 2 times, most recently from aa2f4f6 to b06304d Compare August 22, 2024 09:20
@asodja
Copy link
Member Author

asodja commented Aug 22, 2024

@bot-gradle test this

@bot-gradle
Copy link
Collaborator

I've triggered the following builds for you. Click here to see all build failures.

@asodja
Copy link
Member Author

asodja commented Aug 22, 2024

@bot-gradle test this

@bot-gradle
Copy link
Collaborator

I've triggered the following builds for you. Click here to see all build failures.

@asodja
Upgrade Groovydoc to use lazy properties
7651df7 
Except for classpath due to issue #30273.
@asodja asodja marked this pull request as ready for review August 22, 2024 14:09
@asodja asodja requested review from a team as code owners August 22, 2024 14:10
@asodja asodja merged commit 5b33aba into provider-api-migration/public-api-changes Aug 22, 2024
3 checks passed
@asodja asodja deleted the asodja/upgrade-groovydoc branch August 22, 2024 14:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@octylFractal octylFractal Awaiting requested review from octylFractal octylFractal is a code owner automatically assigned from gradle/bt-jvm

Assignees
No one assigned
Labels
a:chore Minor issue without significant impact p:lazy-migration Issues covered by migration to an all-lazy API
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@asodja @bot-gradle