public inbox for gentoo-dev@lists.gentoo.org
 help / color / mirror / Atom feed
* [gentoo-dev] [PATCH] Create default implementation of pkg_pretend, move CONFIG_CHECK
@ 2021-05-30 17:29 mpagano
  2021-05-30 17:33 ` Michał Górny
  2021-05-30 18:28 ` Ionen Wolkens
  0 siblings, 2 replies; 6+ messages in thread
From: mpagano @ 2021-05-30 17:29 UTC (permalink / raw
  To: gentoo-dev; +Cc: Mike Pagano

From: Mike Pagano <mpagano@gentoo.org>

As the purpose of pkg_pretend is to run sanity checks during
dependency calculation time, provide the default implementation
and perform CONFIG_CHECK within it.

See bug #759238

Signed-off-by: Mike Pagano <mpagano@gentoo.org>
---
 eclass/linux-mod.eclass | 17 ++++++++++++-----
 1 file changed, 12 insertions(+), 5 deletions(-)

diff --git a/eclass/linux-mod.eclass b/eclass/linux-mod.eclass
index 11b0fd0cf..42e541ed1 100644
--- a/eclass/linux-mod.eclass
+++ b/eclass/linux-mod.eclass
@@ -135,7 +135,7 @@
 # It's a read-only variable. It contains the extension of the kernel modules.
 
 inherit eutils linux-info multilib toolchain-funcs
-EXPORT_FUNCTIONS pkg_setup pkg_preinst pkg_postinst src_install src_compile pkg_postrm
+EXPORT_FUNCTIONS pkg_setup pkg_preinst pkg_pretend pkg_postinst src_install src_compile pkg_postrm
 
 case ${MODULES_OPTIONAL_USE_IUSE_DEFAULT:-n} in
   [nNfF]*|[oO][fF]*|0|-) _modules_optional_use_iuse_default='' ;;
@@ -157,7 +157,7 @@ RDEPEND="
 		)
 	${MODULES_OPTIONAL_USE:+)}"
 DEPEND="${RDEPEND}
-    ${MODULES_OPTIONAL_USE}${MODULES_OPTIONAL_USE:+? (}
+	${MODULES_OPTIONAL_USE}${MODULES_OPTIONAL_USE:+? (}
 	sys-apps/sed
 	kernel_linux? ( virtual/linux-sources virtual/libelf )
 	${MODULES_OPTIONAL_USE:+)}"
@@ -567,6 +567,16 @@ find_module_params() {
 # default ebuild functions
 # --------------------------------
 
+# @FUNCTION: linux-mod_pkg_pretend
+# @DESCRIPTION:
+# Check the CONFIG_CHECK options 
+linux-mod_pkg_pretend() {
+
+	debug-print-function ${FUNCNAME} $*
+	# External modules use kernel symbols (bug #591832, #759238)
+	CONFIG_CHECK+=" !TRIM_UNUSED_KSYMS"
+}
+
 # @FUNCTION: linux-mod_pkg_setup
 # @DESCRIPTION:
 # It checks the CONFIG_CHECK options (see linux-info.eclass(5)), verifies that the kernel is
@@ -589,9 +599,6 @@ linux-mod_pkg_setup() {
 		return
 	fi
 
-	# External modules use kernel symbols (bug #591832)
-	CONFIG_CHECK+=" !TRIM_UNUSED_KSYMS"
-
 	linux-info_pkg_setup;
 	require_configured_kernel
 	check_kernel_built;
-- 
2.31.1



^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [gentoo-dev] [PATCH] Create default implementation of pkg_pretend, move CONFIG_CHECK
  2021-05-30 17:29 [gentoo-dev] [PATCH] Create default implementation of pkg_pretend, move CONFIG_CHECK mpagano
@ 2021-05-30 17:33 ` Michał Górny
  2021-05-30 18:28 ` Ionen Wolkens
  1 sibling, 0 replies; 6+ messages in thread
From: Michał Górny @ 2021-05-30 17:33 UTC (permalink / raw
  To: gentoo-dev; +Cc: Mike Pagano

On Sun, 2021-05-30 at 13:29 -0400, mpagano@gentoo.org wrote:
> From: Mike Pagano <mpagano@gentoo.org>
> 
> As the purpose of pkg_pretend is to run sanity checks during
> dependency calculation time, provide the default implementation
> and perform CONFIG_CHECK within it.
> 
> See bug #759238
> 
> Signed-off-by: Mike Pagano <mpagano@gentoo.org>
> ---
>  eclass/linux-mod.eclass | 17 ++++++++++++-----
>  1 file changed, 12 insertions(+), 5 deletions(-)
> 
> diff --git a/eclass/linux-mod.eclass b/eclass/linux-mod.eclass
> index 11b0fd0cf..42e541ed1 100644
> --- a/eclass/linux-mod.eclass
> +++ b/eclass/linux-mod.eclass
> @@ -135,7 +135,7 @@
>  # It's a read-only variable. It contains the extension of the kernel modules.
>  
>  inherit eutils linux-info multilib toolchain-funcs
> -EXPORT_FUNCTIONS pkg_setup pkg_preinst pkg_postinst src_install src_compile pkg_postrm
> +EXPORT_FUNCTIONS pkg_setup pkg_preinst pkg_pretend pkg_postinst src_install src_compile pkg_postrm
>  

Have you verified that this doesn't override pkg_pretend in any
of the eclasses used by the existing consumers?

-- 
Best regards,
Michał Górny




^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [gentoo-dev] [PATCH] Create default implementation of pkg_pretend, move CONFIG_CHECK
  2021-05-30 17:29 [gentoo-dev] [PATCH] Create default implementation of pkg_pretend, move CONFIG_CHECK mpagano
  2021-05-30 17:33 ` Michał Górny
@ 2021-05-30 18:28 ` Ionen Wolkens
  2021-05-30 18:42   ` Mike
  1 sibling, 1 reply; 6+ messages in thread
From: Ionen Wolkens @ 2021-05-30 18:28 UTC (permalink / raw
  To: gentoo-dev

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

On Sun, May 30, 2021 at 01:29:12PM -0400, mpagano@gentoo.org wrote:
> From: Mike Pagano <mpagano@gentoo.org>
> 
> As the purpose of pkg_pretend is to run sanity checks during
> dependency calculation time, provide the default implementation
> and perform CONFIG_CHECK within it.
> 
> See bug #759238
> 
[...]
>  
> +# @FUNCTION: linux-mod_pkg_pretend
> +# @DESCRIPTION:
> +# Check the CONFIG_CHECK options 
> +linux-mod_pkg_pretend() {
> +
> +	debug-print-function ${FUNCNAME} $*
> +	# External modules use kernel symbols (bug #591832, #759238)
> +	CONFIG_CHECK+=" !TRIM_UNUSED_KSYMS"
> +}

Is this not supposed to actually run the checks? Correct me if I'm
missing something but it's just setting the value. linux-mod normally
run the checks through linux-info_pkg_setup and its check_extra_config.

For nvidia-drivers, this value will also be lost for the pkg_setup test
(needed to check, say.. gentoo-kernel emerged in-between) because I 
currently set a local CONFIG_CHECK="..." inside pkg_setup()
(there's also a conditional CONFIG_CHECK, part of why not global)

Some other ebuilds set CONFIG_CHECK in pkg_setup I believe.

Not that I can't change this for nvidia, I guess I could set a global
scope CONFIG_CHECK with !FATAL-only and += the non-fatal ones in
pkg_setup to avoid message duplication.

Have same concerns as mgorny wrt exported pkg_pretend, plus I'd also
need to add my own pkg_pretend wrapper to check MODULES_OPTIONAL_USE
-- 
ionen

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [gentoo-dev] [PATCH] Create default implementation of pkg_pretend, move CONFIG_CHECK
  2021-05-30 18:28 ` Ionen Wolkens
@ 2021-05-30 18:42   ` Mike
  2021-05-30 18:55     ` Ionen Wolkens
  0 siblings, 1 reply; 6+ messages in thread
From: Mike @ 2021-05-30 18:42 UTC (permalink / raw
  To: gentoo-dev



On 5/30/21 2:28 PM, Ionen Wolkens wrote:
> On Sun, May 30, 2021 at 01:29:12PM -0400, mpagano@gentoo.org wrote:
>> From: Mike Pagano <mpagano@gentoo.org>
>>
>> As the purpose of pkg_pretend is to run sanity checks during
>> dependency calculation time, provide the default implementation
>> and perform CONFIG_CHECK within it.
>>
>> See bug #759238
>>
> [...]
>>  
>> +# @FUNCTION: linux-mod_pkg_pretend
>> +# @DESCRIPTION:
>> +# Check the CONFIG_CHECK options 
>> +linux-mod_pkg_pretend() {
>> +
>> +	debug-print-function ${FUNCNAME} $*
>> +	# External modules use kernel symbols (bug #591832, #759238)
>> +	CONFIG_CHECK+=" !TRIM_UNUSED_KSYMS"
>> +}
> 
> Is this not supposed to actually run the checks? Correct me if I'm
> missing something but it's just setting the value. linux-mod normally
> run the checks through linux-info_pkg_setup and its check_extra_config.

Thanks for the analysis, Ionen.
The eclass does not actually run the check as it exists today.
Maybe there was a reason for that when it was originally coded? 

 
> For nvidia-drivers, this value will also be lost for the pkg_setup test
> (needed to check, say.. gentoo-kernel emerged in-between) because I 
> currently set a local CONFIG_CHECK="..." inside pkg_setup()
> (there's also a conditional CONFIG_CHECK, part of why not global)
> 
> Some other ebuilds set CONFIG_CHECK in pkg_setup I believe.

Maybe virtualbox-modules should do it's own CONFIG_CHECK as nvidia-drivers does.
As that package is the impetus of this patch.

> Not that I can't change this for nvidia, I guess I could set a global
> scope CONFIG_CHECK with !FATAL-only and += the non-fatal ones in
> pkg_setup to avoid message duplication.
> 
> Have same concerns as mgorny wrt exported pkg_pretend, plus I'd also
> need to add my own pkg_pretend wrapper to check MODULES_OPTIONAL_USE
> 

-- 
Mike Pagano
Gentoo Developer - Kernel Project
Gentoo Sources - Lead 
E-Mail     : mpagano@gentoo.org
GnuPG FP   : 52CC A0B0 F631 0B17 0142 F83F 92A6 DBEC 81F2 B137
Public Key : http://http://pgp.mit.edu/pks/lookup?search=0x92A6DBEC81F2B137&op=index


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [gentoo-dev] [PATCH] Create default implementation of pkg_pretend, move CONFIG_CHECK
  2021-05-30 18:42   ` Mike
@ 2021-05-30 18:55     ` Ionen Wolkens
  2021-05-30 19:07       ` Mike
  0 siblings, 1 reply; 6+ messages in thread
From: Ionen Wolkens @ 2021-05-30 18:55 UTC (permalink / raw
  To: gentoo-dev

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

On Sun, May 30, 2021 at 02:42:01PM -0400, Mike wrote:
> 
> 
> On 5/30/21 2:28 PM, Ionen Wolkens wrote:
> > On Sun, May 30, 2021 at 01:29:12PM -0400, mpagano@gentoo.org wrote:
> >> From: Mike Pagano <mpagano@gentoo.org>
> >>
> >> As the purpose of pkg_pretend is to run sanity checks during
> >> dependency calculation time, provide the default implementation
> >> and perform CONFIG_CHECK within it.
> >>
> >> See bug #759238
> >>
> > [...]
> >>  
> >> +# @FUNCTION: linux-mod_pkg_pretend
> >> +# @DESCRIPTION:
> >> +# Check the CONFIG_CHECK options 
> >> +linux-mod_pkg_pretend() {
> >> +
> >> +	debug-print-function ${FUNCNAME} $*
> >> +	# External modules use kernel symbols (bug #591832, #759238)
> >> +	CONFIG_CHECK+=" !TRIM_UNUSED_KSYMS"
> >> +}
> > 
> > Is this not supposed to actually run the checks? Correct me if I'm
> > missing something but it's just setting the value. linux-mod normally
> > run the checks through linux-info_pkg_setup and its check_extra_config.
> 
> Thanks for the analysis, Ionen.
> The eclass does not actually run the check as it exists today.
> Maybe there was a reason for that when it was originally coded? 

It does indirectly:
    # External modules use kernel symbols (bug #591832)
    CONFIG_CHECK+=" !TRIM_UNUSED_KSYMS"

    linux-info_pkg_setup;
    ^^^^^ checks ran here

Older nvidia-drivers ebuild was kind of nasty with that, it did its
own checks then the linux-mod.eclass' linux-info call did as well
(duplicate messages), which I now let linux-mod handle.

> 
>  
> > For nvidia-drivers, this value will also be lost for the pkg_setup test
> > (needed to check, say.. gentoo-kernel emerged in-between) because I 
> > currently set a local CONFIG_CHECK="..." inside pkg_setup()
> > (there's also a conditional CONFIG_CHECK, part of why not global)
> > 
> > Some other ebuilds set CONFIG_CHECK in pkg_setup I believe.
> 
> Maybe virtualbox-modules should do it's own CONFIG_CHECK as nvidia-drivers does.
> As that package is the impetus of this patch.
> 
> > Not that I can't change this for nvidia, I guess I could set a global
> > scope CONFIG_CHECK with !FATAL-only and += the non-fatal ones in
> > pkg_setup to avoid message duplication.
> > 
> > Have same concerns as mgorny wrt exported pkg_pretend, plus I'd also
> > need to add my own pkg_pretend wrapper to check MODULES_OPTIONAL_USE
> > 
> 

-- 
ionen

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [gentoo-dev] [PATCH] Create default implementation of pkg_pretend, move CONFIG_CHECK
  2021-05-30 18:55     ` Ionen Wolkens
@ 2021-05-30 19:07       ` Mike
  0 siblings, 0 replies; 6+ messages in thread
From: Mike @ 2021-05-30 19:07 UTC (permalink / raw
  To: gentoo-dev



On 5/30/21 2:55 PM, Ionen Wolkens wrote:
> On Sun, May 30, 2021 at 02:42:01PM -0400, Mike wrote:
>>
>>
>> On 5/30/21 2:28 PM, Ionen Wolkens wrote:
>>> On Sun, May 30, 2021 at 01:29:12PM -0400, mpagano@gentoo.org wrote:
>>>> From: Mike Pagano <mpagano@gentoo.org>
>>>>
>>>> As the purpose of pkg_pretend is to run sanity checks during
>>>> dependency calculation time, provide the default implementation
>>>> and perform CONFIG_CHECK within it.
>>>>
>>>> See bug #759238
>>>>
>>> [...]
>>>>  
>>>> +# @FUNCTION: linux-mod_pkg_pretend
>>>> +# @DESCRIPTION:
>>>> +# Check the CONFIG_CHECK options 
>>>> +linux-mod_pkg_pretend() {
>>>> +
>>>> +	debug-print-function ${FUNCNAME} $*
>>>> +	# External modules use kernel symbols (bug #591832, #759238)
>>>> +	CONFIG_CHECK+=" !TRIM_UNUSED_KSYMS"
>>>> +}
>>>
>>> Is this not supposed to actually run the checks? Correct me if I'm
>>> missing something but it's just setting the value. linux-mod normally
>>> run the checks through linux-info_pkg_setup and its check_extra_config.
>>
>> Thanks for the analysis, Ionen.
>> The eclass does not actually run the check as it exists today.
>> Maybe there was a reason for that when it was originally coded? 
> 
> It does indirectly:
>     # External modules use kernel symbols (bug #591832)
>     CONFIG_CHECK+=" !TRIM_UNUSED_KSYMS"
> 
>     linux-info_pkg_setup;
>     ^^^^^ checks ran here
> 
> Older nvidia-drivers ebuild was kind of nasty with that, it did its
> own checks then the linux-mod.eclass' linux-info call did as well
> (duplicate messages), which I now let linux-mod handle.
> 
>>
>>  
>>> For nvidia-drivers, this value will also be lost for the pkg_setup test
>>> (needed to check, say.. gentoo-kernel emerged in-between) because I 
>>> currently set a local CONFIG_CHECK="..." inside pkg_setup()
>>> (there's also a conditional CONFIG_CHECK, part of why not global)
>>>
>>> Some other ebuilds set CONFIG_CHECK in pkg_setup I believe.
>>
>> Maybe virtualbox-modules should do it's own CONFIG_CHECK as nvidia-drivers does.
>> As that package is the impetus of this patch.
>>
>>> Not that I can't change this for nvidia, I guess I could set a global
>>> scope CONFIG_CHECK with !FATAL-only and += the non-fatal ones in
>>> pkg_setup to avoid message duplication.
>>>
>>> Have same concerns as mgorny wrt exported pkg_pretend, plus I'd also
>>> need to add my own pkg_pretend wrapper to check MODULES_OPTIONAL_USE
>>>
>>
> 

Going to rescind this patch from consideration.  

Mike


^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2021-05-30 19:08 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2021-05-30 17:29 [gentoo-dev] [PATCH] Create default implementation of pkg_pretend, move CONFIG_CHECK mpagano
2021-05-30 17:33 ` Michał Górny
2021-05-30 18:28 ` Ionen Wolkens
2021-05-30 18:42   ` Mike
2021-05-30 18:55     ` Ionen Wolkens
2021-05-30 19:07       ` Mike

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox