| Message ID | 20210612014501.890593-3-irogers@google.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Build and fuzzing related fixes | expand |
On Fri, 11 Jun 2021 18:45:01 -0700 Ian Rogers <irogers@google.com> wrote: > From: Claire Jensen <cjense@google.com> > > Signed-off-by: Claire Jensen <cjense@google.com> > Signed-off-by: Ian Rogers <irogers@google.com> > --- > src/event-utils.h | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/src/event-utils.h b/src/event-utils.h > index d377201..44f7968 100644 > --- a/src/event-utils.h > +++ b/src/event-utils.h > @@ -10,7 +10,7 @@ > #include <stdarg.h> > #include <stdbool.h> > > -#include <event-parse.h> > +#include "event-parse.h" Thanks for the update, but event-parse.h is part of the exported library and even though it happens to be local to this file, we use the '<' and '>' to denote that it is a library header and not a local one. -- Steve > > void tep_warning(const char *fmt, ...); > void tep_info(const char *fmt, ...);
On Sat, Jun 12, 2021 at 4:00 PM Steven Rostedt <rostedt@goodmis.org> wrote: > > On Fri, 11 Jun 2021 18:45:01 -0700 > Ian Rogers <irogers@google.com> wrote: > > > From: Claire Jensen <cjense@google.com> > > > > Signed-off-by: Claire Jensen <cjense@google.com> > > Signed-off-by: Ian Rogers <irogers@google.com> > > --- > > src/event-utils.h | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/src/event-utils.h b/src/event-utils.h > > index d377201..44f7968 100644 > > --- a/src/event-utils.h > > +++ b/src/event-utils.h > > @@ -10,7 +10,7 @@ > > #include <stdarg.h> > > #include <stdbool.h> > > > > -#include <event-parse.h> > > +#include "event-parse.h" > > Thanks for the update, but event-parse.h is part of the exported > library and even though it happens to be local to this file, we use the > '<' and '>' to denote that it is a library header and not a local one. > > -- Steve Would it be more conventional in this situation to use quotes within the library and angles outside? This change is within the library and not with code trying to link against it. Thanks, Ian > > > > > void tep_warning(const char *fmt, ...); > > void tep_info(const char *fmt, ...); >
On Sun, 13 Jun 2021 14:31:42 -0700 Ian Rogers <irogers@google.com> wrote: > Would it be more conventional in this situation to use quotes within > the library and angles outside? This change is within the library and > not with code trying to link against it. Does it matter? If not, what's the purpose of changing it? Note, the change log is empty, and is where rationale for the change is suppose to be added. -- Steve
On Sun, Jun 13, 2021 at 4:29 PM Steven Rostedt <rostedt@goodmis.org> wrote: > > On Sun, 13 Jun 2021 14:31:42 -0700 > Ian Rogers <irogers@google.com> wrote: > > > > Would it be more conventional in this situation to use quotes within > > the library and angles outside? This change is within the library and > > not with code trying to link against it. > > Does it matter? If not, what's the purpose of changing it? Sorry for not having this in the body, in the cover message I added to Claire's email with: > Some build systems are picky about angled vs quotes, fix this minor issue. The problem is with certain build system, we use bazel at Google, where iquote is used for includes within the library, and other flavors outside the library. There's a similar conversation here: https://groups.google.com/g/bazel-discuss/c/6MNuZ5bKoa8?pli=1 the failure you see when building is: In file included from src/event-parse.c:27: src/event-utils.h:13:10: error: 'event-parse.h' file not found with <angled> include; use "quotes" instead #include <event-parse.h> ^~~~~~~~~~~~~~~ "event-parse.h" Arguably -iquote is more correct within the library build, hence bazel using it. The libtraceevent Makefile is using -I, hence this not being a problem for others. We prefer not to carry internal patches, hence wanting to send this upstream. Thanks, Ian > Note, the change log is empty, and is where rationale for the change is > suppose to be added. > > -- Steve
On Mon, 14 Jun 2021 09:14:08 -0700 Ian Rogers <irogers@google.com> wrote: > On Sun, Jun 13, 2021 at 4:29 PM Steven Rostedt <rostedt@goodmis.org> wrote: > > > > On Sun, 13 Jun 2021 14:31:42 -0700 > > Ian Rogers <irogers@google.com> wrote: > > > > > > > Would it be more conventional in this situation to use quotes within > > > the library and angles outside? This change is within the library and > > > not with code trying to link against it. > > > > Does it matter? If not, what's the purpose of changing it? > > Sorry for not having this in the body, in the cover message I added to > Claire's email with: > > > Some build systems are picky about angled vs quotes, fix this minor issue. > > The problem is with certain build system, we use bazel at Google, > where iquote is used for includes within the library, and other > flavors outside the library. There's a similar conversation here: > https://groups.google.com/g/bazel-discuss/c/6MNuZ5bKoa8?pli=1 > > the failure you see when building is: > > In file included from src/event-parse.c:27: > src/event-utils.h:13:10: error: 'event-parse.h' file not found with > <angled> include; use "quotes" instead > #include <event-parse.h> > ^~~~~~~~~~~~~~~ > "event-parse.h" > > Arguably -iquote is more correct within the library build, hence bazel > using it. The libtraceevent Makefile is using -I, hence this not being > a problem for others. We prefer not to carry internal patches, hence > wanting to send this upstream. OK, context is very important :-) With the above explanation, it makes sense, and I'm OK with taking it. Please resend with the above in the commit log. Thanks! -- Steve
diff --git a/src/event-utils.h b/src/event-utils.h index d377201..44f7968 100644 --- a/src/event-utils.h +++ b/src/event-utils.h @@ -10,7 +10,7 @@ #include <stdarg.h> #include <stdbool.h> -#include <event-parse.h> +#include "event-parse.h" void tep_warning(const char *fmt, ...); void tep_info(const char *fmt, ...);