From ab427b4ac59a5fc6cf8be7f361012fb68fdb15b5 Mon Sep 17 00:00:00 2001 From: Qyriad Date: Sun, 19 May 2024 12:57:14 -0600 Subject: [PATCH 1/8] repl: log errors writing to history file These errors are now logged and explicitly ignored, rather than implicitly ignored. Change-Id: Ia26015466a17f2b11952df5317a4d150d79dc184 --- src/libcmd/repl-interacter.cc | 33 +++++++++++++++++++++++++++++++-- src/libcmd/repl-interacter.hh | 5 +++++ 2 files changed, 36 insertions(+), 2 deletions(-) diff --git a/src/libcmd/repl-interacter.cc b/src/libcmd/repl-interacter.cc index d3567e021..41589cda1 100644 --- a/src/libcmd/repl-interacter.cc +++ b/src/libcmd/repl-interacter.cc @@ -1,6 +1,8 @@ #include #include #include +#include +#include #ifdef READLINE #include @@ -176,15 +178,42 @@ bool ReadlineLikeInteracter::getLine(std::string & input, ReplPromptType promptT if (!s) return false; - write_history(historyFile.c_str()); + this->writeHistory(); input += s; input += '\n'; return true; } +void ReadlineLikeInteracter::writeHistory() +{ + int ret = write_history(historyFile.c_str()); + int writeHistErr = errno; + + if (ret == 0) { + return; + } + + // If the open fails, editline returns EOF. If the close fails, editline + // forwards the return value of fclose(), which is EOF on error. + // readline however, returns the errno. + // So if we didn't get exactly EOF, then consider the return value the error + // code; otherwise use the errno we saved above. + // https://github.com/troglobit/editline/issues/66 + if (ret != EOF) { + writeHistErr = ret; + } + + // In any of these cases, we should explicitly ignore the error, but log + // them so the user isn't confused why their history is getting eaten. + + std::string_view const errMsg(std::strerror(writeHistErr)); + warn("ignoring error writing repl history to %s: %s", this->historyFile, errMsg); + +} + ReadlineLikeInteracter::~ReadlineLikeInteracter() { - write_history(historyFile.c_str()); + this->writeHistory(); } AutomationInteracter::Guard AutomationInteracter::init(detail::ReplCompleterMixin *) diff --git a/src/libcmd/repl-interacter.hh b/src/libcmd/repl-interacter.hh index c31b1a1e6..8f815fceb 100644 --- a/src/libcmd/repl-interacter.hh +++ b/src/libcmd/repl-interacter.hh @@ -42,6 +42,11 @@ public: } virtual Guard init(detail::ReplCompleterMixin * repl) override; virtual bool getLine(std::string & input, ReplPromptType promptType) override; + /** Writes the current history to the history file. + * + * This function logs but ignores errors from readline's write_history(). + */ + virtual void writeHistory(); virtual ~ReadlineLikeInteracter() override; }; From 923e501bf7a5d0343c4b547a2670313fb3995aad Mon Sep 17 00:00:00 2001 From: Qyriad Date: Tue, 21 May 2024 12:10:24 -0600 Subject: [PATCH 2/8] add libcmd test for lookupFileArg Change-Id: I9e2ef170ffe916f902daec8b5630d29434c5d5f2 --- tests/unit/libcmd/args.cc | 57 +++++++++++++++++++++++++++++++++++++++ tests/unit/meson.build | 27 +++++++++++++++++++ 2 files changed, 84 insertions(+) create mode 100644 tests/unit/libcmd/args.cc diff --git a/tests/unit/libcmd/args.cc b/tests/unit/libcmd/args.cc new file mode 100644 index 000000000..73550dacf --- /dev/null +++ b/tests/unit/libcmd/args.cc @@ -0,0 +1,57 @@ +#include +#include +#include + +#include +#include + +#include "common-eval-args.hh" +#include "eval.hh" +#include "filetransfer.hh" +#include "shared.hh" +#include "store-api.hh" +#include "util.hh" + +constexpr std::string_view INVALID_CHANNEL = "channel:example"; +constexpr std::string_view CHANNEL_URL = "https://nixos.org/channels/example/nixexprs.tar.xz"; + +namespace nix +{ + +TEST(Arguments, lookupFileArg) { + initNix(); + initGC(); + + std::string const unitDataPath = getEnv("_NIX_TEST_UNIT_DATA").value(); + // Meson should be allowed to pass us a relative path here tbh. + auto const canonDataPath = CanonPath::fromCwd(unitDataPath); + + std::string const searchPathElem = fmt("example=%s", unitDataPath); + + SearchPath searchPath; + searchPath.elements.push_back(SearchPath::Elem::parse(searchPathElem)); + + auto store = openStore("dummy://"); + auto statePtr = std::make_shared(searchPath, store, store); + auto & state = *statePtr; + + SourcePath const foundUnitData = lookupFileArg(state, ""); + EXPECT_EQ(foundUnitData.path, canonDataPath); + + // lookupFileArg should not resolve if anything else is before or after it. + SourcePath const yepEvenSpaces = lookupFileArg(state, " "); + EXPECT_EQ(yepEvenSpaces.path, CanonPath::fromCwd(" ")); + EXPECT_EQ(lookupFileArg(state, "/nixos").path, CanonPath::fromCwd("/nixos")); + + try { + lookupFileArg(state, INVALID_CHANNEL); + } catch (FileTransferError const & ex) { + std::string_view const msg(ex.what()); + EXPECT_NE(msg.find(CHANNEL_URL), msg.npos); + } + + SourcePath const normalFile = lookupFileArg(state, unitDataPath); + EXPECT_EQ(normalFile.path, canonDataPath); +} + +} diff --git a/tests/unit/meson.build b/tests/unit/meson.build index f5355cce8..6041ba497 100644 --- a/tests/unit/meson.build +++ b/tests/unit/meson.build @@ -212,3 +212,30 @@ test( protocol : 'gtest', verbose : true, ) + +libcmd_tester = executable( + 'liblixcmd-tests', + files('libcmd/args.cc'), + dependencies : [ + liblixcmd, + liblixutil, + liblixmain, + liblixexpr, + liblixstore, + gtest, + boost, + ], +) + +test( + 'libcmd-unit-tests', + libcmd_tester, + args : tests_args, + env : { + # No special meaning here, it's just a file laying around that is unlikely to go anywhere + # any time soon. + '_NIX_TEST_UNIT_DATA': meson.project_source_root() / 'src/nix-env/buildenv.nix', + }, + suite : 'check', + protocol : 'gtest', +) From bbc6e99e79ffae629fa928e5d609dcaa50562d30 Mon Sep 17 00:00:00 2001 From: Qyriad Date: Mon, 20 May 2024 13:16:09 -0600 Subject: [PATCH 3/8] add docstring to lookupFileArg Change-Id: Ifc149764f5a15725d3d630677c6da29def4b0f3e --- src/libcmd/common-eval-args.hh | 22 +++++++++++++++++++++- 1 file changed, 21 insertions(+), 1 deletion(-) diff --git a/src/libcmd/common-eval-args.hh b/src/libcmd/common-eval-args.hh index 6359b2579..08a4b65e4 100644 --- a/src/libcmd/common-eval-args.hh +++ b/src/libcmd/common-eval-args.hh @@ -28,6 +28,26 @@ private: std::map autoArgs; }; -SourcePath lookupFileArg(EvalState & state, std::string_view s); +/** @brief Resolve an argument that is generally a file, but could be something that + * is easy to resolve to a file, like a or a tarball URL. + * + * In particular, this will resolve and fetch pseudo-URLs starting with + * @c channel:, flakerefs starting with @c flake:, and anything that + * @ref nix::fetchers::downloadTarball() can take. + * + * Non-absolute files are looked up relative to the current directory(?) + * FIXME: the process's current directory or EvalState's current directory? + * + * @param state The nix::EvalState to base settings, store, and nixPath from. + * + * @param fileArg The the path-ish to resolve. + * + * @return A nix::SourcePath to the resolved and fetched file. + * + * @exception nix::FileTransferError from nix::fetchers::downloadTarball(). Probably others. + * + * @exception nix::ThrownError for failed search path lookup. Probably others. + */ +SourcePath lookupFileArg(EvalState & state, std::string_view fileArg); } From 291ecb97dac675944a96971569f0b8c33bda5578 Mon Sep 17 00:00:00 2001 From: Qyriad Date: Mon, 20 May 2024 13:51:42 -0600 Subject: [PATCH 4/8] cleanup lookupFileArg Change-Id: I2acd56e7a542b12138f43c95af78fdd50e944619 --- src/libcmd/common-eval-args.cc | 32 +++++++++++++++++--------------- 1 file changed, 17 insertions(+), 15 deletions(-) diff --git a/src/libcmd/common-eval-args.cc b/src/libcmd/common-eval-args.cc index 94a4b7922..9beea5aa2 100644 --- a/src/libcmd/common-eval-args.cc +++ b/src/libcmd/common-eval-args.cc @@ -164,28 +164,30 @@ Bindings * MixEvalArgs::getAutoArgs(EvalState & state) return res.finish(); } -SourcePath lookupFileArg(EvalState & state, std::string_view s) +SourcePath lookupFileArg(EvalState & state, std::string_view fileArg) { - if (EvalSettings::isPseudoUrl(s)) { - auto storePath = fetchers::downloadTarball( - state.store, EvalSettings::resolvePseudoUrl(s), "source", false).tree.storePath; + if (EvalSettings::isPseudoUrl(fileArg)) { + auto const url = EvalSettings::resolvePseudoUrl(fileArg); + auto const downloaded = fetchers::downloadTarball( + state.store, + url, + /* name */ "source", + /* locked */ false + ); + StorePath const storePath = downloaded.tree.storePath; return state.rootPath(CanonPath(state.store->toRealPath(storePath))); - } - - else if (s.starts_with("flake:")) { + } else if (fileArg.starts_with("flake:")) { experimentalFeatureSettings.require(Xp::Flakes); - auto flakeRef = parseFlakeRef(std::string(s.substr(6)), {}, true, false); + static constexpr size_t FLAKE_LEN = std::string_view("flake:").size(); + auto flakeRef = parseFlakeRef(std::string(fileArg.substr(FLAKE_LEN)), {}, true, false); auto storePath = flakeRef.resolve(state.store).fetchTree(state.store).first.storePath; return state.rootPath(CanonPath(state.store->toRealPath(storePath))); - } - - else if (s.size() > 2 && s.at(0) == '<' && s.at(s.size() - 1) == '>') { - Path p(s.substr(1, s.size() - 2)); + } else if (fileArg.size() > 2 && fileArg.at(0) == '<' && fileArg.at(fileArg.size() - 1) == '>') { + Path p(fileArg.substr(1, fileArg.size() - 2)); return state.findFile(p); + } else { + return state.rootPath(CanonPath::fromCwd(fileArg)); } - - else - return state.rootPath(CanonPath::fromCwd(s)); } } From 87cf830e106b6fe2a9522619ed9820a9fabef219 Mon Sep 17 00:00:00 2001 From: Qyriad Date: Mon, 20 May 2024 20:54:00 -0600 Subject: [PATCH 5/8] build: make internal-api-docs PHONY Since we're skipping Meson's dependency tracking, for the internal-api-docs custom target, we should just consider it a phony target and build it on every request. Change-Id: I3b0bcea30ee9a4830023ccc5bededf995e96cccc --- doc/internal-api/meson.build | 1 + 1 file changed, 1 insertion(+) diff --git a/doc/internal-api/meson.build b/doc/internal-api/meson.build index 35d8a0e5b..faa30f194 100644 --- a/doc/internal-api/meson.build +++ b/doc/internal-api/meson.build @@ -28,6 +28,7 @@ internal_api_docs = custom_target( output : 'html', install : true, install_dir : datadir / 'doc/nix/internal-api', + build_always_stale : true, ) alias_target('internal-api-html', internal_api_docs) From 9dc5a17107d58fe71f9314b65ab9a34b6189de57 Mon Sep 17 00:00:00 2001 From: Qyriad Date: Mon, 20 May 2024 21:58:27 -0600 Subject: [PATCH 6/8] docs: linkify nix3-build mention in nix-build.md Change-Id: I462a8cf0da42b5045ce84b48dc1841ecdccbb89e --- doc/manual/src/command-ref/nix-build.md | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/doc/manual/src/command-ref/nix-build.md b/doc/manual/src/command-ref/nix-build.md index b548edf82..92e642ddd 100644 --- a/doc/manual/src/command-ref/nix-build.md +++ b/doc/manual/src/command-ref/nix-build.md @@ -14,9 +14,8 @@ # Disambiguation -This man page describes the command `nix-build`, which is distinct from `nix -build`. For documentation on the latter, run `nix build --help` or see `man -nix3-build`. +This man page describes the command [`nix-build`](./new-cli/nix3-build.md), which is distinct from `nix build`. +For documentation on the latter, run `nix build --help` or see `man nix3-build`. # Description From b4adc68c5435e6b6d1d405c58da4c2305f4f64bc Mon Sep 17 00:00:00 2001 From: Qyriad Date: Tue, 21 May 2024 15:37:08 -0600 Subject: [PATCH 7/8] docs: document the cursed file syntax for old cli This documents the fact that nix-build, nix-env, nix-instantiate, and nix-shell accept an extended syntax for their file arguments, including some well-known (but not well documented) aspects, like being able to specify ``, but also https:// tarball URLs, `flake:` prefixed flakerefs, and the cursed `channel:` prefixed hardcoded URLs Same thing for the new CLI incoming :) Change-Id: Ib6d68594a16132805ba5d97526e16f7b3633117e --- doc/manual/src/command-ref/fileish-summary.md | 14 ++++++ doc/manual/src/command-ref/nix-build.md | 50 ++++++++++++++++--- doc/manual/src/command-ref/nix-env.md | 2 +- .../src/command-ref/nix-env/opt-common.md | 16 ++++-- doc/manual/src/command-ref/nix-instantiate.md | 9 ++-- doc/manual/src/command-ref/nix-shell.md | 5 +- 6 files changed, 76 insertions(+), 20 deletions(-) create mode 100644 doc/manual/src/command-ref/fileish-summary.md diff --git a/doc/manual/src/command-ref/fileish-summary.md b/doc/manual/src/command-ref/fileish-summary.md new file mode 100644 index 000000000..31b68aada --- /dev/null +++ b/doc/manual/src/command-ref/fileish-summary.md @@ -0,0 +1,14 @@ + +- A normal filesystem path, like `/home/meow/nixfiles/default.nix` + - Including a directory, like `/home/meow/nixfiles`, equivalent to above +- A single lookup path, like `` or `` +- A URL to a tarball, like `https://github.com/NixOS/nixpkgs/archive/refs/heads/release-23.11.tar.gz` +- A flakeref, introduced by the prefix `flake:`, like `flake:git+https://git.lix.systems/lix-project/lix` +- A channel name, introduced by the prefix `channel:`, like `channel:nixpkgs-unstable`. + - This uses a hard-coded URL pattern and is *not* related to the subscribed channels managed by the [nix-channel](@docroot@/command-ref/nix-channel.md) command. diff --git a/doc/manual/src/command-ref/nix-build.md b/doc/manual/src/command-ref/nix-build.md index 92e642ddd..a91f9b7e0 100644 --- a/doc/manual/src/command-ref/nix-build.md +++ b/doc/manual/src/command-ref/nix-build.md @@ -4,7 +4,7 @@ # Synopsis -`nix-build` [*paths…*] +`nix-build` [*fileish…*] [`--arg` *name* *value*] [`--argstr` *name* *value*] [{`--attr` | `-A`} *attrPath*] @@ -20,19 +20,55 @@ For documentation on the latter, run `nix build --help` or see `man nix3-build`. # Description The `nix-build` command builds the derivations described by the Nix -expressions in *paths*. If the build succeeds, it places a symlink to +expressions in each *fileish*. If the build succeeds, it places a symlink to the result in the current directory. The symlink is called `result`. If there are multiple Nix expressions, or the Nix expressions evaluate to multiple derivations, multiple sequentially numbered symlinks are created (`result`, `result-2`, and so on). -If no *paths* are specified, then `nix-build` will use `default.nix` in +If no *fileish* is specified, then `nix-build` will use `default.nix` in the current directory, if it exists. -If an element of *paths* starts with `http://` or `https://`, it is -interpreted as the URL of a tarball that will be downloaded and unpacked -to a temporary location. The tarball must include a single top-level -directory containing at least a file named `default.nix`. +## Fileish Syntax + +A given *fileish* may take one of a few different forms, the first being a simple filesystem path, e.g. `nix-build /tmp/some-file.nix`. +This has the same semantics as the [import builtin](../language/builtins.md#builtins-import), so specifying a directory is equivalent to specifying `default.nix` within that directory. +It may also be a [search path](./env-common.html#env-NIX_PATH) (also known as a lookup path) like ``, which is convenient to use with `--attr`/`-A`: + +```console +$ nix-build '' -A firefox +``` + +(Note the quotation marks around ``, which will be necessary in most Unix shells.) + +If a *fileish* starts with `http://` or `https://`, it is interpreted as the URL of a tarball which will be fetched and unpacked. +Lix will then `import` the unpacked directory, so these tarballs must include at least single top-level directory with a file called `default.nix` +For example, you could build from a specific version of Nixpkgs with something like: + +```console +$ nix-build "https://github.com/NixOS/nixpkgs/archive/refs/heads/release-23.11.tar.gz" -A firefox +``` + +If a path starts with `flake:`, the rest of the argument is interpreted as a [flakeref](./new-cli/nix3-flake.html#flake-references) (see `nix flake --help` or `man nix3-flake`), which requires the "flakes" experimental feature to be enabled. +Lix will fetch the flake, and then `import` its unpacked directory, so the flake must include a file called `default.nix`. +For example, the flake analogues to the above `nix-build` commands are: + +```console +$ nix-build flake:nixpkgs -A firefox +$ nix-build flake:github:NixOS/nixpkgs/release-23.11 -A firefox +``` + +Finally, for legacy reasons, if a path starts with `channel:`, the rest of the argument is interpreted as the name of a channel to fetch from `https://nixos.org/channels/$CHANNEL_NAME/nixexprs.tar.xz`. +This is a **hard coded URL** pattern and is *not* related the subscribed channels managed by the [nix-channel](./nix-channel.md) command. + +> **Note**: any of the special syntaxes may always be disambiguated by prefixing the path. +> For example: a file in the current directory literally called `` can be addressed as `./`, to escape the special interpretation. + +In summary, a path argument may be one of: + +{{#include ./fileish-summary.md}} + +## Notes `nix-build` is essentially a wrapper around [`nix-instantiate`](nix-instantiate.md) (to translate a high-level Nix diff --git a/doc/manual/src/command-ref/nix-env.md b/doc/manual/src/command-ref/nix-env.md index 5a9e05fed..b3940aeaa 100644 --- a/doc/manual/src/command-ref/nix-env.md +++ b/doc/manual/src/command-ref/nix-env.md @@ -8,7 +8,7 @@ [`--option` *name* *value*] [`--arg` *name* *value*] [`--argstr` *name* *value*] - [{`--file` | `-f`} *path*] + [{`--file` | `-f`} *fileish*] [{`--profile` | `-p`} *path*] [`--system-filter` *system*] [`--dry-run`] diff --git a/doc/manual/src/command-ref/nix-env/opt-common.md b/doc/manual/src/command-ref/nix-env/opt-common.md index 636281b6d..6efd6a4e3 100644 --- a/doc/manual/src/command-ref/nix-env/opt-common.md +++ b/doc/manual/src/command-ref/nix-env/opt-common.md @@ -2,16 +2,22 @@ The following options are allowed for all `nix-env` operations, but may not always have an effect. - - `--file` / `-f` *path*\ + - `--file` / `-f` *fileish*\ Specifies the Nix expression (designated below as the *active Nix expression*) used by the `--install`, `--upgrade`, and `--query --available` operations to obtain derivations. The default is `~/.nix-defexpr`. - If the argument starts with `http://` or `https://`, it is - interpreted as the URL of a tarball that will be downloaded and - unpacked to a temporary location. The tarball must include a single - top-level directory containing at least a file named `default.nix`. + *fileish* is interpreted the same as with [nix-build](../nix-build.md#Path_Syntax). + See that section for complete details (`nix-build --help`), but in summary, a path argument may be one of: + + - A normal filesystem path, like `/home/meow/nixfiles/default.nix` + - Including a directory, like `/home/meow/nixfiles`, equivalent to above + - A single lookup path, like `` or `` + - A URL to a tarball, like `https://github.com/NixOS/nixpkgs/archive/refs/heads/release-23.11.tar.gz` + - A flakeref, introduced by the prefix `flake:`, like `flake:git+https://git.lix.systems/lix-project/lix` + - A channel name, introduced by the prefix `channel:`, like `channel:nixpkgs-unstable`. + - This uses a hard-coded URL pattern and is *not* related to the subscribed channels managed by the [nix-channel](@docroot@/command-ref/nix-channel.md) command. - `--profile` / `-p` *path*\ Specifies the profile to be used by those operations that operate on diff --git a/doc/manual/src/command-ref/nix-instantiate.md b/doc/manual/src/command-ref/nix-instantiate.md index 479c9abcf..f9c7fcac5 100644 --- a/doc/manual/src/command-ref/nix-instantiate.md +++ b/doc/manual/src/command-ref/nix-instantiate.md @@ -11,7 +11,7 @@ [{`--attr`| `-A`} *attrPath*] [`--add-root` *path*] [`--expr` | `-E`] - *files…* + *fileish…* `nix-instantiate` `--find-file` *files…* @@ -25,8 +25,11 @@ of the resulting store derivations are printed on standard output. [store derivation]: ../glossary.md#gloss-store-derivation -If *files* is the character `-`, then a Nix expression will be read from -standard input. +If *fileish* is the character `-`, then a Nix expression will be read from standard input. +Otherwise, each *fileish* is interpreted the same as with [nix-build](./nix-build.md#Path_Syntax). +See that section for complete details (`nix-build --help`), but in summary, a path argument may be one of: + +{{#include ./fileish-summary.md}} # Options diff --git a/doc/manual/src/command-ref/nix-shell.md b/doc/manual/src/command-ref/nix-shell.md index 1eaf3c36a..ce33b264f 100644 --- a/doc/manual/src/command-ref/nix-shell.md +++ b/doc/manual/src/command-ref/nix-shell.md @@ -33,10 +33,7 @@ the environment of a derivation for development. If *path* is not given, `nix-shell` defaults to `shell.nix` if it exists, and `default.nix` otherwise. -If *path* starts with `http://` or `https://`, it is interpreted as the -URL of a tarball that will be downloaded and unpacked to a temporary -location. The tarball must include a single top-level directory -containing at least a file named `default.nix`. +{{#include ./fileish-summary.md}} If the derivation defines the variable `shellHook`, it will be run after `$stdenv/setup` has been sourced. Since this hook is not executed From ea04545e7a538fa52eb6b093fccbed5bf2cb84a9 Mon Sep 17 00:00:00 2001 From: Qyriad Date: Wed, 22 May 2024 17:15:45 -0600 Subject: [PATCH 8/8] docstrings: NixRepl::getDerivationPath: exceptions directly thrown getDerivationPath() directly throws nix::Error for invalid derivations Change-Id: I81ead950060b789794fa683b61c6349fece1690d --- src/libcmd/repl.cc | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/libcmd/repl.cc b/src/libcmd/repl.cc index 525c25560..db9d0bd27 100644 --- a/src/libcmd/repl.cc +++ b/src/libcmd/repl.cc @@ -106,6 +106,11 @@ struct NixRepl void initEnv() override; virtual StringSet completePrefix(const std::string & prefix) override; + + /** + * @exception nix::Error thrown directly if the expression does not evaluate + * to a derivation, or evaluates to an invalid derivation. + */ StorePath getDerivationPath(Value & v); ProcessLineResult processLine(std::string line);