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

Added support for MsLz, added tests for cab, updated net version to 6… #64

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

kammerjaeger
Copy link

…, added nullable types

Changes in this PR:

  • Support for MsLz compressed files, e.g. *.dl_, *.ex_.
  • Test for cab files
  • Support for named streams
  • Update to newer net 6
  • Some bug fixes from null type addition

I have a working linux version, these are the changes that make sense without the complete rework that is needed for linux.
Please let me know what you think.

@adoconnection
Copy link
Owner

epic! do you think we should change target to net6? this will break compatibility with existing projects net3 and net5

@kammerjaeger
Copy link
Author

I asked myself that too. For my projects it is pretty important to make sure that null checks are done properly but I'm also the person who can decided what framework we are using :-)
I don't know what the user-base here is thinking. So you have to let me know if it is okay to switch to a newer net version. Net 6 was the first that supported nullable templates.

Some examples where nullable helps to stop bugs (non obvious nullable results)

  • string? fileName = this.GetProperty(fileIndex, ItemPropId.kpidPath);
    File name can be null if archive does not support saving of file names, for example MsLz, but I presume gz and similar compressions will have the same "problem"
  • Callbacks SetTotal and SetCompleted are allowed to be called with null pointers from 7zip which are best mapped to a nullable type. This is used by the rar archive decompressor
  • Some call parameter are allowed to be null while others are not, for example IInArchive.Open (IInStream stream,ref ulong maxCheckStartPosition, IArchiveOpenCallback? openArchiveCallback)

Please let me know what you think.
I can have a look at something like https://www.meziantou.net/how-to-use-nullable-reference-types-in-dotnet-standard-2-0-and-dotnet-.htm or https://medium.com/codex/dot-net-library-compatibility-with-core-and-framework-6c8e29cc80e0 but I'm not sure this will work. The Linux version is even more complicated if you are even interested to integrate that in the official version.
I will provide the linux version as soon as I have it cleaned up a bit more.

@adoconnection
Copy link
Owner

adoconnection commented Mar 2, 2023

well, we can put some #ifdefs for net6+ with nullables. but the code will be messy I think.
What exact benefit you get out of nullables here in this lib?

@kammerjaeger
Copy link
Author

For me it is:

  • Better API definition
  • Less errors with null problems.

I agree with the #ifdefs making the code messy.
If you are interested in the linux adaption (hopefully it is soon finished) net core 3 will be the minimum as far as I can see.
Lib loading was implemented there https://learn.microsoft.com/en-us/dotnet/api/system.runtime.interopservices.nativelibrary?view=netcore-3.0. Not sure if other things will break.
We could again add a lot of ifdefs around lib loading but that code is already not the easiest to understand.
Anyways, as soon as I have the sub lib loading fixed I will provide a link to a branch.

@kammerjaeger
Copy link
Author

kammerjaeger commented Mar 10, 2023

Here is the first version of the linux port: https://github.com/kammerjaeger/SevenZipExtractor/tree/LinuxPort
It also adds:

  • full auto-detection of the file formats
  • information about available formats
  • extra lib loading (rar is a lib under linux)

I'm not sure this is bug free yet, did not do any mayor testing outside from the test cases, yet. Also it is net 6. If find any time I may look into if it is possible to use earlier versions.

Please let me know if you are even interested in integrating this.

@adoconnection
Copy link
Owner

linux support is a huge step forward! Will give it a try asap! 🚀🚀🚀🎉

@kammerjaeger
Copy link
Author

Some information for linux:
I ran the test under WSL2 Ubuntu, proper linux tests will follow in the next week. I only tested x64, there is no x32 net runtime as far as I can see. I'm not sure if arm would work, there is an arm build of p7zip so it could work if the calling conventions are not different between the platforms.

@kammerjaeger
Copy link
Author

Did you have time looking at it? I added some improvements and bugfixes to the linux port. I am thinking of adding a function to create new archives and I think it would be better to finish this first.

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