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>
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>
As described in the javadoc for org.eclipse.jgit.annotations.Nullable:
Warning: Please do not use this annotation on arrays. Different
annotation processors treat `@Nullable Object[]` differently: some
treat it as an array of nullable objects, for consistency with
versions of `Nullable` defined with `@Target TYPE_USE`, while others
treat it as a nullable array of objects. JGit therefore avoids using
this annotation on arrays altogether.
See the checker-framework manual[1] for details.
[1] http://types.cs.washington.edu/checker-framework/current/checker-framework-manual.html#faq-array-syntax-meaning
Change-Id: I14ffcf80adbb8145d797998de2f2fa6ab84c3ae3
Signed-off-by: Jonathan Nieder <jrn@google.com>
Previous commits block the addition to the repo of dangerous .gitmodules
files, but some could have been committed before those safeguards where
in place.
Add a check in DfsFsck to validate the .gitmodules files in the repo.
Use the same validation than the ReceivePack, translating the
results to FsckErrors.
Note that *all* .gitmodules files in the storage will be checked, not
only the latest version.
Change-Id: I040cf1f31a779419aad0292ba5e6e76eb7f32b66
Signed-off-by: Ivan Frade <ifrade@google.com>
errorType is already null in the caller and callee when unknown, so we
can replace a conditional call to a setter in the only caller with an
unconditionally provided @Nullable constructor parameter.
As a bonus, this lets us mark the field as final.
Change-Id: Ie2f929180e74ffa1aba8ec6caccfa81fbd8bfc04
Signed-off-by: Ivan Frade <ifrade@google.com>
The fsck test needs more detail about the error than an IOException
with an explanatory message.
Add an error identifier to the SubmoduleValidatorException and make
it the only throwable exception when parsing a file.
Change-Id: Ic3f0955b497e1681b25e681e1282e876cdf3d2c5
Signed-off-by: Ivan Frade <ifrade@google.com>
A .gitmodules file can include a submodule without a path to configure
the URL for a submodule that is only present on other branches.
A .gitmodules file can include a submodule with no URL and no path to
reserve the name for a submodule that existed in earlier history but
is not available from any URL any more.
"git fsck" permits both of these cases. Permit them in JGit as well
(instead of throwing NullPointerException).
Change-Id: I3b442639ad79ea7a59227f96406a12e62d3573ae
Reported-by: David Pursehouse <david.pursehouse@gmail.com>
Signed-off-by: Jonathan Nieder <jrn@google.com>
This was not updated with the original introduction of the new method.
Bug: 534731
Change-Id: Ic4589c3a209109a829fbb706a9bf38845134e904
Signed-off-by: Michael Keppler <Michael.Keppler@gmx.de>
Since v4.7.5.201810051826-r~3 (SubmoduleAddCommand: Reject submodule
URIs that look like cli options, 2018-09-24), SubmoduleAddCommand
checks submodule names for ".." path components in
assertValidSubmoduleName. This additional check for the same is
redundant.
Change-Id: I993326a370978880b690dc133a81fa3025935bcb
Signed-off-by: Jonathan Nieder <jrn@gmail.com>
The text "<tree, blob>" with angle brackets should not be used in javadoc
since it is interpreted as an HTML tag and then rejected since it's not a
valid HTML tag. Wrap the text in a @literal tag.
Also add a missing space.
Change-Id: Ide045e8c04a39a916f5b2e964e58c151e4555830
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
The main concern are submodule urls starting with '-' that could pass as
options to an unguarded tool.
Pass through the parser the ids of blobs identified as .gitmodules
files in the ObjectChecker. Load the blobs and parse/validate them
in SubmoduleValidator.
Change-Id: Ia0cc32ce020d288f995bf7bc68041fda36be1963
Signed-off-by: Ivan Frade <ifrade@google.com>
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
In order to validate .gitmodules files, we first need to find them
in the incoming pack.
Do it in the ObjectChecker stage. Check in the tree objects if they
point to a .gitmodules file and report the tree id and the .gitmodules
blob id.
This can be used later to check if the file is in the root of the
project and if the contents are good.
While we're here, make isMacHFSGit more accurate by detecting variants
of filenames that vary in case.
[jn: tweaked NTFS and HFS+ checking; added more tests]
Change-Id: I70802e7d2c1374116149de4f89836b9498f39582
Signed-off-by: Ivan Frade <ifrade@google.com>
Signed-off-by: Jonathan Nieder <jrn@google.com>
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
In C git versions before 2.19.1, the submodule is fetched by running
"git clone <uri> <path>". A URI starting with "-" would be interpreted
as an option, causing security problems. See CVE-2018-17456.
Refuse to add submodules with URIs, names or paths starting with "-",
that could be confused with command line arguments.
[jn: backported to JGit 4.7.y, bringing portions of Masaya Suzuki's
dotdot check code in v5.1.0.201808281540-m3~57 (Add API to specify
the submodule name, 2018-07-12) along for the ride]
Change-Id: I2607c3acc480b75ab2b13386fe2cac435839f017
Signed-off-by: Ivan Frade <ifrade@google.com>
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
EolStreamTypeUtil didn't handle these correctly on Windows.
Add three new tests to verify that the crlf attribute is handled as
described at [1], and that core.eol=native produces the expected
line endings on check-out.
[1] https://git-scm.com/docs/gitattributes
Bug: 497290
Change-Id: Idd9b435e3256c1e3251cc7b966f2f0460e787f07
Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
Breaking implementers is ok in a minor version update following OSGi
semantic versioning. According to [1] adding a default method is ok if
risk of inheriting a method with the same name from multiple interfaces
is low.
[1] https://wiki.eclipse.org/Evolving_Java-based_APIs_2#Evolving_API_Interfaces
Change-Id: Iad354cb73cfff9a96b9a9852a3c4d9ba0c5df430
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
According to Error Prone, the class should also override:
int read(byte[], int, int)
otherwise multi-byte reads from this input stream are likely to be slow.
See https://errorprone.info/bugpattern/InputStreamSlowMultibyteRead
Change-Id: I33ab8cd30013447f2a0363b3a7b1424b79cb1818
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
Since this constructor is package-private, so instead of deprecating
it, we can remove it right away.
Change-Id: I84ba72e8c94fb09412bbf7ce73b7eec72f61e964
Signed-off-by: Jonathan Nieder <jrn@google.com>
Deprecate constructors and methods taking a character set name as
a String, in favor of new variants taking a Charset.
Change-Id: I616c601daf232fa17610dba1087fd902030d46ea
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
Support the deepen-since parameter when requested by a client using
protocol v2. This is done by:
- adding a DepthWalk.RevWalk#setDeepenSince method
- updating DepthGenerator to recognize when deepen-since is set
- recording in DepthWalk.Commit whether a commit is a boundary commit
Existing users of DepthWalk such as UploadPack previously recognized
boundary commits by comparing their depths against the threshold, not
tracking whether any parents were truly excluded. This behavior is
preserved - UploadPack considers a commit as boundary if its depth is
equal to the threshold *or* a parent was excluded (whether by depth or
by deepen-since).
Change-Id: I852bba6b1279f9cc8aee38282e9339d62b8dcddc
Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
In DepthGenerator, commits are always added to the "pending" queue
either at depth 0 (in the constructor) or after a depth check (in
next()), so it is redundant to check for depth after removing them from
the queue. Remove the check.
This redundancy seems to have been present since the introduction of
server-side shallow clone support in commit 9952223e06 ("Implement
server support for shallow clones", 2011-08-21).
Change-Id: Iad334935293367400c2901a25c0f4bf36c437cf2
Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
Replace explicit construction of BufferedReader with calls to the
utility method Files.newBufferedReader, which allows to specify
the charset.
Change-Id: I61b9451dbc8d9cf83fc8a5981292b8fdc713ce37
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
As reported by Error Prone:
An inner class should be static unless it references members of its
enclosing class. An inner class that is made non-static unnecessarily
uses more memory and does not make the intent of the class clear.
See https://errorprone.info/bugpattern/ClassCanBeStatic
Change-Id: Ib99d120532630dba63cf400cc1c61c318286fc41
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
Support multiple prefixes when querying references to allow
implementor to minimize number of RPC calls.
Change-Id: I5f822fd7eaf9756b44750080d3056de138b64f4a
Signed-off-by: Minh Thai <mthai@google.com>
The commit message template contains a superfluous blank at the end of
the first line, which is deleted by this change. This is only relevant
for jgit contributors using Eclipse, not for jgit users.
Change-Id: I462deb49c26fb64b3dc2d1d75f1e40ef302b0fc9
Signed-off-by: Michael Keppler <Michael.Keppler@gmx.de>
This is a base change which prepares for subsequent bugfixes.
Change-Id: Iaadc93df37e45753d700be73669e68c03590adb5
Signed-off-by: Marc Strapetz <marc.strapetz@syntevo.com>
* stable-5.1:
Fix ObjectUploadListener#close
Fix error handling in FileLfsServlet
ObjectDownloadListener#onWritePossible: Make code spec compatible
ObjectDownloadListener: Return from onWritePossible when data is written
Fix IOException when LockToken#close fails
Change-Id: If6e4b4786401c6598b9f3b1c2b7d7081b08acd35
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
* stable-5.0:
Fix ObjectUploadListener#close
Fix error handling in FileLfsServlet
ObjectDownloadListener#onWritePossible: Make code spec compatible
ObjectDownloadListener: Return from onWritePossible when data is written
Fix IOException when LockToken#close fails
Change-Id: Ib7a63b5144a89c213aff3b32c30a2a6526355e64
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
* stable-4.11:
Fix ObjectUploadListener#close
Fix error handling in FileLfsServlet
ObjectDownloadListener#onWritePossible: Make code spec compatible
ObjectDownloadListener: Return from onWritePossible when data is written
Fix IOException when LockToken#close fails
Change-Id: Iffe202a412b3bca1c8d8d7dc5dfd646c49838de9
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
* stable-4.10:
Fix ObjectUploadListener#close
Fix error handling in FileLfsServlet
ObjectDownloadListener#onWritePossible: Make code spec compatible
ObjectDownloadListener: Return from onWritePossible when data is written
Fix IOException when LockToken#close fails
Change-Id: I28ebf6138ccd9425fc05319de78c7716f0bdd199
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
* stable-4.9:
Fix ObjectUploadListener#close
Fix error handling in FileLfsServlet
ObjectDownloadListener#onWritePossible: Make code spec compatible
ObjectDownloadListener: Return from onWritePossible when data is written
Fix IOException when LockToken#close fails
Change-Id: Ib7d01cb0ece8b259156855045a53b8baf3fa2968
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
* stable-4.8:
Fix ObjectUploadListener#close
Fix error handling in FileLfsServlet
ObjectDownloadListener#onWritePossible: Make code spec compatible
ObjectDownloadListener: Return from onWritePossible when data is written
Fix IOException when LockToken#close fails
Change-Id: Id8eb635094336567d9f3c28ec985cd5127d31632
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
* stable-4.7:
Fix ObjectUploadListener#close
Fix error handling in FileLfsServlet
ObjectDownloadListener#onWritePossible: Make code spec compatible
ObjectDownloadListener: Return from onWritePossible when data is written
Fix IOException when LockToken#close fails
Change-Id: Iad9836811be034cf992ea25dad4409addba75115
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
This happened if the LockTokens hard link was already deleted earlier.
Bug: 531759
Change-Id: Idc84bd695fac1a763b3cbb797c9c4c636a16e329
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
Otherwise fetching all tags may pull in commits not on the
specified branches. Canonical git also does this.[1]
[1] https://github.com/git/git/blob/b160b6e69/builtin/clone.c#L1124
Bug: 538768
Change-Id: If0ac75fb9fae0c95d1a48b22954c54d4c3c09a47
Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>
When not all branches are cloned, the fetch refspec for the
remote should not be "+refs/heads/*:refs/remotes/origin/*":
that would fetch all branches on the very next fetch, thus
making a clone with only a subset of the branches rather
pointless.
Instead, produce refspecs for the cloned branches only.
Canonical git also does this for its --single-branch case;
it doesn't have an option to clone only a subset of the branches
(only one or all).
Bug: 466858
Change-Id: Ie871880f757663437efac1e8b3313094f9e629b3
Also-by: Julian Enoch <julian.enoch@ericsson.com>
Signed-off-by: Julian Enoch <julian.enoch@ericsson.com>
Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
Fetch code has been moved to a ProtocolV2Parser, but ls-refs code is
still in UploadPack.
Moving it to the parser makes it easier to test, keeps the parsing
together and makes the two commands follow similar structure.
Change-Id: I573ce543e804ddeb9f83303b4af250b7cddc8cad
Signed-off-by: Ivan Frade <ifrade@google.com>
UploadPack.processShallow is doing too many things and offering a
confusing API. It is filtering or splitting commit ids depending
if a parameter is null and writing them out (or not) depending on another
flag.
Iterate the list and announce to Consumers what object ids need
to be marked as (un)shallow. They decide what to do with them.
As java consumers don't allow to propagate exceptions, define
our own functional interface for it.
Change-Id: I619cf2eed9b1e0338151120b8ef87a463fbe8827
Signed-off-by: Ivan Frade <ifrade@google.com>
shallowCommits variable is nullable only to signal later
if code has gone through the processShallow code.
Use a flag to indicate that condition and make shallowCommits
non-nullable. This makes code clearer and paves the way to untangle
processShallow in a follow-up commit.
Change-Id: I898e6aaf6f860bb6afafbac05653ba116c9b2da6
Signed-off-by: Ivan Frade <ifrade@google.com>
* stable-5.1:
Fix logging null if called process fails
Change-Id: Iba771dd8d680b154cb628c7571a2bbc2ecf50a3b
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
If some process executed by FS#readPipe ends in an error,
the error stream is never set as errorMessage because
FS#GobblerThread#waitForProcessCompletion always returned true.
This caused LOG#warn to be called with null.
Return false whenever FS#GobblerThread#waitForProcessCompletion fails.
Bug: 538723
Change-Id: Ic9492bd688431d52c8665f7a2efec2989e95a4ce
Signed-off-by: Cliffred van Velzen <cliffred@cliffred.nl>
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
* stable-4.11:
Fix NoSuchFileException during directory cleanup in RefDirectory
Externalize warning message in RefDirectory.delete()
Suppress warning for trying to delete non-empty directory
Change-Id: Ic7eaa0b6a4e750bc7853eb549a9e126269ae22cf
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
* stable-4.10:
Fix NoSuchFileException during directory cleanup in RefDirectory
Externalize warning message in RefDirectory.delete()
Suppress warning for trying to delete non-empty directory
Change-Id: I191d56e3c0f11cf53076b06c7e7a05492c7a03f1
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
* stable-4.9:
Fix NoSuchFileException during directory cleanup in RefDirectory
Externalize warning message in RefDirectory.delete()
Suppress warning for trying to delete non-empty directory
Change-Id: Ied2d7a70c5544ff012687a1aeb6e07394d9bd6fd
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
* stable-4.8:
Fix NoSuchFileException during directory cleanup in RefDirectory
Externalize warning message in RefDirectory.delete()
Suppress warning for trying to delete non-empty directory
Change-Id: I5e6cc35f3673545e7ff857e6ed0bcd2c44e50316
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
* stable-4.7:
Fix NoSuchFileException during directory cleanup in RefDirectory
Externalize warning message in RefDirectory.delete()
Suppress warning for trying to delete non-empty directory
Change-Id: I9ec6352b5ff57aa1a3380079dc9165890cc76d49
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
David Pursehouse