Skip to content

Commit

Permalink
Fix build for Mingw-w64
Browse files Browse the repository at this point in the history 
The old code (before commit 6444695)
works well with Mingw-w64 which does not support this_thread.

Signed-off-by: Stefan Weil <[email protected]>
  • Loading branch information
@stweil
stweil committed Nov 29, 2016
1 parent 185a264 commit 120a5db
Showing 1 changed file with 9 additions and 1 deletion.
10 changes: 9 additions & 1 deletion ccstruct/imagedata.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,11 @@
#include "helpers.h"
#include "tprintf.h"

#include <thread>
#if defined(__MINGW32__)
# include <unistd.h>
#else
# include <thread>
#endif

// Number of documents to read ahead while training. Doesn't need to be very
// large.
Expand Down Expand Up @@ -449,7 +453,11 @@ const ImageData* DocumentData::GetPage(int index) {
if (needs_loading) LoadPageInBackground(index);
// We can't directly load the page, or the background load will delete it
// while the caller is using it, so give it a chance to work.
#if defined(__MINGW32__)

This comment has been minimized.

Sign in to view

Copy link
@egorpugin

egorpugin Nov 29, 2016

Contributor

What version of gcc is in mingw?
If it's >= gcc 4.9 , you should just add -std=c++11 somewhere to cc flags.
Please, check it and revert this commit.

This comment has been minimized.

Sign in to view

Copy link
@stweil

stweil Nov 30, 2016

Author Member

Debian Testing comes with gcc-mingw-w64-i686-6.1.1-12+19.1, and the configure based build already sets std=c++11. Nevertheless the include files coming with mingw-w64-i686-dev-5.0.0-1 don't provide this_thread, so reverting would not be a good idea.

This comment has been minimized.

Sign in to view

Copy link
@egorpugin

egorpugin Nov 30, 2016

Contributor

Mingw on debian?
I thought it's something related to windows, no?

This comment has been minimized.

Sign in to view

Copy link
@egorpugin

egorpugin Nov 30, 2016

Contributor

Internet says To access C++11 threads in Windows, you will need a build of Mingw with posix-threads.
and
MinGW-w64 (or rather GCC on windows) needs to be compiled with posix thread support if you want to use std::thread, presumably you downloaded a build with native windows threads.

http://stackoverflow.com/questions/34847804/stdthread-works-in-cygwin-but-not-in-mingw

This comment has been minimized.

Sign in to view

Copy link
@stweil

stweil Nov 30, 2016

Author Member

You don't think that I develop on Windows, do you? :-)

All major Linux distributions support cross builds targeting Windows, and those cross builds are based on Mingw-w64. They even provide a package for nsis, the nullsoft installer which I use for the Tesseract for Windows installer (nsis support was removed from the Tesseract code some time ago).

The cross build Debian packages include both posix and win32 thread support, so compiling with posix thread support is possible (although I usually use the win32 threads) but still does not get std::thread. A closer look shows the reason: the header files which come with the compiler include std::thread and sleep_for (also for win32 threads), but only conditionally: #if defined(_GLIBCXX_HAS_GTHREADS) && defined(_GLIBCXX_USE_C99_STDINT_TR1). Obviously that part is not used for Mingw-w64 which is not glibc and so won't define those macros. Maybe newer versions of Mingw-w64 do this better, but not the current one from Debian testing.

This comment has been minimized.

Sign in to view

Copy link
@egorpugin

egorpugin Nov 30, 2016

Contributor

Is it worth it to support such compiler?

This comment has been minimized.

Sign in to view

Copy link
@zdenop

zdenop Nov 30, 2016

Contributor

Why not if it is maintained?

This comment has been minimized.

Sign in to view

Copy link
@egorpugin

egorpugin Nov 30, 2016 •
edited
Loading

Contributor

Usually platforms or subsystems supported and maintained for users.
And the original author of them maintains them because atleast he is using them. He interested in it.
But maintaining something which is not used by users and more than that has limitations is very questionable.
Maintaining just for fun and adding tons of ifdefs is not cool.

I can only agree with this commit if:

  1. Users use mingw builds on windows.
  2. mingw build is maintained on windows - i.e. not crosscompiled.

Windows is not the platform you couldn't directly access like ios and android.
And who maintains linux builds crosscompiling on windows?
sleep(1);
#else
std::this_thread::sleep_for(std::chrono::seconds(1));
#endif
}
return page;
}
Expand Down

4 comments on commit 120a5db

@amitdo
Copy link
Collaborator

@amitdo amitdo commented on 120a5db Nov 30, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And who maintains linux builds crosscompiling on windows?

Some people actually doing so (I don't have references right now).

@Warblefly
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I normally build Tesseract almost daily from git source for my Multimedia Tools MinGW distribution, adding the library to my FFmpeg builds, for the ability to 'read' burnt-in subtitles. For the time being, I've reverted to Tesseract 1.3.1 because I do not know enough about thread programming to understand quite what changed (beyond adding missing headers). My compiler and all compiled code uses Win32 threads, not POSIX threads. But I shall try today's code straight away.

@stweil
Copy link
Member Author

@stweil stweil commented on 120a5db Dec 1, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The latest code should build with any recent Mingw-w64, no matter whether it is a cross build or native.

The previous code (without this commit) fails with the latest Debian cross packages, but builds on Windows with the latest Mingw-w64 included in Cygwin. I am still investigating the differences between these two cases. Both are based on Win32 threads.

@egorpugin
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My vote is for cygwin and for toolsets with full c++ support.
But this is only unnecessary burden.

Please sign in to comment.