[RFC/PATCH v1] Add Travis CI support

[RFC/PATCH v1] Add Travis CI support

[larsxschneider]

From: Lars Schneider <larsxschneider@gmail.com>

Hi,

I recently broke a few tests...

In order to avoid that in the future I configured Travis CI for Git. With this
patch Travis can run all Git tests including the "git-p4" and "Git-LFS" tests.

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

My idea is that the owner of "https://github.com/git/git" enables this account
for Travis (it's free!). Then we would automatically get the test state for all
official branches.

Every contributor can enable Travis for their respective GitHub accounts. Then
they would know if their patches pass all tests in advance, too.

You can see the state of my branches here:
https://travis-ci.org/larsxschneider/git/branches

It's pretty red. The reason is that maint, master, and next have a failure on
OS X (test "t9815-git-p4-submit-fail.sh" does not pass). Furthmore pu does not
pass on either Linux or OS X (which is propably OK since it is pu).

You can also inspect the build/test logs. Here for instance the log for the
next branch compiled on Linux with gcc:
https://travis-ci.org/larsxschneider/git/jobs/82032861

Cheers,
Lars

Lars Schneider (1):
  Add Travis CI support

 .travis.yml | 28 ++++++++++++++++++++++++++++
 1 file changed, 28 insertions(+)
 create mode 100644 .travis.yml

--
2.5.1

[RFC/PATCH v1] Add Travis CI support

[larsxschneider]

From: Lars Schneider <larsxschneider@gmail.com>

The tests are 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 <larsxschneider@gmail.com>
---
 .travis.yml | 28 ++++++++++++++++++++++++++++
 1 file changed, 28 insertions(+)
 create mode 100644 .travis.yml

diff --git a/.travis.yml b/.travis.yml
new file mode 100644
index 0000000..056cc99
--- /dev/null
+++ b/.travis.yml
@@ -0,0 +1,28 @@
+language: c
+
+os:
+  - linux
+  - osx
+
+compiler:
+  - clang
+  - gcc
+
+before_script:
+  - "if [ ${TRAVIS_OS_NAME:-'linux'} = 'linux' ]; then wget -q https://package.perforce.com/perforce.pubkey -O - | sudo apt-key add -; fi"
+  - "if [ ${TRAVIS_OS_NAME:-'linux'} = 'linux' ]; then echo 'deb http://package.perforce.com/apt/ubuntu precise release' | sudo tee -a /etc/apt/sources.list; fi"
+  - "if [ ${TRAVIS_OS_NAME:-'linux'} = 'linux' ]; then sudo apt-get update -qq; fi"
+  - "if [ ${TRAVIS_OS_NAME:-'linux'} = 'linux' ]; then sudo apt-get install perforce-server; fi"
+  - "if [ ${TRAVIS_OS_NAME:-'linux'} = 'linux' ]; then wget -q https://packagecloud.io/gpg.key -O - | sudo apt-key add -; fi"
+  - "if [ ${TRAVIS_OS_NAME:-'linux'} = 'linux' ]; then sudo apt-get install -y apt-transport-https; fi"
+  - "if [ ${TRAVIS_OS_NAME:-'linux'} = 'linux' ]; then echo 'deb https://packagecloud.io/github/git-lfs/debian/ wheezy main' | sudo tee -a /etc/apt/sources.list; fi"
+  - "if [ ${TRAVIS_OS_NAME:-'linux'} = 'linux' ]; then sudo apt-get update -qq; fi"
+  - "if [ ${TRAVIS_OS_NAME:-'linux'} = 'linux' ]; then sudo apt-get install git-lfs; fi"
+  - "if [ ${TRAVIS_OS_NAME:-'linux'} = 'osx' ]; then brew update; fi"
+  - "if [ ${TRAVIS_OS_NAME:-'linux'} = 'osx' ]; then brew install git-lfs; fi"
+  - "if [ ${TRAVIS_OS_NAME:-'linux'} = 'osx' ]; then brew tap homebrew/binary; fi"
+  - "if [ ${TRAVIS_OS_NAME:-'linux'} = 'osx' ]; then sed -i.bak 's/b42758ebe7b54e672b513c34c88f399d0da7b4de1fd23b9f56d222a4f1f3bae5/e987475bfc54129d8d54a0d54363db3ecf6e6852a00daa0c6ffc20b8df1e0e63/' /usr/local/Library/Taps/homebrew/homebrew-binary/perforce-server.rb; fi"
+  - "if [ ${TRAVIS_OS_NAME:-'linux'} = 'osx' ]; then brew install perforce; fi"
+  - "if [ ${TRAVIS_OS_NAME:-'linux'} = 'osx' ]; then brew install perforce-server; fi"
+
+install: make configure
--
2.5.1

Re: [RFC/PATCH v1] Add Travis CI support

[Junio C Hamano]

larsxschneider@gmail.com writes:

> In order to avoid that in the future I configured Travis CI for Git. With this
> patch Travis can run all Git tests including the "git-p4" and "Git-LFS" tests.

Interesting.  I was wondering about the "p4" part myself.

> My idea is that the owner of "https://github.com/git/git" enables this account
> for Travis (it's free!). Then we would automatically get the test state for all
> official branches.

The last time I heard about this "it's free" thing, I thought I
heard that it wants write access to the repository.  If that is
still the case, the history stored in the GitHub repository the
"it's free" thing has access to can become even less trustworthy
than it currently is.  Those who clone/fetch from it cannot be sure
if the tips of branches are what I pushed there, or they were
changed to a malicious replacement from sideways by the "it's free"
thing, taking advantage of that write access.

Granted, those who clone/fetch cannot be sure unless they trust
GitHub.  The only assurance they have is GitHub's word: "gitster has
account with us, gitster pushes into this repository, and we have
ACL to ensure that gitster is the only person that can update this
repository".  Allowing write-access to a third-party will break that
assurance, even if you trust GitHub.

Of course, this can be improved if we start using signed push into
GitHub.  It is a separate issue in the sense that it would help
GitHub to make that assurance stronger---those who fetch/clone can
be assured that the tips of branches are what I pushed, without even
trusting GitHub.

Re: [RFC/PATCH v1] Add Travis CI support

[Dennis Kaarsemaker]

On do, 2015-09-24 at 17:41 -0700, Junio C Hamano wrote:
> larsxschneider@gmail.com writes:
> 
> > My idea is that the owner of "https://github.com/git/git" enables this account
> > for Travis (it's free!). Then we would automatically get the test state for all
> > official branches.
> 
> The last time I heard about this "it's free" thing, I thought I
> heard that it wants write access to the repository.

It does not need write access to the git data, only to auxiliary GitHub
data: commit status and deployment status (where it can put "this
commit failed tests"), repository hooks (to set up build triggers),
team membership (ro) and email addresses (ro).
-- 
Dennis Kaarsemaker
www.kaarsemaker.net

Re: [RFC/PATCH v1] Add Travis CI support

[Johannes Schindelin]

Hi,

On 2015-09-25 05:14, Dennis Kaarsemaker wrote:
> On do, 2015-09-24 at 17:41 -0700, Junio C Hamano wrote:
>> larsxschneider@gmail.com writes:
>>
>> > My idea is that the owner of "https://github.com/git/git" enables this account
>> > for Travis (it's free!). Then we would automatically get the test state for all
>> > official branches.
>>
>> The last time I heard about this "it's free" thing, I thought I
>> heard that it wants write access to the repository.
> 
> It does not need write access to the git data, only to auxiliary GitHub
> data: commit status and deployment status (where it can put "this
> commit failed tests"), repository hooks (to set up build triggers),
> team membership (ro) and email addresses (ro).

If that still elicits concerns, a fork could be set up that is automatically kept up-to-date via a web hook, and enable Travis CI there.

Junio, if that is something with which you feel more comfortable, I would be willing to set it up. Even if the visibility (read: impact) would be higher if the badges were attached to https://github.com/git/git proper...

Ciao,
Dscho

Re: [RFC/PATCH v1] Add Travis CI support

[Luke Diamand]

On 25 September 2015 at 08:27, Johannes Schindelin
<johannes.schindelin@gmx.de> wrote:
> Hi,
>
> On 2015-09-25 05:14, Dennis Kaarsemaker wrote:
>> On do, 2015-09-24 at 17:41 -0700, Junio C Hamano wrote:
>>> larsxschneider@gmail.com writes:
>>>
>>> > My idea is that the owner of "https://github.com/git/git" enables this account
>>> > for Travis (it's free!). Then we would automatically get the test state for all
>>> > official branches.
>>>
>>> The last time I heard about this "it's free" thing, I thought I
>>> heard that it wants write access to the repository.
>>
>> It does not need write access to the git data, only to auxiliary GitHub
>> data: commit status and deployment status (where it can put "this
>> commit failed tests"), repository hooks (to set up build triggers),
>> team membership (ro) and email addresses (ro).
>
> If that still elicits concerns, a fork could be set up that is automatically kept up-to-date via a web hook, and enable Travis CI there.
>
> Junio, if that is something with which you feel more comfortable, I would be willing to set it up. Even if the visibility (read: impact) would be higher if the badges were attached to https://github.com/git/git proper...
>

It would be less intrusive for the CI system to have a fork. Otherwise
other people using git with the same CI system will get annoying merge
conflicts, and we'll also end up with a repo littered with the control
files from past CI systems if the CI system is ever changed.

>From past experience, if it's configured to email people when things
break, sooner or later it will email the wrong people, probably once
every few seconds over a weekend.

Automated testing is a Good Thing, but it's still software, so needs
maintenance or it will break.


Luke

Re: [RFC/PATCH v1] Add Travis CI support

[Jeff King]

On Thu, Sep 24, 2015 at 05:41:06PM -0700, Junio C Hamano wrote:

> Of course, this can be improved if we start using signed push into
> GitHub.  It is a separate issue in the sense that it would help
> GitHub to make that assurance stronger---those who fetch/clone can
> be assured that the tips of branches are what I pushed, without even
> trusting GitHub.

It's been on my todo list to investigate this further, but I just
haven't gotten around to it. My understanding is that GitHub would need
to store your signed-push certificate somewhere (e.g., in a git tree
that records all of the push certs).

If the point is for clients not to trust GitHub, though, it doesn't
really matter what GitHub does with the cert, as long as it is put
somewhere that clients know to get it.  So I wonder if it would be
helpful to have a microformat that the client would use to look at this.
E.g., it would fetch the cert tree, then confirm that the current ref
values match the latest cert.

-Peff

Re: [RFC/PATCH v1] Add Travis CI support

[Junio C Hamano]

Luke Diamand <luke@diamand.org> writes:

> From past experience, if it's configured to email people when things
> break, sooner or later it will email the wrong people, probably once
> every few seconds over a weekend.
>
> Automated testing is a Good Thing, but it's still software, so needs
> maintenance or it will break.

That does sound like a valid concern (thanks for education---we
should all learn from others' past experience).  Unless it is just a
"set up and forget" thing, I do not think I'd want to be in charge
of it.

Thanks.

Re: [RFC/PATCH v1] Add Travis CI support

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> If the point is for clients not to trust GitHub, though, it doesn't
> really matter what GitHub does with the cert, as long as it is put
> somewhere that clients know to get it.

Correct.  A spiffy Web interface that says "Click this button and we
show you the output of GPG signature verification" would not help.
The push certificate is all about allowing third-parties to conduct
an independent audit, so anything the hosting site computes using
the certificates does not add value, unless the certificates
themselves are exported for such an independent audit.

If somebody found a change to "git push" that makes it pick the
user's wallet and sends a few coins every time it talks to the
hosting site, the hosting site can say it is not their doing by
showing that the tip of the commit that contains such a change came
from me, and it is not their evil doing.  Push certificates help the
hosting site prove their innocence, and those who do not trust the
site can still be convinced by the claim.

There is one scenario that signed push would not help very much,
though.  The hosting site cannot deny that it did not receive a
push.

Following such an incident (perhaps the evil change came as a side
effect of a innocuous looking patch), I would push a commit that
fixes such an issue out to the hosting site (with signed commit).
But if the hosting site deliberately keeps the tip of the branch
unmodified (e.g. you can appear to accept the push to the pusher,
without updating what is served to the general public), there will
be more people who will fetch from the hosting site to contaminate
their copy of git and the damage will spread in the meantime.

When I finally complain to the hosting site that it is deliberately
rejecting the fix that would rob them the illicit revenue source, it
does not help the hosting site to keep copies of push certificates
when it wants to refute such a complaint.  "We publish all push
certificates and there is no record that gitster already tried to
fix the issue" has to be taken with faith in that scenario.

So push certificate is not perfect.  But it does protect hosting
sites and projects hosted on them.

>  So I wonder if it would be
> helpful to have a microformat that the client would use to look at this.
> E.g., it would fetch the cert tree, then confirm that the current ref
> values match the latest cert.

Yeah, that is one possibility.  Just a single flat file that
concatenates all the push cert in the received order would do as an
export format, too ;-)

Re: [RFC/PATCH v1] Add Travis CI support

[Jeff King]

On Fri, Sep 25, 2015 at 11:29:31AM -0700, Junio C Hamano wrote:

> When I finally complain to the hosting site that it is deliberately
> rejecting the fix that would rob them the illicit revenue source, it
> does not help the hosting site to keep copies of push certificates
> when it wants to refute such a complaint.  "We publish all push
> certificates and there is no record that gitster already tried to
> fix the issue" has to be taken with faith in that scenario.

Right. Your earlier examples showed non-repudiation by the original
signer (the hosting site says "you definite pushed this to us, and here
is the signature to prove it, so you cannot deny it"). But in this
example, it is going the other way: the pusher wants the hosting site to
admit to an action.

To do that, the hosting site would have to re-sign the push cert to say
"we got this, it is published", and return the receipt to the original
pusher, who can then use it as proof of the event. Or alternatively, it
could be signed by a third-party notary.

I don't think it is all that interesting an avenue to pursue, though. If
you say "I have this update and the hosting site is not providing it to
people", people are not that interested in whether the hosting site is
being laggy, malicious, or whatever. They are interested in getting your
update. :)

So the more general problem is "I want to make sure I have Junio's
latest push, and I do not want to trust anything else". For that, you
could publish expiring certs (so you can fool me for up to, say, a week,
but after that I consider the old certs to be garbage either way). Or
you could do something clever with a quorum (e.g., N of K hosting sites
say there is no update, so there probably isn't one).

But I think all of that is outside of git's scope. Git provides the
signed ref-state in the form of a push cert. Since it's a small-ish blob
of data, you can use any external mechanism you want to decide on the
correct value of it.

> >  So I wonder if it would be
> > helpful to have a microformat that the client would use to look at this.
> > E.g., it would fetch the cert tree, then confirm that the current ref
> > values match the latest cert.
> 
> Yeah, that is one possibility.  Just a single flat file that
> concatenates all the push cert in the received order would do as an
> export format, too ;-)

I agree that's a more logical format, in a sense; it really is a linear
log. It's just that the receive-pack code already creates a blob for us,
so it's cheap to reference that in tree (and then fetching it is cheap,
too). IOW, git is much better at adding files to trees than it is at
appending to files. :)

-Peff

Re: [RFC/PATCH v1] Add Travis CI support

[Lars Schneider]

On 25 Sep 2015, at 10:05, Luke Diamand <luke@diamand.org> wrote:

> On 25 September 2015 at 08:27, Johannes Schindelin
> <johannes.schindelin@gmx.de> wrote:
>> Hi,
>> 
>> On 2015-09-25 05:14, Dennis Kaarsemaker wrote:
>>> On do, 2015-09-24 at 17:41 -0700, Junio C Hamano wrote:
>>>> larsxschneider@gmail.com writes:
>>>> 
>>>>> My idea is that the owner of "https://github.com/git/git" enables this account
>>>>> for Travis (it's free!). Then we would automatically get the test state for all
>>>>> official branches.
>>>> 
>>>> The last time I heard about this "it's free" thing, I thought I
>>>> heard that it wants write access to the repository.
>>> 
>>> It does not need write access to the git data, only to auxiliary GitHub
>>> data: commit status and deployment status (where it can put "this
>>> commit failed tests"), repository hooks (to set up build triggers),
>>> team membership (ro) and email addresses (ro).
>> 
>> If that still elicits concerns, a fork could be set up that is automatically kept up-to-date via a web hook, and enable Travis CI there.
>> 
>> Junio, if that is something with which you feel more comfortable, I would be willing to set it up. Even if the visibility (read: impact) would be higher if the badges were attached to https://github.com/git/git proper...
>> 
> 
> It would be less intrusive for the CI system to have a fork. Otherwise
> other people using git with the same CI system will get annoying merge
> conflicts, and we'll also end up with a repo littered with the control
> files from past CI systems if the CI system is ever changed.
> 
> From past experience, if it's configured to email people when things
> break, sooner or later it will email the wrong people, probably once
> every few seconds over a weekend.
> 
> Automated testing is a Good Thing, but it's still software, so needs
> maintenance or it will break.

I completely agree with your argument about emails and that software needs maintenance. We could setup this CI to not send any emails. We still could inspect the build/test state of each branch on the Travis CI website. I believe this is valuable because not everyone has e.g. a Mac system at hand to run all tests. This is no theoretical example because t9819 is broken on maint using a Mac.

Lars

Re: [RFC/PATCH v1] Add Travis CI support

[Shawn Pearce]

On Fri, Sep 25, 2015 at 11:52 AM, Jeff King <peff@peff.net> wrote:
> On Fri, Sep 25, 2015 at 11:29:31AM -0700, Junio C Hamano wrote:
>
>> >  So I wonder if it would be
>> > helpful to have a microformat that the client would use to look at this.
>> > E.g., it would fetch the cert tree, then confirm that the current ref
>> > values match the latest cert.
>>
>> Yeah, that is one possibility.  Just a single flat file that
>> concatenates all the push cert in the received order would do as an
>> export format, too ;-)
>
> I agree that's a more logical format, in a sense; it really is a linear
> log. It's just that the receive-pack code already creates a blob for us,
> so it's cheap to reference that in tree (and then fetching it is cheap,
> too). IOW, git is much better at adding files to trees than it is at
> appending to files. :)

FWIW JGit has a micro-format[1] we are starting to use. Its a tree of
the push cert blobs anchored under refs/meta/push-certs.

Inspired by a proposal from gitolite[2], where we store a file in
a tree for each ref name, and the contents of the file is the latest
push cert to affect that ref.

The main modification from that proposal (other than lacking the
out-of-git batching) is to append "@{cert}" to filenames, which allows
storing certificates for both refs/foo and refs/foo/bar. Those
refnames cannot coexist at the same time in a repository, but we do
not want to discard the push certificate responsible for deleting the
ref, which we would have to do if refs/foo in the push cert tree
changed from a tree to a blob.

[1] https://eclipse.googlesource.com/jgit/jgit/+/d5a71e9ca3d95330acdd858306c4f75ae0b01e58
[2] https://github.com/sitaramc/gitolite/blob/cf062b8bb6b21a52f7c5002d33fbc950762c1aa7/contrib/hooks/repo-specific/save-push-signatures

Re: [RFC/PATCH v1] Add Travis CI support

[Matthieu Moy]

Luke Diamand <luke@diamand.org> writes:

> It would be less intrusive for the CI system to have a fork. Otherwise
> other people using git with the same CI system will get annoying merge
> conflicts,

What conflicts are you talking about? The ones in .travis.yml? The point
is to share this file so that people using the same system do not have
to change anything.

And, we're talking about a straightforward 28-lines long file, set up
essentially once and for all. Even if people ever modify it, I don't
forsee conflict resolution in such a simple file as a real problem.

> and we'll also end up with a repo littered with the control files from
> past CI systems if the CI system is ever changed.

Again, we're talking about a short and simple configuration file.

Sure, when we change something, we either get old files lying around or
have to remove the old files. But would we say "Git shouldn't have a
Makefile, because having a Makefile would mean we'd end up with a repo
littered with Makefiles the day we migrate to another build system"?

> From past experience, if it's configured to email people when things
> break, sooner or later it will email the wrong people, probably once
> every few seconds over a weekend.

Are you talking about your experience with Travis-CI in particular, or
with CI systems in general? Is the scenario where Travis-CI sends email
based on actual facts, or only speculation?

My experience with Travis-CI is that it just works (my experience is
limited, but I'm using it for git-multimail, and it's a really
convenient tool). It does send emails by default, but with a very
reasonable policy:

  http://docs.travis-ci.com/user/notifications/

  "By default, email notifications are sent to the committer and the
  commit author, if they are members of the repository (that is, they
  have push or admin permissions for public repositories, or if they
  have pull, push or admin permissions for private repositories)."

In short:

* If the tests always pass, nobody ever get any email from Travis-CI.

* When someone sends a pull-request that fails tests, that someone gets
  an automatic email about the failure. This saves one email round-trip
  "X sends a patch series, Junio notices the failure, Junio sends an
  email about the failure", and shortcuts this as "X sends a PR, and
  gets an email, possibly even before Junio notices".

> Automated testing is a Good Thing, but it's still software, so needs
> maintenance or it will break.

The point of using Travis-CI is precisely to use an externally
maintained system. It's not just software, it's a service (based on
software, obviously).

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/

Re: [RFC/PATCH v1] Add Travis CI support

[Stefan Beller]

On Sun, Sep 27, 2015 at 5:11 AM, Matthieu Moy
<Matthieu.Moy@grenoble-inp.fr> wrote:
>
> My experience with Travis-CI is that it just works

I can second that.
When I was contributing to other projects[1][2], Travis helped a lot.

Currently I have a cronjob to get https://scan.coverity.com/
running on Git a few times a week on the pu branch
(plus $gmane/271826). Additionally to that I could setup
a travis test to that, which would run daily on stefanbeller/pu
(which would be a copy of junios pu branch).

I just logged in to travis and it seems as if they don't require
write access to the repository (any more? They used to require
it, but now they ask for updated permissions which drops
write access to a repository, but then asks for more meta
data permissions, such as web hooks, my email address,
my organizations).

Having observed that there is no reason to not turn it on on
the main repository (set it and forget it).

[1] https://github.com/bjorn/tiled
[2] https://github.com/clintbellanger/flare-engine



>
>   http://docs.travis-ci.com/user/notifications/
>
>   "By default, email notifications are sent to the committer and the
>   commit author, if they are members of the repository (that is, they
>   have push or admin permissions for public repositories, or if they
>   have pull, push or admin permissions for private repositories)."
>
> In short:
>
> * If the tests always pass, nobody ever get any email from Travis-CI.
>
> * When someone sends a pull-request that fails tests, that someone gets
>   an automatic email about the failure. This saves one email round-trip
>   "X sends a patch series, Junio notices the failure, Junio sends an
>   email about the failure", and shortcuts this as "X sends a PR, and
>   gets an email, possibly even before Junio notices".
>
>> Automated testing is a Good Thing, but it's still software, so needs
>> maintenance or it will break.
>
> The point of using Travis-CI is precisely to use an externally
> maintained system. It's not just software, it's a service (based on
> software, obviously).
>
> --
> Matthieu Moy
> http://www-verimag.imag.fr/~moy/
> --
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC/PATCH v1] Add Travis CI support

[Matthieu Moy]

Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:

> * If the tests always pass, nobody ever get any email from Travis-CI.

Actually, I've just been reminded that the repository owner gets one
email per new ref (tag, branch) by default.

Deactivating completely email notification is as simple as (in
.travis.yml):

notifications:
  email: false

and not getting emails when tests pass is done with

notifications:
  email:
    on_success: never

It probably makes sense to do the later in the case of Git, so that
Junio doesn't get spammed when pushing topic branches to
https://github.com/gitster/git.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/

Re: [RFC/PATCH v1] Add Travis CI support

[Junio C Hamano]

Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:

> It probably makes sense to do the later in the case of Git, so that
> Junio doesn't get spammed when pushing topic branches to
> https://github.com/gitster/git.

I won't enable it on github.com:gitster/git anyway, so I do not
think that is a concern.  I thought what people are talking about
was to add it on github.com:git/git, but have I been misreading the
thread?  I do not even own the latter repository (I only can push
into it).

Re: [RFC/PATCH v1] Add Travis CI support

[Matthieu Moy]

Junio C Hamano <gitster@pobox.com> writes:

> Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:
>
>> It probably makes sense to do the later in the case of Git, so that
>> Junio doesn't get spammed when pushing topic branches to
>> https://github.com/gitster/git.
>
> I won't enable it on github.com:gitster/git anyway, so I do not
> think that is a concern.  I thought what people are talking about
> was to add it on github.com:git/git, but have I been misreading the
> thread?  I do not even own the latter repository (I only can push
> into it).

You're right: github.com:gitster/git shouldn't be affected. Builds are
triggered for branches outside github.com:git/git only when a
pull-requests to git/git is submitted.

So, you'd get a "success" email only when pushing a new tag (since the
set of branches does not change).

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/

Re: [RFC/PATCH v1] Add Travis CI support

[Sebastian Schuberth]

On 25.09.2015 05:14, Dennis Kaarsemaker wrote:

>>> My idea is that the owner of "https://github.com/git/git" enables this account
>>> for Travis (it's free!). Then we would automatically get the test state for all
>>> official branches.
>>
>> The last time I heard about this "it's free" thing, I thought I
>> heard that it wants write access to the repository.
>
> It does not need write access to the git data, only to auxiliary GitHub
> data: commit status and deployment status (where it can put "this
> commit failed tests"), repository hooks (to set up build triggers),
> team membership (ro) and email addresses (ro).

Also, as Roberto explained at [1], "If you set up the webhook yourself, 
you don't need to grant the [repository hooks] permissions".

BTW, there's already an attempt at creating a .travis.yml file at [2].

[1] https://github.com/rtyley/submitgit/issues/16#issuecomment-120119634
[2] https://github.com/git/git/pull/154

-- 
Sebastian Schuberth

Re: [RFC/PATCH v1] Add Travis CI support

[Roberto Tyley]

On 28 September 2015 at 19:47, Junio C Hamano <gitster@pobox.com> wrote:
> I won't enable it on github.com:gitster/git anyway, so I do not
> think that is a concern.  I thought what people are talking about
> was to add it on github.com:git/git, but have I been misreading the
> thread?  I do not even own the latter repository (I only can push
> into it).

I was momentarily surprised to hear that Junio doesn't own github.com/git/git
but I had a quick look at the github.com/git organisation, and it turns
out that Peff and Scott Chacon are the current owners - so at the
moment I think they're the only ones who could switch on the GitHub
webhook to hit Travis.

For what it's worth, I'd love to see Travis CI - or any form of CI -
running for the core Git project. It doesn't require giving write
access to Travis, and beyond the good reasons given by Lars,
I'm also personally interested because it opens up the possibility
of some useful enhancements to the submitGit flow - so that you
can't send email to the list without knowing you've broken tests
first.

Regarding Luke's concerns about excess emails coming from CI,
default Travis behaviour is for emails to be sent to the committer and
author, but only if they have write access to the repository the commit
was pushed to:

http://docs.travis-ci.com/user/notifications/#How-is-the-build-email-receiver-determined%3F

If Travis emails do become problematic, you can disable them
completely by adding 2 lines of config to the .travis.yml:

http://docs.travis-ci.com/user/notifications/#Email-notifications

Given this, enabling Travis CI for git/git seems pretty low risk,
are there any strong objections to it happening?

Re: [RFC/PATCH v1] Add Travis CI support

[Junio C Hamano]

On Sat, Oct 3, 2015 at 3:23 PM, Roberto Tyley <roberto.tyley@gmail.com> wrote:
>
> Given this, enabling Travis CI for git/git seems pretty low risk,
> are there any strong objections to it happening?

I still don't see a reason why git/git needs to be the one that is
used, when somebody
so interested (and I seem to see very many of them in the thread) can
sacrifice his or
her own fork and enable it him or herself.

Re: [RFC/PATCH v1] Add Travis CI support

[Junio C Hamano]

Junio C Hamano <gitster@pobox.com> writes:

> On Sat, Oct 3, 2015 at 3:23 PM, Roberto Tyley <roberto.tyley@gmail.com> wrote:
>>
>> Given this, enabling Travis CI for git/git seems pretty low risk,
>> are there any strong objections to it happening?
>
> I still don't see a reason why git/git needs to be the one that is
> used, when somebody
> so interested (and I seem to see very many of them in the thread) can
> sacrifice his or
> her own fork and enable it him or herself.

To state it a bit differently.

If somebody says "I've been maintaining a clone of git/git with
Travis webhooks enabled and as the result caught this many glitches
during the past two months without any ill side effect.  Here are
the patches to fix them, and by the way, the first patch in this
series is not a fix but the configuration to tell Travis how to run
tests so that other people can enable it on _their_ own fork before
they send their own series to the mailing list." in the cover letter
of a patch series, I would appreciate such a series greatly and
would not mind carrying one extra yml file in the tree at all.

But that is not what I am seeing in this thread at all.  I am tired
of hearing people telling others to help them by doing more without
doing the grunt work themselves.

Re: [RFC/PATCH v1] Add Travis CI support

[Jeff King]

On Sat, Oct 03, 2015 at 11:23:52PM +0100, Roberto Tyley wrote:

> On 28 September 2015 at 19:47, Junio C Hamano <gitster@pobox.com> wrote:
> > I won't enable it on github.com:gitster/git anyway, so I do not
> > think that is a concern.  I thought what people are talking about
> > was to add it on github.com:git/git, but have I been misreading the
> > thread?  I do not even own the latter repository (I only can push
> > into it).
> 
> I was momentarily surprised to hear that Junio doesn't own github.com/git/git
> but I had a quick look at the github.com/git organisation, and it turns
> out that Peff and Scott Chacon are the current owners - so at the
> moment I think they're the only ones who could switch on the GitHub
> webhook to hit Travis.

There is a @git/git team on GitHub that should have full access to the
git/git repository, and Junio is on that (but I also do not _expect_
Junio to spend time managing it; he has plenty of other things to do).

I am on vacation at the moment, but am happy to look at it when I get
back in a few weeks.

-Peff

Re: [RFC/PATCH v1] Add Travis CI support

[Matthieu Moy]

Junio C Hamano <gitster@pobox.com> writes:

> On Sat, Oct 3, 2015 at 3:23 PM, Roberto Tyley <roberto.tyley@gmail.com> wrote:
>>
>> Given this, enabling Travis CI for git/git seems pretty low risk,
>> are there any strong objections to it happening?
>
> I still don't see a reason why git/git needs to be the one that is
> used,

The very nice thing with Travis-CI is that it does not only test the
repository's branches, but also all pull-requests. So, if it is
activated on git/git, it will become possible to have a flow like

1) User pushes to his own repo, sends a pull-request,

2) Travis-CI notices the pull-request and builds it (no action needed
   from anyone),

3) Once the build is finished, the user can use e.g. SubmitGit to
   actually submit the code.

This has real benefits for the submitter (know if your code is broken
early), for the reviewers (things like "you have a def-after-use" would
be noticed by a computer before human beings start spending time on the
review), and for you (some issues noticed before a topic enters pu).

There's no extra work for the user at all compared to the standard
pull-request flow (nothing to do, just submit a PR), and a one-time
setup for the project.

Currenty, to mimick this flow, we would need something like

1) User activates Travis-CI on his repo (each user would have to do
   this, not just once)

2) User commits .travis.yml on top of the code to submit

