Linux Archive

Linux Archive (http://www.linux-archive.org/)
-   Ubuntu Kernel Team (http://www.linux-archive.org/ubuntu-kernel-team/)
-   -   UBUNTU: 'fdr editconfig' modification. Easily skip over unwanted menuconfigs. (http://www.linux-archive.org/ubuntu-kernel-team/378730-ubuntu-fdr-editconfig-modification-easily-skip-over-unwanted-menuconfigs.html)

Chase Douglas 05-30-2010 03:09 PM

UBUNTU: 'fdr editconfig' modification. Easily skip over unwanted menuconfigs.
 
On Sun, 2010-05-30 at 10:53 +0100, Lee Jones wrote:
> All,
>
> It is now easier to edit 'per-flavour' configuration files. The old
> system forced the developer to enter a menuconfig system for each
> flavour until eventually landing in the required one. After following
> this process numerous times it can easily become tedious. Now the
> developer is asked whether they want to enter each menuconfig in turn,
> rapidly speeding up the process.

A few notes:

I don't think we would generally consider changes like this to a
released kernel, so this should probably be applied to maverick. If we
do want to put it in lucid, it makes sense to first put it in maverick
anyways. As a side note, because we maintain many branches of releases,
it helps to prepend the email subject line with "[MAVERICK]" so everyone
knows what the patch is for.

There's a lot of whitespace changes here. If you meant to introduce
formatting changes, it's best to do so as a separate patch. If not, then
your editor may be munging things. The inlined diff was munged as well.
If you're using evolution, check that when you insert the text the style
dropdown box is changed from Normal to Preformatted.

I took your patch, applied it to ubuntu-maverick, then did a "git diff
-w HEAD^" to take a look at what really changed:

diff --git a/debian/scripts/misc/kernelconfig b/debian/scripts/misc/kernelconfig
index 71c0f5e..7884bdc 100755
--- a/debian/scripts/misc/kernelconfig
+++ b/debian/scripts/misc/kernelconfig
@@ -51,9 +51,6 @@ for arch in $archs; do
*) kernarch="$arch" ;;
esac

- echo ""
- echo "***************************************"
- echo "* Processing $arch ($kernarch) ... "
archconfdir=$confdir/$arch
flavourconfigs=$(cd $archconfdir && ls config.flavour.*)

@@ -96,9 +93,21 @@ for arch in $archs; do
make O=`pwd`/build ARCH=$kernarch oldconfig ;;
editconfig)
# Interactively edit config parameters
- echo " * Run menuconfig on $arch/$config... Press a key."
- read
- make O=`pwd`/build ARCH=$kernarch menuconfig ;;
+ while : ; do
+ echo -n "Do you want to edit config: $arch/$config? [Y/n] "
+ read choice
+
+ case "$choice" in
+ y* | Y* | "" )
+ make O=`pwd`/build ARCH=$kernarch menuconfig
+ break ;;
+ n* | N* )
+ break ;;
+ *)
+ echo "Entry not valid"
+ esac
+ done
+ ;;
*) # Bad!
exit 1 ;;
esac

Now I can better tell what's going on :). This seems good to me. The
previous behavior is maintained (press enter to edit the config file),
while giving the user the option to skip editing if they know nothing
should change. If you can resend a patch like this with only the salient
changes I will ACK it. When you do, please check that your indentation
style matches the style used throughout the rules files. I'm not sure
the above is conforming.

Thanks,

-- Chase


--
kernel-team mailing list
kernel-team@lists.ubuntu.com
https://lists.ubuntu.com/mailman/listinfo/kernel-team

Lee Jones 06-01-2010 09:46 AM

UBUNTU: 'fdr editconfig' modification. Easily skip over unwanted menuconfigs.
 
> A few notes:
>
> I don't think we would generally consider changes like this to a
> released kernel, so this should probably be applied to maverick. If we
> do want to put it in lucid, it makes sense to first put it in maverick
> anyways. As a side note, because we maintain many branches of releases,
> it helps to prepend the email subject line with "[MAVERICK]" so everyone
> knows what the patch is for.
>



I have seen the "[LUCID]" and "[MAVERICK]" tags and did contemplate
using them, but I had no idea where this patch should go, so
intentionally omitted them.



> There's a lot of whitespace changes here. If you meant to introduce
> formatting changes, it's best to do so as a separate patch.



Noted.



> Now I can better tell what's going on :). This seems good to me. The
> previous behavior is maintained (press enter to edit the config file),
> while giving the user the option to skip editing if they know nothing
> should change.



Thank you. That is the behavior I intended.



> If you can resend a patch like this with only the salient
> changes I will ACK it.



How's this:

The following changes since commit f0819aaf4948e34a44d9d685615ddee74271cd70:
Chase Douglas (1):
UBUNTU: enforce CONFIG_TMPFS_POSIX_ACL=y

are available in the git repository at:

git://kernel.ubuntu.com/lag/ubuntu-lucid.git editconfig

Lee Jones (1):
UBUNTU: 'fdr editconfig' modification. Easily skip over unwanted menuconfigs.

debian/scripts/misc/kernelconfig | 21 +++++++++++++++------
1 files changed, 15 insertions(+), 6 deletions(-)


diff --git a/debian/scripts/misc/kernelconfig b/debian/scripts/misc/kernelconfig
index 71c0f5e..3181978 100755 (executable)
--- a/debian/scripts/misc/kernelconfig
+++ b/debian/scripts/misc/kernelconfig
@@ -51,9 +51,6 @@ for arch in $archs; do
*) kernarch="$arch" ;;
esac

- echo ""
- echo "***************************************"
- echo "* Processing $arch ($kernarch) ... "
archconfdir=$confdir/$arch
flavourconfigs=$(cd $archconfdir && ls config.flavour.*)

@@ -96,9 +93,21 @@ for arch in $archs; do
make O=`pwd`/build ARCH=$kernarch oldconfig ;;
editconfig)
# Interactively edit config parameters
- echo " * Run menuconfig on $arch/$config... Press a key."
- read
- make O=`pwd`/build ARCH=$kernarch menuconfig ;;
+ while : ; do
+ echo -n "Do you want to edit config: $arch/$config? [Y/n] "
+ read choice
+
+ case "$choice" in
+ y* | Y* | "" )
+ make O=`pwd`/build ARCH=$kernarch menuconfig
+ break ;;
+ n* | N* )
+ break ;;
+ *)
+ echo "Entry not valid"
+ esac
+ done
+ ;;
*) # Bad!
exit 1 ;;
esac



> hen you do, please check that your indentation
> style matches the style used throughout the rules files. I'm not sure
> the above is conforming.
>



This is an odd one, as the original file did not conform, even to itself.

For instance; the two case statements:

One indents and uses quotes around the variable:
--------------------------------------------------------------
<snip>
case "$arch" in
amd64) kernarch="x86_64" ;;
</snip>
--------------------------------------------------------------

The other does neither:
--------------------------------------------------------------
<snip>
case $config in
*)
</snip>
--------------------------------------------------------------

The remainder of the kernel doesn't indent, but does use quotes and
Emacs tells me it should be indented.

Your call?

Kind regards,
Lee




--
kernel-team mailing list
kernel-team@lists.ubuntu.com
https://lists.ubuntu.com/mailman/listinfo/kernel-team

Lee Jones 06-01-2010 09:55 AM

UBUNTU: 'fdr editconfig' modification. Easily skip over unwanted menuconfigs.
 
> This is an odd one, as the original file did not conform, even to itself.
>
> For instance; the two case statements:
>
> One indents and uses quotes around the variable:
> --------------------------------------------------------------
> <snip>
> case "$arch" in
> amd64) kernarch="x86_64" ;;
> </snip>
> --------------------------------------------------------------
>
> The other does neither:
> --------------------------------------------------------------
> <snip>
> case $config in
> *)
> </snip>
> --------------------------------------------------------------
>
> The remainder of the kernel doesn't indent, but does use quotes and
> Emacs tells me it should be indented.
>
> Your call?
>
>

Oh, and the BASH manual 'does' indent.

BASH & Emacs say "do indent".
Linux kernel says "don't indent".

I'll not submit a format change request until I have a definitive answer
from you guys.

Kind regards,
Lee

--
kernel-team mailing list
kernel-team@lists.ubuntu.com
https://lists.ubuntu.com/mailman/listinfo/kernel-team

Chase Douglas 06-01-2010 12:28 PM

UBUNTU: 'fdr editconfig' modification. Easily skip over unwanted menuconfigs.
 
On Tue, 2010-06-01 at 10:46 +0100, Lee Jones wrote:
> I have seen the "[LUCID]" and "[MAVERICK]" tags and did contemplate
> using them, but I had no idea where this patch should go, so
> intentionally omitted them.

Very little goes directly into an already released kernel. Usually, bug
fixes are sent to stable@kernel.org, then trickle back down to us. If
the change fixes a really important issue, then we may take it in before
that, but this is the exception rather than the rule.

For development kernels, things are usually flipped. We normally apply
changes as we submit them to the stable queue upstream.

In this instance, where you're resubmitting a patch, I would send it as
a new email (not a reply) with something like the following for a
subject:

[MAVERIC][PATCH v3] UBUNTU: 'fdr editconfig' modification. Easily skip
over unwanted menuconfigs.

Generally, the patch numbering is omitted in a single patch instance.

> How's this:
>
> The following changes since commit f0819aaf4948e34a44d9d685615ddee74271cd70:
> Chase Douglas (1):
> UBUNTU: enforce CONFIG_TMPFS_POSIX_ACL=y
>
> are available in the git repository at:
>
> git://kernel.ubuntu.com/lag/ubuntu-lucid.git editconfig

This needs to be applied to the maverick source tree instead.

>
> Lee Jones (1):
> UBUNTU: 'fdr editconfig' modification. Easily skip over unwanted menuconfigs.
>
> debian/scripts/misc/kernelconfig | 21 +++++++++++++++------
> 1 files changed, 15 insertions(+), 6 deletions(-)
>
>
> diff --git a/debian/scripts/misc/kernelconfig b/debian/scripts/misc/kernelconfig
> index 71c0f5e..3181978 100755 (executable)
> --- a/debian/scripts/misc/kernelconfig
> +++ b/debian/scripts/misc/kernelconfig
> @@ -51,9 +51,6 @@ for arch in $archs; do
> *) kernarch="$arch" ;;
> esac
>
> - echo ""
> - echo "***************************************"
> - echo "* Processing $arch ($kernarch) ... "
> archconfdir=$confdir/$arch
> flavourconfigs=$(cd $archconfdir && ls config.flavour.*)
>
> @@ -96,9 +93,21 @@ for arch in $archs; do
> make O=`pwd`/build ARCH=$kernarch oldconfig ;;
> editconfig)
> # Interactively edit config parameters
> - echo " * Run menuconfig on $arch/$config... Press a key."
> - read
> - make O=`pwd`/build ARCH=$kernarch menuconfig ;;
> + while : ; do
> + echo -n "Do you want to edit config: $arch/$config? [Y/n] "
> + read choice
> +
> + case "$choice" in
> + y* | Y* | "" )
> + make O=`pwd`/build ARCH=$kernarch menuconfig
> + break ;;
> + n* | N* )
> + break ;;
> + *)
> + echo "Entry not valid"
> + esac
> + done
> + ;;
> *) # Bad!
> exit 1 ;;
> esac
>
> > > hen you do, please check that your indentation
> > > style matches the style used throughout the rules files. I'm not sure
> > > the above is conforming.
> > >
> >
> This is an odd one, as the original file did not conform, even to itself.
>
> For instance; the two case statements:
>
> One indents and uses quotes around the variable:
> --------------------------------------------------------------
> <snip>
> case "$arch" in
> amd64) kernarch="x86_64" ;;
> </snip>
> --------------------------------------------------------------
>
> The other does neither:
> --------------------------------------------------------------
> <snip>
> case $config in
> *)
> </snip>
> --------------------------------------------------------------
>
> The remainder of the kernel doesn't indent, but does use quotes and
> Emacs tells me it should be indented.

It's no fun when things aren't standardized :). I would just try to
reproduce the same style as the code around your changes. In this
instance, the only thing that stands out to me is the case statement
should be like:

case "$choice" in
y* | Y* | "" )
make O=`pwd`/build ARCH=$kernarch menuconfig
break ;;

This wouldn't be my first choice, but it's better to keep things
consistent so it's readable. Other than that, I think everything is
good.

-- Chase


--
kernel-team mailing list
kernel-team@lists.ubuntu.com
https://lists.ubuntu.com/mailman/listinfo/kernel-team

Stefan Bader 06-01-2010 01:26 PM

UBUNTU: 'fdr editconfig' modification. Easily skip over unwanted menuconfigs.
 
On 06/01/2010 02:28 PM, Chase Douglas wrote:
> On Tue, 2010-06-01 at 10:46 +0100, Lee Jones wrote:
>> I have seen the "[LUCID]" and "[MAVERICK]" tags and did contemplate
>> using them, but I had no idea where this patch should go, so
>> intentionally omitted them.
>
> Very little goes directly into an already released kernel. Usually, bug
> fixes are sent to stable@kernel.org, then trickle back down to us. If
> the change fixes a really important issue, then we may take it in before
> that, but this is the exception rather than the rule.
>

Not completely true. Changes to our build environment might be taken for
convenience as they have no impact after things are build and that can be well
verified. This one even have no impact on the build itself.
But preferably this gets first into Maverick and then there is another
submission if it is deemed useful for any previous release.
In this special case there could be an "Ignore: yes" to hide it from the
generated changelog, given it is something an end-user does not care much about.
This would also prevent the need of a launchpad bug associated, which is
required for SRU otherwise.


> For development kernels, things are usually flipped. We normally apply
> changes as we submit them to the stable queue upstream.

Not that Greg would be too happy to see a change to debian/rules which he could
not care less. ;-P

> In this instance, where you're resubmitting a patch, I would send it as
> a new email (not a reply) with something like the following for a
> subject:
>
> [MAVERIC][PATCH v3] UBUNTU: 'fdr editconfig' modification. Easily skip
> over unwanted menuconfigs.

Right, that helps to keep the overview what is current or not. Otherwise there
is a long thread with patches attached and one gets confused.

-Stefan

> Generally, the patch numbering is omitted in a single patch instance.
>
>> How's this:
>>
>> The following changes since commit f0819aaf4948e34a44d9d685615ddee74271cd70:
>> Chase Douglas (1):
>> UBUNTU: enforce CONFIG_TMPFS_POSIX_ACL=y
>>
>> are available in the git repository at:
>>
>> git://kernel.ubuntu.com/lag/ubuntu-lucid.git editconfig
>
> This needs to be applied to the maverick source tree instead.
>
>>
>> Lee Jones (1):
>> UBUNTU: 'fdr editconfig' modification. Easily skip over unwanted menuconfigs.
>>
>> debian/scripts/misc/kernelconfig | 21 +++++++++++++++------
>> 1 files changed, 15 insertions(+), 6 deletions(-)
>>
>>
>> diff --git a/debian/scripts/misc/kernelconfig b/debian/scripts/misc/kernelconfig
>> index 71c0f5e..3181978 100755 (executable)
>> --- a/debian/scripts/misc/kernelconfig
>> +++ b/debian/scripts/misc/kernelconfig
>> @@ -51,9 +51,6 @@ for arch in $archs; do
>> *) kernarch="$arch" ;;
>> esac
>>
>> - echo ""
>> - echo "***************************************"
>> - echo "* Processing $arch ($kernarch) ... "
>> archconfdir=$confdir/$arch
>> flavourconfigs=$(cd $archconfdir && ls config.flavour.*)
>>
>> @@ -96,9 +93,21 @@ for arch in $archs; do
>> make O=`pwd`/build ARCH=$kernarch oldconfig ;;
>> editconfig)
>> # Interactively edit config parameters
>> - echo " * Run menuconfig on $arch/$config... Press a key."
>> - read
>> - make O=`pwd`/build ARCH=$kernarch menuconfig ;;
>> + while : ; do
>> + echo -n "Do you want to edit config: $arch/$config? [Y/n] "
>> + read choice
>> +
>> + case "$choice" in
>> + y* | Y* | "" )
>> + make O=`pwd`/build ARCH=$kernarch menuconfig
>> + break ;;
>> + n* | N* )
>> + break ;;
>> + *)
>> + echo "Entry not valid"
>> + esac
>> + done
>> + ;;
>> *) # Bad!
>> exit 1 ;;
>> esac
>>
>>>> hen you do, please check that your indentation
>>>> style matches the style used throughout the rules files. I'm not sure
>>>> the above is conforming.
>>>>
>>>
>> This is an odd one, as the original file did not conform, even to itself.
>>
>> For instance; the two case statements:
>>
>> One indents and uses quotes around the variable:
>> --------------------------------------------------------------
>> <snip>
>> case "$arch" in
>> amd64) kernarch="x86_64" ;;
>> </snip>
>> --------------------------------------------------------------
>>
>> The other does neither:
>> --------------------------------------------------------------
>> <snip>
>> case $config in
>> *)
>> </snip>
>> --------------------------------------------------------------
>>
>> The remainder of the kernel doesn't indent, but does use quotes and
>> Emacs tells me it should be indented.
>
> It's no fun when things aren't standardized :). I would just try to
> reproduce the same style as the code around your changes. In this
> instance, the only thing that stands out to me is the case statement
> should be like:
>
> case "$choice" in
> y* | Y* | "" )
> make O=`pwd`/build ARCH=$kernarch menuconfig
> break ;;
>
> This wouldn't be my first choice, but it's better to keep things
> consistent so it's readable. Other than that, I think everything is
> good.
>
> -- Chase
>
>


