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

PEX compiler breaks build #41

Open
drewsonne opened this issue Feb 20, 2018 · 7 comments
Open

PEX compiler breaks build #41

drewsonne opened this issue Feb 20, 2018 · 7 comments

Comments

@drewsonne
Copy link
Contributor

drewsonne commented Feb 20, 2018

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:

Could not satisfy all requirements for lxml==3.8.0:
    lxml==3.8.0(from: onelogin_aws_cli->onelogin)
/home/travis/.travis/job_stages: line 60: dist/onelogin-aws-login: No such file or directory
@slycoder
Copy link
Contributor

I've gone ahead and reverted #30 for the time being to unblock things.

@mumoshu , if you have suggestions for a fix we can try landing again.

@mumoshu
Copy link
Contributor

mumoshu commented Feb 20, 2018

@drewsonne @slycoder Sorry for breaking things, and thanks for your efforts 🙇

if python3 is installed anyway as mentioned in #30, what is the advantage of...

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?

a homebrew formula which wraps this install in a hidden python3 virtualenv for mac osx

This does sound like a great idea for macOS users 👍

Anyway, I'll keep thinking what I can do for this topic.
I'm ok with any way as long as it eases installation, prevents e.g. my colleagues from breaking their python envs 😉
For example, if it simply turns out to be not a real issue, just a documentation issue, making it a server-side app with a thin client cli(plus extra benefit of proper source ip restriction), etc., that's also ok.
Any input is welcomed!

@drewsonne
Copy link
Contributor Author

@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 pip download... the wheel for each of those env's (which I tried).

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, manylinux1... ) does not match the OS name linux_i386

@drewsonne
Copy link
Contributor Author

I can't create a core homebrew, as:

$ brew audit --new-formula onelogin-aws-cli
onelogin-aws-cli:
  * GitHub repository not notable enough (<20 forks, <20 watchers and <50 stars)

@slycoder it could be hosted on a custom homebrew tap, but not sure how invested you guys are in this. :-) Would need a repo, github.com/physera/homebrew-tap, and I can add the formula we use internally in our company.

after that,

brew tap physera/tap
brew install onelogin-aws-cli

and that would only work for OSX.

Could probably build that into the build process to make it a bit easier.

@drewsonne
Copy link
Contributor Author

... or get your friends and family to all star/watch this repo :-)

@drewsonne
Copy link
Contributor Author

drewsonne commented Apr 19, 2018

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

@slycoder
Copy link
Contributor

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 =).

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

3 participants