dvehrs / podget

Podcast aggregator optimized for running as a scheduled job (i.e. cron) on Linux
GNU General Public License v3.0
114 stars 13 forks source link

OS X 10.10 - podget doesn't run under cron or launchd #50

Closed scottmeup closed 3 years ago

scottmeup commented 4 years ago

I can run podget manually to parse & download podcasts but automated methods using cron or launchd fail.

launchd gives errors: /usr/local/bin/podget: line 90: shopt: inherit_errexit: invalid shell option name /usr/local/bin/podget: line 90: EXIT_ERROR: command not found

launchd is set to load this as an agent for my user.

I've tried commenting out line 90 of /usr/local/bin/podget, which results in an error message that sed and core utils aren't installed. I have confirmed via brew that both gnu sed and core utils are installed.

The cron log shows: Aug 23 10:45:00 machine.name /usr/sbin/cron[22852]: (myUsername) CMD (/usr/local/bin/podget) but downloads aren't processing

Redirecting the output of the cron job shows the same error message as with launchd: /usr/local/bin/podget: line 90: shopt: inherit_errexit: invalid shell option name /usr/local/bin/podget: line 90: EXIT_ERROR: command not found

podget is installed at /usr/local/bin/podget

dvehrs commented 4 years ago

OK the first error is something we are catching in the latest development version. The "inherit_errexit" option was added in Bash 4.4 so you may need to upgrade Bash.

The second issue for sed and coreutils is because Podget is looking for gsed and gexpr somewhere in your configured environment with the hash command. I'm currently testing a pull request by Eric Cook that makes a couple changes to how we handle the gsed/gexpr issue. That may be worth testing for you and you can find it in the dev-llua-coreutils branch.

However it still depends on finding the GNU versions of sed and expr. You may try running "gsed --version" and "gexpr --version" to see that they are both being found in your path and are the GNU versions.

dvehrs commented 4 years ago

New version 0.8.7 released today that should help address these issues.

Let me know if you have any additional problems.

Dave

scottmeup commented 4 years ago

Hi, thanks for the update. I've upgraded podget to the latest version and now receive a different error: Bash added the 'inherit_errexit' shell option in version 4.4, please upgrade.

I think gsed, gexpr and bash are installed and at a minimum required version according to this output?

$ gsed --version gsed (GNU sed) 4.8

$ gexpr --version expr (GNU coreutils) 8.31

$ bash --version GNU bash, version 5.0.17(1)-release (x86_64-apple-darwin14.5.0)

dvehrs commented 4 years ago

OK, it is possible that your version of Bash was compiled without that option. So let's start by checking it.

I will run two tests on Debian Linux. One for the option we are looking for and one for an option that does not exist so you can see the difference.

$ shopt inherit_errexit inherit_errexit off

$ shopt bad-option bash: shopt: bad-option: invalid shell option name

The first lets us know the option exists but is not enabled, so we can do so.

The second is what happens for options that are not supported by this version of Bash. Our test is to watch for the word 'invalid' so that's what makes me wonder if the inherit_errexit option has been compiled into your version of bash.

How did you install Bash?

Dave

scottmeup commented 3 years ago

Hi Dave, It looks like I updated Bash with Macports:

$ port installed | grep bash bash @5.0.17_0 (active)

dvehrs commented 3 years ago

If I'm not mistaken, OS X 10.10 last security update was in 2017. So this may be interesting.

Lets work through this in a few steps.

  1. Load bash as the script does. $ /usr/bin/env bash

  2. Once in bash, lets verify the version $ echo $BASH_VERSION

This will let us know what version of bash the system is loading.

  1. Now lets verify if the inherit_errexit option is available or not. $ shopt inherit_errexit

To see a list of all possible options, just run "shopt"

On my system, the three commands produce output that looks like: $ /usr/bin/env bash $ echo $BASH_VERSION 5.1.0(1)-rc1 $ shopt inherit_errexit inherit_errexit off $ exit

Not all versions of Bash have every option enabled and I'm wondering if the version packaged by Macports is not compiled to include the inherit_errexit option. If it does not support that option then we need to consider other alternatives to testing for it. Theoretically the option makes debugging easier but may be a luxury that is not needed on older systems.

Dave

scottmeup commented 3 years ago

Hi Dave, thanks for the reply. Here's what I got:

$ /usr/bin/env bash $ echo $BASH_VERSION 5.0.17(1)-release $ shopt inherit_errexit inherit_errexit off

dvehrs commented 3 years ago

OK, that's good. It means that the inherit_errexit option is supported by your version of Bash. So the problem may be within how we are testing for it.

I've whipped up a short script that will test in a similar way to how we use the options in Podget. You will notice that it includes a trap function on the ERR condition. We should never hit that, so if you see 'EXIT STATUS: #' in your output, we've failed to account for some possibilities or something is being handled a little bit differently on your system.

#!/usr/bin/env bash

# -----------------------------------------------------------------------------
# Filename:      test_shellopts.sh
# Version:       0.1a
# -----------------------------------------------------------------------------

EC() { echo -e '\e[1;33m'EXIT STATUS: $?'\e[m\n'; }

trap EC ERR

# Print commands and their arguments as they are executed.
set -x

# Set Shell Options similar to how we use them in Podget
set -o errexit
set -o nounset
set -o pipefail
shopt -s extglob

echo

echo "Make sure the 'inherit_errexit' option is unset."
shopt -u inherit_errexit

echo

echo 'Test One for inherit_errexit invalid:'
if (set +o pipefail && shopt inherit_errexit 2>&1 | grep -q "invalid"); then
    echo -e '\nTripped Test One - inherit_errexit is invalid.\n'
fi

echo "End of Test One reached safely"

echo

echo 'Test Two for inherit_errexit off:'
if (set +o pipefail && shopt inherit_errexit 2>&1 | grep -q "off"); then
    echo -e '\nTripped Test Two - inherit_errexit is off.\n'
fi

echo "End of Test Two reached safely"

echo

echo 'Test Three for nonexistent_option:'
if (set +o pipefail && shopt nonexistent_option 2>&1 | grep -q "invalid"); then
    echo -e '\nTripped Test Three, nonexistent_option does not exist.\n'
fi

echo "End of Test Three reached safely"

echo

On my system, this script produces this output:

$ ./test_shellopts.sh
+ set -o errexit
+ set -o nounset
+ set -o pipefail
+ shopt -s extglob
+ echo

+ echo 'Make sure the '\''inherit_errexit'\'' option is unset.'
Make sure the 'inherit_errexit' option is unset.
+ shopt -u inherit_errexit
+ echo

+ echo 'Test One for inherit_errexit invalid:'
Test One for inherit_errexit invalid:
+ set +o pipefail
+ shopt inherit_errexit
+ grep -q invalid
+ echo 'End of Test One reached safely'
End of Test One reached safely
+ echo

+ echo 'Test Two for inherit_errexit off:'
Test Two for inherit_errexit off:
+ set +o pipefail
+ shopt inherit_errexit
+ grep -q off
+ echo -e '\nTripped Test Two - inherit_errexit is off.\n'

Tripped Test Two - inherit_errexit is off.

+ echo 'End of Test Two reached safely'
End of Test Two reached safely
+ echo

+ echo 'Test Three for nonexistent_option:'
Test Three for nonexistent_option:
+ set +o pipefail
+ shopt nonexistent_option
+ grep -q invalid
+ echo -e '\nTripped Test Three, nonexistent_option does not exist.\n'

Tripped Test Three, nonexistent_option does not exist.

+ echo 'End of Test Three reached safely'
End of Test Three reached safely
+ echo

And if we remove or comment to disable the 'set -x' command, it looks like:

$ ./test_shellopts.sh 

Make sure the 'inherit_errexit' option is unset.

Test One for inherit_errexit invalid:
End of Test One reached safely

Test Two for inherit_errexit off:

Tripped Test Two - inherit_errexit is off.

End of Test Two reached safely

Test Three for nonexistent_option:

Tripped Test Three, nonexistent_option does not exist.

End of Test Three reached safely

Now, if you can run this script on your system hopefully we can figure out what is happening.

Dave

scottmeup commented 3 years ago

Hi Dave, This is the output I have. I think it matches your output from a quick check over.

+ set -o errexit
+ set -o nounset
+ set -o pipefail
+ shopt -s extglob
+ echo
+ echo 'Make sure the '\''inherit_errexit'\'' option is unset.'
+ shopt -u inherit_errexit
+ echo
+ echo 'Test One for inherit_errexit invalid:'
+ set +o pipefail
+ shopt inherit_errexit
+ grep -q invalid
+ echo 'End of Test One reached safely'
+ echo
+ echo 'Test Two for inherit_errexit off:'
+ set +o pipefail
+ shopt inherit_errexit
+ grep -q off
+ echo -e '\nTripped Test Two - inherit_errexit is off.\n'
+ echo 'End of Test Two reached safely'
+ echo
+ echo 'Test Three for nonexistent_option:'
+ set +o pipefail
+ shopt nonexistent_option
+ grep -q invalid
+ echo -e '\nTripped Test Three, nonexistent_option does not exist.\n'
+ echo 'End of Test Three reached safely'
+ echo

