Petr Hrebejk [Mon, 30 Nov 2020 19:19:53 +0000 (20:19 +0100)]
Fix PackInvalidException when fetch and repack run concurrently
We are running several servers with jGit. We need to run repack from
time to time to keep the repos performant. I.e. after push we test how
many small packs are in the repo and when a threshold is reached we run
the repack.
After upgrading jGit version we've found that if someone does the clone
at the time repack is running the clone sometimes (not always) fails
because the repack removes .pack file used by the clone. Server
exception and client error attached.
I've tracked down the cause and it seems to be introduced between jGit
5.2 (which we upgraded from) and 5.3 and being caused by this commit:
Move throw of PackInvalidException outside the catch -
https://github.com/eclipse/jgit/commit/afef866a44cd65fef292c174cad445b3fb526400
The problem is that when the throw was inside of the try block the last
catch block catched the exception and called openFailed(false) method.
It is true that it called it with invalidate = false, which is wrong.
The real problem though is that with the throw outside of the try block
the openFail is not called at all and the fields activeWindows and
activeCopyRawData are not set to 0. Which affects the later called tests
like: if (++activeCopyRawData == 1 && activeWindows == 0).
The fix for this is relatively simple keeping the throw outside of the
try block and still having the invalid field set to true. I did
exhaustive testing of the change running concurrent clones and pushes
indefinitely and with the patch applied it never fails while without the
patch it takes relatively short to get the error.
David Ostrovsky [Sun, 29 Nov 2020 12:06:00 +0000 (13:06 +0100)]
Add constants for parsing git wire protocol version
This would allow other JGit users to access and reuse the constants.
Change-Id: I1608802f45586af5f8582afa592e26679e9cebe3 Signed-off-by: David Ostrovsky <david@ostrovsky.org> Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
Matthias Sohn [Wed, 25 Nov 2020 16:00:09 +0000 (17:00 +0100)]
Ensure that GC#deleteOrphans respects pack lock
If pack or index files are guarded by a pack lock (.keep file)
deleteOrphans() should not touch the respective files protected by the
lock file. Otherwise it may interfere with PackInserter concurrently
inserting a new pack file and its index.
The problem was caused by the following race.
All mentioned files are located in "objects/pack/".
File endings relevant in "pack" dir:
.pack
.keep
.idx
.bitmap
When ReceivePack receives a pack file it executes the following steps:
ReceivePack.service():
receivePackAndCheckConnectivity():
receivePack():
receive the pack
parse the pack, returns packLock (.keep file)
PackInserter.flush():
write tmpPck file: "insert_<random>.pack"
write tmpIdx file: "insert_<random>.idx"
real pack name: "pack-<SHA1>.pack"
real index name: "pack-<SHA1>.idx"
atomic rename tmpPack to realPack
atomic rename tmpIdx to tmpIdx
execute commands
unlock pack by removing .keep file
trigger auto gc if enabled
When PackInserter.flush() renames the temporary pack to the final
"pack-xxx.pack" file the temporary pack index file "insert_xxx.idx"
has no matching .pack file with the same base name for a short interval.
If deleteOrphans() ran during that interval it deduced the pack index
file was orphaned. Subsequently the missing pack index caused
MissingObjectExceptions since objects contained in the pack couldn't be
looked up anymore.
Bug: https://bugs.chromium.org/p/gerrit/issues/detail?id=13544
Change-Id: I559c81e4b1d7c487f92a751bd78b987d32c98719 Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
Thomas Wolf [Thu, 19 Nov 2020 12:23:05 +0000 (13:23 +0100)]
PacketLineIn: ensure that END != DELIM
Just allocate the two string objects directly. The previously used
new StringBuilder(0).toString() returns the same object for both END
and DELIM when run on Java 15, which breaks the wire protocol since
then END and DELIM cannot be distinguished anymore.
Bug: 568950
Change-Id: I9d54d7bf484948c24b51a094256bd9d38b085f35 Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>
(cherry picked from commit 7da0f0a8f37e35e9c3108588f1e6f7a7381d8f77)
Thomas Wolf [Thu, 19 Nov 2020 12:23:05 +0000 (13:23 +0100)]
PacketLineIn: ensure that END != DELIM
Just allocate the two string objects directly. The previously used
new StringBuilder(0).toString() returns the same object for both END
and DELIM when run on Java 15, which breaks the wire protocol since
then END and DELIM cannot be distinguished anymore.
Bug: 568950
Change-Id: I9d54d7bf484948c24b51a094256bd9d38b085f35 Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>
(cherry picked from commit 7da0f0a8f37e35e9c3108588f1e6f7a7381d8f77)
Matthias Sohn [Sat, 5 Sep 2020 20:51:02 +0000 (22:51 +0200)]
Merge branch 'master' into stable-5.9
* master:
SshdSession: close channel gracefully
jgit: Add DfsBundleWriter
Prepare 5.10.0-SNAPSHOT builds
ResolveMerger: do not content-merge gitlinks on del/mod conflicts
ResolveMerger: Adding test cases for GITLINK deletion
ResolveMerger: choose OURS on gitlink when ignoreConflicts
ResolveMerger: improving content merge readability
ResolveMerger: extracting createGitLinksMergeResult method
ResolveMerger: Adding test cases for GITLINK merge
Back out the version change to 5.10.0-SNAPSHOT which was done on master
already.
Change-Id: I1a6b1f0b8f5773be47823d74f593d13b16a601d5 Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
Thomas Wolf [Wed, 17 Jun 2020 16:43:32 +0000 (18:43 +0200)]
GPG: include signer's user ID in the signature
Signing a commit with command line git and gpg 2.2.20 includes the
e-mail part of the key's user ID as a "Signer's User ID" subpacket
on the signature.
Implement this for signing via Bouncy Castle.
Bug: 564386
Change-Id: I68906b895349359596cf3451d65f2840c60df856 Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>
Masaya Suzuki [Fri, 24 Jan 2020 00:47:40 +0000 (16:47 -0800)]
jgit: Add DfsBundleWriter
DfsBundleWriter writes out the entire repository to a Git bundle file.
It packs all objects included in the packfile by concatenating all pack
files. This makes the bundle creation fast and cheap. Useful for backing
up a repository as-is.
Terry Parker [Thu, 3 Sep 2020 22:35:24 +0000 (18:35 -0400)]
Merge changes from topic "fix_ui"
* changes:
ResolveMerger: do not content-merge gitlinks on del/mod conflicts
ResolveMerger: Adding test cases for GITLINK deletion
ResolveMerger: choose OURS on gitlink when ignoreConflicts
ResolveMerger: improving content merge readability
ResolveMerger: extracting createGitLinksMergeResult method
ResolveMerger: Adding test cases for GITLINK merge
Matthias Sohn [Sun, 30 Aug 2020 22:37:03 +0000 (00:37 +0200)]
Update target platform to R20200831200620
and update to new Orbit version of the following libraries
- org.apache.commons.codec to 1.14.0.v20200818-1422
- org.apache.httpcomponents.httpclient to 4.5.10.v20200830-2311
- org.bouncycastle.bcpg [1.65.0.v20200527-1955
- org.bouncycastle.bcpkix [1.65.0.v20200527-1955
- org.bouncycastle.bcprov [1.65.1.v20200529-1514
Fix minor issues in target platforms and rename 4.16-staging to 4.16
since that version was released already.
Change-Id: Ifea2600e445e783807a5d94ea23ac3c6550956b5 Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
Demetr Starshov [Wed, 12 Aug 2020 22:01:10 +0000 (15:01 -0700)]
ResolveMerger: do not content-merge gitlinks on del/mod conflicts
Previously ResolveMerger tried to make a fulltext merge entry in case
one of sides got deleted regardless of file mode. This is not
applicable for GITLINK type of entry. After this change it is
rendering appropriate merge result.
Demetr Starshov [Wed, 12 Aug 2020 21:47:15 +0000 (14:47 -0700)]
ResolveMerger: choose OURS on gitlink when ignoreConflicts
Option ignoreConflicts is used when a caller want to create a virtual
commit and use it in a future merge (recursive merge) or show it on
UI (e.g. Gerrit). According to contract in case of ignoreConflicts
ResolveMerger should populate only stage 0 for entries with merge
conflicts as if there is no conflict. Current implementation breaks
this contract for cases when gitlink revision is ambiguous.
Therefore, always select 'ours' when we merge in ignoreConflicts mode.
This will satisfy the contract contract, so recursive merge can
succeed, however it is an arbitrary decision, so it is not guaranteed
to select best GITLINK in all cases.
GITLINK merging is a special case of recursive merge because of
limitations of GITLINK type of entry. It can't contain more than 1 sha-1
so jgit can't write merge conflicts in place like it can with a blob.
Ideally we could signal the conflict with a special value (like
'0000...'), but that must be supported by all tooling (git fsck, c-git)."
Demetr Starshov [Wed, 12 Aug 2020 00:57:10 +0000 (17:57 -0700)]
ResolveMerger: Adding test cases for GITLINK merge
Add test cases which cover content-merge resolve logic.
Git clients try to agressively merge blobs by content, but GITLINK types
of entries can't be merged with each other or with blobs. This change
ensures all possible permutations which can trigger blob and GITLINK
content merge are covered.
Marc Strapetz [Wed, 12 Aug 2020 12:50:20 +0000 (14:50 +0200)]
Fix possible NegativeArraySizeException in PackIndexV1
Due to an integer overflow bug, the current "Index file is too large
for jgit" check did not work properly and subsequently a
NegativeArraySizeException was raised.
Change-Id: I2736efb28987c29e56bc946563b7fa781898a94a Signed-off-by: Marc Strapetz <marc.strapetz@syntevo.com>
Thomas Wolf [Fri, 14 Aug 2020 12:54:38 +0000 (14:54 +0200)]
FS: use binary search to determine filesystem timestamp resolution
Previous code used a minimum granularity of 1 microsecond and would
iterate 233 times on a system where the resolution is 1 second (for
instance, Java 8 on Mac APFS).
New code uses a binary search between the maximum we care about (2
seconds) and zero, with a minimum granularity of also 1 microsecond.
This takes at most 19 iterations (guaranteed). For a file system with 1
second resolution, it takes 4 iterations (1s, 0.5s, 0.8s, 0.9s). With
an up-front check at 1 microsecond and at 1 millisecond this performs
equally well as the old code on file systems with a fine resolution.
(For instance, Java 11 on Mac APFS.)
Also handle obscure cases where the file timestamp implementation may
yield bogus values (as observed on HP NonStop). If such an error case
occurs, log a warning and abort the measurement at the last good value.
Bug: 565707
Change-Id: I82a96729b50c284be7c23fbdf3d0df1bddf60e41 Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>
Matthias Sohn [Sat, 22 Aug 2020 19:19:57 +0000 (21:19 +0200)]
Do not prematurely create directory of jgit's XDG config file
LockFile.lock() will create it anyway when the config file is created.
Bug: 565637
Change-Id: I078b89a695193fd76f130f6de7ac1cf26d2f8f0f Signed-off-by: Matthias Sohn <matthias.sohn@sap.com> Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>
Thomas Wolf [Thu, 20 Aug 2020 20:43:13 +0000 (22:43 +0200)]
FS: write to JGit config in a background thread
Otherwise locking problems may ensue if the JGit config itself is
on a different file system. Since the internal is already updated,
it is not really important when exactly the value gets persisted.
By queueing up separate Runnables executed by a single thread we
avoid concurrent write access to the JGit config, and nested calls
to getFileStoreAttributes(Path) result in serialized attempts to
write.
The thread for writing the config must not be a daemon thread. If
it were, JVM shutdown might kill it anytime, which may lead to
the config not being written, or worse, a config.lock file being
left behind.
Bug: 566170
Change-Id: I07e3d4c5e029d3cec9ab5895002fc4e0c7948c40 Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>
Thomas Wolf [Thu, 20 Aug 2020 20:15:11 +0000 (22:15 +0200)]
FS: don't cache fallback if running in background
If the background job is a little late, the true result might
arrive and be cached later. So make sure we don't cache the large
fallback resolution in the per-directory cache. Otherwise we'd work
with the large fallback until the next restart.
Bug: 566170
Change-Id: I7354a6cfddfc0c05144bb0aa41c23029bd4f6af0 Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>
Thomas Wolf [Tue, 21 Jul 2020 22:51:24 +0000 (00:51 +0200)]
Keep line endings for text files committed with CR/LF on text=auto
Git never converts line endings if the version in the repository is a
text file with CR/LF and text=auto. See [1]: "When the file has been
committed with CRLF, no conversion is done."
Because the sentence just before is about converting line endings on
check-in, I had understood that in commit 60cf85a [2] to mean that no
conversion on check-in was to be done. However, as bug 565048 and a
code inspection of the C git code showed it really means no conversion
is done on check-in *or check-out*.
If the text attribute is not set but core.autocrlf = true, this is
the same as text=auto eol=crlf. C git does not convert on check-out
even on text=auto eol=lf if the index version is a text file with
CR/LF.
For check-in, one has to look at the intended target, which is done
in WorkingTreeIterator since commit 60cf85a. For check-out, it can
be done by looking at the source and can thus be done in the
AutoLFOutputStream.
Additionally, provide a constructor for AutoLFInputStream to do
the same; for cases where the equivalent of a check-out is done via
an input stream obtained from a blob. (EGit does that in its
GitBlobStorage for the Eclipse compare framework; it's more efficient
than using a TemporaryBuffer and DirCacheCheckout.getContent(), and
it avoids the need for a temporary file.)
Adapt existing tests, and add new checkout and merge tests to verify
the resulting files have the correct line endings.
EGit's GitBlobStorage will need to call the new version of
EolStreamTypeUtil.wrapInputStream().
The WindowCache is configured statically with a default
WindowCacheConfig. The default config says (for backwards
compatibility reasons) to publish the MBean. As a result,
the MBean always gets published.
By delaying the MBean registration until the first call to
getInstance() or get(PackFile, long) we can avoid the forced
registration and do it only if not re-configured in the meantime
not to publish the bean. (As is done by Egit, to avoid a very
early costly access to the user and system config during plug-in
activation.)
Bug: 563740
Change-Id: I8a941342c0833acee2107515e64299aada7e0520 Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>
Thomas Wolf [Mon, 10 Aug 2020 19:38:50 +0000 (21:38 +0200)]
DirCache: support index V4
Index format version 4 was introduced in C git in 2012. It's about
time that JGit can deal with it.
Version 4 added prefix path compression. Instead of writing the full
path for each index entry to disk, only the difference to the previous
entry's path is written: a variable-encoded int telling how many bytes
to remove from the previous entry's path to get the common prefix,
followed by the new suffix.
Also, cache entries in a version 4 index are not padded anymore.
Internally, version 3 and version 4 index entries are identical; it's
only the stored format that changes.
Implement this path compression, and make sure we write an index file
that we read previously in the same format. (Only changing from version
2 to version 3 if there are extended flags.)
Add support for the "feature.manyFiles" and the "index.version" git
configs, and honor them when writing a new index file.
Add tests, including a compatibility test that verifies that JGit can
read a version 4 index generated by C git and write an identical
version 4 index.
Bug: 565774
Change-Id: Id83241cf009e50f950eb42f8d56b834fb47da1ed Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>
Thomas Wolf [Sat, 8 Aug 2020 09:53:43 +0000 (11:53 +0200)]
Update javadoc for RemoteSession and SshSessionFactory
The timeout on RemoteSession.exec() cannot be a timeout for the
whole command. It can only be a timeout for setting up the process;
after that it's the application's responsibility to implement some
timeout for the execution of the command, for instance by calling
Process.waitFor(int, TimeUnit) or through other means.
Sessions returned by an SshSessionFactory are already connected and
authenticated -- they must be, because RemoteSession offers no
operations for connecting or authenticating a session.
Change the implementation of SshdExecProcess.waitFor() to wait
indefinitely. The original implementation used the timeout from
RemoteSession.exec() because of that erroneous javadoc.
Change-Id: I3c7ede24ab66d4c81f72d178ce5012d383cd826e Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>
Thomas Wolf [Thu, 6 Aug 2020 21:43:40 +0000 (23:43 +0200)]
Fix JSchProcess.waitFor() with time-out
SshSupport.runSshCommand() had a comment that wait with time-out
could not be used because JSchProcess.exitValue() threw the wrong
unchecked exception when the process was still running.
Fix this and make JSchProcess.exitValue() throw the right exception,
then wait with a time-out in SshSupport.
The Apache sshd client's SshdExecProcess has always used the correct
IllegalThreadStateException.
Add tests for SshSupport.runCommand().
Change-Id: Id30893174ae8be3b9a16119674049337b0cf4381 Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>
Thomas Wolf [Mon, 3 Aug 2020 14:22:37 +0000 (16:22 +0200)]
sshd: work around a race condition in Apache MINA sshd 2.4.0/2.5.x
When exceptions occur very early in the SSH connection setup, it's
possible that an exception gets lost. A subsequent authentication
attempt may then never be notified of the failure, and then wait
indefinitely or until its timeout expires.
This is caused by race conditions in sshd. The issue has been reported
upstream as SSHD-1050,[1] but will be fixed at the earliest in sshd
2.6.0.
Thomas Wolf [Sat, 25 Jul 2020 08:20:29 +0000 (10:20 +0200)]
sshd: store per-session data on the sshd session object
Don't store session properties on the client but in a dedicated
per-session object that is attached to the sshd session.
Also make sure that each sshd session gets its own instance of
IdentityPasswordProvider that asks for passphrases of encrypted
private keys, and also store it on the session itself.
Bug: 563380
Change-Id: Ia88bf9f91cd22b5fd32b5972d8204d60f2de56bf Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>
then I would expect no blobs to be transferred over the wire. Alas,
JGit rewrites filter=blob:none to filter=blob:limit=0, so if the
repository contains an empty file then the empty blob gets
transferred.
Fix it by teaching JGit about filters based on object type to
complement the existing filters based on object size. This prepares
us for other future filters such as object:none.
In particular, this means we do not need to look up the size of the
filtered blobs, which should speed up clones. Noticed by Anna
Pologova and Terry Parker.
Change-Id: Id4b234921a190c108d8be2c87f54dcbfa811602a Signed-off-by: Jonathan Nieder <jrn@google.com>
Jonathan Nieder [Thu, 30 Jul 2020 03:40:50 +0000 (20:40 -0700)]
Add support for tree filters when fetching
Teach the FilterSpec serialization code about tree filters so they can
be communicated over the wire and understood by the server.
While we're here, harden the FilterSpec serialization code to throw
IllegalStateException if we encounter a FilterSpec that cannot be
expressed as a "filter" line. The only public API for creating a
Filterspec is to pass in a "filter" line to be parsed, so these should
not appear in practice.
Change-Id: I9664844059ffbc9c36eb829e2d860f198b9403a0 Signed-off-by: Jonathan Nieder <jrn@google.com>
Thomas Wolf [Tue, 28 Jul 2020 07:44:43 +0000 (09:44 +0200)]
sshd: use PropertyResolver in test
Improve the SshTestGitServer API for accessing the server properties.
Instead of returning the raw property map, return the proper sshd API
abstraction PropertyResolver.
This makes the interface more resilient against upstream changes.
Change-Id: Ie5b685bddc4e59f3eb6c121026d3658d57618ca4 Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>
Thomas Wolf [Fri, 19 Jun 2020 13:29:57 +0000 (15:29 +0200)]
FS_POSIX: avoid prompt to install the XCode tools on OS X
OS X ships with a default /usr/bin/git that is just a wrapper that
at run-time delegates to the selected XCode toolchain, and that
prompts the user to install the XCode command line tools if not
already installed.
This is annoying for people who don't want to do so, since they'll
be prompted on each Eclipse start. Also, since on OS X the $PATH for
applications started via the GUI is not the same as the $PATH as set
via the shell profile, just using /usr/bin/git (which will normally
be found when JGit runs inside Eclipse) may give slightly surprising
results if the user has installed a non-Apple git and changed his
$PATH in the shell such that the non-Apple git is used in the shell.
(For instance by placing /usr/local/bin earlier on the path.) Eclipse
and the shell will use different git executables, and thus different
git system configs.
Therefore, try to find git via bash --login -c 'which git' not only
if we couldn't find it on $PATH but also if we found the default git
/usr/bin/git. If that finds some other git, use that. If the bash
approach also finds /usr/bin/git, double check via xcode-select -p
that an XCode git is present. If not, assume there is no git installed,
and work without any system config.
Bug: 564372
Change-Id: Ie9d010ebd9437a491ba5d92b4ffd1860c203f8ca Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>
Thomas Wolf [Sat, 27 Jun 2020 13:01:01 +0000 (15:01 +0200)]
Remove dependency on JSch from SSH test framework
Use standard java.security to generate test keys, use sshd to write
public key files, and write PKCS#8 PEM files for our non-encrypted
test private keys. This is a format that both JSch and Apache MINA
sshd can read.
Change-Id: I6ec55cfd7346b672a7fb6139d51abfb06d81a394 Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>
Matthias Sohn [Fri, 17 Jul 2020 15:26:17 +0000 (17:26 +0200)]
Use LinkedBlockingQueue for executor determining filesystem attributes
Using a fixed thread pool with unbounded LinkedBlockingQueue fixes the
RejectedExecutionException thrown if too many threads try to
concurrently determine filesystem attributes.
Comparing that to an alternative implementation using an unbounded
thread pool instead showed similar performance with the reproducer (in
range of 100-1000 threads in reproducer) on my mac:
threads time
fixed threadpool up to 5 threads with LinkedBlockingQueue of unlimited
queue size
100 1103 ms
200 1602 ms
300 2369 ms
500 4002 ms
1000 11071 ms
unbounded cached threadpool
100 1108 ms
200 1591 ms
300 2299 ms
500 4577 ms
1000 11196 ms
Bug: 564202
Change-Id: I773da7414a1dca8e548349442dca9b56643be946 Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
David Ostrovsky [Sun, 21 Jun 2020 14:56:57 +0000 (16:56 +0200)]
Bazel: Add workspace status command to stamp final artifact
Include implementation version in jgit library. This version is used
by other products that depend on JGit, and built using Bazel and not
consume officially released artifact from Central or Eclipse own Maven
repository.
Most notably, in Gerrit Code Review JGit agent that was previously
reported as "unknown", is now reported as:
Thomas Wolf [Sat, 11 Jul 2020 13:29:04 +0000 (15:29 +0200)]
DiffFormatter: correctly deal with tracked files in ignored folders
In JGit 5.0, the FileTreeIterator was changed to skip ignored folders
by default. To catch tracked files inside ignored folders, the tree
walk needs to have a DirCacheIterator, and the FileTreeIterator has
to know about that DirCacheIterator via setDirCacheIterator(). (Or
the optimization has to be switched off explicitly via
setWalkIgnoredDirectories(true).)
Skipping ignored directories is an important optimization in some
cases, for instance in node.js/npm projects, where we'd otherwise
traverse the whole huge and deep hierarchy of the typically ignored
node_modules folder.
While all uses of WorkingTreeIterator in JGit had been adapted,
DiffFormatter was forgotten. To make it work correctly (again) also
for such cases, make it set up a WorkingTreeeIterator automatically,
and make sure the WorkingTreeSource can find such files, too. Also
pass the repository to the TreeWalks used inside the DiffFormatter
to pick up the correct attributes, filters, and line-ending settings.
Bug: 565081
Change-Id: Ie88ac81166dc396ba28b83313964c1712b6ca199 Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>
projects / jgit.git / log