Dave Borowitz [Fri, 29 Apr 2016 14:47:17 +0000 (10:47 -0400)]
PersonIdent: Document that name and email aren't trimmed
This might be somewhat surprising behavior to users who might
naturally assume the following invariant:
ident.equals(parseIdent(ident.toExternalString()))
This invariant does not hold since whitespace is only trimmed during
serialization. We don't want to mess with the strings during
initialization, as this is called during the highly-optimized commit
parsing codepath.
Dave Borowitz [Mon, 25 Apr 2016 17:36:46 +0000 (13:36 -0400)]
Expose the ObjectInserter that created an ObjectReader
We've found in Gerrit Code Review that it is common to pass around
both an ObjectReader (or more commonly a RevWalk wrapping one) and an
ObjectInserter. These code paths often assume that the ObjectReader
can read back any objects created by the ObjectInserter without
flushing. However, we previously had no way to enforce that constraint
programmatically, leading to hard-to-spot problems.
Provide a solution by exposing the ObjectInserter that created an
ObjectReader, when known. Callers can either continue passing both
objects and check:
reader.getCreatedFromInserter() == inserter
or they can just pass around ObjectReader and extract the inserter
when it's needed (checking that it's not null at usage time).
To align with the version used in Gerrit's master build.
Change-Id: I3b6e21bf367ad1fb3598dc06b968aee6187d5aed Signed-off-by: David Pursehouse <david.pursehouse@sonymobile.com> Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
Dave Borowitz [Tue, 19 Apr 2016 13:24:18 +0000 (09:24 -0400)]
Support per-BatchRefUpdate atomic transactions
Repurpose RefDatabase#performsAtomicTransactions() slightly, to
indicate that the backend _supports_ atomic transactions, rather than
the current definition, which is that the backend always _uses_ atomic
transactions regardless of whether or not the caller actually wants
them. Allow BatchRefUpdate callers to turn off atomic transactions by
calling setAtomic(false). Defaulting to true means this is backwards
compatible.
Mike Edgar [Wed, 13 Apr 2016 02:23:08 +0000 (22:23 -0400)]
Make UploadPack observe exceptions reading refs
Now if refs are unreadable when serving an upload pack the handler
will fail due to the actual underlying failure. Previously all wants
would be rejected as invalid because Repository.getAllRefs() returned
an empty map.
Testing this required a new subclass of InMemoryRepository so that
an IOException could be injected at the correct time.
Signed-off-by: Michael Edgar <adgar@google.com>
Change-Id: Iac708b1db9d0ccce08c4ef5ace599ea0b57afdc0
Ned Twigg [Sat, 28 Jun 2014 03:22:03 +0000 (20:22 -0700)]
Expose conflicting files in CheckoutConflictException
Change-Id: I5b3b7b0633354d5ccf0c6c320c0df9c93fdf8eeb Signed-off-by: Ned Twigg <ned.twigg@diffplug.com> Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
Fix CommitCommand to be able to skip writing to RefLog
CommitCommand already provided a method to set the comment which should
be written into the reflog. The underlying RefUpdate class supported to
skip writing a reflog entry. But through the CommitCommand API it was
not possible to prevent writing a reflog entry. Fix this and allow
creating commits which don't occur in the reflog.
Terry Parker [Thu, 7 Apr 2016 23:24:10 +0000 (16:24 -0700)]
Filter corrupt objects from DfsReader.selectObjectRepresentation()
PackWriter.writeObject() can get into an infinite loop when corrupt
packs are present. When it finds a pack file with an object that can be
reused it calls DfsPackFile.copyAsIs(). If that method sees an invalid
CRC, it adds the object to the DfsPackFile's corrupt object list and
throws a CorruptObjectException, which it later catches as an
IOException and wraps in a
StoredObjectRepresentationNotAvailableException.
PackWriter.writeObjectImpl() catches that SORNAE and retries the
operation by calling DfsReader.selectObjectRepresentation(). But
currently that method returns the same object which was just seen to
be corrupt.
Change DfsPackFile.isCorrupt() from private to package private, and use
that method in DfsReader.selectObjectRepresentation() to filter out
corrupt objects.
The stack traces that show the problem are:
org.eclipse.jgit.errors.CorruptObjectException.<init>(CorruptObjectException.java:113)
org.eclipse.jgit.internal.storage.dfs.DfsPackFile.copyAsIs(DfsPackFile.java:624)
org.eclipse.jgit.internal.storage.dfs.DfsReader.copyObjectAsIs(DfsReader.java:491)
org.eclipse.jgit.internal.storage.pack.PackWriter.writeObjectImpl(PackWriter.java:1478)
org.eclipse.jgit.internal.storage.pack.PackWriter.writeObject(PackWriter.java:1455)
Hugo Arès [Fri, 18 Sep 2015 18:05:23 +0000 (14:05 -0400)]
Remove repository from cache when it's closed
RepositoryCache has 2 methods to remove a repository from the cache but
they are never called when a repository is closed. Users of the cache
were expected to call one of those 2 methods but how could they have
called them at proper time without having visibility of the repository
usage count.
Ideally, I would have reworked the RepositoryCache to wrap any
repository it opens in a class that would be responsible to unregister
them from the cache when it's really closed, i.e. when usage counter
reaches 0. The problem preventing the wrapping solution is the
RepositoryCache.register method that allows to register an already
opened repository in the cache. Such repositories cannot be wrapped
because callers are still holding a reference on the unwrapped
repository.
Document that RepositoryCache.close method is removing the repository
from the cache as well as closing it and rework
RepositoryCache.unregister method to only remove the repository from the
cache. Use the latter to unregister repository when Repository.doClose
is getting executed.
Change-Id: Ia364816e4da8d7b6cfa72f10758ca31aa8a1f9db Signed-off-by: Hugo Arès <hugo.ares@ericsson.com> Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
Hugo Arès [Tue, 8 Sep 2015 13:21:31 +0000 (09:21 -0400)]
Fix RefDirectory not closing resources
When repositories are opened using the RepositoryCache, they are kept in
memory and when the repository usage counter reaches 0, the
Repository.close method is called which then calls close method on its
reference and object databases.
The problem is that RefDirectory.close method was a no-op and the
reference database was kept in memory. This problem is only happening
when opening a repository using the RepositoryCache because it never
evicts repositories, it's just calling the close method.
Change-Id: Iacb961de8e8b1f5b37824bf0d1a4caf4c6f1233f Signed-off-by: Hugo Arès <hugo.ares@ericsson.com>
Hugo Arès [Fri, 4 Sep 2015 19:32:28 +0000 (15:32 -0400)]
Fix repository cache never closing repository
Repository has a usage counter that is initialized to 1 at
instantiation and this counter is decremented when Repository.close
method is called. There is also a Repository.incrementOpen method that
RepositoryCache uses to increment the usage count when it's returning a
repository that is already opened.
The problem was that RepositoryCache was incrementing the usage count
for repositories that it just opened or registered. The usage count was
2 when it should have been 1.
Incrementing usage count is now only be done for repository that are
served from the cache.
This bug is causing slow memory increase of our Gerrit server until the
server become slow. Even if the RepositoryCache is using SoftReference,
it seems that the JVM is not garbage collecting the repositories because
it's not yet on the edge of being out of memory.
To test this change, I replicated all repositories(11k) from Gerrit
master to one slave. The Gerrit master used memory after this test was
10GB without this change and 3.5GB with.
Change-Id: I86c7b36174e384f106b51fe92f306018fd1dbdf0 Signed-off-by: Hugo Arès <hugo.ares@ericsson.com>
Enable calling of smudge filters when checking out paths
When checking out commits/branches JGit was triggering correctly
configured smudge filters. But when checking out paths (either from
index or from commits) JGit was not triggering smudge filters. Fix
CheckoutCommand to properly call filters.
Add config parameter gc.prunePackExpire for packfile expiration
JGit's Garbage Collector is repacking relevant objects into new
packfiles and is afterwards deleting the now obsolete packfiles. But to
prevent problems caused by race conditions JGit was not deleting
packfiles when they are too young. The same mechanism as for loose
objects and the config parameter gc.pruneExpire was used.
But JGit was reusing the parameter gc.pruneExpire also for packfiles
which may cause a lot of filesystem consumption if gc.pruneExpire was
set to the default of 2 weeks. Only two weeks after packfile creation gc
was allowed to delete this packfile.
This change introduces a new config paramter gc.prunePackExpire with a
default of "1.hour". This parameter is used when packfiles are deleted.
Only packfiles older than the specified time can be deleted.
For loose objects the behaviour is not changed and only the old
parameter gc.pruneExpire is relevant.
Terry Parker [Fri, 25 Mar 2016 20:55:48 +0000 (13:55 -0700)]
In TestRepository, use a consistent clock
The default author and committer objects in TestRepository were
initialized statically and did not use the MockSystemReader passed into
the TestRepository ctor. Make these fields non-static and initialize
them with a consistent clock.
Also make the author and commiter name and email strings public for
tests that want to verify against them.
Change-Id: I88b444b96e22743001b32824d8e4e03c2239aa86 Signed-off-by: Terry Parker <tparker@google.com>
With ignoreRemoteFailures set to true, we can ignore remote failures
(e.g. the branch of a project described in the manifest file does not
exist), skip that project and continue to the next one, instead of fail
the whole operation.
Change-Id: I8b3765713599e34f1411f9bbc7f575ec7c2384e0 Signed-off-by: Yuxuan 'fishy' Wang <fishywang@google.com>
This commit introduces a FileModeStrategy to
the FileTreeIterator class. This provides a way to
allow different modes of traversing a file tree;
for example, to control whether or not a nested
.git directory should be treated as a gitlink.
Ivan Motsch [Thu, 25 Feb 2016 14:39:41 +0000 (15:39 +0100)]
Add EOL stream type detection to TreeWalk
TreeWalk provides the new method getEolStreamType. This new method can
be used with EolStreamTypeUtil in order to create a wrapped InputStream
or OutputStream when reading / writing files. The implementation
implements support for the git configuration options core.crlf, core.eol
and the .gitattributes "text", "eol" and "binary"
CQ: 10896
Bug: 486563
Change-Id: Ie4f6367afc2a6aec1de56faf95120fff0339a358 Signed-off-by: Ivan Motsch <ivan.motsch@bsiag.com> Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
Matthias Sohn [Fri, 4 Mar 2016 13:20:01 +0000 (14:20 +0100)]
Fix RebuildRefTree trying to add HEAD twice to RefTree
14dfa70520 fixed the problem that HEAD wasn't added to the reftree when
rebuilding the reftree in an empty repository where HEAD isn't yet
resolvable. Since non-resolvable refs are filtered out by
RefDatabase.getRefs(ALL) we have to add HEAD to the reftree explicitly
in this special case.
This fix resulted in another bug: rebuilding the reftree in a repository
which has a resolvable HEAD failed with a DirCacheNameConflictException
in RefTree.apply(). If HEAD is resolvable RefDatabase.getRefs(ALL) does
not filter out HEAD. This results in two identical CREATE commands for
HEAD which RefTree.apply() refuses to execute.
Fix this by no longer creating a duplicate CREATE command for HEAD.
See: I46cbc2611b9ae683ef7319dc46af277925dfaee5
Change-Id: I58dd6bcdef88820aa7de29761d43e2edfa18fcbe Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
Ivan Motsch [Thu, 25 Feb 2016 14:39:41 +0000 (15:39 +0100)]
Tests on Windows with URIish fail
The reason is that URIish(URL) and URIish(String) make different parsing
of path / rawPath with regard to drive letters. /C:/... for URL and
C:/... for String. This patch fixes the issue.
Change-Id: I8e2013fff30b7bb198ff733c038e21366667b8a0 Signed-off-by: Ivan Motsch <ivan.motsch@bsiag.com>
Matthias Sohn [Thu, 18 Feb 2016 22:34:03 +0000 (23:34 +0100)]
Remove unused package export from bundle org.eclipse.jgit.lfs.test
This may have caused the spurious compile errors sometimes observed in
Eclipse since org.eclipse.jgit.lfs.lib is a split package to enable
testing package private code in bundle org.eclipse.jgit.lfs.
Change-Id: I0294448965de8ad8c254b26382386ef2b9f6e863 Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
Matthias Sohn [Wed, 3 Feb 2016 10:34:51 +0000 (11:34 +0100)]
Support Amazon S3 based storage for LFS
Add a storage implementation storing large objects in Amazon S3.
The AmazonS3Repository pre-signs download and upload requests.
AWS access and secret key are expected to be in the
$HOME/.aws/credentials file in the following format:
[default]
accessKey = ...
secretKey = ...
Use AWS version 4 request signing [1] because it is more secure and
supported by all regions. The version 3 signing is not supported in
newer regions.
In follow up changes we should:
- implement getVerifyAction() and do actual verification. Subclasses of
S3Repository can implement caching for object meta data (size) in order
to avoid extra roundtrips to S3. Verification should ensure that meta
data store and content of S3 storage are in sync
- HEAD request used in S3Repository.getSize() seems to always return
Content-length 0 in contrast to the documentation [2]. So getSize() does
detect if the object exists in S3 or not but in case the object exists
it always returns size 0
Change-Id: Ic47f094928a259e5264c92b3aacf6d90210907a8 Signed-off-by: Matthias Sohn <matthias.sohn@sap.com> Signed-off-by: Sasa Zivkov <sasa.zivkov@sap.com>
Shawn Pearce [Wed, 17 Feb 2016 00:41:51 +0000 (16:41 -0800)]
Introduce specific WantNotValidException for servers
Capture the internal "want X not valid" state as a specific subclass
of PackProtocolException, allowing this to be more easily identified
in server stack traces and wrapper application code.
Shawn Pearce [Tue, 16 Feb 2016 23:09:05 +0000 (15:09 -0800)]
smart HTTP server: Pass along "want X not valid" to client
If the client sends a SHA-1 that the server does not recognize echo
this back to the client with an explicit error message instead of
the generic "internal server error".
This was always the intent of the implementation but it was being
dropped on smart HTTP due to the UploadPackServlet catching the
PackProtocolException, discarding the buffered message UploadPack
meant to send, and sending along a generic message instead.
Matthias Sohn [Mon, 15 Feb 2016 22:27:28 +0000 (23:27 +0100)]
Merge branch 'stable-4.2'
* stable-4.2:
Don't use deprecated LockFile constructor
Fix warnings about unchecked conversion of MergeResult
MockServletConfig: Fix warning about unchecked conversion of Enumeration
HugeFileTest: Make Git a class member and open in try-with-resource
Suppress "unchecked cast" warnings related to UploadPackFactory.DISABLED
DiffAlgorithms: Fix warnings about variable hiding
DirCacheBasicTest: Open ObjectInserter.Formatter in try-with-resource
DirCacheBuilderIteratorTest: Open TreeWalk in try-with-resource
DirCacheCGitCompatabilityTest: Open TreeWalk in try-with-resource
DirCacheCheckoutMaliciousPathTest: Open Git and RevWalk in t-w-r
DirCacheIteratorTest: Open TreeWalk instances in try-with-resource
ForPathTest: Open TreeWalk in try-with-resource
GitConstructionTest: Open Git instance in try-with-resource
IndexDiffTest: Open Git instances in try-with-resources
ManifestParserTest: Don't use deprecated StringBufferInputStream
InMemoryRepository: Remove unused RevWalk from batch method signature
IndexModificationTimesTest: Open Git instances in try-with-resource
InterIndexDiffFilterTest: Open TreeWalk in try-with-resource
LockFileTest: Open Git instance in try-with-resource
JGit v4.1.2.201602141800-r
MergeCommandTest: Use JUnit's assume to check preconditions
MergeCommandTest: Open Git instances in try-with-resource
Change-Id: Ie5dba6b9132a29e86958a04fa2b76465bcd2c6b5 Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
David Pursehouse [Mon, 15 Feb 2016 08:41:05 +0000 (17:41 +0900)]
HugeFileTest: Make Git a class member and open in try-with-resource
There's only one test method in this module and it's quite long, so
rather than using a try-with-resource and having to indent a huge
block of existing code, make the Git a member variable that gets
initialised and closed in @Before and @After annotated methods.
The methods are named 'before' and 'after' rather than the conventional
'setUp' and 'tearDown' so as not to conflict with the names of the
existing methods in LocalDiskRepositoryTestCase.
Change-Id: I5a4a9b59f244c450dbcae9fdde7d9e0f0cd24e6f Signed-off-by: David Pursehouse <david.pursehouse@sonymobile.com>
David Pursehouse [Mon, 15 Feb 2016 05:57:39 +0000 (14:57 +0900)]
InMemoryRepository: Remove unused RevWalk from batch method signature
The RevWalk given in the arguments is not used. According to the
comment at the top of the method, a new RevWalk is intentionally
used in the implementation.
Remove the unused argument.
Change-Id: Iec81a1341d5bf377801475845b96a465753096ef Signed-off-by: David Pursehouse <david.pursehouse@sonymobile.com>
David Ostrovsky [Sun, 14 Feb 2016 10:36:25 +0000 (11:36 +0100)]
Buck: Simplify root build file
Remove all proxy rules, that were introduced to allow to build Gerrit
with hijacked JGit cell. New approach suggested in: [1], that emulates
real JGit project structure in its own cell, makes them unnecessary.
Add :all rule, that build all artifacts and packages them in zip file.
Add shell binary :jgit_bin rule, that allows to execute JGit binary
from with buck run command, e.g.:
$ buck run jgit_bin status
$ buck run jgit_bin -- --version
Matthias Sohn [Fri, 12 Feb 2016 10:15:19 +0000 (11:15 +0100)]
Merge branch 'stable-4.2'
* stable-4.2:
NoteMapTest: Open TreeWalk instances in try-with-resource
ObjectDirectoryTest: Fix warnings about variable hiding
PackWriterTest: Open RevWalk in try-with-resource
PatchIdDiffFormatterTest: Open Git and PatchIdDiffFormatter in try-with-resource
PathSuffixFilterTest: Open TreeWalk in try-with-resource
PostOrderTreeWalkTest: Open TreeWalk in try-with-resource
PullCommandWithRebaseTest: Open RevWalk in try-with-resource
PushCommandTest: Open Git instances in try-with-resource
RacyGitTests: Open NameConflictTreeWalk in try-with-resource
RecursiveMergerTest: Open TreeWalk and BufferedReader in try-with-resource
ReflogConfigTest: refactor commit method to avoid variable hiding
Update .mailmap
RefDirectoryTest: Fix warning about member variable hiding
ReflogResolveTest: Open Git instances in try-with-resource
ReflogTest: Open Git instances in try-with-resource
RepoCommandTest: Open Git instances in try-with-resource
Change-Id: I7964b699396629e31a9cc5600aedcf4be4e659a8 Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
David Pursehouse [Fri, 12 Feb 2016 05:41:44 +0000 (14:41 +0900)]
MergeCommandTest: Use JUnit's assume to check preconditions
Using the assume method, instead of just returning, will cause the
test to be marked as skipped rather than passed on systems where
the precondition is not satisfied.
Change-Id: I13672371f6cd3c481a0a6247e0eaed3aac6d766e Signed-off-by: David Pursehouse <david.pursehouse@sonymobile.com>
David Pursehouse [Fri, 12 Feb 2016 04:43:56 +0000 (13:43 +0900)]
ReflogConfigTest: refactor commit method to avoid variable hiding
The author and committer parameters were hiding class members of
the same name.
Since the passed in PersonIdent instances were being created from
the class members anyway, remove the parameters and instead create
the PersonIdent instances inline in the method.
Change-Id: I66b057df388835d57f332fdcbadb8a9f4e1094a4 Signed-off-by: David Pursehouse <david.pursehouse@sonymobile.com>