Re: master 7f97cf31dc3: Create blessmail at build time instead of install time

Re: master 7f97cf31dc3: Create blessmail at build time instead of install time

[Pip Cet]

"Robert Pluim" <rpluim@gmail.com> writes:

> branch: master
> commit 7f97cf31dc3eb483f84598f61e9f45805f901067
> Author: Robert Pluim <rpluim@gmail.com>
> Commit: Robert Pluim <rpluim@gmail.com>
>
>     Create blessmail at build time instead of install time
>
>     blessmail is built via the install target, which means it ends up owned
>     by the user doing the install.  It's not installed, so build it at build
>     time instead.
>
>     Reported by Michael Heerdegen <michael_heerdegen@web.de> in
>     <https://lists.gnu.org/archive/html/help-gnu-emacs/2024-08/msg00270.html>
>
>     * Makefile.in (install): Move blessmail target from install to actual-all.

All the patch appears to do is to remove blessmail from 'install', so
it's no longer run at all.  Maybe there's a second part to it that
wasn't committed?  Or am I missing something?

Pip

Re: : master 7f97cf31dc3: Create blessmail at build time instead of install time

[Robert Pluim]

>>>>> On Fri, 23 Aug 2024 12:17:26 +0000, Pip Cet <pipcet@protonmail.com> said:

    Pip> "Robert Pluim" <rpluim@gmail.com> writes:
    >> branch: master
    >> commit 7f97cf31dc3eb483f84598f61e9f45805f901067
    >> Author: Robert Pluim <rpluim@gmail.com>
    >> Commit: Robert Pluim <rpluim@gmail.com>
    >> 
    >> Create blessmail at build time instead of install time
    >> 
    >> blessmail is built via the install target, which means it ends up owned
    >> by the user doing the install.  It's not installed, so build it at build
    >> time instead.
    >> 
    >> Reported by Michael Heerdegen <michael_heerdegen@web.de> in
    >> <https://lists.gnu.org/archive/html/help-gnu-emacs/2024-08/msg00270.html>
    >> 
    >> * Makefile.in (install): Move blessmail target from install to actual-all.

    Pip> All the patch appears to do is to remove blessmail from 'install', so
    Pip> it's no longer run at all.  Maybe there's a second part to it that
    Pip> wasn't committed?  Or am I missing something?

Oops. I just pushed the 2nd half of the patch.

Robert
-- 

Re: : master 7f97cf31dc3: Create blessmail at build time instead of install time

[Pip Cet]

"Robert Pluim" <rpluim@gmail.com> writes:

>>>>>> On Fri, 23 Aug 2024 12:17:26 +0000, Pip Cet <pipcet@protonmail.com> said:
>
>     Pip> "Robert Pluim" <rpluim@gmail.com> writes:
>     >> branch: master
>     >> commit 7f97cf31dc3eb483f84598f61e9f45805f901067
>     >> Author: Robert Pluim <rpluim@gmail.com>
>     >> Commit: Robert Pluim <rpluim@gmail.com>
>     >>
>     >> Create blessmail at build time instead of install time
>     >>
>     >> blessmail is built via the install target, which means it ends up owned
>     >> by the user doing the install.  It's not installed, so build it at build
>     >> time instead.
>     >>
>     >> Reported by Michael Heerdegen <michael_heerdegen@web.de> in
>     >> <https://lists.gnu.org/archive/html/help-gnu-emacs/2024-08/msg00270.html>
>     >>
>     >> * Makefile.in (install): Move blessmail target from install to actual-all.
>
>     Pip> All the patch appears to do is to remove blessmail from 'install', so
>     Pip> it's no longer run at all.  Maybe there's a second part to it that
>     Pip> wasn't committed?  Or am I missing something?
>
> Oops. I just pushed the 2nd half of the patch.

I still don't understand this change, I'm afraid.  IIUC, the idea is
that this message is printed at install time, not at build time:

Assuming /usr/spool/mail is really the mail spool directory, you should
run  lib-src/blessmail /usr/local/libexec/emacs/31.0.50/x86_64-pc-linux-gnu/movemail
as root, to give  movemail  appropriate permissions.
Do that after running  make install.


It's not clear to me that building lib-src/blessmail at build time is
always safe: the user building Emacs might well be in a chroot jail or
restricted so they're unable to access 'rmail-spool-directory'.

As this is really a post-install dependency, I think we need to find a
better way of dealing with it.  Maybe we can do without "blessmail"
entirely, and have "blessmail.el" print the full instructions when run,
rather than putting them into "blessmail"?

Pip

Re: : : master 7f97cf31dc3: Create blessmail at build time instead of install time

[Robert Pluim]

>>>>> On Fri, 23 Aug 2024 13:03:00 +0000, Pip Cet <pipcet@protonmail.com> said:
    Pip> I still don't understand this change, I'm afraid.  IIUC, the idea is
    Pip> that this message is printed at install time, not at build time:

    Pip> Assuming /usr/spool/mail is really the mail spool directory, you should
    Pip> run  lib-src/blessmail /usr/local/libexec/emacs/31.0.50/x86_64-pc-linux-gnu/movemail
    Pip> as root, to give  movemail  appropriate permissions.
    Pip> Do that after running  make install.


    Pip> It's not clear to me that building lib-src/blessmail at build time is
    Pip> always safe: the user building Emacs might well be in a chroot jail or
    Pip> restricted so they're unable to access 'rmail-spool-directory'.

Building it at install time isnʼt safe either, since blessmail can end
up owned by 'root'.

    Pip> As this is really a post-install dependency, I think we need to find a
    Pip> better way of dealing with it.  Maybe we can do without "blessmail"
    Pip> entirely, and have "blessmail.el" print the full instructions when run,
    Pip> rather than putting them into "blessmail"?

That could be done. If we had a 'chown' subr it could even be elisp.

Robert
-- 

Re: : : master 7f97cf31dc3: Create blessmail at build time instead of install time

[Pip Cet]

"Robert Pluim" <rpluim@gmail.com> writes:

>>>>>> On Fri, 23 Aug 2024 13:03:00 +0000, Pip Cet <pipcet@protonmail.com> said:
>     Pip> I still don't understand this change, I'm afraid.  IIUC, the idea is
>     Pip> that this message is printed at install time, not at build time:
>
>     Pip> Assuming /usr/spool/mail is really the mail spool directory, you should
>     Pip> run  lib-src/blessmail /usr/local/libexec/emacs/31.0.50/x86_64-pc-linux-gnu/movemail
>     Pip> as root, to give  movemail  appropriate permissions.
>     Pip> Do that after running  make install.
>
>
>     Pip> It's not clear to me that building lib-src/blessmail at build time is
>     Pip> always safe: the user building Emacs might well be in a chroot jail or
>     Pip> restricted so they're unable to access 'rmail-spool-directory'.
>
> Building it at install time isnʼt safe either, since blessmail can end
> up owned by 'root'.

You're right, of course.

I wonder whether any popular distributors rely on 'blessmail' to do its
thing, in which case we'd probably need to continue having a
lib-src/blessmail executable, or whether it's exclusively end users, in
which case a simple message might be enough.

>     Pip> As this is really a post-install dependency, I think we need to find a
>     Pip> better way of dealing with it.  Maybe we can do without "blessmail"
>     Pip> entirely, and have "blessmail.el" print the full instructions when run,
>     Pip> rather than putting them into "blessmail"?
>
> That could be done. If we had a 'chown' subr it could even be elisp.

Well, we could call 'shell-command'.

My proposal would be something like this patch plus a new make target
"postinstall-blessmail", which would make blessmail.el run rather than
print the commands needed to bless movemail.  If that's even needed at
this point.

From a9d431584075f1d35f84004c995e42f3ea9ac149 Mon Sep 17 00:00:00 2001
From: Pip Cet <pipcet@protonmail.com>
Date: Sat, 24 Aug 2024 13:15:02 +0000
Subject: [PATCH] Don't create a blessmail executable

Instead, modify blessmail.el so it prints the instructions directly.

* Makefile.in (actual-all): Remove 'blessmail'
(install): Add 'blessmail'.
* lib-src/Makefile.in (blessmail): Remove target.
(need-blessmail): Call blessmail.el to print instructions.
* lisp/mail/blessmail.el: Print commands rather than writing them to
'blessmail'.
---
 Makefile.in            |  4 ++--
 lib-src/Makefile.in    | 16 ++------------
 lisp/mail/blessmail.el | 47 ++++++++++++++++++++++--------------------
 3 files changed, 29 insertions(+), 38 deletions(-)

diff --git a/Makefile.in b/Makefile.in
index 30a762ed03b..1b6c7eb05dc 100644
--- a/Makefile.in
+++ b/Makefile.in
@@ -381,7 +381,7 @@ bootstrap-all:
 
 .PHONY: bootstrap-all actual-all advice-on-failure sanity-check
 
-actual-all: ${SUBDIR} info $(gsettings_SCHEMAS:.xml=.valid) src-depending-on-lisp blessmail
+actual-all: ${SUBDIR} info $(gsettings_SCHEMAS:.xml=.valid) src-depending-on-lisp
 
 # ADVICE-ON-FAILURE-BEGIN:all
 # You could try to:
@@ -602,7 +602,7 @@ .PHONY:
 ## don't have to duplicate the list of utilities to install in
 ## this Makefile as well.
 
