ci: include a Visual Studio build & test in our Azure Pipeline #288

dscho · 2019-07-16T15:26:17Z

Git's Continuous Integration (CI) includes an Azure Pipeline that builds Git on Linux, macOS and Windows, in the former two cases even in multiple configurations (using GCC vs clang, 32-bit vs 64-bit, etc). On Windows, we only build using GCC, using (a subset of) Git for Windows' SDK.

Recently, a patch series made it into Git that re-instates the ability to generate project files for use with Visual Studio. The idea there being: contributors can check out a special branch that has those generated files in one generated commit on top of e.g. Git for Windows' master, allowing the contributors to build Git in Visual Studio, without the need for downloading Git for Windows' SDK (which weighs quite a bit: ~600MB download, ~2GB disk footprint). The tests can then be run from a regular Git for Windows Bash.

This patch series adds that axis to Git's Azure Pipeline: the project files are generated, MSBuild (which is kind of the command-line equivalent of Visual Studio's "Build" operation) is used to build Git, and then a parallelized test job runs the test suite in a Portable Git.

These patches are based on js/visual-studio.

Changes since v2:

The overflow check introduced in v1 was consolidated into a single helper.

Changes since v1:

"While at it", we now also check for overflows when doing that -1 - <unsigned> arithmetic.
The JUnit-style XML is finalized also in case that the script aborts e.g. due to an incorrect number of arguments in a test_expect_success call.

Cc: Denton Liu [email protected]

dscho · 2019-07-17T18:10:42Z

/azp run

azure-pipelines · 2019-07-17T18:10:51Z

Azure Pipelines successfully started running 2 pipeline(s).

dscho · 2019-07-30T12:54:38Z

Let's rework this to kill two birds with one stone: use the generated git.sln via msbuild and test it with a PortableGit. This will require the test helper from the busybox-w32 branch in Git for Windows, as neither make nor prove work in PortableGit.

dscho · 2019-08-01T22:38:49Z

The build step should do the equivalent of msbuild /m /p:Configuration=Release;Platform=x64 /t:Rebuild git.sln. The test helper to run the test suite in parallel is here: git-for-windows@20e785b#diff-f8fe62f6e437072fb4dd6780ae91e667 (it would still need to learn how to slice and dice the test scripts, ordering them by size and then picking every k'th).

This function is marked as `NORETURN`, and it indeed does not want to return anything. So let's not declare it with the return type `int`. This fixes the following warning when building with MSVC: C4646: function declared with 'noreturn' has non-void return type Signed-off-by: Johannes Schindelin <[email protected]>

dscho · 2019-09-26T08:29:04Z

/submit

gitgitgadget · 2019-09-26T08:30:41Z

Submitted as [email protected]

WARNING: dscho has no public email address set on GitHub

read-cache.c

t/test-lib.sh

dscho · 2019-09-30T09:54:34Z

/submit

gitgitgadget · 2019-09-30T09:55:59Z

Submitted as [email protected]

builtin/push.c

read-cache.c

gitgitgadget · 2019-10-04T03:43:34Z

This branch is now known as js/azure-pipelines-msvc.

gitgitgadget · 2019-10-06T00:22:52Z

On the Git mailing list, Junio C Hamano wrote (reply to this):

"Johannes Schindelin via GitGitGadget" <[email protected]>
writes:

> Changes since v2:
>
>  * The overflow check introduced in v1 was consolidated into a single
>    helper.

Looks good to me.

> Range-diff vs v2:
>
>   1:  4d0b38125a =  1:  4d0b38125a push: do not pretend to return `int` from `die_push_simple()`
>   2:  8800320590 <  -:  ---------- msvc: avoid using minus operator on unsigned types
>   -:  ---------- >  2:  7fe2a85506 msvc: avoid using minus operator on unsigned types
>   3:  8512a3e96d =  3:  e632a4eef4 winansi: use FLEX_ARRAY to avoid compiler warning

This is less useful than it could be.  

With a larger creation-factor (and we can afford using a larger one,
simply because the user of GGG _knows_ that the two series being
compared are closely related), what is output is entirely readable
(attached at the end).

Oh, while I am suggesting possible improvements on GGG, can we
please tweak the sender date like git-send-email does so that two
messages in the same series do not share the same timestamp?  When
multi-patch series are displayed in MUA or public-inbox News feed
out of order, it almost always is from GGG that gave the same
timestamp to adjacent messages in a series, and it prevents me from
applying them in one go (or saving in one action to a mbox).

What send-email does is, at the beginning for N patch series, to
take the current wallclock time and subtract N seconds from it, and
then give that timestamp to the first message it sends out, and
after that, it increments the timestamp by 1 seconds.

Note that there is no need for any "sleep"---the timestamps are
given by explicitly generating the "Date: " header.  The last time
we looked into this issue, I think the code was trying to do almost
the right thing but it was giving a malformatted timezone and forcing
the sending MTA to override it with the wallclock time or something.

Thanks.

1:  9629f3c751 ! 1:  c097b95a26 msvc: avoid using minus operator on unsigned types
    @@ Commit message
         Signed-off-by: Johannes Schindelin <[email protected]>
         Signed-off-by: Junio C Hamano <[email protected]>
     
    + ## cache.h ##
    +@@ cache.h: struct cache_entry *index_file_exists(struct index_state *istate, const char *na
    +  */
    + int index_name_pos(const struct index_state *, const char *name, int namelen);
    + 
    ++/*
    ++ * Some functions return the negative complement of an insert position when a
    ++ * precise match was not found but a position was found where the entry would
    ++ * need to be inserted. This helper protects that logic from any integer
    ++ * underflow.
    ++ */
    ++static inline int index_pos_to_insert_pos(uintmax_t pos)
    ++{
    ++	if (pos > INT_MAX)
    ++		die("overflow: -1 - %"PRIuMAX, pos);
    ++	return -1 - (int)pos;
    ++}
    ++
    + #define ADD_CACHE_OK_TO_ADD 1		/* Ok to add */
    + #define ADD_CACHE_OK_TO_REPLACE 2	/* Ok to replace file/directory */
    + #define ADD_CACHE_SKIP_DFCHECK 4	/* Ok to skip DF conflict checks */
    +
      ## read-cache.c ##
     @@ read-cache.c: static int add_index_entry_with_check(struct index_state *istate, struct cache_e
    - 	 * we can avoid searching for it.
      	 */
      	if (istate->cache_nr > 0 &&
    --		strcmp(ce->name, istate->cache[istate->cache_nr - 1]->name) > 0)
    + 		strcmp(ce->name, istate->cache[istate->cache_nr - 1]->name) > 0)
     -		pos = -istate->cache_nr - 1;
    -+		strcmp(ce->name, istate->cache[istate->cache_nr - 1]->name) > 0) {
    -+		if (istate->cache_nr > INT_MAX)
    -+			die("overflow: -1 - %u", istate->cache_nr);
    -+		pos = -1 - (int)istate->cache_nr;
    -+	}
    ++		pos = index_pos_to_insert_pos(istate->cache_nr);
      	else
      		pos = index_name_stage_pos(istate, ce->name, ce_namelen(ce), ce_stage(ce));
      
    @@ read-cache.c: static size_t estimate_cache_size(size_t ondisk_size, unsigned int
     
      ## sha1-lookup.c ##
     @@ sha1-lookup.c: int sha1_pos(const unsigned char *sha1, void *table, size_t nr,
    - 			miv = take2(sha1 + ofs);
      			if (miv < lov)
      				return -1;
    --			if (hiv < miv)
    + 			if (hiv < miv)
     -				return -1 - nr;
    -+			if (hiv < miv) {
    -+				if (nr > INT_MAX)
    -+					die("overflow: -1 - %"PRIuMAX,
    -+					    (uintmax_t)nr);
    -+				return -1 - (int)nr;
    -+			}
    ++				return index_pos_to_insert_pos(nr);
      			if (lov != hiv) {
      				/*
      				 * At this point miv could be equal
    @@ sha1-lookup.c: int sha1_pos(const unsigned char *sha1, void *table, size_t nr,
      		mi = lo + (hi - lo) / 2;
      	} while (lo < hi);
     -	return -lo-1;
    -+	if (nr > INT_MAX)
    -+		die("overflow: -1 - %"PRIuMAX, (uintmax_t)lo);
    -+	return -1 - (int)lo;
    ++	return index_pos_to_insert_pos(lo);
      }
      
      int bsearch_hash(const unsigned char *sha1, const uint32_t *fanout_nbo,

gitgitgadget · 2019-10-06T03:56:03Z

This patch series was integrated into pu via git@84f34db.

gitgitgadget · 2019-10-06T10:50:03Z

On the Git mailing list, Johannes Schindelin wrote (reply to this):

Hi Junio,

On Sun, 6 Oct 2019, Junio C Hamano wrote:

> "Johannes Schindelin via GitGitGadget" <[email protected]>
> writes:
>
> > Range-diff vs v2:
> >
> >   1:  4d0b38125a =3D  1:  4d0b38125a push: do not pretend to return `i=
nt` from `die_push_simple()`
> >   2:  8800320590 <  -:  ---------- msvc: avoid using minus operator on=
 unsigned types
> >   -:  ---------- >  2:  7fe2a85506 msvc: avoid using minus operator on=
 unsigned types
> >   3:  8512a3e96d =3D  3:  e632a4eef4 winansi: use FLEX_ARRAY to avoid =
compiler warning
>
> This is less useful than it could be.
>
> With a larger creation-factor (and we can afford using a larger one,
> simply because the user of GGG _knows_ that the two series being
> compared are closely related), what is output is entirely readable
> (attached at the end).

I just implemented this here:
https://github.com/gitgitgadget/gitgitgadget/pull/128 (it still needs to
be reviewed and merged before it takes effect).

> Oh, while I am suggesting possible improvements on GGG, can we
> please tweak the sender date like git-send-email does so that two
> messages in the same series do not share the same timestamp?  When
> multi-patch series are displayed in MUA or public-inbox News feed
> out of order, it almost always is from GGG that gave the same
> timestamp to adjacent messages in a series, and it prevents me from
> applying them in one go (or saving in one action to a mbox).
>
> What send-email does is, at the beginning for N patch series, to
> take the current wallclock time and subtract N seconds from it, and
> then give that timestamp to the first message it sends out, and
> after that, it increments the timestamp by 1 seconds.
>
> Note that there is no need for any "sleep"---the timestamps are
> given by explicitly generating the "Date: " header.  The last time
> we looked into this issue, I think the code was trying to do almost
> the right thing but it was giving a malformatted timezone and forcing
> the sending MTA to override it with the wallclock time or something.

You mentioned this before, and I implemented it. But GMail ignores the
`Date:` header sent by GitGitGadget, and I don't know why. See e.g.
https://public-inbox.org/git/4d0b38125a13d85963be5e524becf48271893e2b.1570=
[email protected]/raw

	[...]
	Date:   Fri, 04 Oct 2019 08:09:25 -0700 (PDT)
	[...]
	X-Google-Original-Date: Fri, 04 Oct 2019 15:09:10 GMT
	[...]

I am fairly certain that the latter is the actual `Date:` line sent to
GMail, and GMail just decides that it will not respect it.

Once https://github.com/gitgitgadget/gitgitgadget/pull/125 makes it into
GitGitGadget (adding the `/preview` command that allows to send patch
series to the PR owner as a test), it should be easier to start
investigating further.

Unless anybody here knows why GMail rejects the header. Maybe it is the
`GMT`?

Ciao,
Dscho

gitgitgadget · 2019-10-06T20:41:08Z

On the Git mailing list, Johannes Schindelin wrote (reply to this):

Hi Junio,

On Sun, 6 Oct 2019, Johannes Schindelin wrote:

> On Sun, 6 Oct 2019, Junio C Hamano wrote:
>
> > "Johannes Schindelin via GitGitGadget" <[email protected]>
> > writes:
> >
> > > Range-diff vs v2:
> > >
> > >   1:  4d0b38125a =3D  1:  4d0b38125a push: do not pretend to return =
`int` from `die_push_simple()`
> > >   2:  8800320590 <  -:  ---------- msvc: avoid using minus operator =
on unsigned types
> > >   -:  ---------- >  2:  7fe2a85506 msvc: avoid using minus operator =
on unsigned types
> > >   3:  8512a3e96d =3D  3:  e632a4eef4 winansi: use FLEX_ARRAY to avoi=
d compiler warning
> >
> > This is less useful than it could be.
> >
> > With a larger creation-factor (and we can afford using a larger one,
> > simply because the user of GGG _knows_ that the two series being
> > compared are closely related), what is output is entirely readable
> > (attached at the end).
>
> I just implemented this here:
> https://github.com/gitgitgadget/gitgitgadget/pull/128 (it still needs to
> be reviewed and merged before it takes effect).

FWIW this is now merged.

Ciao,
Dscho

gitgitgadget · 2019-10-07T01:02:18Z

On the Git mailing list, Junio C Hamano wrote (reply to this):

Johannes Schindelin <[email protected]> writes:

> 	Date:   Fri, 04 Oct 2019 08:09:25 -0700 (PDT)
> 	[...]
> 	X-Google-Original-Date: Fri, 04 Oct 2019 15:09:10 GMT
> 	[...]
>
> I am fairly certain that the latter is the actual `Date:` line sent to
> GMail, and GMail just decides that it will not respect it.

If the submitting program said "Fri, 04 Oct 2019 15:09:10 +0000
(GMT)" instead of "Fri, 04 Oct 2019 15:09:10 GMT", that would match
the format the MTA produced itself, I guess.  I am kind-of surprised
if the problem is the use of the obs-zone format (RFC 2822 page 31),
but anything is possible with GMail X-<.

How does send-email write that date header?  Matching that would be
probably the most appropriate, if possible, given that GGG was
written for send-email refugees, I guess ;-)

Here is what its format_2822_time sub does, so +0000 without any
textual zone name, it is.

	return sprintf("%s, %2d %s %d %02d:%02d:%02d %s%02d%02d",
		       qw(Sun Mon Tue Wed Thu Fri Sat)[$localtm[6]],
		       $localtm[3],
		       qw(Jan Feb Mar Apr May Jun
			  Jul Aug Sep Oct Nov Dec)[$localtm[4]],
		       $localtm[5]+1900,
		       $localtm[2],
		       $localtm[1],
		       $localtm[0],
		       ($offset >= 0) ? '+' : '-',
		       abs($offhour),
		       $offmin,
		       );

gitgitgadget · 2019-10-07T01:17:17Z

On the Git mailing list, Junio C Hamano wrote (reply to this):

Johannes Schindelin <[email protected]> writes:

>> I just implemented this here:
>> https://github.com/gitgitgadget/gitgitgadget/pull/128 (it still needs to
>> be reviewed and merged before it takes effect).
>
> FWIW this is now merged.

Nice.

I didn't quite understand this part, though.

    The default creation factor is 60 (roughly speaking, it wants 60% of
    the lines to match between two patches, otherwise it considers the
    patches to be unrelated).

Would the updated creation factor used which is 95 (roughly
speaking) want 95% of the lines to match between two patches?

That would make the matching logic even pickier and reject more
paring, so I must be reading the statement wrong X-<.

gitgitgadget · 2019-10-07T03:47:26Z

This patch series was integrated into pu via git@f21a852.

gitgitgadget · 2019-10-07T03:47:27Z

This patch series was integrated into next via git@d5a3604.

gitgitgadget · 2019-10-07T16:11:36Z

On the Git mailing list, Thomas Gummerer wrote (reply to this):

On 10/07, Junio C Hamano wrote:
> Johannes Schindelin <[email protected]> writes:
> 
> > 	Date:   Fri, 04 Oct 2019 08:09:25 -0700 (PDT)
> > 	[...]
> > 	X-Google-Original-Date: Fri, 04 Oct 2019 15:09:10 GMT
> > 	[...]
> >
> > I am fairly certain that the latter is the actual `Date:` line sent to
> > GMail, and GMail just decides that it will not respect it.
> 
> If the submitting program said "Fri, 04 Oct 2019 15:09:10 +0000
> (GMT)" instead of "Fri, 04 Oct 2019 15:09:10 GMT", that would match
> the format the MTA produced itself, I guess.  I am kind-of surprised
> if the problem is the use of the obs-zone format (RFC 2822 page 31),
> but anything is possible with GMail X-<.

Yeah, the obs-zone format did seem to be the problem.  I just dug up
the previous thread we had about this, where I confirmed that +0000 as
the timezone worked just fine in my setup through GMail [*1*].  Note
sure if the (GMT) would cause any problems, but I'd agree with
avoiding it as you mention below to make sure GMail doesn't do
anything funny with it.

*1*: https://public-inbox.org/git/[email protected]/

> How does send-email write that date header?  Matching that would be
> probably the most appropriate, if possible, given that GGG was
> written for send-email refugees, I guess ;-)
> 
> Here is what its format_2822_time sub does, so +0000 without any
> textual zone name, it is.
> 
> 	return sprintf("%s, %2d %s %d %02d:%02d:%02d %s%02d%02d",
> 		       qw(Sun Mon Tue Wed Thu Fri Sat)[$localtm[6]],
> 		       $localtm[3],
> 		       qw(Jan Feb Mar Apr May Jun
> 			  Jul Aug Sep Oct Nov Dec)[$localtm[4]],
> 		       $localtm[5]+1900,
> 		       $localtm[2],
> 		       $localtm[1],
> 		       $localtm[0],
> 		       ($offset >= 0) ? '+' : '-',
> 		       abs($offhour),
> 		       $offmin,
> 		       );
> 
>

gitgitgadget · 2019-10-07T19:17:54Z

compat/winansi.c

@@ -546,7 +546,7 @@ static HANDLE swap_osfhnd(int fd, HANDLE new_handle)
 typedef struct _OBJECT_NAME_INFORMATION


On the Git mailing list, Alban Gruin wrote (reply to this):

Hi Johannes, Le 04/10/2019 à 17:09, Johannes Schindelin via GitGitGadget a écrit : > From: Johannes Schindelin <[email protected]> > > MSVC would complain thusly: > > C4200: nonstandard extension used: zero-sized array in struct/union > > Let's just use the `FLEX_ARRAY` constant that we introduced for exactly > this type of scenario. Perhaps this is a good candidate for a semantic patch? Cheers, Alban

gitgitgadget · 2019-10-08T05:08:41Z

This patch series was integrated into pu via git@d5caf5b.

gitgitgadget · 2019-10-09T03:34:53Z

This patch series was integrated into pu via git@2baa6cf.

gitgitgadget · 2019-10-09T07:52:12Z

This patch series was integrated into pu via git@3a9af0c.

gitgitgadget · 2019-10-11T07:44:44Z

This patch series was integrated into pu via git@47102f2.

gitgitgadget · 2019-10-15T07:24:58Z

This patch series was integrated into pu via git@6d5291b.

gitgitgadget · 2019-10-15T07:25:00Z

This patch series was integrated into next via git@6d5291b.

gitgitgadget · 2019-10-15T07:25:01Z

This patch series was integrated into master via git@6d5291b.

gitgitgadget · 2019-10-15T07:25:04Z

Closed via 6d5291b.

dscho added the needs-work These patches have pending issues that need to be resolved before submitting label Jul 16, 2019

dscho force-pushed the azure-pipelines-msvc branch from 4c664c9 to 09dc727 Compare July 17, 2019 14:04

dscho force-pushed the azure-pipelines-msvc branch from 09dc727 to 633a51c Compare July 18, 2019 12:17

dscho changed the base branch from master to js/visual-studio July 19, 2019 12:35

dscho force-pushed the azure-pipelines-msvc branch from 633a51c to 4ef33d3 Compare July 19, 2019 12:41

weekly-digest bot mentioned this pull request Jul 21, 2019

Weekly Digest (14 July, 2019 - 21 July, 2019) #291

Closed

dscho force-pushed the js/visual-studio branch from 638ef1f to 6b408a7 Compare July 29, 2019 18:46

dscho force-pushed the js/visual-studio branch from 6b408a7 to b914084 Compare July 30, 2019 21:47

dscho force-pushed the azure-pipelines-msvc branch 2 times, most recently from 1ca1aa2 to ece2969 Compare September 25, 2019 18:25

dscho added ready to submit Has commits that have not been submitted yet and removed needs-work These patches have pending issues that need to be resolved before submitting labels Sep 25, 2019

dscho changed the title ~~ci: automatically build Git using MSVC in our Azure Pipeline~~ ci: include a Visual Studio build & test in our Azure Pipeline Sep 25, 2019

dscho removed the ready to submit Has commits that have not been submitted yet label Sep 26, 2019

gitgitgadget bot reviewed Sep 26, 2019

View reviewed changes

read-cache.c Show resolved Hide resolved

gitgitgadget bot reviewed Sep 28, 2019

View reviewed changes

t/test-lib.sh Show resolved Hide resolved

weekly-digest bot mentioned this pull request Sep 29, 2019

Weekly Digest (22 September, 2019 - 29 September, 2019) #366

Closed

dscho force-pushed the azure-pipelines-msvc branch from ece2969 to 7af1c01 Compare September 30, 2019 08:53

gitgitgadget bot reviewed Oct 3, 2019

View reviewed changes

builtin/push.c Show resolved Hide resolved

gitgitgadget bot reviewed Oct 3, 2019

View reviewed changes

read-cache.c Outdated Show resolved Hide resolved

gitgitgadget bot added the next label Oct 7, 2019

gitgitgadget bot reviewed Oct 7, 2019

View reviewed changes

This comment has been minimized.

Sign in to view

weekly-digest bot mentioned this pull request Oct 13, 2019

Weekly Digest (6 October, 2019 - 13 October, 2019) #393

Closed

gitgitgadget bot added the master label Oct 15, 2019

gitgitgadget bot closed this Oct 15, 2019

dscho deleted the azure-pipelines-msvc branch October 15, 2019 14:04

		@@ -546,7 +546,7 @@ static HANDLE swap_osfhnd(int fd, HANDLE new_handle)
		typedef struct _OBJECT_NAME_INFORMATION

ci: include a Visual Studio build & test in our Azure Pipeline #288

ci: include a Visual Studio build & test in our Azure Pipeline #288

Uh oh!

Conversation

dscho commented Jul 16, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dscho commented Jul 17, 2019

Uh oh!

azure-pipelines bot commented Jul 17, 2019

Uh oh!

dscho commented Jul 30, 2019

Uh oh!

dscho commented Aug 1, 2019

Uh oh!

dscho commented Sep 26, 2019

Uh oh!

gitgitgadget bot commented Sep 26, 2019

Uh oh!

Uh oh!

Uh oh!

dscho commented Sep 30, 2019

Uh oh!

gitgitgadget bot commented Sep 30, 2019

Uh oh!

Uh oh!

Uh oh!

gitgitgadget bot commented Oct 4, 2019

Uh oh!

gitgitgadget bot commented Oct 6, 2019

Uh oh!

gitgitgadget bot commented Oct 6, 2019

Uh oh!

gitgitgadget bot commented Oct 6, 2019

Uh oh!

gitgitgadget bot commented Oct 6, 2019

Uh oh!

gitgitgadget bot commented Oct 7, 2019

Uh oh!

gitgitgadget bot commented Oct 7, 2019

Uh oh!

gitgitgadget bot commented Oct 7, 2019

Uh oh!

gitgitgadget bot commented Oct 7, 2019

Uh oh!

gitgitgadget bot commented Oct 7, 2019

Uh oh!

gitgitgadget bot Oct 7, 2019

Choose a reason for hiding this comment

Uh oh!

This comment has been minimized.

This comment has been minimized.

gitgitgadget bot commented Oct 8, 2019

Uh oh!

This comment has been minimized.

gitgitgadget bot commented Oct 9, 2019

Uh oh!

gitgitgadget bot commented Oct 9, 2019

Uh oh!

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

gitgitgadget bot commented Oct 11, 2019

Uh oh!

This comment has been minimized.

gitgitgadget bot commented Oct 15, 2019

Uh oh!

gitgitgadget bot commented Oct 15, 2019

Uh oh!

gitgitgadget bot commented Oct 15, 2019

Uh oh!

gitgitgadget bot commented Oct 15, 2019

Uh oh!

Uh oh!

dscho commented Jul 16, 2019 •

edited

Loading