Git development
 help / color / mirror / Atom feed
* Re: [PATCH v2 2/2] t4256: mark support files as LF-only
From: Junio C Hamano @ 2018-12-13  2:40 UTC (permalink / raw)
  To: Johannes Schindelin via GitGitGadget; +Cc: git, Johannes Schindelin
In-Reply-To: <4275b8a5812b7108aecfc027fd6ace9b470a7c88.1544638490.git.gitgitgadget@gmail.com>

"Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com>
writes:

> From: Johannes Schindelin <johannes.schindelin@gmx.de>
>
> The test t4256-am-format-flowed.sh requires carefully applying a
> patch after ignoring padding whitespace. This breaks if the file
> is munged to include CRLF line endings instead of LF.
>
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
> ---

Thanks, will queue.

>  t/.gitattributes | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/t/.gitattributes b/t/.gitattributes
> index e7acedabe1..df05434d32 100644
> --- a/t/.gitattributes
> +++ b/t/.gitattributes
> @@ -16,6 +16,7 @@ t[0-9][0-9][0-9][0-9]/* -whitespace
>  /t4135/* eol=lf
>  /t4211/* eol=lf
>  /t4252/* eol=lf
> +/t4256/1/* eol=lf
>  /t5100/* eol=lf
>  /t5515/* eol=lf
>  /t556x_common eol=lf

^ permalink raw reply

* Re: [PATCH v2 1/2] .gitattributes: ensure t/oid-info/* has eol=lf
From: Junio C Hamano @ 2018-12-13  2:38 UTC (permalink / raw)
  To: Derrick Stolee via GitGitGadget; +Cc: git, Derrick Stolee, SZEDER Gábor
In-Reply-To: <4a22502a318a65f144b3b6542cc5e711a1811c78.1544638490.git.gitgitgadget@gmail.com>

"Derrick Stolee via GitGitGadget" <gitgitgadget@gmail.com> writes:

> From: Derrick Stolee <dstolee@microsoft.com>
>
> The new test_oid machinery in the test library requires reading
> some information from t/oid-info/hash-info and t/oid-info/oid.
> The shell logic that reads from these files is sensitive to CRLF
> line endings, causing a problem when the test suite is run on a
> Windows machine that converts LF to CRLF.
>
> Exclude the files in this folder from this conversion.
>
> Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
> ---

It seems that this step is identical to v1, to which SZEDER Gábor
had trouble with (cf. <20181212133945.GV30222@szeder.dev>).  I am
guessing that the review and v2 e-mails have crossed.

FWIW, I personally do not think "is sensitive to CRLF" is too bad,
as my attempt to clarify it does not make it much better, e.g.

	The logic to read from these files in shell uses built-in
	"read" command, which leaves CR at the end of these text
	files when they are checked out with CRLF line endings, at
	least when run with bash shipped with Git for Windows.  This
	results in an unexpected value in the variable these lines
	are read into, leading the tests to fail.

So, I'll keep what I queued when I received v1 for now.

^ permalink raw reply

* Re: [RFC] A global mailmap service
From: Junio C Hamano @ 2018-12-13  2:17 UTC (permalink / raw)
  To: Lukas Fleischer; +Cc: git
In-Reply-To: <154454625546.29948.6229097078125430492@typhoon>

Lukas Fleischer <lfleischer@lfos.de> writes:

> The basic idea of the service I imagine is simple:
>
> 1. You register a primary email address and specify a password. You
>    receive a verification email to confirm that the address is yours.

I would do so with my current, reachable address, I'd presume.

> 2. At any time, you can add additional email addresses and link them to
>    your primary email address, using your previously specified password.
>    You can also update your primary email address. Any new addresses
>    obtain verification emails such that you cannot steal somebody else's
>    identity.

With this, I won't be able to add my ancient identities that appear
in our history.  I would imagine that one of the common reasons
people use different identities in a project is that people changed
e-mail providers or jobs.


^ permalink raw reply

* Re: [PATCH on master v2] revision: use commit graph in get_reference()
From: Jeff King @ 2018-12-13  1:27 UTC (permalink / raw)
  To: Jonathan Tan; +Cc: gitster, git, stolee
In-Reply-To: <20181212195812.232726-1-jonathantanmy@google.com>

On Wed, Dec 12, 2018 at 11:58:12AM -0800, Jonathan Tan wrote:

> > Yeah, this was the part that took me a bit to figure out, as well. The
> > optimization here is really just avoiding a call to lookup_commit(),
> > which will do a single hash-table lookup. I wonder if that's actually
> > worth this more complex interface (as opposed to just always taking an
> > oid and then always returning a "struct commit", which could be old or
> > new).
> 
> Avoidance of lookup_commit() is more important than an optimization, I
> think. Here, we call lookup_commit() only when we know that that object
> is a commit (by its presence in a commit graph). If we just called it
> blindly, we might mistakenly create a commit for that hash when it is
> actually an object of another type. (We could inline lookup_commit() in
> parse_commit_in_graph_one(), removing the object creation part, but that
> adds complexity as well.)

I was thinking we would only do so in the happy path when we find a
commit. I.e., something like:

  obj = lookup_object(oid); /* does not auto-vivify */
  if (obj && obj->parsed)
	return obj;

  if (we_have_it_in_commit_graph) {
	commit = obj || lookup_commit(oid);
	fill_in_details_from_commit_graph(commit);
	return &commit->obj;
  } else {
	return parse_object(oid);
  }

which is more along the lines of that parse_probably_commit() that
Stolee mentioned.

-Peff

^ permalink raw reply

* Re: [PATCH v3 1/4] pack-protocol.txt: accept error packets in any context
From: Masaya Suzuki @ 2018-12-13  1:17 UTC (permalink / raw)
  To: peff; +Cc: Josh Steadmon, git, Junio C Hamano
In-Reply-To: <20181212110206.GA30673@sigill.intra.peff.net>

On Wed, Dec 12, 2018 at 3:02 AM Jeff King <peff@peff.net> wrote:
>
> On Tue, Dec 11, 2018 at 04:25:15PM -0800, Josh Steadmon wrote:
>
> > From: Masaya Suzuki <masayasuzuki@google.com>
> >
> > In the Git pack protocol definition, an error packet may appear only in
> > a certain context. However, servers can face a runtime error (e.g. I/O
> > error) at an arbitrary timing. This patch changes the protocol to allow
> > an error packet to be sent instead of any packet.
> >
> > Following this protocol spec change, the error packet handling code is
> > moved to pkt-line.c.
>
> This is a change in the spec with an accompanying change in the code,
> which raises the question: what do other implementations do with this
> change (both older Git, and implementations like JGit, libgit2, etc)?

