Skip to content

Commit

Permalink
Fix RemoveDirectoryRecursively for immutable argument (#5584)
Browse files Browse the repository at this point in the history
It's a bad idea to modify an argument inplace anyway, unless this
behavior is explicitly documented and 'necessary'
  • Loading branch information
fingolfin authored Jan 13, 2024
1 parent 588eff9 commit 9d3fdea
Show file tree
Hide file tree
Showing 2 changed files with 26 additions and 1 deletion.
3 changes: 2 additions & 1 deletion lib/files.gi
Original file line number Diff line number Diff line change
Expand Up @@ -373,7 +373,8 @@ InstallGlobalFunction(RemoveDirectoryRecursively,
Error("dirname must be a directory");
return fail;
fi;
while Length(dirname) > 0 and dirname[Length(dirname)] = '/' do
dirname := ShallowCopy(dirname);
while Length(dirname) > 0 and Last(dirname) = '/' do
Remove(dirname);
od;
if Length(dirname) = 0 then
Expand Down
24 changes: 24 additions & 0 deletions tst/testinstall/kernel/streams.tst
Original file line number Diff line number Diff line change
Expand Up @@ -172,6 +172,30 @@ Error, RemoveDir: <filename> must be a string (not the value 'fail')
gap> IsDir(fail);
Error, IsDir: <filename> must be a string (not the value 'fail')

#
gap> tmpdir := MakeImmutable(TmpDirectory());;
gap> subdir := MakeImmutable(Concatenation(tmpdir, "/subdir"));;
gap> CreateDir(subdir);
true
gap> IsDir(subdir);
'D'
gap> RemoveDir(subdir);
true
gap> IsDir(subdir);
fail
gap> CreateDir(subdir);
true
gap> FileString(Concatenation(subdir, "/file"), "data");
4
gap> IsDir(subdir);
'D'
gap> RemoveDirectoryRecursively(tmpdir);
true
gap> IsDir(subdir);
fail
gap> IsDir(tmpdir);
fail

#
gap> IsExistingFile(fail);
Error, IsExistingFile: <filename> must be a string (not the value 'fail')
Expand Down

0 comments on commit 9d3fdea

Please sign in to comment.