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.

Sign up for GitHub

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

Jump to bottom

qscintilla: 2.9.4 -> 2.11.2 and sqlitebrowser: 3.11.2 #56034

Merged
merged 3 commits into from Sep 28, 2019
Merged

qscintilla: 2.9.4 -> 2.11.2 and sqlitebrowser: 3.11.2 #56034

merged 3 commits into from Sep 28, 2019

Conversation

peterhoeg
Copy link
Member

@peterhoeg peterhoeg commented Feb 19, 2019 •
edited

Motivation for this change

sqlitebrowser needs a newer version of qscintilla to work which is included here.

The qt4 version is broken - I will have to run nix-review on this thing to assess the impact.

Anybody on mac who can help to verify on darwin?

Cc: @Ma27 @matthewbauer

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nox --run "nox-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Assured whether relevant documentation is up to date
  • Fits CONTRIBUTING.md.

@peterhoeg
Copy link
Member Author

@GrahamcOfBorg build libsForQt5.qscintilla libsForQt511.qscintilla minc_widgets octave octaveFull octaveHg openscad python27Packages.ovito python27Packages.qscintilla python27Packages.qscintilla-qt5 python37Packages.ovito python37Packages.qscintilla-qt5 qgis qscintilla sonic-pi sqlitebrowser sqliteman tora tortoisehg

@Ma27
Copy link
Member

Ma27 commented Feb 19, 2019

First of all thanks a lot for your work! I just checked out your branch and tested the changes. I noticed the following things:

  • sqlitebrowser broke with a QT plugin failure at startup. I already pushed a fix. I observed the same issue with tora, I'll push a fix for this as well, but rebuilds of it take a bit.
  • The qt4 variant of qscintilla currently fails to build.
  • I currently don't see, how the arpack bump is related to the qscintilla update and probably out of scope, but I may be missing something.

Regarding the broken darwin build: I don't own Apple hardware, so I doubt that I'll be helpful there. IMHO it was right to mark the build as broken on darwin for now, I'd keep that as-is unless a darwin user wants to investigate :)

@Ma27
Copy link
Member

Ma27 commented Feb 19, 2019

tora and sqlitebrowser are fixed now btw :)

@peterhoeg
Copy link
Member Author

Thanks @Ma27 for feedback and the patch.

The arpack change was due to nix review failing when testing this but otherwise unrelated.

@peterhoeg peterhoeg changed the title qscintilla: 2.9.4 -> 2.11.1 and sqlitebrowser: 3.11.1 [WIP] qscintilla: 2.9.4 -> 2.11.1 and sqlitebrowser: 3.11.1 Apr 24, 2019
@peterhoeg
Copy link
Member Author

@GrahamcOfBorg build sqlitebrowser

@Ma27 Ma27 mentioned this pull request May 11, 2019
Merged
10 tasks
@FRidh
Copy link
Member

FRidh commented May 14, 2019

What is the status of this PR?

Ma27 added a commit to Ma27/nixpkgs that referenced this pull request May 14, 2019
@Ma27
sqlitebrowser: 3.10.1 -> 3.11.2
629edee 
The latest releases of `sqlitebrowser` contain several new features like
a dark mode, improved CSV import/export and a lot of new bugfixes:

https://github.com/sqlitebrowser/sqlitebrowser/releases/tag/v3.11.2
https://github.com/sqlitebrowser/sqlitebrowser/releases/tag/v3.11.1
https://github.com/sqlitebrowser/sqlitebrowser/releases/tag/v3.11.0

Also some minor changes were applied to the package definition:

