Re: [PATCH v4 4/4] Add Travis CI support

[Eric Sunshine]

On Fri, Nov 6, 2015 at 3:58 AM,   wrote:
> 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 
> ---
> diff --git a/.travis.yml b/.travis.yml
> @@ -0,0 +1,131 @@
> +  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

Hmm, aren't you losing test coverage by disabling some of these? Is
that desirable for continuous integration testing? Am I missing
something?
--
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

Re: [PATCH v4 4/4] Add Travis CI support

[Sebastian Schuberth]

On Fri, Nov 6, 2015 at 2:11 PM, Lars Schneider  wrote:

> Per platform/compiler (Linux/clang) we run two configurations. One
> normal configuration (see the lonely "-" right under "matrix:") and one
> configuration with all sorts of things are disabled ("NO...").
>
> You can see all 8 configurations ([linux, mac] * [clang, gcc] * [normal,
> NO_...]) here:
> https://travis-ci.org/larsxschneider/git/builds/89598194

Aren't these 8 configurations a bit too much? I see the total running
time is about 2 hours. For my taste, this is way to much to give
developers feedback about the status of their PR. It should be
something < 30 minutes.

IMO, the purpose of the Travis CI configuration mainly is to 1) save
developers work by not requiring to build manually, 2) build on other
platforms than the developer has access to. I doubt that the average
developer manually builds anything close to these 8 configurations
before we had this job, so I wouldn't make it a requirement for Travis
to do much more than a developer could / would to manually.

On the other hand, I see the point in letting a CI system test more
configurations than manually feasible. But that should be done as part
of a different job then. E.g. we could have a "fast" PR validation
job, and a "slow" nightly build job.

-- 
Sebastian Schuberth
--
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

Re: [PATCH v4 4/4] Add Travis CI support

[Sebastian Schuberth]

On Fri, Nov 6, 2015 at 2:28 PM, Lars Schneider  wrote:

> Well, I partly agree. Right now the running time is ~20 min (that means less 
> than your 30min target!). After ~10min you even have all Linux results, Mac 
> takes a bit longer. Travis shows you 2h because that is the time that would 
> be required if all builds where run one after another (we run builds in 
> parallel).

Are you sure about than? I mean, what sense does it make to show how
long it *would* have taken *if* the builds were running serially? I
can see that the longest of the jobs took 21 minutes, which is ok. But
that does not mean that all jobs completed in within 21 minutes. It
could be that not all jobs started at (about) the same time due to a
lack of resources, and that the last job did not compete before the 2
hours were over because it only started to run 1 hours and 40 minutes
befor ethe first job was started.

> That being said, I see your point of to avoiding to burn Travis CI resources 
> meaningless. If I am not mistaken then you can configure Travis in a way that 
> it runs different configurations for different branches. E.g. I would like to 
> run all 8 configurations on maint, master, next and maybe pu. All other 
> branches on peoples own forks should be fine with the default Linux build 
> (~10min).
>
> What do you think?

I think running different configuration per branch makes sense, yes.

-- 
Sebastian Schuberth
--
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

Re: [PATCH v4 4/4] Add Travis CI support

[Lars Schneider]

> On 06 Nov 2015, at 14:57, Sebastian Schuberth  wrote:
> 
> On Fri, Nov 6, 2015 at 2:55 PM, Lars Schneider  
> wrote:
> 
>>> I think running different configuration per branch makes sense, yes.
>> 
>> If the list decides to accept this patch. Do you think that would be a 
>> necessary requirement for the first iteration?
> 
> No. I think this could be addressed later as an improvements. To me
> it's more important to finally get *some* sane Travis CI configuration
> in.
True. However, as I stated in my v4 cover letter the Travis CI integration is 
not yet perfect. I am constantly running builds to find flaky tests. Eg. here 
is one of them in git-p4 area that I will tackle next:
https://s3.amazonaws.com/archive.travis-ci.org/jobs/89603763/log.txt

I also see a weird "prove Tests our of sequence" error one in a while:
https://s3.amazonaws.com/archive.travis-ci.org/jobs/89603770/log.txt

Does anyone have an idea what could cause this?

Thanks,
Lars

--
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

Re: [PATCH v4 4/4] Add Travis CI support

[Lars Schneider]

