Re: [PATCH] Revert "gitweb: Time::HiRes is in core for Perl 5.8"
- From
Ævar Arnfjörð Bjarmason <avarab@gmail.com>
- Date
- Jan 29, 2012, 02:21 UTC
- Message-ID
- <CACBZZX7M83PAdLPXLYoBtcihf=5AHruk9=JZo7mh+uNyLtaOhg@mail.gmail.com>
- In-Reply-To
- <201201271845.39576.jnareb@gmail.com>
On Fri, Jan 27, 2012 at 18:45, Jakub Narebski <jnareb@gmail.com> wrote:
Show 17 quoted lines
> On Mon, 23 Jan 2012, Ævar Arnfjörð Bjarmason wrote: >> On Mon, Jan 23, 2012 at 05:50, Hallvard Breien Furuseth <h.b.furuseth@usit.uio.no> wrote: >> > >> > t9500-gitweb-standalone-no-errors fails: Git 1.7.9.rc2/1.7.8.4, RHEL >> > 6.2, Perl 5.10.1. Reverting 3962f1d756ab41c1d180e35483d1c8dffe51e0d1 >> > fixes it. The commit expects Time::HiRes to be present, but RedHat >> > has split it out to a separate RPM perl-Time-HiRes. Better add a >> > comment about that, so it doesn't get re-reverted. >> > >> > Or pacify the test and expect gitweb@RHEL-users to install the RPM: > [...] > >> This doesn't actually fix the issue, it only sweeps it under the rug >> by making the tests pass, gitweb will still fail to compile on Red >> Hat once installed. > > I think you meant "fail to run" here.
I mean fail to compile, "use" statements are executed at compile time, if it was a "require" outside of BEGIN-time it would fail at runtime.
I realize though that you probably thought I meant fail in Git's Makefile-driven compilation phase, but no, it'll install just fine, however the perl interpreter won't compile it.
Show 13 quoted lines
>> I think the right solution is to partially revert >> 3962f1d756ab41c1d180e35483d1c8dffe51e0d1, but add a comment in the >> code indicating that it's to deal with RedHat's broken fork of Perl. > > I have added comment in commit message, but not in code. I wonder if > it would be enough. > >> However even if it's required in an eval it might still fail at >> runtime in the reset_timer() function, which'll need to deal with it >> too. > > It shouldn't; everything else related to timer is protected with > 'if defined $t0', which is false if Time::HiRes module is not available.
Correct, I didn't look carefully enough.