This allows clients to use the --shallow-exclude parameter (producing a
"deepen-not <ref>" line when communicating with the server) in their fetch
commands when fetching against a JGit server using protocol v2.
Note that the implementation in this commit is somewhat inefficient, as
described in the TODO comment in DepthGenerator.
Change-Id: I9fad3ed9276b624d8f668356ffd99a067dc67ef7
Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
In C Git, when a client fetches with "git fetch --shallow-since=<date>
origin <ref>", and all commits reachable from <ref> are older than
<date>, the server dies with a message "no commits selected for shallow
requests". That is, (1) the --shallow-since filter applies to the commit
pointed to by the ref itself, and (2) there is a check that at least one
commit is not filtered out. (The pack-protocol.txt documentation does
not describe this, but the C implementation does this.)
The implementation in commit 1bb430dc21 ("UploadPack: support
deepen-since in protocol v2", 2018-09-27) does neither (1) nor (2), so
do both of these.
Change-Id: I9946327a71627626ecce34ca2d017d2add8867fc
Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
The request receives a list of capabilities and takes out the "agent" to
offer it on its own setter (getAgent).
Do this at parse time: when reading the line if the capability is
"agent" set it directly in the builder.
This makes the treatment of "agent" consistent in v0/v1 and v2.
Change-Id: Ie4f9f2cad8639adeeaef4921df49a30a8ce5b42f
Signed-off-by: Ivan Frade <ifrade@google.com>
All of the input lines passed to pre-push hook scripts must be properly
terminated by '\n', so that normal shell scripts like the git-supplied
pre-push.sample work properly, even when pushing just a single branch.
With the old code, hook scripts that use the following pattern didn't
process the last line, because 'read' has a non-zero exit status when
EOF is encountered:
while read local_ref local_sha remote_ref remote_sha; do ... done
Change-Id: Id899662ed3fedef6c314fc4b2ddf91a6dcb98cbb
Signed-off-by: Markus Keller <markus.kell.r@gmail.com>
Before this commit, a force checkout would fail if there
were any conflicting files. After this commit, a force
checkout will overwrite the conflicting files, as expected.
Making this work required fixing a bug in DirCacheCheckout.
Before this commit, when DirCacheCheckout had
failOnConflict=false, it would delete all conflicting files
from the working copy and just leave them missing. After
this commit, DirCacheCheckout overwrites conflicting files
with the merge tree.
This change in DirCacheCheckout causes "reset --hard" and
"revert --abort" to behave as expected (previously they
would simply delete conflicting files, now they will be
overwritten from the merge tree).
Change-Id: If7e328ee792ef6511ab7d9c26d8d77c39210ec9f
Signed-off-by: Ned Twigg <ned.twigg@diffplug.com>
UploadPack.getPeerUserAgent() doesn't produce the expected results for
protocol v2 requests. In v2, the agent reported in the request (in an
"agent=" line) is not in the clientCapabilities but in a field on its
own. This makes getPeerUserAgent default to the transport user agent.
Making "agent" a shared property between protocol v0/v1 and v2 fixes the
problem, simplifies the function and harmonizes the implementation
between protocol versions.
In a follow up commit the "agent" will be identified on parsing time,
instead of taking it from the client capabilities.
Change-Id: Idf9825ec4e0b81a1458c8e3701f3e28aafd8a32a
Signed-off-by: Ivan Frade <ifrade@google.com>
In protocol v2, a command request can be followed by server options
(lines like "agent=<>" and "server-option=<>"), but current code
doesn't accept those lines.
Advertise the "server-option" capability, parse the lines and add
them to the request objects.
Other code in JGit can see this options and act accordingly via the
protocol v2 hooks.
This should not require any change in the client side.
Change-Id: If3946390f9cc02d29644b6ca52534b6f757bda9f
Signed-off-by: Ivan Frade <ifrade@google.com>
Error Prone reports that the unsynchronized method skip overrides the
synchronized method in ByteArrayInputStream [1].
This is a test class, so we can just suppress the warning as recommended
in [1].
Note that the suppression causes a warning in Eclipse, because it doesn't
recognize the "UnsynchronizedOverridesSynchronized" as a valid value for
the @SuppressWarnings annotation [2].
[1] https://errorprone.info/bugpattern/UnsynchronizedOverridesSynchronized
[2] https://bugs.eclipse.org/bugs/show_bug.cgi?id=392045
Change-Id: I3e798b448211f1363729091f72fb0ef6a873e599
Signed-off-by: David Pursehouse <david.pursehouse@gmail.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>
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>
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>
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>
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>
This probably doesn't provide any benefit for the tests, but gets
rid of a warning from Error Prone.
See https://errorprone.info/bugpattern/InputStreamSlowMultibyteRead
Change-Id: I584d2e0d18475fad38747b688af6301c423f54a7
Signed-off-by: David Pursehouse <david.pursehouse@gmail.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>
Error Prone reports:
Not calling fail() when expecting an exception masks bugs
See https://errorprone.info/bugpattern/MissingFail
Change-Id: I58ad45a87dcf7d646dce056d20776d62faafbfef
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
Error Prone reports:
Not calling fail() when expecting an exception masks bugs
See https://errorprone.info/bugpattern/MissingFail
Change-Id: I518b524de7cd3802f03b80450cad02ab3f79d57b
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
Error Prone reports:
Not calling fail() when expecting an exception masks bugs
See https://errorprone.info/bugpattern/MissingFail
Change-Id: Ic89f9daef3c31bfbfd1f3c003cb90a373cd74847
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
Error Prone reports:
Not calling fail() when expecting an exception masks bugs
See https://errorprone.info/bugpattern/MissingFail
Change-Id: I463510342bb6e6b99b31a0fe264d953340784393
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
The tests were set up to expect an IllegalArgumentException when
the Config.getInt method was called with a section.key that has
not been set, or explicitly set to an empty string.
However, the IllegalArgumentException never gets thrown because
the getInt method returns the provided default ("1"), and because
there was no call to "fail" after getInt, the incorrect behavior
of the test was not noticed.
Remove the try/catch around getInt, and instead assert that the
expected default value is returned.
Found by Error Prone, which reported:
Not calling fail() when expecting an exception masks bugs
See https://errorprone.info/bugpattern/MissingFail
Change-Id: Ie8e692aba9fb8523241fb8f298d57493923d9f78
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>
The BufferedWriter is opened in a try-with-resource and thus will be
automatically closed.
Presumably the close was added to make sure it is closed before the
subsequent test statements are executed. Instead of explicitly closing
it, let the try-with-resource automatically close it, and move the
subsequent statements out of the try-block.
Change-Id: If5fada2f580ef9cbaad3a0b9216b5200b917781a
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
Replace explicit construction of FileWriter with calls to the
utility method Files.newBufferedWriter, which allows to specify
the charset.
Also convert to try-with-resource.
Change-Id: I9fa3f612b9b2fc5ac12cd79d6e61ca181120dbf5
Signed-off-by: David Pursehouse <david.pursehouse@gmail.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>
There should only be one statement after the expect(...) method.
Any additional statements after the statement that is expected to
throw will never be executed in a passing test. This can lead to
inappropriately passing tests where later incorrect assertions are
skipped by the thrown exception.
See https://errorprone.info/bugpattern/ExpectedExceptionChecker
Change-Id: I20fecf8fb7a243e9da097e6d03fbf8cd69151bf0
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
There should only be one statement after the expect(...) method.
Any additional statements after the statement that is expected to
throw will never be executed in a passing test. This can lead to
inappropriately passing tests where later incorrect assertions are
skipped by the thrown exception.
See https://errorprone.info/bugpattern/ExpectedExceptionChecker
Change-Id: I0d6350fafb281b6bdb04289f4cd5eb4bb159628b
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>
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>
Also check that reflog is enabled by default.
Also check that a file in a folder is reset correctly.
Change-Id: I4bc6649928ce10d7b558901e465e8032b083ade0
Signed-off-by: René Scheibe <rene.scheibe@gmail.com>
The statement:
assertThat(recvStream.available(), is(0));
results in a warning from Eclipse:
The expression of type int is boxed into Integer
because recvStream.available() returns int, but the hamcrest is()
method takes an Integer.
Replace it with the equivalent JUnit assertion.
Also remove the suppression of another similar warning and fix that
in the same way.
Change-Id: I6f18b304a540bcd0a10aec7d3abc7dc6f047fe80
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
In order to support GPG-signed commits, add some methods which will
allow GPG signatures to be parsed out of RevCommit objects.
Later, we can add code to verify the signatures.
Change-Id: Ifcf6b3ac79115c15d3ec4b4eaed07315534d09ac
Signed-off-by: David Turner <dturner@twosigma.com>
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
Use existing test utility methods instead of nested PrintWriter usage.
Change-Id: I324852c7971ae644fa499f377a31d1cf265c7fd9
Signed-off-by: René Scheibe <rene.scheibe@gmail.com>
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>
Jonathan Tan
Ivan Frade
Markus Keller
Ned Twigg
David Pursehouse
Thomas Wolf
Minh Thai
Marc Strapetz
Matthias Sohn
René Scheibe
David Turner