Matthias,
Thank you very much for your review, I'm completely fine with your
proposal. Would be great if would be possible to keep the testing
libraries, byteman is an incredible tool to test concurrency and some
potential racing issues.
I'll check with the Eclipse legal team if we can use it for testing.
Which of the byteman jars do we need on the classpath to build and run the tests ?
We'll have to add them to Orbit so that we can also run those tests from PDE in Eclipse
using the JGit target platform (defining the OSGi classpath).
The version published that you tried still needs some work, especially
the public API that I just rushed to show how to use the proposal. If
you could share your changes I could pick up from there.
Regards,
---
Alex Porcelli
Business Automation Architect
Red Hat Business Systems and Intelligence Group
On Fri, Jan 10, 2020 at 6:59 PM Matthias Sohn <matthias.sohn@xxxxxxxxx> wrote:
>
> On Sat, Nov 16, 2019 at 12:30 AM Alexandre Porcelli Bakos <porcelli@xxxxxxxxxx> wrote:
>>
>> Hi All,
>>
>> A few years ago, I mentioned some work that my team in Red Hat has
>> been executing to create a NIO2 JGit implementation. Back in time,
>> there was some interest. However, we never had the time to isolate it
>> from our project codebase.
>>
>> Finally, during the Gerrit/JGit Hackathon in Sunnyvale, I invested the
>> time to do it, and finally, I made it completely isolated [1]!
>>
>> There's still some work to be done to flexible the architecture,
>> especially around Daemons and Security. However, I'd like to get some
>> input from you if they're still interested in having this component to
>> JGit as a separate bundle (jar).
>>
>> Please let me know what you think and, if it's the case, how to
>> proceed from here.
>>
>> [1] - https://github.com/porcelli/jgit-nio2
>>
>> Regards,
>> ---
>> Alex Porcelli
>> Business Automation Architect
>> Red Hat Business Systems and Intelligence Group
>
>
>
> First of all thanks for this proposal. After meeting at the hackathon I unfortunately didn't find time
> to look at the implementation in detail.
>
> Now in the new year I started to look into jgit-nio2.
>
> Here some comments
>
> License
> JGit is licensed under EDL 1.0 (Eclipse variant of the BSD 3-clause license).
>
> AFAIR you said you can relicense the jgit-nio2 source code from Apache 2.0 to EDL 1.0.
> Can you confirm that ?
>
> Dependencies
> jsch
> As you probably noticed jgit is moving from jsch to the Apache mina sshd client.
> We still support both but the long-term goal is to get rid of jsch since there is no public repository
> only the source archives available in Maven central which do not contain automated tests
> and it's maintained by a single developer who doesn't want to accept contributions.
>
> byteman
> Used in tests and licensed under LGPL. We need to check with the Eclipse legal team
> if this license is ok for test dependencies. I can take care of that. Otherwise we have
> to get rid of the dependency to byteman.
>
>
> OSGi
> All artefacts in JGit are implemented in a way so that they can be run in plain Java runtime
> or in OSGi runtime which is required e.g. for running them in Eclipse.
>
> The productive code and the corresponding tests are split into two different projects/artefacts
> since we need to separate the production code from test code also in OSGi runtime
> hence there's a production OSGi bundle and a test OSGi bundle or OSGi fragment.
>
> I propose we add 2 new projects/artefacts for your contribution:
> org.eclipse.jgit.niofs
> org.eclipse.jgit.niofs.test
> to enable consumption from OSGi/Eclipse
>
> Build
> We check-in Eclipse projects and settings and do double maintenance of dependencies
> in pom.xml (for the Maven build) and OSGI manifests (for OSGi runtime and for compilation
> in Eclipse). In addition there are Eclipse features and a p2 repository for installation into Eclipse.
>
> In addition there's a (partial) bazel build for the artefacts used in Gerrit which consumes
> JGit via a git submodule and integrates the JGit bazel build with the Gerrit bazel build
> so that both projects can be compiled and tested within the same bazel build.
>
> APIs
> We follow OSGi semantic versioning rules [1]. Basically this means service releases (third digit of version
> increases) are for bug fixes, minor releases add new features (second digit of version increases),
> major releases can break APIs. The rule for breaking changes differentiates between
> - breaking clients of the API is only ok in major releases
> - breaking implementers is also ok in minor releases
>
> We separate API and implementation into different packages, internal implementation classes
> are in internal packages e.g. org.eclipse.jgit.transport.internal which we do not guarantee
> to keep stable across releases.
> This separation helps in OSGi environment where dependencies are typically expressed
> using package import statements in the corresponding OSGi manifest.
>
> We use Eclipse API tools to ensure we don't break public API across service and minor releases.
>
> For public API we want complete javadoc.
>
> Code format
> We use the Eclipse built-in code formatter with save rules configured in project settings to
> auto-apply formatting rules when modified code is saved. So far we use tabs for indentation
> and a max line width of 80 characters.
>
> PoC integration into JGit
> I went ahead and spent a couple of hours on your project to bring it into jgit as 2 new
> projects as proposed above and adapt the build to the JGit Maven build and adapted
> versions of common dependencies to those used in JGit elsewhere.
>
> I renamed packages and tried to split them into API and internal packages, though probably
> my guess which packages are internal is not yet correct ;-)
>
>
> sun.reflect.generics.reflectiveObjects.NotImplementedException
>
> I think it's not a good idea to depend on any internal classes in sun.* packages
>
> hence for now I replaced it with UnsupportedOperationException
>
>
> I had to declare ReceivePack.filterCommands() and executeCommands() protected
> so that niofs can overwrite them.
>
> org.eclipse.jgit.niofs already compiles in Eclipse and Maven.
>
> Running the tests org.eclipse.jgit.niofs.test from Maven still has many errors
> which are probably caused by my missing understanding about how the tests work.
> For compiling the tests in Eclipse we need to add assertj and byteman to Eclipse Orbit to
> make them consumable as OSGi bundles via target platform in Eclipse.
> Prerequisite is license approval for these 2 libraries.
>
> If you agree to this approach I can push these changes to the JGit project for review and
> we can continue discussion on the changes in review.
>
> I propose we then iterate over these changes until tests pass and we think it's good enough
> to accept the initial contribution into JGit.
>
> In parallel I can care about getting approval for the initial contribution and the
> new additional dependencies niofs brings.
>
> [1] https://www.osgi.org/wp-content/uploads/SemanticVersioning.pdf
>
> -Matthias
