git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: larsxschneider@gmail.com
Cc: git@vger.kernel.org
Subject: Re: [PATCH v1 1/2] travis-ci: move Travis CI code into dedicated scripts
Date: Mon, 11 Sep 2017 10:17:38 +0900	[thread overview]
Message-ID: <xmqqy3pm3ufh.fsf@gitster.mtv.corp.google.com> (raw)
In-Reply-To: <20170910144429.47346-2-larsxschneider@gmail.com> (larsxschneider@gmail.com's message of "Sun, 10 Sep 2017 16:44:28 +0200")

larsxschneider@gmail.com writes:

> From: Lars Schneider <larsxschneider@gmail.com>
>
> Most of the Travis CI commands are in the '.travis.yml'. The yml format
> does not support functions and therefore code duplication is necessary
> to run commands across all builds.
>
> To fix this, add a library for common CI functions. Move all Travis CI
> code into dedicated scripts and make them call the library first.
>
> Signed-off-by: Lars Schneider <larsxschneider@gmail.com>
> ---

Thanks.  I _think_ you ended up not having to use shell function to
avoid code duplication ;-) but I find the script part of the result
much easier to understand.

Two things I noticed:

 - run-windows-build.sh did not use to run with "set -e" but now it
   does because it includes lib-travisci.sh; if (I didn't check with
   fine toothed comb) the original were not "set -e" clean, we may
   see unwanted errors in run-windows-build.sh, but that is
   sometihng we can fix going forward.

 - it seems that the test output section lost ": <- click here...";
   I do not know if there is a negative consequence of this change.

Indentation of some scripts were a bit too deep and I found it hard
to read them, so the following is what I did (but did not commit)
while reading them through.

Thanks.

 ci/install-dependencies.sh | 51 ++++++++++++++++++++++++----------------------
 ci/print-test-failures.sh  | 18 ++++++++--------
 2 files changed, 36 insertions(+), 33 deletions(-)

diff --git a/ci/install-dependencies.sh b/ci/install-dependencies.sh
index 067e6eb702..a29246af35 100755
--- a/ci/install-dependencies.sh
+++ b/ci/install-dependencies.sh
@@ -5,33 +5,36 @@
 
 . ${0%/*}/lib-travisci.sh
 
+P4WHENCE=http://filehost.perforce.com/perforce/r$LINUX_P4_VERSION
+LFSWHENCE=https://github.com/github/git-lfs/releases/download/v$LINUX_GIT_LFS_VERSION
+
 case "${TRAVIS_OS_NAME:-linux}" in
-	linux)
-		export GIT_TEST_HTTPD=YesPlease
+linux)
+	export GIT_TEST_HTTPD=YesPlease
 
-		mkdir --parents custom/p4
-		pushd custom/p4
-			wget --quiet http://filehost.perforce.com/perforce/r$LINUX_P4_VERSION/bin.linux26x86_64/p4d
-			wget --quiet http://filehost.perforce.com/perforce/r$LINUX_P4_VERSION/bin.linux26x86_64/p4
-			chmod u+x p4d
-			chmod u+x p4
-			export PATH="$(pwd):$PATH"
-		popd
-		mkdir --parents custom/git-lfs
-		pushd custom/git-lfs
-			wget --quiet https://github.com/github/git-lfs/releases/download/v$LINUX_GIT_LFS_VERSION/git-lfs-linux-amd64-$LINUX_GIT_LFS_VERSION.tar.gz
-			tar --extract --gunzip --file "git-lfs-linux-amd64-$LINUX_GIT_LFS_VERSION.tar.gz"
-			cp git-lfs-$LINUX_GIT_LFS_VERSION/git-lfs .
-			export PATH="$(pwd):$PATH"
-		popd
+	mkdir --parents custom/p4
+	pushd custom/p4
+		wget --quiet "$P4WHENCE/bin.linux26x86_64/p4d"
+		wget --quiet "$P4WHENCE/bin.linux26x86_64/p4"
+		chmod u+x p4d
+		chmod u+x p4
+		export PATH="$(pwd):$PATH"
+	popd
+	mkdir --parents custom/git-lfs
+	pushd custom/git-lfs
+		wget --quiet "$LFSWHENCE/git-lfs-linux-amd64-$LINUX_GIT_LFS_VERSION.tar.gz"
+		tar --extract --gunzip --file "git-lfs-linux-amd64-$LINUX_GIT_LFS_VERSION.tar.gz"
+		cp git-lfs-$LINUX_GIT_LFS_VERSION/git-lfs .
+		export PATH="$(pwd):$PATH"
+	popd
 	;;
-	osx)
-		brew update --quiet
-		# Uncomment this if you want to run perf tests:
-		# brew install gnu-time
-		brew install git-lfs gettext
-		brew link --force gettext
-		brew install caskroom/cask/perforce
+osx)
+	brew update --quiet
+	# Uncomment this if you want to run perf tests:
+	# brew install gnu-time
+	brew install git-lfs gettext
+	brew link --force gettext
+	brew install caskroom/cask/perforce
 	;;
 esac
 
diff --git a/ci/print-test-failures.sh b/ci/print-test-failures.sh
index 8583e61839..8c8973cbf3 100755
--- a/ci/print-test-failures.sh
+++ b/ci/print-test-failures.sh
@@ -6,13 +6,13 @@
 . ${0%/*}/lib-travisci.sh
 
 for TEST_EXIT in t/test-results/*.exit
-	do
-		if [ "$(cat "$TEST_EXIT")" != "0" ]
-		then
-			TEST_OUT="${TEST_EXIT%exit}out"
-			echo "------------------------------------------------------------------------"
-			echo "$(tput setaf 1)${TEST_OUT}...$(tput sgr0)"
-			echo "------------------------------------------------------------------------"
-			cat "${TEST_OUT}"
-		fi
+do
+	if [ "$(cat "$TEST_EXIT")" != "0" ]
+	then
+		TEST_OUT="${TEST_EXIT%exit}out"
+		echo "------------------------------------------------------------------------"
+		echo "$(tput setaf 1)${TEST_OUT}...$(tput sgr0)"
+		echo "------------------------------------------------------------------------"
+		cat "${TEST_OUT}"
+	fi
 done

  reply	other threads:[~2017-09-11  1:17 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-09-10 14:44 [PATCH v1 0/2] travis-ci: dedicated scripts + skip duplicated builds larsxschneider
2017-09-10 14:44 ` [PATCH v1 1/2] travis-ci: move Travis CI code into dedicated scripts larsxschneider
2017-09-11  1:17   ` Junio C Hamano [this message]
2017-09-10 14:44 ` [PATCH v1 2/2] travis-ci: skip a branch build if equal tag is present larsxschneider
2017-09-11 14:52   ` SZEDER Gábor
2017-09-12 11:45     ` Lars Schneider

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=xmqqy3pm3ufh.fsf@gitster.mtv.corp.google.com \
    --to=gitster@pobox.com \
    --cc=git@vger.kernel.org \
    --cc=larsxschneider@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).