Git development
 help / color / mirror / Atom feed
* Re: [PATCH] push options: fail properly in the stateless case
From: Junio C Hamano @ 2017-02-08 18:11 UTC (permalink / raw)
  To: Stefan Beller; +Cc: git, jrnieder
In-Reply-To: <20170208010954.19478-1-sbeller@google.com>

Stefan Beller <sbeller@google.com> writes:

> When using non-builtin protocols relying on a transport helper
> (such as http), push options are not propagated to the helper.
>
> Fix this by propagating the push options to the transport helper and

The description up to this point is VERY readable and sensible.  But
that makes the title sound a bit strange.  I read it as if it were
saying "stateless case can never support push-options so fail if the
caller attempts to use one", but that does not seem to be what is
going on.

> adding a test that push options using http fail properly.

Sounds sensible.  What end-user visible effect does this fix have?
IOW, what feature do we use "push-option" for?

Ahh, OK, so you need to describe that there are two issues in order
to be understood by the readers:

 (1) the helper protocol does not propagate push-option
 (2) the http helper is not prepared to handle push-option

You fix (1), and you take advantage of the fact (2) to ensure that
(1) is fixed in the new test.

With such an understanding, the title makes (sort of) sense and you
wouldn't have to be asked "what end-user visible effect/benefit does
this have?"

> +'option push-option <c-string>::
> +	Transmit this push option.
> +

There is no "c-string" in the current documentation used or
defined.  The closest thing I found is

    ... that field will be quoted in the manner of a C string ...

in git-status page, but I do not think you send the value for an
push-option after running quote_c_style(), so I am puzzled.

I'd rather see 'option push-option <string>' as the bullet item, and
in its description say how arbitrary values (if you allow them, that
is) can be used, e.g. "Transmit <string> encoded in such and such
way a the value of the push-option".

^ permalink raw reply

* Re: "disabling bitmap writing, as some objects are not being packed"?
From: Junio C Hamano @ 2017-02-08 17:44 UTC (permalink / raw)
  To: Duy Nguyen; +Cc: David Turner, Git Mailing List, Jeff King
In-Reply-To: <CACsJy8C4DO-GYREUhED3YU_WetoTZaB3MUq1kGfRjA3e-FOLYQ@mail.gmail.com>

Duy Nguyen <pclouds@gmail.com> writes:

> On second thought, perhaps gc.autoDetach should default to false if
> there's no tty, since its main point it to stop breaking interactive
> usage. That would make the server side happy (no tty there).

Sounds like an idea, but wouldn't that keep the end-user coming over
the network waiting after accepting a push until the GC completes, I
wonder.  If an impatient user disconnects, would that end up killing
an ongoing GC?  etc.


^ permalink raw reply

* Re: Fwd: Possibly nicer pathspec syntax?
From: Junio C Hamano @ 2017-02-08 17:39 UTC (permalink / raw)
  To: Duy Nguyen; +Cc: Linus Torvalds, Git Mailing List
In-Reply-To: <CACsJy8AQmg+oRYATU8_gR6zY-=sPN3m9PKtk-kytkSKGK+GG1g@mail.gmail.com>

Duy Nguyen <pclouds@gmail.com> writes:

> On Wed, Feb 8, 2017 at 12:12 PM, Linus Torvalds
> <torvalds@linux-foundation.org> wrote:
>> Two-patch series to follow.
>
> glossary-content.txt update for both patches would be nice.

I am no longer worried about it as I saw somebody actually sent
follow-up patches on this, but I want to pick your brain on one
thing that is related to this codepath.

We have PATHSPEC_PREFER_CWD and PATHSPEC_PREFER_FULL bits in flags,
added at fc12261fea ("parse_pathspec: add PATHSPEC_PREFER_{CWD,FULL}
flags", 2013-07-14), and I think the intent is some commands when
given no pathspec work on all paths in the current subdirectory
while others work on the full tree, regardless of where you are.
"grep" is in the former camp, "log" is in the latter.  And there is
a check to catch a bug in a caller that sets both.

I am wondering about this hunk (this is from the original commit
that added it):

 	if (!entry) {
 		static const char *raw[2];
 
+		if (flags & PATHSPEC_PREFER_FULL)
+			return;
+
+		if (!(flags & PATHSPEC_PREFER_CWD))
+			die("BUG: PATHSPEC_PREFER_CWD requires arguments");
+
 		pathspec->items = item = xmalloc(sizeof(*item));
 		memset(item, 0, sizeof(*item));
 		item->match = prefix;
		... returns a single entry pathspec to cover cwd ...

The BUG message is given when 

 - The command got no pathspec from the caller; and
 - PATHSPEC_PREFER_FULL is not set; and
 - PATHSPEC_PREFER_CWD is NOT set.

but the message says that the caller must have args when it sets
prefer-cwd.  Is this a simple typo?  If so what should it say?

	die("BUG: one of PATHSPEC_PREFER_FULL or _CWD must be set");

If that were the case, we are expressing only one bit of information
(do we limit to cwd, or do we work on full-tree?), but there must
have been a reason why we added two bits and made them mutually
incompatible so that we can express three possibilities.

Does this third possibility (i.e. a caller is allowed to pass
"flags" that does not prefer either) exist to support a command
where the caller MUST have at least one pathspec?  If that were the
case, this wouldn't be a BUG but an end-user error, e.g.

	die("at least one pathspec element is required");

If you know offhand which callers pass neither of the two
PATHSPEC_PREFER_* bits and remember for what purpose you allowed
them to do so, please remind me.  I'll keep digging in the meantime.

Thanks.

^ permalink raw reply

* Re: Trying to use xfuncname without success.
From: Jack Adrian Zappa @ 2017-02-08 17:11 UTC (permalink / raw)
  To: René Scharfe; +Cc: git-mailing-list
In-Reply-To: <271989d5-c383-0c0d-bfcb-f4118f9fa2aa@web.de>

Thanks Rene, but you seem to have missed the point.  NOTHING is
working.  No matter what I put there, it doesn't seem to get matched.

Just to be sure, I tested your regex and again it didn't work.

Someone on the SO site stated they could get it to work on FreeBSD and
I'm on Windows, so this might be a platform thing.  Can anyone else on
Windows please confirm?

Thanks,


A

On Tue, Feb 7, 2017 at 6:18 PM, René Scharfe <l.s.r@web.de> wrote:
> Am 07.02.2017 um 20:21 schrieb Jack Adrian Zappa:
>>
>> I'm trying to setup a hunk header for .natvis files. For some reason,
>> it doesn't seem to be working. I'm following their instructions from
>> here, which doesn't say much in terms of restrictions of the regex,
>> such as, is the matched item considered the hunk header or do I need a
>> group? I have tried both with no success. This is what I have:
>>
>> [diff "natvis"]
>>     xfuncname = "^[\\\t ]*<Type[\\\t ]+Name=\"([^\"])\".*$"
>
>
> The extra "\\" allow backslashes to be used for indentation as well as
> between Type and Name, which is probably not what you want.  And your
> expression only matches single-char Name attributes.  Try:
>
>         xfuncname = "^[\t ]*<Type[\t ]+Name=\"([^\"]+)\".*$"
>
> René

^ permalink raw reply

* Re: init --separate-git-dir does not set core.worktree
From: Stefan Beller @ 2017-02-08 16:13 UTC (permalink / raw)
  To: Duy Nguyen; +Cc: Kyle Meyer, Git Mailing List
In-Reply-To: <CACsJy8C=owNPpND4Ab7bFE24kpWBr5fQdob21DEDCckCXu0Mng@mail.gmail.com>

On Fri, Feb 3, 2017 at 5:38 AM, Duy Nguyen <pclouds@gmail.com> wrote:
> On Thu, Feb 2, 2017 at 7:37 PM, Kyle Meyer <kyle@kyleam.com> wrote:
>> Duy Nguyen <pclouds@gmail.com> writes:
>>
>>> On Thu, Feb 2, 2017 at 10:55 AM, Kyle Meyer <kyle@kyleam.com> wrote:
>>>>
>>>> As of 6311cfaf9 (init: do not set unnecessary core.worktree,
>>>> 2016-09-25), "git init --separate-git-dir" no longer sets core.worktree
>>>> (test below).  Based on the commit message and the corresponding thread
>>>> [1], I don't think this change in behavior was intentional, but I wasn't
>>>> able to understand things well enough to attempt a patch.
>>>
>>> I'm missing some context. Why does --separate-git-dir have to set
>>> core.worktree? What fails for you exactly?
>>
>> Sorry for not providing enough information.  I haven't run into a
>> failure.
>>
>> In Magit, we need to determine the top-level of the working tree from
>> COMMIT_EDITMSG.  Right now that logic [*1*] looks something like this:
>
> This is much better :)
>
>>  * COMMIT_EDITMSG in .git/modules/<module>/: set working tree to the
>>    output of "git rev-parse --show-toplevel"

.git/modules/<module>/ itself is a fully fledged git directory, making use of
the core.worktree variable to have the working tree at an "unusual" place.

"git rev-parse --show-toplevel" is the correct place of the working tree

>>
>>  * COMMIT_EDITMSG in .git/worktrees/<wtree>/: set working tree to the
>>    path in .git/worktrees/<wtree>/gitdir, minus the trailing "/.git"

(in worktree/<wtree>:) "git rev-parse --show-toplevel"s output is empty,
but the gitdir file exists unlike in the other cases.

>>
>>  * COMMIT_EDITMSG in .git: set working tree to the parent directory

(in .git:) "git rev-parse --show-toplevel"s output is empty.

>>
>> This fails for a repo set up with --separate-git-dir [*2*], where the
>> last step will go out into an unrelated repo.  If core.worktree was set
>> and "git rev-parse --show-toplevel" returned the working tree like it
>> did for submodules, things would work.

When using --separate-git-dir you cannot tell where the working tree is from
within the git dir, despite knowing it has to be *somewhere*:

    [core]
        bare = false

So I would hope we'll set core.worktree again in this case. Otherwise how
would we discover the working tree?

>
> OK. If I read this right, given a path of any text file somewhere
> within ".git" directory. you are tasked to find out where the
> associated worktree is? I.e. this is not an emacsclient executed as
> part of "git commit", correct?

I always assumed core.worktree is precisely that analogy, i.e.
core.worktree is the backwards pointer to the .git file (which
is true coming from a submodule background).

>
> So you need some sort of back-link to ".git" location. And
> unfortunately there's no such thing for .git file (unless it points to
> .git/worktrees/...). I'm hesitant to set core.worktree unless it's
> absolutely needed since it may have unexpected interaction with
> $GIT_WORK_TREE and others (not sure if it has any interaction with
> submodules too). I think adding a new file "gitdir" would be a safer
> option.

How would "gitdir" (should it be called worktree/workingtree instead?)
work together with core.worktree set?
Would it point at the .git file or the root level of the working tree?

>
> I'm not entirely sure if enforcing "one worktree - one repository" is
> safe though. The first two bullet points are very specific and we can
> assume that, but ".git" files alone can be used for anything. In
> theory you can always create a secondary worktree (that's not managed
> by "git worktree") by setting GIT_WORK_TREE and "git checkout -f"
> somewhere. But I guess those would be temporary and nobody would want
> magic to point back to them.
>
> As a fall-back mechanism, I think after magit has found the worktree,
> it should verify the found location is the correct worktree, with "git
> rev-parse --git-dir" or something, and alert the user otherwise. I
> think "git rev-parse --git-path COMMIT_MSG" should give back the same
> COMMIT_MSG path (and it applies for any files in .git dir, covering
> all three cases). The user could add some magit-specific files to tell
> magit where the actual worktree is when they hit corner cases.
>
> If the use case is limited to editing COMMIT_EDITMSG only (after it's
> generated by git), it may be best to add `pwd` as a comment to that
> file. You won't have to go through all the magic rules to find it out
> (*). And it helps non-magic users too.
>
> (*) well, you do, because you probably can't expect everybody to have
> latest git version.
>
>> Of course, the issue above isn't a reason that --separate-git-dir should
>> set core.worktree, but the submodule behavior is why we were wondering
>> if it should.
>
> I'm not a submodule person, so I'll pass that "why" question to Stefan.

Good question. As said above I always assumed it is the backlink to know where
the working tree of the submodule is.

Digging through history I found d75219b4a, which explains why that is:

    Since recently a submodule with name <name> has its git directory in the
    .git/modules/<name> directory of the superproject while the work tree
    contains a gitfile pointing there. To make that work the git directory has
    the core.worktree configuration set in its config file to point back to
    the work tree.

I remember reading this some time ago and wondered what the
"to make that work" implies though and IIRC there was nothing I found by
experimentation.

^ permalink raw reply

* Re: init --separate-git-dir does not set core.worktree
From: Stefan Beller @ 2017-02-08 16:14 UTC (permalink / raw)
  To: Kyle Meyer; +Cc: Duy Nguyen, Git Mailing List
In-Reply-To: <877f55r0mk.fsf@kyleam.com>

> [*] https://github.com/magit/magit/issues/460#issuecomment-36035787.

I would agree with the thinking in that issue.

^ permalink raw reply

* Re: [PATCH/RFC] WIP: log: allow "-" as a short-hand for "previous branch"
From: Matthieu Moy @ 2017-02-08 14:40 UTC (permalink / raw)
  To: Siddharth Kannan
  Cc: Junio C Hamano, git, pranit.bauva, peff, pclouds, sandals
In-Reply-To: <20170207191450.GA5569@ubuntu-512mb-blr1-01.localdomain>

Siddharth Kannan <kannan.siddharth12@gmail.com> writes:

> Making a change in sha1_name.c will touch a lot of commands
> (setup_revisions is called from everywhere in the codebase), so, I am
> still trying to figure out how to do this such that the rest of the
> codepath remains unchanged.

Changing sha1_name.c is the way to go *if* we want all commands to
support this. Just like other ways to name a revision...

> I hope that you do not mind this side-effect, but rather, you intended
> for this to happen, right? More commands will start supporting this
> shorthand, suddenly.  (such as format-patch, whatchanged, diff to name
> a very few).

... but: the initial implementation of this '-' shorthand was
special-casing a single command (IIRC, "git checkout") for which the
shorthand was useful.

In a previous discussion, I made an analogy with "cd -" (which is the
source of inspiration of this shorthand AFAIK): "-" did not magically
become "the last visited directory" for all Unix commands, just for
"cd". And in this case, I'm happy with it. For example, I never need
"mkdir -", and I'm happy I can't "rm -fr -" by mistake.

So, it's debatable whether it's a good thing to have all commands
support "-". For example, forcing users to explicitly type "git branch
-d @{1}" and not providing them with a shortcut might be a good thing.

I don't have strong opinion on this: I tend to favor consistency and
supporting "-" everywhere goes in this direction, but I think the
downsides should be considered too. A large part of the exercice here is
to write a good commit message!

Another issue with this is: - is also a common way to say "use stdin
instead of a file", so before enabling - for "previous branch", we need
to make sure it does not introduce any ambiguity. Git does not seem to
use "- for stdin" much (most commands able to read from stdin have an
explicit --stdin option for that), a quick grep in the docs shows only
"git blame --contents -" which is OK because a revision wouldn't make
sense here anyway.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/

^ permalink raw reply

* Re: GSoC 2017: application open, deadline = February 9, 2017
From: Jeff King @ 2017-02-08 15:43 UTC (permalink / raw)
  To: Matthieu Moy
  Cc: git, Pranit Bauva, Lars Schneider, Christian Couder,
	Carlos Martín Nieto, Johannes Schindelin, Thomas Gummerer,
	Siddharth Kannan
In-Reply-To: <vpq37fowx5q.fsf@anie.imag.fr>

On Wed, Feb 08, 2017 at 03:54:25PM +0100, Matthieu Moy wrote:

> >> * We need to write the application, i.e. essentially polish and update
> >>   the text here: https://git.github.io/SoC-2016-Org-Application/ and
> >>   update the list of project ideas and microprojects :
> >>   https://git.github.io/SoC-2017-Ideas/
> >>   https://git.github.io/SoC-2016-Microprojects/
> >
> > That can be done incrementally by people who care (especially mentors)
> > over the next week or so, and doesn't require any real admin
> > coordination. If it happens and the result looks good, then the
> > application process is pretty straightforward.
> >
> > If it doesn't, then we probably ought not to participate in GSoC.
> 
> OK, it seems the last message did not raise a lot of enthousiasm (unless
> I missed some off-list discussion at Git-Merge?).

Nope, there was no discussion that I'm aware of.

> The application deadline is tomorrow. I think it's time to admit that we
> won't participate this year, unless someone steps in really soon.

Yes, I'd agree with that.

Outreachy folks asked if we were interested in participating, but I
think it has roughly the same pre-requisite lists for ideas and
microprojects.

> If we don't participate, I'll add a disclaimer at the top of the
> SoC-related pages on git.github.io to make sure students don't waste
> time preparing an application.

Good idea.

-Peff

^ permalink raw reply

* Re: [PATCH 1/5] add SWAP macro
From: Johannes Schindelin @ 2017-02-08 15:14 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: René Scharfe, Jeff King, Brandon Williams, Johannes Sixt,
	Git List
In-Reply-To: <xmqqr339y6q3.fsf@gitster.mtv.corp.google.com>

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

Hi Junio & René,

On Tue, 7 Feb 2017, Junio C Hamano wrote:

> René Scharfe <l.s.r@web.de> writes:
> 
> > Swapping between different types would then still have to be done
> > manually, but I wonder how common that is -- couldn't find such a case
> > in our tree.
> 
> I do not think it is a common thing to do, and more importantly, I doubt
> we want to hide such a swap inside a macro.  And that is why I said the
> seemingly extra "type" thing may be an improvement over your original
> SWAP() thing if it gives us more type safety.
> 
> It seems that the thread has been quite for a while. Perhaps people are
> happy enough with your patches?  If so, let's move it forward, but I'll
> wait for a while in case follow-up discussion appears soonish.  The
> changes are fairly well isolated and I do not think we are in a hurry.

I am still unhappy about choosing to complicate things and lean heavily on
the compiler to make things right again.

But it appears that I am the only one with this concern, so go ahead. I
promise not to say "I told you so" in case that it breaks things or that
certain platforms are experiencing a disadvantage.

Ciao,
Johannes

^ permalink raw reply

* Re: [PATCH] difftool: fix bug when printing usage
From: Johannes Schindelin @ 2017-02-08 15:06 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: David Aguilar, Git ML, Denton Liu
In-Reply-To: <xmqqd1etzrxj.fsf@gitster.mtv.corp.google.com>

Hi Junio,

On Tue, 7 Feb 2017, Junio C Hamano wrote:

> Junio C Hamano <gitster@pobox.com> writes:
> 
> > Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> >
> >>> > Likewise, this would become
> >>> >
> >>> > 	GIT_CEILING_DIRECTORIES="$PWD/not" \
> >>> > 	test_expect_code 129 git -C not/repo difftool -h >output &&
> >>> > 	grep ^usage: output
> >>> 
> >>> I agree with the intent, but the execution here is "Not quite".
> >>> test_expect_code being a shell function, it does not take the
> >>> "one-shot environment assignment for this single invocation," like
> >>> external commands do.
> >>
> >> So now that we know what is wrong, can you please enlighten me about
> >> what is right?
> >
> > David's original is just fine, isn't it?
> 
> I've also seen people use "env VAR=VAL git command" as the command to be
> tested in t/ scripts.  You can run that under test_expect_code,
> methinks.

That is exactly what David ended up sending out as follow-up patches.

I did not mean to be critical, I just found it to be more helpful to
accompany "that does not work" comments with "but this does" in the past.

Ciao,
Johannes

^ permalink raw reply

* Re: GSoC 2017: application open, deadline = February 9, 2017
From: Matthieu Moy @ 2017-02-08 14:54 UTC (permalink / raw)
  To: Jeff King
  Cc: git, Pranit Bauva, Lars Schneider, Christian Couder,
	Carlos Martín Nieto, Johannes Schindelin, Thomas Gummerer,
	Siddharth Kannan
In-Reply-To: <20170125204504.ebw2sa4uokfwwfnt@sigill.intra.peff.net>

Jeff King <peff@peff.net> writes:

> On Mon, Jan 23, 2017 at 04:02:02PM +0100, Matthieu Moy wrote:
>
>> * We need to write the application, i.e. essentially polish and update
>>   the text here: https://git.github.io/SoC-2016-Org-Application/ and
>>   update the list of project ideas and microprojects :
>>   https://git.github.io/SoC-2017-Ideas/
>>   https://git.github.io/SoC-2016-Microprojects/
>
> That can be done incrementally by people who care (especially mentors)
> over the next week or so, and doesn't require any real admin
> coordination. If it happens and the result looks good, then the
> application process is pretty straightforward.
>
> If it doesn't, then we probably ought not to participate in GSoC.

OK, it seems the last message did not raise a lot of enthousiasm (unless
I missed some off-list discussion at Git-Merge?).

The application deadline is tomorrow. I think it's time to admit that we
won't participate this year, unless someone steps in really soon.

If we don't participate, I'll add a disclaimer at the top of the
SoC-related pages on git.github.io to make sure students don't waste
time preparing an application.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/

^ permalink raw reply

* Re: Git clonebundles
From: Johannes Schindelin @ 2017-02-08 14:28 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Christian Couder, Shawn Pearce, Stefan Saasen, Git Mailing List
In-Reply-To: <xmqqzihxyb66.fsf@gitster.mtv.corp.google.com>

Hi Junio,

On Tue, 7 Feb 2017, Junio C Hamano wrote:

> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> 
> >> If people think it might be useful to have it around to experiment, I
> >> can resurrect and keep that in 'pu' (or rather 'jch'), as long as it
> >> does not overlap and conflict with other topics in flight.  Let me try
> >> that in today's integration cycle.
> >
> > I would like to remind you of my suggestion to make this more publicly
> > visible and substantially easier to play with, by adding it as an
> > experimental feature (possibly guarded via an explicit opt-in config
> > setting).
> 
> I do not understand why you want to give this topic undue prominence
> ovver any other random topic that cook in 'pu' [...]

Since you ask so nicely for an explanation: clonebundles got a really
lively and active discussion at the Contributors' Summit. So it is not
your run of the mill typo fix, the bundle issue is something that clearly
receives a lot of interest in particular from developers who are
unfamiliar with the idiosynchracies of the code Git development.

And I got the very distinct impression that Git would benefit a lot from
these developers, *in particular* since they come with fresh perspectives.

Now, we can make it hard for them (e.g. expecting them to sift through a
few months' worth of What's Cooking mails, to find out whether there has
been any related work, and what is the branch name, if any, and where to
find that branch), and we can alternatively make it easy for them to help
us make Git better.

I would like us to choose the easier route for them. Because it would
benefit us.

Ciao,
Johannes

^ permalink raw reply

* Re: [PATCH 1/2] pathspec magic: add '^' as alias for '!'
From: Cornelius Weig @ 2017-02-08 13:23 UTC (permalink / raw)
  To: Linus Torvalds, Junio C Hamano; +Cc: Git Mailing List
In-Reply-To: <alpine.LFD.2.20.1702072113040.25002@i7.lan>

As Duy pointed out, the glossary needs an update too.

For this one, the cange can be minimal I think:

diff --git a/Documentation/glossary-content.txt b/Documentation/glossary-content.txt
index 8ad29e6..f127fe9 100644
--- a/Documentation/glossary-content.txt
+++ b/Documentation/glossary-content.txt
@@ -386,7 +386,7 @@ Glob magic is incompatible with literal magic.
 
 exclude;;
        After a path matches any non-exclude pathspec, it will be run
-       through all exclude pathspec (magic signature: `!`). If it
+       through all exclude pathspec (magic signature: `!` or `^`). If it
        matches, the path is ignored.
 --
 

^ permalink raw reply related

* Re: [PATCH 2/2] pathspec: don't error out on all-exclusionary pathspec patterns
From: Cornelius Weig @ 2017-02-08 13:23 UTC (permalink / raw)
  To: Linus Torvalds, Junio C Hamano; +Cc: Git Mailing List
In-Reply-To: <alpine.LFD.2.20.1702072113380.25002@i7.lan>

Again, as Duy pointed out this should be documented.

How about something like this:

diff --git a/Documentation/glossary-content.txt b/Documentation/glossary-content.txt
index f127fe9..781cde3 100644
--- a/Documentation/glossary-content.txt
+++ b/Documentation/glossary-content.txt
@@ -387,7 +387,9 @@ Glob magic is incompatible with literal magic.
 exclude;;
        After a path matches any non-exclude pathspec, it will be run
        through all exclude pathspec (magic signature: `!` or `^`). If it
-       matches, the path is ignored.
+       matches, the path is ignored. If only exclude pathspec are given,
+       the exclusion is applied to the result set as if invoked without any
+       pathspec.
 --
 
 [[def_parent]]parent::

^ permalink raw reply related

* Re: [PATCH 0/2] Demonstrate a critical worktree/gc bug
From: Duy Nguyen @ 2017-02-08 12:28 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: Git Mailing List, Junio C Hamano
In-Reply-To: <cover.1486556262.git.johannes.schindelin@gmx.de>

On Wed, Feb 8, 2017 at 7:17 PM, Johannes Schindelin
<johannes.schindelin@gmx.de> wrote:
> ... and a half-working workaround for the auto-gc case.
>
> This patch series really is just another attempt to prod Duy into fixing
> this instead of dabbling with shiny new toys ;-)

FYI work is ongoing [1] [2]. If you want it even faster, go do it yourself.

[1] https://github.com/pclouds/git/commits/prune-in-worktrees-2
[2] https://public-inbox.org/git/20170208113144.8201-1-pclouds@gmail.com/
-- 
Duy

^ permalink raw reply

* [PATCH 2/2] worktree.c: use submodule interface to access refs from another worktree
From: Nguyễn Thái Ngọc Duy @ 2017-02-08 11:31 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Michael Haggerty,
	Nguyễn Thái Ngọc Duy
In-Reply-To: <20170208113144.8201-1-pclouds@gmail.com>

The patch itself is relatively simple: manual parsing code is replaced
with a call to resolve_ref_submodule(). The manual parsing code must die
because only refs/files-backend.c should do that. Why submodule here is
a more interesting question.

From an outside look, any .git/worktrees/foo is seen as a "normal"
repository. You can set GIT_DIR to it and have access to everything,
even shared things that are not literally inside that directory, like
object db or shared refs.

On top of that, linked worktrees point to those directories with ".git"
files. These two make a linked worktree's path "X" a "submodule" (*) (**)
because X/.git is a file that points to a repository somewhere.

As such, we can just linked worktree's path as a submodule. We just need
to make sure they are unique because they are used to lookup submodule
refs store.

Main worktree is a a bit trickier. If we stand at a linked worktree, we
may still need to peek into main worktree's HEAD, for example. We can
treat main worktree's path as submodule as well since git_path_submodule()
can tolerate ".git" dirs, in addition to ".git" files.

The constraint is, if main worktree is X, then the git repo must be at
X/.git. If the user separates .git repo far away and tell git to point
to it via GIT_DIR or something else, then the "main worktree as submodule"
trick fails. Within multiple worktree context, I think we can limit
support to "standard" layout, at least for now.

(*) The differences in sharing object database and refs between
submodules and linked worktrees don't really matter in this context.

(**) At this point, we may want to rename refs *_submodule API to
something more neutral, maybe s/_submodule/_remote/

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 branch.c   |  3 +-
 worktree.c | 99 +++++++++++++++-----------------------------------------------
 worktree.h |  2 +-
 3 files changed, 27 insertions(+), 77 deletions(-)

diff --git a/branch.c b/branch.c
index b955d4f316..db5843718f 100644
--- a/branch.c
+++ b/branch.c
@@ -354,7 +354,8 @@ int replace_each_worktree_head_symref(const char *oldref, const char *newref)
 	for (i = 0; worktrees[i]; i++) {
 		if (worktrees[i]->is_detached)
 			continue;
-		if (strcmp(oldref, worktrees[i]->head_ref))
+		if (worktrees[i]->head_ref &&
+		    strcmp(oldref, worktrees[i]->head_ref))
 			continue;
 
 		if (set_worktree_head_symref(get_worktree_git_dir(worktrees[i]),
diff --git a/worktree.c b/worktree.c
index d633761575..25e5bc9a3e 100644
--- a/worktree.c
+++ b/worktree.c
@@ -19,54 +19,24 @@ void free_worktrees(struct worktree **worktrees)
 	free (worktrees);
 }
 
-/*
- * read 'path_to_ref' into 'ref'.  Also if is_detached is not NULL,
- * set is_detached to 1 (0) if the ref is detached (is not detached).
- *
- * $GIT_COMMON_DIR/$symref (e.g. HEAD) is practically outside $GIT_DIR so
- * for linked worktrees, `resolve_ref_unsafe()` won't work (it uses
- * git_path). Parse the ref ourselves.
- *
- * return -1 if the ref is not a proper ref, 0 otherwise (success)
- */
-static int parse_ref(char *path_to_ref, struct strbuf *ref, int *is_detached)
-{
-	if (is_detached)
-		*is_detached = 0;
-	if (!strbuf_readlink(ref, path_to_ref, 0)) {
-		/* HEAD is symbolic link */
-		if (!starts_with(ref->buf, "refs/") ||
-				check_refname_format(ref->buf, 0))
-			return -1;
-	} else if (strbuf_read_file(ref, path_to_ref, 0) >= 0) {
-		/* textual symref or detached */
-		if (!starts_with(ref->buf, "ref:")) {
-			if (is_detached)
-				*is_detached = 1;
-		} else {
-			strbuf_remove(ref, 0, strlen("ref:"));
-			strbuf_trim(ref);
-			if (check_refname_format(ref->buf, 0))
-				return -1;
-		}
-	} else
-		return -1;
-	return 0;
-}
-
 /**
- * Add the head_sha1 and head_ref (if not detached) to the given worktree
+ * Update head_sha1, head_ref and is_detached of the given worktree
  */
-static void add_head_info(struct strbuf *head_ref, struct worktree *worktree)
+static void add_head_info(struct worktree *wt)
 {
-	if (head_ref->len) {
-		if (worktree->is_detached) {
-			get_sha1_hex(head_ref->buf, worktree->head_sha1);
-		} else {
-			resolve_ref_unsafe(head_ref->buf, 0, worktree->head_sha1, NULL);
-			worktree->head_ref = strbuf_detach(head_ref, NULL);
-		}
-	}
+	int flags;
+	const char *target;
+
+	target = resolve_ref_submodule(wt->path, "HEAD",
+				       RESOLVE_REF_READING,
+				       wt->head_sha1, &flags);
+	if (!target)
+		return;
+
+	if (flags & REF_ISSYMREF)
+		wt->head_ref = xstrdup(target);
+	else
+		wt->is_detached = 1;
 }
 
 /**
@@ -77,9 +47,7 @@ static struct worktree *get_main_worktree(void)
 	struct worktree *worktree = NULL;
 	struct strbuf path = STRBUF_INIT;
 	struct strbuf worktree_path = STRBUF_INIT;
-	struct strbuf head_ref = STRBUF_INIT;
 	int is_bare = 0;
-	int is_detached = 0;
 
 	strbuf_add_absolute_path(&worktree_path, get_git_common_dir());
 	is_bare = !strbuf_strip_suffix(&worktree_path, "/.git");
@@ -91,13 +59,10 @@ static struct worktree *get_main_worktree(void)
 	worktree = xcalloc(1, sizeof(*worktree));
 	worktree->path = strbuf_detach(&worktree_path, NULL);
 	worktree->is_bare = is_bare;
-	worktree->is_detached = is_detached;
-	if (!parse_ref(path.buf, &head_ref, &is_detached))
-		add_head_info(&head_ref, worktree);
+	add_head_info(worktree);
 
 	strbuf_release(&path);
 	strbuf_release(&worktree_path);
-	strbuf_release(&head_ref);
 	return worktree;
 }
 
@@ -106,8 +71,6 @@ static struct worktree *get_linked_worktree(const char *id)
 	struct worktree *worktree = NULL;
 	struct strbuf path = STRBUF_INIT;
 	struct strbuf worktree_path = STRBUF_INIT;
-	struct strbuf head_ref = STRBUF_INIT;
-	int is_detached = 0;
 
 	if (!id)
 		die("Missing linked worktree name");
@@ -127,19 +90,14 @@ static struct worktree *get_linked_worktree(const char *id)
 	strbuf_reset(&path);
 	strbuf_addf(&path, "%s/worktrees/%s/HEAD", get_git_common_dir(), id);
 
-	if (parse_ref(path.buf, &head_ref, &is_detached) < 0)
-		goto done;
-
 	worktree = xcalloc(1, sizeof(*worktree));
 	worktree->path = strbuf_detach(&worktree_path, NULL);
 	worktree->id = xstrdup(id);
-	worktree->is_detached = is_detached;
-	add_head_info(&head_ref, worktree);
+	add_head_info(worktree);
 
 done:
 	strbuf_release(&path);
 	strbuf_release(&worktree_path);
-	strbuf_release(&head_ref);
 	return worktree;
 }
 
@@ -334,8 +292,6 @@ const struct worktree *find_shared_symref(const char *symref,
 					  const char *target)
 {
 	const struct worktree *existing = NULL;
-	struct strbuf path = STRBUF_INIT;
-	struct strbuf sb = STRBUF_INIT;
 	static struct worktree **worktrees;
 	int i = 0;
 
@@ -345,6 +301,10 @@ const struct worktree *find_shared_symref(const char *symref,
 
 	for (i = 0; worktrees[i]; i++) {
 		struct worktree *wt = worktrees[i];
+		const char *symref_target;
+		unsigned char sha1[20];
+		int flags;
+
 		if (wt->is_bare)
 			continue;
 
@@ -359,25 +319,14 @@ const struct worktree *find_shared_symref(const char *symref,
 			}
 		}
 
-		strbuf_reset(&path);
-		strbuf_reset(&sb);
-		strbuf_addf(&path, "%s/%s",
-			    get_worktree_git_dir(wt),
-			    symref);
-
-		if (parse_ref(path.buf, &sb, NULL)) {
-			continue;
-		}
-
-		if (!strcmp(sb.buf, target)) {
+		symref_target = resolve_ref_submodule(wt->path, symref, 0,
+						      sha1, &flags);
+		if ((flags & REF_ISSYMREF) && !strcmp(symref_target, target)) {
 			existing = wt;
 			break;
 		}
 	}
 
-	strbuf_release(&path);
-	strbuf_release(&sb);
-
 	return existing;
 }
 
diff --git a/worktree.h b/worktree.h
index 6bfb985203..5ea5e503fb 100644
--- a/worktree.h
+++ b/worktree.h
@@ -4,7 +4,7 @@
 struct worktree {
 	char *path;
 	char *id;
-	char *head_ref;
+	char *head_ref;		/* NULL if HEAD is broken or detached */
 	char *lock_reason;	/* internal use */
 	unsigned char head_sha1[20];
 	int is_detached;
-- 
2.11.0.157.gd943d85


^ permalink raw reply related

* [PATCH 2/2] worktree: demonstrate a half-working workaround for auto-gc
From: Johannes Schindelin @ 2017-02-08 12:18 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Nguyễn Thái Ngọc Duy
In-Reply-To: <cover.1486556262.git.johannes.schindelin@gmx.de>

While worktrees are marked "experimental", there is really no alternative
to using them when trying to work on multiple patch series in parallel.

Sadly, there is still a bug that causes irretrievable data loss caused by
worktree's incomplete handling of reflogs (and of the multiple indices of
the various worktrees), as this developer can testify a dozen times over.

In the --auto case, we can install a hook that runs before-hand,
accumulates all the worktree-specific refs and reflogs and installs them
into a very special reflog in the common refs namespace. The only
purpose of this stunt is to let gc pick up on those refs and reflogs, of
course, and *not* ignore them.

Unfortunately, this still does not address the "git gc" case, but
hopefully a real fix will be here some time soon.

Also, if there are simply too many loose objects for Git's liking, it will
kick off auto-gc (including the hook, which takes a couple of seconds to
run) that subsequently only says that it won't do anything, so the next
Git operation will kick off another auto-gc (including the multi-second
hook run).

Needless to say: this patch is not actually intended for inclusion, but
instead tries to demonstrate the pain and the dear need for a real bug fix
(and not another 8 months of buggy Git).

So here is hoping to a timely fix. Cheers.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 t/t6500-gc.sh | 26 +++++++++++++++++++++++++-
 1 file changed, 25 insertions(+), 1 deletion(-)

diff --git a/t/t6500-gc.sh b/t/t6500-gc.sh
index e24a4fb611..2f1e52825d 100755
--- a/t/t6500-gc.sh
+++ b/t/t6500-gc.sh
@@ -67,6 +67,30 @@ test_expect_success 'auto gc with too many loose objects does not attempt to cre
 	test_line_count = 2 new # There is one new pack and its .idx
 '
 
+test_expect_success 'install pre-auto-gc hook for worktrees' '
+	mkdir -p .git/hooks &&
+	write_script .git/hooks/pre-auto-gc <<-\EOF
+	echo "Preserving refs/reflogs of worktrees" >&2 &&
+	dir="$(git rev-parse --git-common-dir)" &&
+	refsdir="$dir/logs/refs" &&
+	rm -f "$refsdir"/preserve &&
+	ident="$(GIT_COMMITTER_DATE= git var GIT_COMMITTER_IDENT)" &&
+	(
+		find "$dir"/logs "$dir"/worktrees/*/logs \
+			-type f -exec cat {} \; |
+		cut -d" " -f1
+		find "$dir"/HEAD "$dir"/worktrees/*/HEAD "$dir"/refs \
+			"$dir"/worktrees/*/refs -type f -exec cat {} \; |
+		grep -v "^ref:"
+	) 2>/dev/null |
+	sort |
+	uniq |
+	sed "s/.*/& & $ident	dummy/" >"$dir"/preserve &&
+	mkdir -p "$refsdir" &&
+	mv "$dir"/preserve "$refsdir"/
+	EOF
+'
+
 trigger_auto_gc () {
 	# This is unfortunately very, very ugly
 	gdir="$(git rev-parse --git-common-dir)" &&
@@ -76,7 +100,7 @@ trigger_auto_gc () {
 	git -c gc.auto=1 -c gc.pruneexpire=now -c gc.autodetach=0 gc --auto
 }
 
-test_expect_failure 'gc respects refs/reflogs in all worktrees' '
+test_expect_success 'gc respects refs/reflogs in all worktrees' '
 	test_commit something &&
 	git worktree add worktree &&
 	(
-- 
2.11.1.windows.1

^ permalink raw reply related

* [PATCH 1/2] worktree: demonstrate data lost to auto-gc
From: Johannes Schindelin @ 2017-02-08 12:17 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Nguyễn Thái Ngọc Duy
In-Reply-To: <cover.1486556262.git.johannes.schindelin@gmx.de>

When gc --auto is called in the presence of worktrees, it fails to take
*all* reflogs into account when trying to retain reachable objects, and as
a consequence data integrity goes pretty much to hell.

This patch provides a test case in the hopes that this bug gets fixed,
after an initial attempt has stalled for eight months already.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 t/t6500-gc.sh | 39 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 39 insertions(+)

diff --git a/t/t6500-gc.sh b/t/t6500-gc.sh
index 1762dfa6a3..e24a4fb611 100755
--- a/t/t6500-gc.sh
+++ b/t/t6500-gc.sh
@@ -67,5 +67,44 @@ test_expect_success 'auto gc with too many loose objects does not attempt to cre
 	test_line_count = 2 new # There is one new pack and its .idx
 '
 
+trigger_auto_gc () {
+	# This is unfortunately very, very ugly
+	gdir="$(git rev-parse --git-common-dir)" &&
+	mkdir -p "$gdir"/objects/17 &&
+	touch "$gdir"/objects/17/17171717171717171717171717171717171717 &&
+	touch "$gdir"/objects/17/17171717171717171717171717171717171718 &&
+	git -c gc.auto=1 -c gc.pruneexpire=now -c gc.autodetach=0 gc --auto
+}
+
+test_expect_failure 'gc respects refs/reflogs in all worktrees' '
+	test_commit something &&
+	git worktree add worktree &&
+	(
+		cd worktree &&
+		git checkout --detach &&
+		# avoid implicit tagging of test_commit
+		echo 1 >something.t &&
+		test_tick &&
+		git commit -m worktree-reflog something.t &&
+		git rev-parse --verify HEAD >../commit-reflog &&
+		echo 2 >something.t &&
+		test_tick &&
+		git commit -m worktree-ref something.t &&
+		git rev-parse --verify HEAD >../commit-ref
+	) &&
+	trigger_auto_gc &&
+	git rev-parse --verify $(cat commit-ref)^{commit} &&
+	git rev-parse --verify $(cat commit-reflog)^{commit} &&
+
+	# Now, add a reflog in the top-level dir and verify that `git gc` in
+	# the worktree does not purge that
+	git checkout --detach &&
+	echo 3 >something.t &&
+	test_tick &&
+	git commit -m commondir-reflog something.t &&
+	git rev-parse --verify HEAD >commondir-reflog &&
+	(cd worktree && trigger_auto_gc) &&
+	git rev-parse --verify $(cat commondir-reflog)^{commit}
+'
 
 test_done
-- 
2.11.1.windows.1



^ permalink raw reply related

* [PATCH] rev-parse --git-path: fix output when running in a subdirectory
From: Johannes Schindelin @ 2017-02-08 12:17 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Nguyễn Thái Ngọc Duy

In addition to making git_path() aware of certain file names that need
to be handled differently e.g. when running in worktrees, the commit
557bd833bb (git_path(): be aware of file relocation in $GIT_DIR,
2014-11-30) also snuck in a new option for `git rev-parse`:
`--git-path`.

On the face of it, there is no obvious bug in that commit's diff: it
faithfully calls git_path() on the argument and prints it out, i.e. `git
rev-parse --git-path <filename>` has the same precise behavior as
calling `git_path("<filename>")` in C.

The problem lies deeper, much deeper. In hindsight (which is always
unfair), implementing the .git/ directory discovery in
`setup_git_directory()` by changing the working directory may have
allowed us to avoid passing around a struct that contains information
about the current repository, but it bought us many, many problems.

In this case, when being called in a subdirectory, `git rev-parse`
changes the working directory to the top-level directory before calling
`git_path()`. In the new working directory, the result is correct. But
in the working directory of the calling script, it is incorrect.

Example: when calling `git rev-parse --git-path HEAD` in, say, the
Documentation/ subdirectory of Git's own source code, the string
`.git/HEAD` is printed.

Side note: that bug is hidden when running in a subdirectory of a
worktree that was added by the `git worktree` command: in that case, the
(correct) absolute path of the `HEAD` file is printed.

In the interest of time, this patch does not go the "correct" route to
introduce a struct with repository information (and removing global
state in the process), instead this patch chooses to detect when the
command was called in a subdirectory and forces the result to be an
absolute path.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
Published-As: https://github.com/dscho/git/releases/tag/git-path-in-subdir-v1
Fetch-It-Via: git fetch https://github.com/dscho/git git-path-in-subdir-v1

 builtin/rev-parse.c   | 6 +++++-
 t/t0060-path-utils.sh | 2 ++
 2 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/builtin/rev-parse.c b/builtin/rev-parse.c
index ff13e59e1d..f9d5762bf2 100644
--- a/builtin/rev-parse.c
+++ b/builtin/rev-parse.c
@@ -597,9 +597,13 @@ int cmd_rev_parse(int argc, const char **argv, const char *prefix)
 		}
 
 		if (!strcmp(arg, "--git-path")) {
+			const char *path;
 			if (!argv[i + 1])
 				die("--git-path requires an argument");
-			puts(git_path("%s", argv[i + 1]));
+			path = git_path("%s", argv[i + 1]);
+			if (prefix && !is_absolute_path(path))
+				path = real_path(path);
+			puts(path);
 			i++;
 			continue;
 		}
diff --git a/t/t0060-path-utils.sh b/t/t0060-path-utils.sh
index 444b5a4df8..790584fcc5 100755
--- a/t/t0060-path-utils.sh
+++ b/t/t0060-path-utils.sh
@@ -271,6 +271,8 @@ relative_path "<null>"		"<empty>"	./
 relative_path "<null>"		"<null>"	./
 relative_path "<null>"		/foo/a/b	./
 
+test_git_path "mkdir sub && cd sub && test_when_finished cd .. &&" \
+	foo "$(pwd)/.git/foo"
 test_git_path A=B                info/grafts .git/info/grafts
 test_git_path GIT_GRAFT_FILE=foo info/grafts foo
 test_git_path GIT_GRAFT_FILE=foo info/////grafts foo

base-commit: 6e3a7b3398559305c7a239a42e447c21a8f39ff8
-- 
2.11.1.windows.1

^ permalink raw reply related

* [PATCH 0/2] Demonstrate a critical worktree/gc bug
From: Johannes Schindelin @ 2017-02-08 12:17 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Nguyễn Thái Ngọc Duy

... and a half-working workaround for the auto-gc case.

This patch series really is just another attempt to prod Duy into fixing
this instead of dabbling with shiny new toys ;-)

Changes since "v0":

- split out the test case, both to make it easier for Duy to integrate
  it into the patch series that fixes the bug, as well as to provide a
  little more, very gentle pressure to demonstrate the severity of this
  problem.


Johannes Schindelin (2):
  worktree: demonstrate data lost to auto-gc
  worktree: demonstrate a half-working workaround for auto-gc

 t/t6500-gc.sh | 63 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 63 insertions(+)


base-commit: 6e3a7b3398559305c7a239a42e447c21a8f39ff8
Published-As: https://github.com/dscho/git/releases/tag/gc-worktree-v1
Fetch-It-Via: git fetch https://github.com/dscho/git gc-worktree-v1

-- 
2.11.1.windows.1


^ permalink raw reply

* [PATCH 1/2] refs.c: add resolve_ref_submodule()
From: Nguyễn Thái Ngọc Duy @ 2017-02-08 11:31 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Michael Haggerty,
	Nguyễn Thái Ngọc Duy
In-Reply-To: <20170208113144.8201-1-pclouds@gmail.com>

This is basically the extended version of resolve_gitlink_ref() where we
have access to more info from the underlying resolve_ref_recursively() call.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 refs.c | 20 ++++++++++++++------
 refs.h |  3 +++
 2 files changed, 17 insertions(+), 6 deletions(-)

diff --git a/refs.c b/refs.c
index cd36b64ed9..02e35d83f3 100644
--- a/refs.c
+++ b/refs.c
@@ -1325,18 +1325,18 @@ const char *resolve_ref_unsafe(const char *refname, int resolve_flags,
 				       resolve_flags, sha1, flags);
 }
 
-int resolve_gitlink_ref(const char *submodule, const char *refname,
-			unsigned char *sha1)
+const char *resolve_ref_submodule(const char *submodule, const char *refname,
+				  int resolve_flags, unsigned char *sha1,
+				  int *flags)
 {
 	size_t len = strlen(submodule);
 	struct ref_store *refs;
-	int flags;
 
 	while (len && submodule[len - 1] == '/')
 		len--;
 
 	if (!len)
-		return -1;
+		return NULL;
 
 	if (submodule[len]) {
 		/* We need to strip off one or more trailing slashes */
@@ -1349,9 +1349,17 @@ int resolve_gitlink_ref(const char *submodule, const char *refname,
 	}
 
 	if (!refs)
-		return -1;
+		return NULL;
+
+	return resolve_ref_recursively(refs, refname, resolve_flags, sha1, flags);
+}
+
+int resolve_gitlink_ref(const char *submodule, const char *refname,
+			unsigned char *sha1)
+{
+	int flags;
 
-	if (!resolve_ref_recursively(refs, refname, 0, sha1, &flags) ||
+	if (!resolve_ref_submodule(submodule, refname, 0, sha1, &flags) ||
 	    is_null_sha1(sha1))
 		return -1;
 	return 0;
diff --git a/refs.h b/refs.h
index 9fbff90e79..74542468d8 100644
--- a/refs.h
+++ b/refs.h
@@ -88,6 +88,9 @@ int peel_ref(const char *refname, unsigned char *sha1);
  */
 int resolve_gitlink_ref(const char *submodule, const char *refname,
 			unsigned char *sha1);
+const char *resolve_ref_submodule(const char *submodule, const char *refname,
+				  int resolve_flags, unsigned char *sha1,
+				  int *flags);
 
 /*
  * Return true iff abbrev_name is a possible abbreviation for
-- 
2.11.0.157.gd943d85


^ permalink raw reply related

* [PATCH RFC 0/2] Kill manual ref parsing code in worktree.c
From: Nguyễn Thái Ngọc Duy @ 2017-02-08 11:31 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Michael Haggerty,
	Nguyễn Thái Ngọc Duy

A hundred years ago I added this code because a "standalone" ref
parsing code was not available from refs.c and the file was going through
some heavy changes that refactoring its ref parsing code out was not
an option. I promised to kill this parse_ref() eventually. I'm
fulfilling it today (or soon).

I would really like to you double check the approach I'm using here
(using submodule interface for accessing refs from another worktree)
since that may be the way forward to fix the "gc losing objects" in
multi worktrees. I've given it lots of thoughts in the last 24 hours.
Still can't find any fundamental flaw...

Nguyễn Thái Ngọc Duy (2):
  refs.c: add resolve_ref_submodule()
  worktree.c: use submodule interface to access refs from another worktree

 branch.c   |  3 +-
 refs.c     | 20 +++++++++----
 refs.h     |  3 ++
 worktree.c | 99 +++++++++++++++-----------------------------------------------
 worktree.h |  2 +-
 5 files changed, 44 insertions(+), 83 deletions(-)

-- 
2.11.0.157.gd943d85


^ permalink raw reply

* Re: Non-zero exit code without error
From: Serdar Sahin @ 2017-02-08 10:26 UTC (permalink / raw)
  To: Christian Couder; +Cc: git
In-Reply-To: <CAP8UFD18Sbqo-_ZVyYTJtwNaRc8bFSd0KEYQ1oRH7-G+xnJTJg@mail.gmail.com>

Hi Christian,


We are using a private repo (Github Enterprise). Let me give you the
details you requested.


On Git Server: git version 2.6.5.1574.g5e6a493

On my client: git version 2.10.1 (Apple Git-78)


I’ve tried to reproduce it with public repos, but couldn’t do so. If I
could get an error/log output, that would be sufficient.


I am also including the full output below. (also git gc)


MacOSX:test serdar$ git clone --mirror --depth 50 --no-single-branch
git@git.privateserver.com:Casual/code_repository.git

Cloning into bare repository 'code_repository.git'...

remote: Counting objects: 3362, done.

remote: Compressing objects: 100% (1214/1214), done.

remote: Total 3362 (delta 2335), reused 2968 (delta 2094), pack-reused 0

Receiving objects: 100% (3362/3362), 56.77 MiB | 1.83 MiB/s, done.

Resolving deltas: 100% (2335/2335), done.

MacOSX:test serdar$ cd code_repository.git/

MacOSX:code_repository.git serdar$ GIT_CURL_VERBOSE=1 GIT_TRACE=1  git
fetch --depth 50 origin cc086c96cdffe5c1ac78e6139a7a4b79e7c821ee

13:23:15.648337 git.c:350               trace: built-in: git 'fetch'
'--depth' '50' 'origin' 'cc086c96cdffe5c1ac78e6139a7a4b79e7c821ee'

13:23:15.651127 run-command.c:336       trace: run_command: 'ssh'
'git@git.privateserver.com' 'git-upload-pack
'\''Casual/code_repository.git'\'''

13:23:17.750015 run-command.c:336       trace: run_command: 'gc' '--auto'

13:23:17.750829 exec_cmd.c:189          trace: exec: 'git' 'gc' '--auto'

13:23:17.753983 git.c:350               trace: built-in: git 'gc' '--auto'

MacOSX:code_repository.git serdar$ echo $?

1

MacOSX:code_repository.git serdar$ GIT_CURL_VERBOSE=1 GIT_TRACE=1 git gc --auto

13:23:45.899038 git.c:350               trace: built-in: git 'gc' '--auto'

MacOSX:code_repository.git serdar$ echo $?

0

MacOSX:code_repository.git serdar$

On Wed, Feb 8, 2017 at 1:07 PM, Christian Couder
<christian.couder@gmail.com> wrote:
> Hi,
>
> On Tue, Feb 7, 2017 at 12:27 PM, Serdar Sahin <serdar@peakgames.net> wrote:
>> Hi,
>>
>> When we execute the following lines, the exit code is 1, but it is
>> unclear what is the reason of this exit code. Do you have any idea?
>>
>> git clone --mirror --depth 50 --no-single-branch
>> git@github.hede.com:Casual/hodo-server.git
>
> First, could you tell us the git version you are using on the client
> and on the server, and if this a new problem with newer versions?
> Also is the repos accessible publicly or is it possible to reproduce
> on another repo?
> And what happens using other protocols like HTTP/S?
>
>> Cloning into bare repository 'hodo-server.git'...
>> remote: Counting objects: 3371, done.
>> remote: Compressing objects: 100% (1219/1219), done.
>> remote: Total 3371 (delta 2344), reused 2971 (delta 2098), pack-reused 0
>> Receiving objects: 100% (3371/3371), 56.77 MiB | 2.18 MiB/s, done.
>> Resolving deltas: 100% (2344/2344), done.
>>
>> echo $?
>> 0
>>
>> cd hodo-server.git/
>>
>> GIT_CURL_VERBOSE=1 GIT_TRACE=1  git fetch --depth 50 origin
>> cc086c96cdffe5c1ac78e6139a7a4b79e7c821ee
>> 14:12:35.215889 git.c:350               trace: built-in: git 'fetch'
>> '--depth' '50' 'origin' 'cc086c96cdffe5c1ac78e6139a7a4b79e7c821ee'
>> 14:12:35.217273 run-command.c:336       trace: run_command: 'ssh'
>> 'git@github.hede.com' 'git-upload-pack '\''Casual/hodo-server.git'\'''
>> 14:12:37.301122 run-command.c:336       trace: run_command: 'gc' '--auto'
>> 14:12:37.301866 exec_cmd.c:189          trace: exec: 'git' 'gc' '--auto'
>> 14:12:37.304473 git.c:350               trace: built-in: git 'gc' '--auto'
>>
>> echo $?
>> 1
>
> What happens if you just run 'git gc --auto' after that?

^ permalink raw reply

* Re: subtree merging fails
From: Stavros Liaskos @ 2017-02-08 10:42 UTC (permalink / raw)
  To: David Aguilar; +Cc: Samuel Lijin, git@vger.kernel.org
In-Reply-To: <20170207184437.c6uuuxcmhi434vbc@gmail.com>

@Samuel Lijin
I tried now and I get:
"merge: branch_name
- not something we can merge".
Maybe that is something easy to fix but currently I am using a
workaround script so I am not putting any more effort at this at the
moment.

@David Aguilar
That's true but the trailing slash is already there. This commands
looks promising. An update would be GREAT!

FYI that's the script I am using to address this problem:

#!/bin/bash

function initial {
  if git remote | grep -q lisa_remote
  then
    echo "Subtree delete & update"
    git checkout lisa_branch
    git pull
    git checkout master
    git merge --squash -s subtree --no-commit lisa_branch
    git merge --squash --allow-unrelated-histories -s subtree
--no-commit lisa_branch
  else
    echo "Add subtree"
    git remote add lisa_remote git@xxxxxxxx:lisa/lisa.git
    git fetch lisa_remote
    git checkout -b lisa_branch lisa_remote/master
    git checkout master
    git read-tree --prefix=lisaSubTree/ -u lisa_branch
    gitrm
    git rm --cached -r lisaSubTree/.gitignore
    git rm --cached -r lisaSubTree/*
  fi
}
initial



On Tue, Feb 7, 2017 at 7:44 PM, David Aguilar <davvid@gmail.com> wrote:
> On Tue, Feb 07, 2017 at 08:59:06AM -0600, Samuel Lijin wrote:
>> Have you tried using (without -s subtree) -X subtree=path/to/add/subtree/at?
>>
>> From the man page:
>>
>>           subtree[=<path>]
>>                This option is a more advanced form of subtree
>> strategy, where the strategy
>>                makes a guess on how two trees must be shifted to match
>> with each other when
>>                merging. Instead, the specified path is prefixed (or
>> stripped from the
>>                beginning) to make the shape of two trees to match.
>
> I'm not 100% certain, but it's highly likely that the subtree=<prefix>
> argument needs to include a trailing slash "/" in the prefix,
> otherwise files will be named e.g. "fooREADME" instead of
> "foo/README" when prefix=foo.
>
> These days I would steer users towards the "git-subtree" command in
> contrib/ so that users don't need to deal with these details.  It
> handles all of this stuff for you.
>
> https://github.com/git/git/blob/master/contrib/subtree/git-subtree.txt
>
> https://github.com/git/git/tree/master/contrib/subtree
>
> Updating the progit book to also mention git-subtree, in addition to the
> low-level methods, would probably be a good user-centric change.
> --
> David

^ permalink raw reply

* Bug with fixup and autosquash
From: Ashutosh Bapat @ 2017-02-08 10:10 UTC (permalink / raw)
  To: git

Hi Git-developers,
First of all thanks for developing this wonderful scm tool. It's sooo versatile.

I have been using git rebase heavily these days and seem to have found a bug.

If there are two commit messages which have same prefix e.g.
yyyyyy This is prefix
xxxxxx This is prefix and message

xxxxxx comitted before yyyyyy

Now I commit a fixup to yyyyyy using git commit --fixup yyyyyy
zzzzzz fixup! This is prefix

When I run git rebase -i --autosquash, the script it shows me looks like
pick xxxxxx This is prefix and message
fixup zzzzzz fixup! This is prefix
pick yyyyyy This is prefix

I think the correct order is
pick xxxxxx This is prefix and message
pick yyyyyy This is prefix
fixup zzzzzz fixup! This is prefix

Is that right?

I am using
[ubuntu]git --version
git version 1.7.9.5

--
Best Wishes,
Ashutosh Bapat
EnterpriseDB Corporation
The Postgres Database Company

^ 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