On Wed, Apr 27, 2011 at 07:54:07AM -0500, George McCollister wrote:
> I've replied to all of your comments below.
> 
> On 04/26/2011 08:26 AM, Michael Olbrich wrote:
> >On Mon, Apr 25, 2011 at 01:14:18PM -0500, George McCollister wrote:
> >>opkg and opkg-utils can now be used to build .ipk files and generate
> >>images from them.
> >Please split into multiple commits. E.g. to add a host tool, some
> >generalization, and finlly the actual ipkg/opkg selection.
> I've broken it down as follows. Is this alright or were you thinking
> differently?
> 
> commit 085e8316772b65771ab1c9bd30d1586e3a2977c6
> Author: George McCollister <george.mccollis...@gmail.com>
> Date:   Tue Apr 26 15:13:52 2011 -0500
> 
>     opkg can now be used instead of ipkg
> 
>     opkg and opkg-utils can now be used to build .ipk files and generate
>     images from them.
> 
>     Added template opkg.conf file.
> 
>     No longer default HOST_IPKG_UTILS and HOST_IPKG to yes.
>     HOST_PACKAGE_MANAGEMENT_IPKG will select HOST_IPKG.
>     HOST_IPKG will select HOST_IPKG_UTILS.
> 
>     Added package management selection to hosttools.
> 
>     Changed scripts to use the package management utilities for the
> selected
>     package management system.
> 
>     Signed-off-by: George McCollister <george.mccollis...@gmail.com>
> 
>  generic/etc/opkg/opkg.conf                      |   12 ++++++

Shouldn't this be installed to the target somewhere, or did I miss it.
This could be a separate patch too.

>  rules/host-ipkg-utils.in                        |    2 -
>  rules/host-ipkg.in                              |    3 +-
>  rules/host-package-management.in                |   22 +++++++++++
>  rules/post/image_ipkg.make                      |    9 +++--
>  rules/post/ptxd_make_image_common.make          |    1 +
>  rules/post/ptxd_make_xpkg_common.make           |    4 ++-
>  rules/post/virtual.make                         |    5 +++
>  scripts/ipkg-push                               |    5 ++-
>  scripts/lib/ptxd_make_image_prepare_work_dir.sh |   18 ++++++---
>  scripts/lib/ptxd_make_ipkg_finish.sh            |    3 +-
>  scripts/lib/ptxd_make_opkg_common.sh            |   21 +++++++++++
>  scripts/lib/ptxd_make_opkg_finish.sh            |   45
> +++++++++++++++++++++++
>  13 files changed, 133 insertions(+), 17 deletions(-)

This is still a lot, but I don't think it can be split up.

[...]
> >>diff --git a/rules/host-opkg-utils.make b/rules/host-opkg-utils.make
> >>new file mode 100644
> >>index 0000000..e831714
> >>--- /dev/null
> >>+++ b/rules/host-opkg-utils.make
> >>@@ -0,0 +1,44 @@
> >>+# -*-makefile-*-
> >>+#
> >>+# Copyright (C) 2005 by Robert Schwebel
> >>+#               2010 by Marc Kleine-Budde<m...@pengutronix.de>
> >>+#               2011 by George McCollister<george.mccollis...@gmail.com>
> >This is a new file. Just your (C) is fine.
> Okay, I'll change it. I copied host-ipkg-utils.make to create this however.

Yes, but anything still left from there is in the template too.

> >>+#
> >>+# See CREDITS for details about who has contributed to this project.
> >>+#
> >>+# For further information about the PTXdist project and license conditions
> >>+# see the README file.
> >>+#
> >>+
> >>+#
> >>+# We provide this package
> >>+#
> >>+HOST_PACKAGES-$(PTXCONF_HOST_OPKG_UTILS) += host-opkg-utils
> >>+
> >>+#
> >>+# Paths and names
> >>+#
> >>+HOST_OPKG_UTILS_VERSION    := r4747
> >>+HOST_OPKG_UTILS            := opkg-utils-$(HOST_OPKG_UTILS_VERSION)
> >>+HOST_OPKG_UTILS_SUFFIX     := tar.gz
> >>+HOST_OPKG_UTILS_URL        := 
> >>http://fixme/packages/opkg-utils/$(HOST_OPKG_UTILS).$(HOST_OPKG_UTILS_SUFFIX)
> This was a place holder URL. The file is hosted now and this is
> fixed. Will be correct in next posted patch.
> >>+HOST_OPKG_UTILS_SOURCE     := 
> >>$(SRCDIR)/$(HOST_OPKG_UTILS).$(HOST_OPKG_UTILS_SUFFIX)
> >>+HOST_OPKG_UTILS_DIR        := $(HOST_BUILDDIR)/$(HOST_OPKG_UTILS)
> >>+
> >>+# 
> >>----------------------------------------------------------------------------
> >>+# Get
> >>+# 
> >>----------------------------------------------------------------------------
> >>+
> >>+$(HOST_OPKG_UTILS_SOURCE):
> >>+   @$(call targetinfo)
> >>+   @$(call get, HOST_OPKG_UTILS)
> >remove this stage.
> Will do. I copied this file from host-ipkg-utils.make so I figured
> it didn't need to be cleaned up but I'll do it regardless for now
> on.

That file should be cleaned up at some point. I'm just a bit more pedantic
about new packages, to minimize later cleanups.

> >>+
> >>+# 
> >>----------------------------------------------------------------------------
> >>+# Prepare
> >>+# 
> >>----------------------------------------------------------------------------
> >>+
> >>+HOST_OPKG_UTILS_CONF_TOOL := NO
> >>+HOST_OPKG_UTILS_MAKE_OPT := PREFIX= $(HOST_ENV_CC)
> >>+HOST_OPKG_UTILS_INSTALL_OPT := $(HOST_OPKG_UTILS_MAKE_OPT) install
> >please align the ':='
> dito
> >>+
> >>+# vim: syntax=make
> >>diff --git a/rules/host-opkg.in b/rules/host-opkg.in
> >>new file mode 100644
> >>index 0000000..7309eb0
> >>--- /dev/null
> >>+++ b/rules/host-opkg.in
> >>@@ -0,0 +1,11 @@
> >>+## SECTION=hosttools_noprompt
> >>+
> >>+config HOST_OPKG
> >>+   tristate
> >>+   select HOST_OPKG_UTILS
> >>+   help
> >>+     opkg is used on the development host to install packets into
> >>+     some directory. Example:
> >>+
> >>+           opkg-cl -o . --force-depends -f ../generic/etc/opkg.conf 
> >>install foo.ipk
> >>+
> >>diff --git a/rules/host-opkg.make b/rules/host-opkg.make
> >>new file mode 100644
> >>index 0000000..4996028
> >>--- /dev/null
> >>+++ b/rules/host-opkg.make
> >>@@ -0,0 +1,47 @@
> >>+# -*-makefile-*-
> >>+#
> >>+# Copyright (C) 2005 by Robert Schwebel
> >>+#               2008, 2009 by Marc Kleine-Budde<m...@pengutronix.de>
> >>+#               2011 by George McCollister<george.mccollis...@gmail.com>
> >dito.
> done
> >>+#
> >>+# See CREDITS for details about who has contributed to this project.
> >>+#
> >>+# For further information about the PTXdist project and license conditions
> >>+# see the README file.
> >>+#
> >>+
> >>+#
> >>+# We provide this package
> >>+#
> >>+HOST_PACKAGES-$(PTXCONF_HOST_OPKG) += host-opkg
> >>+
> >>+#
> >>+# Paths and names
> >>+#
> >>+
> >>+HOST_OPKG  = $(OPKG)
> >>+HOST_OPKG_DIR      = $(HOST_BUILDDIR)/$(HOST_OPKG)
> >>+
> >>+# 
> >>----------------------------------------------------------------------------
> >>+# Prepare
> >>+# 
> >>----------------------------------------------------------------------------
> >>+
> >>+HOST_OPKG_ENV      := $(HOST_ENV)
> >>+
> >>+#
> >>+# autoconf
> >>+#
> >>+HOST_OPKG_CONF_TOOL        := autoconf
> >>+HOST_OPKG_CONF_OPT := \
> >>+   $(HOST_AUTOCONF) \
> >>+   --enable-shave \
> >>+   --with-opkglockfile=/lock
> >>+
> >>+HOST_OPKG_CONF_OPT += --disable-pathfinder
> >>+HOST_OPKG_CONF_OPT += --disable-curl
> >>+HOST_OPKG_CONF_OPT += --disable-sha256
> >>+HOST_OPKG_CONF_OPT += --disable-openssl
> >>+HOST_OPKG_CONF_OPT += --disable-ssl-curl
> >>+HOST_OPKG_CONF_OPT += --disable-gpg
> >>+
> >>+# vim: syntax=make
> >>diff --git a/rules/host-package-management.in 
> >>b/rules/host-package-management.in
> >>new file mode 100644
> >>index 0000000..6e60274
> >>--- /dev/null
> >>+++ b/rules/host-package-management.in
> >>@@ -0,0 +1,18 @@
> >>+## SECTION=hosttools
> >>+
> >>+choice
> >>+   prompt "package management  "
> >>+   default HOST_PACKAGE_MANAGEMENT_IPKG
> >>+
> >>+   config HOST_PACKAGE_MANAGEMENT_IPKG
> >>+           bool
> >>+           select HOST_IPKG
> >>+           prompt "ipkg   "
> >>+
> >>+   config HOST_PACKAGE_MANAGEMENT_OPKG
> >>+           bool
> >>+           select HOST_OPKG
> >>+           prompt "opkg   "
> >>+endchoice
> >>+
> >>+#source "generated/libc.in"
> >>diff --git a/rules/post/image_ipkg.make b/rules/post/image_ipkg.make
> >>index 0385383..6eb9b1b 100644
> >>--- a/rules/post/image_ipkg.make
> >>+++ b/rules/post/image_ipkg.make
> >>@@ -11,6 +11,12 @@
> >>
> >>  SEL_ROOTFS-$(PTXCONF_IMAGE_IPKG_PUSH_TO_REPOSITORY) += 
> >> $(STATEDIR)/ipkg-push
> >>
> >>+ifdef PTXCONF_HOST_PACKAGE_MANAGEMENT_OPKG
> >>+package_type := opkg
> >>+else
> >>+package_type := ipkg
> >>+endif
> >>+
> >I think I prefer (in rules/host-package-management.in):
> >
> >config HOST_PACKAGE_MANAGEMENT
> >     string
> >     default "ipkg" if HOST_PACKAGE_MANAGEMENT_IPKG
> >     default "opkg" if HOST_PACKAGE_MANAGEMENT_OPKG
> I followed your advice and changed it work this way.
> >>  ipkg-push : $(STATEDIR)/ipkg-push
> >>
> >>  $(STATEDIR)/ipkg-push: $(STATEDIR)/host-ipkg-utils.install.post 
> >> $(STATEDIR)/world.targetinstall
> >>@@ -18,13 +24,14 @@ $(STATEDIR)/ipkg-push: 
> >>$(STATEDIR)/host-ipkg-utils.install.post $(STATEDIR)/worl
> >>  ifdef PTXCONF_IMAGE_IPKG_FORCED_PUSH
> >>    rm  -rf 
> >> $(PTXCONF_SETUP_IPKG_REPOSITORY)/$(PTXCONF_PROJECT)/dists/$(PTXCONF_PROJECT)$(PTXCONF_PROJECT_VERSION)
> >>  endif
> >>-   @echo "pushing ipkg pakets to ipkg-repository..."
> >>+   @echo "pushing ipkg packets to ipkg-repository..."
> >>    @$(HOST_ENV) $(PTXDIST_TOPDIR)/scripts/ipkg-push \
> >>            --ipkgdir  $(call remove_quotes,$(PKGDIR)) \
> >>            --repodir  $(call 
> >> remove_quotes,$(PTXCONF_SETUP_IPKG_REPOSITORY)) \
> >>            --revision $(call remove_quotes,$(PTXDIST_VERSION_FULL)) \
> >>            --project  $(call remove_quotes,$(PTXCONF_PROJECT)) \
> >>-           --dist     $(call 
> >>remove_quotes,$(PTXCONF_PROJECT)$(PTXCONF_PROJECT_VERSION))
> >>+           --dist     $(call 
> >>remove_quotes,$(PTXCONF_PROJECT)$(PTXCONF_PROJECT_VERSION)) \
> >>+           --type     $(package_type)
> >>    @echo "ipkg-repository updated"
> >>    @touch $@
> >>
> >>@@ -38,7 +45,7 @@ $(PKGDIR)/Packages: 
> >>$(STATEDIR)/host-ipkg-utils.install.post
> >>    @echo "Creating ipkg index '$@'..."
> >>    @rm -f $(PKGDIR)/Packages*
> >>    @$(HOST_ENV) \
> >>-           ipkg-make-index -l "$(PKGDIR)/Packages.filelist" -p "$(@)" 
> >>"$(PKGDIR)"
> >>+           $(package_type)-make-index -l "$(PKGDIR)/Packages.filelist" -p 
> >>"$(@)" "$(PKGDIR)"
> >>    @echo "done."
> >>
> >>  # vim: syntax=make
> >>diff --git a/rules/post/ptxd_make_xpkg_common.make 
> >>b/rules/post/ptxd_make_xpkg_common.make
> >>index c7bff35..9ef7dd1 100644
> >>--- a/rules/post/ptxd_make_xpkg_common.make
> >>+++ b/rules/post/ptxd_make_xpkg_common.make
> >>@@ -1,6 +1,7 @@
> >>  # -*-makefile-*-
> >>  #
> >>  # Copyright (C) 2009, 2010 by Marc Kleine-Budde<m...@pengutronix.de>
> >>+#               2011 by George McCollister<george.mccollis...@gmail.com>
> >>  #
> >>  # See CREDITS for details about who has contributed to this project.
> >>  #
> >>@@ -12,12 +13,21 @@
> >>  # $1: xpkg label
> >>  # $2: PKG, uppercase pkg name
> >>  #
> >>+ifdef PTXCONF_HOST_PACKAGE_MANAGEMENT_OPKG
> >>+xpkg/env/impl = \
> >>+   $(call world/env, $(2))                                                 
> >>\
> >>+   CROSS_STRIP="$(call ptx/escape,$(CROSS_STRIP))"                         
> >>\
> >>+   pkg_xpkg="$(call ptx/escape,$(1))"                                      
> >>\
> >>+   pkg_opkg_extra_args=$(PTXCONF_IMAGE_OPKG_EXTRA_ARGS)                    
> >>\
> >>+   pkg_xpkg_type="opkg"
> >>+else
> >>  xpkg/env/impl = \
> >>    $(call world/env, $(2))                                                 
> >> \
> >>    CROSS_STRIP="$(call ptx/escape,$(CROSS_STRIP))"                         
> >> \
> >>    pkg_xpkg="$(call ptx/escape,$(1))"                                      
> >> \
> >>    pkg_ipkg_extra_args=$(PTXCONF_IMAGE_IPKG_EXTRA_ARGS)                    
> >> \
> >>    pkg_xpkg_type="ipkg"
> >>+endif
> >if you add both "pkg_opkg_extra_args=" and "pkg_ipkg_extra_args=" add use
> >"pkg_xpkg_type=$(PTXCONF_HOST_PACKAGE_MANAGEMENT)" (as I defined above),
> >then the "ifdef" is unnecessary.
> done
> >>
> >>  #
> >>  # $1: xpkg label
> >>diff --git a/rules/post/virtual.make b/rules/post/virtual.make
> >>index a484e52..4ba83fc 100644
> >>--- a/rules/post/virtual.make
> >>+++ b/rules/post/virtual.make
> >>@@ -1,6 +1,7 @@
> >>  # -*-makefile-*-
> >>  #
> >>  # Copyright (C) 2003-2010 by Marc Kleine-Budde<m...@pengutronix.de>
> >>+#               2011 by George McCollister<george.mccollis...@gmail.com>
> >>  # See CREDITS for details about who has contributed to this project.
> >>  #
> >>  # For further information about the PTXdist project and license conditions
> >>@@ -19,6 +20,10 @@ ifdef PTXCONF_HOST_IPKG_UTILS
> >>  $(STATEDIR)/virtual-cross-tools.install: 
> >> $(STATEDIR)/host-ipkg-utils.install.post
> >>  endif
> >>
> >>+ifdef PTXCONF_HOST_OPKG_UTILS
> >>+$(STATEDIR)/virtual-cross-tools.install: 
> >>$(STATEDIR)/host-opkg-utils.install.post
> >>+endif
> >>+
> >>  ifdef PTXCONF_CROSS_PKG_CONFIG_WRAPPER
> >>  $(STATEDIR)/virtual-cross-tools.install: 
> >> $(STATEDIR)/cross-pkg-config-wrapper.install.post
> >>  endif
> >>diff --git a/scripts/ipkg-push b/scripts/ipkg-push
> >>index fec0916..a2eb9ab 100755
> >>--- a/scripts/ipkg-push
> >>+++ b/scripts/ipkg-push
> >I think a follow-up patch to rename this to xpkg-push is appropriate.
> I was thinking that as well. There are some other things I was
> thinking possibly should be renamed as well but I figured it might
> be better to discuss that later.

Of cource. I just noticed it while reading your patch.

> I was also worried about causing
> backwards compatibility problems.

What kind of issues are you thinking about? Pushing to the same repository
with different ptxdist versions?

> >>@@ -31,6 +31,7 @@ usage() {
> >>    echo "  --revision<revision>       dist revision name to be updated"
> >>    echo "  --project<projectname>    project name"
> >>    echo "  --dist<distname>       use this to make a dist release 
> >> (optional)"
> >>+   echo "  --type<package type>   specify package type (default: ipkg)"
> >>    echo
> >>    exit 0
> >>  }
> >>@@ -40,6 +41,7 @@ REPODIR=
> >>  DISTREVISION=
> >>  PROJECT=
> >>  DIST=
> >>+TYPE=ipkg
> >>
> >>
> >>  #
> >>@@ -53,6 +55,7 @@ while [ $# -gt 0 ]; do
> >>            --revision) DISTREVISION=`ptxd_abspath $2`; shift 2 ;;
> >>            --project)  PROJECT=$2;                     shift 2 ;;
> >>            --dist)     DIST=$2;                        shift 2 ;;
> >>+           --type)     TYPE=$2;                        shift 2 ;;
> >>            *)  usage "unknown option $1" ;;
> >>            esac
> >>  done
> >>@@ -190,7 +193,7 @@ done
> >>
> >>  echo "creating index.....: "
> >>
> >>-(cd $REPODIR/$PROJECT/dists/$DIST&&  ipkg-make-index .>  Packages)
> >>+(cd $REPODIR/$PROJECT/dists/$DIST&&  ${TYPE}-make-index .>  Packages)
> >>
> >>  exit
> >>
> >>diff --git a/scripts/lib/ptxd_make_image_prepare_work_dir.sh 
> >>b/scripts/lib/ptxd_make_image_prepare_work_dir.sh
> >>index 055494e..3da2f2b 100644
> >>--- a/scripts/lib/ptxd_make_image_prepare_work_dir.sh
> >>+++ b/scripts/lib/ptxd_make_image_prepare_work_dir.sh
> >>@@ -1,6 +1,7 @@
> >>  #!/bin/bash
> >>  #
> >>  # Copyright (C) 2010 by Marc Kleine-Budde<m...@pengutronix.de>
> >>+#               2011 by George McCollister<george.mccollis...@gmail.com>
> >>  #
> >>  # See CREDITS for details about who has contributed to this project.
> >>  #
> >>@@ -23,11 +24,19 @@
> >>  # out:
> >>  # - $image_permissions            file containing all permissions
> >>  #
> >>-ptxd_make_image_extract_ipkg_files() {
> >>+ptxd_make_image_extract_xpkg_files() {
> >>      # FIXME: consolidate "ptxd_install_setup_src"
> >>      local src="/etc/ipkg.conf"
> >>-    local ipkg_conf="${PTXDIST_TEMPDIR}/${FUNCNAME}_ipkg.conf"
> >>+    local xpkg_conf="${PTXDIST_TEMPDIR}/${FUNCNAME}_xpkg.conf"
> >>+    local xpkg_type
> >I think xpkg_type should be defined from HOST_PACKAGE_MANAGEMENT in
> >image/env
> Done. I called it image_xpkg_type

ok

> >>      local -a list ptxd_reply
> >>+    if ptxd_get_ptxconf "PTXCONF_HOST_PACKAGE_MANAGEMENT_OPKG">  
> >>/dev/null; then
> >>+        src="/etc/opkg/opkg.conf"
> >>+   xpkg_type="opkg"
> >>+    else
> >>+        src="/etc/ipkg.conf"
> >Indent.
> Hopefully I got this right now. This indenting hurts my brain. Seems
> like tabs and spaces are used interchangeably. I apologize If I
> still have it messed up.

see my explanation above. I'm always fixing this manually. It's one of the
reasons why I configured vi to highlight tabs...

[...]
> >>diff --git a/scripts/lib/ptxd_make_opkg_finish.sh 
> >>b/scripts/lib/ptxd_make_opkg_finish.sh
> >>new file mode 100644
> >>index 0000000..97e1034
> >>--- /dev/null
> >>+++ b/scripts/lib/ptxd_make_opkg_finish.sh
> >>@@ -0,0 +1,45 @@
> >>+#!/bin/bash
> >>+#
> >>+# Copyright (C) 2005, 2006, 2007 Robert Schwebel<r.schwe...@pengutronix.de>
> >>+#               2008, 2009, 2010 by Marc Kleine-Budde<m...@pengutronix.de>
> >>+#               2011 by George McCollister<george.mccollis...@gmail.com>
> >>+#
> >>+# See CREDITS for details about who has contributed to this project.
> >>+#
> >>+# For further information about the PTXdist project and license conditions
> >>+# see the README file.
> >>+#
> >>+
> >>+#
> >>+# the actual opkg package creation, will run in fakeroot
> >>+#
> >>+ptxd_make_opkg_finish_impl() {
> >>+    chown -R 0:0 "${pkg_xpkg_tmp}"&&
> >>+    ptxd_make_xpkg_pkg "${pkg_opkg_tmp}" "${pkg_xpkg_cmds}" 
> >>"${pkg_xpkg_perms}"&&
> >>+    opkg-build ${pkg_opkg_extra_args} "${pkg_opkg_tmp}" "${ptx_pkg_dir}"
> >>+}
> >>+export -f ptxd_make_opkg_finish_impl
> >>+
> >>+
> >>+#
> >>+# create an opkg package
> >>+#
> >>+ptxd_make_opkg_finish() {
> >>+    local dep
> >>+
> >>+    # replace space with ", "
> >>+    dep="${pkg_xpkg_deps[*]}"
> >>+    dep="${dep// /, }"
> >>+
> >>+    sed -i -e "s:@DEPENDS@:${dep}:g" "${pkg_xpkg_control}" || return
> >>+
> >>+    local -a fake_args
> >>+    if [ -f "${pkg_fake_env}" ]; then
> >>+   fake_args=( "-i" "${pkg_fake_env}" )
> >>+    fi
> >>+    fake_args[${#fake_args[@]}]="-u"
> >>+
> >>+    export ${!pkg_*} ${!ptx_*}
> >>+    fakeroot "${fake_args[@]}" -- ptxd_make_opkg_finish_impl
> >>+}
> >This looks very much like ptxd_make_ipkg_finish.sh. Can't you change that
> >to ptxd_make_xpkg_finish.sh and use ${pkg_xpkg_type}?
> I have a few concerns with doing this. There is already a file named
> ptxd_make_xpkg_finish.sh and it contains a function named
> ptxd_make_xpkg_finish. Are you suggesting that we eliminate
> ptxd_make_ipkg_finish and ptxd_make_opkg_finish and merge it back
> into the existing ptxd_make_xpkg_finish function? It would really be
> shame to go through the work of merging them back together only to
> find out that we want to change the behavior to be ipkg, opkg
> specific later for some reason. If you think it's worth the trouble
> to do it now just let me know how you want it merged in with
> ptxd_make_xpkg_finish and I'll do it.

don't merge it for now. I need to take a closer look at this.

Michael


-- 
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
ptxdist@pengutronix.de

Reply via email to