Proposal: Opening/Closing Mechanism for Zip Files

[Michael5601]

Proposal: Opening/Closing Mechanism for Zip Files

Background

The Eclipse IDE has no built in functionality to open Zip Files and read or manipulate their content. Because of this, other operations like searching inside of Zip Files or comparing two Zip Files were also not possible.

Goal:

This pull request introduces a mechanism for handling Zip Files within the Eclipse workspace, enhancing the functionality to read and write Zip files. The primary goal is to provide a seamless experience for developers working with zip archives directly within Eclipse.

Description:

Zip files must be opened manually within the workspace by using the new command "Open Zip File" in the menu when right clicking the zip file. It is also possible to open nested zip files.

Zip Files are opened by replacing the file in the workspace with a linked folder that reads and writes the Zip File in the file system. By closing the Zip FIle, the linked folder will be deleted and the file can be seen in the workspace again.

Please note that only ZIP Archives are supported in this current implementation. Other archive types can be added in future improvements. Also linked Zip Files can not be opened with this implementation because the Zip File must be local.

An additional PR for the repository eclipse.platform.ui that grants access to the open/close mechanism for zip files over UI can be found in the following:

eclipse-platform/eclipse.platform.ui#1947

Co-Authored-By: CodeLtDave david.erdoes@vector.com

[laeubi]

Given there is a Zip Filesystem for local files in the JDK already that support read/write I wonder if one really needs the "Open Zip FIle" step? Also the examples contain such a ZipFile system for eclipse already.

So I wonder if you maybe can explain why this design decision was made with linking / extraction of the Zip?

[github-actions]

Test Results

0 files   -   586  0 suites   - 586   0s ⏱️ - 29m 48s
0 tests  - 3 980  0 ✅  - 3 934  0 💤  - 46  0 ❌ ±0 
0 runs   - 4 179  0 ✅  - 4 132  0 💤  - 47  0 ❌ ±0 

Results for commit ba2cef4. ± Comparison against base commit 0b69e82.

♻️ This comment has been updated with latest results.

[mickaelistria]

I think before diving in the code, @laeubi 's comment at #1413 (comment) is most important to answer. IMO, if we're to include a Zip filesystem in Platform, it'd better be the one that is already in the example. UI also could be built on top of this filesystem in example. One benefit is that we could make progress with UI without being blocked by Platform.

[laeubi]

One benefit is that we could make progress with UI without being blocked by Platform.

Honestly I think we don't need any new UI but it seems I have missed the discussion about the requirements for this change, but take as an example the Classpath Container, if there is a jar file (what actually is a zip) it simply has a handle so I can expand it and it shows the content and I can double click files and open editors and so on, I would expect something similar for "something" that support open zip files without any need to open/close/extract/... UI because that's the whole point of a file-system abstraction that you can have "virtual" items pointing to something that is not a regular file.

[iloveeclipse]

Co-Authored-By: CodeLtDave david.erdoes@vector.com

Here and on other PR: ALL contributors must sign ECA.

If david.erdoes@vector.com has not signed ECA, we can't merge this PR.

[laeubi]

eclipsefdn/eca succeeds so I don't see an immediate issue here:

[iloveeclipse]

eclipsefdn/eca succeeds so I don't see an immediate issue here:

OK. Haven't noticed the check was done for the other person as the PR owner.
Looks weird however to see only one author checked. What about @Michael5601 ECA agreement then?

[mickaelistria]

Can someone (a project lead?) gives some opinions about the fact that we already have code for it in the codebase? IMO, this particular PR is not necessarily and we should only discuss whether we want to include the existing zip filesystem implementation in the main feature or not.

[laeubi]

Looks weird however to see only one author checked. What about @Michael5601 ECA agreement then?

I don't know the details @waynebeaton maybe can tell or we need a servicedesk ticket to clarify what / when is checked...

[CodeLtDave]

Given there is a Zip Filesystem for local files in the JDK already that support read/write I wonder if one really needs the "Open Zip FIle" step? Also the examples contain such a ZipFile system for eclipse already.

I appreciate everyone's interest in our project. A frequent topic of discussion has been the existing zip example provided in Eclipse. I want to clarify that our pull request builds upon this example, adding more functionality and integrating it into the main feature.

As @mickaelistria mentioned, the key question is whether this functionality should be part of the main feature, and I firmly believe it should be.

Regarding the nio Zip Filesystem available in the JDK, we utilize its capabilities extensively. Our pull request essentially incorporates the NIO Zip Filesystem into the existing example, resulting in significant performance improvements. Most of the logic can be found in the ZipFileStore.java where the nio Zip Filesystem is opened.

[mickaelistria]

I want to clarify that our pull request builds upon this example, adding more functionality and integrating it into the main feature.

OK, great. Thank you for clarifying!
Do you think it would be possible for you to create a dedicated PR for those improvements and additional tests, but keeping the code in the "example" bundle for the moment? Such a PR could be merged in a breeze and be a valuable iteration while we still discuss the best place to host the filesystem in the end.

[laeubi]

@CodeLtDave can you explain why explicitly Open/Close is required? Can't we reuse the existing functionality (and possibly extend if something is missing here)?

What we already have

To make it more clear I just choose this random project with some java files and it looks like this

Now I expand the handle and it looks like this:

So I have a File (.java) and it shows we there is maybe more to explore and after that I see some more items that are "inside" the file...

As a user I would expect same/similar if it was not a java but a Zip-File for browsing the content similar to what I have for example with Plugin-Dependencies:

What would be new

Now if I open one file in an editor, I would like to be able to save this (and maybe getting asked if I really want to update the zip archive).

Next when I choose the File Search Dialog I would maybe expect some new option to include searching inside archives as well:

Conclusion

So to summarize I don't see we really need any new Actions for that (load/unload) but should use whats already is offered here. I haven't checked in detail how these functionality is implemented so maybe there are good reasons not to do it like this... but it would better match user expectation I think.

[CodeLtDave]

So I wonder if you maybe can explain why this design decision was made with linking / extraction of the Zip?

Regarding the functionality of the ZipFilesystem, as implemented in the example, we don't extract any files for opening or editing. Opening the Zip is akin to walking down the File Tree and collecting all necessary file information.

When a file is modified, it's done by opening the Zip with NIO and using the respective operations.

Based on our testing, this approach is much more lightweight and performant than always extracting the entire Zip and repackaging it. For small changes, this saves significant time and reduces overhead. We didn't investigate the internal workings of NIO in detail, so it's possible that some extraction and repackaging might be happening in the background. However, since it's much faster than manual extraction and repackaging, we believe this is not the case.

Regarding the approach with a linked resource, this seems to be the best method for linking any virtual FileSystem. Since it was already implemented this way in the example project, we believe it is appropriate and fitting for our use case.

[jukzi]

I wish ZIP is support from core. Currently i have to go windows file explorer to open zip files. awkward. Searching in zip files would be even better, but that can be a follow up step. At best JAR files are handled as ZIP files as well, because technically they are. The file ending is rather irrelevant.

[laeubi]

I think then this description is confusing:

Zip Files are opened by replacing the file in the workspace with a linked folder that reads and writes the Zip File in the file system. By closing the Zip FIle, the linked folder will be deleted and the file can be seen in the workspace again.

Why is it a folder then? (see example with java file that is not a folder at all)? As far as I understand the example it is done that way so you can link in any external zip file but here we want to show additional content for a file inside the workbench so this indirection should not be required (see java files and classpath jar files).

[CodeLtDave]

I wish ZIP is support from core. Currently i have to go windows file explorer to open zip files. awkward. Searching in zip files would be even better, but that can be a follow up step. At best JAR files are handled as ZIP files as well, because technically they are. The file ending is rather irrelevant.

@jukzi

The use case for searching in ZIP files is honestly the motivation for this project. Many colleagues, including myself, have often expressed frustration over this limitation. As you mentioned, JAR files can only be differentiated from ZIP files by their structure, specifically the presence of a META-INF/MANIFEST.MF file and possibly a Main-Class attribute.

Given that this is the platform repository, we have manually disabled support for JAR files. We plan to add JAR support, along with all the necessary functionality and tests, in the JDT (Java Development Tools) as suggested by @HeikoKlare.

[CodeLtDave]

I think then this description is confusing:

Zip Files are opened by replacing the file in the workspace with a linked folder that reads and writes the Zip File in the file system. By closing the Zip FIle, the linked folder will be deleted and the file can be seen in the workspace again.

I think you are right, that sounds confusing. Essentially, the zip file is hidden as long as there is a linked folder (or opened Zip) accessing the zip file. When it's closed, the link is deleted and the zip file is visible again.

Why is it a folder then? (see example with java file that is not a folder at all)? As far as I understand the example it is done that way so you can link in any external zip file but here we want to show additional content for a file inside the workbench so this indirection should not be required (see java files and classpath jar files).

I think I misunderstand your question. Every Zip/JAR is a compressed folder and is therefore displayed as such. We didn't change anything about that part from the example.

[CodeLtDave]

I want to clarify that our pull request builds upon this example, adding more functionality and integrating it into the main feature.

OK, great. Thank you for clarifying! Do you think it would be possible for you to create a dedicated PR for those improvements and additional tests, but keeping the code in the "example" bundle for the moment? Such a PR could be merged in a breeze and be a valuable iteration while we still discuss the best place to host the filesystem in the end.

Thank you for the input. I agree that splitting the addition of functionality and moving the example into two separate PRs could be beneficial. I will discuss this approach with my team. Considering the size of this PR and the necessary changes based on your comments, we will need some time to implement this.

[laeubi]

JAR files can only be differentiated from ZIP files by their structure, specifically the presence of a META-INF/MANIFEST.MF file and possibly a Main-Class attribute

Jar files are Zip files, a manifest is not mandatory nor any special headers:
https://docs.oracle.com/en/java/javase/20/docs/specs/jar/jar.html#introduction

A JAR file is essentially a zip file that contains an optional META-INF directory

So I don't see why one should / would disable anything for them?

We didn't change anything about that part from the example.

The example is for linking a filesystem as a new item into the workbench, here we want to show additional childs for an existing file in the workbench. That's an important part I think and would avoid having to "shadow" an existing file, maybe such special resource must implement both IFolder and IFile as otherwise other parts of the system can no longer "see" the zip/jar as a file what could have some implications.