DirCacheEditor unconditionally applied a PathEdit to all stages in the
index. This gives wrong results if one wants to check out a file from
some commit to resolve a conflict: JGit would update the working tree
file multiple times (once per stage), and set all stages to point to
the checked-out blob.
C git replaces the stages by the entry for the checked-out file.
To support this, add a DirCacheEntry.setStage() method so that
CheckoutCommand can force the stage to zero. In DirCacheEditor, keep
only the zero stage if the PathEdit re-set the stage.
Bug: 568038
Change-Id: Ic7c635bb5aaa06ffaaeed50bc5e45702c56fc6d1
Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>
Due to Java bug JDK-8220793 [1] Java cannot set timestamps of a symlink
but only of the symlink target. This bug was fixed in Java 13. Since we
don't have a use case to set the timestamp of the symlink itself simply
document the current behavior of setLastModified methods.
[1] https://bugs.openjdk.java.net/browse/JDK-8220793
Change-Id: Ibc28c1702a1b9845602bd257606fbd44803a43fb
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
PerformanceLogContext threw NullPointerException when multiple threads
tried to add an event to the PerformanceLogContext. The cause for this
is that the ThreadLocal was initialized only in the class constructor
for the first thread; for subsequent threads it was null.
To fix this initialize eventRecords for each thread.
Change-Id: I18ef67dff8f0488e3ad28c9bbc18ce73d5168cf9
Signed-off-by: Alexa Panfil <alexapizza@google.com>
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
Fix IOException occurring when calling
GC on a repository with absent objects/pack folder.
Change-Id: I5be1333a0726f4d7491afd25ddac85451686c30a
Signed-off-by: Nail Samatov <sanail@yandex.ru>
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
This reverts commit f802f06e7f.
I had misunderstood how protocol V2 works. This implementation only
works if the negotiation during fetch is done in one round.
Fixing this is substantial work in BasePackFetchConnection. Basically
I think I'd have to change back negotiate to the V0 version, and have
a doFetch() that does
if protocol V2
doFetchV2()
else
doFetchV0()
with doFetchV0 the old code, and doFetchV2 completely new.
Plus there would need to be a HTTP test case requiring several
negotiation rounds.
This is a couple of days work at least, and I don't know when I will
have the time to revisit this. So although the rest of the code is
fine I prefer to back this out completely and not leave a only half
working implementation in the code for an indeterminate time.
Bug: 553083
Change-Id: Icbbbb09882b3b83f9897deac4a06d5f8dc99d84e
Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>
Make all transports request protocol V2 when fetching. Depending on
the transport, set the GIT_PROTOCOL environment variable (file and
ssh), pass the Git-Protocol header (http), or set the hidden
"\0version=2\0" (git anon). We'll fall back to V0 if the server
doesn't reply with a version 2 answer.
A user can control which protocol the client requests via the git
config protocol.version; if not set, JGit requests protocol V2 for
fetching. Pushing always uses protocol V0 still.
In the API, there is only a new Transport.openFetch() version that
takes a collection of RefSpecs plus additional patterns to construct
the Ref prefixes for the "ls-refs" command in protocol V2. If none
are given, the server will still advertise all refs, even in protocol
V2.
BasePackConnection.readAdvertisedRefs() handles falling back to
protocol V0. It newly returns true if V0 was used and the advertised
refs were read, and false if V2 is used and an explicit "ls-refs" is
needed. (This can't be done transparently inside readAdvertisedRefs()
because a "stateless RPC" transport like TransportHttp may need to
open a new connection for writing.)
BasePackFetchConnection implements the changes needed for the protocol
V2 "fetch" command (simplified ACK handling, delimiters, section
headers).
In TransportHttp, change readSmartHeaders() to also recognize the
"version 2" packet line as a valid smart server indication.
Adapt tests, and run all the HTTP tests not only with both HTTP
connection factories (JDK and Apache HttpClient) but also with both
protocol V0 and V2. Do the same for the SSH transport tests.
Bug: 553083
Change-Id: Ice9866aa78020f5ca8f397cde84dc224bf5d41b4
Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
This problem occurred when calling SubmoduleWalk#getModuleName if the
first submodule processed has a name and a path which do not match
SubmoduleWalk#getModuleName returned the module path instead of the
module name. In order to fix this SubmoduleWalk#getModuleName needs to
ensure that the modules config is loaded.
Bug: 565776
Change-Id: I36ce1fbc64c4849f9d8e39864b825c6e28d344f8
Signed-off-by: John Dallaway <john@dallaway.org.uk>
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
Add new performance logging to register events of type duration.
The proposed logging is similar to the performance logging
in OS Gerrit https://gerrit-review.googlesource.com/c/gerrit/+/225628:
a global Singleton (LoggingContext in Gerrit) is
collecting the performance logs in a thread-safe events list,
and at the end of the monitored command the list of events is
retrieved and written to a log, after which it is cleared.
What this patch does:
The main component is the Singleton (PerformanceLogContext), which
is used to collect the records (PerformanceLogRecord) in one place
(ThreadLocal eventsList) from anywhere using
PerformanceLogContext.getInstance().addEvent().
Reason why this change is needed:
The current monitoring in JGit has several issues:
1. git fetch and git push events are handled separately
(PackStatistics and ReceivedPackStatistics), with no unified way
of writing or reading the statistics.
2. PostUploadHook is only invoked on the event of sending the
pack, which means that the PackStatistics is not available for
the fetch requests that did not end with sending the pack
(negotiation requests).
3. The way the logs are created is different from the performance
log approach, so the long-running operations need to be collected
from both performance log (for JGit DFS overridden operations and
Gerrit operations) and gitlog (for JGit ones).
The proposed performance logging is going to solve the above
mentioned issues: it collects all of the performance logs in one
place, thus accounting for the commands that do not result in
sending a pack. The logs are compatible with the ones on Gerrit.
Moreover, the Singleton is accessible anywhere in the call stack,
which proved to be successful in other projects like Dapper
(https://research.google/pubs/pub36356/).
Signed-off-by: Alexa Panfil <alexapizza@google.com>
Change-Id: Iabfe667a3412d8a9db94aabb0f39b57f43469c41
This enables jgit to use any refs in the refs/ namespace when describing
commits.
Signed-off-by: Jason Yeo <jasonyeo88@gmail.com>
Change-Id: I1fa22d1c39c0e2f5e4c2938c9751d8556494ac26
Reason why this change is needed:
Currently the durations of fetch events are computed by
registering time instants with System.currentTimeMillis()
and calculating the differences with simple minus operation,
but multiple sources suggest that the best practice is to use
the Java 8 Duration and Instant objects.
What this patch does:
Get time measurements with Instant.now() instead of
System.currentTimeMillis() and calculate the duration of fetch
events (Reachability checks and Negotiation) using
Duration.between().toMillis().
Signed-off-by: Alexa Panfil <alexapizza@google.com>
Change-Id: I573a0a0562070083cf5a5a196d9710f69a7fa587
Building with Bazel is failing with this error message:
HttpSupport.java:480: error: [OperatorPrecedence] Use grouping
parenthesis to make the operator precedence explicit
if (c >= 'a' && c <= 'z' || c >= 'A' && c <= 'Z') {
^
(see https://errorprone.info/bugpattern/OperatorPrecedence)
Did you mean 'if ((c >= 'a' && c <= 'z') || (c >= 'A' && c <= 'Z')) {'?
Change-Id: I96089d3158d06ba981cfdf2b03865261b328de23
Validate the extra headers and log but otherwise ignore invalid
headers. An empty http.extraHeader starts the list afresh.
The http.userAgent is restricted to printable 7-bit ASCII, other
characters are replaced by '.'.
Moves a support method from the ssh.apache bundle to HttpSupport in
the main JGit bundle.
Bug:541500
Change-Id: Id2d8df12914e2cdbd936ff00dc824d8f871bd580
Signed-off-by: James Wynn <james@jameswynn.com>
Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>
This enables EGit to override with a lenient variant that logs the
problem and continues with the default value. EGit needs this because
otherwise a corrupt core.excludesFile entry can render the whole UI
unusable (until the git config is fixed) because any use of the
WorkingTreeIterator will throw an InvalidPathException.
This is not a problem on OS X, where all characters are allowed in
file names. But on Windows some characters are forbidden... see bug
567296. The message of the InvalidPathException is not helpful since
it doesn't point to the origin of the problem. EGit can log a much
better message indicating the offending config file and entry in the
Eclipse error log, where the user can see it.
Bug: 567309
Change-Id: I4e57afa715ff3aaa52cd04b5733f69e53af5b1e0
Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>
Reason why this change is needed:
Getting this metric will help estimate how much time will be saved once
the reachability checks get optimized
What this patch does:
Measure time spent by requestValidator.checkWants() in parseWants() and save
it in an instance of PackStatistics.Accumulator.
Signed-off-by: Alexa Panfil <alexapizza@google.com>
Change-Id: Id7fe4016f96549d9511a2c24052dad93cfbb31a4
Reason why this change is needed:
Getting this metric will help estimate how much time is spent
on negotiation in fetch V2.
What this patch does:
Measure time spent on negotiation rounds in UploadPack.fetchV2()
and save it in an instance of PackStatistics.Accumulator.
This is the same way the statistics are already gathered for
protocol V0/V1.
Change-Id: I14e55dd6ff743cb0b21b4953b533269ef069abb1
When comparing git directory paths to check whether one is a prefix
of another, one must add a slash to avoid false prefix matches when
one directory name is a prefix of another. The path "audio" is not
a prefix of the path "audio-new", but would be a prefix of a path
"audio/new".
Bug: 566799
Change-Id: I6f671ca043c7c2c6044eb05a71dc8cca8d0ee040
Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>
This is useful to access git repositories behind a bastion server
(jump host).
Add a constant for the config; rewrite the whole connection initiation
to parse the value and (recursively) set up the chain of hops. Add
tests for a single hop and two different ways to configure a two-hop
chain.
The connection timeout applies to each hop in the chain individually.
Change-Id: Idd25af95aa2ec5367404587e4e530b0663c03665
Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>
Since there is no negotiation for a push, the client is probably sending
redundant objects and bytes which already exist in the server.
Add more metrics in the stats to quantify it. Duplicated size and number
to measure the size and the number of duplicated objects which should
not be pushed.
Change-Id: Iaacd4761ee9366a0a7ec4e26c508eff45c8744de
Signed-off-by: Yunjie Li <yunjieli@google.com>
* stable-5.9:
Prepare 5.9.1-SNAPSHOT builds
JGit v5.9.0.202009080501-r
[releng] Enable japicmp for the fragments added in 5.8.0
GitlinkMergeTest: fix boxing warnings
Remove unused API problem filters
Add missing since tag on BundleWriter#addObjectsAsIs
GPG: include signer's user ID in the signature
Change-Id: Iaa96f9228752540f446fc232a49f31a738fd8d30
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
* master:
SshdSession: close channel gracefully
jgit: Add DfsBundleWriter
Prepare 5.10.0-SNAPSHOT builds
ResolveMerger: do not content-merge gitlinks on del/mod conflicts
ResolveMerger: Adding test cases for GITLINK deletion
ResolveMerger: choose OURS on gitlink when ignoreConflicts
ResolveMerger: improving content merge readability
ResolveMerger: extracting createGitLinksMergeResult method
ResolveMerger: Adding test cases for GITLINK merge
Back out the version change to 5.10.0-SNAPSHOT which was done on master
already.
Change-Id: I1a6b1f0b8f5773be47823d74f593d13b16a601d5
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
DfsBundleWriter writes out the entire repository to a Git bundle file.
It packs all objects included in the packfile by concatenating all pack
files. This makes the bundle creation fast and cheap. Useful for backing
up a repository as-is.
Change-Id: Iee20e4b1ab45b2a178dde8c72093c0dd83f04805
Signed-off-by: Masaya Suzuki <masayasuzuki@google.com>
* changes:
ResolveMerger: do not content-merge gitlinks on del/mod conflicts
ResolveMerger: Adding test cases for GITLINK deletion
ResolveMerger: choose OURS on gitlink when ignoreConflicts
ResolveMerger: improving content merge readability
ResolveMerger: extracting createGitLinksMergeResult method
ResolveMerger: Adding test cases for GITLINK merge
* stable-5.9:
Upgrade maven-resources-plugin to 3.2.0
Upgrade plexus-compiler version to 2.8.8
[bazel] Add missing dependency to slf4j-api
[errorprone] DirCacheEntry: make clear operator precedence
[errorprone] PackWriter#parallelDeltaSearch: avoid suppressed exception
[errorprone] Declare DirCache#version final
Add jgit-4.17-staging target platform for 2020-09
Update target platform to R20200831200620
Signed-off-by: Marco Miller <marco.miller@ericsson.com>
Change-Id: I2e2f41cf6ebbcb45b8978b519db3f1c8f7afb5f4
Previously ResolveMerger tried to make a fulltext merge entry in case
one of sides got deleted regardless of file mode. This is not
applicable for GITLINK type of entry. After this change it is
rendering appropriate merge result.
Signed-off-by: Demetr Starshov <dstarshov@google.com>
Change-Id: Ibdb4557bf8781bdb48bcee6529e37dc80582ed7e
Option ignoreConflicts is used when a caller want to create a virtual
commit and use it in a future merge (recursive merge) or show it on
UI (e.g. Gerrit). According to contract in case of ignoreConflicts
ResolveMerger should populate only stage 0 for entries with merge
conflicts as if there is no conflict. Current implementation breaks
this contract for cases when gitlink revision is ambiguous.
Therefore, always select 'ours' when we merge in ignoreConflicts mode.
This will satisfy the contract contract, so recursive merge can
succeed, however it is an arbitrary decision, so it is not guaranteed
to select best GITLINK in all cases.
GITLINK merging is a special case of recursive merge because of
limitations of GITLINK type of entry. It can't contain more than 1 sha-1
so jgit can't write merge conflicts in place like it can with a blob.
Ideally we could signal the conflict with a special value (like
'0000...'), but that must be supported by all tooling (git fsck, c-git)."
Signed-off-by: Demetr Starshov <dstarshov@google.com>
Change-Id: Id4e9bebc8e828f7a1ef9f83259159137df477d89
Due to an integer overflow bug, the current "Index file is too large
for jgit" check did not work properly and subsequently a
NegativeArraySizeException was raised.
Change-Id: I2736efb28987c29e56bc946563b7fa781898a94a
Signed-off-by: Marc Strapetz <marc.strapetz@syntevo.com>
Previous code used a minimum granularity of 1 microsecond and would
iterate 233 times on a system where the resolution is 1 second (for
instance, Java 8 on Mac APFS).
New code uses a binary search between the maximum we care about (2
seconds) and zero, with a minimum granularity of also 1 microsecond.
This takes at most 19 iterations (guaranteed). For a file system with 1
second resolution, it takes 4 iterations (1s, 0.5s, 0.8s, 0.9s). With
an up-front check at 1 microsecond and at 1 millisecond this performs
equally well as the old code on file systems with a fine resolution.
(For instance, Java 11 on Mac APFS.)
Also handle obscure cases where the file timestamp implementation may
yield bogus values (as observed on HP NonStop). If such an error case
occurs, log a warning and abort the measurement at the last good value.
Bug: 565707
Change-Id: I82a96729b50c284be7c23fbdf3d0df1bddf60e41
Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>
LockFile.lock() will create it anyway when the config file is created.
Bug: 565637
Change-Id: I078b89a695193fd76f130f6de7ac1cf26d2f8f0f
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>
Otherwise locking problems may ensue if the JGit config itself is
on a different file system. Since the internal is already updated,
it is not really important when exactly the value gets persisted.
By queueing up separate Runnables executed by a single thread we
avoid concurrent write access to the JGit config, and nested calls
to getFileStoreAttributes(Path) result in serialized attempts to
write.
The thread for writing the config must not be a daemon thread. If
it were, JVM shutdown might kill it anytime, which may lead to
the config not being written, or worse, a config.lock file being
left behind.
Bug: 566170
Change-Id: I07e3d4c5e029d3cec9ab5895002fc4e0c7948c40
Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>
If the background job is a little late, the true result might
arrive and be cached later. So make sure we don't cache the large
fallback resolution in the per-directory cache. Otherwise we'd work
with the large fallback until the next restart.
Bug: 566170
Change-Id: I7354a6cfddfc0c05144bb0aa41c23029bd4f6af0
Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>
Git never converts line endings if the version in the repository is a
text file with CR/LF and text=auto. See [1]: "When the file has been
committed with CRLF, no conversion is done."
Because the sentence just before is about converting line endings on
check-in, I had understood that in commit 60cf85a [2] to mean that no
conversion on check-in was to be done. However, as bug 565048 and a
code inspection of the C git code showed it really means no conversion
is done on check-in *or check-out*.
If the text attribute is not set but core.autocrlf = true, this is
the same as text=auto eol=crlf. C git does not convert on check-out
even on text=auto eol=lf if the index version is a text file with
CR/LF.
For check-in, one has to look at the intended target, which is done
in WorkingTreeIterator since commit 60cf85a. For check-out, it can
be done by looking at the source and can thus be done in the
AutoLFOutputStream.
Additionally, provide a constructor for AutoLFInputStream to do
the same; for cases where the equivalent of a check-out is done via
an input stream obtained from a blob. (EGit does that in its
GitBlobStorage for the Eclipse compare framework; it's more efficient
than using a TemporaryBuffer and DirCacheCheckout.getContent(), and
it avoids the need for a temporary file.)
Adapt existing tests, and add new checkout and merge tests to verify
the resulting files have the correct line endings.
EGit's GitBlobStorage will need to call the new version of
EolStreamTypeUtil.wrapInputStream().
[1] https://git-scm.com/docs/gitattributes#Documentation/gitattributes.txt-Settostringvalueauto
[2] https://git.eclipse.org/r/c/jgit/jgit/+/127324
Bug: 565048
Change-Id: If1282ef43e2abd00263541bd10a01fe1f5c619fc
Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>
The WindowCache is configured statically with a default
WindowCacheConfig. The default config says (for backwards
compatibility reasons) to publish the MBean. As a result,
the MBean always gets published.
By delaying the MBean registration until the first call to
getInstance() or get(PackFile, long) we can avoid the forced
registration and do it only if not re-configured in the meantime
not to publish the bean. (As is done by Egit, to avoid a very
early costly access to the user and system config during plug-in
activation.)
Bug: 563740
Change-Id: I8a941342c0833acee2107515e64299aada7e0520
Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>
Index format version 4 was introduced in C git in 2012. It's about
time that JGit can deal with it.
Version 4 added prefix path compression. Instead of writing the full
path for each index entry to disk, only the difference to the previous
entry's path is written: a variable-encoded int telling how many bytes
to remove from the previous entry's path to get the common prefix,
followed by the new suffix.
Also, cache entries in a version 4 index are not padded anymore.
Internally, version 3 and version 4 index entries are identical; it's
only the stored format that changes.
Implement this path compression, and make sure we write an index file
that we read previously in the same format. (Only changing from version
2 to version 3 if there are extended flags.)
Add support for the "feature.manyFiles" and the "index.version" git
configs, and honor them when writing a new index file.
Add tests, including a compatibility test that verifies that JGit can
read a version 4 index generated by C git and write an identical
version 4 index.
Bug: 565774
Change-Id: Id83241cf009e50f950eb42f8d56b834fb47da1ed
Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>
The timeout on RemoteSession.exec() cannot be a timeout for the
whole command. It can only be a timeout for setting up the process;
after that it's the application's responsibility to implement some
timeout for the execution of the command, for instance by calling
Process.waitFor(int, TimeUnit) or through other means.
Sessions returned by an SshSessionFactory are already connected and
authenticated -- they must be, because RemoteSession offers no
operations for connecting or authenticating a session.
Change the implementation of SshdExecProcess.waitFor() to wait
indefinitely. The original implementation used the timeout from
RemoteSession.exec() because of that erroneous javadoc.
Change-Id: I3c7ede24ab66d4c81f72d178ce5012d383cd826e
Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>
SshSupport.runSshCommand() had a comment that wait with time-out
could not be used because JSchProcess.exitValue() threw the wrong
unchecked exception when the process was still running.
Fix this and make JSchProcess.exitValue() throw the right exception,
then wait with a time-out in SshSupport.
The Apache sshd client's SshdExecProcess has always used the correct
IllegalThreadStateException.
Add tests for SshSupport.runCommand().
Change-Id: Id30893174ae8be3b9a16119674049337b0cf4381
Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>
If I create a repository containing an empty file and clone it
with
git clone --no-checkout --filter=blob:none \
https://url/of/repository
then I would expect no blobs to be transferred over the wire. Alas,
JGit rewrites filter=blob:none to filter=blob:limit=0, so if the
repository contains an empty file then the empty blob gets
transferred.
Fix it by teaching JGit about filters based on object type to
complement the existing filters based on object size. This prepares
us for other future filters such as object:none.
In particular, this means we do not need to look up the size of the
filtered blobs, which should speed up clones. Noticed by Anna
Pologova and Terry Parker.
Change-Id: Id4b234921a190c108d8be2c87f54dcbfa811602a
Signed-off-by: Jonathan Nieder <jrn@google.com>
Teach the FilterSpec serialization code about tree filters so they can
be communicated over the wire and understood by the server.
While we're here, harden the FilterSpec serialization code to throw
IllegalStateException if we encounter a FilterSpec that cannot be
expressed as a "filter" line. The only public API for creating a
Filterspec is to pass in a "filter" line to be parsed, so these should
not appear in practice.
Change-Id: I9664844059ffbc9c36eb829e2d860f198b9403a0
Signed-off-by: Jonathan Nieder <jrn@google.com>
OS X ships with a default /usr/bin/git that is just a wrapper that
at run-time delegates to the selected XCode toolchain, and that
prompts the user to install the XCode command line tools if not
already installed.
This is annoying for people who don't want to do so, since they'll
be prompted on each Eclipse start. Also, since on OS X the $PATH for
applications started via the GUI is not the same as the $PATH as set
via the shell profile, just using /usr/bin/git (which will normally
be found when JGit runs inside Eclipse) may give slightly surprising
results if the user has installed a non-Apple git and changed his
$PATH in the shell such that the non-Apple git is used in the shell.
(For instance by placing /usr/local/bin earlier on the path.) Eclipse
and the shell will use different git executables, and thus different
git system configs.
Therefore, try to find git via bash --login -c 'which git' not only
if we couldn't find it on $PATH but also if we found the default git
/usr/bin/git. If that finds some other git, use that. If the bash
approach also finds /usr/bin/git, double check via xcode-select -p
that an XCode git is present. If not, assume there is no git installed,
and work without any system config.
Bug: 564372
Change-Id: Ie9d010ebd9437a491ba5d92b4ffd1860c203f8ca
Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>
Using a fixed thread pool with unbounded LinkedBlockingQueue fixes the
RejectedExecutionException thrown if too many threads try to
concurrently determine filesystem attributes.
Comparing that to an alternative implementation using an unbounded
thread pool instead showed similar performance with the reproducer (in
range of 100-1000 threads in reproducer) on my mac:
threads time
fixed threadpool up to 5 threads with LinkedBlockingQueue of unlimited
queue size
100 1103 ms
200 1602 ms
300 2369 ms
500 4002 ms
1000 11071 ms
unbounded cached threadpool
100 1108 ms
200 1591 ms
300 2299 ms
500 4577 ms
1000 11196 ms
Bug: 564202
Change-Id: I773da7414a1dca8e548349442dca9b56643be946
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
Include implementation version in jgit library. This version is used
by other products that depend on JGit, and built using Bazel and not
consume officially released artifact from Central or Eclipse own Maven
repository.
Most notably, in Gerrit Code Review JGit agent that was previously
reported as "unknown", is now reported as:
JGit/v5.8.0.202006091008-r-16-g14c43828d
using this change [1].
[1] https://gerrit-review.googlesource.com/c/gerrit/+/272505
Change-Id: Ia50de9ac35b8dbe9e92d8ad7d0d14cd00f057863
Signed-off-by: David Ostrovsky <david@ostrovsky.org>
In JGit 5.0, the FileTreeIterator was changed to skip ignored folders
by default. To catch tracked files inside ignored folders, the tree
walk needs to have a DirCacheIterator, and the FileTreeIterator has
to know about that DirCacheIterator via setDirCacheIterator(). (Or
the optimization has to be switched off explicitly via
setWalkIgnoredDirectories(true).)
Skipping ignored directories is an important optimization in some
cases, for instance in node.js/npm projects, where we'd otherwise
traverse the whole huge and deep hierarchy of the typically ignored
node_modules folder.
While all uses of WorkingTreeIterator in JGit had been adapted,
DiffFormatter was forgotten. To make it work correctly (again) also
for such cases, make it set up a WorkingTreeeIterator automatically,
and make sure the WorkingTreeSource can find such files, too. Also
pass the repository to the TreeWalks used inside the DiffFormatter
to pick up the correct attributes, filters, and line-ending settings.
Bug: 565081
Change-Id: Ie88ac81166dc396ba28b83313964c1712b6ca199
Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>
Make sure we don't produce a spurious empty line at the end.
Bug: 564428
Change-Id: Ib991d93fbd052baca65d32a7842f07f9ddeb8130
Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>
The message "Too many commands" implies there is a hard limit on the
number of commands, which isn't the case. The limit is on the total
size of the received data, as explained in change I84317d396 which
introduced the configuration setting receive.maxCommandBytes:
shorter reference names allow for more commands, longer reference
names permit fewer commands per batch.
Change the message to:
Commands size exceeds limit defined in receive.maxCommandBytes
Change-Id: I678b78f919b2fec8f8058f3403f2541c26a5d00e
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
MergedReftable ignores the last reftable in the stack while calculating the
minUpdateIndex.
Update the loop indices to include all reftables in the minUpdateIndex
calculation, while skipping position 0 as it is read outside the loop.
Change-Id: I12d3e714581e93d178be79c02408a67ab2bd838e
Signed-off-by: Minh Thai <mthai@google.com>
Currently we're buffering the inflated bitmap entry in
BasePackBitmapIndex to optimize running time. However, this will use
lots of memory during the creation of the pack bitmap index file.
And change 161456, which rewrote the entire getBitmap method, increased
the fetch latency significantly.
This commit introduces getBitmapWithoutCaching method which is used in
the pack bitmap index file creation only and aims to save memory during
garbage collection and not increase fetch latency.
Change-Id: I7b982c9d4e38f5f6193eaa03894e894ba992b33b
Signed-off-by: Yunjie Li <yunjieli@google.com>
The current mechanism for updating the unpack error handler requires
that the error handler is replaced entirely, including communicating
the error to the user. Adding a getter means that delegating
implementations can be constructed so that the error can be processed
before sending to the user, for example for logging.
Change-Id: I4b6f78a041d0f6f5b4076a9a5781565ca3857817
Signed-off-by: Jack Wickham <jwickham@palantir.com>
* stable-5.7:
ObjectDirectoryInserter: Open FileOutputStream in try-with-resource
ObjectDirectoryInserter: Remove redundant 'throws' declarations
ObjectDirectory: Further clean up insertUnpackedObject
ObjectDirectory: Explicitly handle NoSuchFileException
ObjectDirectory: Fail immediately when atomic move is not supported
Change-Id: I05186baa517388680fcc6825c940c4c772f26d32
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
ObjectWritingException and FileNotFoundException are subclasses
of IOException, which is already declared. Error does not need
to be explicitly declared.
Change-Id: I879820a33e10ec3a7ef676adc9c9148d2b3c4b27
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
- The code to move the file is repeated. Split it out into a
utility method.
- Remove the catch block for AtomicMoveNotSupportedException which
is redundant because it's handled in exactly the same way as the
IOException further down. The only exception we need to explicitly
handle differently in this block is NoSuchFileException.
- Improve the comments.
Change-Id: Ifc5490953ffb25ecd1c48a06289eccb3f19910c6
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
The shutdown method releases
- ThreadLocal held by NLS
- GlobalBundleCache used by NLS
- Executor held by WorkQueue
Bug: 437855
Bug: 550529
Change-Id: Icfdccd63668ca90c730ee47a52a17dbd58695ada
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
If a hunk does not apply at the position stated in the hunk header
try to determine its position using the old lines (context and
deleted lines).
This is still a far cry from a full git apply: it doesn't do binary
patches, it doesn't handle git's whitespace options, and it's perhaps
not the fastest on big patches. C git hashes the lines and uses these
hashes to speed up matching hunks (and to do its whitespace magic).
Bug: 562348
Change-Id: Id0796bba059d84e648769d5896f497fde0b787dd
Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>
Running recent error prone version complaining on that code:
CharacterHead.java:22: error: [ProtectedMembersInFinalClass] Make
members of final classes package-private: <init>
protected CharacterHead(char expectedCharacter) {
^
(see https://errorprone.info/bugpattern/ProtectedMembersInFinalClass)
Did you mean 'CharacterHead(char expectedCharacter) {'
Bug: 562756
Change-Id: Ic46a0b07e46235592f6e63db631f583303420b73
Signed-off-by: David Ostrovsky <david@ostrovsky.org>
On the first attempt to move the temp file, NoSuchFileException can
be raised if the destination folder does not exist. Instead of handling
this implicitly in the catch of IOException and then continuing to
create the destination folder and try again, explicitly catch it and
create the destination folder. If any other IOException occurs, treat
it as an unexpected error and return FAILURE.
Subsequently, on the second attempt to move the temp file, if ANY kind
of IOException occurs, also consider this an unexpected error and
return FAILURE.
In both catch blocks for IOException, add logging at ERROR level.
Change-Id: I9de9ee3d2b368be36e02ee1c0daf8e844f7e46c8
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
If atomic move is not supported, AtomicMoveNotSupportedException will
be thrown on the first attempt to move the temp file. There is no
point attempting the move operation a second time because it will only
fail for the same reason.
Add an immediate return of FAILURE on the first occasion. Remove the
unnecessary handling of the exception in the second block.
Change-Id: I4658a8b37cfec2d7ef0217c8346e512968d0964c
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
Running recent error prone version complaining on that code:
RefDatabase.java:444: error: [InvalidInlineTag] Tag name `linkObjectId`
is unknown.
* Includes peeled {@linkObjectId}s. This is the inverse lookup of
^
(see https://errorprone.info/bugpattern/InvalidInlineTag)
Bug: 562756
Change-Id: If91da51d5138fb753c0550eeeb9e3883a394123d
Signed-off-by: David Ostrovsky <david@ostrovsky.org>
Motivation: JSch serves as 'default' implementations of the SSH
transport. If a client application does not use it then there is no need
to pull in this dependency.
Move the classes depending on JSch to an OSGi fragment extending the
org.eclipse.jgit bundle and keep them in the same package as before
since moving them to another package would break API. Defer moving them
to a separate package to the next major release.
Add a new feature org.eclipse.jgit.ssh.jsch feature to enable
installation. With that users can now decide which of the ssh client
integrations (JCraft JSch or Apache Mina SSHD) they want to install.
We will remove the JCraft JSch integration in a later step due to the
reasons discussed in bug 520927.
Bug: 553625
Change-Id: I5979c8a9dbbe878a2e8ac0fbfde7230059d74dc2
Also-by: Michael Dardis <git@md-5.net>
Signed-off-by: Michael Dardis <git@md-5.net>
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
Signed-off-by: David Ostrovsky <david@ostrovsky.org>
Motivation: BouncyCastle serves as 'default' implementation of
the GPG Signer. If a client application does not use it there is no need
to pull in this dependency, especially since BouncyCastle is a large
library.
Move the classes depending on BouncyCastle to an OSGi fragment extending
the org.eclipse.jgit bundle. They are moved to a distinct internal
package in order to avoid split packages. This doesn't break public API
since these classes were already in an internal package before this
change.
Add a new feature org.eclipse.jgit.gpg.bc to enable installation. With
that users can now decide if they want to install it.
Attempts to sign a commit if org.eclipse.jgit.gpg.bc isn't available
will result in ServiceUnavailableException being thrown.
Bug: 559106
Change-Id: I42fd6c00002e17aa9a7be96ae434b538ea86ccf8
Also-by: Michael Dardis <git@md-5.net>
Signed-off-by: Michael Dardis <git@md-5.net>
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
Signed-off-by: David Ostrovsky <david@ostrovsky.org>
If the determination of the user home directory produces a Java File
object with an invalid path, spurious exceptions may occur at the
most inopportune moments anytime later. In the case in the linked bug
report, start-up of EGit failed, leading to numerous user-visible
problems in Eclipse.
So validate the return value of FS.userHomeImpl(). If converting that
File to a Path throws an exception, log the problem and fall back to
Java system property user.home. If that also is not valid, use null.
(A null user home directory is allowed by FS, and calling in Java
new File(null, "some_string") is fine and produces a File relative
to the current working directory.)
Bug: 563739
Change-Id: If9eec0f9a31a45bd815231706285c71b09f8cf56
Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>
Make it possible to programmatically suppress the JMX bean
registration. In EGit it is not needed but can be rather costly
because it occurs during plug-in activation and accesses the
git user config.
Bug: 563740
Change-Id: I07ef7ae2f0208d177d2a03862846a8efe0191956
Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>
Only the presence or absence of whitespace is significant; but not the
actual whitespace characters. Don't compare whitespace bytes.
Compare the C git implementation at [1].
[1] https://github.com/git/git/blob/0d0e1e8/xdiff/xutils.c#L173
Bug: 563570
Change-Id: I2d0522b637ba6b5c8b911b3376a9df5daa9d4c27
Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>
This reverts commit 3aee92478c, which
increased fetch latency significantly.
Change-Id: Id31a94dff83bf7ab2121718ead819bd08306a0b6
Signed-off-by: Yunjie Li <yunjieli@google.com>
A builder API provides a more convenient way to define a customized
SshdSessionFactory by hiding the subclassing.
Also provide a new interface SshConfigStore to abstract away the
specifics of reading a ssh config file, and provide a way to customize
the concrete ssh config implementation to be used. This facilitates
using an alternate ssh config implementation that may or may not be
based on files.
Change-Id: Ib9038e8ff2a4eb3a9ce7b3554d1450befec8e1e1
Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>
Avoid trying other authentication methods on SocketException or on
InterruptedIOException. SocketException is rather fatal, such as
nothing listening on the peer's port, connection reset, or it could
be a connection time-out.
Time-outs enforced by Timeout{Input,Output}Stream may result in
InterruptedIOException being thrown.
In both cases, it makes no sense to try other authentication methods,
and doing so may wrongly report "authentication not supported" or
"cannot open git-upload-pack" or some such instead of reporting a
time-out.
Bug: 563138
Change-Id: I0191b1e784c2471035e550205abd06ec9934fd00
Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>
Config core.eol is to be ignored if core.autocrlf is true or input.[1]
JGit didn't do so when core.autocrlf=input was set.
[1] https://git-scm.com/docs/git-config#Documentation/git-config.txt-coreeol
Bug: 561877
Change-Id: I5e62e0510d160b5113c1090319af09c2bc1bcb59
Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>
In Git 2.10.0 the interpretation of gitattributes changed or was fixed
such that "* text=auto eol=crlf" would indeed still do auto-detection
of text vs. binary content.[1] Previously this was identical to
"* text eol=crlf", i.e., treating all files as text.
JGit still did the latter, which caused surprises because it changed
binary files.
[1] https://github.com/git/git/blob/master/Documentation/RelNotes/2.10.0.txt#L248
Bug: 561341
Change-Id: I5b6fb97b5e86fd950a98537b6b8574f768ae30e5
Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>
Update dependency to Bouncy Castle to 1.65.
Add the IssuerFingerprint as a hashed sub-packet in the signature. If
added unhashed, GPG ignores it.
Bug: 553206
Change-Id: I6807e8e2385e6ec5790f388e4753a44aa9474ebb
Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>
OSGi semantic versioning allows breaking implementers in a minor
release.
Change-Id: Ib55dc43dd3b50b0ef39a7094190f230210aee4b6
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
* changes:
PackBitmapIndex: Set distance threshold
PackBitmapIndex: Not buffer inflated bitmap in BasePackBitmapIndex
PackBitmapIndex: Remove convertedBitmaps in the Remapper
PackBitmapIndex: Reduce memory usage in GC
PackBitmapIndex: Add AddToBitmapWithCacheFilter class
PackBitmapIndex: Add util methods and builder to BitmapCommit
PackBitmapIndex: Move BitmapCommit to a top-level class
Refactor: Make retriveCompressed an method of the Bitmap class
Setting the distance threshold to 2000 in PackWriterBitmapPreparer to
reduce memory usage in garbage collection. When the threshold is 0, GC
for the msm repository would use about 37 GB memory to complete. After
setting it to 2000, GC can finish in 75 min with about 10 GB memory.
Change-Id: I39783eeecbae58261c883735499e61ee1cac75fe
Signed-off-by: Yunjie Li <yunjieli@google.com>
Currently we're buffering the inflated bitmap entry in BasePackBitmapIndex
to optimize running time. However, this will use lots of memory during
the construction of the pack bitmap index file which may cause failure of
garbage collection.
The running time didn't increase significantly, if there's any increase,
after removing the buffering here. The report about usage of time/memory
will come in the next commit.
Change-Id: I874503ecc85714acab7ca62a6a7968c2dc0b56b3
Signed-off-by: Yunjie Li <yunjieli@google.com>
The convertedBitmaps serves for time-optimization purpose. But it's
actually not saving time much but using lots of memory. So remove the
field here to save memory.
Currently the remapper class is only used in the construction of the
bitmap index file. And during the preparation of the file, we're only
getting bitmaps from the remapper when finding objects accessible from
a commit, so bitmap associated with each commit will only be fetched once
and thus the convertedBitmaps would hardly be read, which means that it's
not saving time.
Change-Id: Ic942a8e485135fb177ec21d09282d08ca6646fdb
Signed-off-by: Yunjie Li <yunjieli@google.com>
Currently, the garbage collection is consistently failing for some large
repositories in the building bitmap phase, e.g.Linux-MSM project:
https://source.codeaurora.org/quic/la/kernel/msm-3.18
Historically, bitmap index creation happened in 3 phases:
1. Select the commits to which bitmaps should be attached.
2. Create all bitmaps for these commits, stored in uncompressed format
in the PackBitmapIndexBuilder.
3. Deltify the bitmaps and write them to disk.
We investigated the process. For phase 2 it's most efficient to create
bitmaps starting with oldest commit and moving to the newest commit,
because the newer commits are able to reuse the work for the old ones.
But for bitmap deltification in phase 3, it's better when a newer
commit's bitmap is the base, and the current disk format writes bitmaps
out for the newest commits first.
This change introduces a new collection to hold the deltified and
compressed representations of the bitmaps, keeping a smaller subset of
commits in the PackBitmapIndexBuilder to help make the bitmap index
creation more memory efficient.
And in this commit, we're setting DISTANCE_THRESHOLD to 0 in the
PackWriterBitmapPreparer, which means the garbage collection will not
have much behavoir change and will still use as much memory as before.
Change-Id: I6ec2c3e8dde11805af47874d67d33cf1ef83660e
Signed-off-by: Yunjie Li <yunjieli@google.com>
Add a new revwalk filter, AddToBitmapWithCachedFilter. This filter updates
a client-provided {@code BitmapBuilder} as a side effect of a revwalk.
Similar to {@code AddToBitmapFilter}, it short circuits the walk when it
encounters a commit which is included in the provided bitmap's BitmapIndex.
It also short circuits the walk if it encounters the client-provided
cached commit.
Change-Id: I62cb503016f4d3995d648d92b82baab7f93549a9
Signed-off-by: Yunjie Li <yunjieli@google.com>
Add some utility methods and a builder class for BitmapCommit class in
preparation for improving the memory footprint of GC's bitmap generation
phase.
Change-Id: Ice3d257fc26f3917a65a64eaf53b508b89043caa
Signed-off-by: Yunjie Li <yunjieli@google.com>
Move BitmapCommit from inside the PackWriterBitmapPreparer to a new
top-level class in preparation for improving the memory footprint of GC's
bitmap generation phase.
Change-Id: I4d404a5b3a34998b441d23105197f33d32d39670
Signed-off-by: Yunjie Li <yunjieli@google.com>
Make retrieveCompressed() a method of Bitmap interface to avoid type
casting and later reuse in improving the memory footprint of GC's bitmap
generation phase.
Change-Id: I098d85105cf17af845d43b8c71b4ca48b02fd7da
Signed-off-by: Yunjie Li <yunjieli@google.com>
When downloading LFS objects also accept response code 203 as successful
download. This response may be seen when downloading via a proxy.
Bug: 563022
Change-Id: Iee85fdb451b33369d08859872e5bfc2a67dffa6d
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
Introduce an IterativeConnectivityChecker which runs a connectivity
check with a filtered set of references, and falls back to using the
full set of advertised references.
It uses references during first check attempt:
- References that are ancestors of an incoming commits (e.g., pushing
a commit onto an existing branch or pushing a new branch based on
another branch)
- Additional list of references we know client can be interested in
(e.g. list of open changes for Gerrit)
We tested it inside Google and it improves connectivity for certain
topologies. For example connectivity counts for
chromium.googlesource.com/chromium/src:
percentile_50: 1923 (was: 22777)
percentile_90: 23272 (was: 353003)
percentile_99: 345522 (was: 353435)
This saved ~2 seconds on every push to this repository.
Signed-off-by: Demetr Starshov <dstarshov@google.com>
Change-Id: I6543c2e10ed04622ca795b195665133e690d3b10
Moving transport related internal classes into dedicated subpackage in
o/e/j/internal package.
Signed-off-by: Demetr Starshov <dstarshov@google.com>
Change-Id: I21ed029d359f5f7d8298f102efbb4b1dcdf404ad
Fix NullPointerException occurring when calling
CheckoutCommand with forced == true option when
the branch isn't changed and there is deleted
uncommitted file.
Change-Id: I99bf1fc25e6889f07092320d7bc2772ec5d341b5
Signed-off-by: Nail Samatov <sanail@yandex.ru>
When applying a patch that contains renames or copies using ApplyCommand,
also apply all hunks that apply to the renamed or copied file.
Change-Id: I9f3fa4370458bd7c14beeb2e2b49e846d70203cb
Signed-off-by: Jack Wickham <jwickham@palantir.com>
Before this change, applying a patch will fail if the destination directory
doesn't exist; after, the necessary parent directories are created.
If renaming the file fails, the directories won't be deleted, so this change
isn't atomic. However, ApplyCommand is already not atomic - if one hunk fails
to apply, other hunks still get applied - so I don't think that is a blocker.
Change-Id: Iea36138b806d4e7012176615bcc673756a82f365
Signed-off-by: Jack Wickham <jwickham@palantir.com>
ObjectReachabilityChecker interface is the only public API. The
implementation is instantiated by ObjectWalk and doesn't need to be
visible outside the package.
Change-Id: I5b97bb98990cded637686bdc15c9655330b7780f
Signed-off-by: Ivan Frade <ifrade@google.com>
The bitmap-bassed object reachability checker, tries to find the objects
in the first starter, then adding the second starter... and so on. This
rewards passing the most popular refs first.
Order the refs with heads first, then tags, then others (e.g. changes)
for the object reachability checker. Using streams, delay also the
resolution of the ref to RevObject until necessary.
Change-Id: I9414b76754d7c0ffee1e2eeed6939895c8e92cbe
Signed-off-by: Ivan Frade <ifrade@google.com>
ObjectWalk#createObjectReachabilityChecker() returns the best
implementation for the repo. UploadPack can use the interface and fold
the with/without commits cases in one code path.
Change-Id: I857c11735d1d8e36c3ed8185ff11de8a62e86540
Signed-off-by: Ivan Frade <ifrade@google.com>
Extract ObjectReachabilityChecker interface from the walk-based
implementation, to add a bitmapped based implementation later.
Refactor the test case to use it for both implementations.
Change-Id: Iaac7c6b037723811956ac22625f27d3b4d742139
Signed-off-by: Ivan Frade <ifrade@google.com>
Preparing the code to optimize the bitmap-based object reachability
checker. We are mirroring first the commit reachability checker
structure (interface + 2 implementations).
Move the walk-base reachability checker to its own class.
This class is public at the moment. Later ObjectWalk will return an
interface and this implementation will be package-private.
Change-Id: Ifac70094e1af137291c3607d95e689992f814b26
Signed-off-by: Ivan Frade <ifrade@google.com>
After negotiation phase of a fetch, the advertised ref map is no longer used and
can be safely cleared. For >1GiB repos object selection and packfile writing may
take 10s of minutes. For the chromium.googlesource.com/chromium/src repo, this
advertised ref map is >400MiB. Returning this memory to the Java heap is a major
scalability win.
Change-Id: I00d453c5ef47630c21f199e333e1cfcf47b7e92a
Signed-off-by: Minh Thai <mthai@google.com>
Multiple whitespaces are not normalized when reading properties files,
therefore leading to unwanted space/indentation in console or UI output.
Change-Id: I1f5224fe359e0cac493e0237872afc75dc8b9fbe
Signed-off-by: Michael Keppler <Michael.Keppler@gmx.de>
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
(cherry picked from commit ebbc3efce7)
Multiple whitespaces are not normalized when reading properties files,
therefore leading to unwanted space/indentation in console or UI output.
Change-Id: I1f5224fe359e0cac493e0237872afc75dc8b9fbe
Signed-off-by: Michael Keppler <Michael.Keppler@gmx.de>
Since reftables might have update index ranges that are overlapped.
Change-Id: I8f8215b99a0a978d4dd0155dbaf33e5e06ea8202
Signed-off-by: Minh Thai <mthai@google.com>
(cherry picked from commit 06748c205c)
Ensure files are writable before trying to delete them.
Bug: 408846
Change-Id: I930a547594bba853c33634ae54bd64d236afade3
Signed-off-by: Alexander Nittka <alex@nittka.de>
* stable-5.7:
FS.runInShell(): handle quoted filters and hooksPath containing blanks
Handle non-normalized index also for executable files
Prepare 5.7.1-SNAPSHOT builds
JGit v5.7.0.202003110725-r
Change-Id: I8a8580e44bfa05989d476cf22a029abd4fd407c6
Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>
* stable-5.6:
FS.runInShell(): handle quoted filters and hooksPath containing blanks
Handle non-normalized index also for executable files
Change-Id: I240377e87c073ee7a621a88e39fc319c59fa037a
Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>
Revert commit 2323d7a. Using $0 in the shell command call results in
the command string being taken literally. That was introduced to fix
a problem with backslashes, but is actually not correct.
First, the problem with backslashes occurred only on Win32/Cygwin,
and has been properly fixed in commit 6f268f8.
Second, this is used only for hooks (which don't have backslashes in
their names) and filter commands from the git config, where the user
is responsible for properly quoting or escaping such that the commands
work.
Third, using $0 actually breaks correctly quoted filter commands
like in the bug report. The shell really takes the command literally,
and then doesn't find the command because of quotes.
So revert this change.
At the same time there's a related problem with hooks. If the path to
the hook contains blanks, runInShell() would also fail to find the
hook. In this case, the command doesn't come from user input but is
just a Java File object with an absolute path containing blanks. (Can
occur if core.hooksPath points to such a path with blanks, or if the
repository has such a path.)
The path to the hook as obtained from the file system must be quoted.
Add a test for a hook path with a blank.
This reverts commit 2323d7a1ef.
Bug: 561666
Change-Id: I4d7df13e6c9b245fe1706e191e4316685a8a9d59
Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>
Change-Id: Ifb8227cb62370029d6774f2a22b15d6478c713ca
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
ReceiveCommand can have an error message. This is shown only for some
cases even if it's set. This change uses the error message if it's set,
and fallback to the default message if unset.
Change-Id: I8d906e71ad08cf49bcdb28caea8fcc66798c68ff
Signed-off-by: Masaya Suzuki <masayasuzuki@google.com>
Commit 60cf85a4 corrected the handling of check-in for files where
the index version is non-normalized, i.e., contains CR-LF line endings.
However, it did so only for regular files, not executable files.
Bug: 561438
Change-Id: I372cc990c5efeb00315460f36459c0652d5d1e77
Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>
Since reftables might have update index ranges that are overlapped.
Change-Id: I8f8215b99a0a978d4dd0155dbaf33e5e06ea8202
Signed-off-by: Minh Thai <mthai@google.com>
The recursive merge strategy builds a virtual ancestor merging
recursively the common bases (when more than one) between the
want-to-merge commits. While building this virtual ancestor, content
conflicts are ignored, but current code doesn't do so when a file is
removed.
This was spotted in [1], for example. Merging two commits to build the
virtual ancestor bumped into a conflict (modified in one side, deleted
in the other) that stopped the process.
Follow the "spec" and in case of conflict leave the unmerged content in
the index and working trees.
[1] https://android-review.googlesource.com/c/kernel/common/+/1228962
Change-Id: Ife9c32ae3ac3a87d3660fa1242e07854b65169d5
Signed-off-by: Ivan Frade <ifrade@google.com>
Allow explicitly setting the tag option for the remote configuration
when cloning a repository.
Bug: 561021
Change-Id: Iac43268a2bb231ae7599c3255bf555883d34fa32
Signed-off-by: Alexander Nittka <alex@nittka.de>
The error message for an Exception thrown by StartGenerator when given
both the TOPO flag and the TOPO_KEEP_BRANCH_TOGETHER flag mentions a
non-existent flag, TOPO_NON_INTERMIX. The error message was introduced
in commit e498d43.
Replace TOPO_NON_INTERMIX with TOPO_KEEP_BRANCH_TOGETHER in the error
message of an Exception thrown by the StartGenerator when the TOPO flag
is provided together with the TOPO_KEEP_BRANCH_TOGETHER flag.
Signed-off-by: Alex Spradlin <alexaspradlin@google.com>
Change-Id: Id24640dc08e96a196508fe38ce144aa7e035082f
The topological sort algorithm in TopoSortGenerator for RevWalk may mix
multiple lines of history, producing results that differ from C git's
git-log whose man page states: "Show no parents before all of its
children are shown, and avoid showing commits on multiple lines of
history intermixed." Lines of history are mixed because
TopoSortGenerator merely delays producing a commit until all of its
children have been produced; it does not immediately produce a commit
after its last child has been produced.
Therefore, add a new RevSort option called TOPO_KEEP_BRANCH_TOGETHER
with a new topo sort algorithm in TopoNonIntermixGenerator. In the
Generator, when the last child of a commit has been produced, unpop
that commit so that it will be returned upon the subsequent call to
next(). To avoid producing duplicates, mark commits that have not yet
been produced as TOPO_QUEUED so that when a commit is popped, it is
produced if and only if TOPO_QUEUED is set.
To support nesting with other generators that may produce the same
commit multiple times like DepthGenerator (for example, StartGenerator
does this), do not increment parent inDegree for the same child commit
more than once.
Commit b5e764abd2 modified the existing
TopoSortGenerator to avoid mixing lines of history, but it was reverted
in e40c38ab08 because the new behavior
caused problems for EGit users. This motivated adding a new Generator
for the new behavior.
Signed-off-by: Alex Spradlin <alexaspradlin@google.com>
Change-Id: Icbb24eac98c00e45c175b01e1c8122554f617933
RFC 7538[1] added HTTP response code 308, signifying a permanent
redirect that, contrary to the older 301, does not allow changing
the request method from POST to GET.
[1] https://tools.ietf.org/html/rfc7538
Bug: 560936
Change-Id: Ib65f3a3ed75db51d74d1fe81d4abe6fe92b0ca12
Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>
Does not fix any issue but prevents user from shooting themselves in the
foot with improper configuration.
Suggested by Demetr Starshov at https://git.eclipse.org/r/#/c/157681/
Change-Id: I006d65022f0a7d4066970825d00080c59404fdc3
Signed-off-by: Michael Dardis <git@md-5.net>
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
The FS.setAsyncFileStoreAttributes() static method calls
FileStoreAttributes.setBackground() as its implementation, but there are
other public attributes on this inner class already and there isn't a
real reason why this needs to be private.
By making it public we allow callers to be able to invoke it directly.
Although it doesn't appear that it would make a difference, by calling a
static method on the FS class, all static fields and the transitive
closure of class dependencies must be loaded and initialised, which can
be non-trivial.
Callers referring to FS.setAsyncFileStoreAttributes() may be replaced
with FS.FileStoreAttributes.setBackground() with no change of behaviour
other than improved performance due to less class loading required.
Bug: 560527
Change-Id: I9538acc90da8d18f53fd60d74eb54496857f93a5
Signed-off-by: Alex Blewitt <alex.blewitt@gmail.com>
The change Ic0b974fa (c217d33, "Documentation/technical/reftable:
improve repo layout") defines a new repository layout, which was
agreed with the git-core mailing list.
It addresses the following problems:
* old git clients will not recognize reftable-based repositories, and
look at encompassing directories.
* Poorly written tools might write directly into
.git/refs/heads/BRANCH.
Since we consider JGit reftable as experimental (git-core doesn't
support it yet), we have no backward compatibility. If you created a
repository with reftable between mid-Nov 2019 and now, you can do the
following to convert:
mv .git/refs .git/reftable/tables.list
git config core.repositoryformatversion 1
git config extensions.refStorage reftable
Change-Id: I80df35b9d22a8ab893dcbe9fbd051d924788d6a5
Signed-off-by: Han-Wen Nienhuys <hanwen@google.com>
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
* stable-5.6:
Cygwin expects forward slashes for commands to be run via sh.exe
Make Logger instances final
Move array designators from the variable to the type
Change-Id: I9a5dc570deb478525bf48ef526d8cba5b19418bf
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
FS_Win32_Cygwin replaces backslashes by / as a side-effect of
relativize(). When support for core.hooksPath was added, paths were
relativized in a different place using Path.resolve(), which doesn't
do that transformation. As a result hooks could not be run on Cygwin
in some cases.
Do the transformation in FS_Win32_Cygwin.runInShell(). In all other
places, File or Path objects are used, which give no guarantee about
the file separator (typically the system-dependent default separator),
so doing the transformation earlier still wouldn't guarantee that
sh.exe indeed gets a command string using forward slashes.
Bug: 558577
Change-Id: I3c07eb85f0ac7c5628a2e92f990e5cdb7ecf532f
Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>
Running the JDT cleanup action for using a for-each loop on jgit
Change-Id: Ie724d8bbdff786ab0167089e90a9914a8135103c
Signed-off-by: Lars Vogel <Lars.Vogel@vogella.com>
As reported by Sonar Lint:
Array designators should always be located on the type for better code
readability. Otherwise, developers must look both at the type and the
variable name to know whether or not a variable is an array.
Change-Id: If6b41fed3483d0992d402d8680552ab4bef89ffb
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
currVisit could be null if a blob is marked as start point in
ObjectWalk. Add null check before skipping current tree.
Change-Id: Ic5d876fe2800f3373d136979be6c27d1bbd38dc1
Signed-off-by: Yunjie Li <yunjieli@google.com>
This reverts commit b5e764abd2.
PlotWalk uses the TopoSortGenerator, which is causing problems for EGit users
who rely on the emission of commits being somewhat based on date as in the
previous topo-sort algorithm.
Bug: 560529
Change-Id: I3dbd3598a7aeb960de3fc39352699b4f11a8c226
Signed-off-by: Alex Spradlin <alexaspradlin@google.com>
KnownHosts (implementing HostKeyRepository) in Jsch can return null
which could cause NullPointerException in Stream.of(...)
Change-Id: Iddcf5f34f8c8475a85ca7ae018bbe48d1b3fbbc0
Signed-off-by: Lajos Olah <lajos.olah.jr@gmail.com>
* stable-5.6:
Revert "Prepend hostname to subsection used to store file timestamp resolution"
SimilarityRenameDetector: Fix inconsistent indentation
Use indexOf(char) and lastIndexOf(char) rather than String versions
Reorder modifiers to follow Java Language Specification
GitmoduleEntry: Remove redundant import of class from same package
Remove redundant "static" qualifier from enum declarations
Change-Id: Ibb66bef7e8373f81e3e653c9843d986243446d68
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
This reverts commit e102bbed99.
Resolving the hostname comes with a performance penalty. We no longer
store the timestamp resolution in the global git config which might be
copied around to other machines but in a dedicated jgit config meant for
automatically determined options like timestamp resolution. Hence there
is no strong reason anymore to have a hardware specific identifier in
the subsection name of file timestamp resolution options.
Bug: 560414
Change-Id: If8dcabe981eb1792db84643850faa6033f14b1cf
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
Provide a static factory method to create a PathMatcher.
Bug: 559526
Change-Id: Ib7a4a1bcc658ac2f2a09d365b5b891669dfd7570
Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>
Replace space indentation with tab indentation
Change-Id: Ic130d3bde5d3a73d8f5c6225974153573722d05b
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
An indexOf or lastIndexOf call with a single letter String can be
made more performant by switching to a call with a char argument.
Found with SonarLint.
As a side-effect of this change, we no longer need to suppress the
NON-NLS warnings.
Change-Id: Id44cb996bb74ed30edd560aa91fd8525aafdc8dd
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
The Java Language Specification recommends listing modifiers in
the following order:
1. Annotations
2. public
3. protected
4. private
5. abstract
6. static
7. final
8. transient
9. volatile
10. synchronized
11. native
12. strictfp
Not following this convention has no technical impact, but will reduce
the code's readability because most developers are used to the standard
order.
This was detected using SonarLint.
Change-Id: I9cddecb4f4234dae1021b677e915be23d349a380
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
# By Matthias Sohn (3) and Han-Wen Nienhuys (1)
* stable-5.6:
Update API problem filter
Prepare 5.6.2-SNAPSHOT builds
JGit v5.6.1.202002131546-r
Simplify ReftableCompactor
Change-Id: I16ed174f9fc662934c3ebaea85a60690efbed1c6
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
The topological sort algorithm in TopoSortGenerator for RevWalk may mix
multiple lines of history, producing results that differ from C git's
git log whose man page states: "Show no parents before all of its
children are shown, and avoid showing commits on multiple lines of
history intermixed." Lines of history are mixed because
TopoSortGenerator merely delays a commit until all of its children have
been produced; it does not immediately produce a commit after its last
child has been produced.
Therefore, when the last child of a commit has been produced, unpop the
commit so that it will be returned upon the subsequent call to next() in
TopoSortGenerator. To avoid producing duplicates, mark commits that
have not yet been produced as TOPO_QUEUED so that when a commit is
popped, it is produced if and only if TOPO_QUEUED is set.
To support nesting with other generators that may produce the same
commit multiple times like DepthGenerator (for example, StartGenerator
does this), do not increment parent inDegree for the same child commit
more than once.
Modify tests that assert that TopoSortGenerator mixes lines of commit
history.
Change-Id: I4ee03c7a8e5265d61230b2a01ae3858745b2432b
Signed-off-by: Alex Spradlin <alexaspradlin@google.com>
The ReftableCompactor supported a byteLimit, but this is currently
unused. The FileReftableStack has a more sophisticated strategy that
amortizes compaction costs.
Rename min/maxUpdateIndex to reflogExpire{Min,Max}UpdateIndex to
reflect their purpose more accurately.
Since reflogs are generally pruned chronologically (oldest entries are
expired first), one can only prune entries on full compaction, so they
should not be set by default.
Rephrase the function Reader#minUpdateIndex and maxUpdateIndex. These
vars are documented to affect log entries, but semantically, they are
about ref entries. Since ref entries have their timestamps
delta-compressed, it is important for the min/maxUpdateIndex values to
be coherent between different tables.
The logical timestamps for log entries do not have to be coherent in
different tables, as the timestamps of a log entry is part of the key.
For example, a table written at update index 20 may contain a tombstone
log entry at timestamp 1.
Therefore, we set ReftableWriter's min/maxUpdateIndex from the merged
tables we are compacting, rather than from the compaction settings
(which should only control reflog expiry.)
The previous behavior could drop log entries erroneously, especially
in the presence of tombstone log entries. Unfortunately, testing this
properly requires both an API for adding log tombstones, and a more
refined API for controlling automatic compaction. Hence, no test.
Change-Id: I2f4eb7866f607fddd0629809e8e61f0b9097717f
Signed-off-by: Han-Wen Nienhuys <hanwen@google.com>
* stable-5.6:
reftable: don't check deadline on the first try
reftable: clarify comment
reftable: clear cache on full compaction
reftable: remove outdated comment
reftable: clarify that LogCursor may return a null ReflogEntry
Change-Id: I9458a746311984fa687b3da964805e2568ed37f3