Bug 209579 - databases/mysql57-server, databases/mysql57-client: Make my.cnf path correct according to hier(7) and last upgrade
Summary: databases/mysql57-server, databases/mysql57-client: Make my.cnf path correct ...
Status: Closed FIXED
Alias: None
Product: Ports & Packages
Classification: Unclassified
Component: Individual Port(s) (show other bugs)
Version: Latest
Hardware: Any Any
: --- Affects Some People
Assignee: Thomas Zander
URL:
Keywords: patch, patch-ready
Depends on:
Blocks: 210758
  Show dependency treegraph
 
Reported: 2016-05-17 14:00 UTC by Mahdi Mokhtari
Modified: 2016-07-04 15:22 UTC (History)
3 users (show)

See Also:


Attachments
better inform users about new default location for my.cnf (3.13 KB, patch)
2016-05-17 14:22 UTC, Markus Kohlmeyer
no flags Details | Diff
Backport of databases/mariadb101-server/files/patch-mysys_my__default.c (4.11 KB, patch)
2016-05-17 15:10 UTC, Markus Kohlmeyer
mmokhi: maintainer-approval+
Details | Diff
remove -DSYSCONFDIR from CMAKE_ARGS (1.12 KB, patch)
2016-05-17 20:33 UTC, Markus Kohlmeyer
no flags Details | Diff
merging two patches about my.cnf place (5.04 KB, patch)
2016-05-18 11:46 UTC, Mahdi Mokhtari
mmokhi: maintainer-approval+
Details | Diff
UPDATING entry (785 bytes, patch)
2016-05-18 13:20 UTC, Markus Kohlmeyer
mmokhi: maintainer-approval+
Details | Diff
merging two patches about my.cnf place (updated) (5.04 KB, patch)
2016-06-25 15:23 UTC, Mahdi Mokhtari
mmokhi: maintainer-approval+
Details | Diff
merging two patches about my.cnf place (updated+fixed pkg-plist issues) (5.59 KB, patch)
2016-07-04 00:44 UTC, Mahdi Mokhtari
mmokhi: maintainer-approval+
Details | Diff
Updated patch, UPDATING and messages (8.42 KB, patch)
2016-07-04 13:46 UTC, Thomas Zander
mmokhi: maintainer-approval+
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Mahdi Mokhtari freebsd_committer freebsd_triage 2016-05-17 14:00:31 UTC
Opened specific issue for checking/reviewing/deciding about issue in default path of my.cnf
Comment 1 Mahdi Mokhtari freebsd_committer freebsd_triage 2016-05-17 14:03:12 UTC
Things i guess matters for us are (but not closed to ;D) : 
1)respect hier(7)
2)not surprise users
3)respect mysql original decisions/updates
Comment 2 Markus Kohlmeyer 2016-05-17 14:22:48 UTC
Created attachment 170407 [details]
better inform users about new default location for my.cnf

Proposed patch for your point #2
Comment 3 Mahdi Mokhtari freebsd_committer freebsd_triage 2016-05-17 14:26:39 UTC
(In reply to Markus Kohlmeyer from comment #2)
my.cnf Path is wrong right now ? (i assumed when we was working on mysql upgrade to 5.7.12, we corrected things).

I thought that's why we warned ppl for future changes in pkg-message
Comment 4 Mahdi Mokhtari freebsd_committer freebsd_triage 2016-05-17 14:27:13 UTC
(In reply to Mahdi Mokhtari from comment #3)
I just asked this cause you changed mysql-server.in
Comment 5 Markus Kohlmeyer 2016-05-17 14:58:44 UTC
my.cnf path is and was correct, i just reworded the warning/hint to be more clear to users and added the missing UPDATING entry.
Comment 6 Markus Kohlmeyer 2016-05-17 15:10:56 UTC
Created attachment 170408 [details]
Backport of databases/mariadb101-server/files/patch-mysys_my__default.c

(In reply to Mahdi Mokhtari from comment #1)

Proposed patch for your point #1

Backport of databases/mariadb101-server/files/patch-mysys_my__default.c
Comment 7 Markus Kohlmeyer 2016-05-17 15:15:37 UTC
(In reply to Mahdi Mokhtari from comment #1)
Can you explain your point #3 a bit more please?
Comment 8 Markus Kohlmeyer 2016-05-17 15:20:29 UTC
(In reply to Markus Kohlmeyer from comment #6)

Forgot the new output for reference:


[root@devnoip:~] # /usr/local/libexec/mysqld --verbose --help | grep -A1 'Default options'
Default options are read from the following files in the given order:
/usr/local/etc/my.cnf /usr/local/etc/mysql/my.cnf ~/.my.cnf
Comment 9 Mahdi Mokhtari freebsd_committer freebsd_triage 2016-05-17 15:24:00 UTC
(In reply to Markus Kohlmeyer from comment #5)
Okay i'm okay with this. just few point before we have it approved ;)

Nothing practically changes (in Paths, ...), correct ?

Is it necessary to warn ppl every time service starts ?  (i'm talking about "if [ -f "/usr/local/etc/my.cnf" ]; then")

I think previous (current) wording of pkg-message is better. (as if we say path is gonna change they make themselves ready, but if we say "things [has been] changed" they will be surprised ;D)

What about just adding entry to UPDATING only ?


(In reply to Markus Kohlmeyer from comment #6)
Did you tested your back-port ?
i expected to have this for mysql57-server not client :thinking:


(In reply to Markus Kohlmeyer from comment #7)
point #3 is main reason of issue/PR, when mysql does a change as upstream, we plan to have their changes (updates) respecting FreeBSD env. (instead of ignoring their change and maintaining patches for ignoring them :D)
Comment 10 Markus Kohlmeyer 2016-05-17 15:34:45 UTC
We can stay with the previous wording or improve it even more, as you like ;)

For me UPDATING is always enough, but i know a few sysadmins that never read UPDATING (and pkg-message is often flying by unread, espacially when using tools like portupgrade/portmaster to manage and/or bulk update ports) so these will miss the info. Therefor rc-script seems to me like a legit option, even when annoying some users including myself. I'm open for other solutions and even for removing it from rc-script
Comment 11 Markus Kohlmeyer 2016-05-17 15:36:45 UTC
(In reply to Mahdi Mokhtari from comment #9)
Yes i did a build on 10.3-RELEASE amd64 and short runtime check, both OK on my sytem.
Comment 12 Markus Kohlmeyer 2016-05-17 15:37:46 UTC
> Nothing practically changes (in Paths, ...), correct ?

Correct.
Comment 13 Mahdi Mokhtari freebsd_committer freebsd_triage 2016-05-17 15:53:52 UTC
(In reply to Markus Kohlmeyer from comment #10)
> We can stay with the previous wording or improve it even more, as you like ;)
Thanks so much :D if decision is on me with wording (in metric of how much ppl being shocked), i definitely prefer previous wording, it's soft and kind word of wise advice ;D


> For me UPDATING is always enough, but i know a few sysadmins that never read UPDATING (and pkg-message is often flying by unread, espacially when using tools like portupgrade/portmaster to manage and/or bulk update ports) so these will miss the info. Therefor rc-script seems to me like a legit option, even when annoying some users including myself. I'm open for other solutions and even for removing it from rc-script
I definitely respect your experience as an expert sysadmin (and my preference is using it as much as possible).
If you met ppl who wont be aware of event by (not-)reading UPDATING and pkg-message. i personally think we should make them aware in any way we can (including service file). for annoying part of huge message, i guess we can keep message as short as possible with small message (using a wording like current pkg-message, in point of kind/soft advice of warning that in FUTURE thing will change)


> Yes i did a build on 10.3-RELEASE amd64 and short runtime check, both OK on my sytem.
Thanks for your confirming (and i count it as part of my QA on building), just let me do little linting QA too ;)
Comment 14 Mahdi Mokhtari freebsd_committer freebsd_triage 2016-05-17 16:41:09 UTC
(In reply to Markus Kohlmeyer from comment #13)
I think it's better we join this backported-patch with mysql57-server's existing one.
What do you think about it ?
Comment 15 Mahdi Mokhtari freebsd_committer freebsd_triage 2016-05-17 16:44:55 UTC
(In reply to Mahdi Mokhtari from comment #14)
never mind :D
ignore my previous comment ;)
Both client and server parts are needed ;)
Comment 16 Mahdi Mokhtari freebsd_committer freebsd_triage 2016-05-17 17:06:53 UTC
Comment on attachment 170408 [details]
Backport of databases/mariadb101-server/files/patch-mysys_my__default.c

Lint and Build* are seem to be Okay.
* on 10 and 11, still testing for 9 :D
Comment 17 Mahdi Mokhtari freebsd_committer freebsd_triage 2016-05-17 17:17:33 UTC
(In reply to Mahdi Mokhtari from comment #16)
built successfully done for FBSD9 too ;)
QA done successfully.

Attachment https://bugs.freebsd.org/bugzilla/attachment.cgi?id=170408 is ready to be committed in IMO ;)
Comment 18 Markus Kohlmeyer 2016-05-17 20:33:38 UTC
Created attachment 170417 [details]
remove -DSYSCONFDIR from CMAKE_ARGS


Remove -DSYSCONFDIR from CMAKE_ARGS to avoid possible conflicts with default paths for my.cnf and keyring file and maybe others too


Sorry, forgot it in previous patch
Comment 19 Mahdi Mokhtari freebsd_committer freebsd_triage 2016-05-18 10:52:10 UTC
(In reply to Markus Kohlmeyer from comment #18)
as i remember from removing -DSYSCONFDIR it causes using `/etc/*` yes?
Or with patch that we have backported it will fixed ?
I'm asking this cause of point#1 :D
Comment 20 Markus Kohlmeyer 2016-05-18 11:24:05 UTC
(In reply to Mahdi Mokhtari from comment #19)

> as i remember from removing -DSYSCONFDIR it causes using `/etc/*` yes?

No, but it allows again using /usr/local/etc/my.cnf as it was in previous versions and also is with MariaDB and Percona.

> Or with patch that we have backported it will fixed ?

The patch will remove /etc from the internal search patch for my.cnf and warn the user accordingly. Thats what makes us comply with hier(7) and is the same behavior MariaDB has for some time now.
Comment 21 Mahdi Mokhtari freebsd_committer freebsd_triage 2016-05-18 11:36:24 UTC
(In reply to Mahdi Mokhtari from comment #19)
Yeah i see ;) thanks for clarifying things ;D
Then let me merge these two patches in one patch-file (as aim is one)
Comment 22 Mahdi Mokhtari freebsd_committer freebsd_triage 2016-05-18 11:46:07 UTC
Created attachment 170435 [details]
merging two patches about my.cnf place

QA previously done Okay.
Patch is ready to commit.
Comment 23 Mahdi Mokhtari freebsd_committer freebsd_triage 2016-05-18 11:46:32 UTC
Thanks Markus ;)
Comment 24 Markus Kohlmeyer 2016-05-18 12:46:54 UTC
Please don't forget the UPDATING entry ;)
Comment 25 Mahdi Mokhtari freebsd_committer freebsd_triage 2016-05-18 12:59:33 UTC
(In reply to Mahdi Mokhtari from comment #23)
I was waiting for you to reword the necessary things :)
Comment 26 Markus Kohlmeyer 2016-05-18 13:17:40 UTC
Same wording as in pkg-message and rc-script

diff -Naur UPDATING UPDATING
--- UPDATING	2016-05-17 14:02:17.527385000 +0200
+++ UPDATING	2016-05-17 14:44:07.167788000 +0200
@@ -5,6 +5,15 @@
 You should get into the habit of checking this file for changes each time
 you update your ports collection, before attempting any port upgrades.
 
20160517:
  AFFECTS: users of databases/mysql57-(server|client)
  AUTHOR: rootservice@gmail.com

  Please keep in mind that the default location for my.cnf will be changed
  from "/var/db/mysql/my.cnf" to "/usr/local/etc/mysql/my.cnf" in the near
  future.  If you do not want to move your my.cnf to the new location then
  you must set "mysql_optfile" in /etc/rc.conf to "/var/db/mysql/my.cnf".

 20160511:
   AFFECTS: Users of audio/clementine-player
   AUTHOR: sbruno@FreeBSD.org
Comment 27 Markus Kohlmeyer 2016-05-18 13:20:53 UTC
Created attachment 170441 [details]
UPDATING entry

Ignore previous comment.

As regular patch now.
Comment 28 Mahdi Mokhtari freebsd_committer freebsd_triage 2016-05-18 15:18:49 UTC
Comment on attachment 170441 [details]
UPDATING entry

Thanks for rewording
Markus ;)
Comment 29 Mahdi Mokhtari freebsd_committer freebsd_triage 2016-05-18 15:19:46 UTC
(In reply to Mahdi Mokhtari from comment #28)
Ready to be committed in IMO :)
Comment 30 Mahdi Mokhtari freebsd_committer freebsd_triage 2016-05-28 17:15:02 UTC
DeAssigning from myself so that a committer comes and takes it faster :)
Comment 31 Thomas Zander freebsd_committer 2016-06-25 14:30:39 UTC
Is this up to date since mysql has been updated to new upstream versions?
Comment 32 Mahdi Mokhtari freebsd_committer freebsd_triage 2016-06-25 15:16:27 UTC
(In reply to Thomas Zander from comment #31)
Not all of them updated.
I guess i should regenerate patches that are working with Makefile.
UPDATING entry patch can be committed though (doesn't need to be regenerated IMO :D)
Comment 33 Mahdi Mokhtari freebsd_committer freebsd_triage 2016-06-25 15:23:46 UTC
Created attachment 171791 [details]
merging two patches about my.cnf place (updated)

Updated this patch for solving conflicts about PORTREVISION.
**it was small conflict caused by updating port to 5.7.13 and resetting PORTREVISION to 0
Comment 34 Mahdi Mokhtari freebsd_committer freebsd_triage 2016-06-25 15:24:58 UTC
(In reply to Thomas Zander from comment #31)
@Thomas Zander
Done :)
Both patches are now updated and ready to commit in ;)
Comment 35 Thomas Zander freebsd_committer 2016-06-25 18:38:33 UTC
I have some (minor) questions:
1) The UPDATING entry is still correct? So the port does still follow the old behaviour by default for now?
2) The patch adds hardcoded "/usr/local" strings at some points. This is probably not an issue for package building, but what about the users who operate systems with a custom prefix? The port is actually quite close to work with custom prefix as it seems from the build log :)
Comment 36 Thomas Zander freebsd_committer 2016-06-25 19:03:23 UTC
Also, did you see this one in poudriere testport:
====> Running Q/A tests (stage-qa)
====> Checking for pkg-plist issues (check-plist)
===> Parsing plist
===> Checking for items in STAGEDIR missing from pkg-plist
Error: Orphaned: lib/mysql/plugin/keyring_udf.so
Error: Orphaned: lib/mysql/plugin/test_udf_services.so
===> Checking for items in pkg-plist which are not in STAGEDIR
===> Error: Plist issues found.
*** Error code 1

Stop.
make: stopped in /usr/ports/databases/mysql57-server
====>> Error: check-plist failures detected
Comment 37 Markus Kohlmeyer 2016-06-25 19:34:51 UTC
(In reply to Thomas Zander from comment #35)

The wording of the proposed UPDATING is not 100% correct, a more correct wording would be something like this:


Please keep in mind that the default location for my.cnf has changed from "/var/db/mysql/my.cnf" to "/usr/local/etc/mysql/my.cnf". Please merge your existing my.cnf with the new default one and move it to the new location. If you do not want to merge and move your existing my.cnf then you must set "mysql_optfile" in /etc/rc.conf to the location of your own my.cnf file.
Comment 38 Thomas Zander freebsd_committer 2016-07-03 12:25:55 UTC
(In reply to Mahdi Mokhtari from comment #34)

Mokhi, are you going to address the plist issues from comment #36 ?
We should get this into the tree soonish, as it started blocking other PRs.
Thank you in advance.
Comment 39 Mahdi Mokhtari freebsd_committer freebsd_triage 2016-07-04 00:11:21 UTC
(In reply to Thomas Zander from comment #38)
Yes :)
It's fixed in my local tree.
I'm doing QA and update/regenerate patches today.
Sorry if it's lasted more than expected ;)
Comment 40 Mahdi Mokhtari freebsd_committer freebsd_triage 2016-07-04 00:44:24 UTC
Created attachment 172081 [details]
merging two patches about my.cnf place (updated+fixed pkg-plist issues)

Fixed pkg-list issue + portlint/poudriere check.
I guess it's ready now :D
Comment 41 Thomas Zander freebsd_committer 2016-07-04 06:18:11 UTC
(In reply to Mahdi Mokhtari from comment #40)

OK, thanks for the latest patch set. USE_OPENSSL is deprecated now, but that can also be fixed in a future update.

However, we have to ensure the UPDATING entry is right. Based on Markus's proposal, I would add this to UPDATING:

20160704:
  AFFECTS: users of databases/mysql57-*
  AUTHOR: riggs@FreeBSD.org

  The default location for my.cnf has changed from "/var/db/mysql/my.cnf"
  to "/usr/local/etc/mysql/my.cnf". Existing my.cnf files must be merged
  manually with the new default and move it to the new location. To
  continue using the my.cnf file at the old location, set "mysql_optfile"
  in /etc/rc.conf to point to the location of the existing my.cnf file.

Let me know if this is OK. I have to emphasise the wording "has changed", because 
mysql57-server/pkg-message stills says "will be changed in the near future". So either UPDATING or pkg-message would be wrong.
Comment 42 Markus Kohlmeyer 2016-07-04 09:09:01 UTC
(In reply to Thomas Zander from comment #41)

Your UPDATING entry is OK and correct.


mysql57-server/pkg-message and mysql57-server/files/mysql-server.in have to be corrected to be in par with UPDATING, but this can IMO also wait till next update.
Comment 43 Mahdi Mokhtari freebsd_committer freebsd_triage 2016-07-04 10:52:14 UTC
(In reply to Thomas Zander from comment #41)
(In reply to Markus Kohlmeyer from comment #42)
Hi.
It's Okay in my view :)
Thanks all guys :D, good job ;)
Comment 44 Thomas Zander freebsd_committer 2016-07-04 13:46:22 UTC
Created attachment 172100 [details]
Updated patch, UPDATING and messages

Call me picky, but especially when introducing breaking changes I like to have to documentation in line with the actual behaviour.

Here is my proposal for the patch including UPDATING and the files containing messages in relation to this change.
Comment 45 Mahdi Mokhtari freebsd_committer freebsd_triage 2016-07-04 13:54:52 UTC
(In reply to Thomas Zander from comment #44)
About UPDATING entry, i agree it has better wording now :)
Did you changed Makefile or other patches behavior too ? or just UPDATING updated ?
Should i do QA (or at least poudriere) again ?
Comment 46 Markus Kohlmeyer 2016-07-04 14:02:06 UTC
(In reply to Thomas Zander from comment #44)
I'm fine with your patch and the wording, thanks.

(In reply to Mahdi Mokhtari from comment #45)
He only changed the existing wording in mysql57-server/pkg-message and mysql57-server/files/mysql-server.in additional to your last patch.
Should pass QA without problems.
Comment 47 Mahdi Mokhtari freebsd_committer freebsd_triage 2016-07-04 14:03:53 UTC
(In reply to Markus Kohlmeyer from comment #46)
Okay thanks for clarifying things for me :)
I'm gonna approve+ it then.
Comment 48 commit-hook freebsd_committer 2016-07-04 14:59:58 UTC
A commit references this bug:

Author: riggs
Date: Mon Jul  4 14:59:16 UTC 2016
New revision: 418033
URL: https://svnweb.freebsd.org/changeset/ports/418033

Log:
  Make my.cnf path correct according to hier(7), bump PORTREVISION

  PR:		209579
  Submitted by:	mokhi64@gmail.com
  Reviewed by:	rootservice@gmail.com

Changes:
  head/databases/mysql57-client/Makefile
  head/databases/mysql57-client/files/patch-mysys__ssl_my__default.cc
  head/databases/mysql57-server/Makefile
  head/databases/mysql57-server/files/mysql-server.in
  head/databases/mysql57-server/files/patch-mysys__ssl_my__default.cc
  head/databases/mysql57-server/pkg-message
  head/databases/mysql57-server/pkg-plist
Comment 49 commit-hook freebsd_committer 2016-07-04 15:00:59 UTC
A commit references this bug:

Author: riggs
Date: Mon Jul  4 15:00:44 UTC 2016
New revision: 418034
URL: https://svnweb.freebsd.org/changeset/ports/418034

Log:
  Document change of default location for my.cnf for databases/mysql57-*

  PR:		209579
  Submitted by:	mokhi64@gmail.com
  Reviewed by:	rootservice@gmail.com

Changes:
  head/UPDATING
Comment 50 Mahdi Mokhtari freebsd_committer freebsd_triage 2016-07-04 15:22:43 UTC
(In reply to commit-hook from comment #49)
Thanks ;)
Good job all :)