{"thread":{"id":"50203","subject":"[PATCH] userdiff: Add a builtin pattern for dts files","startedAt":"2019-01-11T21:51:31Z","lastAt":"2019-01-17T22:13:56Z","messageCount":7,"participants":["Stephen Boyd","Alban Gruin","Junio C Hamano","Rob Herring"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"366575","messageId":"20190111215127.57356-1-sboyd@kernel.org","threadId":"50203","inReplyTo":null,"subject":"[PATCH] userdiff: Add a builtin pattern for dts files","fromName":"Stephen Boyd","fromEmail":"sboyd@kernel.org","sentAt":"2019-01-11T21:51:27Z","receivedAt":"2019-01-11T21:51:31Z","isPatch":true,"sender":{"key":"sboyd@kernel.org","avatar":"https://avatars.githubusercontent.com/u/38832?v=4"},"body":"The Linux kernel receives many patches to the devicetree files each\nrelease. The hunk header for those patches typically show nothing,\nmaking it difficult to figure out what node is being modified without\napplying the patch or opening the file and seeking to the context. Let's\nadd a builtin 'dts' pattern to git so that users can get better diff\noutput on dts files when they use the diff=dts driver.\n\nThe regex has been constructed based on the spec at devicetree.org[1]\n\n[1] https://github.com/devicetree-org/devicetree-specification/releases/latest\n\nCc: Rob Herring \nSigned-off-by: Stephen Boyd \n---\n Documentation/gitattributes.txt | 2 ++\n t/t4018-diff-funcname.sh | 1 +\n t/t4018/dts-labels | 8 +++++++\n t/t4018/dts-node-unitless | 8 +++++++\n t/t4018/dts-nodes | 8 +++++++\n t/t4018/dts-reference | 8 +++++++\n t/t4034-diff-words.sh | 1 +\n t/t4034/dts/expect | 37 +++++++++++++++++++++++++++++++++\n t/t4034/dts/post | 32 ++++++++++++++++++++++++++++\n t/t4034/dts/pre | 32 ++++++++++++++++++++++++++++\n userdiff.c | 9 ++++++++\n 11 files changed, 146 insertions(+)\n create mode 100644 t/t4018/dts-labels\n create mode 100644 t/t4018/dts-node-unitless\n create mode 100644 t/t4018/dts-nodes\n create mode 100644 t/t4018/dts-reference\n create mode 100644 t/t4034/dts/expect\n create mode 100644 t/t4034/dts/post\n create mode 100644 t/t4034/dts/pre\n\ndiff --git a/Documentation/gitattributes.txt b/Documentation/gitattributes.txt\nindex b8392fc3300c..14e5784b962d 100644\n--- a/Documentation/gitattributes.txt\n+++ b/Documentation/gitattributes.txt\n@@ -805,6 +805,8 @@ patterns are available:\n \n - `css` suitable for cascading style sheets.\n \n+- `dts` suitable for devicetree (DTS) files.\n+\n - `fortran` suitable for source code in the Fortran language.\n \n - `fountain` suitable for Fountain documents.\ndiff --git a/t/t4018-diff-funcname.sh b/t/t4018-diff-funcname.sh\nindex 22f9f88f0afc..8acd04b206d4 100755\n--- a/t/t4018-diff-funcname.sh\n+++ b/t/t4018-diff-funcname.sh\n@@ -31,6 +31,7 @@ diffpatterns=\"\n \tcpp\n \tcsharp\n \tcss\n+\tdts\n \tfortran\n \tfountain\n \tgolang\ndiff --git a/t/t4018/dts-labels b/t/t4018/dts-labels\nnew file mode 100644\nindex 000000000000..27cd4921cfb6\n--- /dev/null\n+++ b/t/t4018/dts-labels\n@@ -0,0 +1,8 @@\n+/ {\n+\tlabel_1: node1@ff00 {\n+\t\tlabel2: RIGHT {\n+\t\t\tvendor,some-property;\n+\t\t\tChangeMe = <0x45-30>;\n+\t\t};\n+\t};\n+};\ndiff --git a/t/t4018/dts-node-unitless b/t/t4018/dts-node-unitless\nnew file mode 100644\nindex 000000000000..c5287d91416e\n--- /dev/null\n+++ b/t/t4018/dts-node-unitless\n@@ -0,0 +1,8 @@\n+/ {\n+\tlabel_1: node1 {\n+\t\tRIGHT {\n+\t\t\tprop-array = <1>, <4>;\n+\t\t\tChangeMe = <0xffeedd00>;\n+\t\t};\n+\t};\n+};\ndiff --git a/t/t4018/dts-nodes b/t/t4018/dts-nodes\nnew file mode 100644\nindex 000000000000..5a4334bb1645\n--- /dev/null\n+++ b/t/t4018/dts-nodes\n@@ -0,0 +1,8 @@\n+/ {\n+\tlabel_1: node1@ff00 {\n+\t\tRIGHT@deadf00,4000 {\n+\t\t\t#size-cells = <1>;\n+\t\t\tChangeMe = <0xffeedd00>;\n+\t\t};\n+\t};\n+};\ndiff --git a/t/t4018/dts-reference b/t/t4018/dts-reference\nnew file mode 100644\nindex 000000000000..f115d4291d25\n--- /dev/null\n+++ b/t/t4018/dts-reference\n@@ -0,0 +1,8 @@\n+&label_1 {\n+\tTEST = <455>;\n+};\n+\n+&RIGHT {\n+\tvendor,some-property;\n+\tChangeMe = <0x45-30>;\n+};\ndiff --git a/t/t4034-diff-words.sh b/t/t4034-diff-words.sh\nindex 912df91226f2..9a93c2a3e0dd 100755\n--- a/t/t4034-diff-words.sh\n+++ b/t/t4034-diff-words.sh\n@@ -303,6 +303,7 @@ test_language_driver bibtex\n test_language_driver cpp\n test_language_driver csharp\n test_language_driver css\n+test_language_driver dts\n test_language_driver fortran\n test_language_driver html\n test_language_driver java\ndiff --git a/t/t4034/dts/expect b/t/t4034/dts/expect\nnew file mode 100644\nindex 000000000000..ed6ad9c65f8b\n--- /dev/null\n+++ b/t/t4034/dts/expect\n@@ -0,0 +1,37 @@\n+diff --git a/pre b/post\n+index ce95e99..7803aee 100644\n+--- a/pre\n++++ b/post\n+@@ -1,32 +1,32 @@\n+/ {\n+\tthis_handleHANDLE_2: nodenew-node@f00eeda {\n+\t\tcompatible = \"mydevvendor,compat\";\n+\t\tstring-prop = startend: \"hello world!world?\" endstart: ;\n+\t\t#size-cells#address-cells = <0+00+40>;\n+\t\treg = <0xf000xeeda>;\n+\t\tprop = <(1)>;\n+\t\tprop = <(-1e10)>;\n+\t\tprop = <(!31)>;\n+\t\tprop = <(~31)>;\n+\t\tprop = <(31*42)>;\n+\t\tprop = <(31&42)>;\n+\t\tprop = <(31*42)>;\n+\t\tprop = <(31/42)>;\n+\t\tprop = <(31%42)>;\n+\t\tprop = <(3+41+2)>;\n+\t\tprop = <(3-41-2)>;\n+\t\tprop = /bits/ 6432 <(31<<42)>;\n+\t\tprop = <(31>>42)>;\n+\t\tprop = <(31&42)>;\n+\t\tprop = <(31^42)>;\n+\t\tprop = <(31|42)>;\n+\t\tprop = <(31&&42)>;\n+\t\tprop = <(31||42)>;\n+\t\tprop = <(4?51?2:63)>;\n+\t\tlist = <&this_handleHANDLE_2>, <0 0 0 01>;\n+\t};\n+\n+\t&phandlephandle2 {\n+\t\tpre-phandleprop_handle = <&this_handleHANDLE_2>;\n+\t};\n+};\ndiff --git a/t/t4034/dts/post b/t/t4034/dts/post\nnew file mode 100644\nindex 000000000000..7803aee28093\n--- /dev/null\n+++ b/t/t4034/dts/post\n@@ -0,0 +1,32 @@\n+/ {\n+\tHANDLE_2: new-node@eeda {\n+\t\tcompatible = \"vendor,compat\";\n+\t\tstring-prop = end: \"hello world?\" start: ;\n+\t\t#address-cells = <0+40>;\n+\t\treg = <0xeeda>;\n+\t\tprop = <(1)>;\n+\t\tprop = <(-1e10)>;\n+\t\tprop = <(!1)>;\n+\t\tprop = <(~1)>;\n+\t\tprop = <(1*2)>;\n+\t\tprop = <(1&2)>;\n+\t\tprop = <(1*2)>;\n+\t\tprop = <(1/2)>;\n+\t\tprop = <(1%2)>;\n+\t\tprop = <(1+2)>;\n+\t\tprop = <(1-2)>;\n+\t\tprop = /bits/ 32 <(1<<2)>;\n+\t\tprop = <(1>>2)>;\n+\t\tprop = <(1&2)>;\n+\t\tprop = <(1^2)>;\n+\t\tprop = <(1|2)>;\n+\t\tprop = <(1&&2)>;\n+\t\tprop = <(1||2)>;\n+\t\tprop = <(1?2:3)>;\n+\t\tlist = <&HANDLE_2>, <0 0 0 1>;\n+\t};\n+\n+\t&phandle2 {\n+\t\tprop_handle = <&HANDLE_2>;\n+\t};\n+};\ndiff --git a/t/t4034/dts/pre b/t/t4034/dts/pre\nnew file mode 100644\nindex 000000000000..ce95e993ec93\n--- /dev/null\n+++ b/t/t4034/dts/pre\n@@ -0,0 +1,32 @@\n+/ {\n+\tthis_handle: node@f00 {\n+\t\tcompatible = \"mydev\";\n+\t\tstring-prop = start: \"hello world!\" end: ;\n+\t\t#size-cells = <0+0>;\n+\t\treg = <0xf00>;\n+\t\tprop = <1>;\n+\t\tprop = <-1e10>;\n+\t\tprop = <(!3)>;\n+\t\tprop = <(~3)>;\n+\t\tprop = <(3*4)>;\n+\t\tprop = <(3&4)>;\n+\t\tprop = <(3*4)>;\n+\t\tprop = <(3/4)>;\n+\t\tprop = <(3%4)>;\n+\t\tprop = <(3+4)>;\n+\t\tprop = <(3-4)>;\n+\t\tprop = /bits/ 64 <(3<<4)>;\n+\t\tprop = <(3>>4)>;\n+\t\tprop = <(3&4)>;\n+\t\tprop = <(3^4)>;\n+\t\tprop = <(3|4)>;\n+\t\tprop = <(3&&4)>;\n+\t\tprop = <(3||4)>;\n+\t\tprop = <(4?5:6)>;\n+\t\tlist = <&this_handle>, <0 0 0 0>;\n+\t};\n+\n+\t&phandle {\n+\t\tpre-phandle = <&this_handle>;\n+\t};\n+};\ndiff --git a/userdiff.c b/userdiff.c\nindex 97007abe5b16..2bc964e11089 100644\n--- a/userdiff.c\n+++ b/userdiff.c\n@@ -23,6 +23,15 @@ IPATTERN(\"ada\",\n \t \"[a-zA-Z][a-zA-Z0-9_]*\"\n \t \"|[-+]?[0-9][0-9#_.aAbBcCdDeEfF]*([eE][+-]?[0-9_]+)?\"\n \t \"|=>|\\\\.\\\\.|\\\\*\\\\*|:=|/=|>=|<=|<<|>>|<>\"),\n+PATTERNS(\"dts\",\n+\t /* Node name (with optional label and unit address) */\n+\t \"^[ \\t]*((([a-zA-Z_][a-zA-Z0-9_]*: )?[a-zA-Z][a-zA-Z0-9,._+-]*(@[a-zA-Z0-9,._+-]+)?\"\n+\t /* Reference */\n+\t \"|&[a-zA-Z_][a-zA-Z0-9_]*[ \\t]*)[ \\t]*\\\\{)[ \\t]*$\",\n+\t /* -- */\n+\t /* Property names and math operators */\n+\t \"[a-zA-Z0-9,._+?#-]+\"\n+\t \"|[-+*/%&^|!~]\"),\n IPATTERN(\"fortran\",\n \t \"!^([C*]|[ \\t]*!)\\n\"\n \t \"!^[ \\t]*MODULE[ \\t]+PROCEDURE[ \\t]\\n\"\n-- \nSent by a computer through tubes\n\n"},{"id":"366639","messageId":"16335abe-5e7e-fd7a-25f4-373f94e176e1@gmail.com","threadId":"50203","inReplyTo":"20190111215127.57356-1-sboyd@kernel.org","subject":"Re: [PATCH] userdiff: Add a builtin pattern for dts files","fromName":"Alban Gruin","fromEmail":"alban.gruin@gmail.com","sentAt":"2019-01-13T21:26:21Z","receivedAt":"2019-01-13T21:26:31Z","isPatch":true,"sender":{"key":"alban.gruin@gmail.com","avatar":"https://avatars.githubusercontent.com/u/6310153?v=4"},"body":"Hi Stephen,\n\nthank you for your patch. I left a few comments below.\n\nLe 11/01/2019 à 22:51, Stephen Boyd a écrit :\n> The Linux kernel receives many patches to the devicetree files each\n> release. The hunk header for those patches typically show nothing,\n> making it difficult to figure out what node is being modified without\n> applying the patch or opening the file and seeking to the context. Let's\n> add a builtin 'dts' pattern to git so that users can get better diff\n> output on dts files when they use the diff=dts driver.\n> \n> The regex has been constructed based on the spec at devicetree.org[1]\n> \n> [1] https://github.com/devicetree-org/devicetree-specification/releases/latest\n> \n> Cc: Rob Herring \n> Signed-off-by: Stephen Boyd \n> ---\n> Documentation/gitattributes.txt | 2 ++\n> t/t4018-diff-funcname.sh | 1 +\n> t/t4018/dts-labels | 8 +++++++\n> t/t4018/dts-node-unitless | 8 +++++++\n> t/t4018/dts-nodes | 8 +++++++\n> t/t4018/dts-reference | 8 +++++++\n> t/t4034-diff-words.sh | 1 +\n> t/t4034/dts/expect | 37 +++++++++++++++++++++++++++++++++\n> t/t4034/dts/post | 32 ++++++++++++++++++++++++++++\n> t/t4034/dts/pre | 32 ++++++++++++++++++++++++++++\n> userdiff.c | 9 ++++++++\n> 11 files changed, 146 insertions(+)\n> create mode 100644 t/t4018/dts-labels\n> create mode 100644 t/t4018/dts-node-unitless\n> create mode 100644 t/t4018/dts-nodes\n> create mode 100644 t/t4018/dts-reference\n> create mode 100644 t/t4034/dts/expect\n> create mode 100644 t/t4034/dts/post\n> create mode 100644 t/t4034/dts/pre\n> \n> -%<-\n> diff --git a/userdiff.c b/userdiff.c\n> index 97007abe5b16..2bc964e11089 100644\n> --- a/userdiff.c\n> +++ b/userdiff.c\n> @@ -23,6 +23,15 @@ IPATTERN(\"ada\",\n> \t \"[a-zA-Z][a-zA-Z0-9_]*\"\n> \t \"|[-+]?[0-9][0-9#_.aAbBcCdDeEfF]*([eE][+-]?[0-9_]+)?\"\n> \t \"|=>|\\\\.\\\\.|\\\\*\\\\*|:=|/=|>=|<=|<<|>>|<>\"),\n> +PATTERNS(\"dts\",\n> +\t /* Node name (with optional label and unit address) */\n> +\t \"^[ \\t]*((([a-zA-Z_][a-zA-Z0-9_]*: )?[a-zA-Z][a-zA-Z0-9,._+-]*(@[a-zA-Z0-9,._+-]+)?\"\n\nFrom the spec, label and node names “shall be [between] 1 to 31\ncharacters in length”. It’s not enforced here, and I guess it’s not\nreally git’s job to check for this kind of rule. Others may disagree\nwith me, though.\n\nShould labels end with exactly one space after the colon, or can there\nbe more, or none at all?\n\n> +\t /* Reference */\n> +\t \"|&[a-zA-Z_][a-zA-Z0-9_]*[ \\t]*)[ \\t]*\\\\{)[ \\t]*$\",\n\nIt’s not specified in the spec, but these lines must end with a curly\nbrace? What if there is a comment after the curly brace?\n\nThis pattern does not match the root node, but I guess it’s not\nimportant as most of the interesting stuff in a dts is not directly in it.\n\n> +\t /* -- */\n> +\t /* Property names and math operators */\n> +\t \"[a-zA-Z0-9,._+?#-]+\"\n> +\t \"|[-+*/%&^|!~]\"),\n\nThere is a `%' operator here and in your tests, but it’s not mentioned\nin the spec if I’m not mistaken. Does it actually exists?\n\n> IPATTERN(\"fortran\",\n> \t \"!^([C*]|[ \\t]*!)\\n\"\n> \t \"!^[ \\t]*MODULE[ \\t]+PROCEDURE[ \\t]\\n\"\n> \nCheers,\nAlban\n\n"},{"id":"366664","messageId":"xmqq7ef7w0kg.fsf@gitster-ct.c.googlers.com","threadId":"50203","inReplyTo":"16335abe-5e7e-fd7a-25f4-373f94e176e1@gmail.com","subject":"Re: [PATCH] userdiff: Add a builtin pattern for dts files","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-01-14T18:34:07Z","receivedAt":"2019-01-14T18:34:12Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Alban Gruin writes:\n\n> thank you for your patch. I left a few comments below.\n>\n> Le 11/01/2019 à 22:51, Stephen Boyd a écrit:\n>> The Linux kernel receives many patches to the devicetree files each\n>> release. The hunk header for those patches typically show nothing,\n>> making it difficult to figure out what node is being modified without\n>> applying the patch or opening the file and seeking to the context. Let's\n>> add a builtin 'dts' pattern to git so that users can get better diff\n>> output on dts files when they use the diff=dts driver.\n\nA sort of meta-question.\n\nWhat is missing in the current git that prevents the folks involved\nin device-tree project from achieving what this patch tries to\naccomplish without having to wait the Git project to act on it? To\nput it another way, is it a symptom of a bad design that from time\nto time the Git project has to add built-in patterns?\n\nAbility to ship arbitrary piece of text that you would normally\nplace in .git/config is not exactly an answer to the above question,\nand will not happen as that has grave security implications.\n\nBut perhaps we can start accepting an in-tree config-like file whose\ncontents are limited to verified-safe settings\n(e.g. \"diff.*.xfuncname\" and nothing else), so that projects can\nship two files in-tree:\n\n - \".gitattributes\" that says \"*.dts diff=dts\"\n\n - \".gitpreferences\" that says \"[diff \"dts\"] xfuncname=...\" to\n define the pattern the patch under review adds.\n\nwithout waiting for the next release of Git to add one more built-in\npattern?\n\nAnything that defines executable (e.g. \"diff.*.command\") should\nnever be accepted as part of the in-tree config-like file (for two\nreasons: security and portability), but there should be some\n\"obviously safe\" subset of config settings that we can allow project\nto impose on its users, I hope.\n"},{"id":"366665","messageId":"154749087173.169631.13885160480779834976@swboyd.mtv.corp.google.com","threadId":"50203","inReplyTo":"16335abe-5e7e-fd7a-25f4-373f94e176e1@gmail.com","subject":"Re: [PATCH] userdiff: Add a builtin pattern for dts files","fromName":"Stephen Boyd","fromEmail":"sboyd@kernel.org","sentAt":"2019-01-14T18:34:31Z","receivedAt":"2019-01-14T18:34:35Z","isPatch":true,"sender":{"key":"sboyd@kernel.org","avatar":"https://avatars.githubusercontent.com/u/38832?v=4"},"body":"Quoting Alban Gruin (2019-01-13 13:26:21)\n> Hi Stephen,\n> \n> thank you for your patch. I left a few comments below.\n> \n> Le 11/01/2019 à 22:51, Stephen Boyd a écrit :\n> > diff --git a/userdiff.c b/userdiff.c\n> > index 97007abe5b16..2bc964e11089 100644\n> > --- a/userdiff.c\n> > +++ b/userdiff.c\n> > @@ -23,6 +23,15 @@ IPATTERN(\"ada\",\n> > \"[a-zA-Z][a-zA-Z0-9_]*\"\n> > \"|[-+]?[0-9][0-9#_.aAbBcCdDeEfF]*([eE][+-]?[0-9_]+)?\"\n> > \"|=>|\\\\.\\\\.|\\\\*\\\\*|:=|/=|>=|<=|<<|>>|<>\"),\n> > +PATTERNS(\"dts\",\n> > + /* Node name (with optional label and unit address) */\n> > + \"^[ \\t]*((([a-zA-Z_][a-zA-Z0-9_]*: )?[a-zA-Z][a-zA-Z0-9,._+-]*(@[a-zA-Z0-9,._+-]+)?\"\n> \n> From the spec, label and node names “shall be [between] 1 to 31\n> characters in length”. It’s not enforced here, and I guess it’s not\n> really git’s job to check for this kind of rule. Others may disagree\n> with me, though.\n> \n> Should labels end with exactly one space after the colon, or can there\n> be more, or none at all?\n\nThere can be any number of spaces after the colon. I can fix the regex\nhere to accept any amount of whitespace after the colon.\n\n> \n> > + /* Reference */\n> > + \"|&[a-zA-Z_][a-zA-Z0-9_]*[ \\t]*)[ \\t]*\\\\{)[ \\t]*$\",\n> \n> It’s not specified in the spec, but these lines must end with a curly\n> brace? \n\nThat isn't common but it is supported. I can change the regex to look\nfor a line that ends in '{' or something that isn't ';' with anything\nafter the node name?\n\n> What if there is a comment after the curly brace?\n\nThere can be a comment after the curly brace or before the curly brace.\nThe spec allows C style /* */ type comments, in addition to C++ style //\ncomments. I've never really seen that happen in practice though so it's\nnot very common. Grepping the linux sources shows two hits:\n\narch/arm/boot/dts/lpc3250-ea3250.dts:&ohci /* &usbd */ {\narch/arm/boot/dts/lpc3250-phy3250.dts:&ohci /* &usbd */ {\n\n> \n> This pattern does not match the root node, but I guess it’s not\n> important as most of the interesting stuff in a dts is not directly in it.\n\nAgreed.\n\n> \n> > + /* -- */\n> > + /* Property names and math operators */\n> > + \"[a-zA-Z0-9,._+?#-]+\"\n> > + \"|[-+*/%&^|!~]\"),\n> \n> There is a `%' operator here and in your tests, but it’s not mentioned\n> in the spec if I’m not mistaken. Does it actually exists?\n\nThe compiler doesn't seem to complain when it's used. I can send a patch\nto update the spec for this rather esoteric feature. I can also include\nmore tests and support for the boolean relational operators which also\nseem to be supported but probably never used.\n\n"},{"id":"367034","messageId":"e932c7cb-ebf6-1499-00fb-cf0f5e8d9844@gmail.com","threadId":"50203","inReplyTo":"xmqq7ef7w0kg.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH] userdiff: Add a builtin pattern for dts files","fromName":"Alban Gruin","fromEmail":"alban.gruin@gmail.com","sentAt":"2019-01-17T21:26:20Z","receivedAt":"2019-01-17T21:26:31Z","isPatch":true,"sender":{"key":"alban.gruin@gmail.com","avatar":"https://avatars.githubusercontent.com/u/6310153?v=4"},"body":"Hi Junio,\n\nLe 14/01/2019 à 19:34, Junio C Hamano a écrit :\n> Alban Gruin writes:\n> \n>> thank you for your patch. I left a few comments below.\n>>\n>> Le 11/01/2019 à 22:51, Stephen Boyd a écrit:\n>>> The Linux kernel receives many patches to the devicetree files each\n>>> release. The hunk header for those patches typically show nothing,\n>>> making it difficult to figure out what node is being modified without\n>>> applying the patch or opening the file and seeking to the context. Let's\n>>> add a builtin 'dts' pattern to git so that users can get better diff\n>>> output on dts files when they use the diff=dts driver.\n> \n> A sort of meta-question.\n> \n> What is missing in the current git that prevents the folks involved\n> in device-tree project from achieving what this patch tries to\n> accomplish without having to wait the Git project to act on it? To\n> put it another way, is it a symptom of a bad design that from time\n> to time the Git project has to add built-in patterns?\n> \n> Ability to ship arbitrary piece of text that you would normally\n> place in .git/config is not exactly an answer to the above question,\n> and will not happen as that has grave security implications.\n> \n> But perhaps we can start accepting an in-tree config-like file whose\n> contents are limited to verified-safe settings\n> (e.g. \"diff.*.xfuncname\" and nothing else), so that projects can\n> ship two files in-tree:\n> \n> - \".gitattributes\" that says \"*.dts diff=dts\"\n> \n> - \".gitpreferences\" that says \"[diff \"dts\"] xfuncname=...\" to\n> define the pattern the patch under review adds.\n> \n> without waiting for the next release of Git to add one more built-in\n> pattern?\n> \n> Anything that defines executable (e.g. \"diff.*.command\") should\n> never be accepted as part of the in-tree config-like file (for two\n> reasons: security and portability), but there should be some\n> \"obviously safe\" subset of config settings that we can allow project\n> to impose on its users, I hope.\n> \n\nI really don’t know what to think about this. I like your proposal, but\nit will take some time to write such a feature, while there is a patch\nalmost ready to support the dts syntax. But I guess that if it is\nmerged, it will be nearly-impossible to remove from the source if a\nfeature like you proposed is eventually implemented.\n\n"},{"id":"367035","messageId":"0dc48713-7a17-623f-af75-67cca2cd63b6@gmail.com","threadId":"50203","inReplyTo":"154749087173.169631.13885160480779834976@swboyd.mtv.corp.google.com","subject":"Re: [PATCH] userdiff: Add a builtin pattern for dts files","fromName":"Alban Gruin","fromEmail":"alban.gruin@gmail.com","sentAt":"2019-01-17T21:26:30Z","receivedAt":"2019-01-17T21:26:36Z","isPatch":true,"sender":{"key":"alban.gruin@gmail.com","avatar":"https://avatars.githubusercontent.com/u/6310153?v=4"},"body":"Hi, sorry for the late answer.\n\nLe 14/01/2019 à 19:34, Stephen Boyd a écrit :\n> Quoting Alban Gruin (2019-01-13 13:26:21)\n>> Hi Stephen,\n>>\n>> thank you for your patch. I left a few comments below.\n>>\n>> Le 11/01/2019 à 22:51, Stephen Boyd a écrit :\n>>> diff --git a/userdiff.c b/userdiff.c\n>>> index 97007abe5b16..2bc964e11089 100644\n>>> --- a/userdiff.c\n>>> +++ b/userdiff.c\n>>> @@ -23,6 +23,15 @@ IPATTERN(\"ada\",\n>>> \"[a-zA-Z][a-zA-Z0-9_]*\"\n>>> \"|[-+]?[0-9][0-9#_.aAbBcCdDeEfF]*([eE][+-]?[0-9_]+)?\"\n>>> \"|=>|\\\\.\\\\.|\\\\*\\\\*|:=|/=|>=|<=|<<|>>|<>\"),\n>>> +PATTERNS(\"dts\",\n>>> + /* Node name (with optional label and unit address) */\n>>> + \"^[ \\t]*((([a-zA-Z_][a-zA-Z0-9_]*: )?[a-zA-Z][a-zA-Z0-9,._+-]*(@[a-zA-Z0-9,._+-]+)?\"\n>>\n>> From the spec, label and node names “shall be [between] 1 to 31\n>> characters in length”. It’s not enforced here, and I guess it’s not\n>> really git’s job to check for this kind of rule. Others may disagree\n>> with me, though.\n>>\n>> Should labels end with exactly one space after the colon, or can there\n>> be more, or none at all?\n> \n> There can be any number of spaces after the colon. I can fix the regex\n> here to accept any amount of whitespace after the colon.\n> \n>>\n>>> + /* Reference */\n>>> + \"|&[a-zA-Z_][a-zA-Z0-9_]*[ \\t]*)[ \\t]*\\\\{)[ \\t]*$\",\n>>\n>> It’s not specified in the spec, but these lines must end with a curly\n>> brace? \n> \n> That isn't common but it is supported. I can change the regex to look\n> for a line that ends in '{' or something that isn't ';' with anything\n> after the node name?\n> \n>> What if there is a comment after the curly brace?\n> \n> There can be a comment after the curly brace or before the curly brace.\n> The spec allows C style /* */ type comments, in addition to C++ style //\n> comments. I've never really seen that happen in practice though so it's\n> not very common. Grepping the linux sources shows two hits:\n> \n> arch/arm/boot/dts/lpc3250-ea3250.dts:&ohci /* &usbd */ {\n> arch/arm/boot/dts/lpc3250-phy3250.dts:&ohci /* &usbd */ {\n> \n\nI grepped through Linux and uboot’s sources and it seems that “it is not\ncommon” is actually “it does not exists in the wild”. Perhaps it’s not\nworth to support them.\n\n>>\n>>> + /* -- */\n>>> + /* Property names and math operators */\n>>> + \"[a-zA-Z0-9,._+?#-]+\"\n>>> + \"|[-+*/%&^|!~]\"),\n>>\n>> There is a `%' operator here and in your tests, but it’s not mentioned\n>> in the spec if I’m not mistaken. Does it actually exists?\n> \n> The compiler doesn't seem to complain when it's used. I can send a patch\n> to update the spec for this rather esoteric feature. I can also include\n> more tests and support for the boolean relational operators which also\n> seem to be supported but probably never used.\n> \n\nI’d like you to do this, yes.\n\nTo be fair, I don’t know what to think about this patch after Junio’s\nmessage.\n\n"},{"id":"367037","messageId":"CAL_JsqLKQDfK=_M=od2_G6Y4mbdkMn3hLvL+BuSZBEOZKB--pQ@mail.gmail.com","threadId":"50203","inReplyTo":"16335abe-5e7e-fd7a-25f4-373f94e176e1@gmail.com","subject":"Re: [PATCH] userdiff: Add a builtin pattern for dts files","fromName":"Rob Herring","fromEmail":"robh+dt@kernel.org","sentAt":"2019-01-17T22:13:41Z","receivedAt":"2019-01-17T22:13:56Z","isPatch":true,"sender":{"key":"robh+dt@kernel.org","avatar":null},"body":"On Sun, Jan 13, 2019 at 3:26 PM Alban Gruin wrote:\n>\n> Hi Stephen,\n>\n> thank you for your patch. I left a few comments below.\n>\n> Le 11/01/2019 à 22:51, Stephen Boyd a écrit :\n> > The Linux kernel receives many patches to the devicetree files each\n> > release. The hunk header for those patches typically show nothing,\n> > making it difficult to figure out what node is being modified without\n> > applying the patch or opening the file and seeking to the context. Let's\n> > add a builtin 'dts' pattern to git so that users can get better diff\n> > output on dts files when they use the diff=dts driver.\n> >\n> > The regex has been constructed based on the spec at devicetree.org[1]\n> >\n> > [1] https://github.com/devicetree-org/devicetree-specification/releases/latest\n> >\n> > Cc: Rob Herring \n> > Signed-off-by: Stephen Boyd \n> > ---\n> > Documentation/gitattributes.txt | 2 ++\n> > t/t4018-diff-funcname.sh | 1 +\n> > t/t4018/dts-labels | 8 +++++++\n> > t/t4018/dts-node-unitless | 8 +++++++\n> > t/t4018/dts-nodes | 8 +++++++\n> > t/t4018/dts-reference | 8 +++++++\n> > t/t4034-diff-words.sh | 1 +\n> > t/t4034/dts/expect | 37 +++++++++++++++++++++++++++++++++\n> > t/t4034/dts/post | 32 ++++++++++++++++++++++++++++\n> > t/t4034/dts/pre | 32 ++++++++++++++++++++++++++++\n> > userdiff.c | 9 ++++++++\n> > 11 files changed, 146 insertions(+)\n> > create mode 100644 t/t4018/dts-labels\n> > create mode 100644 t/t4018/dts-node-unitless\n> > create mode 100644 t/t4018/dts-nodes\n> > create mode 100644 t/t4018/dts-reference\n> > create mode 100644 t/t4034/dts/expect\n> > create mode 100644 t/t4034/dts/post\n> > create mode 100644 t/t4034/dts/pre\n> >\n> > -%<-\n> > diff --git a/userdiff.c b/userdiff.c\n> > index 97007abe5b16..2bc964e11089 100644\n> > --- a/userdiff.c\n> > +++ b/userdiff.c\n> > @@ -23,6 +23,15 @@ IPATTERN(\"ada\",\n> > \"[a-zA-Z][a-zA-Z0-9_]*\"\n> > \"|[-+]?[0-9][0-9#_.aAbBcCdDeEfF]*([eE][+-]?[0-9_]+)?\"\n> > \"|=>|\\\\.\\\\.|\\\\*\\\\*|:=|/=|>=|<=|<<|>>|<>\"),\n> > +PATTERNS(\"dts\",\n> > + /* Node name (with optional label and unit address) */\n> > + \"^[ \\t]*((([a-zA-Z_][a-zA-Z0-9_]*: )?[a-zA-Z][a-zA-Z0-9,._+-]*(@[a-zA-Z0-9,._+-]+)?\"\n>\n> From the spec, label and node names “shall be [between] 1 to 31\n> characters in length”. It’s not enforced here, and I guess it’s not\n> really git’s job to check for this kind of rule. Others may disagree\n> with me, though.\n\nThe spec does say 31 characters, but that's never been enforced. So,\nof course, there are occurrences in the wild (though maybe they were\njust property names, not node names). In any case, we plan to change\nthe spec to increase the size. To what, I don't know.\n\nRob\n"}]}