> On 06 Nov 2015, at 14:20, Sebastian Schuberth  wrote:
> 
> On Fri, Nov 6, 2015 at 2:11 PM, Lars Schneider  
> wrote:
> 
>> Per platform/compiler (Linux/clang) we run two configurations. One
>> normal configuration (see the lonely "-" right under "matrix:") and one
>> configuration with all sorts of things are disabled ("NO...").
>> 
>> You can see all 8 configurations ([linux, mac] * [clang, gcc] * [normal,
>> NO_...]) here:
>> https://travis-ci.org/larsxschneider/git/builds/89598194
> 
> Aren't these 8 configurations a bit too much? I see the total running
> time is about 2 hours. For my taste, this is way to much to give
> developers feedback about the status of their PR. It should be
> something < 30 minutes.
> 
> IMO, the purpose of the Travis CI configuration mainly is to 1) save
> developers work by not requiring to build manually, 2) build on other
> platforms than the developer has access to. I doubt that the average
> developer manually builds anything close to these 8 configurations
> before we had this job, so I wouldn't make it a requirement for Travis
> to do much more than a developer could / would to manually.
> 
> On the other hand, I see the point in letting a CI system test more
> configurations than manually feasible. But that should be done as part
> of a different job then. E.g. we could have a "fast" PR validation
> job, and a "slow" nightly build job.
> 

Well, I partly agree. Right now the running time is ~20 min (that means less 
than your 30min target!). After ~10min you even have all Linux results, Mac 
takes a bit longer. Travis shows you 2h because that is the time that would be 
required if all builds where run one after another (we run builds in parallel).

That being said, I see your point of to avoiding to burn Travis CI resources 
meaningless. If I am not mistaken then you can configure Travis in a way that 
it runs different configurations for different branches. E.g. I would like to 
run all 8 configurations on maint, master, next and maybe pu. All other 
branches on peoples own forks should be fine with the default Linux build 
(~10min).

What do you think?

Thanks,
Lars--
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

Re: [PATCH v4 4/4] Add Travis CI support

[Lars Schneider]

> On 06 Nov 2015, at 14:36, Sebastian Schuberth  wrote:
> 
> On Fri, Nov 6, 2015 at 2:28 PM, Lars Schneider  
> wrote:
> 
>> Well, I partly agree. Right now the running time is ~20 min (that means less 
>> than your 30min target!). After ~10min you even have all Linux results, Mac 
>> takes a bit longer. Travis shows you 2h because that is the time that would 
>> be required if all builds where run one after another (we run builds in 
>> parallel).
> 
> Are you sure about than? I mean, what sense does it make to show how
> long it *would* have taken *if* the builds were running serially? I
> can see that the longest of the jobs took 21 minutes, which is ok. But
> that does not mean that all jobs completed in within 21 minutes. It
> could be that not all jobs started at (about) the same time due to a
> lack of resources, and that the last job did not compete before the 2
> hours were over because it only started to run 1 hours and 40 minutes
> befor ethe first job was started.
I am fairly certain about this. 

See, here is the first configuration and the first test case of a job:
https://travis-ci.org/larsxschneider/git/jobs/89598195
[08:21:06] t0002-gitfile.sh 

Here is the last configuration and the last test case of the same job:
[08:51:22] t9903-bash-prompt.sh

~30 min for all 8 configurations. You can enable Travis CI for you GitHub 
account and try it easily yourself :-)

> 
>> That being said, I see your point of to avoiding to burn Travis CI resources 
>> meaningless. If I am not mistaken then you can configure Travis in a way 
>> that it runs different configurations for different branches. E.g. I would 
>> like to run all 8 configurations on maint, master, next and maybe pu. All 
>> other branches on peoples own forks should be fine with the default Linux 
>> build (~10min).
>> 
>> What do you think?
> 
> I think running different configuration per branch makes sense, yes.
If the list decides to accept this patch. Do you think that would be a 
necessary requirement for the first iteration?

Thanks,
Lars--
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

Re: [PATCH v4 4/4] Add Travis CI support

[Lars Schneider]

> On 06 Nov 2015, at 10:56, Eric Sunshine  wrote:
> 
> On Fri, Nov 6, 2015 at 3:58 AM,   wrote:
>> 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 
>> ---
>> diff --git a/.travis.yml b/.travis.yml
>> @@ -0,0 +1,131 @@
>> +  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
> 
> Hmm, aren't you losing test coverage by disabling some of these? Is
> that desirable for continuous integration testing? Am I missing
> something?
Per platform/compiler (Linux/clang) we run two configurations. One 
normal configuration (see the lonely "-" right under "matrix:") and one 
configuration with all sorts of things are disabled ("NO...").

You can see all 8 configurations ([linux, mac] * [clang, gcc] * [normal, 
NO_...]) here:
https://travis-ci.org/larsxschneider/git/builds/89598194

Cheers,
Lars 

--
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

Re: [PATCH v4 4/4] Add Travis CI support

[Sebastian Schuberth]

On Fri, Nov 6, 2015 at 2:55 PM, Lars Schneider  wrote:

>> I think running different configuration per branch makes sense, yes.
>
> If the list decides to accept this patch. Do you think that would be a 
> necessary requirement for the first iteration?

No. I think this could be addressed later as an improvements. To me
it's more important to finally get *some* sane Travis CI configuration
in.

-- 
Sebastian Schuberth
--
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