Be less grumpy about existing file permissions. (#359)

* Don't set permissions on shell profiles

* Alter CreateOrInsertFile permissions behavior.

* Fixup tests
This commit is contained in:
Ana Hobden 2023-03-22 09:26:34 -07:00 committed by GitHub
parent 775df0383f
commit f73f8fef38
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
2 changed files with 16 additions and 32 deletions

View file

@ -79,11 +79,12 @@ impl CreateOrInsertIntoFile {
let discovered_mode = discovered_mode & 0o777; let discovered_mode = discovered_mode & 0o777;
if discovered_mode != mode { if discovered_mode != mode {
return Err(ActionError::PathModeMismatch( tracing::debug!(
this.path.clone(), "`{}` has mode `{}`, a mode of `{}` was expected",
this.path.display(),
discovered_mode, discovered_mode,
mode, mode,
)); );
} }
} }
@ -459,17 +460,19 @@ mod test {
} }
#[tokio::test] #[tokio::test]
async fn recognizes_wrong_mode_and_errors() -> eyre::Result<()> { async fn recognizes_wrong_mode_and_does_not_error() -> eyre::Result<()> {
let temp_dir = tempfile::tempdir()?; let temp_dir = tempfile::tempdir()?;
let test_file = temp_dir.path().join("recognizes_wrong_mode_and_errors"); let test_file = temp_dir
.path()
.join("recognizes_wrong_mode_and_does_not_error");
let initial_mode = 0o777; let initial_mode = 0o777;
let expected_mode = 0o000; let expected_mode = 0o666;
write(test_file.as_path(), "Some content").await?; write(test_file.as_path(), "Some content").await?;
tokio::fs::set_permissions(test_file.as_path(), PermissionsExt::from_mode(initial_mode)) tokio::fs::set_permissions(test_file.as_path(), PermissionsExt::from_mode(initial_mode))
.await?; .await?;
match CreateOrInsertIntoFile::plan( let mut action = CreateOrInsertIntoFile::plan(
test_file.clone(), test_file.clone(),
None, None,
None, None,
@ -477,19 +480,11 @@ mod test {
"Some different content".into(), "Some different content".into(),
Position::End, Position::End,
) )
.await .await?;
{
Err(ActionError::PathModeMismatch(path, got, expected)) => { action.try_execute().await?;
assert_eq!(path, test_file.as_path());
assert_eq!(expected, expected_mode); action.try_revert().await?;
assert_eq!(got, initial_mode);
},
_ => {
return Err(eyre!(
"Should have returned an ActionError::PathModeMismatch error"
))
},
}
assert!(test_file.exists(), "File should have not been deleted"); assert!(test_file.exists(), "File should have not been deleted");

View file

@ -3,7 +3,6 @@ use crate::action::{Action, ActionDescription, ActionError, ActionTag, StatefulA
use nix::unistd::User; use nix::unistd::User;
use std::path::{Path, PathBuf}; use std::path::{Path, PathBuf};
use target_lexicon::OperatingSystem;
use tokio::task::JoinSet; use tokio::task::JoinSet;
use tracing::{span, Instrument, Span}; use tracing::{span, Instrument, Span};
@ -88,22 +87,12 @@ impl ConfigureShellProfile {
); );
continue; continue;
} }
// Macs require a different mode on certain files...
let required_mode = match target_lexicon::OperatingSystem::host() {
OperatingSystem::MacOSX {
major: _,
minor: _,
patch: _,
}
| OperatingSystem::Darwin => 0o444,
_ => 0o644,
};
create_or_insert_files.push( create_or_insert_files.push(
CreateOrInsertIntoFile::plan( CreateOrInsertIntoFile::plan(
profile_target_path, profile_target_path,
None, None,
None, None,
required_mode, 0o644,
shell_buf.to_string(), shell_buf.to_string(),
create_or_insert_into_file::Position::Beginning, create_or_insert_into_file::Position::Beginning,
) )