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

sqlitebrowser: fix build #43047

Closed
wants to merge 1 commit into from
Closed

sqlitebrowser: fix build #43047

wants to merge 1 commit into from

Conversation

Ma27
Copy link
Member

@Ma27 Ma27 commented Jul 4, 2018 •
edited

Motivation for this change

This patch slightly cleans up the sqlitebrowser derivation and
switches from a cmake-based build to a qmake build using
qmakeConfigurePhase and the corresponding hook.

QMake is far more suitable for QT-based builds (and suggested e.g. here
#42320 (comment)),
furthermore the CMake build suffers from issues like missing commands
including qt5_use_modules.

Fixes #42320
See https://hydra.nixos.org/build/76952038

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)
  • Fits CONTRIBUTING.md.

@Ma27
sqlitebrowser: fix build
4f15883 
This patch slightly cleans up the `sqlitebrowser` derivation and
switches from a cmake-based build to a qmake build using
`qmakeConfigurePhase` and the corresponding hook.

QMake is far more suitable for QT-based builds (and suggested e.g. here
NixOS#42320 (comment)),
furthermore the CMake build suffers from issues like missing commands
including `qt5_use_modules`.

Fixes NixOS#42320
See https://hydra.nixos.org/build/76952038
@Ma27
Copy link
Member Author

Ma27 commented Jul 4, 2018

/cc @peterhoeg @sjau

@orivej
Copy link
Contributor

orivej commented Jul 4, 2018

@GrahamcOfBorg build sqlitebrowser

@GrahamcOfBorg
Copy link

No attempt on x86_64-darwin (full log)

The following builds were skipped because they don't evaluate on x86_64-darwin: sqlitebrowser

Partial log (click to expand)


a) For `nixos-rebuild` you can set
  { nixpkgs.config.allowUnsupportedSystem = true; }
in configuration.nix to override this.

b) For `nix-env`, `nix-build`, `nix-shell` or any other Nix command you can add
  { allowUnsupportedSystem = true; }
to ~/.config/nixpkgs/config.nix.

@GrahamcOfBorg
Copy link

No attempt on aarch64-linux (full log)

The following builds were skipped because they don't evaluate on aarch64-linux: sqlitebrowser

Partial log (click to expand)


a) For `nixos-rebuild` you can set
  { nixpkgs.config.allowUnfree = true; }
in configuration.nix to override this.

b) For `nix-env`, `nix-build`, `nix-shell` or any other Nix command you can add
  { allowUnfree = true; }
to ~/.config/nixpkgs/config.nix.

@GrahamcOfBorg
Copy link

Success on x86_64-linux (full log)

Attempted: sqlitebrowser

Partial log (click to expand)

post-installation fixup
shrinking RPATHs of ELF executables and libraries in /nix/store/r53p4lkbas5j5spkvi44jhc13x4lnrxi-sqlitebrowser-3.10.1
shrinking /nix/store/r53p4lkbas5j5spkvi44jhc13x4lnrxi-sqlitebrowser-3.10.1/bin/sqlitebrowser
strip is /nix/store/4qvrxzxa535y8304mk195x50b6p9607d-binutils-2.30/bin/strip
stripping (with command strip and flags -S) in /nix/store/r53p4lkbas5j5spkvi44jhc13x4lnrxi-sqlitebrowser-3.10.1/bin
patching script interpreter paths in /nix/store/r53p4lkbas5j5spkvi44jhc13x4lnrxi-sqlitebrowser-3.10.1
checking for references to /build in /nix/store/r53p4lkbas5j5spkvi44jhc13x4lnrxi-sqlitebrowser-3.10.1...
postPatchMkspecs
postPatchMkspecs
/nix/store/r53p4lkbas5j5spkvi44jhc13x4lnrxi-sqlitebrowser-3.10.1

@GrahamcOfBorg
Copy link

No attempt on aarch64-linux (full log)

The following builds were skipped because they don't evaluate on aarch64-linux: sqlitebrowser

Partial log (click to expand)


a) For `nixos-rebuild` you can set
  { nixpkgs.config.allowUnfree = true; }
in configuration.nix to override this.

b) For `nix-env`, `nix-build`, `nix-shell` or any other Nix command you can add
  { allowUnfree = true; }
to ~/.config/nixpkgs/config.nix.

@GrahamcOfBorg
Copy link

Success on x86_64-linux (full log)

Attempted: sqlitebrowser

Partial log (click to expand)

/nix/store/r53p4lkbas5j5spkvi44jhc13x4lnrxi-sqlitebrowser-3.10.1

orivej
orivej reviewed Jul 4, 2018
View reviewed changes
pkgs/development/tools/database/sqlitebrowser/default.nix
{ mkDerivation, lib, fetchFromGitHub, cmake, antlr
, qtbase, qttools, qscintilla, sqlite }:
{ mkDerivation, lib, fetchFromGitHub, qmake, antlr, qscintilla
, qtbase, qttools, sqlite
Copy link
Contributor

Choose a reason for hiding this comment

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

Did you mean to move qscintilla up?

Copy link
Member Author

Choose a reason for hiding this comment

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

nope. in fact sqlitebrowser provides its own qscintila for the build process (but not even the old CMake derivation used this.
But you're absolutely right, I'll revert this to make the diff easier to read.

@orivej
Copy link
Contributor

orivej commented Jul 5, 2018

I have merged #42351, so this PR is no longer necessary. I'd prefer to keep the cmake based build because it appears to be in better shape (no /usr/local, no bundled qscintila) and it is described in the building instructions. I'd still appreciate you becoming the maintainer of this package.

@Ma27
Copy link
Member Author

Ma27 commented Jul 5, 2018

I'd prefer to keep the cmake based build because it appears to be in better shape (no /usr/local, no bundled qscintila) and it is described in the building instructions.

I've always learnt that qmake is more suited for QT-based builds than cmake, but you're right in this case it seems to be better to ues cmake (and I'm not really interested in fixing the qmake instructions upstream 😄 )

I'd still appreciate you becoming the maintainer of this package.

Of course, feel free to add me to the maintainers list! :)
It took me a little while to fully understand the builds of thi spackage so I guess in case of further breakage I could help out %)

@Ma27 Ma27 closed this Jul 5, 2018
@Ma27 Ma27 deleted the fix-sqlitebrowser branch July 5, 2018 09:57
orivej-nixos pushed a commit that referenced this pull request Jul 5, 2018
@orivej
sqlitebrowser: list ma27 as the maintainer (from #43047)
136b947
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@orivej orivej orivej left review comments

Assignees
No one assigned
Labels
10.rebuild-darwin: 0 10.rebuild-linux: 1-10
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@Ma27 @orivej @GrahamcOfBorg