unofficial mirror of guix-patches@gnu.org 
 help / color / mirror / Atom feed
* [bug#43249] Resolve Calibre run-time dependency
@ 2020-09-06 18:32 Prafulla Giri
  2020-09-06 20:09 ` Jonathan Brielmaier
                   ` (2 more replies)
  0 siblings, 3 replies; 16+ messages in thread
From: Prafulla Giri @ 2020-09-06 18:32 UTC (permalink / raw)
  To: 43249


[-- Attachment #1.1: Type: text/plain, Size: 476 bytes --]

Esteemed maintainers,

Currently, Calibre can't open .epub files unless `qtwebengine` package is
also available in one's $GUIX_PROFILE. Neither can the stand-alone
`ebook-viewer` program supplied with Calibre. It exits with the complaint:
"Could not find QtWebEngineProcess".

Attached is a patch to fix the issue. QtWebEngineProcess is now made
available to all Calibre binaries via QTWEBENGINE_PATH set during a new
'wrap-program phase introduced with the patch.

Thank you

[-- Attachment #1.2: Type: text/html, Size: 603 bytes --]

[-- Attachment #2: 0001-gnu-calibre-make-QtWebEngineProcess-available-during.patch --]
[-- Type: text/x-patch, Size: 1783 bytes --]

From cec0d8adf456eff2201fd89bb1fde6cba9d6fa77 Mon Sep 17 00:00:00 2001
From: Prafulla Giri <pratheblackdiamond@gmail.com>
Date: Sun, 6 Sep 2020 23:57:14 +0545
Subject: [PATCH] gnu: calibre: make QtWebEngineProcess available during
 runtime

* gnu/packages/ebook.scm [arguments]: Add new phase 'wrap-program
to make QtWebEngineProcess available to the binaries during run-
time with QTWEBENGINEPROCESS_PATH.
---
 gnu/packages/ebook.scm | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/gnu/packages/ebook.scm b/gnu/packages/ebook.scm
index aab4155d3d..f63d284afb 100644
--- a/gnu/packages/ebook.scm
+++ b/gnu/packages/ebook.scm
@@ -262,6 +262,22 @@
                                             "/share/fonts/truetype")))
                (delete-file-recursively font-dest)
                (symlink font-src font-dest))
+             #t))
+         ;; Make run-time dependencies available to the binaries
+         (add-after 'wrap 'wrap-program
+           (lambda* (#:key inputs outputs #:allow-other-keys)
+             (let ((out (assoc-ref outputs "out"))
+                   (qtwebengine (assoc-ref inputs "qtwebengine")))
+               (with-directory-excursion (string-append out "/bin")
+                 (for-each
+                  (lambda (binary)
+                    (wrap-program binary
+                      ;; Make QtWebEngineProcess available
+                      `("QTWEBENGINEPROCESS_PATH" ":" =
+                        ,(list (string-append
+                                qtwebengine
+                                "/lib/qt5/libexec/QtWebEngineProcess")))))
+                  (find-files "." "."))))
              #t)))))
     (home-page "https://calibre-ebook.com/")
     (synopsis "E-book library management software")
-- 
2.28.0


^ permalink raw reply	[flat|nested] 16+ messages in thread

* [bug#43249] Resolve Calibre run-time dependency
  2020-09-06 18:32 [bug#43249] Resolve Calibre run-time dependency Prafulla Giri
@ 2020-09-06 20:09 ` Jonathan Brielmaier
  2020-09-07  8:12 ` Brendan Tildesley
  2020-09-08 12:22 ` [bug#43249] Prafulla Giri
  2 siblings, 0 replies; 16+ messages in thread
From: Jonathan Brielmaier @ 2020-09-06 20:09 UTC (permalink / raw)
  To: Prafulla Giri, 43249

Brendan already proposed a patch for this problem in his series updating
calibre to 4.22.0 see http://issues.guix.gnu.org/42885#4

Sorry for the duplicated effort, but maybe you can chime in there. It
seems so that there is some work left to get the update merged...

On 06.09.20 20:32, Prafulla Giri wrote:
> Esteemed maintainers,
>
> Currently, Calibre can't open .epub files unless `qtwebengine` package is
> also available in one's $GUIX_PROFILE. Neither can the stand-alone
> `ebook-viewer` program supplied with Calibre. It exits with the complaint:
> "Could not find QtWebEngineProcess".
>
> Attached is a patch to fix the issue. QtWebEngineProcess is now made
> available to all Calibre binaries via QTWEBENGINE_PATH set during a new
> 'wrap-program phase introduced with the patch.
>
> Thank you
>




^ permalink raw reply	[flat|nested] 16+ messages in thread

* [bug#43249] Resolve Calibre run-time dependency
  2020-09-06 18:32 [bug#43249] Resolve Calibre run-time dependency Prafulla Giri
  2020-09-06 20:09 ` Jonathan Brielmaier
@ 2020-09-07  8:12 ` Brendan Tildesley
       [not found]   ` <6492c3cc-07e0-b56b-ea72-99d403770755@brendan.scot>
  2020-09-08 12:22 ` [bug#43249] Prafulla Giri
  2 siblings, 1 reply; 16+ messages in thread
From: Brendan Tildesley @ 2020-09-07  8:12 UTC (permalink / raw)
  To: Prafulla Giri <pratheblackdiamond, Andreas Enge; +Cc: 43249, 43151

There is actually a Bug report by Andreas for this very issue. I created 
a bug report just for updating, and fixed this issue after it while I 
could, without realising. Sorry for all the confusion with things 
happening in 3 different threads.

I created an updated patch just for this one here. 
https://issues.guix.gnu.org/43151#5

Your patch also works I think but it will wrap the programs twice, so 
you will get calibre, .calibre-real, and ..calibre-real-real, etc for 
every program, which seems ugly. My patch reproduces the same PYTHONPATH 
that is set in python-build-system in addition to wrapping PYTHONPATH 
(unless I made a mistake), although at the cost of code duplication. I 
leave it to you and who ever is reviewing this to decide which way is 
more correct and push one, haha.


Please continue any discussion of the QtWebEngine bug on issue 43121: 
43151@debbugs.gnu.org / https://issues.guix.gnu.org/43151





^ permalink raw reply	[flat|nested] 16+ messages in thread

* [bug#43249]
  2020-09-06 18:32 [bug#43249] Resolve Calibre run-time dependency Prafulla Giri
  2020-09-06 20:09 ` Jonathan Brielmaier
  2020-09-07  8:12 ` Brendan Tildesley
@ 2020-09-08 12:22 ` Prafulla Giri
  2020-09-08 13:38   ` [bug#43249] Brendan Tildesley
                     ` (2 more replies)
  2 siblings, 3 replies; 16+ messages in thread
From: Prafulla Giri @ 2020-09-08 12:22 UTC (permalink / raw)
  To: 43249; +Cc: mail

[-- Attachment #1: Type: text/plain, Size: 265 bytes --]

I see.

Yes, it does make sense now why you chose to replace the 'wrap phase.

I wonder.... perhaps it'd be better altogether if the (wrap-program)
procedure could be re-written to not make ..*.real.real programs...? That
would save us a lot of code-duplication...

[-- Attachment #2: Type: text/html, Size: 354 bytes --]

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [bug#43249]
  2020-09-08 12:22 ` [bug#43249] Prafulla Giri
@ 2020-09-08 13:38   ` Brendan Tildesley
  2020-09-08 19:57   ` [bug#43249] Ricardo Wurmus
  2020-09-13 12:43   ` [bug#43249] Brendan Tildesley
  2 siblings, 0 replies; 16+ messages in thread
From: Brendan Tildesley @ 2020-09-08 13:38 UTC (permalink / raw)
  To: Prafulla Giri, 43249

On 8/9/20 10:22 pm, Prafulla Giri wrote:
> I see.
>
> Yes, it does make sense now why you chose to replace the 'wrap phase.
>
> I wonder.... perhaps it'd be better altogether if the (wrap-program) 
> procedure could be re-written to not make ..*.real.real programs...? 
> That would save us a lot of code-duplication...

Actually there is such a thing that can be use for scripts. It's 
wrap-script. I had sort of forgotten about it because I believe it has a 
bug where it passes command line arguments incorrectly, so wasn't using 
it: https://issues.guix.gnu.org/40039 . Would be worth looking in to again.





^ permalink raw reply	[flat|nested] 16+ messages in thread

* [bug#43249]
  2020-09-08 12:22 ` [bug#43249] Prafulla Giri
  2020-09-08 13:38   ` [bug#43249] Brendan Tildesley
@ 2020-09-08 19:57   ` Ricardo Wurmus
  2020-09-10 12:46     ` [bug#43249] Brendan Tildesley
  2020-09-13 12:43   ` [bug#43249] Brendan Tildesley
  2 siblings, 1 reply; 16+ messages in thread
From: Ricardo Wurmus @ 2020-09-08 19:57 UTC (permalink / raw)
  To: Prafulla Giri; +Cc: 43249, mail


Prafulla Giri <pratheblackdiamond@gmail.com> writes:

> I wonder.... perhaps it'd be better altogether if the (wrap-program)
> procedure could be re-written to not make ..*.real.real programs...? That
> would save us a lot of code-duplication...

Looking at the definition of wrap-program in (guix build utils) there is
code that checks if the wrapper already exists; if it does it should
append the new environment variable definitions to the existing
wrapper.  It looks like this doesn’t work reliably.

If someone could figure out why that is we could fix this in the next
core-updates cycle.

-- 
Ricardo




^ permalink raw reply	[flat|nested] 16+ messages in thread

* [bug#43249] bug#43151: Resolve Calibre run-time dependency
       [not found]     ` <20200908201144.GA25269@jurong>
@ 2020-09-09  8:38       ` Prafulla Giri
  0 siblings, 0 replies; 16+ messages in thread
From: Prafulla Giri @ 2020-09-09  8:38 UTC (permalink / raw)
  To: Andreas Enge; +Cc: 42885, 43249-done, Brendan Tildesley, 43151-done

[-- Attachment #1: Type: text/plain, Size: 162 bytes --]

I see. Thank you for the update.

Hopefully (wrap-program) will be fixed soon. That should save us a lot of
code-duplication.

Congratulations, Mr. Tildesley! (:

[-- Attachment #2: Type: text/html, Size: 246 bytes --]

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [bug#43249]
  2020-09-08 19:57   ` [bug#43249] Ricardo Wurmus
@ 2020-09-10 12:46     ` Brendan Tildesley
  2020-09-10 13:22       ` [bug#43249] Ricardo Wurmus
  0 siblings, 1 reply; 16+ messages in thread
From: Brendan Tildesley @ 2020-09-10 12:46 UTC (permalink / raw)
  To: Ricardo Wurmus; +Cc: pratheblackdiamond, 43249

On 9/9/20 5:57 am, Ricardo Wurmus wrote:
> Prafulla Giri <pratheblackdiamond@gmail.com> writes:
>
>> I wonder.... perhaps it'd be better altogether if the (wrap-program)
>> procedure could be re-written to not make ..*.real.real programs...? That
>> would save us a lot of code-duplication...
> Looking at the definition of wrap-program in (guix build utils) there is
> code that checks if the wrapper already exists; if it does it should
> append the new environment variable definitions to the existing
> wrapper.  It looks like this doesn’t work reliably.
>
> If someone could figure out why that is we could fix this in the next
> core-updates cycle.
>
When given "foo", wrap-program checks if ".foo-real" exists and thus 
concludes that "foo" is a wrapper and appends, however, despite the fact 
that the wrapper? procedure exists, it is not actually used at any time 
to check if ".foo-real" its self was passed to wrap-program. Therefore 
it happily wraps wrappers if it is given one. For example 
glib-or-gtk-build-system uses (find-files bindir ".*") to find files to 
pass to wrap-program. This ".*" regular expression matches hidden 
dotfiles. I copy-pasted everything to make a new build system and added 
an error for (wrapper? prog) and it exposed this. changingI guess then 
we should patch wrap-program to add

(when (wrapper? prog)
     (error (string-append prog " is a wrapper. Refusing to wrap.")))

at the start.

Then fix all uses of wrap-program so that they dont recieve -real files.

In glib-or-gtk-build-system.scm, chanding bin-list to:

(bin-list     (filter (lambda (file) (not (wrapper? file)))
                                      (append (find-files bindir ".*")
                                              (find-files libexecdir 
".*"))))

seems to fix it, at least in the case of gedit, a program that currently 
produces a nested wrapper. Ill play with it more tomorrow.





^ permalink raw reply	[flat|nested] 16+ messages in thread

* [bug#43249]
  2020-09-10 12:46     ` [bug#43249] Brendan Tildesley
@ 2020-09-10 13:22       ` Ricardo Wurmus
  2020-09-11  8:18         ` [bug#43249] Brendan Tildesley
  0 siblings, 1 reply; 16+ messages in thread
From: Ricardo Wurmus @ 2020-09-10 13:22 UTC (permalink / raw)
  To: Brendan Tildesley; +Cc: pratheblackdiamond, 43249


Brendan Tildesley <mail@brendan.scot> writes:

> I guess then we should patch wrap-program to add
>
> (when (wrapper? prog)
>     (error (string-append prog " is a wrapper. Refusing to wrap.")))

Should it really refuse to wrap or *add* its variables to the existing
wrapper?

-- 
Ricardo




^ permalink raw reply	[flat|nested] 16+ messages in thread

* [bug#43249]
  2020-09-10 13:22       ` [bug#43249] Ricardo Wurmus
@ 2020-09-11  8:18         ` Brendan Tildesley
  2020-09-11  8:38           ` [bug#43249] Ricardo Wurmus
  0 siblings, 1 reply; 16+ messages in thread
From: Brendan Tildesley @ 2020-09-11  8:18 UTC (permalink / raw)
  To: Ricardo Wurmus; +Cc: pratheblackdiamond, 43249

[-- Attachment #1: Type: text/plain, Size: 765 bytes --]

On 10/9/20 11:22 pm, Ricardo Wurmus wrote:
> Brendan Tildesley <mail@brendan.scot> writes:
>
>> I guess then we should patch wrap-program to add
>>
>> (when (wrapper? prog)
>>      (error (string-append prog " is a wrapper. Refusing to wrap.")))
> Should it really refuse to wrap or *add* its variables to the existing
> wrapper?
>
If there is a /bin/foo and /bin/.foo-real created by wrap-program, and 
we are to run another wrap-program phase, under what circumstances would 
it /not/ be a mistake to call (wrap-program "/bin/.foo-real") instead of 
(wrap-program "/bin/foo")? And if the first one was called, probably it 
was because find-files was used and the packager didn't realise it was 
happening, and therefore it will be double-wrapped, like gedit is.


[-- Attachment #2: Type: text/html, Size: 1345 bytes --]

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [bug#43249]
  2020-09-11  8:18         ` [bug#43249] Brendan Tildesley
@ 2020-09-11  8:38           ` Ricardo Wurmus
  2020-09-12 11:33             ` [bug#43249] Brendan Tildesley
  0 siblings, 1 reply; 16+ messages in thread
From: Ricardo Wurmus @ 2020-09-11  8:38 UTC (permalink / raw)
  To: Brendan Tildesley; +Cc: Prafulla Giri, 43249


Brendan Tildesley <mail@brendan.scot> writes:

> On 10/9/20 11:22 pm, Ricardo Wurmus wrote:
>> Brendan Tildesley <mail@brendan.scot> writes:
>>
>>> I guess then we should patch wrap-program to add
>>>
>>> (when (wrapper? prog)
>>>      (error (string-append prog " is a wrapper. Refusing to wrap.")))
>> Should it really refuse to wrap or *add* its variables to the existing
>> wrapper?
>>
> If there is a /bin/foo and /bin/.foo-real created by wrap-program, and
> we are to run another wrap-program phase, under what circumstances
> would it /not/ be a mistake to call (wrap-program "/bin/.foo-real")
> instead of (wrap-program "/bin/foo")? And if the first one was called,
> probably it was because find-files was used and the packager didn't
> realise it was happening, and therefore it will be double-wrapped,
> like gedit is.

Oh, yes, that would be a mistake.

-- 
Ricardo




^ permalink raw reply	[flat|nested] 16+ messages in thread

* [bug#43249]
  2020-09-11  8:38           ` [bug#43249] Ricardo Wurmus
@ 2020-09-12 11:33             ` Brendan Tildesley
  2020-09-12 12:21               ` [bug#43249] Ricardo Wurmus
  0 siblings, 1 reply; 16+ messages in thread
From: Brendan Tildesley @ 2020-09-12 11:33 UTC (permalink / raw)
  To: Ricardo Wurmus; +Cc: Ludovic Courtès, 43249

On 11/9/20 6:38 pm, Ricardo Wurmus wrote:
> Brendan Tildesley <mail@brendan.scot> writes:
>
>> On 10/9/20 11:22 pm, Ricardo Wurmus wrote:
>>> Brendan Tildesley <mail@brendan.scot> writes:
>>>
>>>> I guess then we should patch wrap-program to add
>>>>
>>>> (when (wrapper? prog)
>>>>       (error (string-append prog " is a wrapper. Refusing to wrap.")))
>>> Should it really refuse to wrap or *add* its variables to the existing
>>> wrapper?
>>>
>> If there is a /bin/foo and /bin/.foo-real created by wrap-program, and
>> we are to run another wrap-program phase, under what circumstances
>> would it /not/ be a mistake to call (wrap-program "/bin/.foo-real")
>> instead of (wrap-program "/bin/foo")? And if the first one was called,
>> probably it was because find-files was used and the packager didn't
>> realise it was happening, and therefore it will be double-wrapped,
>> like gedit is.
> Oh, yes, that would be a mistake.
>
While we're at it, what do you think about changing the moved file from 
/bin/.foo-real into /bin/.real/foo, or maybe it would have to go up a 
directory and go into /.bin-real/foo. That would mean all these dot 
files wouldn't pollute PATH and appear in things like bemenu, or in 
window titles. Also pkill should be able to kill correctly based on the 
name. Would it break more things somehow?





^ permalink raw reply	[flat|nested] 16+ messages in thread

* [bug#43249]
  2020-09-12 11:33             ` [bug#43249] Brendan Tildesley
@ 2020-09-12 12:21               ` Ricardo Wurmus
  0 siblings, 0 replies; 16+ messages in thread
From: Ricardo Wurmus @ 2020-09-12 12:21 UTC (permalink / raw)
  To: Brendan Tildesley; +Cc: Ludovic Courtès, 43249


Brendan Tildesley <mail@brendan.scot> writes:

> On 11/9/20 6:38 pm, Ricardo Wurmus wrote:
>> Brendan Tildesley <mail@brendan.scot> writes:
>>
>>> On 10/9/20 11:22 pm, Ricardo Wurmus wrote:
>>>> Brendan Tildesley <mail@brendan.scot> writes:
>>>>
>>>>> I guess then we should patch wrap-program to add
>>>>>
>>>>> (when (wrapper? prog)
>>>>>       (error (string-append prog " is a wrapper. Refusing to wrap.")))
>>>> Should it really refuse to wrap or *add* its variables to the existing
>>>> wrapper?
>>>>
>>> If there is a /bin/foo and /bin/.foo-real created by wrap-program, and
>>> we are to run another wrap-program phase, under what circumstances
>>> would it /not/ be a mistake to call (wrap-program "/bin/.foo-real")
>>> instead of (wrap-program "/bin/foo")? And if the first one was called,
>>> probably it was because find-files was used and the packager didn't
>>> realise it was happening, and therefore it will be double-wrapped,
>>> like gedit is.
>> Oh, yes, that would be a mistake.
>>
> While we're at it, what do you think about changing the moved file
> from /bin/.foo-real into /bin/.real/foo, or maybe it would have to go
> up a directory and go into /.bin-real/foo. That would mean all these
> dot files wouldn't pollute PATH and appear in things like bemenu, or
> in window titles. Also pkill should be able to kill correctly based on
> the name. Would it break more things somehow?

I’d rather not move these things to other directories because some
applications are very sensitive to relative location.

I think we should be using “wrap-script” where possible (because many
wrapped applications really are scripts) and think of another in-place
wrapping mechanism for binaries.

-- 
Ricardo




^ permalink raw reply	[flat|nested] 16+ messages in thread

* [bug#43249]
  2020-09-08 12:22 ` [bug#43249] Prafulla Giri
  2020-09-08 13:38   ` [bug#43249] Brendan Tildesley
  2020-09-08 19:57   ` [bug#43249] Ricardo Wurmus
@ 2020-09-13 12:43   ` Brendan Tildesley
  2020-09-15 11:50     ` [bug#43249] Prafulla Giri
  2 siblings, 1 reply; 16+ messages in thread
From: Brendan Tildesley @ 2020-09-13 12:43 UTC (permalink / raw)
  To: Prafulla Giri; +Cc: Andreas Enge, 43249

[-- Attachment #1: Type: text/plain, Size: 1270 bytes --]

On 8/9/20 10:22 pm, Prafulla Giri wrote:
> I see.
>
> Yes, it does make sense now why you chose to replace the 'wrap phase.
>
> I wonder.... perhaps it'd be better altogether if the (wrap-program) 
> procedure could be re-written to not make ..*.real.real programs...? 
> That would save us a lot of code-duplication...

I have come to understand wrap-program a little better and I realised 
your patch could have actually been fixed in a better way than I did. 
The issue is with the part of your code that runs


(find-files "." ".*")


This is what matches all the .calibre-real files

If instead of that, it was:

(find-files "." (lambda (file stat) (not (wrapper? file))))

or

(find-files "." (lambda (file stat) (not (string-prefix "." (basename 
file))))

It should avoid double wrapping. An even simpler way would have been to 
use (add-before 'wrap ..., instead of (add-after 'wrap ...

If you are still interested, feel free to make a patch overwriting mine 
to use this more correct method, instead of where i duplicated the wrap 
PYTHONPATH bit.

The fact that this happened is a bug though. I created some patches I 
think fix this for core-updates. It would have made your original patch 
error and force you to fix it: https://issues.guix.gnu.org/43367


[-- Attachment #2: Type: text/html, Size: 2151 bytes --]

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [bug#43249]
  2020-09-13 12:43   ` [bug#43249] Brendan Tildesley
@ 2020-09-15 11:50     ` Prafulla Giri
  2020-09-18 13:26       ` [bug#43249] Prafulla Giri
  0 siblings, 1 reply; 16+ messages in thread
From: Prafulla Giri @ 2020-09-15 11:50 UTC (permalink / raw)
  To: Brendan Tildesley; +Cc: 43249

[-- Attachment #1: Type: text/plain, Size: 1604 bytes --]

Dear Mr. Tildesley,

I have sent in a patch to do as you've suggested.
https://issues.guix.gnu.org/43419

Thank you very much.

I will remember this trick of not wrapping wrappers from now on. Thank you
very much.

On Sun, Sep 13, 2020 at 6:28 PM Brendan Tildesley <mail@brendan.scot> wrote:

> On 8/9/20 10:22 pm, Prafulla Giri wrote:
>
> I see.
>
> Yes, it does make sense now why you chose to replace the 'wrap phase.
>
> I wonder.... perhaps it'd be better altogether if the (wrap-program)
> procedure could be re-written to not make ..*.real.real programs...? That
> would save us a lot of code-duplication...
>
> I have come to understand wrap-program a little better and I realised your
> patch could have actually been fixed in a better way than I did. The issue
> is with the part of your code that runs
>
>
> (find-files "." ".*")
>
>
> This is what matches all the .calibre-real files
> If instead of that, it was:
>
> (find-files "." (lambda (file stat) (not (wrapper? file))))
>
> or
>
> (find-files "." (lambda (file stat) (not (string-prefix "." (basename
> file))))
> It should avoid double wrapping. An even simpler way would have been to
> use (add-before 'wrap ..., instead of (add-after 'wrap ...
>
> If you are still interested, feel free to make a patch overwriting mine to
> use this more correct method, instead of where i duplicated the wrap
> PYTHONPATH bit.
>
> The fact that this happened is a bug though. I created some patches I
> think fix this for core-updates. It would have made your original patch
> error and force you to fix it: https://issues.guix.gnu.org/43367
>
>

[-- Attachment #2: Type: text/html, Size: 2665 bytes --]

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [bug#43249]
  2020-09-15 11:50     ` [bug#43249] Prafulla Giri
@ 2020-09-18 13:26       ` Prafulla Giri
  0 siblings, 0 replies; 16+ messages in thread
From: Prafulla Giri @ 2020-09-18 13:26 UTC (permalink / raw)
  To: Brendan Tildesley; +Cc: 43249

[-- Attachment #1: Type: text/plain, Size: 1864 bytes --]

Mr. Tildesley,

The patch has just been merged: https://issues.guix.gnu.org/43419#1

Thank you for your guidance!

On Tue, Sep 15, 2020 at 5:35 PM Prafulla Giri <pratheblackdiamond@gmail.com>
wrote:

> Dear Mr. Tildesley,
>
> I have sent in a patch to do as you've suggested.
> https://issues.guix.gnu.org/43419
>
> Thank you very much.
>
> I will remember this trick of not wrapping wrappers from now on. Thank you
> very much.
>
> On Sun, Sep 13, 2020 at 6:28 PM Brendan Tildesley <mail@brendan.scot>
> wrote:
>
>> On 8/9/20 10:22 pm, Prafulla Giri wrote:
>>
>> I see.
>>
>> Yes, it does make sense now why you chose to replace the 'wrap phase.
>>
>> I wonder.... perhaps it'd be better altogether if the (wrap-program)
>> procedure could be re-written to not make ..*.real.real programs...? That
>> would save us a lot of code-duplication...
>>
>> I have come to understand wrap-program a little better and I realised
>> your patch could have actually been fixed in a better way than I did. The
>> issue is with the part of your code that runs
>>
>>
>> (find-files "." ".*")
>>
>>
>> This is what matches all the .calibre-real files
>> If instead of that, it was:
>>
>> (find-files "." (lambda (file stat) (not (wrapper? file))))
>>
>> or
>>
>> (find-files "." (lambda (file stat) (not (string-prefix "." (basename
>> file))))
>> It should avoid double wrapping. An even simpler way would have been to
>> use (add-before 'wrap ..., instead of (add-after 'wrap ...
>>
>> If you are still interested, feel free to make a patch overwriting mine
>> to use this more correct method, instead of where i duplicated the wrap
>> PYTHONPATH bit.
>>
>> The fact that this happened is a bug though. I created some patches I
>> think fix this for core-updates. It would have made your original patch
>> error and force you to fix it: https://issues.guix.gnu.org/43367
>>
>>

[-- Attachment #2: Type: text/html, Size: 3282 bytes --]

^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2020-09-18 13:27 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-09-06 18:32 [bug#43249] Resolve Calibre run-time dependency Prafulla Giri
2020-09-06 20:09 ` Jonathan Brielmaier
2020-09-07  8:12 ` Brendan Tildesley
     [not found]   ` <6492c3cc-07e0-b56b-ea72-99d403770755@brendan.scot>
     [not found]     ` <20200908201144.GA25269@jurong>
2020-09-09  8:38       ` [bug#43249] bug#43151: " Prafulla Giri
2020-09-08 12:22 ` [bug#43249] Prafulla Giri
2020-09-08 13:38   ` [bug#43249] Brendan Tildesley
2020-09-08 19:57   ` [bug#43249] Ricardo Wurmus
2020-09-10 12:46     ` [bug#43249] Brendan Tildesley
2020-09-10 13:22       ` [bug#43249] Ricardo Wurmus
2020-09-11  8:18         ` [bug#43249] Brendan Tildesley
2020-09-11  8:38           ` [bug#43249] Ricardo Wurmus
2020-09-12 11:33             ` [bug#43249] Brendan Tildesley
2020-09-12 12:21               ` [bug#43249] Ricardo Wurmus
2020-09-13 12:43   ` [bug#43249] Brendan Tildesley
2020-09-15 11:50     ` [bug#43249] Prafulla Giri
2020-09-18 13:26       ` [bug#43249] Prafulla Giri

unofficial mirror of guix-patches@gnu.org 

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://yhetil.org/guix-patches/1 guix-patches/git/1.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 guix-patches guix-patches/ https://yhetil.org/guix-patches \
		guix-patches@gnu.org
	public-inbox-index guix-patches

Example config snippet for mirrors.
Newsgroup available over NNTP:
	nntp://news.yhetil.org/yhetil.gnu.guix.patches


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git