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

Show numResolved / numUnresolved in resolution view #5923

Merged

Conversation

chrisrueger
Copy link
Contributor

@chrisrueger chrisrueger commented Dec 4, 2023

  • First draft to show simple counters to easier spot unresolved requirements.
  • reqs resolution := optional are not counted as unresolved.
image

- First draft to show simple counters to easier spot unresolved requirements.
- reqs resolution := optional are not counted as unresolved.

Signed-off-by: Christoph Rueger <[email protected]>
@chrisrueger
Copy link
Contributor Author

@pkriens Not sure if this is useful. Initially I thought I could use those counters it to easier find unresolved Requirements. But not sure if I am thinking correctly.

I just added 3 counters now to count Resolved, Optional and Other.
Originally I though "Other" == Unresolved. But not sure anymore that is the case after playing around with it.
I thought maybe adding another filter so that you filter the Requirements list e.g. to only show the Resolved, Optional or Other.

Any thoughts on this?

@pkriens
Copy link
Member

pkriens commented Dec 4, 2023

I think I'd prefer more filters ... The advantage of a graphic UI is that our brains are amazing in detecting patterns. Having more filters will allow you to use this better than numbers. And with the aggregate idea of the view, I am struggling to think what would mean?

If you're at it, it might be an idea to detect duplicate exports? It is ok if they are identical (all attributes should match) but otherwise it would help solve the problem we had.

@chrisrueger
Copy link
Contributor Author

Ok I see, I probably discard the counter idea for now or may revisit them later, in case I find a useful use case.
But nevertheless I am at the code right now, so I will continue the additional filter idea and try to come up with a proposal.

detect duplicate exports

So does that mean:

  • find identical capabilities which are exported by > 1 resource / bundle?

@pkriens
Copy link
Member

pkriens commented Dec 4, 2023

find identical capabilities which are exported by > 1 resource / bundle?

No that is ok :-) the problem is when there primary attribute is the same but they differ in other attributes. Except for osgi.service namespace, the primary attribute is always the same name as the name of the namespace. E.g. for a package the namespace is "osgi.wiring.package" and the attribute "osgi.wiring.package" holds the name of the package. you can find the gory details in PackageNamespace clas. For some asinine reason we made the ServiceNamespace use "objectClass" for the primary attribute.

The problem we're chasing is finding packages that have the same name but differ in their class. Sinces the "hashes" attribute contains the hashes of each class, we'd see differences. The problem was that there were two exporters with different packages with the same name. If they'd been identical, as specified by OSGi, it would be ok.

Signed-off-by: Christoph Rueger <[email protected]>
speeds up search from 7s to 1s for a 3500 item Maven repo. Needed to resort result.

Signed-off-by: Christoph Rueger <[email protected]>
Signed-off-by: Christoph Rueger <[email protected]>
@chrisrueger
Copy link
Contributor Author

The problem we're chasing is finding packages that have the same name but differ in their class. Sinces the "hashes" attribute contains the hashes of each class, we'd see differences. The problem was that there were two exporters with different packages with the same name. If they'd been identical, as specified by OSGi, it would be ok.

Thanks @pkriens for the clarification.

I have pushed an update.
Could you have a look at the testcase to see if it makes sense?
Currently I specifically handle osgi.wiring.package and their bnd.hashes.

Here is how the current draft of the UI looks like:

  • added empty state short message, how to use the resolution view
  • I also added two search filter inputs so that you can now search the Requirements and Capabilities similar to how you can search repos. (TODO currently I Glob-match on req.toString() and cap.toString(). That maybe too broad. let me know if you want that to be different)
image
  • show counter of visible rows (I find this helpful to see that filters change something)
  • in the Capability Panel I additionally show a "Problem"-counter (there are any). Currently a "problem" is exactly the duplicate package problem. (TODO if you have suggestions for different wording e.g. if "Problem" is too harsh...)
  • there is also a new Filter button (yellow warning icon) for the "Problems"
image
  • pressing the new yellow warning button filters the currently visible results and only shows those who were detected as "having duplicate exports with different hashes"
image
  • Double clicking on a Cap brings up the Advanced Repository Search (my previous PR)
image image

Do you think this would have been helpful for debugging the "duplicate exporterts" problems?
More thoughts?

Copy link
Member

@pkriens pkriens left a comment

Choose a reason for hiding this comment

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

LGTM

I think we should commit it and get some experience

@pkriens
Copy link
Member

pkriens commented Dec 5, 2023

Really interesting what's happening here. Maybe we should make a video to show what this is all doing.

Signed-off-by: Christoph Rueger <[email protected]>
@chrisrueger
Copy link
Contributor Author

I think we should commit it and get some experience

Thanks @pkriens for the feedback. I added the varargs suggestion in the testcase.

Ok then let's merge :)

Maybe we should make a video to show what this is all doing.

Sure, open to ideas. Maybe let's talk about it in the next call?

@chrisrueger chrisrueger marked this pull request as ready for review December 5, 2023 15:17
@pkriens pkriens merged commit 4d1c6b0 into bndtools:master Dec 6, 2023
9 checks passed
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.

2 participants