Re: [PATCH v6 6/6] Add Travis CI support

2015-11-20 Thread Lars Schneider 

On 19 Nov 2015, at 15:35, Jeff King  wrote:

> On Thu, Nov 19, 2015 at 09:58:11AM +0100, larsxschnei...@gmail.com wrote:
> 
>> From: Lars Schneider 
>> 
>> The tests are currently executed on "Ubuntu 12.04 LTS Server Edition
>> 64 bit" and on "OS X Mavericks" using gcc and clang.
>> 
>> Perforce and Git-LFS are installed and therefore available for the
>> respective tests.
> 
> My overall impression is that this is a lot more complicated than I was
> expecting. Can we start with something simpler to gain experience with
> Travis?  And then we can add in more complexity later.
> 
> For example:
> 
>> +addons:
>> +  apt:
>> +packages:
>> +- language-pack-is
> 
> I doubt most people are running the t0204 right now. Maybe we should
> start without it.

Well, I think the entire point of a CI is to run automatically tests that most 
people are *not* running. Plus I can't recall any problems with 
language-pack-is and t0204.

> 
>> +env:
>> +  global:
>> +- P4_VERSION="15.1"
>> +- GIT_LFS_VERSION="1.0.2"
> 
> I know p4 is your area of interest, but from a project perspective, it
> seems like an odd choice for the initial set of tests.

See above. Most people do not execute the p4 tests because they don't have p4 
installed. Therefore these tests have the highest risk to brake unnoticed.

> 
>> +- DEFAULT_TEST_TARGET=prove
>> +- GIT_PROVE_OPTS="--timer --jobs 3"
>> +- GIT_TEST_OPTS="--verbose --tee"
> 
> These all make sense, I guess.
> 
>> +- GETTEXT_ISO_LOCALE=YesPlease
>> +- GETTEXT_LOCALE=YesPlease
> 
> What are these? I don't think we have any such Makefile knobs. These
> look like variables that get used internally by the test suite, but we
> shouldn't need to set them.

These are used in t020*. I enabled them to run the tests and I haven't seen any 
problems. However, I am no expert in that area. I'll remove them.

> 
>> +# - GETTEXT_POISON=YesPlease
> 
> I'm assuming the "#" means this is commented out. Can we just drop such
> cruft?

Yes, for the final submission of course. I just wanted to make it stick out as 
I wasn't able to run the tests successfully with this flag. I wonder if someone 
can explain to me how it works.

> 
>> +- GIT_TEST_CHAIN_LINT=YesPlease
> 
> This is the default, and we don't need to specify it.

OK

> 
>> +- GIT_TEST_CLONE_2GB=YesPlease
> 
> Is it a good idea to run such an expensive test?

Well, it's a CI machine that does not block anyone. I doubt that people run 
these tests on their local machines for exactly the reason you just described. 
Therefore I would rather run these tests.

> 
>> +  matrix:
>> +-
>> +  # NO_ICONV=YesPlease
> 
> Ditto here on the commenting.

OK. Setting NO_ICONV brakes a lot of tests. Do we really "support" that flag?

> 
>> +- >
>> +  NO_CURL=YesPlease
> 
> So if I understand correctly, the point of this list is to test
> alternate configurations. So compiling without curl makes some sense, I
> guess. Though mostly it will just shut off a bunch off tests.
> 
>> +  NO_D_INO_IN_DIRENT=YesPlease
> 
> But setting platform compat knobs like this seems weird. Nobody sets
> this manually. Either your platform has it, or it does not. And we are
> already testing on alternate platforms to cover such things.
> 
> If there's a specific config setup of interest, it makes sense to me to
> try to increase test coverage there. But it feels like you've just
> picked a random laundry list of Makefile knobs and tweaked them, without
> any sense of whether they even make sense together, or match the
> platform.
> 
> For instance:
> 
>> +  NO_STRCASESTR=YesPlease
>> +  NO_STRLCPY=YesPlease
> 
> I wouldn't not be surprised if these cause problems building on some
> platforms that _do_ have these functions.

Agreed. As mentioned in gmane/280963 [1] I am no expert in these flags.  Should 
I remove the "NO_*" test configuration all together?

[1] http://article.gmane.org/gmane.comp.version-control.git/280963/match=no_

> 
>> +echo "$(tput setaf 6)Perfoce Server Version$(tput sgr0)";
>> +p4d -V | grep Rev.;
>> +echo "$(tput setaf 6)Perfoce Client Version$(tput sgr0)";
>> +p4 -V | grep Rev.;
> 
> s/Perfoce/Perforce/ :)

Thanks :)


