Skip to content

packet: 0.4.0 → 0.5.1#416076

Merged
Aleksanaa merged 1 commit intoNixOS:masterfrom
honnip:update/packet
Jun 14, 2025
Merged

packet: 0.4.0 → 0.5.1#416076
Aleksanaa merged 1 commit intoNixOS:masterfrom
honnip:update/packet

Conversation

@honnip
Copy link
Contributor

@honnip honnip commented Jun 12, 2025

https://github.com/nozwock/packet/releases/tag/0.5.0
https://github.com/nozwock/packet/releases/tag/0.5.0-1
https://github.com/nozwock/packet/releases/tag/0.5.1

NOTE: This update includes the Nautilus extension. Some additional configuration is required to make it work. See honnip/dotfiles@4a4c01c or nautilus-open-any-terminal module

Things done

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
    • sandbox = relaxed
    • sandbox = true
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • Nixpkgs 25.11 Release Notes (or backporting 24.11 and 25.05 Nixpkgs Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
  • NixOS 25.11 Release Notes (or backporting 24.11 and 25.05 NixOS Release notes)
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md, pkgs/README.md, maintainers/README.md and other contributing documentation in corresponding paths.

Add a 👍 reaction to pull requests you find important.

@github-actions github-actions bot added 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. 10.rebuild-linux: 1 This PR causes 1 package to rebuild on Linux. labels Jun 12, 2025
@nix-owners nix-owners bot requested a review from make-42 June 12, 2025 08:37
@honnip
Copy link
Contributor Author

honnip commented Jun 12, 2025

screenshot

스크린샷 2025-06-12 17-22-24

log:

nautilus[7181]: Packet: No ko_KR localization found for domain: 'packet'

non localized string is displayed in the Nautilus menu, and I have no idea how to fix it

@make-42
Copy link
Contributor

make-42 commented Jun 12, 2025

I'll have a look when I get back home later for both the update and why Korean i18n isn't working. Since the extra options aren't home manager options, I see no reason why we can't just make a module for packet to have a programs.packet attribute set that adds these options

@honnip
Copy link
Contributor Author

honnip commented Jun 12, 2025

I see no reason why we can't just make a programs entry that when enabled adds these options

Yes, you're right. I would like to see in other PR

@make-42
Copy link
Contributor

make-42 commented Jun 12, 2025

Sure!

@make-42
Copy link
Contributor

make-42 commented Jun 12, 2025

It compiles and runs fine on my machine! I use en-US as a locale so I can't reproduce your issue unless I use a VM. However from what I can see, it's supposed to be included in the data/resources/plugins/packet_nautilus.py file at line 113 on build time. It seems to be installed correctly. The only way I think it could be going wrong is if the locale dir isn't being found properly.

@make-42
Copy link
Contributor

make-42 commented Jun 12, 2025

Other than that, lgtm

@nozwock
Copy link

nozwock commented Jun 13, 2025

Currently, the extension only looks in two locale directories, the one given by Flatpak, and another being what python considers to be the default locale directory1

Where are the packet.po files stored in the installed nix package?

Footnotes

  1. https://docs.python.org/3/library/gettext.html#id3

@nozwock
Copy link

nozwock commented Jun 13, 2025

For now you might want to patch the None with a hard coded locale directory that's available in the installed nix package?

-    locale_dirs: List[Path | None] = [None]  # None for system default locale dir
+    locale_dirs: List[Path] = [Path("/path/to/localedir/containing/packet.po's")]

@wegank wegank added 12.approved-by: package-maintainer This PR was reviewed and approved by a maintainer listed in any of the changed packages. 12.approvals: 1 This PR was reviewed and approved by one person. labels Jun 13, 2025
@make-42
Copy link
Contributor

make-42 commented Jun 13, 2025

That's exactly what my code suggestion does:

    substituteInPlace $out/share/packet/plugins/packet_nautilus.py \
      --replace-fail 'locale_dirs: List[Path | None] = [None]' \
                'locale_dirs: List[Path | None] = [Path("'"$out"'/share/locale"), None]'

Adds the local dir path on build

@honnip
Copy link
Contributor Author

honnip commented Jun 13, 2025

Thank you all. It works well

@honnip
Copy link
Contributor Author

honnip commented Jun 13, 2025

nixpkgs-review result

Generated using nixpkgs-review-gha

Command: nixpkgs-review pr 416076

Logs: https://github.com/honnip/nixpkgs-review-gha/actions/runs/15630594040


x86_64-linux (sandbox = true)

✅ 1 package built:
  • packet

aarch64-linux (sandbox = true)

✅ 1 package built:
  • packet

@nozwock
Copy link

nozwock commented Jun 13, 2025

That's exactly what my code suggestion does:

    substituteInPlace $out/share/packet/plugins/packet_nautilus.py \
      --replace-fail 'locale_dirs: List[Path | None] = [None]' \
                'locale_dirs: List[Path | None] = [Path("'"$out"'/share/locale"), None]'

Adds the local dir path on build

Interesting, I think I can just fix it on my end then, by having another directory to look for which is

<meson dest dir>/share/locale

@nozwock
Copy link

nozwock commented Jun 13, 2025

Interesting, I think I can just fix it on my end then, by having another directory to look for which is

<meson dest dir>/share/locale

Actually no, I forgot that I had already thought of this and got rid of the idea, since for a Flatpak build, the meson DEST_DIR would be /app and so the locale dir will end up being /app/share/locale which is not a directory accessible from the extension since it's only valid within the sandbox.

Not sure if the resulting path would be useful for a nixpkg though, you may try locally building with this patch and see if it works or not:

diff --git a/data/resources/plugins/meson.build b/data/resources/plugins/meson.build
index ac60edd..9e055b8 100644
--- a/data/resources/plugins/meson.build
+++ b/data/resources/plugins/meson.build
@@ -1,6 +1,7 @@
 plugins_conf = configuration_data()
 plugins_conf.set('APP_ID', application_id)
 plugins_conf.set('LOCALE_DOMAIN', gettext_package)
+plugins_conf.set('LOCALE_DIR', localedir)
 
 configure_file(
   input: 'packet_nautilus.py.in',
diff --git a/data/resources/plugins/packet_nautilus.py.in b/data/resources/plugins/packet_nautilus.py.in
index 9c5e71e..7288ee9 100755
--- a/data/resources/plugins/packet_nautilus.py.in
+++ b/data/resources/plugins/packet_nautilus.py.in
@@ -23,7 +23,8 @@ def log(*vals: Any):
 # TODO: Maybe have a separate gettext package for plugin scripts that gets
 # copied over alongside the script. Seems more robust?
 def init_i18n() -> gettext.NullTranslations | gettext.GNUTranslations:
-    locale_dirs: List[Path | None] = [None]  # None for system default locale dir
+    # `None` is for system default locale dir
+    locale_dirs: List[Path | None] = [None, Path("@LOCALE_DIR@")]
     (lang, enc) = locale.getlocale()
 
     flatpak_info = None

@make-42
Copy link
Contributor

make-42 commented Jun 13, 2025

Interesting, I think I can just fix it on my end then, by having another directory to look for which is

<meson dest dir>/share/locale

Actually no, I forgot that I had already thought of this and got rid of the idea, since for a Flatpak build, the meson DEST_DIR would be /app and so the locale dir will end up being /app/share/locale which is not a directory accessible from the extension since it's only valid within the sandbox.

Not sure if the resulting path would be useful for a nixpkg though, you may try locally building with this patch and see if it works or not:

diff --git a/data/resources/plugins/meson.build b/data/resources/plugins/meson.build
index ac60edd..9e055b8 100644
--- a/data/resources/plugins/meson.build
+++ b/data/resources/plugins/meson.build
@@ -1,6 +1,7 @@
 plugins_conf = configuration_data()
 plugins_conf.set('APP_ID', application_id)
 plugins_conf.set('LOCALE_DOMAIN', gettext_package)
+plugins_conf.set('LOCALE_DIR', localedir)
 
 configure_file(
   input: 'packet_nautilus.py.in',
diff --git a/data/resources/plugins/packet_nautilus.py.in b/data/resources/plugins/packet_nautilus.py.in
index 9c5e71e..7288ee9 100755
--- a/data/resources/plugins/packet_nautilus.py.in
+++ b/data/resources/plugins/packet_nautilus.py.in
@@ -23,7 +23,8 @@ def log(*vals: Any):
 # TODO: Maybe have a separate gettext package for plugin scripts that gets
 # copied over alongside the script. Seems more robust?
 def init_i18n() -> gettext.NullTranslations | gettext.GNUTranslations:
-    locale_dirs: List[Path | None] = [None]  # None for system default locale dir
+    # `None` is for system default locale dir
+    locale_dirs: List[Path | None] = [None, Path("@LOCALE_DIR@")]
     (lang, enc) = locale.getlocale()
 
     flatpak_info = None

That patch yields the perfect result:
image

If applied upstream it would make it so we don't have to do some ugly substitution to get the locale path. That's awesome

@github-actions github-actions bot added 12.approvals: 2 This PR was reviewed and approved by two persons. and removed 12.approved-by: package-maintainer This PR was reviewed and approved by a maintainer listed in any of the changed packages. 12.approvals: 1 This PR was reviewed and approved by one person. labels Jun 14, 2025
@Aleksanaa Aleksanaa merged commit 57b2cca into NixOS:master Jun 14, 2025
13 of 16 checks passed
@Aleksanaa
Copy link
Member

Aleksanaa commented Jun 14, 2025

Sorry to bother you @wolfgangwalther, but the approval label seems to be doing wrong here. It added 12.approvals: 2 because make-42 approved two times (or some other reasons, like force push?).

@wolfgangwalther
Copy link
Contributor

wolfgangwalther commented Jun 14, 2025

It added 12.approvals: 2 because make-42 approved two times

Yes, that seems to be the case 🙈

Thanks for the report!

Fix in #416668

@wolfgangwalther
Copy link
Contributor

Sorry to bother you

And please keep reporting anything that seems wrong, I appreciate those reports!

nozwock added a commit to nozwock/packet that referenced this pull request Jun 14, 2025
Have another locale directory to look for which is based on meson's
`DESTDIR`. `DESTDIR/share/locale` is where the locale seems to be stored
for the built nixpkg.

References:
- NixOS/nixpkgs#416076 (comment)
@nozwock
Copy link

nozwock commented Jun 14, 2025

If applied upstream it would make it so we don't have to do some ugly substitution to get the locale path. That's awesome

I'll have it in the next release.

@make-42
Copy link
Contributor

make-42 commented Jun 14, 2025

Awesome, I'll also make a nix module for this package to have enable the necessary options for nautilus integration

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. 10.rebuild-linux: 1 This PR causes 1 package to rebuild on Linux. 12.approvals: 2 This PR was reviewed and approved by two persons.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants