diff mbox series

[v5,03/14] object-file: pass filename to fsync_or_die

Message ID 2d1bc4568ac744f11c886a5f964dbe563c04ce8b.1648616734.git.gitgitgadget@gmail.com (mailing list archive)
State Accepted
Commit c4e707f858813c097bd8b488baae07096aa280ad
Headers show
Series core.fsyncmethod: add 'batch' mode for faster fsyncing of multiple objects | expand

Commit Message

Neeraj Singh (WINDOWS-SFS) March 30, 2022, 5:05 a.m. UTC
From: Neeraj Singh <neerajsi@microsoft.com>

If we die while trying to fsync a loose object file, pass the actual
filename we're trying to sync. This is likely to be more helpful for a
user trying to diagnose the cause of the failure than the former
'loose object file' string. It also sidesteps any concerns about
translating the die message differently for loose objects versus
something else that has a real path.

Signed-off-by: Neeraj Singh <neerajsi@microsoft.com>
---
 object-file.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

Comments

Junio C Hamano March 30, 2022, 5:18 p.m. UTC | #1
"Neeraj Singh via GitGitGadget" <gitgitgadget@gmail.com> writes:

> From: Neeraj Singh <neerajsi@microsoft.com>
>
> If we die while trying to fsync a loose object file, pass the actual
> filename we're trying to sync. This is likely to be more helpful for a
> user trying to diagnose the cause of the failure than the former
> 'loose object file' string. It also sidesteps any concerns about
> translating the die message differently for loose objects versus
> something else that has a real path.
>
> Signed-off-by: Neeraj Singh <neerajsi@microsoft.com>
> ---
>  object-file.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)

Looks good and obviously can be done totally outside the series.
Perhaps while the rest of the topic is still cooking, extract this
(we may find others) and have them graduate sooner?

> diff --git a/object-file.c b/object-file.c
> index b254bc50d70..5ffbf3d4fd4 100644
> --- a/object-file.c
> +++ b/object-file.c
> @@ -1888,16 +1888,16 @@ void hash_object_file(const struct git_hash_algo *algo, const void *buf,
>  }
>  
>  /* Finalize a file on disk, and close it. */
> -static void close_loose_object(int fd)
> +static void close_loose_object(int fd, const char *filename)
>  {
>  	if (the_repository->objects->odb->will_destroy)
>  		goto out;
>  
>  	if (fsync_object_files > 0)
> -		fsync_or_die(fd, "loose object file");
> +		fsync_or_die(fd, filename);
>  	else
>  		fsync_component_or_die(FSYNC_COMPONENT_LOOSE_OBJECT, fd,
> -				       "loose object file");
> +				       filename);
>  
>  out:
>  	if (close(fd) != 0)
> @@ -2011,7 +2011,7 @@ static int write_loose_object(const struct object_id *oid, char *hdr,
>  		die(_("confused by unstable object source data for %s"),
>  		    oid_to_hex(oid));
>  
> -	close_loose_object(fd);
> +	close_loose_object(fd, tmp_file.buf);
>  
>  	if (mtime) {
>  		struct utimbuf utb;
Neeraj Singh March 30, 2022, 5:54 p.m. UTC | #2
On Wed, Mar 30, 2022 at 10:18 AM Junio C Hamano <gitster@pobox.com> wrote:
>
> "Neeraj Singh via GitGitGadget" <gitgitgadget@gmail.com> writes:
>
> > From: Neeraj Singh <neerajsi@microsoft.com>
> >
> > If we die while trying to fsync a loose object file, pass the actual
> > filename we're trying to sync. This is likely to be more helpful for a
> > user trying to diagnose the cause of the failure than the former
> > 'loose object file' string. It also sidesteps any concerns about
> > translating the die message differently for loose objects versus
> > something else that has a real path.
> >
> > Signed-off-by: Neeraj Singh <neerajsi@microsoft.com>
> > ---
> >  object-file.c | 8 ++++----
> >  1 file changed, 4 insertions(+), 4 deletions(-)
>
> Looks good and obviously can be done totally outside the series.
> Perhaps while the rest of the topic is still cooking, extract this
> (we may find others) and have them graduate sooner?
>
> > diff --git a/object-file.c b/object-file.c
> > index b254bc50d70..5ffbf3d4fd4 100644
> > --- a/object-file.c
> > +++ b/object-file.c
> > @@ -1888,16 +1888,16 @@ void hash_object_file(const struct git_hash_algo *algo, const void *buf,
> >  }
> >
> >  /* Finalize a file on disk, and close it. */
> > -static void close_loose_object(int fd)
> > +static void close_loose_object(int fd, const char *filename)
> >  {
> >       if (the_repository->objects->odb->will_destroy)
> >               goto out;
> >
> >       if (fsync_object_files > 0)
> > -             fsync_or_die(fd, "loose object file");
> > +             fsync_or_die(fd, filename);
> >       else
> >               fsync_component_or_die(FSYNC_COMPONENT_LOOSE_OBJECT, fd,
> > -                                    "loose object file");
> > +                                    filename);
> >
> >  out:
> >       if (close(fd) != 0)
> > @@ -2011,7 +2011,7 @@ static int write_loose_object(const struct object_id *oid, char *hdr,
> >               die(_("confused by unstable object source data for %s"),
> >                   oid_to_hex(oid));
> >
> > -     close_loose_object(fd);
> > +     close_loose_object(fd, tmp_file.buf);
> >
> >       if (mtime) {
> >               struct utimbuf utb;


Ok. I'll create two new single-patch series with changes that are independent.
diff mbox series

Patch

diff --git a/object-file.c b/object-file.c
index b254bc50d70..5ffbf3d4fd4 100644
--- a/object-file.c
+++ b/object-file.c
@@ -1888,16 +1888,16 @@  void hash_object_file(const struct git_hash_algo *algo, const void *buf,
 }
 
 /* Finalize a file on disk, and close it. */
-static void close_loose_object(int fd)
+static void close_loose_object(int fd, const char *filename)
 {
 	if (the_repository->objects->odb->will_destroy)
 		goto out;
 
 	if (fsync_object_files > 0)
-		fsync_or_die(fd, "loose object file");
+		fsync_or_die(fd, filename);
 	else
 		fsync_component_or_die(FSYNC_COMPONENT_LOOSE_OBJECT, fd,
-				       "loose object file");
+				       filename);
 
 out:
 	if (close(fd) != 0)
@@ -2011,7 +2011,7 @@  static int write_loose_object(const struct object_id *oid, char *hdr,
 		die(_("confused by unstable object source data for %s"),
 		    oid_to_hex(oid));
 
-	close_loose_object(fd);
+	close_loose_object(fd, tmp_file.buf);
 
 	if (mtime) {
 		struct utimbuf utb;