* Using the `pname`/`version` convention now.
* Use internal `qscintilla` rather than our package to fix the build
  (sqlitebrowser/sqlitebrowser#1348 (comment)).
  This can probably be removed when NixOS#56034 is merged.
* Dropped patch which removed Test and Print support as QT 5.12 is now
  used to build the application.
@c0bw3b
Copy link
Contributor

c0bw3b commented May 17, 2019

@GrahamcOfBorg build qscintilla libsForQt5.qscintilla

c0bw3b
c0bw3b reviewed May 17, 2019
View reviewed changes
pkgs/development/libraries/qscintilla/default.nix Outdated

name = "${pname}-${if withQt5 then "qt5" else "qt4"}-${version}";

src = fetchurl {
url = "mirror://sourceforge/pyqt/QScintilla2/QScintilla-${version}/QScintilla_gpl-${version}.zip";
sha256 = "04678skipydx68zf52vznsfmll2v9aahr66g50lcqbr6xsmgr1yi";
url = "https://www.riverbankcomputing.com/static/Downloads/QScintilla/QScintilla_gpl-${version}.tar.gz";
Copy link
Contributor

Choose a reason for hiding this comment

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

Already moved to
https://www.riverbankcomputing.com/static/Downloads/QScintilla/2.11.1/QScintilla_gpl-2.11.1.tar.gz

pull bot pushed a commit to evanjs/nixpkgs that referenced this pull request May 23, 2019
@Ma27 @FRidh
sqlitebrowser: 3.10.1 -> 3.11.2
731fdbe 
The latest releases of `sqlitebrowser` contain several new features like
a dark mode, improved CSV import/export and a lot of new bugfixes:

https://github.com/sqlitebrowser/sqlitebrowser/releases/tag/v3.11.2
https://github.com/sqlitebrowser/sqlitebrowser/releases/tag/v3.11.1
https://github.com/sqlitebrowser/sqlitebrowser/releases/tag/v3.11.0

Also some minor changes were applied to the package definition:

* Using the `pname`/`version` convention now.
* Use internal `qscintilla` rather than our package to fix the build
  (sqlitebrowser/sqlitebrowser#1348 (comment)).
  This can probably be removed when NixOS#56034 is merged.
* Dropped patch which removed Test and Print support as QT 5.12 is now
  used to build the application.
@peterhoeg peterhoeg changed the title qscintilla: 2.9.4 -> 2.11.1 and sqlitebrowser: 3.11.1 qscintilla: 2.9.4 -> 2.11.2 and sqlitebrowser: 3.11.2 Aug 26, 2019
@peterhoeg
qscintilla: 2.9.4 -> 2.11.2
76ad279 
We strip the library suffix as we don't need it and it confuses various
downstream consumers.

Also replace calls to sed with substituteInPlace as the latter will complain in
case it doesn't perform a substitution.
@peterhoeg
sqlitebrowser: do not use vendored qscintilla
96f5d79
@peterhoeg
Copy link
Member Author

I'll run nix-review on this when I get to a slightly less anemic machine...

@peterhoeg
Copy link
Member Author

@GrahamcOfBorg build sqlitebrowser

@peterhoeg
Copy link
Member Author

@GrahamcOfBorg build sqlitebrowser

@lsix
Copy link
Member

lsix commented Sep 19, 2019

I have another fix for qscintilla in #68375 (did not see you did it already). My modifications are slightly different (I keep the _qt? suffix, just symling it to libqscintilla2.so). I guess both approaches are fine, but one needs to make it to master since the qscintilla update holds on other updates.

@ttuegel ttuegel merged commit fccbb5d into NixOS:master Sep 28, 2019
gebner added a commit to gebner/nixpkgs that referenced this pull request Oct 3, 2019
@gebner
qscintilla: fixup qmake files
58ba18d 
This is fallout from NixOS#56034
@gebner gebner mentioned this pull request Oct 3, 2019
Closed
10 tasks
@peterhoeg peterhoeg deleted the u/sqlite branch October 21, 2019 14:23
@peterhoeg peterhoeg restored the u/sqlite branch October 21, 2019 14:48
@peterhoeg peterhoeg deleted the u/sqlite branch October 22, 2019 02:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@c0bw3b c0bw3b c0bw3b left review comments

@ttuegel ttuegel ttuegel approved these changes

@Ma27 Ma27 Awaiting requested review from Ma27

Assignees
No one assigned
Labels
8.has: package (new) 8.has: package (update) 10.rebuild-darwin: 11-100 10.rebuild-linux: 11-100
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
9 participants
@peterhoeg @Ma27 @FRidh @c0bw3b @lsix @ttuegel @ryantm @infinisil @GrahamcOfBorg