From a1baf97f5416a443a385c6f099f19c46ab6bc165 Mon Sep 17 00:00:00 2001 From: Johannes Altmanninger Date: Thu, 20 Nov 2025 10:34:17 +0100 Subject: [PATCH] Do not embed man pages in CMake builds Commit 0709e4be8b6 (Use standalone code paths by default, 2025-10-26) mainly wanted to embed internal functions to unbreak upgrade scenarios. Embedding man pages in CMake has small-ish disadvantages: 1. extra space usage 2. less discoverability 3. a "cyclic" dependency: 1. "sphinx-docs" depends on "fish_indent" 2. "fish_indent" via "crates/build-man-pages" depends on "doc_src/". So every "touch doc_src/foo.rst && ninja -Cbuild sphinx-docs" re-builds fish, just to re-run sphinx-build. The significant one is number 3. It can be worked around by running sphinx-build with stale "fish_indent" but I don't think we want to do that. Let's backtrack a little by stopping embedding man pages in CMake builds; use the on-disk man pages (which we still install). The remaining "regression" from 0709e4be8b6 is that "ninja -Cbuild fish" needs to rebuild whenever anything in "share/" changes. I don't know if that's also annoying? Since man pages for "build/fish" are not in share/, expose the exact path as $__fish_man_dir. --- Cargo.toml | 5 ++- README.rst | 3 +- share/functions/__fish_complete_man.fish | 3 +- .../functions/__fish_data_with_directory.fish | 4 +- share/functions/__fish_man1_pages.fish | 10 +++++ share/functions/__fish_print_commands.fish | 4 +- .../__fish_tried_to_embed_manpages.fish | 4 ++ share/functions/man.fish | 19 ++++----- src/builtins/status.rs | 9 +++- src/env/config_paths.rs | 42 ++++++++++++------- src/env/environment.rs | 2 + .../__fish_tried_to_embed_manpages.fish | 13 ++++++ tests/checks/config-paths.fish | 1 + tests/checks/man.fish | 6 +-- 14 files changed, 86 insertions(+), 39 deletions(-) create mode 100644 share/functions/__fish_man1_pages.fish create mode 100644 share/functions/__fish_tried_to_embed_manpages.fish create mode 100644 tests/checks/__fish_tried_to_embed_manpages.fish diff --git a/Cargo.toml b/Cargo.toml index 189f786c6..b54fd64df 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -86,7 +86,7 @@ bitflags.workspace = true cfg-if.workspace = true errno.workspace = true fish-build-helper.workspace = true -fish-build-man-pages.workspace = true +fish-build-man-pages = { workspace = true, optional = true } fish-gettext-extraction = { workspace = true, optional = true } fish-gettext-maps = { workspace = true, optional = true } fish-printf.workspace = true @@ -150,8 +150,9 @@ name = "fish_key_reader" path = "src/bin/fish_key_reader.rs" [features] -default = ["localize-messages"] +default = ["embed-manpages", "localize-messages"] benchmark = [] +embed-manpages = ["dep:fish-build-man-pages"] # Enable gettext localization at runtime. Requires the `msgfmt` tool to generate catalog data at # build time. localize-messages = ["dep:phf", "dep:fish-gettext-maps"] diff --git a/README.rst b/README.rst index 872626fc6..fe441f03f 100644 --- a/README.rst +++ b/README.rst @@ -184,13 +184,14 @@ You can also install Sphinx another way and drop the ``uv run --no-managed-pytho This will place standalone binaries in ``~/.cargo/bin/``, but you can move them wherever you want. -To disable translations, disable the ``localize-messages`` feature by passing ``--no-default-features`` to cargo. +To disable translations, disable the ``localize-messages`` feature by passing ``--no-default-features --features=embed-manpages`` to cargo. You can also link this build statically (but not against glibc) and move it to other computers. Here are the remaining advantages of a full installation, as currently done by CMake: - Man pages like ``fish(1)`` installed in standard locations, easily accessible from outside fish. +- Separate files for builtins (e.g. ``$PREFIX/share/fish/man/man1/abbr.1``). - A local copy of the HTML documentation, typically accessed via the ``help`` fish function. In Cargo builds, ``help`` will redirect to ``__ - Ability to use our CMake options extra_functionsdir, extra_completionsdir and extra_confdir, diff --git a/share/functions/__fish_complete_man.fish b/share/functions/__fish_complete_man.fish index 2bbcfc4f3..aaf20e8ad 100644 --- a/share/functions/__fish_complete_man.fish +++ b/share/functions/__fish_complete_man.fish @@ -70,8 +70,7 @@ function __fish_complete_man # Fish commands are not given by apropos if not set -ql exclude_fish_commands - __fish_data_list_files man/man1 | - string replace -rf '.*/([^/]+)\.1(\.gz)?$' '$1\t1: fish command' + string join \n -- (__fish_man1_pages)\t'1: fish command' end else return 1 diff --git a/share/functions/__fish_data_with_directory.fish b/share/functions/__fish_data_with_directory.fish index e1bae9e3f..e8b50afbc 100644 --- a/share/functions/__fish_data_with_directory.fish +++ b/share/functions/__fish_data_with_directory.fish @@ -5,8 +5,8 @@ function __fish_data_with_directory set -l cmd $argv[3..] set -l temp set -l directory_ref - if false - set directory_ref $__fish_data_dir/$relative_directory + if test $relative_directory = man/man1 && not __fish_tried_to_embed_manpages + set directory_ref $__fish_man_dir/man1 else set temp (__fish_mktemp_relative -d fish-data) or return diff --git a/share/functions/__fish_man1_pages.fish b/share/functions/__fish_man1_pages.fish new file mode 100644 index 000000000..4f0d543d8 --- /dev/null +++ b/share/functions/__fish_man1_pages.fish @@ -0,0 +1,10 @@ +# localization: skip(private) +function __fish_man1_pages + if __fish_tried_to_embed_manpages + status list-files man/man1 + else + files=$__fish_man_dir/man1/* string join -- \n $files + end | + path basename | + string replace -r -- '.1(.gz)?$' '' +end diff --git a/share/functions/__fish_print_commands.fish b/share/functions/__fish_print_commands.fish index 319c112be..99fc1ad5e 100644 --- a/share/functions/__fish_print_commands.fish +++ b/share/functions/__fish_print_commands.fish @@ -1,7 +1,5 @@ # localization: skip(private) function __fish_print_commands --description "Print a list of documented fish commands" - __fish_data_list_files man/man1 | - string replace -r '.*/' '' | - string replace -r '.1(.gz)?$' '' | + __fish_man1_pages | string match -rv '^fish-(?:changelog|completions|doc|tutorial|faq|for-bash-users|interactive|language|releasenotes|terminal-compatibility)$' end diff --git a/share/functions/__fish_tried_to_embed_manpages.fish b/share/functions/__fish_tried_to_embed_manpages.fish new file mode 100644 index 000000000..7adbe663e --- /dev/null +++ b/share/functions/__fish_tried_to_embed_manpages.fish @@ -0,0 +1,4 @@ +# localization: skip(private) +function __fish_tried_to_embed_manpages + status build-info | string match -rq '(\s)embed-manpages(\s|$)' +end diff --git a/share/functions/man.fish b/share/functions/man.fish index 5fbce4fa8..848a8c33b 100644 --- a/share/functions/man.fish +++ b/share/functions/man.fish @@ -6,17 +6,16 @@ if not command -qs man end function man - # If we have an embedded page, reuse a function that happens to do the - # right thing. - if not set -q argv[2] && - status list-files "man/man1/$(__fish_canonicalize_builtin $argv).1" &>/dev/null - __fish_print_help $argv[1] - return - end - set -l manpath - if false - and set -l fish_manpath (path filter -d $__fish_data_dir/man) + if __fish_tried_to_embed_manpages + # If we have an embedded page, reuse a function that happens to do + # the right thing. + if not set -q argv[2] && + status list-files "man/man1/$(__fish_canonicalize_builtin $argv).1" &>/dev/null + __fish_print_help $argv[1] + return + end + else if set -l fish_manpath (path filter -d $__fish_man_dir) # Prepend fish's man directory if available. # Work around the "builtin" manpage that everything symlinks to, diff --git a/src/builtins/status.rs b/src/builtins/status.rs index 0cea59d5d..e4152ecdc 100644 --- a/src/builtins/status.rs +++ b/src/builtins/status.rs @@ -337,11 +337,16 @@ fn parse_cmd_opts( #[folder = "user_doc/man/man1"] #[prefix = "man/man1/"] struct Docs; - } else { + } else if #[cfg(feature = "embed-manpages")] { #[derive(RustEmbed)] #[folder = "$FISH_RESOLVED_BUILD_DIR/fish-man/man1"] #[prefix = "man/man1/"] struct Docs; + } else { + #[derive(RustEmbed)] + #[folder = "$FISH_RESOLVED_BUILD_DIR"] + #[include = ""] + struct Docs; } ); @@ -601,6 +606,8 @@ pub fn status(parser: &Parser, streams: &mut IoStreams, args: &mut [&wstr]) -> B streams.out.appendln(profile); streams.out.append(L!("Features: ")); let features: &[&str] = &[ + #[cfg(feature = "embed-manpages")] + "embed-manpages", #[cfg(feature = "localize-messages")] "localize-messages", #[cfg(target_feature = "crt-static")] diff --git a/src/env/config_paths.rs b/src/env/config_paths.rs index 5d4227f10..3beb0a9f9 100644 --- a/src/env/config_paths.rs +++ b/src/env/config_paths.rs @@ -12,6 +12,7 @@ pub struct ConfigPaths { pub sysconf: PathBuf, // e.g., /usr/local/etc pub bin: Option, // e.g., /usr/local/bin pub data: Option, // e.g., /usr/local/share + pub man: Option, // e.g., /usr/local/share/fish/man pub doc: Option, // e.g., /usr/local/share/doc/fish } @@ -50,19 +51,24 @@ macro_rules! log_optional_path { } log_optional_path!(bin); log_optional_path!(data); + log_optional_path!(man); log_optional_path!(doc); paths } fn from_exec_path(unresolved_exec_path: &'static FishPath) -> Self { - let default_layout = |exec_path_parent: Option<&Path>| Self { - sysconf: PathBuf::from(SYSCONF_DIR).join("fish"), - bin: option_env!("BINDIR") - .map(PathBuf::from) - // N.B. the argument may be non-canonical here. - .or_else(|| exec_path_parent.map(|p| p.to_owned())), - data: option_env!("DATADIR").map(|p| PathBuf::from(p).join("fish")), - doc: option_env!("DOCDIR").map(PathBuf::from), + let default_layout = |exec_path_parent: Option<&Path>| { + let data = option_env!("DATADIR").map(|p| PathBuf::from(p).join("fish")); + Self { + sysconf: PathBuf::from(SYSCONF_DIR).join("fish"), + bin: option_env!("BINDIR") + .map(PathBuf::from) + // N.B. the argument may be non-canonical here. + .or_else(|| exec_path_parent.map(|p| p.to_owned())), + data: data.clone(), + man: data.map(|data| data.join("man")), + doc: option_env!("DOCDIR").map(PathBuf::from), + } }; let exec_path = { @@ -113,10 +119,12 @@ fn from_exec_path(unresolved_exec_path: &'static FishPath) -> Self { } { FLOG!(config, "Running from relocatable tree"); let prefix = exec_path_parent.parent().unwrap(); + let data = prefix.join("share/fish"); Self { sysconf: prefix.join("etc/fish"), bin: Some(exec_path_parent.to_owned()), - data: Some(prefix.join("share/fish")), + data: Some(data.clone()), + man: Some(data.join("man")), doc: Some(prefix.join("share/doc/fish")), } } else if exec_path.starts_with(BUILD_DIR) { @@ -127,17 +135,21 @@ fn from_exec_path(unresolved_exec_path: &'static FishPath) -> Self { workspace_root.display() ), ); + let user_doc_join = |dir| { + if cfg!(use_prebuilt_docs) { + Some(workspace_root) + } else { + cfg!(using_cmake).then_some(Path::new(BUILD_DIR)) + } + .map(|path| path.join("user_doc").join(dir)) + }; // If we're in Cargo's target directory or in CMake's build directory, use the source files. Self { sysconf: workspace_root.join("etc"), bin: Some(exec_path_parent.to_owned()), data: Some(workspace_root.join("share")), - doc: if cfg!(use_prebuilt_docs) { - Some(workspace_root) - } else { - cfg!(using_cmake).then_some(Path::new(BUILD_DIR)) - } - .map(|p| p.join("user_doc/html")), + man: user_doc_join("man"), + doc: user_doc_join("html"), } } else { FLOG!( diff --git a/src/env/environment.rs b/src/env/environment.rs index 52c4b92d2..61a705972 100644 --- a/src/env/environment.rs +++ b/src/env/environment.rs @@ -418,6 +418,7 @@ fn get_pwd_slash(&self) -> WString { const FISH_DATADIR_VAR: &wstr = L!("__fish_data_dir"); const FISH_SYSCONFDIR_VAR: &wstr = L!("__fish_sysconf_dir"); const FISH_HELPDIR_VAR: &wstr = L!("__fish_help_dir"); +const FISH_MANDIR_VAR: &wstr = L!("__fish_man_dir"); const FISH_BIN_DIR: &wstr = L!("__fish_bin_dir"); const FISH_CONFIG_DIR: &wstr = L!("__fish_config_dir"); const FISH_USER_DATA_DIR: &wstr = L!("__fish_user_data_dir"); @@ -647,6 +648,7 @@ pub fn env_init(paths: Option<&ConfigPaths>, do_uvars: bool, default_paths: bool set_path(FISH_BIN_DIR, paths.bin.as_ref()); set_path(FISH_DATADIR_VAR, paths.data.as_ref()); + set_path(FISH_MANDIR_VAR, paths.man.as_ref()); set_path(FISH_HELPDIR_VAR, paths.doc.as_ref()); } diff --git a/tests/checks/__fish_tried_to_embed_manpages.fish b/tests/checks/__fish_tried_to_embed_manpages.fish new file mode 100644 index 000000000..dad154c3a --- /dev/null +++ b/tests/checks/__fish_tried_to_embed_manpages.fish @@ -0,0 +1,13 @@ +# RUN: %fish %s + +set -l value (string join , \ + embed-manpages="$(__fish_tried_to_embed_manpages && echo true || echo false)" \ + "build-system=$(status build-info | string replace -rf 'Build system: (.*)' '$1')" +) +switch $value + case embed-manpages=true,build-system=Cargo + case embed-manpages=false,build-system=CMake + case '*' + echo Expected embedded man pages to be detected as true iff using Cargo + echo got: $value +end diff --git a/tests/checks/config-paths.fish b/tests/checks/config-paths.fish index 8de52c08e..9eb6a4516 100644 --- a/tests/checks/config-paths.fish +++ b/tests/checks/config-paths.fish @@ -11,6 +11,7 @@ # NOTE: When our executable is located outside the build directory, these are different. # CHECKERR: config: paths.data: {{.*}}/share +# CHECKERR: config: paths.man: {{.*/user_doc/man|\|not found\|}} # CHECKERR: config: paths.doc: {{.*/user_doc/html|\|not found\|}} # CHECKERR: config: sourcing {{.+}}/etc/config.fish diff --git a/tests/checks/man.fish b/tests/checks/man.fish index 673ae5ec8..af65d72cf 100644 --- a/tests/checks/man.fish +++ b/tests/checks/man.fish @@ -1,12 +1,12 @@ # RUN: %fish %s -# Override the test-override again. -__fish_data_with_file functions/__fish_print_help.fish source - # REQUIRES: command -v sphinx-build # REQUIRES: command -v man # REQUIRES: test "$FISH_BUILD_DOCS" != "0" +# Override the test-override again. +__fish_data_with_file functions/__fish_print_help.fish source + set -l deroff col -b -p -x set -lx MANWIDTH 80