Re: [jgit-dev] NIO2 JGit Implementation

[Alexandre Porcelli Bakos <porcelli@xxxxxxxxxx>]

Byteman:
https://search.maven.org/artifact/org.jboss.byteman/byteman-bmunit/4.0.6/jar
https://search.maven.org/artifact/org.jboss.byteman/byteman-rulecheck-maven-plugin/4.0.6/maven-plugin

AssertJ:
https://search.maven.org/artifact/org.assertj/assertj-core/3.14.0/bundle

Regards,
---
Alex Porcelli
Business Automation Architect
Red Hat Business Systems and Intelligence Group

On Tue, Jan 14, 2020 at 8:03 PM Matthias Sohn <matthias.sohn@xxxxxxxxx> wrote:
>
> On Wed, Jan 15, 2020 at 2:01 AM Matthias Sohn <matthias.sohn@xxxxxxxxx> wrote:
>>
>> On Tue, Jan 14, 2020 at 6:19 PM Alexandre Porcelli Bakos <porcelli@xxxxxxxxxx> wrote:
>>>
>>> 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 ?
>
>
> https://search.maven.org/search?q=byteman
>
>>
>> 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
>>>