diff mbox

[bug#57257,00/31] Update LXQt to 1.1.0

Message ID 877d313lpw.fsf@elephly.net
State Accepted
Headers show

Commit Message

Ricardo Wurmus Aug. 21, 2022, 10:55 a.m. UTC 
Hi,

overall this looks good to me.  Thank you for working on it!

There are just a few questions:

- does it really work to use relative file names?  Would it be possible
  to test this roughly in a system test?

- you’re removing a lot of patch-source phases.  Why are these no longer
  needed?

- you’re also removing patch-translations-dir phases.  Do translations
  still work after applying these changes?

- this diff looks wrong to me:

--8<---------------cut here---------------start------------->8---

Comments

\( Aug. 21, 2022, 11:03 a.m. UTC | #1 
On Sun Aug 21, 2022 at 11:55 AM BST, Ricardo Wurmus wrote:
> - this diff looks wrong to me:
>
> --8<---------------cut here---------------start------------->8---
> diff --git a/gnu/packages/lxqt.scm b/gnu/packages/lxqt.scm
> index 08e33ca0e3..d2cbd1e131 100644
> --- a/gnu/packages/lxqt.scm
> +++ b/gnu/packages/lxqt.scm
> @@ -843,32 +843,31 @@ (define-public libfm-qt
>  (define-public pcmanfm-qt
>    (package
>      (name "pcmanfm-qt")
> […]
> +    (arguments
> +     (list
> +      #:tests? #f                       ; no tests
> +      #:phases
> +      #~(modify-phases %standard-phases
> +          (add-before 'configure 'patch-settings.conf.in
> +            (lambda* (#:key inputs #:allow-other-keys)
> +              (let ((wallpaper (search-input-file inputs
> +                                "share/lxqt/wallpapers/waves-logo.png")))
> --8<---------------cut here---------------end--------------->8---
>
>   Shouldn’t it be "/share/lxqt…"?

No, this use of search-input-file is correct.

    -- (
宋文武 Aug. 22, 2022, 12:55 a.m. UTC | #2 
Ricardo Wurmus <rekado@elephly.net> writes:

> Hi,
>
> overall this looks good to me.  Thank you for working on it!
>
> There are just a few questions:
>
> - does it really work to use relative file names?  Would it be possible
>   to test this roughly in a system test?
Yes, the relative file names are for install, at runtime most things
works as designed with XDG_DATA_DIRS and XDG_CONFIG_DIRS.

A system test could verify items in lxqt-config, will look it later...

>
> - you’re removing a lot of patch-source phases.  Why are these no longer
>   needed?

Yeah, I find that use relative file names in LXQtConfigVars.cmake remove
the need of patching other cmake files.
>
> - you’re also removing patch-translations-dir phases.  Do translations
>   still work after applying these changes?

Yes, at runtime they are found under XDG_DATA_DIRS, by
lxqttranslator.cpp:

*searchPath << XdgDirs::dataDirs(QL1C('/') + QL1S(LXQT_RELATIVE_SHARE_TRANSLATIONS_DIR));

> [...]
>
> Feel free to push this!

Okay, thanks for the review!
diff mbox

Patch

diff --git a/gnu/packages/lxqt.scm b/gnu/packages/lxqt.scm
index 08e33ca0e3..d2cbd1e131 100644
--- a/gnu/packages/lxqt.scm
+++ b/gnu/packages/lxqt.scm
@@ -843,32 +843,31 @@  (define-public libfm-qt
 (define-public pcmanfm-qt
   (package
     (name "pcmanfm-qt")
[…]
+    (arguments
+     (list
+      #:tests? #f                       ; no tests
+      #:phases
+      #~(modify-phases %standard-phases
+          (add-before 'configure 'patch-settings.conf.in
+            (lambda* (#:key inputs #:allow-other-keys)
+              (let ((wallpaper (search-input-file inputs
+                                "share/lxqt/wallpapers/waves-logo.png")))
--8<---------------cut here---------------end--------------->8---

  Shouldn’t it be "/share/lxqt…"?


Feel free to push this!

-- 
Ricardo