basher icon indicating copy to clipboard operation
basher copied to clipboard

basher outdated fails on non-existing repo

Open dracorp opened this issue 10 months ago • 6 comments

I had a repository installed by basher that currently does not exist. basher outdated did not list my other repository also installed by basher. Finally I found that git remote update on non-existing repository was failing.

$ git remote update
remote: Repository not found.
fatal: repository 'https://github.com/user/repo.git/' not found

It would be good to handle this case so that the basher does not terminate the process just display the reason for its exit.

The same situation is for:

$ git remote update
fatal: couldn't find remote ref refs/heads/master

Here there was a branch change from master to main. The set -e itself is problematic.

dracorp avatar Apr 15 '24 21:04 dracorp

I've commented set -e and added:

diff --git a/libexec/basher-outdated b/libexec/basher-outdated
index 5cf769a..e87b117 100755
--- a/libexec/basher-outdated
+++ b/libexec/basher-outdated
@@ -3,7 +3,7 @@
 # Summary: Displays a list of outdated packages
 # Usage: basher outdated

-set -e
+#set -e

 shopt -s nullglob

@@ -14,7 +14,10 @@ do
   package_path="$BASHER_PACKAGES_PATH/$package"
   if [ ! -L "$package_path" ]; then
     cd "$package_path"
-    git remote update > /dev/null 2>&1
+    if ! git remote update > /dev/null 2>&1; then
+        echo "ERROR: The command 'git remote update' failed on $package." >&2
+        continue
+    fi
     if git symbolic-ref --short -q HEAD > /dev/null; then
         if [ "$(git rev-list --count HEAD...HEAD@{upstream})" -gt 0 ]; then
           echo "$package"

That was completely enough for me.

dracorp avatar Apr 16 '24 04:04 dracorp

Thanks for the report, this is indeed a problem. Basher should give the user a warning the a repository is now changed or gone.

On the other hand, I'm concerned about disabling "set -e" because it helps catch bugs like this one.

If you're willing to work on a PR, there may be a reliable way to check the output of running git remote update while keeping set -e, or an alternative way to run this check.

juanibiapina avatar Apr 17 '24 15:04 juanibiapina

set -e can definitely be kept with the rest of the changes. A failing condition (if command) won't trigger it :)

pawamoy avatar Apr 17 '24 16:04 pawamoy

Then trap:

trap 'echo "ERROR: The command 'git remote update' failed on $package."' ERR

dracorp avatar Apr 17 '24 21:04 dracorp

No need for traps. Just leave set -e untouched, and use if/elif/else conditions to avoid triggering it. Taking your diff above:

#!/usr/bin/env bash
#
# Summary: Displays a list of outdated packages
# Usage: basher outdated

set -e

shopt -s nullglob

IFS=$'\r\n' packages=($(basher list))

for package in "${packages[@]}"
do
  package_path="$BASHER_PACKAGES_PATH/$package"
  if [ ! -L "$package_path" ]; then
    cd "$package_path"
    if ! git remote update > /dev/null 2>&1; then
        echo "ERROR: The command 'git remote update' failed on $package." >&2
        continue
    fi
    if git symbolic-ref --short -q HEAD > /dev/null; then
        if [ "$(git rev-list --count HEAD...HEAD@{upstream})" -gt 0 ]; then
          echo "$package"
        fi
    fi
  fi
done

Here the only instructions that could cause the script to exit are IFS=$'\r\n' packages=($(basher list)) (when basher list returns a non-zero exit code) and cd "$package_path" when the directory does not exist. If we want to prevent this last one, the same method can be used:

if ! cd "$package_path"; then
    echo "ERROR: package '$package_path' does not exist" >&2
    continue
fi

pawamoy avatar Apr 18 '24 12:04 pawamoy

@pawamoy Yes, you are right.

dracorp avatar Apr 21 '24 18:04 dracorp