[gentoo-dev] [RFC] Enable CMAKE_WARN_UNUSED_CLI by default in cmake-utils for EAPI>=6

[gentoo-dev] [RFC] Enable CMAKE_WARN_UNUSED_CLI by default in cmake-utils for EAPI>=6

[Maciej Mrozowski]

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

Hello,

General advise: do not convert ebuilds inheriting cmake-utils to EAPI 6 unless 
you know what you are doing (you are fully aware of eclass behaviour removed 
with https://bugs.gentoo.org/show_bug.cgi?id=514384).

Background:

Pre EAPI-6 cmake-utils.eclass contained certain feature to mitigate CMake 
variable case changes done by upstream.
This feature was explicitly removed with 
https://bugs.gentoo.org/show_bug.cgi?id=514384 and no alternative was 
proposed.
It opened new area of possible ebuild regression bugs when switching to EAPI-6 
for ebuilds inheriting cmake-utils.eclass.

Unfortunately there is common misconception, also among developers, that it's 
sufficient to simply replace "${cmake-utils_use_with foo)" with "-DWITH_foo=ON" 
etc.
This is MOST OF THE TIME not the case.
When converting cmake-utils ebuild to EAPI>=6, one needs to consult 
CMakeLists.txt wrt case each variable is written with since CMake is case-
sensitive and WITH_FOO != WITH_foo != WITH_Foo.

Proposal:

CMake allows warning about unused CMake variables passed by CLI. Since this is 
how Gentoo passes ebuild configuration options, it's proposed to enable this 
feature.
Unfortunately it won't fail compilation but at least it gives a chance to spot 
case mismatch when reading build output.

Future thoughts:

For better damage control it's technically possible to extend configure phase 
of cmake-utuls eclass to check mycmakeargs against parsed package buildsystem 
but this might not be very reliable.

regards
MM

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

[gentoo-dev] Re: [RFC] Enable CMAKE_WARN_UNUSED_CLI by default in cmake-utils for EAPI>=6

[Maciej Mrozowski]

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

On Monday 02 of May 2016 18:06:44 you wrote:
> Unfortunately there is common misconception, also among developers, that
> it's sufficient to simply replace "${cmake-utils_use_with foo)" with
> "-DWITH_foo=ON" etc.

Obvious errata, should be:
Unfortunately there is common misconception, also among developers, that it's 
sufficient to simply replace "${cmake-utils_use_with foo)" with "-
DWITH_foo=$(usex foo)" etc.

regards
MM

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

[gentoo-dev] Re: [RFC] Enable CMAKE_WARN_UNUSED_CLI by default in cmake-utils for EAPI>=6

[Maciej Mrozowski]

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

On Monday 02 of May 2016 18:13:38 you wrote:
> On Monday 02 of May 2016 18:06:44 you wrote:
> > Unfortunately there is common misconception, also among developers, that
> > it's sufficient to simply replace "${cmake-utils_use_with foo)" with
> > "-DWITH_foo=ON" etc.
> 
> Obvious errata, should be:
> Unfortunately there is common misconception, also among developers, that
> it's sufficient to simply replace "${cmake-utils_use_with foo)" with "-
> DWITH_foo=$(usex foo)" etc.

And commited:

diff --git a/eclass/cmake-utils.eclass b/eclass/cmake-utils.eclass
index ebcb631..427c13f 100644
--- a/eclass/cmake-utils.eclass
+++ b/eclass/cmake-utils.eclass
@@ -87,7 +87,6 @@ _CMAKE_UTILS_ECLASS=1
 # Warn about variables that are declared on the command line
 # but not used. Might give false-positives.
 # "no" to disable (default) or anything else to enable.
-: ${CMAKE_WARN_UNUSED_CLI:=no}
 
 # @ECLASS-VARIABLE: PREFIX
 # @DESCRIPTION:
@@ -113,7 +112,8 @@ _CMAKE_UTILS_ECLASS=1
 # Should be set by user in a per-package basis in /etc/portage/package.env.
 
 case ${EAPI} in
-       2|3|4|5|6) : ;;
+       2|3|4|5) : ${CMAKE_WARN_UNUSED_CLI:=no} ;;
+       6) : ${CMAKE_WARN_UNUSED_CLI:=yes} ;;
        *) die "EAPI=${EAPI:-0} is not supported" ;;
 esac

regards
MM

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

Re: [gentoo-dev] [RFC] Enable CMAKE_WARN_UNUSED_CLI by default in cmake-utils for EAPI>=6

[Andrew Savchenko]

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

