hpc / mpifileutils

File utilities designed for scalability and performance.
https://hpc.github.io/mpifileutils
BSD 3-Clause "New" or "Revised" License
164 stars 64 forks source link

missing symbol attr_copy_action #510

Open holgerBerger opened 2 years ago

holgerBerger commented 2 years ago

when checking out and configuring with

cmake ../mpifileutils \ -DENABLE_LUSTRE=ON \ -DWITH_DTCMP_PREFIX=../install \ -DWITH_LibCircle_PREFIX=../install \ -DCMAKE_INSTALL_PREFIX=../install make install

I get

[ 43%] Linking C executable dbcast ../common/libmfu.so.3.0.0: undefined reference to `attr_copy_action'

I see this commit

96a9fe7d (Olaf Faaland 2021-10-04 18:59:34 -0700 350) if (attr_copy_action(name, &ctx) == ATTR_ACTION_SKIP) { 96a9fe7d (Olaf Faaland 2021-10-04 18:59:34 -0700 351) copy_xattr = 0; 96a9fe7d (Olaf Faaland 2021-10-04 18:59:34 -0700 352) }

I think something is missing here? This symbol appears only at this line in the source.

Actually I am very much interested in that functionality, as I try to copy a lustre into a XFS which fails with older version as -p tries to copy some lustre attributes to the XFS target.

Is the mistake on my side? Or is something missing?

using Rocky Linux release 8.4 (Green Obsidian)

$ rpm -qa | grep libattr libattr-2.4.48-3.el8.x86_64 libattr-devel-2.4.48-3.el8.x86_64 libattr-2.4.48-3.el8.i686

adammoody commented 2 years ago

Thank you for the reporting this issue, @holgerBerger . On my system it happens to link, but I suspect we may need to add a proper CMake check and then add libattr to the link.

Does it work around the problem if you hack the top-level CMakeLists.txt file to add

LIST(APPEND MFU_EXTERNAL_LIBS attr)

as below?

OPTION(ENABLE_XATTRS "Enable code for extended attributes" ON)
IF(ENABLE_XATTRS)
  ADD_DEFINITIONS(-DDCOPY_USE_XATTRS)
  LIST(APPEND MFU_EXTERNAL_LIBS attr)
ENDIF(ENABLE_XATTRS)
holgerBerger commented 2 years ago

yes, that helps!

adammoody commented 2 years ago

Thanks, @holgerBerger . I've got a PR in the works with a more proper fix here:

https://github.com/hpc/mpifileutils/pull/511

I'd like to double-check that this fix also works for you. If it's easier, we could merge it first and fix things up if it doesn't work. We should be able to get this merged sometime in the first week of January. Thanks!

holgerBerger commented 2 years ago

I noticed another glitch: the include path search order has the install directory before the source directory. That probably works most of the time, but not in case structs get new members in the headers as with that patch, as the old include files are found first from previous installs. To make things compile one has to delete old header files first from install location (which feels a bit "wrong") or copy new ones to install location. But I did not immediately spot how that happens in the cmake file, may be it is rather implicit and not easy to change.

adammoody commented 2 years ago

Yes, that has annoyed me, as well. I think I might be able to fix that with adding a BEFORE keyword in the INCLUDE_DIRECTORIES. I'll take a look at that.

adammoody commented 2 years ago

@holgerBerger , I think the second commit in https://github.com/hpc/mpifileutils/pull/514 should resolve that.