threads / patch / 45462

patchtravis-ci: build and test Git on Windows

Subject: [PATCH v1] travis-ci: build and test Git on Windows

## tl;dr

24 messages between Mar 22, 2017 and Mar 24, 2017. Diffs are folded; open one to read it.

replies: 23people: 5as markdown or json

Lars Schneider· Mar 22, 2017, 06:56 UTC · lore

Most Git developers work on Linux and they have no way to know if their changes would break the Git for Windows build. Let's fix that by adding a job to TravisCI that builds and tests Git on Windows. Unfortunately, TravisCI does not support Windows.

Therefore, we did the following:
* Johannes Schindelin set up a Visual Studio Team Services build
  sponsored by Microsoft and made it accessible via an Azure Function
  that speaks a super-simple API. We made TravisCI use this API to
  trigger a build, wait until its completion, and print the build and
  test results.
* A Windows build and test run takes up to 3h and TravisCI has a timeout
  after 50min for Open Source projects. Since the TravisCI job does not
  use heavy CPU/memory/etc. resources, the friendly TravisCI folks
  extended the job timeout for git/git to 3h.
Things, that would need to be done:
* Someone with write access to https://travis-ci.org/git/git would need
  to add the secret token as "GFW_CI_TOKEN" variable in the TravisCI
  repository setting [1]. Afterwards the build should just work.
Things, that might need to be done:
* The Windows box can only process a single build at a time. A second
  Windows build would need to wait until the first finishes. This
  waiting time and the build time after the wait could exceed the 3h
  threshold. If this is a problem, then it is likely to happen every day
  as usually multiple branches are pushed at the same time (pu/next/
  master/maint). I cannot test this as my TravisCI account has the 50min
  timeout. One solution could be to limit the number of concurrent
  TravisCI jobs [2].

[1] https://docs.travis-ci.com/user/environment-variables#Defining-Variables-in-Repository-Settings [2] https://docs.travis-ci.com/user/customizing-the-build#Limiting-Concurrent-Builds

Signed-off-by: Lars Schneider <larsxschneider@gmail.com>
---
Notes:
    Base Ref: master
    Web-Diff: https://github.com/larsxschneider/git/commit/322094c0a2
    Checkout: git fetch https://github.com/larsxschneider/git travisci/win-v1 && git checkout 322094c0a2
 .travis.yml             | 11 ++++++++++
 ci/run-windows-build.sh | 55 +++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 66 insertions(+)
 create mode 100755 ci/run-windows-build.sh
Show changes to 2 files +66 −0

.travis.yml, ci/run-windows-build.sh

diff --git a/.travis.yml b/.travis.yml
index 591cc57b80..a7e98ae519 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -39,6 +39,17 @@ env:
 
 matrix:
   include:
+    - env: Windows
+      os: linux
+      compiler:
+      addons:
+      before_install:
+      before_script:
+      script:
+        - >
+          test "$TRAVIS_REPO_SLUG" != "git/git" ||
+          ci/run-windows-build.sh $GFW_CI_TOKEN $TRAVIS_BRANCH $(git rev-parse HEAD)
+      after_failure:
     - env: Linux32
       os: linux
       services:
diff --git a/ci/run-windows-build.sh b/ci/run-windows-build.sh
new file mode 100755
index 0000000000..324a9ea4e6
--- /dev/null
+++ b/ci/run-windows-build.sh
@@ -0,0 +1,55 @@
+#!/usr/bin/env bash
+#
+# Script to trigger the a Git for Windows build and test run.
+# Pass a token, the branch (only branches on https://github.com/git/git)
+# are supported), and a commit hash.
+#
+
+[ $# -eq 3 ] || (echo "Unexpected number of parameters" && exit 1)
+
+TOKEN=$1
+BRANCH=$2
+COMMIT=$3
+
+gfwci () {
+	curl \
+		-H "Authentication: Bearer $TOKEN" \
+		--silent --retry 5 \
+		"https://git-for-windows-ci.azurewebsites.net/api/TestNow?$1" |
+	sed "$(printf '1s/^\xef\xbb\xbf//')"  # Remove the Byte Order Mark
+}
+
+# Trigger build job
+BUILD_ID=$(gfwci "action=trigger&branch=$BRANCH&commit=$COMMIT&skipTests=false")
+
+# Check if the $BUILD_ID contains a number
+case $BUILD_ID in
+	''|*[!0-9]*) echo $BUILD_ID && exit 1
+esac
+
+echo "Visual Studio Team Services Build #${BUILD_ID}"
+
+# Wait until build job finished
+STATUS=
+RESULT=
+while true
+do
+	LAST_STATUS=$STATUS
+	STATUS=$(gfwci "action=status&buildId=$BUILD_ID")
+	[ "$STATUS" == "$LAST_STATUS" ] || printf "\nStatus: $STATUS "
+	printf "."
+
+	case $STATUS in
+		inProgress|postponed|notStarted) sleep 10                      ;; # continue
+		         "completed: succeeded") RESULT="success";        break;; # success
+		                              *) echo "Unknown: $STATUS"; break;; # failure
+	esac
+done
+
+# Print log
+echo ""
+echo ""
+gfwci "action=log&buildId=$BUILD_ID" | cut -c 30-
+
+# Set exit code for TravisCI
+[ "$RESULT" == "success" ]

base-commit: afd6726309f57f532b4b989a75c1392359c611cc
-- 
2.11.1
Johannes Schindelin· Mar 22, 2017, 15:49 UTC · re: Lars Schneider · lore

Re: [PATCH v1] travis-ci: build and test Git on Windows

Hi Lars,
On Wed, 22 Mar 2017, Lars Schneider wrote:
Show 9 quoted lines
> Things, that might need to be done:
> * The Windows box can only process a single build at a time. A second
>   Windows build would need to wait until the first finishes. This
>   waiting time and the build time after the wait could exceed the 3h
>   threshold. If this is a problem, then it is likely to happen every day
>   as usually multiple branches are pushed at the same time (pu/next/
>   master/maint). I cannot test this as my TravisCI account has the 50min
>   timeout. One solution could be to limit the number of concurrent
>   TravisCI jobs [2].

There is another possibility, too, of course: I may be able to find more resources and add other VMs which could be used to build and run the tests.

Of course, if those tests would not spawn billions of POSIX processes, things would be faster, too, and we could afford to wait for a single build agent.

Show 33 quoted lines
> diff --git a/ci/run-windows-build.sh b/ci/run-windows-build.sh
> new file mode 100755
> index 0000000000..324a9ea4e6
> --- /dev/null
> +++ b/ci/run-windows-build.sh
> @@ -0,0 +1,55 @@
> +#!/usr/bin/env bash
> +#
> +# Script to trigger the a Git for Windows build and test run.
> +# Pass a token, the branch (only branches on https://github.com/git/git)
> +# are supported), and a commit hash.
> +#
> +
> +[ $# -eq 3 ] || (echo "Unexpected number of parameters" && exit 1)
> +
> +TOKEN=$1
> +BRANCH=$2
> +COMMIT=$3
> +
> +gfwci () {
> +	curl \
> +		-H "Authentication: Bearer $TOKEN" \
> +		--silent --retry 5 \
> +		"https://git-for-windows-ci.azurewebsites.net/api/TestNow?$1" |
> +	sed "$(printf '1s/^\xef\xbb\xbf//')"  # Remove the Byte Order Mark
> +}
> +
> +# Trigger build job
> +BUILD_ID=$(gfwci "action=trigger&branch=$BRANCH&commit=$COMMIT&skipTests=false")
> +
> +# Check if the $BUILD_ID contains a number
> +case $BUILD_ID in
> +	''|*[!0-9]*) echo $BUILD_ID && exit 1

Error messages are delivered that way, and they do not start with digits, true. But maybe there is an exit status to indicate to Travis that we cannot decide whether the build failed or succeeded in that case? The most common cause for an error here is that the VM I use for testing is down (which happens every once in a while to save on resources, and I have to manually restart it)...

> +echo "Visual Studio Team Services Build #${BUILD_ID}"
Nice plug, thanks! ;-)
Show 14 quoted lines
> +# Wait until build job finished
> +STATUS=
> +RESULT=
> +while true
> +do
> +	LAST_STATUS=$STATUS
> +	STATUS=$(gfwci "action=status&buildId=$BUILD_ID")
> +	[ "$STATUS" == "$LAST_STATUS" ] || printf "\nStatus: $STATUS "
> +	printf "."
> +
> +	case $STATUS in
> +		inProgress|postponed|notStarted) sleep 10                      ;; # continue
> +		         "completed: succeeded") RESULT="success";        break;; # success
> +		                              *) echo "Unknown: $STATUS"; break;; # failure

Well, there are more values for the status, and we know them, but we do not handle them. Maybe "Unhandled status:"?

Show 10 quoted lines
> +	esac
> +done
> +
> +# Print log
> +echo ""
> +echo ""
> +gfwci "action=log&buildId=$BUILD_ID" | cut -c 30-
> +
> +# Set exit code for TravisCI
> +[ "$RESULT" == "success" ]
Very nice. Thank you so much for keeping me working on this!

Ciao, Dscho

Lars Schneider· Mar 23, 2017, 18:19 UTC · re: Johannes Schindelin · lore

Re: [PATCH v1] travis-ci: build and test Git on Windows

Show 5 quoted lines
> On 22 Mar 2017, at 16:49, Johannes Schindelin <johannes.schindelin@gmx.de> wrote:
> 
> Hi Lars,
> 
> On Wed, 22 Mar 2017, Lars Schneider wrote:
...
Show 22 quoted lines
>> +
>> +gfwci () {
>> +	curl \
>> +		-H "Authentication: Bearer $TOKEN" \
>> +		--silent --retry 5 \
>> +		"https://git-for-windows-ci.azurewebsites.net/api/TestNow?$1" |
>> +	sed "$(printf '1s/^\xef\xbb\xbf//')"  # Remove the Byte Order Mark
>> +}
>> +
>> +# Trigger build job
>> +BUILD_ID=$(gfwci "action=trigger&branch=$BRANCH&commit=$COMMIT&skipTests=false")
>> +
>> +# Check if the $BUILD_ID contains a number
>> +case $BUILD_ID in
>> +	''|*[!0-9]*) echo $BUILD_ID && exit 1
> 
> Error messages are delivered that way, and they do not start with digits,
> true. But maybe there is an exit status to indicate to Travis that we
> cannot decide whether the build failed or succeeded in that case? The most
> common cause for an error here is that the VM I use for testing is down
> (which happens every once in a while to save on resources, and I have to
> manually restart it)...
Curl can return the HTTP code... I'll try to find a way to check for this.
Show 22 quoted lines
> 
>> +echo "Visual Studio Team Services Build #${BUILD_ID}"
> 
> Nice plug, thanks! ;-)
> 
>> +# Wait until build job finished
>> +STATUS=
>> +RESULT=
>> +while true
>> +do
>> +	LAST_STATUS=$STATUS
>> +	STATUS=$(gfwci "action=status&buildId=$BUILD_ID")
>> +	[ "$STATUS" == "$LAST_STATUS" ] || printf "\nStatus: $STATUS "
>> +	printf "."
>> +
>> +	case $STATUS in
>> +		inProgress|postponed|notStarted) sleep 10                      ;; # continue
>> +		         "completed: succeeded") RESULT="success";        break;; # success
>> +		                              *) echo "Unknown: $STATUS"; break;; # failure
> 
> Well, there are more values for the status, and we know them, but we do
> not handle them. Maybe "Unhandled status:"?
Sure! I'll fix that in v2 :-)
- Lars
Junio C Hamano· Mar 22, 2017, 19:29 UTC · re: Lars Schneider · lore

Re: [PATCH v1] travis-ci: build and test Git on Windows

Lars Schneider <larsxschneider@gmail.com> writes:
Show 10 quoted lines
> Therefore, we did the following:
> * Johannes Schindelin set up a Visual Studio Team Services build
>   sponsored by Microsoft and made it accessible via an Azure Function
>   that speaks a super-simple API. We made TravisCI use this API to
>   trigger a build, wait until its completion, and print the build and
>   test results.
> * A Windows build and test run takes up to 3h and TravisCI has a timeout
>   after 50min for Open Source projects. Since the TravisCI job does not
>   use heavy CPU/memory/etc. resources, the friendly TravisCI folks
>   extended the job timeout for git/git to 3h.

The benefit is that Windows CI does not have to subscribe to the GitHub repository to get notified (instead uses Travis as a relay for update notification) and the result can be seen at the same place as results on other platforms Travis natively support are shown?

Very nice.
> Things, that would need to be done:
> * Someone with write access to https://travis-ci.org/git/git would need
>   to add the secret token as "GFW_CI_TOKEN" variable in the TravisCI
>   repository setting [1]. Afterwards the build should just work.

We need to make sure this does not leak to the execution log of Travis.

