add_to_alternates_file: don't add duplicate entries
The add_to_alternates_file function blindly uses
hold_lock_file_for_append to copy the existing contents, and
then adds the new line to it. This has two minor problems:
  1. We might add duplicate entries, which are ugly and
     inefficient.
  2. We do not check that the file ends with a newline, in
     which case we would bogusly append to the final line.
     This is quite unlikely in practice, though, as we call
     this function only from git-clone, so presumably we are
     the only writers of the file (and we always add a
     newline).
Instead of using hold_lock_file_for_append, let's copy the
file line by line, which ensures all records are properly
terminated. If we see an extra line, we can simply abort the
update (there is no point in even copying the rest, as we
know that it would be identical to the original).
As a bonus, we also get rid of some calls to the
static-buffer mkpath and git_path functions.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
			
			
				maint
			
			
		
							parent
							
								
									bc192300c4
								
							
						
					
					
						commit
						77b9b1d13a
					
				
							
								
								
									
										43
									
								
								sha1_file.c
								
								
								
								
							
							
						
						
									
										43
									
								
								sha1_file.c
								
								
								
								
							|  | @ -404,13 +404,46 @@ void read_info_alternates(const char * relative_base, int depth) | |||
| void add_to_alternates_file(const char *reference) | ||||
| { | ||||
| 	struct lock_file *lock = xcalloc(1, sizeof(struct lock_file)); | ||||
| 	int fd = hold_lock_file_for_append(lock, git_path("objects/info/alternates"), LOCK_DIE_ON_ERROR); | ||||
| 	const char *alt = mkpath("%s\n", reference); | ||||
| 	write_or_die(fd, alt, strlen(alt)); | ||||
| 	char *alts = git_pathdup("objects/info/alternates"); | ||||
| 	FILE *in, *out; | ||||
|  | ||||
| 	hold_lock_file_for_update(lock, alts, LOCK_DIE_ON_ERROR); | ||||
| 	out = fdopen_lock_file(lock, "w"); | ||||
| 	if (!out) | ||||
| 		die_errno("unable to fdopen alternates lockfile"); | ||||
|  | ||||
| 	in = fopen(alts, "r"); | ||||
| 	if (in) { | ||||
| 		struct strbuf line = STRBUF_INIT; | ||||
| 		int found = 0; | ||||
|  | ||||
| 		while (strbuf_getline(&line, in, '\n') != EOF) { | ||||
| 			if (!strcmp(reference, line.buf)) { | ||||
| 				found = 1; | ||||
| 				break; | ||||
| 			} | ||||
| 			fprintf_or_die(out, "%s\n", line.buf); | ||||
| 		} | ||||
|  | ||||
| 		strbuf_release(&line); | ||||
| 		fclose(in); | ||||
|  | ||||
| 		if (found) { | ||||
| 			rollback_lock_file(lock); | ||||
| 			lock = NULL; | ||||
| 		} | ||||
| 	} | ||||
| 	else if (errno != ENOENT) | ||||
| 		die_errno("unable to read alternates file"); | ||||
|  | ||||
| 	if (lock) { | ||||
| 		fprintf_or_die(out, "%s\n", reference); | ||||
| 		if (commit_lock_file(lock)) | ||||
| 		die("could not close alternates file"); | ||||
| 			die_errno("unable to move new alternates file into place"); | ||||
| 		if (alt_odb_tail) | ||||
| 		link_alt_odb_entries(alt, strlen(alt), '\n', NULL, 0); | ||||
| 			link_alt_odb_entries(reference, strlen(reference), '\n', NULL, 0); | ||||
| 	} | ||||
| 	free(alts); | ||||
| } | ||||
|  | ||||
| int foreach_alt_odb(alt_odb_fn fn, void *cb) | ||||
|  |  | |||
|  | @ -120,6 +120,11 @@ test_expect_success 'cloning with reference being subset of source (-l -s)' ' | |||
| 	git clone -l -s --reference A B E | ||||
| ' | ||||
|  | ||||
| test_expect_success 'cloning with multiple references drops duplicates' ' | ||||
| 	git clone -s --reference B --reference A --reference B A dups && | ||||
| 	test_line_count = 2 dups/.git/objects/info/alternates | ||||
| ' | ||||
|  | ||||
| test_expect_success 'clone with reference from a tagged repository' ' | ||||
| 	( | ||||
| 		cd A && git tag -a -m tagged HEAD | ||||
|  |  | |||
		Loading…
	
		Reference in New Issue
	
	 Jeff King
						Jeff King