-
Notifications
You must be signed in to change notification settings - Fork 41
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
Add actions that publish IUs used in the build #354
base: master
Are you sure you want to change the base?
Conversation
Since this is specific to and tightly coupled to PDE, wouldn't it then make sense to add it to PDE? |
I don't think that's suitable here with the current state of PDE as it will bind to major parts of PDE in the worst case and is hard to discover/use for P2 users. Maybe one time we will have an |
That's right, but I think it depends on who the target audience is. Can anybody assess if there is realistically a user besides Oomph? |
I would have expected something like this to simply be integrated as part of the existing BundlesAction such that the existing APIs "just work" with either "format". That would seem better. After all, these APIs are there to be able to map a folder representing the source of an OSGi bundle into the corresponding p2 installable unit so ideally would not need to know directly ahead of time if the folder is using a new improved format. |
So if someone really likes to analyze the source for compilation and get all dependencies on of the source bundle
The goal is to provide something for P2 so everyone writing P2 code can use it easily, if it would have been only for Ooomph it would better be placed there in a private package as PDE itself has not any use for that. |
2d8661f
to
6b604b6
Compare
That makes sense, I also assume that the I didn't went trough this change in detail yet, but can the new action also handle classical PDE projects with a hand-written MANIFEST.MF?
I have to admit I'm torn in this case. I understand your point that its good to provide the action with as few dependencies as possible in a relatively prominent place where it can be found by others. But at the same time I cannot tell, if there are other users than Oomph but that can also be due to a lack of imagination and who knows what the future brings. On the other hand having this action, that is tightly coupled to how PDE internally handles the project and the features it provides, located in P2 will probably make it harder to maintain. |
It handles all variants of "classic" or "new" ways of specify the manifest
These things change in years (if not decades) and for example P2 contain lots of more complex stuff from PDE (features, products, ...) without any code-share so I don't think there is any issue here with these trivial things. |
@merks do we want to continue on this? If not suitable for P2 directly this might instead be added to Oompf directly as I understand its mostly needed there. |
I don't know how to test this. 😢 A zip file a sample would be helpful... What's the actual plan for the Platform in terms of keeping the generated MANIFEST.MF checked in? Of course my primary concern is the extent to which a source project's full requirements and full capabilities are known purely from the source as is currently the case with a MANIFEST.MF... |
This is my platform demonstrator / incubation project for automatic manifest generation: for testing build.properties, you can use
I think there is not any concrete type of plan.
This is independent from that, please note that So this is purely to transform such build requirements into something P2 / Ooompf can understand if it want to provision a workspace where the project should be compile (provided capabilities might be added later as well). |
Very good. I will try to find time in the next day or two... |
This adds actions for code that want to transform an PDE project into a set of requirements/capabilities for example to compute the required units of a target platform for such project.
6b604b6
to
6abe0c6
Compare
I'm doing some testing, but I don't understand the design intent. A "normal" bundle publisher takes the MANIFEST.MF into account and also things like the p2.inf. But here I get this as input, note there is no MANIFEST.MF: The only thing that's produced is some information about the build path which is associated with some "synthetic IU"
The rest of the information is the pde.bnd file is ignored completely. Is there something special to be done for a bnd.bnd? |
Correct, because the |
I think perhaps we've overlooked a fundamental point here. These things are used (by Oomph/targlets) before there exists a target platform. In particular, this would be used when there exists only a clone, freshly cloned from git, and only on disk, not in a workspace, no builders, no generators, no workspace, no nothing. With the existing infrastructure, the MANIFEST.MF exists and all works extremely well. If this is to be a replacement for that, and the MANIFEST.MF does not even exist (yet), then whatever information is used to generate the MANIFEST.MF (later) needs to be used to synthesize the InstallableUnit (in the action) that will eventually be synthesized from the generated MANIFEST.MF. Is that feasible? |
I don't think we have overlooked something but you misintepreted the intend, it tells you it want bundle X in a (potential) target platform. I never written this replace |
I don't need to know just build requirements, I need the full IU itself, e.g., the BSN, version, provided capabilities and the required capabilities. The MANIFEST.MF is absent, but was like this before and from that I know everything that needs to be known:
Some of that information is present in the pde.bnd:
But not all of it and I have no idea how some of that information is recovered. Certainly not from locally-available information purely in the file system. So in the end, this does not appear useful for the intended purpose. Perhaps if the logic considered the additional headers in the pde.bnd it would be slightly more useful, but in the current form, it appears that the omission of the MANIFEST.MF from the git repository leads to a non-recoverable loss of information. |
As said this is in addition to anything else, so it is not "just" it is "also"
You can just put the manifest there if you like for your tests, but as said this is not to replace it (even though in my example its not there because I have not checked it in as its not relevant for the build test) but to enhance its information.
That's a completely different topic. |
@merks there is now a more complete demo thanks to @fipro78 here: https://github.com/eclipse-tycho/tycho/tree/main/demo/pde-automatic-manifest this might help to better understand / test what this does. |
This adds actions for code that want to transform an PDE project into a set of requirements/capabilities for example to compute the required units of a target platform for such project.
@merks as discussed at EclipseCon 2023 this adds a new
PdeAction
that allows to determine the Requirements/Capabilites of a source project to be used to collect items for a Target Platform. This code is adapted from Tycho and slightly adjusted an do the following:build.properties
files that containsadditional.bundle
, if yes these are transformed into an Iu requiring this as optional items (as PDE handles them in an optional fashing as well)pde.bnd
file and looks there for required buildpath entriesI was not able to really test this in a context of P2/Ooompf but if you want to give it a try let me know if anything needs adjustments.