Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

basher outdated fails on non-existing repo #123

Open
dracorp opened this issue Apr 15, 2024 · 6 comments
Open

basher outdated fails on non-existing repo #123

dracorp opened this issue Apr 15, 2024 · 6 comments

Comments

@dracorp
Copy link

dracorp commented Apr 15, 2024

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
Copy link
Author

dracorp commented Apr 16, 2024

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.

@juanibiapina
Copy link
Member

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.

@pawamoy
Copy link
Contributor

pawamoy commented Apr 17, 2024

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

@dracorp
Copy link
Author

dracorp commented Apr 17, 2024

Then trap:

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

@pawamoy
Copy link
Contributor

pawamoy commented Apr 18, 2024

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

@dracorp
Copy link
Author

dracorp commented Apr 21, 2024

@pawamoy
Yes, you are right.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants