Switch to using existing version detection code

This also tweaks the existing version detection code to make sure
it is only run once by the binary: seems safe to assume XRootD versions
won't change.

Author: bbockelm

xrootd/plugin_check.go

@@ -23,20 +23,15 @@ import (
 	"os"
 	"os/exec"
 	"path/filepath"
-	"regexp"
 	"runtime"
 	"sort"
 	"strings"
 	"sync"
 
 	"github.com/pkg/errors"
-	log "github.com/sirupsen/logrus"
 )
 
 var (
-	xrootdVersionOnce sync.Once
-	xrootdVersion     string
-
 	pluginSearchPathsOnce sync.Once
 	pluginSearchPaths     []string
 )
@@ -326,30 +321,6 @@ func parsePluginConfigFile(filePath string) (string, bool) {
 	return libPath, enabled
 }
 
-// getXRootDVersion runs 'xrootd -v' and extracts the major version number
-func getXRootDVersion() string {
-	xrootdVersionOnce.Do(func() {
-		cmd := exec.Command("xrootd", "-v")
-		output, err := cmd.CombinedOutput()
-		if err != nil {
-			log.Debugf("Failed to get xrootd version: %v", err)
-			xrootdVersion = ""
-			return
-		}
-
-		// Parse version from output (e.g., "XRootD v5.7.1" or "v6.0.0")
-		re := regexp.MustCompile(`v(\d+)\.\d+\.\d+`)
-		matches := re.FindStringSubmatch(string(output))
-		if len(matches) > 1 {
-			xrootdVersion = matches[1] // Return major version like "5" or "6"
-		} else {
-			xrootdVersion = ""
-		}
-	})
-
-	return xrootdVersion
-}
-
 // getPluginVariants returns a list of possible plugin filenames for a given base name.
 // XRootD automatically adds the major version to plugin names, so libXrdHttpPelican.so
 // may be libXrdHttpPelican-5.so or libXrdHttpPelican-6.so on disk.
@@ -374,7 +345,7 @@ func getPluginVariants(baseName string) []string {
 		variants = append(variants, nameWithoutExt+ext)
 
 		// Try to get the actual XRootD version
-		version := getXRootDVersion()
+		version := GetXrootdMajorVersion()
 		if version != "" {
 			// Use the detected version
 			variants = append(variants, nameWithoutExt+"-"+version+ext)

xrootd/plugin_check_test.go

@@ -448,13 +448,3 @@ func TestGetXRootDRPaths(t *testing.T) {
 		assert.True(t, filepath.IsAbs(path), "All paths should be absolute")
 	}
 }
-
-func TestGetXRootDVersion(t *testing.T) {
-	// This test checks that getXRootDVersion doesn't panic
-	// It may return empty if xrootd is not in PATH, which is okay
-	version := getXRootDVersion()
-	if version != "" {
-		// If we got a version, it should be a number
-		assert.Regexp(t, `^\d+$`, version, "Version should be a single digit")
-	}
-}

xrootd/version.go

@@ -22,7 +22,9 @@ package xrootd
 
 import (
 	"os/exec"
+	"regexp"
 	"strings"
+	"sync"
 
 	"github.com/hashicorp/go-version"
 	"github.com/pkg/errors"
@@ -33,19 +35,65 @@ import (
 // This version must be kept in sync with the version specified in .goreleaser.yml (two locations: RPM and DEB dependencies).
 const MinXrootdVersion = "5.8.2"
 
+var (
+	xrootdVersionOnce   sync.Once
+	xrootdVersionOutput string
+	xrootdVersionErr    error
+)
+
+// ResetXrootdVersionForTesting resets the cached XRootD version output.
+// This should only be used in tests.
+func ResetXrootdVersionForTesting() {
+	xrootdVersionOnce = sync.Once{}
+	xrootdVersionOutput = ""
+	xrootdVersionErr = nil
+}
+
+// getXrootdVersionOutput runs 'xrootd -v' once and caches the result.
+// Subsequent calls return the cached output without re-executing the command.
+func getXrootdVersionOutput() (string, error) {
+	xrootdVersionOnce.Do(func() {
+		// Execute xrootd -v to get version information
+		// Note: xrootd outputs version to stderr, not stdout
+		cmd := exec.Command("xrootd", "-v")
+		output, err := cmd.CombinedOutput()
+		if err != nil {
+			xrootdVersionErr = err
+			return
+		}
+		xrootdVersionOutput = strings.TrimSpace(string(output))
+	})
+	return xrootdVersionOutput, xrootdVersionErr
+}
+
+// GetXrootdMajorVersion returns the major version number of the installed XRootD
+// (e.g., "5" or "6"). Returns an empty string if the version cannot be determined.
+// The result is cached after the first call.
+func GetXrootdMajorVersion() string {
+	output, err := getXrootdVersionOutput()
+	if err != nil || output == "" {
+		return ""
+	}
+
+	re := regexp.MustCompile(`v?(\d+)\.\d+\.\d+`)
+	matches := re.FindStringSubmatch(output)
+	if len(matches) > 1 {
+		return matches[1]
+	}
+	return ""
+}
+
 /*
 * CheckXrootdVersion checks if the installed XRootD version meets the minimum requirement.
 * It executes 'xrootd -v' to retrieve the version and compares it against MinXrootdVersion.
+* The xrootd binary is only invoked once; subsequent calls use the cached result.
 * Returns an error if:
 *   - The xrootd binary is not found in PATH
 *   - The version cannot be determined
 *   - The version is below the minimum requirement
  */
 func CheckXrootdVersion() error {
-	// Execute xrootd -v to get version information
-	// Note: xrootd outputs version to stderr, not stdout
-	cmd := exec.Command("xrootd", "-v")
-	output, err := cmd.CombinedOutput()
+	output, err := getXrootdVersionOutput()
 	if err != nil {
 		// Check if this is a "command not found" error
 		if exitErr, ok := err.(*exec.ExitError); ok {
@@ -60,20 +108,19 @@ func CheckXrootdVersion() error {
 	}
 
 	// Parse the version output
-	versionStr := strings.TrimSpace(string(output))
-	log.Debugf("XRootD version output: %s", versionStr)
+	log.Debugf("XRootD version output: %s", output)
 
 	// Remove leading 'v' if present (e.g., "v5.8.2" -> "5.8.2")
-	versionStr = strings.TrimPrefix(versionStr, "v")
+	versionStr := strings.TrimPrefix(output, "v")
 
 	// Parse the version string
-	xrootdVersion, err := version.NewVersion(versionStr)
+	xrootdVer, err := version.NewVersion(versionStr)
 	if err != nil {
 		return errors.Wrapf(err, "failed to parse XRootD version string '%s'. Please ensure XRootD is properly installed", versionStr)
 	}
 
 	// Normalize to core version (strips pre-release suffixes like -rc1, +git123)
-	xrootdVersion = xrootdVersion.Core()
+	xrootdVer = xrootdVer.Core()
 
 	// Parse the minimum required version
 	requiredVersion, err := version.NewVersion(MinXrootdVersion)
@@ -83,13 +130,13 @@ func CheckXrootdVersion() error {
 	}
 
 	// Compare versions
-	if xrootdVersion.LessThan(requiredVersion) {
+	if xrootdVer.LessThan(requiredVersion) {
 		return errors.Errorf("XRootD version %s is insufficient (minimum required: %s). "+
 			"Please upgrade XRootD to version %s or later. "+
 			"This requirement is necessary for proper operation of Pelican Cache and Origin servers.",
-			xrootdVersion.String(), MinXrootdVersion, MinXrootdVersion)
+			xrootdVer.String(), MinXrootdVersion, MinXrootdVersion)
 	}
 
-	log.Debugf("XRootD version check passed: %s >= %s", xrootdVersion.String(), MinXrootdVersion)
+	log.Debugf("XRootD version check passed: %s >= %s", xrootdVer.String(), MinXrootdVersion)
 	return nil
 }

xrootd/version_test.go

@@ -116,6 +116,9 @@ func TestCheckXrootdVersion(t *testing.T) {
 
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
+			// Reset cached version so each subtest gets a fresh detection
+			ResetXrootdVersionForTesting()
+
 			// Create a mock xrootd binary
 			tmpDir := createMockXrootd(t, tt.version, tt.exitCode)
 
@@ -146,6 +149,9 @@ func TestCheckXrootdVersion(t *testing.T) {
 
 // TestCheckXrootdVersion_BinaryNotFound tests the scenario where xrootd is not installed
 func TestCheckXrootdVersion_BinaryNotFound(t *testing.T) {
+	// Reset cached version for this test
+	ResetXrootdVersionForTesting()
+
 	// Create an empty temporary directory
 	tmpDir := t.TempDir()
 
@@ -187,6 +193,9 @@ func TestCheckXrootdVersion_InvalidVersionFormat(t *testing.T) {
 
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
+			// Reset cached version so each subtest gets a fresh detection
+			ResetXrootdVersionForTesting()
+
 			// Create a mock xrootd binary
 			tmpDir := createMockXrootd(t, tt.version, 0)
 
@@ -211,6 +220,9 @@ func TestCheckXrootdVersion_InvalidVersionFormat(t *testing.T) {
 
 // TestCheckXrootdVersion_RealBinary tests with the actual xrootd binary if available
 func TestCheckXrootdVersion_RealBinary(t *testing.T) {
+	// Reset cached version for this test
+	ResetXrootdVersionForTesting()
+
 	// Check if xrootd is actually available
 	_, err := exec.LookPath("xrootd")
 	if err != nil {

xrootd/version_windows.go

@@ -20,6 +20,12 @@
 
 package xrootd
 
+// ResetXrootdVersionForTesting is a no-op on Windows since XRootD is not supported on this platform.
+func ResetXrootdVersionForTesting() {}
+
+// GetXrootdMajorVersion is a no-op on Windows since XRootD is not supported on this platform.
+func GetXrootdMajorVersion() string { return "" }
+
 // CheckXrootdVersion is a no-op on Windows since XRootD is not supported on this platform.
 // Cache and origin servers are not available on Windows.
 func CheckXrootdVersion() error {