Skip to main content

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] [List Home]
[jgit-dev] [VOTE] Making API incompatible changes

https://git.eclipse.org/r/12422 makes an incompatible API change by
adding a new abstract method to ObjectInserter.

Since its an API break, we would need to bump the next version to 4.0.
However master will deliver to the Kepler stream until next June.
Which means we need to wait 1 year to make this change.

I'm not going to wait a year to add a method to an abstract class that
99.99999% of consuming applications will never extend. I see a few
options:

Option 1)  Google forks JGit.

  We fork. We host the fork at gerrit-review. The fork has the change.
  Our development work continues at gerrit-review.

Option 2)  Bump version to 4.0

  Master jumps to 4.0. We (maybe) keep a kepler branch that will be
  used to deliver to the kepler stream through next June.

Option 3)  Concrete method throws UnsupportedOperationException

  Proposed in comments. The new method throws an exception unless
  the subclass "correctly" knows it should override the method with an
  implementation. To an API checking utility its not an API breakage,
  but now implementors (which should be 0!) do not know they need to
  override the method for correct behavior from JGit.

Option 4)  Stupid API workarounds

  Instead of saying inserter.newReader() we implement a bridge:

    class ObjectReader {
      public static ObjectReader newReader(ObjectInserter ins) {
        if (ins instanceof ObjectInserter2) {
          return ((ObjectInserter2)ins).newReader();
        }
        throw new UnsupportedOperationException();
      }
    }

    package org.eclipse.jgit.lib.internal;

    abstract class ObjectInserter2 extends ObjectInserter

  Like option 3 implementors of ObjectInserter do not know they
  need to implement ObjectInserter2. But now we have a pattern
  we can use to make additional API changes to ObjectInserter,
  if they ever arise.

  I hate this option because it hides a useful function of one type
  (ObjectInserter) inside of a static method of another type. Bleh.
  Another place might be to try and do this on Repository, but its
  the same sort of ugly.


12422 is needed to improve write performance or our hosted Gerrit
solution on a JGit DFS backend. Since Dave Borowitz has most of the
work done we don't want to wait until next June to land the changes.

Similarly Jonathan Neider wants to do some refactoring on the new
archive implementations to support additional use cases, like
combining multiple repositories into a single archive stream, or
annotating archives with their MIME types. So this issue of API
breaking changes is going to crop up again within the next 12 months.


Back to the top