]> source.dussan.org Git - jgit.git/log
jgit.git
7 years agoDo authentication re-tries on HTTP POST 42/99442/10
Thomas Wolf [Fri, 16 Jun 2017 08:25:53 +0000 (10:25 +0200)]
Do authentication re-tries on HTTP POST

There is at least one git server out there (GOGS) that does
not require authentication on the initial GET for
info/refs?service=git-receive-pack but that _does_ require
authentication for the subsequent POST to actually do the push.

This occurs on GOGS with public repositories; for private
repositories it wants authentication up front.

Handle this behavior by adding 401 handling to our POST request.
Note that this is suboptimal; we'll re-send the push data at
least twice if an authentication failure on POST occurs. It
would be much better if the server required authentication
up-front in the GET request.

Added authentication unit tests (using BASIC auth) to the
SmartClientSmartServerTest:

- clone with authentication
- clone with authentication but lacking CredentialsProvider
- clone with authentication and wrong password
- clone with authentication after redirect
- clone with authentication only on POST, but not on GET

Also tested manually in the wild using repositories at try.gogs.io.
That server offers only BASIC auth, so the other paths
(DIGEST, NEGOTIATE, fall back from DIGEST to BASIC) are untested
and I have no way to test them.

* public repository: GET unauthenticated, POST authenticated
  Also tested after clearing the credentials and then entering a
  wrong password: correctly asks three times during the HTTP
  POST for user name and password, then gives up.
* private repository: authentication already on GET; then gets
  applied correctly initially to the POST request, which succeeds.

Also fix the authentication to use the credentials for the redirected
URI if redirects had occurred. We must not present the credentials
for the original URI in that case. Consider a malicious redirect A->B:
this would allow server B to harvest the user credentials for server
A. The unit test for authentication after a redirect also tests for
this.

Bug: 513043
Change-Id: I97ee5058569efa1545a6c6f6edfd2b357c40592a
Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
7 years agoMerge changes Id3994e2d,I5e2a2868,I255af794
David Pursehouse [Fri, 18 Aug 2017 21:05:41 +0000 (17:05 -0400)]
Merge changes Id3994e2d,I5e2a2868,I255af794

* changes:
  LongObjectIdTest: Add back self comparison test
  Format BUILD files with buildifier
  Bazel: Add missing dependency in org.eclipse.jgit.http.test

7 years agoLongObjectIdTest: Add back self comparison test 65/103265/2
David Pursehouse [Thu, 17 Aug 2017 22:15:30 +0000 (23:15 +0100)]
LongObjectIdTest: Add back self comparison test

The test was removed in 4886621 to prevent a warning from
error-prone.

Add it back but rewrite it in a way that does not cause the
warning.

This reverts commit 4886621261c3b5fa2d9c75fd72d19ac186c9f970.

Change-Id: Id3994e2d882a9d08bf548b7778406f8a80fbf830
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
7 years agoFormat BUILD files with buildifier 64/103264/2
David Pursehouse [Thu, 17 Aug 2017 22:22:11 +0000 (23:22 +0100)]
Format BUILD files with buildifier

Change-Id: I5e2a286866b63a8fa2bd29cc2fe432fab2bbe0af
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
7 years agoBazel: Add missing dependency in org.eclipse.jgit.http.test 63/103263/2
David Pursehouse [Thu, 17 Aug 2017 22:21:22 +0000 (23:21 +0100)]
Bazel: Add missing dependency in org.eclipse.jgit.http.test

Change-Id: I255af794856371fdf1a1eceb6bca50a35b71b519
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
7 years agoreftable: reserve standard PackExt 65/101365/26
Shawn Pearce [Mon, 17 Jul 2017 15:40:23 +0000 (08:40 -0700)]
reftable: reserve standard PackExt

Reserve "ref" extension for reftable files.  This allows them to be
used in a DFS repository as a stream in a DfsPackDescription.

Change-Id: Ife781bb64d0bb063333183ad2be70a41a2482513

7 years agoreftable: resolve symbolic references 50/101850/21
Shawn Pearce [Mon, 24 Jul 2017 16:01:55 +0000 (09:01 -0700)]
reftable: resolve symbolic references

resolve(Ref) helps callers recursively chase symbolic references and
is a useful function when wrapping a Reftable inside a RefDatabase, as
RefCursor does not resolve symbolic references during iteration.

Change-Id: I1ba143f403773497972e225dc92c35ecb989e154

7 years agoreftable: support threshold based compaction 24/101224/27
Shawn Pearce [Thu, 13 Jul 2017 18:24:13 +0000 (11:24 -0700)]
reftable: support threshold based compaction

Transactions may wish to merge several tables together as part of an
operation.  Setting a byte limit allows the transaction to consider
only some recent tables, bounding the cost of the compaction.

Change-Id: If037f2cbdc174ff1a215d5917178b33cde4ddaba

7 years agoreftable: compact merged tables 23/101223/27
Shawn Pearce [Thu, 13 Jul 2017 18:04:20 +0000 (11:04 -0700)]
reftable: compact merged tables

A compaction of reftables is just copying the results of a
MergedReftable into a ReftableWriter.  Wrap this up into a utility.

Change-Id: I6f5677d923e9628993a2d8b4b007a9b8662c9045

7 years agoreftable: merge-join reftables 22/101222/27
Shawn Pearce [Thu, 13 Jul 2017 17:44:24 +0000 (10:44 -0700)]
reftable: merge-join reftables

MergedReftable combines multiple reference tables together in a stack,
allowing higher/later tables to shadow earlier/lower tables.  This
forms the basis of a transaction system, where each transaction writes
a new reftable containing only the modified references, and readers
perform a merge on the fly to get the latest value.

Change-Id: Ic2cb750141e8c61a8b2726b2eb95195acb6ddc83

7 years agoreftable: lookup by ObjectId unit tests 75/101775/17
Shawn Pearce [Fri, 21 Jul 2017 19:40:33 +0000 (12:40 -0700)]
reftable: lookup by ObjectId unit tests

Change-Id: Ic819a04e285094e271435dcd027d8006e5897785

7 years agoreftable: reflog unit tests 13/101313/22
Shawn Pearce [Sat, 15 Jul 2017 21:58:04 +0000 (14:58 -0700)]
reftable: reflog unit tests

Change-Id: If719a63ead54ecbcaf7cbe12c71f00435706bc2b

7 years agoreftable: namespace unit tests 52/101152/24
Shawn Pearce [Wed, 12 Jul 2017 23:15:56 +0000 (16:15 -0700)]
reftable: namespace unit tests

