From: Julien Grall <julien.grall@arm.com> To: Volodymyr Babchuk <Volodymyr_Babchuk@epam.com> Cc: Stefano Stabellini <sstabellini@kernel.org>, Wei Liu <wei.liu2@citrix.com>, Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>, George Dunlap <George.Dunlap@eu.citrix.com>, Andrew Cooper <andrew.cooper3@citrix.com>, Ian Jackson <ian.jackson@eu.citrix.com>, Tim Deegan <tim@xen.org>, "xen-devel@lists.xenproject.org" <xen-devel@lists.xenproject.org> Subject: Re: [PATCH v2 2/2] arm: rename tiny64.conf to tiny64_defconfig Date: Tue, 28 May 2019 17:21:41 +0100 [thread overview] Message-ID: <2b392d1c-0837-4395-8ddc-f0cc04bbea83@arm.com> (raw) In-Reply-To: <878sv18977.fsf@epam.com> Hi Volodymyr, Sorry for the late reply. On 5/20/19 3:57 PM, Volodymyr Babchuk wrote: > > Julien Grall writes: > >> Hi, >> >> On 20/05/2019 14:41, Volodymyr Babchuk wrote: >>> Julien Grall writes: >>> >>>> Hi, >>>> >>>> First of all, please add a cover letter when you send a series. This >>>> help for threading and also a place to commend on general feedback. >>> Oh, okay. That was quite simple change and I didn't wanted to spam with >>> extra emails. I will include cover letter next time. >>> >>>> Furthermore, please use scripts/{add, get}_maintainers.pl to find the >>>> correct maintainers. While I agree that CCing REST is a good idea, you >>>> haven't CCed all of them. >>> Problem is that I used this script: >>> >>> $ ./scripts/get_maintainer.pl -f defconfig_v2/v2-0002-arm-rename-tiny64.conf-to-tiny64_defconfig.patch >> >> -f is to be used on actual file in the source tree. So the result >> below makes sense. For actual patch, you have to drop the -f. > Ah, I see. Without -f I'm getting the same message as with > add-maintainers.pl: > > % ./scripts/get_maintainer.pl defconfig_v2/v2-0002-arm-rename-tiny64.conf-to-tiny64_defconfig.patch > ./scripts/get_maintainer.pl: file 'defconfig_v2/v2-0002-arm-rename-tiny64.conf-to-tiny64_defconfig.patch' doesn't appear to be a patch. Add -f to options? > > [...] > >>> >>> % scripts/add_maintainers.pl -v 2 -d defconfig_v2 >>> Processing: v2-0001-makefile-add-support-for-_defconfig-targets.patch >>> Processing: v2-0002-arm-rename-tiny64.conf-to-tiny64_defconfig.patch >>> ./scripts/get_maintainer.pl: file 'defconfig_v2/v2-0002-arm-rename-tiny64.conf-to-tiny64_defconfig.patch' doesn't appear to be a patch. Add -f to options? >> >> I have just tried it and can't find the same error. Could you provide >> more details? Such as where to do call from the exact content of each >> patches... > > My basic flow: > > % git format-patch -v2 -2 -o defconfig_v2 > % scripts/add_maintainers.pl -v 2 -d defconfig_v2 > Processing: v2-0001-makefile-add-support-for-_defconfig-targets.patch > Processing: v2-0002-arm-rename-tiny64.conf-to-tiny64_defconfig.patch > ./scripts/get_maintainer.pl: file 'defconfig_v2/v2-0002-arm-rename-tiny64.conf-to-tiny64_defconfig.patch' doesn't appear to be a patch. Add -f to options? > Then perform: > git send-email -to xen-devel@lists.xenproject.org defconfig_v2/v2-*.patch > > > HEAD (prior to my patches) is at > 278c64519c661c851d37e2a929f006fb8a1dcd01 > > git version 2.21.0 > > Contents of the patch is the exactly the same as in my original > email. You can find both patches at [1]. It looks like the problem is because the second patch only contains renaming. Linux recently fixed it with the following commit: 0455c74788fd "get_maintainer: improve patch recognition" I guess we need to port the patch in Xen. Volodymyr, would you mind to send a patch for it? > >>> >>>> >>>> On 16/05/2019 14:37, Volodymyr Babchuk wrote: >>>>> As build system now supports *_defconfig rules it is good to be able >>>>> to configure minimal XEN image with >>>> >>>> I am afraid this is not correct. tiny64 will not be able to generate a >>>> minimal config to boot on any platform supported by Xen. >>>> >>>> It is meant to be used as a base for tailoring your platform where all >>>> the options are turned off by default. >>>> >>>> So I think offering a direct access is likely going to be misused in >>>> most of the cases without proper documentation. >>> >>> In the original commit message Stefano suggested to use olddefconfig: >>> >>> " Add a tiny kconfig configuration. Enabled only the credit scheduler. >>> It only carries non-default options (use make menuconfig or make >>> olddefconfig to produce a complete .config file). " >>> >>> I don't see any significant difference between >> >> Did you actually try the two approach and see how they differ? > > Yes. I did the following: > > % cp arch/arm/configs/tiny64_defconfig .config > % make olddefconfig > make -f /home/lorc/work/xen/xen/tools/kconfig/Makefile.kconfig ARCH=arm64 SRCARCH=arm HOSTCC="gcc" HOSTCXX="g++" olddefconfig > make[1]: Entering directory '/home/lorc/work/xen/xen' > gcc -Wp,-MD,tools/kconfig/.conf.o.d -D_GNU_SOURCE -DCURSES_LOC="<ncurses.h>" -DNCURSES_WIDECHAR=1 -DLOCALE -c -o tools/kconfig/conf.o tools/kconfig/conf.c > gcc -Wp,-MD,tools/kconfig/.zconf.tab.o.d -D_GNU_SOURCE -DCURSES_LOC="<ncurses.h>" -DNCURSES_WIDECHAR=1 -DLOCALE -Itools/kconfig -c -o tools/kconfig/zconf.tab.o tools/kconfig/zconf.tab.c > gcc -o tools/kconfig/conf tools/kconfig/conf.o tools/kconfig/zconf.tab.o > tools/kconfig/conf -s --olddefconfig Kconfig > make[1]: Leaving directory '/home/lorc/work/xen/xen' > > And > > % make tiny64_defconfig > make -f /home/lorc/work/xen/xen/tools/kconfig/Makefile.kconfig ARCH=arm64 SRCARCH=arm HOSTCC="gcc" HOSTCXX="g++" tiny64_defconfig > make[1]: Entering directory '/home/lorc/work/xen/xen' > gcc -Wp,-MD,tools/kconfig/.conf.o.d -D_GNU_SOURCE -DCURSES_LOC="<ncurses.h>" -DNCURSES_WIDECHAR=1 -DLOCALE -c -o tools/kconfig/conf.o tools/kconfig/conf.c > gcc -Wp,-MD,tools/kconfig/.zconf.tab.o.d -D_GNU_SOURCE -DCURSES_LOC="<ncurses.h>" -DNCURSES_WIDECHAR=1 -DLOCALE -Itools/kconfig -c -o tools/kconfig/zconf.tab.o tools/kconfig/zconf.tab.c > gcc -o tools/kconfig/conf tools/kconfig/conf.o tools/kconfig/zconf.tab.o > make[1]: Leaving directory '/home/lorc/work/xen/xen' > > Then I compared both .config files and found no difference at all: > > % diff -u .config1 .config2 > (displayed nothing) > >>> >>> # cp tiny64.conf .config && make olddefconfig >> >> This one will ask you details on the configuration you want while... > > But it does not, while "make oldconfig" does. Are you sure you are not > confusing oldconfig and olddefconfig targets? I am confusing both :(. Sorry for the noise. > >>> >>> and >>> >>> # make tiny64_defconfig >> >> ... this one will hide the questions. >> >>> >>> Anyways, it is up to you to accept or decline this particular patch. I >>> mostly interested in the first patch in the series, because our build >>> system depends on it. This very patch I sent out only because I wanted >>> to tidy up things a bit. But if you are saying that it is intended to >>> store minimal config in this way, I'm okay with it. >> >> The point of review is to discuss on the approach and find a common agreement. >> >> If you read my previous e-mail, I didn't completely reject the >> approach in my previous e-mail. I pointed out that the user may be >> misled of the name and hence documentation would be useful. > > I'm okay with this. Any ideas how to document it? We don't seem to have a place today where we document the defconfig. I am thinking to put that in docs/misc/arm. I would document the purpose of each config. The documentation could be in a separate patch. Cheers, > > [1] https://github.com/lorc/xen/commits/defconfig_v2 > -- Julien Grall _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
WARNING: multiple messages have this Message-ID (diff)
From: Julien Grall <julien.grall@arm.com> To: Volodymyr Babchuk <Volodymyr_Babchuk@epam.com> Cc: Stefano Stabellini <sstabellini@kernel.org>, Wei Liu <wei.liu2@citrix.com>, Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>, George Dunlap <George.Dunlap@eu.citrix.com>, Andrew Cooper <andrew.cooper3@citrix.com>, Ian Jackson <ian.jackson@eu.citrix.com>, Tim Deegan <tim@xen.org>, "xen-devel@lists.xenproject.org" <xen-devel@lists.xenproject.org> Subject: Re: [Xen-devel] [PATCH v2 2/2] arm: rename tiny64.conf to tiny64_defconfig Date: Tue, 28 May 2019 17:21:41 +0100 [thread overview] Message-ID: <2b392d1c-0837-4395-8ddc-f0cc04bbea83@arm.com> (raw) Message-ID: <20190528162141.BrP_MO60GwyKobkgULlrqE5CV9vI8lIeYEyOHfWXKJc@z> (raw) In-Reply-To: <878sv18977.fsf@epam.com> Hi Volodymyr, Sorry for the late reply. On 5/20/19 3:57 PM, Volodymyr Babchuk wrote: > > Julien Grall writes: > >> Hi, >> >> On 20/05/2019 14:41, Volodymyr Babchuk wrote: >>> Julien Grall writes: >>> >>>> Hi, >>>> >>>> First of all, please add a cover letter when you send a series. This >>>> help for threading and also a place to commend on general feedback. >>> Oh, okay. That was quite simple change and I didn't wanted to spam with >>> extra emails. I will include cover letter next time. >>> >>>> Furthermore, please use scripts/{add, get}_maintainers.pl to find the >>>> correct maintainers. While I agree that CCing REST is a good idea, you >>>> haven't CCed all of them. >>> Problem is that I used this script: >>> >>> $ ./scripts/get_maintainer.pl -f defconfig_v2/v2-0002-arm-rename-tiny64.conf-to-tiny64_defconfig.patch >> >> -f is to be used on actual file in the source tree. So the result >> below makes sense. For actual patch, you have to drop the -f. > Ah, I see. Without -f I'm getting the same message as with > add-maintainers.pl: > > % ./scripts/get_maintainer.pl defconfig_v2/v2-0002-arm-rename-tiny64.conf-to-tiny64_defconfig.patch > ./scripts/get_maintainer.pl: file 'defconfig_v2/v2-0002-arm-rename-tiny64.conf-to-tiny64_defconfig.patch' doesn't appear to be a patch. Add -f to options? > > [...] > >>> >>> % scripts/add_maintainers.pl -v 2 -d defconfig_v2 >>> Processing: v2-0001-makefile-add-support-for-_defconfig-targets.patch >>> Processing: v2-0002-arm-rename-tiny64.conf-to-tiny64_defconfig.patch >>> ./scripts/get_maintainer.pl: file 'defconfig_v2/v2-0002-arm-rename-tiny64.conf-to-tiny64_defconfig.patch' doesn't appear to be a patch. Add -f to options? >> >> I have just tried it and can't find the same error. Could you provide >> more details? Such as where to do call from the exact content of each >> patches... > > My basic flow: > > % git format-patch -v2 -2 -o defconfig_v2 > % scripts/add_maintainers.pl -v 2 -d defconfig_v2 > Processing: v2-0001-makefile-add-support-for-_defconfig-targets.patch > Processing: v2-0002-arm-rename-tiny64.conf-to-tiny64_defconfig.patch > ./scripts/get_maintainer.pl: file 'defconfig_v2/v2-0002-arm-rename-tiny64.conf-to-tiny64_defconfig.patch' doesn't appear to be a patch. Add -f to options? > Then perform: > git send-email -to xen-devel@lists.xenproject.org defconfig_v2/v2-*.patch > > > HEAD (prior to my patches) is at > 278c64519c661c851d37e2a929f006fb8a1dcd01 > > git version 2.21.0 > > Contents of the patch is the exactly the same as in my original > email. You can find both patches at [1]. It looks like the problem is because the second patch only contains renaming. Linux recently fixed it with the following commit: 0455c74788fd "get_maintainer: improve patch recognition" I guess we need to port the patch in Xen. Volodymyr, would you mind to send a patch for it? > >>> >>>> >>>> On 16/05/2019 14:37, Volodymyr Babchuk wrote: >>>>> As build system now supports *_defconfig rules it is good to be able >>>>> to configure minimal XEN image with >>>> >>>> I am afraid this is not correct. tiny64 will not be able to generate a >>>> minimal config to boot on any platform supported by Xen. >>>> >>>> It is meant to be used as a base for tailoring your platform where all >>>> the options are turned off by default. >>>> >>>> So I think offering a direct access is likely going to be misused in >>>> most of the cases without proper documentation. >>> >>> In the original commit message Stefano suggested to use olddefconfig: >>> >>> " Add a tiny kconfig configuration. Enabled only the credit scheduler. >>> It only carries non-default options (use make menuconfig or make >>> olddefconfig to produce a complete .config file). " >>> >>> I don't see any significant difference between >> >> Did you actually try the two approach and see how they differ? > > Yes. I did the following: > > % cp arch/arm/configs/tiny64_defconfig .config > % make olddefconfig > make -f /home/lorc/work/xen/xen/tools/kconfig/Makefile.kconfig ARCH=arm64 SRCARCH=arm HOSTCC="gcc" HOSTCXX="g++" olddefconfig > make[1]: Entering directory '/home/lorc/work/xen/xen' > gcc -Wp,-MD,tools/kconfig/.conf.o.d -D_GNU_SOURCE -DCURSES_LOC="<ncurses.h>" -DNCURSES_WIDECHAR=1 -DLOCALE -c -o tools/kconfig/conf.o tools/kconfig/conf.c > gcc -Wp,-MD,tools/kconfig/.zconf.tab.o.d -D_GNU_SOURCE -DCURSES_LOC="<ncurses.h>" -DNCURSES_WIDECHAR=1 -DLOCALE -Itools/kconfig -c -o tools/kconfig/zconf.tab.o tools/kconfig/zconf.tab.c > gcc -o tools/kconfig/conf tools/kconfig/conf.o tools/kconfig/zconf.tab.o > tools/kconfig/conf -s --olddefconfig Kconfig > make[1]: Leaving directory '/home/lorc/work/xen/xen' > > And > > % make tiny64_defconfig > make -f /home/lorc/work/xen/xen/tools/kconfig/Makefile.kconfig ARCH=arm64 SRCARCH=arm HOSTCC="gcc" HOSTCXX="g++" tiny64_defconfig > make[1]: Entering directory '/home/lorc/work/xen/xen' > gcc -Wp,-MD,tools/kconfig/.conf.o.d -D_GNU_SOURCE -DCURSES_LOC="<ncurses.h>" -DNCURSES_WIDECHAR=1 -DLOCALE -c -o tools/kconfig/conf.o tools/kconfig/conf.c > gcc -Wp,-MD,tools/kconfig/.zconf.tab.o.d -D_GNU_SOURCE -DCURSES_LOC="<ncurses.h>" -DNCURSES_WIDECHAR=1 -DLOCALE -Itools/kconfig -c -o tools/kconfig/zconf.tab.o tools/kconfig/zconf.tab.c > gcc -o tools/kconfig/conf tools/kconfig/conf.o tools/kconfig/zconf.tab.o > make[1]: Leaving directory '/home/lorc/work/xen/xen' > > Then I compared both .config files and found no difference at all: > > % diff -u .config1 .config2 > (displayed nothing) > >>> >>> # cp tiny64.conf .config && make olddefconfig >> >> This one will ask you details on the configuration you want while... > > But it does not, while "make oldconfig" does. Are you sure you are not > confusing oldconfig and olddefconfig targets? I am confusing both :(. Sorry for the noise. > >>> >>> and >>> >>> # make tiny64_defconfig >> >> ... this one will hide the questions. >> >>> >>> Anyways, it is up to you to accept or decline this particular patch. I >>> mostly interested in the first patch in the series, because our build >>> system depends on it. This very patch I sent out only because I wanted >>> to tidy up things a bit. But if you are saying that it is intended to >>> store minimal config in this way, I'm okay with it. >> >> The point of review is to discuss on the approach and find a common agreement. >> >> If you read my previous e-mail, I didn't completely reject the >> approach in my previous e-mail. I pointed out that the user may be >> misled of the name and hence documentation would be useful. > > I'm okay with this. Any ideas how to document it? We don't seem to have a place today where we document the defconfig. I am thinking to put that in docs/misc/arm. I would document the purpose of each config. The documentation could be in a separate patch. Cheers, > > [1] https://github.com/lorc/xen/commits/defconfig_v2 > -- Julien Grall _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
next prev parent reply other threads:[~2019-05-28 16:21 UTC|newest] Thread overview: 32+ messages / expand[flat|nested] mbox.gz Atom feed top 2019-05-16 13:37 [PATCH v2 1/2] makefile: add support for *_defconfig targets Volodymyr Babchuk 2019-05-16 13:37 ` [Xen-devel] " Volodymyr Babchuk 2019-05-16 13:37 ` [PATCH v2 2/2] arm: rename tiny64.conf to tiny64_defconfig Volodymyr Babchuk 2019-05-16 13:37 ` [Xen-devel] " Volodymyr Babchuk 2019-05-20 13:01 ` Julien Grall 2019-05-20 13:01 ` [Xen-devel] " Julien Grall 2019-05-20 13:41 ` Volodymyr Babchuk 2019-05-20 13:41 ` [Xen-devel] " Volodymyr Babchuk 2019-05-20 14:31 ` Julien Grall 2019-05-20 14:31 ` [Xen-devel] " Julien Grall 2019-05-20 14:57 ` Volodymyr Babchuk 2019-05-20 14:57 ` [Xen-devel] " Volodymyr Babchuk 2019-05-28 16:21 ` Julien Grall [this message] 2019-05-28 16:21 ` Julien Grall 2019-05-29 11:40 ` Volodymyr Babchuk 2019-05-29 11:40 ` [Xen-devel] " Volodymyr Babchuk 2019-05-29 15:27 ` Julien Grall 2019-05-29 15:27 ` [Xen-devel] " Julien Grall 2019-06-05 15:58 ` Jan Beulich 2019-06-05 16:01 ` Julien Grall 2019-06-10 20:03 ` Julien Grall 2019-06-11 6:43 ` Jan Beulich 2019-06-11 9:27 ` Julien Grall 2019-06-11 9:41 ` Jan Beulich 2019-06-11 10:12 ` George Dunlap 2019-06-11 13:52 ` Julien Grall 2019-06-11 13:52 ` Julien Grall 2019-06-11 18:52 ` Volodymyr Babchuk 2019-06-12 7:44 ` Jan Beulich 2019-06-15 18:27 ` Julien Grall 2019-05-16 15:10 ` [PATCH v2 1/2] makefile: add support for *_defconfig targets Jan Beulich 2019-05-16 15:10 ` [Xen-devel] " Jan Beulich
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=2b392d1c-0837-4395-8ddc-f0cc04bbea83@arm.com \ --to=julien.grall@arm.com \ --cc=George.Dunlap@eu.citrix.com \ --cc=Volodymyr_Babchuk@epam.com \ --cc=andrew.cooper3@citrix.com \ --cc=ian.jackson@eu.citrix.com \ --cc=konrad.wilk@oracle.com \ --cc=sstabellini@kernel.org \ --cc=tim@xen.org \ --cc=wei.liu2@citrix.com \ --cc=xen-devel@lists.xenproject.org \ /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 a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).