From: Carlo Marcelo Arenas Belón <hidden> Date: 2021-09-22 18:33:51
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 compilers has
been told to error on them.
[1] https://lore.kernel.org/git/patch-1.1-6b2e9af5e67-20210922T103749Z-avarab@gmail.com/
Signed-off-by: Carlo Marcelo Arenas Belón <redacted>
---
Makefile | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
From: Eric Sunshine <hidden> Date: 2021-09-22 18:45:38
On Wed, Sep 22, 2021 at 2:33 PM Carlo Marcelo Arenas Belón
[off-list ref] wrote:
Makefile: avoid breaking compilation database generation with DEPELOPER
s/DEPELOPER/DEVELOPER/
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 compilers has
From: Carlo Marcelo Arenas Belón <hidden> Date: 2021-09-22 18:57:13
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 [off-list ref]
Signed-off-by: Carlo Marcelo Arenas Belón <redacted>
---
Makefile | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
From: Philippe Blain <hidden> Date: 2021-09-22 19:07:14
Hi Carlo,
Le 2021-09-22 à 14:57, Carlo Marcelo Arenas Belón a écrit :
quoted hunk
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 [off-list ref]
Signed-off-by: Carlo Marcelo Arenas Belón <redacted>
---
Makefile | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
On Wed, Sep 22 2021, Carlo Marcelo Arenas Belón wrote:
quoted hunk
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 compilers has
been told to error on them.
[1] https://lore.kernel.org/git/patch-1.1-6b2e9af5e67-20210922T103749Z-avarab@gmail.com/
Signed-off-by: Carlo Marcelo Arenas Belón <redacted>
---
Makefile | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
Sorry about the overlap in
https://lore.kernel.org/git/patch-v2-2.2-6b18bd08894-20210922T220532Z-avarab@gmail.com/;
I didn't see this thread before sending my version.
I think your patch here is better than mine. FWIW I also had this on top
of mine, i.e. emitting output to stderr unconditionally:
https://github.com/avar/git/commit/d4bcc0e617e52df803870df29df82aa3b2205d84
But thinking about it again I think with the rationale in that
not-on-list commit of mine the below is better than either of our
versions v1.
I.e. for COMPUTE_HEADER_DEPENDENCIES the point of the test is that we
turn it on automatically, so it needs to not suck by default. The reason
we're doing this is, per the comment in 3821c38068:
If this variable is set, check that $(CC) indeed supports the `-MJ`
flag, following what is done for automatic dependencies.
Anyone using GENERATE_COMPILATION_DATABASE is turning it on explicitly,
and I daresay if they're using it at all they're either not using
anything but clang, or is keenly aware of the difference.
So do we really need to carry those 17 lines of the Makefile logic
simply to avoid showing this error on say "CC=gcc
GENERATE_COMPILATION_DATABASE=yes":
gcc: error: unrecognized command-line option ‘-MJ’; did you mean ‘-J’?
It doesn't seem worth it to me, especially as we document that we'll use
the "-MJ" flag in the Makefile comment that the person turning on
GENERATE_COMPILATION_DATABASE=yes must have read.
Anyway, I'll leave you to do what you think is best here, and I'm also
fine with just going for the v1 you've got here, it just seems to me
like we're both fixing logic that's been copy/pasted from
COMPUTE_HEADER_DEPENDENCIES, and the reasons we need it for that
facility don't apply at all to GENERATE_COMPILATION_DATABASE.
-- >8 --
From: brian m. carlson <hidden> Date: 2021-09-23 01:41:47
On 2021-09-22 at 18:57:02, Carlo Marcelo Arenas Belón wrote:
quoted hunk
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 [off-list ref]
Signed-off-by: Carlo Marcelo Arenas Belón <redacted>
---
Makefile | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
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.
--
brian m. carlson (he/him or they/them)
Toronto, Ontario, CA
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/
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.