From 0fd76114e3436ac635641d06371fd8833179312d Mon Sep 17 00:00:00 2001 From: Matthias Sohn Date: Fri, 6 Sep 2024 13:42:27 +0200 Subject: Replace custom encoder Constants#encodeASCII by JDK implementation Ensure that the method still throws an IllegalArgumentException for malformed input or if the String contains unmappable characters. Change-Id: I6a340aa1af60c315272ff13b6bf2041ba30c94ca --- .../src/org/eclipse/jgit/lib/Constants.java | 20 +++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/lib/Constants.java b/org.eclipse.jgit/src/org/eclipse/jgit/lib/Constants.java index c27047f25d..cb4a5b51fe 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/lib/Constants.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/lib/Constants.java @@ -12,8 +12,13 @@ package org.eclipse.jgit.lib; +import static java.nio.charset.StandardCharsets.US_ASCII; import static java.nio.charset.StandardCharsets.UTF_8; +import java.nio.CharBuffer; +import java.nio.charset.CharacterCodingException; +import java.nio.charset.CharsetEncoder; +import java.nio.charset.CodingErrorAction; import java.security.MessageDigest; import java.security.NoSuchAlgorithmException; import java.text.MessageFormat; @@ -714,14 +719,15 @@ public final class Constants { * the 7-bit ASCII character space. */ public static byte[] encodeASCII(String s) { - final byte[] r = new byte[s.length()]; - for (int k = r.length - 1; k >= 0; k--) { - final char c = s.charAt(k); - if (c > 127) - throw new IllegalArgumentException(MessageFormat.format(JGitText.get().notASCIIString, s)); - r[k] = (byte) c; + try { + CharsetEncoder encoder = US_ASCII.newEncoder() + .onUnmappableCharacter(CodingErrorAction.REPORT) + .onMalformedInput(CodingErrorAction.REPORT); + return encoder.encode(CharBuffer.wrap(s)).array(); + } catch (CharacterCodingException e) { + throw new IllegalArgumentException( + MessageFormat.format(JGitText.get().notASCIIString, s), e); } - return r; } /** -- cgit v1.2.3 From c99c8b9d00063fd177e4e895ac3e155cd2a0b437 Mon Sep 17 00:00:00 2001 From: Kamil Musin Date: Tue, 8 Oct 2024 13:25:53 +0200 Subject: Remove unnecessary argument handler in MergeBase.java Currently the parsing of command line arguments always results in an error. ``` fatal: Argument "commit-ish" is required ``` This is caused by the fact that the first method (commit_0) is invoked twice. (As if the multiValued = true). I couldn't figure out, why that would be the case, as it's defaults to false and RevCommit handler doesn't seem to have a logic that would override that either. In any case, defining Arguments like this is unnecessary as it's sufficient to define a single ArrayList @Argument that will accumulate all commits. Change-Id: Ifd1278b3059f86618db05bfe264dd5c00879d82b --- org.eclipse.jgit.pgm/src/org/eclipse/jgit/pgm/MergeBase.java | 5 ----- 1 file changed, 5 deletions(-) diff --git a/org.eclipse.jgit.pgm/src/org/eclipse/jgit/pgm/MergeBase.java b/org.eclipse.jgit.pgm/src/org/eclipse/jgit/pgm/MergeBase.java index aacde2f430..a29c4d9f36 100644 --- a/org.eclipse.jgit.pgm/src/org/eclipse/jgit/pgm/MergeBase.java +++ b/org.eclipse.jgit.pgm/src/org/eclipse/jgit/pgm/MergeBase.java @@ -26,11 +26,6 @@ class MergeBase extends TextBuiltin { private boolean all; @Argument(index = 0, metaVar = "metaVar_commitish", required = true) - void commit_0(final RevCommit c) { - commits.add(c); - } - - @Argument(index = 1, metaVar = "metaVar_commitish", required = true) private List commits = new ArrayList<>(); @Override -- cgit v1.2.3 From 462b15633054ea46ccb9d3d1b69b9b81b924dcb1 Mon Sep 17 00:00:00 2001 From: Matthias Sohn Date: Thu, 24 Oct 2024 13:00:42 +0200 Subject: Update bytebuddy to 1.15.7 Change-Id: I8f718f0b06887c8ebfe70941dc2730604f0790e0 --- WORKSPACE | 6 +++--- org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.17.target | 6 +++--- org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.18.target | 6 +++--- org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.19.target | 6 +++--- org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.20.target | 6 +++--- org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.21.target | 6 +++--- org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.22.target | 6 +++--- org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.23.target | 6 +++--- org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.24.target | 6 +++--- org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.25.target | 6 +++--- org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.26.target | 6 +++--- org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.27.target | 6 +++--- org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.28.target | 6 +++--- org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.29.target | 6 +++--- org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.30.target | 6 +++--- org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.31.target | 6 +++--- org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.32.target | 6 +++--- org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.33.target | 6 +++--- org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.34.target | 6 +++--- .../org.eclipse.jgit.target/maven/dependencies.tpd | 4 ++-- pom.xml | 2 +- 21 files changed, 60 insertions(+), 60 deletions(-) diff --git a/WORKSPACE b/WORKSPACE index 46993dd476..0f5472653a 100644 --- a/WORKSPACE +++ b/WORKSPACE @@ -220,18 +220,18 @@ maven_jar( sha1 = "0d26263eb7524252d98e602fc6942996a3195e29", ) -BYTE_BUDDY_VERSION = "1.15.3" +BYTE_BUDDY_VERSION = "1.15.7" maven_jar( name = "bytebuddy", artifact = "net.bytebuddy:byte-buddy:" + BYTE_BUDDY_VERSION, - sha1 = "01b3069696cd9ed55d90b9114ffe3429035ff924", + sha1 = "658064662e33b045bb9499b1c8afeae656974b18", ) maven_jar( name = "bytebuddy-agent", artifact = "net.bytebuddy:byte-buddy-agent:" + BYTE_BUDDY_VERSION, - sha1 = "e619d89ed41a6cedc23bee3549cec8c4ffdaee7b", + sha1 = "0f0a8a3b1440714d548ec54093eb34e3f2726013", ) maven_jar( diff --git a/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.17.target b/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.17.target index d2006b8d60..8b51595fa3 100644 --- a/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.17.target +++ b/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.17.target @@ -1,7 +1,7 @@ - + @@ -193,13 +193,13 @@ net.bytebuddy byte-buddy - 1.15.3 + 1.15.7 jar net.bytebuddy byte-buddy-agent - 1.15.3 + 1.15.7 jar diff --git a/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.18.target b/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.18.target index 882f4e1313..39542d7ec0 100644 --- a/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.18.target +++ b/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.18.target @@ -1,7 +1,7 @@ - + @@ -193,13 +193,13 @@ net.bytebuddy byte-buddy - 1.15.3 + 1.15.7 jar net.bytebuddy byte-buddy-agent - 1.15.3 + 1.15.7 jar diff --git a/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.19.target b/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.19.target index 60e0f8f76f..ab43b5e721 100644 --- a/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.19.target +++ b/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.19.target @@ -1,7 +1,7 @@ - + @@ -193,13 +193,13 @@ net.bytebuddy byte-buddy - 1.15.3 + 1.15.7 jar net.bytebuddy byte-buddy-agent - 1.15.3 + 1.15.7 jar diff --git a/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.20.target b/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.20.target index b0601e8909..f7f577703e 100644 --- a/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.20.target +++ b/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.20.target @@ -1,7 +1,7 @@ - + @@ -193,13 +193,13 @@ net.bytebuddy byte-buddy - 1.15.3 + 1.15.7 jar net.bytebuddy byte-buddy-agent - 1.15.3 + 1.15.7 jar diff --git a/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.21.target b/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.21.target index bd3625d4f5..7096f30533 100644 --- a/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.21.target +++ b/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.21.target @@ -1,7 +1,7 @@ - + @@ -193,13 +193,13 @@ net.bytebuddy byte-buddy - 1.15.3 + 1.15.7 jar net.bytebuddy byte-buddy-agent - 1.15.3 + 1.15.7 jar diff --git a/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.22.target b/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.22.target index 5006956a61..46202ab61e 100644 --- a/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.22.target +++ b/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.22.target @@ -1,7 +1,7 @@ - + @@ -193,13 +193,13 @@ net.bytebuddy byte-buddy - 1.15.3 + 1.15.7 jar net.bytebuddy byte-buddy-agent - 1.15.3 + 1.15.7 jar diff --git a/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.23.target b/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.23.target index 68124f196b..0c3729d8d4 100644 --- a/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.23.target +++ b/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.23.target @@ -1,7 +1,7 @@ - + @@ -193,13 +193,13 @@ net.bytebuddy byte-buddy - 1.15.3 + 1.15.7 jar net.bytebuddy byte-buddy-agent - 1.15.3 + 1.15.7 jar diff --git a/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.24.target b/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.24.target index 9d26a8930c..f0db662c05 100644 --- a/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.24.target +++ b/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.24.target @@ -1,7 +1,7 @@ - + @@ -193,13 +193,13 @@ net.bytebuddy byte-buddy - 1.15.3 + 1.15.7 jar net.bytebuddy byte-buddy-agent - 1.15.3 + 1.15.7 jar diff --git a/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.25.target b/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.25.target index 6ad784208f..63a44d93c2 100644 --- a/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.25.target +++ b/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.25.target @@ -1,7 +1,7 @@ - + @@ -193,13 +193,13 @@ net.bytebuddy byte-buddy - 1.15.3 + 1.15.7 jar net.bytebuddy byte-buddy-agent - 1.15.3 + 1.15.7 jar diff --git a/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.26.target b/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.26.target index 8bff6b28c2..1b9e704c71 100644 --- a/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.26.target +++ b/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.26.target @@ -1,7 +1,7 @@ - + @@ -193,13 +193,13 @@ net.bytebuddy byte-buddy - 1.15.3 + 1.15.7 jar net.bytebuddy byte-buddy-agent - 1.15.3 + 1.15.7 jar diff --git a/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.27.target b/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.27.target index ef4a71aa3e..e67dae5c8c 100644 --- a/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.27.target +++ b/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.27.target @@ -1,7 +1,7 @@ - + @@ -193,13 +193,13 @@ net.bytebuddy byte-buddy - 1.15.3 + 1.15.7 jar net.bytebuddy byte-buddy-agent - 1.15.3 + 1.15.7 jar diff --git a/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.28.target b/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.28.target index 14e81823c9..2abfcced4f 100644 --- a/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.28.target +++ b/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.28.target @@ -1,7 +1,7 @@ - + @@ -193,13 +193,13 @@ net.bytebuddy byte-buddy - 1.15.3 + 1.15.7 jar net.bytebuddy byte-buddy-agent - 1.15.3 + 1.15.7 jar diff --git a/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.29.target b/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.29.target index 7e121c4d2a..376f75757c 100644 --- a/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.29.target +++ b/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.29.target @@ -1,7 +1,7 @@ - + @@ -193,13 +193,13 @@ net.bytebuddy byte-buddy - 1.15.3 + 1.15.7 jar net.bytebuddy byte-buddy-agent - 1.15.3 + 1.15.7 jar diff --git a/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.30.target b/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.30.target index 2ccc22db58..12e3c441f6 100644 --- a/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.30.target +++ b/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.30.target @@ -1,7 +1,7 @@ - + @@ -193,13 +193,13 @@ net.bytebuddy byte-buddy - 1.15.3 + 1.15.7 jar net.bytebuddy byte-buddy-agent - 1.15.3 + 1.15.7 jar diff --git a/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.31.target b/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.31.target index dfedead946..dfd8a26c5f 100644 --- a/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.31.target +++ b/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.31.target @@ -1,7 +1,7 @@ - + @@ -193,13 +193,13 @@ net.bytebuddy byte-buddy - 1.15.3 + 1.15.7 jar net.bytebuddy byte-buddy-agent - 1.15.3 + 1.15.7 jar diff --git a/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.32.target b/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.32.target index 96ed4f1b80..c353b2ef7f 100644 --- a/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.32.target +++ b/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.32.target @@ -1,7 +1,7 @@ - + @@ -193,13 +193,13 @@ net.bytebuddy byte-buddy - 1.15.3 + 1.15.7 jar net.bytebuddy byte-buddy-agent - 1.15.3 + 1.15.7 jar diff --git a/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.33.target b/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.33.target index 8b4a174f12..8f6e7f12a4 100644 --- a/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.33.target +++ b/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.33.target @@ -1,7 +1,7 @@ - + @@ -193,13 +193,13 @@ net.bytebuddy byte-buddy - 1.15.3 + 1.15.7 jar net.bytebuddy byte-buddy-agent - 1.15.3 + 1.15.7 jar diff --git a/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.34.target b/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.34.target index 2ec0ba4fa6..2d37e415c1 100644 --- a/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.34.target +++ b/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.34.target @@ -1,7 +1,7 @@ - + @@ -193,13 +193,13 @@ net.bytebuddy byte-buddy - 1.15.3 + 1.15.7 jar net.bytebuddy byte-buddy-agent - 1.15.3 + 1.15.7 jar diff --git a/org.eclipse.jgit.packaging/org.eclipse.jgit.target/maven/dependencies.tpd b/org.eclipse.jgit.packaging/org.eclipse.jgit.target/maven/dependencies.tpd index e2889b87c6..47443e2f86 100644 --- a/org.eclipse.jgit.packaging/org.eclipse.jgit.target/maven/dependencies.tpd +++ b/org.eclipse.jgit.packaging/org.eclipse.jgit.target/maven/dependencies.tpd @@ -97,12 +97,12 @@ maven bytebuddy dependency { groupId = "net.bytebuddy" artifactId = "byte-buddy" - version = "1.15.3" + version = "1.15.7" } dependency { groupId = "net.bytebuddy" artifactId = "byte-buddy-agent" - version = "1.15.3" + version = "1.15.7" } } diff --git a/pom.xml b/pom.xml index c83ee3eaaf..bcc8973190 100644 --- a/pom.xml +++ b/pom.xml @@ -148,7 +148,7 @@ 2.2 3.26.3 5.15.0 - 1.15.3 + 1.15.7 jacoco -- cgit v1.2.3 From 8188c4a773490bbde9b997045cdbcbbf0d480e9b Mon Sep 17 00:00:00 2001 From: Matthias Sohn Date: Thu, 24 Oct 2024 13:02:21 +0200 Subject: Update mockito to 5.14.2 Change-Id: I0e6442a86ba1790680b832d62d4b04df0713da5f --- WORKSPACE | 4 ++-- org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.17.target | 4 ++-- org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.18.target | 4 ++-- org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.19.target | 4 ++-- org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.20.target | 4 ++-- org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.21.target | 4 ++-- org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.22.target | 4 ++-- org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.23.target | 4 ++-- org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.24.target | 4 ++-- org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.25.target | 4 ++-- org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.26.target | 4 ++-- org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.27.target | 4 ++-- org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.28.target | 4 ++-- org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.29.target | 4 ++-- org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.30.target | 4 ++-- org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.31.target | 4 ++-- org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.32.target | 4 ++-- org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.33.target | 4 ++-- org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.34.target | 4 ++-- .../org.eclipse.jgit.target/maven/dependencies.tpd | 2 +- pom.xml | 2 +- 21 files changed, 40 insertions(+), 40 deletions(-) diff --git a/WORKSPACE b/WORKSPACE index 0f5472653a..d1ed57487d 100644 --- a/WORKSPACE +++ b/WORKSPACE @@ -210,8 +210,8 @@ maven_jar( maven_jar( name = "mockito", - artifact = "org.mockito:mockito-core:5.14.1", - sha1 = "a89b0ce9ee5d92646522caeb27fb92c02a0b4c55", + artifact = "org.mockito:mockito-core:5.14.2", + sha1 = "f7bf936008d7664e2002c3faf0c02071c8d10e7c", ) maven_jar( diff --git a/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.17.target b/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.17.target index 8b51595fa3..872ff86d22 100644 --- a/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.17.target +++ b/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.17.target @@ -1,7 +1,7 @@ - + @@ -79,7 +79,7 @@ org.mockito mockito-core - 5.14.1 + 5.14.2 jar diff --git a/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.18.target b/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.18.target index 39542d7ec0..d58275a415 100644 --- a/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.18.target +++ b/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.18.target @@ -1,7 +1,7 @@ - + @@ -79,7 +79,7 @@ org.mockito mockito-core - 5.14.1 + 5.14.2 jar diff --git a/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.19.target b/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.19.target index ab43b5e721..be15fdd693 100644 --- a/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.19.target +++ b/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.19.target @@ -1,7 +1,7 @@ - + @@ -79,7 +79,7 @@ org.mockito mockito-core - 5.14.1 + 5.14.2 jar diff --git a/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.20.target b/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.20.target index f7f577703e..7de5d871d8 100644 --- a/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.20.target +++ b/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.20.target @@ -1,7 +1,7 @@ - + @@ -79,7 +79,7 @@ org.mockito mockito-core - 5.14.1 + 5.14.2 jar diff --git a/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.21.target b/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.21.target index 7096f30533..6a5e8160b3 100644 --- a/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.21.target +++ b/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.21.target @@ -1,7 +1,7 @@ - + @@ -79,7 +79,7 @@ org.mockito mockito-core - 5.14.1 + 5.14.2 jar diff --git a/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.22.target b/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.22.target index 46202ab61e..4e860ff28a 100644 --- a/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.22.target +++ b/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.22.target @@ -1,7 +1,7 @@ - + @@ -79,7 +79,7 @@ org.mockito mockito-core - 5.14.1 + 5.14.2 jar diff --git a/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.23.target b/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.23.target index 0c3729d8d4..7af598f034 100644 --- a/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.23.target +++ b/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.23.target @@ -1,7 +1,7 @@ - + @@ -79,7 +79,7 @@ org.mockito mockito-core - 5.14.1 + 5.14.2 jar diff --git a/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.24.target b/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.24.target index f0db662c05..2ca0d56c60 100644 --- a/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.24.target +++ b/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.24.target @@ -1,7 +1,7 @@ - + @@ -79,7 +79,7 @@ org.mockito mockito-core - 5.14.1 + 5.14.2 jar diff --git a/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.25.target b/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.25.target index 63a44d93c2..262ae68cee 100644 --- a/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.25.target +++ b/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.25.target @@ -1,7 +1,7 @@ - + @@ -79,7 +79,7 @@ org.mockito mockito-core - 5.14.1 + 5.14.2 jar diff --git a/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.26.target b/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.26.target index 1b9e704c71..e33f0b0c1a 100644 --- a/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.26.target +++ b/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.26.target @@ -1,7 +1,7 @@ - + @@ -79,7 +79,7 @@ org.mockito mockito-core - 5.14.1 + 5.14.2 jar diff --git a/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.27.target b/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.27.target index e67dae5c8c..65fa024ada 100644 --- a/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.27.target +++ b/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.27.target @@ -1,7 +1,7 @@ - + @@ -79,7 +79,7 @@ org.mockito mockito-core - 5.14.1 + 5.14.2 jar diff --git a/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.28.target b/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.28.target index 2abfcced4f..e189127718 100644 --- a/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.28.target +++ b/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.28.target @@ -1,7 +1,7 @@ - + @@ -79,7 +79,7 @@ org.mockito mockito-core - 5.14.1 + 5.14.2 jar diff --git a/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.29.target b/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.29.target index 376f75757c..49442a97f4 100644 --- a/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.29.target +++ b/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.29.target @@ -1,7 +1,7 @@ - + @@ -79,7 +79,7 @@ org.mockito mockito-core - 5.14.1 + 5.14.2 jar diff --git a/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.30.target b/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.30.target index 12e3c441f6..18c4707c82 100644 --- a/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.30.target +++ b/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.30.target @@ -1,7 +1,7 @@ - + @@ -79,7 +79,7 @@ org.mockito mockito-core - 5.14.1 + 5.14.2 jar diff --git a/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.31.target b/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.31.target index dfd8a26c5f..0c23c0c176 100644 --- a/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.31.target +++ b/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.31.target @@ -1,7 +1,7 @@ - + @@ -79,7 +79,7 @@ org.mockito mockito-core - 5.14.1 + 5.14.2 jar diff --git a/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.32.target b/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.32.target index c353b2ef7f..e6ee190905 100644 --- a/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.32.target +++ b/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.32.target @@ -1,7 +1,7 @@ - + @@ -79,7 +79,7 @@ org.mockito mockito-core - 5.14.1 + 5.14.2 jar diff --git a/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.33.target b/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.33.target index 8f6e7f12a4..4cc1959d01 100644 --- a/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.33.target +++ b/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.33.target @@ -1,7 +1,7 @@ - + @@ -79,7 +79,7 @@ org.mockito mockito-core - 5.14.1 + 5.14.2 jar diff --git a/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.34.target b/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.34.target index 2d37e415c1..0dfe190bc7 100644 --- a/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.34.target +++ b/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.34.target @@ -1,7 +1,7 @@ - + @@ -79,7 +79,7 @@ org.mockito mockito-core - 5.14.1 + 5.14.2 jar diff --git a/org.eclipse.jgit.packaging/org.eclipse.jgit.target/maven/dependencies.tpd b/org.eclipse.jgit.packaging/org.eclipse.jgit.target/maven/dependencies.tpd index 47443e2f86..5e29950bc0 100644 --- a/org.eclipse.jgit.packaging/org.eclipse.jgit.target/maven/dependencies.tpd +++ b/org.eclipse.jgit.packaging/org.eclipse.jgit.target/maven/dependencies.tpd @@ -225,7 +225,7 @@ maven mockito dependency { groupId = "org.mockito" artifactId = "mockito-core" - version = "5.14.1" + version = "5.14.2" } } diff --git a/pom.xml b/pom.xml index bcc8973190..00a89e3c10 100644 --- a/pom.xml +++ b/pom.xml @@ -1007,7 +1007,7 @@ org.mockito mockito-core - 5.14.1 + 5.14.2 -- cgit v1.2.3 From 4402f10247169cce87fe950c7e6647bf44298ebd Mon Sep 17 00:00:00 2001 From: Laura Hamelin Date: Wed, 24 Jul 2024 14:01:38 -0700 Subject: dfs: update getBlockCacheStats to return a List of BlockCacheStats Make available all underlying cache table stats for the used cache table implementation. The existing cache table stats implementation only allows a "global" view of the cache table statistics; it does not differentiate between all possible underlying cache tables used. This change allows callers to get the block cache stats broken down per underlying table. These cache stats are intended to be used for monitoring all cache tables independently. Existing usages of getBlockCacheStats now make use of AggregatedBlockCacheStats.fromStatsList to aggregate the list of BlockCacheStats into a single BlockCacheStats instance. Change-Id: I261b3f2849857172397657e5c674b11e09807f27 --- .../storage/dfs/AggregatedBlockCacheStatsTest.java | 23 +++--- .../storage/dfs/ClockBlockCacheTableTest.java | 22 ++++- .../storage/dfs/PackExtBlockCacheTableTest.java | 93 +++++++++++++++++----- .../storage/dfs/AggregatedBlockCacheStats.java | 12 +-- .../internal/storage/dfs/ClockBlockCacheTable.java | 5 +- .../jgit/internal/storage/dfs/DfsBlockCache.java | 31 ++++++-- .../internal/storage/dfs/DfsBlockCacheTable.java | 15 ++-- .../storage/dfs/PackExtBlockCacheTable.java | 9 +-- 8 files changed, 150 insertions(+), 60 deletions(-) diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/dfs/AggregatedBlockCacheStatsTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/dfs/AggregatedBlockCacheStatsTest.java index ac769498e2..2c4b432a01 100644 --- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/dfs/AggregatedBlockCacheStatsTest.java +++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/dfs/AggregatedBlockCacheStatsTest.java @@ -24,9 +24,10 @@ public class AggregatedBlockCacheStatsTest { @Test public void getName() { BlockCacheStats aggregatedBlockCacheStats = AggregatedBlockCacheStats - .fromStatsList("name", List.of()); + .fromStatsList(List.of()); - assertThat(aggregatedBlockCacheStats.getName(), equalTo("name")); + assertThat(aggregatedBlockCacheStats.getName(), + equalTo(AggregatedBlockCacheStats.class.getName())); } @Test @@ -46,8 +47,7 @@ public class AggregatedBlockCacheStatsTest { currentSizes[PackExt.INDEX.getPosition()] = 7; BlockCacheStats aggregatedBlockCacheStats = AggregatedBlockCacheStats - .fromStatsList("name", - List.of(packStats, bitmapStats, indexStats)); + .fromStatsList(List.of(packStats, bitmapStats, indexStats)); assertArrayEquals(aggregatedBlockCacheStats.getCurrentSize(), currentSizes); @@ -73,8 +73,7 @@ public class AggregatedBlockCacheStatsTest { hitCounts[PackExt.INDEX.getPosition()] = 7; BlockCacheStats aggregatedBlockCacheStats = AggregatedBlockCacheStats - .fromStatsList("name", - List.of(packStats, bitmapStats, indexStats)); + .fromStatsList(List.of(packStats, bitmapStats, indexStats)); assertArrayEquals(aggregatedBlockCacheStats.getHitCount(), hitCounts); } @@ -99,8 +98,7 @@ public class AggregatedBlockCacheStatsTest { missCounts[PackExt.INDEX.getPosition()] = 7; BlockCacheStats aggregatedBlockCacheStats = AggregatedBlockCacheStats - .fromStatsList("name", - List.of(packStats, bitmapStats, indexStats)); + .fromStatsList(List.of(packStats, bitmapStats, indexStats)); assertArrayEquals(aggregatedBlockCacheStats.getMissCount(), missCounts); } @@ -131,8 +129,7 @@ public class AggregatedBlockCacheStatsTest { totalRequestCounts[PackExt.INDEX.getPosition()] = 14; BlockCacheStats aggregatedBlockCacheStats = AggregatedBlockCacheStats - .fromStatsList("name", - List.of(packStats, bitmapStats, indexStats)); + .fromStatsList(List.of(packStats, bitmapStats, indexStats)); assertArrayEquals(aggregatedBlockCacheStats.getTotalRequestCount(), totalRequestCounts); @@ -160,8 +157,7 @@ public class AggregatedBlockCacheStatsTest { hitRatios[PackExt.INDEX.getPosition()] = 0; BlockCacheStats aggregatedBlockCacheStats = AggregatedBlockCacheStats - .fromStatsList("Name", - List.of(packStats, bitmapStats, indexStats)); + .fromStatsList(List.of(packStats, bitmapStats, indexStats)); assertArrayEquals(aggregatedBlockCacheStats.getHitRatio(), hitRatios); } @@ -186,8 +182,7 @@ public class AggregatedBlockCacheStatsTest { evictions[PackExt.INDEX.getPosition()] = 7; BlockCacheStats aggregatedBlockCacheStats = AggregatedBlockCacheStats - .fromStatsList("Name", - List.of(packStats, bitmapStats, indexStats)); + .fromStatsList(List.of(packStats, bitmapStats, indexStats)); assertArrayEquals(aggregatedBlockCacheStats.getEvictions(), evictions); } diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/dfs/ClockBlockCacheTableTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/dfs/ClockBlockCacheTableTest.java index cb68bbc515..2e2f86bf80 100644 --- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/dfs/ClockBlockCacheTableTest.java +++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/dfs/ClockBlockCacheTableTest.java @@ -1,8 +1,14 @@ package org.eclipse.jgit.internal.storage.dfs; import static org.eclipse.jgit.internal.storage.dfs.DfsBlockCacheConfig.DEFAULT_NAME; +import static org.eclipse.jgit.internal.storage.dfs.DfsBlockCacheTable.BlockCacheStats; import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.contains; import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.hasSize; +import static org.hamcrest.Matchers.isA; + +import java.util.List; import org.junit.Test; @@ -30,7 +36,8 @@ public class ClockBlockCacheTableTest { ClockBlockCacheTable cacheTable = new ClockBlockCacheTable( createBlockCacheConfig()); - assertThat(cacheTable.getBlockCacheStats().getName(), + assertThat(cacheTable.getBlockCacheStats(), hasSize(1)); + assertThat(cacheTable.getBlockCacheStats().get(0).getName(), equalTo(DEFAULT_NAME)); } @@ -39,7 +46,18 @@ public class ClockBlockCacheTableTest { ClockBlockCacheTable cacheTable = new ClockBlockCacheTable( createBlockCacheConfig().setName(NAME)); - assertThat(cacheTable.getBlockCacheStats().getName(), equalTo(NAME)); + assertThat(cacheTable.getBlockCacheStats(), hasSize(1)); + assertThat(cacheTable.getBlockCacheStats().get(0).getName(), + equalTo(NAME)); + } + + @Test + public void getAllBlockCacheStats() { + ClockBlockCacheTable cacheTable = new ClockBlockCacheTable( + createBlockCacheConfig()); + + List blockCacheStats = cacheTable.getBlockCacheStats(); + assertThat(blockCacheStats, contains(isA(BlockCacheStats.class))); } private static DfsBlockCacheConfig createBlockCacheConfig() { diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/dfs/PackExtBlockCacheTableTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/dfs/PackExtBlockCacheTableTest.java index c5c964bcab..e7627bc4ab 100644 --- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/dfs/PackExtBlockCacheTableTest.java +++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/dfs/PackExtBlockCacheTableTest.java @@ -10,8 +10,10 @@ package org.eclipse.jgit.internal.storage.dfs; +import static org.eclipse.jgit.internal.storage.dfs.DfsBlockCacheTable.BlockCacheStats; import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.hasSize; import static org.hamcrest.Matchers.sameInstance; import static org.junit.Assert.assertArrayEquals; import static org.junit.Assert.assertNotNull; @@ -398,15 +400,51 @@ public class PackExtBlockCacheTableTest { } @Test - public void getBlockCacheStats_getName_returnsPackExtCacheTableName() { - DfsBlockCacheStats packStats = new DfsBlockCacheStats(); - PackExtBlockCacheTable tables = PackExtBlockCacheTable.fromCacheTables( - cacheTableWithStats(/* name= */ "defaultName", packStats), - Map.of(PackExt.PACK, cacheTableWithStats(/* name= */ "packName", - packStats))); + public void getAllBlockCacheStats() { + String defaultTableName = "default table"; + DfsBlockCacheStats defaultStats = new DfsBlockCacheStats( + defaultTableName); + incrementCounter(4, + () -> defaultStats.incrementHit(new TestKey(PackExt.REFTABLE))); + + String packTableName = "pack table"; + DfsBlockCacheStats packStats = new DfsBlockCacheStats(packTableName); + incrementCounter(5, + () -> packStats.incrementHit(new TestKey(PackExt.PACK))); - assertThat(tables.getBlockCacheStats().getName(), - equalTo("defaultName,packName")); + String bitmapTableName = "bitmap table"; + DfsBlockCacheStats bitmapStats = new DfsBlockCacheStats( + bitmapTableName); + incrementCounter(6, () -> bitmapStats + .incrementHit(new TestKey(PackExt.BITMAP_INDEX))); + + DfsBlockCacheTable defaultTable = cacheTableWithStats(defaultStats); + DfsBlockCacheTable packTable = cacheTableWithStats(packStats); + DfsBlockCacheTable bitmapTable = cacheTableWithStats(bitmapStats); + PackExtBlockCacheTable tables = PackExtBlockCacheTable + .fromCacheTables(defaultTable, Map.of(PackExt.PACK, packTable, + PackExt.BITMAP_INDEX, bitmapTable)); + + List statsList = tables.getBlockCacheStats(); + assertThat(statsList, hasSize(3)); + + long[] defaultTableHitCounts = createEmptyStatsArray(); + defaultTableHitCounts[PackExt.REFTABLE.getPosition()] = 4; + assertArrayEquals( + getCacheStatsByName(statsList, defaultTableName).getHitCount(), + defaultTableHitCounts); + + long[] packTableHitCounts = createEmptyStatsArray(); + packTableHitCounts[PackExt.PACK.getPosition()] = 5; + assertArrayEquals( + getCacheStatsByName(statsList, packTableName).getHitCount(), + packTableHitCounts); + + long[] bitmapHitCounts = createEmptyStatsArray(); + bitmapHitCounts[PackExt.BITMAP_INDEX.getPosition()] = 6; + assertArrayEquals( + getCacheStatsByName(statsList, bitmapTableName).getHitCount(), + bitmapHitCounts); } @Test @@ -431,7 +469,8 @@ public class PackExtBlockCacheTableTest { cacheTableWithStats(bitmapStats), PackExt.INDEX, cacheTableWithStats(indexStats))); - assertArrayEquals(tables.getBlockCacheStats().getCurrentSize(), + assertArrayEquals(AggregatedBlockCacheStats + .fromStatsList(tables.getBlockCacheStats()).getCurrentSize(), currentSizes); } @@ -460,7 +499,9 @@ public class PackExtBlockCacheTableTest { cacheTableWithStats(bitmapStats), PackExt.INDEX, cacheTableWithStats(indexStats))); - assertArrayEquals(tables.getBlockCacheStats().getHitCount(), hitCounts); + assertArrayEquals(AggregatedBlockCacheStats + .fromStatsList(tables.getBlockCacheStats()).getHitCount(), + hitCounts); } @Test @@ -488,7 +529,8 @@ public class PackExtBlockCacheTableTest { cacheTableWithStats(bitmapStats), PackExt.INDEX, cacheTableWithStats(indexStats))); - assertArrayEquals(tables.getBlockCacheStats().getMissCount(), + assertArrayEquals(AggregatedBlockCacheStats + .fromStatsList(tables.getBlockCacheStats()).getMissCount(), missCounts); } @@ -523,8 +565,9 @@ public class PackExtBlockCacheTableTest { cacheTableWithStats(bitmapStats), PackExt.INDEX, cacheTableWithStats(indexStats))); - assertArrayEquals(tables.getBlockCacheStats().getTotalRequestCount(), - totalRequestCounts); + assertArrayEquals(AggregatedBlockCacheStats + .fromStatsList(tables.getBlockCacheStats()) + .getTotalRequestCount(), totalRequestCounts); } @Test @@ -554,7 +597,9 @@ public class PackExtBlockCacheTableTest { cacheTableWithStats(bitmapStats), PackExt.INDEX, cacheTableWithStats(indexStats))); - assertArrayEquals(tables.getBlockCacheStats().getHitRatio(), hitRatios); + assertArrayEquals(AggregatedBlockCacheStats + .fromStatsList(tables.getBlockCacheStats()).getHitRatio(), + hitRatios); } @Test @@ -582,10 +627,21 @@ public class PackExtBlockCacheTableTest { cacheTableWithStats(bitmapStats), PackExt.INDEX, cacheTableWithStats(indexStats))); - assertArrayEquals(tables.getBlockCacheStats().getEvictions(), + assertArrayEquals(AggregatedBlockCacheStats + .fromStatsList(tables.getBlockCacheStats()).getEvictions(), evictions); } + private BlockCacheStats getCacheStatsByName( + List blockCacheStats, String name) { + for (BlockCacheStats entry : blockCacheStats) { + if (entry.getName().equals(name)) { + return entry; + } + } + return null; + } + private static void incrementCounter(int amount, Runnable fn) { for (int i = 0; i < amount; i++) { fn.run(); @@ -597,15 +653,16 @@ public class PackExtBlockCacheTableTest { } private static DfsBlockCacheTable cacheTableWithStats( - DfsBlockCacheStats dfsBlockCacheStats) { + BlockCacheStats dfsBlockCacheStats) { return cacheTableWithStats(CACHE_NAME, dfsBlockCacheStats); } private static DfsBlockCacheTable cacheTableWithStats(String name, - DfsBlockCacheStats dfsBlockCacheStats) { + BlockCacheStats dfsBlockCacheStats) { DfsBlockCacheTable cacheTable = mock(DfsBlockCacheTable.class); when(cacheTable.getName()).thenReturn(name); - when(cacheTable.getBlockCacheStats()).thenReturn(dfsBlockCacheStats); + when(cacheTable.getBlockCacheStats()) + .thenReturn(List.of(dfsBlockCacheStats)); return cacheTable; } diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/dfs/AggregatedBlockCacheStats.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/dfs/AggregatedBlockCacheStats.java index 743f4606c4..295b702fa7 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/dfs/AggregatedBlockCacheStats.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/dfs/AggregatedBlockCacheStats.java @@ -20,27 +20,23 @@ import org.eclipse.jgit.internal.storage.pack.PackExt; * Aggregates values for all given {@link BlockCacheStats}. */ class AggregatedBlockCacheStats implements BlockCacheStats { - private final String name; - private final List blockCacheStats; - static BlockCacheStats fromStatsList(String name, + static BlockCacheStats fromStatsList( List blockCacheStats) { if (blockCacheStats.size() == 1) { return blockCacheStats.get(0); } - return new AggregatedBlockCacheStats(name, blockCacheStats); + return new AggregatedBlockCacheStats(blockCacheStats); } - private AggregatedBlockCacheStats(String name, - List blockCacheStats) { - this.name = name; + private AggregatedBlockCacheStats(List blockCacheStats) { this.blockCacheStats = blockCacheStats; } @Override public String getName() { - return name; + return AggregatedBlockCacheStats.class.getName(); } @Override diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/dfs/ClockBlockCacheTable.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/dfs/ClockBlockCacheTable.java index bbee23b061..587d482583 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/dfs/ClockBlockCacheTable.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/dfs/ClockBlockCacheTable.java @@ -12,6 +12,7 @@ package org.eclipse.jgit.internal.storage.dfs; import java.io.IOException; import java.time.Duration; +import java.util.List; import java.util.Map; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.atomic.AtomicReferenceArray; @@ -141,8 +142,8 @@ final class ClockBlockCacheTable implements DfsBlockCacheTable { } @Override - public BlockCacheStats getBlockCacheStats() { - return dfsBlockCacheStats; + public List getBlockCacheStats() { + return List.of(dfsBlockCacheStats); } @Override diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/dfs/DfsBlockCache.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/dfs/DfsBlockCache.java index 0334450fbe..f8e0831e1f 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/dfs/DfsBlockCache.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/dfs/DfsBlockCache.java @@ -11,7 +11,10 @@ package org.eclipse.jgit.internal.storage.dfs; +import static org.eclipse.jgit.internal.storage.dfs.DfsBlockCacheTable.BlockCacheStats; + import java.io.IOException; +import java.util.List; import java.util.concurrent.atomic.AtomicInteger; import java.util.stream.LongStream; @@ -124,7 +127,7 @@ public final class DfsBlockCache { * @return total number of bytes in the cache, per pack file extension. */ public long[] getCurrentSize() { - return dfsBlockCacheTable.getBlockCacheStats().getCurrentSize(); + return getAggregatedBlockCacheStats().getCurrentSize(); } /** @@ -143,7 +146,7 @@ public final class DfsBlockCache { * extension. */ public long[] getHitCount() { - return dfsBlockCacheTable.getBlockCacheStats().getHitCount(); + return getAggregatedBlockCacheStats().getHitCount(); } /** @@ -154,7 +157,7 @@ public final class DfsBlockCache { * extension. */ public long[] getMissCount() { - return dfsBlockCacheTable.getBlockCacheStats().getMissCount(); + return getAggregatedBlockCacheStats().getMissCount(); } /** @@ -163,7 +166,7 @@ public final class DfsBlockCache { * @return total number of requests (hit + miss), per pack file extension. */ public long[] getTotalRequestCount() { - return dfsBlockCacheTable.getBlockCacheStats().getTotalRequestCount(); + return getAggregatedBlockCacheStats().getTotalRequestCount(); } /** @@ -172,7 +175,7 @@ public final class DfsBlockCache { * @return hit ratios */ public long[] getHitRatio() { - return dfsBlockCacheTable.getBlockCacheStats().getHitRatio(); + return getAggregatedBlockCacheStats().getHitRatio(); } /** @@ -183,7 +186,18 @@ public final class DfsBlockCache { * file extension. */ public long[] getEvictions() { - return dfsBlockCacheTable.getBlockCacheStats().getEvictions(); + return getAggregatedBlockCacheStats().getEvictions(); + } + + /** + * Get the list of {@link BlockCacheStats} for all underlying caches. + *

+ * Useful in monitoring caches with breakdown. + * + * @return the list of {@link BlockCacheStats} for all underlying caches. + */ + public List getAllBlockCacheStats() { + return dfsBlockCacheTable.getBlockCacheStats(); } /** @@ -263,6 +277,11 @@ public final class DfsBlockCache { return dfsBlockCacheTable.get(key, position); } + private BlockCacheStats getAggregatedBlockCacheStats() { + return AggregatedBlockCacheStats + .fromStatsList(dfsBlockCacheTable.getBlockCacheStats()); + } + static final class Ref { final DfsStreamKey key; diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/dfs/DfsBlockCacheTable.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/dfs/DfsBlockCacheTable.java index 43ba8c36ab..c3fd07b7bb 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/dfs/DfsBlockCacheTable.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/dfs/DfsBlockCacheTable.java @@ -11,6 +11,7 @@ package org.eclipse.jgit.internal.storage.dfs; import java.io.IOException; +import java.util.List; /** * Block cache table. @@ -125,13 +126,17 @@ public interface DfsBlockCacheTable { T get(DfsStreamKey key, long position); /** - * Get the {@link BlockCacheStats} object for this block cache table's - * statistics. + * Get the list of {@link BlockCacheStats} held by this cache. + *

+ * The returned list has a {@link BlockCacheStats} per configured cache + * table, with a minimum of 1 {@link BlockCacheStats} object returned. + * + * Use {@link AggregatedBlockCacheStats} to combine the results of the stats + * in the list for an aggregated view of the cache's stats. * - * @return the {@link BlockCacheStats} tracking this block cache table's - * statistics. + * @return the list of {@link BlockCacheStats} held by this cache. */ - BlockCacheStats getBlockCacheStats(); + List getBlockCacheStats(); /** * Get the name of the table. diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/dfs/PackExtBlockCacheTable.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/dfs/PackExtBlockCacheTable.java index e45643be84..28b021c68f 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/dfs/PackExtBlockCacheTable.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/dfs/PackExtBlockCacheTable.java @@ -188,11 +188,10 @@ class PackExtBlockCacheTable implements DfsBlockCacheTable { } @Override - public BlockCacheStats getBlockCacheStats() { - return AggregatedBlockCacheStats.fromStatsList(name, - blockCacheTableList.stream() - .map(DfsBlockCacheTable::getBlockCacheStats) - .collect(Collectors.toList())); + public List getBlockCacheStats() { + return blockCacheTableList.stream() + .flatMap(cacheTable -> cacheTable.getBlockCacheStats().stream()) + .collect(Collectors.toList()); } @Override -- cgit v1.2.3 From e5f42ae18e45e5c263ba510924e09cb2bb4bc822 Mon Sep 17 00:00:00 2001 From: Sam Delmerico Date: Mon, 7 Oct 2024 14:34:03 -0700 Subject: PackWriter: make PackWriter.writeIndex() take a PackIndexWriter Previously, the PackWriter implementation required that indexes and their extensions be writable to an OutputStream with a fixed binary format. To support more general index storage formats, allow PackWriter to accept an PackIndexWriter interface which accepts only the objects to store. This allows implementors to choose their storage format. The implementation will be provided by the DfsObjectDatabase. The DfsObjectDatabase is already responsible for providing the OutputStream that was previously used to write indexes. Having it provide a writing interface would be a natural generalization. This idea was previously implemented for PackBitmapIndex writing in https://gerrithub.io/c/eclipse-jgit/jgit/+/1177722. Change-Id: I582d2f3d25d6adb2da243d6d0d7bc405a97d6183 --- .../internal/storage/dfs/DfsGarbageCollector.java | 10 +------ .../jgit/internal/storage/dfs/DfsObjDatabase.java | 34 ++++++++++++++++++++++ .../internal/storage/dfs/DfsPackCompactor.java | 11 +------ .../jgit/internal/storage/pack/PackWriter.java | 26 +++++++++++++---- 4 files changed, 57 insertions(+), 24 deletions(-) diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/dfs/DfsGarbageCollector.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/dfs/DfsGarbageCollector.java index a177669788..b933e942d9 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/dfs/DfsGarbageCollector.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/dfs/DfsGarbageCollector.java @@ -18,7 +18,6 @@ import static org.eclipse.jgit.internal.storage.dfs.DfsObjDatabase.PackSource.RE import static org.eclipse.jgit.internal.storage.dfs.DfsObjDatabase.PackSource.UNREACHABLE_GARBAGE; import static org.eclipse.jgit.internal.storage.dfs.DfsPackCompactor.configureReftable; import static org.eclipse.jgit.internal.storage.pack.PackExt.COMMIT_GRAPH; -import static org.eclipse.jgit.internal.storage.pack.PackExt.INDEX; import static org.eclipse.jgit.internal.storage.pack.PackExt.OBJECT_SIZE_INDEX; import static org.eclipse.jgit.internal.storage.pack.PackExt.PACK; import static org.eclipse.jgit.internal.storage.pack.PackExt.REFTABLE; @@ -687,14 +686,7 @@ public class DfsGarbageCollector { pack.setBlockSize(PACK, out.blockSize()); } - try (DfsOutputStream out = objdb.writeFile(pack, INDEX)) { - CountingOutputStream cnt = new CountingOutputStream(out); - pw.writeIndex(cnt); - pack.addFileExt(INDEX); - pack.setFileSize(INDEX, cnt.getCount()); - pack.setBlockSize(INDEX, out.blockSize()); - pack.setIndexVersion(pw.getIndexVersion()); - } + pw.writeIndex(objdb.getPackIndexWriter(pack, pw.getIndexVersion())); if (source != UNREACHABLE_GARBAGE && packConfig.getMinBytesForObjSizeIndex() >= 0) { try (DfsOutputStream out = objdb.writeFile(pack, diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/dfs/DfsObjDatabase.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/dfs/DfsObjDatabase.java index 616563ffdd..efd666ff27 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/dfs/DfsObjDatabase.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/dfs/DfsObjDatabase.java @@ -12,6 +12,7 @@ package org.eclipse.jgit.internal.storage.dfs; import static java.util.stream.Collectors.joining; import static org.eclipse.jgit.internal.storage.pack.PackExt.BITMAP_INDEX; +import static org.eclipse.jgit.internal.storage.pack.PackExt.INDEX; import java.io.FileNotFoundException; import java.io.IOException; @@ -27,7 +28,9 @@ import java.util.Map; import java.util.Set; import java.util.concurrent.atomic.AtomicReference; +import org.eclipse.jgit.internal.storage.file.BasePackIndexWriter; import org.eclipse.jgit.internal.storage.file.PackBitmapIndexWriterV1; +import org.eclipse.jgit.internal.storage.pack.PackIndexWriter; import org.eclipse.jgit.internal.storage.pack.PackBitmapIndexWriter; import org.eclipse.jgit.internal.storage.pack.PackExt; import org.eclipse.jgit.lib.AnyObjectId; @@ -771,4 +774,35 @@ public abstract class DfsObjDatabase extends ObjectDatabase { } }; } + + /** + * Returns a writer to store the pack index in this object database. + * + * @param pack + * Pack file to which the index is associated. + * @param indexVersion + * which version of the index to write + * @return a writer to store the index associated with the pack + * @throws IOException + * when some I/O problem occurs while creating or writing to + * output stream + */ + public PackIndexWriter getPackIndexWriter( + DfsPackDescription pack, int indexVersion) + throws IOException { + return (objectsToStore, packDataChecksum) -> { + try (DfsOutputStream out = writeFile(pack, INDEX); + CountingOutputStream cnt = new CountingOutputStream(out)) { + final PackIndexWriter iw = BasePackIndexWriter + .createVersion(cnt, + indexVersion); + iw.write(objectsToStore, packDataChecksum); + pack.addFileExt(INDEX); + pack.setFileSize(INDEX, cnt.getCount()); + pack.setBlockSize(INDEX, out.blockSize()); + pack.setIndexVersion(indexVersion); + } + }; + } + } diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/dfs/DfsPackCompactor.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/dfs/DfsPackCompactor.java index 86144b389c..b32cddae77 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/dfs/DfsPackCompactor.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/dfs/DfsPackCompactor.java @@ -12,7 +12,6 @@ package org.eclipse.jgit.internal.storage.dfs; import static org.eclipse.jgit.internal.storage.dfs.DfsObjDatabase.PackSource.COMPACT; import static org.eclipse.jgit.internal.storage.dfs.DfsObjDatabase.PackSource.GC; -import static org.eclipse.jgit.internal.storage.pack.PackExt.INDEX; import static org.eclipse.jgit.internal.storage.pack.PackExt.PACK; import static org.eclipse.jgit.internal.storage.pack.PackExt.REFTABLE; import static org.eclipse.jgit.internal.storage.pack.StoredObjectRepresentation.PACK_DELTA; @@ -45,7 +44,6 @@ import org.eclipse.jgit.revwalk.RevWalk; import org.eclipse.jgit.storage.pack.PackConfig; import org.eclipse.jgit.storage.pack.PackStatistics; import org.eclipse.jgit.util.BlockList; -import org.eclipse.jgit.util.io.CountingOutputStream; /** * Combine several pack files into one pack. @@ -458,14 +456,7 @@ public class DfsPackCompactor { private static void writeIndex(DfsObjDatabase objdb, DfsPackDescription pack, PackWriter pw) throws IOException { - try (DfsOutputStream out = objdb.writeFile(pack, INDEX)) { - CountingOutputStream cnt = new CountingOutputStream(out); - pw.writeIndex(cnt); - pack.addFileExt(INDEX); - pack.setFileSize(INDEX, cnt.getCount()); - pack.setBlockSize(INDEX, out.blockSize()); - pack.setIndexVersion(pw.getIndexVersion()); - } + pw.writeIndex(objdb.getPackIndexWriter(pack, pw.getIndexVersion())); } static ReftableConfig configureReftable(ReftableConfig cfg, diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/pack/PackWriter.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/pack/PackWriter.java index 4fd2eb5798..4c262b0e3a 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/pack/PackWriter.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/pack/PackWriter.java @@ -118,7 +118,7 @@ import org.eclipse.jgit.util.TemporaryBuffer; * {@link #preparePack(ProgressMonitor, Set, Set)}, and streaming with * {@link #writePack(ProgressMonitor, ProgressMonitor, OutputStream)}. If the * pack is being stored as a file the matching index can be written out after - * writing the pack by {@link #writeIndex(OutputStream)}. An optional bitmap + * writing the pack by {@link #writeIndex(PackIndexWriter)}. An optional bitmap * index can be made by calling {@link #prepareBitmapIndex(ProgressMonitor)} * followed by {@link #writeBitmapIndex(PackBitmapIndexWriter)}. *

@@ -1099,12 +1099,28 @@ public class PackWriter implements AutoCloseable { * the index data could not be written to the supplied stream. */ public void writeIndex(OutputStream indexStream) throws IOException { + writeIndex(BasePackIndexWriter.createVersion(indexStream, + getIndexVersion())); + } + + /** + * Create an index file to match the pack file just written. + *

+ * Called after + * {@link #writePack(ProgressMonitor, ProgressMonitor, OutputStream)}. + *

+ * Writing an index is only required for local pack storage. Packs sent on + * the network do not need to create an index. + * + * @param iw + * an @code{PackIndexWriter} instance to write the index + * @throws java.io.IOException + * the index data could not be written to the supplied stream. + */ + public void writeIndex(PackIndexWriter iw) throws IOException { if (isIndexDisabled()) throw new IOException(JGitText.get().cachedPacksPreventsIndexCreation); - long writeStart = System.currentTimeMillis(); - PackIndexWriter iw = BasePackIndexWriter.createVersion(indexStream, - getIndexVersion()); iw.write(sortByName(), packcsum); stats.timeWriting += System.currentTimeMillis() - writeStart; } @@ -2448,7 +2464,7 @@ public class PackWriter implements AutoCloseable { * object graph at selected commits. Writing a bitmap index is an optional * feature that not all pack users may require. *

- * Called after {@link #writeIndex(OutputStream)}. + * Called after {@link #writeIndex(PackIndexWriter)}. *

* To reduce memory internal state is cleared during this method, rendering * the PackWriter instance useless for anything further than a call to write -- cgit v1.2.3 From 9c6b36e97ba561d4737130c6427d6febc0a7d431 Mon Sep 17 00:00:00 2001 From: Ivan Frade Date: Tue, 29 Oct 2024 10:57:28 -0700 Subject: DfsInserter: Read minBytesForObjectSizeIndex only from repo config In general, JGit reads the configuration it needs from the repository configuration. minBytesForObjectSizeIndex is a special case with a setter for subclasses but that is unnecessary. Remove the setter and read the conf from the repo. Make the property final and read it directly from the conf (it is clearer than parsing a whole PackConfig to read a single value). Change-Id: I8d77247452ff65e6c431fdcfebb90ed2ce40aed1 --- .../jgit/internal/storage/dfs/DfsInserter.java | 28 ++++++---------------- 1 file changed, 7 insertions(+), 21 deletions(-) diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/dfs/DfsInserter.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/dfs/DfsInserter.java index 8f09261674..16315bf4f2 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/dfs/DfsInserter.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/dfs/DfsInserter.java @@ -41,12 +41,13 @@ import org.eclipse.jgit.errors.CorruptObjectException; import org.eclipse.jgit.errors.IncorrectObjectTypeException; import org.eclipse.jgit.errors.LargeObjectException; import org.eclipse.jgit.internal.JGitText; -import org.eclipse.jgit.internal.storage.file.PackIndex; import org.eclipse.jgit.internal.storage.file.BasePackIndexWriter; +import org.eclipse.jgit.internal.storage.file.PackIndex; import org.eclipse.jgit.internal.storage.file.PackObjectSizeIndexWriter; import org.eclipse.jgit.internal.storage.pack.PackExt; import org.eclipse.jgit.lib.AbbreviatedObjectId; import org.eclipse.jgit.lib.AnyObjectId; +import org.eclipse.jgit.lib.ConfigConstants; import org.eclipse.jgit.lib.Constants; import org.eclipse.jgit.lib.ObjectId; import org.eclipse.jgit.lib.ObjectIdOwnerMap; @@ -54,7 +55,6 @@ import org.eclipse.jgit.lib.ObjectInserter; import org.eclipse.jgit.lib.ObjectLoader; import org.eclipse.jgit.lib.ObjectReader; import org.eclipse.jgit.lib.ObjectStream; -import org.eclipse.jgit.storage.pack.PackConfig; import org.eclipse.jgit.transport.PackedObjectInfo; import org.eclipse.jgit.util.BlockList; import org.eclipse.jgit.util.IO; @@ -71,6 +71,8 @@ public class DfsInserter extends ObjectInserter { private static final int INDEX_VERSION = 2; final DfsObjDatabase db; + + private final int minBytesForObjectSizeIndex; int compression = Deflater.BEST_COMPRESSION; List objectList; @@ -83,8 +85,6 @@ public class DfsInserter extends ObjectInserter { private boolean rollback; private boolean checkExisting = true; - private int minBytesForObjectSizeIndex = -1; - /** * Initialize a new inserter. * @@ -93,8 +93,9 @@ public class DfsInserter extends ObjectInserter { */ protected DfsInserter(DfsObjDatabase db) { this.db = db; - PackConfig pc = new PackConfig(db.getRepository()); - this.minBytesForObjectSizeIndex = pc.getMinBytesForObjSizeIndex(); + this.minBytesForObjectSizeIndex = db.getRepository().getConfig().getInt( + ConfigConstants.CONFIG_PACK_SECTION, + ConfigConstants.CONFIG_KEY_MIN_BYTES_OBJ_SIZE_INDEX, -1); } /** @@ -112,21 +113,6 @@ public class DfsInserter extends ObjectInserter { void setCompressionLevel(int compression) { this.compression = compression; } - - /** - * Set minimum size for an object to be included in the object size index. - * - *

- * Use 0 for all and -1 for nothing (the pack won't have object size index). - * - * @param minBytes - * only objects with size bigger or equal to this are included in - * the index. - */ - protected void setMinBytesForObjectSizeIndex(int minBytes) { - this.minBytesForObjectSizeIndex = minBytes; - } - @Override public DfsPackParser newPackParser(InputStream in) throws IOException { return new DfsPackParser(db, this, in); -- cgit v1.2.3 From d861ad1f5a1fdaf49b9c4b641e7d298b7acea6ad Mon Sep 17 00:00:00 2001 From: Ivan Frade Date: Tue, 9 Jul 2024 11:10:02 -0700 Subject: [errorprone] util.Stats: Add summary fragment to javadoc Errorprone complains about missing summary in these javadocs. [MissingSummary] A summary fragment is required; consider using the value of the @return b lock as a summary fragment instead. * @return variance of the added values ^ (see https://google.github.io/styleguide/javaguide.html#s7.2-summary-fragment) Did you mean '*Returns variance of the added values.'? Change-Id: I29d633ec95c18b17cc92b069dd1a94fbb2a75c94 --- .../src/org/eclipse/jgit/util/Stats.java | 25 ++++++++++++++++------ 1 file changed, 18 insertions(+), 7 deletions(-) diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/util/Stats.java b/org.eclipse.jgit/src/org/eclipse/jgit/util/Stats.java index d957deb34c..efa6e7ddc3 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/util/Stats.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/util/Stats.java @@ -43,14 +43,18 @@ public class Stats { } /** - * @return number of the added values + * Returns the number of added values + * + * @return the number of added values */ public int count() { return n; } /** - * @return minimum of the added values + * Returns the smallest value added + * + * @return the smallest value added */ public double min() { if (n < 1) { @@ -60,7 +64,9 @@ public class Stats { } /** - * @return maximum of the added values + * Returns the biggest value added + * + * @return the biggest value added */ public double max() { if (n < 1) { @@ -70,9 +76,10 @@ public class Stats { } /** - * @return average of the added values + * Returns the average of the added values + * + * @return the average of the added values */ - public double avg() { if (n < 1) { return Double.NaN; @@ -81,7 +88,9 @@ public class Stats { } /** - * @return variance of the added values + * Returns the variance of the added values + * + * @return the variance of the added values */ public double var() { if (n < 2) { @@ -91,7 +100,9 @@ public class Stats { } /** - * @return standard deviation of the added values + * Returns the standard deviation of the added values + * + * @return the standard deviation of the added values */ public double stddev() { return Math.sqrt(this.var()); -- cgit v1.2.3 From 793df21ed5c75613b0eb7f300857d56779d5c502 Mon Sep 17 00:00:00 2001 From: Ivan Frade Date: Tue, 9 Jul 2024 13:29:15 -0700 Subject: [errorprone] ByteArraySet: Add summary fragment to javadoc Reported by error prone. Change-Id: Icaa69c37d0cde19fc605cb3f3c5f9ed9abfb37d3 --- .../src/org/eclipse/jgit/treewalk/filter/ByteArraySet.java | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/treewalk/filter/ByteArraySet.java b/org.eclipse.jgit/src/org/eclipse/jgit/treewalk/filter/ByteArraySet.java index bcf79a285d..33db6ea661 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/treewalk/filter/ByteArraySet.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/treewalk/filter/ByteArraySet.java @@ -13,12 +13,12 @@ package org.eclipse.jgit.treewalk.filter; -import org.eclipse.jgit.util.RawParseUtils; - import java.util.Arrays; import java.util.Set; import java.util.stream.Collectors; +import org.eclipse.jgit.util.RawParseUtils; + /** * Specialized set for byte arrays, interpreted as strings for use in * {@link PathFilterGroup.Group}. Most methods assume the hash is already know @@ -141,13 +141,19 @@ class ByteArraySet { } /** + * Returns number of arrays in the set + * * @return number of arrays in the set */ int size() { return size; } - /** @return true if {@link #size()} is 0. */ + /** + * Returns true if {@link #size()} is 0 + * + * @return true if {@link #size()} is 0 + */ boolean isEmpty() { return size == 0; } -- cgit v1.2.3 From c9752b0125c84248c468065eba64bf007abae81c Mon Sep 17 00:00:00 2001 From: Ivan Frade Date: Wed, 30 Oct 2024 15:36:09 -0700 Subject: [errorprone] PackWriter: Fix javadoc tag in new #writeIndex method We introduced this method recently and the javadoc is not correct: error: [InvalidInlineTag] This tag is invalid. @code{PackIndexWriter} instance to write the index Change-Id: I34ed3d8b5a121fea9b8163627b46ae4a289c9462 --- .../src/org/eclipse/jgit/internal/storage/pack/PackWriter.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/pack/PackWriter.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/pack/PackWriter.java index 4c262b0e3a..f025d4e3d5 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/pack/PackWriter.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/pack/PackWriter.java @@ -59,9 +59,9 @@ import org.eclipse.jgit.errors.SearchForReuseTimeout; import org.eclipse.jgit.errors.StoredObjectRepresentationNotAvailableException; import org.eclipse.jgit.internal.JGitText; import org.eclipse.jgit.internal.storage.file.BasePackIndexWriter; +import org.eclipse.jgit.internal.storage.file.PackBitmapIndexBuilder; import org.eclipse.jgit.internal.storage.file.PackObjectSizeIndexWriter; import org.eclipse.jgit.internal.storage.file.PackReverseIndexWriter; -import org.eclipse.jgit.internal.storage.file.PackBitmapIndexBuilder; import org.eclipse.jgit.lib.AnyObjectId; import org.eclipse.jgit.lib.AsyncObjectSizeQueue; import org.eclipse.jgit.lib.BatchingProgressMonitor; @@ -1113,7 +1113,7 @@ public class PackWriter implements AutoCloseable { * the network do not need to create an index. * * @param iw - * an @code{PackIndexWriter} instance to write the index + * an {@link PackIndexWriter} instance to write the index * @throws java.io.IOException * the index data could not be written to the supplied stream. */ -- cgit v1.2.3 From 92a9641026306af3136530bc351da1c755df0c27 Mon Sep 17 00:00:00 2001 From: Ivan Frade Date: Wed, 30 Oct 2024 15:45:33 -0700 Subject: [errorprone] HttpConnection: Add missing summary in java The constants don't have summary and errorprone complains about it. Change-Id: Id1470ed9fd54cf7fd684045c5631acc1a8d450c2 --- .../org/eclipse/jgit/transport/http/HttpConnection.java | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/transport/http/HttpConnection.java b/org.eclipse.jgit/src/org/eclipse/jgit/transport/http/HttpConnection.java index 125ee6cbe9..95b8221a8b 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/transport/http/HttpConnection.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/transport/http/HttpConnection.java @@ -36,29 +36,39 @@ import org.eclipse.jgit.annotations.NonNull; */ public interface HttpConnection { /** + * HttpURLConnection#HTTP_OK + * * @see HttpURLConnection#HTTP_OK */ int HTTP_OK = java.net.HttpURLConnection.HTTP_OK; /** + * HttpURLConnection#HTTP_NOT_AUTHORITATIVE + * * @see HttpURLConnection#HTTP_NOT_AUTHORITATIVE * @since 5.8 */ int HTTP_NOT_AUTHORITATIVE = java.net.HttpURLConnection.HTTP_NOT_AUTHORITATIVE; /** + * HttpURLConnection#HTTP_MOVED_PERM + * * @see HttpURLConnection#HTTP_MOVED_PERM * @since 4.7 */ int HTTP_MOVED_PERM = java.net.HttpURLConnection.HTTP_MOVED_PERM; /** + * HttpURLConnection#HTTP_MOVED_TEMP + * * @see HttpURLConnection#HTTP_MOVED_TEMP * @since 4.9 */ int HTTP_MOVED_TEMP = java.net.HttpURLConnection.HTTP_MOVED_TEMP; /** + * HttpURLConnection#HTTP_SEE_OTHER + * * @see HttpURLConnection#HTTP_SEE_OTHER * @since 4.9 */ @@ -85,16 +95,22 @@ public interface HttpConnection { int HTTP_11_MOVED_PERM = 308; /** + * HttpURLConnection#HTTP_NOT_FOUND + * * @see HttpURLConnection#HTTP_NOT_FOUND */ int HTTP_NOT_FOUND = java.net.HttpURLConnection.HTTP_NOT_FOUND; /** + * HttpURLConnection#HTTP_UNAUTHORIZED + * * @see HttpURLConnection#HTTP_UNAUTHORIZED */ int HTTP_UNAUTHORIZED = java.net.HttpURLConnection.HTTP_UNAUTHORIZED; /** + * HttpURLConnection#HTTP_FORBIDDEN + * * @see HttpURLConnection#HTTP_FORBIDDEN */ int HTTP_FORBIDDEN = java.net.HttpURLConnection.HTTP_FORBIDDEN; -- cgit v1.2.3 From c8b1b822a409167b63bd9be11efac0bcd5476a5c Mon Sep 17 00:00:00 2001 From: Ivan Frade Date: Mon, 8 Jul 2024 14:21:45 -0700 Subject: [errorprone] SeparateClassloadertTestRunner: use #split(String,int) Errorprone recommends to use String.split(String, int) as it has a less surprising behaviour with empty entries. https://errorprone.info/bugpattern/StringSplitter Change-Id: I48a01ee18d66bbb4a177aee576629dc5132d4a38 --- .../src/org/eclipse/jgit/junit/SeparateClassloaderTestRunner.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/org.eclipse.jgit.junit/src/org/eclipse/jgit/junit/SeparateClassloaderTestRunner.java b/org.eclipse.jgit.junit/src/org/eclipse/jgit/junit/SeparateClassloaderTestRunner.java index c8c56b21b8..2a482df04a 100644 --- a/org.eclipse.jgit.junit/src/org/eclipse/jgit/junit/SeparateClassloaderTestRunner.java +++ b/org.eclipse.jgit.junit/src/org/eclipse/jgit/junit/SeparateClassloaderTestRunner.java @@ -44,7 +44,7 @@ public class SeparateClassloaderTestRunner extends BlockJUnit4ClassRunner { try { String pathSeparator = System.getProperty("path.separator"); String[] classPathEntries = System.getProperty("java.class.path") - .split(pathSeparator); + .split(pathSeparator, -1); URL[] urls = new URL[classPathEntries.length]; for (int i = 0; i < classPathEntries.length; i++) { urls[i] = Paths.get(classPathEntries[i]).toUri().toURL(); -- cgit v1.2.3 From 7c092ab66e27b3ae68c7fed6d300b8645f7e4173 Mon Sep 17 00:00:00 2001 From: Saril Sudhakaran Date: Tue, 29 Oct 2024 00:17:01 -0500 Subject: DfsGarbageCollector: Add setter for reflog expiration time. JGit reftable writer/compator knows how to prune the history, but the DfsGarbageCollector doesn't expose the time limit. Add a method to DfsGarbageCollector to set the reflog time limit. This value is then passed to the reftable compactor. Callers usually pass here the value from gc.reflogExpire. The reflog block length is stored in 24 bits [1], limiting the size to 16MB. I have observed that in repositories with frequent commits, reflogs hit that size in 6-12 months. [1] https://git-scm.com/docs/reftable Bug: jgit-96 Change-Id: I8b32d6d2b2e1d8af8fb7d9f86225d75f1877eb2f --- .../storage/dfs/DfsGarbageCollectorTest.java | 98 ++++++++++++++++++++++ .../internal/storage/dfs/DfsGarbageCollector.java | 21 +++++ 2 files changed, 119 insertions(+) diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/dfs/DfsGarbageCollectorTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/dfs/DfsGarbageCollectorTest.java index 2be11d32ea..3edd1056be 100644 --- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/dfs/DfsGarbageCollectorTest.java +++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/dfs/DfsGarbageCollectorTest.java @@ -6,6 +6,7 @@ import static org.eclipse.jgit.internal.storage.dfs.DfsObjDatabase.PackSource.IN import static org.eclipse.jgit.internal.storage.dfs.DfsObjDatabase.PackSource.UNREACHABLE_GARBAGE; import static org.eclipse.jgit.internal.storage.pack.PackExt.PACK; import static org.eclipse.jgit.internal.storage.pack.PackExt.REFTABLE; +import static org.eclipse.jgit.lib.Constants.OBJECT_ID_LENGTH; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotNull; @@ -17,6 +18,8 @@ import static org.junit.Assert.fail; import java.io.IOException; import java.util.Arrays; import java.util.Collections; +import java.util.Date; +import java.util.GregorianCalendar; import java.util.concurrent.TimeUnit; import org.eclipse.jgit.internal.storage.commitgraph.CommitGraph; @@ -24,6 +27,7 @@ import org.eclipse.jgit.internal.storage.commitgraph.CommitGraphWriter; import org.eclipse.jgit.internal.storage.dfs.DfsObjDatabase.PackSource; import org.eclipse.jgit.internal.storage.file.PackBitmapIndex; import org.eclipse.jgit.internal.storage.pack.PackExt; +import org.eclipse.jgit.internal.storage.reftable.LogCursor; import org.eclipse.jgit.internal.storage.reftable.RefCursor; import org.eclipse.jgit.internal.storage.reftable.ReftableConfig; import org.eclipse.jgit.internal.storage.reftable.ReftableReader; @@ -37,6 +41,7 @@ import org.eclipse.jgit.lib.ConfigConstants; import org.eclipse.jgit.lib.NullProgressMonitor; import org.eclipse.jgit.lib.ObjectId; import org.eclipse.jgit.lib.ObjectIdRef; +import org.eclipse.jgit.lib.PersonIdent; import org.eclipse.jgit.lib.Ref; import org.eclipse.jgit.lib.Repository; import org.eclipse.jgit.revwalk.RevBlob; @@ -44,6 +49,7 @@ import org.eclipse.jgit.revwalk.RevCommit; import org.eclipse.jgit.revwalk.RevWalk; import org.eclipse.jgit.storage.pack.PackConfig; import org.eclipse.jgit.transport.ReceiveCommand; +import org.eclipse.jgit.util.GitDateParser; import org.eclipse.jgit.util.SystemReader; import org.junit.After; import org.junit.Before; @@ -1275,6 +1281,90 @@ public class DfsGarbageCollectorTest { bitmapIndex.getXorBitmapCount() > 0); } + @Test + public void gitGCWithRefLogExpire() throws Exception { + String master = "refs/heads/master"; + RevCommit commit0 = commit().message("0").create(); + RevCommit commit1 = commit().message("1").parent(commit0).create(); + git.update(master, commit1); + DfsGarbageCollector gc = new DfsGarbageCollector(repo); + gc.setReftableConfig(new ReftableConfig()); + run(gc); + DfsPackDescription t1 = odb.newPack(INSERT); + Ref next = new ObjectIdRef.PeeledNonTag(Ref.Storage.LOOSE, + "refs/heads/next", commit0.copy()); + long currentDay = new Date().getTime(); + GregorianCalendar cal = new GregorianCalendar(SystemReader + .getInstance().getTimeZone(), SystemReader.getInstance() + .getLocale()); + long ten_days_ago = GitDateParser.parse("10 days ago",cal,SystemReader.getInstance() + .getLocale()).getTime() ; + long twenty_days_ago = GitDateParser.parse("20 days ago",cal,SystemReader.getInstance() + .getLocale()).getTime() ; + long thirty_days_ago = GitDateParser.parse("30 days ago",cal,SystemReader.getInstance() + .getLocale()).getTime() ;; + long fifty_days_ago = GitDateParser.parse("50 days ago",cal,SystemReader.getInstance() + .getLocale()).getTime() ; + PersonIdent who2 = new PersonIdent("J.Author", "authemail", currentDay, -8 * 60); + PersonIdent who3 = new PersonIdent("J.Author", "authemail", ten_days_ago, -8 * 60); + PersonIdent who4 = new PersonIdent("J.Author", "authemail", twenty_days_ago, -8 * 60); + PersonIdent who5 = new PersonIdent("J.Author", "authemail", thirty_days_ago, -8 * 60); + PersonIdent who6 = new PersonIdent("J.Author", "authemail", fifty_days_ago, -8 * 60); + + try (DfsOutputStream out = odb.writeFile(t1, REFTABLE)) { + ReftableWriter w = new ReftableWriter(out); + w.setMinUpdateIndex(42); + w.setMaxUpdateIndex(42); + w.begin(); + w.sortAndWriteRefs(Collections.singleton(next)); + w.writeLog("refs/heads/branch", 1, who2, ObjectId.zeroId(),id(2), "Branch Message"); + w.writeLog("refs/heads/branch1", 2, who3, ObjectId.zeroId(),id(3), "Branch Message1"); + w.writeLog("refs/heads/branch2", 2, who4, ObjectId.zeroId(),id(4), "Branch Message2"); + w.writeLog("refs/heads/branch3", 2, who5, ObjectId.zeroId(),id(5), "Branch Message3"); + w.writeLog("refs/heads/branch4", 2, who6, ObjectId.zeroId(),id(6), "Branch Message4"); + w.finish(); + t1.addFileExt(REFTABLE); + t1.setReftableStats(w.getStats()); + } + odb.commitPack(Collections.singleton(t1), null); + + gc = new DfsGarbageCollector(repo); + gc.setReftableConfig(new ReftableConfig()); + // Expire ref log entries older than 30 days + gc.setRefLogExpire(new Date(thirty_days_ago)); + run(gc); + + // Single GC pack present with all objects. + assertEquals(1, odb.getPacks().length); + DfsPackFile pack = odb.getPacks()[0]; + DfsPackDescription desc = pack.getPackDescription(); + + DfsReftable table = new DfsReftable(DfsBlockCache.getInstance(), desc); + try (DfsReader ctx = odb.newReader(); + ReftableReader rr = table.open(ctx); + RefCursor rc = rr.allRefs(); + LogCursor lc = rr.allLogs()) { + assertTrue(rc.next()); + assertEquals(master, rc.getRef().getName()); + assertEquals(commit1, rc.getRef().getObjectId()); + assertTrue(rc.next()); + assertEquals(next.getName(), rc.getRef().getName()); + assertEquals(commit0, rc.getRef().getObjectId()); + assertFalse(rc.next()); + assertTrue(lc.next()); + assertEquals(lc.getRefName(),"refs/heads/branch"); + assertTrue(lc.next()); + assertEquals(lc.getRefName(),"refs/heads/branch1"); + assertTrue(lc.next()); + assertEquals(lc.getRefName(),"refs/heads/branch2"); + // Old entries are purged + assertFalse(lc.next()); + + } + + } + + private RevCommit commitChain(RevCommit parent, int length) throws Exception { for (int i = 0; i < length; i++) { @@ -1364,4 +1454,12 @@ public class DfsGarbageCollectorTest { } return cnt; } + private static ObjectId id(int i) { + byte[] buf = new byte[OBJECT_ID_LENGTH]; + buf[0] = (byte) (i & 0xff); + buf[1] = (byte) ((i >>> 8) & 0xff); + buf[2] = (byte) ((i >>> 16) & 0xff); + buf[3] = (byte) (i >>> 24); + return ObjectId.fromRaw(buf); + } } diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/dfs/DfsGarbageCollector.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/dfs/DfsGarbageCollector.java index a177669788..91440a6498 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/dfs/DfsGarbageCollector.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/dfs/DfsGarbageCollector.java @@ -29,6 +29,7 @@ import java.util.ArrayList; import java.util.Arrays; import java.util.Calendar; import java.util.Collection; +import java.util.Date; import java.util.EnumSet; import java.util.GregorianCalendar; import java.util.HashSet; @@ -100,6 +101,7 @@ public class DfsGarbageCollector { private Set allTags; private Set nonHeads; private Set tagTargets; + private Date refLogExpire; /** * Initialize a garbage collector. @@ -200,6 +202,22 @@ public class DfsGarbageCollector { return this; } + + /** + * Set time limit to the reflog history. + *

+ * Garbage Collector prunes entries from reflog history older than {@code refLogExpire} + *

+ * + * @param refLogExpire + * instant in time which defines refLog expiration + * @return {@code this} + */ + public DfsGarbageCollector setRefLogExpire(Date refLogExpire) { + this.refLogExpire = refLogExpire ; + return this; + } + /** * Set maxUpdateIndex for the initial reftable created during conversion. * @@ -741,6 +759,9 @@ public class DfsGarbageCollector { compact.addAll(stack.readers()); compact.setIncludeDeletes(includeDeletes); compact.setConfig(configureReftable(reftableConfig, out)); + if(refLogExpire != null ){ + compact.setReflogExpireOldestReflogTimeMillis(refLogExpire.getTime()); + } compact.compact(); pack.addFileExt(REFTABLE); pack.setReftableStats(compact.getStats()); -- cgit v1.2.3 From 4ec2413e14043f6d9a0ebd6616fdef2464fe4880 Mon Sep 17 00:00:00 2001 From: Ivan Frade Date: Thu, 31 Oct 2024 12:55:31 -0700 Subject: [errorprone] RefDatabase: #getConflictingNames immutable return Errorprone reports that: This method returns both mutable and immutable collections or maps from different paths. This may be confusing for users of the method. Return always an immutable collection. Change-Id: Id48f3645fd06c8bc72212af180d7d02c7e0b7632 --- org.eclipse.jgit/src/org/eclipse/jgit/lib/RefDatabase.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/lib/RefDatabase.java b/org.eclipse.jgit/src/org/eclipse/jgit/lib/RefDatabase.java index 2cf24185c7..114246beb2 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/lib/RefDatabase.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/lib/RefDatabase.java @@ -160,7 +160,7 @@ public abstract class RefDatabase { if (existing.startsWith(prefix)) conflicting.add(existing); - return conflicting; + return Collections.unmodifiableList(conflicting); } /** -- cgit v1.2.3 From 562d825f26134ee3db30c1791e918fa8c32095f1 Mon Sep 17 00:00:00 2001 From: Ivan Frade Date: Thu, 31 Oct 2024 12:17:09 -0700 Subject: [errorprone] Remove deprecated security manager Errorprone warns about this deprecated classes. The recommendation is stop using SecurityManager all together. The Security Manager is deprecated and subject to removal in a future release. There is no replacement for the Security Manager. See JEP 411 [1] for discussion and alternatives. [1] https://openjdk.org/jeps/411 Change-Id: I3c67136e97d13cf24b85e41d94408631c26e8be8 --- .../api/SecurityManagerMissingPermissionsTest.java | 126 --------------- .../org/eclipse/jgit/api/SecurityManagerTest.java | 173 --------------------- .../org/eclipse/jgit/internal/JGitText.properties | 2 - .../src/org/eclipse/jgit/internal/JGitText.java | 2 - .../eclipse/jgit/transport/SshSessionFactory.java | 7 +- org.eclipse.jgit/src/org/eclipse/jgit/util/FS.java | 35 +---- .../src/org/eclipse/jgit/util/FS_Win32_Cygwin.java | 11 +- .../src/org/eclipse/jgit/util/SystemReader.java | 6 +- .../eclipse/jgit/util/io/ThrowingPrintWriter.java | 7 +- 9 files changed, 7 insertions(+), 362 deletions(-) delete mode 100644 org.eclipse.jgit.test/tst/org/eclipse/jgit/api/SecurityManagerMissingPermissionsTest.java delete mode 100644 org.eclipse.jgit.test/tst/org/eclipse/jgit/api/SecurityManagerTest.java diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/api/SecurityManagerMissingPermissionsTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/api/SecurityManagerMissingPermissionsTest.java deleted file mode 100644 index d0fbdbd090..0000000000 --- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/api/SecurityManagerMissingPermissionsTest.java +++ /dev/null @@ -1,126 +0,0 @@ -/* - * Copyright (c) 2019 Alex Jitianu and others - * - * This program and the accompanying materials are made available under the - * terms of the Eclipse Distribution License v. 1.0 which is available at - * https://www.eclipse.org/org/documents/edl-v10.php. - * - * SPDX-License-Identifier: BSD-3-Clause - */ -package org.eclipse.jgit.api; - -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertFalse; -import static org.junit.Assert.assertTrue; - -import java.io.ByteArrayOutputStream; -import java.io.File; -import java.io.IOException; -import java.io.PrintStream; -import java.nio.file.Files; -import java.nio.file.Path; -import java.security.Policy; -import java.util.Collections; - -import org.eclipse.jgit.junit.RepositoryTestCase; -import org.eclipse.jgit.util.FileUtils; -import org.junit.After; -import org.junit.Before; -import org.junit.Test; - -/** - * Tests that using a SecurityManager does not result in errors logged. - */ -public class SecurityManagerMissingPermissionsTest extends RepositoryTestCase { - - /** - * Collects all logging sent to the logging system. - */ - private final ByteArrayOutputStream errorOutput = new ByteArrayOutputStream(); - - private SecurityManager originalSecurityManager; - - private PrintStream defaultErrorOutput; - - @Override - @Before - public void setUp() throws Exception { - originalSecurityManager = System.getSecurityManager(); - - // slf4j-simple logs to System.err, redirect it to enable asserting - // logged errors - defaultErrorOutput = System.err; - System.setErr(new PrintStream(errorOutput)); - - refreshPolicyAllPermission(Policy.getPolicy()); - System.setSecurityManager(new SecurityManager()); - super.setUp(); - } - - /** - * If a SecurityManager is active a lot of {@link java.io.FilePermission} - * errors are thrown and logged while initializing a repository. - * - * @throws Exception - */ - @Test - public void testCreateNewRepos_MissingPermissions() throws Exception { - File wcTree = new File(getTemporaryDirectory(), - "CreateNewRepositoryTest_testCreateNewRepos"); - - File marker = new File(getTemporaryDirectory(), "marker"); - Files.write(marker.toPath(), Collections.singletonList("Can write")); - assertTrue("Can write in test directory", marker.isFile()); - FileUtils.delete(marker); - assertFalse("Can delete in test direcory", marker.exists()); - - Git git = Git.init().setBare(false) - .setDirectory(new File(wcTree.getAbsolutePath())).call(); - - addRepoToClose(git.getRepository()); - - assertEquals("", errorOutput.toString()); - } - - @Override - @After - public void tearDown() throws Exception { - System.setSecurityManager(originalSecurityManager); - System.setErr(defaultErrorOutput); - super.tearDown(); - } - - /** - * Refresh the Java Security Policy. - * - * @param policy - * the policy object - * - * @throws IOException - * if the temporary file that contains the policy could not be - * created - */ - private static void refreshPolicyAllPermission(Policy policy) - throws IOException { - // Starting with an all permissions policy. - String policyString = "grant { permission java.security.AllPermission; };"; - - // Do not use TemporaryFilesFactory, it will create a dependency cycle - Path policyFile = Files.createTempFile("testpolicy", ".txt"); - - try { - Files.write(policyFile, Collections.singletonList(policyString)); - System.setProperty("java.security.policy", - policyFile.toUri().toURL().toString()); - policy.refresh(); - } finally { - try { - Files.delete(policyFile); - } catch (IOException e) { - // Do not log; the test tests for no logging having occurred - e.printStackTrace(); - } - } - } - -} diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/api/SecurityManagerTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/api/SecurityManagerTest.java deleted file mode 100644 index 2b930a1133..0000000000 --- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/api/SecurityManagerTest.java +++ /dev/null @@ -1,173 +0,0 @@ -/* - * Copyright (C) 2019 Nail Samatov and others - * - * This program and the accompanying materials are made available under the - * terms of the Eclipse Distribution License v. 1.0 which is available at - * https://www.eclipse.org/org/documents/edl-v10.php. - * - * SPDX-License-Identifier: BSD-3-Clause - */ -package org.eclipse.jgit.api; - -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertNotNull; -import static org.junit.Assert.assertTrue; - -import java.io.File; -import java.io.FilePermission; -import java.io.IOException; -import java.lang.reflect.ReflectPermission; -import java.nio.file.Files; -import java.security.Permission; -import java.security.SecurityPermission; -import java.util.ArrayList; -import java.util.List; -import java.util.PropertyPermission; -import java.util.logging.LoggingPermission; - -import javax.security.auth.AuthPermission; - -import org.eclipse.jgit.api.errors.GitAPIException; -import org.eclipse.jgit.junit.JGitTestUtil; -import org.eclipse.jgit.junit.MockSystemReader; -import org.eclipse.jgit.junit.SeparateClassloaderTestRunner; -import org.eclipse.jgit.revwalk.RevCommit; -import org.eclipse.jgit.treewalk.TreeWalk; -import org.eclipse.jgit.util.FileUtils; -import org.eclipse.jgit.util.SystemReader; -import org.junit.After; -import org.junit.Before; -import org.junit.Test; -import org.junit.runner.RunWith; - -/** - *

- * Tests if jgit works if SecurityManager is enabled. - *

- * - *

- * Note: JGit's classes shouldn't be used before SecurityManager is configured. - * If you use some JGit's class before SecurityManager is replaced then part of - * the code can be invoked outside of our custom SecurityManager and this test - * becomes useless. - *

- * - *

- * For example the class {@link org.eclipse.jgit.util.FS} is used widely in jgit - * sources. It contains DETECTED static field. At the first usage of the class - * FS the field DETECTED is initialized and during initialization many system - * operations that SecurityManager can forbid are invoked. - *

- * - *

- * For this reason this test doesn't extend LocalDiskRepositoryTestCase (it uses - * JGit's classes in setUp() method) and other JGit's utility classes. It's done - * to affect SecurityManager as less as possible. - *

- * - *

- * We use SeparateClassloaderTestRunner to isolate FS.DETECTED field - * initialization between different tests run. - *

- */ -@RunWith(SeparateClassloaderTestRunner.class) -public class SecurityManagerTest { - private File root; - - private SecurityManager originalSecurityManager; - - private List permissions = new ArrayList<>(); - - @Before - public void setUp() throws Exception { - // Create working directory - SystemReader.setInstance(new MockSystemReader()); - root = Files.createTempDirectory("jgit-security").toFile(); - - // Add system permissions - permissions.add(new RuntimePermission("*")); - permissions.add(new SecurityPermission("*")); - permissions.add(new AuthPermission("*")); - permissions.add(new ReflectPermission("*")); - permissions.add(new PropertyPermission("*", "read,write")); - permissions.add(new LoggingPermission("control", null)); - - permissions.add(new FilePermission( - System.getProperty("java.home") + "/-", "read")); - - String tempDir = System.getProperty("java.io.tmpdir"); - permissions.add(new FilePermission(tempDir, "read,write,delete")); - permissions - .add(new FilePermission(tempDir + "/-", "read,write,delete")); - - // Add permissions to dependent jar files. - String classPath = System.getProperty("java.class.path"); - if (classPath != null) { - for (String path : classPath.split(File.pathSeparator)) { - permissions.add(new FilePermission(path, "read")); - } - } - // Add permissions to jgit class files. - String jgitSourcesRoot = new File(System.getProperty("user.dir")) - .getParent(); - permissions.add(new FilePermission(jgitSourcesRoot + "/-", "read")); - - // Add permissions to working dir for jgit. Our git repositories will be - // initialized and cloned here. - permissions.add(new FilePermission(root.getPath() + "/-", - "read,write,delete,execute")); - - // Replace Security Manager - originalSecurityManager = System.getSecurityManager(); - System.setSecurityManager(new SecurityManager() { - - @Override - public void checkPermission(Permission requested) { - for (Permission permission : permissions) { - if (permission.implies(requested)) { - return; - } - } - - super.checkPermission(requested); - } - }); - } - - @After - public void tearDown() throws Exception { - System.setSecurityManager(originalSecurityManager); - - // Note: don't use this method before security manager is replaced in - // setUp() method. The method uses FS.DETECTED internally and can affect - // the test. - FileUtils.delete(root, FileUtils.RECURSIVE | FileUtils.RETRY); - } - - @Test - public void testInitAndClone() throws IOException, GitAPIException { - File remote = new File(root, "remote"); - File local = new File(root, "local"); - - try (Git git = Git.init().setDirectory(remote).call()) { - JGitTestUtil.write(new File(remote, "hello.txt"), "Hello world!"); - git.add().addFilepattern(".").call(); - git.commit().setMessage("Initial commit").call(); - } - - try (Git git = Git.cloneRepository().setURI(remote.toURI().toString()) - .setDirectory(local).call()) { - assertTrue(new File(local, ".git").exists()); - - JGitTestUtil.write(new File(local, "hi.txt"), "Hi!"); - git.add().addFilepattern(".").call(); - RevCommit commit1 = git.commit().setMessage("Commit on local repo") - .call(); - assertEquals("Commit on local repo", commit1.getFullMessage()); - assertNotNull(TreeWalk.forPath(git.getRepository(), "hello.txt", - commit1.getTree())); - } - - } - -} diff --git a/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties b/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties index b1450ef057..024ca77f21 100644 --- a/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties +++ b/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties @@ -640,8 +640,6 @@ readFileStoreAttributesFailed=Reading FileStore attributes from user config fail readerIsRequired=Reader is required readingObjectsFromLocalRepositoryFailed=reading objects from local repository failed: {0} readLastModifiedFailed=Reading lastModified of {0} failed -readPipeIsNotAllowed=FS.readPipe() isn't allowed for command ''{0}''. Working directory: ''{1}''. -readPipeIsNotAllowedRequiredPermission=FS.readPipe() isn't allowed for command ''{0}''. Working directory: ''{1}''. Required permission: {2}. readTimedOut=Read timed out after {0} ms receivePackObjectTooLarge1=Object too large, rejecting the pack. Max object size limit is {0} bytes. receivePackObjectTooLarge2=Object too large ({0} bytes), rejecting the pack. Max object size limit is {1} bytes. diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java index 310962b967..0980219e25 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java @@ -670,8 +670,6 @@ public class JGitText extends TranslationBundle { /***/ public String readerIsRequired; /***/ public String readingObjectsFromLocalRepositoryFailed; /***/ public String readLastModifiedFailed; - /***/ public String readPipeIsNotAllowed; - /***/ public String readPipeIsNotAllowedRequiredPermission; /***/ public String readTimedOut; /***/ public String receivePackObjectTooLarge1; /***/ public String receivePackObjectTooLarge2; diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/transport/SshSessionFactory.java b/org.eclipse.jgit/src/org/eclipse/jgit/transport/SshSessionFactory.java index a0194ea8b1..8120df0698 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/transport/SshSessionFactory.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/transport/SshSessionFactory.java @@ -11,8 +11,6 @@ package org.eclipse.jgit.transport; -import java.security.AccessController; -import java.security.PrivilegedAction; import java.util.Iterator; import java.util.ServiceLoader; @@ -99,9 +97,8 @@ public abstract class SshSessionFactory { * @since 5.2 */ public static String getLocalUserName() { - return AccessController - .doPrivileged((PrivilegedAction) () -> SystemReader - .getInstance().getProperty(Constants.OS_USER_NAME_KEY)); + return SystemReader.getInstance() + .getProperty(Constants.OS_USER_NAME_KEY); } /** diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/util/FS.java b/org.eclipse.jgit/src/org/eclipse/jgit/util/FS.java index 860c1c92fd..59bbacfa76 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/util/FS.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/util/FS.java @@ -30,7 +30,6 @@ import java.nio.file.InvalidPathException; import java.nio.file.Path; import java.nio.file.attribute.BasicFileAttributes; import java.nio.file.attribute.FileTime; -import java.security.AccessControlException; import java.text.MessageFormat; import java.time.Duration; import java.time.Instant; @@ -261,31 +260,6 @@ public abstract class FS { private static final AtomicInteger threadNumber = new AtomicInteger(1); - /** - * Don't use the default thread factory of the ForkJoinPool for the - * CompletableFuture; it runs without any privileges, which causes - * trouble if a SecurityManager is present. - *

- * Instead use normal daemon threads. They'll belong to the - * SecurityManager's thread group, or use the one of the calling thread, - * as appropriate. - *

- * - * @see java.util.concurrent.Executors#newCachedThreadPool() - */ - private static final ExecutorService FUTURE_RUNNER = new ThreadPoolExecutor( - 5, 5, 30L, TimeUnit.SECONDS, - new LinkedBlockingQueue<>(), - runnable -> { - Thread t = new Thread(runnable, - "JGit-FileStoreAttributeReader-" //$NON-NLS-1$ - + threadNumber.getAndIncrement()); - // Make sure these threads don't prevent application/JVM - // shutdown. - t.setDaemon(true); - return t; - }); - /** * Use a separate executor with at most one thread to synchronize * writing to the config. We write asynchronously since the config @@ -463,7 +437,7 @@ public abstract class FS { locks.remove(s); } return attributes; - }, FUTURE_RUNNER); + }); f = f.exceptionally(e -> { LOG.error(e.getLocalizedMessage(), e); return Optional.empty(); @@ -1391,13 +1365,6 @@ public abstract class FS { } } catch (IOException e) { LOG.error("Caught exception in FS.readPipe()", e); //$NON-NLS-1$ - } catch (AccessControlException e) { - LOG.warn(MessageFormat.format( - JGitText.get().readPipeIsNotAllowedRequiredPermission, - command, dir, e.getPermission())); - } catch (SecurityException e) { - LOG.warn(MessageFormat.format(JGitText.get().readPipeIsNotAllowed, - command, dir)); } if (debug) { LOG.debug("readpipe returns null"); //$NON-NLS-1$ diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/util/FS_Win32_Cygwin.java b/org.eclipse.jgit/src/org/eclipse/jgit/util/FS_Win32_Cygwin.java index 635351ac84..237879110a 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/util/FS_Win32_Cygwin.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/util/FS_Win32_Cygwin.java @@ -14,8 +14,6 @@ import static java.nio.charset.StandardCharsets.UTF_8; import java.io.File; import java.io.OutputStream; -import java.security.AccessController; -import java.security.PrivilegedAction; import java.util.ArrayList; import java.util.Arrays; import java.util.List; @@ -43,10 +41,7 @@ public class FS_Win32_Cygwin extends FS_Win32 { * @return true if cygwin is found */ public static boolean isCygwin() { - final String path = AccessController - .doPrivileged((PrivilegedAction) () -> System - .getProperty("java.library.path") //$NON-NLS-1$ - ); + final String path = System.getProperty("java.library.path"); //$NON-NLS-1$ if (path == null) return false; File found = FS.searchPath(path, "cygpath.exe"); //$NON-NLS-1$ @@ -99,9 +94,7 @@ public class FS_Win32_Cygwin extends FS_Win32 { @Override protected File userHomeImpl() { - final String home = AccessController.doPrivileged( - (PrivilegedAction) () -> System.getenv("HOME") //$NON-NLS-1$ - ); + final String home = System.getenv("HOME"); //$NON-NLS-1$ if (home == null || home.length() == 0) return super.userHomeImpl(); return resolve(new File("."), home); //$NON-NLS-1$ diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/util/SystemReader.java b/org.eclipse.jgit/src/org/eclipse/jgit/util/SystemReader.java index ed62c71371..03ed925617 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/util/SystemReader.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/util/SystemReader.java @@ -23,8 +23,6 @@ import java.nio.charset.UnsupportedCharsetException; import java.nio.file.InvalidPathException; import java.nio.file.Path; import java.nio.file.Paths; -import java.security.AccessController; -import java.security.PrivilegedAction; import java.text.DateFormat; import java.text.SimpleDateFormat; import java.util.Locale; @@ -670,9 +668,7 @@ public abstract class SystemReader { } private String getOsName() { - return AccessController.doPrivileged( - (PrivilegedAction) () -> getProperty("os.name") //$NON-NLS-1$ - ); + return getProperty("os.name"); //$NON-NLS-1$ } /** diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/util/io/ThrowingPrintWriter.java b/org.eclipse.jgit/src/org/eclipse/jgit/util/io/ThrowingPrintWriter.java index 4764676c88..13982b133c 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/util/io/ThrowingPrintWriter.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/util/io/ThrowingPrintWriter.java @@ -11,8 +11,6 @@ package org.eclipse.jgit.util.io; import java.io.IOException; import java.io.Writer; -import java.security.AccessController; -import java.security.PrivilegedAction; import org.eclipse.jgit.util.SystemReader; @@ -35,10 +33,7 @@ public class ThrowingPrintWriter extends Writer { */ public ThrowingPrintWriter(Writer out) { this.out = out; - LF = AccessController - .doPrivileged((PrivilegedAction) () -> SystemReader - .getInstance().getProperty("line.separator") //$NON-NLS-1$ - ); + LF = SystemReader.getInstance().getProperty("line.separator"); //$NON-NLS-1$ } @Override -- cgit v1.2.3 From facddaccf591c3f3da9384269e1b592ca179a794 Mon Sep 17 00:00:00 2001 From: Ivan Frade Date: Thu, 31 Oct 2024 12:50:00 -0700 Subject: [errorprone] BaseRepositoryBuilder: Use #split(sep, limit) String.split(String) and Pattern.split(CharSequence) have surprising behaviour [1]. We use one of the recommended replacements: #split(sep, limit). [1] https://errorprone.info/bugpattern/StringSplitter Change-Id: Ie1cf7590bd8660d21c79c5c3c1bc2765e5d9462b --- .../src/org/eclipse/jgit/lib/BaseRepositoryBuilder.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/lib/BaseRepositoryBuilder.java b/org.eclipse.jgit/src/org/eclipse/jgit/lib/BaseRepositoryBuilder.java index d232be6276..0c1da83dfb 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/lib/BaseRepositoryBuilder.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/lib/BaseRepositoryBuilder.java @@ -15,6 +15,7 @@ import static org.eclipse.jgit.lib.ConfigConstants.CONFIG_KEY_BARE; import static org.eclipse.jgit.lib.ConfigConstants.CONFIG_KEY_WORKTREE; import static org.eclipse.jgit.lib.Constants.CONFIG; import static org.eclipse.jgit.lib.Constants.DOT_GIT; +import static org.eclipse.jgit.lib.Constants.GITDIR_FILE; import static org.eclipse.jgit.lib.Constants.GIT_ALTERNATE_OBJECT_DIRECTORIES_KEY; import static org.eclipse.jgit.lib.Constants.GIT_CEILING_DIRECTORIES_KEY; import static org.eclipse.jgit.lib.Constants.GIT_COMMON_DIR_KEY; @@ -23,7 +24,6 @@ import static org.eclipse.jgit.lib.Constants.GIT_INDEX_FILE_KEY; import static org.eclipse.jgit.lib.Constants.GIT_OBJECT_DIRECTORY_KEY; import static org.eclipse.jgit.lib.Constants.GIT_WORK_TREE_KEY; import static org.eclipse.jgit.lib.Constants.OBJECTS; -import static org.eclipse.jgit.lib.Constants.GITDIR_FILE; import java.io.File; import java.io.IOException; @@ -485,7 +485,7 @@ public class BaseRepositoryBuilder Date: Fri, 1 Nov 2024 08:58:27 -0700 Subject: DfsPackCompactor: write object size index Currently the compactor is not writing the object size index for packs. As it is using PackWriter to generate the packs, it needs to explicitely call the writes of each extension. Invoke writeObjectSizeIndex in the compactor. The pack writer will write one if the configuration says so. Change-Id: I8d6bbbb5bd67bfc7dd511aa76463512b1e86a45d --- .../internal/storage/dfs/DfsPackCompacterTest.java | 44 ++++++++++++++++++++++ .../internal/storage/dfs/DfsPackCompactor.java | 17 +++++++++ 2 files changed, 61 insertions(+) diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/dfs/DfsPackCompacterTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/dfs/DfsPackCompacterTest.java index c516e30f50..c3b6aa85a2 100644 --- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/dfs/DfsPackCompacterTest.java +++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/dfs/DfsPackCompacterTest.java @@ -12,13 +12,18 @@ package org.eclipse.jgit.internal.storage.dfs; import static org.eclipse.jgit.internal.storage.dfs.DfsObjDatabase.PackSource.COMPACT; import static org.eclipse.jgit.internal.storage.dfs.DfsObjDatabase.PackSource.INSERT; +import static org.eclipse.jgit.internal.storage.pack.PackExt.OBJECT_SIZE_INDEX; import static org.eclipse.jgit.internal.storage.pack.PackExt.PACK; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; import java.io.IOException; +import java.util.Arrays; +import java.util.Optional; import org.eclipse.jgit.junit.TestRepository; +import org.eclipse.jgit.lib.ConfigConstants; import org.eclipse.jgit.revwalk.RevCommit; import org.junit.Before; import org.junit.Test; @@ -98,6 +103,40 @@ public class DfsPackCompacterTest { pack.getPackDescription().getEstimatedPackSize()); } + @Test + public void testObjectSizeIndexWritten() throws Exception { + writeObjectSizeIndex(repo, true); + RevCommit commit0 = commit().message("0").create(); + RevCommit commit1 = commit().message("1").parent(commit0).create(); + git.update("master", commit1); + + compact(); + + Optional compactPack = Arrays.stream(odb.getPacks()) + .filter(pack -> pack.getPackDescription() + .getPackSource() == COMPACT) + .findFirst(); + assertTrue(compactPack.isPresent()); + assertTrue(compactPack.get().getPackDescription().hasFileExt(OBJECT_SIZE_INDEX)); + } + + @Test + public void testObjectSizeIndexNotWritten() throws Exception { + writeObjectSizeIndex(repo, false); + RevCommit commit0 = commit().message("0").create(); + RevCommit commit1 = commit().message("1").parent(commit0).create(); + git.update("master", commit1); + + compact(); + + Optional compactPack = Arrays.stream(odb.getPacks()) + .filter(pack -> pack.getPackDescription() + .getPackSource() == COMPACT) + .findFirst(); + assertTrue(compactPack.isPresent()); + assertFalse(compactPack.get().getPackDescription().hasFileExt(OBJECT_SIZE_INDEX)); + } + private TestRepository.CommitBuilder commit() { return git.commit(); } @@ -108,4 +147,9 @@ public class DfsPackCompacterTest { compactor.compact(null); odb.clearCache(); } + + private static void writeObjectSizeIndex(DfsRepository repo, boolean should) { + repo.getConfig().setInt(ConfigConstants.CONFIG_PACK_SECTION, null, + ConfigConstants.CONFIG_KEY_MIN_BYTES_OBJ_SIZE_INDEX, should ? 0 : -1); + } } diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/dfs/DfsPackCompactor.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/dfs/DfsPackCompactor.java index b32cddae77..f9c01b9d6e 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/dfs/DfsPackCompactor.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/dfs/DfsPackCompactor.java @@ -12,6 +12,7 @@ package org.eclipse.jgit.internal.storage.dfs; import static org.eclipse.jgit.internal.storage.dfs.DfsObjDatabase.PackSource.COMPACT; import static org.eclipse.jgit.internal.storage.dfs.DfsObjDatabase.PackSource.GC; +import static org.eclipse.jgit.internal.storage.pack.PackExt.OBJECT_SIZE_INDEX; import static org.eclipse.jgit.internal.storage.pack.PackExt.PACK; import static org.eclipse.jgit.internal.storage.pack.PackExt.REFTABLE; import static org.eclipse.jgit.internal.storage.pack.StoredObjectRepresentation.PACK_DELTA; @@ -44,6 +45,7 @@ import org.eclipse.jgit.revwalk.RevWalk; import org.eclipse.jgit.storage.pack.PackConfig; import org.eclipse.jgit.storage.pack.PackStatistics; import org.eclipse.jgit.util.BlockList; +import org.eclipse.jgit.util.io.CountingOutputStream; /** * Combine several pack files into one pack. @@ -247,6 +249,7 @@ public class DfsPackCompactor { try { writePack(objdb, outDesc, pw, pm); writeIndex(objdb, outDesc, pw); + writeObjectSizeIndex(objdb, outDesc, pw); PackStatistics stats = pw.getStatistics(); @@ -459,6 +462,20 @@ public class DfsPackCompactor { pw.writeIndex(objdb.getPackIndexWriter(pack, pw.getIndexVersion())); } + private static void writeObjectSizeIndex(DfsObjDatabase objdb, + DfsPackDescription pack, + PackWriter pw) throws IOException { + try (DfsOutputStream out = objdb.writeFile(pack, OBJECT_SIZE_INDEX)) { + CountingOutputStream cnt = new CountingOutputStream(out); + pw.writeObjectSizeIndex(cnt); + if (cnt.getCount() > 0) { + pack.addFileExt(OBJECT_SIZE_INDEX); + pack.setFileSize(OBJECT_SIZE_INDEX, cnt.getCount()); + pack.setBlockSize(OBJECT_SIZE_INDEX, out.blockSize()); + } + } + } + static ReftableConfig configureReftable(ReftableConfig cfg, DfsOutputStream out) { int bs = out.blockSize(); -- cgit v1.2.3 From 6042cbc12960f4b2d6deb924705238717de0d6fd Mon Sep 17 00:00:00 2001 From: Matthias Sohn Date: Tue, 5 Nov 2024 01:42:16 +0100 Subject: Update bytebuddy to 1.15.10 Change-Id: I49cd5bedd86601380a26f7a7213fc78ebd091393 --- WORKSPACE | 6 +++--- org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.17.target | 6 +++--- org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.18.target | 6 +++--- org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.19.target | 6 +++--- org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.20.target | 6 +++--- org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.21.target | 6 +++--- org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.22.target | 6 +++--- org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.23.target | 6 +++--- org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.24.target | 6 +++--- org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.25.target | 6 +++--- org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.26.target | 6 +++--- org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.27.target | 6 +++--- org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.28.target | 6 +++--- org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.29.target | 6 +++--- org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.30.target | 6 +++--- org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.31.target | 6 +++--- org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.32.target | 6 +++--- org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.33.target | 6 +++--- org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.34.target | 6 +++--- .../org.eclipse.jgit.target/maven/dependencies.tpd | 4 ++-- pom.xml | 2 +- 21 files changed, 60 insertions(+), 60 deletions(-) diff --git a/WORKSPACE b/WORKSPACE index d1ed57487d..dc4d322558 100644 --- a/WORKSPACE +++ b/WORKSPACE @@ -220,18 +220,18 @@ maven_jar( sha1 = "0d26263eb7524252d98e602fc6942996a3195e29", ) -BYTE_BUDDY_VERSION = "1.15.7" +BYTE_BUDDY_VERSION = "1.15.10" maven_jar( name = "bytebuddy", artifact = "net.bytebuddy:byte-buddy:" + BYTE_BUDDY_VERSION, - sha1 = "658064662e33b045bb9499b1c8afeae656974b18", + sha1 = "635c873fadd853c084f84fdc3cbd58c5dd8537f9", ) maven_jar( name = "bytebuddy-agent", artifact = "net.bytebuddy:byte-buddy-agent:" + BYTE_BUDDY_VERSION, - sha1 = "0f0a8a3b1440714d548ec54093eb34e3f2726013", + sha1 = "0e8eb255b2c378b9a6c7341e7b0e12f0a5636377", ) maven_jar( diff --git a/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.17.target b/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.17.target index 872ff86d22..d16431df75 100644 --- a/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.17.target +++ b/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.17.target @@ -1,7 +1,7 @@ - + @@ -193,13 +193,13 @@ net.bytebuddy byte-buddy - 1.15.7 + 1.15.10 jar net.bytebuddy byte-buddy-agent - 1.15.7 + 1.15.10 jar diff --git a/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.18.target b/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.18.target index d58275a415..ac9d7c4e95 100644 --- a/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.18.target +++ b/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.18.target @@ -1,7 +1,7 @@ - + @@ -193,13 +193,13 @@ net.bytebuddy byte-buddy - 1.15.7 + 1.15.10 jar net.bytebuddy byte-buddy-agent - 1.15.7 + 1.15.10 jar diff --git a/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.19.target b/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.19.target index be15fdd693..f7fd352a32 100644 --- a/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.19.target +++ b/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.19.target @@ -1,7 +1,7 @@ - + @@ -193,13 +193,13 @@ net.bytebuddy byte-buddy - 1.15.7 + 1.15.10 jar net.bytebuddy byte-buddy-agent - 1.15.7 + 1.15.10 jar diff --git a/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.20.target b/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.20.target index 7de5d871d8..a03692ab28 100644 --- a/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.20.target +++ b/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.20.target @@ -1,7 +1,7 @@ - + @@ -193,13 +193,13 @@ net.bytebuddy byte-buddy - 1.15.7 + 1.15.10 jar net.bytebuddy byte-buddy-agent - 1.15.7 + 1.15.10 jar diff --git a/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.21.target b/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.21.target index 6a5e8160b3..d90aa60409 100644 --- a/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.21.target +++ b/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.21.target @@ -1,7 +1,7 @@ - + @@ -193,13 +193,13 @@ net.bytebuddy byte-buddy - 1.15.7 + 1.15.10 jar net.bytebuddy byte-buddy-agent - 1.15.7 + 1.15.10 jar diff --git a/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.22.target b/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.22.target index 4e860ff28a..2f7eb7adfb 100644 --- a/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.22.target +++ b/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.22.target @@ -1,7 +1,7 @@ - + @@ -193,13 +193,13 @@ net.bytebuddy byte-buddy - 1.15.7 + 1.15.10 jar net.bytebuddy byte-buddy-agent - 1.15.7 + 1.15.10 jar diff --git a/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.23.target b/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.23.target index 7af598f034..4dd26b9961 100644 --- a/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.23.target +++ b/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.23.target @@ -1,7 +1,7 @@ - + @@ -193,13 +193,13 @@ net.bytebuddy byte-buddy - 1.15.7 + 1.15.10 jar net.bytebuddy byte-buddy-agent - 1.15.7 + 1.15.10 jar diff --git a/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.24.target b/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.24.target index 2ca0d56c60..e0a4bab260 100644 --- a/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.24.target +++ b/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.24.target @@ -1,7 +1,7 @@ - + @@ -193,13 +193,13 @@ net.bytebuddy byte-buddy - 1.15.7 + 1.15.10 jar net.bytebuddy byte-buddy-agent - 1.15.7 + 1.15.10 jar diff --git a/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.25.target b/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.25.target index 262ae68cee..700b813313 100644 --- a/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.25.target +++ b/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.25.target @@ -1,7 +1,7 @@ - + @@ -193,13 +193,13 @@ net.bytebuddy byte-buddy - 1.15.7 + 1.15.10 jar net.bytebuddy byte-buddy-agent - 1.15.7 + 1.15.10 jar diff --git a/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.26.target b/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.26.target index e33f0b0c1a..a03967df61 100644 --- a/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.26.target +++ b/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.26.target @@ -1,7 +1,7 @@ - + @@ -193,13 +193,13 @@ net.bytebuddy byte-buddy - 1.15.7 + 1.15.10 jar net.bytebuddy byte-buddy-agent - 1.15.7 + 1.15.10 jar diff --git a/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.27.target b/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.27.target index 65fa024ada..528f6bb74b 100644 --- a/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.27.target +++ b/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.27.target @@ -1,7 +1,7 @@ - + @@ -193,13 +193,13 @@ net.bytebuddy byte-buddy - 1.15.7 + 1.15.10 jar net.bytebuddy byte-buddy-agent - 1.15.7 + 1.15.10 jar diff --git a/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.28.target b/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.28.target index e189127718..dd960ee053 100644 --- a/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.28.target +++ b/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.28.target @@ -1,7 +1,7 @@ - + @@ -193,13 +193,13 @@ net.bytebuddy byte-buddy - 1.15.7 + 1.15.10 jar net.bytebuddy byte-buddy-agent - 1.15.7 + 1.15.10 jar diff --git a/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.29.target b/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.29.target index 49442a97f4..dada6487ec 100644 --- a/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.29.target +++ b/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.29.target @@ -1,7 +1,7 @@ - + @@ -193,13 +193,13 @@ net.bytebuddy byte-buddy - 1.15.7 + 1.15.10 jar net.bytebuddy byte-buddy-agent - 1.15.7 + 1.15.10 jar diff --git a/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.30.target b/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.30.target index 18c4707c82..403ed29801 100644 --- a/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.30.target +++ b/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.30.target @@ -1,7 +1,7 @@ - + @@ -193,13 +193,13 @@ net.bytebuddy byte-buddy - 1.15.7 + 1.15.10 jar net.bytebuddy byte-buddy-agent - 1.15.7 + 1.15.10 jar diff --git a/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.31.target b/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.31.target index 0c23c0c176..f812d63086 100644 --- a/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.31.target +++ b/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.31.target @@ -1,7 +1,7 @@ - + @@ -193,13 +193,13 @@ net.bytebuddy byte-buddy - 1.15.7 + 1.15.10 jar net.bytebuddy byte-buddy-agent - 1.15.7 + 1.15.10 jar diff --git a/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.32.target b/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.32.target index e6ee190905..6bcc689650 100644 --- a/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.32.target +++ b/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.32.target @@ -1,7 +1,7 @@ - + @@ -193,13 +193,13 @@ net.bytebuddy byte-buddy - 1.15.7 + 1.15.10 jar net.bytebuddy byte-buddy-agent - 1.15.7 + 1.15.10 jar diff --git a/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.33.target b/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.33.target index 4cc1959d01..c99e59d7a9 100644 --- a/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.33.target +++ b/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.33.target @@ -1,7 +1,7 @@ - + @@ -193,13 +193,13 @@ net.bytebuddy byte-buddy - 1.15.7 + 1.15.10 jar net.bytebuddy byte-buddy-agent - 1.15.7 + 1.15.10 jar diff --git a/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.34.target b/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.34.target index 0dfe190bc7..fe7e9d292a 100644 --- a/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.34.target +++ b/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.34.target @@ -1,7 +1,7 @@ - + @@ -193,13 +193,13 @@ net.bytebuddy byte-buddy - 1.15.7 + 1.15.10 jar net.bytebuddy byte-buddy-agent - 1.15.7 + 1.15.10 jar diff --git a/org.eclipse.jgit.packaging/org.eclipse.jgit.target/maven/dependencies.tpd b/org.eclipse.jgit.packaging/org.eclipse.jgit.target/maven/dependencies.tpd index 5e29950bc0..630107daed 100644 --- a/org.eclipse.jgit.packaging/org.eclipse.jgit.target/maven/dependencies.tpd +++ b/org.eclipse.jgit.packaging/org.eclipse.jgit.target/maven/dependencies.tpd @@ -97,12 +97,12 @@ maven bytebuddy dependency { groupId = "net.bytebuddy" artifactId = "byte-buddy" - version = "1.15.7" + version = "1.15.10" } dependency { groupId = "net.bytebuddy" artifactId = "byte-buddy-agent" - version = "1.15.7" + version = "1.15.10" } } diff --git a/pom.xml b/pom.xml index 00a89e3c10..a35c81ebd2 100644 --- a/pom.xml +++ b/pom.xml @@ -148,7 +148,7 @@ 2.2 3.26.3 5.15.0 - 1.15.7 + 1.15.10 jacoco -- cgit v1.2.3 From 31ff8955d6ed23aa41456b00b1f151913f28120d Mon Sep 17 00:00:00 2001 From: Matthias Sohn Date: Tue, 5 Nov 2024 02:00:24 +0100 Subject: Update bouncycastle to 1.79 Change-Id: Ib81d73075ebc9dcdc73aacf30fa02ad56a502d51 --- WORKSPACE | 18 +++++++++--------- .../internal/keys/OCBPBEProtectionRemoverFactory.java | 7 +++++++ .../org.eclipse.jgit.target/jgit-4.17.target | 10 +++++----- .../org.eclipse.jgit.target/jgit-4.18.target | 10 +++++----- .../org.eclipse.jgit.target/jgit-4.19.target | 10 +++++----- .../org.eclipse.jgit.target/jgit-4.20.target | 10 +++++----- .../org.eclipse.jgit.target/jgit-4.21.target | 10 +++++----- .../org.eclipse.jgit.target/jgit-4.22.target | 10 +++++----- .../org.eclipse.jgit.target/jgit-4.23.target | 10 +++++----- .../org.eclipse.jgit.target/jgit-4.24.target | 10 +++++----- .../org.eclipse.jgit.target/jgit-4.25.target | 10 +++++----- .../org.eclipse.jgit.target/jgit-4.26.target | 10 +++++----- .../org.eclipse.jgit.target/jgit-4.27.target | 10 +++++----- .../org.eclipse.jgit.target/jgit-4.28.target | 10 +++++----- .../org.eclipse.jgit.target/jgit-4.29.target | 10 +++++----- .../org.eclipse.jgit.target/jgit-4.30.target | 10 +++++----- .../org.eclipse.jgit.target/jgit-4.31.target | 10 +++++----- .../org.eclipse.jgit.target/jgit-4.32.target | 10 +++++----- .../org.eclipse.jgit.target/jgit-4.33.target | 10 +++++----- .../org.eclipse.jgit.target/jgit-4.34.target | 10 +++++----- .../org.eclipse.jgit.target/maven/dependencies.tpd | 8 ++++---- pom.xml | 2 +- 22 files changed, 111 insertions(+), 104 deletions(-) diff --git a/WORKSPACE b/WORKSPACE index dc4d322558..1050a44cff 100644 --- a/WORKSPACE +++ b/WORKSPACE @@ -296,32 +296,32 @@ maven_jar( sha1 = "6f4beb9b482ea0d9db9db0564742aa2e4e0bf3c4", ) -BOUNCYCASTLE_VER = "1.78.1" +BOUNCYCASTLE_VER = "1.79" maven_jar( name = "bcpg", artifact = "org.bouncycastle:bcpg-jdk18on:" + BOUNCYCASTLE_VER, - sha1 = "6c8dbcec20355278ec54840e735f63db2479150e", - src_sha1 = "2ddef60d84dd8c14ebce4c13100f0bc55fed6922", + sha1 = "904dd8a8e1c9f7d58d1ffa7f4ca3fb00736a601f", + src_sha1 = "9e372826141edb213d5921131ee68dc276dc99ef", ) maven_jar( name = "bcprov", artifact = "org.bouncycastle:bcprov-jdk18on:" + BOUNCYCASTLE_VER, - sha1 = "39e9e45359e20998eb79c1828751f94a818d25f8", - src_sha1 = "70f58ec93da543dda6a21614b768cb2e386fd512", + sha1 = "4d8e2732bcee15f1db93df266c3f5b70ce5cac21", + src_sha1 = "8647816d667ee526a8e3a456229ac5f9f96d2315", ) maven_jar( name = "bcutil", artifact = "org.bouncycastle:bcutil-jdk18on:" + BOUNCYCASTLE_VER, - sha1 = "5353ca39fe2f148dab9ca1d637a43d0750456254", - src_sha1 = "8d2e0747f5d806f39a602f7f91610444d88c4e2c", + sha1 = "ecfc5aef97cc7676ea0de5c53c407b9f533f0ad5", + src_sha1 = "00df03977fb0b80395da655623abca9d7d7dcb66", ) maven_jar( name = "bcpkix", artifact = "org.bouncycastle:bcpkix-jdk18on:" + BOUNCYCASTLE_VER, - sha1 = "17b3541f736df97465f87d9f5b5dfa4991b37bb3", - src_sha1 = "3aeaf221772ad0c9c04593688cb86c6eb74d48b9", + sha1 = "7693cec3b8779b74b35466dcaeeaac7409872954", + src_sha1 = "57a60d1d9f75320eef70a095dfae679d97ade1c2", ) diff --git a/org.eclipse.jgit.gpg.bc/src/org/eclipse/jgit/gpg/bc/internal/keys/OCBPBEProtectionRemoverFactory.java b/org.eclipse.jgit.gpg.bc/src/org/eclipse/jgit/gpg/bc/internal/keys/OCBPBEProtectionRemoverFactory.java index 3924d68596..e6f1decbc1 100644 --- a/org.eclipse.jgit.gpg.bc/src/org/eclipse/jgit/gpg/bc/internal/keys/OCBPBEProtectionRemoverFactory.java +++ b/org.eclipse.jgit.gpg.bc/src/org/eclipse/jgit/gpg/bc/internal/keys/OCBPBEProtectionRemoverFactory.java @@ -120,6 +120,13 @@ class OCBPBEProtectionRemoverFactory } } } + + @Override + public byte[] recoverKeyData(int encAlgorithm, int aeadAlgorithm, + byte[] s2kKey, byte[] iv, int packetTag, int keyVersion, + byte[] keyData, byte[] pubkeyData) throws PGPException { + throw new UnsupportedOperationException(); + } }; } } \ No newline at end of file diff --git a/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.17.target b/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.17.target index d16431df75..623e964217 100644 --- a/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.17.target +++ b/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.17.target @@ -1,7 +1,7 @@ - + @@ -209,25 +209,25 @@ org.bouncycastle bcpg-jdk18on - 1.78.1 + 1.79 jar org.bouncycastle bcprov-jdk18on - 1.78.1 + 1.79 jar org.bouncycastle bcpkix-jdk18on - 1.78.1 + 1.79 jar org.bouncycastle bcutil-jdk18on - 1.78.1 + 1.79 jar diff --git a/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.18.target b/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.18.target index ac9d7c4e95..66d68e06f2 100644 --- a/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.18.target +++ b/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.18.target @@ -1,7 +1,7 @@ - + @@ -209,25 +209,25 @@ org.bouncycastle bcpg-jdk18on - 1.78.1 + 1.79 jar org.bouncycastle bcprov-jdk18on - 1.78.1 + 1.79 jar org.bouncycastle bcpkix-jdk18on - 1.78.1 + 1.79 jar org.bouncycastle bcutil-jdk18on - 1.78.1 + 1.79 jar diff --git a/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.19.target b/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.19.target index f7fd352a32..16b9706b3e 100644 --- a/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.19.target +++ b/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.19.target @@ -1,7 +1,7 @@ - + @@ -209,25 +209,25 @@ org.bouncycastle bcpg-jdk18on - 1.78.1 + 1.79 jar org.bouncycastle bcprov-jdk18on - 1.78.1 + 1.79 jar org.bouncycastle bcpkix-jdk18on - 1.78.1 + 1.79 jar org.bouncycastle bcutil-jdk18on - 1.78.1 + 1.79 jar diff --git a/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.20.target b/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.20.target index a03692ab28..9604132842 100644 --- a/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.20.target +++ b/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.20.target @@ -1,7 +1,7 @@ - + @@ -209,25 +209,25 @@ org.bouncycastle bcpg-jdk18on - 1.78.1 + 1.79 jar org.bouncycastle bcprov-jdk18on - 1.78.1 + 1.79 jar org.bouncycastle bcpkix-jdk18on - 1.78.1 + 1.79 jar org.bouncycastle bcutil-jdk18on - 1.78.1 + 1.79 jar diff --git a/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.21.target b/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.21.target index d90aa60409..78149754b4 100644 --- a/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.21.target +++ b/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.21.target @@ -1,7 +1,7 @@ - + @@ -209,25 +209,25 @@ org.bouncycastle bcpg-jdk18on - 1.78.1 + 1.79 jar org.bouncycastle bcprov-jdk18on - 1.78.1 + 1.79 jar org.bouncycastle bcpkix-jdk18on - 1.78.1 + 1.79 jar org.bouncycastle bcutil-jdk18on - 1.78.1 + 1.79 jar diff --git a/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.22.target b/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.22.target index 2f7eb7adfb..b5fcaf31dc 100644 --- a/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.22.target +++ b/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.22.target @@ -1,7 +1,7 @@ - + @@ -209,25 +209,25 @@ org.bouncycastle bcpg-jdk18on - 1.78.1 + 1.79 jar org.bouncycastle bcprov-jdk18on - 1.78.1 + 1.79 jar org.bouncycastle bcpkix-jdk18on - 1.78.1 + 1.79 jar org.bouncycastle bcutil-jdk18on - 1.78.1 + 1.79 jar diff --git a/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.23.target b/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.23.target index 4dd26b9961..b57ac90a93 100644 --- a/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.23.target +++ b/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.23.target @@ -1,7 +1,7 @@ - + @@ -209,25 +209,25 @@ org.bouncycastle bcpg-jdk18on - 1.78.1 + 1.79 jar org.bouncycastle bcprov-jdk18on - 1.78.1 + 1.79 jar org.bouncycastle bcpkix-jdk18on - 1.78.1 + 1.79 jar org.bouncycastle bcutil-jdk18on - 1.78.1 + 1.79 jar diff --git a/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.24.target b/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.24.target index e0a4bab260..dd7081e0ae 100644 --- a/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.24.target +++ b/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.24.target @@ -1,7 +1,7 @@ - + @@ -209,25 +209,25 @@ org.bouncycastle bcpg-jdk18on - 1.78.1 + 1.79 jar org.bouncycastle bcprov-jdk18on - 1.78.1 + 1.79 jar org.bouncycastle bcpkix-jdk18on - 1.78.1 + 1.79 jar org.bouncycastle bcutil-jdk18on - 1.78.1 + 1.79 jar diff --git a/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.25.target b/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.25.target index 700b813313..e5cbf9b35a 100644 --- a/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.25.target +++ b/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.25.target @@ -1,7 +1,7 @@ - + @@ -209,25 +209,25 @@ org.bouncycastle bcpg-jdk18on - 1.78.1 + 1.79 jar org.bouncycastle bcprov-jdk18on - 1.78.1 + 1.79 jar org.bouncycastle bcpkix-jdk18on - 1.78.1 + 1.79 jar org.bouncycastle bcutil-jdk18on - 1.78.1 + 1.79 jar diff --git a/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.26.target b/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.26.target index a03967df61..48ffc2f27f 100644 --- a/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.26.target +++ b/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.26.target @@ -1,7 +1,7 @@ - + @@ -209,25 +209,25 @@ org.bouncycastle bcpg-jdk18on - 1.78.1 + 1.79 jar org.bouncycastle bcprov-jdk18on - 1.78.1 + 1.79 jar org.bouncycastle bcpkix-jdk18on - 1.78.1 + 1.79 jar org.bouncycastle bcutil-jdk18on - 1.78.1 + 1.79 jar diff --git a/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.27.target b/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.27.target index 528f6bb74b..5c7e1a4596 100644 --- a/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.27.target +++ b/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.27.target @@ -1,7 +1,7 @@ - + @@ -209,25 +209,25 @@ org.bouncycastle bcpg-jdk18on - 1.78.1 + 1.79 jar org.bouncycastle bcprov-jdk18on - 1.78.1 + 1.79 jar org.bouncycastle bcpkix-jdk18on - 1.78.1 + 1.79 jar org.bouncycastle bcutil-jdk18on - 1.78.1 + 1.79 jar diff --git a/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.28.target b/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.28.target index dd960ee053..735d1cf5c9 100644 --- a/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.28.target +++ b/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.28.target @@ -1,7 +1,7 @@ - + @@ -209,25 +209,25 @@ org.bouncycastle bcpg-jdk18on - 1.78.1 + 1.79 jar org.bouncycastle bcprov-jdk18on - 1.78.1 + 1.79 jar org.bouncycastle bcpkix-jdk18on - 1.78.1 + 1.79 jar org.bouncycastle bcutil-jdk18on - 1.78.1 + 1.79 jar diff --git a/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.29.target b/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.29.target index dada6487ec..9357dee548 100644 --- a/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.29.target +++ b/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.29.target @@ -1,7 +1,7 @@ - + @@ -209,25 +209,25 @@ org.bouncycastle bcpg-jdk18on - 1.78.1 + 1.79 jar org.bouncycastle bcprov-jdk18on - 1.78.1 + 1.79 jar org.bouncycastle bcpkix-jdk18on - 1.78.1 + 1.79 jar org.bouncycastle bcutil-jdk18on - 1.78.1 + 1.79 jar diff --git a/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.30.target b/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.30.target index 403ed29801..295b9c3c46 100644 --- a/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.30.target +++ b/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.30.target @@ -1,7 +1,7 @@ - + @@ -209,25 +209,25 @@ org.bouncycastle bcpg-jdk18on - 1.78.1 + 1.79 jar org.bouncycastle bcprov-jdk18on - 1.78.1 + 1.79 jar org.bouncycastle bcpkix-jdk18on - 1.78.1 + 1.79 jar org.bouncycastle bcutil-jdk18on - 1.78.1 + 1.79 jar diff --git a/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.31.target b/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.31.target index f812d63086..b9b690ccca 100644 --- a/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.31.target +++ b/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.31.target @@ -1,7 +1,7 @@ - + @@ -209,25 +209,25 @@ org.bouncycastle bcpg-jdk18on - 1.78.1 + 1.79 jar org.bouncycastle bcprov-jdk18on - 1.78.1 + 1.79 jar org.bouncycastle bcpkix-jdk18on - 1.78.1 + 1.79 jar org.bouncycastle bcutil-jdk18on - 1.78.1 + 1.79 jar diff --git a/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.32.target b/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.32.target index 6bcc689650..2407a8708f 100644 --- a/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.32.target +++ b/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.32.target @@ -1,7 +1,7 @@ - + @@ -209,25 +209,25 @@ org.bouncycastle bcpg-jdk18on - 1.78.1 + 1.79 jar org.bouncycastle bcprov-jdk18on - 1.78.1 + 1.79 jar org.bouncycastle bcpkix-jdk18on - 1.78.1 + 1.79 jar org.bouncycastle bcutil-jdk18on - 1.78.1 + 1.79 jar diff --git a/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.33.target b/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.33.target index c99e59d7a9..78433361fe 100644 --- a/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.33.target +++ b/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.33.target @@ -1,7 +1,7 @@ - + @@ -209,25 +209,25 @@ org.bouncycastle bcpg-jdk18on - 1.78.1 + 1.79 jar org.bouncycastle bcprov-jdk18on - 1.78.1 + 1.79 jar org.bouncycastle bcpkix-jdk18on - 1.78.1 + 1.79 jar org.bouncycastle bcutil-jdk18on - 1.78.1 + 1.79 jar diff --git a/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.34.target b/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.34.target index fe7e9d292a..c913b9dcca 100644 --- a/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.34.target +++ b/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.34.target @@ -1,7 +1,7 @@ - + @@ -209,25 +209,25 @@ org.bouncycastle bcpg-jdk18on - 1.78.1 + 1.79 jar org.bouncycastle bcprov-jdk18on - 1.78.1 + 1.79 jar org.bouncycastle bcpkix-jdk18on - 1.78.1 + 1.79 jar org.bouncycastle bcutil-jdk18on - 1.78.1 + 1.79 jar diff --git a/org.eclipse.jgit.packaging/org.eclipse.jgit.target/maven/dependencies.tpd b/org.eclipse.jgit.packaging/org.eclipse.jgit.target/maven/dependencies.tpd index 630107daed..68b457c0f3 100644 --- a/org.eclipse.jgit.packaging/org.eclipse.jgit.target/maven/dependencies.tpd +++ b/org.eclipse.jgit.packaging/org.eclipse.jgit.target/maven/dependencies.tpd @@ -69,22 +69,22 @@ maven bouncycastle dependency { groupId = "org.bouncycastle" artifactId = "bcpg-jdk18on" - version = "1.78.1" + version = "1.79" } dependency { groupId = "org.bouncycastle" artifactId = "bcprov-jdk18on" - version = "1.78.1" + version = "1.79" } dependency { groupId = "org.bouncycastle" artifactId = "bcpkix-jdk18on" - version = "1.78.1" + version = "1.79" } dependency { groupId = "org.bouncycastle" artifactId = "bcutil-jdk18on" - version = "1.78.1" + version = "1.79" } } diff --git a/pom.xml b/pom.xml index a35c81ebd2..e7f24fe083 100644 --- a/pom.xml +++ b/pom.xml @@ -137,7 +137,7 @@ 1.7.36 3.6.3 2.11.0 - 1.78.1 + 1.79 4.8.5.0 3.5.1 3.3.2 -- cgit v1.2.3 From a3dbf3af63db5dce8566a3a1ad03f67c1e0bd090 Mon Sep 17 00:00:00 2001 From: Ivan Frade Date: Tue, 5 Nov 2024 15:03:26 -0800 Subject: [errorprone] bc: Remove unused SExprParser#parseSecretKey errorprone complains about using Date in the SExprParser class. All the usages are in a variant of the parseSecretKey method that doesn't have any callers. Remove the unused method. Change-Id: I80f5aa58877b9da31729cb90b0219e45d96144a8 --- .../jgit/gpg/bc/internal/keys/SExprParser.java | 148 --------------------- 1 file changed, 148 deletions(-) diff --git a/org.eclipse.jgit.gpg.bc/src/org/eclipse/jgit/gpg/bc/internal/keys/SExprParser.java b/org.eclipse.jgit.gpg.bc/src/org/eclipse/jgit/gpg/bc/internal/keys/SExprParser.java index fd030ee032..b062eadb43 100644 --- a/org.eclipse.jgit.gpg.bc/src/org/eclipse/jgit/gpg/bc/internal/keys/SExprParser.java +++ b/org.eclipse.jgit.gpg.bc/src/org/eclipse/jgit/gpg/bc/internal/keys/SExprParser.java @@ -28,7 +28,6 @@ import java.io.IOException; import java.io.InputStream; import java.io.OutputStream; import java.math.BigInteger; -import java.util.Date; import org.bouncycastle.asn1.ASN1ObjectIdentifier; import org.bouncycastle.asn1.x9.ECNamedCurveTable; @@ -40,8 +39,6 @@ import org.bouncycastle.bcpg.ECSecretBCPGKey; import org.bouncycastle.bcpg.ElGamalPublicBCPGKey; import org.bouncycastle.bcpg.ElGamalSecretBCPGKey; import org.bouncycastle.bcpg.HashAlgorithmTags; -import org.bouncycastle.bcpg.PublicKeyAlgorithmTags; -import org.bouncycastle.bcpg.PublicKeyPacket; import org.bouncycastle.bcpg.RSAPublicBCPGKey; import org.bouncycastle.bcpg.RSASecretBCPGKey; import org.bouncycastle.bcpg.S2K; @@ -50,7 +47,6 @@ import org.bouncycastle.bcpg.SymmetricKeyAlgorithmTags; import org.bouncycastle.openpgp.PGPException; import org.bouncycastle.openpgp.PGPPublicKey; import org.bouncycastle.openpgp.PGPSecretKey; -import org.bouncycastle.openpgp.operator.KeyFingerPrintCalculator; import org.bouncycastle.openpgp.operator.PBEProtectionRemoverFactory; import org.bouncycastle.openpgp.operator.PBESecretKeyDecryptor; import org.bouncycastle.openpgp.operator.PGPDigestCalculator; @@ -262,150 +258,6 @@ public class SExprParser { throw new PGPException("unknown key type found"); } - /** - * Parse a secret key from one of the GPG S expression keys. - * - * @param inputStream - * to read from - * @param keyProtectionRemoverFactory - * for decrypting encrypted keys - * @param fingerPrintCalculator - * for calculating key fingerprints - * - * @return a secret key object. - * @throws IOException - * if an IO error occurred - * @throws PGPException - * if a PGP error occurred - */ - public PGPSecretKey parseSecretKey(InputStream inputStream, - PBEProtectionRemoverFactory keyProtectionRemoverFactory, - KeyFingerPrintCalculator fingerPrintCalculator) - throws IOException, PGPException { - SXprUtils.skipOpenParenthesis(inputStream); - - String type; - - type = SXprUtils.readString(inputStream, inputStream.read()); - if (type.equals("protected-private-key") - || type.equals("private-key")) { - SXprUtils.skipOpenParenthesis(inputStream); - - String keyType = SXprUtils.readString(inputStream, - inputStream.read()); - if (keyType.equals("ecc")) { - SXprUtils.skipOpenParenthesis(inputStream); - - String curveID = SXprUtils.readString(inputStream, - inputStream.read()); - String curveName = SXprUtils.readString(inputStream, - inputStream.read()); - - if (curveName.startsWith("NIST ")) { - curveName = curveName.substring("NIST ".length()); - } - - SXprUtils.skipCloseParenthesis(inputStream); - - byte[] qVal; - - SXprUtils.skipOpenParenthesis(inputStream); - - type = SXprUtils.readString(inputStream, inputStream.read()); - // JGit: c.f. https://github.com/bcgit/bc-java/issues/1590. - // There may be a flags sub-list here for ed25519 or curve25519. - if (type.equals("flags")) { - SXprUtils.readString(inputStream, inputStream.read()); - SXprUtils.skipCloseParenthesis(inputStream); - SXprUtils.skipOpenParenthesis(inputStream); - type = SXprUtils.readString(inputStream, - inputStream.read()); - } - if (type.equals("q")) { - qVal = SXprUtils.readBytes(inputStream, inputStream.read()); - } else { - throw new PGPException("no q value found"); - } - - PublicKeyPacket pubPacket = new PublicKeyPacket( - PublicKeyAlgorithmTags.ECDSA, new Date(), - new ECDSAPublicBCPGKey( - ECNamedCurveTable.getOID(curveName), - new BigInteger(1, qVal))); - - SXprUtils.skipCloseParenthesis(inputStream); - - BigInteger d = processECSecretKey(inputStream, curveID, - curveName, qVal, keyProtectionRemoverFactory); - - return new PGPSecretKey( - new SecretKeyPacket(pubPacket, - SymmetricKeyAlgorithmTags.NULL, null, null, - new ECSecretBCPGKey(d).getEncoded()), - new PGPPublicKey(pubPacket, fingerPrintCalculator)); - } else if (keyType.equals("dsa")) { - BigInteger p = readBigInteger("p", inputStream); - BigInteger q = readBigInteger("q", inputStream); - BigInteger g = readBigInteger("g", inputStream); - - BigInteger y = readBigInteger("y", inputStream); - - BigInteger x = processDSASecretKey(inputStream, p, q, g, y, - keyProtectionRemoverFactory); - - PublicKeyPacket pubPacket = new PublicKeyPacket( - PublicKeyAlgorithmTags.DSA, new Date(), - new DSAPublicBCPGKey(p, q, g, y)); - - return new PGPSecretKey( - new SecretKeyPacket(pubPacket, - SymmetricKeyAlgorithmTags.NULL, null, null, - new DSASecretBCPGKey(x).getEncoded()), - new PGPPublicKey(pubPacket, fingerPrintCalculator)); - } else if (keyType.equals("elg")) { - BigInteger p = readBigInteger("p", inputStream); - BigInteger g = readBigInteger("g", inputStream); - - BigInteger y = readBigInteger("y", inputStream); - - BigInteger x = processElGamalSecretKey(inputStream, p, g, y, - keyProtectionRemoverFactory); - - PublicKeyPacket pubPacket = new PublicKeyPacket( - PublicKeyAlgorithmTags.ELGAMAL_ENCRYPT, new Date(), - new ElGamalPublicBCPGKey(p, g, y)); - - return new PGPSecretKey( - new SecretKeyPacket(pubPacket, - SymmetricKeyAlgorithmTags.NULL, null, null, - new ElGamalSecretBCPGKey(x).getEncoded()), - new PGPPublicKey(pubPacket, fingerPrintCalculator)); - } else if (keyType.equals("rsa")) { - BigInteger n = readBigInteger("n", inputStream); - BigInteger e = readBigInteger("e", inputStream); - - BigInteger[] values = processRSASecretKey(inputStream, n, e, - keyProtectionRemoverFactory); - - // TODO: type of RSA key? - PublicKeyPacket pubPacket = new PublicKeyPacket( - PublicKeyAlgorithmTags.RSA_GENERAL, new Date(), - new RSAPublicBCPGKey(n, e)); - - return new PGPSecretKey( - new SecretKeyPacket(pubPacket, - SymmetricKeyAlgorithmTags.NULL, null, null, - new RSASecretBCPGKey(values[0], values[1], - values[2]).getEncoded()), - new PGPPublicKey(pubPacket, fingerPrintCalculator)); - } else { - throw new PGPException("unknown key type: " + keyType); - } - } - - throw new PGPException("unknown key type found"); - } - private BigInteger readBigInteger(String expectedType, InputStream inputStream) throws IOException, PGPException { SXprUtils.skipOpenParenthesis(inputStream); -- cgit v1.2.3 From b2accb0e9c07fa40fa9d7bf266a5763a1f63cc90 Mon Sep 17 00:00:00 2001 From: Thomas Wolf Date: Wed, 6 Nov 2024 19:14:47 +0100 Subject: GPG: use BC PGP secret key parsing out of the box Remove the custom S-expression parsing; BC has gotten many improvements in 1.79 regarding PGP ed25519 keys, AES/OCB encryption, and generally parsing key files. It now can do all we need. Change-Id: I392443e040cce150a9575d18795a7cb8195a3515 Signed-off-by: Thomas Wolf --- org.eclipse.jgit.gpg.bc.test/META-INF/MANIFEST.MF | 11 +- .../jgit/gpg/bc/internal/keys/SecretKeysTest.java | 62 +- org.eclipse.jgit.gpg.bc/META-INF/MANIFEST.MF | 37 +- org.eclipse.jgit.gpg.bc/about.html | 26 - .../eclipse/jgit/gpg/bc/internal/BCText.properties | 14 +- .../org/eclipse/jgit/gpg/bc/internal/BCText.java | 16 +- .../gpg/bc/internal/BouncyCastleGpgSigner.java | 3 +- .../keys/OCBPBEProtectionRemoverFactory.java | 132 ---- .../jgit/gpg/bc/internal/keys/SExprParser.java | 711 --------------------- .../jgit/gpg/bc/internal/keys/SXprUtils.java | 110 ---- .../jgit/gpg/bc/internal/keys/SecretKeys.java | 557 ++-------------- 11 files changed, 94 insertions(+), 1585 deletions(-) delete mode 100644 org.eclipse.jgit.gpg.bc/src/org/eclipse/jgit/gpg/bc/internal/keys/OCBPBEProtectionRemoverFactory.java delete mode 100644 org.eclipse.jgit.gpg.bc/src/org/eclipse/jgit/gpg/bc/internal/keys/SExprParser.java delete mode 100644 org.eclipse.jgit.gpg.bc/src/org/eclipse/jgit/gpg/bc/internal/keys/SXprUtils.java diff --git a/org.eclipse.jgit.gpg.bc.test/META-INF/MANIFEST.MF b/org.eclipse.jgit.gpg.bc.test/META-INF/MANIFEST.MF index c71f9b6541..a86f5ab2c6 100644 --- a/org.eclipse.jgit.gpg.bc.test/META-INF/MANIFEST.MF +++ b/org.eclipse.jgit.gpg.bc.test/META-INF/MANIFEST.MF @@ -8,11 +8,12 @@ Bundle-Vendor: %Bundle-Vendor Bundle-Localization: plugin Bundle-RequiredExecutionEnvironment: JavaSE-17 Require-Bundle: org.hamcrest.core;bundle-version="[1.3.0,2.0.0)" -Import-Package: org.bouncycastle.jce.provider;version="[1.65.0,2.0.0)", - org.bouncycastle.openpgp;version="[1.65.0,2.0.0)", - org.bouncycastle.openpgp.operator;version="[1.65.0,2.0.0)", - org.bouncycastle.openpgp.operator.jcajce;version="[1.65.0,2.0.0)", - org.bouncycastle.util.encoders;version="[1.65.0,2.0.0)", +Import-Package: org.bouncycastle.asn1.cryptlib;version="[1.79.0,2.0.0)", + org.bouncycastle.jce.provider;version="[1.79.0,2.0.0)", + org.bouncycastle.openpgp;version="[1.79.0,2.0.0)", + org.bouncycastle.openpgp.operator;version="[1.79.0,2.0.0)", + org.bouncycastle.openpgp.operator.jcajce;version="[1.79.0,2.0.0)", + org.bouncycastle.util.encoders;version="[1.79.0,2.0.0)", org.eclipse.jgit.gpg.bc.internal;version="[7.1.0,7.2.0)", org.eclipse.jgit.gpg.bc.internal.keys;version="[7.1.0,7.2.0)", org.eclipse.jgit.util.sha1;version="[7.1.0,7.2.0)", diff --git a/org.eclipse.jgit.gpg.bc.test/tst/org/eclipse/jgit/gpg/bc/internal/keys/SecretKeysTest.java b/org.eclipse.jgit.gpg.bc.test/tst/org/eclipse/jgit/gpg/bc/internal/keys/SecretKeysTest.java index fed06103b6..d486c977f0 100644 --- a/org.eclipse.jgit.gpg.bc.test/tst/org/eclipse/jgit/gpg/bc/internal/keys/SecretKeysTest.java +++ b/org.eclipse.jgit.gpg.bc.test/tst/org/eclipse/jgit/gpg/bc/internal/keys/SecretKeysTest.java @@ -1,5 +1,5 @@ /* - * Copyright (C) 2021 Thomas Wolf and others + * Copyright (C) 2021, 2024 Thomas Wolf and others * * This program and the accompanying materials are made available under the * terms of the Eclipse Distribution License v. 1.0 which is available at @@ -9,10 +9,7 @@ */ package org.eclipse.jgit.gpg.bc.internal.keys; -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotNull; -import static org.junit.Assert.assertTrue; import java.io.BufferedInputStream; import java.io.IOException; @@ -20,8 +17,6 @@ import java.io.InputStream; import java.security.Security; import java.util.Iterator; -import javax.crypto.Cipher; - import org.bouncycastle.jce.provider.BouncyCastleProvider; import org.bouncycastle.openpgp.PGPException; import org.bouncycastle.openpgp.PGPPublicKey; @@ -49,39 +44,15 @@ public class SecretKeysTest { } } - private static volatile Boolean haveOCB; - - private static boolean ocbAvailable() { - Boolean haveIt = haveOCB; - if (haveIt != null) { - return haveIt.booleanValue(); - } - try { - Cipher c = Cipher.getInstance("AES/OCB/NoPadding"); //$NON-NLS-1$ - if (c == null) { - haveOCB = Boolean.FALSE; - return false; - } - } catch (NoClassDefFoundError | Exception e) { - haveOCB = Boolean.FALSE; - return false; - } - haveOCB = Boolean.TRUE; - return true; - } - private static class TestData { final String name; final boolean encrypted; - final boolean keyValue; - - TestData(String name, boolean encrypted, boolean keyValue) { + TestData(String name, boolean encrypted) { this.name = name; this.encrypted = encrypted; - this.keyValue = keyValue; } @Override @@ -93,19 +64,12 @@ public class SecretKeysTest { @Parameters(name = "{0}") public static TestData[] initTestData() { return new TestData[] { - new TestData("AFDA8EA10E185ACF8C0D0F8885A0EF61A72ECB11", false, false), - new TestData("2FB05DBB70FC07CB84C13431F640CA6CEA1DBF8A", false, true), - new TestData("66CCECEC2AB46A9735B10FEC54EDF9FD0F77BAF9", true, true), - new TestData("F727FAB884DA3BD402B6E0F5472E108D21033124", true, true), - new TestData("62D43D7F117F7A5E4998ECB6617EE9942D069C14", true, true), - new TestData("faked", false, true) }; - } - - private static byte[] readTestKey(String filename) throws Exception { - try (InputStream in = new BufferedInputStream( - SecretKeysTest.class.getResourceAsStream(filename))) { - return SecretKeys.keyFromNameValueFormat(in); - } + new TestData("AFDA8EA10E185ACF8C0D0F8885A0EF61A72ECB11", false), + new TestData("2FB05DBB70FC07CB84C13431F640CA6CEA1DBF8A", false), + new TestData("66CCECEC2AB46A9735B10FEC54EDF9FD0F77BAF9", true), + new TestData("F727FAB884DA3BD402B6E0F5472E108D21033124", true), + new TestData("62D43D7F117F7A5E4998ECB6617EE9942D069C14", true), + new TestData("faked", false) }; } private static PGPPublicKey readAsc(InputStream in) @@ -131,11 +95,6 @@ public class SecretKeysTest { @Test public void testKeyRead() throws Exception { - if (data.keyValue) { - byte[] bytes = readTestKey(data.name + ".key"); - assertEquals('(', bytes[0]); - assertEquals(')', bytes[bytes.length - 1]); - } try (InputStream pubIn = this.getClass() .getResourceAsStream(data.name + ".asc")) { if (pubIn != null) { @@ -151,11 +110,6 @@ public class SecretKeysTest { : null, publicKey); assertNotNull(secretKey); - } catch (PGPException e) { - // Currently we may not be able to load OCB-encrypted keys. - assertTrue(e.toString(), e.getMessage().contains("OCB")); - assertTrue(data.encrypted); - assertFalse(ocbAvailable()); } } } diff --git a/org.eclipse.jgit.gpg.bc/META-INF/MANIFEST.MF b/org.eclipse.jgit.gpg.bc/META-INF/MANIFEST.MF index 6e3321ebf6..1413c44dcb 100644 --- a/org.eclipse.jgit.gpg.bc/META-INF/MANIFEST.MF +++ b/org.eclipse.jgit.gpg.bc/META-INF/MANIFEST.MF @@ -8,26 +8,23 @@ Bundle-Vendor: %Bundle-Vendor Bundle-Localization: OSGI-INF/l10n/gpg_bc Bundle-Version: 7.1.0.qualifier Bundle-RequiredExecutionEnvironment: JavaSE-17 -Import-Package: org.bouncycastle.asn1;version="[1.69.0,2.0.0)", - org.bouncycastle.asn1.x9;version="[1.69.0,2.0.0)", - org.bouncycastle.bcpg;version="[1.69.0,2.0.0)", - org.bouncycastle.bcpg.sig;version="[1.69.0,2.0.0)", - org.bouncycastle.crypto.ec;version="[1.69.0,2.0.0)", - org.bouncycastle.gpg;version="[1.69.0,2.0.0)", - org.bouncycastle.gpg.keybox;version="[1.69.0,2.0.0)", - org.bouncycastle.gpg.keybox.jcajce;version="[1.69.0,2.0.0)", - org.bouncycastle.jcajce.interfaces;version="[1.69.0,2.0.0)", - org.bouncycastle.jcajce.util;version="[1.69.0,2.0.0)", - org.bouncycastle.jce.provider;version="[1.69.0,2.0.0)", - org.bouncycastle.math.ec;version="[1.69.0,2.0.0)", - org.bouncycastle.math.field;version="[1.69.0,2.0.0)", - org.bouncycastle.openpgp;version="[1.69.0,2.0.0)", - org.bouncycastle.openpgp.jcajce;version="[1.69.0,2.0.0)", - org.bouncycastle.openpgp.operator;version="[1.69.0,2.0.0)", - org.bouncycastle.openpgp.operator.jcajce;version="[1.69.0,2.0.0)", - org.bouncycastle.util;version="[1.69.0,2.0.0)", - org.bouncycastle.util.encoders;version="[1.69.0,2.0.0)", - org.bouncycastle.util.io;version="[1.69.0,2.0.0)", +Import-Package: org.bouncycastle.asn1;version="[1.79.0,2.0.0)", + org.bouncycastle.asn1.x9;version="[1.79.0,2.0.0)", + org.bouncycastle.bcpg;version="[1.79.0,2.0.0)", + org.bouncycastle.bcpg.sig;version="[1.79.0,2.0.0)", + org.bouncycastle.crypto.ec;version="[1.79.0,2.0.0)", + org.bouncycastle.gpg;version="[1.79.0,2.0.0)", + org.bouncycastle.gpg.keybox;version="[1.79.0,2.0.0)", + org.bouncycastle.gpg.keybox.jcajce;version="[1.79.0,2.0.0)", + org.bouncycastle.jcajce.interfaces;version="[1.79.0,2.0.0)", + org.bouncycastle.jcajce.util;version="[1.79.0,2.0.0)", + org.bouncycastle.math.ec;version="[1.79.0,2.0.0)", + org.bouncycastle.math.field;version="[1.79.0,2.0.0)", + org.bouncycastle.openpgp;version="[1.79.0,2.0.0)", + org.bouncycastle.openpgp.jcajce;version="[1.79.0,2.0.0)", + org.bouncycastle.openpgp.operator;version="[1.79.0,2.0.0)", + org.bouncycastle.openpgp.operator.jcajce;version="[1.79.0,2.0.0)", + org.bouncycastle.util.encoders;version="[1.79.0,2.0.0)", org.slf4j;version="[1.7.0,3.0.0)" Export-Package: org.eclipse.jgit.gpg.bc.internal;version="7.1.0";x-friends:="org.eclipse.jgit.gpg.bc.test", org.eclipse.jgit.gpg.bc.internal.keys;version="7.1.0";x-friends:="org.eclipse.jgit.gpg.bc.test" diff --git a/org.eclipse.jgit.gpg.bc/about.html b/org.eclipse.jgit.gpg.bc/about.html index fc527d5a3a..92b9409831 100644 --- a/org.eclipse.jgit.gpg.bc/about.html +++ b/org.eclipse.jgit.gpg.bc/about.html @@ -58,32 +58,6 @@ WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWIS ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.

-
-

org.eclipse.jgit.gpg.bc.internal.keys.SExprParser - MIT

- -

Copyright (c) 2000-2021 The Legion of the Bouncy Castle Inc. -(https://www.bouncycastle.org)

- -

-Permission is hereby granted, free of charge, to any person obtaining a copy of this software -and associated documentation files (the "Software"), to deal in the Software without restriction, -including without limitation the rights to use, copy, modify, merge, publish, distribute, sublicense, -and/or sell copies of the Software, and to permit persons to whom the Software is furnished to do so, -subject to the following conditions: -

-

-The above copyright notice and this permission notice shall be included in all copies or substantial -portions of the Software. -

-

-THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLIED, -INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS FOR A PARTICULAR -PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE -LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR -OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER -DEALINGS IN THE SOFTWARE. -

- diff --git a/org.eclipse.jgit.gpg.bc/resources/org/eclipse/jgit/gpg/bc/internal/BCText.properties b/org.eclipse.jgit.gpg.bc/resources/org/eclipse/jgit/gpg/bc/internal/BCText.properties index 77ca2cd0a4..9e7f98cab1 100644 --- a/org.eclipse.jgit.gpg.bc/resources/org/eclipse/jgit/gpg/bc/internal/BCText.properties +++ b/org.eclipse.jgit.gpg.bc/resources/org/eclipse/jgit/gpg/bc/internal/BCText.properties @@ -1,7 +1,5 @@ corrupt25519Key=Ed25519/Curve25519 public key has wrong length: {0} credentialPassphrase=Passphrase -cryptCipherError=Cannot create cipher to decrypt: {0} -cryptWrongDecryptedLength=Decrypted key has wrong length; expected {0} bytes, got only {1} bytes gpgFailedToParseSecretKey=Failed to parse secret key file {0}. Is the entered passphrase correct? gpgNoCredentialsProvider=missing credentials provider gpgNoKeygrip=Cannot find key {0}: cannot determine key grip @@ -9,22 +7,14 @@ gpgNoKeyring=neither pubring.kbx nor secring.gpg files found gpgNoKeyInLegacySecring=no matching secret key found in legacy secring.gpg for key or user id: {0} gpgNoPublicKeyFound=Unable to find a public-key with key or user id: {0} gpgNoSecretKeyForPublicKey=unable to find associated secret key for public key: {0} -gpgNoSuchAlgorithm=Cannot decrypt encrypted secret key: encryption algorithm {0} is not available gpgNotASigningKey=Secret key ({0}) is not suitable for signing gpgKeyInfo=GPG Key (fingerprint {0}) gpgSigningCancelled=Signing was cancelled +keyAlgorithmMismatch=Secret key has a different algorithm than the public key +keyMismatch=Secret key does not match public key; public key is {0} {1} while secret key is for {2} {3} logWarnGnuPGHome=Cannot access GPG home directory given by environment variable GNUPGHOME={} logWarnGpgHomeProperty=Cannot access GPG home directory given by Java system property jgit.gpg.home={} nonSignatureError=Signature does not decode into a signature object -secretKeyTooShort=Secret key file corrupt; only {0} bytes read -sexprHexNotClosed=Hex number in s-expression not closed -sexprHexOdd=Hex number in s-expression has an odd number of digits -sexprStringInvalidEscape=Invalid escape {0} in s-expression -sexprStringInvalidEscapeAtEnd=Invalid s-expression: quoted string ends with escape character -sexprStringInvalidHexEscape=Invalid hex escape in s-expression -sexprStringInvalidOctalEscape=Invalid octal escape in s-expression -sexprStringNotClosed=String in s-expression not closed -sexprUnhandled=Unhandled token {0} in s-expression signatureInconsistent=Inconsistent signature; key ID {0} does not match issuer fingerprint {1} signatureKeyLookupError=Error occurred while looking for public key signatureNoKeyInfo=No way to determine a public key from the signature diff --git a/org.eclipse.jgit.gpg.bc/src/org/eclipse/jgit/gpg/bc/internal/BCText.java b/org.eclipse.jgit.gpg.bc/src/org/eclipse/jgit/gpg/bc/internal/BCText.java index 705e195e44..fcae7c2b98 100644 --- a/org.eclipse.jgit.gpg.bc/src/org/eclipse/jgit/gpg/bc/internal/BCText.java +++ b/org.eclipse.jgit.gpg.bc/src/org/eclipse/jgit/gpg/bc/internal/BCText.java @@ -1,5 +1,5 @@ /* - * Copyright (C) 2018, 2021 Salesforce and others + * Copyright (C) 2018, 2024 Salesforce and others * * This program and the accompanying materials are made available under the * terms of the Eclipse Distribution License v. 1.0 which is available at @@ -30,8 +30,6 @@ public final class BCText extends TranslationBundle { // @formatter:off /***/ public String corrupt25519Key; /***/ public String credentialPassphrase; - /***/ public String cryptCipherError; - /***/ public String cryptWrongDecryptedLength; /***/ public String gpgFailedToParseSecretKey; /***/ public String gpgNoCredentialsProvider; /***/ public String gpgNoKeygrip; @@ -39,22 +37,14 @@ public final class BCText extends TranslationBundle { /***/ public String gpgNoKeyInLegacySecring; /***/ public String gpgNoPublicKeyFound; /***/ public String gpgNoSecretKeyForPublicKey; - /***/ public String gpgNoSuchAlgorithm; /***/ public String gpgNotASigningKey; /***/ public String gpgKeyInfo; /***/ public String gpgSigningCancelled; + /***/ public String keyAlgorithmMismatch; + /***/ public String keyMismatch; /***/ public String logWarnGnuPGHome; /***/ public String logWarnGpgHomeProperty; /***/ public String nonSignatureError; - /***/ public String secretKeyTooShort; - /***/ public String sexprHexNotClosed; - /***/ public String sexprHexOdd; - /***/ public String sexprStringInvalidEscape; - /***/ public String sexprStringInvalidEscapeAtEnd; - /***/ public String sexprStringInvalidHexEscape; - /***/ public String sexprStringInvalidOctalEscape; - /***/ public String sexprStringNotClosed; - /***/ public String sexprUnhandled; /***/ public String signatureInconsistent; /***/ public String signatureKeyLookupError; /***/ public String signatureNoKeyInfo; diff --git a/org.eclipse.jgit.gpg.bc/src/org/eclipse/jgit/gpg/bc/internal/BouncyCastleGpgSigner.java b/org.eclipse.jgit.gpg.bc/src/org/eclipse/jgit/gpg/bc/internal/BouncyCastleGpgSigner.java index 1d187a5db2..adac9b199d 100644 --- a/org.eclipse.jgit.gpg.bc/src/org/eclipse/jgit/gpg/bc/internal/BouncyCastleGpgSigner.java +++ b/org.eclipse.jgit.gpg.bc/src/org/eclipse/jgit/gpg/bc/internal/BouncyCastleGpgSigner.java @@ -105,7 +105,8 @@ public class BouncyCastleGpgSigner implements Signer { PGPSignatureGenerator signatureGenerator = new PGPSignatureGenerator( new JcaPGPContentSignerBuilder( publicKey.getAlgorithm(), - HashAlgorithmTags.SHA256)); + HashAlgorithmTags.SHA256), + publicKey); signatureGenerator.init(PGPSignature.BINARY_DOCUMENT, privateKey); PGPSignatureSubpacketGenerator subpackets = new PGPSignatureSubpacketGenerator(); subpackets.setIssuerFingerprint(false, publicKey); diff --git a/org.eclipse.jgit.gpg.bc/src/org/eclipse/jgit/gpg/bc/internal/keys/OCBPBEProtectionRemoverFactory.java b/org.eclipse.jgit.gpg.bc/src/org/eclipse/jgit/gpg/bc/internal/keys/OCBPBEProtectionRemoverFactory.java deleted file mode 100644 index e6f1decbc1..0000000000 --- a/org.eclipse.jgit.gpg.bc/src/org/eclipse/jgit/gpg/bc/internal/keys/OCBPBEProtectionRemoverFactory.java +++ /dev/null @@ -1,132 +0,0 @@ -/* - * Copyright (C) 2021 Thomas Wolf and others - * - * This program and the accompanying materials are made available under the - * terms of the Eclipse Distribution License v. 1.0 which is available at - * https://www.eclipse.org/org/documents/edl-v10.php. - * - * SPDX-License-Identifier: BSD-3-Clause - */ -package org.eclipse.jgit.gpg.bc.internal.keys; - -import java.security.NoSuchAlgorithmException; -import java.text.MessageFormat; - -import javax.crypto.Cipher; -import javax.crypto.SecretKey; -import javax.crypto.spec.IvParameterSpec; -import javax.crypto.spec.SecretKeySpec; - -import org.bouncycastle.openpgp.PGPException; -import org.bouncycastle.openpgp.PGPUtil; -import org.bouncycastle.openpgp.operator.PBEProtectionRemoverFactory; -import org.bouncycastle.openpgp.operator.PBESecretKeyDecryptor; -import org.bouncycastle.openpgp.operator.PGPDigestCalculatorProvider; -import org.bouncycastle.util.Arrays; -import org.eclipse.jgit.gpg.bc.internal.BCText; - -/** - * A {@link PBEProtectionRemoverFactory} using AES/OCB/NoPadding for decryption. - * It accepts an AAD in the factory's constructor, so the factory can be used to - * create a {@link PBESecretKeyDecryptor} only for a particular input. - *

- * For JGit's needs, this is sufficient, but for a general upstream - * implementation that limitation might not be acceptable. - *

- */ -class OCBPBEProtectionRemoverFactory - implements PBEProtectionRemoverFactory { - - private final PGPDigestCalculatorProvider calculatorProvider; - - private final char[] passphrase; - - private final byte[] aad; - - /** - * Creates a new factory instance with the given parameters. - *

- * Because the AAD is given at factory level, the {@link PBESecretKeyDecryptor}s - * created by the factory can be used to decrypt only a particular input - * matching this AAD. - *

- * - * @param passphrase to use for secret key derivation - * @param calculatorProvider for computing digests - * @param aad for the OCB decryption - */ - OCBPBEProtectionRemoverFactory(char[] passphrase, - PGPDigestCalculatorProvider calculatorProvider, byte[] aad) { - this.calculatorProvider = calculatorProvider; - this.passphrase = passphrase; - this.aad = aad; - } - - @Override - public PBESecretKeyDecryptor createDecryptor(String protection) - throws PGPException { - return new PBESecretKeyDecryptor(passphrase, calculatorProvider) { - - @Override - public byte[] recoverKeyData(int encAlgorithm, byte[] key, - byte[] iv, byte[] encrypted, int encryptedOffset, - int encryptedLength) throws PGPException { - String algorithmName = PGPUtil - .getSymmetricCipherName(encAlgorithm); - byte[] decrypted = null; - try { - // errorprone: "Dynamically constructed transformation - // strings are also flagged, as they may conceal an instance - // of ECB mode." - @SuppressWarnings("InsecureCryptoUsage") - Cipher c = Cipher - .getInstance(algorithmName + "/OCB/NoPadding"); //$NON-NLS-1$ - SecretKey secretKey = new SecretKeySpec(key, algorithmName); - c.init(Cipher.DECRYPT_MODE, secretKey, - new IvParameterSpec(iv)); - c.updateAAD(aad); - decrypted = new byte[c.getOutputSize(encryptedLength)]; - int decryptedLength = c.update(encrypted, encryptedOffset, - encryptedLength, decrypted); - // doFinal() for OCB will check the MAC and throw an - // exception if it doesn't match - decryptedLength += c.doFinal(decrypted, decryptedLength); - if (decryptedLength != decrypted.length) { - throw new PGPException(MessageFormat.format( - BCText.get().cryptWrongDecryptedLength, - Integer.valueOf(decryptedLength), - Integer.valueOf(decrypted.length))); - } - byte[] result = decrypted; - decrypted = null; // Don't clear in finally - return result; - } catch (NoClassDefFoundError e) { - String msg = MessageFormat.format( - BCText.get().gpgNoSuchAlgorithm, - algorithmName + "/OCB"); //$NON-NLS-1$ - throw new PGPException(msg, - new NoSuchAlgorithmException(msg, e)); - } catch (PGPException e) { - throw e; - } catch (Exception e) { - throw new PGPException( - MessageFormat.format(BCText.get().cryptCipherError, - e.getLocalizedMessage()), - e); - } finally { - if (decrypted != null) { - // Prevent halfway decrypted data leaking. - Arrays.fill(decrypted, (byte) 0); - } - } - } - - @Override - public byte[] recoverKeyData(int encAlgorithm, int aeadAlgorithm, - byte[] s2kKey, byte[] iv, int packetTag, int keyVersion, - byte[] keyData, byte[] pubkeyData) throws PGPException { - throw new UnsupportedOperationException(); - } - }; - } -} \ No newline at end of file diff --git a/org.eclipse.jgit.gpg.bc/src/org/eclipse/jgit/gpg/bc/internal/keys/SExprParser.java b/org.eclipse.jgit.gpg.bc/src/org/eclipse/jgit/gpg/bc/internal/keys/SExprParser.java deleted file mode 100644 index b062eadb43..0000000000 --- a/org.eclipse.jgit.gpg.bc/src/org/eclipse/jgit/gpg/bc/internal/keys/SExprParser.java +++ /dev/null @@ -1,711 +0,0 @@ -/* - * Copyright (c) 2000-2021 The Legion of the Bouncy Castle Inc. (https://www.bouncycastle.org) - *

- * Permission is hereby granted, free of charge, to any person obtaining a copy of this software - * and associated documentation files (the "Software"), to deal in the Software without restriction, - *including without limitation the rights to use, copy, modify, merge, publish, distribute, sublicense, - * and/or sell copies of the Software, and to permit persons to whom the Software is furnished to do so, - * subject to the following conditions: - *

- *

- * The above copyright notice and this permission notice shall be included in all copies or substantial - * portions of the Software. - *

- *

- * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLIED, - * INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS FOR A PARTICULAR - * PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE - * LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR - * OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER - * DEALINGS IN THE SOFTWARE. - *

- */ -package org.eclipse.jgit.gpg.bc.internal.keys; - -import java.io.ByteArrayInputStream; -import java.io.ByteArrayOutputStream; -import java.io.IOException; -import java.io.InputStream; -import java.io.OutputStream; -import java.math.BigInteger; - -import org.bouncycastle.asn1.ASN1ObjectIdentifier; -import org.bouncycastle.asn1.x9.ECNamedCurveTable; -import org.bouncycastle.bcpg.DSAPublicBCPGKey; -import org.bouncycastle.bcpg.DSASecretBCPGKey; -import org.bouncycastle.bcpg.ECDSAPublicBCPGKey; -import org.bouncycastle.bcpg.ECPublicBCPGKey; -import org.bouncycastle.bcpg.ECSecretBCPGKey; -import org.bouncycastle.bcpg.ElGamalPublicBCPGKey; -import org.bouncycastle.bcpg.ElGamalSecretBCPGKey; -import org.bouncycastle.bcpg.HashAlgorithmTags; -import org.bouncycastle.bcpg.RSAPublicBCPGKey; -import org.bouncycastle.bcpg.RSASecretBCPGKey; -import org.bouncycastle.bcpg.S2K; -import org.bouncycastle.bcpg.SecretKeyPacket; -import org.bouncycastle.bcpg.SymmetricKeyAlgorithmTags; -import org.bouncycastle.openpgp.PGPException; -import org.bouncycastle.openpgp.PGPPublicKey; -import org.bouncycastle.openpgp.PGPSecretKey; -import org.bouncycastle.openpgp.operator.PBEProtectionRemoverFactory; -import org.bouncycastle.openpgp.operator.PBESecretKeyDecryptor; -import org.bouncycastle.openpgp.operator.PGPDigestCalculator; -import org.bouncycastle.openpgp.operator.PGPDigestCalculatorProvider; -import org.bouncycastle.util.Arrays; -import org.bouncycastle.util.Strings; - -/** - * A parser for secret keys stored in s-expressions. Original BouncyCastle code - * modified by the JGit team to: - *
    - *
  • handle unencrypted DSA, EC, and ElGamal keys (upstream only handles - * unencrypted RSA)
    • - *
  • handle secret keys using AES/OCB as encryption (those don't have a - * hash)
    • - *
  • fix EC parsing to account for "flags" sub-list present for ed25519 and - * curve25519
    • - *
  • add support for ed25519 OIDs unknown to BouncyCastle
    • - *
- */ -@SuppressWarnings("nls") -public class SExprParser { - private final PGPDigestCalculatorProvider digestProvider; - - /** - * Base constructor. - * - * @param digestProvider - * a provider for digest calculations. Used to confirm key - * protection hashes. - */ - public SExprParser(PGPDigestCalculatorProvider digestProvider) { - this.digestProvider = digestProvider; - } - - /** - * Parse a secret key from one of the GPG S expression keys associating it - * with the passed in public key. - * - * @param inputStream - * to read from - * @param keyProtectionRemoverFactory - * for decrypting encrypted keys - * @param pubKey - * the private key should belong to - * - * @return a secret key object. - * @throws IOException - * if an IO error occurred - * @throws PGPException - * if some PGP error occurred - */ - public PGPSecretKey parseSecretKey(InputStream inputStream, - PBEProtectionRemoverFactory keyProtectionRemoverFactory, - PGPPublicKey pubKey) throws IOException, PGPException { - SXprUtils.skipOpenParenthesis(inputStream); - - String type; - - type = SXprUtils.readString(inputStream, inputStream.read()); - if (type.equals("protected-private-key") - || type.equals("private-key")) { - SXprUtils.skipOpenParenthesis(inputStream); - - String keyType = SXprUtils.readString(inputStream, - inputStream.read()); - if (keyType.equals("ecc")) { - SXprUtils.skipOpenParenthesis(inputStream); - - String curveID = SXprUtils.readString(inputStream, - inputStream.read()); - String curveName = SXprUtils.readString(inputStream, - inputStream.read()); - - SXprUtils.skipCloseParenthesis(inputStream); - - byte[] qVal; - - SXprUtils.skipOpenParenthesis(inputStream); - - type = SXprUtils.readString(inputStream, inputStream.read()); - // JGit: c.f. https://github.com/bcgit/bc-java/issues/1590. - // There may be a flags sub-list here for ed25519 or curve25519. - if (type.equals("flags")) { - SXprUtils.readString(inputStream, inputStream.read()); - SXprUtils.skipCloseParenthesis(inputStream); - SXprUtils.skipOpenParenthesis(inputStream); - type = SXprUtils.readString(inputStream, - inputStream.read()); - } - if (type.equals("q")) { - qVal = SXprUtils.readBytes(inputStream, inputStream.read()); - } else { - throw new PGPException("no q value found"); - } - - SXprUtils.skipCloseParenthesis(inputStream); - - BigInteger d = processECSecretKey(inputStream, curveID, - curveName, qVal, keyProtectionRemoverFactory); - - if (curveName.startsWith("NIST ")) { - curveName = curveName.substring("NIST ".length()); - } - - // JGit: BC doesn't know Ed25519 curve name. - ASN1ObjectIdentifier curveOid = ECNamedCurveTable - .getOID(curveName); - if (curveOid == null) { - curveOid = ObjectIds.getByName(curveName); - } - ECPublicBCPGKey basePubKey = new ECDSAPublicBCPGKey( - curveOid, - new BigInteger(1, qVal)); - ECPublicBCPGKey assocPubKey = (ECPublicBCPGKey) pubKey - .getPublicKeyPacket().getKey(); - if (!ObjectIds.match(basePubKey.getCurveOID(), - assocPubKey.getCurveOID()) - || !basePubKey.getEncodedPoint() - .equals(assocPubKey.getEncodedPoint())) { - throw new PGPException( - "passed in public key does not match secret key"); - } - - return new PGPSecretKey( - new SecretKeyPacket(pubKey.getPublicKeyPacket(), - SymmetricKeyAlgorithmTags.NULL, null, null, - new ECSecretBCPGKey(d).getEncoded()), - pubKey); - } else if (keyType.equals("dsa")) { - BigInteger p = readBigInteger("p", inputStream); - BigInteger q = readBigInteger("q", inputStream); - BigInteger g = readBigInteger("g", inputStream); - - BigInteger y = readBigInteger("y", inputStream); - - BigInteger x = processDSASecretKey(inputStream, p, q, g, y, - keyProtectionRemoverFactory); - - DSAPublicBCPGKey basePubKey = new DSAPublicBCPGKey(p, q, g, y); - DSAPublicBCPGKey assocPubKey = (DSAPublicBCPGKey) pubKey - .getPublicKeyPacket().getKey(); - if (!basePubKey.getP().equals(assocPubKey.getP()) - || !basePubKey.getQ().equals(assocPubKey.getQ()) - || !basePubKey.getG().equals(assocPubKey.getG()) - || !basePubKey.getY().equals(assocPubKey.getY())) { - throw new PGPException( - "passed in public key does not match secret key"); - } - return new PGPSecretKey( - new SecretKeyPacket(pubKey.getPublicKeyPacket(), - SymmetricKeyAlgorithmTags.NULL, null, null, - new DSASecretBCPGKey(x).getEncoded()), - pubKey); - } else if (keyType.equals("elg")) { - BigInteger p = readBigInteger("p", inputStream); - BigInteger g = readBigInteger("g", inputStream); - - BigInteger y = readBigInteger("y", inputStream); - - BigInteger x = processElGamalSecretKey(inputStream, p, g, y, - keyProtectionRemoverFactory); - - ElGamalPublicBCPGKey basePubKey = new ElGamalPublicBCPGKey(p, g, - y); - ElGamalPublicBCPGKey assocPubKey = (ElGamalPublicBCPGKey) pubKey - .getPublicKeyPacket().getKey(); - if (!basePubKey.getP().equals(assocPubKey.getP()) - || !basePubKey.getG().equals(assocPubKey.getG()) - || !basePubKey.getY().equals(assocPubKey.getY())) { - throw new PGPException( - "passed in public key does not match secret key"); - } - - return new PGPSecretKey( - new SecretKeyPacket(pubKey.getPublicKeyPacket(), - SymmetricKeyAlgorithmTags.NULL, null, null, - new ElGamalSecretBCPGKey(x).getEncoded()), - pubKey); - } else if (keyType.equals("rsa")) { - BigInteger n = readBigInteger("n", inputStream); - BigInteger e = readBigInteger("e", inputStream); - - BigInteger[] values = processRSASecretKey(inputStream, n, e, - keyProtectionRemoverFactory); - - // TODO: type of RSA key? - RSAPublicBCPGKey basePubKey = new RSAPublicBCPGKey(n, e); - RSAPublicBCPGKey assocPubKey = (RSAPublicBCPGKey) pubKey - .getPublicKeyPacket().getKey(); - if (!basePubKey.getModulus().equals(assocPubKey.getModulus()) - || !basePubKey.getPublicExponent() - .equals(assocPubKey.getPublicExponent())) { - throw new PGPException( - "passed in public key does not match secret key"); - } - - return new PGPSecretKey(new SecretKeyPacket( - pubKey.getPublicKeyPacket(), - SymmetricKeyAlgorithmTags.NULL, null, null, - new RSASecretBCPGKey(values[0], values[1], values[2]) - .getEncoded()), - pubKey); - } else { - throw new PGPException("unknown key type: " + keyType); - } - } - - throw new PGPException("unknown key type found"); - } - - private BigInteger readBigInteger(String expectedType, - InputStream inputStream) throws IOException, PGPException { - SXprUtils.skipOpenParenthesis(inputStream); - - String type = SXprUtils.readString(inputStream, inputStream.read()); - if (!type.equals(expectedType)) { - throw new PGPException(expectedType + " value expected"); - } - - byte[] nBytes = SXprUtils.readBytes(inputStream, inputStream.read()); - BigInteger v = new BigInteger(1, nBytes); - - SXprUtils.skipCloseParenthesis(inputStream); - - return v; - } - - private static byte[][] extractData(InputStream inputStream, - PBEProtectionRemoverFactory keyProtectionRemoverFactory) - throws PGPException, IOException { - byte[] data; - byte[] protectedAt = null; - - SXprUtils.skipOpenParenthesis(inputStream); - - String type = SXprUtils.readString(inputStream, inputStream.read()); - if (type.equals("protected")) { - String protection = SXprUtils.readString(inputStream, - inputStream.read()); - - SXprUtils.skipOpenParenthesis(inputStream); - - S2K s2k = SXprUtils.parseS2K(inputStream); - - byte[] iv = SXprUtils.readBytes(inputStream, inputStream.read()); - - SXprUtils.skipCloseParenthesis(inputStream); - - byte[] secKeyData = SXprUtils.readBytes(inputStream, - inputStream.read()); - - SXprUtils.skipCloseParenthesis(inputStream); - - PBESecretKeyDecryptor keyDecryptor = keyProtectionRemoverFactory - .createDecryptor(protection); - - // TODO: recognise other algorithms - byte[] key = keyDecryptor.makeKeyFromPassPhrase( - SymmetricKeyAlgorithmTags.AES_128, s2k); - - data = keyDecryptor.recoverKeyData( - SymmetricKeyAlgorithmTags.AES_128, key, iv, secKeyData, 0, - secKeyData.length); - - // check if protected at is present - if (inputStream.read() == '(') { - ByteArrayOutputStream bOut = new ByteArrayOutputStream(); - - bOut.write('('); - int ch; - while ((ch = inputStream.read()) >= 0 && ch != ')') { - bOut.write(ch); - } - - if (ch != ')') { - throw new IOException("unexpected end to SExpr"); - } - - bOut.write(')'); - - protectedAt = bOut.toByteArray(); - } - - SXprUtils.skipCloseParenthesis(inputStream); - SXprUtils.skipCloseParenthesis(inputStream); - } else if (type.equals("d") || type.equals("x")) { - // JGit modification: unencrypted DSA or ECC keys can have an "x" - // here - return null; - } else { - throw new PGPException("protected block not found"); - } - - return new byte[][] { data, protectedAt }; - } - - private BigInteger processDSASecretKey(InputStream inputStream, - BigInteger p, BigInteger q, BigInteger g, BigInteger y, - PBEProtectionRemoverFactory keyProtectionRemoverFactory) - throws IOException, PGPException { - String type; - byte[][] basicData = extractData(inputStream, - keyProtectionRemoverFactory); - - // JGit modification: handle unencrypted DSA keys - if (basicData == null) { - byte[] nBytes = SXprUtils.readBytes(inputStream, - inputStream.read()); - BigInteger x = new BigInteger(1, nBytes); - SXprUtils.skipCloseParenthesis(inputStream); - return x; - } - - byte[] keyData = basicData[0]; - byte[] protectedAt = basicData[1]; - - // - // parse the secret key S-expr - // - InputStream keyIn = new ByteArrayInputStream(keyData); - - SXprUtils.skipOpenParenthesis(keyIn); - SXprUtils.skipOpenParenthesis(keyIn); - - BigInteger x = readBigInteger("x", keyIn); - - SXprUtils.skipCloseParenthesis(keyIn); - - // JGit modification: OCB-encrypted keys don't have and don't need a - // hash - if (keyProtectionRemoverFactory instanceof OCBPBEProtectionRemoverFactory) { - return x; - } - - SXprUtils.skipOpenParenthesis(keyIn); - type = SXprUtils.readString(keyIn, keyIn.read()); - - if (!type.equals("hash")) { - throw new PGPException("hash keyword expected"); - } - type = SXprUtils.readString(keyIn, keyIn.read()); - - if (!type.equals("sha1")) { - throw new PGPException("hash keyword expected"); - } - - byte[] hashBytes = SXprUtils.readBytes(keyIn, keyIn.read()); - - SXprUtils.skipCloseParenthesis(keyIn); - - if (digestProvider != null) { - PGPDigestCalculator digestCalculator = digestProvider - .get(HashAlgorithmTags.SHA1); - - OutputStream dOut = digestCalculator.getOutputStream(); - - dOut.write(Strings.toByteArray("(3:dsa")); - writeCanonical(dOut, "p", p); - writeCanonical(dOut, "q", q); - writeCanonical(dOut, "g", g); - writeCanonical(dOut, "y", y); - writeCanonical(dOut, "x", x); - - // check protected-at - if (protectedAt != null) { - dOut.write(protectedAt); - } - - dOut.write(Strings.toByteArray(")")); - - byte[] check = digestCalculator.getDigest(); - if (!Arrays.constantTimeAreEqual(check, hashBytes)) { - throw new PGPException( - "checksum on protected data failed in SExpr"); - } - } - - return x; - } - - private BigInteger processElGamalSecretKey(InputStream inputStream, - BigInteger p, BigInteger g, BigInteger y, - PBEProtectionRemoverFactory keyProtectionRemoverFactory) - throws IOException, PGPException { - String type; - byte[][] basicData = extractData(inputStream, - keyProtectionRemoverFactory); - - // JGit modification: handle unencrypted EC keys - if (basicData == null) { - byte[] nBytes = SXprUtils.readBytes(inputStream, - inputStream.read()); - BigInteger x = new BigInteger(1, nBytes); - SXprUtils.skipCloseParenthesis(inputStream); - return x; - } - - byte[] keyData = basicData[0]; - byte[] protectedAt = basicData[1]; - - // - // parse the secret key S-expr - // - InputStream keyIn = new ByteArrayInputStream(keyData); - - SXprUtils.skipOpenParenthesis(keyIn); - SXprUtils.skipOpenParenthesis(keyIn); - - BigInteger x = readBigInteger("x", keyIn); - - SXprUtils.skipCloseParenthesis(keyIn); - - // JGit modification: OCB-encrypted keys don't have and don't need a - // hash - if (keyProtectionRemoverFactory instanceof OCBPBEProtectionRemoverFactory) { - return x; - } - - SXprUtils.skipOpenParenthesis(keyIn); - type = SXprUtils.readString(keyIn, keyIn.read()); - - if (!type.equals("hash")) { - throw new PGPException("hash keyword expected"); - } - type = SXprUtils.readString(keyIn, keyIn.read()); - - if (!type.equals("sha1")) { - throw new PGPException("hash keyword expected"); - } - - byte[] hashBytes = SXprUtils.readBytes(keyIn, keyIn.read()); - - SXprUtils.skipCloseParenthesis(keyIn); - - if (digestProvider != null) { - PGPDigestCalculator digestCalculator = digestProvider - .get(HashAlgorithmTags.SHA1); - - OutputStream dOut = digestCalculator.getOutputStream(); - - dOut.write(Strings.toByteArray("(3:elg")); - writeCanonical(dOut, "p", p); - writeCanonical(dOut, "g", g); - writeCanonical(dOut, "y", y); - writeCanonical(dOut, "x", x); - - // check protected-at - if (protectedAt != null) { - dOut.write(protectedAt); - } - - dOut.write(Strings.toByteArray(")")); - - byte[] check = digestCalculator.getDigest(); - if (!Arrays.constantTimeAreEqual(check, hashBytes)) { - throw new PGPException( - "checksum on protected data failed in SExpr"); - } - } - - return x; - } - - private BigInteger processECSecretKey(InputStream inputStream, - String curveID, String curveName, byte[] qVal, - PBEProtectionRemoverFactory keyProtectionRemoverFactory) - throws IOException, PGPException { - String type; - - byte[][] basicData = extractData(inputStream, - keyProtectionRemoverFactory); - - // JGit modification: handle unencrypted EC keys - if (basicData == null) { - byte[] nBytes = SXprUtils.readBytes(inputStream, - inputStream.read()); - BigInteger d = new BigInteger(1, nBytes); - SXprUtils.skipCloseParenthesis(inputStream); - return d; - } - - byte[] keyData = basicData[0]; - byte[] protectedAt = basicData[1]; - - // - // parse the secret key S-expr - // - InputStream keyIn = new ByteArrayInputStream(keyData); - - SXprUtils.skipOpenParenthesis(keyIn); - SXprUtils.skipOpenParenthesis(keyIn); - BigInteger d = readBigInteger("d", keyIn); - SXprUtils.skipCloseParenthesis(keyIn); - - // JGit modification: OCB-encrypted keys don't have and don't need a - // hash - if (keyProtectionRemoverFactory instanceof OCBPBEProtectionRemoverFactory) { - return d; - } - - SXprUtils.skipOpenParenthesis(keyIn); - - type = SXprUtils.readString(keyIn, keyIn.read()); - - if (!type.equals("hash")) { - throw new PGPException("hash keyword expected"); - } - type = SXprUtils.readString(keyIn, keyIn.read()); - - if (!type.equals("sha1")) { - throw new PGPException("hash keyword expected"); - } - - byte[] hashBytes = SXprUtils.readBytes(keyIn, keyIn.read()); - - SXprUtils.skipCloseParenthesis(keyIn); - - if (digestProvider != null) { - PGPDigestCalculator digestCalculator = digestProvider - .get(HashAlgorithmTags.SHA1); - - OutputStream dOut = digestCalculator.getOutputStream(); - - dOut.write(Strings.toByteArray("(3:ecc")); - - dOut.write(Strings.toByteArray("(" + curveID.length() + ":" - + curveID + curveName.length() + ":" + curveName + ")")); - - writeCanonical(dOut, "q", qVal); - writeCanonical(dOut, "d", d); - - // check protected-at - if (protectedAt != null) { - dOut.write(protectedAt); - } - - dOut.write(Strings.toByteArray(")")); - - byte[] check = digestCalculator.getDigest(); - - if (!Arrays.constantTimeAreEqual(check, hashBytes)) { - throw new PGPException( - "checksum on protected data failed in SExpr"); - } - } - - return d; - } - - private BigInteger[] processRSASecretKey(InputStream inputStream, - BigInteger n, BigInteger e, - PBEProtectionRemoverFactory keyProtectionRemoverFactory) - throws IOException, PGPException { - String type; - byte[][] basicData = extractData(inputStream, - keyProtectionRemoverFactory); - - byte[] keyData; - byte[] protectedAt = null; - - InputStream keyIn; - BigInteger d; - - if (basicData == null) { - keyIn = inputStream; - byte[] nBytes = SXprUtils.readBytes(inputStream, - inputStream.read()); - d = new BigInteger(1, nBytes); - - SXprUtils.skipCloseParenthesis(inputStream); - - } else { - keyData = basicData[0]; - protectedAt = basicData[1]; - - keyIn = new ByteArrayInputStream(keyData); - - SXprUtils.skipOpenParenthesis(keyIn); - SXprUtils.skipOpenParenthesis(keyIn); - d = readBigInteger("d", keyIn); - } - - // - // parse the secret key S-expr - // - - BigInteger p = readBigInteger("p", keyIn); - BigInteger q = readBigInteger("q", keyIn); - BigInteger u = readBigInteger("u", keyIn); - - // JGit modification: OCB-encrypted keys don't have and don't need a - // hash - if (basicData == null - || keyProtectionRemoverFactory instanceof OCBPBEProtectionRemoverFactory) { - return new BigInteger[] { d, p, q, u }; - } - - SXprUtils.skipCloseParenthesis(keyIn); - - SXprUtils.skipOpenParenthesis(keyIn); - type = SXprUtils.readString(keyIn, keyIn.read()); - - if (!type.equals("hash")) { - throw new PGPException("hash keyword expected"); - } - type = SXprUtils.readString(keyIn, keyIn.read()); - - if (!type.equals("sha1")) { - throw new PGPException("hash keyword expected"); - } - - byte[] hashBytes = SXprUtils.readBytes(keyIn, keyIn.read()); - - SXprUtils.skipCloseParenthesis(keyIn); - - if (digestProvider != null) { - PGPDigestCalculator digestCalculator = digestProvider - .get(HashAlgorithmTags.SHA1); - - OutputStream dOut = digestCalculator.getOutputStream(); - - dOut.write(Strings.toByteArray("(3:rsa")); - - writeCanonical(dOut, "n", n); - writeCanonical(dOut, "e", e); - writeCanonical(dOut, "d", d); - writeCanonical(dOut, "p", p); - writeCanonical(dOut, "q", q); - writeCanonical(dOut, "u", u); - - // check protected-at - if (protectedAt != null) { - dOut.write(protectedAt); - } - - dOut.write(Strings.toByteArray(")")); - - byte[] check = digestCalculator.getDigest(); - - if (!Arrays.constantTimeAreEqual(check, hashBytes)) { - throw new PGPException( - "checksum on protected data failed in SExpr"); - } - } - - return new BigInteger[] { d, p, q, u }; - } - - private void writeCanonical(OutputStream dOut, String label, BigInteger i) - throws IOException { - writeCanonical(dOut, label, i.toByteArray()); - } - - private void writeCanonical(OutputStream dOut, String label, byte[] data) - throws IOException { - dOut.write(Strings.toByteArray( - "(" + label.length() + ":" + label + data.length + ":")); - dOut.write(data); - dOut.write(Strings.toByteArray(")")); - } -} diff --git a/org.eclipse.jgit.gpg.bc/src/org/eclipse/jgit/gpg/bc/internal/keys/SXprUtils.java b/org.eclipse.jgit.gpg.bc/src/org/eclipse/jgit/gpg/bc/internal/keys/SXprUtils.java deleted file mode 100644 index 220aa285ff..0000000000 --- a/org.eclipse.jgit.gpg.bc/src/org/eclipse/jgit/gpg/bc/internal/keys/SXprUtils.java +++ /dev/null @@ -1,110 +0,0 @@ -/* - * Copyright (c) 2000-2021 The Legion of the Bouncy Castle Inc. (https://www.bouncycastle.org) - *

- * Permission is hereby granted, free of charge, to any person obtaining a copy of this software - * and associated documentation files (the "Software"), to deal in the Software without restriction, - *including without limitation the rights to use, copy, modify, merge, publish, distribute, sublicense, - * and/or sell copies of the Software, and to permit persons to whom the Software is furnished to do so, - * subject to the following conditions: - *

- *

- * The above copyright notice and this permission notice shall be included in all copies or substantial - * portions of the Software. - *

- *

- * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLIED, - * INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS FOR A PARTICULAR - * PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE - * LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR - * OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER - * DEALINGS IN THE SOFTWARE. - *

- */ -package org.eclipse.jgit.gpg.bc.internal.keys; - -// This class is an unmodified copy from Bouncy Castle; needed because it's package-visible only and used by SExprParser. - -import java.io.IOException; -import java.io.InputStream; - -import org.bouncycastle.bcpg.HashAlgorithmTags; -import org.bouncycastle.bcpg.S2K; -import org.bouncycastle.util.io.Streams; - -/** - * Utility functions for looking a S-expression keys. This class will move when - * it finds a better home! - *

- * Format documented here: - * http://git.gnupg.org/cgi-bin/gitweb.cgi?p=gnupg.git;a=blob;f=agent/keyformat.txt;h=42c4b1f06faf1bbe71ffadc2fee0fad6bec91a97;hb=refs/heads/master - *

- */ -class SXprUtils { - private static int readLength(InputStream in, int ch) throws IOException { - int len = ch - '0'; - - while ((ch = in.read()) >= 0 && ch != ':') { - len = len * 10 + ch - '0'; - } - - return len; - } - - static String readString(InputStream in, int ch) throws IOException { - int len = readLength(in, ch); - - char[] chars = new char[len]; - - for (int i = 0; i != chars.length; i++) { - chars[i] = (char) in.read(); - } - - return new String(chars); - } - - static byte[] readBytes(InputStream in, int ch) throws IOException { - int len = readLength(in, ch); - - byte[] data = new byte[len]; - - Streams.readFully(in, data); - - return data; - } - - static S2K parseS2K(InputStream in) throws IOException { - skipOpenParenthesis(in); - - // Algorithm is hard-coded to SHA1 below anyway. - readString(in, in.read()); - byte[] iv = readBytes(in, in.read()); - final long iterationCount = Long.parseLong(readString(in, in.read())); - - skipCloseParenthesis(in); - - // we have to return the actual iteration count provided. - S2K s2k = new S2K(HashAlgorithmTags.SHA1, iv, (int) iterationCount) { - @Override - public long getIterationCount() { - return iterationCount; - } - }; - - return s2k; - } - - static void skipOpenParenthesis(InputStream in) throws IOException { - int ch = in.read(); - if (ch != '(') { - throw new IOException( - "unknown character encountered: " + (char) ch); //$NON-NLS-1$ - } - } - - static void skipCloseParenthesis(InputStream in) throws IOException { - int ch = in.read(); - if (ch != ')') { - throw new IOException("unknown character encountered"); //$NON-NLS-1$ - } - } -} diff --git a/org.eclipse.jgit.gpg.bc/src/org/eclipse/jgit/gpg/bc/internal/keys/SecretKeys.java b/org.eclipse.jgit.gpg.bc/src/org/eclipse/jgit/gpg/bc/internal/keys/SecretKeys.java index a659d38fd3..a56e418bf4 100644 --- a/org.eclipse.jgit.gpg.bc/src/org/eclipse/jgit/gpg/bc/internal/keys/SecretKeys.java +++ b/org.eclipse.jgit.gpg.bc/src/org/eclipse/jgit/gpg/bc/internal/keys/SecretKeys.java @@ -1,5 +1,5 @@ /* - * Copyright (C) 2021 Thomas Wolf and others + * Copyright (C) 2021, 2024 Thomas Wolf and others * * This program and the accompanying materials are made available under the * terms of the Eclipse Distribution License v. 1.0 which is available at @@ -9,34 +9,36 @@ */ package org.eclipse.jgit.gpg.bc.internal.keys; -import java.io.ByteArrayInputStream; -import java.io.ByteArrayOutputStream; -import java.io.EOFException; +import java.io.BufferedInputStream; import java.io.IOException; import java.io.InputStream; -import java.io.StreamCorruptedException; import java.net.URISyntaxException; -import java.nio.charset.StandardCharsets; import java.text.MessageFormat; -import java.util.Arrays; +import org.bouncycastle.bcpg.ECPublicBCPGKey; +import org.bouncycastle.bcpg.PublicKeyAlgorithmTags; +import org.bouncycastle.gpg.PGPSecretKeyParser; +import org.bouncycastle.gpg.SExprParser; +import org.bouncycastle.openpgp.OpenedPGPKeyData; import org.bouncycastle.openpgp.PGPException; import org.bouncycastle.openpgp.PGPPublicKey; import org.bouncycastle.openpgp.PGPSecretKey; import org.bouncycastle.openpgp.operator.PBEProtectionRemoverFactory; import org.bouncycastle.openpgp.operator.PGPDigestCalculatorProvider; +import org.bouncycastle.openpgp.operator.jcajce.JcaKeyFingerprintCalculator; import org.bouncycastle.openpgp.operator.jcajce.JcePBEProtectionRemoverFactory; -import org.bouncycastle.util.io.Streams; import org.eclipse.jgit.api.errors.CanceledException; import org.eclipse.jgit.errors.UnsupportedCredentialItem; import org.eclipse.jgit.gpg.bc.internal.BCText; -import org.eclipse.jgit.util.RawParseUtils; /** * Utilities for reading GPG secret keys from a gpg-agent key file. */ public final class SecretKeys { + // Maximum nesting depth of sub-lists in an S-Expression for a secret key. + private static final int MAX_SEXPR_NESTING = 20; + private SecretKeys() { // No instantiation. } @@ -64,12 +66,6 @@ public final class SecretKeys { UnsupportedCredentialItem, URISyntaxException; } - private static final byte[] PROTECTED_KEY = "protected-private-key" //$NON-NLS-1$ - .getBytes(StandardCharsets.US_ASCII); - - private static final byte[] OCB_PROTECTED = "openpgp-s2k3-ocb-aes" //$NON-NLS-1$ - .getBytes(StandardCharsets.US_ASCII); - /** * Reads a GPG secret key from the given stream. * @@ -99,500 +95,59 @@ public final class SecretKeys { PassphraseSupplier passphraseSupplier, PGPPublicKey publicKey) throws IOException, PGPException, CanceledException, UnsupportedCredentialItem, URISyntaxException { - byte[] data = Streams.readAll(in); - if (data.length == 0) { - throw new EOFException(); - } else if (data.length < 4 + PROTECTED_KEY.length) { - // +4 for "(21:" for a binary protected key - throw new IOException( - MessageFormat.format(BCText.get().secretKeyTooShort, - Integer.toUnsignedString(data.length))); - } - SExprParser parser = new SExprParser(calculatorProvider); - byte firstChar = data[0]; - try { - if (firstChar == '(') { - // Binary format. - PBEProtectionRemoverFactory decryptor = null; - if (matches(data, 4, PROTECTED_KEY)) { - // AES/CBC encrypted. - decryptor = new JcePBEProtectionRemoverFactory( - passphraseSupplier.getPassphrase(), - calculatorProvider); - } - try (InputStream sIn = new ByteArrayInputStream(data)) { - return parser.parseSecretKey(sIn, decryptor, publicKey); - } - } - // Assume it's the new key-value format. - try (ByteArrayInputStream keyIn = new ByteArrayInputStream(data)) { - byte[] rawData = keyFromNameValueFormat(keyIn); - if (!matches(rawData, 1, PROTECTED_KEY)) { - // Not encrypted human-readable format. - try (InputStream sIn = new ByteArrayInputStream( - convertSexpression(rawData))) { - return parser.parseSecretKey(sIn, null, publicKey); - } - } - // An encrypted key from a key-value file. Most likely AES/OCB - // encrypted. - boolean isOCB[] = { false }; - byte[] sExp = convertSexpression(rawData, isOCB); - PBEProtectionRemoverFactory decryptor; - if (isOCB[0]) { - decryptor = new OCBPBEProtectionRemoverFactory( - passphraseSupplier.getPassphrase(), - calculatorProvider, getAad(sExp)); - } else { - decryptor = new JcePBEProtectionRemoverFactory( - passphraseSupplier.getPassphrase(), - calculatorProvider); - } - try (InputStream sIn = new ByteArrayInputStream(sExp)) { - return parser.parseSecretKey(sIn, decryptor, publicKey); - } - } - } catch (IOException e) { - throw new PGPException(e.getLocalizedMessage(), e); - } - } - - /** - * Extract the AAD for the OCB decryption from an s-expression. - * - * @param sExp - * buffer containing a valid binary s-expression - * @return the AAD - */ - private static byte[] getAad(byte[] sExp) { - // Given a key - // @formatter:off - // (protected-private-key (rsa ... (protected openpgp-s2k3-ocb-aes ... )(protected-at ...))) - // A B C D - // The AAD is [A..B)[C..D). (From the binary serialized form.) - // @formatter:on - int i = 1; // Skip initial '(' - while (sExp[i] != '(') { - i++; - } - int aadStart = i++; - int aadEnd = skip(sExp, aadStart); - byte[] protectedPrefix = "(9:protected" //$NON-NLS-1$ - .getBytes(StandardCharsets.US_ASCII); - while (!matches(sExp, i, protectedPrefix)) { - i++; - } - int protectedStart = i; - int protectedEnd = skip(sExp, protectedStart); - byte[] aadData = new byte[aadEnd - aadStart - - (protectedEnd - protectedStart)]; - System.arraycopy(sExp, aadStart, aadData, 0, protectedStart - aadStart); - System.arraycopy(sExp, protectedEnd, aadData, protectedStart - aadStart, - aadEnd - protectedEnd); - return aadData; - } - - /** - * Skips a list including nested lists. - * - * @param sExp - * buffer containing valid binary s-expression data - * @param start - * index of the opening '(' of the list to skip - * @return the index after the closing ')' of the skipped list - */ - private static int skip(byte[] sExp, int start) { - int i = start + 1; - int depth = 1; - while (depth > 0) { - switch (sExp[i]) { - case '(': - depth++; - break; - case ')': - depth--; - break; - default: - // We must be on a length - int j = i; - while (sExp[j] >= '0' && sExp[j] <= '9') { - j++; - } - // j is on the colon - int length = Integer.parseInt( - new String(sExp, i, j - i, StandardCharsets.US_ASCII)); - i = j + length; - } - i++; - } - return i; - } - - /** - * Checks whether the {@code needle} matches {@code src} at offset - * {@code from}. - * - * @param src - * to match against {@code needle} - * @param from - * position in {@code src} to start matching - * @param needle - * to match against - * @return {@code true} if {@code src} contains {@code needle} at position - * {@code from}, {@code false} otherwise - */ - private static boolean matches(byte[] src, int from, byte[] needle) { - if (from < 0 || from + needle.length > src.length) { - return false; - } - return org.bouncycastle.util.Arrays.constantTimeAreEqual(needle.length, - src, from, needle, 0); - } - - /** - * Converts a human-readable serialized s-expression into a binary - * serialized s-expression. - * - * @param humanForm - * to convert - * @return the converted s-expression - * @throws IOException - * if the conversion fails - */ - private static byte[] convertSexpression(byte[] humanForm) - throws IOException { - boolean[] isOCB = { false }; - return convertSexpression(humanForm, isOCB); - } - - /** - * Converts a human-readable serialized s-expression into a binary - * serialized s-expression. - * - * @param humanForm - * to convert - * @param isOCB - * returns whether the s-expression specified AES/OCB encryption - * @return the converted s-expression - * @throws IOException - * if the conversion fails - */ - private static byte[] convertSexpression(byte[] humanForm, boolean[] isOCB) - throws IOException { - int pos = 0; - try (ByteArrayOutputStream out = new ByteArrayOutputStream( - humanForm.length)) { - while (pos < humanForm.length) { - byte b = humanForm[pos]; - if (b == '(' || b == ')') { - out.write(b); - pos++; - } else if (isGpgSpace(b)) { - pos++; - } else if (b == '#') { - // Hex value follows up to the next # - int i = ++pos; - while (i < humanForm.length && isHex(humanForm[i])) { - i++; - } - if (i == pos || humanForm[i] != '#') { - throw new StreamCorruptedException( - BCText.get().sexprHexNotClosed); - } - if ((i - pos) % 2 != 0) { - throw new StreamCorruptedException( - BCText.get().sexprHexOdd); - } - int l = (i - pos) / 2; - out.write(Integer.toString(l) - .getBytes(StandardCharsets.US_ASCII)); - out.write(':'); - while (pos < i) { - int x = (nibble(humanForm[pos]) << 4) - | nibble(humanForm[pos + 1]); - pos += 2; - out.write(x); - } - pos = i + 1; - } else if (isTokenChar(b)) { - // Scan the token - int start = pos++; - while (pos < humanForm.length - && isTokenChar(humanForm[pos])) { - pos++; - } - int l = pos - start; - if (pos - start == OCB_PROTECTED.length - && matches(humanForm, start, OCB_PROTECTED)) { - isOCB[0] = true; - } - out.write(Integer.toString(l) - .getBytes(StandardCharsets.US_ASCII)); - out.write(':'); - out.write(humanForm, start, pos - start); - } else if (b == '"') { - // Potentially quoted string. - int start = ++pos; - boolean escaped = false; - while (pos < humanForm.length - && (escaped || humanForm[pos] != '"')) { - int ch = humanForm[pos++]; - escaped = !escaped && ch == '\\'; - } - if (pos >= humanForm.length) { - throw new StreamCorruptedException( - BCText.get().sexprStringNotClosed); - } - // start is on the first character of the string, pos on the - // closing quote. - byte[] dq = dequote(humanForm, start, pos); - out.write(Integer.toString(dq.length) - .getBytes(StandardCharsets.US_ASCII)); - out.write(':'); - out.write(dq); - pos++; - } else { - throw new StreamCorruptedException( - MessageFormat.format(BCText.get().sexprUnhandled, - Integer.toHexString(b & 0xFF))); - } - } - return out.toByteArray(); + OpenedPGPKeyData data; + try (InputStream keyIn = new BufferedInputStream(in)) { + data = PGPSecretKeyParser.parse(keyIn, MAX_SEXPR_NESTING); } - } - - /** - * GPG-style string de-quoting, which is basically C-style, with some - * literal CR/LF escaping. - * - * @param in - * buffer containing the quoted string - * @param from - * index after the opening quote in {@code in} - * @param to - * index of the closing quote in {@code in} - * @return the dequoted raw string value - * @throws StreamCorruptedException - * if object stream is corrupt - */ - private static byte[] dequote(byte[] in, int from, int to) - throws StreamCorruptedException { - // Result must be shorter or have the same length - byte[] out = new byte[to - from]; - int j = 0; - int i = from; - while (i < to) { - byte b = in[i++]; - if (b != '\\') { - out[j++] = b; - continue; - } - if (i == to) { - throw new StreamCorruptedException( - BCText.get().sexprStringInvalidEscapeAtEnd); - } - b = in[i++]; - switch (b) { - case 'b': - out[j++] = '\b'; - break; - case 'f': - out[j++] = '\f'; - break; - case 'n': - out[j++] = '\n'; - break; - case 'r': - out[j++] = '\r'; - break; - case 't': - out[j++] = '\t'; - break; - case 'v': - out[j++] = 0x0B; - break; - case '"': - case '\'': - case '\\': - out[j++] = b; - break; - case '\r': - // Escaped literal line end. If an LF is following, skip that, - // too. - if (i < to && in[i] == '\n') { - i++; - } - break; - case '\n': - // Same for LF possibly followed by CR. - if (i < to && in[i] == '\r') { - i++; - } - break; - case 'x': - if (i + 1 >= to || !isHex(in[i]) || !isHex(in[i + 1])) { - throw new StreamCorruptedException( - BCText.get().sexprStringInvalidHexEscape); - } - out[j++] = (byte) ((nibble(in[i]) << 4) | nibble(in[i + 1])); - i += 2; - break; - case '0': - case '1': - case '2': - case '3': - if (i + 2 >= to || !isOctal(in[i]) || !isOctal(in[i + 1]) - || !isOctal(in[i + 2])) { - throw new StreamCorruptedException( - BCText.get().sexprStringInvalidOctalEscape); - } - out[j++] = (byte) (((((in[i] - '0') << 3) - | (in[i + 1] - '0')) << 3) | (in[i + 2] - '0')); - i += 3; - break; - default: - throw new StreamCorruptedException(MessageFormat.format( - BCText.get().sexprStringInvalidEscape, - Integer.toHexString(b & 0xFF))); - } + PBEProtectionRemoverFactory decryptor = null; + if (isProtected(data)) { + decryptor = new JcePBEProtectionRemoverFactory( + passphraseSupplier.getPassphrase(), calculatorProvider); } - return Arrays.copyOf(out, j); - } - - /** - * Extracts the key from a GPG name-value-pair key file. - *

- * Package-visible for tests only. - *

- * - * @param in - * {@link InputStream} to read from; should be buffered - * @return the raw key data as extracted from the file - * @throws IOException - * if the {@code in} stream cannot be read or does not contain a - * key - */ - static byte[] keyFromNameValueFormat(InputStream in) throws IOException { - // It would be nice if we could use RawParseUtils here, but GPG compares - // names case-insensitively. We're only interested in the "Key:" - // name-value pair. - int[] nameLow = { 'k', 'e', 'y', ':' }; - int[] nameCap = { 'K', 'E', 'Y', ':' }; - int nameIdx = 0; - for (;;) { - int next = in.read(); - if (next < 0) { - throw new EOFException(); + switch (publicKey.getAlgorithm()) { + case PublicKeyAlgorithmTags.EDDSA_LEGACY: + case PublicKeyAlgorithmTags.Ed25519: + // If we let Bouncy Castle check whether the secret key matches the + // given public key it may get into trouble in some cases with + // ed25519 keys. It appears that we may end up with secret keys + // using the official RFC 8410 OID for ed25519, "1.3.101.112", while + // the public key passed in may have a non-standard OpenPGP-specific + // OID "1.3.6.1.4.1.11591.15.1", or vice versa. Bouncy Castle then + // throws an exception because of the different OIDs. + // + // The work-around is to just read the secret key, and double-check + // later that the OIDs are compatible and the curve points match. + PGPSecretKey secret = data.getKeyData(null, calculatorProvider, + decryptor, new JcaKeyFingerprintCalculator(), + MAX_SEXPR_NESTING); + PGPPublicKey pubKeyRead = secret.getPublicKey(); + int algoRead = pubKeyRead.getAlgorithm(); + if (algoRead != PublicKeyAlgorithmTags.EDDSA_LEGACY + && algoRead != PublicKeyAlgorithmTags.Ed25519) { + throw new PGPException(BCText.get().keyAlgorithmMismatch); } - if (next == '\n') { - nameIdx = 0; - } else if (nameIdx >= 0) { - if (nameLow[nameIdx] == next || nameCap[nameIdx] == next) { - nameIdx++; - if (nameIdx == nameLow.length) { - break; - } - } else { - nameIdx = -1; - } - } - } - // We're after "Key:". Read the value as continuation lines. - int last = ':'; - byte[] rawData; - try (ByteArrayOutputStream out = new ByteArrayOutputStream(8192)) { - for (;;) { - int next = in.read(); - if (next < 0) { - break; - } - if (last == '\n') { - if (next == ' ' || next == '\t') { - // Continuation line; skip this whitespace - last = next; - continue; - } - break; // Not a continuation line - } - out.write(next); - last = next; + ECPublicBCPGKey ec1 = (ECPublicBCPGKey) publicKey + .getPublicKeyPacket().getKey(); + ECPublicBCPGKey ec2 = (ECPublicBCPGKey) pubKeyRead + .getPublicKeyPacket().getKey(); + if (!ObjectIds.match(ec1.getCurveOID(), ec2.getCurveOID()) + || !ec1.getEncodedPoint().equals(ec2.getEncodedPoint())) { + throw new PGPException( + MessageFormat.format(BCText.get().keyMismatch, + ec1.getCurveOID(), ec1.getEncodedPoint(), + ec2.getCurveOID(), ec2.getEncodedPoint())); } - rawData = out.toByteArray(); - } - // GPG trims off trailing whitespace, and a line having only whitespace - // is a single LF. - try (ByteArrayOutputStream out = new ByteArrayOutputStream( - rawData.length)) { - int lineStart = 0; - boolean trimLeading = true; - while (lineStart < rawData.length) { - int nextLineStart = RawParseUtils.nextLF(rawData, lineStart); - if (trimLeading) { - while (lineStart < nextLineStart - && isGpgSpace(rawData[lineStart])) { - lineStart++; - } - } - // Trim trailing - int i = nextLineStart - 1; - while (lineStart < i && isGpgSpace(rawData[i])) { - i--; - } - if (i <= lineStart) { - // Empty line signifies LF - out.write('\n'); - trimLeading = true; - } else { - out.write(rawData, lineStart, i - lineStart + 1); - trimLeading = false; - } - lineStart = nextLineStart; - } - return out.toByteArray(); - } - } - - private static boolean isGpgSpace(int ch) { - return ch == ' ' || ch == '\t' || ch == '\r' || ch == '\n'; - } - - private static boolean isTokenChar(int ch) { - switch (ch) { - case '-': - case '.': - case '/': - case '_': - case ':': - case '*': - case '+': - case '=': - return true; + return secret; default: - if ((ch >= 'a' && ch <= 'z') || (ch >= 'A' && ch <= 'Z') - || (ch >= '0' && ch <= '9')) { - return true; - } - return false; + // For other key types let Bouncy Castle do the check. + return data.getKeyData(publicKey, calculatorProvider, decryptor, + null, MAX_SEXPR_NESTING); } } - private static boolean isHex(int ch) { - return (ch >= '0' && ch <= '9') || (ch >= 'A' && ch <= 'F') - || (ch >= 'a' && ch <= 'f'); + private static boolean isProtected(OpenedPGPKeyData data) { + return SExprParser.ProtectionFormatTypeTags.PROTECTED_PRIVATE_KEY == SExprParser + .getProtectionType(data.getKeyExpression().getString(0)); } - private static boolean isOctal(int ch) { - return (ch >= '0' && ch <= '7'); - } - - private static int nibble(int ch) { - if (ch >= '0' && ch <= '9') { - return ch - '0'; - } else if (ch >= 'A' && ch <= 'F') { - return ch - 'A' + 10; - } else if (ch >= 'a' && ch <= 'f') { - return ch - 'a' + 10; - } - return -1; - } } -- cgit v1.2.3 From 958d053920a27ee1de0b5290099e4f7149de2c4b Mon Sep 17 00:00:00 2001 From: Matthias Sohn Date: Thu, 7 Nov 2024 23:35:13 +0100 Subject: Enhance CommitBuilder#parent to tolerate null parent Change-Id: Ifdeafd040bca8331804c3e7568da0bee5cbd01df --- .../src/org/eclipse/jgit/junit/TestRepository.java | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/org.eclipse.jgit.junit/src/org/eclipse/jgit/junit/TestRepository.java b/org.eclipse.jgit.junit/src/org/eclipse/jgit/junit/TestRepository.java index a2e0a571eb..66cf739ef1 100644 --- a/org.eclipse.jgit.junit/src/org/eclipse/jgit/junit/TestRepository.java +++ b/org.eclipse.jgit.junit/src/org/eclipse/jgit/junit/TestRepository.java @@ -30,6 +30,7 @@ import java.util.List; import java.util.Set; import java.util.TimeZone; +import org.eclipse.jgit.annotations.Nullable; import org.eclipse.jgit.api.Git; import org.eclipse.jgit.dircache.DirCache; import org.eclipse.jgit.dircache.DirCacheBuilder; @@ -1144,15 +1145,18 @@ public class TestRepository implements AutoCloseable { } /** - * set parent commit + * Set parent commit * * @param p - * parent commit + * parent commit, can be {@code null} * @return this commit builder * @throws Exception * if an error occurred */ - public CommitBuilder parent(RevCommit p) throws Exception { + public CommitBuilder parent(@Nullable RevCommit p) throws Exception { + if (p == null) { + return this; + } if (parents.isEmpty()) { DirCacheBuilder b = tree.builder(); parseBody(p); -- cgit v1.2.3 From 93ede18ffd17df936da4e8d0ef28e2ba69019c93 Mon Sep 17 00:00:00 2001 From: Jacek Centkowski Date: Fri, 20 Sep 2024 08:47:13 +0200 Subject: Add `numberOfPackFilesAfterBitmap` to RepoStatistics Introduce a `numberOfPackFilesAfterBitmap` that contains the number of packfiles created since the latest bitmap generation. Notes: * the `repo.getObjectDatabase().getPacks()` that obtains the list of packs (in the existing `getStatistics` function) uses `PackDirectory.scanPacks` that boils down to call `PackDirectory.scanPacksImpl` which is sorting packs prior returning them therefore the `numberOfPackFilesAfterBitmap` is just all packs before the one that has bitmap attached * the improved version of `packAndPrune` function (one that skips non-existent packfiles) was introduced for testing Change-Id: I608011462f104fc002ac527aa405f492a8a4b0c2 --- ...NumberOfPackFilesAfterBitmapStatisticsTest.java | 173 +++++++++++++++++++++ .../org/eclipse/jgit/internal/storage/file/GC.java | 13 +- 2 files changed, 185 insertions(+), 1 deletion(-) create mode 100644 org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/GcNumberOfPackFilesAfterBitmapStatisticsTest.java diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/GcNumberOfPackFilesAfterBitmapStatisticsTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/GcNumberOfPackFilesAfterBitmapStatisticsTest.java new file mode 100644 index 0000000000..e5a391f2e3 --- /dev/null +++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/GcNumberOfPackFilesAfterBitmapStatisticsTest.java @@ -0,0 +1,173 @@ +/* + * Copyright (c) 2024 Jacek Centkowski and others. + * + * This program and the accompanying materials are made available under the + * terms of the Eclipse Distribution License v. 1.0 which is available at + * https://www.eclipse.org/org/documents/edl-v10.php. + * + * SPDX-License-Identifier: BSD-3-Clause + */ + +package org.eclipse.jgit.internal.storage.file; + +import static org.junit.Assert.assertEquals; + +import java.io.BufferedOutputStream; +import java.io.FileOutputStream; +import java.io.IOException; +import java.io.OutputStream; +import java.nio.file.Files; +import java.util.HashSet; +import java.util.Set; +import java.util.stream.StreamSupport; + +import org.eclipse.jgit.internal.storage.file.PackIndex.MutableEntry; +import org.eclipse.jgit.internal.storage.pack.PackExt; +import org.eclipse.jgit.internal.storage.pack.PackWriter; +import org.eclipse.jgit.junit.TestRepository; +import org.eclipse.jgit.lib.NullProgressMonitor; +import org.eclipse.jgit.lib.ObjectId; +import org.eclipse.jgit.lib.PersonIdent; +import org.eclipse.jgit.lib.Ref; +import org.eclipse.jgit.revwalk.RevCommit; +import org.eclipse.jgit.util.FileUtils; +import org.junit.Test; + +public class GcNumberOfPackFilesAfterBitmapStatisticsTest extends GcTestCase { + @Test + public void testShouldReportZeroObjectsForInitializedRepo() + throws IOException { + assertEquals(0L, gc.getStatistics().numberOfPackFilesAfterBitmap); + } + + @Test + public void testShouldReportAllPackFilesWhenNoGcWasPerformed() + throws Exception { + packAndPrune(); + long result = gc.getStatistics().numberOfPackFilesAfterBitmap; + + assertEquals(repo.getObjectDatabase().getPacks().size(), result); + } + + @Test + public void testShouldReportNoObjectsDirectlyAfterGc() throws Exception { + // given + addCommit(null); + gc.gc().get(); + assertEquals(1L, repositoryBitmapFiles()); + assertEquals(0L, gc.getStatistics().numberOfPackFilesAfterBitmap); + } + + @Test + public void testShouldReportNewObjectsAfterGcWhenRepositoryProgresses() + throws Exception { + // commit & gc + RevCommit parent = addCommit(null); + gc.gc().get(); + assertEquals(1L, repositoryBitmapFiles()); + + // progress & pack + addCommit(parent); + packAndPrune(); + + assertEquals(1L, gc.getStatistics().numberOfPackFilesAfterBitmap); + } + + @Test + public void testShouldReportNewObjectsFromTheLatestBitmapWhenRepositoryProgresses() + throws Exception { + // commit & gc + RevCommit parent = addCommit(null); + gc.gc().get(); + assertEquals(1L, repositoryBitmapFiles()); + + // progress & gc + parent = addCommit(parent); + gc.gc().get(); + assertEquals(2L, repositoryBitmapFiles()); + + // progress & pack + addCommit(parent); + packAndPrune(); + + assertEquals(1L, gc.getStatistics().numberOfPackFilesAfterBitmap); + } + + private void packAndPrune() throws Exception { + try (SkipNonExistingFilesTestRepository testRepo = new SkipNonExistingFilesTestRepository( + repo)) { + testRepo.packAndPrune(); + } + } + + private RevCommit addCommit(RevCommit parent) throws Exception { + return tr.branch("master").commit() + .author(new PersonIdent("repo-metrics", "repo@metrics.com")) + .parent(parent).create(); + } + + private long repositoryBitmapFiles() throws IOException { + return StreamSupport + .stream(Files + .newDirectoryStream(repo.getObjectDatabase() + .getPackDirectory().toPath(), "pack-*.bitmap") + .spliterator(), false) + .count(); + } + + /** + * The TestRepository has a {@link TestRepository#packAndPrune()} function + * but it fails in the last step after GC was performed as it doesn't + * SKIP_MISSING files. In order to circumvent it was copied and improved + * here. + */ + private static class SkipNonExistingFilesTestRepository + extends TestRepository { + private final FileRepository repo; + + private SkipNonExistingFilesTestRepository(FileRepository db) throws IOException { + super(db); + repo = db; + } + + @Override + public void packAndPrune() throws Exception { + ObjectDirectory odb = repo.getObjectDatabase(); + NullProgressMonitor m = NullProgressMonitor.INSTANCE; + + final PackFile pack, idx; + try (PackWriter pw = new PackWriter(repo)) { + Set all = new HashSet<>(); + for (Ref r : repo.getRefDatabase().getRefs()) + all.add(r.getObjectId()); + pw.preparePack(m, all, PackWriter.NONE); + + pack = new PackFile(odb.getPackDirectory(), pw.computeName(), + PackExt.PACK); + try (OutputStream out = new BufferedOutputStream( + new FileOutputStream(pack))) { + pw.writePack(m, m, out); + } + pack.setReadOnly(); + + idx = pack.create(PackExt.INDEX); + try (OutputStream out = new BufferedOutputStream( + new FileOutputStream(idx))) { + pw.writeIndex(out); + } + idx.setReadOnly(); + } + + odb.openPack(pack); + updateServerInfo(); + + // alternative packAndPrune implementation that skips missing files + // after GC. + for (Pack p : odb.getPacks()) { + for (MutableEntry e : p) + FileUtils.delete(odb.fileFor(e.toObjectId()), + FileUtils.SKIP_MISSING); + } + } + } +} diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/GC.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/GC.java index cf26f8d284..ee6abc372c 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/GC.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/GC.java @@ -1508,6 +1508,12 @@ public class GC { */ public long numberOfPackFiles; + /** + * The number of pack files that were created after the last bitmap + * generation. + */ + public long numberOfPackFilesAfterBitmap; + /** * The number of objects stored as loose objects. */ @@ -1543,6 +1549,8 @@ public class GC { final StringBuilder b = new StringBuilder(); b.append("numberOfPackedObjects=").append(numberOfPackedObjects); //$NON-NLS-1$ b.append(", numberOfPackFiles=").append(numberOfPackFiles); //$NON-NLS-1$ + b.append(", numberOfPackFilesAfterBitmap=") //$NON-NLS-1$ + .append(numberOfPackFilesAfterBitmap); b.append(", numberOfLooseObjects=").append(numberOfLooseObjects); //$NON-NLS-1$ b.append(", numberOfLooseRefs=").append(numberOfLooseRefs); //$NON-NLS-1$ b.append(", numberOfPackedRefs=").append(numberOfPackedRefs); //$NON-NLS-1$ @@ -1567,8 +1575,11 @@ public class GC { ret.numberOfPackedObjects += p.getIndex().getObjectCount(); ret.numberOfPackFiles++; ret.sizeOfPackedObjects += p.getPackFile().length(); - if (p.getBitmapIndex() != null) + if (p.getBitmapIndex() != null) { ret.numberOfBitmaps += p.getBitmapIndex().getBitmapCount(); + } else { + ret.numberOfPackFilesAfterBitmap++; + } } File objDir = repo.getObjectsDirectory(); String[] fanout = objDir.list(); -- cgit v1.2.3 From 88f8321bafeb9476bbb9b42b349521c68e375e4f Mon Sep 17 00:00:00 2001 From: Ivan Frade Date: Mon, 4 Nov 2024 14:21:24 -0800 Subject: SystemReader: Offer methods with java.time API Error prone explains: The Date API is full of major design flaws and pitfalls and should be avoided at all costs. Prefer the java.time APIs, specifically, java.time.Instant (for physical time) and java.time.LocalDate[Time] (for civil time). Add to SystemReader methods to get the time and timezone in the new java.time classes (Instant/ZoneId) and mark as deprecated their old counterparts. The mapping of methods is: * #getCurrentTime -> #now (returns Instant instead of int) * #getTimezone -> #getTimeZoneAt (returns ZoneOffset intead of int) * #getTimeZone -> #getTimeZoneId (return ZoneId instead of TimeZone) Change-Id: Ic55b2f442a40046ff0ed24f61f566fc7416471be --- .../org/eclipse/jgit/junit/MockSystemReader.java | 18 +++++++ org.eclipse.jgit/.settings/.api_filters | 14 +++++ .../src/org/eclipse/jgit/util/SystemReader.java | 61 ++++++++++++++++++++++ 3 files changed, 93 insertions(+) diff --git a/org.eclipse.jgit.junit/src/org/eclipse/jgit/junit/MockSystemReader.java b/org.eclipse.jgit.junit/src/org/eclipse/jgit/junit/MockSystemReader.java index 419fdb1966..b0365aa7e1 100644 --- a/org.eclipse.jgit.junit/src/org/eclipse/jgit/junit/MockSystemReader.java +++ b/org.eclipse.jgit.junit/src/org/eclipse/jgit/junit/MockSystemReader.java @@ -18,6 +18,9 @@ import java.lang.reflect.Field; import java.text.DateFormat; import java.text.SimpleDateFormat; import java.time.Duration; +import java.time.Instant; +import java.time.ZoneId; +import java.time.ZoneOffset; import java.util.HashMap; import java.util.Locale; import java.util.Map; @@ -201,6 +204,11 @@ public class MockSystemReader extends SystemReader { return now; } + @Override + public Instant now() { + return Instant.ofEpochMilli(now); + } + @Override public MonotonicClock getClock() { return () -> { @@ -236,11 +244,21 @@ public class MockSystemReader extends SystemReader { return getTimeZone().getOffset(when) / (60 * 1000); } + @Override + public ZoneOffset getTimeZoneAt(Instant when) { + return getTimeZoneId().getRules().getOffset(when); + } + @Override public TimeZone getTimeZone() { return TimeZone.getTimeZone("GMT-03:30"); } + @Override + public ZoneId getTimeZoneId() { + return ZoneOffset.ofHoursMinutes(-3, 30); + } + @Override public Locale getLocale() { return Locale.US; diff --git a/org.eclipse.jgit/.settings/.api_filters b/org.eclipse.jgit/.settings/.api_filters index f2c73f5c48..aed6683062 100644 --- a/org.eclipse.jgit/.settings/.api_filters +++ b/org.eclipse.jgit/.settings/.api_filters @@ -36,4 +36,18 @@ + + + + + + + + + + + + + + diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/util/SystemReader.java b/org.eclipse.jgit/src/org/eclipse/jgit/util/SystemReader.java index 03ed925617..7150e471bc 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/util/SystemReader.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/util/SystemReader.java @@ -25,6 +25,9 @@ import java.nio.file.Path; import java.nio.file.Paths; import java.text.DateFormat; import java.text.SimpleDateFormat; +import java.time.Instant; +import java.time.ZoneId; +import java.time.ZoneOffset; import java.util.Locale; import java.util.TimeZone; import java.util.concurrent.atomic.AtomicReference; @@ -166,10 +169,20 @@ public abstract class SystemReader { return System.currentTimeMillis(); } + @Override + public Instant now() { + return Instant.now(); + } + @Override public int getTimezone(long when) { return getTimeZone().getOffset(when) / (60 * 1000); } + + @Override + public ZoneOffset getTimeZoneAt(Instant when) { + return getTimeZoneId().getRules().getOffset(when); + } } /** @@ -227,10 +240,20 @@ public abstract class SystemReader { return delegate.getCurrentTime(); } + @Override + public Instant now() { + return delegate.now(); + } + @Override public int getTimezone(long when) { return delegate.getTimezone(when); } + + @Override + public ZoneOffset getTimeZoneAt(Instant when) { + return delegate.getTimeZoneAt(when); + } } private static volatile SystemReader INSTANCE = DEFAULT; @@ -501,9 +524,21 @@ public abstract class SystemReader { * Get the current system time * * @return the current system time + * + * @deprecated Use {@link #now()} */ + @Deprecated public abstract long getCurrentTime(); + /** + * Get the current system time + * + * @return the current system time + * + * @since 7.1 + */ + public abstract Instant now(); + /** * Get clock instance preferred by this system. * @@ -520,19 +555,45 @@ public abstract class SystemReader { * @param when * a system timestamp * @return the local time zone + * + * @deprecated Use {@link #getTimeZoneAt(Instant)} instead. */ + @Deprecated public abstract int getTimezone(long when); + /** + * Get the local time zone offset at "when" time + * + * @param when + * a system timestamp + * @return the local time zone + * @since 7.1 + */ + public abstract ZoneOffset getTimeZoneAt(Instant when); + /** * Get system time zone, possibly mocked for testing * * @return system time zone, possibly mocked for testing * @since 1.2 + * + * @deprecated Use {@link #getTimeZoneId()} */ + @Deprecated public TimeZone getTimeZone() { return TimeZone.getDefault(); } + /** + * Get system time zone, possibly mocked for testing + * + * @return system time zone, possibly mocked for testing + * @since 7.1 + */ + public ZoneId getTimeZoneId() { + return ZoneId.systemDefault(); + } + /** * Get the locale to use * -- cgit v1.2.3 From a7b5f056d8ff68a0df8968fdd10d3f7faa66d290 Mon Sep 17 00:00:00 2001 From: Laura Hamelin Date: Wed, 6 Nov 2024 13:41:30 -0800 Subject: DfsBlockCacheConfig: propagate hotmap configs to pack ext cache configs CacheHotMap is currently only set on the base DfsBlockCacheConfig and is not propagated down to PackExt specific caches. Because CacheHotMap is set from a method call rather than from Configs, this change sets per-PackExt CacheHotMap configs on PackExt cache configs both when DfsBlockCacheConfig#setCacheHotMap(...) is called, and when DfsBlockCacheConfig#configure(...) is called after setCacheHotMap. The outer DfsBlockCacheConfig keeps the full CacheHotMap for the same reason that the CacheHotMap config is propagated in both setCacheHotMap and configure: the order of operations setting the configuration from Configs and calling setCacheHotMap is not guaranteed. Change-Id: Id9dc32fedca99ecc83c9dc90c24d9616873a202e --- .../storage/dfs/DfsBlockCacheConfigTest.java | 71 ++++++++++++++++++++++ .../internal/storage/dfs/DfsBlockCacheConfig.java | 22 +++++++ 2 files changed, 93 insertions(+) diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/dfs/DfsBlockCacheConfigTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/dfs/DfsBlockCacheConfigTest.java index 65774e6d6c..afa3179cde 100644 --- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/dfs/DfsBlockCacheConfigTest.java +++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/dfs/DfsBlockCacheConfigTest.java @@ -179,6 +179,76 @@ public class DfsBlockCacheConfigTest { is(indexConfig)); } + @Test + public void fromConfig_withExistingCacheHotMap_configWithPackExtConfigsHasHotMaps() { + Config config = new Config(); + addPackExtConfigEntry(config, "pack", List.of(PackExt.PACK), + /* blockLimit= */ 20 * 512, /* blockSize= */ 512); + + addPackExtConfigEntry(config, "bitmap", List.of(PackExt.BITMAP_INDEX), + /* blockLimit= */ 25 * 1024, /* blockSize= */ 1024); + + addPackExtConfigEntry(config, "index", + List.of(PackExt.INDEX, PackExt.OBJECT_SIZE_INDEX, + PackExt.REVERSE_INDEX), + /* blockLimit= */ 30 * 1024, /* blockSize= */ 1024); + + Map cacheHotMap = Map.of(PackExt.PACK, 1, + PackExt.BITMAP_INDEX, 2, PackExt.INDEX, 3, PackExt.REFTABLE, 4); + + DfsBlockCacheConfig cacheConfig = new DfsBlockCacheConfig(); + cacheConfig.setCacheHotMap(cacheHotMap); + cacheConfig.fromConfig(config); + + var configs = cacheConfig.getPackExtCacheConfigurations(); + assertThat(cacheConfig.getCacheHotMap(), is(cacheHotMap)); + assertThat(configs, hasSize(3)); + var packConfig = getConfigForExt(configs, PackExt.PACK); + assertThat(packConfig.getCacheHotMap(), is(Map.of(PackExt.PACK, 1))); + + var bitmapConfig = getConfigForExt(configs, PackExt.BITMAP_INDEX); + assertThat(bitmapConfig.getCacheHotMap(), + is(Map.of(PackExt.BITMAP_INDEX, 2))); + + var indexConfig = getConfigForExt(configs, PackExt.INDEX); + assertThat(indexConfig.getCacheHotMap(), is(Map.of(PackExt.INDEX, 3))); + } + + @Test + public void setCacheHotMap_configWithPackExtConfigs_setsHotMaps() { + Config config = new Config(); + addPackExtConfigEntry(config, "pack", List.of(PackExt.PACK), + /* blockLimit= */ 20 * 512, /* blockSize= */ 512); + + addPackExtConfigEntry(config, "bitmap", List.of(PackExt.BITMAP_INDEX), + /* blockLimit= */ 25 * 1024, /* blockSize= */ 1024); + + addPackExtConfigEntry(config, "index", + List.of(PackExt.INDEX, PackExt.OBJECT_SIZE_INDEX, + PackExt.REVERSE_INDEX), + /* blockLimit= */ 30 * 1024, /* blockSize= */ 1024); + + Map cacheHotMap = Map.of(PackExt.PACK, 1, + PackExt.BITMAP_INDEX, 2, PackExt.INDEX, 3, PackExt.REFTABLE, 4); + + DfsBlockCacheConfig cacheConfig = new DfsBlockCacheConfig() + .fromConfig(config); + cacheConfig.setCacheHotMap(cacheHotMap); + + var configs = cacheConfig.getPackExtCacheConfigurations(); + assertThat(cacheConfig.getCacheHotMap(), is(cacheHotMap)); + assertThat(configs, hasSize(3)); + var packConfig = getConfigForExt(configs, PackExt.PACK); + assertThat(packConfig.getCacheHotMap(), is(Map.of(PackExt.PACK, 1))); + + var bitmapConfig = getConfigForExt(configs, PackExt.BITMAP_INDEX); + assertThat(bitmapConfig.getCacheHotMap(), + is(Map.of(PackExt.BITMAP_INDEX, 2))); + + var indexConfig = getConfigForExt(configs, PackExt.INDEX); + assertThat(indexConfig.getCacheHotMap(), is(Map.of(PackExt.INDEX, 3))); + } + @Test public void fromConfigs_baseConfigOnly_nameSetFromConfigDfsSubSection() { Config config = new Config(); @@ -291,6 +361,7 @@ public class DfsBlockCacheConfigTest { " Name: dfs.pack", " BlockLimit: " + 20 * 512, " BlockSize: 512", " StreamRatio: 0.3", " ConcurrencyLevel: 32", + " CacheHotMapEntry: " + PackExt.PACK + " : " + 10, " PackExts: " + List.of(PackExt.PACK)))); } diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/dfs/DfsBlockCacheConfig.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/dfs/DfsBlockCacheConfig.java index acd7add5a9..d63c208bbb 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/dfs/DfsBlockCacheConfig.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/dfs/DfsBlockCacheConfig.java @@ -30,6 +30,7 @@ import java.util.List; import java.util.Map; import java.util.Set; import java.util.function.Consumer; +import java.util.function.Function; import java.util.stream.Collectors; import org.eclipse.jgit.internal.JGitText; @@ -300,12 +301,24 @@ public class DfsBlockCacheConfig { * map of hot count per pack extension for {@code DfsBlockCache}. * @return {@code this} */ + /* + * TODO The cache HotMap configuration should be set as a config option and + * not passed in through a setter. + */ public DfsBlockCacheConfig setCacheHotMap( Map cacheHotMap) { this.cacheHotMap = Collections.unmodifiableMap(cacheHotMap); + setCacheHotMapToPackExtConfigs(this.cacheHotMap); return this; } + private void setCacheHotMapToPackExtConfigs( + Map cacheHotMap) { + for (DfsBlockCachePackExtConfig packExtConfig : packExtCacheConfigurations) { + packExtConfig.setCacheHotMap(cacheHotMap); + } + } + /** * Get the consumer of cache index events. * @@ -433,6 +446,7 @@ public class DfsBlockCacheConfig { cacheConfigs.add(cacheConfig); } packExtCacheConfigurations = cacheConfigs; + setCacheHotMapToPackExtConfigs(this.cacheHotMap); } private static Set intersection(Set first, Set second) { @@ -551,6 +565,14 @@ public class DfsBlockCacheConfig { return packExtCacheConfiguration; } + void setCacheHotMap(Map cacheHotMap) { + Map packExtHotMap = packExts.stream() + .filter(cacheHotMap::containsKey) + .collect(Collectors.toUnmodifiableMap(Function.identity(), + cacheHotMap::get)); + packExtCacheConfiguration.setCacheHotMap(packExtHotMap); + } + private static DfsBlockCachePackExtConfig fromConfig(Config config, String section, String subSection) { String packExtensions = config.getString(section, subSection, -- cgit v1.2.3 From 20d0bfbb348674a65b6c84826b549ab22ff22c13 Mon Sep 17 00:00:00 2001 From: Thomas Wolf Date: Fri, 8 Nov 2024 18:18:21 +0100 Subject: ssh: Minor simplification in SerialRangeSet Instead of set.headMap(x).lastKey() use set.floorKey(x) and instead of set.tailMap(x).firstKey() use set.ceilingKey(x). Change-Id: I22f44cbe82b9ead06d6ff517d609dfdbc89a758c --- .../jgit/internal/signing/ssh/SerialRangeSet.java | 31 +++++++++------------- 1 file changed, 12 insertions(+), 19 deletions(-) diff --git a/org.eclipse.jgit.ssh.apache/src/org/eclipse/jgit/internal/signing/ssh/SerialRangeSet.java b/org.eclipse.jgit.ssh.apache/src/org/eclipse/jgit/internal/signing/ssh/SerialRangeSet.java index 6a0cec8821..f4eb884239 100644 --- a/org.eclipse.jgit.ssh.apache/src/org/eclipse/jgit/internal/signing/ssh/SerialRangeSet.java +++ b/org.eclipse.jgit.ssh.apache/src/org/eclipse/jgit/internal/signing/ssh/SerialRangeSet.java @@ -9,7 +9,6 @@ */ package org.eclipse.jgit.internal.signing.ssh; -import java.util.SortedMap; import java.util.TreeMap; import org.eclipse.jgit.internal.transport.sshd.SshdText; @@ -50,14 +49,14 @@ class SerialRangeSet { } } - // We use the same data structure as OpenSSH,; basically a - // TreeSet of mutable elements. To get "mutability", the set is - // implemented as a TreeMap with the same elements as keys and values. + // We use the same data structure as OpenSSH; basically a TreeSet of mutable + // SerialRanges. To get "mutability", the set is implemented as a TreeMap + // with the same elements as keys and values. // // get(x) will return null if none of the serial numbers in the range x is // in the set, and some range (partially) overlapping with x otherwise. // - // containsKey will return true if there is any (partially) overlapping + // containsKey(x) will return true if there is any (partially) overlapping // range in the TreeMap. private final TreeMap ranges = new TreeMap<>( SerialRangeSet::compare); @@ -117,21 +116,15 @@ class SerialRangeSet { } // No overlapping range exists: check for coalescing with the // previous/next range - SortedMap head = ranges.headMap(newRange); - if (!head.isEmpty()) { - SerialRange prev = head.lastKey(); - if (newRange.from() - prev.to() == 1) { - ranges.remove(prev); - newRange = new Range(prev.from(), newRange.to()); - } + SerialRange prev = ranges.floorKey(newRange); + if (prev != null && newRange.from() - prev.to() == 1) { + ranges.remove(prev); + newRange = new Range(prev.from(), newRange.to()); } - SortedMap tail = ranges.tailMap(newRange); - if (!tail.isEmpty()) { - SerialRange next = tail.firstKey(); - if (next.from() - newRange.to() == 1) { - ranges.remove(next); - newRange = new Range(newRange.from(), next.to()); - } + SerialRange next = ranges.ceilingKey(newRange); + if (next != null && next.from() - newRange.to() == 1) { + ranges.remove(next); + newRange = new Range(newRange.from(), next.to()); } ranges.put(newRange, newRange); } -- cgit v1.2.3 From 9ea537d5c44876c12d154331c94a7ba14eaa64f8 Mon Sep 17 00:00:00 2001 From: Ivan Frade Date: Fri, 8 Nov 2024 08:49:09 -0800 Subject: DfsGarbageCollector: #setReflogExpire with Instant instead of Date The Date API is full of major design flaws and pitfalls and should be avoided at all costs. Prefer the java.time APIs, specifically, java.time.Instant (for physical time) and java.time.LocalDate[Time] (for civil time). [1] Replace the Date with Instant in the DfsGarbageCollector#setReflogExpire method. [1] https://errorprone.info/bugpattern/JavaUtilDate Change-Id: Ie98e426e63621e8bef96c31bca56aec0c8eef5a6 --- .../jgit/internal/storage/dfs/DfsGarbageCollectorTest.java | 3 ++- .../jgit/internal/storage/dfs/DfsGarbageCollector.java | 11 ++++++----- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/dfs/DfsGarbageCollectorTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/dfs/DfsGarbageCollectorTest.java index 3edd1056be..f9fbfe8db0 100644 --- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/dfs/DfsGarbageCollectorTest.java +++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/dfs/DfsGarbageCollectorTest.java @@ -16,6 +16,7 @@ import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; import java.io.IOException; +import java.time.Instant; import java.util.Arrays; import java.util.Collections; import java.util.Date; @@ -1331,7 +1332,7 @@ public class DfsGarbageCollectorTest { gc = new DfsGarbageCollector(repo); gc.setReftableConfig(new ReftableConfig()); // Expire ref log entries older than 30 days - gc.setRefLogExpire(new Date(thirty_days_ago)); + gc.setRefLogExpire(Instant.ofEpochMilli(thirty_days_ago)); run(gc); // Single GC pack present with all objects. diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/dfs/DfsGarbageCollector.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/dfs/DfsGarbageCollector.java index 6861fe8099..e6068a15ec 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/dfs/DfsGarbageCollector.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/dfs/DfsGarbageCollector.java @@ -24,11 +24,11 @@ import static org.eclipse.jgit.internal.storage.pack.PackExt.REFTABLE; import static org.eclipse.jgit.internal.storage.pack.PackWriter.NONE; import java.io.IOException; +import java.time.Instant; import java.util.ArrayList; import java.util.Arrays; import java.util.Calendar; import java.util.Collection; -import java.util.Date; import java.util.EnumSet; import java.util.GregorianCalendar; import java.util.HashSet; @@ -100,7 +100,7 @@ public class DfsGarbageCollector { private Set allTags; private Set nonHeads; private Set tagTargets; - private Date refLogExpire; + private Instant refLogExpire; /** * Initialize a garbage collector. @@ -212,8 +212,8 @@ public class DfsGarbageCollector { * instant in time which defines refLog expiration * @return {@code this} */ - public DfsGarbageCollector setRefLogExpire(Date refLogExpire) { - this.refLogExpire = refLogExpire ; + public DfsGarbageCollector setRefLogExpire(Instant refLogExpire) { + this.refLogExpire = refLogExpire; return this; } @@ -752,7 +752,8 @@ public class DfsGarbageCollector { compact.setIncludeDeletes(includeDeletes); compact.setConfig(configureReftable(reftableConfig, out)); if(refLogExpire != null ){ - compact.setReflogExpireOldestReflogTimeMillis(refLogExpire.getTime()); + compact.setReflogExpireOldestReflogTimeMillis( + refLogExpire.toEpochMilli()); } compact.compact(); pack.addFileExt(REFTABLE); -- cgit v1.2.3 From e8c414b9cc72cd0e4f8c66b836de4c43f134b102 Mon Sep 17 00:00:00 2001 From: Matthias Sohn Date: Fri, 6 Sep 2024 12:26:44 +0200 Subject: Replace custom encoder `Constants#encode` by JDK implementation Using the implementation provided in the JDK since Java 1.6 by `String#getBytes(Charset)` reduces JGit maintenance effort and improves performance. The method Constants#encode was implemented when JGit still used Java 1.5. See [1]. Kudos to Marcin for proposing to use this improvement in RefWriter [2]. I think it should be used generally. [1] https://repo.or.cz/jgit.git?a=commit;h=bfa3da225f198b19061158499b1135aff07d85b3 [2] https://eclipse.gerrithub.io/c/eclipse-jgit/jgit/+/1195180 Also-By: Marcin Czech Change-Id: I361ed6286b98351a315b8a8ffc3cb845831d35b2 (cherry picked from commit e5d2898997462e0f2409c09497ab62c6cda2dbaf) --- org.eclipse.jgit/src/org/eclipse/jgit/lib/Constants.java | 13 +------------ 1 file changed, 1 insertion(+), 12 deletions(-) diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/lib/Constants.java b/org.eclipse.jgit/src/org/eclipse/jgit/lib/Constants.java index 1a97d111e3..5d7eacb4b8 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/lib/Constants.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/lib/Constants.java @@ -14,7 +14,6 @@ package org.eclipse.jgit.lib; import static java.nio.charset.StandardCharsets.UTF_8; -import java.nio.ByteBuffer; import java.nio.charset.Charset; import java.security.MessageDigest; import java.security.NoSuchAlgorithmException; @@ -688,17 +687,7 @@ public final class Constants { * @see #CHARACTER_ENCODING */ public static byte[] encode(String str) { - final ByteBuffer bb = UTF_8.encode(str); - final int len = bb.limit(); - if (bb.hasArray() && bb.arrayOffset() == 0) { - final byte[] arr = bb.array(); - if (arr.length == len) - return arr; - } - - final byte[] arr = new byte[len]; - bb.get(arr); - return arr; + return str.getBytes(UTF_8); } static { -- cgit v1.2.3 From d34f8b523638fc95b2e7006d02c9f6a756cbba85 Mon Sep 17 00:00:00 2001 From: Matthias Sohn Date: Fri, 6 Sep 2024 13:42:27 +0200 Subject: Replace custom encoder Constants#encodeASCII by JDK implementation Ensure that the method still throws an IllegalArgumentException for malformed input or if the String contains unmappable characters. Change-Id: I6a340aa1af60c315272ff13b6bf2041ba30c94ca (cherry picked from commit 0fd76114e3436ac635641d06371fd8833179312d) --- .../src/org/eclipse/jgit/lib/Constants.java | 20 +++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/lib/Constants.java b/org.eclipse.jgit/src/org/eclipse/jgit/lib/Constants.java index 5d7eacb4b8..1a2f735622 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/lib/Constants.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/lib/Constants.java @@ -12,9 +12,14 @@ package org.eclipse.jgit.lib; +import static java.nio.charset.StandardCharsets.US_ASCII; import static java.nio.charset.StandardCharsets.UTF_8; import java.nio.charset.Charset; +import java.nio.CharBuffer; +import java.nio.charset.CharacterCodingException; +import java.nio.charset.CharsetEncoder; +import java.nio.charset.CodingErrorAction; import java.security.MessageDigest; import java.security.NoSuchAlgorithmException; import java.text.MessageFormat; @@ -667,14 +672,15 @@ public final class Constants { * the 7-bit ASCII character space. */ public static byte[] encodeASCII(String s) { - final byte[] r = new byte[s.length()]; - for (int k = r.length - 1; k >= 0; k--) { - final char c = s.charAt(k); - if (c > 127) - throw new IllegalArgumentException(MessageFormat.format(JGitText.get().notASCIIString, s)); - r[k] = (byte) c; + try { + CharsetEncoder encoder = US_ASCII.newEncoder() + .onUnmappableCharacter(CodingErrorAction.REPORT) + .onMalformedInput(CodingErrorAction.REPORT); + return encoder.encode(CharBuffer.wrap(s)).array(); + } catch (CharacterCodingException e) { + throw new IllegalArgumentException( + MessageFormat.format(JGitText.get().notASCIIString, s), e); } - return r; } /** -- cgit v1.2.3 From ce3a7ca3bf0b5f8ad9d14efb730c8d3d204e802a Mon Sep 17 00:00:00 2001 From: Jacek Centkowski Date: Mon, 11 Nov 2024 12:36:03 +0100 Subject: Rename numberOfPackFilesAfterBitmap to numberOfPackFilesSinceBitmap As sugested in I608011462f1. Change-Id: If66226dd7b08ae768413fa614df5dcb6b44dc118 --- ...NumberOfPackFilesAfterBitmapStatisticsTest.java | 180 --------------------- ...NumberOfPackFilesSinceBitmapStatisticsTest.java | 180 +++++++++++++++++++++ .../org/eclipse/jgit/internal/storage/file/GC.java | 8 +- 3 files changed, 184 insertions(+), 184 deletions(-) delete mode 100644 org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/GcNumberOfPackFilesAfterBitmapStatisticsTest.java create mode 100644 org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/GcNumberOfPackFilesSinceBitmapStatisticsTest.java diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/GcNumberOfPackFilesAfterBitmapStatisticsTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/GcNumberOfPackFilesAfterBitmapStatisticsTest.java deleted file mode 100644 index 820f0c768d..0000000000 --- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/GcNumberOfPackFilesAfterBitmapStatisticsTest.java +++ /dev/null @@ -1,180 +0,0 @@ -/* - * Copyright (c) 2024 Jacek Centkowski and others. - * - * This program and the accompanying materials are made available under the - * terms of the Eclipse Distribution License v. 1.0 which is available at - * https://www.eclipse.org/org/documents/edl-v10.php. - * - * SPDX-License-Identifier: BSD-3-Clause - */ - -package org.eclipse.jgit.internal.storage.file; - -import static org.junit.Assert.assertEquals; - -import java.io.BufferedOutputStream; -import java.io.FileOutputStream; -import java.io.IOException; -import java.io.OutputStream; -import java.nio.file.Files; -import java.util.HashSet; -import java.util.Set; -import java.util.stream.StreamSupport; - -import org.eclipse.jgit.internal.storage.file.PackIndex.MutableEntry; -import org.eclipse.jgit.internal.storage.pack.PackExt; -import org.eclipse.jgit.internal.storage.pack.PackWriter; -import org.eclipse.jgit.junit.TestRepository; -import org.eclipse.jgit.lib.NullProgressMonitor; -import org.eclipse.jgit.lib.ObjectId; -import org.eclipse.jgit.lib.PersonIdent; -import org.eclipse.jgit.lib.Ref; -import org.eclipse.jgit.revwalk.RevCommit; -import org.eclipse.jgit.util.FileUtils; -import org.junit.Test; - -public class GcNumberOfPackFilesAfterBitmapStatisticsTest extends GcTestCase { - @Test - public void testShouldReportZeroObjectsForInitializedRepo() - throws IOException { - assertEquals(0L, gc.getStatistics().numberOfPackFilesAfterBitmap); - } - - @Test - public void testShouldReportAllPackFilesWhenNoGcWasPerformed() - throws Exception { - packAndPrune(); - long result = gc.getStatistics().numberOfPackFilesAfterBitmap; - - assertEquals(repo.getObjectDatabase().getPacks().size(), result); - } - - @Test - public void testShouldReportNoObjectsDirectlyAfterGc() throws Exception { - // given - addCommit(null); - gc.gc().get(); - assertEquals(1L, repositoryBitmapFiles()); - assertEquals(0L, gc.getStatistics().numberOfPackFilesAfterBitmap); - } - - @Test - public void testShouldReportNewObjectsAfterGcWhenRepositoryProgresses() - throws Exception { - // commit & gc - RevCommit parent = addCommit(null); - gc.gc().get(); - assertEquals(1L, repositoryBitmapFiles()); - - // progress & pack - addCommit(parent); - packAndPrune(); - - assertEquals(1L, gc.getStatistics().numberOfPackFilesAfterBitmap); - } - - @Test - public void testShouldReportNewObjectsFromTheLatestBitmapWhenRepositoryProgresses() - throws Exception { - // commit & gc - RevCommit parent = addCommit(null); - gc.gc().get(); - assertEquals(1L, repositoryBitmapFiles()); - - // progress & gc - parent = addCommit(parent); - gc.gc().get(); - assertEquals(2L, repositoryBitmapFiles()); - - // progress & pack - addCommit(parent); - packAndPrune(); - - assertEquals(1L, gc.getStatistics().numberOfPackFilesAfterBitmap); - } - - private void packAndPrune() throws Exception { - try (SkipNonExistingFilesTestRepository testRepo = new SkipNonExistingFilesTestRepository( - repo)) { - testRepo.packAndPrune(); - } - } - - private RevCommit addCommit(RevCommit parent) throws Exception { - PersonIdent ident = new PersonIdent("repo-metrics", "repo@metrics.com"); - TestRepository.CommitBuilder builder = tr.commit() - .author(ident); - if (parent != null) { - builder.parent(parent); - } - RevCommit commit = builder.create(); - tr.update("master", commit); - parent = commit; - return parent; - } - - private long repositoryBitmapFiles() throws IOException { - return StreamSupport - .stream(Files - .newDirectoryStream(repo.getObjectDatabase() - .getPackDirectory().toPath(), "pack-*.bitmap") - .spliterator(), false) - .count(); - } - - /** - * The TestRepository has a {@link TestRepository#packAndPrune()} function - * but it fails in the last step after GC was performed as it doesn't - * SKIP_MISSING files. In order to circumvent it was copied and improved - * here. - */ - private static class SkipNonExistingFilesTestRepository - extends TestRepository { - private final FileRepository repo; - - private SkipNonExistingFilesTestRepository(FileRepository db) throws IOException { - super(db); - repo = db; - } - - @Override - public void packAndPrune() throws Exception { - ObjectDirectory odb = repo.getObjectDatabase(); - NullProgressMonitor m = NullProgressMonitor.INSTANCE; - - final PackFile pack, idx; - try (PackWriter pw = new PackWriter(repo)) { - Set all = new HashSet<>(); - for (Ref r : repo.getRefDatabase().getRefs()) - all.add(r.getObjectId()); - pw.preparePack(m, all, PackWriter.NONE); - - pack = new PackFile(odb.getPackDirectory(), pw.computeName(), - PackExt.PACK); - try (OutputStream out = new BufferedOutputStream( - new FileOutputStream(pack))) { - pw.writePack(m, m, out); - } - pack.setReadOnly(); - - idx = pack.create(PackExt.INDEX); - try (OutputStream out = new BufferedOutputStream( - new FileOutputStream(idx))) { - pw.writeIndex(out); - } - idx.setReadOnly(); - } - - odb.openPack(pack); - updateServerInfo(); - - // alternative packAndPrune implementation that skips missing files - // after GC. - for (Pack p : odb.getPacks()) { - for (MutableEntry e : p) - FileUtils.delete(odb.fileFor(e.toObjectId()), - FileUtils.SKIP_MISSING); - } - } - } -} diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/GcNumberOfPackFilesSinceBitmapStatisticsTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/GcNumberOfPackFilesSinceBitmapStatisticsTest.java new file mode 100644 index 0000000000..42b99ae512 --- /dev/null +++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/GcNumberOfPackFilesSinceBitmapStatisticsTest.java @@ -0,0 +1,180 @@ +/* + * Copyright (c) 2024 Jacek Centkowski and others. + * + * This program and the accompanying materials are made available under the + * terms of the Eclipse Distribution License v. 1.0 which is available at + * https://www.eclipse.org/org/documents/edl-v10.php. + * + * SPDX-License-Identifier: BSD-3-Clause + */ + +package org.eclipse.jgit.internal.storage.file; + +import static org.junit.Assert.assertEquals; + +import java.io.BufferedOutputStream; +import java.io.FileOutputStream; +import java.io.IOException; +import java.io.OutputStream; +import java.nio.file.Files; +import java.util.HashSet; +import java.util.Set; +import java.util.stream.StreamSupport; + +import org.eclipse.jgit.internal.storage.file.PackIndex.MutableEntry; +import org.eclipse.jgit.internal.storage.pack.PackExt; +import org.eclipse.jgit.internal.storage.pack.PackWriter; +import org.eclipse.jgit.junit.TestRepository; +import org.eclipse.jgit.lib.NullProgressMonitor; +import org.eclipse.jgit.lib.ObjectId; +import org.eclipse.jgit.lib.PersonIdent; +import org.eclipse.jgit.lib.Ref; +import org.eclipse.jgit.revwalk.RevCommit; +import org.eclipse.jgit.util.FileUtils; +import org.junit.Test; + +public class GcNumberOfPackFilesSinceBitmapStatisticsTest extends GcTestCase { + @Test + public void testShouldReportZeroObjectsForInitializedRepo() + throws IOException { + assertEquals(0L, gc.getStatistics().numberOfPackFilesSinceBitmap); + } + + @Test + public void testShouldReportAllPackFilesWhenNoGcWasPerformed() + throws Exception { + packAndPrune(); + long result = gc.getStatistics().numberOfPackFilesSinceBitmap; + + assertEquals(repo.getObjectDatabase().getPacks().size(), result); + } + + @Test + public void testShouldReportNoObjectsDirectlyAfterGc() throws Exception { + // given + addCommit(null); + gc.gc().get(); + assertEquals(1L, repositoryBitmapFiles()); + assertEquals(0L, gc.getStatistics().numberOfPackFilesSinceBitmap); + } + + @Test + public void testShouldReportNewObjectsSinceGcWhenRepositoryProgresses() + throws Exception { + // commit & gc + RevCommit parent = addCommit(null); + gc.gc().get(); + assertEquals(1L, repositoryBitmapFiles()); + + // progress & pack + addCommit(parent); + packAndPrune(); + + assertEquals(1L, gc.getStatistics().numberOfPackFilesSinceBitmap); + } + + @Test + public void testShouldReportNewObjectsFromTheLatestBitmapWhenRepositoryProgresses() + throws Exception { + // commit & gc + RevCommit parent = addCommit(null); + gc.gc().get(); + assertEquals(1L, repositoryBitmapFiles()); + + // progress & gc + parent = addCommit(parent); + gc.gc().get(); + assertEquals(2L, repositoryBitmapFiles()); + + // progress & pack + addCommit(parent); + packAndPrune(); + + assertEquals(1L, gc.getStatistics().numberOfPackFilesSinceBitmap); + } + + private void packAndPrune() throws Exception { + try (SkipNonExistingFilesTestRepository testRepo = new SkipNonExistingFilesTestRepository( + repo)) { + testRepo.packAndPrune(); + } + } + + private RevCommit addCommit(RevCommit parent) throws Exception { + PersonIdent ident = new PersonIdent("repo-metrics", "repo@metrics.com"); + TestRepository.CommitBuilder builder = tr.commit() + .author(ident); + if (parent != null) { + builder.parent(parent); + } + RevCommit commit = builder.create(); + tr.update("master", commit); + parent = commit; + return parent; + } + + private long repositoryBitmapFiles() throws IOException { + return StreamSupport + .stream(Files + .newDirectoryStream(repo.getObjectDatabase() + .getPackDirectory().toPath(), "pack-*.bitmap") + .spliterator(), false) + .count(); + } + + /** + * The TestRepository has a {@link TestRepository#packAndPrune()} function + * but it fails in the last step after GC was performed as it doesn't + * SKIP_MISSING files. In order to circumvent it was copied and improved + * here. + */ + private static class SkipNonExistingFilesTestRepository + extends TestRepository { + private final FileRepository repo; + + private SkipNonExistingFilesTestRepository(FileRepository db) throws IOException { + super(db); + repo = db; + } + + @Override + public void packAndPrune() throws Exception { + ObjectDirectory odb = repo.getObjectDatabase(); + NullProgressMonitor m = NullProgressMonitor.INSTANCE; + + final PackFile pack, idx; + try (PackWriter pw = new PackWriter(repo)) { + Set all = new HashSet<>(); + for (Ref r : repo.getRefDatabase().getRefs()) + all.add(r.getObjectId()); + pw.preparePack(m, all, PackWriter.NONE); + + pack = new PackFile(odb.getPackDirectory(), pw.computeName(), + PackExt.PACK); + try (OutputStream out = new BufferedOutputStream( + new FileOutputStream(pack))) { + pw.writePack(m, m, out); + } + pack.setReadOnly(); + + idx = pack.create(PackExt.INDEX); + try (OutputStream out = new BufferedOutputStream( + new FileOutputStream(idx))) { + pw.writeIndex(out); + } + idx.setReadOnly(); + } + + odb.openPack(pack); + updateServerInfo(); + + // alternative packAndPrune implementation that skips missing files + // after GC. + for (Pack p : odb.getPacks()) { + for (MutableEntry e : p) + FileUtils.delete(odb.fileFor(e.toObjectId()), + FileUtils.SKIP_MISSING); + } + } + } +} diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/GC.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/GC.java index 34b5ec01e1..d8eb751136 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/GC.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/GC.java @@ -1512,7 +1512,7 @@ public class GC { * The number of pack files that were created after the last bitmap * generation. */ - public long numberOfPackFilesAfterBitmap; + public long numberOfPackFilesSinceBitmap; /** * The number of objects stored as loose objects. @@ -1549,8 +1549,8 @@ public class GC { final StringBuilder b = new StringBuilder(); b.append("numberOfPackedObjects=").append(numberOfPackedObjects); //$NON-NLS-1$ b.append(", numberOfPackFiles=").append(numberOfPackFiles); //$NON-NLS-1$ - b.append(", numberOfPackFilesAfterBitmap=") //$NON-NLS-1$ - .append(numberOfPackFilesAfterBitmap); + b.append(", numberOfPackFilesSinceBitmap=") //$NON-NLS-1$ + .append(numberOfPackFilesSinceBitmap); b.append(", numberOfLooseObjects=").append(numberOfLooseObjects); //$NON-NLS-1$ b.append(", numberOfLooseRefs=").append(numberOfLooseRefs); //$NON-NLS-1$ b.append(", numberOfPackedRefs=").append(numberOfPackedRefs); //$NON-NLS-1$ @@ -1578,7 +1578,7 @@ public class GC { if (p.getBitmapIndex() != null) ret.numberOfBitmaps += p.getBitmapIndex().getBitmapCount(); else - ret.numberOfPackFilesAfterBitmap++; + ret.numberOfPackFilesSinceBitmap++; } File objDir = repo.getObjectsDirectory(); String[] fanout = objDir.list(); -- cgit v1.2.3 From 8f05f61c54d76504845d47ca4c27967a20430cce Mon Sep 17 00:00:00 2001 From: Ivan Frade Date: Mon, 11 Nov 2024 11:07:21 -0800 Subject: errorprone: Disable javadoc checks in tests Errorprone finds many problems in the tests javadocs. This is noisy in the logs, but fixing them also disturbs the project history and can complicate merges. Disable the javadoc checks in the tests packages. We can fix those javadocs if some other modification happen in the file (as we fix older coding style). Change-Id: Ic221b60afe77ed9c207adbefd9117d2e26107792 --- tools/BUILD | 46 +++++++++++++++++++++++++++++++++------------- 1 file changed, 33 insertions(+), 13 deletions(-) diff --git a/tools/BUILD b/tools/BUILD index 8c424b357b..844f0049e6 100644 --- a/tools/BUILD +++ b/tools/BUILD @@ -10,6 +10,7 @@ default_java_toolchain( java_runtime = "@rules_java//toolchains:remotejdk_17", package_configuration = [ ":error_prone", + ":error_prone_tests", ], source_version = "17", target_version = "17", @@ -22,6 +23,7 @@ default_java_toolchain( java_runtime = "@rules_java//toolchains:remotejdk_21", package_configuration = [ ":error_prone", + ":error_prone_tests", ], source_version = "21", target_version = "21", @@ -32,9 +34,7 @@ default_java_toolchain( # enabled. This warnings list is originally based on: # https://github.com/bazelbuild/BUILD_file_generator/blob/master/tools/bazel_defs/java.bzl # However, feel free to add any additional errors. Thus far they have all been pretty useful. -java_package_configuration( - name = "error_prone", - javacopts = [ +errorprone_checks = [ "-XepDisableWarningsInGeneratedCode", # The XepDisableWarningsInGeneratedCode disables only warnings, but # not errors. We should manually exclude all files generated by @@ -422,37 +422,57 @@ java_package_configuration( "-Xep:WrongOneof:ERROR", "-Xep:XorPower:ERROR", "-Xep:ZoneIdOfZ:ERROR", - ], +] + + +exclude_in_tests = ["-Xep:EmptyBlockTag:WARN", + "-Xep:MissingSummary:WARN"] + +java_package_configuration( + name = "error_prone", + javacopts = errorprone_checks, packages = ["error_prone_packages"], ) +java_package_configuration( + name = "error_prone_tests", + javacopts = [ check for check in errorprone_checks if check not in exclude_in_tests], + packages = ["error_prone_packages_test"], +) + package_group( name = "error_prone_packages", packages = [ - "//org.eclipse.jgit.ant.test/...", "//org.eclipse.jgit.ant/...", "//org.eclipse.jgit.archive/...", - "//org.eclipse.jgit.gpg.bc.test/...", "//org.eclipse.jgit.gpg.bc/...", "//org.eclipse.jgit.http.apache/...", "//org.eclipse.jgit.http.server/...", - "//org.eclipse.jgit.http.test/...", "//org.eclipse.jgit.junit.ssh/...", "//org.eclipse.jgit.junit/...", "//org.eclipse.jgit.junit/http/...", - "//org.eclipse.jgit.lfs.server.test/...", "//org.eclipse.jgit.lfs.server/...", - "//org.eclipse.jgit.lfs.test/...", "//org.eclipse.jgit.lfs/...", - "//org.eclipse.jgit.pgm.test/...", "//org.eclipse.jgit.pgm/...", "//org.eclipse.jgit.ssh.apache.agent/...", - "//org.eclipse.jgit.ssh.apache.test/...", "//org.eclipse.jgit.ssh.apache/...", - "//org.eclipse.jgit.ssh.jsch.test/...", "//org.eclipse.jgit.ssh.jsch/...", - "//org.eclipse.jgit.test/...", "//org.eclipse.jgit.ui/...", "//org.eclipse.jgit/...", ], ) + +package_group( + name = "error_prone_packages_test", + packages = [ + "//org.eclipse.jgit.ant.test/...", + "//org.eclipse.jgit.gpg.bc.test/...", + "//org.eclipse.jgit.http.test/...", + "//org.eclipse.jgit.lfs.server.test/...", + "//org.eclipse.jgit.lfs.test/...", + "//org.eclipse.jgit.pgm.test/...", + "//org.eclipse.jgit.ssh.apache.test/...", + "//org.eclipse.jgit.ssh.jsch.test/...", + "//org.eclipse.jgit.test/...", + ], +) -- cgit v1.2.3