and with set -x commented out

Make sure the 'inherit_errexit' option is unset.

Test One for inherit_errexit invalid:
End of Test One reached safely

Test Two for inherit_errexit off:

Tripped Test Two - inherit_errexit is off.

End of Test Two reached safely

Test Three for nonexistent_option:

Tripped Test Three, nonexistent_option does not exist.

End of Test Three reached safely

Just a thought: would it be helpful to test this under cron or launchd? The current version of podget as of 23 August 2020 was running when launched from terminal but failed when started by cron or launchd.

dvehrs commented 3 years ago

In my experience, it's generally best to confirm that everything works as it should when manually launched. We're almost to the cron/launchd stage.

For these tests, you may want to make a temporary copy of podget as we will make a couple minor changes to it.

  1. Uncomment the fourth line to enable "set -x"
  2. After line 98 which reads "shopt -s extglob", add a line that says "exit 137"

The will give us verbose debugging of the bash options being set.

After making those changes and running it, I get:

{ ~/TMP }$ ./podget
+ ERR_DISPLAYHELP=0
+ ERR_LIBNOTDEF=50
+ ERR_LIBLOWSPACE=51
+ ERR_LIBC6NOTINSTALLED=60
+ ERR_RUNNINGSESSION=70
+ ERR_IMPORTOPML=80
+ ERR_EXPORTOPML=90
+ trap 'EXIT_ERROR ${LINENO} ${?} ${FUNCNAME:-Unconfigured}' ERR
+ trap 'CLEANUP_AND_EXIT 143' TERM
+ trap 'CLEANUP_AND_EXIT 130' INT
+ set -o errexit
+ set -o nounset
+ set -o pipefail
+ set +o pipefail
+ shopt inherit_errexit
+ grep -q invalid
+ shopt -s inherit_errexit
+ shopt -s extglob
+ exit 137

If the script is unable to find the inherit_errexit option then the last line will read "exit 1".

At this stage, you can test it from cron or launchd so we can see how it is starting up. If it works so that it also finishes with an "exit 137" then you can disable line 4 by commenting it and remove the "exit 137" from about line 100. Then try a full run to see what happens.

Good luck! Dave

dvehrs commented 3 years ago

Also if you want, you can setup a cron/launchd session for the test_shellopts.sh script. It may not be necessary bit it could give us a bit more information if there is an issue.

Dave

scottmeup commented 3 years ago

Hi Dave, Running the modified podget from terminal seems to give the same output as what you have:

+ ERR_DISPLAYHELP=0
+ ERR_LIBNOTDEF=50
+ ERR_LIBLOWSPACE=51
+ ERR_LIBC6NOTINSTALLED=60
+ ERR_RUNNINGSESSION=70
+ ERR_IMPORTOPML=80
+ ERR_EXPORTOPML=90
+ trap 'EXIT_ERROR ${LINENO} ${?} ${FUNCNAME:-Unconfigured}' ERR
+ trap 'CLEANUP_AND_EXIT 143' TERM
+ trap 'CLEANUP_AND_EXIT 130' INT
+ set -o errexit
+ set -o nounset
+ set -o pipefail
+ set +o pipefail
+ shopt inherit_errexit
+ grep -q invalid
+ shopt -s inherit_errexit
+ shopt -s extglob
+ exit 137

I ran test_shellopts.sh as a cronjob with the following line in crontab: * * * * * /tmp/test_shellopts.sh >> /tmp/test_shellopts.log 2>&1

Which gave the following output:

/tmp/test_shellopts.sh: line 24: shopt: inherit_errexit: invalid shell option name
\e[1;33mEXIT STATUS: 1\e[m
dvehrs commented 3 years ago

OK, it appears we have a difference in what the script sees when run as a user and when run from cron.

OK, I've got one more quick test script to run as in a shell and as a cron job. This should help use determine the differences. Hopefully I've got enough in here to get us started.

Script:

#!/usr/bin/env bash

# -----------------------------------------------------------------------------
# Filename:      test_bash.sh
# Version:       0.1a
# -----------------------------------------------------------------------------

set +o errexit

echo "BASH VERSION: $BASH_VERSION"

echo

echo "PATH: $PATH"

echo

echo -n "BASH WHERE: "
whereis bash

On my system this tells us:

$ ./test_bash.sh 
BASH VERSION: 5.1.0(1)-rc1

PATH: /usr/share/safe-rm/bin:/home/lowkey/bin:/usr/local/bin:/usr/bin:/bin:/usr/local/games:/usr/games:/snap/bin

BASH WHERE: bash: /bin/bash /etc/bash.bashrc /usr/share/man/man1/bash.1.gz

I'm guessing but I think we may see a difference in a few of these items. The one I'm primarily concerned with is the PATH value.

Dave

scottmeup commented 3 years ago

Hi Dave, Seems cron isn't working with the minimum required bash version from the output of test_bash.sh :

BASH VERSION: 3.2.57(1)-release

PATH: /usr/bin:/bin

BASH WHERE: /bin/bash
dvehrs commented 3 years ago

OK, that's half of what I asked for. When you're trying to get tech support being pithy in your responses is not a virtue, it's a sin.

I'm going to guess that is from cron because it has Bash 3.2 and that is consistent with the errors you describe.
[ NOTE: I didn't read enough. You say it is from cron so we're just missing the shell output to compare to. ]

What we need to know is what is different in the PATH and where Bash is when you run that command in the shell (which is working for our other tests). It's probably also a good idea to verify the locations of gsed and gexpr so if there are any additional changes to the PATH we can make them at the same time.

So you need to run in a shell:

$ echo $PATH
$ which bash
$ which gsed
$ which gexpr

I'm just going to guess for a minute, but lets assume we find out Bash version 5 is at /usr/local/bin/bash and gsed/gexpr are also in that directory. So we need to add one directory to PATH for the command to work.

So our command in crontab would look like: PATH=/usr/local/bin:$PATH /path/to/test_bash.sh OR PATH=/usr/local/bin:/usr/bin:/bin /path/to/test_bash.sh

For example, if I run these commands on my system, this is what I get. Never mind that I search for sed/expr instead of gsed/gexpr that's because I'm on Linux and not Mac OS.

$ echo $PATH
/home/USER/bin:/usr/local/bin:/usr/bin:/bin
$ which bash
bash is /bin/bash
$ which sed
sed is /bin/sed
$ which expr
expr is /home/USER/bin/expr
expr is /usr/bin/expr

In this case, you will notice that expr is in two places (it's actually just a symlink for this example). If I wanted to make sure that I used the first version of expr then I would need to make sure it's directory came first in my PATH value. So my PATH would look like this for the command:

PATH=/home/USER/bin:/usr/bin:/bin /path/to/test_bash.sh 

Now I keep /usr/bin in the PATH because there are likely other things that the script may need that reside in that directory. You may also want to keep /usr/local/bin in there if some items also reside there.

Also keep in mind that the PATH value is processed from left to right. So when items have the same name, the first one found is generally used. In this case /home/USER/bin/expr is used because it is found first.

With a little luck this will fix the issues you've been experiencing.

Dave

scottmeup commented 3 years ago

Hi Dave,

OK, that's half of what I asked for. When you're trying to get tech support being pithy in your responses is not a virtue, it's a sin.

It looks like I misread your previous post, please find the attached output from test_bash.sh run in Bash:

BASH VERSION: 5.0.17(1)-release

PATH: /Users/scottmeup/opt/anaconda3/bin:/Users/scottmeup/opt/anaconda3/condabin:/Library/Frameworks/Python.framework/Versions/3.8/bin:/opt/local/bin:/opt/local/sbin:/usr/local/bin:/usr/local/sbin:/usr/bin:/bin:/usr/sbin:/sbin:/usr/local/bin

BASH WHERE: /bin/bash

the following outputs as run from bash are:

$ echo $PATH
/Users/scottmeup/opt/anaconda3/bin:/Users/scottmeup/opt/anaconda3/condabin:/Library/Frameworks/Python.framework/Versions/3.8/bin:/opt/local/bin:/opt/local/sbin:/usr/local/bin:/usr/local/sbin:/usr/bin:/bin:/usr/sbin:/sbin:/usr/local/bin

$ which bash
/opt/local/bin/bash

$ which gsed
/usr/local/bin/gsed

$ which gexpr
/opt/local/bin/gexpr

 

 

Setting the following job in crontab: * * * * * PATH=/opt/local/bin/:$PATH /tmp/test_bash.sh >> /tmp/test_bash.log 2>&1

gives the following output:

BASH VERSION: 5.0.17(1)-release

PATH: /opt/local/bin/:/usr/bin:/bin

BASH WHERE: /bin/bash

Accordingly, I've set a cron job pointing to the podget we modified earlier: * * * * * PATH=/opt/local/bin/:$PATH /usr/local/bin/podget >> /tmp/podgetCron.log 2>&1

which gives the following output:

+ ERR_DISPLAYHELP=0
+ ERR_LIBNOTDEF=50
+ ERR_LIBLOWSPACE=51
+ ERR_LIBC6NOTINSTALLED=60
+ ERR_RUNNINGSESSION=70
+ ERR_IMPORTOPML=80
+ ERR_EXPORTOPML=90
+ trap 'EXIT_ERROR ${LINENO} ${?} ${FUNCNAME:-Unconfigured}' ERR
+ trap 'CLEANUP_AND_EXIT 143' TERM
+ trap 'CLEANUP_AND_EXIT 130' INT
+ set -o errexit
+ set -o nounset
+ set -o pipefail
+ set +o pipefail
+ shopt inherit_errexit
+ grep -q invalid
+ shopt -s inherit_errexit
+ shopt -s extglob
+ exit 137

Cheers - Scott.

dvehrs commented 3 years ago

Excellent!

Looks like we need to do one small change to the PATH for podget because gsed is in one more directory to include.

If I'm not mistaken, this should work: PATH=/opt/local/bin:/usr/local/bin:$PATH /usr/local/bin/podget

On a side note, there appears to be an issue with the use of 'whereis bash' in the test_bash.sh script. Not sure that's a significant issue because our use of 'which bash' at the commandline gave us the answer we needed. That's certainly an issue on my end, have to figure out which will reliably give the answer I need.

Let me know if we run into any more issues! :-)

Dave

dvehrs commented 3 years ago

Oh and its probably time to comment the "set -x" at the beginning of podget and remove the 'exit 137' at about line 100. Silly me, I'd forget my head if it wasn't attached at times. :-)

Dave

dvehrs commented 3 years ago

Also one more thing to think about. Is there a good way to add this to the podget man page section on cron jobs? If you ran into this issue, someone else out there will too. I'm not sure we need to go into explicit detail but maybe a nice summary of how to handle this issue. That certainly mostly on my side but I'd be open to any ideas you have.

Dave

scottmeup commented 3 years ago

Hi Dave,

PATH=/opt/local/bin:/usr/local/bin:$PATH /usr/local/bin/podget

With that path set in the cron job It looks like podget's output from cron matches the output from bash. Thank you very much.

I've been trying to think of a way to elegantly get around the problem. The only thought I had would be to check the location of the required binaries at install time and make a script that launched podget with those locations set as either path variables or directly referenced. It's definitely not what I'd call elegant though.

For adding details to the man page probably mentioning minimum required bash, gsed and gexpr version, as well as stating that the path to those variables needs to be added to the cron job.

Maybe a combination of the two? Telling the user to set a cron job as * * * * * /location_to/bash /location_to/podget to explicitly call the correct version of bash, and having podget contain a line setting the locations of gsed & gexpr as path variables?

I think most users who are comfortable enough editing cron jobs would be capable of adding the necessary path variable to a cron job as long as they know that it's required. That's probably all that's needed to be honest. A descriptive error message in the output folder might help draw a user's attention to the problem if it's encountered, I'm not sure how many users are familiar with setting up logging from a cron job.

Cheers - Scott

dvehrs commented 3 years ago

That's fairly similar to what I've been considering. Well minus the automatic PATH setting. I haven't figured out an elegant way to handle it either.

Might be best to just include a simple test for minimum Bash version supported, then count on the other items being found missing if the path is incorrect. Then include an example of how to custom set the path for a cron job in the man page.

Then as a final layer, include notes in the INSTALL file for how to determine PATH items when necessary and maybe an example of how to determine what the default PATH for a script called as a cron job is.

That's where I am now, still trying to find an elegant solution.

Dave

dvehrs commented 3 years ago

OK, uploaded a new version today (v0.8.8) with the changes to the man page and INSTALL file.

Hopefully this is enough to help people who may run into issues.

Dave