-
Notifications
You must be signed in to change notification settings - Fork 32
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
PEX compiler breaks build #41
Comments
@drewsonne @slycoder Sorry for breaking things, and thanks for your efforts 🙇
My take is: Less chances to polluting existing python envs to break either onelogin-aws-cli or other existing python-based tools in people's dev machines. I really wish we could bundle literally everything into the pex binary. However, even if some dependencies leaked out of the pex binary, it seems to ease installation (for me). WDYT?
This does sound like a great idea for macOS users 👍 Anyway, I'll keep thinking what I can do for this topic. |
@mumoshu I'm with you on the pex binary. I just wanted to understand motivations as it broke the build :-) You can see the build logs in travis for what happened, but the long story short is that it can't find a wheel dependency to build for all 3 environments, because lxml wheels are OS/platform specific. So you would need to download Then I found that lxml is only distributed as manylinux1, which apparently pex doesn't support (see above). I think the main problem is that wheel names (eg, |
I can't create a core homebrew, as:
@slycoder it could be hosted on a custom homebrew tap, but not sure how invested you guys are in this. :-) Would need a repo, after that,
and that would only work for OSX. Could probably build that into the build process to make it a bit easier. |
... or get your friends and family to all star/watch this repo :-) |
In relation to this, we (@beamly) are hosting a homebrew tap for this. I'm happy to move this to @physera if you would like, but it would be another step in the release process. https://github.com/beamly/homebrew-tap/blob/master/onelogin-aws-cli.rb |
Nice, we don't have any homebrew taps set up so you're more than welcome to host it if you've already got it set up =). |
The 'dist' stage of the pex compilation is breaking the build. This breaking feature was added in #30.
I have spent a couple of days trying to fix this issue, and the problems seems to be that as lxml relies on libxml (used for the SAML assertion parsing), it must have specific wheels for each os you wish to install on. Furthermore, as lxml only distributes with manylinux1 wheels and pex does not support them, I can not see a way forward.
If this was tested/built inside a docker container, I would like to suggest that the dockerimage @mumoshu used before the build be made a part of the travis build process.
I can appreciate the wish for a binary in docker (as the example in the PR provides), but if python3 is installed anyway as mentioned in #30, what is the advantage of:
curl -o onelogin-aws-login <onelogin-aws-login release> && chmod +x onelogin-aws-login
over
pip3 install onelogin-aws-cli
?In regards to alternatives to fixing this, I have built a homebrew formula which wraps this install in a hidden python3 virtualenv for mac osx, and I can try and get it into homebrew core, but that is out of my control to an extent. On the linux side, a deb/rpm/apk could be built, if pex can't be fixed, and this is a feature which is needed.
The PR probably should have had master merged/rebased back into the branch to run the travis build, which would have caught this.This is blocking the config updates (as their build now fails and tests can not be run) I made in prep for a daemon and which would have caught the breaking, so I would like to get it fixed.
If there is no way forward in fixing this, is there a possibility for the PR to be rolled back, so as not to block other work?
@mumoshu do you have any insight into how to fix the lxml issue:
The text was updated successfully, but these errors were encountered: