smartd fails to send an email

Done

Details

3 participants

45mg
Hilton Chain
Tomas Volf

Owner: unassigned

Submitted by: Tomas Volf

Severity: normal

Debbugs page

Tomas Volf wrote 2 months ago

Recipients:(address . bug-guix@gnu.org)

Message-ID:87r056rxbe.fsf@wolfsden.cz

Hi,

I have tried to write a service type for smartd from smartmontools,

however the bundled script fails.

When the test (-m root -M test) is executed, some programs seem to be

missing in the $PATH:

Toggle snippet (6 lines)

Jan 14 01:34:19 localhost smartd[3138]: Executing test of <mail> to root ...

Jan 14 01:34:19 localhost smartd[3138]: Test of <mail> to root produced unexpected output (118 bytes) to STDOUT/STDERR:

Jan 14 01:34:19 localhost smartd[3138]: /gnu/store/ks6lnp8hssm9zkka47ysa4qp9xd9f8bv-smartmontools-7.4/etc/smartd_warning.sh: line 132: sed: command not found

Jan 14 01:34:19 localhost smartd[3138]: Test of <mail> to root: failed (32-bit/8-bit exit status: 32512/127)

At the start of the script I see:

Toggle snippet (3 lines)

export PATH="/usr/local/bin:/usr/bin:/bin"

That does not look correct. I am not sure if any wrapping is necessary.

Have a nice day,

Tomas

There are only two hard things in Computer Science:

cache invalidation, naming things and off-by-one errors.

45mg wrote 2 months ago

Recipients:

Message-ID:87wmeyc67s.fsf@gmail.com

Hi Tomas,

Tomas Volf <~@wolfsden.cz> writes:

Toggle quote (15 lines)> Hi,
>
> I have tried to write a service type for smartd from smartmontools,
> however the bundled script fails.
>
> When the test (-m root -M test) is executed, some programs seem to be
> missing in the $PATH:
>
> --8<---------------cut here---------------start------------->8---
> Jan 14 01:34:19 localhost smartd[3138]: Executing test of <mail> to root ... 
> Jan 14 01:34:19 localhost smartd[3138]: Test of <mail> to root produced unexpected output (118 bytes) to STDOUT/STDERR:  
> Jan 14 01:34:19 localhost smartd[3138]: /gnu/store/ks6lnp8hssm9zkka47ysa4qp9xd9f8bv-smartmontools-7.4/etc/smartd_warning.sh: line 132: sed: command not found 
> Jan 14 01:34:19 localhost smartd[3138]: Test of <mail> to root: failed (32-bit/8-bit exit status: 32512/127) 
> --8<---------------cut here---------------end--------------->8---

I've seen other package definitions deal with this problem by using
`substitute*` to replace executables specified in scripts, etc. (like
sed here) with the corresponding store paths. See 'light' in (gnu
packages linux) for an example.

Tomas Volf wrote 2 months ago

[PATCH] gnu: smartmontools: Fix PATH in smartd_warning.sh.

Recipients:(name . Tomas Volf)(address . ~@wolfsden.cz)

Message-ID:d7320ddeb1939ce0eeeea10f7267883692c269e7.1736898019.git.~@wolfsden.cz

The script started with reset of the $PATH to a value not suitable to Guix.

In addition, the script requires coreutils and sed, so add those into the

$PATH.

* gnu/packages/admin.scm (smartmontools)[arguments]<#:phases>: Add 'fix-path.

Change-Id: Ide97f572e6f369fe24337f945474dc7a65584eda

---

gnu/packages/admin.scm | 15 +++++++++++++--

1 file changed, 13 insertions(+), 2 deletions(-)

Toggle diff (28 lines)diff --git a/gnu/packages/admin.scm b/gnu/packages/admin.scm
index 7f50d5f4e9..098e21ff8a 100644
--- a/gnu/packages/admin.scm
+++ b/gnu/packages/admin.scm
@@ -2921,8 +2921,19 @@ (define-public smartmontools
                 "0gcrzcb4g7f994n6nws26g6x15yjija1gyzd359sjv7r3xj1z9p9"))))
     (build-system gnu-build-system)
     (arguments
-     (list #:make-flags
-           #~(list "BUILD_INFO=\"(Guix)\"")))
+     (list
+      #:make-flags
+      #~(list "BUILD_INFO=\"(Guix)\"")
+      #:phases
+      #~(modify-phases %standard-phases
+          (add-after 'install 'fix-path
+            (lambda _
+              (substitute* (string-append #$output "/etc/smartd_warning.sh")
+                (("export PATH=.*$" all)
+                 (string-append "PATH="
+                                #$(file-append sed "/bin") ":"
+                                #$(file-append coreutils "/bin") ":"
+                                "$PATH\n"))))))))
     (inputs (list libcap-ng))
     (home-page "https://www.smartmontools.org/")
     (synopsis "S.M.A.R.T. harddisk control and monitoring tools")
-- 
2.47.1

Tomas Volf wrote 2 months ago

control message for bug #75550

Recipients:(address . control@debbugs.gnu.org)

Message-ID:338703f3736c814d1d204e9b74acbdc9@wolfsden.cz

reassign 75550 guix-patches

quit

Hilton Chain wrote 2 months ago

Re: bug#75550: [PATCH] gnu: smartmontools: Fix PATH in smartd_warning.sh.

Recipients:(name . Tomas Volf)(address . ~@wolfsden.cz)

Message-ID:87ed14ssck.wl-hako@ultrarare.space

Hi Tomas,

On Wed, 15 Jan 2025 07:40:19 +0800,

Tomas Volf wrote:

Toggle quote (36 lines)>
> The script started with reset of the $PATH to a value not suitable to Guix.
> In addition, the script requires coreutils and sed, so add those into the
> $PATH.
>
> * gnu/packages/admin.scm (smartmontools)[arguments]<#:phases>: Add 'fix-path.
>
> Change-Id: Ide97f572e6f369fe24337f945474dc7a65584eda
> ---
>  gnu/packages/admin.scm | 15 +++++++++++++--
>  1 file changed, 13 insertions(+), 2 deletions(-)
>
> diff --git a/gnu/packages/admin.scm b/gnu/packages/admin.scm
> index 7f50d5f4e9..098e21ff8a 100644
> --- a/gnu/packages/admin.scm
> +++ b/gnu/packages/admin.scm
> @@ -2921,8 +2921,19 @@ (define-public smartmontools
>                  "0gcrzcb4g7f994n6nws26g6x15yjija1gyzd359sjv7r3xj1z9p9"))))
>      (build-system gnu-build-system)
>      (arguments
> -     (list #:make-flags
> -           #~(list "BUILD_INFO=\"(Guix)\"")))
> +     (list
> +      #:make-flags
> +      #~(list "BUILD_INFO=\"(Guix)\"")
> +      #:phases
> +      #~(modify-phases %standard-phases
> +          (add-after 'install 'fix-path
> +            (lambda _
> +              (substitute* (string-append #$output "/etc/smartd_warning.sh")
> +                (("export PATH=.*$" all)
> +                 (string-append "PATH="
> +                                #$(file-append sed "/bin") ":"
> +                                #$(file-append coreutils "/bin") ":"
> +                                "$PATH\n"))))))))

Please add sed and coreutils-minimal to inputs and use search-input-file or

this-package-input instead.

For smartmontools the proper way is to set --with-scriptpath='...' configure

flag, which is documented in its INSTALL file. (It can be disabled with a 'no'

value as well.)

Toggle quote (6 lines)

> (inputs (list libcap-ng))

> (home-page "https://www.smartmontools.org/")

> (synopsis "S.M.A.R.T. harddisk control and monitoring tools")

> --

> 2.47.1

Thanks

Tomas Volf wrote 2 months ago

Recipients:(name . Hilton Chain)(address . hako@ultrarare.space)

Message-ID:877c6kjkib.fsf@wolfsden.cz

Thank you for the review, responses below.

Hilton Chain <hako@ultrarare.space> writes:

Toggle quote (43 lines)> Hi Tomas,
>
> On Wed, 15 Jan 2025 07:40:19 +0800,
> Tomas Volf wrote:
>>
>> The script started with reset of the $PATH to a value not suitable to Guix.
>> In addition, the script requires coreutils and sed, so add those into the
>> $PATH.
>>
>> * gnu/packages/admin.scm (smartmontools)[arguments]<#:phases>: Add 'fix-path.
>>
>> Change-Id: Ide97f572e6f369fe24337f945474dc7a65584eda
>> ---
>>  gnu/packages/admin.scm | 15 +++++++++++++--
>>  1 file changed, 13 insertions(+), 2 deletions(-)
>>
>> diff --git a/gnu/packages/admin.scm b/gnu/packages/admin.scm
>> index 7f50d5f4e9..098e21ff8a 100644
>> --- a/gnu/packages/admin.scm
>> +++ b/gnu/packages/admin.scm
>> @@ -2921,8 +2921,19 @@ (define-public smartmontools
>>                  "0gcrzcb4g7f994n6nws26g6x15yjija1gyzd359sjv7r3xj1z9p9"))))
>>      (build-system gnu-build-system)
>>      (arguments
>> -     (list #:make-flags
>> -           #~(list "BUILD_INFO=\"(Guix)\"")))
>> +     (list
>> +      #:make-flags
>> +      #~(list "BUILD_INFO=\"(Guix)\"")
>> +      #:phases
>> +      #~(modify-phases %standard-phases
>> +          (add-after 'install 'fix-path
>> +            (lambda _
>> +              (substitute* (string-append #$output "/etc/smartd_warning.sh")
>> +                (("export PATH=.*$" all)
>> +                 (string-append "PATH="
>> +                                #$(file-append sed "/bin") ":"
>> +                                #$(file-append coreutils "/bin") ":"
>> +                                "$PATH\n"))))))))
>
> Please add sed and coreutils-minimal to inputs and use search-input-file or
> this-package-input instead.

Why?  I will of course do so, but would first like to understand why.
The current way seems to work and keeps all the references in just one
place.  If I switch to the suggested approach, both `sed' and
`coreutils' will be referenced in two places (once in the `inputs' and
once here) and those will need to be kept in sync.  So, uh, why do it?

