fish: fix aarch64-linux test failures

Parallel locale changes cause flaky tests on fast harder.
2025-03-19 08:37:59 -04:00
parent f88a223845
commit ca8f618ddc
2 changed files with 80 additions and 6 deletions
@@ -0,0 +1,74 @@
+From 1d78c8bd4295262a3118f478e6b3a7c7536fa282 Mon Sep 17 00:00:00 2001
+From: Johannes Altmanninger <aclopte@gmail.com>
+Date: Wed, 19 Mar 2025 09:39:04 +0100
+Subject: [PATCH] Fix concurrent setlocale() in string escape tests
+
+In our C++ implementation, these tests were run serially.  As pointed out in
+https://github.com/fish-shell/fish-shell/issues/11254#issuecomment-2735623229
+we run them in parallel now, which means that one test could be changing
+the global locale used by another.
+
+In theory this could be fine because all tests are setting setting the
+global locale to the same thing but the existence of a lock suggests that
+setlocale() is not guaranteed to be atomic, so it's possible that another
+thread uses a temporarily-invalid locale.
+
+Fixes #11254
+---
+ src/tests/string_escape.rs | 14 ++++++++------
+ 1 file changed, 8 insertions(+), 6 deletions(-)
+
+diff --git a/src/tests/string_escape.rs b/src/tests/string_escape.rs
+index ba8ee7534ebf..4428d679cd35 100644
+--- a/src/tests/string_escape.rs
+++ b/src/tests/string_escape.rs
+@@ -1,3 +1,5 @@
+use std::sync::MutexGuard;
+
+ use crate::common::{
+     escape_string, str2wcstring, unescape_string, wcs2string, EscapeFlags, EscapeStringStyle,
+     UnescapeStringStyle, ENCODE_DIRECT_BASE, ENCODE_DIRECT_END,
+@@ -10,21 +12,21 @@ use rand::{Rng, RngCore};
+ 
+ /// wcs2string is locale-dependent, so ensure we have a multibyte locale
+ /// before using it in a test.
+-fn setlocale() {
+-    let _guard = LOCALE_LOCK.lock().unwrap();
+fn setlocale() -> MutexGuard<'static, ()> {
+    let guard = LOCALE_LOCK.lock().unwrap();
+ 
+     #[rustfmt::skip]
+     const UTF8_LOCALES: &[&str] = &[
+         "C.UTF-8", "en_US.UTF-8", "en_GB.UTF-8", "de_DE.UTF-8", "C.utf8", "UTF-8",
+     ];
+     if crate::libc::MB_CUR_MAX() > 1 {
+-        return;
+        return guard;
+     }
+     for locale in UTF8_LOCALES {
+         let locale = std::ffi::CString::new(locale.to_owned()).unwrap();
+         unsafe { libc::setlocale(libc::LC_CTYPE, locale.as_ptr()) };
+         if crate::libc::MB_CUR_MAX() > 1 {
+-            return;
+            return guard;
+         }
+     }
+     panic!("No UTF-8 locale found");
+@@ -100,7 +102,7 @@ fn test_escape_var() {
+ }
+ 
+ fn escape_test(escape_style: EscapeStringStyle, unescape_style: UnescapeStringStyle) {
+-    setlocale();
+    let _locale_guard = setlocale();
+     let seed: u128 = 92348567983274852905629743984572;
+     let mut rng = get_seeded_rng(seed);
+ 
+@@ -174,7 +176,7 @@ fn str2hex(input: &[u8]) -> String {
+ /// string comes back through double conversion.
+ #[test]
+ fn test_convert() {
+-    setlocale();
+    let _locale_guard = setlocale();
+     let seed = get_rng_seed();
+     let mut rng = get_seeded_rng(seed);
+     let mut origin = Vec::new();
@@ -188,6 +188,10 @@ stdenv.mkDerivation (finalAttrs: {
    # * <https://github.com/LnL7/nix-darwin/issues/122>
    # * <https://github.com/fish-shell/fish-shell/issues/7142>
    ./nix-darwin-path.patch
+
+    # remove 4.0.2
+    # https://github.com/fish-shell/fish-shell/issues/11254
+    ./1d78c8bd4295262a3118f478e6b3a7c7536fa282.patch
  ];

  # Fix FHS paths in tests
@@ -317,16 +321,12 @@ stdenv.mkDerivation (finalAttrs: {
      darwin.system_cmds
    ];

+  # we target the top-level make target which runs all the cmake/ctest
+  # tests, including test_cargo-test
  checkTarget = "fish_run_tests";
  preCheck = ''
    export TERMINFO="${ncurses}/share/terminfo"
  '';
-  checkFlags = lib.optionals (stdenv.hostPlatform.isLinux && stdenv.hostPlatform.isAarch64) [
-    # thread 'tests::string_escape::test_escape_random_url' panicked at src/tests/string_escape.rs:122:9:
-    # assertion `left == right` failed: Escaped and then unescaped string ... but got back a different string ...
-    # https://github.com/fish-shell/fish-shell/issues/11254
-    "--skip=tests::string_escape::test_escape_random_url"
-  ];

  nativeInstallCheckInputs = [
    versionCheckHook