Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Deploy PDE artifacts to maven #936

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

laeubi
Copy link
Contributor

@laeubi laeubi commented Nov 27, 2023

As an alternative to

I think we maybe better should go the path of deploy PDE directly to the maven where we can consume than with maven target locations instead of create a new update site.

This has the following benefits:

  1. No need to setup a download space and if we later decide its not good at all the snapshots get deleted after a while automatically
  2. We can get more experience with this way of deployment and how it works out so PDE can become a blueprint for other projects to do so as well
  3. This could be a first step to make it possible to release artifacts to central without the help of global deploy script
  4. It would allow to see how deploying features to a maven repository works out

If this is in place, we can start to iron out the produced poms and start trying to strip PDE form the aggregator and instead consume it from the target.

FYI @HannesWell @akurtakov @mickaelistria @vogella

@akurtakov
Copy link
Member

akurtakov commented Nov 27, 2023

I fully support deploying to maven.
Having p2 repo is still mandatory IMO but one shouldn't block the other.

@laeubi
Copy link
Contributor Author

laeubi commented Nov 27, 2023

Having p2 repo is still mandatory IMO but one shouldn't block the other.

You can have a p2 repo on maven as well, see https://github.com/eclipse-tycho/tycho/tree/master/demo/p2-maven-site

Beside that I think Eclipse/IBuild/SDK/Simrel/... will still include PDE anyways one way or another.

@akurtakov
Copy link
Member

Having p2 repo is still mandatory IMO but one shouldn't block the other.

You can have a p2 repo on maven as well, see https://github.com/eclipse-tycho/tycho/tree/master/demo/p2-maven-site

Beside that I think Eclipse/IBuild/SDK/Simrel/... will still include PDE anyways one way or another.

So if SDK/I-builds want to include PDE without building it, you propose adding it as maven dependency. Can we say "latest" in maven deps somehow as I don't envision editing target file to pick latest PDE?

@laeubi
Copy link
Contributor Author

laeubi commented Nov 27, 2023

So if SDK/I-builds want to include PDE without building it, you propose adding it as maven dependency

Yes

Can we say "latest" in maven deps somehow

Yes but it is deprecated.

I don't envision editing target file to pick latest PDE?

In the end PDE becomes a third party dep and we do updates for these as well, at best there the just one feature mentioned and one might even use a version range there, that especially the things we need to explore here how it works out and where we need better support or additional features to support such use-cases, but I strongly believe that platform should be leading here even though I know others are already using such setups.

Copy link

github-actions bot commented Nov 27, 2023

Test Results

     270 files  ±0       270 suites  ±0   46m 38s ⏱️ + 3m 52s
  3 327 tests ±0    3 297 ✔️ ±0  30 💤 ±0  0 ±0 
10 278 runs  ±0  10 188 ✔️ ±0  90 💤 ±0  0 ±0 

Results for commit 0ddb87f. ± Comparison against base commit 3a696fb.

♻️ This comment has been updated with latest results.

@laeubi
Copy link
Contributor Author

laeubi commented Nov 27, 2023

by the way I created https://gitlab.eclipse.org/eclipsefdn/helpdesk/-/issues/3993 as it seems PDE is suffering build executors...

@laeubi laeubi force-pushed the deploy_to_maven branch 5 times, most recently from b216b17 to d34b33e Compare November 27, 2023 09:03
@vogella
Copy link
Contributor

vogella commented Nov 27, 2023

I fully support this approach, moving to a pure Maven build is also what a lot of my clients desire and if we use this approach it will help to stabilise the process. P2 update sites are often considered as "extra" or "Eclipse only" technologies by my clients which normally also have a lot of "regular" Maven projects and Maven knowledge.

@laeubi
Copy link
Contributor Author

laeubi commented Nov 27, 2023

It seems some plugins use an older deploy plugin I have no clue why... will try to move this up otherwhise have to wait until master opens and maybe define it in the parent...

@akurtakov
Copy link
Member

It seems some plugins use an older deploy plugin I have no clue why... will try to move this up otherwhise have to wait until master opens and maybe define it in the parent...

Please put it in the parent now. It's better to push such change now rather than multiplying it.

Copy link
Member

@HannesWell HannesWell left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I fully support deploy PDE to a Maven-repo or maven central in a more direct and simple way and I'm happy to use it as a play ground to enhance that method further.

