Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Python 3.6.8 - fix cross-compilation issue #3726

Closed
wants to merge 1 commit into from
Closed

Python 3.6.8 - fix cross-compilation issue #3726

wants to merge 1 commit into from

Conversation

roleoroleo
Copy link
Contributor

@roleoroleo roleoroleo commented Jun 20, 2019

Motivation: I'm trying to use Home Assistant anda the last version deprecates Python 3.5
Linked issues: #3673

I cross-compiled it on x86_64 using a debian 9.9 with docker.
And I was able to test it on armada370.
I realized that the wheel files and the libraries contained still have the wrong notation "linux_x86_64" even if they are arm binaries.
Looking at the previous python 3.5.6 package I noticed that the situation is similar.
This is the output of the sys import executed on my DS214se:

roleo@DiskStation:~$ /var/packages/python3/target/bin/python3   -c 'import sqlite3; import _ctypes; import lxml; import sys; print ("OK:", sys.platform, sys.byteorder, "endian", sys.version)'
OK: linux little endian 3.6.8 (default, Jun 18 2019, 11:06:41)
[GCC 4.9.3 20150311 (prerelease)]

@chickenandpork
Copy link
Member

chickenandpork commented Jun 20, 2019

(repost with correct ID)
I appreciate your work where I made an error, and I really appreciate your time to fix it.

In #3673 comment #3673 (comment) we discussed how to check that the resulting cross-compiled binary works.

I think we're missing proof that the ctypes and some cffi can load correctly as well. I am worried about the pathnames a bit, and I don't think repeating any errors from 3.5.6 buy us any favors. I would allude to both your patch and @ymartin59's comment that agree on simplifying the build process.

The key is whether the binary built on one arch runs correctly in the other, which you have shown to some extent, but may not highlight the nuance of the error previously noticed downstream of the last builds. On your x86_64-built python-3.6.8 for the armada370, please run the following, and please edit your top "is that OK?" to include this so that it's really obvious for the next iteration.

/var/packages/python3/target/bin/python3   -c 'import sqlite3; import _ctypes; import lxml; import sys; print ("OK:", sys.platform, sys.byteorder, "endian", sys.version)'

Specifically, I think the _ctypes exercising cffi, and I think sys exercising the _sysconfigdata, should show whether it's working despite any filename/path errors, and to get beta builds to test more broadly. I would then suggest testing whether homeassistant runs, since it uses a huge list of libraries brought in at postinstall (both wheel and pip-install, I think)

@roleoroleo
Copy link
Contributor Author

I edited my previous post.
Regarding the errors on the names, I did several tests but I couldn't solve the issue.
The problem is related exclusively to the wheels created by "pip wheel".
I tried using the "--plat-name" build option but the compilation process does not complete successfully.

@SynoCommunity SynoCommunity deleted a comment from effesseme Jun 20, 2019
@chickenandpork
Copy link
Member

This is the output of the sys import executed on my DS214se:
To be clear, this is the output on your armada370 built on your x86_64? I think it is, but it requires a hardware lookup rather than being immediately clear.

I think this is a successful sign though.

Yet another check (platform.system) to try to detect issues:

/var/packages/python3/target/bin/python3   -c 'import sqlite3; import _ctypes; import platform; import lxml; import sys; print ("OK:", sys.platform, sys.byteorder, "endian", sys.version, "machine:", platform.machine())'

Does the "machine:" value look correct on your armada370 ?

My hidden agenda here is to be able to semi-automate later release validation. Looking for a value on your armada370 HW that says something different than the x86_64 on your build system

@roleoroleo
Copy link
Contributor Author

roleo@DiskStation:~$ /var/packages/python3/target/bin/python3   -c 'import sqlite3; import _ctypes; import platform; import lxml; import sys; print ("OK:", sys.platform, sys.byteorder, "endian", sys.version, "machine:", platform.machine())'
OK: linux little endian 3.6.8 (default, Jun 18 2019, 11:06:41)
[GCC 4.9.3 20150311 (prerelease)] machine: armv7l

@chickenandpork chickenandpork requested a review from ymartin59 June 21, 2019 07:08
@chickenandpork
Copy link
Member

Looks good to me then; reduces the customization/patch load by 56 lines or so.

Yves ?

@roleoroleo
Copy link
Contributor Author

I'm working on another patch that also solves the problem of library and wheel names.

@schumi2004
Copy link
Contributor

schumi2004 commented Jun 28, 2019

@roleoroleo
Is this ready to be compiled?
I'm also trying to get HA up and running but old Pyhton packages fail on me and was hoping this would solve it ;)

@roleoroleo
Copy link
Contributor Author

roleoroleo commented Jul 1, 2019

Unfortunately homeassistant does not work.
The bcrypt module does not compile, it fails during the pip wheel command.
If you remove bcrypt from the requirements (just to try) the command completes successfully.

@schumi2004
Copy link
Contributor

Yeah I also noticed that. Is bcrypt really needed then?

@roleoroleo
Copy link
Contributor Author

roleoroleo commented Jul 2, 2019

I created a new PR to solve the bcrypt problem.
#3731
If the new PR works correctly this one can be removed.

@schumi2004
Copy link
Contributor

schumi2004 commented Jul 2, 2019

@roleoroleo
Still giving errors for me, sorry ;)
https://pastebin.com/YbQfHi5h

/edit:
Starting from scratch with a new git clone and patch applied, let's wait how it turns out.

/edit2:
Same error
https://pastebin.com/bjR4uzvM

/edit:
Seems it's Ubuntu related. See many messages on the web pointing to a conflict with Pythin3 and Python 2.7 related to lsb_release

@@ -17,7 +17,8 @@ LICENSE = PSF

GNU_CONFIGURE = 1
ADDITIONAL_CFLAGS = -D_LARGEFILE64_SOURCE -D_FILE_OFFSET_BITS=64 -L $(STAGING_INSTALL_PREFIX)/lib -I $(STAGING_INSTALL_PREFIX)/include
CONFIGURE_ARGS = --enable-shared --enable-ipv6 --without-ensurepip --enable-loadable-sqlite-extensions PYTHON_FOR_BUILD=$(HOSTPYTHON) PGEN_FOR_BUILD=$(HOSTPGEN)
#CONFIGURE_ARGS = --enable-shared --enable-ipv6 --without-ensurepip --enable-loadable-sqlite-extensions PYTHON_FOR_BUILD=$(HOSTPYTHON) PGEN_FOR_BUILD=$(HOSTPGEN)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would simply discard old line...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry but I deleted all my forked repos and I don't known how to change the PR.

@ymartin59
Copy link
Contributor

I am processing builds to confirm it works well... Then I would publish my own borgbackup (and maybe homeassistant) updates...

@ymartin59
Copy link
Contributor

Repository and branch no longer available. Superseded by #3731

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants