When setting timeout on push, BasePackConnection creates a timer, which
will be terminated when push finishes. But, when using
SmartHttpPushConnection, it dropped the first timer created in the
constructor and then created another timer in doPush. If new threads are
created faster than the gc collects then this may stop the service if
it's hitting the max process limit. Hence don't create a new timer if it
already exists.
Bug: 474947
Change-Id: I6746ffe4584ad919369afd5bdbba66fe736be314
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
PathFilter and PathFilterGroup form JGit's implementation of git's
path-limiting feature in commands like log and diff. To save time
when traversing trees, a path specification
foo/bar/baz
tells the tree walker not to traverse unrelated trees like qux/. It
does that by returning false from include when the tree walker is
visiting qux and true when it is visiting foo.
Unfortunately that test was implemented to be slightly over-eager: it
doesn't only return true when asked whether to visit a subtree "foo"
but when asked about a plain file "foo" as well. As a result, diffs
and logs restricted to some-file/non-existing-suffix unexpectedly
match against some-file:
$ jgit log -- LICENSE/no-such-file
commit 629fd0d594
Author: Shawn O. Pearce <spearce@spearce.org>
Date: Fri Jul 02 14:52:49 2010 -0700
Clean up LICENSE file
[...]
Fix it by checking against the entry's mode.
Gitiles +log has the same bug and benefits from the same fix.
Callers know not to worry about what subtrees are included in the tree
walk because shouldBeRecursive() returns true in this case, so this
behavior change should be safe. This also better matches the behavior
of C git:
$ empty=$(git mktree </dev/null)
$ git diff-tree --abbrev $empty HEAD -- LICENSE/no-such-file
$ git diff-tree --abbrev $empty HEAD -- tools/no-such-file
:000000 040000 0000000... b62648d... A tools
Bug: 484266
Change-Id: Ib4d53bddd8413a9548622c7b25b338d287d8889d
Expand the existing PathFilterGroup tests to check which paths the
tree entry matches. This expands test coverage by ensuring that
PathFilterGroup's simpler code path to match against a single
PathFilter works correctly.
While at it, move the check on tree entry d/e/f/g.y into two separate
tests: one to check that it doesn't match any of the configured paths,
and another to check that it does not throw StopWalkException to end
the walk early.
Change-Id: I55bd512cd049fc2018659e2f86a4b8650f171fda
Pushing with JGit commandline to e.g. Github failed with "unauthorized"
since HttpUrlConnection calls the configured authenticator implicitly.
The problem is that during a push two requests are sent to the server,
first a GET and then a POST (containing the pack data). The first GET
request sent anonymously is rejected with 401 (unauthorized). When an
Authenticator is installed the java.net classes will use the
Authenticator to ask the user for credentials and retry the request.
But this happens under the hood and JGit level code doesn't see that
this happens.
The next request is the POST but since JGit thinks the first GET request
went through anonymously it doesn't add authentication headers to the
POST request. This POST of course also fails with 401 but since this
request contains a lot of body-data streamed from JGit (the pack file!)
the java.net classes can't simply retry the request with authorization
headers. The whole process fails.
Fix this by using Apache httpclient which doesn't use Authenticator to
retrieve credentials. Instead initialize TransportCommand to use the
default credential provider if no other credentials provider was set
explicitly. org.eclipse.jgit.pgm.Main sets this default for the JGit
command line client.
Change-Id: Ic4e0f8b60d4bd6e69d91eae0c7e1b44cdf851b00
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
This was done for ConsoleCredentialsProvider earlier, we need the
AwtCredentialsProvider for debugging jgit command line since there is no
console in Eclipse. Hence also add support for .netrc here.
Change-Id: Ibbd45b73efc663821866754454cea65e6d03f832
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
BasePackPushConnection#readStringLongTimeout() was setting a timeout 10
times bigger than some other timeout or the pack transfer time. This
could lead to negative integer values when we hit an arithmetic
overflow. Add a check for this situation and set the timeout to
Integer.MAX_VALUE when overflow happens.
Bug: 484352
CC: Eugene Petrenko <eugene.petrenko@gmail.com>
Change-Id: Ie2a86312c1bcb1ec3e6388fa490ab3c845d41808
This change fixes all compiler errors in JGit and replaces possible
NPE's with either appropriate exceptions, avoiding multiple "Nullable
return" method calls or early returning from the method.
Change-Id: I24c8a600ec962d61d5f40abf73eac4203e115240
Signed-off-by: Andrey Loskutov <loskutov@gmx.de>
If an application uses PushConnection directly on the native Git wire
protocols JGit should send along the application's expected oldId, not
the advertised value. This allows the remote peer to compare-and-swap
since it was not tested inside JGit.
Discovered when I tried to use a PushConnection (bypassing the
standard PushProcess) and the client blindly overwrote the remote
reference, even though my app had supplied the wrong ObjectId for
the expectedOldObjectId. This was not expected and cost me over an
hour of debugging, plus "corruption" in the remote repository.
By passing along the exact expectedOldObjectId from the app the
remote side can do the check that the application skipped, and
avoid data loss.
Change-Id: Id3920837e6c47100376225bb4dd61fa3e88c64db
FileTreeIterator was calling by mistake
WorkingTreeIterator.idSubmodule(Entry). Instead it should always compute
idSubmodule on its own.
Change-Id: Id1b988aded06939b1d7edd2671e34bf756896c0e
In I1f5dc07182dbf6bba2a9f4807fdd25b475da4ead, FileBasedConfig got
support for reading a configuration with UTF8 BOM. Apply the same
support to BlobBasedConfig, to make SubmoduleWalk able to parse
.gitmodules configurations with BOM.
Change-Id: I25b5474779952fe2c076180b96fc2869eef190a8
Signed-off-by: Doug Kelly <dougk.ff7@gmail.com>
The ChainingCredentialsProvider gave up chaining to the next provider if
the first one returned no credentials items for the given URI.
Change-Id: I9539c50db35e564db9d43d8ebb71d7e9c6fdcc19
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
JGit's Nullable type was added[1] in the hope of being able to add
nullness annotations that (a) do not preclude building and running
with Java 7 and (b) could be shared by Gerrit, which uses a custom
Nullable type for other reasons[2]. Sharing a type is useful because
Eclipse's null analysis is only able to use one Nullable type at a
time in a given workspace (so for this analysis to function in a
workspace used to develop Gerrit, JGit and Gerrit would need to use
the same Nullable type).
The new Nullable type has CLASS instead of RUNTIME retention because
there wasn't any obvious use for the annotation at run time.
Gerrit uses the Nullable annotation to communicate with Guice. Guice
injection happens at runtime, so it needs to be able to read the
@Nullable annotations at run time[3]. Otherwise Guice produces
provisioning errors, such as
3) null returned by binding at com.google.gerrit.lucene.LuceneChangeIndex$Factory.create()
but parameter 7 of com.google.gerrit.lucene.LuceneChangeIndex.<init>() is not @Nullable
Switch to RUNTIME retention to avoid this.
While at it, update the javadoc to explain more clearly how this
annotation relates to other Nullable types[4]. This should make it
clearer why JGit needed another Nullable type:
A. Avoiding dependency on Java 8
B. RUNTIME retention to allow Guice to read the annotation at run time
C. Named Nullable so Guice can recognize the annotation
D. Not an addition to Java EE's javax.annotation package, to avoid
the split-package problem[2] that prevents the annotation from
being readable at run time when loaded from an OSGi container
E. Avoiding heavyweight dependencies, deprecated dependencies, and
dependencies on package internals
org.checkerframework.checker.nullness.qual.Nullable: A
com.sun.istack.internal.Nullable: B, E
*.CheckForNull, *.NullAllowed, etc: C
edu.umd.cs.findbugs.annotations.Nullable: B, E
javax.annotation.Nullable: D
org.eclipse.jdt.annotation.Nullable: B
org.jetbrains.annotations.Nullable: B
org.jmlspecs.annotation.Nullable: E
android.annotation.Nullable, android.support.annotation.Nullable: E
[1] https://git.eclipse.org/r/59993
[2] https://gerrit-review.googlesource.com/50112
[3] https://github.com/google/guice/blob/master/core/src/com/google/inject/internal/Nullability.java
[4] https://github.com/typetools/checker-framework/blob/5832a01f1/checker/src/org/checkerframework/checker/nullness/NullnessAnnotatedTypeFactory.java#L118http://types.cs.washington.edu/checker-framework/current/checker-framework-manual.html#nullness-related-work
Change-Id: I6c482653d2b53e3509abb11211b67fc29cf2949c
Signed-off-by: Jonathan Nieder <jrn@google.com>
If the password char array is null constructing a new String from this
array fails with a NPE. Add a null check to fix this.
Change-Id: Ifae6eecca38d5f114861f44658a32521e6e96866
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
This should mirror the behavior of `git push --atomic` where the
client asks the server to apply all-or-nothing. Some JGit servers
already support this based on a custom DFS backend. InMemoryRepository
is extended to support atomic push for unit testing purposes.
Local disk server side support inside of JGit is a more complex animal
due to the excessive amount of file locking required to protect every
reference as a loose reference.
Change-Id: I15083fbe48447678e034afeffb4639572a32f50c
PreReceiveHook is given a Collection<ReceiveCommand> and it can be
very useful here to call ReceiveCommand.filter(cmds, NOT_ATTEMPTED).
Overload filter to accept both Iterable and List.
Keep backwards binary compatibility for List by upcasting to Iterable.
Change-Id: Ib1341876c703670945ef209edc8259715ee86c26
Recursively copying a tree into a DirCache is a bottleneck for some
algorithms like the in memory merge code in Gerrit Code Review. Drop
a layer down in the stack and use CanonicalTreeParser directly as the
addition logic only processes 1 tree at a time and does not need the
merge sorting feature (or overhead) of TreeWalk.
Combined with 761814fe9c ("DirCacheEntry: Speed up creation by
avoiding string cast") tree loading 38,900 entries nearly halves
in running time from 70ms to 36ms on some platforms.
Change-Id: If1490ca25de0679a71cf508f59b486f9cc816165
Application code sometimes wants to read a DirCache from an ObjectId,
but its confusing how to do this because its buried inside the
DirCacheBuilder.
Use this utility in a few places within JGit that also want to read
a DirCache from a tree's ObjectId.
Change-Id: I578b7e18e58753d154937f4ab835012b09e5adca
Instead of checking every entry for .gitattributes only look for the
entry on request by TreeWalk. This avoids impacting uses like RevWalk
filtering history.
When the attrs is requested skip to the start of the tree and look for
.gitattributes until either it is found, or it is impossible to be
present. Due to the sorting rules of tree entries .gitattributes
should be among the first or second entries in the tree so very few
entries will need to be considered.
Waiting to find the .gitattributes file by native ordering may miss
attrs for files like .config, which sorts before .gitattributes.
Starting from the front of the tree on demand ensures the attributes
are parsed as early as necessary to process any entry in the tree.
Due to TreeWalk recursively processing up the tree of iterators we
cannot just reset the current CanonicalTreeParser to the start as
parent parsers share the same path buffer as their children.
Resetting a parent to look for .gitattributes may overwrite path
buffer data used by a child iterator. Work around this by building a
new temporary CanonicalTreeParser instance.
Change-Id: Ife950253b687be325340d27e9915c9a40df2641c
Line breaking before , is ugly to read. Most formatters and humans
expect line break after , so update a few offensive locations.
Use String.format() for the construction of the error message when
a bad DirCachEntry is being failed on. This simplifies the code and
its not a performance critical section.
Change-Id: I5d990389e7ba24ef0861cf8ec0026ed030d4aeda
The checkPath function is available as a byte[] form, in fact the
String form just converts to byte[] to run the algorithm.
Having DirCacheEntry take a byte[] -> String -> byte[] to check if
each path is valid is a huge waste of CPU time. On some systems it
can double the time required to read 38,999 files from trees to the
DirCache. This slows down any operation using a DirCache.
Expose the byte[] form and use it for DirCacheEntry creation.
Change-Id: I6db7bc793ece99ff3c356338d793c07c061aeac7
The internal array may be longer than entryCnt, in this case the tail
of the array is padded with null entries. Do not return those to the
caller of getEntriesWithin().
Change-Id: I19efb05e103fab6b739ced407f6e28155a48dba6
If defined in .gitattributes call smudge filter during checkout.
To support checkout where current HEAD,index do not contain attributes
we need to also consider attributes from the tree we checkout. Therefore
CanonicalTreeParser has to learn how to provide attributes.
Change-Id: I168fdb81a8e1a9f991587b3e95a36550ea845f0a
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
When filters are defined for certain paths in gitattributes make
sure that clean filters are processed when adding new content to the
object database.
Change-Id: Iffd72914cec5b434ba4d0de232e285b7492db868
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
Attributes represents a semantic collector of Attribute(s) and replaces
the anonymous Map<String,Attribute>. This class will be returned by
TreeWalk.getAttributes(). It offers convenient access to the attributes
wrapped in the Attributes object. Adds preparations for a future
Attribute Macro Expansion
Change-Id: I8348c8c457a2a7f1f0c48050e10399b0fa1cdbe1
Signed-off-by: Ivan Motsch <ivan.motsch@bsiag.com>
PushCommandTest and RunExternalScriptTest didn't succeed on Windows.
Fix this by expecting a simple line-feed as line ending (instead of the
platform dependent line separator. Additionally correct the computation
of expected URLs in PushCommandTest.
Change-Id: Idcdc41cd7e535ff88df33ea0a249333ed8fc91b0
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
This test expected that the test scripts emit a platform-dependent
newline (crlf on windows, lf on linux). But that's not true. Expected
result should always be a trailing "\n" because the test scripts
explicitly echo a "\n" in the end.
Change-Id: I604e08cda8cebe276b5214ba0f618b6112c3441f
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
Jonathan Nieder
Shawn Pearce
Matthias Sohn
Mike Gilbode
Christian Halstrick
Andrey Loskutov
Shawn Pearce
Kaloyan Raev
Doug Kelly
Ivan Motsch