[PATCH] Teach git-checkout-index to use file suffixes.

[PATCH] Teach git-checkout-index to use file suffixes.

[Shawn Pearce]

Sometimes it is useful to unpack the unmerged stage entries
to the same directory as the tracked file itself, but with
a suffix indicating which stage that version came from.
In many user interface level scripts this is being done
by git-unpack-file followed by creating the necessary
directory structure and then moving the file into the
directory with the requested name.  It is now possible to
perform the same action for a larger set of files directly
through git-checkout-index.

Signed-off-by: Shawn O. Pearce <spearce@spearce.org>

---
 I think this completes the two features I've found missing from
 git-checkout-index:  --stdin and --suffix.  These two options
 should make writing a working directory based merge strategy
 a little easier.

 FYI: I built this on top of my immediately prior patch ('Teach
 git-checkout-index to read filenames from stdin.') so this one
 may not apply cleanly without that patch being applied first.

 Documentation/git-checkout-index.txt |   29 ++++++++++++++++++++++++++++-
 apply.c                              |    2 ++
 cache.h                              |    2 ++
 checkout-index.c                     |   14 ++++++++++++--
 entry.c                              |   10 +++++++---
 read-tree.c                          |    1 +
 6 files changed, 52 insertions(+), 6 deletions(-)

base df23c1119d0af1fbac6b8afd296113e155d9a878
last e1674dc0b01de5c34fada13f7cf5fcbb3be82d09
diff --git a/Documentation/git-checkout-index.txt b/Documentation/git-checkout-index.txt
index b0b6588..f0be2a0 100644
--- a/Documentation/git-checkout-index.txt
+++ b/Documentation/git-checkout-index.txt
@@ -9,7 +9,8 @@ git-checkout-index - Copy files from the
 SYNOPSIS
 --------
 [verse]
-'git-checkout-index' [-u] [-q] [-a] [-f] [-n] [--prefix=<string>]
+'git-checkout-index' [-u] [-q] [-a] [-f] [-n]
+		   [--prefix=<string>] [--suffix=<string>]
 		   [--stage=<number>]
 		   [-z] [--stdin]
 		   [--] [<file>]\*
@@ -43,6 +44,10 @@ OPTIONS
 	When creating files, prepend <string> (usually a directory
 	including a trailing /)
 
+--suffix=<string>::
+	When creating files, append <string> to the name.  The value
+	of <string> must not contain a directory separator (/).
+
 --stage=<number>::
 	Instead of checking out unmerged entries, copy out the
 	files from named stage.  <number> must be between 1 and 3.
@@ -120,6 +125,28 @@ $ git-checkout-index --prefix=.merged- M
 This will check out the currently cached copy of `Makefile`
 into the file `.merged-Makefile`.
 
+Export files with a suffix::
++
+----------------
+$ git-checkout-index --suffix=\#2 --stage=2 Makefile
+----------------
++
+If `Makefile` is unmerged and has a stage 2 entry in the index
+this will check out that version into the file `Makefile#2`.
+
+A suffix may be preferred over a prefix when checking out all
+unmerged entries:
++
+----------------
+$ git-checkout-index --suffix=\#1 --stage=1 --all
+$ git-checkout-index --suffix=\#2 --stage=2 --all
+$ git-checkout-index --suffix=\#3 --stage=3 --all
+----------------
++
+would unpack all unmerged stages into the same directory as the
+tracked file.  (Compare with --prefix=.stage1/ which would have
+created a partial directory tree within `.stage1/`.)
+
 
 Author
 ------
diff --git a/apply.c b/apply.c
index 244718c..1ec8473 100644
--- a/apply.c
+++ b/apply.c
@@ -1307,6 +1307,8 @@ static int check_patch(struct patch *pat
 				/* checkout */
 				costate.base_dir = "";
 				costate.base_dir_len = 0;
+				costate.name_suffix = "";
+				costate.name_suffix_len = 0;
 				costate.force = 0;
 				costate.quiet = 0;
 				costate.not_new = 0;
diff --git a/cache.h b/cache.h
index 58eec00..055e213 100644
--- a/cache.h
+++ b/cache.h
@@ -254,6 +254,8 @@ extern const char *git_committer_info(in
 struct checkout {
 	const char *base_dir;
 	int base_dir_len;
+	const char *name_suffix;
+	int name_suffix_len;
 	unsigned force:1,
 		 quiet:1,
 		 not_new:1,
diff --git a/checkout-index.c b/checkout-index.c
index f54c606..af7b230 100644
--- a/checkout-index.c
+++ b/checkout-index.c
@@ -47,6 +47,8 @@ static int checkout_stage; /* default to
 static struct checkout state = {
 	.base_dir = "",
 	.base_dir_len = 0,
+	.name_suffix = "",
+	.name_suffix_len = 0,
 	.force = 0,
 	.quiet = 0,
 	.not_new = 0,
@@ -180,6 +182,14 @@ int main(int argc, char **argv)
 			state.base_dir_len = strlen(state.base_dir);
 			continue;
 		}
+		if (!strncmp(arg, "--suffix=", 9)) {
+			if (strchr(arg+9, '/')) {
+				die("--suffix cannot contain /");
+			}
+			state.name_suffix = arg+9;
+			state.name_suffix_len = strlen(state.name_suffix);
+			continue;
+		}
 		if (!strncmp(arg, "--stage=", 8)) {
 			int ch = arg[8];
 			if ('1' <= ch && ch <= '3')
@@ -193,8 +203,8 @@ int main(int argc, char **argv)
 		break;
 	}
 
-	if (state.base_dir_len) {
-		/* when --prefix is specified we do not
+	if (state.base_dir_len || state.name_suffix_len) {
+		/* when --prefix or --suffix is specified we do not
 		 * want to update cache.
 		 */
 		if (state.refresh_cache) {
diff --git a/entry.c b/entry.c
index 8fb99bc..dc35a07 100644
--- a/entry.c
+++ b/entry.c
@@ -117,10 +117,14 @@ int checkout_entry(struct cache_entry *c
 {
 	struct stat st;
 	static char path[MAXPATHLEN+1];
-	int len = state->base_dir_len;
+	int len1 = state->base_dir_len;
+	int len2 = strlen(ce->name);
+	int len3 = state->name_suffix_len;
+	char *path_len1 = path + len1;
 
-	memcpy(path, state->base_dir, len);
-	strcpy(path + len, ce->name);
+	memcpy(path, state->base_dir, len1);
+	memcpy(path_len1, ce->name, len2 + 1);
+	memcpy(path_len1 + len2, state->name_suffix, len3 + 1);
 
 	if (!lstat(path, &st)) {
 		unsigned changed = ce_match_stat(ce, &st, 1);
diff --git a/read-tree.c b/read-tree.c
index f39fe5c..f223a0d 100644
--- a/read-tree.c
+++ b/read-tree.c
@@ -281,6 +281,7 @@ static void check_updates(struct cache_e
 {
 	static struct checkout state = {
 		.base_dir = "",
+		.name_suffix = "",
 		.force = 1,
 		.quiet = 1,
 		.refresh_cache = 1,
-- 
1.2.3.gdf23c-dirty

Re: [PATCH] Teach git-checkout-index to use file suffixes.

[Shawn Pearce]

Shawn Pearce <spearce@spearce.org> wrote:
> Sometimes it is useful to unpack the unmerged stage entries
> to the same directory as the tracked file itself, but with
> a suffix indicating which stage that version came from.
> In many user interface level scripts this is being done
> by git-unpack-file followed by creating the necessary
> directory structure and then moving the file into the
> directory with the requested name.  It is now possible to
> perform the same action for a larger set of files directly
> through git-checkout-index.

Junio mentioned in his ``What's in git.git'' email that he's not
sure of this command line interface:

Junio C Hamano <junkio@cox.net> wrote:
> I am not sure about the command line interface of this.  Would
> it make more sense to checkout three stages in one pass?
> 
>     - checkout-index --suffix (Shawn Pearce)

I thought about the same thing myself when I submitted the patch
to the list.  I probably should have talked a little bit about that
in the email.  :-)

I thought about using instead:

  --stage=all --suffix1=\#1 --suffix2\=#2 --suffix3=\#3

but then thought that the performance gains achieved by only forking
git-checkout-index once, scanning the index once, etc. were not
that big of a difference compared to the rather horrible looking
command line syntax that produced and required one to use.

If anyone has any suggestions for these options, please pass them
along.  I'll rebuild the patch to pull all available stages if we
can come up with a suitable way of describing such.

-- 
Shawn.

Re: [PATCH] Teach git-checkout-index to use file suffixes.

[Mark Wooding]

Shawn Pearce <spearce@spearce.org> wrote:

> I thought about using instead:
>
>   --stage=all --suffix1=\#1 --suffix2\=#2 --suffix3=\#3

How about something like

  --suffixes=:#1:#2:#3

uses first character as a delimiter to separate the suffixes.  A single
--suffix option could plausibly provide the suffix if only one stage is
being checked out, and doesn't have the grim delimiter wart.

I suppose, though, that if this is going to be wrapped up in a script,
it doesn't really matter that much.

-- [mdw]

Re: [PATCH] Teach git-checkout-index to use file suffixes.

[Shawn Pearce]

Mark Wooding <mdw@distorted.org.uk> wrote:
> Shawn Pearce <spearce@spearce.org> wrote:
> 
> > I thought about using instead:
> >
> >   --stage=all --suffix1=\#1 --suffix2\=#2 --suffix3=\#3
> 
> How about something like
> 
>   --suffixes=:#1:#2:#3
> 
> uses first character as a delimiter to separate the suffixes.  A single
> --suffix option could plausibly provide the suffix if only one stage is
> being checked out, and doesn't have the grim delimiter wart.

Why let the user choose?  '/' shouldn't be permitted as that
(possibly) would try to create a directory also named the same as
the file (a bad thing).  Then its just two formats, depending on
if you want all stages or just 1 stage:

	--stage=all --suffix=#1/#2/#3

or

	--stage=1 --suffix=#1

?

-- 
Shawn.

Re: [PATCH] Teach git-checkout-index to use file suffixes.

[Junio C Hamano]

Shawn Pearce <spearce@spearce.org> writes:

> Why let the user choose?  '/' shouldn't be permitted as that
> (possibly) would try to create a directory also named the same as
> the file (a bad thing).  Then its just two formats, depending on
> if you want all stages or just 1 stage:
>
> 	--stage=all --suffix=#1/#2/#3
>
> or
>
> 	--stage=1 --suffix=#1

I'll later agree with "why let the user choose" in this message,
but for an entirely different reason.

I am not a great fan of extracting files from stages in the
working tree to begin with.  The git-unpack-file command tries
carefully to avoid conflicts, and its use of temporary files is
inherently, eh, temporary, and Porcelains know what names they
get and know how to clean things up afterwards.

I am sympathetic to what you are trying to achieve here.  Maybe
your Porcelain would invoke a graphical 3-way merge program in a
windowing environment, and the merge program displays the
filename(s) your Porcelain fed it to the end user, without
giving you a way to override it.  In that context, it is not
useful to the end user to show meaningless temporary filename,
and having checkout-index to derive the temporary filename from
real filename may look sensible.

However, --stage=all with --suffix would introduce name clashes
between repeated conflicted merge runs, which requires Porcelain
to be extra careful.  Your last merge run might have involved
three conflicting stages (leaving a.c~1, a.c~2, and a.c~3 in
your working tree) and this time it may be "we removed it while
they modified it" situation (needing to extract a.c~1, a.c~3 but
not a.c~2).  The Porcelain needs to make sure not to get
confused by leftover a.c~2 file in the working tree from the
previous run.

If what you are trying is to reduce the number of checkout-index
calls by your Porcelain to extract conflicted stages, it _might_
make more sense to do something like this instead (I am thinking
aloud, so take this with a big grain of salt -- it may not make
sense at all):

    checkout-index --stage=all checks out higher-order stages in
    made-up temporary filenames, just like git-merge-one-file
    does using git-unpack-file one-by-one, with a single
    invocation.

    It reports the following to its standard output, one record
    per pathname in the index:

	tmp1 <SP> tmp2 <SP> tmp3 <TAB> pathname <RS>

    Here, tmp? is the temporary file names for each stage, or a
    dot for for a nonexistent stage.  pathname is c-quoted as
    usual for non -z output and RS is '\n' (under -z flag,
    pathname is a literal string and RS is NUL).  SP = space
    (ASCII 0x20), TAB = tab (ASCII 0x09).

    Example:

        ".tmp1323 . .tmp1232\tfoo/a.c\n"

    Notice the dot between the two temporary files -- stage2 is
    empty in this example.

Then your Porcelain could run --stage=all and read the output,
and then moving the temporary files around whatever way pleases
it.  Since it needs to be careful about not overwriting the
existing working tree files *and* not getting confused by
existing garbage in the working tree anyway, I do not think it
would make your life much easier to have checkout-index derive
the temporary file names after the real working tree files.

BTW, using TAB to split metainformation and pathname and using
SP to separate metainformation pieces are in line with the
design other git tools use.  With this, you can separate the LHS
metainformation part and pathname using "cut", and you can have
shell IFS to split the metainformation part apart after that.
But that is a minor detail.

Re: [PATCH] Teach git-checkout-index to use file suffixes.

[Shawn Pearce]

Junio C Hamano <junkio@cox.net> wrote:
[...]
> However, --stage=all with --suffix would introduce name clashes
> between repeated conflicted merge runs, which requires Porcelain
> to be extra careful.  Your last merge run might have involved
> three conflicting stages (leaving a.c~1, a.c~2, and a.c~3 in
> your working tree) and this time it may be "we removed it while
> they modified it" situation (needing to extract a.c~1, a.c~3 but
> not a.c~2).  The Porcelain needs to make sure not to get
> confused by leftover a.c~2 file in the working tree from the
> previous run.

Clearly.  pg was trying to delete all of those files before doing its
merge work but failed because of the --ignored bug in git-ls-files;
but this is now fixed.  Still a possibility for confusion does exist.

But in at least one case my Porcelain is Eclipse and a Cygwin
prompt.  In this case I want to view and edit everything in Eclipse.
Having all of the files in the same directory just makes it easier
to view.  Random temporary names in the same directory as the tracked
file would be OK except if I had multiple conflicts in the same
directory, in which case I need some easy way to tell them apart.
At which point we're starting to derive off the tracked file name
and might as well always use well-known names.
 
> If what you are trying is to reduce the number of checkout-index
> calls by your Porcelain to extract conflicted stages, it _might_
> make more sense to do something like this instead (I am thinking
> aloud, so take this with a big grain of salt -- it may not make
> sense at all):
> 
>     checkout-index --stage=all checks out higher-order stages in
>     made-up temporary filenames, just like git-merge-one-file
>     does using git-unpack-file one-by-one, with a single
>     invocation.
> 
>     It reports the following to its standard output, one record
>     per pathname in the index:
> 
> 	tmp1 <SP> tmp2 <SP> tmp3 <TAB> pathname <RS>
[...]

That's not a bad idea.  The only thing I don't like about that
is that git-checkout-index won't build the directory tree for me;
the Porcelain must still be responsible for doing that before it
can rename the temporary files (if available) into the correct
subdirectory.

My plan with git-checkout-index though was originally to just have
it fail if the file already exists, unless -f is given.  So if
a left-over foo.c#2 was still on disk and git-checkout-index was
going to write to that name it would fail.


I see a lot of benefit from the checkout to temporary file names
and let the Porcelian rename (if it desires).

So I'm going to ask you to withdraw the --suffix patch from pu.
I'll write up a new patch using the ideas you suggest above and
submit that instead.

-- 
Shawn.