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

mysql: improve warnings when data directory is present. #60190

Closed
wants to merge 16 commits into from
Closed

mysql: improve warnings when data directory is present. #60190

Show file tree
Hide file tree
Changes from 12 commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
75c610a
[email protected]: Improve warnings when data directory is present.
peterb Aug 26, 2020
b9194c5
Pseudocode of what this should do.
peterb Aug 28, 2020
4497d80
Add note about 5.6 changes which are also required.
peterb Aug 28, 2020
f5c2c71
Move pseudocode to comments and flesh out some working code.
peterb Aug 28, 2020
943133e
Detect MySQL 5.6, refactor.
peterb Sep 1, 2020
a5ce548
Fix style issues.
peterb Sep 6, 2020
6992d7a
MySQL 5.6 formula installation directory.
peterb Sep 6, 2020
ab78b97
Remove pseudocode.
peterb Sep 6, 2020
17eb945
Add note about removing mysqlbug
peterb Sep 6, 2020
ee744af
Remove deprecated mysqlbug utility as it contains shims.
peterb Sep 6, 2020
56d318b
Use tmpdir for socket in test.
peterb Sep 7, 2020
93b8b07
Add notes on another ways of checking which the mysql version.
peterb Sep 8, 2020
e6936de
Update Formula/[email protected]
peterb Sep 9, 2020
2dd4960
Simply legacy mysql formulae by specifying a separate data directory
peterb Sep 9, 2020
94610a2
Merge branch 'patch-2' of https://github.com/peterb/homebrew-core int…
peterb Sep 9, 2020
d61101d
Remove custom socket as it appears that the CI error is unrelated.
peterb Sep 11, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 16 additions & 1 deletion Formula/[email protected]
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,19 @@ class MysqlAT56 < Formula
depends_on "[email protected]"

def datadir
var/"mysql"
return var/"mysql" if default_datadir_already_in_use_by_this_version?

var/"mysql56"
end

# TODO: Could also try running file on files in var/*.frm if they exist
# e.g.
# peter@Peters-iMac homebrew-core % file /usr/local/var/mysql56/mysql/columns_priv.frm
# /usr/local/var/mysql56/mysql/columns_priv.frm: MySQL table definition file Version 9,
# type MYISAM, MySQL version 50647
#
def default_datadir_already_in_use_by_this_version?
(var/"mysql").exist? && var.glob("mysql/*.dblwr").empty? && (var/"mysql/test").exist?
Copy link
Member

Choose a reason for hiding this comment

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

Think it's enough to just check for (var/"mysql").exist??

Copy link
Member

Choose a reason for hiding this comment

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

Could also output something in the caveats based on this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sounds good, I think that a simpler check and a caveat explaining the location of the datadir to the user would be preferable to trying to work it out automatically for them. I'll modify as you suggest.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Update: It turns out this is harder to do than I expected, as putting a conditional in the datadir method means it gets evaluated differently during various stages in the formula.

end

def install
Expand Down Expand Up @@ -62,6 +74,9 @@ def install
system "./mysql-test-run.pl", "status", "--vardir=#{Dir.mktmpdir}"
end

# Remove deprecated bug reporting tool
rm prefix/"bin/mysqlbug"

# Remove the tests directory
rm_rf prefix/"mysql-test"

Expand Down
12 changes: 10 additions & 2 deletions Formula/[email protected]
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,14 @@ class MysqlAT57 < Formula
end

def datadir
var/"mysql"
return var/"mysql" if default_datadir_already_in_use_by_this_version?

var/"mysql57"
end

# TODO: Could also try running file on files in var/*.frm if they exist
def default_datadir_already_in_use_by_this_version?
(var/"mysql").exist? && var.glob("mysql/*.dblwr").empty? && !(var/"mysql/test").exist?
end

def install
Expand Down Expand Up @@ -149,7 +156,8 @@ def plist

port = free_port
pid = fork do
exec bin/"mysqld", "--bind-address=127.0.0.1", "--datadir=#{dir}", "--port=#{port}"
exec bin/"mysqld", "--bind-address=127.0.0.1", "--datadir=#{dir}", "--port=#{port}",
"--socket=#{dir}/mysql.sock}"
end
sleep 2

Expand Down