--
kernel-team mailing list
kernel-team@lists.ubuntu.com
https://lists.ubuntu.com/mailman/listinfo/kernel-team

Lee Jones 06-02-2010 07:50 AM

UBUNTU: 'fdr editconfig' modification. Easily skip over unwanted menuconfigs.
 
All,

It is now easier to edit 'per-flavour' configuration files. The old
system forced the developer to enter a menuconfig system for each
flavour until eventually landing in the required one. After following
this process numerous times it can easily become tedious. Now the
developer is asked whether they want to enter each menuconfig in turn,
rapidly speeding up the process.

The following changes since commit a8e1af2ae65acbed1b4e1009a80d2371a484aec9:
Leann Ogasawara (1):
UBUNTU: Ubuntu-2.6.34-2.9

are available in the git repository at:

git://kernel.ubuntu.com/lag/ubuntu-maverick.git editconfig

Lee Jones (1):
UBUNTU: 'fdr editconfig' modification. Easily skip over unwanted
menuconfigs.


debian/scripts/misc/kernelconfig | 21 +++++++++++++++------
1 files changed, 15 insertions(+), 6 deletions(-)

diff --git a/debian/scripts/misc/kernelconfig
b/debian/scripts/misc/kernelconfig
index 71c0f5e..3181978 100755 (executable)
--- a/debian/scripts/misc/kernelconfig
+++ b/debian/scripts/misc/kernelconfig
@@ -51,9 +51,6 @@ for arch in $archs; do
*) kernarch="$arch" ;;
esac

- echo ""
- echo "***************************************"
- echo "* Processing $arch ($kernarch) ... "
archconfdir=$confdir/$arch
flavourconfigs=$(cd $archconfdir && ls config.flavour.*)

@@ -96,9 +93,21 @@ for arch in $archs; do
make O=`pwd`/build ARCH=$kernarch
oldconfig ;;
editconfig)
# Interactively edit config parameters
- echo " * Run menuconfig on
$arch/$config... Press a key."
- read
- make O=`pwd`/build ARCH=$kernarch
menuconfig ;;
+ while : ; do
+ echo -n "Do you want to edit
config: $arch/$config? [Y/n] "
+ read choice
+
+ case "$choice" in
+ y* | Y* | "" )
+ make O=`pwd`/build
ARCH=$kernarch menuconfig
+ break ;;
+ n* | N* )
+ break ;;
+ *)
+ echo "Entry not valid"
+ esac
+ done
+ ;;
*) # Bad!
exit 1 ;;
esac

Kind regards,
Lee

--
kernel-team mailing list
kernel-team@lists.ubuntu.com
https://lists.ubuntu.com/mailman/listinfo/kernel-team


All times are GMT. The time now is 09:54 PM.

VBulletin, Copyright ©2000 - 2014, Jelsoft Enterprises Ltd.
Content Relevant URLs by vBSEO ©2007, Crawlability, Inc.