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

sphinxsearch: Add support for MySQL & xmlpipe2 #79032

Closed
wants to merge 2 commits into from
Closed

Conversation

@vvs-
Copy link

@vvs- vvs- commented Feb 1, 2020

Motivation for this change

Sphinxsearch doesn't support MySQL sources and xmlpipe2
Closes #70076

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 nixpkgs-review --run "nixpkgs-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.
Copy link
Contributor

@jtojnar jtojnar left a comment

Could you follow out commit style? For example, there could be two commits named like:

  • sphinxsearch: Add support for xmlpipe2
  • sphinxsearch: Add support for MySQL sources
"--enable-id64"
];

nativeBuildInputs = [
pkgconfig
];

buildInputs = [
expat mariadb zlib openssl

This comment has been minimized.

@jtojnar

jtojnar Feb 4, 2020
Contributor

Could you make this one per line? And mention why is zlib or openssl needed?

This comment has been minimized.

@vvs-

vvs- Feb 4, 2020
Author

I should reiterate that I'm not an expert. I don't know exactly why those libraries are needed, but without them configure test for MySQL presence will fail as they are linked there after mariadb.

@jtojnar
Copy link
Contributor

@jtojnar jtojnar commented Feb 4, 2020

Also could you evaluate the changes in closure size as suggested in the OP:

Determined the impact on package closure size (by running nix path-info -h -S $(nix-build --no-out-link -A sphinxsearch) before and after)

@vvs-
Copy link
Author

@vvs- vvs- commented Feb 4, 2020

I'm not sure if I will have time as this is not a priority for me.

BTW don't take it personally, but shouldn't package maintainers care for such things? And I already gave anyone on nixpkgs team write access to my repository branch if they feel inclined to work on it.

@Valodim
Copy link
Contributor

@Valodim Valodim commented Feb 21, 2020

I'll pick this up and make a PR in the next few days. Thanks for the starting point @vvs- :)

@vvs-
Copy link
Author

@vvs- vvs- commented Feb 21, 2020

You are welcome. I'm willing to help, but I can't afford to do everything alone.

@jtojnar jtojnar changed the title Issue #70076 sphinxsearch: Add support for MySQL & xmlpipe2 Feb 21, 2020
@Valodim Valodim mentioned this pull request Feb 21, 2020
5 of 8 tasks complete
@jtojnar jtojnar closed this Feb 27, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

4 participants
You can’t perform that action at this time.