* [PATCH 1/1] merge_config.sh: ignore unwanted grep errors
@ 2019-08-28 20:19 Guillaume Tucker
2019-09-02 14:14 ` Guillaume Tucker
0 siblings, 1 reply; 7+ messages in thread
From: Guillaume Tucker @ 2019-08-28 20:19 UTC (permalink / raw)
To: Masahiro Yamada, Mark Brown
Cc: linux-kbuild, linux-kernel, kernel, Guillaume Tucker
The merge_config.sh script verifies that all the config options have
their expected value in the resulting file and prints any issues as
warnings. These checks aren't intended to be treated as errors given
the current implementation. However, since "set -e" was added, if the
grep command to look for a config option does not find it the script
will then abort prematurely.
Handle the case where the grep exit status is non-zero by setting
ACTUAL_VAL to an empty string to restore previous functionality.
Fixes: cdfca821571d ("merge_config.sh: Check error codes from make")
Signed-off-by: Guillaume Tucker <guillaume.tucker@collabora.com>
---
scripts/kconfig/merge_config.sh | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/scripts/kconfig/merge_config.sh b/scripts/kconfig/merge_config.sh
index d924c51d28b7..d673268d414b 100755
--- a/scripts/kconfig/merge_config.sh
+++ b/scripts/kconfig/merge_config.sh
@@ -177,7 +177,7 @@ make KCONFIG_ALLCONFIG=$TMP_FILE $OUTPUT_ARG $ALLTARGET
for CFG in $(sed -n -e "$SED_CONFIG_EXP1" -e "$SED_CONFIG_EXP2" $TMP_FILE); do
REQUESTED_VAL=$(grep -w -e "$CFG" $TMP_FILE)
- ACTUAL_VAL=$(grep -w -e "$CFG" "$KCONFIG_CONFIG")
+ ACTUAL_VAL=$(grep -w -e "$CFG" "$KCONFIG_CONFIG" || echo)
if [ "x$REQUESTED_VAL" != "x$ACTUAL_VAL" ] ; then
echo "Value requested for $CFG not in final .config"
echo "Requested value: $REQUESTED_VAL"
--
2.20.1
^ permalink raw reply related [flat|nested] 7+ messages in thread* Re: [PATCH 1/1] merge_config.sh: ignore unwanted grep errors
2019-08-28 20:19 [PATCH 1/1] merge_config.sh: ignore unwanted grep errors Guillaume Tucker
@ 2019-09-02 14:14 ` Guillaume Tucker
2019-09-02 14:21 ` Jon Hunter
0 siblings, 1 reply; 7+ messages in thread
From: Guillaume Tucker @ 2019-09-02 14:14 UTC (permalink / raw)
To: Masahiro Yamada, Mark Brown, Jon Hunter
Cc: linux-kbuild, linux-kernel, kernel, linux-tegra
+ Jon Hunter who hit a similar issue
On 28/08/2019 21:19, Guillaume Tucker wrote:
> The merge_config.sh script verifies that all the config options have
> their expected value in the resulting file and prints any issues as
> warnings. These checks aren't intended to be treated as errors given
> the current implementation. However, since "set -e" was added, if the
> grep command to look for a config option does not find it the script
> will then abort prematurely.
>
> Handle the case where the grep exit status is non-zero by setting
> ACTUAL_VAL to an empty string to restore previous functionality.
>
> Fixes: cdfca821571d ("merge_config.sh: Check error codes from make")
> Signed-off-by: Guillaume Tucker <guillaume.tucker@collabora.com>
> ---
> scripts/kconfig/merge_config.sh | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/scripts/kconfig/merge_config.sh b/scripts/kconfig/merge_config.sh
> index d924c51d28b7..d673268d414b 100755
> --- a/scripts/kconfig/merge_config.sh
> +++ b/scripts/kconfig/merge_config.sh
> @@ -177,7 +177,7 @@ make KCONFIG_ALLCONFIG=$TMP_FILE $OUTPUT_ARG $ALLTARGET
> for CFG in $(sed -n -e "$SED_CONFIG_EXP1" -e "$SED_CONFIG_EXP2" $TMP_FILE); do
>
> REQUESTED_VAL=$(grep -w -e "$CFG" $TMP_FILE)
> - ACTUAL_VAL=$(grep -w -e "$CFG" "$KCONFIG_CONFIG")
> + ACTUAL_VAL=$(grep -w -e "$CFG" "$KCONFIG_CONFIG" || echo)
> if [ "x$REQUESTED_VAL" != "x$ACTUAL_VAL" ] ; then
> echo "Value requested for $CFG not in final .config"
> echo "Requested value: $REQUESTED_VAL"
>
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH 1/1] merge_config.sh: ignore unwanted grep errors
2019-09-02 14:14 ` Guillaume Tucker
@ 2019-09-02 14:21 ` Jon Hunter
2019-09-02 14:26 ` Guillaume Tucker
0 siblings, 1 reply; 7+ messages in thread
From: Jon Hunter @ 2019-09-02 14:21 UTC (permalink / raw)
To: Guillaume Tucker, Masahiro Yamada, Mark Brown
Cc: linux-kbuild, linux-kernel, kernel, linux-tegra
On 02/09/2019 15:14, Guillaume Tucker wrote:
> + Jon Hunter who hit a similar issue
Thanks for adding me.
> On 28/08/2019 21:19, Guillaume Tucker wrote:
>> The merge_config.sh script verifies that all the config options have
>> their expected value in the resulting file and prints any issues as
>> warnings. These checks aren't intended to be treated as errors given
>> the current implementation. However, since "set -e" was added, if the
>> grep command to look for a config option does not find it the script
>> will then abort prematurely.
>>
>> Handle the case where the grep exit status is non-zero by setting
>> ACTUAL_VAL to an empty string to restore previous functionality.
>>
>> Fixes: cdfca821571d ("merge_config.sh: Check error codes from make")
>> Signed-off-by: Guillaume Tucker <guillaume.tucker@collabora.com>
>> ---
>> scripts/kconfig/merge_config.sh | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/scripts/kconfig/merge_config.sh b/scripts/kconfig/merge_config.sh
>> index d924c51d28b7..d673268d414b 100755
>> --- a/scripts/kconfig/merge_config.sh
>> +++ b/scripts/kconfig/merge_config.sh
>> @@ -177,7 +177,7 @@ make KCONFIG_ALLCONFIG=$TMP_FILE $OUTPUT_ARG $ALLTARGET
>> for CFG in $(sed -n -e "$SED_CONFIG_EXP1" -e "$SED_CONFIG_EXP2" $TMP_FILE); do
>>
>> REQUESTED_VAL=$(grep -w -e "$CFG" $TMP_FILE)
>> - ACTUAL_VAL=$(grep -w -e "$CFG" "$KCONFIG_CONFIG")
>> + ACTUAL_VAL=$(grep -w -e "$CFG" "$KCONFIG_CONFIG" || echo)
Shouldn't this just be 'true' instead of 'echo'?
Cheers
Jon
--
nvpublic
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH 1/1] merge_config.sh: ignore unwanted grep errors
2019-09-02 14:21 ` Jon Hunter
@ 2019-09-02 14:26 ` Guillaume Tucker
2019-09-02 14:32 ` Jon Hunter
0 siblings, 1 reply; 7+ messages in thread
From: Guillaume Tucker @ 2019-09-02 14:26 UTC (permalink / raw)
To: Jon Hunter, Masahiro Yamada, Mark Brown
Cc: linux-kbuild, linux-kernel, kernel, linux-tegra
On 02/09/2019 15:21, Jon Hunter wrote:
>
> On 02/09/2019 15:14, Guillaume Tucker wrote:
>> + Jon Hunter who hit a similar issue
>
> Thanks for adding me.
>
>> On 28/08/2019 21:19, Guillaume Tucker wrote:
>>> The merge_config.sh script verifies that all the config options have
>>> their expected value in the resulting file and prints any issues as
>>> warnings. These checks aren't intended to be treated as errors given
>>> the current implementation. However, since "set -e" was added, if the
>>> grep command to look for a config option does not find it the script
>>> will then abort prematurely.
>>>
>>> Handle the case where the grep exit status is non-zero by setting
>>> ACTUAL_VAL to an empty string to restore previous functionality.
>>>
>>> Fixes: cdfca821571d ("merge_config.sh: Check error codes from make")
>>> Signed-off-by: Guillaume Tucker <guillaume.tucker@collabora.com>
>>> ---
>>> scripts/kconfig/merge_config.sh | 2 +-
>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/scripts/kconfig/merge_config.sh b/scripts/kconfig/merge_config.sh
>>> index d924c51d28b7..d673268d414b 100755
>>> --- a/scripts/kconfig/merge_config.sh
>>> +++ b/scripts/kconfig/merge_config.sh
>>> @@ -177,7 +177,7 @@ make KCONFIG_ALLCONFIG=$TMP_FILE $OUTPUT_ARG $ALLTARGET
>>> for CFG in $(sed -n -e "$SED_CONFIG_EXP1" -e "$SED_CONFIG_EXP2" $TMP_FILE); do
>>>
>>> REQUESTED_VAL=$(grep -w -e "$CFG" $TMP_FILE)
>>> - ACTUAL_VAL=$(grep -w -e "$CFG" "$KCONFIG_CONFIG")
>>> + ACTUAL_VAL=$(grep -w -e "$CFG" "$KCONFIG_CONFIG" || echo)
>
> Shouldn't this just be 'true' instead of 'echo'?
I just explained why I used "echo" on your thread. Essentially,
I think both can be used but "echo" made more sense to me because
the script is then using the output string from the command
rather than the exit status.
Guillaume
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH 1/1] merge_config.sh: ignore unwanted grep errors
2019-09-02 14:26 ` Guillaume Tucker
@ 2019-09-02 14:32 ` Jon Hunter
2019-09-02 14:49 ` Guillaume Tucker
0 siblings, 1 reply; 7+ messages in thread
From: Jon Hunter @ 2019-09-02 14:32 UTC (permalink / raw)
To: Guillaume Tucker, Masahiro Yamada, Mark Brown
Cc: linux-kbuild, linux-kernel, kernel, linux-tegra
On 02/09/2019 15:26, Guillaume Tucker wrote:
> On 02/09/2019 15:21, Jon Hunter wrote:
>>
>> On 02/09/2019 15:14, Guillaume Tucker wrote:
>>> + Jon Hunter who hit a similar issue
>>
>> Thanks for adding me.
>>
>>> On 28/08/2019 21:19, Guillaume Tucker wrote:
>>>> The merge_config.sh script verifies that all the config options have
>>>> their expected value in the resulting file and prints any issues as
>>>> warnings. These checks aren't intended to be treated as errors given
>>>> the current implementation. However, since "set -e" was added, if the
>>>> grep command to look for a config option does not find it the script
>>>> will then abort prematurely.
>>>>
>>>> Handle the case where the grep exit status is non-zero by setting
>>>> ACTUAL_VAL to an empty string to restore previous functionality.
>>>>
>>>> Fixes: cdfca821571d ("merge_config.sh: Check error codes from make")
>>>> Signed-off-by: Guillaume Tucker <guillaume.tucker@collabora.com>
>>>> ---
>>>> scripts/kconfig/merge_config.sh | 2 +-
>>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>>
>>>> diff --git a/scripts/kconfig/merge_config.sh b/scripts/kconfig/merge_config.sh
>>>> index d924c51d28b7..d673268d414b 100755
>>>> --- a/scripts/kconfig/merge_config.sh
>>>> +++ b/scripts/kconfig/merge_config.sh
>>>> @@ -177,7 +177,7 @@ make KCONFIG_ALLCONFIG=$TMP_FILE $OUTPUT_ARG $ALLTARGET
>>>> for CFG in $(sed -n -e "$SED_CONFIG_EXP1" -e "$SED_CONFIG_EXP2" $TMP_FILE); do
>>>>
>>>> REQUESTED_VAL=$(grep -w -e "$CFG" $TMP_FILE)
>>>> - ACTUAL_VAL=$(grep -w -e "$CFG" "$KCONFIG_CONFIG")
>>>> + ACTUAL_VAL=$(grep -w -e "$CFG" "$KCONFIG_CONFIG" || echo)
>>
>> Shouldn't this just be 'true' instead of 'echo'?
>
> I just explained why I used "echo" on your thread. Essentially,
> I think both can be used but "echo" made more sense to me because
> the script is then using the output string from the command
> rather than the exit status.
Yes just saw that. However, I don't think that using 'echo' is
necessary. The grep command does not output anything and so the variable
will essentially be an empty string, we just need to ensure that no
error is returned from the command. In cases such as these I always use
'true' in conjunction with grep.
Cheers
Jon
--
nvpublic
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH 1/1] merge_config.sh: ignore unwanted grep errors
2019-09-02 14:32 ` Jon Hunter
@ 2019-09-02 14:49 ` Guillaume Tucker
2019-09-02 14:53 ` Jon Hunter
0 siblings, 1 reply; 7+ messages in thread
From: Guillaume Tucker @ 2019-09-02 14:49 UTC (permalink / raw)
To: Jon Hunter, Masahiro Yamada, Mark Brown
Cc: linux-kbuild, linux-kernel, kernel, linux-tegra
On 02/09/2019 15:32, Jon Hunter wrote:
>
> On 02/09/2019 15:26, Guillaume Tucker wrote:
>> On 02/09/2019 15:21, Jon Hunter wrote:
>>>
>>> On 02/09/2019 15:14, Guillaume Tucker wrote:
>>>> + Jon Hunter who hit a similar issue
>>>
>>> Thanks for adding me.
>>>
>>>> On 28/08/2019 21:19, Guillaume Tucker wrote:
>>>>> The merge_config.sh script verifies that all the config options have
>>>>> their expected value in the resulting file and prints any issues as
>>>>> warnings. These checks aren't intended to be treated as errors given
>>>>> the current implementation. However, since "set -e" was added, if the
>>>>> grep command to look for a config option does not find it the script
>>>>> will then abort prematurely.
>>>>>
>>>>> Handle the case where the grep exit status is non-zero by setting
>>>>> ACTUAL_VAL to an empty string to restore previous functionality.
>>>>>
>>>>> Fixes: cdfca821571d ("merge_config.sh: Check error codes from make")
>>>>> Signed-off-by: Guillaume Tucker <guillaume.tucker@collabora.com>
>>>>> ---
>>>>> scripts/kconfig/merge_config.sh | 2 +-
>>>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/scripts/kconfig/merge_config.sh b/scripts/kconfig/merge_config.sh
>>>>> index d924c51d28b7..d673268d414b 100755
>>>>> --- a/scripts/kconfig/merge_config.sh
>>>>> +++ b/scripts/kconfig/merge_config.sh
>>>>> @@ -177,7 +177,7 @@ make KCONFIG_ALLCONFIG=$TMP_FILE $OUTPUT_ARG $ALLTARGET
>>>>> for CFG in $(sed -n -e "$SED_CONFIG_EXP1" -e "$SED_CONFIG_EXP2" $TMP_FILE); do
>>>>>
>>>>> REQUESTED_VAL=$(grep -w -e "$CFG" $TMP_FILE)
>>>>> - ACTUAL_VAL=$(grep -w -e "$CFG" "$KCONFIG_CONFIG")
>>>>> + ACTUAL_VAL=$(grep -w -e "$CFG" "$KCONFIG_CONFIG" || echo)
>>>
>>> Shouldn't this just be 'true' instead of 'echo'?
>>
>> I just explained why I used "echo" on your thread. Essentially,
>> I think both can be used but "echo" made more sense to me because
>> the script is then using the output string from the command
>> rather than the exit status.
>
> Yes just saw that. However, I don't think that using 'echo' is
> necessary. The grep command does not output anything and so the variable
> will essentially be an empty string, we just need to ensure that no
> error is returned from the command. In cases such as these I always use
> 'true' in conjunction with grep.
Sure, that makes sense too. Your solution is arguably a bit
simpler so I agree it would be better to use "true" here.
I can submit a v2 with "true" if that helps, unless you prefer to
send your version of the fix yourself?
Also we're actually using this fix in KernelCI to test it on top
of Mark's patch:
https://github.com/kernelci/linux/commits/staging.kernelci.org
so I can get it tested again with quite a few build variants
using "true". It's kind of trivial but we need a working
merge_config.sh anyway on that branch.
Guillaume
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH 1/1] merge_config.sh: ignore unwanted grep errors
2019-09-02 14:49 ` Guillaume Tucker
@ 2019-09-02 14:53 ` Jon Hunter
0 siblings, 0 replies; 7+ messages in thread
From: Jon Hunter @ 2019-09-02 14:53 UTC (permalink / raw)
To: Guillaume Tucker, Masahiro Yamada, Mark Brown
Cc: linux-kbuild, linux-kernel, kernel, linux-tegra
On 02/09/2019 15:49, Guillaume Tucker wrote:
> On 02/09/2019 15:32, Jon Hunter wrote:
>>
>> On 02/09/2019 15:26, Guillaume Tucker wrote:
>>> On 02/09/2019 15:21, Jon Hunter wrote:
>>>>
>>>> On 02/09/2019 15:14, Guillaume Tucker wrote:
>>>>> + Jon Hunter who hit a similar issue
>>>>
>>>> Thanks for adding me.
>>>>
>>>>> On 28/08/2019 21:19, Guillaume Tucker wrote:
>>>>>> The merge_config.sh script verifies that all the config options have
>>>>>> their expected value in the resulting file and prints any issues as
>>>>>> warnings. These checks aren't intended to be treated as errors given
>>>>>> the current implementation. However, since "set -e" was added, if the
>>>>>> grep command to look for a config option does not find it the script
>>>>>> will then abort prematurely.
>>>>>>
>>>>>> Handle the case where the grep exit status is non-zero by setting
>>>>>> ACTUAL_VAL to an empty string to restore previous functionality.
>>>>>>
>>>>>> Fixes: cdfca821571d ("merge_config.sh: Check error codes from make")
>>>>>> Signed-off-by: Guillaume Tucker <guillaume.tucker@collabora.com>
>>>>>> ---
>>>>>> scripts/kconfig/merge_config.sh | 2 +-
>>>>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>>>>
>>>>>> diff --git a/scripts/kconfig/merge_config.sh b/scripts/kconfig/merge_config.sh
>>>>>> index d924c51d28b7..d673268d414b 100755
>>>>>> --- a/scripts/kconfig/merge_config.sh
>>>>>> +++ b/scripts/kconfig/merge_config.sh
>>>>>> @@ -177,7 +177,7 @@ make KCONFIG_ALLCONFIG=$TMP_FILE $OUTPUT_ARG $ALLTARGET
>>>>>> for CFG in $(sed -n -e "$SED_CONFIG_EXP1" -e "$SED_CONFIG_EXP2" $TMP_FILE); do
>>>>>>
>>>>>> REQUESTED_VAL=$(grep -w -e "$CFG" $TMP_FILE)
>>>>>> - ACTUAL_VAL=$(grep -w -e "$CFG" "$KCONFIG_CONFIG")
>>>>>> + ACTUAL_VAL=$(grep -w -e "$CFG" "$KCONFIG_CONFIG" || echo)
>>>>
>>>> Shouldn't this just be 'true' instead of 'echo'?
>>>
>>> I just explained why I used "echo" on your thread. Essentially,
>>> I think both can be used but "echo" made more sense to me because
>>> the script is then using the output string from the command
>>> rather than the exit status.
>>
>> Yes just saw that. However, I don't think that using 'echo' is
>> necessary. The grep command does not output anything and so the variable
>> will essentially be an empty string, we just need to ensure that no
>> error is returned from the command. In cases such as these I always use
>> 'true' in conjunction with grep.
>
> Sure, that makes sense too. Your solution is arguably a bit
> simpler so I agree it would be better to use "true" here.
>
> I can submit a v2 with "true" if that helps, unless you prefer to
> send your version of the fix yourself?
Seeing as you have already created the patch, please go ahead and send.
Thanks
Jon
--
nvpublic
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2019-09-02 14:53 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-08-28 20:19 [PATCH 1/1] merge_config.sh: ignore unwanted grep errors Guillaume Tucker
2019-09-02 14:14 ` Guillaume Tucker
2019-09-02 14:21 ` Jon Hunter
2019-09-02 14:26 ` Guillaume Tucker
2019-09-02 14:32 ` Jon Hunter
2019-09-02 14:49 ` Guillaume Tucker
2019-09-02 14:53 ` Jon Hunter
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox