Address review comments

Author: maxbrunet

internal/ebutil/libs.go

@@ -7,17 +7,25 @@ import (
 	"path/filepath"
 )
 
-// Based on https://github.com/NVIDIA/libnvidia-container/blob/v1.15.0/src/common.h#L29
+// Container runtimes like NVIDIA mount individual libraries into the container
+// (e.g. `<libname>.so.<driver_version>`) and create symlinks for them
+// (e.g. `<libname>.so.1`). This code helps with finding the right library
+// directory for the target Linux distribution as well as locating the symlinks.
+//
+// Please see [#143 (comment)] for further details.
+//
+// [#143 (comment)]: https://github.com/coder/envbuilder/issues/143#issuecomment-2192405828
 
+// Based on https://github.com/NVIDIA/libnvidia-container/blob/v1.15.0/src/common.h#L29
 const usrLibDir = "/usr/lib64"
 
 const debianVersionFile = "/etc/debian_version"
 
-// getLibDir returns the library directory. It returns a multiarch directory if
-// the distribution is Debian or a derivative.
+// libraryDirectoryPath returns the library directory. It returns a multiarch
+// directory if the distribution is Debian or a derivative.
 //
 // Based on https://github.com/NVIDIA/libnvidia-container/blob/v1.15.0/src/nvc_container.c#L152-L165
-func getLibDir(m mounter) (string, error) {
+func libraryDirectoryPath(m mounter) (string, error) {
 	// Debian and its derivatives use a multiarch directory scheme.
 	if _, err := m.Stat(debianVersionFile); err != nil && !errors.Is(err, os.ErrNotExist) {
 		return "", fmt.Errorf("check if debian: %w", err)
@@ -28,9 +36,10 @@ func getLibDir(m mounter) (string, error) {
 	return usrLibDir, nil
 }
 
-// getLibsSymlinks returns the stats for all library symlinks if the library
-// directory exists.
-func getLibsSymlinks(m mounter, libDir string) (map[string][]string, error) {
+// libraryDirectorySymlinks returns a mapping of each library (basename) with a
+// list of their symlinks (basename). Libraries with no symlinks do not appear
+// in the mapping.
+func libraryDirectorySymlinks(m mounter, libDir string) (map[string][]string, error) {
 	des, err := m.ReadDir(libDir)
 	if err != nil {
 		return nil, fmt.Errorf("read directory %s: %w", libDir, err)
@@ -54,7 +63,6 @@ func getLibsSymlinks(m mounter, libDir string) (map[string][]string, error) {
 		}
 
 		path = filepath.Base(path)
-
 		if _, ok := libsSymlinks[path]; !ok {
 			libsSymlinks[path] = make([]string, 0, 1)
 		}

internal/ebutil/libs_ppc64le.go

@@ -45,12 +45,12 @@ func tempRemount(m mounter, logf func(notcodersdk.LogLevel, string, ...any), bas
 		return func() error { return nil }, fmt.Errorf("get mounts: %w", err)
 	}
 
-	libDir, err := getLibDir(m)
+	libDir, err := libraryDirectoryPath(m)
 	if err != nil {
 		return func() error { return nil }, fmt.Errorf("get lib directory: %w", err)
 	}
 
-	libsSymlinks, err := getLibsSymlinks(m, libDir)
+	libsSymlinks, err := libraryDirectorySymlinks(m, libDir)
 	if err != nil && !errors.Is(err, os.ErrNotExist) {
 		return func() error { return nil }, fmt.Errorf("read lib symlinks: %w", err)
 	}
@@ -66,7 +66,7 @@ func tempRemount(m mounter, logf func(notcodersdk.LogLevel, string, ...any), bas
 				return
 			}
 
-			newLibDir, err := getLibDir(m)
+			newLibDir, err := libraryDirectoryPath(m)
 			if err != nil {
 				merr = multierror.Append(merr, fmt.Errorf("get new lib directory: %w", err))
 				return
@@ -137,6 +137,7 @@ func remount(m mounter, src, dest, libDir string, libsSymlinks map[string][]stri
 		if err != nil {
 			return fmt.Errorf("ensure file path: %w", err)
 		}
+		// This ensure the file is created, it will not be used. It can be closed immediately.
 		f.Close()
 
 		if symlinks, ok := libsSymlinks[stat.Name()]; ok {
@@ -154,7 +155,6 @@ func remount(m mounter, src, dest, libDir string, libsSymlinks map[string][]stri
 	if err := m.Unmount(src, 0); err != nil {
 		return fmt.Errorf("unmount orig src %s: %w", src, err)
 	}
-
 	return nil
 }
 

internal/ebutil/remount.go

@@ -17,9 +17,8 @@ import (
 )
 
 var expectedLibMultiarchDir = map[string]string{
-	"amd64":   "/usr/lib/x86_64-linux-gnu",
-	"arm64":   "/usr/lib/aarch64-linux-gnu",
-	"ppc64le": "/usr/lib/powerpc64le-linux-gnu",
+	"amd64": "/usr/lib/x86_64-linux-gnu",
+	"arm64": "/usr/lib/aarch64-linux-gnu",
 }
 
 func Test_tempRemount(t *testing.T) {