Commit Graph

5054 Commits

Author SHA1 Message Date
David Pursehouse c57c52e1e3 Upgrade errorprone to 2.0.15
Change-Id: I05d3e12178f8f0b5a687c8e09c2dcf80580acfb4
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
2016-12-08 06:41:44 -04:00
David Pursehouse 654ae82970 Replace usage of deprecated EWAHCompressedBitmap.add(long)
The add(long) method was deprecated in favor of addWord(long) in
the 0.8.3 release of JavaEWAH [1].

[1] https://github.com/lemire/javaewah/commit/e443cf5e

Change-Id: I89c397ed02e040f57663d04504399dfdc0889626
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
2016-12-07 22:23:31 -04:00
Jacek Centkowski c7d8fbfaad Expose getObjectToTransfer method of FileLfsServlet
Providing own implementation to doGet/doPut methods is troublesome when
this method is private.

Change-Id: I098cdc5cb90410eaaebc56c88c2d9e168584dd6d
Signed-off-by: Jacek Centkowski <geminica.programs@gmail.com>
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
2016-12-06 16:57:16 +09:00
David Pursehouse c62933bf59 Merge branch 'stable-4.5'
* stable-4.5:
  Use the same variable to check and extract LFS object id

Change-Id: I314bd4373f40843c68853b3999f60d85e08628d9
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
2016-12-06 16:55:28 +09:00
David Pursehouse 196a773e23 Upgrade buck to latest version
The latest version removes dependency on `realpath` which is not
available by default on OSX.

This upgrades buck to the same version used on Gerrit master.

Change-Id: I584211986be4e64f68d4eb905c09d3c5d60133e7
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
2016-12-01 15:57:17 +09:00
Christian Halstrick 930cd43553 Fix merge-base calculation
Fix JGits merge-base calculation in case of inconsistent commit times.
JGit was potentially failing to compute correct merge-bases when the
commit times where inconsistent (a parent commit was younger than a
child commit). The code in MergeBaseGenerator was aware of the fact that
sometimes the discovery of a merge base x can occur after the parents of
x have been seen (see comment in #carryOntoOne()). But in the light of
inconsistent commit times it was possible that these parents of a
merge-base have already been returned as a merge-base.

This commit fixes the bug by buffering all commits generated by
MergeBaseGenerator. It is expected that this buffer will be small
because the number of merge-bases will be small. Additionally a new
flag is used to mark the ancestors of merge-bases. This allows to filter
out the unwanted commits.

Bug: 507584
Change-Id: I9cc140b784c3231b972bd2c3de61a789365237ab
2016-11-28 09:38:19 +01:00
Matthias Sohn 0e31614ebe Configure Maven to run tests concurrently by default
Maven 3.3.1 or later supports sharing project specific command line
options
https://maven.apache.org/docs/3.3.1/release-notes.html

Change-Id: I61ddf08ff8c0ebb76fc2e1d0a0cd5902c65f9469
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
2016-11-25 00:45:17 +01:00
David Pursehouse 872adb71dc Enable error-prone for the Maven build
- update maven-compiler-plugin to 3.6.0
- exclude InsecureCipherFactory from errorprone checks

See
https://maven.apache.org/guides/mini/guide-default-execution-ids.html#Example:_Configuring_compile_to_run_twice
https://groups.google.com/d/topic/error-prone-discuss/pzT45ZMCQOc/discussion

Change-Id: Ic16d3f15cf2ef40de62fe6bfe4b8b35f0c1edc4e
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
2016-11-25 00:45:14 +01:00
Christian Halstrick 1572964ecb Fix encoding of LFSPointer files
LFS pointer files have to be UTF-8 with \n as line ending character.
That is described in [1]. Fix JGit to follow this rules.

[1] https://github.com/github/git-lfs/blob/master/docs/spec.md

Bug: 507120
Change-Id: Ib6bd13f1cc17f1a3de125249b4f250b7b0692396
2016-11-24 17:26:56 +01:00
Jacek Centkowski 8f60297861 Use the same variable to check and extract LFS object id
It is easier to maintain when the same variable is used for both check
and extraction of LFS object id.

Change-Id: I5406f9bc4a085aa164c4565a9667ad2925105190
Signed-off-by: Jacek Centkowski <geminica.programs@gmail.com>
2016-11-24 11:32:48 +01:00
Grace Wang fe329f5db4 Specify RevisionSyntaxException message in Repository#resolve
This does not address all cases where no message is specified, only
cases where Repository#isValidRefName returns false.

Change-Id: Ib88cdabfdcdf37be0053e06949b0e21ad87a9575
Signed-off-by: Grace Wang <gracewang92@gmail.com>
2016-11-24 03:56:01 -04:00
Matthias Sohn c6cfe500b5 Add missing @since tags for new API
Change-Id: I900d745195f58c067fadf209bb92cd3c852c59f4
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
2016-11-23 23:29:41 +01:00
Zhen Chen 8803718493 dump HTTP: Avoid being confused by Content-Length of a gzipped stream
TransportHttp sets 'Accept-Encoding: gzip' to allow the server to
compress HTTP responses. When fetching a loose object over HTTP, it
uses the following code to read the response:

       InputStream in = openInputStream(c);
       int len = c.getContentLength();
       return new FileStream(in, len);

If the content is gzipped, openInputStream decompresses it and produces
the correct content for the object. Unfortunately the Content-Length
header contains the length of the compressed stream instead of the
actual content length. Use a length of -1 instead since we don't know
the actual length.

Loose objects are already compressed, so the gzip encoding typically
produces a longer compressed payload. The value from the Content-Length
is too high, producing EOFException: Short read of block.

Change-Id: I8d5284dad608e3abd8217823da2b365e8cd998b0
Signed-off-by: Zhen Chen <czhen@google.com>
Helped-by: Jonathan Nieder <jrn@google.com>
2016-11-22 21:21:24 -04:00
Jonathan Nieder 4ebe7cc4ca Merge "Close input stream after use" 2016-11-22 15:38:08 -05:00
Zhen Chen ca2183a403 Fix content length in HttpClientConnection
Per the interface specification, the getContentLength method should
return -1 if content length is unknown or greater than
Integer.MAX_VALUE.

For chunked transfer encoding, the content length is not included in the
header, hence will cause a NullPointerException when trying to parse the
content length header.

Change-Id: Iaa36b5c146a8d654e364635fa0bd2d14129baf17
Signed-off-by: Zhen Chen <czhen@google.com>
2016-11-22 12:24:32 -08:00
Zhen Chen 5af3f9bd63 Close input stream after use
The InputStream in FileStream in downloadPack is never closed.

Change-Id: I59975d0b8d51f4b3e3ba9d4496b254d508cb936d
Signed-off-by: Zhen Chen <czhen@google.com>
2016-11-22 12:00:42 -08:00
Shawn Pearce 81f9c18433 Define MonotonicClock interface for advanced timestamps
MonotonicClock can be implemented to provide more certainity about
time than the standard System.currentTimeMillis() can provide. This
can be used by classes such as PersonIdent and Ketch to rely on
more certainity about time moving in a strictly ascending order.

Gerrit Code Review can also leverage this interface through its
embedding of JGit and use MonotonicClock and ProposedTimestamp to
provide stronger assurance that NoteDb time is moving forward.

Change-Id: I1a3cbd49a39b150a0d49b36d572da113ca83a786
2016-11-21 11:34:14 -08:00
Matthias Sohn 608e31126e Add Oxygen target platform and remove target platforms older than Mars
Change-Id: I196c3c95dafa5ea5f165d20aaac2ce07a2c53631
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
2016-11-17 00:35:56 +01:00
Dave Borowitz 5bb434e01f Update JavaEWAH to 1.1.6
Use Oxygen M3 Orbit repository which provides the bundles built using
the new orbit-recipe based build.

CQ: 11658
Change-Id: I7f3dcc966732b32830c75d5daa55383bd028d182
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
2016-11-17 00:26:44 +01:00
Matthias Sohn 52fa09c8d4 Add missing @since tags for new API
Change-Id: Iaf83f66637d6a13e4a6d096ba8529553af7e30ed
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
2016-11-14 16:10:40 -08:00
Shawn Pearce 2685f4b101 Fix cryptoVer constant name to CRYPTO_VER
Change-Id: I46c39f2eceb4d58e49bd6273b87711f35250ab5c
2016-11-14 15:52:43 -08:00
Shawn Pearce 3b2248c5cf RepositoryCache: simplify code
The type parameters can now be inferred when creating
ConcurrentHashMap.

A for loop over the keys of a ConcurrentHashMap doesn't
need to use an Iterator<Map.Entry>; loop syntax handles
this just fine over keySet().

Change-Id: I1f85bb81b77f7cd1caec77197f2f0bf78e4a82a1
2016-11-14 15:51:55 -08:00
Shawn Pearce ca4ef2d24b Add missing @Override found by ErrorProne
Change-Id: I585242507815aad4aa0103fd55a6c369e342ab33
2016-11-14 15:46:28 -08:00
Shawn Pearce 8208da2f59 Deprecate SafeBufferedOutputStream
Java 8 fixed the silent flush during close issue by
FilterOutputStream (base class of BufferedOutputStream)
using try-with-resources to close the stream, getting a
behavior matching what JGit's SafeBufferedOutputStream
was doing:

  try {
    flush();
  } finally {
    out.close();
  }

With Java 8 as the minimum required version to run JGit
it is no longer necessary to override close() or have
this class. Deprecate the class, and use the JRE's version
of close.

Change-Id: Ic0584c140010278dbe4062df2e71be5df9a797b3
2016-11-14 15:33:54 -08:00
David Pursehouse 3e52252622 Merge "Support {get,set}GitwebDescription on InMemoryRepository" 2016-11-14 17:54:17 -05:00
Shawn Pearce 71ea0fe567 Support {get,set}GitwebDescription on InMemoryRepository
This simplifies testing for Gerrit Code Review where
application code is updating the repository description
and the test harness uses InMemoryRepository.

Change-Id: I9fbcc028ae24d90209a862f5f4f03e46bfb71db0
2016-11-14 14:40:21 -08:00
David Pursehouse a45cfee7a3 Organize imports
Change-Id: I7c545d06b1bced678c020fab9af1382bc4416b6e
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
2016-11-14 13:25:20 -08:00
Shawn Pearce 1c70dd6d21 Add {get,set}GitwebDescription to Repository
This method pair allows the caller to read and modify the description
file that is traditionally used by gitweb and cgit when rendering a
repository on the web.

Gerrit Code Review has offered this feature for years as part of
its GitRepositoryManager interface, but its fundamentally a feature
of JGit and its Repository abstraction.

git-core typically initializes a repository with a default value
inside the description file. During getDescription() this string
is converted to null as it is never a useful description.

Change-Id: I0a457026c74e9c73ea27e6f070d5fbaca3439be5
2016-11-14 11:14:35 -08:00
Shawn Pearce eeb0705ef3 Merge "Don't serialize internal hash collision chain link" 2016-11-14 13:27:11 -05:00
Shawn Pearce a0bac65233 Merge "Reduce synchronized scope around ConcurrentHashMap" 2016-11-14 13:25:38 -05:00
Shawn Pearce 013a4b84fd Merge changes Id89b3cd0,I5b0fb122
* changes:
  FileLfsServlet: Make sendError protected
  Make ObjectUploadListener public
2016-11-14 13:23:37 -05:00
Jonathan Nieder 7b8a0a28bf Merge "StreamCopyThread: Remove unused AtomicInteger import" 2016-11-13 19:05:50 -05:00
Jonathan Nieder f21233fd0e StreamCopyThread: Remove unused AtomicInteger import
I forgot to do this in 97f3baa0d3
(StreamCopyThread: Remove unnecessary flushCount, 2016-11-13).

Change-Id: Iaed9f345848cf0f854c9d0debcf94bc831d53054
2016-11-13 16:01:16 -08:00
Matthias Sohn 707e4538c2 Merge "Extract insecure Cipher factory" 2016-11-13 19:00:27 -05:00
Jonathan Nieder 2185d84c1a Merge "Get rid of SoftReference in RepositoryCache" 2016-11-13 18:43:57 -05:00
Shawn Pearce 9df75b755f Extract insecure Cipher factory
Bazel runs ErrorProne by default and ErrorProne rightly complains that
allowing the user to specify any Cipher can lead to insecure code
(in particular, getCipher("AES") operates in ECB mode). Unfortunately
this is required to support existing repositories insecurely stored
on S3.

Extract the insecure factory code to its own class so this can be built
as a java_library() with this check disabled.

Change-Id: I34f381965bdaa25d5aa8ebf6d8d5271b238334e0
2016-11-13 19:28:45 -04:00
David Pursehouse f5a40c26ac FileLfsServlet: Make sendError protected
Change-Id: Id89b3cd0da86a535946feb15254737bbf8d855d4
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
2016-11-13 14:31:17 -08:00
David Pursehouse dfa6895687 Make ObjectUploadListener public
Change-Id: I5b0fb1220bc0e5c660f1b5373abc5868d985c60b
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
2016-11-13 14:31:17 -08:00
Jonathan Nieder 96941550de StreamCopyThread: flush cannot interrupt a write
Because flush calls interrupt with writeLock held, it cannot interrupt
a write.  Simplify by no longer defending against that.

Change-Id: Ib0b39b425335ff7b0ea1b1733562da5392576a15
2016-11-13 13:35:16 -08:00
Jonathan Nieder 97f3baa0d3 StreamCopyThread: Remove unnecessary flushCount
StreamCopyThread#run consistently interrupts itself whenever it
discovers it has been interrupted by StreamCopyThread#flush while not
reading.  The flushCount is not needed to avoid lost flushes.

All in-tree users of StreamCopyThread never flush.  As a nice side
benefit, this avoids the expense of atomic operations that have no
purpose for those users.

Change-Id: I1afe415cd09a67f1891c3baf712a9003ad553062
2016-11-13 13:32:08 -08:00
Shawn Pearce 6aa126ec42 Merge "Switch JSchSession to simple isolated OutputStream" 2016-11-13 16:13:04 -05:00
Hugo Arès dea47b9363 Get rid of SoftReference in RepositoryCache
Now that RepositoryCache have a time based eviction strategy, get rid
of the strategy to evict cache entries if heap memory is running low,
i.e. soft references. Main reason why time based eviction was
implemented was to offer an alternative to the unpredictable soft
references.

Relying on soft references is not working, especially in large heap. The
JVM GC will consider collecting soft references as last resort before
throwing an out of memory error. For example, an application like Gerrit
configured with a 128GB heap, GC will wait until all 128GB is filled
before collecting the soft references so the application will be
suffering long pauses caused by GC for a long time already. In other
words, you will have to restart application because it's unusable before
JVM eviction kicks in.

Keeping the SoftReference in RepositoryCache is causing more harm than
good. If you use the time based eviction (which is the default strategy)
and want to tune JVM to release soft references more aggressively, it
will release repositories from the cache even though they are not
expired which defeats the purpose of the repository cache.

Gerrit uses Lucene library which uses soft references and this is
causing a "memory leak" except if you configure JVM to release soft
references more aggressively which have the nasty side effect of
evicting non expired repositories from the cache.

Change-Id: I9940bd800464c7f007696d0ccde52ea617b2ebce
Signed-off-by: Hugo Arès <hugo.ares@ericsson.com>
2016-11-13 16:03:02 -04:00
Shawn Pearce 659cd813a9 Switch JSchSession to simple isolated OutputStream
Work around issues with JSch not handling interrupts by
isolating the JSch interactions onto another thread.

Run write and flush on a single threaded Executor using
simple Callable operations wrapping the method calls,
waiting on the future to determine the outcome before
allowing the caller to continue.

If any operation was interrupted the state of the stream
becomes fuzzy at close time. The implementation tries to
interrupt the pending write or flush, but this is very
likely to corrupt the stream object, so exceptions are
ignored during such a dirty close.

Change-Id: I42e3ba3d8c35a2e40aad340580037ebefbb99b53
2016-11-13 11:02:29 -08:00
Shawn Pearce 92eab1867d WalkEncryption: Cleanup Java 8 support
Java 8 is now the minimum for JGit, so Java 7
only paths are not necessary.

Change-Id: I0151625fed4d0da95321ebed5cca648b8c29d5f1
2016-11-13 12:17:20 -04:00
Philipp Marx df6f2d6860 Reduce synchronized scope around ConcurrentHashMap
Change-Id: I982a78070efb6bc2d3395330456d62e0d5ce6da7
Signed-off-by: Philipp Marx <smigfu@googlemail.com>
2016-11-12 11:11:19 +01:00
Philipp Marx 8adbfe4da6 Check that DfsBlockCache#blockSize is a power of 2
In case a value is used which isn’t a power of 2 there will be a high
chance of java.lang.ArrayIndexOutBoundsException and
org.eclipse.jgit.errors.CorruptObjectException due to a mismatching
assumption for the DfsBlockCache#blockSizeShift parameter.

Change-Id: Ib348b3704edf10b5f93a3ffab4fa6f09cbbae231
Signed-off-by: Philipp Marx <smigfu@googlemail.com>
2016-11-11 10:43:09 +01:00
Matthias Sohn f8ac03459a Fix loop in auto gc
* GC.tooManyLooseObjects() always responded true since the loop missed
to advance the iterator so it always incremented until the threshold was
exceeded.
* Also fix loop exit criterion which was off by 1.
* Add some tests.

Change-Id: I70976dfaa026efbcf3c46bd45941f37277a18e04
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
2016-11-07 22:31:10 +01:00
David Pursehouse 23135e3280 Update buck to latest version
Update to the same version used on Gerrit's master branch.

Change-Id: I20e4edd099a095c42f23df8cc57241efad2de2ce
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
2016-11-06 23:51:46 +01:00
Jonathan Nieder 881e6b2cbb StreamCopyThread: Do not drop data when flush is observed before writing
StreamCopyThread.flush was introduced in
61645b938bc934fda3b0624c5bac1e3495634750 (Add timeouts to smart
transport clients, 2009-06-19) to support timeouts on write in JSch.
The commit message from that change explains:

   JSch made a timeout on write difficult because they explicitly do
   a catch for InterruptedException inside of their OutputStream.  We
   have to work around that by creating an additional thread that just
   shuttles data between our own OutputStream and the real JSch stream.

The code that runs on that thread is structured as follows:

	while (!done) {
		int n = src.read(buf);
		dst.write(buf, 0, n);
	}

with src being a PipedInputStream representing the data to be written
to JSch.  To add flush support, that change wanted to add an extra step

		if (wantFlush)
			dst.flush();

but to handle the case where the thread is blocked in the read() call
waiting for new input, it needs to interrupt the read. So that is how
it works: the caller runs

	pipeOut.write(some data);
	pipeOut.flush();
	copyThread.flush();

to write some data and force it to flush by interrupting the read.

After the pipeOut.flush(), the StreamCopyThread reads the data that was
written and prepares to copy it out.  If the copyThread.flush() call
interrupts the copyThread before it acquires writeLock and starts
writing, we throw away the data we just read to fulfill the flush.
Oops.

Noticed during the review of e67d59df3f
(StreamCopyThread: Do not let flush interrupt a write, 2016-11-04),
which introduced this bug.

Change-Id: I4aceb5610e1bfb251046097adf46bca54bc1d998
2016-11-04 19:33:02 -04:00
Jonathan Nieder e67d59df3f StreamCopyThread: Do not let flush interrupt a write
flush calls interrupt() to interrupt a pending read and trigger a
flush.  Unfortunately that interrupt() call can also interrupt a
pending write, putting Jsch in a bad state and triggering "Short read
of block" errors.  Add locking to ensure the flush only interrupts
reads as intended.

Change-Id: Ib105d9e107ae43549ced7e6da29c22ee41cde9d8
2016-11-04 13:00:08 -07:00