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

Fix mariadb drivers #73928

Merged
merged 1 commit into from
Nov 23, 2019
Merged

Fix mariadb drivers #73928

merged 1 commit into from
Nov 23, 2019
+21 −14

Conversation

jamii
Copy link
Contributor

@jamii jamii commented Nov 22, 2019 •
edited
Loading

Motivation for this change

The mariadb unixODBC drivers fail to load with an unhelpful error message - #73258

I haven't been able to pin down what exactly is wrong, but I've found that building against the subrepo rather than against the system libmysqlclient leads to working drivers.

I've tested this PR against mariadb on NixOS and OS X Mojave.

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • 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 nix-review --run "nix-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)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.
Notify maintainers

cc @

Sorry, something went wrong.

@jamii jamii force-pushed the fix_mariadb_drivers branch from 053d2dc to 213da44 Compare November 22, 2019 18:49
@jamii
Copy link
Contributor Author

jamii commented Nov 22, 2019 

jamie@machine:~/nixpkgs$ nix-shell -p nix-review --run "nix-review wip"
these paths will be fetched (0.03 MiB download, 0.10 MiB unpacked):
  /nix/store/wvrisx65diwaf934clcx7gld33j7b5bg-nix-review-2.0.1
copying path '/nix/store/wvrisx65diwaf934clcx7gld33j7b5bg-nix-review-2.0.1' from 'https://cache.nixos.org'...
$ git fetch --force https://github.com/NixOS/nixpkgs master:refs/nix-review/0
remote: Enumerating objects: 1216, done.
remote: Counting objects: 100% (1216/1216), done.
remote: Compressing objects: 100% (4/4), done.
remote: Total 2489 (delta 1212), reused 1214 (delta 1212), pack-reused 1273
Receiving objects: 100% (2489/2489), 1.19 MiB | 2.63 MiB/s, done.
Resolving deltas: 100% (1786/1786), completed with 568 local objects.
From https://github.com/NixOS/nixpkgs
 * [new branch]              master     -> refs/nix-review/0
$ git worktree add /home/jamie/.cache/nix-review/rev-053d2dc590afd23d9ca7f2d354666ff3c6ea628e-dirty/nixpkgs 8960509fc5308dd4ab500f537c30ada256695d7a
Preparing worktree (detached HEAD 8960509fc53)
Updating files: 100% (20343/20343), done.
HEAD is now at 8960509fc53 cargo-make: 0.23.0 -> 0.24.0
$ nix-env -f /home/jamie/.cache/nix-review/rev-053d2dc590afd23d9ca7f2d354666ff3c6ea628e-dirty/nixpkgs -qaP --xml --out-path --show-trace
Applying `nixpkgs` diff...
$ nix-env -f /home/jamie/.cache/nix-review/rev-053d2dc590afd23d9ca7f2d354666ff3c6ea628e-dirty/nixpkgs -qaP --xml --out-path --show-trace --meta
Nothing changed
No packages were successfully build, skip nix-shell
$ git worktree prune

@jamii
Copy link
Contributor Author

jamii commented Nov 22, 2019

There don't appear to be any tests.

@jamii
Copy link
Contributor Author

jamii commented Nov 22, 2019

@aanderse

@aanderse aanderse added the 9.needs: port to stable A PR needs a backport to the stable release. label Nov 22, 2019
@aanderse
Copy link
Member

libmysqlclient is no longer being referenced so you should remove it. This will also make the backport to 19.09 easier.

@jamii jamii force-pushed the fix_mariadb_drivers branch from 213da44 to 9b88fc4 Compare November 23, 2019 11:55
aanderse
aanderse reviewed Nov 23, 2019
View reviewed changes
pkgs/development/libraries/unixODBCDrivers/default.nix Outdated
src = fetchFromGitHub {
owner = "MariaDB";
repo = "mariadb-connector-odbc";
rev = "c921b500650eca535010f12e0d83417d2ef915ec";
Copy link
Member

Choose a reason for hiding this comment

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

I'm terribly sorry I didn't notice this before... we generally set rev to something involving version for easy updates. So in this case:

Suggested change
rev = "c921b500650eca535010f12e0d83417d2ef915ec";
rev = version;

I've tested this on NixOS and after this change I'm happy to merge to master. This change deserves a backport as well. Thank you so much for digging into this! Hopefully at some point someone can get to the bottom of our libmysqlclient issue, but until that happens the priority is to have a working package 😄

Thanks again @jamii 🎉

@jamii
unixODBCDrivers.mariadb: build against subrepo instead of libmysqlclient

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
GPG key ID: 4AEE18F83AFDEB23
Expired
Verified
Learn about vigilant mode
fa9a981 
For unknown reasons, building against libmysqlclient results in
unixODBC reporting an error when trying to load the driver, but
building against the subrepo works fine.

Fixes NixOS#73928
@jamii jamii force-pushed the fix_mariadb_drivers branch from 9b88fc4 to fa9a981 Compare November 23, 2019 13:08
aanderse
aanderse approved these changes Nov 23, 2019
View reviewed changes
Copy link
Member

@aanderse aanderse left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@aanderse aanderse merged commit 216f0e6 into NixOS:master Nov 23, 2019
@aanderse
Copy link
Member

Thank you very much @jamii! Are you going to open a backport PR?

@jamii
Copy link
Contributor Author

jamii commented Nov 25, 2019

Not soon. I was asked to spend less time working on nix stuff 🤷‍♂️

@aanderse
Copy link
Member

I'll grab it then. Thanks again... and sorry to hear :-(

@Janik-Haag Janik-Haag added the 12. first-time contribution This PR is the author's first one; please be gentle! label Jun 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@aanderse aanderse

Assignees
No one assigned
Labels
8.has: package (new) This PR adds a new package 9.needs: port to stable A PR needs a backport to the stable release. 10.rebuild-darwin: 1-10 10.rebuild-linux: 1-10 12. first-time contribution This PR is the author's first one; please be gentle!
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@jamii @aanderse @Janik-Haag