A commit A can reach a commit B only if the generation number of A is
strictly larger than the generation number of B. This condition allows
significantly short-circuiting commit-graph walks.
On a copy of the Linux repository where HEAD is contained in v6.3-rc4
but no earlier tag, the command 'git tag --contains HEAD' of
ListTagCommand#call() had the following peformance improvement:
(excluded the startup time of the repo)
Before: 2649ms (core.commitgraph=true)
11909ms (core.commitgraph=false)
After: 91ms (core.commitgraph=true)
11934ms (core.commitgraph=false)
Bug: 574368
Change-Id: Ia2efaa4e9ae598266f72e70eb7e3b27655cbf85b
Signed-off-by: kylezhao <kylezhao@tencent.com>
Loading of pack, bitmap and commit-graph copy the same code to adjust
the input stream buffering.
Extract to a common function. Besides reusing the code, the name hints
what it is doing.
This block aligment seems unnecessary as the reading is from storage
not dfs cache. The channel probably knows better. Left a TODO because
I don't know the original intention.
Change-Id: I18b77ae8189830fcd4d5932b6b5823b693ed6090
* stable-6.5:
Ensure parsed RevCommitCG has derived data from commit-graph
Downgrade maven-site-plugin to 3.12.1
Use wagon-ssh-external to deploy Maven site
Change-Id: Ide721fb088fa04f6276ac495968a45e732f6e139
If a RevCommitCG was newly created and called #parseCanonical(RevWalk,
byte[]) method immediately, its flag was marked as PARSED, but no
derived data was obtained from the commit-graph. This is different from
what we expected.
Change-Id: I5d417efa3c42d211f19e6acf255f761e84d84450
Signed-off-by: kylezhao <kylezhao@tencent.com>
This reverts commit 9c33f7364d.
Reason for revert: This change was based on the false claim that the
packedrefs file lock is held while the CAS is being done, but it is
actually released before the CAS (the in memory lock is still held,
however that does not prevent external actors from updating the
packedrefs files and then another thread from subsequently re-reading it
and updating the in memory packedRefList). Although reverting this
change can cause the CAS to fail, it should not actually matter since
the failure would indicate that another thread has already updated the
in memory packedRefList to either the same version this thread was
trying to update it too, or to a more recent version. Either way,
failing the CAS is then appropriate and should not be problematic.
Although this change reverts the code in the RefDirectory class, it
keeps the "improvements" to the test so that it continues to pass
reliably. The reason for the quotes around the word "improvements" is
because I believe the test alteration actually dramatically changes the
intent of the test, and that the original intent of the test is
untestable with the GC and RefDirectory classes as is.
Change-Id: I3acee7527bb542996dcdfaddfb2bdb45ec444db5
Signed-off-by: Martin Fick <quic_mfick@quicinc.com>
The in-memory copy of packed refs might be outdated by the time the
packed-refs lock is acquired, so ensure the one read from disk is
used after acquiring the lock to prevent commit packed-refs from
throwing an exception. As a side-effect, since this updates the
in-memory copy of packed-refs when it is re-read from disk, it can
prevent other callers needing to re-read if it had changed.
Change-Id: I724c866b330b397e955b5eb04b259eedd9911e93
Signed-off-by: Kaushik Lingarkar <quic_kaushikl@quicinc.com>
Since packed-refs is read from disk anyway, don't rely on the
in-memory copy as that is racy and if outdated, could result in
commit of pack-refs throwing an exception. This change also avoids
a possible unnecessary double read of packed-refs from disk.
Change-Id: I684a64991f53f8bdad58bbd248aae6522d11267d
Signed-off-by: Kaushik Lingarkar <quic_kaushikl@quicinc.com>
There are no more callers (since Iae71cb3) of these methods that need
the returned value. These methods should not have been returning
anything in the first place as that can introduce bugs such as the
one described in Iae71cb3.
Change-Id: I1d083a91603da803a106cfb1506925a82c2ef809
Signed-off-by: Kaushik Lingarkar <quic_kaushikl@quicinc.com>
Introduce a SnapshottingRefDirectory class which allows users to get
a snapshot of the ref database and use it in a request scope (for
example a Gerrit query) instead of having to re-read packed-refs
several times in a request.
This can potentially be further improved to avoid scanning/reading a
loose ref several times in a request. This would especially help
repeated lookups of a packed ref, where we check for the existence of
a loose ref each time.
Change-Id: I634b92877f819f8bf36a3b9586bbc1815108189a
Signed-off-by: Kaushik Lingarkar <quic_kaushikl@quicinc.com>
In the window between refs being packed (via refDb.pack) and obtaining
updates (via applyUpdates), packed-refs may have been updated by another
actor and relying on the previously read contents may lead to losing the
updates done by the other actor. To help avoid this, read packed-refs
from disk to ensure we have the latest copy after it is locked and
before committing updates to it.
Bug: 581641
Change-Id: Iae71cb30830b307d0df929c9131911ee476c711c
Signed-off-by: Kaushik Lingarkar <quic_kaushikl@quicinc.com>
By default, Git will report, to the server, commits reachable from all local refs to find common commits in an attempt to reduce the size of the to-be-received packfile. If specified with negotiation tip, Git will only report commits reachable from the given tips. This is useful to speed up fetches when the user knows which local ref is likely to have commits in common with the upstream ref being fetched.
When negotation-tip is on, use the wanted refs instead of all refs as source of the "have" list to send.
This is controlled by the `fetch.usenegotationtip` flag, false by default. This works only for programmatic fetches and there is no support for it yet in the CLI.
Change-Id: I19f8fe48889bfe0ece7cdf78019b678ede5c6a32
Support the new option index.skipHash which was introduced in git 2.40
[1]. If it is set to true skip computing the git index checksum. This
accelerates Git commands that manipulate the index, such as git add, git
commit, or git status. Instead of storing the checksum, write a trailing
set of bytes with value zero, indicating that the computation was
skipped.
Accept a skipped checksum consisting of 20 null bytes when reading the
index since the option could have been set to true at the time when the
index was written.
[1] https://git-scm.com/docs/git-config#Documentation/git-config.txt-indexskipHash
Bug: 581723
Change-Id: I28ebe44c5ca1cbcb882438665d686452a0c111b2
1. For general errors, throw IOException instead of wrapping them with
PatchApplyException. The wrapping was moved (back) to ApplyCommand.
2. For file specific errors, log the errors as part of
PatchApplier::Result.
3. Change applyPatch() to receive the parsed Patch object, so the caller
can decide how to handle parsing errors.
Background: this utility class was extracted from ApplyCommand on V6.4.0.
During the extraction, we left the exception wrapping by
PatchApplyException intact. This attitude made it harder for the callers to
distinguish between the actual error causes.
Change-Id: Ib0f2b5e97a13df2339d8b65f2fea1c819c161ac3
When commit-graph file already exists in the repository, a newly
created FileCommitGraph didn't scan CommitGraph until the file was
modified, resulting in wrong result.
Change-Id: Ic85676f2d3b6a88f3ae28d4065729926b6fb2f23
Signed-off-by: kylezhao <kylezhao@tencent.com>
From File#lines javadoc: The returned stream from File Lines
encapsulates a Reader. If timely disposal of file system resources is
required, the try-with-resources construct should be used to ensure
that the stream's close method is
invoked after the stream operations are completed.
Wrap File.lines with try-with-resources.
Change-Id: I82c6faa3ef1083f6c7e964f96e9540b4db18eee8
Signed-off-by: Xing Huang <xingkhuang@google.com>
(cherry picked from commit 172a207945)
From File#lines javadoc: The returned stream from File Lines
encapsulates a Reader. If timely disposal of file system resources is
required, the try-with-resources construct should be used to ensure
that the stream's close method is
invoked after the stream operations are completed.
Wrap File.lines with try-with-resources.
Signed-off-by: Xing Huang <xingkhuang@google.com>
Change-Id: I82c6faa3ef1083f6c7e964f96e9540b4db18eee8
Signed-off-by: Xing Huang <xingkhuang@google.com>
In these cases we use Throwable#addSuppressed to ensure the exception
thrown in the catch block preceding the finally block throwing another
exception isn't lost.
Change-Id: I96e78a5c15238ab77ac90ca1901850ce19acfcd8
The native git option to preserve merge commits during rebase
has been changed from pull.rebase=preserve to pull.rebase=merges.
This changeset in jgit makes the same config change. The old "preserve"
option is no longer recognized and is replaced by new option called
"merges".
This makes jgit's rebase configuration compatible with native git
versions 2.34 and newer where the old "preserve" option has been
removed.
Change-Id: Ic07ff954e258115e76465a1593ef3259f4c418a3
Display elapsed time per task if enabled via
ProgressMonitor#showDuration or if system property or environment
variable GIT_TRACE_PERFORMANCE is set to "true". If both the system
property and the environment variable are set the system property takes
precedence.
E.g. using jgit CLI:
$ GIT_TRACE_PERFORMANCE=true jgit clone https://foo.bar/foobar
Cloning into 'foobar'...
remote: Counting objects: 1 [0.002s]
remote: Finding sources: 100% (15531/15531) [0.006s]
Receiving objects: 100% (169737/169737) [13.045s]
Resolving deltas: 100% (67579/67579) [1.842s]
Change-Id: I4d624e7858b286aeddbe7d4e557589986d73659e
PackWriter callers tell the writer what do the want to include in the
pack and invoke #writePack(). Afterwards, they can invoke #writeIndex()
to write the corresponding pack index.
Mirror this for the object-size index, adding a #writeObjectSizeIndex()
method.
Change-Id: Ic319975c72c239cd6488303f7d4cced797e6fe00
* stable-6.4:
If tryLock fails to get the lock another gc has it
Fix GcConcurrentTest#testInterruptGc
Don't swallow IOException in GC.PidLock#lock
Check if FileLock is valid before using or releasing it
Change-Id: Ia2797b44a60342eb9df53f0b3d674cba92a512fc
* stable-6.3:
If tryLock fails to get the lock another gc has it
Fix GcConcurrentTest#testInterruptGc
Don't swallow IOException in GC.PidLock#lock
Check if FileLock is valid before using or releasing it
Change-Id: I5af34c92e423a651db53b4dc45ed844d5f39910d
* stable-6.2:
If tryLock fails to get the lock another gc has it
Fix GcConcurrentTest#testInterruptGc
Don't swallow IOException in GC.PidLock#lock
Check if FileLock is valid before using or releasing it
Change-Id: I5b6b10622b61fde3f0f10455a74ae159a0b69082
* stable-6.1:
If tryLock fails to get the lock another gc has it
Fix GcConcurrentTest#testInterruptGc
Don't swallow IOException in GC.PidLock#lock
Check if FileLock is valid before using or releasing it
Change-Id: I3ffe92566cc145053bb762f612dd96bc6d542c62
* stable-6.0:
If tryLock fails to get the lock another gc has it
Fix GcConcurrentTest#testInterruptGc
Don't swallow IOException in GC.PidLock#lock
Check if FileLock is valid before using or releasing it
Change-Id: Idea23e555c024557d7e39a86efe25f609400b962
* stable-5.13:
If tryLock fails to get the lock another gc has it
Fix GcConcurrentTest#testInterruptGc
Don't swallow IOException in GC.PidLock#lock
Check if FileLock is valid before using or releasing it
Change-Id: I708d0936fa86b028e4da4e7e21f332f8b48ad293
This broke the test GcConcurrentTest#testInterruptGc which expects
ClosedByInterruptException when the thread doing gc is interrupted.
Change-Id: I89e02fc37aceeccb04c20cfc5b71cb8fa21793d6
* stable-6.4:
Use Java 11 ProcessHandle to get pid of the current process
Acquire file lock "gc.pid" before running gc
Silence API errors introduced by 9424052f
Change-Id: Ifa4e56b6ecca9305f3f1685e45450019bfc82e22
* stable-6.3:
Use Java 11 ProcessHandle to get pid of the current process
Acquire file lock "gc.pid" before running gc
Silence API errors introduced by 9424052f
Change-Id: Ic40dbab18616d8d9fe3820b9890c86652b80eb47
* stable-6.2:
Use Java 11 ProcessHandle to get pid of the current process
Acquire file lock "gc.pid" before running gc
Silence API errors introduced by 9424052f
Change-Id: I53cf9675deac0b588048d8224216d2a7e8bd16ec
* stable-6.1:
Use Java 11 ProcessHandle to get pid of the current process
Acquire file lock "gc.pid" before running gc
Silence API errors introduced by 9424052f
Change-Id: I0562a4a224779ccf1e4cc1ff8f5a352e55ab220a
* stable-6.0:
Use Java 11 ProcessHandle to get pid of the current process
Acquire file lock "gc.pid" before running gc
Silence API errors introduced by 9424052f
Change-Id: Ib9a2419253ffcbc90874adbfdb8129fee3178210
C git 2.11 supports setting the equivalent of RequestPolicy.ANY with
uploadpack.allowAnySHA1InWant[1]. Parse this into TransportConfig and
use it from UploadPack.
Add additional tests for [2] and this change.
We can execute "git clone --filter=blob:none --no-checkout" successfully
with config uploadPack.allowFilter is true. But when we checkout, the
git will fetch other missing objects required by the checkout(this is
why we need this config).
When both uploadPack.allowFilter and uploadPack.allowAnySHA1InWant are
true, jgit will support partial clone. If you are using an extremely
large monorepo, this feature can help. It allows users to work on an
incomplete repo which reduces disk usage.
[1] f8edeaa05d
[2] change Id39771a6e42d8082099acde11249306828a053c0
Bug: 573390
Change-Id: I8fe75f03bf1fea7c11e0d67c8637bd05dd1f9b89
Signed-off-by: kylezhao <kylezhao@tencent.com>
Git guards gc by locking a lock file "gc.pid" before starting execution.
The lock file contains the pid and hostname of the process holding the
lock. Git tries to kill the process holding that lock if the lock file
wasn't modified in the last 12 hours and was started from the same host.
Teach JGit to acquire this lock before running gc but skip execution if
another process already holds the lock. Killing the other process could
be undesired if it's a long running application.
If the lock file wasn't modified in the last 12 hours try to lock it and
run gc if locking succeeds.
Register a shutdown hook for the lock file to ensure it is cleaned up if
the process is gracefully killed.
Change-Id: I00b838dcbf4fb0d03863bf7a2cd86b743c6c6971
* stable-6.4:
Fix getPackedRefs to not throw NoSuchFileException
Add pack options to preserve and prune old pack files
Allow to perform PackedBatchRefUpdate without locking loose refs
Document option "core.sha1Implementation" introduced in 59029aec
Change-Id: I36051c623fcd480aa80ed32b4e89f9bdd1b798e0
* stable-6.3:
Fix getPackedRefs to not throw NoSuchFileException
Add pack options to preserve and prune old pack files
Allow to perform PackedBatchRefUpdate without locking loose refs
Document option "core.sha1Implementation" introduced in 59029aec
Change-Id: I1073098fb06eabafdb3c5e7fcf44d55b86a1b152
* stable-6.2:
Fix getPackedRefs to not throw NoSuchFileException
Add pack options to preserve and prune old pack files
Allow to perform PackedBatchRefUpdate without locking loose refs
Document option "core.sha1Implementation" introduced in 59029aec
Change-Id: I765c7302ce84a6a9c28fdef29da2bfaa49477c6e
The object size index can have up to #(blobs-in-repo) entries, taking
a relevant amount of memory. Let operators configure the threshold size
to include objects in the size index.
The index will include objects with size *at or above* this
value (with -1 for none). This is more effective for the
filter-by-size case.
Lowering the threshold adds more objects to the index. This improves
performance at the cost of memory/storage space. For the object-size
case, more calls will use the index instead of reading IO. For the
filter-by-size case, lower threshold means better granularity (if
ObjectReader#isSmallerThan is implemented based only on the index).
Change-Id: I6ccd9334adbbc2abf95fde51dbbfc85b8230ade0
* stable-6.1:
Fix getPackedRefs to not throw NoSuchFileException
Add pack options to preserve and prune old pack files
Allow to perform PackedBatchRefUpdate without locking loose refs
Document option "core.sha1Implementation" introduced in 59029aec
Change-Id: Id32683d5f506e082d39af269803bccee0280cc27
* stable-6.0:
Add pack options to preserve and prune old pack files
Allow to perform PackedBatchRefUpdate without locking loose refs
Document option "core.sha1Implementation" introduced in 59029aec
Change-Id: I876a38c2de8b7d5eaacd00e36b85599f88173221
* stable-5.13:
Add pack options to preserve and prune old pack files
Allow to perform PackedBatchRefUpdate without locking loose refs
Document option "core.sha1Implementation" introduced in 59029aec
Change-Id: I423f410578f5bbe178832b80fef8998a5372182c
Since Files.newInputStream is from java.nio package, it throws
java.nio.file.NoSuchFileException. This was missed in the change
I00da88e. Without this change, getPackedRefs fails with
NoSuchFileException when there is no packed-refs file in a project.
Change-Id: I93c202ddb73a0a5979af8e4d09e45f5645664b45
Signed-off-by: Prudhvi Akhil Alahari <quic_prudhvi@quicinc.com>
Operations like "clone --filter=blob:limit=N" or the "object-info"
command need to read the size of the objects from the storage. An
index would provide those sizes at once rather than having to seek in
the packfile.
Introduce an interface for the Object-size index. This index returns
the inflated size of an object. Not all objects could be indexed (to
limit memory usage).
This implementation indexes only blobs (no trees, nor
commits) *above* certain size threshold (configurable). Lower
threshold adds more objects to the index, consumes more memory and
provides better performance. 0 means "all blobs" and -1 "disabled".
If we don't index everything, for the filter use case is more
efficient to index the biggest objects first: the set is small and
most objects are filtered by NOT being in the index. For the
object-size, the more objects in the index the better, regardless
their size. All together, it is more helpful to index above threshold.
Change-Id: I9ed608ac240677e199b90ca40d420bcad9231489
The object size index stores positions of objects in the main
index (when ordered by sha1). These positions are per-pack and usually
a pack has <16 million objects (there are exceptions but rather
rare). It could save some memory storing these positions in three bytes
instead of four. Note that these positions are sorted and always positive.
Implement a wrapper around a byte[] to access and search "ints" while
they are stored as unsigned 3 bytes.
Change-Id: Iaa26ce8e2272e706e35fe4cdb648fb6ca7591972
The primary index returns the offset in the pack for an
objectId. Internally it keeps the object-ids in lexicographical order,
but doesn't expose an API to find the position of an object-id in that
list. This is needed for the object-size index, that we want to store
as "position-in-idx, size".
Add a #findPosition(object-id) method to the PackIndex interface to
know where an object-id sits in the ordered list of ids in the pack.
Note that this index position is over the list of ordered object-ids,
while reverse-index position is over the list of objects in packed
order.
Change-Id: I89fa146599e347a26d3012d3477d7f5bbbda7ba4
Add the options
- pack.preserveOldPacks
- pack.prunePreserved
This allows to configure in git config if old packs should be preserved
during gc and pruned during the next gc.
The original implementation in 91132bb0 only allows to set these options
using the API.
Change-Id: I5b23ab4f317d12f5ccd234401419913e8263cc9a
JGit knows how to read/write commit graphs but the DFS stack is not
using it yet.
The DFS garbage collector generates a commit-graph with commits
reachable from any ref. The pack is stored as extra stream in the GC
pack. DfsPackFile mimicks how other indices are loaded storing the
reference in DFS cache.
Signed-off-by: Xing Huang <xingkhuang@google.com>
Change-Id: I3f94997377986d21a56b300d8358dd27be37f5de
ObjectReader#getCommitGraph doesn't report errors loading the
commit graph. The caller should be aware of the situation and
ultimately decide what to do.
Add IOException to ObjectReader#getCommitGraph signature. RevWalk
defaults to an empty commit-graph on IO errors.
Signed-off-by: Xing Huang <xingkhuang@google.com>
Change-Id: I38eeacff76c7f926b6dfb192d1e5916e40770024
Add another newBatchUpdate method in the RefDirectory where we can
control if the created PackedBatchRefUpdate will lock the loose refs or
not.
This can be useful in cases when we run programs which have exclusive
access to a Git repository and we know that locking loose refs is
unnecessary and just a performance loss.
Change-Id: I7d0932eb1598a3871a2281b1a049021380234df9
(cherry picked from commit cb90ed0852)
The 'size' packet line is an argument, so it
must be preceeded by a 0001 delimiter. See also git's
t5701-git-serve.sh test,
https://github.com/git/git/blob/8b8d9a2/t/t5701-git-serve.sh#L329
Without this fix, the server will choke on the delimiter line, saying
PackProtocolException: unexpected <empty string>
To test, I ran Gerrit locally with this fix
$ curl -X POST -H 'git-protocol: version=2' -H 'content-type:
application/x-git-upload-pack-request' -H 'accept:
application/x-git-upload-pack-result' --data
$'0018command=object-info\n00010009size\n0031oid
d38b1b92bdb2893eb4505667375563f2d6d4086b\n0000'
http://localhost:8080/git.git/git-upload-pack
=>
0008size0032d38b1b92bdb2893eb4505667375563f2d6d4086b 268590000
The same command completes identically on Gitlab (which supports the
object-info command)
$ curl -X POST -H 'git-protocol: version=2' -H 'content-type:
application/x-git-upload-pack-request' -H 'accept:
application/x-git-upload-pack-result' --data
$'0018command=object-info\n00010009size\n0031oid
d38b1b92bdb2893eb4505667375563f2d6d4086b\n0000'
https://gitlab.com/gitlab-org/git.git/git-upload-pack
=>
0008size0032d38b1b92bdb2893eb4505667375563f2d6d4086b 268590000
In this case, the blob is for the COPYING file in the Git source tree,
which is 26859 bytes long.
Change-Id: Ief4ce1eb9303a3b2479547d7950ef01c7c28f472
This change only affects inCore repositories.
Before this change, any file that wasn't part of the patch
wasn't read, and therefore wasn't part of the output tree.
Change-Id: I246ef957088f17aaf367143f7a0b3af0f8264ffb
Bug: Google b/267270348
* stable-6.4:
Shortcut during git fetch for avoiding looping through all local refs
FetchCommand: fix fetchSubmodules to work on a Ref to a blob
Silence API warnings introduced by I466dcde6
Allow the exclusions of refs prefixes from bitmap
PackWriterBitmapPreparer: do not include annotated tags in bitmap
BatchingProgressMonitor: avoid int overflow when computing percentage
Speedup GC listing objects referenced from reflogs
FileSnapshotTest: Add more MISSING_FILE coverage
Change-Id: Id0ebfbd85eb815716383b9495eb7dd1f54cf4d74
* stable-6.3:
Shortcut during git fetch for avoiding looping through all local refs
FetchCommand: fix fetchSubmodules to work on a Ref to a blob
Silence API warnings introduced by I466dcde6
Allow the exclusions of refs prefixes from bitmap
PackWriterBitmapPreparer: do not include annotated tags in bitmap
BatchingProgressMonitor: avoid int overflow when computing percentage
Speedup GC listing objects referenced from reflogs
FileSnapshotTest: Add more MISSING_FILE coverage
Change-Id: Iefcf5d832bd0087c1027876f2200689e1150abce
* stable-6.2:
Shortcut during git fetch for avoiding looping through all local refs
FetchCommand: fix fetchSubmodules to work on a Ref to a blob
Silence API warnings introduced by I466dcde6
Allow the exclusions of refs prefixes from bitmap
PackWriterBitmapPreparer: do not include annotated tags in bitmap
BatchingProgressMonitor: avoid int overflow when computing percentage
Speedup GC listing objects referenced from reflogs
FileSnapshotTest: Add more MISSING_FILE coverage
Change-Id: I2ff386d9a096277360e6c7bd5535b49984620fb3
* stable-6.1:
Shortcut during git fetch for avoiding looping through all local refs
FetchCommand: fix fetchSubmodules to work on a Ref to a blob
Silence API warnings introduced by I466dcde6
Allow the exclusions of refs prefixes from bitmap
PackWriterBitmapPreparer: do not include annotated tags in bitmap
BatchingProgressMonitor: avoid int overflow when computing percentage
Speedup GC listing objects referenced from reflogs
FileSnapshotTest: Add more MISSING_FILE coverage
Change-Id: Iff2fba026b49463016015b2fae1a42cf76ee2dbb
* stable-6.0:
Shortcut during git fetch for avoiding looping through all local refs
FetchCommand: fix fetchSubmodules to work on a Ref to a blob
Silence API warnings introduced by I466dcde6
Allow the exclusions of refs prefixes from bitmap
PackWriterBitmapPreparer: do not include annotated tags in bitmap
BatchingProgressMonitor: avoid int overflow when computing percentage
Speedup GC listing objects referenced from reflogs
FileSnapshotTest: Add more MISSING_FILE coverage
Change-Id: Ib5055f2f3b8a313c178d6f6c7c5630285ad5a726
* stable-5.13:
Shortcut during git fetch for avoiding looping through all local refs
FetchCommand: fix fetchSubmodules to work on a Ref to a blob
Silence API warnings introduced by I466dcde6
Allow the exclusions of refs prefixes from bitmap
PackWriterBitmapPreparer: do not include annotated tags in bitmap
BatchingProgressMonitor: avoid int overflow when computing percentage
Speedup GC listing objects referenced from reflogs
FileSnapshotTest: Add more MISSING_FILE coverage
Change-Id: I58ad4c210a5e7e5a1ba6b22315b04211c8909950
The FetchProcess needs to verify that all the refs received point
to objects that are reachable from the local refs, which could be
very expensive but is needed to avoid missing objects exceptions
because of broken chains.
When the local repository has a lot of refs (e.g. millions) and the
client is fetching a non-commit object (e.g. refs/sequences/changes in
Gerrit) the reachability check on all local refs can be very expensive
compared to the time to fetch the remote ref.
Example for a 2M refs repository:
- fetching a single non-commit object: 50ms
- checking the reachability of local refs: 30s
A ref pointing to a non-commit object doesn't have any parent or
successor objects, hence would never need to have a reachability check
done. Skipping the askForIsComplete() altogether would save the 30s
time spent in an unnecessary phase.
Signed-off-by: Luca Milanesio <luca.milanesio@gmail.com>
Change-Id: I09ac66ded45cede199ba30f9e71cc1055f00941b
FetchCommand#fetchSubmodules assumed that FETCH_HEAD can always be
parsed as a tree. This isn't true if it refers to a Ref referring to a
BLOB. This is e.g. used in Gerrit for Refs like refs/sequences/changes
which are used to implement sequences stored in git.
Change-Id: I414f5b7d9f2184b2d7d53af1dfcd68cccb725ca4
When running a GC.repack() against a repository with over one
thousands of refs/heads and tens of millions of ObjectIds,
the calculation of all bitmaps associated with all the refs
would result in an unreasonable big file that would take up to
several hours to compute.
Test scenario: repo with 2500 heads / 10M obj Intel Xeon E5-2680 2.5GHz
Before this change: 20 mins
After this change and 2300 heads excluded: 10 mins (90s for bitmap)
Having such a large bitmap file is also slow in the runtime
processing and have negligible or even negative benefits, because
the time lost in reading and decompressing the bitmap in memory
would not be compensated by the time saved by using it.
It is key to preserve the bitmaps for those refs that are mostly
used in clone/fetch and give the ability to exlude some refs
prefixes that are known to be less frequently accessed, even
though they may actually be actively written.
Example: Gerrit sandbox branches may even be actively
used and selected automatically because its commits are very
recent, however, they may bloat the bitmap, making it ineffective.
A mono-repo with tens of thousands of developers may have
a relatively small number of active branches where the
CI/CD jobs are continuously fetching/cloning the code. However,
because Gerrit allows the use of sandbox branches, the
total number of refs/heads may be even tens to hundred
thousands.
Change-Id: I466dcde69fa008e7f7785735c977f6e150e3b644
Signed-off-by: Luca Milanesio <luca.milanesio@gmail.com>
The InMemoryRepository is used in tests (e.g. in gerrit tests) and it
can be useful to create a custom MemRefDatabase for some tests.
Change-Id: I6fbbbfe04400ea1edc988c8788c8eeb06ca8480a
The annotated tags should be excluded from the bitmap associated
with the heads-only packfile. However, this was not happening
because of the check of exclusion of the peeled object instead
of the objectId to be excluded from the bitmap.
Sample use-case:
refs/heads/main
^
|
commit1 <-- commit2 <- annotated-tag1 <- tag1
^
|
commit0
When creating a bitmap for the above commit graph, before this
change all the commits are included (3 bitmaps), which is
incorrect, because all commits reachable from annotated tags
should not be included.
The heads-only bitmap should include only commit0 and commit1
but because PackWriterBitPreparer was checking for the peeled
pointer of tag1 to be excluded (commit2) which was not found in
the list of tags to exclude (annotated-tag1), the commit2 was
included, even if it wasn't reachable only from the head.
Add an additional check for exclusion of the original objectId
for allowing the exclusion of annotated tags and their pointed
commits. Add one specific test associated with an annotated tag
for making sure that this use-case is covered also.
Example repository benchmark for measuring the improvement:
# refs: 400k (2k heads, 88k tags, 310k changes)
# objects: 11M (88k of them are annotate tags)
# packfiles: 2.7G
Before this change:
GC time: 5h
clone --bare time: 7 mins
After this change:
GC time: 20 mins
clone --bare time: 3 mins
Bug: 581267
Signed-off-by: Luca Milanesio <luca.milanesio@gmail.com>
Change-Id: Iff2bfc6587153001837220189a120ead9ac649dc
When cloning huge repositories I observed percentage of object counts
turning negative. This happened if lastWork * 100 exceeded
Integer.MAX_VALUE.
Change-Id: Ic5f5cf5a911a91338267aace4daba4b873ab3900
We are adding commit-graph loading to the DFS stack and the stats object doesn't have fields to track that.
This change replicates the stats of the primary index for the commit-graph.
Signed-off-by: Xing Huang <xingkhuang@google.com>
Change-Id: I4a657bed50083c4ae8bc9f059d4943d612ea2d49
GC needs to get a ReflogReader for all existing refs to list all objects
referenced from reflogs. The existing Repository#getReflogReader method
accepts the ref name and then resolves the Ref to create a ReflogReader.
GC calling that for a huge number of Refs one by one is very slow. GC
first gets all Refs in bulk and then calls getReflogReader for each of
them.
Fix this by adding another getReflogReader method to Repository which
accepts a Ref directly.
This speeds up running JGit gc on a mirror clone of the Gerrit
repository from 15:36 min to 1:08 min. The repository used in this test
had 45k refs, 275k commits and 1.2m git objects.
Change-Id: I474897fdc6652923e35d461c065a29f54d9949f4
* stable-6.4:
Cache trustFolderStat/trustPackedRefsStat value per-instance
Refresh 'objects' dir and retry if a loose object is not found
Change-Id: Iea8038dfde29ab988501469f86ee829e578a2fe8
* stable-6.3:
Cache trustFolderStat/trustPackedRefsStat value per-instance
Refresh 'objects' dir and retry if a loose object is not found
Change-Id: I1db2b51ae8101f345d08235d4f3dc416bfcb42d5
* stable-6.2:
Cache trustFolderStat/trustPackedRefsStat value per-instance
Refresh 'objects' dir and retry if a loose object is not found
Change-Id: Ibc9bffab8c9ef9c39384b53c142d99878f7f3f98
* stable-6.1:
Cache trustFolderStat/trustPackedRefsStat value per-instance
Refresh 'objects' dir and retry if a loose object is not found
Change-Id: I9e876f72f735f58bf02c7862a3d8e657fc46a7b9
Instead of re-reading the config every time the methods using these
values were called, cache the config value at the time of instance
construction. Caching the values improves performance for each of the
method calls. These configs are set based on the filesystem storing the
repository and unlikely to change while an application is running.
Change-Id: I1cae26dad672dd28b766ac532a871671475652df
Signed-off-by: Nasser Grainawi <quic_nasserg@quicinc.com>
A new loose object may not be immediately visible on a NFS
client if it was created on another client. Refreshing the
'objects' dir and trying again can help work around the NFS
behavior.
Here's an E2E problem that this change can help fix. Consider
a Gerrit multi-primary setup with repositories based on NFS.
Add a new patch-set to an existing change and then immediately
fetch the new patch-set of that change. If the fetch is handled
by a Gerrit primary different that the one which created the
patch-set, then we sometimes run into a MissingObjectException
that causes the fetch to fail.
Bug: 581317
Change-Id: Iccc6676c68ef13a1e8b2ff52b3eeca790a89a13d
Signed-off-by: Kaushik Lingarkar <quic_kaushikl@quicinc.com>
RevWalk#createCommit() will inspect the commit-graph file to find the
specified object's graph position and then return a new RevCommitCG
instance.
RevCommitGC is a RevCommit with an additional "pointer" (the position)
to the commit-graph, so it can load the headers and metadata from there
instead of the pack. This saves IO access in walks where the body is not
needed (i.e. #isRetainBody is false and #parseBody is not invoked).
RevWalk uses automatically the commit-graph if available, no action
needed from callers. The commit-graph is fetched on first access from
the reader (that internally can keep it loaded and reuse it between
walks).
The startup cost of reading the entire commit graph is small. After
testing, reading a commit-graph with 1 million commits takes less than
50ms. If we use RepositoryCache, it will not be initialized util the
commit-graph is rewritten.
Bug: 574368
Change-Id: I90d0f64af24f3acc3eae6da984eae302d338f5ee
Signed-off-by: kylezhao <kylezhao@tencent.com>
In shallow repos, GC writes to the commit-graph that shallow commits
do not have parents. This won't be true after a "git fetch --unshallow"
(and before another GC).
Do not write the commit-graph from shallow clones of a repo. The
commit-graph must have the real metadata of commits and that is not
available in a shallow view of the repo.
Change-Id: Ic9f2358ddaa607c74f4dbf289c9bf2a2f0af9ce0
Signed-off-by: kylezhao <kylezhao@tencent.com>
Currently, we always read packed-refs file when 'trustFolderStat'
is false. Introduce a new config 'trustPackedRefsStat' which takes
precedence over 'trustFolderStat' when reading packed refs. Possible
values for this new config are:
* always: Trust packed-refs file attributes
* after_open: Same as 'always', but refresh the file attributes of
packed-refs before trusting it
* never: Always read the packed-refs file
* unset: Fallback to 'trustFolderStat' to determine if the file
attributes of packed-refs can be trusted
Folks whose repositories are on NFS and have traditionally been
setting 'trustFolderStat=false' can now get some performance improvement
with 'trustPackedRefsStat=after_open' as it refreshes the file
attributes of packed-refs (at least on some NFS clients) before
considering it.
For example, consider a repository on NFS with ~500k packed-refs. Here
are some stats which illustrate the improvement with this new config
when reading packed refs on NFS:
trustFolderStat=true trustPackedRefsStat=unset: 0.2ms
trustFolderStat=false trustPackedRefsStat=unset: 155ms
trustFolderStat=false trustPackedRefsStat=after_open: 1.5ms
Change-Id: I00da88e4cceebbcf3475be0fc0011ff65767c111
Signed-off-by: Kaushik Lingarkar <quic_kaushikl@quicinc.com>
A ternary search tree is a type of tree where nodes are arranged in a
manner similar to a binary search tree, but with up to three children
rather than the binary tree's limit of two.
Each node of a ternary search tree stores a single character, a
reference to a value object and references to its three children named
equal kid, lo kid and hi kid. The lo kid pointer must point to a node
whose character value is less than the current node. The hi kid pointer
must point to a node whose character is greater than the current
node.[1] The equal kid points to the next character in the word. Each
node in a ternary search tree represents a prefix of the stored strings.
All strings in the middle subtree of a node start with that prefix.
Like other prefix trees, a ternary search tree can be used as an
associative map with the ability for incremental string search. Ternary
search trees are more space efficient compared to standard prefix trees,
at the cost of speed.
They allow efficient prefix search which is important to implement
searching refs by prefix in a RefDatabase.
Searching by prefix returns all keys if the prefix is an empty string.
Bug: 576165
Change-Id: If160df70151a8e1c1bd6716ee4968e4c45b2c7ac
FileRepository's ObjectReader#getCommitGraph will return commit-graph
when it exists and core.commitGraph is true.
DfsRepository is not supported currently.
Change-Id: I992d43d104cf542797e6949470e95e56de025107
Signed-off-by: kylezhao <kylezhao@tencent.com>
If the last line came from the patch, use the patch to determine whether
or not there should be a trailing newline. Otherwise use the old text.
Add test cases for
- no newline at end, last line not in patch hunk
- no newline at end, last line in patch hunk
- patch removing the last newline
- patch adding a newline at the end of file not having one
all for core.autocrlf false, true, and input.
Add a test case where the "no newline" indicator line is not the last
line of the last hunk. This can happen if the patch ends with removals
at the file end.
Bug: 581234
Change-Id: I09d079b51479b89400ad300d0662c1dcb50deab6
Also-by: Yuriy Mitrofanov <a2terminator@mail.ru>
Signed-off-by: Thomas Wolf <twolf@apache.org>
This change makes JGit can read .git/objects/info/commit-graph file
and then get CommitGraph.
Loading a new commit-graph into memory requires additional time. After
testing, loading a copy of the Linux's commit-graph(1039139 commits)
is under 50ms.
Bug: 574368
Change-Id: Iadfdd6ed437945d3cdfdbe988cf541198140a8bf
Signed-off-by: kylezhao <kylezhao@tencent.com>
Some lines were too long, unnecessary fully qualified class names,
and an assertEquals(actual, expected) when it should have been
assertEquals(expected, actual).
Change-Id: I3b3c46c963afe2fb82a79c1e93970e73778877e5
Signed-off-by: Thomas Wolf <twolf@apache.org>
New readers of #prepareBitmapIndex may be confused about the manual
memory management (hidden mutation and nulling out pointers).
Add two clarifying comments to help future readers.
Change-Id: I93cab1919066efda37e96c47667f6991f67e377e
IO#readFully is often called with the intent to fill the destination
array from beginning to end. The redundant arguments for where to start
and stop filling are opportunities for bugs if specified incorrectly or
if not changed to match a changed array length.
Provide a overloaded method for filling the full destination array.
Change-Id: I964f18f4a061189cce1ca00ff0258669277ff499
Signed-off-by: Anna Papitto <annapapitto@google.com>
Mark the internal package as internal, visible only to the test bundle.
Add an API filter for CoreConfig.DEFAULT_COMMIT_GRAPH_ENABLE.
Change-Id: Ib62a93b873c93daf638b6c57e62fd267e16801bb
Signed-off-by: Thomas Wolf <twolf@apache.org>
The package-private findPostion method has a type in it. The typo will
become more widespread when a file-based implementation class is
introduced.
Correct the spelling to findPosition before the file-based
implementation is introduced.
Change-Id: Ib285f5a3f9a333ace1782dae9b5d425505eb962a
Signed-off-by: Anna Papitto <annapapitto@google.com>
If 'core.commitGraph' and 'gc.writeCommitGraph' are both true, then gc
will rewrite the commit-graph file when 'git gc' is run. Defaults to
false while the commit-graph feature matures.
Bug: 574368
Change-Id: Ic94cd69034c524285c938414610f2e152198e06e
Signed-off-by: kylezhao <kylezhao@tencent.com>
Git introduced a new file storing the topology and some metadata of
the commits in the repo (commitGraph). With this data, git can browse
commit history without parsing the pack, speeding up e.g.
reachability checks.
This change teaches JGit to read commit-graph-format file, following
the upstream format([1]).
JGit can read a commit-graph file from a buffered stream, which means
that we can provide this feature for both FileRepository and
DfsRepository.
[1] https://git-scm.com/docs/commit-graph-format/2.21.0
Bug: 574368
Change-Id: Ib5c0d6678cb242870a0f5841bd413ad3885e95f6
Signed-off-by: kylezhao <kylezhao@tencent.com>
Deleting orphan files depends on .pack and .keep being reverse-sorted
to before the corresponding index files that could be orphans. The new
reverse index file extension (.rev) will break that frail dependency.
Rewrite Gc#deleteOrphans to avoid that dependence by tracking which pack
names have a .pack or .keep file and then deleting any index files that
without a corresponding one. This approach takes linear time instead of
the O(n logn) time needed for sorting.
Change-Id: If83c378ea070b8871d4b01ae008e7bf8270de763
Signed-off-by: Anna Papitto <annapapitto@google.com>
These paths are given to the underlying URI-based transports (s3, sftp,
http), all of which expect forward-slash as the path separator
character.
Change-Id: I3cbb5928c9531a4da4691411bd8ac248fdf47ef2
CommitGraphWriter uses the GraphCommits in for-each loops and doesn't
need the access by position anymore. This was a left-over from
https://git.eclipse.org/r/c/jgit/jgit/+/182832
Remove the unused method.
Change-Id: I39df9bfab2601d581705ddf4cea3c04ed4765ff9
Errorprone run in the bazel build raised this exception:
org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/commitgraph/CommitGraphWriter.java:105:
error: [UnusedException] This catch block catches an exception and
re-throws another, but swallows the caught exception rather than setting
it as a cause. This can make debugging harder.
} catch (InterruptedIOException e) {
^
(see https://errorprone.info/bugpattern/UnusedException)
Did you mean 'throw new
IOException(JGitText.get().commitGraphWritingCancelled, e);'?
Change-Id: Iad832fe17955fc1e60e6a4902bc50fd9dca76b9d
There is no commit graph PackExt because the non-DFS stack is not writing using PackExt mechanism. The extension is needed in DFS to determine the stream to write the commit-graph.
Add a commit graph extension that matches the one in cgit
(https://git-scm.com/docs/commit-graph#_file_layout)
in preparation for adding DFS support for reading and writing commit graphs.
Change-Id: Id14eda9f116a319124981e0bcbc533928b1b5e8c
Signed-off-by: Xing Huang <xingkhuang@google.com>
The expression RefUpdate ru = newUpdate(cmd) is eagerly evaluated before the switch statement.
But it is not used in some switch cases and thus is calculated uselessly.
Move expression evaluation to the switch case where it is actually used.
After such a move, several cases became identical and thus were squashed.
Change-Id: Ifd1976f1c28378e092fb24d7ca9c415cba49f07f
After checking the variable, the same variable was checked again inside
the "if" block, and after the first check, this variable does not
change. Remove the second unnecessary check.
Change-Id: I6a38e67073f7f93105575b8f415ad32d350af602
The NameConflictTreeWalk class is used in merge for iterating over
entries in commits. The class uses a separate iterator for each
commit's tree. In rare cases it can incorrectly report the same entry
twice. As a result, duplicated entries are added to the merge result
and later jgit throws an exception when it tries to process merge
result.
The problem appears only when there is a directory-file conflict for
the last item in trees. Example from the bug:
Commit 1:
* subtree - file
* subtree-0 - file
Commit 2:
* subtree - directory
* subtree-0 - file
Here the names are ordered like this:
"subtree" file <"subtree-0" file < "subtree" directory.
The NameConflictTreeWalk handles similar cases correctly if there are
other files after subtree... in commits - this is processed in the
AbstractTreeIterator.min function. Existing code has a special
optimization for the case, when all trees are pointed to the same
entry name - it skips additional checks. However, this optimization
incorrectly skips checks if one of trees reached the end.
The fix processes a situation when some trees reached the end, while
others are still point to an entry.
bug: 535919
Change-Id: I62fde3dd89779fac282479c093400448b4ac5c86
* stable-6.3:
Remove unused imports
Suppress non-externalized String warnings
Remove unused API problem filters
Silence API errors
Silence API errors
Silence API warnings
Change-Id: I6778c8266bc7e918c943dcabf23aa230f4e998d5
* stable-6.2:
Remove unused imports
Suppress non-externalized String warnings
Remove unused API problem filters
Silence API errors
Silence API errors
Silence API warnings
Change-Id: I71aa9f890c5eb05849ad16a00b9974da5e51171e
introduced by
- addition of configurable SHA1 implementation in 5.13.2
- 3-digit @since 5.9.1 annotations on GitServlet methods
Change-Id: If19853fcc5e3677e5b18e8e3fbbcd2773378dffc
IndexEventConsumer metrics are reported per index PackExt and reverse
indexes did not have one, so they were not included.
Now that there is a REVERSE_INDEX PackExt, enable reporting
IndexEventConsumer metrics for reverse indexes.
Change-Id: Ia6a752f6eb8932a5b4ba45cc15cbc7e0786fd247
Signed-off-by: Anna Papitto <annapapitto@google.com>
Keys used for identifying reverse indexes in the DfsBlockCache use a
custom subclass ForReverseIndex because there was no PackExt for them.
This conflates BlockCacheMetrics for reverse indexes with those for
packs, since the key falls back onto 0 when there is no extension.
Replace the custom ForReverseIndex with a DfsStreamKey usage to bring
keys for the new REVERSE_INDEX extension in line with INDEX and BITMAP
and separate reverse index and pack BlockCacheMetrics.
Change-Id: I305e2c16d2a8cb2a824855ea92e0c9a9b188fce5
Signed-off-by: Anna Papitto <annapapitto@google.com>
Make sure we always get consistent results, whether or not we have the
full data in the buffer.
Change-Id: Ieb379a0c375ad3dd352e63ac2f23bda6ef16c215
Signed-off-by: Thomas Wolf <twolf@apache.org>
* stable-6.3:
[benchmarks] Remove profiler configuration
Add SHA1 benchmark
[benchmarks] Set version of maven-compiler-plugin to 3.8.1
Fix running JMH benchmarks
Add option to allow using JDK's SHA1 implementation
Fix API breakage caused by extracting WorkTreeUpdater
Extract Exception -> HTTP status code mapping for reuse
Don't handle internal git errors as an HTTP error
Ignore IllegalStateException if JVM is already shutting down
Allow to perform PackedBatchRefUpdate without locking loose refs
Change-Id: Ib58879be292c54a2a7f4936ac0986997985c822b
* stable-6.2:
Extract Exception -> HTTP status code mapping for reuse
Don't handle internal git errors as an HTTP error
Allow to perform PackedBatchRefUpdate without locking loose refs
Change-Id: I562be0802efa231023c5f10e6461339b2d7fbacf
* stable-6.1:
Extract Exception -> HTTP status code mapping for reuse
Don't handle internal git errors as an HTTP error
Allow to perform PackedBatchRefUpdate without locking loose refs
Change-Id: Icb321779184d20f3871e236fda1a3acba605a6da
* stable-6.2:
[benchmarks] Remove profiler configuration
Add SHA1 benchmark
[benchmarks] Set version of maven-compiler-plugin to 3.8.1
Fix running JMH benchmarks
Add option to allow using JDK's SHA1 implementation
Ignore IllegalStateException if JVM is already shutting down
Change-Id: I9c1576011c11b4ff8f453d18d9e786cee59860fa
* stable-6.1:
[benchmarks] Remove profiler configuration
Add SHA1 benchmark
[benchmarks] Set version of maven-compiler-plugin to 3.8.1
Fix running JMH benchmarks
Add option to allow using JDK's SHA1 implementation
Ignore IllegalStateException if JVM is already shutting down
Change-Id: Ie433c46a01a0f33848d54ecf99b30a44ca01e286
* stable-6.0:
[benchmarks] Remove profiler configuration
Add SHA1 benchmark
[benchmarks] Set version of maven-compiler-plugin to 3.8.1
Fix running JMH benchmarks
Add option to allow using JDK's SHA1 implementation
Ignore IllegalStateException if JVM is already shutting down
Change-Id: I176419026c3f4fdd8ebd34c61468c1ec3482ff45
There is no reverse index PackExt because the reverse index is not currently
written to a file. This prevents fine-grained performance reporting for reverse
indexes, which will be useful when introducing a reverse index file and
observing performance changes.
Add a reverse index extension that matches the one in cgit
(9bf691b78c/Documentation/gitformat-pack.txt (L302))
in preparation for adding a reverse index file while observing
performance before and after.
Change-Id: Iee53f1e01cf645a3c468892fcf97c8444f9a784a
Signed-off-by: Anna Papitto <annapapitto@google.com>
* stable-5.13:
[benchmarks] Remove profiler configuration
Add SHA1 benchmark
[benchmarks] Set version of maven-compiler-plugin to 3.8.1
Fix running JMH benchmarks
Add option to allow using JDK's SHA1 implementation
Ignore IllegalStateException if JVM is already shutting down
Change-Id: I40105336f0b9e593a8a2c242a9557f854c274fdc
The change If6da9833 moved the computation of SHA1 from the JVM's
JCE to a pure Java implementation with collision detection.
The extra security for public sites comes with a cost of slower
SHA1 processing compared to the native implementation in the JDK.
When JGit is used internally and not exposed to any traffic from
external or untrusted users, the extra cost of the pure Java SHA1
implementation can be avoided, falling back to the previous
native MessageDigest implementation.
Bug: 580310
Change-Id: Ic24c0ba1cb0fb6282b8ca3025ffbffa84035565e
448052dc2e made WorkTreeUpdater package visible which breaks API for
subclasses of ResolveMerger since they cannot access WorkTreeUpdater.
Fix this by moving WorkTreeUpdater into ResolveMerger as a nested class
and mark it protected so that subclasses can use it.
Bug: 581049
Change-Id: I5a2c4953f8514dc0a1b8041c8e069d28370bb2eb
This is from SonarLint (rule.java:S4348)
Regex patterns should not be created needlessly:
When String::replaceAll is used, the first argument should be a real
regular expression. If it’s not the case, String::replace does exactly
the same thing as String::replaceAll without the performance drawback of
the regex.
Change-Id: I00ba967ff4a27eeeb6fccf9373f6df2c94ecd823
The NameConflictTreeWalk class is used in merge for iterating over
entries in commits. The class uses a separate iterator for each
commit's tree. In rare cases it can incorrectly report the same entry
twice. As a result, duplicated entries are added to the merge result
and later jgit throws an exception when it tries to process merge
result.
The problem appears only when there is a directory-file conflict for
the last item in trees. Example from the bug:
Commit 1:
* subtree - file
* subtree-0 - file
Commit 2:
* subtree - directory
* subtree-0 - file
Here the names are ordered like this:
"subtree" file <"subtree-0" file < "subtree" directory.
The NameConflictTreeWalk handles similar cases correctly if there are
other files after subtree... in commits - this is processed in the
AbstractTreeIterator.min function. Existing code has a special
optimization for the case, when all trees are pointed to the same
entry name - it skips additional checks. However, this optimization
incorrectly skips checks if one of trees reached the end.
The fix processes a situation when some trees reached the end, while
others are still point to an entry.
bug: 535919
Change-Id: I62fde3dd89779fac282479c093400448b4ac5c86
Before this change JGit did not support the session-id capability
implemented by native Git in UploadPack. This change implements
advertising the capability from the server and parsing the session-id
received from the client during an UploadPack operation.
Enable the transfer.advertisesid config setting to advertise the
capability from the server. The client may send a session-id capability
in response. If received, the value from this is parsed and available
via the getClientSID method on the UploadPack object.
This change does not add the capability to send a session-id from the
JGit client.
https://git-scm.com/docs/gitprotocol-capabilities#_session_idsession_id
Change-Id: Ib1b6929ff1b3a4528e767925b5e5c44b5d18182f
Signed-off-by: Josh Brown <sjoshbrown@google.com>
The config setting to enable advertising the session-id capability is
currently read in the ReceivePack class. This change moves it to a
common location in the TransferConfig class so that it can be reused
in other places like UploadPack. TransferConfig is also a more logical
place for the setting as it resides in the `transfer` config section.
Set the transfer.advertisesid setting to true to send the session-id
capability to the client.
Change-Id: If68ecb5e68b59f5c452a7992d02e3688b0a86747
Signed-off-by: Josh Brown <sjoshbrown@google.com>
In protocol V0 the client capabilities are appended to the first line.
Parsing session-id is currently only supported during a ReceivePack
operation. This change will parse the client session-id capability if
it has been sent by the client.
If the server sends the session-id capability to the client. The client
may respond with a session ID of its own. FirstWant.fromLine will now
parse the ID and make it available via the getClientSID method.
This change does not add support to send the session-id capability from
the server. The change is necessary to support session-id in UploadPack.
Change-Id: Id3fe44fdf9a72984ee3de9cf40cc4e71d434df4a
Signed-off-by: Josh Brown <sjoshbrown@google.com>
Before this change JGit did not support the session-id capability
implemented by native Git. This change implements advertising the
capability from the server and parsing the session-id received from
the client during a ReceivePack operation.
Enable the transfer.advertisesid config setting to advertise the
capability from the server. The client may send a session-id capability
in response. If received, the value from this is parsed and available
via the getClientSID method on the ReceivePack object. All capabilities
in the form `capability=value` are now split into key value pairs at the
first `=` character. This change replaces specific handling for the
agent capability.
This change does not add advertisement or parsing to UploadPack. This
change also does not add the ability to send a session ID from the JGit
client.
https://git-scm.com/docs/protocol-v2/2.33.0#_session_idsession_id
Change-Id: I56fb115e843b11b27e128c4ac427b05d5ec129d0
Signed-off-by: Josh Brown <sjoshbrown@google.com>
Trying to register/unregister a shutdown hook when the JVM is already in
shutdown throws an IllegalStateException. Ignore this exception since we
can't do anything about it.
Bug: 580953
Change-Id: I8fc6fdd5585837c81ad0ebd6944430856556d90e
Add another newBatchUpdate method in the RefDirectory where we can
control if the created PackedBatchRefUpdate will lock the loose refs or
not.
This can be useful in cases when we run programs which have exclusive
access to a Git repository and we know that locking loose refs is
unnecessary and just a performance loss.
Change-Id: I7d0932eb1598a3871a2281b1a049021380234df9
Reachability bitmaps are designed to speed up the "counting objects"
phase of generating a pack during a clone or fetch. They are not
optimized for Git clients sending a small topic branch via "git push".
In some cases (see [1]), using reachability bitmaps during "git push"
can cause significant performance regressions.
Add PushCommand#setUseBitmaps(boolean) to allow users to tell "git push"
not to use bitmaps.
[1]: https://lore.kernel.org/git/87zhoz8b9o.fsf@evledraar.gmail.com/
Change-Id: I7fb7d26084ec63ddfa7249cf58abb85929b30e56
Signed-off-by: kylezhao <kylezhao@tencent.com>
Fix and complete the implementation of calling the pre-push hook.
Add the missing error stream redirect, and add the missing setters
in Transport and in PushCommand. In Transport, delay setting up a
PrePushHook such that it happens only on a push. Previously, the
hook was set up also for fetches.
Bug: 549246
Change-Id: I64a576dfc6b139426f05d9ea6654027ab805734e
Signed-off-by: Thomas Wolf <twolf@apache.org>
We need the full size of the objects to populate the object-size index
of a pack. This size is not always the one encoded in the object header
in the pack (e.g. for deltas).
Populate the full size of PackedObjectInfos in the PackParser, which is
invoked when receiving a pack e.g. in a push.
Change-Id: I102c20901aefb5e85047e2e526c0d733f82ff74b
Partial clones filter the objects to send by size calling
ObjectReader#getObjectSize per object. This method reads the object from
storage to get the size, which can be expensive.
Offer a #isNotLargerThan method. The default implementation reads the
object, but subclasses can override it with more efficient lookups (e.g.
adding an index).
isNotLargerThan gives implementors more options to optimize than
getObjectIndex (e.g. can be implemented storing only object over certain
size).
Change-Id: Iefd4b1370cb9144f15cc0391286aeeb365e6ea87
On java 17 + Windows OS java.io.File.getCanonicalPath is a very slow
system call which uses most time during clone.
That is since JDK 12 the result of File.getCanonicalPath is not cached
anymore by default:
https://bugs.openjdk.java.net/browse/JDK-8207005
* Use toRealPath() to follow symbolic links also on windows.
* Cache the result.
Bug: 580568
Change-Id: I95f4f5b2babefd7210ee4740646230225ebf3788
CloneCommand, when setNoCheckout(true) was set, did not set HEAD.
With C git, "git clone --no-checkout" does.
Change-Id: Ief3df7e904ce90829a6345a6c3e9ee6a68486ab0
Signed-off-by: Thomas Wolf <twolf@apache.org>
FetchCommand.setShallowSince() and Transport.setDeepenSince() require
a non-null argument.
Change-Id: I1c3a20be518374e380a4e90787ed834438da40ee
Signed-off-by: Thomas Wolf <twolf@apache.org>
PatchApplier now routes updates through the index. This has two
results:
* we can now execute patches in-memory.
* the JGit apply command will now always update the
index to match the working tree.
Change-Id: Id60a88232f05d0367787d038d2518c670cdb543f
Co-authored-by: Han-Wen Nienhuys <hanwen@google.com>
Co-authored-by: Nitzan Gur-Furman <nitzan@google.com>
Previous code would do a content merge on symlinks, and write the merge
result to the working tree as a file. C git doesn't do this; it leaves
a symlink in the working tree unchanged, or in a delete-modify conflict
it would check out "theirs".
Moreover, previous code would write the merge result to the link target,
not to the link. This would overwrite an existing link target, or fail
if the link pointed to a directory.
In link/file conflicts or file/link conflicts, C git always puts the
file into the working tree.
Change conflict handling accordingly. Add tests for all the conflict
cases.
Bug: 580347
Change-Id: I3cffcb4bcf8e336a85186031fff23f0c4b6ee19d
Signed-off-by: Thomas Wolf <twolf@apache.org>
* changes:
Move WorkTreeUpdater to merge package
WorkTreeUpdater: use DirCacheCheckout#StreamSupplier
DirCacheCheckout#getContent: also take InputStream supplier
WorkTreeUpdater: remove safeWrite option
This lets us use DirCacheCheckout for routines that want to write
files in the worktree that aren't available as a git object.
DirCacheCheckout#getContent takes a InputStream supplier rather than
InputStream: if filtering fails with IOException, the data is placed
unfiltered in the checkout. This means that the stream has to be read
again, from the start.
Use it in this way in ApplyCommand. This use is incorrect, though: the
same InputStream is returned twice, so if the read to be retried, the
stream will return 0 bytes. It doesn't really matter, because in
either case, the SHA1 will not match up, and the patch fails.
Change-Id: I2efa9a6da06806ff79b155032fe4b34be8fec09e
DAG."
This reverts commit 6297491e8a.
This is done as a quick fix for the failure of egit tests caused by the
introduction of FilteredRevCommit.
Bug: 580690
Change-Id: Ia6b651dd11b0a4b02d5e52247eb4bf13adf94e27
BlameGenerator."
This reverts commit 5747bba48b.
This is done as a quick fix for the failure of egit tests caused by the
introduction of FilteredRevCommit.
Bug: 580690
Change-Id: Ia0178bc2de4fc825a81207bbd7979bf3a386c955
This was added in Ideaefd5178 to anticipate on writing files for
ApplyCommand, but we are keeping WorkTreeUpdater private to the merge
package for now.
Change-Id: Ifa79dac245e60eb7a77eaea4cc1249222e347d38
An invalid path in the manifest (e.g. '.') is reported by DirCache in a
runtime exception. In server context this becomes a 500 instead of a user error.
Wrap the runtime invalid path exception into a checked ManifestErrorException that
caller can handle.
Change-Id: I61a2104922765506ae232334891057bb06141d97
On executing a copy, mark the destination as updated.
On executing a rename, mark both source and destination as updated.
Change-Id: Ied5b9b0e5a14eac59a06cdd0961e25e143f50ff0
This can allow passing a FilteredRevCommit which is the filtered list of
commit graph making it easier for Blame to work on. This can
significantly improve blame performance since blame can skip expensive
RevWalk.
Change-Id: Ie127cb710d004079e9f53a5802130afdb49a7de1
Reformat using the standard JGit formatter settings. Clean-ups:
* Try to improve javadoc.
* Remove blindly copy-pasted "@since 6.1" annotations.
* Get rid of private method nonNullNonBareRepo(); it's not needed.
* Simplify method nonNullRepo(), and annotate as @NonNull.
* Rename setInCoreFileSizeLimit() to getInCoreFileSizeLimit().
Change-Id: Ib1797e7cf925d87554307468330971e8ab2e05e9
Signed-off-by: Thomas Wolf <twolf@apache.org>
Signed-off-by: Han-Wen Nienhuys <hanwen@google.com>
Previous code loaded the WorkingTreeOptions afresh for every single
file being checked out. This checked the git config (all three files,
repo, user and system config) for having been modified every time.
These checks can be costly, for instance on Windows, or if one of the
three config files is not on a local disk, or on an otherwise slow
storage.
Improve this by loading the options and thus checking the git config
only once before the checkout.
Bug: 579715
Change-Id: I21cd5a808f9d90b5ca2d022f91f0eeb8ca26091c
Signed-off-by: Thomas Wolf <twolf@apache.org>
1. A TemporaryBuffer.LocalFile must be destroyed to ensure the
temporary file gets deleted on disk.
2. TemporaryBuffer.openInputStream() may be used only after
TemporaryBuffer.close().
3. The caller of DirCacheCheckout.getContent() is responsible for
closing the OutputStream!
Change-Id: I46abb0fba27656a1026858e5783fc60d4738a45e
Signed-off-by: Thomas Wolf <twolf@apache.org>
With core.symlinks=false, symlinks are checked out as plain files.
When such a file is re-added to the index, and the index already
contains a symlink there, add the file as a symlink. Previous code
changed the index entry to a regular file.
Bug: 580412
Change-Id: I5497bedc3da89c8b10120b8077c56bc5b67cb791
Signed-off-by: Thomas Wolf <twolf@apache.org>
- add missing @since 6.3 for new protected field workTreeUpdater and new
class WorkTreeUpdater
- suppress API errors caused by removing/adding protected fields and
methods
We follow OSGi semantic versioning which allows breaking implementers in
minor versions which are e.g. subclassing a public class.
Change-Id: I28f0d7b4fdd9a1f0fbc6b137d6c68dda9fe3c11e
* changes:
Reapply "Create util class for work tree updating in both filesystem and index."
ResolveMerger: add coverage for inCore file => directory transition
I649db9ae679ec2606cf7c530b040f8b6b93eb81a added a default implementation
for getShallowCommits and setShallowCommits to DfsObjDatabase, for the
convenience of any implementers that define subclasses. But we forgot
that some implementers inherit from ObjectDatabase directly instead.
Move the default getter and setter to the base class so that such
callers do not need source changes to unbreak their build.
This also lets us update the api_filters to reflect that this is no
longer an API-breaking change.
Change-Id: I5dcca462eb306e511e57907b7d9264d51b3f3014
This reverts commit 5709317f71.
Add a bugfix for deletions in ResolveMergers instantiated with just an
ObjectInserter as argument.
Original change description:
Create util class for work tree updating in both filesystem and index.
This class intends to make future support in index updating easier.
This class currently extracts some logic from ResolveMerger. Logic
related to StreamSupplier was copied from ApplyCommand, which will be
integrated in a following change.
Co-authored-by: Nitzan Gur-Furman <nitzan@google.com>
Co-authored-by: Han-Wen Nienhuys <hanwen@google.com>
Change-Id: Ideaefd51789a382a8b499d1ca7ae0146d032f48b
This reverts commit 5151b324f4. It is
producing NullPointerExceptions during merges, causing Gerrit's
acceptance tests to fail:
com.google.gerrit.extensions.restapi.RestApiException: Cannot rebase ps
[...]
at com.google.gerrit.server.api.changes.RevisionApiImpl.rebase(RevisionApiImpl.java:280)
at com.google.gerrit.acceptance.api.change.ChangeIT.rebaseChangeBase(ChangeIT.java:1584)
Caused by: com.google.gerrit.server.update.UpdateException: java.lang.NullPointerException: repository is required
at com.google.gerrit.server.update.BatchUpdate.executeUpdateRepo(BatchUpdate.java:588)
[...]
Caused by: java.lang.NullPointerException: repository is required
at org.eclipse.jgit.merge.Merger.nonNullRepo(Merger.java:128)
at org.eclipse.jgit.merge.ResolveMerger.addDeletion(ResolveMerger.java:380)
at org.eclipse.jgit.merge.ResolveMerger.processEntry(ResolveMerger.java:553)
at org.eclipse.jgit.merge.ResolveMerger.mergeTreeWalk(ResolveMerger.java:1224)
at org.eclipse.jgit.merge.ResolveMerger.mergeTrees(ResolveMerger.java:1174)
at org.eclipse.jgit.merge.ResolveMerger.mergeImpl(ResolveMerger.java:299)
at org.eclipse.jgit.merge.Merger.merge(Merger.java:233)
at org.eclipse.jgit.merge.Merger.merge(Merger.java:186)
at org.eclipse.jgit.merge.ThreeWayMerger.merge(ThreeWayMerger.java:96)
at com.google.gerrit.server.change.RebaseChangeOp.rebaseCommit(RebaseChangeOp.java:360)
Change-Id: Idf63de81666d0df118d2d93c4f6e014e00dc05b8
Jgit change https://git.eclipse.org/r/c/jgit/jgit/+/193329 adds an implementation for get/set shallow commits in ObjectDatabase. This failed gerrit's acceptance tests since there is no default implementation for them in DfsObjDatabase.
Change-Id: I649db9ae679ec2606cf7c530b040f8b6b93eb81a
String.startsWith() is not a valid test for file path prefixes:
directory "a" is _not_ a prefix of a file "ab", only of "a/b".
Add a proper Paths.isEqualOrPrefix() method and use it in CleanCommand.
Bug: 580478
Change-Id: I6863e6ba94a8ffba6561835cc57044a0945d2770
Signed-off-by: Thomas Wolf <twolf@apache.org>
This can allow passing a FilteredRevCommit which is the filtered list of
commit graph making it easier for Blame to work on. This can
significantly improve blame performance since blame can skip expensive
RevWalk.
Change-Id: I5dab25301d6aef7df6a0bc25a4c553c730199272
This makes RevCommit extensible to allow having different structure of
child-parent relationship. This change is a pre-requsite for having a
FilteredRevCommit that overrides parents from the RevCommit. That then
provides a cheaper way to walk over a subset of RevCommits instead of
an expensive way that applies filters while walking over selected
commits. Useful with Blame which works on a single file and that can be
made performant, if we know all the commits needed by the Blame
algorithm. So Blame algorithm can avoid walking over finding what
commits to blame on.
This change makes parents field on RevCommit private and exposes it
thrrough overrideable methods such as getParents, getParent at index,
getParentCount and setParents. All other files other than RevCommit are
updating the usages of accessing RevCommits parents.
Change-Id: I2d13b001c599cc4ebc92d1ab6e07b07acb3b7fe5
Introduce named constants for packet line headers and use them instead
of direct string literals everywhere. This not only makes the code more
readable because we don't need NON-NLS markers, it also makes it more
robust since we can use the length of these constants instead of magic
numbers.
Change-Id: Ie4b7239e0b479a68a2dc23e6e05f25061d481a31
Signed-off-by: Thomas Wolf <twolf@apache.org>
This adds support for shallow cloning. The CloneCommand and the
FetchCommand now have the new methods setDepth, setShallowSince and
addShallowExclude to tell the server that the client doesn't want to
download the complete history.
Bug: 475615
Change-Id: Ic80fb6efb5474543ae59be590ebe385bec21cc0d