Add additional test cases for looking up entries within a namespace
such as refs/heads/ or refs/tags/, where the seek is passed a name
that ends with '/'.

Change-Id: I5f944de7518cd0090374bddba48d4dd3955a8d72

7 years agoreftable: bulk operation unit tests 51/101151/24
Shawn Pearce [Wed, 12 Jul 2017 23:09:39 +0000 (16:09 -0700)]
reftable: bulk operation unit tests

Add more test cases that cover larger collections of
references, verifying every reference is accessible
both by scan and by seek.

Change-Id: Icada59fdcfc92a4634f6df61baaebb1c37b75d98

7 years agoreftable: basic functionality unit tests 50/101150/24
Shawn Pearce [Wed, 12 Jul 2017 22:05:22 +0000 (15:05 -0700)]
reftable: basic functionality unit tests

This set of tests covers primitive storage of an empty
file, and each type of supported reference.

Change-Id: I3bdff35cae8ae27283051932f20608b3ac353559

7 years agoreftable: debug tools 49/101149/24
Shawn Pearce [Wed, 12 Jul 2017 19:44:03 +0000 (12:44 -0700)]
reftable: debug tools

Simple debug programs to experiment with the reftable file format:

  debug-read-reftable
  debug-write-reftable
  debug-verify-reftable
  debug-benchmark-reftable

Change-Id: I79db351d86900f1e58b17e922e195dff06ee71f1

7 years agoreftable: scan and lookup reftable files 48/101148/24
Shawn Pearce [Wed, 12 Jul 2017 21:01:50 +0000 (14:01 -0700)]
reftable: scan and lookup reftable files

ReftableReader provides sequential scanning support over all
references, a range of references within a subtree (such as
"refs/heads/"), and lookup of a single reference.  Reads can be
accelerated by an index block, if it was created by the writer.

The BlockSource interface provides an abstraction to read from the
reftable's backing storage, supporting a future commit to connect
to JGit DFS and the DfsBlockCache.

Change-Id: Ib0dc5fa937d0c735f2a9ff4439d55c457fea7aa8

7 years agoreftable: create and write reftable files 47/101147/24
Shawn Pearce [Sun, 9 Jul 2017 17:38:24 +0000 (10:38 -0700)]
reftable: create and write reftable files

This is a simple writer to create reftable formatted files.  Follow-up
commits will add support for reading from reftable, debugging
utilities, and tests.

Change-Id: I3d520c3515c580144490b0b45433ea175a3e6e11

7 years agoreftable: file format documentation 74/101774/16
Shawn Pearce [Sat, 22 Jul 2017 17:12:32 +0000 (10:12 -0700)]
reftable: file format documentation

Some repositories contain a lot of references (e.g. android at 866k,
rails at 31k). The reftable format provides:

- Near constant time lookup for any single reference, even when the
  repository is cold and not in process or kernel cache.
- Near constant time verification a SHA-1 is referred to by at least
  one reference (for allow-tip-sha1-in-want).
- Efficient lookup of an entire namespace, such as `refs/tags/`.
- Support atomic push `O(size_of_update)` operations.
- Combine reflog storage with ref storage.

Change-Id: I29d0ff1eee475845660ac9173413e1407adcfbf2

7 years agoAdd support to follow HTTP redirects 61/46261/19
Thomas Wolf [Wed, 22 Apr 2015 15:05:12 +0000 (17:05 +0200)]
Add support to follow HTTP redirects

git-core follows HTTP redirects so JGit should also provide this.

Implement config setting http.followRedirects with possible values
"false" (= never), "true" (= always), and "initial" (only on GET, but
not on POST).[1]

We must do our own redirect handling and cannot rely on the support
that the underlying real connection may offer. At least the JDK's
HttpURLConnection has two features that get in the way:

* it does not allow cross-protocol redirects and thus fails on
  http->https redirects (for instance, on Github).
* it translates a redirect after a POST to a GET unless the system
  property "http.strictPostRedirect" is set to true. We don't want
  to manipulate that system setting nor require it.

Additionally, git has its own rules about what redirects it accepts;[2]
for instance, it does not allow a redirect that adds query arguments.

We handle response codes 301, 302, 303, and 307 as per RFC 2616.[3]
On POST we do not handle 303, and we follow redirects only if
http.followRedirects == true.

Redirects are followed only a certain number of times. There are two
ways to control that limit:

* by default, the limit is given by the http.maxRedirects system
  property that is also used by the JDK. If the system property is
  not set, the default is 5. (This is much lower than the JDK default
  of 20, but I don't see the value of following so many redirects.)
* this can be overwritten by a http.maxRedirects git config setting.

The JGit http.* git config settings are currently all global; JGit has
no support yet for URI-specific settings "http.<pattern>.name". Adding
support for that is well beyond the scope of this change.

Like git-core, we log every redirect attempt (LOG.info) so that users
may know about the redirection having occurred.

Extends the test framework to configure an AppServer with HTTPS support
so that we can test cloning via HTTPS and redirections involving HTTPS.

[1] https://git-scm.com/docs/git-config
[2] https://kernel.googlesource.com/pub/scm/git/git/+/6628eb41db5189c0cdfdced6d8697e7c813c5f0f
[3] https://www.w3.org/Protocols/rfc2616/rfc2616-sec10.html

CQ: 13987
Bug: 465167
Change-Id: I86518cb76842f7d326b51f8715e3bbf8ada89859
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>
7 years agoMerge "Fix off-by-one error in Strings.count()"
Christian Halstrick [Wed, 16 Aug 2017 10:24:43 +0000 (06:24 -0400)]
Merge "Fix off-by-one error in Strings.count()"

7 years agoMerge "Use relative paths for attribute rule matching"
Christian Halstrick [Wed, 16 Aug 2017 10:24:33 +0000 (06:24 -0400)]
Merge "Use relative paths for attribute rule matching"

7 years agoSend a detailed event on working tree modifications 16/100916/8
Thomas Wolf [Fri, 7 Jul 2017 09:26:02 +0000 (11:26 +0200)]
Send a detailed event on working tree modifications

Currently there is no way to determine the precise changes done
to the working tree by a JGit command. Only the CheckoutCommand
actually provides access to the lists of modified, deleted, and
to-be-deleted files, but those lists may be inaccurate (since they
are determined up-front before the working tree is modified) if
the actual checkout then fails halfway through. Moreover, other
JGit commands that modify the working tree do not offer any way to
figure out which files were changed.

This poses problems for EGit, which may need to refresh parts of the
Eclipse workspace when JGit has done java.io file operations.

Provide the foundations for better file change tracking: the working
tree is modified exclusively in DirCacheCheckout. Make it emit a new
type of RepositoryEvent that lists all files that were modified or
deleted, even if the checkout failed halfway through. We update the
'updated' and 'removed' lists determined up-front in case of file
system problems to reflect the actual state of changes made.

EGit thus can register a listener for these events and then knows
exactly which parts of the Eclipse workspace may need to be refreshed.

Two commands manage checking out individual DirCacheEntries themselves:
checkout specific paths, and applying a stash with untracked files.
Make those two also emit such a new WorkingTreeModifiedEvent.

Furthermore, merges may modify files, and clean, rm, and stash create
may delete files.

CQ: 13969
Bug: 500106
Change-Id: I7a100aee315791fa1201f43bbad61fbae60b35cb
Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>
7 years agoMerge branch 'stable-4.8' 77/103077/2
Matthias Sohn [Mon, 14 Aug 2017 22:25:41 +0000 (00:25 +0200)]
Merge branch 'stable-4.8'

* stable-4.8:
  Update Oxygen Orbit p2 repository to R20170516192513
  Fix exception handling for opening bitmap index files

Change-Id: Ica20f5aa0d8a365fe3317765b93520b3abd5d342
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
7 years agoMerge branch 'stable-4.7' into stable-4.8 76/103076/2
Matthias Sohn [Mon, 14 Aug 2017 22:07:23 +0000 (00:07 +0200)]
Merge branch 'stable-4.7' into stable-4.8

* stable-4.7:
  Update Oxygen Orbit p2 repository to R20170516192513
  Fix exception handling for opening bitmap index files

Change-Id: I1e4fcf84506ff4316567bbb1713e84d8d196c2a1
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
7 years agoMerge branch 'stable-4.6' into stable-4.7 75/103075/2
Matthias Sohn [Mon, 14 Aug 2017 21:50:52 +0000 (23:50 +0200)]
Merge branch 'stable-4.6' into stable-4.7

* stable-4.6:
  Update Oxygen Orbit p2 repository to R20170516192513
  Fix exception handling for opening bitmap index files

Change-Id: I669fe48ce0034f9ea1977d38ee39099497422c1c
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
7 years agoMerge branch 'stable-4.5' into stable-4.6 74/103074/2
Matthias Sohn [Mon, 14 Aug 2017 21:37:44 +0000 (23:37 +0200)]
Merge branch 'stable-4.5' into stable-4.6

* stable-4.5:
  Fix exception handling for opening bitmap index files

Change-Id: Ifb511238e3e98b1bc9f79a990807b940a17ebaa6
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
7 years agoUpdate Oxygen Orbit p2 repository to R20170516192513 80/103080/1
Matthias Sohn [Mon, 14 Aug 2017 21:37:21 +0000 (23:37 +0200)]
Update Oxygen Orbit p2 repository to R20170516192513

Change-Id: I7f1b733ca414d77c9df5572df02e742e0a84ba2f
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
7 years agoFix exception handling for opening bitmap index files 98/102598/3
Christian Halstrick [Mon, 7 Aug 2017 12:26:46 +0000 (14:26 +0200)]
Fix exception handling for opening bitmap index files

When creating a new PackFile instance it is specified whether this pack
has an associated bitmap index file or not. This information is cached
and the public method getBitmapIndex() will always assume a bitmap index
file must exist if the cached data tells so. But it may happen that the
packfiles are repacked during a gc in a different process causing the
packfile, bitmap-index and index file to be deleted. Since JGit still
has an open FileHandle on the packfile this file is not really deleted
and can still be accessed. But index and bitmap index file are deleted.
Fix getBitmapIndex() to invalidate the cached packfile instance if such
a situation occurs.

This problem showed up when a gerrit server was serving repositories
which where garbage collected with native git regularly. Fetch and
clone commands for certain repositories failed permanently after a
native git gc had deleted old bitmap index files.

Change-Id: I8e620bec74dd3f310ba42024f9a657062f868f0e
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
7 years agoDo not apply pushInsteadOf to existing pushUris 54/103054/2
Thomas Wolf [Mon, 14 Aug 2017 14:09:29 +0000 (16:09 +0200)]
Do not apply pushInsteadOf to existing pushUris

Per the git config documentation[1], pushInsteadOf is ignored when
a remote has explicit pushUris.

Implement this, and adapt tests.

Up to now JGit mistakenly applied pushInsteadOf also to existing
pushUris. If some repositories had relied on this mis-feature,
pushes may newly suddenly fail (the uncritical case; the config
just needs to be fixed) or even still succeed but push to unexpected
places, namely to the non-rewritten pushUrls (the critical case).

The release notes should point out this change.

[1] https://git-scm.com/docs/git-config

Bug: 393170
Change-Id: I38c83204d2ac74f88f3d22d0550bf5ff7ee86daf
Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>
7 years agoRecord submodule paths with untracked changes as FileMode.GITLINK 79/102879/4
Thomas Wolf [Thu, 10 Aug 2017 13:03:22 +0000 (15:03 +0200)]
Record submodule paths with untracked changes as FileMode.GITLINK

Bug: 520702
Change-Id: I9bb48af9e8f1f2ce7968a82297c7c16f1237f987
Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>
7 years agoFix handling of pushInsteadOf 35/99735/2
Thomas Wolf [Wed, 21 Jun 2017 09:40:04 +0000 (11:40 +0200)]
Fix handling of pushInsteadOf

According to [1], pushInsteadOf is

1. applied to the uris, not to the pushUris
2. ignored if a remote has an explicit pushUri

JGit applied it only to the pushUris. As a result, pushInsteadOf was
ignored for remotes having only a uri, but no pushUri.

This commit implements (1) if there are no pushUris. I did not dare
implement (2) because:

* there are explicit tests for it that expect that pushInsteadOf gets
  applied to existing pushUrls, and
* people may actually use and rely on this JGit behavior.

[1] https://git-scm.com/docs/git-config

Bug: 393170
Change-Id: I6dacbf1768a105190c2a8c5272e7880c1c9c943a
Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>
7 years agoMerge "Ensure EOL stream type is DIRECT when -text attribute is present"
Christian Halstrick [Mon, 14 Aug 2017 07:34:57 +0000 (03:34 -0400)]
Merge "Ensure EOL stream type is DIRECT when -text attribute is present"

7 years agoFix off-by-one error in Strings.count() 35/103035/1
Thomas Wolf [Mon, 14 Aug 2017 06:04:56 +0000 (08:04 +0200)]
Fix off-by-one error in Strings.count()

Change-Id: I0667b1624827d1cf0cc1b81f86c7bb44eafd68a7
Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>
7 years agoRename extensions.refsStorage to refStorage 78/102578/5
Shawn Pearce [Sun, 6 Aug 2017 19:35:17 +0000 (12:35 -0700)]
Rename extensions.refsStorage to refStorage

This matches the proposal that has been discussed at length on
git-core mailing list and seems to be the accepted convention.

Change-Id: I9f6ab15144826893d1e2a4b48a2d657d6dd445ec

7 years agoEnsure EOL stream type is DIRECT when -text attribute is present 97/102997/2
Thomas Wolf [Fri, 11 Aug 2017 20:48:50 +0000 (22:48 +0200)]
Ensure EOL stream type is DIRECT when -text attribute is present

Otherwise fancy combinations of attributes (binary or -text in
combination with crlf or eol) may result in the corruption of binary
data.

Bug: 520910
Change-Id: I3ffc666c13d1b9d2ed987b69a67bfc7f42ccdbfc
Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>
7 years agoUse relative paths for attribute rule matching 94/102994/3
Thomas Wolf [Fri, 11 Aug 2017 19:16:54 +0000 (21:16 +0200)]
Use relative paths for attribute rule matching

Attribute rules must match against the entry path relative to the
attribute node containing the rule. The global entry path is to be
used only for the init and the global node (and of course the root
node).

Bug: 520677
Change-Id: I80389a2dc272a72312729ccd5358d7c75e1ea20a
Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>
7 years agoExpose LongMap in util package 77/102577/3
Shawn Pearce [Sat, 5 Aug 2017 00:16:02 +0000 (17:16 -0700)]
Expose LongMap in util package

This is a useful primitive collection type like IntList.

Change-Id: I04b9b2ba25247df056eb3a1725602f1be6d3b440

7 years agoNB: encode and decode 24-bit ints 93/102393/3
Shawn Pearce [Tue, 1 Aug 2017 15:49:33 +0000 (08:49 -0700)]
NB: encode and decode 24-bit ints

Change-Id: Ie036dc46e5a88a4e87dc52e880505bbe34601ca7

7 years agoUpdate thread-safety warning about Repository 87/102687/1
Shawn Pearce [Tue, 8 Aug 2017 13:44:35 +0000 (06:44 -0700)]
Update thread-safety warning about Repository

Change-Id: I1026a77cc688467d5a89a41121146f1bd3d56fa5

7 years agoReflogWriter: Minor cleanup 94/102394/2
Dave Borowitz [Tue, 1 Aug 2017 13:00:50 +0000 (09:00 -0400)]
ReflogWriter: Minor cleanup

Remove unnecessary finals, use consistent punctuation in Javadoc, reflow
some lines, etc.

Change-Id: Ic64db41c86917725ac649022290621406156bcc4

7 years agoEliminate SectionParser construction boilerplate 21/102421/1
Dave Borowitz [Wed, 2 Aug 2017 20:50:57 +0000 (16:50 -0400)]
Eliminate SectionParser construction boilerplate

Happily, most anonymous SectionParser implementations can be replaced
with FooConfig::new, as long as the constructor takes a single Config
arg. Many of these, the non-public ones, can in turn be inlined. A few
remaining SectionParsers can be lambdas.

Change-Id: I3f563e752dfd2007dd3a48d6d313d20e2685943a

7 years agoMerge changes Ia6c3935c,I46a29eae
David Pursehouse [Wed, 2 Aug 2017 07:12:16 +0000 (03:12 -0400)]
Merge changes Ia6c3935c,I46a29eae

* changes:
  Remove unused import introduced in a551b64
  Silence API errors caused by adding enum constants in dbb137e

7 years agoMerge "git ignore .DS_Store"
Matthias Sohn [Tue, 1 Aug 2017 21:30:00 +0000 (17:30 -0400)]
Merge "git ignore .DS_Store"

7 years agoMerge "git ignore all bazel folders"
Matthias Sohn [Tue, 1 Aug 2017 21:28:48 +0000 (17:28 -0400)]
Merge "git ignore all bazel folders"

7 years agoRemove unused import introduced in a551b64 33/102333/2
Matthias Sohn [Tue, 1 Aug 2017 21:22:56 +0000 (23:22 +0200)]
Remove unused import introduced in a551b64

Change-Id: Ia6c3935cf061590e7305d0a80a1051e9aebcbb43
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
7 years agoSilence API errors caused by adding enum constants in dbb137e 32/102332/2
Matthias Sohn [Tue, 1 Aug 2017 21:21:15 +0000 (23:21 +0200)]
Silence API errors caused by adding enum constants in dbb137e

Change-Id: I46a29eae7b617f3f43f270c40072a1c103ef77f2
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
7 years agoMerge changes I424295df,Ib003f7c8
David Pursehouse [Tue, 1 Aug 2017 14:18:48 +0000 (10:18 -0400)]
Merge changes I424295df,Ib003f7c8

* changes:
  Treat RawText of binary data as file with one single line.
  Trim boilerplate in RawParseUtils_LineMapTest.

7 years agoTreat RawText of binary data as file with one single line. 76/102076/3
Han-Wen Nienhuys [Thu, 27 Jul 2017 11:58:49 +0000 (13:58 +0200)]
Treat RawText of binary data as file with one single line.

This avoids executing mergeAlgorithm.merge on binary data, which is
unlikely to be useful.

Arguably, binary data should not make it to
ResolveMerger#contentMerge, but this approach has the following
advantages:

* binary detection is exact, since it doesn't only look at the start
  of the blob.

* it is cheap, as we have to iterate over the bytes anyway to find
  '\n'.

Signed-off-by: Han-Wen Nienhuys <hanwen@google.com>
Change-Id: I424295df1dc60a719859d9d7c599067891b15792

7 years agoMerge "Use w1 for hashCode of AbbreviatedObjectId"
Terry Parker [Fri, 28 Jul 2017 23:24:11 +0000 (19:24 -0400)]
Merge "Use w1 for hashCode of AbbreviatedObjectId"

7 years agoMerge "IntList: support contains(int)"
David Pursehouse [Fri, 28 Jul 2017 18:18:21 +0000 (14:18 -0400)]
Merge "IntList: support contains(int)"

7 years agoMerge "Replace findbugs by spotbugs"
David Pursehouse [Fri, 28 Jul 2017 17:47:21 +0000 (13:47 -0400)]
Merge "Replace findbugs by spotbugs"

7 years agoUse w1 for hashCode of AbbreviatedObjectId 73/101773/2
Shawn Pearce [Sat, 22 Jul 2017 17:06:25 +0000 (10:06 -0700)]
Use w1 for hashCode of AbbreviatedObjectId

Very short abbreviations that are under 8 hex digits do not
have values in w2. Use w1 as the Java hashCode() instead, so
that the prefix of the abbreviation is always included in the
hashing function used by any java.util.Collection type.

Change-Id: Idaf69f86b62630ba4a022d31b4c293c6d138f557

7 years agoIntList: support contains(int) 72/101772/2
Shawn Pearce [Thu, 20 Jul 2017 08:51:44 +0000 (01:51 -0700)]
IntList: support contains(int)

LongList supports contains(long).
IntList should also support contains(int).

Change-Id: Ic7a81c3c25b0f10d92087b56e9f200b676060f63

7 years agoReplace findbugs by spotbugs 12/101312/3
Matthias Sohn [Sun, 16 Jul 2017 22:31:13 +0000 (00:31 +0200)]
Replace findbugs by spotbugs

SpotBugs [1] is the spiritual successor of FindBugs, carrying on from
the point where it left off with support of its community.

[1] http://spotbugs.readthedocs.io/

Change-Id: I127f2c54b04265b6565e780116617ffa8a4d7eaf
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
7 years agoRefDirectory: Add in-process fair lock for atomic updates 62/102162/2
Dave Borowitz [Fri, 28 Jul 2017 14:48:25 +0000 (10:48 -0400)]
RefDirectory: Add in-process fair lock for atomic updates

In a server scenario such as Gerrit Code Review, there may be many
atomic BatchRefUpdates contending for locks on both the packed-refs file
and some subset of loose refs. We already retry lock acquisition to
improve this situation slightly, but we can do better by using an
in-process lock. This way, instead of retrying and potentially exceeding
their timeout, different threads sharing the same Repository instance
can wait on a fair lock without having to touch the disk lock. Since a
server is probably already using RepositoryCache anyway, there is a high
likelihood of reusing the Repository instance.

Change-Id: If5dd1dc58f0ce62f26131fd5965a0e21a80e8bd3

7 years agoRefDirectory: Retry acquiring ref locks with backoff 31/102031/5
Dave Borowitz [Wed, 26 Jul 2017 20:53:21 +0000 (16:53 -0400)]
RefDirectory: Retry acquiring ref locks with backoff

If a repo frequently uses PackedBatchRefUpdates, there is likely to be
contention on the packed-refs file, so it's not appropriate to fail
immediately the first time we fail to acquire a lock. Add some logic to
RefDirectory to support general retrying of lock acquisition.

Currently, there is a hard-coded wait starting at 100ms and backing off
exponentially to 1600ms, for about 3s of total wait. This is no worse
than the hard-coded backoff that JGit does elsewhere, e.g. in
FileUtils#delete. One can imagine a scheme that uses per-repository
configuration of backoff, and the current interface would support this
without changing any callers.

Change-Id: I4764e11270d9336882483eb698f67a78a401c251

7 years agogit ignore .DS_Store 89/101889/2
David Pursehouse [Tue, 25 Jul 2017 09:39:50 +0000 (10:39 +0100)]
git ignore .DS_Store

Change-Id: I04887b91d07ee93f51d27c9a3597e258b9affc0b
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
7 years agogit ignore all bazel folders 88/101888/2
David Pursehouse [Tue, 25 Jul 2017 09:39:24 +0000 (10:39 +0100)]
git ignore all bazel folders

Change-Id: I38ae02d07020d8ccb8dc1ac4c0fa08ce93512f94
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
7 years agoMerge "Fix committing empty commits"
David Pursehouse [Fri, 28 Jul 2017 10:08:33 +0000 (06:08 -0400)]
Merge "Fix committing empty commits"

7 years agoMerge "Support overriding a batch's reflog on a per-ReceiveCommand basis"
David Pursehouse [Fri, 28 Jul 2017 10:07:08 +0000 (06:07 -0400)]
Merge "Support overriding a batch's reflog on a per-ReceiveCommand basis"

7 years agoFix committing empty commits 41/100641/3
Christian Halstrick [Tue, 4 Jul 2017 14:39:28 +0000 (16:39 +0200)]
Fix committing empty commits

Allow to explicitly create an empty commit even if committing only
certain files.

Bug: 510685
Change-Id: If9bf664d7cd824f8e5bd6765fa6cc739af3d7721

7 years agoMerge changes from topic 'batch-ref-update-reflog'
David Pursehouse [Fri, 28 Jul 2017 09:40:45 +0000 (05:40 -0400)]
Merge changes from topic 'batch-ref-update-reflog'

* changes:
  BatchRefUpdate: Expand javadocs and add @Nullable
  PackedBatchRefUpdate: Write reflogs
  Extract constants for reflog entry message prefixes

7 years agoTrim boilerplate in RawParseUtils_LineMapTest. 75/102075/1
Han-Wen Nienhuys [Thu, 27 Jul 2017 11:49:39 +0000 (13:49 +0200)]
Trim boilerplate in RawParseUtils_LineMapTest.

Signed-off-by: Han-Wen Nienhuys <hanwen@google.com>
Change-Id: Ib003f7c8f2816dd57e941799a665e70ecd6645a2

7 years agoAdd commit check for head references 58/101758/4
Zhen Chen [Fri, 21 Jul 2017 00:20:55 +0000 (17:20 -0700)]
Add commit check for head references

Make sure all refs/heads/* point to a commit object.

Change-Id: I9c7cf347aaf63d5ef604d520c2383c6cf3043890
Signed-off-by: Zhen Chen <czhen@google.com>
7 years agoAdd connectivity check from references 99/101299/8
Zhen Chen [Fri, 14 Jul 2017 18:42:34 +0000 (11:42 -0700)]
Add connectivity check from references

Make sure all objects referenced by references are reachable. Stop at
the first missing object.

Change-Id: Ifcd7392c4321b17d9290bd87f038bc62bc10dabb
Signed-off-by: Zhen Chen <czhen@google.com>
7 years agoAdd dfs fsck implementation 07/99707/13
Zhen Chen [Tue, 20 Jun 2017 22:22:09 +0000 (15:22 -0700)]
Add dfs fsck implementation

JGit already had some fsck-like classes like ObjectChecker which can
check for an individual object.

The read-only FsckPackParser which will parse all objects within a pack
file and check it with ObjectChecker. It will also check the pack index
file against the object information from the pack parser.

Change-Id: Ifd8e0d28eb68ff0b8edd2b51b2fa3a50a544c855
Signed-off-by: Zhen Chen <czhen@google.com>
7 years agoSupport overriding a batch's reflog on a per-ReceiveCommand basis 84/101584/3
Dave Borowitz [Wed, 19 Jul 2017 17:55:30 +0000 (13:55 -0400)]
Support overriding a batch's reflog on a per-ReceiveCommand basis

Change-Id: I86a4b8f6b4f85b2bae64c1b121e4ee527d46de83

7 years agoBatchRefUpdate: Expand javadocs and add @Nullable 83/101583/3
Dave Borowitz [Wed, 19 Jul 2017 17:55:16 +0000 (13:55 -0400)]
BatchRefUpdate: Expand javadocs and add @Nullable

Change-Id: I22d739a9677e24f36323dceadf7d375ac2f446e8

7 years agoPackedBatchRefUpdate: Write reflogs 57/101457/4
Dave Borowitz [Mon, 17 Jul 2017 14:01:10 +0000 (10:01 -0400)]
PackedBatchRefUpdate: Write reflogs

On-disk reflogs are not stored in the packed-refs file, so we cannot
ensure atomic updates. We choose the lesser evil of dropping failed
reflog updates on the floor, rather than throwing an exception even
though the underlying ref updates succeeded.

Add tests for reflogs to BatchRefUpdateTest.

Change-Id: Ia456ba9e36af8e01fde81b19af46a72378e614cd

7 years agoImprove BatchRefUpdateTest readability 59/101359/5
Dave Borowitz [Mon, 17 Jul 2017 15:49:36 +0000 (11:49 -0400)]
Improve BatchRefUpdateTest readability

* Factor out helpers for setting up and executing updates.
* Use common assert methods, with a special enum type that papers over
  the fact that there is no ReceiveCommand.Result for transaction
  aborted.
* Static import ReceiveCommand.Type constants.
* Add blank lines to separate repo setup, update execution, and asserts.

Change-Id: Ic3717f94331abfc7ae3e92065f3fe32026bf7cea

7 years agoExtract constants for reflog entry message prefixes 56/101456/2
Dave Borowitz [Mon, 17 Jul 2017 16:25:09 +0000 (12:25 -0400)]
Extract constants for reflog entry message prefixes

Document explicitly that these are untranslated to (mostly) match C git.

Change-Id: I3abcffb4fd611d053bf4373e5d6a14a66f7b9b6b

7 years agoMove BatchRefUpdate tests to a new file 58/101358/5
Dave Borowitz [Mon, 17 Jul 2017 15:03:38 +0000 (11:03 -0400)]
Move BatchRefUpdate tests to a new file

Run with @Parameterized, so we don't have to duplicate test setup for
each atomic/non-atomic test. We still have to have two different sets of
asserts for the cases where the behavior is different. In fact, this is
a readability win: it emphasizes that performing the exact same setup
except for the atomic setting will have different behavior.

Change-Id: I78a8214075e204732a423341f14c09de273a7854

7 years agoImplement atomic BatchRefUpdates for RefDirectory 71/100771/9
Dave Borowitz [Wed, 5 Jul 2017 17:45:39 +0000 (13:45 -0400)]
Implement atomic BatchRefUpdates for RefDirectory

The existing packed-refs file provides a mechanism for implementing
atomic multi-ref updates without any changes to the on-disk format or
lockfile protocol. We just need to make sure that there are no loose
refs involved in the transaction, which we can achieve by packing the
refs while holding locks on all loose refs. Full details of the
algorithm are in the PackedBatchRefUpdate javadoc.

This change does not implement reflog support, which will come in a
later change.

Change-Id: I09829544a0d4e8dbb141d28c748c3b96ef66fee1

7 years agoSeparate RefUpdate.Result.REJECTED_{MISSING_OBJECT,OTHER_REASON} 32/100932/6
Dave Borowitz [Fri, 7 Jul 2017 15:38:29 +0000 (11:38 -0400)]
Separate RefUpdate.Result.REJECTED_{MISSING_OBJECT,OTHER_REASON}

ReceiveCommand.Result has a slightly richer set of possibilities, so it
makes sense for RefUpdate.Result to have more values in order to match.
In particular, this allows us to return REJECTED_MISSING_OBJECT from
RefUpdate when an object is missing.

The comment in RefUpdate#safeParse about expecting some old objects to be
missing is only applicable to the old ID, not the new ID. A missing new
ID is a bug or programmer error, and we should not update a ref to point
to one.

Fix various tests that started failing because they depended for no good
reason on setting refs to point to nonexistent objects; it's always easy
to create a real object when necessary.

It is possible that some downstream users of RefUpdate.Result might
choose to handle one of the new statuses differently, for example by
providing a more user-readable error message; that is not done in this
change.

Change-Id: I734b1c32d5404752447d9e20329471436ffe05fc

7 years agoAdd missing newlines at ends of Java files 72/100672/3
David Pursehouse [Wed, 5 Jul 2017 01:42:26 +0000 (10:42 +0900)]
Add missing newlines at ends of Java files

Change-Id: Iead36f53d57ead0eb3edd3f9efb63b6630c9c20c
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
7 years agoTemporarily @Ignore flaky CommitCommandTest methods 40/100940/6
Dave Borowitz [Fri, 7 Jul 2017 18:43:57 +0000 (14:43 -0400)]
Temporarily @Ignore flaky CommitCommandTest methods

Change-Id: Ia2c42d014323bd29b85bf76f1a20c83f612406d7

7 years agoFix matching ignores and attributes pattern of form a/b/**. 50/100550/8
Dmitry Pavlenko [Mon, 3 Jul 2017 12:08:15 +0000 (14:08 +0200)]
Fix matching ignores and attributes pattern of form a/b/**.

Fix patch matching for patterns of form a/b/** : this should not match
paths like a/b but still match a/b/ and a/b/c.

Change-Id: Iacbf496a43f01312e7d9052f29c3f9c33807c85d
Signed-off-by: Dmitry Pavlenko <pavlenko@tmatesoft.com>
Signed-off-by: Andrey Loskutov <loskutov@gmx.de>
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
7 years agoMerge changes from topic 'packed-batch-ref-update'
David Pursehouse [Mon, 24 Jul 2017 07:38:42 +0000 (03:38 -0400)]
Merge changes from topic 'packed-batch-ref-update'

* changes:
  Add tests for updating single refs to missing objects
  Fix deleting symrefs
  RefDirectory: Throw exception if CAS of packed ref list fails
  ReceiveCommand: Explicitly check constructor preconditions
  BatchRefUpdate: Document when getPushOptions is null

7 years agoMerge "Make 'inCoreLimit' of LocalFile used in ResolveMerger configurable"
Shawn Pearce [Sat, 22 Jul 2017 22:45:56 +0000 (18:45 -0400)]
Merge "Make 'inCoreLimit' of LocalFile used in ResolveMerger configurable"

7 years agoMake 'inCoreLimit' of LocalFile used in ResolveMerger configurable 14/101714/3
Changcheng Xiao [Fri, 21 Jul 2017 07:58:37 +0000 (09:58 +0200)]
Make 'inCoreLimit' of LocalFile used in ResolveMerger configurable

This change makes it possible to configure the 'inCoreLimit' of LocalFile
used in ResolveMerger#insertMergeResult. Since LocalFile itself has some
risks, e.g. it may be left behind as garbage in case of failure. It should
be good to be able to control the size limit for using LocalFile.

Change-Id: I3dc545ade370b2bbdb7c610ed45d5dd4d39b9e8e
Signed-off-by: Changcheng Xiao <xchangcheng@google.com>
7 years agodfs: optionally store blockSize in DfsPackDescription 42/101742/2
Shawn Pearce [Fri, 21 Jul 2017 14:54:00 +0000 (07:54 -0700)]
dfs: optionally store blockSize in DfsPackDescription

Allow a DFS implementation to report blockSize to DfsPackFile,
bypassing alignment errors and corrections in the DfsBlockCache when
the blockSize of a specific file differs from the cache's configured
blockSize.

Change-Id: Ic376314d4a86a0bd528c033e169d93eef035b233

7 years agodfs: silence resource warnings in DfsBlockCacheTest 03/101603/1
Shawn Pearce [Wed, 19 Jul 2017 21:59:50 +0000 (14:59 -0700)]
dfs: silence resource warnings in DfsBlockCacheTest

Change-Id: Ia934d8578592dc20837944d50acfb8920e260893

7 years agodfs: Fix DataFormatException: 0 bytes to inflate 00/101600/2
Shawn Pearce [Wed, 19 Jul 2017 21:20:23 +0000 (14:20 -0700)]
dfs: Fix DataFormatException: 0 bytes to inflate

When a file uses a different block size (e.g.  500) than the cache
(e.g.  512), and the DfsPackFile's blockSize field has not been
initialized, the cache misaligns block loads.  The cache uses its
default of 512 to compute the block alignment instead of the file's
500.

This causes DfsReader try to set an empty range into an Inflater,
resulting in an object being unable to load.

Change-Id: I7d6352708225f62ef2f216d1ddcbaa64be113df6

7 years agodfs: actually allow current DfsBlock to GC 99/101599/1
Shawn Pearce [Wed, 19 Jul 2017 20:56:06 +0000 (13:56 -0700)]
dfs: actually allow current DfsBlock to GC

Holding the current DfsBlock in a local variable 'b' may prevent the
Java GC from reclaiming it while loading the next block.  Remove the
local variable and rely only on the field.

Change-Id: Ibfc8394cac717b485fdc94d5c8479c3f8ca78ee4

7 years agodfs: test for repositories sharing blocks in DfsBlockCache 98/101598/1
Shawn Pearce [Wed, 19 Jul 2017 20:47:18 +0000 (13:47 -0700)]
dfs: test for repositories sharing blocks in DfsBlockCache

Simple test to verify two DfsRepository instances will reuse the same
DfsBlocks in the DfsBlockCache, even though the DfsStreamKey instance
is now different between their DfsPackFile instances.

Change-Id: I409c109142dea488d189b9ac0d3c319755dce7b4

7 years agoMerge "dfs: only create DfsPackFile if description has PACK"
Shawn Pearce [Wed, 19 Jul 2017 18:49:37 +0000 (14:49 -0400)]
Merge "dfs: only create DfsPackFile if description has PACK"

7 years agodfs: Fix incorrect use of reference == for DfsStreamKey 63/101563/1
Shawn Pearce [Wed, 19 Jul 2017 17:04:09 +0000 (10:04 -0700)]
dfs: Fix incorrect use of reference == for DfsStreamKey

Must use .equals() now with DfsStreamKey.

Change-Id: I35fecbe3895c2078d69213e9c708a9b0613a1c7c

7 years agodfs: Fix build break caused by DfsStreamKey.of signature change 60/101560/1
Shawn Pearce [Wed, 19 Jul 2017 16:32:00 +0000 (09:32 -0700)]
dfs: Fix build break caused by DfsStreamKey.of signature change

Change-Id: I6c49cf42a04dd0d96cfe0751f500a51f56f0bdb8

7 years agodfs: only create DfsPackFile if description has PACK 75/101475/2
Shawn Pearce [Wed, 19 Jul 2017 02:30:07 +0000 (19:30 -0700)]
dfs: only create DfsPackFile if description has PACK

In the future with reftable a DFS implementation may choose to create
a PackDescription that contains only a REFTABLE extension.  Filter
these out by only creating a DfsPackFile if the PackDescription as the
expected PackExt.PACK.

Change-Id: I4c831622378156ae6b68f82c1ee1db5e150893be

7 years agodfs: Fix default DfsStreamKey to include DfsRepositoryDescription 35/101535/1
Shawn Pearce [Wed, 19 Jul 2017 12:53:30 +0000 (05:53 -0700)]
dfs: Fix default DfsStreamKey to include DfsRepositoryDescription

Not all DFS implementations use globally unique pack names in the
DfsPackDescription.  Most require the DfsRepositoryDescription to
qualify the pack.  Include DfsRepositoryDescription in the default
DfsStreamKey implementation, to prevent cache collisions.

Change-Id: I9ebf0c76bf2b414a702ae050b32e42588067bc44

7 years agodfs: Shrink DfsPackDescription.sizeMap storage 34/101534/1
Shawn Pearce [Wed, 19 Jul 2017 12:41:31 +0000 (05:41 -0700)]
dfs: Shrink DfsPackDescription.sizeMap storage

Using a HashMap is overkill for this storage.  PackExt is a
constrained type that permits no more than 32 unique values in the JVM.
Each is assigned a unique index (getPosition), which can be used as
indexes in a simple long[].

Change-Id: Ib8e3b2db15d3fde28989b6f4b9897f8a7bb36f3b

7 years agodfs: Fix caching of index, bitmap index, reverse index 78/101478/1
Shawn Pearce [Wed, 19 Jul 2017 04:58:30 +0000 (21:58 -0700)]
dfs: Fix caching of index, bitmap index, reverse index

When 07f98a8b71 ("Derive DfsStreamKey from DfsPackDescription")
stopped caching DfsPackFile in the DfsBlockCache, the DfsPackFile began
to always load the idx, bitmap, or compute reverse index, as the cache
handles were no longer populated by prior requests.

Rework caching to lookup the objects from the DfsBlockCache if the
local DfsPackFile handle is invalid.  This allows the DfsPackFile to
be more of a flyweight instance across requests.

Change-Id: Ic7b42ce2d90692cccea36deb30c2c76ccc81638b

7 years agodfs: Use special ForReverseIndex DfsStreamKey wrapper instead of derive 77/101477/1
Shawn Pearce [Wed, 19 Jul 2017 04:37:51 +0000 (21:37 -0700)]
dfs: Use special ForReverseIndex DfsStreamKey wrapper instead of derive

While implementing a custom subclass of DfsStreamKey it became obvious
the required derive(String) was making it impossible to construct an
efficient key in all cases.

Instead, use a special wrapper type ForReverseIndex around the INDEX's
own DfsStreamKey to denote the reverse index stream in the
DfsBlockCache.  This adds a smaller layer of boxing, but eliminates
weird issues for DFS implementors using specialized DfsStreamKey
implementations for space efficiency reasons.

Now that DfsStreamKey is reasonably light-weight, avoid allocating the
index and reverse index keys until necessary.  DfsPackFile mostly
holds the DfsBlockCache.Ref handle to the object, and only needs the
DfsStreamKey when its looking up the handle.

Change-Id: Icea78e8f7f1514087b94ef5f525d9573ea2913f2

7 years agoDerive DfsStreamKey from DfsPackDescription 73/101373/5
Shawn Pearce [Mon, 17 Jul 2017 17:24:09 +0000 (10:24 -0700)]
Derive DfsStreamKey from DfsPackDescription

By making this a deterministic function, DfsBlockCache can stop
retaining a map of every DfsPackDescription it has ever seen.  This
fixes a long standing memory leak in DfsBlockCache.

This refactoring also simplifies the idea of setting up more
lightweight objects around streams.

Change-Id: I051e7b96f5454c6b0a0e652d8f4a69c0bed7f6f4

7 years agoAdd tests for updating single refs to missing objects 25/100925/5
Dave Borowitz [Fri, 7 Jul 2017 14:31:01 +0000 (10:31 -0400)]
Add tests for updating single refs to missing objects

The reader may find it surprising that this succeeds without incident
unless there is peeling or a fast-forward check involved. This behavior
may be changed in the future, but for now, just document the current
behavior.

Change-Id: I348b37e93e0264dc0905c4d58ce881852d1dfe5e

7 years agoFix deleting symrefs 31/100931/4
Dave Borowitz [Fri, 7 Jul 2017 15:24:51 +0000 (11:24 -0400)]
Fix deleting symrefs

The RefDirectory implementation of doDelete never considered whether to
delete a symref or its leaf, because the detachingSymbolicRef bit was
never exposed from RefUpdate. The behavior was thus incorrectly to
always delete the symref, never the leaf.

There was no test for this behavior. The only thing that attempted to be
a test was testDeleteHeadInBareRepo, but this test was broken for
reasons unrelated to this bug. Specifically, it set the leaf to point to
a completely nonexistent object, and then asserted that deleting HEAD
resulted in NO_CHANGE. The only reason this test ever passed is because
of a quirk of updateImpl, which treats a missing object as the same as
null. This quirk aside, the test wasn't really testing the right thing.
Turn this into a real test by writing out a real object and pointing the
leaf at that.

Also, add a test for the detachingSymbolicRef case, i.e. deleting the
symref and leaving the leaf alone.

Change-Id: Ib96d2a35b4f99eba0734725486085fc6f9d78aa5

7 years agoRefDirectory: Throw exception if CAS of packed ref list fails 70/100770/7
Dave Borowitz [Wed, 5 Jul 2017 18:19:36 +0000 (14:19 -0400)]
RefDirectory: Throw exception if CAS of packed ref list fails

The contents of the packedRefList AtomicReference should never differ
from what we expect prior to writing, because this segment of the code
is protected by the packed-refs lock file on disk. If it does happen,
whether due to programmer error or a rogue process not respecting the
locking protocol, it's better to let the caller know than to silently
drop the whole commit operation on the floor.

The existing concurrentOnlyOneWritesPackedRefs test is inherently
nondeterministic as written, and was already about 6% flaky as measured
by bazel:

  $ bazel test --runs_per_test=200 //org.eclipse.jgit.test:org_eclipse_jgit_internal_storage_file_GcPackRefsTest
  ...
  INFO: Elapsed time: 42.608s, Critical Path: 10.35s
  //org.eclipse.jgit.test:org_eclipse_jgit_internal_storage_file_GcPackRefsTest FAILED in 12 out of 200 in 1.6s
    Stats over 200 runs: max = 1.6s, min = 1.1s, avg = 1.3s, dev = 0.1s

This flakiness was caused by the assumption that exactly one of the 2
threads would fail, when both might actually succeed in practice due to
racing on the compare-and-swap.

For whatever reason, this change affected the interleaving behavior in
such a way that the flakiness jumped to around 50%. Making the
interleaving of the test fully deterministic is beyond the scope of this
change, but a simple tweak to the assertion is enough to make it pass
consistently 200+ times both before and after this change.

Change-Id: I5ff4dc39ee05bda88d47909acb70118f3d0c8f74

7 years agoReceiveCommand: Explicitly check constructor preconditions 69/100769/6
Dave Borowitz [Wed, 5 Jul 2017 18:07:17 +0000 (14:07 -0400)]
ReceiveCommand: Explicitly check constructor preconditions

Some downstream code checks whether a ReceiveCommand is a create or a
delete based on the type field. Other downstream code (in particular a
good chunk of Gerrit code I wrote) checks the same thing by comparing
oldId/newId to zeroId. Unfortunately, there were no strict checks in the
constructor that ensures that zeroId is only set for oldId/newId if the
type argument corresponds, so a caller that passed mismatched IDs and
types would observe completely undefined behavior as a result. This is
and always has been a misuse of the API; throw IllegalArgumentException
so the caller knows that it is a misuse.

Similarly, throw from the constructor if oldId/newId are null. The
non-nullness requirement was already documented. Fix RefDirectoryTest to
not do the wrong thing.

Change-Id: Ie2d0bfed8a2d89e807a41925d548f0f0ce243ecf

7 years agoBatchRefUpdate: Document when getPushOptions is null 68/100768/4
Dave Borowitz [Wed, 5 Jul 2017 17:45:25 +0000 (13:45 -0400)]
BatchRefUpdate: Document when getPushOptions is null

Change-Id: I4cccda0ec3a8598edb723dc49101a16d603d1e82