SWCompression icon indicating copy to clipboard operation
SWCompression copied to clipboard

AR Format Support

Open Lakr233 opened this issue 3 years ago • 7 comments

We have implemented support for BSD AR file format at: https://github.com/Lessica/SWCompression/tree/feature-bsd-ar

The reason not opening a pull request is that we have noticed you are submoduling a git repo for test files and your pipeline seems to be a little heavy for configurations. During our development, we simplified these process and we make all our AR file test work locally.

We would like yourself to merge the codes if you are ok with it. Estimated more than one repo are required to update for this merge.

The package structure are a little different from the upstream, follow are the changes we made.

Sources/SWCompression/AR
.
├── ArContainer.swift
├── ArEntry.swift
├── ArEntryInfo.swift
├── ArError.swift
├── ArHeader.swift
├── ArParser.swift
├── ArReader.swift
├── ArWriter.swift
├── Data+Ar.swift
└── LittleEndianByteReader+Ar.swift

Are the sources for AR API interface, supports reading and writing.

Sources/SWCompression/Common/Extensions.swift

Is a extension for ourself.

SWCompression/TAR/TarReader.swift 

Is a bug fix for Apple stuff returning mismatch item from their document. If a nil is returned, do not throw an error.

Tests/SWCompressionTests/
.
├── ArCreateTests.swift
├── ArReaderTests.swift
├── ArTests.swift
└── ArWriterTests.swift

Tests/SWCompressionTests/Test Files/AR

Are tests we made.

Following files are changed but not required for merge, generally speaking, we move the unicode test string payload to files.

Tests/SWCompressionTests/Constants.swift
SevenZipTests.swift
TarReaderTests.swift 
TarTests.swift
ZipTests.swift
...

With above changes applied to, we can make another check in README~

|       | Zlib | GZip | XZ  | ZIP | TAR | 7-Zip | AR  |
| ----- | ---- | ---- | --- | --- | --- | ----- | --- |
| Read  | ✅   | ✅    | ✅  | ✅  | ✅   | ✅    | ✅  |
| Write | ✅   | ✅    | TBD | TBD | ✅   | TBD   | ✅ |

Have a nice day~

Lakr233 avatar Jan 22 '22 06:01 Lakr233

Hi @Lakr233,

This is very interesting! I will have a closer look at the implementation and merging it when I have more time, but I have two questions nevertheless.

  1. My current plan is to move away in the future from the "algorithm"-specific error types, such as TarError, DeflateError, etc. As part of this plan all new additions use a more general error type, DataError. I've noticed that you add a new error type ArError, so my question: is it possible to use instead DataError?

  2. With regards to this issue:

The reason not opening a pull request is that we have noticed you are submoduling a git repo for test files and your pipeline seems to be a little heavy for configurations. During our development, we simplified these process and we make all our AR file test work locally.

What did you find particularly complicated about my setup/pipeline? I genuinely curious about this, maybe there is something I can do to simplify it in the future.

tsolomko avatar Jan 22 '22 08:01 tsolomko

  • I've noticed that you add a new error type ArError, so my question: is it possible to use instead DataError? yes

  • What did you find particularly complicated about my setup/pipeline? The main reason is that we do not have permission to access your submodule repo and to avoid road blocks in development. Don't feel like to make changes to your workflow.

Lakr233 avatar Jan 22 '22 10:01 Lakr233

Hi again,

I've been thinking about this and I don't think I can accept these changes without an actual pull request.

The problem is that as it currently stands what you're essentially asking me here is to copy someone else's code (the repository that you've linked here belongs to a different user, not to you) without their express permission.

With the PR from the author I will be assured that:

  1. They consent to contribution to this project;
  2. On what terms this contribution happens (to be precise, this would be on the terms of MIT license due to the inbound=outbound default Github's policy described in its Terms of Service).

tsolomko avatar Jan 23 '22 16:01 tsolomko

  • They consent to contribution to this project

I’m the collaborator of that repo. I have add a notice there in the README to authorize this action. It’s a little bit complicated to explain why not a PR but it should explain the right for you to do so. The author is my friend and we work together for many.

  • On what terms this contribution happens (to be precise, this would be on the terms of MIT license due to the inbound=outbound default Github's policy described in its Terms of Service).

The original code license(I see you did with MIT License) could be applied.

Lakr233 avatar Jan 24 '22 04:01 Lakr233

Would love to see the support for AR. What's holding this back?

mickeyl avatar Jun 14 '23 16:06 mickeyl

@mickeyl

Well, my concerns with accepting the contribution discussed in this issue have never been addressed.

At the same time, I do not see adding support for AR as a high priority for this project, and unfortunately I do not have enough time even for higher priority stuff. So as I result, I have never got around to implementing AR by myself.

tsolomko avatar Jun 15 '23 10:06 tsolomko

I dont understand what you stated as "never been addressed".

Lakr233 avatar Jun 17 '23 05:06 Lakr233