`IsExistingFile`, `IsExecutableFile` may return `true` for directories, too

[fingolfin]

This is a mismatch between what is documented (and what the function names suggest) and what the function actually do:

IsExistingFile returns true if a file with the filename filename exists and can be seen by the GAP process

or

IsExecutableFile returns true if a file with the filename filename exists and the GAP process has execute permissions for the file, or false if this is not the case. Note that execute permissions do not imply that it is possible to execute the file, e.g., it may only be executable on a different machine.

But in practice:

gap> IsExistingFile("/usr");
true
gap> IsExecutableFile("/usr");
true

@ThomasBreuer stumbled over when he had a directory called git inside a directory listed in his PATH and as a result PackageManager thought that was a git executable binary and as a consequence did run into an error.

But also the GAP library function PathSystemProgram suffers from the problem, for the same reason: it iterates over all dirs in PATH (i.e. iterates over DirectoriesSystemPrograms() and in each directory looks for a "file" with the given name

BIND_GLOBAL( "PathSystemProgram", function( name )
    local dir, path;

    for dir in DirectoriesSystemPrograms() do
      path:= Filename( dir, name );
      if IsExecutableFile( path ) then
        return path;
      fi;
    od;

    return fail;
end );

Now how do we fix this? An "obvious" fix would be to restrict IsExecutableFile and friends to only report true for "actual" files.

But of course we aso do need a way to test if a directory exists / is readable / is writeable / etc. and I would not be surprised if there is existing GAP which relies on these functions also "working" for directories...

But let's suppose we agree to restrict this to "actual" files, then: what does that mean? Certainly "regular" files. But if we want to match the behavior of the shell, we should also include symlinks pointing to a real file, but not symlinks pointing to directories. And then, recursively: any symlink pointing to a symlink pointing to ... pointing to a regular file ...

[limakzi]

Classic POSIX filesystem quirk, since everything is inode. Since IsExecutableFile in GAP's C kernel likely relies on a basic POSIX access(path, X_OK), it happily returns true for any directory your user account can traverse? Fix for symlinks could be solved by lstat, I think?

How do we think of changing the GAP System kernel behaviour?

https://github.com/gap-system/gap/blob/master/src/sysfiles.c#L2795