all messages for Guix-related lists mirrored at yhetil.org
 help / color / mirror / code / Atom feed
* [bug#72451] [PATCH core-updates] gnu: gsl: Fix build on i686.
@ 2024-08-03 20:01 Kaelyn Takata via Guix-patches via
  2024-08-18 17:44 ` [bug#72451] fixing the gsl test failures " Kaelyn via Guix-patches via
  0 siblings, 1 reply; 3+ messages in thread
From: Kaelyn Takata via Guix-patches via @ 2024-08-03 20:01 UTC (permalink / raw)
  To: 72451; +Cc: Kaelyn Takata

* 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(-)

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





^ permalink raw reply related	[flat|nested] 3+ messages in thread

* [bug#72451] fixing the gsl test failures on i686
  2024-08-03 20:01 [bug#72451] [PATCH core-updates] gnu: gsl: Fix build on i686 Kaelyn Takata via Guix-patches via
@ 2024-08-18 17:44 ` Kaelyn via Guix-patches via
  2024-08-20 12:42   ` Ludovic Courtès
  0 siblings, 1 reply; 3+ messages in thread
From: Kaelyn via Guix-patches via @ 2024-08-18 17:44 UTC (permalink / raw)
  To: 72451@debbugs.gnu.org, Ludovic Courtès

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




^ permalink raw reply	[flat|nested] 3+ messages in thread

* [bug#72451] fixing the gsl test failures on i686
  2024-08-18 17:44 ` [bug#72451] fixing the gsl test failures " Kaelyn via Guix-patches via
@ 2024-08-20 12:42   ` Ludovic Courtès
  0 siblings, 0 replies; 3+ messages in thread
From: Ludovic Courtès @ 2024-08-20 12:42 UTC (permalink / raw)
  To: Kaelyn; +Cc: 72451@debbugs.gnu.org

Hi Kaelyn,

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

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




^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2024-08-20 12:43 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-08-03 20:01 [bug#72451] [PATCH core-updates] gnu: gsl: Fix build on i686 Kaelyn Takata via Guix-patches via
2024-08-18 17:44 ` [bug#72451] fixing the gsl test failures " Kaelyn via Guix-patches via
2024-08-20 12:42   ` Ludovic Courtès

Code repositories for project(s) associated with this external index

	https://git.savannah.gnu.org/cgit/guix.git

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.