3) User pushes to his repo

4) Travis-CI triggers a build

5) User removes the commit introducing .travis.yml, force-pushes

6) User submit the resulting code.

This is much more work for the user (read: nobody will do it, actually
nobody do it currently) and less convenient for reviewers (who have no
way to check whether the build passed).

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/

Re: [RFC/PATCH v1] Add Travis CI support

[Dennis Kaarsemaker]

On za, 2015-10-03 at 18:37 -0700, Junio C Hamano wrote:
> If somebody says "I've been maintaining a clone of git/git with
> Travis webhooks enabled and as the result caught this many glitches
> during the past two months without any ill side effect.

I've been maintaining a clone of git/git with a different ci system
enabled, and it hasn't really caught anything. Only the occasional test
failure in pu like the one I mailed about yesterday.

The automated testing of pull requests could be useful, but pull
requests don't seem to be used much yet.
-- 
Dennis Kaarsemaker
www.kaarsemaker.net

Re: [RFC/PATCH v1] Add Travis CI support

[Johannes Schindelin]

Hi Junio,

On 2015-10-04 03:37, Junio C Hamano wrote:
> Junio C Hamano <gitster@pobox.com> writes:
> 
>> On Sat, Oct 3, 2015 at 3:23 PM, Roberto Tyley <roberto.tyley@gmail.com> wrote:
>>>
>>> Given this, enabling Travis CI for git/git seems pretty low risk,
>>> are there any strong objections to it happening?
>>
>> I still don't see a reason why git/git needs to be the one that is
>> used, when somebody
>> so interested (and I seem to see very many of them in the thread) can
>> sacrifice his or
>> her own fork and enable it him or herself.
> 
> To state it a bit differently.
> 
> If somebody says "I've been maintaining a clone of git/git with
> Travis webhooks enabled and as the result caught this many glitches
> during the past two months without any ill side effect.

Heh... given that Travis CI requires that .travis.yml file, nobody can really say that they have been using Travis CI *before* you add that file to `master`. If you make successful testing with Travis a *precondition* before adding that file, it is kinda asking for the impossible.

Now, I like Travis, even if I have used Jenkins previously (came as part of my previous day-job). And my experience with Jenkins (in the form of BuildHive) was pretty positive: it *did* catch a couple of breakages. Even with my Git fork.

But I agree with basically everybody who chimed in and said that the biggest bang for the buck would be made by enabling it on https://github.com/git/git.

The only cost I see is for that `.travis.yml` file to live in Git's source code. Small price to pay, if you ask me. If you do not want to use it yourself, that is fine. But I would like to ask for it to be included so that those of us who do want to benefit from Travis' testing are not precluded from doing so [*1*].

As far as I can tell, the patch is fine as-is. Although I would put the `before_script` commands into some file inside `contrib/`.

Thanks,
Dscho

Footnote *1*: of course it would be possible to manually rebase the patch, or to set up a scripted version of that. That is very cumbersome, though, and the benefit would obviously be substantially diminished.

Re: [RFC/PATCH v1] Add Travis CI support

[Junio C Hamano]

Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:

> Junio C Hamano <gitster@pobox.com> writes:
>>
>> I still don't see a reason why git/git needs to be the one that is
>> used,
>
> The very nice thing with Travis-CI is that it does not only test the
> repository's branches, but also all pull-requests.

