| Message ID | 1331549605-13733-1-git-send-email-bryan.wu@canonical.com |
|---|---|
| State | New |
| Headers | show |
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 > >
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
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 > > > > > > >
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,
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
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(-)