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.

Sign up for GitHub

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

Jump to bottom

Fix system package launch validation #221

Merged
merged 2 commits into from
Jul 20, 2022
Merged

Fix system package launch validation #221

merged 2 commits into from
Jul 20, 2022

Conversation

HannesWell
Copy link
Member

Use the actual VM system packages for Launch-Validation.

The list of system-packages in o.e.pde.api.tools/system_packages/<EE>-systempackages.profile files is not suitable for Launch-validation.

Fixes #194.

Besides that I wonder if ApiBaseline.getJavaProfilePropertiesForVMPackage() could be replaced with the same method? This would also make creating the system_packages/<EE>-systempackages.profile files obsolete.

@HannesWell HannesWell mentioned this pull request Jul 18, 2022
Closed
@HannesWell
Copy link
Member Author

@vik-chand do you want to review this?

@HannesWell HannesWell requested a review from vik-chand July 19, 2022 17:53
@HannesWell
Revert "Workaround for issue 194 ( fixes eclipse-pde#194) (eclipse-pd…
df2fd05 
…e#200)"

This reverts commit c9e687a.
@HannesWell
Use the actual system-packages from the JVM for Launch-Validation
84efc8f 
The list of system-packages in the <EE>-systempackages.profile file in
o.e.pde.api.tools is not suitable for Launch-validation, query them from
the VM instead.

Fixes eclipse-pde#194
@HannesWell HannesWell force-pushed the fixSystemPackageLaunchValidation branch from 848dde5 to 84efc8f Compare July 20, 2022 00:28
@vik-chand
Copy link
Member

@vik-chand do you want to review this?

I will review it by EOD today.

vik-chand
vik-chand approved these changes Jul 20, 2022
View reviewed changes
Copy link
Member

@vik-chand vik-chand left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@vik-chand
Copy link
Member

Besides that I wonder if ApiBaseline.getJavaProfilePropertiesForVMPackage() could be replaced with the same method? >This would also make creating the system_packages/-systempackages.profile files obsolete.

I'll try removing the system-package in Y build and assuming launch with Java 19 works with the new code and adding a API baseline works without an error, then we could replace that code too in master stream.

@vik-chand
Copy link
Member

Opened #228 for the other case

@HannesWell
Copy link
Member Author

Thank you for reviewing Vikas.

@HannesWell HannesWell merged commit b0b990a into eclipse-pde:master Jul 20, 2022
@HannesWell HannesWell deleted the fixSystemPackageLaunchValidation branch July 20, 2022 17:51
@flaviodonze
Copy link

Does this fix also apply to the PDE product-export validation?

grafik

@HannesWell
Copy link
Member Author

Does this fix also apply to the PDE product-export validation?

No it doesn't. But I strongly suspect that this has the same cause and can be fixed in the same way. When looking a bit if the API-profiles can be removed I noticed that PDE-Build uses them just like the launch validation did. So I think the same fix can be applied there.
I can probably provide a PR in the next days, unless you want to do that. :)

@HannesWell HannesWell mentioned this pull request Aug 11, 2022
Merged
@HannesWell
Copy link
Member Author

HannesWell commented Aug 11, 2022 •
edited
Loading

Does this fix also apply to the PDE product-export validation?

Created #284, can you test the PR if that also works in your use case?

@gireeshpunathil gireeshpunathil mentioned this pull request Jun 18, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@vik-chand vik-chand vik-chand approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Validation error when launching child eclipse
3 participants
@HannesWell @vik-chand @flaviodonze