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

Support for read_concatenated_archives option #60

Merged
merged 3 commits into from Jan 13, 2014

Conversation

Projects
None yet
2 participants
@kevinoid
Contributor

kevinoid commented Jan 12, 2014

As discussed in Issue 348, here is the first pass at an implementation of the read_concatenated_archives option for bsdtar.

Feel free to suggest alternate implementations or significant changes to how this is implemented. I thought it would be useful to have some code as a starting point for discussion (and so I could see how bad it would be to implement).

Only consume a second all-null record
Currently any record following an all-null record will be consumed.
This is probably not the intended behavior, as it does not match the
comment and consumes/ignores unexpected and unrecognized data.  In
particular, for the read_concatenated_archives case, it could
incorrectly consume a non-null header.

Signed-off-by: Kevin Locke <kevin@kevinlocke.name>
@kientzle

This comment has been minimized.

Show comment
Hide comment
@kientzle

kientzle Jan 12, 2014

Contributor

I have one immediate concern about the recursion when consuming multiple null blocks with padded archives, but this is otherwise very good.

Contributor

kientzle commented Jan 12, 2014

I have one immediate concern about the recursion when consuming multiple null blocks with padded archives, but this is otherwise very good.

@kevinoid

This comment has been minimized.

Show comment
Hide comment
@kevinoid

kevinoid Jan 12, 2014

Contributor

Sure, that makes sense. I've replaced the recursion with a loop in the form you suggested and updated the pull request. Is this a bit closer to what you were thinking?

Contributor

kevinoid commented Jan 12, 2014

Sure, that makes sense. I've replaced the recursion with a loop in the form you suggested and updated the pull request. Is this a bit closer to what you were thinking?

kevinoid added some commits Jan 11, 2014

[PATCH v3] Add read_concatenated_archives
As discussed in http://code.google.com/p/libarchive/issues/detail?id=348
the read_concatenated_archives option provides functionality similar to
the GNUtar --ignore-zeros option in order to support reading from
archives which have been concatenated together.

Changes in v2:
 * Replace recursion with looping for reading concatenated archives.

Changes in v3:
 * Improve comments about archive format detection and looping when
   reading concatenated archives.

Signed-off-by: Kevin Locke <kevin@kevinlocke.name>
Add a test for read_concatenated_archives
Make sure that we are reading the concatenated contents when requested
and only when requested.

Signed-off-by: Kevin Locke <kevin@kevinlocke.name>
@kevinoid

This comment has been minimized.

Show comment
Hide comment
@kevinoid

kevinoid Jan 12, 2014

Contributor

I've updated the pull request to incorporate the feedback from the previous version. When you get a chance, let me know what you think.

Contributor

kevinoid commented Jan 12, 2014

I've updated the pull request to incorporate the feedback from the previous version. When you get a chance, let me know what you think.

@kientzle kientzle merged commit 8aab65b into libarchive:master Jan 13, 2014

@kientzle

This comment has been minimized.

Show comment
Hide comment
@kientzle

kientzle Jan 13, 2014

Contributor

On Jan 12, 2014, at 12:48 PM, Kevin Locke notifications@github.com wrote:

I've updated the pull request to incorporate the feedback from the previous version. When you get a chance, let me know what you think.

Looks good!

I've merged this to the master tree.

Two things you might look into if you have time:

  1. Adjusting the bsdtar command-line so that it accepts
 --ignore-zeros

as a synonym for --option read_concatenated_archives
You should just be able to copy a couple of lines of code
from the --option handler.

  1. Consider adding support for --option read_concatenated_archives
    to the cpio format reader as well. A similar strategy should work.

I'm not sure if this actually makes sense for anything other than
tar and cpio.

Cheers,

Tim

Contributor

kientzle commented Jan 13, 2014

On Jan 12, 2014, at 12:48 PM, Kevin Locke notifications@github.com wrote:

I've updated the pull request to incorporate the feedback from the previous version. When you get a chance, let me know what you think.

Looks good!

I've merged this to the master tree.

Two things you might look into if you have time:

  1. Adjusting the bsdtar command-line so that it accepts
 --ignore-zeros

as a synonym for --option read_concatenated_archives
You should just be able to copy a couple of lines of code
from the --option handler.

  1. Consider adding support for --option read_concatenated_archives
    to the cpio format reader as well. A similar strategy should work.

I'm not sure if this actually makes sense for anything other than
tar and cpio.

Cheers,

Tim

@kevinoid kevinoid deleted the kevinoid:read_concatenated_archives branch Jan 15, 2014

@kevinoid

This comment has been minimized.

Show comment
Hide comment
@kevinoid

kevinoid Jan 15, 2014

Contributor

Great, thanks for merging it! I'd like to see --ignore-zeros added for compatibility with GNU tar. Support for cpio is lower priority for me, but it makes a lot of sense to add.

Contributor

kevinoid commented Jan 15, 2014

Great, thanks for merging it! I'd like to see --ignore-zeros added for compatibility with GNU tar. Support for cpio is lower priority for me, but it makes a lot of sense to add.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment