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

Force qualifier update for p2.core.feature #107

Merged
merged 1 commit into from
Jul 8, 2022

Conversation

mickaelistria
Copy link
Contributor

As some included bundles have changed with the move away from Orbit to
Maven.

As some included bundles have changed with the move away from Orbit to
Maven.
@mickaelistria mickaelistria merged commit e26e040 into eclipse-equinox:master Jul 8, 2022
@laeubi
Copy link
Member

laeubi commented Jul 9, 2022

@mickaelistria Tycho now supports the inclusion of dependencies in features and thus you might want use an require with version range for non-p2 requirements to avoid such bumps in the future.

@HannesWell
Copy link
Member

@mickaelistria Tycho now supports the inclusion of dependencies in features and thus you might want use an require with version range for non-p2 requirements to avoid such bumps in the future.

@laeubi can you provide a link to the Release notes or plugin doc that describes that feature?
Since I recently added many OSGi plugins to many features of equinox, p2 and platform it sounds like that could be converted to imports? If I do that I probably would do that for all included dependencies.

@laeubi
Copy link
Member

laeubi commented Jul 9, 2022

See for example eclipse-equinox/equinox#67

I think for every "external" dependency, that is not part of the build, an <import plugin="..."/> (probably with a version range) would be suitable.

@mickaelistria
Copy link
Contributor Author

Eclipse p2 repository does set includeAllDependencies=true, so we could even just remove all transitive deps from features.

@merks
Copy link
Contributor

merks commented Jul 9, 2022

This feels dejavu. I think the last time we were here, we ended up losing the source bundles did we not? That was not so many weeks ago. Did something change since then? The Eclipse SDK will not be a good SDK anymore (or even a good platform for that matter) if the source bundles for everything are not available in its p2 update site.

@laeubi
Copy link
Member

laeubi commented Jul 9, 2022

@merks I have no clue what particular dejavu is referenced here but if one could share a minimal example I can check if something needs to be fixed in Tycho, at least the includeAllDependencies option is more about binary artifacts included, but when we have a source feature (is there one generated?) I think it should be possible to include the sources as well there.

But even an includeSources option would be something useful, BUT if we are talking about maven artifacts, it might be more valuable to

  1. Finally fix PDE to allow "non-source-bundle-sources"
  2. Add a hook in PDE / JDT that m2e could jump into to simply fetch the source from maven central directly.

@merks
Copy link
Contributor

merks commented Jul 10, 2022

@laeubi I think the other folks will recall this thread:

eclipse-equinox/equinox.framework#41 (comment)

I'm not sure I understand what non-source-bundles-sources are. In any case, if we are going to introduce a new problem (missing sources) that can only be fixed by a not-yet-implemented feature along with a feature implemented in a project that is not part of the Eclipse SDK, then I think we'd be best off to avoid introducing the problem of missing sources in the first place.

As mentioned in the other thread, a common use case is that folks install an Eclipse SDK and then use the running IDE as their target platform, expecting all the sources to be available because that's what an SDK's purpose is and that expectation has been valid. After all, why install any sources in an IDE? Another common use case is to use the platform's p2 update site to build a target platform and again, missing sources will lead to problems for that use case as well.

@laeubi
Copy link
Member

laeubi commented Jul 10, 2022

I'm not sure I understand what non-source-bundles-sources are.

If you have a plain (or maven) project, you can attach any source-jar to a classpath entry. PDE requires that a source-jar is a bundle with a special header, what makes it very unflexible, so Tycho currently generates an extra-jar for maven dependencies or generates a special additional bundle for bundel-projects.

After all, why install any sources in an IDE?

Correct, that does not make much sense, so it would be better if they are not installed like its done with JDT / maven projects :-)

expecting all the sources to be available

Question is, what are "all" sources, should it really include any external library? Maybe even the sources of the JDK used? Sources of DLLs?

So one usually can choose when building an update-site / feature what should be included, and technically a maven dependency is not different from a P2 dependency (actually Tycho don't know the real source in its target) so it seems something in building this SDK stuff is missing something, or generation of the source bundles is disabled in the target location.

@merks
Copy link
Contributor

merks commented Jul 10, 2022

I'm not sure I understand what non-source-bundles-sources are.

If you have a plain (or maven) project, you can attach any source-jar to a classpath entry. PDE requires that a source-jar is a bundle with a special header, what makes it very unflexible, so Tycho currently generates an extra-jar for maven dependencies or generates a special additional bundle for bundel-projects.

So there is a problem not yet addressed with no actual plans to address it nor people to do the work...

After all, why install any sources in an IDE?

Correct, that does not make much sense, so it would be better if they are not installed like its done with JDT / maven projects :-)

It's what people actually do. And these people are the community that we wish to maintain or even grow.

expecting all the sources to be available

Question is, what are "all" sources, should it really include any external library? Maybe even the sources of the JDK used? Sources of DLLs?

All is all to the extent that such sources exist. The JDK also comes with sources.

So one usually can choose when building an update-site / feature what should be included, and technically a maven dependency is not different from a P2 dependency (actually Tycho don't know the real source in its target) so it seems something in building this SDK stuff is missing something, or generation of the source bundles is disabled in the target location.

As far as I understand it, inclusion of a bundle in a feature, specifying that Tycho generate a corresponding source feature, and including both those features in the category.xml is what results in the source bundles of all included bundles being included in the p2 update site. That is why I'm concerned that changing includes to imports will result in missing source bundles. (We have discussed before that includes are different from imports when you suggested removing that concept.) Someone can correct me if this is a mistaken understanding and that corresponding source bundles will not go AWOL.

@laeubi
Copy link
Member

laeubi commented Jul 10, 2022

That is why I'm concerned that changing includes to imports will result in missing source bundles.

The best is to provide an integration-test to demonstrate the issue so we can make sure it works (or not) can enhance/fix and make sure it do never break in the future.

Basically, the source-feature generator just adds new entries to the XML, so I don't see a reason why we should not be able to do the same for imports! (but we for sure need at least the import so Tycho can know about it!)

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.

4 participants