[ptxdist] [PATCH] Detect changes in package patch series

[ptxdist] [PATCH] Detect changes in package patch series

[jon]

From: Jon Ringle <jringle@gridpoint.com>

For a long time it has bothered me that if a package's patches were changed
ptxdist would not detect this change and I would often have old versions of
*-dev.tar.gz packages that got used because the packages patches were
updated.

This commit solves this problem.

Here's how it works:
1) In the package rule makefile add `${PKG}_SERIES_MD5 :=`
2) extract the package and from the packages src dir do `git ptx-patches`
   This will populate the rule makefile _SERIES_MD5 value
3) Anytime you make a patch change to a package and do `git ptx-patches`
   a new value gets populated in the rule makefile, which then causes the
   package to be rebuilt and the *-dev.tar.gz package will have a different
   sum in the filename

Signed-off-by: Jon Ringle <jringle@gridpoint.com>
---
 scripts/git-ptx-patches                | 36 +++++++++++++++++++++++++++-------
 scripts/lib/ptxd_make_world_patchin.sh | 22 +++++++++++++++++++++
 2 files changed, 51 insertions(+), 7 deletions(-)

diff --git a/scripts/git-ptx-patches b/scripts/git-ptx-patches
index f2568f9..738a4ac 100755
--- a/scripts/git-ptx-patches
+++ b/scripts/git-ptx-patches
@@ -2,11 +2,33 @@
 
 PTX_PATCHES_HEADER="# generated by git-ptx-patches"
 
-function _md5sum() {
-	local sum=$(md5sum)
+function _sha1sum() {
+	local sum=$(git rev-parse HEAD)
 	echo "# $sum git-ptx-patches magic"
 }
 
+update_series_md5() {
+	local makefile="$(readlink -f .ptxdist/rule.make)"
+	source .ptxdist/rule.env
+	set -- $(md5sum .ptxdist/series)
+	local md5="${1}"
+
+	local count=$(grep "^${PKG}_SERIES_MD5[	 ]*:=" "${makefile}" 2> /dev/null | wc -l)
+	if [ "${count}" -gt 1 ]; then
+		echo "Error: Could not update patch series md5sum for '${pkg_label}': ${PKG}_SERIES_MD5 found ${count} times in '${makefile}'."
+		exit 1
+	fi
+	local current_md5=$(grep "^PUGIXML_SERIES_MD5[	 ]*:= " ${makefile} |cut -f2 -d=|xargs)
+	if [ "${current_md5}" != "${md5}" ]; then
+		sed -i "s/^\(\<${PKG}_SERIES_MD5[	 ]*:=\) *[a-f0-9]*\$/\1 ${md5}/" "${makefile}"
+		if ! grep -q "${md5}\$" .ptxdist/rule.make; then
+			echo "Warning: ${PKG}_SERIES_MD5 is missing from '${makefile}'."
+		else
+			echo "New patch series checksum for '${pkg_label}': ${md5} in '${makefile}'."
+		fi
+	fi
+}
+
 if [ ! -L .ptxdist/patches ]; then
 	echo "Error: This is not patched by ptxdist. Aborting."
 	exit 1
@@ -22,10 +44,8 @@ tag=base
 
 if grep -q "$PTX_PATCHES_HEADER" .ptxdist/series; then
 	echo "Found series file generated by git-ptx-patches."
-	lines=$(wc -l < .ptxdist/series)
-	lines=$[lines-1]
-	magic=$(head -n$lines .ptxdist/series | _md5sum)
-	if grep -q "^$magic" .ptxdist/series; then
+	magic=$(git rev-parse HEAD)
+	if grep -q "^# $magic" .ptxdist/series; then
 		remove_old=yes
 	else
 		echo "Warning: .ptxdist/series was modified."
@@ -132,7 +152,9 @@ cat .ptxdist/series.0 > .ptxdist/series
 git format-patch -M -N $GIT_EXTRA_ARGS ${tagopt} -o .ptxdist/patches/ ${range} | sed -e 's,^.ptxdist/patches/,,' > .ptxdist/series.auto
 cat .ptxdist/series.auto >> .ptxdist/series
 cat .ptxdist/series.1 >> .ptxdist/series
-cat .ptxdist/series | _md5sum >> .ptxdist/series
+cat .ptxdist/series | _sha1sum >> .ptxdist/series
+
+update_series_md5
 
 # The first line of the patch is 'From <some-git-hash> ...'
 # remove it to avoid unnecessary changes in the patch files.
diff --git a/scripts/lib/ptxd_make_world_patchin.sh b/scripts/lib/ptxd_make_world_patchin.sh
index e57da64..25decf0 100644
--- a/scripts/lib/ptxd_make_world_patchin.sh
+++ b/scripts/lib/ptxd_make_world_patchin.sh
@@ -221,6 +221,8 @@ ptxd_make_world_patchin_apply()
 	pkg_patch_series \
 	pkg_patch_tool
 
+    local PKG="$(ptxd_name_to_NAME "${pkg_label}")"
+
     if [[ "${pkg_url}" =~ ^file:// ]]; then
 	local url="$(ptxd_file_url_path "${pkg_url}")"
 	# local directories are not intended to be patched
@@ -255,6 +257,16 @@ ptxd_make_world_patchin_apply()
     mkdir "${pkg_patchin_dir}/.ptxdist" &&
 
     #
+    # create a ".ptxdist/rule.make" link pointing to the packages rule makefile
+    #
+    ln -s "${pkg_makefile}" "${pkg_patchin_dir}/.ptxdist/rule.make" &&
+    (
+	cat <<-EOF
+	PKG=${PKG}
+	pkg_label=${pkg_label}
+	EOF
+    ) > "${pkg_patchin_dir}/.ptxdist/rule.env" &&
+    #
     # create a ".ptxdist/patches" link pointing to the directory
     # containing the patches
     #
@@ -341,6 +353,7 @@ ptxd_make_world_patchin_apply()
     echo
     echo "pkg_patch_dir:     '$(ptxd_print_path "${pkg_patch_dir:-<none>}")'"
     echo "pkg_patch_series:  '$(ptxd_print_path "${pkg_patch_series:-<none>}")'"
+    echo "pkg_makefile:    '$(ptxd_print_path "${pkg_makefile:-<none>}")'"
     echo
 
     # apply patches if series file is available
@@ -437,6 +450,7 @@ export -f ptxd_make_world_autogen
 # pkg_patchin_dir	where to apply the patches
 # pkg_patch_dir		path to dir that contains the patches
 #			empty if no patches should be applied
+# pkg_makefile		the package's rule makefile
 #
 ptxd_make_world_patchin_init()
 {
@@ -463,6 +477,14 @@ ptxd_make_world_patchin_init()
 	return
     fi
     pkg_patch_dir="${ptxd_reply}"
+
+    #
+    # find rules make
+    #
+    if ! ptxd_find_pkg_makefile "${pkg_label}" ; then
+	return
+    fi
+    pkg_makefile="$(readlink -f "${ptxd_reply}")"
 }
 export -f ptxd_make_world_patchin_init
 
-- 
1.9.1


_______________________________________________
ptxdist mailing list
ptxdist@pengutronix.de

Re: [ptxdist] [PATCH] Detect changes in package patch series

[Uwe Kleine-König]

Hello Jon,

On Thu, Aug 02, 2018 at 09:49:59PM -0400, jon@ringle.org wrote:
> diff --git a/scripts/git-ptx-patches b/scripts/git-ptx-patches
> index f2568f9..738a4ac 100755
> --- a/scripts/git-ptx-patches
> +++ b/scripts/git-ptx-patches
> @@ -2,11 +2,33 @@
>  
>  PTX_PATCHES_HEADER="# generated by git-ptx-patches"
>  
> -function _md5sum() {
> -	local sum=$(md5sum)
> +function _sha1sum() {
> +	local sum=$(git rev-parse HEAD)
>  	echo "# $sum git-ptx-patches magic"
>  }
>  
> +update_series_md5() {
> +	local makefile="$(readlink -f .ptxdist/rule.make)"
> +	source .ptxdist/rule.env
> +	set -- $(md5sum .ptxdist/series)
> +	local md5="${1}"

Is this better than:

	local md5="$(md5sum .ptxdist/series)"

?

If I understand this right, this catches updates to the patch series
that change the series but if I only amend a patch this goes unnoticed.

Given that md5 is considered weak today, maybe use something more modern
instead (e.g. sha256?).

> +	local count=$(grep "^${PKG}_SERIES_MD5[	 ]*:=" "${makefile}" 2> /dev/null | wc -l)
> +	if [ "${count}" -gt 1 ]; then
> +		echo "Error: Could not update patch series md5sum for '${pkg_label}': ${PKG}_SERIES_MD5 found ${count} times in '${makefile}'."
> +		exit 1
> +	fi
> +	local current_md5=$(grep "^PUGIXML_SERIES_MD5[	 ]*:= " ${makefile} |cut -f2 -d=|xargs)

PUGIXML?

> +	if [ "${current_md5}" != "${md5}" ]; then
> +		sed -i "s/^\(\<${PKG}_SERIES_MD5[	 ]*:=\) *[a-f0-9]*\$/\1 ${md5}/" "${makefile}"
> +		if ! grep -q "${md5}\$" .ptxdist/rule.make; then
> +			echo "Warning: ${PKG}_SERIES_MD5 is missing from '${makefile}'."
> +		else
> +			echo "New patch series checksum for '${pkg_label}': ${md5} in '${makefile}'."
> +		fi
> +	fi
> +}
> +
>  if [ ! -L .ptxdist/patches ]; then
>  	echo "Error: This is not patched by ptxdist. Aborting."
>  	exit 1

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

_______________________________________________
ptxdist mailing list
ptxdist@pengutronix.de

Re: [ptxdist] [PATCH] Detect changes in package patch series

[Jon Ringle]

On Fri, Aug 3, 2018 at 3:19 AM Uwe Kleine-König
<u.kleine-koenig@pengutronix.de> wrote:
>
> Hello Jon,
>
> On Thu, Aug 02, 2018 at 09:49:59PM -0400, jon@ringle.org wrote:
> > diff --git a/scripts/git-ptx-patches b/scripts/git-ptx-patches
> > index f2568f9..738a4ac 100755
> > --- a/scripts/git-ptx-patches
> > +++ b/scripts/git-ptx-patches
> > @@ -2,11 +2,33 @@
> >
> >  PTX_PATCHES_HEADER="# generated by git-ptx-patches"
> >
> > -function _md5sum() {
> > -     local sum=$(md5sum)
> > +function _sha1sum() {
> > +     local sum=$(git rev-parse HEAD)
> >       echo "# $sum git-ptx-patches magic"
> >  }
> >
> > +update_series_md5() {
> > +     local makefile="$(readlink -f .ptxdist/rule.make)"
> > +     source .ptxdist/rule.env
> > +     set -- $(md5sum .ptxdist/series)
> > +     local md5="${1}"
>
> Is this better than:
>
>         local md5="$(md5sum .ptxdist/series)"
>
> ?

I was modeling the pattern found in `ptxd_make_world_update_md5()`
found in scripts/lib/ptxd_make_world_check_src.sh:
  | ptxd_make_world_update_md5() {
  |     local PKG="$(ptxd_name_to_NAME "${pkg_label}")"
  |     set -- $(md5sum "${pkg_src}")
  |     local md5="${1}"

I can change it to the way you are suggesting, though

>
> If I understand this right, this catches updates to the patch series
> that change the series but if I only amend a patch this goes unnoticed.

There are two different sums going on here. This one captures the sum
of the series file, but in the next 2 hunks that you trimmed from your
response is the embedded in the series file the sha1 of the HEAD git
commit obtained via `git rev-parse HEAD` in the function _sha1sum.
This will also capture changes to commits in the git repo where the
subject line stays the same and therefore doesn't change the series
patch filename, but does change the HEAD sha1:

@@ -22,10 +44,8 @@ tag=base

 if grep -q "$PTX_PATCHES_HEADER" .ptxdist/series; then
  echo "Found series file generated by git-ptx-patches."
- lines=$(wc -l < .ptxdist/series)
- lines=$[lines-1]
- magic=$(head -n$lines .ptxdist/series | _md5sum)
- if grep -q "^$magic" .ptxdist/series; then
+ magic=$(git rev-parse HEAD)
+ if grep -q "^# $magic" .ptxdist/series; then
  remove_old=yes
  else
  echo "Warning: .ptxdist/series was modified."
@@ -132,7 +152,9 @@ cat .ptxdist/series.0 > .ptxdist/series
 git format-patch -M -N $GIT_EXTRA_ARGS ${tagopt} -o .ptxdist/patches/
${range} | sed -e 's,^.ptxdist/patches/,,' > .ptxdist/series.auto
 cat .ptxdist/series.auto >> .ptxdist/series
 cat .ptxdist/series.1 >> .ptxdist/series
-cat .ptxdist/series | _md5sum >> .ptxdist/series
+cat .ptxdist/series | _sha1sum >> .ptxdist/series
+
+update_series_md5


>
> Given that md5 is considered weak today, maybe use something more modern
> instead (e.g. sha256?).
>
> > +     local count=$(grep "^${PKG}_SERIES_MD5[  ]*:=" "${makefile}" 2> /dev/null | wc -l)
> > +     if [ "${count}" -gt 1 ]; then
> > +             echo "Error: Could not update patch series md5sum for '${pkg_label}': ${PKG}_SERIES_MD5 found ${count} times in '${makefile}'."
> > +             exit 1
> > +     fi
> > +     local current_md5=$(grep "^PUGIXML_SERIES_MD5[   ]*:= " ${makefile} |cut -f2 -d=|xargs)
>
> PUGIXML?

Opps... that was a local package I was testing the feature with. I will fix :)

-Jon

_______________________________________________
ptxdist mailing list
ptxdist@pengutronix.de

Re: [ptxdist] [PATCH] Detect changes in package patch series

[Jon Ringle]

On Fri, Aug 3, 2018 at 3:36 AM Jon Ringle <jon@ringle.org> wrote:
>
> On Fri, Aug 3, 2018 at 3:19 AM Uwe Kleine-König
> <u.kleine-koenig@pengutronix.de> wrote:
> >
> > Hello Jon,
> >
> > On Thu, Aug 02, 2018 at 09:49:59PM -0400, jon@ringle.org wrote:
> > > diff --git a/scripts/git-ptx-patches b/scripts/git-ptx-patches
> > > index f2568f9..738a4ac 100755
> > > --- a/scripts/git-ptx-patches
> > > +++ b/scripts/git-ptx-patches
> > > @@ -2,11 +2,33 @@
> > >
> > >  PTX_PATCHES_HEADER="# generated by git-ptx-patches"
> > >
> > > -function _md5sum() {
> > > -     local sum=$(md5sum)
> > > +function _sha1sum() {
> > > +     local sum=$(git rev-parse HEAD)
> > >       echo "# $sum git-ptx-patches magic"
> > >  }
> > >
> > > +update_series_md5() {
> > > +     local makefile="$(readlink -f .ptxdist/rule.make)"
> > > +     source .ptxdist/rule.env
> > > +     set -- $(md5sum .ptxdist/series)
> > > +     local md5="${1}"
> >
> > Is this better than:
> >
> >         local md5="$(md5sum .ptxdist/series)"
> >
> > ?
>
> I was modeling the pattern found in `ptxd_make_world_update_md5()`
> found in scripts/lib/ptxd_make_world_check_src.sh:
>   | ptxd_make_world_update_md5() {
>   |     local PKG="$(ptxd_name_to_NAME "${pkg_label}")"
>   |     set -- $(md5sum "${pkg_src}")
>   |     local md5="${1}"
>
> I can change it to the way you are suggesting, though

local sha256="$(sha256sum .ptxdist/series)"
does not work, because this assigns to sha256 the value
"36169a70924328a204978a3bee7100efa526059b41a11572bdf210048109a808
.ptxdist/series"

_______________________________________________
ptxdist mailing list
ptxdist@pengutronix.de