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

Not all (direct) requirements of a feature are considered when building an update-site #898

Closed
laeubi opened this issue Apr 20, 2022 · 10 comments · Fixed by #1046
Closed
Milestone

Comments

@laeubi
Copy link
Member

laeubi commented Apr 20, 2022

I have a feature with the following requirement:

   <requires>
      <import plugin="com.google.guava" version="30.1.0.v20210127-2300" match="perfect"/>
   </requires>

this results in the following p2 meta-data:

    <requires size='2'>
      <required namespace='org.eclipse.equinox.p2.iu' name='com.google.guava' range='[30.1.0.v20210127-2300,30.1.0.v20210127-2300]'/>
      <required namespace='org.eclipse.equinox.p2.iu' name='feature.feature.jar' range='[1.0.0.202204191801,1.0.0.202204191801]'>
        <filter>
          (org.eclipse.update.install.features=true)
        </filter>
      </required>
    </requires>

when I now build a repository that includes this feature, the content contains the feature itself and the bundle.

Now I remove the strict version from the feature what results in:

<requires size='2'>
      <required namespace='org.eclipse.equinox.p2.iu' name='com.google.guava' range='0.0.0'/>
      <required namespace='org.eclipse.equinox.p2.iu' name='feature.feature.jar' range='[1.0.0.202204200408,1.0.0.202204200408]'>
        <filter>
          (org.eclipse.update.install.features=true)
        </filter>
      </required>
    </requires>

Now only the feature is included. This behavior is unexpected as I would either expect that in both cases a bundle is included or in no case as both are direct requirements of the feature.

It also encourages people to use strict version ranges what makes those features very inflexible, e.g. if I want to install that feature together with some other one that requires guava 30.1.0.vWhatever I will end up with two items installed.

@laeubi
Copy link
Member Author

laeubi commented Apr 20, 2022

The problem here is that we use the P2 MirrorApplication (in a slightly modified version) this uses the PermissiveSlicer that only has the option to either include all requirements or "strict" ones. But this also means that all requirements of a bundle might be included, what we need here is more a rule to include everything from feature group but not for other IUs. Currently the permissive slice does not allow such filtering (see eclipse-equinox/p2#39).

I see two options here:

  1. we can try to enhance the p2 Mirror application, that would probably require we add more options or make internal state accessible for extension
  2. we could actually replicate the parts required for tycho from the mirror application, this might be more work but is more flexible

BTW @akurtakov org.eclipse.tycho.p2.tools.mirroring.MirrorApplication sets a publishPackFilesAsSiblings is this required anymore?

@akurtakov
Copy link
Member

akurtakov commented Apr 20, 2022

As per https://www.eclipse.org/lists/cross-project-issues-dev/msg18275.html - it shouldn't be. I haven't found time to start on this though. Help is welcome - no API changes but ignore any such pack200 requests and usage internally.

@mickaelistria
Copy link
Contributor

The trick here for p2 is to determine what's a direct requirement and what's not; and what actually has to be included. The current metadata in p2 units do not allow to determine that clearly, and the publisher doesn't propagate such information from features to units. So the choice of p2 to include what has a specific version is 1 heuristic, we can change it to another, but still it would remain a heuristic with its flaws, not a complete solution.

@laeubi
Copy link
Member Author

laeubi commented Apr 20, 2022

The current metadata in p2 units do not allow to determine that clearly

If it is a group IU then it carries all requires/depends of a feature ... I think that's enough to know here, that why I opened eclipse-equinox/p2#39

@akurtakov
Copy link
Member

Will this one be fixed once we upgrade to p2 from 4.24 ? If not it will not make it to the 3.0 release.

@laeubi
Copy link
Member Author

laeubi commented May 31, 2022

Yes this requires the new P2 release first, then it could be implemented.

@laeubi laeubi linked a pull request Jun 18, 2022 that will close this issue
laeubi added a commit to laeubi/tycho that referenced this issue Jun 18, 2022
laeubi added a commit to laeubi/tycho that referenced this issue Jun 19, 2022
laeubi added a commit to laeubi/tycho that referenced this issue Jun 19, 2022
laeubi added a commit to laeubi/tycho that referenced this issue Jun 19, 2022
laeubi added a commit to laeubi/tycho that referenced this issue Jun 24, 2022
laeubi added a commit to laeubi/tycho that referenced this issue Jun 24, 2022
laeubi added a commit to laeubi/tycho that referenced this issue Jun 24, 2022
laeubi added a commit to laeubi/tycho that referenced this issue Jun 24, 2022
laeubi added a commit that referenced this issue Jun 24, 2022
@adisandro
Copy link

My update site grew overnight from 10 to 400 MB after this commit.
Admittedly, I don't understand all the intricacies of p2 packaging, but I thought that the update site should contain only my bundles even if I require other plugins/features, and I can use a "repository-reference location=..." in my site definition to fetch them from somewhere else.
Is there a flag or some option that I'm missing? I don't really want to redistribute half of Eclipse plugins in my update site.

@laeubi
Copy link
Member Author

laeubi commented Jul 7, 2022

I don't really want to redistribute half of Eclipse plugins in my update site.

Then you should not require them in your feature. Adding them there if they are transitive dependencies is not necessary and you don't get any benefit from this.

I thought that the update site should contain only my bundles

P2 don't know what are "your" bundles, so it can only consider what you require. Additional, you can choose to include even non mentioned but transitive dependencies.

@LorenzoBettini
Copy link
Contributor

I don't really want to redistribute half of Eclipse plugins in my update site.

Then you should not require them in your feature. Adding them there if they are transitive dependencies is not necessary and you don't get any benefit from this.

Maybe I'm missing something or forgot something, but from what I remember, such requirements are meant for things that are needed at runtime (while, of course, transitive dependencies for compile are usually taken care of automatically since they are required somewhere in MANIFEST). So this might look like a breaking change.

For example, I deploy a feature that is meant to be used with JDT UI, but I never use JDT in my code: I specify JDT feature as a requirement in my feature (not as included). Then, I'll also add a repository reference in my category to make things easier for the consumers of my update site.

I still haven't tested this feature (it's not released yet, is it?)

@cdietrich
Copy link
Contributor

cdietrich commented Aug 23, 2022

we also see some additional unwanted stuff popping up here
https://ci.eclipse.org/xtext/job/xtext-umbrella/job/cd_tycho300test/lastSuccessfulBuild/artifact/build/p2-repository/plugins/
is there another means to e.g. specify versions for transtive deps features?

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