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

Add 7z container support #2744

Merged
merged 1 commit into from
Feb 25, 2014

Conversation

rolandwalker
Copy link
Contributor

relevant Casks must use depends_on_formula unar

Tests are handled similarly to the (updated) PR #1992 -- they will be skipped unless the developer has unar installed.

The system /usr/bin/file cannot recognize 7z files, so extension and magic_number methods were added to criteria.rb to aid in determining filetypes.

Unlike #1992, this is implemented in terms of an abstract base class Cask::Container::UnarBase. We can use the base class to support Stuffit containers and whatever else is desired from the broad number of filetypes supported by unar. (I plan only 7z, Stuffit, and RAR).

Since unar provides a consistent interface across many archive types, it would actually be possible to skip having a Cask::Container::SevenZip class entirely in favor of a Cask::Container::Unarable which would come last in the list of container classes and function as a kind of catch-all. However, for both technical and stylistic reasons, I think being explicit is a better approach.

@nanoxd
Copy link
Contributor

nanoxd commented Feb 5, 2014

If this allows less complexity in the project, I'm all for it.

@muescha
Copy link
Contributor

muescha commented Feb 7, 2014

Can this depends_on_formula unar better goes from the cask to the unarable lib?

@rolandwalker rolandwalker mentioned this pull request Feb 7, 2014
@rolandwalker
Copy link
Contributor Author

rebased

@rolandwalker
Copy link
Contributor Author

@nanoxd , if you meant that the "catch-all" notion would reduce complexity, not really. Once the unar base class here is introduced, then, for example, adding Stuffit containers takes only 8 lines of code. And the problems of the "catch-all" class are many: it's literally requesting undefined behavior. I can imagine something like #2654 being complicated by the fact that unar also reads DMG files -- the file could "fall through" and our code could take two different cracks at it.

@muescha if you are curious about how the design decisions were reached, you may refer to #1992 and #2305 .

relevant Casks must use `depends_on_formula unar`
rolandwalker added a commit that referenced this pull request Feb 25, 2014
@rolandwalker rolandwalker merged commit 76602af into Homebrew:master Feb 25, 2014
@rolandwalker rolandwalker deleted the sevenzip_containers branch February 25, 2014 17:51
rolandwalker added a commit to rolandwalker/homebrew-cask that referenced this pull request Feb 25, 2014
This will not pass Travis until Homebrew#2744 is merged
rolandwalker added a commit to rolandwalker/homebrew-cask that referenced this pull request Feb 25, 2014
this will not pass Travis until Homebrew#2744 is merged
@muescha
Copy link
Contributor

muescha commented Feb 26, 2014

my comment i continued in #3207

@Homebrew Homebrew locked and limited conversation to collaborators May 8, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants