Re: [PATCH v9 3/7] refs: new public ref function: safe_create_reflog
From: Michael Haggerty <hidden>
Date: 2016-06-15 23:05:50
On 07/21/2015 08:45 AM, David Turner wrote:
quoted hunk ↗ jump to hunk
The safe_create_reflog function creates a reflog, if it does not already exist. The log_ref_setup function becomes private and gains a force_create parameter to force the creation of a reflog even if log_all_ref_updates is false or the refname is not one of the special refnames. The new parameter also reduces the need to store, modify, and restore the log_all_ref_updates global before reflog creation. In a moment, we will use this to add reflog creation commands to git-reflog. Signed-off-by: David Turner <redacted> --- builtin/checkout.c | 14 +++++--------- refs.c | 24 ++++++++++++++++++++---- refs.h | 2 +- 3 files changed, 26 insertions(+), 14 deletions(-) [...]diff --git a/refs.c b/refs.c index 2efa2dc..3277768 100644 --- a/refs.c +++ b/refs.c@@ -3128,8 +3128,13 @@ static int should_autocreate_reflog(const char *refname) !strcmp(refname, "HEAD"); } -/* This function will fill in *err and return -1 on failure */ -int log_ref_setup(const char *refname, struct strbuf *sb_logfile, struct strbuf *err) +/* + * Create a reflog for a ref. If force_create = 0, the reflog will + * only be created for certain refs (those for which + * should_autocreate_reflog returns non-zero. Otherwise, create it + * regardless of the ref name. Fill in *err and return -1 on failure. + */ +static int log_ref_setup(const char *refname, struct strbuf *sb_logfile, struct strbuf *err, int force_create) { int logfd, oflags = O_APPEND | O_WRONLY; char *logfile;@@ -3138,7 +3143,7 @@ int log_ref_setup(const char *refname, struct strbuf *sb_logfile, struct strbuf logfile = sb_logfile->buf; /* make sure the rest of the function can't change "logfile" */ sb_logfile = NULL; - if (should_autocreate_reflog(refname)) { + if (force_create || should_autocreate_reflog(refname)) { if (safe_create_leading_directories(logfile) < 0) { strbuf_addf(err, "unable to create directory for %s: " "%s", logfile, strerror(errno));
I think usually the "strbuf *err" parameter is the last one in our APIs. I noticed this when reviewing patch 5/7, where there is a similar flags argument that comes *before* "err". Consistency would suggest that "force_create" come before "err". Michael -- Michael Haggerty mhagger@alum.mit.edu