Re: [PATCH] use child_process_init() to initialize struct child_process variables

4 messages, 3 authors, 2016-06-15 · open the first message on its own page

Re: [PATCH] use child_process_init() to initialize struct child_process variables

From: Junio C Hamano <hidden>
Date: 2016-06-15 23:02:50

Jeff King [off-list ref] writes:
I peeked at libgit2 and I think it does not support bundles at all yet,
so that is safe. Grepping for "bundle" in dulwich turns up no hits,
either.

Looks like JGit does support them. I did a very brief test, and it seems
to silently ignore a HEAD ref that has the NUL (I guess maybe it just
rejects it as a malformed refname).

We could make JGit happier either by:

  1. Only including the symref magic in ambiguous cases, so that regular
     ones Just Work as usual.

  2. Including two lines, like:

        $sha1 HEAD\0symref=refs/heads/master
	$sha1 HEAD

     which JGit does the right thing with (and git.git seems to, as
     well).
Sounds sensible, even though it looks ugly X-<.

Re: [PATCH] use child_process_init() to initialize struct child_process variables

From: Philip Oakley <hidden>
Date: 2016-06-15 23:02:51

From: "Junio C Hamano" <redacted>
Sent: Monday, November 03, 2014 11:42 PM
Jeff King [off-list ref] writes:
quoted
I peeked at libgit2 and I think it does not support bundles at all 
yet,
so that is safe. Grepping for "bundle" in dulwich turns up no hits,
either.

Looks like JGit does support them. I did a very brief test, and it 
seems
to silently ignore a HEAD ref that has the NUL (I guess maybe it just
rejects it as a malformed refname).

We could make JGit happier either by:

  1. Only including the symref magic in ambiguous cases, so that 
regular
     ones Just Work as usual.

  2. Including two lines, like:

        $sha1 HEAD\0symref=refs/heads/master
$sha1 HEAD

     which JGit does the right thing with (and git.git seems to, as
     well).
Sounds sensible, even though it looks ugly X-<.
I believe that the 'two HEADs' mechanism would also fall foul of the 
'duplicate refs' warning (untested).

Philip 

Re: [PATCH] use child_process_init() to initialize struct child_process variables

From: Jeff King <hidden>
Date: 2016-06-15 23:02:51

On Wed, Nov 05, 2014 at 01:35:21PM -0000, Philip Oakley wrote:
quoted
quoted
 2. Including two lines, like:
[...]
I believe that the 'two HEADs' mechanism would also fall foul of the
'duplicate refs' warning (untested).
It didn't in my very brief testing of what I posted above, but maybe
there is some other case that triggers it that I didn't exercise.

I grepped through the code and the only "duplicate ref" warning I see
comes from the refs.c code, which comes from commit_packed_refs(). If
the duplicate line is HEAD, I think it shouldn't trigger that, as it is
not a regular ref. That would explain why I didn't see it in my testing.

-Peff

Re: [PATCH] use child_process_init() to initialize struct child_process variables

From: Philip Oakley <hidden>
Date: 2016-06-15 23:02:51

From: "Jeff King" <redacted>
On Wed, Nov 05, 2014 at 01:35:21PM -0000, Philip Oakley wrote:
quoted
quoted
quoted
 2. Including two lines, like:
[...]
I believe that the 'two HEADs' mechanism would also fall foul of the
'duplicate refs' warning (untested).
It didn't in my very brief testing of what I posted above, but maybe
there is some other case that triggers it that I didn't exercise.
I'd been testing the inclusion of a duplicate of the ref that matched 
the HEAD symref (rather than HEAD itself), and had hit that message a 
few times, hence my concern.
I grepped through the code and the only "duplicate ref" warning I see
comes from the refs.c code, which comes from commit_packed_refs().
I had it from is_dup_ref(), also in refs.c, though I may have followed 
the call stack incorrectly back to the bundle effects.
If
the duplicate line is HEAD, I think it shouldn't trigger that, as it 
is
not a regular ref. That would explain why I didn't see it in my 
testing.
I've now also done a test and found the same (no warning/error) when 
there are two HEADs listed in the bundle preamble. Sorry for the 
confusion.

--
Philip
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help