Created attachment 169106 [details] Fix r412723 This fixes a regression in r412723 as the manfiles are listed in Makefiles
(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+
Builds OK on 10.3 Brings only back previuos behavior, so nothing to worry about.
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.
Does this fix the build, or does it change the content of the package ?
(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.` :)
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.
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.
> 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 ;)
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.
Created attachment 169454 [details] proposed reworked sed for man/CMakeLists.txt This will avoid false positives if sed is run twice.
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.
(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 ?
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.
(In reply to Markus Kohlmeyer from comment #13) Thanks :) I agree ;) So, which patches i should obsolete ? 2 last ones ?
Yes, last two.
Comment on attachment 169454 [details] proposed reworked sed for man/CMakeLists.txt Obsoleted to avoid changing path of issue (avoid making spaghetti issue).
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).
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
Thanks :)