Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 1 commit into from
Jul 25, 2024

Conversation

emollier
Copy link
Contributor

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.

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.

[Debian bug #1075182]: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1075182

Signed-off-by: Étienne Mollier <emollier@debian.org>
@hakonhagland
Copy link
Owner

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

@emollier
Copy link
Contributor Author

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
Copy link

gregoa commented Jul 25, 2024

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
Copy link
Owner

Ok no problem 😄 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.

@hakonhagland hakonhagland merged commit 8be5b93 into hakonhagland:master Jul 25, 2024
50 checks passed
@gregoa
Copy link

gregoa commented Jul 25, 2024

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

@hakonhagland
Copy link
Owner

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

@gregoa
Copy link

gregoa commented Jul 25, 2024

Thank you, much appreciated!

@mohawk2
Copy link
Contributor

mohawk2 commented Jul 25, 2024

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
Copy link
Owner

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

@mohawk2
Copy link
Contributor

mohawk2 commented Jul 25, 2024

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

@gregoa
Copy link

gregoa commented Jul 25, 2024

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

@hakonhagland
Copy link
Owner

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

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

@mohawk2
Copy link
Contributor

mohawk2 commented Jul 26, 2024

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
Copy link
Contributor

mohawk2 commented Jul 26, 2024

@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
Copy link
Owner

@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

@mohawk2
Copy link
Contributor

mohawk2 commented Jul 27, 2024

@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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants