* Re: Git bisect does not find commit introducing the bug
From: Philip Oakley @ 2017-02-18 22:10 UTC (permalink / raw)
To: Alex Hoffman, Johannes Sixt; +Cc: Stephan Beyer, git
In-Reply-To: <CAMX8fZW2y+iPRfSbXVcHufbM+CsqgekS_0WnCEJ++=njy_TvKg@mail.gmail.com>
From: "Alex Hoffman" <spec@gal.ro>
>> But this is not how Git works. Git computes graph differences, i.e., it
>> subtracts from the commits reachable from v.bad those that are reachable
>> from v.good. This leaves more than just those on some path from v.good to
>> v.bad. And it should work this way. Consider this history:
>>
>> --o--o--o--G--X
>> \ \
>> x--x--x--x--X--B--
>>
>> When you find B bad and G good, than any one of the x or X may have
>> introduced the breakage, not just one of the X.
>>
>
> Thank you for clarifying how git bisect works. How did you find that
> out? Did you check the source code? If that is not documented in the
> man page may be it worth documenting it in order to avoid future
> confusion for other users.
Any suggestions for improving the documentation are always worthwhile. As
someone who asked, what extra info would have helped?
Even beetter if it looks like a patch ;-)
>
> Let's consider your example with distinct names for nodes:
>
> --o1--o2--o3--G--X1
> \ \
> x1--x2--x3--x4--X1--B--
>
> It makes sense that git bisect is expecting a single transition, as
> this is a precondition for a binary search to work. My definition of
> "the transition" is a commit with at least one of its parents as a
> good version, but the commit itself a bad version. I hope we agree
> that git bisect's mission is to search for this transition (as I
> suppose that most of people need such a functionality from git, if not
> exactly from git bisect). How could be x1 or x3 be the transition, if
> chances are that o1 is not a good version? Of course it would make
> sense to me if bisect would check o1 whether good and only then to
> check also x1-x3, but this is not what git makes (at least not in my
> initial example).
>
> If you consider that git bisect's mission is different from finding
> the transition, could you please explain what exact commit git bisect
> is supposed to return (ideally with terms from the graph theory) and
> when it makes sense to return that? Because I do not see any sense in
> looking in the path x1-x3 without knowing that those commits may be a
> transition.
>
>
>> Oh, IMO git bisect was well thought through. If it considered just paths
>> from good to bad, it would not given the correct answer. See the example
>> history above. Bisect authors would not have deemed that sufficiently
>> good
>
> You definitely convinced me that git MUST search more than only in the
> paths between good and bad commits, as the good commit G does not have
> to be the first good commit (thank you for that). My problem/confusion
> is that it returns something that does not make sense to me, because
> it does not make sure it returns a transition.
>
> VG
>
> PS: thank you for continuing this discussion.
>
--
Philip
^ permalink raw reply
* Re: [PATCH 0/5] A series of performance enhancements in the memihash and name-cache area
From: Junio C Hamano @ 2017-02-18 20:48 UTC (permalink / raw)
To: Jeff King; +Cc: Jeff Hostetler, Johannes Schindelin, git, Jeff Hostetler
In-Reply-To: <20170218062943.j2llxuuylqs2qemy@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> On Fri, Feb 17, 2017 at 09:58:21PM -0800, Junio C Hamano wrote:
>
>> Jeff Hostetler <git@jeffhostetler.com> writes:
>>
>> > I'll try to put together a before/after perf-test to better
>> > demonstrate this.
>>
>> I didn't pick up the series while watching these exchanges, as I
>> didn't know how quick your turnaround would be, but now a few days
>> have passed. Just to make sure we won't forget this topic, I'll
>> pick up these 5 patches in the meantime.
>
> Yeah, to be clear my question was not an objection, but mostly
> curiosity and interest.
Yes, it was very clear that it wasn't an objection.
But the other Jeff sounded like a follow-up was to follow shortly if
not imminent so I decided to allocate my time on other topics still
only on the list first while waiting to see what happens.
^ permalink raw reply
* Re: [PATCH v3 02/19] builtin/diff-tree: convert to struct object_id
From: Jeff King @ 2017-02-18 20:01 UTC (permalink / raw)
To: brian m. carlson, Ramsay Jones, git, Michael Haggerty,
Junio C Hamano
In-Reply-To: <20170218191217.thn3c2bympv2g7pm@genre.crustytoothpaste.net>
On Sat, Feb 18, 2017 at 07:12:18PM +0000, brian m. carlson wrote:
> On Fri, Feb 17, 2017 at 10:15:31PM -0500, Jeff King wrote:
> > So for this case, something like the patch below.
> >
> > Incidentally, there's an off-by-one in the original loop of
> > stdin_diff_commit that reads past the end of the trailing NUL for the
> > final sha1 on the line. The problem is the:
> >
> > pos += GIT_SHA1_HEXSZ + 1;
> >
> > which assumes we're slurping up the trailing space. This works in
> > practice because the caller will only permit a string which had a
> > newline (which it converted into a NUL).
> >
> > I suspect that function could be more aggressive about complaining about
> > nonsense on the line, rather than silently ignoring it.
>
> I'd come to basically the same patch, but I did pick up a few niceties
> from your patch, like avoiding the off-by-one issue you mentioned above.
> Can I place your sign-off on the resulting change?
Absolutely. Thanks for taking a look.
-Peff
^ permalink raw reply
* Re: Git bisect does not find commit introducing the bug
From: Christian Couder @ 2017-02-18 19:58 UTC (permalink / raw)
To: Alex Hoffman; +Cc: Johannes Sixt, Stephan Beyer, git
In-Reply-To: <CAMX8fZW2y+iPRfSbXVcHufbM+CsqgekS_0WnCEJ++=njy_TvKg@mail.gmail.com>
On Sat, Feb 18, 2017 at 7:36 PM, Alex Hoffman <spec@gal.ro> wrote:
>> But this is not how Git works. Git computes graph differences, i.e., it
>> subtracts from the commits reachable from v.bad those that are reachable
>> from v.good. This leaves more than just those on some path from v.good to
>> v.bad. And it should work this way. Consider this history:
>>
>> --o--o--o--G--X
>> \ \
>> x--x--x--x--X--B--
>>
>> When you find B bad and G good, than any one of the x or X may have
>> introduced the breakage, not just one of the X.
>>
>
> Thank you for clarifying how git bisect works. How did you find that
> out? Did you check the source code? If that is not documented in the
> man page may be it worth documenting it in order to avoid future
> confusion for other users.
At the end of the git-bisect man page (in the SEE ALSO section) there
is a link to https://github.com/git/git/blob/master/Documentation/git-bisect-lk2009.txt
which has a lot of details about how bisect works.
> Let's consider your example with distinct names for nodes:
>
> --o1--o2--o3--G--X1
> \ \
> x1--x2--x3--x4--X1--B--
>
> It makes sense that git bisect is expecting a single transition, as
> this is a precondition for a binary search to work. My definition of
> "the transition" is a commit with at least one of its parents as a
> good version, but the commit itself a bad version.
What if a commit C has one good parent A and one bad parent B?
Isn't it more likely that the first bad commit is B instead of C?
> I hope we agree
> that git bisect's mission is to search for this transition (as I
> suppose that most of people need such a functionality from git, if not
> exactly from git bisect).
The goal is to find the first bad commit, which is a commit that has
only good parents.
> How could be x1 or x3 be the transition, if
> chances are that o1 is not a good version?
As o1 is an ancestor of G, then o1 is considered good by the bisect algorithm.
If it was bad, it would means that there is a transition from bad to
good between o1 and G.
But when a good commit is an ancestor of the bad commit, git bisect
makes the assumption that there is no transition from bad to good in
the graph.
> Of course it would make
> sense to me if bisect would check o1 whether good and only then to
> check also x1-x3, but this is not what git makes (at least not in my
> initial example).
>
> If you consider that git bisect's mission is different from finding
> the transition, could you please explain what exact commit git bisect
> is supposed to return (ideally with terms from the graph theory) and
> when it makes sense to return that? Because I do not see any sense in
> looking in the path x1-x3 without knowing that those commits may be a
> transition.
I hope it makes sense with the above explanations.
>> Oh, IMO git bisect was well thought through. If it considered just paths
>> from good to bad, it would not given the correct answer. See the example
>> history above. Bisect authors would not have deemed that sufficiently good
>
> You definitely convinced me that git MUST search more than only in the
> paths between good and bad commits, as the good commit G does not have
> to be the first good commit (thank you for that). My problem/confusion
> is that it returns something that does not make sense to me, because
> it does not make sure it returns a transition.
git bisect makes some assumptions that are true most of the time, so
in practice it works well most of the time.
^ permalink raw reply
* Re: [PATCH v3 02/19] builtin/diff-tree: convert to struct object_id
From: brian m. carlson @ 2017-02-18 19:12 UTC (permalink / raw)
To: Jeff King; +Cc: Ramsay Jones, git, Michael Haggerty, Junio C Hamano
In-Reply-To: <20170218031530.2bhlnjcukl4ybt6h@sigill.intra.peff.net>
[-- Attachment #1: Type: text/plain, Size: 1036 bytes --]
On Fri, Feb 17, 2017 at 10:15:31PM -0500, Jeff King wrote:
> So for this case, something like the patch below.
>
> Incidentally, there's an off-by-one in the original loop of
> stdin_diff_commit that reads past the end of the trailing NUL for the
> final sha1 on the line. The problem is the:
>
> pos += GIT_SHA1_HEXSZ + 1;
>
> which assumes we're slurping up the trailing space. This works in
> practice because the caller will only permit a string which had a
> newline (which it converted into a NUL).
>
> I suspect that function could be more aggressive about complaining about
> nonsense on the line, rather than silently ignoring it.
I'd come to basically the same patch, but I did pick up a few niceties
from your patch, like avoiding the off-by-one issue you mentioned above.
Can I place your sign-off on the resulting change?
--
brian m. carlson / brian with sandals: Houston, Texas, US
+1 832 623 2791 | https://www.crustytoothpaste.net/~bmc | My opinion only
OpenPGP: https://keybase.io/bk2204
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 868 bytes --]
^ permalink raw reply
* Re: Git bisect does not find commit introducing the bug
From: Alex Hoffman @ 2017-02-18 18:36 UTC (permalink / raw)
To: Johannes Sixt; +Cc: Stephan Beyer, git
In-Reply-To: <477d3533-d453-9499-e06e-72f45488d421@kdbg.org>
> But this is not how Git works. Git computes graph differences, i.e., it
> subtracts from the commits reachable from v.bad those that are reachable
> from v.good. This leaves more than just those on some path from v.good to
> v.bad. And it should work this way. Consider this history:
>
> --o--o--o--G--X
> \ \
> x--x--x--x--X--B--
>
> When you find B bad and G good, than any one of the x or X may have
> introduced the breakage, not just one of the X.
>
Thank you for clarifying how git bisect works. How did you find that
out? Did you check the source code? If that is not documented in the
man page may be it worth documenting it in order to avoid future
confusion for other users.
Let's consider your example with distinct names for nodes:
--o1--o2--o3--G--X1
\ \
x1--x2--x3--x4--X1--B--
It makes sense that git bisect is expecting a single transition, as
this is a precondition for a binary search to work. My definition of
"the transition" is a commit with at least one of its parents as a
good version, but the commit itself a bad version. I hope we agree
that git bisect's mission is to search for this transition (as I
suppose that most of people need such a functionality from git, if not
exactly from git bisect). How could be x1 or x3 be the transition, if
chances are that o1 is not a good version? Of course it would make
sense to me if bisect would check o1 whether good and only then to
check also x1-x3, but this is not what git makes (at least not in my
initial example).
If you consider that git bisect's mission is different from finding
the transition, could you please explain what exact commit git bisect
is supposed to return (ideally with terms from the graph theory) and
when it makes sense to return that? Because I do not see any sense in
looking in the path x1-x3 without knowing that those commits may be a
transition.
> Oh, IMO git bisect was well thought through. If it considered just paths
> from good to bad, it would not given the correct answer. See the example
> history above. Bisect authors would not have deemed that sufficiently good
You definitely convinced me that git MUST search more than only in the
paths between good and bad commits, as the good commit G does not have
to be the first good commit (thank you for that). My problem/confusion
is that it returns something that does not make sense to me, because
it does not make sure it returns a transition.
VG
PS: thank you for continuing this discussion.
^ permalink raw reply
* Re: Git bisect does not find commit introducing the bug
From: Johannes Sixt @ 2017-02-18 14:18 UTC (permalink / raw)
To: Alex Hoffman; +Cc: Stephan Beyer, git
In-Reply-To: <CAMX8fZVkBU6M8fkUcRr69V97_NTSOGGmPB1U-ObhmVv3i6wQhg@mail.gmail.com>
Am 18.02.2017 um 12:15 schrieb Alex Hoffman:
> No one commented the fact, that I find this very confusing. Don't you
> find this confusing? I will underline, that 'git bisect good v.good'
> will fail if the commit 'v.good' is not a parent of the bad commit,
> meaning there MUST be at least a path between 'v.good' and 'v.bad',
> thus I would expect it looks on this path ONLY. Beside that, this is
> what I understand by 'binary search' (to search on this commit path).
But this is not how Git works. Git computes graph differences, i.e., it
subtracts from the commits reachable from v.bad those that are reachable
from v.good. This leaves more than just those on some path from v.good
to v.bad. And it should work this way. Consider this history:
--o--o--o--G--X
\ \
x--x--x--x--X--B--
When you find B bad and G good, than any one of the x or X may have
introduced the breakage, not just one of the X.
>> Correct. The assumption of bisection is that there is only one
>> transition between GOOD and BAD. By violating that assumption,
>> anything can happen.
>
> I did not find that in the manpage or did I miss it? Why would someone
> assume that the commit graph looks in a certain way?
There is no restriction in the commit graph. The only restriction,
actually assumption, is that there is *one* transition from good to bad
and no transition from bad to good. Otherwise, bisection cannot work.
> I assume, that
> 'git bisect' was not thought through and that it considers the first
> directed path between v.good and v.bad, instead of all paths (in my
> example graph there are two such paths). I will also underline that
> git bisect was designed to work with multiple good commits and one bad
> commit (also multiple paths), but probably NOT with multiple paths
> between the same pair of good and bad commits.
Oh, IMO git bisect was well thought through. If it considered just paths
from good to bad, it would not given the correct answer. See the example
history above. Bisect authors would not have deemed that sufficiently
good ;)
-- Hannes
^ permalink raw reply
* [PATCH v4 15/15] refs: rename get_ref_store() to get_submodule_ref_store() and make it public
From: Nguyễn Thái Ngọc Duy @ 2017-02-18 13:33 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: <20170218133303.3682-1-pclouds@gmail.com>
This function is intended to replace *_submodule() refs API. It provides
a ref store for a specific submodule, which can be operated on by a new
set of refs API.
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
refs.c | 12 ++++++++----
refs.h | 11 +++++++++++
refs/files-backend.c | 2 +-
refs/refs-internal.h | 12 ------------
4 files changed, 20 insertions(+), 17 deletions(-)
diff --git a/refs.c b/refs.c
index 8035c3ea7..bc2247b59 100644
--- a/refs.c
+++ b/refs.c
@@ -1165,7 +1165,7 @@ int head_ref(each_ref_fn fn, void *cb_data)
static int do_for_each_ref(const char *submodule, const char *prefix,
each_ref_fn fn, int trim, int flags, void *cb_data)
{
- struct ref_store *refs = get_ref_store(submodule);
+ struct ref_store *refs = get_submodule_ref_store(submodule);
struct ref_iterator *iter;
if (!refs)
@@ -1338,10 +1338,10 @@ int resolve_gitlink_ref(const char *submodule, const char *refname,
/* We need to strip off one or more trailing slashes */
char *stripped = xmemdupz(submodule, len);
- refs = get_ref_store(stripped);
+ refs = get_submodule_ref_store(stripped);
free(stripped);
} else {
- refs = get_ref_store(submodule);
+ refs = get_submodule_ref_store(submodule);
}
if (!refs)
@@ -1457,13 +1457,17 @@ struct ref_store *get_main_ref_store(void)
return refs;
}
-struct ref_store *get_ref_store(const char *submodule)
+struct ref_store *get_submodule_ref_store(const char *submodule)
{
struct strbuf submodule_sb = STRBUF_INIT;
struct ref_store *refs;
int ret;
if (!submodule || !*submodule) {
+ /*
+ * FIXME: This case is ideally not allowed. But that
+ * can't happen until we clean up all the callers.
+ */
return get_main_ref_store();
}
diff --git a/refs.h b/refs.h
index a35b687d7..5bd065f12 100644
--- a/refs.h
+++ b/refs.h
@@ -556,5 +556,16 @@ int reflog_expire(const char *refname, const unsigned char *sha1,
int ref_storage_backend_exists(const char *name);
struct ref_store *get_main_ref_store(void);
+/*
+ * Return the ref_store instance for the specified submodule. For the
+ * main repository, use submodule==NULL; such a call cannot fail. For
+ * a submodule, the submodule must exist and be a nonbare repository,
+ * otherwise return NULL. If the requested reference store has not yet
+ * been initialized, initialize it first.
+ *
+ * For backwards compatibility, submodule=="" is treated the same as
+ * submodule==NULL.
+ */
+struct ref_store *get_submodule_ref_store(const char *submodule);
#endif /* REFS_H */
diff --git a/refs/files-backend.c b/refs/files-backend.c
index b8ccf4e47..603601f1b 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -3125,7 +3125,7 @@ int set_worktree_head_symref(const char *gitdir, const char *target)
* backends. This function needs to die.
*/
struct files_ref_store *refs =
- files_downcast(get_ref_store(NULL), "set_head_symref");
+ files_downcast(get_main_ref_store(), "set_head_symref");
static struct lock_file head_lock;
struct ref_lock *lock;
struct strbuf head_path = STRBUF_INIT;
diff --git a/refs/refs-internal.h b/refs/refs-internal.h
index 3ef020acf..ed6527ce1 100644
--- a/refs/refs-internal.h
+++ b/refs/refs-internal.h
@@ -640,18 +640,6 @@ struct ref_store {
void base_ref_store_init(struct ref_store *refs,
const struct ref_storage_be *be);
-/*
- * Return the ref_store instance for the specified submodule. For the
- * main repository, use submodule==NULL; such a call cannot fail. For
- * a submodule, the submodule must exist and be a nonbare repository,
- * otherwise return NULL. If the requested reference store has not yet
- * been initialized, initialize it first.
- *
- * For backwards compatibility, submodule=="" is treated the same as
- * submodule==NULL.
- */
-struct ref_store *get_ref_store(const char *submodule);
-
const char *resolve_ref_recursively(struct ref_store *refs,
const char *refname,
int resolve_flags,
--
2.11.0.157.gd943d85
^ permalink raw reply related
* [PATCH v4 14/15] files-backend: remove submodule_allowed from files_downcast()
From: Nguyễn Thái Ngọc Duy @ 2017-02-18 13:33 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: <20170218133303.3682-1-pclouds@gmail.com>
Since submodule or not is irrelevant to files-backend after the last
patch, remove the parameter from files_downcast() and kill
files_assert_main_repository().
PS. This implies that all ref operations are allowed for submodules. But
we may need to look more closely to see if that's really true...
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
refs/files-backend.c | 70 ++++++++++++++++------------------------------------
1 file changed, 21 insertions(+), 49 deletions(-)
diff --git a/refs/files-backend.c b/refs/files-backend.c
index 529c80af1..b8ccf4e47 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -1030,24 +1030,13 @@ static struct ref_store *files_ref_store_create(const char *gitdir)
}
/*
- * Die if refs is for a submodule (i.e., not for the main repository).
- * caller is used in any necessary error messages.
- */
-static void files_assert_main_repository(struct files_ref_store *refs,
- const char *caller)
-{
- /* This function is to be deleted in the next patch */
-}
-
-/*
* Downcast ref_store to files_ref_store. Die if ref_store is not a
* files_ref_store. If submodule_allowed is not true, then also die if
* files_ref_store is for a submodule (i.e., not for the main
* repository). caller is used in any necessary error messages.
*/
-static struct files_ref_store *files_downcast(
- struct ref_store *ref_store, int submodule_allowed,
- const char *caller)
+static struct files_ref_store *files_downcast(struct ref_store *ref_store,
+ const char *caller)
{
struct files_ref_store *refs;
@@ -1057,9 +1046,6 @@ static struct files_ref_store *files_downcast(
refs = (struct files_ref_store *)ref_store;
- if (!submodule_allowed)
- files_assert_main_repository(refs, caller);
-
return refs;
}
@@ -1395,7 +1381,7 @@ static int files_read_raw_ref(struct ref_store *ref_store,
struct strbuf *referent, unsigned int *type)
{
struct files_ref_store *refs =
- files_downcast(ref_store, 1, "read_raw_ref");
+ files_downcast(ref_store, "read_raw_ref");
struct strbuf sb_contents = STRBUF_INIT;
struct strbuf sb_path = STRBUF_INIT;
const char *path;
@@ -1588,7 +1574,6 @@ static int lock_raw_ref(struct files_ref_store *refs,
int ret = TRANSACTION_GENERIC_ERROR;
assert(err);
- files_assert_main_repository(refs, "lock_raw_ref");
*type = 0;
@@ -1812,7 +1797,7 @@ static enum peel_status peel_entry(struct ref_entry *entry, int repeel)
static int files_peel_ref(struct ref_store *ref_store,
const char *refname, unsigned char *sha1)
{
- struct files_ref_store *refs = files_downcast(ref_store, 0, "peel_ref");
+ struct files_ref_store *refs = files_downcast(ref_store, "peel_ref");
int flag;
unsigned char base[20];
@@ -1921,7 +1906,7 @@ static struct ref_iterator *files_ref_iterator_begin(
const char *prefix, unsigned int flags)
{
struct files_ref_store *refs =
- files_downcast(ref_store, 1, "ref_iterator_begin");
+ files_downcast(ref_store, "ref_iterator_begin");
struct ref_dir *loose_dir, *packed_dir;
struct ref_iterator *loose_iter, *packed_iter;
struct files_ref_iterator *iter;
@@ -2059,7 +2044,6 @@ static struct ref_lock *lock_ref_sha1_basic(struct files_ref_store *refs,
int resolve_flags = RESOLVE_REF_NO_RECURSE;
int resolved;
- files_assert_main_repository(refs, "lock_ref_sha1_basic");
assert(err);
lock = xcalloc(1, sizeof(struct ref_lock));
@@ -2184,8 +2168,6 @@ static int lock_packed_refs(struct files_ref_store *refs, int flags)
struct strbuf sb = STRBUF_INIT;
int ret;
- files_assert_main_repository(refs, "lock_packed_refs");
-
if (!timeout_configured) {
git_config_get_int("core.packedrefstimeout", &timeout_value);
timeout_configured = 1;
@@ -2225,8 +2207,6 @@ static int commit_packed_refs(struct files_ref_store *refs)
int save_errno = 0;
FILE *out;
- files_assert_main_repository(refs, "commit_packed_refs");
-
if (!packed_ref_cache->lock)
die("internal error: packed-refs not locked");
@@ -2258,8 +2238,6 @@ static void rollback_packed_refs(struct files_ref_store *refs)
struct packed_ref_cache *packed_ref_cache =
get_packed_ref_cache(refs);
- files_assert_main_repository(refs, "rollback_packed_refs");
-
if (!packed_ref_cache->lock)
die("internal error: packed-refs not locked");
rollback_lock_file(packed_ref_cache->lock);
@@ -2420,7 +2398,7 @@ static void prune_refs(struct files_ref_store *refs, struct ref_to_prune *r)
static int files_pack_refs(struct ref_store *ref_store, unsigned int flags)
{
struct files_ref_store *refs =
- files_downcast(ref_store, 0, "pack_refs");
+ files_downcast(ref_store, "pack_refs");
struct pack_refs_cb_data cbdata;
memset(&cbdata, 0, sizeof(cbdata));
@@ -2453,7 +2431,6 @@ static int repack_without_refs(struct files_ref_store *refs,
struct string_list_item *refname;
int ret, needs_repacking = 0, removed = 0;
- files_assert_main_repository(refs, "repack_without_refs");
assert(err);
/* Look for a packed ref */
@@ -2503,7 +2480,7 @@ static int files_delete_refs(struct ref_store *ref_store,
struct string_list *refnames, unsigned int flags)
{
struct files_ref_store *refs =
- files_downcast(ref_store, 0, "delete_refs");
+ files_downcast(ref_store, "delete_refs");
struct strbuf err = STRBUF_INIT;
int i, result = 0;
@@ -2615,7 +2592,7 @@ static int files_verify_refname_available(struct ref_store *ref_store,
struct strbuf *err)
{
struct files_ref_store *refs =
- files_downcast(ref_store, 1, "verify_refname_available");
+ files_downcast(ref_store, "verify_refname_available");
struct ref_dir *packed_refs = get_packed_refs(refs);
struct ref_dir *loose_refs = get_loose_refs(refs);
@@ -2640,7 +2617,7 @@ static int files_rename_ref(struct ref_store *ref_store,
const char *logmsg)
{
struct files_ref_store *refs =
- files_downcast(ref_store, 0, "rename_ref");
+ files_downcast(ref_store, "rename_ref");
unsigned char sha1[20], orig_sha1[20];
int flag = 0, logmoved = 0;
struct ref_lock *lock;
@@ -2883,7 +2860,7 @@ static int files_create_reflog(struct ref_store *ref_store,
struct strbuf *err)
{
struct files_ref_store *refs =
- files_downcast(ref_store, 0, "create_reflog");
+ files_downcast(ref_store, "create_reflog");
int fd;
if (log_ref_setup(refs, refname, force_create, &fd, err))
@@ -3014,8 +2991,6 @@ static int commit_ref_update(struct files_ref_store *refs,
const unsigned char *sha1, const char *logmsg,
struct strbuf *err)
{
- files_assert_main_repository(refs, "commit_ref_update");
-
clear_loose_ref_cache(refs);
if (files_log_ref_write(refs, lock->ref_name, lock->old_oid.hash,
sha1, logmsg, 0, err)) {
@@ -3124,7 +3099,7 @@ static int files_create_symref(struct ref_store *ref_store,
const char *logmsg)
{
struct files_ref_store *refs =
- files_downcast(ref_store, 0, "create_symref");
+ files_downcast(ref_store, "create_symref");
struct strbuf err = STRBUF_INIT;
struct ref_lock *lock;
int ret;
@@ -3150,7 +3125,7 @@ int set_worktree_head_symref(const char *gitdir, const char *target)
* backends. This function needs to die.
*/
struct files_ref_store *refs =
- files_downcast(get_ref_store(NULL), 0, "set_head_symref");
+ files_downcast(get_ref_store(NULL), "set_head_symref");
static struct lock_file head_lock;
struct ref_lock *lock;
struct strbuf head_path = STRBUF_INIT;
@@ -3188,7 +3163,7 @@ 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");
+ files_downcast(ref_store, "reflog_exists");
struct strbuf sb = STRBUF_INIT;
struct stat st;
int ret;
@@ -3203,7 +3178,7 @@ 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");
+ files_downcast(ref_store, "delete_reflog");
struct strbuf sb = STRBUF_INIT;
int ret;
@@ -3258,7 +3233,7 @@ static int files_for_each_reflog_ent_reverse(struct ref_store *ref_store,
void *cb_data)
{
struct files_ref_store *refs =
- files_downcast(ref_store, 0, "for_each_reflog_ent_reverse");
+ files_downcast(ref_store, "for_each_reflog_ent_reverse");
struct strbuf sb = STRBUF_INIT;
FILE *logfp;
long pos;
@@ -3366,7 +3341,7 @@ static int files_for_each_reflog_ent(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");
+ files_downcast(ref_store, "for_each_reflog_ent");
FILE *logfp;
struct strbuf sb = STRBUF_INIT;
int ret = 0;
@@ -3454,7 +3429,7 @@ 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");
+ files_downcast(ref_store, "reflog_iterator_begin");
struct files_reflog_iterator *iter = xcalloc(1, sizeof(*iter));
struct ref_iterator *ref_iterator = &iter->base;
struct strbuf sb = STRBUF_INIT;
@@ -3666,8 +3641,6 @@ static int lock_ref_for_update(struct files_ref_store *refs,
int ret;
struct ref_lock *lock;
- files_assert_main_repository(refs, "lock_ref_for_update");
-
if ((update->flags & REF_HAVE_NEW) && is_null_sha1(update->new_sha1))
update->flags |= REF_DELETING;
@@ -3792,7 +3765,7 @@ static int files_transaction_commit(struct ref_store *ref_store,
struct strbuf *err)
{
struct files_ref_store *refs =
- files_downcast(ref_store, 0, "ref_transaction_commit");
+ files_downcast(ref_store, "ref_transaction_commit");
int ret = 0, i;
struct string_list refs_to_delete = STRING_LIST_INIT_NODUP;
struct string_list_item *ref_to_delete;
@@ -4000,7 +3973,7 @@ static int files_initial_transaction_commit(struct ref_store *ref_store,
struct strbuf *err)
{
struct files_ref_store *refs =
- files_downcast(ref_store, 0, "initial_ref_transaction_commit");
+ files_downcast(ref_store, "initial_ref_transaction_commit");
int ret = 0, i;
struct string_list affected_refnames = STRING_LIST_INIT_NODUP;
@@ -4122,7 +4095,7 @@ static int files_reflog_expire(struct ref_store *ref_store,
void *policy_cb_data)
{
struct files_ref_store *refs =
- files_downcast(ref_store, 0, "reflog_expire");
+ files_downcast(ref_store, "reflog_expire");
static struct lock_file reflog_lock;
struct expire_reflog_cb cb;
struct ref_lock *lock;
@@ -4227,8 +4200,7 @@ 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 files_ref_store *refs = files_downcast(ref_store, "init_db");
struct strbuf sb = STRBUF_INIT;
/*
--
2.11.0.157.gd943d85
^ permalink raw reply related
* [PATCH v4 13/15] refs: move submodule code out of files-backend.c
From: Nguyễn Thái Ngọc Duy @ 2017-02-18 13:33 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: <20170218133303.3682-1-pclouds@gmail.com>
files-backend is now initialized with a $GIT_DIR. Converting a submodule
path to where real submodule gitdir is located is done in get_ref_store().
The new code in init_submodule_ref_store() is basically a copy of
strbuf_git_path_submodule().
This gives a slight performance improvement for submodules since we
don't convert submodule path to gitdir at every backend call like
before. We pay that once at ref-store creation.
More cleanup in files_downcast() follows shortly. It's separate to keep
noises from this patch.
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
refs.c | 18 +++++++++++++-----
refs/files-backend.c | 24 +++++++-----------------
refs/refs-internal.h | 6 +++---
3 files changed, 23 insertions(+), 25 deletions(-)
diff --git a/refs.c b/refs.c
index 1b60e6180..8035c3ea7 100644
--- a/refs.c
+++ b/refs.c
@@ -9,6 +9,7 @@
#include "refs/refs-internal.h"
#include "object.h"
#include "tag.h"
+#include "submodule.h"
/*
* List of all available backends
@@ -1424,9 +1425,9 @@ static void register_submodule_ref_store(struct ref_store *refs,
/*
* Create, record, and return a ref_store instance for the specified
- * submodule (or the main repository if submodule is NULL).
+ * gitdir (or the main repository if gitdir is NULL).
*/
-static struct ref_store *ref_store_init(const char *submodule)
+static struct ref_store *ref_store_init(const char *gitdir)
{
const char *be_name = "files";
struct ref_storage_be *be = find_ref_storage_backend(be_name);
@@ -1435,7 +1436,7 @@ static struct ref_store *ref_store_init(const char *submodule)
if (!be)
die("BUG: reference backend %s is unknown", be_name);
- refs = be->init(submodule);
+ refs = be->init(gitdir);
return refs;
}
@@ -1460,6 +1461,7 @@ struct ref_store *get_ref_store(const char *submodule)
{
struct strbuf submodule_sb = STRBUF_INIT;
struct ref_store *refs;
+ int ret;
if (!submodule || !*submodule) {
return get_main_ref_store();
@@ -1470,8 +1472,14 @@ struct ref_store *get_ref_store(const char *submodule)
return refs;
strbuf_addstr(&submodule_sb, submodule);
- if (is_nonbare_repository_dir(&submodule_sb))
- refs = ref_store_init(submodule);
+ ret = is_nonbare_repository_dir(&submodule_sb);
+ strbuf_release(&submodule_sb);
+ if (!ret)
+ return refs;
+
+ ret = submodule_to_gitdir(&submodule_sb, submodule);
+ if (!ret)
+ refs = ref_store_init(submodule_sb.buf);
strbuf_release(&submodule_sb);
if (refs)
diff --git a/refs/files-backend.c b/refs/files-backend.c
index dbcaf9bda..529c80af1 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -917,13 +917,6 @@ struct packed_ref_cache {
struct files_ref_store {
struct ref_store base;
- /*
- * The name of the submodule represented by this object, or
- * NULL if it represents the main repository's reference
- * store:
- */
- const char *submodule;
-
struct strbuf gitdir;
struct strbuf gitcommondir;
@@ -945,11 +938,8 @@ 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 (refs->submodule) {
- strbuf_git_path_submodule(sb, refs->submodule,
- "%s", tmp.buf);
- } else if (!strcmp(tmp.buf, "packed-refs") ||
- !strcmp(tmp.buf, "logs")) { /* non refname path */
+ if (!strcmp(tmp.buf, "packed-refs") || !strcmp(tmp.buf, "logs")) {
+ /* non refname path */
strbuf_addf(sb, "%s/%s", refs->gitcommondir.buf, tmp.buf);
} else if (skip_prefix(tmp.buf, "logs/", &ref)) { /* reflog */
if (is_per_worktree_ref(ref))
@@ -1018,7 +1008,7 @@ static void clear_loose_ref_cache(struct files_ref_store *refs)
* Create a new submodule ref cache and add it to the internal
* set of caches.
*/
-static struct ref_store *files_ref_store_create(const char *submodule)
+static struct ref_store *files_ref_store_create(const char *gitdir)
{
struct files_ref_store *refs = xcalloc(1, sizeof(*refs));
struct ref_store *ref_store = (struct ref_store *)refs;
@@ -1028,8 +1018,9 @@ static struct ref_store *files_ref_store_create(const char *submodule)
strbuf_init(&refs->gitdir, 0);
strbuf_init(&refs->gitcommondir, 0);
- if (submodule) {
- refs->submodule = xstrdup(submodule);
+ if (gitdir) {
+ strbuf_addstr(&refs->gitdir, gitdir);
+ get_common_dir_noenv(&refs->gitcommondir, gitdir);
} else {
strbuf_addstr(&refs->gitdir, get_git_dir());
strbuf_addstr(&refs->gitcommondir, get_git_common_dir());
@@ -1045,8 +1036,7 @@ static struct ref_store *files_ref_store_create(const char *submodule)
static void files_assert_main_repository(struct files_ref_store *refs,
const char *caller)
{
- if (refs->submodule)
- die("BUG: %s called for a submodule", caller);
+ /* This function is to be deleted in the next patch */
}
/*
diff --git a/refs/refs-internal.h b/refs/refs-internal.h
index 8c5febf54..3ef020acf 100644
--- a/refs/refs-internal.h
+++ b/refs/refs-internal.h
@@ -482,12 +482,12 @@ struct ref_store;
/* refs backends */
/*
- * Initialize the ref_store for the specified submodule, or for the
- * main repository if submodule == NULL. These functions should call
+ * Initialize the ref_store for the specified gitdir, or for the
+ * current repository if gitdir == NULL. These functions should call
* base_ref_store_init() to initialize the shared part of the
* ref_store and to record the ref_store for later lookup.
*/
-typedef struct ref_store *ref_store_init_fn(const char *submodule);
+typedef struct ref_store *ref_store_init_fn(const char *gitdir);
typedef int ref_init_db_fn(struct ref_store *refs, struct strbuf *err);
--
2.11.0.157.gd943d85
^ permalink raw reply related
* [PATCH v4 12/15] path.c: move some code out of strbuf_git_path_submodule()
From: Nguyễn Thái Ngọc Duy @ 2017-02-18 13:33 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: <20170218133303.3682-1-pclouds@gmail.com>
refs is learning to avoid path rewriting that is done by
strbuf_git_path_submodule(). Factor out this code so it could be reused
by refs*
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
path.c | 34 +++++++---------------------------
submodule.c | 31 +++++++++++++++++++++++++++++++
submodule.h | 1 +
3 files changed, 39 insertions(+), 27 deletions(-)
diff --git a/path.c b/path.c
index efcedafba..3451d2916 100644
--- a/path.c
+++ b/path.c
@@ -475,35 +475,16 @@ const char *worktree_git_path(const struct worktree *wt, const char *fmt, ...)
static int do_submodule_path(struct strbuf *buf, const char *path,
const char *fmt, va_list args)
{
- const char *git_dir;
struct strbuf git_submodule_common_dir = STRBUF_INIT;
struct strbuf git_submodule_dir = STRBUF_INIT;
- const struct submodule *sub;
- int err = 0;
+ int ret;
- strbuf_addstr(buf, path);
- strbuf_complete(buf, '/');
- strbuf_addstr(buf, ".git");
-
- git_dir = read_gitfile(buf->buf);
- if (git_dir) {
- strbuf_reset(buf);
- strbuf_addstr(buf, git_dir);
- }
- if (!is_git_directory(buf->buf)) {
- gitmodules_config();
- sub = submodule_from_path(null_sha1, path);
- if (!sub) {
- err = SUBMODULE_PATH_ERR_NOT_CONFIGURED;
- goto cleanup;
- }
- strbuf_reset(buf);
- strbuf_git_path(buf, "%s/%s", "modules", sub->name);
- }
-
- strbuf_addch(buf, '/');
- strbuf_addbuf(&git_submodule_dir, buf);
+ ret = submodule_to_gitdir(&git_submodule_dir, path);
+ if (ret)
+ goto cleanup;
+ strbuf_complete(&git_submodule_dir, '/');
+ strbuf_addbuf(buf, &git_submodule_dir);
strbuf_vaddf(buf, fmt, args);
if (get_common_dir_noenv(&git_submodule_common_dir, git_submodule_dir.buf))
@@ -514,8 +495,7 @@ static int do_submodule_path(struct strbuf *buf, const char *path,
cleanup:
strbuf_release(&git_submodule_dir);
strbuf_release(&git_submodule_common_dir);
-
- return err;
+ return ret;
}
char *git_pathdup_submodule(const char *path, const char *fmt, ...)
diff --git a/submodule.c b/submodule.c
index 3b98766a6..36b8d1d11 100644
--- a/submodule.c
+++ b/submodule.c
@@ -1514,3 +1514,34 @@ void absorb_git_dir_into_superproject(const char *prefix,
strbuf_release(&sb);
}
}
+
+int submodule_to_gitdir(struct strbuf *buf, const char *submodule)
+{
+ const struct submodule *sub;
+ const char *git_dir;
+ int ret = 0;
+
+ strbuf_reset(buf);
+ strbuf_addstr(buf, submodule);
+ strbuf_complete(buf, '/');
+ strbuf_addstr(buf, ".git");
+
+ git_dir = read_gitfile(buf->buf);
+ if (git_dir) {
+ strbuf_reset(buf);
+ strbuf_addstr(buf, git_dir);
+ }
+ if (!is_git_directory(buf->buf)) {
+ gitmodules_config();
+ sub = submodule_from_path(null_sha1, submodule);
+ if (!sub) {
+ ret = -1;
+ goto cleanup;
+ }
+ strbuf_reset(buf);
+ strbuf_git_path(buf, "%s/%s", "modules", sub->name);
+ }
+
+cleanup:
+ return ret;
+}
diff --git a/submodule.h b/submodule.h
index 05ab674f0..fc3d0303e 100644
--- a/submodule.h
+++ b/submodule.h
@@ -81,6 +81,7 @@ extern int push_unpushed_submodules(struct sha1_array *commits,
int dry_run);
extern void connect_work_tree_and_git_dir(const char *work_tree, const char *git_dir);
extern int parallel_submodules(void);
+int submodule_to_gitdir(struct strbuf *buf, const char *submodule);
/*
* Prepare the "env_array" parameter of a "struct child_process" for executing
--
2.11.0.157.gd943d85
^ permalink raw reply related
* [PATCH v4 11/15] refs.c: make get_main_ref_store() public and use it
From: Nguyễn Thái Ngọc Duy @ 2017-02-18 13:32 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: <20170218133303.3682-1-pclouds@gmail.com>
get_ref_store() will soon be renamed to get_submodule_ref_store().
Together with future get_worktree_ref_store(), the three functions
provide an appropriate ref store for different operation modes. New APIs
will be added to operate directly on ref stores.
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
refs.c | 36 ++++++++++++++++++------------------
refs.h | 2 ++
2 files changed, 20 insertions(+), 18 deletions(-)
diff --git a/refs.c b/refs.c
index 8b640bd05..1b60e6180 100644
--- a/refs.c
+++ b/refs.c
@@ -1308,7 +1308,7 @@ const char *resolve_ref_recursively(struct ref_store *refs,
/* backend functions */
int refs_init_db(struct strbuf *err)
{
- struct ref_store *refs = get_ref_store(NULL);
+ struct ref_store *refs = get_main_ref_store();
return refs->be->init_db(refs, err);
}
@@ -1316,7 +1316,7 @@ int refs_init_db(struct strbuf *err)
const char *resolve_ref_unsafe(const char *refname, int resolve_flags,
unsigned char *sha1, int *flags)
{
- return resolve_ref_recursively(get_ref_store(NULL), refname,
+ return resolve_ref_recursively(get_main_ref_store(), refname,
resolve_flags, sha1, flags);
}
@@ -1439,7 +1439,7 @@ static struct ref_store *ref_store_init(const char *submodule)
return refs;
}
-static struct ref_store *get_main_ref_store(void)
+struct ref_store *get_main_ref_store(void)
{
struct ref_store *refs;
@@ -1488,14 +1488,14 @@ void base_ref_store_init(struct ref_store *refs,
/* backend functions */
int pack_refs(unsigned int flags)
{
- struct ref_store *refs = get_ref_store(NULL);
+ struct ref_store *refs = get_main_ref_store();
return refs->be->pack_refs(refs, flags);
}
int peel_ref(const char *refname, unsigned char *sha1)
{
- struct ref_store *refs = get_ref_store(NULL);
+ struct ref_store *refs = get_main_ref_store();
return refs->be->peel_ref(refs, refname, sha1);
}
@@ -1503,7 +1503,7 @@ int peel_ref(const char *refname, unsigned char *sha1)
int create_symref(const char *ref_target, const char *refs_heads_master,
const char *logmsg)
{
- struct ref_store *refs = get_ref_store(NULL);
+ struct ref_store *refs = get_main_ref_store();
return refs->be->create_symref(refs, ref_target, refs_heads_master,
logmsg);
@@ -1512,7 +1512,7 @@ int create_symref(const char *ref_target, const char *refs_heads_master,
int ref_transaction_commit(struct ref_transaction *transaction,
struct strbuf *err)
{
- struct ref_store *refs = get_ref_store(NULL);
+ struct ref_store *refs = get_main_ref_store();
return refs->be->transaction_commit(refs, transaction, err);
}
@@ -1522,14 +1522,14 @@ int verify_refname_available(const char *refname,
const struct string_list *skip,
struct strbuf *err)
{
- struct ref_store *refs = get_ref_store(NULL);
+ struct ref_store *refs = get_main_ref_store();
return refs->be->verify_refname_available(refs, refname, extra, skip, err);
}
int for_each_reflog(each_ref_fn fn, void *cb_data)
{
- struct ref_store *refs = get_ref_store(NULL);
+ struct ref_store *refs = get_main_ref_store();
struct ref_iterator *iter;
iter = refs->be->reflog_iterator_begin(refs);
@@ -1540,7 +1540,7 @@ int for_each_reflog(each_ref_fn fn, void *cb_data)
int for_each_reflog_ent_reverse(const char *refname, each_reflog_ent_fn fn,
void *cb_data)
{
- struct ref_store *refs = get_ref_store(NULL);
+ struct ref_store *refs = get_main_ref_store();
return refs->be->for_each_reflog_ent_reverse(refs, refname,
fn, cb_data);
@@ -1549,14 +1549,14 @@ int for_each_reflog_ent_reverse(const char *refname, each_reflog_ent_fn fn,
int for_each_reflog_ent(const char *refname, each_reflog_ent_fn fn,
void *cb_data)
{
- struct ref_store *refs = get_ref_store(NULL);
+ struct ref_store *refs = get_main_ref_store();
return refs->be->for_each_reflog_ent(refs, refname, fn, cb_data);
}
int reflog_exists(const char *refname)
{
- struct ref_store *refs = get_ref_store(NULL);
+ struct ref_store *refs = get_main_ref_store();
return refs->be->reflog_exists(refs, refname);
}
@@ -1564,14 +1564,14 @@ int reflog_exists(const char *refname)
int safe_create_reflog(const char *refname, int force_create,
struct strbuf *err)
{
- struct ref_store *refs = get_ref_store(NULL);
+ struct ref_store *refs = get_main_ref_store();
return refs->be->create_reflog(refs, refname, force_create, err);
}
int delete_reflog(const char *refname)
{
- struct ref_store *refs = get_ref_store(NULL);
+ struct ref_store *refs = get_main_ref_store();
return refs->be->delete_reflog(refs, refname);
}
@@ -1583,7 +1583,7 @@ int reflog_expire(const char *refname, const unsigned char *sha1,
reflog_expiry_cleanup_fn cleanup_fn,
void *policy_cb_data)
{
- struct ref_store *refs = get_ref_store(NULL);
+ struct ref_store *refs = get_main_ref_store();
return refs->be->reflog_expire(refs, refname, sha1, flags,
prepare_fn, should_prune_fn,
@@ -1593,21 +1593,21 @@ int reflog_expire(const char *refname, const unsigned char *sha1,
int initial_ref_transaction_commit(struct ref_transaction *transaction,
struct strbuf *err)
{
- struct ref_store *refs = get_ref_store(NULL);
+ struct ref_store *refs = get_main_ref_store();
return refs->be->initial_transaction_commit(refs, transaction, err);
}
int delete_refs(struct string_list *refnames, unsigned int flags)
{
- struct ref_store *refs = get_ref_store(NULL);
+ struct ref_store *refs = get_main_ref_store();
return refs->be->delete_refs(refs, refnames, flags);
}
int rename_ref(const char *oldref, const char *newref, const char *logmsg)
{
- struct ref_store *refs = get_ref_store(NULL);
+ struct ref_store *refs = get_main_ref_store();
return refs->be->rename_ref(refs, oldref, newref, logmsg);
}
diff --git a/refs.h b/refs.h
index 9fbff90e7..a35b687d7 100644
--- a/refs.h
+++ b/refs.h
@@ -555,4 +555,6 @@ int reflog_expire(const char *refname, const unsigned char *sha1,
int ref_storage_backend_exists(const char *name);
+struct ref_store *get_main_ref_store(void);
+
#endif /* REFS_H */
--
2.11.0.157.gd943d85
^ permalink raw reply related
* [PATCH v4 10/15] refs.c: kill register_ref_store(), add register_submodule_ref_store()
From: Nguyễn Thái Ngọc Duy @ 2017-02-18 13:32 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: <20170218133303.3682-1-pclouds@gmail.com>
This is the last function in this code (besides public API) that takes
submodule argument and handles both main/submodule cases. Break it down,
move main store registration in get_main_ref_store() and keep the rest
in register_submodule_ref_store().
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
refs.c | 36 +++++++++++++++++++-----------------
1 file changed, 19 insertions(+), 17 deletions(-)
diff --git a/refs.c b/refs.c
index 549eeccb4..8b640bd05 100644
--- a/refs.c
+++ b/refs.c
@@ -1407,25 +1407,19 @@ static struct ref_store *lookup_submodule_ref_store(const char *submodule)
/*
* Register the specified ref_store to be the one that should be used
- * for submodule (or the main repository if submodule is NULL). It is
- * a fatal error to call this function twice for the same submodule.
+ * for submodule. It is a fatal error to call this function twice for
+ * the same submodule.
*/
-static void register_ref_store(struct ref_store *refs, const char *submodule)
+static void register_submodule_ref_store(struct ref_store *refs,
+ const char *submodule)
{
- if (!submodule) {
- if (main_ref_store)
- die("BUG: main_ref_store initialized twice");
-
- main_ref_store = refs;
- } else {
- if (!submodule_ref_stores.tablesize)
- hashmap_init(&submodule_ref_stores, submodule_hash_cmp, 0);
+ if (!submodule_ref_stores.tablesize)
+ hashmap_init(&submodule_ref_stores, submodule_hash_cmp, 0);
- if (hashmap_put(&submodule_ref_stores,
- alloc_submodule_hash_entry(submodule, refs)))
- die("BUG: ref_store for submodule '%s' initialized twice",
- submodule);
- }
+ if (hashmap_put(&submodule_ref_stores,
+ alloc_submodule_hash_entry(submodule, refs)))
+ die("BUG: ref_store for submodule '%s' initialized twice",
+ submodule);
}
/*
@@ -1442,7 +1436,6 @@ static struct ref_store *ref_store_init(const char *submodule)
die("BUG: reference backend %s is unknown", be_name);
refs = be->init(submodule);
- register_ref_store(refs, submodule);
return refs;
}
@@ -1454,6 +1447,12 @@ static struct ref_store *get_main_ref_store(void)
return main_ref_store;
refs = ref_store_init(NULL);
+ if (refs) {
+ if (main_ref_store)
+ die("BUG: main_ref_store initialized twice");
+
+ main_ref_store = refs;
+ }
return refs;
}
@@ -1474,6 +1473,9 @@ struct ref_store *get_ref_store(const char *submodule)
if (is_nonbare_repository_dir(&submodule_sb))
refs = ref_store_init(submodule);
strbuf_release(&submodule_sb);
+
+ if (refs)
+ register_submodule_ref_store(refs, submodule);
return refs;
}
--
2.11.0.157.gd943d85
^ permalink raw reply related
* [PATCH v4 09/15] refs.c: flatten get_ref_store() a bit
From: Nguyễn Thái Ngọc Duy @ 2017-02-18 13:32 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: <20170218133303.3682-1-pclouds@gmail.com>
This helps the future changes in this code. And because get_ref_store()
is destined to become get_submodule_ref_store(), the "get main store"
code path will be removed eventually. After this the patch to delete
that code will be cleaner.
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
refs.c | 19 +++++++++----------
1 file changed, 9 insertions(+), 10 deletions(-)
diff --git a/refs.c b/refs.c
index 6aab5e4ac..549eeccb4 100644
--- a/refs.c
+++ b/refs.c
@@ -1459,22 +1459,21 @@ static struct ref_store *get_main_ref_store(void)
struct ref_store *get_ref_store(const char *submodule)
{
+ struct strbuf submodule_sb = STRBUF_INIT;
struct ref_store *refs;
if (!submodule || !*submodule) {
return get_main_ref_store();
- } else {
- refs = lookup_submodule_ref_store(submodule);
+ }
- if (!refs) {
- struct strbuf submodule_sb = STRBUF_INIT;
+ refs = lookup_submodule_ref_store(submodule);
+ if (refs)
+ return refs;
- strbuf_addstr(&submodule_sb, submodule);
- if (is_nonbare_repository_dir(&submodule_sb))
- refs = ref_store_init(submodule);
- strbuf_release(&submodule_sb);
- }
- }
+ strbuf_addstr(&submodule_sb, submodule);
+ if (is_nonbare_repository_dir(&submodule_sb))
+ refs = ref_store_init(submodule);
+ strbuf_release(&submodule_sb);
return refs;
}
--
2.11.0.157.gd943d85
^ permalink raw reply related
* [PATCH v4 08/15] refs: rename lookup_ref_store() to lookup_submodule_ref_store()
From: Nguyễn Thái Ngọc Duy @ 2017-02-18 13:32 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: <20170218133303.3682-1-pclouds@gmail.com>
With get_main_ref_store() being used inside get_ref_store(),
lookup_ref_store() is only used for submodule code path. Rename to
reflect that and delete dead code.
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
refs.c | 13 ++++---------
1 file changed, 4 insertions(+), 9 deletions(-)
diff --git a/refs.c b/refs.c
index c3bdc99d8..6aab5e4ac 100644
--- a/refs.c
+++ b/refs.c
@@ -1389,17 +1389,13 @@ static struct ref_store *main_ref_store;
static struct hashmap submodule_ref_stores;
/*
- * Return the ref_store instance for the specified submodule (or the
- * main repository if submodule is NULL). If that ref_store hasn't
- * been initialized yet, return NULL.
+ * Return the ref_store instance for the specified submodule. If that
+ * ref_store hasn't been initialized yet, return NULL.
*/
-static struct ref_store *lookup_ref_store(const char *submodule)
+static struct ref_store *lookup_submodule_ref_store(const char *submodule)
{
struct submodule_hash_entry *entry;
- if (!submodule)
- return main_ref_store;
-
if (!submodule_ref_stores.tablesize)
/* It's initialized on demand in register_ref_store(). */
return NULL;
@@ -1468,7 +1464,7 @@ struct ref_store *get_ref_store(const char *submodule)
if (!submodule || !*submodule) {
return get_main_ref_store();
} else {
- refs = lookup_ref_store(submodule);
+ refs = lookup_submodule_ref_store(submodule);
if (!refs) {
struct strbuf submodule_sb = STRBUF_INIT;
@@ -1479,7 +1475,6 @@ struct ref_store *get_ref_store(const char *submodule)
strbuf_release(&submodule_sb);
}
}
-
return refs;
}
--
2.11.0.157.gd943d85
^ permalink raw reply related
* [PATCH v4 07/15] refs.c: introduce get_main_ref_store()
From: Nguyễn Thái Ngọc Duy @ 2017-02-18 13:32 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: <20170218133303.3682-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 c6af84357..c3bdc99d8 100644
--- a/refs.c
+++ b/refs.c
@@ -1450,15 +1450,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
* [PATCH v4 06/15] files-backend: remove the use of git_path()
From: Nguyễn Thái Ngọc Duy @ 2017-02-18 13:32 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: <20170218133303.3682-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 | 37 +++++++++++++++++++++++++++++++++----
1 file changed, 33 insertions(+), 4 deletions(-)
diff --git a/refs/files-backend.c b/refs/files-backend.c
index b599ddf92..dbcaf9bda 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,15 +940,33 @@ 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);
va_end(vap);
- if (refs->submodule)
+ if (refs->submodule) {
strbuf_git_path_submodule(sb, refs->submodule,
"%s", tmp.buf);
- else
- strbuf_git_path(sb, "%s", tmp.buf);
+ } else if (!strcmp(tmp.buf, "packed-refs") ||
+ !strcmp(tmp.buf, "logs")) { /* non refname path */
+ strbuf_addf(sb, "%s/%s", refs->gitcommondir.buf, tmp.buf);
+ } else if (skip_prefix(tmp.buf, "logs/", &ref)) { /* reflog */
+ if (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);
+ } else { /* refname */
+ switch (ref_type(tmp.buf)) {
+ case REF_TYPE_PER_WORKTREE:
+ case REF_TYPE_PSEUDOREF:
+ strbuf_addf(sb, "%s/%s", refs->gitdir.buf, tmp.buf);
+ break;
+ case REF_TYPE_NORMAL:
+ strbuf_addf(sb, "%s/%s", refs->gitcommondir.buf, tmp.buf);
+ break;
+ }
+ }
strbuf_release(&tmp);
}
@@ -1004,7 +1025,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 v4 05/15] refs.c: share is_per_worktree_ref() to files-backend.c
From: Nguyễn Thái Ngọc Duy @ 2017-02-18 13:32 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: <20170218133303.3682-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 81b64b4ed..c6af84357 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 f732473e1..8c5febf54 100644
--- a/refs/refs-internal.h
+++ b/refs/refs-internal.h
@@ -657,4 +657,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 v4 04/15] files-backend: replace *git_path*() with files_path()
From: Nguyễn Thái Ngọc Duy @ 2017-02-18 13:32 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: <20170218133303.3682-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 | 208 +++++++++++++++++++++++++++------------------------
1 file changed, 111 insertions(+), 97 deletions(-)
diff --git a/refs/files-backend.c b/refs/files-backend.c
index 7eaf28bd6..b599ddf92 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)) {
@@ -2057,7 +2050,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) {
@@ -2179,7 +2172,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);
@@ -2332,7 +2325,9 @@ enum {
* subdirs. flags is a combination of REMOVE_EMPTY_PARENTS_REF and/or
* REMOVE_EMPTY_PARENTS_REFLOG.
*/
-static void try_remove_empty_parents(const char *refname, unsigned int flags)
+static void try_remove_empty_parents(struct files_ref_store *refs,
+ const char *refname,
+ unsigned int flags)
{
struct strbuf buf = STRBUF_INIT;
struct strbuf sb = STRBUF_INIT;
@@ -2359,12 +2354,12 @@ static void try_remove_empty_parents(const char *refname, unsigned int flags)
strbuf_setlen(&buf, q - buf.buf);
strbuf_reset(&sb);
- strbuf_git_path(&sb, "%s", buf.buf);
+ files_path(refs, &sb, "%s", buf.buf);
if ((flags & REMOVE_EMPTY_PARENTS_REF) && rmdir(sb.buf))
flags &= ~REMOVE_EMPTY_PARENTS_REF;
strbuf_reset(&sb);
- strbuf_git_path(&sb, "logs/%s", buf.buf);
+ files_path(refs, &sb, "logs/%s", buf.buf);
if ((flags & REMOVE_EMPTY_PARENTS_REFLOG) && rmdir(sb.buf))
flags &= ~REMOVE_EMPTY_PARENTS_REFLOG;
}
@@ -2373,7 +2368,7 @@ static void try_remove_empty_parents(const char *refname, unsigned int flags)
}
/* 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;
@@ -2395,10 +2390,10 @@ static void prune_ref(struct ref_to_prune *r)
strbuf_release(&err);
}
-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;
}
}
@@ -2421,7 +2416,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;
}
@@ -2457,7 +2452,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;
@@ -2535,12 +2530,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_callback(const char *path, void *cb)
+struct rename_tmp_log_cb {
+ struct files_ref_store *refs;
+ int true_errno;
+};
+
+static int rename_tmp_log_callback(const char *path, void *cb_)
{
struct strbuf sb = STRBUF_INIT;
- int *true_errno = cb;
+ struct rename_tmp_log_cb *cb = cb_;
- strbuf_git_path(&sb, TMP_RENAMED_LOG);
+ files_path(cb->refs, &sb, TMP_RENAMED_LOG);
if (rename(sb.buf, path)) {
/*
* rename(a, b) when b is an existing directory ought
@@ -2549,7 +2549,7 @@ static int rename_tmp_log_callback(const char *path, void *cb)
* but report EISDIR to raceproof_create_file() so
* that it knows to retry.
*/
- *true_errno = errno;
+ cb->true_errno = errno;
strbuf_release(&sb);
if (errno == ENOTDIR)
errno = EISDIR;
@@ -2560,25 +2560,27 @@ static int rename_tmp_log_callback(const char *path, void *cb)
}
}
-static int rename_tmp_log(const char *newrefname)
+static int rename_tmp_log(struct files_ref_store *refs, const char *newrefname)
{
struct strbuf sb = STRBUF_INIT;
+ struct rename_tmp_log_cb cb;
const char *path;
- int ret, true_errno;
+ int ret;
- strbuf_git_path(&sb, "logs/%s", newrefname);
+ files_path(refs, &sb, "logs/%s", newrefname);
path = sb.buf;
- ret = raceproof_create_file(path, rename_tmp_log_callback, &true_errno);
+ cb.refs = refs;
+ ret = raceproof_create_file(path, rename_tmp_log_callback, &cb);
if (ret) {
if (errno == EISDIR)
error("directory not empty: %s", path);
else {
struct strbuf tmp_renamed = STRBUF_INIT;
- strbuf_git_path(&tmp_renamed, TMP_RENAMED_LOG);
+ files_path(refs, &tmp_renamed, TMP_RENAMED_LOG);
error("unable to move logfile %s to %s: %s",
tmp_renamed.buf, path,
- strerror(true_errno));
+ strerror(cb.true_errno));
strbuf_release(&tmp_renamed);
}
}
@@ -2630,7 +2632,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))
@@ -2646,8 +2648,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);
@@ -2674,7 +2676,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);
@@ -2688,7 +2690,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;
@@ -2730,12 +2732,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",
@@ -2803,10 +2805,15 @@ static int open_or_create_logfile(const char *path, void *cb)
* set *logfd to -1. On failure, fill in *err, set *logfd to -1, and
* return -1.
*/
-static int log_ref_setup(const char *refname, int force_create,
+static int log_ref_setup(struct files_ref_store *refs,
+ const char *refname, int force_create,
int *logfd, struct strbuf *err)
{
- char *logfile = git_pathdup("logs/%s", refname);
+ struct strbuf sb = STRBUF_INIT;
+ char *logfile;
+
+ files_path(refs, &sb, "logs/%s", refname);
+ logfile = strbuf_detach(&sb, NULL);
if (force_create || should_autocreate_reflog(refname)) {
if (raceproof_create_file(logfile, open_or_create_logfile, logfd)) {
@@ -2856,12 +2863,11 @@ static int files_create_reflog(struct ref_store *ref_store,
const char *refname, int force_create,
struct strbuf *err)
{
+ struct files_ref_store *refs =
+ files_downcast(ref_store, 0, "create_reflog");
int fd;
- /* Check validity (but we don't need the result): */
- files_downcast(ref_store, 0, "create_reflog");
-
- if (log_ref_setup(refname, force_create, &fd, err))
+ if (log_ref_setup(refs, refname, force_create, &fd, err))
return -1;
if (fd >= 0)
@@ -2896,7 +2902,8 @@ static int log_ref_write_fd(int fd, const unsigned char *old_sha1,
return 0;
}
-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)
{
@@ -2905,7 +2912,7 @@ int files_log_ref_write(const char *refname, const unsigned char *old_sha1,
if (log_all_ref_updates == LOG_REFS_UNSET)
log_all_ref_updates = is_bare_repository() ? LOG_REFS_NONE : LOG_REFS_NORMAL;
- result = log_ref_setup(refname, flags & REF_FORCE_CREATE_REFLOG,
+ result = log_ref_setup(refs, refname, flags & REF_FORCE_CREATE_REFLOG,
&logfd, err);
if (result)
@@ -2919,7 +2926,7 @@ int files_log_ref_write(const char *refname, const unsigned char *old_sha1,
struct strbuf sb = STRBUF_INIT;
int save_errno = errno;
- strbuf_git_path(&sb, "logs/%s", refname);
+ files_path(refs, &sb, "logs/%s", refname);
strbuf_addf(err, "unable to append to '%s': %s",
sb.buf, strerror(save_errno));
strbuf_release(&sb);
@@ -2930,7 +2937,7 @@ int files_log_ref_write(const char *refname, const unsigned char *old_sha1,
struct strbuf sb = STRBUF_INIT;
int save_errno = errno;
- strbuf_git_path(&sb, "logs/%s", refname);
+ files_path(refs, &sb, "logs/%s", refname);
strbuf_addf(err, "unable to append to '%s': %s",
sb.buf, strerror(save_errno));
strbuf_release(&sb);
@@ -2991,8 +2998,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 (files_log_ref_write(lock->ref_name, lock->old_oid.hash, sha1,
- logmsg, 0, err)) {
+ if (files_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);
@@ -3023,8 +3030,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 (files_log_ref_write("HEAD", lock->old_oid.hash, sha1,
- logmsg, 0, &log_err)) {
+ if (files_log_ref_write(refs, "HEAD", lock->old_oid.hash,
+ sha1, logmsg, 0, &log_err)) {
error("%s", log_err.buf);
strbuf_release(&log_err);
}
@@ -3056,24 +3063,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) &&
- files_log_ref_write(refname, lock->old_oid.hash, new_sha1,
+ files_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;
}
@@ -3081,7 +3090,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);
@@ -3110,13 +3119,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;
@@ -3143,7 +3158,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);
@@ -3153,14 +3168,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;
@@ -3169,13 +3183,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;
@@ -3225,15 +3238,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)
@@ -3334,14 +3346,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)
@@ -3423,15 +3434,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;
@@ -3856,7 +3866,8 @@ static int files_transaction_commit(struct ref_store *ref_store,
if (update->flags & REF_NEEDS_COMMIT ||
update->flags & REF_LOG_ONLY) {
- if (files_log_ref_write(lock->ref_name,
+ if (files_log_ref_write(refs,
+ lock->ref_name,
lock->old_oid.hash,
update->new_sha1,
update->msg, update->flags,
@@ -3894,7 +3905,7 @@ static int files_transaction_commit(struct ref_store *ref_store,
update->type & REF_ISSYMREF) {
/* It is a loose reference. */
strbuf_reset(&sb);
- strbuf_git_path(&sb, "%s", lock->ref_name);
+ files_path(refs, &sb, "%s", lock->ref_name);
if (unlink_or_msg(sb.buf, err)) {
ret = TRANSACTION_GENERIC_ERROR;
goto cleanup;
@@ -3917,9 +3928,10 @@ 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);
if (!unlink_or_warn(sb.buf))
- try_remove_empty_parents(ref_to_delete->string,
+ try_remove_empty_parents(refs,
+ ref_to_delete->string,
REMOVE_EMPTY_PARENTS_REFLOG);
strbuf_release(&sb);
}
@@ -3943,7 +3955,8 @@ static int files_transaction_commit(struct ref_store *ref_store,
* can only work because we have already
* removed the lockfile.)
*/
- try_remove_empty_parents(update->refname,
+ try_remove_empty_parents(refs,
+ update->refname,
REMOVE_EMPTY_PARENTS_REF);
}
}
@@ -4098,6 +4111,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;
@@ -4122,7 +4136,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
@@ -4193,25 +4208,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 v4 03/15] files-backend: add files_path()
From: Nguyễn Thái Ngọc Duy @ 2017-02-18 13:32 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: <20170218133303.3682-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 replaced 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 c6c86f850..7eaf28bd6 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 v4 02/15] files-backend: convert git_path() to strbuf_git_path()
From: Nguyễn Thái Ngọc Duy @ 2017-02-18 13:32 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: <20170218133303.3682-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 | 146 ++++++++++++++++++++++++++++++++++++++++-----------
1 file changed, 115 insertions(+), 31 deletions(-)
diff --git a/refs/files-backend.c b/refs/files-backend.c
index 1ebd59ec0..c6c86f850 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -2151,6 +2151,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");
@@ -2159,10 +2161,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,
@@ -2312,6 +2317,7 @@ enum {
static void try_remove_empty_parents(const char *refname, unsigned int flags)
{
struct strbuf buf = STRBUF_INIT;
+ struct strbuf sb = STRBUF_INIT;
char *p, *q;
int i;
@@ -2333,14 +2339,19 @@ static void try_remove_empty_parents(const char *refname, unsigned int flags)
if (q == p)
break;
strbuf_setlen(&buf, q - buf.buf);
- if ((flags & REMOVE_EMPTY_PARENTS_REF) &&
- rmdir(git_path("%s", buf.buf)))
+
+ strbuf_reset(&sb);
+ strbuf_git_path(&sb, "%s", buf.buf);
+ if ((flags & REMOVE_EMPTY_PARENTS_REF) && rmdir(sb.buf))
flags &= ~REMOVE_EMPTY_PARENTS_REF;
- if ((flags & REMOVE_EMPTY_PARENTS_REFLOG) &&
- rmdir(git_path("logs/%s", buf.buf)))
+
+ strbuf_reset(&sb);
+ strbuf_git_path(&sb, "logs/%s", buf.buf);
+ if ((flags & REMOVE_EMPTY_PARENTS_REFLOG) && rmdir(sb.buf))
flags &= ~REMOVE_EMPTY_PARENTS_REFLOG;
}
strbuf_release(&buf);
+ strbuf_release(&sb);
}
/* make sure nobody touched the ref, and unlink */
@@ -2426,7 +2437,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);
@@ -2504,9 +2519,11 @@ static int files_delete_refs(struct ref_store *ref_store,
static int rename_tmp_log_callback(const char *path, void *cb)
{
+ struct strbuf sb = STRBUF_INIT;
int *true_errno = cb;
- if (rename(git_path(TMP_RENAMED_LOG), path)) {
+ strbuf_git_path(&sb, TMP_RENAMED_LOG);
+ if (rename(sb.buf, path)) {
/*
* rename(a, b) when b is an existing directory ought
* to result in ISDIR, but Solaris 5.8 gives ENOTDIR.
@@ -2515,30 +2532,40 @@ static int rename_tmp_log_callback(const char *path, void *cb)
* that it knows to retry.
*/
*true_errno = errno;
+ strbuf_release(&sb);
if (errno == ENOTDIR)
errno = EISDIR;
return -1;
} else {
+ strbuf_release(&sb);
return 0;
}
}
static int rename_tmp_log(const char *newrefname)
{
- char *path = git_pathdup("logs/%s", newrefname);
+ struct strbuf sb = STRBUF_INIT;
+ const char *path;
int ret, true_errno;
+ strbuf_git_path(&sb, "logs/%s", newrefname);
+ path = sb.buf;
ret = raceproof_create_file(path, rename_tmp_log_callback, &true_errno);
if (ret) {
if (errno == EISDIR)
error("directory not empty: %s", path);
- else
+ else {
+ struct strbuf tmp_renamed = STRBUF_INIT;
+
+ strbuf_git_path(&tmp_renamed, TMP_RENAMED_LOG);
error("unable to move logfile %s to %s: %s",
- git_path(TMP_RENAMED_LOG), path,
+ tmp_renamed.buf, path,
strerror(true_errno));
+ strbuf_release(&tmp_renamed);
+ }
}
- free(path);
+ strbuf_release(&sb);
return ret;
}
@@ -2579,9 +2606,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);
@@ -2595,7 +2628,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));
@@ -2674,13 +2712,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;
}
@@ -2854,18 +2898,24 @@ int files_log_ref_write(const char *refname, const unsigned char *old_sha1,
result = log_ref_write_fd(logfd, old_sha1, new_sha1,
git_committer_info(0), msg);
if (result) {
+ struct strbuf sb = STRBUF_INIT;
int save_errno = errno;
+ strbuf_git_path(&sb, "logs/%s", refname);
strbuf_addf(err, "unable to append to '%s': %s",
- git_path("logs/%s", refname), strerror(save_errno));
+ sb.buf, strerror(save_errno));
+ strbuf_release(&sb);
close(logfd);
return -1;
}
if (close(logfd)) {
+ struct strbuf sb = STRBUF_INIT;
int save_errno = errno;
+ strbuf_git_path(&sb, "logs/%s", refname);
strbuf_addf(err, "unable to append to '%s': %s",
- git_path("logs/%s", refname), strerror(save_errno));
+ sb.buf, strerror(save_errno));
+ strbuf_release(&sb);
return -1;
}
return 0;
@@ -3085,22 +3135,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)
@@ -3155,7 +3215,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;
@@ -3261,7 +3323,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;
@@ -3343,12 +3407,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;
}
@@ -3686,6 +3753,7 @@ static int files_transaction_commit(struct ref_store *ref_store,
char *head_ref = NULL;
int head_type;
struct object_id head_oid;
+ struct strbuf sb = STRBUF_INIT;
assert(err);
@@ -3807,7 +3875,9 @@ static int files_transaction_commit(struct ref_store *ref_store,
if (!(update->type & REF_ISPACKED) ||
update->type & REF_ISSYMREF) {
/* It is a loose reference. */
- if (unlink_or_msg(git_path("%s", lock->ref_name), err)) {
+ strbuf_reset(&sb);
+ strbuf_git_path(&sb, "%s", lock->ref_name);
+ if (unlink_or_msg(sb.buf, err)) {
ret = TRANSACTION_GENERIC_ERROR;
goto cleanup;
}
@@ -3827,9 +3897,13 @@ static int files_transaction_commit(struct ref_store *ref_store,
/* Delete the reflogs of any references that were deleted: */
for_each_string_list_item(ref_to_delete, &refs_to_delete) {
- if (!unlink_or_warn(git_path("logs/%s", ref_to_delete->string)))
+ struct strbuf sb = STRBUF_INIT;
+
+ strbuf_git_path(&sb, "logs/%s", ref_to_delete->string);
+ if (!unlink_or_warn(sb.buf))
try_remove_empty_parents(ref_to_delete->string,
REMOVE_EMPTY_PARENTS_REFLOG);
+ strbuf_release(&sb);
}
clear_loose_ref_cache(refs);
@@ -4101,18 +4175,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 v4 01/15] refs-internal.c: make files_log_ref_write() static
From: Nguyễn Thái Ngọc Duy @ 2017-02-18 13:32 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: <20170218133303.3682-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 db3bd42a9..1ebd59ec0 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 fa93c9a32..f732473e1 100644
--- a/refs/refs-internal.h
+++ b/refs/refs-internal.h
@@ -228,10 +228,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 v4 00/15] Remove submodule from files-backend.c
From: Nguyễn Thái Ngọc Duy @ 2017-02-18 13:32 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>
v4:
- is based on master+mh/ref-remove-empty-directory+mh/submodule-hash
instead of mh/submodule-hash alone. It could merge with pu cleanly.
- 06/16 from v5, "correct is_per_worktree_ref", is removed. The
"remove the use of git_path" patch is updated to handle it
correctly even without that patch. More on this later.
- update "replace" typo in a commit message
The story behind the 06/16 removal is, in my simple view, there are
per-repo and per-worktree refs. But refs.c and backends see them as
three types: per-worktree, pseudo and normal/per-repo. Refs like
ORIG_HEAD are _both_ per-worktree and pseudo.
06/16 reclassifies ORIG_HEAD from pseudo to per-worktree, which
triggers a failure in t1400 because as pseudo refs, reflog will not be
created while it will be for per-worktree. This new test is on master,
not maint. I now fall back to treating both per-worktree and pseudo as
per-worktree in files_path() only and keep classification unchanged.
Side note, my other two series seem to apply cleanly on top of this
v3, so no resend.
Nguyễn Thái Ngọc Duy (15):
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
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 | 406 ++++++++++++++++++++++++++++++++-------------------
refs/refs-internal.h | 28 ++--
submodule.c | 31 ++++
submodule.h | 1 +
7 files changed, 396 insertions(+), 261 deletions(-)
--
2.11.0.157.gd943d85
^ permalink raw reply
* Re: [PATCH v2 00/16] Remove submodule from files-backend.c
From: Duy Nguyen @ 2017-02-18 13:15 UTC (permalink / raw)
To: Junio C Hamano
Cc: Git Mailing List, Michael Haggerty, Johannes Schindelin,
Ramsay Jones, Stefan Beller, David Turner
In-Reply-To: <xmqqzihksm0i.fsf@gitster.mtv.corp.google.com>
On Sat, Feb 18, 2017 at 1:35 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Nguyễn Thái Ngọc Duy <pclouds@gmail.com> writes:
>
>> I'll be sending two more follow-up series, if you are interested, soon:
>>
>> 1) v2 of nd/worktree-gc-protection
>>
>> which kills parse_ref() in worktree.c _and_ set_worktree_head_symref()
>> in files-backend.c. Both are bad things that should not have happened.
>> (PS. The topic name is misleading as this is mostly about eliminating
>> warts, unless Junio intended to combine my second series as well)
>
> Your description sounded that these two are just preparatory step
> for the main one that would soon follow, and that was why these two
> patches landed on a topic named as such without any of its friends
> (which were yet to come). If you prefer to keep these a separate
> preparatory step from the remainder and have them graduate sooner,
> let's do so, as that is my preference as well.
>
> Rename it "nd/worktree-kill-parse-ref" perhaps?
That's exactly my branch name (minus the nd/)
--
Duy
^ permalink raw reply
* Re: [git-for-windows] Re: Continuous Testing of Git on Windows
From: Philip Oakley @ 2017-02-18 11:49 UTC (permalink / raw)
To: Junio C Hamano
Cc: Johannes Schindelin, Christian Couder, git-for-windows, git
In-Reply-To: <xmqq60kb0ywi.fsf@gitster.mtv.corp.google.com>
From: "Junio C Hamano" <gitster@pobox.com>
Sent: Thursday, February 16, 2017 12:20 AM
> "Philip Oakley" <philipoakley@iee.org> writes:
>
>> It may even be worth 'splitting' the pu branch sequence into the
>> existing pu (with merges from series that are selected as reasonable),
>> and then a pr branch (public review?) on top of that holding the rest
>> of the series that have been submitted, so that the CI can do a full
>> test on the tips of them to support those devs with limited test
>> capability.
>
> I won't stop you from publishing such a pr branch yourself.
But would others see it?... (rhetorical, better thoughts below))
>
> For patches whose merit is not clear because the problem they try to
> solve is under-explained, whose solution is ill-designed, etc., IOW,
> with issues that makes me judge that they are not interesting enough
> for 'pu',
It is reasonble that that a project's integrator is able to make these
decisions. For some projects they may have a layered approach of descisions
which does allow the gradations for the submitted feature series.
Some of this does fall into dscho's differentiation between those patch
series that should pass the CI (continuous integration) testing, and those
that are there for CT (continuous testing) feedback. This could either be an
extra branch marking the transition, or a named commit similar to the
"pu^{/^### match next}", etc. In some ways it is similar to my 'pr'
suggestion, without the inclusion of the 'all and sundry' series.
For for integrators who are willing/want to recieve any/all contributions
for public view (usually those for projects of a more lenient and less
critical variety), then even the CT grouping could then have those
additional pr submissions. For Git, you provide that that 'voice of reason'
for gatekeeping the pu branch.
> it is not worth my time to deal with whitespace brekages
> in them to make them not even apply, to figure out what base the
> patches are meant to apply to, or to resolve conflicts caused by
> them with topics already in flight, etc.
>
If a centralised CT service was available to the project, maybe via the
GitHub PR process (which isn't curently used by the project) then is may be
a way of allowing the 'all and sundry' contributors to get their ideas upto
a basic level before even bothering yourself (because PRs do not trouble
you).
It may need an extra gatekeeper between the passing patches (PRs) and auto
submission (the Heroku script thingy) which could flood the list with with
inane changes - one only has to look at the
http://stackoverflow.com/questions/tagged/git stream to see that.
At least if there was a break point within pu that allowed differentiation
between the series that should fit a CI view, and those that are still at
the CT stage, then that may help.
Thanks
Philip.
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox