[PATCH] gnu: simple-scan: Update to 3.19.91.

[PATCH] gnu: simple-scan: Update to 3.19.91.

[Tobias Geerinckx-Rice]

* gnu/packages/gnome.scm (simple-scan): Update to 3.19.91.
[arguments]: Disable PackageKit support irrelevant for Guix.
---

Thanks in advance for your loving corrections. :-)

T G-R

 gnu/packages/gnome.scm | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/gnu/packages/gnome.scm b/gnu/packages/gnome.scm
index c945c0e..d9a0bdb 100644
--- a/gnu/packages/gnome.scm
+++ b/gnu/packages/gnome.scm
@@ -3382,7 +3382,7 @@ USB transfers with your high-level application or system daemon.")
 (define-public simple-scan
   (package
     (name "simple-scan")
-    (version "3.17.4")
+    (version "3.19.91")
     (source (origin
               (method url-fetch)
               (uri (string-append "https://launchpad.net/simple-scan/"
@@ -3391,7 +3391,7 @@ USB transfers with your high-level application or system daemon.")
                                   version ".tar.xz"))
               (sha256
                (base32
-                "1pslbv45g01g039zj2b01k08f763kkhzqw8wwz7yh27m7bjllnx6"))))
+                "1c5glf5vxgld41w4jxfqcv17q76qnh43fawpv33hncgh8d283xkf"))))
     (build-system glib-or-gtk-build-system)
     (inputs
      `(("gtk" ,gtk+)
@@ -3409,6 +3409,14 @@ USB transfers with your high-level application or system daemon.")
        ("pkg-config" ,pkg-config)
        ("vala" ,vala)
        ("xmllint" ,libxml2)))
+    (arguments
+     '(#:configure-flags '("--disable-packagekit")
+       #:phases
+       (modify-phases %standard-phases
+         (add-after 'unpack 'clean
+                    (lambda _
+                      ;; Remove a left-over reference to PackageKit.
+                      (delete-file "src/ui.c"))))))
     (home-page "https://launchpad.net/simple-scan")
     (synopsis "Document and image scanner")
     (description "Simple Scan is an easy-to-use application, designed to let
-- 
2.7.0

Re: [PATCH] gnu: simple-scan: Update to 3.19.91.

[Leo Famulari]

On Tue, Mar 08, 2016 at 11:04:35PM +0100, Tobias Geerinckx-Rice wrote:
> * gnu/packages/gnome.scm (simple-scan): Update to 3.19.91.

Thanks for your attention to this package!

Except where necessary for the new version to work, it's best to do
updates in a separate commit from other changes.

I can apply the update myself if you say it's okay to do on its own, or
feel free to submit a revised patch set.

> [arguments]: Disable PackageKit support irrelevant for Guix.

[...]

> +    (arguments
> +     '(#:configure-flags '("--disable-packagekit")
> +       #:phases
> +       (modify-phases %standard-phases
> +         (add-after 'unpack 'clean
> +                    (lambda _
> +                      ;; Remove a left-over reference to PackageKit.
> +                      (delete-file "src/ui.c"))))))

Does 'src/ui.c' cause problems for us? I'd rather not make changes to
upstream code except when necesssary [0], at least not without
discussing it with upstream first.

[0] For example, when fixing portability issues or deleting non-free and
non-essential bundled components. The impact of the change should be
well-understood by reviewers, at the very least ;)

Re: [PATCH] gnu: simple-scan: Update to 3.19.91.

[Tobias Geerinckx-Rice]

Hullo Leo,

On 09/03/2016, Leo Famulari <leo@famulari.name> wrote:
> Except where necessary for the new version to work, it's best to do
> updates in a separate commit from other changes.
>
> I can apply the update myself if you say it's okay to do on its own, or
> feel free to submit a revised patch set.

Er, no. This whole thing is pretty — what's the eufemism — atomic:

PackageKit support is a ‘new’ upstream addition. Guix doesn't ship a
PackageKit expression. Nor would writing one add much value, since its
sole purpose is to serve as an abstraction layer for other, ‘impure’
package managers[1].

I therefore simply disabled it.

> Does 'src/ui.c' cause problems for us? I'd rather not make changes to
> upstream code except when necesssary [0], at least not without
> discussing it with upstream first.

However, a stale file (src/ui.c) in the tarball still references packagekit:

  make[1]: Entering directory
'/tmp/guix-build-simple-scan-3.19.91.drv-0/build/src'
    CC       simple_scan-ui.o
  ../../simple-scan-3.19.91/src/ui.c:28:41: fatal error: \
     packagekit-glib2/packagekit.h: No such file or directory
   #include "packagekit-glib2/packagekit.h"

Simply adding ‘make clean’ doesn't work (because that assumes we're
building in the source directory and Guix doesn't). Removing the
offending file fixes the build & seemed more clear.

> [0] [...] The impact of the change should be
> well-understood by reviewers, at the very least ;)

Would a simple s/PackageKit support/newly-introduced &/ on the commit
message make this clear? Or should I be more verbose?

Kind regards,

T G-R

[1] Simple Scan then invokes PackageKit to automagically install
missing sane backends.

Re: [PATCH] gnu: simple-scan: Update to 3.19.91.

[Tobias Geerinckx-Rice]

An addendum to the mentioning of upstream: they have been aware of the
issue for some time, but it has yet to be fixed.

The bug report[1] mentions some other workarounds, but I preferred a
more explicit approach.

Regards,

T G-R

[1] https://bugs.launchpad.net/simple-scan/+bug/1462769

Re: [PATCH] gnu: simple-scan: Update to 3.19.91.

[Leo Famulari]

On Wed, Mar 09, 2016 at 12:43:52AM +0100, Tobias Geerinckx-Rice wrote:
> Hullo Leo,
> 
> On 09/03/2016, Leo Famulari <leo@famulari.name> wrote:
> > Except where necessary for the new version to work, it's best to do
> > updates in a separate commit from other changes.
> >
> > I can apply the update myself if you say it's okay to do on its own, or
> > feel free to submit a revised patch set.
> 
> Er, no. This whole thing is pretty — what's the eufemism — atomic:
> 
> PackageKit support is a ‘new’ upstream addition. Guix doesn't ship a
> PackageKit expression. Nor would writing one add much value, since its
> sole purpose is to serve as an abstraction layer for other, ‘impure’
> package managers[1].
> 
> I therefore simply disabled it.

I think that lots of software in our distribution has unused features or
portability "goop" that doesn't apply to us, but we don't delete that
code.

> 
> > Does 'src/ui.c' cause problems for us? I'd rather not make changes to
> > upstream code except when necesssary [0], at least not without
> > discussing it with upstream first.
> 
> However, a stale file (src/ui.c) in the tarball still references packagekit:
> 
>   make[1]: Entering directory
> '/tmp/guix-build-simple-scan-3.19.91.drv-0/build/src'
>     CC       simple_scan-ui.o
>   ../../simple-scan-3.19.91/src/ui.c:28:41: fatal error: \
>      packagekit-glib2/packagekit.h: No such file or directory
>    #include "packagekit-glib2/packagekit.h"
> 
> Simply adding ‘make clean’ doesn't work (because that assumes we're
> building in the source directory and Guix doesn't). Removing the
> offending file fixes the build & seemed more clear.

So, it fails to build from source if you don't delete this file?

I still don't understand why we should delete the source file.

After searching online for "packagekit simple scan" I found the Nix
package definition for Simple Scan 3.19.91. They pass to ./configure
'--disable-packagekit'. Would that work?

https://github.com/NixOS/nixpkgs/blob/master/pkgs/applications/graphics/simple-scan/default.nix

Re: [PATCH] gnu: simple-scan: Update to 3.19.91.

[Tobias Geerinckx-Rice]

Firstly, thanks for the feedback!

On 09/03/2016, Leo Famulari <leo@famulari.name> wrote:
>> I therefore simply disabled it.
>
> I think that lots of software in our distribution has unused features or
> portability "goop" that doesn't apply to us, but we don't delete that
> code.

Running ‘./configure --disable-*’ isn't deleting code. The rest is a bugfix.

> So, it fails to build from source if you don't delete this file?

Correct.

> I still don't understand why we should delete the source file.

Because it fails to build from source if we don't.

Again, this is a stale (generated) file. Did you read the linked bug report?

There are various ways to code this, but none that don't amount to
deleting (generated) source files.[1]

> After searching online for "packagekit simple scan" I found the Nix
> package definition for Simple Scan 3.19.91. They

For the record: me.

> [...] pass to ./configure '--disable-packagekit'. Would that work?

So do ‘we’:

On Tue, Mar 08, 2016 at 11:04:35PM +0100, Tobias Geerinckx-Rice wrote:
> '(#:configure-flags '("--disable-packagekit")

But that alone is not enough. Not without removing the stale file.

Kind regards,

T G-R (nckx on #guix in about a quarter of an hour, if that helps :-)

[1] OK, ‘touch’ing random things, but that's just fooling make into
overwriting it, which is the same thing.

Re: [PATCH] gnu: simple-scan: Update to 3.19.91.

[Leo Famulari]

On Wed, Mar 09, 2016 at 01:25:04AM +0100, Tobias Geerinckx-Rice wrote:
> On 09/03/2016, Leo Famulari <leo@famulari.name> wrote:
> > [...] pass to ./configure '--disable-packagekit'. Would that work?
> 
> So do ‘we’:
> 
> On Tue, Mar 08, 2016 at 11:04:35PM +0100, Tobias Geerinckx-Rice wrote:
> > '(#:configure-flags '("--disable-packagekit")

Oops! Serves me right for trying to squeeze this review in earlier ;)

> There are various ways to code this, but none that don't amount to
> deleting (generated) source files.[1]

I didn't realize this was generated C code. In that case it's closer to
a compiled binary than source code, don't you think? Can we delete all
the generated files and rebuild them from source?

Re: [PATCH] gnu: simple-scan: Update to 3.19.91.

[Leo Famulari]

On Tue, Mar 08, 2016 at 08:39:37PM -0500, Leo Famulari wrote:
> On Wed, Mar 09, 2016 at 01:25:04AM +0100, Tobias Geerinckx-Rice wrote:
> > On 09/03/2016, Leo Famulari <leo@famulari.name> wrote:
> > > [...] pass to ./configure '--disable-packagekit'. Would that work?
> > 
> > So do ‘we’:
> > 
> > On Tue, Mar 08, 2016 at 11:04:35PM +0100, Tobias Geerinckx-Rice wrote:
> > > '(#:configure-flags '("--disable-packagekit")
> 
> Oops! Serves me right for trying to squeeze this review in earlier ;)
> 
> > There are various ways to code this, but none that don't amount to
> > deleting (generated) source files.[1]
> 
> I didn't realize this was generated C code. In that case it's closer to
> a compiled binary than source code, don't you think? Can we delete all
> the generated files and rebuild them from source?

Anyways, that is probably something to look into later. I think it makes
sense to do this update, remove that file, and include a link to the bug
report with a bit of context.

Does anyone have any objections to that plan?

Re: [PATCH] gnu: simple-scan: Update to 3.19.91.

[Ludovic Courtès]

Leo Famulari <leo@famulari.name> skribis:

> On Tue, Mar 08, 2016 at 08:39:37PM -0500, Leo Famulari wrote:
>> On Wed, Mar 09, 2016 at 01:25:04AM +0100, Tobias Geerinckx-Rice wrote:
>> > On 09/03/2016, Leo Famulari <leo@famulari.name> wrote:
>> > > [...] pass to ./configure '--disable-packagekit'. Would that work?
>> > 
>> > So do ‘we’:
>> > 
>> > On Tue, Mar 08, 2016 at 11:04:35PM +0100, Tobias Geerinckx-Rice wrote:
>> > > '(#:configure-flags '("--disable-packagekit")
>> 
>> Oops! Serves me right for trying to squeeze this review in earlier ;)
>> 
>> > There are various ways to code this, but none that don't amount to
>> > deleting (generated) source files.[1]
>> 
>> I didn't realize this was generated C code. In that case it's closer to
>> a compiled binary than source code, don't you think? Can we delete all
>> the generated files and rebuild them from source?
>
> Anyways, that is probably something to look into later. I think it makes
> sense to do this update, remove that file, and include a link to the bug
> report with a bit of context.
>
> Does anyone have any objections to that plan?

I’m not sure I fully grasped everything, but the plan looks good.  And
since it’s an update and the problem was already there, let’s not annoy
Tobias more than this.  :-)

Eventually, I agree it’ll be best to remove the generated C files,
though.

Ludo’.

Re: [PATCH] gnu: simple-scan: Update to 3.19.91.

[Leo Famulari]

On Wed, Mar 09, 2016 at 02:28:03PM +0100, Ludovic Courtès wrote:
> Leo Famulari <leo@famulari.name> skribis:
> 
> > On Tue, Mar 08, 2016 at 08:39:37PM -0500, Leo Famulari wrote:
> >> On Wed, Mar 09, 2016 at 01:25:04AM +0100, Tobias Geerinckx-Rice wrote:
> >> > On 09/03/2016, Leo Famulari <leo@famulari.name> wrote:
> >> > > [...] pass to ./configure '--disable-packagekit'. Would that work?
> >> > 
> >> > So do ‘we’:
> >> > 
> >> > On Tue, Mar 08, 2016 at 11:04:35PM +0100, Tobias Geerinckx-Rice wrote:
> >> > > '(#:configure-flags '("--disable-packagekit")
> >> 
> >> Oops! Serves me right for trying to squeeze this review in earlier ;)
> >> 
> >> > There are various ways to code this, but none that don't amount to
> >> > deleting (generated) source files.[1]
> >> 
> >> I didn't realize this was generated C code. In that case it's closer to
> >> a compiled binary than source code, don't you think? Can we delete all
> >> the generated files and rebuild them from source?
> >
> > Anyways, that is probably something to look into later. I think it makes
> > sense to do this update, remove that file, and include a link to the bug
> > report with a bit of context.
> >
> > Does anyone have any objections to that plan?
> 
> I’m not sure I fully grasped everything, but the plan looks good.  And
> since it’s an update and the problem was already there, let’s not annoy
> Tobias more than this.  :-)

Pushed with some additional context as fb9ca51130a.

Re: [PATCH] gnu: simple-scan: Update to 3.19.91.

[Danny Milosavljevic]

Note: all the toolbar icons are broken for me in simple-scan.

open("/home/dannym/.icons/default/index.theme", O_RDONLY) = -1 ENOENT (No such file or directory)
open("/run/current-system/profile/share/icons", O_RDONLY|O_NONBLOCK|O_DIRECTORY|O_CLOEXEC) = 20
stat("/gnu/store/kfb8frkg06fr4b4v6pbabyidkcpvzsqi-simple-scan-3.19.91/share/simple-scan/icons/hicolor/scalable/stock/text", 0x7ffc5f80b300) = -1 ENOENT (No such file or directory)
stat("/home/dannym/.local/share/icons/hicolor/scalable/stock/form", 0x7ffc5f80b300) = -1 ENOENT (No such file or directory)
...etcetc

Re: [PATCH] gnu: simple-scan: Update to 3.19.91.

[Danny Milosavljevic]

> Note: all the toolbar icons are broken for me in simple-scan.
> 
> open("/home/dannym/.icons/default/index.theme", O_RDONLY) = -1 ENOENT (No such file or directory)
> open("/run/current-system/profile/share/icons", O_RDONLY|O_NONBLOCK|O_DIRECTORY|O_CLOEXEC) = 20
> stat("/gnu/store/kfb8frkg06fr4b4v6pbabyidkcpvzsqi-simple-scan-3.19.91/share/simple-scan/icons/hicolor/scalable/stock/text", 0x7ffc5f80b300) = -1 ENOENT (No such file or directory)
> stat("/home/dannym/.local/share/icons/hicolor/scalable/stock/form", 0x7ffc5f80b300) = -1 ENOENT (No such file or directory)
> ...etcetc

I installed hicolor-icon-theme now. It's still the same.

Then I tried 

$ e ~/.config/gtk-3.0/settings.ini 
[Settings]
gtk-icon-theme-name = hicolor

And it's still the same...

Re: [PATCH] gnu: simple-scan: Update to 3.19.91.

[Tobias Geerinckx-Rice]

Hallo Danny,

On 13/03/2016, Danny Milosavljevic <dannym@scratchpost.org> wrote:
>> Note: all the toolbar icons are broken for me in simple-scan.

Did they break after this update? Does reverting fb9ca51 solve the problem?

I noticed no such problems while testing, and can only reproduce this
by running something like:

   ~$ XDG_DATA_DIRS= simple-scan

I'm not on GuixSD now, but shouldn't XDG_DATA_DIRS contain
/gnu/store/...-adwaita-icon-theme-.../share there as well? If not,
I'll certainly try to fix this when I am.

Kind regards,

T G-R