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

[UPDATED] - Fixes compilation issues, supercedes previous PRs. #93

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

themrrobert
Copy link
Contributor

@themrrobert themrrobert commented Mar 19, 2024

Description:

This builds on the fantastic work of @additional-pumpkin,
Supercedes / Closes PR #90 and PR #91

Changes:

  • I removed the obsolete/deprecated/removed std:__binary_function template, as that is now handled entirely by the compiler as of C++17.
  • I also had to modify the ZLIB config in chessx.pro, as the default config wasn't building/linking correctly.

Details:

I haven't messed with QT programming, or Windows linking/ABI in many years, so I could have missed something, but it was looking for extern's that didn't exist in the zlib libraries. (quazipfile.o was referencing __imp_z_gzflush, __imp_z_gzread, __imp_z_gzwrite, etc, that I couldn't find anywhere using dumpbin /symbols

Worth noting I tried first without installing external zlib, and also with external zlib, same problem, and the external zlib build didn't have the referenced implementation functions, either.

Notes:

The INSTALL.md file needs additional updates. Currently, (at least) C++17 is required at minimum to build all requirements, however C++20 is specified in the build deps per PR #90, so I'll defer to the maintainer on how to update the remainder of the file.

additional-pumpkin and others added 4 commits September 19, 2023 18:10
The file main.cpp includes mainwindow.h without USE_SOUND being defined.
If USE_SOUND is defined when building mainwindow.cpp they'll get linked
with different sized MainWindow classes. main.cpp will then allocate
less than enough space for MainWindow causing it to segfault.
@themrrobert themrrobert changed the title Fix compilation on windows [UPDATED] - Fixes compilation issues, supercedes previous PRs. Mar 19, 2024
@@ -0,0 +1,58 @@
#ifndef QT6COMPAT_H

Choose a reason for hiding this comment

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

This file is a duplicate of src/gui/qt6compat.h.

@limitedAtonement limitedAtonement mentioned this pull request Mar 22, 2024
@IAHM-COL
Copy link
Contributor

I still get compile errors with this PR

chessx ˚ fix-compilation-pull-request [  ✪    ] ⇒ make                                                                                                               [141] 03/26/24 ˚  4:53PM
/usr/bin/qmake-qt5 -o Makefile chessx.pro
g++ -c -pipe -O2 -std=gnu++1z -Wall -Wextra -D_REENTRANT -fPIC -DQT_DEPRECATED_WARNINGS -DUSE_C11 -DUSE_SOUND -DQUAZIP_STATIC -DQT_NO_CAST_TO_ASCII -DQT_USE_QSTRINGBUILDER -DUSE_SCID -DQT_NO_DEBUG_OUTPUT -DNDEBUG -DQT_NO_DEBUG -DQT_SVG_LIB -DQT_PRINTSUPPORT_LIB -DQT_WIDGETS_LIB -DQT_MULTIMEDIA_LIB -DQT_GUI_LIB -DQT_NETWORK_LIB -DQT_XML_LIB -DQT_CORE_LIB -I. -Idep/scid/code/src -Isrc/database/scid -Isrc/database -Isrc/guess -Isrc/gui -Isrc/dialogs -Isrc/quazip -I/usr/src/3rdparty/zlib -Isrc/generated -I/usr/include/qt5 -I/usr/include/qt5/QtSvg -I/usr/include/qt5/QtPrintSupport -I/usr/include/qt5/QtWidgets -I/usr/include/qt5/QtMultimedia -I/usr/include/qt5/QtGui -I/usr/include/qt5/QtNetwork -I/usr/include/qt5/QtXml -I/usr/include/qt5/QtCore -Isrc/generated -I/usr/lib64/qt5/mkspecs/linux-g++ -o obj_rel/annotation.o src/database/annotation.cpp
src/database/annotation.cpp: In member function ‘virtual void Annotation::toggle(const QString&)’:
src/database/annotation.cpp:20:11: error: ‘class QStringList’ has no member named ‘remove’; did you mean ‘removeAt’?
         l.remove(n);
           ^~~~~~
           removeAt
src/database/annotation.cpp: In member function ‘virtual void Annotation::removeOne(const QRegularExpression&)’:
src/database/annotation.cpp:35:11: error: ‘class QStringList’ has no member named ‘remove’; did you mean ‘removeAt’?
         l.remove(n);
           ^~~~~~
           removeAt
make: *** [Makefile:3106: obj_rel/annotation.o] Error 1

