| 1 |
On Wed, Sep 24, 2008 at 4:51 AM, Bo Ørsted Andresen <bo.andresen@××××.dk> wrote: |
| 2 |
> On Tuesday 23 September 2008 21:39:52 Thomas Sachau wrote: |
| 3 |
>> default_src_install() { |
| 4 |
>> if [ -f Makefile -o -f GNUmakefile -o -f makefile ]; then |
| 5 |
>> if emake DESTDIR="${D} install || einstall ; then |
| 6 |
>> die "install failed" |
| 7 |
>> else |
| 8 |
>> if [[ -n ${DOCS} ]]; then |
| 9 |
>> dodoc ${DOCS} || die "dodoc failed" |
| 10 |
>> fi |
| 11 |
>> fi |
| 12 |
>> fi |
| 13 |
>> } |
| 14 |
>> |
| 15 |
>> Any more comments? Good? Bad? Interested? |
| 16 |
> |
| 17 |
> Now figure out the four flaws in the above code. |
| 18 |
|
| 19 |
Even though IMO that comment is quite condescending, I'll list out the |
| 20 |
flaws in that code: |
| 21 |
|
| 22 |
>> if [ -f Makefile -o -f GNUmakefile -o -f makefile ]; then |
| 23 |
[...] |
| 24 |
>> fi |
| 25 |
|
| 26 |
- So if those makefiles don't exist, the package should just carry on |
| 27 |
without installing anything? |
| 28 |
|
| 29 |
>> if emake DESTDIR="${D} install || einstall ; then |
| 30 |
>> die "install failed" |
| 31 |
|
| 32 |
- It is quite useless to run *both* emake install and einstall. |
| 33 |
Default are not supposed to be lazy-maintainer-proof. The maintainer |
| 34 |
should make sure that the ebuild works with emake install, and *if* it |
| 35 |
doesn't, use einstall |
| 36 |
- The above code will cause a die when either one of emake install or |
| 37 |
einstall are *successful*. The opposite behaviour is desired. |
| 38 |
|
| 39 |
>> else |
| 40 |
>> if [[ -n ${DOCS} ]]; then |
| 41 |
>> dodoc ${DOCS} || die "dodoc failed" |
| 42 |
>> fi |
| 43 |
|
| 44 |
- So, if emake install || einstall fails, one should just install the |
| 45 |
docs? The opposite behaviour is desired. |
| 46 |
|
| 47 |
If a default src_install is desired, it should cater to the most |
| 48 |
common use-cases and leave it to the maintainer to override it if |
| 49 |
desired. |
| 50 |
|
| 51 |
default_src_install() { |
| 52 |
emake DESTDIR="${D}" install || die "emake install failed" |
| 53 |
if [ -n "${DOCS}" ]; then |
| 54 |
dodoc ${DOCS} || die "dodoc failed" |
| 55 |
else |
| 56 |
# No die here because we don't know if any of these exist |
| 57 |
dodoc AUTHORS ChangeLog NEWS README |
| 58 |
fi |
| 59 |
} |
| 60 |
|
| 61 |
-- |
| 62 |
~Nirbheek Chauhan |
Archives