Thomas Wolf [Fri, 1 Apr 2022 14:56:05 +0000 (16:56 +0200)]
[sshd] Better user feedback on authentication failure
When authentication fails, JGit produces an exception with an error
message telling the user that it could not log in (including the host
name). The causal chain has an SshException from Apache MINA sshd with
message "No more authentication methods available".
This is not very helpful. The user was left without any indication why
authentication failed.
Include in the exception message a log of all attempted authentications.
That way, the user can see which keys were tried, in which order and
with which signature algorithms. The log also reports authentication
attempts for gssapi-with-mic or password authentication. For
keyboard-interactive Apache MINA sshd is lacking a callback interface.
The way Apache MINA sshd loads keys from files, the file names are lost
in higher layers. Add a mechanism to record on the session for each
key fingerprint the file it was loaded from, if any. That way the
exception message can refer to keys by file name, which is easier to
understand by users than the rather cryptic key fingerprints.
Bug: 571390
Change-Id: Ic4b6ce6b99f307d5e798fcc91b16b9ffd995d224 Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>
Thomas Wolf [Sun, 3 Apr 2022 19:06:57 +0000 (21:06 +0200)]
De-couple ServiceLoader calls from class loading
Use the holder pattern to de-couple the loading of super classes from
the ServiceLoader calls to set up global instances. This prevents
potential lock inversions.
Bug: 579550
Change-Id: Ie8284e4d6d680ddd4cc6a486bbefe8ed00266240 Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>
Thomas Wolf [Sun, 3 Apr 2022 18:11:29 +0000 (20:11 +0200)]
GpgSigner: prevent class lock inversion on the default signer
Don't store the default signer in a static field of the abstract
superclass GpgSigner. This many lead to a lock inversion on the class
initialization locks if there are concurrent loads of the GpgSigner
class and of one of its subclasses, and that subclass happens to be
the one the ServiceLoader wants to load.
Use the holder pattern to de-couple the loading of class GpgSigner
from the ServiceLoader call.
Bug: 579550
Change-Id: Ifac0ea0c8985a09fe0518d0dabc072fafd6db907 Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>
yunjieli [Mon, 28 Mar 2022 21:47:02 +0000 (14:47 -0700)]
Fetch: Introduce negative refspecs.
Implement negative refspecs in JGit fetch, following C Git. Git
supports negative refspecs in source only while this change supports
them in both source and destination.
If one branch is equal to any branch or matches any pattern in the
negative refspecs collection, the branch will not be fetched even if
it's in the toFetch collection.
With this feature, users can express more complex patterns during fetch.
Introduce a new benchmark that shows a typical use-case
of opening a cached repository and fetching one ref from
a repository with a high number of refs.
This specific benchmark is tailored to the Gerrit use-case
of reading frequently individual refs SHA1s and by ref-name
prefix from the All-Users repository.
Include the following variables for the benchmark:
- numBranches (from 100 up to 50000)
- trustFolderStat (true or false)
- useRefTable (true or false)
The benchmark needs to be evaluated on a local high-perf SSD
and on a slower NFS network disk.
Matthias Sohn [Wed, 16 Mar 2022 15:32:06 +0000 (16:32 +0100)]
[pgm tests] Relax version constraints for hamcrest
We updated hamcrest to 2.2. but there is no need to prevent that
older versions of hamcrest can be used. Hence relax the lower bound
to 1.1 like in other bundles.
Thomas Wolf [Sun, 23 Jan 2022 14:43:24 +0000 (15:43 +0100)]
Use git config core.commentChar
This concerns committing, creating merge conflict messages and creating
and editing squash messages. In a squash message, once the comment
character has been determined initially is always the first character.
Note that if core.commentChar=auto and there is a sequence of squashes,
it may be necessary to change the comment character when a new message
is added.
Bug: 579325
Change-Id: Idca19284a0240cd322e7512ea299a03658e1b2c1 Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>
Thomas Wolf [Sun, 20 Feb 2022 23:20:52 +0000 (00:20 +0100)]
[push, lfs] Tell the pre-push hook whether the push is a dry run
This is a feature that does not exist in C git: an external pre-push
hook doesn't know whether the push is run as a dry run. But for
internal hooks written in Java it is entirely possible to give a hook
this information.
In JGit with its internal LFS implementation, this enables us to not
perform LFS uploads in a dry run. This is kind of important because
EGit frequently does a dry-run before doing the actual push to give the
user a way to review what would be pushed before it actually happens.
Doing an LFS upload of potentially huge files during a dry-run is
wasteful, makes the dry run not actually a dry run, and leads to
uploading the same file twice if followed by a real push.
Use the information in the LfsPrePushHook to only do the initial call
to the LFS server, but then skipping the actual upload if the push is
a dry run. That way, a failure to contact the LFS server leads to an
error in the dry run, as it should.
Bug: 546567
Change-Id: I155430f27c4979d91096ba72fd95c3775dd3f28b Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>
Thomas Wolf [Tue, 15 Mar 2022 21:48:48 +0000 (22:48 +0100)]
DirCacheCheckout: use a LinkedHashMap instead of HashMap
This guarantees that updates are checked out in git order, which
is important for LFS if a .lfsconfig file is used. That file comes
early in git order, and the LFS smudge filter will consider the
working tree version. To ensure that on branch switches the correct
version of that file is used, the checkout order must be stable and
should be the git order.
Change-Id: I20f6d11bf08558f9d5adfd2be71e36321460038c Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>
Thomas Wolf [Tue, 15 Mar 2022 18:12:46 +0000 (19:12 +0100)]
Re-try reading a file when there are concurrent writes
Git and JGit are very careful to replace git files atomically when
writing. The normal mechanism for this is to write to a temporary
file and then to rename it atomically to the final destination. This
works fine on POSIX-compliant systems, but on systems where renaming
may not be atomic, exceptions may be thrown if code tries to read
the file while the rename is still ongoing. This happens in particular
on Windows, where the typical symptom is that a FileNotFoundException
with message "The process cannot access the file because it is being
used by another process" is thrown, but file.isFile() == true at the
same time.
In FileBasedConfig, a re-try was already implemented for this case.
But the same problem can also occur in other places, for instance
in RefDirectory when reading loose or packed refs. Additionally,
JGit has similar re-tries when a stale NFS file handle is detected,
but that mechanism wasn't used consistently (only for git configs
and packed refs, but not for loose refs).
Factor out the general re-try mechanism for reading into a new method
FileUtils.readWithRetry() and use that in all three places. The
re-try parameters are hardcoded: at most 5 times for stale NFS handles,
and at most 5 times with increasing backoff delays (50, 100, 200, 400,
and 800ms) for the above concurrent write case.
Bug: 579116
Change-Id: If0c2ad367446d3c0f32b509274cf8e814aca12cf Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>
Thomas Wolf [Sat, 26 Feb 2022 11:46:50 +0000 (12:46 +0100)]
[sideband] Ensure last bit of progress channel is written
If the last sideband progress message didn't end in \r or \n, there
may still be a buffered message at the end of a fetch or push. Ensure
that message gets written, too, even if it may be only partial.
Bug: 575629
Change-Id: I38edccb5cffb89e00e468480b43c7d951fb63e8e Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>
and LsRemoteCommand is called with an HTTPS GitHub repository URL, the
command should actually rewrite this to an SSH URI and use the SSH
transport.
Actually this same problem may exist everywhere Transport is used with
an URIish instead of with a remote name. However, doing this translation
in Transport.open(URIish) and in Transport.open(Repository, URIish,
String) if no remote name is given would change the behavior and might
break assumptions made in existing clients. For instance, EGit's
PushOperation assumes that the URI obtained from PushResult.getURI()
was the same as was passed in to Transport.open(Repository, URIish).
URIs obtained from a RemoteConfig have this translation applied
transparently, and in Transport we cannot know for sure whether or
not a URI has already been translated, if needed. So doing this in
Transport might also lead to translating URIs twice.
Therefore this commit does the translation in LsRemoteCommand, where
we can be sure that it won't affect other use cases. If other cases
besides LsRemoteCommand are found where such a URI translation is
missing, it'll have to be done at higher levels, possibly even in
client code directly.
Bug: 544769
Change-Id: I5df54a925d30b55d98e21f37f2851fe79649b064 Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>
Matthias Sohn [Tue, 8 Mar 2022 16:23:57 +0000 (17:23 +0100)]
Merge branch 'stable-6.1'
* stable-6.1:
Prepare 6.1.1-SNAPSHOT builds
JGit v6.1.0.202203080745-r
[checkout] Use .gitattributes from the commit to be checked out
Don't use final for method parameters
[push] support the "matching" RefSpecs ":" and "+:"
[push] Call the pre-push hook later in the push process
IndexDiff: use tree filter also for SubmoduleWalk
Run license check with option -Ddash.projectId=technology.jgit
Exclude transitive dependencies of sshd-sftp
Update DEPENDENCIES for 6.1.0 release
Add dependency to dash-licenses
Fix typos of some keys in LfsText
Sort LfsText entries alphabetically
Support for "lfs.url" from ".lfsconfig"
Thomas Wolf [Sun, 13 Feb 2022 22:30:36 +0000 (23:30 +0100)]
[checkout] Use .gitattributes from the commit to be checked out
JGit used only one set of attributes constructed from the global and
info attributes, plus the attributes from working tree, index, and
HEAD.
These attributes must be used to determine whether the working tree is
dirty.
But for actually checking out a file, one must use the attributes from
global, info, and *the commit to be checked out*. Otherwise one may not
pick up definitions that are only in the .gitattributes of the commit
to be checked out or that are changed in that commit with respect to
the attributes currently in HEAD, the index, or the working tree.
Maintain in TreeWalk different Attributes per tree, and add operations
to determine EOL handling and smudge filters per tree.
Use the new methods in DirCacheCheckout and ResolveMerger. Note that
merging in JGit actually used the attributes from the base, not those
from ours, which looks dubious at least. It now uses those from ours,
and for checking out the ones from theirs.
The canBeContentMerged() determination was also done from the base
attributes, and is newly done from the ours attributes. Possibly this
should take into account all three attributes, and only if all three
agree the item can be content merged, a content merge should be
attempted? (What if the binary/text setting changes between base, ours,
or theirs?)
Also note that JGit attempts to perform content merges on non-binary
LFS files; there it used the filter attribute from base, too, even for
the ours and theirs versions. Newly it takes the filter attribute from
the correct tree. I'm not convinced doing content merges on potentially
huge files like LFS files is really a good idea.
Add tests in FilterCommandsTest and LfsGitTest to verify the behavior.
Open question: using index and working tree as fallback for the
attributes of ours (assuming it is HEAD) is OK. But does it also make
sense for base and theirs in merging?
Bug: 578707
Change-Id: I0bf433e9e3eb28479b6272e17c0666e175e67d08 Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>
Thomas Wolf [Sun, 20 Feb 2022 17:11:49 +0000 (18:11 +0100)]
[push] support the "matching" RefSpecs ":" and "+:"
The implementation of push.default=matching was not correct.
It used the RefSpec "refs/heads/*:refs/heads/*", which would push
_all_ local branches. But "matching" must push only those local
branches for which a remote branch with the same name already exists
at the remote.
This RefSpec can be expanded only once the advertisement from the
remote has been received.
Enhance RefSpec so that ":" and "+:" can be represented. Introduce a
special RemoteRefUpdate for such a RefSpec; it must carry through the
fetch RefSpecs to be able to fill in the remote tracking updates as
needed. Implement the expansion in PushProcess.
Bug: 353405
Change-Id: I54a4bfbb0a6a7d77b9128bf4a9c951d6586c3df4 Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>
Thomas Wolf [Sun, 20 Feb 2022 17:10:24 +0000 (18:10 +0100)]
[push] Call the pre-push hook later in the push process
Call the pre-push hook only after having received the remote
advertisement and having determined rejections, like C git does.
Also similar to C git, don't pass rejected or up-to-date updates
to the pre-push hook.
Bug: 578852
Change-Id: I51d379ea7bd8234ec815f8f4a9fa325816f476cf Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>
Thomas Wolf [Fri, 17 Jul 2020 17:41:31 +0000 (19:41 +0200)]
IndexDiff: use tree filter also for SubmoduleWalk
The only uses of IndexDiff.setFilter() in JGit and EGit set a path
filter. Passing the filter on to the SubmoduleWalk gives the desired
result, which is consistent with command-line git.
Bug: 565251
Change-Id: I8eca1ed73eb1d237b8785f369352f72af9e0e168 Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>
Matthias Sohn [Thu, 3 Mar 2022 23:30:36 +0000 (00:30 +0100)]
Exclude transitive dependencies of sshd-sftp
We don't need the dependencies of sshd-sftp to sshd-common and
sshd-core since they are contained in sshd-osgi.
Excluding them helps the dash IP log tool to not list them as required
dependencies of jgit.
Matthias Fromme [Mon, 3 Jan 2022 12:06:34 +0000 (13:06 +0100)]
Support for "lfs.url" from ".lfsconfig"
- New class LfsConfig to enrich repository configuration by settings
from ".lfsconfig" file respecting configuration file precedence.
- Adapted LfsConnectionFactory to use LfsConfig instead of directly
using configuration from repository to calculate url of the lfs
repository
Matthias Sohn [Wed, 2 Mar 2022 20:10:03 +0000 (21:10 +0100)]
Merge branch 'master' into stable-6.1
* master:
Describe: add support for core.abbrev config option
Add a typed config getter for integers confined to a range
Remove odd prefix of PersonIdent test class
PersonIdent: Add ctors that accept Instant in addition to Date
Remove ignored potentiallyUnclosedCloseable check
Make precedence more explicit
[pgm] Add describe --abbrev option
Cap describe abbrev option
DescribeCommand: Add support for --abbrev=0
Matthias Sohn [Mon, 21 Feb 2022 01:29:27 +0000 (02:29 +0100)]
Describe: add support for core.abbrev config option
If core.abbrev is unset or "auto" estimate abbreviation length like C
git does:
- Estimate repository's object count by only considering packed objects,
round up to next power of 2
- With the order of 2^len objects, we expect a collision at 2^(len/2).
But we also care about hex chars, not bits, and there are 4 bits per
hex. So all together we need to divide by 2; but we also want to round
odd numbers up, hence adding one before dividing.
- For small repos use at least 7 hexdigits
- If object database fails to determine object count use 7 hexdigits as
fallback
If it is set to "no" do not abbreviate object-ids.
Otherwise set it to the configured value capped to the range between 4
and length of an unabbreviated object-id.
David Ostrovsky [Wed, 29 Dec 2021 20:18:52 +0000 (21:18 +0100)]
PersonIdent: Add ctors that accept Instant in addition to Date
Error Prone is flagging Date-API as obsolete and recommends to migrate
to Instant and LocalDate. Given that more JGit users starting to migrate
to new Time API, offer ctors that accept Instant type and also add new
getter that returns when attribute as Instant type.
Fabio Ponciroli [Thu, 3 Feb 2022 08:49:13 +0000 (09:49 +0100)]
Remove SuppressWarnings since currently ignored
The following warning was raised by Eclipse:
"At least one of the problems in category
'unused' is not analysed due to a compiler option being ignored"
The org.eclipse.jdt.core.compiler.problem.unusedTypeParameter compiler
option is set to ignore, hence the warning suppression is redundant.
Thomas Wolf [Sun, 13 Feb 2022 22:24:14 +0000 (23:24 +0100)]
Simplify implementation of WorkingTreeIterator
All the filtering in WorkingTreeIterator is for check-in, i.e., clean
filtering. The implementation was in some parts too general, passing
around an OperationType. But since it's always CHECKIN_OP, that's not
actually necessary.
Change-Id: I73f8bc059e485a073e456962868f52b3a3db4fc1 Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>
Thomas Wolf [Fri, 11 Feb 2022 16:18:20 +0000 (17:18 +0100)]
Transport: load all refs only if push refspecs have wildcards
There is no need to load all refs if there are no wildcard push
refspecs. Load them lazily on the first wildcard refspec encountered
instead of loading them up-front.
Change-Id: I6d0e981f9ed4997dbdefeb7f83f37ff4f33e06a5 Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>
Thomas Wolf [Thu, 10 Feb 2022 18:04:21 +0000 (19:04 +0100)]
PushCommand: determine remote from git config if not given
Add ConfigConstants and expose branch.<name>.pushRemote in the
BranchConfig. Use the branch configuration and remote.pushDefault
if no remote is given explicitly. If nothing is configured, fall
back to "origin".
Bug: 578676
Change-Id: I6bb141ff02c8b04980ec34b26ef248b72614c3c9 Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>
Rolf Theunissen [Sun, 10 Feb 2019 17:40:26 +0000 (18:40 +0100)]
PushCommand: consider push.default when no RefSpecs are given
When no RefSpecs are given, PushCommand until now simply fell back to
pushing the current branch to an upstream branch of the same name. This
corresponds to push.default=current. Any setting from the git config
for push.default was simply ignored.
Implement the other modes (nothing, matching, upstream, and simple),
too. Add a setter and getter for the PushDefault so that an application
can force a particular mode to be used. For backwards compatibility,
use "current" as the default setting; to figure out the value from the
git config, which defaults to "simple", call setPushDefault(null).
Bug: 351314
Change-Id: I86c5402318771e47d80b137e99947762e1150bb4 Signed-off-by: Rolf Theunissen <rolf.theunissen@gmail.com> Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>
BasePackConnection::readAdvertisedRefsImpl was creating an exception by
calling `noRepository`, and then blindly calling `initCause` on it. As
`noRepository` can be overridden, it's not guaranteed to be missing a
cause.
BasePackPushConnection overrides `noRepository` and initiates a fetch,
which may throw a `NoRemoteRepositoryException` with a cause.
In this case calling `initCause` threw an `IllegalStateException`.
In order to throw the correct exception, we now return the
BasePackPushConnection exception and suppress the one thrown by
BasePackConnection
Nail Samatov [Mon, 7 Feb 2022 15:13:58 +0000 (18:13 +0300)]
Support LFS Server URL without .git suffix
According to Git LFS documentation, URLs with and without .git suffix
should be supported. By default, Git LFS will append .git/info/lfs to
the end of a Git remote URL. To build the LFS server URL it will use:
Fix the LfsConnectionFactory accordingly. Move a utility method to
add the ".git" suffix if not present yet from FileResolver to
StringUtils and use it.
Bug: 578621
Change-Id: I8d3645872d5f03bb8e82c9c73647adb3e81ce484 Signed-off-by: Nail Samatov <sanail@yandex.ru> Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>
Add a way for the handler to tell whether the commit should generate a
Gerrit Change-Id. Augment the ModifyResult interface, and set the flag
on the CommitCommand.
This enables users to have a Change-ID be generated when squashing or
rewording commits. A possibly already existing Change-Id will remain
unchanged.
Bug: 440211
Change-Id: I66a72e0646876d162a7011235cca969e20acf060 Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>
Thomas Wolf [Mon, 17 Jan 2022 23:37:33 +0000 (00:37 +0100)]
RebaseCommand: fix commit message in "fixup" case
JGit accumulated in MESSAGE_FIXUP commit messages of a fixup sequence,
just like it did in MESSAGE_SQUASH, and on the last step of a sequence
of fixups used that file, after stripping all comment lines, as the
commit message. That also stripped any lines from the original commit
message that happened to start with the comment character.
This is not how this is supposed to work. MESSAGE_FIXUP must contain
the original commit message of the base commit that is amended, and
the file contains the verbatim commit message for the final fixup.[1]
Change the implementation accordingly, and add new tests.
Han-Wen Nienhuys [Mon, 31 Jan 2022 11:23:55 +0000 (12:23 +0100)]
reftable: tweaks for Windows
Reload the stack _before_ trying to delete the files. This ensures we
don't trip over our own open file handles when deleting compacted
tables.
If there is another process reading the file, it may be impossible to
delete the compacted tables. In this case, ignore the failure.
For cleaning the garbage in this case, the protocol as described in
https://www.git-scm.com/docs/reftable#_windows should be implemented.
This is left for another commit.
Ivan Frade [Tue, 1 Feb 2022 22:41:44 +0000 (17:41 -0500)]
Merge changes I11366273,I256e1572
* changes:
RepoCommand: Offer to set extra files in the destination repository
RepoCommand: Move bare/regular superproject writing to their own classes
Ivan Frade [Tue, 23 Nov 2021 22:42:01 +0000 (14:42 -0800)]
RepoCommand: Offer to set extra files in the destination repository
We want to save in the destination repository what manifest created its
structure. This helps to detect and debug failures in the manifest ->
superproject translations. The src commit should be easily readable from
the superproject tip.
Offer an API to write a file in the destination repository. RepoCommand
callers (e.g. gerrit supermanifest plugin) can use this to add a
file with the repo/ref/hash of the manifest.
Alternatives considered to write the source repo/ref/hash:
* .gitattributes of the .gitmodules file. Some updates in the manifest
don't touch the .gitmodules (e.g. a linkfile change), so it can fall
out of sync.
* commit message. Caller would need to follow the commit history to
find the latest modification by repo command. This is not helpful
e.g. for build bots that want to get the value in one call.
projects / jgit.git / log