kdave / btrfs-progs

Development of userspace BTRFS tools
GNU General Public License v2.0
527 stars 239 forks source link

convert build breaks with e2fsprogs v1.47.1-rc2: `implicit declaration of function 'inode_includes'` #785

Closed katexochen closed 1 month ago

katexochen commented 2 months ago

There is a new release candidate for e2fsprogs https://github.com/tytso/e2fsprogs/releases/tag/v1.47.1-rc2

Linking btrfs-progs v6.8 against this version of e2fsprogs leads to the following compile error:

convert/source-ext2.c: In function 'ext4_copy_inode_timespec_extra':
convert/source-ext2.c:733:13: warning: implicit declaration of function 'inode_includes' [-Wimplicit-function-declaration]
  733 |         if (inode_includes(inode_size, i_ ## xtime ## _extra)) {                        \
      |             ^~~~~~~~~~~~~~
convert/source-ext2.c:769:9: note: in expansion of macro 'EXT4_COPY_XTIME'
  769 |         EXT4_COPY_XTIME(atime, dst, tv_sec, tv_nsec);
      |         ^~~~~~~~~~~~~~~
convert/source-ext2.c:733:40: error: 'i_atime_extra' undeclared (first use in this function)
  733 |         if (inode_includes(inode_size, i_ ## xtime ## _extra)) {                        \
      |                                        ^~
convert/source-ext2.c:769:9: note: in expansion of macro 'EXT4_COPY_XTIME'
  769 |         EXT4_COPY_XTIME(atime, dst, tv_sec, tv_nsec);
      |         ^~~~~~~~~~~~~~~
convert/source-ext2.c:733:40: note: each undeclared identifier is reported only once for each function it appears in
  733 |         if (inode_includes(inode_size, i_ ## xtime ## _extra)) {                        \
      |                                        ^~
convert/source-ext2.c:769:9: note: in expansion of macro 'EXT4_COPY_XTIME'
  769 |         EXT4_COPY_XTIME(atime, dst, tv_sec, tv_nsec);
      |         ^~~~~~~~~~~~~~~
convert/source-ext2.c:733:40: error: 'i_mtime_extra' undeclared (first use in this function)
  733 |         if (inode_includes(inode_size, i_ ## xtime ## _extra)) {                        \
      |                                        ^~
convert/source-ext2.c:770:9: note: in expansion of macro 'EXT4_COPY_XTIME'
  770 |         EXT4_COPY_XTIME(mtime, dst, tv_sec, tv_nsec);
      |         ^~~~~~~~~~~~~~~
convert/source-ext2.c:733:40: error: 'i_ctime_extra' undeclared (first use in this function)
  733 |         if (inode_includes(inode_size, i_ ## xtime ## _extra)) {                        \
      |                                        ^~
convert/source-ext2.c:771:9: note: in expansion of macro 'EXT4_COPY_XTIME'
  771 |         EXT4_COPY_XTIME(ctime, dst, tv_sec, tv_nsec);
      |         ^~~~~~~~~~~~~~~
convert/source-ext2.c:774:40: error: 'i_crtime_extra' undeclared (first use in this function)
  774 |         if (inode_includes(inode_size, i_crtime_extra)) {
      |                                        ^~~~~~~~~~~~~~

from https://github.com/tytso/e2fsprogs/commit/ca8bc9240a00665dd4c96de350e610add8543a08

Fix inode_includes() macro to properly wrap "inode" parameter, and rename to ext2fs_inode_includes() to avoid potential name clashes. Use this to check inode field inclusion in debugfs instead of bare constants for inode field offsets.

kdave commented 1 month ago

Thanks for the heads-up. There are some trick around the inode_includes in btrfs-progs, a check in configure that defines that eventually. With the update it adds prefix ext2fs_ so it'll be another thing to check for, we need to support e2fsprogs < 1.47.1. As a simplest fix adding the prefixed version as well so whatever is needed will be used.

kdave commented 1 month ago

Can you please test if https://github.com/kdave/btrfs-progs/commit/51b0c6f61eee2d60cb67109da15e21b1e8101bcc fixes the build? (also in branch ci/convert-e2fsprogs-1471)

katexochen commented 1 month ago

Can you please test if 51b0c6f fixes the build? (also in branch ci/convert-e2fsprogs-1471)

Yes, builds fine in mine environment. Thanks for the quick fix!

kdave commented 1 month ago

Thanks, fix added to devel.