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

can't build bdist_wheel for 0.4.14 #136

Closed
edmondsw opened this issue Jul 18, 2018 · 10 comments
Closed

can't build bdist_wheel for 0.4.14 #136

edmondsw opened this issue Jul 18, 2018 · 10 comments

Comments

@edmondsw
Copy link

edmondsw commented Jul 18, 2018

Running this command with the newly-released 0.4.14 version on an ppc64le Ubuntu 16.04 system:

/usr/bin/python -u -c "import setuptools, tokenize;__file__='/tmp/pip-install-oozK0O/greenlet/setup.py';f=getattr(tokenize, 'open', open)(__file__);code=f.read().replace('\r\n', '\n');f.close();exec(compile(code, __file__, 'exec'))" bdist_wheel -d /tmp/pip-wheel-tQBkp3 --python-tag cp27

yields the following error:

running build
running build_ext
building 'greenlet' extension
creating build
creating build/temp.linux-ppc64le-2.7
powerpc64le-linux-gnu-gcc -pthread -DNDEBUG -g -fwrapv -O2 -Wall -Wstrict-prototypes -fno-strict-aliasing -Wdate-time -D_FORTIFY_SOURCE=2 -g -fstack-protector-strong -Wformat -Werror=format-security -fPIC -I/usr/include/python2.7 -c greenlet.c -o build/temp.linux-ppc64le-2.7/greenlet.o -fno-tree-dominator-opts
In file included from slp_platformselect.h:16:0,
                    from greenlet.c:343:
   platform/switch_ppc64_linux.h: In function 'slp_switch':
   platform/switch_ppc64_linux.h:80:5: error: PIC register clobbered by 'r30' in 'asm'
        __asm__ volatile ("" : : : REGS_TO_SAVE);
        ^
   platform/switch_ppc64_linux.h:95:5: error: PIC register clobbered by 'r30' in 'asm'
        __asm__ volatile ("" : : : REGS_TO_SAVE);
        ^
   error: command 'powerpc64le-linux-gnu-gcc' failed with exit status 1```
@snaury
Copy link
Contributor

snaury commented Jul 18, 2018

I was told in #130 this is a gcc bug, which was fixed in https://gcc.gnu.org/git/?p=gcc.git;a=commitdiff;h=9213244550335bcb2b8590a0d7d58ac74c932361

@edmondsw
Copy link
Author

Thanks, snaury. But the latest gcc available for Ubuntu 16.04 LTS is 5.4.0, which does not have the commit referenced above.

Also, it seems to me that #130 most likely caused this. We were not seeing an issue until today, i.e. immediately after 0.4.14 released (the first release with #130).

@snaury
Copy link
Contributor

snaury commented Jul 18, 2018

I would suggest filing a bug report with Ubuntu, or could you maybe switch to official packages instead of building from source? I can't revert the patch, according to ABI it's callee saved, not doing so might cause some weird side-effects/crashes when switching.

openstack-gerrit pushed a commit to openstack/requirements that referenced this issue Jul 24, 2018
The greenlet 0.4.14 release has a bug [1] that breaks at least the
PowerVM CI, and probably a lot more. Dropping u-c back to 0.4.13 to
avoid this issue.

[1] python-greenlet/greenlet#136

Change-Id: I6e81e5a7c384ec273ff4c4665f7df44192707814
@prometheanfire
Copy link

talking to internal gentoo people they also suggested fixing the actual gcc issue as the proper fix.

openstack-gerrit pushed a commit to openstack/nova that referenced this issue Jul 24, 2018
Due to an issue introduced with greenlet 0.4.14 and
gcc on ppc64le systems, we can't run powerkvm or
powervm third party CI, so this change blacklists the
0.4.14 version until the issue [1] is resolved.

The related upper-constraints change is at [2].

[1] python-greenlet/greenlet#136
[2] https://review.openstack.org/584881/

Depends-On: https://review.openstack.org/584881
Change-Id: I3899a0597c75f19cf55d696790ac31a5103db18f
@edmondsw
Copy link
Author

edmondsw commented Aug 1, 2018

I'm not at all convinced that this is a gcc bug. I am seeing the same issue after setting up "Toolchain test builds" PPA (https://launchpad.net/~ubuntu-toolchain-r/+archive/ubuntu/test) to get access to newer versions of gcc. I upgraded gcc-5 and still see the same issue (expected). I then installed gcc-6, gcc-7, and gcc-8 one at a time and tried with each. Same issue. If this is a gcc bug, it doesn't appear to have been fixed yet... i.e. it's not the gcc bug referenced above.

@edmondsw
Copy link
Author

edmondsw commented Aug 1, 2018

I found that powerpc64le-linux-gnu-gcc was a symlink still pointing to gcc-5, so installing gcc-7 didn't mean I was actually using it. I updated the symlinks to use gcc-7 and the problem went away. So this does appear to be resolved with the change that went into gcc-7.

@wiggin15
Copy link

wiggin15 commented Sep 3, 2018

FYI, this is not just about Ubuntu 16.04.
All RedHat-based distributions (RHEL/CentOS/Oracle Linux), including the latest, come with gcc 4.x (see https://access.redhat.com/solutions/19458). This also applies for more distributions like older (but still supported) versions of SLES. And Ubuntu.
We can't always upgrade gcc to a non-default version (we don't always have root access on all of our systems), so we can't install greenlet 0.4.14 on any of these operating systems.

This is a regression from 0.4.13 and prevents us from using the latest version (I see OpenStack already blacklisted this version; unfortunately we are now doing the same)

@snaury
Copy link
Contributor

snaury commented Sep 3, 2018

I can't just revert that patch, because then it would become a regression on newer gcc versions. It's probably possible to switch to manual save/load for this register, maybe as simple as something like this:

void * r30;
...
__asm__ volatile ("std 30, %0" : "=m" (r30));
...
__asm__ volatile ("ld 30, %0" : : "m" (r30));
...

This is already being done for r2, so it's probably possible to do for r30, which would probably mean it would be safe to remove r30 from REGS_TO_SAVE.

Since I don't have access to the platform I cannot test this and cannot make a PR myself. Anyone with access to and familiar with the platform is welcome to make a PR, making sure it actually compiles and works.

@wiggin15
Copy link

wiggin15 commented Sep 4, 2018

I can confirm that the diff you suggested compiles and the tests run successfully with it:
https://gist.github.com/wiggin15/438dd4911e711b60f433f5b629d7113f

@snaury snaury closed this as completed in 86f72de Sep 4, 2018
@snaury
Copy link
Contributor

snaury commented Sep 4, 2018

Thank you @wiggin15 ! I'll try to make a new 0.4.15 release at the end of the week (probably friday).

tanaypf9 pushed a commit to tanaypf9/pf9-requirements that referenced this issue May 20, 2024
The greenlet 0.4.14 release has a bug [1] that breaks at least the
PowerVM CI, and probably a lot more. Dropping u-c back to 0.4.13 to
avoid this issue.

[1] python-greenlet/greenlet#136

Change-Id: I6e81e5a7c384ec273ff4c4665f7df44192707814
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