unofficial mirror of guix-devel@gnu.org 
 help / color / mirror / code / Atom feed
* [PATCH] gnu: Add sendmail
@ 2016-09-16 16:21 John Darrington
  2016-09-17  4:51 ` Alex Vong
  0 siblings, 1 reply; 9+ messages in thread
From: John Darrington @ 2016-09-16 16:21 UTC (permalink / raw)
  To: guix-devel; +Cc: John Darrington

* gnu/packages/mail.scm (sendmail): New variable.
---
 gnu/packages/mail.scm | 80 +++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 80 insertions(+)

diff --git a/gnu/packages/mail.scm b/gnu/packages/mail.scm
index e344683..aff6a2c 100644
--- a/gnu/packages/mail.scm
+++ b/gnu/packages/mail.scm
@@ -1410,3 +1410,83 @@ provides HTML mail archiving with index, mail thread linking,
 etc; plus other capabilities including support for MIME and
 powerful user customization features.")
     (license gpl2+)))
+
+
+(define-public sendmail
+  (package
+    (name "sendmail")
+    (version "8.15.2")
+    (source
+     (origin
+       (method url-fetch)
+       (uri (string-append
+             "ftp://ftp.sendmail.org/pub/sendmail/sendmail."
+             version ".tar.gz"))
+       (sha256
+        (base32
+         "0fdl9ndmspqspdlmghzxlaqk56j3yajk52d7jxcg21b7sxglpy94"))))
+    (build-system gnu-build-system)
+    (arguments
+     `(#:tests? #f  ; There are no tests
+       #:phases
+       (modify-phases %standard-phases
+         (add-before 'build 'replace-/bin/sh
+           (lambda _
+             (substitute*
+                 (append
+                  (list "smrsh/smrsh.c" "sendmail/conf.c" "contrib/mailprio"
+                        "contrib/mmuegel" "devtools/bin/configure.sh")
+                  (find-files "." ".*\\.m4")
+                  (find-files "." ".*\\.cf"))
+               (("/bin/sh") (which "bash")))
+
+             (substitute* "devtools/bin/Build"
+               (("SHELL=/bin/sh") (string-append "SHELL=" (which "bash"))))
+             #t))
+         (replace 'configure
+           (lambda _
+
+             ;; Render harmless any attempts to chown or chmod
+             (substitute* "devtools/bin/install.sh"
+               (("owner=\\$2") "owner=''")
+               (("group=\\$2") "group=''"))
+
+             (with-output-to-file "devtools/Site/site.config.m4"
+               (lambda ()
+                 (format #t "
+define(`confCC', `gcc')
+define(`confOPTIMIZE', `-g -O2')
+define(`confLIBS', `-lresolv')
+define(`confINSTALL', `~a/devtools/bin/install.sh')
+define(`confDEPEND_TYPE', `CC-M')
+define(`confINST_DEP', `')
+" (getcwd))))))
+         (replace 'build
+           (lambda _
+             (system* "sh" "Build")
+             (with-directory-excursion "cf/cf"
+               (begin
+                 (copy-file "generic-linux.mc" "sendmail.mc")
+                 (zero? (system* "sh" "Build" "sendmail.cf"))))))
+         (add-before 'install 'pre-install
+           (lambda _
+             (let ((out (assoc-ref %outputs "out")))
+               (mkdir-p (string-append out "/usr/bin"))
+               (mkdir-p (string-append out "/usr/sbin"))
+               (mkdir-p (string-append out "/etc/mail"))
+               (setenv "DESTDIR" out)
+               (with-directory-excursion "cf/cf"
+                 (zero? (system* "sh" "Build" "install-cf")))))))))
+    (inputs
+     `(("m4" ,m4)
+       ("perl" ,perl)))
+    (home-page "http://sendmail.org")
+    (synopsis
+     "Highly configurable Mail Transfer Agent (MTA)")
+    (description
+     "Sendmail is a mail transfer agent (MTA) originally developed by Eric
+Allman.  It is highly configurable and supports many delivery methods and many
+transfer protocols.")
+    (license (non-copyleft "file://LICENSE"
+                           "See LICENSE in the distribution."))))
+
-- 
2.1.4

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

* Re: [PATCH] gnu: Add sendmail
  2016-09-16 16:21 [PATCH] gnu: Add sendmail John Darrington
@ 2016-09-17  4:51 ` Alex Vong
  2016-09-17  8:50   ` John Darrington
  0 siblings, 1 reply; 9+ messages in thread
From: Alex Vong @ 2016-09-17  4:51 UTC (permalink / raw)
  To: John Darrington; +Cc: guix-devel

Hello,

Thanks for the patch. I do not know how to set up a mail server, so I
can only comment on generic things. You will have to wait for sysadmin
to help :)

John Darrington <jmd@gnu.org> writes:

> * gnu/packages/mail.scm (sendmail): New variable.
> ---
>  gnu/packages/mail.scm | 80 +++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 80 insertions(+)
>
> diff --git a/gnu/packages/mail.scm b/gnu/packages/mail.scm
> index e344683..aff6a2c 100644
> --- a/gnu/packages/mail.scm
> +++ b/gnu/packages/mail.scm
> @@ -1410,3 +1410,83 @@ provides HTML mail archiving with index, mail thread linking,
>  etc; plus other capabilities including support for MIME and
>  powerful user customization features.")
>      (license gpl2+)))
> +
> +
> +(define-public sendmail
> +  (package
> +    (name "sendmail")
> +    (version "8.15.2")
> +    (source
> +     (origin
> +       (method url-fetch)
> +       (uri (string-append
> +             "ftp://ftp.sendmail.org/pub/sendmail/sendmail."
> +             version ".tar.gz"))
> +       (sha256
> +        (base32
> +         "0fdl9ndmspqspdlmghzxlaqk56j3yajk52d7jxcg21b7sxglpy94"))))
> +    (build-system gnu-build-system)
> +    (arguments
> +     `(#:tests? #f  ; There are no tests
There is a test/ directory in the tarball which needs seperate
compilation. The readme in the test/ directory says the test needs root
privilege, but this is not a problem, since the build daemon has root
privilege.

> +       #:phases
> +       (modify-phases %standard-phases
> +         (add-before 'build 'replace-/bin/sh
> +           (lambda _
> +             (substitute*
> +                 (append
> +                  (list "smrsh/smrsh.c" "sendmail/conf.c" "contrib/mailprio"
> +                        "contrib/mmuegel" "devtools/bin/configure.sh")
> +                  (find-files "." ".*\\.m4")
> +                  (find-files "." ".*\\.cf"))
I think this can be simplified to:
`("smrsh/smrsh.c" "sendmail/conf.c"
   "contrib/mailprio" "contrib/mmuegel"
   "devtools/bin/configure.sh"
   .@(find-files "." ".*\\.m4")
   ,@(find-files "." ".*\\.cf"))
using the quasi-quote quasi-unquote-splicing notation, which is similar
to string interpolation in shell "foo bar $(CAR) $(TAR)".

> +                        "contrib/mmuegel" "devtools/bin/configure.sh")
> +               (("/bin/sh") (which "bash")))
> +
> +             (substitute* "devtools/bin/Build"
> +               (("SHELL=/bin/sh") (string-append "SHELL=" (which "bash"))))
> +             #t))
I think the `#t' is not neccessary here, since `substitute*' uses
`substitute', which will either return #t or throw an exception.

> +         (replace 'configure
> +           (lambda _
> +
> +             ;; Render harmless any attempts to chown or chmod
> +             (substitute* "devtools/bin/install.sh"
> +               (("owner=\\$2") "owner=''")
> +               (("group=\\$2") "group=''"))
> +
> +             (with-output-to-file "devtools/Site/site.config.m4"
> +               (lambda ()
> +                 (format #t "
> +define(`confCC', `gcc')
> +define(`confOPTIMIZE', `-g -O2')
> +define(`confLIBS', `-lresolv')
> +define(`confINSTALL', `~a/devtools/bin/install.sh')
> +define(`confDEPEND_TYPE', `CC-M')
> +define(`confINST_DEP', `')
> +" (getcwd))))))
> +         (replace 'build
> +           (lambda _
> +             (system* "sh" "Build")
I think there is a missing `zero?' here.

> +             (with-directory-excursion "cf/cf"
> +               (begin
> +                 (copy-file "generic-linux.mc" "sendmail.mc")
> +                 (zero? (system* "sh" "Build" "sendmail.cf"))))))
> +         (add-before 'install 'pre-install
> +           (lambda _
> +             (let ((out (assoc-ref %outputs "out")))
> +               (mkdir-p (string-append out "/usr/bin"))
> +               (mkdir-p (string-append out "/usr/sbin"))
> +               (mkdir-p (string-append out "/etc/mail"))
> +               (setenv "DESTDIR" out)
> +               (with-directory-excursion "cf/cf"
> +                 (zero? (system* "sh" "Build" "install-cf")))))))))
> +    (inputs
> +     `(("m4" ,m4)
> +       ("perl" ,perl)))
> +    (home-page "http://sendmail.org")
> +    (synopsis
> +     "Highly configurable Mail Transfer Agent (MTA)")
> +    (description
> +     "Sendmail is a mail transfer agent (MTA) originally developed by Eric
> +Allman.  It is highly configurable and supports many delivery methods and many
> +transfer protocols.")
> +    (license (non-copyleft "file://LICENSE"
> +                           "See LICENSE in the distribution."))))
> +

Thanks,
Alex

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

* Re: [PATCH] gnu: Add sendmail
  2016-09-17  4:51 ` Alex Vong
@ 2016-09-17  8:50   ` John Darrington
  2016-09-17  9:03     ` John Darrington
  2016-09-17  9:38     ` Alex Vong
  0 siblings, 2 replies; 9+ messages in thread
From: John Darrington @ 2016-09-17  8:50 UTC (permalink / raw)
  To: Alex Vong; +Cc: guix-devel, John Darrington

[-- Attachment #1: Type: text/plain, Size: 6152 bytes --]

On Sat, Sep 17, 2016 at 12:51:20PM +0800, Alex Vong wrote:
     Hello,
     
     Thanks for the patch. I do not know how to set up a mail server, so I
     can only comment on generic things. You will have to wait for sysadmin
     to help :)
     
     John Darrington <jmd@gnu.org> writes:
     
     > * gnu/packages/mail.scm (sendmail): New variable.
     > ---
     >  gnu/packages/mail.scm | 80 +++++++++++++++++++++++++++++++++++++++++++++++++++
     >  1 file changed, 80 insertions(+)
     >
     > diff --git a/gnu/packages/mail.scm b/gnu/packages/mail.scm
     > index e344683..aff6a2c 100644
     > --- a/gnu/packages/mail.scm
     > +++ b/gnu/packages/mail.scm
     > @@ -1410,3 +1410,83 @@ provides HTML mail archiving with index, mail thread linking,
     >  etc; plus other capabilities including support for MIME and
     >  powerful user customization features.")
     >      (license gpl2+)))
     > +
     > +
     > +(define-public sendmail
     > +  (package
     > +    (name "sendmail")
     > +    (version "8.15.2")
     > +    (source
     > +     (origin
     > +       (method url-fetch)
     > +       (uri (string-append
     > +             "ftp://ftp.sendmail.org/pub/sendmail/sendmail."
     > +             version ".tar.gz"))
     > +       (sha256
     > +        (base32
     > +         "0fdl9ndmspqspdlmghzxlaqk56j3yajk52d7jxcg21b7sxglpy94"))))
     > +    (build-system gnu-build-system)
     > +    (arguments
     > +     `(#:tests? #f  ; There are no tests
     There is a test/ directory in the tarball which needs seperate
     compilation. The readme in the test/ directory says the test needs root
     privilege, but this is not a problem, since the build daemon has root
     privilege.

They need more than that.  They need the binary to be installed setuid.  So these
cannot be run as package tests.  They must be tested after the as-yet-to-be-written
service is installed.  But I'll update the comment to make it more clear.
     
     > +       #:phases
     > +       (modify-phases %standard-phases
     > +         (add-before 'build 'replace-/bin/sh
     > +           (lambda _
     > +             (substitute*
     > +                 (append
     > +                  (list "smrsh/smrsh.c" "sendmail/conf.c" "contrib/mailprio"
     > +                        "contrib/mmuegel" "devtools/bin/configure.sh")
     > +                  (find-files "." ".*\\.m4")
     > +                  (find-files "." ".*\\.cf"))
     I think this can be simplified to:
     `("smrsh/smrsh.c" "sendmail/conf.c"
        "contrib/mailprio" "contrib/mmuegel"
        "devtools/bin/configure.sh"
        .@(find-files "." ".*\\.m4")
        ,@(find-files "." ".*\\.cf"))
     using the quasi-quote quasi-unquote-splicing notation, which is similar
     to string interpolation in shell "foo bar $(CAR) $(TAR)".

Is that simpler?  It has more characters?
     
     > +                        "contrib/mmuegel" "devtools/bin/configure.sh")
     > +               (("/bin/sh") (which "bash")))
     > +
     > +             (substitute* "devtools/bin/Build"
     > +               (("SHELL=/bin/sh") (string-append "SHELL=" (which "bash"))))
     > +             #t))
     I think the `#t' is not neccessary here, since `substitute*' uses
     `substitute', which will either return #t or throw an exception.

WTF??   Didn't you complain earlier this week when I *didn't* put #t in exactly this
scenario??
     
     > +         (replace 'configure
     > +           (lambda _
     > +
     > +             ;; Render harmless any attempts to chown or chmod
     > +             (substitute* "devtools/bin/install.sh"
     > +               (("owner=\\$2") "owner=''")
     > +               (("group=\\$2") "group=''"))
     > +
     > +             (with-output-to-file "devtools/Site/site.config.m4"
     > +               (lambda ()
     > +                 (format #t "
     > +define(`confCC', `gcc')
     > +define(`confOPTIMIZE', `-g -O2')
     > +define(`confLIBS', `-lresolv')
     > +define(`confINSTALL', `~a/devtools/bin/install.sh')
     > +define(`confDEPEND_TYPE', `CC-M')
     > +define(`confINST_DEP', `')
     > +" (getcwd))))))
     > +         (replace 'build
     > +           (lambda _
     > +             (system* "sh" "Build")
     I think there is a missing `zero?' here.


Would that make any difference? Since only the last expression would get returned from lambda ?
     
     > +             (with-directory-excursion "cf/cf"
     > +               (begin
     > +                 (copy-file "generic-linux.mc" "sendmail.mc")
     > +                 (zero? (system* "sh" "Build" "sendmail.cf"))))))
     > +         (add-before 'install 'pre-install
     > +           (lambda _
     > +             (let ((out (assoc-ref %outputs "out")))
     > +               (mkdir-p (string-append out "/usr/bin"))
     > +               (mkdir-p (string-append out "/usr/sbin"))
     > +               (mkdir-p (string-append out "/etc/mail"))
     > +               (setenv "DESTDIR" out)
     > +               (with-directory-excursion "cf/cf"
     > +                 (zero? (system* "sh" "Build" "install-cf")))))))))
     > +    (inputs
     > +     `(("m4" ,m4)
     > +       ("perl" ,perl)))
     > +    (home-page "http://sendmail.org")
     > +    (synopsis
     > +     "Highly configurable Mail Transfer Agent (MTA)")
     > +    (description
     > +     "Sendmail is a mail transfer agent (MTA) originally developed by Eric
     > +Allman.  It is highly configurable and supports many delivery methods and many
     > +transfer protocols.")
     > +    (license (non-copyleft "file://LICENSE"
     > +                           "See LICENSE in the distribution."))))
     > +
     
     Thanks,
     Alex

-- 
Avoid eavesdropping.  Send strong encrypted email.
PGP Public key ID: 1024D/2DE827B3 
fingerprint = 8797 A26D 0854 2EAB 0285  A290 8A67 719C 2DE8 27B3
See http://sks-keyservers.net or any PGP keyserver for public key.


[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

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

* Re: [PATCH] gnu: Add sendmail
  2016-09-17  8:50   ` John Darrington
@ 2016-09-17  9:03     ` John Darrington
  2016-09-17  9:38     ` Alex Vong
  1 sibling, 0 replies; 9+ messages in thread
From: John Darrington @ 2016-09-17  9:03 UTC (permalink / raw)
  To: John Darrington; +Cc: guix-devel, John Darrington

[-- Attachment #1: Type: text/plain, Size: 1059 bytes --]

On Sat, Sep 17, 2016 at 10:50:35AM +0200, John Darrington wrote:
     On Sat, Sep 17, 2016 at 12:51:20PM +0800, Alex Vong wrote:
          
          > +                        "contrib/mmuegel" "devtools/bin/configure.sh")
          > +               (("/bin/sh") (which "bash")))
          > +
          > +             (substitute* "devtools/bin/Build"
          > +               (("SHELL=/bin/sh") (string-append "SHELL=" (which "bash"))))
          > +             #t))
          I think the `#t' is not neccessary here, since `substitute*' uses
          `substitute', which will either return #t or throw an exception.
     
     WTF??   Didn't you complain earlier this week when I *didn't* put #t in exactly this
     scenario??


Ahh no.  That was a different Alex.  I apologise.
          

J'


-- 
Avoid eavesdropping.  Send strong encrypted email.
PGP Public key ID: 1024D/2DE827B3 
fingerprint = 8797 A26D 0854 2EAB 0285  A290 8A67 719C 2DE8 27B3
See http://sks-keyservers.net or any PGP keyserver for public key.


[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

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

* Re: [PATCH] gnu: Add sendmail
  2016-09-17  8:50   ` John Darrington
  2016-09-17  9:03     ` John Darrington
@ 2016-09-17  9:38     ` Alex Vong
  2016-09-17 10:11       ` John Darrington
  1 sibling, 1 reply; 9+ messages in thread
From: Alex Vong @ 2016-09-17  9:38 UTC (permalink / raw)
  To: John Darrington; +Cc: guix-devel, John Darrington

Hi,

John Darrington <john@darrington.wattle.id.au> writes:

> They need more than that.  They need the binary to be installed
> setuid.  So these
> cannot be run as package tests.  They must be tested after the
> as-yet-to-be-written
> service is installed.  But I'll update the comment to make it more clear.
>
I see, so this is a circular dependency problem here. The service
depends on this package, but the test depends on the service. Am I
right?

>      > +       #:phases
>      > +       (modify-phases %standard-phases
>      > +         (add-before 'build 'replace-/bin/sh
>      > +           (lambda _
>      > +             (substitute*
>      > +                 (append
>      > + (list "smrsh/smrsh.c" "sendmail/conf.c" "contrib/mailprio"
>      > +                        "contrib/mmuegel" "devtools/bin/configure.sh")
>      > +                  (find-files "." ".*\\.m4")
>      > +                  (find-files "." ".*\\.cf"))
>      I think this can be simplified to:
>      `("smrsh/smrsh.c" "sendmail/conf.c"
>         "contrib/mailprio" "contrib/mmuegel"
>         "devtools/bin/configure.sh"
>         .@(find-files "." ".*\\.m4")
>         ,@(find-files "." ".*\\.cf"))
>      using the quasi-quote quasi-unquote-splicing notation, which is similar
>      to string interpolation in shell "foo bar $(CAR) $(TAR)".
>
> Is that simpler?  It has more characters?
>
Hmmm, it is more of a style thing. You can ask how others think about
it.

>      > +                        "contrib/mmuegel" "devtools/bin/configure.sh")
>      > +               (("/bin/sh") (which "bash")))
>      > +
>      > +             (substitute* "devtools/bin/Build"
>      > + (("SHELL=/bin/sh") (string-append "SHELL=" (which "bash"))))
>      > +             #t))
>      I think the `#t' is not neccessary here, since `substitute*' uses
>      `substitute', which will either return #t or throw an exception.
>
> WTF??  Didn't you complain earlier this week when I *didn't* put #t in
> exactly this
> scenario??
>
Yes, I am a different Alex :)
Also, it seems we are not being consistent here, sometimes we put `#t'
after `substitute*', sometimes we don't. Anyone has an idea?

>      > +         (replace 'configure
>      > +           (lambda _
>      > +
>      > +             ;; Render harmless any attempts to chown or chmod
>      > +             (substitute* "devtools/bin/install.sh"
>      > +               (("owner=\\$2") "owner=''")
>      > +               (("group=\\$2") "group=''"))
>      > +
>      > +             (with-output-to-file "devtools/Site/site.config.m4"
>      > +               (lambda ()
>      > +                 (format #t "
>      > +define(`confCC', `gcc')
>      > +define(`confOPTIMIZE', `-g -O2')
>      > +define(`confLIBS', `-lresolv')
>      > +define(`confINSTALL', `~a/devtools/bin/install.sh')
>      > +define(`confDEPEND_TYPE', `CC-M')
>      > +define(`confINST_DEP', `')
>      > +" (getcwd))))))
>      > +         (replace 'build
>      > +           (lambda _
>      > +             (system* "sh" "Build")
>      I think there is a missing `zero?' here.
>
>
> Would that make any difference? Since only the last expression would
> get returned from lambda ?
>      
You are right. The following should chain things up correctly:

(and (zero? (system* "sh" "Build"))
     (with-directory-excursion "cf/cf"
       (begin
         (copy-file "generic-linux.mc" "sendmail.mc")
         (zero? (system* "sh" "Build" "sendmail.cf")))))

>      > +             (with-directory-excursion "cf/cf"
>      > +               (begin
>      > +                 (copy-file "generic-linux.mc" "sendmail.mc")
>      > +                 (zero? (system* "sh" "Build" "sendmail.cf"))))))
>      > +         (add-before 'install 'pre-install
>      > +           (lambda _
>      > +             (let ((out (assoc-ref %outputs "out")))
>      > +               (mkdir-p (string-append out "/usr/bin"))
>      > +               (mkdir-p (string-append out "/usr/sbin"))
>      > +               (mkdir-p (string-append out "/etc/mail"))
>      > +               (setenv "DESTDIR" out)
>      > +               (with-directory-excursion "cf/cf"
>      > +                 (zero? (system* "sh" "Build" "install-cf")))))))))
>      > +    (inputs
>      > +     `(("m4" ,m4)
>      > +       ("perl" ,perl)))
>      > +    (home-page "http://sendmail.org")
>      > +    (synopsis
>      > +     "Highly configurable Mail Transfer Agent (MTA)")
>      > +    (description
>      > + "Sendmail is a mail transfer agent (MTA) originally developed
>      > by Eric
>      > +Allman.  It is highly configurable and supports many delivery
>      > methods and many
>      > +transfer protocols.")
>      > +    (license (non-copyleft "file://LICENSE"
>      > +                           "See LICENSE in the distribution."))))
>      > +
>      
>      Thanks,
>      Alex

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

* Re: [PATCH] gnu: Add sendmail
  2016-09-17  9:38     ` Alex Vong
@ 2016-09-17 10:11       ` John Darrington
  2016-09-18 19:14         ` Alex Kost
  0 siblings, 1 reply; 9+ messages in thread
From: John Darrington @ 2016-09-17 10:11 UTC (permalink / raw)
  To: Alex Vong; +Cc: guix-devel, John Darrington

[-- Attachment #1: Type: text/plain, Size: 3469 bytes --]

On Sat, Sep 17, 2016 at 05:38:26PM +0800, Alex Vong wrote:
     Hi,
     
     John Darrington <john@darrington.wattle.id.au> writes:
     
     > They need more than that.  They need the binary to be installed
     > setuid.  So these
     > cannot be run as package tests.  They must be tested after the
     > as-yet-to-be-written
     > service is installed.  But I'll update the comment to make it more clear.
     >
     I see, so this is a circular dependency problem here. The service
     depends on this package, but the test depends on the service. Am I
     right?

More or less :)  
     
     >      > +       #:phases
     >      > +       (modify-phases %standard-phases
     >      > +         (add-before 'build 'replace-/bin/sh
     >      > +           (lambda _
     >      > +             (substitute*
     >      > +                 (append
     >      > + (list "smrsh/smrsh.c" "sendmail/conf.c" "contrib/mailprio"
     >      > +                        "contrib/mmuegel" "devtools/bin/configure.sh")
     >      > +                  (find-files "." ".*\\.m4")
     >      > +                  (find-files "." ".*\\.cf"))
     >      I think this can be simplified to:
     >      `("smrsh/smrsh.c" "sendmail/conf.c"
     >         "contrib/mailprio" "contrib/mmuegel"
     >         "devtools/bin/configure.sh"
     >         .@(find-files "." ".*\\.m4")
     >         ,@(find-files "." ".*\\.cf"))
     >      using the quasi-quote quasi-unquote-splicing notation, which is similar
     >      to string interpolation in shell "foo bar $(CAR) $(TAR)".
     >
     > Is that simpler?  It has more characters?
     >
     Hmmm, it is more of a style thing. You can ask how others think about
     it.

I don't have any strong opinions on either style.
     
     >      > +                        "contrib/mmuegel" "devtools/bin/configure.sh")
     >      > +               (("/bin/sh") (which "bash")))
     >      > +
     >      > +             (substitute* "devtools/bin/Build"
     >      > + (("SHELL=/bin/sh") (string-append "SHELL=" (which "bash"))))
     >      > +             #t))
     >      I think the `#t' is not neccessary here, since `substitute*' uses
     >      `substitute', which will either return #t or throw an exception.
     >
     > WTF??  Didn't you complain earlier this week when I *didn't* put #t in
     > exactly this
     > scenario??
     >
     Yes, I am a different Alex :)
     Also, it seems we are not being consistent here, sometimes we put `#t'
     after `substitute*', sometimes we don't. Anyone has an idea?

I did raise some suggestions in my earlier posts.  But again I don't have any strong
opinion.
     
     > Would that make any difference? Since only the last expression would
     > get returned from lambda ?
     >      
     You are right. The following should chain things up correctly:
     
     (and (zero? (system* "sh" "Build"))
          (with-directory-excursion "cf/cf"
            (begin
              (copy-file "generic-linux.mc" "sendmail.mc")
              (zero? (system* "sh" "Build" "sendmail.cf")))))

You're right.  I'll do it that way.
     

Thanks,

J'

-- 
Avoid eavesdropping.  Send strong encrypted email.
PGP Public key ID: 1024D/2DE827B3 
fingerprint = 8797 A26D 0854 2EAB 0285  A290 8A67 719C 2DE8 27B3
See http://sks-keyservers.net or any PGP keyserver for public key.


[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

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

* Re: [PATCH] gnu: Add sendmail
  2016-09-17 10:11       ` John Darrington
@ 2016-09-18 19:14         ` Alex Kost
  2016-09-24  8:22           ` Alex Vong
  0 siblings, 1 reply; 9+ messages in thread
From: Alex Kost @ 2016-09-18 19:14 UTC (permalink / raw)
  To: John Darrington; +Cc: guix-devel, John Darrington

John Darrington (2016-09-17 12:11 +0200) wrote:

> On Sat, Sep 17, 2016 at 05:38:26PM +0800, Alex Vong wrote:
[...]
>      >      > +                        "contrib/mmuegel" "devtools/bin/configure.sh")
>      >      > +               (("/bin/sh") (which "bash")))
>      >      > +
>      >      > +             (substitute* "devtools/bin/Build"
>      >      > + (("SHELL=/bin/sh") (string-append "SHELL=" (which "bash"))))
>      >      > +             #t))
>      >      I think the `#t' is not neccessary here, since `substitute*' uses
>      >      `substitute', which will either return #t or throw an exception.
>      >
>      > WTF??  Didn't you complain earlier this week when I *didn't* put #t in
>      > exactly this
>      > scenario??
>      >
>      Yes, I am a different Alex :)
>      Also, it seems we are not being consistent here, sometimes we put `#t'
>      after `substitute*', sometimes we don't. Anyone has an idea?
>
> I did raise some suggestions in my earlier posts.  But again I don't have any strong
> opinion.

I have a strong opinion: if a docstring of a procedure says what value
it returns, we can rely on it, otherwise we should not guess what value
will be returned.  In case of 'substitute*' (and 'substitute'), the
returned value is not specified, so I think if a phase ends with
'substitute*', we should (or even must) add #t after it.

-- 
Alex

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

* Re: [PATCH] gnu: Add sendmail
  2016-09-18 19:14         ` Alex Kost
@ 2016-09-24  8:22           ` Alex Vong
  2016-09-24 17:50             ` Alex Kost
  0 siblings, 1 reply; 9+ messages in thread
From: Alex Vong @ 2016-09-24  8:22 UTC (permalink / raw)
  To: Alex Kost; +Cc: guix-devel, John Darrington

Alex Kost <alezost@gmail.com> writes:

> John Darrington (2016-09-17 12:11 +0200) wrote:
>
>> On Sat, Sep 17, 2016 at 05:38:26PM +0800, Alex Vong wrote:
> [...]
>>      >      > + "contrib/mmuegel" "devtools/bin/configure.sh")
>>      >      > +               (("/bin/sh") (which "bash")))
>>      >      > +
>>      >      > +             (substitute* "devtools/bin/Build"
>>      >      > + (("SHELL=/bin/sh") (string-append "SHELL=" (which "bash"))))
>>      >      > +             #t))
>>      >      I think the `#t' is not neccessary here, since `substitute*' uses
>>      >      `substitute', which will either return #t or throw an exception.
>>      >
>>      > WTF??  Didn't you complain earlier this week when I *didn't* put #t in
>>      > exactly this
>>      > scenario??
>>      >
>>      Yes, I am a different Alex :)
>>      Also, it seems we are not being consistent here, sometimes we put `#t'
>>      after `substitute*', sometimes we don't. Anyone has an idea?
>>
>> I did raise some suggestions in my earlier posts.  But again I don't
>> have any strong
>> opinion.
>
> I have a strong opinion: if a docstring of a procedure says what value
> it returns, we can rely on it, otherwise we should not guess what value
> will be returned.  In case of 'substitute*' (and 'substitute'), the
> returned value is not specified, so I think if a phase ends with
> 'substitute*', we should (or even must) add #t after it.

I see your point that one should not be relying on undocumented
features, which I agree. But I also see an alternative: to make
'substitute*' either return true or throw an exception and document
it. I think the heart of the problem is scheme is "untyped", so we rely
on the documentation. What do you think?

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

* Re: [PATCH] gnu: Add sendmail
  2016-09-24  8:22           ` Alex Vong
@ 2016-09-24 17:50             ` Alex Kost
  0 siblings, 0 replies; 9+ messages in thread
From: Alex Kost @ 2016-09-24 17:50 UTC (permalink / raw)
  To: Alex Vong; +Cc: guix-devel, John Darrington

Alex Vong (2016-09-24 16:22 +0800) wrote:

> Alex Kost <alezost@gmail.com> writes:
>
>> John Darrington (2016-09-17 12:11 +0200) wrote:
>>
>>> On Sat, Sep 17, 2016 at 05:38:26PM +0800, Alex Vong wrote:
>> [...]
>>>      >      > + "contrib/mmuegel" "devtools/bin/configure.sh")
>>>      >      > +               (("/bin/sh") (which "bash")))
>>>      >      > +
>>>      >      > +             (substitute* "devtools/bin/Build"
>>>      >      > + (("SHELL=/bin/sh") (string-append "SHELL=" (which "bash"))))
>>>      >      > +             #t))
>>>      >      I think the `#t' is not neccessary here, since `substitute*' uses
>>>      >      `substitute', which will either return #t or throw an exception.
>>>      >
>>>      > WTF??  Didn't you complain earlier this week when I *didn't* put #t in
>>>      > exactly this
>>>      > scenario??
>>>      >
>>>      Yes, I am a different Alex :)
>>>      Also, it seems we are not being consistent here, sometimes we put `#t'
>>>      after `substitute*', sometimes we don't. Anyone has an idea?
>>>
>>> I did raise some suggestions in my earlier posts.  But again I don't
>>> have any strong
>>> opinion.
>>
>> I have a strong opinion: if a docstring of a procedure says what value
>> it returns, we can rely on it, otherwise we should not guess what value
>> will be returned.  In case of 'substitute*' (and 'substitute'), the
>> returned value is not specified, so I think if a phase ends with
>> 'substitute*', we should (or even must) add #t after it.
>
> I see your point that one should not be relying on undocumented
> features, which I agree. But I also see an alternative: to make
> 'substitute*' either return true or throw an exception and document
> it. I think the heart of the problem is scheme is "untyped", so we rely
> on the documentation. What do you think?

I don't think my opinion matters, I'm not the man to decide.  As for me,
I would prefer if 'substitute*' returned #t if substitution was
successful and #f if it failed (if nothing was substituted).

OTOH 'substitute*' is only one of the several procedures that are used
often in phases; there also can be met such general guile procedures as
'setenv', 'copy-file', etc.  After them (in the end of the phases) we
have to use #t anyway.

-- 
Alex

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

end of thread, other threads:[~2016-09-24 17:52 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-09-16 16:21 [PATCH] gnu: Add sendmail John Darrington
2016-09-17  4:51 ` Alex Vong
2016-09-17  8:50   ` John Darrington
2016-09-17  9:03     ` John Darrington
2016-09-17  9:38     ` Alex Vong
2016-09-17 10:11       ` John Darrington
2016-09-18 19:14         ` Alex Kost
2016-09-24  8:22           ` Alex Vong
2016-09-24 17:50             ` Alex Kost

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).