Commit Graph

166 Commits

Author SHA1 Message Date
Christian Halstrick f3fb5824ba Add builder-style API to jgit and Commit & Log cmd
Added a new package org.eclipse.jgit.api and a builder-style API for
jgit. Added also the first implementation for two git commands: Commit
and Log.

This API is intended to be used by external components when
functionalities of the standard git commands are required. It will also
help to ease writing JGit tests.

For internal usages this API may often not be optimal because the git
commands are doing much more than required or they expect parameters of
an unappropriate type.

Change-Id: I71ac4839ab9d2f848307eba9252090c586b4146b
Signed-off-by: Christian Halstrick <christian.halstrick@sap.com>
2010-05-10 15:17:55 +02:00
Robin Rosenberg 541ad72ac6 Merge "Added MERGING_RESOLVED repository state" 2010-05-08 17:16:26 -04:00
Robin Rosenberg 0df679aea1 Merge "A stages field and getter for GitIndex entry introduced" 2010-05-08 17:13:25 -04:00
Robin Rosenberg a496410df9 A stages field and getter for GitIndex entry introduced
Currently, if the Index contains a file in more than one stage, only
the last entry (containing the highest stage) will be registered in
GitIndex. For applications it can be useful to not only know about the
highest stage, but also which other stages are present, e.g. to detect
the type of conflict the file is in.

Change-Id: I2d4ff9f6023335d9ba6ea25d8e77c8e283ae53cb
Signed-off-by: Robin Rosenberg <robin.rosenberg@dewire.com>
2010-05-08 23:12:19 +02:00
Christian Halstrick b9ab040b45 Added MERGING_RESOLVED repository state
The repository state tells in which state the repo is and also which actions
are currently allowed. The state MERGING is telling that a commit is not
possible. But this is only true in the case of unmerged paths in the index.
When we are merging but have resolved all conflicts then we are in a special
state: We are still merging (means the next commit should have multiple
parents) but a commit is now allowed.

Since the MERGING state "canCommit()" cannot be enhanced to return true/false
based on the index state (MERGING is an enum value which does not have a
reference to the repository its state it is representing) I had to introduce a new
state MERGING_RESOLVED. This new state will report that a commit is possible.

CAUTION: there might be the chance that users of jgit previously blindly did a
plain commit (with only one parent) when the RepositoryState allowed them to
do so. With this change these users will now be confronted with a RepositoryState
which says a commit is possible but before they can commit they'll have to
check the MERGE_MESSAGE and MERGE_HEAD files and use the info from these
files.

Change-Id: I0a885e2fe8c85049fb23722351ab89cf2c81a431
Signed-off-by: Christian Halstrick <christian.halstrick@sap.com>
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
2010-05-08 22:03:18 +02:00
Shawn O. Pearce dd63f5cfc1 Fix FooterLine.matches(FooterKey) on same length keys
If two keys are the same length, but don't share the same sequence
of characters, we were incorrectly claiming they still matched due
to a bug in the for loop condition.  I used the wrong variable and
the loop never executed, resulting in equality anytime the two keys
being compared were the same length.

Use the proper local variable to loop through the arrays, and add
a JUnit test to verify equality works as expected.

Change-Id: I4a02400e65a9b2e0da925b05a2cc4b579e1dd33a
Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
2010-05-04 16:25:20 -07:00
Chris Aniszczyk d011a377cb Merge "Fix handling of corruption for truncated objects" 2010-05-03 03:40:36 -04:00
Chris Aniszczyk 28e42cb463 Merge "Don't insert the same pack twice into a pack list" 2010-05-03 03:40:06 -04:00
Chris Aniszczyk 11096a89a5 Merge changes I0d339b9f,I0e6673b8
* changes:
  Favor earlier PackFile instances over later duplicates
  Cleanup duplicated object reuse code in PackWriter
2010-05-03 03:39:47 -04:00
Robin Rosenberg c10e134157 Fix handling of corruption for truncated objects
If a loose object was corrupted by truncation, JGit would hang.

Change-Id: I7e4c14f44183a5fcb37c1562e81682bddeba80ad
Signed-off-by: Robin Rosenberg <robin.rosenberg@dewire.com>
2010-05-01 09:50:38 +02:00
Chris Aniszczyk f1946b0669 Cleaning up provider and feature names
It is incorrect to use Eclipse.org as the providerName now,
we'll use Eclipse JGit.

Change-Id: I1621b93d4f401176704e7c43935a5ce0c8ee8419
Signed-off-by: Chris Aniszczyk <caniszczyk@gmail.com>
2010-04-27 09:26:25 -05:00
Shawn O. Pearce 374c28057a Don't insert the same pack twice into a pack list
If a concurrent thread picks up a newly created PackFile and adds
it to the pack list before the IndexPack thread itself can insert
the item onto the front of the list, do nothing and use the item
that was picked up by that other concurrent scanning thread.

This avoids a potential condition where the same pack exists in
memory twice, which causes confusion later during a rescan of the
directory because we don't know exactly which PackFile instance
should be retained into the new list, and which should be discarded.

We can stop searching through the old pack list as soon as the
sort function declares that the item to insert should be before
the item already in the list.  Because the list is always sorted
by modification time (in seconds), we should never encounter a
case where the pack is positioned at the wrong spot in the list.
This early break out still permits an efficient implementation of
the common case, inserting a new pack at the head of the list.

Change-Id: Ice4459bbd4ee9487078aff5257893883d04f05fb
Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
2010-04-26 17:33:53 -07:00
Shawn O. Pearce a0a52897ed Favor earlier PackFile instances over later duplicates
There is a potential race condition during insertPack that can lead
to us having the same pack file open twice in the same directory.

A different thread can miss an object on disk, and trigger a scan
of the directory, and notice the pack that was put in by IndexPack.
So the pack winds up in the newly created PackList.

The IndexPack thread then wakes up and finishes its insertPack by
creating a new PackFile and inserting it into position 0 of the list.
We now have the same pack listed twice.

Readers will favor the earlier PackFile instance, because its the
first one they come across as they iterate through the list.

Keep that earlier one when we scan the pack directory again, as
this will avoid needing to purge out all of the windows that may
have been cached.

Of course we should also fix that race condition, but this block
was taking the wrong resolution if this error ever shows up, so
lets first fix the block to use a more sane resolution.

Change-Id: I0d339b9fd1dd8012e8fe5a564b893c0f69109e28
Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
2010-04-26 17:32:04 -07:00
Shawn O. Pearce eeed0abd16 Cleanup duplicated object reuse code in PackWriter
This reuse line was identical between the two branches related to
reusing a delta, or reusing a whole object.  Either way they reuse
the body of the object as-is.  So just make that a common function
after the header is written.

Change-Id: I0e6673b8e813c8c08c594ea2ba546fd366339d5d
Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
2010-04-26 17:29:10 -07:00
Robin Rosenberg 4ef96296f7 Merge "Fix NPE during InflaterCache return after corrupt loose object" 2010-04-24 08:19:01 -04:00
Shawn O. Pearce dafa8fbff4 Fix NPE during InflaterCache return after corrupt loose object
If a corrupt loose object is read, UnpackedObjectLoader was disposing
of the Inflater, and then attempting to return the disposed Inflater
to the InflaterCache.  Since the disposed Inflater had its native
libz resource deallocated and its reference cleared out, the Inflater
threw NullPointerException and refused to reset itself before being
put back into the cache.

Instead of disposing of the Inflater when corruption is found, do
nothing, and allow it to be returned to the cache.  The instance
will get reset, and should be usable by a future caller.

Bug: 310291
Change-Id: I44f2247c08b6e04fa62f8399609341b07508c096
Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
2010-04-23 11:16:25 -07:00
Shawn O. Pearce f36df5dc6a Merge branch 'receive-pack-filter'
* receive-pack-filter:
  ReceivePack: Clarify the check reachable option
  ReceivePack: Micro-optimize object lookup when checking connectivity
  ReceivePack: Correct type of not provided object
  IndexPack: Tighten up new and base object bookkeeping
  ReceivePack: Remove need new,base object id properties
  ReceivePack: Discard IndexPack as soon as possible
  ReceivePack: fix ensureProvidedObjectsVisible on thin packs

Change-Id: I4ef2fcb931f3219872e0519abfcee220191d5133
2010-04-19 18:20:42 -07:00
Matthias Sohn 9605fcc0fb Merge "ObjectIdSubclassMap: Correct Iterator to throw NoSuchElementException" 2010-04-17 18:35:38 -04:00
Matthias Sohn f1be93eb87 Merge "ObjectIdSubclassMap: Add isEmpty() method" 2010-04-17 18:29:16 -04:00
Robin Rosenberg c2960cdf65 Merge "IndexPack: Correct thin pack fix using less than 20 bytes" 2010-04-17 07:26:45 -04:00
Shawn O. Pearce 585dcb7a1c ReceivePack: Clarify the check reachable option
This option was mis-named from day 1.  Its not checking that the
objects provided by the client are reachable, its actually doing
a scan to prove that objects referenced by the client are already
reachable through another reference on the server, or were sent
as part of the pack from the client.

Rename it checkReferencedObjectsAreReachable, since we really are
trying to validate that objects referenced by the client's actions
are reachable to the client.

We also need to ensure we run checkConnectivity() anytime this is
enabled, even if the caller didn't turn on fsck for object formats.
Otherwise the check would be completely bypassed.

Change-Id: Ic352ddb0ca8464d407c6da5c83573093e018af19
Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
2010-04-16 17:04:38 -07:00
Shawn O. Pearce a770205070 ReceivePack: Micro-optimize object lookup when checking connectivity
If we are checking the visibility of everything referenced in the
pack that isn't already reachable by a reference, it needs to be
in the provided set.  Since the provided set lists everything that
is in this pack, we can avoid checking to see if the blob exists
on disk, because we know it should be there, it was found in the
pack we just consumed.

Change-Id: Ie3c7746f734d13077242100a68e048f1ac18c34a
Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
2010-04-16 17:04:38 -07:00
Shawn O. Pearce 6029bb24ad ReceivePack: Correct type of not provided object
If a tree was referenced but not provided in the pack, report it
as a missing tree and not as a missing blob.

Change-Id: Iab05705349cdf0d30cc3f8afc6698a8d2a941343
Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
2010-04-16 17:04:37 -07:00
Shawn O. Pearce 2bb8defa54 IndexPack: Tighten up new and base object bookkeeping
The only current consumer of these collections is ReceivePack,
where it needs to test ObjectId equality between a RevObject and an
ObjectId.  There we were copying from a traditional HashSet<ObjectId>
into an ObjectIdSubclassMap<ObjectId>, as the latter can perform
hashing using ObjectId's native value support, bypassing RevObject's
override on hashCode() and equals().  Instead of doing that copy,
directly create ObjectIdSubclassMap instances inside of ReceivePack.

We also only need to record the objects that do not appear in the
incoming pack, and were therefore copied from the local repositiory
in order to complete delta resolution.  Instead of listing everything
that used an OBJ_REF_DELTA format, list only the objects that we
pulled from the destination repository via a normal ObjectLoader.

ReceivePack can now discard the IndexPack object, and all of its
other data, as soon as these collections are held by the check
connectivity method.  This frees up memory for the ObjectWalk's
own RevObject pool.

