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.

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

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.

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 GrahamcOfBorg added 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild 10.rebuild-linux: 1-10 labels Jul 4, 2018
@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

{ 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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
10.rebuild-darwin: 0 This PR does not cause any packages to rebuild 10.rebuild-linux: 1-10
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants