FS_POSIX.createNewFile(File) failed to properly implement atomic file
creation on NFS using the algorithm [1]:
- name of the hard link must be unique to prevent that two processes
using different NFS clients try to create the same link. This would
render nlink useless to detect if there was a race.
- the hard link must be retained for the lifetime of the file since we
don't know when the state of the involved NFS clients will be
synchronized. This depends on NFS configuration options.
To fix these issues we need to change the signature of createNewFile
which would break API. Hence deprecate the old method
FS.createNewFile(File) and add a new method createNewFileAtomic(File).
The new method returns a LockToken which needs to be retained by the
caller (LockFile) until all involved NFS clients synchronized their
state. Since we don't know when the NFS caches are synchronized we need
to retain the token until the corresponding file is no longer needed.
The LockToken must be closed after the LockFile using it has been
committed or unlocked. On Posix, if core.supportsAtomicCreateNewFile =
false this will delete the hard link which guarded the atomic creation
of the file. When acquiring the lock fails ensure that the hard link is
removed.
[1] https://www.time-travellers.org/shane/papers/NFS_considered_harmful.html
also see file creation flag O_EXCL in
http://man7.org/linux/man-pages/man2/open.2.html
Change-Id: I84fcb16143a5f877e9b08c6ee0ff8fa4ea68a90d
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
When core.supportsAtomicCreateNewFile was set to false and the
repository was located on a filesystem which doesn't support the file
attribute "unix:nlink" then FS_POSIX#createNewFile may report an error
even if everything was ok. Modify FS_POSIX#createNewFile to silently
ignore this situation. An example of such a filesystem is sshfs where
reading "unix:nlink" always returns 1 (instead of throwing a exception).
Bug: 537969
Change-Id: I6deda7672fa7945efa8706ea1cd652272604ff19
Also-by: Thomas Wolf <thomas.wolf@paranor.ch>
* master:
UploadPackTest: Avoid unnecessarily boxing int into Integer
Parse signature of GPG-signed commits
Deprecate Constants.CHARSET in favor of StandardCharsets.UTF_8
Simplify file creation in ResetCommandTest
Specify charset when constructing InputStreamReader
Update Orbit to S20180829155938 for 2018-09 RC1
Replace useminimalnegotation flag with maxhaves flag
Extract protocol v2 fetch request parsing to its own class
FetchV2Request: Rename shallowSince to deepenSince
FetchV2Request: Correct typo in parameter name
FetchV2Request round up (doneReceived, renamings)
Use right variable in error message about depth
UploadPack: avoid conflating shallow commit lists in protocol v2
Bazel: Use hyphen instead of underscore in external repository names
Format BUILD files with buildifier 0.15.0
Upgrade maven-compiler-plugin to 3.8.0
Change-Id: Ibc6feb828e4ff20a3158e6b75dcfce7617e00b0e
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
I88304d34c and Ia555bce00 modified the way errors are handled when
trying to delete non-empty reference folders. Before, this error was
silently ignored as it was considered an expected output. Now, every
failed folder delete is logged which can be noisy.
Ignore the DirectoryNotEmptyException but log any other error avoiding
deletion of an eligible folder.
Signed-off-by: Hector Oswaldo Caballero <hector.caballero@ericsson.com>
Change-Id: I194512f67885231d62c03976ae683e5cc450ec7c
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>
Client will stop sending haves when the number of haves sent reaches maxhaves.
Change-Id: I1e5b1525be4c67f20a81ca24a2770c20eb5c1271
Signed-off-by: Minh Thai <mthai@google.com>
The parsing code for protocol v2 fetch doesn't have any dependency on
the rest of UploadPack.
Move it to its own class. This makes testing easier (no need to
instantiate the full UploadPack), simplifies the code in UploadPack and
increases modularity.
At the moment, the parser needs to know about the reference database to
validate incoming references. This dependency could be easily removed
moving the validation later in the flow, after the parsing, where other
validations are already happening. Postponing that to keep this patch
about moving unmodified code around.
Change-Id: I7ad29a6b99caa7c12c06f5a7f30ab6a5f6e44dc7
Signed-off-by: Ivan Frade <ifrade@google.com>
This fetch parameter is called deepen-since in the protocol. Call it
the same thing in the request object to make the code easier to reason
about.
This doesn't touch UploadPack#shallowSince, which is likely to be
eliminated altogether in a later patch anyway.
Change-Id: I8ef34bc7ad12fae3a9057ae951367cc024e1a1cb
Suggested-by: Ivan Frade <ifrade@google.com>
Signed-off-by: Jonathan Nieder <jrn@google.com>
There is an extra 'd' in deependNotRefs. Noticed during code review.
Change-Id: I93d8d7951fe5c351b62e23bdf5bad0ebd631017d
Signed-off-by: Jonathan Nieder <jrn@google.com>
Make "doneReceived" a member of the fetch request. It indicates if the
"done" line has been received (so it makes sense there) and makes all
the code after the parsing depend only on the request.
Rename "shallowExcludeRefs" to "deepenNot". Those refs come in
"deepen-not" lines in the protocol, and this name makes clearer the
intention.
Change-Id: I7bec65de04930277266491d278de7c3af7d8cbe6
Signed-off-by: Ivan Frade <ifrade@google.com>
* stable-5.1:
Bazel: Use hyphen instead of underscore in external repository names
Bazel: Format all build files with buildifier 0.15.0
ChangeIdUtilTest: Remove unused notestCommitDashV
Change-Id: Ie9d77a7716591246b87fb59ac85214417fe8309d
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
* stable-5.0:
Bazel: Use hyphen instead of underscore in external repository names
Bazel: Format all build files with buildifier 0.15.0
ChangeIdUtilTest: Remove unused notestCommitDashV
Change-Id: I7c2fc7d694d7d891cac96cb805c89c8bdf7c5cbf
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
* stable-4.11:
Bazel: Use hyphen instead of underscore in external repository names
Bazel: Format all build files with buildifier 0.15.0
ChangeIdUtilTest: Remove unused notestCommitDashV
Change-Id: I5ebb1058927ea5a478eae9e44b7d96f123a22523
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
* stable-4.10:
Bazel: Use hyphen instead of underscore in external repository names
Bazel: Format all build files with buildifier 0.15.0
ChangeIdUtilTest: Remove unused notestCommitDashV
Change-Id: I500dd4e63302abb19ad669988b3a427cef8ef3d4
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
* stable-4.9:
Bazel: Use hyphen instead of underscore in external repository names
Bazel: Format all build files with buildifier 0.15.0
ChangeIdUtilTest: Remove unused notestCommitDashV
Change-Id: I37e936de0c06d4b9f17724ac4f1feb83f6c15ae3
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
* stable-4.8:
Bazel: Use hyphen instead of underscore in external repository names
Bazel: Format all build files with buildifier 0.15.0
ChangeIdUtilTest: Remove unused notestCommitDashV
Change-Id: I17436237cd66ca1c2800ad5ab0142f4a2bc07328
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
* stable-4.7:
Bazel: Use hyphen instead of underscore in external repository names
Bazel: Format all build files with buildifier 0.15.0
ChangeIdUtilTest: Remove unused notestCommitDashV
Change-Id: I414ade902dc38b696c566dd604000e3d289f3973
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
When this exception is thrown, the `depth` member variable isn't set
yet, resulting in a confusing error message: "Invalid depth: 0".
Change-Id: I8a2bd5e1d9bec00acb0b8857bbf6821e95bf1369
Signed-off-by: Ivan Frade <ifrade@google.com>
At the moment there are two copies of the client shallow commit list:
one in the request and another in the clientShallowCommits member of
the class.
The verifyShallowCommit function was removing missing object ids
from the member but not the request list, and code afterwards was
using the request's version.
In practice, this didn't cause trouble because these shallow commits
are used as endpoint for a walk, and missing ids are just never reached.
Change-Id: I70a8f1fd46de135da09f16e5d954693c8438ffcb
Signed-off-by: Ivan Frade <ifrade@google.com>
Recent Bazel versions support the hyphen character in external
repository names. On the Gerrit project, the repository names
were harmonized to consistently use hyphen.
As a side effect, it is no longer possible to build jgit from source
in the gerrit tree, due to the different repository names.
Rename the dependencies to use hyphens, consistent with gerrit.
Change-Id: Ideebd858ddd3f0e6f765643001642dfb6c12441f
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
Matthias Sohn
Christian Halstrick
Hector Caballero
David Pursehouse
David Turner
René Scheibe
Jonathan Nieder
Minh Thai
Ivan Frade