[Powertop] [PATCH v2 00/14] pull: various changes, mostly build system related

[Powertop] [PATCH v2 00/14] pull: various changes, mostly build system related

[Sami Kerola]

[-- Attachment #1: Type: text/plain, Size: 2079 bytes --]

Hello,

This change set is related to previous submission, that is discussed in
thread below:

https://lists.01.org/pipermail/powertop/2014-November/001664.html

My assumption is that these changes may not be completely in acceptable
to be part of upstream just before release.  See the link for details.

Sami Kerola (14):
  configure: require new enough automake version
  configure: use autoconf-archive ax_pthread.m4
  add missing space to --version print out
  csstoh.sh: check every single return value
  configure: use AC_USE_SYSTEM_EXTENSIONS to determine features
  powertop.m4: GCC_FORTIFY_SOURCE_CC copy idea of macro from Archlinux
    pacman
  configure: use autoconf-archive ax_cxx_compile_stdcxx_11.m4
  main: fix incompatible operand type
  move variables to protected scope
  make --quiet option to suppress various info messages
  squeeze all extra bits from size of binary and html report
  add timestamp to html report
  fix typo in message
  add missing include

 .gitignore                           |  39 +++-
 PowerTop.png                         | Bin 6975 -> 5542 bytes
 configure.ac                         |  21 ++-
 m4/ax_cxx_compile_stdcxx_11.m4       | 142 +++++++++++++++
 m4/ax_pthread.m4                     | 332 +++++++++++++++++++++++++++++++++++
 m4/gcc_fortify_source_cc.m4          |  29 +++
 src/Makefile.am                      |   1 -
 src/cpu/rapl/rapl_interface.cpp      |   7 +-
 src/cpu/rapl/rapl_interface.h        |   9 +-
 src/csstoh.sh                        |  40 +++--
 src/lib.h                            |   1 +
 src/main.cpp                         |   8 +-
 src/report/report-formatter-base.cpp |   2 -
 src/report/report-formatter-csv.cpp  |   2 -
 src/report/report-formatter-html.cpp | 248 +++++++++++++-------------
 src/report/report.cpp                |   5 +-
 16 files changed, 719 insertions(+), 167 deletions(-)
 create mode 100644 m4/ax_cxx_compile_stdcxx_11.m4
 create mode 100644 m4/ax_pthread.m4
 create mode 100644 m4/gcc_fortify_source_cc.m4

-- 
2.1.3

Re: [Powertop] [PATCH v2 00/14] pull: various changes, mostly build system related

[Alexandra Yates]

[-- Attachment #1: Type: text/plain, Size: 2666 bytes --]

Hi Sami,


> Hello,
>
> This change set is related to previous submission, that is discussed in
> thread below:
>
> https://lists.01.org/pipermail/powertop/2014-November/001664.html
>
> My assumption is that these changes may not be completely in acceptable
> to be part of upstream just before release.  See the link for details.
>
> Sami Kerola (14):
>   configure: require new enough automake version
>   configure: use autoconf-archive ax_pthread.m4
>   add missing space to --version print out
>   csstoh.sh: check every single return value
>   configure: use AC_USE_SYSTEM_EXTENSIONS to determine features
>   powertop.m4: GCC_FORTIFY_SOURCE_CC copy idea of macro from Archlinux
>     pacman
>   configure: use autoconf-archive ax_cxx_compile_stdcxx_11.m4
>   main: fix incompatible operand type
>   move variables to protected scope
>   make --quiet option to suppress various info messages
>   squeeze all extra bits from size of binary and html report
>   add timestamp to html report
>   fix typo in message
>   add missing include
>
>  .gitignore                           |  39 +++-
>  PowerTop.png                         | Bin 6975 -> 5542 bytes
>  configure.ac                         |  21 ++-
>  m4/ax_cxx_compile_stdcxx_11.m4       | 142 +++++++++++++++
>  m4/ax_pthread.m4                     | 332
> +++++++++++++++++++++++++++++++++++
>  m4/gcc_fortify_source_cc.m4          |  29 +++
>  src/Makefile.am                      |   1 -
>  src/cpu/rapl/rapl_interface.cpp      |   7 +-
>  src/cpu/rapl/rapl_interface.h        |   9 +-
>  src/csstoh.sh                        |  40 +++--
>  src/lib.h                            |   1 +
>  src/main.cpp                         |   8 +-
>  src/report/report-formatter-base.cpp |   2 -
>  src/report/report-formatter-csv.cpp  |   2 -
>  src/report/report-formatter-html.cpp | 248 +++++++++++++-------------
>  src/report/report.cpp                |   5 +-
>  16 files changed, 719 insertions(+), 167 deletions(-)
>  create mode 100644 m4/ax_cxx_compile_stdcxx_11.m4
>  create mode 100644 m4/ax_pthread.m4
>  create mode 100644 m4/gcc_fortify_source_cc.m4
>
> --
> 2.1.3
>
> _______________________________________________
> PowerTop mailing list
> PowerTop(a)lists.01.org
> https://lists.01.org/mailman/listinfo/powertop
>

Patches look good.  Two things:

1- I could not get this patch to apply
"configure: require new enough automake version"
Patch is empty.  Was it split wrong?

2- Please get the latest checkpatch.pl from the linux kernel scripts and
run it over your files before creating the patches.

Thank you,
Alexandra.

Re: [Powertop] [PATCH v2 00/14] pull: various changes, mostly build system related

[Sami Kerola]

[-- Attachment #1: Type: text/plain, Size: 1395 bytes --]

On 14 November 2014 04:30, Alexandra Yates
<alexandra.yates(a)linux.intel.com> wrote:

Hi Alexandra, and all,

> Patches look good.

Nice to hear.

> Two things:
>
> 1- I could not get this patch to apply
> "configure: require new enough automake version"
> Patch is empty.  Was it split wrong?

Empty? I don't understand, I can see the change in mail archive.

https://lists.01.org/pipermail/powertop/2014-November/001666.html

Instead of sending the same 14 patches without any changes to them
could you consider merging from my repo to avoid weird mail content
issue(s).

cd powertop
git remote add sami https://github.com/kerolasa/powertop.git
git fetch sami
git checkout master
git merge sami/various
git remote remove sami

If all OK git push to what ever you call upstream repo, often origin,
but there is no way I could be sure.

> 2- Please get the latest checkpatch.pl from the linux kernel scripts and
> run it over your files before creating the patches.

The script reported few of the following, but I don't think they are
meaningful in this case.

If any of these errors are false positives, please report
them to the maintainer, see CHECKPATCH in MAINTAINERS.
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?

The checkpatch.pl did not report any other errors.

-- 
Sami Kerola
http://www.iki.fi/kerolasa/

Re: [Powertop] [PATCH v2 00/14] pull: various changes, mostly build system related

[Alexandra Yates]

[-- Attachment #1: Type: text/plain, Size: 1602 bytes --]


> On 14 November 2014 04:30, Alexandra Yates
> <alexandra.yates(a)linux.intel.com> wrote:
>
> Hi Alexandra, and all,
>
>> Patches look good.
>
> Nice to hear.
>
>> Two things:
>>
>> 1- I could not get this patch to apply
>> "configure: require new enough automake version"
>> Patch is empty.  Was it split wrong?
>
> Empty? I don't understand, I can see the change in mail archive.
>
> https://lists.01.org/pipermail/powertop/2014-November/001666.html
>
> Instead of sending the same 14 patches without any changes to them
> could you consider merging from my repo to avoid weird mail content
> issue(s).
>
> cd powertop
> git remote add sami https://github.com/kerolasa/powertop.git
> git fetch sami
> git checkout master
> git merge sami/various
> git remote remove sami
>
> If all OK git push to what ever you call upstream repo, often origin,
> but there is no way I could be sure.
>
>> 2- Please get the latest checkpatch.pl from the linux kernel scripts and
>> run it over your files before creating the patches.
>
> The script reported few of the following, but I don't think they are
> meaningful in this case.
>
> If any of these errors are false positives, please report
> them to the maintainer, see CHECKPATCH in MAINTAINERS.
> WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
>
> The checkpatch.pl did not report any other errors.
>
> --
> Sami Kerola
> http://www.iki.fi/kerolasa/
>

Sami,

When I run checkpatch.pl I get white spaces and over 80 char line errors
on your patches.

Thank you,
Alexandra.

Re: [Powertop] [PATCH v2 00/14] pull: various changes, mostly build system related

[Sami Kerola]

[-- Attachment #1: Type: text/plain, Size: 707 bytes --]

On 17 November 2014 04:45, Alexandra Yates
<alexandra.yates(a)linux.intel.com> wrote:
> When I run checkpatch.pl I get white spaces and over 80 char line errors
> on your patches.

Hi Alexandra,

You are right, I missed in the change 0004 'csstoh.sh: check every
single return value' a white space error. That is now fixed in:

https://github.com/kerolasa/powertop/commit/6d7e331488db1e1428925579431242b8fb1c6007

80 char line errors are strange. I do not see them. Are we using the
same version of the checkpatch.pl?

$ git describe
v3.18-rc5
$ shasum scripts/checkpatch.pl
a13cad76323e71c97ba279838feef9730509fc59  scripts/checkpatch.pl

-- 
Sami Kerola
http://www.iki.fi/kerolasa/

Re: [Powertop] [PATCH v2 00/14] pull: various changes, mostly build system related

[Alexandra Yates]

[-- Attachment #1: Type: text/plain, Size: 881 bytes --]


> On 17 November 2014 04:45, Alexandra Yates
> <alexandra.yates(a)linux.intel.com> wrote:
>> When I run checkpatch.pl I get white spaces and over 80 char line errors
>> on your patches.
>
> Hi Alexandra,
>
> You are right, I missed in the change 0004 'csstoh.sh: check every
> single return value' a white space error. That is now fixed in:
>
> https://github.com/kerolasa/powertop/commit/6d7e331488db1e1428925579431242b8fb1c6007
>
> 80 char line errors are strange. I do not see them. Are we using the
> same version of the checkpatch.pl?
>
> $ git describe
> v3.18-rc5
> $ shasum scripts/checkpatch.pl
> a13cad76323e71c97ba279838feef9730509fc59  scripts/checkpatch.pl
>
> --
> Sami Kerola
> http://www.iki.fi/kerolasa/
>
I'm using the tip of the branch from

git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git

Thank you,
Alexandra.

Re: [Powertop] [PATCH v2 00/14] pull: various changes, mostly build system related

[Alexandra Yates]

[-- Attachment #1: Type: text/plain, Size: 1666 bytes --]


> On 18 November 2014 at 18:08, Alexandra Yates
> <alexandra.yates(a)linux.intel.com> wrote:
>>> <alexandra.yates(a)linux.intel.com> wrote:
>>>> When I run checkpatch.pl I get white spaces and over 80 char line
>>>> errors
>>>> on your patches.
>>>
>>> Hi Alexandra,
>>>
>>> You are right, I missed in the change 0004 'csstoh.sh: check every
>>> single return value' a white space error. That is now fixed in:
>>>
>>> https://github.com/kerolasa/powertop/commit/6d7e331488db1e1428925579431242b8fb1c6007
>>>
>>> 80 char line errors are strange. I do not see them. Are we using the
>>> same version of the checkpatch.pl?
>>>
>>> $ git describe
>>> v3.18-rc5
>>> $ shasum scripts/checkpatch.pl
>>> a13cad76323e71c97ba279838feef9730509fc59  scripts/checkpatch.pl
>>
>> I'm using the tip of the branch from
>>
>> git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
>
> Hii Alexandra,
>
> Congratulations on the new release.
>
> I suppose now after the release my patch set can be put back on track.
> Since I do not see the same errors as you could you run the
> checkpatch.pl against the 14 changes and send me the output. That
> would help me a lot.
>
> --
> Sami Kerola
> http://www.iki.fi/kerolasa/
>

 Hi Sami,

Here is the output from checkpatch.pl

   ERROR: trailing whitespace
   14 #139: FILE: src/csstoh.sh:37:
   15 +const char css[] = $

And The other error is that I can't get patch 0002 to commit.

I will be out of the office from tomorrow until Dec 10th visiting family 
in Ireland.

If we don't get these patches today we will as soon as I get back to work.

Thank you,
Alexandra.