Gentoo Websites Logo
Go to: Gentoo Home Documentation Forums Lists Bugs Planet Store Wiki Get Gentoo!

Bug 690784

Summary: app-misc/pip3line: unacceptable hackery on top of PYTHON_TARGETS
Product: Gentoo Linux Reporter: Michał Górny <mgorny>
Component: Current packagesAssignee: No maintainer - Look at https://wiki.gentoo.org/wiki/Project:Proxy_Maintainers if you want to take care of it <maintainer-needed>
Status: RESOLVED WONTFIX    
Severity: normal CC: monsieurp, proxy-maint
Priority: Normal    
Version: unspecified   
Hardware: All   
OS: Linux   
Whiteboard:
Package list:
Runtime testing required: ---

Description Michał Górny archtester Gentoo Infrastructure gentoo-dev Security 2019-07-26 22:26:28 UTC
Touching PYTHON_TARGETS directly is forbidden.  The eclass provides complete API to manipulate them that covers the corner cases.  Please fix the ebuild to use the eclass correctly, also fix dependencies and missing REQUIRED_USE.

Also CC-ing the person who merged it without proper review.
Comment 1 Gabriel Caudrelier 2019-07-31 00:19:47 UTC
Zoglene and Virgil dupras were the original reviewer of the ebuild, one has to look at the Github Pull history. Adding them as well.


>> Touching PYTHON_TARGETS directly is forbidden.

I have just looked upon the Gentoo documentation and could not find anything dictating this. But maybe I have missed it.

I am also not "touching" the PYTHON_TARGETS, just iterating over the values. It should be fairly easy to understand if you look at the ebuild.

>> The eclass provides complete API to manipulate them that covers the corner cases.  Please fix the ebuild to use the eclass correctly, also fix dependencies and missing REQUIRED_USE.

I _am_ using the ebuild API as suggested by Virgil and Zoglene.

I also have no idea why I would be using REQUIRED_USE.

The creation of this ebuild was a fairly lengthy and arduous process, partially being my fault at misinterpreting some of the comments, but also because this package required unusual configuration in order to overcome the limit of the Python CMake module.

I am not going to re-explain it to yet another person.

If anyone has a better implementation to solve the problems posed by CMake and Python embedding, fell free to share and reopen this "bug report".

In the meantime I am going to close it.
Comment 2 Michał Górny archtester Gentoo Infrastructure gentoo-dev Security 2019-07-31 02:59:05 UTC
The Python project documentation is the source of policy on how Python is to be used.  Anything not explicitly documented there is forbidden as fragile and prone to break.
Comment 3 Gabriel Caudrelier 2019-08-03 22:00:45 UTC
@mgorny

when it breaks I will fix it, that's what (proxy-)maintainers are responsible for, isn't?

But then again there is no bug here, the ebuild works well as far as I know.

You also haven't offered any solution to this "problem", yet you are a Gentoo dev.

One may wonder if you don't have an ulterior motive.
Comment 4 Michał Górny archtester Gentoo Infrastructure gentoo-dev Security 2019-08-03 22:23:23 UTC
All maintainers are responsible for ensuring that their packages follow standing policies.  GLEP 48 [1] clearly states:

| Just because a particular QA violation has yet to cause an issue does not change
| the fact that it is still a QA violation.

I'm calling for the last time that you start respecting policies, and at least show some effort to fix the ebuild.  If you aren't going to do that, please kindly leave the bug open as long as the faulty ebuild is there.

[1] https://www.gentoo.org/glep/glep-0048.html
Comment 5 Gabriel Caudrelier 2019-08-04 01:14:17 UTC
>> I'm calling for the last time that you start respecting policies, and at least show some effort to fix the ebuild.

I would love to, unfortunately you haven't say anything about how to fix it, or what is the actual problem.

The package still pass QA as well (I just try). So this is not a QA issue.

And again there is nothing to fix.
Comment 6 Matthias Maier gentoo-dev 2019-08-04 03:43:01 UTC
Dear all,

I am reopening this bug as there are some QA issues with the current ebuilds for app-misc/pip3line.


Michał is referring to the following lines in the ebuild:

 63         local targets=( ${PYTHON_TARGETS} )                                      
 64         for target in ${targets[@]}; do                                          
 65             if python_is_python3 ${target}; then                                 
 66                 python_export ${target} PYTHON PYTHON_LIBPATH PYTHON_INCLUDEDIR  
 67                 mycmakeargs+=(-DWITH_PYTHON3=ON                                  
 68                     -DPYTHON3_INCLUDE_DIRS=${PYTHON_INCLUDEDIR}                  
 69                     -DPYTHON3_LIBRARIES=${PYTHON_LIBPATH}                        
 70                 )                                                                
 71                 break                                                            
 72             fi                                                                   
 73         done                                                                     
 74         for target in ${targets[@]}; do                                          
 75             if ! python_is_python3 ${target}; then                               
 76                 python_export ${target} PYTHON PYTHON_LIBPATH PYTHON_INCLUDEDIR  
 77                 mycmakeargs+=(-DWITH_PYTHON27=ON                                 
 78                     -DPYTHON27_INCLUDE_DIRS=${PYTHON_INCLUDEDIR}                 
 79                     -DPYTHON27_LIBRARIES=${PYTHON_LIBPATH}                       
 80                 )                                                                
 81                 break                                                            
 82             fi                                                                   
 83         done 

This does not look right (for starters your for loop overrides cmake parameters). What problem (in terms of cmake parameters) are you trying to solve here that cannot be solved by using the python-r1 eclass directly? [1,2]

Michał would you mind elaborating on how to use python_foreach_impl for this task?

[1] https://wiki.gentoo.org/wiki/Project:Python/python-r1
[2] https://wiki.gentoo.org/wiki/Project:Python/python-r1#Build-time_functions


[1]
Comment 7 Michał Górny archtester Gentoo Infrastructure gentoo-dev Security 2019-08-04 05:04:02 UTC
(In reply to Gabriel Caudrelier from comment #5)
> The package still pass QA as well (I just try). So this is not a QA issue.

No, it doesn't:

https://qa-reports.gentoo.org/output/gentoo-ci/output.html#app-misc/pip3line

(In reply to Matthias Maier from comment #6)
> Michał would you mind elaborating on how to use python_foreach_impl for this
> task?

I would.  I was thoroughly disrespected, and I do not wish to work with that person.  Furthermore, given his aggressive attitude so far I don't believe he should be a proxied maintainer.

If you want to work with him, then please by all means do.  Otherwise, I'm just going to kick him and move the ebuild to maintainer-needed + treecleaning candidate.
Comment 8 David Seifert gentoo-dev 2019-08-04 11:53:55 UTC
(In reply to Gabriel Caudrelier from comment #5)
> >> I'm calling for the last time that you start respecting policies, and at least show some effort to fix the ebuild.
> 
> I would love to, unfortunately you haven't say anything about how to fix it,
> or what is the actual problem.
> 
> The package still pass QA as well (I just try). So this is not a QA issue.
> 
> And again there is nothing to fix.

Please fix it or we will mark it as a QA case and have it removed forcibly. Someone from the python team told you not to rely on python eclass implementation details, yet you're willing to argue around facts. The python team wrote the API, they get to determine whether you're abusing internals of it.
Comment 9 Gabriel Caudrelier 2019-08-04 21:12:28 UTC
(In reply to Matthias Maier from comment #6)
> Dear all,
> 
> I am reopening this bug as there are some QA issues with the current ebuilds
> for app-misc/pip3line.
> 
> 
> Michał is referring to the following lines in the ebuild:
> 
>  63         local targets=( ${PYTHON_TARGETS} )                             
> 
>  64         for target in ${targets[@]}; do                                 
> 
>  65             if python_is_python3 ${target}; then                        
> 
>  66                 python_export ${target} PYTHON PYTHON_LIBPATH
> PYTHON_INCLUDEDIR  
>  67                 mycmakeargs+=(-DWITH_PYTHON3=ON                         
> 
>  68                     -DPYTHON3_INCLUDE_DIRS=${PYTHON_INCLUDEDIR}         
> 
>  69                     -DPYTHON3_LIBRARIES=${PYTHON_LIBPATH}               
> 
>  70                 )                                                       
> 
>  71                 break                                                   
> 
>  72             fi                                                          
> 
>  73         done                                                            
> 
>  74         for target in ${targets[@]}; do                                 
> 
>  75             if ! python_is_python3 ${target}; then                      
> 
>  76                 python_export ${target} PYTHON PYTHON_LIBPATH
> PYTHON_INCLUDEDIR  
>  77                 mycmakeargs+=(-DWITH_PYTHON27=ON                        
> 
>  78                     -DPYTHON27_INCLUDE_DIRS=${PYTHON_INCLUDEDIR}        
> 
>  79                     -DPYTHON27_LIBRARIES=${PYTHON_LIBPATH}              
> 
>  80                 )                                                       
> 
>  81                 break                                                   
> 
>  82             fi                                                          
> 
>  83         done 
> 
> This does not look right (for starters your for loop overrides cmake
> parameters). What problem (in terms of cmake parameters) are you trying to
> solve here that cannot be solved by using the python-r1 eclass directly?
> [1,2]
> 
> Michał would you mind elaborating on how to use python_foreach_impl for this
> task?
> 
> [1] https://wiki.gentoo.org/wiki/Project:Python/python-r1
> [2]
> https://wiki.gentoo.org/wiki/Project:Python/python-r1#Build-time_functions
> 
> 
> [1]

I need to compile against the latest version of each Python Major version, and only those one.

For example if there is python 2.7,  python 3.4 and python 3.5 present (and selected) on the system, I only need to select Python 2.7 and Python 3.5.

The python_foreach_impl will runs the function against all versions indiscriminately, which is going to be a problem as I only need the latest one.



In regards to the cmake configuration override this is done because cmake only compiles against the latest python version present on the system, regardless of what you specify as a minimum.

For example let's assume that there are 4 implementations present on the current system:

python 2.7, python 3.4, python 3.5 and Python 3.6

but the user only specify python 2.7,  python 3.4 and python 3.5 in their PYTHON_TARGETS

When cmake compiles the plugin for Python2.7 it will auto-select Python3.6, same when it comes to compiling the Python3 plugin.

This would go against the defined PYTHON_TARGETS. So the only solution found was to loop over the values, take the highest one in each Major branch and override the cmake configuration.

I am open to better implementation solutions.
Comment 10 Gabriel Caudrelier 2019-08-04 22:06:27 UTC
This package is going to be removed from the Gentoo tree.
Comment 11 Michał Górny archtester Gentoo Infrastructure gentoo-dev Security 2019-08-05 03:33:46 UTC
Don't close bugs as long as the package is there.
Comment 12 Konstantin 2019-08-28 02:11:18 UTC
This can be solved by using REQUIRED_USE and python_foreach_impl

REQUIRED_USE would be something like

python_targets_python3_5? (
 !python_targets_python3_6
 !python_targets_python3_7
)
python_targets_python3_6? (
 !python_targets_python3_7
)