Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Not compiling on Win10 with Cygwin #1347

Closed
frankkopp opened this issue Dec 9, 2019 · 14 comments
Closed

Not compiling on Win10 with Cygwin #1347

frankkopp opened this issue Dec 9, 2019 · 14 comments

Comments

@frankkopp
Copy link

In file included from ..../_GITHUB/spdlog/src/spdlog.cpp:12:0:
..../_GITHUB/spdlog/include/spdlog/details/os-inl.h: In function 'void spdlog::details::os::prevent_child_fd(FILE*)':
..../_GITHUB/spdlog/include/spdlog/details/os-inl.h:139:15: error: 'fileno' was not declared in this scope
auto fd = fileno(f);

fileno is unknown.

Cygwin 3.0.7
CMake 3.15.3

Also copying headers to other project only works for "master" branch. 1.x branch does not work:
#includes are not working. I found a workaround by changing the #inclide paths.

@tt4g
Copy link
Contributor

tt4g commented Dec 9, 2019

FYI.

There is not defined _WIN32 by CMake if using Cygwin compier.
If the target platform is Windows OS, You can define _WIN32 by specifying set(CMAKE_LEGACY_CYGWIN_WIN32 1) in CMakeLists.txt.

Another way is to specify -std=gnu++11 as a compiler option.

@gabime
Copy link
Owner

gabime commented Dec 9, 2019

Also copying headers to other project only works for "master" branch. 1.x branch does not work:
#includes are not working. I found a workaround by changing the #inclide paths.

@frankkopp Please provide more details what is the problem and what is the workaround

@gabime
Copy link
Owner

gabime commented Dec 9, 2019

@tt4g I think I will fix spdlog's CMakeLists.txt and addset(CMAKE_LEGACY_CYGWIN_WIN32 =1) if built under cygwin (and mingw?)

@tt4g
Copy link
Contributor

tt4g commented Dec 9, 2019

I think I will fix spdlog's CMakeLists.txt and addset(CMAKE_LEGACY_CYGWIN_WIN32 =1) if built under cygwin (and mingw?)

@gabime I can't decide if that is a good solution.

I have used MinGW a little with MSYS2.
MinGW library did not work on Windows OS unless the PATH environment variable was set. Because it links with MinGW library. Therefore, the release for Windows OS felt complicated.
This problem also seems to be in Cygwin.

Also, I think MinGW and Cygwin have an ABI compatibility problem with the MSVC library.

However, MSYS2 provides the MSYS2_PATH_TYPE=inherit option that takes over Windows environment variables and allows to use the Windows API with MinGW.

Based on these facts, I believe that the developer decides whether to use the POSIX API or the Windows API, depending on which platform the library built with MinGW or Cygwin is used.

Fortunately, CMake can specify command line option -DCMAKE_LEGACY_CYGWIN_WIN32=0 or 1 when building.
I can't answer meson because I'm not familiar with it.

@frankkopp
Copy link
Author

frankkopp commented Dec 9, 2019

Also copying headers to other project only works for "master" branch. 1.x branch does not work:
#includes are not working. I found a workaround by changing the #inclide paths.

@frankkopp Please provide more details what is the problem and what is the workaround

I'm using CLION and cygwin (newest version incl. cmake and compiler).
There I have a project which compiles fine.
I have copied the spdlog header files from the "master" branch and it works fine

When I use the header files from the 1.x branch (or release tag 1.4.2) I get errors saying the include files are not found.

I'm not very proficient in CMAKE - but docu says "just copy header files".

Here the diff for my changes so my project compiles with the spdlog headers:
patch.diff.txt

@tt4g
Copy link
Contributor

tt4g commented Dec 9, 2019

Here the diff for my changes so my project compiles with the spdlog headers:

There are a few things I noticed.

Some files do not exist in the current master branch, Such as spdlog/details/os-inl.h.
Do you know which commit in master the source file refers to?

And most of the differences in pach.diff.txt show that the include path has been changed to a relative path.
It may be affected by the order of header paths specified to the compiler.
If there is a header file with the same name, it is possible that an unintended file is included.

@frankkopp
Copy link
Author

@tt4g I think I will fix spdlog's CMakeLists.txt and addset(CMAKE_LEGACY_CYGWIN_WIN32 =1) if built under cygwin (and mingw?)

I'm not very proficient in CMAKE and platform topics.

But I know the CYGWIN defines __CYGWIN__.

Googletest for example uses this:
#ifdef __CYGWIN__
....

So I think the issue is not CMAKE or CYGWIN but it is in the *.h files.
I guess the solution would be to support CYGWIN in the *.h files:

Example
SPDLOG_INLINE void prevent_child_fd(FILE *f) { #ifdef _WIN32 #do what is needed for _WIN32 #elif __CYGWIN__ #do what is needed for cygwin #else ... #endif }

Unfortunately I'm not experienced enough to know how to fix this line for Cygwin:
file: os-inl.h line: 139
auto fd = fileno(f);

It is an undeclared identifier in my environment.

@frankkopp
Copy link
Author

There are a few things I noticed.

Some files do not exist in the current master branch, Such as spdlog/details/os-inl.h.
Do you know which commit in master the source file refers to?

I was refering to the 1.x branch.

And most of the differences in pach.diff.txt show that the include path has been changed to a relative path.
It may be affected by the order of header paths specified to the compiler.
If there is a header file with the same name, it is possible that an unintended file is included.

I changed the include paths so that it would compile in my environment. The former paths did not work for me (files not found). I just changed these to relative paths to see if it compiles at all.

Be carefull - I'm having two different issues here:

  1. can't complile spdlog (1.x branch) itself due to cygwin not knowing fileno()
  2. when coping all header files from the 1.x branch to my project the header includes do not work. (it worked with the "master" branch header files.

Thanks
Frank

@gabime
Copy link
Owner

gabime commented Dec 9, 2019

when coping all header files from the 1.x branch to my project the header includes do not work. (it worked with the "master" branch header files.

Did you set the include search flag to point to the copied folder?

@tt4g
Copy link
Contributor

tt4g commented Dec 9, 2019

So I think the issue is not CMAKE or CYGWIN but it is in the *.h files.
I guess the solution would be to support CYGWIN in the *.h files:

In my opinion, there is no problem with the header file because fileno was not found in the difference of pach.diff.txt.

@tt4g
Copy link
Contributor

tt4g commented Dec 9, 2019

Also copying headers to other project only works for "master" branch. 1.x branch does not work:
#includes are not working. I found a workaround by changing the #inclide paths.

Here the diff for my changes so my project compiles with the spdlog headers:

Is patch.diff.txt the difference between 1.x branch and 1.x branch after changing #include paths?
If so, please ignore my opinion below.

In my opinion, there is no problem with the header file because fileno was not found in the difference of pach.diff.txt.

I thought it was the difference between master branch and v1.x branch (both changed #include paths).

gabime added a commit that referenced this issue Dec 10, 2019
@gabime
Copy link
Owner

gabime commented Dec 10, 2019

Fixed and verified. The solution is to add in CMakeLists.txt

if(CMAKE_SYSTEM_NAME MATCHES "CYGWIN")
	set(CMAKE_CXX_EXTENSIONS  ON)
endif()

As for using the header-only by copy - works fine in latets commit. Please make sure you set correctly the search path flag to the parent of the "spdlog" folder

@gabime gabime closed this as completed Dec 10, 2019
@frankkopp
Copy link
Author

Wow - super fast! Thanks. Will test tomorrow and provide feedback.

@frankkopp
Copy link
Author

Confirmed that I can now compile spdlog and also include it in my project.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants