Re: [PATCH][resend] python-native: distutils: don't use libdir, remove dead code path

[Richard Purdie <richard.purdie@linuxfoundation.org>]

On Mon, 2012-03-12 at 20:22 +0100, Andreas Oberritter wrote:
> On 12.03.2012 18:58, Saul Wold wrote:
> > On 03/07/2012 01:07 PM, Andreas Oberritter wrote:
> >> * Coming from OE-classic it was surprising that python-native now
> >>    requires 'libdir' to be exported. Otherwise autoconf would fail
> >>    to detect python libraries. This happend using a customized
> >>    environment setup script to use OE's compiler and libs without
> >>    bitbake.
> >> * Use sys.lib instead of libdir's suffix.
> >> * While at it, simplify redundant if/and-statments.
> >>
> >> Signed-off-by: Andreas Oberritter<obi@opendreambox.org>
> >> ---
> >> * This patch hasn't been commented since its first submission
> >>    on Feb 21st.
> >> * Original patch URL: http://patches.openembedded.org/patch/21481/
> >>
> >>   ...2-distutils-prefix-is-inside-staging-area.patch |   15
> >> +++++----------
> >>   1 files changed, 5 insertions(+), 10 deletions(-)
> >>
> > This will need a PR bump so the modified patch will be noticed, I know
> > this patch was put in a while ago.
> 
> Right. I'm waiting before resubmitting it. See below.
> 
> > I am not so sure about the changes though, I have been meaning to dig
> > into this, the orignial code looks strange in that it includes
> > plat_specific in the else clause!  You are also dropping the EXEC_PREFIX
> 
> I'm dropping EXEC_PREFIX, because it's dead code.
> 
> The code from the original patch can be rewritten as:
> 
> if plat_specific:
> 	if plat_specific:
> 		prefix = os.environ['STAGING_LIBDIR'].rstrip(lib_basename)
> else:
> 	if plat_specific:
> 		prefix = EXEC_PREFIX
> 	else:
> 		prefix = PREFIX
> 
> Does this make it clear?
>  
> > How have you tested this change?
> 
> I built from scratch and also runtime tested it, but without multilib. 
> 
> As I'm currently investing a different problem in python, digging into
> python internals, I doubt that sys.lib really contains the correct
> value for multilib, unless multilib builds have multiple python-natives.
> 
> I think it would be better to just drop the last element of STAGING_LIBDIR,
> i.e.:
> 
> if plat_specific:
> 	prefix = '/'.join(os.environ['STAGING_LIBDIR'].split('/')[:-1])
> else:
> 	prefix = PREFIX
> 
> Do you think this would be OK?

Whilst we merged this patch, its now creating all kinds of issues and is
breaking builds. As a small testcase:

libdir=2 BUILD_SYS=1 HOST_SYS=2 /media/build1/poky/build/tmp/sysroots/x86_64-linux/usr/bin/python -c "import sys; from distutils import sysconfig; sys.stdout.write(sysconfig.get_python_lib(0,0,prefix='foorbar'))"
Traceback (most recent call last):
  File "<string>", line 1, in <module>
  File "/media/build1/poky/build/tmp/sysroots/x86_64-linux/usr/lib/python2.7/distutils/sysconfig.py", line 113, in get_python_lib
    lib_basename = os.environ['STAGING_LIBDIR'].split('/')[-1]
  File "/media/build1/poky/build/tmp/sysroots/x86_64-linux/usr/lib/python2.7/UserDict.py", line 23, in __getitem__
    raise KeyError(key)
KeyError: 'STAGING_LIBDIR'

which leads to packaging warnings from xcb-proto's python module
components due to pythondir not being set to anything. Its looking
likely I'll have to revert this whilst the impact of that patch is
better understood.

Yes, I appreciate you could export STAGING_LIBDIR everywhere but we
never used to have to do that and I'd really prefer we didn't have to.

Cheers,

Richard