Provide a build system for linux

[galtgendo]

...or that was the original intent, but some serious problems have arisen.

I've decided to try with qmake first, gcc 5.4.

That means C++11 by default (and qmake explicitly adds '-std=gnu++11').

Well, the start wasn't all that bad (correcting to 'QtWidgets/QStyle' in zflowlayout.h and to 'smartvector.h' in zkanjigridmodel.h), but after that I've got a flood of errors.

I can't say yet if it's a c+11 problem or if gcc simply hates your templating style, but a couple of examples:

../smartvector.h:90:32: error: ‘operator+’ in ‘smartvector_iterator<T, Alloc>::self_type’ does not name a template type
     friend self_type ::operator+<>(int n, const self_type &);

../smartvector.h:103:46: error: expected nested-name-specifier before ‘basetype’
     smartvector_move_iterator(const typename basetype &base) : base(base) {}
                                              ^
../smartvector.h:103:55: error: expected ‘,’ or ‘...’ before ‘&’ token
     smartvector_move_iterator(const typename basetype &base) : base(base) {}
                                                       ^
../smartvector.h: In constructor ‘smartvector_move_iterator<T, Alloc>::smartvector_move_iterator(int)’:
../smartvector.h:103:73: error: expected primary-expression before ‘)’ token
     smartvector_move_iterator(const typename basetype &base) : base(base) {}

./smartvector.h: In member function ‘smartvector_move_iterator<T, Alloc>::self_type smartvector_move_iterator<T, Alloc>::operator+(int) const’:
../smartvector.h:110:63: error: there are no arguments to ‘_base’ that depend on a template parameter, so a declaration of ‘_base’ must be available [-fpermissive]
     self_type operator+(int n) const { return self_type(_base() + n); }

../smartvector.h:237:39: error: redeclaration of ‘typedef typename smartvector<T, Alloc>::base::size_type smartvector<T, Alloc>::size_type’
     typedef typename base::size_type  size_type;
                                       ^
../smartvector.h:225:38: note: previous declaration ‘typedef typename smartvector<T, Alloc>::base::size_type smartvector<T, Alloc>::size_type’
     typedef typename base::size_type size_type;

../qcharstring.h: In instantiation of ‘bool operator==(const STR&, const QCharString&) [with STR = QStringRef]’:
../qcharstring.h:305:79:   required from here
../qcharstring.h:305:79: error: explicit instantiation of ‘bool operator==(const STR&, const QCharString&) [with STR = QStringRef]’ but no definition available [-fpermissive]
 emplate bool operator==<QStringRef>(const QStringRef &a, const QCharString &b);

That's not the complete output for collectwordsform.cpp, but should give you the general idea.

[galtgendo]

I stand corrected. It's actually "gcc is picky and abhors many sloppy constructs accepted by MSVC".

For example, it seems all stream >> make_z{vec,str,date} constructs need to be turned into auto foo = make_z{vec,str,date}; stream >> foo;. It's bit tedious, even if that particular case was not that hard to figure out; but there are many more, some looking far more tricky.

[galtgendo]

While cleaning up make_z{vec,str,date} (in a very dirty way) significantly reduced size of the build log (as those errors were quite verbose), I'm slowly reaching the limit of my C++ skills (which are actually somewhat basic). I'm down to about 34 errors.

The largest remaining group of errors seems to be error: invalid initialization of non-const reference of type ‘foo&’ from an rvalue of type ‘foo’ and a few related constructs.

