From 0ea123ae75d1e5a40c8642da3fe49defc8a8c952 Mon Sep 17 00:00:00 2001 From: Mark Waite Date: Sat, 1 Jul 2023 12:52:28 -0600 Subject: [PATCH 1/4] Remove uncalled static method --- src/main/java/hudson/plugins/git/GitSCM.java | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/main/java/hudson/plugins/git/GitSCM.java b/src/main/java/hudson/plugins/git/GitSCM.java index 45dba933e6..410961919e 100644 --- a/src/main/java/hudson/plugins/git/GitSCM.java +++ b/src/main/java/hudson/plugins/git/GitSCM.java @@ -958,10 +958,6 @@ private static StandardUsernameCredentials lookupScanCredentials(@NonNull Run Date: Sat, 1 Jul 2023 13:06:34 -0600 Subject: [PATCH 2/4] Remove redundant null checks in AbstractGitSCMSource --- src/main/java/jenkins/plugins/git/AbstractGitSCMSource.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/main/java/jenkins/plugins/git/AbstractGitSCMSource.java b/src/main/java/jenkins/plugins/git/AbstractGitSCMSource.java index bad590804c..d67c897706 100644 --- a/src/main/java/jenkins/plugins/git/AbstractGitSCMSource.java +++ b/src/main/java/jenkins/plugins/git/AbstractGitSCMSource.java @@ -1136,7 +1136,7 @@ protected List retrieveActions(@CheckForNull SCMSourceEvent event, @NonN target = target.substring(Constants.R_HEADS.length()); } List result = new ArrayList<>(); - if (target != null && !target.isBlank()) { + if (!target.isBlank()) { result.add(new GitRemoteHeadRefAction(getRemote(), target)); } return result; @@ -1159,7 +1159,7 @@ protected List retrieveActions(@CheckForNull SCMSourceEvent event, @NonN target = target.substring(Constants.R_HEADS.length()); } List result = new ArrayList<>(); - if (target != null && !target.isBlank()) { + if (!target.isBlank()) { result.add(new GitRemoteHeadRefAction(getRemote(), target)); } return result; @@ -1188,7 +1188,7 @@ protected List retrieveActions(@CheckForNull SCMSourceEvent event, @NonN target = target.substring(Constants.R_HEADS.length()); } List result = new ArrayList<>(); - if (target != null && !target.isBlank()) { + if (!target.isBlank()) { result.add(new GitRemoteHeadRefAction(getRemote(), target)); } return result; From 2931c164ff8cac83a21e630765ecc8c2605cecfb Mon Sep 17 00:00:00 2001 From: Mark Waite Date: Sat, 1 Jul 2023 13:41:02 -0600 Subject: [PATCH 3/4] Fix spotbugs warning on repo cache size updates Spotbugs correctly reported a race condition where the check for insertion of a value might report the value is missing then another thread inserted the value before the current thread performed the `put`. Simpler to `put` the value every time the method is called and then check for the rare case when the value that was put is smaller than the value that was there perviously. Repository size cache is used as a hint. Errors in the size cache may lead to suboptimal choices temporarily, but they should not lead to incorrect behavior. --- .../jenkins/plugins/git/GitToolChooser.java | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/src/main/java/jenkins/plugins/git/GitToolChooser.java b/src/main/java/jenkins/plugins/git/GitToolChooser.java index ef71ba783c..faf8c043fb 100644 --- a/src/main/java/jenkins/plugins/git/GitToolChooser.java +++ b/src/main/java/jenkins/plugins/git/GitToolChooser.java @@ -269,17 +269,15 @@ private boolean setSizeFromInternalCache(String repoURL) { /** Cache the estimated repository size for variants of repository URL */ private void assignSizeToInternalCache(String repoURL, long repoSize) { repoURL = convertToCanonicalURL(repoURL); - if (repositorySizeCache.containsKey(repoURL)) { - long oldSize = repositorySizeCache.get(repoURL); - if (oldSize < repoSize) { - LOGGER.log(Level.FINE, "Replacing old repo size {0} with new size {1} for repo {2}", new Object[]{oldSize, repoSize, repoURL}); - repositorySizeCache.put(repoURL, repoSize); - } else if (oldSize > repoSize) { - LOGGER.log(Level.FINE, "Ignoring new size {1} in favor of old size {0} for repo {2}", new Object[]{oldSize, repoSize, repoURL}); - } - } else { + Long oldSize = repositorySizeCache.put(repoURL, repoSize); + if (oldSize == null) { LOGGER.log(Level.FINE, "Caching repo size {0} for repo {1}", new Object[]{repoSize, repoURL}); - repositorySizeCache.put(repoURL, repoSize); + } else if (oldSize < repoSize) { + LOGGER.log(Level.FINE, "Replaced old repo size {0} with new size {1} for repo {2}", new Object[]{oldSize, repoSize, repoURL}); + } else if (oldSize > repoSize) { + /* Put back the larger old repo size and log a warning. This is not harmful but should be quite rare */ + LOGGER.log(Level.WARNING, "Ignoring new repo size {1} in favor of old size {0} for repo {2}", new Object[]{oldSize, repoSize, repoURL}); + repositorySizeCache.put(repoURL, oldSize); } } From 840c41069d216b7c8340f60890d7c81c9f54b733 Mon Sep 17 00:00:00 2001 From: Mark Waite Date: Sat, 1 Jul 2023 13:10:23 -0600 Subject: [PATCH 4/4] Increase spotbugs checks Add spotbugs exclusions reported as part of the increased checks. --- pom.xml | 2 +- src/spotbugs/excludesFilter.xml | 77 +++++++++++++++++++++++++++++++++ 2 files changed, 78 insertions(+), 1 deletion(-) create mode 100644 src/spotbugs/excludesFilter.xml diff --git a/pom.xml b/pom.xml index 9bacb86381..1c3ba0b171 100644 --- a/pom.xml +++ b/pom.xml @@ -80,7 +80,7 @@ false Max - Medium + Low diff --git a/src/spotbugs/excludesFilter.xml b/src/spotbugs/excludesFilter.xml new file mode 100644 index 0000000000..bf1e7ef977 --- /dev/null +++ b/src/spotbugs/excludesFilter.xml @@ -0,0 +1,77 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + +