[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Xen-devel] [PATCH v2 2/2] arm: rename tiny64.conf to tiny64_defconfig
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@xxxxxxxxxxxxxxxxxxxx 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@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel
|