For example, in https://travis-ci.org/git/git/jobs/213616973, which is a log of the documentation build for #1335.6 ran for the 'master' branch, you can see "ci/test-documentation.sh" string appearing in the log twice. This comes from "script:" part, which is the same mechanism this patch uses to invoke the new script with sekrit on the command line.

I am expecting that no expansion of "$GFW_CI_TOKEN" will be shown in the output, but I've seen an incident where an unsuspecting 'set -x' or '$cmd -v' revealed something that shouldn't have been made public. I want to make sure we are sure that the command line this patch adds does not get echoed with expansion to the log.

Is GFW_CI_TOKEN known to be safe without double-quote around it, by the way?

> Things, that might need to be done:
> * The Windows box can only process a single build at a time. A second
>   Windows build would need to wait until the first finishes.

Perhaps instead of accumulating pending requests, perhaps we can arrange so that Travis skips a build/test request that is not even started yet for the same branch? For branches that are never rewound, a breakage in an earlier pushout would either show in a later pushout of the same branch (if breakage is not fixed yet), or doesn't (if the later pushout was to fix that breakage), and in either case, it is not useful to test the earlier pushout when a newer one is already available for testing. For branches that are constantly rewound, again, it is not useful to test the earlier pushout when a newer one is already available for testing.

Show 7 quoted lines
> diff --git a/ci/run-windows-build.sh b/ci/run-windows-build.sh
> new file mode 100755
> index 0000000000..324a9ea4e6
> --- /dev/null
> +++ b/ci/run-windows-build.sh
> @@ -0,0 +1,55 @@
> +#!/usr/bin/env bash

I know this is not a usual scripted Porcelain that must be usable by all users, but I do not see anything that requires bash-ism in the script. Can we just do "#!/bin/sh", avoid bash-isms, and follow the usual coding guidelines in general?

> +[ $# -eq 3 ] || (echo "Unexpected number of parameters" && exit 1)
i.e.e.g "test $# = 3" || ...
> +# Check if the $BUILD_ID contains a number
> +case $BUILD_ID in
> +	''|*[!0-9]*) echo $BUILD_ID && exit 1
> +esac
Too deep an indent of a case arm, i.e. align them with case/esac, like
        case $BUILD_ID in
        ''|*[!0-9]*) echo $BUILD_ID && exit 1
        esac
Thanks.
Johannes Schindelin· Mar 23, 2017, 16:22 UTC · re: Junio C Hamano · lore

Re: [PATCH v1] travis-ci: build and test Git on Windows

Hi Junio,
On Wed, 22 Mar 2017, Junio C Hamano wrote:
Show 18 quoted lines
> Lars Schneider <larsxschneider@gmail.com> writes:
> 
> > Therefore, we did the following:
> > * Johannes Schindelin set up a Visual Studio Team Services build
> >   sponsored by Microsoft and made it accessible via an Azure Function
> >   that speaks a super-simple API. We made TravisCI use this API to
> >   trigger a build, wait until its completion, and print the build and
> >   test results.
> > * A Windows build and test run takes up to 3h and TravisCI has a timeout
> >   after 50min for Open Source projects. Since the TravisCI job does not
> >   use heavy CPU/memory/etc. resources, the friendly TravisCI folks
> >   extended the job timeout for git/git to 3h.
> 
> The benefit is that Windows CI does not have to subscribe to the
> GitHub repository to get notified (instead uses Travis as a relay
> for update notification) and the result can be seen at the same
> place as results on other platforms Travis natively support are
> shown?  

Almost... Windows CI *cannot* subscribe to the GitHub repository, as only owners can install web hooks and give permission to update build status.

But yeah, you understand correctly: this innocuous change (along with a ton of work I already finished on my side) allows us to let Travis trigger Windows build & test and to attach the log in the same place as the Linux/OSX results are already accessible.

Show 20 quoted lines
> > Things, that would need to be done:
> > * Someone with write access to https://travis-ci.org/git/git would need
> >   to add the secret token as "GFW_CI_TOKEN" variable in the TravisCI
> >   repository setting [1]. Afterwards the build should just work.
> 
> We need to make sure this does not leak to the execution log of
> Travis.
> 
> For example, in https://travis-ci.org/git/git/jobs/213616973, which
> is a log of the documentation build for #1335.6 ran for the 'master'
> branch, you can see "ci/test-documentation.sh" string appearing in
> the log twice.  This comes from "script:" part, which is the same
> mechanism this patch uses to invoke the new script with sekrit on
> the command line.
> 
> I am expecting that no expansion of "$GFW_CI_TOKEN" will be shown in
> the output, but I've seen an incident where an unsuspecting 'set -x'
> or '$cmd -v' revealed something that shouldn't have been made
> public.  I want to make sure we are sure that the command line this
> patch adds does not get echoed with expansion to the log.

Right, typically there is a way in CI setups that marks certain strings as secret and whenever they appear in the log, they will be blotted out.

> Is GFW_CI_TOKEN known to be safe without double-quote around it, by
> the way?
Yes, it is safe without double-quotes. I generated it using:
	dd if=/dev/urandom bs=20 count=1 2> /dev/null | base64
Show 14 quoted lines
> > Things, that might need to be done:
> > * The Windows box can only process a single build at a time. A second
> >   Windows build would need to wait until the first finishes.
> 
> Perhaps instead of accumulating pending requests, perhaps we can
> arrange so that Travis skips a build/test request that is not even
> started yet for the same branch?  For branches that are never
> rewound, a breakage in an earlier pushout would either show in a
> later pushout of the same branch (if breakage is not fixed yet), or
> doesn't (if the later pushout was to fix that breakage), and in
> either case, it is not useful to test the earlier pushout when a
> newer one is already available for testing.  For branches that are
> constantly rewound, again, it is not useful to test the earlier
> pushout when a newer one is already available for testing.

Yes, I think we have to use some kind of "skip" status if the build failed to run or finish in time. But I thought that the "timeout" status would fulfill that desire...

Ciao, Dscho

Jeff King· Mar 23, 2017, 18:01 UTC · re: Johannes Schindelin · lore

Re: [PATCH v1] travis-ci: build and test Git on Windows

On Thu, Mar 23, 2017 at 05:22:51PM +0100, Johannes Schindelin wrote:
Show 13 quoted lines
> > The benefit is that Windows CI does not have to subscribe to the
> > GitHub repository to get notified (instead uses Travis as a relay
> > for update notification) and the result can be seen at the same
> > place as results on other platforms Travis natively support are
> > shown?  
> 
> Almost... Windows CI *cannot* subscribe to the GitHub repository, as only
> owners can install web hooks and give permission to update build status.
> 
> But yeah, you understand correctly: this innocuous change (along with a
> ton of work I already finished on my side) allows us to let Travis trigger
> Windows build & test and to attach the log in the same place as the
> Linux/OSX results are already accessible.

I don't mind adding a webhook if it helps. If I understand correctly that would just handle the notification site. But then if the Windows build status were public, we could have Travis simply fetch it to keep the build reports all together, without having to worry about a secret token.

I don't mind proceeding with the secret token, though. You're the owner of the service the token accesses, so if you're comfortable with it, I am.

Show 11 quoted lines
> > > Things, that would need to be done:
> > > * Someone with write access to https://travis-ci.org/git/git would need
> > >   to add the secret token as "GFW_CI_TOKEN" variable in the TravisCI
> > >   repository setting [1]. Afterwards the build should just work.
> > 
> > We need to make sure this does not leak to the execution log of
> > Travis.
> [...]
> 
> Right, typically there is a way in CI setups that marks certain strings as
> secret and whenever they appear in the log, they will be blotted out.

I think both Junio and I have access to the Travis config. Travis does have a "this is secret" flag for setup config. But I think we'd need to verify that running the Travis build does not leak the variable in any other way.

For instance, if it's in the environment, can I push up a branch that does "set | grep GFW_CI_TOKEN", open a PR, and see it? I don't know the answer.

-Peff
Junio C Hamano· Mar 23, 2017, 19:12 UTC · re: Jeff King · lore

Re: [PATCH v1] travis-ci: build and test Git on Windows

Jeff King <peff@peff.net> writes:
> For instance, if it's in the environment, can I push up a branch that
> does "set | grep GFW_CI_TOKEN", open a PR, and see it? I don't know the
> answer.
I think the documentation said
    Variables defined in repository settings are the same for all
    builds, and when you restart an old build, it uses the latest
    values. These variables are not automatically available to
    forks.
so we should be safe as long as we do not build against PRs.

On the other hand, perhaps a contributor may want to build and test his own PR that may affect Windows platform, and such a contributor may be helped if the main repository sets things up to build against PRs.

I personally think it is a separate issue and we shouldn't set it up to build against PRs. If Windows CI wants to help these contributors, it can give out the token to them, without relying on the travis setup for the main repository.

https://docs.travis-ci.com/user/environment-variables#Defining-Variables-in-Repository-Settings
Jeff King· Mar 23, 2017, 19:17 UTC · re: Junio C Hamano · lore

Re: [PATCH v1] travis-ci: build and test Git on Windows

On Thu, Mar 23, 2017 at 12:12:15PM -0700, Junio C Hamano wrote:
Show 14 quoted lines
> Jeff King <peff@peff.net> writes:
> 
> > For instance, if it's in the environment, can I push up a branch that
> > does "set | grep GFW_CI_TOKEN", open a PR, and see it? I don't know the
> > answer.
> 
> I think the documentation said
> 
>     Variables defined in repository settings are the same for all
>     builds, and when you restart an old build, it uses the latest
>     values. These variables are not automatically available to
>     forks.
> 
> so we should be safe as long as we do not build against PRs.
I think we do build against PRs now. E.g.:
  https://travis-ci.org/git/git/builds/213896051
But it looks like we can turn that off.
Show 9 quoted lines
> On the other hand, perhaps a contributor may want to build and test
> his own PR that may affect Windows platform, and such a contributor
> may be helped if the main repository sets things up to build against
> PRs.
> 
> I personally think it is a separate issue and we shouldn't set it up
> to build against PRs.  If Windows CI wants to help these
> contributors, it can give out the token to them, without relying on
> the travis setup for the main repository.

Hrm, it does mean that people have no way to test on Windows until the branch hits pu. Which is not ideal.

-Peff
Lars Schneider· Mar 23, 2017, 19:26 UTC · re: Jeff King · lore

Re: [PATCH v1] travis-ci: build and test Git on Windows

Show 24 quoted lines
> On 23 Mar 2017, at 20:17, Jeff King <peff@peff.net> wrote:
> 
> On Thu, Mar 23, 2017 at 12:12:15PM -0700, Junio C Hamano wrote:
> 
>> Jeff King <peff@peff.net> writes:
>> 
>>> For instance, if it's in the environment, can I push up a branch that
>>> does "set | grep GFW_CI_TOKEN", open a PR, and see it? I don't know the
>>> answer.
>> 
>> I think the documentation said
>> 
>>    Variables defined in repository settings are the same for all
>>    builds, and when you restart an old build, it uses the latest
>>    values. These variables are not automatically available to
>>    forks.
>> 
>> so we should be safe as long as we do not build against PRs.
> 
> I think we do build against PRs now. E.g.:
> 
>  https://travis-ci.org/git/git/builds/213896051
> 
> But it looks like we can turn that off.

When we add a secret variable, then TravisCI will not build Pull Requests for git/git anymore:

"[...] we do not provide these values to untrusted builds, triggered by pull requests from another repository."

See: https://docs.travis-ci.com/user/environment-variables/#Defining-Variables-in-Repository-Settings

However, I don't think that is a big deal because git/git doesn't support Pull Requests anyways. Plus, if a contributor is interested in TravisCI results, then the contributor can setup TravisCI for their own fork easily.

Show 12 quoted lines
>> On the other hand, perhaps a contributor may want to build and test
>> his own PR that may affect Windows platform, and such a contributor
>> may be helped if the main repository sets things up to build against
>> PRs.
>> 
>> I personally think it is a separate issue and we shouldn't set it up
>> to build against PRs.  If Windows CI wants to help these
>> contributors, it can give out the token to them, without relying on
>> the travis setup for the main repository.
> 
> Hrm, it does mean that people have no way to test on Windows until the
> branch hits pu. Which is not ideal.

I agree it's not ideal. But I think it is an improvement to check pu/next/master/maint continuously :-)

Cheers, Lars

Junio C Hamano· Mar 23, 2017, 19:30 UTC · re: Lars Schneider · lore

Re: [PATCH v1] travis-ci: build and test Git on Windows

Lars Schneider <larsxschneider@gmail.com> writes:
> "[...] we do not provide these values to untrusted builds, 
> triggered by pull requests from another repository."
>
> See: https://docs.travis-ci.com/user/environment-variables/#Defining-Variables-in-Repository-Settings

OK, it is a releaf to see an indication that somebody over there is thinking. Thanks.

So we do _not_ have to turn it off; as soon as we define sekrit variables, they will turn it off for us ;-).

Show 5 quoted lines
>> Hrm, it does mean that people have no way to test on Windows until the
>> branch hits pu. Which is not ideal.
>
> I agree it's not ideal. But I think it is an improvement to check
> pu/next/master/maint continuously :-)

I am not sure what you mean. We are building each and every branch updates already, and I do not see any improvement over what we are doing now. Care to elaborate?

Lars Schneider· Mar 23, 2017, 19:36 UTC · re: Junio C Hamano · lore

Re: [PATCH v1] travis-ci: build and test Git on Windows

Show 24 quoted lines
> On 23 Mar 2017, at 20:30, Junio C Hamano <gitster@pobox.com> wrote:
> 
> Lars Schneider <larsxschneider@gmail.com> writes:
> 
>> "[...] we do not provide these values to untrusted builds, 
>> triggered by pull requests from another repository."
>> 
>> See: https://docs.travis-ci.com/user/environment-variables/#Defining-Variables-in-Repository-Settings
> 
> OK, it is a releaf to see an indication that somebody over there is
> thinking.  Thanks.
> 
> So we do _not_ have to turn it off; as soon as we define sekrit
> variables, they will turn it off for us ;-).
> 
>>> Hrm, it does mean that people have no way to test on Windows until the
>>> branch hits pu. Which is not ideal.
>> 
>> I agree it's not ideal. But I think it is an improvement to check
>> pu/next/master/maint continuously :-)
> 
> I am not sure what you mean.  We are building each and every branch
> updates already, and I do not see any improvement over what we are
> doing now.  Care to elaborate?

We are building each and every branch on TravisCI right now - but only on Linux and OSX. With this change we also build it on Windows. That should help to spot Windows related issues more quickly I think.

- Lars
Junio C Hamano· Mar 23, 2017, 20:10 UTC · re: Lars Schneider · lore

Re: [PATCH v1] travis-ci: build and test Git on Windows

Lars Schneider <larsxschneider@gmail.com> writes:
Show 11 quoted lines
>>> I agree it's not ideal. But I think it is an improvement to check
>>> pu/next/master/maint continuously :-)
>> 
>> I am not sure what you mean.  We are building each and every branch
>> updates already, and I do not see any improvement over what we are
>> doing now.  Care to elaborate?
>
> We are building each and every branch on TravisCI right now - but 
> only on Linux and OSX. With this change we also build it on
> Windows. That should help to spot Windows related issues more
> quickly I think.

We are losing the build for PR for folks who later submit patches as the price for that. "building each and every branch" stay constant and the trade-off is between building four integration branches on Windows and allowing contributors to easily check their work early.

If your "But I think it is an improvement" was followed by "to check the four branches on Windows continuously", though, I wouldn't have had to ask the question, as I tend to agree it is a better trade off.

Thanks.
Jeff King· Mar 23, 2017, 19:38 UTC · re: Lars Schneider · lore

Re: [PATCH v1] travis-ci: build and test Git on Windows

On Thu, Mar 23, 2017 at 08:26:15PM +0100, Lars Schneider wrote:
Show 13 quoted lines
> > I think we do build against PRs now. E.g.:
> > 
> >  https://travis-ci.org/git/git/builds/213896051
> > 
> > But it looks like we can turn that off.
> 
> When we add a secret variable, then TravisCI will not build Pull Requests
> for git/git anymore:
> 
> "[...] we do not provide these values to untrusted builds, 
> triggered by pull requests from another repository."
> 
> See: https://docs.travis-ci.com/user/environment-variables/#Defining-Variables-in-Repository-Settings

Ah, OK, that makes sense. So we would only have to worry about our _own_ code accidentally disclosing it. But that should be easy to look for by grepping the log (did somebody do that?).

> However, I don't think that is a big deal because git/git doesn't
> support Pull Requests anyways. Plus, if a contributor is interested
> in TravisCI results, then the contributor can setup TravisCI for
> their own fork easily.

Yeah, agreed. It's not like we are blocking the merge button with a failing status.

Show 5 quoted lines
> > Hrm, it does mean that people have no way to test on Windows until the
> > branch hits pu. Which is not ideal.
> 
> I agree it's not ideal. But I think it is an improvement to check
> pu/next/master/maint continuously :-)

Oh, I agree this is a step forward from the status quo. I just wondered if we could do even better.

As a side note, I've started having travis run on all of the topic branches in peff/git, with the idea to get early feedback on OS X problems (and now hopefully Windows ones). My two issues so far are:

  - I have a lot of work-in-progress branches. I put "-wip" at the end
    of the branch name for my own scripts. It looks like I can put "[ci
    skip]" in the commit subject to convince Travis to skip them, but
    that's a little annoying. Is there any way to skip based on just the
    branch name? I couldn't find one.
  - The OS X builds seem to regularly time out. That at least marks a
    "!" in the build status screen instead of an "X", but it's a lot of
    noise (and it misses the point for me, which is testing on OS X; I
    already build regularly on Linux).
-Peff
Lars Schneider· Mar 23, 2017, 20:00 UTC · re: Jeff King · lore

Re: [PATCH v1] travis-ci: build and test Git on Windows

Show 21 quoted lines
> On 23 Mar 2017, at 20:38, Jeff King <peff@peff.net> wrote:
> 
> On Thu, Mar 23, 2017 at 08:26:15PM +0100, Lars Schneider wrote:
> 
>>> I think we do build against PRs now. E.g.:
>>> 
>>> https://travis-ci.org/git/git/builds/213896051
>>> 
>>> But it looks like we can turn that off.
>> 
>> When we add a secret variable, then TravisCI will not build Pull Requests
>> for git/git anymore:
>> 
>> "[...] we do not provide these values to untrusted builds, 
>> triggered by pull requests from another repository."
>> 
>> See: https://docs.travis-ci.com/user/environment-variables/#Defining-Variables-in-Repository-Settings
> 
> Ah, OK, that makes sense. So we would only have to worry about our _own_
> code accidentally disclosing it. But that should be easy to look for by
> grepping the log (did somebody do that?).

This is how a successful Windows build would look like: https://travis-ci.org/larsxschneider/git/jobs/214391822

Dscho's token is not in the log.
Show 26 quoted lines
>> However, I don't think that is a big deal because git/git doesn't
>> support Pull Requests anyways. Plus, if a contributor is interested
>> in TravisCI results, then the contributor can setup TravisCI for
>> their own fork easily.
> 
> Yeah, agreed. It's not like we are blocking the merge button with a
> failing status.
> 
>>> Hrm, it does mean that people have no way to test on Windows until the
>>> branch hits pu. Which is not ideal.
>> 
>> I agree it's not ideal. But I think it is an improvement to check
>> pu/next/master/maint continuously :-)
> 
> Oh, I agree this is a step forward from the status quo. I just wondered
> if we could do even better.
> 
> As a side note, I've started having travis run on all of the topic
> branches in peff/git, with the idea to get early feedback on OS X
> problems (and now hopefully Windows ones). My two issues so far are:
> 
>  - I have a lot of work-in-progress branches. I put "-wip" at the end
>    of the branch name for my own scripts. It looks like I can put "[ci
>    skip]" in the commit subject to convince Travis to skip them, but
>    that's a little annoying. Is there any way to skip based on just the
>    branch name? I couldn't find one.

We can blacklist these branches with a regex in the travis.yml: https://docs.travis-ci.com/user/customizing-the-build#Building-Specific-Branches

>  - The OS X builds seem to regularly time out. That at least marks a
>    "!" in the build status screen instead of an "X", but it's a lot of
>    noise (and it misses the point for me, which is testing on OS X; I
>    already build regularly on Linux).

Well, I guess the reason is that OSX is a bit harder to virtualize compared to Linux. In general on git/git the OSX builds are OK.

I build a little stats page to visualize the results over time: https://larsxschneider.github.io/git-ci-stats/

We have 2 OSX jobs and 2 Linux jobs per build. I am usually only digging into issues if both jobs are red.

Maybe TravisCI throttles your usage somehow as you push a lot of commits?

Keep in mind, all the TravisCI compute hours are free... considering that I think it is quite OK :-)

- Lars
Jeff King· Mar 23, 2017, 20:20 UTC · re: Lars Schneider · lore

Re: [PATCH v1] travis-ci: build and test Git on Windows

On Thu, Mar 23, 2017 at 09:00:33PM +0100, Lars Schneider wrote:
Show 8 quoted lines
> > Ah, OK, that makes sense. So we would only have to worry about our _own_
> > code accidentally disclosing it. But that should be easy to look for by
> > grepping the log (did somebody do that?).
> 
> This is how a successful Windows build would look like:
> https://travis-ci.org/larsxschneider/git/jobs/214391822
> 
> Dscho's token is not in the log.
Great, thank you for double-checking.
Show 8 quoted lines
> >  - I have a lot of work-in-progress branches. I put "-wip" at the end
> >    of the branch name for my own scripts. It looks like I can put "[ci
> >    skip]" in the commit subject to convince Travis to skip them, but
> >    that's a little annoying. Is there any way to skip based on just the
> >    branch name? I couldn't find one.
> 
> We can blacklist these branches with a regex in the travis.yml:
> https://docs.travis-ci.com/user/customizing-the-build#Building-Specific-Branches

I had a feeling it might be something like that. So we would all need to agree on the convention for WIP branch names. If other people like the idea, I'm happy to make a patch, but I don't want to impose my own weird conventions on everyone else.

> Maybe TravisCI throttles your usage somehow as you push a lot of commits?
Could be. Looking at:
  https://travis-ci.org/peff/git/branches

It seems to timeout on over half the branches (in fact, there are only a few that passed all of the tests). My pattern is particularly spiky from Travis's perspective, because once a day I rebase everything on top of master and push them the whole thing in a bunch. So they 75 branches, all at once. That seems like it would be ripe for throttling (though I would much rather they just queue the builds and do them one at a time).

> Keep in mind, all the TravisCI compute hours are free... considering
> that I think it is quite OK :-)

I don't blame Travis at all. But if the tool does not produce reliable results, then I will start to ignore it. And somebody on this thread (not you) has been complaining repeatedly about developers ignoring CI results.

-Peff
Junio C Hamano· Mar 23, 2017, 20:30 UTC · re: Jeff King · lore

Re: [PATCH v1] travis-ci: build and test Git on Windows

Jeff King <peff@peff.net> writes:
Show 7 quoted lines
>> We can blacklist these branches with a regex in the travis.yml:
>> https://docs.travis-ci.com/user/customizing-the-build#Building-Specific-Branches
>
> I had a feeling it might be something like that. So we would all need to
> agree on the convention for WIP branch names. If other people like the
> idea, I'm happy to make a patch, but I don't want to impose my own weird
> conventions on everyone else.

I can go with any convention, but I'd be more pleased if you made sure that "do not build this with CI" and "this is WIP" are kept as two separate concepts, as I can see having some WIP that I do want to get tested.

Perhaps a substring "/noci-" anywhere in the branch name, or something silly like that?

> I don't blame Travis at all. But if the tool does not produce reliable
> results, then I will start to ignore it.

Yes, that is a real issue. I was wondering if we can have a knob that can be controled with the repository setting to limit which "build jobs" are run, so that you can use that web UI to set an "environment variable" that lists only .3 and .4 (which correspond to these two OSX builds) and .travis.yml takes notice of the variable setting, or something.

Jeff King· Mar 23, 2017, 20:41 UTC · re: Junio C Hamano · lore

Re: [PATCH v1] travis-ci: build and test Git on Windows

On Thu, Mar 23, 2017 at 01:30:41PM -0700, Junio C Hamano wrote:
Show 15 quoted lines
> >> We can blacklist these branches with a regex in the travis.yml:
> >> https://docs.travis-ci.com/user/customizing-the-build#Building-Specific-Branches
> >
> > I had a feeling it might be something like that. So we would all need to
> > agree on the convention for WIP branch names. If other people like the
> > idea, I'm happy to make a patch, but I don't want to impose my own weird
> > conventions on everyone else.
> 
> I can go with any convention, but I'd be more pleased if you made
> sure that "do not build this with CI" and "this is WIP" are kept as
> two separate concepts, as I can see having some WIP that I do want
> to get tested.
> 
> Perhaps a substring "/noci-" anywhere in the branch name, or
> something silly like that?

Hrm, most of the point for me was _not_ having to define the two concepts separately. Let me try it for a while with "[ci skip]" in the tip commit subject, and see how painful I find that. My goal is eventually to turn on notifications, so I'd quickly be reminded if I forgot such a marker.

-Peff
Lars Schneider· Mar 23, 2017, 20:39 UTC · re: Jeff King · lore

Re: [PATCH v1] travis-ci: build and test Git on Windows

Show 14 quoted lines
> On 23 Mar 2017, at 21:20, Jeff King <peff@peff.net> wrote:
> 
> On Thu, Mar 23, 2017 at 09:00:33PM +0100, Lars Schneider wrote:
> 
>>> Ah, OK, that makes sense. So we would only have to worry about our _own_
>>> code accidentally disclosing it. But that should be easy to look for by
>>> grepping the log (did somebody do that?).
>> 
>> This is how a successful Windows build would look like:
>> https://travis-ci.org/larsxschneider/git/jobs/214391822
>> 
>> Dscho's token is not in the log.
> 
> Great, thank you for double-checking.

I can see that one could think that this variables leaks, though. I think in v2 I won't pass the token as parameter. The token variable is an environment variable anyways and I can just use it in the script.

Show 13 quoted lines
>>> - I have a lot of work-in-progress branches. I put "-wip" at the end
>>>   of the branch name for my own scripts. It looks like I can put "[ci
>>>   skip]" in the commit subject to convince Travis to skip them, but
>>>   that's a little annoying. Is there any way to skip based on just the
>>>   branch name? I couldn't find one.
>> 
>> We can blacklist these branches with a regex in the travis.yml:
>> https://docs.travis-ci.com/user/customizing-the-build#Building-Specific-Branches
> 
> I had a feeling it might be something like that. So we would all need to
> agree on the convention for WIP branch names. If other people like the
> idea, I'm happy to make a patch, but I don't want to impose my own weird
> conventions on everyone else.

This makes sense to me. If someone complains with a good argument then we can still revert it.

Show 12 quoted lines
>> Maybe TravisCI throttles your usage somehow as you push a lot of commits?
> 
> Could be. Looking at:
> 
>  https://travis-ci.org/peff/git/branches
> 
> It seems to timeout on over half the branches (in fact, there are only a
> few that passed all of the tests). My pattern is particularly spiky from
> Travis's perspective, because once a day I rebase everything on top of
> master and push them the whole thing in a bunch. So they 75 branches,
> all at once. That seems like it would be ripe for throttling (though I
> would much rather they just queue the builds and do them one at a time).

Could you try to set this to 7 or less in your TravisCI? https://docs.travis-ci.com/user/customizing-the-build#Limiting-Concurrent-Builds

I am curious if this improves the situation.
- Lars
Jeff King· Mar 23, 2017, 20:42 UTC · re: Lars Schneider · lore

Re: [PATCH v1] travis-ci: build and test Git on Windows

On Thu, Mar 23, 2017 at 09:39:14PM +0100, Lars Schneider wrote:
Show 15 quoted lines
> > Could be. Looking at:
> > 
> >  https://travis-ci.org/peff/git/branches
> > 
> > It seems to timeout on over half the branches (in fact, there are only a
> > few that passed all of the tests). My pattern is particularly spiky from
> > Travis's perspective, because once a day I rebase everything on top of
> > master and push them the whole thing in a bunch. So they 75 branches,
> > all at once. That seems like it would be ripe for throttling (though I
> > would much rather they just queue the builds and do them one at a time).
> 
> Could you try to set this to 7 or less in your TravisCI?
> https://docs.travis-ci.com/user/customizing-the-build#Limiting-Concurrent-Builds
> 
> I am curious if this improves the situation.

Sure, that's easy enough to try. I'll let it go for a few days with that and see if it improves.

-Peff
Johannes Schindelin· Mar 24, 2017, 23:50 UTC · re: Jeff King · lore

Re: [PATCH v1] travis-ci: build and test Git on Windows

Hi Peff,
On Thu, 23 Mar 2017, Jeff King wrote:
Show 5 quoted lines
> My pattern is particularly spiky from Travis's perspective, because once
> a day I rebase everything on top of master and push them the whole thing
> in a bunch. So they 75 branches, all at once. That seems like it would
> be ripe for throttling (though I would much rather they just queue the
> builds and do them one at a time).

Travis is optimized for Continuous Integration, i.e. one or maybe two integration branches that merge PRs every once in a while.

What we would need is Continuous Testing, really, because we do not do Continuous Integration at all.

On the point of the sekrit variable: all I tried to do is to avoid overwhelming the (currently) single VM with plenty of jobs. If the build & test would not take so darned long on Windows due to going in and out of the POSIX emulation layer a gazillion times (caused by intense shell scripting), the tests could be faster. But that would require a massive amount of work, and I simply have too much on my plate to take that task on in addition.

If you have an idea how to prevent the overloading of the VM by any means that does not involve that token, please tell me.

Ciao, Dscho

Samuel Lijin· Mar 23, 2017, 21:04 UTC · re: Jeff King · lore

Re: [PATCH v1] travis-ci: build and test Git on Windows

On Thu, Mar 23, 2017 at 2:38 PM, Jeff King <peff@peff.net> wrote:
Show 46 quoted lines
> On Thu, Mar 23, 2017 at 08:26:15PM +0100, Lars Schneider wrote:
>
>> > I think we do build against PRs now. E.g.:
>> >
>> >  https://travis-ci.org/git/git/builds/213896051
>> >
>> > But it looks like we can turn that off.
>>
>> When we add a secret variable, then TravisCI will not build Pull Requests
>> for git/git anymore:
>>
>> "[...] we do not provide these values to untrusted builds,
>> triggered by pull requests from another repository."
>>
>> See: https://docs.travis-ci.com/user/environment-variables/#Defining-Variables-in-Repository-Settings
>
> Ah, OK, that makes sense. So we would only have to worry about our _own_
> code accidentally disclosing it. But that should be easy to look for by
> grepping the log (did somebody do that?).
>
>> However, I don't think that is a big deal because git/git doesn't
>> support Pull Requests anyways. Plus, if a contributor is interested
>> in TravisCI results, then the contributor can setup TravisCI for
>> their own fork easily.
>
> Yeah, agreed. It's not like we are blocking the merge button with a
> failing status.
>
>> > Hrm, it does mean that people have no way to test on Windows until the
>> > branch hits pu. Which is not ideal.
>>
>> I agree it's not ideal. But I think it is an improvement to check
>> pu/next/master/maint continuously :-)
>
> Oh, I agree this is a step forward from the status quo. I just wondered
> if we could do even better.
>
> As a side note, I've started having travis run on all of the topic
> branches in peff/git, with the idea to get early feedback on OS X
> problems (and now hopefully Windows ones). My two issues so far are:
>
>   - I have a lot of work-in-progress branches. I put "-wip" at the end
>     of the branch name for my own scripts. It looks like I can put "[ci
>     skip]" in the commit subject to convince Travis to skip them, but
>     that's a little annoying. Is there any way to skip based on just the
>     branch name? I couldn't find one.

I think you can "safelist" (whitelist) branches to build with regexes: https://docs.travis-ci.com/user/customizing-the-build#Building-Specific-Branches

>   - The OS X builds seem to regularly time out. That at least marks a
>     "!" in the build status screen instead of an "X", but it's a lot of
>     noise (and it misses the point for me, which is testing on OS X; I
>     already build regularly on Linux).
I suspect this happens because they don't have a lot of macOS runners
- I've seen those jobs wait for hours (even on private jobs) before a
runner gets freed up.
> -Peff
Junio C Hamano· Mar 23, 2017, 19:15 UTC · re: Jeff King · lore

Re: [PATCH v1] travis-ci: build and test Git on Windows

Jeff King <peff@peff.net> writes:
> I think both Junio and I have access to the Travis config. Travis does
> have a "this is secret" flag for setup config. But I think we'd need to
> verify that running the Travis build does not leak the variable in any
> other way.

I am not sure if I want to "Authorize application" at the GitHub site to give Travis that broad set of permissions, though. That is why I do not "log in with GitHub" to Travis.

I just logged into Travis and it seems to me that git/git is set to build branch updates (which sounds sensible) and also is set to build pull request updates. That somehow sounds like a dangerous mix with the "secret environment variables" thing, at least to me.

> For instance, if it's in the environment, can I push up a branch that
> does "set | grep GFW_CI_TOKEN", open a PR, and see it? I don't know the
> answer.
Lars Schneider· Mar 23, 2017, 18:23 UTC · re: Junio C Hamano · lore

Re: [PATCH v1] travis-ci: build and test Git on Windows

Show 20 quoted lines
> On 22 Mar 2017, at 20:29, Junio C Hamano <gitster@pobox.com> wrote:
> 
> Lars Schneider <larsxschneider@gmail.com> writes:
> 
>> Therefore, we did the following:
>> * Johannes Schindelin set up a Visual Studio Team Services build
>>  sponsored by Microsoft and made it accessible via an Azure Function
>>  that speaks a super-simple API. We made TravisCI use this API to
>>  trigger a build, wait until its completion, and print the build and
>>  test results.
>> * A Windows build and test run takes up to 3h and TravisCI has a timeout
>>  after 50min for Open Source projects. Since the TravisCI job does not
>>  use heavy CPU/memory/etc. resources, the friendly TravisCI folks
>>  extended the job timeout for git/git to 3h.
> 
> The benefit is that Windows CI does not have to subscribe to the
> GitHub repository to get notified (instead uses Travis as a relay
> for update notification) and the result can be seen at the same
> place as results on other platforms Travis natively support are
> shown?  
Correct!
> Very nice.
Thanks :-)
Show 7 quoted lines
>> Things, that would need to be done:
>> * Someone with write access to https://travis-ci.org/git/git would need
>>  to add the secret token as "GFW_CI_TOKEN" variable in the TravisCI
>>  repository setting [1]. Afterwards the build should just work.
> 
> We need to make sure this does not leak to the execution log of
> Travis.
True. I think I took care of this and it should not leak!
Show 14 quoted lines
>> Things, that might need to be done:
>> * The Windows box can only process a single build at a time. A second
>>  Windows build would need to wait until the first finishes.
> 
> Perhaps instead of accumulating pending requests, perhaps we can
> arrange so that Travis skips a build/test request that is not even
> started yet for the same branch?  For branches that are never
> rewound, a breakage in an earlier pushout would either show in a
> later pushout of the same branch (if breakage is not fixed yet), or
> doesn't (if the later pushout was to fix that breakage), and in
> either case, it is not useful to test the earlier pushout when a
> newer one is already available for testing.  For branches that are
> constantly rewound, again, it is not useful to test the earlier
> pushout when a newer one is already available for testing.

Well, look at this. TravisCi *just* released a solution to exactly that problem! Good timing! https://blog.travis-ci.com/2017-03-22-introducing-auto-cancellation

Show 12 quoted lines
>> diff --git a/ci/run-windows-build.sh b/ci/run-windows-build.sh
>> new file mode 100755
>> index 0000000000..324a9ea4e6
>> --- /dev/null
>> +++ b/ci/run-windows-build.sh
>> @@ -0,0 +1,55 @@
>> +#!/usr/bin/env bash
> 
> I know this is not a usual scripted Porcelain that must be usable by
> all users, but I do not see anything that requires bash-ism in the
> script.  Can we just do "#!/bin/sh", avoid bash-isms, and follow the
> usual coding guidelines in general?
Sure!
Show 14 quoted lines
>> +[ $# -eq 3 ] || (echo "Unexpected number of parameters" && exit 1)
> 
> i.e.e.g "test $# = 3" || ...
> 
>> +# Check if the $BUILD_ID contains a number
>> +case $BUILD_ID in
>> +	''|*[!0-9]*) echo $BUILD_ID && exit 1
>> +esac
> 
> Too deep an indent of a case arm, i.e. align them with case/esac, like
> 
>        case $BUILD_ID in
>        ''|*[!0-9]*) echo $BUILD_ID && exit 1
>        esac
Will do in v2!

Thanks, Lars

Junio C Hamano· Mar 23, 2017, 20:16 UTC · re: Lars Schneider · lore

Re: [PATCH v1] travis-ci: build and test Git on Windows

Lars Schneider <larsxschneider@gmail.com> writes:
> Things, that would need to be done:
> * Someone with write access to https://travis-ci.org/git/git would need
>   to add the secret token as "GFW_CI_TOKEN" variable in the TravisCI
>   repository setting [1]. Afterwards the build should just work.

As coordinating the timing of when this happens with the timing when merging this topic to 'pu' happens is an unnecessary cognitive burden, can you make a small change to this part:

> +      script:
> +        - >
> +          test "$TRAVIS_REPO_SLUG" != "git/git" ||
Add
	test -z "$GFW_CI_TOKEN" ||

here, so that the sript is not run while "Someone with write access" hasn't got around to doing it.

> +          ci/run-windows-build.sh $GFW_CI_TOKEN $TRAVIS_BRANCH $(git rev-parse HEAD)

It would still be a good idea to dq all these $VARIABLE and $(COMMAND OUTPUT) references, as the script expects three parameters.

> + ...
> +[ $# -eq 3 ] || (echo "Unexpected number of parameters" && exit 1)
Thanks.

← back to recent threads