Merge pull request apptainer#2507 from DrDaveD/finish-sylabs2026

chore: fix revive lint for golangci-lint 1.54.0, from sylabs2026

cmd/internal/cli/key_pull.go

@@ -62,7 +62,7 @@ func doKeyPullCmd(ctx context.Context, fingerprint string, co ...client.Option)
 	keyring := sypgp.NewHandle(path, opts...)
 
 	// get matching keyring
-	el, err := sypgp.FetchPubkey(ctx, fingerprint, false, co...)
+	el, err := sypgp.FetchPubkey(ctx, fingerprint, co...)
 	if err != nil {
 		return fmt.Errorf("unable to pull key from server: %v", err)
 	}

cmd/internal/cli/pull.go

@@ -259,7 +259,7 @@ func pullRun(cmd *cobra.Command, args []string) {
 			sylog.Warningf("Skipping container verification")
 		}
 	case ShubProtocol:
-		_, err := shub.PullToFile(ctx, imgCache, pullTo, pullFrom, tmpDir, noHTTPS)
+		_, err := shub.PullToFile(ctx, imgCache, pullTo, pullFrom, noHTTPS)
 		if err != nil {
 			sylog.Fatalf("While pulling shub image: %v\n", err)
 		}
@@ -269,12 +269,12 @@ func pullRun(cmd *cobra.Command, args []string) {
 			sylog.Fatalf("Unable to make docker oci credentials: %s", err)
 		}
 
-		_, err = oras.PullToFile(ctx, imgCache, pullTo, pullFrom, tmpDir, ociAuth, noHTTPS, reqAuthFile)
+		_, err = oras.PullToFile(ctx, imgCache, pullTo, pullFrom, ociAuth, noHTTPS, reqAuthFile)
 		if err != nil {
 			sylog.Fatalf("While pulling image from oci registry: %v", err)
 		}
 	case HTTPProtocol, HTTPSProtocol:
-		_, err := net.PullToFile(ctx, imgCache, pullTo, pullFrom, tmpDir)
+		_, err := net.PullToFile(ctx, imgCache, pullTo, pullFrom)
 		if err != nil {
 			sylog.Fatalf("While pulling from image from http(s): %v\n", err)
 		}

internal/app/apptainer/keyserver_list.go

@@ -22,6 +22,7 @@ import (
 )
 
 // KeyserverList prints information about remote configurations
+// FIXME - remoteName is not being honored
 func KeyserverList(remoteName string, usrConfigFile string) (err error) {
 	c := &remote.Config{}
 

internal/pkg/build/apps/apps.go

@@ -397,7 +397,7 @@ func copyFiles(b *types.Bundle, a *App) error {
 			dst = splitLine[1]
 		}
 
-		if err := copyFile(src, filepath.Join(appBase, dst)); err != nil {
+		if err := copyWithfLr(src, filepath.Join(appBase, dst)); err != nil {
 			return err
 		}
 	}
@@ -457,17 +457,18 @@ func appData(b *types.Bundle, a *App) string {
 	return filepath.Join(b.RootfsPath, "/scif/data/", a.Name)
 }
 
-func copyFile(src, dst string) error {
+// FIXME: Replace with Go, or existing util function?
+func copyWithfLr(src, dst string) error {
 	cp, err := bin.FindBin("cp")
 	if err != nil {
 		return err
 	}
 
 	var stderr bytes.Buffer
-	copyCommand := exec.Command(cp, "-fLr", src, dst)
-	copyCommand.Stderr = &stderr
+	copyCmd := exec.Command(cp, "-fLr", src, dst)
+	copyCmd.Stderr = &stderr
 	sylog.Debugf("Copying %v to %v", src, dst)
-	if err := copyCommand.Run(); err != nil {
+	if err := copyCmd.Run(); err != nil {
 		return fmt.Errorf("while copying %v to %v: %v: %v", src, dst, err, stderr.String())
 	}
 

internal/pkg/build/sources/conveyorPacker_arch.go

@@ -45,7 +45,7 @@ type ArchConveyorPacker struct {
 
 // prepareFakerootEnv prepares a build environment to
 // make fakeroot working with pacstrap.
-func (cp *ArchConveyorPacker) prepareFakerootEnv(_ context.Context) (func(), error) {
+func (cp *ArchConveyorPacker) prepareFakerootEnv() (func(), error) {
 	truePath, err := bin.FindBin("true")
 	if err != nil {
 		return nil, fmt.Errorf("while searching true command: %s", err)
@@ -113,7 +113,9 @@ func (cp *ArchConveyorPacker) prepareFakerootEnv(_ context.Context) (func(), err
 }
 
 // Get just stores the source
-func (cp *ArchConveyorPacker) Get(ctx context.Context, b *types.Bundle) (err error) {
+//
+// FIXME: use context for cancellation.
+func (cp *ArchConveyorPacker) Get(_ context.Context, b *types.Bundle) (err error) {
 	cp.b = b
 
 	err = cp.getBootstrapOptions()
@@ -139,7 +141,7 @@ func (cp *ArchConveyorPacker) Get(ctx context.Context, b *types.Bundle) (err err
 
 	insideUserNs, setgroupsAllowed := namespaces.IsInsideUserNamespace(os.Getpid())
 	if insideUserNs && setgroupsAllowed {
-		umountFn, err := cp.prepareFakerootEnv(ctx)
+		umountFn, err := cp.prepareFakerootEnv()
 		if umountFn != nil {
 			defer umountFn()
 		}

internal/pkg/build/sources/conveyorPacker_busybox.go

@@ -34,6 +34,8 @@ type BusyBoxConveyorPacker struct {
 }
 
 // Get just stores the source
+//
+// FIXME: use context for cancellation.
 func (c *BusyBoxConveyor) Get(_ context.Context, b *types.Bundle) (err error) {
 	c.b = b
 

internal/pkg/build/sources/conveyorPacker_yum.go

@@ -48,6 +48,8 @@ type YumConveyorPacker struct {
 }
 
 // Get downloads container information from the specified source
+//
+// FIXME: use context for cancellation.
 func (c *YumConveyor) Get(_ context.Context, b *types.Bundle) (err error) {
 	c.b = b
 

internal/pkg/build/sources/conveyorPacker_zypper.go

@@ -60,6 +60,8 @@ func machine() (string, error) {
 
 // Get downloads container information from the specified source
 //
+// FIXME: use context for cancellation.
+//
 //nolint:maintidx
 func (cp *ZypperConveyorPacker) Get(_ context.Context, b *types.Bundle) error {
 	var suseconnectProduct, suseconnectModver string

internal/pkg/client/library/pull.go

@@ -100,10 +100,7 @@ func downloadWrapper(ctx context.Context, c *libClient.Client, imagePath, arch s
 		}
 	}(time.Now())
 
-	if err := DownloadImage(ctx, c, imagePath, arch, libraryRef, pb); err != nil {
-		return err
-	}
-	return nil
+	return DownloadImage(ctx, c, imagePath, arch, libraryRef, pb)
 }
 
 // Pull will pull a library image to the cache or direct to a temporary file if cache is disabled

internal/pkg/client/net/pull.go

@@ -193,7 +193,7 @@ func Pull(ctx context.Context, imgCache *cache.Handle, pullFrom string, tmpDir s
 }
 
 // PullToFile will pull an http(s) image to the specified location, through the cache, or directly if cache is disabled
-func PullToFile(ctx context.Context, imgCache *cache.Handle, pullTo, pullFrom, _ string) (imagePath string, err error) {
+func PullToFile(ctx context.Context, imgCache *cache.Handle, pullTo, pullFrom string) (imagePath string, err error) {
 	directTo := ""
 	if imgCache.IsDisabled() {
 		directTo = pullTo

internal/pkg/client/oras/oras.go

@@ -34,6 +34,8 @@ import (
 )
 
 // DownloadImage downloads a SIF image specified by an oci reference to a file using the included credentials
+//
+// FIXME: use context for cancellation.
 func DownloadImage(ctx context.Context, path, ref string, ociAuth *authn.AuthConfig, noHTTPS bool, reqAuthFile string) error {
 	rt := client.NewRoundTripper(ctx, nil)
 	im, err := remoteImage(ref, ociAuth, noHTTPS, rt, reqAuthFile)
@@ -115,6 +117,8 @@ func DownloadImage(ctx context.Context, path, ref string, ociAuth *authn.AuthCon
 
 // UploadImage uploads the image specified by path and pushes it to the provided oci reference,
 // it will use credentials if supplied
+//
+// FIXME: use context for cancellation.
 func UploadImage(_ context.Context, path, ref string, ociAuth *authn.AuthConfig, noHTTPS bool, reqAuthFile string) error {
 	// ensure that are uploading a SIF
 	if err := ensureSIF(path); err != nil {
@@ -187,6 +191,8 @@ func ensureSIF(filepath string) error {
 }
 
 // RefHash returns the digest of the SIF layer of the OCI manifest for supplied ref
+//
+// FIXME: use context for cancellation.
 func RefHash(_ context.Context, ref string, ociAuth *authn.AuthConfig, noHTTPS bool, reqAuthFile string) (v1.Hash, error) {
 	im, err := remoteImage(ref, ociAuth, noHTTPS, nil, reqAuthFile)
 	if err != nil {

internal/pkg/client/oras/pull.go

@@ -83,7 +83,7 @@ func Pull(ctx context.Context, imgCache *cache.Handle, pullFrom, tmpDir string,
 }
 
 // PullToFile will pull an oras image to the specified location, through the cache, or directly if cache is disabled
-func PullToFile(ctx context.Context, imgCache *cache.Handle, pullTo, pullFrom, _ string, ociAuth *authn.AuthConfig, noHTTPS bool, reqAuthFile string) (imagePath string, err error) {
+func PullToFile(ctx context.Context, imgCache *cache.Handle, pullTo, pullFrom string, ociAuth *authn.AuthConfig, noHTTPS bool, reqAuthFile string) (imagePath string, err error) {
 	directTo := ""
 	if imgCache.IsDisabled() {
 		directTo = pullTo

internal/pkg/client/shub/pull.go

@@ -196,7 +196,7 @@ func Pull(ctx context.Context, imgCache *cache.Handle, pullFrom, tmpDir string,
 }
 
 // PullToFile will pull a shub image to the specified location, through the cache, or directly if cache is disabled
-func PullToFile(ctx context.Context, imgCache *cache.Handle, pullTo, pullFrom, _ string, noHTTPS bool) (imagePath string, err error) {
+func PullToFile(ctx context.Context, imgCache *cache.Handle, pullTo, pullFrom string, noHTTPS bool) (imagePath string, err error) {
 	directTo := ""
 	if imgCache.IsDisabled() {
 		directTo = pullTo

internal/pkg/image/packer/gocryptfs.go

@@ -195,8 +195,5 @@ func cmdRun(commands []string, stdout, stderr io.Writer) error {
 	if stderr != nil {
 		cmd.Stderr = stderr
 	}
-	if err := cmd.Run(); err != nil {
-		return err
-	}
-	return nil
+	return cmd.Run()
 }

internal/pkg/image/unpacker/squashfs_test.go

@@ -53,14 +53,18 @@ func TestSquashfs(t *testing.T) {
 	})
 }
 
-func testSquashfs(t *testing.T, _ string) {
+func testSquashfs(t *testing.T, tmpParent string) {
 	s := NewSquashfs()
 
 	if !s.HasUnsquashfs() {
 		t.Skip("unsquashfs not found")
 	}
 
-	dir := t.TempDir()
+	dir, err := os.MkdirTemp(tmpParent, "test-squashfs-")
+	if err != nil {
+		t.Fatalf("while creating tmpdir: %v", err)
+	}
+	defer os.RemoveAll(dir)
 
 	// create archive with files present in this directory
 	archive := createArchive(t)

internal/pkg/remote/credential/login_handler.go

@@ -172,7 +172,6 @@ func (h *keyserverHandler) login(u *url.URL, username, password string, insecure
 	}, nil
 }
 
-//nolint:revive,nolintlint
-func (h *keyserverHandler) logout(u *url.URL, reqAuthFile string) error {
+func (h *keyserverHandler) logout(_ *url.URL, _ string) error {
 	return nil
 }

internal/pkg/remote/remote_test.go

@@ -93,13 +93,13 @@ func TestWriteToReadFrom(t *testing.T) {
 
 			test.c.WriteTo(&r)
 
-			item, err := ReadFrom(&r)
+			newConfig, err := ReadFrom(&r)
 			if err != nil {
 				t.Errorf("unexpected failure running %s test: %s", test.name, err)
 			}
 
-			if !reflect.DeepEqual(test.c, *item) {
-				t.Errorf("failed to read/write config:\n\thave: %v\n\twant: %v", test.c, *item)
+			if !reflect.DeepEqual(test.c, *newConfig) {
+				t.Errorf("failed to read/write config:\n\thave: %v\n\twant: %v", test.c, *newConfig)
 			}
 		})
 	}

internal/pkg/runtime/engine/apptainer/container_linux.go

@@ -1945,7 +1945,7 @@ func (c *container) addHomeLayer(system *mount.System, source, dest string) erro
 
 // addHomeNoLayer is responsible for staging the home directory and adding the base
 // directory of the staged home into the container when overlay/underlay are unavailable
-func (c *container) addHomeNoLayer(system *mount.System, _, dest string) error {
+func (c *container) addHomeNoLayer(system *mount.System, dest string) error {
 	flags := uintptr(syscall.MS_BIND | syscall.MS_REC)
 
 	homeBase := fs.RootDir(dest)
@@ -1999,7 +1999,7 @@ func (c *container) addHomeMount(system *mount.System) error {
 	sessionLayer := c.engine.EngineConfig.GetSessionLayer()
 	sylog.Debugf("Adding home directory mount [%v:%v] to list using layer: %s\n", stagingDir, dest, sessionLayer)
 	if !c.isLayerEnabled() {
-		return c.addHomeNoLayer(system, stagingDir, dest)
+		return c.addHomeNoLayer(system, dest)
 	}
 	return c.addHomeLayer(system, stagingDir, dest)
 }

internal/pkg/runtime/engine/apptainer/process_linux.go

@@ -717,7 +717,7 @@ func injectEnvHandler(senv map[string]string, noEval bool) interpreter.OpenHandl
 	}
 }
 
-func runtimeVarsHandler(_ map[string]string) interpreter.OpenHandler {
+func runtimeVarsHandler() interpreter.OpenHandler {
 	var once sync.Once
 
 	return func(_ string, _ int, _ os.FileMode) (io.ReadWriteCloser, error) {
@@ -752,7 +752,7 @@ func sylogBuiltin(_ context.Context, argv []string) error {
 }
 
 // getAllEnvBuiltin display all exported variables in the form KEY=VALUE.
-func getAllEnvBuiltin(_ *interpreter.Shell) interpreter.ShellBuiltin {
+func getAllEnvBuiltin() interpreter.ShellBuiltin {
 	return func(ctx context.Context, _ []string) error {
 		hc := interp.HandlerCtx(ctx)
 
@@ -892,10 +892,10 @@ func runActionScript(engineConfig *apptainerConfig.EngineConfig) ([]string, []st
 	senv := engineConfig.GetApptainerEnv()
 	shell.RegisterOpenHandler("/.inject-apptainer-env.sh", injectEnvHandler(senv, engineConfig.GetNoEval()))
 
-	shell.RegisterOpenHandler("/.singularity.d/env/99-runtimevars.sh", runtimeVarsHandler(senv))
+	shell.RegisterOpenHandler("/.singularity.d/env/99-runtimevars.sh", runtimeVarsHandler())
 
 	// register few builtin
-	shell.RegisterShellBuiltin("getallenv", getAllEnvBuiltin(shell))
+	shell.RegisterShellBuiltin("getallenv", getAllEnvBuiltin())
 	shell.RegisterShellBuiltin("sylog", sylogBuiltin)
 	shell.RegisterShellBuiltin("fixpath", fixPathBuiltin)
 	shell.RegisterShellBuiltin("hash", hashBuiltin)

internal/pkg/sypgp/sypgp.go

@@ -902,7 +902,7 @@ func date(s string) string {
 }
 
 // FetchPubkey pulls a public key from the Key Service.
-func FetchPubkey(ctx context.Context, fingerprint string, _ bool, opts ...client.Option) (openpgp.EntityList, error) {
+func FetchPubkey(ctx context.Context, fingerprint string, opts ...client.Option) (openpgp.EntityList, error) {
 	// Decode fingerprint and ensure proper length.
 	var fp []byte
 	fp, err := hex.DecodeString(fingerprint)

internal/pkg/sypgp/sypgp_test.go

@@ -136,7 +136,7 @@ func TestFetchPubkey(t *testing.T) {
 				client.OptHTTPClient(srv.Client()),
 			}
 
-			el, err := FetchPubkey(context.Background(), tt.fingerprint, false, opts...)
+			el, err := FetchPubkey(context.Background(), tt.fingerprint, opts...)
 			if (err != nil) != tt.wantErr {
 				t.Fatalf("unexpected error: %v", err)
 				return

pkg/util/capabilities/config_test.go

@@ -50,13 +50,13 @@ func TestReadFromWriteTo(t *testing.T) {
 
 			test.c.WriteTo(&r)
 
-			item, err := ReadFrom(&r)
+			newConfig, err := ReadFrom(&r)
 			if err != nil {
 				t.Errorf("unexpected failure running %s test: %s", test.name, err)
 			}
 
-			if !reflect.DeepEqual(test.c, *item) {
-				t.Errorf("failed to read/write config:\n\thave: %v\n\twant: %v", test.c, *item)
+			if !reflect.DeepEqual(test.c, *newConfig) {
+				t.Errorf("failed to read/write config:\n\thave: %v\n\twant: %v", test.c, *newConfig)
 			}
 		})
 	}