Resolve Calibre run-time dependency

DoneSubmitted by Prafulla Giri.
Details
5 participants
  • Andreas Enge
  • Jonathan Brielmaier
  • Brendan Tildesley
  • Prafulla Giri
  • Ricardo Wurmus
Owner
unassigned
Severity
normal
P
P
Prafulla Giri wrote on 6 Sep 20:32 +0200
(address . guix-patches@gnu.org)
CAFw+=j010xqQencD-4RiGBswmkK9HAyxnnqnQ2NN_nHkZbtGew@mail.gmail.com
Esteemed maintainers,
Currently, Calibre can't open .epub files unless `qtwebengine` package isalso 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 madeavailable to all Calibre binaries via QTWEBENGINE_PATH set during a new'wrap-program phase introduced with the patch.
Thank you
Attachment: file
From cec0d8adf456eff2201fd89bb1fde6cba9d6fa77 Mon Sep 17 00:00:00 2001From: Prafulla Giri <pratheblackdiamond@gmail.com>Date: Sun, 6 Sep 2020 23:57:14 +0545Subject: [PATCH] gnu: calibre: make QtWebEngineProcess available during runtime
* gnu/packages/ebook.scm [arguments]: Add new phase 'wrap-programto make QtWebEngineProcess available to the binaries during run-time with QTWEBENGINEPROCESS_PATH.--- gnu/packages/ebook.scm | 16 ++++++++++++++++ 1 file changed, 16 insertions(+)
Toggle diff (29 lines)diff --git a/gnu/packages/ebook.scm b/gnu/packages/ebook.scmindex 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
J
J
Jonathan Brielmaier wrote on 6 Sep 22:09 +0200
90c46d1c-9d4f-3f6d-cf33-2920fdbafdb2@web.de
Brendan already proposed a patch for this problem in his series updatingcalibre to 4.22.0 see http://issues.guix.gnu.org/42885#4
Sorry for the duplicated effort, but maybe you can chime in there. Itseems so that there is some work left to get the update merged...
On 06.09.20 20:32, Prafulla Giri wrote:
Toggle quote (13 lines)> 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>
B
B
Brendan Tildesley wrote on 7 Sep 10:12 +0200
Re: Resolve Calibre run-time dependency
a99e52ae-decc-0396-6d4e-d3dff56dd5d2@brendan.scot
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
P
P
Prafulla Giri wrote on 8 Sep 14:22 +0200
(address . 43249@debbugs.gnu.org)(address . mail@brendan.scot)
CAFw+=j1b0cu6mXcdXdVdYkQT7_OswTYVexWCM3iQsvhiWaKSZQ@mail.gmail.com
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...? Thatwould save us a lot of code-duplication...
Attachment: file
B
B
Brendan Tildesley wrote on 8 Sep 15:38 +0200
cef435d2-ab48-96b8-6fb2-0e757c30f44e@brendan.scot
On 8/9/20 10:22 pm, Prafulla Giri wrote:
Toggle quote (8 lines)> 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.
R
R
Ricardo Wurmus wrote on 8 Sep 21:57 +0200
Re: [bug#43249]
(name . Prafulla Giri)(address . pratheblackdiamond@gmail.com)
87d02wca8d.fsf@elephly.net
Prafulla Giri <pratheblackdiamond@gmail.com> writes:
Toggle quote (4 lines)> 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 iscode that checks if the wrapper already exists; if it does it shouldappend the new environment variable definitions to the existingwrapper. It looks like this doesn’t work reliably.
If someone could figure out why that is we could fix this in the nextcore-updates cycle.
-- Ricardo
A
A
Andreas Enge wrote on 8 Sep 22:11 +0200
Re: bug#43151: Resolve Calibre run-time dependency
(name . Brendan Tildesley)(address . mail@brendan.scot)
20200908201144.GA25269@jurong
Hello,
On Mon, Sep 07, 2020 at 06:15:15PM +1000, Brendan Tildesley wrote:
Toggle quote (8 lines)> 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.
thanks to both of your for your patches! I just pushed Brendan's, whichwraps only once at the price of copy-pasting from another package. I confirmthat it works, also to click on an epub file from within calibre. Closingthe two bugs asking about qtwebengine.
Concerning the update, is mathjax a required input now, or could we justleave it out and update nevertheless?
Andreas
Closed
P
P
Prafulla Giri wrote on 9 Sep 10:38 +0200
(name . Andreas Enge)(address . andreas@enge.fr)
CAFw+=j0McVLzs-vxeB19Je91Npq7G+AS3X2WZmiGY7fzB8UWTA@mail.gmail.com
I see. Thank you for the update.
Hopefully (wrap-program) will be fixed soon. That should save us a lot ofcode-duplication.
Congratulations, Mr. Tildesley! (:
Attachment: file
Closed
B
B
Brendan Tildesley wrote on 10 Sep 14:46 +0200
Re: [bug#43249]
(name . Ricardo Wurmus)(address . rekado@elephly.net)
330470a6-e6d2-e679-32ac-a308d89a5e92@brendan.scot
On 9/9/20 5:57 am, Ricardo Wurmus wrote:
Toggle quote (13 lines)> 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.
R
R
Ricardo Wurmus wrote on 10 Sep 15:22 +0200
(name . Brendan Tildesley)(address . mail@brendan.scot)
87tuw54vhy.fsf@elephly.net
Brendan Tildesley <mail@brendan.scot> writes:
Toggle quote (5 lines)> 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 existingwrapper?
-- Ricardo
B
B
Brendan Tildesley wrote on 11 Sep 10:18 +0200
(name . Ricardo Wurmus)(address . rekado@elephly.net)
a8557b34-6258-0ce9-60e5-937d52caf038@brendan.scot
On 10/9/20 11:22 pm, Ricardo Wurmus wrote:
Toggle quote (9 lines)> 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: file
R
R
Ricardo Wurmus wrote on 11 Sep 10:38 +0200
(name . Brendan Tildesley)(address . mail@brendan.scot)
87een83dy8.fsf@elephly.net
Brendan Tildesley <mail@brendan.scot> writes:
Toggle quote (18 lines)> 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
B
B
Brendan Tildesley wrote on 12 Sep 13:33 +0200
(name . Ricardo Wurmus)(address . rekado@elephly.net)
00cb09fe-a5bb-2713-6683-bbda733f4839@brendan.scot
On 11/9/20 6:38 pm, Ricardo Wurmus wrote:
Toggle quote (21 lines)> 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?
R
R
Ricardo Wurmus wrote on 12 Sep 14:21 +0200
(name . Brendan Tildesley)(address . mail@brendan.scot)
877dsz18yp.fsf@elephly.net
Brendan Tildesley <mail@brendan.scot> writes:
Toggle quote (29 lines)> 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 someapplications are very sensitive to relative location.
I think we should be using “wrap-script” where possible (because manywrapped applications really are scripts) and think of another in-placewrapping mechanism for binaries.
-- Ricardo
B
B
Brendan Tildesley wrote on 13 Sep 14:43 +0200
Re:
(name . Prafulla Giri)(address . pratheblackdiamond@gmail.com)
dd95373c-24cb-44ab-6771-c14ed0da11b2@brendan.scot
On 8/9/20 10:22 pm, Prafulla Giri wrote:
Toggle quote (8 lines)> 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: file
P
P
Prafulla Giri wrote on 15 Sep 13:50 +0200
(name . Brendan Tildesley)(address . mail@brendan.scot)(address . 43249@debbugs.gnu.org)
CAFw+=j15_YoTP5WT3gU+DN9WGyMZOeyn6_xKvkp8B4twAxD8Sg@mail.gmail.com
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 youvery much.
On Sun, Sep 13, 2020 at 6:28 PM Brendan Tildesley <mail@brendan.scot> wrote:
Toggle quote (39 lines)> 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: file
P
P
Prafulla Giri wrote on 18 Sep 15:26 +0200
(name . Brendan Tildesley)(address . mail@brendan.scot)(address . 43249@debbugs.gnu.org)
CAFw+=j24Rpq8cQex0_SoqmFgsM_yQzrjNeDYv80QrYyDkHSUUQ@mail.gmail.com
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:
Toggle quote (52 lines)> 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: file
?