From 6f6a5510e48a0838bf65f5b64f3b0a56012bcfd0 Mon Sep 17 00:00:00 2001 From: Philipp Nanz Date: Fri, 29 Oct 2021 18:36:35 +0200 Subject: [PATCH 1/2] Add feature to ignore git commits by id --- docs/index.md | 1 + license-maven-plugin-git/README.md | 2 + .../maven/plugin/license/git/GitLookup.java | 33 ++++++++++++--- .../plugin/license/git/GitLookupTest.java | 41 +++++++++++++++---- 4 files changed, 63 insertions(+), 14 deletions(-) diff --git a/docs/index.md b/docs/index.md index 8d17cde29..1280fef28 100644 --- a/docs/index.md +++ b/docs/index.md @@ -73,6 +73,7 @@ Basically, when you are developing a project either in open source or in a compa * [@stain](https://github.com/stain) * [@vromero](https://github.com/vromero) * [@yoosiba](https://github.com/yoosiba) +* [@philippn](https://github.com/philippn) Please let me know if your name is missing! diff --git a/license-maven-plugin-git/README.md b/license-maven-plugin-git/README.md index 797872eae..c8e557a03 100644 --- a/license-maven-plugin-git/README.md +++ b/license-maven-plugin-git/README.md @@ -20,6 +20,8 @@ Which properties is license-maven-plugin-git adding? The full git history for a file is required for accurate determination of the first commit (for the creation author name/email, creation year, or existence years). The plugin will warn if it detects a shallow git repository. If you are certain your shallow depth will still permit determination of these values you may suppress the warning by setting the parameter `license.warnIfShallow` to false. +It is possible to filter out specific commits by adding one or more (comma-separated) SHA-1 hashes to the property `license.git.commitsToIgnore`. This property will only affect the computation of the last change year, not of the creation year. + If you are using properties requiring the year of the last commit of a file but not using the "creation" properties requiring the first commit, you can improve performance by limiting the git history for each file using the property `license.git.maxCommitsLookup`. A value of 1 would provide the best performance but could be impacted if the last commit year is earlier than the year of previous commits. How to use license-maven-plugin-git diff --git a/license-maven-plugin-git/src/main/java/com/mycila/maven/plugin/license/git/GitLookup.java b/license-maven-plugin-git/src/main/java/com/mycila/maven/plugin/license/git/GitLookup.java index a4a8783f5..18f92eaaa 100644 --- a/license-maven-plugin-git/src/main/java/com/mycila/maven/plugin/license/git/GitLookup.java +++ b/license-maven-plugin-git/src/main/java/com/mycila/maven/plugin/license/git/GitLookup.java @@ -21,16 +21,21 @@ import java.io.UncheckedIOException; import java.util.Arrays; import java.util.Calendar; +import java.util.Collections; import java.util.Date; import java.util.Iterator; import java.util.Locale; import java.util.Map; +import java.util.Set; import java.util.TimeZone; +import java.util.stream.Collectors; + import org.eclipse.jgit.api.Git; import org.eclipse.jgit.api.Status; import org.eclipse.jgit.api.errors.GitAPIException; import org.eclipse.jgit.diff.DiffConfig; import org.eclipse.jgit.lib.Constants; +import org.eclipse.jgit.lib.ObjectId; import org.eclipse.jgit.lib.PersonIdent; import org.eclipse.jgit.lib.Repository; import org.eclipse.jgit.revwalk.FollowFilter; @@ -57,6 +62,7 @@ public class GitLookup implements Closeable { private static final String COPYRIGHT_LAST_YEAR_MAX_COMMITS_LOOKUP_KEY = "license.git.copyrightLastYearMaxCommitsLookup"; public static final String COPYRIGHT_LAST_YEAR_SOURCE_KEY = "license.git.copyrightLastYearSource"; public static final String COPYRIGHT_LAST_YEAR_TIME_ZONE_KEY = "license.git.copyrightLastYearTimeZone"; + public static final String COMMITS_TO_IGNORE_KEY = "license.git.commitsToIgnore"; public enum DateSource { AUTHOR, COMMITER @@ -68,6 +74,7 @@ public enum DateSource { private final Repository repository; private final TimeZone timeZone; private final boolean shallow; + private final Set commitsToIgnore; /** * Lazily initializes #gitLookup assuming that all subsequent calls to this method will be related @@ -81,7 +88,7 @@ public static GitLookup create(File file, Map props) { GitLookup.DateSource dateSource = GitLookup.DateSource.valueOf( dateSourceString.toUpperCase(Locale.US)); String checkCommitsCountString = props.get(MAX_COMMITS_LOOKUP_KEY); - // Backwads compatibility + // Backwards compatibility if (checkCommitsCountString == null) { checkCommitsCountString = props.get(COPYRIGHT_LAST_YEAR_MAX_COMMITS_LOOKUP_KEY); } @@ -90,6 +97,15 @@ public static GitLookup create(File file, Map props) { checkCommitsCountString = checkCommitsCountString.trim(); checkCommitsCount = Integer.parseInt(checkCommitsCountString); } + String commitsToIgnoreString = props.get(COMMITS_TO_IGNORE_KEY); + Set commitsToIgnore = Collections.emptySet(); + if (commitsToIgnoreString != null) { + commitsToIgnoreString = commitsToIgnoreString.trim(); + commitsToIgnore = Arrays.stream(commitsToIgnoreString.split(",")) + .map(String::trim) + .map(ObjectId::fromString) + .collect(Collectors.toSet()); + } final TimeZone timeZone; String tzString = props.get(COPYRIGHT_LAST_YEAR_TIME_ZONE_KEY); switch (dateSource) { @@ -108,7 +124,7 @@ public static GitLookup create(File file, Map props) { throw new IllegalStateException( "Unexpected " + GitLookup.DateSource.class.getName() + " " + dateSource); } - return new GitLookup(file, dateSource, timeZone, checkCommitsCount); + return new GitLookup(file, dateSource, timeZone, checkCommitsCount, commitsToIgnore); } /** @@ -122,10 +138,11 @@ public static GitLookup create(File file, Map props) { * @param dateSource where to read the commit dates from - committer date or author date * @param timeZone the time zone if {@code dateSource} is {@link DateSource#COMMITER}; * otherwise must be {@code null}. - * @param checkCommitsCount + * @param checkCommitsCount the number of historical commits, per file, to check + * @param commitsToIgnore the commits to ignore while inspecting the history for {@code anyFile} * @throws IOException */ - private GitLookup(File anyFile, DateSource dateSource, TimeZone timeZone, int checkCommitsCount) { + private GitLookup(File anyFile, DateSource dateSource, TimeZone timeZone, int checkCommitsCount, Set commitsToIgnore) { try { this.repository = new FileRepositoryBuilder().findGitDir(anyFile).build(); /* A workaround for https://bugs.eclipse.org/bugs/show_bug.cgi?id=457961 */ @@ -155,6 +172,7 @@ private GitLookup(File anyFile, DateSource dateSource, TimeZone timeZone, int ch "Unexpected " + DateSource.class.getName() + " " + dateSource); } this.checkCommitsCount = checkCommitsCount; + this.commitsToIgnore = commitsToIgnore; } catch (IOException e) { throw new UncheckedIOException(e); } @@ -165,7 +183,7 @@ private GitLookup(File anyFile, DateSource dateSource, TimeZone timeZone, int ch * year is taken either from the committer date or from the author identity depending on how {@link #dateSource} was * initialized. *

- * See also the note on time zones in {@link #GitLookup(File, DateSource, TimeZone, int)}. + * See also the note on time zones in {@link #GitLookup(File, DateSource, TimeZone, int, Set)}. * * @param file for which the year should be retrieved * @return year of last modification of the file @@ -182,6 +200,9 @@ int getYearOfLastChange(File file) throws GitAPIException, IOException { int commitYear = 0; RevWalk walk = getGitRevWalk(repoRelativePath, false); for (RevCommit commit : walk) { + if (commitsToIgnore.contains(commit.getId())) { + continue; + } int y = getYearFromCommit(commit); if (y > commitYear) { commitYear = y; @@ -314,4 +335,4 @@ private String getAuthorEmailFromCommit(RevCommit commit) { public void close() { repository.close(); } -} +} \ No newline at end of file diff --git a/license-maven-plugin-git/src/test/java/com/mycila/maven/plugin/license/git/GitLookupTest.java b/license-maven-plugin-git/src/test/java/com/mycila/maven/plugin/license/git/GitLookupTest.java index 55e5e520b..1ade8a88a 100644 --- a/license-maven-plugin-git/src/test/java/com/mycila/maven/plugin/license/git/GitLookupTest.java +++ b/license-maven-plugin-git/src/test/java/com/mycila/maven/plugin/license/git/GitLookupTest.java @@ -157,7 +157,7 @@ void reuseProvider() throws GitAPIException, IOException { @Test void timezone() throws GitAPIException, IOException { try { - GitLookup.create(gitRepoRoot.toFile(), buildProps(DateSource.AUTHOR, "GMT", "10")); + GitLookup.create(gitRepoRoot.toFile(), buildProps(DateSource.AUTHOR, "GMT", "10", null)); Assertions.fail("RuntimeException expected"); } catch (RuntimeException e) { if (e.getMessage().contains( @@ -170,12 +170,12 @@ void timezone() throws GitAPIException, IOException { /* null is GMT */ GitLookup nullTzLookup = GitLookup.create(gitRepoRoot.toFile(), - buildProps(DateSource.COMMITER, null, "10")); + buildProps(DateSource.COMMITER, null, "10", null)); assertLastChange(nullTzLookup, "dir1/file3.txt", 2010); /* explicit GMT */ GitLookup gmtLookup = GitLookup.create(gitRepoRoot.toFile(), - buildProps(DateSource.COMMITER, "GMT", "10")); + buildProps(DateSource.COMMITER, "GMT", "10", null)); assertLastChange(gmtLookup, "dir1/file3.txt", 2010); /* @@ -183,12 +183,12 @@ void timezone() throws GitAPIException, IOException { * 2011 in the CET (+01:00) time zone */ GitLookup cetLookup = GitLookup.create(gitRepoRoot.toFile(), - buildProps(DateSource.COMMITER, "CET", "10")); + buildProps(DateSource.COMMITER, "CET", "10", null)); assertLastChange(cetLookup, "dir1/file3.txt", 2011); } - private Map buildProps(DateSource ds, String tz, String history) { + private Map buildProps(DateSource ds, String tz, String history, String commitsToIgnoreCSV) { Map props = new HashMap<>(); if (history != null) { props.put(GitLookup.MAX_COMMITS_LOOKUP_KEY, history); @@ -199,15 +199,40 @@ private Map buildProps(DateSource ds, String tz, String history) if (ds != null) { props.put(GitLookup.COPYRIGHT_LAST_YEAR_SOURCE_KEY, ds.name()); } + if (commitsToIgnoreCSV != null) { + props.put(GitLookup.COMMITS_TO_IGNORE_KEY, commitsToIgnoreCSV); + } return props; } private GitLookup newAuthorLookup() { - return GitLookup.create(gitRepoRoot.toFile(), buildProps(DateSource.AUTHOR, null, "10")); + return GitLookup.create(gitRepoRoot.toFile(), buildProps(DateSource.AUTHOR, null, "10", null)); } private GitLookup newCommitterLookup() { - return GitLookup.create(gitRepoRoot.toFile(), buildProps(DateSource.COMMITER, null, "10")); + return GitLookup.create(gitRepoRoot.toFile(), buildProps(DateSource.COMMITER, null, "10", null)); + } + + @Test + public void ignoreCommitsInLastChange() throws GitAPIException, IOException { + assertLastChange(newAuthorLookup("95d52919cbe340dc271cf1f5ec68cf36705bd3a3"), "dir1/file1.txt", 2004); + assertLastChange(newCommitterLookup("95d52919cbe340dc271cf1f5ec68cf36705bd3a3"), "dir1/file1.txt", 2004); + } + + @Test + public void doNotIgnoreCommitsInCreation() throws GitAPIException, IOException { + assertCreation(newAuthorLookup("53b44baedc5a378f9b665da12f298e1003793219"), "dir1/file1.txt", 2000); + assertCreation(newCommitterLookup("53b44baedc5a378f9b665da12f298e1003793219"), "dir1/file1.txt", 2000); + } + + private GitLookup newAuthorLookup(String... commitsToIgnore) throws IOException { + String commitsToIgnoreCSV = String.join(",", commitsToIgnore); + return GitLookup.create(gitRepoRoot.toFile(), buildProps(DateSource.AUTHOR, null, "10", commitsToIgnoreCSV)); + } + + private GitLookup newCommitterLookup(String... commitsToIgnore) throws IOException { + String commitsToIgnoreCSV = String.join(",", commitsToIgnore); + return GitLookup.create(gitRepoRoot.toFile(), buildProps(DateSource.COMMITER, null, "10", commitsToIgnoreCSV)); } private void assertLastChange(GitLookup provider, String relativePath, int expected) throws @@ -224,4 +249,4 @@ private void assertCreation(GitLookup provider, String relativePath, int expecte Assertions.assertEquals(expected, actual); } -} +} \ No newline at end of file From 7f8396bdc195b2f851d57c409efd8aa23d598ccf Mon Sep 17 00:00:00 2001 From: Mathieu Carbou Date: Wed, 30 Mar 2022 11:58:19 +0200 Subject: [PATCH 2/2] Removed unnecessary failure regarding TZ, filtering out empty strings and cleanup property parsing --- .../maven/plugin/license/git/GitLookup.java | 106 +++++++----------- .../plugin/license/git/GitLookupTest.java | 14 +-- 2 files changed, 45 insertions(+), 75 deletions(-) diff --git a/license-maven-plugin-git/src/main/java/com/mycila/maven/plugin/license/git/GitLookup.java b/license-maven-plugin-git/src/main/java/com/mycila/maven/plugin/license/git/GitLookup.java index 18f92eaaa..7acd1bbf1 100644 --- a/license-maven-plugin-git/src/main/java/com/mycila/maven/plugin/license/git/GitLookup.java +++ b/license-maven-plugin-git/src/main/java/com/mycila/maven/plugin/license/git/GitLookup.java @@ -15,21 +15,23 @@ */ package com.mycila.maven.plugin.license.git; +import static java.util.Objects.requireNonNull; + import java.io.Closeable; import java.io.File; import java.io.IOException; import java.io.UncheckedIOException; import java.util.Arrays; import java.util.Calendar; -import java.util.Collections; import java.util.Date; import java.util.Iterator; -import java.util.Locale; import java.util.Map; +import java.util.Objects; +import java.util.Optional; import java.util.Set; import java.util.TimeZone; import java.util.stream.Collectors; - +import java.util.stream.Stream; import org.eclipse.jgit.api.Git; import org.eclipse.jgit.api.Status; import org.eclipse.jgit.api.errors.GitAPIException; @@ -81,49 +83,36 @@ public enum DateSource { * to the same git repository. */ public static GitLookup create(File file, Map props) { - String dateSourceString = props.get(COPYRIGHT_LAST_YEAR_SOURCE_KEY); - if (dateSourceString == null) { - dateSourceString = GitLookup.DateSource.AUTHOR.name(); - } - GitLookup.DateSource dateSource = GitLookup.DateSource.valueOf( - dateSourceString.toUpperCase(Locale.US)); - String checkCommitsCountString = props.get(MAX_COMMITS_LOOKUP_KEY); - // Backwards compatibility - if (checkCommitsCountString == null) { - checkCommitsCountString = props.get(COPYRIGHT_LAST_YEAR_MAX_COMMITS_LOOKUP_KEY); - } - int checkCommitsCount = Integer.MAX_VALUE; - if (checkCommitsCountString != null) { - checkCommitsCountString = checkCommitsCountString.trim(); - checkCommitsCount = Integer.parseInt(checkCommitsCountString); - } - String commitsToIgnoreString = props.get(COMMITS_TO_IGNORE_KEY); - Set commitsToIgnore = Collections.emptySet(); - if (commitsToIgnoreString != null) { - commitsToIgnoreString = commitsToIgnoreString.trim(); - commitsToIgnore = Arrays.stream(commitsToIgnoreString.split(",")) - .map(String::trim) - .map(ObjectId::fromString) - .collect(Collectors.toSet()); - } - final TimeZone timeZone; - String tzString = props.get(COPYRIGHT_LAST_YEAR_TIME_ZONE_KEY); - switch (dateSource) { - case COMMITER: - timeZone = tzString == null ? GitLookup.DEFAULT_ZONE : TimeZone.getTimeZone(tzString); - break; - case AUTHOR: - if (tzString != null) { - throw new RuntimeException(COPYRIGHT_LAST_YEAR_TIME_ZONE_KEY + " must not be set with " - + COPYRIGHT_LAST_YEAR_SOURCE_KEY + " = " + GitLookup.DateSource.AUTHOR.name() - + " because git author name already contains time zone information."); - } - timeZone = null; - break; - default: - throw new IllegalStateException( - "Unexpected " + GitLookup.DateSource.class.getName() + " " + dateSource); - } + final GitLookup.DateSource dateSource = Optional.ofNullable(props.get(COPYRIGHT_LAST_YEAR_SOURCE_KEY)) + .map(String::trim) + .map(String::toUpperCase) + .map(GitLookup.DateSource::valueOf) + .orElse(GitLookup.DateSource.AUTHOR); + + final int checkCommitsCount = Stream.of( + MAX_COMMITS_LOOKUP_KEY, + COPYRIGHT_LAST_YEAR_MAX_COMMITS_LOOKUP_KEY) // Backwards compatibility + .map(props::get) + .filter(Objects::nonNull) + .map(String::trim) + .map(Integer::parseInt) + .findFirst() + .orElse(Integer.MAX_VALUE); + + final Set commitsToIgnore = Stream.of(COMMITS_TO_IGNORE_KEY) + .map(props::get) + .filter(Objects::nonNull) + .flatMap(s -> Stream.of(s.split(","))) + .map(String::trim) + .filter(s -> !s.isEmpty()) + .map(ObjectId::fromString) + .collect(Collectors.toSet()); + + final TimeZone timeZone = Optional.ofNullable(props.get(COPYRIGHT_LAST_YEAR_TIME_ZONE_KEY)) + .map(String::trim) + .map(TimeZone::getTimeZone) + .orElse(DEFAULT_ZONE); + return new GitLookup(file, dateSource, timeZone, checkCommitsCount, commitsToIgnore); } @@ -143,6 +132,11 @@ public static GitLookup create(File file, Map props) { * @throws IOException */ private GitLookup(File anyFile, DateSource dateSource, TimeZone timeZone, int checkCommitsCount, Set commitsToIgnore) { + requireNonNull(anyFile); + requireNonNull(dateSource); + requireNonNull(timeZone); + requireNonNull(commitsToIgnore); + try { this.repository = new FileRepositoryBuilder().findGitDir(anyFile).build(); /* A workaround for https://bugs.eclipse.org/bugs/show_bug.cgi?id=457961 */ @@ -152,25 +146,9 @@ private GitLookup(File anyFile, DateSource dateSource, TimeZone timeZone, int ch // Closing the repository will close the FileObjectDatabase. // Here the newReader() is a WindowCursor which delegates the getShallowCommits() to the FileObjectDatabase. this.shallow = !this.repository.getObjectDatabase().newReader().getShallowCommits().isEmpty(); - this.pathResolver = new GitPathResolver(repository.getWorkTree().getAbsolutePath()); this.dateSource = dateSource; - switch (dateSource) { - case COMMITER: - this.timeZone = timeZone == null ? DEFAULT_ZONE : timeZone; - break; - case AUTHOR: - if (timeZone != null) { - throw new IllegalArgumentException( - "Time zone must be null with dateSource " + DateSource.AUTHOR.name() - + " because git author name already contains time zone information."); - } - this.timeZone = null; - break; - default: - throw new IllegalStateException( - "Unexpected " + DateSource.class.getName() + " " + dateSource); - } + this.timeZone = timeZone; this.checkCommitsCount = checkCommitsCount; this.commitsToIgnore = commitsToIgnore; } catch (IOException e) { @@ -298,7 +276,7 @@ private RevWalk getGitRevWalk(String repoRelativePath, boolean oldestCommitsFirs } private int getCurrentYear() { - return toYear(System.currentTimeMillis(), timeZone != null ? timeZone : DEFAULT_ZONE); + return toYear(System.currentTimeMillis(), timeZone); } private int getYearFromCommit(RevCommit commit) { diff --git a/license-maven-plugin-git/src/test/java/com/mycila/maven/plugin/license/git/GitLookupTest.java b/license-maven-plugin-git/src/test/java/com/mycila/maven/plugin/license/git/GitLookupTest.java index 1ade8a88a..3d3460816 100644 --- a/license-maven-plugin-git/src/test/java/com/mycila/maven/plugin/license/git/GitLookupTest.java +++ b/license-maven-plugin-git/src/test/java/com/mycila/maven/plugin/license/git/GitLookupTest.java @@ -156,17 +156,9 @@ void reuseProvider() throws GitAPIException, IOException { @Test void timezone() throws GitAPIException, IOException { - try { - GitLookup.create(gitRepoRoot.toFile(), buildProps(DateSource.AUTHOR, "GMT", "10", null)); - Assertions.fail("RuntimeException expected"); - } catch (RuntimeException e) { - if (e.getMessage().contains( - "license.git.copyrightLastYearTimeZone must not be set with license.git.copyrightLastYearSource = AUTHOR because git author name already contains time zone information.")) { - /* expected */ - } else { - throw e; - } - } + // do not fail if a tz is./mv set, it will just be unused + // it allows parent poms to pre-defined properties and let sub modules use them if needed + GitLookup.create(gitRepoRoot.toFile(), buildProps(DateSource.AUTHOR, "GMT", "10", null)); /* null is GMT */ GitLookup nullTzLookup = GitLookup.create(gitRepoRoot.toFile(),