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

Remove tycho.mode=maven #676

Closed
laeubi opened this issue Feb 22, 2022 · 27 comments · Fixed by #981 or #3426
Closed

Remove tycho.mode=maven #676

laeubi opened this issue Feb 22, 2022 · 27 comments · Fixed by #981 or #3426
Milestone

Comments

@laeubi
Copy link
Member

laeubi commented Feb 22, 2022

We should no longer support tycho.mode=maven it was often used to package an update-site in a regular maven build or to build "mixed reactors", for both we already have better solutions now.

@laeubi laeubi added this to the 3.0 milestone Feb 22, 2022
@mickaelistria
Copy link
Contributor

Isn't m2e still using it?

@laeubi
Copy link
Member Author

laeubi commented Feb 22, 2022

m2e build is currently using this but the goal is to get rid of it with tycho 2.7

@HannesWell
Copy link
Member

m2e build is currently using this but the goal is to get rid of it with tycho 2.7

That's right. But I think Tycho should still support cases like the current first build step of m2e. These are edge-cases no question, but it can be handy to build a project with a partly different configuration to for example generate some resource in advance.

I suspect the problem to be that the module containing the target-platform definition does not participate in the build, but I have not verified that.
Given that is the case one could either try to add the TP module to the build or Tycho would not fail in case the TP-module is not found and would just print a warning instead. Alternatively maybe it could work to disable the target-platform-configuration, but I don't think this is possible yet (because that goal is actually never executed), isn't it? But in the end this just re-introduces tycho.mode=maven in another way.

@HannesWell
Copy link
Member

HannesWell commented Feb 22, 2022

It indeed worked to add the tp-module in the tycho.mode=maven build. To not interfere with the regular build I added it in the profile that is activated only when tycho.mode=maven was used, please see the third commit in eclipse-m2e/m2e-core#589
This has the positive effect that only the profile has to be activated and the additional tycho.mode=maven property can be skipped, which makes the build command simpler. I think this is an 'workaround' for the better, since I expect a profile to be used in cases similar to the one of M2E anyway.

The only problem left is, that it does not work when the MANIFEST.MF was not yet generated and fails with the exception below.

Do you think the project setup could fall back to the pom.xml (which I think has to be present in such cases)?

[INFO] Scanning for projects...
[ERROR] Internal error: org.eclipse.tycho.core.osgitools.OsgiManifestParserException: Exception parsing OSGi MANIFEST C:\dev\git\eclipse.m2e-core\m2e-maven-runtime\org.eclipse.m2e.archetype.common\META-INF\MANIFEST.MF: Manifest file not found -> [Help 1]
org.apache.maven.InternalErrorException: Internal error: org.eclipse.tycho.core.osgitools.OsgiManifestParserException: Exception parsing OSGi MANIFEST C:\dev\git\eclipse.m2e-core\m2e-maven-runtime\org.eclipse.m2e.archetype.common\META-INF\MANIFEST.MF: Manifest file not found
	at org.apache.maven.DefaultMaven.execute(DefaultMaven.java:120)
	at org.apache.maven.cli.MavenCli.execute(MavenCli.java:972)
	at org.apache.maven.cli.MavenCli.doMain(MavenCli.java:293)
	at org.apache.maven.cli.MavenCli.main(MavenCli.java:196)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
	at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.base/java.lang.reflect.Method.invoke(Method.java:566)
	at org.codehaus.plexus.classworlds.launcher.Launcher.launchEnhanced(Launcher.java:282)
	at org.codehaus.plexus.classworlds.launcher.Launcher.launch(Launcher.java:225)
	at org.codehaus.plexus.classworlds.launcher.Launcher.mainWithExitCode(Launcher.java:406)
	at org.codehaus.plexus.classworlds.launcher.Launcher.main(Launcher.java:347)
Caused by: org.eclipse.tycho.core.osgitools.OsgiManifestParserException: Exception parsing OSGi MANIFEST C:\dev\git\eclipse.m2e-core\m2e-maven-runtime\org.eclipse.m2e.archetype.common\META-INF\MANIFEST.MF: Manifest file not found
	at org.eclipse.tycho.core.osgitools.DefaultBundleReader.loadManifestFromDirectory(DefaultBundleReader.java:93)
	at org.eclipse.tycho.core.osgitools.DefaultBundleReader.doLoadManifest(DefaultBundleReader.java:60)
	at org.eclipse.tycho.core.osgitools.DefaultBundleReader.loadManifest(DefaultBundleReader.java:51)
	at org.eclipse.tycho.core.osgitools.OsgiBundleProject.readArtifactKey(OsgiBundleProject.java:194)
	at org.eclipse.tycho.core.osgitools.OsgiBundleProject.setupProject(OsgiBundleProject.java:176)
	at org.eclipse.tycho.core.resolver.DefaultTychoResolver.setupProject(DefaultTychoResolver.java:89)
	at org.eclipse.tycho.core.maven.TychoMavenLifecycleParticipant.afterProjectsRead(TychoMavenLifecycleParticipant.java:111)
	at org.apache.maven.DefaultMaven.doExecute(DefaultMaven.java:264)
	at org.apache.maven.DefaultMaven.doExecute(DefaultMaven.java:192)
	at org.apache.maven.DefaultMaven.execute(DefaultMaven.java:105)
	... 11 more
[ERROR] 
[ERROR] To see the full stack trace of the errors, re-run Maven with the -e switch.
[ERROR] Re-run Maven using the -X switch to enable full debug logging.
[ERROR] 
[ERROR] For more information about the errors and possible solutions, please read the following articles:
[ERROR] [Help 1] http://cwiki.apache.org/confluence/display/MAVEN/InternalErrorException

@laeubi
Copy link
Member Author

laeubi commented Feb 23, 2022

You don't need a target platform at all, but as I said I actually don't see any reason why one want to use tycho but disable it afterwards? Actually the 2.7. release was meant for handling those "edge-cases" in a more maven friendly way, thats why classpath calculation is now moved to the regular build-phase.

The only problem left is, that it does not work when the MANIFEST.MF was not yet generated and fails with the exception below.

When the manifest needs to be generated, you should use the maven-bundle-plugin packaging and not eclipse-plugin packaging. If packages from that generation are to be consumed in the same reactor build, you additionally need to enable pomdependecnies = consider

See for example https://github.com/eclipse/tycho/tree/master/tycho-its/projects/mixed.reactor for a mixed reactor build.

@HannesWell
Copy link
Member

When thinking about it again, that reasonable and I think you are right.
With PR eclipse-m2e/m2e-core#466 I already started to convert the projects but I encountered problems, mainly in the IDE that the content of the referenced Maven-jars is not available for down-stream projects, but it could be possible to handle these problems separated from the build problems and to convert the projects nonetheless. But I have to continue my work there and see if that works.

@laeubi
Copy link
Member Author

laeubi commented Feb 23, 2022

Sure its a bit work-in-progress but I hope we can get this working at least for m2e 2.0 and tycho 3.0 ...

@Bananeweizen
Copy link
Contributor

Wouldn't that lead to having the very long target resolution back for commands like "help:effective-pom"? I thought running those really non tycho specific goals quickly was the main reason for introducing that mode...

@akurtakov
Copy link
Member

akurtakov commented Apr 30, 2022

@Bananeweizen I agree with you. It sounds like it will create some noticeable slowdowns to pure Maven (just clean, help:, etc.) workflows.

@laeubi
Copy link
Member Author

laeubi commented Apr 30, 2022

@Bananeweizen @akurtakov We already have special handling for "clean only" workflow, so it would be better to add automatically detection of those instead of having some mostly magic property. Beside that, the goal is to speed up the early stages and delay actual resolution to the appropriate maven phases so this might become even irrelevant in the future at all, see

@akurtakov
Copy link
Member

I forgot about the special handling of clean. Speeding up things by default when identified without having to know about special switch like tycho.mode=maven sounds even better.
@Bananeweizen Can you thing of anything else but help: being slower?

@akurtakov
Copy link
Member

@laeubi as there doesn't seem to be any interest in keeping it, feel free to go on and remove it.

@laeubi laeubi removed this from the 3.0 milestone May 18, 2022
@laeubi
Copy link
Member Author

laeubi commented May 18, 2022

@akurtakov its on my list, but will take some more time I'll remove the milestone for now.

@Bananeweizen
Copy link
Contributor

@akurtakov I'm fine with removing it also. In fact, using only some non Tycho goal in a Tycho project happens so seldom that I myself also completely forget to add the magic property then. Therefore I've not really been using this in practice. Some automated detection instead of a property would surely work better for more users, since they don't have to know anything about the differences then.

akurtakov added a commit to akurtakov/tycho that referenced this issue May 31, 2022
This was helpful for cases where tycho resolution is not needed but very
few people are aware of it and thus not widely used. As custom support
to exclude "clean" (probably the most widely used) has been added the
reasons to have it diminished significantly. If there is need for more
to be excluded it should be done like for "clean" rather than rely on
undocumented property.
Fixes eclipse-tycho#676
akurtakov added a commit that referenced this issue May 31, 2022
This was helpful for cases where tycho resolution is not needed but very
few people are aware of it and thus not widely used. As custom support
to exclude "clean" (probably the most widely used) has been added the
reasons to have it diminished significantly. If there is need for more
to be excluded it should be done like for "clean" rather than rely on
undocumented property.
Fixes #676
@sratz
Copy link
Contributor

sratz commented Jun 3, 2022

I somehow missed this issue earlier-on and just noticed this after the removal got merged.

This would be causing considerable issues in our workflows.

We currently have ~800 artifacts in our build and the target platform consists of many mvn:groupid:artifactid:version:zip entries referring artifacts with significant size (several GB in total).

We have several workflows, in which we used tycho.mode=maven:

  • Bump versions (mvn org.eclipse.tycho:tycho-versions-plugin:set-version ...)
  • Custom invocation of external goals (mvn foo:bar:some-proprietary-goal ...) which definitely do not need the tycho lifecycle participant to be active.

These workflows are now impacted severely:

  • They used to take only a few seconds.
  • Now they take
    • at minimum 10 minutes for the dependency resolution phase, if the ~/.m2 cache is fully up-to-date
    • much longer if the cache is not up-to-date, because all the artifacts of the target platform need to be downloaded.

From the looks of it the removal of tycho.mode=maven was rather trivial.

Can you reconsider adding it back, and instead giving it a proper documentation / disclaimer?

(At least until significant progress has been made on #839)

@akurtakov
Copy link
Member

Can we have a deal here? I revert, you help us by documenting it as you seem to be a person using it extensively.

@akurtakov
Copy link
Member

Reverted with 9673011

@sratz
Copy link
Contributor

sratz commented Jun 3, 2022

Thank you!

you help us by documenting it as you seem to be a person using it extensively.

Sure. I will provide a pull-request.

@LorenzoBettini
Copy link
Contributor

In my recent experience, using "tycho.mode=maven" was the only way to run dependency:go-offline correctly... otherwise, I get errors trying to resolve from Maven central non-existing artifacts of the shape

Failed to execute goal 
org.apache.maven.plugins:maven-dependency-plugin:3.3.0:go-offline (default-cli) on project ...:
org.eclipse.aether.resolution.DependencyResolutionException:
Could not find artifact 
p2.eclipse.plugin:org.antlr.generator:jar:3.2.0.v201405091103 
in central (https://repo.maven.apache.org/maven2)

the name of the non existing Maven artifact to resolve is likely to change if I run the build again...

@laeubi
Copy link
Member Author

laeubi commented Oct 25, 2022

@LorenzoBettini can you open a dedicated bug for this, maybe with an example or even provide an integration-test to demonstrate the issue?

@basilevs
Copy link
Contributor

Another usecase is offline handling of operations like cleaning and help. In my experience Tycho usually fails on any kind of P2 problems including network.

@laeubi
Copy link
Member Author

laeubi commented Jun 14, 2023

Another usecase is offline handling of operations like cleaning and help. In my experience Tycho usually fails on any kind of P2 problems including network.

This is no longer true since Tycho 3.x with the improved http cache:

https://github.com/eclipse-tycho/tycho/blob/master/RELEASE_NOTES.md#improved-p2-transport-for-more-efficiently-http-cache-handling-and-improved-offline-mode

here also, if you see any issues please don't hesitate to report them (at best provide an integration-test to demonstrate the issue), especially with Tycho 5 things will likely change a lot and we will drop all things with unclear usecase and/or missing integration-test coverage.

@basilevs
Copy link
Contributor

basilevs commented Jun 14, 2023

Another usecase is offline handling of operations like cleaning and help. In my experience Tycho usually fails on any kind of P2 problems including network.

This is no longer true since Tycho 3.x

Following command worked for me with Maven mode, but not anymore (missing manifest dependency breaks set-version even in offline mode). Should I create a separate issue, or is this a direct consequence of Maven mode removal?

$mvn --offline org.eclipse.tycho:tycho-versions-plugin:set-version  -DnewVersion=0.0.13-SNAPSHOT
.........
[INFO] ### Using TychoRepositoryTransport for remote P2 access ###
[INFO]     Cache location:         /Users/vasiligulevich/.m2/repository/.cache/tycho
[INFO]     Transport mode:         offline
[INFO]     Update mode:            cache first
[INFO]     Minimum cache duration: 60 minutes
........
[INFO] Adding repository https://download.eclipse.org/releases/2023-03
[INFO] Resolving dependencies of MavenProject: org.vgcpge.eclipse.copilot:org.vgcpge.copilot.ls:0.0.12-SNAPSHOT @ /Users/vasiligulevich/git/eclipse.copilot/org.vgcpge.copilot.ls/.polyglot.META-INF
[INFO] {osgi.os=win32, osgi.ws=win32, org.eclipse.update.install.features=true, osgi.arch=x86_64}
[ERROR] Cannot resolve project dependencies:
[ERROR]   Software being installed: org.vgcpge.copilot.ls 0.0.12.qualifier
[ERROR]   Missing requirement: org.vgcpge.copilot.ls 0.0.12.qualifier requires 'osgi.bundle; org.eclipse.lsp4j.jscdfonrpc 0.0.0' but it could not be found

@laeubi
Copy link
Member Author

laeubi commented Jun 14, 2023

We have

if you can enhance this to show the issue it would be great and much easier to analyze and make sure it does not break in the future.

@laeubi laeubi added this to the 5.0 milestone Oct 10, 2023
@drsgoodall
Copy link

Sorry I'm late to the party - I use this flag for clean, tycho-versions-plugin and with a license/copyright header update plugin.

@laeubi
Copy link
Member Author

laeubi commented Jan 15, 2024

@drsgoodall can you check with latest Tycho that it is still required?

@drsgoodall
Copy link

With Tycho 4.0.4 these goals all run quickly without the tycho.mode flag.

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 a pull request may close this issue.

9 participants