hakonhagland / perl-math-gsl

Building Math::GSL with GitHub Actions
1 stars 2 forks source link

swig/Multiroots.i: fix build failure with gcc 14. #36

Closed emollier closed 1 month ago

emollier commented 1 month ago

As identified in Debian bug #1075182, Math::GSL fails to build with gcc 14 with the following kind of symptoms due to a number of warnings in the C compiler now gone fatal by default:

xs/Multiroots_wrap.2.8.c:2950:12:
error: assignment to ‘gsl_multiroot_function *’
       {aka ‘struct gsl_multiroot_function_struct *’}
       from incompatible pointer type ‘gsl_multiroot_function **’
       {aka ‘struct gsl_multiroot_function_struct **’}
       [-Wincompatible-pointer-types]
 2950 |       arg1 = &f;
      |            ^

This change fixes/workarounds the issue by casting the &f pointer to the expected type. Other approaches trying to fix the native type of f straight away result in a segmentation faults while running tests, and any other approaches were anything but obvious due to entanglement with swig, but maybe there is something more appropriate than casting.

hakonhagland commented 1 month ago

Thanks for the fix! Would you be able to add a github action test case for gcc 13/14?

emollier commented 1 month ago

Hi Håkon,

I'm afraid not. I have vague experience with gitlab CI, but I never worked with github actions. But thanks for asking, I see how useful it is.

Have a nice day, :) Étienne.

gregoa commented 1 month ago

I also have no idea about github actions; while browsing around a bit I noticed that https://github.com/hakonhagland/perl-math-gsl/blob/master/.github/workflows/actions.yml uses macos-latestand that for the macos images gcc-14 seems to be available: GCC 14 (Homebrew GCC 14.1.0_2) - available by gcc-14 alias. Now I have no idea what a gcc-14 alias means in practical terms :/ but maybe this shows a possible direction.

Cheers, gregor (also member of the Debian Perl Group)

hakonhagland commented 1 month ago

Ok no problem :smile: I will take your word for it that it fixes the issue for gcc 14 for now. I will try to add some GitHub action test cases later.

gregoa commented 1 month ago

If it helps, I can confirm that @emollier's patch fixes the gcc-14 related build failures on Debian/unstable :)

hakonhagland commented 1 month ago

@gregoa Thanks! New release with the fix published here: https://github.com/hakonhagland/perl-math-gsl/releases/tag/v0.48

gregoa commented 1 month ago

Thank you, much appreciated!

mohawk2 commented 1 month ago

Just FYI (and this may be because I don't understand something): I see @hakonhagland refers there to 0.48, while Debian just showed a new upload of 0.45. I also note there's no tag for 0.45, but there is for 0.46-0.48, while those aren't showing on MetaCPAN.

hakonhagland commented 1 month ago

@mohawk2 Thanks, I thought I would wait uploading to CPAN until it builds on Windows, see #35.

mohawk2 commented 1 month ago

@hakonhagland Fair enough. You're aware that MetaCPAN doesn't know about 0.46 and 0.47?

gregoa commented 1 month ago

The upload to Debian was upstream's 0.45 plus the patch from this pull request.

hakonhagland commented 1 month ago

You're aware that MetaCPAN doesn't know about 0.46 and 0.47?

@mohawk2 Yes, do you see any issues with that?

mohawk2 commented 1 month ago

You're aware that MetaCPAN doesn't know about 0.46 and 0.47?

@mohawk2 Yes, do you see any issues with that?

That would depend on whether you intended to have version 0.46 etc uploaded to CPAN. If you didn't, then I guess that's OK but I don't see why that would be something you'd bother to tag (and version bump for). If you did, then it failed to become visible either due to a PAUSE/CPAN problem, or it didn't successfully get uploaded. Either way, that's not how I do my distributions ;-)

mohawk2 commented 1 month ago

@mohawk2 Thanks, I thought I would wait uploading to CPAN until it builds on Windows, see #35.

By the way, could this be an instance of letting the perfect stand in the way of a material improvement?

hakonhagland commented 1 month ago

@mohawk2 :smile: Actually, I think I was concerned about the CPAN testers having to retest the distro for not so substantial changes. But I am not familiar with the workflow of the testers, they might have no problem with retesting the module for what I know

mohawk2 commented 1 month ago

@mohawk2 😄 Actually, I think I was concerned about the CPAN testers having to retest the distro for not so substantial changes. But I am not familiar with the workflow of the testers, they might have no problem with retesting the module for what I know

As far as I know, legitimate releases even with only small changes are fine with them. I think there is a line, but it is probably with distros that are using CPAN as quasi-version control, like Net::FullAuto. This distro is nothing remotely like that.