Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix: properly handle installer language changes #1746

Merged
merged 18 commits into from
Nov 14, 2024
Merged

fix: properly handle installer language changes #1746

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
119364f
fix(rust): fix the D-Bus manager's LocaleProxy path
imobachgs Nov 11, 2024
970ce9b
refactor(web): convert installerL10n to TypeScript
imobachgs Nov 12, 2024
33309f3
refactor(web): annotate installerL10n types
imobachgs Nov 12, 2024
4145587
fix(web): fix capitalization of RFC 5646 language tags
imobachgs Nov 12, 2024
fc46977
fix(web): fix installer language handling
imobachgs Nov 12, 2024
d72bcbb
fix(web): better fallback value for unsupported languages
imobachgs Nov 12, 2024
9d57f26
fix(web): refresh products on language change
imobachgs Nov 12, 2024
92d21fe
fix(web): set UI language after the keymap
imobachgs Nov 13, 2024
8edf7bb
fix(web): move installerL10n tests to TypeScript
imobachgs Nov 13, 2024
9c7b125
fix(web): adapt installerL10n tests
imobachgs Nov 13, 2024
c3f6425
fix(web): adapt App tests to language fixes
imobachgs Nov 13, 2024
a6e0cc5
fix(service): do not crash when checking storage issues before probing
imobachgs Nov 13, 2024
b8acc1d
fix(rust): fix the Locale proxy default path
imobachgs Nov 13, 2024
01153b5
docs: update changes files
imobachgs Nov 13, 2024
a3cf8ab
Merge branch 'master' into bsc-1233160
imobachgs Nov 13, 2024
c1f640e
Rename a duplicate interface to org.opensuse.Agama1.LocaleMixin
mvidner Nov 13, 2024
688397a
refactor(web): do not use cancellablePromise in installerL10n context
imobachgs Nov 13, 2024
e4631f9
docs(web): fix a typo
imobachgs Nov 13, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions rust/agama-lib/src/localization/proxies.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@
use zbus::proxy;
#[proxy(
default_service = "org.opensuse.Agama1",
default_path = "/org/opensuse/Agama1/Locale",
interface = "org.opensuse.Agama1.Locale",
assume_defaults = true
)]
Expand Down
2 changes: 1 addition & 1 deletion rust/agama-lib/src/proxies.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,6 @@ mod issues;
pub use issues::IssuesProxy;

mod locale;
pub use locale::LocaleProxy;
pub use locale::LocaleMixinProxy;

pub mod jobs;
9 changes: 5 additions & 4 deletions rust/agama-lib/src/proxies/locale.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
//! # D-Bus interface proxy for: `org.opensuse.Agama1.Locale`
//! # D-Bus interface proxy for: `org.opensuse.Agama1.LocaleMixin`
//!
//! This code was generated by `zbus-xmlgen` `5.0.0` from D-Bus introspection data.
//! Source: `org.opensuse.Agama1.Manager.bus.xml`.
Expand All @@ -20,11 +20,12 @@
//! [D-Bus standard interfaces]: https://dbus.freedesktop.org/doc/dbus-specification.html#standard-interfaces,
use zbus::proxy;
#[proxy(
default_service = "org.opensuse.Agama1",
interface = "org.opensuse.Agama1.Locale",
default_service = "org.opensuse.Agama.Manager1",
default_path = "/org/opensuse/Agama/Manager1",
interface = "org.opensuse.Agama1.LocaleMixin",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch... except it does not work: 🤣 😢

When I run this code and change the UI langue in the browser, journalctl has:

Nov 12 13:10:23 c75fbede8655 agama-web-server[28278]: Sending message: Msg { type: MethodCall, serial: 157, sender: UniqueName(":1.7"), path: ObjectPath("/org/freedesktop/Locale"), iface: InterfaceName("org.freedesktop.DBus.Properties"), member: MemberName("Set"), body: Structure(Dynamic { fields: [Str, Str, Variant] }), fds: [] }
...
Nov 12 13:10:23 c75fbede8655 agama-web-server[28278]: 5:185m5:185mCould not synchronize settings in the localization D-Bus service: D-Bus service error: org.freedesktop.DBus.Error.UnknownObject: Unknown object '/org/freedesktop/Locale'

Looking up "Could not synchronize settings" in the source, I arrive at a update_dbus fn, which uses a set_locales fn, defined in...

agama/rust/agama-lib/src/localization/proxies.rs

Line 62 in 65939ce

fn set_locales(&self, value: &[&str]) -> zbus::Result<()>;

Which is a similar, yet different file from this diff
🤷

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, it works for me (TM) 😉 Actually, I would not expect the set_locales method to be involved because that's responsible for changing the locales to install, not the installer's language. Are you changing the language through the "Installer Options" menu? Did you restart the agama-dbus-server process too (restarting the agama.service should be enough)?

