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

We should select a minimum Qt version #371

Closed
FiloSpaTeam opened this Issue Apr 26, 2018 · 17 comments

Comments

Projects
None yet
3 participants
@FiloSpaTeam
Contributor

FiloSpaTeam commented Apr 26, 2018

This can be helpfull to convert some old style code Qt4 to Qt5 for example. Or use newer directive.

At the moment Qt 5.2 seems to be enough minimalist :)

@annejan annejan added the discussion label Apr 26, 2018

@FiloSpaTeam

This comment has been minimized.

Contributor

FiloSpaTeam commented Apr 28, 2018

An example:
connect changed style with Qt5 and there are part of code with Old Qt4 and parts with Qt5.
In this case we are able to compile with Qt4?

@FiloSpaTeam

This comment has been minimized.

Contributor

FiloSpaTeam commented May 14, 2018

So the minimum version is Qt 5.10? 💃

@annejan

This comment has been minimized.

Member

annejan commented May 14, 2018

I think it should be 😁
Although I don't mind it being a soft-limit for now and have 5.6 as a "hard" limit 😉

@FiloSpaTeam

This comment has been minimized.

Contributor

FiloSpaTeam commented May 14, 2018

Ok so i'll work to migrate all to Qt 5. We have some connect in old style and so on 👍
Later i'll work to refactor and separate Indominus MainWindow 👯‍♂️

@5bentz

This comment has been minimized.

5bentz commented Jun 19, 2018

I confirm that 5.6 is required for QtPass to compile, since Ubuntu 16.04 LTS cannot compile QtPass with Qt5.5. It's high time for me to upgrade to 1804! :]

@FiloSpaTeam

This comment has been minimized.

Contributor

FiloSpaTeam commented Jun 19, 2018

Seems good enough :D

@annejan

This comment has been minimized.

Member

annejan commented Jun 19, 2018

I'll update the docs and site tomorrow..

@5bentz

This comment has been minimized.

5bentz commented Jun 19, 2018

More precisely, the build stops at Qt::AA_EnableHighDpiScaling, saying that it is not a member of Qt.
And indeed, it was added in Qt5.6.

As a side note, I also casually #included <QWidget> in src/usersdialog.cpp to workaround errors here and there...

I'd like to mention I was able to build QtPass up to v1.2.1 for French support & the security 👍
Anyway, besides me, who's building Qt applications on Ubuntu Xenial 1604? 🤔

@FiloSpaTeam

This comment has been minimized.

Contributor

FiloSpaTeam commented Jun 20, 2018

More precisely, the build stops at Qt::AA_EnableHighDpiScaling, saying that it is not a member of Qt.
And indeed, it was added in Qt5.6.

I noticed this feature during one of the last pull request. 👍

I think we should consider that some distros use "old" version of Qt and we should adapt. :D

@annejan

This comment has been minimized.

Member

annejan commented Jun 20, 2018

Yah that sounds like something to easily macro out

@annejan

This comment has been minimized.

Member

annejan commented Jun 20, 2018

@5bentz I'll take your fixes and macro them in, currently installing clean Ubu 14 and 16 VMs to test 👍

Also I'll update documentation and site on this a bit . .

annejan added a commit that referenced this issue Jun 20, 2018

@annejan

This comment has been minimized.

Member

annejan commented Jun 20, 2018

Steps:

  • Clean Ubuntu 14.04.5 VM from osboxes.org
  • sudo apt-get update && sudo apt-get install git build-essential qt5-default qttools5-dev-tools
  • git clone https://github.com/ijhack/qtpass
  • cd qtpass
  • qmake && make

Qt Version 5.2.1 is now the lowest tested version and 5.11.1 the highest

@annejan

This comment has been minimized.

Member

annejan commented Jun 20, 2018

NB: The unit tests don't work on Ubuntu 14.04 probably some C++11 thing but the app runs like it should

@annejan

This comment has been minimized.

Member

annejan commented Jun 20, 2018

Same steps on Ubuntu 16.04.4 Qt Version 5.5.1
Same result

Ubuntu 12.04.5 apparently only has Qt3 and Qt4 standard . .
Swapped qt5-default qttools5-dev-tools for qt4-dev-tools Qt Version 4.8.1
No es bueno!! Many compile errors . .

So I'll just use a ppa by inserting the steps:

  • sudo apt-add-repository ppa:canonical-qt5-edgers/ubuntu1204-qt5
  • qmake -qt5

Qt Version 5.0.2
Almost the same as 4.8 . . and . . well . . I think we can all agree to 5.2 as minimum . . right?

Although I think at-least some of the issues on Ubuntu 12.04 are caused by the old version of gcc

@annejan annejan closed this Jun 20, 2018

@5bentz

This comment has been minimized.

5bentz commented Jun 25, 2018

Thanks! I see the GUI has seen a dramatic overhaul!

Ubuntu 16.04.4 LTS with Qt 5.5.1, gcc 5.4

I confirm I can compile it (with errors) but the app runs fine 💯

For those being surprised by the errors, the first and the last errors are:
First

In file included from tst_util.cpp:1:0:
../../../src/filecontent.h:20:36: error: expected ‘)’ before ‘<’ token
   NamedValues(std::initializer_list<NamedValue> values);

Gcc not understanding 'std::initializer_list'? Mmmh it is supported though.

Last

tst_util.cpp:100:56: error: no matching function for call to ‘NamedValues::NamedValues(<brace-enclosed initializer list>)’
In file included from tst_util.cpp:1:0:
../../../src/filecontent.h:19:3: note: candidate: NamedValues::NamedValues()
   NamedValues();

The rest are only errors in tst_* files (files for testing I assume).

@annejan

This comment has been minimized.

Member

annejan commented Jun 25, 2018

Yep, the tst_* files are for (unit) testing.

Perhaps we could add some flags (for GCC to understand C++11 stuff)
Might give it a try when I spin up an Ubuntu VM tomorrow . .

@annejan

This comment has been minimized.

Member

annejan commented Jun 26, 2018

And yes @5bentz . .

Adding CONFIG += c++11 to the tests/tests.pri file works like a charm and make check works on Ubuntu 16 and even 14 now!! 1cec9a3

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment