On 2008/09/04 23:23, Brian wrote:
> I fixed the package installation process and dependencies.  This port will 
> now install properly.  Please test.
> 
> If you are installing on an iPod other than a nano, please send me a 
> disklabel readout of the iPod.  
> 
> Again, check out: http://www.rockbox.org for a list of mp3 players this 
> firmware will install on.  Additional install instructions can also be found 
> at that site.
> 
> Thanks for all the help.
> 
> Brian
> 
> Note: this is my first port, so please look out for errors I might have made.

well done :-)

I have quite a few comments but don't be disheartened,
this is not the simplest choice of software for a first port,
it looks like a good learning experience!

> COMMENT =               Rockbox Utility

this could be a bit more descriptive, maybe something like
"tool to install Rockbox firmware on portable music players"

> PKGNAME =               rbutil-${VERSION}
> VERSION =               1.0.6

no need to use a separate variable for VERSION if you only use
it in one place. though I'm wondering if it might be better to use
the SVN id as part of this, and it can be used to set WRKDIST
explicitly rather than wildcard as you have:

> WRKBUILD =              ${WRKDIR}/rockbox-*/rbutil/rbutilqt
> WRKDIST =               ${WRKDIR}/rockbox-*/rbutil

> # Would be better to have a snapshot dist file, so that the source isn't a 
> moving target

not just better, essential.

> NO_CHECKSUM =           YES

we can't put something in the tree which basically downloads what
could be some totally random software and runs it on build machines
without checking what it is. NO_CHECKSUM is for special cases e.g.
where the source is not fetched from a MASTER_SITE, rather it's
included right in the tree. (very uncommon).

> # Need to know what to do to make this happen.

fetch a copy and optionally repack (maybe without the whole
rockbox firmware source tree), then either host it yourself,
or ask here if someone can help with that.

> CHECK_LIB_DEPENDS = YES

oh, I didn't know you could do that, well spotted :) it's not
something that would be normally set in a port's own Makefile though,
rather something you could set in mk.conf, or on the make command
line. (probably more useful on bulk build machines to identify
problems; while you're working on a port, it's quicker to "make
port-lib-depends-check" now we have that option).

> WRKINST =               ${WRKDIR}/fake

this should just be left at the default

> # bad install target: install_target
> # work around until I learn a better way

>         cd ${WRKBUILD}  && cp rbutilqt ${DESTDIR}/usr/local/bin  

${INSTALL_PROGRAM}

>         mkdir -p ${DESTDIR}/usr/local/share/doc/rbutil/docs

${INSTALL_DATA_DIR}

>         cp -R  ${WRKDIR}/rockbox-*/docs/* 
> ${DESTDIR}/usr/local/share/doc/rbutil/docs

${INSTALL_DATA}

..

there are also quite a few places with spaces rather than tabs,
you need to take care of this with Makefiles, there are places
where you _must_ use tabs, but even where it's not strictly
required it should be done anyway. (Also trailing spaces on
the do-configure line).

here's an update with some of the above taken into account,
and with patches regenerated (we use "make update-patches" for
this, which generates patches against .orig files in ${WRKSRC}).
I haven't touched MASTER_SITES yet. Also adjusted WRKSRC/WRKDIST,
tweaked the installed docs a little and and re-run "make plist"

No supported hardware so I can't test it actually works here,
though it starts up and looks alright :-)

Attachment: rbutil.tgz
Description: application/tar-gz

Reply via email to