Patchwork UBUNTU: use right enforce for the right branch

login
register
mail settings
Submitter Bryan Wu
Date March 12, 2012, 10:53 a.m.
Message ID <1331549605-13733-1-git-send-email-bryan.wu@canonical.com>
Download mbox | patch
Permalink /patch/146057/
State New
Headers show

Comments

Bryan Wu - March 12, 2012, 10:53 a.m.
When running config-check, it always open debian.master/enforce file
instead of debian.ti-omap4/enforce even on ti-omap4 branch.

This patch fix this issue and tested on Marvell armadaxp branch.

Signed-off-by: Bryan Wu <bryan.wu@canonical.com>
---
 debian/rules.d/4-checks.mk       |    2 +-
 debian/scripts/misc/kernelconfig |    2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)
Leann Ogasawara - March 12, 2012, 6 p.m.
Applied to Precise master-next.

Andy and Tim, with this change $sharedconfdir there doesn't appear to be
used anywhere else.  Am I missing something or can we remove it
completely?

Thanks,
Leann

On Mon, 2012-03-12 at 18:53 +0800, Bryan Wu wrote:
> When running config-check, it always open debian.master/enforce file
> instead of debian.ti-omap4/enforce even on ti-omap4 branch.
> 
> This patch fix this issue and tested on Marvell armadaxp branch.
> 
> Signed-off-by: Bryan Wu <bryan.wu@canonical.com>
> ---
>  debian/rules.d/4-checks.mk       |    2 +-
>  debian/scripts/misc/kernelconfig |    2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/debian/rules.d/4-checks.mk b/debian/rules.d/4-checks.mk
> index c4df2fa..1a7aade 100644
> --- a/debian/rules.d/4-checks.mk
> +++ b/debian/rules.d/4-checks.mk
> @@ -20,5 +20,5 @@ checks-%: module-check-% abi-check-%
>  # Check the config against the known options list.
>  config-prepare-check-%: $(stampdir)/stamp-prepare-tree-%
>  	@perl -f $(DROOT)/scripts/config-check \
> -		$(builddir)/build-$*/.config "$(arch)" "$*" "$(sharedconfdir)" "$(skipconfig)"
> +		$(builddir)/build-$*/.config "$(arch)" "$*" "$(commonconfdir)" "$(skipconfig)"
>  
> diff --git a/debian/scripts/misc/kernelconfig b/debian/scripts/misc/kernelconfig
> index 8812b61..ea4a60e 100755
> --- a/debian/scripts/misc/kernelconfig
> +++ b/debian/scripts/misc/kernelconfig
> @@ -157,7 +157,7 @@ for arch in $archs; do
>  		flavour="${config##*.}"
>  		if [ -f $archconfdir/$config ]; then
>  			fullconf="$tmpdir/CONFIGS/$arch-$config"
> -			"$bindir/../config-check" "$fullconf" "$arch" "$flavour" "$sharedconfdir" "0" || let "fail=$fail+1"
> +			"$bindir/../config-check" "$fullconf" "$arch" "$flavour" "$confdir" "0" || let "fail=$fail+1"
>  		fi
>  	done
>  done
> -- 
> 1.7.9.1
> 
>
Andy Whitcroft - March 12, 2012, 6:02 p.m.
On Mon, Mar 12, 2012 at 06:53:25PM +0800, Bryan Wu wrote:
> When running config-check, it always open debian.master/enforce file
> instead of debian.ti-omap4/enforce even on ti-omap4 branch.
> 
> This patch fix this issue and tested on Marvell armadaxp branch.
> 
> Signed-off-by: Bryan Wu <bryan.wu@canonical.com>
> ---
>  debian/rules.d/4-checks.mk       |    2 +-
>  debian/scripts/misc/kernelconfig |    2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/debian/rules.d/4-checks.mk b/debian/rules.d/4-checks.mk
> index c4df2fa..1a7aade 100644
> --- a/debian/rules.d/4-checks.mk
> +++ b/debian/rules.d/4-checks.mk
> @@ -20,5 +20,5 @@ checks-%: module-check-% abi-check-%
>  # Check the config against the known options list.
>  config-prepare-check-%: $(stampdir)/stamp-prepare-tree-%
>  	@perl -f $(DROOT)/scripts/config-check \
> -		$(builddir)/build-$*/.config "$(arch)" "$*" "$(sharedconfdir)" "$(skipconfig)"
> +		$(builddir)/build-$*/.config "$(arch)" "$*" "$(commonconfdir)" "$(skipconfig)"
>  
> diff --git a/debian/scripts/misc/kernelconfig b/debian/scripts/misc/kernelconfig
> index 8812b61..ea4a60e 100755
> --- a/debian/scripts/misc/kernelconfig
> +++ b/debian/scripts/misc/kernelconfig
> @@ -157,7 +157,7 @@ for arch in $archs; do
>  		flavour="${config##*.}"
>  		if [ -f $archconfdir/$config ]; then
>  			fullconf="$tmpdir/CONFIGS/$arch-$config"
> -			"$bindir/../config-check" "$fullconf" "$arch" "$flavour" "$sharedconfdir" "0" || let "fail=$fail+1"
> +			"$bindir/../config-check" "$fullconf" "$arch" "$flavour" "$confdir" "0" || let "fail=$fail+1"
>  		fi
>  	done
>  done

That actually was a deliberate choice to use the master copy.  As we
want the enforcer checks to be consistant across all of the branches.
It is possible to have branch specific rules in the master branch as the
flavour and architecture names are both exposed.

It is possible you have a compelling reason to have different rules,
though I would expect that the things in the enforcer are mostly
'ubuntu' things and required for good compatibility with Ubuntu
userspace.

Now of course, this may not be true for every branch (though it has been
so far) and we may want to have some overrides per branch for those sorts
of things.  So perhaps you could give us an example of something which
prevents the rules being common.

-apw
Leann Ogasawara - March 12, 2012, 6:10 p.m.
On Mon, 2012-03-12 at 11:00 -0700, Leann Ogasawara wrote:
> Applied to Precise master-next.
> 
> Andy and Tim, with this change $sharedconfdir there doesn't appear to be
> used anywhere else.  Am I missing something or can we remove it
> completely?

Just saw Andy's follow up email.  Am going to drop this from master-next
for now based on his response.

> Thanks,
> Leann
> 
> On Mon, 2012-03-12 at 18:53 +0800, Bryan Wu wrote:
> > When running config-check, it always open debian.master/enforce file
> > instead of debian.ti-omap4/enforce even on ti-omap4 branch.
> > 
> > This patch fix this issue and tested on Marvell armadaxp branch.
> > 
> > Signed-off-by: Bryan Wu <bryan.wu@canonical.com>
> > ---
> >  debian/rules.d/4-checks.mk       |    2 +-
> >  debian/scripts/misc/kernelconfig |    2 +-
> >  2 files changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/debian/rules.d/4-checks.mk b/debian/rules.d/4-checks.mk
> > index c4df2fa..1a7aade 100644
> > --- a/debian/rules.d/4-checks.mk
> > +++ b/debian/rules.d/4-checks.mk
> > @@ -20,5 +20,5 @@ checks-%: module-check-% abi-check-%
> >  # Check the config against the known options list.
> >  config-prepare-check-%: $(stampdir)/stamp-prepare-tree-%
> >  	@perl -f $(DROOT)/scripts/config-check \
> > -		$(builddir)/build-$*/.config "$(arch)" "$*" "$(sharedconfdir)" "$(skipconfig)"
> > +		$(builddir)/build-$*/.config "$(arch)" "$*" "$(commonconfdir)" "$(skipconfig)"
> >  
> > diff --git a/debian/scripts/misc/kernelconfig b/debian/scripts/misc/kernelconfig
> > index 8812b61..ea4a60e 100755
> > --- a/debian/scripts/misc/kernelconfig
> > +++ b/debian/scripts/misc/kernelconfig
> > @@ -157,7 +157,7 @@ for arch in $archs; do
> >  		flavour="${config##*.}"
> >  		if [ -f $archconfdir/$config ]; then
> >  			fullconf="$tmpdir/CONFIGS/$arch-$config"
> > -			"$bindir/../config-check" "$fullconf" "$arch" "$flavour" "$sharedconfdir" "0" || let "fail=$fail+1"
> > +			"$bindir/../config-check" "$fullconf" "$arch" "$flavour" "$confdir" "0" || let "fail=$fail+1"
> >  		fi
> >  	done
> >  done
> > -- 
> > 1.7.9.1
> > 
> > 
> 
> 
>
Bryan Wu - March 13, 2012, 2:12 a.m.
On Tue, Mar 13, 2012 at 2:02 AM, Andy Whitcroft <apw@canonical.com> wrote:
> On Mon, Mar 12, 2012 at 06:53:25PM +0800, Bryan Wu wrote:
>> When running config-check, it always open debian.master/enforce file
>> instead of debian.ti-omap4/enforce even on ti-omap4 branch.
>>
>> This patch fix this issue and tested on Marvell armadaxp branch.
>>
>> Signed-off-by: Bryan Wu <bryan.wu@canonical.com>
>> ---
>>  debian/rules.d/4-checks.mk       |    2 +-
>>  debian/scripts/misc/kernelconfig |    2 +-
>>  2 files changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/debian/rules.d/4-checks.mk b/debian/rules.d/4-checks.mk
>> index c4df2fa..1a7aade 100644
>> --- a/debian/rules.d/4-checks.mk
>> +++ b/debian/rules.d/4-checks.mk
>> @@ -20,5 +20,5 @@ checks-%: module-check-% abi-check-%
>>  # Check the config against the known options list.
>>  config-prepare-check-%: $(stampdir)/stamp-prepare-tree-%
>>       @perl -f $(DROOT)/scripts/config-check \
>> -             $(builddir)/build-$*/.config "$(arch)" "$*" "$(sharedconfdir)" "$(skipconfig)"
>> +             $(builddir)/build-$*/.config "$(arch)" "$*" "$(commonconfdir)" "$(skipconfig)"
>>
>> diff --git a/debian/scripts/misc/kernelconfig b/debian/scripts/misc/kernelconfig
>> index 8812b61..ea4a60e 100755
>> --- a/debian/scripts/misc/kernelconfig
>> +++ b/debian/scripts/misc/kernelconfig
>> @@ -157,7 +157,7 @@ for arch in $archs; do
>>               flavour="${config##*.}"
>>               if [ -f $archconfdir/$config ]; then
>>                       fullconf="$tmpdir/CONFIGS/$arch-$config"
>> -                     "$bindir/../config-check" "$fullconf" "$arch" "$flavour" "$sharedconfdir" "0" || let "fail=$fail+1"
>> +                     "$bindir/../config-check" "$fullconf" "$arch" "$flavour" "$confdir" "0" || let "fail=$fail+1"
>>               fi
>>       done
>>  done
>
> That actually was a deliberate choice to use the master copy.  As we
> want the enforcer checks to be consistant across all of the branches.
> It is possible to have branch specific rules in the master branch as the
> flavour and architecture names are both exposed.
>