Having two interfaces with the same name (org.opensuse.Agama1.Locale) is unfortunate, I know, but they are different things:

  • localization/proxies refers to the Locale API (implemented in agama-dbus-server). We kept a D-Bus interface because it was needed for internal communication. But the idea is to get rid of it and rely only on HTTP.
  • proxies/locale refers to an interface that is implemented by all Ruby-based services which allows to change the language.

I wonder if we should add default_path = "/org/opensuse/Agama1/Locale", to the 1st proxy, but in my testing system it seems to work (unless I am missing something).

In any case, I will build an image that includes all the changes so we can perform more tests.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have fixed both proxies and now I see no error in the logs.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Having two interfaces with the same name (org.opensuse.Agama1.Locale) is unfortunate, I know, but they are different things:

It's not unfortunate, it is wrong 😄 but it happens to work now. I hope I can add a simple fix, otherwise postpone it for another PR

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, it is wrong 😅 The point is that we plan to drop one of them, so it won't be duplicated anymore.

assume_defaults = true
)]
pub trait Locale {
pub trait LocaleMixin {
/// SetLocale method
fn set_locale(&self, locale: &str) -> zbus::Result<()>;
}
2 changes: 1 addition & 1 deletion rust/agama-server/src/l10n/web.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ use crate::{
};
use agama_lib::{
error::ServiceError, localization::model::LocaleConfig, localization::LocaleProxy,
proxies::LocaleProxy as ManagerLocaleProxy,
proxies::LocaleMixinProxy as ManagerLocaleProxy,
};
use agama_locale_data::LocaleId;
use axum::{
Expand Down
6 changes: 6 additions & 0 deletions rust/package/agama.changes
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,9 @@
-------------------------------------------------------------------
Wed Nov 13 12:03:02 UTC 2024 - Imobach Gonzalez Sosa <[email protected]>

- Properly update the localization settings of the D-Bus services
(bsc#1233159, bsc#1233160).

-------------------------------------------------------------------
Mon Nov 11 09:52:59 UTC 2024 - Imobach Gonzalez Sosa <[email protected]>

Expand Down
2 changes: 1 addition & 1 deletion service/lib/agama/dbus/interfaces/locale.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ module Locale
include Yast::I18n
include Yast::Logger

LOCALE_INTERFACE = "org.opensuse.Agama1.Locale"
LOCALE_INTERFACE = "org.opensuse.Agama1.LocaleMixin"

def self.included(base)
base.class_eval do
Expand Down
1 change: 1 addition & 0 deletions service/lib/agama/storage/manager.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -262,6 +262,7 @@ def system_issues
# @return [Array<Issue>]
def probing_issues
y2storage_issues = Y2Storage::StorageManager.instance.raw_probed.probing_issues
return [] if y2storage_issues.nil?

y2storage_issues.map do |y2storage_issue|
Issue.new(y2storage_issue.message,
Expand Down
6 changes: 6 additions & 0 deletions service/package/rubygem-agama-yast.changes
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,9 @@
-------------------------------------------------------------------
Wed Nov 13 12:14:06 UTC 2024 - Imobach Gonzalez Sosa <[email protected]>

- Do not crash when trying to change the language of the storage
service before the "config" phase (gh#agama-project/agama#1746).

-------------------------------------------------------------------
Tue Nov 5 16:11:35 UTC 2024 - Martin Vidner <[email protected]>

Expand Down
11 changes: 11 additions & 0 deletions web/package/agama-web-ui.changes
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,14 @@
-------------------------------------------------------------------
Wed Nov 13 12:06:41 UTC 2024 - Imobach Gonzalez Sosa <[email protected]>

- Several translation fixes (gh#agama-project/agama#1746):
- Use the correct capitalization for RFC 5646 language tags
(e.g., "pt-BR" instead of "pt-BR" instead of "pt-br") (bsc#1233160).
- Translate the products descriptions when the user changes
the language (gh#agama-project/agama#1724).
- Fallback to a similar language if the given one is not supported
(e.g., "es" for "es-AR") (gh#agama-project/agama#860).

-------------------------------------------------------------------
Wed Nov 6 06:06:51 UTC 2024 - Michal Filka <[email protected]>

Expand Down
126 changes: 63 additions & 63 deletions web/share/locales.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,65 +1,65 @@
{
"af": "za",
"am": "et",
"ar": "eg",
"ast": "es",
"be": "by",
"bg": "bg",
"bn": "in",
"bs": "ba",
"ca": "es",
"cs": "cz",
"cy": "gb",
"da": "dk",
"de": "de",
"en": "us",
"el": "gr",
"es": "es",
"et": "ee",
"eu": "es",
"fa": "ir",
"fi": "fi",
"fr": "fr",
"gl": "es",
"gu": "in",
"he": "il",
"hi": "in",
"hr": "hr",
"hu": "hu",
"id": "id",
"it": "it",
"ja": "jp",
"ka": "kz",
"km": "kh",
"kn": "in",
"ko": "kr",
"lt": "lt",
"lv": "lv",
"mk": "mk",
"mr": "in",
"ms": "my",
"my": "mm",
"nb": "no",
"nds": "de",
"ne": "np",
"nl": "nl",
"nn": "no",
"pa": "in",
"pl": "pl",
"pt": "pt",
"ro": "ro",
"ru": "ru",
"si": "lk",
"sk": "sk",
"sl": "si",
"sq": "al",
"sr": "rs",
"sv": "se",
"ta": "in",
"tg": "tj",
"th": "th",
"tr": "tr",
"uk": "ua",
"vi": "vn",
"zu": "za"
"af": "ZA",
"am": "ET",
"ar": "EG",
"ast": "ES",
"be": "BY",
"bg": "BG",
"bn": "IN",
"bs": "BA",
"ca": "ES",
"cs": "CZ",
"cy": "GB",
"da": "DK",
"de": "DE",
"en": "US",
"el": "GR",
"es": "ES",
"et": "EE",
"eu": "ES",
"fa": "IR",
"fi": "FI",
"fr": "FR",
"gl": "ES",
"gu": "IN",
"he": "IL",
"hi": "IN",
"hr": "HR",
"hu": "HU",
"id": "ID",
"it": "IT",
"ja": "JP",
"ka": "KZ",
"km": "KH",
"kn": "IN",
"ko": "KR",
"lt": "LT",
"lv": "LV",
"mk": "MK",
"mr": "IN",
"ms": "MY",
"my": "MM",
"nb": "NO",
"nds": "DE",
"ne": "NP",
"nl": "NL",
"nn": "NO",
"pa": "IN",
"pl": "PL",
"pt": "PT",
"ro": "RO",
"ru": "RU",
"si": "LK",
"sk": "SK",
"sl": "SI",
"sq": "AL",
"sr": "RS",
"sv": "SE",
"ta": "IN",
"tg": "TJ",
"th": "TH",
"tr": "TR",
"uk": "UA",
"vi": "VN",
"zu": "ZA"
}
13 changes: 7 additions & 6 deletions web/share/update-languages.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
#

from argparse import ArgumentParser
from typing import Optional
from langtable import language_name
from pathlib import Path
import json
Expand All @@ -32,9 +33,9 @@

class Locale:
language: str
territory: str
territory: Optional[str]

def __init__(self, language: str, territory: str = None):
def __init__(self, language: str, territory: Optional[str] = None):
self.language = language
self.territory = territory

Expand All @@ -44,13 +45,13 @@ def code(self):
def name(self, include_territory: bool = False):
if include_territory:
return language_name(languageId=self.language,
territoryId=self.territory.upper())
territoryId=self.territory)
else:
return language_name(languageId=self.language)


class PoFile:
path: str
path: Path
locale: Locale

def __init__(self, path: Path):
Expand Down Expand Up @@ -85,7 +86,7 @@ class Languages:
def __init__(self):
self.content = dict()

def update(self, po_files, lang2territory: str, threshold: int):
def update(self, po_files, lang2territory, threshold: int):
"""
Generate the list of supported locales

Expand All @@ -97,7 +98,7 @@ def update(self, po_files, lang2territory: str, threshold: int):
:param threshold Percentage of the strings that must be covered to
include the locale in the manifest
"""
supported = [Locale("en", "us")]
supported = [Locale("en", "US")]

for path in po_files:
po_file = PoFile(path)
Expand Down
4 changes: 2 additions & 2 deletions web/src/App.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ jest.mock("~/api/l10n", () => ({
...jest.requireActual("~/api/l10n"),
fetchConfig: jest.fn().mockResolvedValue({
uiKeymap: "en",
uiLocale: "en_us",
uiLocale: "en_US",
}),
updateConfig: jest.fn(),
}));
Expand Down Expand Up @@ -98,7 +98,7 @@ jest.mock("~/components/product/ProductSelectionProgress", () => () => <div>Prod
describe("App", () => {
beforeEach(() => {
// setting the language through a cookie
document.cookie = "agamaLang=en-us; path=/;";
document.cookie = "agamaLang=en-US; path=/;";
(createClient as jest.Mock).mockImplementation(() => {
return {};
});
Expand Down
2 changes: 1 addition & 1 deletion web/src/components/core/InstallerOptions.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ export default function InstallerOptions({ isOpen = false, onClose }: InstallerO
const onSubmit = async (e: React.FormEvent<HTMLFormElement>) => {
e.preventDefault();
setInProgress(true);
changeKeymap(keymap);
await changeKeymap(keymap);
changeLanguage(language)
.then(close)
.catch(() => setInProgress(false));
Expand Down
Loading
Loading