git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: 'setup_work_tree()' considered harmful

From
Linus Torvalds <torvalds@linux-foundation.org>
Date
Jun 17, 2008, 00:52 UTC
Message-ID
<alpine.LFD.1.10.0806161746170.2949@woody.linux-foundation.org>
In-Reply-To
<alpine.LFD.1.10.0806161723081.2949@woody.linux-foundation.org>
On Mon, 16 Jun 2008, Linus Torvalds wrote:
Show 5 quoted lines
> 
> After that, I fixed "write_loose_object()" to not unnecessarily try to 
> open a git object file, because every single caller has already done a 
> "has_sha1_file(sha1)" or "has_loose_object(sha1)" check before calling 
> that function, so trying to open it again is just pointless.

In case anybody cares, the obvious diff for this part is appended. The FIXME! was wrong anyway - any collision detection needs to be done by the callers when they do the "do we already have this object". Doing it in write_loose_object() is too late (not to mention the wrong place to try to do so anyway - not all callers generate new objects: some callers just turn a packed object into a loose one).

			Linus
---
From: Linus Torvalds <torvalds@linux-foundation.org>
Date: Mon, 16 Jun 2008 17:17:10 -0700
Subject: [PATCH] write_loose_object: don't bother trying to read an old object

Before even calling this, all callers have done a "has_sha1_file(sha1)" or "has_loose_object(sha1)" check, so there is no point in doing a second check.

If something races with us on object creation, we handle that in the final link() that moves it to the right place.

Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
---
 sha1_file.c |   14 --------------
 1 files changed, 0 insertions(+), 14 deletions(-)
diff --git a/sha1_file.c b/sha1_file.c
index f9ec069..4255099 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -2160,20 +2160,6 @@ static int write_loose_object(const unsigned char *sha1, char *hdr, int hdrlen,
 	static char tmpfile[PATH_MAX];
 
 	filename = sha1_file_name(sha1);
-	fd = open(filename, O_RDONLY);
-	if (fd >= 0) {
-		/*
-		 * FIXME!!! We might do collision checking here, but we'd
-		 * need to uncompress the old file and check it. Later.
-		 */
-		close(fd);
-		return 0;
-	}
-
-	if (errno != ENOENT) {
-		return error("sha1 file %s: %s\n", filename, strerror(errno));
-	}
-
 	fd = create_tmpfile(tmpfile, sizeof(tmpfile), filename);
 	if (fd < 0) {
 		if (errno == EPERM)
-- 
1.5.6.rc3.7.g336d0.dirty
Previous: Linus TorvaldsNext: Johannes Schindelin
Message 2 of 5 in “'setup_work_tree()' considered harmful”
  1. Linus TorvaldsJun 17, 2008
  2. Linus TorvaldsJun 17, 2008
  3. Johannes SchindelinJun 17, 2008
  4. Mike HommeyJun 18, 2008
  5. Linus TorvaldsJun 18, 2008

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.