From 3294b22a6845f08daf095ed425f16877da8ab040 Mon Sep 17 00:00:00 2001 From: John Ericson Date: Sun, 6 Apr 2025 17:17:54 -0400 Subject: [PATCH 1/2] Clean some header related things. Revert most of "Hack together a fix for the public headers" - The `libmain` change is kept, and one more libmain change is made. (Need to update Meson and Nix per the package alike). - The S3 situation is fixed in a different way: the variable is public now, used in the header, and fixed accordingly. - Fix TODO for `HAVE_EMBEDDED_SANDBOX_SHELL` This reverts commit 2b51250534899329906273ae80463ccfe8455d08. --- src/libexpr/expr-config.hh | 3 --- src/libexpr/include/nix/expr/config.hh | 1 - src/libexpr/include/nix/expr/meson.build | 1 - src/libexpr/meson.build | 11 ---------- src/libmain/meson.build | 6 +++-- src/libstore-tests/meson.build | 3 --- src/libstore-tests/s3-binary-cache-store.cc | 7 +++--- src/libstore/filetransfer.cc | 6 ++--- .../nix/store/s3-binary-cache-store.hh | 10 +++++++-- src/libstore/include/nix/store/s3.hh | 2 +- src/libstore/meson.build | 22 ++++++++----------- src/libstore/s3-binary-cache-store.cc | 6 ++--- .../unix/build/local-derivation-goal.cc | 2 +- 13 files changed, 32 insertions(+), 48 deletions(-) delete mode 100644 src/libexpr/expr-config.hh delete mode 120000 src/libexpr/include/nix/expr/config.hh diff --git a/src/libexpr/expr-config.hh b/src/libexpr/expr-config.hh deleted file mode 100644 index e28b461c0..000000000 --- a/src/libexpr/expr-config.hh +++ /dev/null @@ -1,3 +0,0 @@ -// TODO: Remove this damn file while keeping public config headers working -#error \ - "This file is a placeholder. It only exists so that meson accepts the symbolic link include/nix/expr/config.hh to this file, but we expect meson to overwrite it with the real file. Apparently that did not happen. I deeply apologize for this mess." diff --git a/src/libexpr/include/nix/expr/config.hh b/src/libexpr/include/nix/expr/config.hh deleted file mode 120000 index 45d3ca29d..000000000 --- a/src/libexpr/include/nix/expr/config.hh +++ /dev/null @@ -1 +0,0 @@ -../../../expr-config.hh \ No newline at end of file diff --git a/src/libexpr/include/nix/expr/meson.build b/src/libexpr/include/nix/expr/meson.build index 3eb80de68..01275e52e 100644 --- a/src/libexpr/include/nix/expr/meson.build +++ b/src/libexpr/include/nix/expr/meson.build @@ -10,7 +10,6 @@ config_pub_h = configure_file( headers = [config_pub_h] + files( 'attr-path.hh', 'attr-set.hh', - 'config.hh', 'eval-cache.hh', 'eval-error.hh', 'eval-gc.hh', diff --git a/src/libexpr/meson.build b/src/libexpr/meson.build index 402bca0e1..2e773938d 100644 --- a/src/libexpr/meson.build +++ b/src/libexpr/meson.build @@ -79,11 +79,6 @@ config_priv_h = configure_file( output : 'expr-config-private.hh', ) -config_pub_h = configure_file( - configuration : configdata_pub, - output : 'expr-config.hh', -) - subdir('nix-meson-build-support/common') parser_tab = custom_target( @@ -168,8 +163,6 @@ subdir('primops') subdir('nix-meson-build-support/export-all-symbols') subdir('nix-meson-build-support/windows-version') -headers += [config_pub_h] - this_library = library( 'nixexpr', sources, @@ -188,8 +181,4 @@ install_headers(headers, subdir : 'nix/expr', preserve_path : true) libraries_private = [] -nixexpr_dep = declare_dependency( - include_directories : include_directories('.'), - link_with : this_library, -) subdir('nix-meson-build-support/export') diff --git a/src/libmain/meson.build b/src/libmain/meson.build index 4f78d265b..65fcb6239 100644 --- a/src/libmain/meson.build +++ b/src/libmain/meson.build @@ -17,12 +17,14 @@ subdir('nix-meson-build-support/deps-lists') configdata = configuration_data() deps_private_maybe_subproject = [ - # This dependency may be very limited; was introduced for NIX_USE_BOEHMGC macro dependency - dependency('nix-expr'), ] deps_public_maybe_subproject = [ dependency('nix-util'), dependency('nix-store'), + # FIXME: This is only here for the NIX_USE_BOEHMGC macro dependency + # Removing nix-expr will make the build more concurrent and is + # architecturally nice, perhaps. + dependency('nix-expr'), ] subdir('nix-meson-build-support/subprojects') diff --git a/src/libstore-tests/meson.build b/src/libstore-tests/meson.build index eb3d14530..1822a3520 100644 --- a/src/libstore-tests/meson.build +++ b/src/libstore-tests/meson.build @@ -40,9 +40,6 @@ deps_private += gtest configdata = configuration_data() configdata.set_quoted('PACKAGE_VERSION', meson.project_version()) -aws_s3 = dependency('aws-cpp-sdk-s3', required : false) -configdata.set('ENABLE_S3', aws_s3.found().to_int()) - config_priv_h = configure_file( configuration : configdata, output : 'store-tests-config.hh', diff --git a/src/libstore-tests/s3-binary-cache-store.cc b/src/libstore-tests/s3-binary-cache-store.cc index dbb414f2b..251e96172 100644 --- a/src/libstore-tests/s3-binary-cache-store.cc +++ b/src/libstore-tests/s3-binary-cache-store.cc @@ -1,10 +1,9 @@ -#include "store-tests-config.hh" -#if ENABLE_S3 +#include "nix/store/s3-binary-cache-store.hh" + +#if NIX_WITH_S3_SUPPORT # include -# include "nix/store/s3-binary-cache-store.hh" - namespace nix { TEST(S3BinaryCacheStore, constructConfig) diff --git a/src/libstore/filetransfer.cc b/src/libstore/filetransfer.cc index 6de458823..475637d74 100644 --- a/src/libstore/filetransfer.cc +++ b/src/libstore/filetransfer.cc @@ -9,7 +9,7 @@ #include "nix/util/signals.hh" #include "store-config-private.hh" -#if ENABLE_S3 +#if NIX_WITH_S3_SUPPORT #include #endif @@ -756,7 +756,7 @@ struct curlFileTransfer : public FileTransfer #endif } -#if ENABLE_S3 +#if NIX_WITH_S3_SUPPORT std::tuple parseS3Uri(std::string uri) { auto [path, params] = splitUriAndParams(uri); @@ -779,7 +779,7 @@ struct curlFileTransfer : public FileTransfer if (hasPrefix(request.uri, "s3://")) { // FIXME: do this on a worker thread try { -#if ENABLE_S3 +#if NIX_WITH_S3_SUPPORT auto [bucketName, key, params] = parseS3Uri(request.uri); std::string profile = getOr(params, "profile", ""); diff --git a/src/libstore/include/nix/store/s3-binary-cache-store.hh b/src/libstore/include/nix/store/s3-binary-cache-store.hh index eec2dc6ee..7bc04aa4a 100644 --- a/src/libstore/include/nix/store/s3-binary-cache-store.hh +++ b/src/libstore/include/nix/store/s3-binary-cache-store.hh @@ -1,9 +1,13 @@ #pragma once ///@file -#include "nix/store/binary-cache-store.hh" +#include "nix/store/config.hh" -#include +#if NIX_WITH_S3_SUPPORT + +# include "nix/store/binary-cache-store.hh" + +# include namespace nix { @@ -125,3 +129,5 @@ public: }; } + +#endif diff --git a/src/libstore/include/nix/store/s3.hh b/src/libstore/include/nix/store/s3.hh index 5ac5b9a9f..9c159ba0f 100644 --- a/src/libstore/include/nix/store/s3.hh +++ b/src/libstore/include/nix/store/s3.hh @@ -1,7 +1,7 @@ #pragma once ///@file #include "store-config-private.hh" -#if ENABLE_S3 +#if NIX_WITH_S3_SUPPORT #include "nix/util/ref.hh" diff --git a/src/libstore/meson.build b/src/libstore/meson.build index e9f817482..59c62fe04 100644 --- a/src/libstore/meson.build +++ b/src/libstore/meson.build @@ -126,7 +126,8 @@ deps_private += sqlite # AWS C++ SDK has bad pkg-config. See # https://github.com/aws/aws-sdk-cpp/issues/2673 for details. aws_s3 = dependency('aws-cpp-sdk-s3', required : false) -configdata_priv.set('ENABLE_S3', aws_s3.found().to_int()) +# The S3 store definitions in the header will be hidden based on this variables. +configdata_pub.set('NIX_WITH_S3_SUPPORT', aws_s3.found().to_int()) if aws_s3.found() aws_s3 = declare_dependency( include_directories: include_directories(aws_s3.get_variable('includedir')), @@ -153,13 +154,13 @@ endforeach busybox = find_program(get_option('sandbox-shell'), required : false) +# This one goes in config.h +# The path to busybox is passed as a -D flag when compiling this_library. +# This solution is inherited from the old make buildsystem +# TODO: do this differently? +configdata_priv.set('HAVE_EMBEDDED_SANDBOX_SHELL', get_option('embedded-sandbox-shell').to_int()) + 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. - # This solution is inherited from the old make buildsystem - # TODO: do this differently? - # TODO: at least define it unconditionally, so we get checking from -Wundef - configdata_priv.set('HAVE_EMBEDDED_SANDBOX_SHELL', 1) hexdump = find_program('hexdump', native : true) embedded_sandbox_shell_gen = custom_target( 'embedded-sandbox-shell.gen.hh', @@ -182,11 +183,6 @@ config_priv_h = configure_file( output : 'store-config-private.hh', ) -config_pub_h = configure_file( - configuration : configdata_pub, - output : 'store-config.hh', -) - subdir('nix-meson-build-support/common') sources = files( @@ -369,7 +365,7 @@ this_library = library( install : true, ) -install_headers(headers + [ config_pub_h ], subdir : 'nix/store', preserve_path : true) +install_headers(headers, subdir : 'nix/store', preserve_path : true) libraries_private = [] diff --git a/src/libstore/s3-binary-cache-store.cc b/src/libstore/s3-binary-cache-store.cc index 4e51e728a..87f5feb45 100644 --- a/src/libstore/s3-binary-cache-store.cc +++ b/src/libstore/s3-binary-cache-store.cc @@ -1,10 +1,10 @@ -#include "store-config-private.hh" -#if ENABLE_S3 +#include "nix/store/s3-binary-cache-store.hh" + +#if NIX_WITH_S3_SUPPORT #include #include "nix/store/s3.hh" -#include "nix/store/s3-binary-cache-store.hh" #include "nix/store/nar-info.hh" #include "nix/store/nar-info-disk-cache.hh" #include "nix/store/globals.hh" diff --git a/src/libstore/unix/build/local-derivation-goal.cc b/src/libstore/unix/build/local-derivation-goal.cc index 185c98bb6..892513ea8 100644 --- a/src/libstore/unix/build/local-derivation-goal.cc +++ b/src/libstore/unix/build/local-derivation-goal.cc @@ -2017,7 +2017,7 @@ void LocalDerivationGoal::runChild() for (auto & i : pathsInChroot) { if (i.second.source == "/proc") continue; // backwards compatibility - #ifdef HAVE_EMBEDDED_SANDBOX_SHELL + #if HAVE_EMBEDDED_SANDBOX_SHELL if (i.second.source == "__embedded_sandbox_shell__") { static unsigned char sh[] = { #include "embedded-sandbox-shell.gen.hh" From 7a7fe350d55803e3ff73bc0645b0c498b0a0eff9 Mon Sep 17 00:00:00 2001 From: John Ericson Date: Sun, 6 Apr 2025 17:57:43 -0400 Subject: [PATCH 2/2] Get rid of raw `-D` defines, always use private config files Now that we have the private vs public distinction, we can do this without leaking information downstream. --- src/libstore-tests/meson.build | 3 +- src/libstore/meson.build | 154 ++++++++++++++------------------- src/nix/man-pages.cc | 1 + src/nix/meson.build | 20 ++--- 4 files changed, 75 insertions(+), 103 deletions(-) diff --git a/src/libstore-tests/meson.build b/src/libstore-tests/meson.build index 1822a3520..8a1ff40f0 100644 --- a/src/libstore-tests/meson.build +++ b/src/libstore-tests/meson.build @@ -40,6 +40,8 @@ deps_private += gtest configdata = configuration_data() configdata.set_quoted('PACKAGE_VERSION', meson.project_version()) +configdata.set_quoted('NIX_STORE_DIR', nix_store.get_variable('storedir')) + config_priv_h = configure_file( configuration : configdata, output : 'store-tests-config.hh', @@ -89,7 +91,6 @@ this_exe = executable( include_directories : include_dirs, # TODO: -lrapidcheck, see ../libutil-support/build.meson link_args: linker_export_flags + ['-lrapidcheck'], - cpp_args : [ '-DNIX_STORE_DIR="' + nix_store.get_variable('storedir') + '"' ], # get main from gtest install : true, ) diff --git a/src/libstore/meson.build b/src/libstore/meson.build index 59c62fe04..ae50686b6 100644 --- a/src/libstore/meson.build +++ b/src/libstore/meson.build @@ -154,12 +154,14 @@ endforeach busybox = find_program(get_option('sandbox-shell'), required : false) -# This one goes in config.h -# The path to busybox is passed as a -D flag when compiling this_library. -# This solution is inherited from the old make buildsystem -# TODO: do this differently? configdata_priv.set('HAVE_EMBEDDED_SANDBOX_SHELL', get_option('embedded-sandbox-shell').to_int()) +if get_option('embedded-sandbox-shell') + configdata_priv.set_quoted('SANDBOX_SHELL', '__embedded_sandbox_shell__') +elif busybox.found() + configdata_priv.set_quoted('SANDBOX_SHELL', busybox.full_path()) +endif + if get_option('embedded-sandbox-shell') hexdump = find_program('hexdump', native : true) embedded_sandbox_shell_gen = custom_target( @@ -178,6 +180,66 @@ if get_option('embedded-sandbox-shell') generated_headers += embedded_sandbox_shell_gen endif +fs = import('fs') + +prefix = get_option('prefix') +# For each of these paths, assume that it is relative to the prefix unless +# it is already an absolute path (which is the default for store-dir, localstatedir, and log-dir). +path_opts = [ + # Meson built-ins. + 'datadir', + 'mandir', + 'libdir', + 'includedir', + 'libexecdir', + # Homecooked Nix directories. + 'store-dir', + 'localstatedir', + 'log-dir', +] +# For your grepping pleasure, this loop sets the following variables that aren't mentioned +# literally above: +# store_dir +# localstatedir +# log_dir +# profile_dir +foreach optname : path_opts + varname = optname.replace('-', '_') + path = get_option(optname) + if fs.is_absolute(path) + set_variable(varname, path) + else + set_variable(varname, prefix / path) + endif +endforeach + +# sysconfdir doesn't get anything installed to directly, and is only used to +# tell Nix where to look for nix.conf, so it doesn't get appended to prefix. +sysconfdir = get_option('sysconfdir') +if not fs.is_absolute(sysconfdir) + sysconfdir = '/' / sysconfdir +endif + +# Aside from prefix itself, each of these was made into an absolute path +# by joining it with prefix, unless it was already an absolute path +# (which is the default for store-dir, localstatedir, and log-dir). +configdata_priv.set_quoted('NIX_PREFIX', prefix) +configdata_priv.set_quoted('NIX_STORE_DIR', store_dir) +configdata_priv.set_quoted('NIX_DATA_DIR', datadir) +configdata_priv.set_quoted('NIX_STATE_DIR', localstatedir / 'nix') +configdata_priv.set_quoted('NIX_LOG_DIR', log_dir) +configdata_priv.set_quoted('NIX_CONF_DIR', sysconfdir / 'nix') +configdata_priv.set_quoted('NIX_MAN_DIR', mandir) + +lsof = find_program('lsof', required : false) +configdata_priv.set_quoted( + 'LSOF', + lsof.found() + ? lsof.full_path() + # Just look up on the PATH + : 'lsof', +) + config_priv_h = configure_file( configuration : configdata_priv, output : 'store-config-private.hh', @@ -266,89 +328,6 @@ else subdir('unix') endif -fs = import('fs') - -prefix = get_option('prefix') -# For each of these paths, assume that it is relative to the prefix unless -# it is already an absolute path (which is the default for store-dir, localstatedir, and log-dir). -path_opts = [ - # Meson built-ins. - 'datadir', - 'mandir', - 'libdir', - 'includedir', - 'libexecdir', - # Homecooked Nix directories. - 'store-dir', - 'localstatedir', - 'log-dir', -] -# For your grepping pleasure, this loop sets the following variables that aren't mentioned -# literally above: -# store_dir -# localstatedir -# log_dir -# profile_dir -foreach optname : path_opts - varname = optname.replace('-', '_') - path = get_option(optname) - if fs.is_absolute(path) - set_variable(varname, path) - else - set_variable(varname, prefix / path) - endif -endforeach - -# sysconfdir doesn't get anything installed to directly, and is only used to -# tell Nix where to look for nix.conf, so it doesn't get appended to prefix. -sysconfdir = get_option('sysconfdir') -if not fs.is_absolute(sysconfdir) - sysconfdir = '/' / sysconfdir -endif - -lsof = find_program('lsof', required : false) - -# Aside from prefix itself, each of these was made into an absolute path -# by joining it with prefix, unless it was already an absolute path -# (which is the default for store-dir, localstatedir, and log-dir). -cpp_str_defines = { - 'NIX_PREFIX': prefix, - 'NIX_STORE_DIR': store_dir, - 'NIX_DATA_DIR': datadir, - 'NIX_STATE_DIR': localstatedir / 'nix', - 'NIX_LOG_DIR': log_dir, - 'NIX_CONF_DIR': sysconfdir / 'nix', - 'NIX_MAN_DIR': mandir, -} - -if lsof.found() - lsof_path = lsof.full_path() -else - # Just look up on the PATH - lsof_path = 'lsof' -endif -cpp_str_defines += { - 'LSOF': lsof_path -} - -if get_option('embedded-sandbox-shell') - cpp_str_defines += { - 'SANDBOX_SHELL': '__embedded_sandbox_shell__' - } -elif busybox.found() - cpp_str_defines += { - 'SANDBOX_SHELL': busybox.full_path() - } -endif - -cpp_args = [] - -foreach name, value : cpp_str_defines - cpp_args += [ - '-D' + name + '=' + '"' + value + '"' - ] -endforeach - subdir('nix-meson-build-support/export-all-symbols') subdir('nix-meson-build-support/windows-version') @@ -359,7 +338,6 @@ this_library = library( config_priv_h, dependencies : deps_public + deps_private + deps_other, include_directories : include_dirs, - cpp_args : cpp_args, link_args: linker_export_flags, prelink : true, # For C++ static initializers install : true, diff --git a/src/nix/man-pages.cc b/src/nix/man-pages.cc index 8da439e7b..8585c164c 100644 --- a/src/nix/man-pages.cc +++ b/src/nix/man-pages.cc @@ -1,4 +1,5 @@ #include "man-pages.hh" +#include "cli-config-private.hh" #include "nix/util/file-system.hh" #include "nix/util/current-process.hh" #include "nix/util/environment-variables.hh" diff --git a/src/nix/meson.build b/src/nix/meson.build index b258778cc..3cb45f1f5 100644 --- a/src/nix/meson.build +++ b/src/nix/meson.build @@ -39,13 +39,16 @@ configdata = configuration_data() configdata.set_quoted('NIX_CLI_VERSION', meson.project_version()) fs = import('fs') +prefix = get_option('prefix') bindir = get_option('bindir') -if not fs.is_absolute(bindir) - bindir = get_option('prefix') / bindir -endif +bindir = fs.is_absolute(bindir) ? bindir : prefix / bindir configdata.set_quoted('NIX_BIN_DIR', bindir) +mandir = get_option('mandir') +mandir = fs.is_absolute(mandir) ? mandir : prefix / mandir +configdata.set_quoted('NIX_MAN_DIR', mandir) + config_priv_h = configure_file( configuration : configdata, output : 'cli-config-private.hh', @@ -174,16 +177,6 @@ if host_machine.system() != 'windows' ] endif -fs = import('fs') -prefix = get_option('prefix') - -mandir = get_option('mandir') -mandir = fs.is_absolute(mandir) ? mandir : prefix / mandir - -cpp_args= [ - '-DNIX_MAN_DIR="@0@"'.format(mandir) -] - include_dirs = [include_directories('.')] this_exe = executable( @@ -191,7 +184,6 @@ this_exe = executable( sources, dependencies : deps_private_subproject + deps_private + deps_other, include_directories : include_dirs, - cpp_args : cpp_args, link_args: linker_export_flags, install : true, )