Make sure that SmudgeFilter calls LfsPointer.parseLfsPointer() with
a stream that supports mark/reset, and make sure that parseLfsPointer()
resets the stream properly if it decides that the stream content is not
a LFS pointer.
Add a test.
Bug: 570758
Change-Id: I2593d67cff31b2dfdfaaa48e437331f0ed877915
Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>
Parsing an LFS pointer must check the input more to not run into
exceptions. LfsPoint.parseLfsPointer() is used in various places to
determine whether a blob is a LFS pointer; it is not only called with
valid LFS pointers. Tighten the validations and return null if they
fail. All callers already do check for a null return value.
Also, LfsPointer implemented Comparable but did not override equals().
This is rather unusual and actually warned against in the javadoc of
Comparable. Implement equals() and hashCode().
Add more tests.
Bug: 570744
Change-Id: I90ca264d0a250275cf1907e9dcfcee5eab80df0f
Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>
[spotbugs] parse time using thread-safe DateTimeFormatter
LfsConnectionFactory used a static SimpleDateFormat which isn't
thread-safe. Use DateTimeFormatter instead to fix this.
Change-Id: Id580251c999e1e412c269f37b29860d310124c89
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
LfsConnectionFactory#getLfsUrl: Fix unconditional break in for-loop
When iterating over the remote URLs to find one that matches "origin",
it always exits after the first iteration whether it has found the
remote or not. The break should be inside the conditional block so
that it exits when "origin" is found, otherwise continues to iterate
over the remaining remote URLs.
Found by Sonar Lint.
Change-Id: Ic969e54071d1cf095334007c1c1bab6579044dd2
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
Running recent error prone version complaining on that code:
LfsPointer.java:171: error: [BadComparable] Possible sign flip from
narrowing conversion
return (int) (getSize() - o.getSize());
^
(see https://errorprone.info/bugpattern/BadComparable)
Did you mean 'return Long.compare(getSize(), o.getSize());'?
Bug: 562756
Change-Id: I0522f1025319a9290c448a064fbafdb4b16d1d59
Signed-off-by: David Ostrovsky <david@ostrovsky.org>
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>
Reorder modifiers to follow Java Language Specification
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>
Enable UnusedException at ERROR level which causes the build to fail
in many places with:
[UnusedException] This catch block catches an symbol and re-throws
another, but swallows the caught symbol rather than setting it as a
cause. This can make debugging harder.
Fix it by setting the caught exception as cause on the subsequently
thrown exception.
Note: The grammatically incorrect error message is copy-pasted as-is
from the version of ErrorProne currently used in Bazel; it has been
fixed by [1] in the latest version.
[1] https://github.com/google/error-prone/commit/d57a39c
Change-Id: I11ed38243091fc12f64f1b2db404ba3f1d2e98b5
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
This will change the behavior in the CLI to resemble that of C-Git more
closely by printing the stderr of the hooks to the CLI stderr
independently of the exit code of the hook.
This is also useful for the corresponding EGIT-Change, which will add
the ability to show the hook output in eclipse.
With this also the stderr can be shown even if the exit code is 0.
Bug: 553471
Change-Id: Ie7bc503fe39e270e9b93dd1108b5879f02a12b4c
Signed-off-by: Tim Neumann <Tim.Neumann@advantest.com>
Enable and fix "Statement unnecessarily nested within else clause" warnings
Since [1] the gerrit project includes jgit as a submodule, and has this
warning enabled, resulting in 100s of warnings in the console.
Also enable the warning here, and fix them.
At the same time, add missing braces around adjacent and nearby one-line
blocks.
[1] https://gerrit-review.googlesource.com/c/gerrit/+/227897
Change-Id: I81df3fc7ed6eedf6874ce1a3bedfa727a1897e4c
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
Cache user global and system-wide git configurations
So far the git configuration and the system wide git configuration were
always reloaded when jgit accessed these global configuration files to
access global configuration options which are not in the context of a
single git repository. Cache these configurations in SystemReader and
only reload them if their file metadata observed using FileSnapshot
indicates a modification.
Change-Id: I092fe11a5d95f1c5799273cacfc7a415d0b7786c
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>
[error prone] suppress AmbiguousMethodReference in AnyLongObjectId
Move the implementation of the static equals() method to a new method
and suppress the error. Deprecate the old method to signal that we
intend to remove it in the next major release.
See https://errorprone.info/bugpattern/AmbiguousMethodReference
Change-Id: I712697a411ab44c6e05ae4604eb2dcb9c0f8abd3
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
[error prone] fix ReferenceEquality warning in static equals methods
Implement a helper method suppressing the ReferenceEquality error prone
warning and use it to fix this warning in static equals methods where
this comparison is used to implement fast path of static equals
implementation.
See https://errorprone.info/bugpattern/ReferenceEquality
Change-Id: I33538a3406007d24efec3a504e031ca1069572ed
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
These are useful to avoid typos, and also for tab completion.
Change-Id: I0f2d267e46b36bc40297c9657c447f3fd8b9f831
Signed-off-by: David Turner <dturner@twosigma.com>
Error Prone: Increase severity of NonOverridingEquals to ERROR
Error Prone reports the warning on several classes:
[NonOverridingEquals] equals method doesn't override Object.equals;
if this is a type-specific helper for a method that does override
Object.equals, either inline it into the callers or rename it to
avoid ambiguity.
See https://errorprone.info/bugpattern/NonOverridingEquals
Most of these are in the public API, so we can't rename or inline them
without breaking the API. FileSnapshot is not part of the public API,
but clients may be using it anyway, so we also shouldn't change that.
Suppress all the warnings instead. Having the check at severity ERROR
will at least make sure we don't introduce any new occurrences.
Change-Id: I92345c11256f06b4fa03ccc13337f72af5a43591
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
Format @NonNull on return value as method annotation
For example, instead of using
public @NonNull String getMyFavoriteString() { ... }
use
@NonNull
public String getMyFavoriteString() { ... }
This makes the style more consistent (the existing JGit code base
tends to lean toward the second style) and makes the source code
better reflect how the annotation is parsed, as a METHOD annotation.
Longer term, we should switch to a TYPE_USE annotation and switch to
the first style.
Noticed using a style checker that follows
https://google.github.io/styleguide/javaguide.html#s4.8.5-annotations
Change-Id: I9b9fa08035d805ca660520f812a84d2f47eff507
Reported-by: Ivan Frade <ifrade@google.com>
Signed-off-by: Jonathan Nieder <jrn@google.com>
Format @Nullable on return value as method annotation
For example, instead of using
public @Nullable String getMyFavoriteString() { ... }
use
@Nullable
public String getMyFavoriteString() { ... }
This makes the style more consistent (the existing JGit code base
tends to lean toward the second style) and makes the source code
better reflect how the annotation is parsed, as a METHOD annotation.
Longer term, we should switch to a TYPE_USE annotation and switch to
the first style.
Noticed using a style checker that follows
https://google.github.io/styleguide/javaguide.html#s4.8.5-annotations
Change-Id: I07f4e67cc149fb8007f696a4663e10d4bfc57e3a
Reported-by: Ivan Frade <ifrade@google.com>
Signed-off-by: Jonathan Nieder <jrn@google.com>
Specify charset when constructing InputStreamReader
ErrorProne warns [1] about implicit use of the platform default charset,
which can result in differing behaviour between JVM executions or
incorrect behavior if the encoding of the data source doesn't match
expectations.
[1] http://errorprone.info/bugpattern/DefaultCharset
Change-Id: I0fd489d352170339c3867355cd24324dfdbd4b59
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
On recent VMs, collection.toArray(new T[0]) is faster than
collection.toArray(new T[collection.size()]). Since it is also more
readable, it should now be the preferred way of collection to array
conversion.
https://shipilev.net/blog/2016/arrays-wisdom-ancients/
Change-Id: I80388532fb4b2b0663ee1fe8baa94f5df55c8442
Signed-off-by: Michael Keppler <Michael.Keppler@gmx.de>
Propagate failure of ssh command to caller of SshSupport
When SshSupport.runSshCommand fails since the executed external ssh
command failed throw a CommandFailedException.
If discovery of LFS server fails due to failure of the
git-lfs-authenticate command chain the CommandFailureException to the
LfsConfigInvalidException in order to allow root cause analysis in the
application using that.
Change-Id: I2f9ea2be11274549f6d845937164c248b3d840b2
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
* URIish seems to have a tiny feature (bug?). The path of the URI
starts with a '/' only if the URI has a port set (it seems).
* GitHub does not return SSH authorization on a single line as Gerrit
does - need to account for that.
* Increase the SSH git-lfs-authenticate timeout, as GitHub sometimes
responds slower than expected.
* Guard against NPE in case the download action does not contain any
additional headers.
Change-Id: Icd1ead3d015479fd4b8bbd42ed42129b0abfb95c
Signed-off-by: Markus Duft <markus.duft@ssi-schaefer.com>
* Larger eager timeout to compensate for high-latency lines
* Respect eager timeout in case the server uses "expiresIn"
Change-Id: Id87da1eea874e70b69eaccf35c84af4c3bb50770
Signed-off-by: Markus Duft <markus.duft@ssi-schaefer.com>
Remove it from
* package private functions.
* try blocks
* for loops
this was done with the following python script:
$ cat f.py
import sys
import re
import os
def replaceFinal(m):
return m.group(1) + "(" + m.group(2).replace('final ', '') + ")"
methodDecl = re.compile(r"^([\t ]*[a-zA-Z_ ]+)\(([^)]*)\)")
def subst(fn):
input = open(fn)
os.rename(fn, fn + "~")
dest = open(fn, 'w')
for l in input:
l = methodDecl.sub(replaceFinal, l)
dest.write(l)
dest.close()
for root, dirs, files in os.walk(".", topdown=False):
for f in files:
if not f.endswith('.java'):
continue
full = os.path.join(root, f)
print full
subst(full)
Change-Id: If533a75a417594fc893e7c669d2c1f0f6caeb7ca
Signed-off-by: Han-Wen Nienhuys <hanwen@google.com>
The NPE occurred in conjunction with a symbolic ref (origin/HEAD).
Change-Id: I291636818a121ca00e0df25de5b6fc71a48d447f
Signed-off-by: Markus Duft <markus.duft@ssi-schaefer.com>
Introduce SshSupport to centralize SSH related utility code
As discussed with Thomas here:
https://git.eclipse.org/r/#/c/83506/31/org.eclipse.jgit.lfs/src/org/eclipse/jgit/lfs/SmudgeFilter.java@349
Move the code from ConfigureGerritAfterCloneTask to SshSupport and
eliminate the slightly modified copy of the code from
LfsConnectionFactory. Separate EGit commit will eliminate the code from
ConfigureGerritAfterCloneTask.
Change-Id: Ifb5adb1342e0fc1f2a70cddf693408d4e0ef7906
Signed-off-by: Markus Duft <markus.duft@ssi-schaefer.com>
LFS: Adjust some API to make integration into tools (EGit,...) easier
Make the install command accessible without requiring reflection.
Expose the isEnabled(Repository) API to be able to check if calling the
install command is required for a repository.
Change-Id: I17e6eaefb6afda17fea8162cbf0cb86a20506753
Signed-off-by: Markus Duft <markus.duft@ssi-schaefer.com>
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
DirCacheCheckout has a warning about non-localised string "lfs". Other
classes use org.eclipse.jgit.lfs.lib.Constants but that is not visible
to DirCacheCheckout.
Add a new constant in ConfigConstants and use that in DirCacheCheckout.
Replace existing uses of org.eclipse.jgit.lfs.lib.Constants.LFS with
the new constant, except where it is referring to the folder name.
Change-Id: I0f21b951babff9a2e579d68c4de0c62ee4bc23d4
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
LFS: Enable LFS support for the CLI, better error handling
Enable LFS support for the CLI by registering the according filters.
Errors during filter creation must be propagated up the call stack, as a
failure to create a filter should be treated as fatal if the filter is
required.
Change-Id: I3833757839bdda97cd01b6c21c1613d199e2692d
Signed-off-by: Markus Duft <markus.duft@ssi-schaefer.com>
As it is right now some streams leak out of the filter construct. This
change clarifies responsibilities and fixes stream leaks
Change-Id: Ib9717d43a701a06a502434d64214d13a392de5ab
Signed-off-by: Markus Duft <markus.duft@ssi-schaefer.com>
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
LFS: support merge/rebase/cherry-pick/diff/compare with LFS files
Respect merge=lfs and diff=lfs attributes where required to replace (in
memory) the content of LFS pointers with the actual blob content from
the LFS storage (and vice versa when staging/merging).
Does not implement general support for merge/diff attributes for any
other use case apart from LFS.
Change-Id: Ibad8875de1e0bee8fe3a1dffb1add93111534cae
Signed-off-by: Markus Duft <markus.duft@ssi-schaefer.com>
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
LFS: Dramatically improve checkout speed with SSH authentication
SSH Authentication is quite expensive (~120ms on localhost against
Gerrit with LFS plugin). The SSH authentication typically also sends a
validity time of the returned token, which allows to re-use it for a
certain time, avoiding the expensive authentication on every download
request. This improves checkout times by large factors depending on the
LFS object amount/sizes.
Also make sure that all instances of Gson used by LFS are configured in
the same way.
Change-Id: I422c94c37021b4322789b3829fa0185e25d683f2
Signed-off-by: Markus Duft <markus.duft@ssi-schaefer.com>
If JGit built in LFS support is enabled for the current repository (or
user/system), any existing pre-push hook will cause an exception for the
time beeing, as only a single pre-push hook is supported.
Thus either native pre-push hooks OR JGit built-in LFS support may be
enabled currently, but not both.
Change-Id: Ie7d2b90e26e948d9cca3d05a7a19489488c75895
Signed-off-by: Markus Duft <markus.duft@ssi-schaefer.com>
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
Transfer data in chunks of 8k Transferring data byte per byte is slow,
running checkout with CleanFilter on a 2.9MB file takes 20 seconds.
Using a buffer of 8k shrinks this time to 70ms.
Also register the filter commands in a way that the native GIT LFS can
be used alongside with JGit.
Implements auto-discovery of LFS server URL when cloning from a Gerrit
LFS server.
Change-Id: I452a5aa177dcb346d92af08b27c2e35200f246fd
Also-by: Christian Halstrick <christian.halstrick@sap.com>
Signed-off-by: Markus Duft <markus.duft@ssi-schaefer.com>
InvalidLongObjectIdException: Fix formatting of message
The message is formatted as:
Invalid id: : abcde...
but should be:
Invalid id: abcde...
Change-Id: Ie15cacdcf2f168edaee262e6cf8061ebfe9d998d
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
Use constants from StandardCharsets instead of hard-coded strings
Instead of hard-coding the charset strings "US-ASCII", "UTF-8", and
"ISO-8859-1", use the corresponding constants from StandardCharsets.
UnsupportedEncodingException is not thrown when the StandardCharset
constants are used, so remove the now redundant handling.
Because the encoding names are no longer hard-coded strings, also
remove redundant $NON-NLS warning suppressions.
Also replace existing usages of the constants with static imports.
Change-Id: I0a4510d3d992db5e277f009a41434276f95bda4e
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
InvalidLongObjectIdException: Fix formatting of message
The message is formatted as:
Invalid id: : abcde...
but should be:
Invalid id: abcde...
Change-Id: Ie15cacdcf2f168edaee262e6cf8061ebfe9d998d
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
When invoking File.toPath(), an (unchecked) InvalidPathException may be
thrown which should be converted to a checked IOException.
For now, we will replace File.toPath() by FileUtils.toPath() only for
code which can already handle IOExceptions.
Change-Id: I0f0c5fd2a11739e7a02071adae9a5550985d4df6
Signed-off-by: Marc Strapetz <marc.strapetz@syntevo.com>
Thomas Wolf
Matthias Sohn
David Pursehouse
David Ostrovsky
Tim Neumann
David Turner
Carsten Hammer
Jonathan Nieder
Michael Keppler
Markus Duft
Han-Wen Nienhuys
Marc Strapetz