Conversation
mehmet-karaman
force-pushed
the
zstd_support
branch
2 times, most recently
from
8d91b8d to
1e141a1
Compare
iloveeclipse
left a comment
iloveeclipse
left a comment
There was a problem hiding this comment.
- If the zstd support doesn't work or disabled, editor opens and is empty etc.
So far so good. But editor should not allow to save or change anything, right now it allows to add entries, it will be dirty after that but fails to save via NPE:
java.lang.NullPointerException: Cannot invoke "org.eclipse.core.runtime.content.IContentType.getFileSpecs(int)" because the return value of "org.eclipse.core.runtime.content.IContentTypeManager.getContentType(String)" is null
at zipeditor.model.ZipContentDescriber.getContentTypeForFileExtension(ZipContentDescriber.java:99)
at zipeditor.ZipEditor.internalSave(ZipEditor.java:384)
at zipeditor.ZipEditor.doSave(ZipEditor.java:364)
Ideally editor would disable all actions, or, if it is toomuch work, at least do not fail with NPE but with a message that the file can't be changed.
- If the zip can't be opened (zstd is disabled) and also sometimes if it is enabled (so it seem to be existing issue) I get following errors in the log:
java.lang.NullPointerException: Cannot invoke "zipeditor.ZipOutlinePage.setInput(zipeditor.model.Node)" because "this.this$0.fOutlinePage" is null
at zipeditor.ZipEditor$19.run(ZipEditor.java:1125)
at org.eclipse.swt.widgets.RunnableLock.run(RunnableLock.java:40)
at org.eclipse.swt.widgets.Synchronizer.runAsyncMessages(Synchronizer.java:132)
at org.eclipse.swt.widgets.Display.runAsyncMessages(Display.java:5047)
at org.eclipse.swt.widgets.Display.readAndDispatch(Display.java:4527)
at org.eclipse.e4.ui.internal.workbench.swt.PartRenderingEngine$5.run(PartRenderingEngine.java:1151)
at org.eclipse.core.databinding.observable.Realm.runWithDefault(Realm.java:339)
at org.eclipse.e4.ui.internal.workbench.swt.PartRenderingEngine.run(PartRenderingEngine.java:1042)
at org.eclipse.e4.ui.internal.workbench.E4Workbench.createAndRunUI(E4Workbench.java:153)
at org.eclipse.ui.internal.Workbench.lambda$3(Workbench.java:668)
Fix:
diff --git a/ZipEditor/src/zipeditor/ZipEditor.java b/ZipEditor/src/zipeditor/ZipEditor.java
index 5561f2c..1869e05 100644
--- a/ZipEditor/src/zipeditor/ZipEditor.java
+++ b/ZipEditor/src/zipeditor/ZipEditor.java
@@ -549,2 +549,3 @@
fZipActionGroup.dispose();
+ fModel = null;
super.dispose();
@@ -1124,3 +1125,5 @@
public void run() {
- fOutlinePage.setInput(fModel.getRoot());
+ if(fOutlinePage != null && fModel != null) {
+ fOutlinePage.setInput(fModel.getRoot());
+ }
}
- Right clicking on a zip file with zstd disabled shows an error popup with the message that zstd support is not there. I believe this error popup is not needed here, logging should be enough.
Stack:
zipeditor.model.ZipEditorZstdException: Support for zstd is deactivated. Please check the zip editor settings.
at zipeditor.model.zstd.ZstdUtilities.getInputStream(ZstdUtilities.java:16)
at zipeditor.model.ZipModel$1.createZstdInputStream(ZipModel.java:115)
at org.apache.commons.compress.archivers.zip.ZipArchiveInputStream.getNextZipEntry(ZipArchiveInputStream.java:791)
at org.apache.commons.compress.archivers.zip.ZipArchiveInputStream.getNextEntry(ZipArchiveInputStream.java:648)
at zipeditor.model.ZipModel.detectType(ZipModel.java:119)
at zipeditor.model.ZipContentDescriber.describe(ZipContentDescriber.java:144)
at org.eclipse.core.internal.content.ContentTypeCatalog.describe(ContentTypeCatalog.java:295)
at org.eclipse.core.internal.content.ContentTypeCatalog.collectMatchingByContents(ContentTypeCatalog.java:266)
at org.eclipse.core.internal.content.ContentTypeCatalog.internalFindContentTypesForSubset(ContentTypeCatalog.java:490)
at org.eclipse.core.internal.content.ContentTypeCatalog.internalFindContentTypesFor(ContentTypeCatalog.java:549)
at org.eclipse.core.internal.content.ContentTypeCatalog.findContentTypesFor(ContentTypeCatalog.java:383)
at org.eclipse.core.internal.content.ContentTypeMatcher.findContentTypesFor(ContentTypeMatcher.java:57)
at org.eclipse.lsp4e.LSPEclipseUtils.getFileContentTypes(LSPEclipseUtils.java:1327)
at org.eclipse.lsp4e.LanguageServersRegistry.canUseLanguageServer(LanguageServersRegistry.java:463)
at org.eclipse.lsp4e.HasLanguageServerPropertyTester.test(HasLanguageServerPropertyTester.java:29)
at org.eclipse.core.internal.expressions.Property.test(Property.java:65)
at org.eclipse.core.expressions.TestExpression.evaluate(TestExpression.java:107)
at org.eclipse.core.expressions.CompositeExpression.evaluateAnd(CompositeExpression.java:54)
at org.eclipse.core.internal.expressions.AdaptExpression.evaluate(AdaptExpression.java:121)
at org.eclipse.core.expressions.CompositeExpression.evaluateOr(CompositeExpression.java:68)
at org.eclipse.core.expressions.OrExpression.evaluate(OrExpression.java:26)
at org.eclipse.core.expressions.CompositeExpression.evaluateAnd(CompositeExpression.java:54)
at org.eclipse.core.internal.expressions.IterateExpression.evaluate(IterateExpression.java:163)
at org.eclipse.e4.ui.internal.workbench.ContributionsAnalyzer.isCoreExpressionVisible(ContributionsAnalyzer.java:263)
at org.eclipse.e4.ui.internal.workbench.ContributionsAnalyzer.isVisible(ContributionsAnalyzer.java:237)
at org.eclipse.e4.ui.workbench.renderers.swt.ContributionRecord.computeVisibility(ContributionRecord.java:167)
at org.eclipse.e4.ui.workbench.renderers.swt.ContributionRecord.updateVisibility(ContributionRecord.java:98)
at org.eclipse.e4.ui.workbench.renderers.swt.MenuManagerRendererFilter.updateElementVisibility(MenuManagerRendererFilter.java:173)
at org.eclipse.e4.ui.workbench.renderers.swt.MenuManagerRendererFilter.updateElementVisibility(MenuManagerRendererFilter.java:184)
at org.eclipse.e4.ui.workbench.renderers.swt.MenuManagerShowProcessor.showMenu(MenuManagerShowProcessor.java:248)
- If zstd support is disabled, I expect "usual" zip files to work. However, creating a new empty zip file, adding few entries and save results in an error. Same happens if I open existing regular zip file, add an entry and try to save:
java.lang.NullPointerException: Cannot invoke "org.eclipse.core.runtime.content.IContentType.getFileSpecs(int)" because the return value of "org.eclipse.core.runtime.content.IContentTypeManager.getContentType(String)" is null
at zipeditor.model.ZipContentDescriber.getContentTypeForFileExtension(ZipContentDescriber.java:99)
at zipeditor.ZipEditor.internalSave(ZipEditor.java:384)
at zipeditor.ZipEditor.doSave(ZipEditor.java:364)
That should not happen.
|
Another observation: starting without jnilib and aircompressor: the preference page doesn't disable the comboand the checkbox for zstd support is enabled and checked. Should be all disabled and some error label shown like "to enable zstd install jnilib or aircompressor libraries" |
Good observation, as the libraries are optional, we have to set the default "zstd active" flag to false. And this should be cared by the user if he wants to activate this, he has to take care of the installed libraries.. |
|
Could reproduce the bug in the Package Explorer.. Zstd support was active, the expander besides the zip elements tree item was visible.. Disabled the Zstd Support and expandede that node and it has thrown the NPE.. Now this is also fixed.. |
mehmet-karaman
force-pushed
the
zstd_support
branch
from
4d9d981 to
ce957cf
Compare
ZipEditor/src/zipeditor/preferences/ZipEditorPreferencePage.java
Outdated
Show resolved
Hide resolved
iloveeclipse
left a comment
iloveeclipse
left a comment
There was a problem hiding this comment.
Nnow I see that updating entries in existing not-zstd compressed file create zstd entries:
The file below was created with default compression and .classpath inside was updated while the preference "ZSTD Default Compression" was enabled.
However, the preference should affect only new files, not existing.
Additionally to that, if such file is saved, next attempt to open it with zstd disabled results in an error message is shown "An error occurred trying to save":
After that the zip file is opened but the entry encoded with zstd compression is not shown (OK).
However, after that one could modify any other entry in the zip & save the zip, loosing the zstd encoded entry (NOK).
mehmet-karaman
force-pushed
the
zstd_support
branch
from
3f2a65d to
1a298c2
Compare
|
Hi, |
|
@uvoigt : at some point we assumed there is no one interested and most resent patches are not pushed here. We have few commits on top which clean up preferences page and "mavenize" the build to produce a fully self contained p2 repository with all dependencies included. |
|
Thanks @iloveeclipse, |
|
Pushed the latest changes.
The maven build can be started with "mvn clean install". The built artifacts can be found in the following path "/zipeditor/ZipEditor.UpdateSite/target/repository" |
|
- fixed wrong "wrapped" m2e bundle dependency on airlift in tests - fixed wrong classpath entry for aircompressor in zipeditor - fixed project settings to refer to proper Java 21 environment - bumped zipeditor bundle/feature/update site version to 1.2.0
- moved to a central utility - added handlers to handle these library usages. This helps to avoid the ClassNotFoundExceptions - Added message in Zstd Preferences Page to show if the Library is available or not.
- active flag is used also with checking if any library is available - fixed NPEs - Added hint in zip editor if zstd isn't available. - fix Project Explorer expanding bug (if afterwards the zstd was deactivated) - changed resource close() to try with resources.
- Add new files by Drag And Drop with Zstd compression if the new pref is true. - Fixed Zip entry properties bug with zsdt in method combobox. - Implemented PR comments. - changed two dim. array of libraries (combobox edit field) to more readable records and conversion to two dim. array. - added checkbox selection of zstd default in dialog according the default zstd compression state. - added additional checks if zstd is used as default but this method is not used in this zip file.
Was checking before that JNI lib is selected, now its checking that aircompressor is not selected.
mehmet-karaman
force-pushed
the
zstd_support
branch
from
45604da to
3b1e7d5
Compare
|
Hello @uvoigt , I've done the rebase, there are no conflicts anymore. |
|
The maven build can be started with "mvn clean install". The built artifacts can be found in the following path "/zipeditor/ZipEditor.UpdateSite/target/repository" After that the target platform can be selected as /ZipEditor/latest.target .. |
|
when building I get Is it correct to depend on snapshots? |
|
I would like to have the maven build for the next release that should include the changes of Btw., I asked AI and it generates nearly the same stuff. |
|
I would like to prepare a PR that contains the mavenization + ZipArchiveInputStream fallback.. |
|
Today I had so much time. I added the maven things + Github action to the open-file-performance branch |
|
Ok, would you like to continue with the commons-compress integration (as fallback) or shall i continue from here? Do you have time to continue? |
|
Maybe tomorrow I have some time too. So I will try that. |
|
Let me know if you need some help. Thanks. |
|
@mehmet-karaman I would offer to merge master into your branch because I caused the conflicts. I created fewer files when adding Maven support. It already runs in the Github CI so I want to stay with it. New release 1.2.0 is already out. |
3 participants
Implemented Zstd support upon the Apache commons compress zip archive implementation.