* [PATCH v2 1/4] commit: make 'commit_graft_pos' non-static
2020-04-30 19:48 [PATCH v2 0/4] shallow: extract a header file Taylor Blau
@ 2020-04-30 19:48 ` Taylor Blau
2020-04-30 20:55 ` Junio C Hamano
2020-04-30 19:48 ` [PATCH v2 2/4] shallow: extract a header file for shallow-related functions Taylor Blau
` (3 subsequent siblings)
4 siblings, 1 reply; 8+ messages in thread
From: Taylor Blau @ 2020-04-30 19:48 UTC (permalink / raw)
To: git; +Cc: gitster, jonathantanmy, jrnieder, sunshine
In the next patch, some functions will be moved from 'commit.c' to have
prototypes in a new 'shallow.h' and their implementations in
'shallow.c'.
Three functions in 'commit.c' use 'commit_graft_pos()' (they are
'register_commit_graft()', 'lookup_commit_graft()', and
'unregister_shallow()'). The first two of these will stay in 'commit.c',
but the latter will move to 'shallow.c', and thus needs
'commit_graft_pos' to be non-static.
Prepare for that by making 'commit_graft_pos' non-static so that it can
be called from both 'commit.c' and 'shallow.c'.
Signed-off-by: Taylor Blau <me@ttaylorr.com>
---
commit.c | 2 +-
commit.h | 1 +
2 files changed, 2 insertions(+), 1 deletion(-)
diff --git a/commit.c b/commit.c
index c7099daeac..b76f7d72be 100644
--- a/commit.c
+++ b/commit.c
@@ -110,7 +110,7 @@ static const unsigned char *commit_graft_sha1_access(size_t index, void *table)
return commit_graft_table[index]->oid.hash;
}
-static int commit_graft_pos(struct repository *r, const unsigned char *sha1)
+int commit_graft_pos(struct repository *r, const unsigned char *sha1)
{
return sha1_pos(sha1, r->parsed_objects->grafts,
r->parsed_objects->grafts_nr,
diff --git a/commit.h b/commit.h
index ab91d21131..eb42e8b6d2 100644
--- a/commit.h
+++ b/commit.h
@@ -236,6 +236,7 @@ struct commit_graft {
typedef int (*each_commit_graft_fn)(const struct commit_graft *, void *);
struct commit_graft *read_graft_line(struct strbuf *line);
+int commit_graft_pos(struct repository *r, const unsigned char *sha1);
int register_commit_graft(struct repository *r, struct commit_graft *, int);
void prepare_commit_graft(struct repository *r);
struct commit_graft *lookup_commit_graft(struct repository *r, const struct object_id *oid);
--
2.26.0.113.ge9739cdccc
^ permalink raw reply related [flat|nested] 8+ messages in thread* Re: [PATCH v2 1/4] commit: make 'commit_graft_pos' non-static
2020-04-30 19:48 ` [PATCH v2 1/4] commit: make 'commit_graft_pos' non-static Taylor Blau
@ 2020-04-30 20:55 ` Junio C Hamano
2020-04-30 21:11 ` Taylor Blau
0 siblings, 1 reply; 8+ messages in thread
From: Junio C Hamano @ 2020-04-30 20:55 UTC (permalink / raw)
To: Taylor Blau; +Cc: git, jonathantanmy, jrnieder, sunshine
Taylor Blau <me@ttaylorr.com> writes:
> -static int commit_graft_pos(struct repository *r, const unsigned char *sha1)
> +int commit_graft_pos(struct repository *r, const unsigned char *sha1)
> {
> return sha1_pos(sha1, r->parsed_objects->grafts,
> r->parsed_objects->grafts_nr,
> diff --git a/commit.h b/commit.h
> index ab91d21131..eb42e8b6d2 100644
> --- a/commit.h
> +++ b/commit.h
> @@ -236,6 +236,7 @@ struct commit_graft {
> typedef int (*each_commit_graft_fn)(const struct commit_graft *, void *);
>
> struct commit_graft *read_graft_line(struct strbuf *line);
> +int commit_graft_pos(struct repository *r, const unsigned char *sha1);
In an earlier exchange, I saw this:
>> - could include a comment saying that it's an index into
>> r->parsed_objects->grafts
>
> This and the below are both good ideas to me. I prefer this one, since
> we'd have to duplicate yet another static function
> ('commit_graft_sha1_access()' directly above) that is called by this
> one.
>
>> - I'm usually loathe to suggest unnecessary duplication of code, but
>> it might make sense to duplicate the function into shallow.c. Or
>> even to inline it there (in the single call site, that ends up
>> being pretty readable).
>
> I am not at all offended by duplication of code where it makes sense to
> do so, but having to duplicate two functions seems like we'd be better
> off simply documenting the function in commit.h.
and I think I agree with that direction. Forgot to add those
comments?
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH v2 1/4] commit: make 'commit_graft_pos' non-static
2020-04-30 20:55 ` Junio C Hamano
@ 2020-04-30 21:11 ` Taylor Blau
0 siblings, 0 replies; 8+ messages in thread
From: Taylor Blau @ 2020-04-30 21:11 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Taylor Blau, git, jonathantanmy, jrnieder, sunshine
On Thu, Apr 30, 2020 at 01:55:11PM -0700, Junio C Hamano wrote:
> Taylor Blau <me@ttaylorr.com> writes:
>
> > -static int commit_graft_pos(struct repository *r, const unsigned char *sha1)
> > +int commit_graft_pos(struct repository *r, const unsigned char *sha1)
> > {
> > return sha1_pos(sha1, r->parsed_objects->grafts,
> > r->parsed_objects->grafts_nr,
> > diff --git a/commit.h b/commit.h
> > index ab91d21131..eb42e8b6d2 100644
> > --- a/commit.h
> > +++ b/commit.h
> > @@ -236,6 +236,7 @@ struct commit_graft {
> > typedef int (*each_commit_graft_fn)(const struct commit_graft *, void *);
> >
> > struct commit_graft *read_graft_line(struct strbuf *line);
> > +int commit_graft_pos(struct repository *r, const unsigned char *sha1);
>
> In an earlier exchange, I saw this:
>
> >> - could include a comment saying that it's an index into
> >> r->parsed_objects->grafts
> >
> > This and the below are both good ideas to me. I prefer this one, since
> > we'd have to duplicate yet another static function
> > ('commit_graft_sha1_access()' directly above) that is called by this
> > one.
> >
> >> - I'm usually loathe to suggest unnecessary duplication of code, but
> >> it might make sense to duplicate the function into shallow.c. Or
> >> even to inline it there (in the single call site, that ends up
> >> being pretty readable).
> >
> > I am not at all offended by duplication of code where it makes sense to
> > do so, but having to duplicate two functions seems like we'd be better
> > off simply documenting the function in commit.h.
>
> and I think I agree with that direction. Forgot to add those
> comments?
Thanks for remembering. I did apply this locally, but must've dropped it
on the floor during a rebase. In any case, please swap in this patch
instead:
-- >8 --
Subject: [PATCH] commit: make 'commit_graft_pos' non-static
In the next patch, some functions will be moved from 'commit.c' to have
prototypes in a new 'shallow.h' and their implementations in
'shallow.c'.
Three functions in 'commit.c' use 'commit_graft_pos()' (they are
'register_commit_graft()', 'lookup_commit_graft()', and
'unregister_shallow()'). The first two of these will stay in 'commit.c',
but the latter will move to 'shallow.c', and thus needs
'commit_graft_pos' to be non-static.
Prepare for that by making 'commit_graft_pos' non-static so that it can
be called from both 'commit.c' and 'shallow.c'.
Signed-off-by: Taylor Blau <me@ttaylorr.com>
---
commit.c | 2 +-
commit.h | 2 ++
2 files changed, 3 insertions(+), 1 deletion(-)
diff --git a/commit.c b/commit.c
index c7099daeac..b76f7d72be 100644
--- a/commit.c
+++ b/commit.c
@@ -110,7 +110,7 @@ static const unsigned char *commit_graft_sha1_access(size_t index, void *table)
return commit_graft_table[index]->oid.hash;
}
-static int commit_graft_pos(struct repository *r, const unsigned char *sha1)
+int commit_graft_pos(struct repository *r, const unsigned char *sha1)
{
return sha1_pos(sha1, r->parsed_objects->grafts,
r->parsed_objects->grafts_nr,
diff --git a/commit.h b/commit.h
index ab91d21131..0fe1e1b570 100644
--- a/commit.h
+++ b/commit.h
@@ -236,6 +236,8 @@ struct commit_graft {
typedef int (*each_commit_graft_fn)(const struct commit_graft *, void *);
struct commit_graft *read_graft_line(struct strbuf *line);
+/* commit_graft_pos returns an index into r->parsed_objects->grafts. */
+int commit_graft_pos(struct repository *r, const unsigned char *sha1);
int register_commit_graft(struct repository *r, struct commit_graft *, int);
void prepare_commit_graft(struct repository *r);
struct commit_graft *lookup_commit_graft(struct repository *r, const struct object_id *oid);
--
2.26.0.113.ge9739cdccc
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH v2 2/4] shallow: extract a header file for shallow-related functions
2020-04-30 19:48 [PATCH v2 0/4] shallow: extract a header file Taylor Blau
2020-04-30 19:48 ` [PATCH v2 1/4] commit: make 'commit_graft_pos' non-static Taylor Blau
@ 2020-04-30 19:48 ` Taylor Blau
2020-04-30 19:48 ` [PATCH v2 3/4] shallow.h: document '{commit,rollback}_shallow_file' Taylor Blau
` (2 subsequent siblings)
4 siblings, 0 replies; 8+ messages in thread
From: Taylor Blau @ 2020-04-30 19:48 UTC (permalink / raw)
To: git; +Cc: gitster, jonathantanmy, jrnieder, sunshine
There are many functions in commit.h that are more related to shallow
repositories than they are to any sort of generic commit machinery.
Likely this began when there were only a few shallow-related functions,
and commit.h seemed a reasonable enough place to put them.
But, now there are a good number of shallow-related functions, and
placing them all in 'commit.h' doesn't make sense.
This patch extracts a 'shallow.h', which takes all of the declarations
from 'commit.h' for functions which already exist in 'shallow.c'. We
will bring the remaining shallow-related functions defined in 'commit.c'
in a subsequent patch.
For now, move only the ones that already are implemented in 'shallow.c',
and update the necessary includes.
Signed-off-by: Taylor Blau <me@ttaylorr.com>
---
builtin/fetch.c | 1 +
builtin/pack-objects.c | 1 +
builtin/prune.c | 1 +
builtin/receive-pack.c | 1 +
builtin/repack.c | 1 +
builtin/rev-parse.c | 1 +
commit-graph.c | 1 +
commit.c | 14 +--------
commit.h | 48 -------------------------------
environment.c | 1 +
fetch-pack.c | 1 +
git.c | 1 +
send-pack.c | 1 +
shallow.c | 14 +++++++++
shallow.h | 64 ++++++++++++++++++++++++++++++++++++++++++
upload-pack.c | 1 +
16 files changed, 91 insertions(+), 61 deletions(-)
create mode 100644 shallow.h
diff --git a/builtin/fetch.c b/builtin/fetch.c
index bf6bab80fa..a292acd1c0 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -27,6 +27,7 @@
#include "branch.h"
#include "promisor-remote.h"
#include "commit-graph.h"
+#include "shallow.h"
#define FORCED_UPDATES_DELAY_WARNING_IN_MS (10 * 1000)
diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index dc7c58ce3f..303ae312a9 100644
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -34,6 +34,7 @@
#include "dir.h"
#include "midx.h"
#include "trace2.h"
+#include "shallow.h"
#define IN_PACK(obj) oe_in_pack(&to_pack, obj)
#define SIZE(obj) oe_size(&to_pack, obj)
diff --git a/builtin/prune.c b/builtin/prune.c
index 2b76872ad2..028ff786d6 100644
--- a/builtin/prune.c
+++ b/builtin/prune.c
@@ -7,6 +7,7 @@
#include "parse-options.h"
#include "progress.h"
#include "object-store.h"
+#include "shallow.h"
static const char * const prune_usage[] = {
N_("git prune [-n] [-v] [--progress] [--expire <time>] [--] [<head>...]"),
diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index 652661fa99..8e3f9bc35f 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -28,6 +28,7 @@
#include "protocol.h"
#include "commit-reach.h"
#include "worktree.h"
+#include "shallow.h"
static const char * const receive_pack_usage[] = {
N_("git receive-pack <git-dir>"),
diff --git a/builtin/repack.c b/builtin/repack.c
index 0781763b06..2550603608 100644
--- a/builtin/repack.c
+++ b/builtin/repack.c
@@ -12,6 +12,7 @@
#include "packfile.h"
#include "object-store.h"
#include "promisor-remote.h"
+#include "shallow.h"
static int delta_base_offset = 1;
static int pack_kept_objects = -1;
diff --git a/builtin/rev-parse.c b/builtin/rev-parse.c
index 06056434ed..669dd2fd6f 100644
--- a/builtin/rev-parse.c
+++ b/builtin/rev-parse.c
@@ -16,6 +16,7 @@
#include "split-index.h"
#include "submodule.h"
#include "commit-reach.h"
+#include "shallow.h"
#define DO_REVS 1
#define DO_NOREV 2
diff --git a/commit-graph.c b/commit-graph.c
index f013a84e29..d316de5021 100644
--- a/commit-graph.c
+++ b/commit-graph.c
@@ -16,6 +16,7 @@
#include "hashmap.h"
#include "replace-object.h"
#include "progress.h"
+#include "shallow.h"
#define GRAPH_SIGNATURE 0x43475048 /* "CGPH" */
#define GRAPH_CHUNKID_OIDFANOUT 0x4f494446 /* "OIDF" */
diff --git a/commit.c b/commit.c
index b76f7d72be..87686a7055 100644
--- a/commit.c
+++ b/commit.c
@@ -20,6 +20,7 @@
#include "refs.h"
#include "commit-reach.h"
#include "run-command.h"
+#include "shallow.h"
static struct commit_extra_header *read_commit_extra_header_lines(const char *buf, size_t len, const char **);
@@ -245,19 +246,6 @@ int for_each_commit_graft(each_commit_graft_fn fn, void *cb_data)
return ret;
}
-int unregister_shallow(const struct object_id *oid)
-{
- int pos = commit_graft_pos(the_repository, oid->hash);
- if (pos < 0)
- return -1;
- if (pos + 1 < the_repository->parsed_objects->grafts_nr)
- MOVE_ARRAY(the_repository->parsed_objects->grafts + pos,
- the_repository->parsed_objects->grafts + pos + 1,
- the_repository->parsed_objects->grafts_nr - pos - 1);
- the_repository->parsed_objects->grafts_nr--;
- return 0;
-}
-
struct commit_buffer {
void *buffer;
unsigned long size;
diff --git a/commit.h b/commit.h
index eb42e8b6d2..5cd984939b 100644
--- a/commit.h
+++ b/commit.h
@@ -248,55 +248,7 @@ struct commit *get_fork_point(const char *refname, struct commit *commit);
struct oid_array;
struct ref;
-int register_shallow(struct repository *r, const struct object_id *oid);
-int unregister_shallow(const struct object_id *oid);
-int commit_shallow_file(struct repository *r, struct lock_file *lk);
-void rollback_shallow_file(struct repository *r, struct lock_file *lk);
int for_each_commit_graft(each_commit_graft_fn, void *);
-int is_repository_shallow(struct repository *r);
-struct commit_list *get_shallow_commits(struct object_array *heads,
- int depth, int shallow_flag, int not_shallow_flag);
-struct commit_list *get_shallow_commits_by_rev_list(
- int ac, const char **av, int shallow_flag, int not_shallow_flag);
-void set_alternate_shallow_file(struct repository *r, const char *path, int override);
-int write_shallow_commits(struct strbuf *out, int use_pack_protocol,
- const struct oid_array *extra);
-void setup_alternate_shallow(struct lock_file *shallow_lock,
- const char **alternate_shallow_file,
- const struct oid_array *extra);
-const char *setup_temporary_shallow(const struct oid_array *extra);
-void advertise_shallow_grafts(int);
-
-/*
- * Initialize with prepare_shallow_info() or zero-initialize (equivalent to
- * prepare_shallow_info with a NULL oid_array).
- */
-struct shallow_info {
- struct oid_array *shallow;
- int *ours, nr_ours;
- int *theirs, nr_theirs;
- struct oid_array *ref;
-
- /* for receive-pack */
- uint32_t **used_shallow;
- int *need_reachability_test;
- int *reachable;
- int *shallow_ref;
- struct commit **commits;
- int nr_commits;
-};
-
-void prepare_shallow_info(struct shallow_info *, struct oid_array *);
-void clear_shallow_info(struct shallow_info *);
-void remove_nonexistent_theirs_shallow(struct shallow_info *);
-void assign_shallow_commits_to_refs(struct shallow_info *info,
- uint32_t **used,
- int *ref_status);
-int delayed_reachability_test(struct shallow_info *si, int c);
-#define PRUNE_SHOW_ONLY 1
-#define PRUNE_QUICK 2
-void prune_shallow(unsigned options);
-extern struct trace_key trace_shallow;
int interactive_add(int argc, const char **argv, const char *prefix, int patch);
int run_add_interactive(const char *revision, const char *patch_mode,
diff --git a/environment.c b/environment.c
index 10c9061c43..aaca0e91ac 100644
--- a/environment.c
+++ b/environment.c
@@ -17,6 +17,7 @@
#include "argv-array.h"
#include "object-store.h"
#include "chdir-notify.h"
+#include "shallow.h"
int trust_executable_bit = 1;
int trust_ctime = 1;
diff --git a/fetch-pack.c b/fetch-pack.c
index a618f3b029..401d028e41 100644
--- a/fetch-pack.c
+++ b/fetch-pack.c
@@ -22,6 +22,7 @@
#include "connected.h"
#include "fetch-negotiator.h"
#include "fsck.h"
+#include "shallow.h"
static int transfer_unpack_limit = -1;
static int fetch_unpack_limit = -1;
diff --git a/git.c b/git.c
index b07198fe03..9b689143c6 100644
--- a/git.c
+++ b/git.c
@@ -4,6 +4,7 @@
#include "help.h"
#include "run-command.h"
#include "alias.h"
+#include "shallow.h"
#define RUN_SETUP (1<<0)
#define RUN_SETUP_GENTLY (1<<1)
diff --git a/send-pack.c b/send-pack.c
index 0407841ae8..e0ccfef75a 100644
--- a/send-pack.c
+++ b/send-pack.c
@@ -15,6 +15,7 @@
#include "sha1-array.h"
#include "gpg-interface.h"
#include "cache.h"
+#include "shallow.h"
int option_parse_push_signed(const struct option *opt,
const char *arg, int unset)
diff --git a/shallow.c b/shallow.c
index 5010a6c732..76e00893fe 100644
--- a/shallow.c
+++ b/shallow.c
@@ -14,6 +14,7 @@
#include "commit-slab.h"
#include "list-objects.h"
#include "commit-reach.h"
+#include "shallow.h"
void set_alternate_shallow_file(struct repository *r, const char *path, int override)
{
@@ -38,6 +39,19 @@ int register_shallow(struct repository *r, const struct object_id *oid)
return register_commit_graft(r, graft, 0);
}
+int unregister_shallow(const struct object_id *oid)
+{
+ int pos = commit_graft_pos(the_repository, oid->hash);
+ if (pos < 0)
+ return -1;
+ if (pos + 1 < the_repository->parsed_objects->grafts_nr)
+ MOVE_ARRAY(the_repository->parsed_objects->grafts + pos,
+ the_repository->parsed_objects->grafts + pos + 1,
+ the_repository->parsed_objects->grafts_nr - pos - 1);
+ the_repository->parsed_objects->grafts_nr--;
+ return 0;
+}
+
int is_repository_shallow(struct repository *r)
{
FILE *fp;
diff --git a/shallow.h b/shallow.h
new file mode 100644
index 0000000000..b50a85ed7e
--- /dev/null
+++ b/shallow.h
@@ -0,0 +1,64 @@
+#ifndef SHALLOW_H
+#define SHALLOW_H
+
+#include "lockfile.h"
+#include "object.h"
+#include "repository.h"
+#include "strbuf.h"
+
+void set_alternate_shallow_file(struct repository *r, const char *path, int override);
+int register_shallow(struct repository *r, const struct object_id *oid);
+int unregister_shallow(const struct object_id *oid);
+int is_repository_shallow(struct repository *r);
+int commit_shallow_file(struct repository *r, struct lock_file *lk);
+void rollback_shallow_file(struct repository *r, struct lock_file *lk);
+
+struct commit_list *get_shallow_commits(struct object_array *heads,
+ int depth, int shallow_flag, int not_shallow_flag);
+struct commit_list *get_shallow_commits_by_rev_list(
+ int ac, const char **av, int shallow_flag, int not_shallow_flag);
+int write_shallow_commits(struct strbuf *out, int use_pack_protocol,
+ const struct oid_array *extra);
+
+void setup_alternate_shallow(struct lock_file *shallow_lock,
+ const char **alternate_shallow_file,
+ const struct oid_array *extra);
+
+const char *setup_temporary_shallow(const struct oid_array *extra);
+
+void advertise_shallow_grafts(int);
+
+#define PRUNE_SHOW_ONLY 1
+#define PRUNE_QUICK 2
+void prune_shallow(unsigned options);
+
+/*
+ * Initialize with prepare_shallow_info() or zero-initialize (equivalent to
+ * prepare_shallow_info with a NULL oid_array).
+ */
+struct shallow_info {
+ struct oid_array *shallow;
+ int *ours, nr_ours;
+ int *theirs, nr_theirs;
+ struct oid_array *ref;
+
+ /* for receive-pack */
+ uint32_t **used_shallow;
+ int *need_reachability_test;
+ int *reachable;
+ int *shallow_ref;
+ struct commit **commits;
+ int nr_commits;
+};
+
+void prepare_shallow_info(struct shallow_info *, struct oid_array *);
+void clear_shallow_info(struct shallow_info *);
+void remove_nonexistent_theirs_shallow(struct shallow_info *);
+void assign_shallow_commits_to_refs(struct shallow_info *info,
+ uint32_t **used,
+ int *ref_status);
+int delayed_reachability_test(struct shallow_info *si, int c);
+
+extern struct trace_key trace_shallow;
+
+#endif /* SHALLOW_H */
diff --git a/upload-pack.c b/upload-pack.c
index c53249cac1..e71b068291 100644
--- a/upload-pack.c
+++ b/upload-pack.c
@@ -26,6 +26,7 @@
#include "serve.h"
#include "commit-graph.h"
#include "commit-reach.h"
+#include "shallow.h"
/* Remember to update object flag allocation in object.h */
#define THEY_HAVE (1u << 11)
--
2.26.0.113.ge9739cdccc
^ permalink raw reply related [flat|nested] 8+ messages in thread* [PATCH v2 3/4] shallow.h: document '{commit,rollback}_shallow_file'
2020-04-30 19:48 [PATCH v2 0/4] shallow: extract a header file Taylor Blau
2020-04-30 19:48 ` [PATCH v2 1/4] commit: make 'commit_graft_pos' non-static Taylor Blau
2020-04-30 19:48 ` [PATCH v2 2/4] shallow: extract a header file for shallow-related functions Taylor Blau
@ 2020-04-30 19:48 ` Taylor Blau
2020-04-30 19:48 ` [PATCH v2 4/4] shallow: use struct 'shallow_lock' for additional safety Taylor Blau
2020-04-30 19:49 ` [PATCH v2 0/4] shallow: extract a header file Taylor Blau
4 siblings, 0 replies; 8+ messages in thread
From: Taylor Blau @ 2020-04-30 19:48 UTC (permalink / raw)
To: git; +Cc: gitster, jonathantanmy, jrnieder, sunshine
When 'commit_shallow_file()' and 'rollback_shallow_file()' were
introduced, they did not have a documenting comment, when they could
have benefited from one.
Add a brief note about what these functions do, and make a special note
that they reset stat-validity checks.
Helped-by: Eric Sunshine <sunshine@sunshineco.com>
Signed-off-by: Taylor Blau <me@ttaylorr.com>
---
shallow.h | 2 ++
1 file changed, 2 insertions(+)
diff --git a/shallow.h b/shallow.h
index b50a85ed7e..b3bf200bf9 100644
--- a/shallow.h
+++ b/shallow.h
@@ -10,7 +10,9 @@ void set_alternate_shallow_file(struct repository *r, const char *path, int over
int register_shallow(struct repository *r, const struct object_id *oid);
int unregister_shallow(const struct object_id *oid);
int is_repository_shallow(struct repository *r);
+/* commit $GIT_DIR/shallow and reset stat-validity checks */
int commit_shallow_file(struct repository *r, struct lock_file *lk);
+/* rollback $GIT_DIR/shallow and reset stat-validity checks */
void rollback_shallow_file(struct repository *r, struct lock_file *lk);
struct commit_list *get_shallow_commits(struct object_array *heads,
--
2.26.0.113.ge9739cdccc
^ permalink raw reply related [flat|nested] 8+ messages in thread* [PATCH v2 4/4] shallow: use struct 'shallow_lock' for additional safety
2020-04-30 19:48 [PATCH v2 0/4] shallow: extract a header file Taylor Blau
` (2 preceding siblings ...)
2020-04-30 19:48 ` [PATCH v2 3/4] shallow.h: document '{commit,rollback}_shallow_file' Taylor Blau
@ 2020-04-30 19:48 ` Taylor Blau
2020-04-30 19:49 ` [PATCH v2 0/4] shallow: extract a header file Taylor Blau
4 siblings, 0 replies; 8+ messages in thread
From: Taylor Blau @ 2020-04-30 19:48 UTC (permalink / raw)
To: git; +Cc: gitster, jonathantanmy, jrnieder, sunshine
In previous patches, the functions 'commit_shallow_file' and
'rollback_shallow_file' were introduced to reset the shallowness
validity checks on a repository after potentially modifying
'.git/shallow'.
These functions can be made safer by wrapping the 'struct lockfile *' in
a new type, 'shallow_lock', so that they cannot be called with a raw
lock (and potentially misused by other code that happens to possess a
lockfile, but has nothing to do with shallowness).
This patch introduces that type as a thin wrapper around 'struct
lockfile', and updates the two aforementioned functions and their
callers to use it.
Suggested-by: Junio C Hamano <gitster@pobox.com>
Helped-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Taylor Blau <me@ttaylorr.com>
---
builtin/receive-pack.c | 2 +-
fetch-pack.c | 2 +-
shallow.c | 22 +++++++++++-----------
shallow.h | 21 ++++++++++++++++++---
4 files changed, 31 insertions(+), 16 deletions(-)
diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index 8e3f9bc35f..73cebc2f55 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -856,7 +856,7 @@ static void refuse_unconfigured_deny_delete_current(void)
static int command_singleton_iterator(void *cb_data, struct object_id *oid);
static int update_shallow_ref(struct command *cmd, struct shallow_info *si)
{
- struct lock_file shallow_lock = LOCK_INIT;
+ struct shallow_lock shallow_lock = SHALLOW_LOCK_INIT;
struct oid_array extra = OID_ARRAY_INIT;
struct check_connected_options opt = CHECK_CONNECTED_INIT;
uint32_t mask = 1 << (cmd->index % 32);
diff --git a/fetch-pack.c b/fetch-pack.c
index 401d028e41..27d2d08cc0 100644
--- a/fetch-pack.c
+++ b/fetch-pack.c
@@ -35,7 +35,7 @@ static int fetch_fsck_objects = -1;
static int transfer_fsck_objects = -1;
static int agent_supported;
static int server_supports_filtering;
-static struct lock_file shallow_lock;
+static struct shallow_lock shallow_lock;
static const char *alternate_shallow_file;
static struct strbuf fsck_msg_types = STRBUF_INIT;
diff --git a/shallow.c b/shallow.c
index 76e00893fe..7e0a41ea68 100644
--- a/shallow.c
+++ b/shallow.c
@@ -92,16 +92,16 @@ static void reset_repository_shallow(struct repository *r)
stat_validity_clear(r->parsed_objects->shallow_stat);
}
-int commit_shallow_file(struct repository *r, struct lock_file *lk)
+int commit_shallow_file(struct repository *r, struct shallow_lock *lk)
{
- int res = commit_lock_file(lk);
+ int res = commit_lock_file(&lk->lock);
reset_repository_shallow(r);
return res;
}
-void rollback_shallow_file(struct repository *r, struct lock_file *lk)
+void rollback_shallow_file(struct repository *r, struct shallow_lock *lk)
{
- rollback_lock_file(lk);
+ rollback_lock_file(&lk->lock);
reset_repository_shallow(r);
}
@@ -366,22 +366,22 @@ const char *setup_temporary_shallow(const struct oid_array *extra)
return "";
}
-void setup_alternate_shallow(struct lock_file *shallow_lock,
+void setup_alternate_shallow(struct shallow_lock *shallow_lock,
const char **alternate_shallow_file,
const struct oid_array *extra)
{
struct strbuf sb = STRBUF_INIT;
int fd;
- fd = hold_lock_file_for_update(shallow_lock,
+ fd = hold_lock_file_for_update(&shallow_lock->lock,
git_path_shallow(the_repository),
LOCK_DIE_ON_ERROR);
check_shallow_file_for_update(the_repository);
if (write_shallow_commits(&sb, 0, extra)) {
if (write_in_full(fd, sb.buf, sb.len) < 0)
die_errno("failed to write to %s",
- get_lock_file_path(shallow_lock));
- *alternate_shallow_file = get_lock_file_path(shallow_lock);
+ get_lock_file_path(&shallow_lock->lock));
+ *alternate_shallow_file = get_lock_file_path(&shallow_lock->lock);
} else
/*
* is_repository_shallow() sees empty string as "no
@@ -414,7 +414,7 @@ void advertise_shallow_grafts(int fd)
*/
void prune_shallow(unsigned options)
{
- struct lock_file shallow_lock = LOCK_INIT;
+ struct shallow_lock shallow_lock = SHALLOW_LOCK_INIT;
struct strbuf sb = STRBUF_INIT;
unsigned flags = SEEN_ONLY;
int fd;
@@ -428,14 +428,14 @@ void prune_shallow(unsigned options)
strbuf_release(&sb);
return;
}
- fd = hold_lock_file_for_update(&shallow_lock,
+ fd = hold_lock_file_for_update(&shallow_lock.lock,
git_path_shallow(the_repository),
LOCK_DIE_ON_ERROR);
check_shallow_file_for_update(the_repository);
if (write_shallow_commits_1(&sb, 0, NULL, flags)) {
if (write_in_full(fd, sb.buf, sb.len) < 0)
die_errno("failed to write to %s",
- get_lock_file_path(&shallow_lock));
+ get_lock_file_path(&shallow_lock.lock));
commit_shallow_file(the_repository, &shallow_lock);
} else {
unlink(git_path_shallow(the_repository));
diff --git a/shallow.h b/shallow.h
index b3bf200bf9..5b4a96dcd6 100644
--- a/shallow.h
+++ b/shallow.h
@@ -10,10 +10,25 @@ void set_alternate_shallow_file(struct repository *r, const char *path, int over
int register_shallow(struct repository *r, const struct object_id *oid);
int unregister_shallow(const struct object_id *oid);
int is_repository_shallow(struct repository *r);
+
+/*
+ * Lock for updating the $GIT_DIR/shallow file.
+ *
+ * Use `commit_shallow_file()` to commit an update, or
+ * `rollback_shallow_file()` to roll it back. In either case, any
+ * in-memory cached information about which commits are shallow will be
+ * appropriately invalidated so that future operations reflect the new
+ * state.
+ */
+struct shallow_lock {
+ struct lock_file lock;
+};
+#define SHALLOW_LOCK_INIT { LOCK_INIT }
+
/* commit $GIT_DIR/shallow and reset stat-validity checks */
-int commit_shallow_file(struct repository *r, struct lock_file *lk);
+int commit_shallow_file(struct repository *r, struct shallow_lock *lk);
/* rollback $GIT_DIR/shallow and reset stat-validity checks */
-void rollback_shallow_file(struct repository *r, struct lock_file *lk);
+void rollback_shallow_file(struct repository *r, struct shallow_lock *lk);
struct commit_list *get_shallow_commits(struct object_array *heads,
int depth, int shallow_flag, int not_shallow_flag);
@@ -22,7 +37,7 @@ struct commit_list *get_shallow_commits_by_rev_list(
int write_shallow_commits(struct strbuf *out, int use_pack_protocol,
const struct oid_array *extra);
-void setup_alternate_shallow(struct lock_file *shallow_lock,
+void setup_alternate_shallow(struct shallow_lock *shallow_lock,
const char **alternate_shallow_file,
const struct oid_array *extra);
--
2.26.0.113.ge9739cdccc
^ permalink raw reply related [flat|nested] 8+ messages in thread* Re: [PATCH v2 0/4] shallow: extract a header file
2020-04-30 19:48 [PATCH v2 0/4] shallow: extract a header file Taylor Blau
` (3 preceding siblings ...)
2020-04-30 19:48 ` [PATCH v2 4/4] shallow: use struct 'shallow_lock' for additional safety Taylor Blau
@ 2020-04-30 19:49 ` Taylor Blau
4 siblings, 0 replies; 8+ messages in thread
From: Taylor Blau @ 2020-04-30 19:49 UTC (permalink / raw)
To: Taylor Blau; +Cc: git, gitster, jonathantanmy, jrnieder, sunshine
On Thu, Apr 30, 2020 at 01:48:43PM -0600, Taylor Blau wrote:
> Hi,
>
> Here's a reroll of my series to introduce 'shallow.h' after some very
> helpful review from yesterday evening and early this morning.
Oops. Forgot to set --in-reply-to to the v1 of this series, which can be
found in [1]. Since I already sent the cover letter and patches, let's
just pick it up from here.
Thanks,
Taylor
[1]: https://lore.kernel.org/git/cover.1588199705.git.me@ttaylorr.com/
^ permalink raw reply [flat|nested] 8+ messages in thread