-
-
Notifications
You must be signed in to change notification settings - Fork 5.5k
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
Fix download agent search relying on throwing of Sys.which()
#28157
Conversation
This fixes issues on systems that don't have `which` available
try | ||
downloadcmd = basename(something(Sys.which.(download_agents)...)) | ||
catch | ||
error("no download agent available; install curl, wget, or fetch") | ||
end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would be better (and not longer) to write this as a loop, rather than obscuring all errors with this message
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What else can throw here except something(nothing)
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree it would be nicer to do without try/catch entirely, since that's now possible here.
When this code was transferred over, it relied on
Sys.which()
throwing, which was changed in theSys.which()
PR, but this code snippet never got updated.