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

Linux Git Build failure: wxWidgets 3.0 wxOVERRIDE [BUG] #4282

Closed
clytle374 opened this issue May 21, 2020 · 9 comments
Closed

Linux Git Build failure: wxWidgets 3.0 wxOVERRIDE [BUG] #4282

clytle374 opened this issue May 21, 2020 · 9 comments

Comments

@clytle374
Copy link

Latest git pull. version_2.3.0-alpha0-88-gdb32c1f15
Gentoo Linux

I read that wxWidgets 3.1 is the standard here, but not available on my system. Didn't have any trouble until recently. I can run a bisect if it's needed, or will try to answer any other questions to help find a resolution. Here is failure.

`
[ 71%] Building CXX object tests/libslic3r/CMakeFiles/libslic3r_tests.dir/test_elephant_foot_compensation.cpp.o
[ 71%] Building CXX object src/slic3r/CMakeFiles/libslic3r_gui.dir/pchheader.cpp.o
/home/cory/sources/prusa/tests/fff_print/test_skirt_brim.cpp:15:12: warning: ‘int get_brim_tool(const string&)’ defined but not used [-Wunused-function]
15 | static int get_brim_tool(const std::string &gcode)
| ^~~~~~~~~~~~~
[ 71%] Building CXX object src/slic3r/CMakeFiles/libslic3r_gui.dir/GUI/AboutDialog.cpp.o
In file included from /home/cory/sources/prusa/src/slic3r/GUI/Plater.hpp:17,
from /home/cory/sources/prusa/src/slic3r/GUI/MainFrame.hpp:15,
from /home/cory/sources/prusa/src/slic3r/GUI/GUI_App.hpp:7,
from /home/cory/sources/prusa/src/slic3r/GUI/AboutDialog.cpp:6:
/home/cory/sources/prusa/src/slic3r/GUI/Search.hpp:231:43: error: expected ‘;’ at end of member declaration
231 | virtual unsigned int GetColumnCount() const wxOVERRIDE { return colMax; }
| ^~~~~
| ;
/home/cory/sources/prusa/src/slic3r/GUI/Search.hpp:231:49: error: ‘wxOVERRIDE’ does not name a type
231 | virtual unsigned int GetColumnCount() const wxOVERRIDE { return colMax; }
| ^~~~~~~~~~
/home/cory/sources/prusa/src/slic3r/GUI/Search.hpp:232:54: error: expected ‘;’ at end of member declaration
232 | virtual wxString GetColumnType(unsigned int col) const wxOVERRIDE;
| ^~~~~
| ;
/home/cory/sources/prusa/src/slic3r/GUI/Search.hpp:232:60: error: ‘wxOVERRIDE’ does not name a type
232 | virtual wxString GetColumnType(unsigned int col) const wxOVERRIDE;
| ^~~~~~~~~~
/home/cory/sources/prusa/src/slic3r/GUI/Search.hpp:233:88: error: expected ‘;’ at end of member declaration
233 | virtual void GetValueByRow(wxVariant& variant, unsigned int row, unsigned int col) const wxOVERRIDE;
| ^~~~~
| ;
/home/cory/sources/prusa/src/slic3r/GUI/Search.hpp:233:94: error: ‘wxOVERRIDE’ does not name a type
233 | virtual void GetValueByRow(wxVariant& variant, unsigned int row, unsigned int col) const wxOVERRIDE;
| ^~~~~~~~~~
/home/cory/sources/prusa/src/slic3r/GUI/Search.hpp:234:93: error: expected ‘;’ at end of member declaration
234 | virtual bool GetAttrByRow(unsigned int row, unsigned int col, wxDataViewItemAttr& attr) const wxOVERRIDE { return true; }
| ^~~~~
| ;
/home/cory/sources/prusa/src/slic3r/GUI/Search.hpp:234:99: error: ‘wxOVERRIDE’ does not name a type
234 | virtual bool GetAttrByRow(unsigned int row, unsigned int col, wxDataViewItemAttr& attr) const wxOVERRIDE { return true; }
| ^~~~~~~~~~
/home/cory/sources/prusa/src/slic3r/GUI/Search.hpp:235:92: error: expected ‘;’ at end of member declaration
235 | virtual bool SetValueByRow(const wxVariant& variant, unsigned int row, unsigned int col) wxOVERRIDE { return false; }
| ^
| ;
/home/cory/sources/prusa/src/slic3r/GUI/Search.hpp:235:94: error: ‘wxOVERRIDE’ does not name a type
235 | virtual bool SetValueByRow(const wxVariant& variant, unsigned int row, unsigned int col) wxOVERRIDE { return false; }

`

@clytle374
Copy link
Author

As someone commented in commit 5c1d736 it breaks compatibility with wxwidgets 3.0 and -DSLIC3R_WX_STABLE=1 will cause build to fail.

I understand wanting to move forward, but wxwidgets 3.1 is still in development.

Thanks
Cory

@clytle374 clytle374 changed the title Linux Git Build failure: wxWidgets 3.0 wxOVERRIDE Linux Git Build failure: wxWidgets 3.0 wxOVERRIDE [BUG] May 24, 2020
@bubnikv
Copy link
Collaborator

bubnikv commented May 25, 2020

Thanks for heads up.

Nearly nobody uses PrusaSlicer compiled against wxWidgets 3.0, I estimate the use base of PrusaSlicer in that configuration to 0.03%. We produce PrusaSlicer builds compiled against wxWidgets 3.1 for years. It is an unlucky situation, that the wxWidgets team does not have time or motivation to declare the wxWidgets 3.1 stable. I know it is about the ABI of the library and they have a rule that the odd releases are ABI stable, so they would have to release wxWidgets 3.2 and then it would take another year or two before this library would trickle into LTS distros.

As the wxWidgets 3.0 are distributed by linux distros, we are in an unlucky situation that we should still support it. However we should invest more effort into transitioning to wxWidgets 3.1 / GTK3.

@pix
Copy link

pix commented May 25, 2020

Maybe we can define wxOVERRIDE as it is defined in https://github.com/wxWidgets/wxWidgets/blob/master/include/wx/defs.h#L282

#ifdef HAVE_OVERRIDE
    #define wxOVERRIDE override
#else /*  !HAVE_OVERRIDE */
    #define wxOVERRIDE
#endif /*  HAVE_OVERRIDE */

Compiling the last version on Linux needs this and another line commented (I'm recompiling PrusaSlicer as of now in order to report it)

@clytle374
Copy link
Author

I managed to overlay 3.1 into my system and got it building again. Runs but have issue with platter scaling. I'll give that a few days to see if it gets resolved before creating an issue on that.

Close this if you want
Thanks, Cory

@lukasmatena
Copy link
Collaborator

I may be missing something here, but as far as I know, PrusaSlicer uses C++17 standard. Can't we just use plain override instead of the wxOVERRIDE macro, which seems to be a remnant of pre-C++11 times? @YuSanka ?

lukasmatena added a commit that referenced this issue May 27, 2020
Now that we use C++17, there is no point in using it in PrusaSlicer codebase
@lukasmatena
Copy link
Collaborator

lukasmatena commented May 27, 2020

I just committed (f2f1cfe) what I proposed (with @YuSanka's approval). It should now build fine against wxWidgets 3.0.
Thanks for reporting. Please, let us know when we break it again someplace else. Closing the issue.

@pix
Copy link

pix commented May 27, 2020

Sorry for the delay, I forgot to post the other fix:

The preprocessor guard seems invalid as EnableMarkup is not available on 3.0. I needed to comment that line in order to compile it using 3.0 on Ubuntu.

 #if wxUSE_MARKUP
-    markupRenderer->EnableMarkup();
+    // markupRenderer->EnableMarkup();
 #endif // wxUSE_MARKUP

@lukasmatena
Copy link
Collaborator

@pix I did not notice there's more. You're right. The wxUSE_MARKUP macro was added in 2011 (wxWidgets/wxWidgets@f5bdfc6), while the EnableMarkup function much later in 2016 (wxWidgets/wxWidgets@74c0462). So it first appeared in v3.1.1.

I'm not sure if there is a better fix than

 #if wxUSE_MARKUP
+  #if wxCHECK_VERSION(3, 1, 1)
     markupRenderer->EnableMarkup();
+  #endif
 #endif // wxUSE_MARKUP

Lovely. Could you please check it works so I can commit it into master? Thanks.

@lukasmatena
Copy link
Collaborator

@YuSanka and myself have created 6d432f5 - an ultimate fix of all our issues.
Closing (again).

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

4 participants