Commit Graph

1739 Commits

Author SHA1 Message Date
Matthias Sohn aec22e74cf Prepare 4.8.0-SNAPSHOT builds
Change-Id: Ifea6750e79d417a8a2a891b3b5f96d68c7200011
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
2017-04-05 14:49:49 +02:00
Shawn Pearce db2493e7d8 Merge "Make diff locations more consistent" 2017-04-04 22:26:38 -04:00
David Pursehouse cab9dee333 Merge branch 'stable-4.7'
* stable-4.7:
  PushConnectionTest#limitCommandBytes: Increase maxCommandBytes

Change-Id: I354f46475ee96eb3b31abd303b5df2f3023fcf75
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
2017-04-05 07:58:34 +09:00
Dave Borowitz edb6593ef0 ResolveMergerTest: Add tests for content merges
Change-Id: I2b85cc239e9b3090426c956cbf9af3deb4106a41
2017-04-04 17:27:15 -04:00
David Pursehouse 61a943e03d PushConnectionTest#limitCommandBytes: Increase maxCommandBytes
A higher limit is required to account for proper JGit version number
being sent in the UserAgent.

The version string "4.7.0.201704031717-r" is 20 characters, however
the strings used during development are shorter:

- When running from mvn, "4.7.0.qualifier" is used; 15 characters
- When running in Eclipse, "unknown" is used; 7 characters

Change-Id: I9aca2f71389a42fedce305e9078db016869c3d1a
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
2017-04-04 21:37:48 +09:00
KB Sriram 4a985f5aa8 Make diff locations more consistent
DiffAlgorithms can return different edit locations for inserts or
deletes, if they can be "shifted" up or down repeating blocks of
lines. This causes the 3-way merge to apply both edits, resulting in
incorrectly removing or duplicating lines.

Augment an existing "tidy-up" stage in DiffAlgorithm to move all
shiftable edits (not just the last INSERT edit) to a consistent
location, and add test cases for previously incorrect merges.

Bug: 514095
Change-Id: I5fe150a2fc04e1cdb012d22609d86df16dfb0b7e
Signed-off-by: KB Sriram <kbsriram@google.com>
2017-04-03 16:45:13 -07:00
David Pursehouse 5f902f07cc PullCommand: Add support for recursing into submodules
Add a new API method to set the recurse mode, and pass the mode into
the fetch command.

Extend the existing FetchCommandRecurseSubmodulesTest to also perform
the same tests for fetch. Rename the test class accordingly.

Change-Id: I12553af47774b4778f7011e1018bd575a7909bd0
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
2017-03-24 00:02:45 +01:00
David Ostrovsky fa2851ce78 bazel: Consume hamcrest through transitive dependency
In I3ab958ce8 explicit dependency in lib/BUILD were defined and most
of the bazel build implementation was switched to using it. Switch
test.bzl test implementation to using explicit dependencies as well.

Change-Id: I4413d1a45addeeb2a980d07669fa034c2eebb3a4
Signed-off-by: David Ostrovsky <david@ostrovsky.org>
2017-03-23 06:47:49 +01:00
David Pursehouse 2d0ce094b4 Remove Buck build
Buck will be replaced with Bazel

Change-Id: I3cf07d7aaaa2a58bac34e16c50af5416693254ac
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
2017-03-22 01:41:21 +01:00
David Ostrovsky 0f6ddb372b Add remaining parts of the bazel build
Add bazel build for ui and junit.http, and the test packages.

A number of different test labels are supported:

  api
  attributes
  dfs
  diff
  http
  lfs
  lfs-server
  nls
  notes
  pack
  patch
  pgm
  reftree
  revplot
  revwalk
  storage
  submodule
  symlinks
  transport
  treewalk
  util

To run all tests:

  bazel test //...

To run specific tests, using labels:

  bazel test --test_tag_filters=api,dfs,revplot,treewalk //...

Change-Id: Ic41b05a79d855212e67b1b4707e9c6b4dc9ea70d
Signed-off-by: David Ostrovsky <david@ostrovsky.org>
Signed-off-by: Jonathan Nieder <jrn@google.com>
2017-03-21 09:24:12 +01:00
David Ostrovsky cee9d444e9 Delegate crypto algorithm creation to InsecureCipherFactory
This is a preparation change to Bazel build implementation. Error
Prone rejects the code with variable crypto algorithm as insecure
see: [1].

[1] http://errorprone.info/bugpattern/InsecureCryptoUsage

Change-Id: I92db70a7da454bc364597a995e8be5dccc2d6427
Signed-off-by: David Ostrovsky <david@ostrovsky.org>
2017-03-21 09:23:49 +01:00
David Pursehouse 5e5fbb6035 Fix test configuration to run RacyGitTests, and fix testRacyGitDetection
With the filename suffix "Tests", the module was not included in tests
when building with Maven, and without the @Test annotations the tests
didn't get executed under Eclipse or buck test.

testRacyGitDetection was failing because the index file did not exist.

Add the missing configuration, the missing annotations, and add a call
to reset() in testRacyGitDetection to force creation of the index file.

Change-Id: I29dd8f89c36fef4ab40bedce7f4a26bd9b2390e4
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
2017-03-19 10:56:06 +09:00
David Ostrovsky 9b4f1913fb RevFlagSetTest: Fix compilation error flagged by error prone
This fixes error flagged by error prone:

Java compilation in rule '//org.eclipse.jgit.test:jgit' failed: Worker
process sent response with exit code: 1.
org.eclipse.jgit.test/tst/org/eclipse/jgit/revwalk/RevFlagSetTest.java:149:
error: [CollectionIncompatibleType] Argument '"bob"' should not be
passed to this method; its type String is not compatible with its
collection's type argument RevFlag
		assertFalse(set.contains("bob"));

Change-Id: I4a971ce92fee55e28b2ab0c7b716ac20fa9c6709
Signed-off-by: David Ostrovsky <david@ostrovsky.org>
2017-03-18 16:47:16 +01:00
David Pursehouse 2fe1a3abbe FetchCommand: Fix detection of submodule recursion mode
The submodule.name.fetchRecurseSubmodules value was being read from the
configuration of the submodule, but it should be read from the config
of the parent repository.

Also, the fetch.recurseSubmodules value from the parent repository's
configuration was not being considered at all.

Fix both of these and add tests. Now the precedence of the recurse mode
is determined as follows:

 1. Value passed to the API
 2. Value configured in submodule.name.fetchRecurseSubmodules
 3. Value configured in fetch.recurseSubmodules
 4. Default to "on demand"

Change-Id: Ic23b7c40b5f39135fb3fd754c597dd4bcc94240c
2017-03-10 13:17:39 +09:00
David Pursehouse 503d59044f FetchCommand: Add basic support for recursing into submodules
Extend FetchCommand to expose a new method, setRecurseSubmodules(mode),
which allows to set the mode to ON, OFF or ON_DEMAND.

After fetching a repository, its submodules are recursively fetched:

- When the mode is YES, submodules are always fetched.

- When the mode is NO, submodules are not fetched.

- When the mode is ON_DEMAND, submodules are only fetched when the
  parent repository receives an update of the submodule and the new
  revision is not already in the submodule.

The mode is determined in the following order of precedence:

- Value specified in the API call using setRecurseSubmodules.

- Value specified in the repository's config under the key
  submodule.name.fetchRecurseSubmodules

- Defaults to ON_DEMAND if neither of the previous is set.

Extend FetchResult to recursively include results for submodules, as
a map of the submodule path to an instance of FetchResult.

Test setup is based on testCloneRepositoryWithNestedSubmodules.

Change-Id: Ibc841683763307cb76e78e142e0da5b11b1add2a
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
2017-03-04 09:31:16 +09:00
Thomas Wolf 1f3e74ed9f Make Repository.normalizeBranchName less strict
This operation was added recently with the goal to provide some
way to auto-correct invalid user input, or to provide a correction
suggestion to the user -- EGit uses it now that way. But the initial
implementation was very restrictive; it removed all non-ASCII
characters and even slashes.

Understandably end users were not happy with that. Git has no such
restriction to ASCII-only; nor does JGit. Branch names should be
meaningful to the end user, and if a user-supplied branch name is
invalid for technical reasons, a "normalized" name should still
be meaningful to the user.

Rewrite to attempt a minimal fix such that the result will pass
isValidRefName.

* Replace all Unicode whitespace by underscore.
* Replace troublesome special characters by dash.
* Collapse sequences of underscores, dots, and dashes.
* Remove underscores, dots, and dashes following slashes, and
  collapse sequences of slashes.
* Strip leading and trailing sequences of slashes, dots, dashes,
  and underscores.
* Avoid the ".lock" extension.
* Avoid the Windows reserved device names.
* If input name is null return an empty String so callers don't need to
check for null.

This still allows branch names with single slashes as separators
between components, avoids some pitfalls that isValidRefName() tests
for, and leaves other character untouched and thus allows non-ASCII
branch names.

Also move the function from the bottom of the file up to where
isValidRefName is implemented.

Bug: 512508
Change-Id: Ia0576d9b2489162208c05e51c6d54e9f0c88c3a7
Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
2017-03-04 00:23:42 +01:00
Jonathan Nieder 45f62576de Merge "SHA-1: collision detection support" 2017-03-02 13:26:45 -05:00
Shawn Pearce 83ad74b6b9 SHA-1: collision detection support
Update SHA1 class to include a Java port of sha1dc[1]'s ubc_check,
which can detect the attack pattern used by the SHAttered[2] authors.

Given the shattered example files that have the same SHA-1, this
modified implementation can identify there is risk of collision given
only one file in the pair:

  $ jgit ...
  [main] WARN org.eclipse.jgit.util.sha1.SHA1 - SHA-1 collision 38762cf7f55934b34d179ae6a4c80cadccbb7f0a

When JGit detects probability of a collision the SHA1 class now warns
on the logger, reporting the object's SHA-1 hash, and then throws a
Sha1CollisionException to the caller.

From the paper[3] by Marc Stevens, the probability of a false positive
identification of a collision is about 14 * 2^(-160), sufficiently low
enough for any detected collision to likely be a real collision.

git-core[4] may adopt sha1dc before the system migrates to an entirely
new hash function.  This commit enables JGit to remain compatible with
that move to sha1dc, and help protect users by warning if similar
attacks as SHAttered are identified.

Performance declined about 8% (detection off), now:

  MessageDigest        238.41 MiB/s
  MessageDigest        244.52 MiB/s
  MessageDigest        244.06 MiB/s
  MessageDigest        242.58 MiB/s

  SHA1                 216.77 MiB/s (was ~240.83 MiB/s)
  SHA1                 220.98 MiB/s
  SHA1                 221.76 MiB/s
  SHA1                 221.34 MiB/s

This decline in throughput is attributed to the step loop unrolling in
compress(), which was necessary to easily fit the UbcCheck logic into
the hash function.  Using helper functions s1-s4 reduces the code
explosion, providing acceptable throughput.

With detection enabled (default):

  SHA1 detectCollision 180.12 MiB/s
  SHA1 detectCollision 181.59 MiB/s
  SHA1 detectCollision 181.64 MiB/s
  SHA1 detectCollision 182.24 MiB/s

  sha1dc (native C)   ~206.28 MiB/s
  sha1dc (native C)   ~204.47 MiB/s
  sha1dc (native C)   ~203.74 MiB/s

Average time across 100,000 calls to hash 4100 bytes (such as a commit
or tree) for the various algorithms available to JGit also shows SHA1
is slower than MessageDigest, but by an acceptable margin:

  MessageDigest        17 usec
  SHA1                 18 usec
  SHA1 detectCollision 22 usec

Time to index-pack for git.git (217982 objects, 69 MiB) has increased:

  MessageDigest   SHA1 w/ detectCollision
  -------------   -----------------------
         20.12s   25.25s
         19.87s   25.48s
         20.04s   25.26s

    avg  20.01s   25.33s    +26%

Being implemented in Java with these additional safety checks is
clearly a penalty, but throughput is still acceptable given the
increased security against object name collisions.

[1] https://github.com/cr-marcstevens/sha1collisiondetection
[2] https://shattered.it/
[3] https://marc-stevens.nl/research/papers/C13-S.pdf
[4] https://public-inbox.org/git/20170223230621.43anex65ndoqbgnf@sigill.intra.peff.net/

Change-Id: I9fe4c6d8fc5e5a661af72cd3246c9e67b1b9fee6
2017-02-28 16:38:43 -08:00
Magnus Vigerlöf 2a5d20c138 Correct the boolean logic for filtering paths
The TreeWalk filtering classes need to support the three different
meanings of the return value the path comparison generates.
A new path comparison method (isPathMatch) is created with
three distinct return values (isPathPrefix use value '0' to
encode two of these) which will makes it possible for the logical
operators (especially NOT) to aggregate a correct verdict.

A filter like: AND(Path("path"), NOT(Path("path/to/other")))
Should filter out 'path/to/other/file', but not 'path/to/my/file'.

The path-limiting feature when testing path/to/my/file, would
result to run test for the following paths:

    path
    path/to
    path/to/my
    path/to/my/file

isPathPrefix('path/to/other') will return '0' for the first two
and since there is no way for NOT to distinguish between an exact
match and a match indicating that the tested path is a 'parent',
it will incorrectly return false and thus remove everything below
'path' immediately.
isPathMatch has a distinguished value for 'parent' matches that
will be preserved through the logic operators and should not
cause an over-eager removal of paths.

The functionality of isPathPrefix is required by other parts
and is untouched.

Unit tests are included to ensure that the logical functionality
is correct and can be preserved.

Change-Id: Ice2ca9406f09f1b179569e99b86a0e5d77baa20d
Signed-off-by: Magnus Vigerlöf <magnus.vigerlof@gmail.com>
2017-02-28 23:56:33 +01:00
Shawn Pearce 1bf7d3f290 SHA1: support reset() and reuse instances
Allow SHA1 instances to be reused to compute another hash value, and
resume caching them in ObjectInserter and PackParser.  This shaves a
small amount of running time off parsing git.git's pack file:

  before   after
  ------   ------
  25.25s   25.55s
  25.48s   25.06s
  25.26s   24.94s

Almost noise (small difference), but recycling the instances reduces
some stress on the memory allocator finding two 80 word message block
arrays needed for hashing and collision detection.

Change-Id: I4af88a720e81460293bc5c5d1d3db1a831e7e228
2017-02-26 15:26:53 -08:00
Shawn Pearce 982f5d1bf1 Pure Java SHA-1
This implementation is derived straight from the description written
in RFC 3174.  On Mac OS X with Java 1.8.0_91 it offers similar
throughput as MessageDigest SHA-1:

  system   239.75 MiB/s
  system   244.71 MiB/s
  system   245.00 MiB/s
  system   244.92 MiB/s

  sha1     234.08 MiB/s
  sha1     244.50 MiB/s
  sha1     242.99 MiB/s
  sha1     241.73 MiB/s

This is the fastest implementation I could come up with.  Common SHA-1
implementation tricks such as unrolling loops creates a method too
large for the JIT to effectively optimize, resulting in lower overall
hashing throughput. Using a preprocessor to perform the register
renaming of A-E also didn't help, as again the method was too large
for the JIT to effectively optimize.

Fortunately the fastest version is a naive, straight-forward
implementation very close to the description in RFC 3174.

Change-Id: I228b05c4a294ca2ad51386cf0e47978c68e1aa42
2017-02-26 11:16:19 -08:00
David Pursehouse 3b4448637f Enable and fix warnings about redundant specification of type arguments
Since the introduction of generic type parameter inference in Java 7,
it's not necessary to explicitly specify the type of generic parameters.

Enable the warning in Eclipse, and fix all occurrences.

Change-Id: I9158caf1beca5e4980b6240ac401f3868520aad0
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
2017-02-20 22:47:23 +01:00
David Pursehouse fceac7e44d Add some more missing @Override annotations
Change-Id: Ic13160920b986edde87c928c473240cc9c034f50
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
2017-02-20 11:32:22 +09:00
David Pursehouse 7ac182f4e4 Enable and fix 'Should be tagged with @Override' warning
Set missingOverrideAnnotation=warning in Eclipse compiler preferences
which enables the warning:

  The method <method> of type <type> should be tagged with @Override
  since it actually overrides a superclass method

Justification for this warning is described in:

  http://stackoverflow.com/a/94411/381622

Enabling this causes in excess of 1000 warnings across the entire
code-base. They are very easy to fix automatically with Eclipse's
"Quick Fix" tool.

Fix all of them except 2 which cause compilation failure when the
project is built with mvn; add TODO comments on those for further
investigation.

Change-Id: I5772061041fd361fe93137fd8b0ad356e748a29c
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
2017-02-19 20:05:08 -04:00
Naoki Takezoe 1448ec37f9 Set commit time to ZipArchiveEntry
Archived zip files for a same commit have different MD5 hash because
mdate and mdate in the header of zip entries are not specified. In
this case, Commons Compress sets an archived time.

In the original git implementation, it's set a commit time:
e2b2d6a172/archive.c (L378)

By this fix, archive command sets the commit time to ZipArchiveEntry
when RevCommit is given as an archiving target.

Change-Id: I30dd8710e910cdf42d57742f8709e9803930a123
Signed-off-by: Naoki Takezoe <takezoe@gmail.com>
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
2017-02-18 10:47:27 +01:00
David Turner d3962fef6b GC: don't loosen doomed objects
If the pruneexpire config is set to "now", then any unreferenced loose
objects are immediately eligible for gc.  So there is no need to
actually write the loose objects.

Users who run hosting services which sometimes accept large, entirely
garbage packs might set the following configurations:

gc.pruneExpire = now
gc.prunePackExpire = 2.weeks

Then garbage objects will be kept around in packs, but after two weeks
the packs themselves will get deleted.

For client-side users of jgit, the default settings will loosen
garbage objects, and, after an hour, delete the old packs in which
they resided.

Change-Id: I8f686ac60b40181b1ee92ac6c313c3f33b55c44c
Signed-off-by: David Turner <dturner@twosigma.com>
2017-02-17 11:26:09 -05:00
Shawn Pearce 0bff481d45 Limit receive commands
Place a configurable upper bound on the amount of command data
received from clients during `git push`.  The limit is applied to the
encoded wire protocol format, not the JGit in-memory representation.
This allows clients to flexibly use the limit; shorter reference names
allow for more commands, longer reference names permit fewer commands
per batch.

Based on data gathered from many repositories at $DAY_JOB, the average
reference name is well under 200 bytes when encoded in UTF-8 (the wire
encoding).  The new 3 MiB default receive.maxCommandBytes allows about
11,155 references in a single `git push` invocation.  A Gerrit Code
Review system with six-digit change numbers could still encode 29,399
references in the 3 MiB maxCommandBytes limit.

Change-Id: I84317d396d25ab1b46820e43ae2b73943646032c
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
2017-02-11 00:20:36 +01:00
David Pursehouse 158d3722d3 IndexDiffSubmoduleTest: Fix negative use count
submoduleStandalone is created by createWorkRepository() which adds
the created repository to the set of repositories to be closed in
the test teardown. It is therefore not necessary to explicitly close
it.

