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

typetools optionality? #94

Closed
yrashk opened this issue Oct 13, 2016 · 8 comments
Closed

typetools optionality? #94

yrashk opened this issue Oct 13, 2016 · 8 comments

Comments

@yrashk
Copy link

yrashk commented Oct 13, 2016

Would it be possible to make typetools dependency optional (at least for OSGi manifest's purpose), as it requires sun.reflect and while it is possible to configure an OSGi container to expose more of private packages, it's definitely not clean, considering that (in my case) I am not using this functionality as I am relying on a simpler solution.

@npgall
Copy link
Owner

npgall commented Oct 16, 2016

Hi Yurii,

As far as I know, TypeTools itself fails gracefully (returns type UNKNOWN to CQEngine) if sun.* classes are not available. And AFAIK CQEngine currently handles this condition.

My plan with CQEngine's QueryFactory was to isolate the methods which require this functionality from the ones which don't. So applications which want to have CQEngine auto-detect the types can call the corresponding methods, and applications which want to avoid the auto-detection and provide the types explicitly can do so. So I don't think there currently is a hard requirement on the availability of sun.* classes.

What happens in an OSGi container if you disable access to those methods? From CQEngine's perspective this should be fine. Or, is the problem with the manifest of the upstream TypeTools project?

@yrashk
Copy link
Author

yrashk commented Oct 16, 2016

The problem with OSGi is that the dependency on sun.reflect is declared as
mandatory, and therefore I must have it exposed in order to install 2.8.0
as an OSGi bundle.

This can be addressed in TypeTools manifest, too. I was just thinking to
see if you declare typetools package import in cqengine as optional, that
might solve the issue in one step.
On Mon, Oct 17, 2016 at 4:57 AM Niall Gallagher notifications@github.com
wrote:

Hi Yurii,

As far as I know, TypeTools itself fails gracefully (returns type UNKNOWN
to CQEngine) if sun.* classes are not available. And AFAIK CQEngine
currently handles this condition.

My plan with CQEngine's QueryFactory was to isolate the methods which
require this functionality from the ones which don't. So applications which
want to have CQEngine auto-detect the types can call the corresponding
methods, and applications which want to avoid the auto-detection and
provide the types explicitly can do so. So I don't think there currently is
a hard requirement on the availability of sun.* classes.

What happens in an OSGi container if you disable access to those methods?
From CQEngine's perspective this should be fine. Or, is the problem with
the manifest of the upstream TypeTools project?


You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
#94 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/AAABxFabytiS2CJytYlOgNg5yuUBLmWUks5q0p2ugaJpZM4KVpl4
.

@npgall
Copy link
Owner

npgall commented Oct 16, 2016

My intent was that while CQEngine would have a hard dependency on TypeTools, TypeTools would have that soft dependency on sun.* classes. And AFAIK this is the case in terms of how the code works.

Does it make more sense to fix this (OSGi manifest problem?) upstream in TypeTools, rather than in CQEngine?

Is an optional dependency in OSGi the same as an optional dependency in Maven?
If so, would setting the dependency as optional in OSGi imply that the functionality is available by default, or unavailable by default?

My preference would be for all features to be available by default, but of course to support users who want to exclude certain features/dependencies (via extra config in their project) if they need to.

On 17 Oct 2016, at 00:04, Yurii Rashkovskii notifications@github.com wrote:

The problem with OSGi is that the dependency on sun.reflect is declared as
mandatory, and therefore I must have it exposed in order to install 2.8.0
as an OSGi bundle.

This can be addressed in TypeTools manifest, too. I was just thinking to
see if you declare typetools package import in cqengine as optional, that
might solve the issue in one step.
On Mon, Oct 17, 2016 at 4:57 AM Niall Gallagher notifications@github.com
wrote:

Hi Yurii,

As far as I know, TypeTools itself fails gracefully (returns type UNKNOWN
to CQEngine) if sun.* classes are not available. And AFAIK CQEngine
currently handles this condition.

My plan with CQEngine's QueryFactory was to isolate the methods which
require this functionality from the ones which don't. So applications which
want to have CQEngine auto-detect the types can call the corresponding
methods, and applications which want to avoid the auto-detection and
provide the types explicitly can do so. So I don't think there currently is
a hard requirement on the availability of sun.* classes.

What happens in an OSGi container if you disable access to those methods?
From CQEngine's perspective this should be fine. Or, is the problem with
the manifest of the upstream TypeTools project?


You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
#94 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/AAABxFabytiS2CJytYlOgNg5yuUBLmWUks5q0p2ugaJpZM4KVpl4
.


You are receiving this because you commented.
Reply to this email directly, view it on GitHub #94 (comment), or mute the thread https://github.com/notifications/unsubscribe-auth/ACuJikHIzV4mWJW0f0irJQYzPoIuz-_1ks5q0q2ZgaJpZM4KVpl4.

@yrashk
Copy link
Author

yrashk commented Oct 16, 2016

The reason why I thought of addressing it in cqenine is that it seemed to
me that making sun.reflect optional in typetools doesn't make much sense as
without it, typetools can't do anything at all. cqengine can do a lot
without typetools, it's only one feature that'll be missing.
On Mon, Oct 17, 2016 at 6:29 AM Niall Gallagher notifications@github.com
wrote:

My intent was that while CQEngine would have a hard dependency on
TypeTools, TypeTools would have that soft dependency on sun.* classes. And
AFAIK this is the case in terms of how the code works.

Does it make more sense to fix this (OSGi manifest problem?) upstream in
TypeTools, rather than in CQEngine?

Is an optional dependency in OSGi the same as an optional dependency in
Maven?
If so, would setting the dependency as optional in OSGi imply that the
functionality is available by default, or unavailable by default?

My preference would be for all features to be available by default, but of
course to support users who want to exclude certain features/dependencies
(via extra config in their project) if they need to.

On 17 Oct 2016, at 00:04, Yurii Rashkovskii notifications@github.com
wrote:

The problem with OSGi is that the dependency on sun.reflect is declared
as
mandatory, and therefore I must have it exposed in order to install 2.8.0
as an OSGi bundle.

This can be addressed in TypeTools manifest, too. I was just thinking to
see if you declare typetools package import in cqengine as optional, that
might solve the issue in one step.
On Mon, Oct 17, 2016 at 4:57 AM Niall Gallagher <
notifications@github.com>
wrote:

Hi Yurii,

As far as I know, TypeTools itself fails gracefully (returns type
UNKNOWN
to CQEngine) if sun.* classes are not available. And AFAIK CQEngine
currently handles this condition.

My plan with CQEngine's QueryFactory was to isolate the methods which
require this functionality from the ones which don't. So applications
which
want to have CQEngine auto-detect the types can call the corresponding
methods, and applications which want to avoid the auto-detection and
provide the types explicitly can do so. So I don't think there
currently is
a hard requirement on the availability of sun.* classes.

What happens in an OSGi container if you disable access to those
methods?
From CQEngine's perspective this should be fine. Or, is the problem
with
the manifest of the upstream TypeTools project?


You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
#94 (comment),
or mute
the thread
<
https://github.com/notifications/unsubscribe-auth/AAABxFabytiS2CJytYlOgNg5yuUBLmWUks5q0p2ugaJpZM4KVpl4

.


You are receiving this because you commented.
Reply to this email directly, view it on GitHub <
https://github.com/npgall/cqengine/issues/94#issuecomment-254081827>, or
mute the thread <
https://github.com/notifications/unsubscribe-auth/ACuJikHIzV4mWJW0f0irJQYzPoIuz-_1ks5q0q2ZgaJpZM4KVpl4
.


You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
#94 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/AAABxJSP-Y-APBBWVvk9UHGI_7PQM8wVks5q0rNbgaJpZM4KVpl4
.

@npgall
Copy link
Owner

npgall commented Oct 17, 2016

I'm not sure how to solve this. What do you suggest?

If I mark the TypeTools dependency (or the sun.* dependency) as optional in CQEngine's OSGi manifest, does that mean that it will be included by default or excluded by default?

@yrashk
Copy link
Author

yrashk commented Oct 18, 2016

If net.jodah.typetools is declared as an optional package in cqengine's OSGi manifest, it'll still mean additional action has to be taken by those who integrate with OSGi, as the maven dependency will still have to be excluded.

If a maven dependency is declared as optional (https://maven.apache.org/guides/introduction/introduction-to-optional-and-excludes-dependencies.html) then those who DO use this functionally will have to declare this dependency manually.

If we follow your intuition and instead convince @jhalterman to mark sun.reflect as an optional package, then none of the above will be necessary. I am only concerned this might seem a weird choice considered that without sun.reflect typetools aren't useful. But it is useful to mark this dependency as optional in typetools OSGi manifest when typetools is used as a transitive dependency and is not planned to be used.

@npgall
Copy link
Owner

npgall commented Jan 18, 2017

Hi Yurii, have you had any more thoughts on this?
The last update was that you were going to ask @jhalterman if the upstream project can be updated. Any update on this?

@npgall
Copy link
Owner

npgall commented Sep 15, 2018

Closing this. Feel free to reopen if necessary.

@npgall npgall closed this as completed Sep 15, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants