git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Clemens Buchacher <drizzd@gmx.net>
To: git@vger.kernel.org
Cc: "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>,
	"Duy Nguyen" <pclouds@gmail.com>,
	"brian m. carlson" <sandals@crustytoothpaste.net>,
	"Junio C Hamano" <gitster@pobox.com>,
	"Orgad Shaneh" <orgads@gmail.com>
Subject: [PATCH v2] checkout files in-place
Date: Mon, 11 Jun 2018 22:39:58 +0200	[thread overview]
Message-ID: <20180611203958.GA1306@Sonnenschein.localdomain> (raw)
In-Reply-To: <20180610194444.GA1913@Sonnenschein.localdomain>

When replacing files with new content during checkout, we do not write
to them in place. Instead we unlink and recreate the files in order to
let the system figure out ownership and permissions for the new file,
taking umask into account.

It is safe to do this on Linux file systems, even if open file handles
still exist, because unlink only removes the directory reference to the
file. On Windows, however, a file cannot be deleted until all handles to
it are closed. If a file cannot be deleted, its name cannot be reused.

This causes files to be deleted, but not checked out when switching
branches. This is frequently an issue with Qt Creator, which
continuously opens files in the work tree, as reported here:
https://github.com/git-for-windows/git/issues/1653

This change adds the core.checkoutInPlace option. If enabled, checkout
will open files for writing the new content in place. This fixes the
issue, but with this approach the system will not update file
permissions according to umask. Only essential updates of write and
executable permissions are performed.

The in-place checkout is therefore optional. It could be enabled by Git
installers on Windows, where umask is irrelevant.

Signed-off-by: Clemens Buchacher <drizzd@gmx.net>
Reviewed-by: Orgad Shaneh <orgads@gmail.com>
Reviewed-by: "brian m. carlson" <sandals@crustytoothpaste.net>
Reviewed-by: Duy Nguyen <pclouds@gmail.com>
Reviewed-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---

Tested on Windows with Git-for-Windows and with Windows Subsystem for
Linux.

 Documentation/config.txt    | 11 ++++++
 cache.h                     |  2 ++
 config.c                    |  5 +++
 entry.c                     | 18 ++++++++--
 environment.c               |  1 +
 t/t2031-checkout-inplace.sh | 82 +++++++++++++++++++++++++++++++++++++++++++++
 6 files changed, 116 insertions(+), 3 deletions(-)
 create mode 100755 t/t2031-checkout-inplace.sh

diff --git a/Documentation/config.txt b/Documentation/config.txt
index ab641bf..0860a81 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -912,6 +912,17 @@ core.sparseCheckout::
 	Enable "sparse checkout" feature. See section "Sparse checkout" in
 	linkgit:git-read-tree[1] for more information.
 
+core.checkoutInPlace::
+	Check out file contents in place. By default Git checkout removes existing
+	work tree files before it replaces them with different content. If this
+	option is enabled, Git will overwrite the contents of existing files in
+	place. This is useful on Windows, where open file handles to a removed file
+	prevent creating new files at the same path.
+	Note that the current implementation of in-place checkout makes no effort
+	to update read/write permissions according to umask. Permissions are
+	however modified to enable write access and to update executable
+	permissions.
+
 core.abbrev::
 	Set the length object names are abbreviated to.  If
 	unspecified or set to "auto", an appropriate value is
diff --git a/cache.h b/cache.h
index 89a107a..5b8c4d6 100644
--- a/cache.h
+++ b/cache.h
@@ -815,6 +815,7 @@ extern int fsync_object_files;
 extern int core_preload_index;
 extern int core_commit_graph;
 extern int core_apply_sparse_checkout;
+extern int checkout_inplace;
 extern int precomposed_unicode;
 extern int protect_hfs;
 extern int protect_ntfs;
@@ -1518,6 +1519,7 @@ struct checkout {
 	unsigned force:1,
 		 quiet:1,
 		 not_new:1,
+		 inplace:1,
 		 refresh_cache:1;
 };
 #define CHECKOUT_INIT { NULL, "" }
diff --git a/config.c b/config.c
index fbbf0f8..8b35ecd 100644
--- a/config.c
+++ b/config.c
@@ -1318,6 +1318,11 @@ static int git_default_core_config(const char *var, const char *value)
 		return 0;
 	}
 
