TS-47508 Accept a folder for the git-properties-jar option

CHANGELOG.md

@@ -5,6 +5,7 @@ We use [semantic versioning](http://semver.org/):
 - PATCH version when you make backwards compatible bug fixes.
 
 # Next version
+- [feature] _agent_: `git-properties-jar` now also accepts a folder, which is searched for `git.properties` files. Previously, a folder was rejected with a warning and no commit was auto-detected.
 
 # 38.0.0
 - [breaking] _agent_: Log lines now show the simple class name (`INFO Agent - ...`) instead of the fully qualified one.

agent/src/main/kotlin/com/teamscale/jacoco/agent/commit_resolution/git_properties/GitMultiProjectPropertiesLocator.kt

@@ -11,7 +11,7 @@ import java.util.concurrent.Executor
 import java.util.concurrent.Executors
 
 /**
- * Searches a Jar/War/Ear/... file for a git.properties file in order to enable upload for the commit described therein,
+ * Searches a Jar/War/Ear/... file or a folder for a git.properties file in order to enable upload for the commit described therein,
  * e.g. to Teamscale, via a [DelayedTeamscaleMultiProjectUploader]. Specifically, this searches for the
  * 'teamscale.project' property specified in each of the discovered 'git.properties' files.
  */
@@ -37,15 +37,15 @@ class GitMultiProjectPropertiesLocator(
 	)
 
 	/**
-	 * Asynchronously searches the given jar file for git.properties files and adds a corresponding uploader to the
+	 * Asynchronously searches the given jar file or folder for git.properties files and adds a corresponding uploader to the
 	 * multi-project uploader.
 	 */
 	override fun searchFileForGitPropertiesAsync(file: File, isJarFile: Boolean) {
 		executor.execute { searchFile(file, isJarFile) }
 	}
 
 	/**
-	 * Synchronously searches the given jar file for git.properties files and adds a corresponding uploader to the
+	 * Synchronously searches the given jar file or folder for git.properties files and adds a corresponding uploader to the
 	 * multi-project uploader.
 	 */
 	@VisibleForTesting

agent/src/main/kotlin/com/teamscale/jacoco/agent/commit_resolution/git_properties/GitSingleProjectPropertiesLocator.kt

@@ -1,6 +1,7 @@
 package com.teamscale.jacoco.agent.commit_resolution.git_properties
 
 import com.teamscale.jacoco.agent.logging.LoggingUtils.getLogger
+import com.teamscale.jacoco.agent.options.AgentOptions
 import com.teamscale.jacoco.agent.upload.delay.DelayedUploader
 import com.teamscale.jacoco.agent.util.DaemonThreadFactory
 import java.io.File
@@ -10,8 +11,8 @@
 import java.util.concurrent.Executors
 
 /**
- * Searches a Jar/War/Ear/... file for a git.properties file in order to enable upload for the commit described therein,
- * e.g. to Teamscale, via a [DelayedUploader].
+ * Searches a Jar/War/Ear/... file or a folder for a git.properties file in order to enable upload for the commit
+ * described therein, e.g. to Teamscale, via a [DelayedUploader].
  */
 class GitSingleProjectPropertiesLocator<T>(
 	private val uploader: DelayedUploader<T>,
@@ -27,17 +28,17 @@
 ) : IGitPropertiesLocator {
 	private val logger = getLogger(this)
 	private var foundData: T? = null
-	private var jarFileWithGitProperties: File? = null
+	private var fileWithGitProperties: File? = null
 
 	/**
-	 * Asynchronously searches the given jar file for a git.properties file.
+	 * Asynchronously searches the given jar file or folder for a git.properties file.
 	 */
 	override fun searchFileForGitPropertiesAsync(file: File, isJarFile: Boolean) {
 		executor.execute { searchFile(file, isJarFile) }
 	}
 
 	private fun searchFile(file: File, isJarFile: Boolean) {
-		logger.debug("Searching jar file {} for a single git.properties", file)
+		logger.debug("Searching {} for a single git.properties", file)
 		try {
 			val data = dataExtractor.extractData(file, isJarFile, recursiveSearch, gitPropertiesCommitTimeFormat)
 			if (data.isEmpty()) {
@@ -60,9 +61,9 @@
 								" Otherwise, you may" +
 								" be uploading to the wrong project/commit which will result in incorrect coverage data" +
 								" displayed in Teamscale. If you cannot fix the inconsistency, you can manually" +
-								" specify a Jar/War/Ear/... file from which to read the correct git.properties" +
-								" file with the agent's teamscale-git-properties-jar parameter.",
-						jarFileWithGitProperties, foundData, file, data
+								" specify a Jar/War/Ear/... file or folder from which to read the correct git.properties" +
+								" file with the agent's ${AgentOptions.GIT_PROPERTIES_JAR_OPTION} parameter.",
+						fileWithGitProperties, foundData, file, data
 					)
 				}
 				return
@@ -73,7 +74,7 @@
 				dataEntry
 			)
 			foundData = dataEntry
-			jarFileWithGitProperties = file
+			fileWithGitProperties = file
 			uploader.setCommitAndTriggerAsynchronousUpload(dataEntry)
 		} catch (e: IOException) {
 			logger.error(

agent/src/main/kotlin/com/teamscale/jacoco/agent/options/AgentOptions.kt

@@ -56,7 +56,7 @@ import kotlin.math.max
  * Parses agent command line options.
  */
 open class AgentOptions(private val logger: ILogger) {
-	/** See [GIT_PROPERTIES_JAR_OPTION]  */
+	/** See [GIT_PROPERTIES_JAR_OPTION]. May be an archive or a folder.  */
 	@JvmField
 	var gitPropertiesJar: File? = null
 
@@ -446,7 +446,7 @@ open class AgentOptions(private val logger: ILogger) {
 						jar.absolutePath + " for a git.properties file."
 			)
 			artifactoryConfig.commitInfo = ArtifactoryConfig.parseGitProperties(
-				jar, searchGitPropertiesRecursively, gitPropertiesCommitTimeFormat
+				jar, !jar.isDirectory(), searchGitPropertiesRecursively, gitPropertiesCommitTimeFormat
 			)
 		}
 		if (!artifactoryConfig.hasCommitInfo()) {
@@ -472,7 +472,7 @@ open class AgentOptions(private val logger: ILogger) {
 						"auto-detect it by searching the provided " + GIT_PROPERTIES_JAR_OPTION + " at " +
 						jar.absolutePath + " for a git.properties file."
 			)
-			startGitPropertiesSearchInJarFile(uploader, jar)
+			startGitPropertiesSearch(uploader, jar)
 			return uploader
 		}
 
@@ -503,7 +503,7 @@ open class AgentOptions(private val logger: ILogger) {
 						" auto-detect it by searching the provided " + GIT_PROPERTIES_JAR_OPTION + " at " +
 						jar.absolutePath + " for a git.properties file."
 			)
-			startMultiGitPropertiesFileSearchInJarFile(uploader, jar)
+			startMultiGitPropertiesFileSearch(uploader, jar)
 			return uploader
 		}
 		logger.info(
@@ -515,15 +515,15 @@ open class AgentOptions(private val logger: ILogger) {
 		return uploader
 	}
 
-	private fun startGitPropertiesSearchInJarFile(
+	private fun startGitPropertiesSearch(
 		uploader: DelayedUploader<ProjectAndCommit>,
 		gitPropertiesJar: File
 	) {
 		GitSingleProjectPropertiesLocator(
 			uploader, searchGitPropertiesRecursively, gitPropertiesCommitTimeFormat
 		) { file, isJarFile, recursiveSearch, timeFormat ->
 			getProjectRevisionsFromGitProperties(file, isJarFile, recursiveSearch, timeFormat)
-		}.searchFileForGitPropertiesAsync(gitPropertiesJar, true)
+		}.searchFileForGitPropertiesAsync(gitPropertiesJar, !gitPropertiesJar.isDirectory())
 	}
 
 	private fun registerSingleGitPropertiesLocator(
@@ -556,13 +556,13 @@ open class AgentOptions(private val logger: ILogger) {
 			TeamscaleUploader(teamscaleServer, reportFormat)
 		}
 
-	private fun startMultiGitPropertiesFileSearchInJarFile(
+	private fun startMultiGitPropertiesFileSearch(
 		uploader: DelayedTeamscaleMultiProjectUploader,
 		gitPropertiesJar: File
 	) {
 		GitMultiProjectPropertiesLocator(
 			uploader, searchGitPropertiesRecursively, gitPropertiesCommitTimeFormat
-		).searchFileForGitPropertiesAsync(gitPropertiesJar, true)
+		).searchFileForGitPropertiesAsync(gitPropertiesJar, !gitPropertiesJar.isDirectory())
 	}
 
 	private fun registerMultiGitPropertiesLocator(
@@ -718,7 +718,10 @@ open class AgentOptions(private val logger: ILogger) {
 		const val DEFAULT_EXCLUDES =
 			"kotlin.*:shadow.*:com.sun.*:sun.*:org.eclipse.*:org.junit.*:junit.*:org.apache.*:org.slf4j.*:javax.*:org.gradle.*:java.*:org.jboss.*:org.wildfly.*:org.springframework.*:com.fasterxml.*:jakarta.*:org.aspectj.*:org.h2.*:org.hibernate.*:org.assertj.*:org.mockito.*:org.thymeleaf.*"
 
-		/** Option name that allows to specify a jar file that contains the git commit hash in a git.properties file.  */
+		/**
+		 * Option name that allows to specify a jar/war/ear/aar file or a folder that contains the git commit hash in a
+		 * git.properties file.
+		 */
 		const val GIT_PROPERTIES_JAR_OPTION = "git-properties-jar"
 
 		/**

agent/src/main/kotlin/com/teamscale/jacoco/agent/options/AgentOptionsParser.kt

@@ -291,11 +291,11 @@ class AgentOptionsParser @VisibleForTesting internal constructor(
 					"The option " + ArtifactoryConfig.ARTIFACTORY_GIT_PROPERTIES_JAR_OPTION + " is deprecated. It still has an effect, " +
 							"but should be replaced with the equivalent option " + AgentOptions.GIT_PROPERTIES_JAR_OPTION + "."
 				)
-				options.gitPropertiesJar = getGitPropertiesJarFile(value)
+				options.gitPropertiesJar = getGitPropertiesFileOrFolder(value)
 				return true
 			}
 			AgentOptions.GIT_PROPERTIES_JAR_OPTION -> {
-				options.gitPropertiesJar = getGitPropertiesJarFile(value)
+				options.gitPropertiesJar = getGitPropertiesFileOrFolder(value)
 				return true
 			}
 			ArtifactoryConfig.ARTIFACTORY_GIT_PROPERTIES_COMMIT_DATE_FORMAT_OPTION -> {
@@ -393,17 +393,18 @@ class AgentOptionsParser @VisibleForTesting internal constructor(
 		readConfigFromString(options, configuration.profilerConfiguration!!.configurationOptions)
 	}
 
-	private fun getGitPropertiesJarFile(path: String): File? {
-		val jarFile = File(path)
-		if (!jarFile.exists()) {
+	/**
+	 * Resolves the path given for [AgentOptions.GIT_PROPERTIES_JAR_OPTION]. Both an archive (jar/war/ear/aar) and a
+	 * folder are accepted, since the git.properties search handles either. A path that does not exist is not treated
+	 * as a fatal error, as the profiled application should keep running in that case.
+	 */
+	private fun getGitPropertiesFileOrFolder(path: String): File? {
+		val gitPropertiesSearchRoot = File(path)
+		if (!gitPropertiesSearchRoot.exists()) {
 			logger.warn("The path provided with the ${AgentOptions.GIT_PROPERTIES_JAR_OPTION} option does not exist: $path. Continuing without searching it for git.properties files.")
 			return null
 		}
-		if (!jarFile.isFile()) {
-			logger.warn("The path provided with the ${AgentOptions.GIT_PROPERTIES_JAR_OPTION} option is not a regular file (probably a folder instead): $path. Continuing without searching it for git.properties files.")
-			return null
-		}
-		return jarFile
+		return gitPropertiesSearchRoot
 	}
 
 	/**

agent/src/main/kotlin/com/teamscale/jacoco/agent/upload/artifactory/ArtifactoryConfig.kt

@@ -4,6 +4,7 @@ import com.teamscale.jacoco.agent.commit_resolution.git_properties.CommitInfo
 import com.teamscale.jacoco.agent.commit_resolution.git_properties.GitPropertiesLocatorUtils
 import com.teamscale.jacoco.agent.commit_resolution.git_properties.InvalidGitPropertiesException
 import com.teamscale.jacoco.agent.options.AgentOptionParseException
+import com.teamscale.jacoco.agent.options.AgentOptions
 import com.teamscale.jacoco.agent.options.AgentOptionsParser
 import com.teamscale.jacoco.agent.upload.UploaderException
 import com.teamscale.jacoco.agent.upload.artifactory.ArtifactoryConfig.Companion.ARTIFACTORY_API_KEY_OPTION
@@ -171,37 +172,44 @@ class ArtifactoryConfig {
 			}
 		}
 
-		/** Parses the commit information form a git.properties file.  */
+		/**
+		 * Parses the commit information from a git.properties file. The search root is either an archive
+		 * (isJarFile = true) or a folder.
+		 */
 		@Throws(UploaderException::class)
 		fun parseGitProperties(
-			jarFile: File, searchRecursively: Boolean, gitPropertiesCommitTimeFormat: DateTimeFormatter?
+			gitPropertiesSearchRoot: File,
+			isJarFile: Boolean,
+			searchRecursively: Boolean,
+			gitPropertiesCommitTimeFormat: DateTimeFormatter?
 		): CommitInfo? {
 			try {
 				val commitInfo = GitPropertiesLocatorUtils.getCommitInfoFromGitProperties(
-					jarFile,
-					true,
+					gitPropertiesSearchRoot,
+					isJarFile,
 					searchRecursively,
 					gitPropertiesCommitTimeFormat
 				)
 				if (commitInfo.isEmpty()) {
 					throw UploaderException(
-						"Found no git.properties files in $jarFile." +
-								" The 'artifactory-git-properties-jar' option must point to a JAR that contains" +
-								" a git.properties with the commit from which the JAR was built."
+						"Found no git.properties files in $gitPropertiesSearchRoot." +
+								" The '${AgentOptions.GIT_PROPERTIES_JAR_OPTION}' option must point to a" +
+								" Jar/War/Ear/Aar file or a folder that contains a git.properties with the commit from" +
+								" which the profiled code was built."
 					)
 				}
 				if (commitInfo.size > 1) {
 					throw UploaderException(
-						("Found multiple git.properties files in " + jarFile
+						("Found multiple git.properties files in " + gitPropertiesSearchRoot
 								+ ". Uploading to multiple projects is currently not possible with Artifactory. "
 								+ "Please contact CQSE if you need this feature.")
 					)
 				}
 				return commitInfo.firstOrNull()
 			} catch (e: IOException) {
-				throw UploaderException("Could not locate a valid git.properties file in $jarFile", e)
+				throw UploaderException("Could not locate a valid git.properties file in $gitPropertiesSearchRoot", e)
 			} catch (e: InvalidGitPropertiesException) {
-				throw UploaderException("Could not locate a valid git.properties file in $jarFile", e)
+				throw UploaderException("Could not locate a valid git.properties file in $gitPropertiesSearchRoot", e)
 			}
 		}
 	}

agent/src/test/kotlin/com/teamscale/jacoco/agent/options/AgentOptionsTest.kt

@@ -267,17 +267,17 @@ class AgentOptionsTest {
 	}
 
 	/**
-	 * Test that agent continues to run if the user provided a folder via the
-	 * [AgentOptions.GIT_PROPERTIES_JAR_OPTION] jar option.
+	 * Test that a folder is accepted for the [AgentOptions.GIT_PROPERTIES_JAR_OPTION] option, since the git.properties
+	 * search handles folders as well as archives.
 	 */
 	@Test
 	@Throws(Exception::class)
-	fun testGitPropertiesJarDoesNotAcceptFolders() {
+	fun testGitPropertiesJarAcceptsFolders() {
 		val jarFile = File(javaClass.getResource("nested-jar.war")!!.file)
 		val agentOptions = parseAndMaybeThrow(
 			"${AgentOptions.GIT_PROPERTIES_JAR_OPTION}=${jarFile.getParent()}"
 		)
-		Assertions.assertThat(agentOptions.gitPropertiesJar).isNull()
+		Assertions.assertThat(agentOptions.gitPropertiesJar).isDirectory()
 	}
 
 	/** Tests that supplying version info is supported in Testwise mode.  */

agent/src/test/kotlin/com/teamscale/jacoco/agent/upload/delay/DelayedCommitDescriptorRetrievalTest.kt

@@ -20,6 +20,20 @@ class DelayedCommitDescriptorRetrievalTest {
 	@Test
 	@Throws(Exception::class)
 	fun locatorShouldTriggerUploadOfCachedXmls(@TempDir outputPath: Path) {
+		assertLocatorTriggersUpload(
+			outputPath, File(javaClass.getResource("git-properties.jar")!!.toURI()), isJarFile = true
+		)
+	}
+
+	@Test
+	@Throws(Exception::class)
+	fun locatorShouldTriggerUploadOfCachedXmlsForFolder(@TempDir outputPath: Path) {
+		assertLocatorTriggersUpload(
+			outputPath, File(javaClass.getResource("git-properties-folder")!!.toURI()), isJarFile = false
+		)
+	}
+
+	private fun assertLocatorTriggersUpload(outputPath: Path, gitPropertiesSearchRoot: File, isJarFile: Boolean) {
 		val storeExecutor = Executors.newSingleThreadExecutor()
 		val coverageFilePath = outputPath
 			.resolve(String.format("jacoco-%d.xml", ZonedDateTime.now().toInstant().toEpochMilli()))
@@ -31,14 +45,14 @@ class DelayedCommitDescriptorRetrievalTest {
 		val locatorExecutor = Executors.newSingleThreadExecutor()
 		val locator = GitSingleProjectPropertiesLocator(
 			store, true, null, locatorExecutor
-		) { file, isJarFile, recursiveSearch, timeFormatter ->
+		) { file, isArchive, recursiveSearch, timeFormatter ->
 			getCommitInfoFromGitProperties(
-				file, isJarFile, recursiveSearch, timeFormatter
+				file, isArchive, recursiveSearch, timeFormatter
 			)
 		}
 
 		store.upload(coverageFile)
-		locator.searchFileForGitPropertiesAsync(File(javaClass.getResource("git-properties.jar")!!.toURI()), true)
+		locator.searchFileForGitPropertiesAsync(gitPropertiesSearchRoot, isJarFile)
 		locatorExecutor.shutdown()
 		locatorExecutor.awaitTermination(5, TimeUnit.SECONDS)
 		storeExecutor.shutdown()

agent/src/test/resources/com/teamscale/jacoco/agent/upload/delay/git-properties-folder/git.properties

@@ -0,0 +1,3 @@
+git.commit.id=72c7b3f7e6c4802414283cdf7622e6127f3f8976
+git.branch=master
+git.commit.time=2022-02-24T15:43:23+0100