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>
The WorkingTreeSource produced an ObjectLoader that returned
inconsistent sizes: the file size in getSize(), but then a
correctly filtered smaller stream in openStream(). This resulted
either in an IOE "short read of block" or in an EOFException
depending on the resulting filtered size.
Fix this by ensuring that getSize() does return the size of the
filtered stream.
Bug: 530106
Change-Id: I7c7c85036047dc10030ed29c1d5a6c7f34f2bdff
Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>
Replace hard-coded "UTF-8" string with the constant.
Change-Id: Ie812add2df28e984090563ec7c6e2c0366616424
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
Respect merge=lfs and diff=lfs attributes where required to replace (in
memory) the content of LFS pointers with the actual blob content from
the LFS storage (and vice versa when staging/merging).
Does not implement general support for merge/diff attributes for any
other use case apart from LFS.
Change-Id: Ibad8875de1e0bee8fe3a1dffb1add93111534cae
Signed-off-by: Markus Duft <markus.duft@ssi-schaefer.com>
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
This is based on the ObjectIdSerialization class written by Shawn Pearce
for the Gerrit Code Review project in 2009 [1]. As mentioned in the
commit message there, it should be part of core JGit.
This implementation is slightly different to Shawn's version. Rather
than having separate methods for null/non-null ids, single methods are
implemented with @Nullable annotations.
[1] https://gerrit-review.googlesource.com/c/gerrit/+/9792
Change-Id: I7599cf8bd1ecd546e2252783d6d672eb76804060
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
Processing of negated rules, like !bin/ was not working correctly: they
were interpreted too broad, resulting in unexpected untracked files
which should actually be ignored
Bug: 409664
Change-Id: I0a422fd6607941461bf2175c9105a0311612efa0
Signed-off-by: Marc Strapetz <marc.strapetz@syntevo.com>
**/ should match only directories, but not files
Change-Id: I885c83e5912cac5bff338ba657faf6bb9ec94064
Signed-off-by: Marc Strapetz <marc.strapetz@syntevo.com>
* changes:
RepoCommand: generate relative submodule URLs from absolute URLs.
RepoCommand: don't record new commit if tree did not change
RepoCommand: persist unreadable submodules in .gitmodules
If a manifest file specifies an absolute URL on the same host on which
the superproject resides, rewrite the URLs to be relative.
Signed-off-by: Han-Wen Nienhuys <hanwen@google.com>
Change-Id: Id616611e5195998fb665c8e7806539a3a02e219a
In cases where a manifest file mixes different remotes, a Gerrit
server process may not have access to all remotes, and won't be able
to produce a full submodule tree.
Preserving this information in .gitmodules will let downstream clients
reconstruct the full tree.
Signed-off-by: Han-Wen Nienhuys <hanwen@google.com>
Change-Id: I52f5d3f288e771dca0af2b4dd3f3fa0f940dcf15
* stable-4.10:
Fix ssh host name handling for Jsch
Jsch overrides the port in the URI with the one in ~/.ssh/config
Change-Id: I860fc61ceb12ae792b1ee7421046ecd32373b9f8
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
* stable-4.9:
Fix ssh host name handling for Jsch
Jsch overrides the port in the URI with the one in ~/.ssh/config
Change-Id: Iff9076f65e767bbe8df016337b631bdaeb40ad98
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
If we give Jsch access to the ssh config file, we must _not_ resolve
the host name from the alias. Instead we must give the alias (i.e.,
the host name as is in the URI) to Jsch, so that it finds the same
ssh config entry.
Otherwise if the hostname in the URI, which is taken as an alias in
ssh config ("Host" line), is unequal to the "Hostname" line, and
there happens to be another ssh config entry with that translated
host name as alias, Jsch will pick up that second entry, and we end
up with a strange mixture of both.
Add tests for this case.
Bug: 531118
Change-Id: I249d8c073b0190ed110a69dca5b9be2a749822c3
Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>
Jsch unconditionally overwrites the port from the ssh config
file (if a port is specified there), even if the URI explicitly does
give a different port.
Fix this, and add tests.
Change-Id: I7b014543c7ece26270e366db39d7647f82d64f0d
Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>
The InitCommand returns a Git that is instantiated with the newly
created Repository, but the Repository is not closed with the Git
resulting in resource leaks.
Create the Git with `closeRepo` set to true, such that the Repository
is also closed when the Git is closed.
Adjust the tests to use try-with-resource on the Git instance.
Change-Id: Ib26e7428c7d8840956d1edb09e53b93e23e6fe5a
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
Android an Chrome have several repos with >300k refs. We sometimes see
negotiations of >100k rounds. This change provides a "minimal negotiation"
feature on the client side that limits how many "have" lines the client
sends. The client extracts the current SHA-1 values for the refs in its
wants set, and terminates negotiation early when all of those values have
been sent as haves. If a new branch is being fetched then that set will
be empty and the client will terminate after current default minimum
of two rounds.
This feature is gated behind a "fetch.useminimalnegotiation" configuration
flag, which defaults to false.
Change-Id: Ib12b095cac76a59da6e8f72773c4129e3b32ff2b
Signed-off-by: Terry Parker <tparker@google.com>
This would allow compact and GC process to clean up duplicate ref names in the reftables.
Change-Id: I2b9df0bf72dba63cc3525e374982e60559a776c2
Signed-off-by: Minh Thai <mthai@google.com>
When CleanCommand is collecting the files and folders to be deleted
it may happen that the list of directories contains obsolete entries.
E.g. a folder and its parent folder may be in the list. Only the
parent folder would be sufficient.
This was a reason for hitting FileNotFoundExceptions when finally
trying to delete the files and folders. Improve CleanCommand
to ignore files to be deleted which are already gone.
Bug: 514434
Change-Id: I10caa01bfb9cec5967dfdaea50c6e4a713eeeabd
Signed-off-by: Matthias Sohn <matthias.sohn@sap.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
This doesn't handle the really hard thing, which is merging spurious
conflicts inside .gitmodules files. That's OK: git.git doesn't
either. Users can resolve the conflict themselves and then commit
the merge.
Previously, jgit would crash when attempting to merge conflicting
submodule changes. Even if there was no conflict, after a merge which
adds submodules, the repository would have been missing empty
directories for newly-added submodules.
This patch fixes the crash, and adds the empty directories where
necessary. It ensures that the index is in a conflicted state when
submodule changes conflict.
Reported-by: Alexey Korobkov
Bug: 494551
Change-Id: I79db6798c2bdcc1159b5b2589b02da198dc906a1
Signed-off-by: David Turner <dturner@twosigma.com>
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
Commit fc7d407 corrected line endings for working tree files resulting
from merges when CRLF translations are to be done. However, that also
resulted in the file content being put as-is into the index, which is
wrong. The index must contain the file content with reverse CRLF
translations applied.
With core.autocrlf=true, the working tree file should have CR-LF, but
the index blob must still contain only LF.
Fix this oversight and apply the inverse translation when updating the
index, similar to what is done in AddCommand.
Bug: 499615
Change-Id: I3a33931318bdb580b2390f3450f91ea8f258a6a4
Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>
Consistently require 1.7.0. We ship 1.7.2 with our p2 repository but
there is no need to require 1.7.2 since it should be API compatible with
1.7.0.
Change-Id: I8467bb14316cb24daa79e89275332107d2716190
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
Merges are performed using the raw text as stored in the git
repository. When we write the merge result, we must apply the
correct CRLF settings. Otherwise the line endings in the result
will be wrong.
Bug: 499615
Change-Id: I37a9b987e9404c97645d2720cd1c7c04c076a96b
Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
* Section and key names in git config files are case-insensitive.
* If an include directive is invalid, include the line in the
exception message.
* If inclusion of the included file fails, put the file name into
the exception message so that the user knows in which file the
problem is.
Change-Id: If920943af7ff93f5321b3d315dfec5222091256c
Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>
Jsch caches keys (aka identities) specified in ~/.ssh/config via
IndentityFile only for the current Jsch Session. This results in
multiple password prompts for successive sessions.
Do the handling of IdentityFile exclusively in JGit, as it was before
4.9. JGit uses different Jsch instances per host and caches the
IdentityFile there, allowing it to be re-used in different sessions
for the same host.
* Add comments to explain this.
* Move the JschBugFixingConfig from OpenSshConfig to
JschConfigSessionFactory to have all these Jsch work-arounds
in one place.
* Make that config hide the IdentityFile config from Jsch to avoid
that Jsch overrides the JGit behavior.
Bug: 529173
Change-Id: Ib36c34a2921ba736adeb64de71323c2b91151613
Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>
Instead of instantiating a new Random on each invocation of newLargeBlob,
create it once and reuse it.
This fixes a warning raised by Spotbugs about the Random object being
created and only used once.
Change-Id: I5b8e6ccbbc92641811537808aed9eae2034c1133
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
Change-Id: Iaaefc2cbafbf083d6ab158b1c378ec69cc76d282
Signed-off-by: David Turner <dturner@twosigma.com>
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
* stable-4.9:
Strings#convertGlob: fix escaping of patterns like [\[].
Change-Id: I18d55537002b3153db35f8a6b60f2f5317d17248
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
Originally the patterns were escaped twice leading
to wrong matching results.
Bug: 528886
Change-Id: I26e201b4b0ef51cac08f940b76f381260fa925ca
Signed-off-by: Dmitry Pavlenko <pavlenko@tmatesoft.com>
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
These are ignored by C git when parsing:
$ git config -f - --list <<EOF
[foo "x\0y"]
bar = baz
[foo "x\qy"]
bar = baz
[foo "x\by"]
bar = baz
[foo "x\ny"]
bar = baz
[foo "x\ty"]
bar = baz
EOF
foo.x0y.bar=baz
foo.xqy.bar=baz
foo.xby.bar=baz
foo.xny.bar=baz
foo.xty.bar=baz
This behavior is different from value parsing, where an invalid escape
sequence is an error (which JGit already does as well):
$ git config -f - --list <<EOF
[foo]
bar = x\qy
EOF
fatal: bad config line 2 in standard input
Change-Id: Ifd40129b37d9a62df3d886d8d7e22f766f54e9d1
* stable-4.9:
PackInserter: Ensure objects are written at the end of the pack
ObjectInserter: Add warning about mixing read-back with writes
Change-Id: I308e7c1c6b72e8d4d9b5d0f4f51e9815fc92d7d7
When interleaving reads and writes from an unflushed pack, we forgot to
reset the file pointer back to the end of the file before writing more
new objects. This had at least two unfortunate effects:
* The pack data was potentially corrupt, since we could overwrite
previous portions of the file willy-nilly.
* The CountingOutputStream would report more bytes read than the size
of the file, which stored the wrong PackedObjectInfo, which would
cause EOFs during reading.
We already had a test in PackInserterTest which was supposed to catch
bugs like this, by interleaving reads and writes. Unfortunately, it
didn't catch the bug, since as an implementation detail we always read a
full buffer's worth of data from the file when inflating during
readback. If the size of the file was less than the offset of the object
we were reading back plus one buffer (8192 bytes), we would completely
accidentally end up back in the right place in the file.
So, add another test for this case where we read back a small object
positioned before a large object. Before the fix, this test exhibited
exactly the "Unexpected EOF" error reported at crbug.com/gerrit/7668.
Change-Id: I74f08f3d5d9046781d59e5bd7c84916ff8225c3b
Previously, Config was using the same method for both escaping and
parsing subsection names and config values. The goal was presumably code
savings, but unfortunately, these two pieces of the git config format
are simply different.
In git v2.15.1, Documentation/config.txt says the following about
subsection names:
"Subsection names are case sensitive and can contain any characters
except newline (doublequote `"` and backslash can be included by
escaping them as `\"` and `\\`, respectively). Section headers cannot
span multiple lines. Variables may belong directly to a section or to
a given subsection."
And, later in the same documentation section, about values:
"A line that defines a value can be continued to the next line by
ending it with a `\`; the backquote and the end-of-line are stripped.
Leading whitespaces after 'name =', the remainder of the line after
the first comment character '#' or ';', and trailing whitespaces of
the line are discarded unless they are enclosed in double quotes.
Internal whitespaces within the value are retained verbatim.
Inside double quotes, double quote `"` and backslash `\` characters
must be escaped: use `\"` for `"` and `\\` for `\`.
The following escape sequences (beside `\"` and `\\`) are recognized:
`\n` for newline character (NL), `\t` for horizontal tabulation (HT,
TAB) and `\b` for backspace (BS). Other char escape sequences
(including octal escape sequences) are invalid."
The main important differences are that subsection names have a limited
set of supported escape sequences, and do not support newlines at all,
either escaped or unescaped. Arguably, it would be easy to support
escaped newlines, but C git simply does not:
$ git config -f foo.config $'foo.bar\nbaz.quux' value
error: invalid key (newline): foo.bar
baz.quux
I468106ac was an attempt to fix one bug in escapeValue, around leading
whitespace, without having to rewrite the whole escaping/parsing code.
Unfortunately, because escapeValue was used for escaping subsection
names as well, this made it possible to write invalid config files, any
time Config#toText is called with a subsection name with trailing
whitespace, like {foo }.
Rather than pile hacks on top of hacks, fix it for real by largely
rewriting the escaping and parsing code.
In addition to fixing escape sequences, fix (and write tests for) a few
more issues in the old implementation:
* Now that we can properly parse it, always emit newlines as "\n" from
escapeValue, rather than the weird (but still supported) syntax with a
non-quoted trailing literal "\n\" before the newline. In addition to
producing more readable output and matching the behavior of C git,
this makes the escaping code much simpler.
* Disallow '\0' entirely within both subsection names and values, since
due to Unix command line argument conventions it is impossible to pass
such values to "git config".
* Properly preserve intra-value whitespace when parsing, rather than
collapsing it all to a single space.
Change-Id: I304f626b9d0ad1592c4e4e449a11b136c0f8b3e3
It was already increased in 61a943e and 661232b but is still not
enough to take into account snapshot versions that are 100 or more
commits ahead of tag, i.e. 4.9.2.201712150930-r.105-gc1d37ca27
Change-Id: Ibeff73adae06b92fe5bb9c5eced9e4c6a08c437c
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
The Config class must be safe to run against untrusted input files.
Reading arbitrary local system paths using include.path is risky for
servers, including Gerrit Code Review.
This was fixed on master [1] by making "readIncludedConfig" a noop
by default. This allows only FileBasedConfig, which originated from
local disk, to read local system paths.
However, the "readIncludedConfig" method was only introduced in [2]
which was needed by [3], both of which are only on the master branch.
On the stable branch only Config supports includes. Therefore this
commit simply disables the include functionality.
[1] https://git.eclipse.org/r/#/c/113371/
[2] https://git.eclipse.org/r/#/c/111847/
[3] https://git.eclipse.org/r/#/c/111848/
Bug: 528781
Change-Id: I9a3be3f1d07c4b6772bff535a2556e699a61381c
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
The Config class must be safe to run against untrusted input files.
Reading arbitrary local system paths using include.path is risky for
servers, including Gerrit Code Review. Return null by default to
incide the include should be ignored.
Only FileBasedConfig which originated from local disk should be trying
to read local system paths. FileBasedConfig already overrides this
method with its own implementation.
Change-Id: I2ff31753868aa1bbac4a6843a4c23e50bd6f46f3
Path.getFileName() may return null if the path has zero elements.
Enclose the dereference in a null-check.
Change-Id: I7ea3d3f07edc13a80b593d28e8fd512a4e1ed56b
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
There are several different version ranges specified in the various
manifest files.
Align them all to the same range: [4.12,5.0.0)
Change-Id: I02205b8b8546c9f53ed431b5fd9abf6ddcda4423
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
Instead of hard-coding the charset strings "US-ASCII", "UTF-8", and
"ISO-8859-1", use the corresponding constants from StandardCharsets.
UnsupportedEncodingException is not thrown when the StandardCharset
constants are used, so remove the now redundant handling.
Because the encoding names are no longer hard-coded strings, also
remove redundant $NON-NLS warning suppressions.
Also replace existing usages of the constants with static imports.
Change-Id: I0a4510d3d992db5e277f009a41434276f95bda4e
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
ConfigTest#pathToString is not visible to FileBasedConfigTest when
bulding with bazel.
Move it to FileUtils rather than messing about with the bazel build
rules to make it visible.
Change-Id: Idcfd4822699dac9dc4a426088a929a9cd31bf53f
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
Relative include.path are now resolved against the config's parent
directory. include.path starting with ~/ are resolved against the
user's home directory
Change-Id: I91911ef404126618b1ddd3589294824a0ad919e6
Signed-off-by: Marc Strapetz <marc.strapetz@syntevo.com>
Change-Id: I37a2ef611aef97faf1b891a9660c1745435a915d
Signed-off-by: Marc Strapetz <marc.strapetz@syntevo.com>
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
* stable-4.9:
GC: Delete stale temporary packs and indexes
Change-Id: I49b37845ee8a465404b801a2d8de0205a2e7ba30
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
When a GC operation is interrupted, temporary packs and indexes can be
left on the pack folder. In big, busy repositories this can lead to
significant amounts of wasted disk space if this interruption is done
with a certain frequency.
Remove stale temporary packs and indexes at the end of the GC process so
they do not accumulate. To avoid interfering with a possible concurrent
JGit GC process in the same repository, only delete temporary files that
are older than one day.
Change-Id: If9b6c1e57fac8a6a0ecc0a703089634caba4caae
Signed-off-by: Hector Caballero <hector.caballero@ericsson.com>
* stable-4.9:
Ignore warning for minor version change without API change
Silence boxing warning
Prepare 4.5.5-SNAPSHOT builds
JGit v4.5.4.201711221230-r
Fix LockFile semantics when running on NFS
Honor trustFolderStats also when reading packed-refs
Prepare 4.5.4-SNAPSHOT builds
JGit v4.5.3.201708160445-r
Change-Id: Icc33d2e36f140e8714fce088379673a8834ae9de
- this is a new warning option in Eclipse 4.7 and higher
- we always change version of all bundles in a release to keep release
engineering simple
Change-Id: Ic7523d77b67b2802f1bab3bc70af250d712a034f
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
* stable-4.9:
Yet another work-around for a Jsch bug: timeouts
Change-Id: I7cf227c62a3c06f91cee1a6c61719b6fe50da883
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
Jsch 0.1.54 passes on the values from ~/.ssh/config for
"ServerAliveInterval" and "ConnectTimeout" as read from
the config file to java.net.Socket.setSoTimeout(). That
method expects milliseconds, but the values in the config
file are seconds!
The missing conversion in Jsch means that the timeout is
set way too low, and if the server doesn't respond within
that very short time frame, Jsch kills the connection and
then throws an exception with a message such as "session is
down" or "timeout in waiting for rekeying process".
As a work-around, do the conversion to milliseconds in the
Jsch-facing Config interface of OpenSshConfig. That way Jsch
already gets these values as milliseconds.
Bug: 526867
Change-Id: Ibc9b93f7722fffe10f3e770dfe7fdabfb3b97e74
Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>
A tombstone will prevent a delayed reference update from resurrecting the
deleted reference.
Change-Id: Id9f4df43d435a299ff16cef614821439edef9b11
Signed-off-by: Minh Thai <mthai@google.com>
Do not use 0 as the unset value for minUpdateIndex, as input reftables
may have minUpdateIndex starting at 0.
Change-Id: Ie040a6b73d4a5eba5521e51d0ee4580713c84a3e
Signed-off-by: Minh Thai <mthai@google.com>
So far, in order to get the pack directory it was necessary to resolve
it from the object directory. This resolution is already done when
creating the object directory, so simplify the call by just adding a
getter to the pack directory.
Change-Id: I69e783141dc6739024e8b3d5acc30843edd651a7
Signed-off-by: Hector Caballero <hector.caballero@ericsson.com>
When invoking File.toPath(), an (unchecked) InvalidPathException may be
thrown which should be converted to a checked IOException.
For now, we will replace File.toPath() by FileUtils.toPath() only for
code which can already handle IOExceptions.
Change-Id: I0f0c5fd2a11739e7a02071adae9a5550985d4df6
Signed-off-by: Marc Strapetz <marc.strapetz@syntevo.com>
* changes:
BitmapWalker: do not revisit objects in bitmap
Use bitmaps for non-commit reachability checks
Make PackWriterBitmapWalker public
UploadPackTest: construct commits in test method
Currently, unless RequestPolicy#ANY is used, UploadPack rejects all
non-commit "want" lines unless they were advertized. This is fine,
except when "uploadpack.allowreachablesha1inwant" is true
(corresponding to RequestPolicy#REACHABLE_COMMIT), in which case one
would expect that "want"-ing anything reachable would work.
(There is no restriction that "want" lines must only contain commits -
it is allowed for refs to directly point to trees and blobs, and
requesting for them using "want" lines works.)
This commit has been written to avoid performance regressions as much
as possible. In the usual (and currently working) case where the only
unadvertized things requested are commits, we do a standard RevWalk in
order to avoid incurring the cost of loading bitmaps. However, if
unadvertized non-commits are requested, bitmaps are used instead, and
if there are no bitmaps, a WantNotValidException is thrown (as is
currently done).
Change-Id: I68ed4abd0e477ff415c696c7544ccaa234df7f99
Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
JGit's delta handling code requires the target to be a single byte
array. Any attempt to inflate a delta larger than fits in the 2GiB
limit will fail with some form of array index exceptions. Check for
this overflow early and abort pack parsing.
Change-Id: I5bb3a71f1e4f4e0e89b8a177c7019a74ee6194da
This new warning was introduced in Eclipse 4.7 Oxygen [1].
The only instances of the warning are in test code that is asserting
that some class does not compare equal to Strings. As in the Gerrit
project [2] these asserts are arguably overkill, but arguably also
a reasonable test of an equals implementation. Ignore the warning in
these cases.
Note that if the project is opened in an earlier version of Eclipse,
a warning "Unsupported @SuppressWarnings" will be emitted.
[1] https://www.eclipse.org/eclipse/news/4.7/M6/
[2] https://gerrit-review.googlesource.com/#/c/gerrit/+/110339/
Change-Id: I08ea33d71e6009cf0f37e6492a475931f447256b
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
In a subsequent commit, more tests will be added. This commit allows
those tests to reuse fields.
Change-Id: Icbd17d158cfe3ba4dacbd8a11a67f9e7607b41b3
Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
* stable-4.9:
PackInserter: Implement newReader()
Move some strings from DfsText to JGitText
FileRepository: Add pack-based inserter implementation
ObjectDirectory: Factor a method to close open pack handles
ObjectDirectory: Remove last modified check in insertPack
Change-Id: Ifc9ed6f5d8336bc978818a64eae122bceb933e5d
Applications that use ObjectInserters to create lots of individual
objects may prefer to avoid cluttering up the object directory with
loose objects. Add a specialized inserter implementation that produces a
single pack file no matter how many objects. This inserter is loosely
based on the existing DfsInserter implementation, but is simpler since
we don't need to buffer blocks in memory before writing to storage.
An alternative for such applications would be to write out the loose
objects and then repack just those objects later. This operation is not
currently supported with the GC class, which always repacks existing
packs when compacting loose objects. This in turn requires more
CPU-intensive reachability checks and extra I/O to copy objects from old
packs to new packs.
So, the choice was between implementing a new variant of repack, or not
writing loose objects in the first place. The latter approach is likely
less code overall, and avoids unnecessary I/O at runtime.
The current implementation does not yet support newReader() for reading
back objects.
Change-Id: I2074418f4e65853b7113de5eaced3a6b037d1a17
This makes detection of binaries exact for ResolveMerger and
DiffFormatter: they will classify files as binary regardless of where
the '\0' occurs in the text.
Signed-off-by: Han-Wen Nienhuys <hanwen@google.com>
Change-Id: Id4342a199628d9406bfa04af1b023c27a47d4014
This method creates a RawText from a blob, but avoids reading the blob
if the start contains null bytes. This should reduce the amount of
garbage that Gerrit produces for changes with binaries.
Signed-off-by: Han-Wen Nienhuys <hanwen@google.com>
Change-Id: Idd202d20251f2d1653e5f1ca374fe644c2cf205f
* stable-4.9:
Avoid bad rounding "1 year, 12 months" in date formatter
Ensure that ~ in ssh config is replaced before Jsch sees it
Change-Id: If6ca55f9447aaea3d7c2d36c03520d5e6dd5193e
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
Round first, then calculate the labels. This avoids "x years, 12 months"
and instead produces "x+1 years".
One test case has been added for the original example the bug was found
with, and one assertion has been moved from an existing test case to the
new test case, since it also triggered the bug.
Bug: 525907
Change-Id: I3270af3850c4fb7bae9123a0a6582f93055c9780
Signed-off-by: Michael Keppler <Michael.Keppler@gmx.de>
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
This ensure DfsReftableDatabase is tested by the same test suites that
use/test InMemoryRepository. It also simplifies the logic of
InMemoryRepository and brings its compatibility story closer to any
other DFS repository that uses reftables for its reference storage.
Change-Id: I881469fd77ed11a9239b477633510b8c482a19ca
Signed-off-by: Minh Thai <mthai@google.com>
Signed-off-by: Terry Parker <tparker@google.com>
Do tilde replacement for values from the ssh config file that are
file names in all cases to make sure that they are already replaced
when Jsch tries to get the values.
Previously, OpenSshConfig did tilde replacement only for the
IdentityFile in the JGit-facing "Host" interface and left the
replacement in the Jsch-facing "Config" interface to Jsch.
But on Windows the JGit notion of what should be used to replace the
tilde differs from Jsch's replacement. Jsch always replaces the tilde
by the value of the system property "user.home", whereas JGit also
considers some environment variables like %HOME%. This can lead to
rather surprising failures as in the case of bug 526175 where
%HOME% != user.home.
Prior to commit 9d24470 (i.e.,prior to JGit 4.9.0) this problem never
occurred because Jsch was completely unaware of the ssh config file
and all host and IdentityFile handling happened exclusively in JGit.
Bug: 526175
Change-Id: I1511699664ffea07cb58ed751cfdb79b15e3a99e
Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>
Allow creating symbolic references with link, and deleting them or
switching to ObjectId with unlink. How this happens is up to the
individual RefDatabase.
The default implementation detaches RefUpdate if a symbolic reference
is involved, supporting these command instances on RefDirectory.
Unfortunately the packed-refs file does not support storing symrefs,
so atomic transactions involving more than one symref command are
failed early.
Updating InMemoryRepository is deferred until reftable lands, as I
plan to switch InMemoryRepository to use reftable for its internal
storage representation.
Change-Id: Ibcae068b17a2fc6d958f767f402a570ad88d9151
Signed-off-by: Minh Thai <mthai@google.com>
Signed-off-by: Terry Parker <tparker@google.com>
Round first, then calculate the labels. This avoids "x years, 12 months"
and instead produces "x+1 years".
One test case has been added for the original example the bug was found
with, and one assertion has been moved from an existing test case to the
new test case, since it also triggered the bug.
Bug: 525907
Change-Id: I3270af3850c4fb7bae9123a0a6582f93055c9780
Signed-off-by: Michael Keppler <Michael.Keppler@gmx.de>
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
Shadow commits in the RevWalk in the UploadPack object may cause the
UNINTERESTING flag not being carried over to their parents commits since
they were marked NO_PARENTS during the assumeShallow or
initializeShallowCommits call.
A new RevWalk needs to be created for this reason, but instead of
creating a new RevWalk from Repository, we can reuse the ObjectReader in
the RevWalk of UploadPack to load objects.
Change-Id: Ic3fee0512d35b4f555c60e696a880f8b192e4439
Signed-off-by: Zhen Chen <czhen@google.com>
Per git-config(1), core.logAllRefUpdates auto-creates reflogs for HEAD
and for refs under heads, notes, tags, and for HEAD. Add notes and
remove stash from ReflogWriter#shouldAutoCreateLog. Explicitly force
writing reflogs for refs/stash at call sites, now that this is
supported.
Change-Id: I3a46d2c2703b7c243e0ee2bbf6948279800c485c
Even if a repository has core.logAllRefUpdates=true, ReflogWriter does
not create reflog files unless the refs are under a hard-coded list of
prefixes, or unless the forceWrite bit is set. Expose the forceWrite bit
on a per-update basis in RefUpdate/BatchRefUpdate/ReceiveCommand,
creating RefLogWriters as necessary.
Change-Id: Ifc851fba00f76bf56d4134f821d0576b37810f80
The ReflogWriter constructor just took a Repository and called
getDirectory() on it to figure out the reflog dirs, but not all
Repository instances use this storage format for reflogs, so it's
incorrect to attempt to use ReflogWriter when there is not a
RefDirectory directly involved. In practice, ReflogWriter was mostly
only used by the implementation of RefDirectory, so enforcing this is
mostly just shuffling around calls in the same internal package.
The one exception is StashDropCommand, which writes to a reflog lock
file directly. This was a reasonable implementation decision, because
there is no general reflog interface in JGit beyond using
(Batch)RefUpdate to write new entries to the reflog. So to implement
"git stash drop <N>", which removes an arbitrary element from the
reflog, it's fair to fall back to the RefDirectory implementation.
Creating and using a more general interface is well beyond the scope of
this change.
That said, the old behavior of writing out the reflog file even if
that's not the reflog format used by the given Repository is clearly
wrong. Fail fast in this case instead.
Change-Id: I9bd4b047bc3e28a5607fd346ec2400dde9151730
This test was never being run. Since it was introduced it was
named "notest.." which meant it didn't run with JUnit3, and
since it is not annotated @Test it also doesn't run with JUnit4.
When compiling with Bazel 0.6.0, error-prone raises an error
that the public method is not annotated with @Ignore or @Test.
Given that the test has never been run anyway, we can just
remove it.
Bug: 525415
Change-Id: Ie9a54f89fe42e0c201f547ff54ff1d419ce37864
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
Git has a rather elaborate mechanism to specify HTTP configuration
options per URL, based on pattern matching the URL against "http"
subsection names.[1] The URLs used for this matching are always the
original URLs; redirected URLs do not participate.
* Scheme and host must match exactly case-insensitively.
* An optional user name must match exactly.
* Ports must match exactly after default ports have been filled in.
* The path of a subsection, if any, must match a segment prefix of
the path of the URL.
* Matches with user name take precedence over equal-length path
matches without, but longer path matches are preferred over
shorter matches with user name.
Implement this for JGit. Factor out the HttpConfig from TransportHttp
and implement the matching and override mechanism.
The set of supported settings is still the same; JGit currently
supports only followRedirects, postBuffer, and sslVerify, plus the
JGit-specific maxRedirects key.
Add tests for path normalization and prefix matching only on segment
separators, and use the new mechanism in SmartClientSmartServerSslTest
to disable sslVerify selectively for only the test server URLs.
Compare also bug 374703 and bug 465492. With this commit it would be
possible to set sslVerify to false for only the git server using a
self-signed certificate instead of having to switch it off globally
via http.sslVerify.
[1] https://git-scm.com/docs/git-config
Change-Id: I42a3c2399cb937cd7884116a2a32fcaa7a418fcb
Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>
This will be used later when adding for support for recursing
submodules on push.
Change-Id: Ie2a183e5404a32046de9f6524e6ceeec37919671
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
With atomic ref updates using packed refs, JGit did not fire a
RefsChangedEvent. This resulted in a user-visible regression in
EGit: the UI would not update after a "Fetch from upstream...".
Presumably it would also make Gerrit miss out on ref changes?
Strengthen the BatchRefUpdateTest by also asserting the expected
number of RefsChangedEvents, and ensure modCnt is incremented in
RefDirectory.commitPackedRefs() when refs really changed (as opposed
to some internal housekeeping operation, such as packing loose refs).
Bug: 521296
Change-Id: Ia985bda1d99f45a5f89c8020ca4845e7a66e743e
Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>
Much of the time the caller can specify a RefSpec succinctly using a
string, and doesn't care about calling setters. Add a convenience method
for this case, and use it where applicable in JGit core.
Change-Id: Ic3fac7fc568eee4759236a5264d2e7e5f9b9716d
An application can choose to invoke setAdvertisedRefs multiple times,
for example several AdvertiseRefsHook installed in a chain. Each of
these invocations populates the advertisedHaves collection with the
unique set of ObjectIds.
This can lead to a server over-advertising with ".have" lines if the
first hook pushes in a lot of references, and the second hook filters
this to a subset. ReceivePack will advertise the unique objects from
the first hook using ".have" lines, which may lead to a huge
advertisement sent to the client.
This can also contribute to a very slow connectivity check after the
pack is parsed as ReceivePack calls markUninteresting on every commit
in advertisedHaves. This may require expanding a lot of subtrees to
mark all trees as uninteresting as well. On a very big repository
this can lead to a many-second stall.
Clear the advertisedHaves collection any time the refs are updated.
Add a test to verify the correct set of objects was sent.
Change-Id: I97f6998d0597251444a2e846a3ea1f461bae96f9
The tests:
- testCheckBlobNotCorrupt
- testCheckBlobCorrupt
create instances of ObjectChecker that are the same.
The tests:
- testCheckBlobWithBlobObjectCheckerNotCorrupt
- testCheckBlobWithBlobObjectCheckerCorrupt
also create instances of ObjectChecker that are the same.
Factor these instances out to constants instead of creating them
in the tests.
The `checker` member is still created anew in each test, since some
of the tests change its state.
Change-Id: I2d90263829d01d208632185b1ec2f678ae1a3f4c
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
Add tests for "true" and "false" matching to "YES" and "NO".
Change-Id: I58223855022871ac4b21bd34ff6a9cd00fce30a1
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
If a ReftableConfig has been supplied by the caller, write out a
reftable as a sibling of the the GC pack, alongside the heads.
To bootstrap from a non-reftable system, the refs are read from the
DfsRefDatabase if no GC reftables are present. Its assumed the
references are fully current, and do not need to be merged with any
other reftables. Any non-GC reftables will be pruned at the end of
the GC cycle, just like any packs that were replaced.
If a GC reftable is present, all existing reftables are compacted, and
references from DfsRefDatabase are only used to seed the packer. Its
assumed these are consistent with each other.
Change-Id: Ie397eb58aaaefb6865c816d9b39de3ac12998019
ServerSocket.accept() is not interruptible: a thread busy in accept()
may not react to Thread.interrupt() and may not return from accept()
via an InterruptedException. Close the socket instead to make the
daemon's listener thread terminate.
* Close the listening socket to get the listening thread to exit
instead of interrupting it.
* Add a stopAndWait() method that stops the listening thread and
then waits until it has indeed finished.
* Set SO_REUSE_ADDRESS on the listening socket.
Bug: 376369
Change-Id: I9d6014103e6dcb0173daea134feb44dc52c5c69a
Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>
While parsing .gitmodules, the name of the submodule subsection is
purely arbitrary: it frequently is the path of the submodule, but
there's no requirement for it to be. By building a map of paths to
the section name in .gitmodules, we can more accurately return
the submodule URL.
Bug: 508801
Change-Id: I8399ccada1834d4cc5d023344b97dcf8d5869b16
Also-by: Doug Kelly <dougk.ff7@gmail.com>
Signed-off-by: Doug Kelly <dougk.ff7@gmail.com>
Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
Some tests call out to external cgit. Those tests all failed for me
locally on Mac. Turned out that the reason was that the system git
config used by the git in the bazel run contained paths with ~/ but
somehow $HOME was not set. As a result the external git returned
with exit code 128.
Fix this by passing along $HOME explicitly. Also improve assertions
to make sure we do get the stderr of the external command in the
test log.
I hadn't noticed that until now because apparently the maven build
does pass along $HOME.
Change-Id: I7069676d5cc7b23a71e79a4866fe8acab5a405f4
Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>
Make jsch visible to the test bundle and add the dependency.
Change-Id: I0c49ee9b8f64fe8a8c74d2f08865917eb33069b4
Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>
Do not automatically organize imports using a save action since this
seems to be buggy and removed some annotations org.eclipse.jgit.pgm
needs to use args4j.
Change-Id: I5a91292c3b9241ce2dde3e4ecce14ad460097129
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
Revert the following save actions which were introduced in c0ad77d8:
- always use braces around blocks
- remove unused imports
Other than I expected save actions are run globally on edited files -
and not only on edited code lines only.
Hence revert the save action "Convert control statement bodies to
blocks" which would affect a large number of code lines not affected by
the change editing some small part of a class. This would generate a
large number of changes which may lead to many unnecessary conflicts.
Total number of affected lines across jgit would be around 10k lines.
Also revert "Remove unused imports" since it erroneously removes imports
of some annotations needed by pgm classes using args4j.
Change-Id: I879a47f68e664129e6124cf25c1ae1f6a2d7a5aa
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
Add the following Eclipse save actions executed when saving modified
lines. This should help to reduce manual work needed to maintain a clean
and consistent code style:
- organize imports
- always use braces around blocks
- add missing annotations
- @Override including implementation of interface methods
- @Deprecated
- remove
- unused imports
- unnecessary $NON-NLS$ tags
- redundant type arguments
Also add default values for new settings that were introduced in recent
Eclipse versions up to Neon since we updated save rules the last time.
Change-Id: Idc90b249df044d0552f04edf01a5f607c4846f50
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
Some repositories can have a policy that do not accept certain blobs. To
check if the incoming pack file contains such blobs, ObjectChecker can
be used. However, this ObjectChecker is not called by PackParser if the
blob is stored as a whole. This is because the object can be so large
that it doesn't fit in memory.
This change introduces BlobObjectChecker. This interface takes chunks of
a blob instead of the entire object. ObjectChecker can optionally return
a BlobObjectChecker. This won't change existing ObjectChecker
implementation; existing implementation continues to receive deltified
blob objects only.
Change-Id: Ic33a92c2de42bd7a89786a4da26b7a648b25218d
Signed-off-by: Masaya Suzuki <masayasuzuki@google.com>
** matching always tries the empty match first. If a mismatch occurs
later, the ** must be extended by exactly one segment and matching must
resume with the matcher following the ** matcher.
Bug: 520920
Change-Id: Id019ad1c773bd645ae92e398021952f8e961f45c
Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>
Path pattern matching for attribute rules is different than matching
for excluded files.
The first difference concerns patterns without slashes. For
gitattributes those must match on the last component only, not on
any earlier segment. This is true also for directory-only patterns.
The second difference concerns directory-only patterns. Those also
must not match on a prefix or segment except the last one. They do
not apply recursively to all files beneath.
And third, matches only on a prefix must match for gitattributes
only if the last matcher was "/**".
Add a new parameter for such path matching to IMatcher.matches() and
pass it through as appropriate (false for gitignore, true for
gitattributes). As far as gitignore is concerned, there is no change.
New tests have been added, and some existing attribute matching tests
have been fixed since they operated on wrong assumptions.
Bug: 508568
Change-Id: Ie825dc2cac8a85a72a7eeb0abb888f3193d21dd2
Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>
These tests verify that JGit matches the same as C git, for
both attribute matching (.gitattributes) and file exclusion matching
(.gitignore). These tests work by setting up a test repository and
test rules, and then determine excluded files or attributes both with
JGit and with the native C git, and then compare the results.
For .gitignore tests, we run
git ls-files --ignored --exclude-standard -o
and for attribute tests we use
git check-attr --stdin --all
and pass the list of all files in the repository via stdin.
Change-Id: I5b40946e04ff4a97456be7dffe09374323b7c89d
Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>
The Eclipse compiler raises errors for unthrown exceptions declared to
be thrown by test methods introduced in 88e45399.
Change-Id: I0d91c89e1b20ceff52c38b759abf906cc94e9902
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
Except for %p and %r and partially %C, we can do token substitutions
as defined by OpenSSH inside the config file parser. %p and %r can
be replaced only if specified in the config; if not, it would be the
caller's responsibility to replace them with values obtained from the
URI to connect to.
Jsch doesn't know about token substitutions at all. By doing the
replacements as good as we can in the config file parser, we can
make Jsch support most of these tokens.
%i is not handled at all as Java has no concept of a "user ID".
Includes unit tests.
Bug: 496170
Change-Id: If9d324090707de5d50c740b0d4455aefa8db46ee
Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>
Ensure the Jsch instance used knows about ~/.ssh/config. This
enables Jsch to honor more user configurations (see
com.jcraft.jsch.Session.applyConfig()), in particular also the
UserKnownHostsFile configuration, or additional identities given
via multiple IdentityFile entries.
Turn JGit's OpenSshConfig into a full parser that can be a
Jsch-compliant ConfigRepository. This avoids a few bugs
in Jsch's OpenSSHConfig and keeps the JGit-facing interface
unchanged. At the same time we can supply a JGit OpenSshConfig
instance as a ConfigRepository to Jsch. And since they'll both
work from the same object, we can also be sure that the parsing
behavior is identical.
The parser does not handle the "Match" and "Include" keys, and it
doesn't do %-token substitutions (yet).
Note that Jsch doesn't handle multi-valued UserKnownHostFile
entries as known by modern OpenSSH.[1]
[1] http://man.openbsd.org/OpenBSD-current/man5/ssh_config.5
Additional tests for new features are provided in OpenSshConfigTest.
Bug: 490939
Change-Id: Ic683bd412fa8c5632142aebba4a07fad4c64c637
Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
Add an update_index to every reference in a reftable, storing the
exact transaction that last modified the reference. This is necessary
to fix some merge race conditions.
Consider updates at T1, T3 are present in two reftables. Compacting
these will create a table with range [T1,T3]. If T2 arrives during
or after the compaction its impossible for readers to know how to
merge the [T1,T3] table with the T2 table.
With an explicit update_index per reference, MergedReftable is able to
individually sort each reference, merging individual entries at T3
from [T1,T3] ahead of identically named entries appearing in T2.
Change-Id: Ie4065d4176a5a0207dcab9696ae05d086e042140
* changes:
LongObjectIdTest: Add back self comparison test
Format BUILD files with buildifier
Bazel: Add missing dependency in org.eclipse.jgit.http.test
resolve(Ref) helps callers recursively chase symbolic references and
is a useful function when wrapping a Reftable inside a RefDatabase, as
RefCursor does not resolve symbolic references during iteration.
Change-Id: I1ba143f403773497972e225dc92c35ecb989e154
A compaction of reftables is just copying the results of a
MergedReftable into a ReftableWriter. Wrap this up into a utility.
Change-Id: I6f5677d923e9628993a2d8b4b007a9b8662c9045
MergedReftable combines multiple reference tables together in a stack,
allowing higher/later tables to shadow earlier/lower tables. This
forms the basis of a transaction system, where each transaction writes
a new reftable containing only the modified references, and readers
perform a merge on the fly to get the latest value.
Change-Id: Ic2cb750141e8c61a8b2726b2eb95195acb6ddc83
Add additional test cases for looking up entries within a namespace
such as refs/heads/ or refs/tags/, where the seek is passed a name
that ends with '/'.
Change-Id: I5f944de7518cd0090374bddba48d4dd3955a8d72
Add more test cases that cover larger collections of
references, verifying every reference is accessible
both by scan and by seek.
Change-Id: Icada59fdcfc92a4634f6df61baaebb1c37b75d98
This set of tests covers primitive storage of an empty
file, and each type of supported reference.
Change-Id: I3bdff35cae8ae27283051932f20608b3ac353559
Currently there is no way to determine the precise changes done
to the working tree by a JGit command. Only the CheckoutCommand
actually provides access to the lists of modified, deleted, and
to-be-deleted files, but those lists may be inaccurate (since they
are determined up-front before the working tree is modified) if
the actual checkout then fails halfway through. Moreover, other
JGit commands that modify the working tree do not offer any way to
figure out which files were changed.
This poses problems for EGit, which may need to refresh parts of the
Eclipse workspace when JGit has done java.io file operations.
Provide the foundations for better file change tracking: the working
tree is modified exclusively in DirCacheCheckout. Make it emit a new
type of RepositoryEvent that lists all files that were modified or
deleted, even if the checkout failed halfway through. We update the
'updated' and 'removed' lists determined up-front in case of file
system problems to reflect the actual state of changes made.
EGit thus can register a listener for these events and then knows
exactly which parts of the Eclipse workspace may need to be refreshed.
Two commands manage checking out individual DirCacheEntries themselves:
checkout specific paths, and applying a stash with untracked files.
Make those two also emit such a new WorkingTreeModifiedEvent.
Furthermore, merges may modify files, and clean, rm, and stash create
may delete files.
CQ: 13969
Bug: 500106
Change-Id: I7a100aee315791fa1201f43bbad61fbae60b35cb
Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>
When creating a new PackFile instance it is specified whether this pack
has an associated bitmap index file or not. This information is cached
and the public method getBitmapIndex() will always assume a bitmap index
file must exist if the cached data tells so. But it may happen that the
packfiles are repacked during a gc in a different process causing the
packfile, bitmap-index and index file to be deleted. Since JGit still
has an open FileHandle on the packfile this file is not really deleted
and can still be accessed. But index and bitmap index file are deleted.
Fix getBitmapIndex() to invalidate the cached packfile instance if such
a situation occurs.
This problem showed up when a gerrit server was serving repositories
which where garbage collected with native git regularly. Fetch and
clone commands for certain repositories failed permanently after a
native git gc had deleted old bitmap index files.
Change-Id: I8e620bec74dd3f310ba42024f9a657062f868f0e
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
Per the git config documentation[1], pushInsteadOf is ignored when
a remote has explicit pushUris.
Implement this, and adapt tests.
Up to now JGit mistakenly applied pushInsteadOf also to existing
pushUris. If some repositories had relied on this mis-feature,
pushes may newly suddenly fail (the uncritical case; the config
just needs to be fixed) or even still succeed but push to unexpected
places, namely to the non-rewritten pushUrls (the critical case).
The release notes should point out this change.
[1] https://git-scm.com/docs/git-config
Bug: 393170
Change-Id: I38c83204d2ac74f88f3d22d0550bf5ff7ee86daf
Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>
According to [1], pushInsteadOf is
1. applied to the uris, not to the pushUris
2. ignored if a remote has an explicit pushUri
JGit applied it only to the pushUris. As a result, pushInsteadOf was
ignored for remotes having only a uri, but no pushUri.
This commit implements (1) if there are no pushUris. I did not dare
implement (2) because:
* there are explicit tests for it that expect that pushInsteadOf gets
applied to existing pushUrls, and
* people may actually use and rely on this JGit behavior.
[1] https://git-scm.com/docs/git-config
Bug: 393170
Change-Id: I6dacbf1768a105190c2a8c5272e7880c1c9c943a
Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>
This matches the proposal that has been discussed at length on
git-core mailing list and seems to be the accepted convention.
Change-Id: I9f6ab15144826893d1e2a4b48a2d657d6dd445ec
Otherwise fancy combinations of attributes (binary or -text in
combination with crlf or eol) may result in the corruption of binary
data.
Bug: 520910
Change-Id: I3ffc666c13d1b9d2ed987b69a67bfc7f42ccdbfc
Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>
Attribute rules must match against the entry path relative to the
attribute node containing the rule. The global entry path is to be
used only for the init and the global node (and of course the root
node).
Bug: 520677
Change-Id: I80389a2dc272a72312729ccd5358d7c75e1ea20a
Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>
This avoids executing mergeAlgorithm.merge on binary data, which is
unlikely to be useful.
Arguably, binary data should not make it to
ResolveMerger#contentMerge, but this approach has the following
advantages:
* binary detection is exact, since it doesn't only look at the start
of the blob.
* it is cheap, as we have to iterate over the bytes anyway to find
'\n'.
Signed-off-by: Han-Wen Nienhuys <hanwen@google.com>
Change-Id: I424295df1dc60a719859d9d7c599067891b15792
Very short abbreviations that are under 8 hex digits do not
have values in w2. Use w1 as the Java hashCode() instead, so
that the prefix of the abbreviation is always included in the
hashing function used by any java.util.Collection type.
Change-Id: Idaf69f86b62630ba4a022d31b4c293c6d138f557
In a server scenario such as Gerrit Code Review, there may be many
atomic BatchRefUpdates contending for locks on both the packed-refs file
and some subset of loose refs. We already retry lock acquisition to
improve this situation slightly, but we can do better by using an
in-process lock. This way, instead of retrying and potentially exceeding
their timeout, different threads sharing the same Repository instance
can wait on a fair lock without having to touch the disk lock. Since a
server is probably already using RepositoryCache anyway, there is a high
likelihood of reusing the Repository instance.
Change-Id: If5dd1dc58f0ce62f26131fd5965a0e21a80e8bd3
If a repo frequently uses PackedBatchRefUpdates, there is likely to be
contention on the packed-refs file, so it's not appropriate to fail
immediately the first time we fail to acquire a lock. Add some logic to
RefDirectory to support general retrying of lock acquisition.
Currently, there is a hard-coded wait starting at 100ms and backing off
exponentially to 1600ms, for about 3s of total wait. This is no worse
than the hard-coded backoff that JGit does elsewhere, e.g. in
FileUtils#delete. One can imagine a scheme that uses per-repository
configuration of backoff, and the current interface would support this
without changing any callers.
Change-Id: I4764e11270d9336882483eb698f67a78a401c251
Make sure all objects referenced by references are reachable. Stop at
the first missing object.
Change-Id: Ifcd7392c4321b17d9290bd87f038bc62bc10dabb
Signed-off-by: Zhen Chen <czhen@google.com>
JGit already had some fsck-like classes like ObjectChecker which can
check for an individual object.
The read-only FsckPackParser which will parse all objects within a pack
file and check it with ObjectChecker. It will also check the pack index
file against the object information from the pack parser.
Change-Id: Ifd8e0d28eb68ff0b8edd2b51b2fa3a50a544c855
Signed-off-by: Zhen Chen <czhen@google.com>
On-disk reflogs are not stored in the packed-refs file, so we cannot
ensure atomic updates. We choose the lesser evil of dropping failed
reflog updates on the floor, rather than throwing an exception even
though the underlying ref updates succeeded.
Add tests for reflogs to BatchRefUpdateTest.
Change-Id: Ia456ba9e36af8e01fde81b19af46a72378e614cd
* Factor out helpers for setting up and executing updates.
* Use common assert methods, with a special enum type that papers over
the fact that there is no ReceiveCommand.Result for transaction
aborted.
* Static import ReceiveCommand.Type constants.
* Add blank lines to separate repo setup, update execution, and asserts.
Change-Id: Ic3717f94331abfc7ae3e92065f3fe32026bf7cea
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
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
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
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>
* 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
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
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
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
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
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
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
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
This renaming supports reusing DfsStreamKey in a future commit
to index other PackExt type streams inside of the DfsBlockCache.
Change-Id: Ib52d374e47724ccb837f4fbab1fc85c486c5b408
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>
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>
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>
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>
Change-Id: Idcc93c2ca95938995d489cffda649c7d7b26c50e
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
If set, "singlePack" will create a single GC pack file for all
objects reachable from refs/*. If not set, the GC pack will contain
object reachable from refs/heads/* and refs/tags/*, and the GC_REST
pack will contain all other reachable objects.
Change-Id: I56bcb6a9da2c10a0909c2f940c025db6f3acebcb
Signed-off-by: Terry Parker <tparker@google.com>
When a command invoked from readPipe fails to launch (i.e. the exec call
fails due to a missing command executable), Process.start() throws,
which gets caught by the generic IOException handler, resulting in a
null return. This change detects this case and rethrows a
CommandFailedException instead.
Additionally, this change uses /bin/sh instead of bash for its posix
command failure test, to accomodate building in environments where bash
is unavailable.
Change-Id: Ifae51e457e5718be610c0a0914b18fe35ea7b008
Signed-off-by: Bryan Donlan <bdonlan@gmail.com>
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
Commit db77610 ensured that all refs/tags commits are added to the
primary GC pack. It did that by adding all of the refs/tags commits
to the primary GC pack PackWriter's "interesting" object set.
Unfortunately, all commit objects in the "interesting" set are
selected as commits for which bitmap indices will be built. In a
repository like chromium with lots of tags, this changed the number of
bitmaps created from <700 to >10000. That puts huge memory pressure on
the GC task.
This change restores the original behavior of ignoring tags when
selecting commits for bitmaps.
In the "uninteresting" set, commits for refs/heads and refs/tags for
unannotated tags can not be differentiated. We instead identify
refs/tags commits by passing their ObjectIds as a new "noBitmaps"
parameter to the PackWriter.preparePack() methods.
PackWriterBitmapPreparer.setupTipCommitBitmaps() can then use that
"noBitmaps" parameter to exclude those commits.
Change-Id: Icd287c6b04fc1e48de773033fe432a9b0e904ac5
Signed-off-by: Terry Parker <tparker@google.com>
DirCacheCheckout is generating names for temporary files. It was not checking
the length of this filenames. It may happen that a generated filename is
longer than 255 chars which causes problems on certain platforms. Make sure
that filenames for temporary files do not exceed 255 chars.
Bug: 508823
Change-Id: I9475c04351ce3faebdc6ad40ea4faa3c326815f4
Delete the condition to check whether the garbage pack creation time
is older than the last GC operation, because it's not possible to
find the last GC operation time when there is no GC pack.
Add additional tests to make sure the contents of the expired garbage
packs are considered during the GC operation and any actively
referenced objects from the garbage packs are copied successfully
into the GC pack before deleting the garbage pack.
Change-Id: I09e8b2656de8ba7f9b996724ad1961d908e937b6
Signed-off-by: Thirumala Reddy Mutchukota <thirumala@google.com>
Android wants them to work, and we're only interested in them for bare
repos, so add them just for that.
Make sure to use symlinks instead of just using the copyfile
implementation. Some scripts look up where they're actually located in
order to find related files, so they need the link back to their
project.
Change-Id: I929b69b2505f03036f69e25a55daf93842871f30
Signed-off-by: Dan Willemsen <dwillemsen@google.com>
Signed-off-by: Stefan Beller <sbeller@google.com>
Signed-off-by: Jeff Gaston <jeffrygaston@google.com>
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
This is necessary for deploying submodules on android.googlesource.com.
* Allow an empty base URL. This is useful if the 'fetch' field is "."
and all names are relative to some host root.
* The URLs in the resulting superproject are relative to the
superproject's URL. Add RepoCommand#setDestinationURI to
set this. If unset, the existing behavior is maintained.
* Add two tests for the Android and Gerrit case, checking the URL
format in .gitmodules; the tests use a custom RemoteReader which is
representative of the use of this class in Gerrit's Supermanifest
plugin.
Change-Id: Ia75530226120d75aa0017c5410fd65d0563e91b
Signed-off-by: Han-Wen Nienhuys <hanwen@google.com>
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
* stable-4.7:
Cleanup and test trailing slash handling in ManifestParser
ManifestParser: Throw exception if remote does not have fetch attribute
Change-Id: Ia9dc3110bcbdae05175851ce647ffd11c542f4c0
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
This is a workaround for
https://bugs.openjdk.java.net/browse/JDK-4666701.
Change-Id: Idd04657e8d95a841d72230f8881b6b899daadbc2
Signed-off-by: Han-Wen Nienhuys <hanwen@google.com>
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
In the repo manifest documentation [1] the fetch attribute is marked
as "#REQUIRED".
If the fetch attribute is not specified, this would previously result in
NullPointerException. Throw a SAXException instead.
[1] https://gerrit.googlesource.com/git-repo/+/master/docs/manifest-format.txt
Change-Id: Ib8ed8cee6074fe6bf8f9ac6fc7a1664a547d2d49
Signed-off-by: Han-Wen Nienhuys <hanwen@google.com>
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
It was already increased in 61a943e, but that was still not enough to
take into account the length of snapshot versions.
Change-Id: Ib54cec97e97042fe274b87a3a1afa9bb06c8bf19
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
All that's really required to run a merge operation is a single
ObjectInserter, from which we can construct a RevWalk, plus a Config
that declares a diff algorithm. Provide some factory methods that don't
take Repository.
Change-Id: Ib884dce2528424b5bcbbbbfc043baec1886b9bbd
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>
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>
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>
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>
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>
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>