Git development
 help / color / mirror / Atom feed
* Re: RFE: version-controlled merge rules
From: Jonathan Nieder @ 2018-12-27 23:55 UTC (permalink / raw)
  To: H. Peter Anvin; +Cc: git
In-Reply-To: <ad875f1e-54e1-e19f-cd65-95ab503c6de2@zytor.com>

Hi,

H. Peter Anvin wrote:

> [merge "version"]
>         name = Version file merge driver
>         driver = sort -V -r %O %A %B | head -1 > %A.tmp.1 && mv -f %A.tmp.1 %A
[...]
> However, I can't even put this in .gitattributes, because doing so would break
> any user who *doesn't* have the previous rule defined locally. Even worse, if
> this rule needs to change, propagating it to all new users has to be done
> manually... never mind if it needs to vary by branch!
>
> The simplest way to address this would presumably be to let the
> repository/working directory contain a .gitconfig file that can contain rules
> like that.  (Allowing it to be in the repository proper is probably a
> requirement for merges to be handled correctly on bare repositories; I'm not
> sure how .gitattributes is handled for that.)

The main issue I see is that this would make it a little *too* easy to
run arbitrary code on the user's machine.  Build systems often already
lead to that, but users are more familiar with the risks for build
than for version control.

See [1] for some related discussion.

That said, using the include.path feature (see git-config(1)), it's
possible to do something similar:

	[include]
		path = ../.gitconfig

Thanks and hope that helps,
Jonathan

[1] https://public-inbox.org/git/20171002234517.GV19555@aiede.mtv.corp.google.com/

^ permalink raw reply

* Re: [PATCH v2] Simplify handling of setup_git_directory_gently() failure cases.
From: Erin Dahlgren @ 2018-12-27 23:46 UTC (permalink / raw)
  To: Jeff King; +Cc: Junio C Hamano, git, Johannes Schindelin
In-Reply-To: <20181227162417.GA23147@sigill.intra.peff.net>

On Thu, Dec 27, 2018 at 8:24 AM Jeff King <peff@peff.net> wrote:
>
> On Wed, Dec 26, 2018 at 02:22:39PM -0800, Junio C Hamano wrote:
>
> > >> Side note: One of the secondary goals of my patch was to make it
> > >> really obvious that neither the GIT_DIR_HIT_CEILING nor the
> > >> GIT_DIR_HIT_MOUNT_POINT case can get us into the block protected by
> > >> (startup_info->have_repository || getenv(GIT_DIR_ENVIRONMENT)). With
> > >> your suggestion (and it's a fair one) I don't think that's feasible
> > >> without more significant refactoring. Should I just settle with a
> > >> comment that explains this?
> > >
> > > Yeah, I think a comment would probably be sufficient. Though we could
> > > also perhaps make the two cases (i.e., we found a repo or not) more
> > > clear. Something like:
> > >
> > >   if (!*nongit_ok || !*nongit_ok) {
> >
> > WTH is (A || A)?
>
> Heh, I should learn to cut and paste better. This should be:
>
>   if (!nongit_ok || !*nongit_ok)
>
> (which comes from the current code).

Yep, but I think we can benefit from De Morgan's law here, where:

  (!nongit_ok || !*nongit_ok) == (!(nongit_ok && *nongit_ok))

PATCH v3 (just sent) uses that transformation like this:

if (nongit_ok && *nongit_ok) {
  ... startup_info->has_repository = 0;
} else {
  // !nongit_ok || !*nongit_ok
  .. startup_info->has_repository = 1;
}

Because IMHO (nongit_ok && *nongit_ok) is easier to read and reason
about. Added brief comments as well.

Thanks.

- Erin

>
> -Peff

^ permalink raw reply

* Re: [PATCH 0/2] Improve documentation on UTF-16
From: brian m. carlson @ 2018-12-27 23:45 UTC (permalink / raw)
  To: Johannes Sixt; +Cc: git, Lars Schneider, Torsten Bögershausen
In-Reply-To: <435b6870-379c-7183-da99-35aec5cf1137@kdbg.org>

[-- Attachment #1: Type: text/plain, Size: 3455 bytes --]

On Thu, Dec 27, 2018 at 08:55:27PM +0100, Johannes Sixt wrote:
> Am 27.12.18 um 17:43 schrieb brian m. carlson:
> > You've got part of this. For UTF-16LE and UTF-16BE, a U+FEFF is part of
> > the text, as would a second one be if we had two at the beginning of a
> > UTF-16 or UTF-8 sequence. If someone produces UTF-16LE and places a
> > U+FEFF at the beginning of it, when we encode to UTF-8, we emit only one
> > U+FEFF, which has the wrong semantics.
> > 
> > To be correct here and accept a U+FEFF, we'd need to check for a U+FEFF
> > at the beginning of a UTF-16LE or UTF-16BE sequence and ensure we encode
> > an extra U+FEFF at the beginning of the UTF-8 data (one for BOM and one
> > for the text) and then strip it off when we decode. That's kind of ugly,
> > and since iconv doesn't do that itself, we'd have to.
> 
> But why do you add another U+FEFF on the way to UTF-8? There is one in the
> incoming UTF-16 data, and only *that* one must be converted. If there is no
> U+FEFF in the UTF-16 data, the should not be one in UTF-8, either.
> Puzzled...

So for UTF-16, there must be a BOM. For UTF-16LE and UTF-16BE, there
must not be a BOM. So if we do this:

  $ printf '\xfe\xff\x00\x0a' | iconv -f UTF-16BE -t UTF-16 | xxd -g1
  00000000: ff fe ff fe 0a 00                                ......

That U+FEFF we have in the input is part of the text as a ZWNBSP; it is
not a BOM. We end up with two U+FEFF values. The first is the BOM that's
required as part of UTF-16. The second is semantically part of the text
and has the semantics of a zero-width non-breaking space.

In UTF-8, if the sequence starts with U+FEFF, it has the semantics of a
BOM just like in UTF-16 (except that it's optional): it's not part of
the text, and should be stripped off. So when we receive a UTF-16LE or
UTF-16BE sequence and it contains a U+FEFF (which is part of the text),
we need to insert a BOM in front of the sequence that's part of the text
to keep the semantics.

Essentially, we have this situation:

Text (in memory):  U+FEFF U+000A
Semantics of text: ZWNBSP NL
UTF-16BE:          FE FF  00 0A
Semantics:         ZWNBSP NL
UTF-16:            FE FF FE FF  00 0A
Semantics:         BOM   ZWNBSP NL
UTF-8:             EF BB BF EF BB BF 0A
Semantics:         BOM      ZWNBSP   NL

If you don't have a U+FEFF, then things can be simpler:

Text (in memory):  U+0041 U+0042 U+0043
Semantics of text: A      B      C
UTF-16BE:          00 41 00 42 00 43
Semantics:         A     B     C
UTF-16:            FE FF 00 41 00 42 00 43
Semantics:         BOM   A     B     C
UTF-8:             41 42 43
Semantics:         A  B  C
UTF-8 (optional):  EF BB BF 41 42 43
Semantics:         BOM      A  B  C

(I have picked big-endian UTF-16 here, but little-endian is fine, too;
this is just easier for me to type.)

This is all a huge edge case involving correctly serializing code
points. By rejecting U=FEFF in UTF-16BE and UTF-16LE, we don't have to
deal with any of it.

As mentioned, I think patching Git for Windows's iconv is the smallest,
most achievable solution to this, because it means we don't have to
handle any of this edge case ourselves. Windows and WSL users can both
write "UTF-16" and get a BOM and little-endian behavior, while we can
delegate all the rest of the encoding stuff to libiconv.
-- 
brian m. carlson: Houston, Texas, US
OpenPGP: https://keybase.io/bk2204

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 868 bytes --]

^ permalink raw reply

* Re: Missed Commit in 2.20.1
From: Ævar Arnfjörð Bjarmason @ 2018-12-27 23:44 UTC (permalink / raw)
  To: Randall S. Becker
  Cc: 'Stefan Beller', 'Junio C Hamano', git,
	'Joachim Schmitz'
In-Reply-To: <001501d49e38$931aa0a0$b94fe1e0$@nexbridge.com>


On Thu, Dec 27 2018, Randall S. Becker wrote:

> On December 27, 2018 17:40, Ævar Arnfjörð Bjarmason wrote:
>> On Wed, Dec 26 2018, Randall S. Becker wrote:
>>
>> > Please stay tuned for patches. We are very much looking forward to
>> > having the two (or three) different NonStop hardware personalities
>> > supported without mods in the very near future. Our goal, assuming
>> > those patches are acceptable, is to move our build/test/distro into a
>> > Jenkins config that runs with minimal human involvement (a.k.a. me).
>>
>> Portability patches like that are definitely wanted.
>>
>> In case you haven't seen my recent work on getting GitLab CI up & running
>> check out https://public-
>> inbox.org/git/875zwm15k2.fsf@evledraar.gmail.com/
>>
>> It differs from existing CI implementations for git.git in being focused on
>> doing the actual run on remote hosts that can be ssh'd to.
>>
>> So perhaps you'd be interested in some of:
>>
>> a) Contributing a NonStop box to the GCC Compile Farm
>>    (https://cfarm.tetaneutral.net/machines/list/). Then I can add it to
>>    my tests, but also other people porting free software will fix bugs
>>    pro-actively before you spot them.
>
> If I win the lottery, sure. Right now, a contribution like that is a bit beyond my budget. I'm not sure that anything "GCC" will fly with management since GCC does not port to the platform at all at this point in time. Many have tried. Many have failed. We're limited to c89 and c99.
>
>> b) I now have a gitlab-runner I maintain powering this git-ci.git stuff
>>    & presenting it on gitlab.com, if you give me SSH access I can add it
>>    to my own runs...
>
> Sorry, no can do on this one.
>
>> c) ...or you can just run your own gitlab-runner on
>>    https://gitlab.com/git-vcs/git-ci/ (although this amounts to giving
>>    me ssh access, since you'll be running my code)....
>
> This may be more possible. I've been considering putting up a GitLab instance but it's a matter of not having enough time. I have more than enough LXC ubuntu instances still available for something like that.

FWIW it's gitlab-runner, not gitlab, you can run gitlab-runner (and I
do) without installing any of the rest of gitlab. It's basically a
daemon that sits in a loop polling to see if there's new jobs for it,

So the extent of the setup is that I have a Debian box that has:

    vm ~ (master=) $ sudo grep -v token /etc/gitlab-runner/config.toml
    concurrent = 10
    check_interval = 30

    [[runners]]
      name = "gcc-farm"
      url = "https://gitlab.com/"
      executor = "shell"
      [runners.cache]

(Secret token pruned out), that's what sets it up as "gcc-farm" (see
https://gitlab.com/git-vcs/git-ci/blob/master/.gitlab-ci.yml), which
just runs this shellscript:
https://gitlab.com/git-vcs/git-ci/blob/master/ci/gitlab/run-on-gcc-farm.sh

I.e. for a given job name (it extracts the hostname from that) it ssh's
to that machine after scp-ing a given git.git revision to it, compiles &
tests, and all the output is then visible at :
https://gitlab.com/git-vcs/git-ci/-/jobs?scope=finished

I still need to write the rather trivial bit that'll run this as a
cronjob and push as new git.git revs become available, but so far I've
been wanting to get it passing 100% as a baseline, which hasn't happend
due to wanting to handle transitory failures (e.g. ssh to some boxes
timing out) and smoking out the various intermittent failures on some of
the odd platforms/distro versions.

>> d) ... or reuse the CI code I wrote to setup your own runner/pusher
>>    against NonStop, only you'd have access to this....
>
> More likely. Private chat worth it perhaps.

Sure, any time.

>> e) Or do whatever you're planning with Jenkins.
>
> We are currently using Jenkins to build/test git. I was thinking about contributing a Jenkinsfile that would build on a Controller (what happens today for our git port), or setting up a parameterized form for SSH for an Agent that might be better in a farm setting. I am close to the point where human interaction is limited to 'git branch -f production vn.mm.l' and git is tested and built for distribution without further touching. At least once my platform patches are applied it will be.

Makes sense, I thought you were just working on this as a new thing to
do CI testing.

>> If you want to just go with e) that's fine, just saying that you could re-use
>> some existing stuff with a-d) if you wanted.
>
> I am interested. Let's see how my $DAYJOB goes in the next few months. I really do like the idea of setting up a community instance of GitLab to do this and include a CI runner. Hmmm.

As noted above it's just a runner that's needed. I'm using gitlab.com's
instance to present the results, I *could* also setup my own, but no
reason to if they're willing to host it for free.

B.t.w. I'm sure the same can be done with GitHub/Travis or Azure CI
etc., I was just familiar with GitLab's, and wanted something where to
begin with I could have my own overlay of custom CI on top of git.git
without integrating patches upstream yet while I see if that even makes
sense.

^ permalink raw reply

* [PATCH v3] Simplify handling of setup_git_directory_gently() failure cases.
From: Erin Dahlgren @ 2018-12-27 23:36 UTC (permalink / raw)
  To: git; +Cc: Johannes Schindelin, Junio C Hamano, Jeff King, Erin Dahlgren
In-Reply-To: <1544922308-740-1-git-send-email-eedahlgren@gmail.com>

setup_git_directory_gently() expects two types of failures to
discover a git directory (e.g. .git/):

  - GIT_DIR_HIT_CEILING: could not find a git directory in any
	parent directories of the cwd.
  - GIT_DIR_HIT_MOUNT_POINT: could not find a git directory in
	any parent directories up to the mount point of the cwd.

Both cases are handled in a similar way, but there are misleading
and unimportant differences. In both cases, setup_git_directory_gently()
should:

  - Die if we are not in a git repository. Otherwise:
  - Set nongit_ok = 1, indicating that we are not in a git repository
	but this is ok.
  - Call strbuf_release() on any non-static struct strbufs that we
	allocated.

Before this change are two misleading additional behaviors:

  - GIT_DIR_HIT_CEILING: setup_nongit() changes to the cwd for no
	apparent reason. We never had the chance to change directories
	up to this point so chdir(current cwd) is pointless.
  - GIT_DIR_HIT_MOUNT_POINT: strbuf_release() frees the buffer
	of a static struct strbuf (cwd). This is unnecessary because the
	struct is static so its buffer is always reachable. This is also
	misleading because nowhere else in the function is this buffer
	released.

This change eliminates these two misleading additional behaviors and
deletes setup_nogit() because the code is clearer without it. The
result is that we can see clearly that GIT_DIR_HIT_CEILING and
GIT_DIR_HIT_MOUNT_POINT lead to the same behavior (ignoring the
different help messages).

During review, this change was amended to additionally include:

  - Neither GIT_DIR_HIT_CEILING nor GIT_DIR_HIT_MOUNT_POINT may
	return early from setup_git_directory_gently() before the
	GIT_PREFIX environment variable is reset. Change both cases to
	break instead of return. See GIT_PREFIX below for more details.

  - GIT_DIR_NONE: setup_git_directory_gently_1() never returns this
	value, but if it ever did, setup_git_directory_gently() would
	incorrectly record that it had found a repository. Explicitly
	BUG on this case because it is underspecified.

  - GIT_PREFIX: this environment variable must always match the
	value of startup_info->prefix and the prefix returned from
	setup_git_directory_gently(). Make how we handle this slightly
	more repetitive but also more clear.

  - setup_git_env() and repo_set_hash_algo(): Add comments showing
	that only GIT_DIR_EXPLICIT, GIT_DIR_DISCOVERED, and GIT_DIR_BARE
	will cause setup_git_directory_gently() to call these setup
	functions. This was obvious (but partly incorrect) before this
	change when GIT_DIR_HIT_MOUNT_POINT returned early from
	setup_git_directory_gently().
---
Changes in v3:

  - Re-aligned arguments to die() calls to match formatting convention.

  - Neither GIT_DIR_HIT_CEILING nor GIT_DIR_HIT_MOUNT_POINT may
  return early from setup_git_directory_gently() before the
  GIT_PREFIX environment variable is reset. Change both cases to
  break instead of return. See GIT_PREFIX below for more details.

  - GIT_DIR_NONE: setup_git_directory_gently_1() never returns this
  value, but if it ever did, setup_git_directory_gently() would
  incorrectly record that it had found a repository. Explicitly
  BUG on this case because it is underspecified.

  - GIT_PREFIX: this environment variable must always match the
  value of startup_info->prefix and the prefix returned from
  setup_git_directory_gently(). Make how we handle this slightly
  more repetitive but also more clear.

  - setup_git_env() and repo_set_hash_algo(): Add comments showing
  that only GIT_DIR_EXPLICIT, GIT_DIR_DISCOVERED, and GIT_DIR_BARE
  will cause setup_git_directory_gently() to call these setup
  functions. This was obvious (but partly incorrect) before this
  change when GIT_DIR_HIT_MOUNT_POINT returned early from
  setup_git_directory_gently().

 setup.c | 75 ++++++++++++++++++++++++++++++++++++++---------------------------
 1 file changed, 44 insertions(+), 31 deletions(-)

diff --git a/setup.c b/setup.c
index 1be5037..eb8332b 100644
--- a/setup.c
+++ b/setup.c
@@ -831,16 +831,6 @@ static const char *setup_bare_git_dir(struct strbuf *cwd, int offset,
 	return NULL;
 }
 
-static const char *setup_nongit(const char *cwd, int *nongit_ok)
-{
-	if (!nongit_ok)
-		die(_("not a git repository (or any of the parent directories): %s"), DEFAULT_GIT_DIR_ENVIRONMENT);
-	if (chdir(cwd))
-		die_errno(_("cannot come back to cwd"));
-	*nongit_ok = 1;
-	return NULL;
-}
-
 static dev_t get_device_or_die(const char *path, const char *prefix, int prefix_len)
 {
 	struct stat buf;
@@ -1054,7 +1044,7 @@ const char *setup_git_directory_gently(int *nongit_ok)
 {
 	static struct strbuf cwd = STRBUF_INIT;
 	struct strbuf dir = STRBUF_INIT, gitdir = STRBUF_INIT;
-	const char *prefix;
+	const char *prefix = NULL;
 	struct repository_format repo_fmt;
 
 	/*
@@ -1079,9 +1069,6 @@ const char *setup_git_directory_gently(int *nongit_ok)
 	strbuf_addbuf(&dir, &cwd);
 
 	switch (setup_git_directory_gently_1(&dir, &gitdir, 1)) {
-	case GIT_DIR_NONE:
-		prefix = NULL;
-		break;
 	case GIT_DIR_EXPLICIT:
 		prefix = setup_explicit_git_dir(gitdir.buf, &cwd, &repo_fmt, nongit_ok);
 		break;
@@ -1097,29 +1084,52 @@ const char *setup_git_directory_gently(int *nongit_ok)
 		prefix = setup_bare_git_dir(&cwd, dir.len, &repo_fmt, nongit_ok);
 		break;
 	case GIT_DIR_HIT_CEILING:
-		prefix = setup_nongit(cwd.buf, nongit_ok);
+		if (!nongit_ok)
+			die(_("not a git repository (or any of the parent directories): %s"),
+			    DEFAULT_GIT_DIR_ENVIRONMENT);
+		*nongit_ok = 1;
 		break;
 	case GIT_DIR_HIT_MOUNT_POINT:
-		if (nongit_ok) {
-			*nongit_ok = 1;
-			strbuf_release(&cwd);
-			strbuf_release(&dir);
-			return NULL;
-		}
-		die(_("not a git repository (or any parent up to mount point %s)\n"
-		      "Stopping at filesystem boundary (GIT_DISCOVERY_ACROSS_FILESYSTEM not set)."),
-		    dir.buf);
+		if (!nongit_ok)
+			die(_("not a git repository (or any parent up to mount point %s)\n"
+			      "Stopping at filesystem boundary (GIT_DISCOVERY_ACROSS_FILESYSTEM not set)."),
+			    dir.buf);
+		*nongit_ok = 1;
+		break;
+	case GIT_DIR_NONE:
+		/*
+		 * As a safeguard against setup_git_directory_gently_1 returning
+		 * this value, fallthrough to BUG. Otherwise it is possible to
+		 * set startup_info->have_repository to 1 when we did nothing to
+		 * find a repository.
+		 */
 	default:
 		BUG("unhandled setup_git_directory_1() result");
 	}
 
-	if (prefix)
-		setenv(GIT_PREFIX_ENVIRONMENT, prefix, 1);
-	else
+	/*
+	 * At this point, nongit_ok is stable. If it is non-NULL and points
+	 * to a non-zero value, then this means that we haven't found a
+	 * repository and that the caller expects startup_info to reflect
+	 * this.
+	 *
+	 * Regardless of the state of nongit_ok, startup_info->prefix and
+	 * the GIT_PREFIX environment variable must always match. For details
+	 * see Documentation/config/alias.txt.
+	 */
+	if (nongit_ok && *nongit_ok) {
+		startup_info->have_repository = 0;
+		startup_info->prefix = NULL;
 		setenv(GIT_PREFIX_ENVIRONMENT, "", 1);
-
-	startup_info->have_repository = !nongit_ok || !*nongit_ok;
-	startup_info->prefix = prefix;
+	} else {
+		// !nongit_ok || !*nongit_ok
+		startup_info->have_repository = 1;
+		startup_info->prefix = prefix;
+		if (prefix)
+			setenv(GIT_PREFIX_ENVIRONMENT, prefix, 1);
+		else
+			setenv(GIT_PREFIX_ENVIRONMENT, "", 1);
+	}
 
 	/*
 	 * Not all paths through the setup code will call 'set_git_dir()' (which
@@ -1132,7 +1142,10 @@ const char *setup_git_directory_gently(int *nongit_ok)
 	 * the user has set GIT_DIR.  It may be beneficial to disallow bogus
 	 * GIT_DIR values at some point in the future.
 	 */
-	if (startup_info->have_repository || getenv(GIT_DIR_ENVIRONMENT)) {
+	if (// GIT_DIR_EXPLICIT, GIT_DIR_DISCOVERED, GIT_DIR_BARE
+	    startup_info->have_repository ||
+	    // GIT_DIR_EXPLICIT
+	    getenv(GIT_DIR_ENVIRONMENT)) {
 		if (!the_repository->gitdir) {
 			const char *gitdir = getenv(GIT_DIR_ENVIRONMENT);
 			if (!gitdir)
-- 
2.7.4


^ permalink raw reply related

* RE: Missed Commit in 2.20.1
From: Randall S. Becker @ 2018-12-27 23:04 UTC (permalink / raw)
  To: 'Ævar Arnfjörð Bjarmason'
  Cc: 'Stefan Beller', 'Junio C Hamano', git,
	'Joachim Schmitz'
In-Reply-To: <87pntmegq1.fsf@evledraar.gmail.com>

On December 27, 2018 17:40, Ævar Arnfjörð Bjarmason wrote:
> On Wed, Dec 26 2018, Randall S. Becker wrote:
> 
> > Please stay tuned for patches. We are very much looking forward to
> > having the two (or three) different NonStop hardware personalities
> > supported without mods in the very near future. Our goal, assuming
> > those patches are acceptable, is to move our build/test/distro into a
> > Jenkins config that runs with minimal human involvement (a.k.a. me).
> 
> Portability patches like that are definitely wanted.
> 
> In case you haven't seen my recent work on getting GitLab CI up & running
> check out https://public-
> inbox.org/git/875zwm15k2.fsf@evledraar.gmail.com/
> 
> It differs from existing CI implementations for git.git in being focused on
> doing the actual run on remote hosts that can be ssh'd to.
> 
> So perhaps you'd be interested in some of:
> 
> a) Contributing a NonStop box to the GCC Compile Farm
>    (https://cfarm.tetaneutral.net/machines/list/). Then I can add it to
>    my tests, but also other people porting free software will fix bugs
>    pro-actively before you spot them.

If I win the lottery, sure. Right now, a contribution like that is a bit beyond my budget. I'm not sure that anything "GCC" will fly with management since GCC does not port to the platform at all at this point in time. Many have tried. Many have failed. We're limited to c89 and c99.

> b) I now have a gitlab-runner I maintain powering this git-ci.git stuff
>    & presenting it on gitlab.com, if you give me SSH access I can add it
>    to my own runs...

Sorry, no can do on this one.

> c) ...or you can just run your own gitlab-runner on
>    https://gitlab.com/git-vcs/git-ci/ (although this amounts to giving
>    me ssh access, since you'll be running my code)....

This may be more possible. I've been considering putting up a GitLab instance but it's a matter of not having enough time. I have more than enough LXC ubuntu instances still available for something like that.

> d) ... or reuse the CI code I wrote to setup your own runner/pusher
>    against NonStop, only you'd have access to this....

More likely. Private chat worth it perhaps.

> e) Or do whatever you're planning with Jenkins.

We are currently using Jenkins to build/test git. I was thinking about contributing a Jenkinsfile that would build on a Controller (what happens today for our git port), or setting up a parameterized form for SSH for an Agent that might be better in a farm setting. I am close to the point where human interaction is limited to 'git branch -f production vn.mm.l' and git is tested and built for distribution without further touching. At least once my platform patches are applied it will be.

> If you want to just go with e) that's fine, just saying that you could re-use
> some existing stuff with a-d) if you wanted.

I am interested. Let's see how my $DAYJOB goes in the next few months. I really do like the idea of setting up a community instance of GitLab to do this and include a CI runner. Hmmm.

Cheers,
Randall



^ permalink raw reply

* Re: Missed Commit in 2.20.1
From: Ævar Arnfjörð Bjarmason @ 2018-12-27 22:40 UTC (permalink / raw)
  To: Randall S. Becker
  Cc: 'Stefan Beller', 'Junio C Hamano', git,
	'Joachim Schmitz'
In-Reply-To: <002401d49d07$325c7900$97156b00$@nexbridge.com>


On Wed, Dec 26 2018, Randall S. Becker wrote:

> Please stay tuned for patches. We are very much looking forward to having
> the two (or three) different NonStop hardware personalities supported
> without mods in the very near future. Our goal, assuming those patches are
> acceptable, is to move our build/test/distro into a Jenkins config that runs
> with minimal human involvement (a.k.a. me).

Portability patches like that are definitely wanted.

In case you haven't seen my recent work on getting GitLab CI up &
running check out
https://public-inbox.org/git/875zwm15k2.fsf@evledraar.gmail.com/

It differs from existing CI implementations for git.git in being focused
on doing the actual run on remote hosts that can be ssh'd to.

So perhaps you'd be interested in some of:

a) Contributing a NonStop box to the GCC Compile Farm
   (https://cfarm.tetaneutral.net/machines/list/). Then I can add it to
   my tests, but also other people porting free software will fix bugs
   pro-actively before you spot them.

b) I now have a gitlab-runner I maintain powering this git-ci.git stuff
   & presenting it on gitlab.com, if you give me SSH access I can add it
   to my own runs...

c) ...or you can just run your own gitlab-runner on
   https://gitlab.com/git-vcs/git-ci/ (although this amounts to giving
   me ssh access, since you'll be running my code)....

d) ... or reuse the CI code I wrote to setup your own runner/pusher
   against NonStop, only you'd have access to this....

e) Or do whatever you're planning with Jenkins.

If you want to just go with e) that's fine, just saying that you could
re-use some existing stuff with a-d) if you wanted.

^ permalink raw reply

* [PATCH v3 2/4] config.mak.uname: support for modern HPE NonStop config.
From: randall.s.becker @ 2018-12-27 22:38 UTC (permalink / raw)
  To: git; +Cc: Randall S. Becker

From: "Randall S. Becker" <rsbecker@nexbridge.com>

A number of configuration options are not automatically detected by
configure mechanisms, including the location of Perl and Python.

There was a problem at a specific set of operating system versions
that caused getopt to have compile errors. Account for this by
providing emulation defines for those versions.

Signed-off-by: Randall S. Becker <rsbecker@nexbridge.com>
---
 config.mak.uname | 34 +++++++++++++++++++++++++++++-----
 1 file changed, 29 insertions(+), 5 deletions(-)

diff --git a/config.mak.uname b/config.mak.uname
index 3ee7da0e23..aa4432ac2f 100644
--- a/config.mak.uname
+++ b/config.mak.uname
@@ -441,26 +441,45 @@ ifeq ($(uname_S),NONSTOP_KERNEL)
 	# INLINE='' would just replace one set of warnings with another and
 	# still not compile in c89 mode, due to non-const array initializations.
 	CC = cc -c99
+	# Build down-rev compatible objects that don't use our new getopt_long.
+	ifeq ($(uname_R).$(uname_V),J06.21)
+		CC += -WRVU=J06.20
+	endif
+	ifeq ($(uname_R).$(uname_V),L17.02)
+		CC += -WRVU=L16.05
+	endif
+
 	# Disable all optimization, seems to result in bad code, with -O or -O2
 	# or even -O1 (default), /usr/local/libexec/git-core/git-pack-objects
 	# abends on "git push". Needs more investigation.
-	CFLAGS = -g -O0
+	CFLAGS = -g -O0 -Winline
 	# We'd want it to be here.
 	prefix = /usr/local
- 	# Our's are in ${prefix}/bin (perl might also be in /usr/bin/perl).
+	# perl and python must be in /usr/bin on NonStop - supplied by HPE
+	# with operating system in that managed directory.
-	PERL_PATH = ${prefix}/bin/perl
-	PYTHON_PATH = ${prefix}/bin/python
-
+	PERL_PATH = /usr/bin/perl
+	PYTHON_PATH = /usr/bin/python
+	# The current /usr/coreutils/rm at lowest support level does not work
+	# with the git test structure. Long paths cause nftw as in
+	# 'trash directory...' cause rm to terminate prematurely without fully
+	# removing the directory at OS releases J06.21 and L17.02.
+	# Default to the older rm until those two releases are deprecated.
+	RM = /bin/rm -f
 	# As detected by './configure'.
 	# Missdetected, hence commented out, see below.
 	#NO_CURL = YesPlease
 	# Added manually, see above.
+	NEEDS_SSL_WITH_CURL = YesPlease
+	NEEDS_CRYPTO_WITH_SSL = YesPlease
+	HAVE_DEV_TTY = YesPlease
 	HAVE_LIBCHARSET_H = YesPlease
 	HAVE_STRINGS_H = YesPlease
 	NEEDS_LIBICONV = YesPlease
 	NEEDS_LIBINTL_BEFORE_LIBICONV = YesPlease
 	NO_SYS_SELECT_H = UnfortunatelyYes
 	NO_D_TYPE_IN_DIRENT = YesPlease
+	NO_GETTEXT = YesPlease
 	NO_HSTRERROR = YesPlease
 	NO_STRCASESTR = YesPlease
 	NO_MEMMEM = YesPlease
@@ -470,8 +489,13 @@ ifeq ($(uname_S),NONSTOP_KERNEL)
 	NO_MKDTEMP = YesPlease
 	# Currently libiconv-1.9.1.
 	OLD_ICONV = UnfortunatelyYes
-	NO_REGEX = YesPlease
+	NO_REGEX=NeedsStartEnd
 	NO_PTHREADS = UnfortunatelyYes

 	# Not detected (nor checked for) by './configure'.
 	# We don't have SA_RESTART on NonStop, unfortunalety.
-- 
2.17.0.10.gb132f7033


^ permalink raw reply related

* Re: [PATCH v12 04/26] ident: add the ability to provide a "fallback identity"
From: Johannes Schindelin @ 2018-12-27 21:24 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Paul-Sebastian Ungureanu, git, t.gummerer
In-Reply-To: <xmqqpntoq8zs.fsf@gitster-ct.c.googlers.com>

Hi Junio,

On Wed, 26 Dec 2018, Junio C Hamano wrote:

> Paul-Sebastian Ungureanu <ungureanupaulsebastian@gmail.com> writes:
> 
> > +static void set_env_if(const char *key, const char *value, int *given, int bit)
> > +{
> > +	if ((*given & bit) || getenv(key))
> > +		return; /* nothing to do */
> > +	setenv(key, value, 0);
> > +	*given |= bit;
> > +}
> 
> We call setenv(3) with overwrite=0 but we protect the call with a
> check for existing value with getenv(3), which feels a bit like an
> anti-pattern.  Wouldn't the following be simpler to follow, I wonder?
> 
> 	if (!(*given & bit)) {
> 		setenv(key, value, 1);
> 		*given |= bit;
> 	}
> 
> The only case these two may behave differently is when '*given' does
> not have the 'bit' set but the environment 'key' already exists.

Indeed, this is the case where your version would actually do the wrong
thing. Imagine that GIT_AUTHOR_NAME is set already. Your code would
*override* it. But that is not what we want to do here. We want to *fall
back* if there is no already-configured value.

And of course we won't set the `given` bit if we don't fall back here;
that should be done somewhere else, where that environment variable (that
we *refuse* to overwrite) is *actually* used.

Ciao,
Dscho

> The proposed patch will leave 'bit' in '*given' unset, so when a
> later code says "let's see if author_ident is explicitly given, and
> complain otherwise", such a check will trigger and cause complaint.
> 
> On the other hand, the simplified version does not allow the
> "explicitly-given" bits to be left unset, so it won't cause
> complaint.
> 
> Isn't it a BUG() if *given lacks 'bit' when the corresponding
> environment variable 'key' is missing?  IOW, I would understand
> an implementation that is more elaborate than the simplified one I
> just gave above were something like
> 
> 	if (!(*given & bit)) {
> 		if (getenv(key))
> 			BUG("why does %s exist and no %x bit set???", key, bit);
> 		setenv(key, value, 0);
> 		*given |= bit;
> 	}
> 
> but I do not quite understand the reasoning behind the "check either
> the bit, or the environment variable" in the proposed patch.
> 
> > +void prepare_fallback_ident(const char *name, const char *email)
> > +{
> > +	set_env_if("GIT_AUTHOR_NAME", name,
> > +		   &author_ident_explicitly_given, IDENT_NAME_GIVEN);
> > +	set_env_if("GIT_AUTHOR_EMAIL", email,
> > +		   &author_ident_explicitly_given, IDENT_MAIL_GIVEN);
> > +	set_env_if("GIT_COMMITTER_NAME", name,
> > +		   &committer_ident_explicitly_given, IDENT_NAME_GIVEN);
> > +	set_env_if("GIT_COMMITTER_EMAIL", email,
> > +		   &committer_ident_explicitly_given, IDENT_MAIL_GIVEN);
> > +}
> 
> Introducing this function alone without a caller and without
> function doc is a bit unfriendly to future callers, who must be
> careful when to call it, I think.  For example, they must know that
> it will be a disaster if they call this before they call
> git_ident_config(), right?
> 
> > +
> >  static int buf_cmp(const char *a_begin, const char *a_end,
> >  		   const char *b_begin, const char *b_end)
> >  {
> 
> 
> 

^ permalink raw reply

* Re: [PATCH 4/6] config: use OPT_FILENAME()
From: Eric Sunshine @ 2018-12-27 20:40 UTC (permalink / raw)
  To: Nguyễn Thái Ngọc Duy; +Cc: Git List
In-Reply-To: <20181227155611.10585-5-pclouds@gmail.com>

On Thu, Dec 27, 2018 at 10:56 AM Nguyễn Thái Ngọc Duy <pclouds@gmail.com> wrote:
> Do not handle prefix directly. It's simpler to use OPT_FILENAME()
> instead. The othe reason for doing this is because this code (where

s/othe/other/

> the deleted code is) will be factored out and called when "prefix" is
> not available.
>
> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>

^ permalink raw reply

* Re: [PATCH 2/6] worktree.c: add get_worktree_config()
From: Eric Sunshine @ 2018-12-27 20:36 UTC (permalink / raw)
  To: Nguyễn Thái Ngọc Duy; +Cc: Git List
In-Reply-To: <20181227155611.10585-3-pclouds@gmail.com>

On Thu, Dec 27, 2018 at 10:56 AM Nguyễn Thái Ngọc Duy <pclouds@gmail.com> wrote:
> "git config --worktree" can write to the write file whether

s/write file/right file/

> extensions.worktreeConfig is enabled or not. In order to do the same
> using config API, we need to determine the right file to write to. Add
> this function for that purpose. This is the basis for the coming
> repo_config_set_worktree()
>
> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>

^ permalink raw reply

* Re: [PATCH 74/75] diff --no-index: use parse_options() instead of diff_opt_parse()
From: Eric Sunshine @ 2018-12-27 20:34 UTC (permalink / raw)
  To: Nguyễn Thái Ngọc Duy; +Cc: Git List
In-Reply-To: <20181227162536.15895-15-pclouds@gmail.com>

On Thu, Dec 27, 2018 at 11:26 AM Nguyễn Thái Ngọc Duy <pclouds@gmail.com> wrote:
> While at there, move exit() back to the caller. It's easier to see the
> flow that way then burying it in diff-no-index.c

s/then/than/

> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>

^ permalink raw reply

* RFE: merge driver without overwrite
From: H. Peter Anvin @ 2018-12-27 20:21 UTC (permalink / raw)
  To: git

The current definition for merge drivers require the output file %A to be
overwritten. When using a pipeline of Unix commands, this often results in %A
being truncated too early, requiring the user to add a temporary file managed
explicitly.

It would be far preferable if git could manage this; perhaps an %N marker for
a temporary filename, which, if given, is treated as the output; notionally:

	if [ $? -eq 0 -a -f %N ]; then mv -f %N %A; else rm -f %N; fi

	-hpa

^ permalink raw reply

* RFE: version-controlled merge rules
From: H. Peter Anvin @ 2018-12-27 20:16 UTC (permalink / raw)
  To: git

Right now, merge rules can get selected in .gitattributes, which is
version-controlled. However, there does not appear to be any way to *define*
custom merge rules which is version controlled.

There are a lot of different files which can benefit from custom merge rules,
especially ones that are in some ways cumulative or version/tree-dependent.
For example, I use this rule to merge version files:

[merge "version"]
        name = Version file merge driver
        driver = sort -V -r %O %A %B | head -1 > %A.tmp.1 && mv -f %A.tmp.1 %A

(Incidentally: the need for an explicit temp file here is frustrating. It
would be better if git could manage the temporary file. Overwriting %A
directly truncates the file too early.  See other email.)

However, I can't even put this in .gitattributes, because doing so would break
any user who *doesn't* have the previous rule defined locally. Even worse, if
this rule needs to change, propagating it to all new users has to be done
manually... never mind if it needs to vary by branch!

The simplest way to address this would presumably be to let the
repository/working directory contain a .gitconfig file that can contain rules
like that.  (Allowing it to be in the repository proper is probably a
requirement for merges to be handled correctly on bare repositories; I'm not
sure how .gitattributes is handled for that.)

	-hpa

^ permalink raw reply

* Re: [PATCH v2 2/4] config.mak.uname: support for modern HPE NonStop config.
From: Eric Sunshine @ 2018-12-27 20:03 UTC (permalink / raw)
  To: randall.s.becker; +Cc: Git List, Randall S. Becker
In-Reply-To: <20181227183513.2860-1-randall.s.becker@rogers.com>

On Thu, Dec 27, 2018 at 1:35 PM <randall.s.becker@rogers.com> wrote:
> A number of configuration options are not automatically detected by
> configure mechanisms, including the location of Perl and Python.
>
> There was a problem at a specific set of operating system versions
> that caused getopt to have compile errors. Accounted for this by

s/Accounted/Account/

> providing emulation defines for those versions.
>
> Signed-off-by: Randall S. Becker <rsbecker@nexbridge.com>
> ---
> diff --git a/config.mak.uname b/config.mak.uname
> @@ -441,26 +441,45 @@ ifeq ($(uname_S),NONSTOP_KERNEL)
>         # Our's are in ${prefix}/bin (perl might also be in /usr/bin/perl).
> -       PERL_PATH = ${prefix}/bin/perl
> -       PYTHON_PATH = ${prefix}/bin/python
> +       PERL_PATH = /usr/bin/perl
> +       PYTHON_PATH = /usr/bin/python

Referring to your answer[1] to my question about ${prefix}, should the
comment above these lines, which talks about ${prefix}, be removed by
this patch?

[1]: https://public-inbox.org/git/000601d49e0b$e11d7520$a3585f60$@rogers.com/

> +       # The current /usr/coreutils/rm at lowest support level does not work
> +       # with the git test structure. Long paths cause nftw as in
> +       # 'trash directory...' break at OS releases J06.21 and L17.02.

"break" in what fashion? What is the actual incorrect behavior?

I'm not asking for my own edification particularly, but rather to help
the person who comes after you supporting this platform. That person
will need to understand the exact nature of the problem in order to
determine if this work-around is still needed or if it can be retired
or somehow conditionalized. Stated another way, the reason behind this
change still seems like a black-box, mysterious and under-explained.
There is nothing concrete here upon which to grasp to gain an
understanding of the problem.

> +       # Default to the older rm until those two releases are deprecated.
> +       RM = /bin/rm -f

^ permalink raw reply

* Re: [PATCH 0/2] Improve documentation on UTF-16
From: Johannes Sixt @ 2018-12-27 19:55 UTC (permalink / raw)
  To: brian m. carlson; +Cc: git, Lars Schneider, Torsten Bögershausen
In-Reply-To: <20181227164353.GC423984@genre.crustytoothpaste.net>

Am 27.12.18 um 17:43 schrieb brian m. carlson:
> On Thu, Dec 27, 2018 at 11:06:17AM +0100, Johannes Sixt wrote:
>> It worries me that theoretical correctness is regarded higher than existing
>> practice. I do not care a lot what some RFC tells what programs should do if
>> the majority of the software does something different and that behavior has
>> been proven useful in practice.
> 
> The majority of OSes produce the behavior I document here, and they are
> the majority of systems on the Internet. Windows is the outlier here,
> although a significant one. It is a common user of UTF-16 and its
> variants, but so are Java and JavaScript, and they're present on a lot
> of devices. Swallowing the U+FEFF would break compatibility with those
> systems.
> 
> The issue that Windows users are seeing is that libiconv always produces
> big-endian data for UTF-16, and they always want little-endian. glibc
> produces native-endian data, which is what Windows users want. Git for
> Windows could patch libiconv to do that (and that is the simple,
> five-minute solution to this problem), but we'd still want to warn
> people that they're relying on unspecified behavior, hence this series.
> 
> I would even be willing to patch Git for Windows's libiconv if somebody
> could point me to the repo (although I obviously cannot test it, not
> being a Windows user). I feel strongly, though, that fixing this is
> outside of the scope of Git proper, and it's not a thing we should be
> handling here.

Please appologize that I leave the majority of what you said uncommented 
as I am not deep in the matter and don't have a firm understanding of 
all the issues. I'll just trust what you said is sound.

Just one thing: Please do the count by *users* (or existing files or 
number of charactes exchanged or something similar); do not just count 
OSs; I mean, Windows is *not* the outlier if it handles 90% of the 
UTF-16 data in the world. (I'm just making up numbers here, but I think 
you get the point.)

>> My understanding is that there is no such thing as a "byte order marker". It
>> just so happens that when the first character in some UTF-16 text file
>> begins with a ZWNBSP, then it is possible to derive the endianness of the
>> file automatically. Other then that, that very first code point U+FEFF *is
>> part of the data* and must not be removed when the data is reencoded. If Git
>> does something different, it is bogus, IMO.
> 
> You've got part of this. For UTF-16LE and UTF-16BE, a U+FEFF is part of
> the text, as would a second one be if we had two at the beginning of a
> UTF-16 or UTF-8 sequence. If someone produces UTF-16LE and places a
> U+FEFF at the beginning of it, when we encode to UTF-8, we emit only one
> U+FEFF, which has the wrong semantics.
> 
> To be correct here and accept a U+FEFF, we'd need to check for a U+FEFF
> at the beginning of a UTF-16LE or UTF-16BE sequence and ensure we encode
> an extra U+FEFF at the beginning of the UTF-8 data (one for BOM and one
> for the text) and then strip it off when we decode. That's kind of ugly,
> and since iconv doesn't do that itself, we'd have to.

But why do you add another U+FEFF on the way to UTF-8? There is one in 
the incoming UTF-16 data, and only *that* one must be converted. If 
there is no U+FEFF in the UTF-16 data, the should not be one in UTF-8, 
either. Puzzled...

-- Hannes

^ permalink raw reply

* Re: [PATCH] config.mak.dev: add -Wformat
From: Jonathan Nieder @ 2018-12-27 18:59 UTC (permalink / raw)
  To: Thomas Gummerer; +Cc: Jeff King, git, Josh Steadmon, Masaya Suzuki
In-Reply-To: <20181012191531.GA22611@hank.intra.tgummerer.com>

+cc: Masaya Suzuki
In October, Thomas Gummerer wrote:
> On 10/12, Jonathan Nieder wrote:
>> Jeff King wrote:
>>> On Fri, Oct 12, 2018 at 07:40:37PM +0100, Thomas Gummerer wrote:

>>>> 801fa63a90 ("config.mak.dev: add -Wformat-security", 2018-09-08) added
>>>> the -Wformat-security to the flags set in config.mak.dev.  In the gcc
>>>> man page this is documented as:
>>>>
>>>>          If -Wformat is specified, also warn about uses of format
>>>>          functions that represent possible security problems.  [...]
>>>>
>>>> That commit did however not add the -Wformat flag, and -Wformat is not
>>>> specified anywhere else by default, so the added -Wformat-security had
>>>> no effect.  Newer versions of gcc (gcc 8.2.1 in this particular case)
>>>> warn about this and thus compilation fails with this option set.
>> [...]
>>> -Wformat is part of -Wall, which we already turn on by default (even for
>>> non-developer builds).
[...]
>> Thomas, do you use autoconf to generate config.mak.autogen?  I'm
>> wondering if that produces a CFLAGS that doesn't include -Wall.
>
> No, this was all my mistake :)

As discussed in [1], autoconf appears to not put -Wall in CFLAGS:

 $ make configure
     GEN configure
 $ ./configure
[...]
 config.status: creating config.mak.autogen
 config.status: executing config.mak.autogen commands
 $ grep CFLAGS config.mak.autogen
 CFLAGS = -g -O2
 PTHREAD_CFLAGS=-pthread

So this trap for the unwary is still around.

Can we revive this patch?  Does it just need a clearer commit message,
or were there other objections?

>>> I'm not opposed to making config.mak.dev a bit more redundant to handle
>>> this case, but we'd probably want to include all of -Wall, since it
>>> contains many other warnings we'd want to make sure are enabled.
>>
>> Do you mean putting -Wall instead of -Wformat?
>>
>> Should we add -Wextra too?  From a quick test, it seems to build okay.
>
> We do have that with setting DEVELOPER=extra-all.

Even better.  What do you think of making DEVELOPER=YesPlease imply
that?

Thanks,
Jonathan

[1] https://public-inbox.org/git/CAJB1erVmZQd_kLU1fqL7cURrEUz2EJ4Br0kgVQt7T-mk3s95dQ@mail.gmail.com/

^ permalink raw reply

* Re: [PATCH] Specify -Wformat along with -Wformat-security
From: Masaya Suzuki @ 2018-12-27 18:46 UTC (permalink / raw)
  To: Duy Nguyen; +Cc: Git Mailing List
In-Reply-To: <CACsJy8CM=H1njP3ZzuReS0u_YRPTS6pGhFWWMBHoaKVNBYiXXA@mail.gmail.com>

On Thu, Dec 27, 2018 at 10:36 AM Duy Nguyen <pclouds@gmail.com> wrote:
>
> On Thu, Dec 27, 2018 at 7:18 PM Masaya Suzuki <masayasuzuki@google.com> wrote:
> >
> > Without -Wformat, -Wformat-security won't work.
> >
> > > cc1: error: -Wformat-security ignored without -Wformat [-Werror=format-security]
>
> Compiler name and version?

I'm not familar with the tools, so please be patient.

According to Makefile, `CC = cc`. With `cc --version`, it says:

cc (Debian 7.3.0-5) 7.3.0
Copyright (C) 2017 Free Software Foundation, Inc.
This is free software; see the source for copying conditions.  There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

Jonathan said there was the same patch. Looks like
https://public-inbox.org/git/20181012184037.15076-1-t.gummerer@gmail.com/.
I'll check my config again.

^ permalink raw reply

* Re: [PATCH] Specify -Wformat along with -Wformat-security
From: Duy Nguyen @ 2018-12-27 18:35 UTC (permalink / raw)
  To: Masaya Suzuki; +Cc: Git Mailing List
In-Reply-To: <20181227065855.68632-1-masayasuzuki@google.com>

On Thu, Dec 27, 2018 at 7:18 PM Masaya Suzuki <masayasuzuki@google.com> wrote:
>
> Without -Wformat, -Wformat-security won't work.
>
> > cc1: error: -Wformat-security ignored without -Wformat [-Werror=format-security]

Compiler name and version?

>
> Signed-off-by: Masaya Suzuki <masayasuzuki@google.com>
> ---
>  config.mak.dev | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/config.mak.dev b/config.mak.dev
> index bbeeff44f..aae9db67d 100644
> --- a/config.mak.dev
> +++ b/config.mak.dev
> @@ -7,6 +7,7 @@ CFLAGS += -pedantic
>  CFLAGS += -DUSE_PARENS_AROUND_GETTEXT_N=0
>  endif
>  CFLAGS += -Wdeclaration-after-statement
> +CFLAGS += -Wformat
>  CFLAGS += -Wformat-security
>  CFLAGS += -Wno-format-zero-length
>  CFLAGS += -Wold-style-definition
> --
> 2.20.1.415.g653613c723-goog
>