Change-Id: Ib6f525b644fdeaaf1934df39cc2d3583a0d883dc
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
2017-02-10 19:48:52 +09:00
David Pursehouse 51239129b3 FetchCommandTest: Don't declare specific exceptions in test methods
Change-Id: Ie0f8a0f7a9c2c383be6ae8265353daac7f5a89fa
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
2017-02-09 15:10:15 +09:00
David Pursehouse 16dc88fe10 PushCommandTest: Remove unused variables to prevent errors in Eclipse
Change-Id: Ie656b18fb151bf1e3c2dcc0438a77e32102991c2
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
2017-02-09 15:10:15 +09:00
Shawn Pearce 8fce17a995 Merge "push: support per-ref force-with-lease" 2017-02-08 22:27:06 -05:00
David Turner 46d35a8502 push: support per-ref force-with-lease
When rebasing, force-pushing has a race condition: someone else might
have pushed a commit since the one you just rewrote. The force-with-lease
option prevents this by ensuring that the ref's old value is the one
that you expected.

Change-Id: I97ca9f8395396c76332bdd07c486e60549ca4401
Signed-off-by: David Turner <dturner@twosigma.com>
2017-02-08 19:42:33 -05:00
Shawn Pearce d67b183537 Prefer smaller GC files during DFS garbage collection
In 8ac65d33ed PackWriter changed its
behavior to always prefer the last object representation presented
to it by the ObjectReuseAsIs implementation. This was a fix to avoid
delta chain cycles.

Unfortunately it can lead to suboptimal compression when concurrent
GCs are run on the same repository. One case is automatic GC running
(with default settings) in parallel to a manual GC that has disabled
delta reuse in order to generate new smaller deltas for the entire
history of the repository.

Running GC with no-reuse generally requires more CPU time, which
also translates to a longer running time.  This can lead to a race
where the automatic GC completes before the no-reuse GC, leaving
the repository in a state such as:

  no-reuse GC:   size 1 GiB, mtime = 18:45
  auto GC:       size 8 GiB, mtime = 17:30

With the default sort ordering, the smaller no-reuse GC pack is
sorted earlier in the pack list, due to its more recent mtime.

During object reuse in a future GC, these smaller representations
are considered first by PackWriter, but are all discarded when the
auto GC file from 17:30 is examined second (due to its older mtime).

Work around this in two ways.

Well formed DFS repositories should have at most 1 GC pack. If
2 or more GC packs exist, break the sorting tie by selecting the
smaller file earlier in the pack list. This allows all normal read
code paths to favor the smaller file, which places less pressure
on the DfsBlockCache. If any GC race happens, readers serving clone
requests will prefer the file that is smaller.

During object reuse, flip this ordering so that the smaller file is
last. This allows PackWriter to see smaller deltas last, replacing
larger representations that were previously considered from other
pack files.

Change-Id: I0b7dc8bb9711c82abd6bd16643f518cfccc6d31a
2017-02-08 14:37:12 -08:00
Shawn Pearce 61d4922928 Fix missing deltas near type boundaries
Delta search was discarding discovered deltas if an object appeared
near a type boundary in the delta search window. This has caused JGit
to produce larger pack files than other implementations of the packing
algorithm.

Delta search works by pushing prior objects into a search window, an
ordered list of objects to attempt to delta compress the next object
against. (The window size is bounded, avoiding O(N^2) behavior.)

For implementation reasons multiple object types can appear in the
input list, and the window. PackWriter commonly passes both trees and
blobs in the input list handed to the DeltaWindow algorithm. The pack
file format requires an object to only delta compress against the same
type, so the DeltaWindow algorithm must stop doing comparisions if a
blob would be compared to a tree.

Because the input list is sorted by object type and the window is
recently considered prior objects, once a wrong type is discovered in
the window the search algorithm stops and uses the current result.

Unfortunately the termination condition was discarding any found
delta by setting deltaBase and deltaBuf to null when it was trying
to break the window search.

When this bug occurs, the state of the DeltaWindow looks like this:

                                 current
                                  |
                                 \ /
  input list:  tree0 tree1 blob1 blob2

  window:      blob1 tree1 tree0
                / \
                 |
              res.prev

As the loop iterates to the right across the window, it first finds
that blob1 is a suitable delta base for blob2, and temporarily holds
this in the bestDelta/deltaBuf fields. It then considers tree1, but
tree1 has the wrong type (blob != tree), so the window loop must give
up and fall through the remaining code.

Moving the condition up and discarding the window contents allows
the bestDelta/deltaBuf to be kept, letting the final file delta
compress blob1 against blob0.

The impact of this bug (and its fix) on real world repositories is
likely minimal. The boundary from blob to tree happens approximately
once in the search, as the input list is sorted by type. Only the
first window size worth of blobs (e.g. 10 or 250) were failing to
produce a delta in the final file.

This bug fix does produce significantly different results for small
test repositories created in the unit test suite, such as when a pack
may contains 6 objects (2 commits, 2 trees, 2 blobs).  Packing test
cases can now better sample different output pack file sizes depending
on delta compression and object reuse flags in PackConfig.

Change-Id: Ibec09398d0305d4dbc0c66fce1daaf38eb71148f
2017-02-08 14:36:24 -08:00
Shawn Pearce 12c8462602 Merge "Reintroduce garbage pack coalescing when ttl > 0." 2017-02-08 00:23:40 -05:00
Thirumala Reddy Mutchukota 006f4d4d29 Reintroduce garbage pack coalescing when ttl > 0.
Disabling the garbage pack coalescing when garbageTtl > 0 can result in
lot of garbage packs if they are created within the garbageTtl time.

To avoid a large number of garbage packs, re-introducing garbage pack
coalescing for the packs that are created within a single calendar day
when the garbageTtl is more than one day or one third of the garbageTtl.

Change-Id: If969716aeb55fb4fd0ff71d75f41a07638cd5a69
Signed-off-by: Thirumala Reddy Mutchukota <thirumala@google.com>
2017-02-07 20:34:31 -08:00
Matthias Sohn f8d232213c Branch normalizer should not normalize already valid branch names
Change-Id: Ib746655e32a37c4ad323f1d12ac0817de8fa56cf
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
2017-02-07 00:24:39 +01:00
Hongkai Liu a33663fd4e Detect stale-file-handle error in causal chain
Cover the case where the exception is wrapped up as a
cause, e.g., PackIndex#open(File).

Change-Id: I0df5b1e9c2ff886bdd84dee3658b6a50866699d1
Signed-off-by: Hongkai Liu <hongkai.liu@ericsson.com>
2017-01-30 22:36:59 -04:00
David Pursehouse 62411453f1 Merge branch 'stable-4.6'
* stable-4.6:
  Clean up orphan files in GC

Change-Id: I4fb6b4cd03d032535a9c04ede784bea880b4536b
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
2017-01-31 09:31:10 +09:00
David Pursehouse 25ab5b4d9b Merge "Don't rely on default locale when using toUpperCase() and toLowerCase()" 2017-01-30 07:32:32 -05:00
David Pursehouse c64412e1f9 Merge FileTreeIteratorJava7Test into FileTreeIteratorTest
JGit now requires Java 8, so it is no longer necessary to have a
separate class for Java 7 specific tests. Remove it and merge its
tests into the existing FileTreeIteratorTest.

FileTreeIteratorTest has an @Before annotated method that sets up
some files in the git, which breaks the tests which have assumptions
on the file names. Add adjustments.

Change-Id: I14f88d8e079e1677c8dfbc1fcbf4444ea8265365
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
2017-01-30 13:23:52 +01:00
Hongkai Liu b198f77c1e Rename FileUtilTest to FileUtilsTest and merge in FileUtils7Test
Rename the test class to match the name of the class under test.

JGit now requires Java 8 so it is no longer necessary to have a
separate class (FileUtils7Test) for Java 7 tests. Merge those into
FileUtilsTest.

Change-Id: I39dd7e76a2e4ce97319c7d52261b0a1546879788
Signed-off-by: Hongkai Liu <hongkai.liu@ericsson.com>
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
2017-01-30 02:42:56 -04:00
Hongkai Liu 8fd500e20c Clean up orphan files in GC
An orphan file is either a bitmap or an idx file in pack folder,
and its corresponding pack file is missing.

