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

[RFC]the OSGi semantics of bundle uninstall. #554

Merged
merged 2 commits into from
May 13, 2023

Conversation

PengZheng
Copy link
Contributor

@PengZheng PengZheng commented May 12, 2023

The behavior of the uninstall() method of Bundle interface is specified as follows:

To whatever extent possible, the Framework must remove any resources related to the bundle. This method must always uninstall the bundle from the persistent storage of the Framework.

Clearly, the current implementation, which is introduced by #476, does not touch persistent storage.
As seen in TEST_F(CxxBundleArchiveTestSuite, BundleArchiveReusedTest), after uninstall, the bundle archive is intact.
The previous implementation does set the bundle's persistent state to UNINSTALLED, but does nothing more than that.
Therefore, both are incorrect.

This PR tries to implement the correct behavior without affecting framework_shutdown.

"To whatever extent possible, the Framework must remove any resources related to the bundle. This method must always uninstall the bundle from the persistent storage of the Framework."
@PengZheng PengZheng requested a review from pnoltes May 12, 2023 14:40
@codecov-commenter
Copy link

codecov-commenter commented May 12, 2023

Codecov Report

Merging #554 (24587e0) into master (29cb4a3) will increase coverage by 0.06%.
The diff coverage is 100.00%.

❗ Current head 24587e0 differs from pull request most recent head b1ab9ca. Consider uploading reports for the commit b1ab9ca to get more accurate results

@@            Coverage Diff             @@
##           master     #554      +/-   ##
==========================================
+ Coverage   77.65%   77.72%   +0.06%     
==========================================
  Files         229      229              
  Lines       34928    34928              
==========================================
+ Hits        27125    27149      +24     
+ Misses       7803     7779      -24     
Impacted Files Coverage Δ
libs/framework/src/framework.c 77.71% <100.00%> (ø)
...framework/src/framework_bundle_lifecycle_handler.c 55.67% <100.00%> (ø)

... and 5 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

Copy link
Contributor

@pnoltes pnoltes left a comment

Choose a reason for hiding this comment

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

LGTM. If possible I think we should adhere to the OSGi spec.

Exceptions IMO are if it does not fit in the language (no garbage collect, no default recursive mutexes, no class loader, no reflection, etc), does not fit more resources-constrained runtime environment and also if the OSGi spec API is too arcanic too use in C/C++ (i.e. original service tracker api).

@PengZheng PengZheng merged commit 636b11a into master May 13, 2023
@PengZheng PengZheng deleted the feature/correct_uninstall_semantics branch May 13, 2023 08:14
@PengZheng PengZheng restored the feature/correct_uninstall_semantics branch May 14, 2023 03:07
@PengZheng PengZheng deleted the feature/correct_uninstall_semantics branch May 14, 2023 03:20
@PengZheng PengZheng mentioned this pull request May 14, 2023
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 this pull request may close these issues.

3 participants