* [PATCH v1] travis-ci: build and test Git on Windows @ 2017-03-22 6:56 Lars Schneider 2017-03-22 15:49 ` Johannes Schindelin ` (2 more replies) 0 siblings, 3 replies; 24+ messages in thread From: Lars Schneider @ 2017-03-22 6:56 UTC (permalink / raw) To: git; +Cc: Johannes.Schindelin, gitster, peff 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 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 ^ permalink raw reply related [flat|nested] 24+ messages in thread
* Re: [PATCH v1] travis-ci: build and test Git on Windows 2017-03-22 6:56 [PATCH v1] travis-ci: build and test Git on Windows Lars Schneider @ 2017-03-22 15:49 ` Johannes Schindelin 2017-03-23 18:19 ` Lars Schneider 2017-03-22 19:29 ` Junio C Hamano 2017-03-23 20:16 ` Junio C Hamano 2 siblings, 1 reply; 24+ messages in thread From: Johannes Schindelin @ 2017-03-22 15:49 UTC (permalink / raw) To: Lars Schneider; +Cc: git, gitster, peff Hi Lars, On Wed, 22 Mar 2017, Lars Schneider wrote: > 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. > 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! ;-) > +# 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:"? > + 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 ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v1] travis-ci: build and test Git on Windows 2017-03-22 15:49 ` Johannes Schindelin @ 2017-03-23 18:19 ` Lars Schneider 0 siblings, 0 replies; 24+ messages in thread From: Lars Schneider @ 2017-03-23 18:19 UTC (permalink / raw) To: Johannes Schindelin; +Cc: git, gitster, peff > On 22 Mar 2017, at 16:49, Johannes Schindelin <johannes.schindelin@gmx.de> wrote: > > Hi Lars, > > On Wed, 22 Mar 2017, Lars Schneider wrote: ... >> + >> +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. > >> +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 ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v1] travis-ci: build and test Git on Windows 2017-03-22 6:56 [PATCH v1] travis-ci: build and test Git on Windows Lars Schneider 2017-03-22 15:49 ` Johannes Schindelin @ 2017-03-22 19:29 ` Junio C Hamano 2017-03-23 16:22 ` Johannes Schindelin 2017-03-23 18:23 ` Lars Schneider 2017-03-23 20:16 ` Junio C Hamano 2 siblings, 2 replies; 24+ messages in thread From: Junio C Hamano @ 2017-03-22 19:29 UTC (permalink / raw) To: Lars Schneider; +Cc: git, Johannes.Schindelin, peff 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? 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. > 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. ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v1] travis-ci: build and test Git on Windows 2017-03-22 19:29 ` Junio C Hamano @ 2017-03-23 16:22 ` Johannes Schindelin 2017-03-23 18:01 ` Jeff King 2017-03-23 18:23 ` Lars Schneider 1 sibling, 1 reply; 24+ messages in thread From: Johannes Schindelin @ 2017-03-23 16:22 UTC (permalink / raw) To: Junio C Hamano; +Cc: Lars Schneider, git, peff Hi Junio, On Wed, 22 Mar 2017, Junio C Hamano 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? 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. > > 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 > > 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 ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v1] travis-ci: build and test Git on Windows 2017-03-23 16:22 ` Johannes Schindelin @ 2017-03-23 18:01 ` Jeff King 2017-03-23 19:12 ` Junio C Hamano 2017-03-23 19:15 ` Junio C Hamano 0 siblings, 2 replies; 24+ messages in thread From: Jeff King @ 2017-03-23 18:01 UTC (permalink / raw) To: Johannes Schindelin; +Cc: Junio C Hamano, Lars Schneider, git On Thu, Mar 23, 2017 at 05:22:51PM +0100, Johannes Schindelin wrote: > > 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. > > > 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 ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v1] travis-ci: build and test Git on Windows 2017-03-23 18:01 ` Jeff King @ 2017-03-23 19:12 ` Junio C Hamano 2017-03-23 19:17 ` Jeff King 2017-03-23 19:15 ` Junio C Hamano 1 sibling, 1 reply; 24+ messages in thread From: Junio C Hamano @ 2017-03-23 19:12 UTC (permalink / raw) To: Jeff King; +Cc: Johannes Schindelin, Lars Schneider, git 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 ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v1] travis-ci: build and test Git on Windows 2017-03-23 19:12 ` Junio C Hamano @ 2017-03-23 19:17 ` Jeff King 2017-03-23 19:26 ` Lars Schneider 0 siblings, 1 reply; 24+ messages in thread From: Jeff King @ 2017-03-23 19:17 UTC (permalink / raw) To: Junio C Hamano; +Cc: Johannes Schindelin, Lars Schneider, git 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. > 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 ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v1] travis-ci: build and test Git on Windows 2017-03-23 19:17 ` Jeff King @ 2017-03-23 19:26 ` Lars Schneider 2017-03-23 19:30 ` Junio C Hamano 2017-03-23 19:38 ` Jeff King 0 siblings, 2 replies; 24+ messages in thread From: Lars Schneider @ 2017-03-23 19:26 UTC (permalink / raw) To: Jeff King; +Cc: Junio C Hamano, Johannes Schindelin, git > 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. >> 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 ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v1] travis-ci: build and test Git on Windows 2017-03-23 19:26 ` Lars Schneider @ 2017-03-23 19:30 ` Junio C Hamano 2017-03-23 19:36 ` Lars Schneider 2017-03-23 19:38 ` Jeff King 1 sibling, 1 reply; 24+ messages in thread From: Junio C Hamano @ 2017-03-23 19:30 UTC (permalink / raw) To: Lars Schneider; +Cc: Jeff King, Johannes Schindelin, git 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? ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v1] travis-ci: build and test Git on Windows 2017-03-23 19:30 ` Junio C Hamano @ 2017-03-23 19:36 ` Lars Schneider 2017-03-23 20:10 ` Junio C Hamano 0 siblings, 1 reply; 24+ messages in thread From: Lars Schneider @ 2017-03-23 19:36 UTC (permalink / raw) To: Junio C Hamano; +Cc: Jeff King, Johannes Schindelin, git > 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 ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v1] travis-ci: build and test Git on Windows 2017-03-23 19:36 ` Lars Schneider @ 2017-03-23 20:10 ` Junio C Hamano 0 siblings, 0 replies; 24+ messages in thread From: Junio C Hamano @ 2017-03-23 20:10 UTC (permalink / raw) To: Lars Schneider; +Cc: Jeff King, Johannes Schindelin, git Lars Schneider <larsxschneider@gmail.com> writes: >>> 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. ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v1] travis-ci: build and test Git on Windows 2017-03-23 19:26 ` Lars Schneider 2017-03-23 19:30 ` Junio C Hamano @ 2017-03-23 19:38 ` Jeff King 2017-03-23 20:00 ` Lars Schneider 2017-03-23 21:04 ` Samuel Lijin 1 sibling, 2 replies; 24+ messages in thread From: Jeff King @ 2017-03-23 19:38 UTC (permalink / raw) To: Lars Schneider; +Cc: Junio C Hamano, Johannes Schindelin, git 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. - 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 ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v1] travis-ci: build and test Git on Windows 2017-03-23 19:38 ` Jeff King @ 2017-03-23 20:00 ` Lars Schneider 2017-03-23 20:20 ` Jeff King 2017-03-23 21:04 ` Samuel Lijin 1 sibling, 1 reply; 24+ messages in thread From: Lars Schneider @ 2017-03-23 20:00 UTC (permalink / raw) To: Jeff King; +Cc: Junio C Hamano, Johannes Schindelin, git > 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. >> 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 ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v1] travis-ci: build and test Git on Windows 2017-03-23 20:00 ` Lars Schneider @ 2017-03-23 20:20 ` Jeff King 2017-03-23 20:30 ` Junio C Hamano ` (2 more replies) 0 siblings, 3 replies; 24+ messages in thread From: Jeff King @ 2017-03-23 20:20 UTC (permalink / raw) To: Lars Schneider; +Cc: Junio C Hamano, Johannes Schindelin, git 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 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 ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v1] travis-ci: build and test Git on Windows 2017-03-23 20:20 ` Jeff King @ 2017-03-23 20:30 ` Junio C Hamano 2017-03-23 20:41 ` Jeff King 2017-03-23 20:39 ` Lars Schneider 2017-03-24 23:50 ` Johannes Schindelin 2 siblings, 1 reply; 24+ messages in thread From: Junio C Hamano @ 2017-03-23 20:30 UTC (permalink / raw) To: Jeff King; +Cc: Lars Schneider, Johannes Schindelin, git Jeff King <peff@peff.net> writes: >> 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. ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v1] travis-ci: build and test Git on Windows 2017-03-23 20:30 ` Junio C Hamano @ 2017-03-23 20:41 ` Jeff King 0 siblings, 0 replies; 24+ messages in thread From: Jeff King @ 2017-03-23 20:41 UTC (permalink / raw) To: Junio C Hamano; +Cc: Lars Schneider, Johannes Schindelin, git On Thu, Mar 23, 2017 at 01:30:41PM -0700, Junio C Hamano wrote: > >> 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 ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v1] travis-ci: build and test Git on Windows 2017-03-23 20:20 ` Jeff King 2017-03-23 20:30 ` Junio C Hamano @ 2017-03-23 20:39 ` Lars Schneider 2017-03-23 20:42 ` Jeff King 2017-03-24 23:50 ` Johannes Schindelin 2 siblings, 1 reply; 24+ messages in thread From: Lars Schneider @ 2017-03-23 20:39 UTC (permalink / raw) To: Jeff King; +Cc: Junio C Hamano, Johannes Schindelin, git > 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. >>> - 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. >> 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 ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v1] travis-ci: build and test Git on Windows 2017-03-23 20:39 ` Lars Schneider @ 2017-03-23 20:42 ` Jeff King 0 siblings, 0 replies; 24+ messages in thread From: Jeff King @ 2017-03-23 20:42 UTC (permalink / raw) To: Lars Schneider; +Cc: Junio C Hamano, Johannes Schindelin, git On Thu, Mar 23, 2017 at 09:39:14PM +0100, Lars Schneider wrote: > > 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 ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v1] travis-ci: build and test Git on Windows 2017-03-23 20:20 ` Jeff King 2017-03-23 20:30 ` Junio C Hamano 2017-03-23 20:39 ` Lars Schneider @ 2017-03-24 23:50 ` Johannes Schindelin 2 siblings, 0 replies; 24+ messages in thread From: Johannes Schindelin @ 2017-03-24 23:50 UTC (permalink / raw) To: Jeff King; +Cc: Lars Schneider, Junio C Hamano, git Hi Peff, On Thu, 23 Mar 2017, Jeff King wrote: > 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 ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v1] travis-ci: build and test Git on Windows 2017-03-23 19:38 ` Jeff King 2017-03-23 20:00 ` Lars Schneider @ 2017-03-23 21:04 ` Samuel Lijin 1 sibling, 0 replies; 24+ messages in thread From: Samuel Lijin @ 2017-03-23 21:04 UTC (permalink / raw) To: Jeff King Cc: Lars Schneider, Junio C Hamano, Johannes Schindelin, git@vger.kernel.org On Thu, Mar 23, 2017 at 2:38 PM, 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?). > >> 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 ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v1] travis-ci: build and test Git on Windows 2017-03-23 18:01 ` Jeff King 2017-03-23 19:12 ` Junio C Hamano @ 2017-03-23 19:15 ` Junio C Hamano 1 sibling, 0 replies; 24+ messages in thread From: Junio C Hamano @ 2017-03-23 19:15 UTC (permalink / raw) To: Jeff King; +Cc: Johannes Schindelin, Lars Schneider, git 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. ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v1] travis-ci: build and test Git on Windows 2017-03-22 19:29 ` Junio C Hamano 2017-03-23 16:22 ` Johannes Schindelin @ 2017-03-23 18:23 ` Lars Schneider 1 sibling, 0 replies; 24+ messages in thread From: Lars Schneider @ 2017-03-23 18:23 UTC (permalink / raw) To: Junio C Hamano; +Cc: git, Johannes.Schindelin, peff > 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 :-) >> 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! >> 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 >> 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! >> +[ $# -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 ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v1] travis-ci: build and test Git on Windows 2017-03-22 6:56 [PATCH v1] travis-ci: build and test Git on Windows Lars Schneider 2017-03-22 15:49 ` Johannes Schindelin 2017-03-22 19:29 ` Junio C Hamano @ 2017-03-23 20:16 ` Junio C Hamano 2 siblings, 0 replies; 24+ messages in thread From: Junio C Hamano @ 2017-03-23 20:16 UTC (permalink / raw) To: Lars Schneider; +Cc: git, Johannes.Schindelin, peff 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. ^ permalink raw reply [flat|nested] 24+ messages in thread
end of thread, other threads:[~2017-03-24 23:50 UTC | newest] Thread overview: 24+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2017-03-22 6:56 [PATCH v1] travis-ci: build and test Git on Windows Lars Schneider 2017-03-22 15:49 ` Johannes Schindelin 2017-03-23 18:19 ` Lars Schneider 2017-03-22 19:29 ` Junio C Hamano 2017-03-23 16:22 ` Johannes Schindelin 2017-03-23 18:01 ` Jeff King 2017-03-23 19:12 ` Junio C Hamano 2017-03-23 19:17 ` Jeff King 2017-03-23 19:26 ` Lars Schneider 2017-03-23 19:30 ` Junio C Hamano 2017-03-23 19:36 ` Lars Schneider 2017-03-23 20:10 ` Junio C Hamano 2017-03-23 19:38 ` Jeff King 2017-03-23 20:00 ` Lars Schneider 2017-03-23 20:20 ` Jeff King 2017-03-23 20:30 ` Junio C Hamano 2017-03-23 20:41 ` Jeff King 2017-03-23 20:39 ` Lars Schneider 2017-03-23 20:42 ` Jeff King 2017-03-24 23:50 ` Johannes Schindelin 2017-03-23 21:04 ` Samuel Lijin 2017-03-23 19:15 ` Junio C Hamano 2017-03-23 18:23 ` Lars Schneider 2017-03-23 20:16 ` Junio C Hamano
Code repositories for project(s) associated with this public inbox https://80x24.org/mirrors/git.git This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).