Feature Request: Maven Artifact Support following PDE

[SuperOok]

Very recently, the m2 Team has released a new feature for PDE to support also Maven artifacts in PDE target platforms. This holds both, for builds in Eclipse PDE and for Tycho.

Here is a detailed article about this new feature: https://läubisoft.gmbh/en/articles/using-maven-artifacts-in-pde-rcp-and-tycho-builds/

It would be great, if this feature would also be available in the target DSL, i.e. it would be required to specify maven dependencies in the text specification and transform them to the corresponding target specification as stated in the article.

[mbarbero]

Thanks for reporting this. We welcome patches to add support for such a feature.

[hannesN]

I'd like to implement a solution: My dsl suggestion would be something like this:

mvn location=https://optionally/path/to/repo {

groupid:artifactId:version ... }

I propose to implement the dsl changes and the generator first and provide some content assist in a follow-up ticket.

What dou you think?

[merks]

Certainly that will create dependencies on m2e which is fine, though everyone will get those dependencies. I trust that there are actual APIs that can be reused for this TPD implementation, not just "implementation classes" that can arbitrarily change from release to release. Given all the thumbs up, I expect a lot of people will appreciate such a feature!

There are quite a few configuration options that need consideration:

Feature generation is also cool:

<?xml version="1.0" encoding="UTF-8" standalone="no"?>
<?pde version="3.8"?>
<target name="Maven" sequenceNumber="1">
    <locations>
        <location includeDependencyDepth="none" includeDependencyScopes="compile" includeSource="true" missingManifest="error" type="Maven">
            <feature id="org.eclipse.oomph.maven.all" label="Oomph Maven All" version="1.0.0.qualifier">
                <description url="http://www.example.com/description"></description>
                <copyright url="http://www.example.com/copyright"></copyright>
                <license url="http://www.example.com/license"></license>
            </feature>
            <dependencies>
                <dependency>
                    <groupId>org.ow2.asm</groupId>
                    <artifactId>asm-analysis</artifactId>
                    <version>9.3</version>
                    <type>jar</type>
                </dependency>
                <dependency>
                    <groupId>org.ow2.asm</groupId>
                    <artifactId>asm</artifactId>
                    <version>9.3</version>
                    <type>jar</type>
                </dependency>
                <dependency>
                    <groupId>org.ow2.asm</groupId>
                    <artifactId>asm-tree</artifactId>
                    <version>9.3</version>
                    <type>jar</type>
                </dependency>
            </dependencies>
        </location>
    </locations>
</target>

[laeubi]

My dsl suggestion would be something like this:

please note that there are probably many repositories one can specify, also note that there are other options as well for a location as @merks pointed out.

Essentially one can construct MavenTargetLocation and then call serialize()

[hannesN]

I implemented the following syntax:

Which generates the correct target definition:

So before I start writing test and formatting code, I want to know, what do you think.

[hannesN]

If you want to try it out, I am pushing the current state into my fork at https://github.com/hannesn/targetplatform-dsl/

[laeubi]

@hannesN maybe instead of maven_location use simply maven but in general it looks good to me (even though I not using the target dsl). Also the term "artifact" is not correct I think it should more be "dependency"... an artifact is something that maps to a dependency. So if you use naming that maps to the maven xml syntax I think it would make it easier for people to use.

[hannesN]

Thanks for the feedback. Both of your suggestions make sense. I will change that.

[merks]

FYI, I've noticed that in general a dependency can include BND instructions:

https://github.com/eclipse/tm4e/blob/f797d2c6632f5639f32604950e41f02a8664acc0/target-platform/tm4e-target.target#L44-L52

I don't think the dialog provides a field for entering that information though so this is easily overlooked. I'm not say you must/should support this, it's just an FYI...

Do you plan to flesh out the feature support for the description, copyright, and license, including their URLs?

[laeubi]

I don't think the dialog provides a field for entering that information though so this is easily overlooked.

[merks]

That's cool. 😃 I easily overlooked that. 👅

It's perhaps overkill to try to design a bunch of grammar for this level of BND detail...

[laeubi]

m2e just handle this a big fat text-block (aka CDATA) ... The same for feature data...

[hannesN]

Yeah I'd like to keep it as simple as possible for now. I am also not planning to resolve/check whether the dependencies can be resolved, although I wrote the code in a way it could be added later.

For now, I suggest a minimum of functionality, which is basically seen in the screenshots above. Formatting will be added, but that would it be. Added more functions should be a new issue if it is really needed.

Regarding the feature descriptions: I don't think those are editable in the dialog as well.

[laeubi]

You can edit the feature in the target-source tab... as this is not an option for the dsl, I would recommend to either have a simple textblock that is XML ... or one would need a feature-dsl as well.

[HannesWell]

Your proposal looks good so far. 👍🏽

Essentially one can construct MavenTargetLocation and then call serialize()

That class is not public API, it is just exported for the pde.ui and marked as 'internal' and IIRC we treated it as such in M2E in the past. I didn't check if you are using that, I just wanted to make aware of that.

[laeubi]

That class is not public API, it is just exported for the pde.ui and marked as 'internal' and IIRC we treated it as such in M2E in the past. I didn't check if you are using that, I just wanted to make aware of that.

Not API of what? Everything regarding targets is actually "not API" and constructing an XML by hand is also "not API", so any construct that wants to build on top of m2e /be compatible has to deal with adjustments in that area and be aware that it is closely coupled to possible changes in that area. So from my side as the "non-api-designer" I can say tha this is the most closest thing one can use to replicate the xml and/or benefit from bugfixes and getting compile error as soon as we change/adjust something ;-)

[guw]

Is it possible to re-enable the nightly builds? I'd like to help testing this. https://download.eclipse.org/cbi/updates/tpd/nightly/latest/index.html

[merks]

I just did a build a few minutes ago.

Sorry, I think I did some builds without promoting them, but mistakenly assumed that I had promoted the changes for this issue's PR. So the following build and all any subsequent builds do/will contain these changes:

https://download.eclipse.org/cbi/updates/tpd/nightly/latest

Apologies for the confusion!

[guw]

@hannesN Got it working for a simple file. However, I noticed that it does not work when the Maven tpd file is included from another tpd file. In this case it generates a target file without the Maven location.

[hannesN]

I'll look into it, @guw .

[hannesN]

I created a new issue for included files. I thought this one is already closed by the MR. See #127

I already started working on a solution.

[hannesN]

I created the PR #128

[hannesN]

Should we close this issue?

[merks]

@hannesN Yes, I think you've complete this support and if folks find problems or want additional not-yet-implemented features a new dedicated issue should be opened for it.

Thanks for all your excellent work in this area! 🥇

[guw]

@hannesN Do you remember if the support for custom bnd instructions was implemented? I checked the README but it's not documented.

[hannesN]

Well, as I am not sure what you mean with that, I guess I didn't implement it.

[guw]

@hannesN bnd instructions is the thing you and Ed are discussing here:

            <instructions><![CDATA[
Bundle-Name:           Bundle derived from maven artifact ${mvnGroupId}:${mvnArtifactId}:${mvnVersion}
version:               ${version_cleanup;${mvnVersion}}
Bundle-SymbolicName:   org.${mvnArtifactId}
Bundle-Version:        ${version}
Import-Package:        *;resolution:=optional
Export-Package:        *;version="${version}";-noimport:=true
DynamicImport-Package: *
]]></instructions>

[merks]

I also mentioned this in this issue above:

https://github.com/eclipse-cbi/targetplatform-dsl/issues/115#issuecomment-1361235931

[hannesN]

@guw Could you open a new issue? I'll try to find some time on Friday to investigate a possible solution.