On Mon, 02 May 2016 18:06:44 +0200 Maciej Mrozowski wrote:
> Hello,
> 
> General advise: do not convert ebuilds inheriting cmake-utils to EAPI 6 unless 
> you know what you are doing (you are fully aware of eclass behaviour removed 
> with https://bugs.gentoo.org/show_bug.cgi?id=514384).
> 
> Background:
> 
> Pre EAPI-6 cmake-utils.eclass contained certain feature to mitigate CMake 
> variable case changes done by upstream.
> This feature was explicitly removed with 
> https://bugs.gentoo.org/show_bug.cgi?id=514384 and no alternative was 
> proposed.
> It opened new area of possible ebuild regression bugs when switching to EAPI-6 
> for ebuilds inheriting cmake-utils.eclass.
> 
> Unfortunately there is common misconception, also among developers, that it's 
> sufficient to simply replace "${cmake-utils_use_with foo)" with "-DWITH_foo=ON" 
> etc.
> This is MOST OF THE TIME not the case.
> When converting cmake-utils ebuild to EAPI>=6, one needs to consult 
> CMakeLists.txt wrt case each variable is written with since CMake is case-
> sensitive and WITH_FOO != WITH_foo != WITH_Foo.
> 
> Proposal:
> 
> CMake allows warning about unused CMake variables passed by CLI. Since this is 
> how Gentoo passes ebuild configuration options, it's proposed to enable this 
> feature.
> Unfortunately it won't fail compilation but at least it gives a chance to spot 
> case mismatch when reading build output.
> 
> Future thoughts:
> 
> For better damage control it's technically possible to extend configure phase 
> of cmake-utuls eclass to check mycmakeargs against parsed package buildsystem 
> but this might not be very reliable.

For me the real confusion was from this line:

    die "${FUNCNAME[1]} is banned in EAPI 6 and later: use -D$1${arg}=\"\$(usex $2)\" instead"

It recommends to use ${arg} without any warning about case, so when I just
copied what it recommends: -DWITH_nls="$(usex nls)", I had a nice surprise
and fun debugging.

Best regards,
Andrew Savchenko

[-- Attachment #2: Type: application/pgp-signature, Size: 819 bytes --]

Re: [gentoo-dev] [RFC] Enable CMAKE_WARN_UNUSED_CLI by default in cmake-utils for EAPI>=6

[Maciej Mrozowski]

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

On Wednesday 18 of May 2016 09:22:53 Andrew Savchenko wrote:
> On Mon, 02 May 2016 18:06:44 +0200 Maciej Mrozowski wrote:
> > Hello,
> > 
> > General advise: do not convert ebuilds inheriting cmake-utils to EAPI 6
> > unless you know what you are doing (you are fully aware of eclass
> > behaviour removed with https://bugs.gentoo.org/show_bug.cgi?id=514384).
> > 
> > Background:
> > 
> > Pre EAPI-6 cmake-utils.eclass contained certain feature to mitigate CMake
> > variable case changes done by upstream.
> > This feature was explicitly removed with
> > https://bugs.gentoo.org/show_bug.cgi?id=514384 and no alternative was
> > proposed.
> > It opened new area of possible ebuild regression bugs when switching to
> > EAPI-6 for ebuilds inheriting cmake-utils.eclass.
> > 
> > Unfortunately there is common misconception, also among developers, that
> > it's sufficient to simply replace "${cmake-utils_use_with foo)" with
> > "-DWITH_foo=ON" etc.
> > This is MOST OF THE TIME not the case.
> > When converting cmake-utils ebuild to EAPI>=6, one needs to consult
> > CMakeLists.txt wrt case each variable is written with since CMake is case-
> > sensitive and WITH_FOO != WITH_foo != WITH_Foo.
> > 
> > Proposal:
> > 
> > CMake allows warning about unused CMake variables passed by CLI. Since
> > this is how Gentoo passes ebuild configuration options, it's proposed to
> > enable this feature.
> > Unfortunately it won't fail compilation but at least it gives a chance to
> > spot case mismatch when reading build output.
> > 
> > Future thoughts:
> > 
> > For better damage control it's technically possible to extend configure
> > phase of cmake-utuls eclass to check mycmakeargs against parsed package
> > buildsystem but this might not be very reliable.
> 
> For me the real confusion was from this line:
> 
>     die "${FUNCNAME[1]} is banned in EAPI 6 and later: use
> -D$1${arg}=\"\$(usex $2)\" instead"
> 
> It recommends to use ${arg} without any warning about case, so when I just
> copied what it recommends: -DWITH_nls="$(usex nls)", I had a nice surprise
> and fun debugging.

Ah, there you go..

@kensington
Come on, man, you should have known better.

Invalid suggestion removed. Thanks for noticing.
I prefer to have developers figure out the right EAPI-6 migration path 
themselves rather than blindly relying on suggestions:

diff --git a/eclass/cmake-utils.eclass b/eclass/cmake-utils.eclass
index 427c13f..5958230 100644
--- a/eclass/cmake-utils.eclass
+++ b/eclass/cmake-utils.eclass
@@ -161,7 +161,7 @@ _cmake_use_me_now() {
        local arg=$2
        [[ ! -z $3 ]] && arg=$3
 
-       [[ ${EAPI} == [2345] ]] || die "${FUNCNAME[1]} is banned in EAPI 6 and 
later: use -D$1${arg}=\"\$(usex $2)\" instead"
+       [[ ${EAPI} == [2345] ]] || die "${FUNCNAME[1]} is banned in EAPI 6 and 
later"
 
        local uper capitalised x
        [[ -z $2 ]] && die "cmake-utils_use-$1 <USE flag> [<flag name>]"
@@ -184,7 +184,7 @@ _cmake_use_me_now_inverted() {
        [[ ! -z $3 ]] && arg=$3
 
        if [[ ${EAPI} != [2345] && "${FUNCNAME[1]}" != cmake-
utils_use_find_package ]] ; then
-               die "${FUNCNAME[1]} is banned in EAPI 6 and later: use -
D$1${arg}=\"\$(usex $2)\" instead"
+               die "${FUNCNAME[1]} is banned in EAPI 6 and later"
        fi
 
        local uper capitalised x

regards
MM

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 181 bytes --]