Merge pull request PelicanPlatform#1171 from haoming29/fix-version-out

Fix version flag output

.github/workflows/test.yml

@@ -144,3 +144,5 @@ jobs:
       run: ./github_scripts/get_put_test.sh
     - name: Run End-to-End Test for Director stat
       run: ./github_scripts/stat_test.sh
+    - name: Run End-to-End Test for --version flag
+      run: ./github_scripts/version_test.sh

cmd/main.go

@@ -59,7 +59,7 @@ func handleCLI(args []string) error {
 		// version info regardless of the commands and whether they are defined
 		// * Remove the -v shorthand since in "origin serve" flagset it's already used for "volume" flag
 		if args[len(args)-1] == "--version" {
-			config.PrintPelicanVersion()
+			config.PrintPelicanVersion(os.Stdout)
 			return nil
 		}
 		err := Execute()

cmd/main_test.go

@@ -110,37 +110,20 @@ func TestHandleCLIVersionFlag(t *testing.T) {
 	batchTest := func(t *testing.T, arguments []string, expected string) {
 		got := ""
 
-		if expected == mockVersionOutput {
-			// Redirect output to a pip
-			oldStderr := os.Stderr
-			r, w, _ := os.Pipe()
-			os.Stderr = w
+		// Redirect output to a pipe
+		oldStdout := os.Stdout
+		r, w, _ := os.Pipe()
+		os.Stdout = w
 
-			err := handleCLI(arguments)
-			require.NoError(t, err)
+		err := handleCLI(arguments)
+		require.NoError(t, err)
 
-			// Close the write of pip and redirect output back to Stderr
-			w.Close()
-			out, _ := io.ReadAll(r)
-			os.Stderr = oldStderr
+		// Close the write of pipe and redirect output back to Stderr
+		w.Close()
+		out, _ := io.ReadAll(r)
+		os.Stdout = oldStdout
 
-			got = strings.TrimSpace(string(out))
-		} else {
-			// Redirect output to a pip
-			oldStdout := os.Stdout
-			r, w, _ := os.Pipe()
-			os.Stdout = w
-
-			err := handleCLI(arguments)
-			require.NoError(t, err)
-
-			// Close the write of pip and redirect output back to Stderr
-			w.Close()
-			out, _ := io.ReadAll(r)
-			os.Stdout = oldStdout
-
-			got = strings.TrimSpace(string(out))
-		}
+		got = strings.TrimSpace(string(out))
 
 		if expected != mockVersionOutput {
 			// If the expected string is not the version output, use Contains to check

cmd/object_copy.go

@@ -104,7 +104,7 @@ func copyMain(cmd *cobra.Command, args []string) {
 	}
 
 	if val, err := cmd.Flags().GetBool("version"); err == nil && val {
-		config.PrintPelicanVersion()
+		config.PrintPelicanVersion(os.Stdout)
 		os.Exit(0)
 	}
 

cmd/plugin.go

@@ -136,7 +136,7 @@ func stashPluginMain(args []string) {
 			fmt.Println("StartdAttrs = \"PelicanPluginVersion\"")
 			os.Exit(0)
 		} else if args[0] == "-version" || args[0] == "-v" {
-			config.PrintPelicanVersion()
+			config.PrintPelicanVersion(os.Stdout)
 			os.Exit(0)
 		} else if args[0] == "-upload" {
 			log.Debugln("Upload detected")

config/config.go

@@ -952,11 +952,11 @@ func setXrootdRunLocations(currentServers ServerType, dir string) error {
 	return nil
 }
 
-func PrintPelicanVersion() {
-	fmt.Fprintln(os.Stderr, "Version:", GetVersion())
-	fmt.Fprintln(os.Stderr, "Build Date:", GetBuiltDate())
-	fmt.Fprintln(os.Stderr, "Build Commit:", GetBuiltCommit())
-	fmt.Fprintln(os.Stderr, "Built By:", GetBuiltBy())
+func PrintPelicanVersion(out *os.File) {
+	fmt.Fprintln(out, "Version:", GetVersion())
+	fmt.Fprintln(out, "Build Date:", GetBuiltDate())
+	fmt.Fprintln(out, "Build Commit:", GetBuiltCommit())
+	fmt.Fprintln(out, "Built By:", GetBuiltBy())
 }
 
 // Print Pelican configuration to stderr

github_scripts/version_test.sh

@@ -0,0 +1,80 @@
+#!/bin/bash -xe
+
+# Copyright (C) 2024, Pelican Project, Morgridge Institute for Research
+#
+# Licensed under the Apache License, Version 2.0 (the "License"); you
+# may not use this file except in compliance with the License.  You may
+# obtain a copy of the License at
+#
+#    http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+
+# This tests the --version flag of various Pelican binaries (pelican/stash/osdf)
+
+set -e
+
+mkdir -p /tmp/pelican-test/version-test
+
+export PELICAN_CONFIGDIR=/tmp/pelican-test/version-test
+
+# Function to cleanup after test ends
+cleanup() {
+    # Clean up temporary files
+    rm -rf /tmp/pelican-test/version-test
+    rm -f ./stash
+    rm -f ./osdf
+    unset PELICAN_CONFIGDIR
+}
+
+# Setup trap with the PID as an argument to the cleanup function
+trap 'cleanup' EXIT
+
+if [ ! -f "./stash" ]; then
+    cp ./pelican ./stash
+fi
+
+if [ ! -f "./osdf" ]; then
+    cp ./pelican ./osdf
+fi
+
+stdout=$(./pelican --version)
+
+# Use variables for comparison or matching
+if [[ "$stdout" == *"Version: "* ]]; then
+    echo "pelican --version Version found in stdout"
+else
+    echo "Version not found in stdout running pelican --version"
+    echo "Test failed"
+    exit 1
+fi
+
+stdout=$(./stash --version)
+
+# Use variables for comparison or matching
+if [[ "$stdout" == *"Version: "* ]]; then
+    echo "stash --version Version found in stdout"
+else
+    echo "Version not found in stdout running stash --version"
+    echo "Test failed"
+    exit 1
+fi
+
+stdout=$(./osdf --version)
+
+# Use variables for comparison or matching
+if [[ "$stdout" == *"Version: "* ]]; then
+    echo "osdf --version Version found in stdout"
+else
+    echo "Version not found in stdout running osdf --version"
+    echo "Test failed"
+    exit 1
+fi
+
+echo "Test succeeded"
+exit 0

launchers/launcher.go

@@ -58,7 +58,7 @@ func LaunchModules(ctx context.Context, modules config.ServerType) (servers []se
 
 	ctx, shutdownCancel = context.WithCancel(ctx)
 
-	config.PrintPelicanVersion()
+	config.PrintPelicanVersion(os.Stderr) // Print Pelican version to stderr at server start
 
 	// Print Pelican config at server start if it's in debug or info level
 	if log.GetLevel() >= log.InfoLevel {