David Pursehouse [Sat, 29 Sep 2018 04:23:22 +0000 (13:23 +0900)]
Bazel: Increase severity of MissingFail to ERROR
All instances of this potential bug have been cleaned up in
preceding commits. Increase the severity to ERROR so that it
is easier to detect reoccurences.
Change-Id: I25beebcea1f01f468e0f2b1d24a83511029c077c Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
David Pursehouse [Sat, 29 Sep 2018 03:54:37 +0000 (12:54 +0900)]
ConfigTest: Fix tests for getting empty config value as int
The tests were set up to expect an IllegalArgumentException when
the Config.getInt method was called with a section.key that has
not been set, or explicitly set to an empty string.
However, the IllegalArgumentException never gets thrown because
the getInt method returns the provided default ("1"), and because
there was no call to "fail" after getInt, the incorrect behavior
of the test was not noticed.
Remove the try/catch around getInt, and instead assert that the
expected default value is returned.
Found by Error Prone, which reported:
Not calling fail() when expecting an exception masks bugs
See https://errorprone.info/bugpattern/MissingFail
Change-Id: Ie8e692aba9fb8523241fb8f298d57493923d9f78 Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
Jonathan Tan [Wed, 12 Sep 2018 19:32:27 +0000 (12:32 -0700)]
UploadPack: support deepen-since in protocol v2
Support the deepen-since parameter when requested by a client using
protocol v2. This is done by:
- adding a DepthWalk.RevWalk#setDeepenSince method
- updating DepthGenerator to recognize when deepen-since is set
- recording in DepthWalk.Commit whether a commit is a boundary commit
Existing users of DepthWalk such as UploadPack previously recognized
boundary commits by comparing their depths against the threshold, not
tracking whether any parents were truly excluded. This behavior is
preserved - UploadPack considers a commit as boundary if its depth is
equal to the threshold *or* a parent was excluded (whether by depth or
by deepen-since).
Change-Id: I852bba6b1279f9cc8aee38282e9339d62b8dcddc Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
Jonathan Tan [Wed, 12 Sep 2018 19:49:40 +0000 (12:49 -0700)]
DepthGenerator: remove redundant depth check
In DepthGenerator, commits are always added to the "pending" queue
either at depth 0 (in the constructor) or after a depth check (in
next()), so it is redundant to check for depth after removing them from
the queue. Remove the check.
This redundancy seems to have been present since the introduction of
server-side shallow clone support in commit 9952223e06 ("Implement
server support for shallow clones", 2011-08-21).
Change-Id: Iad334935293367400c2901a25c0f4bf36c437cf2 Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
David Pursehouse [Thu, 27 Sep 2018 09:01:03 +0000 (18:01 +0900)]
FileRepositoryBuilderTest: Don't explicitly close BufferedWriter in try-with-resource
The BufferedWriter is opened in a try-with-resource and thus will be
automatically closed.
Presumably the close was added to make sure it is closed before the
subsequent test statements are executed. Instead of explicitly closing
it, let the try-with-resource automatically close it, and move the
subsequent statements out of the try-block.
Change-Id: If5fada2f580ef9cbaad3a0b9216b5200b917781a Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
David Pursehouse [Tue, 25 Sep 2018 05:11:23 +0000 (14:11 +0900)]
Make inner classes static where possible
As reported by Error Prone:
An inner class should be static unless it references members of its
enclosing class. An inner class that is made non-static unnecessarily
uses more memory and does not make the intent of the class clear.
See https://errorprone.info/bugpattern/ClassCanBeStatic
Change-Id: Ib99d120532630dba63cf400cc1c61c318286fc41 Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
David Pursehouse [Tue, 25 Sep 2018 02:59:11 +0000 (11:59 +0900)]
Bazel: Increase severity of ExpectedExceptionChecker to ERROR
Incorrect usage of ExpectedException has been cleaned up in the
preceding commits. Increase the severity from WARN to ERROR so
that it is easier to catch any reoccurences.
Change-Id: I57decf620b37e85413bda53723f839c02b488f2c Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
Matthias Sohn [Tue, 25 Sep 2018 08:26:19 +0000 (10:26 +0200)]
Update .mailmap
The .mailmap file is used to map author and committer names and
email addresses to canonical real names and email addresses.
See https://git-scm.com/docs/git-check-mailmap
Change-Id: I720326ad204bd0737e8ace2620ccbd1347114ef4 Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
David Ostrovsky [Tue, 25 Sep 2018 06:20:57 +0000 (08:20 +0200)]
ObjectDownloadListener#onWritePossible: Add comment on return statement
It is not obvious why this return statement is needed. Clarify with a
comment that otherwise endless loop may show up when recent versions
of Jetty are used.
Change-Id: I8e5d4de51869fb1179bf599bfb81bcd7d745874b Signed-off-by: David Ostrovsky <david@ostrovsky.org>
David Pursehouse [Tue, 25 Sep 2018 02:57:26 +0000 (11:57 +0900)]
T0003_BasicTest: Fix incorrect usage of ExpectedException
There should only be one statement after the expect(...) method.
Any additional statements after the statement that is expected to
throw will never be executed in a passing test. This can lead to
inappropriately passing tests where later incorrect assertions are
skipped by the thrown exception.
See https://errorprone.info/bugpattern/ExpectedExceptionChecker
Change-Id: I20fecf8fb7a243e9da097e6d03fbf8cd69151bf0 Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
David Pursehouse [Sat, 22 Sep 2018 10:28:50 +0000 (19:28 +0900)]
ProtocolV2ParserTest: Fix incorrect usage of ExpectedException
There should only be one statement after the expect(...) method.
Any additional statements after the statement that is expected to
throw will never be executed in a passing test. This can lead to
inappropriately passing tests where later incorrect assertions are
skipped by the thrown exception.
See https://errorprone.info/bugpattern/ExpectedExceptionChecker
Change-Id: I0d6350fafb281b6bdb04289f4cd5eb4bb159628b Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
Michael Keppler [Sat, 22 Sep 2018 08:37:01 +0000 (10:37 +0200)]
Fix Mylyn commit message template
The commit message template contains a superfluous blank at the end of
the first line, which is deleted by this change. This is only relevant
for jgit contributors using Eclipse, not for jgit users.
Change-Id: I462deb49c26fb64b3dc2d1d75f1e40ef302b0fc9 Signed-off-by: Michael Keppler <Michael.Keppler@gmx.de>
Change-Id: I207a368555bfb7ddab8d782d46d563ce779a6211 Signed-off-by: David Ostrovsky <david@ostrovsky.org> Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
David Pursehouse [Tue, 18 Sep 2018 00:19:41 +0000 (09:19 +0900)]
Merge branch 'stable-5.1'
* stable-5.1:
Fix ObjectUploadListener#close
Fix error handling in FileLfsServlet
ObjectDownloadListener#onWritePossible: Make code spec compatible
ObjectDownloadListener: Return from onWritePossible when data is written
Fix IOException when LockToken#close fails
Change-Id: If6e4b4786401c6598b9f3b1c2b7d7081b08acd35 Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
David Pursehouse [Tue, 18 Sep 2018 00:19:13 +0000 (09:19 +0900)]
Merge branch 'stable-5.0' into stable-5.1
* stable-5.0:
Fix ObjectUploadListener#close
Fix error handling in FileLfsServlet
ObjectDownloadListener#onWritePossible: Make code spec compatible
ObjectDownloadListener: Return from onWritePossible when data is written
Fix IOException when LockToken#close fails
Change-Id: Ib7a63b5144a89c213aff3b32c30a2a6526355e64 Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
David Pursehouse [Tue, 18 Sep 2018 00:18:34 +0000 (09:18 +0900)]
Merge branch 'stable-4.11' into stable-5.0
* stable-4.11:
Fix ObjectUploadListener#close
Fix error handling in FileLfsServlet
ObjectDownloadListener#onWritePossible: Make code spec compatible
ObjectDownloadListener: Return from onWritePossible when data is written
Fix IOException when LockToken#close fails
Change-Id: Iffe202a412b3bca1c8d8d7dc5dfd646c49838de9 Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
David Pursehouse [Tue, 18 Sep 2018 00:16:58 +0000 (09:16 +0900)]
Merge branch 'stable-4.10' into stable-4.11
* stable-4.10:
Fix ObjectUploadListener#close
Fix error handling in FileLfsServlet
ObjectDownloadListener#onWritePossible: Make code spec compatible
ObjectDownloadListener: Return from onWritePossible when data is written
Fix IOException when LockToken#close fails
Change-Id: I28ebf6138ccd9425fc05319de78c7716f0bdd199 Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
David Pursehouse [Tue, 18 Sep 2018 00:16:26 +0000 (09:16 +0900)]
Merge branch 'stable-4.9' into stable-4.10
* stable-4.9:
Fix ObjectUploadListener#close
Fix error handling in FileLfsServlet
ObjectDownloadListener#onWritePossible: Make code spec compatible
ObjectDownloadListener: Return from onWritePossible when data is written
Fix IOException when LockToken#close fails
Change-Id: Ib7d01cb0ece8b259156855045a53b8baf3fa2968 Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
David Pursehouse [Tue, 18 Sep 2018 00:15:57 +0000 (09:15 +0900)]
Merge branch 'stable-4.8' into stable-4.9
* stable-4.8:
Fix ObjectUploadListener#close
Fix error handling in FileLfsServlet
ObjectDownloadListener#onWritePossible: Make code spec compatible
ObjectDownloadListener: Return from onWritePossible when data is written
Fix IOException when LockToken#close fails
Change-Id: Id8eb635094336567d9f3c28ec985cd5127d31632 Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
David Pursehouse [Tue, 18 Sep 2018 00:15:02 +0000 (09:15 +0900)]
Merge branch 'stable-4.7' into stable-4.8
* stable-4.7:
Fix ObjectUploadListener#close
Fix error handling in FileLfsServlet
ObjectDownloadListener#onWritePossible: Make code spec compatible
ObjectDownloadListener: Return from onWritePossible when data is written
Fix IOException when LockToken#close fails
Change-Id: Iad9836811be034cf992ea25dad4409addba75115 Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
Matthias Sohn [Sun, 16 Sep 2018 22:35:33 +0000 (00:35 +0200)]
Fix error handling in FileLfsServlet
Check in #sendError method if the response was committed already.
If yes we cannot set response status or send an error message, last
resort is to close the outputstream.
If the response wasn't yet committed first reset the response before
using writer to send the error message to the client since mixing STREAM
and WRITE mode (mixing asynchronous and blocking I/O) is illegal in
servlet 3.1.
see the following bugs in the gerrit and jetty issue trackers
https://bugs.chromium.org/p/gerrit/issues/detail?id=9667
https://bugs.chromium.org/p/gerrit/issues/detail?id=9721
https://github.com/eclipse/jetty.project/issues/2911
Change-Id: Ie35563c2e0ac1c5e918185a746622589a880dc7f Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
David Ostrovsky [Sat, 15 Sep 2018 19:09:51 +0000 (21:09 +0200)]
ObjectDownloadListener#onWritePossible: Make code spec compatible
Current code violates the ServletOutputStream contract. For every
out.isReady() == true either write or close of that ServletOutputStream
should be called.
See also this issue upstream for more context: [1].
Change-Id: Ied575f3603a6be0d2dafc6c3329d685fc212c7a3 Signed-off-by: David Ostrovsky <david@ostrovsky.org> Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
David Ostrovsky [Sat, 15 Sep 2018 19:09:51 +0000 (21:09 +0200)]
ObjectDownloadListener: Return from onWritePossible when data is written
When buffer was written not only call AsyncContext#complete() but also
return from the ObjectDownloadListener#onWritePossible(). This avoids
endless loop after upgrading from Jetty 9.3.x to 9.4.x lines.
In Jetty example implementation:[1] the return statemnt is also used:
// If we are at EOF then complete
if (len < 0)
{
async.complete();
return;
}
Thomas Wolf [Thu, 23 Jul 2015 18:14:46 +0000 (14:14 -0400)]
Fix fetch refspecs when not cloning all branches
When not all branches are cloned, the fetch refspec for the
remote should not be "+refs/heads/*:refs/remotes/origin/*":
that would fetch all branches on the very next fetch, thus
making a clone with only a subset of the branches rather
pointless.
Instead, produce refspecs for the cloned branches only.
Canonical git also does this for its --single-branch case;
it doesn't have an option to clone only a subset of the branches
(only one or all).
Bug: 466858
Change-Id: Ie871880f757663437efac1e8b3313094f9e629b3 Also-by: Julian Enoch <julian.enoch@ericsson.com> Signed-off-by: Julian Enoch <julian.enoch@ericsson.com> Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch> Signed-off-by: David Pursehouse <david.pursehouse@gmail.com> Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
Ivan Frade [Tue, 28 Aug 2018 20:30:14 +0000 (13:30 -0700)]
Untangle UploadPack.processShallow
UploadPack.processShallow is doing too many things and offering a
confusing API. It is filtering or splitting commit ids depending
if a parameter is null and writing them out (or not) depending on another
flag.
Iterate the list and announce to Consumers what object ids need
to be marked as (un)shallow. They decide what to do with them.
As java consumers don't allow to propagate exceptions, define
our own functional interface for it.
Change-Id: I619cf2eed9b1e0338151120b8ef87a463fbe8827 Signed-off-by: Ivan Frade <ifrade@google.com>
Ivan Frade [Thu, 13 Sep 2018 23:02:42 +0000 (16:02 -0700)]
UploadPack.fetchV2: Make shallow commits list non-nullable
shallowCommits variable is nullable only to signal later
if code has gone through the processShallow code.
Use a flag to indicate that condition and make shallowCommits
non-nullable. This makes code clearer and paves the way to untangle
processShallow in a follow-up commit.
Change-Id: I898e6aaf6f860bb6afafbac05653ba116c9b2da6 Signed-off-by: Ivan Frade <ifrade@google.com>
If some process executed by FS#readPipe ends in an error,
the error stream is never set as errorMessage because
FS#GobblerThread#waitForProcessCompletion always returned true.
This caused LOG#warn to be called with null.
Matthias Sohn [Wed, 12 Sep 2018 12:50:44 +0000 (14:50 +0200)]
Merge branch 'stable-5.1'
* stable-5.1:
Fix NoSuchFileException during directory cleanup in RefDirectory
Update japicmp to 0.13.0
Prepare 5.1.1-SNAPSHOT builds
JGit v5.1.0.201809111528-r
Externalize warning message in RefDirectory.delete()
Suppress warning for trying to delete non-empty directory
Fix replacement quoting for replaceAll in filter command
Prepare 4.11.3-SNAPSHOT builds
JGit v4.11.2.201809100523-r
Change-Id: Ia27fec6dabf87db26e2fd29c17570932649e234a Signed-off-by: David Pursehouse <david.pursehouse@gmail.com> Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
David Pursehouse [Wed, 12 Sep 2018 12:25:51 +0000 (21:25 +0900)]
Merge branch 'stable-5.0' into stable-5.1
* stable-5.0:
Fix NoSuchFileException during directory cleanup in RefDirectory
Externalize warning message in RefDirectory.delete()
Suppress warning for trying to delete non-empty directory
Change-Id: I9371f354d685545cc6a268420e44ae4c9577680f Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
David Pursehouse [Wed, 12 Sep 2018 10:03:26 +0000 (19:03 +0900)]
Merge branch 'stable-4.11' into stable-5.0
* stable-4.11:
Fix NoSuchFileException during directory cleanup in RefDirectory
Externalize warning message in RefDirectory.delete()
Suppress warning for trying to delete non-empty directory
Change-Id: Ic7eaa0b6a4e750bc7853eb549a9e126269ae22cf Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
David Pursehouse [Wed, 12 Sep 2018 08:02:09 +0000 (17:02 +0900)]
Merge branch 'stable-4.10' into stable-4.11
* stable-4.10:
Fix NoSuchFileException during directory cleanup in RefDirectory
Externalize warning message in RefDirectory.delete()
Suppress warning for trying to delete non-empty directory
Change-Id: I191d56e3c0f11cf53076b06c7e7a05492c7a03f1 Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
David Pursehouse [Wed, 12 Sep 2018 07:38:48 +0000 (16:38 +0900)]
Merge branch 'stable-4.9' into stable-4.10
* stable-4.9:
Fix NoSuchFileException during directory cleanup in RefDirectory
Externalize warning message in RefDirectory.delete()
Suppress warning for trying to delete non-empty directory
Change-Id: Ied2d7a70c5544ff012687a1aeb6e07394d9bd6fd Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
David Pursehouse [Wed, 12 Sep 2018 06:55:07 +0000 (15:55 +0900)]
Merge branch 'stable-4.8' into stable-4.9
* stable-4.8:
Fix NoSuchFileException during directory cleanup in RefDirectory
Externalize warning message in RefDirectory.delete()
Suppress warning for trying to delete non-empty directory
Change-Id: I5e6cc35f3673545e7ff857e6ed0bcd2c44e50316 Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
David Pursehouse [Wed, 12 Sep 2018 05:05:46 +0000 (14:05 +0900)]
Merge branch 'stable-4.7' into stable-4.8
* stable-4.7:
Fix NoSuchFileException during directory cleanup in RefDirectory
Externalize warning message in RefDirectory.delete()
Suppress warning for trying to delete non-empty directory
Change-Id: I9ec6352b5ff57aa1a3380079dc9165890cc76d49 Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
Matthias Sohn [Thu, 23 Aug 2018 09:44:28 +0000 (11:44 +0200)]
Externalize warning message in RefDirectory.delete()
Change-Id: Icec16c01853a3f5ea016d454b3d48624498efcce Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
(cherry picked from commit 5e68fe245fb97f38620ea683dbeab724bf86da4c) Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
Thomas Wolf [Sun, 19 Aug 2018 18:48:06 +0000 (20:48 +0200)]
Suppress warning for trying to delete non-empty directory
This is actually a fairly common occurrence; deleting the parent
directories can work only if the file deleted was the last one
in the directory.
Bug: 537872
Change-Id: I86d1d45e1e2631332025ff24af8dfd46c9725711 Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>
(cherry picked from commit d9e767b431eae7978613cc8e0ade7467ec04376c) Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>