From 26791e8412bbb5058007418b97f9517f911384cd Mon Sep 17 00:00:00 2001 From: Andreas Braun Date: Mon, 25 Jan 2021 13:39:13 +0100 Subject: [PATCH] Address code review feedback --- dist/index.js | 38 ++++++++++++++-------- src/extensions.ts | 76 ++++++++++++++++++++++++++----------------- src/scripts/common.sh | 20 +++++++++++- src/scripts/darwin.sh | 18 ---------- src/scripts/linux.sh | 18 ---------- 5 files changed, 89 insertions(+), 81 deletions(-) diff --git a/dist/index.js b/dist/index.js index 1aba8c63..1384c87e 100644 --- a/dist/index.js +++ b/dist/index.js @@ -2901,13 +2901,7 @@ async function addExtensionDarwin(extension_csv, version) { const version_extension = version + extension; const [ext_name, ext_version] = extension.split('-'); const ext_prefix = await utils.getExtensionPrefix(ext_name); - // Install extensions from a GitHub tarball. This needs to be checked first - // as the version may also match the semver check below. - const urlMatches = extension.match(/.*-(.*)\/(.*)@(.*)/); - if (urlMatches != null) { - add_script += await utils.joins('\nadd_extension_from_github', ext_name, urlMatches[1], urlMatches[2], urlMatches[3], ext_prefix); - return; - } + let matches; switch (true) { // match :extension case /^:/.test(ext_name): @@ -2930,6 +2924,17 @@ async function addExtensionDarwin(extension_csv, version) { case /.*-(stable|beta|alpha|devel|snapshot|rc|preview)/.test(version_extension): add_script += await utils.joins('\nadd_unstable_extension', ext_name, ext_version, ext_prefix); return; + // match extensions from GitHub. Do this before checking for semver as + // the version may match that as well + case /.*-(.*)\/(.*)@(.*)/.test(extension): + matches = /.*-(.*)\/(.*)@(.*)/.exec(extension); + if (matches == null) { + // Shouldn't happen + add_script += await utils.getUnsupportedLog(extension, version, 'darwin'); + return; + } + add_script += await utils.joins('\nadd_extension_from_github', ext_name, matches[1], matches[2], matches[3], ext_prefix); + return; // match semver case /.*-\d+\.\d+\.\d+.*/.test(version_extension): add_script += await utils.joins('\nadd_pecl_extension', ext_name, ext_version, ext_prefix); @@ -3056,13 +3061,7 @@ async function addExtensionLinux(extension_csv, version) { const version_extension = version + extension; const [ext_name, ext_version] = extension.split('-'); const ext_prefix = await utils.getExtensionPrefix(ext_name); - // Install extensions from a GitHub tarball. This needs to be checked first - // as the version may also match the semver check below. - const urlMatches = extension.match(/.*-(.*)\/(.*)@(.*)/); - if (urlMatches != null) { - add_script += await utils.joins('\nadd_extension_from_github', ext_name, urlMatches[1], urlMatches[2], urlMatches[3], ext_prefix); - return; - } + let matches; switch (true) { // Match :extension case /^:/.test(ext_name): @@ -3088,6 +3087,17 @@ async function addExtensionLinux(extension_csv, version) { case /.*-(stable|beta|alpha|devel|snapshot|rc|preview)/.test(version_extension): add_script += await utils.joins('\nadd_unstable_extension', ext_name, ext_version, ext_prefix); return; + // match extensions from GitHub. Do this before checking for semver as + // the version may match that as well + case /.*-(.*)\/(.*)@(.*)/.test(extension): + matches = /.*-(.*)\/(.*)@(.*)/.exec(extension); + if (matches == null) { + // Shouldn't happen + add_script += await utils.getUnsupportedLog(extension, version, 'linux'); + return; + } + add_script += await utils.joins('\nadd_extension_from_github', ext_name, matches[1], matches[2], matches[3], ext_prefix); + return; // match semver versions case /.*-\d+\.\d+\.\d+.*/.test(version_extension): add_script += await utils.joins('\nadd_pecl_extension', ext_name, ext_version, ext_prefix); diff --git a/src/extensions.ts b/src/extensions.ts index 40a950b8..916b4806 100644 --- a/src/extensions.ts +++ b/src/extensions.ts @@ -17,21 +17,7 @@ export async function addExtensionDarwin( const version_extension: string = version + extension; const [ext_name, ext_version]: string[] = extension.split('-'); const ext_prefix = await utils.getExtensionPrefix(ext_name); - - // Install extensions from a GitHub tarball. This needs to be checked first - // as the version may also match the semver check below. - const urlMatches = extension.match(/.*-(.*)\/(.*)@(.*)/); - if (urlMatches != null) { - add_script += await utils.joins( - '\nadd_extension_from_github', - ext_name, - urlMatches[1], - urlMatches[2], - urlMatches[3], - ext_prefix - ); - return; - } + let matches: RegExpExecArray; switch (true) { // match :extension @@ -69,6 +55,28 @@ export async function addExtensionDarwin( ext_prefix ); return; + // match extensions from GitHub. Do this before checking for semver as + // the version may match that as well + case /.*-(.*)\/(.*)@(.*)/.test(extension): + matches = /.*-(.*)\/(.*)@(.*)/.exec(extension) as RegExpExecArray; + if (matches == null) { + // Shouldn't happen + add_script += await utils.getUnsupportedLog( + extension, + version, + 'darwin' + ); + return; + } + add_script += await utils.joins( + '\nadd_extension_from_github', + ext_name, + matches[1], + matches[2], + matches[3], + ext_prefix + ); + return; // match semver case /.*-\d+\.\d+\.\d+.*/.test(version_extension): add_script += await utils.joins( @@ -239,21 +247,7 @@ export async function addExtensionLinux( const version_extension: string = version + extension; const [ext_name, ext_version]: string[] = extension.split('-'); const ext_prefix = await utils.getExtensionPrefix(ext_name); - - // Install extensions from a GitHub tarball. This needs to be checked first - // as the version may also match the semver check below. - const urlMatches = extension.match(/.*-(.*)\/(.*)@(.*)/); - if (urlMatches != null) { - add_script += await utils.joins( - '\nadd_extension_from_github', - ext_name, - urlMatches[1], - urlMatches[2], - urlMatches[3], - ext_prefix - ); - return; - } + let matches: RegExpExecArray; switch (true) { // Match :extension @@ -296,6 +290,28 @@ export async function addExtensionLinux( ext_prefix ); return; + // match extensions from GitHub. Do this before checking for semver as + // the version may match that as well + case /.*-(.*)\/(.*)@(.*)/.test(extension): + matches = /.*-(.*)\/(.*)@(.*)/.exec(extension) as RegExpExecArray; + if (matches == null) { + // Shouldn't happen + add_script += await utils.getUnsupportedLog( + extension, + version, + 'linux' + ); + return; + } + add_script += await utils.joins( + '\nadd_extension_from_github', + ext_name, + matches[1], + matches[2], + matches[3], + ext_prefix + ); + return; // match semver versions case /.*-\d+\.\d+\.\d+.*/.test(version_extension): add_script += await utils.joins( diff --git a/src/scripts/common.sh b/src/scripts/common.sh index c4804ba7..1cc7c571 100644 --- a/src/scripts/common.sh +++ b/src/scripts/common.sh @@ -267,4 +267,22 @@ add_composertool() { # Function to get PHP version in semver format. php_semver() { php"$version" -v | grep -Eo -m 1 "[0-9]+\.[0-9]+\.[0-9]+" | head -n 1 -} \ No newline at end of file +} + +# Function to install extension from a GitHub repository +add_extension_from_github() { + extension=$1 + org=$2 + repo=$3 + release=$4 + prefix=$5 + ( + add_devtools phpize + delete_extension "$extension" + git clone --recurse-submodules -b "$release" https://github.com/"$org"/"$repo" /tmp/"$repo-$release" || exit 1 + cd /tmp/"$repo-$release" || exit 1 + phpize && ./configure && make -j"$(nproc)" && sudo make install + enable_extension "$extension" "$prefix" + ) >/dev/null 2>&1 + add_extension_log "$extension-$org/$repo@$release" "Installed and enabled" +} diff --git a/src/scripts/darwin.sh b/src/scripts/darwin.sh index 08c9fce6..7fd28f07 100644 --- a/src/scripts/darwin.sh +++ b/src/scripts/darwin.sh @@ -39,24 +39,6 @@ add_pecl_extension() { fi } -# Function to install extension from a GitHub repository -add_extension_from_github() { - extension=$1 - org=$2 - repo=$3 - release=$4 - prefix=$5 - ( - add_devtools phpize - delete_extension "$extension" - git clone --recurse-submodules -b "$release" https://github.com/"$org"/"$repo" /tmp/"$repo-$release" || exit 1 - cd /tmp/"$repo-$release" || exit 1 - phpize && ./configure && make -j"$(nproc)" && sudo make install - enable_extension "$extension" "$prefix" - ) >/dev/null 2>&1 - add_extension_log "$extension-$org/$repo@$release" "Installed and enabled" -} - # Function to fetch a brew tap fetch_brew_tap() { tap=$1 diff --git a/src/scripts/linux.sh b/src/scripts/linux.sh index f022bbb8..d1c1a994 100644 --- a/src/scripts/linux.sh +++ b/src/scripts/linux.sh @@ -148,24 +148,6 @@ add_pecl_extension() { fi } -# Function to install extension from a GitHub repository -add_extension_from_github() { - extension=$1 - org=$2 - repo=$3 - release=$4 - prefix=$5 - ( - add_devtools phpize - delete_extension "$extension" - git clone --recurse-submodules -b "$release" https://github.com/"$org"/"$repo" /tmp/"$repo-$release" || exit 1 - cd /tmp/"$repo-$release" || exit 1 - phpize && ./configure && make -j"$(nproc)" && sudo make install - enable_extension "$extension" "$prefix" - ) >/dev/null 2>&1 - add_extension_log "$extension-$org/$repo@$release" "Installed and enabled" -} - # Function to setup phpize and php-config. add_devtools() { tool=$1