Skip to content

Dir.makePath handles .. in the sub_path differently per-platform #18452

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

Closed
squeek502 opened this issue Jan 5, 2024 · 3 comments · Fixed by #18453
Closed

Dir.makePath handles .. in the sub_path differently per-platform #18452

squeek502 opened this issue Jan 5, 2024 · 3 comments · Fixed by #18453
Labels
bug Observed behavior contradicts documented or intended behavior contributor friendly This issue is limited in scope and/or knowledge of Zig internals. docs os-linux os-windows standard library This issue involves writing Zig code for the standard library.
Milestone

Comments

@squeek502
Copy link
Collaborator

squeek502 commented Jan 5, 2024

Zig Version

0.12.0-dev.2018+9a56228c2

Steps to Reproduce and Observed Behavior

From a test in #17499 (cc @rootbeer):

test "makepath relative walks" {
    if (builtin.os.tag == .wasi) return error.SkipZigTest;

    var tmp = tmpDir(.{});
    defer tmp.cleanup();

    const relPath = try fs.path.join(testing.allocator, &.{
        "first", "..", "second", "..", "third", "..", "first", "A", "..", "B", "..", "C",
    });
    defer testing.allocator.free(relPath);

    try tmp.dir.makePath(relPath);

    // verify created directories exist:
    try expectDir(tmp.dir, "first" ++ fs.path.sep_str ++ "A");
    try expectDir(tmp.dir, "first" ++ fs.path.sep_str ++ "B");
    try expectDir(tmp.dir, "first" ++ fs.path.sep_str ++ "C");
    try expectDir(tmp.dir, "second");
    try expectDir(tmp.dir, "third");
}

Expected Behavior

All platforms to have similar behavior

@squeek502 squeek502 added the bug Observed behavior contradicts documented or intended behavior label Jan 5, 2024
@andrewrk andrewrk added this to the 0.13.0 milestone Jan 5, 2024
@andrewrk andrewrk added the contributor friendly This issue is limited in scope and/or knowledge of Zig internals. label Jan 5, 2024
@squeek502
Copy link
Collaborator Author

squeek502 commented Jan 5, 2024

An additional note: the Linux behavior here matches the behavior of mkdir -p on Linux:

$ ls
$ mkdir -p first/../second/../third
$ ls
first  second  third

So this seems to basically be the same dilemma as #7751 and #4658 (comment)

@andrewrk andrewrk added os-windows standard library This issue involves writing Zig code for the standard library. labels Jan 5, 2024
@andrewrk
Copy link
Member

andrewrk commented Jan 5, 2024

One possible solution is to merely document the differences in platform behavior and leave it to the API user to handle the differences appropriately. For example, some of the file system API mentions that atomicity is platform-dependent.

I think the most important concern is 1. making it practical to write optimal software that does the best combination of syscalls on any given OS, closely followed by 2. making it maintainable to do this for all the supported operating systems being abstracted over.

In other words, I would rather solve the problem by pushing the OS differences onto the API user, than do a bunch of work behind the scenes to make one system emulate a different one.

@squeek502
Copy link
Collaborator Author

Agreed, I think that's the most viable path forward for something like this where trying to hammer one platform's behavior to match the shape of another would likely incur a significant 'optimality' penalty in one direction or another. Plus, matching the platform convention for each platform doesn't seem like the worst thing in this case.

Can always revisit this decision later if there's more context that's missing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Observed behavior contradicts documented or intended behavior contributor friendly This issue is limited in scope and/or knowledge of Zig internals. docs os-linux os-windows standard library This issue involves writing Zig code for the standard library.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants