Bug #14397 for Filter-Crypto: Patch to correctly detect x86

Thu Sep 01 04:10:26 2005 cosimo [...] cpan.org - Ticket created

Subject:

Patch to correctly detect x86_64 crypto libs

The attached patch allows a clean build also on x86_64 machines, where Filter::Crypto::_locate_lib_dir() fails to locate `$(PREFIX)/lib64' dirs. This happens for me on a Red Hat Enterprise Linux 4.0 installation. Attached patch solves the problem and allows clean build and install.

--- Filter-Crypto-1.14/inc/Module/Install/PRIVATE/Filter/Crypto.pm 2005-06-01 12:02:30.000000000 +0200 +++ Filter-Crypto-1.14_cosimo/inc/Module/Install/PRIVATE/Filter/Crypto.pm 2005-09-01 10:05:25.301147947 +0200 @@ -465,7 +465,13 @@ # or out/ (up to and including 0.8.1b). my $prefix_dir = $self->_prefix_dir(); my($dir, $lib_dir); - if (-d ($dir = catdir($prefix_dir, 'lib'))) { + + # Detect 64bit libraries (x86_64 machines) + if (-d ($dir = catdir($prefix_dir, 'lib64'))) { + $lib_dir = $dir; + } + # Or plain `lib'... + elsif (-d ($dir = catdir($prefix_dir, 'lib'))) { $lib_dir = $dir; } elsif ($self->is_win32()) {

Thu Sep 01 04:58:12 2005 shay [...] cpan.org - Correspondence added

[COSIMO - Thu Sep 1 04:10:26 2005]: Show quoted text

> The attached patch allows a clean build also on x86_64 machines, > where Filter::Crypto::_locate_lib_dir() fails to locate > `$(PREFIX)/lib64' dirs. > This happens for me on a Red Hat Enterprise Linux 4.0 installation. > Attached patch solves the problem and allows clean build and install.

Thanks for the bug report and patch. It would feel more natural to me to check for the lib directory before the lib64 directory. If you could confirm that this would also work for you then I'll apply your patch to my development version and make a new release soon.

Thu Sep 01 06:15:40 2005 cosimo [...] cpan.org - Correspondence added

[SHAY - Thu Sep 1 04:58:12 2005]: Show quoted text

> [COSIMO - Thu Sep 1 04:10:26 2005]: >

> > The attached patch allows a clean build also on x86_64 machines, > > where Filter::Crypto::_locate_lib_dir() fails to locate > > `$(PREFIX)/lib64' dirs. > > This happens for me on a Red Hat Enterprise Linux 4.0 installation. > > Attached patch solves the problem and allows clean build and

> install. > > Thanks for the bug report and patch.

Thanks for your quick reply. Show quoted text

> It would feel more natural to me to check for the lib directory before > the lib64 directory. If you could confirm that this would also work > for > you then I'll apply your patch to my development version and make a > new > release soon.

I also thought so, but in 64 bit environments there is also the `/usr/lib' folder. Putting `/usr/lib' detection first is always going to ignore `/usr/lib64' folder, even if it is present. So I had to move `lib64' detection first, or it didn't work properly...

Thu Sep 01 06:39:34 2005 shay [...] cpan.org - Correspondence added

[COSIMO - Thu Sep 1 06:15:40 2005]: Show quoted text

> [SHAY - Thu Sep 1 04:58:12 2005]: >

> > It would feel more natural to me to check for the lib directory

> before

> > the lib64 directory. If you could confirm that this would also work > > for > > you then I'll apply your patch to my development version and make a > > new > > release soon.

> > I also thought so, but in 64 bit environments there is also the > `/usr/lib' folder. Putting `/usr/lib' detection first is always going > to > ignore `/usr/lib64' folder, even if it is present. > So I had to move `lib64' detection first, or it didn't work > properly...

Oh, I see. That's my fault for testing for the lib dir separately from the lib file. I will change it to look inside each candidate lib dir to see if an appropriate lib file exists in it, and only select a lib dir that actually contains the necessary lib file. That way it'll reject /usr/lib because it doesn't contain a libcrypto.a and then move onto /usr/lib64. The inc dir detection works in much this way already. I should have done the same for the lib dir from the outset.

Thu Sep 01 08:57:49 2005 shay [...] cpan.org - Correspondence added

[SHAY - Thu Sep 1 06:39:34 2005]: Show quoted text

> Oh, I see. That's my fault for testing for the lib dir separately > from > the lib file. > > I will change it to look inside each candidate lib dir to see if an > appropriate lib file exists in it, and only select a lib dir that > actually contains the necessary lib file. > > That way it'll reject /usr/lib because it doesn't contain a > libcrypto.a > and then move onto /usr/lib64. > > The inc dir detection works in much this way already. I should have > done the same for the lib dir from the outset.

I have worked the changes described above into my current development version, which I have attached here. Would you be able to test this and confirm that the lib64 library detection now works? If it's all OK then I'll release this very soon. Thanks.

Download Filter-Crypto-1.15.beta.tar.gz
application/x-gzip 132.6k

Message body not shown because it is not plain text.

Thu Sep 01 14:15:50 2005 cosimo [...] cpan.org - Correspondence added

[SHAY - Thu Sep 1 08:57:49 2005]: Show quoted text

