Re: [PATCH 07/11] rerere: use repo_read_index_or_die

[Stefan Beller]

Hi Thomas,

On Sun, May 20, 2018 at 10:45 AM, Thomas Gummerer  wrote:
> On 05/16, Stefan Beller wrote:
>> By switching to repo_read_index_or_die, we'll get a slightly different
>> error message ("index file corrupt") as well as localization of it.
>
> Apart from the slightly different error message, and the localization
> (both of which I think are a good thing), I notice that this also
> turns a "return error(...)" into a "die(...)".  I thought we were
> going more towards not 'die'ing in libgit.a code, and letting the
> caller handling the errors?  Either way I think this change should be
> described in the commit message.

oops, I will to drop or fix this patch from the series as I think the
not die()ing is a good idea for libgit.

> Also all other messages in 'rerere.c' are currently not translated.
> I'm currently working on a series that includes a patch to do just
> that (amongst some other changes to 'rerere'), which I'm hoping to
> send soon-ish, but the textual conflicts should we still want this
> patch should be easy to solve.

I did not mark this series well enough, it was a mere attempt to
"see how it goes", but was retracted as a whole[1] after Junio
dreaded some merge conflicts.

[1] 
https://public-inbox.org/git/CAGZ79kbvjoTq5079Ks+h2HNb+D99RELYPcJk2=pvzf9-y8d...@mail.gmail.com/

So do not feel bad about any merge conflicts in rerere, as this
series will not land any time soon.

Stefan

Re: [PATCH 07/11] rerere: use repo_read_index_or_die

[Thomas Gummerer]

On 05/16, Stefan Beller wrote:
> By switching to repo_read_index_or_die, we'll get a slightly different
> error message ("index file corrupt") as well as localization of it.

Apart from the slightly different error message, and the localization
(both of which I think are a good thing), I notice that this also
turns a "return error(...)" into a "die(...)".  I thought we were
going more towards not 'die'ing in libgit.a code, and letting the
caller handling the errors?  Either way I think this change should be
described in the commit message.

Also all other messages in 'rerere.c' are currently not translated.
I'm currently working on a series that includes a patch to do just
that (amongst some other changes to 'rerere'), which I'm hoping to
send soon-ish, but the textual conflicts should we still want this
patch should be easy to solve.

> Signed-off-by: Stefan Beller 
> ---
>  rerere.c | 10 --
>  1 file changed, 4 insertions(+), 6 deletions(-)
> 
> diff --git a/rerere.c b/rerere.c
> index 18cae2d11c9..5f35dd66f90 100644
> --- a/rerere.c
> +++ b/rerere.c
> @@ -10,6 +10,7 @@
>  #include "attr.h"
>  #include "pathspec.h"
>  #include "sha1-lookup.h"
> +#include "repository.h"
>  
>  #define RESOLVED 0
>  #define PUNTED 1
> @@ -567,8 +568,7 @@ static int check_one_conflict(int i, int *type)
>  static int find_conflict(struct string_list *conflict)
>  {
>   int i;
> - if (read_cache() < 0)
> - return error("Could not read index");
> + repo_read_index_or_die(the_repository);
>  
>   for (i = 0; i < active_nr;) {
>   int conflict_type;
> @@ -600,8 +600,7 @@ int rerere_remaining(struct string_list *merge_rr)
>   int i;
>   if (setup_rerere(merge_rr, RERERE_READONLY))
>   return 0;
> - if (read_cache() < 0)
> - return error("Could not read index");
> + repo_read_index_or_die(the_repository);
>  
>   for (i = 0; i < active_nr;) {
>   int conflict_type;
> @@ -1103,8 +1102,7 @@ int rerere_forget(struct pathspec *pathspec)
>   struct string_list conflict = STRING_LIST_INIT_DUP;
>   struct string_list merge_rr = STRING_LIST_INIT_DUP;
>  
> - if (read_cache() < 0)
> - return error("Could not read index");
> + repo_read_index_or_die(the_repository);
>  
>   fd = setup_rerere(&merge_rr, RERERE_NOAUTOUPDATE);
>   if (fd < 0)
> -- 
> 2.17.0.582.gccdcbd54c44.dirty
>