Re: [RFC/ PATCH 2/5] unpack_trees: group errors by type

> From: Diane <diane.gasselin@ensimag.imag.fr>
>
> When an error is encountered, it calls add_rejected_file() which either
> - directly displays the error message if in plumbing mode
> - or stores it so that it will be displayed at the end of display_error_msgs(),
>
> Storing the files by error type permits to have a list of files for
> which there is the same error instead of having a serie of almost
> identical errors.
>
> As each bind_overlap error combines a file and an old file, a list cannot be
> done, therefore, theses errors are not stored but directly displayed.
>
> Signed-off-by: Diane Gasselin <diane.gasselin@ensimag.imag.fr>
> Signed-off-by: Axel Bonnet <axel.bonnet@ensimag.imag.fr>
> Signed-off-by: Clément Poulain <clement.poulain@ensimag.imag.fr>
> ---
> It appears that in verify_absent_sparse(), verify_absent_1() is called with
> ERRORMSG(o, would_lose_orphaned) as the error message.
> Yet, in verify_absent_1(), this error message error_msg does not
> seem to be used and at the end of the function, a would_lose_untracked error
> is treated (before displayed and now added). Is it normal?
>
>  unpack-trees.c |  128 ++++++++++++++++++++++++++++++++++++++++++++++++++++----
>  unpack-trees.h |   12 +++++
>  2 files changed, 132 insertions(+), 8 deletions(-)
>
> diff --git a/unpack-trees.c b/unpack-trees.c
> index c29a9e0..1e2f48d 100644
> --- a/unpack-trees.c
> +++ b/unpack-trees.c
> @@ -45,6 +45,21 @@ static struct unpack_trees_error_msgs unpack_plumbing_errors = {
>        ? ((o)->msgs.fld) \
>        : (unpack_plumbing_errors.fld) )
>
> +/*
> + * Store error messages in an array, each case
> + * corresponding to a error message type
> + */
> +typedef enum {
> +       would_overwrite,
> +       not_uptodate_file,
> +       not_uptodate_dir,
> +       would_lose_untracked,
> +       would_lose_untracked_removed,
> +       sparse_not_uptodate_file
> +} unpack_trees_error;
> +#define NB_UNPACK_TREES_ERROR 6
> +struct rejected_files *unpack_rejects[NB_UNPACK_TREES_ERROR];
> +
>  static void add_entry(struct unpack_trees_options *o, struct cache_entry *ce,
>        unsigned int set, unsigned int clear)
>  {
> @@ -60,6 +75,88 @@ static void add_entry(struct unpack_trees_options *o, struct cache_entry *ce,
>  }
>
>  /*
> + * add error messages on file <file> and action <action>
> + * corresponding to the type <e> with the message <msg>
> + * indicating if it should be display in porcelain or not
> + */
> +static int add_rejected_file(unpack_trees_error e,
> +                            const char *file,
> +                            const char *action,
> +                            int porcelain,
> +                            const char *msg)
> +{
> +       struct rejected_files_list *newentry;
> +       /*
> +        * simply display the given error message if in plumbing mode
> +        */
> +       if (!porcelain) {
> +               error(msg,file,action);
> +               return -1;
> +       }
> +       /*
> +        * if there is a porcelain error message defined,
> +        * the error is stored in order to be nicely displayed later
> +        */
> +       if (e == would_lose_untracked && !strcmp(action,"removed"))
> +               e = would_lose_untracked_removed;
> +
> +       if (!unpack_rejects[e]) {
> +               unpack_rejects[e] = malloc(sizeof(struct rejected_files));
> +               unpack_rejects[e]->list = NULL;
> +               unpack_rejects[e]->size = 0;
> +       }
> +       newentry = malloc(sizeof(struct rejected_files_list));
> +       newentry->file = (char *)file;
> +       newentry->next = unpack_rejects[e]->list;
> +       unpack_rejects[e]->list = newentry;
> +       unpack_rejects[e]->msg = msg;
> +       unpack_rejects[e]->action = (char *)action;
> +       unpack_rejects[e]->size += strlen(file)+strlen("\n")+strlen("\t");
> +       return -1;
> +}
> +
> +/*
> + * free all the structures allocated for the error <e>
> + */
> +static void free_rejected_files(unpack_trees_error e)
> +{
> +       while(unpack_rejects[e]->list) {
> +               struct rejected_files_list *del = unpack_rejects[e]->list;
> +               unpack_rejects[e]->list = unpack_rejects[e]->list->next;
> +               free(del);
> +       }
> +       free(unpack_rejects[e]);
> +}
> +
> +/*
> + * display all the error messages stored in a nice way
> + */
> +static void display_error_msgs()
> +{
> +       int i;
> +       int hasPorcelain = 0;
> +       for (i=0; i<NB_UNPACK_TREES_ERROR; i++) {
> +               if (unpack_rejects[i] && unpack_rejects[i]->list) {
> +                       hasPorcelain = 1;
> +                       struct rejected_files_list *f = unpack_rejects[i]->list;
> +                       char *action = unpack_rejects[i]->action;
> +                       char *file = malloc(unpack_rejects[i]->size+1);
> +                       *file = '\0';
> +                       while (f) {
> +                               strcat(file,"\t");
> +                               strcat(file,f->file);
> +                               strcat(file,"\n");
> +                               f = f->next;
> +                       }
> +                       error(unpack_rejects[i]->msg,file,action);
> +                       free_rejected_files(i);
> +               }
> +       }
> +       if (hasPorcelain)
> +               printf("Aborting\n");
> +}
> +
> +/*
>  * Unlink the last component and schedule the leading directories for
>  * removal, such that empty directories get removed.
>  */
> @@ -819,6 +916,7 @@ done:
>        return ret;
>
>  return_failed:
> +       display_error_msgs();
>        mark_all_ce_unused(o->src_index);
>        ret = unpack_failed(o, NULL);
>        goto done;
> @@ -828,7 +926,9 @@ return_failed:
>
>  static int reject_merge(struct cache_entry *ce, struct unpack_trees_options *o)
>  {
> -       return error(ERRORMSG(o, would_overwrite), ce->name);
> +       return add_rejected_file(would_overwrite, ce->name, NULL,
> +                                (o && (o)->msgs.would_overwrite),
> +                                ERRORMSG(o, would_overwrite));
>  }
>
>  static int same(struct cache_entry *a, struct cache_entry *b)
> @@ -850,7 +950,7 @@ static int same(struct cache_entry *a, struct cache_entry *b)
>  */
>  static int verify_uptodate_1(struct cache_entry *ce,
>                                   struct unpack_trees_options *o,
> -                                  const char *error_msg)
> +                                  unpack_trees_error error)
>  {
>        struct stat st;
>
> @@ -874,8 +974,16 @@ static int verify_uptodate_1(struct cache_entry *ce,
>        }
>        if (errno == ENOENT)
>                return 0;
> -       return o->gently ? -1 :
> -               error(error_msg, ce->name);
> +       if (error == sparse_not_uptodate_file)
> +               return o->gently ? -1 :
> +                       add_rejected_file(sparse_not_uptodate_file, ce->name, NULL,
> +                                         (o && (o)->msgs.sparse_not_uptodate_file),
> +                                         ERRORMSG(o, sparse_not_uptodate_file));
> +       else
> +               return o->gently ? -1 :
> +                       add_rejected_file(not_uptodate_file, ce->name, NULL,
> +                                         (o && (o)->msgs.not_uptodate_file),
> +                                         ERRORMSG(o, not_uptodate_file));
>  }
>
>  static int verify_uptodate(struct cache_entry *ce,
> @@ -883,13 +991,13 @@ static int verify_uptodate(struct cache_entry *ce,
>  {
>        if (!o->skip_sparse_checkout && will_have_skip_worktree(ce, o))
>                return 0;
> -       return verify_uptodate_1(ce, o, ERRORMSG(o, not_uptodate_file));
> +       return verify_uptodate_1(ce, o, not_uptodate_file);
>  }
>
>  static int verify_uptodate_sparse(struct cache_entry *ce,
>                                  struct unpack_trees_options *o)
>  {
> -       return verify_uptodate_1(ce, o, ERRORMSG(o, sparse_not_uptodate_file));
> +       return verify_uptodate_1(ce, o, sparse_not_uptodate_file);
>  }
>
>  static void invalidate_ce_path(struct cache_entry *ce, struct unpack_trees_options *o)
> @@ -976,7 +1084,9 @@ static int verify_clean_subdirectory(struct cache_entry *ce, const char *action,
>        i = read_directory(&d, pathbuf, namelen+1, NULL);
>        if (i)
>                return o->gently ? -1 :
> -                       error(ERRORMSG(o, not_uptodate_dir), ce->name);
> +                       add_rejected_file(not_uptodate_dir, ce->name, NULL,
> +                                         (o && (o)->msgs.not_uptodate_dir),
> +                                         ERRORMSG(o, not_uptodate_dir));
>        free(pathbuf);
>        return cnt;
>  }
> @@ -1058,7 +1168,9 @@ static int verify_absent_1(struct cache_entry *ce, const char *action,
>                }
>
>                return o->gently ? -1 :
> -                       error(ERRORMSG(o, would_lose_untracked), ce->name, action);
> +                       add_rejected_file(would_lose_untracked, ce->name, action,
> +                                         (o && (o)->msgs.would_lose_untracked),
> +                                         ERRORMSG(o, would_lose_untracked));
>        }
>        return 0;
>  }
> diff --git a/unpack-trees.h b/unpack-trees.h
> index ef70eab..49cc1ee 100644
> --- a/unpack-trees.h
> +++ b/unpack-trees.h
> @@ -19,6 +19,18 @@ struct unpack_trees_error_msgs {
>        const char *would_lose_orphaned;
>  };
>
> +struct rejected_files_list {
> +       char *file;
> +       struct rejected_files_list *next;
> +};
> +
> +struct rejected_files {
> +       char *action;
> +       const char *msg;
> +       size_t size;
> +       struct rejected_files_list *list;
> +};
> +
>  struct unpack_trees_options {
>        unsigned int reset,
>                     merge,
> --
> 1.6.6.7.ga5fe3
>
>