Details
Major Description
When building with -DWITH_WSREP=OFF -DPLUGIN_WSREP_INFO=NO, I am still finding many wsrep-related files installed in the INSTALLDIR. Specifically:
usr/include/mysql/server/mysql/service_wsrep.h
|
usr/include/mysql/server/private/wsrep*.h
|
usr/share/mysql/wsrep_notify
|
usr/share/doc/README-wsrep
|
usr/share/man/man1/wsrep_sst_rsync_wan.1
|
usr/share/man/man1/wsrep_sst_mariabackup.1
|
usr/share/man/man1/wsrep_sst_mysqldump.1
|
usr/share/man/man1/wsrep_sst_rsync.1
|
usr/share/man/man1/wsrep_sst_common.1
|
Whether or not the includes files should be installed I don't know, and the SQL in wsrep_notify and the README-wsrep could also be argued. But, certainly the man pages should not be installed as the tools are not built or installed.
I would suggest that none of these should be installed, and that the build configurtion should turn them all off if -DWITH_WSREP=OFF.
Attachments
Issue Links
- causes
-
MDEV-30344 MTR tests fail when built without WSREP
-
- Closed
-
Activity
| Field | Original Value | New Value |
|---|---|---|
| Attachment | mariadb_MDEV-23230.patch [ 53017 ] |
| Affects Version/s | 10.5.5 [ 24423 ] |
| Fix Version/s | 10.4 [ 22408 ] | |
| Fix Version/s | 10.5 [ 23123 ] |
| Assignee | Julius Goryavsky [ sysprg ] |
This issue still affects 10.5.10, and is causing undesired files to be installed without us manually patching this. could you please integrate the supplied change or one like it?
Thank you.
| Affects Version/s | 10.5.8 [ 25023 ] |
| Workflow | MariaDB v3 [ 111423 ] | MariaDB v4 [ 142114 ] |
This is still affecting 10.5.13 and 10.5.16. We're continuing to patch our local build process to avoid these unnecessary files being installed on our product.
Can this be corrected in 10.5.x? Can you also please report if it is also present in 10.6.x or 10.7.x? If not, then I can accept working around it until a later release, but I worry it may be ongoing.
cross, your on the right track.
Small style bits to group just the WSREP component different:
|
yours |
- INSTALL_DOCUMENTATION(Docs/INSTALL-BINARY Docs/README-wsrep COMPONENT Readme)
|
+ IF(WITH_WSREP)
|
+ INSTALL_DOCUMENTATION(Docs/INSTALL-BINARY Docs/README-wsrep COMPONENT Readme)
|
+ ELSE()
|
+ INSTALL_DOCUMENTATION(Docs/INSTALL-BINARY COMPONENT Readme)
|
+ ENDIF()
|
|
suggested |
- INSTALL_DOCUMENTATION(Docs/INSTALL-BINARY Docs/README-wsrep COMPONENT Readme)
|
+ IF(WITH_WSREP)
|
+ INSTALL_DOCUMENTATION(Docs/README-wsrep COMPONENT Readme)
|
+ ENDIF()
|
+ INSTALL_DOCUMENTATION(Docs/INSTALL-BINARY COMPONENT Readme)
|
10.6 starts to group some WSREP components under variables so taking some of those changes limited to WSREP, back porting those to 10.5, and apply your changes and that would make the merge up a little easier.
Patch/pull request welcome.
If attaching patch here can you indicate the licence choice: BSD New 3 Clause License, or under the MariaDB Contributor Agreement.
| Assignee | Julius Goryavsky [ sysprg ] | Daniel Black [ danblack ] |
| Attachment | mariadb_MDEV-23230-1.patch [ 66882 ] |
I took your suggested structure, and the only place a saw a change for variables was in man/CMakeLists.txt, and I pulled that in. Also I noted that in 10.6 there is still a wsrep man page outside of that variable. Should wsrep_sst_mysqldump.1 be left out of MAN1_WSREP? I presume it was an oversight and have corrected in my patch.
Let me know how this looks. As to license, Mariadb Contributor Agreement is fine. I need no ownership of these changes.
mariadb_MDEV-23230.patch
| Affects Version/s | 10.5.16 [ 27508 ] |
Thanks Chris, good corrections.
Also excluded include/*/wsrep.h too and galera system files.
https://github.com/MariaDB/server/pull/2334 for final review.
| Fix Version/s | 10.4 [ 22408 ] |
| Status | Open [ 1 ] | In Progress [ 3 ] |
| Assignee | Daniel Black [ danblack ] | Andrew Hutchings [ JIRAUSER52179 ] |
| Status | In Progress [ 3 ] | In Review [ 10002 ] |
Thank you. I didn't catch that galera should be excluded too, thanks. And re STRING(APPEND, I knew about the "only works in recent cmake" but thought that the large number of other STRING() uses meant it was okay. Now I know better. 
Looks great!
It was deceptive, all the STRING(APPEND was under Windows which has a higher CMake version threshold. We've still got cmake-ancient in rhel/cento-7.
Glad your happy with it. Sorry it too so long. And thanks for the ping on maria-discuss.
| Affects Version/s | 10.6.11 [ 28441 ] |
Just confirmed that these files (or a very similar set) are also installed on 10.6.11 compiled with -DWITH_WSREP=OFF -DPLUGIN_WSREP_INFO=NO. Thanks. Looking forward to the fix getting into the trees.
| Status | In Review [ 10002 ] | Stalled [ 10000 ] |
| Fix Version/s | 10.5.20 [ 28512 ] | |
| Fix Version/s | 10.5 [ 23123 ] | |
| Resolution | Fixed [ 1 ] | |
| Status | Stalled [ 10000 ] | Closed [ 6 ] |
Thanks. I see this got pulled into 10.5 today, but it is noted as fixed in 10.5.20, not 10.5.19 which should be the next out. So curious for a "why" on that, and a pointer to track this getting up into 10.6 (and later) so I can plan for which 10.6 release will have it. I hope/expect to have our internal product update to 10.6.x shortly. Thank you.
| Fix Version/s | 10.5.19 [ 28511 ] | |
| Fix Version/s | 10.5.20 [ 28512 ] |
| Fix Version/s | 10.6.12 [ 28513 ] | |
| Fix Version/s | 10.7.8 [ 28515 ] | |
| Fix Version/s | 10.8.7 [ 28517 ] | |
| Fix Version/s | 10.9.5 [ 28519 ] | |
| Fix Version/s | 10.10.3 [ 28521 ] | |
| Fix Version/s | 10.11.2 [ 28523 ] |
The changes introduce failures in MTR tests when WITH_WSREP=OFF. See MDEV-30344.
| Link |
This issue causes |
The following patch will prevent installation of the man pages and support/doc files. Removing the headers would be a little more complicated, and I'm not 100% sure those aren't something some users might want even when building without wsrep. Insight would be appreciated, I can figure out how to prevent those as well.
This is a quick patch, style may not be what you want, but it accomplishes my goal.