[PATCH V2] add function to check for git config user

[PATCH V2] add function to check for git config user

[Stephano Cetola]

Changed since V1:
forgot to add Yocto bug #, also cleaned up commit message

Stephano Cetola (1):
  sanity: add function to check for git config user

 meta/classes/buildhistory.bbclass | 13 ++++---------
 meta/classes/kernel-yocto.bbclass |  2 ++
 meta/classes/sanity.bbclass       | 15 +++++++++++++++
 3 files changed, 21 insertions(+), 9 deletions(-)

-- 
2.10.0

[PATCH V2] sanity: add function to check for git config user

[Stephano Cetola]

If attempting to patch a git repo without a proper git config setup,
an error will occur saying user.name/user.email are needed by git
am/apply. After some code was removed from kernel-yocto, it was
simple enough to reproduce this error by creating a kernel patch and
using a container to build.

This patch abstracts out functionality that existed in buildhistory
for use in other classes. It also adds a call to this functionality
to the kernel-yocto class.

Fixes [YOCTO #10346]

introduced in
OE-core revision 0f698dfd1c8bbc0d53ae7977e26685a7a3df52a3

Signed-off-by: Stephano Cetola <stephano.cetola@linux.intel.com>
---
 meta/classes/buildhistory.bbclass | 13 ++++---------
 meta/classes/kernel-yocto.bbclass |  2 ++
 meta/classes/sanity.bbclass       | 15 +++++++++++++++
 3 files changed, 21 insertions(+), 9 deletions(-)

diff --git a/meta/classes/buildhistory.bbclass b/meta/classes/buildhistory.bbclass
index 6e5de0e..ed79029 100644
--- a/meta/classes/buildhistory.bbclass
+++ b/meta/classes/buildhistory.bbclass
@@ -688,6 +688,7 @@ END
 	rm $commitmsgfile
 }
 
+inherit sanity
 buildhistory_commit() {
 	if [ ! -d ${BUILDHISTORY_DIR} ] ; then
 		# Code above that creates this dir never executed, so there can't be anything to commit
@@ -708,15 +709,9 @@ END
 			git tag -f build-minus-2 build-minus-1 > /dev/null 2>&1 || true
 			git tag -f build-minus-1 > /dev/null 2>&1 || true
 		fi
-		# If the user hasn't set up their name/email, set some defaults
-		# just for this repo (otherwise the commit will fail with older
-		# versions of git)
-		if ! git config user.email > /dev/null ; then
-			git config --local user.email "buildhistory@${DISTRO}"
-		fi
-		if ! git config user.name > /dev/null ; then
-			git config --local user.name "buildhistory"
-		fi
+
+		check_git_config "buildhistory"
+
 		# Check if there are new/changed files to commit (other than metadata-revs)
 		repostatus=`git status --porcelain | grep -v " metadata-revs$"`
 		HOSTNAME=`hostname 2>/dev/null || echo unknown`
diff --git a/meta/classes/kernel-yocto.bbclass b/meta/classes/kernel-yocto.bbclass
index 53659f2..035edeb 100644
--- a/meta/classes/kernel-yocto.bbclass
+++ b/meta/classes/kernel-yocto.bbclass
@@ -156,9 +156,11 @@ do_kernel_metadata() {
 	fi
 }
 
+inherit sanity
 do_patch() {
 	cd ${S}
 
+	check_git_config "kernel-yocto"
 	meta_dir=$(kgit --meta)
 	(cd ${meta_dir}; ln -sf patch.queue series)
 	if [ -f "${meta_dir}/series" ]; then
diff --git a/meta/classes/sanity.bbclass b/meta/classes/sanity.bbclass
index 7682ffb..40033ab 100644
--- a/meta/classes/sanity.bbclass
+++ b/meta/classes/sanity.bbclass
@@ -1023,3 +1023,18 @@ python check_sanity_eventhandler() {
 
     return
 }
+
+# If the user hasn't set up their name/email, set some defaults
+check_git_config() {
+	if [ -z "$1" ]; then
+		name="OE"
+	else
+		name="$1"
+	fi
+	if ! git config user.email > /dev/null ; then
+        	git config --local user.email "${name}@${DISTRO}"
+	fi
+	if ! git config user.name > /dev/null ; then
+        	git config --local user.name "${name}"
+	fi
+}
-- 
2.10.0

Re: [PATCH V2] sanity: add function to check for git config user

[Burton, Ross]

[-- Attachment #1: Type: text/plain, Size: 685 bytes --]

On 3 October 2016 at 01:43, Stephano Cetola <stephano.cetola@linux.intel.com
> wrote:

> +# If the user hasn't set up their name/email, set some defaults
> +check_git_config() {
> +       if [ -z "$1" ]; then
> +               name="OE"
> +       else
> +               name="$1"
> +       fi
> +       if ! git config user.email > /dev/null ; then
> +               git config --local user.email "${name}@${DISTRO}"
> +       fi
> +       if ! git config user.name > /dev/null ; then
> +               git config --local user.name "${name}"
> +       fi
> +}
>

Instead of embedding this into sanity.bbclass, could it just go into
meta/lib/oe/ somewhere?

Ross

[-- Attachment #2: Type: text/html, Size: 1369 bytes --]

Re: [PATCH V2] sanity: add function to check for git config user

[Stephano Cetola]

On 10/03, Burton, Ross wrote:
> On 3 October 2016 at 01:43, Stephano Cetola <stephano.cetola@linux.intel.com
> > wrote:
> 
> > +# If the user hasn't set up their name/email, set some defaults
> > +check_git_config() {
> > +       if [ -z "$1" ]; then
> > +               name="OE"
> > +       else
> > +               name="$1"
> > +       fi
> > +       if ! git config user.email > /dev/null ; then
> > +               git config --local user.email "${name}@${DISTRO}"
> > +       fi
> > +       if ! git config user.name > /dev/null ; then
> > +               git config --local user.name "${name}"
> > +       fi
> > +}
> >
> 
> Instead of embedding this into sanity.bbclass, could it just go into
> meta/lib/oe/ somewhere?
> 
> Ross


Yeah, I saw /meta/lib/oe/patch when Paul mentioned it. It looks like
things should be moving in this direction log-term anyhow. I could
move this to patch.py for now, and then submit a bug to move
functionality as well, if that's the direction we'd want to go.

Cheers,
Stephano

Re: [PATCH V2] sanity: add function to check for git config user

[Richard Purdie]

On Sun, 2016-10-02 at 17:43 -0700, Stephano Cetola wrote:
> If attempting to patch a git repo without a proper git config setup,
> an error will occur saying user.name/user.email are needed by git
> am/apply. After some code was removed from kernel-yocto, it was
> simple enough to reproduce this error by creating a kernel patch and
> using a container to build.
> 
> This patch abstracts out functionality that existed in buildhistory
> for use in other classes. It also adds a call to this functionality
> to the kernel-yocto class.
> 
> Fixes [YOCTO #10346]
> 
> introduced in
> OE-core revision 0f698dfd1c8bbc0d53ae7977e26685a7a3df52a3
> 
> Signed-off-by: Stephano Cetola <stephano.cetola@linux.intel.com>
> ---
>  meta/classes/buildhistory.bbclass | 13 ++++---------
>  meta/classes/kernel-yocto.bbclass |  2 ++
>  meta/classes/sanity.bbclass       | 15 +++++++++++++++
>  3 files changed, 21 insertions(+), 9 deletions(-)
> 
> diff --git a/meta/classes/buildhistory.bbclass
> b/meta/classes/buildhistory.bbclass
> index 6e5de0e..ed79029 100644
> --- a/meta/classes/buildhistory.bbclass
> +++ b/meta/classes/buildhistory.bbclass
> @@ -688,6 +688,7 @@ END
>  	rm $commitmsgfile
>  }
>  
> +inherit sanity
>  buildhistory_commit() {
>  	if [ ! -d ${BUILDHISTORY_DIR} ] ; then
>  		# Code above that creates this dir never executed,
> so there can't be anything to commit
> @@ -708,15 +709,9 @@ END
>  			git tag -f build-minus-2 build-minus-1 >
> /dev/null 2>&1 || true
>  			git tag -f build-minus-1 > /dev/null 2>&1 ||
> true
>  		fi
> -		# If the user hasn't set up their name/email, set
> some defaults
> -		# just for this repo (otherwise the commit will fail
> with older
> -		# versions of git)
> -		if ! git config user.email > /dev/null ; then
> -			git config --local user.email
> "buildhistory@${DISTRO}"
> -		fi
> -		if ! git config user.name > /dev/null ; then
> -			git config --local user.name "buildhistory"
> -		fi
> +
> +		check_git_config "buildhistory"
> +
>  		# Check if there are new/changed files to commit
> (other than metadata-revs)
>  		repostatus=`git status --porcelain | grep -v "
> metadata-revs$"`
>  		HOSTNAME=`hostname 2>/dev/null || echo unknown`
> diff --git a/meta/classes/kernel-yocto.bbclass b/meta/classes/kernel-
> yocto.bbclass
> index 53659f2..035edeb 100644
> --- a/meta/classes/kernel-yocto.bbclass
> +++ b/meta/classes/kernel-yocto.bbclass
> @@ -156,9 +156,11 @@ do_kernel_metadata() {
>  	fi
>  }
>  
> +inherit sanity
>  do_patch() {
>  	cd ${S}
>  
> +	check_git_config "kernel-yocto"
>  	meta_dir=$(kgit --meta)
>  	(cd ${meta_dir}; ln -sf patch.queue series)
>  	if [ -f "${meta_dir}/series" ]; then
> diff --git a/meta/classes/sanity.bbclass
> b/meta/classes/sanity.bbclass
> index 7682ffb..40033ab 100644
> --- a/meta/classes/sanity.bbclass
> +++ b/meta/classes/sanity.bbclass
> @@ -1023,3 +1023,18 @@ python check_sanity_eventhandler() {
>  
>      return
>  }
> +
> +# If the user hasn't set up their name/email, set some defaults
> +check_git_config() {
> +	if [ -z "$1" ]; then
> +		name="OE"
> +	else
> +		name="$1"
> +	fi
> +	if ! git config user.email > /dev/null ; then
> +        	git config --local user.email "${name}@${DISTRO}"
> +	fi
> +	if ! git config user.name > /dev/null ; then
> +        	git config --local user.name "${name}"
> +	fi
> +}

I worry you're causing a load of the classes to end up cross linked
here which is bad from a usability standpoint. Nobody can easily
disable the sanity checks after this patch.

If we're keeping this as a shell function this function would probably
go to utils.bbclass. Adding something to lib/oe is probably ultimately
the way to go but I appreciate its harder to write the above in a
python lib, so perhaps utils.bbclass is good enough for now? We can
assume utils is inherited globally.

Cheers,

Richard