Remove completely the empty directories under refs/<namespace>
including the first level partition of the changes, when they are
completely empty.
Bug: 536777
Change-Id: I88304d34cc42435919c2d1480258684d993dfdca
Signed-off-by: Luca Milanesio <luca.milanesio@gmail.com>
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
When processing a fetch using protocol v2, UploadPack#fetchV2 sends an
extraneous flush pkt when also sending a packfile (#sendPack sending its
own flush pkt). Update that method to only send the flush pkt if the
packfile is not being sent.
Change-Id: I7117a264bccd2d7f3a048645fcb8425a9d78d526
Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
In a0c9016abd ("upload-pack: send refs' objects despite "filter"",
2018-07-09), Git updated the "filter" option in the fetch-pack
upload-pack protocol to not filter objects explicitly specified in
"want" lines, even if they match the criterion of the filter. Update
JGit to match that behavior.
Change-Id: Ia4d74326edb89e61062e397e05483298c50f9232
Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
Don't try to delete folders if the merger chooses THEIRS, but all of
BASE, OURS, and THEIRS contain the folder.
Add a test for rebase with auto-stash and subdirectories that
verifies this case. The needless directory deletion and reporting
such directories in getModifiedFiles() was the root cause of bug
536880.
Note even with this fix, bug 536880 will not be fixed in all cases
yet. There may still be cases where the set of modified files ends
up containing directories. This will be dealt with in EGit where
this set is used. (See https://git.eclipse.org/r/#/c/126242/ .)
Bug: 536880
Change-Id: I62b4571a1c1d4415934a6cb4270e0c8036deb2e9
Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>
RacyGitTests depend on filesystem timer resolution. We wait for a file
system timer tick, remember that time as t1, modify a file and assume
that this file has a lastmodified of t1.
If this assumption is not fulfilled then ignore the test result.
Bug: 526111
Change-Id: Ia38b7d2f99171ef54b8f9fe5be343cf9fcfd3971
Currently SubmoduleAddCommand always uses the path as submodule name.
This patch lets the caller specify a submodule name.
SubmoduleUpdateCommand still does not make use of the submodule name
(see bug 535027) but Git does. To avoid triggering CVE-2018-11235,
do some validation on the name to avoid '..' path components.
[jn: fleshed out commit message, mostly to work around flaky CI]
Change-Id: I6879c043c6d7973556e2080387f23c246e3d76a5
Signed-off-by: Masaya Suzuki <masayasuzuki@google.com>
Signed-off-by: Jonathan Nieder <jrn@google.com>
Make the method names more consistent and their semantics simpler:
hasRef and seekRef to look up a single exact reference by name and
hasRefsByPrefix and seekRefsByPrefix to look up multiple references by
name prefix.
In particular, splitting hasRef into two separate methods for its
different uses makes DfsReftableDatabase.isNameConflicting easier to
follow.
[jn: fleshed out commit message]
Change-Id: I71106068ff3ec4f7e14dd9eb6ee6b5fab8d14d0b
Signed-off-by: Minh Thai <mthai@google.com>
Signed-off-by: Jonathan Nieder <jrn@google.com>
Reftable implementation of RefDatabase.getRefsByPrefix() should be
more performant, as references are filtered directly by prefix;
instead of fetching the whole subtree then filter by prefix.
Change-Id: If4f5f8c08285ea1eaec9efb83c3d864cea7a1321
Signed-off-by: Minh Thai <mthai@google.com>
After packaging references, the folders containing these references are
not deleted. In a busy repository, this causes operations to slow down
as traversing the references tree becomes longer.
Delete empty reference folders after the loose references have been
packed.
To avoid deleting a folder that was just created by another concurrent
operation, only delete folders that were not modified in the last 30
seconds.
Signed-off-by: Hector Oswaldo Caballero <hector.caballero@ericsson.com>
Change-Id: Ie79447d6121271cf5e25171be377ea396c7028e0
Signed-off-by: Luca Milanesio <luca.milanesio@gmail.com>
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
It is convenient for TestRepository to return fully parsed
objects from its commit()/tree()/blob() methods, so that test
code doesn't have to remember to parse them before making
assertions about them.
Update TestRepostiory to return fully parsed objects.
Adjust the tests that are affected by this change in behavior.
Change-Id: I09d03d0c80ad22cb7092f4a2eaed99d40a10af63
Signed-off-by: Terry Parker <tparker@google.com>
In a new RevWalk, if the first object parsed is one of the
shallow commits, the following happens:
1) RevCommit.parseCanonical() is called on a new "r1" RevCommit.
2) RevCommit.parseCanonical() immediately calls
RevWalk.initializeShallowCommits().
3) RevWalk.initializeShallowCommits() calls lookupCommit(id),
creating and adding a new "r2" version of this same object and
marking its parents empty.
4) RevCommit.parseCanonical() initializes the "r1" RevCommit's
fields, including the parents.
5) RevCommit.parseCanonical()'s caller uses the "r1" commit that
has parents, losing the fact that it is a shallow commit.
This change passes the current RevCommit as an argument to
RevWalk.initializeShallowCommits() so that method can set its
parents empty rather than creating the duplicate "r2" commit.
Change-Id: I67b79aa2927dd71ac7b0d8f8917f423dcaf08c8a
Signed-off-by: Terry Parker <tparker@google.com>
Test that the commit selection algorithm provides appropriate
coverage across all branches.
Change-Id: I82c71b52068f01e8cef2398aecfec8e144d9a68b
Signed-off-by: Terry Parker <tparker@google.com>
The same errors are suppressed when generating javadoc.
Suppress the errors during site generation.
Change-Id: I83bd1c10e5de82f47c351e7edf2c9230be4b1f21
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
* stable-5.0:
Prepare 5.0.1-SNAPSHOT builds
JGit v5.0.0.201806131550-r
JGit v5.0.0.201806131210-r
Downgrade Apache httpclient to 4.5.2.v20170210-0925
RefUpdateTest: Refactor to not use deprecated Repository#getAllRefs
Propagate failure of ssh command to caller of SshSupport
Make JGit describe behaves same as c-git for lightweight tags
Fix issues with LFS on GitHub (SSH)
Change-Id: I0471440919adfdbfc72996711d9e0bbd1f3cf477
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
JGit now considers lightweight tags only if the --tags option is set
i.e. `git.describe().setAllTags(true)` has to be set, else the default
is now as in c git:
Only annotated tags are evaluated unless you pass true
equivalent to --tags (or --all) by the option setAllTags.
Hint: This (still) doesn't address any difference between c-git
`--all` and `!--all --tags` behavior;
perhaps this might be a follow up request
Bug: 423206
Change-Id: I9a3699756df0b9c6a7c74a7e8887dea0df17c8e7
Signed-off-by: Marcel Trautwein <me+eclipse@childno.de>
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
* stable-5.0:
Use constant for ".lock"
Simplify locking of FileRepository's index snapshot
Refactor FileRepository.detectIndexChange()
Change-Id: Ifd427711359bcf38b2c877b2143d45bff0c9895a
Signed-off-by: Jonathan Nieder <jrn@google.com>
* stable-5.0:
Teach UploadPack "filter" in protocol v2 fetch
Refactor test of capabilities output
Refactor v2 advertisement into own function
Refactor parsing of "filter" into its own method
Disallow unknown args to "fetch" in protocol v2
Teach UploadPack shallow fetch in protocol v2
Refactor unshallowCommits to local variable
Add protocol v2 support in http
Give info/refs services more control over response
Change-Id: I1683902222e076e1091795e94790a264550afb7b
Signed-off-by: Jonathan Nieder <jrn@google.com>
If the configuration variable uploadpack.allowfilter is true, advertise
that "filter" is supported, and support it if the client sends such an
argument.
Change-Id: I7de66c0a0ada46ff71c5ba124d4ffa7c47254c3b
Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
A subsequent patch will dynamically generate the capability
advertisement, so the capability advertisements produced are not always
the same. Separate the checking of the advertisements into its own test
method.
Change-Id: I768d14b9d1a244d5d886c42ffd62ef3957b518fb
Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
JGit's implementation of the fetch command of protocol v2, unlike its
implementation of ls-refs, currently tolerates unknown arguments.
Tighten fetch to not allow unrecognized arguments and add tests to
verify this behavior for both ls-refs and fetch.
Change-Id: I321161d568bd638252fab1a47b06b924d472a669
Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
Add support for the "shallow" and "deepen" parameters in the "fetch"
command in the fetch-pack/upload-pack protocol v2. Advertise support for
this in the capability advertisement.
TODO: implement deepen-relative, deepen-since, deepen-not
Change-Id: I7ffd80d6c38872f9d713ac7d6e0412106b3766d7
Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
Signed-off-by: Jonathan Nieder <jrn@google.com>
Provide a factory for comparators that use the default heuristics except
with a different ordering of PackSources.
Change-Id: I0809b64deb3d0486040076946fdbdad650d69240
There are several ways of comparing DfsPackDescriptions for different
purposes, such as object lookup search order and reftable ordering. Some
of these are later compounded into comparators on other objects, so they
appear in the code as Comparator<DfsReftable>, for example.
Put all the DfsPackDescription comparators in static methods on
DfsPackDescription itself. Stop implementing Comparable, to avoid giving
the impression that there is always one true and correct way of sorting
packs.
Change-Id: Ia5ca65249c13373f7ef5b8a5d1ad50a26577706c
Rather than requiring callers to do their own computations based on the
package-private "category" number, provide an actual
Comparator<PackSource> instance, and explicitly discourage usage of
default Enum comparison.
Construct the default comparator using a builder pattern based on
defining equivalence classes. This gives us the same behavior as the old
category field in PackSource, with an abstraction that does not leak the
implementation detail of comparing rank numbers.
Change-Id: I6757211397ab1bc181d61298e073f88b69dbefc3
Tests that even a commit with a huge message can be committed and read
if the WindowCache's streamFileThreshold is configured large enough.
Bug: 535092
Change-Id: Id8090c608625010caf11dff7971b47882b5fd20f
Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>
The canonical implementation also doesn't. Compare current
code in remote.c, function get_stale_heads_cb.[1] Not handling
symrefs in this case was introduced in canonical git in [2]
in 2008.
[1] https://github.com/git/git/blob/v2.17.0/remote.c#L2259
[2] https://github.com/git/git/commit/740fdd27f0
Bug: 533549
Change-Id: If348d56bb4a96b8aa7141f7e7b5a0d3dd4e7808b
Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>
Callers of getAllRefs that only iterate over the `values()` of the
returned map can be trivially fixed to call getRefDatabase().getRefs()
instead.
Only fix those where the calling method is already declared to throw
IOException, to avoid potential API changes.
Change-Id: I2b05f785077a1713953cfd42df7bf915f889f90b
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
Callers should use getRefDatabase().peel(ref) instead since it
doesn't swallow the IOException.
Adapt all trivial callers to user the alternative.
DescribeCommand still uses the deprecated method and is not adapted in
this change since it will require more refactoring to add handling of
the IOException.
Change-Id: I14d4a95a5e0570548753b9fc5c03d024dc3ff832
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
Make FileTreeIterator not enter ignored directories by default. We
only need to enter ignored directories if we do some operation against
git, and there is at least one tracked file underneath an ignored
directory.
Walking ignored directories should be avoided as much as possible as
it is a potential performance bottleneck. Some projects have a lot of
files or very deep hierarchies in ignored directories; walking those
may be costly (especially so on Windows). See for instance also bug
500106.
Provide a FileTreeIterator.setWalkIgnoredDirectories() operation to
force the iterator to iterate also through otherwise ignored
directories. Useful for tests (IgnoreNodeTest, CGitIgnoreTest), or
to implement things like "git ls-files --ignored".
Add tests in DirCacheCheckoutTest, and amend IndexDiffTest to test a
little bit more.
Bug: 388582
Change-Id: I6ff584a42c55a07120a4369fd308409431bdb94a
Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>
Callers should use getRefDatabase().getRefsByPrefix(R_TAGS) instead.
Adjust the tests accordingly.
Bug: 534731
Change-Id: Ib28ae365e42720268996ff46e34cae1745ad545c
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
Remove it from
* package private functions.
* try blocks
* for loops
this was done with the following python script:
$ cat f.py
import sys
import re
import os
def replaceFinal(m):
return m.group(1) + "(" + m.group(2).replace('final ', '') + ")"
methodDecl = re.compile(r"^([\t ]*[a-zA-Z_ ]+)\(([^)]*)\)")
def subst(fn):
input = open(fn)
os.rename(fn, fn + "~")
dest = open(fn, 'w')
for l in input:
l = methodDecl.sub(replaceFinal, l)
dest.write(l)
dest.close()
for root, dirs, files in os.walk(".", topdown=False):
for f in files:
if not f.endswith('.java'):
continue
full = os.path.join(root, f)
print full
subst(full)
Change-Id: If533a75a417594fc893e7c669d2c1f0f6caeb7ca
Signed-off-by: Han-Wen Nienhuys <hanwen@google.com>
Add support for the "ofs-delta" parameter in the "fetch" command in
the fetch-pack/upload-pack protocol v2.
Change-Id: I728cf986082fce4ddeb6a6435897692e15e60cc7
Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
Add support for the "include-tag" parameter in the "fetch" command in
the fetch-pack/upload-pack protocol v2.
In order to determine which tags to include, only objects pointed to by
refs starting with "refs/tags/" are checked. This restriction is for
performance reasons and to match the behavior of Git (see add_ref_tag()
in builtin/pack-objects.c).
Change-Id: I7d70aa09bcc8a525218ff1559e286c2a610258ca
Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
Signed-off-by: Jonathan Nieder <jrn@google.com>
This makes binary detection exact in ResolveMerger and DiffFormatter
This has the same intention as
Id4342a199628d9406bfa04af1b023c27a47d4014 but preserves backward
compatibility of the signature of RawParseUtils.lineMap.
Change-Id: Ia24a4e716592bab3363ae24e3a46315a7511154f
Signed-off-by: Han-Wen Nienhuys <hanwen@google.com>
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
It often fails on my machine, both in maven and bazel.
This patch marks the test flaky[1] in bazel so that "bazel test" can
run it a few times before declaring failure.
[1] https://docs.bazel.build/versions/master/be/common-definitions.html#test.flaky
Bug: 534285
Change-Id: Ibe5414fefbffe4e8f86af7047608d51cf5df5c47
Eclipse reports these as errors, so remove them.
Change-Id: Ic53d8003f9faef38fe776af5a73794e7bb1dfc49
Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
Add support for the "no-progress" parameter in the "fetch" command in
the fetch-pack/upload-pack protocol v2.
Change-Id: I6a6d6b1534f44845254b81d0e1f5c4ba2ac3d10b
Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
Add support for the "thin-pack" parameter in the "fetch" command in
the fetch-pack/upload-pack protocol v2.
Change-Id: I39a37b2b66a16929137d35c718a3acf2afb6b0b5
Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
Add basic support for the "fetch" command in the fetch-pack/upload-pack
protocol v2. This patch teaches "have" and "done".
The protocol specification (Documentation/technical/protocol-v2.txt in
the Git project) states:
want <oid>
Indicates to the server an object which the client wants to
retrieve. Wants can be anything and are not limited to
advertised objects.
It is unspecified whether the server should respect the
uploadpack.allowtipsha1inwant option etc. when serving packfiles. This
patch is conservative in that the server respects them.
Change-Id: I3dbec172239712ef9286a15b8407e86b87ea7863
Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
Add support for the "ref-prefix" parameter in the "ls-refs" command in
the fetch-pack/upload-pack protocol v2.
Change-Id: If9cf93b2646f75d50a11b5f482594f014d59a836
Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
Implement support for Git protocol v2's "ls-refs" command and its
"symrefs" and "peel" parameters.
This adds support for this command to UploadPack but the git://,
ssh://, and git:// transports do not make use of it yet. That will
have to wait for later patches.
Change-Id: I8abc6bcc6ed4a88c165677ff1245625aca01267b
Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
Signed-off-by: Jonathan Nieder <jrn@google.com>
Add initial support for protocol v2 of the fetch-pack/upload-pack
protocol. This protocol is described in the Git project in
"Documentation/technical/protocol-v2.txt".
This patch adds support for protocol v2 (without any capabilities) to
UploadPack. Adaptations of callers to make use of this support will
come in subsequent patches.
[jn: split from a larger patch; tweaked the API to make UploadPack
handle parsing the extra parameters and config instead of requiring
each caller to do such parsing]
Change-Id: I79399fa0dce533fdc8c1dbb6756748818cee45b0
Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
Signed-off-by: Jonathan Nieder <jrn@google.com>
Most pkt-lines (data-pkts) have the form
pkt-len pkt-payload
where pkt-len is a string of 4 hexadecimal digits representing the
size in bytes of the pkt-line. Since this size includes the size of
the pkt-len, no data-pkt has a length less than 4.
A pkt-line with a length field less than 4 can thus be used for
other purposes. In Git protocol v1, the only such pkt-line was
flush-pkt = "0000"
which was used to mark the end of a stream. Protocol v2 (see
Documentation/technical/protocol-v2.txt in git.git) introduces a
second special pkt-line type:
delim-pkt = "0001"
used to mark the end of a section within a stream, for example to
separate capabilities from the content of a command.
[jn: split out from a larger patch that made use of this support]
Change-Id: I10e7824fa24ed74c4f45624bd490bba978cf5c34
Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
Signed-off-by: Jonathan Nieder <jrn@google.com>
The existing RefDatabase#getRefs abstract method (to be implemented by
ref database backends) has the following issues:
- It returns a map with a key (the name of the ref with the prefix
removed) which is potentially superfluous (it can be derived by the
caller if need be) and confusing (in that the prefix is removed).
- The prefix is required to end with a '/', but some backends (e.g.
reftable) have fast search by prefix regardless of what the last
character of the prefix is.
Add a new method #getRefsByPrefix that does not have these issues. This
is non-abstract with a default implementation that uses #getRefs (for
backwards compatibility), but ref database backends can reimplement it.
This also prepares for supporting "ref-prefix" in the "ls-refs" command
in the fetch-pack/upload-pack protocol v2, which does not require that
the prefix end with a '/'.
Change-Id: I4c92f852e8c1558095dd460b5fd7b602c1d82df1
Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
Signed-off-by: Jonathan Nieder <jrn@google.com>
Otherwise successful, non-conflicting merges will never get a
Gerrit Change-Id.
Bug: 358206
Change-Id: I9b599ad01d9f7332200c1d81a1ba6ce5ef990ab5
Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>
Various places on the client side of the push were creating unordered
maps and sets of ref names, resulting in ReceivePack processing commands
in an order other than what the client provided. This is normally not
problematic for clients, who don't typically care about the order in
which ref updates are applied to the storage layer.
However, it does make it difficult to write deterministic tests of
ReceivePack or hooks whose output depends on the order in which commands
are processed, for example if informational per-ref messages are written
to a sideband.[1]
Add a test that ensures the ordering of commands both internally in
ReceivePack and in the output PushResult.
[1] Real-world example:
https://gerrit-review.googlesource.com/c/gerrit/+/171871/1/javatests/com/google/gerrit/acceptance/git/PushPermissionsIT.java#149
Change-Id: I7f1254b4ebf202d4dcfc8e59d7120427542d0d9e
Previously @ was allowed e.g. in branch names, but not as the last
character. The case that @ is the last character was not handled.
Change-Id: Ic33870b22236f7a5ec7b54007f1b0cefd9354bfb
Getting attributes of files on Windows is an expensive operation.
Windows stores file attributes in the directory, so they are
basically available "for free" when a directory is listed. The
implementation of Java's Files.walkFileTree() takes advantage of
that (at least in the OpenJDK implementation for Windows) and
provides the attributes from the directory to a FileVisitor.
Using Files.walkFileTree() with a maximum depth of 1 is thus a
good approach on Windows to get both the file names and the
attributes in one go.
In my tests, this gives a significant speed-up of FileTreeIterator
over the "normal" way: using File.listFiles() and then reading the
attributes of each file individually. The speed-up is hard to
quantify exactly, but in my tests I've observed consistently 30-40%
for staging 500 files one after another, each individually, and up
to 50% for individual TreeWalks with a FileTreeIterator.
On Unix, this technique is detrimental. Unix stores file attributes
differently, and getting attributes of individual files is not costly.
On Unix, the old way of doing a listFiles() and getting individual
attributes (both native operations) is about three times faster than
using walkFileTree, which is implemented in Java.
Therefore, move the operation to FS/FS_Win32 and call it from
FileTreeIterator, so that we can have different implementations
depending on the file system.
A little performance test program is included as a JUnit test (to be
run manually).
While this does speed up things on Windows, it doesn't solve the basic
problem of bug 532300: the iterator always gets the full directory
listing and the attributes of all files, and the more files there are
the longer that takes.
Bug: 532300
Change-Id: Ic5facb871c725256c2324b0d97b95e6efc33282a
Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>
The class names imply that RecursiveMergerTest tests the RecursiveMerger
and ResolveMergerTest tests the ResolveMerger.
In fact, both of them include coverage of both strategies; the difference
is that RecursiveMergerTest is only testing criss-cross merges.
The tests cannot be combined into a single class because the criss-cross
test methods have additional data points.
Instead, rename the classes to more meaningful names.
Change-Id: I7ca8a03a3b7e351e2d4fcaca3b3186c098a3ca66
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
Define strategiesUnderTest as an array of MergeStrategy using the
@DataPoints annotation, rather than two separate variables each
annotated as @DataPoint.
This makes the implementation consistent with RecursiveMergerTest.
Change-Id: I9f1d525b38cb59634ba054c7779dc4af1fc46e25
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
The local variable 'remote' hides the class scope variable
of the same name.
Change-Id: I7410c33678677ce2a14691772466d91e8139e3fa
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
* stable-4.11:
ObjectIdSerializer: Support serialization of known non-null ObjectId
Change-Id: Ie430fa2c5d13ae698d884a37d0d03884ebbf25ec
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
Teach UploadPack to advertise the filter capability and support a
"filter" line in the request, accepting blob sizes only, if the
configuration variable "uploadpack.allowfilter" is true. This feature is
currently in the "master" branch of Git, and as of the time of writing,
this feature is to be released in Git 2.17.
This is incomplete in that the filter-by-sparse-specification feature
also supported by Git is not included in this patch.
If a JGit server were to be patched with this commit, and a repository
on that server configured with RequestPolicy.ANY or
RequestPolicy.REACHABLE_COMMIT_TIP, a Git client built from the "master"
branch would be able to perform a partial clone.
Change-Id: If72b4b422c06ab432137e9e5272d353b14b73259
Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
The implementation of ObjectIdSerializer, added in change I7599cf8bd,
is not equivalent to the original implementation in Gerrit [1].
The Gerrit implementation provides separate methods to (de)serialize
instances of ObjectId that are known to be non-null. In these methods,
no "marker" is written to the stream. Replacing Gerrit's implementation
with ObjectIdSerializer [2] broke persistent caches because it started
writing markers where they were not expected [3].
Since ObjectIdSerializer is included in JGit 4.11 we can't change the
existing #write and #read methods. Keep those as-is, but extend the
Javadoc to clarify that they support possibly null ObjectId instances.
Add new methods #writeWithoutMarker and #readWithoutMarker to support
the cases where the ObjectId is known to be non-null and the marker
should not be written to the serialization stream.
Also:
- Replace the hard-coded `0` and `1` markers with constants that can
be linked from the Javadocs.
- Include the marker value in the "Invalid flag before ObjectId"
exception message.
[1] https://gerrit-review.googlesource.com/c/gerrit/+/9792
[2] https://gerrit-review.googlesource.com/c/gerrit/+/165851
[3] https://gerrit-review.googlesource.com/c/gerrit/+/165952
Change-Id: Iaf84c3ec32ecf83efffb306fdb4940cc85740f3f
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
In most of the tests, the temporary buffer is explicitly destroyed in
a finally block after being closed. This is not possible if using the
try-with-resource construct, because the variable is not accessible in
the finally block scope.
Change-Id: I3bab30695ddd12e1a0ae107989638428fe3ef551
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
When an auto-closeable resources is not opened in try-with-resource,
the warning "should be managed by try-with-resource" is emitted by
Eclipse.
Fix the ones that can be silenced simply by moving the declaration of
the variable into a try-with-resource.
In cases where we explicitly call the close() method, for example in
tests where we are testing specific behavior caused by the close(),
suppress the warning.
Leave the ones that will require more significant refcactoring to fix.
They can be done in separate commits that can be reviewed and tested
in isolation.
Change-Id: I9682cd20fb15167d3c7f9027cecdc82bc50b83c4
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
Parameter negateFirstMatch is not honored anymore
Change-Id: Idff1a92643c1431c7e34a7730f8414135e1ac196
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
Matthias Sohn