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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



> On 22 Mar 2017, at 20:29, Junio C Hamano <gitster@xxxxxxxxx> wrote:
> 
> Lars Schneider <larsxschneider@xxxxxxxxx> 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



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]