Right, actually when I was building kernel package, I found it always
use debian.master/config/enforce instead of
debain.ti-omap4/config/enforce. So thought that's a issue maybe and
make a patch to fix this straight forward then.

I agree we can just maintain one enforce file in debian.master which
can be shared by other branches, because we already have armel/armhf
setting in that enforce file.

> It is possible you have a compelling reason to have different rules,
> though I would expect that the things in the enforcer are mostly
> 'ubuntu' things and required for good compatibility with Ubuntu
> userspace.
>
> Now of course, this may not be true for every branch (though it has been
> so far) and we may want to have some overrides per branch for those sorts
> of things.  So perhaps you could give us an example of something which
> prevents the rules being common.
>

No problem, if we find some change of enforce we need add, we can post
patch against debian.master.

BTW, so we can delete debian.ti-omap4/config/enforce, which is useless
in ti-omap4 branch currently.

Thanks,

Patch

diff --git a/debian/rules.d/4-checks.mk b/debian/rules.d/4-checks.mk
index c4df2fa..1a7aade 100644
--- a/debian/rules.d/4-checks.mk
+++ b/debian/rules.d/4-checks.mk
@@ -20,5 +20,5 @@  checks-%: module-check-% abi-check-%
 # Check the config against the known options list.
 config-prepare-check-%: $(stampdir)/stamp-prepare-tree-%
 	@perl -f $(DROOT)/scripts/config-check \
-		$(builddir)/build-$*/.config "$(arch)" "$*" "$(sharedconfdir)" "$(skipconfig)"
+		$(builddir)/build-$*/.config "$(arch)" "$*" "$(commonconfdir)" "$(skipconfig)"
 
diff --git a/debian/scripts/misc/kernelconfig b/debian/scripts/misc/kernelconfig
index 8812b61..ea4a60e 100755
--- a/debian/scripts/misc/kernelconfig
+++ b/debian/scripts/misc/kernelconfig
@@ -157,7 +157,7 @@  for arch in $archs; do
 		flavour="${config##*.}"
 		if [ -f $archconfdir/$config ]; then
 			fullconf="$tmpdir/CONFIGS/$arch-$config"
-			"$bindir/../config-check" "$fullconf" "$arch" "$flavour" "$sharedconfdir" "0" || let "fail=$fail+1"
+			"$bindir/../config-check" "$fullconf" "$arch" "$flavour" "$confdir" "0" || let "fail=$fail+1"
 		fi
 	done
 done