[PATCH] Treat :tangle-mode as an octal value not integer

[PATCH] Treat :tangle-mode as an octal value not integer

[Jeremy Cowgar]

As an org user I would expect :tangle-mode 0660 to produce a file that
has user rw, group rw, other nothing. Instead, what really happens
currently is 0660 is treated as an integer which is actually
3140. This produces unexpected file permissions.

Prior to this patch to have rw,rw,none, one has to convert 0660 octal
into an integer (432) and then specify :tangle-mode 432. This is counter
intuitive and requires multiple steps.

After this patch, one just specifies the octal code as you do with
chmod. :tangle-mode 0660 results in a file that is rw,rw,none.
---
 lisp/ob-tangle.el | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/lisp/ob-tangle.el b/lisp/ob-tangle.el
index 2dd1d031c..154bd5145 100644
--- a/lisp/ob-tangle.el
+++ b/lisp/ob-tangle.el
@@ -246,7 +246,12 @@ matching a regular expression."
 			     (get-spec (lambda (name) (cdr (assq name (nth 4 spec)))))
 			     (she-bang (let ((sheb (funcall get-spec :shebang)))
 				         (when (> (length sheb) 0) sheb)))
-			     (tangle-mode (funcall get-spec :tangle-mode)))
+			     (tangle-mode (let ((tmode (funcall get-spec :tangle-mode)))
+                                            (when tmode
+                                              ;; convert integer representing an octal
+                                              ;; number to its real octal value
+                                              (string-to-number
+                                               (number-to-string tmode) 8)))))
 		        (unless (string-equal block-lang lang)
 			  (setq lang block-lang)
 			  (let ((lang-f (org-src-get-lang-mode lang)))
-- 
2.30.2

Re: [PATCH] Treat :tangle-mode as an octal value not integer

[tomas]

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

On Tue, Sep 28, 2021 at 10:54:48AM -0400, Jeremy Cowgar wrote:
> As an org user I would expect :tangle-mode 0660 to produce a file that
> has user rw, group rw, other nothing. Instead, what really happens
> currently is 0660 is treated as an integer which is actually
> 3140. This produces unexpected file permissions.
> 
> Prior to this patch to have rw,rw,none, one has to convert 0660 octal
> into an integer (432) and then specify :tangle-mode 432. This is counter
> intuitive and requires multiple steps.
> 
> After this patch, one just specifies the octal code as you do with
> chmod. :tangle-mode 0660 results in a file that is rw,rw,none.

Hm. This looks dangerous: interpreting an integer as octal just
due to the context?

I do understand why, but still...

Why not recommend Elisp's explicit syntax for octal representation,
i.e. :tangle-mode #o660? And put a prominent note in the docs,
of course.

Cheers
 - t

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

Re: [PATCH] Treat :tangle-mode as an octal value not integer

[Jeremy Cowgar]

On 2021-09-29 02:39, tomas@tuxteam.de wrote:
> On Tue, Sep 28, 2021 at 10:54:48AM -0400, Jeremy Cowgar wrote:
>> As an org user I would expect :tangle-mode 0660 to produce a file that
>> has user rw, group rw, other nothing. Instead, what really happens
>> currently is 0660 is treated as an integer which is actually
>> 3140. This produces unexpected file permissions.
>> 
> 
> Hm. This looks dangerous: interpreting an integer as octal just
> due to the context?
> 
> I do understand why, but still...
> 
> Why not recommend Elisp's explicit syntax for octal representation,
> i.e. :tangle-mode #o660? And put a prominent note in the docs,
> of course.

Are you suggesting this currently works or that the patch should be
changed to make that work? A quick try on my local system (pre-patch),
I receive the error:

   Wrong type argument: fixnump, "#o660"

--
Jeremy

Re: [PATCH] Treat :tangle-mode as an octal value not integer

[tomas]

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

On Wed, Sep 29, 2021 at 02:55:46AM -0400, Jeremy Cowgar wrote:
> On 2021-09-29 02:39, tomas@tuxteam.de wrote:
> >On Tue, Sep 28, 2021 at 10:54:48AM -0400, Jeremy Cowgar wrote:
> >>As an org user I would expect :tangle-mode 0660 to produce a file that
> >>has user rw, group rw, other nothing. Instead, what really happens
> >>currently is 0660 is treated as an integer which is actually
> >>3140. This produces unexpected file permissions.
> >>
> >
> >Hm. This looks dangerous: interpreting an integer as octal just
> >due to the context?
> >
> >I do understand why, but still...
> >
> >Why not recommend Elisp's explicit syntax for octal representation,
> >i.e. :tangle-mode #o660? And put a prominent note in the docs,
> >of course.
> 
> Are you suggesting this currently works or that the patch should be
> changed to make that work? A quick try on my local system (pre-patch),
> I receive the error:
> 
>   Wrong type argument: fixnump, "#o660"

Oh. It seems Org converts that to string based on its own
rules instead of on Elisp's.

Note that I'm not dead set /against/ your proposal. I was pointing
out a possible inconsistency and arguing for some reflection, however
it may turn out.

I'd have a slight preference for the #oXXX variant (and for Org
agreeing with Elisp on what a number is ;-) but it's just that,
a slight preference.

The UNIX shell's convention "leading zero to denote base 8" (coming 
from C, I don't know where that comes from) is one not very happy
hack, IMO. But if that's what is in people's muscle memory, so
be it ;-)

Cheers
 - t

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

Re: [PATCH] Treat :tangle-mode as an octal value not integer

[dkrm]

Jeremy Cowgar <jeremy@cowgar.com> writes:

> On 2021-09-29 02:39, tomas@tuxteam.de wrote:
>> On Tue, Sep 28, 2021 at 10:54:48AM -0400, Jeremy Cowgar wrote:
>>> As an org user I would expect :tangle-mode 0660 to produce a file that
>>> has user rw, group rw, other nothing. Instead, what really happens
>>> currently is 0660 is treated as an integer which is actually
>>> 3140. This produces unexpected file permissions.
>>> 
>> Hm. This looks dangerous: interpreting an integer as octal just
>> due to the context?
>> I do understand why, but still...
>> Why not recommend Elisp's explicit syntax for octal representation,
>> i.e. :tangle-mode #o660? And put a prominent note in the docs,
>> of course.
>
> Are you suggesting this currently works or that the patch should be
> changed to make that work? A quick try on my local system (pre-patch),
> I receive the error:
>
>   Wrong type argument: fixnump, "#o660"

You have to use the `identity` function for it to works: :tangle-mode (identity #o660)

Re: [PATCH] Treat :tangle-mode as an octal value not integer

[tomas]

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

On Wed, Sep 29, 2021 at 09:52:23AM +0200, dkrm wrote:
> 
> Jeremy Cowgar <jeremy@cowgar.com> writes:

[...]

> > Are you suggesting this currently works or that the patch should be
> > changed to make that work? A quick try on my local system (pre-patch),
> > I receive the error:
> >
> >   Wrong type argument: fixnump, "#o660"
> 
> You have to use the `identity` function for it to works: :tangle-mode (identity #o660)

Not the original poster, but I'd understand if they think
this is "too roundabout" to just express a file mode.

I think #o would have been acceptable, but changing that
now might break a lot of stuff out there (every param
beginning with `#o'?

OTOH, adding special rules for params seems at least as
dangerous.

Are there better ideas?

Cheers
 - t

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

Re: [PATCH] Treat :tangle-mode as an octal value not integer

[Gyro Funch]

On 2021-09-29 10:22 AM, tomas@tuxteam.de wrote:
> On Wed, Sep 29, 2021 at 09:52:23AM +0200, dkrm wrote:
>>
>> Jeremy Cowgar <jeremy@cowgar.com> writes:
> 
> [...]
> 
>>> Are you suggesting this currently works or that the patch should be
>>> changed to make that work? A quick try on my local system (pre-patch),
>>> I receive the error:
>>>
>>>    Wrong type argument: fixnump, "#o660"
>>
>> You have to use the `identity` function for it to works: :tangle-mode (identity #o660)
> 
> Not the original poster, but I'd understand if they think
> this is "too roundabout" to just express a file mode.
> 
> I think #o would have been acceptable, but changing that
> now might break a lot of stuff out there (every param
> beginning with `#o'?
> 
> OTOH, adding special rules for params seems at least as
> dangerous.
> 
> Are there better ideas?
> 
> Cheers
>   - t
> 

I don't know if it would ever be ambiguous, but could :tangle-mode have 
the ability to infer if it were integer- or octal-format based on 
checking against 'reasonable' permission settings in octal notation?

Kind regards,
gyro

Re: [PATCH] Treat :tangle-mode as an octal value not integer

[tomas]

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

On Wed, Sep 29, 2021 at 11:29:06AM +0200, Gyro Funch wrote:

[...]

> I don't know if it would ever be ambiguous, but could :tangle-mode
> have the ability to infer if it were integer- or octal-format based
> on checking against 'reasonable' permission settings in octal
> notation?

To me, that sounds rather scary. But I'm a timid person :-)

Cheers
 - t

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

Re: [PATCH] Treat :tangle-mode as an octal value not integer

[Jeremy Cowgar]

On 2021-09-29 07:07, tomas@tuxteam.de wrote:
> On Wed, Sep 29, 2021 at 11:29:06AM +0200, Gyro Funch wrote:
> 
> [...]
> 
>> I don't know if it would ever be ambiguous, but could :tangle-mode
>> have the ability to infer if it were integer- or octal-format based
>> on checking against 'reasonable' permission settings in octal
>> notation?
> 
> To me, that sounds rather scary. But I'm a timid person :-)
> 

To keep things from breaking, what if the system were smart and
if it sees #o prefix, it then parses as an octal, otherwise it keeps
it's current behavior?

Then add that to the docs. I understand about backwards compatibility
and that was my greatest fear with this patch when creating it.

Was just using org-babel-tangle for configuration files and had some
that I wanted to make 0700 and 0600. I soon learned, that didn't work
out as planned :-)

- Jeremy

Re: [PATCH] Treat :tangle-mode as an octal value not integer

[tomas]

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

On Wed, Sep 29, 2021 at 09:17:54AM -0400, Jeremy Cowgar wrote:
> On 2021-09-29 07:07, tomas@tuxteam.de wrote:
> >On Wed, Sep 29, 2021 at 11:29:06AM +0200, Gyro Funch wrote:
> >
> >[...]
> >
> >>I don't know if it would ever be ambiguous, but could :tangle-mode
> >>have the ability to infer if it were integer- or octal-format based
> >>on checking against 'reasonable' permission settings in octal
> >>notation?
> >
> >To me, that sounds rather scary. But I'm a timid person :-)
> >
> 
> To keep things from breaking, what if the system were smart and
> if it sees #o prefix, it then parses as an octal, otherwise it keeps
> it's current behavior?

That was roughly my idea: come closer to the Emacs Lisp int
representation (whether only for this case or more generally).

But I'm not deep enough in Org to even venture a recommendation.

> Then add that to the docs. I understand about backwards compatibility
> and that was my greatest fear with this patch when creating it.
> 
> Was just using org-babel-tangle for configuration files and had some
> that I wanted to make 0700 and 0600. I soon learned, that didn't work
> out as planned :-)

I definitely understand your surprise. I know it the other way
around. Back then (TM), it was customary in Windows to write out
the leading zeros in the IPv4 octets, to fill them up to three
places. Something like 192.168.042.001 -- Heavens knows why.

Unix utilities interpret those with a leading zero as an octal
representation (that's how atoi() or strtol() in its default
mode work). I had hours of fun with my Windows colleagues ;-)

Cheers
 - t

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

Re: [PATCH] Treat :tangle-mode as an octal value not integer

[Greg Minshall]

Tomas,

> Why not recommend Elisp's explicit syntax for octal representation,
> i.e. :tangle-mode #o660? And put a prominent note in the docs,
> of course.

as much as my fingers are used to "0660 ==> octal", this probably makes
sense.

on the other hand, to protect users, might it be worthwhile ("just due
to the context") to flag ":tangle-mode 0..." integers as an error for
the user to deal with?  (and point them at, e.g., #o660.)

cheers, Greg

Re: [PATCH] Treat :tangle-mode as an octal value not integer

[Bastien]

Hi Jeremy,

Jeremy Cowgar <jeremy@cowgar.com> writes:

> As an org user I would expect :tangle-mode 0660 to produce a file that
> has user rw, group rw, other nothing. Instead, what really happens
> currently is 0660 is treated as an integer which is actually
> 3140. This produces unexpected file permissions.

(Just re-adding this as a patch for updates.orgmode.org.)

-- 
 Bastien

Re: [PATCH] Treat :tangle-mode as an octal value not integer

[Timothy]

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

Hi  Jeremy,

> As an org user I would expect :tangle-mode 0660 to produce a file that
> has user rw, group rw, other nothing. Instead, what really happens
> currently is 0660 is treated as an integer which is actually
> 3140. This produces unexpected file permissions.

I agree that :tangle-mode could be more user-friendly. However, I think we can
go further. Currently, only (identity #o755 / 493) works, however I think it would be
good if it worked like chmod and accepted most of the following forms:
⁃ `#o755'
⁃ `755' (because people are used to this, as technically misleading as it may be,
  as long as we can tell “:tangle-mode 356” from “:tangle-mode (identity #o544)”)
⁃ `rw' (equivalent to a=rw, and so #o555)
⁃ `a=rw,u+x' (equivalent to #o755) [hardest to support, so maybe?]

And then I’d also be in favour of accepting
⁃ `rw-r--r--' (equivalent to #o544)

I think as long as it’s clear what’s intended, and it’s not some home-baked
non-standard format, or terribly annoying to support — why not?

All the best,
Timothy

Re: [PATCH] Treat :tangle-mode as an octal value not integer

[tomas]

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

On Wed, Sep 29, 2021 at 09:48:47PM +0800, Timothy wrote:
> Hi  Jeremy,
> 
> > As an org user I would expect :tangle-mode 0660 to produce a file that
> > has user rw, group rw, other nothing. Instead, what really happens
> > currently is 0660 is treated as an integer which is actually
> > 3140. This produces unexpected file permissions.
> 
> I agree that :tangle-mode could be more user-friendly. However, I think we can
> go further. Currently, only (identity #o755 / 493) works, however I think it would be
> good if it worked like chmod and accepted most of the following forms:
> ⁃ `#o755'
> ⁃ `755' (because people are used to this, as technically misleading as it may be,
>   as long as we can tell “:tangle-mode 356” from “:tangle-mode (identity #o544)”)
> ⁃ `rw' (equivalent to a=rw, and so #o555)
> ⁃ `a=rw,u+x' (equivalent to #o755) [hardest to support, so maybe?]

So you favour going the "full custom special parser". You're much more
involved in Org, so I think your gut feeling counts more than mine here :)

`755' is the funniest one, since, strictly speaking it doesn't correspond
to anything "out there" (note that the shell command `chmod' wants the
leading zero, or, well, it will do surprising things if you don't
provide it ;-)

But then, if it is clear that :tangle-mode is doing its own parsing,
it won't matter much.

Cheers
 - t

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

Re: [PATCH] Treat :tangle-mode as an octal value not integer

[Timothy]

<tomas@tuxteam.de> writes:

> So you favour going the "full custom special parser". You're much more
> involved in Org, so I think your gut feeling counts more than mine here :)

Well, I'm not sure that my feeling is representative of experienced Org users,
my opinion basically boils down to:

>> I think as long as it’s clear what’s intended, and it’s not some home-baked
>> non-standard format, or terribly annoying to support — why not?

Rephrased: I think there are a few arguably "sensible" formats that a user could
reasonably assume, and if we can support most of them without introducing
ambiguity in parsing or interpretation (and I think we can), can't we make
everyone happy?

> `755' is the funniest one, since, strictly speaking it doesn't correspond
> to anything "out there" (note that the shell command `chmod' wants the
> leading zero, or, well, it will do surprising things if you don't
> provide it ;-)

Consider my suggestion amended to 0755 etc. :)

Anyway, as an example here's a code snippet that implements everything I've
mentioned.

#+begin_src emacs-lisp
(defvar org-tangle-default-mode #o544
  "The default mode for tangled files, as an integer.")

(defun org-interpret-file-mode (mode)
  (cond
   ((integerp mode) mode)
   ((not (stringp mode))
    (user-error "File mode %S not recognised as a valid format." mode))
   ((string-match-p "^0[0-7][0-7][0-7]$" mode)
    (string-to-number mode 8))
   ((string-match-p "^#o[0-7][0-7][0-7]$" mode)
    (string-to-number (substring mode 2) 8))
   ((string-match-p "^[ugoa]*\\(?:[+-=][rwxXstugo]*\\)+\\(,[ugoa]*\\(?:[+-=][rwxXstugo]*\\)+\\)*$" mode)
    (file-modes-symbolic-to-number mode 0))
   ((string-match-p "^[rwx-]\\{3\\}$" mode)
    (file-modes-symbolic-to-number (concat "u=" mode) org-tangle-default-mode))
   ((string-match-p "^[rwx-]\\{9\\}$" mode)
    (file-modes-symbolic-to-number (concat  "u=" (substring mode 0 3)
                                           ",g=" (substring mode 3 6)
                                           ",a=" (substring mode 6 9))
                                   org-tangle-default-mode))
   (t (user-error "File mode %S not recognised as a valid format." mode))))
#+end_src

--
Timothy

Re: [PATCH] Treat :tangle-mode as an octal value not integer

[Jeremy Cowgar]

On 2021-09-29 10:58, Timothy wrote:
>>> I think as long as it’s clear what’s intended, and it’s not some 
>>> home-baked
>>> non-standard format, or terribly annoying to support — why not?
> 
> Anyway, as an example here's a code snippet that implements everything 
> I've
> mentioned.
> 
> ... code removed for brevity ...
> 

I love it! Much better than my proposed patch.

--
Jeremy

Re: [PATCH] Treat :tangle-mode as an octal value not integer

[Timothy]

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

Hi  Jeremy,

> I love it! Much better than my proposed patch.

I’m about to send a patch based on my snippet, so I’m marking this patch as cancelled.

All the best,
Timothy

Re: [PATCH] Treat :tangle-mode as an octal value not integer

[tomas]

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

On Wed, Sep 29, 2021 at 10:58:43PM +0800, Timothy wrote:
> 
> <tomas@tuxteam.de> writes:
> 
> > So you favour going the "full custom special parser". You're much more
> > involved in Org, so I think your gut feeling counts more than mine here :)
> 
> Well, I'm not sure that my feeling is representative of experienced Org users,
> my opinion basically boils down to:

Given your activity of last, I'm certain that your take has
more weight than mine :)

[...]

Beautiful :)

Thanks
 - t

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

Re: [PATCH] Treat :tangle-mode as an octal value not integer

[Greg Minshall]

Tomas,

in fact, i'm quite used to doing `chmod 755 foo.org`.  i do it now in
bash, used to do it in csh, and it seems to work (as expected, afaict)
also in sh.  all on arch linux.  (`chmod +755 foo.org` *does* seem to
give odd results. :)

cheers, Greg

Re: [PATCH] Treat :tangle-mode as an octal value not integer

[tomas]

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

On Wed, Sep 29, 2021 at 08:18:11PM +0300, Greg Minshall wrote:
> Tomas,
> 
> in fact, i'm quite used to doing `chmod 755 foo.org`.  i do it now in
> bash, used to do it in csh, and it seems to work (as expected, afaict)
> also in sh.  all on arch linux.  (`chmod +755 foo.org` *does* seem to
> give odd results. :)

D'oh. You are right. Actually, chmod from coreutils does its own
parsing (function mode_compile, online e.g. here [1]).

Cheers

[1] https://sources.debian.org/src/coreutils/8.32-4/lib/modechange.c/#L134

 - t

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

Re: [PATCH] Treat :tangle-mode as an octal value not integer

[Timothy]

Just removing this from updates.orgmode.org.