Junio C Hamano <gitster@xxxxxxxxx> wrote:
> Lea Wiemann <lewiemann@xxxxxxxxx> writes:
>
> > Marcus Griep wrote:
> >> diff --git a/perl/Git.pm b/perl/Git.pm
> >>
> >> +require File::Spec;
> >
> > This makes Git.pm dependent on Perl 5.6.1.
>
> Ouch. Thanks for being extra careful.
>
> Unfortunately I've already pulled these changes via Eric.
>
> Among the existing Perl scripts, cvsexportcommit and cvsimport already do
> use it, so do svnimport and cidaemon in contrib.
>
> > ... Hence to avoid
> > complaints about failing tests, I suggest that you add a check for
> > File::Spec availability at the beginning of any test that (indirectly)
> > uses Git.pm.
>
> Hmm, wouldn't something like this (untested) be more contained?
>
> ---
> perl/Git.pm | 16 ++++++++++++++--
> 1 files changed, 14 insertions(+), 2 deletions(-)
>
> diff --git a/perl/Git.pm b/perl/Git.pm
> index 405f68f..2a92945 100644
> --- a/perl/Git.pm
> +++ b/perl/Git.pm
> @@ -1023,7 +1035,7 @@ sub _temp_cache {
> }
What about just lazy requiring inside _temp_cache() so it
won't get loaded by folks that don't need it? (completely untested):
eval { require File::Temp };
if ($@) {
throw Error::Simple("couldn't require File::Temp: $@");
}
eval { require File::Spec };
if ($@) {
throw Error::Simple("couldn't require File::Spec: $@");
}
It'll also remove the minor performance hit CGI/gitweb users got since
we won't load these extra modules during startup.
> $$temp_fd = File::Temp->new(
> TEMPLATE => 'Git_XXXXXX',
> - DIR => File::Spec->tmpdir
> + DIR => $tmpdir,
> ) or throw Error::Simple("couldn't open new temp file");
> $$temp_fd->autoflush;
> binmode $$temp_fd;
Fwiw, git-svn has been a File::Temp user for a few months since Adam's
cat-file optimization; but it also has lower visibility since boxes
with <5.6.1 probably don't have SVN. git-svn previously used
IO::File->new_tmpfile exclusively (and very heavily).
--
Eric Wong
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
