aboutsummaryrefslogtreecommitdiffstats
path: root/org.eclipse.jgit.test
Commit message (Collapse)AuthorAgeFilesLines
* Organize importsDavid Pursehouse2016-11-1424-41/+27
| | | | | Change-Id: I7c545d06b1bced678c020fab9af1382bc4416b6e Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
* Add {get,set}GitwebDescription to RepositoryShawn Pearce2016-11-141-0/+86
| | | | | | | | | | | | | | | | This method pair allows the caller to read and modify the description file that is traditionally used by gitweb and cgit when rendering a repository on the web. Gerrit Code Review has offered this feature for years as part of its GitRepositoryManager interface, but its fundamentally a feature of JGit and its Repository abstraction. git-core typically initializes a repository with a default value inside the description file. During getDescription() this string is converted to null as it is never a useful description. Change-Id: I0a457026c74e9c73ea27e6f070d5fbaca3439be5
* Check that DfsBlockCache#blockSize is a power of 2Philipp Marx2016-11-111-0/+88
| | | | | | | | | | In case a value is used which isn’t a power of 2 there will be a high chance of java.lang.ArrayIndexOutBoundsException and org.eclipse.jgit.errors.CorruptObjectException due to a mismatching assumption for the DfsBlockCache#blockSizeShift parameter. Change-Id: Ib348b3704edf10b5f93a3ffab4fa6f09cbbae231 Signed-off-by: Philipp Marx <smigfu@googlemail.com>
* Fix loop in auto gcMatthias Sohn2016-11-073-2/+142
| | | | | | | | | | * GC.tooManyLooseObjects() always responded true since the loop missed to advance the iterator so it always incremented until the threshold was exceeded. * Also fix loop exit criterion which was off by 1. * Add some tests. Change-Id: I70976dfaa026efbcf3c46bd45941f37277a18e04 Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
* Make streamFileThreshold configurableKevin Corcoran2016-10-241-0/+21
| | | | | | | | | | | | | Previously, the streamFileThreshold, the threshold at which a file would be streamed rather than loaded entirely into memory, was only configurable on a global basis. This commit makes this threshold configurable on a per-loader basis. Bug: 490404 Change-Id: I492c18c3155dbf56eedda9044a61d76120fd75f9 Signed-off-by: Kevin Corcoran <kevin.corcoran@puppetlabs.com> Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
* TreeFormatter: disallow empty filenames in treesDavid Turner2016-10-192-1/+18
| | | | | | | | | Git barfs on these (and they don't make any sense), so we certainly shouldn't write them. Change-Id: I3faf8554a05f0fd147be2e63fbe55987d3f88099 Signed-off-by: David Turner <dturner@twosigma.com> Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
* DiffCommandTest: Don't call toString on String instancesDavid Pursehouse2016-10-191-5/+5
| | | | | Change-Id: Ib308b3498593d595b3d8741a9b2d241bbc7441c3 Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
* FileNameMatcherTest: Use Character.valueOf rather than new CharacterDavid Pursehouse2016-10-191-1/+1
| | | | | Change-Id: I9d6e20a258d34ae1d2700fbe8e6c6e3b0ba94424 Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
* Use valueOf rather than constructor for Integer and BooleanDavid Pursehouse2016-10-181-1/+1
| | | | | Change-Id: I1c65b2e40ba6ec5860903b11b4631e014f3dc5ce Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
* Upgrade buck to 7b7817c48f30687781040b2b82ac9218d5c4eaa4David Pursehouse2016-10-181-1/+0
| | | | | | | | | | | | | | | | | Upgrade to match the version used on Gerrit's master branch. Requires a couple of modifications to make the tests work: - Remove source_under_test parameters from java_test calls. - Add vm_args with explicit setting of tmpdir location for http tests. This is needed due to upstream changes in temporary directory handling [1]. [1] https://github.com/facebook/buck/issues/946 Change-Id: I5d5dd5edc335d44b118e8587f69ba89b83fc7fbb Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
* Merge branch 'stable-4.5'Matthias Sohn2016-10-141-0/+21
|\ | | | | | | | | | | | | | | | | | | | | | | * stable-4.5: Unconditionally close repositories in RepositoryCache.clear() Fix eviction of repositories with negative usage count Adapt to parameter removed from RepositoryCache.unregisterAndCloseRepository(). Change-Id: I7087667056ced401a3b3a027977f2715cd77a1c5 Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
| * Fix eviction of repositories with negative usage countHugo Arès2016-10-121-0/+21
| | | | | | | | | | | | | | | | | | | | If the repository close method was called twice (or more) for one open, the usage count became negative and the repository was never be evicted from the cache because the method checking if repository is expired was not considering negative usage count. Change-Id: I18a80c415c54c37d1b9def2b311ff2d0afa455ca Signed-off-by: Hugo Arès <hugo.ares@ericsson.com>
* | Fix symlink content comparison on MacOS in tree walkThomas Wolf2016-10-112-0/+264
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Symlinks on MacOS are written as UTF-8 NFD, but readSymbolicLink().toString() converts to NFC with potentially fewer bytes. May occur in particular if the link target has non-ASCII characters for which the NFC and NFD encodings differ. This may lead to an EOFException: Short read of block. This causes all kinds of weird effects in EGit, ranging from failing rebases (which report the exception to the user) to EGit decorations in the navigator silently disappearing (and never coming back). * Rename readContentAsNormalizedString() to readSymlinkTarget() as it's called only for symlinks. Also make it protected. * Fix by allowing the read to succeed even if less than the expected number of bytes are returned by the entry's input stream. * Override in FileTreeIterator to use fs.readSymlink() directly. Includes a new MacOS-only test. Change-Id: I264c5972d67b1cbb1ed690580f5706e671b9affd Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch> Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
* | Fix CheckoutCommand to return updated files even on NONDELETED statusChristian Halstrick2016-09-271-0/+33
| | | | | | | | | | | | | | | | | | | | | | CheckoutCommand was not returning updated and removed files in case of an overall status of NONDELETED. That's status which occurs especially on the Windows platform when Checkout wanted to delete files but the filesystem doesn't allow this. The situation is more seldom on linux/mac because open filehandles don't stop a deletion attempt and checkout succeeds more often. Change-Id: I4828008e58c09bd8f9edaf0f7eda0a79c629fb57
* | Merge branch 'stable-4.5'Matthias Sohn2016-09-241-0/+146
|\| | | | | | | | | | | | | | | * stable-4.5: Fix carrying over flags during a RevWalk Change-Id: Ibf4573c5664271dfa7a6ecc3ede6eaad749f89d8 Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
| * Merge "Fix carrying over flags during a RevWalk" into stable-4.5Shawn Pearce2016-09-241-0/+146
| |\
| | * Fix carrying over flags during a RevWalkChristian Halstrick2016-09-231-0/+146
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | There was a bug when carrying over flags from a merge commit to its non-first parents. The first parent of a merge commit was handled differently and correct but the non-first parents are handled by a recursive algorithm. Flags should be copied from the root merge commit to parent-2, to grandparent-2, ... up to the limit of STACK_DEPTH==500 parents-levels. But the recursive algorithm was always copying only to the direct parents of the merge commit and not the grand*-parents. This seems to be no problem when commits are handled in a strict date order because then copying only one level is no problem if children are handled before parents. But when commits are not seperated anymore by distinctive correct dates (e.g. because all commits have the same date) then it may happen that a merge-parent is handled before the merge commit and when dealing later with the merge commit one has to copy flags down to more than one level Bug: 501211 Change-Id: I2d79a7cf1e3bce21a490905ccd9d5e502d7b8421
| * | Prepare 4.5.1-SNAPSHOT buildsMatthias Sohn2016-09-212-42/+42
| | | | | | | | | | | | | | | Change-Id: I3305e8a09a3fb06f25a316cff2bdbb551d3ade68 Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
| * | JGit v4.5.0.201609210915-rv4.5.0.201609210915-rMatthias Sohn2016-09-212-2/+2
| |/ | | | | | | | | Change-Id: Idc02a1a1d74f84605d764c239803f0cfbad94eb7 Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
* | pgm, test: Add missing dependency for buck buildDavid Pursehouse2016-09-221-0/+1
| | | | | | | | | | Change-Id: I26d69fb6d35c6fb120360ef143d1b1f565d4014c Signed-off-by: David Pursehouse <david.pursehouse@gmail.com> Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
* | Change JGit minimum execution environment to JavaSE-1.8Matthias Sohn2016-09-208-125/+5
| | | | | | | | | | Bug: 500059 Change-Id: I47f3f6749a67da52029f84e002d9b155ed56d2b7 Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
* | Add built-in LFS smudge filter for local caseChristian Halstrick2016-09-202-28/+102
| | | | | | | | | | | | | | | | | | | | | | | | Adds a JGit built-in implementation of the "git lfs smudge" filter. This filter should do the same as the one described in [1] besides that it only supports the local case when the lfs objects are already present in the media directory. Remote cases where download of LFS objects from an LFS server is needed will be done in a later commit. [1] https://github.com/github/git-lfs/blob/master/docs/man/git-lfs-smudge.1.ronn Change-Id: I8ff661d4edd3667ef7f86f3b4fa33e568eb4c8f4
* | Add built-in LFS clean filterChristian Halstrick2016-09-202-2/+79
| | | | | | | | | | | | | | | | | | | | | | | | Adds a JGit built-in implementation of the "git lfs clean" filter. This filter should do the same as the one described in [1]. But since this filter is written in Java and can be called by JGit without forking new processes it should be much faster [1] https://github.com/github/git-lfs/blob/master/docs/man/git-lfs-clean.1.ronn Change-Id: If60e387e97870245b4bd765eda6717eb84cffb1d
* | Add support for built-in smudge filtersChristian Halstrick2016-09-201-0/+94
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | JGit supports smudge filters defined in repository configuration. The filters are implemented as external programs filtering content by accepting the original content (as seen in git's object database) on stdin and which emit the filtered content on stdout. This content is then written to the file in the working tree. To run such a filter JGit has to start an external process and pump data into/from this process. This commit adds support for built-in smudge filters which are implemented in Java and which are executed by jgit's main thread. When a filter is defined in the configuration as "jgit://builtin/<filterDriverName>/smudge" then JGit will lookup in a static map whether a builtin filter is registered under this name. If found such a filter is called to do the filtering. The functionality in this commit requires that a program using JGit explicitly calls the JGit API to register built-in implementations for specific smudge filters. In follow-up commits configuration parameters will be added which trigger such registrations. Change-Id: Ia743aa0dbed795e71e5792f35ae55660e0eb3c24
* | Add support for built-in clean filtersChristian Halstrick2016-09-201-0/+157
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | JGit supports clean filters defined in repository configuration. The filters are implemented as external programs filtering content by accepting the original content (as seen in the working tree) on stdin and which emit the filtered content on stdout. To run such a filter JGit has to start an external process and pump data into/from this process. This commit adds support for clean filters which are implemented in Java and which are executed by jgit's main thread. When a filter is defined in the configuration as "jgit://builtin/<filterDriverName>/clean" then JGit will lookup in a static map whether a filter is registered under this name. If found such a filter is called to do the filtering. The functionality in this commit requires that a program using JGit explicitly calls the JGit API to register built-in implementations for specific clean filters. In follow-up commits configuration parameters will be added which trigger such registrations. Other commits will add implementations for lfs filters. Change-Id: I0344d3c54801c9a46e5a606c5df17e5f2e17b2be
* | Prepare 4.6.0-SNAPSHOT buildsMatthias Sohn2016-09-192-42/+42
|/ | | | | Change-Id: Id2eafc331ee32c332c2a9b867b05c260beb0d10f Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
* Handle all values of branch.[name].rebaseThomas Wolf2016-09-152-37/+58
| | | | | | | | | | | | | | | | | BranchConfig treated this config property as a boolean, but git also allows the values "preserve" and "interactive". Config property pull.rebase also allows the same values. Replace private enum PullCommand.PullRebaseMode by new public enum BranchConfig.BranchRebaseMode and adapt all uses. Add a new setter to PullCommand. Note: PullCommand will treat "interactive" like "true", i.e., as a non-interactive rebase. Not sure how "interactive" should be handled. At least it won't balk on it. Bug: 499482 Change-Id: I7309360f5662b2c2efa1bd8ea6f112c63cf064af Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>
* Ignore trailing spaces in directory rule patternsAndrey Loskutov2016-09-143-1/+43
| | | | | | Bug: 500967 Change-Id: I7fabc2654af97011c62f46d5c30ee992341e45e2 Signed-off-by: Andrey Loskutov <loskutov@gmx.de>
* Add support for post-commit hooksMartin Goellnitz2016-09-131-0/+62
| | | | | Change-Id: I6691b454404dd4db3c690ecfc7515de765bc2ef7 Signed-off-by: Martin Goellnitz <m.goellnitz@outlook.com> Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
* Don't log error if system git config does not existMatthias Sohn2016-09-051-0/+13
| | | | | | | | | | - enhance FS.readPipe to throw an exception if the external command fails to enable the caller to handle the command failure - reduce log level to warning if system git config does not exist - improve log message Bug: 476639 Change-Id: I94ae3caec22150dde81f1ea8e1e665df55290d42 Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
* Add missing dependency to slf4j-log4j bridgeMatthias Sohn2016-09-051-1/+2
| | | | | | Without the bridge JGit tests don't show log output in Eclipse console. Change-Id: I7acce1f1787960b5ca98377cb5c7f599a8a220b5 Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
* Rename FSJava7Test to FSTestMatthias Sohn2016-09-031-1/+1
| | | | | | | Since 4.0 JGit does no longer support Java versions older than Java 7 so there is no need anymore to mention Java 7 in the class name. Change-Id: Ic46c9d89a7e919ae4a69487fa06de0478d2b21f0 Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
* ReceivePack: refactor push option parsingShawn Pearce2016-08-261-10/+8
| | | | | | | | | | | | | Refactor all of the push option support code to allocate the list immediately before parsing the options section off the stream. Move option support down to ReceivePack instead of BaseReceivePack. Push options are specific to the ReceivePack protocol and are not likely to appear in the 4 year old subscription proposal. These changes are OK before JGit 4.5 ships as no consumer should be relying on these new APIs. Change-Id: Ib07d18c877628aba07da07cd91875f918d509c49
* Fix push option initalization on HTTPStefan Beller2016-08-261-1/+1
| | | | | | | | | | | Initialize pushOptions when we decide to use them, instead of when we advertise them. In the case of HTTP the advertisement is in a different network request, hence in a different instance of the BaseReceivePack. Change-Id: I094c60942e04de82cb6d8433c9cd43a46ffae332 Signed-off-by: Stefan Beller <sbeller@google.com>
* Clarify the semantics of DfsRefDatabase#compareAndPutMasaya Suzuki2016-08-251-0/+39
| | | | | | | | | | | | | | | | | | | | | | | | | | | DfsRefDatabase#compareAndPut had a vague semantics for reference matching. Because of this, an operation to make a symbolic reference had been broken for some DFS implementations even if they followed the contract of compareAndPut. The clarified semantics requires the implementations to satisfy the followings: * Matching references should be both symbolic references or both object ID references. * If both are symbolic references, both should have the same target name. * If both are object ID references, both should have the same object ID. This semantics is defined based on https://git.eclipse.org/r/#/c/77416/. Before this commit, DfsRefDatabase couldn't see the target of symbolic references. InMemoryRepository is changed to comply with the new semantics. This semantics change can affect the existing DFS implementations that only checks object IDs. This commit adds two tests that the previous InMemoryRepository couldn't pass. Change-Id: I6c6b5d3cc8241a81f4a37782381c88e8a59fdf15 Signed-off-by: Masaya Suzuki <masayasuzuki@google.com>
* NoteMapTest: Add missing @Test annotationsDave Borowitz2016-08-261-1/+7
| | | | | | | | | | The RepositoryTestCase hierarchy no longer comes from TestCase, so all test methods must have @Test. Fix one test that was broken but never run; fortunately this was just a typo in the test code. Change-Id: I3ac8ccdab5e2d5539c63d7b0a88d8bdb0c5ff66e Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
* Use FS#lastModified instead of File#lastModifiedMasaya Suzuki2016-08-243-15/+18
| | | | | | | | | | | | | | | This fixes the tests failed in JDK8. FS uses java.nio API to get file attributes. The timestamps obtained from that API are more precise than the ones from java.io.File#lastModified() since Java8. This difference accidentally makes JGit detect newly added files as smudged. Use the precised timestamp to avoid this false positive. Bug: 500058 Change-Id: I9e587583c85cb6efa7562ad6c5f26577869a2e7c Signed-off-by: Masaya Suzuki <masayasuzuki@google.com> Signed-off-by: Andrey Loskutov <loskutov@gmx.de>
* Added Java 7 launch config with LANG env. variable setAndrey Loskutov2016-08-241-0/+30
| | | | | | | This avoids symlink test errors on Linux Change-Id: Id8193524c40394a90b8315ab0b8256670d618cb5 Signed-off-by: Andrey Loskutov <loskutov@gmx.de>
* Enhance ResetCommand to allow disabling reflog updateMatthias Sohn2016-08-171-0/+40
| | | | | | | This will be used by EGit for implementing commit amend in the staging view (see Idcd1efeeee8b3065bae36e285bfc0af24ab1e88f). Change-Id: Ice9ebbb1c0c3314c679f4db40cdd3664f61c27c3 Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
* Merge "Add path src/ to source path in build.properties"Matthias Sohn2016-08-081-1/+2
|\
| * Add path src/ to source path in build.propertiesMatthias Sohn2016-08-061-1/+2
| | | | | | | | | | | | This fixes the warning "src/ is missing from source.." Change-Id: I166e3a6a3d5230e4110d3283ec4dbc7d1dfe6732 Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
* | Merge "Skip cleaning inner repositories by default in CleanCommand"Christian Halstrick2016-08-071-0/+40
|\ \
| * | Skip cleaning inner repositories by default in CleanCommandMatthaus Owens2016-08-041-0/+40
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Previously jgit would attempt to clean git repositories that had not been committed by calling a non-recursive delete on them, which would fail as they are directories. This commit addresses that issue in the following ways. Repositories are skipped in a default clean, similarly to cgit and only cleaned when the force flag is applied. When the force flag is applied repositories are deleted using a recursive delete call. The force flag and setForce method are added here to CleanCommand to support this change. Bug: 498367 Change-Id: Ib6cfff65a033d0d0f76395060bf76719e13fc467 Signed-off-by: Matthaus Owens <matthaus@puppetlabs.com>
* | | Merge "Add testCleanDirsWithSubmodule test to CleanCommandTest"Christian Halstrick2016-08-071-0/+28
|\| | | |/ |/|
| * Add testCleanDirsWithSubmodule test to CleanCommandTestMatthaus Owens2016-08-041-0/+28
| | | | | | | | | | | | | | | | | | This commit adds some test coverage to cleaning a repository with a submodule, which did not previously exist. Bug: 498367 Change-Id: Ia5c4e4cc53488800dd486f8556dc57656783f1c4 Signed-off-by: Matthaus Owens <matthaus@puppetlabs.com>
* | Merge changes I27961679,I91be6165,If0dbd562Jonathan Nieder2016-08-051-3/+0
|\ \ | | | | | | | | | | | | | | | | | | * changes: LfsProtocolServlet: Allow access to objects in request LfsProtocolServlet: Allow getLargeFileRepository to raise exceptions Remove references to org.eclipse.jgit.java7
| * | Remove references to org.eclipse.jgit.java7David Pursehouse2016-08-051-3/+0
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The bundle org.eclipse.jgit.java7 was removed in 4.0. Remove references to it from the README.md. Remove reference to it from org.eclipse.jgit.test/.project, which causes an error message when opening the project in Eclipse: Resource '/org.eclipse.jgit.java7' does not exist. Change-Id: If0dbd562dcd60550bec3c0f793289474b7624bce Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
* | | Shallow fetch/clone: Make --depth mean the total history depthTerry Parker2016-08-051-7/+49
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | cgit changed the --depth parameter to mean the total depth of history rather than the depth of ancestors to be returned [1]. JGit still uses the latter meaning, so update it to match cgit. depth=0 still means a non-shallow clone. depth=1 now means only the wants rather than the wants and their direct parents. This is accomplished by changing the semantic meaning of "depth" in UploadPack and PackWriter to mean the total depth of history desired, while keeping "depth" in DepthWalk.{RevWalk,ObjectWalk} to mean the depth of traversal. Thus UploadPack and PackWriter always initialize their DepthWalks with "depth-1". [1] upload-pack: fix off-by-one depth calculation in shallow clone https://code.googlesource.com/git/+/682c7d2f1a2d1a5443777237450505738af2ff1a Change-Id: I87ed3c0f56c37e3491e367a41f5e555c4207ff44 Signed-off-by: Terry Parker <tparker@google.com>
* | | Shallow fetch: Respect "shallow" linesTerry Parker2016-08-051-22/+75
|/ / | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | When fetching from a shallow clone, the client sends "have" lines to tell the server about objects it already has and "shallow" lines to tell where its local history terminates. In some circumstances, the server fails to honor the shallow lines and fails to return objects that the client needs. UploadPack passes the "have" lines to PackWriter so PackWriter can omit them from the generated pack. UploadPack processes "shallow" lines by calling RevWalk.assumeShallow() with the set of shallow commits. RevWalk creates and caches RevCommits for these shallow commits, clearing out their parents. That way, walks correctly terminate at the shallow commits instead of assuming the client has history going back behind them. UploadPack converts its RevWalk to an ObjectWalk, maintaining the cached RevCommits, and passes it to PackWriter. Unfortunately, to support shallow fetches the PackWriter does the following: if (shallowPack && !(walk instanceof DepthWalk.ObjectWalk)) walk = new DepthWalk.ObjectWalk(reader, depth); That is, when the client sends a "deepen" line (fetch --depth=<n>) and the caller has not passed in a DepthWalk.ObjectWalk, PackWriter throws away the RevWalk that was passed in and makes a new one. The cleared parent lists prepared by RevWalk.assumeShallow() are lost. Fortunately UploadPack intends to pass in a DepthWalk.ObjectWalk. It tries to create it by calling toObjectWalkWithSameObjects() on a DepthWalk.RevWalk. But it doesn't work: because DepthWalk.RevWalk does not override the standard RevWalk#toObjectWalkWithSameObjects implementation, the result is a plain ObjectWalk instead of an instance of DepthWalk.ObjectWalk. The result is that the "shallow" information is thrown away and objects reachable from the shallow commits can be omitted from the pack sent when fetching with --depth from a shallow clone. Multiple factors collude to limit the circumstances under which this bug can be observed: 1. Commits with depth != 0 don't enter DepthGenerator's pending queue. That means a "have" cannot have any effect on DepthGenerator unless it is also a "want". 2. DepthGenerator#next() doesn't call carryFlagsImpl(), so the uninteresting flag is not propagated to ancestors there even if a "have" is also a "want". 3. JGit treats a depth of 1 as "1 past the wants". Because of (2), the only place the UNINTERESTING flag can leak to a shallow commit's parents is in the carryFlags() call from markUninteresting(). carryFlags() only traverses commits that have already been parsed: commits yet to be parsed are supposed to inherit correct flags from their parent in PendingGenerator#next (which doesn't happen here --- that is (2)). So the list of commits that have already been parsed becomes relevant. When we hit the markUninteresting() call, all "want"s, "have"s, and commits to be unshallowed have been parsed. carryFlags() only affects the parsed commits. If the "want" is a direct parent of a "have", then it carryFlags() marks it as uninteresting. If the "have" was also a "shallow", then its parent pointer should have been null and the "want" shouldn't have been marked, so we see the bug. If the "want" is a more distant ancestor then (2) keeps the uninteresting state from propagating to the "want" and we don't see the bug. If the "shallow" is not also a "have" then the shallow commit isn't parsed so (2) keeps the uninteresting state from propagating to the "want so we don't see the bug. Here is a reproduction case (time flowing left to right, arrows pointing to parents). "C" must be a commit that the client reports as a "have" during negotiation. That can only happen if the server reports it as an existing branch or tag in the first round of negotiation: A <-- B <-- C <-- D First do git clone --depth 1 <repo> which yields D as a "have" and C as a "shallow" commit. Then try git fetch --depth 1 <repo> B:refs/heads/B Negotiation sets up: have D, shallow C, have C, want B. But due to this bug B is marked as uninteresting and is not sent. Change-Id: I6e14b57b2f85e52d28cdcf356df647870f475440 Signed-off-by: Terry Parker <tparker@google.com>
* | Shallow fetch: avoid sending unneeded blobsTerry Parker2016-08-041-3/+69
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | When doing an incremental fetch from JGit, "have" commits are marked as "uninteresting". In a non-shallow fetch, when the RevWalk hits an "uninteresting" commit it marks the commit's corresponding tree as uninteresting. That has the effect of dropping those trees and all the trees and blobs they reference out of the thin pack returned to the client. However, shallow fetches use a DepthWalk to limit the RevWalk, which nearly always causes the RevWalk to terminate before encountering the "have" commits. As a result the pack created for the incremental fetch never encounters "uninteresting" tree objects and thus includes duplicate objects that it knows the client already has. Change-Id: I7b1f7c3b0d83e04d34cd2fa676f1ad4fec904c05 Signed-off-by: Terry Parker <tparker@google.com>