OK, that is the first real argument I heard for enabling it on
git/git that is worth listening to.

Practically, it has little value to run CI (whose only test is to
run "make test") on branches that I publish in that repository.  By
the time a change hits that repository, "make test" has been run on
my end already, and the only thing the CI would catch is platform
dependent glitches (e.g. Windows and Mac), dependency-related ones
(e.g. p4), or breakages I already know about [*1*].

But we _do_ want to see tested patches submitted to the list so that
reviewers do not have to waste time on obviously bogus patches
reviewing (and the integrator wasting time on deconflicting).  A
test that is PR-initiated would give us a real value there.

The repository that is used for the PR-initiated test does not have
to be git/git (it only has to be a central well-known repository),
but similar to arrangement for SubmitGit, I agree that git/git would
be a good candidate for that "central well-known" one.  There is not
much point in introducing another "if you want your topics tested,
throw a PR against this other repository".

So,... I would not mind a patch that adds a CI configuration file (I
would really prefer it to be a battle-tested one, though) to my
tree, and I would not mind if CI is enabled on git/git, if Peff or
somebody more security-minded than me thinks it is safe to do so.

One final question.  Which configuration file does the CI use when
running a PR-initiated test?  The one already in the repository
i.e. the target of the proposed pull, or the one that is possibly
updated by the PR?

