pauldreik
Define "feature macros" for symlink, make it compilable on cygwin
Define _XOPEN_SOURCE and _POSIX_C_SOURCE appropriately in order to be able to use functions such as symlink() on cygwin.
Before that it would error while running make about not being able to find a function called symlink. After looking into the header files, I've found it requires at least one of those macros in order to be defined.
The error was:
g++ -std=c++11 -DHAVE_CONFIG_H -I. -g -O2 -MT Fileinfo.o -MD -MP -MF .deps/Fileinfo.Tpo -c -o Fileinfo.o Fileinfo.cc
Fileinfo.cc: In lambda function:
Fileinfo.cc:280:14: error: ‘symlink’ was not declared in this scope
return symlink(target.c_str(), filename.c_str());
^~~~~~~
Fileinfo.cc:280:14: note: suggested alternative: ‘unlink’
return symlink(target.c_str(), filename.c_str());
^~~~~~~
unlink
Additionally, man 2 symlink (on Ubuntu 18.04) says, it'll be defined for:
_XOPEN_SOURCE >= 500 || _POSIX_C_SOURCE >= 200112L
|| /* Glibc versions <= 2.19: */ _BSD_SOURCE
However, defining _BSD_SOURCE would cause cygwin sys/feature.h to define _DEFAULT_SOURCE, which in turns always redefines _POSIX_C_SOURCE, causing an redefinition warning when the config.h file is processed again. Thus _BSD_SOURCE isn't defined, for now.
This is also my first time modifying autoconfig files. I hope I did everything alright. Note that it just builds and is functional (can find duplicates), but I haven't tried anything advanced yet (like dryrun, actually soft or hard symlinking).
Sorry for replying so late.
Is this how those macros are supposed to be used (set by the user)?
Regardless, it may be wise to check for symlink support during configure time so rdfind can compile also without it. That would require some kind of mechanism to inhibit the symlink tests.
For a code change like this, some kind of CI for testing this is needed. Preferably cygwin.
Sorry for replying so late.
I already forgot about this.
Is this how those macros are supposed to be used (set by the user)?
According to the linked man page 7 feature_test_macros, these macros should just simply defined before including the appropriate header, like either on the command line or a simple #define (which seems what I've done).
It also says that _POSIX_C_SOURCE is in the POSIX.1 standard, but _BSD_SOURCE is an extension.
On cygwin I get the 3p version of the man page which doesn't say anything about it, but also claims that the cygwin implementation can differ.
With my initial comment above, it seems that defining _POSIX_C_SOURCE should be enough in most cases, except maybe very legacy systems(/BSDs).
Regardless, it may be wise to check for symlink support during configure time so rdfind can compile also without it. That would require some kind of mechanism to inhibit the symlink tests.
I believe autotools has such functionality available, but we still need to define those macros above so in case it is available it can be used.
For a code change like this, some kind of CI for testing this is needed. Preferably cygwin.
I agree, but I haven't looked on how you would do that yet. AFAIK appveyor can run windows MSVC, but not sure if we can also use cygwin on there.
Thanks for providing information on how to use the macros!
I removed appveyor and moved what it ran into github actions instead. Do you think something https://github.com/egor-tensin/setup-cygwin could be used?
This is a continuation of #56 as well, but fits better in this thread.
I looked a bit more into the specific definitions that are needed to enable symlink. Symlink is defined in sys/unistd.h. In the version of that header that is used by MSYS and Cygwin the function definition is guarded by
#if __BSD_VISIBLE || __POSIX_VISIBLE >= 200112 || __XSI_VISIBLE >= 4
Note that this is a bit different than the definition that @Neui found on Ubuntu 18.04.
For our purposes the most important one is __POSIX_VISIBLE . This is defined in sys/features.h:
#if (_POSIX_C_SOURCE - 0) >= 200809L
#define __POSIX_VISIBLE 200809
#elif (_POSIX_C_SOURCE - 0) >= 200112L
#define __POSIX_VISIBLE 200112
#elif (_POSIX_C_SOURCE - 0) >= 199506L
#define __POSIX_VISIBLE 199506
#elif (_POSIX_C_SOURCE - 0) >= 199309L
#define __POSIX_VISIBLE 199309
#elif (_POSIX_C_SOURCE - 0) >= 2 || defined(_XOPEN_SOURCE)
#define __POSIX_VISIBLE 199209
#elif defined(_POSIX_SOURCE) || defined(_POSIX_C_SOURCE)
#define __POSIX_VISIBLE 199009
#else
#define __POSIX_VISIBLE 0
#endif
You can see here that this means we should set _POSIX_C_SOURCE to 200112. Defining _XOPEN_SOURCE , _BSD_SOURCE and/or _DEFAULT_SOURCE should not be needed, and in fact enables more extensions than are necessary, as also explained on https://www.gnu.org/software/libc/manual/html_node/Feature-Test-Macros.html. Since @pauldreik indicated he's not a fan of extensions, I believe we shouldn't enable more of them than strictly needed.
Sidenote, my earlier solution of just enabling all extensions is equivalent to defining _GNU_SOURCE and undefining __STRICT_ANSI__, which in turn leads to (among others) #define _POSIX_C_SOURCE 200809L, which is of course larger than 200112L.
I tested with ./configure CXXFLAGS="-std=c++11 -D_POSIX_C_SOURCE=200112" and that works fine.
I removed appveyor and moved what it ran into github actions instead. Do you think something egor-tensin/setup-cygwin could be used?
No idea, I've never used GitHub Actions, but the repository looks like it could work.
Because of BartLH I've now added a commit to just keep the _POSIX_C_SOURCE definition (in addition to rebase on top of the current main branch). It compiles on Cygwin and on Ubuntu 20.04. I'm not doing the symlink detection in this specific PR to keep things simple here.
I added a cygwin job in https://github.com/pauldreik/rdfind/pull/136 . It did not require any changes. Help wanted to test if the cygwin executable works, and if it would be possible to execute the tests.
Since no changes were needed to make it compile, I think this PR should be closed. Thanks for the work!