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

sphinxsearch: Add support for MySQL & xmlpipe2 #80727

Merged
merged 5 commits into from Feb 27, 2020
Merged

sphinxsearch: Add support for MySQL & xmlpipe2 #80727

merged 5 commits into from Feb 27, 2020

Conversation

Valodim
Copy link
Contributor

@Valodim Valodim commented Feb 21, 2020 •
edited

Motivation for this change

This PR obsoletes #79032.

I added support for MySQL sources, and optionally xmlpipe2. I don't use xmlpipe2 myself, so I didn't test that. I'll test MySQL source in the next few days in a small production environment. :)

The version and main source were pulled in as arguments? Not sure what that was about, overriding those should be done with overrideAttrs. I also enabled parallel builds (works fine for me), and added myself as maintainer. The 2.X branch of sphinxsearch isn't actively supported anymore though, so I don't expect any updates. The 3.X line isn't open source (yet), and has enough incompatibilities to go into its own sphinxsearch3 package anyways.

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • Built on platform(s)
    • NixOS
  • 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 nixpkgs-review --run "nixpkgs-review wip" no packages depend on this
  • 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.

@Valodim
Copy link
Contributor Author

Valodim commented Feb 21, 2020

Whoops. I built this against release-19.09 and it seems I didn't correctly rebase to master. will fix.

@Valodim
Copy link
Contributor Author

Valodim commented Feb 21, 2020

Ok, should work now. Closure sizes:

sphinxsearch on master
/nix/store/id1lrdwxwwlndsb5w3hr4v69zlhg5i4l-sphinxsearch-2.2.11 57.2M
sphinxsearch on pr, enableMysql = false
/nix/store/8asbjxjxzbilhc25r1mj01zs2slhg71b-sphinxsearch-2.2.11 57.2M
sphinxsearch on pr, enableMysql = true (default)
/nix/store/qjhabpy7dhvs8s3cmhzvzz2kkasvk6p2-sphinxsearch-2.2.11 73.5M
sphinxsearch on pr, enableMysql = true, enableXmlpipe2 = true
/nix/store/z39nqslg1z21x3kc5smxcyqfqpp06ixk-sphinxsearch-2.2.11 73.8M

@ofborg ofborg bot requested a review from ederoyd46 February 21, 2020 14:51
@Valodim
Copy link
Contributor Author

Valodim commented Feb 21, 2020

Running in production, works just fine with a mysql source 👍

@Valodim
Copy link
Contributor Author

Valodim commented Feb 27, 2020

Can I get a review on this one? :) @jtojnar @vvs-

@Valodim
Copy link
Contributor Author

Valodim commented Feb 27, 2020

Thanks for the review @jtojnar! Fixed and tested compilation in all build configurations.

@jtojnar
Copy link
Contributor

jtojnar commented Feb 27, 2020

@GrahamcOfBorg build sphinxsearch

vvs-
vvs- reviewed Feb 27, 2020
View reviewed changes
Copy link

@vvs- vvs- left a comment

Choose a reason for hiding this comment

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

I guess there is no policy to prefer some particular database? This looks specific to NixOS as some other distributions I know always link to MariaDB for some reason.

@Valodim
Copy link
Contributor Author

Valodim commented Feb 27, 2020 •
edited

Build failure on darwin :( seems like it's due to this upstream bug: http://sphinxsearch.com/bugs/view.php?id=2578

I imported a workaround for this particular issue from Homebrew:
https://github.com/Homebrew/homebrew-core/pull/8619/files#diff-df176d75fcc42a0ade34983e9ffc3193R47

I don't have a macOS to test on, unfortunately.

\\ edit

seems github is having some problems, my pushed fix didn't make it through yet :P

@jtojnar
Copy link
Contributor

jtojnar commented Feb 27, 2020

@GrahamcOfBorg build sphinxsearch

@Valodim
Copy link
Contributor Author

Valodim commented Feb 27, 2020

🎉

@jtojnar jtojnar merged commit 1a4ab0e into NixOS:master Feb 27, 2020
@jtojnar
Copy link
Contributor

jtojnar commented Feb 27, 2020

Thanks.

@Valodim Valodim deleted the sphinxsearch-master branch February 27, 2020 16:33
@jtojnar jtojnar mentioned this pull request Feb 27, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jtojnar jtojnar jtojnar left review comments

@vvs- vvs- vvs- left review comments

@ederoyd46 ederoyd46 Awaiting requested review from ederoyd46

Assignees
No one assigned
Labels
10.rebuild-darwin: 1-10 10.rebuild-linux: 1-10 11.by: package-maintainer
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@Valodim @jtojnar @vvs-