On Sat, Jan 24, 2004 at 01:42:48PM +0100, Mattia Barbon wrote:
> I have some questions about your App::Packer changes.
ok.. hopefully I have some answers.. ;-)
> > ...along with the accessors, mutators, etc, that you might expect
> > with
> > a module such as this.
>
> + my $fe = $self->{FRONTEND} =
> + ($args{frontend}->can('new'))? $args{frontend}->new :
> $args{frontend};
> Is it intentional that passing in a reference to an object having a ->new method
> (such as an App::Packer::Frontend::ModuleInfo instance) will call the method on the
> reference? Can this be changed to:
> my $fe = $args{frontend}; $fe = ref( $fe ) ? $fe : $fe->new;
sure.. At least I don't see any problems
>
> + $self->_set_args( %args );
> + $self->_set_options( %args );
> What does this accomplish that can't be accomplished with ->set_options?
> And why did you need a new ->set_args method?
mostly a matter of convenience and ease of refactoring pp. @ARGV became $self->{args},
etc. Its not absolutely necessary, but it is useful if you are trying to script a
frontend like pp.
You could change that so that args is an element of options, of course, but they
just felt like they should be separate because they are common to both the frontend
and the backend. Instead of:
my $pack = new App::Packer
(
frontend => 'Module::ScanDeps',
backend => 'App::Packer::Backend::PAR',
backopts =>
{
...
args => [EMAIL PROTECTED]
},
frontopts =>
{
args => [EMAIL PROTECTED]
}
);
you'd have
my $pack = new App::Packer
(
frontend => 'Module::ScanDeps',
backend => 'App::Packer::Backend::PAR',
backopts => { ... },
args => [EMAIL PROTECTED]
)
which seems to me cleaner.. of course its not quite as flexible, but
that is how I defined args - as things that were 'common between the two'.
But I also made a 'opts' config var which I defined as 'options that would be
passed both to the backend and the frontend' which is sort of the same thing.
Right now, however the two are mutually exclusive - you can't have both a
frontopts and an opts at the same time. This could/should be changed.
> + $be->set_front($fe) if ($be->can('set_front'));
> App::Packer is all about separating frontend and backend, while this change
> allows to couple them. Could you explain why it was necessary (i.e. why you
> could not separate Module::ScanDeps and PAR functionality)?
I didn't fully separate them out because I thought it would have taken a
large rewrite. Right now, all pp does is use the *module* Module::ScanDeps, and
a couple of its functions.
To do what you suggest would have meant not only refactoring pp, but refactoring
Module::ScanDeps, putting in methods to both extract and transfer data to
App::Packer::Backend::PAR, and a whole bunch of go-between code in App::Packer.
I felt that it would have required more time and energy - tuits - than I had.
If someone wants to write another front-end and backend, say for java code or for
c-code; they are welcome to intermingle the two further - I specifically made it
optional for people to define a set_front method - but its an option that
gives module writers freedom, IMO.
> - $this->_frontend->set_file( $file );
> + $self->backend->set_file($file);
> Why was this change necessary? Apart it breaking
> documented A::P interface, can't the same information
> be gained through the 'file' argument to A::P::B::set_files.
yeah, that could be changed back. Probably should be.. However, since frontend no
longer
'calculates_info' or handles the files, I couldn't use set_files.
> +sub go
> +sub generate_pack
> +sub run_pack
> +sub add_manifest
> +sub pack_manifest
> I could use a bit of explanation on those methods. I reckon they
> are somewhat PAR-specific, but the goal oof A::P is *not* being
> frontend/backend-specific. With that information I hope I will be able
> to fit these methods in a backend-agnostic interface.
Ok,
'go' - allows the user to both generate and run the executable if they so desire.
Right now, it simply runs $fe->go() and $be->go() if they exist. Or, do whatever
the backend says 'go' does..
generate_pack() - just does the generating of the file. Pretty much equivalent
to write, except since all Module::ScanDeps does is scan dependencies, all the
calculating of the size, etc. is done in the backend.
run_pack() - again, runs the packed file that you just generated.
add_manifest() - files that you added 'by hand' - extra files that aren't related to
the packing proces.
pack_manifest() - files that you added through the packing process.
They are general methods but all this right now is done through the backend -
because 'Module::ScanDeps' is again, a light-weight module. You are welcome to try to
migrate the functionality from A::P::B::PAR, but it might be messy because of all
the switches and environmental variables that A::P::B::PAR understands. For example,
if you set the 'a' switch, that means 'add additional files'.
If you are going to change it so the front end does this work, then it too will need
to understand '-a'. And if there's a variable "PAR_ADDITIONAL_FILES" or somesuch
which effects the files that are packed in, then Module::ScanDeps will need to
know about that. That's why I decided to do what I did... it seemed too much work
otherwise, and it seemed the right way to break it out - ScanDeps does the scanning,
Packer decides what to scan and pack.
> +sub add_back_options
> +sub add_front_options
> They seem to be unused and redundant; can they be removed?
All they are are convenience methods. Instead of:
$self->set_options( { frontopts => { .... } }
you say
$self->set_front_options( ... )
I'm not sure if they should be removed, but I think for consistency sake,
they should be renamed...
Ed
> Thanks in advance for your patience.
> Mattia
>
no not at all.. thanks for the module... I'm just sorry I didn't have time enough
to make PAR compliant with it without bending it a bit. You are welcome to
take my changes and do with them whatever you like. However if I was you I'd
try refactoring A::P::B::PAR and Module::ScanDeps to fit whatever you come up with..
Ed