-- 
Duy

^ permalink raw reply

* [PATCH v2 2/4] config.mak.uname: support for modern HPE NonStop config.
From: randall.s.becker @ 2018-12-27 18:35 UTC (permalink / raw)
  To: git; +Cc: Randall S. Becker

From: "Randall S. Becker" <rsbecker@nexbridge.com>

A number of configuration options are not automatically detected by
configure mechanisms, including the location of Perl and Python.

There was a problem at a specific set of operating system versions
that caused getopt to have compile errors. Accounted for this by
providing emulation defines for those versions.

Signed-off-by: Randall S. Becker <rsbecker@nexbridge.com>
---
 config.mak.uname | 34 +++++++++++++++++++++++++++++-----
 1 file changed, 29 insertions(+), 5 deletions(-)

diff --git a/config.mak.uname b/config.mak.uname
index 3ee7da0e23..aa4432ac2f 100644
--- a/config.mak.uname
+++ b/config.mak.uname
@@ -441,26 +441,45 @@ ifeq ($(uname_S),NONSTOP_KERNEL)
 	# INLINE='' would just replace one set of warnings with another and
 	# still not compile in c89 mode, due to non-const array initializations.
 	CC = cc -c99
+	# Build down-rev compatible objects that don't use our new getopt_long.
+	ifeq ($(uname_R).$(uname_V),J06.21)
+		CC += -WRVU=J06.20
+	endif
+	ifeq ($(uname_R).$(uname_V),L17.02)
+		CC += -WRVU=L16.05
+	endif
+
 	# Disable all optimization, seems to result in bad code, with -O or -O2
 	# or even -O1 (default), /usr/local/libexec/git-core/git-pack-objects
 	# abends on "git push". Needs more investigation.
-	CFLAGS = -g -O0
+	CFLAGS = -g -O0 -Winline
 	# We'd want it to be here.
 	prefix = /usr/local
 	# Our's are in ${prefix}/bin (perl might also be in /usr/bin/perl).
-	PERL_PATH = ${prefix}/bin/perl
-	PYTHON_PATH = ${prefix}/bin/python
-
+	PERL_PATH = /usr/bin/perl
+	PYTHON_PATH = /usr/bin/python
+	# The current /usr/coreutils/rm at lowest support level does not work
+	# with the git test structure. Long paths cause nftw as in
+	# 'trash directory...' break at OS releases J06.21 and L17.02.
+	# Default to the older rm until those two releases are deprecated.
+	RM = /bin/rm -f
 	# As detected by './configure'.
 	# Missdetected, hence commented out, see below.
 	#NO_CURL = YesPlease
 	# Added manually, see above.
+	NEEDS_SSL_WITH_CURL = YesPlease
+	NEEDS_CRYPTO_WITH_SSL = YesPlease
+	HAVE_DEV_TTY = YesPlease
 	HAVE_LIBCHARSET_H = YesPlease
 	HAVE_STRINGS_H = YesPlease
 	NEEDS_LIBICONV = YesPlease
 	NEEDS_LIBINTL_BEFORE_LIBICONV = YesPlease
 	NO_SYS_SELECT_H = UnfortunatelyYes
 	NO_D_TYPE_IN_DIRENT = YesPlease
+	NO_GETTEXT = YesPlease
 	NO_HSTRERROR = YesPlease
 	NO_STRCASESTR = YesPlease
 	NO_MEMMEM = YesPlease
@@ -470,8 +489,13 @@ ifeq ($(uname_S),NONSTOP_KERNEL)
 	NO_MKDTEMP = YesPlease
 	# Currently libiconv-1.9.1.
 	OLD_ICONV = UnfortunatelyYes
-	NO_REGEX = YesPlease
+	NO_REGEX=NeedsStartEnd
 	NO_PTHREADS = UnfortunatelyYes

 	# Not detected (nor checked for) by './configure'.
 	# We don't have SA_RESTART on NonStop, unfortunalety.
-- 
2.17.0.10.gb132f7033


^ permalink raw reply related

* [PATCHv2] imap-send: Fix compilation without deprecated OpenSSL APIs
From: Rosen Penev @ 2018-12-27 18:00 UTC (permalink / raw)
  To: git

Initialization in OpenSSL has been deprecated in version 1.1. This makes
compilation fail when deprecated APIs for OpenSSL are compile-time
disabled.

Signed-off-by: Rosen Penev <rosenp@gmail.com>
---
 imap-send.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/imap-send.c b/imap-send.c
