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

Support specification of remote repositories for Maven-PDE-Targets #202

Closed
HannesWell opened this issue May 18, 2021 · 32 comments · Fixed by #283
Closed

Support specification of remote repositories for Maven-PDE-Targets #202

HannesWell opened this issue May 18, 2021 · 32 comments · Fixed by #283
Assignees
Labels
pde-integration M2Eclipse PDE Integration

Comments

@HannesWell
Copy link
Contributor

The Maven-Target type is a great feature.
But what is missing at the moment is the possibility to specify any other remote repository than the default maven-central from which Maven artifacts and their dependencies are fetched.

I've seen that this was already discussed in PR #21 where the Maven-Target was added and that at the moment the only way to use another repository is to add it in the Maven-Settings. However this approach is not really feasible because then the Maven settings of every machine that needs to build the project have to be adjusted and kept in sync. Those of my colleagues, the build-server etc.

Therefore I suggest to add a possibility to specify a repository in the target file.

In the mentioned PR you already discussed to bundle the targets per repository but discarded this idea as to complicated. Other approaches could be to allow a nested repository element per Maven-Target (could be cumbersome if multiple artifacts of an remote repository are used) or to allow a Maven-repository element nested directly in the locations element or even in the target element. of a .target file.
In the latter cases each additionally specified repo could be considered for each maven target. This would also mimic the Maven behavior were you specify a List of repositories and a List of dependencies but (usually) not which dependency comes from which repo. At least I don't know that it would be possible.

@laeubi
Copy link
Member

laeubi commented May 18, 2021

Currently m2e only supports one "global" maven context, what would be required to support (pom defined) repositories would be a maven-context per project.

Specifying the repository in the file itself would often not enough. e.g. one might want to define mirrors, credentials (encrypted or not), have different profiles, snapshots enabled or not and so on. Some other drawbacks you have already mentioned in your request.

That's why currently (user) settings.xml is indeed currently the way to go as otherwise great parts of the maven configuration needs to be replicated.

@laeubi laeubi added the pde-integration M2Eclipse PDE Integration label May 18, 2021
@laeubi
Copy link
Member

laeubi commented May 18, 2021

Related discussion for Tycho https://bugs.eclipse.org/bugs/show_bug.cgi?id=570611

@HannesWell
Copy link
Contributor Author

Thanks for your quick answer and sorry for my late response.

Specifying the repository in the file itself would often not enough. e.g. one might want to define mirrors, credentials (encrypted or not), have different profiles, snapshots enabled or not and so on. Some other drawbacks you have already mentioned in your request.

Maybe this can be solved step by step and only the most common aspects of specifying other repos are covered first?
But as you said replicating most of the maven configuration is not the best way to go. At least with the syntax I suggest to stick as close to maven as possible. This way users can simply copy the relevant parts of the configuration to the target file.

But for me being able to specify other repos is not urgent. So if you consider this issue in your plans for the maven-target feature it would be great.

@laeubi
Copy link
Member

laeubi commented Jun 4, 2021

There is one other point that is similar to what mentioned here, we would need to think about how this is to be presented to the user so there should be an UI for add/remove/edit the location (or should we support multiple ones?) as well.

So if you consider this issue in your plans for the maven-target feature it would be great.

yep this is on my list as well as #212 that essentially will allow to specify more than one dependency per target. Currently there is no funding and/or customer supporting new features in that area so I have to see when/if I can get some free time to push this forward.

@vogella
Copy link
Contributor

vogella commented Jun 17, 2021

The option to specify the URL for the Maven repository would be great. Maybe the location tag could be enhanced to include an URL?

@HannesWell
Copy link
Contributor Author

HannesWell commented Jun 17, 2021

There is one other point that is similar to what mentioned here, we would need to think about how this is to be presented to the user so there should be an UI for add/remove/edit the location (or should we support multiple ones?) as well.

That's a good plan.

One idea is to have a pruned/miniature pom-editor where one can insert only a dependencies and a repositories section or instead two XML editor fields, one for the dependencies and one for the repositories section were one can insert the corresponding child elements. This would lead to a relatively simple UI and one can simply copy the dependency and repository elements from an existing pom file.
In the end full Editor support like in the pom-editor regarding input validation, content assist support and maybe even formatting would be great.

While I think this would be a handy approach for experienced maven users, unexperienced users would maybe prefer UI were they can insert the required data step-by-step in separate fields, if possible with drop-down menus and so on.
In this case a dynamic list were you can add/remove dependencies or repositories could be suitable.

Of course both approaches could be mixed in two different modes, but this would of course lead to more code to create, maintain etc. We could also try to do it similar to the current approach: Have a nice element-wise UI and you can add elements from the clipboard (multiple elements and mixed dependencies and repositories should be supported too).

In general I suggest, that we now should try to achieve a complete solution (or at least should have the whole picture in mind) and cover all aspects of Maven dependencies, which are suitable and possible to translate into the Eclipse/PDE world.
Otherwise, if we add them part by part, we could end up with a patchwork where we have special solutions for each aspect that are difficult to extend.

And I also suggest to stick to the Maven Syntax. Pom's are xml files, a target-platform is a xml-file, so why not nest a dependencies and a repositories element into a location element and allow something like the following? This way we don't have to re-invent something.

<location includeSource="true" missingManifest="generate" type="Maven">
	<dependencies>
		<dependency>
			<groupId>org.group</groupId>
			<artifactId>artifact1</artifactId>
			<exclusions>
				<exclusion>
					<groupId>bad.group</groupId>
					<artifactId>artifact3</artifactId>
				</exclusion>
			</exclusions>
		</dependency>
		<dependency>
			<groupId>org.group</groupId>
			<artifactId>artifact1</artifactId>
		</dependency>
	</dependencies>
	<repositories>
		<repository>
			<id>my-own-repo</id>
			<url>https://repo.eclipse.org/content/repositories/some/cool/stuff/</url>
			<snapshots>
				<enabled>true</enabled>
			</snapshots>
			<releases>
				<enabled>true</enabled>
			</releases>
		</repository>
	</repositories>
	<instructions><![CDATA[
Bundle-Name:           Bundle derived from maven artifact ${mvnGroupId}:${mvnArtifactId}:${mvnVersion}
version:               ${version_cleanup;${mvnVersion}}
Bundle-SymbolicName:   wrapped.${mvnGroupId}.${mvnArtifactId}
Bundle-Version:        ${version}
]]></instructions>
</location>

The instructions would then apply to all dependencies of the location element.

This way we could also start with a reduced set of supported features and add them later bit by bit, but are not in danger to be blocked in the future because the syntax is already clear. For not supported elements a warning/error could be emitted.

As special shortcut and to not break existing target-files, we could keep the current syntax for single dependency elements from default repositories.

@vogella
Copy link
Contributor

vogella commented Jun 18, 2021

Sounds great @HannesWell.

As for the PDE UI in the target editor I think it would be fine to display a "Maven" section in the tree representation of the target editor without the option to edit it and only allow the editing of it in the "Source" tab.

@laeubi
Copy link
Member

laeubi commented Jul 20, 2021

I have created a basic UI for editing repository entries:
grafik

a target might then looks like this:

<target name="extraRepository">
	<locations>
		<location includeDependencyScope="compile" includeSource="true" missingManifest="generate" type="Maven">
			<dependencies>
				<dependency>
				  <groupId>edu.ucar</groupId>
				  <artifactId>cdm</artifactId>
				  <version>5.0.0</version>
				</dependency>
			</dependencies>
			<repositories>
				<repository>
					<id>unidata-all</id>
					<url>https://artifacts.unidata.ucar.edu/repository/unidata-all/</url>
				</repository>
			</repositories>
		</location>
	</locations>
</target>

