1 |
>>>>> On Mon, 18 Apr 2011, Jorge Manuel B S Vicetto wrote: |
2 |
|
3 |
> The mysql team now uses 3 eclasses: mysql-v2.eclass[2] (base |
4 |
> eclass), mysql-autotools.eclass[3] (for autotools based releases) |
5 |
> and mysql-cmake.eclass[4] (for cmake based releases). The first 2 |
6 |
> eclasses are complete, pending any updates from the review. The |
7 |
> mysql-cmake eclass is still under development, but can also benefit |
8 |
> from a review. |
9 |
|
10 |
I didn't go through all of it, but here are a few things that I've |
11 |
noticed in mysql-v2.eclass: |
12 |
|
13 |
> # @ECLASS: mysql.eclass |
14 |
|
15 |
Shouldn't this match the filename of the eclass? (Same for |
16 |
mysql-autotools.eclass.) |
17 |
|
18 |
> # @DESCRIPTION: |
19 |
> # The mysql.eclass provides almost all the code to build the mysql ebuilds |
20 |
> # including the src_unpack, src_prepare, src_configure, src_compile, |
21 |
> # scr_install, pkg_preinst, pkg_postinst, pkg_config and pkg_postrm |
22 |
> # phase hooks. |
23 |
|
24 |
Name of the eclass should be updated. |
25 |
|
26 |
> MYSQL_EXPF="src_unpack src_compile src_install" |
27 |
> case "${EAPI:-0}" in |
28 |
> 2|3|4) MYSQL_EXPF+=" src_prepare src_configure" ;; |
29 |
> *) die "Unsupported EAPI: ${EAPI}" ;; |
30 |
> esac |
31 |
|
32 |
> EXPORT_FUNCTIONS ${MYSQL_EXPF} |
33 |
|
34 |
You don't need a global variable here: |
35 |
,---- |
36 |
| EXPORT_FUNCTIONS src_unpack src_compile src_install |
37 |
| case "${EAPI:-0}" in |
38 |
| 2|3|4) EXPORT_FUNCTIONS src_prepare src_configure ;; |
39 |
| *) die "Unsupported EAPI: ${EAPI}" ;; |
40 |
| esac |
41 |
`---- |
42 |
|
43 |
or even: |
44 |
,---- |
45 |
| case "${EAPI:-0}" in |
46 |
| 2|3|4) ;; |
47 |
| *) die "Unsupported EAPI: ${EAPI}" ;; |
48 |
| esac |
49 |
| EXPORT_FUNCTIONS src_unpack src_prepare src_configure src_compile src_install |
50 |
`---- |
51 |
|
52 |
> # @ECLASS-VARIABLE: XTRADB_VER |
53 |
> # @DESCRIPTION: |
54 |
> # Version of the XTRADB storage engine |
55 |
> XTRADB_VER="${XTRADB_VER}" |
56 |
|
57 |
Is this assignment needed, or could you use @DEFAULT_UNSET instead? |
58 |
(Assuming it's for the eclass manpage.) Same for other variables. |
59 |
|
60 |
> # Having different flavours at the same time is not a good idea |
61 |
> for i in "mysql" "mysql-community" "mysql-cluster" "mariadb" ; do |
62 |
> [[ "${i}" == ${PN} ]] || |
63 |
|
64 |
Quotes are not necessary here. |
65 |
|
66 |
> pbxt_patch_available \ |
67 |
> && PBXT_P="pbxt-${PBXT_VERSION}" \ |
68 |
> && PBXT_SRC_URI="http://www.primebase.org/download/${PBXT_P}.tar.gz mirror://sourceforge/pbxt/${PBXT_P}.tar.gz" \ |
69 |
> && SRC_URI="${SRC_URI} pbxt? ( ${PBXT_SRC_URI} )" \ |
70 |
|
71 |
> # PBXT_NEWSTYLE means pbxt is in storage/ and gets enabled as other plugins |
72 |
> # vs. built outside the dir |
73 |
> pbxt_available \ |
74 |
> && IUSE="${IUSE} pbxt" \ |
75 |
> && mysql_version_is_at_least "5.1.40" \ |
76 |
> && PBXT_NEWSTYLE=1 |
77 |
|
78 |
> xtradb_patch_available \ |
79 |
> && XTRADB_P="percona-xtradb-${XTRADB_VER}" \ |
80 |
> && XTRADB_SRC_URI_COMMON="${PERCONA_VER}/source/${XTRADB_P}.tar.gz" \ |
81 |
> && XTRADB_SRC_B1="http://www.percona.com/" \ |
82 |
> && XTRADB_SRC_B2="${XTRADB_SRC_B1}/percona-builds/" \ |
83 |
> && XTRADB_SRC_URI1="${XTRADB_SRC_B2}/Percona-Server/Percona-Server-${XTRADB_SRC_URI_COMMON}" \ |
84 |
> && XTRADB_SRC_URI2="${XTRADB_SRC_B2}/xtradb/${XTRADB_SRC_URI_COMMON}" \ |
85 |
> && XTRADB_SRC_URI3="${XTRADB_SRC_B1}/${PN}/xtradb/${XTRADB_SRC_URI_COMMON}" \ |
86 |
> && SRC_URI="${SRC_URI} xtradb? ( ${XTRADB_SRC_URI1} ${XTRADB_SRC_URI2} ${XTRADB_SRC_URI3} )" \ |
87 |
> && IUSE="${IUSE} xtradb" |
88 |
|
89 |
Probably a matter of taste, but I'd use "if" blocks instead of the |
90 |
multiple && here. |
91 |
|
92 |
> mv --strip-trailing-slashes -T "${old_MY_DATADIR_s}" "${MY_DATADIR_s}" \ |
93 |
|
94 |
Both options --strip-trailing-slashes and -T are GNUisms and may not |
95 |
exist on other userlands (like BSD). |
96 |
|
97 |
Ulrich |