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

python27 on arm64 and failing tests for all archs #956

Open
7 of 8 tasks
TheSin- opened this issue Jan 25, 2023 · 23 comments
Open
7 of 8 tasks

python27 on arm64 and failing tests for all archs #956

TheSin- opened this issue Jan 25, 2023 · 23 comments

Comments

@TheSin-
Copy link
Member

TheSin- commented Jan 25, 2023

configure: error: Unexpected output of 'arch' on OSX

Haven't looked into it, but likely just need to update the version or update configure?

  • arm64

TESTS

TCL/TK errors

  • test_tcl (Requires tcltk 8.6.13+)

MemoryErrors (arm64 only)

  • test_ascii_formatd
  • test_multiprocessing
  • test_str
  • test_subprocess
  • test_threading
  • test_unicode
@TheSin- TheSin- added the arm64 Build issues and fixes for Apple Silicon label Jan 25, 2023
@TheSin-
Copy link
Member Author

TheSin- commented Jan 25, 2023

We also want to check so that we don't try to make fat binaries, in the CompileScript

move --with-universal-archs=intel-64

        # for macOS 11, need help finding libz in new system-library cache
        # https://bugs.python.org/issue41116
        # pthread is still not found, so disable test_ctypes for now as well
        if [ "$(uname -r | cut -d. -f1)" -ge 20 ]; then
                DIST_CONFIG_PARAMS="--enable-universalsdk=$(xcrun --sdk macosx --show-sdk-path) --with-universal-archs=intel-64"
        fi

to

        # for macOS 11, need help finding libz in new system-library cache
        # https://bugs.python.org/issue41116
        # pthread is still not found, so disable test_ctypes for now as well
        if [ "$(uname -r | cut -d. -f1)" -ge 20 -a "%m" != "arm64"]; then
                DIST_CONFIG_PARAMS="--enable-universalsdk=$(xcrun --sdk macosx --show-sdk-path) --with-universal-archs=intel-64"
        fi

Add this to the PatchScript

# fix for arm64
awk 'NR==8477{print "\tarm64)\n\t\tMACOSX_DEFAULT_ARCH=\"arm64\"\n\t\t\;;"}1' configure > configure.new
mv configure.new configure
chmod +x configure

@dhomeier
Copy link
Contributor

dhomeier commented Jan 25, 2023

Yes, configure only knows of x86_64 and ppc64 as (64bit) MACOSX_DEFAULT_ARCHs (and apparently keeps on adding -arch i386 -arch x86_64 everywhere).

@TheSin-
Copy link
Member Author

TheSin- commented Jan 25, 2023

So far this did work, it's compiling while see if it finished

I'm expecting more errors but I'll deal as they come

@TheSin-
Copy link
Member Author

TheSin- commented Jan 25, 2023

Python build finished, but the necessary bits to build these modules were not found:
bsddb185           dl                 imageop         
linuxaudiodev      ossaudiodev        spwd            
sunaudiodev       lib                                    
To find the necessary bits, look in setup.py in detect_modules() for the module's name.
(Find package build should have 7 missing)

I assume it's due to zlib, there are notes of that in the info file so I'll see if I can fix that.

x86_64 version for compare

Python build finished, but the necessary bits to build these modules were not found:
bsddb185           dl                 imageop         
linuxaudiodev      ossaudiodev        spwd            
sunaudiodev                                           
To find the necessary bits, look in setup.py in detect_modules() for the module's name.
(Fink package build should have 7 missing)

@TheSin-
Copy link
Member Author

TheSin- commented Jan 25, 2023

Fixed it, needed isysroot in CFLAGS, universalsdk was adding that, not universalsdk shouldn't be required on both

so the full patch is both from #956 (comment)

and change this ins CompileScript

export CFLAGS="-Wno-nullability-completness"

to

export CFLAGS="-Wno-nullability-completness -isysroot $SDK_PATH"

@TheSin-
Copy link
Member Author

TheSin- commented Jan 25, 2023

@nieder @dmacks can you confirm sanity before I commit it?

@TheSin-
Copy link
Member Author

TheSin- commented Jan 26, 2023

Technically we dont' need universals on x86 even so

# for macOS 11, need help finding libz in new system-library cache
        # https://bugs.python.org/issue41116
        # pthread is still not found, so disable test_ctypes for now as well
        if [ "$(uname -r | cut -d. -f1)" -ge 20 ]; then
                DIST_CONFIG_PARAMS="--enable-universalsdk=$(xcrun --sdk macosx --show-sdk-path) --with-universal-archs=intel-64"
        fi

can just be removed completely and just add isysroot to CFLAGS

@TheSin-
Copy link
Member Author

TheSin- commented Jan 26, 2023

Proposed changes (tests are running on 13.2/x86 and 13.2/arm64 ATM), I'd love if someone with 10.x could test as well for make sure it's safe bellow 11.x

--- ../../../stable/main/finkinfo/languages/python27.info	2023-01-14 14:03:31
+++ python27.info	2023-01-26 08:18:32
@@ -3,7 +3,7 @@
 Package: python%type_pkg[python]
 # GDBM_COMPAT
 Version: 2.7.18
-Revision: 5
+Revision: 6
 Epoch: 1
 Type: python 2.7
 Maintainer: Daniel Johnson <daniel@daniel-johnson.org>
@@ -84,15 +84,16 @@
 	# for OS X 10.14, ensure full SDK is available, but include it after any fink specific versions.
 	if [ "$(uname -r | cut -d. -f1)" -ge 18 ]; then
 		SDK_PATH="$(xcrun --sdk macosx --show-sdk-path)"
-		export CFLAGS="-Wno-nullability-completeness"
+		export CFLAGS="-Wno-nullability-completeness -isysroot $SDK_PATH"
 		export CPPFLAGS="-I%p/include -I%p/include/ncursesw -I$SDK_PATH/usr/include"
 	fi
+	# No Longer required with -isysroot in CFLAGS
 	# for macOS 11, need help finding libz in new system-library cache
 	# https://bugs.python.org/issue41116
 	# pthread is still not found, so disable test_ctypes for now as well
-	if [ "$(uname -r | cut -d. -f1)" -ge 20 ]; then
-		DIST_CONFIG_PARAMS="--enable-universalsdk=$(xcrun --sdk macosx --show-sdk-path) --with-universal-archs=intel-64"
-	fi
+	#if [ "$(uname -r | cut -d. -f1)" -ge 20 -a "%m" != "arm64" ]; then
+	#	DIST_CONFIG_PARAMS="--enable-universalsdk=$(xcrun --sdk macosx --show-sdk-path) --with-universal-archs=intel-64"
+	#fi
 
 	# https://bugs.python.org/issue28456 and radar://28372390
 	# https://daniel.haxx.se/blog/2016/10/11/poll-on-mac-10-12-is-broken/

@TheSin-
Copy link
Member Author

TheSin- commented Jan 26, 2023

Both arm and x86 fail test_tcl with

AssertionError: '-e' != '-9223372036854775808'

arm has 6 additional failures that are all MemoryErrors (test_ascii_formatd, test_multiprocessing, test_str, test_subprocess, test_threading, test_unicode)

which I'm not surprised with the age of python 2.7

I feel this is a win/pass and I would like to commit this patch to python27 so arm can use it.

Need to figure out how to make those failed tests none fatal though so the build will finish with tests, obviously without tests it's fine already.

@dmacks
Copy link
Member

dmacks commented Jan 28, 2023

TheSin's 'proposed changes...' two days ago on my 10.13 give identical build transcripts, test failures, and same set of .deb contents as usual on my 10.13.

@dhomeier
Copy link
Contributor

Builds on 10.14.6, both with the old fink installation and the dpkg-1.16 one, and without the test_tcl failure; but all crashing after

0:03:11 load avg: 5.25 [334/397/5] test_test_support
make: *** [test] Killed: 9
DYLD_LIBRARY_PATH=/opt/sw/src/fink.build/python27-2.7.18-6/Python-2.7.18 ./python -Wd -3 -E -tt  ./Lib/test/regrtest.py -l -w -x test_socket test_distutils test_ssl test_multiprocessing_spawn test_asyncio test_httpservers test_logging test_xmlrpc test_ctypes
== CPython 2.7.18 (default, Jan 28 2023, 23:55:07) [GCC Apple LLVM 14.0.0 (clang-1400.0.29.202)]
==   Darwin-22.3.0-arm64-arm-64bit little-endian
==   /opt/sw/src/fink.build/python27-2.7.18-6/Python-2.7.18/build/test_python_28322

Both arm and x86 fail test_tcl with

AssertionError: '-e' != '-9223372036854775808'

Apparently a Tcl bug on 13.2; the tcltk test suite is failing on exactly the same e.g.

==== expr-37.14 expr edge cases FAILED
==== Contents of test case:

    set dividend $min
    set divisor [expr {$min + 2}]
    set q [expr {$dividend / $divisor}]
    set r [expr {$dividend % $divisor}]
    list $q * $divisor + $r = [expr {($q * $divisor) + $r}]

---- Result was:
1 * -9223372036854775806 + -2 = -e
---- Result should have been (exact matching):
1 * -9223372036854775806 + -2 = -9223372036854775808

Remarkably -2**63+1 still works, and -2**63-1 and everything down to -2**1000 works again, it's just failing on -2**63!

arm has 6 additional failures that are all MemoryErrors (test_ascii_formatd, test_multiprocessing, test_str, test_subprocess, test_threading, test_unicode)

Never getting past the
./Lib/test/regrtest.py -l -w -x test_socket test_distutils test_ssl test_multiprocessing_spawn test_asyncio test_httpservers test_logging test_xmlrpc test_ctypes
on either arch, so cannot check the remaining 54-ish tests.

@dhomeier
Copy link
Contributor

==== expr-37.14 expr edge cases FAILED
==== Contents of test case:

Looks like updating to tcltk-8.6.13 will fix this; however packaging that is an entirely different beast.

@dmacks
Copy link
Member

dmacks commented Jan 29, 2023

That test_test_support crash is the same place the test-suite crashes on 10.13.

tcltk is indeed...a beast.

@dhomeier
Copy link
Contributor

tcltk is indeed...a beast.

I have an update to 8.6.13 in https://github.com/dhomeier/fink-distributions/tree/tcltk-8.6.13, if anyone wants to have a look at it. Builds and passes all tests on arm64 and x86_64, but using pkgconf; and I have not spent any thoughts on updating the library version (the main Shlibs versions are unchanged, but itcl and other bundled libraries are installed with incremented patch levels).

TheSin- added a commit that referenced this issue Jan 29, 2023
… 11+ failing test_tcl see #956, On arm failing 6 more tests due to memory errors, still need to look into, but at least it builds and runs for now as the depends run dep on python27, keeping #956 open till tests are fixed
@TheSin-
Copy link
Member Author

TheSin- commented Feb 1, 2023

so the issue was in ctypes, it was redefining ffi_closure_free and ffi_closure_alloc when using an external libffi.

I've removed those 2 and all MemoryErrors in tests are cleared.

Add this to PatchScript and build with -m

        # fix for using external ffi
        perl -pi -e 's,void ffi_closure_free,void Py_ffi_closure_free,g' Modules/_ctypes/malloc_closure.c
        perl -pi -e 's,void \*ffi_closure_alloc,void *Py_ffi_closure_alloc,g' Modules/_ctypes/malloc_closure.c

How ever now I see a segfault on test 310 for test_str

I just wanted to post my progress. so far though obviously it's not fixed yet.

@TheSin-
Copy link
Member Author

TheSin- commented Feb 1, 2023

I'm using this to try to find the issues (though this is on the 3.x branch so I need to take some liberties in figuring it out)

python/cpython@b29d0a5

@TheSin-
Copy link
Member Author

TheSin- commented Feb 1, 2023

The more I look at it the more I think we should use that entire patch :\ cherry picking from it it just moving the memory errors to allocs and such. Looking at it I think the entire patch could be applied with some fuzziness

@TheSin-
Copy link
Member Author

TheSin- commented Feb 2, 2023

With this setup arm64 now only fails the test_tcl just like x86, the changes should only affect the arm64 build.

I've also now been able to successfully install setuptools-tng-py27
python27.zip

@dmacks
Copy link
Member

dmacks commented Feb 2, 2023

Please include a note in python27.info citing the source of python27-arm64.patch. But given it's only applied to ARM platforms, I can't comment on it beyond that.

@TheSin-
Copy link
Member Author

TheSin- commented Feb 2, 2023

will do, though it is modified, specifically in the callproc.c since that patch is for Python3 I had to mod it a bit.

I just wanted to make sure I applied it to arm64 only properly and didn't break anything on x86. I'll add the notes and release it since it fixes other py27 builds for us.

@TheSin-
Copy link
Member Author

TheSin- commented Feb 2, 2023

Pushed and rev up for those on arm that have it built already. Keeping open for the test_tcl issue now.

@TheSin- TheSin- changed the title python27 on arm64 python27 on arm64 and failing tests for all arcs Feb 2, 2023
@TheSin- TheSin- changed the title python27 on arm64 and failing tests for all arcs python27 on arm64 and failing tests for all archs Feb 2, 2023
@TheSin- TheSin- removed the arm64 Build issues and fixes for Apple Silicon label Feb 2, 2023
@dhomeier
Copy link
Contributor

dhomeier commented Feb 2, 2023

Pushed and rev up for those on arm that have it built already. Keeping open for the test_tcl issue now.

#983, not really anything on the Python side to do, except perhaps to bump the deps to tcltk (>= 8.6.13-1).

@nieder
Copy link
Member

nieder commented Jun 20, 2023

I get a total hang on test_popen2 on 13.4. Should we just add that and test_test_support to the skipped tests?

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

No branches or pull requests

4 participants