+	if (!strcmp(var, "core.checkoutinplace")) {
+		checkout_inplace = git_config_bool(var, value);
+		return 0;
+	}
+
 	if (!strcmp(var, "core.precomposeunicode")) {
 		precomposed_unicode = git_config_bool(var, value);
 		return 0;
diff --git a/entry.c b/entry.c
index 2101201..a599fc1 100644
--- a/entry.c
+++ b/entry.c
@@ -78,8 +78,13 @@ static void remove_subtree(struct strbuf *path)
 
 static int create_file(const char *path, unsigned int mode)
 {
+	int flags;
+	if (checkout_inplace)
+		flags = O_WRONLY | O_CREAT | O_TRUNC;
+	else
+		flags = O_WRONLY | O_CREAT | O_EXCL;
 	mode = (mode & 0100) ? 0777 : 0666;
-	return open(path, O_WRONLY | O_CREAT | O_EXCL, mode);
+	return open(path, flags, mode);
 }
 
 static void *read_blob_entry(const struct cache_entry *ce, unsigned long *size)
@@ -467,8 +472,15 @@ int checkout_entry(struct cache_entry *ce,
 			if (!state->force)
 				return error("%s is a directory", path.buf);
 			remove_subtree(&path);
-		} else if (unlink(path.buf))
-			return error_errno("unable to unlink old '%s'", path.buf);
+		} else if (checkout_inplace) {
+			if (!(st.st_mode & 0200) ||
+			    (trust_executable_bit && (st.st_mode & 0100) != (ce->ce_mode & 0100)))
+				if (chmod(path.buf, (ce->ce_mode & 0100) ? 0777 : 0666))
+					return error_errno(_("unable to change mode of '%s'"), path.buf);
+		} else {
+			if (unlink(path.buf))
+				return error_errno(_("unable to unlink old '%s'"), path.buf);
+		}
 	} else if (state->not_new)
 		return 0;
 
diff --git a/environment.c b/environment.c
index 2a6de23..5b91f30 100644
--- a/environment.c
+++ b/environment.c
@@ -68,6 +68,7 @@ char *notes_ref_name;
 int grafts_replace_parents = 1;
 int core_commit_graph;
 int core_apply_sparse_checkout;
+int checkout_inplace;
 int merge_log_config = -1;
 int precomposed_unicode = -1; /* see probe_utf8_pathname_composition() */
 unsigned long pack_size_limit_cfg;
diff --git a/t/t2031-checkout-inplace.sh b/t/t2031-checkout-inplace.sh
new file mode 100755
index 0000000..d70ecc4
--- /dev/null
+++ b/t/t2031-checkout-inplace.sh
@@ -0,0 +1,82 @@
+#!/bin/sh
+
+test_description='in-place checkout'
+. ./test-lib.sh
+
+test_expect_success 'setup' '
+
+	test_commit hello world &&
+	git branch other &&
+	test_commit hello-again world
+'
+
+test_expect_success 'in-place checkout overwrites open file' '
+
+	git config core.checkoutInPlace true &&
+	git checkout -f master &&
+	exec 8<world &&
+	git checkout other &&
+	exec 8<&- &&
+	echo hello >expect &&
+	test_cmp expect world
+'
+
+test_expect_success 'in-place checkout overwrites read-only file' '
+
+	git config core.checkoutInPlace true &&
+	git checkout -f master &&
+	chmod -w world &&
+	git checkout other &&
+	echo hello >expect &&
+	test_cmp expect world
+'
+
+test_expect_success 'in-place checkout updates executable permission' '
+
+	git config core.checkoutInPlace true &&
+	git checkout -f master^0 &&
+	test_chmod +x world &&
+	git commit -m executable &&
+	git checkout other &&
+	test ! -x world
+'
+
+test_expect_success POSIXPERM 'regular checkout respects umask' '
+
+	git config core.checkoutInPlace false &&
+	git checkout -f master &&
+	chmod 0660 world &&
+	umask 0022 &&
+	git checkout other &&
+	actual=$(ls -l world) &&
+	case "$actual" in
+	-rw-r--r--*)
+		: happy
+		;;
+	*)
+		echo Oops, world is not 0644 but $actual
+		false
+		;;
+	esac
+'
+
+test_expect_success POSIXPERM 'in-place checkout ignores umask' '
+
+	git config core.checkoutInPlace true &&
+	git checkout -f master &&
+	chmod 0660 world &&
+	umask 0022 &&
+	git checkout other &&
+	actual=$(ls -l world) &&
+	case "$actual" in
+	-rw-rw----*)
+		: happy
+		;;
+	*)
+		echo Oops, world is not 0660 but $actual
+		false
+		;;
+	esac
+'
+
+test_done
-- 
2.7.4

  parent reply	other threads:[~2018-06-11 20:40 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-06-10 19:44 [PATCH] checkout files in-place Clemens Buchacher
2018-06-11  2:04 ` brian m. carlson
2018-06-11 17:48   ` Clemens Buchacher
2018-06-11 18:02   ` Junio C Hamano
2018-06-11 20:22     ` Clemens Buchacher
2018-06-11 17:59 ` Ævar Arnfjörð Bjarmason
2018-06-11 20:35 ` Edward Thomson
2018-06-11 20:57   ` Clemens Buchacher
     [not found]     ` <CAGHpTBJFwToEwnk4P17AJ+z-55Nzc04OBbTvsbFRrkXJpfXAkQ@mail.gmail.com>
2018-06-12  8:51       ` Edward Thomson
2018-06-13  7:39         ` Orgad Shaneh
2018-06-11 20:39 ` Clemens Buchacher [this message]
2018-06-11 21:38   ` [PATCH v2] " Ævar Arnfjörð Bjarmason
2018-06-11 22:46     ` Junio C Hamano

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=20180611203958.GA1306@Sonnenschein.localdomain \
    --to=drizzd@gmx.net \
    --cc=avarab@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=orgads@gmail.com \
    --cc=pclouds@gmail.com \
    --cc=sandals@crustytoothpaste.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).