yaml-cpp: 0.8.0 -> 0.9.0

[whispersofthedawn]

changelog: https://github.com/jbeder/yaml-cpp/releases/tag/yaml-cpp-0.9.0
diff: jbeder/yaml-cpp@0.8.0...yaml-cpp-0.9.0

Things done

• Built on platform:
  • x86_64-linux
  • aarch64-linux
  • aarch64-darwin
• Tested, as applicable:
  • NixOS tests in nixos/tests.
  • Package tests at passthru.tests.
  • Tests in lib/tests or pkgs/test for functions and "core" functionality.
• Ran nixpkgs-review on this PR. See nixpkgs-review usage.
• Tested basic functionality of all binary files, usually in ./result/bin/.
• Nixpkgs Release Notes
  • Package update: when the change is major or breaking.
• NixOS Release Notes
  • Module addition: when adding a new NixOS module.
  • Module update: when the change is significant.
• Fits CONTRIBUTING.md, pkgs/README.md, maintainers/README.md and other READMEs.
• Follows the automation/AI policy.

[OPNA2608]

(trying my best to nixpkgs-review rn, but this gets pulled into too much stuff with big closures, so I'll run out of disk space soon.
Edit: nope, too large. switching to spot-testing…)

starship-sf64:

nixpkgs/pkgs/by-name/st/starship-sf64/package.nix

Lines 99 to 110 in c043004

	# Include cmake4 patch
	# Remove when yaml-cpp.src is updated to include it
	yaml-patched = applyPatches {
	src = yaml-cpp.src;
	patches = [
	(fetchpatch {
	name = "yaml-cpp-fix-cmake-4.patch";
	url = "https://github.com/jbeder/yaml-cpp/commit/c2680200486572baf8221ba052ef50b58ecd816e.patch";
	hash = "sha256-1kXRa+xrAbLEhcJxNV1oGHPmayj1RNIe6dDWXZA3mUA=";
	})
	];
	};

This can be dropped.

nixpkgs/pkgs/by-name/st/starship-sf64/package.nix

Line 143 in c043004

yaml-cpp_src = srcOnly yaml-patched;

And this changed to yaml-cpp_src = srcOnly yaml-cpp;

[OPNA2608]

It seems like this bump causes most of the test_cpu* tests in opencolorio to segfault on my end (x86_64-linux). Can you confirm this?

When running the test_cpu_exec binary manually:

[ 129/1169] [Config / inactive_color_space_precedence                    ] - PASSED
/build/source/tests/cpu/Config_tests.cpp:6502:
FAILED: exception thrown from config = OCIO::Config::CreateFromStream(is): "Error: Loading the OCIO profile failed. yaml-cpp: error at line 25, column 1: plain value cannot start with flow indicator character"
Segmentation fault         (core dumped) ./test_cpu_exec

Edit: Seems like one of their tests has intentionally messy YAML that turns out to not actually be valid, so yaml-cpp throws an error on that now.

diff --git a/pkgs/by-name/op/opencolorio/package.nix b/pkgs/by-name/op/opencolorio/package.nix
index a18f3a0f3843..aeb6c04294eb 100644
--- a/pkgs/by-name/op/opencolorio/package.nix
+++ b/pkgs/by-name/op/opencolorio/package.nix
@@ -32,7 +32,14 @@ stdenv.mkDerivation rec {
     hash = "sha256-b4tdQ9VH9M7hAD5Uuxu4QKwwpaVwroj/Bvg+Zsy0M1M=";
   };
 
-  postPatch = lib.optionalString stdenv.hostPlatform.isDarwin ''
+  # This is *not* how you express a multi-line array in YAML...
+  postPatch = ''
+    substituteInPlace tests/cpu/Config_tests.cpp \
+      --replace-fail \
+        'inactive_colorspaces: [cs1\t\n   \n,   \ncs2]\n' \
+        'inactive_colorspaces:\t\n   \n   - \t   cs1\t\n\n   \n   -\tcs2\t  \t\n'
+  ''
+  + lib.optionalString stdenv.hostPlatform.isDarwin ''
     # these tests don't like being run headless on darwin. no builtin
     # way of skipping tests so this is what we're reduced to.
     substituteInPlace tests/cpu/Config_tests.cpp \

