libarchive's CMakeLists.txt finds major() when it shouldn't

[kwrobot]

Original issue 125 created by Google Code user audiofanatic on 2011-01-04T08:44:44.000Z:

When building the cmlibarchive project with LSB compilers (LSB = Linux Standards Base), the archive_entry.c file generates compiler errors because it relies on the following functions which are not provided by the LSB:

major
minor
makedev

On linux, these are generally implemented as macros which forward to functions like gnu_dev_makedev, etc., but they actually have very simple inlineable implementations. In fact, with certain GCC flags, these macros/functions *are* fully inlined. It would seem that this has been noted by the cmlibarchive developers too, since they have recently added the following to archive_entry.c

#if !defined(HAVE_MAJOR) && !defined(major)
/* Replacement for major/minor/makedev. */
#define major(x) ((int)(0x00ff & ((x) >> 8)))
#define minor(x) ((int)(0xffff00ff & (x)))
#define makedev(maj,min) ((0xff00 & ((maj)<<8)) | (0xffff00ff & (min)))
#endif

The HAVE_MAJOR switch is the problem for LSB compilers. It is set earlier in archive_entry.c and it merely depends on one of MAJOR_IN_MKDEV or MAJOR_IN_SYSMACROS being defined. Unfortunately, the top level CMakeLists.txt file does this detection without considering LSB compilers, since the LSB does not provide either mkdev.h nor sysmacros.h, but the CMakeLists.txt file doesn't account for this. The result is that system versions of these headers can be found, which is incorrect/dangerous when using LSB compilers. This is easy to fix with the attached patch to the top level CMakeLists.txt file.

Note that this bug was originally reported to KitWare since it affects CMake itself. They have requested that this issue be fixed in cmlibarchive itself since they import cmlibarchive sources. For reference, see here:

http://public.kitware.com/Bug/view.php?id=11648

See attachment: CMakeLists.txt.patch

[kwrobot]

Comment #1 originally posted by bradking on 2011-01-04T14:06:56.000Z:

The name of this project is "libarchive", not "cmlibrachive".  The latter is the mangled name CMake uses in its optionally builtin copy.  Nevertheless, this issue affects plain libarchive too.

From CMakeLists.txt.patch:

+GET_FILENAME_COMPONENT(ccomp_lsbcheck ${CMAKE_C_COMPILER} NAME)
+IF(NOT ccomp_lsbcheck STREQUAL "lsbcc")

This is not a good way to determine if the compiler is LSB.  However, it should not matter.  If LSB does not provide sys/mkdev.h then how can

  CHECK_SYMBOL_EXISTS(major            "sys/mkdev.h"     MAJOR_IN_MKDEV)

pass at all?  It should fail.

[kwrobot]

Comment #2 originally posted by kientzle on 2011-01-04T16:50:57.000Z:

Libarchive should already build on systems that lack major/minor/makedev, which is why the the code section you quote was added in late 2009.  The patch you propose is unfortunately not a suitable fix:  knowing the compiler does not determine what headers and/or standard library functions are available.

Have you tried to reproduce this with the libarchive sources from libarchive.googlecode.com?  (Version 2.8.4 is the most recent stable version.)

What exact error are you seeing when you try to build?

Do you see errors when you try to configure libarchive using CMake?  Using the provided configure script?

What version of libarchive are you using?

Does your system have either sys/mkdev.h or sys/sysmacros.h?

It appears you encountered this when trying to build CMake; have you tried building CMake with a binary package of libarchive (most Linux distros have some version of libarchive in their package systems)?

Can you provide us more detailed instructions on how to reproduce this (e.g., on Ubuntu 10.10, install the "XXX" packages and then configure libarchive with this command line).

[kwrobot]

Comment #3 originally posted by Google Code user audiofanatic on 2011-01-04T23:41:13.000Z:

In the case of the lsbcc and lsbc++ compilers, knowing the compiler tells you precisely what headers and libraries are available because that is exactly what the LSB does for you. The whole point of the LSB is to define a set of headers, libraries, types, constants, etc. that the code can rely on being present. The other part of that however is that when you want to build an application to be LSB-compliant, it can *only* use those headers, libraries, etc. that the LSB spec includes.

The problem with the top level CMakeLists.txt file is that it doesn't differentiate between the headers that the LSB build packages provide and those that the system provides. When using the lsbcc compiler, the headers must come from /opt/lsb/include and no system headers should be referenced - if system headers were pulled in, that means the application is then potentially relying on things not in the LSB and hence you lose the whole point of using the lsb compiler in the first place. With the patch I provided, if the lsb compiler is detected, the result will be that both MAJOR_IN_MKDEV and MAJOR_IN_SYSMACROS will be undefined. This is what we want, because that will result in the HAVE_MAJOR symbol in archive_entry.c being undefined as well, and this will activate the block of #define's which provide the major, minor and makedev macros in archive_entry.c that I showed in my original bug report.

As things currently stand, the lsbcc compiler/linker complains because if major, minor and makedev are not inlined, they end up referencing functions which the LSB does not provide and hence you get unresolved symbols in the final binary. The things needed are already in archive_entry.c, we just need to fix the detection of sys/sysmacros.h and sys/mkdev.h which, as things stand, return incorrect results when using the LSB compiler. This is because the *system* include area *does* provide sys/sysmacros.h for example, but this header is not allowed to be used.

My concern is not with building libarchive as a separate project, I need to get CMake itself building with LSB compilers. The CMake project imports the libarchive sources into its own source tree, but fixing this problem for CMake requires fixing the situation within the libarchive project. I cannot use binary libarchive packages because I need to ensure everything is built with the LSB compiler (this gives compatability guarantees for the resultant binary).

To reproduce this problem, start by working on any LSB-4.0 compliant linux distribution. I've been using SLED 11, but there are others as well. See here for the list of certified distributions:

https://www.linuxfoundation.org/lsb-cert/productdir.php?by_lsb

Then you need to download the source for CMake 2.8.3:

http://www.cmake.org/files/v2.8/cmake-2.8.3.tar.gz

You will also need to install the LSB build packages, which are available from the following location (I'm using the packages in repo-x86_64):

http://ftp.linuxfoundation.org/pub/lsb/repositories/yum/lsb-4.1

You will need the lsb-build-* packages, all of which will create things in /opt/lsb. Note that I'm using my own Qt libraries too, since they also need to be built from source with LSB compilers. If you want the details of how to do that as well, you can see a summary bug report in Qt's bug tracker here, but beware that it is perhaps not for the faint-hearted:

http://bugreports.qt.nokia.com/browse/QTBUG-16385

I suspect that's more work than you want to go through. That's why I've provided a patch for you which hopefully is minimal and which also should have no side effects when not working with LSB compilers. ;)

[kwrobot]

Comment #4 originally posted by Google Code user audiofanatic on 2011-01-04T23:54:39.000Z:

I should add that I'm more than happy to be shown a better way to detect the LSB compiler. ;)

[kwrobot]

Comment #6 originally posted by Google Code user audiofanatic on 2011-01-05T00:28:50.000Z:

This would probably be a more robust way to detect the LSB compiler:

INCLUDE(CheckCSourceRuns)
CHECK_C_SOURCE_RUNS("int main() { return __LSB_VERSION__ ? 0 : 1; }" HAVE_LSB_COMPILER)
IF(NOT HAVE_LSB_COMPILER)
 ...

[kwrobot]

Comment #7 originally posted by jsonn on 2011-01-05T03:27:32.000Z:

I'm sorry, but to me this looks like a clear WONTFIX. If the LSB compiler is not enforcing the visibility, it is broken and useless. There is no reason why any portable open source package should have to hard code headers just for the sake of LSB. It would completely defeat the purpose.

For that reason, I would suggest to start by filling a Problem Report against LSB itself...

[kwrobot]

Comment #8 originally posted by kientzle on 2011-01-05T03:38:27.000Z:

"When using the lsbcc compiler, the headers must come from /opt/lsb/include and no system headers should be referenced..."

I'm still confused.  Are you using the LSB compiler during configuration?  If the LSB compiler only accesses headers in /opt/lsb/include, and those headers aren't there, then I don't understand how the configuration is finding those headers.

"... and hence you get unresolved symbols in the final binary ..."

What unresolved symbols are you seeing?

If you're seeing problems with major(), then something is deeply screwed up.  The existing CMakeLists.txt causes CMake to compile and link a C program that references the major() function.  If that succeeds at configure time, but not at build time, then the configuration process is going badly awry and you are probably not going to get a functioning libarchive at the end.

If the error involves the minor() or makedev() functions, then we should add detection for those functions, not for the name of the environment.

[kwrobot]

Comment #9 originally posted by Google Code user audiofanatic on 2011-01-05T03:47:43.000Z:

Joerg,

Sorry, perhaps my explanations were not sufficiently clear. Let's have another go. ;) 

The LSB is a specification that all the major linux distributions aim to meet. They will frequently go beyond what the LSB requires, but the LSB is meant to reflect a minimum set of headers, interfaces, libraries, commands, etc. that is common across linux distributions so that a single RPM package can be used on all LSB-compatible linux flavours. The primary reason for the existence of the LSB compilers is to allow application developers to target this specific standard and ensure that the binaries produced will be compatible with that specification.

In practice, the lsb compiler is simply forwarding through to the gcc compilers with appropriate flags added. The compiler is far from broken! It is enforcing precisely the restrictions that the developer wants it to so that their binaries meet the LSB spec. Don't think of it as the compiler not seeing things that exist on the system, think of it as the compiler *ensuring* that system things are not seen and instead the LSB-provided standard headers, etc. are used instead. The headers, libraries, etc. that the LSB build packages provide are purpose-built to reflect the LSB spec, which has its roots in common practice among the various linux distributions. If something is not present in the LSB spec, it is often for good reason - including that it might not be widely supported among linux distributions or implemented inconsistently, etc.

[kwrobot]

Comment #10 originally posted by Google Code user audiofanatic on 2011-01-05T04:14:36.000Z:

kientzle,

The headers will be there in /opt/lsb/include if the lsb compiler is being used. The LSB build packages are structured in such a way that the C compiler package (lsb-build-cc) depends on the package that contains the headers (lsb-build-base).

The symbols that are unresolved are dependent on the linux distribution, but typically they will be something like:

gnu_dev_major
gnu_dev_minor
gnu_dev_makedev

Note that major(), minor() and makedev() are typically implemented as macros rather than functions on most linux distributions. The LSB spec does NOT provide any of these three macros, nor does it provide the gnu_dev_* functions. There have been discussions about these things (see for example http://bugs.linuxbase.org/show_bug.cgi?id=2166 ), including over the past week on the lsb-discuss mailing list. The washup from all this is that since these macros have inlineable implementations, the gnu_dev_* functions do not need to be added to the LSB spec. It is an open discussion whether the three macros should be added, but the reality is that changes to the spec filter through very slowly. It can take years for linux distributions to get certified for the next LSB release in some cases, and the practical reality is that app developers typically can't wait that long for a build problem to be fixed!

So there isn't a problem with major() per se. The problem is more about how it got defined and was it in an LSB-compatible way. To be absolutely crystal clear, the current implementation in archive_entry.c is fine, as long as the detection for MAJOR_IN_MKDEV and MAJOR_IN_SYSMACROS is reliable. In the case of the lsb compilers, the way these tests are currently implemented in the top level CMakeLists.txt file will allow system headers to be pulled in when they shouldn't. The change I propose is very simple and will only affect the case where LSB compilers are used. All other scenarios remain unaffected by the proposed change. Hard to see the down side of accepting the patch....

[kwrobot]

Comment #11 originally posted by kientzle on 2011-01-05T04:54:31.000Z:

What are the actual error messages you are seeing when you try to build with lsbcc?

The downside of your patch is that it doesn't fix any real problem.  Based on your description so far, the existing configuration setup should work for you without your patch.  Clearly it doesn't, which means the real problem is somewhere else.

Without more information, we can't really do anything until one of us has time to try to replicate your environment and figure out what's really going on.

[kwrobot]

Comment #12 originally posted by Google Code user audiofanatic on 2011-01-05T05:14:32.000Z:

The existing setup does not work without the patch when using lsb compilers. I downloaded the libarchive 2.8.4 source release and built it successfully with the default compiler on SLED 11 SP1. I then wiped the build completely and tried again, this time with the LSB compiler. Here's the build error that results:

Linking C executable ../../bin/libarchive_test                                                                        
CMakeFiles/libarchive_test.dir/__/archive_entry.c.o: In function `archive_entry_dev':                                 
archive_entry.c:(.text+0xa93): undefined reference to `gnu_dev_makedev'
CMakeFiles/libarchive_test.dir/__/archive_entry.c.o: In function `archive_entry_devmajor':
archive_entry.c:(.text+0xadd): undefined reference to `gnu_dev_major'
CMakeFiles/libarchive_test.dir/__/archive_entry.c.o: In function `archive_entry_devminor':
archive_entry.c:(.text+0xb1e): undefined reference to `gnu_dev_minor'
CMakeFiles/libarchive_test.dir/__/archive_entry.c.o: In function `archive_entry_rdev':
archive_entry.c:(.text+0xe09): undefined reference to `gnu_dev_makedev'
CMakeFiles/libarchive_test.dir/__/archive_entry.c.o: In function `archive_entry_rdevmajor':
archive_entry.c:(.text+0xe5f): undefined reference to `gnu_dev_major'
CMakeFiles/libarchive_test.dir/__/archive_entry.c.o: In function `archive_entry_rdevminor':
archive_entry.c:(.text+0xea6): undefined reference to `gnu_dev_minor'
collect2: ld returned 1 exit status
make[2]: *** [bin/libarchive_test] Error 1
make[1]: *** [libarchive/test/CMakeFiles/libarchive_test.dir/all] Error 2
make: *** [all] Error 2


I then wiped the build again, put in my patch to CMakeLists.txt and the build completed without error.

[kwrobot]

Comment #13 originally posted by kientzle on 2011-01-05T07:54:33.000Z:

Thank you!

The configuration process specifically checks for major(), and you're seeing a link error for that.  So the configuration is finding that function but the build environment doesn't have it.  There are many possible explanations for this kind of mismatch between configuration and build; some could impact a lot of environments other than just LSB.

Can you please provide the config.h and CMakeCache.txt files that were generated when you configured with the LSB compiler?  (The Google Code interface provides "Attach a file" as an option just below the editor.)

Can you tell us the exact commands you used to configure and build with the LSB compiler, including any environment variables you set?

[kwrobot]

Comment #14 originally posted by Google Code user audiofanatic on 2011-01-05T09:02:18.000Z:

I'm doing an out of source build so keep that in mind. Other than that, nothing special (ie no env changes).

cmake -DCMAKE_C_COMPILER:PATH=/opt/lsb/bin/lsbcc ../libarchive-2.8.4
make

See attachment: config.h
See attachment: CMakeCache.txt

[kwrobot]

Comment #15 originally posted by Google Code user audiofanatic on 2011-01-05T10:36:41.000Z:

Sorry, the config.h and CMakeCache.txt that I attached previously were for the *patched* sources. Here's the same files when using unpatched sources as per the 2.8.4 release. A diff between the two sets is also attached for convenience.

See attachment: config.h
See attachment: CMakeCache.txt
See attachment: afterCMake.patch

[kwrobot]

Comment #16 originally posted by kientzle on 2011-01-05T17:47:16.000Z:

That helps.  I see one obvious problem: this line is finding major in sys/sysmacros.h when it shouldn't:

CHECK_SYMBOL_EXISTS(major            "sys/sysmacros.h" MAJOR_IN_SYSMACROS)

So we need to narrow down why CHECK_SYMBOL_EXISTS is not working correctly for you; it could be a cmake problem (unlikely), some oddity in your environment (unlikely), or there's some error in libarchive's CMakeLists.txt that's causing this to behave in an unexpected manner.

Could you please try the five-line CMakeLists.txt file below?  Just put it into an empty directory, run 

cmake -DCMAKE_C_COMPILER:PATH=/opt/lsb/bin/lsbcc .

and let me know the results.

PROJECT(testproject C)
CMAKE_MINIMUM_REQUIRED(VERSION 2.6.3 FATAL_ERROR)
SET(CMAKE_BUILD_TYPE "Debug")
INCLUDE(CheckSymbolExists)
CHECK_SYMBOL_EXISTS(major            "sys/sysmacros.h" MAJOR_IN_SYSMACROS)

The results of this experiment will help narrow down exactly where the problem is.

[kwrobot]

Comment #17 originally posted by Google Code user audiofanatic on 2011-01-05T21:14:03.000Z:

The CMakeCache.txt from that is attached - it still says the symbol exists.

I did some digging into CMake and I think I can see what's happening. The CHECK_SYMBOL_EXISTS test effectively reduces down to this:

#include <sys/sysmacros.h>

void cmakeRequireSymbol(int dummy,...){(void)dummy;}
int main()
{
#ifndef major
  cmakeRequireSymbol(0,&major);
#endif
  return 0;
}

Since major is a #define macro, the above test never actually tries to link anything with it. In fact, the test merely confirms that major is defined but never verifies whether it is useable. I guess that's in keeping with the name of the cmake function, since it is only checking that it *exists*, not that it can be used.

The question here is whether a different test is needed. Perhaps a TRY_COMPILE would be more robust? I've attached two other files which show how this could be done. When I used this approach, MAJOR_IN_SYSMACROS was defined for the non-lsb compiler and was undefined for LSB compiler (which is what we are aiming for here). The approach may need to be cleaned up a bit, but hopefully you get the idea.

See attachment: CMakeCache.txt
See attachment: checkExists.c
See attachment: CMakeLists.txt

[kwrobot]

Comment #18 originally posted by Google Code user audiofanatic on 2011-01-05T21:19:02.000Z:

Actually, a cleaner version would be this (no need for a separate external file and let cmake do more of the scaffolding):

PROJECT(testproject C)
CMAKE_MINIMUM_REQUIRED(VERSION 2.6.3 FATAL_ERROR)
SET(CMAKE_BUILD_TYPE "Debug")
INCLUDE(CheckCSourceCompiles)
CHECK_C_SOURCE_COMPILES("#include <sys/sysmacros.h>\nint main() { return major(256); }" MAJOR_IN_SYSMACROS)

[kwrobot]

Comment #19 originally posted by kientzle on 2011-01-05T22:28:23.000Z:

Excellent!  Now we're really getting somewhere.  I suspect this may be affecting other people who are using non-default compilers, so I'm pretty excited about getting a robust solution.   (We've had a few questions from people trying to build libarchive using cross-compilers but never gotten enough information to be able to track down the issue.)

Now that you've identified the general issue, I was able to track down some related documentation.  I wonder if CHECK_FUNCTION_EXISTS would be an even better choice here?

It seems that there's a natural flow that we should be following throughout the CMakeLists.txt file:
  * Check for header files, build up CMAKE_REQUIRED_INCLUDES with header files that exist
  * Check for libraries, build up CMAKE_REQUIRED_LIBRARIES with libraries that exist
  * Check for required functions (exploiting the above two settings)

I wonder if we're miusing CHECK_SYMBOL_EXISTS in other places?  Hmmm...

[kwrobot]

Comment #20 originally posted by kientzle on 2011-01-08T22:10:27.000Z:

Please try the attached patch to libarchive's CMakeLists.txt and let me know how this works.  I believe this addresses the issue you found and also fixes several other misuses of CHECK_SYMBOL_EXISTS.

See attachment: cmake_verify_linkage.patch

[kwrobot]

Comment #21 originally posted by Google Code user audiofanatic on 2011-01-11T23:09:45.000Z:

It looks like this patch is already in the latest SVN, so I've tested that. The cmake step goes okay, but I can't verify the build due to build errors in the current SVN head. The end of the compiler output with the error I see follows:

[  3%] Building C object libarchive/CMakeFiles/archive.dir/archive_read_disk_posix.c.o                                
/local_scratch/sco265/libarchive-read-only/libarchive/archive_read_disk_posix.c: In function ‘setup_current_filesystem’:
/local_scratch/sco265/libarchive-read-only/libarchive/archive_read_disk_posix.c:1029: error: storage size of ‘sfs’ isn’t known
cc1: warnings being treated as errors
/local_scratch/sco265/libarchive-read-only/libarchive/archive_read_disk_posix.c:1034: error: implicit declaration of function ‘statfs’
/local_scratch/sco265/libarchive-read-only/libarchive/archive_read_disk_posix.c:1029: error: unused variable ‘sfs’
make[2]: *** [libarchive/CMakeFiles/archive.dir/archive_read_disk_posix.c.o] Error 1
make[1]: *** [libarchive/CMakeFiles/archive.dir/all] Error 2
make: *** [all] Error 2

[kwrobot]

Comment #22 originally posted by Google Code user audiofanatic on 2011-01-11T23:13:29.000Z:

FYI, if you want to know if a particular function, macro, type, variable, header, etc. is provided by the LSB compilers, you can put in the symbol in the search box of the LSB navigator and get all the info you need:

http://dev.linuxfoundation.org/navigator/commons/welcome.php

[kwrobot]

Comment #23 originally posted by bradking on 2011-01-12T13:39:16.000Z:

Tim, in regard to SVN commit 2866, here is some CMake history.

CHECK_FUNCTION_EXISTS was created first, but it doesn't work for "functions" that the implementation happens to choose to provide as macros, like "isnan" IIRC.  It also only checked that the standard libraries contain the given function, not that a header actually declares it such that it can be called.

CHECK_SYMBOL_EXISTS was intended to be a successor to CHECK_FUNCTION_EXISTS that ensures the symbol is declared in a header and accounts for possible definition as a macro.  It *does* link an executable, but if the header defines the symbol as a macro then the test assumes it works and doesn't try to take the address of a macro.

Clearly there is a use for both.  Ultimately the only 100% safe check for any specific function is to write a custom source file that includes headers and actually tries invoking the function with the expected signature.  This is what your commit does for 'major', which is good.  For other symbols like strftime then whether to use CHECK_FUNCTION_EXISTS or CHECK_SYMBOL_EXISTS is not clear-cut based just on linking.

[kwrobot]

Comment #24 originally posted by kientzle on 2011-01-12T16:48:12.000Z:

Michihiro has fixed the problem with statfs()  (a missing check).  Please try again.

Brad:  Thanks for the history; I'll review that commit again with this in mind.

[kwrobot]

Comment #25 originally posted by Google Code user audiofanatic on 2011-01-13T21:50:03.000Z:

Nope, Michihiro's changes made no difference. I still get the same build error. Looking at the #elif condition around line 1107, the wrong header is being checked. For statfs, you need to check for sys/statfs.h, not sys/vfs.h. It should also be noted that statvfs would be available, so it would be even better if the #if logic resulted in code that used statvfs instead of statfs. There is already a check to see if sys/statvfs.h is present, so combining HAVE_SYS_STATVFS_H and HAVE_STATVFS would seem to be the appropriate test for LSB (but that's just my relatively naive guess).

Note also that the statvfs struct is available in the LSB too, so that adds extra weight towards using statvfs instead of statfs. That said, both should work (at least until the deprecation turns into full removal from the LSB).

[kwrobot]

Comment #26 originally posted by Google Code user audiofanatic on 2011-01-13T22:04:56.000Z:

Sorry, forgot to say which file I was looking in! It was archive_read_disk_posix.c

[kwrobot]

Comment #27 originally posted by kientzle on 2011-01-26T04:20:31.000Z:

libarchive/trunk r2946 has another attempt to fix the statvfs issue.  Please let us know.

[kwrobot]

Comment #28 originally posted by Google Code user audiofanatic on 2011-01-26T09:06:00.000Z:

Now a slightly different error:

/local_scratch/sco265/libarchive-read-only/libarchive/archive_read_disk_posix.c: In function ‘setup_current_filesystem’:
/local_scratch/sco265/libarchive-read-only/libarchive/archive_read_disk_posix.c:1448: error: ‘fstatfs’ is deprecated (declared at /opt/lsb/include/sys/statfs.h:272)
/local_scratch/sco265/libarchive-read-only/libarchive/archive_read_disk_posix.c:1461: error: ‘fstatfs’ is deprecated (declared at /opt/lsb/include/sys/statfs.h:272)
/local_scratch/sco265/libarchive-read-only/libarchive/archive_read_disk_posix.c: In function ‘tree_current_stat’:
/local_scratch/sco265/libarchive-read-only/libarchive/archive_read_disk_posix.c:2092: error: implicit declaration of function ‘fstatat’


The first couple of errors related to fstatfs deprecation should probably be warnings rather than errors, but the LSB state of fstatfs seems a bit confused. According to the LSB navigator, fstatfs appeared in LSB 1.0, was withdrawn in 2.0 and then re-appeared in 3.1! It recommends using fstatvfs instead, so basically it's a similar situation as for statfs/statvfs.

The missing declaration for fstatat is because it is not and never has been in the LSB (and it also only became available in the linux kernel as of 2.6.16 according to my Fedora 14 man page). At that point in the source code, there is a test HAVE_FSTATAT which appears to be returning the incorrect result for LSB, since it isn't available and yet HAVE_FSTATAT is defined. The alternative #else block is also a bit of a grey area for the LSB, since the stat() function is also not present in the LSB headers, but potentially it should be. I've asked the lsb-discuss mailing list for some clarification and will report back here once things become clearer. The relevant link and quote follow:

http://refspecs.linux-foundation.org/LSB_4.0.0/LSB-Core-generic/LSB-Core-generic/baselib---xstat.html

"This interface is source-level only and should not be visible on binary level. It should be transformed to call to __xstat at compilation time."

[kwrobot]

Comment #29 originally posted by Google Code user audiofanatic on 2011-01-26T09:08:31.000Z:

Oh, wait, looks like the same situation for fstatat():

http://refspecs.linux-foundation.org/LSB_4.0.0/LSB-Core-generic/LSB-Core-generic/baselib---fxstatat-1.html

So fstatat() should be available. I'll seek clarification on lsb-discuss for that too.

[kwrobot]

Comment #30 originally posted by Google Code user audiofanatic on 2011-01-26T21:34:59.000Z:

Turns out the whole fstatat() and stat() situation with LSB is rather complicated. For the details, see here:

http://bugs.linuxbase.org/show_bug.cgi?id=2426

Basically, fstatat() and stat() are "magically" transformed to __fxstat() and __xstat() by the LSB-specific libc_nonshared.a library, so you can call fstatat() and stat() normally as you would expect. Unfortunately, the LSB headers currently don't contain a prototype for fstatat(), but this seems to be an oversight in the LSB packages. While that is annoying, libarchive should still be able to handle this by properly setting HAVE_FSTATAT using CHECK_C_SOURCE_COMPILES and fall back to stat() when there is no fstatat(). There *is* a function prototype for stat() in the LSB headers, so this should work with the libarchive code as it currently stands once HAVE_FSTATAT is being set correctly.

[kwrobot]

Comment #31 originally posted by Google Code user audiofanatic on 2011-01-26T21:53:22.000Z:

To be more specific about fstatat(), the CMakeLists.txt currently uses the following to test for it:

CHECK_FUNCTION_EXISTS_GLIBC(fstatat HAVE_FSTATAT)

With LSB, fstatat() doesn't exist in GLIBC, nor in *any* binary (as previously stated, it gets magically transformed to call a different function). I think this might be a similar situation to what we saw with the original part of this bug report. The current way of testing for fstatat() doesn't seem to be testing robustly. Ideally, it should try to compile a dummy program which includes a genuine function call to fstatat(). I must admin that I don't quite follow how CHECK_FUNCTION_EXISTS_GLIBC works - I can see that it does use a TRY_COMPILE, but this doesn't seem to be working 100% in this case.

[kwrobot]

Comment #32 originally posted by kientzle on 2011-01-27T06:20:54.000Z:

"Ideally, it should try to compile a dummy program which includes a genuine function call to fstatat()."

You're absolutely right.  Unfortunately, I don't have time to work on this right now.  If you'd like to contribute patches to implement this, I'm happy to consider them.

I suggest you look at the changes I committed for major() and try doing something similar with CHECK_C_SOURCE_COMPILES.  TRY_COMPILE can do more, but it's considerably more complex to use.

[kwrobot]

Comment #33 originally posted by Google Code user audiofanatic on 2011-01-27T07:01:08.000Z:

Here's a patch which results in things building okay with LSB compilers. Note that we have to explicitly include the "-Wimplicit -Werror" flags because the LSB compilers emit deprecation warnings for statfs() and fstatfs(), which get turned into errors in the real build. The patch does this but perhaps not in a way which you'd like because it will apply to all subsequent tests (which might not be a bad thing actually).

Also note that the header paths used in the tests CHECK_C_SOURCE_COMPILES tests are those that the LSB says should be used for these functions. Hence, they should be correct for most unix variants. Not sure if that's enough for you, but since you were previously testing against GLIBC (ie unix-specific anyway), I suspect it should be fine.

Anyway, this patch works for me. Hope its enough for you to see what needs to be done.

See attachment: CMakeLists.txt.patch

[kwrobot]

Comment #34 originally posted by kientzle on 2011-01-27T16:59:02.000Z:

Unconditionally setting compiler flags won't work here.  We build with a lot of different compilers (Visual C++, for example) that have wildly different compiler options.

But this is certainly an improvement.  We're getting closer.

[kientzle]

Closing old issues.