Re: Windows build on Travis CI (was: Re: [PATCH v2 01/36] t/helper: add an empty test-tool program)

[Johannes Schindelin]

Hi Gábor,

On Tue, 27 Mar 2018, SZEDER Gábor wrote:

> On Tue, Mar 27, 2018 at 3:57 PM, Johannes Schindelin
>  wrote:
> >
> > On Tue, 27 Mar 2018, SZEDER Gábor wrote:
> >
> >> On Tue, Mar 27, 2018 at 12:14 AM, Johannes Schindelin
> >>  wrote:
> >> > However, it seems that something is off, as
> >> > ba5bec9589e9eefe2446044657963e25b7c8d88e is reported as fine on Windows:
> >> > https://travis-ci.org/git/git/jobs/358260023 (while there is clearly a 
> >> > red
> >> > X next to that commit in
> >> > https://github.com/git/git/commits/ba5bec9589e9eefe2446044657963e25b7c8d88e,
> >> > that X is due to a hiccup on macOS).
> >> >
> >> > It seems that the good-trees feature for Travis does not quite work as
> >> > intended. Gábor?
> >>
> >> AFAICT it works as expected.
> >>
> >> When a build job encounters a commit with a tree that has previously
> >> been built and tested successfully, then first it says so, like this:
> >>
> >>   https://travis-ci.org/szeder/git/jobs/347295038#L635
> >
> > But what if it has not been built successfully (as was the case here)?
> > This very commit that is "succeeding" on Travis fails to compile on
> > Windows.
> 
> Then why has the GfW web app reported success?
> 
>   https://travis-ci.org/git/git/jobs/358260023#L512

Oy. There was a shift in build steps, so that shows the wrong output. The
correct build step ends thusly:

-- snip --
[...]
2018-03-26T06:50:55.371Z Checking out files:  97% (3136/3232)   
2018-03-26T06:50:55.0106984Z Checking out files:  98% (3168/3232)   
2018-03-26T06:50:55.0223806Z Checking out files:  99% (3200/3232)   
2018-03-26T06:50:55.0227819Z Checking out files: 100% (3232/3232)   
2018-03-26T06:50:55.0228191Z Checking out files: 100% (3232/3232), done.
2018-03-26T06:50:55.0343621Z HEAD is now at 90bbd502d Sync with Git 2.16.3
2018-03-26T06:50:55.0759061Z Updating upstream
2018-03-26T06:50:56.3001946Z From https://github.com/git/git
2018-03-26T06:50:56.3002737Z  * [new branch]  maint  -> 
upstream/maint
2018-03-26T06:50:56.3003056Z  * [new branch]  master -> 
upstream/master
2018-03-26T06:50:56.3003832Z  * [new branch]  next   -> 
upstream/next
2018-03-26T06:50:56.3354328Z  * [new branch]  pu -> upstream/pu
2018-03-26T06:50:56.3354880Z  * [new branch]  todo   -> 
upstream/todo
2018-03-26T06:50:56.8219992Z fatal: Not a valid commit name 
7a6a7fb7d0ab1052db113318478f9e40e66e59dc
2018-03-26T06:50:56.8236547Z Commit 7a6a7fb7d0ab1052db113318478f9e40e66e59dc is 
not on branch upstream/master; skipping
```

So as you see, by the time we fetched `pu`, it was no longer reachable
(otherwise we would have been able to fetch it).

That's a bummer.

Ciao,
Dscho

Windows build on Travis CI (was: Re: [PATCH v2 01/36] t/helper: add an empty test-tool program)

[SZEDER Gábor]

On Tue, Mar 27, 2018 at 3:57 PM, Johannes Schindelin
 wrote:
> Hi Gábor,
>
> On Tue, 27 Mar 2018, SZEDER Gábor wrote:
>
>> On Tue, Mar 27, 2018 at 12:14 AM, Johannes Schindelin
>>  wrote:
>> > However, it seems that something is off, as
>> > ba5bec9589e9eefe2446044657963e25b7c8d88e is reported as fine on Windows:
>> > https://travis-ci.org/git/git/jobs/358260023 (while there is clearly a red
>> > X next to that commit in
>> > https://github.com/git/git/commits/ba5bec9589e9eefe2446044657963e25b7c8d88e,
>> > that X is due to a hiccup on macOS).
>> >
>> > It seems that the good-trees feature for Travis does not quite work as
>> > intended. Gábor?
>>
>> AFAICT it works as expected.
>>
>> When a build job encounters a commit with a tree that has previously
>> been built and tested successfully, then first it says so, like this:
>>
>>   https://travis-ci.org/szeder/git/jobs/347295038#L635
>
> But what if it has not been built successfully (as was the case here)?
> This very commit that is "succeeding" on Travis fails to compile on
> Windows.

Then why has the GfW web app reported success?

  https://travis-ci.org/git/git/jobs/358260023#L512

>> and then skips the rest of the build job (see the 'exit 0' a few lines
>> later).
>>
>> In case of this Windows build job we haven't seen this tree yet:
>>
>>   https://travis-ci.org/git/git/jobs/358260023#L467
>>
>> so the build job continues as usual (see the 'test -z Windows' two lines
>> later).
>>
>> Unfortunately, I have no idea about how the rest of the Windows build
>> job is supposed to work...
>
> Maybe Travis timed out waiting for the result, and marked it as a success?

This Windows build ran for 9 min 27 sec, i.e. not long enough for a
timeout on Travis CI.  (OTOH, clearly not long enough to build Git and
run the test suite on Windows, I know.)

BTW, a timeouted build job is marked as "errored" and the timeout is
mentioned in its trace log:

  https://travis-ci.org/git/git/jobs/331669291#L509

Re: [PATCH v2 01/36] t/helper: add an empty test-tool program

[Johannes Schindelin]

Hi Gábor,

On Tue, 27 Mar 2018, SZEDER Gábor wrote:

> On Tue, Mar 27, 2018 at 12:14 AM, Johannes Schindelin
>  wrote:
> > However, it seems that something is off, as
> > ba5bec9589e9eefe2446044657963e25b7c8d88e is reported as fine on Windows:
> > https://travis-ci.org/git/git/jobs/358260023 (while there is clearly a red
> > X next to that commit in
> > https://github.com/git/git/commits/ba5bec9589e9eefe2446044657963e25b7c8d88e,
> > that X is due to a hiccup on macOS).
> >
> > It seems that the good-trees feature for Travis does not quite work as
> > intended. Gábor?
> 
> AFAICT it works as expected.
> 
> When a build job encounters a commit with a tree that has previously
> been built and tested successfully, then first it says so, like this:
> 
>   https://travis-ci.org/szeder/git/jobs/347295038#L635

But what if it has not been built successfully (as was the case here)?
This very commit that is "succeeding" on Travis fails to compile on
Windows.

> and then skips the rest of the build job (see the 'exit 0' a few lines
> later).
> 
> In case of this Windows build job we haven't seen this tree yet:
> 
>   https://travis-ci.org/git/git/jobs/358260023#L467
> 
> so the build job continues as usual (see the 'test -z Windows' two lines
> later).
> 
> Unfortunately, I have no idea about how the rest of the Windows build
> job is supposed to work...

Maybe Travis timed out waiting for the result, and marked it as a success?

Ciao,
Dscho

Re: [PATCH v2 01/36] t/helper: add an empty test-tool program

[SZEDER Gábor]

On Tue, Mar 27, 2018 at 12:14 AM, Johannes Schindelin
 wrote:
> However, it seems that something is off, as
> ba5bec9589e9eefe2446044657963e25b7c8d88e is reported as fine on Windows:
> https://travis-ci.org/git/git/jobs/358260023 (while there is clearly a red
> X next to that commit in
> https://github.com/git/git/commits/ba5bec9589e9eefe2446044657963e25b7c8d88e,
> that X is due to a hiccup on macOS).
>
> It seems that the good-trees feature for Travis does not quite work as
> intended. Gábor?

AFAICT it works as expected.

When a build job encounters a commit with a tree that has previously
been built and tested successfully, then first it says so, like this:

  https://travis-ci.org/szeder/git/jobs/347295038#L635

and then skips the rest of the build job (see the 'exit 0' a few lines
later).

In case of this Windows build job we haven't seen this tree yet:

  https://travis-ci.org/git/git/jobs/358260023#L467

so the build job continues as usual (see the 'test -z Windows' two lines
later).

Unfortunately, I have no idea about how the rest of the Windows build
job is supposed to work...

Re: [PATCH v2 01/36] t/helper: add an empty test-tool program

[Johannes Schindelin]

Hi Duy,

On Mon, 26 Mar 2018, Duy Nguyen wrote:

> On Mon, Mar 26, 2018 at 5:27 PM, Johannes Schindelin
>  wrote:
> >
> > On Sat, 24 Mar 2018, Nguyễn Thái Ngọc Duy wrote:
> >
> >> diff --git a/t/helper/test-tool.c b/t/helper/test-tool.c
> >> new file mode 100644
> >> index 00..c730f718ca
> >> --- /dev/null
> >> +++ b/t/helper/test-tool.c
> >> @@ -0,0 +1,27 @@
> >> +#include "git-compat-util.h"
> >> +#include "test-tool.h"
> >> +
> >> +struct test_cmd {
> >> + const char *name;
> >> + int (*main)(int argc, const char **argv);
> >
> > This makes the build fail on Windows, as we override `main` in
> > compat/mingw.h:
> 
> Sigh.. not complaining, but I wish somebody tries to compile git with
> wine (and automate it in travis). This way we could at least cover the
> compilation part for all major platforms. Probably too small for a
> GSoC (and making the test suite pass with wine may be too large for
> GSoC)

We do have Continuous Testing of maint, master, next & pu.

However, it seems that something is off, as
ba5bec9589e9eefe2446044657963e25b7c8d88e is reported as fine on Windows:
https://travis-ci.org/git/git/jobs/358260023 (while there is clearly a red
X next to that commit in
https://github.com/git/git/commits/ba5bec9589e9eefe2446044657963e25b7c8d88e,
that X is due to a hiccup on macOS).

It seems that the good-trees feature for Travis does not quite work as
intended. Gábor?

Ciao,
Dscho

Re: [PATCH v2 01/36] t/helper: add an empty test-tool program

[Duy Nguyen]

On Mon, Mar 26, 2018 at 5:27 PM, Johannes Schindelin
 wrote:
> Hi Duy,
>
> On Sat, 24 Mar 2018, Nguyễn Thái Ngọc Duy wrote:
>
>> diff --git a/t/helper/test-tool.c b/t/helper/test-tool.c
>> new file mode 100644
>> index 00..c730f718ca
>> --- /dev/null
>> +++ b/t/helper/test-tool.c
>> @@ -0,0 +1,27 @@
>> +#include "git-compat-util.h"
>> +#include "test-tool.h"
>> +
>> +struct test_cmd {
>> + const char *name;
>> + int (*main)(int argc, const char **argv);
>
> This makes the build fail on Windows, as we override `main` in
> compat/mingw.h:

Sigh.. not complaining, but I wish somebody tries to compile git with
wine (and automate it in travis). This way we could at least cover the
compilation part for all major platforms. Probably too small for a
GSoC (and making the test suite pass with wine may be too large for
GSoC)
-- 
Duy

Re: [PATCH v2 01/36] t/helper: add an empty test-tool program

[Johannes Schindelin]

Hi Duy,

On Sat, 24 Mar 2018, Nguyễn Thái Ngọc Duy wrote:

> diff --git a/t/helper/test-tool.c b/t/helper/test-tool.c
> new file mode 100644
> index 00..c730f718ca
> --- /dev/null
> +++ b/t/helper/test-tool.c
> @@ -0,0 +1,27 @@
> +#include "git-compat-util.h"
> +#include "test-tool.h"
> +
> +struct test_cmd {
> + const char *name;
> + int (*main)(int argc, const char **argv);

This makes the build fail on Windows, as we override `main` in
compat/mingw.h:

/*
 * A replacement of main() that adds win32 specific initialization.
 */

void mingw_startup(void);
#define main(c,v) dummy_decl_mingw_main(void); \
static int mingw_main(c,v); \
int main(int argc, const char **argv) \
{ \
mingw_startup(); \
return mingw_main(__argc, (void *)__argv); \
} \
static int mingw_main(c,v)


I know, I know, now that common-main.c is a thing, we could simply pluck a
`platform_specific_pre_main()` or some such, which is overridden in
compat/mingw.h to `mingw_startup` and to a no-op in git-compat-util.h as a
fall-back.

The truth is: I simply did not get around to doing this yet.

In the meantime, could we have this SQUASH???, please?

-- snipsnap --
diff --git a/t/helper/test-tool.c b/t/helper/test-tool.c
index cd5e28b045a..87066ced62a 100644
--- a/t/helper/test-tool.c
+++ b/t/helper/test-tool.c
@@ -3,7 +3,7 @@
 
 struct test_cmd {
const char *name;
-   int (*main)(int argc, const char **argv);
+   int (*fn)(int argc, const char **argv);
 };
 
 static struct test_cmd cmds[] = {
@@ -55,7 +55,7 @@ int cmd_main(int argc, const char **argv)
if (!strcmp(cmds[i].name, argv[1])) {
argv++;
argc--;
-   return cmds[i].main(argc, argv);
+   return cmds[i].fn(argc, argv);
}
}
die("There is no test named '%s'", argv[1]);

[PATCH v2 01/36] t/helper: add an empty test-tool program

[Nguyễn Thái Ngọc Duy]

This will become an umbrella program that absorbs most [1] t/helper
programs in. By having a single executable binary we reduce disk usage
(libgit.a is replicated by every t/helper program) and shorten link
time a bit.

Running "make --jobs=1; du -sh t/helper" with ccache fully populated,
it takes 27 seconds and 277MB at the beginning of this series, 17
seconds and 42MB at the end.

[1] There are a couple programs that will not become part of
test-tool: test-line-buffer and test-svn-fe have extra
dependencies and test-fake-ssh's program name has to be a single
word for some ssh tests.

Signed-off-by: Nguyễn Thái Ngọc Duy 
---
 Makefile |  6 +-
 t/helper/test-tool.c | 27 +++
 t/helper/test-tool.h |  4 
 3 files changed, 36 insertions(+), 1 deletion(-)
 create mode 100644 t/helper/test-tool.c
 create mode 100644 t/helper/test-tool.h

diff --git a/Makefile b/Makefile
index a1d8775adb..2376646e98 100644
--- a/Makefile
+++ b/Makefile
@@ -546,6 +546,7 @@ SCRIPT_PERL =
 SCRIPT_PYTHON =
 SCRIPT_SH =
 SCRIPT_LIB =
+TEST_BUILTINS_OBJS =
 TEST_PROGRAMS_NEED_X =
 
 # Having this variable in your environment would break pipelines because
@@ -690,6 +691,7 @@ TEST_PROGRAMS_NEED_X += test-string-list
 TEST_PROGRAMS_NEED_X += test-submodule-config
 TEST_PROGRAMS_NEED_X += test-subprocess
 TEST_PROGRAMS_NEED_X += test-svn-fe
+TEST_PROGRAMS_NEED_X += test-tool
 TEST_PROGRAMS_NEED_X += test-urlmatch-normalization
 TEST_PROGRAMS_NEED_X += test-wildmatch
 
@@ -2083,7 +2085,7 @@ VCSSVN_OBJS += vcs-svn/fast_export.o
 VCSSVN_OBJS += vcs-svn/svndiff.o
 VCSSVN_OBJS += vcs-svn/svndump.o
 
-TEST_OBJS := $(patsubst %$X,%.o,$(TEST_PROGRAMS))
+TEST_OBJS := $(patsubst %$X,%.o,$(TEST_PROGRAMS)) $(patsubst 
%,t/helper/%,$(TEST_BUILTINS_OBJS))
 OBJECTS := $(LIB_OBJS) $(BUILTIN_OBJS) $(PROGRAM_OBJS) $(TEST_OBJS) \
$(XDIFF_OBJS) \
$(VCSSVN_OBJS) \
@@ -2494,6 +2496,8 @@ t/helper/test-svn-fe$X: $(VCSSVN_LIB)
 
 .PRECIOUS: $(TEST_OBJS)
 
+t/helper/test-tool$X: $(patsubst %,t/helper/%,$(TEST_BUILTINS_OBJS))
+
 t/helper/test-%$X: t/helper/test-%.o GIT-LDFLAGS $(GITLIBS)
$(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) 
$(filter %.a,$^) $(LIBS)
 
diff --git a/t/helper/test-tool.c b/t/helper/test-tool.c
new file mode 100644
index 00..c730f718ca
--- /dev/null
+++ b/t/helper/test-tool.c
@@ -0,0 +1,27 @@
+#include "git-compat-util.h"
+#include "test-tool.h"
+
+struct test_cmd {
+   const char *name;
+   int (*main)(int argc, const char **argv);
+};
+
+static struct test_cmd cmds[] = {
+};
+
+int cmd_main(int argc, const char **argv)
+{
+   int i;
+
+   if (argc < 2)
+   die("I need a test name!");
+
+   for (i = 0; i < ARRAY_SIZE(cmds); i++) {
+   if (!strcmp(cmds[i].name, argv[1])) {
+   argv++;
+   argc--;
+   return cmds[i].main(argc, argv);
+   }
+   }
+   die("There is no test named '%s'", argv[1]);
+}
diff --git a/t/helper/test-tool.h b/t/helper/test-tool.h
new file mode 100644
index 00..6ce57ae0cc
--- /dev/null
+++ b/t/helper/test-tool.h
@@ -0,0 +1,4 @@
+#ifndef __TEST_TOOL_H__
+#define __TEST_TOOL_H__
+
+#endif
-- 
2.17.0.rc0.348.gd5a49e0b6f

[PATCH v2 01/36] t/helper: add an empty test-tool program

[Nguyễn Thái Ngọc Duy]

This will become an umbrella program that absorbs most [1] t/helper
programs in. By having a single executable binary we reduce disk usage
(libgit.a is replicated by every t/helper program) and shorten link
time a bit.

Running "make --jobs=1; du -sh t/helper" with ccache fully populated,
it takes 27 seconds and 277MB at the beginning of this series, 17
seconds and 42MB at the end.

[1] There are a couple programs that will not become part of
test-tool: test-line-buffer and test-svn-fe have extra
dependencies and test-fake-ssh's program name has to be a single
word for some ssh tests.

Signed-off-by: Nguyễn Thái Ngọc Duy 
---
 Makefile |  6 +-
 t/helper/test-tool.c | 27 +++
 t/helper/test-tool.h |  4 
 3 files changed, 36 insertions(+), 1 deletion(-)
 create mode 100644 t/helper/test-tool.c
 create mode 100644 t/helper/test-tool.h

diff --git a/Makefile b/Makefile
index a1d8775adb..2376646e98 100644
--- a/Makefile
+++ b/Makefile
@@ -546,6 +546,7 @@ SCRIPT_PERL =
 SCRIPT_PYTHON =
 SCRIPT_SH =
 SCRIPT_LIB =
+TEST_BUILTINS_OBJS =
 TEST_PROGRAMS_NEED_X =
 
 # Having this variable in your environment would break pipelines because
@@ -690,6 +691,7 @@ TEST_PROGRAMS_NEED_X += test-string-list
 TEST_PROGRAMS_NEED_X += test-submodule-config
 TEST_PROGRAMS_NEED_X += test-subprocess
 TEST_PROGRAMS_NEED_X += test-svn-fe
+TEST_PROGRAMS_NEED_X += test-tool
 TEST_PROGRAMS_NEED_X += test-urlmatch-normalization
 TEST_PROGRAMS_NEED_X += test-wildmatch
 
@@ -2083,7 +2085,7 @@ VCSSVN_OBJS += vcs-svn/fast_export.o
 VCSSVN_OBJS += vcs-svn/svndiff.o
 VCSSVN_OBJS += vcs-svn/svndump.o
 
-TEST_OBJS := $(patsubst %$X,%.o,$(TEST_PROGRAMS))
+TEST_OBJS := $(patsubst %$X,%.o,$(TEST_PROGRAMS)) $(patsubst 
%,t/helper/%,$(TEST_BUILTINS_OBJS))
 OBJECTS := $(LIB_OBJS) $(BUILTIN_OBJS) $(PROGRAM_OBJS) $(TEST_OBJS) \
$(XDIFF_OBJS) \
$(VCSSVN_OBJS) \
@@ -2494,6 +2496,8 @@ t/helper/test-svn-fe$X: $(VCSSVN_LIB)
 
 .PRECIOUS: $(TEST_OBJS)
 
+t/helper/test-tool$X: $(patsubst %,t/helper/%,$(TEST_BUILTINS_OBJS))
+
 t/helper/test-%$X: t/helper/test-%.o GIT-LDFLAGS $(GITLIBS)
$(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) 
$(filter %.a,$^) $(LIBS)
 
diff --git a/t/helper/test-tool.c b/t/helper/test-tool.c
new file mode 100644
index 00..c730f718ca
--- /dev/null
+++ b/t/helper/test-tool.c
@@ -0,0 +1,27 @@
+#include "git-compat-util.h"
+#include "test-tool.h"
+
+struct test_cmd {
+   const char *name;
+   int (*main)(int argc, const char **argv);
+};
+
+static struct test_cmd cmds[] = {
+};
+
+int cmd_main(int argc, const char **argv)
+{
+   int i;
+
+   if (argc < 2)
+   die("I need a test name!");
+
+   for (i = 0; i < ARRAY_SIZE(cmds); i++) {
+   if (!strcmp(cmds[i].name, argv[1])) {
+   argv++;
+   argc--;
+   return cmds[i].main(argc, argv);
+   }
+   }
+   die("There is no test named '%s'", argv[1]);
+}
diff --git a/t/helper/test-tool.h b/t/helper/test-tool.h
new file mode 100644
index 00..6ce57ae0cc
--- /dev/null
+++ b/t/helper/test-tool.h
@@ -0,0 +1,4 @@
+#ifndef __TEST_TOOL_H__
+#define __TEST_TOOL_H__
+
+#endif
-- 
2.17.0.rc0.348.gd5a49e0b6f