Bug 208634

Summary: databases/mysql57-{server,client}: Fix regression in r412723
Product: Ports & Packages Reporter: Markus Kohlmeyer <rootservice>
Component: Individual Port(s)Assignee: Thomas Zander <riggs>
Status: Closed FIXED    
Severity: Affects Many People CC: mmokhi, riggs
Priority: --- Flags: mmokhi: maintainer-feedback+
Version: Latest   
Hardware: Any   
OS: Any   
Bug Depends on:    
Bug Blocks: 206998    
Attachments:
Description Flags
Fix r412723
mmokhi: maintainer-approval+
proposed reworked sed for man/CMakeLists.txt
none
alternativ to both previous patches, removes post-patch macros none

Description Markus Kohlmeyer 2016-04-08 14:16:49 UTC
Created attachment 169106 [details]
Fix r412723

This fixes a regression in r412723 as the manfiles are listed in Makefiles
Comment 1 Mahdi Mokhtari freebsd_committer freebsd_triage 2016-04-12 03:59:59 UTC
(In reply to Markus Kohlmeyer from comment #0)

Hi, and thanks for your helps In mysql ports.
Patch seems okay for me  :)

Building tested? You confirm/verify? Or I should do test it before approve+
Comment 2 Markus Kohlmeyer 2016-04-12 09:07:55 UTC
Builds OK on 10.3

Brings only back previuos behavior, so nothing to worry about.
Comment 3 Mahdi Mokhtari freebsd_committer freebsd_triage 2016-04-12 11:51:53 UTC
Comment on attachment 169106 [details]
Fix r412723

(In reply to Markus Kohlmeyer from comment #2)
> Brings only back previuos behavior, so nothing to worry about.
Then Okay :D
Approved+ ;)
Waiting for a committer to take it !
Thanks Markus.
Comment 4 Mathieu Arnold freebsd_committer 2016-04-18 18:43:05 UTC
Does this fix the build, or does it change the content of the package ?
Comment 5 Mahdi Mokhtari freebsd_committer freebsd_triage 2016-04-18 18:50:01 UTC
(In reply to Mathieu Arnold from comment #4)
This patch, shortens the amount of patch.
In brief words, instead of doing FILE(GLOB MAN1_FILES $MANY_THINGS) it shortens it to FILE(GLOB MAN1_FILES *.1).
So you can call it `it changes nothing functional, only cosmetic.`
:)
Comment 6 Markus Kohlmeyer 2016-04-18 19:18:36 UTC
This patch lets 

post-patch:
        @${REINPLACE_CMD} 's/*.1/${MMAN1}/' ${WRKSRC}/man/CMakeLists.txt

do its work (as it was before) and makes the patched in hardcoded list of manfiles obsolet and the patches to man/CMakeFiles.txt smaller.

Thus this is only cosmetic and does not need a REVbump. Can be commited without extra QA as it does not effect build, install or runtime.

See mysql57-client/Makefile and mysql57-server/Makefile for reference.
Comment 7 Markus Kohlmeyer 2016-04-18 19:26:22 UTC
This slips in everytime one does someting like:

make -C /usr/ports/databases/mysql57-server patch
make -C /usr/ports/databases/mysql57-server makepatch

So it has to be reverted everytime manually again.
Comment 8 Mahdi Mokhtari freebsd_committer freebsd_triage 2016-04-18 19:32:48 UTC
> Thus this is only cosmetic and does not need a REVbump. Can be commited without extra QA as it does not effect build, install or runtime.
I've done its QA, though ;)
Comment 9 Markus Kohlmeyer 2016-04-18 19:40:55 UTC
To avoid false positives we could do a better sed, so instead of

@${REINPLACE_CMD} 's/*.1/${MMAN1}/' ${WRKSRC}/man/CMakeLists.txt

we should use something like

@${REINPLACE_CMD} 's/\(GLOB\ MAN1_FILES\)\ \*\.1/\1 ${MMAN1}/' ${WRKSRC}/man/CMakeLists.txt

in the future. I can prepare a patch for this if necessary.


Alternativly we can omit the post-patch macros and hardcode the lists of manfiles, but that may be harder to maintain as it might be overseen on future updates.
Comment 10 Markus Kohlmeyer 2016-04-18 20:11:58 UTC
Created attachment 169454 [details]
proposed reworked sed for man/CMakeLists.txt

This will avoid false positives if sed is run twice.
Comment 11 Markus Kohlmeyer 2016-04-18 20:18:34 UTC
Created attachment 169456 [details]
alternativ to both previous patches, removes post-patch macros

This will remove the post-patch macros from the Makefiles and leaves the lists of manfiles hardcoded in the mysql57-*/files/patch-man_CMakeLists.txt.


The other two patches will be obsolet if this one is choosen. Applies clealy after commit of 206998.
Comment 12 Mahdi Mokhtari freebsd_committer freebsd_triage 2016-04-20 10:58:04 UTC
(In reply to Markus Kohlmeyer from comment #11)
I think it's better we don't make a spaghetti issue :) [how ever, maybe the other patches are better in some cases].
I myself think as long as the patch doesn't change anything on the user view (nor fixing critical issue), let's make it go forward as planned, and do optimizing things in separate issue. (this also reduces our work in generating new patches against always changing HEAD :D)

What's your opinion, guys ?
Comment 13 Markus Kohlmeyer 2016-04-20 11:33:18 UTC
Makes sense, so lets commit the first patch, close this bug and open a new one for the rest after 5.7.12 is in tree.
Comment 14 Mahdi Mokhtari freebsd_committer freebsd_triage 2016-04-20 12:13:23 UTC
(In reply to Markus Kohlmeyer from comment #13)
Thanks :)
I agree ;)
So, which patches i should obsolete ? 2 last ones ?
Comment 15 Markus Kohlmeyer 2016-04-20 12:33:36 UTC
Yes, last two.
Comment 16 Mahdi Mokhtari freebsd_committer freebsd_triage 2016-04-20 12:54:54 UTC
Comment on attachment 169454 [details]
proposed reworked sed for man/CMakeLists.txt

Obsoleted to avoid changing path of issue (avoid making spaghetti issue).
Comment 17 Mahdi Mokhtari freebsd_committer freebsd_triage 2016-04-20 12:55:00 UTC
Comment on attachment 169456 [details]
alternativ to both previous patches, removes post-patch macros

Obsoleted to avoid changing path of issue (avoid making spaghetti issue).
Comment 18 commit-hook freebsd_committer 2016-04-23 07:16:27 UTC
A commit references this bug:

Author: riggs
Date: Sat Apr 23 07:15:57 UTC 2016
New revision: 413856
URL: https://svnweb.freebsd.org/changeset/ports/413856

Log:
  Cosmetic change of handling manual pages; preparation for upcoming update

  PR:		208634
  Submitted by:	rootservice@gmail.com
  Approved by:	mokhi64@gmail.com (maintainer)

Changes:
  head/databases/mysql57-client/files/patch-man_CMakeLists.txt
  head/databases/mysql57-server/files/patch-man_CMakeLists.txt
Comment 19 Mahdi Mokhtari freebsd_committer freebsd_triage 2016-04-23 08:25:36 UTC
Thanks :)