search

Perl5-Alien / Alien-Base

Base classes for Alien:: modules (deprecated, see Alien-Build)
Other
35 stars 19 forks source link

move archive extraction logic into overridable method #142

Closed salva closed 8 years ago

salva commented 8 years ago

The logic to extract archives (i.e. tar files) was hardcoded inside the 'ACTION_alien_code' method to use Archive::Extract. That didn't allowed one to use some custom mechanism.

This patch moves that logic into the new method 'alien_extract_archive' which can be overrided in subclasses.

plicease commented 8 years ago

This would be useful. Aye.

jberger commented 8 years ago

It seems that the new method returns a relative path which is cleaned up later. Should it be made to return an absolute path? It seems like that would be the most consistent api design but perhaps I'm overthinking?

plicease commented 8 years ago

That would allow the method return a relative path, but the path stored would be absolute.

salva commented 8 years ago

@jberger, Archive::Extract returns an absolute path, so in the past, making it absolute was not necessary. The default implementation for alien_extract_archive still relies on Archive::Extract and so it returns an absolute path.

The issue is that an overridden alien_extract_archive may return a relative path and that would cause a later stage to fail, so this patch just forces the path to be absolute to make the life of the module user easier.

salva commented 8 years ago

@plicease, what's the advantage of doing that over calling File::Spec:rel2abs explicitly?

plicease commented 8 years ago

@salva nothing really.

plicease commented 8 years ago

If you put the File::Spec:rel2abs back then there are less unknowns, that might be best.

plicease commented 8 years ago

Also, although I don't think it should be a problem, my suggestion would store "\" style paths in the working_directory on windows, so nevermind my terrible idea sorry!

plicease commented 8 years ago

@jberger, as per what @salva said, I think making sure that an overridden version of this method will be converted into absolute is the best thing

jberger commented 8 years ago

That's a reasonable point. Don't assume, ensure. In that case, Aye!

On Wed, Dec 30, 2015 at 12:08 PM Graham Ollis notifications@github.com wrote:

@jberger https://github.com/jberger, as per what @salva https://github.com/salva said, I think making sure that an overridden version of this method will be converted into absolute is the best thing

— Reply to this email directly or view it on GitHub https://github.com/Perl5-Alien/Alien-Base/pull/142#issuecomment-168047030 .

mohawk2 commented 8 years ago

Two ayes have it!