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

[ptxdist] [PATCH v3] 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_SHA256 :=`
2) extract the package and from the packages src dir do `git ptx-patches`
   This will populate the rule makefile _SERIES_SHA256 value
3) Anytime you make a patch change to a package and do `git ptx-patches`
   the series file gets updated with the `git rev-parse HEAD` value which
   causes the series file to have a new sha256 value and therefore 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 cfghash
   in the filename

Signed-off-by: Jon Ringle <jringle@gridpoint.com>
---

I only updated the commit comment in v3

 scripts/git-ptx-patches                | 38 +++++++++++++++++++++++++++-------
 scripts/lib/ptxd_make_world_patchin.sh | 22 ++++++++++++++++++++
 2 files changed, 52 insertions(+), 8 deletions(-)

diff --git a/scripts/git-ptx-patches b/scripts/git-ptx-patches
index f2568f9..d37c936 100755
--- a/scripts/git-ptx-patches
+++ b/scripts/git-ptx-patches
@@ -2,9 +2,31 @@
 
 PTX_PATCHES_HEADER="# generated by git-ptx-patches"
 
-function _md5sum() {
-	local sum=$(md5sum)
-	echo "# $sum git-ptx-patches magic"
+function _gitsha1sum() {
+	local sha1=$(git rev-parse HEAD)
+	echo "# $sha1 git-ptx-patches magic"
+}
+
+update_series_sha256() {
+	local makefile="$(readlink -f .ptxdist/rule.make)"
+	source .ptxdist/rule.env
+	set -- $(sha256sum .ptxdist/series)
+	local sha256="${1}"
+
+	local count=$(grep "^${PKG}_SERIES_SHA256[	 ]*:=" "${makefile}" 2> /dev/null | wc -l)
+	if [ "${count}" -gt 1 ]; then
+		echo "Error: Could not update patch series sha256sum for '${pkg_label}': ${PKG}_SERIES_SHA256 found ${count} times in '${makefile}'."
+		exit 1
+	fi
+	local current_sha256=$(grep "^${PKG}_SERIES_SHA256[	 ]*:= " "${makefile}" |cut -f2 -d=|xargs)
+	if [ "${current_sha256}" != "${sha256}" ]; then
+		sed -i "s/^\(\<${PKG}_SERIES_SHA256[	]*:=\) *[a-f0-9]*\$/\1 ${sha256}/" "${makefile}"
+		if ! grep -q "${sha256}\$" "${makefile}"; then
+			echo "Warning: ${PKG}_SERIES_SHA256 is missing from '${makefile}'."
+		else
+			echo "New patch series checksum for '${pkg_label}': ${sha256} in '${makefile}'."
+		fi
+	fi
 }
 
 if [ ! -L .ptxdist/patches ]; then
@@ -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 | _gitsha1sum >> .ptxdist/series
+
+update_series_sha256
 
 # 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 v3] Detect changes in package patch series

[Uwe Kleine-König]

On Fri, Aug 03, 2018 at 04:41:56AM -0400, jon@ringle.org wrote:
> 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_SHA256 :=`
> 2) extract the package and from the packages src dir do `git ptx-patches`
>    This will populate the rule makefile _SERIES_SHA256 value
> 3) Anytime you make a patch change to a package and do `git ptx-patches`
>    the series file gets updated with the `git rev-parse HEAD` value which
>    causes the series file to have a new sha256 value and therefore 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 cfghash
>    in the filename

I didn't test, but if the value of $(git rev-parse HEAD) is relevant:
Are you aware that this isn't reproducible, i.e. it changes by

	ptxdist clean pukyxml
	ptxdist extract --git pukyxml

?

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 v3] Detect changes in package patch series

[Jon Ringle]

[-- Attachment #1.1: Type: text/plain, Size: 1585 bytes --]

On Fri, Aug 3, 2018 at 4:51 AM Uwe Kleine-König <
u.kleine-koenig@pengutronix.de> wrote:

> On Fri, Aug 03, 2018 at 04:41:56AM -0400, jon@ringle.org wrote:
> > 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_SHA256 :=`
> > 2) extract the package and from the packages src dir do `git ptx-patches`
> >    This will populate the rule makefile _SERIES_SHA256 value
> > 3) Anytime you make a patch change to a package and do `git ptx-patches`
> >    the series file gets updated with the `git rev-parse HEAD` value which
> >    causes the series file to have a new sha256 value and therefore 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
> cfghash
> >    in the filename
>
> I didn't test, but if the value of $(git rev-parse HEAD) is relevant:
> Are you aware that this isn't reproducible, i.e. it changes by
>
>         ptxdist clean pukyxml
>         ptxdist extract --git pukyxml
>
> ?
>

under what circumstances would you get a different value for the `git
rev-parse HEAD` if the package version, series and patches being applied
has not changed?

-Jon

[-- Attachment #1.2: Type: text/html, Size: 2183 bytes --]

[-- Attachment #2: Type: text/plain, Size: 91 bytes --]

_______________________________________________
ptxdist mailing list
ptxdist@pengutronix.de

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

[Uwe Kleine-König]

On Fri, Aug 03, 2018 at 05:02:03AM -0400, Jon Ringle wrote:
> On Fri, Aug 3, 2018 at 4:51 AM Uwe Kleine-König <
> u.kleine-koenig@pengutronix.de> wrote:
> 
> > On Fri, Aug 03, 2018 at 04:41:56AM -0400, jon@ringle.org wrote:
> > > 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_SHA256 :=`
> > > 2) extract the package and from the packages src dir do `git ptx-patches`
> > >    This will populate the rule makefile _SERIES_SHA256 value
> > > 3) Anytime you make a patch change to a package and do `git ptx-patches`
> > >    the series file gets updated with the `git rev-parse HEAD` value which
> > >    causes the series file to have a new sha256 value and therefore 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
> > cfghash
> > >    in the filename
> >
> > I didn't test, but if the value of $(git rev-parse HEAD) is relevant:
> > Are you aware that this isn't reproducible, i.e. it changes by
> >
> >         ptxdist clean pukyxml
> >         ptxdist extract --git pukyxml
> >
> > ?
> >
> 
> under what circumstances would you get a different value for the `git
> rev-parse HEAD` if the package version, series and patches being applied
> has not changed?

The commiter-date changes.

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 v3] Detect changes in package patch series

[Michael Olbrich]

On Fri, Aug 03, 2018 at 04:41:56AM -0400, jon@ringle.org wrote:
> 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_SHA256 :=`
> 2) extract the package and from the packages src dir do `git ptx-patches`
>    This will populate the rule makefile _SERIES_SHA256 value
> 3) Anytime you make a patch change to a package and do `git ptx-patches`
>    the series file gets updated with the `git rev-parse HEAD` value which
>    causes the series file to have a new sha256 value and therefore 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 cfghash
>    in the filename

This is rather limited. I've been thinking about a more general solution:

I'd like to hash all patches + series during extract and create a stamp
file (like the one for the rules and options) during 'extract'. And save
some data to recreate the hash:
- list of all possible patch dirs (in case of new patches in the BSP)
- relative paths to patches and series.

I think we can recreate the hashes for all enabled packages with this with
reasonable startup overhead.

Then depend on the new stamp with the extract stage. This will need some
extra checks, so we don't loose local changes:
- make it optional
- abort before deleting the source tree if there is a .git
Stuff like this.

And of course add the hash to the config/rules hash for *-dev.tar.gz.

Unfortunately, I don't know when I'll find some time to implement this.

Michael

> Signed-off-by: Jon Ringle <jringle@gridpoint.com>
> ---
> 
> I only updated the commit comment in v3
> 
>  scripts/git-ptx-patches                | 38 +++++++++++++++++++++++++++-------
>  scripts/lib/ptxd_make_world_patchin.sh | 22 ++++++++++++++++++++
>  2 files changed, 52 insertions(+), 8 deletions(-)
> 
> diff --git a/scripts/git-ptx-patches b/scripts/git-ptx-patches
> index f2568f9..d37c936 100755
> --- a/scripts/git-ptx-patches
> +++ b/scripts/git-ptx-patches
> @@ -2,9 +2,31 @@
>  
>  PTX_PATCHES_HEADER="# generated by git-ptx-patches"
>  
> -function _md5sum() {
> -	local sum=$(md5sum)
> -	echo "# $sum git-ptx-patches magic"
> +function _gitsha1sum() {
> +	local sha1=$(git rev-parse HEAD)
> +	echo "# $sha1 git-ptx-patches magic"
> +}
> +
> +update_series_sha256() {
> +	local makefile="$(readlink -f .ptxdist/rule.make)"
> +	source .ptxdist/rule.env
> +	set -- $(sha256sum .ptxdist/series)
> +	local sha256="${1}"
> +
> +	local count=$(grep "^${PKG}_SERIES_SHA256[	 ]*:=" "${makefile}" 2> /dev/null | wc -l)
> +	if [ "${count}" -gt 1 ]; then
> +		echo "Error: Could not update patch series sha256sum for '${pkg_label}': ${PKG}_SERIES_SHA256 found ${count} times in '${makefile}'."
> +		exit 1
> +	fi
> +	local current_sha256=$(grep "^${PKG}_SERIES_SHA256[	 ]*:= " "${makefile}" |cut -f2 -d=|xargs)
> +	if [ "${current_sha256}" != "${sha256}" ]; then
> +		sed -i "s/^\(\<${PKG}_SERIES_SHA256[	]*:=\) *[a-f0-9]*\$/\1 ${sha256}/" "${makefile}"
> +		if ! grep -q "${sha256}\$" "${makefile}"; then
> +			echo "Warning: ${PKG}_SERIES_SHA256 is missing from '${makefile}'."
> +		else
> +			echo "New patch series checksum for '${pkg_label}': ${sha256} in '${makefile}'."
> +		fi
> +	fi
>  }
>  
>  if [ ! -L .ptxdist/patches ]; then
> @@ -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 | _gitsha1sum >> .ptxdist/series
> +
> +update_series_sha256
>  
>  # 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

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

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

[Michael Olbrich]

On Mon, Aug 27, 2018 at 05:59:25PM +0200, Michael Olbrich wrote:
> On Fri, Aug 03, 2018 at 04:41:56AM -0400, jon@ringle.org wrote:
> > 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_SHA256 :=`
> > 2) extract the package and from the packages src dir do `git ptx-patches`
> >    This will populate the rule makefile _SERIES_SHA256 value
> > 3) Anytime you make a patch change to a package and do `git ptx-patches`
> >    the series file gets updated with the `git rev-parse HEAD` value which
> >    causes the series file to have a new sha256 value and therefore 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 cfghash
> >    in the filename
> 
> This is rather limited. I've been thinking about a more general solution:
> 
> I'd like to hash all patches + series during extract and create a stamp
> file (like the one for the rules and options) during 'extract'. And save
> some data to recreate the hash:
> - list of all possible patch dirs (in case of new patches in the BSP)
> - relative paths to patches and series.
> 
> I think we can recreate the hashes for all enabled packages with this with
> reasonable startup overhead.
> 
> Then depend on the new stamp with the extract stage. This will need some
> extra checks, so we don't loose local changes:
> - make it optional
> - abort before deleting the source tree if there is a .git
> Stuff like this.
> 
> And of course add the hash to the config/rules hash for *-dev.tar.gz.

... and this last step won't work, because we cannot calculate the correct
hash without running 'extract' first :-/. I need to think about this some
more.

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