Matthias Sohn [Fri, 2 Oct 2020 19:54:53 +0000 (21:54 +0200)]
Don't install 3rd party dependency bundles via features
Instead provide them only in the p2 repository. This way they are
available when installing from the jgit p2 repository but we are not
enforcing the version we bring but can also use the version available in
Eclipse if it matches our requirements.
Bug: 514326
Bug: 566475
Change-Id: I3e8d0bad12cfb0c1003ade3e6f13e9af35626f14 Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
Matthias Sohn [Wed, 2 Dec 2020 14:49:35 +0000 (15:49 +0100)]
Merge branch 'master' into stable-5.10
* master:
Prepare 5.3.10-SNAPSHOT builds
JGit v5.3.9.202012012026-r
Prepare 5.1.16-SNAPSHOT builds
JGit v5.1.15.202012011955-r
Fix PackInvalidException when fetch and repack run concurrently
Upgrade maven-pmd-plugin to 3.14.0
Update Orbit to R20201130205003 for 2020-12
Use new protocol version constants
PacketLineInTest: test for END and DELIM being distinguishable
Add constants for parsing git wire protocol version
Ignore missing javadoc tags in test bundles
Bazel: Allow to build and run the tests with JDK 15
[releng] japicmp: update last release version
Add support for reading symrefs from pack capabilities
Change-Id: I5afbbb912f502991f0cf9c2501b024f5f00144ba Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
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 [Thu, 19 Nov 2020 00:55:35 +0000 (01:55 +0100)]
Ignore missing javadoc tags in test bundles
It seems Eclipse 4.18 reports them as error whereas earlier versions
ignored this maybe since we don't require javadoc comments for all the
test bundles.
Change-Id: I3f4d42ce681ea5c2b4b302991d2641290ac8561d Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
David Ostrovsky [Mon, 23 Nov 2020 19:04:07 +0000 (20:04 +0100)]
Bazel: Allow to build and run the tests with JDK 15
To avoid JDK specific bugs in future, like Bug: 568950, and given that
upcoming Bazel release 4.0.0 added support to JDK 15 java toolchain,
add definition for remote JDK 15 to WORKSPACE file and add build and
test instructions.
To build and execute the tests with JDK 15 on Linux run:
Lee Worrall [Sat, 10 Oct 2020 03:31:19 +0000 (11:31 +0800)]
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>
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>
Matthias Sohn [Wed, 25 Nov 2020 16:39:25 +0000 (17:39 +0100)]
Merge branch 'master' into stable-5.10
* master:
Update Orbit to S20201118210000 and add target for 4.18
PacketLineIn: ensure that END != DELIM
PacketLineIn: ensure that END != DELIM
Allow to resolve a conflict by checking out a file
Update Orbit to I20201111205634
Document that setLastModified sets time of symlink target
Fix bug in PerformanceLogContext
Fix IOException occurring during gc
Change-Id: I2980552381d1ae61b9b2d81d7289de37d6bf4cae 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)
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>
Thomas Wolf [Sun, 15 Nov 2020 17:45:12 +0000 (18:45 +0100)]
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>
Matthias Sohn [Mon, 17 Aug 2020 12:23:29 +0000 (14:23 +0200)]
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.
Alexa Panfil [Fri, 23 Oct 2020 17:23:23 +0000 (17:23 +0000)]
Fix bug in PerformanceLogContext
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>
Nail Samatov [Tue, 27 Oct 2020 13:56:24 +0000 (16:56 +0300)]
Fix IOException occurring during gc
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>
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>
Nail Samatov [Tue, 27 Oct 2020 14:37:24 +0000 (17:37 +0300)]
Close Repository to fix tests failing on Windows
Fix tests failing on Windows because Repository
instance is created but not closed on tear down.
Change-Id: I72488ba5efeec95110926b1fbd56b7d96fca0d37 Signed-off-by: Nail Samatov <sanail@yandex.ru> Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
Thomas Wolf [Sun, 2 Aug 2020 17:22:05 +0000 (19:22 +0200)]
Client-side protocol V2 support for fetching
Make all transports request protocol V2 when fetching. Depending on
the transport, set the GIT_PROTOCOL environment variable (file and
ssh), pass the Git-Protocol header (http), or set the hidden
"\0version=2\0" (git anon). We'll fall back to V0 if the server
doesn't reply with a version 2 answer.
A user can control which protocol the client requests via the git
config protocol.version; if not set, JGit requests protocol V2 for
fetching. Pushing always uses protocol V0 still.
In the API, there is only a new Transport.openFetch() version that
takes a collection of RefSpecs plus additional patterns to construct
the Ref prefixes for the "ls-refs" command in protocol V2. If none
are given, the server will still advertise all refs, even in protocol
V2.
BasePackConnection.readAdvertisedRefs() handles falling back to
protocol V0. It newly returns true if V0 was used and the advertised
refs were read, and false if V2 is used and an explicit "ls-refs" is
needed. (This can't be done transparently inside readAdvertisedRefs()
because a "stateless RPC" transport like TransportHttp may need to
open a new connection for writing.)
BasePackFetchConnection implements the changes needed for the protocol
V2 "fetch" command (simplified ACK handling, delimiters, section
headers).
In TransportHttp, change readSmartHeaders() to also recognize the
"version 2" packet line as a valid smart server indication.
Adapt tests, and run all the HTTP tests not only with both HTTP
connection factories (JDK and Apache HttpClient) but also with both
protocol V0 and V2. Do the same for the SSH transport tests.
Bug: 553083
Change-Id: Ice9866aa78020f5ca8f397cde84dc224bf5d41b4 Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch> Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
John Dallaway [Wed, 2 Sep 2020 13:15:23 +0000 (14:15 +0100)]
Ensure .gitmodules is loaded when accessing submodule name
This problem occurred when calling SubmoduleWalk#getModuleName if the
first submodule processed has a name and a path which do not match
SubmoduleWalk#getModuleName returned the module path instead of the
module name. In order to fix this SubmoduleWalk#getModuleName needs to
ensure that the modules config is loaded.
Bug: 565776
Change-Id: I36ce1fbc64c4849f9d8e39864b825c6e28d344f8 Signed-off-by: John Dallaway <john@dallaway.org.uk> Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
Alexa Panfil [Mon, 12 Oct 2020 08:55:46 +0000 (08:55 +0000)]
Add new performance logging
Add new performance logging to register events of type duration.
The proposed logging is similar to the performance logging
in OS Gerrit https://gerrit-review.googlesource.com/c/gerrit/+/225628:
a global Singleton (LoggingContext in Gerrit) is
collecting the performance logs in a thread-safe events list,
and at the end of the monitored command the list of events is
retrieved and written to a log, after which it is cleared.
What this patch does:
The main component is the Singleton (PerformanceLogContext), which
is used to collect the records (PerformanceLogRecord) in one place
(ThreadLocal eventsList) from anywhere using
PerformanceLogContext.getInstance().addEvent().
Reason why this change is needed:
The current monitoring in JGit has several issues:
1. git fetch and git push events are handled separately
(PackStatistics and ReceivedPackStatistics), with no unified way
of writing or reading the statistics.
2. PostUploadHook is only invoked on the event of sending the
pack, which means that the PackStatistics is not available for
the fetch requests that did not end with sending the pack
(negotiation requests).
3. The way the logs are created is different from the performance
log approach, so the long-running operations need to be collected
from both performance log (for JGit DFS overridden operations and
Gerrit operations) and gitlog (for JGit ones).
The proposed performance logging is going to solve the above
mentioned issues: it collects all of the performance logs in one
place, thus accounting for the commands that do not result in
sending a pack. The logs are compatible with the ones on Gerrit.
Moreover, the Singleton is accessible anywhere in the call stack,
which proved to be successful in other projects like Dapper
(https://research.google/pubs/pub36356/).
Alexa Panfil [Fri, 9 Oct 2020 16:52:04 +0000 (16:52 +0000)]
Compute time differences with Duration
Reason why this change is needed:
Currently the durations of fetch events are computed by
registering time instants with System.currentTimeMillis()
and calculating the differences with simple minus operation,
but multiple sources suggest that the best practice is to use
the Java 8 Duration and Instant objects.
What this patch does:
Get time measurements with Instant.now() instead of
System.currentTimeMillis() and calculate the duration of fetch
events (Reachability checks and Negotiation) using
Duration.between().toMillis().
David Ostrovsky [Fri, 2 Oct 2020 16:49:21 +0000 (18:49 +0200)]
Fix OperatorPrecedence warning flagged by error prone
Building with Bazel is failing with this error message:
HttpSupport.java:480: error: [OperatorPrecedence] Use grouping
parenthesis to make the operator precedence explicit
if (c >= 'a' && c <= 'z' || c >= 'A' && c <= 'Z') {
^
(see https://errorprone.info/bugpattern/OperatorPrecedence)
Did you mean 'if ((c >= 'a' && c <= 'z') || (c >= 'A' && c <= 'Z')) {'?
James Wynn [Mon, 31 Aug 2020 20:32:13 +0000 (15:32 -0500)]
Support "http.userAgent" and "http.extraHeader" from the git config
Validate the extra headers and log but otherwise ignore invalid
headers. An empty http.extraHeader starts the list afresh.
The http.userAgent is restricted to printable 7-bit ASCII, other
characters are replaced by '.'.
Moves a support method from the ssh.apache bundle to HttpSupport in
the main JGit bundle.
Bug:541500
Change-Id: Id2d8df12914e2cdbd936ff00dc824d8f871bd580 Signed-off-by: James Wynn <james@jameswynn.com> Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>
Thomas Wolf [Thu, 24 Sep 2020 19:34:29 +0000 (21:34 +0200)]
Add TypedConfigGetter.getPath()
This enables EGit to override with a lenient variant that logs the
problem and continues with the default value. EGit needs this because
otherwise a corrupt core.excludesFile entry can render the whole UI
unusable (until the git config is fixed) because any use of the
WorkingTreeIterator will throw an InvalidPathException.
This is not a problem on OS X, where all characters are allowed in
file names. But on Windows some characters are forbidden... see bug
567296. The message of the InvalidPathException is not helpful since
it doesn't point to the origin of the problem. EGit can log a much
better message indicating the offending config file and entry in the
Eclipse error log, where the user can see it.
Bug: 567309
Change-Id: I4e57afa715ff3aaa52cd04b5733f69e53af5b1e0 Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>
Reason why this change is needed:
Getting this metric will help estimate how much time is spent
on negotiation in fetch V2.
What this patch does:
Measure time spent on negotiation rounds in UploadPack.fetchV2()
and save it in an instance of PackStatistics.Accumulator.
This is the same way the statistics are already gathered for
protocol V0/V1.
Thomas Wolf [Thu, 10 Sep 2020 21:39:07 +0000 (23:39 +0200)]
IndexDiffFilter: handle path prefixes correctly
When comparing git directory paths to check whether one is a prefix
of another, one must add a slash to avoid false prefix matches when
one directory name is a prefix of another. The path "audio" is not
a prefix of the path "audio-new", but would be a prefix of a path
"audio/new".
Bug: 566799
Change-Id: I6f671ca043c7c2c6044eb05a71dc8cca8d0ee040 Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>
projects / jgit.git / log