Change-Id: I22ef71b45c2045a0202e7fd550a770ee1f6f38a6
Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
2010-04-16 17:04:26 -07:00
Shawn O. Pearce 329a0e1689 ReceivePack: Remove need new,base object id properties
These are more like internal implementation details of how IndexPack
works with ReceivePack to validate the incoming object stream.
Callers who are embedding the ReceivePack logic in their own
application don't really need to know the details of which objects
were used for delta bases in the incoming thin pack, or exactly
which objects were newly transmitted.

Hide these from the API, as exposing them through ReceivePack was
an early mistake.

Change-Id: I7ee44a314fa19e6a8520472ce05de92c324ad43e
Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
2010-04-16 16:32:33 -07:00
Shawn O. Pearce 8279361de8 ReceivePack: Discard IndexPack as soon as possible
The IndexPack object carries a good bit of state within itself about
the objects received over the wire.  The earlier we can discard it,
the sooner the GC is able to reclaim this chunk of memory for other
uses.  So drop it as soon as we are certain the pack is valid and we
have no connectivity concerns.

Change-Id: I1e8bc87c2e9183733043622237a064e55957891f
Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
2010-04-16 16:32:33 -07:00
Shawn O. Pearce 7a91b180c1 ReceivePack: fix ensureProvidedObjectsVisible on thin packs
If ensureProvidedObjectsVisible is enabled we expected any trees or
blobs directly reachable from an advertised reference to be marked
with UNINTERESTING.  Unfortunately ObjectWalk doesn't bother setting
this until the traversal is complete.  Even then it won't necessarily
set it on every tree if the corresponding commit wasn't popped.

When we are going to check the base objects for the received pack,
ensure the UNINTERESTING flag gets carried into every immediately
reachable tree or blob, because these are the ones that the client
might try to use as delta bases in a thin pack.

Change-Id: I5d5fdcf07e25ac9fc360e79a25dff491925e4101
Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
2010-04-16 16:32:23 -07:00
Shawn O. Pearce 466bec3cc9 ObjectIdSubclassMap: Correct Iterator to throw NoSuchElementException
The Iterator contract says next() shall throw NoSuchElementException
if there are no more items remaining in the iteration.  We got this
wrong when I originally wrote the implementation, so fix it.

Change-Id: Iea25e6569ead5c8b3128b8a368c5b2caebec7ecc
Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
2010-04-16 16:30:21 -07:00
Shawn O. Pearce 4cc7b1c5b0 ObjectIdSubclassMap: Add isEmpty() method
This class behaves like a cross between a Set and a Map, sometimes
we might expect to use the method isEmpty() to test for size() == 0.
So implement it, reducing the surprise folks get when they are given
one of these objects.

Change-Id: I0d68e1243da8e62edf79c6ba4fd925f643e80a88
Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
2010-04-16 16:30:21 -07:00
Shawn O. Pearce 06ee913c8d IndexPack: Correct thin pack fix using less than 20 bytes
If we need to append less than 20 bytes in order to fix a thin pack
and make it complete, we need to set the length of our file back to
the actual number of bytes used because the original SHA-1 footer was
not completely overwritten.  That extra data will confuse the header
and footer fixup logic when it tries to read to the end of the file.

This isn't a very common case to occur, which is why we've never
seen it before.  Getting a delta that requires a whole object which
uses less than 20 bytes in pack representation is really hard.
Generally a delta generator won't make these, because the delta
would be bigger than simply deflating the whole object.  I only
managed to do this with a hand-crafted pack file where a 1 byte
delta was pointed to a 1 byte whole object.

Normally we try really hard to avoid truncating, because its
typically not safe across network filesystems.  But the odds of
this occurring are very low.  This truncation is done on a file
we have open for writing, will append more content onto, and is
a temporary file that we won't move into position for others to
see until we've validated its SHA-1 is sane.  I don't think the
truncate on NFS issue is something we need to worry about here.

Change-Id: I102b9637dfd048dc833c050890d142f43c1e75ae
Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
2010-04-16 15:56:23 -07:00
Robin Rosenberg 6da38b9474 Optimize ref scanning
We can avoid one stat call by trying to perform a directory
listing without checking if the reference File is a directory.
Attempting a directory listing is defined to return. The other
case for null returns from list is when an I/O error occcurs.

Both cases are now intepreted as a possible plain reference. I/O
errors when reading plain references will be handled (ignored)
in scanRef().

Change-Id: I9906ed8c42eab4d6029c781aab87b3b07c1a1d2c
Signed-off-by: Robin Rosenberg <robin.rosenberg@dewire.com>
2010-04-13 23:00:53 +02:00
Matthias Sohn d29618dd41 Merge "Make Repository.getConfig aware of changed config" 2010-04-13 04:16:58 -04:00
Jens Baumgart cc905e7d4b Make Repository.getConfig aware of changed config
In the current implementation Repository reads user and repository 
config only at creation point of time.
The new implementatiopn checks in Repository.getConfig if user or 
repository config have changed on disk and reload the config if 
required. 

Change-Id: Ibd97515919ef66c6f8aa1a4fe8a11a6711335dad
Signed-off-by: Jens Baumgart <jens.baumgart@sap.com>
2010-04-12 11:48:45 +02:00
Shawn Pearce 333a0536a7 Merge "Speed up check for modifications of tracked resources" 2010-04-10 22:41:15 -04:00
Robin Rosenberg b919a94348 Speed up check for modifications of tracked resources
We only need to check file existense if some other stat returns
a value that may mean that the file does not exist. File.length() == 0
or File.lastModified() == 0 are two such properties. We use length
here.

Change-Id: If626b12e7bb4da994b5c086f6a5b7a12c187261c
Signed-off-by: Robin Rosenberg <robin.rosenberg@dewire.com>
2010-04-10 17:17:35 +02:00
Robin Rosenberg fa4c3fe461 JGit plugin not compatible with Eclipse 3.4
The JSch bundle in Eclipse 3.4 does not export its packages with
version numbers. Use Require-Bundle on version 0.1.37 that comes
with Eclipse 3.4

There is no 0.1.37 in the maven repositories so the pom still refers
to 0.1.41 so the build can get the compile time dependencies right.

Bug: 308031
CQ: 3904 jsch Version: 0.1.37 (using Orbit CQ2014)

Change-Id: I12eba86bfbe584560c213882ebba58bf1f9fa0c1
Signed-off-by: Robin Rosenberg <robin.rosenberg@dewire.com>
2010-04-05 12:25:06 +02:00
Marc Strapetz 2b6c555aee Make parsing of PersonIdent from raw byte array fault-tolerant.
RawParseUtils.parsePersonIdent handles now those invalid byte sequences
which would result in IndexOutOfBoundsException and returns null in this
case.
2010-03-23 09:21:18 +01:00
Shawn O. Pearce 8014dbe9bf Merge branch 'stable-0.7'
* stable-0.7:
  Fix EGit deadlock listing branches of SSH remote
2010-03-22 08:20:39 -07:00
Shawn O. Pearce 0dc93a2f4a Fix EGit deadlock listing branches of SSH remote
When listing branches, EGit only reads the advertisement and
then disconnects.  When it closes down the pack channel the remote
side is waiting for the client to send our list of commands, or a
flush-pkt to let it know there is nothing to do.

However if an error thread is open watching the SSH stderr stream,
we ask for it to finish before we send the flush-pkt.  Unfortunately
the thread won't terminate until the main output stream closes,
which is waiting for the flush-pkt.  A classic network deadlock.

If the output stream needs a flush-pkt we send it before we wait
for the error stream to close.  If the flush-pkt is rejected, we
close down the output stream early, assuming that the remote side
is broken and we will get error information soon.

