Can't install `` for `perl-5.10.1` on macOS 14 Sonoma

[bombsimon]

Can't install `[email protected]` for `perl-5.10.1` on macOS 14 Sonoma

[autarch]

I can't get my CI to work with older Perls on macOS. I'll do a trial release with the fix and monitor CPAN Testers.

[autarch]

Fixed in 1.65.

[bombsimon]

I'm not familiar with the API in perl.h at all but I tried to be naive and just removed the definition of dt_isfinite if on macOS and with this diff all the tests now passes with make test with perl 5.10.1:

diff --git a/DateTime.xs b/DateTime.xs
index 09778cf..9409144 100644
--- a/DateTime.xs
+++ b/DateTime.xs
@@ -13,7 +13,9 @@
 #     define dt_isfinite Perl_isfinite
 #   endif
 # else
-#   define dt_isfinite Perl_isfinite
+#   ifndef __APPLE__
+#     define dt_isfinite Perl_isfinite
+#   endif
 # endif

 #define DAYS_PER_400_YEARS  146097

Looking at perl.h bundled with perl 5.10.1 I don't even see they refer to Perl_isfinite so that would explain why it's not defined on 5.10. Compared to on the blead branch where it's mentioned 37 times and seems guaranteed to be set. No clue why it works on 5.10 on Linux in CI though 🤔

Never mind, it's for sure defined here: https://github.com/Perl/perl5/blob/5348debf9fd57fc15c26529386769684fab96e57/perl.h#L2140-L2154

Seems to be this line that fails which is the one calling Perl_is_inf which fails: https://github.com/Perl/perl5/blob/5348debf9fd57fc15c26529386769684fab96e57/perl.h#L2150. Not sure if we should add an extra guard to this codebase and check if Perl_is_inf is defined before setting dt_isfinite or if we should just skip this for Apple on <= 5.10. Haven't tested if this works on versions between 5.10.1 and 5.32.0.

[bombsimon]

Is this a typo that never got discovered somehow? Searching for Perl_is_inf yields very few results.

It was originally added 2001 in Perl/perl5@758a5d7, updated in 2014 in Perl/perl5@4efd38a and then shortly after removed in Perl/perl5@25c46df but without any mention regarding the use of Perl_is_inf and Perl_is_nan. This was only ever used if Perl_fp_class_finite wasn't defined so maybe that's why it doesn't affect Windows or Linux systems.

If this actually is a typo maybe we can fix this by redefining this given the same conditional as the original code? This fix also solves my issue:

diff --git a/DateTime.xs b/DateTime.xs
index 09778cf..c546d05 100644
--- a/DateTime.xs
+++ b/DateTime.xs
@@ -13,6 +13,10 @@
 #     define dt_isfinite Perl_isfinite
 #   endif
 # else
+#   if !defined(Perl_fp_class_finite)
+#     define Perl_is_inf Perl_isinf
+#     define Perl_is_nan Perl_isnan
+#   endif
 #   define dt_isfinite Perl_isfinite
 # endif

If you think that's reasonable I'll happily make a PR!

[autarch]

I think the simplest thing is just to not worry about this on older macOS builds, similarly to how the code currently handles Windows pre-5.22.0. I'd note that 5.10.1 is now more than 14 years old. I don't want to invest a huge amount of effort in supporting it.

[bombsimon]

I think the simplest thing is just to not worry about this on older macOS builds, similarly to how the code currently handles Windows pre-5.22.0. I'd note that 5.10.1 is now more than 14 years old. I don't want to invest a huge amount of effort in supporting it.

Yeah that sounds reasonable! I just wanted to do the right thing:tm: but I agree it's not worth spending effort on such old versions. Tbh I haven't used such old versions in a long time and the package I mentioned is probably not used much. It was mostly since both my package and this one claims to support this version.

I created a PR with your suggested fix. Closed this, missed your branch without PR.

I can't get my CI to work with older Perls on macOS. I'll do a trial release with the fix and monitor CPAN Testers.

Let me know if there's anything you want me to try for you if you don't have access to macOS!

[autarch]

If you can try the trial release with an older Perl on macOS, I'd greatly appreciate it.

[bombsimon]

Works fine both on my machine and in CI: https://github.com/personnummer/perl/actions/runs/6689612280/job/18173436632

Thank's a bunch!