aboutsummaryrefslogtreecommitdiffstats
Commit message (Collapse)AuthorAgeFilesLines
* Move BatchRefUpdate tests to a new fileDave Borowitz2017-07-253-493/+480
| | | | | | | | | | 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
* Implement atomic BatchRefUpdates for RefDirectoryDave Borowitz2017-07-256-41/+815
| | | | | | | | | | | | | | 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
* Separate RefUpdate.Result.REJECTED_{MISSING_OBJECT,OTHER_REASON}Dave Borowitz2017-07-257-98/+187
| | | | | | | | | | | | | | | | | | | | | | | 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
* Add missing newlines at ends of Java filesDavid Pursehouse2017-07-2545-45/+45
| | | | | Change-Id: Iead36f53d57ead0eb3edd3f9efb63b6630c9c20c Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
* Temporarily @Ignore flaky CommitCommandTest methodsDave Borowitz2017-07-251-0/+3
| | | | Change-Id: Ia2c42d014323bd29b85bf76f1a20c83f612406d7
* Fix matching ignores and attributes pattern of form a/b/**.Dmitry Pavlenko2017-07-243-12/+39
| | | | | | | | | | 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>
* Merge changes from topic 'packed-batch-ref-update'David Pursehouse2017-07-249-44/+269
|\ | | | | | | | | | | | | | | | | * 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
| * Add tests for updating single refs to missing objectsDave Borowitz2017-07-171-0/+88
| | | | | | | | | | | | | | | | | | 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
| * Fix deleting symrefsDave Borowitz2017-07-173-4/+78
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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
| * RefDirectory: Throw exception if CAS of packed ref list failsDave Borowitz2017-07-172-17/+31
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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
| * ReceiveCommand: Explicitly check constructor preconditionsDave Borowitz2017-07-174-22/+68
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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
| * BatchRefUpdate: Document when getPushOptions is nullDave Borowitz2017-07-171-1/+4
| | | | | | | | Change-Id: I4cccda0ec3a8598edb723dc49101a16d603d1e82
* | Merge "Make 'inCoreLimit' of LocalFile used in ResolveMerger configurable"Shawn Pearce2017-07-222-2/+23
|\ \
| * | Make 'inCoreLimit' of LocalFile used in ResolveMerger configurableChangcheng Xiao2017-07-222-2/+23
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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>
* | | dfs: optionally store blockSize in DfsPackDescriptionShawn Pearce2017-07-216-19/+59
|/ / | | | | | | | | | | | | | | | | 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
* | dfs: silence resource warnings in DfsBlockCacheTestShawn Pearce2017-07-191-0/+2
| | | | | | | | Change-Id: Ia934d8578592dc20837944d50acfb8920e260893
* | dfs: Fix DataFormatException: 0 bytes to inflateShawn Pearce2017-07-193-14/+63
| | | | | | | | | | | | | | | | | | | | | | | | | | 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
* | dfs: actually allow current DfsBlock to GCShawn Pearce2017-07-191-2/+1
| | | | | | | | | | | | | | | | 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
* | dfs: test for repositories sharing blocks in DfsBlockCacheShawn Pearce2017-07-191-0/+103
| | | | | | | | | | | | | | | | 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
* | Merge "dfs: only create DfsPackFile if description has PACK"Shawn Pearce2017-07-191-3/+2
|\ \
| * | dfs: only create DfsPackFile if description has PACKShawn Pearce2017-07-191-3/+2
| | | | | | | | | | | | | | | | | | | | | | | | | | | 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
* | | dfs: Fix incorrect use of reference == for DfsStreamKeyShawn Pearce2017-07-191-1/+1
| | | | | | | | | | | | | | | | | | Must use .equals() now with DfsStreamKey. Change-Id: I35fecbe3895c2078d69213e9c708a9b0613a1c7c
* | | dfs: Fix build break caused by DfsStreamKey.of signature changeShawn Pearce2017-07-192-2/+6
| | | | | | | | | | | | Change-Id: I6c49cf42a04dd0d96cfe0751f500a51f56f0bdb8
* | | dfs: Fix default DfsStreamKey to include DfsRepositoryDescriptionShawn Pearce2017-07-192-16/+12
| | | | | | | | | | | | | | | | | | | | | | | | | | | 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
* | | dfs: Shrink DfsPackDescription.sizeMap storageShawn Pearce2017-07-191-17/+10
|/ / | | | | | | | | | | | | | | | | 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
* | dfs: Fix caching of index, bitmap index, reverse indexShawn Pearce2017-07-182-30/+59
| | | | | | | | | | | | | | | | | | | | | | | | | | 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
* | dfs: Use special ForReverseIndex DfsStreamKey wrapper instead of deriveShawn Pearce2017-07-182-30/+23
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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
* | Derive DfsStreamKey from DfsPackDescriptionShawn Pearce2017-07-1710-119/+132
| | | | | | | | | | | | | | | | | | | | | | 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
* | Extract BlockBasedFile base class for DfsPackFileShawn Pearce2017-07-174-174/+234
| | | | | | | | | | | | | | | | | | | | | | This new base class has the minimum set of properties and methods necessary for DfsBlockCache to manage blocks of a file in the cache. Subclasses can use DfsBlockCache for any content. This refactoring opens the door for additional PackExt types other than PACK to be stored on a block-by-block basis by the DfsBlockCache. Change-Id: I307228fc805c3ff0c596783beb24fd52bec35ba8
* | Use separate DfsStreamKey for PackIndexShawn Pearce2017-07-171-20/+6
| | | | | | | | | | | | | | | | Instead of overloading the pack's DfsStreamKey with negative positions for the idx, reverse idx and bitmap, assign a unique DfsStreamKey for each of these related streams. Change-Id: Ie048036c74a1d1bbf5ea7e888452dc0c1adf992f
* | Rename DfsPackKey to DfsStreamKeyShawn Pearce2017-07-178-39/+36
|/ | | | | | | This renaming supports reusing DfsStreamKey in a future commit to index other PackExt type streams inside of the DfsBlockCache. Change-Id: Ib52d374e47724ccb837f4fbab1fc85c486c5b408
* Add missing @since 4.9 for new API PackParser.setExpectedObjectCount()Matthias Sohn2017-07-081-0/+1
| | | | Change-Id: I58fa956aea37c696dbc35ecd229d8971d532923f Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
* Merge changes from topic 'packed-batch-ref-update'Dave Borowitz2017-07-073-22/+36
|\ | | | | | | | | | | | | * changes: RefList: Support capacity <= 0 on new builders Short-circuit writing packed-refs if no refs were packed BatchRefUpdate: Clarify some ref prefix calls
| * RefList: Support capacity <= 0 on new buildersDave Borowitz2017-07-051-2/+3
| | | | | | | | | | | | | | | | Callers may estimate the size, and their estimate may be zero. Silently allow this, rather than throwing IndexOutOfBoundsException later during add. Change-Id: Ife236f9f4ce469c57b18e76cf4fad6feb52cb2b0
| * Short-circuit writing packed-refs if no refs were packedDave Borowitz2017-07-051-6/+22
| | | | | | | | Change-Id: Id691905599b242e48f590138a96e0c86132308fd
| * BatchRefUpdate: Clarify some ref prefix callsDave Borowitz2017-07-051-14/+11
| | | | | | | | | | | | | | | | | | Inline the old addRefToPrefixes, since it was just a glorified addAll. Split getPrefixes into a variant, addPrefixesTo, that doesn't allocate a small Collection on every invocation. Use this in the tight loop of getTakenPrefixes. Change-Id: I25cc7feef0c8e312820d85b7ed48559da49b83d2
* | Make possible to overwrite the object countZhen Chen2017-07-051-11/+27
|/ | | | | | | | | | | | | | | Right now, PackParser relies on the object count from the pack header. However, when creating Dfs INSERT packs, the object count is not known at the beginning of the operation. And when we append the base to a RECEIVE pack, we can't modify the pack header for object count in most Dfs implementations. Make it possible to tell PackParser the expected object count by adding a setter for expectedObjectCount, implementation can overwrite the object count in onPackHeader function. Change-Id: I646ca33ab2b843de84edc287abfb65803a56a927 Signed-off-by: Zhen Chen <czhen@google.com>
* Merge "Support -merge attribute in binary macro"Christian Halstrick2017-07-037-10/+651
|\
| * Support -merge attribute in binary macroMathieu Cartaud2017-06-277-10/+651
| | | | | | | | | | | | | | | | | | | | | | | | | | | | The merger is now able to react to the use of the merge attribute. The value unset and the custom value 'binary' are handled (-merge and merge=binary) Since the specification of the merge attribute states that when the attribute is unset, ours version must be kept in case of a conflict, we don't overwrite the file but keep the local version. Bug: 517128 Change-Id: Ib5fbf17bdaf727bc5d0e106ce88f2620d9f87a6f Signed-off-by: Mathieu Cartaud <mathieu.cartaud@obeo.fr>
* | Use read ahead during copyPackThroughCacheShawn Pearce2017-06-272-18/+47
| | | | | | | | | | | | | | | | | | | | If a block is missing from the block cache, open the pack stream, retain the ReadableChannel, and turn on read-ahead. This should help to load a medium sized pack into a cold cache more quickly from a slower IO stream, as the pack is scanned sequentially and missing blocks are more likely to be available through the read-ahead. Change-Id: I3300d936b9299be6d9eb642992df7c04bb439cde
* | Add a test for parsing fsck config options and expose FsckMode enumDavid Turner2017-06-232-2/+23
|/ | | | | | | | | | | | | These config options allow overriding the message type (error, warn or ignore) of a specific message ID such as missingEmail. The supported fsck message IDs are defined in ObjectChecker.ErrorType. Since TransferConfig.FsckMode wasn't public parsing fsck configuration options like e.g. fsck.missingEmail=ignore failed with an IllegalAccessException. Fix this by declaring this enum public. Change-Id: I3f41ff7a76a846250a63ce92a9fd111eb347269f Signed-off-by: David Turner <dturner@twosigma.com> Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
* Add --match option for `jgit describe` to CLIOliver Lockwood2017-06-224-3/+59
| | | | | | | | | This adds --match option for glob(7) matchers on git tags to jgit describe in CLI. Bug: 518377 Change-Id: I745988d565dd4391e8b3e5a91bbfbae575333819 Signed-off-by: Oliver Lockwood <oliver.lockwood@cantab.net>
* Fix bug in multiple tag handling on DescribeCommandOliver Lockwood2017-06-212-23/+61
| | | | | | | | | | | In the case of multiple tags on the same commit, jgit previously only ever looked at the last of those tags; git behaviour is to return the first tag (or first matching one if --match is specified). Bug: 518377 Change-Id: I3b6b58ad9f8aa3879ae35b84542b7bddc74a27d6 Signed-off-by: Oliver Lockwood <oliver.lockwood@cantab.net>
* Support --match functionality in DescribeCommandOliver Lockwood2017-06-202-12/+71
| | | | | | | | | A `match()` method has been added to the DescribeCommand, allowing users to specify one or more `glob(7)` matchers as per Git convention. Bug: 518377 Change-Id: Ib4cf34ce58128eed0334adf6c4a052dbea62c601 Signed-off-by: Oliver Lockwood <oliver.lockwood@cantab.net> Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
* Allow to programmatically set FastForwardMode for PullCommandMatthias Sohn2017-06-161-5/+29
| | | | | Bug: 517847 Change-Id: I70d12dbe347a3d7a3528687ee04e52a2052bfb93 Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
* Add support for config "pull.ffMattias Neuling2017-06-162-0/+17
| | | | | | | | | When the configuration entry 'pull.ff' exists the merge of the pull will use the value as fast forward option. Bug: 474174 Change-Id: Ic8db2f00095ed81528667b064ff523911e6c122e Signed-off-by: Mattias Neuling <neuling@dakosy.de> Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
* Fetch/PullCommand: Improve Javadoc of setRecurseSubmodulesDavid Pursehouse2017-06-162-2/+13
| | | | | | | | Annotate the `recurse` parameter as @Nullable and expand the Javadoc to clarify the precedence of options. Change-Id: I7aee800cdbf8243133a0d353ef79b97b67ce011e Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
* Improve javadoc for MergeCommand.setFastForward()Matthias Sohn2017-06-151-3/+7
| | | | | | | | - mark parameter to be nullable - explain that we fallback to value of merge.ff if set to null and to --ff if also not configured there Change-Id: Id077763b95195d21543ac637f9939a6d4179e982 Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
* Add tests for SubmoduleConfigDavid Pursehouse2017-06-151-0/+91
| | | | | Change-Id: Idcc93c2ca95938995d489cffda649c7d7b26c50e Signed-off-by: David Pursehouse <david.pursehouse@gmail.com> Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
* Merge branch 'stable-4.8'Matthias Sohn2017-06-150-0/+0
|\ | | | | | | | | | | | | | | | | * stable-4.8: Prepare 4.8.1-SNAPSHOT builds JGit v4.8.0.201706111038-r Change-Id: I32024b36093eb095539e02b1788d074bc5237d9f Signed-off-by: David Pursehouse <david.pursehouse@gmail.com> Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>