git@vger.kernel.org list mirror (unofficial, one of many)
 help / color / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: "Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com>
Cc: git@vger.kernel.org, Denton Liu <liu.denton@gmail.com>,
	Johannes Schindelin <johannes.schindelin@gmx.de>
Subject: Re: [PATCH v3 00/13] ci: include a Visual Studio build & test in our Azure Pipeline
Date: Sun, 06 Oct 2019 09:19:03 +0900
Message-ID: <xmqqimp26808.fsf@gitster-ct.c.googlers.com> (raw)
In-Reply-To: <pull.288.v3.git.gitgitgadget@gmail.com> (Johannes Schindelin via GitGitGadget's message of "Fri, 04 Oct 2019 08:09:23 -0700 (PDT)")

"Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com>
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 <johannes.schindelin@gmx.de>
         Signed-off-by: Junio C Hamano <gitster@pobox.com>
     
    + ## 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,


  parent reply index

Thread overview: 72+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-09-26  8:30 [PATCH " Johannes Schindelin via GitGitGadget
2019-09-26  8:30 ` [PATCH 01/13] push: do not pretend to return `int` from `die_push_simple()` Johannes Schindelin via GitGitGadget
2019-09-26  8:30 ` [PATCH 02/13] msvc: avoid using minus operator on unsigned types Johannes Schindelin via GitGitGadget
2019-09-26 17:20   ` Denton Liu
2019-09-26 21:01     ` Johannes Schindelin
2019-09-26 23:57       ` Denton Liu
2019-09-30  9:50         ` Johannes Schindelin
2019-09-26  8:30 ` [PATCH 03/13] winansi: use FLEX_ARRAY to avoid compiler warning Johannes Schindelin via GitGitGadget
2019-09-26  8:30 ` [PATCH 04/13] compat/win32/path-utils.h: add #include guards Johannes Schindelin via GitGitGadget
2019-09-26  8:30 ` [PATCH 05/13] msvc: ignore some libraries when linking Johannes Schindelin via GitGitGadget
2019-09-26  8:30 ` [PATCH 06/13] msvc: handle DEVELOPER=1 Johannes Schindelin via GitGitGadget
2019-09-26  8:30 ` [PATCH 07/13] msvc: work around a bug in GetEnvironmentVariable() Johannes Schindelin via GitGitGadget
2019-09-26  8:30 ` [PATCH 08/13] vcxproj: only copy `git-remote-http.exe` once it was built Johannes Schindelin via GitGitGadget
2019-09-26  8:30 ` [PATCH 09/13] vcxproj: include more generated files Johannes Schindelin via GitGitGadget
2019-09-26  8:30 ` [PATCH 10/13] test-tool run-command: learn to run (parts of) the testsuite Johannes Schindelin via GitGitGadget
2019-09-26  8:30 ` [PATCH 11/13] tests: let --immediate and --write-junit-xml play well together Johannes Schindelin via GitGitGadget
2019-09-28 22:22   ` Junio C Hamano
2019-09-30  9:52     ` Johannes Schindelin
2019-09-26  8:30 ` [PATCH 12/13] ci: really use shallow clones on Azure Pipelines Johannes Schindelin via GitGitGadget
2019-09-26  8:30 ` [PATCH 13/13] ci: also build and test with MS Visual Studio " Johannes Schindelin via GitGitGadget
2019-09-30  9:55 ` [PATCH v2 00/13] ci: include a Visual Studio build & test in our Azure Pipeline Johannes Schindelin via GitGitGadget
2019-09-30  9:55   ` [PATCH v2 02/13] msvc: avoid using minus operator on unsigned types Johannes Schindelin via GitGitGadget
2019-10-03 22:44     ` Junio C Hamano
2019-10-04  9:55       ` Johannes Schindelin
2019-10-04 17:09         ` Johannes Sixt
2019-10-04 21:24           ` Johannes Schindelin
2019-10-06  0:02             ` Junio C Hamano
2019-10-06 10:53               ` Johannes Sixt
2019-10-08 12:04                 ` Johannes Schindelin
2019-10-08 21:13                   ` Johannes Sixt
2019-09-30  9:55   ` [PATCH v2 01/13] push: do not pretend to return `int` from `die_push_simple()` Johannes Schindelin via GitGitGadget
2019-10-03 22:37     ` Junio C Hamano
2019-10-04  9:36       ` Johannes Schindelin
2019-09-30  9:55   ` [PATCH v2 03/13] winansi: use FLEX_ARRAY to avoid compiler warning Johannes Schindelin via GitGitGadget
2019-09-30  9:55   ` [PATCH v2 04/13] compat/win32/path-utils.h: add #include guards Johannes Schindelin via GitGitGadget
2019-09-30  9:55   ` [PATCH v2 05/13] msvc: ignore some libraries when linking Johannes Schindelin via GitGitGadget
2019-09-30  9:55   ` [PATCH v2 06/13] msvc: handle DEVELOPER=1 Johannes Schindelin via GitGitGadget
2019-09-30  9:55   ` [PATCH v2 07/13] msvc: work around a bug in GetEnvironmentVariable() Johannes Schindelin via GitGitGadget
2019-09-30  9:55   ` [PATCH v2 08/13] vcxproj: only copy `git-remote-http.exe` once it was built Johannes Schindelin via GitGitGadget
2019-09-30  9:55   ` [PATCH v2 09/13] vcxproj: include more generated files Johannes Schindelin via GitGitGadget
2019-09-30  9:55   ` [PATCH v2 10/13] test-tool run-command: learn to run (parts of) the testsuite Johannes Schindelin via GitGitGadget
2019-09-30  9:55   ` [PATCH v2 11/13] tests: let --immediate and --write-junit-xml play well together Johannes Schindelin via GitGitGadget
2019-09-30  9:55   ` [PATCH v2 12/13] ci: really use shallow clones on Azure Pipelines Johannes Schindelin via GitGitGadget
2019-09-30  9:55   ` [PATCH v2 13/13] ci: also build and test with MS Visual Studio " Johannes Schindelin via GitGitGadget
2019-10-04 15:09   ` [PATCH v3 00/13] ci: include a Visual Studio build & test in our Azure Pipeline Johannes Schindelin via GitGitGadget
2019-10-04 15:09     ` [PATCH v3 01/13] push: do not pretend to return `int` from `die_push_simple()` Johannes Schindelin via GitGitGadget
2019-10-04 15:09     ` [PATCH v3 02/13] msvc: avoid using minus operator on unsigned types Johannes Schindelin via GitGitGadget
2019-10-04 15:09     ` [PATCH v3 03/13] winansi: use FLEX_ARRAY to avoid compiler warning Johannes Schindelin via GitGitGadget
2019-10-07 19:16       ` Alban Gruin
2019-10-04 15:09     ` [PATCH v3 04/13] compat/win32/path-utils.h: add #include guards Johannes Schindelin via GitGitGadget
2019-10-04 15:09     ` [PATCH v3 05/13] msvc: ignore some libraries when linking Johannes Schindelin via GitGitGadget
2019-10-04 15:09     ` [PATCH v3 06/13] msvc: handle DEVELOPER=1 Johannes Schindelin via GitGitGadget
2019-10-04 15:09     ` [PATCH v3 07/13] msvc: work around a bug in GetEnvironmentVariable() Johannes Schindelin via GitGitGadget
2019-10-04 15:09     ` [PATCH v3 08/13] vcxproj: only copy `git-remote-http.exe` once it was built Johannes Schindelin via GitGitGadget
2019-10-04 15:09     ` [PATCH v3 09/13] vcxproj: include more generated files Johannes Schindelin via GitGitGadget
2019-10-04 15:09     ` [PATCH v3 10/13] test-tool run-command: learn to run (parts of) the testsuite Johannes Schindelin via GitGitGadget
2019-10-04 15:09     ` [PATCH v3 11/13] tests: let --immediate and --write-junit-xml play well together Johannes Schindelin via GitGitGadget
2019-10-04 15:09     ` [PATCH v3 12/13] ci: really use shallow clones on Azure Pipelines Johannes Schindelin via GitGitGadget
2019-10-04 15:09     ` [PATCH v3 13/13] ci: also build and test with MS Visual Studio " Johannes Schindelin via GitGitGadget
2019-10-06  0:19     ` Junio C Hamano [this message]
2019-10-06 10:45       ` [PATCH v3 00/13] ci: include a Visual Studio build & test in our Azure Pipeline Johannes Schindelin
2019-10-06 20:38         ` Johannes Schindelin
2019-10-07  1:14           ` Junio C Hamano
2019-10-07 21:51             ` Johannes Schindelin
2019-10-08  2:19               ` Junio C Hamano
2019-10-08 12:46                 ` Johannes Schindelin
2019-10-09 13:57                   ` Philip Oakley
2019-10-10  9:03                     ` Johannes Schindelin
2019-10-10 10:12                       ` Philip Oakley
2019-10-07  0:59         ` Junio C Hamano
2019-10-07 16:08           ` Thomas Gummerer
2019-10-11 22:06             ` Johannes Schindelin

Reply instructions:

You may reply publically to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=xmqqimp26808.fsf@gitster-ct.c.googlers.com \
    --to=gitster@pobox.com \
    --cc=git@vger.kernel.org \
    --cc=gitgitgadget@gmail.com \
    --cc=johannes.schindelin@gmx.de \
    --cc=liu.denton@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

git@vger.kernel.org list mirror (unofficial, one of many)

Archives are clonable:
	git clone --mirror https://public-inbox.org/git
	git clone --mirror http://ou63pmih66umazou.onion/git
	git clone --mirror http://czquwvybam4bgbro.onion/git
	git clone --mirror http://hjrcffqmbrq6wope.onion/git

Example config snippet for mirrors

Newsgroups are available over NNTP:
	nntp://news.public-inbox.org/inbox.comp.version-control.git
	nntp://ou63pmih66umazou.onion/inbox.comp.version-control.git
	nntp://czquwvybam4bgbro.onion/inbox.comp.version-control.git
	nntp://hjrcffqmbrq6wope.onion/inbox.comp.version-control.git
	nntp://news.gmane.org/gmane.comp.version-control.git

 note: .onion URLs require Tor: https://www.torproject.org/

AGPL code for this site: git clone https://public-inbox.org/public-inbox.git