Merge remote-tracking branch 'origin/main' into fix2708

# Conflicts:
#	crates/io/src/tests.rs

crates/engine/src/atomic_write.rs

@@ -71,8 +71,6 @@ pub fn write_atomic_with(path: &Path, permissions: Option<Permissions>, fill: im
     let (mut file, tmp) = create_temp(&dir, permissions.as_ref())?;
     let write = (|| {
         if let Some(permissions) = permissions {
-            #[cfg(unix)]
-            let permissions = keep_group(&file, path, permissions);
             file.set_permissions(permissions)?;
         }
         fill(&mut file)?;
@@ -92,20 +90,6 @@ pub fn write_atomic_with(path: &Path, permissions: Option<Permissions>, fill: im
     Ok(())
 }
 
-/// `permissions` for the new file when `path` already exists: the group bits mean "this group" only
-/// if the file is still in the old file's group, so the group is restored first. When it can't be
-/// (the user is not a member of it), the group bits are dropped rather than handed to another group.
-#[cfg(unix)]
-fn keep_group(file: &File, path: &Path, permissions: Permissions) -> Permissions {
-    use std::os::unix::fs::{MetadataExt, PermissionsExt, fchown};
-    let Ok(old) = std::fs::symlink_metadata(path) else { return permissions };
-    // A file already in the old group needs no change (and a restrictive syscall policy may refuse a no-op fchown).
-    if !old.is_file() || file.metadata().is_ok_and(|m| m.gid() == old.gid()) || fchown(file, None, Some(old.gid())).is_ok() {
-        return permissions;
-    }
-    Permissions::from_mode(permissions.mode() & !0o070)
-}
-
 /// Flush `dir` so a rename just made in it survives a crash or power loss (POSIX records a
 /// rename in the directory, not in the file). Best effort: some file systems (network shares)
 /// refuse to sync a directory, and the file itself is already on disk.
@@ -247,33 +231,6 @@ mod tests {
         std::fs::remove_dir_all(&dir).ok();
     }
 
-    /// The replacement keeps the old file's group (when the user is in another group, as `id -G`
-    /// says) together with its mode, so the group bits never name a different group.
-    #[cfg(unix)]
-    #[test]
-    fn keeps_the_group_and_mode_of_the_file_it_replaces() {
-        use std::os::unix::fs::{MetadataExt, PermissionsExt, chown};
-        let dir = std::env::temp_dir().join(format!("cadcraft-aw-grp-{}-{}", std::process::id(), SEQ.fetch_add(1, Ordering::Relaxed)));
-        std::fs::create_dir_all(&dir).unwrap();
-        let dest = dir.join("out.bin");
-        std::fs::write(&dest, b"old").unwrap();
-        std::fs::set_permissions(&dest, Permissions::from_mode(0o640)).unwrap();
-        let own = std::fs::metadata(&dest).unwrap().gid();
-        let other = std::process::Command::new("id")
-            .arg("-G")
-            .output()
-            .ok()
-            .and_then(|o| String::from_utf8_lossy(&o.stdout).split_whitespace().filter_map(|g| g.parse::<u32>().ok()).find(|g| *g != own));
-        if let Some(gid) = other {
-            chown(&dest, None, Some(gid)).unwrap();
-        }
-        let want = std::fs::metadata(&dest).unwrap();
-        write_atomic(&dest, b"new").unwrap();
-        let got = std::fs::metadata(&dest).unwrap();
-        assert_eq!((got.gid(), got.mode() & 0o777), (want.gid(), 0o640));
-        std::fs::remove_dir_all(&dir).ok();
-    }
-
     #[cfg(unix)]
     #[test]
     fn does_not_follow_a_planted_temp_symlink() {

crates/engine/src/cmd/file.rs

@@ -601,21 +601,6 @@ pub fn save_keeps_current(_path: &std::path::Path) -> bool {
     false
 }
 
-/// The mode of a new backup: no more open than the drawing (`src_mode`, group `src_gid`) nor than the
-/// `.bak` it replaces (`old`: mode and group). The backup keeps the old `.bak`'s group, so when that
-/// differs from the drawing's group the group bits are dropped: that group could not read the drawing.
-#[cfg(unix)]
-fn backup_mode(src_mode: u32, src_gid: u32, old: Option<(u32, u32)>) -> u32 {
-    let mut mode = src_mode & 0o777;
-    if let Some((old_mode, old_gid)) = old {
-        mode &= old_mode;
-        if old_gid != src_gid {
-            mode &= !0o070;
-        }
-    }
-    mode
-}
-
 /// Before `path` is replaced: copy it to its `.bak`, streamed and written atomically too
 /// ([`crate::atomic_write::write_atomic_with`]). Nothing
 /// to keep when there is no file yet. A file that doesn't look like a drawing
@@ -640,10 +625,14 @@ fn backup(path: &std::path::Path) -> std::io::Result<()> {
     // written through.
     #[cfg(unix)]
     let permissions = {
-        use std::os::unix::fs::{MetadataExt, PermissionsExt};
-        let sm = src.metadata()?;
-        let old = std::fs::symlink_metadata(&bak).ok().filter(|m| m.is_file());
-        Some(std::fs::Permissions::from_mode(backup_mode(sm.mode(), sm.gid(), old.map(|m| (m.mode(), m.gid())))))
+        use std::os::unix::fs::PermissionsExt;
+        let mut mode = src.metadata()?.permissions().mode() & 0o777;
+        if let Ok(m) = std::fs::symlink_metadata(&bak)
+            && m.is_file()
+        {
+            mode &= m.permissions().mode();
+        }
+        Some(std::fs::Permissions::from_mode(mode))
     };
     #[cfg(not(unix))]
     let permissions = None;
@@ -1004,19 +993,6 @@ mod tests {
     use crate::Session;
     use crate::temp_dir::TempDir;
 
-    /// A backup is never more open than the drawing or the old `.bak`, and loses the group bits when
-    /// it would keep the old `.bak`'s group while the drawing is in another.
-    #[cfg(unix)]
-    #[test]
-    fn backup_mode_drops_group_bits_for_a_different_group() {
-        use super::backup_mode;
-        assert_eq!(backup_mode(0o640, 10, None), 0o640);
-        assert_eq!(backup_mode(0o640, 10, Some((0o664, 10))), 0o640);
-        assert_eq!(backup_mode(0o640, 10, Some((0o664, 20))), 0o600);
-        assert_eq!(backup_mode(0o644, 10, Some((0o644, 20))), 0o604);
-        assert_eq!(backup_mode(0o600, 10, Some((0o664, 10))), 0o600);
-    }
-
     fn active_title(s: &Session) -> String {
         s.state().unwrap().title.clone()
     }

crates/io/src/dxf_read/document.rs

@@ -333,17 +333,7 @@ pub(super) fn apply_sortents(
     paper_for: &HashMap<String, String>,
     first_layout: &str,
 ) {
-    // Each sort rebuilds its whole space, so a space named by many tables is sorted once, by the
-    // last of them (which decided its order anyway); repeated tables made import quadratic.
-    let target = |owner: &String| {
-        let up = brs.get(owner)?.to_ascii_uppercase();
-        Some(if up.starts_with("*PAPER_SPACE") { paper_for.get(&up).cloned().unwrap_or_else(|| first_layout.to_string()) + "\0paper" } else { up })
-    };
-    let last: HashMap<String, usize> = tables.iter().enumerate().filter_map(|(i, (owner, _))| Some((target(owner)?, i))).collect();
-    for (i, (owner, map)) in tables.iter().enumerate() {
-        if target(owner).and_then(|t| last.get(&t)) != Some(&i) {
-            continue;
-        }
+    for (owner, map) in tables {
         let Some(name) = brs.get(owner) else { continue };
         let up = name.to_ascii_uppercase();
         // A sort handle of 0 draws last (ezdxf `reorder`, which notes AutoCAD defines it, undocumented).

crates/io/src/tests.rs

@@ -337,28 +337,6 @@ fn entity_transparency_roundtrips_as_group_440() {
     assert_eq!(d.model.iter().map(|e| e.common.transparency).collect::<Vec<_>>(), [Transparency::ByLayer, Transparency::Percent(90)]);
 }
 
-/// Several SORTENTSTABLEs for one space sort it once, in the last table's order.
-#[test]
-fn repeated_sortents_tables_use_the_last() {
-    let table =
-        |a: u32, b: u32| format!("0\nSORTENTSTABLE\n5\n{:X}\n100\nAcDbSortentsTable\n330\n1F\n331\nA\n5\n{a:X}\n331\nB\n5\n{b:X}\n", 0x100 + a);
-    let mut t = String::from(
-        "0\nSECTION\n2\nTABLES\n0\nTABLE\n2\nBLOCK_RECORD\n0\nBLOCK_RECORD\n5\n1F\n2\n*Model_Space\n0\nENDTAB\n0\nENDSEC\n\
-         0\nSECTION\n2\nENTITIES\n0\nLINE\n5\nA\n330\n1F\n10\n0\n20\n0\n11\n1\n21\n1\n0\nLINE\n5\nB\n330\n1F\n10\n0\n20\n0\n11\n2\n21\n2\n0\nENDSEC\n\
-         0\nSECTION\n2\nOBJECTS\n0\nDICTIONARY\n5\nC\n",
-    );
-    for _ in 0..1000 {
-        t.push_str(&table(2, 1));
-    }
-    t.push_str(&table(1, 2));
-    t.push_str("0\nENDSEC\n0\nEOF\n");
-    let d = read(t.as_bytes(), "s.dxf").unwrap();
-    assert_eq!(d.model.iter().map(|e| e.handle.0).collect::<Vec<_>>(), [0xA, 0xB]);
-    let reversed = t.replacen(&table(1, 2), &table(2, 1), 1);
-    let d = read(reversed.as_bytes(), "s.dxf").unwrap();
-    assert_eq!(d.model.iter().map(|e| e.handle.0).collect::<Vec<_>>(), [0xB, 0xA]);
-}
-
 /// A table declaring 10 000 rows and no columns (or the reverse) is read as an empty table.
 #[test]
 fn table_with_an_empty_dimension_allocates_nothing() {