[TAR] Init a TarEntry with a fileURL + InputStream

[aonez]

Added the ability to initialize a TarEntry using a fileURL in conjunction with and InputStream.

This is an alternative as having to pass all the file's data. This enables to process large files without reading the whole file on memory first, since it then gets read in blocks of 1024 bytes.

[tsolomko]

Hi @aonez,

Thank you for your proposal. Unfortunately, as it stands right now I cannot merge it for two reasons.

On the practical side, your changes are not source compatible as is evidenced by failures of automatic builds in CI. Specifically, changing non-throwing function TarContainer.create into a throwing one breaks compatibility.

At the same time, I have philosophical reservations against these additions. They introduce interactions between SWCompression and the filesystem which are currently absent by design. One of the reasons for this is that interacting with the filesystem is a large potential source of errors and must be done very carefully. This is best exemplified by this line in the proposed changes:

let input = InputStream(fileAtPath: file.path)!

Note the optional unwrapping happening here, which can cause crash at the runtime, and as such it should be either explained, why it can never crash, or a preemptive check of some kind must be performed that would result in an error being thrown instead of the crash.

P.S. I would be more open to the changes if instead of URL they were using InputStream, FileHandle, or something similar as the input.

P.P.S. Note that your changes do not even solve the issue of memory consumption, as the data of the entries is still collected in the memory while TarContainer is being created. Modifying TarWriter seems to me more suitable for achieving your goal.

[aonez]

Fair enough. I was very lazy with the unwrapping indeed.

On the practical side, your changes are not source compatible as is evidenced by failures of automatic builds in CI. Specifically, changing non-throwing function TarContainer.create into a throwing one breaks compatibility.

If the only issue with the tests is the throwing change I can deal with that.

P.S. I would be more open to the changes if instead of URL they were using InputStream, FileHandle, or something similar as the input.

Can do that for sure, It will be better that way.

P.P.S. Note that your changes do not even solve the issue of memory consumption, as the data of the entries is still collected in the memory while TarContainer is being created. Modifying TarWriter seems to me more suitable for achieving your goal.

In my tests this solved the memory issues, but you know more this code than me so I must be missing something here.

I'll review the code and the pull when I have some time and let's see if it has a place and meaning in your framework.

Salut!

[tsolomko]

In my tests this solved the memory issues, but you know more this code than me so I must be missing something here.

I would like to ask, how do you measure the memory usage? In my experience memory measurements are quite complicated to perform (correctly), so I am genuinely curious.

With regards to my doubts about your approach, as I understand it, your new version of the appendAsTarBlock function still loads the entire files into memory (even though it does it in chunks), converts and appends them to the out variable in the TarContainer.create function, so by the time create returns, all of the files in the container are loaded into the memory inside this out variable.

[aonez]

I'm very sorry I've mixed two frameworks and this is indeed unfinished code, so the tests were made using Tarscape not SWCompression. I'm very sorry for all the noise.

I'm closing this one now but if you think this somehow makes sense let me know and will update the code and the pull.

[aonez]

Oh and for checking the memory usage I use Xcode Instruments 🙌🏼

[tsolomko]

I'm very sorry I've mixed two frameworks and this is indeed unfinished code, so the tests were made using Tarscape not SWCompression. I'm very sorry for all the noise.

I'm closing this one now but if you think this somehow makes sense let me know and will update the code and the pull.

No worries, and thank you for bringing my attention to that project. I've looked at their code and I can see where confusion comes from: there is a lot of code taken straight from SWCompression. 😆

Oh and for checking the memory usage I use Xcode Instruments 🙌🏼

Ah, I see. Probably it is easier in your case, since I assume you are working on an application that you can then profile in Instruments. In my case the only thing I have is Xcode tests using XCTest, and I found it difficult to make it load them in Instruments.