Error Prone reports:
[ClassNewInstance] Class.newInstance() bypasses exception checking;
prefer getDeclaredConstructor().newInstance()
See https://errorprone.info/bugpattern/ClassNewInstance
This was the only occurrence of the warning in the code base; now it's
fixed, increase the severity to ERROR to prevent future occurrences.
Change-Id: Ic04d1c5d2bd458bbb4bb399d6ce9d147bd48d0b1
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
By using File#setLastModified, we can create a racy git situation
stably.
Tested with --runs_per_test=100
Bug: 526111
Change-Id: I60b3632d353e19f335668325aa603640be423f58
Signed-off-by: Masaya Suzuki <masayasuzuki@google.com>
Archives created by the ArchiveCommand didn't produce deterministic
archive hashes. For RevCommits RevWalk.parseTree returns the root tree
instead of the RevCommit hence retrieving the commit's timestamp didn't
work. Instead use RevWalk.parseAny and extract the tree manually.
Archive entries store timestamps with 1 second resolution hence we need
to wait longer when creating the same archive twice and compare archive
hashes. Otherwise hash comparison in tests wouldn't fail without this
patch.
Bug: 548312
Change-Id: I437d515de51cf68265584d28a8446cebe6341b79
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
update Maven plugins
- ecj to 3.17.0
- error_prone_core to 2.3.3
- plexus-compiler-eclipse to 2.8.5
- plexus-compiler-javac to 2.8.5
- plexus-compiler-javac-errorprone to 2.8.5
Change-Id: I51ecb44538915ed84db041510562394bce977a3e
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
update Maven plugins
- jacoco-maven-plugin to 0.8.4
- japicmp-maven-plugin to 0.14.1
- maven-compiler-plugin to 3.8.1
- maven-deploy-plugin to 3.0.0-M1
- maven-enforcer-plugin to 3.0.0-M2
- maven-install-plugin to 3.0.0-M1
- maven-jar-plugin to 3.1.2
- maven-javadoc-plugin to 3.1.0
- maven-jxr-plugin to 3.0.0
- maven-pmd-plugin to 3.12.0
- maven-resources-plugin to 3.1.0
- maven-shade-plugin to 3.2.1
- maven-source-plugin to 3.1.0
- maven-surefire-plugin to 3.0.0-M3
- spotbugs-maven-plugin to 3.1.12
- tycho to 1.3.0
- tycho-pack200a-plugin to 1.3.0
- tycho-pack200b-plugin to 1.3.0
Cleanup Maven warnings
- pin version of all used Maven plugins
- remove deprecated way to declare minimum Maven version
Change-Id: If23e2e2bb03e5e1e7b1eb9d4924a8faa0aa3704e
Signed-off-by: Matthias Sohn <matthias.sohn@sap.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>
(cherry picked from commit ee40efcea4)
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>
ParseableSimpleDateFormat is an enum, and enums must be immutable,
hence the member should be final. At the same time, make it private
since it does not need to be publicly visible.
Change-Id: I7e181f591038d556f1123b6e37adf8441059e99a
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
Reference comparison is intentional. The END and DELIM string
constants are used as sentinels and will always be the same
instances.
Suppress both ReferenceEquality and StringEquality warnings.
Change-Id: I4ce0495702c56b3911f42f26c2f81d28073cbe19
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
Reference comparison with EMPTY and MISSING_FILE is intended; these
are static instances used as markers, and will always be the same
instances.
Change-Id: Ic27f5b797bdb9370cf8f6b3b7bb3f1523d4a454c
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
Reference comparison of the RevCommit objects is OK; BlameGenerator uses
a single RevWalk which caches the RevCommits, so if a given commit is
cached the RevWalk will always return the same instance.
Factor the comparison out to a method, and suppress the warning with an
explanatory comment.
Change-Id: I5a148001dba7749ac15119aed388adb12b6f51ad
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
Otherwize the jgit command line does not support the xz archive format.
Change-Id: I64897e658e4ed761614948b9d432f26e83034f15
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
Issues reported by downstream analyzers.
The "hash" method can be static.
It is a good practice to group overloaded methods. Move the write(URL)
method with the other writes.
Change-Id: Ia42c0d7081333edcb77e58d5e627929d29672490
Signed-off-by: Ivan Frade <ifrade@google.com>
Downstream analyzers reported empty fields in the javadoc. I corrected
few more details:
* Fill empty javadoc fields.
* Use <p> to separate description paragraphs.
* End description paragraphs with a period.
* Remove period at the end of field descriptions.
Change-Id: I749e4b821fc855999caddc442ac788fa514386ea
Signed-off-by: Ivan Frade <ifrade@google.com>
Reported by downstream analyzers. Suppress the warning on reference
equality for isMissing and fill an empty javadoc field.
Change-Id: I3494423daf2a53ca10e0a9c66553f00204c35396
Signed-off-by: Ivan Frade <ifrade@google.com>
Using the | and & operators in boolean conditions results in a warning
from Error Prone:
[ShortCircuitBoolean]
Prefer the short-circuiting boolean operators && and || to & and |.
see https://errorprone.info/bugpattern/ShortCircuitBoolean
Change-Id: I182f986263b8b9ac189907f4bd1662b4092a52d8
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
Using the | and & operators in boolean conditions results in a warning
from Error Prone:
[ShortCircuitBoolean]
Prefer the short-circuiting boolean operators && and || to & and |.
see https://errorprone.info/bugpattern/ShortCircuitBoolean
Change-Id: I6cccca3fdd28bf93b302a9b8a66e68ac912cb60d
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
Using the | and & operators in boolean conditions results in a warning
from Error Prone:
[ShortCircuitBoolean]
Prefer the short-circuiting boolean operators && and || to & and |.
see https://errorprone.info/bugpattern/ShortCircuitBoolean
Change-Id: I4275c60306e43c74030c4465ba02cb853ad444e1
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
Fix all remaining instances of the OperatorPrededence warning, by adding
parentheses to make the precedence explicit.
Change-Id: Ib296dfed09f9be042d0ff0f7fad8214e4dd766b4
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
* master:
Handle missing "ours" stage in WorkingTreeIterator.hasCrLfInIndex()
Config: Add helper method to check for empty value
ErrorProne: Increase severity of FutureReturnValueIgnored to ERROR
FS_Win32: Add missing parentheses on if-blocks
Upgrade spotbugs to 3.1.12
Abbreviated{Long}ObjectId: Make operator precedence explicit
GC: Update TODO comments
FS_POSIX: Fix reference comparison of Boolean.FALSE
Increase bazel timeout for long running tests
Use bazelisk to switch between used bazel version
Bump minimum Bazel version to 0.26.1
Bazel: Bump skylib library version to 0.8.0
Retry loading config when locked by another process
Make pull --rebase on an unborn branch do a checkout
Warn if configured cookie file is missing
Handle escaped CR-LF in git config files
DescribeCommand: use glob match instead of path match
Fix off-by-one error in RebaseTodoFile when reading a todo file
Consistently use "!isEmpty()" to detect non-empty list
TransportHttp: Check for non-empty list with "!isEmpty()" rather than
"size() > 0"
TransportHttp: Fix comparison of size with ">= 0"
NetscapeCookieFileTest: Split HttpCookiesMatcher to own class
Bazel: Add missing dependency on mockito for TransportHttpTest
Determine hard-linking and nlink support per FileStore
Support reading and writing cookies.
Repository: Add getIdentifier() method to avoid instanceof operator
Update to Orbit R20190602212107
PacketLineIn: Deprecate the END constant
PacketLineIn: Add an iterator over strings in the input stream
Replace most usages of PacketLineIn.END with PacketLineIn.end()
PacketLineIn: Deprecate DELIM constant
Replace trivial reference comparison of PacketLineIn.{DELIM,END}
PacketLineIn: Rename isDelim to isDelimiter
ProtocolV2ParserTest: Fix typo in comment
Upgrade Bouncy Castle to 1.61
Update to Orbit R20190531194818 and rollback update to Ant 1.10.6
cli: Add the --always option to describe
DescribeCommand: Support the "always" option
cli: Add the --tags option to describe
DescribeCommand: Consistenly omit the default value
Remove excess blank line in FileUtilsTest
PacketLineIn: Add helper methods to check for END and DELIM
UploadPackTest: Rename variable to avoid hiding class member
UploadPackTest: Add missing <> operator on instantiation of ArrayList
BitmapCalculator: javadoc fixes
RevWalkUtils: add progress callback to findBranchesReachableFrom
Upgrade maven-source-plugin to 3.1.0
Upgrade maven-jar-plugin to 3.1.2
Upgrade jacoco-maven-plugin to 0.8.4
BitmapCalculator and its test: add missing license header
RevWalk: new method createReachabilityChecker()
Change-Id: I4d76c7c0dbe6411c842f3468b709f7df51789c08
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
In a delete-modify conflict with the deletion as "ours" there may be
no stage 2 in the index. Add appropriate null checks. Add a new test
for this case, and verify that the file gets added with a single LF
after conflict resolution with core.autocrlf=true. This matches the
behavior of canonical git for this case.
Bug: 547724
Change-Id: I1bafdb83d9b78bf85294c78325e818e72fae53bc
Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>
Rename MAGIC_EMPTY_VALUE to MISSING_ENTRY, make it private, and add
a helper method to check if a given string is that value.
This avoids that callers trigger the "reference equality" warning
from Error Prone.
Change-Id: Idc76f78c0cf1828aa48d02ee33911a4b5df50355
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
The only remaining code where the return value is ignored is in tests.
Update them to store the value and perform a basic assertion.
Change-Id: I29ef5bd5dd0648aac3490f9e47ecc74544109652
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
The TODO comments say "in 5.0", but 5.0 was already released without
resolving them. Remove "in 5.0" on the assumption that the mentioned
improvements still need to be done at some point.
Change-Id: I3eb429803e2266de3fc490e1f3912991c08aa1ad
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
EolRepositoryTest and GcCommitSelectionTest timed out frequently when
running unit tests using bazel with the default timeout "moderate"
(300s). Increase timeout of these tests to "long" (900s).
Change-Id: I43588cf950f55b50f868d9fe9c66d22bd428a54c
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
Bazelisk is utility to switch to the right Bazel version, that we
used to have with Buck build tool: [1].
Bazelisk will download the right Bazel version only once and will use
it in subsequent calls:
$ bazelisk build :release
2019/06/06 16:22:15 Downloading \
https://releases.bazel.build/0.26.1/release/bazel-0.26.1-linux-x86_64...
Bazelisk is storing the binaries in user's cache directory: [2], e.g.
on Linux OS:
$ ls -1 ~/.cache/bazelisk/bin
bazel-0.26.1-linux-x86_64
* [1] https://github.com/bazelbuild/bazelisk
* [2] https://golang.org/pkg/os/#UserCacheDir
Change-Id: Ia9180fb75f8cc17a0a0232622cf33a13bfad6b60
Signed-off-by: David Ostrovsky <david@ostrovsky.org>
When loading the config, a FileNotFoundException may occur if the file
exists but cannot be read (see [1]). This is the case on Windows with a
virus scanner checking the file. Therefore if the file exists and that
exception is thrown, retry multiple times, similar to how this was
already implemented for IOException.
[1] https://docs.oracle.com/javase/8/docs/api/java/io/FileNotFoundException.html
Bug: 529522
Change-Id: Ic5dc3b7b24bb0005d6256ed00513bc7c0b91e613
Signed-off-by: Michael Keppler <Michael.Keppler@gmx.de>
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
A merging pull on an unborn branch was already supported. But a
rebasing pull failed. If the user has pull.rebase = true in his
user config, the pull would try to rebase. Rebasing needs a parent
commit, though. Native git handles this case:
git init
git remote add origin <URI>
git pull --rebase origin master
Check up front in PullCommand for the unborn head and just do a
checkout in this case. MergeCommand already has similar code.
Bug: 544965
Change-Id: I1277e1ac0b0364b4623fd791f3d6b07bd5f58fca
Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>
We logged a stack trace if the configured http.cookieFile was missing.
Instead only log a warning.
Bug: 548081
Change-Id: I42e39f5ad8ffce7b43162e5068f60af073b8a126
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
Canonical git treats CR-LF in config files as LF.[1][2] JGit does so,
too, except when escaped as a line continuation. Correct this and
treat the sequence \-CR-LF as a line continuation.
[1] https://github.com/git/git/commit/db2c075d9
[2] https://github.com/git/git/blob/v2.21.0/config.c#L485
Bug: 545850
Change-Id: I51e7378a22c21b3baa3701163c423d04c900af5a
Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
Otherwise tags may fail to match if their name contains slashes.
Canonical git also uses its wildcard matcher in glob mode.[1]
[1] https://github.com/git/git/blob/v2.21.0/builtin/describe.c#L182
Bug: 546703
Change-Id: I122c7959974fa1fc6a53dfc65837e4314a8badd4
Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>
Commit messages of length 1 were not read. 'lineEnd' is the offset
of the last character in the line before the terminating LF or CR-LF,
and 'nextSpace' is actually the offset of the character _after_ the
next space. With a one-character commit message, nextSpace == lineEnd.
The code also assumes the commit message to be optional, but actually
failed in that case because it read beyond the line ending. Fix that,
too.
Add a test case for reading a todo file.
Bug: 546245
Change-Id: I368d63615930ea2398a6230e756442fd88870654
Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>
Replace "size() > 0" with "!isEmpty()" where appropriate.
In the Status implementation we can drop the check; the subsequent
loop will only execute when the list is non-empty anyway.
Change-Id: I355aff551a603373e702a9d44304f087b476263c
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
Error Prone reports:
[SizeGreaterThanOrEqualsZero] Comparison of a size >= 0 is always true,
did you intend to check for non-emptiness?
see https://errorprone.info/bugpattern/SizeGreaterThanOrEqualsZero
Change-Id: Ie964771cacca4b15569eb45f6e273ad2a7e2e49c
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
David Pursehouse
Masaya Suzuki
Matthias Sohn
Ivan Frade
Thomas Wolf
David Ostrovsky
Michael Keppler