Git development
 help / color / mirror / Atom feed
* Re: [PATCH 0/3] delete_ref(): support reflog messages
From: Jeff King @ 2017-02-17  8:17 UTC (permalink / raw)
  To: Kyle Meyer; +Cc: Junio C Hamano, git
In-Reply-To: <20170217035800.13214-1-kyle@kyleam.com>

On Thu, Feb 16, 2017 at 10:57:57PM -0500, Kyle Meyer wrote:

> [Sorry for the slow response.]

No problem. The pace of open source varies wildly. :)

> >   - "git branch -m" does seem to realize when we are renaming HEAD,
> >     because it updates HEAD to point to the new branch name. But it
> >     should probably insert another reflog entry mentioning the rename
> >     (we do for "git checkout foo", even when "foo" has the same sha1 as
> >     the current HEAD).
> 
> I haven't worked out how to do this part yet.  I'm guessing the change
> will involve modifying split_head_update().
> 
> If this is added, should it be instead of, rather than in addition to,
> the deletion entry?  If a "Branch: renamed ..." entry is present, it
> doesn't seem like the deletion entry is providing any additional
> information.

I think you could do an "instead of" that goes from sha1 X to X, and
just mentions the rename. Or you could add a second entry after the
delete that takes it from 0{40} back to X.

I suspect the latter is easier to do, and I doubt anybody would care
that much of the exact form. These entries aren't really doing anything
for reachability. They're just giving an audit log of what happened. So
I don't think anybody would really care unless they were debugging a
confusing situation by hand. And as long as there's enough information
to figure out what happened, they'll be happy.

-Peff

^ permalink raw reply

* Re: [PATCH 2/3] update-ref: pass reflog message argument to delete_refs
From: Jeff King @ 2017-02-17  8:22 UTC (permalink / raw)
  To: Kyle Meyer; +Cc: Junio C Hamano, git
In-Reply-To: <20170217035800.13214-3-kyle@kyleam.com>

On Thu, Feb 16, 2017 at 10:57:59PM -0500, Kyle Meyer wrote:

> Now that delete_refs() accepts a reflog message, pass the
> user-provided message to delete_refs() rather than silently dropping
> it.  The doesn't matter for the deleted ref's log because the log is
> deleted along with the ref, but this entry will show up in HEAD's
> reflog when deleting a checked out branch.

Sounds good.

> diff --git a/builtin/update-ref.c b/builtin/update-ref.c
> index a41f9adf1..f642acc22 100644
> --- a/builtin/update-ref.c
> +++ b/builtin/update-ref.c
> @@ -435,7 +435,7 @@ int cmd_update_ref(int argc, const char **argv, const char *prefix)
>  		 */
>  		return delete_ref(refname,
>  				  (oldval && !is_null_sha1(oldsha1)) ? oldsha1 : NULL,
> -				  flags, NULL);
> +				  flags, msg);

This looks obviously correct.

> diff --git a/t/t1400-update-ref.sh b/t/t1400-update-ref.sh
> index b0ffc0b57..65918d984 100755
> --- a/t/t1400-update-ref.sh
> +++ b/t/t1400-update-ref.sh
> @@ -85,6 +85,15 @@ test_expect_success "delete $m (by HEAD)" '
>  '
>  rm -f .git/$m
>  
> +test_expect_success "deleting current branch adds message to HEAD's log" '
> +	git update-ref $m $A &&
> +	git symbolic-ref HEAD $m &&
> +	git update-ref -mdelmsg -d $m &&
> +	! test -f .git/$m &&
> +	grep "delmsg$" .git/logs/HEAD >/dev/null
> +'
> +rm -f .git/$m

I think covering this with a test is good.

I don't know if it's also worth testing that deleting via HEAD also
writes the reflog. I.e.,:

  git update-ref -m delete-by-head -d HEAD

Some of the style here is a bit out-dated, but I think you are just
matching the surrounding tests.  So that's OK by me (though a patch to
modernize the whole thing would be welcome, too).

For reference, the two things I notice are:

  - we prefer test_path_is_missing to "! test -f" these days.

  - we don't redirect the output of grep (it's handled already in
    non-verbose mode, and in verbose mode we try to be...verbose).

-Peff

^ permalink raw reply

* git alias for options
From: hIpPy @ 2017-02-17  8:23 UTC (permalink / raw)
  To: git

Git has aliases for git commands. Is there a (an inbuilt) way to alias
options? If not, what is the reason?

Thanks,
hippy

^ permalink raw reply

* Re: [PATCH 3/3] rename_ref: replace empty deletion message in HEAD's log
From: Jeff King @ 2017-02-17  8:31 UTC (permalink / raw)
  To: Kyle Meyer; +Cc: Junio C Hamano, git
In-Reply-To: <20170217035800.13214-4-kyle@kyleam.com>

On Thu, Feb 16, 2017 at 10:58:00PM -0500, Kyle Meyer wrote:

> When the current branch is renamed, the deletion of the old ref is
> recorded in HEAD's log with an empty message.  Now that delete_refs()
> accepts a reflog message, provide a more descriptive message.  This
> message will not be included in the reflog of the renamed branch, but
> its log already covers the renaming event with a message of "Branch:
> renamed ...".

Right, makes sense. The code overall looks fine, though I have one
nit:

> @@ -2616,10 +2617,15 @@ static int files_rename_ref(struct ref_store *ref_store,
>  		return error("unable to move logfile logs/%s to "TMP_RENAMED_LOG": %s",
>  			oldrefname, strerror(errno));
>  
> -	if (delete_ref(oldrefname, orig_sha1, REF_NODEREF, NULL)) {
> +	strbuf_addf(&logmsg_del, "Deleted %s", oldrefname);

We've been anything but consistent with our reflog messages in the past,
but I think the general guideline for new ones is to use "command:
action". Of course we don't _know_ the action here, because we're deep
in rename_ref().

Should we have the caller pass it in for us, as we do with delete_ref()
and update_ref()?

I see we actually already have a "logmsg" parameter. It already says
"Branch: renamed %s to %s". Could we just reuse that? I know that this
step is technically just the deletion, but I think it more accurately
describes the whole operation that the deletion is part of.

-Peff

^ permalink raw reply

* Re: [PATCH v2 11/19] builtin/replace: convert to struct object_id
From: Michael Haggerty @ 2017-02-17  8:44 UTC (permalink / raw)
  To: brian m. carlson, git; +Cc: Jeff King
In-Reply-To: <20170214023141.842922-12-sandals@crustytoothpaste.net>

On 02/14/2017 03:31 AM, brian m. carlson wrote:
> Convert various uses of unsigned char [20] to struct object_id.  Rename
> replace_object_sha1 to rename_object_oid.  Finally, specify a constant
> in terms of GIT_SHA1_HEXSZ.

The new name is not rename_object_oid but rather replace_object_oid.

> [...]

Michael


^ permalink raw reply

* Re: [PATCH v2 14/19] hex: introduce parse_oid_hex
From: Michael Haggerty @ 2017-02-17  9:26 UTC (permalink / raw)
  To: brian m. carlson, git; +Cc: Jeff King
In-Reply-To: <20170214023141.842922-15-sandals@crustytoothpaste.net>

On 02/14/2017 03:31 AM, brian m. carlson wrote:
> Introduce a function, parse_oid_hex, which parses a hexadecimal object
> ID and if successful, sets a pointer to just beyond the last character.
> This allows for simpler, more robust parsing without needing to
> hard-code integer values throughout the codebase.
> 
> Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
> ---
>  cache.h | 8 ++++++++
>  hex.c   | 8 ++++++++
>  2 files changed, 16 insertions(+)
> 
> diff --git a/cache.h b/cache.h
> index 61fc86e6d7..5dc89a058c 100644
> --- a/cache.h
> +++ b/cache.h
> @@ -1319,6 +1319,14 @@ extern char *oid_to_hex_r(char *out, const struct object_id *oid);
>  extern char *sha1_to_hex(const unsigned char *sha1);	/* static buffer result! */
>  extern char *oid_to_hex(const struct object_id *oid);	/* same static buffer as sha1_to_hex */
>  
> +/*
> + * Parse a hexadecimal object ID starting from hex, updating the pointer
> + * specified by p when parsing stops.  The resulting object ID is stored in oid.
> + * Returns 0 on success.  Parsing will stop on the first NUL or other invalid
> + * character.
> + */
> +extern int parse_oid_hex(const char *hex, struct object_id *oid, const char **p);
> +

I like this function. This is a convenient kind of interface to work
with. A few minor comments:

If you rename the nondescript `p` parameter to, say, `end`, its purpose
would be more transparent. Alternatively, `skip_prefix()` calls the
corresponding parameter `out`.

It would be nice for the docstring to mention that the object ID must be
a full, 40-character hex string. Otherwise "Parsing will stop on the
first NUL or other invalid character" makes it sound like the function
might be satisfied with fewer than 40 characters.

Finally, you might mention the useful fact that `p` is only updated if
the function succeeds.

> [...]

Michael


^ permalink raw reply

* Re: topological index field for commit objects
From: Jeff King @ 2017-02-17  9:26 UTC (permalink / raw)
  To: Jakub Narębski; +Cc: Git Mailing List
In-Reply-To: <CANQwDwfk2k+qGtx-_RqoLKObAgyV+ebE57UAd-VXDv86HDw2vg@mail.gmail.com>

On Sat, Feb 04, 2017 at 02:43:01PM +0100, Jakub Narębski wrote:

> >>>> Do Git use EWAH / EWOK bitmaps for reachability analysis, or is it still
> >>>> limited to object counting?
> >>>
> >>> At GitHub we are using them for --contains analysis, along with mass
> >>> ahead/behind (e.g., as in https://github.com/gitster/git/branches). My
> >>> plan is to send patches upstream, but they need some cleanup first.
> >>
> >> Ping. have you got time to clean up those patches?
> >
> > No, I haven't. Don't hold your breath; it's something I hope to work on
> > in the next 6 months, not the next 6 weeks.
> 
> Ping, Was there any progress on this front? It is now almost 6 months
> later...
> 
> Could those patches be made available in a "dirty" form?

I just pushed them up to the "jk/ahead-behind" branch of
https://github.com/peff/git.

They were originally done on top of v1.9.1, but I forward-ported them to
the current "master" just now. The result compiles, but I haven't really
tested it extensively. Caveat applier.

-Peff

^ permalink raw reply

* Re: topological index field for commit objects
From: Jakub Narębski @ 2017-02-17  9:28 UTC (permalink / raw)
  To: Jeff King; +Cc: Git Mailing List
In-Reply-To: <20170217092616.ulassn3472stbfga@sigill.intra.peff.net>

On 17 February 2017 at 10:26, Jeff King <peff@peff.net> wrote:
> On Sat, Feb 04, 2017 at 02:43:01PM +0100, Jakub Narębski wrote:
>
>> >>>> Do Git use EWAH / EWOK bitmaps for reachability analysis, or is it still
>> >>>> limited to object counting?
>> >>>
>> >>> At GitHub we are using them for --contains analysis, along with mass
>> >>> ahead/behind (e.g., as in https://github.com/gitster/git/branches). My
>> >>> plan is to send patches upstream, but they need some cleanup first.
>> >>
>> >> Ping. have you got time to clean up those patches?
>> >
>> > No, I haven't. Don't hold your breath; it's something I hope to work on
>> > in the next 6 months, not the next 6 weeks.
>>
>> Ping, Was there any progress on this front? It is now almost 6 months
>> later...
>>
>> Could those patches be made available in a "dirty" form?
>
> I just pushed them up to the "jk/ahead-behind" branch of
> https://github.com/peff/git.
>
> They were originally done on top of v1.9.1, but I forward-ported them to
> the current "master" just now. The result compiles, but I haven't really
> tested it extensively. Caveat applier.

Thanks a lot! I'll check this out.

-- 
Jakub Narebski

^ permalink raw reply

* Re: [PATCH v2 15/19] refs: simplify parsing of reflog entries
From: Michael Haggerty @ 2017-02-17  9:41 UTC (permalink / raw)
  To: brian m. carlson, git; +Cc: Jeff King
In-Reply-To: <20170214023141.842922-16-sandals@crustytoothpaste.net>

On 02/14/2017 03:31 AM, brian m. carlson wrote:
> The current code for reflog entries uses a lot of hard-coded constants,
> making it hard to read and modify.  Use parse_oid_hex and two temporary
> variables to simplify the code and reduce the use of magic constants.
> 
> Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
> ---
>  refs/files-backend.c | 12 +++++++-----
>  1 file changed, 7 insertions(+), 5 deletions(-)
> 
> diff --git a/refs/files-backend.c b/refs/files-backend.c
> index d7a5fd2a7c..09227a3f63 100644
> --- a/refs/files-backend.c
> +++ b/refs/files-backend.c
> @@ -3117,12 +3117,14 @@ static int show_one_reflog_ent(struct strbuf *sb, each_reflog_ent_fn fn, void *c
>  	char *email_end, *message;
>  	unsigned long timestamp;
>  	int tz;
> +	const char *p = sb->buf;
> +	const int minlen = 2 * GIT_SHA1_HEXSZ + 3;
>  
>  	/* old SP new SP name <email> SP time TAB msg LF */
> -	if (sb->len < 83 || sb->buf[sb->len - 1] != '\n' ||
> -	    get_oid_hex(sb->buf, &ooid) || sb->buf[40] != ' ' ||
> -	    get_oid_hex(sb->buf + 41, &noid) || sb->buf[81] != ' ' ||
> -	    !(email_end = strchr(sb->buf + 82, '>')) ||
> +	if (sb->len < minlen || sb->buf[sb->len - 1] != '\n' ||
> +	    parse_oid_hex(p, &ooid, &p) || *p++ != ' ' ||
> +	    parse_oid_hex(p, &noid, &p) || *p++ != ' ' ||
> +	    !(email_end = strchr(p, '>')) ||
>  	    email_end[1] != ' ' ||
>  	    !(timestamp = strtoul(email_end + 2, &message, 10)) ||
>  	    !message || message[0] != ' ' ||
> @@ -3136,7 +3138,7 @@ static int show_one_reflog_ent(struct strbuf *sb, each_reflog_ent_fn fn, void *c
>  		message += 6;
>  	else
>  		message += 7;
> -	return fn(&ooid, &noid, sb->buf + 82, timestamp, tz, message, cb_data);
> +	return fn(&ooid, &noid, sb->buf + minlen - 1, timestamp, tz, message, cb_data);

I think `sb->buf + minlen - 1` is just `p` here, isn't it?

Also, I think instead of the initial test `sb->len < minlen`, it would
be enough to check `!sb->len` (i.e., error if it's zero to avoid a
buffer underflow in `sb->buf[sb->len - 1])`, because the
`parse_oid_hex()` calls together with the `*p++ != ' '` and
`sb->buf[sb->len - 1] != '\n'` checks already ensure that the string has
at least the minimum length.

Then you could do away with `minlen` entirely.

Michael


^ permalink raw reply

* Re: [PATCH v2 00/19] object_id part 6
From: Michael Haggerty @ 2017-02-17  9:55 UTC (permalink / raw)
  To: brian m. carlson, git; +Cc: Jeff King
In-Reply-To: <20170214023141.842922-1-sandals@crustytoothpaste.net>

On 02/14/2017 03:31 AM, brian m. carlson wrote:
> This is another series in the continuing conversion to struct object_id.
> 
> This series converts more of the builtin directory and some of the refs
> code to use struct object_id. Additionally, it implements an
> nth_packed_object_oid function which provides a struct object_id version
> of the nth_packed_object function, and a parse_oid_hex function that
> makes parsing easier.
> 
> The patch to use parse_oid_hex in the refs code has been split out into
> its own patch, just because I'm wary of that code and potentially
> breaking things, and I want it to be easy to revert in case things go
> wrong.  I have no reason to believe it is anything other than fully
> functional, however.
> 
> Changes from v1:
> * Implement parse_oid_hex and use it.
> * Make nth_packed_object_oid take a variable into which to store the
>   object ID.  This avoids concerns about unsafe casts.
> * Rebase on master.

Thanks as always for working on this!

I skimmed over the patches (looking more carefully at the refs-related
ones) and left a few minor comments but didn't find anything serious.

I'm curious; what fraction of the overall convert-to-object_id campaign
do you estimate is done so far? Are you getting close to the promised
land yet?

Michael


^ permalink raw reply

* Re: [PATCH] tempfile: avoid "ferror | fclose" trick
From: Michael Haggerty @ 2017-02-17 10:42 UTC (permalink / raw)
  To: Jeff King; +Cc: Andreas Schwab, Junio C Hamano, Jáchym Barvínek, git
In-Reply-To: <20170217080759.2357wzdiuymcyosw@sigill.intra.peff.net>

On 02/17/2017 09:07 AM, Jeff King wrote:
> [...]
> That's similar to what I wrote earlier, but if we don't mind overwriting
> errno unconditionally, I think just:
> 
>   errno = EIO; /* covers ferror(), overwritten by failing fclose() */
>   err |= ferror(fp);
>   err |= fclose(fp);
> 
> does the same thing.

True; I'd forgotten the convention that non-failing functions are
allowed to change errno. Your solution is obviously simpler and faster.

Michael


^ permalink raw reply

* Re: body-CC-comment regression
From: Johan Hovold @ 2017-02-17 11:06 UTC (permalink / raw)
  To: Matthieu Moy
  Cc: Johan Hovold, git, Jeff King, Kevin Daudt, Junio C Hamano,
	Larry Finger
In-Reply-To: <vpqlgt6hug6.fsf@anie.imag.fr>

On Thu, Feb 16, 2017 at 07:16:57PM +0100, Matthieu Moy wrote:
> Johan Hovold <johan@kernel.org> writes:
> 
> > Hi,
> >
> > I recently noticed that after an upgrade, git-send-email (2.10.2)
> > started aborting when trying to send patches that had a linux-kernel
> > stable-tag in its body. For example,
> >
> > 	Cc: <stable@vger.kernel.org>	# 4.4
> >
> > was now parsed as
> >
> > 	"stable@vger.kernel.org#4.4"
> >
> > which resulted in
> >
> > 	Died at /usr/libexec/git-core/git-send-email line 1332, <FIN> line 1.
> 
> This has changed in e3fdbcc8e1 (parse_mailboxes: accept extra text after
> <...> address, 2016-10-13), released v2.11.0 as you noticed:
> 
> > The problem with the resulting fixes that are now in 2.11.1 is that
> > git-send-email no longer discards the trailing comment but rather
> > shoves it into the name after adding some random white space:
> >
> > 	"# 3 . 3 . x : 1b9508f : sched : Rate-limit newidle" <stable@vger.kernel.org>"
> >
> > This example is based on the example from
> > Documentation/process/stable-kernel-rules.rst:
> >
> > 	Cc: <stable@vger.kernel.org> # 3.3.x: 1b9508f: sched: Rate-limit newidle
> >
> > and this format for stable-tags has been documented at least since 2009
> > and 8e9b9362266d ("Doc/stable rules: add new cherry-pick logic"), and
> > has been supported by git since 2012 and 831a488b76e0 ("git-send-email:
> > remove garbage after email address") I believe.
> >
> > Can we please revert to the old behaviour of simply discarding such
> > comments (from body-CC:s) or at least make it configurable through a
> > configuration option?
> 
> The problem is that we now accept list of emails instead of just one
> email, so it's hard to define what "comments after the email", for
> example
> 
> Cc: <foo@example.com> # , <boz@example.com>
> 
> Is not accepted as two emails.
> 
> So, just stripping whatever comes after # before parsing the list of
> emails would change the behavior once more, and possibly break other
> user's flow. Dropping the garbage after the email while parsing is
> possible, but only when we use our in-house parser (and we currently use
> Perl's Mail::Address when available).
> 
> So, a proper fix is far from obvious, and unfortunately I won't have
> time to work on that, at least not before a while.

There is another option, namely to only accept a single address for tags
in the body. I understand that being able to copy a CC-header to either
the header section or to the command line could be useful, but I don't
really see the point in allowing this in the tags in the body (a SoB
always has one address, and so should a CC-tag).

And since this is a regression for something that has been working for
years that was introduced by a new feature, I also think it's reasonable
to (partially) revert the feature.

> OTOH, the current behavior isn't that bad. It accepts the input, and
> extracts a valid email out of it. Just the display name is admitedly
> suboptimal ...

Yeah, but the display name can end up with so much noise that auto-cc is
effectively broken for people submitting kernel patches (with stable
tags) as the only way to avoid it is to suppress all bodycc.

So what I'm proposing is to revert to the earlier behaviour of only
allowing one address per body tag by simply discarding anything
after the address.

Something like the below seems to do the trick.

Thanks,
Johan



From f551b4ca9926624dc7af6c286d7cf0f97af39541 Mon Sep 17 00:00:00 2001
From: Johan Hovold <johan@kernel.org>
Date: Fri, 17 Feb 2017 11:55:47 +0100
Subject: [PATCH] send-email: only allow one address per body tag

Adding comments after a tag in the body is a common practise (e.g. in
the Linux kernel) and git-send-email has been supporting this for years
by removing any trailing cruft after the address.

After some recent changes, any trailing comment is now instead appended
to the recipient name (with some random white space inserted) resulting
in undesirable noise in the headers, for example:

CC: "# 3 . 3 . x : 1b9508f : sched : Rate-limit newidle" <stable@vger.kernel.org>

Revert to the earlier behaviour of discarding anything after the (first)
address in a tag while parsing the body.

Note that multiple addresses after are still allowed after a
command-line switch (and in a CC-header).

Fixes: b1c8a11c8024 ("send-email: allow multiple emails using --cc, --to
and --bcc")
Fixes: e3fdbcc8e164 ("parse_mailboxes: accept extra text after <...>
address")
Signed-off-by: Johan Hovold <johan@kernel.org>
---
 git-send-email.perl | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/git-send-email.perl b/git-send-email.perl
index 068d60b3e698..eea0a517f71b 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -1563,7 +1563,7 @@ foreach my $t (@files) {
 	# Now parse the message body
 	while(<$fh>) {
 		$message .=  $_;
-		if (/^(Signed-off-by|Cc): (.*)$/i) {
+		if (/^(Signed-off-by|Cc): ([^>]*>?)/i) {
 			chomp;
 			my ($what, $c) = ($1, $2);
 			chomp $c;
-- 
2.11.1

^ permalink raw reply related

* dotfiles in git template dir are not copied
From: greg0ire @ 2017-02-17  9:31 UTC (permalink / raw)
  To: git

Hi,

I noticed yesterday that dotfiles inside the directory configured in 
init.templatedir are not copied when creating a new repository.

Here is the line I think is responsible for this behavior : 
https://github.com/git/git/blob/master/builtin/init-db.c#L48

Is it a bug or a feature?

Steps to reproduce, provided you already have a template dir configured :

cd $(git config --path --get init.templatedir)
touch copied
touch .not_copied
cd /tmp
mkdir whatever
cd whatever
git init
ls -la .git

On my machine, the last command does not list .not_copied .

--
greg0ire

^ permalink raw reply

* Re: git alias for options
From: Michael J Gruber @ 2017-02-17 13:15 UTC (permalink / raw)
  To: hIpPy, git
In-Reply-To: <CAM_JFCz+9mxp37BTT7XPJ0fMd41DdbAxnvQF7id9msH+SDe6_Q@mail.gmail.com>

hIpPy venit, vidit, dixit 17.02.2017 09:23:
> Git has aliases for git commands. Is there a (an inbuilt) way to alias
> options? If not, what is the reason?
> 
> Thanks,
> hippy
> 

You can setup an alias for "command with options", for example:

git help s
`git s' is aliased to `status -s -b -uno .'

As you see here, this can include non-option arguments such as the
pathsepc '.'.

You cannot alias options independent of the command, though.

Michael

^ permalink raw reply

* Re: body-CC-comment regression
From: Matthieu Moy @ 2017-02-17 13:16 UTC (permalink / raw)
  To: Johan Hovold; +Cc: git, Jeff King, Kevin Daudt, Junio C Hamano, Larry Finger
In-Reply-To: <20170217110642.GD2625@localhost>

Johan Hovold <johan@kernel.org> writes:

> There is another option, namely to only accept a single address for tags
> in the body. I understand that being able to copy a CC-header to either
> the header section or to the command line could be useful, but I don't
> really see the point in allowing this in the tags in the body (a SoB
> always has one address, and so should a CC-tag).

I mostly agree for the SoB, but why should a Cc tag have only one email?

The "multiple emails per Cc: field" has been there for a while already
(b1c8a11c8024 released in 2.6.0, sept 2015), some users may have got
used to it. What you are proposing breaks their flow.

> And since this is a regression for something that has been working for
> years that was introduced by a new feature, I also think it's reasonable
> to (partially) revert the feature.

I'd find it rather ironic to fix your case by breaking a feature that
has been working for more than a year :-(. What would you answer to a
contributor comming one year from now and proposing to revert your
reversion because it breaks his flow?

All that said, I think another fix would be both satisfactory for
everyone and rather simple:

1) Stop calling Mail::Address even if available. It used to make sense
   to do that when our in-house parser was really poor, but we now have
   something essentially as good as Mail::Address. We test our parser
   against Mail::Address and we do have a few known differences (see
   t9000), but they are really corner-cases and shouldn't matter.

   A good consequence of this is that we stop depending on the way Perl
   is installed to parse emails. Regardless of the current issue, I
   think it is a good thing.

2) Modify our in-house parser to discard garbage after the >. The patch
   should look like (untested):

--- a/perl/Git.pm
+++ b/perl/Git.pm
@@ -903,11 +903,11 @@ sub parse_mailboxes {
        my (@addr_list, @phrase, @address, @comment, @buffer) = ();
        foreach my $token (@tokens) {
                if ($token =~ /^[,;]$/) {
-                       # if buffer still contains undeterminated strings
-                       # append it at the end of @address or @phrase
-                       if ($end_of_addr_seen) {
-                               push @phrase, @buffer;
-                       } else {
+                       # if buffer still contains undeterminated
+                       # strings append it at the end of @address,
+                       # unless we already saw the closing >, in
+                       # which case we discard it.
+                       if (!$end_of_addr_seen) {
                                push @address, @buffer;
                        }
 
What do you think?

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

^ permalink raw reply

* Re: git alias for options
From: Ævar Arnfjörð Bjarmason @ 2017-02-17 13:50 UTC (permalink / raw)
  To: hIpPy; +Cc: Git Mailing List
In-Reply-To: <CAM_JFCz+9mxp37BTT7XPJ0fMd41DdbAxnvQF7id9msH+SDe6_Q@mail.gmail.com>

On Fri, Feb 17, 2017 at 9:23 AM, hIpPy <hippy2981@gmail.com> wrote:
> Git has aliases for git commands. Is there a (an inbuilt) way to alias
> options? If not, what is the reason?

This has long been on my  wishlist, because there's a lot of
copy/pasted logic all over Git to make git foo --whatever aliased to
foo.whatever in the config, but only for some options.

It should ideally be part of something every option just supports, via
the getopts struct.

However, it can't allow you to modify whatever option you want,
because some things like git-commit-tree should not be customized
based on config, it would break things that rely on the plumbing
commands.

So it would have to be a whitelist for each option we allow to be
configured like this via the getopts struct.

Also there are surely other edge cases, like maybe the config option
now doesn't 1=1 map to the name for the option in some cases, or the
flag should be config-able but is has no long form (which we'd like
for the config), then we'd want to add that etc.

^ permalink raw reply

* [PATCH v3 01/16] refs-internal.c: make files_log_ref_write() static
From: Nguyễn Thái Ngọc Duy @ 2017-02-17 14:04 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, Michael Haggerty, Johannes Schindelin,
	Ramsay Jones, Stefan Beller, novalis,
	Nguyễn Thái Ngọc Duy
In-Reply-To: <20170217140436.17336-1-pclouds@gmail.com>

Created in 5f3c3a4e6f (files_log_ref_write: new function - 2015-11-10)
but probably never used outside refs-internal.c

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

diff --git a/refs/files-backend.c b/refs/files-backend.c
index cdb6b8ff5..75565c3aa 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -165,6 +165,9 @@ static struct ref_entry *create_dir_entry(struct files_ref_store *ref_store,
 					  const char *dirname, size_t len,
 					  int incomplete);
 static void add_entry_to_dir(struct ref_dir *dir, struct ref_entry *entry);
+static int files_log_ref_write(const char *refname, const unsigned char *old_sha1,
+			       const unsigned char *new_sha1, const char *msg,
+			       int flags, struct strbuf *err);
 
 static struct ref_dir *get_ref_dir(struct ref_entry *entry)
 {
diff --git a/refs/refs-internal.h b/refs/refs-internal.h
index 33adbf93b..59e65958a 100644
--- a/refs/refs-internal.h
+++ b/refs/refs-internal.h
@@ -222,10 +222,6 @@ struct ref_transaction {
 	enum ref_transaction_state state;
 };
 
-int files_log_ref_write(const char *refname, const unsigned char *old_sha1,
-			const unsigned char *new_sha1, const char *msg,
-			int flags, struct strbuf *err);
-
 /*
  * Check for entries in extras that are within the specified
  * directory, where dirname is a reference directory name including
-- 
2.11.0.157.gd943d85


^ permalink raw reply related

* [PATCH v3 02/16] files-backend: convert git_path() to strbuf_git_path()
From: Nguyễn Thái Ngọc Duy @ 2017-02-17 14:04 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, Michael Haggerty, Johannes Schindelin,
	Ramsay Jones, Stefan Beller, novalis,
	Nguyễn Thái Ngọc Duy
In-Reply-To: <20170217140436.17336-1-pclouds@gmail.com>

git_path() and friends are going to be killed in files-backend.c in near
future. And because there's a risk with overwriting buffer in
git_path(), let's convert them all to strbuf_git_path(). We'll have
easier time killing/converting strbuf_git_path() then because we won't
have to worry about memory management again.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 refs/files-backend.c | 111 +++++++++++++++++++++++++++++++++++++++++----------
 1 file changed, 89 insertions(+), 22 deletions(-)

diff --git a/refs/files-backend.c b/refs/files-backend.c
index 75565c3aa..f0c878b92 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -2169,6 +2169,8 @@ static int lock_packed_refs(struct files_ref_store *refs, int flags)
 	static int timeout_configured = 0;
 	static int timeout_value = 1000;
 	struct packed_ref_cache *packed_ref_cache;
+	struct strbuf sb = STRBUF_INIT;
+	int ret;
 
 	files_assert_main_repository(refs, "lock_packed_refs");
 
@@ -2177,10 +2179,13 @@ static int lock_packed_refs(struct files_ref_store *refs, int flags)
 		timeout_configured = 1;
 	}
 
-	if (hold_lock_file_for_update_timeout(
-			    &packlock, git_path("packed-refs"),
-			    flags, timeout_value) < 0)
+	strbuf_git_path(&sb, "packed-refs");
+	ret = hold_lock_file_for_update_timeout(&packlock, sb.buf,
+						flags, timeout_value);
+	strbuf_release(&sb);
+	if (ret < 0)
 		return -1;
+
 	/*
 	 * Get the current packed-refs while holding the lock.  If the
 	 * packed-refs file has been modified since we last read it,
@@ -2335,6 +2340,9 @@ static void try_remove_empty_parents(char *name)
 	for (q = p; *q; q++)
 		;
 	while (1) {
+		struct strbuf sb = STRBUF_INIT;
+		int ret;
+
 		while (q > p && *q != '/')
 			q--;
 		while (q > p && *(q-1) == '/')
@@ -2342,7 +2350,10 @@ static void try_remove_empty_parents(char *name)
 		if (q == p)
 			break;
 		*q = '\0';
-		if (rmdir(git_path("%s", name)))
+		strbuf_git_path(&sb, "%s", name);
+		ret = rmdir(sb.buf);
+		strbuf_release(&sb);
+		if (ret)
 			break;
 	}
 }
@@ -2431,7 +2442,11 @@ static int repack_without_refs(struct files_ref_store *refs,
 		return 0; /* no refname exists in packed refs */
 
 	if (lock_packed_refs(refs, 0)) {
-		unable_to_lock_message(git_path("packed-refs"), errno, err);
+		struct strbuf sb = STRBUF_INIT;
+
+		strbuf_git_path(&sb, "packed-refs");
+		unable_to_lock_message(sb.buf, errno, err);
+		strbuf_release(&sb);
 		return -1;
 	}
 	packed = get_packed_refs(refs);
@@ -2529,8 +2544,10 @@ static int rename_tmp_log(const char *newrefname)
 {
 	int attempts_remaining = 4;
 	struct strbuf path = STRBUF_INIT;
+	struct strbuf tmp_renamed_log = STRBUF_INIT;
 	int ret = -1;
 
+	strbuf_git_path(&tmp_renamed_log, TMP_RENAMED_LOG);
  retry:
 	strbuf_reset(&path);
 	strbuf_git_path(&path, "logs/%s", newrefname);
@@ -2546,7 +2563,7 @@ static int rename_tmp_log(const char *newrefname)
 		goto out;
 	}
 
-	if (rename(git_path(TMP_RENAMED_LOG), path.buf)) {
+	if (rename(tmp_renamed_log.buf, path.buf)) {
 		if ((errno==EISDIR || errno==ENOTDIR) && --attempts_remaining > 0) {
 			/*
 			 * rename(a, b) when b is an existing
@@ -2574,6 +2591,7 @@ static int rename_tmp_log(const char *newrefname)
 	ret = 0;
 out:
 	strbuf_release(&path);
+	strbuf_release(&tmp_renamed_log);
 	return ret;
 }
 
@@ -2614,9 +2632,15 @@ static int files_rename_ref(struct ref_store *ref_store,
 	int flag = 0, logmoved = 0;
 	struct ref_lock *lock;
 	struct stat loginfo;
-	int log = !lstat(git_path("logs/%s", oldrefname), &loginfo);
+	struct strbuf sb_oldref = STRBUF_INIT;
+	struct strbuf sb_newref = STRBUF_INIT;
+	struct strbuf tmp_renamed_log = STRBUF_INIT;
+	int log, ret;
 	struct strbuf err = STRBUF_INIT;
 
+	strbuf_git_path(&sb_oldref, "logs/%s", oldrefname);
+	log = !lstat(sb_oldref.buf, &loginfo);
+	strbuf_release(&sb_oldref);
 	if (log && S_ISLNK(loginfo.st_mode))
 		return error("reflog for %s is a symlink", oldrefname);
 
@@ -2630,7 +2654,12 @@ static int files_rename_ref(struct ref_store *ref_store,
 	if (!rename_ref_available(oldrefname, newrefname))
 		return 1;
 
-	if (log && rename(git_path("logs/%s", oldrefname), git_path(TMP_RENAMED_LOG)))
+	strbuf_git_path(&sb_oldref, "logs/%s", oldrefname);
+	strbuf_git_path(&tmp_renamed_log, TMP_RENAMED_LOG);
+	ret = log && rename(sb_oldref.buf, tmp_renamed_log.buf);
+	strbuf_release(&sb_oldref);
+	strbuf_release(&tmp_renamed_log);
+	if (ret)
 		return error("unable to move logfile logs/%s to "TMP_RENAMED_LOG": %s",
 			oldrefname, strerror(errno));
 
@@ -2709,13 +2738,19 @@ static int files_rename_ref(struct ref_store *ref_store,
 	log_all_ref_updates = flag;
 
  rollbacklog:
-	if (logmoved && rename(git_path("logs/%s", newrefname), git_path("logs/%s", oldrefname)))
+	strbuf_git_path(&sb_newref, "logs/%s", newrefname);
+	strbuf_git_path(&sb_oldref, "logs/%s", oldrefname);
+	if (logmoved && rename(sb_newref.buf, sb_oldref.buf))
 		error("unable to restore logfile %s from %s: %s",
 			oldrefname, newrefname, strerror(errno));
+	strbuf_git_path(&tmp_renamed_log, TMP_RENAMED_LOG);
 	if (!logmoved && log &&
-	    rename(git_path(TMP_RENAMED_LOG), git_path("logs/%s", oldrefname)))
+	    rename(tmp_renamed_log.buf, sb_oldref.buf))
 		error("unable to restore logfile %s from "TMP_RENAMED_LOG": %s",
 			oldrefname, strerror(errno));
+	strbuf_release(&sb_newref);
+	strbuf_release(&sb_oldref);
+	strbuf_release(&tmp_renamed_log);
 
 	return 1;
 }
@@ -3111,22 +3146,32 @@ int set_worktree_head_symref(const char *gitdir, const char *target)
 static int files_reflog_exists(struct ref_store *ref_store,
 			       const char *refname)
 {
+	struct strbuf sb = STRBUF_INIT;
 	struct stat st;
+	int ret;
 
 	/* Check validity (but we don't need the result): */
 	files_downcast(ref_store, 0, "reflog_exists");
 
-	return !lstat(git_path("logs/%s", refname), &st) &&
-		S_ISREG(st.st_mode);
+	strbuf_git_path(&sb, "logs/%s", refname);
+	ret = !lstat(sb.buf, &st) && S_ISREG(st.st_mode);
+	strbuf_release(&sb);
+	return ret;
 }
 
 static int files_delete_reflog(struct ref_store *ref_store,
 			       const char *refname)
 {
+	struct strbuf sb = STRBUF_INIT;
+	int ret;
+
 	/* Check validity (but we don't need the result): */
 	files_downcast(ref_store, 0, "delete_reflog");
 
-	return remove_path(git_path("logs/%s", refname));
+	strbuf_git_path(&sb, "logs/%s", refname);
+	ret = remove_path(sb.buf);
+	strbuf_release(&sb);
+	return ret;
 }
 
 static int show_one_reflog_ent(struct strbuf *sb, each_reflog_ent_fn fn, void *cb_data)
@@ -3181,7 +3226,9 @@ static int files_for_each_reflog_ent_reverse(struct ref_store *ref_store,
 	/* Check validity (but we don't need the result): */
 	files_downcast(ref_store, 0, "for_each_reflog_ent_reverse");
 
-	logfp = fopen(git_path("logs/%s", refname), "r");
+	strbuf_git_path(&sb, "logs/%s", refname);
+	logfp = fopen(sb.buf, "r");
+	strbuf_release(&sb);
 	if (!logfp)
 		return -1;
 
@@ -3287,7 +3334,9 @@ static int files_for_each_reflog_ent(struct ref_store *ref_store,
 	/* Check validity (but we don't need the result): */
 	files_downcast(ref_store, 0, "for_each_reflog_ent");
 
-	logfp = fopen(git_path("logs/%s", refname), "r");
+	strbuf_git_path(&sb, "logs/%s", refname);
+	logfp = fopen(sb.buf, "r");
+	strbuf_release(&sb);
 	if (!logfp)
 		return -1;
 
@@ -3369,12 +3418,15 @@ static struct ref_iterator *files_reflog_iterator_begin(struct ref_store *ref_st
 {
 	struct files_reflog_iterator *iter = xcalloc(1, sizeof(*iter));
 	struct ref_iterator *ref_iterator = &iter->base;
+	struct strbuf sb = STRBUF_INIT;
 
 	/* Check validity (but we don't need the result): */
 	files_downcast(ref_store, 0, "reflog_iterator_begin");
 
 	base_ref_iterator_init(ref_iterator, &files_reflog_iterator_vtable);
-	iter->dir_iterator = dir_iterator_begin(git_path("logs"));
+	strbuf_git_path(&sb, "logs");
+	iter->dir_iterator = dir_iterator_begin(sb.buf);
+	strbuf_release(&sb);
 	return ref_iterator;
 }
 
@@ -3843,8 +3895,13 @@ static int files_transaction_commit(struct ref_store *ref_store,
 		ret = TRANSACTION_GENERIC_ERROR;
 		goto cleanup;
 	}
-	for_each_string_list_item(ref_to_delete, &refs_to_delete)
-		unlink_or_warn(git_path("logs/%s", ref_to_delete->string));
+	for_each_string_list_item(ref_to_delete, &refs_to_delete) {
+		struct strbuf sb = STRBUF_INIT;
+
+		strbuf_git_path(&sb, "logs/%s", ref_to_delete->string);
+		unlink_or_warn(sb.buf);
+		strbuf_release(&sb);
+	}
 	clear_loose_ref_cache(refs);
 
 cleanup:
@@ -4098,18 +4155,28 @@ static int files_reflog_expire(struct ref_store *ref_store,
 
 static int files_init_db(struct ref_store *ref_store, struct strbuf *err)
 {
+	struct strbuf sb = STRBUF_INIT;
+
 	/* Check validity (but we don't need the result): */
 	files_downcast(ref_store, 0, "init_db");
 
 	/*
 	 * Create .git/refs/{heads,tags}
 	 */
-	safe_create_dir(git_path("refs/heads"), 1);
-	safe_create_dir(git_path("refs/tags"), 1);
+	strbuf_git_path(&sb, "refs/heads");
+	safe_create_dir(sb.buf, 1);
+	strbuf_reset(&sb);
+	strbuf_git_path(&sb, "refs/tags");
+	safe_create_dir(sb.buf, 1);
+	strbuf_reset(&sb);
 	if (get_shared_repository()) {
-		adjust_shared_perm(git_path("refs/heads"));
-		adjust_shared_perm(git_path("refs/tags"));
+		strbuf_git_path(&sb, "refs/heads");
+		adjust_shared_perm(sb.buf);
+		strbuf_reset(&sb);
+		strbuf_git_path(&sb, "refs/tags");
+		adjust_shared_perm(sb.buf);
 	}
+	strbuf_release(&sb);
 	return 0;
 }
 
-- 
2.11.0.157.gd943d85


^ permalink raw reply related

* [PATCH v3 03/16] files-backend: add files_path()
From: Nguyễn Thái Ngọc Duy @ 2017-02-17 14:04 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, Michael Haggerty, Johannes Schindelin,
	Ramsay Jones, Stefan Beller, novalis,
	Nguyễn Thái Ngọc Duy
In-Reply-To: <20170217140436.17336-1-pclouds@gmail.com>

This will be the replacement for both git_path() and git_path_submodule()
in this file. The idea is backend takes a git path and use that,
oblivious of submodule, linked worktrees and such.

This is the middle step towards that. Eventually the "submodule" field
in 'struct files_ref_store' should be replace by "gitdir". And a
compound ref_store is created to combine two files backends together,
one represents the shared refs in $GIT_COMMON_DIR, one per-worktree. At
that point, files_path() becomes a wrapper of strbuf_vaddf().

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 refs/files-backend.c | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/refs/files-backend.c b/refs/files-backend.c
index f0c878b92..abb8a95e0 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -930,6 +930,24 @@ struct files_ref_store {
 /* Lock used for the main packed-refs file: */
 static struct lock_file packlock;
 
+__attribute__((format (printf, 3, 4)))
+static void files_path(struct files_ref_store *refs, struct strbuf *sb,
+		       const char *fmt, ...)
+{
+	struct strbuf tmp = STRBUF_INIT;
+	va_list vap;
+
+	va_start(vap, fmt);
+	strbuf_vaddf(&tmp, fmt, vap);
+	va_end(vap);
+	if (refs->submodule)
+		strbuf_git_path_submodule(sb, refs->submodule,
+					  "%s", tmp.buf);
+	else
+		strbuf_git_path(sb, "%s", tmp.buf);
+	strbuf_release(&tmp);
+}
+
 /*
  * Increment the reference count of *packed_refs.
  */
-- 
2.11.0.157.gd943d85


^ permalink raw reply related

* [PATCH v3 00/16] Remove submodule from files-backend.c
From: Nguyễn Thái Ngọc Duy @ 2017-02-17 14:04 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, Michael Haggerty, Johannes Schindelin,
	Ramsay Jones, Stefan Beller, novalis,
	Nguyễn Thái Ngọc Duy
In-Reply-To: <20170216114818.6080-1-pclouds@gmail.com>

v3 only changes 07/16 but it's kinda important because I broke
packed-refs. packed-refs' path became $GIT_DIR/packed-refs instead of
$GIT_COMMON_DIR/packed-refs and as a result the majority of refs will
disappear in linked worktrees. Interdiff

diff --git a/refs/files-backend.c b/refs/files-backend.c
index 685ea5c14..82be3f90f 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -938,9 +938,11 @@ static void files_path(struct files_ref_store *refs, struct strbuf *sb,
 	va_start(vap, fmt);
 	strbuf_vaddf(&tmp, fmt, vap);
 	va_end(vap);
-	if (is_per_worktree_ref(tmp.buf) ||
-	    (skip_prefix(tmp.buf, "logs/", &ref) &&
-	     is_per_worktree_ref(ref)))
+	if (!strcmp(tmp.buf, "packed-refs") || !strcmp(tmp.buf, "logs"))
+		strbuf_addf(sb, "%s/%s", refs->gitcommondir.buf, tmp.buf);
+	else if (is_per_worktree_ref(tmp.buf) ||
+		 (skip_prefix(tmp.buf, "logs/", &ref) &&
+		  is_per_worktree_ref(ref)))
 		strbuf_addf(sb, "%s/%s", refs->gitdir.buf, tmp.buf);
 	else
 		strbuf_addf(sb, "%s/%s", refs->gitcommondir.buf, tmp.buf);

Nguyễn Thái Ngọc Duy (16):
  refs-internal.c: make files_log_ref_write() static
  files-backend: convert git_path() to strbuf_git_path()
  files-backend: add files_path()
  files-backend: replace *git_path*() with files_path()
  refs.c: share is_per_worktree_ref() to files-backend.c
  refs-internal.h: correct is_per_worktree_ref()
  files-backend: remove the use of git_path()
  refs.c: introduce get_main_ref_store()
  refs: rename lookup_ref_store() to lookup_submodule_ref_store()
  refs.c: flatten get_ref_store() a bit
  refs.c: kill register_ref_store(), add register_submodule_ref_store()
  refs.c: make get_main_ref_store() public and use it
  path.c: move some code out of strbuf_git_path_submodule()
  refs: move submodule code out of files-backend.c
  files-backend: remove submodule_allowed from files_downcast()
  refs: rename get_ref_store() to get_submodule_ref_store() and make it public

 path.c               |  34 ++---
 refs.c               | 144 +++++++++++----------
 refs.h               |  13 ++
 refs/files-backend.c | 349 +++++++++++++++++++++++++++++++--------------------
 refs/refs-internal.h |  28 ++---
 submodule.c          |  31 +++++
 submodule.h          |   1 +
 7 files changed, 348 insertions(+), 252 deletions(-)

-- 
2.11.0.157.gd943d85


^ permalink raw reply related

* [PATCH v3 04/16] files-backend: replace *git_path*() with files_path()
From: Nguyễn Thái Ngọc Duy @ 2017-02-17 14:04 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, Michael Haggerty, Johannes Schindelin,
	Ramsay Jones, Stefan Beller, novalis,
	Nguyễn Thái Ngọc Duy
In-Reply-To: <20170217140436.17336-1-pclouds@gmail.com>

This centralizes all path rewriting of files-backend.c in one place so
we have easier time removing the path rewriting later. There could be
some hidden indirect git_path() though, I didn't audit the code to the
bottom.

Side note: set_worktree_head_symref() is a bad boy and should not be in
files-backend.c (probably should not exist in the first place). But
we'll leave it there until we have better multi-worktree support in refs
before we update it.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 refs/files-backend.c | 185 ++++++++++++++++++++++++++-------------------------
 1 file changed, 94 insertions(+), 91 deletions(-)

diff --git a/refs/files-backend.c b/refs/files-backend.c
index abb8a95e0..24f5bf7f1 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -165,7 +165,8 @@ static struct ref_entry *create_dir_entry(struct files_ref_store *ref_store,
 					  const char *dirname, size_t len,
 					  int incomplete);
 static void add_entry_to_dir(struct ref_dir *dir, struct ref_entry *entry);
-static int files_log_ref_write(const char *refname, const unsigned char *old_sha1,
+static int files_log_ref_write(struct files_ref_store *refs,
+			       const char *refname, const unsigned char *old_sha1,
 			       const unsigned char *new_sha1, const char *msg,
 			       int flags, struct strbuf *err);
 
@@ -1178,12 +1179,10 @@ static void read_packed_refs(FILE *f, struct ref_dir *dir)
 static struct packed_ref_cache *get_packed_ref_cache(struct files_ref_store *refs)
 {
 	char *packed_refs_file;
+	struct strbuf sb = STRBUF_INIT;
 
-	if (refs->submodule)
-		packed_refs_file = git_pathdup_submodule(refs->submodule,
-							 "packed-refs");
-	else
-		packed_refs_file = git_pathdup("packed-refs");
+	files_path(refs, &sb, "packed-refs");
+	packed_refs_file = strbuf_detach(&sb, NULL);
 
 	if (refs->packed &&
 	    !stat_validity_check(&refs->packed->validity, packed_refs_file))
@@ -1249,10 +1248,7 @@ static void read_loose_refs(const char *dirname, struct ref_dir *dir)
 	size_t path_baselen;
 	int err = 0;
 
-	if (refs->submodule)
-		err = strbuf_git_path_submodule(&path, refs->submodule, "%s", dirname);
-	else
-		strbuf_git_path(&path, "%s", dirname);
+	files_path(refs, &path, "%s", dirname);
 	path_baselen = path.len;
 
 	if (err) {
@@ -1394,10 +1390,7 @@ static int files_read_raw_ref(struct ref_store *ref_store,
 	*type = 0;
 	strbuf_reset(&sb_path);
 
-	if (refs->submodule)
-		strbuf_git_path_submodule(&sb_path, refs->submodule, "%s", refname);
-	else
-		strbuf_git_path(&sb_path, "%s", refname);
+	files_path(refs, &sb_path, "%s", refname);
 
 	path = sb_path.buf;
 
@@ -1585,7 +1578,7 @@ static int lock_raw_ref(struct files_ref_store *refs,
 	*lock_p = lock = xcalloc(1, sizeof(*lock));
 
 	lock->ref_name = xstrdup(refname);
-	strbuf_git_path(&ref_file, "%s", refname);
+	files_path(refs, &ref_file, "%s", refname);
 
 retry:
 	switch (safe_create_leading_directories(ref_file.buf)) {
@@ -2052,7 +2045,7 @@ static struct ref_lock *lock_ref_sha1_basic(struct files_ref_store *refs,
 	if (flags & REF_DELETING)
 		resolve_flags |= RESOLVE_REF_ALLOW_BAD_NAME;
 
-	strbuf_git_path(&ref_file, "%s", refname);
+	files_path(refs, &ref_file, "%s", refname);
 	resolved = !!resolve_ref_unsafe(refname, resolve_flags,
 					lock->old_oid.hash, type);
 	if (!resolved && errno == EISDIR) {
@@ -2197,7 +2190,7 @@ static int lock_packed_refs(struct files_ref_store *refs, int flags)
 		timeout_configured = 1;
 	}
 
-	strbuf_git_path(&sb, "packed-refs");
+	files_path(refs, &sb, "packed-refs");
 	ret = hold_lock_file_for_update_timeout(&packlock, sb.buf,
 						flags, timeout_value);
 	strbuf_release(&sb);
@@ -2343,7 +2336,7 @@ static int pack_if_possible_fn(struct ref_entry *entry, void *cb_data)
  * Remove empty parents, but spare refs/ and immediate subdirs.
  * Note: munges *name.
  */
-static void try_remove_empty_parents(char *name)
+static void try_remove_empty_parents(struct files_ref_store *refs, char *name)
 {
 	char *p, *q;
 	int i;
@@ -2368,7 +2361,7 @@ static void try_remove_empty_parents(char *name)
 		if (q == p)
 			break;
 		*q = '\0';
-		strbuf_git_path(&sb, "%s", name);
+		files_path(refs, &sb, "%s", name);
 		ret = rmdir(sb.buf);
 		strbuf_release(&sb);
 		if (ret)
@@ -2377,7 +2370,7 @@ static void try_remove_empty_parents(char *name)
 }
 
 /* make sure nobody touched the ref, and unlink */
-static void prune_ref(struct ref_to_prune *r)
+static void prune_ref(struct files_ref_store *refs, struct ref_to_prune *r)
 {
 	struct ref_transaction *transaction;
 	struct strbuf err = STRBUF_INIT;
@@ -2397,13 +2390,13 @@ static void prune_ref(struct ref_to_prune *r)
 	}
 	ref_transaction_free(transaction);
 	strbuf_release(&err);
-	try_remove_empty_parents(r->name);
+	try_remove_empty_parents(refs, r->name);
 }
 
-static void prune_refs(struct ref_to_prune *r)
+static void prune_refs(struct files_ref_store *refs, struct ref_to_prune *r)
 {
 	while (r) {
-		prune_ref(r);
+		prune_ref(refs, r);
 		r = r->next;
 	}
 }
@@ -2426,7 +2419,7 @@ static int files_pack_refs(struct ref_store *ref_store, unsigned int flags)
 	if (commit_packed_refs(refs))
 		die_errno("unable to overwrite old ref-pack file");
 
-	prune_refs(cbdata.ref_to_prune);
+	prune_refs(refs, cbdata.ref_to_prune);
 	return 0;
 }
 
@@ -2462,7 +2455,7 @@ static int repack_without_refs(struct files_ref_store *refs,
 	if (lock_packed_refs(refs, 0)) {
 		struct strbuf sb = STRBUF_INIT;
 
-		strbuf_git_path(&sb, "packed-refs");
+		files_path(refs, &sb, "packed-refs");
 		unable_to_lock_message(sb.buf, errno, err);
 		strbuf_release(&sb);
 		return -1;
@@ -2558,17 +2551,17 @@ static int files_delete_refs(struct ref_store *ref_store,
  */
 #define TMP_RENAMED_LOG  "logs/refs/.tmp-renamed-log"
 
-static int rename_tmp_log(const char *newrefname)
+static int rename_tmp_log(struct files_ref_store *refs, const char *newrefname)
 {
 	int attempts_remaining = 4;
 	struct strbuf path = STRBUF_INIT;
 	struct strbuf tmp_renamed_log = STRBUF_INIT;
 	int ret = -1;
 
-	strbuf_git_path(&tmp_renamed_log, TMP_RENAMED_LOG);
+	files_path(refs, &tmp_renamed_log, TMP_RENAMED_LOG);
  retry:
 	strbuf_reset(&path);
-	strbuf_git_path(&path, "logs/%s", newrefname);
+	files_path(refs, &path, "logs/%s", newrefname);
 	switch (safe_create_leading_directories_const(path.buf)) {
 	case SCLD_OK:
 		break; /* success */
@@ -2656,7 +2649,7 @@ static int files_rename_ref(struct ref_store *ref_store,
 	int log, ret;
 	struct strbuf err = STRBUF_INIT;
 
-	strbuf_git_path(&sb_oldref, "logs/%s", oldrefname);
+	files_path(refs, &sb_oldref, "logs/%s", oldrefname);
 	log = !lstat(sb_oldref.buf, &loginfo);
 	strbuf_release(&sb_oldref);
 	if (log && S_ISLNK(loginfo.st_mode))
@@ -2672,8 +2665,8 @@ static int files_rename_ref(struct ref_store *ref_store,
 	if (!rename_ref_available(oldrefname, newrefname))
 		return 1;
 
-	strbuf_git_path(&sb_oldref, "logs/%s", oldrefname);
-	strbuf_git_path(&tmp_renamed_log, TMP_RENAMED_LOG);
+	files_path(refs, &sb_oldref, "logs/%s", oldrefname);
+	files_path(refs, &tmp_renamed_log, TMP_RENAMED_LOG);
 	ret = log && rename(sb_oldref.buf, tmp_renamed_log.buf);
 	strbuf_release(&sb_oldref);
 	strbuf_release(&tmp_renamed_log);
@@ -2700,7 +2693,7 @@ static int files_rename_ref(struct ref_store *ref_store,
 			struct strbuf path = STRBUF_INIT;
 			int result;
 
-			strbuf_git_path(&path, "%s", newrefname);
+			files_path(refs, &path, "%s", newrefname);
 			result = remove_empty_directories(&path);
 			strbuf_release(&path);
 
@@ -2714,7 +2707,7 @@ static int files_rename_ref(struct ref_store *ref_store,
 		}
 	}
 
-	if (log && rename_tmp_log(newrefname))
+	if (log && rename_tmp_log(refs, newrefname))
 		goto rollback;
 
 	logmoved = log;
@@ -2756,12 +2749,12 @@ static int files_rename_ref(struct ref_store *ref_store,
 	log_all_ref_updates = flag;
 
  rollbacklog:
-	strbuf_git_path(&sb_newref, "logs/%s", newrefname);
-	strbuf_git_path(&sb_oldref, "logs/%s", oldrefname);
+	files_path(refs, &sb_newref, "logs/%s", newrefname);
+	files_path(refs, &sb_oldref, "logs/%s", oldrefname);
 	if (logmoved && rename(sb_newref.buf, sb_oldref.buf))
 		error("unable to restore logfile %s from %s: %s",
 			oldrefname, newrefname, strerror(errno));
-	strbuf_git_path(&tmp_renamed_log, TMP_RENAMED_LOG);
+	files_path(refs, &tmp_renamed_log, TMP_RENAMED_LOG);
 	if (!logmoved && log &&
 	    rename(tmp_renamed_log.buf, sb_oldref.buf))
 		error("unable to restore logfile %s from "TMP_RENAMED_LOG": %s",
@@ -2817,11 +2810,13 @@ static int commit_ref(struct ref_lock *lock)
  * should_autocreate_reflog returns non-zero.  Otherwise, create it
  * regardless of the ref name.  Fill in *err and return -1 on failure.
  */
-static int log_ref_setup(const char *refname, struct strbuf *logfile, struct strbuf *err, int force_create)
+static int log_ref_setup(struct files_ref_store *refs, const char *refname,
+			 struct strbuf *logfile, struct strbuf *err,
+			 int force_create)
 {
 	int logfd, oflags = O_APPEND | O_WRONLY;
 
-	strbuf_git_path(logfile, "logs/%s", refname);
+	files_path(refs, logfile, "logs/%s", refname);
 	if (force_create || should_autocreate_reflog(refname)) {
 		if (safe_create_leading_directories(logfile->buf) < 0) {
 			strbuf_addf(err, "unable to create directory for '%s': "
@@ -2864,11 +2859,10 @@ static int files_create_reflog(struct ref_store *ref_store,
 {
 	int ret;
 	struct strbuf sb = STRBUF_INIT;
+	struct files_ref_store *refs =
+		files_downcast(ref_store, 0, "create_reflog");
 
-	/* Check validity (but we don't need the result): */
-	files_downcast(ref_store, 0, "create_reflog");
-
-	ret = log_ref_setup(refname, &sb, err, force_create);
+	ret = log_ref_setup(refs, refname, &sb, err, force_create);
 	strbuf_release(&sb);
 	return ret;
 }
@@ -2899,7 +2893,8 @@ static int log_ref_write_fd(int fd, const unsigned char *old_sha1,
 	return 0;
 }
 
-static int log_ref_write_1(const char *refname, const unsigned char *old_sha1,
+static int log_ref_write_1(struct files_ref_store *refs,
+			   const char *refname, const unsigned char *old_sha1,
 			   const unsigned char *new_sha1, const char *msg,
 			   struct strbuf *logfile, int flags,
 			   struct strbuf *err)
@@ -2909,7 +2904,7 @@ static int log_ref_write_1(const char *refname, const unsigned char *old_sha1,
 	if (log_all_ref_updates < 0)
 		log_all_ref_updates = !is_bare_repository();
 
-	result = log_ref_setup(refname, logfile, err, flags & REF_FORCE_CREATE_REFLOG);
+	result = log_ref_setup(refs, refname, logfile, err, flags & REF_FORCE_CREATE_REFLOG);
 
 	if (result)
 		return result;
@@ -2933,21 +2928,23 @@ static int log_ref_write_1(const char *refname, const unsigned char *old_sha1,
 	return 0;
 }
 
-static int log_ref_write(const char *refname, const unsigned char *old_sha1,
+static int log_ref_write(struct files_ref_store *refs,
+			 const char *refname, const unsigned char *old_sha1,
 			 const unsigned char *new_sha1, const char *msg,
 			 int flags, struct strbuf *err)
 {
-	return files_log_ref_write(refname, old_sha1, new_sha1, msg, flags,
-				   err);
+	return files_log_ref_write(refs, refname, old_sha1, new_sha1,
+				   msg, flags, err);
 }
 
-int files_log_ref_write(const char *refname, const unsigned char *old_sha1,
+int files_log_ref_write(struct files_ref_store *refs,
+			const char *refname, const unsigned char *old_sha1,
 			const unsigned char *new_sha1, const char *msg,
 			int flags, struct strbuf *err)
 {
 	struct strbuf sb = STRBUF_INIT;
-	int ret = log_ref_write_1(refname, old_sha1, new_sha1, msg, &sb, flags,
-				  err);
+	int ret = log_ref_write_1(refs, refname, old_sha1, new_sha1, msg,
+				  &sb, flags, err);
 	strbuf_release(&sb);
 	return ret;
 }
@@ -3004,7 +3001,8 @@ static int commit_ref_update(struct files_ref_store *refs,
 	files_assert_main_repository(refs, "commit_ref_update");
 
 	clear_loose_ref_cache(refs);
-	if (log_ref_write(lock->ref_name, lock->old_oid.hash, sha1, logmsg, 0, err)) {
+	if (log_ref_write(refs, lock->ref_name, lock->old_oid.hash,
+			  sha1, logmsg, 0, err)) {
 		char *old_msg = strbuf_detach(err, NULL);
 		strbuf_addf(err, "cannot update the ref '%s': %s",
 			    lock->ref_name, old_msg);
@@ -3035,8 +3033,8 @@ static int commit_ref_update(struct files_ref_store *refs,
 		if (head_ref && (head_flag & REF_ISSYMREF) &&
 		    !strcmp(head_ref, lock->ref_name)) {
 			struct strbuf log_err = STRBUF_INIT;
-			if (log_ref_write("HEAD", lock->old_oid.hash, sha1,
-					  logmsg, 0, &log_err)) {
+			if (log_ref_write(refs, "HEAD", lock->old_oid.hash,
+					  sha1, logmsg, 0, &log_err)) {
 				error("%s", log_err.buf);
 				strbuf_release(&log_err);
 			}
@@ -3068,23 +3066,26 @@ static int create_ref_symlink(struct ref_lock *lock, const char *target)
 	return ret;
 }
 
-static void update_symref_reflog(struct ref_lock *lock, const char *refname,
+static void update_symref_reflog(struct files_ref_store *refs,
+				 struct ref_lock *lock, const char *refname,
 				 const char *target, const char *logmsg)
 {
 	struct strbuf err = STRBUF_INIT;
 	unsigned char new_sha1[20];
 	if (logmsg && !read_ref(target, new_sha1) &&
-	    log_ref_write(refname, lock->old_oid.hash, new_sha1, logmsg, 0, &err)) {
+	    log_ref_write(refs, refname, lock->old_oid.hash,
+			  new_sha1, logmsg, 0, &err)) {
 		error("%s", err.buf);
 		strbuf_release(&err);
 	}
 }
 
-static int create_symref_locked(struct ref_lock *lock, const char *refname,
+static int create_symref_locked(struct files_ref_store *refs,
+				struct ref_lock *lock, const char *refname,
 				const char *target, const char *logmsg)
 {
 	if (prefer_symlink_refs && !create_ref_symlink(lock, target)) {
-		update_symref_reflog(lock, refname, target, logmsg);
+		update_symref_reflog(refs, lock, refname, target, logmsg);
 		return 0;
 	}
 
@@ -3092,7 +3093,7 @@ static int create_symref_locked(struct ref_lock *lock, const char *refname,
 		return error("unable to fdopen %s: %s",
 			     lock->lk->tempfile.filename.buf, strerror(errno));
 
-	update_symref_reflog(lock, refname, target, logmsg);
+	update_symref_reflog(refs, lock, refname, target, logmsg);
 
 	/* no error check; commit_ref will check ferror */
 	fprintf(lock->lk->tempfile.fp, "ref: %s\n", target);
@@ -3121,13 +3122,19 @@ static int files_create_symref(struct ref_store *ref_store,
 		return -1;
 	}
 
-	ret = create_symref_locked(lock, refname, target, logmsg);
+	ret = create_symref_locked(refs, lock, refname, target, logmsg);
 	unlock_ref(lock);
 	return ret;
 }
 
 int set_worktree_head_symref(const char *gitdir, const char *target)
 {
+	/*
+	 * FIXME: this obviously will not work well for future refs
+	 * backends. This function needs to die.
+	 */
+	struct files_ref_store *refs =
+		files_downcast(get_ref_store(NULL), 0, "set_head_symref");
 	static struct lock_file head_lock;
 	struct ref_lock *lock;
 	struct strbuf head_path = STRBUF_INIT;
@@ -3154,7 +3161,7 @@ int set_worktree_head_symref(const char *gitdir, const char *target)
 	lock->lk = &head_lock;
 	lock->ref_name = xstrdup(head_rel);
 
-	ret = create_symref_locked(lock, head_rel, target, NULL);
+	ret = create_symref_locked(refs, lock, head_rel, target, NULL);
 
 	unlock_ref(lock); /* will free lock */
 	strbuf_release(&head_path);
@@ -3164,14 +3171,13 @@ int set_worktree_head_symref(const char *gitdir, const char *target)
 static int files_reflog_exists(struct ref_store *ref_store,
 			       const char *refname)
 {
+	struct files_ref_store *refs =
+		files_downcast(ref_store, 0, "reflog_exists");
 	struct strbuf sb = STRBUF_INIT;
 	struct stat st;
 	int ret;
 
-	/* Check validity (but we don't need the result): */
-	files_downcast(ref_store, 0, "reflog_exists");
-
-	strbuf_git_path(&sb, "logs/%s", refname);
+	files_path(refs, &sb, "logs/%s", refname);
 	ret = !lstat(sb.buf, &st) && S_ISREG(st.st_mode);
 	strbuf_release(&sb);
 	return ret;
@@ -3180,13 +3186,12 @@ static int files_reflog_exists(struct ref_store *ref_store,
 static int files_delete_reflog(struct ref_store *ref_store,
 			       const char *refname)
 {
+	struct files_ref_store *refs =
+		files_downcast(ref_store, 0, "delete_reflog");
 	struct strbuf sb = STRBUF_INIT;
 	int ret;
 
-	/* Check validity (but we don't need the result): */
-	files_downcast(ref_store, 0, "delete_reflog");
-
-	strbuf_git_path(&sb, "logs/%s", refname);
+	files_path(refs, &sb, "logs/%s", refname);
 	ret = remove_path(sb.buf);
 	strbuf_release(&sb);
 	return ret;
@@ -3236,15 +3241,14 @@ static int files_for_each_reflog_ent_reverse(struct ref_store *ref_store,
 					     each_reflog_ent_fn fn,
 					     void *cb_data)
 {
+	struct files_ref_store *refs =
+		files_downcast(ref_store, 0, "for_each_reflog_ent_reverse");
 	struct strbuf sb = STRBUF_INIT;
 	FILE *logfp;
 	long pos;
 	int ret = 0, at_tail = 1;
 
-	/* Check validity (but we don't need the result): */
-	files_downcast(ref_store, 0, "for_each_reflog_ent_reverse");
-
-	strbuf_git_path(&sb, "logs/%s", refname);
+	files_path(refs, &sb, "logs/%s", refname);
 	logfp = fopen(sb.buf, "r");
 	strbuf_release(&sb);
 	if (!logfp)
@@ -3345,14 +3349,13 @@ static int files_for_each_reflog_ent(struct ref_store *ref_store,
 				     const char *refname,
 				     each_reflog_ent_fn fn, void *cb_data)
 {
+	struct files_ref_store *refs =
+		files_downcast(ref_store, 0, "for_each_reflog_ent");
 	FILE *logfp;
 	struct strbuf sb = STRBUF_INIT;
 	int ret = 0;
 
-	/* Check validity (but we don't need the result): */
-	files_downcast(ref_store, 0, "for_each_reflog_ent");
-
-	strbuf_git_path(&sb, "logs/%s", refname);
+	files_path(refs, &sb, "logs/%s", refname);
 	logfp = fopen(sb.buf, "r");
 	strbuf_release(&sb);
 	if (!logfp)
@@ -3434,15 +3437,14 @@ static struct ref_iterator_vtable files_reflog_iterator_vtable = {
 
 static struct ref_iterator *files_reflog_iterator_begin(struct ref_store *ref_store)
 {
+	struct files_ref_store *refs =
+		files_downcast(ref_store, 0, "reflog_iterator_begin");
 	struct files_reflog_iterator *iter = xcalloc(1, sizeof(*iter));
 	struct ref_iterator *ref_iterator = &iter->base;
 	struct strbuf sb = STRBUF_INIT;
 
-	/* Check validity (but we don't need the result): */
-	files_downcast(ref_store, 0, "reflog_iterator_begin");
-
 	base_ref_iterator_init(ref_iterator, &files_reflog_iterator_vtable);
-	strbuf_git_path(&sb, "logs");
+	files_path(refs, &sb, "logs");
 	iter->dir_iterator = dir_iterator_begin(sb.buf);
 	strbuf_release(&sb);
 	return ref_iterator;
@@ -3866,8 +3868,8 @@ static int files_transaction_commit(struct ref_store *ref_store,
 
 		if (update->flags & REF_NEEDS_COMMIT ||
 		    update->flags & REF_LOG_ONLY) {
-			if (log_ref_write(lock->ref_name, lock->old_oid.hash,
-					  update->new_sha1,
+			if (log_ref_write(refs, lock->ref_name,
+					  lock->old_oid.hash, update->new_sha1,
 					  update->msg, update->flags, err)) {
 				char *old_msg = strbuf_detach(err, NULL);
 
@@ -3916,7 +3918,7 @@ static int files_transaction_commit(struct ref_store *ref_store,
 	for_each_string_list_item(ref_to_delete, &refs_to_delete) {
 		struct strbuf sb = STRBUF_INIT;
 
-		strbuf_git_path(&sb, "logs/%s", ref_to_delete->string);
+		files_path(refs, &sb, "logs/%s", ref_to_delete->string);
 		unlink_or_warn(sb.buf);
 		strbuf_release(&sb);
 	}
@@ -4078,6 +4080,7 @@ static int files_reflog_expire(struct ref_store *ref_store,
 	int status = 0;
 	int type;
 	struct strbuf err = STRBUF_INIT;
+	struct strbuf sb = STRBUF_INIT;
 
 	memset(&cb, 0, sizeof(cb));
 	cb.flags = flags;
@@ -4102,7 +4105,8 @@ static int files_reflog_expire(struct ref_store *ref_store,
 		return 0;
 	}
 
-	log_file = git_pathdup("logs/%s", refname);
+	files_path(refs, &sb, "logs/%s", refname);
+	log_file = strbuf_detach(&sb, NULL);
 	if (!(flags & EXPIRE_REFLOGS_DRY_RUN)) {
 		/*
 		 * Even though holding $GIT_DIR/logs/$reflog.lock has
@@ -4173,25 +4177,24 @@ static int files_reflog_expire(struct ref_store *ref_store,
 
 static int files_init_db(struct ref_store *ref_store, struct strbuf *err)
 {
+	struct files_ref_store *refs =
+		files_downcast(ref_store, 0, "init_db");
 	struct strbuf sb = STRBUF_INIT;
 
-	/* Check validity (but we don't need the result): */
-	files_downcast(ref_store, 0, "init_db");
-
 	/*
 	 * Create .git/refs/{heads,tags}
 	 */
-	strbuf_git_path(&sb, "refs/heads");
+	files_path(refs, &sb, "refs/heads");
 	safe_create_dir(sb.buf, 1);
 	strbuf_reset(&sb);
-	strbuf_git_path(&sb, "refs/tags");
+	files_path(refs, &sb, "refs/tags");
 	safe_create_dir(sb.buf, 1);
 	strbuf_reset(&sb);
 	if (get_shared_repository()) {
-		strbuf_git_path(&sb, "refs/heads");
+		files_path(refs, &sb, "refs/heads");
 		adjust_shared_perm(sb.buf);
 		strbuf_reset(&sb);
-		strbuf_git_path(&sb, "refs/tags");
+		files_path(refs, &sb, "refs/tags");
 		adjust_shared_perm(sb.buf);
 	}
 	strbuf_release(&sb);
-- 
2.11.0.157.gd943d85


^ permalink raw reply related

* [PATCH v3 06/16] refs-internal.h: correct is_per_worktree_ref()
From: Nguyễn Thái Ngọc Duy @ 2017-02-17 14:04 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, Michael Haggerty, Johannes Schindelin,
	Ramsay Jones, Stefan Beller, novalis,
	Nguyễn Thái Ngọc Duy
In-Reply-To: <20170217140436.17336-1-pclouds@gmail.com>

All refs outside refs/ directory is per-worktree, not just HEAD.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 refs/refs-internal.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/refs/refs-internal.h b/refs/refs-internal.h
index f4aed49f5..69d02b6ba 100644
--- a/refs/refs-internal.h
+++ b/refs/refs-internal.h
@@ -653,7 +653,7 @@ const char *resolve_ref_recursively(struct ref_store *refs,
 
 static inline int is_per_worktree_ref(const char *refname)
 {
-	return !strcmp(refname, "HEAD") ||
+	return !starts_with(refname, "refs/") ||
 		starts_with(refname, "refs/bisect/");
 }
 
-- 
2.11.0.157.gd943d85


^ permalink raw reply related

* [PATCH v3 05/16] refs.c: share is_per_worktree_ref() to files-backend.c
From: Nguyễn Thái Ngọc Duy @ 2017-02-17 14:04 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, Michael Haggerty, Johannes Schindelin,
	Ramsay Jones, Stefan Beller, novalis,
	Nguyễn Thái Ngọc Duy
In-Reply-To: <20170217140436.17336-1-pclouds@gmail.com>

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

diff --git a/refs.c b/refs.c
index 4f845798b..7a474198e 100644
--- a/refs.c
+++ b/refs.c
@@ -489,12 +489,6 @@ int dwim_log(const char *str, int len, unsigned char *sha1, char **log)
 	return logs_found;
 }
 
-static int is_per_worktree_ref(const char *refname)
-{
-	return !strcmp(refname, "HEAD") ||
-		starts_with(refname, "refs/bisect/");
-}
-
 static int is_pseudoref_syntax(const char *refname)
 {
 	const char *c;
diff --git a/refs/refs-internal.h b/refs/refs-internal.h
index 59e65958a..f4aed49f5 100644
--- a/refs/refs-internal.h
+++ b/refs/refs-internal.h
@@ -651,4 +651,10 @@ const char *resolve_ref_recursively(struct ref_store *refs,
 				    int resolve_flags,
 				    unsigned char *sha1, int *flags);
 
+static inline int is_per_worktree_ref(const char *refname)
+{
+	return !strcmp(refname, "HEAD") ||
+		starts_with(refname, "refs/bisect/");
+}
+
 #endif /* REFS_REFS_INTERNAL_H */
-- 
2.11.0.157.gd943d85


^ permalink raw reply related

* [PATCH v3 07/16] files-backend: remove the use of git_path()
From: Nguyễn Thái Ngọc Duy @ 2017-02-17 14:04 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, Michael Haggerty, Johannes Schindelin,
	Ramsay Jones, Stefan Beller, novalis,
	Nguyễn Thái Ngọc Duy
In-Reply-To: <20170217140436.17336-1-pclouds@gmail.com>

Given $GIT_DIR and $GIT_COMMON_DIR, files-backend is now in charge of
deciding what goes where. The end goal is to pass $GIT_DIR only. A
refs "view" of a linked worktree is a logical ref store that combines
two files backends together.

(*) Not entirely true since strbuf_git_path_submodule() still does path
translation underneath. But that's for another patch.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 refs/files-backend.c | 22 ++++++++++++++++++++--
 1 file changed, 20 insertions(+), 2 deletions(-)

diff --git a/refs/files-backend.c b/refs/files-backend.c
index 24f5bf7f1..07cf2cb93 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -924,6 +924,9 @@ struct files_ref_store {
 	 */
 	const char *submodule;
 
+	struct strbuf gitdir;
+	struct strbuf gitcommondir;
+
 	struct ref_entry *loose;
 	struct packed_ref_cache *packed;
 };
@@ -937,6 +940,7 @@ static void files_path(struct files_ref_store *refs, struct strbuf *sb,
 {
 	struct strbuf tmp = STRBUF_INIT;
 	va_list vap;
+	const char *ref;
 
 	va_start(vap, fmt);
 	strbuf_vaddf(&tmp, fmt, vap);
@@ -944,8 +948,14 @@ static void files_path(struct files_ref_store *refs, struct strbuf *sb,
 	if (refs->submodule)
 		strbuf_git_path_submodule(sb, refs->submodule,
 					  "%s", tmp.buf);
+	else if (!strcmp(tmp.buf, "packed-refs") || !strcmp(tmp.buf, "logs"))
+		strbuf_addf(sb, "%s/%s", refs->gitcommondir.buf, tmp.buf);
+	else if (is_per_worktree_ref(tmp.buf) ||
+		 (skip_prefix(tmp.buf, "logs/", &ref) &&
+		  is_per_worktree_ref(ref)))
+		strbuf_addf(sb, "%s/%s", refs->gitdir.buf, tmp.buf);
 	else
-		strbuf_git_path(sb, "%s", tmp.buf);
+		strbuf_addf(sb, "%s/%s", refs->gitcommondir.buf, tmp.buf);
 	strbuf_release(&tmp);
 }
 
@@ -1004,7 +1014,15 @@ static struct ref_store *files_ref_store_create(const char *submodule)
 
 	base_ref_store_init(ref_store, &refs_be_files);
 
-	refs->submodule = xstrdup_or_null(submodule);
+	strbuf_init(&refs->gitdir, 0);
+	strbuf_init(&refs->gitcommondir, 0);
+
+	if (submodule) {
+		refs->submodule = xstrdup(submodule);
+	} else {
+		strbuf_addstr(&refs->gitdir, get_git_dir());
+		strbuf_addstr(&refs->gitcommondir, get_git_common_dir());
+	}
 
 	return ref_store;
 }
-- 
2.11.0.157.gd943d85


^ permalink raw reply related

* [PATCH v3 08/16] refs.c: introduce get_main_ref_store()
From: Nguyễn Thái Ngọc Duy @ 2017-02-17 14:04 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, Michael Haggerty, Johannes Schindelin,
	Ramsay Jones, Stefan Beller, novalis,
	Nguyễn Thái Ngọc Duy
In-Reply-To: <20170217140436.17336-1-pclouds@gmail.com>

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 refs.c | 16 ++++++++++++----
 1 file changed, 12 insertions(+), 4 deletions(-)

diff --git a/refs.c b/refs.c
index 7a474198e..10994d992 100644
--- a/refs.c
+++ b/refs.c
@@ -1445,15 +1445,23 @@ static struct ref_store *ref_store_init(const char *submodule)
 	return refs;
 }
 
+static struct ref_store *get_main_ref_store(void)
+{
+	struct ref_store *refs;
+
+	if (main_ref_store)
+		return main_ref_store;
+
+	refs = ref_store_init(NULL);
+	return refs;
+}
+
 struct ref_store *get_ref_store(const char *submodule)
 {
 	struct ref_store *refs;
 
 	if (!submodule || !*submodule) {
-		refs = lookup_ref_store(NULL);
-
-		if (!refs)
-			refs = ref_store_init(NULL);
+		return get_main_ref_store();
 	} else {
 		refs = lookup_ref_store(submodule);
 
-- 
2.11.0.157.gd943d85


^ 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