On Fri, Jun 04, 2010 at 11:38:23PM +0200, Remy Bohmer wrote:
> Hi,
> 
> 2010/6/4 Michael Olbrich <[email protected]>:
> > On Fri, Jun 04, 2010 at 10:34:18PM +0200, Remy Bohmer wrote:
> >> When files in the projectroot directory are being used in install_copy() 
> >> and
> >> install_alternative() then there are no dependencies to these files.
> >>
> >> This is being solved by storing all dependencies of the targetinstall
> >> stage in a seperate state/<package>.deps file.
> >> These state/<package>.deps files are included in the top-level ptxdist 
> >> makefile
> >> and this makes ptxdist properly handle the dependencies.
> >
> > I haven't tested it but I like the idea. But I think this is not something
> > we should merge right before the release. Marc?
> 
> I did not shoot it in as last minute fix for something, so I would
> suggest to merge it in right after the release...
> Then it can settle for a whole release cycle.

Agreed.

> 
> >> Signed-off-by: Remy Bohmer <[email protected]>
> >> Signed-off-by: Bart vdr. Meulen <[email protected]>
> >> ---
> >>  bin/ptxdist                          |    3 +++
> >>  rules/other/Toplevel.make            |    3 +++
> >>  scripts/lib/ptxd_make_install.sh     |    3 ++-
> >>  scripts/lib/ptxd_make_xpkg_common.sh |    1 +
> >>  scripts/lib/ptxd_make_xpkg_pkg.sh    |   10 ++++++++++
> >>  5 files changed, 19 insertions(+), 1 deletions(-)
> >>
> >> diff --git a/bin/ptxdist b/bin/ptxdist
> >> index fda0770..8953be0 100755
> >> --- a/bin/ptxdist
> >> +++ b/bin/ptxdist
> >> @@ -965,13 +965,16 @@ drop() {
> >>
> >>       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
> >
> > What's this part for?
> 
> When any state will be dropped the targetinstall phase will always run
> at some point, so in that case the deps file is not needed and a new
> deps-file will finally be generated.
> If this deps file is not removed here, it would become an forever
> growing deps files. Besides, if the configuration changes between 2
> compiles the deps file will not demand dependencies that might no
> longer exist due to changed config.
> So, this is required to make it robust to changes in configurations
> between incremental builds.

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.

> 
> >> diff --git a/rules/other/Toplevel.make b/rules/other/Toplevel.make
> >> index 51f671e..35740d0 100644
> >> --- a/rules/other/Toplevel.make
> >> +++ b/rules/other/Toplevel.make
> >> @@ -86,6 +86,9 @@ PTX_PACKAGES_SELECTED       := \
> >>  # might be non existent
> >>  -include $(wildcard $(POSTRULESDIR)/*.make)
> >>  -include $(wildcard $(PROJECTPOSTRULESDIR)/*.make)
> >> +# install_alternative and install_copy has some configuration defined
> >> +# dependencies. include the files specifying these dependencies.
> >> +-include $(wildcard $(STATEDIR)/*.deps)
> >>
> >>  # 
> >> ----------------------------------------------------------------------------
> >>  # just the "print" target
> >> diff --git a/scripts/lib/ptxd_make_install.sh 
> >> b/scripts/lib/ptxd_make_install.sh
> >> index 539a67a..93209e9 100644
> >> --- a/scripts/lib/ptxd_make_install.sh
> >> +++ b/scripts/lib/ptxd_make_install.sh
> >> @@ -45,7 +45,8 @@ ptxd_make_install_init() {
> >>      rm -fr   -- \
> >>       "${pkg_xpkg_tmp}" \
> >>       "${pkg_xpkg_cmds}" \
> >> -     "${pkg_xpkg_perms}" &&
> >> +     "${pkg_xpkg_perms}" \
> >> +     "${pkg_xpkg_install_deps}" &&
> >>      mkdir -p -- "${pkg_ipkg_control_dir}" &&
> >>      touch "${pkg_xpkg_cmds}" || return
> >>
> >> diff --git a/scripts/lib/ptxd_make_xpkg_common.sh 
> >> b/scripts/lib/ptxd_make_xpkg_common.sh
> >> index 89cde2c..a756f20 100644
> >> --- a/scripts/lib/ptxd_make_xpkg_common.sh
> >> +++ b/scripts/lib/ptxd_make_xpkg_common.sh
> >> @@ -40,6 +40,7 @@ ptxd_make_xpkg_init() {
> >>      pkg_xpkg_license_file="${ptx_state_dir}/${pkg_xpkg}.license"
> >>
> >>      # packaging stuff
> >> +    pkg_xpkg_install_deps="${ptx_state_dir}/${pkg_xpkg}.deps"
> >>      pkg_xpkg_perms="${ptx_state_dir}/${pkg_xpkg}.perms"
> >>      pkg_xpkg_cmds="${ptx_state_dir}/${pkg_xpkg}.cmds"
> >>      pkg_xpkg_tmp="${ptx_pkg_dir}/${pkg_xpkg}.tmp"
> >> diff --git a/scripts/lib/ptxd_make_xpkg_pkg.sh 
> >> b/scripts/lib/ptxd_make_xpkg_pkg.sh
> >> index bdc100a..156dbed 100644
> >> --- a/scripts/lib/ptxd_make_xpkg_pkg.sh
> >> +++ b/scripts/lib/ptxd_make_xpkg_pkg.sh
> >> @@ -72,6 +72,16 @@ ptxd_install_setup_src() {
> >>
> >>      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.
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.

> 
> >> +             echo "${deprule//${PTXDIST_TOPDIR}/\$(PTXDIST_TOPDIR)}" >> \
> >> +                                                     
> >> ${pkg_xpkg_install_deps}
> >>           return
> >>       fi
> >>      done
> >
> > mol
> >
> > --
> > 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]
> >
> 
> -- 
> ptxdist mailing list
> [email protected]
> 

-- 
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]

Reply via email to