Nevertheless I also think that PDE should still also be provided through a p2 repo.
While I understand that some may like to get it through Maven I also assume that there is also a large user base that would like to obtain it through p2 as it is now.
But luckily we can do both so there is no need to choose. :)

It is to be discussed if PDE then gets its own p2-repo or will just continue to be provided through the existing eclipse/updates p2-repo.

And although it is actually not directly related to this PR and should be more discussed as part of eclipse-platform/eclipse.platform.releng.aggregator#1472: Even if PDE gets it own build there should be frequent builds of PDE against the latest versions of the other parts of the SDK, just like it is done at the moment with the I-builds. If PDE is still build as part of the nightly builds or a master build is triggered after a I-build, doesn't really matter. But I think it is important to have that since PDE usues many parts of other SDK areas that are not public API and may change at any time and we will only notice required actions quickly if we have frequent builds.

Comment on lines +125 to +128
<pluginManagement>
<plugins>
</plugins>
</pluginManagement>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks unnecessary.

Suggested change
<pluginManagement>
<plugins>
</plugins>
</pluginManagement>

stages {
stage('Build') {
steps {
wrap([$class: 'Xvnc', useXauthority: true]) {
sh '''
mvn clean verify --batch-mode -Dmaven.repo.local=$WORKSPACE/.m2/repository \
mvn clean ${MVN_GOALS} --batch-mode -Dmaven.repo.local=$WORKSPACE/.m2/repository \
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you convert this text-block to a GString block (i.e. by replacing leading and trailing ''' by """) you can probably inline the function call and save the indirection through the environment variable.

Suggested change
mvn clean ${MVN_GOALS} --batch-mode -Dmaven.repo.local=$WORKSPACE/.m2/repository \
mvn clean ${getMavenGoals()} --batch-mode -Dmaven.repo.local=$WORKSPACE/.m2/repository \

Comment on lines +119 to +123
<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-deploy-plugin</artifactId>
<version>3.1.1</version>
</plugin>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't this unnecessary with eclipse-platform/eclipse.platform.releng.aggregator#1582?

Suggested change
<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-deploy-plugin</artifactId>
<version>3.1.1</version>
</plugin>

@mickaelistria
Copy link
Contributor

I also fully support deploying to Maven.
Just to make sure, are p2 metdata published with the artifacts? I think they must be published in general to handle cases where some p2.inf matters in the output installation.

@mickaelistria
Copy link
Contributor

I think they must be published in general to handle cases where some p2.inf matters in the output installation.

Actually, if the p2.inf is included in the artifact and is honored later by the PDE target resolution, and by Tycho target resolution and by p2-maven-site, there seems to be no need to store them in Maven repo as they can be inferred later.
@laeubi Can you confirm that embedded p2.inf are honored by those consumers?

@laeubi
Copy link
Contributor Author

laeubi commented Nov 28, 2023

Just to make sure, are p2 metdata published with the artifacts? I think they must be published in general to handle cases where some p2.inf matters in the output installation.

p2 metadata is attached to the project and would then be published along with the artifact

Actually, if the p2.inf is included in the artifact and is honored later by the PDE target resolution, and by Tycho target resolution and by p2-maven-site

p2.inf is only considered at build time but not later on, but p2-maven-site will contain it as it is a regular p2 site just deployed at maven repository instead of http/ftp/....

@mickaelistria
Copy link
Contributor

p2.inf is only considered at build time but not later on

IIRC, if the p2.inf is embedded at the right place in the artifact, the p2 publisher would always process it. So if client use the publisher, sharing the p2.inf is enough as every client should then regenerate the same p2 metadata.
But anyway, whatever works: if p2 metadata are published for this Maven artifacts directly (instead of assuming clients will regenerate them properly), that's good too.

@laeubi
Copy link
Contributor Author

laeubi commented Nov 28, 2023

IIRC, if the p2.inf is embedded at the right place in the artifact, the p2 publisher would always process it. So if client use the publisher, sharing the p2.inf is enough as every client should then regenerate the same p2 metadata.

That's possible at least Tycho do not do any special processing if used as maven artifact in a target everything is delegated to P2 publisher. But p2.inf is not the only source in a Tycho build for p2 metadata.

But anyway, whatever works: if p2 metadata are published for this Maven artifacts directly (instead of assuming clients will regenerate them properly), that's good too.

This is one thing we should/can check then as well if Tycho also would download such data and use it later on.

@vogella
Copy link
Contributor

vogella commented Dec 20, 2023

Maybe we could we also directly publish to repo.eclipse.org?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants