DevTools component is for Chrome DevTools, reassigned to build for further triage.

This seems not to be only for bsdiff. Any C++ header like <base/time/time.h>, etc. are detected as c headers.

sorry, but i'm not seeing the problem here, and the bug report isn't exactly clear

i'm assuming you're talking about running `cros lint` on these .h files, and those are throwing errors ?  the only thing that does is execute cpplint.py from depot_tools, and cpplint.py expects .h files are C++ headers, not C headers.

so what is the trouble you're seeing ?  why do you think they're "being detected as C headers" ?  i'm not even sure what that means.

Ok. I should've been more clear.

For example when I run repo upload with a file change (e.g. https://chromium-review.googlesource.com/629638) It complains about c system headers being after c++ headers like this: 

Errors:
            * Hook script "../../../../chromite/bin/cros lint ${PRESUBMIT_FILES}" failed with code 1:
              /usr/local/google/home/ahassani/chromiumos/src/aosp/system/update_engine/real_system_state.cc:21:  Found C system header after C++ system header. Should be: real_system_state.h, c system, c++ system, other.  [build/include_order] [4]
              /usr/local/google/home/ahassani/chromiumos/src/aosp/system/update_engine/real_system_state.cc:22:  Found C system header after C++ system header. Should be: real_system_state.h, c system, c++ system, other.  [build/include_order] [4]
              /usr/local/google/home/ahassani/chromiumos/src/aosp/system/update_engine/real_system_state.cc:23:  Found C system header after C++ system header. Should be: real_system_state.h, c system, c++ system, other.  [build/include_order] [4]
              /usr/local/google/home/ahassani/chromiumos/src/aosp/system/update_engine/real_system_state.cc:24:  Found C system header after C++ system header. Should be: real_system_state.h, c system, c++ system, other.  [build/include_order] [4]
              /usr/local/google/home/ahassani/chromiumos/src/aosp/system/update_engine/real_system_state.cc:25:  Found C system header after C++ system header. Should be: real_system_state.h, c system, c++ system, other.  [build/include_order] [4]
              /usr/local/google/home/ahassani/chromiumos/src/aosp/system/update_engine/real_system_state.cc:26:  Found C system header after C++ system header. Should be: real_system_state.h, c system, c++ system, other.  [build/include_order] [4]
              Done processing /usr/local/google/home/ahassani/chromiumos/src/aosp/system/update_engine/real_system_state.cc
              Total errors found: 6
              13:39:17: ERROR: linter found errors in 1 files



for example is base/time/time.h considered a system header or a library header? It is definitely not a C header. It is c++ right? So why those errors? The Google C++ style guide (https://google.github.io/styleguide/cppguide.html#Names_and_Order_of_Includes) says: Related header, C library, C++ library, other libraries' .h, your project's .h.

Thanks :)

i don't see why CL:629638 would trigger any of that output.  it isn't touching any source files, so why is it linting them ?

but lets ignore that and just run `cros lint real_system_state.cc` by hand and get the same output.  in platform2 we turn off build/include_order and enforce build/include_alpha, and leave the rest to manual review.

if you want to dissect cpplint.py output in more detail, probably want to start a thread on chromium-dev@ as we just use it from Chromium unmodified.

i don't know the exact answer off hand, but this isn't CrOS specific.

I'm sorry, I gave the wrong url, here is the actual CL: https://chromium-review.googlesource.com/c/aosp/platform/system/update_engine/+/629856

But it doesn't matter, you know what I mean. I'll open up a chromium-dev thread.

Ok, as with the discussion in the chromium-dev, it seems that in CrOS we are using angle-brackets instead of quotes for library includes and cpplint assumes they are system c files. As said in #5, we can do the same thing for the update engine, but still we will get other lint errors because cpplint was never ON for UE. 

One of the major lint errors is the complain about wrong guard style. For example we use UPDATE_ENGINE_FOO_H_, but the linter is complaining it should be FOO_H_. CrOS uses the same style in UE, but I don't see header_guard is being disabled anywhere. 

So for now I think a solution would be to do whatever platform2/CPPLINT.cfg does + disabling header_guard and allow further fixes come up whenever someone sends a new CL. Does that sound reasonable? Is it possible to include platform2/CPPLINT.cfg in the update_engine CPPLINT.cfg?

Thanks,

That sounds fine to me. I think that platform2's CPPLINT.cfg is used from other places outside of platform2 (see e.g. https://goto.google.com/yubqz).

The following revision refers to this bug:
  https://chromium.googlesource.com/aosp/platform/system/update_engine/+/eb875805d8b897913c190ffb0bf87b275037ac2a

commit eb875805d8b897913c190ffb0bf87b275037ac2a
Author: Amin Hassani <ahassani@google.com>
Date: Thu Aug 24 04:17:25 2017

update_engine: Add CPPLINT.cfg

Adds CPPLINT.cfg which disables build/include_order and
build/include_guard and enables build/alpha_order.

BUG= chromium:753892 
TEST=repo upload --cbr .

Change-Id: I3be65e3be2670948de5e765d50c0a032ecbd5f81
Reviewed-on: https://chromium-review.googlesource.com/630478
Commit-Ready: Amin Hassani <ahassani@chromium.org>
Tested-by: Amin Hassani <ahassani@chromium.org>
Reviewed-by: Mike Frysinger <vapier@chromium.org>

[add] https://crrev.com/eb875805d8b897913c190ffb0bf87b275037ac2a/CPPLINT.cfg

The core cause of the problem still exist since in CrOS we use brackets include instead of double quotations, in the future we probably need to fix this!