IJHack / QtPass

QtPass is a multi-platform GUI for pass, the standard unix password manager.
https://qtpass.org/
GNU General Public License v3.0
1.03k stars 162 forks source link

We should select a minimum Qt version #371

Closed FiloSpaTeam closed 6 years ago

FiloSpaTeam commented 6 years ago

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 :)

FiloSpaTeam commented 6 years ago

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 commented 6 years ago

So the minimum version is Qt 5.10? :dancer:

annejan commented 6 years ago

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 commented 6 years ago

Ok so i'll work to migrate all to Qt 5. We have some connect in old style and so on :+1: Later i'll work to refactor and separate Indominus MainWindow :dancing_men:

5bentz commented 6 years ago

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 commented 6 years ago

Seems good enough :D

annejan commented 6 years ago

I'll update the docs and site tomorrow..

5bentz commented 6 years ago

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 :+1: Anyway, besides me, who's building Qt applications on Ubuntu Xenial 1604? :thinking:

FiloSpaTeam commented 6 years ago

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. :+1:

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

annejan commented 6 years ago

Yah that sounds like something to easily macro out

annejan commented 6 years ago

@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 commented 6 years ago

Steps:

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

annejan commented 6 years ago

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

annejan commented 6 years ago

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:

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

5bentz commented 6 years ago

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 :100:

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 commented 6 years ago

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 commented 6 years ago

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!! https://github.com/IJHack/QtPass/commit/1cec9a3a5892e2a2fc3023bc87f9c0a78f1a5cd2