diff mbox series

[bug#65922,qt-team,v2,1/4] gnu: qtbase: Do not capture Python (again).

Message ID ff4d270dd33e8ef2baf9cf3a89029548c61a208d.1694626350.git.maxim.cournoyer@gmail.com
State New
Headers show
Series Avoid capturing python in qtbase, reinstate tests | expand

Commit Message

Maxim Cournoyer Sept. 13, 2023, 5:33 p.m. UTC 
Fixes <https://issues.guix.gnu.org/65457>.

* gnu/packages/qt.scm (qtbase-5) [arguments]: Add #:disallowed-references
argument to guard against the problem reoccurring in the future.
Add do-not-capture-python phase.
(qtbase) [arguments]: Update do-not-capture-python phase.
---

(no changes since v1)

 gnu/packages/qt.scm | 20 ++++++++++++++++----
 1 file changed, 16 insertions(+), 4 deletions(-)

Comments

Andreas Enge Sept. 14, 2023, 9:07 a.m. UTC | #1 
Hello,

I just happened to have the time for one patch :)

Am Wed, Sep 13, 2023 at 01:33:13PM -0400 schrieb Maxim Cournoyer:
> * gnu/packages/qt.scm (qtbase-5) [arguments]: Add #:disallowed-references
> argument to guard against the problem reoccurring in the future.
> Add do-not-capture-python phase.
> (qtbase) [arguments]: Update do-not-capture-python phase.

This one looks good, except for indentation there, I think:
> +             (substitute*
> +                 (search-input-file
> +                  outputs "lib/qt5/mkspecs/features/uikit/devices.py")
> +               (((which "python3"))
> +                "/usr/bin/env python3")))))))

Andreas
Maxim Cournoyer Sept. 17, 2023, 2:51 a.m. UTC | #2 
Hi Andreas,

Andreas Enge <andreas@enge.fr> writes:

> Hello,
>
> I just happened to have the time for one patch :)
>
> Am Wed, Sep 13, 2023 at 01:33:13PM -0400 schrieb Maxim Cournoyer:
>> * gnu/packages/qt.scm (qtbase-5) [arguments]: Add #:disallowed-references
>> argument to guard against the problem reoccurring in the future.
>> Add do-not-capture-python phase.
>> (qtbase) [arguments]: Update do-not-capture-python phase.
>
> This one looks good, except for indentation there, I think:
>> +             (substitute*
>> +                 (search-input-file
>> +                  outputs "lib/qt5/mkspecs/features/uikit/devices.py")
>> +               (((which "python3"))
>> +                "/usr/bin/env python3")))))))

It's actually valid -- I just doubled check :-)

Thanks for taking a peek.
Andreas Enge Sept. 17, 2023, 10:12 a.m. UTC | #3 
Am Sat, Sep 16, 2023 at 10:51:22PM -0400 schrieb Maxim Cournoyer:
> > This one looks good, except for indentation there, I think:
> >> +             (substitute*
> >> +                 (search-input-file
> >> +                  outputs "lib/qt5/mkspecs/features/uikit/devices.py")
> >> +               (((which "python3"))
> >> +                "/usr/bin/env python3")))))))
> It's actually valid -- I just doubled check :-)

Hm, I thought that arguments to functions should be aligned, so that
"(search-..." and "(((which" should start in the same column?
And that we would indent by either 1 or 2 (according to arcane rules I
never memorised)?

Andreas
Maxim Cournoyer Sept. 17, 2023, 12:55 p.m. UTC | #4 
Hi Andreas,

Andreas Enge <andreas@enge.fr> writes:

> Am Sat, Sep 16, 2023 at 10:51:22PM -0400 schrieb Maxim Cournoyer:
>> > This one looks good, except for indentation there, I think:
>> >> +             (substitute*
>> >> +                 (search-input-file
>> >> +                  outputs "lib/qt5/mkspecs/features/uikit/devices.py")
>> >> +               (((which "python3"))
>> >> +                "/usr/bin/env python3")))))))
>> It's actually valid -- I just doubled check :-)
>
> Hm, I thought that arguments to functions should be aligned, so that
> "(search-..." and "(((which" should start in the same column?
> And that we would indent by either 1 or 2 (according to arcane rules I
> never memorised)?

According to our indentation rules in .dir-locals.el:

--8<---------------cut here---------------start------------->8---
(put 'substitute* 'scheme-indent-function 1)
--8<---------------cut here---------------end--------------->8---

Which, following the Emacs documentation for scheme-indent-function then
lisp-indent-function (C-h f scheme-indent-function RET), says (where N
is the 1 argument):

--8<---------------cut here---------------start------------->8---
* an integer N, meaning indent the first N arguments specially
  (like ordinary function arguments), and then indent any further
  arguments like a body;
--8<---------------cut here---------------end--------------->8---

The indentation produced is because of that special rule.
diff mbox series

Patch

diff --git a/gnu/packages/qt.scm b/gnu/packages/qt.scm
index b7e394d663c..a158f8114fe 100644
--- a/gnu/packages/qt.scm
+++ b/gnu/packages/qt.scm
@@ -392,7 +392,7 @@  (define-public qtbase-5
            fontconfig
            freetype
            glib
-           gtk+               ;for GTK theme support
+           gtk+                         ;for GTK theme support
            harfbuzz
            icu4c
            libinput-minimal
@@ -437,7 +437,8 @@  (define-public qtbase-5
            vulkan-headers
            ruby-2.7))
     (arguments
-     `(#:configure-flags
+     `(#:disallowed-references ,(list python)
+       #:configure-flags
        (let ((out (assoc-ref %outputs "out")))
          (list "-verbose"
                "-prefix" out
@@ -583,7 +584,16 @@  (define-public qtbase-5
                (("^\\s*(QLibrary xcursorLib\\(QLatin1String\\(\")(Xcursor\"\\), 1\\);)" _ a b)
                 (string-append a (assoc-ref inputs "libxcursor") "/lib/lib" b))
                (("^\\s*(xcursorLib.setFileName\\(QLatin1String\\(\")(Xcursor\"\\)\\);)" _ a b)
-                (string-append a (assoc-ref inputs "libxcursor") "/lib/lib" b))))))))
+                (string-append a (assoc-ref inputs "libxcursor") "/lib/lib" b)))))
+         (add-after 'install 'do-not-capture-python
+           (lambda* (#:key outputs #:allow-other-keys)
+             ;; For some reason, patching the file after the
+             ;; patch-source-shebangs phase doesn't work for Qt 5.
+             (substitute*
+                 (search-input-file
+                  outputs "lib/qt5/mkspecs/features/uikit/devices.py")
+               (((which "python3"))
+                "/usr/bin/env python3")))))))
     (native-search-paths
      (list (search-path-specification
             (variable "QMAKEPATH")
@@ -724,10 +734,12 @@  (define-public qtbase
                 (substitute* "src/corelib/CMakeLists.txt"
                   (("/bin/ls")
                    (search-input-file inputs "bin/ls")))))
+            (delete 'do-not-capture-python) ;move after patch-source-shebangs
             (add-after 'patch-source-shebangs 'do-not-capture-python
               (lambda _
                 (substitute* '("mkspecs/features/uikit/devices.py"
-                               "util/testrunner/qt-testrunner.py")
+                               "util/testrunner/qt-testrunner.py"
+                               "util/testrunner/sanitizer-testrunner.py")
                   (((which "python3"))
                    "/usr/bin/env python3"))))
             (replace 'configure