[PATCH core-updates] gnu: gsl: Fix build on i686.

  • Done
  • quality assurance status badge
Details
2 participants
  • Kaelyn Takata
  • Ludovic Courtès
Owner
unassigned
Submitted by
Kaelyn Takata
Severity
normal
K
K
Kaelyn Takata wrote on 3 Aug 22:01 +0200
(address . guix-patches@gnu.org)(name . Kaelyn Takata)(address . kaelyn.alexi@protonmail.com)
71397042b223b98e0937b56018749c89eeb638e2.1722715149.git.kaelyn.alexi@protonmail.com
* gnu/packages/maths.scm (gsl)[arguments]<#:make-flags>: Include the default
CFLAGS, restoring the optimized build and fixing test failures on i686.

Change-Id: I64a43368b000995e03810b33de131aba4203d02b
---
gnu/packages/maths.scm | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

Toggle diff (15 lines)
diff --git a/gnu/packages/maths.scm b/gnu/packages/maths.scm
index c06eddcc34..984aa35066 100644
--- a/gnu/packages/maths.scm
+++ b/gnu/packages/maths.scm
@@ -729,7 +729,7 @@ (define-public gsl
(build-system gnu-build-system)
(arguments
(let ((system (%current-system)))
- `(#:make-flags (list "CFLAGS=-fPIC")
+ `(#:make-flags (list "CFLAGS=-g -O2 -fPIC")
#:phases
(modify-phases %standard-phases
,@(cond

base-commit: baad95b19a24401cad8bee7290e5dbf3b3f38287
--
2.45.2
K
K
Kaelyn wrote on 18 Aug 19:44 +0200
Re: fixing the gsl test failures on i686
AjI8cbW3PjJvv3wF1tQnJTlG446yYgI0AL-wMrkXuYRh9RnQTLY2fRnzW4KqcznqmH3_rYh9hwfwa26O9jOrHVYo6de9ZXVEQbxaPxPRPxs=@protonmail.com
Hi Ludo,

I noticed this morning that you recently pushed commit fa8dbbe59d to fix the i686-linux gsl test failures. I'd submitted a similar fix about two weeks ago (along with a couple of other i686-linux fixes that affect building wine64), and I wanted to mention that I don't think your commit does the right thing with CFLAGS. When I figured out the error, I noticed a side-effect of passing CFLAGS=-fPIC as the #:make-flags is that it disables the optimized build (or at least relies on whatever optimizations the compiler defaults to, which AFAIK is little to none). While your patch also cleans up the definition a bit by no longer disabling some tests, I think the correct approach is to include the package's default CFLAGS as part of the #:make-flags as I did in issue 72451 so that optimizations aren't unintentionally disabled.

Cheers,
Kaelyn
L
L
Ludovic Courtès wrote on 20 Aug 14:42 +0200
(name . Kaelyn)(address . kaelyn.alexi@protonmail.com)(name . 72451@debbugs.gnu.org)(address . 72451@debbugs.gnu.org)
877ccbtlf3.fsf@gnu.org
Hi Kaelyn,

Kaelyn <kaelyn.alexi@protonmail.com> skribis:

Toggle quote (2 lines)
> I noticed this morning that you recently pushed commit fa8dbbe59d to fix the i686-linux gsl test failures. I'd submitted a similar fix about two weeks ago (along with a couple of other i686-linux fixes that affect building wine64), and I wanted to mention that I don't think your commit does the right thing with CFLAGS. When I figured out the error, I noticed a side-effect of passing CFLAGS=-fPIC as the #:make-flags is that it disables the optimized build (or at least relies on whatever optimizations the compiler defaults to, which AFAIK is little to none). While your patch also cleans up the definition a bit by no longer disabling some tests, I think the correct approach is to include the package's default CFLAGS as part of the #:make-flags as I did in issue 72451 so that optimizations aren't unintentionally disabled.

Oh, apologies for not noticing https://issues.guix.gnu.org/72451.

What you did looks correct to me. I added a FIXME about ‘CFLAGS=-fPIC’
stating roughly what you explained here.

The reason I did not change CFLAGS on other architectures was to avoid a
massive rebuild at this late stage of the branch (‘gsl’ has 2.9K
dependents).

But I agree: we should clean that up for all platforms, and check which
of the tests really need to be skipped on 32-bit platforms once compiled
with optimizations. We can create a branch with such a patch once
‘core-updates’ is merged.

WDYT?

Thanks,
Ludo’.
L
L
Ludovic Courtès wrote on 28 Aug 23:18 +0200
control message for bug #72451
(address . control@debbugs.gnu.org)
87y14ggxch.fsf@gnu.org
close 72451
quit
?
Your comment

This issue is archived.

To comment on this conversation send an email to 72451@debbugs.gnu.org

To respond to this issue using the mumi CLI, first switch to it
mumi current 72451
Then, you may apply the latest patchset in this issue (with sign off)
mumi am -- -s
Or, compose a reply to this issue
mumi compose
Or, send patches to this issue
mumi send-email *.patch