config_paths: model embed-data dichotomy better

Rather than having every single config path be an Option<Path>,
clarify that we define either all or nothing.

If we want to decide this at runtime, we'd use an enum; but it's all
known at compile time, so we can give the reader even more information.

Unfortunately this means that compile errors in non-embed-data
code paths might go unnoticed for a while, and rust-analyzer often
doesn't work in those code paths. But that's a general problem with
the compile-time feature, it seems orthodox to ifdef away as much
as possible.

There are some odd data paths that don't follow the "all or nothing",
the next commits will fix this.

Note that this breaks localization for target/debug/fish built with
embed-data. But as soon as fish was moved out of the repo, that was
already broken.
This commit is contained in:
Johannes Altmanninger
2025-09-06 13:30:04 +02:00
parent cacb9f50b8
commit f7a2d56046
4 changed files with 54 additions and 51 deletions

View File

@@ -186,14 +186,7 @@ fn read_init(parser: &Parser, paths: &ConfigPaths) {
}
#[cfg(not(feature = "embed-data"))]
{
let datapath = str2wcstring(
paths
.data
.as_ref()
.expect("Non-embed build not having a data path. That's a bug")
.as_os_str()
.as_bytes(),
);
let datapath = str2wcstring(paths.data.as_os_str().as_bytes());
if !source_config_in_directory(parser, &datapath) {
// If we cannot read share/config.fish, our internal configuration,
// something is wrong.

View File

@@ -7,11 +7,13 @@
/// env_init.
#[derive(Default)]
pub struct ConfigPaths {
pub data: Option<PathBuf>, // e.g., /usr/local/share
pub sysconf: PathBuf, // e.g., /usr/local/etc
pub doc: PathBuf, // e.g., /usr/local/share/doc/fish
pub bin: Option<PathBuf>, // e.g., /usr/local/bin
pub locale: Option<PathBuf>, // e.g., /usr/local/share/locale
#[cfg(not(feature = "embed-data"))]
pub data: PathBuf, // e.g., /usr/local/share
pub sysconf: PathBuf, // e.g., /usr/local/etc
pub doc: PathBuf, // e.g., /usr/local/share/doc/fish
pub bin: Option<PathBuf>, // e.g., /usr/local/bin
#[cfg(not(feature = "embed-data"))]
pub locale: PathBuf, // e.g., /usr/local/share/locale
}
pub static CONFIG_PATHS: Lazy<ConfigPaths> = Lazy::new(|| {
@@ -29,27 +31,28 @@ pub struct ConfigPaths {
let paths = ConfigPaths::new(&argv0, exec_path);
#[cfg(not(feature = "embed-data"))]
FLOGF!(config, "paths.data: %ls", paths.data.display().to_string());
FLOGF!(
config,
"determine_config_directory_paths() results:\npaths.data: %ls\npaths.sysconf: \
%ls\npaths.doc: %ls\npaths.bin: %ls\npaths.locale: %ls",
paths
.data
.clone()
.map(|x| x.display().to_string())
.unwrap_or("|not found|".to_string()),
paths.sysconf.display().to_string(),
paths.doc.display().to_string(),
"paths.sysconf: %ls",
paths.sysconf.display().to_string()
);
FLOGF!(config, "paths.doc: %ls", paths.doc.display().to_string());
FLOGF!(
config,
"paths.bin: %ls",
paths
.bin
.clone()
.map(|x| x.display().to_string())
.unwrap_or("|not found|".to_string()),
paths
.locale
.clone()
.map(|x| x.display().to_string())
.unwrap_or("|not found|".to_string()),
);
#[cfg(not(feature = "embed-data"))]
FLOGF!(
config,
"paths.locale: %ls",
paths.locale.display().to_string()
);
paths
@@ -64,11 +67,11 @@ fn static_paths() -> Self {
// Fall back to what got compiled in.
FLOG!(config, "Using compiled in paths:");
Self {
data: Some(PathBuf::from(env!("DATADIR")).join("fish")),
data: PathBuf::from(env!("DATADIR")).join("fish"),
sysconf: PathBuf::from(SYSCONF_DIR).join("fish"),
doc: DOC_DIR.into(),
bin: Some(PathBuf::from(env!("BINDIR"))),
locale: Some(PathBuf::from(env!("LOCALEDIR"))),
locale: PathBuf::from(env!("LOCALEDIR")),
}
}
@@ -76,11 +79,9 @@ fn static_paths() -> Self {
fn new(_argv0: &Path, exec_path: PathBuf) -> Self {
FLOG!(config, "embed-data feature is active, ignoring data paths");
ConfigPaths {
data: None,
sysconf: PathBuf::from(SYSCONF_DIR).join("fish"),
doc: DOC_DIR.into(),
bin: exec_path.parent().map(|x| x.to_path_buf()),
locale: None,
}
}
@@ -104,11 +105,11 @@ fn new(argv0: &Path, exec_path: PathBuf) -> Self {
workspace_root.display()
);
return ConfigPaths {
data: Some(workspace_root.join("share")),
data: workspace_root.join("share"),
sysconf: workspace_root.join("etc"),
doc: workspace_root.join("user_doc/html"),
bin: Some(exec_path.parent().unwrap().to_owned()),
locale: Some(workspace_root.join("share/locale")),
locale: workspace_root.join("share/locale"),
};
}
@@ -117,15 +118,15 @@ fn new(argv0: &Path, exec_path: PathBuf) -> Self {
if exec_path.ends_with("bin/fish") {
let base_path = exec_path.parent().unwrap().parent().unwrap();
paths = ConfigPaths {
data: Some(base_path.join("share/fish")),
data: base_path.join("share/fish"),
sysconf: base_path.join("etc/fish"),
doc: base_path.join("share/doc/fish"),
bin: Some(base_path.join("bin")),
locale: Some(base_path.join("share/locale")),
locale: base_path.join("share/locale"),
}
}
if paths.data.clone().is_some_and(|x| x.exists()) && paths.sysconf.exists() {
if paths.data.exists() && paths.sysconf.exists() {
// The docs dir may not exist; in that case fall back to the compiled in path.
if !paths.doc.exists() {
paths.doc = PathBuf::from(DOC_DIR);

View File

@@ -646,8 +646,11 @@ pub fn env_init(paths: Option<&ConfigPaths>, do_uvars: bool, default_paths: bool
setup_user(vars);
if let Some(paths) = paths {
if let Some(ddir) = &paths.data {
let datadir = str2wcstring(ddir.as_os_str().as_bytes());
#[cfg(feature = "embed-data")]
vars.set_empty(FISH_DATADIR_VAR, EnvMode::GLOBAL);
#[cfg(not(feature = "embed-data"))]
{
let datadir = str2wcstring(paths.data.as_os_str().as_bytes());
vars.set_one(FISH_DATADIR_VAR, EnvMode::GLOBAL, datadir.clone());
if default_paths {
@@ -657,8 +660,6 @@ pub fn env_init(paths: Option<&ConfigPaths>, do_uvars: bool, default_paths: bool
scstr.push_str("/functions");
vars.set_one(L!("fish_function_path"), EnvMode::GLOBAL, scstr);
}
} else {
vars.set_empty(FISH_DATADIR_VAR, EnvMode::GLOBAL);
}
vars.set_one(

View File

@@ -1,10 +1,7 @@
use std::collections::HashMap;
use std::ffi::CString;
use std::os::unix::ffi::OsStrExt;
use std::sync::Mutex;
use crate::common::{charptr2wcstring, wcs2zstring, PACKAGE_NAME};
use crate::env::CONFIG_PATHS;
use crate::common::{charptr2wcstring, wcs2zstring};
#[cfg(test)]
use crate::tests::prelude::*;
use crate::wchar::prelude::*;
@@ -17,15 +14,19 @@ mod internal {
use std::ffi::CStr;
extern "C" {
fn gettext(msgid: *const c_char) -> *mut c_char;
#[cfg(not(feature = "embed-data"))]
fn bindtextdomain(domainname: *const c_char, dirname: *const c_char) -> *mut c_char;
#[cfg(not(feature = "embed-data"))]
fn textdomain(domainname: *const c_char) -> *mut c_char;
}
pub fn fish_gettext(msgid: &CStr) -> *const c_char {
unsafe { gettext(msgid.as_ptr()) }
}
#[cfg(not(feature = "embed-data"))]
pub fn fish_bindtextdomain(domainname: &CStr, dirname: &CStr) -> *mut c_char {
unsafe { bindtextdomain(domainname.as_ptr(), dirname.as_ptr()) }
}
#[cfg(not(feature = "embed-data"))]
pub fn fish_textdomain(domainname: &CStr) -> *mut c_char {
unsafe { textdomain(domainname.as_ptr()) }
}
@@ -37,9 +38,11 @@ mod internal {
pub fn fish_gettext(msgid: &CStr) -> *const c_char {
msgid.as_ptr()
}
#[cfg(not(feature = "embed-data"))]
pub fn fish_bindtextdomain(_domainname: &CStr, _dirname: &CStr) -> *mut c_char {
std::ptr::null_mut()
}
#[cfg(not(feature = "embed-data"))]
pub fn fish_textdomain(_domainname: &CStr) -> *mut c_char {
std::ptr::null_mut()
}
@@ -49,13 +52,18 @@ pub fn fish_textdomain(_domainname: &CStr) -> *mut c_char {
// Really init wgettext.
fn wgettext_really_init() {
let Some(ref localepath) = CONFIG_PATHS.locale else {
return;
};
let package_name = CString::new(PACKAGE_NAME).unwrap();
let localedir = CString::new(localepath.as_os_str().as_bytes()).unwrap();
fish_bindtextdomain(&package_name, &localedir);
fish_textdomain(&package_name);
#[cfg(not(feature = "embed-data"))]
{
use crate::common::PACKAGE_NAME;
use crate::env::CONFIG_PATHS;
use std::ffi::CString;
use std::os::unix::ffi::OsStrExt;
let package_name = CString::new(PACKAGE_NAME).unwrap();
let localedir = CString::new(CONFIG_PATHS.locale.as_os_str().as_bytes()).unwrap();
fish_bindtextdomain(&package_name, &localedir);
fish_textdomain(&package_name);
}
}
fn wgettext_init_if_necessary() {