From e4056b9afdf732068a309e9cac959190a640f055 Mon Sep 17 00:00:00 2001
From: John Ericson <John.Ericson@Obsidian.Systems>
Date: Thu, 4 Jul 2024 17:48:27 -0400
Subject: [PATCH] Apply suggestions from code review

Co-authored-by: Robert Hensing <roberth@users.noreply.github.com>
---
 build-utils-meson/deps-lists/meson.build  | 2 +-
 build-utils-meson/diagnostics/meson.build | 3 ---
 src/libexpr-c/meson.build                 | 2 +-
 src/libexpr-c/package.nix                 | 1 -
 src/libstore-c/meson.build                | 2 +-
 src/libstore/meson.build                  | 3 ++-
 tests/unit/libexpr/meson.build            | 3 +--
 7 files changed, 6 insertions(+), 10 deletions(-)

diff --git a/build-utils-meson/deps-lists/meson.build b/build-utils-meson/deps-lists/meson.build
index 89fbfdb36..237eac545 100644
--- a/build-utils-meson/deps-lists/meson.build
+++ b/build-utils-meson/deps-lists/meson.build
@@ -20,7 +20,7 @@ deps_private = [ ]
 # C library, whose public interface --- including public but not private
 # dependencies --- will also likewise soon be stable.
 #
-# N.B. For distributions that care about "ABI" stablity and not just
+# N.B. For distributions that care about "ABI" stability and not just
 # "API" stability, the private dependencies also matter as they can
 # potentially affect the public ABI.
 deps_public = [ ]
diff --git a/build-utils-meson/diagnostics/meson.build b/build-utils-meson/diagnostics/meson.build
index eb0c636c0..2b79f6566 100644
--- a/build-utils-meson/diagnostics/meson.build
+++ b/build-utils-meson/diagnostics/meson.build
@@ -9,8 +9,5 @@ add_project_arguments(
   # Enable assertions in libstdc++ by default. Harmless on libc++. Benchmarked
   # at ~1% overhead in `nix search`.
   #
-  # FIXME: remove when we get meson 1.4.0 which will default this to on for us:
-  # https://mesonbuild.com/Release-notes-for-1-4-0.html#ndebug-setting-now-controls-c-stdlib-assertions
-  '-D_GLIBCXX_ASSERTIONS=1',
   language : 'cpp',
 )
diff --git a/src/libexpr-c/meson.build b/src/libexpr-c/meson.build
index fb9ade28d..2a2669b3e 100644
--- a/src/libexpr-c/meson.build
+++ b/src/libexpr-c/meson.build
@@ -69,7 +69,7 @@ headers = [config_h] + files(
   'nix_api_value.h',
 )
 
-# TODO don't install this once tests don't use it.
+# TODO move this header to libexpr, maybe don't use it in tests?
 headers += files('nix_api_expr_internal.h')
 
 subdir('build-utils-meson/export-all-symbols')
diff --git a/src/libexpr-c/package.nix b/src/libexpr-c/package.nix
index 81e42cf6a..b445a8187 100644
--- a/src/libexpr-c/package.nix
+++ b/src/libexpr-c/package.nix
@@ -1,7 +1,6 @@
 { lib
 , stdenv
 , mkMesonDerivation
-, releaseTools
 
 , meson
 , ninja
diff --git a/src/libstore-c/meson.build b/src/libstore-c/meson.build
index 426f07a34..917de4cda 100644
--- a/src/libstore-c/meson.build
+++ b/src/libstore-c/meson.build
@@ -61,7 +61,7 @@ headers = [config_h] + files(
   'nix_api_store.h',
 )
 
-# TODO don't install this once tests don't use it.
+# TODO don't install this once tests don't use it and/or move the header into `libstore`, non-`c`
 headers += files('nix_api_store_internal.h')
 
 subdir('build-utils-meson/export-all-symbols')
diff --git a/src/libstore/meson.build b/src/libstore/meson.build
index f94a454da..7444cba20 100644
--- a/src/libstore/meson.build
+++ b/src/libstore/meson.build
@@ -118,7 +118,8 @@ busybox = find_program(get_option('sandbox-shell'), required : false)
 if get_option('embedded-sandbox-shell')
   # This one goes in config.h
   # The path to busybox is passed as a -D flag when compiling this_library.
-  # Idk why, ask the old buildsystem.
+  # This solution is inherited from the old make buildsystem
+  # TODO: do this differently?
   configdata.set('HAVE_EMBEDDED_SANDBOX_SHELL', 1)
   hexdump = find_program('hexdump', native : true)
   embedded_sandbox_shell_gen = custom_target(
diff --git a/tests/unit/libexpr/meson.build b/tests/unit/libexpr/meson.build
index 71865b59f..a7b22f7f1 100644
--- a/tests/unit/libexpr/meson.build
+++ b/tests/unit/libexpr/meson.build
@@ -30,7 +30,7 @@ subdir('build-utils-meson/export-all-symbols')
 rapidcheck = dependency('rapidcheck')
 deps_private += rapidcheck
 
-gtest = dependency('gtest', main : true)
+gtest = dependency('gtest')
 deps_private += gtest
 
 gtest = dependency('gmock')
@@ -77,7 +77,6 @@ this_exe = executable(
   include_directories : include_dirs,
   # TODO: -lrapidcheck, see ../libutil-support/build.meson
   link_args: linker_export_flags + ['-lrapidcheck'],
-  # get main from gtest
   install : true,
 )