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
sqlitebrowser: 3.10.0 -> 3.10.1 and make it work with Qt 5.9 #31120
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Builds and starts here, I'll give it a go! 👍
|
||
cmakeFlags = [ "-DUSE_QT5=TRUE" ]; | ||
|
||
# A regression was introduced in CMakeLists.txt on v3.9.x | ||
# See https://github.com/sqlitebrowser/sqlitebrowser/issues/832 and issues/755 | ||
postPatch = '' | ||
substituteInPlace CMakeLists.txt --replace 'project("DB Browser for SQLite")' 'project(sqlitebrowser)' | ||
substituteInPlace CMakeLists.txt \ | ||
--replace 'project("DB Browser for SQLite")' 'project(sqlitebrowser)' \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That substitution is not needed anymore: CMakeLists.txt was fixed upstream for 3.10.x
You can also remove the 2 lines of comments above regarding this past issue.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done (and thanks!)
mkDerivation rec { | ||
version = "3.10.0"; | ||
let | ||
qscintillaLib = (qscintilla.override { withQt5 = true; }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
libsForQt5
provide qscintilla
with withQt5
, no need to override.
homepage = http://sqlitebrowser.org/; | ||
license = licenses.gpl3; | ||
maintainers = with maintainers; [ matthiasbeyer ]; | ||
platforms = platforms.linux; # can only test on linux |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please, don't align attributes for simplicity and consistency with other packages.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We need a style guide...
Unless @ttuegel has any problems with the ugly "make it work with qt 5.9" stuff in here, I'm going to go ahead and merge this. |
Motivation for this change
We were building it against qt 5.6 but it can be made to work with 5.9. Not sure if this is the right way to do this though (getting rid of references to Test and PrintSupport).
Cc: @matthiasbeyer @ttuegel
Things done
build-use-sandbox
innix.conf
on non-NixOS)nix-shell -p nox --run "nox-review wip"
./result/bin/
)