Merge pull request #599 from cqse/ts/40875_artifactory_jar

TS-40875 Fix recursive detection of JAR files in artifactory

CHANGELOG.md

@@ -5,6 +5,10 @@ We use [semantic versioning](http://semver.org/):
 - PATCH version when you make backwards compatible bug fixes.
 
 # Next version
+- [fix] _agent_: `search-git-properties-recursively` was not considered when jar was given via `artifactory-git-properties-jar`
+- [deprecation] _agent_: `artifactory-git-properties-jar` is deprecated now. Replace the option with `git-properties-jar`. The old option still works but is an alias now for `git-properties-jar`.
+- [feature] _agent_: Added `git-properties-commit-date-format` (replaces `artifactory-git-properties-commit-date-format`), which now also can be used in non-artifactory cases.
+- [fix] _agent_: `search-git-properties-recursively` did only consider nested jar files, but no `war`, `ear`, `aar`
 
 # 34.1.1
 - [fix] _agent_: Loading a profiler configuration from Teamscale was not possible if the potentially necessary proxy settings were not set yet.

agent/README.md

@@ -176,6 +176,7 @@ patterns with `*`, `**` and `?`.
   - the properties `git.branch` and `git.commit.time` (in the format `yyyy-MM-dd'T'HH:mm:ssZ` or `yyyy-MM-dd'T'HH:mm:ssXXX`) or
   - the properties `teamscale.commit.branch` and `teamscale.commit.time` (either as an epoch timestamp or in one of the two formats above)
 - `search-git-properties-recursively` Specifies whether to search for git.properties files recursively in folders or archive (jar, war, ear, aar) files. Default: true.
+- `git-properties-commit-date-format` The Java data pattern `git.commit.time` is encoded with in `git.properties`. Defaults to `yyyy-MM-dd'T'HH:mm:ssZ`.
 - `teamscale-message` (optional): the commit message shown within Teamscale for the coverage upload (Default is "Agent
   coverage upload").
 - `config-file` (optional): a file which contains one or more of the previously named options as `key=value` entries
@@ -232,11 +233,6 @@ patterns with `*`, `**` and `?`.
   This can be used to encode e.g. a partition name that is parsed later on via Teamscale Artifactory connector options.
 - `artifactory-path-suffix` (optional): The path within the storage location between
   the default path and the uploaded artifact.
-- `artifactory-git-properties-jar` (optional): Specify a Jar to search a `git.properties` file within.
-  If not specified, Git commit information is extracted from the first found `git.properties` file.
-  See `git-properties-jar` for details.
-- `artifactory-git-properties-commit-date-format` (optional):
-  The Java data pattern `git.commit.time` is encoded with in `git.properties`. Defaults to `yyyy-MM-dd'T'HH:mm:ssZ`.
 
 ### The new standard upload schema
 ```

agent/src/main/java/com/teamscale/jacoco/agent/commit_resolution/git_properties/GitMultiProjectPropertiesLocator.java

@@ -4,11 +4,13 @@
 import com.teamscale.jacoco.agent.upload.teamscale.DelayedTeamscaleMultiProjectUploader;
 import com.teamscale.jacoco.agent.util.DaemonThreadFactory;
 import com.teamscale.jacoco.agent.util.LoggingUtils;
+import org.jetbrains.annotations.Nullable;
 import org.jetbrains.annotations.VisibleForTesting;
 import org.slf4j.Logger;
 
 import java.io.File;
 import java.io.IOException;
+import java.time.format.DateTimeFormatter;
 import java.util.List;
 import java.util.concurrent.Executor;
 import java.util.concurrent.Executors;
@@ -27,19 +29,22 @@ public class GitMultiProjectPropertiesLocator implements IGitPropertiesLocator {
 
 	private final boolean recursiveSearch;
 
-	public GitMultiProjectPropertiesLocator(DelayedTeamscaleMultiProjectUploader uploader, boolean recursiveSearch) {
+	private final @Nullable DateTimeFormatter gitPropertiesCommitTimeFormat;
+
+	public GitMultiProjectPropertiesLocator(DelayedTeamscaleMultiProjectUploader uploader, boolean recursiveSearch, @Nullable DateTimeFormatter gitPropertiesCommitTimeFormat) {
 		// using a single threaded executor allows this class to be lock-free
 		this(uploader, Executors
 				.newSingleThreadExecutor(
 						new DaemonThreadFactory(GitMultiProjectPropertiesLocator.class,
-								"git.properties Jar scanner thread")), recursiveSearch);
+								"git.properties Jar scanner thread")), recursiveSearch, gitPropertiesCommitTimeFormat);
 	}
 
 	public GitMultiProjectPropertiesLocator(DelayedTeamscaleMultiProjectUploader uploader, Executor executor,
-			boolean recursiveSearch) {
+			boolean recursiveSearch, @Nullable DateTimeFormatter gitPropertiesCommitTimeFormat) {
 		this.uploader = uploader;
 		this.executor = executor;
 		this.recursiveSearch = recursiveSearch;
+		this.gitPropertiesCommitTimeFormat = gitPropertiesCommitTimeFormat;
 	}
 
 	/**
@@ -62,7 +67,7 @@ void searchFile(File file, boolean isJarFile) {
 			List<ProjectAndCommit> projectAndCommits = GitPropertiesLocatorUtils.getProjectRevisionsFromGitProperties(
 					file,
 					isJarFile,
-					recursiveSearch);
+					recursiveSearch, gitPropertiesCommitTimeFormat);
 			if (projectAndCommits.isEmpty()) {
 				logger.debug("No git.properties file found in {}", file);
 				return;

agent/src/main/java/com/teamscale/jacoco/agent/commit_resolution/git_properties/GitPropertiesLocatorUtils.java

@@ -7,8 +7,8 @@
 import com.teamscale.report.util.BashFileSkippingInputStream;
 import org.conqat.lib.commons.collections.Pair;
 import org.jetbrains.annotations.NotNull;
+import org.jetbrains.annotations.Nullable;
 
-import javax.annotation.Nullable;
 import java.io.File;
 import java.io.IOException;
 import java.io.InputStream;
@@ -85,26 +85,30 @@ public class GitPropertiesLocatorUtils {
 	 */
 	private static final String GIT_PROPERTIES_DEFAULT_GRADLE_DATE_FORMAT = "yyyy-MM-dd'T'HH:mm:ssZ";
 
-	/** File ending of Java archive packages */
-	public static final String JAR_FILE_ENDING = ".jar";
-
 	/**
 	 * Reads the git SHA1 and branch and timestamp from the given jar file's git.properties and builds a commit
 	 * descriptor out of it. If no git.properties file can be found, returns null.
 	 *
 	 * @throws IOException                   If reading the jar file fails.
 	 * @throws InvalidGitPropertiesException If a git.properties file is found but it is malformed.
 	 */
-	public static List<CommitInfo> getCommitInfoFromGitProperties(
-			File file, boolean isJarFile, boolean recursiveSearch) throws IOException, InvalidGitPropertiesException {
-		List<Pair<String, Properties>> entriesWithProperties = findGitPropertiesInFile(file, isJarFile,
-				recursiveSearch);
+	public static List<CommitInfo> getCommitInfoFromGitProperties(File file, boolean isJarFile,
+			boolean recursiveSearch,
+			@Nullable DateTimeFormatter gitPropertiesCommitTimeFormat)
+			throws IOException, InvalidGitPropertiesException {
+		List<Pair<String, Properties>> entriesWithProperties = GitPropertiesLocatorUtils.findGitPropertiesInFile(file,
+				isJarFile, recursiveSearch);
 		List<CommitInfo> result = new ArrayList<>();
+
 		for (Pair<String, Properties> entryWithProperties : entriesWithProperties) {
-			CommitInfo commitInfo = getCommitInfoFromGitProperties(entryWithProperties.getSecond(),
-					entryWithProperties.getFirst(), file, null);
+			String entry = entryWithProperties.getFirst();
+			Properties properties = entryWithProperties.getSecond();
+
+			CommitInfo commitInfo = GitPropertiesLocatorUtils.getCommitInfoFromGitProperties(properties, entry, file,
+					gitPropertiesCommitTimeFormat);
 			result.add(commitInfo);
 		}
+
 		return result;
 	}
 
@@ -122,8 +126,7 @@ public static Pair<File, Boolean> extractGitPropertiesSearchRoot(
 		switch (protocol) {
 			case "file":
 				File jarOrClassFolderFile = new File(jarOrClassFolderUrl.toURI());
-				if (jarOrClassFolderFile.isDirectory() || org.conqat.lib.commons.string.StringUtils.endsWithOneOf(
-						jarOrClassFolderUrl.getPath().toLowerCase(), ".jar", ".war", ".ear", ".aar")) {
+				if (jarOrClassFolderFile.isDirectory() || isJarLikeFile(jarOrClassFolderUrl.getPath())) {
 					return Pair.createPair(new File(jarOrClassFolderUrl.toURI()), !jarOrClassFolderFile.isDirectory());
 				}
 				break;
@@ -183,8 +186,7 @@ private static String extractArtefactUrl(URL jarOrClassFolderUrl) {
 			String segment = pathSegments[segmentIdx];
 			artefactUrlBuilder.append(segment);
 			artefactUrlBuilder.append("/");
-			if (org.conqat.lib.commons.string.StringUtils.endsWithOneOf(
-					segment, ".jar", ".war", ".ear", ".aar")) {
+			if (isJarLikeFile(segment)) {
 				break;
 			}
 			segmentIdx += 1;
@@ -195,6 +197,11 @@ private static String extractArtefactUrl(URL jarOrClassFolderUrl) {
 		return artefactUrlBuilder.toString();
 	}
 
+	private static boolean isJarLikeFile(String segment) {
+		return org.conqat.lib.commons.string.StringUtils.endsWithOneOf(
+				segment.toLowerCase(), ".jar", ".war", ".ear", ".aar");
+	}
+
 	/**
 	 * Reads the 'teamscale.project' property values and the git SHA1s or branch + timestamp from all git.properties
 	 * files contained in the provided folder or archive file.
@@ -203,13 +210,14 @@ private static String extractArtefactUrl(URL jarOrClassFolderUrl) {
 	 * @throws InvalidGitPropertiesException If a git.properties file is found but it is malformed.
 	 */
 	public static List<ProjectAndCommit> getProjectRevisionsFromGitProperties(
-			File file, boolean isJarFile, boolean recursiveSearch) throws IOException, InvalidGitPropertiesException {
+			File file, boolean isJarFile, boolean recursiveSearch,
+			@Nullable DateTimeFormatter gitPropertiesCommitTimeFormat) throws IOException, InvalidGitPropertiesException {
 		List<Pair<String, Properties>> entriesWithProperties = findGitPropertiesInFile(file, isJarFile,
 				recursiveSearch);
 		List<ProjectAndCommit> result = new ArrayList<>();
 		for (Pair<String, Properties> entryWithProperties : entriesWithProperties) {
 			CommitInfo commitInfo = getCommitInfoFromGitProperties(entryWithProperties.getSecond(),
-					entryWithProperties.getFirst(), file, null);
+					entryWithProperties.getFirst(), file, gitPropertiesCommitTimeFormat);
 			String project = entryWithProperties.getSecond().getProperty(GIT_PROPERTIES_TEAMSCALE_PROJECT);
 			if (commitInfo.isEmpty() && StringUtils.isEmpty(project)) {
 				throw new InvalidGitPropertiesException(
@@ -273,7 +281,7 @@ private static List<Pair<String, Properties>> findGitPropertiesInNestedJarFiles(
 			File directoryFile) throws IOException {
 		List<Pair<String, Properties>> result = new ArrayList<>();
 		List<File> jarFiles = FileSystemUtils.listFilesRecursively(directoryFile,
-				file -> file.getName().endsWith(JAR_FILE_ENDING));
+				file -> isJarLikeFile(file.getName()));
 		for (File jarFile : jarFiles) {
 			JarInputStream is = new JarInputStream(Files.newInputStream(jarFile.toPath()));
 			String relativeFilePath = directoryFile.getName() + File.separator + directoryFile.toPath()
@@ -323,7 +331,7 @@ static List<Pair<String, Properties>> findGitPropertiesInArchive(
 				Properties gitProperties = new Properties();
 				gitProperties.load(in);
 				result.add(Pair.createPair(fullEntryName, gitProperties));
-			} else if (entry.getName().endsWith(JAR_FILE_ENDING) && recursiveSearch) {
+			} else if (isJarLikeFile(entry.getName()) && recursiveSearch) {
 				result.addAll(findGitPropertiesInArchive(new JarInputStream(in), fullEntryName, true));
 			}
 		}

agent/src/main/java/com/teamscale/jacoco/agent/commit_resolution/git_properties/GitSingleProjectPropertiesLocator.java

@@ -3,10 +3,12 @@
 import com.teamscale.jacoco.agent.upload.delay.DelayedUploader;
 import com.teamscale.jacoco.agent.util.DaemonThreadFactory;
 import com.teamscale.jacoco.agent.util.LoggingUtils;
+import org.checkerframework.checker.nullness.qual.Nullable;
 import org.slf4j.Logger;
 
 import java.io.File;
 import java.io.IOException;
+import java.time.format.DateTimeFormatter;
 import java.util.List;
 import java.util.concurrent.Executor;
 import java.util.concurrent.Executors;
@@ -26,28 +28,32 @@ public class GitSingleProjectPropertiesLocator<T> implements IGitPropertiesLocat
 	private final DataExtractor<T> dataExtractor;
 
 	private final boolean recursiveSearch;
+	private final @Nullable DateTimeFormatter gitPropertiesCommitTimeFormat;
 
 	public GitSingleProjectPropertiesLocator(DelayedUploader<T> uploader, DataExtractor<T> dataExtractor,
-											 boolean recursiveSearch) {
+			boolean recursiveSearch,
+			@Nullable DateTimeFormatter gitPropertiesCommitTimeFormat) {
 		// using a single threaded executor allows this class to be lock-free
 		this(uploader, dataExtractor, Executors
 						.newSingleThreadExecutor(
 								new DaemonThreadFactory(GitSingleProjectPropertiesLocator.class,
 										"git.properties Jar scanner thread")),
-				recursiveSearch);
+				recursiveSearch, gitPropertiesCommitTimeFormat);
 	}
 
 	/**
 	 * Visible for testing. Allows tests to control the {@link Executor} in order to test the asynchronous functionality
 	 * of this class.
 	 */
 	public GitSingleProjectPropertiesLocator(DelayedUploader<T> uploader, DataExtractor<T> dataExtractor,
-											 Executor executor,
-											 boolean recursiveSearch) {
+			Executor executor,
+			boolean recursiveSearch,
+			@Nullable DateTimeFormatter gitPropertiesCommitTimeFormat) {
 		this.uploader = uploader;
 		this.dataExtractor = dataExtractor;
 		this.executor = executor;
 		this.recursiveSearch = recursiveSearch;
+		this.gitPropertiesCommitTimeFormat = gitPropertiesCommitTimeFormat;
 	}
 
 	/**
@@ -61,7 +67,7 @@ public void searchFileForGitPropertiesAsync(File file, boolean isJarFile) {
 	private void searchFile(File file, boolean isJarFile) {
 		logger.debug("Searching jar file {} for a single git.properties", file);
 		try {
-			List<T> data = dataExtractor.extractData(file, isJarFile, recursiveSearch);
+			List<T> data = dataExtractor.extractData(file, isJarFile, recursiveSearch, gitPropertiesCommitTimeFormat);
 			if (data.isEmpty()) {
 				logger.debug("No git.properties files found in {}", file.toString());
 				return;
@@ -103,6 +109,7 @@ private void searchFile(File file, boolean isJarFile) {
 	public interface DataExtractor<T> {
 		/** Extracts data from the JAR. */
 		List<T> extractData(File file, boolean isJarFile,
-							boolean recursiveSearch) throws IOException, InvalidGitPropertiesException;
+				boolean recursiveSearch,
+				@Nullable DateTimeFormatter gitPropertiesCommitTimeFormat) throws IOException, InvalidGitPropertiesException;
 	}
 }

agent/src/main/java/com/teamscale/jacoco/agent/options/AgentOptions.java

@@ -79,9 +79,19 @@ public class AgentOptions {
 	/** See {@link AgentOptions#GIT_PROPERTIES_JAR_OPTION} */
 	/* package */ File gitPropertiesJar;
 
+	/**
+	 * Related to {@link AgentOptions#GIT_PROPERTIES_COMMIT_DATE_FORMAT_OPTION}
+	 */
+	public DateTimeFormatter gitPropertiesCommitTimeFormat = null;
+
 	/** Option name that allows to specify a jar file that contains the git commit hash in a git.properties file. */
 	public static final String GIT_PROPERTIES_JAR_OPTION = "git-properties-jar";
 
+	/**
+	 * Specifies the date format in which the commit timestamp in the git.properties file is formatted.
+	 */
+	public static final String GIT_PROPERTIES_COMMIT_DATE_FORMAT_OPTION = "git-properties-commit-date-format";
+
 	/**
 	 * The original options passed to the agent.
 	 */
@@ -455,7 +465,14 @@ public IUploader createUploader(Instrumentation instrumentation) throws Uploader
 	}
 
 	@NotNull
-	private IUploader createArtifactoryUploader(Instrumentation instrumentation) {
+	private IUploader createArtifactoryUploader(Instrumentation instrumentation) throws UploaderException {
+		if (gitPropertiesJar != null) {
+			logger.info("You did not provide a commit to upload to directly, so the Agent will try to" +
+					"auto-detect it by searching the provided " + GIT_PROPERTIES_JAR_OPTION + " at " +
+					gitPropertiesJar.getAbsolutePath() + " for a git.properties file.");
+			artifactoryConfig.commitInfo = ArtifactoryConfig.parseGitProperties(gitPropertiesJar,
+					this.searchGitPropertiesRecursively, this.gitPropertiesCommitTimeFormat);
+		}
 		if (!artifactoryConfig.hasCommitInfo()) {
 			logger.info("You did not provide a commit to upload to directly, so the Agent will try and" +
 					" auto-detect it by searching all profiled Jar/War/Ear/... files for a git.properties file.");
@@ -518,14 +535,16 @@ private DelayedTeamscaleMultiProjectUploader createTeamscaleMultiProjectUploader
 	private void startGitPropertiesSearchInJarFile(DelayedUploader<ProjectAndCommit> uploader,
 			File gitPropertiesJar) {
 		GitSingleProjectPropertiesLocator<ProjectAndCommit> locator = new GitSingleProjectPropertiesLocator<>(uploader,
-				GitPropertiesLocatorUtils::getProjectRevisionsFromGitProperties, this.searchGitPropertiesRecursively);
+				GitPropertiesLocatorUtils::getProjectRevisionsFromGitProperties, this.searchGitPropertiesRecursively,
+				this.gitPropertiesCommitTimeFormat);
 		locator.searchFileForGitPropertiesAsync(gitPropertiesJar, true);
 	}
 
 	private void registerSingleGitPropertiesLocator(DelayedUploader<ProjectAndCommit> uploader,
 			Instrumentation instrumentation) {
 		GitSingleProjectPropertiesLocator<ProjectAndCommit> locator = new GitSingleProjectPropertiesLocator<>(uploader,
-				GitPropertiesLocatorUtils::getProjectRevisionsFromGitProperties, this.searchGitPropertiesRecursively);
+				GitPropertiesLocatorUtils::getProjectRevisionsFromGitProperties, this.searchGitPropertiesRecursively,
+				this.gitPropertiesCommitTimeFormat);
 		instrumentation.addTransformer(new GitPropertiesLocatingTransformer(locator, getLocationIncludeFilter()));
 	}
 
@@ -551,14 +570,14 @@ private DelayedUploader<ProjectAndCommit> createDelayedSingleProjectTeamscaleUpl
 	private void startMultiGitPropertiesFileSearchInJarFile(DelayedTeamscaleMultiProjectUploader uploader,
 			File gitPropertiesJar) {
 		GitMultiProjectPropertiesLocator locator = new GitMultiProjectPropertiesLocator(uploader,
-				this.searchGitPropertiesRecursively);
+				this.searchGitPropertiesRecursively, this.gitPropertiesCommitTimeFormat);
 		locator.searchFileForGitPropertiesAsync(gitPropertiesJar, true);
 	}
 
 	private void registerMultiGitPropertiesLocator(DelayedTeamscaleMultiProjectUploader uploader,
 			Instrumentation instrumentation) {
 		GitMultiProjectPropertiesLocator locator = new GitMultiProjectPropertiesLocator(uploader,
-				this.searchGitPropertiesRecursively);
+				this.searchGitPropertiesRecursively, this.gitPropertiesCommitTimeFormat);
 		instrumentation.addTransformer(new GitPropertiesLocatingTransformer(locator, getLocationIncludeFilter()));
 	}
 
@@ -571,9 +590,8 @@ private IUploader createDelayedArtifactoryUploader(Instrumentation instrumentati
 				}, outputDirectory);
 		GitSingleProjectPropertiesLocator<CommitInfo> locator = new GitSingleProjectPropertiesLocator<>(
 				uploader,
-				(file, isJarFile, recursiveSearch) -> ArtifactoryConfig.parseGitProperties(
-						file, isJarFile, artifactoryConfig.gitPropertiesCommitTimeFormat, recursiveSearch),
-				this.searchGitPropertiesRecursively);
+				GitPropertiesLocatorUtils::getCommitInfoFromGitProperties,
+				this.searchGitPropertiesRecursively, this.gitPropertiesCommitTimeFormat);
 		instrumentation.addTransformer(new GitPropertiesLocatingTransformer(locator, getLocationIncludeFilter()));
 		return uploader;
 	}
@@ -731,7 +749,7 @@ public boolean shouldIgnoreUncoveredClasses() {
 
 	/** @return the {@link TeamscaleProxyOptions} for the given protocol. */
 	public TeamscaleProxyOptions getTeamscaleProxyOptions(ProxySystemProperties.Protocol protocol) {
-		if(protocol == ProxySystemProperties.Protocol.HTTP) {
+		if (protocol == ProxySystemProperties.Protocol.HTTP) {
 			return teamscaleProxyOptionsForHttp;
 		}
 		return teamscaleProxyOptionsForHttps;