index b4eb886e2..877a4e368 100644
--- a/imap-send.c
+++ b/imap-send.c
@@ -284,8 +284,10 @@ static int ssl_socket_connect(struct imap_socket *sock, int use_tls_only, int ve
 	int ret;
 	X509 *cert;
 
+#if (OPENSSL_VERSION_NUMBER < 0x10100000L)
 	SSL_library_init();
 	SSL_load_error_strings();
+#endif
 
 	meth = SSLv23_method();
 	if (!meth) {
-- 
2.20.1


^ permalink raw reply related

* Re: [PATCH] imap-send: Fix compilation without deprecated OpenSSL APIs
From: Rosen Penev @ 2018-12-27 17:55 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <xmqqwonvpjiy.fsf@gitster-ct.c.googlers.com>

On Wed, Dec 26, 2018 at 10:32 PM Junio C Hamano <gitster@pobox.com> wrote:
>
> Rosen Penev <rosenp@gmail.com> writes:
>
> > Initialization in OpenSSL has been deprecated in version 1.1.
>
> https://www.openssl.org/docs/man1.0.2/ssl/SSL_library_init.html says
>
>         SSL_library_init() must be called before any other action takes
>         place.
>
> https://www.openssl.org/docs/man1.1.0/ssl/SSL_library_init.html says
> the same.
Later on in the document it mentions that it is deprecated.
>
> Which makes it necessary for us to defend the following claim
>
> > This makes
> > compilation fail when deprecated APIs for OpenSSL are compile-time
> > disabled.
>
> as a valid problem description more rigorously.  To me, the cursory
> web-serfing I did above makes me suspect that an OpenSSL
> implementation with such a compile-time disabling _is_ buggy, as it
> forbids the API users to call an API function they are told to call
> before doing anything else.
I agree the man page is misleading. The changelog for 1.1.0 is very
clear though:

Added support for auto-initialisation and de-initialisation of the library.
     OpenSSL no longer requires explicit init or deinit routines to be called,
     except in certain circumstances. See the OPENSSL_init_crypto() and
     OPENSSL_init_ssl() man pages for further information.
     [Matt Caswell]

https://www.openssl.org/news/changelog.html#x12

>
> > Signed-off-by: Rosen Penev <rosenp@gmail.com>
> > ---
> >  imap-send.c | 2 ++
> >  1 file changed, 2 insertions(+)
> >
> > diff --git a/imap-send.c b/imap-send.c
> > index b4eb886e2..21f741c8c 100644
> > --- a/imap-send.c
> > +++ b/imap-send.c
> > @@ -284,8 +284,10 @@ static int ssl_socket_connect(struct imap_socket *sock, int use_tls_only, int ve
> >       int ret;
> >       X509 *cert;
> >
> > +#if (OPENSSL_VERSION_NUMBER < 0x10000000L)
>
> https://www.openssl.org/docs/man1.1.0/crypto/OPENSSL_VERSION_NUMBER.html
>
> says that OPENSSL_VERSION_NUMBER is of form 0xMNNFFPPS where M is
> major, NN is minor, FF is fix, PP is patch and S is status, and
> gives an example that 0x00906023 stands for 0.9.6.b beta 3 (M=0,
> NN=09, FF=06, PP=02 and S=3).  So "< 0x10000000L" means "anything
> with M smaller than 1".  IOW, we would no longer call _init() for
> e.g. "version 1.0.0 beta 0".  That contradicts with the first claim
> of the proposed log message ("deprecated in 1.1" implying that it is
> not yet deprecated in say 1.0.2).
This is a mistake. I will send a v2 to fix.

Oh I see what I did wrong. I mistakenly copied the above
OPENSSL_VERSION_NUMBER check without looking carefully at the number.
>
>
>
> >       SSL_library_init();
> >       SSL_load_error_strings();
> > +#endif
> >
> >       meth = SSLv23_method();
> >       if (!meth) {

^ permalink raw reply

* RE: [PATCH v1 2/4] config.mak.uname: support for modern HPE NonStop config.
From: Randall S. Becker @ 2018-12-27 17:44 UTC (permalink / raw)
  To: 'Eric Sunshine'; +Cc: 'Git List', 'Randall S. Becker'
In-Reply-To: <CAPig+cQ4p8kgAWji3r6WnudZdT4TOG15s1ip6p5SXmTec25mPw@mail.gmail.com>

On December 27, 2018 12:03, Eric Sunshine wrote:
> On Wed, Dec 26, 2018 at 6:05 PM <randall.s.becker@rogers.com> wrote:
> > A number of configuration options are not automatically detected by
> > configure mechanisms, including the location of Perl and Python.
> > [...]
> > Signed-off-by: Randall S. Becker <rsbecker@nexbridge.com>
> > ---
> > diff --git a/config.mak.uname b/config.mak.uname @@ -441,26 +441,45
> @@
> > ifeq ($(uname_S),NONSTOP_KERNEL)
> >         # Our's are in ${prefix}/bin (perl might also be in /usr/bin/perl).
> > -       PERL_PATH = ${prefix}/bin/perl
> > -       PYTHON_PATH = ${prefix}/bin/python
> > +       PERL_PATH = /usr/bin/perl
> > +       PYTHON_PATH = /usr/bin/python
> 
> Is the in-code comment about ${prefix} still applicable after this change?

The ${prefix} is not applicable on this platform for perl and python. Those locations must be in /usr/bin and are managed by the Operating System vendor not by customers. The change is wrapped in an IF so is only applicable to NonStop.
> 
> > +       # The current /usr/coreutils/rm at lowest support level does not work
> > +       # with the git test structure. Default to the older rm.
> > +       RM = /bin/rm -f
> 
> This comment would be far more helpful if it explained in what way 'rm'
> actually fails with the test suite. Without that information, the comment is
> effectively useless.

There is a temporary failure in the vendor supplied rm. The cause we never disclosed to my team. Honestly, it created a large amount of angst that should be gone but there are still old OS versions that have this issue. This is as much as we know.

> 
> >         # As detected by './configure'.
> >         # Missdetected, hence commented out, see below.
> >         #NO_CURL = YesPlease
> >         # Added manually, see above.
> > +       # Missdetected, hence commented out, see below.
> > +       #NO_CURL = YesPlease
> > +       # Added manually, see above.
> 
> These added lines are just duplicating the existing line immediately above. That's weird. I'll fix it. Thanks for the catch.
> 
> > +       # Not detected by ./configure. Add manually.
> > +       NEEDS_SSL_WITH_CURL = YesPlease
> > +       NEEDS_CRYPTO_WITH_SSL = YesPlease
> > +       HAVE_DEV_TTY = YesPlease
> 
> I find these comments about 'configure' "misdetecting" or "not detecting"
> features confusing. The point of config.mak.uname is to provide sane
> defaults for people building without using the 'configure' script, so it feels
> weird to be talking about 'configure'
> here. Also, what does it mean to say that 'configure' "misdetects"?
> Does that mean that it fails to write assignments such as "NO_CURL =
> YesPlease" to config.autogen or does it mean that it writes incorrect
> assignments to that file?

This came from another team. We can't currently use config.autogen anyway on the platform - this came from the prior attempt at porting. By misdetect I mean just that, however. We do not get good values.
> 
> > @@ -470,8 +489,13 @@ ifeq ($(uname_S),NONSTOP_KERNEL)
> > +       ifdef NO_PTHREADS
> > +       else # WIP, use of Posix User Threads is planned but not working yet
> > +               PTHREAD_CFLAGS = -D_PUT_MODEL_ -I/usr/include
> > +               PTHREAD_LIBS = -lput
> > +       endif
> 
> Why not a simpler 'ifndef'?

Another team is current working on the PTHREAD implementation and wanted this. I think what happened was that we have non-pthread requirements under development. I can remove this.


^ permalink raw reply

* Re: [PATCH v2 8/8] tests: mark tests broken under GIT_TEST_PROTOCOL_VERSION=2
From: Jonathan Nieder @ 2018-12-27 17:10 UTC (permalink / raw)
  To: Ævar Arnfjörð Bjarmason
  Cc: Junio C Hamano, Jeff King, git, Brandon Williams, Jonathan Tan
In-Reply-To: <87va3fdxcd.fsf@evledraar.gmail.com>

Hi,

Ævar Arnfjörð Bjarmason wrote:
> On Wed, Dec 26 2018, Junio C Hamano wrote:

>> Hmph.  The other overzealous thing you could do is to strenthen A
>> and "fix" the security issue in v2?  Which letter comes before A in
>> the alphabet? ;-)

Yes, agreed.  This is what I was hinting at in [1] with "it's a plain
bug".

> Sure, but that being useful is predicated on this supposed security
> mechanism being useful and not just security-through-obscurity, as noted
> in side-threads I don't think we have a convincing argument either way
> (and the one we do have is more on the "it's not secure" side).
>
> Of course we had that with v1 all along, but now that v2 is in released
> versions and in this insecure mode, we have a reason to closely look at
> whether we need to be issuing security releases, or doubling down on the
> "SECURITY" wording in git-fetch and then not carrying the mode forward.

Just for the record, as I've already said, I would be strongly against
removing this feature.  I know of multiple populations that make use
of it, and removing it would not serve them well.

Changing defaults and documentation is a separate story.

Sincerely,
Jonathan

[1] https://public-inbox.org/git/20181217231452.GA13835@google.com/

^ permalink raw reply

* Re: [PATCH v1 2/4] config.mak.uname: support for modern HPE NonStop config.
From: Eric Sunshine @ 2018-12-27 17:02 UTC (permalink / raw)
  To: randall.s.becker; +Cc: Git List, Randall S. Becker
In-Reply-To: <20181226230523.16572-3-randall.s.becker@rogers.com>

On Wed, Dec 26, 2018 at 6:05 PM <randall.s.becker@rogers.com> wrote:
> A number of configuration options are not automatically detected by
> configure mechanisms, including the location of Perl and Python.
> [...]
> Signed-off-by: Randall S. Becker <rsbecker@nexbridge.com>
> ---
> diff --git a/config.mak.uname b/config.mak.uname
> @@ -441,26 +441,45 @@ ifeq ($(uname_S),NONSTOP_KERNEL)
>         # Our's are in ${prefix}/bin (perl might also be in /usr/bin/perl).
> -       PERL_PATH = ${prefix}/bin/perl
> -       PYTHON_PATH = ${prefix}/bin/python
> +       PERL_PATH = /usr/bin/perl
> +       PYTHON_PATH = /usr/bin/python

Is the in-code comment about ${prefix} still applicable after this change?

> +       # The current /usr/coreutils/rm at lowest support level does not work
> +       # with the git test structure. Default to the older rm.
> +       RM = /bin/rm -f

This comment would be far more helpful if it explained in what way
'rm' actually fails with the test suite. Without that information, the
comment is effectively useless.

>         # As detected by './configure'.
>         # Missdetected, hence commented out, see below.
>         #NO_CURL = YesPlease
>         # Added manually, see above.
> +       # Missdetected, hence commented out, see below.
> +       #NO_CURL = YesPlease
> +       # Added manually, see above.

These added lines are just duplicating the existing line immediately above.

> +       # Not detected by ./configure. Add manually.
> +       NEEDS_SSL_WITH_CURL = YesPlease
> +       NEEDS_CRYPTO_WITH_SSL = YesPlease
> +       HAVE_DEV_TTY = YesPlease

I find these comments about 'configure' "misdetecting" or "not
detecting" features confusing. The point of config.mak.uname is to
provide sane defaults for people building without using the
'configure' script, so it feels weird to be talking about 'configure'
here. Also, what does it mean to say that 'configure' "misdetects"?
Does that mean that it fails to write assignments such as "NO_CURL =
YesPlease" to config.autogen or does it mean that it writes incorrect
assignments to that file?

> @@ -470,8 +489,13 @@ ifeq ($(uname_S),NONSTOP_KERNEL)
> +       ifdef NO_PTHREADS
> +       else # WIP, use of Posix User Threads is planned but not working yet
> +               PTHREAD_CFLAGS = -D_PUT_MODEL_ -I/usr/include
> +               PTHREAD_LIBS = -lput
> +       endif

Why not a simpler 'ifndef'?

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox