Skip to content

Commit

Permalink
Consider locales encoding (#992)
Browse files Browse the repository at this point in the history
## Problem

See #987.

## Solution

* The encoding should be included in the locale, although at this point
we always use `UTF-8` (check `localectl list-locales`).
* I adapted the web UI to ignore the encoding (it is only interested in
the language).

## Updating the spec file

I updated the `spec` file according to the latest changes of
cargo_vendor:

```
* cargo_config is no longer created - it's part of the vendor.tar now
    * You can safely remove lines related to cargo_config from your spec file

* multiple cargotoml files can be specified and share a single vendor.tar
    * If multiple cargo.toml files are present update does not work. This is a known
      limitation of the process

* cargo_audit is now part of cargo_vendor, meaning you don't have to configure it separately
```

## Testing

* Manually tested using a branch in OBS.
  • Loading branch information
imobachgs authored Jan 16, 2024
2 parents def9a31 + f153650 commit df72d49
Show file tree
Hide file tree
Showing 12 changed files with 62 additions and 30 deletions.
4 changes: 1 addition & 3 deletions rust/agama-dbus-server/src/l10n/helpers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,7 @@ pub fn init_locale() -> Result<LocaleCode, Box<dyn std::error::Error>> {
/// Sets the service locale.
///
pub fn set_service_locale(locale: &LocaleCode) {
// Let's force the encoding to be 'UTF-8'.
let locale = format!("{}.UTF-8", locale);
if setlocale(LocaleCategory::LcAll, locale).is_none() {
if setlocale(LocaleCategory::LcAll, locale.to_string()).is_none() {
log::warn!("Could not set the locale");
}
}
20 changes: 17 additions & 3 deletions rust/agama-locale-data/src/locale.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,12 +11,16 @@ pub struct LocaleCode {
pub language: String,
// ISO-3166
pub territory: String,
// encoding: String,
pub encoding: String,
}

impl Display for LocaleCode {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
write!(f, "{}_{}", &self.language, &self.territory)
write!(
f,
"{}_{}.{}",
&self.language, &self.territory, &self.encoding
)
}
}

Expand All @@ -25,6 +29,7 @@ impl Default for LocaleCode {
Self {
language: "en".to_string(),
territory: "US".to_string(),
encoding: "UTF-8".to_string(),
}
}
}
Expand All @@ -37,14 +42,23 @@ impl TryFrom<&str> for LocaleCode {
type Error = InvalidLocaleCode;

fn try_from(value: &str) -> Result<Self, Self::Error> {
let locale_regexp: Regex = Regex::new(r"^([[:alpha:]]+)_([[:alpha:]]+)").unwrap();
let locale_regexp: Regex =
Regex::new(r"^([[:alpha:]]+)_([[:alpha:]]+)(?:\.(.+))?").unwrap();

let captures = locale_regexp
.captures(value)
.ok_or_else(|| InvalidLocaleCode(value.to_string()))?;

let encoding = captures
.get(3)
.map(|e| e.as_str())
.unwrap_or("UTF-8")
.to_string();

Ok(Self {
language: captures.get(1).unwrap().as_str().to_string(),
territory: captures.get(2).unwrap().as_str().to_string(),
encoding,
})
}
}
Expand Down
5 changes: 5 additions & 0 deletions rust/package/agama-cli.changes
Original file line number Diff line number Diff line change
@@ -1,3 +1,8 @@
-------------------------------------------------------------------
Thu Jan 11 15:34:15 UTC 2024 - Imobach Gonzalez Sosa <[email protected]>

- Include the encoding as part of the locales (gh#openSUSE/agama#987).

-------------------------------------------------------------------
Mon Jan 8 17:02:40 UTC 2024 - José Iván López González <[email protected]>

Expand Down
4 changes: 0 additions & 4 deletions rust/package/agama-cli.spec
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,6 @@ License: GPL-2.0-only
Url: https://github.com/opensuse/agama
Source0: agama.tar
Source1: vendor.tar.zst
# Generated by the cargo_vendor OBS service
Source2: cargo_config
BuildRequires: cargo-packaging
BuildRequires: pkgconfig(openssl)
# used in tests for dbus service
Expand Down Expand Up @@ -64,8 +62,6 @@ DBus service for agama project. It provides so far localization service.

%prep
%autosetup -a1 -n agama
mkdir .cargo
cp %{SOURCE2} .cargo/config
# Remove exec bits to prevent an issue in fedora shebang checking. Uncomment only if required.
# find vendor -type f -name \*.rs -exec chmod -x '{}' \;

Expand Down
6 changes: 4 additions & 2 deletions service/lib/agama/dbus/software_service.rb
Original file line number Diff line number Diff line change
Expand Up @@ -98,13 +98,15 @@ def dbus_objects
# Language change callback handler, activate new locale in the libzypp backend
# @param locale [String] the new locale
def locale_handler(locale)
language, = locale.split(".")

# set the locale in the Language module, when changing the repository
# (product) it calls Pkg.SetTextLocale(Language.language) internally
Yast::Language.Set(locale)
Yast::Language.Set(language)

# set libzypp locale (for communication only, Pkg.SetPackageLocale
# call can be used for *installing* the language packages)
Yast::Pkg.SetTextLocale(locale)
Yast::Pkg.SetTextLocale(language)

# refresh all enabled repositories to download the missing translation files
Yast::Pkg.SourceGetCurrent(true).each do |src|
Expand Down
8 changes: 4 additions & 4 deletions service/lib/agama/ui_locale.rb
Original file line number Diff line number Diff line change
Expand Up @@ -45,11 +45,11 @@ def initialize(locale_client, &block)
private

def change_locale(locale)
# TODO: check if we can use UTF-8 everywhere including strange arch consoles
Yast::WFM.SetLanguage(locale, "UTF-8")
language, encoding = locale.split(".")
Yast::WFM.SetLanguage(language, encoding)
# explicitly set ENV to get localization also from libraries like libstorage
ENV["LANG"] = locale + ".UTF-8"
log.info "set yast language to #{locale}"
ENV["LANG"] = locale
log.info "set yast locale to #{locale}"
# explicit call to textdomain to force fast gettext change of language ASAP
textdomain "installation"
end
Expand Down
6 changes: 6 additions & 0 deletions service/package/rubygem-agama.changes
Original file line number Diff line number Diff line change
@@ -1,3 +1,9 @@
-------------------------------------------------------------------
Thu Jan 11 15:32:44 UTC 2024 - Imobach Gonzalez Sosa <[email protected]>

- Handle the encoding included in the UILocale D-Bus property
(gh#openSUSE/agama#987).

-------------------------------------------------------------------
Thu Jan 11 12:08:29 UTC 2024 - Ladislav Slezák <[email protected]>

Expand Down
3 changes: 2 additions & 1 deletion service/test/agama/dbus/manager_service_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,8 @@
let(:users_obj) { instance_double(Agama::DBus::Users, path: "/org/opensuse/Agama/Users1") }

let(:locale_client) do
instance_double(Agama::DBus::Clients::Locale, ui_locale: "en_US", on_ui_locale_change: nil)
instance_double(Agama::DBus::Clients::Locale, ui_locale: "en_US.UTF-8",
on_ui_locale_change: nil)
end

before do
Expand Down
6 changes: 6 additions & 0 deletions web/package/cockpit-agama.changes
Original file line number Diff line number Diff line change
@@ -1,3 +1,9 @@
-------------------------------------------------------------------
Thu Jan 11 15:34:26 UTC 2024 - Imobach Gonzalez Sosa <[email protected]>

- Ignore the encoding from the UILocale D-Bus property
(gh#openSUSE/agama#987).

-------------------------------------------------------------------
Mon Jan 8 15:55:39 UTC 2024 - David Diaz <[email protected]>

Expand Down
2 changes: 1 addition & 1 deletion web/src/client/l10n.js
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ const LOCALE_PATH = "/org/opensuse/Agama1/Locale";

/**
* @typedef {object} Locale
* @property {string} id - Language id (e.g., "en_US").
* @property {string} id - Language id (e.g., "en_US.UTF-8").
* @property {string} name - Language name (e.g., "English").
* @property {string} territory - Territory name (e.g., "United States").
*/
Expand Down
8 changes: 6 additions & 2 deletions web/src/context/installerL10n.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -109,12 +109,15 @@ function languageFromQuery() {
* @see https://www.rfc-editor.org/info/bcp78
*/
function languageFromLocale(locale) {
return locale.replace("_", "-").toLowerCase();
const [language] = locale.split(".");
return language.replace("_", "-").toLowerCase();
}

/**
* Converts a RFC 5646 language tag to a locale.
*
* It forces the encoding to "UTF-8".
*
* @param {string} language
* @return {string}
*
Expand All @@ -124,7 +127,8 @@ function languageFromLocale(locale) {
*/
function languageToLocale(language) {
const [lang, country] = language.split("-");
return (country) ? `${lang}_${country.toUpperCase()}` : lang;
const locale = (country) ? `${lang}_${country.toUpperCase()}` : lang;
return `${locale}.UTF-8`;
}

/**
Expand Down
20 changes: 10 additions & 10 deletions web/src/context/installerL10n.test.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ describe("InstallerL10nProvider", () => {
describe("when the Cockpit language is already set", () => {
beforeEach(() => {
document.cookie = "CockpitLang=en-us; path=/;";
getUILocaleFn.mockResolvedValueOnce("en_US");
getUILocaleFn.mockResolvedValueOnce("en_US.UTF-8");
});

it("displays the children content and does not reload", async () => {
Expand All @@ -116,8 +116,8 @@ describe("InstallerL10nProvider", () => {
describe("when the Cockpit language is set to an unsupported language", () => {
beforeEach(() => {
document.cookie = "CockpitLang=de-de; path=/;";
getUILocaleFn.mockResolvedValueOnce("de_DE");
getUILocaleFn.mockResolvedValueOnce("es_ES");
getUILocaleFn.mockResolvedValueOnce("de_DE.UTF-8");
getUILocaleFn.mockResolvedValueOnce("es_ES.UTF-8");
});

it("uses the first supported language from the browser", async () => {
Expand All @@ -137,7 +137,7 @@ describe("InstallerL10nProvider", () => {
);

await waitFor(() => screen.getByText("hola"));
expect(setUILocaleFn).toHaveBeenCalledWith("es_ES");
expect(setUILocaleFn).toHaveBeenCalledWith("es_ES.UTF-8");
});
});

Expand All @@ -146,7 +146,7 @@ describe("InstallerL10nProvider", () => {
// Ensure both, UI and backend mock languages, are in sync since
// client.setUILocale is mocked too.
// See navigator.language in the beforeAll at the top of the file.
getUILocaleFn.mockResolvedValue("es_ES");
getUILocaleFn.mockResolvedValue("es_ES.UTF-8");
});

it("sets the preferred language from browser and reloads", async () => {
Expand Down Expand Up @@ -201,7 +201,7 @@ describe("InstallerL10nProvider", () => {
describe("when the Cockpit language is already set to 'cs-cz'", () => {
beforeEach(() => {
document.cookie = "CockpitLang=cs-cz; path=/;";
getUILocaleFn.mockResolvedValueOnce("cs_CZ");
getUILocaleFn.mockResolvedValueOnce("cs_CZ.UTF-8");
});

it("displays the children content and does not reload", async () => {
Expand Down Expand Up @@ -246,14 +246,14 @@ describe("InstallerL10nProvider", () => {
);

await waitFor(() => screen.getByText("ahoj"));
expect(setUILocaleFn).toHaveBeenCalledWith("cs_CZ");
expect(setUILocaleFn).toHaveBeenCalledWith("cs_CZ.UTF-8");
});
});

describe("when the Cockpit language is not set", () => {
beforeEach(() => {
getUILocaleFn.mockResolvedValueOnce("en_US");
getUILocaleFn.mockResolvedValueOnce("cs_CZ");
getUILocaleFn.mockResolvedValueOnce("en_US.UTF-8");
getUILocaleFn.mockResolvedValueOnce("cs_CZ.UTF-8");
setUILocaleFn.mockResolvedValue();
});

Expand All @@ -274,7 +274,7 @@ describe("InstallerL10nProvider", () => {
);

await waitFor(() => screen.getByText("ahoj"));
expect(setUILocaleFn).toHaveBeenCalledWith("cs_CZ");
expect(setUILocaleFn).toHaveBeenCalledWith("cs_CZ.UTF-8");
});
});
});
Expand Down

0 comments on commit df72d49

Please sign in to comment.