unofficial mirror of guix-patches@gnu.org 
 help / color / mirror / code / Atom feed
* [bug#73391] [PATCH 0/2] SANE: fix a locking bug for plustek backend
@ 2024-09-20 19:03 neox
  2024-09-20 19:03 ` [bug#73392] [PATCH 1/2] gnu: sane-backends-minimal: fix lock path " neox
                   ` (2 more replies)
  0 siblings, 3 replies; 15+ messages in thread
From: neox @ 2024-09-20 19:03 UTC (permalink / raw)
  To: 73391; +Cc: Adrien 'neox' Bourmault

From: Adrien 'neox' Bourmault <neox@gnu.org>

Hi.
While attempting to use a Canon LiDE 30 scanner, which is supported by SANE in
its current version, I noticed a malfunction with the plustek backend. No
application seemed capable of detecting the scanner, even though the 
`sane-find-scanner` command indicated its presence, and the device IDs matched
those in the plustek backend configuration.

I investigated further using the following command:

  SANE_DEBUG_SANEI_USB=128 SANE_DEBUG_PLUSTEK=255 strace -fye open,openat scanimage -L

Here is a summary of the output:

  [23:00:21.688008] [plustek] usbDev_open(auto,) - 0x1c1e5630
  [pid 1209-20241] openat(AT_FDCWD</home/neox>, "/gnu/store/gzc1m4n79d1fgby8l58dsaq7nrzyhc14-sane-backends-1.3.1/var/lock/LCK..libusb:001:009", O_WRONLY|O_CREAT|O_EXCL, 0644) = -1 EROFS (Read-only file system)
  [23:00:21.688089] [plustek] sanei_access_lock failed: 11
  [23:00:21.688131] [plustek] open failed: -1

It seems that the issue is the attempt to open a lock file in the store, which
is not possible due to the read-only nature of the file system.

To address this, there were two possible approaches: either move the lock file
to another location or disable the locking mechanism altogether.

After some research, I found that this bug has been resolved in both Debian [1]
and nixOS [2], and it has also been reported to the upstream project [3]. Debian
opted to disable the locking mechanism, while nixOS moved the lock file to a
runtime-generated location using systemd (by patching the install phase to
prevent the creation of the lock directory). However, since the locking
mechanism allows the use of multiple devices simultaneously[4], which can be
useful, nixOS's solution appears to be the better approach. I followed this
method, proposing to create the lock directory during the activation of the
sane service.

I tested these patch with my Canon LiDE 30 and have been able to (finally)
scan with it.

[1] Debian bug report: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=973490
[2] nixOS bug report: https://github.com/NixOS/nixpkgs/issues/273280
[3] Upstream issue: https://gitlab.com/sane-project/backends/-/issues/363
[4] https://gitlab.com/sane-project/backends/-/issues/363#note_443142177

Adrien 'neox' Bourmault (2):
  gnu: sane-backends-minimal: fix lock path for plustek backend
  services: sane-service-type: create lock path for plustek backend

 gnu/packages/scanner.scm |  9 ++++++++-
 gnu/services/desktop.scm | 13 +++++++++++++
 2 files changed, 21 insertions(+), 1 deletion(-)

base-commit: 1b6ce1796abdf497f61f426d61339318f4f4f23d
-- 
2.46.0





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

* [bug#73392] [PATCH 1/2] gnu: sane-backends-minimal: fix lock path for plustek backend
  2024-09-20 19:03 [bug#73391] [PATCH 0/2] SANE: fix a locking bug for plustek backend neox
@ 2024-09-20 19:03 ` neox
       [not found]   ` <handler.73392.B.172685906722683.ack@debbugs.gnu.org>
  2024-09-20 19:03 ` [bug#73393] [PATCH 2/2] services: sane-service-type: create lock path for plustek backend neox
  2024-09-20 20:39 ` [bug#73391] [PATCH v2 0/2] SANE: fix a locking bug " neox
  2 siblings, 1 reply; 15+ messages in thread
From: neox @ 2024-09-20 19:03 UTC (permalink / raw)
  To: 73392; +Cc: Adrien 'neox' Bourmault

From: Adrien 'neox' Bourmault <neox@gnu.org>

* gnu/packages/scanner.scm (sane-backends-minimal)
[arguments]<#:configure-flags>: add "--with-lockdir=/var/lock/sane"
[arguments]<#:phases>: add disable-lockdir-creation to prevent creating the
 lockpath during install

Change-Id: I338c16cd4c0bfa0d165c9906b0f1f87ab79a4f75
Signed-off-by: Adrien 'neox' Bourmault <neox@gnu.org>
---
 gnu/packages/scanner.scm | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/gnu/packages/scanner.scm b/gnu/packages/scanner.scm
index a2faaa2728..a20d27ad2a 100644
--- a/gnu/packages/scanner.scm
+++ b/gnu/packages/scanner.scm
@@ -136,7 +136,8 @@ (define-public sane-backends-minimal
     (inputs
      (list libusb))
     (arguments
-     `(#:phases
+     `(#:configure-flags '("--with-lockdir=/var/lock/sane") ;; Avoid errors with plustek
+       #:phases
        (modify-phases %standard-phases
          (add-before 'bootstrap 'zap-unnecessary-git-dependency
            (lambda _
@@ -145,6 +146,12 @@ (define-public sane-backends-minimal
                (("/bin/sh") (which "sh")))
              (with-output-to-file ".tarball-version"
                (lambda _ (format #t ,version)))))
+         (add-before 'configure 'disable-lockdir-creation
+           (lambda _
+             ;; Modify the Makefile.am to prevent the creation of the lock dir
+             (substitute* "backend/Makefile.am"
+               (("^install-lockpath:.*$")
+                "install-lockpath: # pass"))))
          (add-before 'configure 'disable-backends
            (lambda _
              (setenv "BACKENDS" " ")
-- 
2.46.0





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

* [bug#73393] [PATCH 2/2] services: sane-service-type: create lock path for plustek backend
  2024-09-20 19:03 [bug#73391] [PATCH 0/2] SANE: fix a locking bug for plustek backend neox
  2024-09-20 19:03 ` [bug#73392] [PATCH 1/2] gnu: sane-backends-minimal: fix lock path " neox
@ 2024-09-20 19:03 ` neox
  2024-09-22  8:55   ` Adrien 'neox' Bourmault
  2024-09-20 20:39 ` [bug#73391] [PATCH v2 0/2] SANE: fix a locking bug " neox
  2 siblings, 1 reply; 15+ messages in thread
From: neox @ 2024-09-20 19:03 UTC (permalink / raw)
  To: 73393; +Cc: Adrien 'neox' Bourmault

From: Adrien 'neox' Bourmault <neox@gnu.org>

* gnu/services/desktop.scm (sane-service-type): extend with an activation
 service to create the lockpath and give the right permissions


Change-Id: I187886d12b5f0b4fe21b03de178ea2187205387a
Signed-off-by: Adrien 'neox' Bourmault <neox@gnu.org>
---
 gnu/services/desktop.scm | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/gnu/services/desktop.scm b/gnu/services/desktop.scm
index 274aeeef9b..9eae4178fb 100644
--- a/gnu/services/desktop.scm
+++ b/gnu/services/desktop.scm
@@ -1408,6 +1408,17 @@ (define %sane-accounts
   ;; The '60-libsane.rules' udev rules refers to the "scanner" group.
   (list (user-group (name "scanner") (system? #t))))
 
+(define %sane-activation
+  #~(begin
+      (use-modules (guix build utils))
+      (let ((lockpath "/var/lock/sane")
+            (gid (vector-ref (getgrnam "scanner") 2)))
+        ;; Create the lock directory at runtime and give right perms
+        (mkdir-p lockpath)
+        (chown lockpath -1 gid)
+        (chmod lockpath #o770))
+      #t))
+
 (define sane-service-type
   (service-type
    (name 'sane)
@@ -1418,6 +1429,8 @@ (define sane-service-type
    (default-value sane-backends-minimal)
    (extensions
     (list (service-extension udev-service-type list)
+          (service-extension activation-service-type
+                             (const %sane-activation))
           (service-extension account-service-type
                              (const %sane-accounts))))))
 
-- 
2.46.0





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

* [bug#73392] Acknowledgement ([PATCH 1/2] gnu: sane-backends-minimal: fix lock path for plustek backend)
       [not found]   ` <handler.73392.B.172685906722683.ack@debbugs.gnu.org>
@ 2024-09-20 19:21     ` Adrien 'neox' Bourmault
  0 siblings, 0 replies; 15+ messages in thread
From: Adrien 'neox' Bourmault @ 2024-09-20 19:21 UTC (permalink / raw)
  To: 73392, 73393

Hi, please close as this is a duplicate of #73391 due to a
misconfiguration on my part. Thanks a lot and sorry about this.




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

* [bug#73391] [PATCH v2 0/2] SANE: fix a locking bug for plustek backend
  2024-09-20 19:03 [bug#73391] [PATCH 0/2] SANE: fix a locking bug for plustek backend neox
  2024-09-20 19:03 ` [bug#73392] [PATCH 1/2] gnu: sane-backends-minimal: fix lock path " neox
  2024-09-20 19:03 ` [bug#73393] [PATCH 2/2] services: sane-service-type: create lock path for plustek backend neox
@ 2024-09-20 20:39 ` neox
  2024-09-20 20:39   ` [bug#73391] [PATCH v2 1/2] gnu: sane-backends-minimal: fix lock path " neox
                     ` (4 more replies)
  2 siblings, 5 replies; 15+ messages in thread
From: neox @ 2024-09-20 20:39 UTC (permalink / raw)
  To: 73391; +Cc: Adrien 'neox' Bourmault

From: Adrien 'neox' Bourmault <neox@gnu.org>

Hi, I just realized I forgot to add my copyright notice in the files I
modified. This v2 fixes this.

Adrien 'neox' Bourmault (2):
  gnu: sane-backends-minimal: fix lock path for plustek backend
  services: sane-service-type: create lock path for plustek backend

 gnu/packages/scanner.scm | 10 +++++++++-
 gnu/services/desktop.scm | 14 ++++++++++++++
 2 files changed, 23 insertions(+), 1 deletion(-)


base-commit: 1b6ce1796abdf497f61f426d61339318f4f4f23d
-- 
2.46.0





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

* [bug#73391] [PATCH v2 1/2] gnu: sane-backends-minimal: fix lock path for plustek backend
  2024-09-20 20:39 ` [bug#73391] [PATCH v2 0/2] SANE: fix a locking bug " neox
@ 2024-09-20 20:39   ` neox
  2024-09-21  8:03     ` [bug#73406] " Adrien 'neox' Bourmault
  2024-09-22  9:07     ` [bug#73393] " Adrien 'neox' Bourmault
  2024-09-20 20:39   ` [bug#73391] [PATCH v2 2/2] services: sane-service-type: create " neox
                     ` (3 subsequent siblings)
  4 siblings, 2 replies; 15+ messages in thread
From: neox @ 2024-09-20 20:39 UTC (permalink / raw)
  To: 73391; +Cc: Adrien 'neox' Bourmault

From: Adrien 'neox' Bourmault <neox@gnu.org>

* gnu/packages/scanner.scm (sane-backends-minimal)
[arguments]<#:configure-flags>: add "--with-lockdir=/var/lock/sane"
[arguments]<#:phases>: add disable-lockdir-creation to prevent creating the
 lockpath during install

Change-Id: I338c16cd4c0bfa0d165c9906b0f1f87ab79a4f75
Signed-off-by: Adrien 'neox' Bourmault <neox@gnu.org>
---
 gnu/packages/scanner.scm | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/gnu/packages/scanner.scm b/gnu/packages/scanner.scm
index a2faaa2728..a346f004ae 100644
--- a/gnu/packages/scanner.scm
+++ b/gnu/packages/scanner.scm
@@ -5,6 +5,7 @@
 ;;; Copyright © 2017, 2019, 2020, 2022 Tobias Geerinckx-Rice <me@tobias.gr>
 ;;; Copyright © 2018 Efraim Flashner <efraim@flashner.co.il>
 ;;; Copyright © 2022 João Gabriel <joaog.bastos@protonmail.ch>
+;;; Copyright © 2024 Adrien Bourmault <neox@gnu.org>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -136,7 +137,8 @@ (define-public sane-backends-minimal
     (inputs
      (list libusb))
     (arguments
-     `(#:phases
+     `(#:configure-flags '("--with-lockdir=/var/lock/sane") ;; Avoid errors with plustek
+       #:phases
        (modify-phases %standard-phases
          (add-before 'bootstrap 'zap-unnecessary-git-dependency
            (lambda _
@@ -145,6 +147,12 @@ (define-public sane-backends-minimal
                (("/bin/sh") (which "sh")))
              (with-output-to-file ".tarball-version"
                (lambda _ (format #t ,version)))))
+         (add-before 'configure 'disable-lockdir-creation
+           (lambda _
+             ;; Modify the Makefile.am to prevent the creation of the lock dir
+             (substitute* "backend/Makefile.am"
+               (("^install-lockpath:.*$")
+                "install-lockpath: # pass"))))
          (add-before 'configure 'disable-backends
            (lambda _
              (setenv "BACKENDS" " ")
-- 
2.46.0





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

* [bug#73391] [PATCH v2 2/2] services: sane-service-type: create lock path for plustek backend
  2024-09-20 20:39 ` [bug#73391] [PATCH v2 0/2] SANE: fix a locking bug " neox
  2024-09-20 20:39   ` [bug#73391] [PATCH v2 1/2] gnu: sane-backends-minimal: fix lock path " neox
@ 2024-09-20 20:39   ` neox
  2024-09-21  8:03     ` [bug#73406] " Adrien 'neox' Bourmault
  2024-09-22  9:07     ` [bug#73393] " Adrien 'neox' Bourmault
  2024-09-21  8:01   ` [bug#73406] [PATCH v2 0/2] SANE: fix a locking bug " Adrien 'neox' Bourmault
                     ` (2 subsequent siblings)
  4 siblings, 2 replies; 15+ messages in thread
From: neox @ 2024-09-20 20:39 UTC (permalink / raw)
  To: 73391; +Cc: Adrien 'neox' Bourmault

From: Adrien 'neox' Bourmault <neox@gnu.org>

* gnu/services/desktop.scm (sane-service-type): extend with an activation
 service to create the lockpath and give the right permissions

Change-Id: I187886d12b5f0b4fe21b03de178ea2187205387a
Signed-off-by: Adrien 'neox' Bourmault <neox@gnu.org>
---
 gnu/services/desktop.scm | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/gnu/services/desktop.scm b/gnu/services/desktop.scm
index 274aeeef9b..500527cb50 100644
--- a/gnu/services/desktop.scm
+++ b/gnu/services/desktop.scm
@@ -17,6 +17,7 @@
 ;;; Copyright © 2021, 2022 muradm <mail@muradm.net>
 ;;; Copyright © 2023 Bruno Victal <mirai@makinata.eu>
 ;;; Copyright © 2023 Zheng Junjie <873216071@qq.com>
+;;; Copyright © 2024 Adrien Bourmault <neox@gnu.org>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -1408,6 +1409,17 @@ (define %sane-accounts
   ;; The '60-libsane.rules' udev rules refers to the "scanner" group.
   (list (user-group (name "scanner") (system? #t))))
 
+(define %sane-activation
+  #~(begin
+      (use-modules (guix build utils))
+      (let ((lockpath "/var/lock/sane")
+            (gid (vector-ref (getgrnam "scanner") 2)))
+        ;; Create the lock directory at runtime and give right perms
+        (mkdir-p lockpath)
+        (chown lockpath -1 gid)
+        (chmod lockpath #o770))
+      #t))
+
 (define sane-service-type
   (service-type
    (name 'sane)
@@ -1418,6 +1430,8 @@ (define sane-service-type
    (default-value sane-backends-minimal)
    (extensions
     (list (service-extension udev-service-type list)
+          (service-extension activation-service-type
+                             (const %sane-activation))
           (service-extension account-service-type
                              (const %sane-accounts))))))
 
-- 
2.46.0





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

* [bug#73406] [PATCH v2 0/2] SANE: fix a locking bug for plustek backend
  2024-09-20 20:39 ` [bug#73391] [PATCH v2 0/2] SANE: fix a locking bug " neox
  2024-09-20 20:39   ` [bug#73391] [PATCH v2 1/2] gnu: sane-backends-minimal: fix lock path " neox
  2024-09-20 20:39   ` [bug#73391] [PATCH v2 2/2] services: sane-service-type: create " neox
@ 2024-09-21  8:01   ` Adrien 'neox' Bourmault
  2024-09-22  9:07   ` [bug#73393] " Adrien 'neox' Bourmault
  2024-09-24 18:51   ` Liliana Marie Prikler
  4 siblings, 0 replies; 15+ messages in thread
From: Adrien 'neox' Bourmault @ 2024-09-21  8:01 UTC (permalink / raw)
  To: 73406; +Cc: Adrien 'neox' Bourmault

Hi.
While attempting to use a Canon LiDE 30 scanner, which is supported by SANE in
its current version, I noticed a malfunction with the plustek backend. No
application seemed capable of detecting the scanner, even though the
`sane-find-scanner` command indicated its presence, and the device IDs matched
those in the plustek backend configuration.

I investigated further using the following command:

  SANE_DEBUG_SANEI_USB=128 SANE_DEBUG_PLUSTEK=255 strace -fye open,openat scanimage -L

Here is a summary of the output:

  [23:00:21.688008] [plustek] usbDev_open(auto,) - 0x1c1e5630
  [pid 1209-20241] openat(AT_FDCWD</home/neox>, "/gnu/store/gzc1m4n79d1fgby8l58dsaq7nrzyhc14-sane-backend>
  [23:00:21.688089] [plustek] sanei_access_lock failed: 11
  [23:00:21.688131] [plustek] open failed: -1

It seems that the issue is the attempt to open a lock file in the store, which
is not possible due to the read-only nature of the file system.

To address this, there were two possible approaches: either move the lock file
to another location or disable the locking mechanism altogether.

After some research, I found that this bug has been resolved in both Debian [1]
and nixOS [2], and it has also been reported to the upstream project [3]. Debian
opted to disable the locking mechanism, while nixOS moved the lock file to a
runtime-generated location using systemd (by patching the install phase to
prevent the creation of the lock directory). However, since the locking
mechanism allows the use of multiple devices simultaneously[4], which can be
useful, nixOS's solution appears to be the better approach. I followed this
method, proposing to create the lock directory during the activation of the
sane service.

I tested these patch with my Canon LiDE 30 and have been able to (finally)
scan with it.

I realized I forgot to add my copyright notice in the files I
modified. This v2 fixes this.

[1] Debian bug report: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=973490
[2] nixOS bug report: https://github.com/NixOS/nixpkgs/issues/273280
[3] Upstream issue: https://gitlab.com/sane-project/backends/-/issues/363
[4] https://gitlab.com/sane-project/backends/-/issues/363#note_443142177

Adrien 'neox' Bourmault (2):
  gnu: sane-backends-minimal: fix lock path for plustek backend
  services: sane-service-type: create lock path for plustek backend

 gnu/packages/scanner.scm | 10 +++++++++-
 gnu/services/desktop.scm | 14 ++++++++++++++
 2 files changed, 23 insertions(+), 1 deletion(-)


base-commit: 1b6ce1796abdf497f61f426d61339318f4f4f23d
-- 
2.46.0





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

* [bug#73406] [PATCH v2 1/2] gnu: sane-backends-minimal: fix lock path for plustek backend
  2024-09-20 20:39   ` [bug#73391] [PATCH v2 1/2] gnu: sane-backends-minimal: fix lock path " neox
@ 2024-09-21  8:03     ` Adrien 'neox' Bourmault
  2024-09-22  9:07     ` [bug#73393] " Adrien 'neox' Bourmault
  1 sibling, 0 replies; 15+ messages in thread
From: Adrien 'neox' Bourmault @ 2024-09-21  8:03 UTC (permalink / raw)
  To: 73406; +Cc: Adrien 'neox' Bourmault

* gnu/packages/scanner.scm (sane-backends-minimal)
[arguments]<#:configure-flags>: add "--with-lockdir=/var/lock/sane"
[arguments]<#:phases>: add disable-lockdir-creation to prevent creating the
 lockpath during install

Change-Id: I338c16cd4c0bfa0d165c9906b0f1f87ab79a4f75
Signed-off-by: Adrien 'neox' Bourmault <neox@gnu.org>
---
 gnu/packages/scanner.scm | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/gnu/packages/scanner.scm b/gnu/packages/scanner.scm
index a2faaa2728..a346f004ae 100644
--- a/gnu/packages/scanner.scm
+++ b/gnu/packages/scanner.scm
@@ -5,6 +5,7 @@
 ;;; Copyright © 2017, 2019, 2020, 2022 Tobias Geerinckx-Rice <me@tobias.gr>
 ;;; Copyright © 2018 Efraim Flashner <efraim@flashner.co.il>
 ;;; Copyright © 2022 João Gabriel <joaog.bastos@protonmail.ch>
+;;; Copyright © 2024 Adrien Bourmault <neox@gnu.org>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -136,7 +137,8 @@ (define-public sane-backends-minimal
     (inputs
      (list libusb))
     (arguments
-     `(#:phases
+     `(#:configure-flags '("--with-lockdir=/var/lock/sane") ;; Avoid errors with plustek
+       #:phases
        (modify-phases %standard-phases
          (add-before 'bootstrap 'zap-unnecessary-git-dependency
            (lambda _
@@ -145,6 +147,12 @@ (define-public sane-backends-minimal
                (("/bin/sh") (which "sh")))
              (with-output-to-file ".tarball-version"
                (lambda _ (format #t ,version)))))
+         (add-before 'configure 'disable-lockdir-creation
+           (lambda _
+             ;; Modify the Makefile.am to prevent the creation of the lock dir
+             (substitute* "backend/Makefile.am"
+               (("^install-lockpath:.*$")
+                "install-lockpath: # pass"))))
          (add-before 'configure 'disable-backends
            (lambda _
              (setenv "BACKENDS" " ")
-- 
2.46.0





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

* [bug#73406] [PATCH v2 2/2] services: sane-service-type: create lock path for plustek backend
  2024-09-20 20:39   ` [bug#73391] [PATCH v2 2/2] services: sane-service-type: create " neox
@ 2024-09-21  8:03     ` Adrien 'neox' Bourmault
  2024-09-22  9:07     ` [bug#73393] " Adrien 'neox' Bourmault
  1 sibling, 0 replies; 15+ messages in thread
From: Adrien 'neox' Bourmault @ 2024-09-21  8:03 UTC (permalink / raw)
  To: 73406; +Cc: Adrien 'neox' Bourmault

* gnu/services/desktop.scm (sane-service-type): extend with an activation
 service to create the lockpath and give the right permissions

Change-Id: I187886d12b5f0b4fe21b03de178ea2187205387a
Signed-off-by: Adrien 'neox' Bourmault <neox@gnu.org>
---
 gnu/services/desktop.scm | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/gnu/services/desktop.scm b/gnu/services/desktop.scm
index 274aeeef9b..500527cb50 100644
--- a/gnu/services/desktop.scm
+++ b/gnu/services/desktop.scm
@@ -17,6 +17,7 @@
 ;;; Copyright © 2021, 2022 muradm <mail@muradm.net>
 ;;; Copyright © 2023 Bruno Victal <mirai@makinata.eu>
 ;;; Copyright © 2023 Zheng Junjie <873216071@qq.com>
+;;; Copyright © 2024 Adrien Bourmault <neox@gnu.org>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -1408,6 +1409,17 @@ (define %sane-accounts
   ;; The '60-libsane.rules' udev rules refers to the "scanner" group.
   (list (user-group (name "scanner") (system? #t))))
 
+(define %sane-activation
+  #~(begin
+      (use-modules (guix build utils))
+      (let ((lockpath "/var/lock/sane")
+            (gid (vector-ref (getgrnam "scanner") 2)))
+        ;; Create the lock directory at runtime and give right perms
+        (mkdir-p lockpath)
+        (chown lockpath -1 gid)
+        (chmod lockpath #o770))
+      #t))
+
 (define sane-service-type
   (service-type
    (name 'sane)
@@ -1418,6 +1430,8 @@ (define sane-service-type
    (default-value sane-backends-minimal)
    (extensions
     (list (service-extension udev-service-type list)
+          (service-extension activation-service-type
+                             (const %sane-activation))
           (service-extension account-service-type
                              (const %sane-accounts))))))
 
-- 
2.46.0





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

* [bug#73393] [PATCH 2/2] services: sane-service-type: create lock path for plustek backend
  2024-09-20 19:03 ` [bug#73393] [PATCH 2/2] services: sane-service-type: create lock path for plustek backend neox
@ 2024-09-22  8:55   ` Adrien 'neox' Bourmault
  0 siblings, 0 replies; 15+ messages in thread
From: Adrien 'neox' Bourmault @ 2024-09-22  8:55 UTC (permalink / raw)
  To: 73393

Sorry for all that noise, that's my first time using Debbugs and I got
a little lost here, trying to obtain a clear merge...




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

* [bug#73393] [PATCH v2 0/2] SANE: fix a locking bug for plustek backend
  2024-09-20 20:39 ` [bug#73391] [PATCH v2 0/2] SANE: fix a locking bug " neox
                     ` (2 preceding siblings ...)
  2024-09-21  8:01   ` [bug#73406] [PATCH v2 0/2] SANE: fix a locking bug " Adrien 'neox' Bourmault
@ 2024-09-22  9:07   ` Adrien 'neox' Bourmault
  2024-09-24 18:51   ` Liliana Marie Prikler
  4 siblings, 0 replies; 15+ messages in thread
From: Adrien 'neox' Bourmault @ 2024-09-22  9:07 UTC (permalink / raw)
  To: 73393; +Cc: Adrien 'neox' Bourmault

Hi.
While attempting to use a Canon LiDE 30 scanner, which is supported by SANE in
its current version, I noticed a malfunction with the plustek backend. No
application seemed capable of detecting the scanner, even though the
`sane-find-scanner` command indicated its presence, and the device IDs matched
those in the plustek backend configuration.

I investigated further using the following command:

  SANE_DEBUG_SANEI_USB=128 SANE_DEBUG_PLUSTEK=255 strace -fye open,openat scanimage -L

Here is a summary of the output:

  [23:00:21.688008] [plustek] usbDev_open(auto,) - 0x1c1e5630
  [pid 1209-20241] openat(AT_FDCWD</home/neox>, "/gnu/store/gzc1m4n79d1fgby8l58dsaq7nrzyhc14-sane-backend>
  [23:00:21.688089] [plustek] sanei_access_lock failed: 11
  [23:00:21.688131] [plustek] open failed: -1

It seems that the issue is the attempt to open a lock file in the store, which
is not possible due to the read-only nature of the file system.

To address this, there were two possible approaches: either move the lock file
to another location or disable the locking mechanism altogether.

After some research, I found that this bug has been resolved in both Debian [1]
and nixOS [2], and it has also been reported to the upstream project [3]. Debian
opted to disable the locking mechanism, while nixOS moved the lock file to a
runtime-generated location using systemd (by patching the install phase to
prevent the creation of the lock directory). However, since the locking
mechanism allows the use of multiple devices simultaneously[4], which can be
useful, nixOS's solution appears to be the better approach. I followed this
method, proposing to create the lock directory during the activation of the
sane service.

I tested these patch with my Canon LiDE 30 and have been able to (finally)
scan with it.

I realized I forgot to add my copyright notice in the files I
modified. This v2 fixes this.

[1] Debian bug report: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=973490
[2] nixOS bug report: https://github.com/NixOS/nixpkgs/issues/273280
[3] Upstream issue: https://gitlab.com/sane-project/backends/-/issues/363
[4] https://gitlab.com/sane-project/backends/-/issues/363#note_443142177

Adrien 'neox' Bourmault (2):
  gnu: sane-backends-minimal: fix lock path for plustek backend
  services: sane-service-type: create lock path for plustek backend

 gnu/packages/scanner.scm | 10 +++++++++-
 gnu/services/desktop.scm | 14 ++++++++++++++
 2 files changed, 23 insertions(+), 1 deletion(-)


base-commit: 1b6ce1796abdf497f61f426d61339318f4f4f23d
-- 
2.46.0





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

* [bug#73393] [PATCH v2 1/2] gnu: sane-backends-minimal: fix lock path for plustek backend
  2024-09-20 20:39   ` [bug#73391] [PATCH v2 1/2] gnu: sane-backends-minimal: fix lock path " neox
  2024-09-21  8:03     ` [bug#73406] " Adrien 'neox' Bourmault
@ 2024-09-22  9:07     ` Adrien 'neox' Bourmault
  1 sibling, 0 replies; 15+ messages in thread
From: Adrien 'neox' Bourmault @ 2024-09-22  9:07 UTC (permalink / raw)
  To: 73393; +Cc: Adrien 'neox' Bourmault

* gnu/packages/scanner.scm (sane-backends-minimal)
[arguments]<#:configure-flags>: add "--with-lockdir=/var/lock/sane"
[arguments]<#:phases>: add disable-lockdir-creation to prevent creating the
 lockpath during install

Change-Id: I338c16cd4c0bfa0d165c9906b0f1f87ab79a4f75
Signed-off-by: Adrien 'neox' Bourmault <neox@gnu.org>
---
 gnu/packages/scanner.scm | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/gnu/packages/scanner.scm b/gnu/packages/scanner.scm
index a2faaa2728..a346f004ae 100644
--- a/gnu/packages/scanner.scm
+++ b/gnu/packages/scanner.scm
@@ -5,6 +5,7 @@
 ;;; Copyright © 2017, 2019, 2020, 2022 Tobias Geerinckx-Rice <me@tobias.gr>
 ;;; Copyright © 2018 Efraim Flashner <efraim@flashner.co.il>
 ;;; Copyright © 2022 João Gabriel <joaog.bastos@protonmail.ch>
+;;; Copyright © 2024 Adrien Bourmault <neox@gnu.org>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -136,7 +137,8 @@ (define-public sane-backends-minimal
     (inputs
      (list libusb))
     (arguments
-     `(#:phases
+     `(#:configure-flags '("--with-lockdir=/var/lock/sane") ;; Avoid errors with plustek
+       #:phases
        (modify-phases %standard-phases
          (add-before 'bootstrap 'zap-unnecessary-git-dependency
            (lambda _
@@ -145,6 +147,12 @@ (define-public sane-backends-minimal
                (("/bin/sh") (which "sh")))
              (with-output-to-file ".tarball-version"
                (lambda _ (format #t ,version)))))
+         (add-before 'configure 'disable-lockdir-creation
+           (lambda _
+             ;; Modify the Makefile.am to prevent the creation of the lock dir
+             (substitute* "backend/Makefile.am"
+               (("^install-lockpath:.*$")
+                "install-lockpath: # pass"))))
          (add-before 'configure 'disable-backends
            (lambda _
              (setenv "BACKENDS" " ")
-- 
2.46.0





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

* [bug#73393] [PATCH v2 2/2] services: sane-service-type: create lock path for plustek backend
  2024-09-20 20:39   ` [bug#73391] [PATCH v2 2/2] services: sane-service-type: create " neox
  2024-09-21  8:03     ` [bug#73406] " Adrien 'neox' Bourmault
@ 2024-09-22  9:07     ` Adrien 'neox' Bourmault
  1 sibling, 0 replies; 15+ messages in thread
From: Adrien 'neox' Bourmault @ 2024-09-22  9:07 UTC (permalink / raw)
  To: 73393; +Cc: Adrien 'neox' Bourmault

* gnu/services/desktop.scm (sane-service-type): extend with an activation
 service to create the lockpath and give the right permissions

Change-Id: I187886d12b5f0b4fe21b03de178ea2187205387a
Signed-off-by: Adrien 'neox' Bourmault <neox@gnu.org>
---
 gnu/services/desktop.scm | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/gnu/services/desktop.scm b/gnu/services/desktop.scm
index 274aeeef9b..500527cb50 100644
--- a/gnu/services/desktop.scm
+++ b/gnu/services/desktop.scm
@@ -17,6 +17,7 @@
 ;;; Copyright © 2021, 2022 muradm <mail@muradm.net>
 ;;; Copyright © 2023 Bruno Victal <mirai@makinata.eu>
 ;;; Copyright © 2023 Zheng Junjie <873216071@qq.com>
+;;; Copyright © 2024 Adrien Bourmault <neox@gnu.org>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -1408,6 +1409,17 @@ (define %sane-accounts
   ;; The '60-libsane.rules' udev rules refers to the "scanner" group.
   (list (user-group (name "scanner") (system? #t))))
 
+(define %sane-activation
+  #~(begin
+      (use-modules (guix build utils))
+      (let ((lockpath "/var/lock/sane")
+            (gid (vector-ref (getgrnam "scanner") 2)))
+        ;; Create the lock directory at runtime and give right perms
+        (mkdir-p lockpath)
+        (chown lockpath -1 gid)
+        (chmod lockpath #o770))
+      #t))
+
 (define sane-service-type
   (service-type
    (name 'sane)
@@ -1418,6 +1430,8 @@ (define sane-service-type
    (default-value sane-backends-minimal)
    (extensions
     (list (service-extension udev-service-type list)
+          (service-extension activation-service-type
+                             (const %sane-activation))
           (service-extension account-service-type
                              (const %sane-accounts))))))
 
-- 
2.46.0





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

* [bug#73393] [PATCH v2 0/2] SANE: fix a locking bug for plustek backend
  2024-09-20 20:39 ` [bug#73391] [PATCH v2 0/2] SANE: fix a locking bug " neox
                     ` (3 preceding siblings ...)
  2024-09-22  9:07   ` [bug#73393] " Adrien 'neox' Bourmault
@ 2024-09-24 18:51   ` Liliana Marie Prikler
  4 siblings, 0 replies; 15+ messages in thread
From: Liliana Marie Prikler @ 2024-09-24 18:51 UTC (permalink / raw)
  To: Adrien 'neox' Bourmault, 73393

Hi Adrien,

Mumi appears to have this thing again where clicking on some button to
retrieve a message retrieves the wrong message.  Thus, I will answer to
this one while referencing the stuff in the patches.

Am Sonntag, dem 22.09.2024 um 11:07 +0200 schrieb Adrien 'neox'
Bourmault:
>   gnu: sane-backends-minimal: fix lock path for plustek backend
It would be nice if we had an actual patch here rather than a crude
substitute*.  I know it's a sed script on Nix, but that feels kinda
weird to say the least.

>   services: sane-service-type: create lock path for plustek backend
You should look into making this atomic.  It's currently three syscalls
in a row, which would make this vulnerable to race conditions. 
Consider passing a mode to mkdir.

Cheers




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

end of thread, other threads:[~2024-09-24 18:54 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-09-20 19:03 [bug#73391] [PATCH 0/2] SANE: fix a locking bug for plustek backend neox
2024-09-20 19:03 ` [bug#73392] [PATCH 1/2] gnu: sane-backends-minimal: fix lock path " neox
     [not found]   ` <handler.73392.B.172685906722683.ack@debbugs.gnu.org>
2024-09-20 19:21     ` [bug#73392] Acknowledgement ([PATCH 1/2] gnu: sane-backends-minimal: fix lock path for plustek backend) Adrien 'neox' Bourmault
2024-09-20 19:03 ` [bug#73393] [PATCH 2/2] services: sane-service-type: create lock path for plustek backend neox
2024-09-22  8:55   ` Adrien 'neox' Bourmault
2024-09-20 20:39 ` [bug#73391] [PATCH v2 0/2] SANE: fix a locking bug " neox
2024-09-20 20:39   ` [bug#73391] [PATCH v2 1/2] gnu: sane-backends-minimal: fix lock path " neox
2024-09-21  8:03     ` [bug#73406] " Adrien 'neox' Bourmault
2024-09-22  9:07     ` [bug#73393] " Adrien 'neox' Bourmault
2024-09-20 20:39   ` [bug#73391] [PATCH v2 2/2] services: sane-service-type: create " neox
2024-09-21  8:03     ` [bug#73406] " Adrien 'neox' Bourmault
2024-09-22  9:07     ` [bug#73393] " Adrien 'neox' Bourmault
2024-09-21  8:01   ` [bug#73406] [PATCH v2 0/2] SANE: fix a locking bug " Adrien 'neox' Bourmault
2024-09-22  9:07   ` [bug#73393] " Adrien 'neox' Bourmault
2024-09-24 18:51   ` Liliana Marie Prikler

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/guix.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).