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


--
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: [RFC/PATCH v1] Add Travis CI support

[Matthieu Moy]

Dennis Kaarsemaker  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/
--
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: [RFC/PATCH v1] Add Travis CI support

[Junio C Hamano]

Dennis Kaarsemaker  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.
--
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: [RFC/PATCH v1] Add Travis CI support

[Matthieu Moy]

Junio C Hamano  writes:

> On Sat, Oct 3, 2015 at 3:23 PM, Roberto Tyley  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/
--
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: [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


--
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: [RFC/PATCH v1] Add Travis CI support

[Johannes Schindelin]

Hi Junio,

On 2015-10-04 03:37, Junio C Hamano wrote:
> Junio C Hamano  writes:
> 
>> On Sat, Oct 3, 2015 at 3:23 PM, Roberto Tyley  
>> 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.
--
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: [RFC/PATCH v1] Add Travis CI support

[Junio C Hamano]

Matthieu Moy  writes:

> Junio C Hamano  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 ;-)


--
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: [RFC/PATCH v1] Add Travis CI support

[Junio C Hamano]

Matthieu Moy  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.
--
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: [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


--
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: [RFC/PATCH v1] Add Travis CI support

[Roberto Tyley]

On 28 September 2015 at 19:47, Junio C Hamano  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?
--
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: [RFC/PATCH v1] Add Travis CI support

[Junio C Hamano]

On Sat, Oct 3, 2015 at 3:23 PM, Roberto Tyley  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 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: [RFC/PATCH v1] Add Travis CI support

[Junio C Hamano]

Junio C Hamano  writes:

> On Sat, Oct 3, 2015 at 3:23 PM, Roberto Tyley  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.
--
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: [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  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
--
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: [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
--
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: [RFC/PATCH v1] Add Travis CI support

[Matthieu Moy]

Matthieu Moy  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/
--
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: [RFC/PATCH v1] Add Travis CI support

[Junio C Hamano]

Matthieu Moy  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).
--
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: [RFC/PATCH v1] Add Travis CI support

[Stefan Beller]

On Sun, Sep 27, 2015 at 5:11 AM, Matthieu Moy
 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 majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
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: [RFC/PATCH v1] Add Travis CI support

[Matthieu Moy]

Junio C Hamano  writes:

> Matthieu Moy  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/
--
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: [RFC/PATCH v1] Add Travis CI support

[Matthieu Moy]

Luke Diamand  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/
--
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: [RFC/PATCH v1] Add Travis CI support

[Lars Schneider]

On 25 Sep 2015, at 10:05, Luke Diamand  wrote:

> On 25 September 2015 at 08:27, Johannes Schindelin
>  wrote:
>> Hi,
>> 
>> On 2015-09-25 05:14, Dennis Kaarsemaker wrote:
>>> On do, 2015-09-24 at 17:41 -0700, Junio C Hamano wrote:
 larsxschnei...@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--
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: [RFC/PATCH v1] Add Travis CI support

[Shawn Pearce]

On Fri, Sep 25, 2015 at 11:52 AM, Jeff King  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
--
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: [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
--
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: [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
--
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: [RFC/PATCH v1] Add Travis CI support

[Junio C Hamano]

Luke Diamand  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.



--
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: [RFC/PATCH v1] Add Travis CI support

[Junio C Hamano]

Jeff King  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 ;-)
--
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: [RFC/PATCH v1] Add Travis CI support

[Luke Diamand]

On 25 September 2015 at 08:27, Johannes Schindelin
 wrote:
> Hi,
>
> On 2015-09-25 05:14, Dennis Kaarsemaker wrote:
>> On do, 2015-09-24 at 17:41 -0700, Junio C Hamano wrote:
>>> larsxschnei...@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
--
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: [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:
>> larsxschnei...@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
--
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: [RFC/PATCH v1] Add Travis CI support

[Dennis Kaarsemaker]

On do, 2015-09-24 at 17:41 -0700, Junio C Hamano wrote:
> larsxschnei...@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


--
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: [RFC/PATCH v1] Add Travis CI support

[Junio C Hamano]

larsxschnei...@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.
--
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