From 3daeeaefb115f47bf52ae96fa2b3f01842ec0f0f Mon Sep 17 00:00:00 2001 From: Jade Lovelace Date: Wed, 31 Jul 2024 20:37:27 -0700 Subject: build: implement clang-tidy using our plugin The principle of this is that you can either externally build it with Nix (actual implementation will be in a future commit), or it can be built with meson if the Nix one is not passed in. The idea I have is that dev shells don't receive the one from Nix to avoid having to build it, but CI can use the one from Nix and save some gratuitous rebuilds. The design of this is that you can run `ninja -C build clang-tidy` and it will simply correctly clang-tidy the codebase in spite of PCH bullshit caused by the cc-wrapper. This is a truly horrendous number of hacks in a ball, caused by bugs in several pieces of software, and I am not even getting started. I don't consider this to fix the clang-tidy issue filing, since we still have a fair number of issues to fix even on the existing minimal configuration, and I have not yet implemented it in CI. Realistically we will need to do something like https://github.com/Ericsson/codechecker to be able to silence warnings without physically touching the code, or at least *diff* reports between versions. Also, the run-clang-tidy output design is rather atrocious and must not be inflicted upon anyone I have respect for, since it buries the diagnostics in a pile of invocation logs. We would do really well to integrate with the Gerrit SARIF stuff so we can dump the reports on people in a user-friendly manner. Related: https://git.lix.systems/lix-project/lix/issues/147 Change-Id: Ifefe533f3b56874795de231667046b2da6ff2461 --- clang-tidy/.clang-format | 1 - clang-tidy/.editorconfig | 4 - clang-tidy/FixIncludes.cc | 90 ---------------------- clang-tidy/FixIncludes.hh | 21 ----- clang-tidy/HasPrefixSuffix.cc | 80 ------------------- clang-tidy/HasPrefixSuffix.hh | 25 ------ clang-tidy/LixClangTidyChecks.cc | 19 ----- clang-tidy/README.md | 56 -------------- clang-tidy/meson.build | 13 ---- doc/manual/rl-next/clang-tidy-sorta.md | 10 +++ flake.nix | 4 + justfile | 10 +++ meson.build | 2 + meson.options | 8 +- meson/clang-tidy/build_required_targets.py | 21 +++++ meson/clang-tidy/clean_compdb.py | 53 +++++++++++++ meson/clang-tidy/fake.cc | 0 meson/clang-tidy/meson.build | 98 ++++++++++++++++++++++++ subprojects/lix-clang-tidy/.clang-format | 1 + subprojects/lix-clang-tidy/.editorconfig | 4 + subprojects/lix-clang-tidy/FixIncludes.cc | 90 ++++++++++++++++++++++ subprojects/lix-clang-tidy/FixIncludes.hh | 21 +++++ subprojects/lix-clang-tidy/HasPrefixSuffix.cc | 80 +++++++++++++++++++ subprojects/lix-clang-tidy/HasPrefixSuffix.hh | 25 ++++++ subprojects/lix-clang-tidy/LixClangTidyChecks.cc | 19 +++++ subprojects/lix-clang-tidy/README.md | 56 ++++++++++++++ subprojects/lix-clang-tidy/default.nix | 44 +++++++++++ subprojects/lix-clang-tidy/meson.build | 18 +++++ subprojects/lix-clang-tidy/meson.options | 3 + 29 files changed, 565 insertions(+), 311 deletions(-) delete mode 100644 clang-tidy/.clang-format delete mode 100644 clang-tidy/.editorconfig delete mode 100644 clang-tidy/FixIncludes.cc delete mode 100644 clang-tidy/FixIncludes.hh delete mode 100644 clang-tidy/HasPrefixSuffix.cc delete mode 100644 clang-tidy/HasPrefixSuffix.hh delete mode 100644 clang-tidy/LixClangTidyChecks.cc delete mode 100644 clang-tidy/README.md delete mode 100644 clang-tidy/meson.build create mode 100644 doc/manual/rl-next/clang-tidy-sorta.md create mode 100755 meson/clang-tidy/build_required_targets.py create mode 100755 meson/clang-tidy/clean_compdb.py create mode 100644 meson/clang-tidy/fake.cc create mode 100644 meson/clang-tidy/meson.build create mode 100644 subprojects/lix-clang-tidy/.clang-format create mode 100644 subprojects/lix-clang-tidy/.editorconfig create mode 100644 subprojects/lix-clang-tidy/FixIncludes.cc create mode 100644 subprojects/lix-clang-tidy/FixIncludes.hh create mode 100644 subprojects/lix-clang-tidy/HasPrefixSuffix.cc create mode 100644 subprojects/lix-clang-tidy/HasPrefixSuffix.hh create mode 100644 subprojects/lix-clang-tidy/LixClangTidyChecks.cc create mode 100644 subprojects/lix-clang-tidy/README.md create mode 100644 subprojects/lix-clang-tidy/default.nix create mode 100644 subprojects/lix-clang-tidy/meson.build create mode 100644 subprojects/lix-clang-tidy/meson.options diff --git a/clang-tidy/.clang-format b/clang-tidy/.clang-format deleted file mode 100644 index cd8995543..000000000 --- a/clang-tidy/.clang-format +++ /dev/null @@ -1 +0,0 @@ -BasedOnStyle: llvm diff --git a/clang-tidy/.editorconfig b/clang-tidy/.editorconfig deleted file mode 100644 index 19ee09eec..000000000 --- a/clang-tidy/.editorconfig +++ /dev/null @@ -1,4 +0,0 @@ -# LLVM style code is 2-space indented -[*.{cc,hh}] -indent_style = space -indent_size = 2 diff --git a/clang-tidy/FixIncludes.cc b/clang-tidy/FixIncludes.cc deleted file mode 100644 index 602d3d355..000000000 --- a/clang-tidy/FixIncludes.cc +++ /dev/null @@ -1,90 +0,0 @@ -#include "FixIncludes.hh" -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include - -namespace nix::clang_tidy { - -using namespace clang; -using namespace clang::tidy; - -class FixIncludesCallbacks : public PPCallbacks { -public: - ClangTidyCheck &Check; - Preprocessor &PP; - FixIncludesCallbacks(ClangTidyCheck &Check, Preprocessor &PP) - : Check(Check), PP(PP) {} - -private: - bool Ignore = false; - virtual void LexedFileChanged(FileID FID, LexedFileChangeReason Reason, - SrcMgr::CharacteristicKind FileType, - FileID PrevFID, SourceLocation Loc) override; - - virtual void InclusionDirective(SourceLocation HashLoc, - const Token &IncludeTok, StringRef FileName, - bool IsAngled, CharSourceRange FilenameRange, - OptionalFileEntryRef File, - StringRef SearchPath, StringRef RelativePath, - const Module *Imported, - SrcMgr::CharacteristicKind FileType) override; -}; - -void FixIncludesCallbacks::LexedFileChanged(FileID, LexedFileChangeReason, - SrcMgr::CharacteristicKind FileType, - FileID, SourceLocation) { - Ignore = FileType != SrcMgr::C_User; -} - -void FixIncludesCallbacks::InclusionDirective( - SourceLocation, const Token &, StringRef FileName, bool IsAngled, - CharSourceRange FilenameRange, OptionalFileEntryRef File, StringRef, - StringRef, const Module *, SrcMgr::CharacteristicKind) { - if (Ignore) - return; - - // FIXME: this is kinda evil, but this is a one-time fixup - const std::vector SourceDirs = {"src/", "include/lix/"}; - - const auto Bracketize = [IsAngled](StringRef s) { - return IsAngled ? ("<" + s + ">").str() : ("\"" + s + "\"").str(); - }; - - for (const auto &SourceDir : SourceDirs) { - const bool IsAlreadyFixed = FileName.starts_with("lix/lib"); - if (File && File->getNameAsRequested().contains(SourceDir) && - !IsAlreadyFixed) { - StringRef Name = File->getNameAsRequested(); - auto Idx = Name.find(SourceDir); - assert(Idx != std::string::npos); - std::string Suffix = Name.drop_front(Idx + SourceDir.length()).str(); - - if (!Suffix.starts_with("lib")) { - llvm::dbgs() << "ignored: " << Suffix << "\n"; - return; - } - - Suffix = "lix/" + Suffix; - - auto Diag = Check.diag(FilenameRange.getBegin(), - "include needs to specify the source subdir"); - - Diag << FilenameRange - << FixItHint::CreateReplacement(FilenameRange, Bracketize(Suffix)); - } - } -} - -void FixIncludesCheck::registerPPCallbacks(const SourceManager &, - Preprocessor *PP, Preprocessor *) { - PP->addPPCallbacks(std::make_unique(*this, *PP)); -} - -}; // namespace nix::clang_tidy diff --git a/clang-tidy/FixIncludes.hh b/clang-tidy/FixIncludes.hh deleted file mode 100644 index ea890cd39..000000000 --- a/clang-tidy/FixIncludes.hh +++ /dev/null @@ -1,21 +0,0 @@ -#pragma once -///@file - -#include -#include -#include - -namespace nix::clang_tidy { - -using namespace clang; -using namespace clang::tidy; - -class FixIncludesCheck : public ClangTidyCheck { - public: - FixIncludesCheck(StringRef Name, ClangTidyContext *Context) - : ClangTidyCheck(Name, Context) {} - - void registerPPCallbacks(const SourceManager &SM, Preprocessor *PP, Preprocessor *ModuleExpanderPP) override; -}; - -}; diff --git a/clang-tidy/HasPrefixSuffix.cc b/clang-tidy/HasPrefixSuffix.cc deleted file mode 100644 index e29b67e7c..000000000 --- a/clang-tidy/HasPrefixSuffix.cc +++ /dev/null @@ -1,80 +0,0 @@ -#include "HasPrefixSuffix.hh" -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include - -namespace nix::clang_tidy { -using namespace clang::ast_matchers; -using namespace clang; - -void HasPrefixSuffixCheck::registerMatchers(ast_matchers::MatchFinder *Finder) { - Finder->addMatcher( - traverse(clang::TK_AsIs, - callExpr(callee(functionDecl(anyOf(hasName("hasPrefix"), - hasName("hasSuffix"))) - .bind("callee-decl")), - optionally(hasArgument( - 0, cxxConstructExpr( - hasDeclaration(functionDecl(hasParameter( - 0, parmVarDecl(hasType( - asString("const char *"))))))) - .bind("implicit-cast")))) - .bind("call")), - this); -} - -void HasPrefixSuffixCheck::check( - const ast_matchers::MatchFinder::MatchResult &Result) { - - const auto *CalleeDecl = Result.Nodes.getNodeAs("callee-decl"); - auto FuncName = std::string(CalleeDecl->getName()); - std::string NewName; - if (FuncName == "hasPrefix") { - NewName = "starts_with"; - } else if (FuncName == "hasSuffix") { - NewName = "ends_with"; - } else { - llvm_unreachable("nix-has-prefix: invalid callee"); - } - - const auto *MatchedDecl = Result.Nodes.getNodeAs("call"); - const auto *ImplicitConvertArg = - Result.Nodes.getNodeAs("implicit-cast"); - - const auto *Lhs = MatchedDecl->getArg(0); - const auto *Rhs = MatchedDecl->getArg(1); - auto Diag = diag(MatchedDecl->getExprLoc(), FuncName + " is deprecated"); - - std::string Text = ""; - - // Form possible cast to string_view, or nothing. - if (ImplicitConvertArg) { - Text = "std::string_view("; - Text.append(tooling::getText(*Lhs, *Result.Context)); - Text.append(")."); - } else { - Text.append(*tooling::buildAccess(*Lhs, *Result.Context)); - } - - // Call .starts_with. - Text.append(NewName); - Text.push_back('('); - Text.append(tooling::getText(*Rhs, *Result.Context)); - Text.push_back(')'); - - Diag << FixItHint::CreateReplacement(MatchedDecl->getSourceRange(), Text); - - // for (const auto *arg : MatchedDecl->arguments()) { - // arg->dumpColor(); - // arg->getType().dump(); - // } -} -}; // namespace nix::clang_tidy diff --git a/clang-tidy/HasPrefixSuffix.hh b/clang-tidy/HasPrefixSuffix.hh deleted file mode 100644 index 8693b6767..000000000 --- a/clang-tidy/HasPrefixSuffix.hh +++ /dev/null @@ -1,25 +0,0 @@ -#pragma once -///@file -/// This is an example of a clang-tidy automated refactoring against the Nix -/// codebase. The refactoring has been completed in -/// https://gerrit.lix.systems/c/lix/+/565 so this code is around as -/// an example. - -#include -#include -#include - -namespace nix::clang_tidy { - -using namespace clang; -using namespace clang::tidy; -using namespace llvm; - -class HasPrefixSuffixCheck : public ClangTidyCheck { -public: - HasPrefixSuffixCheck(StringRef Name, ClangTidyContext *Context) - : ClangTidyCheck(Name, Context) {} - void registerMatchers(ast_matchers::MatchFinder *Finder) override; - void check(const ast_matchers::MatchFinder::MatchResult &Result) override; -}; -}; // namespace nix::clang_tidy diff --git a/clang-tidy/LixClangTidyChecks.cc b/clang-tidy/LixClangTidyChecks.cc deleted file mode 100644 index b3503dd3a..000000000 --- a/clang-tidy/LixClangTidyChecks.cc +++ /dev/null @@ -1,19 +0,0 @@ -#include -#include -#include "FixIncludes.hh" -#include "HasPrefixSuffix.hh" - -namespace nix::clang_tidy { -using namespace clang; -using namespace clang::tidy; - -class NixClangTidyChecks : public ClangTidyModule { - public: - void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override { - CheckFactories.registerCheck("lix-hasprefixsuffix"); - CheckFactories.registerCheck("lix-fixincludes"); - } -}; - -static ClangTidyModuleRegistry::Add X("lix-module", "Adds lix specific checks"); -}; diff --git a/clang-tidy/README.md b/clang-tidy/README.md deleted file mode 100644 index c2d1cb258..000000000 --- a/clang-tidy/README.md +++ /dev/null @@ -1,56 +0,0 @@ -# Clang tidy lints for Lix - -This is a skeleton of a clang-tidy lints library for Lix. - -Currently there is one check (which is already obsolete as it has served its -goal and is there as an example), `HasPrefixSuffixCheck`. - -## Running fixes/checks - -One file: - -``` -ninja -C build && clang-tidy --checks='-*,lix-*' --load=build/liblix-clang-tidy.so -p ../compile_commands.json -header-filter '\.\./src/.*\.h' --fix ../src/libcmd/installables.cc -``` - -Several files, in parallel: - -``` -ninja -C build && run-clang-tidy -checks='-*,lix-*' -load=build/liblix-clang-tidy.so -p .. -header-filter '\.\./src/.*\.h' -fix ../src | tee -a clang-tidy-result -``` - -## Resources - -* https://firefox-source-docs.mozilla.org/code-quality/static-analysis/writing-new/clang-query.html -* https://clang.llvm.org/docs/LibASTMatchersReference.html -* https://devblogs.microsoft.com/cppblog/exploring-clang-tooling-part-3-rewriting-code-with-clang-tidy/ - -## Developing new checks - -Put something like so in `myquery.txt`: - -``` -set traversal IgnoreUnlessSpelledInSource -# ^ Ignore implicit AST nodes. May need to use AsIs depending on how you are -# working. -set bind-root true -# ^ true unless you use any .bind("foo") commands -set print-matcher true -enable output dump -match callExpr(callee(functionDecl(hasName("hasPrefix"))), optionally(hasArgument( 0, cxxConstructExpr(hasDeclaration(functionDecl(hasParameter(0, parmVarDecl(hasType(asString("const char *"))).bind("meow2")))))))) -``` - -Then run, e.g. `clang-query --preload hasprefix.query -p compile_commands.json src/libcmd/installables.cc`. - -With this you can iterate a query before writing it in C++ and suffering from -C++. - -### Tips and tricks for the C++ - -There is a function `dump()` on many things that will dump to stderr. Also -`llvm::errs()` lets you print to stderr. - -When I wrote `HasPrefixSuffixCheck`, I was not really able to figure out how -the structured replacement system was supposed to work. In principle you can -describe the replacement with a nice DSL. Look up the Stencil system in Clang -for details. diff --git a/clang-tidy/meson.build b/clang-tidy/meson.build deleted file mode 100644 index c59164a72..000000000 --- a/clang-tidy/meson.build +++ /dev/null @@ -1,13 +0,0 @@ -project('lix-clang-tidy', ['cpp', 'c'], - version : '0.1', - default_options : ['warning_level=3', 'cpp_std=c++20']) - -llvm = dependency('Clang', version: '>= 14', modules: ['libclang']) -sources = files( - 'HasPrefixSuffix.cc', - 'LixClangTidyChecks.cc', - 'FixIncludes.cc', -) - -shared_module('lix-clang-tidy', sources, - dependencies: llvm) diff --git a/doc/manual/rl-next/clang-tidy-sorta.md b/doc/manual/rl-next/clang-tidy-sorta.md new file mode 100644 index 000000000..5f3f84348 --- /dev/null +++ b/doc/manual/rl-next/clang-tidy-sorta.md @@ -0,0 +1,10 @@ +--- +synopsis: "clang-tidy support" +cls: 1697 +issues: fj#147 +credits: jade +category: Development +--- + +`clang-tidy` can be used to lint Lix with a limited set of lints using `ninja -C build clang-tidy` and `ninja -C build clang-tidy-fix`. +In practice, this fixes the built-in meson rule that was used the same as above being broken ever since precompiled headers were introduced. diff --git a/flake.nix b/flake.nix index a1fc947b7..14fdf4147 100644 --- a/flake.nix +++ b/flake.nix @@ -197,6 +197,8 @@ busybox-sandbox-shell = final.busybox-sandbox-shell or final.default-busybox-sandbox-shell; }; + lix-clang-tidy = final.callPackage ./subprojects/lix-clang-tidy { }; + # Export the patched version of boehmgc that Lix uses into the overlay # for consumers of this flake. boehmgc-nix = final.nix.passthru.boehmgc-nix; @@ -384,6 +386,8 @@ rec { inherit (nixpkgsFor.${system}.native) nix; default = nix; + + inherit (nixpkgsFor.${system}.native) lix-clang-tidy; } // ( lib.optionalAttrs (builtins.elem system linux64BitSystems) { diff --git a/justfile b/justfile index 6a93fa63f..1576c759d 100644 --- a/justfile +++ b/justfile @@ -25,3 +25,13 @@ install *OPTIONS: (build OPTIONS) # Run tests test *OPTIONS: meson test -C build --print-errorlogs {{ OPTIONS }} + +alias clang-tidy := lint + +lint: + ninja -C build clang-tidy + +alias clang-tidy-fix := lint-fix + +lint-fix: + ninja -C build clang-tidy-fix diff --git a/meson.build b/meson.build index ed50dff78..8577657d9 100644 --- a/meson.build +++ b/meson.build @@ -548,3 +548,5 @@ if enable_tests subdir('tests/unit') subdir('tests/functional') endif + +subdir('meson/clang-tidy') diff --git a/meson.options b/meson.options index fc2050809..679d88347 100644 --- a/meson.options +++ b/meson.options @@ -1,7 +1,7 @@ # vim: filetype=meson option('enable-build', type : 'boolean', value : true, - description : 'Set to false to not actually build. Only really makes sense with -Dinternal-api-docs=true', + description : 'set to false to not actually build. Only really makes sense with -Dinternal-api-docs=true', ) option('gc', type : 'feature', @@ -37,7 +37,7 @@ option('tests-brief', type : 'boolean', value : false, ) option('profile-build', type : 'feature', value: 'disabled', - description : 'whether to enable -ftime-trace in clang builds, allowing for speeding up the build.' + description : 'whether to enable -ftime-trace in clang builds, allowing for diagnosing the cause of build time.' ) option('store-dir', type : 'string', value : '/nix/store', @@ -68,3 +68,7 @@ option('profile-dir', type : 'string', value : 'etc/profile.d', option('enable-pch-std', type : 'boolean', value : true, description : 'whether to use precompiled headers for C++\'s standard library (breaks clangd if you\'re using GCC)', ) + +option('lix-clang-tidy-checks-path', type : 'string', value : '', + description: 'path to lix-clang-tidy-checks library file, if providing it externally. Uses an internal one if this is not set', +) diff --git a/meson/clang-tidy/build_required_targets.py b/meson/clang-tidy/build_required_targets.py new file mode 100755 index 000000000..5c0e9641e --- /dev/null +++ b/meson/clang-tidy/build_required_targets.py @@ -0,0 +1,21 @@ +#!/usr/bin/env python3 +import subprocess + +def get_targets_of_rule(build_root: str, rule_name: str) -> list[str]: + return subprocess.check_output(['ninja', '-C', build_root, '-t', 'targets', 'rule', rule_name]).decode().strip().splitlines() + +def ninja_build(build_root: str, targets: list[str]): + subprocess.check_call(['ninja', '-C', build_root, '--', *targets]) + +def main(): + import argparse + ap = argparse.ArgumentParser(description='Builds required targets for clang-tidy') + ap.add_argument('build_root', help='Ninja build root', type=str) + + args = ap.parse_args() + + targets = [t for t in get_targets_of_rule(args.build_root, 'CUSTOM_COMMAND') if t.endswith('gen.hh')] + ninja_build(args.build_root, targets) + +if __name__ == '__main__': + main() diff --git a/meson/clang-tidy/clean_compdb.py b/meson/clang-tidy/clean_compdb.py new file mode 100755 index 000000000..6736fe63a --- /dev/null +++ b/meson/clang-tidy/clean_compdb.py @@ -0,0 +1,53 @@ +#!/usr/bin/env python3 +# Deletes the PCH arguments from a compilation database, to workaround nixpkgs +# stdenv having a cc-wrapper that is impossible to use for anything except cc +# itself, for example, clang-tidy. + +import json +import shlex + + +def process_compdb(compdb: list[dict]) -> list[dict]: + + def munch_command(args: list[str]) -> list[str]: + out = [] + eat_next = False + for i, arg in enumerate(args): + if arg == '-fpch-preprocess': + # as used with gcc + continue + elif arg == '-include-pch' or (arg == '-include' and args[i + 1] == 'precompiled-headers.hh'): + # -include-pch some-pch (clang), or -include some-pch (gcc) + eat_next = True + continue + if not eat_next: + out.append(arg) + eat_next = False + return out + + def chomp(item: dict) -> dict: + item = item.copy() + item['command'] = shlex.join(munch_command(shlex.split(item['command']))) + return item + + return [chomp(x) for x in compdb if not x['file'].endswith('precompiled-headers.hh')] + + +def main(): + import argparse + ap = argparse.ArgumentParser( + description='Delete pch arguments from compilation database') + ap.add_argument('input', + type=argparse.FileType('r'), + help='Input json file') + ap.add_argument('output', + type=argparse.FileType('w'), + help='Output json file') + args = ap.parse_args() + + input_json = json.load(args.input) + json.dump(process_compdb(input_json), args.output, indent=2) + + +if __name__ == '__main__': + main() diff --git a/meson/clang-tidy/fake.cc b/meson/clang-tidy/fake.cc new file mode 100644 index 000000000..e69de29bb diff --git a/meson/clang-tidy/meson.build b/meson/clang-tidy/meson.build new file mode 100644 index 000000000..9aba5fbdc --- /dev/null +++ b/meson/clang-tidy/meson.build @@ -0,0 +1,98 @@ +# The clang-tidy target for Lix + +run_clang_tidy = find_program('run-clang-tidy', required : false) +# Although this looks like it wants to be pkg-config, pkg-config does not +# really work for *plugins*, which are executable-like .so files that also +# cannot be found via find_program. Fun! +if get_option('lix-clang-tidy-checks-path') != '' + lix_clang_tidy_so = get_option('lix-clang-tidy-checks-path') + lix_clang_tidy_so_found = true +else + lix_clang_tidy_subproj = subproject( + 'lix-clang-tidy', + required : false, + default_options : {'build-by-default': false} + ) + if lix_clang_tidy_subproj.found() + lix_clang_tidy_so = lix_clang_tidy_subproj.get_variable('lix_clang_tidy') + lix_clang_tidy_so_found = true + else + lix_clang_tidy_so_found = false + endif +endif + +# Due to numerous problems, such as: +# - Meson does not expose pch targets, but *fine*, I can just ask Ninja for +# them with `ninja -t targets rule cpp_PCH` and build them manually: +# https://github.com/mesonbuild/meson/issues/13499 +# - Nixpkgs stdenv buries the cc-wrapper under a giant pile of assumptions +# about the cc-wrapper actually being used on the cc of a stdenv, rather than +# independently for clang-tidy, and we need to use cc-wrapper to get the +# correct hardening flags so that clang-tidy can actually parse the PCH file +# +# I give up. I am going to delete the damn PCH args and then it will work. +meson.add_postconf_script( + python, + meson.current_source_dir() / 'clean_compdb.py', + meson.global_build_root() / 'compile_commands.json', + meson.current_build_dir() / 'compile_commands.json', +) + +# Horrible hack to get around not being able to depend on another target's +# generated headers in any way in the meson DSL +# https://github.com/mesonbuild/meson/issues/12817 which was incorrectly +# closed, if you *actually* need to generate the files once. +# Also related: https://github.com/mesonbuild/meson/issues/3667 +# +# Or we could ban meson generators because their design is broken. +build_all_generated_headers = custom_target( + command : [ + python, + meson.current_source_dir() / 'build_required_targets.py', + meson.global_build_root(), + ], + output : 'generated_headers.stamp', + build_by_default : false, + build_always_stale : true, +) + +if lix_clang_tidy_so_found + run_clang_tidy_args = [ + '-load', + lix_clang_tidy_so, + '-p', + # We have to workaround a run-clang-tidy bug too, so we must give the + # directory name rather than the actual compdb file. + # https://github.com/llvm/llvm-project/issues/101440 + meson.current_build_dir(), + '-quiet', + ] + run_target( + 'clang-tidy', + command : [ + # XXX: This explicitly invokes it with python because of a nixpkgs bug + # where clang-unwrapped does not patch interpreters in run-clang-tidy. + # However, making clang-unwrapped depend on python is also silly, so idk. + python, + run_clang_tidy, + run_clang_tidy_args, + '-warnings-as-errors', + '*', + ], + depends : [ + build_all_generated_headers, + ], + ) + run_target( + 'clang-tidy-fix', + command : [ + python, + run_clang_tidy, + run_clang_tidy_args, + '-fix', + ], + depends : [ + build_all_generated_headers, + ], + ) +endif diff --git a/subprojects/lix-clang-tidy/.clang-format b/subprojects/lix-clang-tidy/.clang-format new file mode 100644 index 000000000..cd8995543 --- /dev/null +++ b/subprojects/lix-clang-tidy/.clang-format @@ -0,0 +1 @@ +BasedOnStyle: llvm diff --git a/subprojects/lix-clang-tidy/.editorconfig b/subprojects/lix-clang-tidy/.editorconfig new file mode 100644 index 000000000..19ee09eec --- /dev/null +++ b/subprojects/lix-clang-tidy/.editorconfig @@ -0,0 +1,4 @@ +# LLVM style code is 2-space indented +[*.{cc,hh}] +indent_style = space +indent_size = 2 diff --git a/subprojects/lix-clang-tidy/FixIncludes.cc b/subprojects/lix-clang-tidy/FixIncludes.cc new file mode 100644 index 000000000..602d3d355 --- /dev/null +++ b/subprojects/lix-clang-tidy/FixIncludes.cc @@ -0,0 +1,90 @@ +#include "FixIncludes.hh" +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +namespace nix::clang_tidy { + +using namespace clang; +using namespace clang::tidy; + +class FixIncludesCallbacks : public PPCallbacks { +public: + ClangTidyCheck &Check; + Preprocessor &PP; + FixIncludesCallbacks(ClangTidyCheck &Check, Preprocessor &PP) + : Check(Check), PP(PP) {} + +private: + bool Ignore = false; + virtual void LexedFileChanged(FileID FID, LexedFileChangeReason Reason, + SrcMgr::CharacteristicKind FileType, + FileID PrevFID, SourceLocation Loc) override; + + virtual void InclusionDirective(SourceLocation HashLoc, + const Token &IncludeTok, StringRef FileName, + bool IsAngled, CharSourceRange FilenameRange, + OptionalFileEntryRef File, + StringRef SearchPath, StringRef RelativePath, + const Module *Imported, + SrcMgr::CharacteristicKind FileType) override; +}; + +void FixIncludesCallbacks::LexedFileChanged(FileID, LexedFileChangeReason, + SrcMgr::CharacteristicKind FileType, + FileID, SourceLocation) { + Ignore = FileType != SrcMgr::C_User; +} + +void FixIncludesCallbacks::InclusionDirective( + SourceLocation, const Token &, StringRef FileName, bool IsAngled, + CharSourceRange FilenameRange, OptionalFileEntryRef File, StringRef, + StringRef, const Module *, SrcMgr::CharacteristicKind) { + if (Ignore) + return; + + // FIXME: this is kinda evil, but this is a one-time fixup + const std::vector SourceDirs = {"src/", "include/lix/"}; + + const auto Bracketize = [IsAngled](StringRef s) { + return IsAngled ? ("<" + s + ">").str() : ("\"" + s + "\"").str(); + }; + + for (const auto &SourceDir : SourceDirs) { + const bool IsAlreadyFixed = FileName.starts_with("lix/lib"); + if (File && File->getNameAsRequested().contains(SourceDir) && + !IsAlreadyFixed) { + StringRef Name = File->getNameAsRequested(); + auto Idx = Name.find(SourceDir); + assert(Idx != std::string::npos); + std::string Suffix = Name.drop_front(Idx + SourceDir.length()).str(); + + if (!Suffix.starts_with("lib")) { + llvm::dbgs() << "ignored: " << Suffix << "\n"; + return; + } + + Suffix = "lix/" + Suffix; + + auto Diag = Check.diag(FilenameRange.getBegin(), + "include needs to specify the source subdir"); + + Diag << FilenameRange + << FixItHint::CreateReplacement(FilenameRange, Bracketize(Suffix)); + } + } +} + +void FixIncludesCheck::registerPPCallbacks(const SourceManager &, + Preprocessor *PP, Preprocessor *) { + PP->addPPCallbacks(std::make_unique(*this, *PP)); +} + +}; // namespace nix::clang_tidy diff --git a/subprojects/lix-clang-tidy/FixIncludes.hh b/subprojects/lix-clang-tidy/FixIncludes.hh new file mode 100644 index 000000000..ea890cd39 --- /dev/null +++ b/subprojects/lix-clang-tidy/FixIncludes.hh @@ -0,0 +1,21 @@ +#pragma once +///@file + +#include +#include +#include + +namespace nix::clang_tidy { + +using namespace clang; +using namespace clang::tidy; + +class FixIncludesCheck : public ClangTidyCheck { + public: + FixIncludesCheck(StringRef Name, ClangTidyContext *Context) + : ClangTidyCheck(Name, Context) {} + + void registerPPCallbacks(const SourceManager &SM, Preprocessor *PP, Preprocessor *ModuleExpanderPP) override; +}; + +}; diff --git a/subprojects/lix-clang-tidy/HasPrefixSuffix.cc b/subprojects/lix-clang-tidy/HasPrefixSuffix.cc new file mode 100644 index 000000000..e29b67e7c --- /dev/null +++ b/subprojects/lix-clang-tidy/HasPrefixSuffix.cc @@ -0,0 +1,80 @@ +#include "HasPrefixSuffix.hh" +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +namespace nix::clang_tidy { +using namespace clang::ast_matchers; +using namespace clang; + +void HasPrefixSuffixCheck::registerMatchers(ast_matchers::MatchFinder *Finder) { + Finder->addMatcher( + traverse(clang::TK_AsIs, + callExpr(callee(functionDecl(anyOf(hasName("hasPrefix"), + hasName("hasSuffix"))) + .bind("callee-decl")), + optionally(hasArgument( + 0, cxxConstructExpr( + hasDeclaration(functionDecl(hasParameter( + 0, parmVarDecl(hasType( + asString("const char *"))))))) + .bind("implicit-cast")))) + .bind("call")), + this); +} + +void HasPrefixSuffixCheck::check( + const ast_matchers::MatchFinder::MatchResult &Result) { + + const auto *CalleeDecl = Result.Nodes.getNodeAs("callee-decl"); + auto FuncName = std::string(CalleeDecl->getName()); + std::string NewName; + if (FuncName == "hasPrefix") { + NewName = "starts_with"; + } else if (FuncName == "hasSuffix") { + NewName = "ends_with"; + } else { + llvm_unreachable("nix-has-prefix: invalid callee"); + } + + const auto *MatchedDecl = Result.Nodes.getNodeAs("call"); + const auto *ImplicitConvertArg = + Result.Nodes.getNodeAs("implicit-cast"); + + const auto *Lhs = MatchedDecl->getArg(0); + const auto *Rhs = MatchedDecl->getArg(1); + auto Diag = diag(MatchedDecl->getExprLoc(), FuncName + " is deprecated"); + + std::string Text = ""; + + // Form possible cast to string_view, or nothing. + if (ImplicitConvertArg) { + Text = "std::string_view("; + Text.append(tooling::getText(*Lhs, *Result.Context)); + Text.append(")."); + } else { + Text.append(*tooling::buildAccess(*Lhs, *Result.Context)); + } + + // Call .starts_with. + Text.append(NewName); + Text.push_back('('); + Text.append(tooling::getText(*Rhs, *Result.Context)); + Text.push_back(')'); + + Diag << FixItHint::CreateReplacement(MatchedDecl->getSourceRange(), Text); + + // for (const auto *arg : MatchedDecl->arguments()) { + // arg->dumpColor(); + // arg->getType().dump(); + // } +} +}; // namespace nix::clang_tidy diff --git a/subprojects/lix-clang-tidy/HasPrefixSuffix.hh b/subprojects/lix-clang-tidy/HasPrefixSuffix.hh new file mode 100644 index 000000000..8693b6767 --- /dev/null +++ b/subprojects/lix-clang-tidy/HasPrefixSuffix.hh @@ -0,0 +1,25 @@ +#pragma once +///@file +/// This is an example of a clang-tidy automated refactoring against the Nix +/// codebase. The refactoring has been completed in +/// https://gerrit.lix.systems/c/lix/+/565 so this code is around as +/// an example. + +#include +#include +#include + +namespace nix::clang_tidy { + +using namespace clang; +using namespace clang::tidy; +using namespace llvm; + +class HasPrefixSuffixCheck : public ClangTidyCheck { +public: + HasPrefixSuffixCheck(StringRef Name, ClangTidyContext *Context) + : ClangTidyCheck(Name, Context) {} + void registerMatchers(ast_matchers::MatchFinder *Finder) override; + void check(const ast_matchers::MatchFinder::MatchResult &Result) override; +}; +}; // namespace nix::clang_tidy diff --git a/subprojects/lix-clang-tidy/LixClangTidyChecks.cc b/subprojects/lix-clang-tidy/LixClangTidyChecks.cc new file mode 100644 index 000000000..b3503dd3a --- /dev/null +++ b/subprojects/lix-clang-tidy/LixClangTidyChecks.cc @@ -0,0 +1,19 @@ +#include +#include +#include "FixIncludes.hh" +#include "HasPrefixSuffix.hh" + +namespace nix::clang_tidy { +using namespace clang; +using namespace clang::tidy; + +class NixClangTidyChecks : public ClangTidyModule { + public: + void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override { + CheckFactories.registerCheck("lix-hasprefixsuffix"); + CheckFactories.registerCheck("lix-fixincludes"); + } +}; + +static ClangTidyModuleRegistry::Add X("lix-module", "Adds lix specific checks"); +}; diff --git a/subprojects/lix-clang-tidy/README.md b/subprojects/lix-clang-tidy/README.md new file mode 100644 index 000000000..c2d1cb258 --- /dev/null +++ b/subprojects/lix-clang-tidy/README.md @@ -0,0 +1,56 @@ +# Clang tidy lints for Lix + +This is a skeleton of a clang-tidy lints library for Lix. + +Currently there is one check (which is already obsolete as it has served its +goal and is there as an example), `HasPrefixSuffixCheck`. + +## Running fixes/checks + +One file: + +``` +ninja -C build && clang-tidy --checks='-*,lix-*' --load=build/liblix-clang-tidy.so -p ../compile_commands.json -header-filter '\.\./src/.*\.h' --fix ../src/libcmd/installables.cc +``` + +Several files, in parallel: + +``` +ninja -C build && run-clang-tidy -checks='-*,lix-*' -load=build/liblix-clang-tidy.so -p .. -header-filter '\.\./src/.*\.h' -fix ../src | tee -a clang-tidy-result +``` + +## Resources + +* https://firefox-source-docs.mozilla.org/code-quality/static-analysis/writing-new/clang-query.html +* https://clang.llvm.org/docs/LibASTMatchersReference.html +* https://devblogs.microsoft.com/cppblog/exploring-clang-tooling-part-3-rewriting-code-with-clang-tidy/ + +## Developing new checks + +Put something like so in `myquery.txt`: + +``` +set traversal IgnoreUnlessSpelledInSource +# ^ Ignore implicit AST nodes. May need to use AsIs depending on how you are +# working. +set bind-root true +# ^ true unless you use any .bind("foo") commands +set print-matcher true +enable output dump +match callExpr(callee(functionDecl(hasName("hasPrefix"))), optionally(hasArgument( 0, cxxConstructExpr(hasDeclaration(functionDecl(hasParameter(0, parmVarDecl(hasType(asString("const char *"))).bind("meow2")))))))) +``` + +Then run, e.g. `clang-query --preload hasprefix.query -p compile_commands.json src/libcmd/installables.cc`. + +With this you can iterate a query before writing it in C++ and suffering from +C++. + +### Tips and tricks for the C++ + +There is a function `dump()` on many things that will dump to stderr. Also +`llvm::errs()` lets you print to stderr. + +When I wrote `HasPrefixSuffixCheck`, I was not really able to figure out how +the structured replacement system was supposed to work. In principle you can +describe the replacement with a nice DSL. Look up the Stencil system in Clang +for details. diff --git a/subprojects/lix-clang-tidy/default.nix b/subprojects/lix-clang-tidy/default.nix new file mode 100644 index 000000000..1bfc2d9a4 --- /dev/null +++ b/subprojects/lix-clang-tidy/default.nix @@ -0,0 +1,44 @@ +{ + lib, + stdenv, + cmake, + meson, + ninja, + llvmPackages, +}: +let + inherit (lib) fileset; +in +stdenv.mkDerivation { + pname = "lix-clang-tidy-checks"; + # Setting the version to the Lix version is just going to cause pointless + # rebuilds due to versionSuffix and similar, and I cannot conceive of a usage + # where we actually care about its version since this is internal-only. + version = "0.1"; + + src = fileset.toSource { + root = ./.; + fileset = fileset.unions [ + ./meson.build + ./meson.options + (fileset.fileFilter ( + { hasExt, ... }: + builtins.any hasExt [ + "cc" + "hh" + ] + ) ./.) + ]; + }; + + nativeBuildInputs = [ + meson + cmake + ninja + ]; + + buildInputs = [ + llvmPackages.llvm + llvmPackages.clang-unwrapped.dev + ]; +} diff --git a/subprojects/lix-clang-tidy/meson.build b/subprojects/lix-clang-tidy/meson.build new file mode 100644 index 000000000..ef0226420 --- /dev/null +++ b/subprojects/lix-clang-tidy/meson.build @@ -0,0 +1,18 @@ +project('lix-clang-tidy', ['cpp', 'c'], + version : '0.1', + default_options : ['warning_level=3', 'cpp_std=c++20'] +) + +llvm = dependency('Clang', version: '>= 17', modules: ['libclang']) +sources = files( + 'HasPrefixSuffix.cc', + 'LixClangTidyChecks.cc', + 'FixIncludes.cc', +) + +lix_clang_tidy = shared_module('lix-clang-tidy', sources, + dependencies: llvm, + # overrides build_by_default, see https://github.com/mesonbuild/meson/issues/13498 + install : get_option('build-by-default'), + build_by_default : get_option('build-by-default') +) diff --git a/subprojects/lix-clang-tidy/meson.options b/subprojects/lix-clang-tidy/meson.options new file mode 100644 index 000000000..57f0f713b --- /dev/null +++ b/subprojects/lix-clang-tidy/meson.options @@ -0,0 +1,3 @@ +option('build-by-default', type : 'boolean', value : true, + description : 'set to false to not actually build targets by default. This is a hack to deal with meson lacking a build_by_default default option and building subprojects by default' +) -- cgit v1.2.3 From 0f998056fabe314fb4bde296c0970805e086307e Mon Sep 17 00:00:00 2001 From: Jade Lovelace Date: Wed, 31 Jul 2024 20:59:33 -0700 Subject: misc docs/meson tidying The docs page has an incorrect escape that leads to a backslash appearing in output. Meson stuff is self-explanatory, just shortens and simplifies a bit. Change-Id: Ib63adf934efd3caeb82ca82988f230e8858a79f9 --- doc/manual/src/language/advanced-attributes.md | 1 - src/nix/meson.build | 17 +++++++---------- 2 files changed, 7 insertions(+), 11 deletions(-) diff --git a/doc/manual/src/language/advanced-attributes.md b/doc/manual/src/language/advanced-attributes.md index ec21b9990..e9006a3ae 100644 --- a/doc/manual/src/language/advanced-attributes.md +++ b/doc/manual/src/language/advanced-attributes.md @@ -326,7 +326,6 @@ Derivations can declare some infrequently used optional attributes. ``` - [`unsafeDiscardReferences`]{#adv-attr-unsafeDiscardReferences}\ - When using [structured attributes](#adv-attr-structuredAttrs), the attribute `unsafeDiscardReferences` is an attribute set with a boolean value for each output name. If set to `true`, it disables scanning the output for runtime dependencies. diff --git a/src/nix/meson.build b/src/nix/meson.build index 97387e402..80223a390 100644 --- a/src/nix/meson.build +++ b/src/nix/meson.build @@ -1,8 +1,8 @@ -generate_manpage_gen = gen_header.process(meson.project_source_root() / 'doc/manual/generate-manpage.nix') - -utils_gen = gen_header.process(meson.project_source_root() / 'doc/manual/utils.nix') - -get_env_gen = gen_header.process('get-env.sh') +nix_generated_headers = [ + gen_header.process(meson.project_source_root() / 'doc/manual/generate-manpage.nix'), + gen_header.process(meson.project_source_root() / 'doc/manual/utils.nix'), + gen_header.process('get-env.sh'), +] # src/nix/profile.cc includes src/nix/profile.md, which includes "doc/files/profiles.md.gen.hh". # Unfortunately, https://github.com/mesonbuild/meson/issues/2320. @@ -18,7 +18,7 @@ run_command( meson.current_build_dir() / 'doc/files/profiles.md', check : true, ) -profiles_md_gen = gen_header.process( +nix_generated_headers += gen_header.process( meson.current_build_dir() / 'doc/files/profiles.md', preserve_path_from : meson.current_build_dir(), ) @@ -74,10 +74,7 @@ nix_sources = files( nix = executable( 'nix', nix_sources, - generate_manpage_gen, - utils_gen, - get_env_gen, - profiles_md_gen, + nix_generated_headers, nix2_commands_sources, dependencies : [ libasanoptions, -- cgit v1.2.3 From 700762d8b24350bbe537258167244ec9c84fcae3 Mon Sep 17 00:00:00 2001 From: Jade Lovelace Date: Thu, 1 Aug 2024 21:32:11 -0700 Subject: manual: fix a syntax error in redirects.js that made it not do anything lol lmao Let's put in a syntax checker in CI so we do not have to deal with this nonsense ever again. Change-Id: I0fe875e0cfc59ab1783087762e5bb07e09ded105 --- doc/manual/redirects.js | 2 +- flake.nix | 18 ++++++++++++++++++ 2 files changed, 19 insertions(+), 1 deletion(-) diff --git a/doc/manual/redirects.js b/doc/manual/redirects.js index f270d31a4..7a61de643 100644 --- a/doc/manual/redirects.js +++ b/doc/manual/redirects.js @@ -345,7 +345,7 @@ const redirects = { "linux": "uninstall.html#linux", "macos": "uninstall.html#macos", "uninstalling": "uninstall.html", - } + }, "contributing/hacking.html": { "nix-with-flakes": "#building-nix-with-flakes", "classic-nix": "#building-nix", diff --git a/flake.nix b/flake.nix index 14fdf4147..64d16b588 100644 --- a/flake.nix +++ b/flake.nix @@ -292,6 +292,24 @@ werror = true; }; + # Although this might be nicer to do with pre-commit, that would + # require adding 12MB of nodejs to the dev shell, whereas building it + # in CI with Nix avoids that at a cost of slower feedback on rarely + # touched files. + jsSyntaxCheck = + let + nixpkgs = nixpkgsFor.x86_64-linux.native; + inherit (nixpkgs) pkgs; + docSources = lib.fileset.toSource { + root = ./doc; + fileset = lib.fileset.fileFilter (f: f.hasExt "js") ./doc; + }; + in + pkgs.runCommand "js-syntax-check" { } '' + find ${docSources} -type f -print -exec ${pkgs.nodejs-slim}/bin/node --check '{}' ';' + touch $out + ''; + # Make sure that nix-env still produces the exact same result # on a particular version of Nixpkgs. evalNixpkgs = -- cgit v1.2.3 From 378ec5fb0611e314511a7afc806664205846fc2e Mon Sep 17 00:00:00 2001 From: Jade Lovelace Date: Thu, 1 Aug 2024 12:26:16 -0700 Subject: Implement forcing CLI colour on, and document it better This is necessary to make some old tests work when testing colour against non-interactive outputs. Change-Id: Id89f8a1f45c587fede35a69db85f7a52f2c0a981 --- doc/manual/src/contributing/testing.md | 1 + src/libutil/terminal.cc | 18 +++++++++++++++--- src/libutil/terminal.hh | 9 +++++++++ 3 files changed, 25 insertions(+), 3 deletions(-) diff --git a/doc/manual/src/contributing/testing.md b/doc/manual/src/contributing/testing.md index cea6ee3bf..33197b0ba 100644 --- a/doc/manual/src/contributing/testing.md +++ b/doc/manual/src/contributing/testing.md @@ -427,6 +427,7 @@ I grepped `src/` for `get[eE]nv\("` to find the mentions in Lix code. - `NIX_SHOW_STATS_PATH` - Writes those statistics into a file at the given path instead of stdout. Undocumented. - `NIX_SHOW_SYMBOLS` - Dumps the symbol table into the show-stats json output. - `TERM` - If `dumb` or unset, disables ANSI colour output. +- `FORCE_COLOR`, `CLICOLOR_FORCE` - Enables ANSI colour output if `NO_COLOR`/`NOCOLOR` not set. - `NO_COLOR`, `NOCOLOR` - Disables ANSI colour output. - `_NIX_DEVELOPER_SHOW_UNKNOWN_LOCATIONS` - Highlights unknown locations in errors. - `NIX_PROFILE` - Selects which profile `nix-env` will operate on. Documented elsewhere. diff --git a/src/libutil/terminal.cc b/src/libutil/terminal.cc index b58331d04..68d358dc5 100644 --- a/src/libutil/terminal.cc +++ b/src/libutil/terminal.cc @@ -9,9 +9,21 @@ namespace nix { bool shouldANSI() { - return isatty(STDERR_FILENO) - && getEnv("TERM").value_or("dumb") != "dumb" - && !(getEnv("NO_COLOR").has_value() || getEnv("NOCOLOR").has_value()); + // Implements the behaviour described by https://bixense.com/clicolors/ + // As well as https://force-color.org/ for compatibility, since it fits in the same shape. + // NO_COLOR CLICOLOR CLICOLOR_FORCE Colours? + // set x x No + // unset x set Yes + // unset x unset If attached to a terminal + // [we choose the "modern" approach of colour-by-default] + auto compute = []() -> bool { + bool mustNotColour = getEnv("NO_COLOR").has_value() || getEnv("NOCOLOR").has_value(); + bool shouldForce = getEnv("CLICOLOR_FORCE").has_value() || getEnv("FORCE_COLOR").has_value(); + bool isTerminal = isatty(STDERR_FILENO) && getEnv("TERM").value_or("dumb") != "dumb"; + return !mustNotColour && (shouldForce || isTerminal); + }; + static bool cached = compute(); + return cached; } std::string filterANSIEscapes(std::string_view s, bool filterAll, unsigned int width) diff --git a/src/libutil/terminal.hh b/src/libutil/terminal.hh index 43df5bd70..6b8d59182 100644 --- a/src/libutil/terminal.hh +++ b/src/libutil/terminal.hh @@ -9,6 +9,15 @@ namespace nix { /** * Determine whether ANSI escape sequences are appropriate for the * present output. + * + * This follows the rules described on https://bixense.com/clicolors/ + * with CLICOLOR defaulted to enabled (and thus ignored). + * + * That is to say, the following procedure is followed in order: + * - NO_COLOR or NOCOLOR set -> always disable colour + * - CLICOLOR_FORCE or FORCE_COLOR set -> enable colour + * - The output is a tty; TERM != "dumb" -> enable colour + * - Otherwise -> disable colour */ bool shouldANSI(); -- cgit v1.2.3 From 5f0ef50077002f0308fc45f7d0a01a508c970516 Mon Sep 17 00:00:00 2001 From: Jade Lovelace Date: Wed, 31 Jul 2024 22:06:18 -0700 Subject: cli: eat terminal codes from stdout also This *should* be sound, plus or minus the amount that the terminal code eating code is messed up already. This is useful for testing CLI output because it will strip the escapes enough to just shove the expected output in a file. Change-Id: I8a9b58fafb918466ac76e9ab585fc32fb9294819 --- doc/manual/rl-next/clicolor-clarity.md | 26 ++++++++++++++++++++++++++ src/libutil/logging.cc | 2 +- src/libutil/terminal.cc | 5 +++-- src/libutil/terminal.hh | 3 ++- tests/functional/search.sh | 2 ++ 5 files changed, 34 insertions(+), 4 deletions(-) create mode 100644 doc/manual/rl-next/clicolor-clarity.md diff --git a/doc/manual/rl-next/clicolor-clarity.md b/doc/manual/rl-next/clicolor-clarity.md new file mode 100644 index 000000000..8a289e362 --- /dev/null +++ b/doc/manual/rl-next/clicolor-clarity.md @@ -0,0 +1,26 @@ +--- +synopsis: "Better usage of colour control environment variables" +cls: [1699, 1702] +credits: [jade] +category: Improvements +--- + +Lix now heeds `NO_COLOR`/`NOCOLOR` for more output types, such as that used in `nix search`, `nix flake metadata` and similar. + +It also now supports `CLICOLOR_FORCE`/`FORCE_COLOR` to force colours regardless of whether there is a terminal on the other side. + +It now follows rules compatible with those described on with `CLICOLOR` defaulted to enabled. + +That is to say, the following procedure is followed in order: +- NO_COLOR or NOCOLOR set + + Always disable colour +- CLICOLOR_FORCE or FORCE_COLOR set + + Enable colour +- The output is a tty; TERM != "dumb" + + Enable colour +- Otherwise + + Disable colour diff --git a/src/libutil/logging.cc b/src/libutil/logging.cc index 53460f729..cbeb7aa36 100644 --- a/src/libutil/logging.cc +++ b/src/libutil/logging.cc @@ -37,7 +37,7 @@ void Logger::warn(const std::string & msg) void Logger::writeToStdout(std::string_view s) { - writeFull(STDOUT_FILENO, s); + writeFull(STDOUT_FILENO, filterANSIEscapes(s, !shouldANSI(), std::numeric_limits::max(), false)); writeFull(STDOUT_FILENO, "\n"); } diff --git a/src/libutil/terminal.cc b/src/libutil/terminal.cc index 68d358dc5..2ba1cb81b 100644 --- a/src/libutil/terminal.cc +++ b/src/libutil/terminal.cc @@ -26,7 +26,8 @@ bool shouldANSI() return cached; } -std::string filterANSIEscapes(std::string_view s, bool filterAll, unsigned int width) +// FIXME(jade): replace with TerminalCodeEater. wowie this is evil code. +std::string filterANSIEscapes(std::string_view s, bool filterAll, unsigned int width, bool eatTabs) { std::string t, e; size_t w = 0; @@ -55,7 +56,7 @@ std::string filterANSIEscapes(std::string_view s, bool filterAll, unsigned int w t += e; } - else if (*i == '\t') { + else if (*i == '\t' && eatTabs) { i++; t += ' '; w++; while (w < (size_t) width && w % 8) { t += ' '; w++; diff --git a/src/libutil/terminal.hh b/src/libutil/terminal.hh index 6b8d59182..2c422ecff 100644 --- a/src/libutil/terminal.hh +++ b/src/libutil/terminal.hh @@ -30,7 +30,8 @@ bool shouldANSI(); */ std::string filterANSIEscapes(std::string_view s, bool filterAll = false, - unsigned int width = std::numeric_limits::max()); + unsigned int width = std::numeric_limits::max(), + bool eatTabs = true); /** * Recalculate the window size, updating a global variable. Used in the diff --git a/tests/functional/search.sh b/tests/functional/search.sh index d9c7a75da..1a2a20089 100644 --- a/tests/functional/search.sh +++ b/tests/functional/search.sh @@ -29,6 +29,8 @@ nix search -f search.nix '' ^ | grepQuiet hello ## Tests for multiple regex/match highlighting +# FIXME: possibly not test this with colour in the future +export CLICOLOR_FORCE=1 e=$'\x1b' # grep doesn't support \e, \033 or even \x1b # Multiple overlapping regexes (( $(nix search -f search.nix '' 'oo' 'foo' 'oo' | grep -c "$e\[32;1mfoo$e\\[0;1m") == 1 )) -- cgit v1.2.3 From bd1344ec54e1935f19cf50a8ea15c43792f1c767 Mon Sep 17 00:00:00 2001 From: Jade Lovelace Date: Wed, 31 Jul 2024 22:15:54 -0700 Subject: nix flake metadata: print modified dates for input flakes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This was always in the lock file and we can simply actually print it. The test for this is a little bit silly but it should correctly control for my daring to exercise timezone code *and* locale code in a test, which I strongly suspect nobody dared do before. Sample (abridged): ``` Path: /nix/store/gaxb42z68bcr8lch467shvmnhjjzgd8b-source Last modified: 1970-01-01 00:16:40 Inputs: ├───flake-compat: github:edolstra/flake-compat/0f9255e01c2351cc7d116c072cb317785dd33b33 │ Last modified: 2023-10-04 13:37:54 ├───flake-utils: github:numtide/flake-utils/b1d9ab70662946ef0850d488da1c9019f3a9752a │ Last modified: 2024-03-11 08:33:50 │ └───systems: github:nix-systems/default/da67096a3b9bf56a91d16901293e51ba5b49a27e │ Last modified: 2023-04-09 08:27:08 ``` Change-Id: I355f82cb4b633974295375ebad646fb6e2107f9b --- doc/manual/rl-next/flake-metadata-time.md | 28 +++ src/nix/flake.cc | 24 +- tests/functional/flakes/flake-metadata.sh | 36 +++ tests/functional/flakes/flake-metadata/flake.lock | 245 +++++++++++++++++++++ .../flakes/flake-metadata/metadata.out.expected | 36 +++ tests/functional/meson.build | 3 +- 6 files changed, 366 insertions(+), 6 deletions(-) create mode 100644 doc/manual/rl-next/flake-metadata-time.md create mode 100644 tests/functional/flakes/flake-metadata.sh create mode 100644 tests/functional/flakes/flake-metadata/flake.lock create mode 100644 tests/functional/flakes/flake-metadata/metadata.out.expected diff --git a/doc/manual/rl-next/flake-metadata-time.md b/doc/manual/rl-next/flake-metadata-time.md new file mode 100644 index 000000000..8664b5a75 --- /dev/null +++ b/doc/manual/rl-next/flake-metadata-time.md @@ -0,0 +1,28 @@ +--- +synopsis: "`nix flake metadata` prints modified date" +cls: 1700 +credits: jade +category: Improvements +--- + +Ever wonder "gee, when *did* I update nixpkgs"? +Wonder no more, because `nix flake metadata` now simply tells you the times every locked flake input was updated: + +``` +<...> +Description: The purely functional package manager +Path: /nix/store/c91yi8sxakc2ry7y4ac1smzwka4l5p78-source +Revision: c52cff582043838bbe29768e7da232483d52b61d-dirty +Last modified: 2024-07-31 22:15:54 +Inputs: +├───flake-compat: github:edolstra/flake-compat/0f9255e01c2351cc7d116c072cb317785dd33b33 +│ Last modified: 2023-10-04 06:37:54 +├───nix2container: github:nlewo/nix2container/3853e5caf9ad24103b13aa6e0e8bcebb47649fe4 +│ Last modified: 2024-07-10 13:15:56 +├───nixpkgs: github:NixOS/nixpkgs/e21630230c77140bc6478a21cd71e8bb73706fce +│ Last modified: 2024-07-25 11:26:27 +├───nixpkgs-regression: github:NixOS/nixpkgs/215d4d0fd80ca5163643b03a33fde804a29cc1e2 +│ Last modified: 2022-01-24 11:20:45 +└───pre-commit-hooks: github:cachix/git-hooks.nix/f451c19376071a90d8c58ab1a953c6e9840527fd + Last modified: 2024-07-15 04:21:09 +``` diff --git a/src/nix/flake.cc b/src/nix/flake.cc index 9d18b81b8..672930342 100644 --- a/src/nix/flake.cc +++ b/src/nix/flake.cc @@ -209,6 +209,11 @@ struct CmdFlakeMetadata : FlakeCommand, MixJSON { auto lockedFlake = lockFlake(); auto & flake = lockedFlake.flake; + auto formatTime = [](time_t time) -> std::string { + std::ostringstream os{}; + os << std::put_time(std::localtime(&time), "%F %T"); + return os.str(); + }; if (json) { nlohmann::json j; @@ -260,7 +265,7 @@ struct CmdFlakeMetadata : FlakeCommand, MixJSON if (auto lastModified = flake.lockedRef.input.getLastModified()) logger->cout( ANSI_BOLD "Last modified:" ANSI_NORMAL " %s", - std::put_time(std::localtime(&*lastModified), "%F %T")); + formatTime(*lastModified)); if (!lockedFlake.lockFile.root->inputs.empty()) logger->cout(ANSI_BOLD "Inputs:" ANSI_NORMAL); @@ -275,16 +280,25 @@ struct CmdFlakeMetadata : FlakeCommand, MixJSON bool last = i + 1 == node.inputs.size(); if (auto lockedNode = std::get_if<0>(&input.second)) { - logger->cout("%s" ANSI_BOLD "%s" ANSI_NORMAL ": %s", - prefix + (last ? treeLast : treeConn), input.first, + // ├───agenix: github:ryantm/agenix/8d37c5bdeade12b6479c85acd133063ab53187a0 + logger->cout("%s%s" ANSI_BOLD "%s" ANSI_NORMAL ": %s", + prefix, last ? treeLast : treeConn, input.first, (*lockedNode)->lockedRef); + // ├───lix: https://git.lix.systems/api/v1/repos/lix-project <....> + // │ Last modified: 2024-07-31 21:01:34 + if (auto lastModified = (*lockedNode)->lockedRef.input.getLastModified()) { + logger->cout("%s%s" ANSI_BOLD "%s" ANSI_NORMAL ": %s", + prefix, last ? treeNull : treeLine, "Last modified", formatTime(*lastModified)); + } + bool firstVisit = visited.insert(*lockedNode).second; if (firstVisit) recurse(**lockedNode, prefix + (last ? treeNull : treeLine)); } else if (auto follows = std::get_if<1>(&input.second)) { - logger->cout("%s" ANSI_BOLD "%s" ANSI_NORMAL " follows input '%s'", - prefix + (last ? treeLast : treeConn), input.first, + // │ ├───darwin follows input 'flake-utils' + logger->cout("%s%s" ANSI_BOLD "%s" ANSI_NORMAL " follows input '%s'", + prefix, last ? treeLast : treeConn, input.first, printInputPath(*follows)); } } diff --git a/tests/functional/flakes/flake-metadata.sh b/tests/functional/flakes/flake-metadata.sh new file mode 100644 index 000000000..ab5a69b3a --- /dev/null +++ b/tests/functional/flakes/flake-metadata.sh @@ -0,0 +1,36 @@ +source ./common.sh + +flakeDir=$TEST_ROOT/flake +mkdir -p "$flakeDir" + +cat > "$flakeDir/flake.nix" <<-'EOF' +{ + inputs = { + nixpkgs.url = "github:nixos/nixpkgs/nixos-unstable-small"; + flake-utils.url = "github:numtide/flake-utils"; + flake-compat = { + url = "github:edolstra/flake-compat"; + flake = false; + }; + lanzaboote = { + url = "github:nix-community/lanzaboote"; + inputs.nixpkgs.follows = "nixpkgs"; + inputs.flake-utils.follows = "flake-utils"; + inputs.flake-compat.follows = "flake-compat"; + }; + }; + + outputs = { ... }: {}; +} +EOF + +cp flake-metadata/flake.lock "$flakeDir" +touch -d @1000 "$flakeDir/flake.nix" "$flakeDir/flake.lock" "$flakeDir" + +# For some reason we use NIX_STORE_DIR which causes unstable paths. This is +# goofy. We can just use --store, which sets rootDir and does not have this +# problem. +actualStore=$NIX_STORE_DIR +unset NIX_STORE_DIR +NOCOLOR=1 TZ=UTC LC_ALL=C.UTF-8 nix flake metadata --store "$actualStore" "$flakeDir" | grep -v -e 'Locked URL:' -e 'Resolved URL:' > "$TEST_ROOT/metadata.out.actual" +diff -u flake-metadata/metadata.out.expected "$TEST_ROOT/metadata.out.actual" diff --git a/tests/functional/flakes/flake-metadata/flake.lock b/tests/functional/flakes/flake-metadata/flake.lock new file mode 100644 index 000000000..62b7dc3e8 --- /dev/null +++ b/tests/functional/flakes/flake-metadata/flake.lock @@ -0,0 +1,245 @@ +{ + "nodes": { + "crane": { + "inputs": { + "nixpkgs": [ + "lanzaboote", + "nixpkgs" + ] + }, + "locked": { + "lastModified": 1711299236, + "narHash": "sha256-6/JsyozOMKN8LUGqWMopKTSiK8N79T8Q+hcxu2KkTXg=", + "owner": "ipetkov", + "repo": "crane", + "rev": "880573f80d09e18a11713f402b9e6172a085449f", + "type": "github" + }, + "original": { + "owner": "ipetkov", + "repo": "crane", + "type": "github" + } + }, + "flake-compat": { + "flake": false, + "locked": { + "lastModified": 1696426674, + "narHash": "sha256-kvjfFW7WAETZlt09AgDn1MrtKzP7t90Vf7vypd3OL1U=", + "owner": "edolstra", + "repo": "flake-compat", + "rev": "0f9255e01c2351cc7d116c072cb317785dd33b33", + "type": "github" + }, + "original": { + "owner": "edolstra", + "repo": "flake-compat", + "type": "github" + } + }, + "flake-parts": { + "inputs": { + "nixpkgs-lib": [ + "lanzaboote", + "nixpkgs" + ] + }, + "locked": { + "lastModified": 1709336216, + "narHash": "sha256-Dt/wOWeW6Sqm11Yh+2+t0dfEWxoMxGBvv3JpIocFl9E=", + "owner": "hercules-ci", + "repo": "flake-parts", + "rev": "f7b3c975cf067e56e7cda6cb098ebe3fb4d74ca2", + "type": "github" + }, + "original": { + "owner": "hercules-ci", + "repo": "flake-parts", + "type": "github" + } + }, + "flake-utils": { + "inputs": { + "systems": "systems" + }, + "locked": { + "lastModified": 1710146030, + "narHash": "sha256-SZ5L6eA7HJ/nmkzGG7/ISclqe6oZdOZTNoesiInkXPQ=", + "owner": "numtide", + "repo": "flake-utils", + "rev": "b1d9ab70662946ef0850d488da1c9019f3a9752a", + "type": "github" + }, + "original": { + "owner": "numtide", + "repo": "flake-utils", + "type": "github" + } + }, + "gitignore": { + "inputs": { + "nixpkgs": [ + "lanzaboote", + "pre-commit-hooks-nix", + "nixpkgs" + ] + }, + "locked": { + "lastModified": 1709087332, + "narHash": "sha256-HG2cCnktfHsKV0s4XW83gU3F57gaTljL9KNSuG6bnQs=", + "owner": "hercules-ci", + "repo": "gitignore.nix", + "rev": "637db329424fd7e46cf4185293b9cc8c88c95394", + "type": "github" + }, + "original": { + "owner": "hercules-ci", + "repo": "gitignore.nix", + "type": "github" + } + }, + "lanzaboote": { + "inputs": { + "crane": "crane", + "flake-compat": [ + "flake-compat" + ], + "flake-parts": "flake-parts", + "flake-utils": [ + "flake-utils" + ], + "nixpkgs": [ + "nixpkgs" + ], + "pre-commit-hooks-nix": "pre-commit-hooks-nix", + "rust-overlay": "rust-overlay" + }, + "locked": { + "lastModified": 1713369831, + "narHash": "sha256-G4OGxvlIIjphpkxcRAkf1QInYsAeqbfNh6Yl1JLy2uM=", + "owner": "nix-community", + "repo": "lanzaboote", + "rev": "850f27322239f8cfa56b122cc9a278ab99a49015", + "type": "github" + }, + "original": { + "owner": "nix-community", + "repo": "lanzaboote", + "type": "github" + } + }, + "nixpkgs": { + "locked": { + "lastModified": 1719824438, + "narHash": "sha256-pY0wosAgcr9W4vmGML0T3BVhQiGuKoozCbs2t+Je1zc=", + "owner": "nixos", + "repo": "nixpkgs", + "rev": "7f993cdf26ccef564eabf31fdb40d140821e12bc", + "type": "github" + }, + "original": { + "owner": "nixos", + "ref": "nixos-unstable-small", + "repo": "nixpkgs", + "type": "github" + } + }, + "nixpkgs-stable": { + "locked": { + "lastModified": 1710695816, + "narHash": "sha256-3Eh7fhEID17pv9ZxrPwCLfqXnYP006RKzSs0JptsN84=", + "owner": "NixOS", + "repo": "nixpkgs", + "rev": "614b4613980a522ba49f0d194531beddbb7220d3", + "type": "github" + }, + "original": { + "owner": "NixOS", + "ref": "nixos-23.11", + "repo": "nixpkgs", + "type": "github" + } + }, + "pre-commit-hooks-nix": { + "inputs": { + "flake-compat": [ + "lanzaboote", + "flake-compat" + ], + "flake-utils": [ + "lanzaboote", + "flake-utils" + ], + "gitignore": "gitignore", + "nixpkgs": [ + "lanzaboote", + "nixpkgs" + ], + "nixpkgs-stable": "nixpkgs-stable" + }, + "locked": { + "lastModified": 1710923068, + "narHash": "sha256-6hOpUiuxuwpXXc/xfJsBUJeqqgGI+JMJuLo45aG3cKc=", + "owner": "cachix", + "repo": "pre-commit-hooks.nix", + "rev": "e611897ddfdde3ed3eaac4758635d7177ff78673", + "type": "github" + }, + "original": { + "owner": "cachix", + "repo": "pre-commit-hooks.nix", + "type": "github" + } + }, + "root": { + "inputs": { + "flake-compat": "flake-compat", + "flake-utils": "flake-utils", + "lanzaboote": "lanzaboote", + "nixpkgs": "nixpkgs" + } + }, + "rust-overlay": { + "inputs": { + "flake-utils": [ + "lanzaboote", + "flake-utils" + ], + "nixpkgs": [ + "lanzaboote", + "nixpkgs" + ] + }, + "locked": { + "lastModified": 1711246447, + "narHash": "sha256-g9TOluObcOEKewFo2fR4cn51Y/jSKhRRo4QZckHLop0=", + "owner": "oxalica", + "repo": "rust-overlay", + "rev": "dcc802a6ec4e9cc6a1c8c393327f0c42666f22e4", + "type": "github" + }, + "original": { + "owner": "oxalica", + "repo": "rust-overlay", + "type": "github" + } + }, + "systems": { + "locked": { + "lastModified": 1681028828, + "narHash": "sha256-Vy1rq5AaRuLzOxct8nz4T6wlgyUR7zLU309k9mBC768=", + "owner": "nix-systems", + "repo": "default", + "rev": "da67096a3b9bf56a91d16901293e51ba5b49a27e", + "type": "github" + }, + "original": { + "owner": "nix-systems", + "repo": "default", + "type": "github" + } + } + }, + "root": "root", + "version": 7 +} diff --git a/tests/functional/flakes/flake-metadata/metadata.out.expected b/tests/functional/flakes/flake-metadata/metadata.out.expected new file mode 100644 index 000000000..72e57eca7 --- /dev/null +++ b/tests/functional/flakes/flake-metadata/metadata.out.expected @@ -0,0 +1,36 @@ +Path: /nix/store/gaxb42z68bcr8lch467shvmnhjjzgd8b-source +Last modified: 1970-01-01 00:16:40 +Inputs: +├───flake-compat: github:edolstra/flake-compat/0f9255e01c2351cc7d116c072cb317785dd33b33 +│ Last modified: 2023-10-04 13:37:54 +├───flake-utils: github:numtide/flake-utils/b1d9ab70662946ef0850d488da1c9019f3a9752a +│ Last modified: 2024-03-11 08:33:50 +│ └───systems: github:nix-systems/default/da67096a3b9bf56a91d16901293e51ba5b49a27e +│ Last modified: 2023-04-09 08:27:08 +├───lanzaboote: github:nix-community/lanzaboote/850f27322239f8cfa56b122cc9a278ab99a49015 +│ Last modified: 2024-04-17 16:03:51 +│ ├───crane: github:ipetkov/crane/880573f80d09e18a11713f402b9e6172a085449f +│ │ Last modified: 2024-03-24 16:53:56 +│ │ └───nixpkgs follows input 'lanzaboote/nixpkgs' +│ ├───flake-compat follows input 'flake-compat' +│ ├───flake-parts: github:hercules-ci/flake-parts/f7b3c975cf067e56e7cda6cb098ebe3fb4d74ca2 +│ │ Last modified: 2024-03-01 23:36:56 +│ │ └───nixpkgs-lib follows input 'lanzaboote/nixpkgs' +│ ├───flake-utils follows input 'flake-utils' +│ ├───nixpkgs follows input 'nixpkgs' +│ ├───pre-commit-hooks-nix: github:cachix/pre-commit-hooks.nix/e611897ddfdde3ed3eaac4758635d7177ff78673 +│ │ Last modified: 2024-03-20 08:24:28 +│ │ ├───flake-compat follows input 'lanzaboote/flake-compat' +│ │ ├───flake-utils follows input 'lanzaboote/flake-utils' +│ │ ├───gitignore: github:hercules-ci/gitignore.nix/637db329424fd7e46cf4185293b9cc8c88c95394 +│ │ │ Last modified: 2024-02-28 02:28:52 +│ │ │ └───nixpkgs follows input 'lanzaboote/pre-commit-hooks-nix/nixpkgs' +│ │ ├───nixpkgs follows input 'lanzaboote/nixpkgs' +│ │ └───nixpkgs-stable: github:NixOS/nixpkgs/614b4613980a522ba49f0d194531beddbb7220d3 +│ │ Last modified: 2024-03-17 17:16:56 +│ └───rust-overlay: github:oxalica/rust-overlay/dcc802a6ec4e9cc6a1c8c393327f0c42666f22e4 +│ Last modified: 2024-03-24 02:14:07 +│ ├───flake-utils follows input 'lanzaboote/flake-utils' +│ └───nixpkgs follows input 'lanzaboote/nixpkgs' +└───nixpkgs: github:nixos/nixpkgs/7f993cdf26ccef564eabf31fdb40d140821e12bc + Last modified: 2024-07-01 09:00:38 diff --git a/tests/functional/meson.build b/tests/functional/meson.build index 7a9c7182f..2b5dfe422 100644 --- a/tests/functional/meson.build +++ b/tests/functional/meson.build @@ -69,8 +69,9 @@ functional_tests_scripts = [ 'flakes/unlocked-override.sh', 'flakes/absolute-paths.sh', 'flakes/build-paths.sh', - 'flakes/flake-registry.sh', 'flakes/flake-in-submodule.sh', + 'flakes/flake-metadata.sh', + 'flakes/flake-registry.sh', 'flakes/subdir-flake.sh', 'gc.sh', 'nix-collect-garbage-d.sh', -- cgit v1.2.3 From 9238e62ae6b7b8bf00d53b48e0c3ae33285dd4ab Mon Sep 17 00:00:00 2001 From: Jade Lovelace Date: Wed, 31 Jul 2024 23:04:28 -0700 Subject: flake & doxygen: update tagline This tagline was left over from CppNix and we should make it tastier. Change-Id: Ia182b86f6e751591be71a50521992ad73c7b38b5 --- doc/internal-api/doxygen.cfg.in | 2 +- flake.nix | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/doc/internal-api/doxygen.cfg.in b/doc/internal-api/doxygen.cfg.in index 55bccebd7..73fba6948 100644 --- a/doc/internal-api/doxygen.cfg.in +++ b/doc/internal-api/doxygen.cfg.in @@ -20,7 +20,7 @@ OUTPUT_DIRECTORY = @docdir@ # for a project that appears at the top of each page and should give viewer a # quick idea about the purpose of the project. Keep the description short. -PROJECT_BRIEF = "Nix, the purely functional package manager; unstable internal interfaces" +PROJECT_BRIEF = "Lix: A modern, delicious implementation of the Nix package manager; unstable internal interfaces" # If the GENERATE_LATEX tag is set to YES, doxygen will generate LaTeX output. # The default value is: YES. diff --git a/flake.nix b/flake.nix index 64d16b588..c160daed9 100644 --- a/flake.nix +++ b/flake.nix @@ -1,5 +1,5 @@ { - description = "The purely functional package manager"; + description = "Lix: A modern, delicious implementation of the Nix package manager"; inputs = { nixpkgs.url = "github:NixOS/nixpkgs/nixos-24.05-small"; -- cgit v1.2.3 From ca9d3e6e00ec452701d9d3b7e909eff61799f739 Mon Sep 17 00:00:00 2001 From: Jade Lovelace Date: Thu, 1 Aug 2024 13:42:14 -0700 Subject: tree-wide: fix various lint warnings Change-Id: I0fc80718eb7e02d84cc4b5d5deec4c0f41116134 --- src/libexpr/flake/config.cc | 2 +- src/libexpr/parser/parser.cc | 7 ++++--- src/libstore/filetransfer.cc | 13 +++++++++++-- src/libstore/machines.cc | 2 +- src/libutil/archive.cc | 2 +- src/libutil/file-descriptor.cc | 2 +- src/libutil/hash.cc | 2 +- src/libutil/shlex.cc | 2 ++ src/libutil/url.cc | 2 +- tests/unit/libutil-support/tests/cli-literate-parser.cc | 9 ++------- 10 files changed, 25 insertions(+), 18 deletions(-) diff --git a/src/libexpr/flake/config.cc b/src/libexpr/flake/config.cc index adcf7fd10..558b3e9b9 100644 --- a/src/libexpr/flake/config.cc +++ b/src/libexpr/flake/config.cc @@ -53,7 +53,7 @@ void ConfigFile::apply() bool trusted = whitelist.count(baseName); if (!trusted) { - switch (nix::fetchSettings.acceptFlakeConfig) { + switch (nix::fetchSettings.acceptFlakeConfig.get()) { case AcceptFlakeConfig::True: { trusted = true; break; diff --git a/src/libexpr/parser/parser.cc b/src/libexpr/parser/parser.cc index a00586c36..b7a105fe7 100644 --- a/src/libexpr/parser/parser.cc +++ b/src/libexpr/parser/parser.cc @@ -12,7 +12,6 @@ #include "state.hh" #include -#include #include // flip this define when doing parser development to enable some g checks. @@ -254,7 +253,8 @@ struct AttrState : SubexprState { std::vector attrs; - void pushAttr(auto && attr, PosIdx) { attrs.emplace_back(std::move(attr)); } + template + void pushAttr(T && attr, PosIdx) { attrs.emplace_back(std::forward(attr)); } }; template<> struct BuildAST { @@ -290,7 +290,8 @@ struct InheritState : SubexprState { std::unique_ptr from; PosIdx fromPos; - void pushAttr(auto && attr, PosIdx pos) { attrs.emplace_back(std::move(attr), pos); } + template + void pushAttr(T && attr, PosIdx pos) { attrs.emplace_back(std::forward(attr), pos); } }; template<> struct BuildAST { diff --git a/src/libstore/filetransfer.cc b/src/libstore/filetransfer.cc index fcb947f96..566dc65d4 100644 --- a/src/libstore/filetransfer.cc +++ b/src/libstore/filetransfer.cc @@ -477,8 +477,17 @@ struct curlFileTransfer : public FileTransfer ~curlFileTransfer() { - stopWorkerThread(); - + try { + stopWorkerThread(); + } catch (nix::Error e) { + // This can only fail if a socket to our own process cannot be + // written to, so it is always a bug in the program if it fails. + // + // Joining the thread would probably only cause a deadlock if this + // happened, so just die on purpose. + printError("failed to join curl file transfer worker thread: %1%", e.what()); + std::terminate(); + } workerThread.join(); if (curlm) curl_multi_cleanup(curlm); diff --git a/src/libstore/machines.cc b/src/libstore/machines.cc index 833482815..d0897b81f 100644 --- a/src/libstore/machines.cc +++ b/src/libstore/machines.cc @@ -33,7 +33,7 @@ Machine::Machine(decltype(storeUri) storeUri, systemTypes(systemTypes), sshKey(sshKey), maxJobs(maxJobs), - speedFactor(speedFactor == 0.0f ? 1.0f : std::move(speedFactor)), + speedFactor(speedFactor == 0.0f ? 1.0f : speedFactor), supportedFeatures(supportedFeatures), mandatoryFeatures(mandatoryFeatures), sshPublicHostKey(sshPublicHostKey) diff --git a/src/libutil/archive.cc b/src/libutil/archive.cc index 5fb33ef56..d4da18f14 100644 --- a/src/libutil/archive.cc +++ b/src/libutil/archive.cc @@ -192,7 +192,7 @@ static Generator parseObject(Source & source, const Path & path) #define EXPECT(raw, kind) \ do { \ const auto s = readString(source); \ - if (s != raw) { \ + if (s != (raw)) { \ throw badArchive("expected " kind " tag"); \ } \ co_yield MetadataString{s}; \ diff --git a/src/libutil/file-descriptor.cc b/src/libutil/file-descriptor.cc index ab69b5754..037cd5297 100644 --- a/src/libutil/file-descriptor.cc +++ b/src/libutil/file-descriptor.cc @@ -131,7 +131,7 @@ AutoCloseFD::AutoCloseFD(AutoCloseFD && that) : fd{that.fd} } -AutoCloseFD & AutoCloseFD::operator =(AutoCloseFD && that) +AutoCloseFD & AutoCloseFD::operator =(AutoCloseFD && that) noexcept(false) { close(); fd = that.fd; diff --git a/src/libutil/hash.cc b/src/libutil/hash.cc index a762dc940..0ce82f273 100644 --- a/src/libutil/hash.cc +++ b/src/libutil/hash.cc @@ -229,7 +229,7 @@ Hash::Hash(std::string_view rest, HashType type, bool isSRI) for (unsigned int n = 0; n < rest.size(); ++n) { char c = rest[rest.size() - n - 1]; - unsigned char digit; + size_t digit; for (digit = 0; digit < base32Chars.size(); ++digit) /* !!! slow */ if (base32Chars[digit] == c) break; if (digit >= 32) diff --git a/src/libutil/shlex.cc b/src/libutil/shlex.cc index 21fa0502a..b923fef65 100644 --- a/src/libutil/shlex.cc +++ b/src/libutil/shlex.cc @@ -62,6 +62,8 @@ std::vector shell_split(const std::string & input) begin = ++iterator; } break; + // no other relevant cases; silence exhaustiveness compiler warning + default: break; } } diff --git a/src/libutil/url.cc b/src/libutil/url.cc index 87146ca56..2de50dd4d 100644 --- a/src/libutil/url.cc +++ b/src/libutil/url.cc @@ -63,7 +63,7 @@ std::string percentDecode(std::string_view in) if (i + 2 >= in.size()) throw BadURL("invalid URI parameter '%s'", in); try { - decoded += std::stoul(std::string(in, i + 1, 2), 0, 16); + decoded += char8_t(std::stoul(std::string(in, i + 1, 2), 0, 16)); i += 3; } catch (...) { throw BadURL("invalid URI parameter '%s'", in); diff --git a/tests/unit/libutil-support/tests/cli-literate-parser.cc b/tests/unit/libutil-support/tests/cli-literate-parser.cc index f74fe85eb..3aa55859c 100644 --- a/tests/unit/libutil-support/tests/cli-literate-parser.cc +++ b/tests/unit/libutil-support/tests/cli-literate-parser.cc @@ -1,20 +1,16 @@ #include "cli-literate-parser.hh" #include "escape-string.hh" -#include "escape-char.hh" -#include "libexpr/print.hh" #include "types.hh" #include #include #include #include -#include #include #include #include "cli-literate-parser.hh" #include "escape-string.hh" #include "fmt.hh" -#include "libexpr/print.hh" #include "shlex.hh" #include "types.hh" #include "strings.hh" @@ -361,9 +357,8 @@ const char * ParseError::what() const noexcept return what_->c_str(); } else { auto escaped = escapeString(rest, {.maxLength = 256, .escapeNonPrinting = true}); - auto hint = - new HintFmt("Parse error: Expected %1%, got:\n%2%", expected, Uncolored(escaped)); - what_ = hint->str(); + auto hint = HintFmt("Parse error: Expected %1%, got:\n%2%", expected, Uncolored(escaped)); + what_ = hint.str(); return what_->c_str(); } } -- cgit v1.2.3