[PATCH] convert add_to_alternates_file to use repository struct

[PATCH] convert add_to_alternates_file to use repository struct

[Stefan Beller]

The long term plan is to move most functionality to be included via
the repository struct, starting somewhere, convert add_to_alternates_file
in this patch, which has link_alt_odb_entries, which is converted, too.
Any caller outside add_to_alternates_file, just uses `the_repository`
as the repository argument to keep the functionality the same.

Signed-off-by: Stefan Beller <sbeller@google.com>
---

  For the series "object store is embedded in the repository object",
  I realized we have to convert sha1_file first as that is the foundation
  of all the object loading and writing. However there are currently other
  series in flight, such that I do not want to change all of sha1_file
  as it would yield serious conflicts. 

  This goes on top of origin/sb/sha1-file-cleanup and is one of the
  minimum viable things to get started with converting sha1_file to
  use the repository object.
  
  Thanks,
  Stefan

 builtin/clone.c |  9 +++++----
 cache.h         |  2 +-
 sha1_file.c     | 23 +++++++++++++++--------
 3 files changed, 21 insertions(+), 13 deletions(-)

diff --git a/builtin/clone.c b/builtin/clone.c
index 08b5cc433c..b8d170d055 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -25,6 +25,7 @@
 #include "remote.h"
 #include "run-command.h"
 #include "connected.h"
+#include "repository.h"
 
 /*
  * Overall FIXMEs:
@@ -327,7 +328,7 @@ static int add_one_reference(struct string_list_item *item, void *cb_data)
 	} else {
 		struct strbuf sb = STRBUF_INIT;
 		strbuf_addf(&sb, "%s/objects", ref_git);
-		add_to_alternates_file(sb.buf);
+		add_to_alternates_file(the_repository, sb.buf);
 		strbuf_release(&sb);
 	}
 
@@ -369,12 +370,12 @@ static void copy_alternates(struct strbuf *src, struct strbuf *dst,
 		if (!line.len || line.buf[0] == '#')
 			continue;
 		if (is_absolute_path(line.buf)) {
-			add_to_alternates_file(line.buf);
+			add_to_alternates_file(the_repository, line.buf);
 			continue;
 		}
 		abs_path = mkpathdup("%s/objects/%s", src_repo, line.buf);
 		if (!normalize_path_copy(abs_path, abs_path))
-			add_to_alternates_file(abs_path);
+			add_to_alternates_file(the_repository, abs_path);
 		else
 			warning("skipping invalid relative alternate: %s/%s",
 				src_repo, line.buf);
@@ -452,7 +453,7 @@ static void clone_local(const char *src_repo, const char *dest_repo)
 	if (option_shared) {
 		struct strbuf alt = STRBUF_INIT;
 		strbuf_addf(&alt, "%s/objects", src_repo);
-		add_to_alternates_file(alt.buf);
+		add_to_alternates_file(the_repository, alt.buf);
 		strbuf_release(&alt);
 	} else {
 		struct strbuf src = STRBUF_INIT;
diff --git a/cache.h b/cache.h
index 4109efcf24..fe1f1bc66a 100644
--- a/cache.h
+++ b/cache.h
@@ -1565,7 +1565,7 @@ struct alternate_object_database *alloc_alt_odb(const char *dir);
  * Add the directory to the on-disk alternates file; the new entry will also
  * take effect in the current process.
  */
-extern void add_to_alternates_file(const char *dir);
+extern void add_to_alternates_file(struct repository *r, const char *dir);
 
 /*
  * Add the directory to the in-memory list of alternates (along with any
diff --git a/sha1_file.c b/sha1_file.c
index 9186e2c6c7..298185f550 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -22,6 +22,7 @@
 #include "pack-revindex.h"
 #include "sha1-lookup.h"
 #include "bulk-checkin.h"
+#include "repository.h"
 #include "streaming.h"
 #include "dir.h"
 #include "mru.h"
@@ -422,8 +423,9 @@ static const char *parse_alt_odb_entry(const char *string,
 	return end;
 }
 
-static void link_alt_odb_entries(const char *alt, int len, int sep,
-				 const char *relative_base, int depth)
+static void link_alt_odb_entries(const struct repository *r, const char *alt,
+				 int len, int sep, const char *relative_base,
+				 int depth)
 {
 	struct strbuf objdirbuf = STRBUF_INIT;
 	struct strbuf entry = STRBUF_INIT;
@@ -470,7 +472,8 @@ static void read_info_alternates(const char * relative_base, int depth)
 	map = xmmap(NULL, mapsz, PROT_READ, MAP_PRIVATE, fd, 0);
 	close(fd);
 
-	link_alt_odb_entries(map, mapsz, '\n', relative_base, depth);
+	link_alt_odb_entries(the_repository, map, mapsz, '\n',
+			     relative_base, depth);
 
 	munmap(map, mapsz);
 }
@@ -487,10 +490,11 @@ struct alternate_object_database *alloc_alt_odb(const char *dir)
 	return ent;
 }
 
-void add_to_alternates_file(const char *reference)
+void add_to_alternates_file(struct repository *r, const char *reference)
 {
 	struct lock_file *lock = xcalloc(1, sizeof(struct lock_file));
-	char *alts = git_pathdup("objects/info/alternates");
+	char *alts = repo_git_path(r, "objects/info/alternates");
+
 	FILE *in, *out;
 
 	hold_lock_file_for_update(lock, alts, LOCK_DIE_ON_ERROR);
@@ -527,7 +531,8 @@ void add_to_alternates_file(const char *reference)
 		if (commit_lock_file(lock))
 			die_errno("unable to move new alternates file into place");
 		if (alt_odb_tail)
-			link_alt_odb_entries(reference, strlen(reference), '\n', NULL, 0);
+			link_alt_odb_entries(r, reference, strlen(reference),
+					     '\n', NULL, 0);
 	}
 	free(alts);
 }
@@ -540,7 +545,8 @@ void add_to_alternates_memory(const char *reference)
 	 */
 	prepare_alt_odb();
 
-	link_alt_odb_entries(reference, strlen(reference), '\n', NULL, 0);
+	link_alt_odb_entries(the_repository, reference, strlen(reference),
+			     '\n', NULL, 0);
 }
 
 /*
@@ -643,7 +649,8 @@ void prepare_alt_odb(void)
 	if (!alt) alt = "";
 
 	alt_odb_tail = &alt_odb_list;
-	link_alt_odb_entries(alt, strlen(alt), PATH_SEP, NULL, 0);
+	link_alt_odb_entries(the_repository, alt, strlen(alt),
+			     PATH_SEP, NULL, 0);
 
 	read_info_alternates(get_object_directory(), 0);
 }
-- 
2.14.0.rc0.3.g6c2e499285

Re: [PATCH] convert add_to_alternates_file to use repository struct

[Brandon Williams]

On 08/15, Stefan Beller wrote:
> The long term plan is to move most functionality to be included via
> the repository struct, starting somewhere, convert add_to_alternates_file
> in this patch, which has link_alt_odb_entries, which is converted, too.
> Any caller outside add_to_alternates_file, just uses `the_repository`
> as the repository argument to keep the functionality the same.
> 
> Signed-off-by: Stefan Beller <sbeller@google.com>
> ---
> 
>   For the series "object store is embedded in the repository object",
>   I realized we have to convert sha1_file first as that is the foundation
>   of all the object loading and writing. However there are currently other
>   series in flight, such that I do not want to change all of sha1_file
>   as it would yield serious conflicts. 
> 
>   This goes on top of origin/sb/sha1-file-cleanup and is one of the
>   minimum viable things to get started with converting sha1_file to
>   use the repository object.
>   
>   Thanks,
>   Stefan
> 
>  builtin/clone.c |  9 +++++----
>  cache.h         |  2 +-
>  sha1_file.c     | 23 +++++++++++++++--------
>  3 files changed, 21 insertions(+), 13 deletions(-)
> 
> diff --git a/builtin/clone.c b/builtin/clone.c
> index 08b5cc433c..b8d170d055 100644
> --- a/builtin/clone.c
> +++ b/builtin/clone.c
> @@ -25,6 +25,7 @@
>  #include "remote.h"
>  #include "run-command.h"
>  #include "connected.h"
> +#include "repository.h"
>  
>  /*
>   * Overall FIXMEs:
> @@ -327,7 +328,7 @@ static int add_one_reference(struct string_list_item *item, void *cb_data)
>  	} else {
>  		struct strbuf sb = STRBUF_INIT;
>  		strbuf_addf(&sb, "%s/objects", ref_git);
> -		add_to_alternates_file(sb.buf);
> +		add_to_alternates_file(the_repository, sb.buf);
>  		strbuf_release(&sb);
>  	}
>  
> @@ -369,12 +370,12 @@ static void copy_alternates(struct strbuf *src, struct strbuf *dst,
>  		if (!line.len || line.buf[0] == '#')
>  			continue;
>  		if (is_absolute_path(line.buf)) {
> -			add_to_alternates_file(line.buf);
> +			add_to_alternates_file(the_repository, line.buf);
>  			continue;
>  		}
>  		abs_path = mkpathdup("%s/objects/%s", src_repo, line.buf);
>  		if (!normalize_path_copy(abs_path, abs_path))
> -			add_to_alternates_file(abs_path);
> +			add_to_alternates_file(the_repository, abs_path);
>  		else
>  			warning("skipping invalid relative alternate: %s/%s",
>  				src_repo, line.buf);
> @@ -452,7 +453,7 @@ static void clone_local(const char *src_repo, const char *dest_repo)
>  	if (option_shared) {
>  		struct strbuf alt = STRBUF_INIT;
>  		strbuf_addf(&alt, "%s/objects", src_repo);
> -		add_to_alternates_file(alt.buf);
> +		add_to_alternates_file(the_repository, alt.buf);
>  		strbuf_release(&alt);
>  	} else {
>  		struct strbuf src = STRBUF_INIT;
> diff --git a/cache.h b/cache.h
> index 4109efcf24..fe1f1bc66a 100644
> --- a/cache.h
> +++ b/cache.h
> @@ -1565,7 +1565,7 @@ struct alternate_object_database *alloc_alt_odb(const char *dir);
>   * Add the directory to the on-disk alternates file; the new entry will also
>   * take effect in the current process.
>   */
> -extern void add_to_alternates_file(const char *dir);
> +extern void add_to_alternates_file(struct repository *r, const char *dir);
>  
>  /*
>   * Add the directory to the in-memory list of alternates (along with any
> diff --git a/sha1_file.c b/sha1_file.c
> index 9186e2c6c7..298185f550 100644
> --- a/sha1_file.c
> +++ b/sha1_file.c
> @@ -22,6 +22,7 @@
>  #include "pack-revindex.h"
>  #include "sha1-lookup.h"
>  #include "bulk-checkin.h"
> +#include "repository.h"
>  #include "streaming.h"
>  #include "dir.h"
>  #include "mru.h"
> @@ -422,8 +423,9 @@ static const char *parse_alt_odb_entry(const char *string,
>  	return end;
>  }
>  
> -static void link_alt_odb_entries(const char *alt, int len, int sep,
> -				 const char *relative_base, int depth)
> +static void link_alt_odb_entries(const struct repository *r, const char *alt,
> +				 int len, int sep, const char *relative_base,
> +				 int depth)

Looks like the passed in repository 'r' isn't used in this function.  Is this intentional?  

>  {
>  	struct strbuf objdirbuf = STRBUF_INIT;
>  	struct strbuf entry = STRBUF_INIT;
> @@ -470,7 +472,8 @@ static void read_info_alternates(const char * relative_base, int depth)
>  	map = xmmap(NULL, mapsz, PROT_READ, MAP_PRIVATE, fd, 0);
>  	close(fd);
>  
> -	link_alt_odb_entries(map, mapsz, '\n', relative_base, depth);
> +	link_alt_odb_entries(the_repository, map, mapsz, '\n',
> +			     relative_base, depth);
>  
>  	munmap(map, mapsz);
>  }
> @@ -487,10 +490,11 @@ struct alternate_object_database *alloc_alt_odb(const char *dir)
>  	return ent;
>  }
>  
> -void add_to_alternates_file(const char *reference)
> +void add_to_alternates_file(struct repository *r, const char *reference)
>  {
>  	struct lock_file *lock = xcalloc(1, sizeof(struct lock_file));
> -	char *alts = git_pathdup("objects/info/alternates");
> +	char *alts = repo_git_path(r, "objects/info/alternates");
> +
>  	FILE *in, *out;
>  
>  	hold_lock_file_for_update(lock, alts, LOCK_DIE_ON_ERROR);
> @@ -527,7 +531,8 @@ void add_to_alternates_file(const char *reference)
>  		if (commit_lock_file(lock))
>  			die_errno("unable to move new alternates file into place");
>  		if (alt_odb_tail)
> -			link_alt_odb_entries(reference, strlen(reference), '\n', NULL, 0);
> +			link_alt_odb_entries(r, reference, strlen(reference),
> +					     '\n', NULL, 0);
>  	}
>  	free(alts);
>  }
> @@ -540,7 +545,8 @@ void add_to_alternates_memory(const char *reference)
>  	 */
>  	prepare_alt_odb();
>  
> -	link_alt_odb_entries(reference, strlen(reference), '\n', NULL, 0);
> +	link_alt_odb_entries(the_repository, reference, strlen(reference),
> +			     '\n', NULL, 0);
>  }
>  
>  /*
> @@ -643,7 +649,8 @@ void prepare_alt_odb(void)
>  	if (!alt) alt = "";
>  
>  	alt_odb_tail = &alt_odb_list;
> -	link_alt_odb_entries(alt, strlen(alt), PATH_SEP, NULL, 0);
> +	link_alt_odb_entries(the_repository, alt, strlen(alt),
> +			     PATH_SEP, NULL, 0);
>  
>  	read_info_alternates(get_object_directory(), 0);
>  }
> -- 
> 2.14.0.rc0.3.g6c2e499285
> 

-- 
Brandon Williams

Re: [PATCH] convert add_to_alternates_file to use repository struct

[Stefan Beller]

On Tue, Aug 15, 2017 at 6:04 PM, Brandon Williams <bmwill@google.com> wrote:
> On 08/15, Stefan Beller wrote:
>> The long term plan is to move most functionality to be included via
>> the repository struct, starting somewhere, convert add_to_alternates_file
>> in this patch, which has link_alt_odb_entries, which is converted, too.
>> Any caller outside add_to_alternates_file, just uses `the_repository`
>> as the repository argument to keep the functionality the same.
>>
>> Signed-off-by: Stefan Beller <sbeller@google.com>
>> ---
>>
>>   For the series "object store is embedded in the repository object",
>>   I realized we have to convert sha1_file first as that is the foundation
>>   of all the object loading and writing. However there are currently other
>>   series in flight, such that I do not want to change all of sha1_file
>>   as it would yield serious conflicts.
>>
>>   This goes on top of origin/sb/sha1-file-cleanup and is one of the
>>   minimum viable things to get started with converting sha1_file to
>>   use the repository object.
>>
>>   Thanks,
>>   Stefan
>>
>>  builtin/clone.c |  9 +++++----
>>  cache.h         |  2 +-
>>  sha1_file.c     | 23 +++++++++++++++--------
>>  3 files changed, 21 insertions(+), 13 deletions(-)
>>
>> diff --git a/builtin/clone.c b/builtin/clone.c
>> index 08b5cc433c..b8d170d055 100644
>> --- a/builtin/clone.c
>> +++ b/builtin/clone.c
>> @@ -25,6 +25,7 @@
>>  #include "remote.h"
>>  #include "run-command.h"
>>  #include "connected.h"
>> +#include "repository.h"
>>
>>  /*
>>   * Overall FIXMEs:
>> @@ -327,7 +328,7 @@ static int add_one_reference(struct string_list_item *item, void *cb_data)
>>       } else {
>>               struct strbuf sb = STRBUF_INIT;
>>               strbuf_addf(&sb, "%s/objects", ref_git);
>> -             add_to_alternates_file(sb.buf);
>> +             add_to_alternates_file(the_repository, sb.buf);
>>               strbuf_release(&sb);
>>       }
>>
>> @@ -369,12 +370,12 @@ static void copy_alternates(struct strbuf *src, struct strbuf *dst,
>>               if (!line.len || line.buf[0] == '#')
>>                       continue;
>>               if (is_absolute_path(line.buf)) {
>> -                     add_to_alternates_file(line.buf);
>> +                     add_to_alternates_file(the_repository, line.buf);
>>                       continue;
>>               }
>>               abs_path = mkpathdup("%s/objects/%s", src_repo, line.buf);
>>               if (!normalize_path_copy(abs_path, abs_path))
>> -                     add_to_alternates_file(abs_path);
>> +                     add_to_alternates_file(the_repository, abs_path);
>>               else
>>                       warning("skipping invalid relative alternate: %s/%s",
>>                               src_repo, line.buf);
>> @@ -452,7 +453,7 @@ static void clone_local(const char *src_repo, const char *dest_repo)
>>       if (option_shared) {
>>               struct strbuf alt = STRBUF_INIT;
>>               strbuf_addf(&alt, "%s/objects", src_repo);
>> -             add_to_alternates_file(alt.buf);
>> +             add_to_alternates_file(the_repository, alt.buf);
>>               strbuf_release(&alt);
>>       } else {
>>               struct strbuf src = STRBUF_INIT;
>> diff --git a/cache.h b/cache.h
>> index 4109efcf24..fe1f1bc66a 100644
>> --- a/cache.h
>> +++ b/cache.h
>> @@ -1565,7 +1565,7 @@ struct alternate_object_database *alloc_alt_odb(const char *dir);
>>   * Add the directory to the on-disk alternates file; the new entry will also
>>   * take effect in the current process.
>>   */
>> -extern void add_to_alternates_file(const char *dir);
>> +extern void add_to_alternates_file(struct repository *r, const char *dir);
>>
>>  /*
>>   * Add the directory to the in-memory list of alternates (along with any
>> diff --git a/sha1_file.c b/sha1_file.c
>> index 9186e2c6c7..298185f550 100644
>> --- a/sha1_file.c
>> +++ b/sha1_file.c
>> @@ -22,6 +22,7 @@
>>  #include "pack-revindex.h"
>>  #include "sha1-lookup.h"
>>  #include "bulk-checkin.h"
>> +#include "repository.h"
>>  #include "streaming.h"
>>  #include "dir.h"
>>  #include "mru.h"
>> @@ -422,8 +423,9 @@ static const char *parse_alt_odb_entry(const char *string,
>>       return end;
>>  }
>>
>> -static void link_alt_odb_entries(const char *alt, int len, int sep,
>> -                              const char *relative_base, int depth)
>> +static void link_alt_odb_entries(const struct repository *r, const char *alt,
>> +                              int len, int sep, const char *relative_base,
>> +                              int depth)
>
> Looks like the passed in repository 'r' isn't used in this function.  Is this intentional?
>

Doh. I missed that part in the implementation, I need to pipe it through to
link_alt_odb_entry.