[pacman-dev,2/3] libmakepkg: implement extendable source protocols

Message ID 20180529043056.12491-2-eschwartz@archlinux.org
State Superseded, archived
Headers show
Series [pacman-dev,1/3] libmakepkg: optimize get_protocol to always return proto, not proto+uri | expand

Commit Message

Eli Schwartz May 29, 2018, 4:30 a.m. UTC
Lookup the existence of matching functions for each protocol, and
fallback on the generic file handler. New source protocols can then be
added via thirdparty libmakepkg drop-ins without requiring modifications
to source.sh

Fixes FS#49076

Signed-off-by: Eli Schwartz <eschwartz@archlinux.org>
---
 scripts/libmakepkg/source.sh.in | 46 +++++++++------------------------
 1 file changed, 12 insertions(+), 34 deletions(-)

Comments

Allan McRae June 4, 2018, 6:56 a.m. UTC | #1
On 29/05/18 14:30, Eli Schwartz wrote:
> Lookup the existence of matching functions for each protocol, and
> fallback on the generic file handler. New source protocols can then be
> added via thirdparty libmakepkg drop-ins without requiring modifications
> to source.sh
> 
> Fixes FS#49076
> 
> Signed-off-by: Eli Schwartz <eschwartz@archlinux.org>
> ---
>  scripts/libmakepkg/source.sh.in | 46 +++++++++------------------------
>  1 file changed, 12 insertions(+), 34 deletions(-)
> 
> diff --git a/scripts/libmakepkg/source.sh.in b/scripts/libmakepkg/source.sh.in
> index 198efd5e..c07ce76d 100644
> --- a/scripts/libmakepkg/source.sh.in
> +++ b/scripts/libmakepkg/source.sh.in
> @@ -60,25 +60,15 @@ download_sources() {
>  
>  		local proto=$(get_protocol "$netfile")
>  		case "$proto" in
> -			local)
> -				download_local "$netfile"
> -				;;
> -			bzr)
> -				(( get_vcs )) && download_bzr "$netfile"
> -				;;
> -			git)
> -				(( get_vcs )) && download_git "$netfile"
> -				;;
> -			hg)
> -				(( get_vcs )) && download_hg "$netfile"
> -				;;
> -			svn)
> -				(( get_vcs )) && download_svn "$netfile"
> -				;;
> -			*)
> -				download_file "$netfile"
> +			bzr|git|hg|svn)
> +				(( get_vcs )) || continue
>  				;;

Should this be moved into the download_$proto functions?  I'd guess most
of the dropins would be for other vcs systems, and this is not extendable.

>  		esac
> +		if declare -f download_$proto > /dev/null; then
> +			download_$proto "$netfile"
> +		else
> +			download_file "$netfile"
> +		fi
>  
>  		popd &>/dev/null
>  	done
> @@ -92,22 +82,10 @@ extract_sources() {
>  	for netfile in "${all_sources[@]}"; do
>  		local file=$(get_filename "$netfile")
>  		local proto=$(get_protocol "$netfile")
> -		case "$proto" in
> -			bzr)
> -				extract_bzr "$netfile"
> -				;;
> -			git)
> -				extract_git "$netfile"
> -				;;
> -			hg)
> -				extract_hg "$netfile"
> -				;;
> -			svn)
> -				extract_svn "$netfile"
> -				;;
> -			*)
> -				extract_file "$file"
> -				;;
> -		esac
> +		if declare -f extract_$proto > /dev/null; then
> +			extract_$proto "$netfile"
> +		else
> +			extract_file "$file"
> +		fi
>  	done
>  }
>
Eli Schwartz June 4, 2018, 10:46 a.m. UTC | #2
On 06/04/2018 02:56 AM, Allan McRae wrote:
>> +			bzr|git|hg|svn)
>> +				(( get_vcs )) || continue
>>  				;;
> 
> Should this be moved into the download_$proto functions?  I'd guess most
> of the dropins would be for other vcs systems, and this is not extendable.
Hmm.

Well, other VCS systems could implement private routines for "return 0"
early on this, but it isn't strictly necessary for here. It's mildly
more verbose, but OTOH it also makes things slightly more obvious. I
guess just for the sake of clarity it should do as you suggested...

Patch

diff --git a/scripts/libmakepkg/source.sh.in b/scripts/libmakepkg/source.sh.in
index 198efd5e..c07ce76d 100644
--- a/scripts/libmakepkg/source.sh.in
+++ b/scripts/libmakepkg/source.sh.in
@@ -60,25 +60,15 @@  download_sources() {
 
 		local proto=$(get_protocol "$netfile")
 		case "$proto" in
-			local)
-				download_local "$netfile"
-				;;
-			bzr)
-				(( get_vcs )) && download_bzr "$netfile"
-				;;
-			git)
-				(( get_vcs )) && download_git "$netfile"
-				;;
-			hg)
-				(( get_vcs )) && download_hg "$netfile"
-				;;
-			svn)
-				(( get_vcs )) && download_svn "$netfile"
-				;;
-			*)
-				download_file "$netfile"
+			bzr|git|hg|svn)
+				(( get_vcs )) || continue
 				;;
 		esac
+		if declare -f download_$proto > /dev/null; then
+			download_$proto "$netfile"
+		else
+			download_file "$netfile"
+		fi
 
 		popd &>/dev/null
 	done
@@ -92,22 +82,10 @@  extract_sources() {
 	for netfile in "${all_sources[@]}"; do
 		local file=$(get_filename "$netfile")
 		local proto=$(get_protocol "$netfile")
-		case "$proto" in
-			bzr)
-				extract_bzr "$netfile"
-				;;
-			git)
-				extract_git "$netfile"
-				;;
-			hg)
-				extract_hg "$netfile"
-				;;
-			svn)
-				extract_svn "$netfile"
-				;;
-			*)
-				extract_file "$file"
-				;;
-		esac
+		if declare -f extract_$proto > /dev/null; then
+			extract_$proto "$netfile"
+		else
+			extract_file "$file"
+		fi
 	done
 }