Re: [PATCH] checkout files in-place

From: brian m. carlson <sandals@crustytoothpaste.net>
Date: Jun 11, 2018, 02:04 UTC
Message-ID: <20180611020411.GE38834@genre.crustytoothpaste.net>
In-Reply-To: <20180610194444.GA1913@Sonnenschein.localdomain>

On Sun, Jun 10, 2018 at 09:44:45PM +0200, Clemens Buchacher wrote:

Show 9 quoted lines

> diff --git a/Documentation/config.txt b/Documentation/config.txt
> index b6cb997164..17af0fe163 100644
> --- a/Documentation/config.txt
> +++ b/Documentation/config.txt
> @@ -923,6 +923,14 @@ core.sparseCheckout::
>  	Enable "sparse checkout" feature. See section "Sparse checkout" in
>  	linkgit:git-read-tree[1] for more information.
>  
> +core.checkoutInplace::

Perhaps "core.checkoutInPlace" (captialized "place")?

> +	Checkout file contents in-place. By default Git checkout removes existing

"Check out".

> +	work tree files before it replaces them with different contents. If this
> +	option is enabled Git will overwrite the contents of existing files
> +	in-place. This is useful on systems where open file handles to a removed

Here and above, uou don't need to hyphenate "in place" when used as an adverb, only when using it as an adjective before the noun (e.g. "in-place checkout").

> +	file prevent creating new files at the same path. Note that Git will not
> +	update read/write permissions according to umask.

I'm wondering if it's worth a mention that running out of disk space (or quota) will cause data to be truncated.

Show 12 quoted lines

>  static void *read_blob_entry(const struct cache_entry *ce, unsigned long *size)
> @@ -470,8 +475,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);

So in-place checkout won't work if the mode changes and we're not the owner of the file. One place where I could see people wanting to use this on Unix is shared repositories with BSD group semantics, but that wouldn't work reliably.

I don't see that as a problem, as that isn't the issue this patch is trying to solve, but it may end up biting people.

-- 
brian m. carlson: Houston, Texas, US
OpenPGP: https://keybase.io/bk2204