Re: [PATCH v7 5/5] linux: Add mount_setattr

[Adhemerval Zanella Netto <adhemerval.zanella@linaro.org>]

Hi Rudi,

Thanks for track this down, it does seems a better alternative to avoid
mixing glibc and Linux headers.  Even though we try to make them
compatible, it means implementation detais (such internal defines)
or extra boilerplate code (such as has_include) needs to be added
to handle it.

On 12/07/22 10:11, Rudi Heitbaum wrote:
> Hi Adhemerval,
> 
> I tracked down the build error with systemd being the
> HAVE_STRUCT_STATX was not being set in the meson.build - due to the
> conflict occurring with the inclusion of linux/fs.h.
> 
> The fix was to drop linux/fs.h (the same as in gcc libsanitizer.) I'll
> raise an issue on the systemd GitHub.
> The following patch (whilst not tested on < glibc 2.36) allowed the
> compile to succeed.)
> 
> --- a/meson.build       2022-07-12 12:20:25.366313151 +0000
> +++ b/meson.build       2022-07-12 12:46:19.276121413 +0000
> @@ -479,7 +479,6 @@
>   #include <uchar.h>
>   #include <sys/mount.h>
>   #include <sys/stat.h>
> -#include <linux/fs.h>
>   '''
> 
>   foreach decl : ['char16_t',
> --- a/src/shared/mount-util.c   2022-06-02 18:07:11.000000000 +0000
> +++ b/src/shared/mount-util.c   2022-07-12 12:52:46.644787385 +0000
> @@ -7,7 +7,6 @@
>   #include <sys/statvfs.h>
>   #include <unistd.h>
>   #include <linux/loop.h>
> -#include <linux/fs.h>
> 
>   #include "alloc-util.h"
>   #include "chase-symlinks.h"
> --- a/src/core/namespace.c      2022-06-02 18:07:11.000000000 +0000
> +++ b/src/core/namespace.c      2022-07-12 12:56:35.807502035 +0000
> @@ -6,7 +6,6 @@
>   #include <stdio.h>
>   #include <sys/mount.h>
>   #include <unistd.h>
> -#include <linux/fs.h>
> 
>   #include "alloc-util.h"
>   #include "base-filesystem.h"
> 
> On Mon, 11 Jul 2022 at 23:13, Rudi Heitbaum <rudi@heitbaum.com> wrote:
>>
>> Hi Adhemerval,
>>
>> linux-Add-fsconfig.patch broke the build, I reverted the others for a
>> clean revert.
>> Just adding linux-Add-fsconfig.patch back causes the systemd build to fail.
>>
>> Thanks
>> Rudi
>>
>> On Mon, 11 Jul 2022 at 22:51, Adhemerval Zanella
>> <adhemerval.zanella@linaro.org> wrote:
>>>
>>>
>>>
>>>> On 9 Jul 2022, at 23:28, Rudi Heitbaum <rudi@heitbaum.com> wrote:
>>>>
>>>> I am facing the same issue compiling gcc-12.1.0. but since
>>>> linux-Add-fsconfig.patch also facing the following error with
>>>> systemd-251.2.
>>>>
>>>> I was successfully able to compile systemd-251.2 by reverting:
>>>> - linux-Add-fsconfig.patch
>>>> - linux-Add-fspick.patch
>>>> - linux-Add-open_tree.patch
>>>> - linux-Add-tst-mount-to-check-for-Linux-new-mount-API.patch
>>>> - linux-Add-mount_setattr.patch
>>>> - stdlib-Implement-mbrtoc8-c8rtomb-and-the-char8_t-typ.patch
>>>> from 2a5b4f7a715921a232f67f6810268c6cd6aa0af2
>>>
>>> Which patch exactly broke the build, because from the logs is hard to
>>> tell exactly (specially because it should not interfere with statx
>>> definition)?
>>>
>>>>
>>>> Executing (target): ninja
>>>> [100/868] Compiling C object src/basic/libbasic.a.p/chase-symlinks.c.o
>>>> FAILED: src/basic/libbasic.a.p/chase-symlinks.c.o
>>>> /var/media/DATA/home-rudi/LibreELEC.tv/build.LibreELEC-Generic.x86_64-11.0-devel/toolchain/bin/x86_64-libreelec-linux-gnu-gcc -Isrc/basic/libbasic.a.p -Isrc/basic -I../src/basic -Isrc/fundamental -I../src/fundamental -Isrc/systemd -I../src/systemd -I. -I.. -I/var/media/DATA/home-rudi/LibreELEC.tv/build.LibreELEC-Generic.x86_64-11.0-devel/toolchain/x86_64-libreelec-linux-gnu/sysroot/usr/include -fdiagnostics-color=always -D_FILE_OFFSET_BITS=64 -Wall -Winvalid-pch -Wextra -std=gnu11 -O0 -Wno-format-signedness -Wno-missing-field-initializers -Wno-unused-parameter -Wdate-time -Wendif-labels -Werror=format=2 -Werror=implicit-function-declaration -Werror=incompatible-pointer-types -Werror=int-conversion -Werror=overflow -Werror=override-init -Werror=return-type -Werror=shift-count-overflow -Werror=shift-overflow=2 -Werror=undef -Wfloat-equal -Wimplicit-fallthrough=5 -Winit-self -Wlogical-op -Wmissing-include-dirs -Wmissing-noreturn -Wnested-externs -Wold-style-definition -Wpointer-arith -Wredundant-decls -Wshadow -Wstrict-aliasing=2 -Wstrict-prototypes -Wsuggest-attribute=noreturn -Wunused-function -Wwrite-strings -Wno-unused-result -Werror=missing-declarations -Werror=missing-prototypes -fdiagnostics-show-option -fno-common -fno-strict-aliasing -fstack-protector -fstack-protector-strong -fvisibility=hidden --param=ssp-buffer-size=4 -ffunction-sections -fdata-sections -Werror=shadow -include config.h -march=x86-64-v3 -Wall -pipe -O2 -fomit-frame-pointer -DNDEBUG -fno-schedule-insns -fno-schedule-insns2 -Wno-format-truncation -fPIC -pthread -fvisibility=default -MD -MQ src/basic/libbasic.a.p/chase-symlinks.c.o -MF src/basic/libbasic.a.p/chase-symlinks.c.o.d -o src/basic/libbasic.a.p/chase-symlinks.c.o -c ../src/basic/chase-symlinks.c
>>>> In file included from ../src/basic/stat-util.h:13,
>>>>                  from ../src/basic/chase-symlinks.h:7,
>>>>                  from ../src/basic/chase-symlinks.c:6:
>>>> ../src/basic/missing_stat.h:39:8: error: redefinition of 'struct statx_timestamp'
>>>>    39 | struct statx_timestamp {
>>>>       |        ^~~~~~~~~~~~~~~
>>>> In file included from /var/media/DATA/home-rudi/LibreELEC.tv/build.LibreELEC-Generic.x86_64-11.0-devel/toolchain/x86_64-libreelec-linux-gnu/sysroot/usr/include/bits/statx.h:31,
>>>>                  from /var/media/DATA/home-rudi/LibreELEC.tv/build.LibreELEC-Generic.x86_64-11.0-devel/toolchain/x86_64-libreelec-linux-gnu/sysroot/usr/include/sys/stat.h:465,
>>>>                  from ../src/basic/stat-util.h:7:
>>>> /var/media/DATA/home-rudi/LibreELEC.tv/build.LibreELEC-Generic.x86_64-11.0-devel/toolchain/x86_64-libreelec-linux-gnu/sysroot/usr/include/linux/stat.h:56:8: note: originally defined here
>>>>    56 | struct statx_timestamp {
>>>>       |        ^~~~~~~~~~~~~~~
>>>> ../src/basic/missing_stat.h:45:8: error: redefinition of 'struct statx'
>>>>    45 | struct statx STATX_DEFINITION;
>>>>       |        ^~~~~
>>>> /var/media/DATA/home-rudi/LibreELEC.tv/build.LibreELEC-Generic.x86_64-11.0-devel/toolchain/x86_64-libreelec-linux-gnu/sysroot/usr/include/linux/stat.h:99:8: note: originally defined here
>>>>    99 | struct statx {
>>>>       |        ^~~~~
>>>>
>>>> Regards
>>>> Rudi
>>>>
>>>> On Wed, Jul 06, 2022 at 09:21:32AM -0300, Adhemerval Zanella wrote:
>>>>>
>>>>>> On 6 Jul 2022, at 06:25, Andreas Schwab <schwab@suse.de> wrote:
>>>>>>
>>>>>> This is causing a conflict with <linux/mount.h>, breaking the build of
>>>>>> gcc.
>>>>>>
>>>>>> In file included from /usr/include/linux/fs.h:19,
>>>>>>                 from ../../../../libsanitizer/sanitizer_common/sanitizer_platform_limits_posix.cpp:76:
>>>>>> /usr/include/linux/mount.h:95:6: error: multiple definition of 'enum fsconfig_command'
>>>>>>   95 | enum fsconfig_command {
>>>>>>      |      ^~~~~~~~~~~~~~~~
>>>>>> In file included from ../../../../libsanitizer/sanitizer_common/sanitizer_platform_limits_posix.cpp:63:
>>>>>> /usr/include/sys/mount.h:189:6: note: previous definition here
>>>>>> 189 | enum fsconfig_command
>>>>>>      |      ^~~~~~~~~~~~~~~~
>>>>>> /usr/include/linux/mount.h:129:8: error: redefinition of 'struct mount_attr'
>>>>>> 129 | struct mount_attr {
>>>>>>      |        ^~~~~~~~~~
>>>>>> /usr/include/sys/mount.h:161:8: note: previous definition of 'struct mount_attr'
>>>>>> 161 | struct mount_attr
>>>>>>      |        ^~~~~~~~~~
>>>>>> make[4]: *** [Makefile:617: sanitizer_platform_limits_posix.lo] Error 1
>>>>>
>>>>> I am not sure how to handle it, glibc sys/mount.h should be a standalone
>>>>> header.  Maybe check if _UAPI_LINUX_MOUNT_H is defined so to use the
>>>>> kernel definition? Or check the kernel version and __has_include and use
>>>>> kernel version instead?
>>>>>
>>>>>>
>>>>>> --
>>>>>> Andreas Schwab, SUSE Labs, schwab@suse.de
>>>>>> GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE  1748 E4D4 88E3 0EEA B9D7
>>>>>> "And now for something completely different."
>>>>>
>>>