Matthias Sohn [Sun, 27 Dec 2020 01:11:47 +0000 (02:11 +0100)]
Fix errorprone configuration for maven-compiler-plugin with javac
See https://errorprone.info/docs/installation.
Add new profile jdk8 to enable running errorprone with javac on java 8
and java 11. Remove errorprone configuration from benchmark module,
didn't find a way to make it work and this module does not contain any
productive code.
Change-Id: I6a84195af05e6cea9e7c04ad5cd4c79742e80cb3 Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
Matthias Sohn [Wed, 24 Feb 2021 13:54:52 +0000 (14:54 +0100)]
Merge branch 'master' into stable-5.11
* master: (35 commits)
[releng] japicmp: update last release version
IgnoreNode: include path to file for invalid .gitignore patterns
FastIgnoreRule: include bad pattern in log message
init: add config option to set default for the initial branch name
init: allow specifying the initial branch name for the new repository
Fail clone if initial branch doesn't exist in remote repository
GPG: fix reading unprotected old-format secret keys
Update Orbit to S20210216215844
Add missing bazel dependency for o.e.j.gpg.bc.test
GPG: handle extended private key format
dfs: handle short copies
[GPG] Provide a factory for the BouncyCastleGpgSigner
Fix boxing warnings
GPG: compute the keygrip to find a secret key
GPG signature verification via BouncyCastle
Post commit hook failure should not cause commit failure
Allow to define additional Hook classes outside JGit
GitHook: use default charset for output and error streams
GitHook: use generic OutputStream instead of PrintStream
Update jetty to 9.4.36.v20210114
...
Thomas Wolf [Tue, 23 Feb 2021 17:10:08 +0000 (18:10 +0100)]
IgnoreNode: include path to file for invalid .gitignore patterns
Include the full file path of the .gitignore file and the line number
of the invalid pattern. Also include the pattern itself.
.gitignore files inside the repository are reported with their
repository-relative path; files outside (from git config
core.excludesFile or .git/info/exclude) are reported with their
full absolute path.
Bug: 571143
Change-Id: Ibe5969679bc22cff923c62e3ab9801d90d6d06d1 Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>
Thomas Wolf [Tue, 23 Feb 2021 12:11:56 +0000 (13:11 +0100)]
FastIgnoreRule: include bad pattern in log message
When a .gitignore pattern cannot be parsed include the pattern in the
log message. Just reporting "not closed bracket" isn't helpful if the
user doesn't know in which pattern the problem occurred.
Even better would be to include the full path of the .gitignore file
that contained the offending pattern. This is not implemented in this
change; it may need new API and needs more thought.
Bug: 571143
Change-Id: Id5b16d9cf550544ba3ad409a02041946fa8516ab Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>
Matthias Sohn [Mon, 25 Jan 2021 01:43:18 +0000 (02:43 +0100)]
init: add config option to set default for the initial branch name
We introduced the option --initial-branch=<branch-name> to allow
initializing a new repository with a different initial branch.
To allow users to override the initial branch name more permanently
(i.e. without having to specify the name manually for each 'git init'),
introduce the 'init.defaultBranch' option.
This option was added to git in 2.28.0.
See https://git-scm.com/docs/git-config#Documentation/git-config.txt-initdefaultBranch
Bug: 564794
Change-Id: I679b14057a54cd3d19e44460c4a5bd3a368ec848 Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
Matthias Sohn [Mon, 25 Jan 2021 00:54:03 +0000 (01:54 +0100)]
init: allow specifying the initial branch name for the new repository
Add option --initial-branch/-b to InitCommand and the CLI init command.
This is the first step to implement support for the new option
init.defaultBranch. Both were added to git in release 2.28.
See https://git-scm.com/docs/git-init#Documentation/git-init.txt--bltbranch-namegt
Bug: 564794
Change-Id: Ia383b3f90b5549db80f99b2310450a7faf6bce4c Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
Thomas Wolf [Sun, 24 Jan 2021 01:13:43 +0000 (02:13 +0100)]
GPG: handle extended private key format
Add detection for the key-value pair format that was available in
gpg-agent for some time already and that has become the default since
gpg-agent 2.2.20. If a secret key in the .gnupg/private-keys-v1.d
directory is found to have this format, extract the human-readable key
from it, convert it to the binary serialized form and hand that to
BouncyCastle.
Encrypted keys in the new format may use AES/OCB. OCB is a patent-
encumbered algorithm; although there is a license for open-source
software, that may not be good enough and OCB may not be available in
Java. It is not available in the default security provider in Java,
and it is also not available in the BouncyCastle version included in
Eclipse.
Implement AES/OCB decryption, throwing a PGPException with a nice
message if the algorithm is not available. Include a copy of the normal
s-expression parser of BouncyCastle and fix it to properly handle data
from such keys: such keys do not contain an internal hash since the
AES/OCB cipher includes and checks a MAC already.
Bug: 570501
Change-Id: Ifa6391a809a84cfc6ae7c6610af6a79204b4143b Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>
wh [Thu, 17 Dec 2020 18:14:32 +0000 (18:14 +0000)]
dfs: handle short copies
`copy` is documented as possibly returning a smaller number of bytes
than requested. In practice, this can occur if a block is cached and the
reader never pulls in the file to check its size.
Thomas Wolf [Sun, 17 Jan 2021 15:21:28 +0000 (16:21 +0100)]
GPG: compute the keygrip to find a secret key
The gpg-agent stores secret keys in individual files in the secret
key directory private-keys-v1.d. The files have the key's keygrip
(in upper case) as name and extension ".key".
A keygrip is a SHA1 hash over the parameters of the public key. By
computing this keygrip, we can pre-compute the expected file name and
then check only that one file instead of having to iterate over all
keys stored in that directory.
This file naming scheme is actually an implementation detail of
gpg-agent. It is unlikely to change, though. The keygrip itself is
computed via libgcrypt and will remain stable according to the GPG
main author.[1]
Add an implementation for calculating the keygrip and include tests.
Do not iterate over files in BouncyCastleGpgKeyLocator but only check
the single file identified by the keygrip.
Ideally upstream BouncyCastle would provide such a getKeyGrip() method.
But as it re-builds GPG and libgcrypt internals, it's doubtful it would
be included there, and since BouncyCastle even lacks a number of curve
OIDs for ed25519/curve25519 and uses the short-Weierstrass parameters
instead of the more common Montgomery parameters, including it there
might be quite a bit of work.
Thomas Wolf [Thu, 7 Jan 2021 16:11:57 +0000 (17:11 +0100)]
GPG signature verification via BouncyCastle
Add a GpgSignatureVerifier interface, plus a factory to create
instances thereof that is provided via the ServiceLoader mechanism.
Implement the new interface for BouncyCastle. A verifier maintains
an internal LRU cache of previously found public keys to speed up
verifying multiple objects (tag or commits). Mergetags are not handled.
Provide a new VerifySignatureCommand in org.eclipse.jgit.api together
with a factory method Git.verifySignature(). The command can verify
signatures on tags or commits, and can be limited to accept only tags
or commits. Provide a new public WrongObjectTypeException thrown when
the command is limited to either tags or commits and a name resolves
to some other object kind.
In jgit.pgm, implement "git tag -v", "git log --show-signature", and
"git show --show-signature". The output is similar to command-line
gpg invoked via git, but not identical. In particular, lines are not
prefixed by "gpg:" but by "bc:".
Trust levels for public keys are read from the keys' trust packets,
not from GPG's internal trust database. A trust packet may or may
not be set. Command-line GPG produces more warning lines depending
on the trust level, warning about keys with a trust level below
"full".
There are no unit tests because JGit still doesn't have any setup to
do signing unit tests; this would require at least a faked .gpg
directory with pre-created key rings and keys, and a way to make the
BouncyCastle classes use that directory instead of the default. See
bug 547538 and also bug 544847.
Tested manually with a small test repository containing signed and
unsigned commits and tags, with signatures made with different keys
and made by command-line git using GPG 2.2.25 and by JGit using
BouncyCastle 1.65.
Bug: 547751
Change-Id: If7e34aeed6ca6636a92bf774d893d98f6d459181 Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>
Matthias Sohn [Sun, 7 Feb 2021 22:48:57 +0000 (23:48 +0100)]
Allow to define additional Hook classes outside JGit
EGit wants to add gitflow specific hooks in org.eclipse.egit.gitflow.
Make GitHook public to allow sub-classing outside of the
org.eclipse.jgit.hooks package.
Change-Id: I439575ec901e3610b5cf9d66f7641c8324faa865 Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
Marija Savtchouk [Thu, 21 Jan 2021 15:08:57 +0000 (15:08 +0000)]
Allow dir/file conflicts in virtual base commit on recursive merge.
If RecursiveMerger finds multiple base commits, it tries to compute
the virtual ancestor to use as a base for the three way merge.
Currently, the content conflicts between ancestors are ignored (file
staged with the conflict markers). If the path is a file in one ancestor
and a dir in the other, it results in NoMergeBaseException
(CONFLICTS_DURING_MERGE_BASE_CALCULATION).
Allow these conflicts by ignoring this unmerged path in the virtual
base. The merger will compute diff in the children instead and it
can be further fixed manually if needed.
Change-Id: Id59648ae1d6bdf300b26fff513c3204317b755ab Signed-off-by: Marija Savtchouk <mariasavtchouk@google.com>
Thomas Wolf [Sun, 24 Jan 2021 00:57:09 +0000 (01:57 +0100)]
GPG: support git config gpg.program
Add it to the GpgConfig. Change GpgConfig to load the values once only.
Add a parameter to the GpgObjectSigner interface's operations to pass
in a GpgConfig. Update CommitCommand and TagCommand to pass the value
to the signer. Let the signer decide whether it can actually produce
the wanted signature type (openpgp or x509).
No behavior change. But this makes it possible to implement different
signers that might support x509 signatures, or use gpg.program and
shell out to an external GPG executable for signing.
Change-Id: I427f83eb1ece81c310e1cddd85315f6f88cc99ea Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>
Adithya Chakilam [Sat, 30 Jan 2021 01:35:04 +0000 (19:35 -0600)]
Fix DateRevQueue tie breaks with more than 2 elements
DateRevQueue is expected to give out the commits that have higher
commit time. But in case of tie(same commit time), it should give
the commit that is inserted first. This is inferred from the
testInsertTie test case written for DateRevQueue. Also that test
case, right now uses just two commits which caused it not to fail
with the current implementation, so added another commit to make
the test more robust.
By fixing the DateRevQueue, we would also match the behaviour of
LogCommand.addRange(c1,c2) with git log c1..c2. A test case for
the same is added to show that current behaviour is not the
expected one.
By fixing addRange(), the order in which commits are applied during
a rebase is altered. Rebase logic should have never depended upon
LogCommand.addRange() since the intended order of addRange() is not
the order a rebase should use. So, modify the RebaseCommand to use
RevWalk directly with TopoNonIntermixSortGenerator.
Add a new LogCommandTest.addRangeWithMerge() test case which creates
commits in the following order:
A - B - C - M
\ /
-D-
Using git 2.30.0, git log B..M outputs: M C D
LogCommand.addRange(B, M) without this fix outputs: M D C
LogCommand.addRange(B, M) with this fix outputs: M C D
Matthias Sohn [Sun, 3 Jan 2021 01:10:58 +0000 (02:10 +0100)]
Fix FileRepository#convertToReftable which failed if no reflog existed
Deleting non-existing files when converting to reftable without backup
caused convertToReftable to fail. Observed this on a mirrored repository
which had no reflogs. Fix this by skipping missing files during
deletion.
Change-Id: I3bb913d5bfddccc6813677b873006efb849a6ebc Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
David Ostrovsky [Sat, 25 Jul 2020 08:00:11 +0000 (10:00 +0200)]
Migrate to Apache MINA sshd 2.6.0 and Orbit I20210203173513
Re-enable DSA, DSA_CERT, and RSA_CERT public key authentication.
DSA is discouraged for a long time already, but it might still be
way too disruptive to completely drop it. RSA is discouraged for
far less long, and dropping that would be really disruptive.
Adapt to the changed property handling. Remove work-arounds for
shortcomings of earlier sshd versions.
Use Orbit I20210203173513, which includes sshd 2.6.0. This also bumps
apache.httpclient to 4.5.13 and apache.httpcore to 4.4.14.
Change-Id: I2d24a1ce4cc9f616a94bb5c4bdaedbf20dc6638e Signed-off-by: David Ostrovsky <david@ostrovsky.org> Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch> Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
Thomas Wolf [Fri, 29 Jan 2021 11:12:28 +0000 (12:12 +0100)]
LFS: make pointer parsing more robust
Parsing an LFS pointer must check the input more to not run into
exceptions. LfsPoint.parseLfsPointer() is used in various places to
determine whether a blob is a LFS pointer; it is not only called with
valid LFS pointers. Tighten the validations and return null if they
fail. All callers already do check for a null return value.
Also, LfsPointer implemented Comparable but did not override equals().
This is rather unusual and actually warned against in the javadoc of
Comparable. Implement equals() and hashCode().
Add more tests.
Bug: 570744
Change-Id: I90ca264d0a250275cf1907e9dcfcee5eab80df0f Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>
Terry Parker [Mon, 25 Jan 2021 06:18:21 +0000 (22:18 -0800)]
Move reachability checker generation into the ObjectReader object
Reachability checkers are retrieved from RevWalk and ObjectWalk objects:
* RevWalk.createReachabilityChecker()
* ObjectWalk.createObjectReachabilityChecker()
Since RevWalks and ObjectWalks are themselves directly instantiated
in hundreds of places (e.g. UploadPack...) overriding them in a
consistent way requires overloading 100s of methods, which isn't
feasible. Moving reachability checker generation to a more central
place solves that problem.
The ObjectReader object seems a good place from which to get
reachability checkers, because reachability checkers return
information about relationships between objects. ObjectDatabases
delegate many operations to ObjectReaders, and reachability bitmaps
are attached to ObjectReaders.
The Bitmapped and Pedestrian reachability checker objects were
package private in the org.eclipse.jgit.revwalk package. This change
makes them public and moves them to the
org.eclipse.jgit.internal.revwalk package. Corresponding tests are
also moved.
Motivation:
1) Reachability checking algorithms need to scale. One of the
internal Android repositories has ~2.4 million refs/changes/*
references, causing bad long tail performance in reachability
checks.
2) Reachability check performance is impacted by repository
topography: number of refs, number of objects, amounts of
related vs. unrelated history.
3) Reachability check performance is also affected by per-branch
access (Gerrit branch permissions) since different users can
see different branches.
4) Reachability check performance isn't affected by any state in a
RevWalk or ObjectWalk.
I don't yet know if a single algorithm will work for all cases in #2
and #3. We may need to evolve the ReachabilityChecker interfaces
over time to solve the Gerrit branch permissions case, or use
Gerrit-specific identity information to solve that in an efficient
way.
This change takes the existing public API and moves it to the
ObjectReader/whole repository level, which is where we can do
consistent customizations for #2 and #3. We intend to upstream the
best of whatever works, but anticipate the need for multiple rounds
of experimentation.
Change-Id: I9185feff43551fb387957c436112d5250486833d Signed-off-by: Terry Parker <tparker@google.com>
Add the "compression-level" option to all ArchiveCommand formats
Different archive formats support a compression level in the range
[0-9]. The value 0 is for lowest compressions and 9 for highest. Highest
levels produce output files of smaller sizes but require more memory to
do the compression.
This change allows passing a "compression-level" option to the git
archive command and implements using it for different file formats.
Change-Id: I5758f691c37ba630dbac24db67bb7da827bbc8e1 Signed-off-by: Youssef Elghareeb <ghareeb@google.com> Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
Jonathan Tan [Wed, 27 Jan 2021 18:36:43 +0000 (13:36 -0500)]
Merge changes I36d9b63e,I8c5db581,I2c02e89c
* changes:
Compare getting all refs except specific refs with seek and with filter
Add getsRefsByPrefixWithSkips (excluding prefixes) to ReftableDatabase
Add seekPastPrefix method to RefCursor
Gal Paikin [Mon, 7 Dec 2020 14:18:34 +0000 (15:18 +0100)]
Compare getting all refs except specific refs with seek and with filter
There are currently two ways to get all refs except a specific ref, we
add two methods that perform both and compare the two different approaches.
This change adds two methods that compares the two different approaches
of such query:
1. Get all the refs, and then filter by refs that don't start with the
prefix (current approach).
2. Get all refs until encountering a ref that is part of the prefix we
should exclude, skip using seekPastPrefix, and continue (new approach).
This works since the refs are sorted.
Specifically in Gerrit, we often have thousands of refs that are not
refs/changes, and millions of refs/changes, hence the second approach
should be much faster. In Jgit in general it's still expected to provide
a better result even if we're skipping a smaller chunk of the refs
since the complexity here is O(logn) with a binary search, rather than
O(number of skipped refs).
We ran this benchmark on a big chunk of chromium/src's reftable. To run
it, we first create the reftable:
Result:
total time the action took using seek: 36925 usec
total time the action took using filter: 874382 usec
number of refs that start with prefix: 4266.
number of refs that don't start with prefix: 1962695.
Similarly for Android's biggest repository, platform/frameworks/base
(still only partial result):
total time the action took using seek: 9020 usec
total time the action took using filter: 143166 usec
number of refs that start with prefix: 296.
number of refs that don't start with prefix: 60400.
In conclusion, it's easy to see an improvement of a factor of 15-20x for
large Gerrit repositories!
Signed-off-by: Gal Paikin <paiking@google.com>
Change-Id: I36d9b63eb259804c774864429cf2c761cd099cc3
Gal Paikin [Mon, 30 Nov 2020 14:57:06 +0000 (15:57 +0100)]
Add getsRefsByPrefixWithSkips (excluding prefixes) to ReftableDatabase
We sometimes want to get all the refs except specific prefixes,
similarly to getRefsByPrefix that gets all the refs of a specific
prefix.
We now create a new method that gets all refs matching a prefix except a
set of specific prefixes.
One use-case is for Gerrit to be able to get all the refs except
refs/changes; in Gerrit we often have lots of refs/changes, but very
little other refs. Currently, to get all the refs except refs/changes we
need to get all the refs and then filter the refs/changes, which is very
inefficient. With this method, we can simply skip the unneeded prefix so
that we don't have to go over all the elements.
RefDirectory still uses the inefficient implementation, since there
isn't a simple way to use Refcursor to achieve the efficient
implementation (as done in ReftableDatabase).
Signed-off-by: Gal Paikin <paiking@google.com>
Change-Id: I8c5db581acdeb6698e3d3a2abde8da32f70c854c
Gal Paikin [Thu, 19 Nov 2020 17:05:04 +0000 (18:05 +0100)]
Add seekPastPrefix method to RefCursor
This method will be used by the follow-up change. This useful if we want
to go over all the changes after a specific ref.
For example, the new method allows us to create a follow-up that would
go over all the refs until we reach a specific ref (e.g refs/changes/),
and then we use seekPastPrefix(refs/changes/) to read the rest of the refs,
thus basically we return all refs except a specific prefix.
When seeking past a prefix, the previous condition that created the
RefCursor still applies. E.g, if the cursor was created by
seekRefsWithPrefix, we can skip some refs but we will not return refs
that are not starting with this prefix.
Signed-off-by: Gal Paikin <paiking@google.com>
Change-Id: I2c02e89c877fe90da8619cb8a4a9a0c865f238ef
Han-Wen Nienhuys [Mon, 25 Jan 2021 15:54:52 +0000 (16:54 +0100)]
reftable: add random suffix to table names
In some circumstances (eg. compacting a stack that has deletions), the
result may have a {min, max} range that already exists. In these
cases, we would rename onto an already existing file, which does not
work on Windows. By adding a random suffix, we disambiguate the files,
and avoid this failure scenario.
Thomas Wolf [Mon, 18 Jan 2021 15:05:58 +0000 (16:05 +0100)]
Correct the minimum required version of Apache httpclient
org.eclipse.jgit.http.apache uses several features that exist only
since httpclient 4.4, but its MANIFEST.MF still had a lower bound of
4.3.0. Bump this to 4.4.0 for all packages from httpclient. 4.3.0 for
the packages from httpcore is fine.
Do a similar clean-up in the other bundles using packages from Apache
httpclient (http.test, lfs, lfs.server, lfs.server.test)
Bug: 570451
Change-Id: Iffdde2a9bd0d65db2e5201a08cffbf03597e2866 Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>
Thomas Wolf [Wed, 2 Dec 2020 22:07:01 +0000 (23:07 +0100)]
TransportHttp: support preemptive Basic authentication
If the caller knows already HTTP Basic authentication will be needed
and if it also already has the username and password, preemptive
authentication is a little bit more efficient since it avoids the
initial 401 response.
Add a setPreemptiveBasicAuthentication(username, password) method
to TransportHttp. Client code could call this for instance in a
TransportConfigCallback. The method throws an IllegalStateException
if it is called after an HTTP request has already been made.
Additionally, a URI can include userinfo. Although it is not
recommended to put passwords in URIs, JGit's URIish and also the
Java URL and URI classes still allow it. The underlying HTTP
connection may omit these fields though. If present, take these
fields as additional source for preemptive Basic authentication if
setPreemptiveBasicAuthentication() has not been called.
No preemptive authentication will be done if the connection is
redirected to a different host.
Add tests.
Bug: 541327
Change-Id: Id00b975e56a15b532de96f7bbce48106d992a22b Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>
Thomas Wolf [Mon, 30 Nov 2020 11:14:22 +0000 (12:14 +0100)]
TransportHttp: shared SSLContext during fetch or push
TransportHttp makes several HTTP requests. The SSLContext and socket
factory must be shared over these requests, otherwise authentication
information may not be propagated correctly from one request to the
next. This is important for authentication mechanisms that rely on
client-side state, like NEGOTIATE (either NTLM, if the underlying HTTP
library supports it, or Kerberos). In particular, SPNEGO cannot
authenticate on a POST request; the authentication must come from the
initial GET request, which implies that the POST request must use the
same SSLContext and socket factory that was used for the GET.
Change the way HTTPS connections are configured. Introduce the concept
of a GitSession, which is a client-side HTTP session over several HTTPS
requests. TransportHttp creates such a session and uses it to configure
all HTTP requests during that session (fetch or push). This gives a way
to abstract away the differences between JDK and Apache HTTP connections
and to configure SSL setup outside.
A GitSession can maintain state and thus give all HTTP requests in a
session the same socket factory.
Introduce an extension interface HttpConnectionFactory2 that adds a
method to obtain a new GitSession. Implement this for both existing
HTTP connection factories. Change TransportHttp to use the new
GitSession to configure HTTP connections.
The old methods for disabling SSL verification still exist to support
possibly external connection and connection factory implementations
that do not make use of the new GitSession yet.
Bug: 535850
Change-Id: Iedf67464e4e353c1883447c13c86b5a838e678f1 Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>
Thomas Wolf [Sun, 29 Nov 2020 15:22:39 +0000 (16:22 +0100)]
TransportHttp: make the connection factory configurable
Previously, TransportHttp always used the globally set connection
factory. This is problematic if that global factory is changed in
the middle of a fetch or push operation. Initialize the factory to
use in the constructor, then use that factory for all HTTP requests
made through this transport. Provide a setter and a getter for it
so that client code can customize the factory, if needed, in a
TransportConfigCallback.
Once a factory has been used on a TransportHttp instance it cannot
be changed anymore.
Make the global static factory reference volatile.
Change-Id: I7c6ee16680407d3724e901c426db174a3125ba1c Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>
Thomas Wolf [Thu, 7 Jan 2021 16:10:45 +0000 (17:10 +0100)]
Tag message must not include the signature
Signatures on tags are just tacked onto the end of the message.
Getting the message must not return the signature. Compare [1]
and [2] in C git, which both drop a signature at the end of an
object body.
Thomas Wolf [Wed, 6 Jan 2021 11:13:48 +0000 (12:13 +0100)]
Protocol V2: don't log spurious ACKs in UploadPack
UploadPack may log ACKs in protocol V2 that it doesn't send (if it
got a "done" from the client), or may log ACKs twice. That makes
packet log analysis difficult.
Add a new constructor to PacketLineOut to omit all logging from an
instance, and use it in UploadPack.
Change-Id: Ic29ef5f9a05cbcf5f4858a4e1b206ef0e6421c65 Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>
Thomas Wolf [Sun, 3 Jan 2021 22:07:18 +0000 (23:07 +0100)]
Protocol V2: respect MAX_HAVES only once we got at least one ACK
The negotiation in the git protocol contains a cutoff: if the client
has sent more than MAX_HAVES "have" lines without getting an ACK, it
gives up and sends a "done". MAX_HAVES is 256.
However, this cutoff must kick in only if at least one ACK has been
received. Otherwise the client may give up way too early, which makes
the server send all its history. See [1].
This was missed when protocol V2 was implemented for fetching in JGit
in commit 0853a241.
Compare also C git commit 0b07eecf6ed.[2] C git had the same bug.[3][4]
Matthias Sohn [Mon, 28 Dec 2020 01:21:17 +0000 (02:21 +0100)]
RepositoryCache: declare schedulerLock final
This fixes errorprone error [SynchronizeOnNonFinalField]: Synchronizing
on non-final fields is not safe: if the field is ever updated, different
threads may end up locking on different objects.
Matthias Sohn [Sun, 3 Jan 2021 15:08:24 +0000 (16:08 +0100)]
[spotbugs]: Fix potential NPE in FileSnapshot constructor
File#getParent can return null which caused this spotbugs warning.
FS.FileStoreAttributes#get already gets the parent directory if the
passed File is not a directory and checks for null. Hence there is no
need to get the parent directory in the FileSnapshot constructor.
Change-Id: I77f71503cffb05970ab8d9ba55b69c96c53098b9 Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
Matthias Sohn [Sun, 3 Jan 2021 14:33:36 +0000 (15:33 +0100)]
FileSnapshot: don't try to read file attributes twice
If file doesn't exist set state to MISSING_FILE immediately. Doing that
by calling File#lastModified and File#length effectively does the same
since they set the value to 0 if the file doesn't exist.
Log an error if a different exception than NoSuchFileException is
caught.
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 (stateless protocol, 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. The SSH tests are much slower and much more
focused on the SSH protocol and SSH key handling. Factor out two
very simple cloning and pulling tests and make those run with
protocol V2.
Bug: 553083
Change-Id: I357c7f5daa7efb2872f1c64ee6f6d54229031ae1 Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>
Thomas Wolf [Wed, 30 Dec 2020 16:17:44 +0000 (17:17 +0100)]
Use Map interface instead of ConcurrentHashMap class
On Android, the co-variant override of ConcurrentHashMap.keySet()
introduced in Java 8 was undone. [1] If compiled Java code calls that
co-variant override directly, one gets a NoSuchMethodError exception
at run-time on Android.
Making the code call that method via Map.keySet() side-steps this
problem.
This is similar to bug 496262, where the same problem cropped up when
compiling with Java 8 against a Java 7 target, but here we cannot use
bootclasspath. We build against Java 8, not against the Android version
of it.
Recent Android versions should have some bytecode "magic" that adds the
co-variant override in bytecode (see the commit referenced in [1]), but
on older Android version this problem may still occur. (Or perhaps the
"magic" is ineffective...) There are two pull requests on Github for
this problem, both from 2020, [2][3] while the Android commit [1] is
from March 2018. Apparently people still occasionally run into this
problem in the wild.
Thomas Wolf [Wed, 30 Dec 2020 09:12:34 +0000 (10:12 +0100)]
Fix NPE in DirCacheCheckout
If a file exists in head, merge, and the working tree, but not in
the index, and we're doing a force checkout, the checkout must be
an "update", not a "keep".
This is a follow-up on If3a9b9e60064459d187c7db04eb4471a72c6cece.
Bug: 569962
Change-Id: I59a7ac41898ddc1dd90e86b09b621a41fdf45667 Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>
Thomas Wolf [Tue, 29 Dec 2020 09:15:20 +0000 (10:15 +0100)]
GPG user ID matching: use case-insensitive matching
Although not mentioned in the GPG documentation at [1], GPG uses
case-insensitive matching also for the '<' (exact e-mail) and '@'
(partial e-mail) operators. Matching for '=' (full exact match) is
case-sensitive. Compare [2].
Thomas Wolf [Mon, 28 Dec 2020 21:53:50 +0000 (22:53 +0100)]
Don't export package from test bundle
Do not export the test-only package org.eclipse.jgit.transport from
bundle org.eclipse.jgit.ssh.jsch.test. Doing so can confuse the build
in Eclipse: other bundles that import this package may then also pick
up this test package, leading to non-test sources depending on test
sources and to build cycles.
Change-Id: I9f73b7a8d13bc4a2fe58bd2f1d33068164a13991 Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>
projects / jgit.git / log