Skip to content

Commit

Permalink
gnu: check2: Fix tests on i686-linux.
Browse files Browse the repository at this point in the history
Reported upstream to <catchorg/Catch2#2796>.  It is
expected that SSE2 is enabled for i686 builds or tests fail.

* gnu/packages/check.scm (check2)[arguments]: Enable SSE2 for x86_64-linux and
i686-linux in configure-flags.

Co-authored-by: Richard Sent <richard@freakingpenguin.com>
Co-authored-by: Jo Gay <@jane.lx.gay>
Change-Id: I99205f92b66ab3d10affbfb58918f37069ba82ec
  • Loading branch information
podiki and RJSent committed Jan 13, 2024
1 parent 31e736d commit 18393fc
Showing 1 changed file with 9 additions and 1 deletion.
10 changes: 9 additions & 1 deletion gnu/packages/check.scm
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,7 @@
#:use-module (guix build-system python)
#:use-module (guix build-system trivial)
#:use-module (guix deprecation)
#:use-module (ice-9 match)
#:use-module (srfi srfi-1))

(define-public pict
Expand Down Expand Up @@ -621,7 +622,14 @@ pattern.")
(arguments
(list
#:configure-flags
#~(list "-DCATCH_DEVELOPMENT_BUILD=ON"
#~(list #$@(match (%current-system)
((or "x86_64-linux" "i686-linux")
;; Tests fail on i686-linux without SSE2 for floats, see
;; upstream report
;; <https://github.com/catchorg/Catch2/issues/2796>.
'("-DCMAKE_CXX_FLAGS=-msse2 -mfpmath=sse"))
(_ '()))
"-DCATCH_DEVELOPMENT_BUILD=ON"
"-DCATCH_ENABLE_WERROR=OFF"
"-DBUILD_SHARED_LIBS=ON")))
(inputs (list python-wrapper))
Expand Down

10 comments on commit 18393fc

@vcunat
Copy link

@vcunat vcunat commented on 18393fc Jan 25, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wouldn't touch this on x86_64-linux. Just saying; it probably doesn't make any practical difference.

@podiki
Copy link
Contributor Author

@podiki podiki commented on 18393fc Jan 26, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Guix generally doesn't enable processor features. In this case, even if it doesn't make a difference for x86_64, I think it is better to be explicit of where we enable SSE2 (even if it was already used by default there? I'm not sure). In any event, this matches what we do in other places for SSE2. The point here was that it is needed on i686 or else tests fail.

@podiki
Copy link
Contributor Author

@podiki podiki commented on 18393fc Jan 26, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wouldn't touch this on x86_64-linux. Just saying; it probably doesn't make any practical difference.

Also, not sure who this comment was for (since this is a Guix mirror), but figured I'd reply since I got a notification.

@vcunat
Copy link

@vcunat vcunat commented on 18393fc Jan 27, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just felt like pointing it out to whoever is invested in the change, nothing more 🤷🏽

@vcunat
Copy link

@vcunat vcunat commented on 18393fc Jan 27, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe I should've posted explicitly that x86_64-linux (originally called amd64) included mandatory SSE2 since its first definition, and also math defaults to use that.

@podiki
Copy link
Contributor Author

@podiki podiki commented on 18393fc Jan 28, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, hence not a real change or problem on x86_64; it was only i686 that failed the test. As for those "invested in the change" the real point I was making is that this is not GNU Guix but just someone's mirror. So for this, or any future comments, I would direct you there if you want it to be seen :)

@vcunat
Copy link

@vcunat vcunat commented on 18393fc Jan 28, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For future reference, where exactly should I've commented?

@podiki
Copy link
Contributor Author

@podiki podiki commented on 18393fc Jan 28, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah sorry, that would have been helpful! GNU Guix homepage for all the info: https://guix.gnu.org/ In particular, while there are mailing lists for commits and the like, the usual ones:

@vcunat
Copy link

@vcunat vcunat commented on 18393fc Jan 28, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well... I wanted to comment on a commit, pinging people relevant to the commit (like author or merger). Is guix still so small that it's possible to handle a general stream of everything like these names suggest?

@podiki
Copy link
Contributor Author

@podiki podiki commented on 18393fc Jan 28, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I guess so. Some people are more active on one front than another. People might only interact by submitting patches and responding to discussions there, others are on IRC all the time but not mailing lists, etc. Comments on a commit I often see as going to guix-devel cc'ing the relevant people as well. Some people must subscribe to the commits mailing list (since they will respond by cc'ing guix-devel about a commit and quoting the message from the commits list), but I would imagine not many? Personally, I keep an eye on the git log instead.

Anyway, yeah, not sure how to quantify the size compared to other distros. Certainly smaller than Nix, Arch, and so on. But not just a couple of people, as you can see from the git log. Then again, we are very much behind on reviewing patches and the like. Help welcome, it is a good size group (and very friendly) to be able to make contributions and be involved! If you (or anyone else seeing this) had any interest, of course.

Please sign in to comment.