laeubi added a commit to laeubi/m2e-core that referenced this issue Jul 20, 2021
Maven-PDE-Targets

Signed-off-by: Christoph Läubrich <laeubi@laeubi-soft.de>
@laeubi laeubi linked a pull request Jul 20, 2021 that will close this issue
@vogella
Copy link
Contributor

vogella commented Jul 20, 2021

Thanks! Very useful for clients which needs to control the source of their libraries.

laeubi added a commit that referenced this issue Jul 20, 2021
Fix #202 Support specification of remote repositories for Maven-PDE-Targets
@HannesWell
Copy link
Contributor Author

@laeubi thanks a lot!
Your PRs #283 and #279 are amazing! They add a lot more value to m2e-pde. It's a joy to use them. 🥇

I think it is a good decision to stay close to the Maven syntax. This way all relevant/possible aspects of Maven dependencies can be implemented step by step without the need to re-invent the wheel and at the same time to make it simple to copy snippets from poms directly into the target file.

I also like the handling of 'old style' entries.

I've already tested it and it works very well.

@HannesWell
Copy link
Contributor Author

Sounds great @HannesWell.

As for the PDE UI in the target editor I think it would be fine to display a "Maven" section in the tree representation of the target editor without the option to edit it and only allow the editing of it in the "Source" tab.

At the moment working in the source Tab is IMO only useful, if you know exactly what you are doing. The tooling support is very limited, even though there is some basic support for the standard locations.

However this gave me the idea that it would be great if it were as convenient, regarding tooling support, to work in a .target file as for example it is to modify a pom.xml, with content-assist on all levels and documentation for elements and attributes.
This would be a good alternative to the often cumbersome Target-Editor and could in the end be a replacement of the Target-Platform-DSL (but with the advantage that there is no additional syntax to support).
Actually it could be like the target-platform DSL but with the regular target editor syntax.

The tooling support should be designed in a way that it is possible for location-types not maintained by pde (for example, like the Maven type) to participate as well and add own content assist proposals and documentation.

This would merge the advantages of the Target-Platform DSL, fast response and fluent work-flow, with those of a regular .target file, works well in the IDE and Tycho and is the 'standard' for Eclipse-PDE and therefore has many extensions like the m2e-Maven type or Oomphs Targlet-Containr. At the same time it would avoid the disadvantages of both. It would avoid the slow Target-Editor for a .target file and the not very fluent work-flow with and it would avoid the additional syntax to maintain of the TP-DSL, which does not support e.g. Maven or targlet targets and has to be converted into a .target file so it can be used during builds. Two files that have to be in sync, can also go out of sync accidentally.

But this is probably a bigger task and has to go to PDE.
But personally I would see this as the perfect editor to work on a target definition. I'm actually a big fan of the TP-DSL but due to the limited support in Tycho and the missing Maven-type I use a .target file and often modify it in the Source tab.

@mickaelistria
Copy link
Contributor

At the moment working in the source Tab is IMO only useful, if you know exactly what you are doing. The tooling support is very limited, even though there is some basic support for the standard locations.

IMO, the source tab is the one that provides highest efficiency for a .target that's only made of p2 repositories. The completion in it is more interesting that whatever the wizards show in other pages.

However this gave me the idea that it would be great if it were as convenient, regarding tooling support, to work in a .target file as for example it is to modify a pom.xml, with content-assist on all levels and documentation for elements and attributes.

+1, the .target editor is open to such improvements as far as I know.

Actually it could be like the target-platform DSL but with the regular target editor syntax.

+1000, the main benefit of the TP-DSL used to be the good textual editor, but I've always advocated it'd be more profitable -and even easier and more sustainable- to improve the existing .target editor to have good edition support than to create a new file.

The tooling support should be designed in a way that it is possible for location-types not maintained by pde (for example, like the Maven type) to participate as well and add own content assist proposals and documentation.

