Skip to content

Fix for incorrect configure python library variable - #90

Merged
grisha merged 1 commit into
grisha:masterfrom
ajarzyn:bug/configuration
Apr 12, 2021
Merged

Fix for incorrect configure python library variable#90
grisha merged 1 commit into
grisha:masterfrom
ajarzyn:bug/configuration

Conversation

@ajarzyn

@ajarzyn ajarzyn commented Mar 5, 2020

Copy link
Copy Markdown

BLDLIBRARY isn't consistent depending on the how the original python got built.

> python  -m sysconfig | grep BLDLIB
	BLDLIBRARY = "-L. -lpython2.7"

> python  -m sysconfig | grep BLDLIB
	BLDLIBRARY = ""

> python -m sysconfig | grep BLDLIB
	BLDLIBRARY = "libpython3.8.a"

locally i've reverted the config code back to pre-ecbdcf17884551b823fcef12cd5bd91103bde1fc (nov 7, 2019)

+    if test "$PyMAJVERSION" -eq "3"; then
+        LDLIBS1="-lpython${PyMAJVERSION}"
+    else
+        LDLIBS1="-lpython${PyVERSION}"
+    fi
-    LDLIBS1=`${PYTHON_BIN} -c 'import distutils.sysconfig;\
-        print(distutils.sysconfig.get_config_var("LDLIBRARY"))'`

Originally posted by @ewah in #81 (comment)

Addressing the issue with python script.

@grisha

grisha commented Apr 10, 2021

Copy link
Copy Markdown
Owner

So are you saying that #90 should be merged and what about #86?

@ajarzyn

ajarzyn commented Apr 12, 2021

Copy link
Copy Markdown
Author

#90 will overwrite changes from #86. In my opinion #86 will just change group affected by that problem and because of that it shoulnd't be merged at all.
As the @ewah commented under #81 using BLDLIBRARY can't guarantee proper result, as the result is related to build method of python's binary.

Thus we need better method to get python version.

@grisha
grisha merged commit 83e9b96 into grisha:master Apr 12, 2021
@tstabrawa tstabrawa mentioned this pull request Oct 30, 2021
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

See More

Successfully merging this pull request may close these issues.

2 participants

@ajarzyn @grisha