Toggle quote (5 lines)

> For smartmontools the proper way is to set --with-scriptpath='...' configure

> flag, which is documented in its INSTALL file. (It can be disabled with a 'no'

> value as well.)

This does indeed seem to be more elegant approach, will use it for v2.

Toggle quote (9 lines)

>> (inputs (list libcap-ng))

>> (home-page "https://www.smartmontools.org/")

>> (synopsis "S.M.A.R.T. harddisk control and monitoring tools")

>> --

>> 2.47.1

> Thanks

Tomas

There are only two hard things in Computer Science:

cache invalidation, naming things and off-by-one errors.

Hilton Chain wrote 2 months ago

Recipients:(name . Tomas Volf)(address . ~@wolfsden.cz)

Message-ID:877c6kyykl.wl-hako@ultrarare.space

On Fri, 24 Jan 2025 23:31:56 +0800,

Tomas Volf wrote:

Toggle quote (15 lines)> [...]
> >> +                 (string-append "PATH="
> >> +                                #$(file-append sed "/bin") ":"
> >> +                                #$(file-append coreutils "/bin") ":"
> >> +                                "$PATH\n"))))))))
> >
> > Please add sed and coreutils-minimal to inputs and use search-input-file or
> > this-package-input instead.
>
> Why?  I will of course do so, but would first like to understand why.
> The current way seems to work and keeps all the references in just one
> place.  If I switch to the suggested approach, both `sed' and
> `coreutils' will be referenced in two places (once in the `inputs' and
> once here) and those will need to be kept in sync.  So, uh, why do it?

The reason is to record dependency information at <package> level, since this is
where the UI and most of the tools work.  (`guix show', `guix refresh', package
transformation etc.)

For referencing dependencies from inputs in arguments, the benefit is mainly for
transformations.  Maybe it doesn't make much sense in this case (sed and
coreutils), but it's a good practice to follow.

Tomas Volf wrote 1 months ago

Recipients:(name . Hilton Chain)(address . hako@ultrarare.space)

Message-ID:87o6zqfv17.fsf@wolfsden.cz

Hilton Chain <hako@ultrarare.space> writes:

Toggle quote (21 lines)> On Fri, 24 Jan 2025 23:31:56 +0800,
> Tomas Volf wrote:
>> [...]
>> >> +                 (string-append "PATH="
>> >> +                                #$(file-append sed "/bin") ":"
>> >> +                                #$(file-append coreutils "/bin") ":"
>> >> +                                "$PATH\n"))))))))
>> >
>> > Please add sed and coreutils-minimal to inputs and use search-input-file or
>> > this-package-input instead.
>>
>> Why?  I will of course do so, but would first like to understand why.
>> The current way seems to work and keeps all the references in just one
>> place.  If I switch to the suggested approach, both `sed' and
>> `coreutils' will be referenced in two places (once in the `inputs' and
>> once here) and those will need to be kept in sync.  So, uh, why do it?
>
> The reason is to record dependency information at <package> level, since this is
> where the UI and most of the tools work.  (`guix show', `guix refresh', package
> transformation etc.)

Is there a technical reason it cannot inspect the gexps used or is it

just matter of "no one wrote the code yet"?

Toggle quote (5 lines)

> For referencing dependencies from inputs in arguments, the benefit is mainly for

> transformations. Maybe it doesn't make much sense in this case (sed and

> coreutils), but it's a good practice to follow.

I did not realize this at all. That is very good to know.

Tomas

There are only two hard things in Computer Science:

cache invalidation, naming things and off-by-one errors.

Tomas Volf wrote 1 months ago

[PATCH v2] gnu: smartmontools: Fix PATH in smartd_warning.sh.

Recipients:(address . 75550@debbugs.gnu.org)(name . Tomas Volf)(address . ~@wolfsden.cz)

Message-ID:3a4c0627e02cfe286e21017c77cc8100bb42b4fc.1738105215.git.~@wolfsden.cz

The script started with reset of the $PATH to a value not suitable to Guix.

In addition, the script requires coreutils and sed, so add those into the

$PATH.

* gnu/packages/admin.scm (smartmontools)[inputs]: Add sed, coreutils-minimal.

[arguments]<#:configure-flags>: Pass --with-scriptpath=.

Change-Id: Ide97f572e6f369fe24337f945474dc7a65584eda

---

gnu/packages/admin.scm | 13 ++++++++++---

1 file changed, 10 insertions(+), 3 deletions(-)

Toggle diff (26 lines)diff --git a/gnu/packages/admin.scm b/gnu/packages/admin.scm
index fd5d67ec50..e21f3f6e19 100644
--- a/gnu/packages/admin.scm
+++ b/gnu/packages/admin.scm
@@ -3051,9 +3051,16 @@ (define-public smartmontools
                 "0gcrzcb4g7f994n6nws26g6x15yjija1gyzd359sjv7r3xj1z9p9"))))
     (build-system gnu-build-system)
     (arguments
-     (list #:make-flags
-           #~(list "BUILD_INFO=\"(Guix)\"")))
-    (inputs (list libcap-ng))
+     (list
+      #:make-flags
+      #~(list "BUILD_INFO=\"(Guix)\"")
+      #:configure-flags
+      #~(list (format #f "--with-scriptpath=~a:~a:$PATH"
+                      (dirname (search-input-file %build-inputs "bin/sed"))
+                      (dirname (search-input-file %build-inputs "bin/true"))))))
+    (inputs (list coreutils-minimal
+                  libcap-ng
+                  sed))
     (home-page "https://www.smartmontools.org/")
     (synopsis "S.M.A.R.T. harddisk control and monitoring tools")
     (description
-- 
2.47.1

Tomas Volf wrote 1 months ago

Re: bug#75550: [PATCH] gnu: smartmontools: Fix PATH in smartd_warning.sh.

Recipients:(name . Hilton Chain)(address . hako@ultrarare.space)

Message-ID:87h65ifspf.fsf@wolfsden.cz

Hilton Chain <hako@ultrarare.space> writes:

Toggle quote (8 lines)

> Please add sed and coreutils-minimal to inputs and use search-input-file or

> this-package-input instead.

> For smartmontools the proper way is to set --with-scriptpath='...' configure

> flag, which is documented in its INSTALL file. (It can be disabled with a 'no'

> value as well.)

Thank you for the review, I have sent v2 that hopefully incorporates

your feedback.

Tomas

There are only two hard things in Computer Science:

cache invalidation, naming things and off-by-one errors.

Hilton Chain wrote 1 months ago

Re: [bug#75550] [PATCH v2] gnu: smartmontools: Fix PATH in smartd_warning.sh.

Recipients:(name . Tomas Volf)(address . ~@wolfsden.cz)

Message-ID:87jzaccwo4.wl-hako@ultrarare.space

On Wed, 29 Jan 2025 07:00:15 +0800,

Tomas Volf wrote:

Toggle quote (32 lines)>
> The script started with reset of the $PATH to a value not suitable to Guix.
> In addition, the script requires coreutils and sed, so add those into the
> $PATH.
>
> * gnu/packages/admin.scm (smartmontools)[inputs]: Add sed, coreutils-minimal.
> [arguments]<#:configure-flags>: Pass --with-scriptpath=.
>
> Change-Id: Ide97f572e6f369fe24337f945474dc7a65584eda
> ---
>  gnu/packages/admin.scm | 13 ++++++++++---
>  1 file changed, 10 insertions(+), 3 deletions(-)
>
> diff --git a/gnu/packages/admin.scm b/gnu/packages/admin.scm
> index fd5d67ec50..e21f3f6e19 100644
> --- a/gnu/packages/admin.scm
> +++ b/gnu/packages/admin.scm
> @@ -3051,9 +3051,16 @@ (define-public smartmontools
>                  "0gcrzcb4g7f994n6nws26g6x15yjija1gyzd359sjv7r3xj1z9p9"))))
>      (build-system gnu-build-system)
>      (arguments
> -     (list #:make-flags
> -           #~(list "BUILD_INFO=\"(Guix)\"")))
> -    (inputs (list libcap-ng))
> +     (list
> +      #:make-flags
> +      #~(list "BUILD_INFO=\"(Guix)\"")
> +      #:configure-flags
> +      #~(list (format #f "--with-scriptpath=~a:~a:$PATH"
> +                      (dirname (search-input-file %build-inputs "bin/sed"))
> +                      (dirname (search-input-file %build-inputs "bin/true"))))))

I have modified #:configure-flags to use `this-package-input', to have proper

cross build support:

Toggle snippet (7 lines)

#~(list (format #f "--with-scriptpath=~{~a:~}$PATH"

(map (lambda (pkg)

(in-vicinity pkg "bin"))

'#$(list (this-package-input "coreutils-minimal")

(this-package-input "sed")))))

Thinking of the limitation of %build-inputs, it might be convenient to define

`inputs' and `native-inputs' directly in builder's environment. 🤔

Anyway, applied as f7fc4caf7da8f1d7cc76b3bc6ac0c4e643507454 with above change.

Toggle quote (9 lines)

> + (inputs (list coreutils-minimal

> + libcap-ng

> + sed))

> (home-page "https://www.smartmontools.org/")

> (synopsis "S.M.A.R.T. harddisk control and monitoring tools")

> (description

> --

> 2.47.1

Thanks

Closed

Your comment

This issue is archived.

To comment on this conversation send an email to 75550@debbugs.gnu.org

To respond to this issue using the mumi CLI, first switch to it

mumi current 75550

Then, you may apply the latest patchset in this issue (with sign off)

mumi am -- -s

Or, compose a reply to this issue

mumi compose

Or, send patches to this issue

mumi send-email *.patch

You may also tag this issue. See list of standard tags. For example, to set the confirmed and easy tags

mumi command -t +confirmed -t +easy

Or, remove the moreinfo tag and set the help tag

mumi command -t -moreinfo -t +help

is:open	open issues
is:done	closed issues
submitter:<who>	search issue submitter
author:<who>	search by message author
date:yesterday..now	search by issue date
mdate:3m..2d	search by message date