#426: install_files.pl and install_dev_files.pl have a fair bit of duplicate
code
---------------------+------------------------------------------------------
Reporter: wayland | Owner: jkeenan
Type: patch | Status: assigned
Priority: normal | Milestone:
Component: install | Version: 0.9.1
Severity: medium | Keywords: install
Lang: | Patch: new
Platform: |
---------------------+------------------------------------------------------
Comment(by jkeenan):
Replying to [comment:12 jkeenan]:
>
> We would appreciate it if people could review this patch and give the
new versions of ''install_files.pl'' and ''install_dev_files.pl'' some
road testing.
I decided to take my own advice and try to use ''make install'' for the
first time, albeit with `--prefix` set to a disposable directory. Results
were not good. In both ''make install'' and ''make install-dev'' I got
failures like this:
{{{
/usr/local/bin/perl tools/dev/install_files.pl \
--buildprefix= \
--prefix=/pseudoinstall \
--exec-prefix=/pseudoinstall \
--bindir=/pseudoinstall/bin \
--libdir=/pseudoinstall/lib \
--includedir=/pseudoinstall/include \
--destdir= \
--docdir=/pseudoinstall/share/doc \
--versiondir=/parrot/1.1.0-devel \
MANIFEST MANIFEST.generated
Unknown install location in MANIFEST for file 'LICENSE
[main]doc'
make: *** [install] Error 9
}}}
`LICENSE` is the first file in ''MANIFEST'' that has a defined location:
`[main]doc`. At first I wondered whether this might have been due to the
fact that in ''config/init/install.pm'', we had not yet changed `doc_dir`
to `docdir`. I made that replacement globally, but I continued to get the
same results.
I then ran ''install_files.pl'' through the Perl 5 debugger. I discovered
that the stumbling block is in this loop inside
`Parrot::Install::lines_to_files()`:
{{{
foreach my $tkey (@$transformorder) {
$thash = $metatransforms->{$tkey};
}}}
### NEXT LINE IS PROBLEMATIC
{{{
unless($thash->{ismeta} ? $metadata{$tkey} : $entry =~
/$tkey/) { next; }
$filehash = &{ $thash->{transform} }($filehash);
ref($filehash) eq 'HASH' or die "Error: transform didn't
return a hash for key '$tkey'\n";
$filehash->{Dest} = File::Spec->catdir(
$options_ref->{$thash->{optiondir} . 'dir'},
@{ $filehash->{DestDirs} },
$filehash->{Dest}
);
last FIXFILE;
}
die "Unknown install location in MANIFEST for file
'$entry'\n";
}}}
I found that as we loop through the elements of `...@$transformorder`,
`LICENSE` never gets past the `next` in the line marked PROBLEMATIC. That
suggests that `$filehash` -- and `$filehash->{Dest}` in particular -- is
never well defined. Consequently, we fall through to the `die` beneath
the loop.
I don't yet understand what's going on here, but I must say that the
control flow of this line is really baffling:
{{{
unless($thash->{ismeta} ? $metadata{$tkey} : $entry =~ /$tkey/) { next;
}
}}}
An `unless` statement whose condition is itself a ternary!
So our last submitted patch is not yet ready for merging into trunk.
Suggestions? Stay tuned!
Thank you very much.[[BR]]
kid51
--
Ticket URL: <https://trac.parrot.org/parrot/ticket/426#comment:13>
Parrot <https://trac.parrot.org/parrot/>
Parrot Development
_______________________________________________
parrot-tickets mailing list
[email protected]
http://lists.parrot.org/mailman/listinfo/parrot-tickets