> [SHAY - Thu Sep 1 06:39:34 2005]: >

> > I will change it to look inside each candidate lib dir to see if an > > appropriate lib file exists in it, and only select a lib dir that > > actually contains the necessary lib file.

> > I have worked the changes described above into my current development > version, which I have attached here. > > Would you be able to test this and confirm that the lib64 library > detection now works?

It does not work. It fails detecting the lib name, and thus it fails to build. Reason is that `$lib_name' is empty at the end of _probe_for_lib_file(). Please check the attached patch that seems to correct the problem. Builds and tests fine for me. Probably, if one has `/usr/lib' *and* `/usr/lib64' both installed, `lib64' should be preferred... That's just my € 0.02...

--- Filter-Crypto-1.15/inc/Module/Install/PRIVATE/Filter/Crypto.pm 2005-09-01 14:16:50.000000000 +0200 +++ Filter-Crypto-1.15_cosimo/inc/Module/Install/PRIVATE/Filter/Crypto.pm 2005-09-01 20:08:36.000000000 +0200 @@ -564,6 +564,11 @@ $lib_file = $file; $lib_name = 'crypto'; } + else + { + # No library found! + return; + } } return $lib_file, $lib_name;

Fri Sep 02 04:02:53 2005 shay [...] cpan.org - Correspondence added

[COSIMO - Thu Sep 1 14:15:50 2005]: Show quoted text

> [SHAY - Thu Sep 1 08:57:49 2005]: >

> > [SHAY - Thu Sep 1 06:39:34 2005]: > >

> > > I will change it to look inside each candidate lib dir to see if

> an

> > > appropriate lib file exists in it, and only select a lib dir that > > > actually contains the necessary lib file.

> > > > I have worked the changes described above into my current

> development

> > version, which I have attached here. > > > > Would you be able to test this and confirm that the lib64 library > > detection now works?

> > It does not work. > It fails detecting the lib name, and thus it fails to build. Reason is > that `$lib_name' is empty at the end of _probe_for_lib_file(). > Please check the attached patch that seems to correct the problem. > Builds and tests fine for me.

Doh! You're quite correct - _probe_for_lib_file() needs to return an empty list, not a ("true") list of undefined values, if the lib is not found. I applied a variation of your patch to make sure that other routes through the "if" block that are unsuccessful also return an empty list. Show quoted text

> > Probably, if one has `/usr/lib' *and* `/usr/lib64' both installed, > `lib64' should be preferred... That's just my € 0.02...

OK, I've swapped the check around to probe lib64 first. The new version (1.15) is now uploaded to CPAN. Thanks very much for your assistance!

Mon Sep 05 07:15:34 2005 shay [...] cpan.org - Correspondence added

Did the new release work OK for you? Can I close this ticket now?

Tue Sep 06 02:33:32 2005 cosimo [...] cpan.org - Correspondence added

[SHAY - Mon Sep 5 07:15:34 2005]: Show quoted text

> Did the new release work OK for you? Can I close this ticket now?

Sorry for the delay. I was trying to test 1.15 in different environments. 32/64 bit libraries detection works ok. However: 1) there is a problem with `scripts/crypt_file' that seems to be DOS formatted (cr+lf). This makes a test in `t/03_script.t' fail. It seems that pod2usage() is not able to extract the documentation from DOS formatted files (?), and `make test' fails. Converting `scripts/crypt_file' with dos2unix solves the problem. 2) Another problem is in `t/04_par.t'. I must tell you that I always had this problem, also with older versions of Filter::Crypto. I can report and detail it in another ticket (or by simple email) if you want. Excluding problem 1), 1.15 is ok with me.

Tue Sep 06 05:29:53 2005 shay [...] cpan.org - Correspondence added

[COSIMO - Tue Sep 6 02:33:32 2005]: Show quoted text

> [SHAY - Mon Sep 5 07:15:34 2005]: >

> > Did the new release work OK for you? Can I close this ticket now?

> > Sorry for the delay. > I was trying to test 1.15 in different environments. > 32/64 bit libraries detection works ok. > However: > > 1) there is a problem with `scripts/crypt_file' that seems to > be DOS formatted (cr+lf). This makes a test in `t/03_script.t' fail. > It seems that pod2usage() is not able to extract the documentation > from > DOS formatted files (?), and `make test' fails. > Converting `scripts/crypt_file' with dos2unix solves the problem.

You're quite correct. Release 1.15 accidentally made all the files Windows end-of-line format :-( I've made a new release (1.16) which corrects this. Thanks for the spot. Show quoted text

> > 2) Another problem is in `t/04_par.t'. I must tell you that I always > had > this problem, also with older versions of Filter::Crypto. I can report > and detail it in another ticket (or by simple email) if you want.

A new ticket would be useful to assist anyone else with the same problem who is searching the archives. Show quoted text

> > Excluding problem 1), 1.15 is ok with me.

Cool. I'll close this ticket then.

Tue Sep 06 05:30:47 2005 shay [...] cpan.org - Status changed from 'new' to 'resolved'

Tue Sep 06 05:31:05 2005 shay [...] cpan.org - Fixed in 1.15 added

Bug #14397 for Filter-Crypto: Patch to correctly detect x86_64 crypto libs