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

Feature restrictions are not taken into account when using emptyVersion #845

Closed
laeubi opened this issue Apr 5, 2022 · 13 comments · Fixed by #846
Closed

Feature restrictions are not taken into account when using emptyVersion #845

laeubi opened this issue Apr 5, 2022 · 13 comments · Fixed by #846
Assignees
Labels
🚨 regression Regression compared to previous release - blocks upcoming release and other merges sponsored The bugfix or feature was gently sponsored by an individual or company
Milestone

Comments

@laeubi
Copy link
Member

laeubi commented Apr 5, 2022

Given a feature like this:

<?xml version="1.0" encoding="UTF-8"?>
<feature
      id="com.test.sample.feature"
      version="1.0.0">
   <includes id="com.test.base.feature" version="0.0.0"/>
   <requires>
      <import feature="com.test.base.feature" match="compatible" version="1.0.0"/>
   </requires>
</feature>

according to the docs a match rule of match="compatible" means:

dependent plug-in version must be at least at the version specified, or at a higher service level or minor level (major version level must equal the specified version).

So here one would expect that only a version < 2 is considered a valid match, but Tycho still tries to resolve e.g. a version 2.0 and fails in this case if not all requirements for 2.0 are fulfilled, there is an example project here: https://github.com/and-k/tycho-issue-new

As a workaround one could specify <includes id="com.test.base.feature" version="1.0.0"/> but this duplicates the information from the restriction therefore it seems to be related to the use of the emptyVersion here.

FYI @and-k

@laeubi
Copy link
Member Author

laeubi commented Apr 5, 2022

The error one sees here is:

[ERROR] Cannot resolve project dependencies:
[ERROR] Software being installed: com.test.sample.product 1.0.0.qualifier
[ERROR] Missing requirement: com.test.base.feature.feature.group 2.0.0 requires 'org.eclipse.equinox.p2.iu; org.apache.activemq.activemq-core [5.4.0,5.4.0]' but it could not be found
[ERROR] Cannot satisfy dependency: com.test.sample.feature.feature.group 1.0.0 depends on: org.eclipse.equinox.p2.iu; com.test.base.feature.feature.group [2.0.0,2.0.0]
[ERROR] Cannot satisfy dependency: com.test.sample.product 1.0.0.qualifier depends on: org.eclipse.equinox.p2.iu; com.test.sample.feature.feature.group [1.0.0,1.0.1)

The important part is com.test.base.feature.feature.group 2.0.0 requires 'org.eclipse.equinox.p2.iu; org.apache.activemq.activemq-core [5.4.0,5.4.0]' but it could not be foundwhere one can see that the version 2 is chosen for com.test.base.feature.feature (what has an unsolvable requirement here on intention).

Also to note is, that this error only occurs on the assemble-repository phase and not at the resolution phase of the feature and/or product, so probably this is not a bug of Tycho but of p2 or a combination of both.

@laeubi
Copy link
Member Author

laeubi commented Apr 5, 2022

This is a regression somewhere between Tycho 0.22.0 and Tycho 0.26.0 where it fails the first time with this error.

@laeubi laeubi added the 🚨 regression Regression compared to previous release - blocks upcoming release and other merges label Apr 5, 2022
@laeubi
Copy link
Member Author

laeubi commented Apr 5, 2022

Looking at the p2content.xml for the feature in Tycho 0.22.0 the relevant part looks like this:

    <requires size='3'>
      <required namespace='org.eclipse.equinox.p2.iu' name='com.test.base.feature.feature.group' range='[1.0.0,1.0.0]'/>
      <required namespace='org.eclipse.equinox.p2.iu' name='com.test.base.feature.feature.group' range='[1.0.0,2.0.0)'/>
      <required namespace='org.eclipse.equinox.p2.iu' name='com.test.sample.feature.feature.jar' range='[1.0.0,1.0.0]'>
        <filter>
          (org.eclipse.update.install.features=true)
        </filter>
      </required>
    </requires>

From Tycho 0.26.0 on it looks like this:

    <requires size='3'>
      <required namespace='org.eclipse.equinox.p2.iu' name='com.test.base.feature.feature.group' range='[2.0.0,2.0.0]'/>
      <required namespace='org.eclipse.equinox.p2.iu' name='com.test.base.feature.feature.group' range='[1.0.0,2.0.0)'/>
      <required namespace='org.eclipse.equinox.p2.iu' name='com.test.sample.feature.feature.jar' range='[1.0.0,1.0.0]'>
        <filter>
          (org.eclipse.update.install.features=true)
        </filter>
      </required>
    </requires>

As one can see the required range now chooses the largest version (2.0) as an exact version in contrast to state before.
As it is actually impossible that all could be fulfilled together:

  1. require feature with version 2
  2. require feature with version 1 or higher but excluding 2
  3. requiring feature with version 1

