2018-01-05 13:05:31

by Alexandre Belloni

[permalink] [raw]
Subject: Re: [V4, 01/11] Documentation: Add license-rules.rst to describe how to properly identify file licenses

Hi,

I'm definitively late to the party but...

On 17/11/2017 at 11:00:33 +0100, Thomas Gleixner wrote:
> +2. Style:
> +
> + The SPDX license identifier is added in form of a comment. The comment
> + style depends on the file type::
> +
> + C source: // SPDX-License-Identifier: <SPDX License Expression>
> + C header: /* SPDX-License-Identifier: <SPDX License Expression> */
> + ASM: /* SPDX-License-Identifier: <SPDX License Expression> */
> + scripts: # SPDX-License-Identifier: <SPDX License Expression>
> + .rst: .. SPDX-License-Identifier: <SPDX License Expression>
> + .dts{i}: // SPDX-License-Identifier: <SPDX License Expression>

dtc doesn't handle // comments. This works in the kernel tree because
dts files are preprocessed by CPP. But this doesn't work when using dtc
directly (most likely when compiling DT overlays). So, the choice is
between making dtc handle // comments or changing the documentation.

I don't have an opinion and Rob doesn't seem to care but I think we need
to do something now instead of letting each maintainer have to handle
this issue.

> + If a specific tool cannot handle the standard comment style, then the
> + appropriate comment mechanism which the tool accepts shall be used. This
> + is the reason for having the "/\* \*/" style comment in C header
> + files. There was build breakage observed with generated .lds files where
> + 'ld' failed to parse the C++ comment. This has been fixed by now, but
> + there are still older assembler tools which cannot handle C++ style
> + comments.
> +
--
Alexandre Belloni, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com


2018-01-05 18:56:00

by Russell King (Oracle)

[permalink] [raw]
Subject: Re: [V4, 01/11] Documentation: Add license-rules.rst to describe how to properly identify file licenses

On Fri, Jan 05, 2018 at 02:05:26PM +0100, Alexandre Belloni wrote:
> Hi,
>
> I'm definitively late to the party but...
>
> On 17/11/2017 at 11:00:33 +0100, Thomas Gleixner wrote:
> > +2. Style:
> > +
> > + The SPDX license identifier is added in form of a comment. The comment
> > + style depends on the file type::
> > +
> > + C source: // SPDX-License-Identifier: <SPDX License Expression>
> > + C header: /* SPDX-License-Identifier: <SPDX License Expression> */
> > + ASM: /* SPDX-License-Identifier: <SPDX License Expression> */
> > + scripts: # SPDX-License-Identifier: <SPDX License Expression>
> > + .rst: .. SPDX-License-Identifier: <SPDX License Expression>
> > + .dts{i}: // SPDX-License-Identifier: <SPDX License Expression>
>
> dtc doesn't handle // comments. This works in the kernel tree because
> dts files are preprocessed by CPP. But this doesn't work when using dtc
> directly (most likely when compiling DT overlays). So, the choice is
> between making dtc handle // comments or changing the documentation.

Does all the CPP macros that we use for things like GPIOs work when you
avoid CPP?

What I'm saying is that preprocessing the DTS in the kernel tree with
CPP appears to be a necessity, and doing so will deal with the C++
comments.

--
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 8.8Mbps down 630kbps up
According to speedtest.net: 8.21Mbps down 510kbps up

2018-01-05 22:48:10

by Frank Rowand

[permalink] [raw]
Subject: Re: [V4, 01/11] Documentation: Add license-rules.rst to describe how to properly identify file licenses

On 01/05/18 05:05, Alexandre Belloni wrote:
> Hi,
>
> I'm definitively late to the party but...
>
> On 17/11/2017 at 11:00:33 +0100, Thomas Gleixner wrote:
>> +2. Style:
>> +
>> + The SPDX license identifier is added in form of a comment. The comment
>> + style depends on the file type::
>> +
>> + C source: // SPDX-License-Identifier: <SPDX License Expression>
>> + C header: /* SPDX-License-Identifier: <SPDX License Expression> */
>> + ASM: /* SPDX-License-Identifier: <SPDX License Expression> */
>> + scripts: # SPDX-License-Identifier: <SPDX License Expression>
>> + .rst: .. SPDX-License-Identifier: <SPDX License Expression>
>> + .dts{i}: // SPDX-License-Identifier: <SPDX License Expression>
>
> dtc doesn't handle // comments. This works in the kernel tree because

dtc does handle // comments.


$ cat test_comment.dts
/dts-v1/;

// this is comment 1

/* this is comment 2 */

/ {
#address-cells = < 1 >;
#size-cells = < 1 >;

// this is comment 3

tree_1: soc@0 {
reg = <0x0 0x0>;
};

};


Using the dtc version in linux v4.14:

$ dtc -v
Version: DTC 1.4.4-g756ffc4f

$ dtc -O dts test_comment.dts
/dts-v1/;

/ {
#address-cells = <0x1>;
#size-cells = <0x1>;

tree_1: soc@0 {
reg = <0x0 0x0>;
};
};


Using the latest dtc version in the git_dtc repo:

$ dtc -v
Version: DTC 1.4.6-ge5438801

$ dtc -O dts test_comment.dts
/dts-v1/;

/ {
#address-cells = <0x1>;
#size-cells = <0x1>;

tree_1: soc@0 {
reg = <0x0 0x0>;
};
};

-Frank


> dts files are preprocessed by CPP. But this doesn't work when using dtc
> directly (most likely when compiling DT overlays). So, the choice is
> between making dtc handle // comments or changing the documentation.
>
> I don't have an opinion and Rob doesn't seem to care but I think we need
> to do something now instead of letting each maintainer have to handle
> this issue.
>
>> + If a specific tool cannot handle the standard comment style, then the
>> + appropriate comment mechanism which the tool accepts shall be used. This
>> + is the reason for having the "/\* \*/" style comment in C header
>> + files. There was build breakage observed with generated .lds files where
>> + 'ld' failed to parse the C++ comment. This has been fixed by now, but
>> + there are still older assembler tools which cannot handle C++ style
>> + comments.
>> +

2018-01-05 23:05:24

by Alexandre Belloni

[permalink] [raw]
Subject: Re: [V4, 01/11] Documentation: Add license-rules.rst to describe how to properly identify file licenses

On 05/01/2018 at 14:48:04 -0800, Frank Rowand wrote:
> On 01/05/18 05:05, Alexandre Belloni wrote:
> > Hi,
> >
> > I'm definitively late to the party but...
> >
> > On 17/11/2017 at 11:00:33 +0100, Thomas Gleixner wrote:
> >> +2. Style:
> >> +
> >> + The SPDX license identifier is added in form of a comment. The comment
> >> + style depends on the file type::
> >> +
> >> + C source: // SPDX-License-Identifier: <SPDX License Expression>
> >> + C header: /* SPDX-License-Identifier: <SPDX License Expression> */
> >> + ASM: /* SPDX-License-Identifier: <SPDX License Expression> */
> >> + scripts: # SPDX-License-Identifier: <SPDX License Expression>
> >> + .rst: .. SPDX-License-Identifier: <SPDX License Expression>
> >> + .dts{i}: // SPDX-License-Identifier: <SPDX License Expression>
> >
> > dtc doesn't handle // comments. This works in the kernel tree because
>
> dtc does handle // comments.
>

Hum, correct. I got a report saying it didn't and when I only checked
the COMMENT line of dtc-lexer.l. For whatever reason, I missed the
following line handling c++ comments.

I'll get back to the original report, trying to find why it was not
compiling.

--
Alexandre Belloni, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com