On Mon, 10 Aug 2009 07:50 +0200, "Thomas Klausner" <d...@cpan.org> wrote: > Hi! > > On Sat, Aug 08, 2009 at 09:07:55AM -0600, Curtis Jewell wrote: > > This should be close to executable, (I pulled it out of another project > > of mine) and once cleaned up, should do the trick: > > Would it be ok for you if I tried to integrate your code into > Module::Build alongside Davids suggestions?
Sure. Not a problem. One thing I oopsed on: This: if (0 == scalar (%files)) { croak 'Empty or no MANIFEST file'; } should be this: if (0 == scalar (%{$files})) { croak 'Empty or no MANIFEST file'; } > One more short question: > > > FILE: > > foreach my $direntry ( readdir $dir_handle ) { > > Why are you not using File::Find or similar? Probably because I would have: 1) Had to go back to 5.00503's File::Find and seen if it did what I wanted it to. [This code was supposed to be runnable on 5.00503 without any additional modules (other than Module::Build and its requirements) being installed. Of course, the last version of Module::Build to be installable on that low a version of perl was 0.2808_04 - but I was willing to support doing that. Since newer versions are requiring 5.6.1, you can delete the gensym call and the code around it. ] 2) Had to decide to not use it, since I couldn't pass parameters (specifically, the $files hash and the destination directory) into the subroutine it calls on each file. I would need File::Find::find to return a list instead, and then loop over the list to check it against the manifest. Easier to just readdir the directories and process file files myself [I want to keep use of global or package-level 'my' variables to a minimum. I'm already using too many for my comfort.] Maybe I could have put the wanted subroutine within the install_share subroutine, so it could have used its 'my' variables... but that looks even uglier than just doing a recursive foreach loop against the directory contents. 3) Also, keeping track of the "3 locations" each file has to go would be even harder (and consume more CPU time, because I would have had to make some File::Spec calls per-file, rather than per-directory) than it is now. [The "3 locations" being referred to are the source location in local format, the source location in Unix format (in order to compare to the MANIFEST file,) and the destination (in blib) location in local format.] Also, for your information, as you do the integration: 1) The global variable %ARGS is supposed to be the Module::Build options hash - M::B::Functions calls Module::Build->new (or Module::Build::Subclass->new) at the end of processing all its routines with that as a parameter. Of course, since the goal is to integrate this into Module::Build now, you can just call its routines instead of setting (and accessing) keys in a multi-level argument hash. 2: The package-level 'my' variable @install_types is the list of install types I need to add to the Module::Build object after I create it, like so: foreach my $type (@install_types) { $object->add_build_element($type); } Since again, we're working with an already created Module::Build object, we can just call add_build_element() on it immediately, instead of having to store a list and waiting to do it at the end. 3: You'll see calls to _installdir() twice in there. I should have shown what that did to you, as well. sub _installdir { return $Config{'sitelibexp'} unless ( defined $ARGS{install_type} ); return $Config{'sitelibexp'} if ( 'site' eq $ARGS{install_type} ); return $Config{'privlibexp'} if ( 'perl' eq $ARGS{install_type} ); return $Config{'vendorlibexp'} if ( 'vendor' eq $ARGS{install_type} ); croak 'Invalid install type'; } --Curtis -- Curtis Jewell swords...@csjewell.fastmail.us %DCL-E-MEM-BAD, bad memory -VMS-F-PDGERS, pudding between the ears [I use PC-Alpine, which deliberately does not display colors and pictures in HTML mail]