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>
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>
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>
Remove it from
* package private functions.
* try blocks
* for loops
this was done with the following python script:
$ cat f.py
import sys
import re
import os
def replaceFinal(m):
return m.group(1) + "(" + m.group(2).replace('final ', '') + ")"
methodDecl = re.compile(r"^([\t ]*[a-zA-Z_ ]+)\(([^)]*)\)")
def subst(fn):
input = open(fn)
os.rename(fn, fn + "~")
dest = open(fn, 'w')
for l in input:
l = methodDecl.sub(replaceFinal, l)
dest.write(l)
dest.close()
for root, dirs, files in os.walk(".", topdown=False):
for f in files:
if not f.endswith('.java'):
continue
full = os.path.join(root, f)
print full
subst(full)
Change-Id: If533a75a417594fc893e7c669d2c1f0f6caeb7ca
Signed-off-by: Han-Wen Nienhuys <hanwen@google.com>
Significantly speed up FileTreeIterator on Windows
Getting attributes of files on Windows is an expensive operation.
Windows stores file attributes in the directory, so they are
basically available "for free" when a directory is listed. The
implementation of Java's Files.walkFileTree() takes advantage of
that (at least in the OpenJDK implementation for Windows) and
provides the attributes from the directory to a FileVisitor.
Using Files.walkFileTree() with a maximum depth of 1 is thus a
good approach on Windows to get both the file names and the
attributes in one go.
In my tests, this gives a significant speed-up of FileTreeIterator
over the "normal" way: using File.listFiles() and then reading the
attributes of each file individually. The speed-up is hard to
quantify exactly, but in my tests I've observed consistently 30-40%
for staging 500 files one after another, each individually, and up
to 50% for individual TreeWalks with a FileTreeIterator.
On Unix, this technique is detrimental. Unix stores file attributes
differently, and getting attributes of individual files is not costly.
On Unix, the old way of doing a listFiles() and getting individual
attributes (both native operations) is about three times faster than
using walkFileTree, which is implemented in Java.
Therefore, move the operation to FS/FS_Win32 and call it from
FileTreeIterator, so that we can have different implementations
depending on the file system.
A little performance test program is included as a JUnit test (to be
run manually).
While this does speed up things on Windows, it doesn't solve the basic
problem of bug 532300: the iterator always gets the full directory
listing and the attributes of all files, and the more files there are
the longer that takes.
Bug: 532300
Change-Id: Ic5facb871c725256c2324b0d97b95e6efc33282a
Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>
FS#runProcess: Fix OutputStream left unclosed after IOException
The runProcess method creates an OutputStream that is not managed by
a try-with-resource because it's manually closed and any IOException
raised by the close() method is explicitly ignored.
Suppress the resource warning with an explanatory comment.
Enclose the call to StreamGobbler#copy in an inner try-block, and move
the call to close() inside its finally block. This prevents the stream
from being left unclosed if StreamGobbler#copy raises IOException.
Change-Id: Idca9adfc4d87e0989d787ad8239c055c0c849814
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
Fix IllegalThreadStateException if stderr closed without exiting
If some process executed by FS#readPipe lived for a while after
closing stderr, FS#GobblerThread#run failed with an
IllegalThreadStateException exception when accessing p.exitValue()
for the process which is still alive.
Add Process#waitFor calls to wait for the process completion.
Bug: 528335
Change-Id: I87e0b6f9ad0b995dbce46ddfb877e33eaf3ae5a6
Signed-off-by: Dmitry Pavlenko <pavlenko@tmatesoft.com>
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
When running on NFS there was a chance that JGits LockFile
semantic is broken because File#createNewFile() may allow
multiple clients to create the same file in parallel. This
change provides a fix which is only used when the new config
option core.supportsAtomicCreateNewFile is set to false. The
default for this option is true. This option can only be set in the
global or the system config file. The repository config file is not
taken into account in this case.
If the config option core.supportsAtomicCreateNewFile is true
then File#createNewFile() is trusted and the behaviour doesn't
change.
But if core.supportsAtomicCreateNewFile is set to false then after
successful creation of the lock file a hardlink to that lock file is
created and the attribute nlink of the lock file is checked to be 2. If
multiple clients manage to create the same lock file nlink would be
greater than 2 showing the error.
This expensive workaround is described in
https://www.time-travellers.org/shane/papers/NFS_considered_harmful.html
section III.d) "Exclusive File Creation"
Change-Id: I3d2cc48d8eb280d5f7039eb94da37804f903be6a
There is a possibility of hitting NPE on a logger if it is not the first
statically initialized member. For example, if another static
initializer creates an instance of its class and the logger is used
from the constructor.
Change-Id: I51fa855a8883c107f2e4ef5ac039dc12a571a7ae
Fix null return from FS.readPipe when command fails to launch
When a command invoked from readPipe fails to launch (i.e. the exec call
fails due to a missing command executable), Process.start() throws,
which gets caught by the generic IOException handler, resulting in a
null return. This change detects this case and rethrows a
CommandFailedException instead.
Additionally, this change uses /bin/sh instead of bash for its posix
command failure test, to accomodate building in environments where bash
is unavailable.
Change-Id: Ifae51e457e5718be610c0a0914b18fe35ea7b008
Signed-off-by: Bryan Donlan <bdonlan@gmail.com>
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
Android wants them to work, and we're only interested in them for bare
repos, so add them just for that.
Make sure to use symlinks instead of just using the copyfile
implementation. Some scripts look up where they're actually located in
order to find related files, so they need the link back to their
project.
Change-Id: I929b69b2505f03036f69e25a55daf93842871f30
Signed-off-by: Dan Willemsen <dwillemsen@google.com>
Signed-off-by: Stefan Beller <sbeller@google.com>
Signed-off-by: Jeff Gaston <jeffrygaston@google.com>
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
Enable and fix warnings about redundant specification of type arguments
Since the introduction of generic type parameter inference in Java 7,
it's not necessary to explicitly specify the type of generic parameters.
Enable the warning in Eclipse, and fix all occurrences.
Change-Id: I9158caf1beca5e4980b6240ac401f3868520aad0
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
Enable and fix 'Should be tagged with @Override' warning
Set missingOverrideAnnotation=warning in Eclipse compiler preferences
which enables the warning:
The method <method> of type <type> should be tagged with @Override
since it actually overrides a superclass method
Justification for this warning is described in:
http://stackoverflow.com/a/94411/381622
Enabling this causes in excess of 1000 warnings across the entire
code-base. They are very easy to fix automatically with Eclipse's
"Quick Fix" tool.
Fix all of them except 2 which cause compilation failure when the
project is built with mvn; add TODO comments on those for further
investigation.
Change-Id: I5772061041fd361fe93137fd8b0ad356e748a29c
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
Change StreamGobbler to Runnable to avoid unused Future
It can be considered a programming error to create a Future<T>
but do nothing with that object. There is an async computation
happening and without holding and checking the Future for done
or exception the caller has no idea if it has completed.
FS doesn't really care about these StreamGobblers finishing.
Instead use Runnable with execute(Runnable), which doesn't
return a Future.
Change-Id: I93b66d1f6c869e66be5c1169d8edafe781e601f6
FS: Fix lazy initialization of non-volatile static field
The 'factory' field is lazy initialized in the detect() method.
According to FindBugs:
Because the compiler or processor may reorder instructions, threads
are not guaranteed to see a completely initialized object, if the
method can be called by multiple threads.
Fix this by declaring the member as 'volatile'.
Change-Id: Ib32663bb28c9564584256e01f625b4e7875e6223
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
Don't log error if system git config does not exist
- enhance FS.readPipe to throw an exception if the external command
fails to enable the caller to handle the command failure
- reduce log level to warning if system git config does not exist
- improve log message
Bug: 476639
Change-Id: I94ae3caec22150dde81f1ea8e1e665df55290d42
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
AddCommandTest is flaky because IOException is thrown sometimes.
Caused by: java.io.IOException: Stream closed
at java.lang.ProcessBuilder$NullOutputStream.write(ProcessBuilder.java:433)
at java.io.OutputStream.write(OutputStream.java:116)
at java.io.BufferedOutputStream.flushBuffer(BufferedOutputStream.java:82)
at java.io.BufferedOutputStream.flush(BufferedOutputStream.java:140)
at java.io.FilterOutputStream.close(FilterOutputStream.java:158)
at org.eclipse.jgit.util.FS.runProcess(FS.java:993)
at org.eclipse.jgit.util.FS.execute(FS.java:1102)
at org.eclipse.jgit.treewalk.WorkingTreeIterator.filterClean(WorkingTreeIterator.java:470)
... 22 more
OpenJDK replaces the underlying OutputStream with NullOutputStream when
the process exits. This throws IOException for all write operation. When
it exits before JGit writes the input to the pipe buffer, the input
stays in BufferedOutputStream. The close method tries to write it again,
and IOException is thrown.
Since we ignore IOException in StreamGobbler, we also ignore it when
we close the stream.
Fixes Bug 499633.
Change-Id: I30c7ac78e05b00bd0152f697848f4d17d53efd17
Signed-off-by: Masaya Suzuki <draftcode@gmail.com>
When FS.runProcess was called and an InputStream was given the method
tried to pump the whole InputStream to the process. When the method
ended the InputStream was not giving any data anymore. Consequently
close the InputStream inside the method.
Change-Id: I0ed738a775e5c977b21447d195acee1ecf5e2cb9
Update existing @Nullable uses to use the JGit-internal version of the
annotation.
Change-Id: I95234ffad4c3110f9597fbd3a2760f653e22ecf7
Signed-off-by: Terry Parker <tparker@google.com>
Enhance FS.runProcess() to support stdin-redirection and binary data
In order to support filters in gitattributes FS.runProcess() is made
public. Support for stdin redirection has been added. Support for binary
data on stdin/stdout (as used be clean/smudge filters) has been added.
Change-Id: Ice2c152e9391368dc5748d7b825a838e3eb755f9
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
Fixed random errors in discoverGitSystemConfig() on Linux where the
process error stream was closed by readPipe() before or while
GobblerThread was reading from it.
Marked readPipe() as @Nullable and fixed potential NPE in
discoverGitSystemConfig() on readPipe() return value.
Fixed process error output randomly mixed with other threads log
messages.
Change-Id: Id882af2762cfb75f010f693b2e1c46eb6968ee82
Signed-off-by: Andrey Loskutov <loskutov@gmx.de>
[performance] Remove synthetic access$ methods in lib, util and dircache
Java compiler must generate synthetic access methods for private methods
and fields of the enclosing class if they are accessed from inner
classes and vice versa.
While invisible in the code, those synthetic access methods exist in the
bytecode and seem to produce some extra execution overhead at runtime
(compared with the direct access to this fields or methods), see
https://git.eclipse.org/r/58948/.
By removing the "private" access modifier from affected methods and
fields we help compiler to avoid generation of synthetic access methods
and hope to improve execution performance.
To validate changes, one can either use javap or use Bytecode Outline
plugin in Eclipse. In both cases one should look for "synthetic
access$<number>" methods at the end of the class and inner class files
in question - there should be none.
NB: don't mix this "synthetic access$" methods up with "public synthetic
bridge" methods generated to allow generic method override return types.
Change-Id: Ie7b65f251ec4452d5a5ed48aa0f272cf49a9aecd
Signed-off-by: Andrey Loskutov <loskutov@gmx.de>
Since 4.0 we require Java 7 so there is no longer a need to override the
following methods in FS_POSIX, FS_Win32, FS_Win32_Cygwin
- lastModified()
- setLastModified()
- length()
- isSymlink()
- exists()
- isDirectory()
- isFile()
- isHidden()
Hence implement these methods in FS and remove overrides in subclasses.
Change-Id: I5dbde6ec806c66c86ac542978918361461021294
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
Change FS not to throw NPE when facing InMemory databases
The FS class and the subclasses FS_POSIX assumed in the findHook()
method that every repository has a valid gitDir. But in tests when using
in-memory-repositories this is not true and this method was generating
NPEs. Change the method to return null if no repository directory can be
determined.
Change-Id: I38a4d36dc6452b5dacae3d0dbf562b569ca3c19b
Cleanup Attributes and remove obsoleted Java7BasicAttributes class
After jgit moved to Java 7 there is no need in an extra
Java7BasicAttributes class. Also all fields of Attributes can be made
final now.
Change-Id: I0be6daf7758189b0eecc4e26294bd278ed8bf7a0
Signed-off-by: Andrey Loskutov <loskutov@gmx.de>
FS: Extract GobblerThread into a private static class
The primary goal is to improve exception readability. Since this is a
standalone thread, just logging the stack trace of the caught
exception is not very useful:
java.io.IOException: Stream closed
at java.io.BufferedInputStream.getBufIfOpen(BufferedInputStream.java:162)
at java.io.BufferedInputStream.read(BufferedInputStream.java:258)
at org.eclipse.jgit.util.FS$2.run(FS.java:451)
Providing a named class eliminates the "FS$2", and including the
command name provides a little more context in the error message.
A future improvement might include the stack trace that created the
GobblerThread as well.
Change-Id: Ibf16d15b47a85b6f41844a177e398c2fc94f27b0
FS: Allow to manually set the path to the Git system config file
Now that d7a4473 removed the gitprefix property, we did not have a way to
specify the path to the Git system config file in case
discoverGitSystemConfig() fails. Fix that by introducing a member variable
that caches the result of discoverGitSystemConfig() as well as a setter
method to overwrite the content of that variable.
Change-Id: Icd965bffbe2f11b18c9505ee2ddd2afad5b64d70
Signed-off-by: Sebastian Schuberth <sschuberth@gmail.com>
FS: Improve javadoc of some recently introduced methods
Change-Id: I31e788ee20ac3e8439559d9060d39e9792f6dc7d
Signed-off-by: Sebastian Schuberth <sschuberth@gmail.com>
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
The only purpose of the gitprefix logic was to determine the path to the
system-wide config file. This is now done by discoverGitSystemConfig()
independent of the gitprefix, so get rid of this unused code.
Change-Id: Iaa88df9bd066dc1ed4067c18618af809e49876b3
Signed-off-by: Sebastian Schuberth <sschuberth@gmail.com>
FS: Add a method to discover the system-wide config file
Change-Id: I969e26a5ab5f8ca3ab29024f405c1e34afdba493
Signed-off-by: Sebastian Schuberth <sschuberth@gmail.com>
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
FS: Extend readPipe() to optionally take additional environment
Change-Id: I4db7763826e4ada92074317d4d1c9a32299f3af8
Signed-off-by: Sebastian Schuberth <sschuberth@gmail.com>
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
Split discoverGitPrefix() code out into discoverGitExe()
Change-Id: I700540eec06efb24eeb09bfcb40420820c32d156
Signed-off-by: Sebastian Schuberth <sschuberth@gmail.com>
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
Move resolveGrandparentFile() to the base class for wider use
Change-Id: I67ec732ea2e5345a6946783f0c5ef60c07ce254e
Signed-off-by: Sebastian Schuberth <sschuberth@gmail.com>
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
FS.readPipe() shouldn't log IOException as an error
This unintentionally was changed from severity debug to error which is
causing unexpected log entries.
Bug: 463349
Change-Id: I4b6d42a1420652ab6824e237bd231ba86896acbf
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
Merge bundle org.eclipse.jgit.java7 into org.eclipse.jgit
As we moved minimum Java version to 7 we don't need a separate bundle
and feature for JGit features depending on Java 7 anymore.
Change-Id: Ib5da61b0886ddbdea65298f1e8c6d65c9879ced1
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
Refactored pre-commit hook to make it less invasive.
Hooks are now obtained via a convenient API like git commands, and
callers don't have to check for their existence.
The pre-commit hook has been updated accordingly.
Change-Id: I3383ffb10e2f3b588d7367b9139b606ec7f62758
Signed-off-by: Laurent Delaigue <laurent.delaigue@obeo.fr>
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
David Pursehouse
Matthias Sohn
Cliffred van Velzen
Han-Wen Nienhuys
Thomas Wolf
Dmitry Pavlenko
Christian Halstrick
Saša Živkov
Bryan Donlan
Dan Willemsen
Shawn Pearce
Masaya Suzuki
Terry Parker
Andrey Loskutov
Kaloyan Raev
Dave Borowitz
Sebastian Schuberth
Laurent Delaigue