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

mysql-client 8.0.18; create mysql-client@5.7; merge mysql-connector-c into mysql-client #46566

Closed
wants to merge 21 commits into from

Conversation

Bo98
Copy link
Member

@Bo98 Bo98 commented Nov 9, 2019

  • Have you followed the guidelines for contributing?
  • Have you checked that there aren't other open pull requests for the same formula update/change?
  • Have you built your formula locally with brew install --build-from-source <formula>, where <formula> is the name of the formula you're submitting?
  • Is your test running fine brew test <formula>, where <formula> is the name of the formula you're submitting?
  • Does your build pass brew audit --strict <formula> (after doing brew install <formula>)?

Note that I have not yet tested all the formulae I have revision bumped. It is possible some will be incompatible with 8.0 and will require to depend on mysql-client@5.7.

@Bo98
Copy link
Member Author

Bo98 commented Nov 9, 2019

It would help if I wasn't an idiot: ntopng: undefined method 'revision' for #<SoftwareSpec:0x00007fee5bca8f68>

@Bo98
Copy link
Member Author

Bo98 commented Nov 10, 2019

No compile errors? I'm suspicious.

Makes me wonder if we need mysql-client@5.7. Yes we should because some APIs have changed but seemingly nothing affecting anything on Homebrew surprisingly.

@Bo98 Bo98 marked this pull request as ready for review November 10, 2019 00:23
@Bo98 Bo98 mentioned this pull request Nov 10, 2019
17 tasks
@fxcoudert
Copy link
Member

poke @commitay who introduced this formula, with its Pinned at 5.7.*` comment: #28085 & #27210

@commitay
Copy link
Contributor

When mysql was upgraded to 8.0 it couldn't be built on el capitan, which still had several months of support left at the time. As there was a dozen or so formulae with a mysql dep we made mysql-client rather than pinning all of them to the versioned mysql@5.7 formula.

@fxcoudert
Copy link
Member

OK so now we probably simply want to remove mysql-client, right?

@Bo98
Copy link
Member Author

Bo98 commented Nov 14, 2019

It comes to no surprise that mysql-client can be seen as MySQL without the server part. And indeed it is the same source code.
Equally, mysql-client can be seen as the Connector/C replacement - and indeed it recommended for people wanting to connect to MySQL servers as clients.

Although El Capitan may be the reason for pinning 5.7, I don't really see how that was the reason to have created mysql-client over using the already existing mysql@5.7 formula.

mysql-client vs mysql isn't actually too dissimilar to mariadb-connector-c vs mariadb, though they're developed slightly differently than the way MySQL do it. One advantage I see is that you can have mysql-client and mariadb-connector-c installed together without conflicts, but you cannot have mysql and mariadb installed together without unlinking one. mysql-client also allows formulae to work with any of mysql, mariadb and percona-server as the actual server. Though I suppose all of these could be done anyway with brew unlink.

Whatever path is chosen, I strongly encourage having the mysql-connector-c -> whatever rename, particular with the high install count. The mapping to mysql-client was natural because they became synonyms. The mapping to mysql server less so but it is just a superset of the client.

@fxcoudert fxcoudert added the maintainer feedback Additional maintainers' opinions may be needed label Nov 16, 2019
@fxcoudert
Copy link
Member

@Homebrew/core what's your opinion?

Copy link
Member

@chenrui333 chenrui333 left a comment

Choose a reason for hiding this comment

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

Lgtm

@SMillerDev
Copy link
Member

Works for me. And after this all the bumped formula depend on "mysql-client"? Or still on the connector naming?

@Bo98
Copy link
Member Author

Bo98 commented Nov 17, 2019

There is no change the dependencies. The bumped formulae depended on mysql-client before. Nothing depended on mysql-connector-c, though there's some conflicts_with I should probably remove that I forgot about.

@SMillerDev
Copy link
Member

Yeah, that would be great.

@SMillerDev SMillerDev added the almost there PR is nearly ready to merge label Nov 17, 2019
@Bo98
Copy link
Member Author

Bo98 commented Nov 17, 2019

The old CI job expired anyway so me doing that will get a fresh one again.

@fxcoudert fxcoudert removed almost there PR is nearly ready to merge maintainer feedback Additional maintainers' opinions may be needed labels Nov 18, 2019
@fxcoudert fxcoudert closed this in 05faeff Nov 18, 2019
@Bo98 Bo98 deleted the mysql-client branch November 18, 2019 11:17
jebrosen added a commit to jebrosen/Rocket that referenced this pull request Dec 1, 2019
Homebrew/homebrew-core#46566 merged 'mysql-connector-c' into
'mysql-client', which does not install into the same paths.
@lock lock bot added the outdated PR was locked due to age label Jan 2, 2020
@lock lock bot locked as resolved and limited conversation to collaborators Jan 2, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
outdated PR was locked due to age
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants