On Fri, Dec 05, 2014 at 12:08:21AM +0100, Michael Haggerty wrote:
> Extract two functions, reflog_expiry_prepare() and
> reflog_expiry_cleanup(), from expire_reflog(). This is a further step
> towards separating the code for deciding on expiration policy from the
> code that manages the physical expiration.
>
> This change requires a couple of local variables from expire_reflog()
> to be turned into fields of "struct expire_reflog_cb". More
> reorganization of the callback data will follow in later commits.
>
> Signed-off-by: Michael Haggerty <mhagger@xxxxxxxxxxxx>
Reviewed-by: Stefan Beller <sbeller@xxxxxxxxxx>
> ---
> In fact, the work done in reflog_expire_cleanup() doesn't really need
> to be done via a callback, because it doesn't need to be done while
> the reference lock is held. But the symmetry between prepare and
> cleanup is kindof nice. Perhaps some future policy decision will want
> to do some final work under the reference lock?
>
> But it would be easy to get rid of this third callback function and
> have the callers do the work themselves after calling expire_reflog().
> I don't have a string feeling either way.
>
> builtin/reflog.c | 94 +++++++++++++++++++++++++++++++-------------------------
> 1 file changed, 52 insertions(+), 42 deletions(-)
>
> diff --git a/builtin/reflog.c b/builtin/reflog.c
> index 7bc6e0f..ebfa635 100644
> --- a/builtin/reflog.c
> +++ b/builtin/reflog.c
> @@ -43,6 +43,8 @@ struct expire_reflog_cb {
> unsigned long mark_limit;
> struct cmd_reflog_expire_cb *cmd;
> unsigned char last_kept_sha1[20];
> + struct commit *tip_commit;
> + struct commit_list *tips;
> };
>
> struct collected_reflog {
> @@ -363,6 +365,54 @@ static int push_tip_to_list(const char *refname, const unsigned char *sha1, int
> return 0;
> }
>
> +static void reflog_expiry_prepare(const char *refname,
> + const unsigned char *sha1,
> + struct expire_reflog_cb *cb)
> +{
> + if (!cb->cmd->expire_unreachable || !strcmp(refname, "HEAD")) {
> + cb->tip_commit = NULL;
> + cb->unreachable_expire_kind = UE_HEAD;
> + } else {
> + cb->tip_commit = lookup_commit_reference_gently(sha1, 1);
> + if (!cb->tip_commit)
> + cb->unreachable_expire_kind = UE_ALWAYS;
> + else
> + cb->unreachable_expire_kind = UE_NORMAL;
> + }
> +
> + if (cb->cmd->expire_unreachable <= cb->cmd->expire_total)
> + cb->unreachable_expire_kind = UE_ALWAYS;
> +
> + cb->mark_list = NULL;
> + cb->tips = NULL;
> + if (cb->unreachable_expire_kind != UE_ALWAYS) {
> + if (cb->unreachable_expire_kind == UE_HEAD) {
> + struct commit_list *elem;
> + for_each_ref(push_tip_to_list, &cb->tips);
> + for (elem = cb->tips; elem; elem = elem->next)
> + commit_list_insert(elem->item, &cb->mark_list);
> + } else {
> + commit_list_insert(cb->tip_commit, &cb->mark_list);
> + }
> + cb->mark_limit = cb->cmd->expire_total;
> + mark_reachable(cb);
> + }
> +}
> +
> +static void reflog_expiry_cleanup(struct expire_reflog_cb *cb)
> +{
> + if (cb->unreachable_expire_kind != UE_ALWAYS) {
> + if (cb->unreachable_expire_kind == UE_HEAD) {
> + struct commit_list *elem;
> + for (elem = cb->tips; elem; elem = elem->next)
> + clear_commit_marks(elem->item, REACHABLE);
> + free_commit_list(cb->tips);
> + } else {
> + clear_commit_marks(cb->tip_commit, REACHABLE);
> + }
> + }
> +}
> +
> static struct lock_file reflog_lock;
>
> static int expire_reflog(const char *refname, const unsigned char *sha1, void *cb_data)
> @@ -371,8 +421,6 @@ static int expire_reflog(const char *refname, const unsigned char *sha1, void *c
> struct expire_reflog_cb cb;
> struct ref_lock *lock;
> char *log_file;
> - struct commit *tip_commit;
> - struct commit_list *tips;
> int status = 0;
>
> memset(&cb, 0, sizeof(cb));
> @@ -400,47 +448,9 @@ static int expire_reflog(const char *refname, const unsigned char *sha1, void *c
>
> cb.cmd = cmd;
>
> - if (!cmd->expire_unreachable || !strcmp(refname, "HEAD")) {
> - tip_commit = NULL;
> - cb.unreachable_expire_kind = UE_HEAD;
> - } else {
> - tip_commit = lookup_commit_reference_gently(sha1, 1);
> - if (!tip_commit)
> - cb.unreachable_expire_kind = UE_ALWAYS;
> - else
> - cb.unreachable_expire_kind = UE_NORMAL;
> - }
> -
> - if (cmd->expire_unreachable <= cmd->expire_total)
> - cb.unreachable_expire_kind = UE_ALWAYS;
> -
> - cb.mark_list = NULL;
> - tips = NULL;
> - if (cb.unreachable_expire_kind != UE_ALWAYS) {
> - if (cb.unreachable_expire_kind == UE_HEAD) {
> - struct commit_list *elem;
> - for_each_ref(push_tip_to_list, &tips);
> - for (elem = tips; elem; elem = elem->next)
> - commit_list_insert(elem->item, &cb.mark_list);
> - } else {
> - commit_list_insert(tip_commit, &cb.mark_list);
> - }
> - cb.mark_limit = cmd->expire_total;
> - mark_reachable(&cb);
> - }
> -
> + reflog_expiry_prepare(refname, sha1, &cb);
> for_each_reflog_ent(refname, expire_reflog_ent, &cb);
> -
> - if (cb.unreachable_expire_kind != UE_ALWAYS) {
> - if (cb.unreachable_expire_kind == UE_HEAD) {
> - struct commit_list *elem;
> - for (elem = tips; elem; elem = elem->next)
> - clear_commit_marks(elem->item, REACHABLE);
> - free_commit_list(tips);
> - } else {
> - clear_commit_marks(tip_commit, REACHABLE);
> - }
> - }
> + reflog_expiry_cleanup(&cb);
>
> if (cb.newlog) {
> if (close_lock_file(&reflog_lock)) {
> --
> 2.1.3
>
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html