| Message ID | 20190429165413.9337-6-dann.frazier@canonical.com |
|---|---|
| State | New |
| Headers | show |
| Series | debian/rules cleanup | expand |
On Mon, Apr 29, 2019 at 10:54:13AM -0600, dann frazier wrote: > The difference between $(src_fullversion) and $(src_version) is not > self-evident. Use short names for metadata vars about this package, > and prefix those describing the unsigned counterpart with "unsigned_". > > Signed-off-by: dann frazier <dann.frazier@canonical.com> So generally I think this is ok and makes things clearer. However I found a couple of things, one which looks like it will break things, and it makes me wonder how well it's been tested. > --- > debian/rules | 46 ++++++++++++++++++++++++---------------------- > 1 file changed, 24 insertions(+), 22 deletions(-) > > diff --git a/debian/rules b/debian/rules > index 395eedd..b5a64c7 100755 > --- a/debian/rules > +++ b/debian/rules > @@ -4,22 +4,24 @@ > > DEB_HOST_ARCH = $(shell dpkg-architecture -qDEB_HOST_ARCH) > > -# Work out the source package name and version. We assume the source package > -# is the name of this package with -signed stripped. The version is identical > -# to this package less any rebuild suffic (+signedN). > -src_package = $(shell dpkg-parsechangelog -S Source | sed -e 's/-signed//') > -src_fullversion = $(shell dpkg-parsechangelog -S Version) > -src_version = $(shell echo $(src_fullversion) | sed -e 's/+signed[0-9]*.*//') > -src_abi = $(shell echo "$(src_version)" | sed -ne 's/\([0-9]*\.[0-9]*\.[0-9]*\-[0-9]*\)\..*/\1/p') > +src = $(shell LC_ALL=C dpkg-parsechangelog -S Source) > +ver = $(shell LC_ALL=C dpkg-parsechangelog -S Version) You add back LC_ALL=C after removing them in a previous patch. > +abi = $(shell echo "$(ver)" | sed -ne 's/\([0-9]*\.[0-9]*\.[0-9]*\-[0-9]*\)\..*/\1/p') > + > +# Work out the source package name and version of the unsigned package > +# By convention, it is the name of this package with -signed stripped. > +# The version is identical to this package less any rebuild suffix (+signedN). > +unsigned_src = $(shell echo $(src) | sed -e 's/-signed//') > +unsigned_ver = $(shell echo $(ver) | sed -e 's/+signed[0-9]*.*//') > > # We build our control file. This has to be done before dh runs otherwise > # we have no binary files and we will not run the appropriate targets. > pre-clean: > sed <debian/control.stub >debian/control \ > - -e "s/ABI/$(src_abi)/g" \ > - -e "s/UNSIGNED_SRC_PACKAGE/$(src_package)/g" \ > - -e "s/UNSIGNED_SRC_VERSION/$(src_version)/g" > - rm -rf ./$(src_version) UNSIGNED SIGNED > + -e "s/ABI/$(abi)/g" \ > + -e "s/UNSIGNED_SRC_PACKAGE/$(unsigned_src)/g" \ > + -e "s/UNSIGNED_SRC_VERSION/$(unsigned_ver)/g" > + rm -rf ./$(unsigned_ver) UNSIGNED SIGNED > rm -f debian/linux-image-*.install \ > debian/linux-image-*.preinst \ > debian/linux-image-*.prerm \ > @@ -35,11 +37,11 @@ clean:: pre-clean > dh $@ > > override_dh_auto_build: > - ./download-signed "linux-libc-dev" "$(src_version)" "$(src_package)" > - #./download-unsigned "$(DEB_HOST_ARCH)" "$(src_version)" > + ./download-signed "linux-libc-dev" "$(unsigned_ver)" "$(unsigned_package)" I think you mean unsigned_src here, not unsigned_package. > + #./download-unsigned "$(DEB_HOST_ARCH)" "$(unsigned_ver)" > mkdir SIGNED > ( \ > - cd "$(src_version)" || exit 1; \ > + cd "$(unsigned_ver)" || exit 1; \ > for s in *.efi.signed; do \ > [ ! -f "$$s" ] && continue; \ > base=$$(echo "$$s" | sed -e 's/.efi.signed//'); \ > @@ -64,10 +66,10 @@ override_dh_auto_build: > > override_dh_auto_install: > for signed in "SIGNED"/*; do \ > - flavour=$$(echo "$$signed" | sed -e "s@.*-$(src_abi)-@@"); \ > + flavour=$$(echo "$$signed" | sed -e "s@.*-$(abi)-@@"); \ > instfile=$$(echo "$$signed" | sed -e "s@[^/]*/@@" \ > - -e "s@-$(src_abi)-.*@@"); \ > - verflav="$(src_abi)-$$flavour"; \ > + -e "s@-$(abi)-.*@@"); \ > + verflav="$(abi)-$$flavour"; \ > \ > package="kernel-signed-image-$$verflav-di"; \ > echo "$$package: adding $$signed"; \ > @@ -77,19 +79,19 @@ override_dh_auto_install: > echo "$$package: adding $$signed"; \ > echo "$$signed boot" >>"debian/$$package.install"; \ > \ > - ./generate-depends linux-image-unsigned-$$verflav $(src_version) \ > + ./generate-depends linux-image-unsigned-$$verflav $(unsigned_ver) \ > linux-image-$$verflav \ > >>"debian/linux-image-$$verflav.substvars"; \ > \ > for which in postinst postrm preinst prerm; do \ > template="debian/templates/image.$$which.in"; \ > script="debian/$$package.$$which"; \ > - sed -e "s/@abiname@/$(src_abi)/g" \ > + sed -e "s/@abiname@/$(abi)/g" \ > -e "s/@localversion@/-$$flavour/g" \ > -e "s/@image-stem@/$$instfile/g" \ > <"$$template" >"$$script"; \ > done; \ > - echo "interest linux-update-$(src_abi)-$$flavour" \ > + echo "interest linux-update-$(abi)-$$flavour" \ > >"debian/$$package.triggers"; \ > done > dh_install > @@ -98,8 +100,8 @@ override_dh_builddeb: > dh_builddeb > for pkg in $$(dh_listpackages); do \ > case $$pkg in *dbgsym) ;; *) continue ;; esac; \ > - mv ../$${pkg}_$(src_fullversion)_$(DEB_HOST_ARCH).deb \ > - ../$${pkg}_$(src_fullversion)_$(DEB_HOST_ARCH).ddeb; \ > + mv ../$${pkg}_$(ver)_$(DEB_HOST_ARCH).deb \ > + ../$${pkg}_$(ver)_$(DEB_HOST_ARCH).ddeb; \ > sed -i "/^$${pkg}_/s/\.deb /.ddeb /" debian/files; \ > done > > -- > 2.20.1 > > > -- > kernel-team mailing list > kernel-team@lists.ubuntu.com > https://lists.ubuntu.com/mailman/listinfo/kernel-team
On Wed, May 29, 2019 at 10:33 AM Seth Forshee <seth.forshee@canonical.com> wrote: > > On Mon, Apr 29, 2019 at 10:54:13AM -0600, dann frazier wrote: > > The difference between $(src_fullversion) and $(src_version) is not > > self-evident. Use short names for metadata vars about this package, > > and prefix those describing the unsigned counterpart with "unsigned_". > > > > Signed-off-by: dann frazier <dann.frazier@canonical.com> > > So generally I think this is ok and makes things clearer. However I > found a couple of things, one which looks like it will break things, and > it makes me wonder how well it's been tested. Thanks Seth. Yes - this does highlight an issue with my testing, which was limited to verifying that the control file was being generated identically before & after in both hwe & non-hwe cases. I'll address both issues you identified and verify binary package contents as well before submitting a v2. -dann > > > --- > > debian/rules | 46 ++++++++++++++++++++++++---------------------- > > 1 file changed, 24 insertions(+), 22 deletions(-) > > > > diff --git a/debian/rules b/debian/rules > > index 395eedd..b5a64c7 100755 > > --- a/debian/rules > > +++ b/debian/rules > > @@ -4,22 +4,24 @@ > > > > DEB_HOST_ARCH = $(shell dpkg-architecture -qDEB_HOST_ARCH) > > > > -# Work out the source package name and version. We assume the source package > > -# is the name of this package with -signed stripped. The version is identical > > -# to this package less any rebuild suffic (+signedN). > > -src_package = $(shell dpkg-parsechangelog -S Source | sed -e 's/-signed//') > > -src_fullversion = $(shell dpkg-parsechangelog -S Version) > > -src_version = $(shell echo $(src_fullversion) | sed -e 's/+signed[0-9]*.*//') > > -src_abi = $(shell echo "$(src_version)" | sed -ne 's/\([0-9]*\.[0-9]*\.[0-9]*\-[0-9]*\)\..*/\1/p') > > +src = $(shell LC_ALL=C dpkg-parsechangelog -S Source) > > +ver = $(shell LC_ALL=C dpkg-parsechangelog -S Version) > > You add back LC_ALL=C after removing them in a previous patch. > > > +abi = $(shell echo "$(ver)" | sed -ne 's/\([0-9]*\.[0-9]*\.[0-9]*\-[0-9]*\)\..*/\1/p') > > + > > +# Work out the source package name and version of the unsigned package > > +# By convention, it is the name of this package with -signed stripped. > > +# The version is identical to this package less any rebuild suffix (+signedN). > > +unsigned_src = $(shell echo $(src) | sed -e 's/-signed//') > > +unsigned_ver = $(shell echo $(ver) | sed -e 's/+signed[0-9]*.*//') > > > > # We build our control file. This has to be done before dh runs otherwise > > # we have no binary files and we will not run the appropriate targets. > > pre-clean: > > sed <debian/control.stub >debian/control \ > > - -e "s/ABI/$(src_abi)/g" \ > > - -e "s/UNSIGNED_SRC_PACKAGE/$(src_package)/g" \ > > - -e "s/UNSIGNED_SRC_VERSION/$(src_version)/g" > > - rm -rf ./$(src_version) UNSIGNED SIGNED > > + -e "s/ABI/$(abi)/g" \ > > + -e "s/UNSIGNED_SRC_PACKAGE/$(unsigned_src)/g" \ > > + -e "s/UNSIGNED_SRC_VERSION/$(unsigned_ver)/g" > > + rm -rf ./$(unsigned_ver) UNSIGNED SIGNED > > rm -f debian/linux-image-*.install \ > > debian/linux-image-*.preinst \ > > debian/linux-image-*.prerm \ > > @@ -35,11 +37,11 @@ clean:: pre-clean > > dh $@ > > > > override_dh_auto_build: > > - ./download-signed "linux-libc-dev" "$(src_version)" "$(src_package)" > > - #./download-unsigned "$(DEB_HOST_ARCH)" "$(src_version)" > > + ./download-signed "linux-libc-dev" "$(unsigned_ver)" "$(unsigned_package)" > > I think you mean unsigned_src here, not unsigned_package. > > > + #./download-unsigned "$(DEB_HOST_ARCH)" "$(unsigned_ver)" > > mkdir SIGNED > > ( \ > > - cd "$(src_version)" || exit 1; \ > > + cd "$(unsigned_ver)" || exit 1; \ > > for s in *.efi.signed; do \ > > [ ! -f "$$s" ] && continue; \ > > base=$$(echo "$$s" | sed -e 's/.efi.signed//'); \ > > @@ -64,10 +66,10 @@ override_dh_auto_build: > > > > override_dh_auto_install: > > for signed in "SIGNED"/*; do \ > > - flavour=$$(echo "$$signed" | sed -e "s@.*-$(src_abi)-@@"); \ > > + flavour=$$(echo "$$signed" | sed -e "s@.*-$(abi)-@@"); \ > > instfile=$$(echo "$$signed" | sed -e "s@[^/]*/@@" \ > > - -e "s@-$(src_abi)-.*@@"); \ > > - verflav="$(src_abi)-$$flavour"; \ > > + -e "s@-$(abi)-.*@@"); \ > > + verflav="$(abi)-$$flavour"; \ > > \ > > package="kernel-signed-image-$$verflav-di"; \ > > echo "$$package: adding $$signed"; \ > > @@ -77,19 +79,19 @@ override_dh_auto_install: > > echo "$$package: adding $$signed"; \ > > echo "$$signed boot" >>"debian/$$package.install"; \ > > \ > > - ./generate-depends linux-image-unsigned-$$verflav $(src_version) \ > > + ./generate-depends linux-image-unsigned-$$verflav $(unsigned_ver) \ > > linux-image-$$verflav \ > > >>"debian/linux-image-$$verflav.substvars"; \ > > \ > > for which in postinst postrm preinst prerm; do \ > > template="debian/templates/image.$$which.in"; \ > > script="debian/$$package.$$which"; \ > > - sed -e "s/@abiname@/$(src_abi)/g" \ > > + sed -e "s/@abiname@/$(abi)/g" \ > > -e "s/@localversion@/-$$flavour/g" \ > > -e "s/@image-stem@/$$instfile/g" \ > > <"$$template" >"$$script"; \ > > done; \ > > - echo "interest linux-update-$(src_abi)-$$flavour" \ > > + echo "interest linux-update-$(abi)-$$flavour" \ > > >"debian/$$package.triggers"; \ > > done > > dh_install > > @@ -98,8 +100,8 @@ override_dh_builddeb: > > dh_builddeb > > for pkg in $$(dh_listpackages); do \ > > case $$pkg in *dbgsym) ;; *) continue ;; esac; \ > > - mv ../$${pkg}_$(src_fullversion)_$(DEB_HOST_ARCH).deb \ > > - ../$${pkg}_$(src_fullversion)_$(DEB_HOST_ARCH).ddeb; \ > > + mv ../$${pkg}_$(ver)_$(DEB_HOST_ARCH).deb \ > > + ../$${pkg}_$(ver)_$(DEB_HOST_ARCH).ddeb; \ > > sed -i "/^$${pkg}_/s/\.deb /.ddeb /" debian/files; \ > > done > > > > -- > > 2.20.1 > > > > > > -- > > kernel-team mailing list > > kernel-team@lists.ubuntu.com > > https://lists.ubuntu.com/mailman/listinfo/kernel-team
diff --git a/debian/rules b/debian/rules index 395eedd..b5a64c7 100755 --- a/debian/rules +++ b/debian/rules @@ -4,22 +4,24 @@ DEB_HOST_ARCH = $(shell dpkg-architecture -qDEB_HOST_ARCH) -# Work out the source package name and version. We assume the source package -# is the name of this package with -signed stripped. The version is identical -# to this package less any rebuild suffic (+signedN). -src_package = $(shell dpkg-parsechangelog -S Source | sed -e 's/-signed//') -src_fullversion = $(shell dpkg-parsechangelog -S Version) -src_version = $(shell echo $(src_fullversion) | sed -e 's/+signed[0-9]*.*//') -src_abi = $(shell echo "$(src_version)" | sed -ne 's/\([0-9]*\.[0-9]*\.[0-9]*\-[0-9]*\)\..*/\1/p') +src = $(shell LC_ALL=C dpkg-parsechangelog -S Source) +ver = $(shell LC_ALL=C dpkg-parsechangelog -S Version) +abi = $(shell echo "$(ver)" | sed -ne 's/\([0-9]*\.[0-9]*\.[0-9]*\-[0-9]*\)\..*/\1/p') + +# Work out the source package name and version of the unsigned package +# By convention, it is the name of this package with -signed stripped. +# The version is identical to this package less any rebuild suffix (+signedN). +unsigned_src = $(shell echo $(src) | sed -e 's/-signed//') +unsigned_ver = $(shell echo $(ver) | sed -e 's/+signed[0-9]*.*//') # We build our control file. This has to be done before dh runs otherwise # we have no binary files and we will not run the appropriate targets. pre-clean: sed <debian/control.stub >debian/control \ - -e "s/ABI/$(src_abi)/g" \ - -e "s/UNSIGNED_SRC_PACKAGE/$(src_package)/g" \ - -e "s/UNSIGNED_SRC_VERSION/$(src_version)/g" - rm -rf ./$(src_version) UNSIGNED SIGNED + -e "s/ABI/$(abi)/g" \ + -e "s/UNSIGNED_SRC_PACKAGE/$(unsigned_src)/g" \ + -e "s/UNSIGNED_SRC_VERSION/$(unsigned_ver)/g" + rm -rf ./$(unsigned_ver) UNSIGNED SIGNED rm -f debian/linux-image-*.install \ debian/linux-image-*.preinst \ debian/linux-image-*.prerm \ @@ -35,11 +37,11 @@ clean:: pre-clean dh $@ override_dh_auto_build: - ./download-signed "linux-libc-dev" "$(src_version)" "$(src_package)" - #./download-unsigned "$(DEB_HOST_ARCH)" "$(src_version)" + ./download-signed "linux-libc-dev" "$(unsigned_ver)" "$(unsigned_package)" + #./download-unsigned "$(DEB_HOST_ARCH)" "$(unsigned_ver)" mkdir SIGNED ( \ - cd "$(src_version)" || exit 1; \ + cd "$(unsigned_ver)" || exit 1; \ for s in *.efi.signed; do \ [ ! -f "$$s" ] && continue; \ base=$$(echo "$$s" | sed -e 's/.efi.signed//'); \ @@ -64,10 +66,10 @@ override_dh_auto_build: override_dh_auto_install: for signed in "SIGNED"/*; do \ - flavour=$$(echo "$$signed" | sed -e "s@.*-$(src_abi)-@@"); \ + flavour=$$(echo "$$signed" | sed -e "s@.*-$(abi)-@@"); \ instfile=$$(echo "$$signed" | sed -e "s@[^/]*/@@" \ - -e "s@-$(src_abi)-.*@@"); \ - verflav="$(src_abi)-$$flavour"; \ + -e "s@-$(abi)-.*@@"); \ + verflav="$(abi)-$$flavour"; \ \ package="kernel-signed-image-$$verflav-di"; \ echo "$$package: adding $$signed"; \ @@ -77,19 +79,19 @@ override_dh_auto_install: echo "$$package: adding $$signed"; \ echo "$$signed boot" >>"debian/$$package.install"; \ \ - ./generate-depends linux-image-unsigned-$$verflav $(src_version) \ + ./generate-depends linux-image-unsigned-$$verflav $(unsigned_ver) \ linux-image-$$verflav \ >>"debian/linux-image-$$verflav.substvars"; \ \ for which in postinst postrm preinst prerm; do \ template="debian/templates/image.$$which.in"; \ script="debian/$$package.$$which"; \ - sed -e "s/@abiname@/$(src_abi)/g" \ + sed -e "s/@abiname@/$(abi)/g" \ -e "s/@localversion@/-$$flavour/g" \ -e "s/@image-stem@/$$instfile/g" \ <"$$template" >"$$script"; \ done; \ - echo "interest linux-update-$(src_abi)-$$flavour" \ + echo "interest linux-update-$(abi)-$$flavour" \ >"debian/$$package.triggers"; \ done dh_install @@ -98,8 +100,8 @@ override_dh_builddeb: dh_builddeb for pkg in $$(dh_listpackages); do \ case $$pkg in *dbgsym) ;; *) continue ;; esac; \ - mv ../$${pkg}_$(src_fullversion)_$(DEB_HOST_ARCH).deb \ - ../$${pkg}_$(src_fullversion)_$(DEB_HOST_ARCH).ddeb; \ + mv ../$${pkg}_$(ver)_$(DEB_HOST_ARCH).deb \ + ../$${pkg}_$(ver)_$(DEB_HOST_ARCH).ddeb; \ sed -i "/^$${pkg}_/s/\.deb /.ddeb /" debian/files; \ done
The difference between $(src_fullversion) and $(src_version) is not self-evident. Use short names for metadata vars about this package, and prefix those describing the unsigned counterpart with "unsigned_". Signed-off-by: dann frazier <dann.frazier@canonical.com> --- debian/rules | 46 ++++++++++++++++++++++++---------------------- 1 file changed, 24 insertions(+), 22 deletions(-)