From: Bruce Ashfield <bruce.ashfield@windriver.com> To: Darren Hart <dvhart@infradead.org>, Olof Johansson <olof@lixom.net> Cc: Michal Marek <mmarek@suse.com>, <linux-kbuild@vger.kernel.org>, <linux-kernel@vger.kernel.org>, <dvhart@linux.intel.com> Subject: Re: [PATCH 00/10] merge_config misc reworks and testcases Date: Wed, 28 Oct 2015 01:30:59 -0400 [thread overview] Message-ID: <56305D93.5020702@windriver.com> (raw) In-Reply-To: <20151028050224.GB1854@malice.jf.intel.com> On 10/28/2015 01:02 AM, Darren Hart wrote: > On Wed, Oct 28, 2015 at 09:42:01AM +0900, Olof Johansson wrote: >> Hi, >> >> Somewhat wide distribution list here, since I've added everyone who's >> touched the script, with the presumption that those have been the major >> users of it. Please make sure none of these changes break your use cases. > > +Bruce. I think you were going to test these with the linux-yocto tooling. Did > you get a chance to run that test? I'd like your thoughts on the two comments > below: I ran some initial tests, but I didn't end up doing a full update due to other constraints. But in the next few weeks, I can do that update and full run. > >> >> I've done some reworks of merge_config.sh. I was quite hesitant to start >> this since there are no good ways to see if your changes break others >> or not, so the first thing I did was to add some tests. I know this is >> highly unorthodox so try not to panic. :-) >> >> As far as what this series does is: >> >> - Adds a way to pass in CONFIG_FOO=<value> on the command line, it gets >> treated as a single-entry fragment file >> >> - The script now prints the warnings on stderr, and returns non-0 when >> something is encountered > > This one might impact linux-yocto usage, Bruce? That said, it seems like the > right thing to do. So I'd still like to see it go in, but we may need to plan to > update the dependent tooling to use it. I don't directly let the merge_config output be visible, but capture it and then do more processing later. So while this may mean that I have to update some wrappers to capture stderr, it shouldn't be a big deal. > >> >> - Optionally, it'll also return non-0 when a redundant entry is found. I >> presumed people rely on -r not being a failure so I did this separately >> >> - CONFIG_FOO=n and "# CONFIG_FOO is not set" is now treated the same, >> and using the former doesn't cause an invalid warning when the results >> are checked at the end >> >> - Slightly odd things happened if a fragment contains the same option >> twice: It'd produce a warning that was malformed. Now just ignore that >> and use only the latest value of said option. > > This one will likely impact usage as well. linux-yocto does want to report when > there is an override, not as an error, but for informational purposes - "Where > does my option get clobbered?" I haven't looked at the patches yet (and I will shortly), but if that is within a single fragment, I can live with it going away, since it is easy to check that outside of the merge script. But if this is a redefinition between fragments, that's something different and something that I capture and report to users, and yes, I currently take it from the output of the merge_config run. If it goes away, I'd have to recreate it somehow. So if this can at least be maintained as enabled via a parameter, that would be be ideal. Otherwise, I'll have to recreate the output some other way. Bruce > > >
WARNING: multiple messages have this Message-ID (diff)
From: Bruce Ashfield <bruce.ashfield@windriver.com> To: Darren Hart <dvhart@infradead.org>, Olof Johansson <olof@lixom.net> Cc: Michal Marek <mmarek@suse.com>, linux-kbuild@vger.kernel.org, linux-kernel@vger.kernel.org, dvhart@linux.intel.com Subject: Re: [PATCH 00/10] merge_config misc reworks and testcases Date: Wed, 28 Oct 2015 01:30:59 -0400 [thread overview] Message-ID: <56305D93.5020702@windriver.com> (raw) In-Reply-To: <20151028050224.GB1854@malice.jf.intel.com> On 10/28/2015 01:02 AM, Darren Hart wrote: > On Wed, Oct 28, 2015 at 09:42:01AM +0900, Olof Johansson wrote: >> Hi, >> >> Somewhat wide distribution list here, since I've added everyone who's >> touched the script, with the presumption that those have been the major >> users of it. Please make sure none of these changes break your use cases. > > +Bruce. I think you were going to test these with the linux-yocto tooling. Did > you get a chance to run that test? I'd like your thoughts on the two comments > below: I ran some initial tests, but I didn't end up doing a full update due to other constraints. But in the next few weeks, I can do that update and full run. > >> >> I've done some reworks of merge_config.sh. I was quite hesitant to start >> this since there are no good ways to see if your changes break others >> or not, so the first thing I did was to add some tests. I know this is >> highly unorthodox so try not to panic. :-) >> >> As far as what this series does is: >> >> - Adds a way to pass in CONFIG_FOO=<value> on the command line, it gets >> treated as a single-entry fragment file >> >> - The script now prints the warnings on stderr, and returns non-0 when >> something is encountered > > This one might impact linux-yocto usage, Bruce? That said, it seems like the > right thing to do. So I'd still like to see it go in, but we may need to plan to > update the dependent tooling to use it. I don't directly let the merge_config output be visible, but capture it and then do more processing later. So while this may mean that I have to update some wrappers to capture stderr, it shouldn't be a big deal. > >> >> - Optionally, it'll also return non-0 when a redundant entry is found. I >> presumed people rely on -r not being a failure so I did this separately >> >> - CONFIG_FOO=n and "# CONFIG_FOO is not set" is now treated the same, >> and using the former doesn't cause an invalid warning when the results >> are checked at the end >> >> - Slightly odd things happened if a fragment contains the same option >> twice: It'd produce a warning that was malformed. Now just ignore that >> and use only the latest value of said option. > > This one will likely impact usage as well. linux-yocto does want to report when > there is an override, not as an error, but for informational purposes - "Where > does my option get clobbered?" I haven't looked at the patches yet (and I will shortly), but if that is within a single fragment, I can live with it going away, since it is easy to check that outside of the merge script. But if this is a redefinition between fragments, that's something different and something that I capture and report to users, and yes, I currently take it from the output of the merge_config run. If it goes away, I'd have to recreate it somehow. So if this can at least be maintained as enabled via a parameter, that would be be ideal. Otherwise, I'll have to recreate the output some other way. Bruce > > >
next prev parent reply other threads:[~2015-10-28 6:20 UTC|newest] Thread overview: 31+ messages / expand[flat|nested] mbox.gz Atom feed top 2015-10-28 0:42 [PATCH 00/10] merge_config misc reworks and testcases Olof Johansson 2015-10-28 0:42 ` [PATCH 01/10] merge_config.sh: factor out value parsing Olof Johansson 2015-10-28 5:24 ` Darren Hart 2015-10-28 0:42 ` [PATCH 02/10] merge_config.sh: print warnings on stderr Olof Johansson 2015-10-28 5:56 ` Darren Hart 2015-10-28 0:42 ` [PATCH 03/10] merge_config.sh: minor argument parsing refactoring Olof Johansson 2015-10-28 6:00 ` Darren Hart 2015-10-28 0:42 ` [PATCH 04/10] merge_config.sh: exit non-0 in case of failures Olof Johansson 2015-10-28 6:19 ` Darren Hart 2015-10-28 0:42 ` [PATCH 05/10] merge_config.sh: Better handling of CONFIG_FOO=n Olof Johansson 2015-10-28 6:26 ` Darren Hart 2015-10-28 6:30 ` Bruce Ashfield 2015-10-28 6:30 ` Bruce Ashfield 2015-10-28 0:42 ` [PATCH 06/10] merge_config.sh: only consider last value of symbols Olof Johansson 2015-10-28 6:36 ` Darren Hart 2015-10-28 0:42 ` [PATCH 07/10] merge_config.sh: add tests Olof Johansson 2015-10-28 7:00 ` Darren Hart 2015-10-28 7:07 ` Olof Johansson 2015-10-28 0:42 ` [PATCH 08/10] merge_config.sh: use trap for cleanup Olof Johansson 2015-10-28 7:11 ` Darren Hart 2015-10-28 7:28 ` Darren Hart 2015-10-28 0:42 ` [PATCH 09/10] merge_config.sh: allow single configs to be passed in on cmdline Olof Johansson 2015-10-28 7:22 ` Darren Hart 2015-10-28 0:42 ` [PATCH 10/10] merge_config.sh: add tests for cmdline configs Olof Johansson 2015-10-28 7:32 ` Darren Hart 2015-10-28 0:46 ` [PATCH 00/10] merge_config misc reworks and testcases Olof Johansson 2015-11-06 16:07 ` Michal Marek 2015-10-28 5:02 ` Darren Hart 2015-10-28 5:30 ` Bruce Ashfield [this message] 2015-10-28 5:30 ` Bruce Ashfield 2015-10-28 7:05 ` Darren Hart
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=56305D93.5020702@windriver.com \ --to=bruce.ashfield@windriver.com \ --cc=dvhart@infradead.org \ --cc=dvhart@linux.intel.com \ --cc=linux-kbuild@vger.kernel.org \ --cc=linux-kernel@vger.kernel.org \ --cc=mmarek@suse.com \ --cc=olof@lixom.net \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.