Repository: spyder-ide/spyder
Pull request: PR: Preserve file permissions when saving in the editor
<!--- Make sure to read the Contributing Guidelines:                   --->
<!--- https://github.com/spyder-ide/spyder/blob/master/CONTRIBUTING.md --->
<!--- and follow PEP 8, PEP 257 and Spyder's code style:               --->
<!--- https://github.com/spyder-ide/spyder/wiki/Dev:-Coding-Style      --->

## Description of Changes


* [ ] Wrote at least one-line docstrings (for any new functions)
* [x] Added unit test(s) covering the changes (if testable)
<!--- Remember that an image/animation is worth a thousand words! --->
* [ ] Included a screenshot or animation (if affecting the UI, see [Licecap](https://www.cockos.com/licecap/))


<!--- Explain what you've done and why --->

Preserve file permission when saving with `atomic_writes`


### Issue(s) Resolved

<!--- List the issue(s) below, in the form "Fixes #1234"; one per line --->

Fixes #9381 


### Affirmation

By submitting this Pull Request or typing my (user)name below,
I affirm the [Developer Certificate of Origin](https://developercertificate.org)
with respect to all commits and content included in this PR,
and understand I am releasing the same under Spyder's MIT (Expat) license.

<!--- TYPE YOUR USER/NAME AFTER THE FOLLOWING: --->
I certify the above statement is true and correct: dalthviz

<!--- Thanks for your help making Spyder better for everyone! --->


Problem statement:
Overwritten file permissions on save
## Description
If one saves a file it a) cannot follow my symlink and b) removes my file permissions such that it's not executable anymore. In other words I think on each save the original file is deleted and replaced by a new one

### What steps will reproduce the problem?
Click save



## Versions

* Spyder version: 4.0.0b2 
* Python version: 3.6.7
* Qt version: 5.12.3
* PyQt5 version: 5.12.2
* Operating System: Linux 4.15.0-50-generic

### Dependencies

```
pygments >=2.0     :  2.4.0 (OK)
qdarkstyle >=2.6.4 :  2.6.8 (OK)
sphinx >=0.6.6     :  2.0.1 (OK)
psutil >=0.3       :  5.6.2 (OK)
pyls >=0.19.0;<0.25:  0.24.0 (OK)
nbconvert >=4.0    :  5.5.0 (OK)
pandas >=0.13.1    :  0.23.4 (OK)
numpy >=1.7        :  1.16.3 (OK)
sympy >=0.7.3      :  1.1.1 (OK)
cython >=0.21      :  0.26.1 (OK)
qtconsole >=4.2.0  :  4.4.4 (OK)
IPython >=4.0      :  7.5.0 (OK)
matplotlib >=2.0.0 :  2.2.2 (OK)
pylint >=0.25      :  2.3.1 (OK)
```

Hints from issue discussion:
> In other words I think on each save the original file is deleted and replaced by a new one

That's correct. This is the behavior since beta2 to ensure that we write atomically on disk. The only thing we can fix is preserving file permissions but not working with symlinks.

@dalthviz, please take a look at this one.
@dalthviz, we also need to avoid opening symlinks in the Editor.
Thanks for your reply.
Ok, I can understand your decision to introduce atomic writing which avoids corrupted or incomplete files at any time(?). My rather exotic case is it store the files in one place and execute the files via symlinks somewhere else. But since I use a separate terminal I'm not really affected by this new behaviour. Just a change of habit.
Anyway, can't you follow the symlink and save it there (with the same file permissions as before)?
>Anyway, can't you follow the symlink and save it there (with the same file permissions as before)?

You mean that opening a symlink should open the file it points to always?
Yes, that's what I meant. But I haven't thought it through
> You mean that opening a symlink should open the file it points to always?

This makes a lot of sense to me.
But that makes atomic write saves harder for us to handle.
As I understand it, the suggestion is that if A is a symlink pointing to B and the user opens A, then Spyder should open B instead. At that point, we can forget about A, so I don't think it makes saves harder.
That is how I understand it as well
At least the part with using the same file permissions doesn't seem to be too complicated. How about a bounty of 10€ until end of June? (So it's more a promise to donate on open collective)

Commit message: Editor: Add support for Python 2 error handling

Patch to review:
diff --git a/spyder/utils/encoding.py b/spyder/utils/encoding.py
index e75660f0e75..55df5cf9bc0 100644
--- a/spyder/utils/encoding.py
+++ b/spyder/utils/encoding.py
@@ -235,10 +235,25 @@ def write(text, filename, encoding='utf-8', mode='wb'):
         with open(filename, mode) as textfile:
             textfile.write(text)
     else:
+        # Based in the solution at untitaker/python-atomicwrites#42
+        # Needed to fix file permissions overwritting
+        # See spyder-ide/spyder#9381
+        try:
+            FileNotFoundError
+        except NameError:  # Python 2
+            FileNotFoundError = OSError
+        try:
+            original_mode = os.stat(filename).st_mode
+        except FileNotFoundError:
+            # Creating a new file, emulate what os.open() does
+            umask = os.umask(0)
+            os.umask(umask)
+            original_mode = 0o777 & ~umask
         with atomic_write(filename,
                           overwrite=True,
                           mode=mode) as textfile:
             textfile.write(text)
+        os.chmod(filename, original_mode)
     return encoding
 
 
diff --git a/spyder/utils/tests/test_encoding.py b/spyder/utils/tests/test_encoding.py
index e802e0fe6ba..ccb34249fb5 100644
--- a/spyder/utils/tests/test_encoding.py
+++ b/spyder/utils/tests/test_encoding.py
@@ -7,13 +7,33 @@
 
 import pytest
 import os
+import stat
 
-from spyder.utils.encoding import is_text_file, get_coding
+from spyder.utils.encoding import is_text_file, get_coding, write
 
 __location__ = os.path.realpath(os.path.join(os.getcwd(),
                                              os.path.dirname(__file__)))
 
 
+def test_permissions(tmpdir):
+    """Check that file permissions are preserved."""
+    p_file = tmpdir.mkdir("permissions").join("permissions_text.txt")
+
+    # Write file and define execution permissions
+    write("Some text", str(p_file))
+    st = os.stat(p_file)
+    mode = st.st_mode | stat.S_IEXEC
+    os.chmod(p_file, mode)
+
+    old_mode = os.stat(p_file).st_mode
+
+    # Write the file and check permissions
+    write("Some text and more", str(p_file))
+    new_mode = os.stat(p_file).st_mode
+
+    assert old_mode == new_mode
+
+
 def test_is_text_file(tmpdir):
     p = tmpdir.mkdir("sub").join("random_text.txt")
     p.write("Some random text")


Code review comments:
diff_hunk: @@ -235,10 +235,25 @@ def write(text, filename, encoding='utf-8', mode='wb'):
         with open(filename, mode) as textfile:
             textfile.write(text)
     else:
+        # Based in the solution at untitaker/python-atomicwrites#42
+        # Needed to fix file permissions overwritting
+        # See spyder-ide/spyder#9381
+        try:
+            FileNotFoundError
+        except NameError:  # Python 2
+            FileNotFoundError = OSError
path: spyder/utils/encoding.py
text: @user1:
@author, this is very convoluted. Let's use `OSError` for now and let's leave a comment below saying that we should really use `FileNotFoundError` when we move to Python 3.