Teach git-checkout-index to use file suffixes.

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.

---
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

index b0b6588..f0be2a0 100644
@@ -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
 ------

index 244718c..1ec8473 100644
@@ -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;

index 58eec00..055e213 100644
@@ -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,

index f54c606..af7b230 100644
@@ -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) {

index 8fb99bc..dc35a07 100644
@@ -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);

index f39fe5c..f223a0d 100644
@@ -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

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

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.

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]

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.

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.

Junio C Hamano <[email protected]> wrote:
[...]

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.

[...]

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.