Inter-revision diff: cover letter

Comparing v3 (message) to v2 (message)

--- v3
+++ v2
@@ -6,26 +6,25 @@
 functions should do, to give callers a chance to clean up after an
 error.
 
-Changes since v2:
+Changes since v1:
 
-- the commit message of the read_populate_opts() patch now clarifies why
-  we do not take care of git_config_from_file() possibly die()ing.
+- two "return error()"s replacing "die_errno()"s were turned into "return
+  error_errno()"s instead.
 
-- the save_head() and the save_opts() conditionals are now separate, for
-  improved readability.
+- an strbuf is now released when format_todo() failed (and may have left
+  the strbuf with allocated memory).
 
-- the fast_forward_to() libification now happens in its own commit.
+- a superfluous space (which was inherited from the previous code) was
+  fixed, while at it.
 
-- checkout_fast_forward_to() is now libified, too.
+- fixed commit messages to report that callers of the libified functions
+  are already libified.
 
-- added a code comment to clarify why we don't care about
-  git_parse_source() being able to die().
-
-- added rollbacks in case of failure in read_and_refresh_cache() and
-  save_head().
+- reordered patches to ensure that callers of libified functions are
+  already libified.
 
 
-Johannes Schindelin (17):
+Johannes Schindelin (14):
   sequencer: lib'ify sequencer_pick_revisions()
   sequencer: do not die() in do_pick_commit()
   sequencer: lib'ify write_message()
@@ -40,121 +39,64 @@
   sequencer: lib'ify save_head()
   sequencer: lib'ify save_todo()
   sequencer: lib'ify save_opts()
-  sequencer: lib'ify fast_forward_to()
-  lib'ify checkout_fast_forward_to()
-  sequencer: ensure to release the lock when we could not read the index
 
- merge.c     |   9 ++-
- sequencer.c | 197 ++++++++++++++++++++++++++++++++++++++----------------------
- 2 files changed, 131 insertions(+), 75 deletions(-)
+ sequencer.c | 172 ++++++++++++++++++++++++++++++++++++------------------------
+ 1 file changed, 104 insertions(+), 68 deletions(-)
 
-Published-As: https://github.com/dscho/git/releases/tag/libify-sequencer-v3
-Fetch-It-Via: git fetch https://github.com/dscho/git libify-sequencer-v3
+Published-As: https://github.com/dscho/git/releases/tag/libify-sequencer-v2
+Fetch-It-Via: git fetch https://github.com/dscho/git libify-sequencer-v2
 
-Interdiff vs v2:
+Interdiff vs v1:
 
- diff --git a/merge.c b/merge.c
- index 5db7d56..23866c9 100644
- --- a/merge.c
- +++ b/merge.c
- @@ -57,7 +57,8 @@ int checkout_fast_forward(const unsigned char *head,
-  
-  	refresh_cache(REFRESH_QUIET);
-  
- -	hold_locked_index(lock_file, 1);
- +	if (hold_locked_index(lock_file, 0) < 0)
- +		return -1;
-  
-  	memset(&trees, 0, sizeof(trees));
-  	memset(&opts, 0, sizeof(opts));
- @@ -90,7 +91,9 @@ int checkout_fast_forward(const unsigned char *head,
-  	}
-  	if (unpack_trees(nr_trees, t, &opts))
-  		return -1;
- -	if (write_locked_index(&the_index, lock_file, COMMIT_LOCK))
- -		die(_("unable to write new index file"));
- +	if (write_locked_index(&the_index, lock_file, COMMIT_LOCK)) {
- +		rollback_lock_file(lock_file);
- +		return error(_("unable to write new index file"));
- +	}
-  	return 0;
-  }
  diff --git a/sequencer.c b/sequencer.c
- index b6481bb..eec8a60 100644
+ index caba11d..b6481bb 100644
  --- a/sequencer.c
  +++ b/sequencer.c
- @@ -644,14 +644,18 @@ static int read_and_refresh_cache(struct replay_opts *opts)
-  {
-  	static struct lock_file index_lock;
-  	int index_fd = hold_locked_index(&index_lock, 0);
- -	if (read_index_preload(&the_index, NULL) < 0)
- +	if (read_index_preload(&the_index, NULL) < 0) {
- +		rollback_lock_file(&index_lock);
-  		return error(_("git %s: failed to read the index"),
-  			action_name(opts));
- +	}
-  	refresh_index(&the_index, REFRESH_QUIET|REFRESH_UNMERGED, NULL, NULL, NULL);
-  	if (the_index.cache_changed && index_fd >= 0) {
- -		if (write_locked_index(&the_index, &index_lock, COMMIT_LOCK))
- +		if (write_locked_index(&the_index, &index_lock, COMMIT_LOCK)) {
- +			rollback_lock_file(&index_lock);
-  			return error(_("git %s: failed to refresh the index"),
-  				action_name(opts));
- +		}
+ @@ -464,7 +464,7 @@ static int do_pick_commit(struct commit *commit, struct replay_opts *opts)
+  		 * to work on.
+  		 */
+  		if (write_cache_as_tree(head, 0, NULL))
+ -			return error (_("Your index file is unmerged."));
+ +			return error(_("Your index file is unmerged."));
+  	} else {
+  		unborn = get_sha1("HEAD", head);
+  		if (unborn)
+ @@ -756,8 +756,8 @@ static int read_populate_todo(struct commit_list **todo_list,
+  
+  	fd = open(git_path_todo_file(), O_RDONLY);
+  	if (fd < 0)
+ -		return error(_("Could not open %s (%s)"),
+ -			git_path_todo_file(), strerror(errno));
+ +		return error_errno(_("Could not open %s"),
+ +				   git_path_todo_file());
+  	if (strbuf_read(&buf, fd, 0) < 0) {
+  		close(fd);
+  		strbuf_release(&buf);
+ @@ -841,8 +841,8 @@ static int create_seq_dir(void)
+  		return -1;
   	}
-  	rollback_lock_file(&index_lock);
-  	return 0;
- @@ -812,6 +816,12 @@ static int read_populate_opts(struct replay_opts **opts)
-  {
-  	if (!file_exists(git_path_opts_file()))
-  		return 0;
- +	/*
- +	 * The function git_parse_source(), called from git_config_from_file(),
- +	 * may die() in case of a syntactically incorrect file. We do not care
- +	 * about this case, though, because we wrote that file ourselves, so we
- +	 * are pretty certain that it is syntactically correct.
- +	 */
-  	if (git_config_from_file(populate_opts_cb, git_path_opts_file(), *opts) < 0)
-  		return error(_("Malformed options sheet: %s"),
-  			git_path_opts_file());
- @@ -853,14 +863,20 @@ static int save_head(const char *head)
-  	int fd;
-  
-  	fd = hold_lock_file_for_update(&head_lock, git_path_head_file(), 0);
- -	if (fd < 0)
- +	if (fd < 0) {
- +		rollback_lock_file(&head_lock);
-  		return error_errno(_("Could not lock HEAD"));
- +	}
-  	strbuf_addf(&buf, "%s\n", head);
- -	if (write_in_full(fd, buf.buf, buf.len) < 0)
- +	if (write_in_full(fd, buf.buf, buf.len) < 0) {
- +		rollback_lock_file(&head_lock);
-  		return error_errno(_("Could not write to %s"),
-  				   git_path_head_file());
- -	if (commit_lock_file(&head_lock) < 0)
- +	}
- +	if (commit_lock_file(&head_lock) < 0) {
- +		rollback_lock_file(&head_lock);
-  		return error(_("Error wrapping up %s."), git_path_head_file());
- +	}
+  	else if (mkdir(git_path_seq_dir(), 0777) < 0)
+ -		return error(_("Could not create sequencer directory %s (%s)"),
+ -			  git_path_seq_dir(), strerror(errno));
+ +		return error_errno(_("Could not create sequencer directory %s"),
+ +				   git_path_seq_dir());
   	return 0;
   }
   
- @@ -1135,8 +1151,9 @@ int sequencer_pick_revisions(struct replay_opts *opts)
-  		return -1;
-  	if (get_sha1("HEAD", sha1) && (opts->action == REPLAY_REVERT))
-  		return error(_("Can't revert as initial commit"));
- -	if (save_head(sha1_to_hex(sha1)) ||
- -			save_opts(opts))
- +	if (save_head(sha1_to_hex(sha1)))
- +		return -1;
- +	if (save_opts(opts))
-  		return -1;
-  	return pick_commits(todo_list, opts);
-  }
+ @@ -941,8 +941,10 @@ static int save_todo(struct commit_list *todo_list, struct replay_opts *opts)
+  	if (fd < 0)
+  		return error_errno(_("Could not lock '%s'"),
+  				   git_path_todo_file());
+ -	if (format_todo(&buf, todo_list, opts) < 0)
+ +	if (format_todo(&buf, todo_list, opts) < 0) {
+ +		strbuf_release(&buf);
+  		return error(_("Could not format %s."), git_path_todo_file());
+ +	}
+  	if (write_in_full(fd, buf.buf, buf.len) < 0) {
+  		strbuf_release(&buf);
+  		return error_errno(_("Could not write to %s"),
 
 -- 
-2.10.0.windows.1.10.g803177d
+2.10.0.rc1.99.gcd66998
 
-base-commit: 6ebdac1bab966b720d776aa43ca188fe378b1f4b
+base-commit: 5cb0d5ad05e027cbddcb0a3c7518ddeea0f7c286
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help