Refs are not advertised as part of the protocol v2 capability
advertisement. Don't call AdvertiseRefsHook.
Noticed because many implementations of AdvertiseRefsHook read all
refs in order to call UploadPack#setAdvertisedRefs, causing the
capability advertisement to be as slow as a v0 ref advertisement with
some RefDatabase implementations.
Such an AdvertiseRefsHook is of dubious utility (a better place to
determine which refs are advertised is in the RefDatabase
implementation itself, as in Gerrit), but at any rate since it's not
bringing about any benefit here, we can skip the hook call.
TODO:
- call an appropriate hook instead (https://bugs.eclipse.org/534847)
- add tests
[jn: fleshed out commit message; added TODO notes]
Change-Id: I6eb60ccfb251a45432954467a9ae9c1079a8c8b5
Signed-off-by: Masaya Suzuki <masayasuzuki@google.com>
Signed-off-by: Jonathan Nieder <jrn@google.com>
With this patch, a server spawned by "jgit daemon" can be accessed using
protocol v2 from a Git client that supports it (for example, "git" with
the appropriate patches). This is only activated if the repository's
config has "protocol.version" be 2.
This required a change to the package-private #execute methods in
DaemonService to allow passing of extra parameters.
This has been tested with a patched Git.
Change-Id: Icf043efec7ce956d72b075fc6dc7a87d5a2da82a
Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
Add support for the "ofs-delta" parameter in the "fetch" command in
the fetch-pack/upload-pack protocol v2.
Change-Id: I728cf986082fce4ddeb6a6435897692e15e60cc7
Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
Add support for the "include-tag" parameter in the "fetch" command in
the fetch-pack/upload-pack protocol v2.
In order to determine which tags to include, only objects pointed to by
refs starting with "refs/tags/" are checked. This restriction is for
performance reasons and to match the behavior of Git (see add_ref_tag()
in builtin/pack-objects.c).
Change-Id: I7d70aa09bcc8a525218ff1559e286c2a610258ca
Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
Signed-off-by: Jonathan Nieder <jrn@google.com>
When OPTION_INCLUDE_TAG is set, UploadPack#sendPack uses the #refs
instance variable as a source of information of tags. A subsequent patch
will need to supply this information to #sendPack without
modifying #refs, so refactor #sendPack to take in this information
through a parameter instead.
Note that prior to this patch, #refs was used twice in #sendPack: once
to generate the argument to PackWriter#setTagTargets, and once to
determine if any tags need to be included in the packfile. This patch
only updates the latter use, since the former is meant not only for
"true" tag targets but any object that should be hoisted earlier during
packing (see the documentation of PackWriter#setTagTargets).
This patch does not introduce any functionality change.
Change-Id: I70ed65a1041334abeda8d4bac98cce7cae7efcdf
Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
Signed-off-by: Jonathan Nieder <jrn@google.com>
Add an internal lineMapOrNull helper that returns null when the file
is binary.
This is simpler than using an exception for control flow and avoids
having to override fillInStackTrace to avoid a performance regression.
Change-Id: Ib8bb8df6a6bbd60c62cfb3b4c484a962a98b7507
This allows to differentiate if index was changed by an external git
command or by JGit itself.
Change-Id: Iae692ba7d9bf01a288b3fb2dc2d07aec9891c712
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
This makes binary detection exact in ResolveMerger and DiffFormatter
This has the same intention as
Id4342a199628d9406bfa04af1b023c27a47d4014 but preserves backward
compatibility of the signature of RawParseUtils.lineMap.
Change-Id: Ia24a4e716592bab3363ae24e3a46315a7511154f
Signed-off-by: Han-Wen Nienhuys <hanwen@google.com>
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
https://tools.ietf.org/html/rfc7616 says:
5.12. Parameter Randomness
The security of this protocol is critically dependent on the
randomness of the randomly chosen parameters, such as client and
server nonces. These should be generated by a strong random or
properly seeded pseudorandom source (see [RFC4086]).
Change-Id: I4da5316cb1eb3f59ae06c070ce1c3335e9ee87d6
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
* stable-4.11:
Retry stale file handles on .git/config file
Change-Id: I4fe6152c3c40dde9cb88913cc9706852de0fd712
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
* stable-4.9:
Retry stale file handles on .git/config file
Change-Id: I6db7256dbd1c71b23e1231809642ca21e996e066
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
On a local non-NFS filesystem the .git/config file will be orphaned if
it is replaced by a new process while the current process is reading the
old file. The current process successfully continues to read the
orphaned file until it closes the file handle.
Since NFS servers do not keep track of open files, instead of orphaning
the old .git/config file, such a replacement on an NFS filesystem will
instead cause the old file to be garbage collected (deleted). A stale
file handle exception will be raised on NFS clients if the file is
garbage collected (deleted) on the server while it is being read. Since
we no longer have access to the old file in these cases, the previous
code would just fail. However, in these cases, reopening the file and
rereading it will succeed (since it will open the new replacement file).
Since retrying the read is a viable strategy to deal with stale file
handles on the .git/config file, implement such a strategy.
Since it is possible that the .git/config file could be replaced again
while rereading it, loop on stale file handle exceptions, up to 5 extra
times, trying to read the .git/config file again, until we either read
the new file, or find that the file no longer exists. The limit of 5 is
arbitrary, and provides a safe upper bounds to prevent infinite loops
consuming resources in a potential unforeseen persistent error
condition.
Change-Id: I6901157b9dfdbd3013360ebe3eb40af147a8c626
Signed-off-by: Nasser Grainawi <nasser@codeaurora.org>
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
It often fails on my machine, both in maven and bazel.
This patch marks the test flaky[1] in bazel so that "bazel test" can
run it a few times before declaring failure.
[1] https://docs.bazel.build/versions/master/be/common-definitions.html#test.flaky
Bug: 534285
Change-Id: Ibe5414fefbffe4e8f86af7047608d51cf5df5c47
That site serves from https now.
Reported-by: Nicholas Glorioso <glorioso@google.com>
Change-Id: I2150a18425a1fe3ab5a022882ffe06ccbde17f16
Signed-off-by: Jonathan Nieder <jrn@google.com>
This is easier to type and makes it clearer that it only returns refs
and not the pseudo-refs returned by getAdditionalRefs. It also puts us
in a better position to add a method to the Repository class later
that delegates to this one without colliding with the existing
Repository#getAllRefs method that returns a Map<String, Ref>.
While at it, clarify the javadoc of getRefs and hasRefs to make the
same point.
Suggested-by: David Pursehouse <david.pursehouse@gmail.com>
Change-Id: I23497c66ac7b5e0c987b91efbc9e9cc29924ca66
Signed-off-by: Jonathan Nieder <jrn@google.com>
Callers can now say:
db.getRefDatabase().hasRefs()
rather than the more verbose:
!db.getRefDatabase().getAllRefs().isEmpty()
The default implementation simply uses getAllRefs().isEmpty(), but a
derived class could possibly override the method with a more efficient
implementation.
Change-Id: I5244520708a1a7d9adb351f10e43fc39d98e22a1
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
The previous version suggested testing w2 first because w1 was used
for hashing, but in fact, hashCode returns w2. The order (w3, w4, w5,
w1, w2) might be better on 64-bit processors too, since it allows
comparing 64 bits at a time, although perhaps on a modern SIMD
processor, the entire 160 bytes would be compared at once anyway.
Change-Id: Ieb69606d3c1456aeff36bffe99a71587ea76e977
Signed-off-by: David Turner <dturner@twosigma.com>
Currently to get all refs, callers must use:
getRefsByPrefix(ALL)
Introduce getAllRefs, which does this, and migrate all existing
callers of getRefsByPrefix(ALL).
Change-Id: I7b1687c162c8ae836dc7db3ccc7ac847863f691d
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
The Javadoc refers to the deprecated getRefs method. Update it to refer
to getRefsByPrefix which is the recommended replacement of getRefs.
Change-Id: I61f2abcf1a3794f40a1746317dbc18aa0beb87a7
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
Eclipse warns that DfsReader should be managed by try-with-resource.
As described in 1484d6e (LargePackedWholeObject: Do not reuse released
inflater, 2018-04-26), the DfsReader is owned and closed by the
PackInputStream or explicitly closed in the try block's finally.
Suppress the warning with a brief explanatory comment.
Change-Id: I4187c935742072f3ee7f2d3551a6a98d40fc2702
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
LargePackedWholeObject.openStream produces a stream that allows
reading a large object. This stream holds a DfsReader that takes care
of caching delta bases etc and in particular holds zlib Inflater for
use while reading the each delta in the packfile.
At DfsReader creation time, the Inflater is acquired from a global
InflaterCache to avoid initialization overhead in case there is an
existing Inflater available for reuse. When done with the Inflater,
the DfsReader is responsible for returning it to the cache for reuse.
The DfsReader is AutoClosable to remind the caller to close it and
release the Inflater when finished with it.
b0ac5f9c89 (LargePackedWholeObject:
Refactor to open DfsReader in try-with-resource, 2018-04-11) tried to
clarify the lifetime of the DfsReader but was too aggressive: when
this function returns, PackInputStream owns the DfsReader and is
already going to release it. Worse, the returned InflaterInputStream
holds a reference to the DfsReader's inflater, making releasing the
DfsReader not only unnecessary but unsafe.
The Inflater gets released into the InflaterCache's pool, to be
acquired by another caller that uses it concurrently with the
InflaterInputStream. This results in errors, such as
java.util.zip.ZipException: incorrect header check
at java.util.zip.InflaterInputStream.read(InflaterInputStream.java:164)
at java.util.zip.InflaterInputStream.skip(InflaterInputStream.java:208)
at java.io.BufferedInputStream.skip(BufferedInputStream.java:377)
and
java.util.zip.DataFormatException: incorrect header check
at java.util.zip.Inflater.inflateBytes(Native Method)
at java.util.zip.Inflater.inflate(Inflater.java:259)
at org.eclipse.jgit.internal.storage.dfs.DfsReader.inflate(DfsReader.java:783)
at org.eclipse.jgit.internal.storage.dfs.DfsPackFile.decompress(DfsPackFile.java:420)
at org.eclipse.jgit.internal.storage.dfs.DfsPackFile.load(DfsPackFile.java:767)
and
Caused by: java.util.zip.ZipException: incorrect header check
at java.util.zip.InflaterInputStream.read(InflaterInputStream.java:164)
at java.io.BufferedInputStream.fill(BufferedInputStream.java:246)
at java.io.BufferedInputStream.read1(BufferedInputStream.java:286)
at java.io.BufferedInputStream.read(BufferedInputStream.java:345)
at org.eclipse.jgit.lib.ObjectStream$Filter.read(ObjectStream.java:219)
at org.eclipse.jgit.util.IO.readFully(IO.java:233)
at org.eclipse.jgit.transport.PackParser.checkObjectCollision(PackParser.java:1173)
Verified in production. It should be possible to make a
straightforward unit test for this using the InflaterCache state but
that can wait for a followup commit.
Change-Id: Iaf1d6fd368b64f76c520d215fd270a6098a1f236
Found instructions for configuring maven-compiler-plugin with ecj in
[1]. Verified that ecj run in this way raises build errors when executed
on commit d3ef5213.
Define profiles "ecj" for using Eclipse compiler and "javac" for using
javac including errorprone. By default ecj will be used.
use ecj:
$ mvn -Pecj clean install
use javac:
$ mvn -Pjavac clean install
TODO: find out how to run ecj with errorprone from Maven.
[1] https://stackoverflow.com/questions/33164976/using-eclipse-java-compiler-ecj-in-maven-builds
Change-Id: I716b603b57612b953e603387c82fd01eb1b5ca97
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
Eclipse reports these as errors, so remove them.
Change-Id: Ic53d8003f9faef38fe776af5a73794e7bb1dfc49
Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
* Photon throws null analysis errors on the repeated invocation of those
previously null checked methods. Extract them to a local variable to
avoid this. (the null analysis is configured in project properties)
* setUseProtocolV2() misses @since tag. Problem was introduced with
332bc61124. Might be caused by the long
delay of 2 months from creation to merging.
Change-Id: Ibbb1a1580b604b8e7cd4bf7edc4643e292b6b4a8
Signed-off-by: Michael Keppler <Michael.Keppler@gmx.de>
Add support for the "no-progress" parameter in the "fetch" command in
the fetch-pack/upload-pack protocol v2.
Change-Id: I6a6d6b1534f44845254b81d0e1f5c4ba2ac3d10b
Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
Add support for the "thin-pack" parameter in the "fetch" command in
the fetch-pack/upload-pack protocol v2.
Change-Id: I39a37b2b66a16929137d35c718a3acf2afb6b0b5
Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
Add basic support for the "fetch" command in the fetch-pack/upload-pack
protocol v2. This patch teaches "have" and "done".
The protocol specification (Documentation/technical/protocol-v2.txt in
the Git project) states:
want <oid>
Indicates to the server an object which the client wants to
retrieve. Wants can be anything and are not limited to
advertised objects.
It is unspecified whether the server should respect the
uploadpack.allowtipsha1inwant option etc. when serving packfiles. This
patch is conservative in that the server respects them.
Change-Id: I3dbec172239712ef9286a15b8407e86b87ea7863
Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
Add support for the "ref-prefix" parameter in the "ls-refs" command in
the fetch-pack/upload-pack protocol v2.
Change-Id: If9cf93b2646f75d50a11b5f482594f014d59a836
Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
Once we have read the user's entire command, there is no more need to
buffer our response --- even the strictest servlet engine allows
writing output once the input has been consumed. Noticed when the
analogous code in the "fetch" command (introduced in a later patch)
overflowed its buffer:
java.lang.OutOfMemoryError
at java.io.ByteArrayOutputStream.hugeCapacity(ByteArrayOutputStream.java:123)
[...]
at org.eclipse.jgit.transport.UploadPack.sendPack(UploadPack.java:1905)
at org.eclipse.jgit.transport.UploadPack.sendPack(UploadPack.java:1741)
at org.eclipse.jgit.transport.UploadPack.fetchV2(UploadPack.java:1001)
at org.eclipse.jgit.transport.UploadPack.serviceV2(UploadPack.java:1030)
at org.eclipse.jgit.transport.UploadPack.upload(UploadPack.java:726)
at org.eclipse.jgit.http.server.UploadPackServlet.doPost(UploadPackServlet.java:195)
at javax.servlet.http.HttpServlet.service(HttpServlet.java:637)
Change-Id: I33df56f1cb1c6c2c25ee95426cb7ad665134ac6b
Masaya Suzuki
Jonathan Tan
Matthias Sohn
David Pursehouse
Han-Wen Nienhuys
Jonathan Nieder
Nasser Grainawi
Carsten Pfeiffer
David Turner
Michael Keppler