This is the messiest that I could get working. This'll need a report on the opencolorio repo…

[OPNA2608]

Seems like tests also stopped being built in 0.8.0 because their cmake_dependent_option(YAML_CPP_BUILD_TESTS ...) requires BUILD_TESTING to be explicitly passed in as true, while we default to only passing -DBUILD_TESTING:BOOL=OFF when !doCheck. Could you also add (lib.cmakeBool "YAML_CPP_BUILD_TESTS" finalAttrs.finalPackage.doCheck) to cmakeFlags?

---

Other than the above two issues, my spot testing of various packages I maintain / that looked important indicates this might be good.

nixpkgs-review result

Generated using nixpkgs-review.

Command: nixpkgs-review pr 560590 --package mir --package calamares --package lomiri.lomiri-session --package lomiri.lomiri-system-settings-unwrapped --package mir_2_15 --package lomiri.hfd-service --package yaml-cpp --package miriway --package lomiri-qt6.deviceinfo --package openmw --package lomiri.qtmir --package qtcreator --package mongodb --package lomiri.lomiri-system-settings --package blender --package lomiri.deviceinfo --package krita --package lomiri.lomiri --package freecad --package starship-sf64 --package miracle-wm --package vtk
Commit: da2fd809c1aaf04040a1a0c87e98ef32687f74b7

---

x86_64-linux

❌ 2 packages failed to build:

• blender
• krita

✅ 20 packages built:

• calamares
• freecad
• lomiri-qt6.deviceinfo
• lomiri.deviceinfo
• lomiri.hfd-service
• lomiri.lomiri
• lomiri.lomiri-session
• lomiri.lomiri-system-settings
• lomiri.lomiri-system-settings-unwrapped
• lomiri.qtmir
• mir
• mir_2_15
• miracle-wm
• miriway
• mongodb
• openmw
• qtcreator
• starship-sf64
• vtk
• yaml-cpp

blender and krita both depend on opencolorio. Ran .passthru.tests for all successfully built packages.

[whispersofthedawn]

thank you for the reviews and extensive testing :) i'll admit i didn't anticipate that this would cause much breakage, and so i didn't test as much myself beforehand. sorry about that.

Seems like tests also stopped being built in 0.8.0 because their cmake_dependent_option(YAML_CPP_BUILD_TESTS ...) requires BUILD_TESTING to be explicitly passed in as true, while we default to only passing -DBUILD_TESTING:BOOL=OFF when !doCheck. Could you also add (lib.cmakeBool "YAML_CPP_BUILD_TESTS" finalAttrs.finalPackage.doCheck) to cmakeFlags?

done, tests seem to be working. fwiw i did try to enable YAML_USE_SYSTEM_GTEST and use our gtest from nixpkgs, but it seems like gtest requires C++17 and yaml-cpp pins C++11 if it's not explicitly defined. it's a short part of the build, so this is probably not a problem.

Edit: Seems like one of their tests has intentionally messy YAML that turns out to not actually be valid, so yaml-cpp throws an error on that now.
...
This is the messiest that I could get working. This'll need a report on the opencolorio repo…

i've added your fix to opencolorio, seems to build fine now. blender and krita are a bit too expensive for me to build on my machine, unfortunately. would you like to open an issue/pr with opencolorio to link to this, or would you prefer i did that? up to you, just wanted to give you the chance since you found the issue.

[OPNA2608]

Please reorder the commits to avoid an inbetween state where things are broken (i.e. when bisecting): yaml-cpp 0.9.0 fixes to other packages first, then the bump to 0.9.0.

