* bug#16133: 24.3; copy-file fails on chmod when copying to FAT filesystem @ 2013-12-13 19:51 Artem Chuprina 2013-12-13 22:51 ` Glenn Morris ` (2 more replies) 0 siblings, 3 replies; 33+ messages in thread From: Artem Chuprina @ 2013-12-13 19:51 UTC (permalink / raw) To: 16133 Please describe exactly what actions triggered the bug, and the precise symptoms of the bug. If you can, give a recipe starting from `emacs -Q': Having org on ext4 filesystem with 0664 permission on org/work.org, and c1 on FAT filesystem with file permissions 0075 (in fact, SD card on Android system, so such strange permissions) I do M-: copy-file("org/work.org" "c1/work.org" t) and get an error Debugger entered--Lisp error: (file-error "Doing chmod" "operation not permitted" $ copy-file("org/work.org" "c1/work.org" t) eval((copy-file "org/work.org" "c1/work.org" t) nil) eval-expression((copy-file "org/work.org" "c1/work.org" t) nil) call-interactively(eval-expression nil nil) That's right, it cannot do a chmod, but it cannot do it there at all, so there is no sense to fail here. The problem does not exist in GNU Emacs 23.4.1 (arm-unknown-linux-gnueabihf) of 2013-07-01 on henze, modified by Debian In GNU Emacs 24.3.1 (arm-unknown-linux-gnueabihf) of 2013-10-01 on hummel, modified by Debian Configured using: `configure '--build' 'arm-linux-gnueabihf' '--build' 'arm-linux-gnueabihf' '--prefix=/usr' '--sharedstatedir=/var/lib' '--libexecdir=/usr/lib' '--localstatedir=/var/lib' '--infodir=/usr/share/info' '--mandir=/usr/share/man' '--with-pop=yes' '--enable-locallisppath=/etc/emacs24:/etc/emacs:/usr/local/share/emacs/24.3/site-l\ isp:/usr/local/share/emacs/site-lisp:/usr/share/emacs/24.3/site-lisp:/usr/share/em\ acs/site-lisp' '--with-crt-dir=/usr/lib/arm-linux-gnueabihf' '--with-x=no' '--without-gconf' '--without-gsettings' 'build_alias=arm-linux-gnueabihf' 'CFLAGS=-g -O2 -fstack-protector --param=ssp-buffer-size=4 -Wformat -Werror=format-security -Wall' 'LDFLAGS=-Wl,-z,relro' 'CPPFLAGS=-D_FORTIFY_SOURCE=2'' Important settings: value of $LC_MESSAGES: C value of $LANG: en_US.UTF-8 locale-coding-system: utf-8-unix default enable-multibyte-characters: t Major mode: Fundamental Recent input: ESC : ( c o p y - f i l e SPC " o r g / e m a c s . DEL DEL DEL DEL DEL DEL w o r k . o r g " SPC " c 1 / w o r k . o r g " ) RET C-] ^ permalink raw reply [flat|nested] 33+ messages in thread
* bug#16133: 24.3; copy-file fails on chmod when copying to FAT filesystem 2013-12-13 19:51 bug#16133: 24.3; copy-file fails on chmod when copying to FAT filesystem Artem Chuprina @ 2013-12-13 22:51 ` Glenn Morris 2013-12-13 22:55 ` Glenn Morris 2013-12-14 10:10 ` Artem Chuprina 2013-12-20 23:27 ` Paul Eggert 2013-12-22 0:01 ` Paul Eggert 2 siblings, 2 replies; 33+ messages in thread From: Glenn Morris @ 2013-12-13 22:51 UTC (permalink / raw) To: Artem Chuprina; +Cc: 16133 Artem Chuprina wrote: > Having org on ext4 filesystem with 0664 permission on org/work.org, and > c1 on FAT filesystem with file permissions 0075 (in fact, SD card on > Android system, so such strange permissions) I do > > M-: copy-file("org/work.org" "c1/work.org" t) > > and get an error > > Debugger entered--Lisp error: (file-error "Doing chmod" "operation not > permitted" $ I'm not sure this is a bug. The file is copied, but there is an error trying to preserve permissions. Because eval-expression-debug-on-error is non-nil by default, the debugger pops up. Emacs's behaviour seems the same as using cp -a org/work.org c1/work.org on the command-line. "-a" is appropriate because C-h f copy-file says: This function always sets the file modes of the output file to match the input file. ^ permalink raw reply [flat|nested] 33+ messages in thread
* bug#16133: 24.3; copy-file fails on chmod when copying to FAT filesystem 2013-12-13 22:51 ` Glenn Morris @ 2013-12-13 22:55 ` Glenn Morris 2013-12-14 10:10 ` Artem Chuprina 1 sibling, 0 replies; 33+ messages in thread From: Glenn Morris @ 2013-12-13 22:55 UTC (permalink / raw) To: Artem Chuprina; +Cc: 16133 Glenn Morris wrote: > Emacs's behaviour seems the same as using > > cp -a org/work.org c1/work.org > > on the command-line. "cp --preserve=mode" would have been a better analogy. ^ permalink raw reply [flat|nested] 33+ messages in thread
* bug#16133: 24.3; copy-file fails on chmod when copying to FAT filesystem 2013-12-13 22:51 ` Glenn Morris 2013-12-13 22:55 ` Glenn Morris @ 2013-12-14 10:10 ` Artem Chuprina 2013-12-14 20:19 ` Glenn Morris 1 sibling, 1 reply; 33+ messages in thread From: Artem Chuprina @ 2013-12-14 10:10 UTC (permalink / raw) To: Glenn Morris; +Cc: 16133 Glenn Morris -> Artem Chuprina @ Fri, 13 Dec 2013 17:51:34 -0500: >> Having org on ext4 filesystem with 0664 permission on org/work.org, and >> c1 on FAT filesystem with file permissions 0075 (in fact, SD card on >> Android system, so such strange permissions) I do >> >> M-: copy-file("org/work.org" "c1/work.org" t) >> >> and get an error >> >> Debugger entered--Lisp error: (file-error "Doing chmod" "operation not >> permitted" $ GM> I'm not sure this is a bug. GM> The file is copied, but there is an error trying to preserve permissions. GM> Because eval-expression-debug-on-error is non-nil by default, the GM> debugger pops up. GM> Emacs's behaviour seems the same as using GM> cp -a org/work.org c1/work.org GM> on the command-line. "-a" is appropriate because C-h f copy-file says: GM> This function always sets the file modes of the output file to match GM> the input file. The problem is that now it is impossible to use other functions that use copy-file. I've encountered this bug trying to use org-mobile-push that, among other, copies several files to its target directory (which I want to be on FAT partition for Android's MobileOrg). On the first such file, copy-file throws an error, and whole operation stops there and so fails. org-mobile-push catches error and don't enter a debugger, but it don't work either, thinking that it failed to copy file. Yet file is indeed copied, just (totally unneeded in this case) chmod failed. I think that it is a good idea to _attempt to_ copy permissions. But it is a bad idea to throw an error if file is copied well but we cannot set permissions. These days it is very common to copy files between incompatible file systems. Most of us use FAT, many use CIFS, there are also NFS, davfs, sshfs etc. So I think that a reasonable behavior would be to try to set permissions, but don't fail on chmod error. At least, on copy-file level (if chmod function itself throws an error, this is ok, but copy-file should not). ^ permalink raw reply [flat|nested] 33+ messages in thread
* bug#16133: 24.3; copy-file fails on chmod when copying to FAT filesystem 2013-12-14 10:10 ` Artem Chuprina @ 2013-12-14 20:19 ` Glenn Morris 2013-12-14 20:46 ` Josh ` (3 more replies) 0 siblings, 4 replies; 33+ messages in thread From: Glenn Morris @ 2013-12-14 20:19 UTC (permalink / raw) To: Artem Chuprina; +Cc: Paul Eggert, 16133 I think you are right that it is a problem. I think the problem was in documenting copy-file to always copy permissions in the first place. But I imagine it is far too late to change that. Not sure what the right thing to do is here... Someone may be relying on the fact that copy-file copies permissions, and want it it throw an error if it fails: http://debbugs.gnu.org/cgi/bugreport.cgi?bug=8306 Others, like you, may not care. Since copy-file has only relatively recently starting giving an error, maybe the least bad thing would be for it to give a message instead of an error? I don't know how we tidy this up for the future though. Add yet another optional argument "error-if-permissions-fail"? Yuck. ^ permalink raw reply [flat|nested] 33+ messages in thread
* bug#16133: 24.3; copy-file fails on chmod when copying to FAT filesystem 2013-12-14 20:19 ` Glenn Morris @ 2013-12-14 20:46 ` Josh 2013-12-14 20:57 ` Eli Zaretskii 2013-12-14 20:55 ` Eli Zaretskii ` (2 subsequent siblings) 3 siblings, 1 reply; 33+ messages in thread From: Josh @ 2013-12-14 20:46 UTC (permalink / raw) To: Glenn Morris; +Cc: Paul Eggert, Artem Chuprina, 16133 On Sat, Dec 14, 2013 at 12:19 PM, Glenn Morris <rgm@gnu.org> wrote: > I don't know how we tidy this up for the future though. > Add yet another optional argument "error-if-permissions-fail"? Yuck. It might be reasonable to error only when the existing PRESERVE-UID-GID argument was non-nil. ^ permalink raw reply [flat|nested] 33+ messages in thread
* bug#16133: 24.3; copy-file fails on chmod when copying to FAT filesystem 2013-12-14 20:46 ` Josh @ 2013-12-14 20:57 ` Eli Zaretskii 2013-12-14 21:21 ` Josh 0 siblings, 1 reply; 33+ messages in thread From: Eli Zaretskii @ 2013-12-14 20:57 UTC (permalink / raw) To: Josh; +Cc: ran, 16133, eggert > From: Josh <josh@foxtail.org> > Date: Sat, 14 Dec 2013 12:46:59 -0800 > Cc: Paul Eggert <eggert@cs.ucla.edu>, Artem Chuprina <ran@lasgalen.net>, > 16133@debbugs.gnu.org > > On Sat, Dec 14, 2013 at 12:19 PM, Glenn Morris <rgm@gnu.org> wrote: > > I don't know how we tidy this up for the future though. > > Add yet another optional argument "error-if-permissions-fail"? Yuck. > > It might be reasonable to error only when the existing > PRESERVE-UID-GID argument was non-nil. That argument has nothing to do with mode bits, which is what this bug is about. ^ permalink raw reply [flat|nested] 33+ messages in thread
* bug#16133: 24.3; copy-file fails on chmod when copying to FAT filesystem 2013-12-14 20:57 ` Eli Zaretskii @ 2013-12-14 21:21 ` Josh 2013-12-15 3:44 ` Eli Zaretskii 0 siblings, 1 reply; 33+ messages in thread From: Josh @ 2013-12-14 21:21 UTC (permalink / raw) To: Eli Zaretskii Cc: Валерия Лихушина, 16133, Paul Eggert On Sat, Dec 14, 2013 at 12:57 PM, Eli Zaretskii <eliz@gnu.org> wrote: >> From: Josh <josh@foxtail.org> >> Date: Sat, 14 Dec 2013 12:46:59 -0800 >> Cc: Paul Eggert <eggert@cs.ucla.edu>, Artem Chuprina <ran@lasgalen.net>, >> 16133@debbugs.gnu.org >> >> On Sat, Dec 14, 2013 at 12:19 PM, Glenn Morris <rgm@gnu.org> wrote: >> > I don't know how we tidy this up for the future though. >> > Add yet another optional argument "error-if-permissions-fail"? Yuck. >> >> It might be reasonable to error only when the existing >> PRESERVE-UID-GID argument was non-nil. > > That argument has nothing to do with mode bits, which is what this bug > is about. That argument being nil indicates that the user doesn't care about preserving UID and GID. Are there cases when the user would care about preserving mode bits but not UID and GID? ^ permalink raw reply [flat|nested] 33+ messages in thread
* bug#16133: 24.3; copy-file fails on chmod when copying to FAT filesystem 2013-12-14 21:21 ` Josh @ 2013-12-15 3:44 ` Eli Zaretskii 0 siblings, 0 replies; 33+ messages in thread From: Eli Zaretskii @ 2013-12-15 3:44 UTC (permalink / raw) To: Josh; +Cc: ran, 16133, eggert > From: Josh <josh@foxtail.org> > Date: Sat, 14 Dec 2013 13:21:58 -0800 > Cc: Glenn Morris <rgm@gnu.org>, Paul Eggert <eggert@cs.ucla.edu>, > Валерия Лихушина <ran@lasgalen.net>, > 16133@debbugs.gnu.org > > >> It might be reasonable to error only when the existing > >> PRESERVE-UID-GID argument was non-nil. > > > > That argument has nothing to do with mode bits, which is what this bug > > is about. > > That argument being nil indicates that the user doesn't care about > preserving UID and GID. Are there cases when the user would care > about preserving mode bits but not UID and GID? Definitely. MS-Windows is one such case. ^ permalink raw reply [flat|nested] 33+ messages in thread
* bug#16133: 24.3; copy-file fails on chmod when copying to FAT filesystem 2013-12-14 20:19 ` Glenn Morris 2013-12-14 20:46 ` Josh @ 2013-12-14 20:55 ` Eli Zaretskii 2013-12-14 21:07 ` Achim Gratz 2013-12-15 14:38 ` Artem Chuprina 3 siblings, 0 replies; 33+ messages in thread From: Eli Zaretskii @ 2013-12-14 20:55 UTC (permalink / raw) To: Glenn Morris; +Cc: eggert, ran, 16133 > From: Glenn Morris <rgm@gnu.org> > Date: Sat, 14 Dec 2013 15:19:33 -0500 > Cc: Paul Eggert <eggert@cs.ucla.edu>, 16133@debbugs.gnu.org > > Since copy-file has only relatively recently starting giving an error, > maybe the least bad thing would be for it to give a message instead of > an error? Yes, I think this is the way to go. That's what "cp -p" does, FWIW. ^ permalink raw reply [flat|nested] 33+ messages in thread
* bug#16133: 24.3; copy-file fails on chmod when copying to FAT filesystem 2013-12-14 20:19 ` Glenn Morris 2013-12-14 20:46 ` Josh 2013-12-14 20:55 ` Eli Zaretskii @ 2013-12-14 21:07 ` Achim Gratz 2013-12-15 14:38 ` Artem Chuprina 3 siblings, 0 replies; 33+ messages in thread From: Achim Gratz @ 2013-12-14 21:07 UTC (permalink / raw) To: 16133 Glenn Morris writes: > Someone may be relying on the fact that copy-file copies permissions, > and want it it throw an error if it fails: > > http://debbugs.gnu.org/cgi/bugreport.cgi?bug=8306 > > Others, like you, may not care. If the contract of copy-file is that it always copies permissions, then it should throw an error and not silently corrupt the permissions. That makes it useless on filesystems without permissions and where/when the user has insufficient rights to install permissions which is possible when ACL are in effect, so the function would need to have an extra argument to ignore the error or not try to change permissions at all. > Since copy-file has only relatively recently starting giving an error, > maybe the least bad thing would be for it to give a message instead of > an error? In this case an alternative would be that copying permissions is done on a best effort basis and may fail across file systems or in certain other situations without raising an error (although I still think that it should be possible to check for that). > I don't know how we tidy this up for the future though. > Add yet another optional argument "error-if-permissions-fail"? Yuck. How about copy-file-with-permissions (which fails when it can't do what the name says) and copy-file which does best-effort? Regards, Achim. -- +<[Q+ Matrix-12 WAVE#46+305 Neuron microQkb Andromeda XTk Blofeld]>+ SD adaptation for Waldorf rackAttack V1.04R1: http://Synth.Stromeko.net/Downloads.html#WaldorfSDada ^ permalink raw reply [flat|nested] 33+ messages in thread
* bug#16133: 24.3; copy-file fails on chmod when copying to FAT filesystem 2013-12-14 20:19 ` Glenn Morris ` (2 preceding siblings ...) 2013-12-14 21:07 ` Achim Gratz @ 2013-12-15 14:38 ` Artem Chuprina 2013-12-16 14:15 ` Stefan Monnier 3 siblings, 1 reply; 33+ messages in thread From: Artem Chuprina @ 2013-12-15 14:38 UTC (permalink / raw) To: Glenn Morris; +Cc: Paul Eggert, 16133 Glenn Morris -> Artem Chuprina @ Sat, 14 Dec 2013 15:19:33 -0500: GM> I think you are right that it is a problem. GM> I think the problem was in documenting copy-file to always copy GM> permissions in the first place. But I imagine it is far too late to GM> change that. Not sure what the right thing to do is here... GM> Someone may be relying on the fact that copy-file copies permissions, GM> and want it it throw an error if it fails: GM> http://debbugs.gnu.org/cgi/bugreport.cgi?bug=8306 GM> Others, like you, may not care. To clarify, many others, including me, CARE of copy-file NOT throwing error when it can copy file, but not its permissions, as it did for many years before. That is, the mentioned patch broke that for-many-years successful behavior, and me personally had to return back to emacs 23 to avoid it. Also, it seems that backup facility also depends on copy-file (probably not, I've not done a research, but I've seen message "cannot make a backup file, backing up to ~/.emacs.d/%backup" (or like this) when editing file on the same VFAT filesystem. GM> Since copy-file has only relatively recently starting giving an error, GM> maybe the least bad thing would be for it to give a message instead of GM> an error? As for me, this would be OK, because I cannot imagine any scenario when I'd prefer copied file to be left at target, but copy-file throwing an error. I want either success if file contents was copied successfully (and possibly a message informing me that chmod or chown failed), or, if I REALLY CARE OF EXACT COPY, that file itself is NOT copied at all (that is, reverted if ok-if-exist was true). Yes, I think that chown or copying ACLs also should not throw an error on failure. GM> I don't know how we tidy this up for the future though. GM> Add yet another optional argument "error-if-permissions-fail"? Yuck. I think no one will end up with that argument set true. Probably, someone will try to, but his users will soon ask him not to do so. Another possible solution would be to introduce a global variable, say, copy-file-signal-error-on-content-only :), allowing end-user (that is, not a library author who calls copy-file, like org-mobile-push in my case, but end user, like me) to disable throwing errors if file content copied ok, but meta information copying failed (not only chmod, but also chown, copying ACLs, times etc.) I expect that most users would like that variable to be true by default :) ^ permalink raw reply [flat|nested] 33+ messages in thread
* bug#16133: 24.3; copy-file fails on chmod when copying to FAT filesystem 2013-12-15 14:38 ` Artem Chuprina @ 2013-12-16 14:15 ` Stefan Monnier 0 siblings, 0 replies; 33+ messages in thread From: Stefan Monnier @ 2013-12-16 14:15 UTC (permalink / raw) To: Artem Chuprina; +Cc: Paul Eggert, 16133 GM> Since copy-file has only relatively recently starting giving an error, GM> maybe the least bad thing would be for it to give a message instead of GM> an error? Yes, that's the better choice for now. Stefan ^ permalink raw reply [flat|nested] 33+ messages in thread
* bug#16133: 24.3; copy-file fails on chmod when copying to FAT filesystem 2013-12-13 19:51 bug#16133: 24.3; copy-file fails on chmod when copying to FAT filesystem Artem Chuprina 2013-12-13 22:51 ` Glenn Morris @ 2013-12-20 23:27 ` Paul Eggert 2013-12-22 0:01 ` Paul Eggert 2 siblings, 0 replies; 33+ messages in thread From: Paul Eggert @ 2013-12-20 23:27 UTC (permalink / raw) To: Artem Chuprina; +Cc: 16133 I couldn't reproduce the problem on my Ubuntu system (Ubuntu 13.10); when I copied a file to a FAT file system there was no diagnostic. I don't understand what is meant by "FAT filesystem with file permissions 0075". I tried to create a directory with those permissions, using "mkdir dir; chmod 0075 dir", but as far as Ubuntu was concerned the chmod had no effect (it claimed to be successful, but it didn't change the directory permissions). Similarly, when Emacs copy-file runs, chmod has no effect but returns 0 so Emacs thinks the chmod worked and does not throw an error. Perhaps there's something unusual about how your file system was mounted? If so, it'd help to know more about it in order to reproduce the problem. ^ permalink raw reply [flat|nested] 33+ messages in thread
* bug#16133: 24.3; copy-file fails on chmod when copying to FAT filesystem 2013-12-13 19:51 bug#16133: 24.3; copy-file fails on chmod when copying to FAT filesystem Artem Chuprina 2013-12-13 22:51 ` Glenn Morris 2013-12-20 23:27 ` Paul Eggert @ 2013-12-22 0:01 ` Paul Eggert 2013-12-22 3:47 ` Eli Zaretskii 2 siblings, 1 reply; 33+ messages in thread From: Paul Eggert @ 2013-12-22 0:01 UTC (permalink / raw) To: 16133; +Cc: Artem Chuprina > That's what "cp -p" does, FWIW. I just checked the source code, and it looks like 'cp -p' reports an error if fchmod fails, so trunk Emacs is behaving similarly to GNU cp here. Artem, can you run Emacs under strace, and give the exact sequence of system calls just before Emacs reports an error? That may help us reproduce the problem. ^ permalink raw reply [flat|nested] 33+ messages in thread
* bug#16133: 24.3; copy-file fails on chmod when copying to FAT filesystem 2013-12-22 0:01 ` Paul Eggert @ 2013-12-22 3:47 ` Eli Zaretskii 2013-12-22 4:01 ` Paul Eggert 0 siblings, 1 reply; 33+ messages in thread From: Eli Zaretskii @ 2013-12-22 3:47 UTC (permalink / raw) To: Paul Eggert; +Cc: ran, 16133 > Date: Sat, 21 Dec 2013 16:01:07 -0800 > From: Paul Eggert <eggert@cs.ucla.edu> > CC: Artem Chuprina <ran@lasgalen.net>, Eli Zaretskii <eliz@gnu.org> > > > That's what "cp -p" does, FWIW. > > I just checked the source code, and it looks like 'cp -p' > reports an error if fchmod fails It reports an error, but keeps copying the rest of the files. > so trunk Emacs is behaving similarly to GNU cp here. No, it doesn't: Emacs throws an error, and stops the copy operation, even if there are more files to copy. The suggestion was to report an error, but not throw to top level. ^ permalink raw reply [flat|nested] 33+ messages in thread
* bug#16133: 24.3; copy-file fails on chmod when copying to FAT filesystem 2013-12-22 3:47 ` Eli Zaretskii @ 2013-12-22 4:01 ` Paul Eggert 2013-12-22 15:50 ` Artem Chuprina 2013-12-22 16:24 ` Eli Zaretskii 0 siblings, 2 replies; 33+ messages in thread From: Paul Eggert @ 2013-12-22 4:01 UTC (permalink / raw) To: Eli Zaretskii; +Cc: ran, 16133 Eli Zaretskii wrote: > Emacs throws an error, and stops the copy operation, > even if there are more files to copy. copy-file copies just one file, so there can't be any more files to copy. When GNU 'cp' is acting like copy-file and is copying just one file, it doesn't do anything more to the file after fchmod fails -- it simply exits with nonzero status, which corresponds to Emacs throwing an error. ^ permalink raw reply [flat|nested] 33+ messages in thread
* bug#16133: 24.3; copy-file fails on chmod when copying to FAT filesystem 2013-12-22 4:01 ` Paul Eggert @ 2013-12-22 15:50 ` Artem Chuprina 2013-12-22 19:03 ` Paul Eggert 2013-12-22 16:24 ` Eli Zaretskii 1 sibling, 1 reply; 33+ messages in thread From: Artem Chuprina @ 2013-12-22 15:50 UTC (permalink / raw) To: Paul Eggert; +Cc: 16133 Paul Eggert -> Eli Zaretskii @ Sat, 21 Dec 2013 20:01:24 -0800: >> Emacs throws an error, and stops the copy operation, >> even if there are more files to copy. PE> copy-file copies just one file, so there can't be any more PE> files to copy. When GNU 'cp' is acting like copy-file and PE> is copying just one file, it doesn't do anything more to the PE> file after fchmod fails -- it simply exits with nonzero status, PE> which corresponds to Emacs throwing an error. Please note that, first, GNU cp HAS mode that allows to use in scripts when copying to other filesystems. That is, it has a mode in which it returns success if it successfully copied file, but not its meta information. copy-file in emacs 24 - HAS NOT. Moreover, GNU cp works just in that mode BY DEFAULT. That is, its authors do understand that this is the mode of operation that most users want, not the mode that fails in that situation. So, if I write #!/bin/sh -e cp /from/ext2/filesystem/... /to/fat/filesystem/ # some code follows that depend on files on target I get an error from cp ONLY if it could not copy file(s) (this is quite reasonable, and this is just what most cp users want), not if it could not copy permissions (this has no sense in this case at all, FAT filesystem just has no permissions, and chmod always fails there). If for some strange reason I really insist on copying permissions also, I should explicitly state it, and in this case this would be my fault, not cp authors. With copy-file in emacs24 I just cannot write such code, and moreover, much code that used copy-file before, just stopped working. (I've mentioned org-mobile-push, and emacs' own file backup code, just to note two). Now ALL those libraries authors, including emacs authors, need to wrap EVERY call to copy-file to catch code, and thoroughly filter exceptions to check whether this was just chmod failure (which is acceptable) or real copy failure. Because in reality only copy problem is not acceptable, not chmod problem. This is the problem with copy-file. And I don't remember exactly, but it seems to me that even defadvice is not guaranteed to help, because copy-file is a primitive function, and can be called from C code - I really suspect that backup problem cannot be fixed by advice just by this reason. ^ permalink raw reply [flat|nested] 33+ messages in thread
* bug#16133: 24.3; copy-file fails on chmod when copying to FAT filesystem 2013-12-22 15:50 ` Artem Chuprina @ 2013-12-22 19:03 ` Paul Eggert 2013-12-22 20:13 ` Artem Chuprina 0 siblings, 1 reply; 33+ messages in thread From: Paul Eggert @ 2013-12-22 19:03 UTC (permalink / raw) To: Artem Chuprina; +Cc: 16133 Artem Chuprina wrote: > With copy-file in emacs24 I just cannot write such code It's certainly *possible* to write such code: just catch the error and continue. > much code that used copy-file before, just stopped working. (I've > mentioned org-mobile-push, and emacs' own file backup code This is a more serious matter, but it's a judgment call as to whether it stopped working or started working. Some users want permissions to be saved, as well as contents. Others don't. Emacs should support both usages. I'm still puzzled as to why you're observing the problem and I am not. Can you do an 'strace' on an Emacs that's exhibiting the problem? It'd be helpful to see exactly which system calls are being invoked, and the errno value for the failing one. Possibly there is a small workaround here that wouldn't require redesining copy-file. ^ permalink raw reply [flat|nested] 33+ messages in thread
* bug#16133: 24.3; copy-file fails on chmod when copying to FAT filesystem 2013-12-22 19:03 ` Paul Eggert @ 2013-12-22 20:13 ` Artem Chuprina 2013-12-23 23:58 ` Paul Eggert 0 siblings, 1 reply; 33+ messages in thread From: Artem Chuprina @ 2013-12-22 20:13 UTC (permalink / raw) To: Paul Eggert; +Cc: 16133 Paul Eggert -> Artem Chuprina @ Sun, 22 Dec 2013 11:03:24 -0800: >> With copy-file in emacs24 I just cannot write such code PE> It's certainly *possible* to write such code: just catch PE> the error and continue. Not. It's possible to write code that does the same, but it would be MUCH more code. Just to fight bad default behavior. It's like instead of #!/bin/sh -e cp .... you should EVER write #!/bin/sh -e cp ... || rc=$? case $rc in 5,7,13,17) ;; # ignore chown, chmod, chattr and like problems *) exit $rc esac Almost nobody almost never wants this. >> much code that used copy-file before, just stopped working. (I've >> mentioned org-mobile-push, and emacs' own file backup code PE> This is a more serious matter, but it's a judgment call as to PE> whether it stopped working or started working. Some users PE> want permissions to be saved, as well as contents. Others PE> don't. Emacs should support both usages. I've seen insisting on chown/chmod success when copying in ONE task only: rsync backup of the WHOLE system. I've never seen somebody who will try to use emacs' copy-file for that task. I think that if you want Emacs to support such a strange behavior, it would be OK. But NOT as default behavior. As you appeal to GNU cp, see its default behavior: BY DEFAULT it TRIES to save permissions and owner/group, but DOES NOT fail if cannot. If you personally insist that this should be failure, cp allows this to you, but you must express this EXPLICITLY. I'm sure that copy-file should behave in this style. PE> I'm still puzzled as to why you're observing the problem and PE> I am not. Can you do an 'strace' on an Emacs that's exhibiting PE> the problem? It'd be helpful to see exactly which system calls PE> are being invoked, and the errno value for the failing one. PE> Possibly there is a small workaround here that wouldn't require PE> redesining copy-file. $ touch testfile $ strace -otrace emacs24 -Q M-: (copy-file "testfile" "c1/testfile") Debugger entered--Lisp error: (file-error "Doing chmod" "operation not permitted" $ copy-file("testfile" "c1/testfile") eval((copy-file "testfile" "c1/testfile") nil) eval-expression((copy-file "testfile" "c1/testfile") nil) call-interactively(eval-expression nil nil) C-x C-c Relevant part of trace: stat64("/home/ran/c1/testfile", 0xbeb0ff38) = -1 ENOENT (No such file or directory) lstat64("/home/ran/c1/testfile", 0xbeb0ff20) = -1 ENOENT (No such file or directory) open("/home/ran/testfile", O_RDONLY|O_LARGEFILE) = 4 fstat64(4, {st_mode=S_IFREG|0644, st_size=0, ...}) = 0 open("/home/ran/c1/testfile", O_WRONLY|O_CREAT|O_EXCL|O_TRUNC|O_LARGEFILE, 0644) = 5 read(4, "", 16384) = 0 fchmod(5, 0644) = -1 EPERM (Operation not permitted) stat64("/usr/share/emacs/24.3/lisp/debug.elc", 0xbeb0fbe8) = -1 ENOENT (No such file or directory) ... (search for debugger) stat64("/usr/share/emacs/24.3/lisp/emacs-lisp/debug.elc", {st_mode=S_IFREG|0644, st_size=27568, ...}) = 0 open("/usr/share/emacs/24.3/lisp/emacs-lisp/debug.elc", O_RDONLY|O_LARGEFILE) = 6 Indeed, after your request, I've tried to reproduce the bug on my netbook, and for the first time could not. Then I thought a little more, and mounted FAT partition with options similar to those on Android in question, that is, mount -o fmask=0700,dmask=0700 /dev/sda1 /fat (on Android they are exactly rw,dirsync,nosuid,nodev,noexec,relatime,uid=1000,gid=1015,fmask=0702,dmask=0702,allow_utime=0020,codepage=cp437,iocharset=iso8859-1,shortname=mixed,utf8,errors=remount-ro) The problem reproduced. Further investigation shows that with (very reasonable on a multi-user Linux system) mount options for VFAT mount -o fmask=0111,dmask=0 /dev/sda1 /fat (that is, all files belong to root:root, files have permissions 0666, directories - 0777) we have the same problem. That is, relevant part is that the resulting file does not belong to me but I can write to it, I can create files there, but I cannot change their metainfo, because they are not mine. On Android I can write through group permissions, in this case - through other permissions, effect is the same. If filesystem belongs to me, fchmod silently succeeds (permissions don't change, of cause). The problem also not reproduced if filesystem is mounted with quiet option (it is designed just for this). It is also very reasonable on Linux systems when mounting FAT, but Android does not use this option when mounting SD cards. ^ permalink raw reply [flat|nested] 33+ messages in thread
* bug#16133: 24.3; copy-file fails on chmod when copying to FAT filesystem 2013-12-22 20:13 ` Artem Chuprina @ 2013-12-23 23:58 ` Paul Eggert 2013-12-24 6:52 ` Artem Chuprina 0 siblings, 1 reply; 33+ messages in thread From: Paul Eggert @ 2013-12-23 23:58 UTC (permalink / raw) To: Artem Chuprina; +Cc: 16133 [-- Attachment #1: Type: text/plain, Size: 1225 bytes --] Artem Chuprina wrote: > As you appeal to GNU cp, see > its default behavior: BY DEFAULT it TRIES to save permissions and > owner/group No, by default GNU cp does not try to copy either owner/group or permissions to an existing destination. It does not invoke chmod or chown unless you use something like 'cp -p'. > I can create files there, but I cannot change > their metainfo, because they are not mine. This is an unusual setup, at least for me. If I create a file, I should be able to change its metainformation. I expect this setup will cause problems with other applications, not just Emacs. GNU tar would be one example. That being said, we should be able to work around the problem by having copy-file behave more like 'cp'. That is, copy-file should not invoke chmod by default; it should invoke chmod only if it's told to preserve permissions (or preserve ownership, since that often involves temporarily revoking permissions for security reasons). That way, plain copy-file should work with your setup, although you'll still have trouble with copy-file with the last arg t (which asks to copy permissions). Attached is a proposed patch to do that, against trunk bzr 115721. Does it solve your problem? [-- Attachment #2: copy-file.diff --] [-- Type: text/x-patch, Size: 15512 bytes --] === modified file 'doc/lispref/ChangeLog' --- doc/lispref/ChangeLog 2013-12-23 11:27:29 +0000 +++ doc/lispref/ChangeLog 2013-12-23 23:53:26 +0000 @@ -1,3 +1,8 @@ +2013-12-23 Paul Eggert <eggert@cs.ucla.edu> + + Plain copy-file no longer chmods an existing destination (Bug#16133). + * files.texi (Changing Files): Document this. + 2013-12-23 Xue Fuqiao <xfq.free@gmail.com> * eval.texi (Special Forms): Document `special-form-p'. === modified file 'doc/lispref/files.texi' --- doc/lispref/files.texi 2013-12-23 08:50:31 +0000 +++ doc/lispref/files.texi 2013-12-23 23:53:26 +0000 @@ -1563,8 +1563,6 @@ interactive call, a prefix argument specifies a non-@code{nil} value for @var{time}. -This function copies the file modes, too. - If argument @var{preserve-uid-gid} is @code{nil}, we let the operating system decide the user and group ownership of the new file (this is usually set to the user running Emacs). If @var{preserve-uid-gid} is @@ -1572,10 +1570,11 @@ file. This works only on some operating systems, and only if you have the correct permissions to do so. -If the optional argument @var{preserve-extended-attributes} is -non-@code{nil}, and Emacs has been built with the appropriate support, -this function attempts to copy the file's extended attributes, such as -its SELinux context and ACL entries (@pxref{File Attributes}). +If the optional argument @var{preserve-permissions} is non-@code{nil}, +this function copies the file's permissions, such as its file modes, +its SELinux context, and ACL entries (@pxref{File Attributes}). +Otherwise, if the destination is created its file permission bits are +those of the source, masked by the default file permissions. @end deffn @deffn Command make-symbolic-link filename newname &optional ok-if-exists === modified file 'etc/ChangeLog' --- etc/ChangeLog 2013-12-23 22:27:49 +0000 +++ etc/ChangeLog 2013-12-23 23:53:26 +0000 @@ -1,3 +1,8 @@ +2013-12-23 Paul Eggert <eggert@cs.ucla.edu> + + Plain copy-file no longer chmods an existing destination (Bug#16133). + * NEWS: Document this. + 2013-12-23 Teodor Zlatanov <tzz@lifelogs.com> * NEWS: Updated for `gnutls-verify-error', cfengine-mode, and === modified file 'etc/NEWS' --- etc/NEWS 2013-12-23 13:05:27 +0000 +++ etc/NEWS 2013-12-23 23:53:26 +0000 @@ -889,6 +889,12 @@ `file-extended-attributes'. The attributes can be applied to another file using `set-file-extended-attributes'. +** By default `copy-file' no longer copies file permission bits to an +existing destination; and it sets the file permission bits of a newly +created destination to those of the source, masked by the default file +permissions. To copy the file permission bits, pass t as the +PRESERVE-PERMISSIONS argument of `copy-file'. + ** `visited-file-modtime' now returns -1 for nonexistent files. Formerly it returned a list (-1 LOW USEC PSEC), but this was ambiguous in the presence of files with negative time stamps. @@ -1030,8 +1036,8 @@ +++ *** The 6th argument to `copy-file' has been renamed to -PRESERVE-EXTENDED-ATTRIBUTES as it now handles both SELinux context -and ACL entries. +PRESERVE-PERMISSIONS as it now handles ACL entries and the traditional +Unix file permission bits as well as SELinux context. +++ *** The function `file-ownership-preserved-p' now has an optional === modified file 'src/ChangeLog' --- src/ChangeLog 2013-12-23 19:24:25 +0000 +++ src/ChangeLog 2013-12-23 23:53:26 +0000 @@ -1,3 +1,20 @@ +2013-12-23 Paul Eggert <eggert@cs.ucla.edu> + + Plain copy-file no longer chmods an existing destination (Bug#16133). + * fileio.c (realmask): Now a static var, not a local. + (barf_or_query_if_file_exists): New arg KNOWN_TO_EXIST. + Remove arg STATPTR. All uses changed. + (Fcopy_file): Do not alter permissions of existing destinations, + unless PRESERVE-PERMISSIONS (renamed from + PRESERVE-EXTENDED-ATTRIBUTES) is non-nil. + Avoid race when testing for existing destinations and for + when input and output files are the same. + If changing the group fails, adjust both default and + preserved permissions so that access is not granted to the + wrong group. + (Fset_default_file_modes, init_fileio): Update realmask. + (Fdefault_file_modes): Use realmask instead of calling umask. + 2013-12-23 Eli Zaretskii <eliz@gnu.org> * xdisp.c (tool_bar_height): Use WINDOW_PIXEL_WIDTH to set up the === modified file 'src/fileio.c' --- src/fileio.c 2013-12-17 17:46:31 +0000 +++ src/fileio.c 2013-12-23 23:53:26 +0000 @@ -95,6 +95,9 @@ /* True during writing of auto-save files. */ static bool auto_saving; +/* Emacs's real umask. */ +static mode_t realmask; + /* Nonzero umask during creation of auto-save directories. */ static mode_t auto_saving_dir_umask; @@ -1858,20 +1861,16 @@ } \f /* Signal an error if the file ABSNAME already exists. + If KNOWN_TO_EXIST, the file is known to exist. + QUERYSTRING is a name for the action that is being considered + to alter the file. If INTERACTIVE, ask the user whether to proceed, and bypass the error if the user says to go ahead. - QUERYSTRING is a name for the action that is being considered - to alter the file. - - *STATPTR is used to store the stat information if the file exists. - If the file does not exist, STATPTR->st_mode is set to 0. - If STATPTR is null, we don't store into it. - If QUICK, ask for y or n, not yes or no. */ static void -barf_or_query_if_file_exists (Lisp_Object absname, const char *querystring, - bool interactive, struct stat *statptr, +barf_or_query_if_file_exists (Lisp_Object absname, bool known_to_exist, + const char *querystring, bool interactive, bool quick) { Lisp_Object tem, encoded_filename; @@ -1880,14 +1879,16 @@ encoded_filename = ENCODE_FILE (absname); - /* `stat' is a good way to tell whether the file exists, - regardless of what access permissions it has. */ - if (lstat (SSDATA (encoded_filename), &statbuf) >= 0) + if (! known_to_exist && lstat (SSDATA (encoded_filename), &statbuf) == 0) { if (S_ISDIR (statbuf.st_mode)) xsignal2 (Qfile_error, build_string ("File is a directory"), absname); + known_to_exist = true; + } + if (known_to_exist) + { if (! interactive) xsignal2 (Qfile_already_exists, build_string ("File already exists"), absname); @@ -1902,15 +1903,7 @@ if (NILP (tem)) xsignal2 (Qfile_already_exists, build_string ("File already exists"), absname); - if (statptr) - *statptr = statbuf; - } - else - { - if (statptr) - statptr->st_mode = 0; - } - return; + } } DEFUN ("copy-file", Fcopy_file, Scopy_file, 2, 6, @@ -1937,16 +1930,21 @@ If PRESERVE-UID-GID is non-nil, we try to transfer the uid and gid of FILE to NEWNAME. -If PRESERVE-EXTENDED-ATTRIBUTES is non-nil, we try to copy additional -attributes of FILE to NEWNAME, such as its SELinux context and ACL -entries (depending on how Emacs was built). */) - (Lisp_Object file, Lisp_Object newname, Lisp_Object ok_if_already_exists, Lisp_Object keep_time, Lisp_Object preserve_uid_gid, Lisp_Object preserve_extended_attributes) +If PRESERVE-PERMISSIONS is non-nil, copy permissions of FILE to NEWNAME; +this includes the file modes, along with ACL entries and SELinux +context if present. Otherwise, if NEWNAME is created its file +permission bits are those of FILE, masked by the default file +permissions. */) + (Lisp_Object file, Lisp_Object newname, Lisp_Object ok_if_already_exists, + Lisp_Object keep_time, Lisp_Object preserve_uid_gid, + Lisp_Object preserve_permissions) { - struct stat out_st; Lisp_Object handler; struct gcpro gcpro1, gcpro2, gcpro3, gcpro4; ptrdiff_t count = SPECPDL_INDEX (); Lisp_Object encoded_file, encoded_newname; + bool already_exists = false; + mode_t new_mask; #if HAVE_LIBSELINUX security_context_t con; int conlength = 0; @@ -1981,22 +1979,20 @@ if (!NILP (handler)) RETURN_UNGCPRO (call7 (handler, Qcopy_file, file, newname, ok_if_already_exists, keep_time, preserve_uid_gid, - preserve_extended_attributes)); + preserve_permissions)); encoded_file = ENCODE_FILE (file); encoded_newname = ENCODE_FILE (newname); +#ifdef WINDOWSNT if (NILP (ok_if_already_exists) || INTEGERP (ok_if_already_exists)) - barf_or_query_if_file_exists (newname, "copy to it", - INTEGERP (ok_if_already_exists), &out_st, 0); - else if (stat (SSDATA (encoded_newname), &out_st) < 0) - out_st.st_mode = 0; + barf_or_query_if_file_exists (newname, false, "copy to it", + INTEGERP (ok_if_already_exists), false); -#ifdef WINDOWSNT result = w32_copy_file (SSDATA (encoded_file), SSDATA (encoded_newname), !NILP (keep_time), !NILP (preserve_uid_gid), - !NILP (preserve_extended_attributes)); + !NILP (preserve_permissions)); switch (result) { case -1: @@ -2022,7 +2018,7 @@ if (fstat (ifd, &st) != 0) report_file_error ("Input file status", file); - if (!NILP (preserve_extended_attributes)) + if (!NILP (preserve_permissions)) { #if HAVE_LIBSELINUX if (is_selinux_enabled ()) @@ -2034,32 +2030,46 @@ #endif } - if (out_st.st_mode != 0 - && st.st_dev == out_st.st_dev && st.st_ino == out_st.st_ino) - report_file_errno ("Input and output files are the same", - list2 (file, newname), 0); - /* We can copy only regular files. */ if (!S_ISREG (st.st_mode)) report_file_errno ("Non-regular file", file, S_ISDIR (st.st_mode) ? EISDIR : EINVAL); - { #ifndef MSDOS - int new_mask = st.st_mode & (!NILP (preserve_uid_gid) ? 0600 : 0666); + new_mask = st.st_mode & 0777; + if (!NILP (preserve_uid_gid) || !NILP (preserve_permissions)) + new_mask &= 0700; #else - int new_mask = S_IREAD | S_IWRITE; + new_mask = S_IREAD | S_IWRITE; #endif - ofd = emacs_open (SSDATA (encoded_newname), - (O_WRONLY | O_TRUNC | O_CREAT - | (NILP (ok_if_already_exists) ? O_EXCL : 0)), - new_mask); - } + + ofd = emacs_open (SSDATA (encoded_newname), O_WRONLY | O_CREAT | O_EXCL, + new_mask); + if (ofd < 0 && errno == EEXIST) + { + if (NILP (ok_if_already_exists) || INTEGERP (ok_if_already_exists)) + barf_or_query_if_file_exists (newname, true, "copy to it", + INTEGERP (ok_if_already_exists), false); + already_exists = true; + ofd = emacs_open (SSDATA (encoded_newname), O_WRONLY, 0); + } if (ofd < 0) report_file_error ("Opening output file", newname); record_unwind_protect_int (close_file_unwind, ofd); + if (already_exists) + { + struct stat out_st; + if (fstat (ofd, &out_st) != 0) + report_file_error ("Output file status", newname); + if (st.st_dev == out_st.st_dev && st.st_ino == out_st.st_ino) + report_file_errno ("Input and output files are the same", + list2 (file, newname), 0); + if (ftruncate (ofd, 0) != 0) + report_file_error ("Truncating output file", newname); + } + immediate_quit = 1; QUIT; while ((n = emacs_read (ifd, buf, sizeof buf)) > 0) @@ -2071,26 +2081,41 @@ /* Preserve the original file permissions, and if requested, also its owner and group. */ { - mode_t mode_mask = 07777; + mode_t preserved_permissions = st.st_mode & 07777; + mode_t default_permissions = st.st_mode & 0777 & ~realmask; if (!NILP (preserve_uid_gid)) { /* Attempt to change owner and group. If that doesn't work attempt to change just the group, as that is sometimes allowed. Adjust the mode mask to eliminate setuid or setgid bits - that are inappropriate if the owner and group are wrong. */ + or group permissions bits that are inappropriate if the + owner or group are wrong. */ if (fchown (ofd, st.st_uid, st.st_gid) != 0) { - mode_mask &= ~06000; if (fchown (ofd, -1, st.st_gid) == 0) - mode_mask |= 02000; + preserved_permissions &= ~04000; + else + { + preserved_permissions &= ~06000; + + /* Copy the other bits to the group bits, since the + group is wrong. */ + preserved_permissions &= ~070; + preserved_permissions |= (preserved_permissions & 7) << 3; + default_permissions &= ~070; + default_permissions |= (default_permissions & 7) << 3; + } } } - switch (!NILP (preserve_extended_attributes) + switch (!NILP (preserve_permissions) ? qcopy_acl (SSDATA (encoded_file), ifd, SSDATA (encoded_newname), ofd, - st.st_mode & mode_mask) - : fchmod (ofd, st.st_mode & mode_mask)) + preserved_permissions) + : (already_exists + || (new_mask & ~realmask) == default_permissions) + ? 0 + : fchmod (ofd, default_permissions)) { case -2: report_file_error ("Copying permissions from", file); case -1: report_file_error ("Copying permissions to", newname); @@ -2307,8 +2332,8 @@ #endif if (NILP (ok_if_already_exists) || INTEGERP (ok_if_already_exists)) - barf_or_query_if_file_exists (newname, "rename to it", - INTEGERP (ok_if_already_exists), 0, 0); + barf_or_query_if_file_exists (newname, false, "rename to it", + INTEGERP (ok_if_already_exists), false); if (rename (SSDATA (encoded_file), SSDATA (encoded_newname)) < 0) { int rename_errno = errno; @@ -2387,8 +2412,8 @@ if (NILP (ok_if_already_exists) || INTEGERP (ok_if_already_exists)) - barf_or_query_if_file_exists (newname, "make it a new name", - INTEGERP (ok_if_already_exists), 0, 0); + barf_or_query_if_file_exists (newname, false, "make it a new name", + INTEGERP (ok_if_already_exists), false); unlink (SSDATA (newname)); if (link (SSDATA (encoded_file), SSDATA (encoded_newname)) < 0) @@ -2449,8 +2474,8 @@ if (NILP (ok_if_already_exists) || INTEGERP (ok_if_already_exists)) - barf_or_query_if_file_exists (linkname, "make it a link", - INTEGERP (ok_if_already_exists), 0, 0); + barf_or_query_if_file_exists (linkname, false, "make it a link", + INTEGERP (ok_if_already_exists), false); if (symlink (SSDATA (encoded_filename), SSDATA (encoded_linkname)) < 0) { /* If we didn't complain already, silently delete existing file. */ @@ -3137,10 +3162,17 @@ This setting is inherited by subprocesses. */) (Lisp_Object mode) { + mode_t oldrealmask, oldumask, newumask; CHECK_NUMBER (mode); - - umask ((~ XINT (mode)) & 0777); - + oldrealmask = realmask; + newumask = ~ XINT (mode) & 0777; + + block_input (); + realmask = newumask; + oldumask = umask (newumask); + unblock_input (); + + eassert (oldumask == oldrealmask); return Qnil; } @@ -3149,14 +3181,7 @@ The value is an integer. */) (void) { - mode_t realmask; Lisp_Object value; - - block_input (); - realmask = umask (0); - umask (realmask); - unblock_input (); - XSETINT (value, (~ realmask) & 0777); return value; } @@ -4697,7 +4722,7 @@ filename = Fexpand_file_name (filename, Qnil); if (!NILP (mustbenew) && !EQ (mustbenew, Qexcl)) - barf_or_query_if_file_exists (filename, "overwrite", 1, 0, 1); + barf_or_query_if_file_exists (filename, false, "overwrite", true, true); if (STRINGP (visit)) visit_file = Fexpand_file_name (visit, Qnil); @@ -5765,6 +5790,9 @@ void init_fileio (void) { + realmask = umask (0); + umask (realmask); + valid_timestamp_file_system = 0; /* fsync can be a significant performance hit. Often it doesn't ^ permalink raw reply [flat|nested] 33+ messages in thread
* bug#16133: 24.3; copy-file fails on chmod when copying to FAT filesystem 2013-12-23 23:58 ` Paul Eggert @ 2013-12-24 6:52 ` Artem Chuprina 2013-12-24 9:58 ` Andreas Schwab ` (2 more replies) 0 siblings, 3 replies; 33+ messages in thread From: Artem Chuprina @ 2013-12-24 6:52 UTC (permalink / raw) To: Paul Eggert; +Cc: 16133 Paul Eggert -> Artem Chuprina @ Mon, 23 Dec 2013 15:58:29 -0800: >> As you appeal to GNU cp, see >> its default behavior: BY DEFAULT it TRIES to save permissions and >> owner/group PE> No, by default GNU cp does not try to copy either owner/group PE> or permissions to an existing destination. It does not invoke PE> chmod or chown unless you use something like 'cp -p'. You are wrong. zsh% umask 002 zsh% touch testfile zsh% ls -l testfile -rw-rw-r-- 1 ran ran 0 Дек 24 10:37 testfile zsh% chmod 600 testfile zsh% cp testfile testfile.copy zsh% ls -l testfile.copy -rw------- 1 ran ran 0 Дек 24 10:38 testfile.copy Debian GNU/Linux 7.3 (wheezy) >> I can create files there, but I cannot change >> their metainfo, because they are not mine. PE> This is an unusual setup, at least for me. PE> If I create a file, I should be able to change its metainformation. Key phrase: "at least for me". This is usual setup with FAT filesystems on every multiuser system. Because FAT cannot keep owner and there are multiple users, it is mounted so that all the files belong to root or another system user, not to some real user. PE> I expect this setup will cause problems with other applications, PE> not just Emacs. GNU tar would be one example. You are wrong again. zsh% tar tvf ~/testfiles.tar -rw------- ran/ran 0 2013-12-24 10:37 testfile -rw------- ran/ran 0 2013-12-24 10:38 testfile.copy zsh% cd /fat zsh% tar xvf ~/testfiles.tar testfile testfile.copy zsh% echo $? 0 zsh% ls -l testfile* -rw-rw-rw- 1 root root 0 Дек 24 10:37 testfile -rw-rw-rw- 1 root root 0 Дек 24 10:38 testfile.copy PE> That being said, we should be able to work around the problem PE> by having copy-file behave more like 'cp'. That is, copy-file PE> should not invoke chmod by default; it should invoke chmod only PE> if it's told to preserve permissions (or preserve ownership, PE> since that often involves temporarily revoking permissions for PE> security reasons). That way, plain copy-file should work with PE> your setup, although you'll still have trouble with PE> copy-file with the last arg t (which asks to copy permissions). This is also reasonable behavior, but not the best one, and it is inconsistent with such of GNU cp and GNU tar, as proved above. PE> Attached is a proposed patch to do that, against trunk bzr 115721. PE> Does it solve your problem? I'll try to check it today. ^ permalink raw reply [flat|nested] 33+ messages in thread
* bug#16133: 24.3; copy-file fails on chmod when copying to FAT filesystem 2013-12-24 6:52 ` Artem Chuprina @ 2013-12-24 9:58 ` Andreas Schwab 2013-12-24 10:22 ` Artem Chuprina 2013-12-24 16:51 ` Artem Chuprina 2013-12-29 18:31 ` Paul Eggert 2 siblings, 1 reply; 33+ messages in thread From: Andreas Schwab @ 2013-12-24 9:58 UTC (permalink / raw) To: Artem Chuprina; +Cc: Paul Eggert, 16133 Artem Chuprina <ran@lasgalen.net> writes: > zsh% cp testfile testfile.copy That's not an existing destination. Andreas. -- Andreas Schwab, schwab@linux-m68k.org GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5 "And now for something completely different." ^ permalink raw reply [flat|nested] 33+ messages in thread
* bug#16133: 24.3; copy-file fails on chmod when copying to FAT filesystem 2013-12-24 9:58 ` Andreas Schwab @ 2013-12-24 10:22 ` Artem Chuprina 2013-12-24 17:39 ` Paul Eggert 0 siblings, 1 reply; 33+ messages in thread From: Artem Chuprina @ 2013-12-24 10:22 UTC (permalink / raw) To: Andreas Schwab; +Cc: Paul Eggert, 16133 Andreas Schwab -> Artem Chuprina @ Tue, 24 Dec 2013 10:58:46 +0100: >> zsh% cp testfile testfile.copy AS> That's not an existing destination. Well, your correction is right. cp's behavior is more complex than "just try to save permissions". When the target file exists, cp does not try unless explicitly asked to. But when creating file, it does (that is, file's permissions are copied from source, not just left as set by umask). ^ permalink raw reply [flat|nested] 33+ messages in thread
* bug#16133: 24.3; copy-file fails on chmod when copying to FAT filesystem 2013-12-24 10:22 ` Artem Chuprina @ 2013-12-24 17:39 ` Paul Eggert 0 siblings, 0 replies; 33+ messages in thread From: Paul Eggert @ 2013-12-24 17:39 UTC (permalink / raw) To: Artem Chuprina; +Cc: 16133 Artem Chuprina wrote: > When the target file exists, cp does > not try unless explicitly asked to. But when creating file, it does > (that is, file's permissions are copied from source, not just left as > set by umask). Even when creating a file, GNU cp by default does not use 'chmod' to copy the file's permissions (12 bits). Instead, it merely creates the file with the permissions of the source as modified by the umask (9 bits). That is why it doesn't run into a problem in your setup. The proposed patch would cause GNU Emacs to act similarly, which is why I expect it to work for you. ^ permalink raw reply [flat|nested] 33+ messages in thread
* bug#16133: 24.3; copy-file fails on chmod when copying to FAT filesystem 2013-12-24 6:52 ` Artem Chuprina 2013-12-24 9:58 ` Andreas Schwab @ 2013-12-24 16:51 ` Artem Chuprina 2013-12-29 18:31 ` Paul Eggert 2 siblings, 0 replies; 33+ messages in thread From: Artem Chuprina @ 2013-12-24 16:51 UTC (permalink / raw) To: Paul Eggert; +Cc: 16133 Artem Chuprina <ran@lasgalen.net> writes: > PE> Attached is a proposed patch to do that, against trunk bzr 115721. > PE> Does it solve your problem? > > I'll try to check it today. Sorry, I could not. Now I'm on my vacation, far from home, and I cannot check it for 3 weeks. ^ permalink raw reply [flat|nested] 33+ messages in thread
* bug#16133: 24.3; copy-file fails on chmod when copying to FAT filesystem 2013-12-24 6:52 ` Artem Chuprina 2013-12-24 9:58 ` Andreas Schwab 2013-12-24 16:51 ` Artem Chuprina @ 2013-12-29 18:31 ` Paul Eggert 2 siblings, 0 replies; 33+ messages in thread From: Paul Eggert @ 2013-12-29 18:31 UTC (permalink / raw) To: Artem Chuprina; +Cc: 16133-done Artem Chuprina wrote: > PE> I expect this setup will cause problems with other applications, > PE> not just Emacs. GNU tar would be one example. > > You are wrong again. Well, as Andreas mentioned, I was right about cp, so the "again" in your remark is incorrect; but you are correct about 'tar'; it uses a trick in which it sets the umask to 0 to avoid the need for fchmod. Still, many programs don't use that trick, and you'll run into problems with these programs. GNU cpio would be one example; if you do something like "find . -print | cpio -pdmuv .../dest", where "dest" is in that file system, cpio will report an error because it can't change the permission of the file that it creates. In reviewing the email in this bug report it appears that we do have a real problem here, and that a "real" solution will probably require redesigning the API significantly so that there's a copy-file variant that doesn't throw errors, or something like that. Right now we're just trying to fix bugs, though, so I installed the more-conservative change, where copy-file doesn't try to fchmod an existing file -- this causes copy-file to act more like plain 'cp' and I expect it's what more copy-file users would expect anyway. ^ permalink raw reply [flat|nested] 33+ messages in thread
* bug#16133: 24.3; copy-file fails on chmod when copying to FAT filesystem 2013-12-22 4:01 ` Paul Eggert 2013-12-22 15:50 ` Artem Chuprina @ 2013-12-22 16:24 ` Eli Zaretskii 2013-12-22 17:37 ` Paul Eggert 1 sibling, 1 reply; 33+ messages in thread From: Eli Zaretskii @ 2013-12-22 16:24 UTC (permalink / raw) To: Paul Eggert; +Cc: ran, 16133 > Date: Sat, 21 Dec 2013 20:01:24 -0800 > From: Paul Eggert <eggert@cs.ucla.edu> > CC: 16133@debbugs.gnu.org, ran@lasgalen.net > > Eli Zaretskii wrote: > > Emacs throws an error, and stops the copy operation, > > even if there are more files to copy. > > copy-file copies just one file, so there can't be any more > files to copy. When GNU 'cp' is acting like copy-file and > is copying just one file, it doesn't do anything more to the > file after fchmod fails -- it simply exits with nonzero status, > which corresponds to Emacs throwing an error. I was referring to the use case described here: http://debbugs.gnu.org/cgi/bugreport.cgi?bug=16133#14 The 'cp' equivalent of that is when it is invoked to copy more than one file, or copy a directory. In that case, it does not exit on the first fchown error (at least the version I have here doesn't), but instead reports the error and continues. Signaling an error from copy-file makes it inconvenient to write functions that copy many files or entire directories. ^ permalink raw reply [flat|nested] 33+ messages in thread
* bug#16133: 24.3; copy-file fails on chmod when copying to FAT filesystem 2013-12-22 16:24 ` Eli Zaretskii @ 2013-12-22 17:37 ` Paul Eggert 2013-12-22 18:35 ` Eli Zaretskii 0 siblings, 1 reply; 33+ messages in thread From: Paul Eggert @ 2013-12-22 17:37 UTC (permalink / raw) To: Eli Zaretskii; +Cc: ran, 16133 Eli Zaretskii wrote: > I was referring to the use case described here: > > http://debbugs.gnu.org/cgi/bugreport.cgi?bug=16133#14 > > The 'cp' equivalent of that is when it is invoked to copy more than > one file, or copy a directory. The use case described there does not involve merely copy-file; it also involves some other function (org-mobile-push, I guess) that uses copy-file to copy files one at a time, and which gives up if one instance of copy-file fails. This is akin to a shell script that uses cp to copy files one a time, and which aborts if cp fails. Such a script would behave the same way that org-mobile-push behaves now, if I'm understanding the bug report correctly. One fix would be to modify org-mobile-push to not throw an error when copy-file fails, but instead to continue. Perhaps this is the desired behavior, not only for fchmod failure, but for other copy-file failures such as write error. In any case org-mobile-push could ignore some or all errors, or report them at the end, or whatever it likes. ^ permalink raw reply [flat|nested] 33+ messages in thread
* bug#16133: 24.3; copy-file fails on chmod when copying to FAT filesystem 2013-12-22 17:37 ` Paul Eggert @ 2013-12-22 18:35 ` Eli Zaretskii 2013-12-22 18:54 ` Paul Eggert 0 siblings, 1 reply; 33+ messages in thread From: Eli Zaretskii @ 2013-12-22 18:35 UTC (permalink / raw) To: Paul Eggert; +Cc: ran, 16133 > Date: Sun, 22 Dec 2013 09:37:26 -0800 > From: Paul Eggert <eggert@cs.ucla.edu> > CC: 16133@debbugs.gnu.org, ran@lasgalen.net > > Eli Zaretskii wrote: > > I was referring to the use case described here: > > > > http://debbugs.gnu.org/cgi/bugreport.cgi?bug=16133#14 > > > > The 'cp' equivalent of that is when it is invoked to copy more than > > one file, or copy a directory. > > The use case described there does not involve merely copy-file; > it also involves some other function (org-mobile-push, I guess) > that uses copy-file to copy files one at a time, > and which gives up if one instance of copy-file fails. > > This is akin to a shell script that uses cp to > copy files one a time, and which aborts if cp fails. > Such a script would behave the same way that org-mobile-push > behaves now, if I'm understanding the bug report correctly. > > One fix would be to modify org-mobile-push to not throw an error > when copy-file fails, but instead to continue. Perhaps this is the > desired behavior, not only for fchmod failure, but for other copy-file > failures such as write error. In any case org-mobile-push could > ignore some or all errors, or report them at the end, or whatever > it likes. Sorry, I disagree: it is not the business of copy-file to decide when to interrupt its caller. It is only justified to do that when the error is fatal; this one isn't: the file was copied. copy-file is akin to a library function. A library function should never decide on its own when to call it quits, because a library function never has enough context to do TRT. ^ permalink raw reply [flat|nested] 33+ messages in thread
* bug#16133: 24.3; copy-file fails on chmod when copying to FAT filesystem 2013-12-22 18:35 ` Eli Zaretskii @ 2013-12-22 18:54 ` Paul Eggert 2013-12-22 20:32 ` Artem Chuprina 2013-12-22 21:00 ` Eli Zaretskii 0 siblings, 2 replies; 33+ messages in thread From: Paul Eggert @ 2013-12-22 18:54 UTC (permalink / raw) To: Eli Zaretskii; +Cc: ran, 16133 Eli Zaretskii wrote: > A library function should > never decide on its own when to call it quits Copy-file signals errors in many circumstances; this is just one of them. If the goal is to have a library function that does not "decide on its own when to call it quits", then the function should return a value indicating the error, rather than throwing an exception. That might be too much of a change to copy-file, but we could have a variant of copy-file that does that, suitable for library use. > It is only justified to do that when the > error is fatal; this one isn't: the file was copied. The contents of the file were copied, but the permissions were not. The specification of copy-file says that it copies both contents and permissions. I would favor changing the specification of copy-file, so that it doesn't copy the permissions unless its 6th argument is non-nil. We could change the name of the 6th argument to PRESERVE-PERMISSIONS. The 6th argument already governs how permissions are preserved, so this would be a reasonable change. I don't think it'd hurt existing applications, since the default permissions when creating a file would be the same as it is now -- the only change would be that copy-file wouldn't attempt to change the permissions of an already-existing file unless PRESERVE-PERMISSIONS is non-nil. ^ permalink raw reply [flat|nested] 33+ messages in thread
* bug#16133: 24.3; copy-file fails on chmod when copying to FAT filesystem 2013-12-22 18:54 ` Paul Eggert @ 2013-12-22 20:32 ` Artem Chuprina 2013-12-22 21:00 ` Eli Zaretskii 1 sibling, 0 replies; 33+ messages in thread From: Artem Chuprina @ 2013-12-22 20:32 UTC (permalink / raw) To: Paul Eggert; +Cc: 16133 Paul Eggert -> Eli Zaretskii @ Sun, 22 Dec 2013 10:54:48 -0800: PE> I would favor changing the specification of copy-file, PE> so that it doesn't copy the permissions PE> unless its 6th argument is non-nil. We could change PE> the name of the 6th argument to PRESERVE-PERMISSIONS. PE> The 6th argument already governs how permissions are PE> preserved, so this would be a reasonable change. I don't PE> think it'd hurt existing applications, since the default PE> permissions when creating a file would be the same as PE> it is now -- the only change would be that copy-file PE> wouldn't attempt to change the permissions of an already-existing PE> file unless PRESERVE-PERMISSIONS is non-nil. I'd prefer copy-file behavior just like cp's. That is, ever try to preserve permissions, owner, ACLs etc, but by default throw an error only if file CONTENT is not copied successfully. Metainformation copy failures should throw error on explicit request. Probably, it would be reasonable to have not several parameters, but one parameters THROW-ERROR-ON(-META) with a list of flags, like '(acl owner times), t for all, nil (default) for none. Also like cp, it should NOT try to copy times by default, only on explicit request. But also, request to copy times in NOT yet a reason to throw error. For backward compatibility, it has sense to keep PRESERVE-UID-GID, but also for backward compatibility, with BACKWARD-COMPATIBLE behavior (that is, NOT throw an exception, AGGRHH!). If we don't bother backward compatibility, it would be better to remove PRESERVE-UID-GID entirely, replacing it with THROW-ERROR-ON. ^ permalink raw reply [flat|nested] 33+ messages in thread
* bug#16133: 24.3; copy-file fails on chmod when copying to FAT filesystem 2013-12-22 18:54 ` Paul Eggert 2013-12-22 20:32 ` Artem Chuprina @ 2013-12-22 21:00 ` Eli Zaretskii 1 sibling, 0 replies; 33+ messages in thread From: Eli Zaretskii @ 2013-12-22 21:00 UTC (permalink / raw) To: Paul Eggert; +Cc: ran, 16133 > Date: Sun, 22 Dec 2013 10:54:48 -0800 > From: Paul Eggert <eggert@cs.ucla.edu> > CC: 16133@debbugs.gnu.org, ran@lasgalen.net > > Eli Zaretskii wrote: > > A library function should > > never decide on its own when to call it quits > > Copy-file signals errors in many circumstances; > this is just one of them. If the goal is to have > a library function that does not "decide on its > own when to call it quits", then the function should > return a value indicating the error, rather than > throwing an exception. That might be too much of > a change to copy-file, but we could have a variant > of copy-file that does that, suitable for library > use. It could be a workable alternative indeed, but I don't see why copy-file couldn't do it itself: it currently returns no useful value, so no code relies on its return value. > I would favor changing the specification of copy-file, > so that it doesn't copy the permissions > unless its 6th argument is non-nil. The case in point is when you want to copy permission, if possible, but will settle for a copy without permission if not. IOW, no need to give up without trying. Perhaps we should have a special value of the 6th argument which means treat a failure to copy permissions as fatal. ^ permalink raw reply [flat|nested] 33+ messages in thread
end of thread, other threads:[~2013-12-29 18:31 UTC | newest] Thread overview: 33+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2013-12-13 19:51 bug#16133: 24.3; copy-file fails on chmod when copying to FAT filesystem Artem Chuprina 2013-12-13 22:51 ` Glenn Morris 2013-12-13 22:55 ` Glenn Morris 2013-12-14 10:10 ` Artem Chuprina 2013-12-14 20:19 ` Glenn Morris 2013-12-14 20:46 ` Josh 2013-12-14 20:57 ` Eli Zaretskii 2013-12-14 21:21 ` Josh 2013-12-15 3:44 ` Eli Zaretskii 2013-12-14 20:55 ` Eli Zaretskii 2013-12-14 21:07 ` Achim Gratz 2013-12-15 14:38 ` Artem Chuprina 2013-12-16 14:15 ` Stefan Monnier 2013-12-20 23:27 ` Paul Eggert 2013-12-22 0:01 ` Paul Eggert 2013-12-22 3:47 ` Eli Zaretskii 2013-12-22 4:01 ` Paul Eggert 2013-12-22 15:50 ` Artem Chuprina 2013-12-22 19:03 ` Paul Eggert 2013-12-22 20:13 ` Artem Chuprina 2013-12-23 23:58 ` Paul Eggert 2013-12-24 6:52 ` Artem Chuprina 2013-12-24 9:58 ` Andreas Schwab 2013-12-24 10:22 ` Artem Chuprina 2013-12-24 17:39 ` Paul Eggert 2013-12-24 16:51 ` Artem Chuprina 2013-12-29 18:31 ` Paul Eggert 2013-12-22 16:24 ` Eli Zaretskii 2013-12-22 17:37 ` Paul Eggert 2013-12-22 18:35 ` Eli Zaretskii 2013-12-22 18:54 ` Paul Eggert 2013-12-22 20:32 ` Artem Chuprina 2013-12-22 21:00 ` Eli Zaretskii
Code repositories for project(s) associated with this public inbox https://git.savannah.gnu.org/cgit/emacs.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).