Matthias Sohn [Tue, 24 Sep 2024 08:51:22 +0000 (10:51 +0200)]
AdvertisedRequestValidator: fix WantNotValidException caused by race
Fetch with protocol V2 failed under the following conditions
- fetch uses bidirectional protocol (git, ssh) which uses a shortcut
to determine invalid wants
- not all wants are advertised
- race condition: wanted ref is updated during fetch by another thread
after the thread serving upload-pack determined wants and before it
checks not advertised wants
Fix this by calling
`new ReachableCommitRequestValidator().checkWants(up, wants)`
instead of throwing WantNotValidException in [1]
if this race happened in the same way like it's done for unidirectional
protocols (http) [2].
Matthias Sohn [Tue, 20 Aug 2024 13:20:37 +0000 (15:20 +0200)]
Merge branch 'stable-6.8' into stable-6.9
* stable-6.8:
Update tycho to 4.0.8
Update org.eclipse.dash:license-tool-plugin to 1.1.0
Fix "Comparison of narrow type with wide type in loop condition"
JGit v5.13.3.202401111512-r
Matthias Sohn [Tue, 20 Aug 2024 12:56:04 +0000 (14:56 +0200)]
Merge branch 'stable-6.7' into stable-6.8
* stable-6.7:
Update tycho to 4.0.8
Update org.eclipse.dash:license-tool-plugin to 1.1.0
Fix "Comparison of narrow type with wide type in loop condition"
JGit v5.13.3.202401111512-r
Matthias Sohn [Tue, 20 Aug 2024 12:54:08 +0000 (14:54 +0200)]
Merge branch 'stable-6.6' into stable-6.7
* stable-6.6:
Update tycho to 4.0.8
Update org.eclipse.dash:license-tool-plugin to 1.1.0
Fix "Comparison of narrow type with wide type in loop condition"
JGit v5.13.3.202401111512-r
Matthias Sohn [Tue, 20 Aug 2024 12:28:33 +0000 (14:28 +0200)]
Merge branch 'stable-6.5' into stable-6.6
* stable-6.5:
Update org.eclipse.dash:license-tool-plugin to 1.1.0
Fix "Comparison of narrow type with wide type in loop condition"
JGit v5.13.3.202401111512-r
Matthias Sohn [Sun, 18 Aug 2024 16:35:29 +0000 (18:35 +0200)]
Merge branch 'stable-6.4' into stable-6.5
* stable-6.4:
Update org.eclipse.dash:license-tool-plugin to 1.1.0
Fix "Comparison of narrow type with wide type in loop condition"
JGit v5.13.3.202401111512-r
Matthias Sohn [Fri, 9 Aug 2024 09:53:01 +0000 (11:53 +0200)]
Fix "Comparison of narrow type with wide type in loop condition"
This issue was detected by a GitHub CodeQL security scan run on JGit
source code.
Description of the error raised by the security scan:
"In a loop condition, comparison of a value of a narrow type with a
value of a wide type may always evaluate to true if the wider value is
sufficiently large (or small). This is because the narrower value may
overflow. This can lead to an infinite loop."
Fix this by using type `long` for the local variable `done`.
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.
Dariusz Luksza [Fri, 17 Nov 2023 19:28:53 +0000 (19:28 +0000)]
Add tests for handling pack files removal during fetch
Although this could sound like a corner case, it really can occur out
there in the real world. Especially in the Gerrit world where the
repositories could be GC'ed on a separate process or system.
The `FileNotFoundException` seems to be handled correctly in
`PackFile#doOpen` (line 671) and it will mark the pack as invalid. But
triggering that code path was not an easy task.
First of all, we need to add a new commit to the `master` branch of the
test repository after `UploadPack` object is created.
Secondly, in the refspec for fetch, commit id instead of "regular"
refspec must be used.
With both in place, we can see a warning log statement about deleted
pack file. And the fetch succeeds!
Also, tests for the removal of *.idx and *.bitmap files were added.
This unveiled a corner for the *.idx file deletion while fetching, as
the test will fail with "Unreachable pack index" IOException only
when the HEAD commit is empty.
Thomas Wolf [Wed, 10 Jan 2024 17:36:50 +0000 (18:36 +0100)]
RebaseCommand: fix stopping on root commit conflicts
If rebasing runs into a conflict when applying a root commit from an
independent branch, there is no parent commit. Write an empty patch
file in that case like C git does.
Bug: jgit-6
Change-Id: I315313673d2abf29639d7d96c958d599961ba276 Signed-off-by: Thomas Wolf <twolf@apache.org>
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.
Dmitrii Naumenko [Wed, 22 Nov 2023 18:11:17 +0000 (19:11 +0100)]
CherryPick: add ability to customise cherry-picked commit message
Originally I wanted to support a feature similar to `-x` options from
https://git-scm.com/docs/git-cherry-pick#_options.
The idea was to append original commit hash in this format:
```
my original commit message
(cherry picked from commit 75355897dc28e9975afed028c1a6d8c6b97b2a3c)
```
This can be useful information in some integrations.
I decided to make it in a more generic way
and pass custom `CherryPickCommitMessageProvider` implementation.
One of the two default implementations can append original commit hash
Matthias Sohn [Sat, 20 Jan 2024 00:03:52 +0000 (01:03 +0100)]
Merge branch 'stable-6.8'
* stable-6.8:
Introduce a PriorityQueue sorting RevCommits by commit timestamp
Remove org.eclipse.jgit.benchmark/.factorypath
Update jmh to 1.37 for org.eclipse.jgit.benchmark
Matthias Sohn [Fri, 19 Jan 2024 23:40:42 +0000 (00:40 +0100)]
Merge branch 'stable-6.7' into stable-6.8
* stable-6.7:
Introduce a PriorityQueue sorting RevCommits by commit timestamp
Remove org.eclipse.jgit.benchmark/.factorypath
Update jmh to 1.37 for org.eclipse.jgit.benchmark
Matthias Sohn [Fri, 19 Jan 2024 23:18:25 +0000 (00:18 +0100)]
Merge branch 'stable-6.6' into stable-6.7
* stable-6.6:
Introduce a PriorityQueue sorting RevCommits by commit timestamp
Remove org.eclipse.jgit.benchmark/.factorypath
Update jmh to 1.37 for org.eclipse.jgit.benchmark
Luca Milanesio [Mon, 13 Jun 2022 22:09:55 +0000 (23:09 +0100)]
Introduce a PriorityQueue sorting RevCommits by commit timestamp
The DateRevQueue uses a tailor-made algorithm to keep
RevCommits sorted by reversed commit timestamp, which has a O(n*n/2)
complexity and caused the explosion of the Git fetch times to
tens of seconds.
The standard Java PriorityQueue provides a O(n*log(n)) complexity
and scales much better with the increase of the number of
RevCommits.
Introduce a new implementation DateRevPriorityQueue of the DateRevQueue
based on PriorityQueue.
Enable usage of the new DateRevPriorityQueue implementation by setting
the system property REVWALK_USE_PRIORITY_QUEUE=true. By default the old
implementation DateRevQueue is used.
Dariusz Luksza [Fri, 17 Nov 2023 19:28:53 +0000 (19:28 +0000)]
Add tests for handling pack files removal during fetch
Although this could sound like a corner case, it really can occur out
there in the real world. Especially in the Gerrit world where the
repositories could be GC'ed on a separate process or system.
The `FileNotFoundException` seems to be handled correctly in
`PackFile#doOpen` (line 671) and it will mark the pack as invalid. But
triggering that code path was not an easy task.
First of all, we need to add a new commit to the `master` branch of the
test repository after `UploadPack` object is created.
Secondly, in the refspec for fetch, commit id instead of "regular"
refspec must be used.
With both in place, we can see a warning log statement about deleted
pack file. And the fetch succeeds!
Also, tests for the removal of *.idx and *.bitmap files were added.
This unveiled a corner for the *.idx file deletion while fetching, as
the test will fail with "Unreachable pack index" IOException only
when the HEAD commit is empty.
PackWriterBitmapPreparer: Set limit on excessive branch count
If there are too many branches then the bitmap
indexing selects only the tip commits of the least active
branches to reduce the amount of bitmaps to load on request.
This can still be a problem if the number of inactive branches
rival or exceed the total number of commits selected
for the active branches.
Limit the number of branches that receive only-tip bitmaps.
This reduces the memory pressure of loading all the bitmaps,
and allows us to model the size of the bitmap index without
considering the number of branches.
Bitmaps are generated for branches in order of most recent commit,
and follow these rules:
* The first {@code DEFAULT_BITMAP_EXCESSIVE_BRANCH_COUNT} most active
branches have full bitmap coverage.
* The {@code DEFAULT_BITMAP_EXCESSIVE_BRANCH_COUNT} to {@code
DEFAULT_BITMAP_EXCESSIVE_BRANCH_TIP_COUNT} most active branches have
only the tip commit covered.
* The remaining branches have no bitmap coverage.
To prevent effecting existing repositories, the default value is set
at Integer.MAX_VALUE.
projects / jgit.git / log