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

Support for pulling prerequisites described in a cpanfile. #144

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Support for pulling prerequisites described in a cpanfile. #144

wants to merge 4 commits into from

Conversation

hartzell
Copy link

Hi Jeff,

Here's a first cut at support for reading prerequisites from a cpanfile.

I ended up slipping it into the Pull action, it seemed to be minimally disruptive and I couldn't bring myself to cut-and-paste the rest of the Pull stuff.

Added a test to make sure that handling no targets and no cpanfile does the right thing and added a bunch of tests for various cpanfile monkeyshines. I'm not entirely sure that I'm using the test stuff to its fullest potential.

Comments, please.

Pull requires targets.  Add a test for being called w/out any and
check the exception that gets thrown.
Add an optional argument to the Pull action that names a cpanfile.

1. loads the file using Module::CPANfile,
2. merges prerequisites from the
   phases = qw(configure build test runtime develop) and the
   types = qw(requires recommends suggests)
3. and uses the resulting requirements to add packages to the set
   of targets to be pulled.

Targets are no longer required (in the Moose attribute sense) so
add a test to make sure that we still correctly handle not having
any.
@thaljef
Copy link
Owner

thaljef commented Feb 23, 2014

I see: cpanfile.snapshot is the new name for carton.lock.

@hartzell
Copy link
Author

ps. There's an obvious fancier step of adding command line switches to explicitly specify phase/type. Not sure if it's worth it, whether it's better to specify e.g. "test and everything below" or name each individually, etc... Wait for a use case....

@thaljef
Copy link
Owner

thaljef commented Feb 24, 2014

This looks great. Try it for a while and then we can think about merging it.

Lately, I've been looking into extending Pinto to support Ruby and/or Python. So I might prefer to put this in a separate command, since it is very Perl-specific. But we'll cross that bridge when we get to it.

George Hartzell added 2 commits March 11, 2014 10:36
Add a --cpanfile option to the pull command.  Option currently
requires a value.  TODO supply a default value of 'cpanfile'.

I had to "enhance" the way that arg_attribute and args_from_stdin
work.  Prior to this change they had no information about the context
in which they were running and just returned constants.

Now they are passed the $opts and $args from above and can decide
whether the Action should collect stuff from stdin or not.

In this case, if a cpanfile is specified then there should be no
additional args, either on the command line or on stdin.
@thaljef
Copy link
Owner

thaljef commented May 3, 2014

Some thoughts...

If we add --cpanfile to the pull command, the next logical step is to add it to the install command. Therefore, I would want to create a role (maybe Pinto::Role::CpanfileReader) that does the work of turning a cpanfile into a list of targets.

Supporting --cpanfile with remote repositories is a bit tricky because the client will have to read the file locally, translate them into targets, and then send the request to the servers. I guess it could also just send the cpanfile itself to the server, but that doesn't seem right to me.

Either way, the solution is to create a custom Pinto::Remote::Action subclass that does the necessary client-side stuff. Pinto::Remote::Action::Add and Pinto::Remote::Action::Install are examples of this. All other commands just use the Pinto::Remote::Action base class which simply packs the arguments into some JSON and POSTs it to the server.

I see that you also had to tweak some stuff up in App::Pinto::Command. That is probably unavoidable, but I want to look at that carefully. It might also make sense to make the argument for --cpanfile optional, and default it to just ./cpanfile since I think that is the customary filename.

Down the road, it will probably make sense to have options that affect what Pinto does with respect to prereqs for different phases (e.g. configure|build|test|runtime|develop) which would also impact how the cpanfile is used. But I haven't figured out what that should look like.

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