JGit is similar to Git. It parses "ERR " in limited places. When it sees an ERR
packet in an unexpected place, it'll fail somewhere in the parsing code.

https://github.com/eclipse/jgit/blob/30c6c7542190c149e2aee792f992a312a5fc5793/org.eclipse.jgit/src/org/eclipse/jgit/transport/PacketLineIn.java#L145-L147
https://github.com/eclipse/jgit/blob/f40b39345cd9b54473ee871bff401fe3d394ffe3/org.eclipse.jgit/src/org/eclipse/jgit/transport/BasePackConnection.java#L208

I'm not familiar with libgit2 code, but it seems it handles this at a
lower level. An error type packet is parsed out at a low level, and
the error handling is done by the callers of the packet parser.

https://github.com/libgit2/libgit2/blob/bea65980c7a42e34edfafbdc40b199ba7b2a564e/src/transports/smart_pkt.c#L482-L483

I cannot find an ERR packet handling in go-git. It seems to me that if
an ERR packet appears it treats it as a parsing error.

https://github.com/src-d/go-git/blob/master/plumbing/protocol/packp/common.go#L60-L62

>
> I think the answer for older Git is "hang up unceremoniously", which is
> probably OK given the semantics of the change. And I'd suspect most
> other implementations would do the same. I just wonder if anybody tested
> it with other implementations.

I'm thinking aloud here. There would be two aspects of the protocol
compatibility: (1) new clients speak to old servers (2) old clients
speak to a new server that speaks the updated protocol.

For (1), I assume that in the Git pack protocol, a packet starting
from "ERR " does not appear naturally except for a very special case
that the server doesn't support sideband, but using the updated
protocol. As you mentioned, at first it looks like this can mistakenly
parse the pack file of git-receive-pack as an ERR packet, assuming
that git-receive-pack's pack file is packetized. Actually
git-receive-pack's pack file is not packetized in the Git pack
protocol (https://github.com/git/git/blob/master/builtin/receive-pack.c#L1695).
I recently wrote a Git protocol parser
(https://github.com/google/gitprotocolio), and I confirmed that this
is the case at least for the HTTP transport. git-upload-pack's pack
file is indeed packetized, but packetized with sideband. Except for
the case where sideband is not used, the packfiles wouldn't be
considered as an ERR packet accidentally.

For (2), if the old clients see an unexpected ERR packet, they cannot
parse it. They would handle this unparsable data as if the server is
not speaking Git protocol correctly. Even if the old clients just
ignore the packet, due to the nature of the ERR packet, the server
won't send further data. The client won't be able to proceed. Overall,
the clients anyway face an error, and the only difference would be
whether the clients can show an error nicely or not. The new clients
will show the errors nicely to users. Old clients will not.

>
> > +An error packet is a special pkt-line that contains an error string.
> > +
> > +----
> > +  error-line     =  PKT-LINE("ERR" SP explanation-text)
> > +----
> > +
> > +Throughout the protocol, where `PKT-LINE(...)` is expected, an error packet MAY
> > +be sent. Once this packet is sent by a client or a server, the data transfer
> > +process defined in this protocol is terminated.
>
> The packfile data is typically packetized, too, and contains arbitrary
> data (that could have "ERR" in it). It looks like we don't specifically
> say PKT-LINE() in that part of the protocol spec, though, so I think
> this is OK.

As I described above, as far as I can see, the packfile in
git-upload-pack is not packetized. The packfile in git-receive-pack is
packetized but typically with sideband. At least at the Git pack
protocol level, this should be OK.

>
> Likewise, in the implementation:
>
> > diff --git a/pkt-line.c b/pkt-line.c
> > index 04d10bbd03..ce9e42d10e 100644
> > --- a/pkt-line.c
> > +++ b/pkt-line.c
> > @@ -346,6 +346,10 @@ enum packet_read_status packet_read_with_status(int fd, char **src_buffer,
> >               return PACKET_READ_EOF;
> >       }
> >
> > +     if (starts_with(buffer, "ERR ")) {
> > +             die(_("remote error: %s"), buffer + 4);
> > +     }
> > +
> >       if ((options & PACKET_READ_CHOMP_NEWLINE) &&
> >           len && buffer[len-1] == '\n')
> >               len--;
>
> This ERR handling has been moved to a very low level. What happens if
> we're passing arbitrary data via the packet_read() code? Could we
> erroneously trigger an error if a packfile happens to have the bytes
> "ERR " at a packet boundary?
>
> For packfiles via upload-pack, I _think_ we're OK, because we only
> packetize it when a sideband is in use. In which case this would never
> match, because we'd have "\1" in the first byte slot.
>
> But are there are other cases we need to worry about? Just
> brainstorming, I can think of:
>
>   1. We also pass packetized packfiles between git-remote-https and
>      the stateless-rpc mode of fetch-pack/send-pack. And I don't think
>      we use sidebands there.
>
>   2. The packet code is used for long-lived clean/smudge filters these
>      days, which also pass arbitrary data.
>
> So I think it's probably not a good idea to unconditionally have callers
> of packet_read_with_status() handle this. We'd need a flag like
> PACKET_READ_RESPECT_ERR, and to trigger it from the appropriate callers.

This is outside of the Git pack protocol so having a separate parsing
mode makes sense to me.

>
> -Peff

^ permalink raw reply

* Re: Git Repository
From: brian m. carlson @ 2018-12-12 23:46 UTC (permalink / raw)
  To: John Lopez; +Cc: git
In-Reply-To: <3e8e01d49262$27e55be0$77b013a0$@iarc.nv.gov>

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

On Wed, Dec 12, 2018 at 01:32:17PM -0800, John Lopez wrote:
> Afternoon,
> 
> I am inquiring to see if you have a repo for your software where we can
> point our 3rd party software to automatically download your software? It
> requires CAB's not sure  if you do this or not, if so can you send me the
> link for future versions

You haven't specified what platform you're looking for, but by the
mention of CAB files, I'll assume it's Windows.

The Git Project doesn't distribute anything other than source; we
provide a Git repository and tarballs. However, others do distribute
various packages, and for Windows that's typically Git for Windows,
which you can find at https://gitforwindows.org/. Their releases page
doesn't seem to have any CAB files for download, but you could try
asking in their GitHub repository[0] if you'd like that format.

[0] https://github.com/git-for-windows/git
-- 
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: [PATCH 3/4] submodule--helper: fix BUG message in ensure_core_worktree
From: Stefan Beller @ 2018-12-12 22:46 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <xmqqsgz87bj9.fsf@gitster-ct.c.googlers.com>

> Unlike the step 2/4 I commented on, this does explain what this
> wants to do and why, at least when looked from sideways.  Is the
> above saying the same as the following two-liner?
>
>         An ealier mistake while rebasing to produce 74d4731da1
>         failed to update this BUG message.  Fix this.

I am not sure if it was rebasing, which was executed mistakenly.
So maybe just saying "74d4731da1 contains a faulty BUG
message. Fix it." would do.

The intent of the longer message was to shed light in how I found
the BUG (ie. I did not see the BUG message, which would ask me
to actually fix a bug, but found it via code inspection), which I
thought was valuable information, too.

^ permalink raw reply

* Re: [PATCH 0/4]
From: Stefan Beller @ 2018-12-12 22:35 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <xmqqefas8ss4.fsf@gitster-ct.c.googlers.com>

On Fri, Dec 7, 2018 at 9:57 PM Junio C Hamano <gitster@pobox.com> wrote:
>
> Stefan Beller <sbeller@google.com> writes:
>
> > A couple days before the 2.19 release we had a bug report about
> > broken submodules[1] and reverted[2] the commits leading up to them.
> >
> > The behavior of said bug fixed itself by taking a different approach[3],
> > specifically by a weaker enforcement of having `core.worktree` set in a
> > submodule [4].
> >
> > The revert [2] was overly broad as we neared the release, such that we wanted
> > to rather keep the known buggy behavior of always having `core.worktree` set,
> > rather than figuring out how to fix the new bug of having 'git submodule update'
> > not working in old style repository setups.
> >
> > This series re-introduces those reverted patches, with no changes in code,
> > but with drastically changed commit messages, as those focus on why it is safe
> > to re-introduce them instead of explaining the desire for the change.
>
> The above was a bit too cryptic for me to grok, so let me try
> rephrasing to see if I got them all correctly.
>
>  - three-patch series leading to 984cd77ddb were meant to fix some
>    bug, but the series itself was buggy and caused problems; we got
>    rid of them

yes.

>  - the problem 984cd77ddb wanted to fix was fixed differently

e98317508c02*

>    without reintroducing the problem three-patch series introduced.
>    That fix is already with us since 4d6d6ef1fc.

yes.

>  - now these three changes that were problematic in the past is
>    resent without any update (other than that it has one preparatory
>    patch to add tests).

One of the three changes was problematic, (e98317508c02),
the other two are good (in company of the third).

But those two were not good on their own, which is why we
reverted all three at once.

Now that we have a different approach for the third,
we could re-introduce the two.
(4fa4f90ccd8, 984cd77ddbf0)

We do that, but with precaution (an extra test);
additional careful reading found a typo, hence
we have "a third" patch, but that is totally different
than what above was referred to "one of three".


> Is that what is going on?  Obviously I am not getting "the other"
> benefit we wanted to gain out of these three patches (because the
> above description fails to explain what that is), other than to fix
> the issue that was fixed by 4d6d6ef1fc.

The other benefit refers to
7e25437d35 (Merge branch 'sb/submodule-core-worktree', 2018-07-18)
which was reverted as a whole.
It's goal was to handle core.worktree appropriately.

(Instead of having it there all the time, only have it when
a working tree is present)

> Sorry for being puzzled...

This means I need to revamp the commit messages and
cover letter altogether.

Stefan

^ permalink raw reply

* [PATCH] git-quiltimport: Add --keep-non-patch option
From: Laura Abbott @ 2018-12-12 22:32 UTC (permalink / raw)
  To: git; +Cc: Laura Abbott

git-am has the --keep-non-patch option to pass -b to
git-mailinfo for keeping subject prefixes intact. Allow
this option to be used with quiltimport as well.

Signed-off-by: Laura Abbott <labbott@redhat.com>
---
 Documentation/git-quiltimport.txt | 5 ++++-
 git-quiltimport.sh                | 6 +++++-
 2 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/Documentation/git-quiltimport.txt b/Documentation/git-quiltimport.txt
index 8cf952b4d..70562dc4c 100644
--- a/Documentation/git-quiltimport.txt
+++ b/Documentation/git-quiltimport.txt
@@ -10,7 +10,7 @@ SYNOPSIS
 --------
 [verse]
 'git quiltimport' [--dry-run | -n] [--author <author>] [--patches <dir>]
-		[--series <file>]
+		[--series <file>] [--keep-non-patch]
 
 
 DESCRIPTION
@@ -56,6 +56,9 @@ The default for the series file is <patches>/series
 or the value of the `$QUILT_SERIES` environment
 variable.
 
+--keep-non-patch::
+	Pass `-b` flag to 'git mailinfo' (see linkgit:git-mailinfo[1]).
+
 GIT
 ---
 Part of the linkgit:git[1] suite
diff --git a/git-quiltimport.sh b/git-quiltimport.sh
index 6d3a88dec..e3d390974 100755
--- a/git-quiltimport.sh
+++ b/git-quiltimport.sh
@@ -8,6 +8,7 @@ n,dry-run     dry run
 author=       author name and email address for patches without any
 patches=      path to the quilt patches
 series=       path to the quilt series file
+keep-non-patch Pass -b to git mailinfo
 "
 SUBDIRECTORY_ON=Yes
 . git-sh-setup
@@ -32,6 +33,9 @@ do
 		shift
 		QUILT_SERIES="$1"
 		;;
+	--keep-non-patch)
+		MAILINFO_OPT="-b"
+		;;
 	--)
 		shift
 		break;;
@@ -98,7 +102,7 @@ do
 		continue
 	fi
 	echo $patch_name
-	git mailinfo "$tmp_msg" "$tmp_patch" \
+	git mailinfo $MAILINFO_OPT "$tmp_msg" "$tmp_patch" \
 		<"$QUILT_PATCHES/$patch_name" >"$tmp_info" || exit 3
 	test -s "$tmp_patch" || {
 		echo "Patch is empty.  Was it split wrong?"
-- 
2.19.1


^ permalink raw reply related

* RE: High locking contention during repack?
From: Iucha, Florin @ 2018-12-12 21:50 UTC (permalink / raw)
  To: git@vger.kernel.org; +Cc: Ævar Arnfjörð Bjarmason, Jeff King
In-Reply-To: <SN1PR12MB23844F35A879EA185AD6B00B95A70@SN1PR12MB2384.namprd12.prod.outlook.com>

> Is there an easy way to figure out what object we're processing in create_delta? I have filtered out some large data files in .gitattributes already.

For the record:
* add breakpoint on create_delta at diff-delta.c:321
* stop the execution
* switch to the worker thread
* select frame 3: threaded_find_deltas at builtin/pack-objects.c:2408
* p/x me->list.idx.oid.hash

florin

^ permalink raw reply

* Git Repository
From: John Lopez @ 2018-12-12 21:32 UTC (permalink / raw)
  To: git

Afternoon,

I am inquiring to see if you have a repo for your software where we can
point our 3rd party software to automatically download your software? It
requires CAB's not sure  if you do this or not, if so can you send me the
link for future versions

Thank you for your time and support.
V/R
John Lopez
Systems Engineer



^ permalink raw reply

* Re: How to perform efficient incremental update of a git repo from a large SVN repo? Bug or clueless user?
From: Mateusz Loskot @ 2018-12-12 21:37 UTC (permalink / raw)
  To: git
In-Reply-To: <9181bfae-8a36-2051-179f-438706ba7968@3ds.com>

On Wed, 12 Dec 2018 at 21:13, James Mason <James.Mason@3ds.com> wrote:
>
> I have a large and active SVN repository.  More than 36,000 revisions,
> thousands of branches - new ones created daily - and a non-standard
> layout.  Also, a secondary git repository that serves as a faithful
> "git" copy of the work accumulating in SVN (git version 2.9.5).  I seek
> an efficient way to routinely (nightly?) update the git repo with new
> changes accumulating in SVN.

SubGit [1]

I've been using it to translate SVN repository with ~40K revisions,
producing ~25GB Git bare repo.
SubGit is x10-x100... faster than `git svn`.

[1] https://subgit.com/

Best regards,
-- 
Mateusz Loskot, http://mateusz.loskot.net

^ permalink raw reply

* [PATCH] submodule: correct documentation for open_submodule
From: Stefan Beller @ 2018-12-12 20:58 UTC (permalink / raw)
  To: sbeller; +Cc: git, gitster, jonathantanmy
In-Reply-To: <20181212202205.170998-1-sbeller@google.com>

When 99017ffac8 (submodule: use submodule repos for object lookup, 2018-11-13)
was merged, I had not yet addressed all outstanding review comments, such
as adapting the documentation for the function `open_submodule`, which
changed its signature during the development of that series.
Fix the documentation, and annotate setting of `submodule_prefix`
to indicate we're dealing with a submodule repository.

Signed-off-by: Stefan Beller <sbeller@google.com>
---

> I'll incorporate these changes once resending.

... only to realize it hit next already, so we'd prefer updates instead
of resending the whole series.

Maybe a patch like this?

 submodule.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/submodule.c b/submodule.c
index 262f03f118..4486ff664b 100644
--- a/submodule.c
+++ b/submodule.c
@@ -483,12 +483,13 @@ void prepare_submodule_repo_env(struct argv_array *out)
 }
 
 /*
- * Initialize 'out' based on the provided submodule path.
+ * Initialize a repository struct for a submodule based on the provided 'path'.
  *
  * Unlike repo_submodule_init, this tolerates submodules not present
  * in .gitmodules. This function exists only to preserve historical behavior,
  *
- * Returns 0 on success, -1 when the submodule is not present.
+ * Returns the repository struct on success,
+ * NULL when the submodule is not present.
  */
 static struct repository *open_submodule(const char *path)
 {
@@ -501,6 +502,7 @@ static struct repository *open_submodule(const char *path)
 		return NULL;
 	}
 
+	/* Mark it as a submodule */
 	out->submodule_prefix = xstrdup(path);
 
 	strbuf_release(&sb);
-- 
2.20.0.rc2.230.gc28305e538


^ permalink raw reply related

* Re: [PATCH 18/23] submodule: use submodule repos for object lookup
From: Stefan Beller @ 2018-12-12 20:22 UTC (permalink / raw)
  To: sbeller; +Cc: git, gitster, jonathantanmy
In-Reply-To: <CAGZ79kZ72eQPiQ_KW1QkiRqMFUAUpCQCHZdk3RSMVK7TGPE2aw@mail.gmail.com>

Signed-off-by: Stefan Beller <sbeller@google.com>
---

On Thu, Nov 15, 2018 at 11:54 AM Jonathan Tan <jonathantanmy@google.com> wrote:
> Other than that, this patch looks good.

I'll incorporate these changes once resending.

Stefan


 submodule.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/submodule.c b/submodule.c
index 262f03f118..4486ff664b 100644
--- a/submodule.c
+++ b/submodule.c
@@ -483,12 +483,13 @@ void prepare_submodule_repo_env(struct argv_array *out)
 }
 
 /*
- * Initialize 'out' based on the provided submodule path.
+ * Initialize a repository struct for a submodule based on the provided 'path'.
  *
  * Unlike repo_submodule_init, this tolerates submodules not present
  * in .gitmodules. This function exists only to preserve historical behavior,
  *
- * Returns 0 on success, -1 when the submodule is not present.
+ * Returns the repository struct on success,
+ * NULL when the submodule is not present.
  */
 static struct repository *open_submodule(const char *path)
 {
@@ -501,6 +502,7 @@ static struct repository *open_submodule(const char *path)
 		return NULL;
 	}
 
+	/* Mark it as a submodule */
 	out->submodule_prefix = xstrdup(path);
 
 	strbuf_release(&sb);
-- 
2.20.0.rc2.230.gc28305e538


^ permalink raw reply related

* How to perform efficient incremental update of a git repo from a large SVN repo? Bug or clueless user?
From: James Mason @ 2018-12-12 19:19 UTC (permalink / raw)
  To: git

I have a large and active SVN repository.  More than 36,000 revisions,
thousands of branches - new ones created daily - and a non-standard
layout.  Also, a secondary git repository that serves as a faithful
"git" copy of the work accumulating in SVN (git version 2.9.5).  I seek
an efficient way to routinely (nightly?) update the git repo with new
changes accumulating in SVN.

Most of what I have found on the web suggests that, when properly
configured, incremental update will be correctly accomplished by "git
svn fetch".  My experience so far is that - while that will eventually
work - it is remarkably slow.  It's true that "git svn fetch" doesn't
redundantly add content - but it doesn't resume without appearing to
walk over all the old SVN revisions (watch the contents of
svn/.metadata).  For a large SVN repository - this can take days.

I initially posed this as a question on stack overflow (see "Routinely
updating a git repo from a large svn repo" ).  It appeared that I had
learned something about how to use "--revision" to speed things along -
so I recently added that as an answer to my own question (update now
taking routinely less than an hour).  Still that all seemed really odd.
"--revision" isn't documented as an option to "git svn fetch", you need
to dig START out of .git/svn/.metadata and END out of the SVN repository.

Today I searched on the string "branches-MaxRev" and found something
more explicitly at odds with my experience (in the git-svn document):

     Note that git-svn keeps track of the highest revision in which a
branch or tag has appeared. If the subset of
     branches or tags is changed after fetching, then
$GIT_DIR/svn/.metadata must be manually edited to remove
     (or reset) branches-maxRev and/or tags-maxRev as appropriate."

I read the documentation as saying that "git svn fetch" tracks where it
left off (which was my assumption before experiencing otherwise).   So
what I'm seeing seems like a bug - but I'm worried that my use of
"--revision" may not be entirely legitimate.

Any assistance, advice, etc., would be most appreciated.

-jrm

This email and any attachments are intended solely for the use of the individual or entity to whom it is addressed and may be confidential and/or privileged.

If you are not one of the named recipients or have received this email in error,

(i) you should not read, disclose, or copy it,

(ii) please notify sender of your receipt by reply email and delete this email and all attachments,

(iii) Dassault Systèmes does not accept or assume any liability or responsibility for any use of or reliance on this email.


Please be informed that your personal data are processed according to our data privacy policy as described on our website. Should you have any questions related to personal data protection, please contact 3DS Data Protection Officer at 3DS.compliance-privacy@3ds.com<mailto:3DS.compliance-privacy@3ds.com>


For other languages, go to https://www.3ds.com/terms/email-disclaimer

^ permalink raw reply

* Efficient incremental update of a git repo from a large SVN repo. Bug or clueless user?
From: James Mason @ 2018-12-12 20:11 UTC (permalink / raw)
  To: git

I have a large and active SVN repository.  More than 36,000 revisions,
thousands of branches - new ones created daily - and a non-standard
layout.  Also, a secondary git repository that serves as a faithful
"git" copy of the work accumulating in SVN (git version 2.9.5).  I seek
an efficient way to routinely (nightly?) update the git repo with new
changes accumulating in SVN.

Most of what I have found on the web suggests that, when properly
configured, incremental update will be correctly accomplished by "git
svn fetch".  My experience so far is that - while that will eventually
work - it is remarkably slow.  It's true that "git svn fetch" doesn't
redundantly add content - but it doesn't resume without appearing to
walk over all the old SVN revisions (watch the contents of
svn/.metadata).  For a large SVN repository - this can take days.

I initially posed this as a question on stack overflow (see "Routinely
updating a git repo from a large svn repo" ).  It appeared that I had
learned something about how to use "--revision" to speed things along -
so I recently added that as an answer to my own question (update now
taking routinely less than an hour).  Still that all seemed really odd.
"--revision" isn't documented as an option to "git svn fetch", you need
to dig START out of .git/svn/.metadata and END out of the SVN repository.

Today I searched on the string "branches-MaxRev" and found something
more explicitly at odds with my experience (in the git-svn document):

     Note that git-svn keeps track of the highest revision in which a
branch or tag has appeared. If the subset of
     branches or tags is changed after fetching, then
$GIT_DIR/svn/.metadata must be manually edited to remove
     (or reset) branches-maxRev and/or tags-maxRev as appropriate."

I read the documentation as saying that "git svn fetch" tracks where it
left off (which was my assumption before experiencing otherwise).   So
what I'm seeing seems like a bug - but I'm worried that my use of
"--revision" may not be entirely legitimate.

Any assistance, advice, etc., would be most appreciated.

-jrm

This email and any attachments are intended solely for the use of the individual or entity to whom it is addressed and may be confidential and/or privileged.

If you are not one of the named recipients or have received this email in error,

(i) you should not read, disclose, or copy it,

(ii) please notify sender of your receipt by reply email and delete this email and all attachments,

(iii) Dassault Systèmes does not accept or assume any liability or responsibility for any use of or reliance on this email.


Please be informed that your personal data are processed according to our data privacy policy as described on our website. Should you have any questions related to personal data protection, please contact 3DS Data Protection Officer at 3DS.compliance-privacy@3ds.com<mailto:3DS.compliance-privacy@3ds.com>


For other languages, go to https://www.3ds.com/terms/email-disclaimer

^ permalink raw reply

* Re: review "git range-diff -h" output
From: Duy Nguyen @ 2018-12-12 20:09 UTC (permalink / raw)
  To: Stefan Beller; +Cc: Git Mailing List
In-Reply-To: <CAGZ79kZwS4J_bOd+2iTbVTJqMW_+P5Lzt1Zs92JBtfkBnj3juw@mail.gmail.com>

On Wed, Dec 12, 2018 at 9:05 PM Stefan Beller <sbeller@google.com> wrote:
>
> On Wed, Dec 12, 2018 at 11:47 AM Duy Nguyen <pclouds@gmail.com> wrote:
> >
> > I'm not going to bother you with patches (yet) but I could use a few
> > eyeballs to check the help usage (and perhaps the option grouping) for
> > commands that take diff options.
>
> The grouping looks good from a cursory view, but ...
>
> > Forget about hiding irrelevant options for this particular command for
> > now. This is something we can do later.
>
> .. this seems relevant: range-diff is a diff of diffs,
> so for that particular command, I'd want to know if the option
> (e.g. --stat) is applied to the inner or outer diff.

Yeah I'll probably should send "git diff --no-index -h" instead.

> Now back to your topic: Are you planning to send patches that
> autogenerate these helps based on our source code?
> Should I pay attention to some specific part of the output?

All this comes from a new 'struct option' array in diff.c, very much
like how we handle options everywhere else. So yeah sort of auto
generation, but all the text has to be typed in manually first.
-- 
Duy

^ permalink raw reply

* Re: review "git range-diff -h" output
From: Stefan Beller @ 2018-12-12 20:05 UTC (permalink / raw)
  To: Duy Nguyen; +Cc: git
In-Reply-To: <20181212194658.GA25117@duynguyen.home>

On Wed, Dec 12, 2018 at 11:47 AM Duy Nguyen <pclouds@gmail.com> wrote:
>
> I'm not going to bother you with patches (yet) but I could use a few
> eyeballs to check the help usage (and perhaps the option grouping) for
> commands that take diff options.

The grouping looks good from a cursory view, but ...

> Forget about hiding irrelevant options for this particular command for
> now. This is something we can do later.

.. this seems relevant: range-diff is a diff of diffs,
so for that particular command, I'd want to know if the option
(e.g. --stat) is applied to the inner or outer diff.

Now back to your topic: Are you planning to send patches that
autogenerate these helps based on our source code?
Should I pay attention to some specific part of the output?

^ permalink raw reply

* Re: [PATCH on master v2] revision: use commit graph in get_reference()
From: Jonathan Tan @ 2018-12-12 19:58 UTC (permalink / raw)
  To: peff; +Cc: gitster, jonathantanmy, git, stolee
In-Reply-To: <20181211105439.GA8452@sigill.intra.peff.net>

> On Sun, Dec 09, 2018 at 09:51:28AM +0900, Junio C Hamano wrote:
> 
> > > -static int parse_commit_in_graph_one(struct commit_graph *g, struct commit *item)
> > > +static struct commit *parse_commit_in_graph_one(struct repository *r,
> > > +						struct commit_graph *g,
> > > +						struct commit *shell,
> > > +						const struct object_id *oid)
> > 
> > Now the complexity of the behaviour of this function deserves to be
> > documented in a comment in front.  Let me see if I can get it
> > correctly without such a comment by explaining the function aloud.
> > 
> > The caller may or may not have already obtained an in-core commit
> > object for a given object name, so shell could be NULL but otherwise
> > it could be used for optimization.  When shell==NULL, the function
> > looks up the commit object using the oid parameter instead.  The
> > returned in-core commit has the parents etc. filled as if we ran
> > parse_commit() on it.  If the commit is not yet in the graph, the
> > caller may get a NULL even if the commit exists.

In the next revision, I'll unify parse_commit_in_graph_one() (quoted
above) with parse_commit_in_graph(), so that the comment I wrote for the
latter can cover the entire functionality. I think the comment covers
the details that you outline here.

> Yeah, this was the part that took me a bit to figure out, as well. The
> optimization here is really just avoiding a call to lookup_commit(),
> which will do a single hash-table lookup. I wonder if that's actually
> worth this more complex interface (as opposed to just always taking an
> oid and then always returning a "struct commit", which could be old or
> new).

Avoidance of lookup_commit() is more important than an optimization, I
think. Here, we call lookup_commit() only when we know that that object
is a commit (by its presence in a commit graph). If we just called it
blindly, we might mistakenly create a commit for that hash when it is
actually an object of another type. (We could inline lookup_commit() in
parse_commit_in_graph_one(), removing the object creation part, but that
adds complexity as well.)

^ permalink raw reply

* Re: [PATCH 2/2] RF+ENH(TST): compare the entire list of submodule status --recursive to stay intact
From: Stefan Beller @ 2018-12-12 19:48 UTC (permalink / raw)
  To: debian; +Cc: git
In-Reply-To: <20181211040839.17472-2-debian@onerussian.com>

On Mon, Dec 10, 2018 at 8:09 PM Yaroslav Halchenko
<debian@onerussian.com> wrote:

Thanks for the patches. The first patch looks good to me!

> [PATCH 2/2] RF+ENH(TST): compare the entire list of submodule status --recursive to stay intact

The subject is a bit cryptic (specifically the first part before the
colon), maybe

  t7406: compare entire submodule status for --reset-hard mode

?


> For submodule update --reset-hard the best test is comparison of the
> entire status as shown by submodule status --recursive.  Upon update
> --reset-hard we should get back to the original state, with all the
> branches being the same (no detached HEAD) and commits identical to
> original  (so no merges, new commits, etc).

"original state" can mean different things to different people. I'd think
we could be more precise:

   ... we should get to the state that the submodule is reset to the
    object id as the superprojects gitlink points at, irrespective of the
    submodule branch.


>  test_expect_success 'submodule update --merge staying on master' '
>         (cd super/submodule &&
> -         git reset --hard HEAD~1
> +        git reset --hard HEAD~1

unrelated white space change?

>         ) &&
>         (cd super &&
>          (cd submodule &&
> @@ -307,16 +318,28 @@ test_expect_success 'submodule update --merge staying on master' '
>  '
>
>  test_expect_success 'submodule update --reset-hard staying on master' '
> [..]
> +'
> +

The tests look good to me, though I wonder if we'd rather want to inline
{record/compare}_submodule_status as then you'd not need to look it up
and the functions are rather short?

^ permalink raw reply

* review "git range-diff -h" output
From: Duy Nguyen @ 2018-12-12 19:46 UTC (permalink / raw)
  To: git

I'm not going to bother you with patches (yet) but I could use a few
eyeballs to check the help usage (and perhaps the option grouping) for
commands that take diff options.

Forget about hiding irrelevant options for this particular command for
now. This is something we can do later.

usage: git range-diff [<options>] <old-base>..<old-tip> <new-base>..<new-tip>
   or: git range-diff [<options>] <old-tip>...<new-tip>
   or: git range-diff [<options>] <base> <old-tip> <new-tip>

    --creation-factor <n>
                          Percentage by which creation is weighted
    --no-dual-color       use simple diff colors

Diff output format options
    -p, --patch           generate patch
    -s, --no-patch        suppress diff output
    -u                    generate patch
    -U, --unified <n>     generate diffs with <n> lines context
    -W, --function-context
                          generate diffs with <n> lines context
    --raw                 generate the diff in raw format
    --patch-with-raw      synonym for '-p --raw'
    --patch-with-stat     synonym for '-p --stat'
    --numstat             machine friendly --stat
    --shortstat           output only the last line of --stat
    -X, --dirstat[=<param1,param2>...]
                          output the distribution of relative amount of changes for each sub-directory
    --cumulative          synonym for --dirstat=cumulative
    --dirstat-by-file[=<param1,param2>...]
                          synonym for --dirstat=files,param1,param2...
    --check               warn if changes introduce conflict markers or whitespace errors
    --summary             condensed summary such as creations, renames and mode changes
    --name-only           show only names of changed files
    --name-status         show only names and status of changed files
    --stat[=<width>[,<name-width>[,<count>]]]
                          generate diffstat
    --stat-width <width>  generate diffstat with a given width
    --stat-name-width <width>
                          generate diffstat with a given name width
    --stat-graph-width <width>
                          generate diffstat with a given graph width
    --stat-count <count>  generate diffstat with limited lines
    --compact-summary     generate compact summary in diffstat
    --binary              output a binary diff that can be applied
    --full-index          show full pre- and post-image object names on the "index" lines
    --color[=<when>]      show colored diff
    --ws-error-highlight <kind>
                          highlight whitespaces errors in the context, old or new lines in the diff
    -z                    do not munge pathnames and use NULs as output field terminators in --raw or --numstat
    --abbrev[=<n>]        use <n> digits to display SHA-1s
    --src-prefix <prefix>
                          show the given source prefix instead of "a/"
    --dst-prefix <prefix>
                          show the given source prefix instead of "b/"
    --line-prefix <prefix>
                          prepend an additional prefix to every line of output
    --no-prefix           no not show any source or destination prefix
    --inter-hunk-context <n>
                          show context between diff hunks up to the specified number of lines
    --output-indicator-new <char>
                          specify the character to indicate a new line instead of '+'
    --output-indicator-old <char>
                          specify the character to indicate an old line instead of '-'
    --output-indicator-context <char>
                          specify the character to indicate a context instead of ' '

Diff rename options
    -B, --break-rewrites[=<n>[/<m>]]
                          break complete rewrite changes into pairs of delete and create
    -M, --find-renames[=<n>]
                          detect renames
    -D, --irreversible-delete
                          omit the preimage for deletes
    -C, --find-copies[=<n>]
                          detect copies
    --find-copies-harder  use unmodified files as source to find copies
    --no-renames          disable rename detection
    --rename-empty        use empty blobs as rename source
    --follow              continue listing the history of a file beyond renames
    -l <n>                prevent rename/copy detection if the number of rename/copy targets exceeds given limit

Diff algorithm options
    --minimal             produce the smallest possible diff
    -w, --ignore-all-space
                          ignore whitespace when comparing lines
    -b, --ignore-space-change
                          ignore changes in amount of whitespace
    --ignore-space-at-eol
                          ignore changes in whitespace at EOL
    --ignore-cr-at-eol    ignore carrier-return at the end of line
    --ignore-blank-lines  ignore changes whose lines are all blank
    --indent-heuristic    heuristic to shift diff hunk boundaries for easy reading
    --patience            generate diff using the "patience diff" algorithm
    --histogram           generate diff using the "histogram diff" algorithm
    --diff-algorithm <algorithm>
                          choose a diff algorithm
    --anchored <text>     generate diff using the "anchored diff" algorithm
    --word-diff[=<mode>]  show word diff, using <mode> to delimit changed words
    --word-diff-regex <regex>
                          use <regex> to decide what a word is
    --color-words[=<regex>]
                          equivalent to --word-diff=color --word-diff-regex=<regex>
    --color-moved[=<mode>]
                          move lines of code are colored differently
    --color-moved-ws <mode>
                          how white spaces are ignored in --color-moved

Diff other options
    --relative[=<prefix>]
                          when run from subdir, exclude changes outside and show relative paths
    -a, --text            treat all files as text
    -R                    swap two inputs, reverse the diff
    --exit-code           exit with 1 if there were differences, 0 otherwise
    --quiet               disable all output of the program
    --ext-diff            allow an external diff helper to be executed
    --textconv            run external text conversion filters when comparing binary files
    --ignore-submodules[=<when>]
                          ignore changes to submodules in the diff generation
    --submodule[=<format>]
                          specify how differences in submodules are shown
    --ita-invisible-in-index
                          hide 'git add -N' entries from the index
    --ita-visible-in-index
                          treat 'git add -N' entries as real in the index
    -S <string>           look for differences that change the number of occurrences of the specified string
    -G <regex>            look for differences that change the number of occurrences of the specified regex
    --pickaxe-all         show all changes in the changeset with -S or -G
    --pickaxe-regex       treat <string> in -S as extended POSIX regular expression
    -O <file>             override diff.orderFile configuration variable
    --find-object <object-id>
                          look for differences that change the number of occurrences of the specified object
    --diff-filter [(A|C|D|M|R|T|U|X|B)...[*]]
                          select files by diff type
    --output <file>       Output to a specific file


^ permalink raw reply

* Re: [PATCH] Re: [wishlist] submodule.update config
From: Stefan Beller @ 2018-12-12 19:31 UTC (permalink / raw)
  To: Yaroslav Halchenko; +Cc: git
In-Reply-To: <20181211051032.GQ4633@hopa.kiewit.dartmouth.edu>

> But again, I must confess, that either I forgot or just do not see a
> clear use-case/demand for submodule.update config myself any longer,

ok, let's drop that patch then.

> Probably I need to try "submodules update --merge" to see what is that
> rough edge which makes it different from the potential "merge
> --recurse-submodules", or is it easy to describe? ;-)

I think the branch handling would be the difference. I'd expect
"merge --recurse-submodules" to be sensible about staying on
the branch both in the superproject and submodule, whereas
"submodule update --merge" is too much plumbing, that we'd
expect a sensible branch handling (detached HEAD is just fine,
right?)

The merge result would be the same, I'd think.

>
> I wonder if may be instead of pestering you about this config one, I
> should ask about pointers on how to accomplish "revert
> --recurse-submodules"

What do you want to do in revert --recurse-submodules?
When you have "revert --recurse-submodules $COMMIT",
would that revert all submodule commits introduced in
that commit as well as the regular superproject revert?

This would require either opening multiple editors
(once per submodule and at last for the superproject)
or we'd have to do fancy snip-snapping of the user input,
e.g. providing a template like:

    Revert "$title"

    This reverts commit $COMMIT.

    # The above is for the superproject commit
    # Please enter the commit message  ...
    #
    # Changes to be committed:
    #       ...
    # --8<-- DO NOT DELETE THIS LINE
    # Below is the commit for submodule $submodule:
    Revert $submodule_range

    This reverts commits $maybe_many

    # The above is for the submodule commit
    # Please ...

I guess it may be easier to just have multiple
editors opened sequentially to give a commit
message.

>  or where to poke to make it possible to clone
> recursively from  http://datasets.datalad.org/ where  we do not place
> submodules all under the very top /.git/modules ;-)

Not sure what you mean there?

^ permalink raw reply

* Re: [PATCH] shortlog: pass the mailmap into the revision walker
From: Junio C Hamano @ 2018-12-12 19:19 UTC (permalink / raw)
  To: CB Bailey; +Cc: git
In-Reply-To: <20181212171052.13415-1-cb@hashpling.org>

CB Bailey <cb@hashpling.org> writes:

> From: CB Bailey <cbailey32@bloomberg.net>
>
> shortlog always respects the mailmap in its output. Pass the mailmap
> into the revision walker to allow the mailmap to be used with revision
> limiting options such as '--author'.

I am a bit torn on this.  

If an author of interest has entries in the mailmap, with the same
name but three or more different e-mail addresses, you could do

    $ git shortlog --author=junio@kernel.org --author=junio@twinsun.com

to find commits by the author under these two addresses, and exclude
commits by the same author made under other addresses.  With this
patch applied, it becomes no longer possible, and you can only look
for commits written under all of my identities, or none of them,
which is a minor annoyance.

But more importantly, the user now needs to know which one is the
"canonical" spelling of the author's ident [*1*].  Asking for
commits written by junio@kernel.org will not yield any result with
the patch applied, no?

For the author in the above example, luckily, AUTHOR_NAME is the
same and unique, so without your patch, you could still do

    $ git shortlog --author="Junio C"

to grab all of them, but there may be authors for whom there is no
catch-all substring that would match all of the idents they ever
used but still unique enough to match only that author, and I do
agree that the new feature proposed by the patch in question would
have uses in such a case.

Which nudges me to say that there needs a way to ask for the
original behaviour, disabling the rewriting of commit author
identity before --author filter kicks in.

There may also need a way to ask omitting mailmap processing even at
the output phase.  I do not think it is a particularly interesting
feature to be able to ask for my commits under only two of my
identities [*2*], but as long as that "feature" exists, it also
should be possible to see the result of that "feature" more clearly
by not merging them into a single list.  "shortlog --no-use-mailmap"
may be a way to do so, but unlike "log", the command currently does
not take the "--[no-]use-mailmap" option.


[Footnote]

*1* Passing the query string given as --author=... through the
    mailmap machinery _might_ be a possible ingredient to solve "the
    user needs to know the caonical spelling of the author ident"
    issue this patch has, but I do not think it generally is
    possible (e.g. how would you rewrite --author='.*@kernel.org').

*2* In other words, I think the loss of "find only my @kernel.org
    and @twinsun.com commits" is mere annoyance and people can live
    with it.  But I think "you must look for gitster@pobox.com, and
    looking for junio@kernel.org will not find my commits made under
    that identity" is a show stopper.  That is why I use a very weak
    "may also need" to this "optionally not using mailmap at all"
    feature.

^ permalink raw reply

* RE: High locking contention during repack?
From: Iucha, Florin @ 2018-12-12 19:05 UTC (permalink / raw)
  To: git@vger.kernel.org; +Cc: Ævar Arnfjörð Bjarmason, Jeff King
In-Reply-To: <SN1PR12MB2384587486B6D10F2784CEB795A70@SN1PR12MB2384.namprd12.prod.outlook.com>

Is there an easy way to figure out what object we're processing in create_delta? I have filtered out some large data files in .gitattributes already.

florin

^ permalink raw reply

* [PATCH v2] run-command: report exec failure
From: Junio C Hamano @ 2018-12-12 18:36 UTC (permalink / raw)
  To: git; +Cc: Jeff King, Johannes Schindelin
In-Reply-To: <xmqqsgz4jkgl.fsf@gitster-ct.c.googlers.com>

In 321fd823 ("run-command: mark path lookup errors with ENOENT",
2018-10-24), we rewrote the logic to execute a command by looking
in the directories on $PATH; as a side effect, a request to run a
command that is not found on $PATH is noticed even before a child
process is forked to execute it.

We however stopped to report an exec failure in such a case by
mistake.  Add a logic to report the error unless silent-exec-failure
is requested, to match the original code.

Reported-by: John Passaro <john.a.passaro@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---

 * This time, tests look for the command name in the message, to
   avoid getting affected by the differences the error is reported
   by two codepaths (Windows codepath uses "spawn" while others say
   "run"), which was pointed out by Dscho.

   I am taking that https://travis-ci.org/git/git/jobs/466908193
   that succeeded on Windows as a sign that this is now OK there.

 run-command.c          | 2 ++
 t/t0061-run-command.sh | 9 ++++++---
 2 files changed, 8 insertions(+), 3 deletions(-)

diff --git a/run-command.c b/run-command.c
index d679cc267c..e2bc18a083 100644
--- a/run-command.c
+++ b/run-command.c
@@ -728,6 +728,8 @@ int start_command(struct child_process *cmd)
 	if (prepare_cmd(&argv, cmd) < 0) {
 		failed_errno = errno;
 		cmd->pid = -1;
+		if (!cmd->silent_exec_failure)
+			error_errno("cannot run %s", cmd->argv[0]);
 		goto end_of_spawn;
 	}
 
diff --git a/t/t0061-run-command.sh b/t/t0061-run-command.sh
index b9cfc03a53..8a484878ec 100755
--- a/t/t0061-run-command.sh
+++ b/t/t0061-run-command.sh
@@ -14,11 +14,13 @@ EOF
 >empty
 
 test_expect_success 'start_command reports ENOENT (slash)' '
-	test-tool run-command start-command-ENOENT ./does-not-exist
+	test-tool run-command start-command-ENOENT ./does-not-exist 2>err &&
+	test_i18ngrep "\./does-not-exist" err
 '
 
 test_expect_success 'start_command reports ENOENT (no slash)' '
-	test-tool run-command start-command-ENOENT does-not-exist
+	test-tool run-command start-command-ENOENT does-not-exist 2>err &&
+	test_i18ngrep "does-not-exist" err
 '
 
 test_expect_success 'run_command can run a command' '
@@ -34,7 +36,8 @@ test_expect_success 'run_command is restricted to PATH' '
 	write_script should-not-run <<-\EOF &&
 	echo yikes
 	EOF
-	test_must_fail test-tool run-command run-command should-not-run
+	test_must_fail test-tool run-command run-command should-not-run 2>err &&
+	test_i18ngrep "should-not-run" err
 '
 
 test_expect_success !MINGW 'run_command can run a script without a #! line' '
-- 
2.20.0


^ permalink raw reply related


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