{"thread":{"id":"46599","subject":"[PATCH] convert add_to_alternates_file to use repository struct","startedAt":"2017-08-16T00:46:15Z","lastAt":"2017-08-16T01:12:31Z","messageCount":3,"participants":["Stefan Beller","Brandon Williams"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"326476","messageId":"20170816004558.6706-1-sbeller@google.com","threadId":"46599","inReplyTo":null,"subject":"[PATCH] convert add_to_alternates_file to use repository struct","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2017-08-16T00:45:58Z","receivedAt":"2017-08-16T00:46:15Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"The long term plan is to move most functionality to be included via\nthe repository struct, starting somewhere, convert add_to_alternates_file\nin this patch, which has link_alt_odb_entries, which is converted, too.\nAny caller outside add_to_alternates_file, just uses `the_repository`\nas the repository argument to keep the functionality the same.\n\nSigned-off-by: Stefan Beller <sbeller@google.com>\n---\n\n  For the series \"object store is embedded in the repository object\",\n  I realized we have to convert sha1_file first as that is the foundation\n  of all the object loading and writing. However there are currently other\n  series in flight, such that I do not want to change all of sha1_file\n  as it would yield serious conflicts. \n\n  This goes on top of origin/sb/sha1-file-cleanup and is one of the\n  minimum viable things to get started with converting sha1_file to\n  use the repository object.\n  \n  Thanks,\n  Stefan\n\n builtin/clone.c |  9 +++++----\n cache.h         |  2 +-\n sha1_file.c     | 23 +++++++++++++++--------\n 3 files changed, 21 insertions(+), 13 deletions(-)\n\ndiff --git a/builtin/clone.c b/builtin/clone.c\nindex 08b5cc433c..b8d170d055 100644\n--- a/builtin/clone.c\n+++ b/builtin/clone.c\n@@ -25,6 +25,7 @@\n #include \"remote.h\"\n #include \"run-command.h\"\n #include \"connected.h\"\n+#include \"repository.h\"\n \n /*\n  * Overall FIXMEs:\n@@ -327,7 +328,7 @@ static int add_one_reference(struct string_list_item *item, void *cb_data)\n \t} else {\n \t\tstruct strbuf sb = STRBUF_INIT;\n \t\tstrbuf_addf(&sb, \"%s/objects\", ref_git);\n-\t\tadd_to_alternates_file(sb.buf);\n+\t\tadd_to_alternates_file(the_repository, sb.buf);\n \t\tstrbuf_release(&sb);\n \t}\n \n@@ -369,12 +370,12 @@ static void copy_alternates(struct strbuf *src, struct strbuf *dst,\n \t\tif (!line.len || line.buf[0] == '#')\n \t\t\tcontinue;\n \t\tif (is_absolute_path(line.buf)) {\n-\t\t\tadd_to_alternates_file(line.buf);\n+\t\t\tadd_to_alternates_file(the_repository, line.buf);\n \t\t\tcontinue;\n \t\t}\n \t\tabs_path = mkpathdup(\"%s/objects/%s\", src_repo, line.buf);\n \t\tif (!normalize_path_copy(abs_path, abs_path))\n-\t\t\tadd_to_alternates_file(abs_path);\n+\t\t\tadd_to_alternates_file(the_repository, abs_path);\n \t\telse\n \t\t\twarning(\"skipping invalid relative alternate: %s/%s\",\n \t\t\t\tsrc_repo, line.buf);\n@@ -452,7 +453,7 @@ static void clone_local(const char *src_repo, const char *dest_repo)\n \tif (option_shared) {\n \t\tstruct strbuf alt = STRBUF_INIT;\n \t\tstrbuf_addf(&alt, \"%s/objects\", src_repo);\n-\t\tadd_to_alternates_file(alt.buf);\n+\t\tadd_to_alternates_file(the_repository, alt.buf);\n \t\tstrbuf_release(&alt);\n \t} else {\n \t\tstruct strbuf src = STRBUF_INIT;\ndiff --git a/cache.h b/cache.h\nindex 4109efcf24..fe1f1bc66a 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -1565,7 +1565,7 @@ struct alternate_object_database *alloc_alt_odb(const char *dir);\n  * Add the directory to the on-disk alternates file; the new entry will also\n  * take effect in the current process.\n  */\n-extern void add_to_alternates_file(const char *dir);\n+extern void add_to_alternates_file(struct repository *r, const char *dir);\n \n /*\n  * Add the directory to the in-memory list of alternates (along with any\ndiff --git a/sha1_file.c b/sha1_file.c\nindex 9186e2c6c7..298185f550 100644\n--- a/sha1_file.c\n+++ b/sha1_file.c\n@@ -22,6 +22,7 @@\n #include \"pack-revindex.h\"\n #include \"sha1-lookup.h\"\n #include \"bulk-checkin.h\"\n+#include \"repository.h\"\n #include \"streaming.h\"\n #include \"dir.h\"\n #include \"mru.h\"\n@@ -422,8 +423,9 @@ static const char *parse_alt_odb_entry(const char *string,\n \treturn end;\n }\n \n-static void link_alt_odb_entries(const char *alt, int len, int sep,\n-\t\t\t\t const char *relative_base, int depth)\n+static void link_alt_odb_entries(const struct repository *r, const char *alt,\n+\t\t\t\t int len, int sep, const char *relative_base,\n+\t\t\t\t int depth)\n {\n \tstruct strbuf objdirbuf = STRBUF_INIT;\n \tstruct strbuf entry = STRBUF_INIT;\n@@ -470,7 +472,8 @@ static void read_info_alternates(const char * relative_base, int depth)\n \tmap = xmmap(NULL, mapsz, PROT_READ, MAP_PRIVATE, fd, 0);\n \tclose(fd);\n \n-\tlink_alt_odb_entries(map, mapsz, '\\n', relative_base, depth);\n+\tlink_alt_odb_entries(the_repository, map, mapsz, '\\n',\n+\t\t\t     relative_base, depth);\n \n \tmunmap(map, mapsz);\n }\n@@ -487,10 +490,11 @@ struct alternate_object_database *alloc_alt_odb(const char *dir)\n \treturn ent;\n }\n \n-void add_to_alternates_file(const char *reference)\n+void add_to_alternates_file(struct repository *r, const char *reference)\n {\n \tstruct lock_file *lock = xcalloc(1, sizeof(struct lock_file));\n-\tchar *alts = git_pathdup(\"objects/info/alternates\");\n+\tchar *alts = repo_git_path(r, \"objects/info/alternates\");\n+\n \tFILE *in, *out;\n \n \thold_lock_file_for_update(lock, alts, LOCK_DIE_ON_ERROR);\n@@ -527,7 +531,8 @@ void add_to_alternates_file(const char *reference)\n \t\tif (commit_lock_file(lock))\n \t\t\tdie_errno(\"unable to move new alternates file into place\");\n \t\tif (alt_odb_tail)\n-\t\t\tlink_alt_odb_entries(reference, strlen(reference), '\\n', NULL, 0);\n+\t\t\tlink_alt_odb_entries(r, reference, strlen(reference),\n+\t\t\t\t\t     '\\n', NULL, 0);\n \t}\n \tfree(alts);\n }\n@@ -540,7 +545,8 @@ void add_to_alternates_memory(const char *reference)\n \t */\n \tprepare_alt_odb();\n \n-\tlink_alt_odb_entries(reference, strlen(reference), '\\n', NULL, 0);\n+\tlink_alt_odb_entries(the_repository, reference, strlen(reference),\n+\t\t\t     '\\n', NULL, 0);\n }\n \n /*\n@@ -643,7 +649,8 @@ void prepare_alt_odb(void)\n \tif (!alt) alt = \"\";\n \n \talt_odb_tail = &alt_odb_list;\n-\tlink_alt_odb_entries(alt, strlen(alt), PATH_SEP, NULL, 0);\n+\tlink_alt_odb_entries(the_repository, alt, strlen(alt),\n+\t\t\t     PATH_SEP, NULL, 0);\n \n \tread_info_alternates(get_object_directory(), 0);\n }\n-- \n2.14.0.rc0.3.g6c2e499285\n\n"},{"id":"326477","messageId":"20170816010432.GD21874@google.com","threadId":"46599","inReplyTo":"20170816004558.6706-1-sbeller@google.com","subject":"Re: [PATCH] convert add_to_alternates_file to use repository struct","fromName":"Brandon Williams","fromEmail":"bmwill@google.com","sentAt":"2017-08-16T01:04:32Z","receivedAt":"2017-08-16T01:04:40Z","isPatch":true,"sender":{"key":"bwilliams.eng@gmail.com","avatar":null},"body":"On 08/15, Stefan Beller wrote:\n> The long term plan is to move most functionality to be included via\n> the repository struct, starting somewhere, convert add_to_alternates_file\n> in this patch, which has link_alt_odb_entries, which is converted, too.\n> Any caller outside add_to_alternates_file, just uses `the_repository`\n> as the repository argument to keep the functionality the same.\n> \n> Signed-off-by: Stefan Beller <sbeller@google.com>\n> ---\n> \n>   For the series \"object store is embedded in the repository object\",\n>   I realized we have to convert sha1_file first as that is the foundation\n>   of all the object loading and writing. However there are currently other\n>   series in flight, such that I do not want to change all of sha1_file\n>   as it would yield serious conflicts. \n> \n>   This goes on top of origin/sb/sha1-file-cleanup and is one of the\n>   minimum viable things to get started with converting sha1_file to\n>   use the repository object.\n>   \n>   Thanks,\n>   Stefan\n> \n>  builtin/clone.c |  9 +++++----\n>  cache.h         |  2 +-\n>  sha1_file.c     | 23 +++++++++++++++--------\n>  3 files changed, 21 insertions(+), 13 deletions(-)\n> \n> diff --git a/builtin/clone.c b/builtin/clone.c\n> index 08b5cc433c..b8d170d055 100644\n> --- a/builtin/clone.c\n> +++ b/builtin/clone.c\n> @@ -25,6 +25,7 @@\n>  #include \"remote.h\"\n>  #include \"run-command.h\"\n>  #include \"connected.h\"\n> +#include \"repository.h\"\n>  \n>  /*\n>   * Overall FIXMEs:\n> @@ -327,7 +328,7 @@ static int add_one_reference(struct string_list_item *item, void *cb_data)\n>  \t} else {\n>  \t\tstruct strbuf sb = STRBUF_INIT;\n>  \t\tstrbuf_addf(&sb, \"%s/objects\", ref_git);\n> -\t\tadd_to_alternates_file(sb.buf);\n> +\t\tadd_to_alternates_file(the_repository, sb.buf);\n>  \t\tstrbuf_release(&sb);\n>  \t}\n>  \n> @@ -369,12 +370,12 @@ static void copy_alternates(struct strbuf *src, struct strbuf *dst,\n>  \t\tif (!line.len || line.buf[0] == '#')\n>  \t\t\tcontinue;\n>  \t\tif (is_absolute_path(line.buf)) {\n> -\t\t\tadd_to_alternates_file(line.buf);\n> +\t\t\tadd_to_alternates_file(the_repository, line.buf);\n>  \t\t\tcontinue;\n>  \t\t}\n>  \t\tabs_path = mkpathdup(\"%s/objects/%s\", src_repo, line.buf);\n>  \t\tif (!normalize_path_copy(abs_path, abs_path))\n> -\t\t\tadd_to_alternates_file(abs_path);\n> +\t\t\tadd_to_alternates_file(the_repository, abs_path);\n>  \t\telse\n>  \t\t\twarning(\"skipping invalid relative alternate: %s/%s\",\n>  \t\t\t\tsrc_repo, line.buf);\n> @@ -452,7 +453,7 @@ static void clone_local(const char *src_repo, const char *dest_repo)\n>  \tif (option_shared) {\n>  \t\tstruct strbuf alt = STRBUF_INIT;\n>  \t\tstrbuf_addf(&alt, \"%s/objects\", src_repo);\n> -\t\tadd_to_alternates_file(alt.buf);\n> +\t\tadd_to_alternates_file(the_repository, alt.buf);\n>  \t\tstrbuf_release(&alt);\n>  \t} else {\n>  \t\tstruct strbuf src = STRBUF_INIT;\n> diff --git a/cache.h b/cache.h\n> index 4109efcf24..fe1f1bc66a 100644\n> --- a/cache.h\n> +++ b/cache.h\n> @@ -1565,7 +1565,7 @@ struct alternate_object_database *alloc_alt_odb(const char *dir);\n>   * Add the directory to the on-disk alternates file; the new entry will also\n>   * take effect in the current process.\n>   */\n> -extern void add_to_alternates_file(const char *dir);\n> +extern void add_to_alternates_file(struct repository *r, const char *dir);\n>  \n>  /*\n>   * Add the directory to the in-memory list of alternates (along with any\n> diff --git a/sha1_file.c b/sha1_file.c\n> index 9186e2c6c7..298185f550 100644\n> --- a/sha1_file.c\n> +++ b/sha1_file.c\n> @@ -22,6 +22,7 @@\n>  #include \"pack-revindex.h\"\n>  #include \"sha1-lookup.h\"\n>  #include \"bulk-checkin.h\"\n> +#include \"repository.h\"\n>  #include \"streaming.h\"\n>  #include \"dir.h\"\n>  #include \"mru.h\"\n> @@ -422,8 +423,9 @@ static const char *parse_alt_odb_entry(const char *string,\n>  \treturn end;\n>  }\n>  \n> -static void link_alt_odb_entries(const char *alt, int len, int sep,\n> -\t\t\t\t const char *relative_base, int depth)\n> +static void link_alt_odb_entries(const struct repository *r, const char *alt,\n> +\t\t\t\t int len, int sep, const char *relative_base,\n> +\t\t\t\t int depth)\n\nLooks like the passed in repository 'r' isn't used in this function.  Is this intentional?  \n\n>  {\n>  \tstruct strbuf objdirbuf = STRBUF_INIT;\n>  \tstruct strbuf entry = STRBUF_INIT;\n> @@ -470,7 +472,8 @@ static void read_info_alternates(const char * relative_base, int depth)\n>  \tmap = xmmap(NULL, mapsz, PROT_READ, MAP_PRIVATE, fd, 0);\n>  \tclose(fd);\n>  \n> -\tlink_alt_odb_entries(map, mapsz, '\\n', relative_base, depth);\n> +\tlink_alt_odb_entries(the_repository, map, mapsz, '\\n',\n> +\t\t\t     relative_base, depth);\n>  \n>  \tmunmap(map, mapsz);\n>  }\n> @@ -487,10 +490,11 @@ struct alternate_object_database *alloc_alt_odb(const char *dir)\n>  \treturn ent;\n>  }\n>  \n> -void add_to_alternates_file(const char *reference)\n> +void add_to_alternates_file(struct repository *r, const char *reference)\n>  {\n>  \tstruct lock_file *lock = xcalloc(1, sizeof(struct lock_file));\n> -\tchar *alts = git_pathdup(\"objects/info/alternates\");\n> +\tchar *alts = repo_git_path(r, \"objects/info/alternates\");\n> +\n>  \tFILE *in, *out;\n>  \n>  \thold_lock_file_for_update(lock, alts, LOCK_DIE_ON_ERROR);\n> @@ -527,7 +531,8 @@ void add_to_alternates_file(const char *reference)\n>  \t\tif (commit_lock_file(lock))\n>  \t\t\tdie_errno(\"unable to move new alternates file into place\");\n>  \t\tif (alt_odb_tail)\n> -\t\t\tlink_alt_odb_entries(reference, strlen(reference), '\\n', NULL, 0);\n> +\t\t\tlink_alt_odb_entries(r, reference, strlen(reference),\n> +\t\t\t\t\t     '\\n', NULL, 0);\n>  \t}\n>  \tfree(alts);\n>  }\n> @@ -540,7 +545,8 @@ void add_to_alternates_memory(const char *reference)\n>  \t */\n>  \tprepare_alt_odb();\n>  \n> -\tlink_alt_odb_entries(reference, strlen(reference), '\\n', NULL, 0);\n> +\tlink_alt_odb_entries(the_repository, reference, strlen(reference),\n> +\t\t\t     '\\n', NULL, 0);\n>  }\n>  \n>  /*\n> @@ -643,7 +649,8 @@ void prepare_alt_odb(void)\n>  \tif (!alt) alt = \"\";\n>  \n>  \talt_odb_tail = &alt_odb_list;\n> -\tlink_alt_odb_entries(alt, strlen(alt), PATH_SEP, NULL, 0);\n> +\tlink_alt_odb_entries(the_repository, alt, strlen(alt),\n> +\t\t\t     PATH_SEP, NULL, 0);\n>  \n>  \tread_info_alternates(get_object_directory(), 0);\n>  }\n> -- \n> 2.14.0.rc0.3.g6c2e499285\n> \n\n-- \nBrandon Williams\n"},{"id":"326479","messageId":"CAGZ79kYacubf_8qF=3heuwzEEa93KLV8_LsJo5hemhpKVbujfg@mail.gmail.com","threadId":"46599","inReplyTo":"20170816010432.GD21874@google.com","subject":"Re: [PATCH] convert add_to_alternates_file to use repository struct","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2017-08-16T01:12:25Z","receivedAt":"2017-08-16T01:12:31Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Tue, Aug 15, 2017 at 6:04 PM, Brandon Williams <bmwill@google.com> wrote:\n> On 08/15, Stefan Beller wrote:\n>> The long term plan is to move most functionality to be included via\n>> the repository struct, starting somewhere, convert add_to_alternates_file\n>> in this patch, which has link_alt_odb_entries, which is converted, too.\n>> Any caller outside add_to_alternates_file, just uses `the_repository`\n>> as the repository argument to keep the functionality the same.\n>>\n>> Signed-off-by: Stefan Beller <sbeller@google.com>\n>> ---\n>>\n>>   For the series \"object store is embedded in the repository object\",\n>>   I realized we have to convert sha1_file first as that is the foundation\n>>   of all the object loading and writing. However there are currently other\n>>   series in flight, such that I do not want to change all of sha1_file\n>>   as it would yield serious conflicts.\n>>\n>>   This goes on top of origin/sb/sha1-file-cleanup and is one of the\n>>   minimum viable things to get started with converting sha1_file to\n>>   use the repository object.\n>>\n>>   Thanks,\n>>   Stefan\n>>\n>>  builtin/clone.c |  9 +++++----\n>>  cache.h         |  2 +-\n>>  sha1_file.c     | 23 +++++++++++++++--------\n>>  3 files changed, 21 insertions(+), 13 deletions(-)\n>>\n>> diff --git a/builtin/clone.c b/builtin/clone.c\n>> index 08b5cc433c..b8d170d055 100644\n>> --- a/builtin/clone.c\n>> +++ b/builtin/clone.c\n>> @@ -25,6 +25,7 @@\n>>  #include \"remote.h\"\n>>  #include \"run-command.h\"\n>>  #include \"connected.h\"\n>> +#include \"repository.h\"\n>>\n>>  /*\n>>   * Overall FIXMEs:\n>> @@ -327,7 +328,7 @@ static int add_one_reference(struct string_list_item *item, void *cb_data)\n>>       } else {\n>>               struct strbuf sb = STRBUF_INIT;\n>>               strbuf_addf(&sb, \"%s/objects\", ref_git);\n>> -             add_to_alternates_file(sb.buf);\n>> +             add_to_alternates_file(the_repository, sb.buf);\n>>               strbuf_release(&sb);\n>>       }\n>>\n>> @@ -369,12 +370,12 @@ static void copy_alternates(struct strbuf *src, struct strbuf *dst,\n>>               if (!line.len || line.buf[0] == '#')\n>>                       continue;\n>>               if (is_absolute_path(line.buf)) {\n>> -                     add_to_alternates_file(line.buf);\n>> +                     add_to_alternates_file(the_repository, line.buf);\n>>                       continue;\n>>               }\n>>               abs_path = mkpathdup(\"%s/objects/%s\", src_repo, line.buf);\n>>               if (!normalize_path_copy(abs_path, abs_path))\n>> -                     add_to_alternates_file(abs_path);\n>> +                     add_to_alternates_file(the_repository, abs_path);\n>>               else\n>>                       warning(\"skipping invalid relative alternate: %s/%s\",\n>>                               src_repo, line.buf);\n>> @@ -452,7 +453,7 @@ static void clone_local(const char *src_repo, const char *dest_repo)\n>>       if (option_shared) {\n>>               struct strbuf alt = STRBUF_INIT;\n>>               strbuf_addf(&alt, \"%s/objects\", src_repo);\n>> -             add_to_alternates_file(alt.buf);\n>> +             add_to_alternates_file(the_repository, alt.buf);\n>>               strbuf_release(&alt);\n>>       } else {\n>>               struct strbuf src = STRBUF_INIT;\n>> diff --git a/cache.h b/cache.h\n>> index 4109efcf24..fe1f1bc66a 100644\n>> --- a/cache.h\n>> +++ b/cache.h\n>> @@ -1565,7 +1565,7 @@ struct alternate_object_database *alloc_alt_odb(const char *dir);\n>>   * Add the directory to the on-disk alternates file; the new entry will also\n>>   * take effect in the current process.\n>>   */\n>> -extern void add_to_alternates_file(const char *dir);\n>> +extern void add_to_alternates_file(struct repository *r, const char *dir);\n>>\n>>  /*\n>>   * Add the directory to the in-memory list of alternates (along with any\n>> diff --git a/sha1_file.c b/sha1_file.c\n>> index 9186e2c6c7..298185f550 100644\n>> --- a/sha1_file.c\n>> +++ b/sha1_file.c\n>> @@ -22,6 +22,7 @@\n>>  #include \"pack-revindex.h\"\n>>  #include \"sha1-lookup.h\"\n>>  #include \"bulk-checkin.h\"\n>> +#include \"repository.h\"\n>>  #include \"streaming.h\"\n>>  #include \"dir.h\"\n>>  #include \"mru.h\"\n>> @@ -422,8 +423,9 @@ static const char *parse_alt_odb_entry(const char *string,\n>>       return end;\n>>  }\n>>\n>> -static void link_alt_odb_entries(const char *alt, int len, int sep,\n>> -                              const char *relative_base, int depth)\n>> +static void link_alt_odb_entries(const struct repository *r, const char *alt,\n>> +                              int len, int sep, const char *relative_base,\n>> +                              int depth)\n>\n> Looks like the passed in repository 'r' isn't used in this function.  Is this intentional?\n>\n\nDoh. I missed that part in the implementation, I need to pipe it through to\nlink_alt_odb_entry.\n"}]}