-install: actual-all install-arch-indep install-etcdoc install-arch-dep install-$(NTDIR) install-eln install-gsettings-schemas
+install: actual-all install-arch-indep install-etcdoc install-arch-dep install-$(NTDIR) blessmail install-eln install-gsettings-schemas
 	@true
 
 ## Ensure that $subdir contains a subdirs.el file.
diff --git a/lib-src/Makefile.in b/lib-src/Makefile.in
index 3cdf1620781..dd827b8df69 100644
--- a/lib-src/Makefile.in
+++ b/lib-src/Makefile.in
@@ -294,22 +294,10 @@ .PHONY:
 LOADLIBES = ../lib/libgnu.a $(LIBS_SYSTEM)
 $(EXE_FILES): ../lib/libgnu.a
 
-## Only used if we need blessmail, but no harm in always defining.
-## This makes the actual blessmail executable.
-blessmail: $(srcdir)/../lisp/mail/blessmail.el
-	$(AM_V_GEN)$(EMACS) $(EMACSOPT) -l $<
-	$(AM_V_at)chmod +x $@
-
 ## This checks if we need to run blessmail.
 ## Do not charge ahead and do it!  Let the installer decide.
-need-blessmail: blessmail
-	@if [ `wc -l <blessmail` != 2 ] ; then \
-	  dir=`sed -n -e 's/echo mail directory = \(.*\)/\1/p' blessmail`; \
-	  echo "Assuming $$dir is really the mail spool directory, you should"; \
-	  echo "run  lib-src/blessmail $(DESTDIR)${archlibdir}/movemail${EXEEXT}"; \
-	  echo "as root, to give  movemail${EXEEXT}  appropriate permissions."; \
-	  echo "Do that after running  make install."; \
-	fi
+need-blessmail: $(srcdir)/../lisp/mail/blessmail.el
+	@${EMACS} --batch --eval '(setq movemail-path "$(DESTDIR)${archlibdir}/movemail${EXEEXT}")' --load $<
 
 ## This is the target invoked by the top-level Makefile.
 maybe-blessmail: $(BLESSMAIL_TARGET)
diff --git a/lisp/mail/blessmail.el b/lisp/mail/blessmail.el
index 47e9bb58e06..6c3836f0c2f 100644
--- a/lisp/mail/blessmail.el
+++ b/lisp/mail/blessmail.el
@@ -23,8 +23,7 @@
 
 ;;; Commentary:
 
-;; This is loaded into a bare Emacs to create the blessmail script,
-;; which (on systems that need it) is used during installation
+;; This is loaded into Emacs to print instructions
 ;; to give appropriate permissions to movemail.
 ;;
 ;; It has to be done from Lisp in order to be sure of getting the
@@ -32,11 +31,11 @@
 
 ;;; Code:
 
-;; These are no longer needed because we run this in emacs instead of temacs.
-;; (message "Using load-path %s" load-path)
-;; (load "paths.el")
-;; It is not safe to load site-init.el here, because it might have things in it
-;; that won't load properly unless all the rest of Emacs is loaded.
+(defun blessmail-command (string)
+  (if (and (boundp 'blessmail-doit)
+           blessmail-doit)
+      (shell-command string)
+    (message (concat "  " string))))
 
 (let ((dirname (directory-file-name rmail-spool-directory))
       linkname attr modes)
@@ -45,24 +44,28 @@
     (setq dirname (if (file-name-absolute-p linkname)
 		      linkname
 		    (concat (file-name-directory dirname) linkname))))
-  (insert "#!/bin/sh\n")
   (setq attr (file-attributes dirname))
   (if (not (eq t (car attr)))
-      (insert (format "echo %s is not a directory\n" rmail-spool-directory))
+      (message (format "%s is not a directory\n" rmail-spool-directory))
     (setq modes (file-attribute-modes attr))
-    (cond ((= ?w (aref modes 8))
-	   ;; Nothing needs to be done.
-	   )
-	  ((= ?w (aref modes 5))
-	   (insert "chgrp " (number-to-string (file-attribute-group-id attr))
-		   " $* && chmod g+s $*\n"))
-	  ((= ?w (aref modes 2))
-	   (insert "chown " (number-to-string (file-attribute-user-id attr))
-		   " $* && chmod u+s $*\n"))
-	  (t
-	   (insert "chown root $* && chmod u+s $*\n"))))
-  (insert "echo mail directory = " dirname "\n"))
-(write-region (point-min) (point-max) "blessmail")
+    (unless (= ?w (aref modes 8))
+      (message (concat "Assuming %s is really the mail spool directory, you should\n"
+                       "run the following commands as root, to give  %s\n"
+                       "appropriate permissions:\n")
+               dirname (file-name-nondirectory movemail-path))
+      (cond
+       ((= ?w (aref modes 5))
+	(blessmail-command
+         (concat "chgrp " (number-to-string (file-attribute-group-id attr))
+		 " " movemail-path " && chmod g+s " movemail-path)))
+       ((= ?w (aref modes 2))
+        (blessmail-command
+         (concat "chown " (number-to-string (file-attribute-user-id attr))
+		 " " movemail-path " && chmod u+s " movemail-path)))
+       (t
+        (blessmail-command
+         (concat "chown root " movemail-path " && chmod u+s " movemail-path)))))
+    (message "\nDo that after running  make install.")))
 (kill-emacs)
 
 ;;; blessmail.el ends here
-- 
2.45.2

Re: : : master 7f97cf31dc3: Create blessmail at build time instead of install time

[Michael Heerdegen via Emacs development discussions.]

Pip Cet <pipcet@protonmail.com> writes:

> +(defun blessmail-command (string)
> +  (if (and (boundp 'blessmail-doit)
> +           blessmail-doit)
> +      (shell-command string)
> +    (message (concat "  " string))))

Better (message "%s" ...) maybe?  But my main point is:

> +    (unless (= ?w (aref modes 8))
> +      (message (concat "Assuming %s is really the mail spool directory, you should\n"
> +                       "run the following commands as root, to give  %s\n"
> +                       "appropriate permissions:\n")
> +               dirname (file-name-nondirectory movemail-path))
> +      (cond
> +       ((= ?w (aref modes 5))
> +	(blessmail-command
> +         (concat "chgrp " (number-to-string (file-attribute-group-id attr))
> +		 " " movemail-path " && chmod g+s " movemail-path)))
> +       ((= ?w (aref modes 2))
> +        (blessmail-command
> +         (concat "chown " (number-to-string (file-attribute-user-id attr))
> +		 " " movemail-path " && chmod u+s " movemail-path)))
> +       (t
> +        (blessmail-command
> +         (concat "chown root " movemail-path " && chmod u+s " movemail-path)))))
> +    (message "\nDo that after running  make install.")))
>  (kill-emacs)

Not an expert for shell stuff, but: wouldn't this cause trouble for
people using a non-posix compliant shell, like the popular fish shell?

AFAIU this will be called before the user can even set `shell-file-name'.


Michael.

Re: : : master 7f97cf31dc3: Create blessmail at build time instead of install time

[Pip Cet]

"Michael Heerdegen via \"Emacs development discussions.\"" <emacs-devel@gnu.org> writes:
> Pip Cet <pipcet@protonmail.com> writes:
>
>> +(defun blessmail-command (string)
>> +  (if (and (boundp 'blessmail-doit)
>> +           blessmail-doit)
>> +      (shell-command string)
>> +    (message (concat "  " string))))
>
> Better (message "%s" ...) maybe?  But my main point is:

Yes, you're right.

>> +    (unless (= ?w (aref modes 8))
>> +      (message (concat "Assuming %s is really the mail spool directory, you should\n"
>> +                       "run the following commands as root, to give  %s\n"
>> +                       "appropriate permissions:\n")
>> +               dirname (file-name-nondirectory movemail-path))
>> +      (cond
>> +       ((= ?w (aref modes 5))
>> +	(blessmail-command
>> +         (concat "chgrp " (number-to-string (file-attribute-group-id attr))
>> +		 " " movemail-path " && chmod g+s " movemail-path)))
>> +       ((= ?w (aref modes 2))
>> +        (blessmail-command
>> +         (concat "chown " (number-to-string (file-attribute-user-id attr))
>> +		 " " movemail-path " && chmod u+s " movemail-path)))
>> +       (t
>> +        (blessmail-command
>> +         (concat "chown root " movemail-path " && chmod u+s " movemail-path)))))
>> +    (message "\nDo that after running  make install.")))
>>  (kill-emacs)
>
> Not an expert for shell stuff, but: wouldn't this cause trouble for
> people using a non-posix compliant shell, like the popular fish shell?

You mean because of the "&&"?  That appears to work with a recent fish
shell version, but I'm not sure about other shells people might use as
their root shell.

> AFAIU this will be called before the user can even set `shell-file-name'.

I believe that's correct, but as the commands are only printed, so far,
it's maybe less of a problem.  With non-POSIX root shells, pretty much
all bets are off, though, aren't they?  It's not even clear sh -c
'chrgrp ... && chmod ...' would work?

Pip

Re: : : master 7f97cf31dc3: Create blessmail at build time instead of install time

[Michael Heerdegen]

Pip Cet <pipcet@protonmail.com> writes:

> I believe that's correct, but as the commands are only printed, so far,
> it's maybe less of a problem.  With non-POSIX root shells, pretty much
> all bets are off, though, aren't they?  It's not even clear sh -c
> 'chrgrp ... && chmod ...' would work?

They are only printed?  Then I don't see a problem.  Else I would have
suggested to try to avoid using a shell.

Michael.