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.

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

Fix some more IP problems in Gradle build #29267

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -26,11 +26,11 @@ import org.gradle.kotlin.dsl.*


// `generatePrecompiledScriptPluginAccessors` task invokes this method without `gradle.build-environment` applied
fun Project.getBuildEnvironmentExtension(): BuildEnvironmentExtension? = extensions.findByType(BuildEnvironmentExtension::class.java)
fun Project.getBuildEnvironmentExtension(): BuildEnvironmentExtension = extensions.getByType(BuildEnvironmentExtension::class.java)

fun Project.getBuildEnvironmentExtensionOrNull(): BuildEnvironmentExtension? = extensions.findByType(BuildEnvironmentExtension::class.java)

fun Project.repoRoot(): Directory = getBuildEnvironmentExtension()?.repoRoot?.get() ?: layout.projectDirectory.parentOrRoot()

fun Project.repoRoot(): Directory = getBuildEnvironmentExtensionOrNull()?.repoRoot?.get() ?: layout.projectDirectory.parentOrRoot()

fun Directory.parentOrRoot(): Directory = if (this.file("version.txt").asFile.exists()) {
this
Expand All @@ -50,10 +50,10 @@ fun Project.releasedVersionsFile() = repoRoot().file("released-versions.json")
/**
* We use command line Git instead of JGit, because JGit's `Repository.resolve` does not work with worktrees.
*/
fun Project.currentGitBranchViaFileSystemQuery(): Provider<String> = getBuildEnvironmentExtension()?.gitBranch ?: objects.property(String::class.java)
fun Project.currentGitBranchViaFileSystemQuery(): Provider<String> = getBuildEnvironmentExtensionOrNull()?.gitBranch ?: objects.property(String::class.java)


fun Project.currentGitCommitViaFileSystemQuery(): Provider<String> = getBuildEnvironmentExtension()?.gitCommitId ?: objects.property(String::class.java)
fun Project.currentGitCommitViaFileSystemQuery(): Provider<String> = getBuildEnvironmentExtensionOrNull()?.gitCommitId ?: objects.property(String::class.java)


// pre-test/master/queue/alice/feature -> master
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,11 +20,31 @@ import gradlebuild.basics.BuildEnvironmentService
with(layout.rootDirectory) {
gradle.lifecycle.beforeProject {
val service = gradle.sharedServices.registerIfAbsent("buildEnvironmentService", BuildEnvironmentService::class) {
assert(project.path == ":") {
// We rely on the fact that root is configured first
"BuildEnvironmentService should be registered by the root"
}
parameters.rootProjectDir = this@with
parameters.rootProjectBuildDir = project.layout.buildDirectory
parameters.artifactoryUserName = providers.gradleProperty("artifactoryUserName")
parameters.artifactoryPassword = providers.gradleProperty("artifactoryPassword")
parameters.testVersions = providers.gradleProperty("testVersions")
parameters.integtestAgentAllowed = providers.gradleProperty("org.gradle.integtest.agent.allowed")
parameters.integtestDebug = providers.gradleProperty("org.gradle.integtest.debug")
parameters.integtestLauncherDebug = providers.gradleProperty("org.gradle.integtest.launcher.debug")
parameters.integtestVerbose = providers.gradleProperty("org.gradle.integtest.verbose")
}
val buildEnvironmentExtension = extensions.create("buildEnvironment", BuildEnvironmentExtension::class)
buildEnvironmentExtension.gitCommitId = service.flatMap { it.gitCommitId }
buildEnvironmentExtension.gitBranch = service.flatMap { it.gitBranch }
buildEnvironmentExtension.repoRoot = this@with
buildEnvironmentExtension.rootProjectBuildDir = service.flatMap { it.parameters.rootProjectBuildDir }
buildEnvironmentExtension.artifactoryUserName = service.flatMap { it.parameters.artifactoryUserName }
buildEnvironmentExtension.artifactoryPassword = service.flatMap { it.parameters.artifactoryPassword }
buildEnvironmentExtension.testVersions = service.flatMap { it.parameters.testVersions }
buildEnvironmentExtension.integtestAgentAllowed = service.flatMap { it.parameters.integtestAgentAllowed }
buildEnvironmentExtension.integtestDebug = service.flatMap { it.parameters.integtestDebug }
buildEnvironmentExtension.integtestLauncherDebug = service.flatMap { it.parameters.integtestLauncherDebug }
buildEnvironmentExtension.integtestVerbose = service.flatMap { it.parameters.integtestVerbose }
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -24,4 +24,12 @@ interface BuildEnvironmentExtension {
val gitCommitId: Property<String>
val gitBranch: Property<String>
val repoRoot: DirectoryProperty
val rootProjectBuildDir: DirectoryProperty
val artifactoryUserName: Property<String>
val artifactoryPassword: Property<String>
Copy link
Member

Choose a reason for hiding this comment

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

❓ Can we avoid storing passwords in Properties here?

Copy link
Member Author

@asodja asodja May 23, 2024

Choose a reason for hiding this comment

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

I guess we cound store them in env variables and read env variables here:

val artifactoryUserName
get() = getBuildEnvironmentExtension().artifactoryUserName.orNull
val artifactoryUserPassword
get() = getBuildEnvironmentExtension().artifactoryPassword.orNull

Would that be a better solution?

We publish only from CI right? So then we also need to pass these variables differently in teamcity builds.

Copy link
Member

Choose a reason for hiding this comment

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

The password is already available in an environment variable

password("env.ORG_GRADLE_PROJECT_artifactoryUserPassword", "%gradle.internal.repository.build-tool.publish.password%")

AFAICT we could reference that as we only need the password if we actually publish, correct me if I'm wrong @blindpirate

Copy link
Collaborator

Choose a reason for hiding this comment

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

Note that ORG_GRADLE_PROJECT_artifactoryUserPassword actually injects a gradle property, i.e. it equals -PartifactoryUserPassword=X. See https://docs.gradle.org/current/userguide/build_environment.html#sec:project_properties

So we can get the property via gradle property, not env variable.

val testVersions: Property<String>
val integtestAgentAllowed: Property<String>
val integtestDebug: Property<String>
val integtestLauncherDebug: Property<String>
val integtestVerbose: Property<String>
}
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
package gradlebuild.basics

import org.gradle.api.file.DirectoryProperty
import org.gradle.api.provider.Property
import org.gradle.api.provider.Provider
import org.gradle.api.provider.ProviderFactory
import org.gradle.api.services.BuildService
Expand All @@ -29,6 +30,14 @@ abstract class BuildEnvironmentService : BuildService<BuildEnvironmentService.Pa

interface Parameters : BuildServiceParameters {
val rootProjectDir: DirectoryProperty
val rootProjectBuildDir: DirectoryProperty
val artifactoryUserName: Property<String>
val artifactoryPassword: Property<String>
Copy link
Member

Choose a reason for hiding this comment

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

❓ Can we avoid storing passwords in Properties here?

val testVersions: Property<String>
val integtestAgentAllowed: Property<String>
val integtestDebug: Property<String>
val integtestLauncherDebug: Property<String>
val integtestVerbose: Property<String>
}

@get:Inject
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ def DOCUMENTATION_ATTRIBUTE = objects.named(Category, Category.DOCUMENTATION)
def SOURCES_ATTRIBUTE = objects.named(DocsType, "gradle-source-folders")

configurations {
baseline
def baseline = baseline {}
baselineClasspath {
extendsFrom baseline
attributes.attribute(ARTIFACT_TYPE, 'gradle-classpath')
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
package gradlebuild.integrationtests

import gradlebuild.basics.capitalize
import gradlebuild.basics.getBuildEnvironmentExtension
import gradlebuild.basics.repoRoot
import gradlebuild.basics.testSplitExcludeTestClasses
import gradlebuild.basics.testSplitIncludeTestClasses
Expand Down Expand Up @@ -195,9 +196,10 @@ fun IntegrationTest.setUpAgentIfNeeded(testType: TestType, executer: String) {
})
}

val environmentExtension = project.getBuildEnvironmentExtension()
val integTestUseAgentSysPropName = "org.gradle.integtest.agent.allowed"
if (project.hasProperty(integTestUseAgentSysPropName)) {
val shouldUseAgent = (project.property(integTestUseAgentSysPropName) as? String).toBoolean()
if (environmentExtension.integtestAgentAllowed.isPresent) {
val shouldUseAgent = environmentExtension.integtestAgentAllowed.get().toBoolean()
systemProperties[integTestUseAgentSysPropName] = shouldUseAgent.toString()
}
}
Expand All @@ -206,26 +208,28 @@ fun IntegrationTest.setUpAgentIfNeeded(testType: TestType, executer: String) {
private
fun IntegrationTest.addDebugProperties() {
// TODO Move magic property out
if (project.hasProperty("org.gradle.integtest.debug")) {
val environmentExtension = project.getBuildEnvironmentExtension()
if (environmentExtension.integtestDebug.isPresent) {
systemProperties["org.gradle.integtest.debug"] = "true"
testLogging.showStandardStreams = true
}
// TODO Move magic property out
if (project.hasProperty("org.gradle.integtest.verbose")) {
if (environmentExtension.integtestVerbose.isPresent) {
testLogging.showStandardStreams = true
}
// TODO Move magic property out
if (project.hasProperty("org.gradle.integtest.launcher.debug")) {
if (environmentExtension.integtestLauncherDebug.isPresent) {
systemProperties["org.gradle.integtest.launcher.debug"] = "true"
}
}


fun DistributionTest.setSystemPropertiesOfTestJVM(defaultVersions: String) {
// use -PtestVersions=all or -PtestVersions=1.2,1.3…
val extension = project.getBuildEnvironmentExtension()
val integTestVersionsSysProp = "org.gradle.integtest.versions"
if (project.hasProperty("testVersions")) {
systemProperties[integTestVersionsSysProp] = project.property("testVersions")
if (extension.testVersions.isPresent) {
systemProperties[integTestVersionsSysProp] = extension.testVersions.get()
} else {
systemProperties[integTestVersionsSysProp] = defaultVersions
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import gradlebuild.basics.maxTestDistributionPartitionSecond
import gradlebuild.basics.maxTestDistributionRemoteExecutors
import gradlebuild.basics.predictiveTestSelectionEnabled
import gradlebuild.basics.rerunAllTests
import gradlebuild.basics.buildRunningOnCi
import gradlebuild.basics.testDistributionEnabled
import gradlebuild.basics.testJavaVendor
import gradlebuild.basics.testJavaVersion
Expand Down Expand Up @@ -306,7 +307,7 @@ fun configureTests() {

fun removeTeamcityTempProperty() {
// Undo: https://github.com/JetBrains/teamcity-gradle/blob/e1dc98db0505748df7bea2e61b5ee3a3ba9933db/gradle-runner-agent/src/main/scripts/init.gradle#L818
if (project.hasProperty("teamcity")) {
if (project.buildRunningOnCi.get() && project.hasProperty("teamcity")) {
@Suppress("UNCHECKED_CAST") val teamcity = project.property("teamcity") as MutableMap<String, Any>
teamcity["teamcity.build.tempDir"] = ""
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import gradlebuild.basics.buildBranch
import gradlebuild.basics.buildCommitId
import gradlebuild.basics.capitalize
import gradlebuild.basics.defaultPerformanceBaselines
import gradlebuild.basics.getBuildEnvironmentExtension
import gradlebuild.basics.includePerformanceTestScenarios
import gradlebuild.basics.logicalBranch
import gradlebuild.basics.performanceBaselines
Expand Down Expand Up @@ -310,7 +311,7 @@ class PerformanceTestPlugin : Plugin<Project> {
// determineBaselines.determinedBaselines -> buildCommitDistribution.baselines
val determineBaselines = tasks.register("determineBaselines", DetermineBaselines::class, false)
val buildCommitDistribution = tasks.register("buildCommitDistribution", BuildCommitDistribution::class)
val buildCommitDistributionsDir = project.rootProject.layout.buildDirectory.dir("commit-distributions")
val buildCommitDistributionsDir = project.getBuildEnvironmentExtension().rootProjectBuildDir.dir("commit-distributions")

determineBaselines.configure {
configuredBaselines = extension.baselines
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
* limitations under the License.
*/

import gradlebuild.basics.getBuildEnvironmentExtension
import org.gradle.external.javadoc.StandardJavadocDocletOptions
import java.time.Year

Expand All @@ -29,10 +30,10 @@ val artifactoryUrl
get() = System.getenv("GRADLE_INTERNAL_REPO_URL") ?: ""

val artifactoryUserName
get() = findProperty("artifactoryUserName") as String?
get() = getBuildEnvironmentExtension().artifactoryUserName.orNull

val artifactoryUserPassword
get() = findProperty("artifactoryUserPassword") as String?
get() = getBuildEnvironmentExtension().artifactoryPassword.orNull

publishing {
publications {
Expand Down
20 changes: 10 additions & 10 deletions platforms/documentation/docs/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -109,8 +109,8 @@ tasks.named("stageDocs") {

samples {
templates {
javaAndroidApplication
structuringSoftwareProjects
javaAndroidApplication {}
structuringSoftwareProjects {}
springBootWebApplication {
target = "app"
}
Expand All @@ -121,11 +121,11 @@ samples {
sourceDirectory = gradlePluginInJava.sourceDirectory
target = "buildSrc"
}
buildSrcPluginJavaModuleTransform
buildSrcPluginJavaModuleTransform {}

javaApplication
javaListLibrary
javaUtilitiesLibrary
javaApplication {}
javaListLibrary {}
javaUtilitiesLibrary {}
javaListLibraryInMyLibrary {
sourceDirectory = javaListLibrary.sourceDirectory
target = "my-library"
Expand Down Expand Up @@ -179,8 +179,8 @@ samples {
target = "application"
}

groovyListLibrary
groovyUtilitiesLibrary
groovyListLibrary {}
groovyUtilitiesLibrary {}
groovyListLibraryInMyLibrary {
sourceDirectory = groovyListLibrary.sourceDirectory
target = "my-library"
Expand All @@ -190,7 +190,7 @@ samples {
target = "my-library"
}

projectInfoPlugin
projectInfoPlugin {}

precompiledScriptPluginUtils {
target = "convention-plugins"
Expand All @@ -199,7 +199,7 @@ samples {
sourceDirectory = precompiledScriptPluginUtils.sourceDirectory
target = "buildSrc"
}
problemsApiUsage
problemsApiUsage {}
}

// TODO: Do this lazily so we don't need to walk the filesystem during configuration
Expand Down
Loading