reflog + refs-backend: move "verbose" out of the backend

Move the handling of the "verbose" flag entirely out of
"refs/files-backend.c" and into "builtin/reflog.c". This allows the
backend to stop knowing about the EXPIRE_REFLOGS_VERBOSE flag.

The expire_reflog_ent() function shouldn't need to deal with the
implementation detail of whether or not we're emitting verbose output,
by doing this the --verbose output becomes backend-agnostic, so
reftable will get the same output.

I think the output is rather bad currently, and should e.g. be
implemented with some better future mode of progress.[ch], but that's
a topic for another improvement.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/builtin/reflog.c b/builtin/reflog.c
index 4ff63846058a39771a40b9b9c0000da7d3b3004b..a4b1dd27e13c93c670489f11c7c20b980952ba2a 100644 (file)
--- a/builtin/reflog.c
+++ b/builtin/reflog.c
@@ -45,6 +45,7 @@ struct expire_reflog_policy_cb {
        struct cmd_reflog_expire_cb cmd;
        struct commit *tip_commit;
        struct commit_list *tips;
+       unsigned int dry_run:1;
 };
 
 struct worktree_reflogs {
@@ -319,6 +320,28 @@ static int should_expire_reflog_ent(struct object_id *ooid, struct object_id *no
        return 0;
 }
 
+static int should_expire_reflog_ent_verbose(struct object_id *ooid,
+                                           struct object_id *noid,
+                                           const char *email,
+                                           timestamp_t timestamp, int tz,
+                                           const char *message, void *cb_data)
+{
+       struct expire_reflog_policy_cb *cb = cb_data;
+       int expire;
+
+       expire = should_expire_reflog_ent(ooid, noid, email, timestamp, tz,
+                                         message, cb);
+
+       if (!expire)
+               printf("keep %s", message);
+       else if (cb->dry_run)
+               printf("would prune %s", message);
+       else
+               printf("prune %s", message);
+
+       return expire;
+}
+
 static int push_tip_to_list(const char *refname, const struct object_id *oid,
                            int flags, void *cb_data)
 {
@@ -539,6 +562,8 @@ static int cmd_reflog_expire(int argc, const char **argv, const char *prefix)
        int i, status, do_all, all_worktrees = 1;
        int explicit_expiry = 0;
        unsigned int flags = 0;
+       int verbose = 0;
+       reflog_expiry_should_prune_fn *should_prune_fn = should_expire_reflog_ent;
 
        default_reflog_expire_unreachable = now - 30 * 24 * 3600;
        default_reflog_expire = now - 90 * 24 * 3600;
@@ -576,7 +601,7 @@ static int cmd_reflog_expire(int argc, const char **argv, const char *prefix)
                else if (!strcmp(arg, "--single-worktree"))
                        all_worktrees = 0;
                else if (!strcmp(arg, "--verbose"))
-                       flags |= EXPIRE_REFLOGS_VERBOSE;
+                       verbose = 1;
                else if (!strcmp(arg, "--")) {
                        i++;
                        break;
@@ -587,6 +612,9 @@ static int cmd_reflog_expire(int argc, const char **argv, const char *prefix)
                        break;
        }
 
+       if (verbose)
+               should_prune_fn = should_expire_reflog_ent_verbose;
+
        /*
         * We can trust the commits and objects reachable from refs
         * even in older repository.  We cannot trust what's reachable
@@ -599,10 +627,10 @@ static int cmd_reflog_expire(int argc, const char **argv, const char *prefix)
                revs.do_not_die_on_missing_tree = 1;
                revs.ignore_missing = 1;
                revs.ignore_missing_links = 1;
-               if (flags & EXPIRE_REFLOGS_VERBOSE)
+               if (verbose)
                        printf(_("Marking reachable objects..."));
                mark_reachable_objects(&revs, 0, 0, NULL);
-               if (flags & EXPIRE_REFLOGS_VERBOSE)
+               if (verbose)
                        putchar('\n');
        }
 
@@ -624,12 +652,15 @@ static int cmd_reflog_expire(int argc, const char **argv, const char *prefix)
                free_worktrees(worktrees);
 
                for_each_string_list_item(item, &collected.reflogs) {
-                       struct expire_reflog_policy_cb cb = { .cmd = cmd };
+                       struct expire_reflog_policy_cb cb = {
+                               .cmd = cmd,
+                               .dry_run = !!(flags & EXPIRE_REFLOGS_DRY_RUN),
+                       };
 
                        set_reflog_expiry_param(&cb.cmd, explicit_expiry, item->string);
                        status |= reflog_expire(item->string, flags,
                                                reflog_expiry_prepare,
-                                               should_expire_reflog_ent,
+                                               should_prune_fn,
                                                reflog_expiry_cleanup,
                                                &cb);
                }
@@ -647,7 +678,7 @@ static int cmd_reflog_expire(int argc, const char **argv, const char *prefix)
                set_reflog_expiry_param(&cb.cmd, explicit_expiry, ref);
                status |= reflog_expire(ref, flags,
                                        reflog_expiry_prepare,
-                                       should_expire_reflog_ent,
+                                       should_prune_fn,
                                        reflog_expiry_cleanup,
                                        &cb);
                free(ref);
@@ -670,6 +701,8 @@ static int cmd_reflog_delete(int argc, const char **argv, const char *prefix)
        struct cmd_reflog_expire_cb cmd = { 0 };
        int i, status = 0;
        unsigned int flags = 0;
+       int verbose = 0;
+       reflog_expiry_should_prune_fn *should_prune_fn = should_expire_reflog_ent;
 
        for (i = 1; i < argc; i++) {
                const char *arg = argv[i];
@@ -680,7 +713,7 @@ static int cmd_reflog_delete(int argc, const char **argv, const char *prefix)
                else if (!strcmp(arg, "--updateref"))
                        flags |= EXPIRE_REFLOGS_UPDATE_REF;
                else if (!strcmp(arg, "--verbose"))
-                       flags |= EXPIRE_REFLOGS_VERBOSE;
+                       verbose = 1;
                else if (!strcmp(arg, "--")) {
                        i++;
                        break;
@@ -691,6 +724,9 @@ static int cmd_reflog_delete(int argc, const char **argv, const char *prefix)
                        break;
        }
 
+       if (verbose)
+               should_prune_fn = should_expire_reflog_ent_verbose;
+
        if (argc - i < 1)
                return error(_("no reflog specified to delete"));
 
@@ -698,7 +734,9 @@ static int cmd_reflog_delete(int argc, const char **argv, const char *prefix)
                const char *spec = strstr(argv[i], "@{");
                char *ep, *ref;
                int recno;
-               struct expire_reflog_policy_cb cb = { 0 };
+               struct expire_reflog_policy_cb cb = {
+                       .dry_run = !!(flags & EXPIRE_REFLOGS_DRY_RUN),
+               };
 
                if (!spec) {
                        status |= error(_("not a reflog: %s"), argv[i]);
@@ -723,7 +761,7 @@ static int cmd_reflog_delete(int argc, const char **argv, const char *prefix)
                cb.cmd = cmd;
                status |= reflog_expire(ref, flags,
                                        reflog_expiry_prepare,
-                                       should_expire_reflog_ent,
+                                       should_prune_fn,
                                        reflog_expiry_cleanup,
                                        &cb);
                free(ref);

diff --git a/refs.h b/refs.h
index 92360e55a2045970a2a75ed6ca5505f0a0beb8fa..8f91a7f9ff27ec0909c32d14122bf0f1a5c49573 100644 (file)
--- a/refs.h
+++ b/refs.h
@@ -820,8 +820,7 @@ enum ref_type ref_type(const char *refname);
 enum expire_reflog_flags {
        EXPIRE_REFLOGS_DRY_RUN = 1 << 0,
        EXPIRE_REFLOGS_UPDATE_REF = 1 << 1,
-       EXPIRE_REFLOGS_VERBOSE = 1 << 2,
-       EXPIRE_REFLOGS_REWRITE = 1 << 3
+       EXPIRE_REFLOGS_REWRITE = 1 << 2,
 };
 
 /*

diff --git a/refs/files-backend.c b/refs/files-backend.c
index 5f8586a36e351219e966662bfdd62ebaa169d0a1..6178ad8c77c31396f63aff6e47cda46ea62b3e8a 100644 (file)
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -3086,11 +3086,12 @@ cleanup:
 }
 
 struct expire_reflog_cb {
-       unsigned int flags;
        reflog_expiry_should_prune_fn *should_prune_fn;
        void *policy_cb;
        FILE *newlog;
        struct object_id last_kept_oid;
+       unsigned int rewrite:1,
+                    dry_run:1;
 };
 
 static int expire_reflog_ent(struct object_id *ooid, struct object_id *noid,
@@ -3098,33 +3099,27 @@ static int expire_reflog_ent(struct object_id *ooid, struct object_id *noid,
                             const char *message, void *cb_data)
 {
        struct expire_reflog_cb *cb = cb_data;
-       struct expire_reflog_policy_cb *policy_cb = cb->policy_cb;
+       reflog_expiry_should_prune_fn *fn = cb->should_prune_fn;
 
-       if (cb->flags & EXPIRE_REFLOGS_REWRITE)
+       if (cb->rewrite)
                ooid = &cb->last_kept_oid;
 
-       if ((*cb->should_prune_fn)(ooid, noid, email, timestamp, tz,
-                                  message, policy_cb)) {
-               if (cb->flags & EXPIRE_REFLOGS_DRY_RUN)
-                       printf("would prune %s", message);
-               else if (cb->flags & EXPIRE_REFLOGS_VERBOSE)
-                       printf("prune %s", message);
-       } else {
-               if (!(cb->flags & EXPIRE_REFLOGS_DRY_RUN)) {
-                       fprintf(cb->newlog, "%s %s %s %"PRItime" %+05d\t%s",
-                               oid_to_hex(ooid), oid_to_hex(noid),
-                               email, timestamp, tz, message);
-                       oidcpy(&cb->last_kept_oid, noid);
-               }
-               if (cb->flags & EXPIRE_REFLOGS_VERBOSE)
-                       printf("keep %s", message);
-       }
+       if (fn(ooid, noid, email, timestamp, tz, message, cb->policy_cb))
+               return 0;
+
+       if (cb->dry_run)
+               return 0; /* --dry-run */
+
+       fprintf(cb->newlog, "%s %s %s %"PRItime" %+05d\t%s", oid_to_hex(ooid),
+               oid_to_hex(noid), email, timestamp, tz, message);
+       oidcpy(&cb->last_kept_oid, noid);
+
        return 0;
 }
 
 static int files_reflog_expire(struct ref_store *ref_store,
                               const char *refname,
-                              unsigned int flags,
+                              unsigned int expire_flags,
                               reflog_expiry_prepare_fn prepare_fn,
                               reflog_expiry_should_prune_fn should_prune_fn,
                               reflog_expiry_cleanup_fn cleanup_fn,
@@ -3142,7 +3137,8 @@ static int files_reflog_expire(struct ref_store *ref_store,
        const struct object_id *oid;
 
        memset(&cb, 0, sizeof(cb));
-       cb.flags = flags;
+       cb.rewrite = !!(expire_flags & EXPIRE_REFLOGS_REWRITE);
+       cb.dry_run = !!(expire_flags & EXPIRE_REFLOGS_DRY_RUN);
        cb.policy_cb = policy_cb_data;
        cb.should_prune_fn = should_prune_fn;
 
@@ -3178,7 +3174,7 @@ static int files_reflog_expire(struct ref_store *ref_store,
 
        files_reflog_path(refs, &log_file_sb, refname);
        log_file = strbuf_detach(&log_file_sb, NULL);
-       if (!(flags & EXPIRE_REFLOGS_DRY_RUN)) {
+       if (!cb.dry_run) {
                /*
                 * Even though holding $GIT_DIR/logs/$reflog.lock has
                 * no locking implications, we use the lock_file
@@ -3205,7 +3201,7 @@ static int files_reflog_expire(struct ref_store *ref_store,
        refs_for_each_reflog_ent(ref_store, refname, expire_reflog_ent, &cb);
        (*cleanup_fn)(cb.policy_cb);
 
-       if (!(flags & EXPIRE_REFLOGS_DRY_RUN)) {
+       if (!cb.dry_run) {
                /*
                 * It doesn't make sense to adjust a reference pointed
                 * to by a symbolic ref based on expiring entries in
@@ -3215,7 +3211,7 @@ static int files_reflog_expire(struct ref_store *ref_store,
                 */
                int update = 0;
 
-               if ((flags & EXPIRE_REFLOGS_UPDATE_REF) &&
+               if ((expire_flags & EXPIRE_REFLOGS_UPDATE_REF) &&
                    !is_null_oid(&cb.last_kept_oid)) {
                        int ignore_errno;
                        int type;