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

[2054] fix OSGI support (manifest, class loading) #2081

Merged
merged 3 commits into from Dec 23, 2021
Merged

[2054] fix OSGI support (manifest, class loading) #2081

merged 3 commits into from Dec 23, 2021

Conversation

jherkel
Copy link
Contributor

@jherkel jherkel commented Sep 9, 2021

see issue #2054

This patch fixes next things:

  1. Java Service Loader issues -> I need to update MANIFEST file
  2. loading of liquibase.build.properties file -> I need to load this file via bundle.getEntry
  3. class loading problem for CustomChangeWrapper.java -> there is a problem that liquibase bundle cannot see classes from other bundles. This can be solved via dynamic import:* in MANIFEST.MF but it is not recommended way from OSGI point of view. So my solution is that there is a Activator class that is called when liquibase bundle is started or stopped. Under "start" method I registered BundleTrackerCustomizer that checked all bundles if they contain a special manifest entry "Liquibase-Custom-Change-Packages". When the entry exists bundle will be used for searching of CustomChange class.

Format for "Liquibase-Custom-Change-Packages" :

  • <- all packages from bundle are allowed
    com.xyz.test,com.xyz.test2 <- only CustomChange classes from bundle that lie in these two packages (or packages under these packages) are allowed.

Dev Handoff Notes (Internal Use)

Links

Testing

Dev Verification

Relying on people who know OSGi better to verify it improves it vs. what is in 4.6.2

┆Issue is synchronized with this Jira Bug by Unito

@molivasdat
Copy link
Contributor

Hi @jherkel Thanks for creating this PR.

A member of the Liquibase team will take a look at your contribution and may suggest:

The PR will be prioritized according to our internal development and testing capacity.

We’ll let you know when it’s ready to move to the next step or if any changes are needed.

@molivasdat molivasdat added DBAll ImpactLow IntegrationCDI RiskMedium Changes that require more testing or that affect many different code paths. Severity4 TypeBug labels Sep 16, 2021
@molivasdat molivasdat added this to To Do in Conditioning++ via automation Sep 21, 2021
…erkel-master

# Conflicts:
#	liquibase-core/src/main/java/liquibase/util/LiquibaseUtil.java
@nvoxland nvoxland changed the base branch from master to 1_9 December 8, 2021 18:52
@nvoxland nvoxland changed the base branch from 1_9 to master December 8, 2021 18:52
@nvoxland
Copy link
Contributor

nvoxland commented Dec 8, 2021

I merged our master into your fork so that there are no conflicts and the build should pass and product artifacts.

I don't know OSGi well. So, @jherkel could you check that the snapshot artifact in the "checks" tab above still works for you on OSGi?

We've just finished some large build logic changes, so besides the merge conflict I want to make sure the change still works.

@nvoxland nvoxland moved this from To Do to In discussion in Conditioning++ Dec 8, 2021
@nvoxland nvoxland linked an issue Dec 8, 2021 that may be closed by this pull request
@driessamyn
Copy link

I merged our master into your fork so that there are no conflicts and the build should pass and product artifacts.

I don't know OSGi well. So, @jherkel could you check that the snapshot artifact in the "checks" tab above still works for you on OSGi?

We've just finished some large build logic changes, so besides the merge conflict I want to make sure the change still works.

FYI. I can confirm that this fork still works in the Felix OSGi framework after the latest merge.

@nvoxland nvoxland moved this from In discussion to Code Review in Conditioning++ Dec 9, 2021
@jherkel
Copy link
Contributor Author

jherkel commented Dec 9, 2021

@nvoxland I will check it over the weekend.

@suryaaki2 suryaaki2 moved this from Code Review to Ready for Handoff (In JIRA) in Conditioning++ Dec 13, 2021
@nvoxland
Copy link
Contributor

Did you get a chance to verify the change, @jherkel ?

@jherkel
Copy link
Contributor Author

jherkel commented Dec 21, 2021 via email

@jherkel
Copy link
Contributor Author

jherkel commented Dec 21, 2021

I've verified the latest merge with Apache Karaf 4.3.4 and it works

@nvoxland
Copy link
Contributor

Great, thank you very much, @jherkel!

@nvoxland nvoxland merged commit 72e0a24 into liquibase:master Dec 23, 2021
Conditioning++ automation moved this from Ready for Handoff (In JIRA) to Done Dec 23, 2021
@nvoxland nvoxland removed this from Done in Conditioning++ Jan 6, 2022
@nvoxland nvoxland added this to the v4.7.0 milestone Jan 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
DBAll ImpactLow IntegrationCDI RiskMedium Changes that require more testing or that affect many different code paths. Severity4 SyncTicket TypeBug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Liquibase 4.4.3 doesn't work under OSGI (Karaf 4.3.2)
5 participants