Those tend to happen mostly in places with patterns like std::swap(std::move(foo), std::move(bar)) (std::move doesn't seem to be the culprit - for example std::swap(result, std::vector<TreeItem*>()); fails too).

(though for that last one result = std::vector<TreeItem*>() seems to work - not quite sure if that's quite correct)

Any ideas ?

[galtgendo]

I think I hate this codebase.

this patch is a result - not quite sure it's correct, but compiler errors are done, yet the warnings remain and there's now a shorter list of linking errors.

Some were dealt with by removing inline, but I can't really tell what's with the others. this is just the linking command and errors - the functions seem to be there, so...:confused:

[z1dev]

I have since updated the project to work with vs2015 which is more picky than vs2013 was about the standard. It does not include your patches for gcc yet, but it must have got rid of some of the c++11 errors.

I first wanted to release for Windows before starting to fix the code for compiling with gcc, but I'm almost there. (Need to write a release readme and some extra license information before I can upload.) When it's done I'll check your patch and make the code compile under Linux without errors.

I had the errors with inlines too, so I removed them. I also had to explicitly cast some variables to stop the compiler from complaining. (I think this would only have been warning in gcc.) There was a class where the default constructor and operator= got deleted during compilation, because I provided a move. (vs2013 doesn't follow c++11 standards so close, and it still generated them. I just had to write classname() = default; and classname& operator=(const classname &other) = default;)

I see you made some changes to the qxtglobalshortcut_x11.cpp. I'll have to check if it's possible to keep it unchanged, since it comes from the Qxt project that has a separate license.

I'll have to check what's up with the stream >> make_z{vec,str,date} kind of errors and other template code. It's possible there are some things not exactly matching the current version of the c++ standards. Visual Studio was never complaining about some things in template code, and I tend to get lazy when working with it. (On the other hand I see you removed some typename keywords, which interestingly did cause an error in VS2013 but works fine in VS2015. - I'm not actually sure about this but I won't go back to test it now. I would have to restore the project files to the previous version.)

I haven't finished checking every change in your patch yet. I'll be back to that after the first test release on Windows.

[z1dev]

I couldn't stop myself and went through the whole patch. I didn't touch the template code that "should" work, but fixed others. I.E. I fixed the error where I left out the this-> from inherited members. (VS is very lenient with these.)

The undefined function references with the QCharString operators are the result of the change in the patch:

-template bool operator==<QStringRef>(const QStringRef &a, const QCharString &b);
-template bool operator==<QStringRef>(const QCharString &a, const QStringRef &b);
-template bool operator!=<QStringRef>(const QStringRef &a, const QCharString &b);
-template bool operator!=<QStringRef>(const QCharString &a, const QStringRef &b);
+template<> bool operator==<QStringRef>(const QStringRef &a, const QCharString &b);
+template<> bool operator==<QStringRef>(const QCharString &a, const QStringRef &b);
+template<> bool operator!=<QStringRef>(const QStringRef &a, const QCharString &b);
+template<> bool operator!=<QStringRef>(const QCharString &a, const QStringRef &b);

I double checked, but forward declaring template specializations should generate the appropriate code in standard c++. I'll have to see what GCC complains about. For now I have a possible fix. Instead of leaving these in the header, I added a dummy function in qcharstring.cpp that causes the compiler to generate the functions. You can check out the source code because I have uploaded it. If GCC is smart, this might not work either, because it might ignore the dummy function that's never called. I don't understand the other undefined reference, but it might be possible to solve the same way.

The last point for now:

-QImage renderFromSvg(QImage &dest, QString svgpath, QRect r);
+QImage renderFromSvg(QImage &&dest, QString svgpath, QRect r);

This is just one of multiple functions where you fixed something that GCC possibly complained about. This looks wrong to me since dest is not a value to be thrown away. It makes no sense to pass it as an rvalue reference. What is the error message if you leave the normal reference unchanged?

[galtgendo]

Thank you for replying. This may turn into another series of multi-posts, but I'll be posting this as I revisit the first patch.

In (a somewhat) random order:

• simply int -> uintptr_t for QVariants is insufficient (at least in Qt 5.7); it leads to error: conversion is ambiguous (on this point: for Windows, do you build it as 32bit or 64bit ? my patch is for 64bit Linux)
• T -> TT for those two operators in smartvector.h is to prevent declaration of ‘class T’ shadows template parm ‘class T’
• those additional <cmath> includes cover differences in libstdc++ includes
• as for that smartvector_iterator<T, Alloc>:: bit in smartvector.h, see https://isocpp.org/wiki/faq/templates#nondependent-name-lookup-types (I don't really understand it, but this way it works)

[galtgendo]

As for renderFromSvg, it was a bad guesstimate, due to seeking a shorthand.

Seems the proper solution is:

QPixmap temp(_iconW, _iconH);
QPixmap pix = renderFromSvg(temp, QStringLiteral("foo"), , QRect(0, 0, _iconW, _iconH));

It's just another case of error: invalid initialization of non-const reference of type ‘foo&’ from an rvalue of type ‘foo’' (same group all those stream >> make_zfoo(...) changes deal with)

[galtgendo]

As for template -> template<>, that change might have been unnecessary for the errors, but there's a significant block of warnings of friend declaration of foo declares a non-template function type - I think that I've mistakenly made the change while trying to come up with something about those.

[galtgendo]

On unrelated note, the move of the block in worddeck.cpp is due to error: specialization of foo after instantiation.

[galtgendo]

...and removing (quint8) in that one place is due to error: lvalue required as left operand of assignment

[galtgendo]

Those invalid initialization errors are also reason for

std::swap(result, std::vector<TreeItem*>());

to

result = std::vector<TreeItem*>();

(and the second similar one) even if that might not be quite correct.

[galtgendo]

Anyway, updated patch and link log - the second one is mostly unchanged.

[galtgendo]

Even among compiler warnings there doesn't seem to be anything related to those linker errors; those are mostly type qualifiers ignored on function return type, comparison between signed and unsigned integer, unused parameter, a few unused variable, three non-template function blocks (2 in smartvectors.h, 1 in zkanjimain.h), some enumeration value ‘foo’ not handled in switch and a handful of others (like array subscript has type ‘char’ or reorder warnings for classes), but nothing looking related.

[galtgendo]

OK, it seems the solution of the linking problem is nearly trivial - those functions both are and aren't there : see here

Well, those linking errors are simply just the places you've managed to trigger it, as you're defining a lot of template functions outside headers - in most places it doesn't matter, in a few - it does.

[z1dev]

I'm compiling zkanji as 32 bit executable on Windows, and I plan to do the same on Linux. I first wanted to port the old code, which is 32 bit, so it seemed simpler to stay with that. Converting to 64 bit would be an option but I don't want to waste another month on fixing possible problems. Can you try compiling with 32 bit and see if it's better?

[galtgendo]

Well, generating the data was a bit annoying - you should probably provide zdict.zks as a separate download somewhere.

After that, the program started and didn't crash upon looking up a word, so...limited success :sweat_smile: ?

In the meanwhile, I've thrown it at clang - a couple warnings became errors, as expected, the list of warnings differs significantly between gcc and clang - clang doesn't complain about friends, but throws fits about overrides and virtuals...though perhaps a bit more recent gcc than 5.4 would throw more of those too :shrug:.

I'll attach both full build logs shortly.

[galtgendo]

...I spoke too soon.

What I thought was the main program, was in fact a part of the import process. After choosing 'Skip All' (I don't know this program, so have no idea what that 'pick replacement' question was about; it's phase 7/8 - 'Building commons tree...'), it seems the process got halted - there doesn't seem to be any cpu activity from the program, it's just displaying the popup.

[galtgendo]

...sorry, but for Linux, the app pretty much must be native. While some people might have some 32bit libs installed on 64bit systems (mainly for wine), installing 32bit Qt5 with all of its deps is plain insane.

[galtgendo]

OK, another misunderstanding - pressing 'Skip All' in that replacement dialog triggers the idle loop, while clicking 'Skip word' till list empties gets you to 'Finish' button, which lets the process complete.

Odd choice.

[galtgendo]

Will write more later, for now two interesting lines written to the console during a run consisting of only running the program, then quitting:

QCoreApplication::postEvent: Unexpected null receiver
QCoreApplication::postEvent: Unexpected null receiver
QBasicTimer::start: QBasicTimer can only be used with threads started with QThread
QBasicTimer::start: QBasicTimer can only be used with threads started with QThread
QBasicTimer::start: QBasicTimer can only be used with threads started with QThread
QBasicTimer::start: QBasicTimer can only be used with threads started with QThread
QBasicTimer::start: QBasicTimer can only be used with threads started with QThread
QBasicTimer::start: QBasicTimer can only be used with threads started with QThread

First two happen right at the start, the rest upon closing the window.

[z1dev]

You can find a previous version of the zks file in the old zkanji releases on SourceForge. The program only works under windows, but the same data file should work.

I will find out how much pain it is to install 32bit Qt5 on Linux. I used some "magic", converting between types the C way, and I'm not entirely sure the program won't be broken in 64bit compilation.

The replacement dialog shouldn't even come up unless you have an old zkanji lying around somewhere, and you imported data files from there. It might be a bug in this case. The replacement dialog was a rushed work I did very early in development. The design choice was basically: how to do it fast so I can get to the real program?

I'm not sure what those console lines are about, I see a lot of things printed under VS too by Qt. I assume it's a bug inside Qt itself, but I haven't checked.

[z1dev]

A second thought, the replacement dialog must be for the JLPT word list. It's not a bug then but the dialog was rushed nonetheless.

[galtgendo]

Well, such cleanup is long overdue (by about a decade) - I'm not just talking about zkanji, I very much mean it for any program.

AFAICT, some effort will be needed, but not all that much; mostly in cases where you were storing pointers in data models. Do , however, take note of ILL64/LL64 difference (sizeof(long)).

[galtgendo]

Anyway, updated patch (besides errors, addresses a few warnings (like reorders and some of not handled in the switch cases))

gcc log

clang log

the struct <-> class changes were just warnings, but also close to trivial.

[galtgendo]

Sorry, wrong clang log - this is the right one.

One more thing: using std::nullptr_t; - for some reason, gcc doesn't complain, but clang generates an error without it - yet another header difference.

[z1dev]

I found out how much pain it really is to compile Qt under Linux for 32bit. I'll keep the 32bit version on Windows for now and compile zkanji for 64bit on Linux. The ILL64/LL64 difference doesn't matter, since long or int was all the same under VS when compiling 32bit executable, so it made no sense to use long.

I fixed the stream << ... or stream >> ... and make_z... template code by fixing the template itself, so the rest of the code works fine now. My version of gcc is 5.3.1, and it did complain about the single use of nullptr_t. I'm using Qt Creator as my IDE.

I made some other changes as well, but I won't be fixing the warnings that gcc listed, because it's impossible to get rid of most of them. I already changed the source in VS when you started posting your patches so I was forced to fix everything by hand, but it led me to the solution for many problems.

I compiled zkanji on Linux and it is working more or less. I'll have to check whether the changes I uploaded here are complete or not. (I was fixing things both in my virtual machine and VS in parallel and might have forgotten something.) It's not perfect yet though, I had to skip the example sentences import as it bugged out near the end.

[z1dev]

The example sentences import wasn't bugged. Someone made a mistake in the latest update of the Tanaka Corpus file. I'm checking for this now so the example sentences file is generated correctly.

I have uploaded my version of the .pro file (for Linux only.) Hopefully zkanji can be compiled on Linux with this. I'll close this issue on Monday unless something else comes up.

[galtgendo]

The changes in Qxt/qxtglobalshortcut_x11.cpp are still necessary - that qpa header referenced there is a private Qt header - no program should be referencing it and there's no reason to expect them being installed on the system even if the standard Qt headers are.

Also, Qt is a special case for qmake - there's no need to add that include path, especially that you've made it version specific.

Now, as for pkg-config and its parameters: using it on linux is standard, Xorg has been using it for several years now, so the files required should be there with the headers. For libX11, on most systems, adding the lib, like you did, would have been safe, but this way is usually covers a couple more cases; xcb is a bit on ham-fisted side, as it only seems to be using the header, not the lib - so needs a bit more thought, even if it's safe.

The patch now is much shorter. Perhaps I'll look at the warnings later.

[galtgendo]

...probably should have put those two pkg-config lines behind unix:

[galtgendo]

...also, something went wrong in one of your patches: while zkanji starts and allows import, it fails later on, with "Date of installed dictionary or the file itself is invalid" popup.

Something goes wrong at the last step: stream >> make_zdate(dt);

[z1dev]

I have installed Qt on my Linux VM as it was written on the official Qt website, and the include was there. It doesn't have the _p at the end of the file name so I assumed it's not private and should be safe to use. Just checked and it has a warning saying it shouldn't be used. It's beyond me why they even mark some of the headers with _p if the whole folder is private.

I've only used a very small part of Qxt, to be able to install global shortcuts in the system. It would be best to throw that library away and write the code for zkanji. It's easy enough to do on Windows and didn't need a huge switch like it does in the Qxt code.

I'll integrate your patch in the source code and see how it compiles.

I don't get the same error message after importing the base dictionary, I wonder what's different.

[z1dev]

I wonder why you need to add cmath to the headers of some files. It compiles perfectly for me without it. Can you change those to use #ifdef to only add the include with some specific compilers? I'm not sure what check would be the best there, since I don't get errors for it.

[galtgendo]

It seems to be caused by some time of life shenanigans: the date is valid within QDataStream& operator>>(QDataStream &stream, ZDateTimeStr<QDateTime> date) operator, but as it returns, the reference becomes invalid.

If instead I put the relevant portion of the operator in Dictionary::fileWriteDate(const QString &filename) (conversion of the read value to QDateTime), things go past that point.

Though this raised two questions:

1. In how many other places things go wrong with make_zdate ?
2. Are other make_zfoo constructs similarly affected ?

[z1dev]

Your changes to the .pro file and qxtglobalshortcut_x11.cpp won't compile for me.

EDIT: I get undefined reference to symbol 'XStringToKeysym'

[z1dev]

The problem with the ZDateTimeStr function is that in one of my latest fixes, as I ported it back to VS on Windows from my virtual Linux, I deleted the & in the ZDateTimeStr constructor. It should go like this: ZDateTimeStr(T &dt) : dt(dt) { ; }

[galtgendo]

I wonder why you need to add cmath to the headers of some files. It compiles perfectly for me without it. Can you change those to use #ifdef to only add the include with some specific compilers? I'm not sure what check would be the best there, since I don't get errors for it.

That wouldn't be a correct solution, as your compiler already is including either <cmath> or math.h, so for you, those are close to noops for the preprocessor. But as those two files are using functions from that header, if the compiler doesn't include them implicitly, includes must be explicit.

gcc has been going through this header cleanup (as to include only what's being used by the header itself) for quite awhile and major releases tend to include warnings about such changes.

As for that linking error, probably the distro you're using is splitting headers into -dev packages, so you need to have that one installed for libX11 (and probably libxcb too). The other option is that you haven't properly set up your build env: you need to export PKG_CONFIG_PATH with value pointing to the dir pkg-config files for those packages are installed.

(if you tell how did you set things up and which distro you're using, I might make a better guess (personally, I use a distro where the above is a non-issue, as all the development files (like headers or pkg-config files) are installed, as long as they're installed by the upstream))

Adding that &, seems to have worked, though I needed to delete a few user files manually, cause zkanji was terminating with "User date doesn't match dictionary date".

Not really related user question: in "Browse Japanese" mode, is entering kanji supposed to not work ? Cause in "Japanese to English", it works just fine.

[z1dev]

I'll add the cmath includes then. I see it's for the std::abs calls. I'm used to compilers not requiring it.

The culprit for not linking libX11 correctly was a typo. I wrote unis: instead of unix: in front of your additions of CONFIG and PKGCONFIG in the Qt project file. It works fine now. I still wish to get rid of the Qxt source files completely, but it's not urgent.

To your question: "Browse Japanese" uses the alphabetic or AIUEO ordering of every word in the dictionary. It can only search the kana field. There is no unified standard way of ordering kanji, so I can't use that. There are still many usability issues in this first release. I will have to prevent kanji typed or copied in the search field there.

[bakatrouble]

Why did you use Visual Studio in the first place? As you are using Qt, it's more natural to code in QtCreator.

[bakatrouble]

Managed to build Zkanji in QtCreator (Manjaro/Arch Linux, default build settings - Qt 5.9.1, GCC 7.1.1) with a few code fixes, will create pull request now.

[z1dev]

I'm using VS because I'm coding under Windows. It has a much better editor and debugger than Qt Creator, but I'm using Qt Creator on Linux, which is my main IDE there.

[z1dev]

I'll close this issue, since building on Linux doesn't seem to be a problem any more. If something comes up, please open a new one with a more specific title. Like wrong window positioning on Linux.