Make ident dynamic, not just a hardcoded value of "$Id".

From: Maksym Sobolyev <[email protected]>

This allows ident to be something like $FreeBSD$ so it provides matching functionality for repos migrated from CVS / SVN.

This works by allowing ident to have a parameter, i.e.:

* ident=MyCustomId

In .gitattributes.

---
Make ident dynamic, not just a hardcoded value of "$Id".

This allows ident to be something like $FreeBSD$ so it provides matching functionality for repos migrated from CVS / SVN.

This works by allowing ident to have a parameter, i.e.:

* ident=MyCustomId

In .gitattributes.

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1074%2Fsobomax%2Fpr-custom_ident-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1074/sobomax/pr-custom_ident-v1
Pull-Request: https://github.com/git/git/pull/1074

 builtin/checkout--worker.c              |  17 +++-
 convert.c                               | 114 +++++++++++++++---------
 convert.h                               |   7 +-
 parallel-checkout.c                     |  11 ++-
 parallel-checkout.h                     |   8 +-
 t/t2082-parallel-checkout-attributes.sh |   7 +-
 6 files changed, 108 insertions(+), 56 deletions(-)

index fb9fd13b73c..61ba39402ad 100644
@@ -9,7 +9,7 @@ static void packet_to_pc_item(const char *buffer, int len,
 			      struct parallel_checkout_item *pc_item)
 {
 	const struct pc_item_fixed_portion *fixed_portion;
-	const char *variant;
+	const char *variant, *ident_action;
 	char *encoding;
 
 	if (len < sizeof(struct pc_item_fixed_portion))

@@ -19,7 +19,8 @@ static void packet_to_pc_item(const char *buffer, int len,
 	fixed_portion = (struct pc_item_fixed_portion *)buffer;
 
 	if (len - sizeof(struct pc_item_fixed_portion) !=
-		fixed_portion->name_len + fixed_portion->working_tree_encoding_len)
+		fixed_portion->name_len + fixed_portion->working_tree_encoding_len +
+		fixed_portion->ident_action_len)
 		BUG("checkout worker received corrupted item");
 
 	variant = buffer + sizeof(struct pc_item_fixed_portion);

@@ -43,11 +44,21 @@ static void packet_to_pc_item(const char *buffer, int len,
 	pc_item->ce->ce_namelen = fixed_portion->name_len;
 	pc_item->ce->ce_mode = fixed_portion->ce_mode;
 	memcpy(pc_item->ce->name, variant, pc_item->ce->ce_namelen);
+	variant += pc_item->ce->ce_namelen;
 	oidcpy(&pc_item->ce->oid, &fixed_portion->oid);
 
+	if (fixed_portion->ident_action_len) {
+		ident_action = xmemdupz(variant,
+					fixed_portion->ident_action_len);
+		variant += fixed_portion->ident_action_len;
+	} else {
+		ident_action = NULL;
+	}
+
 	pc_item->id = fixed_portion->id;
 	pc_item->ca.crlf_action = fixed_portion->crlf_action;
-	pc_item->ca.ident = fixed_portion->ident;
+	pc_item->ca.ident_action.id = ident_action;
+	pc_item->ca.ident_action.id_len = fixed_portion->ident_action_len;
 	pc_item->ca.working_tree_encoding = encoding;
 }
 

index 5ee1c322058..a02632e8104 100644
@@ -76,11 +76,16 @@ enum convert_crlf_action {
 
 struct convert_driver;
 
+struct ident_action {
+	const char *id;
+	int id_len;
+};
+
 struct conv_attrs {
 	struct convert_driver *drv;
 	enum convert_crlf_action attr_action; /* What attr says */
 	enum convert_crlf_action crlf_action; /* When no attr is set, use core.autocrlf */
-	int ident;
+	struct ident_action ident_action; /* What ident says */
 	const char *working_tree_encoding; /* Supported encoding or default encoding if NULL */
 };
 

index ddc0ff3c064..f9a3f2ff25b 100644
@@ -403,13 +403,15 @@ static void send_one_item(int fd, struct parallel_checkout_item *pc_item)
 	size_t name_len = pc_item->ce->ce_namelen;
 	size_t working_tree_encoding_len = working_tree_encoding ?
 					   strlen(working_tree_encoding) : 0;
+	const char *ident_action_id = pc_item->ca.ident_action.id;
+	size_t ident_action_len = pc_item->ca.ident_action.id_len;
 
 	/*
 	 * Any changes in the calculation of the message size must also be made
 	 * in is_eligible_for_parallel_checkout().
 	 */
 	len_data = sizeof(struct pc_item_fixed_portion) + name_len +
-		   working_tree_encoding_len;
+		   working_tree_encoding_len + ident_action_len;
 
 	data = xmalloc(len_data);
 

@@ -417,7 +419,7 @@ static void send_one_item(int fd, struct parallel_checkout_item *pc_item)
 	fixed_portion->id = pc_item->id;
 	fixed_portion->ce_mode = pc_item->ce->ce_mode;
 	fixed_portion->crlf_action = pc_item->ca.crlf_action;
-	fixed_portion->ident = pc_item->ca.ident;
+	fixed_portion->ident_action_len = ident_action_len;
 	fixed_portion->name_len = name_len;
 	fixed_portion->working_tree_encoding_len = working_tree_encoding_len;
 	/*

@@ -434,6 +436,11 @@ static void send_one_item(int fd, struct parallel_checkout_item *pc_item)
 		variant += working_tree_encoding_len;
 	}
 	memcpy(variant, pc_item->ce->name, name_len);
+	variant += name_len;
+	if (ident_action_len) {
+		memcpy(variant, ident_action_id, ident_action_len);
+		variant += ident_action_len;
+	}
 
 	packet_write(fd, data, len_data);
 

index 80f539bcb77..c3c282f516b 100644
@@ -76,9 +76,9 @@ struct parallel_checkout_item {
 
 /*
  * The fixed-size portion of `struct parallel_checkout_item` that is sent to the
- * workers. Following this will be 2 strings: ca.working_tree_encoding and
- * ce.name; These are NOT null terminated, since we have the size in the fixed
- * portion.
+ * workers. Following this will be 3 strings: ca.working_tree_encoding, ca.name
+ * and ca.ident_action.id; These are NOT null terminated, since we have the size
+ * in the fixed portion.
  *
  * Note that not all fields of conv_attrs and cache_entry are passed, only the
  * ones that will be required by the workers to smudge and write the entry.

@@ -88,7 +88,7 @@ struct pc_item_fixed_portion {
 	struct object_id oid;
 	unsigned int ce_mode;
 	enum convert_crlf_action crlf_action;
-	int ident;
+	size_t ident_action_len;
 	size_t working_tree_encoding_len;
 	size_t name_len;
 };

index 25254579618..822957a8dc8 100755
@@ -20,16 +20,19 @@ test_expect_success 'parallel-checkout with ident' '
 	(
 		cd ident &&
 		echo "A ident" >.gitattributes &&
+		echo "C ident=MyCusomVeryLongAndWordyId" >>.gitattributes &&
 		echo "\$Id\$" >A &&
 		echo "\$Id\$" >B &&
+		echo "\$MyCusomVeryLongAndWordyId\$" >C &&
 		git add -A &&
 		git commit -m id &&
 
-		rm A B &&
+		rm A B C &&
 		test_checkout_workers 2 git reset --hard &&
 		hexsz=$(test_oid hexsz) &&
 		grep -E "\\\$Id: [0-9a-f]{$hexsz} \\\$" A &&
-		grep "\\\$Id\\\$" B
+		grep "\\\$Id\\\$" B &&
+		grep -E "\\\$MyCusomVeryLongAndWordyId: [0-9a-f]{$hexsz} \\\$" C
 	)
 '
 

base-commit: 225bc32a989d7a22fa6addafd4ce7dcd04675dbf

gitgitgadget

I would have expected that, just like cvs uses its configuration mechanism for LocalKeyword, this would belong to the .git/config file.

The attribute mechanism is inherently for stuff that you would want to specify per-path, so the "do we do keyword expansion for this file (yes/no)?" is a good match to be an attribute, and that question is what the original 'ident' attribute asks.

I do not think "what keyword do we want to expand as if it were $Id$?" is a question we want to be giving different answers per path, and it does not smell like a good match to be an attribute.

By allowing a potentially separate values for

*.txt ident=FreeBSD
*.cc ident=OpenBSD

this gives long rope to create a mess to the users.

I also think that the ident expansion is overrated, and have a feeling that we should discourage its use, not encourage it, but that is a separate story.

Thanks.

From: Maksym Sobolyev <[email protected]>

This allows ident to be something like $FreeBSD$ so it provides matching functionality for repos migrated from CVS / SVN.

This works by allowing ident to have a parameter, i.e.:

* ident=MyCustomId

In .gitattributes.

---
Make ident dynamic, not just a hardcoded value of "$Id".

This allows ident to be something like $FreeBSD$ so it provides matching functionality for repos migrated from CVS / SVN.

This works by allowing ident to have a parameter, i.e.:

* ident=MyCustomId

In .gitattributes.

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1074%2Fsobomax%2Fpr-custom_ident-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1074/sobomax/pr-custom_ident-v2
Pull-Request: https://github.com/git/git/pull/1074

Range-diff vs v1:

1: 52e0adde935 ! 1: 16713788a26 Make ident dynamic, not just a hardcoded value of "$Id".
@@ Commit message

+ ## Documentation/gitattributes.txt ##
+@@ Documentation/gitattributes.txt: sign $ upon checkout. Any byte sequence that begins with
+ $Id: and ends with $ in the worktree file is replaced
+ with $Id$ upon check-in.
+
++The ident attribute can also provide an optional value,
++which if supplied is going to be used for expansion instead of
++the string Id.
++
++------------------------
++*.[ch] ident=FreeBSD
++------------------------
++
+
+ filter
+ ^^^^^^^^
+
## builtin/checkout--worker.c ##
@@ builtin/checkout--worker.c: static void packet_to_pc_item(const char *buffer, int len,
struct parallel_checkout_item *pc_item)

 Documentation/gitattributes.txt         |   8 ++
 builtin/checkout--worker.c              |  17 +++-
 convert.c                               | 114 +++++++++++++++---------
 convert.h                               |   7 +-
 parallel-checkout.c                     |  11 ++-
 parallel-checkout.h                     |   8 +-
 t/t2082-parallel-checkout-attributes.sh |   7 +-
 7 files changed, 116 insertions(+), 56 deletions(-)

index 83fd4e19a41..9e486f3e8d3 100644
@@ -382,6 +382,14 @@ sign `$` upon checkout.  Any byte sequence that begins with
 `$Id:` and ends with `$` in the worktree file is replaced
 with `$Id$` upon check-in.
 
+The `ident` attribute can also provide an optional value,
+which if supplied is going to be used for expansion instead of
+the string `Id`.
+
+------------------------
+*.[ch]		ident=FreeBSD
+------------------------
+
 
 `filter`
 ^^^^^^^^

index fb9fd13b73c..61ba39402ad 100644
@@ -9,7 +9,7 @@ static void packet_to_pc_item(const char *buffer, int len,
 			      struct parallel_checkout_item *pc_item)
 {
 	const struct pc_item_fixed_portion *fixed_portion;
-	const char *variant;
+	const char *variant, *ident_action;
 	char *encoding;
 
 	if (len < sizeof(struct pc_item_fixed_portion))

@@ -19,7 +19,8 @@ static void packet_to_pc_item(const char *buffer, int len,
 	fixed_portion = (struct pc_item_fixed_portion *)buffer;
 
 	if (len - sizeof(struct pc_item_fixed_portion) !=
-		fixed_portion->name_len + fixed_portion->working_tree_encoding_len)
+		fixed_portion->name_len + fixed_portion->working_tree_encoding_len +
+		fixed_portion->ident_action_len)
 		BUG("checkout worker received corrupted item");
 
 	variant = buffer + sizeof(struct pc_item_fixed_portion);

@@ -43,11 +44,21 @@ static void packet_to_pc_item(const char *buffer, int len,
 	pc_item->ce->ce_namelen = fixed_portion->name_len;
 	pc_item->ce->ce_mode = fixed_portion->ce_mode;
 	memcpy(pc_item->ce->name, variant, pc_item->ce->ce_namelen);
+	variant += pc_item->ce->ce_namelen;
 	oidcpy(&pc_item->ce->oid, &fixed_portion->oid);
 
+	if (fixed_portion->ident_action_len) {
+		ident_action = xmemdupz(variant,
+					fixed_portion->ident_action_len);
+		variant += fixed_portion->ident_action_len;
+	} else {
+		ident_action = NULL;
+	}
+
 	pc_item->id = fixed_portion->id;
 	pc_item->ca.crlf_action = fixed_portion->crlf_action;
-	pc_item->ca.ident = fixed_portion->ident;
+	pc_item->ca.ident_action.id = ident_action;
+	pc_item->ca.ident_action.id_len = fixed_portion->ident_action_len;
 	pc_item->ca.working_tree_encoding = encoding;
 }
 

index 5ee1c322058..a02632e8104 100644
@@ -76,11 +76,16 @@ enum convert_crlf_action {
 
 struct convert_driver;
 
+struct ident_action {
+	const char *id;
+	int id_len;
+};
+
 struct conv_attrs {
 	struct convert_driver *drv;
 	enum convert_crlf_action attr_action; /* What attr says */
 	enum convert_crlf_action crlf_action; /* When no attr is set, use core.autocrlf */
-	int ident;
+	struct ident_action ident_action; /* What ident says */
 	const char *working_tree_encoding; /* Supported encoding or default encoding if NULL */
 };
 

index ddc0ff3c064..f9a3f2ff25b 100644
@@ -403,13 +403,15 @@ static void send_one_item(int fd, struct parallel_checkout_item *pc_item)
 	size_t name_len = pc_item->ce->ce_namelen;
 	size_t working_tree_encoding_len = working_tree_encoding ?
 					   strlen(working_tree_encoding) : 0;
+	const char *ident_action_id = pc_item->ca.ident_action.id;
+	size_t ident_action_len = pc_item->ca.ident_action.id_len;
 
 	/*
 	 * Any changes in the calculation of the message size must also be made
 	 * in is_eligible_for_parallel_checkout().
 	 */
 	len_data = sizeof(struct pc_item_fixed_portion) + name_len +
-		   working_tree_encoding_len;
+		   working_tree_encoding_len + ident_action_len;
 
 	data = xmalloc(len_data);
 

@@ -417,7 +419,7 @@ static void send_one_item(int fd, struct parallel_checkout_item *pc_item)
 	fixed_portion->id = pc_item->id;
 	fixed_portion->ce_mode = pc_item->ce->ce_mode;
 	fixed_portion->crlf_action = pc_item->ca.crlf_action;
-	fixed_portion->ident = pc_item->ca.ident;
+	fixed_portion->ident_action_len = ident_action_len;
 	fixed_portion->name_len = name_len;
 	fixed_portion->working_tree_encoding_len = working_tree_encoding_len;
 	/*

@@ -434,6 +436,11 @@ static void send_one_item(int fd, struct parallel_checkout_item *pc_item)
 		variant += working_tree_encoding_len;
 	}
 	memcpy(variant, pc_item->ce->name, name_len);
+	variant += name_len;
+	if (ident_action_len) {
+		memcpy(variant, ident_action_id, ident_action_len);
+		variant += ident_action_len;
+	}
 
 	packet_write(fd, data, len_data);
 

index 80f539bcb77..c3c282f516b 100644
@@ -76,9 +76,9 @@ struct parallel_checkout_item {
 
 /*
  * The fixed-size portion of `struct parallel_checkout_item` that is sent to the
- * workers. Following this will be 2 strings: ca.working_tree_encoding and
- * ce.name; These are NOT null terminated, since we have the size in the fixed
- * portion.
+ * workers. Following this will be 3 strings: ca.working_tree_encoding, ca.name
+ * and ca.ident_action.id; These are NOT null terminated, since we have the size
+ * in the fixed portion.
  *
  * Note that not all fields of conv_attrs and cache_entry are passed, only the
  * ones that will be required by the workers to smudge and write the entry.

@@ -88,7 +88,7 @@ struct pc_item_fixed_portion {
 	struct object_id oid;
 	unsigned int ce_mode;
 	enum convert_crlf_action crlf_action;
-	int ident;
+	size_t ident_action_len;
 	size_t working_tree_encoding_len;
 	size_t name_len;
 };

index 25254579618..822957a8dc8 100755
@@ -20,16 +20,19 @@ test_expect_success 'parallel-checkout with ident' '
 	(
 		cd ident &&
 		echo "A ident" >.gitattributes &&
+		echo "C ident=MyCusomVeryLongAndWordyId" >>.gitattributes &&
 		echo "\$Id\$" >A &&
 		echo "\$Id\$" >B &&
+		echo "\$MyCusomVeryLongAndWordyId\$" >C &&
 		git add -A &&
 		git commit -m id &&
 
-		rm A B &&
+		rm A B C &&
 		test_checkout_workers 2 git reset --hard &&
 		hexsz=$(test_oid hexsz) &&
 		grep -E "\\\$Id: [0-9a-f]{$hexsz} \\\$" A &&
-		grep "\\\$Id\\\$" B
+		grep "\\\$Id\\\$" B &&
+		grep -E "\\\$MyCusomVeryLongAndWordyId: [0-9a-f]{$hexsz} \\\$" C
 	)
 '
 

base-commit: 225bc32a989d7a22fa6addafd4ce7dcd04675dbf

gitgitgadget

Hi, Maksym

I haven't read the entire patch (and I don't normally use the ident feature), but I left a few comments below:

$ echo 'f	ident=$weird:$' >.gitattributes
$ echo '$$weird:$$' >f
$ git add .
$ git commit -m files
$ rm f
$ git checkout f
$ cat f

$ echo foo >> f
$ git add f
$ git cat-file -p :f

	packed_item_size = sizeof(struct pc_item_fixed_portion) + ce->ce_namelen +
		(ca->working_tree_encoding ? strlen(ca->working_tree_encoding) : 0);

@@ -6,5 +6,5 @@
 $customId: bebd07c752ffffd6779e1056db5de66c3bb733ed $
 $customId: bebd07c752ffffd6779e1056db5de66c3bb733ed $
 $customId: NoTerminatingSymbol
-$customId: Foreign Commit With Spaces $
+$customId: bebd07c752ffffd6779e1056db5de66c3bb733ed $
 $customId: NoTerminatingSymbolAtEOF
\ No newline at end of file

It is a valid question. I actually think these two bytes should be excluded from the custom ID string --- FWIW, I also do not think there is any practical problem if we limited the set of characters to [A-Za-z0-9] and nothing else.

If we are not restricting the characters that can be used in the custom ID placeholder, we probably should have tests that use allowed but unusual characters.

Thanks.

There is a test in t/t0021-conversion.sh that ensures that we recognise expanded and unexpanded keyword strings using various lines that has placeholder-lookalike on it, which starts like this:

# If an expanded ident ever gets into the repository, we want to make sure that # it is collapsed before being expanded again on checkout test_expect_success expanded_in_repo '
{
echo "File with expanded keywords"
echo "\$Id\$"
echo "\$Id:\$"
echo "\$Id: 0000000000000000000000000000000000000000 \$" echo "\$Id: NoSpaceAtEnd\$"
echo "\$Id:NoSpaceAtFront \$"
echo "\$Id:NoSpaceAtEitherEnd\$"
echo "\$Id: NoTerminatingSymbol"
echo "\$Id: Foreign Commit With Spaces \$"
} >expanded-keywords.0 &&
...

It would be good to have a copy of this test with the placeholder customized to something unusual like " Id:", "Id: $ $", ":", "$ $" etc. to make sure we can parse out the keyword correctly without imposing the letters that can be used in the placeholder.

    Side note.  I of course do not literally mean to "copy" and make
    the number of tests explode.  I think it can be done with a loop
    to test various custom strings, which may begin like ...

	for kwd in "Id" " Id:" "Id $" "Id: $ $" ":" "$ $"
	do
		cat >expanded-keywords.0 <<-EOF &&
		\$$kwd\$
		\$$kwd:\$
		\$$kwd: 0000000000000000000000000000000000000000 \$
		\$$kwd: NoSpaceAtEnd\$
		\$$kwd:NoSpaceAtFront \$
		\$$kwd:NoSpaceAtEitherEnd\$
		\$$kwd: NoTerminatingSymbol
		\$$kwd: Foreign Commit With Spaces \$
		EOF
		...

If we can do without restriction, that would be even better, but I do not think it is the end of the world if we found some corner case that a certain customized placeholder can make the syntax ambiguous and we cannot recognise expanded keywords in the working tree files, as it is reasonable to limit the letters we allow in placeholders.

One thing we should not do is to tell users that they can use anything as their customized Id string, and then later find such a corner case and tell them now we forbid certain letters. To avoid such a mistake, it is better to start reasonably tight, allowing what we know will never cause problems (like alphanumerics) and forbidding what we know will not be missed in real-world usecases (like whitespaces and punctuations). After we discover that certain things that we initially forbid have real uses in the future, we can loosen the restriction. "Start loose and then tighten" will lead to a regression when viewed from the end-user's workflow. "Start tight and then loosen" will not have that problem.

Just like cvs uses its configuration mechanism for LocalKeyword, this belongs to the .git/config file.

The attribute mechanism is inherently for stuff that you would want to specify per-path, so the "do we do keyword expansion for this file (yes/no)?" is a good match to be an attribute, and that question is what the original 'ident' attribute asks.

I do not think "what keyword do we want to expand as if it were $Id$?" is a question we want to be giving different answers per path, and it does not smell like a good match to be an attribute.

By allowing a potentially separate values for

*.txt ident=FreeBSD
*.cc ident=OpenBSD

this gives long rope to create a mess to the users. In addition, a project can easily attack and break its users by having .gitattributes with overly long ident defined, as it would cause the attribute code to trigger the die() call added by this patch.

In short, I do not think the approach taken by this patch to use per-path attribute is a good idea at all.

I suspect that this patch mimics crlf_action when naming the new "ident_action" thing, but that is a poor choice. crlf_action does specify what different actions to be taken, but the setting does not change the way 'ident' filter behaves in any fundamental way. It just changes the constant string used to locate and replace from $Id$ to something else. ident_keyword (as this is "keyword expansion" behaviour) may be a better name.

There is no inherent reason why the custom ident string must be shorter than 255, even though it might not be a bad idea to have a reasonable floor for the length.

Sounds good.

I do not think the "structure" should be relevant, not just its name but its presence. As I said already, the data (i.e. the name of the custom token that replaces $Id$) should not come from the attribute subsystem---it belongs to the config subsystem, I would think.

Thanks.

From: Maksym Sobolyev <[email protected]>

This allows ident to be something like $FreeBSD$ so it provides matching functionality for repos migrated from CVS / SVN.

This works by allowing ident to have a parameter, i.e.:

* ident=MyCustomId

In .gitattributes.

Extend the ident expansion tests to also verify custom ident.

---
Make ident dynamic, not just a hardcoded value of "$Id".

This allows ident to be something like $FreeBSD$ so it provides matching functionality for repos migrated from CVS / SVN.

This works by allowing ident to have a parameter, i.e.:

* ident=MyCustomId

In .gitattributes.

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1074%2Fsobomax%2Fpr-custom_ident-v3
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1074/sobomax/pr-custom_ident-v3
Pull-Request: https://github.com/git/git/pull/1074

Range-diff vs v2:

1: 16713788a26 ! 1: 74a705e5bbe Make ident dynamic, not just a hardcoded value of "$Id".

     @@ Commit message
      
          In .gitattributes.
      
     +    Extend the ident expansion tests to also verify custom ident.
     +
          Signed-off-by: Maksym Sobolyev <[email protected]>
      
       ## Documentation/gitattributes.txt ##
     @@ convert.c: static int read_convert_config(const char *var, const char *value, vo
       }
       
      -static int count_ident(const char *cp, unsigned long size)
     -+#define ID_STR "Id"
     -+
     -+#define GIT_MAX_IDENT_LEN 255
     ++#define ID_STR_DEFAULT "Id"
      +
      +static int count_ident(const char *cp, unsigned long size,
      +		       const struct ident_action *idact)
     @@ convert.c: static int ident_to_git(const char *src, size_t len,
       
      -		if (len > 3 && !memcmp(src, "Id:", 3)) {
      -			dollar = memchr(src + 3, '$', len - 3);
     -+		if (len > idact->id_len + 1 && !memcmp(src, idact->id, idact->id_len) && src[idact->id_len + 1] == ':') {
     ++		if (len > idact->id_len + 1 && !memcmp(src, idact->id, idact->id_len) && src[idact->id_len] == ':') {
      +			dollar = memchr(src + idact->id_len + 1, '$', len - (idact->id_len + 1));
       			if (!dollar)
       				break;
     @@ convert.c: static struct convert_driver *git_path_check_convert(struct attr_chec
      -	return !!ATTR_TRUE(value);
      +	if (!ATTR_UNSET(value) && !ATTR_FALSE(value)) {
      +		if (ATTR_TRUE(value))
     -+			idact.id = ID_STR;
     ++			idact.id = ID_STR_DEFAULT;
      +		else
      +			idact.id = value;
      +		idact.id_len = strlen(idact.id);
     -+		if (idact.id_len > GIT_MAX_IDENT_LEN)
     -+			die(_("ident value length exceeds GIT_MAX_IDENT_LEN"));
      +	}
      +	return idact;
       }
     @@ convert.c: struct ident_filter {
       	struct stream_filter filter;
       	struct strbuf left;
       	int state;
     --	char ident[GIT_MAX_HEXSZ + 5]; /* ": x40 $" */
      +	const struct ident_action *idact;
     -+	char ident[GIT_MAX_HEXSZ + GIT_MAX_IDENT_LEN + 3]; /* ": x40 $" */
     + 	char ident[GIT_MAX_HEXSZ + 5]; /* ": x40 $" */
       };
       
     - static int is_foreign_ident(const char *str)
     +-static int is_foreign_ident(const char *str)
     ++static int is_foreign_ident(const struct ident_action *idact, const char *str)
     + {
     + 	int i;
     + 
     +-	if (!skip_prefix(str, "$Id: ", &str))
     ++	if (str[0] != '$' || strlen(str) < idact->id_len + 3 ||
     ++	    memcmp(str + 1, idact->id, idact->id_len) != 0 ||
     ++	    !skip_prefix(str + 1 + idact->id_len, ": ", &str))
     + 		return 0;
     + 	for (i = 0; str[i]; i++) {
     + 		if (isspace(str[i]) && str[i+1] != '$')
      @@ convert.c: static int ident_filter_fn(struct stream_filter *filter,
       			   char *output, size_t *osize_p)
       {
     @@ convert.c: static int ident_filter_fn(struct stream_filter *filter,
       		case IDENT_SKIPPING:
       			/* fallthrough */
      @@ convert.c: static int ident_filter_fn(struct stream_filter *filter,
     + 			strbuf_addch(&ident->left, ch);
       			if (ch != '\n' && ch != '$')
       				continue;
     - 			if (ch == '$' && !is_foreign_ident(ident->left.buf)) {
     +-			if (ch == '$' && !is_foreign_ident(ident->left.buf)) {
      -				strbuf_setlen(&ident->left, sizeof(head) - 1);
     ++			if (ch == '$' && !is_foreign_ident(idact, ident->left.buf)) {
      +				strbuf_setlen(&ident->left, idact->id_len + 1);
       				strbuf_addstr(&ident->left, ident->ident);
       			}
     @@ convert.h: enum convert_crlf_action {
       
      +struct ident_action {
      +	const char *id;
     -+	int id_len;
     ++	size_t id_len;
      +};
      +
       struct conv_attrs {
     @@ convert.h: enum convert_crlf_action {
       
      
       ## parallel-checkout.c ##
     +@@ parallel-checkout.c: static int is_eligible_for_parallel_checkout(const struct cache_entry *ce,
     + 		return 0;
     + 
     + 	packed_item_size = sizeof(struct pc_item_fixed_portion) + ce->ce_namelen +
     +-		(ca->working_tree_encoding ? strlen(ca->working_tree_encoding) : 0);
     ++		(ca->working_tree_encoding ? strlen(ca->working_tree_encoding) : 0) +
     ++		ca->ident_action.id_len;
     + 
     + 	/*
     + 	 * The amount of data we send to the workers per checkout item is
      @@ parallel-checkout.c: static void send_one_item(int fd, struct parallel_checkout_item *pc_item)
       	size_t name_len = pc_item->ce->ce_namelen;
       	size_t working_tree_encoding_len = working_tree_encoding ?
     @@ parallel-checkout.h: struct pc_item_fixed_portion {
       	size_t name_len;
       };
      
     + ## t/t0021-conversion.sh ##
     +@@ t/t0021-conversion.sh: test_expect_success setup '
     + 	{
     + 	    echo "*.t filter=rot13"
     + 	    echo "*.i ident"
     ++	    echo "*.ci ident=customId"
     + 	} >.gitattributes &&
     + 
     + 	{
     + 	    echo a b c d e f g h i j k l m
     + 	    echo n o p q r s t u v w x y z
     + 	    echo '\''$Id$'\''
     ++	    echo '\''$customId$'\''
     + 	} >test &&
     + 	cat test >test.t &&
     + 	cat test >test.o &&
     + 	cat test >test.i &&
     +-	git add test test.t test.i &&
     +-	rm -f test test.t test.i &&
     +-	git checkout -- test test.t test.i &&
     ++	cat test >test.ci &&
     ++	git add test test.t test.i test.ci &&
     ++	rm -f test test.t test.i test.ci &&
     ++	git checkout -- test test.t test.i test.ci &&
     + 
     + 	echo "content-test2" >test2.o &&
     + 	echo "content-test3 - filename with special characters" >"test3 '\''sq'\'',\$x=.o"
     + '
     + 
     +-script='s/^\$Id: \([0-9a-f]*\) \$/\1/p'
     ++script_i='s/^\$Id: \([0-9a-f]*\) \$/\1/p'
     ++script_ci='s/^\$customId: \([0-9a-f]*\) \$/\1/p'
     + 
     + test_expect_success check '
     + 
     +@@ t/t0021-conversion.sh: test_expect_success check '
     + 	# ident should be stripped in the repository
     + 	git diff --raw --exit-code :test :test.i &&
     + 	id=$(git rev-parse --verify :test) &&
     +-	embedded=$(sed -ne "$script" test.i) &&
     ++	embedded=$(sed -ne "$script_i" test.i) &&
     ++	nembedded=$(sed -ne "$script_ci" test.i) &&
     + 	test "z$id" = "z$embedded" &&
     ++	test "z" = "z$nembedded" &&
     ++	embedded=$(sed -ne "$script_ci" test.ci) &&
     ++	nembedded=$(sed -ne "$script_i" test.ci) &&
     ++	test "z$id" = "z$embedded" &&
     ++	test "z" = "z$nembedded" &&
     + 
     + 	git cat-file blob :test.t >test.r &&
     + 
     +@@ t/t0021-conversion.sh: test_expect_success check '
     + 	test_cmp test.r test.t
     + '
     + 
     ++gen_expanded_keywords() {
     ++	local id="${1}"
     ++	echo "File with expanded keywords"
     ++	echo "\$$id\$"
     ++	echo "\$$id:\$"
     ++	echo "\$$id: 0000000000000000000000000000000000000000 \$"
     ++	echo "\$$id: NoSpaceAtEnd\$"
     ++	echo "\$$id:NoSpaceAtFront \$"
     ++	echo "\$$id:NoSpaceAtEitherEnd\$"
     ++	echo "\$$id: NoTerminatingSymbol"
     ++	echo "\$$id: Foreign Commit With Spaces \$"
     ++	printf "\$$id: NoTerminatingSymbolAtEOF"
     ++}
     ++
     ++gen_expected_output_0() {
     ++	local id="${1}"
     ++	local hid="${2}"
     ++	echo "File with expanded keywords"
     ++	echo "\$$id: $hid \$"
     ++	echo "\$$id: $hid \$"
     ++	echo "\$$id: $hid \$"
     ++	echo "\$$id: $hid \$"
     ++	echo "\$$id: $hid \$"
     ++	echo "\$$id: $hid \$"
     ++	echo "\$$id: NoTerminatingSymbol"
     ++	echo "\$$id: Foreign Commit With Spaces \$"
     ++}
     ++
     ++gen_expected_output() {
     ++	local id="${1}"
     ++	gen_expected_output_0 "${@}"
     ++	printf "\$$id: NoTerminatingSymbolAtEOF"
     ++}
     ++
     ++gen_expected_output_crlf() {
     ++	local id="${1}"
     ++	gen_expected_output_0 "${@}" | append_cr
     ++	printf "\$$id: NoTerminatingSymbolAtEOF"
     ++}
     ++
     + # If an expanded ident ever gets into the repository, we want to make sure that
     + # it is collapsed before being expanded again on checkout
     + test_expect_success expanded_in_repo '
     +-	{
     +-		echo "File with expanded keywords"
     +-		echo "\$Id\$"
     +-		echo "\$Id:\$"
     +-		echo "\$Id: 0000000000000000000000000000000000000000 \$"
     +-		echo "\$Id: NoSpaceAtEnd\$"
     +-		echo "\$Id:NoSpaceAtFront \$"
     +-		echo "\$Id:NoSpaceAtEitherEnd\$"
     +-		echo "\$Id: NoTerminatingSymbol"
     +-		echo "\$Id: Foreign Commit With Spaces \$"
     +-	} >expanded-keywords.0 &&
     +-
     +-	{
     +-		cat expanded-keywords.0 &&
     +-		printf "\$Id: NoTerminatingSymbolAtEOF"
     +-	} >expanded-keywords &&
     ++	gen_expanded_keywords Id >expanded-keywords &&
     ++	gen_expanded_keywords customId >expanded-keywords_ci &&
     + 	cat expanded-keywords >expanded-keywords-crlf &&
     ++	cat expanded-keywords_ci >expanded-keywords-crlf_ci &&
     + 	git add expanded-keywords expanded-keywords-crlf &&
     ++	git add expanded-keywords_ci expanded-keywords-crlf_ci &&
     + 	git commit -m "File with keywords expanded" &&
     + 	id=$(git rev-parse --verify :expanded-keywords) &&
     ++	id_ci=$(git rev-parse --verify :expanded-keywords_ci) &&
     + 
     +-	{
     +-		echo "File with expanded keywords"
     +-		echo "\$Id: $id \$"
     +-		echo "\$Id: $id \$"
     +-		echo "\$Id: $id \$"
     +-		echo "\$Id: $id \$"
     +-		echo "\$Id: $id \$"
     +-		echo "\$Id: $id \$"
     +-		echo "\$Id: NoTerminatingSymbol"
     +-		echo "\$Id: Foreign Commit With Spaces \$"
     +-	} >expected-output.0 &&
     +-	{
     +-		cat expected-output.0 &&
     +-		printf "\$Id: NoTerminatingSymbolAtEOF"
     +-	} >expected-output &&
     +-	{
     +-		append_cr <expected-output.0 &&
     +-		printf "\$Id: NoTerminatingSymbolAtEOF"
     +-	} >expected-output-crlf &&
     ++	gen_expected_output Id $id >expected-output &&
     ++	gen_expected_output customId $id_ci >expected-output_ci &&
     ++	gen_expected_output_crlf Id $id >expected-output-crlf &&
     ++	gen_expected_output_crlf customId $id_ci >expected-output-crlf_ci &&
     + 	{
     + 		echo "expanded-keywords ident"
     ++		echo "expanded-keywords_ci ident=customId"
     + 		echo "expanded-keywords-crlf ident text eol=crlf"
     ++		echo "expanded-keywords-crlf_ci ident=customId text eol=crlf"
     + 	} >>.gitattributes &&
     + 
     + 	rm -f expanded-keywords expanded-keywords-crlf &&
     ++	rm -f expanded-keywords_ci expanded-keywords-crlf_ci &&
     + 
     + 	git checkout -- expanded-keywords &&
     + 	test_cmp expected-output expanded-keywords &&
     + 
     + 	git checkout -- expanded-keywords-crlf &&
     +-	test_cmp expected-output-crlf expanded-keywords-crlf
     ++	test_cmp expected-output-crlf expanded-keywords-crlf &&
     ++
     ++	git checkout -- expanded-keywords_ci &&
     ++	test_cmp expected-output_ci expanded-keywords_ci &&
     ++
     ++	git checkout -- expanded-keywords-crlf_ci &&
     ++	test_cmp expected-output-crlf_ci expanded-keywords-crlf_ci
     + '
     + 
     + # The use of %f in a filter definition is expanded to the path to
     +
       ## t/t2082-parallel-checkout-attributes.sh ##
      @@ t/t2082-parallel-checkout-attributes.sh: test_expect_success 'parallel-checkout with ident' '
       	(

 Documentation/gitattributes.txt         |   8 ++
 builtin/checkout--worker.c              |  17 +++-
 convert.c                               | 116 ++++++++++++++----------
 convert.h                               |   7 +-
 parallel-checkout.c                     |  14 ++-
 parallel-checkout.h                     |   8 +-
 t/t0021-conversion.sh                   | 115 ++++++++++++++---------
 t/t2082-parallel-checkout-attributes.sh |   7 +-
 8 files changed, 192 insertions(+), 100 deletions(-)

index 83fd4e19a41..9e486f3e8d3 100644
@@ -382,6 +382,14 @@ sign `$` upon checkout.  Any byte sequence that begins with
 `$Id:` and ends with `$` in the worktree file is replaced
 with `$Id$` upon check-in.
 
+The `ident` attribute can also provide an optional value,
+which if supplied is going to be used for expansion instead of
+the string `Id`.
+
+------------------------
+*.[ch]		ident=FreeBSD
+------------------------
+
 
 `filter`
 ^^^^^^^^

index fb9fd13b73c..61ba39402ad 100644
@@ -9,7 +9,7 @@ static void packet_to_pc_item(const char *buffer, int len,
 			      struct parallel_checkout_item *pc_item)
 {
 	const struct pc_item_fixed_portion *fixed_portion;
-	const char *variant;
+	const char *variant, *ident_action;
 	char *encoding;
 
 	if (len < sizeof(struct pc_item_fixed_portion))

@@ -19,7 +19,8 @@ static void packet_to_pc_item(const char *buffer, int len,
 	fixed_portion = (struct pc_item_fixed_portion *)buffer;
 
 	if (len - sizeof(struct pc_item_fixed_portion) !=
-		fixed_portion->name_len + fixed_portion->working_tree_encoding_len)
+		fixed_portion->name_len + fixed_portion->working_tree_encoding_len +
+		fixed_portion->ident_action_len)
 		BUG("checkout worker received corrupted item");
 
 	variant = buffer + sizeof(struct pc_item_fixed_portion);

@@ -43,11 +44,21 @@ static void packet_to_pc_item(const char *buffer, int len,
 	pc_item->ce->ce_namelen = fixed_portion->name_len;
 	pc_item->ce->ce_mode = fixed_portion->ce_mode;
 	memcpy(pc_item->ce->name, variant, pc_item->ce->ce_namelen);
+	variant += pc_item->ce->ce_namelen;
 	oidcpy(&pc_item->ce->oid, &fixed_portion->oid);
 
+	if (fixed_portion->ident_action_len) {
+		ident_action = xmemdupz(variant,
+					fixed_portion->ident_action_len);
+		variant += fixed_portion->ident_action_len;
+	} else {
+		ident_action = NULL;
+	}
+
 	pc_item->id = fixed_portion->id;
 	pc_item->ca.crlf_action = fixed_portion->crlf_action;
-	pc_item->ca.ident = fixed_portion->ident;
+	pc_item->ca.ident_action.id = ident_action;
+	pc_item->ca.ident_action.id_len = fixed_portion->ident_action_len;
 	pc_item->ca.working_tree_encoding = encoding;
 }
 

index 5ee1c322058..2422e289784 100644
@@ -76,11 +76,16 @@ enum convert_crlf_action {
 
 struct convert_driver;
 
+struct ident_action {
+	const char *id;
+	size_t id_len;
+};
+
 struct conv_attrs {
 	struct convert_driver *drv;
 	enum convert_crlf_action attr_action; /* What attr says */
 	enum convert_crlf_action crlf_action; /* When no attr is set, use core.autocrlf */
-	int ident;
+	struct ident_action ident_action; /* What ident says */
 	const char *working_tree_encoding; /* Supported encoding or default encoding if NULL */
 };
 

index ddc0ff3c064..b5908c299af 100644
@@ -91,7 +91,8 @@ static int is_eligible_for_parallel_checkout(const struct cache_entry *ce,
 		return 0;
 
 	packed_item_size = sizeof(struct pc_item_fixed_portion) + ce->ce_namelen +
-		(ca->working_tree_encoding ? strlen(ca->working_tree_encoding) : 0);
+		(ca->working_tree_encoding ? strlen(ca->working_tree_encoding) : 0) +
+		ca->ident_action.id_len;
 
 	/*
 	 * The amount of data we send to the workers per checkout item is

@@ -403,13 +404,15 @@ static void send_one_item(int fd, struct parallel_checkout_item *pc_item)
 	size_t name_len = pc_item->ce->ce_namelen;
 	size_t working_tree_encoding_len = working_tree_encoding ?
 					   strlen(working_tree_encoding) : 0;
+	const char *ident_action_id = pc_item->ca.ident_action.id;
+	size_t ident_action_len = pc_item->ca.ident_action.id_len;
 
 	/*
 	 * Any changes in the calculation of the message size must also be made
 	 * in is_eligible_for_parallel_checkout().
 	 */
 	len_data = sizeof(struct pc_item_fixed_portion) + name_len +
-		   working_tree_encoding_len;
+		   working_tree_encoding_len + ident_action_len;
 
 	data = xmalloc(len_data);
 

@@ -417,7 +420,7 @@ static void send_one_item(int fd, struct parallel_checkout_item *pc_item)
 	fixed_portion->id = pc_item->id;
 	fixed_portion->ce_mode = pc_item->ce->ce_mode;
 	fixed_portion->crlf_action = pc_item->ca.crlf_action;
-	fixed_portion->ident = pc_item->ca.ident;
+	fixed_portion->ident_action_len = ident_action_len;
 	fixed_portion->name_len = name_len;
 	fixed_portion->working_tree_encoding_len = working_tree_encoding_len;
 	/*

@@ -434,6 +437,11 @@ static void send_one_item(int fd, struct parallel_checkout_item *pc_item)
 		variant += working_tree_encoding_len;
 	}
 	memcpy(variant, pc_item->ce->name, name_len);
+	variant += name_len;
+	if (ident_action_len) {
+		memcpy(variant, ident_action_id, ident_action_len);
+		variant += ident_action_len;
+	}
 
 	packet_write(fd, data, len_data);
 

index 80f539bcb77..c3c282f516b 100644
@@ -76,9 +76,9 @@ struct parallel_checkout_item {
 
 /*
  * The fixed-size portion of `struct parallel_checkout_item` that is sent to the
- * workers. Following this will be 2 strings: ca.working_tree_encoding and
- * ce.name; These are NOT null terminated, since we have the size in the fixed
- * portion.
+ * workers. Following this will be 3 strings: ca.working_tree_encoding, ca.name
+ * and ca.ident_action.id; These are NOT null terminated, since we have the size
+ * in the fixed portion.
  *
  * Note that not all fields of conv_attrs and cache_entry are passed, only the
  * ones that will be required by the workers to smudge and write the entry.

@@ -88,7 +88,7 @@ struct pc_item_fixed_portion {
 	struct object_id oid;
 	unsigned int ce_mode;
 	enum convert_crlf_action crlf_action;
-	int ident;
+	size_t ident_action_len;
 	size_t working_tree_encoding_len;
 	size_t name_len;
 };

index 25254579618..822957a8dc8 100755
@@ -20,16 +20,19 @@ test_expect_success 'parallel-checkout with ident' '
 	(
 		cd ident &&
 		echo "A ident" >.gitattributes &&
+		echo "C ident=MyCusomVeryLongAndWordyId" >>.gitattributes &&
 		echo "\$Id\$" >A &&
 		echo "\$Id\$" >B &&
+		echo "\$MyCusomVeryLongAndWordyId\$" >C &&
 		git add -A &&
 		git commit -m id &&
 
-		rm A B &&
+		rm A B C &&
 		test_checkout_workers 2 git reset --hard &&
 		hexsz=$(test_oid hexsz) &&
 		grep -E "\\\$Id: [0-9a-f]{$hexsz} \\\$" A &&
-		grep "\\\$Id\\\$" B
+		grep "\\\$Id\\\$" B &&
+		grep -E "\\\$MyCusomVeryLongAndWordyId: [0-9a-f]{$hexsz} \\\$" C
 	)
 '
 

base-commit: 225bc32a989d7a22fa6addafd4ce7dcd04675dbf

gitgitgadget