I am wondering if that can be an avenue for a possible mischief.

Thanks.


[Footnote]

*1* I occasionally do push out 'pu' with known breakages (e.g. the
recent 'lmdb' one) to make sure people are running the test suite so
that they will work with the topic author to resolve the issue
without having to wait for me to tell the topic author about it;
letting CI catch that kind of breakage would not add much value,
because it is already known ;-)

Re: [RFC/PATCH v1] Add Travis CI support

[Junio C Hamano]

Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:

> Currenty, to mimick this flow, we would need something like
>
> 1) User activates Travis-CI on his repo (each user would have to do
>    this, not just once)
>
> 2) User commits .travis.yml on top of the code to submit
>
> 3) User pushes to his repo
>
> 4) Travis-CI triggers a build
>
> 5) User removes the commit introducing .travis.yml, force-pushes
>
> 6) User submit the resulting code.

I do not think it has to be so convoluted.  I know this would appear
to be more or less a moot point, as the long term direction would be
to enable one on git/git and do PR-initiated tests, but I am writing
it here because I would really prefer that the CI configuration file
that will be added to my tree is a "battle tested" one.

A motivated user who wants to send a patch to add it to my tree can:

 (1) Fork from an ancient place, e.g. v2.0.0, and add the CI
     configuration file.  Call that branch "travis".

 (2) Prepare topics that he wants to test (not related to "add CI
     integration to Git" topic) on its own topics, branching from my
     'master' or 'maint' depending on the target track.

 (3) Keep a branch that merges (2) with (1).  This could be a set of
     branches, one per topics in (2).

 (4) Have the CI monitor (3).

 (5) Make sure tests pass.  Send (2) out via whatever means,
     e.g. via SubmitGit.

And keep the set-up for a few months to make sure everything looks
good, before sending (1) out via SubmitGit.

Re: [RFC/PATCH v1] Add Travis CI support

[Dennis Kaarsemaker]

On zo, 2015-10-04 at 10:46 -0700, Junio C Hamano wrote:
> One final question.  Which configuration file does the CI use when
> running a PR-initiated test?  The one already in the repository
> i.e. the target of the proposed pull, or the one that is possibly
> updated by the PR?
>
> I am wondering if that can be an avenue for a possible mischief.

The latter. And it can, as it can enable notifications.

-- 
Dennis Kaarsemaker
www.kaarsemaker.net

Re: [RFC/PATCH v1] Add Travis CI support

[Matthieu Moy]

Dennis Kaarsemaker <dennis@kaarsemaker.net> writes:

> On zo, 2015-10-04 at 10:46 -0700, Junio C Hamano wrote:
>> One final question.  Which configuration file does the CI use when
>> running a PR-initiated test?  The one already in the repository
>> i.e. the target of the proposed pull, or the one that is possibly
>> updated by the PR?
>>
>> I am wondering if that can be an avenue for a possible mischief.
>
> The latter. And it can, as it can enable notifications.

OK, so an attacker can send emails (by faking one of the repository
owner's identity on a commit, and then submitting a pull-request for
this commit). But such attacker could already send emails via GitHub to
all repository watchers (not just owners) by sending pull-requests. Or
by using his mailer.

Other than that, Travis-CI uses a container-based infrastructure to
ensure clean and independent builds. So, an attacker could trigger a
build doing "rm -fr /" or whatever without impacting other builds.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/

Re: [RFC/PATCH v1] Add Travis CI support

[Junio C Hamano]

Dennis Kaarsemaker <dennis@kaarsemaker.net> writes:

> On zo, 2015-10-04 at 10:46 -0700, Junio C Hamano wrote:
>> One final question.  Which configuration file does the CI use when
>> running a PR-initiated test?  The one already in the repository
>> i.e. the target of the proposed pull, or the one that is possibly
>> updated by the PR?
>>
>> I am wondering if that can be an avenue for a possible mischief.
>
> The latter. And it can, as it can enable notifications.

So it can add a slight annoyance if somebody wanted to, but not much
over the annoyance a random pull-request can already give to project
participants.  IOW, nothing to worry about.

Thanks.

Re: [RFC/PATCH v1] Add Travis CI support

[Sebastian Schuberth]

On 10/4/2015 19:46, Junio C Hamano wrote:

>> The very nice thing with Travis-CI is that it does not only test the
>> repository's branches, but also all pull-requests.
>
> OK, that is the first real argument I heard for enabling it on
> git/git that is worth listening to.

I was mentioning that very argument in the context of PRs filed for use 
with submitgit already back in July in the conversation at [1] in which 
you took part.

[1] https://github.com/rtyley/submitgit/issues/16

Regards,
Sebastian