GNU bug report logs - #54588
29.0.50; [PATCH] Don't use `eshell-convert' when not needed / Fix setting umask in Eshell

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 54588 in the body.
You can then email your comments to 54588 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Jim Porter <jporterbugs <at> gmail.com>
To: bug-gnu-emacs <at> gnu.org
Subject: 29.0.50; [PATCH] Don't use `eshell-convert' when not needed / Fix
 setting umask in Eshell
Date: Sat, 26 Mar 2022 15:40:00 -0700

[Message part 1 (text/plain, inline)]

There are two tightly-coupled reasons for this bug: first, I want to 
make some future improvements to `eshell-convert', so to prevent any 
potential for breakage, I wanted to reduce the number of places that use 
it when a simpler alternative exists.

Second, there's an obscure bug in setting the umask in Eshell when you 
pass it an actual number (as opposed to a numeric string). From "emacs 
-Q --eval '(eshell)'":

  ~ $ umask
  002
  ~ $ umask 222
  Warning: umask changed for all new files created by Emacs.
  ~ $ umask
  222
  ~ $ umask $(identity #o222)
  Warning: umask changed for all new files created by Emacs.
  ~ $ umask
  146

The code is pretty complex, so I'll explain what's happening under the 
hood. When calling `umask 222', the "222" is converted to a decimal 
number by Eshell and passed to `eshell/umask'; then, 
`eshell-eval-using-options' converts that number (again, in decimal) 
back to a string. Next, `eshell/umask' calls `eshell-convert' to convert 
it *back* to a decimal number. If that worked, it calls 
`number-to-string' to convert it to a string again, then turns it into a 
character escape sequence like "?\222" and finally calls 
`read-from-string' on that to get a number.

The `umask $(identity #o222)' case is similar, except that Eshell 
doesn't need to do the initial string-to-number conversion. However, 
then `eshell-eval-using-options' gets confused since it converts the 
value to a decimal string, throwing off the subsequent conversions.

In my patch, the behavior is changed as follows. First, when calling 
`umask 222', the "222" is passed as a string, with no conversion. Next, 
`eshell-eval-using-options' is set to preserve arguments, so if you pass 
an actual number (as in `umask $(identity #o222)'), it doesn't try to 
stringify it. Then, if the argument is a string, `eshell/umask' converts 
it to an octal number. Finally, the numeric argument is used to set the 
umask.

I also simplified the code for printing the umask a bit. That part just 
splits the symbolic and non-symbolic cases up so there's less duplicated 
work, plus simplifies the `format' call for the non-symbolic case.

[0001-Add-tests-for-Eshell-s-umask-command.patch (text/plain, attachment)]

[0002-Don-t-use-eshell-convert-when-all-we-want-is-a-numbe.patch (text/plain, attachment)]

Message #8 received at 54588 <at> debbugs.gnu.org (full text, mbox):

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Jim Porter <jporterbugs <at> gmail.com>
Cc: 54588 <at> debbugs.gnu.org
Subject: Re: bug#54588: 29.0.50; [PATCH] Don't use `eshell-convert' when not
 needed / Fix setting umask in Eshell
Date: Tue, 29 Mar 2022 16:51:36 +0200

Jim Porter <jporterbugs <at> gmail.com> writes:

> In my patch, the behavior is changed as follows. First, when calling
> `umask 222', the "222" is passed as a string, with no
> conversion. Next, `eshell-eval-using-options' is set to preserve
> arguments, so if you pass an actual number (as in `umask $(identity
> #o222)'), it doesn't try to stringify it. Then, if the argument is a
> string, `eshell/umask' converts it to an octal number. Finally, the
> numeric argument is used to set the umask.
>
> I also simplified the code for printing the umask a bit. That part
> just splits the symbolic and non-symbolic cases up so there's less
> duplicated work, plus simplifies the `format' call for the
> non-symbolic case.

Makes sense to me; pushed to Emacs 29.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

Message #13 received at 54588 <at> debbugs.gnu.org (full text, mbox):

From: Jim Porter <jporterbugs <at> gmail.com>
To: Lars Ingebrigtsen <larsi <at> gnus.org>
Cc: 54588 <at> debbugs.gnu.org
Subject: Re: bug#54588: 29.0.50; [PATCH] Don't use `eshell-convert' when not
 needed / Fix setting umask in Eshell
Date: Tue, 29 Mar 2022 09:30:47 -0700

On 3/29/2022 7:51 AM, Lars Ingebrigtsen wrote:
> Makes sense to me; pushed to Emacs 29.

Thanks.

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.