Re: [jgit-dev] NIO2 JGit Implementation

[Alexandre Porcelli Bakos <porcelli@xxxxxxxxxx>]

Thank you, Mathias!

Please ask the 4.0.6 as it looks like there's a bug in the recent
versions that fail to execute as expected. And I'm still waiting for
your local changes so I can start from there some additional work
needed - especially on the public API side.

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

On Wed, Jan 15, 2020 at 11:45 AM Matthias Sohn <matthias.sohn@xxxxxxxxx> wrote:
>
> On Wed, Jan 15, 2020 at 3:26 PM Alexandre Porcelli Bakos <porcelli@xxxxxxxxxx> wrote:
>>
>> Byteman:
>> https://search.maven.org/artifact/org.jboss.byteman/byteman-bmunit/4.0.6/jar
>
>
> Ok, I'll request the latest version 4.0.9.
>
> We also need to add its mandatory runtime dependencies to Orbit to enable running these tests from Eclipse:
> org.jboss.byteman:byteman:jar:4.0.9:compile
> org.jboss.byteman:byteman-submit:jar:4.0.9:compile
> org.jboss.byteman:byteman-install:jar:4.0.9:compile
>
> $ mvn dependency:tree -Dverbose -f ~/.m2/repository/org/jboss/byteman/byteman-bmunit/4.0.9/byteman-bmunit-4.0.9.pom
> [INFO] --- maven-dependency-plugin:2.8:tree (default-cli) @ byteman-bmunit ---
> [INFO] org.jboss.byteman:byteman-bmunit:jar:4.0.9
> [INFO] +- org.testng:testng:jar:6.8.5:compile
> [INFO] |  +- (junit:junit:jar:4.8.2:compile - version managed from 4.10; omitted for duplicate)
> [INFO] |  +- org.beanshell:bsh:jar:2.0b4:compile
> [INFO] |  +- com.beust:jcommander:jar:1.27:compile
> [INFO] |  \- org.yaml:snakeyaml:jar:1.6:compile
> [INFO] +- junit:junit:jar:4.8.2:compile
> [INFO] +- org.jboss.byteman:byteman:jar:4.0.9:compile
> [INFO] +- org.jboss.byteman:byteman-submit:jar:4.0.9:compile
> [INFO] +- org.jboss.byteman:byteman-install:jar:4.0.9:compile
> [INFO] |  \- (com.sun:tools:jar:1.6:system - omitted for duplicate)
> [INFO] \- com.sun:tools:jar:1.6:system
>
>>
>> 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
>
>
> will request this as well
>>
>> 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
>> >>>
>>