Skip to content

mariadb-connector-c: fix socket path#70010

Merged
aanderse merged 1 commit intoNixOS:masterfrom
ajs124:fix/mariadb-connector-c-socket-path
Oct 10, 2019
Merged

mariadb-connector-c: fix socket path#70010
aanderse merged 1 commit intoNixOS:masterfrom
ajs124:fix/mariadb-connector-c-socket-path

Conversation

@ajs124
Copy link
Member

@ajs124 ajs124 commented Sep 29, 2019

Motivation for this change

Fix socket path. The cmakeFlag changed from MYSQL_ to MARIADB_ some time recently.
This broke my setup.

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 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 @thoughtpolice

@ofborg ofborg bot requested a review from globin September 29, 2019 11:32
@ofborg ofborg bot added 10.rebuild-darwin: 501+ This PR causes many rebuilds on Darwin and should normally target the staging branches. 10.rebuild-darwin: 501-1000 This PR causes many rebuilds on Darwin and should normally target the staging branches. 10.rebuild-linux: 501+ This PR causes many rebuilds on Linux and should normally target the staging branches. 10.rebuild-linux: 1001-2500 This PR causes many rebuilds on Linux and should target the staging branches. labels Sep 29, 2019
@aanderse
Copy link
Member

cc @Izorkin for informational purposes.

@dasJ
Copy link
Member

dasJ commented Sep 30, 2019

@ajs124 Does this need a backport?

@ajs124
Copy link
Member Author

ajs124 commented Sep 30, 2019

No backport needed, I think. Maybe to 19.09? MariaDB packaging works quite different on 19.03.

@Izorkin
Copy link
Contributor

Izorkin commented Oct 2, 2019

No need change PR to staging?

@ajs124 ajs124 mentioned this pull request Oct 9, 2019
10 tasks
@aanderse
Copy link
Member

aanderse commented Oct 9, 2019

@GrahamcOfBorg test wordpress

@ajs124 I'm surprised this hasn't been merged yet... I assume without this change mariadb-connector-c throws a warning during compilation or something? Just so we can say we dotted our i's and crossed our t's can you please list an application or two you tested with this change?

@ajs124
Copy link
Member Author

ajs124 commented Oct 9, 2019

@aanderse That's the thing. No warning, no nothing. At least from what I saw. Just that the path is different in the library.

I tested it with my dovecot and @mmilata seems to have run into it with his sympa PR. I'm sure there are more things out there, but most applications probably connect to MariaDB over TCP, so they won't run into this.

@aanderse
Copy link
Member

aanderse commented Oct 9, 2019

Turns out wordpress isn't a good test to show this works.

I assume this was mentioned in release notes at some point? Maybe we can link that? If @Izorkin approves this PR and @mmilata confirms this fixes his issue I would consider that adequate testing, review, and peer review for merge.

@ajs124
Copy link
Member Author

ajs124 commented Oct 9, 2019

I can't find a changelog entry, but this commit seems to be responsible. It's from 2016, but it seems like nixpkgs only has 3.x since August of this year.

@Izorkin
Copy link
Contributor

Izorkin commented Oct 9, 2019

I do not use a socket connection. I do not know how to check.

@Izorkin
Copy link
Contributor

Izorkin commented Oct 9, 2019

@mmilata
Copy link
Member

mmilata commented Oct 9, 2019

Confirming that this fixes my issue. Thanks for pinging me, yesterday I spent some time being really puzzled why it's happening without being able to trace it to this package.

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.

Seems straight forward enough. Merging based on feedback and detective work from all parties involved. Thanks everyone! 🎉

@aanderse aanderse merged commit f878596 into NixOS:master Oct 10, 2019
@ajs124 ajs124 deleted the fix/mariadb-connector-c-socket-path branch February 7, 2020 22:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

10.rebuild-darwin: 501-1000 This PR causes many rebuilds on Darwin and should normally target the staging branches. 10.rebuild-darwin: 501+ This PR causes many rebuilds on Darwin and should normally target the staging branches. 10.rebuild-linux: 501+ This PR causes many rebuilds on Linux and should normally target the staging branches. 10.rebuild-linux: 1001-2500 This PR causes many rebuilds on Linux and should target the staging branches.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants