Ivan Frade [Thu, 16 May 2024 19:28:53 +0000 (12:28 -0700)]
WalkFetchConnection: Remove marked packs on all function exits
[1] replaces Iterator.remove() with a list of "toRemove" that gets
processed when returning at the end. There are two others returns in
the function where the list is not processed.
Let the method report the broken packages and wrap it so the caller
can clean them up in any case.
In https://gerrithub.io/c/eclipse-jgit/jgit/+/1194015, LinkedList was
replaced with ArrayList in DfsReader and WalkFetchConnection. In this
case, the Iterator.remove() method of List is called, which is an O(n)
operation for ArrayList. This results in an O(n^2) algorithm.
Instead of reverting to LinkedList, use a HashSet and LinkedHashmap
instead. This maintains O(1) removal, and is less likely to be treated
as an antipattern than LinkedList.
A likely innocuous usage of Iterator.remove() in UnionInputStream was
also fixed.
Patrick Hiesel [Fri, 10 Mar 2023 15:50:37 +0000 (16:50 +0100)]
Allow applying a patch with conflicts
In some settings, we want to let users apply a patch that does
not cleanly apply and add conflict markers. In Gerrit, this is
useful when cherry picking (via Git patches) from one host to
another.
This commit takes a simple approach: If a hunk doesn't apply,
go to the pre-image line, treat all lines in pre-image length
as left side of the conflict and all context and newly added
lines as right side of the conflict.
Thomas Wolf [Mon, 6 May 2024 17:32:12 +0000 (19:32 +0200)]
sshd: fix IdentiesOnly if SSH agent is enabled and has keys
Commit a44b9e8bf changed the logic so that we try to read a public key
from the file given first, and only then try the file with the ".pub"
extension. Unfortunately the exception handling was not sufficient to
correctly deal with the given file containing a private key.
Apache MINA SSHD may throw a StreamCorruptedException when one tries
to read a public key from a file containing a private key. Handle
this exception in addition to GeneralSecurityException, and change
the order of exception handlers because StreamCorruptedException is
an IOException.
Bug: jgit-53
Change-Id: I7dddc2c11aa75d7663f7fe41652df612bf8c88cd Signed-off-by: Thomas Wolf <twolf@apache.org>
Yury Molchan [Wed, 6 Dec 2023 02:29:39 +0000 (04:29 +0200)]
Check an execution bit by reading it from the file system
Files.isExecutable() checks possibility to execute a file from the JVM,
not POSIX attributes. So it fails when Java Security Manager does not
allow to execute a file, but file has X-bit.
Ivan Frade [Thu, 28 Mar 2024 22:07:34 +0000 (15:07 -0700)]
PackIndex: Make iterator static (pass object count in ctor)
The iterator uses an implicit reference to the external class to
call #getObjectCount(). This implicit reference prevents PackIndex to
become an interface. The object count is immutable and known at
index (and iterator) construction time.
Pass the object count in the constructor of the iterator and make it
static.
Ivan Frade [Thu, 28 Mar 2024 21:55:09 +0000 (14:55 -0700)]
PackIndex: move checksum to the subclasses
PackIndex is almost an interface, and making it so simplifies writing
implementations over other storages. Checksum and its getter is the
only functionality that is class specific.
Make getChecksum abstract and implement it in the subclasses.
David Ostrovsky [Tue, 17 Oct 2023 07:47:35 +0000 (09:47 +0200)]
Bazel: Add support for JDK 21
There are two failing tests when switching to JDK 21.
One failure is related to the changed behaviour related to the locale
providers. Adapt `GitDateFormatterTest` to changes in unicode [1].
Second failure related to changed behaviour in URL.openConnection(),
see: [2] for more details.
Before JDK 20, some of the parsing/validation performed by the JDK
built-in URLStreamHander implementations were delayed until
URL::openConnection or URLConnection::connect was called. Starting
JDK 20, some of these parsing/validations are now performed early,
i.e. within URL constructors.
IOW, the assumption made in HttpSupport.TesttestMalformedUri() isn't
met any more: providing mailformed URI to the URL ctor now throws an
exception starting with JDK 20. To rectify the problem, remove the
offending test.
Test plan:
To build with JDK 21 and run the tests locally:
$> bazel test --config=java21 //...
To build with JDK 21 and run the tests on RBE:
$> bazel test --config=remote21 --remote_instance_name=$PROJECT //...
Matthias Sohn [Mon, 29 Apr 2024 13:24:02 +0000 (15:24 +0200)]
Fix warning about using raw type
This fixes the warning in UploadPackHandleDeletedPackFileTest. line 116:
"Type safety: The method register(Object, Repository) belongs to the raw
type TestProtocol. References to generic type TestProtocol<C> should be
parameterized."
David Ostrovsky [Sun, 28 Apr 2024 12:30:56 +0000 (14:30 +0200)]
Bazel: Update RBE docker image to Ubuntu 22.04 and JDK 17
In this change we update the JDK used in RBE to JDK 17.
Use Ubuntu 22.04 docker image with JDK 17 from Bazel project as the new
base image. So that we don't need to maintain our own docker image any
more.
Also align the naming convention for the docker images with the Bazel
project: <OS version>-<JDK version>, and remove "rbe"-prefix from the
docker image name, because the image doesn't have any RBE specifics and
could be used for Gerrit build and tests outside of the RBE pipeline.
Ivan Frade [Fri, 26 Apr 2024 21:04:08 +0000 (14:04 -0700)]
PackIndex: Make #getOffset protected to allow out-of-package subclasses
PackIndex#getOffset is abstract and package-private, limiting
subclasses to the same package. Given than the class and all other
relevant methods are public, this looks like an oversight and it
should be ok to write subclasses in other packages.
Make the method protected, allowing to write PackIndex subclasses in
other packages.
Sruteesh [Wed, 14 Feb 2024 16:49:39 +0000 (22:19 +0530)]
ResolveMerger: Fix the issue with binary modify-modify conflicts
1) If the file was marked as binary by git attributes, we should add the
path to conflicts if content differs in OURS and THEIRS
2) If the path is a file in OURS, THEIRS and BASE and if it is a binary
in any one of them, no content merge should be attempted and the file
content is kept as is in the work tree
Sam Delmerico [Wed, 10 Apr 2024 18:03:46 +0000 (11:03 -0700)]
PackBitmapIndexBuilder.StoredEntry: add getter for objectId
If you only have access to the PackBitmapIndexBuilder there is no way to
get the ObjectId from a StoredEntry instance without also having access
to a reverse pack index. The StoredEntry can provide the idxPosition,
but the PackBitmapIndexBuilder's getObject method requires a
ridxPosition in order to find an ObjectId.
Providing an ObjectId from the StoredEntry gives this information
directly and also allows a caller to get the ridxPosition if desired by
calling PackBitmapIndexBuilder.findPosition(objectId) without needing
an index object.
This closes the operations of the PackBitmapIndexBuilder such that any
method can be called by using information provided by the other methods.
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.
Max Haslbeck [Tue, 12 Mar 2024 13:23:20 +0000 (13:23 +0000)]
Add more tests on rewriting parents in a RevWalk
Change I4e4ff67fb279edbcc3461496b132cea774fb742f introduced new
behaviour that also rewrote parents in a RevWalk if no TreeFilter was
set. This led to unexpected behaviour when users were fetching from
chromium.googlesource.com. I added a few more tests to better describe
the behaviour of RevWalk in regards to rewriting parents.
Change-Id: I1a5e5f8a1de9d8dd0e3664918ac010644b3ef87b Signed-off-by: Max Haslbeck <haslbeck@google.com>
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.
projects / jgit.git / log