Sam Delmerico [Tue, 9 Apr 2024 23:56:38 +0000 (16:56 -0700)]
PackBitmapIndex: clarify naming of getObject inputs
The documentation for the getObject function seems to be a little
outdated. This commit clarifies that this function accepts an offset
based on the ordering of objects in the pack.
The previous code computed the boundaries of the base part of
conflict hunks using line numbers computed for the 'ours' and
'theirs' revisions, leading to incorrect boundaries.
This only affects the presentation of merge conflicts in diff3
mode (which is probably not used a lot yet on Gerrit).
Successful merges are unaffected.
Thomas Wolf [Sat, 9 Mar 2024 23:04:41 +0000 (00:04 +0100)]
[gpg] Correct finding public keys from pubring.gpg
With a master key not enabled for signing, and a signing sub-key, key
lookup went wrong in several ways and might not find a suitable key for
signing or for signature verification. Fix the code so that it finds
the sub-key, even if user.signingKey is specified not with a key ID but
with an with an e-mail. (Sub-keys don't have user ids, those are
attached only on the master key.)
Change-Id: I9d1f641c49b173d4daffb3fd2e74f5aabd856e39 Signed-off-by: Thomas Wolf <twolf@apache.org>
The S-expression parser from Bouncy Castle parsed such keys wrongly;
there is a "flags" sub-list before the "q" value. Additionally, the
parser validates the key read against the given public key, this failed
because Bouncy Castle does not know the OID of curve name "Ed25519".
Fix this and add a test for reading an ed25519 GPG key.
Bug: jgit-27
Change-Id: Ia50445b88759927d2e80b9871d498fbe5ad201bc Signed-off-by: Thomas Wolf <twolf@apache.org>
Sam Delmerico [Wed, 27 Mar 2024 19:09:42 +0000 (12:09 -0700)]
PackBitmapIndexBuilder: allow repeated call of getCompressedBitmaps()
Each call of the current implementation of
PackBitmapIndexBuilder.getCompressedBitmaps reverses the class member that is returned. So two subsequent calls of this function will
receive reversed structures.
Instead we should just reverse a copy of the structure. This way the
PackBitmapIndexBuilder can continue to efficiently accept new bitmaps
via processBitmapForWrite (because we can continue to add to the back of
the bitmapsToWrite ArrayList) while ensuring that the bitmaps are
returned in the correct ordering from getCompressedBitmaps.
Ivan Frade [Mon, 1 Apr 2024 19:58:27 +0000 (12:58 -0700)]
DfsPackFile: make public the constructor with bitmap loader
The constructor with the bitmap loader as a parameter is supposed to
be used from subclasses in the ObjectDatabase, but those subclasses
cannot see it with its current package-private visibility.
Sam Delmerico [Mon, 25 Mar 2024 17:52:26 +0000 (10:52 -0700)]
PackBitmapIndex: convert from class to interface
This abstract class can be an interface after
https://review.gerrithub.io/c/eclipse-jgit/jgit/+/1184614 moved the
packChecksum data behind a getter interface. This also allows easier
overriding of this interface with custom types.
Nasser Grainawi [Fri, 23 Feb 2024 17:34:37 +0000 (09:34 -0800)]
Cache refreshed loose ref dirs in SnapshottingRefDirectory
Update SnapshottingRefDirectory to have a cache of dirs refreshed for
loose refs. This should help improve performance when 'after_open'
setting is used for 'trustLooseRefStat' as duplicate refreshes are
avoided when a snapshot of the ref database is used in a request scope.
Change-Id: I8f66e7cee572e477d29abe2d9db69e97bca3ee4c Signed-off-by: Nasser Grainawi <quic_nasserg@quicinc.com> Co-authored-by: Martin Fick <quic_mfick@quicinc.com> Co-authored-by: Kaushik Lingarkar <quic_kaushikl@quicinc.com>
Sam Delmerico [Tue, 19 Mar 2024 00:14:24 +0000 (17:14 -0700)]
PackBitmapIndex: hide packChecksum behind getter
This allows more flexibility in overriding the PackBitmapIndex and also
allows us to turn PackBitmapIndex into an interface in
https://review.gerrithub.io/c/eclipse-jgit/jgit/+/1178201.
Xing Huang [Thu, 7 Mar 2024 17:41:36 +0000 (11:41 -0600)]
TreeRevFilter: correct changedPathFilter usage for multi-paths inclusion
The expected behavior of TreeRevFilter when filtering multiple file
paths is to include commits that changed at least one of the given
paths; only skipping them if they did not change any of the given
paths. The current changedPathFilter utilization logic is skipping a
commit if there exists at least one given path that the commit did not
change, disregarding the rest of the given paths.
Enforcing all given paths to be checked by the changedPathFilter, only
skipping a commit if changedPathFilter return negative on all given
paths.
Xing Huang [Fri, 15 Mar 2024 20:19:15 +0000 (15:19 -0500)]
PathFilterGroup: implement getPathsBestEffort()
getPathsBestEffort() is a method in the TreeFilter class
to retrieve file paths specified by the caller. PathFilterGroup do not
propagate the paths of their subfilters as it does not implement the
getPathsBestEffort() method, resulting in the caller only getting an
empty list of paths.
Override getPathsBestEffort() in PathFilterGroup to propagate subfilter
values.
Thomas Wolf [Sun, 10 Mar 2024 18:37:51 +0000 (19:37 +0100)]
CleanupService: preload JgitText if not running in OSGi
Load class JGitText eagerly when ShutdownHook is used. If not running
in an OSGi framework, the hook executes as a JVM shutdown hook. Normally
classloading in a shutdown hook should not be a problem, but there are
some other frameworks besides OSGi that may close classloaders before
such a JVM shutdown hook runs, for instance Plexus classworlds used in
maven.
This is not a full fix for classloading problems in JGit shutdown hooks.
If a listener causes additional classes to be loaded, there may still
be problems. But the current listeners registered with ShutdownHook all
appear to use only classes that should already be loaded; except
JGitText.
Bug: jgit-36
Change-Id: Iab00134b4720adb19e1394c38ad79f1c43c3466b Signed-off-by: Thomas Wolf <twolf@apache.org>
Xing Huang [Tue, 12 Mar 2024 21:24:10 +0000 (16:24 -0500)]
CommitGraphLoader: receive readChangedPaths as parameter
commitGraph.readChangedPaths controls the read of bloom filter
chunks from commit graph. CommitGraphLoader uses SystemReader
to read this conf, so it only sees its value when it is set in
$XDG_CONFIG_HOME/jgit/config (and not, for example, in the repo
config).
Pass the readChangedPaths value to the method that reads the commit
graph. Callers should read the value from the right configuration.
This change is a noop, but allows to move callers to the new method
progressively. Follow up changes will remove the previous method and
its usages.
Thomas Wolf [Wed, 21 Feb 2024 21:52:45 +0000 (22:52 +0100)]
FS_POSIX.runInShell(): on MacOS use a login shell
On Mac, $PATH for UI programs is not the same as in a shell (terminal).
It is typically much shorter. This may lead to surprises when hooks that
work fine via the command-line git do not work when run via JGit in a UI
application, such as EGit in Eclipse.
Therefore use a login shell to run hooks and other commands spawned by
git if we're on MacOS. This will give the hooks the same environment as
in a terminal.
Bug: egit-16
Change-Id: Id2e5485c6d3080d3ef8baa61ad7f6f198f77c590 Signed-off-by: Thomas Wolf <twolf@apache.org>
Thomas Wolf [Sat, 3 Feb 2024 21:22:16 +0000 (22:22 +0100)]
[ssh] Implement the "Ciphers" SSH config
Upstream will remove the CBC algorithms aes128-cbc, aes192-cbc, and
aes256-cbc from the server's KEX proposal in the next release. Removal
of these algorithms by default in the client is planned for the release
after that. These CBC algorithms were found vulnerable back in 2008,[1]
and OpenSSH does not propose them: server-side since 2014, client-side
since 2017.
It is _highly_ unlikely that the removal of these algorithms by default
would affect any JGit user. Nevertheless, let's give users a way to
explicitly specify ciphers (including enabling deprecated algorithms)
via their ~/.ssh/config file.
[1] https://www.kb.cert.org/vuls/id/958563
Change-Id: I7444861df3a7f526277fef2485773a20ac74ae8a Signed-off-by: Thomas Wolf <twolf@apache.org>
Thomas Wolf [Fri, 8 Mar 2024 18:48:27 +0000 (19:48 +0100)]
ShutdownHook: run on bundle deactivation if in OSGi
Running as a JVM shutdown hook is far too late in an OSGi framework; by
the time the JVM shuts down, the OSGi framework will normally already
have deactivated and unloaded bundles, and thus the JGit cleanup code
may try to work with unloaded classes for which there will be no
classloader anymore.
When JGit is used in an OSGi framework, the cleanups must run on bundle
deactivation, not on JVM shut down.
Add a declarative OSGi service CleanupService. This is a normal Java
class that has no dependencies on any OSGi bundle or interface, but
that is declared in the MANIFEST.MF and in an OSGi Service XML as an
OSGi immediate component. Set the bundle activation policy to "lazy".
(A declarative service is used instead of a bundle activator because the
latter would need to implement the OSGi interface BundleActivator, but
JGit should not have dependencies on OSGi.)
When JGit runs in an OSGi framework, the framework will create an
instance of CleanupService through the no-args constructor when (and
before) the first class from this bundle is loaded. This instance thus
knows that it is operating in OSGi, and will run the ShutdownHook when
the bundle is deactivated: bundle deactivation will deactivate the
CleanupService instance.
When JGit does not run in an OSGi framework, the OSGi service
declaration will be ignored, and there will be no already existing
CleanupService instance. We create one lazily, which thus knows that
it is not operating in OSGi, and which will use a JVM shutdown hook to
run the ShutdownHook.
Laura Hamelin [Tue, 27 Feb 2024 17:40:24 +0000 (09:40 -0800)]
DfsBlockCache: move cache table specific implementations to a new class
This move of the cache table specific implementation to its own class
and extracting the method definition to an interface will allow for
additional reworking of the dfs block cache with the goal of letting
users implement their own context specific cache tables.
This work does not intend to change how the dfs block cache is accessed,
limiting the change to DfsBlockCache internal changes only.
Change-Id: Ief6755d1dcf54b4f73f9fe8d22ccb9e2952cb0c4 Signed-off-by: Laura Hamelin <haowl@google.com>
With repositories on NFS, JGit can read an old value of a loose ref
or miss the existence of a loose ref if file attributes of the loose
ref or its parent directories are cached by NFS. Introduce a new config
'core.trustLooseRefStat' that will optionally refresh file attributes of
the loose ref (at least on some NFS clients).
Possible values for this new config are:
* always: Trust loose ref file attributes (default)
* after_open: Similar to 'always', but refresh the file attributes of
the loose ref and its parent directories before trusting
it
The default is set to always trust the file attributes as after_open is
known to degrade performance.
In a subsequent change, SnapshottingRefDirectory will be updated to
cache the directories that were refreshed to avoid duplicate work and
thereby improve performance to some extent for the after_open setting.
Matthias Sohn [Wed, 28 Feb 2024 18:43:26 +0000 (19:43 +0100)]
Merge branch 'master' into stable-6.9
* master:
Update SECURITY.md
DfsObjDatabase: Let object database instantiate DfsPackFiles
DfsPackFile: Abstract the bitmap loading to support other backends
Remove unused API problem filters
Support public key in IdentityFile
Revert "StartGenerator: Fix parent rewrite with non-default RevFilter"
DfsReader#getObjectSize: use size index if possible
Ivan Frade [Thu, 22 Feb 2024 22:15:10 +0000 (14:15 -0800)]
DfsObjDatabase: Let object database instantiate DfsPackFiles
DfsPackfile used to have only one constructor and it is invoked from 3
locations. Now we can construct DfsPackFiles with different bitmap
loaders, so it is helpful to concentrate the instantiation in one location.
Create DfsPackFile instances in the object database. This let
subclasses choose how to initialize those instances.
Ivan Frade [Wed, 21 Feb 2024 23:06:38 +0000 (15:06 -0800)]
DfsPackFile: Abstract the bitmap loading to support other backends
Current code reads the bitmap index from the pack extension and loads
all bitmaps into memory, with its IO and memory cost. We could
consider to store the bitmaps on e.g. a database and load them on
demand.
Abstract the loading of the PackBitmapIndex in an interface that can
be implemented with other backends.
Konrad Windszus [Tue, 20 Feb 2024 13:30:39 +0000 (14:30 +0100)]
Support public key in IdentityFile
Public keys in the SSH configuration's "IdentityFile" are used for
example by Password Managers like KeePassXC
(https://keepassxc.org/docs/#faq-ssh-agent-auth-errors) or 1Password
(https://developer.1password.com/docs/ssh/agent/advanced/#match-key-with-host)
to match keys in the SSH agent with specific hosts.
Luca Milanesio [Wed, 10 Jan 2024 19:38:46 +0000 (19:38 +0000)]
Allow to discover bitmap on disk created after the packfile
When the bitmap file was created *after* a packfile had been
loaded into the memory, JGit was unable to discover them.
That happed because of two problems:
1. The PackDirectory.getPacks() does not implement the usual
while loop that is scanning through the packs directory
as in the other parts of JGit.
2. The scan packs does not look for newly created bitmap files
if the packfile is already loaded in memory.
Implement the normal packfiles scanning whenever the PackDirectory
needs to return a list of packs, and make sure that any reused
Pack object would have its associated bitmap properly refreshed
from disk.
Adapt the assertions in GcConcurrentTest with the rescanned list
of Pack from the objects/packs directory.
Ivan Frade [Tue, 20 Feb 2024 20:43:45 +0000 (12:43 -0800)]
DfsReader#getObjectSize: use size index if possible
getObjectSize reads the size from the first bytes of the object in the
pack, using IO. For blobs and depending on the configuration, the size
could be available in the object size index.
Try to get the size from the object size index if available. Read from
the pack otherwise.
Note that GC uses #getObjectSize from the existing pack to write the
next object size index.
Matthias Sohn [Tue, 20 Feb 2024 17:47:48 +0000 (18:47 +0100)]
Update maven plugins
- build-helper-maven-plugin to 3.5.0
- cyclonedx-maven-plugin to 2.7.11
- eclipse-jarsigner-plugin to 1.4.3
- git-commit-id-maven-plugin to 7.0.0
- japicmp-maven-plugin to 0.18.5
- maven-clean-plugin to 3.3.2
- maven-compiler-plugin to 3.12.1
- maven-enforcer-plugin to 3.4.1
- maven-jxr-plugin to 3.3.2<
- maven-site-plugin to 4.0.0-M13
- spotbugs-maven-plugin to 4.8.3.1
- tycho plugins to 4.0.6
Nasser Grainawi [Tue, 14 Nov 2023 20:55:48 +0000 (13:55 -0700)]
storage.file: Do not throw NSFE when deleting tmp files
If we try to delete a tmp file and it already doesn't exist, then
there's no problem and we don't need to throw an exception. This is
especially true for these existing cases using FileUtils.RETRY because
any missing files on retries are already considered successes. Not
throwing the exception quiets some logs and improves overall exception
handling.
Motiejus Jakštys [Thu, 28 Dec 2023 14:33:28 +0000 (16:33 +0200)]
FS.detectSymlinkSupport: fix a race
When multiple JGit clients are instantiated concurrently, they may try
to create the same symlink at the same time. When that happens, the
second thread will return an error (because the symlink already exists)
and that `FS` instance will think that symlinks are not supported,
causing havoc.
Change-Id: I362b933ff63a1471e3a5d70cc8c35eb2f25cc0dd Signed-off-by: Motiejus Jakštys <motiejus@jakstys.lt>
Matthias Sohn [Mon, 19 Feb 2024 23:13:18 +0000 (23:13 +0000)]
Merge changes I8e577a23,Ic8e8b288
* changes:
Errorprone EscapedEntity: Do not use HTML entities inside @code javadoc
ChangedPathFilter: Suppress warning about backing array (errorprone)
Matthias Sohn [Mon, 19 Feb 2024 23:02:20 +0000 (23:02 +0000)]
Merge changes I9225d6a3,I9cf093ca,Id5a80635
* changes:
RawParseUtils.nextLfSkippingSplitLines: fulfil contract as stated
[gpg] Refactor the GpgSignatureVerifier
RawParseUtils: utility method to get a header value
Dariusz Luksza [Thu, 15 Feb 2024 10:48:55 +0000 (10:48 +0000)]
Merge branch 'stable-6.8' into master
* stable-6.8:
RefDirectory: Do not unlock until after deleting loose ref
Add missing javadoc description for declared exception
SnapshottingRefDirectory: Invalidate snapshot after locking ref for update
SnapshottingRefDir: Replace lambas with method refs
SnapshottingRefDir: Reduce casts with overrides
[errorprone] Fix wrong comparison which always evaluated to false
[errorprone] Remove unnecessary comparison
Dariusz Luksza [Thu, 15 Feb 2024 10:43:11 +0000 (10:43 +0000)]
Merge branch 'stable-6.7' into stable-6.8
* stable-6.7:
RefDirectory: Do not unlock until after deleting loose ref
Add missing javadoc description for declared exception
SnapshottingRefDirectory: Invalidate snapshot after locking ref for update
SnapshottingRefDir: Replace lambas with method refs
SnapshottingRefDir: Reduce casts with overrides
[errorprone] Fix wrong comparison which always evaluated to false
[errorprone] Remove unnecessary comparison
Dariusz Luksza [Thu, 15 Feb 2024 10:36:41 +0000 (10:36 +0000)]
Merge branch 'stable-6.6' into stable-6.7
* stable-6.6:
RefDirectory: Do not unlock until after deleting loose ref
Add missing javadoc description for declared exception
SnapshottingRefDirectory: Invalidate snapshot after locking ref for update
SnapshottingRefDir: Replace lambas with method refs
SnapshottingRefDir: Reduce casts with overrides
Nasser Grainawi [Fri, 26 Jan 2024 01:59:15 +0000 (18:59 -0700)]
RefDirectory: Do not unlock until after deleting loose ref
Fix a potential race condition where we would remove our loose ref lock
file before deleting the loose ref itself. This race could result in the
current thread deleting a loose ref newly written by another thread.
Other callers seem to be following the correct pattern, but improve the
method naming to try to help future callers.
Nasser Grainawi [Thu, 25 Jan 2024 23:29:05 +0000 (16:29 -0700)]
SnapshottingRefDirectory: Invalidate snapshot after locking ref for
update
When using the SnapshottingRefDirectory, if a thread has already read
packed-refs, then another actor updates packed-refs, the original
thread may create an update that is based on the old cached/snapshotted
packed-refs content. That update could effectively perform a forced
update unintentionally because it is unaware of the new content.
This seems particularly likely to happen in a scenario where a loose
ref was just packed. If the ref was loose, our thread would see the
current ref value (because we don't snapshot loose refs and always read
them from disk), but since there is no loose ref, we expect to find the
current value in packed-refs. However, (before this change) we rely
on our snapshot of packed-refs which does not contain the updated ref
value.
Invalidating the cache after the loose ref is locked ensures that the
ref value does not change again before we read it to perform the update.
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
Dariusz Luksza [Mon, 12 Feb 2024 10:14:29 +0000 (10:14 +0000)]
Merge branch 'stable-6.8' into master
* stable-6.8:
Improve handling of NFS stale handle errors
Fix handling of missing pack index file
Add tests for handling pack files removal during fetch
Dariusz Luksza [Mon, 12 Feb 2024 09:58:41 +0000 (09:58 +0000)]
Merge branch 'stable-6.7' into stable-6.8
* stable-6.7:
Improve handling of NFS stale handle errors
Fix handling of missing pack index file
Add tests for handling pack files removal during fetch
Dariusz Luksza [Mon, 12 Feb 2024 09:56:36 +0000 (09:56 +0000)]
Merge branch 'stable-6.6' into stable-6.7
* stable-6.6:
Improve handling of NFS stale handle errors
Fix handling of missing pack index file
Add tests for handling pack files removal during fetch
PackWriterBitmapPreparer: Do not generate bitmaps for excessive branch tips.
In https://review.gerrithub.io/c/eclipse-jgit/jgit/+/1174407, I changed the bitmap selection algorithm to cap the number of branches included in the bitmap.
There was a bug in that change. When the selection algorithm iterates
over the commits in a branch B, it automatically includes a bitmap for
the tip of any other branch C that happens to be on B. So even though we never iterated over C, we would still index the tip commit of C.
Keep a list of the "excessive" branch tips and check that the commit is not in there before generating a bitmap. We only skip tips that would be selected as a result of being a tip. If it would be selected for some other reason (e.g. it is one of the first 100 commits in master) then it is not skipped.
Thomas Wolf [Thu, 1 Feb 2024 18:09:50 +0000 (19:09 +0100)]
[gpg] Refactor the GpgSignatureVerifier
Add a new method verify(GpgConfig, byte[], byte[]) and deprecate the
existing verify(byte[], byte[]). Some implementations of the interface
may need the GpgConfig.
Factor out extracting the raw armored signature from commits or tags
into an abstract AbstractGpgSignatureVerifier class so that different
implementations don't have to re-implement that bit. Call the new verify
method, passing along the GpgConfig.
This makes the GPG interfaces more versatile and facilitates
implementing an alternate GpgSignatureVerifier.
Dariusz Luksza [Mon, 20 Nov 2023 11:53:19 +0000 (11:53 +0000)]
Improve handling of NFS stale handle errors
Mark packfile as invalid when NFS stale handle error occurs.
This should fix broken fetch operations when the repo is located on the
NFS system and is GC'ed on a separate system (or process). Which may
result in the index, pack or bitmap file being removed when they are
accessed from the fetch operation.
Dariusz Luksza [Mon, 20 Nov 2023 11:00:51 +0000 (11:00 +0000)]
Fix handling of missing pack index file
As demonstrated in
`UploadPackHandleDeletedPackFile.testV2IdxFileRemovedDuringUploadPack`
the fetch operation will fail when the pack index file is removed.
This is due to a wrapping of `FileNotFoundException` (which is a
subclass of `IOExeption`) in an `IOException` at PackIndex L#68. This
is then changing the behaviour of error handling in
`Pack.file.getBitmapIndex()` where the `FileNotFoundException` is
swallowed and allows the fetch process to continue. With FNFE being
wrapped in IOE, this blows up and breaks the fetch operation.
Simply rethrowing `FileNotFoundException` from `PackFile.open()` fixes
the broken fetch operation. This will also mark the whole pack as
invalid in the `IOException` handler in `Pack.idx()` method.
projects / jgit.git / log