>> +before_script: make configure && ./configure && make --jobs=2
> 
> Hmm. I wonder if we actually want to use autoconf here at all; most devs
> do not use it, and Git should generally compile out of the box based on
> config.mak.uname (and if it doesn't, it would be nice to know).
> 
> There may be some optional packages that autoconf helps with, though.

Compiling without autoconf works for Linux but not for OS X:
https://travis-ci.org/larsxschneider/git/jobs/9770


> So overall, I dunno. I do not want to create a bunch of work for you in
> splitting it up. But I'm hesitant to merge something that has a bunch of
> lines that I'm not sure I understand the reasoning for. :-/

I understand. How about this: I create a new roll where I...
... remove the

Re: [PATCH v6 6/6] Add Travis CI support

2015-11-19 Thread Jeff King 
On Thu, Nov 19, 2015 at 09:58:11AM +0100, larsxschnei...@gmail.com wrote:

> From: Lars Schneider 
> 
> The tests are currently executed on "Ubuntu 12.04 LTS Server Edition
> 64 bit" and on "OS X Mavericks" using gcc and clang.
> 
> Perforce and Git-LFS are installed and therefore available for the
> respective tests.

My overall impression is that this is a lot more complicated than I was
expecting. Can we start with something simpler to gain experience with
Travis?  And then we can add in more complexity later.

For example:

> +addons:
> +  apt:
> +packages:
> +- language-pack-is

I doubt most people are running the t0204 right now. Maybe we should
start without it.

> +env:
> +  global:
> +- P4_VERSION="15.1"
> +- GIT_LFS_VERSION="1.0.2"

I know p4 is your area of interest, but from a project perspective, it
seems like an odd choice for the initial set of tests.

> +- DEFAULT_TEST_TARGET=prove
> +- GIT_PROVE_OPTS="--timer --jobs 3"
> +- GIT_TEST_OPTS="--verbose --tee"

These all make sense, I guess.

> +- GETTEXT_ISO_LOCALE=YesPlease
> +- GETTEXT_LOCALE=YesPlease

What are these? I don't think we have any such Makefile knobs. These
look like variables that get used internally by the test suite, but we
shouldn't need to set them.

> +# - GETTEXT_POISON=YesPlease

I'm assuming the "#" means this is commented out. Can we just drop such
cruft?

> +- GIT_TEST_CHAIN_LINT=YesPlease

This is the default, and we don't need to specify it.

> +- GIT_TEST_CLONE_2GB=YesPlease

Is it a good idea to run such an expensive test?

> +  matrix:
> +-
> +  # NO_ICONV=YesPlease

Ditto here on the commenting.

> +- >
> +  NO_CURL=YesPlease

So if I understand correctly, the point of this list is to test
alternate configurations. So compiling without curl makes some sense, I
guess. Though mostly it will just shut off a bunch off tests.

> +  NO_D_INO_IN_DIRENT=YesPlease

But setting platform compat knobs like this seems weird. Nobody sets
this manually. Either your platform has it, or it does not. And we are
already testing on alternate platforms to cover such things.

If there's a specific config setup of interest, it makes sense to me to
try to increase test coverage there. But it feels like you've just
picked a random laundry list of Makefile knobs and tweaked them, without
any sense of whether they even make sense together, or match the
platform.

For instance:

> +  NO_STRCASESTR=YesPlease
> +  NO_STRLCPY=YesPlease

I wouldn't not be surprised if these cause problems building on some
platforms that _do_ have these functions.

> +echo "$(tput setaf 6)Perfoce Server Version$(tput sgr0)";
> +p4d -V | grep Rev.;
> +echo "$(tput setaf 6)Perfoce Client Version$(tput sgr0)";
> +p4 -V | grep Rev.;

s/Perfoce/Perforce/ :)

> +before_script: make configure && ./configure && make --jobs=2

Hmm. I wonder if we actually want to use autoconf here at all; most devs
do not use it, and Git should generally compile out of the box based on
config.mak.uname (and if it doesn't, it would be nice to know).

There may be some optional packages that autoconf helps with, though.


So overall, I dunno. I do not want to create a bunch of work for you in
splitting it up. But I'm hesitant to merge something that has a bunch of
lines that I'm not sure I understand the reasoning for. :-/

I was hoping the first cut could just be telling Travis to run "make
test", without much fanfare beyond that. Maybe that's not realistic.

-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v6 6/6] Add Travis CI support

2015-11-19 Thread larsxschneider 
From: Lars Schneider 

The tests are currently executed on "Ubuntu 12.04 LTS Server Edition
64 bit" and on "OS X Mavericks" using gcc and clang.

Perforce and Git-LFS are installed and therefore available for the
respective tests.

Signed-off-by: Lars Schneider 
---
 .travis.yml | 131 
 1 file changed, 131 insertions(+)
 create mode 100644 .travis.yml

diff --git a/.travis.yml b/.travis.yml
new file mode 100644
index 000..61c70fa
--- /dev/null
+++ b/.travis.yml
@@ -0,0 +1,131 @@
+language: c
+
+os:
+  - linux
+  - osx
+
+compiler:
+  - clang
+  - gcc
+
+addons:
+  apt:
+packages:
+- language-pack-is
+
+env:
+  global:
+- P4_VERSION="15.1"
+- GIT_LFS_VERSION="1.0.2"
+- DEFAULT_TEST_TARGET=prove
+- GIT_PROVE_OPTS="--timer --jobs 3"
+- GIT_TEST_OPTS="--verbose --tee"
+- GETTEXT_ISO_LOCALE=YesPlease
+- GETTEXT_LOCALE=YesPlease
+# - GETTEXT_POISON=YesPlease
+- GIT_TEST_CHAIN_LINT=YesPlease
+- GIT_TEST_CLONE_2GB=YesPlease
+# - GIT_TEST_LONG=YesPlease
+  matrix:
+-
+  # NO_ICONV=YesPlease
+- >
+  NO_CURL=YesPlease
+  NO_D_INO_IN_DIRENT=YesPlease
+  NO_DEFLATE_BOUND=YesPlease
+  NO_EXPAT=YesPlease
+  NO_GECOS_IN_PWENT=YesPlease
+  NO_GETTEXT=YesPlease
+  NO_HMAC_CTX_CLEANUP=YesPlease
+  NO_HSTRERROR=YesPlease
+  NO_INET_NTOP=YesPlease
+  NO_INET_PTON=YesPlease
+  NO_INITGROUPS=YesPlease
+  NO_INTTYPES_H=YesPlease
+  NO_IPV6=YesPlease
+  NO_IPV6=YesPlease
+  NO_LIBGEN_H=YesPlease
+  NO_MEMMEM=YesPlease
+  NO_MKDTEMP=YesPlease
+  NO_MKSTEMPS=YesPlease
+  NO_MMAP=YesPlease
+  NO_NSEC=YesPlease
+  NO_OPENSSL=YesPlease
+  NO_PERL=YesPlease
+  NO_PTHREADS=YesPlease
+  NO_REGEX=YesPlease
+  NO_SETENV=YesPlease
+  NO_SETITIMER=YesPlease
+  NO_SOCKADDR_STORAGE=YesPlease
+  NO_STRCASESTR=YesPlease
+  NO_STRLCPY=YesPlease
+  NO_STRTOUMAX=YesPlease
+  NO_STRUCT_ITIMERVAL=YesPlease
+  NO_SYMLINK_HEAD=YesPlease
+  NO_SYS_POLL_H=YesPlease
+  NO_SYS_SELECT_H=YesPlease
+  NO_UINTMAX_T=YesPlease
+  NO_UNSETENV=YesPlease
+
+before_install:
+  - >
+case "${TRAVIS_OS_NAME:-linux}" in
+linux)
+  mkdir --parents custom/p4
+  pushd custom/p4
+wget --quiet 
http://filehost.perforce.com/perforce/r$P4_VERSION/bin.linux26x86_64/p4d
+wget --quiet 
http://filehost.perforce.com/perforce/r$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$GIT_LFS_VERSION/git-lfs-linux-amd64-$GIT_LFS_VERSION.tar.gz
+tar --extract --gunzip --file 
"git-lfs-linux-amd64-$GIT_LFS_VERSION.tar.gz"
+cp git-lfs-$GIT_LFS_VERSION/git-lfs .
+export PATH="$(pwd):$PATH"
+  popd
+  ;;
+osx)
+  brew_force_set_latest_binary_hash () {
+FORMULA=$1
+SHA=$(brew fetch --force $FORMULA 2>&1 | grep ^SHA256: | cut -d ' ' -f 
2)
+sed -E -i.bak "s/sha256 \"[0-9a-f]{64}\"/sha256 \"$SHA\"/g" \
+  /usr/local/Library/Taps/homebrew/homebrew-binary/$FORMULA.rb
+  }
+  brew update --quiet
+  brew tap homebrew/binary --quiet
+  brew_force_set_latest_binary_hash perforce
+  brew_force_set_latest_binary_hash perforce-server
+  brew install git-lfs perforce-server perforce
+  ;;
+esac;
+echo "$(tput setaf 6)Perfoce Server Version$(tput sgr0)";
+p4d -V | grep Rev.;
+echo "$(tput setaf 6)Perfoce Client Version$(tput sgr0)";
+p4 -V | grep Rev.;
+echo "$(tput setaf 6)Git-LFS Version$(tput sgr0)";
+git-lfs version;
+
+before_script: make configure && ./configure && make --jobs=2
+
+script: make --quiet test
+
+after_failure:
+  - >
+: '<-- Click here to see detailed test output! 
   ';
+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;
+done;
+
+notifications:
+  email: false
-- 
2.5.1

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html