From 88d71b34e9f5143efb1a9ee45edfeb5d60ecca31 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lars=20G=C3=B6ttgens?= Date: Thu, 6 Feb 2025 01:17:58 +0100 Subject: [PATCH 1/8] Make all root paths absolute --- lib/package.gi | 1 + src/sysroots.c | 17 ++++++++++++++++- src/sysroots.h | 2 +- 3 files changed, 18 insertions(+), 2 deletions(-) diff --git a/lib/package.gi b/lib/package.gi index ba0612a535..94a9ecdbca 100644 --- a/lib/package.gi +++ b/lib/package.gi @@ -1864,6 +1864,7 @@ InstallGlobalFunction( SetPackagePath, function( pkgname, pkgpath ) InstallGlobalFunction( ExtendRootDirectories, function( rootpaths ) local i; + rootpaths:= List( rootpaths, GAP_realpath ); rootpaths:= Filtered( rootpaths, path -> not path in GAPInfo.RootPaths ); if not IsEmpty( rootpaths ) then # 'DirectoriesLibrary' concatenates root paths with directory names. diff --git a/src/sysroots.c b/src/sysroots.c index 42e4e42fe9..29784f5310 100644 --- a/src/sysroots.c +++ b/src/sysroots.c @@ -18,6 +18,7 @@ #include "sysstr.h" #include "system.h" +#include #include @@ -197,7 +198,7 @@ void SySetGapRootPath(const Char * string) return; const UInt userhomelen = strlen(userhome); for (i = 0; i < MAX_GAP_DIRS && SyGapRootPaths[i][0]; i++) { - const UInt pathlen = strlen(SyGapRootPaths[i]); + UInt pathlen = strlen(SyGapRootPaths[i]); if (SyGapRootPaths[i][0] == '~' && userhomelen + pathlen < sizeof(SyGapRootPaths[i])) { SyMemmove(SyGapRootPaths[i] + userhomelen, @@ -205,6 +206,20 @@ void SySetGapRootPath(const Char * string) SyGapRootPaths[i] + 1, pathlen); memcpy(SyGapRootPaths[i], userhome, userhomelen); } + + // convert all paths to absolute paths + Char tempstr[GAP_PATH_MAX]; + + if (NULL == realpath(SyGapRootPaths[i], tempstr)) { + SySetErrorNo(); + } else { + strxcpy(SyGapRootPaths[i], tempstr, sizeof(SyGapRootPaths[i])); + pathlen = strlen(SyGapRootPaths[i]); + if (SyGapRootPaths[i][pathlen - 1] != '/') { + SyGapRootPaths[i][pathlen] = '/'; + SyGapRootPaths[i][pathlen + 1] = '\0'; + } + } } } diff --git a/src/sysroots.h b/src/sysroots.h index ae7341ce94..264e89cb43 100644 --- a/src/sysroots.h +++ b/src/sysroots.h @@ -38,7 +38,7 @@ void SySetGapRootPath(const Char * string); ** ** must point to a buffer of at least characters. This function ** then searches for a readable file with the name in the system -** area. If sich a file is found then its absolute path is copied into +** area. If such a file is found then its absolute path is copied into ** , and is returned. If no file is found or if is not big ** enough, then is set to an empty string and NULL is returned. */ From ff9537eb7148ed0aea9a1240f1d9e667045bfd97 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lars=20G=C3=B6ttgens?= Date: Thu, 6 Feb 2025 01:18:10 +0100 Subject: [PATCH 2/8] Make all package dirs absolute --- lib/package.gi | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/lib/package.gi b/lib/package.gi index 94a9ecdbca..af34bf2596 100644 --- a/lib/package.gi +++ b/lib/package.gi @@ -304,7 +304,7 @@ InstallGlobalFunction( InitializePackagesInfoRecords, function( arg ) # the first time this is called, add the cmd line args to the list if IsEmpty(GAPInfo.PackageDirectories) then for pkgdirstrs in GAPInfo.CommandLineOptions.packagedirs do - pkgdirs:= List( SplitString( pkgdirstrs, ";" ), Directory ); + pkgdirs:= List( List( SplitString( pkgdirstrs, ";" ), GAP_realpath ), Directory ); for pkgdir in pkgdirs do if not pkgdir in GAPInfo.PackageDirectories then Add( GAPInfo.PackageDirectories, pkgdir ); @@ -1897,8 +1897,10 @@ InstallGlobalFunction( ExtendPackageDirectories, function( paths_or_dirs ) changed:= false; for p in paths_or_dirs do if IsString( p ) then - p:= Directory( p ); - elif not IsDirectory( p ) then + p:= Directory( GAP_realpath ( p ) ); + elif IsDirectory( p ) then + p:= Directory( GAP_realpath ( p![1] ) ); + else Error("input must be a list of path strings or directory objects"); fi; if not p in GAPInfo.PackageDirectories then From 7bbf142c7d358464825bdcfbd89d635063cda2a0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lars=20G=C3=B6ttgens?= Date: Fri, 14 Feb 2025 10:15:26 +0100 Subject: [PATCH 3/8] Update src/sysroots.c Co-authored-by: Max Horn --- src/sysroots.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/sysroots.c b/src/sysroots.c index 29784f5310..acef6363ad 100644 --- a/src/sysroots.c +++ b/src/sysroots.c @@ -208,7 +208,7 @@ void SySetGapRootPath(const Char * string) } // convert all paths to absolute paths - Char tempstr[GAP_PATH_MAX]; + char tempstr[PATH_MAX]; if (NULL == realpath(SyGapRootPaths[i], tempstr)) { SySetErrorNo(); From aebcd2ad4a3b07c749d47928d44ee4bc77432e7f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lars=20G=C3=B6ttgens?= Date: Fri, 18 Sep 2026 15:52:50 +0200 Subject: [PATCH 4/8] kernel: add SyRealpath helper Wrap realpath, or _fullpath on native Windows, in one place and use it for GAP_realpath and the root paths. Assisted-by: Claude Code (Fable 5.1) --- src/streams.c | 6 +----- src/sysfiles.c | 14 ++++++++++++++ src/sysfiles.h | 12 ++++++++++++ src/sysroots.c | 5 ++--- 4 files changed, 29 insertions(+), 8 deletions(-) diff --git a/src/streams.c b/src/streams.c index db5a23a2ac..48e2f5e8dc 100644 --- a/src/streams.c +++ b/src/streams.c @@ -1163,11 +1163,7 @@ static Obj FuncGAP_realpath(Obj self, Obj path) RequireStringRep(SELF_NAME, path); char resolved_path[GAP_PATH_MAX]; -#ifdef SYS_IS_MINGW - if (NULL == _fullpath(resolved_path, CONST_CSTR_STRING(path), sizeof(resolved_path))) { -#else - if (NULL == realpath(CONST_CSTR_STRING(path), resolved_path)) { -#endif + if (NULL == SyRealpath(CONST_CSTR_STRING(path), resolved_path)) { SySetErrorNo(); return Fail; } diff --git a/src/sysfiles.c b/src/sysfiles.c index b4c0221de7..1187e4b0ff 100644 --- a/src/sysfiles.c +++ b/src/sysfiles.c @@ -2912,6 +2912,20 @@ Int SyIsExistingFile ( const Char * name ) return res; } +/**************************************************************************** +** +*F SyRealpath( , ) . . . . . . . . . absolute canonical path +*/ +Char * SyRealpath(const Char * path, Char * buf) +{ +#ifdef SYS_IS_MINGW + return _fullpath(buf, path, GAP_PATH_MAX); +#else + return realpath(path, buf); +#endif +} + + /**************************************************************************** ** *F SyIsReadableFile( ) . . . . . . . . . . . is file readable diff --git a/src/sysfiles.h b/src/sysfiles.h index bca4cba689..3913a106da 100644 --- a/src/sysfiles.h +++ b/src/sysfiles.h @@ -350,6 +350,18 @@ void SySetErrorNo(void); Int SyIsExistingFile(const Char * name); +/**************************************************************************** +** +*F SyRealpath( , ) . . . . . . . . . absolute canonical path +** +** 'SyRealpath' stores the absolute path of with all symlinks +** resolved in , which must have room for 'GAP_PATH_MAX' characters, +** and returns . On failure, e.g. if does not exist, it returns +** NULL and sets 'errno'. +*/ +Char * SyRealpath(const Char * path, Char * buf); + + /**************************************************************************** ** *F SyIsReadableFile( ) . . . . . . . . . . . is file readable diff --git a/src/sysroots.c b/src/sysroots.c index 92a8b3cd95..d832ca3738 100644 --- a/src/sysroots.c +++ b/src/sysroots.c @@ -18,7 +18,6 @@ #include "sysstr.h" #include "system.h" -#include #include @@ -218,9 +217,9 @@ void SySetGapRootPath(const Char * string) } // convert all paths to absolute paths - char tempstr[PATH_MAX]; + char tempstr[GAP_PATH_MAX]; - if (NULL == realpath(SyGapRootPaths[i], tempstr)) { + if (NULL == SyRealpath(SyGapRootPaths[i], tempstr)) { SySetErrorNo(); } else { strxcpy(SyGapRootPaths[i], tempstr, sizeof(SyGapRootPaths[i])); From 0278cb9272420e519f4bd1fdc0d493796e5ec98a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lars=20G=C3=B6ttgens?= Date: Fri, 18 Sep 2026 15:59:42 +0200 Subject: [PATCH 5/8] kernel: make root paths absolute even without HOME The conversion sat inside the tilde-expansion loop, which returned early when HOME was unset or empty, leaving relative root paths as given. Also append the trailing slash with strxcat so a truncated path cannot overrun its buffer. Assisted-by: Claude Code (Fable 5.1) --- src/sysroots.c | 42 +++++++++++++++++++----------------------- 1 file changed, 19 insertions(+), 23 deletions(-) diff --git a/src/sysroots.c b/src/sysroots.c index d832ca3738..39af197ab1 100644 --- a/src/sysroots.c +++ b/src/sysroots.c @@ -203,31 +203,27 @@ void SySetGapRootPath(const Char * string) // TODO; instead of iterating over all entries each time, just // do this for the new entries char * userhome = getenv("HOME"); - if (!userhome || !*userhome) - return; - const UInt userhomelen = strlen(userhome); - for (i = 0; i < MAX_GAP_DIRS && SyGapRootPaths[i][0]; i++) { - UInt pathlen = strlen(SyGapRootPaths[i]); - if (SyGapRootPaths[i][0] == '~' && - userhomelen + pathlen < sizeof(SyGapRootPaths[i])) { - SyMemmove(SyGapRootPaths[i] + userhomelen, - // don't copy the ~ but the trailing '\0' - SyGapRootPaths[i] + 1, pathlen); - memcpy(SyGapRootPaths[i], userhome, userhomelen); + if (userhome && *userhome) { + const UInt userhomelen = strlen(userhome); + for (i = 0; i < MAX_GAP_DIRS && SyGapRootPaths[i][0]; i++) { + const UInt pathlen = strlen(SyGapRootPaths[i]); + if (SyGapRootPaths[i][0] == '~' && + userhomelen + pathlen < sizeof(SyGapRootPaths[i])) { + SyMemmove(SyGapRootPaths[i] + userhomelen, + // don't copy the ~ but the trailing '\0' + SyGapRootPaths[i] + 1, pathlen); + memcpy(SyGapRootPaths[i], userhome, userhomelen); + } } + } - // convert all paths to absolute paths - char tempstr[GAP_PATH_MAX]; - - if (NULL == SyRealpath(SyGapRootPaths[i], tempstr)) { - SySetErrorNo(); - } else { - strxcpy(SyGapRootPaths[i], tempstr, sizeof(SyGapRootPaths[i])); - pathlen = strlen(SyGapRootPaths[i]); - if (SyGapRootPaths[i][pathlen - 1] != '/') { - SyGapRootPaths[i][pathlen] = '/'; - SyGapRootPaths[i][pathlen + 1] = '\0'; - } + // make all paths absolute; paths that do not exist are left as is + for (i = 0; i < MAX_GAP_DIRS && SyGapRootPaths[i][0]; i++) { + char buf[GAP_PATH_MAX]; + if (SyRealpath(SyGapRootPaths[i], buf)) { + strxcpy(SyGapRootPaths[i], buf, sizeof(SyGapRootPaths[i])); + if (SyGapRootPaths[i][strlen(SyGapRootPaths[i]) - 1] != '/') + strxcat(SyGapRootPaths[i], "/", sizeof(SyGapRootPaths[i])); } } } From a8a15436e71b7314d2914194dbd4d900474befc9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lars=20G=C3=B6ttgens?= Date: Fri, 18 Sep 2026 16:09:35 +0200 Subject: [PATCH 6/8] lib: dedupe root paths after adding the trailing slash GAP_realpath strips trailing slashes while every entry of GAPInfo.RootPaths ends in one, so the duplicate check in ExtendRootDirectories never matched. Assisted-by: Claude Code (Fable 5.1) --- lib/package.gi | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/lib/package.gi b/lib/package.gi index 6f1a096f3c..bfa78e7498 100644 --- a/lib/package.gi +++ b/lib/package.gi @@ -1859,14 +1859,14 @@ InstallGlobalFunction( ExtendRootDirectories, function( rootpaths ) local i; rootpaths:= List( rootpaths, GAP_realpath ); + # 'DirectoriesLibrary' concatenates root paths with directory names. + for i in [ 1 .. Length( rootpaths ) ] do + if not EndsWith( rootpaths[i], "/" ) then + rootpaths[i]:= Concatenation( rootpaths[i], "/" ); + fi; + od; rootpaths:= Filtered( rootpaths, path -> not path in GAPInfo.RootPaths ); if not IsEmpty( rootpaths ) then - # 'DirectoriesLibrary' concatenates root paths with directory names. - for i in [ 1 .. Length( rootpaths ) ] do - if not EndsWith( rootpaths[i], "/" ) then - rootpaths[i]:= Concatenation( rootpaths[i], "/" ); - fi; - od; # Append the new root paths. GAPInfo.RootPaths:= Immutable( Concatenation( GAPInfo.RootPaths, rootpaths ) ); From 14ef85329f8570fdadd8faeb6b2a574b56b246d7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lars=20G=C3=B6ttgens?= Date: Fri, 18 Sep 2026 16:11:34 +0200 Subject: [PATCH 7/8] lib: use Filename to read a Directory's path Avoid poking at the internal representation in ExtendPackageDirectories. Assisted-by: Claude Code (Fable 5.1) --- lib/package.gi | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/lib/package.gi b/lib/package.gi index bfa78e7498..5ed19406dd 100644 --- a/lib/package.gi +++ b/lib/package.gi @@ -1890,13 +1890,12 @@ InstallGlobalFunction( ExtendPackageDirectories, function( paths_or_dirs ) local p, changed; changed:= false; for p in paths_or_dirs do - if IsString( p ) then - p:= Directory( GAP_realpath ( p ) ); - elif IsDirectory( p ) then - p:= Directory( GAP_realpath ( p![1] ) ); - else + if IsDirectory( p ) then + p:= Filename( p, "" ); + elif not IsString( p ) then Error("input must be a list of path strings or directory objects"); fi; + p:= Directory( GAP_realpath( p ) ); if not p in GAPInfo.PackageDirectories then Add( GAPInfo.PackageDirectories, p ); changed:= true; From 8e498bf16ef1a6cb35b6d6417f3a5982a8fcbaca Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lars=20G=C3=B6ttgens?= Date: Fri, 18 Sep 2026 16:15:02 +0200 Subject: [PATCH 8/8] tst: check that equivalent paths are not added twice Assisted-by: Claude Code (Fable 5.1) --- tst/testinstall/package.tst | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/tst/testinstall/package.tst b/tst/testinstall/package.tst index a2577c2519..10a1194161 100644 --- a/tst/testinstall/package.tst +++ b/tst/testinstall/package.tst @@ -690,6 +690,18 @@ gap> Last( GAPInfo.PackagesInfo.mockpkg ).InstallationPath = > GAPInfo.PackagesLoaded.mockpkg[1]; true +# paths are made absolute, so equivalent paths are not added again +gap> n:= Length( GAPInfo.PackageDirectories );; +gap> ExtendPackageDirectories( [ Concatenation( Filename( mockpkgpath, "" ), +> "../mockpkg" ) ] ); +gap> Length( GAPInfo.PackageDirectories ) = n; +true +gap> n:= Length( GAPInfo.RootPaths );; +gap> ExtendRootDirectories( List( Filtered( GAPInfo.RootPaths, IsDirectoryPath ), +> path -> Concatenation( path, "./" ) ) ); +gap> Length( GAPInfo.RootPaths ) = n; +true + # gap> SetPackagePath( "mockpkg", Filename( mockpkgpath, "" ) ); gap> SetPackagePath( "mockpkg", "/some/other/directory" );