| Message ID | 20210922185702.4328-1-carenas@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2] Makefile: avoid breaking compilation database generation with DEVELOPER | expand |
Hi Carlo, Le 2021-09-22 à 14:57, Carlo Marcelo Arenas Belón a écrit : > 3821c38068 (Makefile: add support for generating JSON compilation > database, 2020-09-03), adds a feature to be used with clang to generate > a compilation database by copying most of what was done before with the > header dependency, but by doing so includes on its availability check > the CFLAGS which became specially problematic once DEVELOPER=1 implied > -pedantic as pointed out by Ævar[1]. > > Remove the unnecessary flags in the availability test, so it will work > regardless of which other warnings are enabled or if the compiler has > been told to error on them. > > [1] https://lore.kernel.org/git/patch-1.1-6b2e9af5e67-20210922T103749Z-avarab@gmail.com/ > > Helped-by: Eric Sunshine <sunshine@sunshineco.com> > Signed-off-by: Carlo Marcelo Arenas Belón <carenas@gmail.com> > --- > Makefile | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/Makefile b/Makefile > index 9df565f27b..d5c6d0ea3b 100644 > --- a/Makefile > +++ b/Makefile > @@ -1302,7 +1302,7 @@ GENERATE_COMPILATION_DATABASE = no > endif > > ifeq ($(GENERATE_COMPILATION_DATABASE),yes) > -compdb_check = $(shell $(CC) $(ALL_CFLAGS) \ > +compdb_check = $(shell $(CC) \ > -c -MJ /dev/null \ > -x c /dev/null -o /dev/null 2>&1; \ > echo $$?) Thanks for cleaning that up. Acked-by: Philippe Blain <levraiphilippeblain@gmail.com> Philippe.
On 2021-09-22 at 18:57:02, Carlo Marcelo Arenas Belón wrote: > 3821c38068 (Makefile: add support for generating JSON compilation > database, 2020-09-03), adds a feature to be used with clang to generate > a compilation database by copying most of what was done before with the > header dependency, but by doing so includes on its availability check > the CFLAGS which became specially problematic once DEVELOPER=1 implied > -pedantic as pointed out by Ævar[1]. > > Remove the unnecessary flags in the availability test, so it will work > regardless of which other warnings are enabled or if the compiler has > been told to error on them. > > [1] https://lore.kernel.org/git/patch-1.1-6b2e9af5e67-20210922T103749Z-avarab@gmail.com/ > > Helped-by: Eric Sunshine <sunshine@sunshineco.com> > Signed-off-by: Carlo Marcelo Arenas Belón <carenas@gmail.com> > --- > Makefile | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/Makefile b/Makefile > index 9df565f27b..d5c6d0ea3b 100644 > --- a/Makefile > +++ b/Makefile > @@ -1302,7 +1302,7 @@ GENERATE_COMPILATION_DATABASE = no > endif > > ifeq ($(GENERATE_COMPILATION_DATABASE),yes) > -compdb_check = $(shell $(CC) $(ALL_CFLAGS) \ > +compdb_check = $(shell $(CC) \ > -c -MJ /dev/null \ > -x c /dev/null -o /dev/null 2>&1; \ > echo $$?) Are you sure this results in a functional set of files? As I understand it, the reason that clangd needs these files is because it needs to know what include arguments and headers are supposed to be used, since C programs don't have a standard layout. In this case, it looks like you're removing all of the -I arguments, so in that case clangd wouldn't be able to find all the files it's supposed to. Of course, if I've misunderstood, and somehow we get those arguments elsewhere, that's fine, but I just want to be sure we don't regress the behavior.
On Wed, Sep 22, 2021 at 6:41 PM brian m. carlson <sandals@crustytoothpaste.net> wrote: > > diff --git a/Makefile b/Makefile > > index 9df565f27b..d5c6d0ea3b 100644 > > --- a/Makefile > > +++ b/Makefile > > @@ -1302,7 +1302,7 @@ GENERATE_COMPILATION_DATABASE = no > > endif > > > > ifeq ($(GENERATE_COMPILATION_DATABASE),yes) > > -compdb_check = $(shell $(CC) $(ALL_CFLAGS) \ > > +compdb_check = $(shell $(CC) \ > > -c -MJ /dev/null \ > > -x c /dev/null -o /dev/null 2>&1; \ > > echo $$?) > > Are you sure this results in a functional set of files? no; it does not This call is only meant to be used to check if your compiler supports the feature (which as Ævar points out[1], might not be the best thing to do in this case), though After this fix the files are being generated (in a different place with their expected flags) and look healthy, but would be helpful to know you see no regressions. Carlo [1] https://lore.kernel.org/git/87tuic5cdo.fsf@evledraar.gmail.com/
On Wed, Sep 22 2021, Carlo Arenas wrote: > On Wed, Sep 22, 2021 at 6:41 PM brian m. carlson > <sandals@crustytoothpaste.net> wrote: > >> > diff --git a/Makefile b/Makefile >> > index 9df565f27b..d5c6d0ea3b 100644 >> > --- a/Makefile >> > +++ b/Makefile >> > @@ -1302,7 +1302,7 @@ GENERATE_COMPILATION_DATABASE = no >> > endif >> > >> > ifeq ($(GENERATE_COMPILATION_DATABASE),yes) >> > -compdb_check = $(shell $(CC) $(ALL_CFLAGS) \ >> > +compdb_check = $(shell $(CC) \ >> > -c -MJ /dev/null \ >> > -x c /dev/null -o /dev/null 2>&1; \ >> > echo $$?) >> >> Are you sure this results in a functional set of files? > > no; it does not > > This call is only meant to be used to check if your compiler supports > the feature (which as Ævar points out[1], might not be the best thing > to do in this case), though > > After this fix the files are being generated (in a different place > with their expected flags) and look healthy, but would be helpful to > know you see no regressions. I had the same thought as brian, but you're right, since we never use the result of this it's OK. IOW this check is really functionally equivalent to something like: cc --help | grep -q -F -- -MJ Or whatever, i.e. we're just checking if it's clang & supports the -MJ option. > [1] https://lore.kernel.org/git/87tuic5cdo.fsf@evledraar.gmail.com/
diff --git a/Makefile b/Makefile index 9df565f27b..d5c6d0ea3b 100644 --- a/Makefile +++ b/Makefile @@ -1302,7 +1302,7 @@ GENERATE_COMPILATION_DATABASE = no endif ifeq ($(GENERATE_COMPILATION_DATABASE),yes) -compdb_check = $(shell $(CC) $(ALL_CFLAGS) \ +compdb_check = $(shell $(CC) \ -c -MJ /dev/null \ -x c /dev/null -o /dev/null 2>&1; \ echo $$?)
3821c38068 (Makefile: add support for generating JSON compilation database, 2020-09-03), adds a feature to be used with clang to generate a compilation database by copying most of what was done before with the header dependency, but by doing so includes on its availability check the CFLAGS which became specially problematic once DEVELOPER=1 implied -pedantic as pointed out by Ævar[1]. Remove the unnecessary flags in the availability test, so it will work regardless of which other warnings are enabled or if the compiler has been told to error on them. [1] https://lore.kernel.org/git/patch-1.1-6b2e9af5e67-20210922T103749Z-avarab@gmail.com/ Helped-by: Eric Sunshine <sunshine@sunshineco.com> Signed-off-by: Carlo Marcelo Arenas Belón <carenas@gmail.com> --- Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)