[spotbugs] Fix potential NPE in FileRepository#convertToReftable
File#listFiles can return null. Use Files#list which does not return
null and should be faster since it's returning directory entries lazily
while File#listFiles fetches them eagerly.
Change-Id: I3bfe2a52278244fc469143692c06b05d9af0d0d4
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
If TransportException is thrown in the finally block of execute()
ensure that the exception handled in the previous catch block isn't
suppressed.
Change-Id: I670acdfb4d36e7a419a9a79ae9faab2e085a43ee
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
If TransportException is thrown in the finally block of
downloadPackedObject() ensure that the exception handled in the previous
catch block isn't suppressed.
Change-Id: I23982a2b215e38f681cc1719788985e60232699a
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
This was experimental code and never used in production.
Change-Id: Ia3da7f2b82d9e365cec2ccf9397cbc47439cd150
Signed-off-by: Han-Wen Nienhuys <hanwen@google.com>
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
Split out loose object handling from ObjectDirectory
The ObjectDirectory class manages the interactions for the entire object
database, this includes loose objects, packfiles, alternates, and
shallow commits. To help reduce the complexity of this class, abstract
some of the loose object specific details into a class which understands
just this, leaving the ObjectDirectory to focus more on the interactions
between the different mechanisms.
Change-Id: I39f3a74d6308f042a2a2baa57769f4acde5ba5e0
Signed-off-by: Martin Fick <mfick@codeaurora.org>
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
The ObjectDirectory class manages the interactions for the entire object
database, this includes loose objects, packfiles, alternates, and
shallow commits. To help reduce the complexity of this class, abstract
some of the packfile specific details into a class which understands
just this, leaving the ObjectDirectory to focus more on the interactions
between the different mechanisms.
Change-Id: I5cc87b964434b0afa860b3fe23867a77b3c3a4f2
Signed-off-by: Martin Fick <mfick@codeaurora.org>
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
Some clients may wish to allow NO_CHANGE lightweight tag updates
without setting the force flag. (For instance EGit does so.)
Command-line git does not allow this.
Propagate the RefUpdate result via the RefAlreadyExistsException.
That way a client has the possibility to catch it and check the
failure reason without having to parse the exception message, and
take appropriate action, like ignoring the exception on NO_CHANGE.
Change-Id: I60e7a15a3c309db4106cab87847a19b6d24866f6
Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>
TagCommand: make -f work with lightweight tags for NO_CHANGE
JGit treated a NO_CHANGE RefUpdate as an error in all cases. But when
updating a lightweight tag, this is a successful result if -f was
specified.
Change-Id: Iddfa6d6a6dc8bf8fed81138a008ebc32d5f960bd
Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>
Add the two config constants from C git that can switch on signing
of annotated tags. Add them to the GpgConfig, and implement actually
signing a tag in TagCommand.
The interactions between command line options for "git tag" and config
options is a bit murky in C git. There are two config settings for it:
* tag.gpgSign is the main option, if set to true, it kicks in if
neither -s nor -u are given on the command line.
* tag.forceSignAnnotated signs only tags created via "git tag -m",
but only if command-line option "-a" is not present. It applies
even if tag.gpgSign is set explicitly to false.
Giving -s or -u on the command line also forces an annotated tag
since lightweight tags cannot be signed.
Bug: 386908
Change-Id: Ic8a1a44b5f12f47d5cdf3aae2456c1f6ca9ef057
Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>
Factor out a common ObjectBuilder as super class of CommitBuilder
and TagBuilder, and make the GpgSigner work on ObjectBuilder.
In order not to break API, add the new method for signing an
ObjectBuilder in a new interface GpgObjectSigner.
The signature for a tag is just tacked onto the end of the tag
message. The message of a signed tag must end in LF.
Bug: 386908
Change-Id: I5e021e3c927f4051825cd7355b129113b949455e
Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>
Added check for null on DirCacheEntry in checkoutEntry method
Observed the error when trying to force checkout from a branch
that had no changes on it. When the 'keep()' method from 'DirCacheCheckout'
method was called the 'DirCacheEntry e' was null and was passed like
this to the 'checkoutEntry()' method where the 'getObjectId()' is
being called on the 'e' object
Change-Id: If3a9b9e60064459d187c7db04eb4471a72c6cece
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 -
afef866a44
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.
See: https://www.eclipse.org/lists/jgit-dev/msg04014.html
Bug: 569349
Change-Id: I9dbf8801c8d3131955ad7124f42b62095d96da54
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
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>
Add support for reading symrefs from pack capabilities
A SymbolicRef is added to the advertised refs for any symref in
capabilities whose target is an advertised ref; this may replace an
existing entry, such as HEAD.
When cloning, if any advertised HEAD is symbolic then use the target
rather than looking for an advertised ref with a matching objectId.
Add --symref option to LsRemote command.
Bug: 514052
Change-Id: Idfb48e6f6e8dcfe57a6896883fe6d84d533aa9d0
Signed-off-by: Lee Worrall <worrall.la@gmail.com>
GC#deleteOrphans: handle failure to list files in pack directory
- log an error
- either there is no list or it is incomplete hence return immediately
Change-Id: Ieee5378ca06304056b9ccc30c1acd5f52360052d
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
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>
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 7da0f0a8f3)
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 7da0f0a8f3)
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>
Allow to resolve a conflict by checking out a file
DirCacheEditor unconditionally applied a PathEdit to all stages in the
index. This gives wrong results if one wants to check out a file from
some commit to resolve a conflict: JGit would update the working tree
file multiple times (once per stage), and set all stages to point to
the checked-out blob.
C git replaces the stages by the entry for the checked-out file.
To support this, add a DirCacheEntry.setStage() method so that
CheckoutCommand can force the stage to zero. In DirCacheEditor, keep
only the zero stage if the PathEdit re-set the stage.
Bug: 568038
Change-Id: Ic7c635bb5aaa06ffaaeed50bc5e45702c56fc6d1
Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>
Document that setLastModified sets time of symlink target
Due to Java bug JDK-8220793 [1] Java cannot set timestamps of a symlink
but only of the symlink target. This bug was fixed in Java 13. Since we
don't have a use case to set the timestamp of the symlink itself simply
document the current behavior of setLastModified methods.
[1] https://bugs.openjdk.java.net/browse/JDK-8220793
Change-Id: Ibc28c1702a1b9845602bd257606fbd44803a43fb
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
PerformanceLogContext threw NullPointerException when multiple threads
tried to add an event to the PerformanceLogContext. The cause for this
is that the ThreadLocal was initialized only in the class constructor
for the first thread; for subsequent threads it was null.
To fix this initialize eventRecords for each thread.
Change-Id: I18ef67dff8f0488e3ad28c9bbc18ce73d5168cf9
Signed-off-by: Alexa Panfil <alexapizza@google.com>
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
Fix IOException occurring when calling
GC on a repository with absent objects/pack folder.
Change-Id: I5be1333a0726f4d7491afd25ddac85451686c30a
Signed-off-by: Nail Samatov <sanail@yandex.ru>
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
Revert "Client-side protocol V2 support for fetching"
This reverts commit f802f06e7f.
I had misunderstood how protocol V2 works. This implementation only
works if the negotiation during fetch is done in one round.
Fixing this is substantial work in BasePackFetchConnection. Basically
I think I'd have to change back negotiate to the V0 version, and have
a doFetch() that does
if protocol V2
doFetchV2()
else
doFetchV0()
with doFetchV0 the old code, and doFetchV2 completely new.
Plus there would need to be a HTTP test case requiring several
negotiation rounds.
This is a couple of days work at least, and I don't know when I will
have the time to revisit this. So although the rest of the code is
fine I prefer to back this out completely and not leave a only half
working implementation in the code for an indeterminate time.
Bug: 553083
Change-Id: Icbbbb09882b3b83f9897deac4a06d5f8dc99d84e
Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>
Matthias Sohn
Han-Wen Nienhuys
Martin Fick
David Ostrovsky
Thomas Wolf
Tudor Matrescu
Petr Hrebejk
Lee Worrall
Alexa Panfil
Nail Samatov