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

Please don't transitively reference a p2 repository with abitrary JustJ JRE versions #1751

Open
merks opened this issue May 6, 2024 · 21 comments · Fixed by #1753
Open

Please don't transitively reference a p2 repository with abitrary JustJ JRE versions #1751

merks opened this issue May 6, 2024 · 21 comments · Fixed by #1753

Comments

@merks
Copy link
Contributor

merks commented May 6, 2024

In a 2024-03 package, these are the URIs added automatically.

image

If I disable m23 and do check for updates, it grows to this list:

image

With m2e's update site enabled, check for updates grows to this list:

image

With WWD in the list, we end up with this problem:

eclipse-equinox/p2#508

We really would like to avoid that 10,000's of users end up updating their 2024-03 installation to 2024-06 only to find it doesn't start because they aren't using Java 21.

WWD says the problem is fixed:

eclipse-wildwebdeveloper/wildwebdeveloper#1380 (comment)

We just need to be sure that m2e doesn't not transitively drag in a p2 repository with a JRE > 17 but that p2 does not actually use to install a JRE > 17 but rather just makes p2 not complain. Without the m2e repository in the list, the update only updates some things:

image

FYI @jonahgraham

@HannesWell
Copy link
Contributor

We just need to be sure that m2e doesn't not transitively drag in a p2 repository with a JRE > 17 but that p2 does not actually use to install a JRE > 17 but rather just makes p2 not complain. Without the m2e repository in the list, the update only updates some things:

Would it be sufficient to just update to Wild Web Developer 1.3.4 ?
If we have to remove the reference we either have to mirror the WWD artifacts in the m2e repo or have to give up its self-containment (but in that case it would probably be easier to just mirror it)

@mickaelistria
Copy link
Contributor

Would it be sufficient to just update to Wild Web Developer 1.3.4 ?

I hope and believe it would.

@merks
Copy link
Contributor Author

merks commented May 7, 2024

Yes, probably. But we'll need to test it.

HannesWell added a commit to HannesWell/m2e-core that referenced this issue May 7, 2024
HannesWell added a commit to HannesWell/m2e-core that referenced this issue May 7, 2024
@HannesWell
Copy link
Contributor

Yes, probably. But we'll need to test it.

Then lets test #1753.

HannesWell added a commit to HannesWell/m2e-core that referenced this issue May 7, 2024
HannesWell added a commit to HannesWell/m2e-core that referenced this issue May 8, 2024
HannesWell added a commit to HannesWell/m2e-core that referenced this issue May 9, 2024
HannesWell added a commit that referenced this issue May 9, 2024
@merks
Copy link
Contributor Author

merks commented May 9, 2024

Could you share a link to an update site that is representative of the update site that is referenced by the touchpoint-added update site for the release? It will be good to validate the solution is effective.

@HannesWell
Copy link
Contributor

Could you share a link to an update site that is representative of the update site that is referenced by the touchpoint-added update site for the release? It will be good to validate the solution is effective.

There is currently only a snapshot at

If you would like to have an immutable contribution to SimRel I can create one.

@merks
Copy link
Contributor Author

merks commented May 9, 2024

No I can work with that just fine.

@merks
Copy link
Contributor Author

merks commented May 9, 2024

Replacing the update site with the newer m2e one, after an update check "floods" the list with everything that m2e chooses to use in it's builds:

image

But at least it no longer offers to do an update that would fail:

image

So that's a good thing, though the feedback is confusing and poor....


I think the behavior of me2's one additional update site

image

effectively injecting the update channels for a whole whack of other projects

image

is just asking for problems. Is it really m2e's business to add a specific EMF update site to the EPP product's updates?

And what is https://download.eclipse.org/e4/snapshots/org.eclipse.e4.ui doing in this list I wonder? I think it's reasonable to argue that if m2e causes it to be added me2 ought to track down where it's coming from and getting rid it...

FYI @jonahgraham

@laeubi
Copy link
Member

laeubi commented May 9, 2024

Sorry for little rant here BUT

Replacing the update site with the newer m2e one, after an update check "floods" the list

but first m2e is not 'flooding' anything here, it is P2 and it is a known problem but no one seem to care enough to mitigate this. So please don't blame m2e for bugs (some call it a feature) of P2 ...

with everything that m2e chooses to use in it's builds

It includes a reference to repositories where artifacts can potentially be found that are a direct requirement of m2e and we don't want to mirror them and we don't want to fore people to add all that sites manually. The choice to add them as a top user visible updatesite that is queried for additional updates is done by P2, so don't blame m2e for this P2 "feature" (what is a bug in my opinion).

And what is https://download.eclipse.org/e4/snapshots/org.eclipse.e4.ui doing in this list I wonder?

Who knows, its not part of the references if the m2e site.

I think it's reasonable to argue that if m2e causes it to be added me2 ought to track down where it's coming from and getting rid it...

Not really... again m2e is doing nothing here its P2, if one want it should be doable to have a tool that traverses all references and print the path to such site.

@HannesWell
Copy link
Contributor

I wouldn't have ranted here, but I share Christophs opinion.
We added the references in order to provide a self-contained repository without mirroring all dependencies to reduce the required disk-space at the EF and potential mirrors.

At the time this was done I wasn't aware that P2 adds all references of a repo as available software-site in the UI and it wasn't intended. I recall there was some discussion about that in P2 but I didn't follow that discussion yet. So if it where possible to mark the references as hidden/secondary/passive (IIRC the default can't be changed) I would be happy to do that.
Respectively I would change the Tycho feature to automatically compute the references from the TP to mark the references as such.

@HannesWell
Copy link
Contributor

And what is https://download.eclipse.org/e4/snapshots/org.eclipse.e4.ui doing in this list I wonder? I think it's reasonable to argue that if m2e causes it to be added me2 ought to track down where it's coming from and getting rid it...

That's indeed unexpected, but it is not in m2e's repository

<repository name='Maven Integration for Eclipse Repository' type='org.eclipse.equinox.internal.p2.metadata.repository.LocalMetadataRepository' version='1'>
  <properties size='2'>
    <property name='p2.timestamp' value='1715257007878'/>
    <property name='p2.compressed' value='true'/>
  </properties>
  <references size='18'>
    <repository uri='https://download.eclipse.org/eclipse/updates/4.31' url='https://download.eclipse.org/eclipse/updates/4.31' type='0' options='1'/>
    <repository uri='https://download.eclipse.org/eclipse/updates/4.31' url='https://download.eclipse.org/eclipse/updates/4.31' type='1' options='1'/>
    <repository uri='https://download.eclipse.org/egit/updates-6.9' url='https://download.eclipse.org/egit/updates-6.9' type='0' options='1'/>
    <repository uri='https://download.eclipse.org/egit/updates-6.9' url='https://download.eclipse.org/egit/updates-6.9' type='1' options='1'/>
    <repository uri='https://download.eclipse.org/lsp4e/releases/0.26.0' url='https://download.eclipse.org/lsp4e/releases/0.26.0' type='0' options='1'/>
    <repository uri='https://download.eclipse.org/lsp4e/releases/0.26.0' url='https://download.eclipse.org/lsp4e/releases/0.26.0' type='1' options='1'/>
    <repository uri='https://download.eclipse.org/lsp4j/updates/releases/0.22.0' url='https://download.eclipse.org/lsp4j/updates/releases/0.22.0' type='0' options='1'/>
    <repository uri='https://download.eclipse.org/lsp4j/updates/releases/0.22.0' url='https://download.eclipse.org/lsp4j/updates/releases/0.22.0' type='1' options='1'/>
    <repository uri='https://download.eclipse.org/modeling/emf/emf/builds/release/2.37.0' url='https://download.eclipse.org/modeling/emf/emf/builds/release/2.37.0' type='0' options='1'/>
    <repository uri='https://download.eclipse.org/modeling/emf/emf/builds/release/2.37.0' url='https://download.eclipse.org/modeling/emf/emf/builds/release/2.37.0' type='1' options='1'/>
    <repository uri='https://download.eclipse.org/mylyn/docs/releases/3.0.48' url='https://download.eclipse.org/mylyn/docs/releases/3.0.48' type='0' options='1'/>
    <repository uri='https://download.eclipse.org/mylyn/docs/releases/3.0.48' url='https://download.eclipse.org/mylyn/docs/releases/3.0.48' type='1' options='1'/>
    <repository uri='https://download.eclipse.org/tm4e/releases/0.11.0' url='https://download.eclipse.org/tm4e/releases/0.11.0' type='0' options='1'/>
    <repository uri='https://download.eclipse.org/tm4e/releases/0.11.0' url='https://download.eclipse.org/tm4e/releases/0.11.0' type='1' options='1'/>
    <repository uri='https://download.eclipse.org/webtools/downloads/drops/R3.33.0/R-3.33.0-20240304165142/repository' url='https://download.eclipse.org/webtools/downloads/drops/R3.33.0/R-3.33.0-20240304165142/repository' type='0' options='1'/>
    <repository uri='https://download.eclipse.org/webtools/downloads/drops/R3.33.0/R-3.33.0-20240304165142/repository' url='https://download.eclipse.org/webtools/downloads/drops/R3.33.0/R-3.33.0-20240304165142/repository' type='1' options='1'/>
    <repository uri='https://download.eclipse.org/wildwebdeveloper/releases/1.3.4' url='https://download.eclipse.org/wildwebdeveloper/releases/1.3.4' type='0' options='1'/>
    <repository uri='https://download.eclipse.org/wildwebdeveloper/releases/1.3.4' url='https://download.eclipse.org/wildwebdeveloper/releases/1.3.4' type='1' options='1'/>
  </references>

Indeed the EGit repository has that reference (having searched further after finding it in EGit):

<repository name='EGit P2 Repository' type='org.eclipse.equinox.internal.p2.metadata.repository.LocalMetadataRepository' version='1'>
  <properties size='2'>
    <property name='p2.timestamp' value='1709634611930'/>
    <property name='p2.compressed' value='true'/>
  </properties>
  <references size='4'>
    <repository uri='https://download.eclipse.org/e4/snapshots/org.eclipse.e4.ui' url='https://download.eclipse.org/e4/snapshots/org.eclipse.e4.ui' type='0' options='1'/>
    <repository uri='https://download.eclipse.org/e4/snapshots/org.eclipse.e4.ui' url='https://download.eclipse.org/e4/snapshots/org.eclipse.e4.ui' type='1' options='1'/>
    <repository uri='https://download.eclipse.org/tools/orbit/simrel/orbit-aggregation/release/4.30.0' url='https://download.eclipse.org/tools/orbit/simrel/orbit-aggregation/release/4.30.0' type='0' options='1'/>
    <repository uri='https://download.eclipse.org/tools/orbit/simrel/orbit-aggregation/release/4.30.0' url='https://download.eclipse.org/tools/orbit/simrel/orbit-aggregation/release/4.30.0' type='1' options='1'/>
  </references>
...

if one want it should be doable to have a tool that traverses all references and print the path to such site.

That would be helpful and should not be to complicated.

@merks
Copy link
Contributor Author

merks commented May 10, 2024

I wouldn't have ranted here either because I know it is kind of pointless. But this has cost me a day of effort so far and I can foresee this causing problems in the future just as it has caused problems in the past. When there is the significant potential that 10,000s of users might end up with IDEs that don't start in June, proactive steps needs to be taken. The users care not one hoot who is to blame for problems. All that matters is to eliminate and avoid problems.

As for p2's known problems, the fact p2 adds referenced repositories to the preferences is by design, see org.eclipse.equinox.internal.p2.metadata.repository.LocalMetadataRepository.publishRepositoryReferences(). In any case, that is not actually relevant because even if p2 didn't make those repositories visible in the preferences, it would still be using them under the covers. After all that is the point of the references and that is why m2e is specifying them, i.e., that the p2 planners sees them and uses them. That is the underlying issue. So the URIs not being made visible in the preferences would have just make it even harder to know from where the content is coming; likely I would have spent two days tracking down phantom problems instead of just one.

Anyway, it is pointless for me to rant here because m2e will do whatever it chooses. The unfortunate consequence is that how well m2e manages its repository references has a direct impact on the EPP packages because m2e adds its update site everywhere whether I like it or not. So everyone is stuck with that choice. When that causes problems, who tracks those down before they create a super bad impression that "every new Eclipse release breaks something"? Apparently that would be me.

Even the fact that m2e apparently builds only against released dependencies suggests that m2e will not notice when new versions of dependencies cause breakage. For example, are you really getting mylyn docs from the right place? I.e., https://download.eclipse.org/mylyn/docs/releases/3.0.48 versus https://download.eclipse.org/mylyn/updates/release/4.2.0/ I wonder? Note the major version increase. Anyway, this continues to be ranting that will achieve nothing because this is how m2e wants it and I do not have the capacity to ensure this this doesn't go astray. I can only hope that this will be properly managed but even the Mylyn thing doesn't give me a warm fuzzy feeling.

@laeubi
Copy link
Member

laeubi commented May 10, 2024

In any case, that is not actually relevant because even if p2 didn't make those repositories visible in the preferences, it would still be using them under the covers.

This is only partially true, P2 will always use them to resolve dependencies and download artifacts, but it will not use them to discover updates.

For that purpose there is RepositoryTracker.getKnownRepositories(ProvisioningSession) that queries the repositories that are "known" if we now look at ColocatedRepositoryTracker it specifically asks for metadata repositories of type IRepositoryManager.REPOSITORIES_NON_SYSTEM what leads to IRepository.PROP_SYSTEM that is described as:

/**
 * The key for a boolean property indicating that the repository
 * is a system repository.  System repositories are implementation details
 * that are not subject to general access, hidden from the typical user, etc.
 * This property is never stored in the repository itself, but is instead tracked and
 * managed by an {@link IRepositoryManager}.
 * @see IRepositoryManager#getRepositoryProperty(URI, String)
 */

so I think it is fair to say that your initial list is defiantly that what is "subject to general access" as it was either added by the user or the product creator, but whats happens after the update is " implementation details that are not subject to general access, hidden from the typical user" as these are just referenced from the user set and should not be visible nor be queried afterwards for additional updates. And this is for me clearly a bug in P2 that is worth being fixed there instead of on trying to force hundred of subprojects into a certain (manual) management of their update sites.

Because that's what really is an issue, that next times after this update check additional things are checked for updates and possibly suggesting wild combinations or incompatible / unwanted updates.

Why do I know that? Because this already break one of my products and I now maintain an own list of "KnownRepositories" I use for the Update Operation and everything runs clean and smooth...

So as a concrete WWD case:
If I add WWD and WWD references JustJ it can install JustJ if it is a requirement, but it should not add JustJ to the user visible list because then afterwards it will not only check for WWD updates but also for JustJ updates.

The same applies to m2e that references WWD or whatever ... it should check for m2e updates but never for any "referenced" and that's also what happens on the first update check.

@merks
Copy link
Contributor Author

merks commented May 10, 2024

I suspect you have a point about further updates in the future once the site is in the list. For the case that prompted this "witch hunt", the problem is that a Java 22 EE provider was found immediately leading to an update that doesn't launch.


Don't overlook my comment about Mylyn. What's up with that? Will it be forgotten if I don't open an issue? Will m2e publish its release to reference only the 2024-06 releases of all its referenced dependencies? I'm highly doubtful because you'd have to wait quite long for those all to exist.


Also note that I have yet more work now to ensure that Egit to manages its references properly. I hope not to have to police 70 projects each doing similar things but not doing it really well because at some point this all exceeds my capacity.

@laeubi
Copy link
Member

laeubi commented May 10, 2024

I suspect you have a point about further updates in the future once the site is in the list. For the case that prompted this "witch hunt", the problem is that a Java 22 EE provider was found immediately leading to an update that doesn't launch.

I don't want to say that this will magically fix everything, but the real issue causing thing is after the reference become visible, especially even if m2e / wwd / jgit / ...70 other project ... would remove it they will stay forever unless deleted by the user (that never added them what makes them to complain!

Don't overlook my comment about Mylyn.

Regarding mylin (where I don't know if/why m2e require it) and other dependencies m2e will reference what it requires for that release, when it is updated (like WWD) it will be updated (automatically) as well, and m2e is always Release-X (where X >= 1) this is not an issue as then units with higher version will simply override the ones from the references, so a reference (for me) is more like a composite (and should behave like that). This has been the case for some releases now and never caused any issues.

Also note that I have yet more work now to ensure that Egit to manages its references properly.

That's the point mentioned above, managing things manually is utterly hard, prone to errors, easy to get wrong and therefore m2e uses an automated approach (what might has other pitfalls maybe) what at least ensures that we do not publish outdated / wrong / unused references other people then have to discover and/or fix.

@HannesWell
Copy link
Contributor

Sorry, I don't have much time to answer and no time do anything over the weekend but I'll have a look at all your points at the beginning of next week to find short and long term solutions.

And thank you for being pro-active, that's definitely better for everyone. The current state was done in good faith but it looks like it needs to be enhanced.
Maybe just referencing only the artifact repos could be a solution?

@merks
Copy link
Contributor Author

merks commented May 10, 2024

There’s no rush. I do appreciate the responsiveness.

One needs the content metadata for the planner to make the plan.

@HannesWell HannesWell reopened this May 10, 2024
@laeubi
Copy link
Member

laeubi commented May 11, 2024

I have now created a PR that fixes the issue of growing sites due to referenced repositories:

I think there is not much more to do for m2e here. While only reference the artifact repos would work if one then mirrors the metadata (what is much smaller) I'm not convinced it is a good idea as even if we should use stable sites, any change in the metadata of the reference might mess up things and leading to obscure "No repository contains artifact" errors in the worst case.

@HannesWell
Copy link
Contributor

I have now created a PR that fixes the issue of growing sites due to referenced repositories:

* [Remember referenced repositories as system repositories instead of user eclipse-equinox/p2#512](https://github.com/eclipse-equinox/p2/pull/512)

@merks are you fine with the current state and this can be closed as completed or should at least the reference to the WWD repo be removed and their artifacts mirrored instead?

I think there is not much more to do for m2e here. While only reference the artifact repos would work if one then mirrors the metadata (what is much smaller) I'm not convinced it is a good idea as even if we should use stable sites, any change in the metadata of the reference might mess up things and leading to obscure "No repository contains artifact" errors in the worst case.

Yes that's what I meant in my previous comment, create a self-contained metadata repository (with the known includeAllDependencies but remove the artifacts from other projects and only reference their artifact repository.
I think that should lead us closer to the desired state: Only the 'metadata' required by m2e is contained in m2e's repo but the artifacts itself don't need to be mirrored.

If the referenced repos change, well that's indeed a problem. But it is already now a problem although at the moment it would lead to resolution errors. If the error messages in case only the artifact is missing are not clear enough, they should be improved. But in general I would not say one error is worse than the other. In both cases users have to add another repository by themself.

@merks
Copy link
Contributor Author

merks commented May 14, 2024

Has the mylyn repository reference been updated to actually get the latest proper wiki text ?

Other than that, with @laeubi’s improvements to p2, we are on a better track.

@HannesWell
Copy link
Contributor

Has the mylyn repository reference been updated to actually get the latest proper wiki text ?

Not yet, thanks for the reminded. My mailbox was flooded yesterday and I forgot that point.
Here we go: #1755

I only had to add it to make lsp4e resolve. It would be great if the lsp4e repo would also provide it directly by mirroring or indirectly using a corresponding repo-reference.

Using the same configuration of the tycho-p2-repository-plugin as M2E makes this really simple and ensures the repo uses always references the same URLs as the target. Of course then one should not use changing repositories!

<includeAllDependencies>true</includeAllDependencies>
<includeAllSources>true</includeAllSources>
<filterProvided>true</filterProvided>
<addIUTargetRepositoryReferences>true</addIUTargetRepositoryReferences>
<repositoryReferenceFilter>
<addOnlyProviding>true</addOnlyProviding>
<exclude>
<location>https://download.eclipse.org/tools/orbit/**</location>
</exclude>
</repositoryReferenceFilter>

Other than that, with @laeubi’s improvements to p2, we are on a better track.

Great 🎉

kysmith-csg pushed a commit to kysmith-csg/m2e-core that referenced this issue Jun 3, 2024
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.

4 participants