1 |
Dnia 16 maja 2016 11:39:23 CEST, Joerg Bornkessel <hd_brummy@g.o> napisał(a): |
2 |
>Hallo, |
3 |
>after my last commit disaster, |
4 |
>i bring my changes to review before i break some things again. |
5 |
> |
6 |
>- Added changes to make it work with eapi=6 |
7 |
>- removed some unneeded code parts (never they was used in any |
8 |
>ebuilds, i though they was integrated to make the eclass more |
9 |
>flexibel,...) |
10 |
|
11 |
After reading this I don't see anything breakingly wrong, though I haven't tested it. |
12 |
|
13 |
However, a few general suggestions: |
14 |
|
15 |
1. Split this into multiple commits. Generic cleanup should happen separately from EAPI 6 support. And it would probably be a good idea to remove features one at a time, so that reverting would be possible. |
16 |
|
17 |
2. Prefer logic that evaluates to true for future EAPIs. For example, instead of [[ ${EAPI} == 6 ]], it's better to say != [012345], so that we won't have to extend the list with every new EAPI if nothing changes. |
18 |
|
19 |
3. Please namespace the eclass functions. Using generic names is really asking for trouble. vdr_ prefix should be sufficient. |
20 |
|
21 |
4. Please kill that src_util helper. It makes the logic terribly hard to follow, and there's really no reason to use case over separate functions. |
22 |
|
23 |
5. There are some missing || die, for example after cd. |
24 |
|
25 |
I'm sorry i can't really to specific parts of the eclass but I'm replying from a phone. |
26 |
|
27 |
> |
28 |
><snipp .diff> |
29 |
>-- vdr-plugin-2.eclass 2016-05-15 22:03:21.807417485 +0200 |
30 |
>+++ vdr-plugin-2.eclass.new 2016-05-15 22:01:10.000000000 +0200 |
31 |
>@@ -1,4 +1,4 @@ |
32 |
>-# Copyright 1999-2015 Gentoo Foundation |
33 |
>+# Copyright 1999-2016 Gentoo Foundation |
34 |
> # Distributed under the terms of the GNU General Public License v2 |
35 |
> # $Id$ |
36 |
> |
37 |
>@@ -90,7 +90,7 @@ |
38 |
> # @CODE |
39 |
> |
40 |
> # Applying your own local/user patches: |
41 |
>-# This is done by using the |
42 |
>+# This is done by using the |
43 |
> # (EAPI = 4,5) epatch_user() function of the eutils.eclass, |
44 |
> # (EAPI = 6) eapply_user function integrated in EAPI = 6. |
45 |
> # Simply add your patches into one of these directories: |
46 |
>@@ -104,10 +104,7 @@ |
47 |
> inherit flag-o-matic toolchain-funcs unpacker |
48 |
> |
49 |
> case ${EAPI:-0} in |
50 |
>- 4|5) |
51 |
>- ;; |
52 |
>- 6) |
53 |
>- ewarn "EAPI 6 support for test purpose only, plz report bugs to |
54 |
>vdr@g.o" |
55 |
>+ 4|5|6) |
56 |
> ;; |
57 |
> *) die "EAPI ${EAPI} unsupported." |
58 |
> ;; |
59 |
>@@ -156,6 +153,7 @@ |
60 |
> # EBUILD=${CATEGORY}/${PN} |
61 |
> # EBUILD_V=${PVR} |
62 |
> # EOT |
63 |
>+# obsolet? fix me later... |
64 |
> { |
65 |
> echo "VDRPLUGIN_DB=1" |
66 |
> echo "CREATOR=ECLASS" |
67 |
>@@ -232,6 +230,7 @@ |
68 |
> #sed -i Makefile \ |
69 |
> # -e "s:^DVBDIR.*$:DVBDIR = ${DVB_INCLUDE_DIR}:" \ |
70 |
> # -e 's:-I$(DVBDIR)/include:-I$(DVBDIR):' |
71 |
>+ # obsolet? fix me later... |
72 |
> |
73 |
> if ! grep -q APIVERSION Makefile; then |
74 |
> ebegin " Converting to APIVERSION" |
75 |
>@@ -289,7 +288,7 @@ |
76 |
> linguas_support() { |
77 |
> # Patching Makefile for linguas support. |
78 |
># Only locales, enabled through the LINGUAS (make.conf) variable will |
79 |
>be |
80 |
>-# "compiled" and installed. |
81 |
>+# compiled and installed. |
82 |
> |
83 |
> einfo "Patching for Linguas support" |
84 |
> einfo "available Languages for ${P} are:" |
85 |
>@@ -311,12 +310,9 @@ |
86 |
> vdr_i18n() { |
87 |
> # i18n handling was deprecated since >=media-video/vdr-1.5.9, |
88 |
> # finally with >=media-video/vdr-1.7.27 it has been dropped entirely |
89 |
>and some |
90 |
>-# plugins will fail to "compile" because they're still using the old |
91 |
>variant. |
92 |
>+# plugins will fail to compile because they're still using the old |
93 |
>variant. |
94 |
> # Simply remove the i18n.o object from Makefile (OBJECT) and |
95 |
> # remove "static const tI18nPhrase*" from i18n.h. |
96 |
>-# |
97 |
>-# Plugins that are still using the old method will be pmasked until |
98 |
>they're |
99 |
>-# fixed or in case of maintainer timeout they'll be masked for |
100 |
>removal. |
101 |
> |
102 |
> gettext_missing |
103 |
> |
104 |
>@@ -391,6 +387,7 @@ |
105 |
> |
106 |
> # Plugins need to be compiled with position independent code, |
107 |
>otherwise linking |
108 |
> # VDR against it will fail |
109 |
>+ # depricated if fi, as we have only >=vdr-2 in the tree, fix me |
110 |
>later... |
111 |
> if has_version ">=media-video/vdr-1.7.13"; then |
112 |
> append-cxxflags -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE |
113 |
>-D_LARGEFILE64_SOURCE |
114 |
> fi |
115 |
>@@ -500,7 +497,9 @@ |
116 |
> die "vdr-plugin-2_src_prepare not called!" |
117 |
> fi |
118 |
> |
119 |
>- [[ ${PATCHES[@]} ]] && epatch "${PATCHES[@]}" |
120 |
>+ [[ ${EAPI} == [45] ]] && [[ ${PATCHES[@]} ]] && epatch |
121 |
>"${PATCHES[@]}" |
122 |
>+ [[ ${EAPI} == "6" ]] && [[ ${PATCHES[@]} ]] && eapply |
123 |
>"${PATCHES[@]}" |
124 |
>+ |
125 |
> debug-print "$FUNCNAME: applying user patches" |
126 |
> |
127 |
> vdr-plugin-2_src_util prepare |
128 |
>@@ -522,14 +521,12 @@ |
129 |
> fi |
130 |
> cd "${S}" |
131 |
> |
132 |
>- BUILD_TARGETS=${BUILD_TARGETS:-${VDRPLUGIN_MAKE_TARGET:-all |
133 |
>}} |
134 |
>- emake ${BUILD_PARAMS} \ |
135 |
>- ${BUILD_TARGETS} \ |
136 |
>- LOCALEDIR="${TMP_LOCALE_DIR}" \ |
137 |
>- LOCDIR="${TMP_LOCALE_DIR}" \ |
138 |
>- LIBDIR="${S}" \ |
139 |
>- TMPDIR="${T}" \ |
140 |
>- || die "emake failed" |
141 |
>+ emake all ${BUILD_PARAMS} \ |
142 |
>+ LOCALEDIR="${TMP_LOCALE_DIR}" \ |
143 |
>+ LOCDIR="${TMP_LOCALE_DIR}" \ |
144 |
>+ LIBDIR="${S}" \ |
145 |
>+ TMPDIR="${T}" \ |
146 |
>+ || die "emake all failed" |
147 |
> ;; |
148 |
> esac |
149 |
> |
150 |
>@@ -570,12 +567,11 @@ |
151 |
> |
152 |
> local SOFILE_STRING=$(grep SOFILE Makefile) |
153 |
> if [[ -n ${SOFILE_STRING} ]]; then |
154 |
>- BUILD_TARGETS=${BUILD_TARGETS:-${VDRPLUGIN_MAKE_TARGET:-install |
155 |
>}} |
156 |
>- einstall ${BUILD_PARAMS} \ |
157 |
>- ${BUILD_TARGETS} \ |
158 |
>- TMPDIR="${T}" \ |
159 |
>- DESTDIR="${D}" \ |
160 |
>- || die "einstall (makefile target) failed" |
161 |
>+ emake install \ |
162 |
>+ ${BUILD_PARAMS} \ |
163 |
>+ TMPDIR="${T}" \ |
164 |
>+ DESTDIR="${D}" \ |
165 |
>+ || die "emake install (makefile target) failed" |
166 |
> else |
167 |
> dev_check "Plugin use still the old Makefile handling" |
168 |
> insinto "${VDR_PLUGIN_DIR}" |
169 |
>@@ -609,9 +605,14 @@ |
170 |
> create_header_checksum_file ${vdr_plugin_list} |
171 |
> create_plugindb_file ${vdr_plugin_list} |
172 |
> |
173 |
>- local docfile |
174 |
>- for docfile in README* HISTORY CHANGELOG; do |
175 |
>- [[ -f ${docfile} ]] && dodoc ${docfile} |
176 |
>+ local commondoc=( README* HISTORY CHANGELOG ) |
177 |
>+ for docfile in "${commondoc[@]}"; do |
178 |
>+ if [[ ${EAPI} == "6" ]]; then |
179 |
>+ local DOCS="${DOCS} ${docfile}" |
180 |
>+ [[ -f ${docfile} ]] && einstalldocs "${DOCS[@]}" |
181 |
>+ else |
182 |
>+ [[ -f ${docfile} ]] && dodoc ${docfile} |
183 |
>+ fi |
184 |
> done |
185 |
> |
186 |
> # if VDR_CONFD_FILE is empty and ${FILESDIR}/confd exists take it |
187 |
></snapp> |
188 |
> |
189 |
>Attached: |
190 |
>vdr-plugin-2.eclass |
191 |
>vdr-plugin-2.eclass.diff |
192 |
>vdr-plugin-2.eclass.new |
193 |
> |
194 |
> |
195 |
>Cheers |
196 |
>/dev/joerg |
197 |
|
198 |
|
199 |
-- |
200 |
Best regards, |
201 |
Michał Górny (by phone) |