[ python-Bugs-1050828 ] reindent.py strips execute permission on Unix

[SourceForge.net]

Bugs item #1050828, was opened at 2004-10-20 11:48
Message generated for change (Comment added) made by dripton
You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=105470&aid=1050828&group_id=5470

Category: Demos and Tools
Group: Python 2.4
Status: Open
Resolution: None
Priority: 5
Submitted By: David Ripton (dripton)
Assigned to: Facundo Batista (facundobatista)
Summary: reindent.py strips execute permission on Unix

Initial Comment:
Tools/scripts/reindent.py strips execute permission
from its target file.

If the tool detects that changes need to be made, then
the target file is renamed to its original name +
'.bak', then a new file is created with the original
file's name and its whitespace-modified contents.  No
manipulation of the new file's permissions is done, so
it ends up with the default for the user's umask.

This makes reindent.py annoying to run in an automated
fashion, if you rely on execute permission.

The fix is trivial, except for portability.  Here's the
Posix-only version:

+            if os.name == "posix":
+                mode = os.stat(bak).st_mode
+                os.chmod(file, mode)

Patch available upon request.

----------------------------------------------------------------------

>Comment By: David Ripton (dripton)
Date: 2004-10-27 10:47

Message:
Logged In: YES 
user_id=9425

s/wrong/right/ in my last comment.  The target is more
important than the backup, but we should go for the win/win
and give both of them the correct permissions, since it's easy.

----------------------------------------------------------------------

Comment By: David Ripton (dripton)
Date: 2004-10-27 10:45

Message:
Logged In: YES 
user_id=9425

The patch is an improvement (better to have the wrong
permissions on the target file than on the backup file), but
I think it would be even better if the .bak file had the
same permissions as the target file.

How about using shutil.copy2 (which preserves permissions,
among other things) instead of shutil.copy?

Thanks.


----------------------------------------------------------------------

Comment By: Johannes Gijsbers (jlgijsbers)
Date: 2004-10-27 08:38

Message:
Logged In: YES 
user_id=469548

The patch works for me (Linux 2.6.8-1-686 i686 GNU/Linux)
and a coworker (Mac OS X 10.3).

----------------------------------------------------------------------

Comment By: Facundo Batista (facundobatista)
Date: 2004-10-26 20:44

Message:
Logged In: YES 
user_id=752496

This fixes the problem: the original file metadata
(permisions, etc) stays unchanged.

The issue with the metadata of the bak file is none: the new
file has the user defaults permissions, group and owner. In
the previous version, the original file left with those
defaults permissions, so security is not getting worse.

The only left issue is, if the user wants to recover the
data from the bak file, he would need to change its metadata
(this is minor, if something goes wrong and the bak is
needed, it's likely that the user will need a manual approach).

My primary concern is that I don't want to commit the
changes now that we're beta without testing the changes in
other platforms (however, I'll ask this on python-dev).

----------------------------------------------------------------------

Comment By: Tim Peters (tim_one)
Date: 2004-10-25 23:19

Message:
Logged In: YES 
user_id=31435

Sorry, I don't normally run on Unix, so I have no way to tell 
whether this fixes whatever the complaint is.  Offhand it 
doesn't seem right to create a .bak file with default 
permissions either.  For example, if only the user had 
permission to get at the original, letting the .bak file have 
broader permissions seems wrong too.

----------------------------------------------------------------------

Comment By: Facundo Batista (facundobatista)
Date: 2004-10-25 22:57

Message:
Logged In: YES 
user_id=752496

Attaching the file.

----------------------------------------------------------------------

Comment By: Facundo Batista (facundobatista)
Date: 2004-10-24 17:20

Message:
Logged In: YES 
user_id=752496

Instead of renaming the file and creating a new one, just
copied the original and that's all (the target file is open
with write mode, the content replaced, but the permissions
stay).

A patch is attached.

Tim, I'm assigning this to you to doublecheck (not found
unit tests for this script).

.    Facundo

----------------------------------------------------------------------

You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=105470&aid=1050828&group_id=5470