Bug 206805 - databases/mysql57-client: Symlink mysqlclient to mysqlclient_r library (fixes consumer port linking)
Summary: databases/mysql57-client: Symlink mysqlclient to mysqlclient_r library (fixes...
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: Steven Hartland
URL:
Keywords: easy, patch, patch-ready
Depends on:
Blocks: 206998
  Show dependency treegraph
 
Reported: 2016-01-31 21:31 UTC by Steven Hartland
Modified: 2016-04-16 13:26 UTC (History)
3 users (show)

See Also:
mmokhi: maintainer-feedback+
junovitch: merge-quarterly-


Attachments
Add symlinks for lib_r's (761 bytes, patch)
2016-01-31 21:50 UTC, Steven Hartland
mmokhi: maintainer-approval+
koobs: maintainer-approval+
Details | Diff
patch for supporting lib_r (1.16 KB, patch)
2016-02-14 13:12 UTC, Mahdi Mokhtari
mmokhi: maintainer-approval+
Details | Diff
patch for supporting lib_r (1.17 KB, patch)
2016-02-14 16:38 UTC, Steven Hartland
no flags Details | Diff
patch for this issue solves QA issue (and bumps PORTREVISION) (1.29 KB, patch)
2016-02-14 17:06 UTC, Mahdi Mokhtari
mmokhi: maintainer-approval+
Details | Diff
patch for supporting lib_r (1.16 KB, patch)
2016-02-14 17:06 UTC, Steven Hartland
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Steven Hartland freebsd_committer 2016-01-31 21:31:38 UTC
mysql 5.7 changes lib_r from mysqlclient_r to mysqlclient which causes badly created packages to fail due to missing -lmysqlclient_r

One such port is qt4-mysql-plugin but I expect there are more.

This could be fixed with cleaver workarounds in each port or upstream changes so they use mysql_config to determine correct link settings, but that's quite invasive and time consuming, so it may be better to patch mysql57-client to create a symlink between libmysqlclient_r.so and libmysqlclient.so.
Comment 1 Steven Hartland freebsd_committer 2016-01-31 21:50:26 UTC
Created attachment 166367 [details]
Add symlinks for lib_r's
Comment 2 Mahdi Mokhtari freebsd_committer freebsd_triage 2016-02-01 10:49:05 UTC
(In reply to Steven Hartland from comment #1)
Hi.
Thanks for your feedback on mysql57 ports :)
from mysql-devs-guys point of view 'qt4-mysql-plugin' is using mysql57-lib-devel.
so it should use mysql_config --cflags to build itself (and there shouldn't be problem then :)

In another issue (issue#206119) i had a discussion with someone and i've asked mysql-dev-guys' opinion in such cases.

Maybe i can talk with qt-dev-guys and convince them yo change their way :) because in this case MySQL is considered their upstream :D

What's your opinion ?
i personally think quality is more important than time :D, so we have to use better way ignoring its time :D
Comment 3 Steven Hartland freebsd_committer 2016-02-01 11:01:29 UTC
It doesn't and I'm assuming others don't if you see the percona port they had the same issue and did the same as I've done here, as the impact to get all the upstreams to fix their build is too high.
Comment 4 Kubilay Kocak freebsd_committer freebsd_triage 2016-02-01 11:20:41 UTC
+1 on a temporary symlink until more permanent upstream changes can be made.

Is there any reason MySQL can't create these symlinks for compatibility, or at least create an option at build time to do so?

@Steven, could you confirm this change passes QA (poudriere) please. I'm assuming its been build tested against consumer ports, and they find the library once the links are in place.

Also, would you like to commit this change? I'm happy (approve) it to go in, once QA is confirmed here and if Mahdi approves. If so, please assign this issue to yourself :)

@Mahdi, can you approve the attachment once Steven provides QA confirmation please?

You can create a separate issue for yourself to "Get upstreams to fix/provide compatibility for new library name" to track the permanent fix
Comment 5 Mahdi Mokhtari freebsd_committer freebsd_triage 2016-02-01 11:38:30 UTC
@Kubilay, @Steven: sure ill approve if something like this happened before :D
@Steven, could you confirm this change passes QA (poudriere) please ?
also ill try to contact Qt guys to make'em aware of it :)
so comment on added-lines is good idea IMO :)
Also as ive submitted two other patches solving 2-3 problems for mysql ports
please commit it after those, to avoid conflicts (and make my work easier ;D)

Thanks.
Comment 6 Mahdi Mokhtari freebsd_committer freebsd_triage 2016-02-01 11:38:59 UTC
(In reply to Mahdi Mokhtari from comment #5)
Also i think patch should be generated against head :D
Comment 7 Steven Hartland freebsd_committer 2016-02-01 20:36:33 UTC
Confirmed it passed testport and I then could successfully build qt4-mysql-plugin including again testport.
Comment 8 Mahdi Mokhtari freebsd_committer freebsd_triage 2016-02-02 04:05:48 UTC
(In reply to Steven Hartland from comment #7)

 > Confirmed it passed testport

Then okay from my side, except I think patch should be generated against HEAD, no?

Just another question (Only for my knowledge :D), couldn't we put these on post-stage target instead of post-install? And/Or put this link's name inside pkg-plist too? (I know post-install is correct as I saw many uses of it by googling. i'm just, "curious", about post-stage :D)

BTW, I'll generate this (currently submitted) patch against HEAD ;), thanks.
Comment 9 Steven Hartland freebsd_committer 2016-02-02 08:55:44 UTC
It was created from head ;-)
Comment 10 Mahdi Mokhtari freebsd_committer freebsd_triage 2016-02-02 10:08:06 UTC
(In reply to Steven Hartland from comment #9)
I mean also from ports/ dir.
But seems it created with 'diff' inside 'mysql57-client/' dir.
 :)
Comment 11 Mahdi Mokhtari freebsd_committer freebsd_triage 2016-02-02 11:23:56 UTC
Comment on attachment 166367 [details]
Add symlinks for lib_r's

I approved it '+' :D
Thanks for your helps :)
Comment 12 Kubilay Kocak freebsd_committer freebsd_triage 2016-02-02 11:30:06 UTC
Comment on attachment 166367 [details]
Add symlinks for lib_r's

port committer approval
Comment 13 Kubilay Kocak freebsd_committer freebsd_triage 2016-02-02 11:32:35 UTC
Over to you Steven

This patch is an MFH candidate, please include MFH: 2016Q1 in your commit log message including (along with a PR: 206805 reference), to obtain ports-secteam approval for the merge to quarterly. You may merge using PORTSDIR/Tools/scripts/mfh
Comment 14 Steven Hartland freebsd_committer 2016-02-02 11:38:20 UTC
Sorry koobs, I only have src commit bit not ports :(
Comment 15 Mahdi Mokhtari freebsd_committer freebsd_triage 2016-02-02 20:38:07 UTC
Comment on attachment 166367 [details]
Add symlinks for lib_r's

>--- Makefile.pre-lib_r	2016-01-31 21:34:05.019245752 +0000
>+++ Makefile	2016-01-31 21:44:40.761202143 +0000
>@@ -3,7 +3,7 @@
> 
> PORTNAME=	mysql
> PKGNAMESUFFIX=	57-client
>-PORTREVISION=	3
>+PORTREVISION=	4
> 
> COMMENT=	Multithreaded SQL database (client)
> 
>@@ -32,4 +32,11 @@ MMAN1=		comp_err.1 msql2mysql.1 mysql.1 
> 
> CLIENT_ONLY=	yes
> 
>+# MySQL 5.7 uses the same lib for libs as for libs_r, so for now we just create a symlink
>+# to prevent breaking installed ports.
>+post-install:
>+	@${LN} -s libmysqlclient.a ${STAGEDIR}${PREFIX}/lib/mysql/libmysqlclient_r.a
>+	@${LN} -s libmysqlclient.so ${STAGEDIR}${PREFIX}/lib/mysql/libmysqlclient_r.so
>+	@${LN} -s libmysqlclient.so.20 ${STAGEDIR}${PREFIX}/lib/mysql/libmysqlclient_r.so.20
>+
> .include "${MASTERDIR}/Makefile"
Comment 16 Mahdi Mokhtari freebsd_committer freebsd_triage 2016-02-02 20:39:53 UTC
(In reply to Mahdi Mokhtari from comment #15)
I think using @${CMD} is better than ${CMD}.
So i've edited patch.
** previous one was correct too :D
*** it's choosing between better and MOAR better

Thanks.
Comment 17 Kubilay Kocak freebsd_committer freebsd_triage 2016-02-14 12:18:37 UTC
(In reply to Steven Hartland from comment #14)

I've provided you with approval to commit this change to ports :)

Please add 

Approved by: koobs (ports)

to the commit log along with the other references provided in my comment 13
Comment 18 Kubilay Kocak freebsd_committer freebsd_triage 2016-02-14 12:20:00 UTC
Mahdi will be providing an updated patch (and QA confirmation, regenerated against the port once it's been updated when bug 206998 is committed.
Comment 19 Kubilay Kocak freebsd_committer freebsd_triage 2016-02-14 12:21:54 UTC
Actually, ignore that. The version update (in bug 206998) needs to come last.
Comment 20 Mahdi Mokhtari freebsd_committer freebsd_triage 2016-02-14 13:12:49 UTC
Created attachment 166982 [details]
patch for supporting lib_r

regenerated for being committed before other mysql issues.

Up to you @Steven :D
Comment 21 Mahdi Mokhtari freebsd_committer freebsd_triage 2016-02-14 13:16:24 UTC
(In reply to Mahdi Mokhtari from comment #20)
Passes QA
Portlint : okay
poudriere okay (93.0 10.1 11.0)
Comment 22 Steven Hartland freebsd_committer 2016-02-14 16:38:55 UTC
Created attachment 166988 [details]
patch for supporting lib_r

Previous patch + PORTREVISION bump

@mokhi64 if you can just confirm you're happy with this version I'll commit under koobs approval
Comment 23 Mahdi Mokhtari freebsd_committer freebsd_triage 2016-02-14 16:54:38 UTC
(In reply to Steven Hartland from comment #22)
is it necessary for using PORTREVISION for a slave port (mysql57-client) ?
Comment 24 Mahdi Mokhtari freebsd_committer freebsd_triage 2016-02-14 16:57:19 UTC
in server port (master port) we have PORTREVISION defined in a way to be shared between Master and slave ports
Comment 25 Steven Hartland freebsd_committer 2016-02-14 16:58:53 UTC
If you don't the change will not picked up by pkg upgrade mysql57-client as the full version will be the same, so yes if you make any change which doesn't change port version you need to bump the port revision; at least that's my understanding of it.
Comment 26 Mahdi Mokhtari freebsd_committer freebsd_triage 2016-02-14 17:01:00 UTC
(In reply to Steven Hartland from comment #25)
So what about
PORTREVISION?=	4
instead of
PORTREVISION=	4
?

also portlint QA failed, i think we have extra newlines in our patch :D
Comment 27 Steven Hartland freebsd_committer 2016-02-14 17:02:17 UTC
(In reply to Mahdi Mokhtari from comment #24)
Yes but this change doesn't effect server only client hence its the client which needs the bump.

This is why PORTREVISON in the parent is set with =? so it can be overridden by the client.

Its a little messy if the parent then gets bumped twice as it then needs both ports to be edited, I don't believe theirs any way to avoid that.
Comment 28 Mahdi Mokhtari freebsd_committer freebsd_triage 2016-02-14 17:06:48 UTC
Created attachment 166991 [details]
patch for this issue solves QA issue (and bumps PORTREVISION)

2 fatal portlint errors fixed

FATAL: Makefile: order must be 'BLAH BLAH'
FATAL: extra new lines
Comment 29 Steven Hartland freebsd_committer 2016-02-14 17:06:50 UTC
Created attachment 166992 [details]
patch for supporting lib_r

Removed extra blank lines introduced by patch -2
Comment 30 Steven Hartland freebsd_committer 2016-02-14 17:07:31 UTC
(In reply to Mahdi Mokhtari from comment #28)
Yer looks like you introduced those in your edit of my original patch, I've just uploaded a fixed version.
Comment 31 Mahdi Mokhtari freebsd_committer freebsd_triage 2016-02-14 17:08:15 UTC
(In reply to Steven Hartland from comment #30)
oh both of us attached in same time :)
Comment 32 Mahdi Mokhtari freebsd_committer freebsd_triage 2016-02-14 17:11:03 UTC
(In reply to Steven Hartland from comment #30)
PORTREVISION Should be before PKGNAMESUFFIX.
portlint errors in fatal way.
i solved it in patch :D
Comment 33 Mahdi Mokhtari freebsd_committer freebsd_triage 2016-02-14 17:17:27 UTC
@Steven: are you okay with this ?
:)
Comment 34 commit-hook freebsd_committer 2016-02-14 18:28:50 UTC
A commit references this bug:

Author: smh
Date: Sun Feb 14 18:28:35 UTC 2016
New revision: 408878
URL: https://svnweb.freebsd.org/changeset/ports/408878

Log:
  Add MySQL 5.7 symlinks for mysqlclient_r libs

  MySQL 5.7 changes lib_r from mysqlclient_r to mysqlclient which causes
  packages which don't correctly use mysql_config to determine library
  locations to fail due to missing -lmysqlclient_r.

  As there are quite a few ports, most of which will require upstream fixes
  for now we create symlinks so said ports build without issue.

  PR:		206805
  Reviewed by:	Mahdi Mokhtari (maintainer)
  Approved by:	koobs (ports)
  MFH:		2016Q1
  Sponsored by:	Multiplay

Changes:
  head/databases/mysql57-client/Makefile
  head/databases/mysql57-client/pkg-plist
Comment 35 Mahdi Mokhtari freebsd_committer freebsd_triage 2016-02-14 18:34:09 UTC
@Steven:
Thanks mate :D
Comment 36 Mahdi Mokhtari freebsd_committer freebsd_triage 2016-04-15 12:06:32 UTC
(In reply to Mahdi Mokhtari from comment #35)
Any progress on MFH ing ?
Can we close this issue?
Comment 37 Jason Unovitch freebsd_committer 2016-04-16 12:42:59 UTC
(In reply to Mahdi Mokhtari from comment #36)
Closing.  The MFH at this point is irrelevant as the referenced commit was already in the current 2016Q2 branch.
Comment 38 Mahdi Mokhtari freebsd_committer freebsd_triage 2016-04-16 13:24:23 UTC
(In reply to Jason Unovitch from comment #37)
Thanks ;)