Provide a public interface PackLock exposing only the unlock() method.
Rename the internal PackLock class to PackLockImpl and have it implement
the new interface.
This way PackParser doesn't expose an internal class via its API
anymore, and client can still unlock pack locks that were created.
Bug: 576340
Change-Id: I976739f4ab28fe1f9ba7f35653a69a913aa68841
Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>
Remove the unused parameter, which had a non-API type anyway.
Bug: 576340
Change-Id: Id71c01a643e1f31a8ff61ff69f7915c373db3263
Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>
While building the commit for the destination project, RepoCommand
catches GitApiExceptions and wraps them into ManifestErrorException
(a subclass of GitApiException). This hides the real exception from the
caller and prevent them to do a fine-grained catch.
Specifically this is problematic for gerrit's supermanifest plugin, that
won't see ConcurrentRefUpdate exceptions to detect lock-failures.
Use ManifestErrorException to wrap non-GitApiExceptions, let
GitApiExceptions pass through as they are.
Change-Id: Ia2cda244e65993bb07c89cd6435507d5d0754dd4
Use the GitAPIException CanceledException instead of IOException
CancelledException in the rename detector.
Change-Id: I5121719eb714a123ec57769fc113c84857bd3c6c
Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>
Skip javadoc generation for test bundles.
Use character entities < and > for < and > outside of
code-formatted spans.
Change-Id: I66e1a1dc98881c61f93c9e5561c5513896b2ba01
Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>
We've used "GIT_EDITOR=edit git config --system --edit" to determine
the location of the git system config for a long time. But git 2.34.0
always expects this command to have a tty, but there isn't one when
called from Java. If there isn't one, the Java process may get a
SIGTTOU from the child process and hangs.
Arguably it's a bug in C git 2.34.0 to unconditionally assume there
was a tty. But JGit needs a fix *now*, otherwise any application using
JGit will lock up if git 2.34.0 is installed on the machine.
Therefore, use a different approach if the C git found is 2.8.0 or
newer: parse the output of
git config --system --show-origin --list -z
"--show-origin" exists since git 2.8.0; it prefixes the values with
the file name of the config file they come from, which is the system
config file for this command. (This works even if the first item in
the system config is an include.)
Bug: 577358
Change-Id: I3ef170ed3f488f63c3501468303119319b03575d
Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>
CommitConfig.getCommitTemplateContent() has changed API.
Change-Id: If72ed79c43c167b99f356b69cc095d6bbf4d66e8
Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>
Fixes an issue that commit template file could not be found if it has a
relative path instead of absolute path.
Relative path is probably common if git config --local is used.
Bug: 446355
Change-Id: I8ddf2be672647be825fd9c01af82809d31bb8356
With change https://git.eclipse.org/r/c/jgit/jgit/+/186455 a thread
loading a bitmap index could hold two ref locks at the same time (one
for bitmap and one for either index or reverse index). So it is possible
that two threads loading bitmaps end up in a deadlock for ref locks e.g.
threadA has refLock[1] (for bitmap) and wants refLock[2] (for index or
revIndex) and threadB has refLock[2] (for bitmap) and wants refLock[1].
This change introduces separate pools of locks per pack extension
instead of a shared pool. So threads loading bitmap can hold two
locks but with different extensions and no overlap, e.g. threadA holds
refLock[BITMAP_INDEX][1] and refLock[INDEX][2] and threadB holds
refLock[BITMAP_INDEX][2] and refLock[INDEX][1].
More unit tests were added to cover various paralell loading scenarios.
Signed-off-by: Alina Djamankulova <adjama@google.com>
Change-Id: I89704b4721c21548929608d3798ef60925280755
We use BinaryBlobException to signal a binary blob was found and never
make use of its stack trace. Suppress filling in the stack trace to
avoid the performance penalty coming with that.
See https://shipilev.net/blog/2014/exceptional-performance/
Change-Id: Iae1f1c19a1fa8aef4f6569822557171130299958
Since OpenSSH 7.8, the ProxyJump directive accepts the value "none"[1]
to override and clear a setting that might otherwise be contributed by
another (wildcard) host entry.
[1] https://bugzilla.mindrot.org/show_bug.cgi?id=2869
Change-Id: Ia35e82c6f8c58d5c6b8040cda7a07b220f43fc21
Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>
Bring our SSH config parser up-to-date with respect to changes in
OpenSSH. In particular, they fixed[1] the handling of line comments
such that #-characters inside strings are not considered. This means
that we have to parse strings with escaped quotes correctly.
[1] https://bugzilla.mindrot.org/show_bug.cgi?id=3288
Change-Id: Ifbd9014127e8d51e7c8792e237f3fc2a9a0719d2
Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>
It appears that the OpenSSH documentation[1] has changed; it now allows
more flags for a number of keys.
[1] https://man.openbsd.org/ssh_config.5#TOKENS
Change-Id: I55df174f86a3fd4a6ef22687dc433ac9f9ad181d
Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>
Once a factory supports different SSH agents on the same platform,
which is planned for Windows once we use Apache MINA sshd 2.8.0,
client code may need to have a way to specify which SSH agent shall
be used when the SSH config doesn't define anything.
Add a mechanism by which a ConnectorFactory can tell what Connectors
it may provide. Client code can use this to set the identityAgent
parameter of ConnectorFactory.create() to the wanted default if it
would be null otherwise.
A ConnectorDescriptor is a pair of strings: an internal name, and a
display name. The latter is included because client code might want to
communicate agent names to the user, be it in error messages or in some
chooser dialog where a user could define which of several alternative
SSH agents should be used as default. The internal name is intended to
be used in the IdentityAgent directive in ~/.ssh/config.
Also make the ConnectorFactory discovered via the ServiceLoader
accessible and overrideable. Provide static get/setDefault() methods,
similar to the SshSessionFactory itself.
Change-Id: Ie3d077395d32dfddc72bc8627e92b23636938182
Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>
This is a prerequisite for removing the JSch support bundle; otherwise
OpenSshConfigFile would be left without tests.
Copy OpenSshConfigTest from the JSch support bundle and adapt all tests
to perform the equivalent checks on OpenSshConfigFile directly. Add a
new lookupDefault() method to the SshConfigStore interface and implement
it so that it behaves the same and the tests work identically.
Change-Id: I046abd9197a8484003e77005024e5d973456f1a3
Make the default toolchain use Java 11, and fix two errorprone findings
introduced recently.
Change-Id: Iff51206fe8bdf096cb7d88cb1a499002550766cd
Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>
C git considers not only files containing NUL bytes as binary but also
files containing lone CRs. Implement this also for JGit.
C git additionally counts printable vs. non-printable characters and
considers files that have non_printable_count > printable_count / 128
also as binary. This is not implemented because such counting probably
only makes sense if one looks at the full file or blob content. The
Auto[CR]LF* streams in JGit look only at the first few KiB of a stream
in order not to buffer too much.
For the C git implementation, see [1].
[1] https://github.com/git/git/blob/7e27bd589d/convert.c#L35
Bug: 576971
Change-Id: Ia169b59bdbf1477f32ee2014eeb8406f81d4b1ab
Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>
Move the code to parse numbers with an optional 'k', 'm', or 'g' suffix
from the config file handling to StringUtils. This enables me to re-use
it in EGit, which has duplicate code in StorageSizeFieldEditor.
As this is generally useful functionality, providing it in the library
makes sense.
Change-Id: I86e4f5f62e14f99b35726b198ba3bbf1669418d9
Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>
The various streams used in JGit for text/binary and CR-LF detection
used different buffer sizes. Most used 8000, but one used 8KiB, and one
used 8096 (SIC!) bytes.
Considering only the first 8kB of a file/blob is not sufficient; it
may give behavior incompatible with C git. C git considers the whole
blob; since it uses memory-mapped files it can do so with acceptable
performance. Doing this in JGit would most likely incur a noticeable
performance penalty. But 8kB is a bit small; in the file in bug 576971
the limit was hit before the first CR-LF, which occurred on line 155
at offset 9759 in the file.
Make RawText.FIRST_FEW_BYTES only a default and minimum setting, and
set it to 8KiB. Make the actual buffer size configurable: provide
static methods getBufferSize() and setBuffersize(), and use
getBufferSize() throughout instead of the constant.
This enables users of the JGit library to set their own possibly larger
buffer size.
Bug: 576971
Change-Id: I447762c9a5147a521f73d2864ba59ed89f555d54
Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>
Add tests for files having been checked in with mixed LF and CR-LF
line endings, and then being checked out with text or text=auto and
eol=lf or eol=crlf. These test cases were missing, and thus commit
efd1cc05 missed that AutoCRLFOutputStream needs the same detection
as AutoLFOutputStream.
Fix AutoCRLFOutputStream to not convert line endings if the blob in
the repository contains CR-LF.
Bug: 575393
Change-Id: Id0c7ae772e282097e95fddcd3f1f9d82aae31e43
Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>
JEP 400 (Java 18) will change the default character set to UTF-8
unconditionally.[1] Introduce SystemReader.getDefaultCharset() that
provides the locale-dependent charset the way JEP 400 recommends.
Change all code locations using Charset.defaultCharset() to use the
new SystemReader method instead.
[1] https://openjdk.java.net/jeps/400
Change-Id: I986f97a410d2fc70748b6f93228a2d45ff100b2c
Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>
This change is a fix to http://git.eclipse.org/r/c/jgit/jgit/+/183562
that was reverted in http://git.eclipse.org/r/c/jgit/jgit/+/184978
due to deadlocks. Separate locks in DfsBlockFile are removed to rely
on getting value from DfsBlockCache with region locking in place.
With this change bitmap index creation is not blocked on index and
reverse index full initialization in DfsPackFile. Now bitmap index
and index could be read from storage in parallel in separate threads.
A unit test is added for parallel index loading.
Signed-off-by: Alina Djamankulova <adjama@google.com>
Change-Id: Ic6d9c5a4a254628636aa98a5008447a27a003f69
* stable-5.13:
Fix missing peel-part in lsRefsV2 for loose annotated tags
Fix RevWalk.getMergedInto() ignores annotated tags
Optimize RevWalk.getMergedInto()
reftable: drop code for truncated reads
reftable: pass on invalid object ID in conversion
Update eclipse-jarsigner-plugin to 1.3.2
Fix running benchmarks from bazel
Update eclipse-jarsigner-plugin to 1.3.2
Add org.bouncycastle.bcutil to p2 repository
Change-Id: I789e3c84045cc9dcd8a26de34d09fc1e3ccc41e7
* stable-5.12:
Fix missing peel-part in lsRefsV2 for loose annotated tags
Fix RevWalk.getMergedInto() ignores annotated tags
Optimize RevWalk.getMergedInto()
reftable: drop code for truncated reads
reftable: pass on invalid object ID in conversion
Update eclipse-jarsigner-plugin to 1.3.2
Fix running benchmarks from bazel
Update eclipse-jarsigner-plugin to 1.3.2
Change-Id: Ie5da8d2d07f39ab5aaeca560040637635d4eb04c
* stable-5.11:
Fix missing peel-part in lsRefsV2 for loose annotated tags
reftable: drop code for truncated reads
reftable: pass on invalid object ID in conversion
Update eclipse-jarsigner-plugin to 1.3.2
Fix running benchmarks from bazel
Update eclipse-jarsigner-plugin to 1.3.2
Change-Id: I2122b3966bddb10d43d439eb8c5d6cc4583c6ba6
* stable-5.10:
Fix missing peel-part in lsRefsV2 for loose annotated tags
reftable: drop code for truncated reads
reftable: pass on invalid object ID in conversion
Update eclipse-jarsigner-plugin to 1.3.2
Fix running benchmarks from bazel
Update eclipse-jarsigner-plugin to 1.3.2
Change-Id: I23ccb89306263ce042ce782d881812d4b5187672
* stable-5.9:
Fix missing peel-part in lsRefsV2 for loose annotated tags
reftable: drop code for truncated reads
reftable: pass on invalid object ID in conversion
Update eclipse-jarsigner-plugin to 1.3.2
Fix running benchmarks from bazel
Update eclipse-jarsigner-plugin to 1.3.2
Change-Id: I4e7c7727ebfa9a7748477f2300bd66d775548ad6
We observed the following issue:
$ git tag -a v1.0 -m v1.0
$ git push origin tag v1.0
$ git ls-remote origin v1.0^{}
... empty result ...
On the server (Gerrit) side run git-gc to pack the refs:
$ git gc
Repeat the ls-remote from the client and the result is correct:
$ git ls-remote origin v1.0^{}
7ad85c810de3ae922903d4bdd17c53cd627260ba refs/tags/v1.0^{}
Unfortunately, the existing UploadPackTest didn't reveal this issue
although it provided the test case needed to do so: testV2LsRefsPeel.
This is because The UploadPackTest uses InMemoryRepository which
internally uses Dfs* implementations. The issue is only reproducible
when using the FileRepository.
It is a non-trivial task to refactor the UploadPackTest to work against
both InMemoryRepository and FileRepository and this change is not trying
to do that. This change creates a new test:
UploadPackLsRefsFileRepositoryTest and copies the necesssary code from
the UploadPackTest.
Change-Id: Icfc7d0ca63f1524bafe24c9626ce12ea72aa3718
Signed-off-by: Saša Živkov <sasa.zivkov@sap.com>
Let certain pack extensions that are expensive to load from storage
(e.g. pack index, bitmap index) stay in DFS block cache longer than
others by overriding default cache count through DfsBlockCacheConfig
Don't change default behavior when cache override map is empty. Use int
cacheCount instead of boolean hot for Ref<T>
Signed-off-by: Alina Djamankulova <adjama@google.com>
Change-Id: I18062784ec9cc14dbba3e4bb8d9509440cf2d44f
If an annotated tag refers to a commit, we should not ignore it.
Change-Id: I77504f93636e9e984540e7d8535ef301adce6a80
Signed-off-by: kylezhao <kylezhao@tencent.com>
Transitive Relation Definition:
On the DAG of commit history, if A can reach B, C can reach A, then C
can reach B.
Example:
As is shown in the graph below:
1 - 2 - 3 - 4 (side)
\
5 - 6^ (master) - 7 (topic)
Find out which branches is 2 merged into:
After we calculated that master contains 2, we can mark 6 as TEMP_MARK
to avoid unwanted walks.
When we want to figure out if 2 is merge into the topic, the traversal
path would be [7, 6] instead of [7, 6, 5, 3, 2].
Test:
This change can significantly improve performance for tags.
On a copy of the Linux repository, the command 'git tag --contains
<commit>' had the following performance improvement:
commit | Before | After | Rel %
47a44d27ca | 29251ms | 6687ms | -77%
90327e7dff | 21388ms | 6256ms | -70%
f85fac0efa | 11150ms | 7338ms | -34%
The current version ignores tags, even though the tag is a type of
the ref.
Follow-up commits I'll fix it.
Change-Id: Ie6295ca4d16070499912af462239e679a97cce47
Signed-off-by: kylezhao <kylezhao@tencent.com>
Reviewed-by: Christian Halstrick <christian.halstrick@sap.com>
Reviewed-by: Martin Fick <mfick@codeaurora.org>
* stable-5.8:
reftable: drop code for truncated reads
reftable: pass on invalid object ID in conversion
Update eclipse-jarsigner-plugin to 1.3.2
Change-Id: I9e66ef90dc9a65bac47b35705d679bf992bd72b9
* stable-5.7:
reftable: drop code for truncated reads
reftable: pass on invalid object ID in conversion
Update eclipse-jarsigner-plugin to 1.3.2
Change-Id: I88c47ff57f4829baec5b19aad3d8d6bd21f31a86
* stable-5.6:
reftable: drop code for truncated reads
reftable: pass on invalid object ID in conversion
Update eclipse-jarsigner-plugin to 1.3.2
Change-Id: I1c18f5f435f4a4a86e0548a310dbfc74191e1ed5
To avoid a warning about a name clash with the field.
Change-Id: Ib1950486a9c63647e4240a268a3c9a7d613f3303
Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>
In Java 11 type arguments for anonymous subclasses can be inferred
and don't need to be specified. This resolves a number of compiler
warnings.
Change-Id: I55eff3babb7628aa0627085e65a1b45eb12c2cd3
Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>
This ensures the compiler compiles against the public, supported and
documented API for a specific VM version (here 11) [1]. This also means
that
we don't need EE descriptors in Eclipse anymore in order to ensure that
only supported APIs of the selected Java version can be used.
According to [2] if option --release is used --source and --target
options can't be used.
While we are at it also add default value for all new jdt core options
added in Eclipse 4.21.
[1] https://docs.oracle.com/en/java/javase/11/tools/javac.html
[2] https://docs.oracle.com/en/java/javase/14/docs/specs/man/javac.html#option-release
Change-Id: I852a5d7b0a3210751c15d79ec91915b4c01c41e2
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
The reftable format is a block based format, but allows for variably
sized blocks. This obviously happens for reflog blocks (which are zlib
compressed), but is also accepted for index blocks: In the spec, this
is motivated as
To achieve constant O(1) disk seeks for lookups the index must be
a single level, which is permitted to exceed the file's
configured block size, but not the format's max block size of
15.99 MiB.
Hence, when parsing a block, one cannot be sure of its exact size:
after reading a default-size block (eg. 4kb), the block header may
state that the block is in fact larger.
Before, the code would mark the block as `truncated`, noting
// Its OK during sequential scan for an index block to have been
// partially read and be truncated in-memory. This happens when
// the index block is larger than the file's blockSize. Caller
// will break out of its scan loop once it sees the blockType.
This looks like either
* a remnant of never-implemented functionality. There is no reason to
ever sequentially scan an index block.
* alluding to sequential scan of the data blocks before the index
blocks (eg. scanning refs, which ends when we find the first ref index
block, and we can then ignore the index block).
This comment is followed by code that populates the
restartTbl/restartCnt fields relative to the (possibly truncated)
buffer. If the buffer is truncated, this essentially reads garbage,
leading to OOB array access when using the index block.
Fix this by dropping the truncated logic and issuing a second read if
the first read was short.
Add a test.
We have never observed this failure scenario at Google. We use 64kb
blocksize, which requires us to need fewer index entries. The reftable
spec mentions an Android repo of size 36M. With 64kb blocks, that's
just 562 index entries. Even with historical growth, we are long from
requiring an index whose size exceeds a single block.
When adding the analogous test for seeking refs, there was no failure.
This points to another possibility which is that the code tries to
avoid writing large index blocks for refs.
I did not investigate further which one it is.
Fixes https://bugs.eclipse.org/bugs/show_bug.cgi?id=576250
Bug: 576250
Change-Id: I41ec21fac9e526ef57b3d6fb57b988bd353ee338
Signed-off-by: Han-Wen Nienhuys <hanwen@google.com>
Before, while trying to determine if an object ID was a tag or not,
the reftable conversion would yield an exception.
Change-Id: I3688a0ffa9e774ba27f320e3840ff8cada21ecf0
Matthias Sohn