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

Initial support for AIX big archive format. #3

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

Conversation

adiroiban
Copy link

Problem description

This branch add basic support for AIX AR big format archive as documented here: https://publib.boulder.ibm.com/infocenter/pseries/v5r3/index.jsp?topic=/com.ibm.aix.files/doc/aixfiles/ar_big.htm

Changes description

A new archive class was created since the format is different than BSD/GNU.

file type is reported as HEADER_GENERIC

file header for each file from archive is variable and this is why it is parsed from 2 read operations.

I have switched some methods from hidden (__X) to private (_X) so that I can reuse them in the new code.

New code is written with PEP8 and PEP257 (for docstring) styleguide...with the exception of tab indentation.
Please let me know if I should use other styleguide.

The code is written using TDD and this is why all tests are placed in a single file. There is no dedicated test file for failures. All tests for the new format are located in a single file, with docstring documenting the purpose of each tests. Also, no external / system files were used for testing, all file content is in memory or generated in tests.

How to try and test the changes

Please check that changes make sense and let me know if anything needs changes.

If you want to check a real file here are some files copied from an AIX system:

Simple archive libg.a
https://dl.dropboxusercontent.com/u/174543/aix-ar/libg.a

libpam with 2 members, with initial padding and 1 with padded header and anotehr without padded header
https://dl.dropboxusercontent.com/u/174543/aix-ar/libpam.a

shr.0 from libpam.a extracted using aix ar tool:
https://dl.dropboxusercontent.com/u/174543/aix-ar/shr.o

Thanks!

@viraptor
Copy link
Owner

Great! I didn't know AIX has its own extension. Will be really happy to include it. Reading the changes now...

@viraptor
Copy link
Owner

Really appreciate the effort.

One thing that will really need to be fixed is support for python2.6--3.3 (travis above has all the info).

Otherwise, I see the change could share some more code (for example both archive classes have similar init-s). If the AIX format is different enough to make a separate class hierarchy worth it, then maybe it's better to separate both BSD and GNU in the same way. This would also mean getting the inheritance/interfaces consistent - for example: AIXBigFileHeader doesn't inherit ArchiveFileHeader now and ArchiveFileHeader could use relative_file_offset of its own.
If they're not that different, I'd rather put AIX handling in the current classes behind some conditionals.

I'll definitely merge your change, but I'm not sure in what form it will end up yet... Probably if/when I get bored during the holidays :)

@adiroiban
Copy link
Author

I will fix the python2.6 and 3.3.

I will also change to code to share more code.

For the initial diff, I wanted to focus on new functionality without touching / refactoring the existing code and keep the diff to a minimum to simplify the review.

Thanks for the review.

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