{"thread":{"id":"47365","subject":"[WIP 0/2] Submodule ref backend that mirrors superproject","startedAt":"2017-12-01T22:50:23Z","lastAt":"2017-12-05T22:28:27Z","messageCount":5,"participants":["Jonathan Tan","Stefan Beller"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"333942","messageId":"cover.1512168087.git.jonathantanmy@google.com","threadId":"47365","inReplyTo":null,"subject":"[WIP 0/2] Submodule ref backend that mirrors superproject","fromName":"Jonathan Tan","fromEmail":"jonathantanmy@google.com","sentAt":"2017-12-01T22:50:05Z","receivedAt":"2017-12-01T22:50:23Z","isPatch":false,"sender":{"key":"jonathantanmy@fastmail.com","avatar":null},"body":"I sent out an earlier email [1] that discusses the idea of a submodule\nref backend that mirrors the superproject. Basically, if this backend is\nused (for example, through a configuration option), the submodule itself\nwill not store any \"refs/...\" refs, but will check the gitlink of the\ncommit of the ref of the same name in the superproject. For example, if\nthe superproject has at refs/heads/foo:\n\n    superproject/\n      sub [gitlink -> 1234...]\n\nand at refs/heads/bar:\n\n    superproject/\n      sub [gitlink -> 5678...]\n\nInside sub, \"git rev-parse foo\" will output \"1234...\", and \"git rev-parse\nbar\" will output \"5678...\", even though \"foo\" and \"bar\" are not defined\nanywhere inside sub.\n\n(The submodule handles refs that do not start with \"refs/\" - for\nexample, HEAD and FETCH_HEAD - like usual.)\n\n[1] also describes what happens when the submodule attempts to write to\nany \"refs/...\" ref.\n\nFor those interested, here's what such an implementation might look\nlike, and a test to demonstrate such functionality. I have partial\nread-only functionality - a lot of it still remains to be done.\n\n[1] https://public-inbox.org/git/20171108172945.33c42a0e91b4ac494217b788@google.com/\n\nJonathan Tan (2):\n  submodule: refactor acquisition of superproject info\n  submodule: read-only super-backed ref backend\n\n Makefile                       |   1 +\n refs.c                         |  11 +-\n refs/refs-internal.h           |   1 +\n refs/sp-backend.c              | 261 +++++++++++++++++++++++++++++++++++++++++\n submodule.c                    | 107 +++++++++++------\n submodule.h                    |   5 +\n t/t1406-submodule-ref-store.sh |  26 ++++\n 7 files changed, 374 insertions(+), 38 deletions(-)\n create mode 100644 refs/sp-backend.c\n\n-- \n2.15.0.531.g2ccb3012c9-goog\n\n"},{"id":"333943","messageId":"14e821d45e4ac2faea1f9023d43c43c4675672e7.1512168087.git.jonathantanmy@google.com","threadId":"47365","inReplyTo":"cover.1512168087.git.jonathantanmy@google.com","subject":"[WIP 1/2] submodule: refactor acquisition of superproject info","fromName":"Jonathan Tan","fromEmail":"jonathantanmy@google.com","sentAt":"2017-12-01T22:50:06Z","receivedAt":"2017-12-01T22:50:27Z","isPatch":false,"sender":{"key":"jonathantanmy@fastmail.com","avatar":null},"body":"Signed-off-by: Jonathan Tan <jonathantanmy@google.com>\n---\n submodule.c | 76 +++++++++++++++++++++++++++++++++++++------------------------\n submodule.h |  3 +++\n 2 files changed, 49 insertions(+), 30 deletions(-)\n\ndiff --git a/submodule.c b/submodule.c\nindex bb531e0e5..ce511180e 100644\n--- a/submodule.c\n+++ b/submodule.c\n@@ -1977,16 +1977,24 @@ void absorb_git_dir_into_superproject(const char *prefix,\n \t}\n }\n \n-const char *get_superproject_working_tree(void)\n+/*\n+ * Get the full filename of the gitlink entry corresponding to the\n+ * superproject having this repo as a submodule.\n+ *\n+ * full_name will point to an internal buffer upon success.\n+ *\n+ * Return 0 if successful, 1 if not (for example, if the parent\n+ * directory is not a repo or an unrelated one).\n+ */\n+int get_superproject_entry(const char **full_name)\n {\n+\tstatic struct strbuf sb = STRBUF_INIT;\n+\n \tstruct child_process cp = CHILD_PROCESS_INIT;\n-\tstruct strbuf sb = STRBUF_INIT;\n \tconst char *one_up = real_path_if_valid(\"../\");\n \tconst char *cwd = xgetcwd();\n-\tconst char *ret = NULL;\n \tconst char *subpath;\n \tint code;\n-\tssize_t len;\n \n \tif (!is_inside_work_tree())\n \t\t/*\n@@ -1994,11 +2002,15 @@ const char *get_superproject_working_tree(void)\n \t\t * We might have a superproject, but it is harder\n \t\t * to determine.\n \t\t */\n-\t\treturn NULL;\n+\t\treturn 1;\n \n \tif (!one_up)\n-\t\treturn NULL;\n+\t\treturn 1;\n \n+\t/*\n+\t * No need to reset sb, since relative_path() will do it if\n+\t * necessary.\n+\t */\n \tsubpath = relative_path(cwd, one_up, &sb);\n \n \tprepare_submodule_repo_env(&cp.env_array);\n@@ -2017,45 +2029,49 @@ const char *get_superproject_working_tree(void)\n \tif (start_command(&cp))\n \t\tdie(_(\"could not start ls-files in ..\"));\n \n-\tlen = strbuf_read(&sb, cp.out, PATH_MAX);\n+\tstrbuf_read(&sb, cp.out, PATH_MAX);\n \tclose(cp.out);\n \n-\tif (starts_with(sb.buf, \"160000\")) {\n-\t\tint super_sub_len;\n-\t\tint cwd_len = strlen(cwd);\n-\t\tchar *super_sub, *super_wt;\n+\tcode = finish_command(&cp);\n \n+\tif (starts_with(sb.buf, \"160000\")) {\n \t\t/*\n \t\t * There is a superproject having this repo as a submodule.\n \t\t * The format is <mode> SP <hash> SP <stage> TAB <full name> \\0,\n \t\t * We're only interested in the name after the tab.\n \t\t */\n-\t\tsuper_sub = strchr(sb.buf, '\\t') + 1;\n-\t\tsuper_sub_len = sb.buf + sb.len - super_sub - 1;\n-\n-\t\tif (super_sub_len > cwd_len ||\n-\t\t    strcmp(&cwd[cwd_len - super_sub_len], super_sub))\n-\t\t\tdie (_(\"BUG: returned path string doesn't match cwd?\"));\n-\n-\t\tsuper_wt = xstrdup(cwd);\n-\t\tsuper_wt[cwd_len - super_sub_len] = '\\0';\n-\n-\t\tret = real_path(super_wt);\n-\t\tfree(super_wt);\n+\t\tchar *tab = strchr(sb.buf, '\\t');\n+\t\tif (!tab)\n+\t\t\tdie(\"BUG: ls-files returned line with no tab\");\n+\t\t*full_name = tab + 1;\n+\t\treturn 0;\n \t}\n-\tstrbuf_release(&sb);\n-\n-\tcode = finish_command(&cp);\n \n \tif (code == 128)\n \t\t/* '../' is not a git repository */\n-\t\treturn NULL;\n-\tif (code == 0 && len == 0)\n+\t\treturn 1;\n+\tif (code == 0)\n \t\t/* There is an unrelated git repository at '../' */\n+\t\treturn 1;\n+\tdie(_(\"ls-tree returned unexpected return code %d\"), code);\n+}\n+\n+const char *get_superproject_working_tree(void)\n+{\n+\tconst char *full_name;\n+\tchar *super_wt;\n+\tsize_t len;\n+\tconst char *ret;\n+\n+\tif (get_superproject_entry(&full_name))\n \t\treturn NULL;\n-\tif (code)\n-\t\tdie(_(\"ls-tree returned unexpected return code %d\"), code);\n \n+\tsuper_wt = xstrdup(xgetcwd());\n+\tif (!strip_suffix(super_wt, full_name, &len))\n+\t\tdie(\"BUG: returned path string doesn't match cwd?\");\n+\tsuper_wt[len] = '\\0';\n+\tret = real_path(super_wt);\n+\tfree(super_wt);\n \treturn ret;\n }\n \ndiff --git a/submodule.h b/submodule.h\nindex f0da0277a..29ab302cc 100644\n--- a/submodule.h\n+++ b/submodule.h\n@@ -134,6 +134,9 @@ extern void absorb_git_dir_into_superproject(const char *prefix,\n  * Return the absolute path of the working tree of the superproject, which this\n  * project is a submodule of. If this repository is not a submodule of\n  * another repository, return NULL.\n+ *\n+ * The return value, if not NULL, points to the same shared buffer as the one\n+ * returned by real_path().\n  */\n extern const char *get_superproject_working_tree(void);\n \n-- \n2.15.0.531.g2ccb3012c9-goog\n\n"},{"id":"333944","messageId":"bba84ff0be9c2cbd87bff77a481a74698d90062f.1512168087.git.jonathantanmy@google.com","threadId":"47365","inReplyTo":"cover.1512168087.git.jonathantanmy@google.com","subject":"[WIP 2/2] submodule: read-only super-backed ref backend","fromName":"Jonathan Tan","fromEmail":"jonathantanmy@google.com","sentAt":"2017-12-01T22:50:07Z","receivedAt":"2017-12-01T22:50:31Z","isPatch":false,"sender":{"key":"jonathantanmy@fastmail.com","avatar":null},"body":"Note that a few major parts are still missing:\n - special handling of the current branch of the superproject\n - writing (whether \"refs/...\" to the superproject as an index change or\n   a commit, or non-\"refs/...\" directly to the subproject like usual)\n\nSigned-off-by: Jonathan Tan <jonathantanmy@google.com>\n---\n Makefile                       |   1 +\n refs.c                         |  11 +-\n refs/refs-internal.h           |   1 +\n refs/sp-backend.c              | 261 +++++++++++++++++++++++++++++++++++++++++\n submodule.c                    |  43 +++++--\n submodule.h                    |   2 +\n t/t1406-submodule-ref-store.sh |  26 ++++\n 7 files changed, 331 insertions(+), 14 deletions(-)\n create mode 100644 refs/sp-backend.c\n\ndiff --git a/Makefile b/Makefile\nindex e53750ca0..74120b5d7 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -858,6 +858,7 @@ LIB_OBJS += refs/files-backend.o\n LIB_OBJS += refs/iterator.o\n LIB_OBJS += refs/packed-backend.o\n LIB_OBJS += refs/ref-cache.o\n+LIB_OBJS += refs/sp-backend.o\n LIB_OBJS += ref-filter.o\n LIB_OBJS += remote.o\n LIB_OBJS += replace_object.o\ndiff --git a/refs.c b/refs.c\nindex 339d4318e..1f7922733 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -1575,12 +1575,17 @@ static struct ref_store *lookup_ref_store_map(struct hashmap *map,\n static struct ref_store *ref_store_init(const char *gitdir,\n \t\t\t\t\tunsigned int flags)\n {\n-\tconst char *be_name = \"files\";\n-\tstruct ref_storage_be *be = find_ref_storage_backend(be_name);\n+\tstruct ref_storage_be *be;\n \tstruct ref_store *refs;\n \n+\tif (getenv(\"USE_SP\")) {\n+\t\tbe = &refs_be_sp;\n+\t} else {\n+\t\tbe = &refs_be_files;\n+\t}\n+\n \tif (!be)\n-\t\tdie(\"BUG: reference backend %s is unknown\", be_name);\n+\t\tdie(\"BUG: reference backend %s is unknown\", \"files\");\n \n \trefs = be->init(gitdir, flags);\n \treturn refs;\ndiff --git a/refs/refs-internal.h b/refs/refs-internal.h\nindex dd834314b..a8ec03d90 100644\n--- a/refs/refs-internal.h\n+++ b/refs/refs-internal.h\n@@ -651,6 +651,7 @@ struct ref_storage_be {\n \n extern struct ref_storage_be refs_be_files;\n extern struct ref_storage_be refs_be_packed;\n+extern struct ref_storage_be refs_be_sp;\n \n /*\n  * A representation of the reference store for the main repository or\ndiff --git a/refs/sp-backend.c b/refs/sp-backend.c\nnew file mode 100644\nindex 000000000..31e8cec4b\n--- /dev/null\n+++ b/refs/sp-backend.c\n@@ -0,0 +1,261 @@\n+#include \"../cache.h\"\n+#include \"../config.h\"\n+#include \"../refs.h\"\n+#include \"refs-internal.h\"\n+#include \"ref-cache.h\"\n+#include \"packed-backend.h\"\n+#include \"../iterator.h\"\n+#include \"../dir-iterator.h\"\n+#include \"../lockfile.h\"\n+#include \"../object.h\"\n+#include \"../dir.h\"\n+#include \"../submodule.h\"\n+\n+/*\n+ * Future: need to be in \"struct repository\"\n+ * when doing a full libification.\n+ */\n+struct sp_ref_store {\n+\tstruct ref_store base;\n+\tunsigned int store_flags;\n+\n+\t/*\n+\t * Ref store of this repository (the submodule), used only for the\n+\t * reflog.\n+\t */\n+\tstruct ref_store *files;\n+\n+\t/*\n+\t * Ref store of the superproject, for refs.\n+\t */\n+\tstruct ref_store *files_superproject;\n+};\n+\n+/*\n+ * Create a new submodule ref cache and add it to the internal\n+ * set of caches.\n+ */\n+static struct ref_store *sp_init(const char *gitdir, unsigned int flags)\n+{\n+\tstruct sp_ref_store *refs = xcalloc(1, sizeof(*refs));\n+\tstruct ref_store *ref_store = (struct ref_store *)refs;\n+\n+\tbase_ref_store_init(ref_store, &refs_be_sp);\n+\trefs->store_flags = flags;\n+\trefs->files = refs_be_files.init(gitdir, flags);\n+\n+\treturn ref_store;\n+}\n+\n+/*\n+ * Downcast ref_store to sp_ref_store. Die if ref_store is not a\n+ * sp_ref_store. required_flags is compared with ref_store's\n+ * store_flags to ensure the ref_store has all required capabilities.\n+ * \"caller\" is used in any necessary error messages.\n+ */\n+static struct sp_ref_store *sp_downcast(struct ref_store *ref_store,\n+\t\t\t\t\t      unsigned int required_flags,\n+\t\t\t\t\t      const char *caller)\n+{\n+\tstruct sp_ref_store *refs;\n+\n+\tif (ref_store->be != &refs_be_sp)\n+\t\tdie(\"BUG: ref_store is type \\\"%s\\\" not \\\"sp\\\" in %s\",\n+\t\t    ref_store->be->name, caller);\n+\n+\trefs = (struct sp_ref_store *)ref_store;\n+\n+\tif ((refs->store_flags & required_flags) != required_flags)\n+\t\tdie(\"BUG: operation %s requires abilities 0x%x, but only have 0x%x\",\n+\t\t    caller, required_flags, refs->store_flags);\n+\n+\treturn refs;\n+}\n+\n+static int sp_read_raw_ref(struct ref_store *ref_store,\n+\t\t\t      const char *refname, struct object_id *oid,\n+\t\t\t      struct strbuf *referent, unsigned int *type)\n+{\n+\tstruct sp_ref_store *refs;\n+\n+\trefs = sp_downcast(ref_store, REF_STORE_READ, \"read_raw_ref\");\n+\n+\tif (!starts_with(refname, \"refs/\")) {\n+\t\treturn refs_read_raw_ref(refs->files, refname, oid, referent, type);\n+\t}\n+\n+\t/* read from the superproject instead */\n+\treturn get_superproject_gitlink_oid(refname, oid);\n+}\n+\n+static struct ref_iterator *sp_ref_iterator_begin(\n+\t\tstruct ref_store *ref_store,\n+\t\tconst char *prefix, unsigned int flags)\n+{\n+\treturn empty_ref_iterator_begin();\n+}\n+\n+static int sp_pack_refs(struct ref_store *ref_store, unsigned int flags)\n+{\n+\t/* no op */\n+\treturn 0;\n+}\n+\n+static int sp_delete_refs(struct ref_store *ref_store, const char *msg,\n+\t\t\t     struct string_list *refnames, unsigned int flags)\n+{\n+\t/* unsupported */\n+\treturn -1;\n+}\n+\n+static int sp_rename_ref(struct ref_store *ref_store,\n+\t\t\t    const char *oldrefname, const char *newrefname,\n+\t\t\t    const char *logmsg)\n+{\n+\t/* unsupported */\n+\treturn -1;\n+}\n+\n+static int sp_copy_ref(struct ref_store *ref_store,\n+\t\t\t    const char *oldrefname, const char *newrefname,\n+\t\t\t    const char *logmsg)\n+{\n+\t/* unsupported */\n+\treturn -1;\n+}\n+\n+static int sp_create_reflog(struct ref_store *ref_store,\n+\t\t\t       const char *refname, int force_create,\n+\t\t\t       struct strbuf *err)\n+{\n+\tstruct sp_ref_store *refs =\n+\t\tsp_downcast(ref_store, REF_STORE_WRITE, \"create_reflog\");\n+\treturn refs_create_reflog(refs->files, refname, force_create, err);\n+}\n+\n+static int sp_create_symref(struct ref_store *ref_store,\n+\t\t\t       const char *refname, const char *target,\n+\t\t\t       const char *logmsg)\n+{\n+\t/* unsupported */\n+\treturn -1;\n+}\n+\n+static int sp_reflog_exists(struct ref_store *ref_store,\n+\t\t\t       const char *refname)\n+{\n+\tstruct sp_ref_store *refs =\n+\t\tsp_downcast(ref_store, REF_STORE_READ, \"reflog_exists\");\n+\treturn refs_reflog_exists(refs->files, refname);\n+}\n+\n+static int sp_delete_reflog(struct ref_store *ref_store,\n+\t\t\t       const char *refname)\n+{\n+\tstruct sp_ref_store *refs =\n+\t\tsp_downcast(ref_store, REF_STORE_WRITE, \"delete_reflog\");\n+\treturn refs_delete_reflog(refs->files, refname);\n+}\n+\n+static int sp_for_each_reflog_ent_reverse(struct ref_store *ref_store,\n+\t\t\t\t\t     const char *refname,\n+\t\t\t\t\t     each_reflog_ent_fn fn,\n+\t\t\t\t\t     void *cb_data)\n+{\n+\tstruct sp_ref_store *refs =\n+\t\tsp_downcast(ref_store, REF_STORE_READ,\n+\t\t\t       \"for_each_reflog_ent_reverse\");\n+\treturn refs_for_each_reflog_ent_reverse(refs->files, refname, fn, cb_data);\n+}\n+\n+static int sp_for_each_reflog_ent(struct ref_store *ref_store,\n+\t\t\t\t     const char *refname,\n+\t\t\t\t     each_reflog_ent_fn fn, void *cb_data)\n+{\n+\tstruct sp_ref_store *refs =\n+\t\tsp_downcast(ref_store, REF_STORE_READ,\n+\t\t\t       \"for_each_reflog_ent\");\n+\treturn refs_for_each_reflog_ent(refs->files, refname, fn, cb_data);\n+}\n+\n+static struct ref_iterator *sp_reflog_iterator_begin(struct ref_store *ref_store)\n+{\n+\tstruct sp_ref_store *refs =\n+\t\tsp_downcast(ref_store, REF_STORE_READ,\n+\t\t\t       \"reflog_iterator_begin\");\n+\treturn refs->files->be->reflog_iterator_begin(refs->files);\n+}\n+\n+static int sp_transaction_prepare(struct ref_store *ref_store,\n+\t\t\t\t     struct ref_transaction *transaction,\n+\t\t\t\t     struct strbuf *err)\n+{\n+\treturn -1;\n+}\n+\n+static int sp_transaction_finish(struct ref_store *ref_store,\n+\t\t\t\t    struct ref_transaction *transaction,\n+\t\t\t\t    struct strbuf *err)\n+{\n+\treturn -1;\n+}\n+\n+static int sp_transaction_abort(struct ref_store *ref_store,\n+\t\t\t\t   struct ref_transaction *transaction,\n+\t\t\t\t   struct strbuf *err)\n+{\n+\treturn -1;\n+}\n+\n+static int sp_initial_transaction_commit(struct ref_store *ref_store,\n+\t\t\t\t\t    struct ref_transaction *transaction,\n+\t\t\t\t\t    struct strbuf *err)\n+{\n+\treturn -1;\n+}\n+\n+static int sp_reflog_expire(struct ref_store *ref_store,\n+\t\t\t       const char *refname, const struct object_id *oid,\n+\t\t\t       unsigned int flags,\n+\t\t\t       reflog_expiry_prepare_fn prepare_fn,\n+\t\t\t       reflog_expiry_should_prune_fn should_prune_fn,\n+\t\t\t       reflog_expiry_cleanup_fn cleanup_fn,\n+\t\t\t       void *policy_cb_data)\n+{\n+\tstruct sp_ref_store *refs =\n+\t\tsp_downcast(ref_store, REF_STORE_WRITE, \"reflog_expire\");\n+\treturn refs_reflog_expire(refs->files, refname, oid, flags, prepare_fn, should_prune_fn, cleanup_fn, policy_cb_data);\n+}\n+\n+static int sp_init_db(struct ref_store *ref_store, struct strbuf *err)\n+{\n+\treturn 0;\n+}\n+\n+struct ref_storage_be refs_be_sp = {\n+\tNULL,\n+\t\"sp\",\n+\tsp_init,\n+\tsp_init_db,\n+\tsp_transaction_prepare,\n+\tsp_transaction_finish,\n+\tsp_transaction_abort,\n+\tsp_initial_transaction_commit,\n+\n+\tsp_pack_refs,\n+\tsp_create_symref,\n+\tsp_delete_refs,\n+\tsp_rename_ref,\n+\tsp_copy_ref,\n+\n+\tsp_ref_iterator_begin,\n+\tsp_read_raw_ref,\n+\n+\tsp_reflog_iterator_begin,\n+\tsp_for_each_reflog_ent,\n+\tsp_for_each_reflog_ent_reverse,\n+\tsp_reflog_exists,\n+\tsp_create_reflog,\n+\tsp_delete_reflog,\n+\tsp_reflog_expire\n+};\ndiff --git a/submodule.c b/submodule.c\nindex ce511180e..1ffaeec82 100644\n--- a/submodule.c\n+++ b/submodule.c\n@@ -471,6 +471,7 @@ static void prepare_submodule_repo_env_no_git_dir(struct argv_array *out)\n void prepare_submodule_repo_env(struct argv_array *out)\n {\n \tprepare_submodule_repo_env_no_git_dir(out);\n+\targv_array_pushf(out, \"USE_SP\");\n \targv_array_pushf(out, \"%s=%s\", GIT_DIR_ENVIRONMENT,\n \t\t\t DEFAULT_GIT_DIR_ENVIRONMENT);\n }\n@@ -1986,7 +1987,7 @@ void absorb_git_dir_into_superproject(const char *prefix,\n  * Return 0 if successful, 1 if not (for example, if the parent\n  * directory is not a repo or an unrelated one).\n  */\n-int get_superproject_entry(const char **full_name)\n+static int get_superproject_entry(const char *ref, const char **full_name, struct object_id *oid)\n {\n \tstatic struct strbuf sb = STRBUF_INIT;\n \n@@ -2016,9 +2017,11 @@ int get_superproject_entry(const char **full_name)\n \tprepare_submodule_repo_env(&cp.env_array);\n \targv_array_pop(&cp.env_array);\n \n-\targv_array_pushl(&cp.args, \"--literal-pathspecs\", \"-C\", \"..\",\n-\t\t\t\"ls-files\", \"-z\", \"--stage\", \"--full-name\", \"--\",\n-\t\t\tsubpath, NULL);\n+\targv_array_pushl(&cp.args, \"--literal-pathspecs\", \"-C\", \"..\", NULL);\n+\tif (ref)\n+\t\targv_array_pushl(&cp.args, \"ls-tree\", \"-z\", \"--full-name\", \"-r\", ref, subpath, NULL);\n+\telse\n+\t\targv_array_pushl(&cp.args, \"ls-files\", \"-z\", \"--full-name\", \"--stage\", \"--\", subpath, NULL);\n \tstrbuf_reset(&sb);\n \n \tcp.no_stdin = 1;\n@@ -2037,13 +2040,24 @@ int get_superproject_entry(const char **full_name)\n \tif (starts_with(sb.buf, \"160000\")) {\n \t\t/*\n \t\t * There is a superproject having this repo as a submodule.\n-\t\t * The format is <mode> SP <hash> SP <stage> TAB <full name> \\0,\n-\t\t * We're only interested in the name after the tab.\n+\t\t * The format is:\n+\t\t * [ls-tree] <mode> SP <type> SP <hash> TAB <full name> \\0\n+\t\t * [ls-files] <mode> SP <hash> SP <stage> TAB <full name> \\0\n \t\t */\n-\t\tchar *tab = strchr(sb.buf, '\\t');\n-\t\tif (!tab)\n-\t\t\tdie(\"BUG: ls-files returned line with no tab\");\n-\t\t*full_name = tab + 1;\n+\t\tif (full_name) {\n+\t\t\tchar *tab = strchr(sb.buf, '\\t');\n+\t\t\tif (!tab)\n+\t\t\t\tdie(\"BUG: ls-files returned line with no tab\");\n+\t\t\t*full_name = tab + 1;\n+\t\t}\n+\t\tif (oid) {\n+\t\t\tchar *space = strchr(sb.buf, ' ');\n+\t\t\tconst char *p;\n+\t\t\tif (ref)\n+\t\t\t\tspace = strchr(space + 1, ' ');\n+\t\t\tif (parse_oid_hex(space + 1, oid, &p))\n+\t\t\t\tdie(\"BUG: Could not read OID: %s\", space + 1);\n+\t\t}\n \t\treturn 0;\n \t}\n \n@@ -2063,7 +2077,7 @@ const char *get_superproject_working_tree(void)\n \tsize_t len;\n \tconst char *ret;\n \n-\tif (get_superproject_entry(&full_name))\n+\tif (get_superproject_entry(NULL, &full_name, NULL))\n \t\treturn NULL;\n \n \tsuper_wt = xstrdup(xgetcwd());\n@@ -2075,6 +2089,13 @@ const char *get_superproject_working_tree(void)\n \treturn ret;\n }\n \n+int get_superproject_gitlink_oid(const char *ref, struct object_id *oid)\n+{\n+\tif (get_superproject_entry(ref, NULL, oid))\n+\t\treturn 1;\n+\treturn 0;\n+}\n+\n /*\n  * Put the gitdir for a submodule (given relative to the main\n  * repository worktree) into `buf`, or return -1 on error.\ndiff --git a/submodule.h b/submodule.h\nindex 29ab302cc..3a836beab 100644\n--- a/submodule.h\n+++ b/submodule.h\n@@ -140,4 +140,6 @@ extern void absorb_git_dir_into_superproject(const char *prefix,\n  */\n extern const char *get_superproject_working_tree(void);\n \n+extern int get_superproject_gitlink_oid(const char *ref, struct object_id *oid);\n+\n #endif\ndiff --git a/t/t1406-submodule-ref-store.sh b/t/t1406-submodule-ref-store.sh\nindex c32d4cc46..46ba7a272 100755\n--- a/t/t1406-submodule-ref-store.sh\n+++ b/t/t1406-submodule-ref-store.sh\n@@ -98,4 +98,30 @@ test_expect_success 'create-reflog() not allowed' '\n \ttest_must_fail $RUN create-reflog HEAD 1\n '\n \n+test_expect_success 'ref backend based on superproject data' '\n+\trm -rf sub super &&\n+\n+\tgit init sub &&\n+\ttest_commit -C sub first &&\n+\ttest_commit -C sub second &&\n+\n+\tgit init super &&\n+\n+\t# master branch in superproject, submodule at second\n+\tgit -C super submodule add ../sub sub &&\n+\tgit -C super commit -m x &&\n+\n+\t# anotherbranch in superproject, submodule at first\n+\tgit -C super checkout -b anotherbranch &&\n+\tgit -C super/sub checkout first &&\n+\tgit -C super commit -a -m x &&\n+\n+\t# Notice that rev-parse can parse \"anotherbranch\" and see that it\n+\t# points to first, even though a branch with the name \"anotherbranch\"\n+\t# is only defined in the superproject\n+\tgit -C sub rev-parse first >expect &&\n+\tUSE_SP=1 git -C super/sub rev-parse anotherbranch >actual &&\n+\ttest_cmp expect actual\n+'\n+\n test_done\n-- \n2.15.0.531.g2ccb3012c9-goog\n\n"},{"id":"334215","messageId":"CAGZ79kZgzWRSmXv0JOryhYKU7gnZw2DvEK7e7MXjv7MWCF6T7Q@mail.gmail.com","threadId":"47365","inReplyTo":"14e821d45e4ac2faea1f9023d43c43c4675672e7.1512168087.git.jonathantanmy@google.com","subject":"Re: [WIP 1/2] submodule: refactor acquisition of superproject info","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2017-12-05T19:57:18Z","receivedAt":"2017-12-05T19:57:52Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Fri, Dec 1, 2017 at 2:50 PM, Jonathan Tan <jonathantanmy@google.com> wrote:\n> Signed-off-by: Jonathan Tan <jonathantanmy@google.com>\n> ---\n>  submodule.c | 76 +++++++++++++++++++++++++++++++++++++------------------------\n>  submodule.h |  3 +++\n\n\nThis patch reads very similar to [1], which was a preparation part of the series\n\"[RFC PATCH 0/4] git-status reports relation to superproject\"; there\nwe also need\nthe same information about the superproject. I'll take a closer look\nand compare these\ntwo patches if it turns out we want to go this way long term.\n\n[1] https://public-inbox.org/git/20171108195509.7839-3-sbeller@google.com/\n"},{"id":"334228","messageId":"CAGZ79kZv-P55acwkXZUfDXPAmwng2yD17awNkZ85RbaJ2O0ppw@mail.gmail.com","threadId":"47365","inReplyTo":"bba84ff0be9c2cbd87bff77a481a74698d90062f.1512168087.git.jonathantanmy@google.com","subject":"Re: [WIP 2/2] submodule: read-only super-backed ref backend","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2017-12-05T22:28:13Z","receivedAt":"2017-12-05T22:28:27Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Fri, Dec 1, 2017 at 2:50 PM, Jonathan Tan <jonathantanmy@google.com> wrote:\n> Note that a few major parts are still missing:\n>  - special handling of the current branch of the superproject\n>  - writing (whether \"refs/...\" to the superproject as an index change or\n>    a commit, or non-\"refs/...\" directly to the subproject like usual)\n>\n> Signed-off-by: Jonathan Tan <jonathantanmy@google.com>\n\n\n> --- a/refs.c\n> +++ b/refs.c\n> @@ -1575,12 +1575,17 @@ static struct ref_store *lookup_ref_store_map(struct hashmap *map,\n>  static struct ref_store *ref_store_init(const char *gitdir,\n>                                         unsigned int flags)\n>  {\n> -       const char *be_name = \"files\";\n> -       struct ref_storage_be *be = find_ref_storage_backend(be_name);\n> +       struct ref_storage_be *be;\n>         struct ref_store *refs;\n>\n> +       if (getenv(\"USE_SP\")) {\n> +               be = &refs_be_sp;\n> +       } else {\n> +               be = &refs_be_files;\n> +       }\n> +\n>         if (!be)\n> -               die(\"BUG: reference backend %s is unknown\", be_name);\n> +               die(\"BUG: reference backend %s is unknown\", \"files\");\n\nIf we pursue this approach more than just RFC-ish we would probably not use\nan environment variable but have some detection logic for the\ndifferent ref backends.\n\nFor example Shawns refstable[1] proposal uses the configuration\n`extensions.refStorage == reftable`, which this proposal could reuse and\nset to 'indexed_superproject' or 'commit_in_superproject', depending on the\nwrite semantics that we'd need to disucss (or do we want to offer 2 different\nsuperproject backends having these different semantics?)\n\nMore to the code here, we could still use `find_ref_storage_backend()`\ndepending on the env:\n\n    const char *be_name = \"files\";\n    ...\n+    if (getenv(\"USE_SP\"))\n+        be_name = \"sp-backend\";\n    ...\n\nand then we'd have to change the 'next ' pointer in refs_be_files\n(in refs/files-backend.c line ~3100) to point at the superproject backend\ninstead of NULL.\n\n[1] https://googlers.googlesource.com/sop/jgit/+/reftable/Documentation/technical/reftable.md#Repository-format\n\n> +static int sp_read_raw_ref(struct ref_store *ref_store,\n> +                             const char *refname, struct object_id *oid,\n> +                             struct strbuf *referent, unsigned int *type)\n> +{\n> +       struct sp_ref_store *refs;\n> +\n> +       refs = sp_downcast(ref_store, REF_STORE_READ, \"read_raw_ref\");\n> +\n> +       if (!starts_with(refname, \"refs/\")) {\n> +               return refs_read_raw_ref(refs->files, refname, oid, referent, type);\n> +       }\n> +\n> +       /* read from the superproject instead */\n> +       return get_superproject_gitlink_oid(refname, oid);\n> +}\n\nThis function would need to get more sophisticated logic I would assume;\nthe current RFC is a read-only thing, such that it may be sufficient\nto differentiate\nbetween refs/* and specials such as [FETCH_]HEAD, but as seen in the\nsp_ref_store struct definition we're already keeping a local reflog. I would\nassume that remotes are also local to the submodule instead of reusing the\nsuperprojects remote, for the sake of pulling in upstream changes of the\nsubmodule instead of being fully integrated.\n\n\n\n> diff --git a/submodule.c b/submodule.c\n> index ce511180e..1ffaeec82 100644\n> --- a/submodule.c\n> +++ b/submodule.c\n> @@ -471,6 +471,7 @@ static void prepare_submodule_repo_env_no_git_dir(struct argv_array *out)\n>  void prepare_submodule_repo_env(struct argv_array *out)\n>  {\n>         prepare_submodule_repo_env_no_git_dir(out);\n> +       argv_array_pushf(out, \"USE_SP\");\n>         argv_array_pushf(out, \"%s=%s\", GIT_DIR_ENVIRONMENT,\n>                          DEFAULT_GIT_DIR_ENVIRONMENT);\n>  }\n\nThis is an easy way to enforce all submodules use the superproject refs mode,\nwhich makes it easy to make recursive commands (e.g. `git checkout --recursive`\nwould not need to pass down the exact gitlink sha1, but could pass\ndown the branch\nname or even commit-ish \"HEAD^^\" and the submodule stays in perfect\nsync (according\nto superproject commit-ish). Once we take this further I would imagine\nthere is an option\nin the submodule that specifies what mode it is using. Not sure if\nwe'd want to keep the\nenvironment variable to force that mode from the superproject, though.\n\n> @@ -1986,7 +1987,7 @@ void absorb_git_dir_into_superproject(const char *prefix,\n>   * Return 0 if successful, 1 if not (for example, if the parent\n>   * directory is not a repo or an unrelated one).\n>   */\n> -int get_superproject_entry(const char **full_name)\n> +static int get_superproject_entry(const char *ref, const char **full_name, struct object_id *oid)\n\nMaking it static could be part of the prior patch?\n\nWhen the ref is given we use `ls-tree -r` instead of `ls-files --stage`\nwhich give similar outputs to have the parsing logic overlap below,\nthe difference\nis whether the index is looked up or the given ref.\n\n> @@ -2016,9 +2017,11 @@ int get_superproject_entry(const char **full_name)\n>         prepare_submodule_repo_env(&cp.env_array);\n>         argv_array_pop(&cp.env_array);\n>\n> -       argv_array_pushl(&cp.args, \"--literal-pathspecs\", \"-C\", \"..\",\n> -                       \"ls-files\", \"-z\", \"--stage\", \"--full-name\", \"--\",\n> -                       subpath, NULL);\n> +       argv_array_pushl(&cp.args, \"--literal-pathspecs\", \"-C\", \"..\", NULL);\n> +       if (ref)\n> +               argv_array_pushl(&cp.args, \"ls-tree\", \"-z\", \"--full-name\", \"-r\", ref, subpath, NULL);\n\n-- between ref and path?\n\n> @@ -2063,7 +2077,7 @@ const char *get_superproject_working_tree(void)\n>         size_t len;\n>         const char *ret;\n>\n> -       if (get_superproject_entry(&full_name))\n> +       if (get_superproject_entry(NULL, &full_name, NULL))\n>                 return NULL;\n>\n>         super_wt = xstrdup(xgetcwd());\n> @@ -2075,6 +2089,13 @@ const char *get_superproject_working_tree(void)\n>         return ret;\n>  }\n>\n> +int get_superproject_gitlink_oid(const char *ref, struct object_id *oid)\n> +{\n> +       if (get_superproject_entry(ref, NULL, oid))\n\nI see; the get_superproject_entry function is more powerful, than what\nI proposed\nin the status series. It takes vastly different arguments, for\ninspection, so the caller\nis responsible instead of having dumber callers and two different functions\n\n>\n> +test_expect_success 'ref backend based on superproject data' '\n> +       rm -rf sub super &&\n> +\n> +       git init sub &&\n> +       test_commit -C sub first &&\n> +       test_commit -C sub second &&\n> +\n> +       git init super &&\n> +\n> +       # master branch in superproject, submodule at second\n> +       git -C super submodule add ../sub sub &&\n> +       git -C super commit -m x &&\n> +\n> +       # anotherbranch in superproject, submodule at first\n> +       git -C super checkout -b anotherbranch &&\n> +       git -C super/sub checkout first &&\n> +       git -C super commit -a -m x &&\n> +\n> +       # Notice that rev-parse can parse \"anotherbranch\" and see that it\n> +       # points to first, even though a branch with the name \"anotherbranch\"\n> +       # is only defined in the superproject\n> +       git -C sub rev-parse first >expect &&\n> +       USE_SP=1 git -C super/sub rev-parse anotherbranch >actual &&\n> +       test_cmp expect actual\n> +'\n\nThanks for writing this proof of concept.\nAs Jonathan Nieder mentioned another similar direction of this idea could be\nused to decorate the branches in the submodule to indicate where the\nsuperproject\npoints to.\n\nI think we'd need to hash out the details of the write operations,\nbefore going further here,\nbut this is the best long term solution so far IMHO. (Short term we\nmight carry local\npatches that make submodules more repo like).\n\nThanks,\nStefan\n"}]}