Change-Id: I8d078a339077756220c113f49d206b1bf295d434
Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
2010-03-20 19:18:14 -07:00
Shawn O. Pearce 14e469c44e Qualify builds as 0.8.0
Since the API is changing relative to 0.7.0, we'll call our next
release 0.8.1.  But until that gets released, builds from master
will be 0.8.0.qualifier.

Change-Id: I921e984f51ce498610c09e0db21be72a533fee88
Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
2010-03-20 19:06:58 -07:00
Shawn O. Pearce 7182fbc422 Qualify post-0.7.0 builds
Change-Id: I5afdc624b28fab37b28dd2cc71d334198672eef3
Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
2010-03-20 18:58:37 -07:00
Shawn O. Pearce 72b8fa786a JGit 0.7.0
Change-Id: I9b00a4041c19115e81326afd2213b98603f789ad
2010-03-18 19:31:23 -07:00
Nico Sallembien 0f95d2d046 Add a paranoid 'must be provided' option to ReceivePack
By default a receive pack assumes that its user will only provide
references to objects that the user already has access to on their
local client.  In certain cases, an additional check to verify the
references point only to reachable objects is necessary.

This additional checking is useful when the code doesn't trust
the client not to provide a forged SHA-1 reference to an object,
in an attempt to access parts of the DAG that they weren't allowed
to see by the configured RefFilter.

Change-Id: I3e4b8505cb2992e3e4be253abb14a1501e47b970
Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
2010-03-18 11:37:59 -07:00
Shawn O. Pearce 6fabb6d204 Merge branch 'stable-0.7'
* stable-0.7:
  Reuse the line buffer between strings in PacketLineIn
  http.server: Use TemporaryBuffer and compress some responses
  Reduce multi-level buffered streams in transport code
  Fix smart HTTP client buffer alignment
  Use "ERR message" for early ReceivePack problems
  Catch and report "ERR message" during remote advertisements
  Wait for EOF on stderr before finishing SSH channel
  Capture non-progress side band #2 messages and put in result
  ReceivePack: Enable side-band-64k capability for status reports
  Use more restrictive patterns for sideband progress scraping
  Prefix remote progress tasks with "remote: "
  Decode side-band channel number as unsigned integer
  Refactor SideBandInputStream construction
  Refactor SideBandOutputStream to be buffered
2010-03-12 17:04:48 -08:00
Shawn O. Pearce 23bd331cb2 Merge branch 'push-sideband' into stable-0.7
* push-sideband:
  Reuse the line buffer between strings in PacketLineIn
  http.server: Use TemporaryBuffer and compress some responses
  Reduce multi-level buffered streams in transport code
  Fix smart HTTP client buffer alignment
  Use "ERR message" for early ReceivePack problems
  Catch and report "ERR message" during remote advertisements
  Wait for EOF on stderr before finishing SSH channel
  Capture non-progress side band #2 messages and put in result
  ReceivePack: Enable side-band-64k capability for status reports
  Use more restrictive patterns for sideband progress scraping
  Prefix remote progress tasks with "remote: "
  Decode side-band channel number as unsigned integer
  Refactor SideBandInputStream construction
  Refactor SideBandOutputStream to be buffered

Change-Id: Ic9689e64e8c87971f2fd402cb619082309d5587f
2010-03-12 17:00:54 -08:00
Shawn O. Pearce 89cdc3b713 Reuse the line buffer between strings in PacketLineIn
When reading pkt-lines off an InputStream we are quite likely to
consume a whole group of fairly short lines in rapid succession, such
as in the have exchange that occurs in the fetch-pack/upload-pack
protocol.  Rather than allocating a throwaway buffer for each
line's raw byte sequence, reuse a buffer that is equal to the small
side-band packet size, which is 1000 bytes.  Text based pkt-lines
are required to be less than this size because many widely deployed
versions of C Git use a statically allocated array of this length.

Change-Id: Ia5c8e95b85020f7f80b6d269dda5059b092d274d
Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
2010-03-12 16:10:30 -08:00
Shawn O. Pearce 2156aa894c Reduce multi-level buffered streams in transport code
Some transports actually provide stream buffering on their own,
without needing to be wrapped up inside of a BufferedInputStream in
order to smooth out system calls to read or write.  A great example
of this is the JSch SSH client, or the Apache MINA SSHD server.
Both use custom buffering to packetize the streams into the encrypted
SSH channel, and wrapping them up inside of a BufferedInputStream
or BufferedOutputStream is relatively pointless.

Our SideBandOutputStream implementation also provides some fairly
large buffering, equal to one complete side-band packet on the main
data channel.  Wrapping that inside of a BufferedOutputStream just to
smooth out small writes from PackWriter causes extra data copies, and
provides no advantage.  We can save some memory and some CPU cycles
by letting PackWriter dump directly into the SideBandOutputStream's
internal buffer array.

Instead we push the buffering streams down to be as close to the
network socket (or operating system pipe) as possible.  This allows
us to smooth out the smaller reads/writes from pkt-line messages
during advertisement and negotation, but avoid copying altogether
when the stream switches to larger writes over a side band channel.

Change-Id: I2f6f16caee64783c77d3dd1b2a41b3cc0c64c159
Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
2010-03-12 16:08:14 -08:00
Shawn O. Pearce 882d03f70e Fix smart HTTP client buffer alignment
This proved to be a pretty difficult to find bug.  If we read exactly
the number of response bytes from the UnionInputStream and didn't
try to read beyond that length, the last connection's InputStream is
still inside of the UnionInputStream, and UnionInputStream.isEmpty()
returns false.  But there is no data present, so the next read
request to our UnionInputStream returns EOF at a point where the
HTTP client code should have started a new request in order to get
more data.

Instead of wrapping the UnionInputStream, push an dummy stream onto
the end of it which when invoked always starts the next request and
then returns EOF.  The UnionInputStream will automatically pop that
dummy stream out, and then read the next request's stream.

This way we never get into the state where we don't think we need
to run another request in order to satisfy the current read request,
but we really do.

The bug was hidden for so long because BasePackConnection.init()
was always wrapping the InputStream into a BufferedInputStream
with an 8 KiB buffer.  This made the odds of us reading from the
UnionInputStream the exact number of available bytes quite low, as
the BufferedInputStream would always try to read a full buffer size.

Change-Id: I02b5ec3ef6853688687d91de000a5fbe2354915d
Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
2010-03-12 16:08:14 -08:00
Shawn O. Pearce d8c3e98d73 Use "ERR message" for early ReceivePack problems
If the application wants to, it can use sendError(String) to send one
or more error messages to clients before the advertisements are sent.
These will cause a C Git client to break out of the advertisement
parsing loop, display "remote error: message\n", and terminate.

Servers can optionally use this to send a detailed error to a client
explaining why it cannot use the ReceivePack service on a repository.
Over smart HTTP these errors are sent in a 200 OK response, and
are in the payload, allowing the Git client to give the end-user
the custom message rather than the generic error "403 Forbidden".

Change-Id: I03f4345183765d21002118617174c77f71427b5a
Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
2010-03-12 16:08:14 -08:00
Shawn O. Pearce 1f4a30b80d Catch and report "ERR message" during remote advertisements
GitHub broke the native git protocol a while ago by interjecting an
"ERR message" line into the upload-pack or receive-pack advertisement
list.  This didn't match the expected pattern, so it caused existing
C Git clients to abort with a protocol exception.

These days, C Git clients actually look for this message and abort
with a more graceful notice to the end-user.  JGit should do the
same, including setting up a custom exception type that makes it
easier for higher-level UIs to identify a message from the remote
site and present it to the user.

Change-Id: I51ab62a382cfaf1082210e8bfaa69506fd0d9786
Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
2010-03-12 16:08:14 -08:00