[gentoo-dev] [PATCH] cmake-utils.eclass: die if ninja is enabled but not installed

[gentoo-dev] [PATCH] cmake-utils.eclass: die if ninja is enabled but not installed

[Michael Palimaka]

This could happen if ninja is manually enabled (eg. make.conf) but not installed
---
 eclass/cmake-utils.eclass | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/eclass/cmake-utils.eclass b/eclass/cmake-utils.eclass
index 480cd09..012b13f 100644
--- a/eclass/cmake-utils.eclass
+++ b/eclass/cmake-utils.eclass
@@ -228,6 +228,11 @@ _generator_to_use() {
 
 	case ${CMAKE_MAKEFILE_GENERATOR} in
 		ninja)
+			# if ninja is enabled but not installed, the build could fail
+			# this could happen if ninja is manually enabled (eg. make.conf) but not installed
+			if ! has_version dev-util/ninja; then
+				die "CMAKE_MAKEFILE_GENERATOR is set to ninja, but ninja is not installed. Please install dev-util/ninja or unset CMAKE_MAKEFILE_GENERATOR."
+			fi
 			generator_name="Ninja"
 			;;
 		emake)
-- 
2.4.9

Re: [gentoo-dev] [PATCH] cmake-utils.eclass: die if ninja is enabled but not installed

[Michał Górny]

Dnia 15 października 2015 17:44:47 CEST, Michael Palimaka <kensington@gentoo.org> napisał(a):
>This could happen if ninja is manually enabled (eg. make.conf) but not
>installed
>---
> eclass/cmake-utils.eclass | 5 +++++
> 1 file changed, 5 insertions(+)
>
>diff --git a/eclass/cmake-utils.eclass b/eclass/cmake-utils.eclass
>index 480cd09..012b13f 100644
>--- a/eclass/cmake-utils.eclass
>+++ b/eclass/cmake-utils.eclass
>@@ -228,6 +228,11 @@ _generator_to_use() {
> 
> 	case ${CMAKE_MAKEFILE_GENERATOR} in
> 		ninja)
>+			# if ninja is enabled but not installed, the build could fail
>+			# this could happen if ninja is manually enabled (eg. make.conf)
>but not installed
>+			if ! has_version dev-util/ninja; then

I'd suggest avoiding has_version and just checking for the binary. type -P, I think. Ciaran can give you the rationale, I believe.

>+				die "CMAKE_MAKEFILE_GENERATOR is set to ninja, but ninja is not
>installed. Please install dev-util/ninja or unset
>CMAKE_MAKEFILE_GENERATOR."
>+			fi
> 			generator_name="Ninja"
> 			;;
> 		emake)

-- 
Best regards,
Michał Górny

[gentoo-dev] Re: [PATCH] cmake-utils.eclass: die if ninja is enabled but not installed

[Michael Palimaka]

On 16/10/15 03:04, Michał Górny wrote:
> 
> 
> Dnia 15 października 2015 17:44:47 CEST, Michael Palimaka <kensington@gentoo.org> napisał(a):
>> This could happen if ninja is manually enabled (eg. make.conf) but not
>> installed
>> ---
>> eclass/cmake-utils.eclass | 5 +++++
>> 1 file changed, 5 insertions(+)
>>
>> diff --git a/eclass/cmake-utils.eclass b/eclass/cmake-utils.eclass
>> index 480cd09..012b13f 100644
>> --- a/eclass/cmake-utils.eclass
>> +++ b/eclass/cmake-utils.eclass
>> @@ -228,6 +228,11 @@ _generator_to_use() {
>>
>> 	case ${CMAKE_MAKEFILE_GENERATOR} in
>> 		ninja)
>> +			# if ninja is enabled but not installed, the build could fail
>> +			# this could happen if ninja is manually enabled (eg. make.conf)
>> but not installed
>> +			if ! has_version dev-util/ninja; then
> 
> I'd suggest avoiding has_version and just checking for the binary. type -P, I think. Ciaran can give you the rationale, I believe.

There's no guarantee that the binary will be provided by dev-util/ninja
(we've had a bug about this already).

Re: [gentoo-dev] Re: [PATCH] cmake-utils.eclass: die if ninja is enabled but not installed

[Michał Górny]

[-- Attachment #1: Type: text/plain, Size: 1366 bytes --]

On Fri, 16 Oct 2015 03:22:48 +1100
Michael Palimaka <kensington@gentoo.org> wrote:

> On 16/10/15 03:04, Michał Górny wrote:
> > 
> > 
> > Dnia 15 października 2015 17:44:47 CEST, Michael Palimaka <kensington@gentoo.org> napisał(a):  
> >> This could happen if ninja is manually enabled (eg. make.conf) but not
> >> installed
> >> ---
> >> eclass/cmake-utils.eclass | 5 +++++
> >> 1 file changed, 5 insertions(+)
> >>
> >> diff --git a/eclass/cmake-utils.eclass b/eclass/cmake-utils.eclass
> >> index 480cd09..012b13f 100644
> >> --- a/eclass/cmake-utils.eclass
> >> +++ b/eclass/cmake-utils.eclass
> >> @@ -228,6 +228,11 @@ _generator_to_use() {
> >>
> >> 	case ${CMAKE_MAKEFILE_GENERATOR} in
> >> 		ninja)
> >> +			# if ninja is enabled but not installed, the build could fail
> >> +			# this could happen if ninja is manually enabled (eg. make.conf)
> >> but not installed
> >> +			if ! has_version dev-util/ninja; then  
> > 
> > I'd suggest avoiding has_version and just checking for the binary. type -P, I think. Ciaran can give you the rationale, I believe.  
> 
> There's no guarantee that the binary will be provided by dev-util/ninja
> (we've had a bug about this already).

Excuse me but did you agree with me, then commit the old version anyway?

-- 
Best regards,
Michał Górny
<http://dev.gentoo.org/~mgorny/>

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 949 bytes --]

Re: [gentoo-dev] Re: [PATCH] cmake-utils.eclass: die if ninja is enabled but not installed

[Matt Turner]

On Sat, Oct 17, 2015 at 10:37 AM, Michał Górny <mgorny@gentoo.org> wrote:
> On Fri, 16 Oct 2015 03:22:48 +1100
> Michael Palimaka <kensington@gentoo.org> wrote:
>
>> On 16/10/15 03:04, Michał Górny wrote:
>> >
>> >
>> > Dnia 15 października 2015 17:44:47 CEST, Michael Palimaka <kensington@gentoo.org> napisał(a):
>> >> This could happen if ninja is manually enabled (eg. make.conf) but not
>> >> installed
>> >> ---
>> >> eclass/cmake-utils.eclass | 5 +++++
>> >> 1 file changed, 5 insertions(+)
>> >>
>> >> diff --git a/eclass/cmake-utils.eclass b/eclass/cmake-utils.eclass
>> >> index 480cd09..012b13f 100644
>> >> --- a/eclass/cmake-utils.eclass
>> >> +++ b/eclass/cmake-utils.eclass
>> >> @@ -228,6 +228,11 @@ _generator_to_use() {
>> >>
>> >>    case ${CMAKE_MAKEFILE_GENERATOR} in
>> >>            ninja)
>> >> +                  # if ninja is enabled but not installed, the build could fail
>> >> +                  # this could happen if ninja is manually enabled (eg. make.conf)
>> >> but not installed
>> >> +                  if ! has_version dev-util/ninja; then
>> >
>> > I'd suggest avoiding has_version and just checking for the binary. type -P, I think. Ciaran can give you the rationale, I believe.
>>
>> There's no guarantee that the binary will be provided by dev-util/ninja
>> (we've had a bug about this already).
>
> Excuse me but did you agree with me, then commit the old version anyway?

I didn't seem like he agreed with you. He explained that checking for
a binary named ninja isn't sufficient because some versions of
net-irc/ninja install a 'ninja' binary. See bug
https://bugs.gentoo.org/show_bug.cgi?id=436804

Re: [gentoo-dev] Re: [PATCH] cmake-utils.eclass: die if ninja is enabled but not installed

[Michał Górny]

[-- Attachment #1: Type: text/plain, Size: 2330 bytes --]

On Sat, 17 Oct 2015 10:49:13 -0700
Matt Turner <mattst88@gentoo.org> wrote:

> On Sat, Oct 17, 2015 at 10:37 AM, Michał Górny <mgorny@gentoo.org> wrote:
> > On Fri, 16 Oct 2015 03:22:48 +1100
> > Michael Palimaka <kensington@gentoo.org> wrote:
> >  
> >> On 16/10/15 03:04, Michał Górny wrote:  
> >> >
> >> >
> >> > Dnia 15 października 2015 17:44:47 CEST, Michael Palimaka <kensington@gentoo.org> napisał(a):  
> >> >> This could happen if ninja is manually enabled (eg. make.conf) but not
> >> >> installed
> >> >> ---
> >> >> eclass/cmake-utils.eclass | 5 +++++
> >> >> 1 file changed, 5 insertions(+)
> >> >>
> >> >> diff --git a/eclass/cmake-utils.eclass b/eclass/cmake-utils.eclass
> >> >> index 480cd09..012b13f 100644
> >> >> --- a/eclass/cmake-utils.eclass
> >> >> +++ b/eclass/cmake-utils.eclass
> >> >> @@ -228,6 +228,11 @@ _generator_to_use() {
> >> >>
> >> >>    case ${CMAKE_MAKEFILE_GENERATOR} in
> >> >>            ninja)
> >> >> +                  # if ninja is enabled but not installed, the build could fail
> >> >> +                  # this could happen if ninja is manually enabled (eg. make.conf)
> >> >> but not installed
> >> >> +                  if ! has_version dev-util/ninja; then  
> >> >
> >> > I'd suggest avoiding has_version and just checking for the binary. type -P, I think. Ciaran can give you the rationale, I believe.  
> >>
> >> There's no guarantee that the binary will be provided by dev-util/ninja
> >> (we've had a bug about this already).  
> >
> > Excuse me but did you agree with me, then commit the old version anyway?  
> 
> I didn't seem like he agreed with you. He explained that checking for
> a binary named ninja isn't sufficient because some versions of
> net-irc/ninja install a 'ninja' binary. See bug
> https://bugs.gentoo.org/show_bug.cgi?id=436804

Well, he didn't make it clear it's about *another* binary. In any case,
two wrongs don't make a right. has_version() should really be avoided.
In any case, I'd even prefer doing negative has_version() for known-bad
net-irc/ninja (assuming we really need to care about it still) as
has_version check failing in this case would be much less irritating
that has_version check required to proceed.

-- 
Best regards,
Michał Górny
<http://dev.gentoo.org/~mgorny/>

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 949 bytes --]