Provide a way to opt out of the behavior introduced by issue 898, i.e., don't include imports in the p2 repository #1281
Comments
This is a valid choice and that's what release are for: for people who don't like to change.
You don't need a configuration option, simply remove requirements from your feature that you actually don't like to require. So you already ca configure this.
I don't think this is a valid assumptions. People simply add things, press button as they think it might fix something. So as long as you can not came up with a case where it is "needed" beside "my feature require this" ...
The previous behavior was wrong and this is no fixed. Requirements are "strictly required" so if you think they are not simply do not include them...
That's why it is noticed in the release notes if you think it needs a more exhaustive description feel free to propose a change, its always better if users write it from their point of view than the developer. |
It's not a permanent solution though. Eventually I will be forced to use a newer version because of something the platform does or provides...
As I mentioned they are there for a reason.
I do not make assumptions about my own project's code. I'm stating a fact. Those things are intentionally there and are not the result of pushing a button. Same for EPP. It's intentional to specify the additional requirements so that they are present in the p2 metadata, but definitely not intended to copy the content into the repo.
How can you say the old behavior was wrong? It's what I expect and how it's been for 1/2 a decade. The new behavior is not what I want. So right, wrong, or indifferent is not so much the issue but rather about letting me choose what I want versus taking that choice way. Instead you say I must have the new correct behavior, all else is wrong. So I must edit all my feature..xml to remove what I want to be there because you say it's no officially wrong to have it if I don't want my repo to duplicate the content of dependencies. That feels draconian and unhelpful to me...
You don't really expect that everyone paws through release note do you? |
Requirements are transtively resolved, so what do those extra requirements in feature add here? What is broken if those are removed? |
|
Specifically for EPP, this p2 metadata needed to generate the product catalog because otherwise nothing in the metadata specifies the |
No one forces you to use new features, but actually that's the way with everything in your dependency chain, CI system, Java versions, dependencies, build tools, even operating systems or hardware! At least Tycho can't afford the burden of keeping 100% backward compatibility by switches for every feature / fix
And it is intentional that Tycho includes everything that is specified in the P2 metadata.
Because the generated meta-data was incomplete. Now this is fixed and this leads to the correct behavior of including the declared requirements.
What do you think they are for if not for people reading them before upgrading to a newer release? |
I didn't realize that. But it seems so obvious now.
I have a hard time affording being downstream from
So it behaves as if includeAllRequirements is true even when that's false? If not, how is the behavior different from includeAllRequirements?
But I need that to be the case! Maybe you don't notice that problem when you are not now downstream from 5000 dependencies, but surely everyone does not want to include all their dependencies in the repositories.
Answering a question with a question. It would never occur to me that there purpose of release nodes is to be read. |
Sorry, I don't understand. |
|
in the past we deliberate moved stuff from include to import to avoid packaging it to our repository. |
|
As I said here #898 (comment) Maybe I'm missing something or forgot something, but from what I remember, such requirements in features 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 (Is it in Tycho 2.7.4?) |
|
@mickaelistri Here's such a product with such Those requirements don't end up in the p2 metadata so a p2.inf is generated to express those, i.e., the ones with filter |
|
The old behavior was not a bug but just correct and in line with common expectation! The format and the semantics of feature.xml were not invented by Tycho. I can't understand why some Tycho people decide to re-interpret these established semantics and break all their consumers. In feature.xml we can use two different elements, and , where the former installs an IU while the latter is more like a "require" without any permanent effect beyond the resolution operation. In p2 that would be greedy vs. non-greedy. Would anyone claim that there's no value in this difference?! So why are you making them behave the same? Eclipse runtime respects these semantics in an installation and PDE does so in a target platform. We can expect that our favorite build tool continues to do so, as well. If there are reasons for different behavior in special, possibly personal situations then please offer an option to opt-in to this new behavior. It's so easy to be not-disruptive to your established user base... |
No.
As explained, not all requirements are included, only those of the features you declare to be included into your update-site. If you include in your updatesites features that require "everything" then of course this "everything" is included.
If you don't want stuff to be included, simply don't include it in your feature. There is no such thing as "requirement to be packed" versus "requirement not to be packed"... there is only "requirements of this feature you included" and for that there is no difference in the P2 meta-data is simply the install-time equivalent to
what is resolved at build time. |
how can i say: if you install this feature please also install this other feature without adding a dependency to all contents of that other feature and without including it on our update site then. any why is there a and a |
Can you give any normative reference here please? I mean beside "we did it always like this" ....
Please take a look at the resulting P2 metadata, they are the same there is nothing like "more like" or "greedy", they are both represented exactly the same! The only difference is, that if you give a "not a version" aka "0.0.0" the version for the first is resolved at build time, while the later is resolved at install time.
Please take a look at the help page (emphasis by me):
So obviously this is not "without any permanent effect" it is a strict requirement and thus Tycho includes it when calculating strict requirements. |
|
@laeubi Here's a simple example
where myfeature looks like this <?xml version="1.0" encoding="UTF-8"?>
<feature
id="myfeature"
label="Myfeature"
version="1.0.0.qualifier">
<includes
id="org.eclipse.help"
version="0.0.0"/>
<requires>
<import plugin="com.google.inject"/>
<import feature="org.eclipse.jdt" version="3.18.1200.v20220607-0700"/>
</requires>
<plugin
id="mybundle"
download-size="0"
install-size="0"
version="0.0.0"
unpack="false"/>
<plugin
id="com.google.guava"
download-size="0"
install-size="0"
version="0.0.0"
unpack="false"/>
</feature>So, it INCLUDES the bundle "guava" and the feature "eclipse help", while it REQUIRES the bundle "google inject (guice)" and feature "JDT". Now, from Eclipse, run "Export deployable feature to create the corresponding P2 site" and that's the layout So you see that INCLUDED stuff is part of the P2 site (transitively), while REQUIRED stuff is not. |
By that argument shouldn't Tycho always be including all non-optional plug-in & package dependencies? Which it actually only does if "includeAllRequirments" is set to true |
Please see explanation above.
There are many thing Tycho do differently than PDE. And as you can see PDE includes much more than declared in the If you think your example is useful for Tycho as well provide an integration-test to demonstrate the use-case and we can see whats the outcome here. |
@laeubi here's from the "Plug-in Development Environment Guide":
And that's consistent with the behavior I described above (and also implemented by Tycho before this change): only included stuff is part of the p2 site |
So feature requirements should be treated differently to plugin requirements? As if I add a plugin directly to a p2 repo, without using a feature, Tycho does not include all of its requiremments, unless includeAllRequirments is true. I'm trying to understand the logic behind this change, as on the surface it appears inconsistent. |
I'm not going to let you waste my time. Everything has been said, by others and myself. You keep arguing and arguing for something that nobody wants... |
|
This change came as a huge surprise to us as well. We meticulously take care of what features/plugins to Due to this change, this suddenly changes what we are shipping. So this is a highly incompatible change for a bugfix release / micro increment of Tycho 2.7.x. As others have explained, the differentiation between include and require is significant. To summarize:
|
All requirements will be added if you choose
Me neither, feel free to use another tool that better suits what "everybody" wants, maybe PDE build? Fact is:
So instead of complaining, if someone thinks there is a problem provide an integration-test to demonstrate the issue and then we can decide of a solution. |
We've only just upgrade from 2.7.3 to 2.7.4, because there were no issues that affected us in 2.7.3. We do not use 3.0.0 yet, because we cannot use SNAPSHOT builds in our corporate release build environment.
Yes, in the end during installation it all boils down to P2 doing its thing.
I can try to provide an example of a common scenario that depicts our shipments model. |
|
@laeubi (disclaimer: it's not my intention to be offensive and direct in this comment; if I sound so, it's only due to the fact I'm not a native speaker)
I think the integration test you wrote for fixing #898 should be enough to verify this: the
What I verify in my build is that the generated p2 site can be successfully used to install my features, by relying on repository reference. I don't check that the requirements are not going to end in the p2 repository because I'm relying on the correct behavior that had been implemented up to now
Yes, there is: you asked for documentation and I quoted the documentation in comment #1281 (comment) If Tycho does more than PDE (e.g., category) we're all happy and thankful, but in this case, Tycho is doing something completely different than the documented behavior; and you see how many situations have been broken...
I think here you're talking about metadata? But for the p2 site things are different.
As I said above, I guess the integration test project is already in place. It should be enough to verify that imported bundles (well, yes, also features, but I guess verifying that for bundles should be enough) are not going into the p2 site. It's true that I could write the Java code for this verification, but it would take me a lot of time to set up the environment (I contributed to Tycho in the past, but I don't have the workspace in my compute anymore) and try to understand the API for performing such a verification. Doing that for a committer should take a few minutes since I guess you already know where to put your hands on. |
Actually it should be included as per bug description, as explained above, whether or not it was included depend on the
So I assume this still works as expected.
No that is the misunderstanding here, Tycho is not build some "Update Site", as Update Site is some old way how it was packed. So what is changed now is that Tycho generates correct P2 metadata and P2 assembles a more correct update-site ... So if one want to exclude certain content for whatever reason (e.g. all content already contained in another update-site) then this should be supported a the Assemble mojo and not at the P2-Meta data generation. For example as described by @LorenzoBettini if I don't want dependent features but only those listed in the site then one might have such an option there. But I don't see to have an |
|
i still have no clue how to do with the new way: when i want feature a to be installed i also want to have feature b installed. i dont want to repackage b cause it is a separate eclipse project. |
That's why I said we should add an integration test for that use case, because at the moment there is only an integration test that uses a bundle requirement but we (sadly) have none that has a feature requirement. So maybe a feature requirement leads to (undesired) effect that the feature is included and then the requirements of the feature that requires other feature and so on ... but that's hard to tell without having an example for exactly this. |
|
I think it's questionable to talk about 'right' versus 'wrong', and 'consistent' versus 'inconsistent' while at the same time arguing that there is no normative definition of the behavior. There is no right or wrong without a definition, there are just opinions about to the long-standing behavior of the last 5 years versus the new behavior you introduced (and I'm not sure anyone actually wants). I wonder, for example, if that new behavior is driven by a concrete use case. E.g., I wonder if there was some update site with specific content you could not achieve without this new behavior; I'm doubtful. It's certainly the case now that some of us can no longer produce the update site with restricted content that we want, with the specific metadata we want, with the new behavior where required implies inclusion in the site. Note too that having you call the update site with the exact content we want and the exact metadata we want, and always got before now, 'inconsistent' because it's not p2-complete is kind of misguided at best if not downright insulting at worst. Arguably to include all requirements of all features mentioned in the category.xml but not doing so for all bundles mentioned in the category.xml is also inconsistent behavior, but in the end 'consistency' of a site is just not a meaningful concept. Complete versus incomplete is simply not the same as consistent versus inconsistent. Hopefully people don't have to explain that repeatedly; it has been explained several times now. Stop and think for a just moment the real semantic/behavior difference between an include versus an import. An include specifies a specific exact version, not a range, and the resulting p2 requirement can only be satisfied by one specific artifact version, often one being built in the current build. If that version of that artifact is not in the assembled into p2 update site, some other very specific update site with exactly that version will need to be composed, available, or referenced when p2 installs from the site. But that's not true when a version range is specified. In that case there are an unbounded number of different artifacts that will produce a consistent installation. When I build an update site for EMF's generator, which produces Java files, it doesn't make much sense to install that feature if JDT's feature is not also installed. But that doesn't mean I want one specific version of JDT in my update site when all versions Helios through today will work nicely. I simply don't want any version of JDT in my update site. I just want JDT installed when the generator is installed, hence the requirement. But it's no longer possible to specify this range-based requirement without forcing a specific version to be composed into my update site. Note that this the third repetition of this explanation. It's also fully fleshed out here with a feature.xml: |
Signed-off-by: Christian Dietrich <christian.dietrich@itemis.de>
Signed-off-by: Christian Dietrich <christian.dietrich@itemis.de>
|
ok i guess the reproducer is showing the problem now |
|
@cdietrich But the imported (requirement) bundle should not be there either, while in your test you make sure it's there... see the layout produced by Eclipse (which is also what Tycho used to generate) in #1281 (comment) |
I recall that thread too! I think this is the one: |
|
Yes I just looked at the featurerequirement |
|
@cdietrich thanks a lot for providing the test-case, as mentioned in the PR we should slightly adjust it to cover the different cases but beside that I think its fine! |
|
I've tested our cases and confirmed the repo content with the latest 3.0.0 snapshot is the same as with Tycho 2.7.1. Thanks for the fix |
|
can confirm the Xtext build behaves as expected again too. thx for fixing @laeubi |
Signed-off-by: Christian Dietrich <christian.dietrich@itemis.de>
Signed-off-by: Christian Dietrich <christian.dietrich@itemis.de>
|
Thanks @laeubi ! Will it be ported to 2.7.5? |
It was reverted for 2.7.5-SNAPSHOT, build is running here: https://ci.eclipse.org/tycho/job/tycho-github/job/tycho-2.7.x/56/ |
Signed-off-by: Christian Dietrich <christian.dietrich@itemis.de>
|
Thanks for the revert for Tycho 2.7.x code line! I have tried to summarize the whole problem in eclipse-equinox/p2#138 (comment) |
@laeubi do you plan to release 2.7.5? |
|
There is no real "plan", so if some people test the snapshot and there is no more things to fix/back port we can start the process any time. |
|
@laeubi I tested the 2.7.5-SNAPSHOT and for it me it works fine |
See eclipse-tycho/tycho#1281 Change-Id: I5d63a10cdcf757970332041582973c2ddc29ad28 Signed-off-by: Pierre-Charles David <pierre-charles.david@obeo.fr>
Please provide a configuration option to opt out of the new behavior that was introduced by this issue:
#898
The new behavior has already broken EPP's repository assembly build:
https://bugs.eclipse.org/bugs/show_bug.cgi?id=580592#c3
Note the comment in that bug:
I think I am going to stay on older Tycho for the foreseeable future.I tested 2.7.4 with EMF's and here too the repository contains more than it has for the past 5 years of using Tycho. E.g., these plugins are now all present and I definitely don't want to duplicate such content in each EMF repository:
Only strictly included plugins (and features) ended up in the repo before but now all imported things end up in the repo as well. Worse still, I see no mechanism to fix this, other than to delete things from the feature.xml. But if I didn't need or want those imports in the feature.xml they wouldn't be there there in the first place. So I have lost significant control over what's in my repository and I need that back. Even this change of behavior as the default new behavior is rather questionable because none of us expect the repository to grow when switching to the latest and greatest Tycho version; many of us would easily fail to notice this change.
The text was updated successfully, but these errors were encountered: