diff mbox series

[v2] fakeroot: fix to work with glibc 2.33

Message ID 20210212054210.684510-1-ilya.lipnitskiy@gmail.com
State Superseded
Headers show
Series [v2] fakeroot: fix to work with glibc 2.33 | expand

Commit Message

Ilya Lipnitskiy Feb. 12, 2021, 5:42 a.m. UTC
The following commit removed _STAT_VER definitions from glibc:
https://sourceware.org/git/?p=glibc.git;a=commitdiff;h=8ed005daf0ab03e142500324a34087ce179ae78e

That subsequently broke fakeroot:
https://bugs.archlinux.org/task/69572
https://bugzilla.redhat.com/show_bug.cgi?id=1889862#c13
https://forum.openwrt.org/t/unable-to-build-toolchain-fakeroot-fails-perhaps-others-after-it/87966

Make the patch based on Jan Pazdziora's suggestion from here:
https://lists.fedoraproject.org/archives/list/devel@lists.fedoraproject.org/message/SMQ3RYXEYTVZH6PLQMKNB3NM4XLPMNZO/

Additionally, add wrappers for newly exported symbols in glibc.

Tested on my x86_64 Arch Linux machine, fakeroot unit tests pass.
Also tested by building various .ipks and examining the tar contents, to
ensure that the owner uid/gid was 0/0.

Signed-off-by: Ilya Lipnitskiy <ilya.lipnitskiy@gmail.com>
---
 .../300-glibc-2.33-compatibility.patch        | 74 +++++++++++++++++++
 1 file changed, 74 insertions(+)
 create mode 100644 tools/fakeroot/patches/300-glibc-2.33-compatibility.patch

Comments

Philip Prindeville Feb. 12, 2021, 7:23 a.m. UTC | #1
Comments...


> On Feb 11, 2021, at 10:42 PM, Ilya Lipnitskiy <ilya.lipnitskiy@gmail.com> wrote:
> 
> The following commit removed _STAT_VER definitions from glibc:
> https://sourceware.org/git/?p=glibc.git;a=commitdiff;h=8ed005daf0ab03e142500324a34087ce179ae78e
> 
> That subsequently broke fakeroot:
> https://bugs.archlinux.org/task/69572
> https://bugzilla.redhat.com/show_bug.cgi?id=1889862#c13
> https://forum.openwrt.org/t/unable-to-build-toolchain-fakeroot-fails-perhaps-others-after-it/87966
> 
> Make the patch based on Jan Pazdziora's suggestion from here:
> https://lists.fedoraproject.org/archives/list/devel@lists.fedoraproject.org/message/SMQ3RYXEYTVZH6PLQMKNB3NM4XLPMNZO/
> 
> Additionally, add wrappers for newly exported symbols in glibc.
> 
> Tested on my x86_64 Arch Linux machine, fakeroot unit tests pass.
> Also tested by building various .ipks and examining the tar contents, to
> ensure that the owner uid/gid was 0/0.
> 
> Signed-off-by: Ilya Lipnitskiy <ilya.lipnitskiy@gmail.com>
> ---
> .../300-glibc-2.33-compatibility.patch        | 74 +++++++++++++++++++
> 1 file changed, 74 insertions(+)
> create mode 100644 tools/fakeroot/patches/300-glibc-2.33-compatibility.patch
> 
> diff --git a/tools/fakeroot/patches/300-glibc-2.33-compatibility.patch b/tools/fakeroot/patches/300-glibc-2.33-compatibility.patch
> new file mode 100644
> index 0000000000..2e6beab095
> --- /dev/null
> +++ b/tools/fakeroot/patches/300-glibc-2.33-compatibility.patch
> @@ -0,0 +1,74 @@
> +--- a/libfakeroot.c
> ++++ b/libfakeroot.c
> +@@ -90,6 +90,16 @@
> + #define SEND_GET_XATTR64(a,b,c) send_get_xattr64(a,b)
> + #endif
> + 
> ++#ifndef _STAT_VER
> ++ #if defined (__aarch64__)
> ++  #define _STAT_VER 0
> ++ #elif defined (__x86_64__)
> ++  #define _STAT_VER 1
> ++ #else
> ++  #define _STAT_VER 3
> ++ #endif
> ++#endif
> ++
> + /*
> +    These INT_* (which stands for internal) macros should always be used when
> +    the fakeroot library owns the storage of the stat variable.
> +@@ -1358,6 +1368,54 @@ int renameat(int olddir_fd, const char *
> + #endif /* HAVE_FSTATAT */
> + 
> + 
> ++#ifdef __GLIBC__
> ++#if __GLIBC_PREREQ(2,33)

Minor nit, but please combine these into a single line:

#if defined(__GLIBC__) && __GLIBC_PREREQ(2,33)

As that's easier to grep.


> ++/* Glibc 2.33 exports symbols for these functions in the shared lib */
> ++int lstat(const char *file_name, struct stat *statbuf) {
> ++   return WRAP_LSTAT LSTAT_ARG(_STAT_VER, file_name, statbuf);
> ++}
> ++int stat(const char *file_name, struct stat *st) {
> ++   return WRAP_STAT STAT_ARG(_STAT_VER, file_name, st);
> ++}
> ++int fstat(int fd, struct stat *st) {
> ++   return WRAP_FSTAT FSTAT_ARG(_STAT_VER, fd, st);
> ++}
> ++#ifdef HAVE_FSTATAT

Please indent nested #if or #ifdef's.


> ++int fstatat(int dir_fd, const char *path, struct stat *st, int flags) {
> ++   return WRAP_FSTATAT FSTATAT_ARG(_STAT_VER, dir_fd, path, st, flags);
> ++}
> ++#endif
> ++#ifdef STAT64_SUPPORT
> ++int lstat64(const char *file_name, struct stat64 *st) {
> ++   return WRAP_LSTAT64 LSTAT64_ARG(_STAT_VER, file_name, st);
> ++}
> ++int stat64(const char *file_name, struct stat64 *st) {
> ++   return WRAP_STAT64 STAT64_ARG(_STAT_VER, file_name, st);
> ++}
> ++int fstat64(int fd, struct stat64 *st) {
> ++   return WRAP_FSTAT64 FSTAT64_ARG(_STAT_VER, fd, st);
> ++}
> ++#ifdef HAVE_FSTATAT

Ditto.


> ++int fstatat64(int dir_fd, const char *path, struct stat64 *st, int flags) {
> ++   return WRAP_FSTATAT64 FSTATAT64_ARG(_STAT_VER, dir_fd, path, st, flags);
> ++}
> ++#endif
> ++#endif
> ++int mknod(const char *pathname, mode_t mode, dev_t dev) {
> ++   return WRAP_MKNOD MKNOD_ARG(_STAT_VER, pathname, mode, &dev);
> ++}
> ++#ifdef HAVE_FSTATAT
> ++#ifdef HAVE_MKNODAT

Combine as:

#if defined(HAVE_FSTATAT) && defined(HAVE_MKNODAT)


> ++int mknodat(int dir_fd, const char *pathname, mode_t mode, dev_t dev) {
> ++   return WRAP_MKNODAT MKNODAT_ARG(_STAT_VER, dir_fd, pathname, mode, &dev);
> ++}
> ++#endif
> ++#endif

Combined...


> ++
> ++#endif /* __GLIBC_PREPREQ */
> ++#endif /* __GLIBC__ */

Combined...

> ++
> ++
> + #ifdef FAKEROOT_FAKENET
> + pid_t fork(void)
> + {
> -- 
> 2.30.1
Ilya Lipnitskiy Feb. 12, 2021, 5:52 p.m. UTC | #2
Hi Philip,

On Thu, Feb 11, 2021 at 11:23 PM Philip Prindeville
<philipp_subx@redfish-solutions.com> wrote:
>
> Minor nit, but please combine these into a single line:
>
>
> Please indent nested #if or #ifdef's.
>
>
I can make the style changes, but a couple of points to consider:
1. These changes have already been submitted upstream[0];
2. If you look elsewhere within libfakeroot.c it is peppered with
#ifdefs that are not indented or combined, so I mostly followed the
existing code style (or the absence thereof)

Ilya

[0]: https://salsa.debian.org/clint/fakeroot/-/merge_requests/8
Philip Prindeville Feb. 14, 2021, 2:39 a.m. UTC | #3
> On Feb 12, 2021, at 10:52 AM, Ilya Lipnitskiy <ilya.lipnitskiy@gmail.com> wrote:
> 
> Hi Philip,
> 
> On Thu, Feb 11, 2021 at 11:23 PM Philip Prindeville
> <philipp_subx@redfish-solutions.com> wrote:
>> 
>> Minor nit, but please combine these into a single line:
>> 
>> 
>> Please indent nested #if or #ifdef's.
>> 
>> 
> I can make the style changes, but a couple of points to consider:
> 1. These changes have already been submitted upstream[0];
> 2. If you look elsewhere within libfakeroot.c it is peppered with
> #ifdefs that are not indented or combined, so I mostly followed the
> existing code style (or the absence thereof)
> 
> Ilya
> 
> [0]: https://salsa.debian.org/clint/fakeroot/-/merge_requests/8


If it's already been submitted and accepted, then leave it.

If they ask you to resubmit with changes, add that too please.

-Philip
Ilya Lipnitskiy Feb. 14, 2021, 4:38 a.m. UTC | #4
> If they ask you to resubmit with changes, add that too please.
Somehow I doubt that the upstream will be quick to respond, but one may hope :)

I went ahead and fixed a couple more compiler warnings and an error
when building with DEBUG enabled. I have also applied your suggested
style changes. I'll roll the patch series shortly.

- Ilya
diff mbox series

Patch

diff --git a/tools/fakeroot/patches/300-glibc-2.33-compatibility.patch b/tools/fakeroot/patches/300-glibc-2.33-compatibility.patch
new file mode 100644
index 0000000000..2e6beab095
--- /dev/null
+++ b/tools/fakeroot/patches/300-glibc-2.33-compatibility.patch
@@ -0,0 +1,74 @@ 
+--- a/libfakeroot.c
++++ b/libfakeroot.c
+@@ -90,6 +90,16 @@
+ #define SEND_GET_XATTR64(a,b,c) send_get_xattr64(a,b)
+ #endif
+ 
++#ifndef _STAT_VER
++ #if defined (__aarch64__)
++  #define _STAT_VER 0
++ #elif defined (__x86_64__)
++  #define _STAT_VER 1
++ #else
++  #define _STAT_VER 3
++ #endif
++#endif
++
+ /*
+    These INT_* (which stands for internal) macros should always be used when
+    the fakeroot library owns the storage of the stat variable.
+@@ -1358,6 +1368,54 @@ int renameat(int olddir_fd, const char *
+ #endif /* HAVE_FSTATAT */
+ 
+ 
++#ifdef __GLIBC__
++#if __GLIBC_PREREQ(2,33)
++/* Glibc 2.33 exports symbols for these functions in the shared lib */
++int lstat(const char *file_name, struct stat *statbuf) {
++   return WRAP_LSTAT LSTAT_ARG(_STAT_VER, file_name, statbuf);
++}
++int stat(const char *file_name, struct stat *st) {
++   return WRAP_STAT STAT_ARG(_STAT_VER, file_name, st);
++}
++int fstat(int fd, struct stat *st) {
++   return WRAP_FSTAT FSTAT_ARG(_STAT_VER, fd, st);
++}
++#ifdef HAVE_FSTATAT
++int fstatat(int dir_fd, const char *path, struct stat *st, int flags) {
++   return WRAP_FSTATAT FSTATAT_ARG(_STAT_VER, dir_fd, path, st, flags);
++}
++#endif
++#ifdef STAT64_SUPPORT
++int lstat64(const char *file_name, struct stat64 *st) {
++   return WRAP_LSTAT64 LSTAT64_ARG(_STAT_VER, file_name, st);
++}
++int stat64(const char *file_name, struct stat64 *st) {
++   return WRAP_STAT64 STAT64_ARG(_STAT_VER, file_name, st);
++}
++int fstat64(int fd, struct stat64 *st) {
++   return WRAP_FSTAT64 FSTAT64_ARG(_STAT_VER, fd, st);
++}
++#ifdef HAVE_FSTATAT
++int fstatat64(int dir_fd, const char *path, struct stat64 *st, int flags) {
++   return WRAP_FSTATAT64 FSTATAT64_ARG(_STAT_VER, dir_fd, path, st, flags);
++}
++#endif
++#endif
++int mknod(const char *pathname, mode_t mode, dev_t dev) {
++   return WRAP_MKNOD MKNOD_ARG(_STAT_VER, pathname, mode, &dev);
++}
++#ifdef HAVE_FSTATAT
++#ifdef HAVE_MKNODAT
++int mknodat(int dir_fd, const char *pathname, mode_t mode, dev_t dev) {
++   return WRAP_MKNODAT MKNODAT_ARG(_STAT_VER, dir_fd, pathname, mode, &dev);
++}
++#endif
++#endif
++
++#endif /* __GLIBC_PREPREQ */
++#endif /* __GLIBC__ */
++
++
+ #ifdef FAKEROOT_FAKENET
+ pid_t fork(void)
+ {