git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Derrick Stolee <dstolee@microsoft.com>
To: "git@vger.kernel.org" <git@vger.kernel.org>
Cc: "peff@peff.net" <peff@peff.net>,
	"gitster@pobox.com" <gitster@pobox.com>,
	"pclouds@gmail.com" <pclouds@gmail.com>,
	Derrick Stolee <dstolee@microsoft.com>
Subject: [PATCH v2] commit-graph: fix UX issue when .lock file exists
Date: Thu, 10 May 2018 17:42:52 +0000	[thread overview]
Message-ID: <20180510174242.44346-1-dstolee@microsoft.com> (raw)
In-Reply-To: <20180509141523.89896-1-dstolee@microsoft.com>

We use the lockfile API to avoid multiple Git processes from writing to
the commit-graph file in the .git/objects/info directory. In some cases,
this directory may not exist, so we check for its existence.

The existing code does the following when acquiring the lock:

1. Try to acquire the lock.
2. If it fails, try to create the .git/object/info directory.
3. Try to acquire the lock, failing if necessary.

The problem is that if the lockfile exists, then the mkdir fails, giving
an error that doesn't help the user:

  "fatal: cannot mkdir .git/objects/info: File exists"

While technically this honors the lockfile, it does not help the user.

Instead, do the following:

1. Check for existence of .git/objects/info; create if necessary.
2. Try to acquire the lock, failing if necessary.

The new output looks like:

  fatal: Unable to create
  '<dir>/.git/objects/info/commit-graph.lock': File exists.

  Another git process seems to be running in this repository, e.g.
  an editor opened by 'git commit'. Please make sure all processes
  are terminated then try again. If it still fails, a git process
  may have crashed in this repository earlier:
  remove the file manually to continue.

Helped-by: Jeff King <peff@peff.net>
Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
---
 commit-graph.c | 22 +++++-----------------
 1 file changed, 5 insertions(+), 17 deletions(-)

diff --git a/commit-graph.c b/commit-graph.c
index a8c337dd77..bb54c1214c 100644
--- a/commit-graph.c
+++ b/commit-graph.c
@@ -1,5 +1,6 @@
 #include "cache.h"
 #include "config.h"
+#include "dir.h"
 #include "git-compat-util.h"
 #include "lockfile.h"
 #include "pack.h"
@@ -640,7 +641,6 @@ void write_commit_graph(const char *obj_dir,
 	struct hashfile *f;
 	uint32_t i, count_distinct = 0;
 	char *graph_name;
-	int fd;
 	struct lock_file lk = LOCK_INIT;
 	uint32_t chunk_ids[5];
 	uint64_t chunk_offsets[5];
@@ -754,23 +754,11 @@ void write_commit_graph(const char *obj_dir,
 	compute_generation_numbers(&commits);
 
 	graph_name = get_commit_graph_filename(obj_dir);
-	fd = hold_lock_file_for_update(&lk, graph_name, 0);
-
-	if (fd < 0) {
-		struct strbuf folder = STRBUF_INIT;
-		strbuf_addstr(&folder, graph_name);
-		strbuf_setlen(&folder, strrchr(folder.buf, '/') - folder.buf);
-
-		if (mkdir(folder.buf, 0777) < 0)
-			die_errno(_("cannot mkdir %s"), folder.buf);
-		strbuf_release(&folder);
-
-		fd = hold_lock_file_for_update(&lk, graph_name, LOCK_DIE_ON_ERROR);
-
-		if (fd < 0)
-			die_errno("unable to create '%s'", graph_name);
-	}
+	if (safe_create_leading_directories(graph_name))
+		die_errno(_("unable to create leading directories of %s"),
+			  graph_name);
 
+	hold_lock_file_for_update(&lk, graph_name, LOCK_DIE_ON_ERROR);
 	f = hashfd(lk.tempfile->fd, lk.tempfile->filename.buf);
 
 	hashwrite_be32(f, GRAPH_SIGNATURE);

base-commit: 34fdd433396ee0e3ef4de02eb2189f8226eafe4e
-- 
2.16.2.329.gfb62395de6


  parent reply	other threads:[~2018-05-10 17:42 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-05-09 14:15 [PATCH 0/1] Fix UX issue with commit-graph.lock Derrick Stolee
2018-05-09 14:15 ` [PATCH 1/1] commit-graph: fix UX issue when .lock file exists Derrick Stolee
2018-05-09 14:42   ` Jeff King
2018-05-09 14:53     ` Derrick Stolee
2018-05-10  4:53       ` Jeff King
2018-05-09 16:02 ` [PATCH 0/1] Fix UX issue with commit-graph.lock Duy Nguyen
2018-05-10  7:00 ` Junio C Hamano
2018-05-10 12:52   ` Derrick Stolee
2018-05-11  1:28     ` Junio C Hamano
2018-05-10 17:42 ` Derrick Stolee [this message]
2018-05-11  8:59   ` [PATCH v2] commit-graph: fix UX issue when .lock file exists Jeff King

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20180510174242.44346-1-dstolee@microsoft.com \
    --to=dstolee@microsoft.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=pclouds@gmail.com \
    --cc=peff@peff.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).