starship-sf64's srcOnly yaml-cpp is actually fine with version 0.8.0 - the CMake compatibility fix is in its patches, and srcOnly explicitly differs from pkg.src by applying all patches.
As for opencolorio, the fix just makes their YAML more valid, so I don't expect any issues when used with 0.8.0.

---

Tried to submit a fix to opencolorio, but they want a CLA signature, and I do not feel comfortable with giving them my full legal address for a very simple YAML fix… Opened issue AcademySoftwareFoundation/OpenColorIO#2340 instead.

Even smaller fix for the issue, after some more messing around during daytime hours: yaml-cpp just wants the continued array elements to be indented properly.

diff --git a/pkgs/by-name/op/opencolorio/package.nix b/pkgs/by-name/op/opencolorio/package.nix
index 2bec1449e10f..2d745ca4cdba 100644
--- a/pkgs/by-name/op/opencolorio/package.nix
+++ b/pkgs/by-name/op/opencolorio/package.nix
@@ -34,10 +34,11 @@ stdenv.mkDerivation rec {
 
   # This test produces an invalid multi-line array in YAML, which is rejected
   # by yaml-cpp 0.9.0.
+  # https://github.com/AcademySoftwareFoundation/OpenColorIO/issues/2340
   postPatch = ''
     substituteInPlace tests/cpu/Config_tests.cpp --replace-fail \
       'inactive_colorspaces: [cs1\t\n   \n,   \ncs2]\n' \
-      'inactive_colorspaces:\t\n   \n   - \t   cs1\t\n\n   \n   -\tcs2\t  \t\n'
+      'inactive_colorspaces: [cs1\t\n   \n  ,   \n  cs2]\n'
   ''
   + lib.optionalString stdenv.hostPlatform.isDarwin ''
     # these tests don't like being run headless on darwin. no builtin

[OPNA2608]

Thanks! nixpkgs-reviewing again…

[qubitnano]

spaghettikart also patches yaml like starship-sf64

[OPNA2608]

Spot-testing:

---

nixpkgs-review result

Generated using nixpkgs-review.

Command: nixpkgs-review pr 560590 --package mongodb --package mir --package lomiri.qtmir --package lomiri.hfd-service --package lomiri-qt6.deviceinfo --package miriway --package calamares --package lomiri.lomiri-session --package qtcreator --package yaml-cpp --package openmw --package lomiri.deviceinfo --package mir_2_15 --package krita --package starship-sf64 --package miracle-wm --package lomiri.lomiri-system-settings --package freecad --package blender --package vtk --package lomiri.lomiri --package lomiri.lomiri-system-settings-unwrapped
Commit: fb38a17144532711c700752d3a9cbb8bfe66a979

---

x86_64-linux

✅ 22 packages built:

• blender
• calamares
• freecad
• krita
• lomiri-qt6.deviceinfo
• lomiri.deviceinfo
• lomiri.hfd-service
• lomiri.lomiri
• lomiri.lomiri-session
• lomiri.lomiri-system-settings
• lomiri.lomiri-system-settings-unwrapped
• lomiri.qtmir
• mir
• mir_2_15
• miracle-wm
• miriway
• mongodb
• openmw
• qtcreator
• starship-sf64
• vtk
• yaml-cpp

Also successfully built all of the above packages' .passthru.tests. As far as my storage permits me to test things, this seems functionally fine now.

[OPNA2608]

LGTM, hope this doesn't break anything that I was unable to test. (I also checked spaghettikart between the nixpkgs-review and this.)

Many thanks! 💚

(letting this sit for a week before hitting merge, in case anyone else sees/notices anything that still needs fixing)

[whispersofthedawn]

thank you so much for the thorough reviews and testing!

[OPNA2608]

You're welcome! Thanks for putting up with all the back-and-forth.

Hoping nothing that I wasn't able to nixpkgs-review will break… 🤞