See
https://docs.oracle.com/en/java/javase/11/docs/api/java.base/java/util/concurrent/CopyOnWriteArrayList.html
https://errorprone.info/bugpattern/ModifyCollectionInEnhancedForLoop
Matthias Sohn [Sat, 16 Sep 2023 20:19:41 +0000 (22:19 +0200)]
[errorprone] AddCommand#filepattern: use a more specific type
Variable type can use a more specific type to convey more information to
callers.
private Collection<String> filepatterns;
^
(see https://errorprone.info/bugpattern/PreferredInterfaceType)
Ivan Frade [Fri, 8 Sep 2023 20:23:09 +0000 (13:23 -0700)]
CommitGraphWriter: Decouple Stats from computing bloom filters
The public stats object is created only to be populated by the computation of
bloom filters.
Make the computation return its numbers with the results and copy them
to the stats when needed. This eliminates the side effects from the
computation and makes it easier to add more data to the stats later.
Ronald Bhuleskar [Mon, 18 Sep 2023 18:45:11 +0000 (11:45 -0700)]
Documentation: Move writeChangedPaths flag from commitGraph to gc section.
The flag commitGraph.writeChangedPaths is not correct. The option was introduced in commit 3b77e33 (Jul 19th) under gc section so updating the documentation to reflect it under correct section.
Matthias Sohn [Fri, 15 Sep 2023 09:48:05 +0000 (11:48 +0200)]
[errorprone] Fix wrong comparison which always evaluated to false
org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/commitgraph/GraphObjectIndex.java:59:
error: [ComparisonOutOfRange] ints may have a value in the range
-2147483648 to 2147483647; therefore, this comparison to
Integer.MAX_VALUE will always evaluate to false
if (table[k] > Integer.MAX_VALUE) {
^
See https://errorprone.info/bugpattern/ComparisonOutOfRange
We need to check if variable `uint` of type `long` exceeds the maximum
possible int value before casting it to `int` below.
This was introduced in Ib5c0d6678cb242870a0f5841bd413ad3885e95f6
Matthias Sohn [Fri, 15 Sep 2023 09:44:09 +0000 (11:44 +0200)]
[errorprone] Remove unnecessary comparison
Raised by errorprone:
org.eclipse.jgit/src/org/eclipse/jgit/lib/CommitConfig.java:406: error:
[ComparisonOutOfRange] chars may have a value in the range 0 to 65535;
therefore, this comparison to 0 will always evaluate to true
if (ch >= 0 && ch < inUse.length) {
^
see https://errorprone.info/bugpattern/ComparisonOutOfRange
Matthias Sohn [Fri, 8 Sep 2023 20:57:05 +0000 (22:57 +0200)]
Use ShutdownHook to gracefully handle JVM shutdown
in all classes which already registered their own shutdown hook
- CloneCommand
- GC#PidLock
- FS#FileStoreAttributes
- LocalDiskRepositoryTestCase#Cleanup
Matthias Sohn [Fri, 8 Sep 2023 00:14:20 +0000 (02:14 +0200)]
Add ShutdownHook to cleanup FileLocks on graceful JVM shutdown
This should avoid stale lock files if the JVM is terminated gracefully.
Implement a ShutdownHook which can register/unregister listeners which
need to do some cleanup during graceful JVM shutdown. This hook is
registered as a Java shutdown hook and when the JVM shuts down
calls #onShutdown of registered listeners using a parallel stream
to let them run concurrently.
See https://docs.oracle.com/javase/8/docs/technotes/guides/lang/hook-design.html
Thomas Wolf [Mon, 11 Sep 2023 18:58:15 +0000 (20:58 +0200)]
OSGi: move plugin localization to subdirectory
OSGi can have its plugin localization at an arbitrary place; there is
no need to have it in a top-level plugin.properties file. In non-OSGi
environments having the files at the root level may mean that these
files clash with each other, or, as in the referenced bug, with some
third-party plug-in's plugin.properties, which may not even have
anything to do with localization.
Move our OSGi localization to a subfolder OSGI-INF/l10n. For OSGi
environments, that's just as good, and for non-OSGi environments it
avoid clashes with other root level items on the classpath or in a fat
JAR.
For fragments, use neither plugin.properties (which would clash with the
host plug-in's plugin.properties) nor fragment.properties (which might
clash with other fragments for the same fragment host bundle). Instead
use names "relative" to the host bundle.
Bug: 582394
Change-Id: Ifbcd046d912e2cfe86c0f7259c5ca8de599d9aa1 Signed-off-by: Thomas Wolf <twolf@apache.org>
Jonathan Nieder [Tue, 12 Sep 2023 00:12:17 +0000 (17:12 -0700)]
ssh: Remove redundant null check for home.getAbsoluteFile()
File#getAbsoluteFile is non-nullable, so this check can never trigger.
Worse, getAbsoluteFile can throw an exception such as
InvalidPathException, and since this call isn't in the "try" block
that checks for that, the exception would then escape the getSession
call.
Noticed because the exception is being thrown in googlesource.com's
custom SshdSessionFactory, causing incoming ssh requests to fail
(noticed using internal tests).
Change-Id: I57f2d5e497ff678b17573f79827b6e1d9a6c9b9f Signed-off-by: Jonathan Nieder <jrn@google.com>
Ivan Frade [Fri, 8 Sep 2023 15:35:47 +0000 (08:35 -0700)]
DfsPackFile: Record index loads only in one place
Each index can be set in the reader from two locations: the dfs cache
callback or the code afterwards. The pack is emitting the load event
in both cases, when the reference is set. This is brittle (right now
it is missing events for BITMAP_INDEX and COMMIT_GRAPH).
Emit the index loaded event only once, after going through the cache
code. The fact that the reference was set in the callback or the main
code is irrelevant. Also, the reader is per-thread, so there shouldn't
be any concurrency involved triggering double counts.
Ivan Frade [Fri, 8 Sep 2023 15:20:31 +0000 (08:20 -0700)]
DfsPackfile: Emit the index load with the index object
The DfsReader#emitIndexLoad expects a reference to the loaded object,
not to its identity hash. This makes listeneres recalculate an
identity hash over an Int object.
Matthias Sohn [Thu, 7 Sep 2023 00:23:57 +0000 (02:23 +0200)]
Merge branch 'stable-6.7'
* stable-6.7:
Prepare 6.7.1-SNAPSHOT builds
Switch back to wagon-ssh-external
Fix generation of maven site
JGit v6.7.0.202309050840-r
Remove unused API problem filters
[releng] Bump japicmp base version to 6.6.0.202305301015-r
Fix list of 3rd party bundles in p2 repo
Add missing source bundle org.osgi.service.cm.source to target platform
Prepare 6.6.2-SNAPSHOT builds
JGit v6.6.1.202309021850-r
Checkout: better directory handling
Document commit-graph options supported by JGit
Fix warning raised for local variable hiding DfsPackFile#index
Suppress boxing warnings in DfsPackFile
Remove unused API problem filters
Suppress boxing warnings in tests
Fix warning about empty block
Update mockito to 5.5.0
Update byte-buddy to 1.14.7
Prepare 6.7.0-SNAPSHOT builds
JGit v6.7.0.202308301100-rc1
Thomas Wolf [Fri, 11 Aug 2023 19:40:13 +0000 (21:40 +0200)]
Checkout: better directory handling
When checking out a file into the working tree ensure that all parent
directories of the file below the working tree root are actually
directories and do exist before we try to create the file.
When multiple files are to be checked out (or even a whole tree), this
may check the same directories over and over again. Asking the file
system every time for file attributes is a potentially expensive
operation. As a remedy, introduce an in-memory cache of directory
states for a particular check-out operation.
Apply the same fix also in the ResolveMerger, which may also check out
files, and also in the PatchApplier. In PatchApplier, also validate
paths.
Change-Id: Ie12864c54c9f901a2ccee7caddec73027f353111 Signed-off-by: Thomas Wolf <twolf@apache.org> Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
Ivan Frade [Fri, 1 Sep 2023 18:25:50 +0000 (11:25 -0700)]
CommitGraphWriter: throw exception on unknown chunk
CommitGraphWriter first defines the chunks and then writes them. If at
write time a chunk is unknown, it is ignored. This is brittle: if
somebody adds a chunk to the header but not to the actual writing, the
commit-graph is broken and there is no error reported anywhere.
Throw exception if at write time a chunk is unknown. This can only
happen by a coding error in the writer.
Matthias Sohn [Wed, 30 Aug 2023 14:43:08 +0000 (16:43 +0200)]
Merge branch 'master' into stable-6.7
* master:
Remove the cbi-snapshots Maven repository
Update Orbit to orbit-aggregation/release/4.29.0
Add target platform for Eclipse 2023-09 (4.29)
Use release p2 repo for Eclipse 2023-06 (4.28)
Update tycho to 4.0.2
Update jmh to 1.37
Update bouncycastle to 1.76
Fix some tests in ConfigTest
Handle global git config $XDG_CONFIG_HOME/git/config
IO: use JDK convenience methods
org.eclipse.jgit.junit.ssh/.settings/.api_filters: fix unclosed tags
ReadChangedPathFilter: fix Non-externalized string literal warning
Introduce core.packedIndexGitUseStrongRefs config key
DfsReader: Make PackLoadListener interface visible to subclasses
DfsGarbageCollector: provide commit graph stats
DfsGarbageCollector: put only GC commits into the commit graph
DfsReader: Expose when indices are loaded
Thomas Wolf [Wed, 5 Jul 2023 20:21:30 +0000 (22:21 +0200)]
Handle global git config $XDG_CONFIG_HOME/git/config
C git uses this alternate fallback location if the file exists and
~/.gitconfig does not. Implement this also for JGit.
If both files exist, reading behavior is as if the XDG config was
inserted between the HOME config and the system config. Writing
behaviour is different: all changes will be applied only in the HOME
config. Updates will occur in the XDG config only if the HOME config
does not exist.
This is consistent with the behavior of C git; compare [1], especially
the sections on FILES and SCOPES, and the description of the --global
option.
[1] https://git-scm.com/docs/git-config
Bug: 581875
Change-Id: I2460b9aa963fd2811ed8a5b77b05107d916f2b44 Signed-off-by: Thomas Wolf <twolf@apache.org>
Introduce a core.packedIndexGitUseStrongRefs configuration key, which
defaults to true so that the current behavior does not change. However,
setting it to false allows soft references to be used for Pack indices
instead of strong references so that they can be garbage collected when
there is memory pressure.
Pack objects can be large when associated with pack files with large
object counts, and this memory is not really accounted for or tracked by
the WindowCache and it can be very substantial at times, especially with
many large object count projects. A particularly problematic use case is
Gerrit's ls-projects command which loads very little data in the
WindowCache via ByteWindows, but ends up loading and holding many entire
indices in memory, sometimes even after the ByteWindows for their Pack
objects have already been garbage collected since they won't get cleared
until after a new ByteWindow is loaded. By using SoftReferences, single
use indices can get cleared when there is memory pressure and OOMs can
be easily avoided, drastically reducing the amount of memory required to
perform an ls-projects on large sites with many projects and large
object counts.
On one of our test sites, an ls-projects command with strong index
references requires more than 66GB of heap to complete successfully,
with soft index references it requires less than 23GB.
Change-Id: I3cb3df52f4ce1b8c554d378807218f199077d80b Signed-off-by: Martin Fick <quic_mfick@quicinc.com> Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
Ivan Frade [Wed, 16 Aug 2023 20:26:39 +0000 (13:26 -0700)]
DfsGarbageCollector: put only GC commits into the commit graph
GC puts all commits reachable from heads and tags into the GC pack,
and commits reachable only from other refs (e.g. refs/changes) into
GC_REST. The commit-graph contains all commits in GC and GC_REST. This
produces too big commit graphs in some repos, beating the purpose of
loading the index.
Limit the commit graph to commits reachable from heads and tags
(i.e. commits in the GC pack).
Ivan Frade [Fri, 28 Jul 2023 09:41:26 +0000 (11:41 +0200)]
DfsReader: Expose when indices are loaded
We want to measure the data used to serve a request. As a first step,
we want to know how many indices are accessed during the request and
their sizes.
Expose an interface in DfsReader to announce when an index is loaded
into the reader, i.e. when its reference is set.
The interface is more flexible to implementors (what/how to collect)
than the existing DfsReaderIOStats object.