The textual .target editor uses the Generic Editor. The Generic Editor is extensible. m2e could easily add some completion or hover participants to support Maven location types.

@laeubi
Copy link
Member

laeubi commented Jul 28, 2021

@HannesWell @vogella Support for this was also added to tycho-snapshot, would be good to test if there are any issues.

@HannesWell
Copy link
Contributor Author

HannesWell commented Jul 30, 2021

Thanks for this!
I just tested it and encountered problems during the Maven/Tycho build with the following dependency:

<dependency>
	<groupId>nl.jqno.equalsverifier</groupId>
	<artifactId>equalsverifier</artifactId>
	<version>3.6</version>
	<type>jar</type>
</dependency>

The Manifest generated by m2e for the equalsverifier.jar contains this version header: Bundle-Version: 3.6
In the Eclipse IDE I can declare a dependency to the wrapped bundle using
Require-Bundle: wrapped.nl.jqno.equalsverifier.equalsverifier;bundle-version="3.6.0"
or
Require-Bundle: wrapped.nl.jqno.equalsverifier.equalsverifier;bundle-version="3.6"
and both work fine. Even tough the Eclipse-IDE seems to be in favour of the first version: I can only remove the micro/service version when modifying the MANIFEST.MF directly, when removing it trough the Editor in the Dependencies tab the zero is always added.

But when I try to run the Maven/Tycho build the target-platform resolution fails with the following error:

[ERROR]   Software being installed: my.plugin.tests 1.0.0.qualifier
[ERROR]   Missing requirement: my.plugin.tests.util.unit 1.0.0.qualifier requires 'osgi.bundle; wrapped.nl.jqno.equalsverifier.equalsverifier 3.6.0' but it could not be found
[ERROR]   Cannot satisfy dependency: my.plugin.tests 1.0.0.qualifier depends on: osgi.bundle; my.plugin.util.unit 0.0.0

This happens regardless if I reference the wrapped bundle using the version "3.6.0" or "3.6". The error message is the same in both cases.

This does not happen with the current Tycho 2.4.0 release. So I think the problem is actually in Tycho.

@HannesWell
Copy link
Contributor Author

Actually it could be like the target-platform DSL but with the regular target editor syntax.

+1000, the main benefit of the TP-DSL used to be the good textual editor, but I've always advocated it'd be more profitable -and even easier and more sustainable- to improve the existing .target editor to have good edition support than to create a new file.

I have read some of those discussions and I fully agree with you. I just wonder why there are so many complains about that editor but only few/no attempts to improve it.

The tooling support should be designed in a way that it is possible for location-types not maintained by pde (for example, like the Maven type) to participate as well and add own content assist proposals and documentation.

The textual .target editor uses the Generic Editor. The Generic Editor is extensible. m2e could easily add some completion or hover participants to support Maven location types.

Thanks for your remarks, that's good to know. Unfortunately I have other topics on my list, that have higher priority for me at the moment. But I will come back to it once they are done (hopefully in autumn/winter of this year) unless somebody else already handled it.

@laeubi
Copy link
Member

laeubi commented Jul 31, 2021

I just wonder why there are so many complains about that editor but only few/no attempts to improve it.

One point is, that even people complain much, they often refuse to start change something as it is not that bad and simply want to proceed with their daily work.

I can only tell from my experience with contributing to PDE (or Eclipse Platform in general) that it is often very hard to even contribute basic features as there are always discussions if the feature is valuable, if there are riks to 'break' users that are used to the old behaviors and so on. So I can fully understand if people decide that's much more profitable to just start from scratch.

A good example is the (now at least accepted) maven target platform support:

  1. There where always strong complains but people seem to resignation instead of change anything as there are at least some (uncomfortable) ways to archive the goal
  2. While I was able to provide a P.O.C. in a relative short time (3 - 4 days) there was no feedback from my request to integrate/improve it, essentially it has taken three years until I found the time beside my daily (payed) business and more important, gained the necessary insight and reputation in the different projects in eclipse eco-system to integrate it myself.
  3. Adding the basic support to Tycho for making such a change useful was a hard work as there are a lot of discussions if we really need it, a lot of mental reservation if its even a good idea as we already have 'good practices' like orbit and so on
  4. And finally I needed to invest some more days/weeks to get some rather trivial changes into the PDE Target Editor UI so we can have some more advanced editing capabilities, wait for the usual release delays and so on to actually use it.

The only thing making it finally happen was that I'm convinced it will be a great feature and never resigned, but I can totally understand that not everyone is willing / motivated to keep track for a volunteer project > 3 years and as we see here we are still not feature complete with it.

The reficio/p2-maven-plugin is the counterpart here (as the TP-DSL is for the target editor) where it was decided to instead spin off a completely new, unconstrained approach and it has 25 (!) contributors, numerous releases dating back to 2013, so one can't argue there is no one willing to contribute for such features per-se.

But when I try to run the Maven/Tycho build the target-platform resolution fails with the following error:

You should see an output in the log about the resolved items and there version/names maybe this gives a hint? If not at least a tycho-integration test would be good to further investigate, the TargetPlatformLocationsTest would be a good place for this and currently contains already tests for different manifestations of the target features.

@vogella
Copy link
Contributor

vogella commented Jul 31, 2021

@laeubi would be nice if can get into a faster development and feedback loop in platform and PDE. We already release every 3 months so the release time should not be an issue anymore. One of the issue is that almost nobody reviews incoming changes. If you want I can nominate you as PDE committer to speed up the process.

@laeubi
Copy link
Member

laeubi commented Jul 31, 2021

@vogella thanks for the offer but I must confess that especially for PDE my last attempts to suggest improvements/new features where very frustrating and I don't expect becoming a committer will change much here... this has essentially made it much unlikely that I'll start a new attempt in the near future beside (for my specific use case) absolute necessary buxfixes. Currently even none of my customers has any focus on improving PDE tooling but are more addicted to switch over to alternative toolings (e.g. BND Tools, InteliJ, ...) so I don't expect much from that side either.

@HannesWell
Copy link
Contributor Author

@laeubi thank you for sharing your insights/experience´. I fully understand that it is frustrating and requires a lot endurance to work an a feature and convince people over such a long time. But I also understand the committers, because if something is not working like before users often start to complain even tough there is an assumed better way because for whatever reason they prefer the old way. So I think OSS in general requires cooperation and patience from all participating parties. And IMHO the fear to break existing code is a big sign of bad test coverage (but likes to write tests?).
But I'm thankful, that you made the Maven-targets possible and that they become better and better. :)

And I hope/think that improved tooling support for the target editor's source page will be accepted without resistance because it is almost not existing yet, so there is almost no chance to break anything.

But when I try to run the Maven/Tycho build the target-platform resolution fails with the following error:

Please disregard my remark. It was my fault because, I did something else wrong and everything works fine.

@laeubi
Copy link
Member

laeubi commented Aug 16, 2021

Please disregard my remark. It was my fault because, I did something else wrong and everything works fine.

Great, let me know if you encounter any issues.

@vogella
Copy link
Contributor

vogella commented Aug 16, 2021

Some PDE reviews are also fast, see for example @laeubi Gerrit from today https://git.eclipse.org/r/c/pde/eclipse.pde.ui/+/184053

@vogella
Copy link
Contributor

vogella commented Jan 10, 2022

I opened a clarification question for this feature in #488

@LuluDavid
Copy link

@laeubi I recently had some issues in Eclipse UI to retrieve some Maven dependencies specified in my target platform.
These dependencies come from a remote repository set up on our intranet with a login-password, which I logged accordingly in my settings.xml. I have two potential issues :

  • Either I specify this repository in my parent's pom.xml in the tag, and I can retrieve these maven dependencies in my target platform in CLI, but in UI, non-snapshot dependencies are not seemingly resolved with a UI error "Missing groupId:artifactId:jar:version". The only workaround I found is to remove .remote.repositories file from ~/.m2/groupId/artifactId/version for this to work, which explains why snapshot dependencies are correctly resolved as they ignore this file.
  • Either I specify this repository in my target platform directly with the syntax you mentioned above, but the authentication fails and I get an error :
    [ERROR] Failed to resolve target definition ...: ... Could not transfer artifact ... from/to myserver (https://myserver.com): authentication failed for https://myserver.com/path/to/myjar.jar, status: 401 Unauthorized
    So maybe I would need to specify the target platform to use ~/.m2/settings.xml somehow, but it is already done in Preferences -> Maven -> User Settings.

I am sorry if I cannot provide an example for obvious reasons, but maybe there is a clear explanation for this bug in the code. Thanks for reading.

@laeubi
Copy link
Member

laeubi commented Sep 28, 2022

Specify additional repository in repository is the right choice. The settings used are those in the Preferences > Maven > User Settings so that should be fine, without an runnable example it is hard to guess what might be wrong beside this.

As a maven repository is just some http-server serving some files you might be able still create a test-case (e.g. as JUnit) to demonstrate the issue.

@LuluDavid
Copy link

LuluDavid commented Sep 28, 2022

Ok, then I can provide at least an example of the first option mentioned above with a non-login repo.
I tried to produce a minimal example here, I wrote everything you need to know in the README.
Waiting for your feedback.

@laeubi
Copy link
Member

laeubi commented Sep 28, 2022

I'm not quite sure but you wrote:

You will observe that contrarily to CLI, edu.ucar:cdm (5.1.0) is resolved but not edu.ucar:d4lib (5.5.3), as repositories is not specified for the second one.

So actually it does work when you added the repository but your expectation is that it never works? If the repository is actually queried depends on the state of your local repository, so I'm not sure if I understand the issue correctly....

@LuluDavid
Copy link

Sorry if it is unclear.
I already add the <repositories> tag in the parent's pom, so I expect the target platform to automatically retrieve Maven dependencies from this repository (as for Maven central dependencies).
In CLI, it works fine, but in UI, I notice that I additionally have to specify the <repositories> tag in the target platform file. Is it expected behavior ?
If yes, I guess that the second option I mentioned above would be the correct way to deal with it (i.e only have the <repositories> tag in the target platform file), but I will have to find a way to resolve the authentication issue.

@laeubi
Copy link
Member

laeubi commented Sep 28, 2022

Sorry if it is unclear.
I already add the tag in the parent's pom, so I expect the target platform to automatically retrieve Maven dependencies from this repository (as for Maven central dependencies).

There is not really a connection between maven.pom and target, thats why you either need:

  1. Specify a repository in the global maven user configuration
  2. Specify an "additional repository (but still authenticate has to go to the global config)

@LuluDavid
Copy link

Ok, my bad, I expected my parent POM's <repositories> to be forwarded to the target platform.
I tried to move this tag only in my target platform, but I still have authentication issues. When you say "specify a repository in the global maven user configuration", you mean encrypting passwords in settings-security.xml & settings.xml (under the <server> tag), right ?
Or is there additional tags to add somewhere else ?

@laeubi
Copy link
Member

laeubi commented Sep 29, 2022

Here is an example for configuring an additional repository in the settings.xml:

https://github.com/eclipse-tycho/tycho/wiki#configure-snapshots-repository-globally

@LuluDavid
Copy link

Ok, I got it to work properly, the syntax made me believe that the target platform defined a <repositories> tag itself, whereas it just references an existing one (either in the global settings or in a parent maven.pom file).
Thanks !

@laeubi
Copy link
Member

laeubi commented Sep 29, 2022

Instead of editing the file as text you better edit it with the editor

the id is the one one has to use in case of authentication.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pde-integration M2Eclipse PDE Integration
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants