Skip to content

Add all modules to the DarkPAN, not just the main one. #3

Open
wants to merge 1 commit into from

1 participant

@thaljef
thaljef commented Jul 7, 2011

Instead of adding only the "main" module, we add all the
modules that the distro provides. This is basically what
the Pause indexer would do.

CPAN::Mini::Inject is flawed in this regard. The add()
method is basically designed to inject distros that contain
only a single module. But any non-trivial distro will
have several modules. The mcpani[1] command works around
this limitation by calling the add() method for each
module that the distro provides. Each call to add() also
copies the *.tar.gz into the repository. So it ends up
copying the same file over and over. I think the correct
solution is to change the interface on the add() method
to support multiple module names, which could all have
different version numbers.

I suspect the decision to design CPAN::Mini::Inject is the
result of an old anti-pattern. Once upon a time, it was
typical for CPAN authors to only declare dependence
on the main module of a distro within their Makefile.PL
or Build.PL script. For example, my code might say:

use PPI::Document;

but in my Makefile.PL it would say:

requires => { 'PPI' => 0 };

This implies that PPI and PPI::Document will always be in
the same distro. But you cannot rely on that -- authors
are free to move modules into other distros at any time.
Technically, this would not constitute an interface
change, because the dependency is upon the module, not
the distribution it belongs to.

These days, the correct approach is to list every module
that you C in your code as a dependency. This
ensures you'll get the physical files that you require,
even if their logical location moves from one distro to
another.

Jeffrey Thalhammer Add all modules in the dist, not just the main one
Instead of adding only the "main" module, we add all the
modules that the distro provides.  This is basically what
the Pause indexer would do.

CPAN::Mini::Inject is flawed in this regard.  The add()
method is basically designed to inject distros that contain
only a single module.  But any non-trivial distro will
have several modules.  The mcpani[1] command works around
this limitation by calling the add() method for each
module that the distro provides.  Each call to add() also
copies the *.tar.gz into the repository.  So it ends up
copying the same file over and over.  I think the correct
solution is to change the interface on the add() method
to support multiple module names, which could all have
different version numbers.

I suspect the decision to design CPAN::Mini::Inject is the
result of an old anti-pattern.  Once upon a time, it was
typical for CPAN authors to only declare dependence
on the *main* module of a distro within their Makefile.PL
or Build.PL script.  For example, my code might say:

use PPI::Document;

but in my Makefile.PL it would say:

requires => { 'PPI' => 0 };

This implies that PPI and PPI::Document will always be in
the same distro.  But you cannot rely on that -- authors
are free to move modules into other distros at any time.
Technically, this would not constitute an interface
change, becaus the dependency is upon the module, not
the distribution it belongs to.

These days, the correct approach is to list every module
that you C<use> in your code as a dependency.  This
ensures you'll get the physical files that you require,
even if their logical location moves from one distro to
another.
0bfc5f2
@thaljef
thaljef commented Jul 12, 2011

FYI: I've submitted a patch to allow CPAN::Mini::Inject::add() to accept multiple module names/versions. I believe this is preferable to calling add() multiple times as I've done on this fork. If my changes to CPAN::Mini::Inject are accepted, I'll revise this fork of CPAN::Dark. My pull request for CPAN::Mini::Inject is here:

AndyA/CPAN--Mini--Inject#12

Also, magnificent-tears has also opened this pull request on a different fork of CPAN::Mini:

wchristian/CPAN--Mini--Inject#2

If the module discovery feature is moved into the CPAN::Mini library as magnificent-tears suggest, then that would eliminate the need for CPAN::Dark to do it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.