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

[Diane Gasselin <diane.gasselin@ensimag.imag.fr>]

Sorry about this.
We had a problem sending our patch, only one message was sent and we
don't know what happened to the others (they have not been returned to
us).
As soon as the problem is fixed, we will send the entire patch.
Sorry again for the noise.

Le 9 juin 2010 14:44, Diane Gasselin <diane.gasselin@ensimag.imag.fr> a écrit :
> 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
>
>