From a11fe15e74b06ef612a79789165fbf375423723a Mon Sep 17 00:00:00 2001 From: Eliah Kagan Date: Sun, 18 Aug 2024 20:10:13 -0400 Subject: [PATCH 1/8] Fix assertion message for joining `c:` to `relative` This rewrites the custom message in that assertion so it states the applicable semantics and avoids claiming that `c:relative` and `c:\relative` are equivalent, which is not generally, nor usually, the case. (The message was also written with an unescaped `\r` in a non-raw string literal, but a literal `\r` was intended. The new wording no longer includes an explicit path, but if that is added then the problem can be avoided by writing `\\r` or writing a raw string.) This relates to: - https://github.com/Byron/gitoxide/pull/1374/files/e955770c0b5c06a6c8518a06df4aa0cc3b506f16#r1721045107 --- gix-fs/tests/stack/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gix-fs/tests/stack/mod.rs b/gix-fs/tests/stack/mod.rs index c79db87dbf9..58132d3694c 100644 --- a/gix-fs/tests/stack/mod.rs +++ b/gix-fs/tests/stack/mod.rs @@ -59,7 +59,7 @@ fn path_join_handling() { assert_eq!( p("c:").join("relative"), p("c:relative"), - "drive + relative = strange joined result with missing backslash, but it's a valid path that works just like `c:\relative`" + "drive + relative = relative to the drive-specific current directory" ); assert_eq!( p("c:\\").join("relative"), From 695f8255c01afa0e9b0196daa2151d401845a0d2 Mon Sep 17 00:00:00 2001 From: Eliah Kagan Date: Sun, 18 Aug 2024 20:26:59 -0400 Subject: [PATCH 2/8] Improve messages about joining `C:` on the right Although the path `C:` is relative because it indicates the current directory on the C: drive, it still "takes over" when concatenated after at most if not all paths (including absolute paths). When the paths it is concatenated with are absolute paths with drive letters other than C:, this behavior, while not obvious, is conceptually clear such that no other behavior could be correct. This is because the (drive-specific) current directory on the C: drive is not located on another drive, and therefore not found under any path that specifies a drive that is not C:. (There are other situations where `C:` on the right takes precedence where the explanation is less elegant, but they are mostly not relevant to the assertions currently present here.) This relates to: - https://github.com/Byron/gitoxide/pull/1374/files/e955770c0b5c06a6c8518a06df4aa0cc3b506f16#r1721055278 - https://github.com/Byron/gitoxide/pull/1374/files/e955770c0b5c06a6c8518a06df4aa0cc3b506f16#r1721079186 --- gix-fs/tests/stack/mod.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/gix-fs/tests/stack/mod.rs b/gix-fs/tests/stack/mod.rs index 58132d3694c..dbe0e027fe2 100644 --- a/gix-fs/tests/stack/mod.rs +++ b/gix-fs/tests/stack/mod.rs @@ -92,7 +92,7 @@ fn path_join_handling() { assert_eq!( p("d:/").join("C:"), p("C:"), - "d-drive + c-drive = c-drive - interesting, as C: is supposed to be relative" + "d-drive + c-drive-relative = c-drive-relative - C: is relative but not on D:" ); assert_eq!( p("d:\\").join("C:\\"), @@ -113,7 +113,7 @@ fn path_join_handling() { assert_eq!( p("\\\\.").join("C:"), p("C:"), - "device-namespace-unc + win-drive-relative = win-drive-relative - c: was supposed to be relative, but it's not acting like it." + "device-namespace-unc + win-drive-relative = win-drive-relative - C: as a relative path is not the C: device, so this is not \\\\.\\C:" ); assert_eq!(p("relative").join("C:"), p("C:"), "relative + win-drive = win-drive"); From 9de0ae0b9eb0e9304a17413344b0562d98a78d3b Mon Sep 17 00:00:00 2001 From: Eliah Kagan Date: Sun, 18 Aug 2024 21:20:00 -0400 Subject: [PATCH 3/8] Fix accidental duplicate assertion meant to test bs_absolute --- gix-fs/tests/stack/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gix-fs/tests/stack/mod.rs b/gix-fs/tests/stack/mod.rs index dbe0e027fe2..f06d4111b8f 100644 --- a/gix-fs/tests/stack/mod.rs +++ b/gix-fs/tests/stack/mod.rs @@ -42,7 +42,7 @@ fn path_join_handling() { ); let bs_absolute = p("\\absolute"); assert!( - absolute.is_relative(), + bs_absolute.is_relative(), "on Windows, strange single-backslash paths are relative (and relative to the current drive)" ); assert_eq!( From f0d3e7c658c80bd034122ca2918f194d64a7b113 Mon Sep 17 00:00:00 2001 From: Eliah Kagan Date: Sun, 18 Aug 2024 21:22:51 -0400 Subject: [PATCH 4/8] Rename variables to avoid confusion Since the paths `/absolute` and `\absolute` are not absolute paths on Windows. --- gix-fs/tests/stack/mod.rs | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/gix-fs/tests/stack/mod.rs b/gix-fs/tests/stack/mod.rs index f06d4111b8f..a7b7099ee26 100644 --- a/gix-fs/tests/stack/mod.rs +++ b/gix-fs/tests/stack/mod.rs @@ -35,24 +35,24 @@ fn p(s: &str) -> &Path { #[test] #[cfg(windows)] fn path_join_handling() { - let absolute = p("/absolute"); + let looks_absolute = p("/absolute"); assert!( - absolute.is_relative(), + looks_absolute.is_relative(), "on Windows, absolute Linux paths are considered relative (and relative to the current drive)" ); - let bs_absolute = p("\\absolute"); + let bs_looks_absolute = p("\\absolute"); assert!( - bs_absolute.is_relative(), + bs_looks_absolute.is_relative(), "on Windows, strange single-backslash paths are relative (and relative to the current drive)" ); assert_eq!( - p("relative").join(absolute), - absolute, + p("relative").join(looks_absolute), + looks_absolute, "relative + absolute = absolute - however, they kind of act like they are absolute in conjunction with relative base paths" ); assert_eq!( - p("relative").join(bs_absolute), - bs_absolute, + p("relative").join(bs_looks_absolute), + bs_looks_absolute, "relative + absolute = absolute - backslashes aren't special here, and it just acts like it's absolute" ); @@ -68,22 +68,22 @@ fn path_join_handling() { ); assert_eq!( - p("\\\\?\\base").join(absolute), + p("\\\\?\\base").join(looks_absolute), p("\\\\?\\base\\absolute"), "absolute1 + unix-absolute2 = joined result with backslash" ); assert_eq!( - p("\\\\.\\base").join(absolute), + p("\\\\.\\base").join(looks_absolute), p("\\\\.\\base\\absolute"), "absolute1 + absolute2 = joined result with backslash (device namespace)" ); assert_eq!( - p("\\\\?\\base").join(bs_absolute), + p("\\\\?\\base").join(bs_looks_absolute), p("\\\\?\\base\\absolute"), "absolute1 + absolute2 = joined result" ); assert_eq!( - p("\\\\.\\base").join(bs_absolute), + p("\\\\.\\base").join(bs_looks_absolute), p("\\\\.\\base\\absolute"), "absolute1 + absolute2 = joined result (device namespace)" ); From a34c0db0f0d2d88563c8ca396341124d06db3b8b Mon Sep 17 00:00:00 2001 From: Eliah Kagan Date: Sun, 18 Aug 2024 21:30:21 -0400 Subject: [PATCH 5/8] Update assertion messages to reflect status of `/` and `\` paths --- gix-fs/tests/stack/mod.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/gix-fs/tests/stack/mod.rs b/gix-fs/tests/stack/mod.rs index a7b7099ee26..82d60b37ec7 100644 --- a/gix-fs/tests/stack/mod.rs +++ b/gix-fs/tests/stack/mod.rs @@ -38,7 +38,7 @@ fn path_join_handling() { let looks_absolute = p("/absolute"); assert!( looks_absolute.is_relative(), - "on Windows, absolute Linux paths are considered relative (and relative to the current drive)" + "on Windows, 'absolute' Linux paths are relative (and relative to the current drive)" ); let bs_looks_absolute = p("\\absolute"); assert!( @@ -48,12 +48,12 @@ fn path_join_handling() { assert_eq!( p("relative").join(looks_absolute), looks_absolute, - "relative + absolute = absolute - however, they kind of act like they are absolute in conjunction with relative base paths" + "relative + unix-absolute = unix-absolute - the relative path without a drive is replaced" ); assert_eq!( p("relative").join(bs_looks_absolute), bs_looks_absolute, - "relative + absolute = absolute - backslashes aren't special here, and it just acts like it's absolute" + "relative + unix-absolute = unix-absolute - the relative path without a drive is replaced - backslashes aren't special here" ); assert_eq!( From 4d740337f11f502950feb648810add765b68eda8 Mon Sep 17 00:00:00 2001 From: Eliah Kagan Date: Sun, 18 Aug 2024 21:34:09 -0400 Subject: [PATCH 6/8] Clarify assertion messages involving `\\localhost` This abbreviates the significance of `\\localhost` more specifically, as win-absolute-unc-host rather than win-absolue-unc. It also points out what seems unusual about joining `\` and `\\localhost` to get `\localhost` with just one backslash. This message should be further revised to state why this behavior happens or otherwise describe it clearly, once known. (Or if this turns out to be a bug in the standard library, then that can be stated, with an issue link either in the code or commit message.) This relates to: - https://github.com/Byron/gitoxide/pull/1374/files/e955770c0b5c06a6c8518a06df4aa0cc3b506f16#r1721079809 --- gix-fs/tests/stack/mod.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/gix-fs/tests/stack/mod.rs b/gix-fs/tests/stack/mod.rs index 82d60b37ec7..e7e708a930a 100644 --- a/gix-fs/tests/stack/mod.rs +++ b/gix-fs/tests/stack/mod.rs @@ -120,12 +120,12 @@ fn path_join_handling() { assert_eq!( p("/").join("\\\\localhost"), p("\\localhost"), - "unix-absolute + win-absolute-unc = win-absolute-unc" + "unix-absolute + win-absolute-unc-host = strangely, single-backslashed host" ); assert_eq!( p("relative").join("\\\\localhost"), p("\\\\localhost"), - "relative + win-absolute-unc = win-absolute-unc" + "relative + win-absolute-unc-host = win-absolute-unc-host" ); } From b4e1a78f7ce4e8348701d401f3e8649cb81fbc4f Mon Sep 17 00:00:00 2001 From: Eliah Kagan Date: Sun, 18 Aug 2024 21:41:45 -0400 Subject: [PATCH 7/8] Fix non-Windows tests with `\\localhost` paths So they test what they strongly appear to be intending to test: the behavior of a UNC-style path `\\localhost`. They were instead testing `\localhost` because `\\` in a non-raw string becomes a single backslash. Although the symbolic representations in the assertion messages were clear enough to identify what the tests intended to test, this also adjusts those slightly so they remain comparable to the corresponding Windows tests whose assertion messages were recently changed. --- gix-fs/tests/stack/mod.rs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/gix-fs/tests/stack/mod.rs b/gix-fs/tests/stack/mod.rs index e7e708a930a..398e5c584d9 100644 --- a/gix-fs/tests/stack/mod.rs +++ b/gix-fs/tests/stack/mod.rs @@ -165,14 +165,14 @@ fn path_join_handling() { ); assert_eq!( - p("/").join("\\localhost"), - p("/\\localhost"), - "absolute + win-absolute-unc = joined result" + p("/").join("\\\\localhost"), + p("/\\\\localhost"), + "absolute + win-absolute-unc-host = joined result" ); assert_eq!( - p("relative").join("\\localhost"), - p("relative/\\localhost"), - "relative + win-absolute-unc = joined result" + p("relative").join("\\\\localhost"), + p("relative/\\\\localhost"), + "relative + win-absolute-unc-host = joined result" ); } From 025e788685d3bb070c4995bf19ac8bddb30ec9c1 Mon Sep 17 00:00:00 2001 From: Eliah Kagan Date: Sun, 18 Aug 2024 21:50:55 -0400 Subject: [PATCH 8/8] Use raw string literals for paths with backslashes --- gix-fs/tests/stack/mod.rs | 58 +++++++++++++++++++-------------------- 1 file changed, 29 insertions(+), 29 deletions(-) diff --git a/gix-fs/tests/stack/mod.rs b/gix-fs/tests/stack/mod.rs index 398e5c584d9..b6903396d93 100644 --- a/gix-fs/tests/stack/mod.rs +++ b/gix-fs/tests/stack/mod.rs @@ -40,7 +40,7 @@ fn path_join_handling() { looks_absolute.is_relative(), "on Windows, 'absolute' Linux paths are relative (and relative to the current drive)" ); - let bs_looks_absolute = p("\\absolute"); + let bs_looks_absolute = p(r"\absolute"); assert!( bs_looks_absolute.is_relative(), "on Windows, strange single-backslash paths are relative (and relative to the current drive)" @@ -62,29 +62,29 @@ fn path_join_handling() { "drive + relative = relative to the drive-specific current directory" ); assert_eq!( - p("c:\\").join("relative"), - p("c:\\relative"), + p(r"c:\").join("relative"), + p(r"c:\relative"), "absolute + relative = joined result" ); assert_eq!( - p("\\\\?\\base").join(looks_absolute), - p("\\\\?\\base\\absolute"), + p(r"\\?\base").join(looks_absolute), + p(r"\\?\base\absolute"), "absolute1 + unix-absolute2 = joined result with backslash" ); assert_eq!( - p("\\\\.\\base").join(looks_absolute), - p("\\\\.\\base\\absolute"), + p(r"\\.\base").join(looks_absolute), + p(r"\\.\base\absolute"), "absolute1 + absolute2 = joined result with backslash (device namespace)" ); assert_eq!( - p("\\\\?\\base").join(bs_looks_absolute), - p("\\\\?\\base\\absolute"), + p(r"\\?\base").join(bs_looks_absolute), + p(r"\\?\base\absolute"), "absolute1 + absolute2 = joined result" ); assert_eq!( - p("\\\\.\\base").join(bs_looks_absolute), - p("\\\\.\\base\\absolute"), + p(r"\\.\base").join(bs_looks_absolute), + p(r"\\.\base\absolute"), "absolute1 + absolute2 = joined result (device namespace)" ); @@ -95,36 +95,36 @@ fn path_join_handling() { "d-drive + c-drive-relative = c-drive-relative - C: is relative but not on D:" ); assert_eq!( - p("d:\\").join("C:\\"), - p("C:\\"), + p(r"d:\").join(r"C:\"), + p(r"C:\"), "d-drive-with-bs + c-drive-with-bs = c-drive-with-bs - nothing special happens with backslashes" ); assert_eq!( - p("c:\\").join("\\\\.\\"), - p("\\\\.\\"), + p(r"c:\").join(r"\\.\"), + p(r"\\.\"), "c-drive-with-bs + device-namespace-unc = device-namespace-unc" ); assert_eq!( p("/").join("C:/"), - p("C:\\"), + p(r"C:\"), "unix-absolute + win-drive = win-drive, strangely enough it changed the trailing slash to backslash, so better not have trailing slashes" ); - assert_eq!(p("/").join("C:\\"), p("C:\\"), "unix-absolute + win-drive = win-drive"); + assert_eq!(p("/").join(r"C:\"), p(r"C:\"), "unix-absolute + win-drive = win-drive"); assert_eq!( - p("\\\\.").join("C:"), + p(r"\\.").join("C:"), p("C:"), - "device-namespace-unc + win-drive-relative = win-drive-relative - C: as a relative path is not the C: device, so this is not \\\\.\\C:" + r"device-namespace-unc + win-drive-relative = win-drive-relative - C: as a relative path is not the C: device, so this is not \\.\C:" ); assert_eq!(p("relative").join("C:"), p("C:"), "relative + win-drive = win-drive"); assert_eq!( - p("/").join("\\\\localhost"), - p("\\localhost"), + p("/").join(r"\\localhost"), + p(r"\localhost"), "unix-absolute + win-absolute-unc-host = strangely, single-backslashed host" ); assert_eq!( - p("relative").join("\\\\localhost"), - p("\\\\localhost"), + p("relative").join(r"\\localhost"), + p(r"\\localhost"), "relative + win-absolute-unc-host = win-absolute-unc-host" ); } @@ -154,8 +154,8 @@ fn path_join_handling() { assert_eq!(p("/").join("C:"), p("/C:"), "absolute + win-drive = joined result"); assert_eq!(p("/").join("C:/"), p("/C:/"), "absolute + win-absolute = joined result"); assert_eq!( - p("/").join("C:\\"), - p("/C:\\"), + p("/").join(r"C:\"), + p(r"/C:\"), "absolute + win-absolute = joined result" ); assert_eq!( @@ -165,13 +165,13 @@ fn path_join_handling() { ); assert_eq!( - p("/").join("\\\\localhost"), - p("/\\\\localhost"), + p("/").join(r"\\localhost"), + p(r"/\\localhost"), "absolute + win-absolute-unc-host = joined result" ); assert_eq!( - p("relative").join("\\\\localhost"), - p("relative/\\\\localhost"), + p("relative").join(r"\\localhost"), + p(r"relative/\\localhost"), "relative + win-absolute-unc-host = joined result" ); }