On Sat, Jun 05, 2010 at 11:11:00AM +0200, Remy Bohmer wrote:
> >> >> if [ -z "${2}" ]; then
> >> >> statefile="${1}"
> >> >> + depsfile="${1%%.*}.deps"
> >> >> else
> >> >> statefile="${1}.${2}"
> >> >> + depsfile="${1}.deps"
> >> >> fi
> >> >>
> >> >> echo
> >> >> if [ -e "${STATEDIR}/${statefile}" ]; then
> >> >> rm -f -- "${STATEDIR}/${statefile}"
> >> >> + rm -f -- "${STATEDIR}/${depsfile}"
> >> >> echo "dropping ${statefile}"
> >> >> echo
> >> >> exit
> >> >
> > hmm, but this code is only called for explicit "ptxdist drop ...". I agree
> > that the depsfile should be removed, but this is not the right place.
>
> In that case, do you maybe have an idea for a better location?
I mentioned one later: ptxd_make_install_init
> >> >> for src in "${li...@]}"; do
> >> >> if [ -e "${src}" ]; then
> >> >> + # Since the dependency to the source files is dynamic we
> >> >> store
> >> >> + # the dependency information in a dependency file that
> >> >> can be
> >> >> + # included in the make files itself.
> >> >> + deprule="${ptx_state_dir}/${pkg_xpkg}.targetinstall:
> >> >> ${src}"
> >
> > I missed this earlier: This is wrong. ${pkg_xpkg} is the name of the ipkg
> > not the name of the ptxdist package. E.g. ${pkg_xpkg} could be
> > "kernel-modules" but you want "kernel". I think this should be ${pkg_label}
> > But I'm not sure if it is defined here.
>
> I checked it, and pkg_label is defined here, so I can use it for this. Thanks.
>
> > Note that ${pkg_xpkg_install_deps} should be based on ${pkg_xpkg}. This way
> > you can delete it in ptxd_make_install_init the ${pkg_xpkg_perms}.
> > Marc, does that sound right?
>
> >> >> +
> >> >> + # The PTXDIST_TOPDIR changes per build (git commit), so
> >> >> make
> >> >> + # sure that files referenced from the generic projectroot
> >> >> can
> >> >> + # always be found even if the PTXDIST_TOPDIR changes.
> >> >
> >> > I think this comment is specific to your work process.
> >>
> >> It may be, but by replacing the installation path by the original
> >> variable in the dep file (see below), I made it robust if ptxdist is
> >> being upgraded, changed, moved or something like that. The
> >> PTXDIST_TOPDIR looks here like '
> >> <somepath>/lib/ptxdist-2010.05.0-00111-g7b468d8', and get different
> >> names due to changing releases and changing commits/builds.
> >> It is very annoying if there are dep files in the state dir that has a
> >> hard reference to the installation directory of ptxdist which seem to
> >> change from time to time.
> >> This comment was meant to explain this... (Apparently it did not do
> >> its job very well ;-)) )
> >
> > I understand the issue and I agree that this should be here. But the
> > comment is confusing. Just say something about PTXDIST_TOPDIR could change.
> > The fact that this may be caused by a git commit is not really relevant
> > here.
>
> Ok, will rework.
>
> I will also do the same thing for PTXDIST_WORKSPACE as I did for
> PTXDIST_TOPDIR, like this:
> deprule=${deprule//${PTXDIST_WORKSPACE}/\$(PTXDIST_WORKSPACE)}
> This makes the depfile much more readable.
Sounds good.
mol
>
> >> >> + echo "${deprule//${PTXDIST_TOPDIR}/\$(PTXDIST_TOPDIR)}"
> >> >> >> \
> >> >> +
> >> >> ${pkg_xpkg_install_deps}
> >> >> return
--
Pengutronix e.K. | |
Industrial Linux Solutions | http://www.pengutronix.de/ |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
--
ptxdist mailing list
[email protected]
