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

Add Bytecode Outline to JDT UI #364

Closed
iloveeclipse opened this issue Jan 4, 2023 · 30 comments
Closed

Add Bytecode Outline to JDT UI #364

iloveeclipse opened this issue Jan 4, 2023 · 30 comments
Assignees
Labels
enhancement New feature or request
Milestone

Comments

@iloveeclipse
Copy link
Member

Follow up on https://bugs.eclipse.org/bugs/show_bug.cgi?id=540436 / iloveeclipse/bytecodeoutline#8 .

One goal would be to add "Bytecode Outline" view to JDT UI.
Stretch goal would be to replace default class file editor (that shows bytecode for classes with missing source) with one crafted by BCO.

I had no time yet to work on this, but I think that should be an easy task now.
The main issue is to move the code from https://github.com/iloveeclipse/bytecodeoutline & setup maven build in JDT UI so the bundle is picked up by platform build.

@iloveeclipse iloveeclipse added help wanted Extra attention is needed enhancement New feature or request labels Jan 4, 2023
@LunarLaurus
Copy link

I believe the work done here is enough for the Maven aspect: https://github.com/LunarLaurus/eclipse.jdt.ui/tree/BytecodeOutline

My fresh Oomph install for making these changes doesn't let me launch it as an Eclipse Application (Even with a fresh clone of JDT), but I don't see why this shouldn't work.

iloveeclipse added a commit to iloveeclipse/eclipse.platform.releng.aggregator that referenced this issue Jan 13, 2023
iloveeclipse added a commit to eclipse-platform/eclipse.platform.releng.aggregator that referenced this issue Jan 13, 2023
iloveeclipse added a commit to iloveeclipse/eclipse.jdt.ui that referenced this issue Jan 14, 2023
iloveeclipse added a commit to iloveeclipse/eclipse.jdt.ui that referenced this issue Jan 15, 2023
The code is based on this commit, just updated vendor / manifest:
iloveeclipse/bytecodeoutline@9fe31d0

See
- iloveeclipse/bytecodeoutline#8
- eclipse-jdt#364
@LunarLaurus
Copy link

replace default class file editor

Am I correct in assuming this will be the right class to target for further integration?
org/eclipse/jdt/internal/ui/javaeditor/ClassFileEditor.java
If yes, great. 👍

@iloveeclipse
Copy link
Member Author

Yes. Good luck ...
Please note, your changes should wait for #385 to be merged (or you should check that PR out and add your changes on top).

@LunarLaurus
Copy link

LunarLaurus commented Jan 16, 2023

I have seen that it is in the process of being approved and merged, good work.
Currently trying to get an Eclipse instance working where jdt.ui.* packages can be edited without missing dependencies so that I am ready once merge occurs. Trying to use the installer in advance mode to set up Eclipse for Eclipse developers with just the jdt.ui.* packages available.

Edit: This seems to have worked successfully! Wonderful news.

iloveeclipse added a commit to iloveeclipse/eclipse.jdt.ui that referenced this issue Jan 16, 2023
The code is based on this commit, just updated vendor / manifest:
iloveeclipse/bytecodeoutline@9fe31d0

See
- iloveeclipse/bytecodeoutline#8
- eclipse-jdt#364
@iloveeclipse
Copy link
Member Author

See ClassFileBytesDisassembler for the current class file disassembler used. I think the idea of original request was to have the output of that being replaced by properly colored / syntax highlighted bytecode (ideally with hovers, outline etc).

So I would first recommend to try out "just" making the disassembled source "nice looking". Outline & navigation are harder.

iloveeclipse added a commit that referenced this issue Jan 16, 2023
The code is based on this commit, just updated vendor / manifest:
iloveeclipse/bytecodeoutline@9fe31d0

See
- iloveeclipse/bytecodeoutline#8
- #364
@iloveeclipse iloveeclipse removed the help wanted Extra attention is needed label Jan 16, 2023
@iloveeclipse iloveeclipse added this to the 4.27 M2 milestone Jan 16, 2023
@iloveeclipse iloveeclipse self-assigned this Jan 16, 2023
@LunarLaurus
Copy link

So I can create a new oomph intsance of for jdt packages, however while it pulls down the org.eclipse.jdt.bcoview repository within git\eclipse.jdt.ui, Eclipse itself fails to add bcoview as a project. Importing as a maven project into the jdt.ui working set works, but it still cannot find the newly added asm bundles.
image

Do I need to wait for this to deploy everywhere or?
I will try again tomorrow, just wanted to start now 😆

@iloveeclipse
Copy link
Member Author

Do I need to wait for this to deploy everywhere or?

I assume the dependencies are in the target platform now (because the code builds), but not in the resulted SDK package (because the view is not referenced by any feature / product).

I assume we need same addition like in eclipse-platform/eclipse.platform.releng#180.

I will prepare PR as soon as eclipse-platform/eclipse.platform.releng#180 will be merged, to avoid merge conflicts.

@akurtakov
Copy link
Contributor

Let me run a full I-build to be sure I haven't missed anything for the old views addition first.

@iloveeclipse
Copy link
Member Author

Let me run a full I-build to be sure I haven't missed anything for the old views addition first.

I've also noticed there is no org.eclipse.jdt.bcoview feature yet (I didn't wanted to copy old feature that has no value).
I assume I have to create one now? I can quickly add one.

@akurtakov
Copy link
Contributor

Yes, you would have to create such a feature so there is smth end user installable .

@iloveeclipse
Copy link
Member Author

OK, I will add that feature then.

iloveeclipse added a commit to iloveeclipse/eclipse.jdt.ui that referenced this issue Jan 17, 2023
Adds `org.eclipse.jdt.bcoview.feature` and integrates with maven build

See eclipse-jdt#364
iloveeclipse added a commit to iloveeclipse/eclipse.jdt.ui that referenced this issue Jan 17, 2023
Adds `org.eclipse.jdt.bcoview.feature` and integrates with maven build

See eclipse-jdt#364
iloveeclipse added a commit to iloveeclipse/eclipse.jdt.ui that referenced this issue Jan 17, 2023
Adds `org.eclipse.jdt.bcoview.feature` and integrates with maven build

See eclipse-jdt#364
iloveeclipse added a commit that referenced this issue Jan 17, 2023
Adds `org.eclipse.jdt.bcoview.feature` and integrates with maven build

See #364
iloveeclipse added a commit to iloveeclipse/eclipse.platform.releng that referenced this issue Jan 17, 2023
iloveeclipse added a commit to eclipse-platform/eclipse.platform.releng that referenced this issue Jan 18, 2023
@LunarLaurus
Copy link

Tried to get this rolling today with yet another fresh install via Oomph, but now I get this.
Not really sure where to report this properly, but figure you do.
Sorry to be such a pain @iloveeclipse 🚧

image

@iloveeclipse
Copy link
Member Author

Please retry once again. I've just built SDK again (https://download.eclipse.org/eclipse/downloads/drops4/I20230118-0200/), probably you've seen the version https://download.eclipse.org/eclipse/downloads/drops4/I20230117-1800/

@LunarLaurus
Copy link

Appears this still occurs even after updating Oomph. Should I just wait longer?

@iloveeclipse
Copy link
Member Author

@LunarLaurus : honestly speaking, no idea, I'm not a heavy oomph user.
Can you please provide exact steps you use to get to the error above?

@merks : not sure if a screenshot in comment before is enough for you to diagnose the problem.

At least in the plain SDK https://download.eclipse.org/eclipse/downloads/drops4/I20230118-0200/ Bytecode Outline view is included & functional:

image

@iloveeclipse
Copy link
Member Author

Could be again mismatch between Orbit's and maven bundle names: org.objectweb.asm.tree.analysis vs org.objectweb.asm.analysis. The later one is in Orbit, the former one is from Maven / platform.

@merks
Copy link
Contributor

merks commented Jan 18, 2023

If the file it's complaining really does not exist on disk and the problem recurs if you try again, then probably you need to try to repair the bundle pool:

https://www.eclipse.org/forums/index.php/m/1835944/?srch=repair+bundle+pool#msg_1835944

As mentioned in that thread, sometimes virus scanners keep the file from being moved from the download location to the pool...

@LunarLaurus
Copy link

Was just in a meeting, let me poke things and then do a fresh Oomph run.
It's certainly not an AV issue though. Thank you both for being helpful!

@LunarLaurus
Copy link

I nuked my bundle pool and it redownloaded everything.
Sorry for the screenshots, but I feel it explains this issue best.

Oomph setup.
image
JDT UI selected for development.
image
Oomph Okay.
image
Eclipse Staging.
image
Asm Utils still missing?
image
Version Information.
image
ASM Utils is installed.
image
BCO Exists.
image

@merks
Copy link
Contributor

merks commented Jan 18, 2023

Does the file exist on your file system at the indicate path shown in the dialog?

Window -> Preferences -> Oomph -> Bundles Pools -> Analyze Agent... should help find and fix problems.

image

@LunarLaurus
Copy link

Removing the entire .p2 directory worked 🥂

@merks
Copy link
Contributor

merks commented Jan 18, 2023

I've disabled the virus scanner from scanning my p2 folder and from scanning the folder containing all my installations. This helps make the installations faster too!

@iloveeclipse
Copy link
Member Author

I've disabled the virus scanner from scanning my p2 folder and from scanning the folder containing all my installations

Yea, I did that too for all of my system files, and the speedup is insane. PS: I'm using Linux :-)

@iloveeclipse
Copy link
Member Author

One goal would be to add "Bytecode Outline" view to JDT UI.

Done & verified with https://download.eclipse.org/eclipse/downloads/drops4/I20230118-0200

Stretch goal would be to replace default class file editor (that shows bytecode for classes with missing source) with one crafted by BCO.

@LunarLaurus : If you still have interest to continue, please open a new ticket here with an appropriate title & description.

I think the main part is done now => closing this issue.

@noopur2507
Copy link
Member

BCO
The string "Bytecode Outline" is not translated as reported in the testing (see screenshot). @iloveeclipse, could you please check if it is externalized properly and mention the location of the externalized string here?

@merks
Copy link
Contributor

merks commented Mar 13, 2023

Just the feature doesn't make it translatable, but that's only affects the display in p2, e.g., install/update or about:

image

Babel has not had a release in more than a year and is hoping to produce a new release for 2023-06 so until then, there is no translation available for such new strings...

@noopur2507
Copy link
Member

Looks like the preference page string is hard-coded in /org.eclipse.jdt.bcoview/plugin.xml.

Can you please create a patch to fix it in the master and the R4_27_maintenance branches?

@iloveeclipse
Copy link
Member Author

I will do, but not today. I assume this is not urgent.

@noopur2507
Copy link
Member

You can create the patch tomorrow. It's required by IBM products team to pass 4.27 testing.

@iloveeclipse
Copy link
Member Author

You can create the patch tomorrow.

I would try.

It's required by IBM products team to pass 4.27 testing.

Shouldn't that be driven by IBM products team then?

Ideally it would be found during code reviews, but once code is merged - at least short after merge, not few days before release. In our company we run all the Eclipse related tests we have every week against latest Eclipse build, so we usually detect regressions latest in one week after merge to master.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

5 participants