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

Bug 380152 - Support Maven option --also-make-dependents, and --resume-from #48

Closed
lorenzodallavecchia opened this issue Apr 10, 2021 · 26 comments · Fixed by #577
Closed

Comments

@lorenzodallavecchia
Copy link

lorenzodallavecchia commented Apr 10, 2021

See https://bugs.eclipse.org/bugs/show_bug.cgi?id=380152

The bug is still occurring. Due to an architectural change in Maven 3.2.1 in the Reactor, Tycho can no longer compute the dependencies if the input modules are filtered with options like -pl. Therefore, options like -am, -amd and -rf do not work.

See the original BugZilla issue for details and also see https://bugs.eclipse.org/bugs/show_bug.cgi?id=494760 which is likely caused by the same problem.

Pascal Rapicault CLA Friend 2012-05-21 10:29:58 EDT

Created attachment 215967 [details]
Test projects

Running maven with -amd or -rf will cause the build to fail because of the artifacts that are normally part of a full reactor build can't be found as they are not being built.

For example, in the following projects (see attachment)

  • testA (bundle)
  • testB (bundle, depends on A)
  • testFeature (include testA and testB)

executing mvn clean install -amd -pl net.rapicault.demo:testB
will fail with the following error message.

[ERROR] Failed to execute goal org.eclipse.tycho:tycho-packaging-plugin:0.14.1:package-feature (default-package-feature) on project testFeature: Execution default-package-feature of goal org.eclipse.tycho:tycho-packaging-plugin:0.14.1:package-feature failed: net.rapicault.demo:testA:eclipse-plugin:1.0.0-SNAPSHOT does not provide an artifact with classifier 'null' -> [Help 1]

I tried various tricks like adding a p2 repository that would include the artifacts from a previous successful build but again, no luck here.

[tag] [reply] [−] Comment 1 Tobias Oberlies CLA Friend 2012-05-23 12:17:38 EDT

I wouldn't consider this a bug - I don't believe that Tycho was ever meant to work with this option. We can keep this bug as enhancement request though.

[tag] [reply] [−] Comment 2 Pascal Rapicault CLA Friend 2012-05-23 12:27:05 EDT

What about the -rf option?

[tag] [reply] [−] Comment 3 Tobias Oberlies CLA Friend 2012-05-29 03:06:53 EDT

I don't know enough about these options to say if it would make sense to support these in Tycho.

Maven experts: What do you think?

[tag] [reply] [−] Comment 4 Tobias Oberlies CLA Friend 2012-10-04 13:14:37 EDT

This is what seems to happen in Maven: With --projects (and probably similarly with -rf), Maven scans all projects aggregated by the entry point POM, but then only executes the build for some of the projects.

In Tycho, this means the following:

  • The dependency resolution is done for all projects because Tycho listens to the afterProjectsRead Maven event that is fired for all aggregated projects. This cannot be changed, because Tycho currently uses its dependency resolution to determine the build order.
  • The dependency resolution result then contains references between reactor projects. Unlike with a full build, it is not guaranteed that upstream reactor artifacts have been built, leading to the described error.

To fix this, we'd need to recompute the target platform at the beginning of a module build (from a regular mojo) and either re-do dependency resolution or amend the previous dependency resolution result. All this is close to what needs to be done for bug 353889, so I don't think that this will be implemented independently of that major restructuring.

@kriegaex
Copy link
Contributor

kriegaex commented Jan 20, 2022

This is a major shortcoming. Some Eclipse projects are really large and slow to build. Waiting for the whole project to be built just to run a single test in another module is a nightmare. It would be great if this would finally get fixed, if it is broken since Maven 3.2.1, which was released in February 2014, i.e. eight years ago.

@laeubi
Copy link
Member

laeubi commented Jan 20, 2022

Some Eclipse projects are really large and slow to build. Waiting for the whole project to be built just to run a single test in another module is a nightmare.

You probably could use the waiting time to prepare a patch for this :-)

@laeubi
Copy link
Member

laeubi commented Jan 20, 2022

Just some update that might help if someone is willing to provide a patch:

  • The original bug mentioned Bug 353889 as a perquisite witch is fixed now in Tycho 2.7.0-SNAPSHOT
  • The mentioned restriction that tycho needs to resolve all projects is no longer true in Tycho 2.7.0-SNAPSHOT where we support partial resolving
  • Real -fae support #520 is a bit related here and adding support for -fae would give the necessary insights whats going on in tycho to start with this

What could be done right now is using the tycho snapshot enable pomDependecies=consider for the build (to trigger partial resolving) and see how far it goes...

Beside that, if this is crucial to someones business and likes to speed up the development in that area a sponsoring would allow me to assign more time-slots for this particular issue.

@kriegaex
Copy link
Contributor

kriegaex commented Jan 20, 2022

@laeubi, what makes you think that I speak OSGi or Tycho? And what kind of attitude is it, that in some OSS projects, maintainers or other contributors speak this sentence whenever a user who is not a committer or previous contributor to the project creates or even comments on an issue. Is that some kind of Pavlov reflex? "It is OSS, so just fix it yourself." Whom are you helping with such a comment? Don't you think that if I was capable of and had the time for preparing a PR, I would not have done so already?

@laeubi
Copy link
Member

laeubi commented Jan 20, 2022

what makes you think that I speak OSGi or Tycho

Tycho uses standard java techniques, you complain about slow eclipse projects in combination with tycho what almost always implies OSGi+Java so you are all set to start, beside that you can start learning. If there are any questions/problems we are here to help you getting started.

And what kind of attitude

I can offer you with some options, you don't have to choose any. I do fix things that are important to me, sometimes I fix things just for fun and sometimes someone is paying me to fix things that are important to them.

This ticket does not fall in any of these categories right now, so someone finally has to change this but obviously no one care in the past eight years.

"It is OSS, so just fix it yourself."

You can fix it or you can pay someone or you can wait until someone else do it, I think this essentially is the bottom line. Large parts of OSS is driven by volunteering work.

... and had the time for preparing a PR, I would not have done so already?

Well its seems we are all in the same boat then, or do you assume Tycho contributor just waiting for a ping to push their already (eight years) prepared PRs? :-)

@kriegaex
Copy link
Contributor

kriegaex commented Jan 20, 2022

The fact that I sometimes have to build an Eclipse project (in my case AJDT, AspectJ Development Tools), because I am helping to do the bare minimum to keep it alive, does not mean that I am feeling inclined to learn OSGi in general, Tycho or Eclipse in particular or how certain Tycho-related plugins relate to the Maven lifecycle. This issue here is potentially slowing down everyone working with Tycho + Maven, not just myself. Tycho simply does not conform to the expected behaviour for any Maven plugin. Normally developers can e.g. run mvn -DskipTests=true install and then simply run tests in the modules they are interested in later, maybe fixing them along the way. Maven would normally find the artifacts the module depends on in the local Maven repository and just use them, instead of requiring a full build every time. I have to accept that this has low priority for you, but I do not understand why. This could be a low-hanging fruit and major time saver for many developers, maybe even including yourself. It is just sad that knowing Maven fairly well is rendered utterly useless when trying to build Tycho projects.

@kriegaex
Copy link
Contributor

kriegaex commented Jan 20, 2022

Just some update that might help if someone is willing to provide a patch: (...)

Thanks, this is a great overview and potentially helpful to anyone trying to tackle this. Maybe later I can give the 2.7.0 snapshot with pomDependecies=consider a try and report back how far it gets me.

BTW, I also just noticed that -fae does not work, but reports success in all modules. So thanks for mentioning the related issue.

@laeubi
Copy link
Member

laeubi commented Jan 20, 2022

This issue here is potentially slowing down everyone

Given on the feedback on this ticket obviously not...

Tycho simply does not conform to the expected behaviour for any Maven plugin.

Tycho works well with numerous maven plugins, haven't heard any complains about general misconception. Anyways, your free to choose a better toll (there is ant4eclipse and pde-build in particular).

Normally developers can e.g. run mvn -DskipTests=true install and then simply run tests in the modules they are interested in later,

I often do this exact workflow, so it does work and I can't see how this is related to --also-make-dependents, and --resume-from what I have never required to use for such use-case.

Maven would normally find the artifacts the module depends on in the local Maven repository

And so does tycho if you do not forbids this explicitly.

I have to accept that this has low priority for you, but I do not understand why.

I don't know what particular issue you see but because in general your described workflow works without any issues for me...and probably also for others. On the other hand implementing this is very complex and probably error prone without much gain.

Most developers (in the eclipse project area) are also most probably executing their test directly from the IDE instead of fire up a maven build for this purpose, another reason this seems not an issue for most tycho-users (or potential contributors).

@kriegaex
Copy link
Contributor

Most developers (in the eclipse project area) are also most probably executing their test directly from the IDE instead of fire up a maven build for this purpose, another reason this seems not an issue for most tycho-users (or potential contributors).

That is probably true. I work in IntelliJ IDEA, not in an Eclipse plugin project. So I completely rely on Maven, trying to touch as little Eclipse stuff as possible. So I am kind of an exotic bird in this regard. Thanks for all your valuable feedback and for your time, Christoph. 🙂

@laeubi
Copy link
Member

laeubi commented Jan 20, 2022

I work in IntelliJ IDEA

I think this is indeed not very well covered area, but again, contributions are welcome :-)

@kriegaex
Copy link
Contributor

Maybe later I can give the 2.7.0 snapshot with pomDependecies=consider a try and report back how far it gets me.

I searched the web for pomDependecies=consider, but did not find any description of how to use it. Is it

  • a system property? If so, which plugin(s) need it?
  • a plugin or plugin goal configuration setting? If so, which plugin(s) and/or goal(s) can use it?
  • something else?

@kriegaex
Copy link
Contributor

enable pomDependecies=consider

OK, I just noticed that there simply was a typo in your post amd I copied & pasted it when searching the web. It is pomDependencies, not pomDependecies. It was so subtle, I did not notice.

@joeshannon
Copy link
Contributor

This would be a very useful feature. For a use case example: we have a workspace/build with many (~40) different products (containing variations of included features/plugins). In general only one is required to be built at a time, ideally with e.g. mvn --projects product1 --also-make clean verify if this was supported in Tycho. At the moment we workaround this by using maven profiles for the products (but still build all plugins every time) but this is not as elegant or efficient as being able to specify a single module to build.

@joeshannon
Copy link
Contributor

I have spent some time previously looking at how this might be achieved but ran into some issues. The way that Maven handles the --also-make/--also-make-dependents options is that is looks at the list of projects requested (--projects) and resolves the dependency graph, then adds any reactor projects that are required. The issue is that this happens before TychoMavenLifecycleParticipant#afterProjectsRead is invoked, this means that none of the dependency information has been injected into the MavenProjects and therefore Maven doesn't include the dependencies in the list of projects to build (unless there are dependencies in the pom.xml).

I had tried to get TychoMavenLifecycleParticipant to have access to all of the reactor projects (MavenSession#getAllProjects instead of MavenSession#getProjects) which might have some potential however there is a hint in DefaultMaven that it doesn't expect the participants to add or remove projects from the session.

@laeubi
Copy link
Member

laeubi commented Jan 21, 2022

@joeshannon this really sounds that there is some need to get support from maven first (e.g. some kind of MavenLifecycleParticipant having a method like "computeDepdentProjects" or something.
Do you think you can bring that topic to the maven-people and explain what/why this is needed?

@joeshannon
Copy link
Contributor

Yes I can try and put together a question for the Maven Developer mailing list.

@sratz
Copy link
Contributor

sratz commented Jan 21, 2022

@joeshannon this really sounds that there is some need to get support from maven first (e.g. some kind of MavenLifecycleParticipant having a method like "computeDepdentProjects" or something. Do you think you can bring that topic to the maven-people and explain what/why this is needed?

I believe the proper API for that is already available:
https://maven.apache.org/ref/3.8.3/maven-core/apidocs/org/apache/maven/graph/GraphBuilder.html

It is possible to implement (or extend the org.apache.maven.graph.DefaultGraphBuilder) and tell maven to use that via a maven extension:

@Component(role = GraphBuilder.class, hint = GraphBuilder.HINT)

.mvn/extension.xml:

<?xml version="1.0" encoding="UTF-8"?>
<extensions>
    <extension>
        <groupId>foo.bar.mavencustomizations</groupId>
        <artifactId>graphbuilder</artifactId>
        <version>0.1.0-SNAPSHOT</version>
    </extension>
</extensions>

We have been looking into this already and I believe we have a PoC of this somewhere. I can see if we can dig that out.

Maybe it would be possible for Tycho to register such an extension by default and supply a custom GraphBuilder.

@laeubi
Copy link
Member

laeubi commented Jan 21, 2022

@sratz thanks for sharing this, I think this would be indeed the right place for Tycho to participate!

Can you open a dedicated issue for this? I'll try to take a look at it then if we can improve at least here.

@laeubi
Copy link
Member

laeubi commented Jan 22, 2022

Could someone provide a small reproducer project that shows some usages of the --also-make-dependents, --resume-from and --also-make like mentioned in #551?

The smaller the project (less modules, less code) the better, so we probably can include this in the tycho integration test suite. Some bonus points could be gained by providing the project as an integration test as a PR in the first place.

@joeshannon
Copy link
Contributor

Could someone provide a small reproducer project that shows some usages of the --also-make-dependents, --resume-from and --also-make like mentioned in #551?

I have made this PR as an example but it is a bit rough around the edges.

@kriegaex
Copy link
Contributor

kriegaex commented Jan 31, 2022

Thanks @laeubi for your effort. It is impressive what you did, even though I have not tested anything yet.

Last week, I had started preparing an example Maven project with some interdependent modules in order to test on the console how building certain modules with or without additional CLI options works. It was not an automated test, but a nice playground for different scenarios. But I ran out of time before starting to OSGI-ify all modules. I would have needed to do that alongside reading an OSGi tutorial, which would have taken me much more time than I had then. Actually, the project should have had two branches: one pure JAVA and one OSGi using Tycho. That would have made the two builds comparable with regard to what happens or ought to happen when using certain CLI option in order to build certain modules.

I am happy that someone else provided an example project. I have not checked how many and which scenarios it covers, but I am hoping that all the major ones are covered.

@lorenzodallavecchia
Copy link
Author

That's really great. I'm going to test it on our build and see if it works.

Although I agree that this is not a "must-have" feature, I think it can greatly improve some workflows. For example, if that brittle integration test fails just on release day, you can just rerun part of the build. And this also plays nice with the expectations of a native Maven user.

Thanks @laeubi for your hard work. It is very much appreciated!

@laeubi
Copy link
Member

laeubi commented Jan 31, 2022

@lorenzodallavecchia maybe tycho users are just not very much used to this features, so any examples are great to share so we can see the value it brings.
At least it is a nice feature to see how tightly-coupled your plugins are if you builds one and its dependents.

@sratz
Copy link
Contributor

sratz commented Jan 31, 2022

Thanks a lot @laeubi for following up on this!

I have just tested this on our code-base and it's almost working. But somehow, whenever -am is used, everything ends up in the build, as if -pl was not specified in the first place.

See also my comment in the integration test PR #564, where this can be reproduced as well.

@laeubi
Copy link
Member

laeubi commented Feb 1, 2022

Can anyone of the participants here suggest a an useful description for the release notes? I think it is better written by someone actually using this feature so we get the wording correct.

@laeubi laeubi added this to the 2.7 milestone Feb 13, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants