| Message ID | 996d1d52c27da1a7725ed425ac895c3a7b5b7301.1499592591.git.yann.morin.1998@free.fr |
|---|---|
| State | Accepted |
| Headers | show |
Hello, On Sun, 9 Jul 2017 11:29:59 +0200, Yann E. MORIN wrote: > diff --git a/package/kvm-unit-tests/Config.in b/package/kvm-unit-tests/Config.in > index 7eab0c25d6..3db10fc820 100644 > --- a/package/kvm-unit-tests/Config.in > +++ b/package/kvm-unit-tests/Config.in > @@ -1,15 +1,19 @@ > +config BR2_PACKAGE_KVM_UNIT_TESTS_ARCH_SUPPORTS > + bool > + # On ARM, it uses virtualization extensions > + default y if BR2_cortex_a7 || BR2_cortex_a12 || \ > + BR2_cortex_a15 || BR2_cortex_a17 > + default y if BR2_i386 || BR2_x86_64 > + default y if BR2_powerpc64 || BR2_powerpc64le > + > config BR2_PACKAGE_KVM_UNIT_TESTS > bool "kvm-unit-tests" > + depends on BR2_PACKAGE_KVM_UNIT_TESTS_ARCH_SUPPORTS > # on i386 and x86-64, __builtin_reachable is used, so we need > # gcc 4.5 at least. on i386, we use the target gcc, while on > # x86-64 we use the host gcc (see .mk file for details) > - # On ARM, it uses virtualization extensions > - depends on BR2_cortex_a7 || BR2_cortex_a12 || \ > - BR2_cortex_a15 || BR2_cortex_a17 || \ > - (BR2_i386 && BR2_TOOLCHAIN_GCC_AT_LEAST_4_5) || \ > - BR2_powerpc64 || \ > - BR2_powerpc64le || \ > - (BR2_x86_64 && BR2_HOST_GCC_AT_LEAST_4_5) > + depends on BR2_TOOLCHAIN_GCC_AT_LEAST_4_5 if BR2_i386 > + depends on BR2_HOST_GCC_AT_LEAST_4_5 if BR2_x86_64 Didn't know this syntax was valid. We usually do: depends on BR2_TOOLCHAIN_GCC_AT_LEAST_4_5 || !BR2_i386 but if the "depends on ... if ..." syntax works, I'm happy with it. > select BR2_HOSTARCH_NEEDS_IA32_COMPILER if BR2_x86_64=y > help > kvm-unit-tests is a project as old as KVM. As its name > @@ -28,3 +32,7 @@ config BR2_PACKAGE_KVM_UNIT_TESTS > features are submitted with accompanying unit tests. > > http://www.linux-kvm.org/page/KVM-unit-tests > + > +comment "kvm-unit-tests needs a toolchain w/ gcc >= 4.5" > + depends on BR2_i386 > + depends on !BR2_TOOLCHAIN_GCC_AT_LEAST_4_5 What about the x86_64 dependency on a host compiler >= 4.5 ? But those Config.in comments are not really related to the introduction of the _ARCH_SUPPORTS variable, so perhaps they should be in a separate patch. If you don't want to respin but agree with what I said, just let me know, I can fixup when applying. Thanks! Thomas
Thomas, All, On 2017-07-10 18:09 +0200, Thomas Petazzoni spake thusly: > On Sun, 9 Jul 2017 11:29:59 +0200, Yann E. MORIN wrote: > > > diff --git a/package/kvm-unit-tests/Config.in b/package/kvm-unit-tests/Config.in > > index 7eab0c25d6..3db10fc820 100644 > > --- a/package/kvm-unit-tests/Config.in > > +++ b/package/kvm-unit-tests/Config.in [--SNIP--] > > @@ -28,3 +32,7 @@ config BR2_PACKAGE_KVM_UNIT_TESTS > > features are submitted with accompanying unit tests. > > > > http://www.linux-kvm.org/page/KVM-unit-tests > > + > > +comment "kvm-unit-tests needs a toolchain w/ gcc >= 4.5" > > + depends on BR2_i386 > > + depends on !BR2_TOOLCHAIN_GCC_AT_LEAST_4_5 > > What about the x86_64 dependency on a host compiler >= 4.5 ? In the manual, it is said that one should not add a comment about a dependency on the host toolchain... > But those Config.in comments are not really related to the introduction > of the _ARCH_SUPPORTS variable, so perhaps they should be in a separate > patch. Well, the comment did not exist previously indeed, but the dependency on gcc did exist. > If you don't want to respin but agree with what I said, just let > me know, I can fixup when applying. If you want to add this _one_ comment in a separate patch, I'm fine with you splitting it. Regards, Yann E. MORIN. > Thanks! > > Thomas > -- > Thomas Petazzoni, CTO, Free Electrons > Embedded Linux and Kernel engineering > http://free-electrons.com
On 10-07-17 18:20, Yann E. MORIN wrote: > Thomas, All, > > On 2017-07-10 18:09 +0200, Thomas Petazzoni spake thusly: >> On Sun, 9 Jul 2017 11:29:59 +0200, Yann E. MORIN wrote: >> >>> diff --git a/package/kvm-unit-tests/Config.in b/package/kvm-unit-tests/Config.in >>> index 7eab0c25d6..3db10fc820 100644 >>> --- a/package/kvm-unit-tests/Config.in >>> +++ b/package/kvm-unit-tests/Config.in > [--SNIP--] >>> @@ -28,3 +32,7 @@ config BR2_PACKAGE_KVM_UNIT_TESTS >>> features are submitted with accompanying unit tests. >>> >>> http://www.linux-kvm.org/page/KVM-unit-tests >>> + >>> +comment "kvm-unit-tests needs a toolchain w/ gcc >= 4.5" >>> + depends on BR2_i386 >>> + depends on !BR2_TOOLCHAIN_GCC_AT_LEAST_4_5 >> What about the x86_64 dependency on a host compiler >= 4.5 ? > In the manual, it is said that one should not add a comment about a > dependency on the host toolchain... I *think* Thomas didn't mean to say "host compiler". I think he meant: Shouldn't this be: depends on BR2_i386 || BR2_x86_64 depends on !BR2_TOOLCHAIN_GCC_AT_LEAST_4_5 Regards, Arnout [snip]
Arnout, All, On 2017-07-10 21:56 +0200, Arnout Vandecappelle spake thusly: > On 10-07-17 18:20, Yann E. MORIN wrote: > > Thomas, All, > > > > On 2017-07-10 18:09 +0200, Thomas Petazzoni spake thusly: > >> On Sun, 9 Jul 2017 11:29:59 +0200, Yann E. MORIN wrote: > >> > >>> diff --git a/package/kvm-unit-tests/Config.in b/package/kvm-unit-tests/Config.in > >>> index 7eab0c25d6..3db10fc820 100644 > >>> --- a/package/kvm-unit-tests/Config.in > >>> +++ b/package/kvm-unit-tests/Config.in > > [--SNIP--] > >>> @@ -28,3 +32,7 @@ config BR2_PACKAGE_KVM_UNIT_TESTS > >>> features are submitted with accompanying unit tests. > >>> > >>> http://www.linux-kvm.org/page/KVM-unit-tests > >>> + > >>> +comment "kvm-unit-tests needs a toolchain w/ gcc >= 4.5" > >>> + depends on BR2_i386 > >>> + depends on !BR2_TOOLCHAIN_GCC_AT_LEAST_4_5 > >> What about the x86_64 dependency on a host compiler >= 4.5 ? > > In the manual, it is said that one should not add a comment about a > > dependency on the host toolchain... > > I *think* Thomas didn't mean to say "host compiler". I think he meant: > > Shouldn't this be: > depends on BR2_i386 || BR2_x86_64 > depends on !BR2_TOOLCHAIN_GCC_AT_LEAST_4_5 No, because for x86_64 we are using the host compiler. See the .mk file: # For all architectures but x86-64, we use the target # compiler. However, for x86-64, we use the host compiler, as # kvm-unit-tests builds 32 bit code, which Buildroot toolchains for # x86-64 cannot do. ifneq ($(BR2_x86_64),y) KVM_UNIT_TESTS_CONF_OPTS += --cross-prefix="$(TARGET_CROSS)" endif So for x86_64 there is no dependency to add on the target compiler at all. Regards, Yann E. MORIN. > > Regards, > Arnout > > [snip] > > -- > Arnout Vandecappelle arnout at mind be > Senior Embedded Software Architect +32-16-286500 > Essensium/Mind http://www.mind.be > G.Geenslaan 9, 3001 Leuven, Belgium BE 872 984 063 RPR Leuven > LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle > GPG fingerprint: 7493 020B C7E3 8618 8DEC 222C 82EB F404 F9AC 0DDF
On 10-07-17 22:10, Yann E. MORIN wrote: > Arnout, All, > > On 2017-07-10 21:56 +0200, Arnout Vandecappelle spake thusly: >> On 10-07-17 18:20, Yann E. MORIN wrote: >>> Thomas, All, >>> >>> On 2017-07-10 18:09 +0200, Thomas Petazzoni spake thusly: >>>> On Sun, 9 Jul 2017 11:29:59 +0200, Yann E. MORIN wrote: >>>> >>>>> diff --git a/package/kvm-unit-tests/Config.in b/package/kvm-unit-tests/Config.in >>>>> index 7eab0c25d6..3db10fc820 100644 >>>>> --- a/package/kvm-unit-tests/Config.in >>>>> +++ b/package/kvm-unit-tests/Config.in >>> [--SNIP--] >>>>> @@ -28,3 +32,7 @@ config BR2_PACKAGE_KVM_UNIT_TESTS >>>>> features are submitted with accompanying unit tests. >>>>> >>>>> http://www.linux-kvm.org/page/KVM-unit-tests >>>>> + >>>>> +comment "kvm-unit-tests needs a toolchain w/ gcc >= 4.5" >>>>> + depends on BR2_i386 >>>>> + depends on !BR2_TOOLCHAIN_GCC_AT_LEAST_4_5 >>>> What about the x86_64 dependency on a host compiler >= 4.5 ? >>> In the manual, it is said that one should not add a comment about a >>> dependency on the host toolchain... >> >> I *think* Thomas didn't mean to say "host compiler". I think he meant: >> >> Shouldn't this be: >> depends on BR2_i386 || BR2_x86_64 >> depends on !BR2_TOOLCHAIN_GCC_AT_LEAST_4_5 > > No, because for x86_64 we are using the host compiler. See the .mk file: > > # For all architectures but x86-64, we use the target > # compiler. However, for x86-64, we use the host compiler, as > # kvm-unit-tests builds 32 bit code, which Buildroot toolchains for > # x86-64 cannot do. > ifneq ($(BR2_x86_64),y) > KVM_UNIT_TESTS_CONF_OPTS += --cross-prefix="$(TARGET_CROSS)" > endif > > So for x86_64 there is no dependency to add on the target compiler at > all. D'oh, I missed that subtlety, thanks for the explanation. Regards, Arnout
Arnout, All, On 2017-07-10 22:12 +0200, Arnout Vandecappelle spake thusly: > On 10-07-17 22:10, Yann E. MORIN wrote: > > On 2017-07-10 21:56 +0200, Arnout Vandecappelle spake thusly: > >> On 10-07-17 18:20, Yann E. MORIN wrote: > >>> On 2017-07-10 18:09 +0200, Thomas Petazzoni spake thusly: > >>>> On Sun, 9 Jul 2017 11:29:59 +0200, Yann E. MORIN wrote: > >>>>> +comment "kvm-unit-tests needs a toolchain w/ gcc >= 4.5" > >>>>> + depends on BR2_i386 > >>>>> + depends on !BR2_TOOLCHAIN_GCC_AT_LEAST_4_5 > >>>> What about the x86_64 dependency on a host compiler >= 4.5 ? > >>> In the manual, it is said that one should not add a comment about a > >>> dependency on the host toolchain... > >> > >> I *think* Thomas didn't mean to say "host compiler". I think he meant: > >> > >> Shouldn't this be: > >> depends on BR2_i386 || BR2_x86_64 > >> depends on !BR2_TOOLCHAIN_GCC_AT_LEAST_4_5 > > > > No, because for x86_64 we are using the host compiler. See the .mk file: > > > > # For all architectures but x86-64, we use the target > > # compiler. However, for x86-64, we use the host compiler, as > > # kvm-unit-tests builds 32 bit code, which Buildroot toolchains for > > # x86-64 cannot do. > > ifneq ($(BR2_x86_64),y) > > KVM_UNIT_TESTS_CONF_OPTS += --cross-prefix="$(TARGET_CROSS)" > > endif > > > > So for x86_64 there is no dependency to add on the target compiler at > > all. > > D'oh, I missed that subtlety, thanks for the explanation. Yep, no problem. I got bitten by it as well, that's why I "fixed" it (and I should have improved the commit log a bit more, it seems ;-) ). Thanks for the reviews! :-) Regards, Yann E. MORIN. > Regards, > Arnout > > -- > Arnout Vandecappelle arnout at mind be > Senior Embedded Software Architect +32-16-286500 > Essensium/Mind http://www.mind.be > G.Geenslaan 9, 3001 Leuven, Belgium BE 872 984 063 RPR Leuven > LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle > GPG fingerprint: 7493 020B C7E3 8618 8DEC 222C 82EB F404 F9AC 0DDF
Hello, On Mon, 10 Jul 2017 18:20:56 +0200, Yann E. MORIN wrote: > > What about the x86_64 dependency on a host compiler >= 4.5 ? > > In the manual, it is said that one should not add a comment about a > dependency on the host toolchain... The manual says one thing, reality says another: package/collectd/Config.in:comment "write_prometheus needs a toolchain w/ C++, host gcc >= 4.5" package/cppcms/Config.in:comment "icu support needs a toolchain w/ gcc >= 4.8, host gcc >= 4.8" package/icu/Config.in:comment "icu needs a toolchain w/ C++, wchar, threads, gcc >= 4.8, host gcc >= 4.8" package/kodi/Config.in:comment "kodi needs python w/ .py modules, a uClibc or glibc toolchain w/ C++, locale, threads, wchar, dynamic library, gcc >= 4.8, host gcc >= 4.6" package/mfgtools/Config.in.host:comment "mfgtools needs host gcc >= 4.8" package/midori/Config.in:comment "midori needs libgtk3 and a glibc toolchain w/ C++, gcc >= 4.9, host gcc >= 4.8" package/php/Config.ext:comment "intl support needs a toolchain w/ C++, wchar, threads, gcc >= 4.8, host gcc >= 4.8" package/protobuf-c/Config.in:comment "protobuf-c needs a toolchain w/ C++, threads, host gcc >= 4.5" package/python-mwscrape2slob/Config.in:comment "python-mwscrape2slob needs a toolchain w/ C++, gcc >= 4.8, host gcc >= 4.8" package/python-pyicu/Config.in:comment "python-pyicu needs a toolchain w/ C++, gcc >= 4.8, host gcc >= 4.8" package/python-slob/Config.in:comment "python-slob needs a toolchain w/ C++, gcc >= 4.8, host gcc >= 4.8" package/qt5/qt5base/Config.in:comment "icu support needs a toolchain w/ gcc >= 4.8, host gcc >= 4.8" package/qt5/qt5webkit/Config.in:comment "qt5webkit needs a toolchain w/ dynamic library, gcc >= 4.8, host gcc >= 4.8" package/riemann-c-client/Config.in:comment "riemann-c-client needs a toolchain w/ C++, threads, host gcc >= 4.5" package/webkitgtk/Config.in:comment "webkitgtk needs libgtk3 and a glibc toolchain w/ C++, gcc >= 4.9, host gcc >= 4.8" I'm not sure why we would not add Config.in comment for dependencies on the host toolchain. What's the reasoning for *not* adding such dependencies ? Best regards, Thomas
Thomas, All, On 2017-07-10 23:47 +0200, Thomas Petazzoni spake thusly: > On Mon, 10 Jul 2017 18:20:56 +0200, Yann E. MORIN wrote: > > > What about the x86_64 dependency on a host compiler >= 4.5 ? > > In the manual, it is said that one should not add a comment about a > > dependency on the host toolchain... > > The manual says one thing, reality says another: > > package/collectd/Config.in:comment "write_prometheus needs a toolchain w/ C++, host gcc >= 4.5" > package/cppcms/Config.in:comment "icu support needs a toolchain w/ gcc >= 4.8, host gcc >= 4.8" > package/icu/Config.in:comment "icu needs a toolchain w/ C++, wchar, threads, gcc >= 4.8, host gcc >= 4.8" > package/kodi/Config.in:comment "kodi needs python w/ .py modules, a uClibc or glibc toolchain w/ C++, locale, threads, wchar, dynamic library, gcc >= 4.8, host gcc >= 4.6" > package/mfgtools/Config.in.host:comment "mfgtools needs host gcc >= 4.8" > package/midori/Config.in:comment "midori needs libgtk3 and a glibc toolchain w/ C++, gcc >= 4.9, host gcc >= 4.8" > package/php/Config.ext:comment "intl support needs a toolchain w/ C++, wchar, threads, gcc >= 4.8, host gcc >= 4.8" > package/protobuf-c/Config.in:comment "protobuf-c needs a toolchain w/ C++, threads, host gcc >= 4.5" > package/python-mwscrape2slob/Config.in:comment "python-mwscrape2slob needs a toolchain w/ C++, gcc >= 4.8, host gcc >= 4.8" > package/python-pyicu/Config.in:comment "python-pyicu needs a toolchain w/ C++, gcc >= 4.8, host gcc >= 4.8" > package/python-slob/Config.in:comment "python-slob needs a toolchain w/ C++, gcc >= 4.8, host gcc >= 4.8" > package/qt5/qt5base/Config.in:comment "icu support needs a toolchain w/ gcc >= 4.8, host gcc >= 4.8" > package/qt5/qt5webkit/Config.in:comment "qt5webkit needs a toolchain w/ dynamic library, gcc >= 4.8, host gcc >= 4.8" > package/riemann-c-client/Config.in:comment "riemann-c-client needs a toolchain w/ C++, threads, host gcc >= 4.5" > package/webkitgtk/Config.in:comment "webkitgtk needs libgtk3 and a glibc toolchain w/ C++, gcc >= 4.9, host gcc >= 4.8" > > I'm not sure why we would not add Config.in comment for dependencies on > the host toolchain. What's the reasoning for *not* adding such > dependencies ? Because there is not much a user can do about the toolchain on host host machine? As much as it is "possible" to change the version for the cross-gcc, the native compiler is often not replaceable. This is most probably the case on enterprise-grade servers, where users are not root (and are stuck with a precambrian host compiler)... I wrote the comment, checked the manual for the template, then removed the comment. Oh well, here it is again, then: comment "kvm-unit-tests needs a host gcc >= 4.5" depends on BR2_x86_64 depends on !BR2_HOST_GCC_AT_LEAST_4_5 Hehe! ;-) Regards, Yann E. MORIN. > Best regards, > > Thomas > -- > Thomas Petazzoni, CTO, Free Electrons > Embedded Linux and Kernel engineering > http://free-electrons.com
On 10-07-17 23:57, Yann E. MORIN wrote: > As much as it is "possible" to change the version for the cross-gcc, the > native compiler is often not replaceable. This is most probably the case > on enterprise-grade servers, where users are not root (and are stuck > with a precambrian host compiler)... Building a native compiler from source is annoying, but not impossible. Libc is a different story, but a compiler is feasible. In addition, many users can indeed install a decent compiler either from the distro or from a PPA, so really it does make sense to have such a comment. What you *really* can't change is your target architecture and its features. Regards, Arnout
Hello, On Sun, 9 Jul 2017 11:29:59 +0200, Yann E. MORIN wrote: > + depends on BR2_TOOLCHAIN_GCC_AT_LEAST_4_5 if BR2_i386 > + depends on BR2_HOST_GCC_AT_LEAST_4_5 if BR2_x86_64 Did you actually test this? "make menuconfig" complains: thomas@windsurf:~/projets/buildroot (master)$ make menuconfig package/kvm-unit-tests/Config.in:16: syntax error package/kvm-unit-tests/Config.in:15: invalid option package/kvm-unit-tests/Config.in:17: syntax error package/kvm-unit-tests/Config.in:16: invalid option Makefile:876: recipe for target 'menuconfig' failed So I've replaced them with the more usual: + depends on BR2_TOOLCHAIN_GCC_AT_LEAST_4_5 || !BR2_i386 + depends on BR2_HOST_GCC_AT_LEAST_4_5 || !BR2_x86_64 I've also re-add the Config.in comment about the host gcc >= 4.5 dependency. Applied with those changes. Thanks! Thomas
Thomas, All, On 2017-07-22 23:28 +0200, Thomas Petazzoni spake thusly: > On Sun, 9 Jul 2017 11:29:59 +0200, Yann E. MORIN wrote: > > + depends on BR2_TOOLCHAIN_GCC_AT_LEAST_4_5 if BR2_i386 > > + depends on BR2_HOST_GCC_AT_LEAST_4_5 if BR2_x86_64 > > Did you actually test this? "make menuconfig" complains: > > thomas@windsurf:~/projets/buildroot (master)$ make menuconfig > package/kvm-unit-tests/Config.in:16: syntax error > package/kvm-unit-tests/Config.in:15: invalid option > package/kvm-unit-tests/Config.in:17: syntax error > package/kvm-unit-tests/Config.in:16: invalid option > Makefile:876: recipe for target 'menuconfig' failed I am really surprised, because I am sure I did test it, yes. But apparently, I did not. I have no idea about what hapenned... :-/ > So I've replaced them with the more usual: > > + depends on BR2_TOOLCHAIN_GCC_AT_LEAST_4_5 || !BR2_i386 > + depends on BR2_HOST_GCC_AT_LEAST_4_5 || !BR2_x86_64 > > I've also re-add the Config.in comment about the host gcc >= 4.5 > dependency. > > Applied with those changes. Thanks! Thanks! Regards, Yann E. MORIN.
diff --git a/package/kvm-unit-tests/Config.in b/package/kvm-unit-tests/Config.in index 7eab0c25d6..3db10fc820 100644 --- a/package/kvm-unit-tests/Config.in +++ b/package/kvm-unit-tests/Config.in @@ -1,15 +1,19 @@ +config BR2_PACKAGE_KVM_UNIT_TESTS_ARCH_SUPPORTS + bool + # On ARM, it uses virtualization extensions + default y if BR2_cortex_a7 || BR2_cortex_a12 || \ + BR2_cortex_a15 || BR2_cortex_a17 + default y if BR2_i386 || BR2_x86_64 + default y if BR2_powerpc64 || BR2_powerpc64le + config BR2_PACKAGE_KVM_UNIT_TESTS bool "kvm-unit-tests" + depends on BR2_PACKAGE_KVM_UNIT_TESTS_ARCH_SUPPORTS # on i386 and x86-64, __builtin_reachable is used, so we need # gcc 4.5 at least. on i386, we use the target gcc, while on # x86-64 we use the host gcc (see .mk file for details) - # On ARM, it uses virtualization extensions - depends on BR2_cortex_a7 || BR2_cortex_a12 || \ - BR2_cortex_a15 || BR2_cortex_a17 || \ - (BR2_i386 && BR2_TOOLCHAIN_GCC_AT_LEAST_4_5) || \ - BR2_powerpc64 || \ - BR2_powerpc64le || \ - (BR2_x86_64 && BR2_HOST_GCC_AT_LEAST_4_5) + depends on BR2_TOOLCHAIN_GCC_AT_LEAST_4_5 if BR2_i386 + depends on BR2_HOST_GCC_AT_LEAST_4_5 if BR2_x86_64 select BR2_HOSTARCH_NEEDS_IA32_COMPILER if BR2_x86_64=y help kvm-unit-tests is a project as old as KVM. As its name @@ -28,3 +32,7 @@ config BR2_PACKAGE_KVM_UNIT_TESTS features are submitted with accompanying unit tests. http://www.linux-kvm.org/page/KVM-unit-tests + +comment "kvm-unit-tests needs a toolchain w/ gcc >= 4.5" + depends on BR2_i386 + depends on !BR2_TOOLCHAIN_GCC_AT_LEAST_4_5
Move all architecture options to their own symbol, so that it is easier to add more variants in the future. The dependency on cross-gcc >= 4.5 is only valid for i386, as we use the host gcc for x86_64. Adapt the dependency accordingly. Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr> Cc: Cyril Bur <cyrilbur@gmail.com> --- Changes v2 -> v3: - move gcc dependency back to the main symbol (Arnout) - add comment (Arnout) - fix dependency on gcc version --- package/kvm-unit-tests/Config.in | 22 +++++++++++++++------- 1 file changed, 15 insertions(+), 7 deletions(-)