so this actually will result in both versions installed because only that way all requirements could be fulfilled.

laeubi added a commit to laeubi/tycho that referenced this issue Apr 5, 2022
Signed-off-by: Christoph Läubrich <laeubi@laeubi-soft.de>
@laeubi laeubi self-assigned this Apr 5, 2022
@laeubi laeubi added the sponsored The bugfix or feature was gently sponsored by an individual or company label Apr 5, 2022
@laeubi
Copy link
Member Author

laeubi commented Apr 7, 2022

One observation:
the p2content.xml is generated at the p2-metadata-default phase. At that stage a jar is already build with the feature included and that where the 2.0.0 version originates from:

Original feature.xml:

<?xml version="1.0" encoding="UTF-8"?>
<feature id="com.test.sample.feature" version="1.0.0">
   <includes id="com.test.base.feature" version="0.0.0"/> 
   <requires>
      <import feature="com.test.base.feature" match="compatible" version="1.0.0"/>
   </requires>
</feature>

Target (and packed) feature:

<?xml version="1.0" encoding="UTF-8"?>
<feature id="com.test.sample.feature" version="1.0.0">
   <includes id="com.test.base.feature" version="2.0.0"/> 
   <requires>
      <import feature="com.test.base.feature" match="compatible" version="1.0.0"/>
   </requires>
</feature>

So the error is not in the p2metadata generation but in the packaging phase where the version is already replaced and thus p2 can't know about the initial emptyVersion restriction.

This also explains why setting the version to 1.0.0 changes the behavior, as then no automatic version expansion is performed by tycho.

@mickaelistria
Copy link
Contributor

Feature inclusions are meant to use fully qualified versions. I think that has been true about forever, even before p2. This constraint could be removed, but it would probably have to happen in p2 publisher first.

@laeubi
Copy link
Member Author

laeubi commented Apr 7, 2022

There is special code to expand those versions in Tycho and 0.0.0 is actually the default in PDE, if I click on the version button it reads "synchronize versions on build" so I think this is actually desired behavior?

Anyways I never understand what are the difference between includes and requires as in the end both end up in the P2 metadata, so it is no real "inclusion" as the description states...

@mickaelistria
Copy link
Contributor

It's pre-p2 legacy. Includes used to tell one particular feature was to be included in every update-site together with the parent feature; and this was resolved at build-time. But with p2, it matters much less, and requires/import is usually leading to better results.

@laeubi
Copy link
Member Author

laeubi commented Apr 7, 2022

So probably we should adjust PDE to no longer offer this option in the UI and convert old feature formats on save?

@laeubi
Copy link
Member Author

laeubi commented Apr 7, 2022

Just another though, before I'm fixing to much legacy:

Would it be valid to simply drop the includes if there is an additional import with the same ID?

@mickaelistria
Copy link
Contributor

The includes is expected to replace the version by the specific version that's available at build time and use specific version in the dependencies. includes is then a very special and more restrictive form of requires/import. I'm not sure we can easily drop it as it should be higher priority in most cases.

@laeubi
Copy link
Member Author

laeubi commented Apr 7, 2022

Actually P2 handles them equally, it results in

<required namespace='org.eclipse.equinox.p2.iu' name='com.test.base.feature.feature.group' range='[2.0.0,2.0.0]'/>
<required namespace='org.eclipse.equinox.p2.iu' name='com.test.base.feature.feature.group' range='[1.0.0,2.0.0)'/>

so from the p2 metadata it seems duplicated and leads to a strange result where version 2 and version 1 is included, I don't think that's what intended here.

So my idea is if there is an emptyVersion in the includes and a requires with the same id, we can simply drop the includes what will result in

<required namespace='org.eclipse.equinox.p2.iu' name='com.test.base.feature.feature.group' range='[1.0.0,2.0.0)'/>

@laeubi
Copy link
Member Author

laeubi commented Apr 7, 2022

I also wonder if the same is true for the plugin should one actually not use that as well but

  <requires>
      <import plugin="..."/>
   </requires>

instead?

laeubi added a commit to laeubi/tycho that referenced this issue Apr 7, 2022
…nt when using

emptyVersion

Signed-off-by: Christoph Läubrich <laeubi@laeubi-soft.de>
laeubi added a commit that referenced this issue Apr 7, 2022
emptyVersion

Signed-off-by: Christoph Läubrich <laeubi@laeubi-soft.de>
@laeubi
Copy link
Member Author

laeubi commented Apr 7, 2022

The fix for this issue was gently sponsored by Compart AG.

laeubi added a commit that referenced this issue Apr 7, 2022
@laeubi laeubi added this to the 3.0 milestone Sep 16, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🚨 regression Regression compared to previous release - blocks upcoming release and other merges sponsored The bugfix or feature was gently sponsored by an individual or company
Projects
None yet
2 participants