Change-Id: I3c4cb1f7aa99dd7b398bdb8d513f528d7761edff
Signed-off-by: Hongkai Liu <hongkai.liu@ericsson.com>
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
2017-01-30 00:55:36 +01:00
Matthias Sohn 53ad437382 RepositoryCacheTest: avoid to close already closed repository
The tearDown() of the superclass closed the repository once more which
led to a negative use count warning logged by Repository.close().

Change-Id: I331f85a540c68264a53456276c32f72b79113d61
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
2017-01-28 21:19:55 +01:00
David Pursehouse ac6353e9e5 RefUpdateTest: Don't call createBareRepository in try-with-resource
createBareRepository adds the created repo to the list of repos to be
closed in the superclass's teardown. Wrapping it in try-with-resource
causes it to be closed too many times, resulting in a corrupt use
count.

Change-Id: I4c70630bf6008544324dda453deb141f4f89472c
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
2017-01-28 19:16:48 +01:00
David Pursehouse 25b14084c9 ReceivePackAdvertiseRefsHookTest: Don't close repositories in teardown
The repositories get added to the "toClose" set by createBareRepository,
and are then closed in the superclass's tearDown method.

Explicitly closing them in this test class's teardown causes the use
count to go negative when subsequently closed again by the superclass.

Change-Id: Idcbb16b4cf4bf0640d7e4ac15d1926d8a27c1251
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
2017-01-28 17:46:28 +01:00
David Pursehouse 883856110c RepoCommandTest: Don't wrap create{Bare,Work}Directory in t-w-r
These methods add the created Repository into "toClose", and they are
then closed by LocalDiskRepositoryTestCase's tearDown method.

Calling them in try-with-resource causes them to first be closed in
the test method, and then again in tearDown, which results in the use
count going negative and a log message on the console.

While this is not a serious problem, having so many false positives
in the logs will potentially drown out real cases of Repository being
closed too many times.

Change-Id: Ib374445e101dc11cb840957b8b19ee1caf777392
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
2017-01-28 17:46:27 +01:00
David Pursehouse a9a3af4af7 GitConstructionTest: Remove unnecessary calls to Repository.close()
The repositories are already closed in the superclass teardown due
to being added to the "toClose" set.

Change-Id: I768ba8a02fc585907687caf37e2e283434016c04
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
2017-01-28 17:46:27 +01:00
Matthias Sohn a4feeb0194 Don't rely on default locale when using toUpperCase() and toLowerCase()
Otherwise these methods may produce unexpected results if used for
strings that are intended to be interpreted locale independently.
Examples are programming language identifiers, protocol keys, and HTML
tags. For instance, "TITLE".toLowerCase() in a Turkish locale returns
"t\u0131tle", where '\u0131' is the LATIN SMALL LETTER DOTLESS I
character.

See
https://docs.oracle.com/javase/8/docs/api/java/lang/String.html#toLowerCase--
http://blog.thetaphi.de/2012/07/default-locales-default-charsets-and.html

Bug: 511238
Change-Id: Id8d8f37d84d62239c918b81f8d883ed798d87656
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
2017-01-28 15:06:15 +01:00
Thirumala Reddy Mutchukota c9f55032a2 Record the estimated size of the pack files.
The Compacter and Garbage Collector will record the estimated size of
the newly going to be created compact, gc or garbage packs. This
information can be used by the clients to better make a call on how to
actually store the pack based on the approximated expected size.

Added a new protected method DfsObjDatabase.newPack(PackSource
packSource, long estimatedPackSize), so that the clients can override
this method to make use of the estimatedPackSize while creating a new
PackDescription object. The default implementation of this method is
equivalent to
newPack(packSource).setEstimatedPackSize(estimatedPackSize). I didn't
make it abstract because that would force all the existing sub classes
of DfsObjDatabase to implement this method. Due to this default
implementation, the estimatedPackSize is added to DfsPackDescription
using a setter instead of a constructor parameter (even though
constructor parameter would be a better choice as this value is set only
during the object creation).

Change-Id: Iade1122633ea774c2e842178a6a6cbb4a57b598b
Signed-off-by: Thirumala Reddy Mutchukota <thirumala@google.com>
2017-01-26 12:01:59 -08:00