One more step in removing state from UploadPack, using the request
object instead.
Unfortunately, hooks get from UploadPack information about the current
request. Changing the hooks to receive the request is a public API
change, so at the moment lets keep a reference to the current request.
This kills half the benefit of using a request object vs fields, but
at least we still get better modularity.
Change-Id: I86803d876a621b727c66ee73f2880c93190629e9
Signed-off-by: Ivan Frade <ifrade@google.com>
All data required in this function is available in the request object.
Use that object instead of class members. This reduces class state and
is more readable.
Make the function use a request object and remove the now unnecessary
field "deepenNotRefs".
Change-Id: If861e44c2860a78cf19f456d1b3feb7ddc314cce
Signed-off-by: Ivan Frade <ifrade@google.com>
These properties are protocol v2 specific, but they have clear default
no-op values and having them in the common superclass simplifies client
code.
Move properties deepenSince and deepenNotRefs up to FetchRequest. In
FetchV0Request, they are initialized with their no-op values (0 for
deepenSince and empty list for deepenNotRefs)
Change-Id: I9d46a6dfbe29ebd794b5a6482033cdc70d411a23
Signed-off-by: Ivan Frade <ifrade@google.com>
filterBlobLimit is not part of the UploadPack state, and as field
of the class is difficult to see where it is set or accessed.
Use the request object instead of a field. This reduces
UploadPack state and makes clearer how the value is used.
Change-Id: I96a04a5a8b31bf2243de701e1fd7ebb4080b49e2
Signed-off-by: Ivan Frade <ifrade@google.com>
Mark reference fields as final, annotate constructor parameters and
getters as @NonNull when appropiate and assert the incoming references
are non-null.
Change-Id: I0ef9a513a99313bf461fe9629ce6cc8b409bdedb
Signed-off-by: Ivan Frade <ifrade@google.com>
Some code apply to both, v1 and v2 requests, so it should receive
just a request instance.
Move all common fields to an abstract superclass that can be passed
to "version neutral" functions.
Change-Id: I47c22fb12065bc93767f78175e2b36cc43ccb5c5
Signed-off-by: Ivan Frade <ifrade@google.com>
In FetchV0Request, the fields "wantsIds" and "options" are called
"wantIds" and "clientCapabilities". Those names describe them better.
Rename FetchV2Request fields to follow fetch v0. This will make easier
to extract a superclass later.
Take also the chance to polish the javadoc.
Change-Id: Ia17dbbab8084f39cc529fef9ca5c65e189073767
Signed-off-by: Ivan Frade <ifrade@google.com>
Protocol v0/v1 parsing code doesn't have any real dependency on UploadPack.
Move it to its class and use a request object to read the data in
UploadPack.
This makes the code easier to test, keeps similar structure than protocol v2,
reduces the line count of UploadPack and paves the way to remove the
members as implicit parameters in it.
Change-Id: I8188da8bd77e90230a7e37c02d800ea18463694f
Signed-off-by: Ivan Frade <ifrade@google.com>
Uncaught exceptions are handled by java.lang.Thread's handler, which
prints it to stderr.
This is useful because InternalPushConnection is used in tests, and
during development, the server side may have programming errors that
manifest as RuntimeExceptions.
Before this change, all types of failures would lead to a uniform
failure message "test://test/conn0: push not permitted" on the client.
Signed-off-by: Han-Wen Nienhuys <hanwen@google.com>
Change-Id: I0983cbeb86d36fa7a9313373f5fce54971f804ec
5c134f4d removed closing the input stream when we reached end of the
stream. This caused file handle leaks.
Bug: 540049
Change-Id: I48082b537077c7471fc160f59aa04deb99687d9b
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
* stable-4.6:
Replace Findbugs with Spotbugs in org.eclipse.jgit/pom.xml
Replace FindBugs with SpotBugs
Change-Id: I24417e4ebbba31f7ff6896d585ef807327411392
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
* stable-4.5:
Replace Findbugs with Spotbugs in org.eclipse.jgit/pom.xml
Replace FindBugs with SpotBugs
Change-Id: I1c077e8f3530ac717b1603d3307fd15d4335b8fe
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
First-want line parsing accepts lines with an optional whitespace, when
the spec is strict requiring a white space.
Validate the line enforcing that there is a white space between oid and
capabilities list.
Change-Id: I45ada67030e0720f9b402c298be18c7518c799b1
Signed-off-by: Ivan Frade <ifrade@google.com>
In protocol v0/v1 pack negotiation, the first want line contains the
options the client wants in effect. This parsing is done in UploadPack
but it doesn't have any interaction with that class.
Move the code to its own class and package, mark the current one
as deprecated (it is public API) and add unit tests.
Take the chance to move the parsing code from the constructor to a
factory method, making the class a simple container of results.
Change-Id: I1757f535dda78a4111a1c12c3a3b455a4b6f0c51
Signed-off-by: Ivan Frade <ifrade@google.com>
The usage of non-short-circuit logic is intentional, per the inline
comment added in change Ib4b35e357 as a follow-up to Ie3761ffb4 which
was a previously rejected attempt to "fix" a similar warning that had
been raised by FindBugs.
Change-Id: I3f6729f954d45d30ce697356d2ab3cc877d3ad54
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
This reverts commit 37c7fbd661.
These filters weren't unused. Without them Eclipse raises 4 API errors.
Change-Id: I5ce443d40b5f517be4a315479e81246d40af1983
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
Check the bazel version using the checker from bazel_skylib, and
require at least version 0.17.1 which is the minimum version that
does not suffer from the Java API mismatch issue [1].
The implementation is borrowed from the Gerrit project.
[1] https://github.com/bazelbuild/bazel/issues/6027
Change-Id: I224250088a1f5072fcaa3ec81228f4d2cb8cb389
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
* changes:
Upgrade wagon-ssh to 3.2.0
Upgrade maven-jxr-plugin to 3.0.0
Upgrade maven-shade-plugin to 3.2.0
Upgrade jacoco-maven-plugin to 0.8.2
Upgrade maven-project-info-reports-plugin to 3.0.0
Upgrade maven-enforcer-plugin to 3.0.0-M2
Upgrade maven surefire plugins to 2.22.0
Ensure same version of maven-surefire-plugin and maven-surefire-report-plugin
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>
SpotBugs [1] is the spiritual successor of FindBugs, carrying on from
the point where it left off with support of its community.
This is a backport of [1] which originally did the replacement on the
master branch. This change updates to the current latest version, so
that we can get the benefit of its checks when pushing changes to the
stable branches.
[1] https://spotbugs.github.io/
[2] https://git.eclipse.org/r/#/c/101312/
Change-Id: Ib73d56b5980b55f4d7e09d87abec3138cac3d3dc
Signed-off-by: David Pursehouse <david.pursehouse@gmail.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>
Ivan Frade
David Pursehouse
Han-Wen Nienhuys
Matthias Sohn
Jonathan Nieder
Michael Keppler