GNU bug report logs - #26500
[PATCH] gnu: Add mcomix.

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 26500 in the body.
You can then email your comments to 26500 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: nee <nee <at> cock.li>
To: guix-patches <at> gnu.org
Subject: [PATCH] gnu: Add mcomix.
Date: Fri, 14 Apr 2017 19:50:52 +0200

[Message part 1 (text/plain, inline)]

Hello, here is mcomix.
I tested it with zip, CBZ, 7zip pdf files, and a directory with normal
images, they all work fine.

I also tried unrar, but couldn't extract any archive with it, neither
through mcomix, nor manually through the commandline tool, so this might
be a separate problem.

[0001-gnu-Add-mcomix.patch (text/x-patch, attachment)]

Message #8 received at 26500 <at> debbugs.gnu.org (full text, mbox):

From: Arun Isaac <arunisaac <at> systemreboot.net>
To: 26500 <at> debbugs.gnu.org
Subject: Re: bug#26500: [PATCH] gnu: Add mcomix.
Date: Sat, 15 Apr 2017 14:06:22 +0530

> Hello, here is mcomix.

Thanks for the patch!

> I tested it with zip, CBZ, 7zip pdf files, and a directory with normal
> images, they all work fine.
>
> I also tried unrar, but couldn't extract any archive with it, neither
> through mcomix, nor manually through the commandline tool, so this might
> be a separate problem.

I'm yet to actually test running the application. Will do so and get
back in a while. Meanwhile, other changes.

> +    ;; Python 2.5 or newer (Python 3 and up is not supported)
> +    (arguments `(#:python ,python-2))

You can remove the comment. It's more or less obvious from the
'arguments' section that mcomix needs python 2.

> +    (propagated-inputs `(("python2-pygtk" ,python2-pygtk)
> +                         ("gtk+" ,gtk+)
> +                         ("python2-pillow" ,python2-pillow)))

These are inputs not propagated-inputs. propagated-inputs are only
required for python libraries. For python applications such as mcomix,
the executable gets wrapped in a script which sets the correct
PYTHONPATH variable. Look at $out/bin/mcomix and $out/bin/.mcomix-real

Also, is the gtk+ dependency required? mcomix seems to build and run
fine on my system without it.

> +    (description "A customizable image viewer that specializes as
> +a comic and manga reader.  It supports a variety of container formats
> +including CBR, CBZ, CB7, CBT, LHA.

Please make this "MComix is a ... ". See
https://www.gnu.org/software/guix/manual/html_node/Synopses-and-Descriptions.html

> +    (license license:gpl2)))

This should be license:gpl2+. When no license version is explicitly
specified in the source headers, it is of the "or later" variant. See
https://lists.gnu.org/archive/html/guix-devel/2017-02/msg00026.html

Also, add a copyright header for yourself at the top of the source file
(gnu/packages/image-viewers.scm in this case).

Message #11 received at 26500 <at> debbugs.gnu.org (full text, mbox):

From: ng0 <contact.ng0 <at> cryptolab.net>
To: Arun Isaac <arunisaac <at> systemreboot.net>
Cc: 26500 <at> debbugs.gnu.org
Subject: Re: bug#26500: [PATCH] gnu: Add mcomix.
Date: Sat, 15 Apr 2017 09:09:32 +0000

Arun Isaac transcribed 1.8K bytes:
> 
> > Hello, here is mcomix.
> 
> Thanks for the patch!
> 
> > I tested it with zip, CBZ, 7zip pdf files, and a directory with normal
> > images, they all work fine.
> >
> > I also tried unrar, but couldn't extract any archive with it, neither
> > through mcomix, nor manually through the commandline tool, so this might
> > be a separate problem.
> 
> I'm yet to actually test running the application. Will do so and get
> back in a while. Meanwhile, other changes.
> 
> > +    ;; Python 2.5 or newer (Python 3 and up is not supported)
> > +    (arguments `(#:python ,python-2))
> 
> You can remove the comment. It's more or less obvious from the
> 'arguments' section that mcomix needs python 2.

No, I think a short comment behind or above the #:python keyword is
necessary, in case someone in the future wonders "Hey, why do we package
this for python 2 only and not for python 3" even if both languages are
developed in parallel.
 
> > +    (propagated-inputs `(("python2-pygtk" ,python2-pygtk)
> > +                         ("gtk+" ,gtk+)
> > +                         ("python2-pillow" ,python2-pillow)))
> 
> These are inputs not propagated-inputs. propagated-inputs are only
> required for python libraries. For python applications such as mcomix,
> the executable gets wrapped in a script which sets the correct
> PYTHONPATH variable. Look at $out/bin/mcomix and $out/bin/.mcomix-real
> 
> Also, is the gtk+ dependency required? mcomix seems to build and run
> fine on my system without it.
> 
> > +    (description "A customizable image viewer that specializes as
> > +a comic and manga reader.  It supports a variety of container formats
> > +including CBR, CBZ, CB7, CBT, LHA.
> 
> Please make this "MComix is a ... ". See
> https://www.gnu.org/software/guix/manual/html_node/Synopses-and-Descriptions.html
> 
> > +    (license license:gpl2)))
> 
> This should be license:gpl2+. When no license version is explicitly
> specified in the source headers, it is of the "or later" variant. See
> https://lists.gnu.org/archive/html/guix-devel/2017-02/msg00026.html
> 
> Also, add a copyright header for yourself at the top of the source file
> (gnu/packages/image-viewers.scm in this case).
> 
> 
> 

-- 
PGP and more: https://people.pragmatique.xyz/ng0/

Message #14 received at 26500 <at> debbugs.gnu.org (full text, mbox):

From: Arun Isaac <arunisaac <at> systemreboot.net>
To: 26500 <at> debbugs.gnu.org
Subject: Re: bug#26500: [PATCH] gnu: Add mcomix.
Date: Sun, 16 Apr 2017 14:23:02 +0530

> No, I think a short comment behind or above the #:python keyword is
> necessary, in case someone in the future wonders "Hey, why do we package
> this for python 2 only and not for python 3" even if both languages are
> developed in parallel.

Ok, it doesn't hurt to keep the comment. Let's keep it.

> +    (description "A customizable image viewer that specializes as
> +a comic and manga reader.  It supports a variety of container formats
> +including CBR, CBZ, CB7, CBT, LHA.
> +For 7z support install the p7zip package.
> +For rar support install the unrar package.
> +For pdf support install the mupdf package.")

Instead of mentioning p7zip, unrar and mupdf in the description, it
would be better if we could make them inputs and patch all references to
the executables in the source with their absolute paths. Could you try this?

Message #17 received at 26500 <at> debbugs.gnu.org (full text, mbox):

From: nee <nee <at> cock.li>
To: Arun Isaac <arunisaac <at> systemreboot.net>, 26500 <at> debbugs.gnu.org
Subject: Re: bug#26500: [PATCH] gnu: Add mcomix.
Date: Mon, 17 Apr 2017 06:45:49 +0200

[Message part 1 (text/plain, inline)]

Am 16.04.2017 um 10:53 schrieb Arun Isaac:
> Instead of mentioning p7zip, unrar and mupdf in the description, it
> would be better if we could make them inputs and patch all references to
> the executables in the source with their absolute paths. Could you try this?
I could try that, but mupdf is a pretty heavy dependency. guix size says
it's self-size is 208MiB and total-size is 491MiB.
I personally never needed it with mcomix, so I like that it is optional.
Is there some way to define a package with optional inputs?

For now I'm appending my WIP that should address your previous remarks.

[0001-gnu-Add-mcomix.patch (text/x-patch, attachment)]

Message #20 received at 26500 <at> debbugs.gnu.org (full text, mbox):

From: Arun Isaac <arunisaac <at> systemreboot.net>
To: 26500 <at> debbugs.gnu.org
Subject: Re: bug#26500: [PATCH] gnu: Add mcomix.
Date: Tue, 18 Apr 2017 00:51:31 +0530

>> Instead of mentioning p7zip, unrar and mupdf in the description, it
>> would be better if we could make them inputs and patch all references to
>> the executables in the source with their absolute paths. Could you try this?
> I could try that, but mupdf is a pretty heavy dependency. guix size says
> it's self-size is 208MiB and total-size is 491MiB.
> I personally never needed it with mcomix, so I like that it is optional.
> Is there some way to define a package with optional inputs?

As far as I understand, there is no way to specify optional inputs
(yet?). Here are two of many threads discussing this.

https://lists.gnu.org/archive/html/guix-devel/2014-11/msg00549.html
https://lists.gnu.org/archive/html/guix-devel/2015-05/msg00379.html

If mupdf is a heavy dependency and it is not commonly needed, you can
drop it and just package with 7z and rar as dependencies.

> +    (arguments `(#:python ,python-2))
> +    (inputs `(("python2-pygtk" ,python2-pygtk)
> +              ("python2-pillow" ,python2-pillow)))

Sort these inputs alphabetically, as I mentioned in the crawl thread.

LGTM, otherwise! Please send an updated patch. Thanks!

Message #23 received at 26500 <at> debbugs.gnu.org (full text, mbox):

From: nee <nee <at> cock.li>
To: 26500 <at> debbugs.gnu.org
Subject: Re: bug#26500: [PATCH] gnu: Add mcomix.
Date: Wed, 19 Apr 2017 00:09:41 +0200

[Message part 1 (text/plain, inline)]

Am 17.04.2017 um 21:21 schrieb Arun Isaac:
> 
> If mupdf is a heavy dependency and it is not commonly needed, you can
> drop it and just package with 7z and rar as dependencies.

I have successfully patched in the path for 7zip.
I init the variable that stores it with a unicode string. I make the
assumption that the store path will never contain a ' character.


I read a little bit about unrar and it's issues with mcomix.
Unrar is very old and only supports RAR2, as someone says in the mcomix
issues it is unable to unpack most rar archives created after 2002¹. On
other distro mcomix currently relies on an unfree rar package², and
there is an open ticket³ about switching to a free library like
libarchiver (which supports RAR5 and is also used by file-roller, which
works fine on GuixSD).

I tried unrar on the commandline and it would list file names, but
always fail on unpacking. When called by mcomix, it would always try to
read a password from stdin and freeze the gui.
I will omit rar support for now, since unrar is mostly useless.

> 
> Sort these inputs alphabetically, as I mentioned in the crawl thread.
> 
> LGTM, otherwise! Please send an updated patch. Thanks!
> 
Okay.

(1) https://sourceforge.net/p/mcomix/feature-requests/82/#2bc5
(2) https://sourceforge.net/p/mcomix/feature-requests/82/
(3) https://sourceforge.net/p/mcomix/feature-requests/85/

[0001-gnu-Add-mcomix.patch (text/x-patch, attachment)]

Message #26 received at 26500 <at> debbugs.gnu.org (full text, mbox):

From: Arun Isaac <arunisaac <at> systemreboot.net>
To: 26500 <at> debbugs.gnu.org
Subject: Re: bug#26500: [PATCH] gnu: Add mcomix.
Date: Wed, 19 Apr 2017 21:23:49 +0530

nee writes:

>> If mupdf is a heavy dependency and it is not commonly needed, you can
>> drop it and just package with 7z and rar as dependencies.
>
> I have successfully patched in the path for 7zip.
> I init the variable that stores it with a unicode string. I make the
> assumption that the store path will never contain a ' character.

> I read a little bit about unrar and it's issues with mcomix.
> Unrar is very old and only supports RAR2, as someone says in the mcomix
> issues it is unable to unpack most rar archives created after 2002¹. On
> other distro mcomix currently relies on an unfree rar package², and
> there is an open ticket³ about switching to a free library like
> libarchiver (which supports RAR5 and is also used by file-roller, which
> works fine on GuixSD).
>
> I tried unrar on the commandline and it would list file names, but
> always fail on unpacking. When called by mcomix, it would always try to
> read a password from stdin and freeze the gui.
> I will omit rar support for now, since unrar is mostly useless.

Ok, that's fine.

> @@ -28,6 +29,7 @@
>    #:use-module (guix packages)
>    #:use-module (guix build-system gnu)
>    #:use-module (guix build-system cmake)
> +  #:use-module (guix build-system python)
>    #:use-module (gnu packages autotools)
>    #:use-module (gnu packages algebra)
>    #:use-module (gnu packages base)
> @@ -46,6 +48,7 @@
>    #:use-module (gnu packages pkg-config)
>    #:use-module (gnu packages qt)
>    #:use-module (gnu packages xorg)
> +  #:use-module (gnu packages python)
>    #:use-module (gnu packages))

Since these modules are already in alphabetical order, it would be good
to keep them that way. Insert (gnu packages python) at an appropriate
position.

> +    (inputs `(("python2-pillow" ,python2-pillow)
> +              ("python2-pygtk" ,python2-pygtk)
> +              ("p7zip" ,p7zip)))

Please put these in alphabetical order too. That means p7zip will go on
the top.

> +    (description "MComix is a customizable image viewer that specializes as
> +a comic and manga reader.  It supports a variety of container formats
> +including CBZ, CB7, CBT, LHA.
> +CBR and rar are currently unsupported, due to unfree dependencies.
> +For optional pdf support, you can install the mupdf package.")

Can we move the information about RAR and PDF support to a comment above
the package definition, as done for the octave package?

Message #29 received at 26500 <at> debbugs.gnu.org (full text, mbox):

From: ng0 <contact.ng0 <at> cryptolab.net>
To: Arun Isaac <arunisaac <at> systemreboot.net>
Cc: 26500 <at> debbugs.gnu.org
Subject: Re: bug#26500: [PATCH] gnu: Add mcomix.
Date: Wed, 19 Apr 2017 19:54:44 +0000

Arun Isaac transcribed 2.4K bytes:
> 
> nee writes:
> 
> >> If mupdf is a heavy dependency and it is not commonly needed, you can
> >> drop it and just package with 7z and rar as dependencies.
> >
> > I have successfully patched in the path for 7zip.
> > I init the variable that stores it with a unicode string. I make the
> > assumption that the store path will never contain a ' character.
> 
> > I read a little bit about unrar and it's issues with mcomix.
> > Unrar is very old and only supports RAR2, as someone says in the mcomix
> > issues it is unable to unpack most rar archives created after 2002¹. On
> > other distro mcomix currently relies on an unfree rar package², and
> > there is an open ticket³ about switching to a free library like
> > libarchiver (which supports RAR5 and is also used by file-roller, which
> > works fine on GuixSD).
> >
> > I tried unrar on the commandline and it would list file names, but
> > always fail on unpacking. When called by mcomix, it would always try to
> > read a password from stdin and freeze the gui.
> > I will omit rar support for now, since unrar is mostly useless.

Oh... this could be why zathura-cb has massive issues with most .cbr comics
I have! I had some ideas, but thanks for explaining this issue.

> Ok, that's fine.
> 
> > @@ -28,6 +29,7 @@
> >    #:use-module (guix packages)
> >    #:use-module (guix build-system gnu)
> >    #:use-module (guix build-system cmake)
> > +  #:use-module (guix build-system python)
> >    #:use-module (gnu packages autotools)
> >    #:use-module (gnu packages algebra)
> >    #:use-module (gnu packages base)
> > @@ -46,6 +48,7 @@
> >    #:use-module (gnu packages pkg-config)
> >    #:use-module (gnu packages qt)
> >    #:use-module (gnu packages xorg)
> > +  #:use-module (gnu packages python)
> >    #:use-module (gnu packages))
> 
> Since these modules are already in alphabetical order, it would be good
> to keep them that way. Insert (gnu packages python) at an appropriate
> position.
> 
> > +    (inputs `(("python2-pillow" ,python2-pillow)
> > +              ("python2-pygtk" ,python2-pygtk)
> > +              ("p7zip" ,p7zip)))
> 
> Please put these in alphabetical order too. That means p7zip will go on
> the top.
> 
> > +    (description "MComix is a customizable image viewer that specializes as
> > +a comic and manga reader.  It supports a variety of container formats
> > +including CBZ, CB7, CBT, LHA.
> > +CBR and rar are currently unsupported, due to unfree dependencies.
> > +For optional pdf support, you can install the mupdf package.")
> 
> Can we move the information about RAR and PDF support to a comment above
> the package definition, as done for the octave package?
> 
> 
> 

-- 
PGP and more: https://people.pragmatique.xyz/ng0/

Message #32 received at 26500 <at> debbugs.gnu.org (full text, mbox):

From: ludo <at> gnu.org (Ludovic Courtès)
To: Arun Isaac <arunisaac <at> systemreboot.net>, nee <nee <at> cock.li>
Cc: 26500 <at> debbugs.gnu.org
Subject: Re: bug#26500: [PATCH] gnu: Add mcomix.
Date: Fri, 21 Apr 2017 11:00:27 +0200

Hello!

Arun Isaac <arunisaac <at> systemreboot.net> skribis:

> nee writes:

[...]

>> @@ -28,6 +29,7 @@
>>    #:use-module (guix packages)
>>    #:use-module (guix build-system gnu)
>>    #:use-module (guix build-system cmake)
>> +  #:use-module (guix build-system python)
>>    #:use-module (gnu packages autotools)
>>    #:use-module (gnu packages algebra)
>>    #:use-module (gnu packages base)
>> @@ -46,6 +48,7 @@
>>    #:use-module (gnu packages pkg-config)
>>    #:use-module (gnu packages qt)
>>    #:use-module (gnu packages xorg)
>> +  #:use-module (gnu packages python)
>>    #:use-module (gnu packages))
>
> Since these modules are already in alphabetical order, it would be good
> to keep them that way. Insert (gnu packages python) at an appropriate
> position.
>
>> +    (inputs `(("python2-pillow" ,python2-pillow)
>> +              ("python2-pygtk" ,python2-pygtk)
>> +              ("p7zip" ,p7zip)))
>
> Please put these in alphabetical order too. That means p7zip will go on
> the top.
>
>> +    (description "MComix is a customizable image viewer that specializes as
>> +a comic and manga reader.  It supports a variety of container formats
>> +including CBZ, CB7, CBT, LHA.
>> +CBR and rar are currently unsupported, due to unfree dependencies.
>> +For optional pdf support, you can install the mupdf package.")
>
> Can we move the information about RAR and PDF support to a comment above
> the package definition, as done for the octave package?

I agree with the comments, though I also think this shouldn’t block the
patch or drive nee away.  :-)

Ludo’.

Message #37 received at 26500-done <at> debbugs.gnu.org (full text, mbox):

From: Arun Isaac <arunisaac <at> systemreboot.net>
To: 26500-done <at> debbugs.gnu.org
Subject: Re: bug#26500: [PATCH] gnu: Add mcomix.
Date: Fri, 21 Apr 2017 23:27:55 +0530

> I agree with the comments, though I also think this shouldn’t block the
> patch or drive nee away.  :-)

Ok, pushed with changes! 

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.