And

src/database/bitboard.cpp: In member function ‘bool BitBoard::compare(const BitBoard&) const’:
src/database/bitboard.cpp:1785:18: error: ‘memcmp’ is not a member of ‘std’
     return (std::memcmp(m_piece, b.m_piece, sizeof(m_piece))==0);
                  ^~~~~~
src/database/bitboard.cpp:1785:18: note: suggested alternative: ‘mem_fn’
     return (std::memcmp(m_piece, b.m_piece, sizeof(m_piece))==0);
                  ^~~~~~
                  mem_fn
make: *** [Makefile:3137: obj_rel/bitboard.o] Error 1

@IAHM-COL
Copy link
Contributor

I still get compile errors with this PR

Forgot mentioning

chessx ˚ fix-compilation-pull-request [ u ✪    ] ⇒ gcc --version                                                                                                       [2] 03/26/24 ˚  4:55PM
gcc (SUSE Linux) 7.5.0
Copyright (C) 2017 Free Software Foundation, Inc.
This is free software; see the source for copying conditions.  There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

 
chessx ˚ fix-compilation-pull-request [ u ✪    ] ⇒ gmake --version                                                                                                         03/26/24 ˚  4:56PM
GNU Make 4.2.1
Built for x86_64-suse-linux-gnu
Copyright (C) 1988-2016 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html>
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.

@limitedAtonement
Copy link

limitedAtonement commented Mar 27, 2024

What are you running to get these results? Try mkdir build && cd build && qmake6 && make -j 50; (50 should be whatever number is suitable for your system.)

@IAHM-COL
Copy link
Contributor

IAHM-COL commented Apr 26, 2024

What are you running to get these results? Try mkdir build && cd build && qmake6 && make -j 50; (50 should be whatever number is suitable for your system.)

@limitedAtonement @Isarhamster

I can confirm this patch fixes current compilation issues:

With current master 183e7e7 there is a compilation error that interrupts on the Polyglot source.

With this patch at d475990 compilation is succesful, when using Qt6, as indicated by @limitedAtonement above.

I suggest this patch be given high priority

Tested on

QMake version 3.1
Using Qt version 6.4.2 in /usr/lib64

g++ (SUSE Linux) 7.5.0

@IAHM-COL
Copy link
Contributor

What are you running to get these results? Try mkdir build && cd build && qmake6 && make -j 50; (50 should be whatever number is suitable for your system.)

@limitedAtonement

Thanks for your help compiling. I was attempting to build with Qt5 and this does NOT longer work. Qt6 is now required.
I had to install qt6 and several components for it, so it took me a while, but right now my build-sandbox is updated and working. From your instructions up here a change is required: ../qmake6

mkdir build && cd build && ../qmake6 && make -j 4

@IAHM-COL
Copy link
Contributor

IAHM-COL commented Jul 2, 2024

@limitedAtonement

I don't know if this PR is necessary anymore.

I got successful compilation of the master branch with qmake6, as shown with the code in the comment above, on commit 83783a5

[Again, compiling test was on linux]

Regards

@IAHM-COL
Copy link
Contributor

IAHM-COL commented Jul 2, 2024

@limitedAtonement @Isarhamster

Howerver the changes in this pull request on the file
https://github.com/Isarhamster/chessx/blob/master/INSTALL.md
would actually get my vote for being added to master, regardless.

Since the software now requires Qt6 and the commands as shown in the update here,

@limitedAtonement
Copy link

@IAHM-COL

mkdir build && cd build && ../qmake6 && make -j 4

that doesn't work. There is no qmake6 in the repository. Maybe you meant qmake6 ..? That appears to work on master now:

mkdir build && cd build && qmake6 .. && make -j 45

I can make and run the project, and everything looks good. I agree that the documentation needs updated to reflect that this is a qt6 project.

@IAHM-COL
Copy link
Contributor

IAHM-COL commented Jul 2, 2024

@IAHM-COL

mkdir build && cd build && ../qmake6 && make -j 4

that doesn't work. There is no qmake6 in the repository. Maybe you meant qmake6 ..? That appears to work on master now:

mkdir build && cd build && qmake6 .. && make -j 45

I can make and run the project, and everything looks good. I agree that the documentation needs updated to reflect that this is a qt6 project.

yes, my typo.

indeed

mkdir build && cd build && qmake6 ../ && make -j 4

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

Successfully merging this pull request may close these issues.

4 participants