| Message ID | 20181008115146.17797-1-lothar.felten@gmail.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | [1/1] package/dtc: fix include guards for older kernel/u-boot | expand |
Lothar, On Mon, Oct 8, 2018 at 6:52 AM Lothar Felten <lothar.felten@gmail.com> wrote: > > Patch dtc to accept libfdt include guards with and without leading underscore. > A compiler warning is added to notify the user. > > Signed-off-by: Lothar Felten <lothar.felten@gmail.com> > --- > ...de-guards-for-older-kernel-u-boot-sources.patch | 42 ++++++++++++++++++++++ > 1 file changed, 42 insertions(+) > create mode 100644 package/dtc/0001-Fix-include-guards-for-older-kernel-u-boot-sources.patch > > diff --git a/package/dtc/0001-Fix-include-guards-for-older-kernel-u-boot-sources.patch b/package/dtc/0001-Fix-include-guards-for-older-kernel-u-boot-sources.patch > new file mode 100644 > index 0000000000..40cb0bdb2a > --- /dev/null > +++ b/package/dtc/0001-Fix-include-guards-for-older-kernel-u-boot-sources.patch > @@ -0,0 +1,42 @@ > +From b1f8b84489c96465b63485b884238b61d31ca84d Mon Sep 17 00:00:00 2001 > +From: Lothar Felten <lothar.felten@gmail.com> > +Date: Mon, 8 Oct 2018 13:29:44 +0200 > +Subject: [PATCH 1/1] Fix include guards for older kernel/u-boot sources > + > +Linux kernels before 4.17 and U-Boot versions before 2018.07 use libfdt include guards with leading underscores. > +Those have been removed in dtc-1.4.7. Is there an reference that could be listed to either 1) A upstream mailinglist/patchwork link that proposes adding this patch 2) If the patch below can't be upstreamed, a reference to the commit that is reverting this support you're adding back in and maybe a summary of why the backwards compatibility isn't' accepted. Matt
Hello Matthew, > Lothar, > > > On Mon, Oct 8, 2018 at 6:52 AM Lothar Felten <lothar.felten@gmail.com > > wrote: > > > > Patch dtc to accept libfdt include guards with and without leading > > underscore. > > A compiler warning is added to notify the user. > > > > Signed-off-by: Lothar Felten <lothar.felten@gmail.com> > > --- > > ...de-guards-for-older-kernel-u-boot-sources.patch | 42 > > ++++++++++++++++++++++ > > 1 file changed, 42 insertions(+) > > create mode 100644 package/dtc/0001-Fix-include-guards-for-older- > > kernel-u-boot-sources.patch > > > > diff --git a/package/dtc/0001-Fix-include-guards-for-older-kernel- > > u-boot-sources.patch b/package/dtc/0001-Fix-include-guards-for- > > older-kernel-u-boot-sources.patch > > new file mode 100644 > > index 0000000000..40cb0bdb2a > > --- /dev/null > > +++ b/package/dtc/0001-Fix-include-guards-for-older-kernel-u-boot- > > sources.patch > > @@ -0,0 +1,42 @@ > > +From b1f8b84489c96465b63485b884238b61d31ca84d Mon Sep 17 00:00:00 > > 2001 > > +From: Lothar Felten <lothar.felten@gmail.com> > > +Date: Mon, 8 Oct 2018 13:29:44 +0200 > > +Subject: [PATCH 1/1] Fix include guards for older kernel/u-boot > > sources > > + > > +Linux kernels before 4.17 and U-Boot versions before 2018.07 use > > libfdt include guards with leading underscores. > > +Those have been removed in dtc-1.4.7. > > Is there an reference that could be listed to either > 1) A upstream mailinglist/patchwork link that proposes adding this > patch > 2) If the patch below can't be upstreamed, a reference to the commit > that is reverting this support you're adding back in and maybe a > summary of why the backwards compatibility isn't' accepted. This patch is a temporary fix for this issue: https://git.buildroot.org/buildroot/tree/CHANGES#n35 It does not fix any bug, just an incompatibility of a new dtc and older Linux kernels / u-boot loaders. Once all Linux kernel versions are >=4.17 and u-boot versions are >=2018.07 this patch is no longer required: they will all use the include guard without underscore. I could also patch the beaglebone kernel, but other targets have the same issue (e.g. bananapi_m1). > > Matt Lothar
On 8/10/18 15:10, Matthew Weber wrote: > Lothar, > > > On Mon, Oct 8, 2018 at 6:52 AM Lothar Felten <lothar.felten@gmail.com> wrote: >> >> Patch dtc to accept libfdt include guards with and without leading underscore. >> A compiler warning is added to notify the user. >> >> Signed-off-by: Lothar Felten <lothar.felten@gmail.com> >> --- >> ...de-guards-for-older-kernel-u-boot-sources.patch | 42 ++++++++++++++++++++++ >> 1 file changed, 42 insertions(+) >> create mode 100644 package/dtc/0001-Fix-include-guards-for-older-kernel-u-boot-sources.patch >> >> diff --git a/package/dtc/0001-Fix-include-guards-for-older-kernel-u-boot-sources.patch b/package/dtc/0001-Fix-include-guards-for-older-kernel-u-boot-sources.patch >> new file mode 100644 >> index 0000000000..40cb0bdb2a >> --- /dev/null >> +++ b/package/dtc/0001-Fix-include-guards-for-older-kernel-u-boot-sources.patch >> @@ -0,0 +1,42 @@ >> +From b1f8b84489c96465b63485b884238b61d31ca84d Mon Sep 17 00:00:00 2001 >> +From: Lothar Felten <lothar.felten@gmail.com> >> +Date: Mon, 8 Oct 2018 13:29:44 +0200 >> +Subject: [PATCH 1/1] Fix include guards for older kernel/u-boot sources >> + >> +Linux kernels before 4.17 and U-Boot versions before 2018.07 use libfdt include guards with leading underscores. >> +Those have been removed in dtc-1.4.7. > > Is there an reference that could be listed to either > 1) A upstream mailinglist/patchwork link that proposes adding this patch > 2) If the patch below can't be upstreamed, a reference to the commit > that is reverting this support you're adding back in and maybe a > summary of why the backwards compatibility isn't' accepted. I expect that it is not acceptable for upstream because this is only needed when you include *both* libfdt.h from dtc and the one from the local copy, which is clearly completely bogus. It happens due to bugs in the build system of u-boot and of the beaglebone fork of the kernel (AFAIK it doesn't happen with any upstream kernel version). So "fixing" it like this is really a workaround for broken u-boot/kernel that is only applicable to Buildroot, because we have no easy way to generically apply patches to u-boot/kernel while we can easily do it for dtc. Lothar, if you agree with the above analysis, could you include that in the commit message of 0001-Fix-include-guards-for-older-kernel-u-boot-sources.patch ? Also: - please wrap the commit message of the patch itself at 72 columns as well; - please number it properly, i.e. it should have number 2 instead of 1. Patch 1 was committed almost a month ago... Regards, Arnout
Lothar, All, On 2018-10-08 13:51 +0200, Lothar Felten spake thusly: > Patch dtc to accept libfdt include guards with and without leading underscore. > A compiler warning is added to notify the user. Please, do not add the warning. We should be as transparent as possible when we add such workarounds. It is not unseen that a configure script parses the output of gcc to detect stuff, and expect the output to be either empty or a specific regexp; adding this warning would break that. Otherwise, the patch is simple enough, and looks OKish as a workaround. Maybe we could suggest upstream to carry such a patch? And then, it would be their call to add the warning or not. Regards, Yann E. MORIN. > Signed-off-by: Lothar Felten <lothar.felten@gmail.com> > --- > ...de-guards-for-older-kernel-u-boot-sources.patch | 42 ++++++++++++++++++++++ > 1 file changed, 42 insertions(+) > create mode 100644 package/dtc/0001-Fix-include-guards-for-older-kernel-u-boot-sources.patch > > diff --git a/package/dtc/0001-Fix-include-guards-for-older-kernel-u-boot-sources.patch b/package/dtc/0001-Fix-include-guards-for-older-kernel-u-boot-sources.patch > new file mode 100644 > index 0000000000..40cb0bdb2a > --- /dev/null > +++ b/package/dtc/0001-Fix-include-guards-for-older-kernel-u-boot-sources.patch > @@ -0,0 +1,42 @@ > +From b1f8b84489c96465b63485b884238b61d31ca84d Mon Sep 17 00:00:00 2001 > +From: Lothar Felten <lothar.felten@gmail.com> > +Date: Mon, 8 Oct 2018 13:29:44 +0200 > +Subject: [PATCH 1/1] Fix include guards for older kernel/u-boot sources > + > +Linux kernels before 4.17 and U-Boot versions before 2018.07 use libfdt include guards with leading underscores. > +Those have been removed in dtc-1.4.7. > +This patch handles both include guard types and allows the compilation of older Linux kernel and u-boot sources. > + > +Signed-off-by: Lothar Felten <lothar.felten@gmail.com> > +--- > + libfdt/libfdt.h | 4 ++++ > + libfdt/libfdt_env.h | 4 ++++ > + 2 files changed, 8 insertions(+) > + > +diff --git a/libfdt/libfdt.h b/libfdt/libfdt.h > +index 830b77e..bef4566 100644 > +--- a/libfdt/libfdt.h > ++++ b/libfdt/libfdt.h > +@@ -1,3 +1,7 @@ > ++#ifdef _LIBFDT_H > ++#warning "Please consider updating your kernel and/or u-boot version" > ++#define LIBFDT_H > ++#endif > + #ifndef LIBFDT_H > + #define LIBFDT_H > + /* > +diff --git a/libfdt/libfdt_env.h b/libfdt/libfdt_env.h > +index eb20538..6a61e6a 100644 > +--- a/libfdt/libfdt_env.h > ++++ b/libfdt/libfdt_env.h > +@@ -1,3 +1,7 @@ > ++#ifdef _LIBFDT_ENV_H > ++#warning "Please consider updating your kernel and/or u-boot version" > ++#define LIBFDT_ENV_H > ++#endif > + #ifndef LIBFDT_ENV_H > + #define LIBFDT_ENV_H > + /* > +-- > +2.11.0 > + > -- > 2.11.0 >
Hello, On Mon, 8 Oct 2018 13:51:46 +0200, Lothar Felten wrote: > Patch dtc to accept libfdt include guards with and without leading underscore. > A compiler warning is added to notify the user. > > Signed-off-by: Lothar Felten <lothar.felten@gmail.com> So I guess this would fix the problem when host-dtc is built before some old Linux/U-Boot version. However, I suppose it doesn't fix the problem when you have a recent version of libfdt installed on your system (through your distribution), that doesn't play well with those older Linux/U-Boot versions. Correct ? Thomas
Thomas, All, On 2018-10-08 18:02 +0200, Thomas Petazzoni spake thusly: > On Mon, 8 Oct 2018 13:51:46 +0200, Lothar Felten wrote: > > Patch dtc to accept libfdt include guards with and without leading underscore. > > A compiler warning is added to notify the user. > > > > Signed-off-by: Lothar Felten <lothar.felten@gmail.com> > > So I guess this would fix the problem when host-dtc is built before > some old Linux/U-Boot version. > > However, I suppose it doesn't fix the problem when you have a > recent version of libfdt installed on your system (through your > distribution), that doesn't play well with those older Linux/U-Boot > versions. Correct ? Probably not, indeed, but that is still better (IMHO) than the few hacks we already tried with -isystem or the likes, which do not really work either... Regards, Yann E. MORIN.
On 8/10/18 18:32, Yann E. MORIN wrote: > Thomas, All, > > On 2018-10-08 18:02 +0200, Thomas Petazzoni spake thusly: >> On Mon, 8 Oct 2018 13:51:46 +0200, Lothar Felten wrote: >>> Patch dtc to accept libfdt include guards with and without leading underscore. >>> A compiler warning is added to notify the user. >>> >>> Signed-off-by: Lothar Felten <lothar.felten@gmail.com> >> >> So I guess this would fix the problem when host-dtc is built before >> some old Linux/U-Boot version. >> >> However, I suppose it doesn't fix the problem when you have a >> recent version of libfdt installed on your system (through your >> distribution), that doesn't play well with those older Linux/U-Boot >> versions. Correct ? > > Probably not, indeed, but that is still better (IMHO) than the few hacks > we already tried with -isystem or the likes, which do not really work > either... As far as we tested it, [1] *does* really work. However, it doesn't fix the issue for U-Boot because there the build system is really broken: it misses the necessary -I to include the internal libfdt.h, so only the fdt_host.h that explicitly includes ../include/libfdt.h uses the internal one; other includes use <libfdt.h> so the host-provided one. So U-Boot before v2018.03 is just broken beyond repair. Note that this patch will solve the problem for the time being. However, as soon as dtc's libfdt.h contains some declaration that is incompatible with the one in U-Boot pre-2018.03, U-Boot will break again. A possibly more fundamental workaround for U-Boot could be to pass -I$(UBOOT_SRCDIR)/include in HOSTCC, but that has the potential to introduce additional breakage... Regards, Arnout [1] http://patchwork.ozlabs.org/patch/964474/
Arnout, All, On 2018-10-08 21:21 +0200, Arnout Vandecappelle spake thusly: > On 8/10/18 18:32, Yann E. MORIN wrote: > > On 2018-10-08 18:02 +0200, Thomas Petazzoni spake thusly: > >> However, I suppose it doesn't fix the problem when you have a > >> recent version of libfdt installed on your system (through your > >> distribution), that doesn't play well with those older Linux/U-Boot > >> versions. Correct ? > > Probably not, indeed, but that is still better (IMHO) than the few hacks > > we already tried with -isystem or the likes, which do not really work > > either... > As far as we tested it, [1] *does* really work. > [1] http://patchwork.ozlabs.org/patch/964474/ No it does not. Please see commit 6f8162cf8c1abef7e0a4771fe0d6b26a28f5c2b6 which got reverted by 255b6f80d395ef048f46cfcf75dba690c56af657, and which was supposed to fix a very similar issue... Regards, Yann E. MORIN.
Hello, On Mon, 2018-10-08 at 18:02 +0200, Thomas Petazzoni wrote: > Hello, > > On Mon, 8 Oct 2018 13:51:46 +0200, Lothar Felten wrote: > > Patch dtc to accept libfdt include guards with and without leading > > underscore. > > A compiler warning is added to notify the user. > > > > Signed-off-by: Lothar Felten <lothar.felten@gmail.com> > > So I guess this would fix the problem when host-dtc is built before > some old Linux/U-Boot version. > > However, I suppose it doesn't fix the problem when you have a > recent version of libfdt installed on your system (through your > distribution), that doesn't play well with those older Linux/U-Boot > versions. Correct ? Maybe. I have no libfdt headers on my host. This patch fixes the failures caused by host-dtc (ver 1.4.7) and older u-boot or kernel versions. Building an older kernel without libfdt headers on the host works (it uses the libfdt from the kernel) as long as host-dtc is not selected. Just creating a /usr/include/[libfdt/]libfdt_env.h with a single #error does not prevent a u-boot-2016.09.01 build, so I suppose the include paths are ok. I expect this to be a temporary fix until all targets use a recent u- boot and kernel. > Thomas Lothar
On 8/10/18 21:31, Yann E. MORIN wrote: > Arnout, All, > > On 2018-10-08 21:21 +0200, Arnout Vandecappelle spake thusly: >> On 8/10/18 18:32, Yann E. MORIN wrote: >>> On 2018-10-08 18:02 +0200, Thomas Petazzoni spake thusly: >>>> However, I suppose it doesn't fix the problem when you have a >>>> recent version of libfdt installed on your system (through your >>>> distribution), that doesn't play well with those older Linux/U-Boot >>>> versions. Correct ? >>> Probably not, indeed, but that is still better (IMHO) than the few hacks >>> we already tried with -isystem or the likes, which do not really work >>> either... >> As far as we tested it, [1] *does* really work. >> [1] http://patchwork.ozlabs.org/patch/964474/ > > No it does not. Please see commit 6f8162cf8c1abef7e0a4771fe0d6b26a28f5c2b6 > which got reverted by 255b6f80d395ef048f46cfcf75dba690c56af657, and which > was supposed to fix a very similar issue... Ah, good that you saw this. Unfortunately 255b6f80d just says that there were too many issues. So I went back to the mail archives, and as far as I can see the only issue was with python. Python's setup script will parse CPPFLAGS to find the include directories, but it doesn't look at -isystem, only at -I. So it is actually quite easy to fix. The reason that it got reverted so quickly without looking for a real fix is that this was just before 2016.08-rc1, so the revert looked like the easy way out. And nobody took the time back then to try to fix the issue fundamentally. For reference: the failures occur when building in a minimal environment (particularly, no system zlib devel package). It happened for host-python-setuptools, tvheadend and mongodb, at least. Regards, Arnout
On 08/10/2018 12:51, Lothar Felten wrote: > Patch dtc to accept libfdt include guards with and without leading underscore. > A compiler warning is added to notify the user. > > Signed-off-by: Lothar Felten <lothar.felten@gmail.com> At the Buildroot meeting, we agreed that the warning is OK, so I extended the commit message and applied to master, thanks. Regards, Arnout > --- > ...de-guards-for-older-kernel-u-boot-sources.patch | 42 ++++++++++++++++++++++ > 1 file changed, 42 insertions(+) > create mode 100644 package/dtc/0001-Fix-include-guards-for-older-kernel-u-boot-sources.patch > > diff --git a/package/dtc/0001-Fix-include-guards-for-older-kernel-u-boot-sources.patch b/package/dtc/0001-Fix-include-guards-for-older-kernel-u-boot-sources.patch > new file mode 100644 > index 0000000000..40cb0bdb2a > --- /dev/null > +++ b/package/dtc/0001-Fix-include-guards-for-older-kernel-u-boot-sources.patch > @@ -0,0 +1,42 @@ > +From b1f8b84489c96465b63485b884238b61d31ca84d Mon Sep 17 00:00:00 2001 > +From: Lothar Felten <lothar.felten@gmail.com> > +Date: Mon, 8 Oct 2018 13:29:44 +0200 > +Subject: [PATCH 1/1] Fix include guards for older kernel/u-boot sources > + > +Linux kernels before 4.17 and U-Boot versions before 2018.07 use libfdt include guards with leading underscores. > +Those have been removed in dtc-1.4.7. > +This patch handles both include guard types and allows the compilation of older Linux kernel and u-boot sources. > + > +Signed-off-by: Lothar Felten <lothar.felten@gmail.com> > +--- > + libfdt/libfdt.h | 4 ++++ > + libfdt/libfdt_env.h | 4 ++++ > + 2 files changed, 8 insertions(+) > + > +diff --git a/libfdt/libfdt.h b/libfdt/libfdt.h > +index 830b77e..bef4566 100644 > +--- a/libfdt/libfdt.h > ++++ b/libfdt/libfdt.h > +@@ -1,3 +1,7 @@ > ++#ifdef _LIBFDT_H > ++#warning "Please consider updating your kernel and/or u-boot version" > ++#define LIBFDT_H > ++#endif > + #ifndef LIBFDT_H > + #define LIBFDT_H > + /* > +diff --git a/libfdt/libfdt_env.h b/libfdt/libfdt_env.h > +index eb20538..6a61e6a 100644 > +--- a/libfdt/libfdt_env.h > ++++ b/libfdt/libfdt_env.h > +@@ -1,3 +1,7 @@ > ++#ifdef _LIBFDT_ENV_H > ++#warning "Please consider updating your kernel and/or u-boot version" > ++#define LIBFDT_ENV_H > ++#endif > + #ifndef LIBFDT_ENV_H > + #define LIBFDT_ENV_H > + /* > +-- > +2.11.0 > + >
>>>>> "Lothar" == Lothar Felten <lothar.felten@gmail.com> writes: > Patch dtc to accept libfdt include guards with and without leading underscore. > A compiler warning is added to notify the user. > Signed-off-by: Lothar Felten <lothar.felten@gmail.com> Committed to 2018.08.x, thanks.
diff --git a/package/dtc/0001-Fix-include-guards-for-older-kernel-u-boot-sources.patch b/package/dtc/0001-Fix-include-guards-for-older-kernel-u-boot-sources.patch new file mode 100644 index 0000000000..40cb0bdb2a --- /dev/null +++ b/package/dtc/0001-Fix-include-guards-for-older-kernel-u-boot-sources.patch @@ -0,0 +1,42 @@ +From b1f8b84489c96465b63485b884238b61d31ca84d Mon Sep 17 00:00:00 2001 +From: Lothar Felten <lothar.felten@gmail.com> +Date: Mon, 8 Oct 2018 13:29:44 +0200 +Subject: [PATCH 1/1] Fix include guards for older kernel/u-boot sources + +Linux kernels before 4.17 and U-Boot versions before 2018.07 use libfdt include guards with leading underscores. +Those have been removed in dtc-1.4.7. +This patch handles both include guard types and allows the compilation of older Linux kernel and u-boot sources. + +Signed-off-by: Lothar Felten <lothar.felten@gmail.com> +--- + libfdt/libfdt.h | 4 ++++ + libfdt/libfdt_env.h | 4 ++++ + 2 files changed, 8 insertions(+) + +diff --git a/libfdt/libfdt.h b/libfdt/libfdt.h +index 830b77e..bef4566 100644 +--- a/libfdt/libfdt.h ++++ b/libfdt/libfdt.h +@@ -1,3 +1,7 @@ ++#ifdef _LIBFDT_H ++#warning "Please consider updating your kernel and/or u-boot version" ++#define LIBFDT_H ++#endif + #ifndef LIBFDT_H + #define LIBFDT_H + /* +diff --git a/libfdt/libfdt_env.h b/libfdt/libfdt_env.h +index eb20538..6a61e6a 100644 +--- a/libfdt/libfdt_env.h ++++ b/libfdt/libfdt_env.h +@@ -1,3 +1,7 @@ ++#ifdef _LIBFDT_ENV_H ++#warning "Please consider updating your kernel and/or u-boot version" ++#define LIBFDT_ENV_H ++#endif + #ifndef LIBFDT_ENV_H + #define LIBFDT_ENV_H + /* +-- +2.11.0 +
Patch dtc to accept libfdt include guards with and without leading underscore. A compiler warning is added to notify the user. Signed-off-by: Lothar Felten <lothar.felten@gmail.com> --- ...de-guards-for-older-kernel-u-boot-sources.patch | 42 ++++++++++++++++++++++ 1 file changed, 42 insertions(+) create mode 100644 package/dtc/0001-Fix-include-guards-for-older-kernel-u-boot-sources.patch