From c659e0146674c6bda49c156eca3f90d10eb31ec5 Mon Sep 17 00:00:00 2001 From: Bas van Dijk Date: Tue, 14 Apr 2020 12:44:29 +0200 Subject: [PATCH 1/2] refactor: the install.sh derivation * refactors the `install.sh` Nix derivations to be easier to follow. * Remove the hack for shellcheck local lint rules and makes sure shellcheck and shfmt are correctly applied to the script. * Fixes linting errors that arose during the above step. * Slightly adapt `publish.nix` to copy `manifest.json` directly from a local path instead of from the `install.sh` derivation. There's no reason for these to be coupled like this. * Because of the above the `s3cp` utility had to be refactored. Before it took a `pkg`, `path` and `dstDir` parameter. Now this is simplified to a `src` and `dst` parameter. --- ci/release.nix | 5 -- default.nix | 7 +- public/default.nix | 128 +++++++++-------------------------- public/install/020_flags.sh | 17 +++-- public/install/100_log.sh | 2 +- public/install/999_footer.sh | 6 +- publish.nix | 28 ++++---- 7 files changed, 63 insertions(+), 130 deletions(-) diff --git a/ci/release.nix b/ci/release.nix index 18b86f1c23..01875e5a2d 100644 --- a/ci/release.nix +++ b/ci/release.nix @@ -11,10 +11,5 @@ let ci = import ./ci.nix { inherit src releaseVersion; }; in if !doRelease then {} else { - # TODO: remove these jobs when the `publish.dfx` job below - # is working successfully and the CloudFront CDN is online. - # See: https://dfinity.atlassian.net/browse/INF-1143 - inherit (ci) dfx-release install-sh-release; - publish.dfx.x86_64-linux = ci.publish.dfx.x86_64-linux; } diff --git a/default.nix b/default.nix index 492eb0f0f5..f9308c4053 100644 --- a/default.nix +++ b/default.nix @@ -1,5 +1,5 @@ { system ? builtins.currentSystem -, src ? null +, src ? builtins.fetchGit ./. , releaseVersion ? "latest" , RustSec-advisory-db ? null , pkgs ? import ./nix { inherit system RustSec-advisory-db; } @@ -17,8 +17,7 @@ rec { inherit (pkgs) nix-fmt nix-fmt-check; - public = import ./public { inherit pkgs src releaseVersion; }; - inherit (public) install-sh-release install-sh; + install = import ./public { inherit pkgs src; }; # This is to make sure CI evaluates shell derivations, builds their # dependencies and populates the hydra cache with them. We also use this in @@ -37,6 +36,6 @@ rec { publish = import ./publish.nix { inherit pkgs releaseVersion; - inherit (jobset) dfx-release install-sh-release; + inherit (jobset) dfx-release install; }; } diff --git a/public/default.nix b/public/default.nix index 0ec2181333..b517241abf 100644 --- a/public/default.nix +++ b/public/default.nix @@ -1,27 +1,29 @@ { pkgs ? import ../nix { inherit system; } , system ? builtins.currentSystem -, src ? null -, releaseVersion ? "latest" +, src ? builtins.fetchGit ../. }: - let - public = pkgs.lib.noNixFiles (pkgs.lib.gitOnlySource ../. "public"); - version = releaseVersion; - gitDir = pkgs.lib.gitDir ../.; -in - -rec { - install-sh = - pkgs.runCommandNoCC "install-sh" { - inherit public; + install = + pkgs.runCommandNoCC "install.sh.template" { + src = pkgs.lib.gitOnlySource ../. ./install; preferLocalBuild = true; allowSubstitutes = false; + nativeBuildInputs = [ pkgs.shfmt pkgs.shellcheck ]; + allowedReferences = []; } '' - # git describe --abbrev=7 --tags - mkdir -p $out + echo "Running shellcheck..." + shellcheck --shell=sh $src/*.sh \ + --exclude SC2039 \ + --exclude SC2154 \ + --exclude SC2034 + + echo "Running shfmt..." + shfmt -d -p -i 4 -ci -bn -s $src/*.sh - cat $public/install/*.sh > $out/install.sh + echo "Creating output..." + cat $src/*.sh > $out + echo "Fixing up..." # Get rid of comments that don't start with '##' or '#!'. sed -i " /#!.*/p @@ -29,84 +31,20 @@ rec { /^ *$/d /^ *#/d s/ *#.*// - " $out/install.sh + " $out ''; - - # Check if the install.sh script is well formatted and - # has no shellcheck issues. We ignore 'local' warning by shellcheck, because - # any existing sh implementation supports it. - # TODO: streamline mkRelease and this - install-sh-lint = - let - shfmtOpts = "-p -i 4 -ci -bn -s"; - shellcheckOpts = "-s sh -S warning"; - in - pkgs.runCommandNoCC "install-sh-lint" { - inherit version public; - buildInputs = [ install-sh pkgs.shfmt pkgs.shellcheck ]; - preferLocalBuild = true; - allowSubstitutes = false; - } '' - set -Eeuo pipefail - # Check if we have an sh compatible script - shckResult="$(shellcheck -Cnever -f gcc ${shellcheckOpts} "$public/install.sh" | \ - grep -v "warning: In POSIX sh, 'local' is undefined. \[SC2039\]" | \ - sed -e "s%^${install-sh}/?%%g" || true)" - - if [ -n "$shckResult" ] ; then - echo "There are some shellcheck warnings:" - echo - echo "$shckResult" - echo - exit 1 - fi - - # Check if the file is properly formatted - if ! shfmt ${shfmtOpts} -d "${install-sh}/install.sh"; then - echo "Formatting error. Please run:" - echo - echo "shfmt ${shfmtOpts} -w public/install.sh" - exit 1 - fi - - if grep source "${install-sh}/install.sh"; then - echo "Found a source above in the output. There should be none remaining (inlined)." - exit 1 - fi - - # Make sure Nix sees the output. - touch $out - ''; - - # The following prepares a manifest for copying install.sh - install-sh-release = - pkgs.lib.linuxOnly ( - pkgs.runCommandNoCC "install-sh-release" { - inherit version; - - # `revision` will be printed by `install.sh` as follows: - # - # log "Executing DFINITY SDK install script, commit: @revision@" - revision = - if src != null - then src.rev - else pkgs.lib.commitIdFromGitRepo (pkgs.lib.gitDir ../.); - - manifest = ./manifest.json; - buildInputs = [ pkgs.jo install-sh-lint install-sh ]; - preferLocalBuild = true; - allowSubstitutes = false; - } '' - set -Eeuo pipefail - - mkdir -p $out - - version_manifest_file=$out/manifest.json - - cp $manifest $version_manifest_file - # we stamp the file with the revision - substitute "${install-sh}/install.sh" $out/install.sh \ - --subst-var revision - '' - ); -} +in +pkgs.lib.linuxOnly ( + pkgs.runCommandNoCC "install.sh" { + # `revision` will be printed by `install.sh` as follows: + # + # log "Executing DFINITY SDK install script, commit: @revision@" + revision = src.rev; + preferLocalBuild = true; + allowSubstitutes = false; + inherit install; + } '' + # we stamp the file with the revision + substitute $install $out --subst-var revision + '' +) diff --git a/public/install/020_flags.sh b/public/install/020_flags.sh index 39fdd72246..1a946371db 100644 --- a/public/install/020_flags.sh +++ b/public/install/020_flags.sh @@ -12,19 +12,20 @@ DFX_BOOL_FLAGS="" # $2 - A description of the flag. This is not currently used but will be when we have enough # flags to implement help. define_flag_BOOL() { - local VARNAME="flag_$(echo $1 | tr /a-z/ /A-Z)" - eval $VARNAME="\${$VARNAME:-}" + local VARNAME + VARNAME="flag_$(echo "$1" | tr /a-z/ /A-Z)" + eval "$VARNAME=\${$VARNAME:-}" DFX_BOOL_FLAGS="${DFX_BOOL_FLAGS}--${1} $VARNAME $2" } # Get the flag name of a line in the flag description. get_flag_name() { - echo $1 + echo "$1" } # Get the variable name of a line in the flag description. get_var_name() { - echo $2 + echo "$2" } # Read all the command line flags and set the flag_XXXX environment variables. @@ -35,6 +36,8 @@ get_var_name() { # Environment variables are set according to flags defined and flags. read_flags() { # Set values from command line. + # shellcheck disable=SC2199 + # https://github.com/koalaman/shellcheck/wiki/SC2199 while [[ "$@" ]]; do local ARG=$1 shift @@ -45,11 +48,11 @@ read_flags() { [ "$line" ] || break IFS="$OLD_IFS" - FLAG=$(get_flag_name $line) - VARNAME=$(get_var_name $line) + FLAG="$(get_flag_name "$line")" + VARNAME="$(get_var_name "$line")" if [ "$ARG" == "$FLAG" ]; then - eval $VARNAME="1" + eval "$VARNAME=1" fi done done diff --git a/public/install/100_log.sh b/public/install/100_log.sh index 01398638fa..92c676b36f 100644 --- a/public/install/100_log.sh +++ b/public/install/100_log.sh @@ -1,7 +1,7 @@ ## 100_log.sh log() { - if $_ansi_escapes_are_valid; then + if "$_ansi_escapes_are_valid"; then printf "\33[1minfo:\33[0m %s\n" "$1" 1>&2 else printf '%s\n' "$1" 1>&2 diff --git a/public/install/999_footer.sh b/public/install/999_footer.sh index 2ff532b85d..5dc8162d4c 100644 --- a/public/install/999_footer.sh +++ b/public/install/999_footer.sh @@ -27,7 +27,7 @@ get_tag_from_manifest_json() { get_manifest_version() { local _version - _version="$(downloader ${DFX_MANIFEST_JSON_URL} - | get_tag_from_manifest_json latest)" || return 2 + _version="$(downloader "${DFX_MANIFEST_JSON_URL}" - | get_tag_from_manifest_json latest)" || return 2 printf %s "${_version}" } @@ -36,8 +36,8 @@ validate_install_dir() { local dir="${1%/}" # We test it's a directory and writeable. - ! [ -d $dir ] && return 1 - ! [ -w $dir ] && return 2 + ! [ -d "$dir" ] && return 1 + ! [ -w "$dir" ] && return 2 # We also test it's in the $PATH of the user. case ":$PATH:" in diff --git a/publish.nix b/publish.nix index d57cd3b654..ca1653a50e 100644 --- a/publish.nix +++ b/publish.nix @@ -5,19 +5,18 @@ # This script will be executed by DFINITY's Continuous Deployment # system. That system will also set the correct AWS credentials and the # DFINITY_DOWNLOAD_BUCKET environment variable. -{ pkgs, releaseVersion, dfx-release, install-sh-release }: +{ pkgs, releaseVersion, dfx-release, install }: let s3cp = pkgs.lib.writeCheckedShellScriptBin "s3cp" [] '' set -eu PATH="${pkgs.lib.makeBinPath [ pkgs.awscli ]}" - pkg="$1"; path="$2"; dstDir="$3"; contentType="$4"; cacheControl="$5" - src="$pkg/$path" - dst="s3://$DFINITY_DOWNLOAD_BUCKET/$dstDir/$path" + src="$1"; dst="$2"; contentType="$3"; cacheControl="$4" + dstUrl="s3://$DFINITY_DOWNLOAD_BUCKET/$dst" if [ -d "$src" ]; then - echo "Can't copy $src to $dst because it's a directory. Please specify a file instead." 1>&2; exit 1; + echo "Can't copy $src to $dstUrl because it's a directory. Please specify a file instead." 1>&2; exit 1; fi - echo "Uploading $src to $dst (--cache-control $cacheControl, --content-type $contentType)..." - aws s3 cp "$src" "$dst" \ + echo "Uploading $src to $dstUrl (--cache-control $cacheControl, --content-type $contentType)..." + aws s3 cp "$src" "$dstUrl" \ --cache-control "$cacheControl" \ --no-guess-mime-type --content-type "$contentType" \ --no-progress @@ -49,16 +48,16 @@ in v="${releaseVersion}" cache_long="max-age=31536000" # 1 year - path="dfx-$v.tar.gz" + file="dfx-$v.tar.gz" dir="sdk/dfx/$v" - s3cp "${dfx-release.x86_64-linux}" "$path" "$dir/x86_64-linux" "application/gzip" "$cache_long" - s3cp "${dfx-release.x86_64-darwin}" "$path" "$dir/x86_64-darwin" "application/gzip" "$cache_long" + s3cp "${dfx-release.x86_64-linux}/$file" "$dir/x86_64-linux/$file" "application/gzip" "$cache_long" + s3cp "${dfx-release.x86_64-darwin}/$file" "$dir/x86_64-darwin/$file" "application/gzip" "$cache_long" slack "$SLACK_CHANNEL_BUILD_NOTIFICATIONS_WEBHOOK" < Date: Tue, 14 Apr 2020 22:07:44 +0200 Subject: [PATCH 2/2] build: make install.sh executable so we can test it --- public/default.nix | 1 + 1 file changed, 1 insertion(+) diff --git a/public/default.nix b/public/default.nix index b517241abf..c11d8e8727 100644 --- a/public/default.nix +++ b/public/default.nix @@ -46,5 +46,6 @@ pkgs.lib.linuxOnly ( } '' # we stamp the file with the revision substitute $install $out --subst-var revision + chmod +x $out '' )