From ea012af083312307b707bf6deb76cb2153f529b7 Mon Sep 17 00:00:00 2001 From: Benjamin Peinhardt Date: Thu, 6 Nov 2025 11:15:41 -0600 Subject: [PATCH 1/4] fix error message bugs --- CHANGELOG.md | 4 ++++ gleam.toml | 2 +- src/simplifile.gleam | 11 +-------- src/simplifile_erl.erl | 47 ++++++++++++++++++++++++++++++++++++-- src/simplifile_js.mjs | 4 ++-- test/simplifile_test.gleam | 16 +++++++++++++ 6 files changed, 69 insertions(+), 15 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 456be30..84885ff 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,10 @@ ## Unreleased +## v2.3.1 - 6 November 2025 +- Fix inconsistent error coming from `filelib:ensure_dir`. +- Fix bug with error string typo for `Enotdir` on JS. + ## v2.3.0 - 24 June 2025 - Add `create_link` function for hard-linking - Fix bug where one could silently write non-byte aligned data to file, now throws explicit error. diff --git a/gleam.toml b/gleam.toml index 06ed8ee..33a959a 100644 --- a/gleam.toml +++ b/gleam.toml @@ -1,5 +1,5 @@ name = "simplifile" -version = "2.3.0" +version = "2.3.1" description = "Basic file operations that work on all targets" licences = ["Apache-2.0"] diff --git a/src/simplifile.gleam b/src/simplifile.gleam index f908e3b..b38c40c 100644 --- a/src/simplifile.gleam +++ b/src/simplifile.gleam @@ -507,16 +507,7 @@ pub fn create_file(at filepath: String) -> Result(Nil, FileError) { /// `./a/b.txt`, a folder named `b.txt` will be created, so be sure /// to pass only the path to the required directory. pub fn create_directory_all(dirpath: String) -> Result(Nil, FileError) { - let is_abs = filepath.is_absolute(dirpath) - let path = - dirpath - |> filepath.split - |> list.fold("", filepath.join) - let path = case is_abs { - True -> "/" <> path - False -> path - } - do_create_dir_all(path <> "/") + do_create_dir_all(dirpath <> "/") } @external(erlang, "simplifile_erl", "create_dir_all") diff --git a/src/simplifile_erl.erl b/src/simplifile_erl.erl index 810c2f3..e6da221 100644 --- a/src/simplifile_erl.erl +++ b/src/simplifile_erl.erl @@ -6,7 +6,7 @@ -module(simplifile_erl). -% We call the +% We call the -compile({no_auto_import,[link/2]}). %% API @@ -148,7 +148,50 @@ delete(Dir) -> %% Creates the entire path for a given directory to exist create_dir_all(Filename) -> - posix_result(filelib:ensure_dir(Filename)). + posix_result(ensure_path(Filename)). + +ensure_path("") -> {error, einval}; +ensure_path(".") -> {ok, nil}; +ensure_path("/") -> {ok, nil}; + +ensure_path(Path) -> + case file:read_file_info(Path) of + % The entry already exists + {ok, FileInfo} -> + case FileInfo#file_info.type of + % It's a directory, so we're good + directory -> + {ok, nil}; + % It's a file or some other weird "file", no good + _ -> + {error, eexist} + end; + + % The entry does not already exist, so we need to make it + {error, enoent} -> + case filename:dirname(Path) of + Parent when Parent =:= Path -> + {error,einval}; + Parent -> + % Ensure the parent directory exists and then create the next segment. + case ensure_path(Parent) of + {ok, nil} -> + case file:make_dir(Path) of + ok -> {ok, nil}; + % This is the handle the possible race condition that somebody else created + % the directory in between the file info check and now. + % If the directory exists, it's okay, because that's what we wanted anyway. + {error, eexist} -> {ok, nil}; + {error, Reason} -> {error, Reason} + end; + {error, Reason} -> + {error, Reason} + end + end; + + {error, Reason} -> + {error, Reason} + end. %% Move file from one path to another rename_file(Source, Destination) -> diff --git a/src/simplifile_js.mjs b/src/simplifile_js.mjs index 22afe4b..1c003df 100644 --- a/src/simplifile_js.mjs +++ b/src/simplifile_js.mjs @@ -136,7 +136,7 @@ export function createSymlink(target, path) { * Create the "hard link" called path pointing to the target * * @param {string} target - * @param {sting} path + * @param {string} path * returns {ok | GError} */ export function createLink(target, path) { @@ -364,7 +364,7 @@ function cast_error(error_code) { return new $simplifile.Enosys(); case "ENOBLK": return new $simplifile.Enotblk(); - case "ENODIR": + case "ENOTDIR": return new $simplifile.Enotdir(); case "ENOTSUP": return new $simplifile.Enotsup(); diff --git a/test/simplifile_test.gleam b/test/simplifile_test.gleam index 1b3f315..515845c 100644 --- a/test/simplifile_test.gleam +++ b/test/simplifile_test.gleam @@ -753,3 +753,19 @@ pub fn rename_file_succeeds_at_renaming_a_directory_test() { fi |> file_info_type |> should.equal(Directory) read(new_dir <> "/i_am_a_file.txt") |> should.be_ok |> should.equal("Hello") } +// This test passes but is commented out for now as it relies on an existing file on my machine. +// TODO: Clean up this test to create the required file automatically. + +// pub fn inconsistent_errors_test() { +// // I have created a directory with sudo in tmp at /tmp/wibble +// let err = simplifile.create_directory("/tmp/wibble/wobble") |> should.be_error +// err |> should.equal(Eacces) + +// let err = simplifile.create_directory_all("/tmp/wibble/wobble/wumbo") |> should.be_error +// err |> should.equal(Eacces) + +// // Great, now let's fix the other issue +// simplifile.create_file("./tmp/wumbo") |> should.be_ok +// let err = simplifile.create_directory_all("./tmp/wumbo/wombo") |> should.be_error +// err |> should.equal(Enotdir) +// } From aa8444b73770bddc3661e981c53e25a4649da68b Mon Sep 17 00:00:00 2001 From: Benjamin Peinhardt Date: Thu, 6 Nov 2025 11:37:15 -0600 Subject: [PATCH 2/4] cleanup --- src/simplifile_erl.erl | 7 ++++--- test/simplifile_test.gleam | 8 ++++++-- 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/src/simplifile_erl.erl b/src/simplifile_erl.erl index e6da221..ca0c7a6 100644 --- a/src/simplifile_erl.erl +++ b/src/simplifile_erl.erl @@ -6,7 +6,6 @@ -module(simplifile_erl). -% We call the -compile({no_auto_import,[link/2]}). %% API @@ -162,9 +161,9 @@ ensure_path(Path) -> % It's a directory, so we're good directory -> {ok, nil}; - % It's a file or some other weird "file", no good + % It's a file or some other weird "file", no good. _ -> - {error, eexist} + {error, enotdir} end; % The entry does not already exist, so we need to make it @@ -189,6 +188,8 @@ ensure_path(Path) -> end end; + % This case actually does occur in the recursion. For example, when trying to ensure_path("wibble/wumbo/wombo"), + % if wumbo is a file and not a directory, the enotdir will come through this case. {error, Reason} -> {error, Reason} end. diff --git a/test/simplifile_test.gleam b/test/simplifile_test.gleam index 515845c..4c3f7d3 100644 --- a/test/simplifile_test.gleam +++ b/test/simplifile_test.gleam @@ -753,6 +753,7 @@ pub fn rename_file_succeeds_at_renaming_a_directory_test() { fi |> file_info_type |> should.equal(Directory) read(new_dir <> "/i_am_a_file.txt") |> should.be_ok |> should.equal("Hello") } + // This test passes but is commented out for now as it relies on an existing file on my machine. // TODO: Clean up this test to create the required file automatically. @@ -761,11 +762,14 @@ pub fn rename_file_succeeds_at_renaming_a_directory_test() { // let err = simplifile.create_directory("/tmp/wibble/wobble") |> should.be_error // err |> should.equal(Eacces) -// let err = simplifile.create_directory_all("/tmp/wibble/wobble/wumbo") |> should.be_error +// let err = +// simplifile.create_directory_all("/tmp/wibble/wobble/wumbo") +// |> should.be_error // err |> should.equal(Eacces) // // Great, now let's fix the other issue // simplifile.create_file("./tmp/wumbo") |> should.be_ok -// let err = simplifile.create_directory_all("./tmp/wumbo/wombo") |> should.be_error +// let err = +// simplifile.create_directory_all("./tmp/wumbo/wombo") |> should.be_error // err |> should.equal(Enotdir) // } From 059c88250d2d5536322754390ea2888c81740f15 Mon Sep 17 00:00:00 2001 From: Benjamin Peinhardt Date: Thu, 6 Nov 2025 12:05:44 -0600 Subject: [PATCH 3/4] go back to ensure_path --- src/simplifile_erl.erl | 47 +------------------------------------- test/simplifile_test.gleam | 2 +- 2 files changed, 2 insertions(+), 47 deletions(-) diff --git a/src/simplifile_erl.erl b/src/simplifile_erl.erl index ca0c7a6..09bef7a 100644 --- a/src/simplifile_erl.erl +++ b/src/simplifile_erl.erl @@ -147,52 +147,7 @@ delete(Dir) -> %% Creates the entire path for a given directory to exist create_dir_all(Filename) -> - posix_result(ensure_path(Filename)). - -ensure_path("") -> {error, einval}; -ensure_path(".") -> {ok, nil}; -ensure_path("/") -> {ok, nil}; - -ensure_path(Path) -> - case file:read_file_info(Path) of - % The entry already exists - {ok, FileInfo} -> - case FileInfo#file_info.type of - % It's a directory, so we're good - directory -> - {ok, nil}; - % It's a file or some other weird "file", no good. - _ -> - {error, enotdir} - end; - - % The entry does not already exist, so we need to make it - {error, enoent} -> - case filename:dirname(Path) of - Parent when Parent =:= Path -> - {error,einval}; - Parent -> - % Ensure the parent directory exists and then create the next segment. - case ensure_path(Parent) of - {ok, nil} -> - case file:make_dir(Path) of - ok -> {ok, nil}; - % This is the handle the possible race condition that somebody else created - % the directory in between the file info check and now. - % If the directory exists, it's okay, because that's what we wanted anyway. - {error, eexist} -> {ok, nil}; - {error, Reason} -> {error, Reason} - end; - {error, Reason} -> - {error, Reason} - end - end; - - % This case actually does occur in the recursion. For example, when trying to ensure_path("wibble/wumbo/wombo"), - % if wumbo is a file and not a directory, the enotdir will come through this case. - {error, Reason} -> - {error, Reason} - end. + posix_result(filelib:ensure_path(Filename)). %% Move file from one path to another rename_file(Source, Destination) -> diff --git a/test/simplifile_test.gleam b/test/simplifile_test.gleam index 4c3f7d3..f040319 100644 --- a/test/simplifile_test.gleam +++ b/test/simplifile_test.gleam @@ -765,7 +765,7 @@ pub fn rename_file_succeeds_at_renaming_a_directory_test() { // let err = // simplifile.create_directory_all("/tmp/wibble/wobble/wumbo") // |> should.be_error -// err |> should.equal(Eacces) +// [Eacces, Enoent] |> list.contains(err) |> should.be_true // // Great, now let's fix the other issue // simplifile.create_file("./tmp/wumbo") |> should.be_ok From 9e3751c731f641e9fbf40141ddd12bb9deb5f518 Mon Sep 17 00:00:00 2001 From: Benjamin Peinhardt Date: Thu, 6 Nov 2025 12:15:38 -0600 Subject: [PATCH 4/4] update test --- CHANGELOG.md | 1 - test/simplifile_test.gleam | 26 +++++++------------------- 2 files changed, 7 insertions(+), 20 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 84885ff..5d49b3c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,7 +5,6 @@ ## Unreleased ## v2.3.1 - 6 November 2025 -- Fix inconsistent error coming from `filelib:ensure_dir`. - Fix bug with error string typo for `Enotdir` on JS. ## v2.3.0 - 24 June 2025 diff --git a/test/simplifile_test.gleam b/test/simplifile_test.gleam index f040319..48701c6 100644 --- a/test/simplifile_test.gleam +++ b/test/simplifile_test.gleam @@ -754,22 +754,10 @@ pub fn rename_file_succeeds_at_renaming_a_directory_test() { read(new_dir <> "/i_am_a_file.txt") |> should.be_ok |> should.equal("Hello") } -// This test passes but is commented out for now as it relies on an existing file on my machine. -// TODO: Clean up this test to create the required file automatically. - -// pub fn inconsistent_errors_test() { -// // I have created a directory with sudo in tmp at /tmp/wibble -// let err = simplifile.create_directory("/tmp/wibble/wobble") |> should.be_error -// err |> should.equal(Eacces) - -// let err = -// simplifile.create_directory_all("/tmp/wibble/wobble/wumbo") -// |> should.be_error -// [Eacces, Enoent] |> list.contains(err) |> should.be_true - -// // Great, now let's fix the other issue -// simplifile.create_file("./tmp/wumbo") |> should.be_ok -// let err = -// simplifile.create_directory_all("./tmp/wumbo/wombo") |> should.be_error -// err |> should.equal(Enotdir) -// } +pub fn parse_errors_test() { + // Ensuring the ENOTDIR comes through correctly + simplifile.create_file("./tmp/wumbo") |> should.be_ok + let err = + simplifile.create_directory_all("./tmp/wumbo/wombo") |> should.be_error + err |> should.equal(Enotdir) +}