Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix updating of readonly files #320

Merged

Conversation

rasa
Copy link
Contributor

@rasa rasa commented Jan 31, 2021

What does this PR do?

Allows yadm alt to update files that are read-only (mode 400 / -r--------).

What issues does this PR fix or reference?

None.

Previous Behavior

google-authenticator stores its configuration in ~/.google_authenticator. Each time the user logs in, the file is set to mode 400, even if the file is 600. yadm alt would then fail to move the $temp_file to the $output file, as the $output file was read-only.

New Behavior

yadm alt can successfully update read-only files.

Have tests been written for this change?

No, but all current tests pass.

Have these commits been signed with GnuPG?

Yes.


Please review yadm's Contributing Guide for best practices.

@TheLocehiliosan TheLocehiliosan self-assigned this Feb 20, 2021
@TheLocehiliosan TheLocehiliosan added this to In progress in 3.1.0 via automation Feb 20, 2021
@TheLocehiliosan
Copy link
Owner

@rasa - Are you sure this is actually a problem? I'm fairly sure that mv replaces a file and does not edit the file, so it is actually the permissions of the directory that matter. I think any read-only file can have another file moved into its place, without problem.

Can you explain where you are seeing this problem? Is it on a specific platform?

I'm curious if you could do a test like this there:

touch ro
chmod 0400 ro
echo testing > src_file
mv -f src_file ro

@rasa
Copy link
Contributor Author

rasa commented Feb 28, 2021

You're right. It's specific to this environment. It's a WSL instance where /home/user has been bind mounted to an NTFS-backed /mnt/c/Users/user. When I cd ~ ; mv -f src_file ro I get:

mv: cannot move 'src_file' to 'ro': Permission denied

Fails in both WSL v1 and v2. Given that it's an environment specific issue, I'd understand if you'd want to reject it, but other setups may see this issue too. Perhaps network mounts via samba or NFS or sshfs or ...? Too edge case?

@TheLocehiliosan
Copy link
Owner

TheLocehiliosan commented Feb 28, 2021

No, I don't think it's a problem to include. I can't see how it will cause any problems. I really wanted to understand the issue, because I couldn't see it myself. Thanks.

@TheLocehiliosan
Copy link
Owner

@rasa - I made an adjustment to this so the read-only state doesn't need to be saved and preserved... Instead the perms are copied onto the already moved output file after the move. Forcing the output to be writable before the move is still done. The change is in branch pr/320. Can you confirm this still works in your case? Or is there some reason that the permissions from the template file cannot be used in that case?

@rasa
Copy link
Contributor Author

rasa commented Mar 5, 2021

@TheLocehiliosan Sorry for the delay. I tested the pr/320 branch in a WSL environment, and templated a 0400 file just fine.

@TheLocehiliosan TheLocehiliosan merged commit 3977376 into TheLocehiliosan:develop Mar 11, 2021
3.1.0 automation moved this from In progress to Done Mar 11, 2021
TheLocehiliosan added a commit that referenced this pull request Mar 22, 2021
* Use `git clone` directly during clone (#289, #323)
* Fix compatibility bug with Git completions (#318, #321)
* Support relative paths for --yadm-* and -w (#301)
* Improve parsing of if-statement in default template (#303)
* Read files without running cat in subshells (#317)
* Improve portability of updating read-only files (#320)
* Various code improvements (#306, #307, #311)
TheLocehiliosan added a commit that referenced this pull request Apr 3, 2021
* Use `git clone` directly during clone (#289, #323)
* Fix compatibility bug with Git completions (#318, #321)
* Support relative paths for --yadm-* and -w (#301)
* Improve parsing of if-statement in default template (#303)
* Read files without running cat in subshells (#317)
* Improve portability of updating read-only files (#320)
* Various code improvements (#306, #307, #311)
@rasa rasa deleted the rs/update-read-only-files branch May 17, 2021 14:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
3.1.0
  
Done
Development

Successfully merging this pull request may close these issues.

None yet

2 participants