wandera / 1password-client

Python wrapper for OnePassword CLI developed at Wandera.
MIT License
47 stars 25 forks source link

Check op is installed before any filesystem change #28

Closed iainelder closed 3 years ago

iainelder commented 3 years ago

Before this change the package installer would try to modify system folders before checking whether the changes were necessary.

That would fail unless the installation runs as a priveleged user.

If op is already installed, then we can avoid the problem.

Fixes: https://github.com/wandera/1password-client/issues/25

iainelder commented 3 years ago

@dtpryce Is the logic still correct? I'm unable to test the branch to handle macOS.

Is the pragma in the right place?

dtpryce commented 3 years ago

I'll branch and check this tomorrow ... Looks ok but might suggest some logging to make it clear to users what's going on

dtpryce commented 3 years ago

Ok the logic is just right but like I mentioned I want to add a bit more logging. And since we are about to move to GH actions, I'll make the change on branch in this repo for you and then you can test it before PR locally. Once we have removed travis and replaced with GH we will release a new version with some other updates too :) hope that timeline is ok for you?

iainelder commented 3 years ago

Fine by me! Feel free to make any improvements to this PR. I'll be watching here for updates :-)

dtpryce commented 3 years ago

If you want to try early it should be on this branch https://github.com/wandera/1password-client/tree/pre-installed-op and you can install locally using pip install -e . or python setup.py install ... otherwise wait for the new version and test there

iainelder commented 3 years ago

It fails because of a syntax error here.

https://github.com/wandera/1password-client/blob/de42798f060fdbe0da5024ab8165f5277bf1a9ec/install_op.py#L37-L38

Full output below:

$ pip install -e .
Obtaining file:///tmp/tmp.AIBPzaVqS7/1password-client
Collecting wget>=3.2
  Using cached wget-3.2-py3-none-any.whl
Collecting pyyaml>=5.4
  Using cached PyYAML-5.4.1-cp38-cp38-manylinux1_x86_64.whl (662 kB)
Collecting pycryptodome>=3.9.7
  Using cached pycryptodome-3.10.1-cp35-abi3-manylinux2010_x86_64.whl (1.9 MB)
Collecting pexpect>=4.7.0
  Using cached pexpect-4.8.0-py2.py3-none-any.whl (59 kB)
Collecting ptyprocess>=0.5
  Using cached ptyprocess-0.7.0-py2.py3-none-any.whl (13 kB)
Installing collected packages: ptyprocess, wget, pyyaml, pycryptodome, pexpect, 1password
  Running setup.py develop for 1password
    ERROR: Command errored out with exit status 1:
     command: /home/isme/.local/share/virtualenvs/af4b7c140ceb7d3/bin/python -c 'import io, os, sys, setuptools, tokenize; sys.argv[0] = '"'"'/tmp/tmp.AIBPzaVqS7/1password-client/setup.py'"'"'; __file__='"'"'/tmp/tmp.AIBPzaVqS7/1password-client/setup.py'"'"';f = getattr(tokenize, '"'"'open'"'"', open)(__file__) if os.path.exists(__file__) else io.StringIO('"'"'from setuptools import setup; setup()'"'"');code = f.read().replace('"'"'\r\n'"'"', '"'"'\n'"'"');f.close();exec(compile(code, __file__, '"'"'exec'"'"'))' develop --no-deps
         cwd: /tmp/tmp.AIBPzaVqS7/1password-client/
    Complete output (35 lines):
    running develop
    running egg_info
    creating 1password.egg-info
    writing 1password.egg-info/PKG-INFO
    writing dependency_links to 1password.egg-info/dependency_links.txt
    writing requirements to 1password.egg-info/requires.txt
    writing top-level names to 1password.egg-info/top_level.txt
    writing manifest file '1password.egg-info/SOURCES.txt'
    reading manifest file '1password.egg-info/SOURCES.txt'
    reading manifest template 'MANIFEST.in'
    warning: no files found matching 'LICENSE.txt'
    adding license file 'LICENSE'
    writing manifest file '1password.egg-info/SOURCES.txt'
    running build_ext
    Creating /home/isme/.local/share/virtualenvs/af4b7c140ceb7d3/lib/python3.8/site-packages/1password.egg-link (link to .)
    Adding 1password 0.5.3 to easy-install.pth file

    Installed /tmp/tmp.AIBPzaVqS7/1password-client
    Traceback (most recent call last):
      File "<string>", line 1, in <module>
      File "/tmp/tmp.AIBPzaVqS7/1password-client/setup.py", line 33, in <module>
        setup(
      File "/home/isme/.local/share/virtualenvs/af4b7c140ceb7d3/lib/python3.8/site-packages/setuptools/__init__.py", line 153, in setup
        return distutils.core.setup(**attrs)
      File "/usr/lib/python3.8/distutils/core.py", line 148, in setup
        dist.run_commands()
      File "/usr/lib/python3.8/distutils/dist.py", line 966, in run_commands
        self.run_command(cmd)
      File "/usr/lib/python3.8/distutils/dist.py", line 985, in run_command
        cmd_obj.run()
      File "/tmp/tmp.AIBPzaVqS7/1password-client/setup.py", line 11, in run
        from install_op import install_op
      File "/tmp/tmp.AIBPzaVqS7/1password-client/install_op.py", line 37, in <module>
        "i686": "https://cache.agilebits.com/dist/1P/op/pkg/{}/op_linux_386_{].zip".format(version_string,
    ValueError: expected '}' before end of string
    ----------------------------------------
ERROR: Command errored out with exit status 1: /home/isme/.local/share/virtualenvs/af4b7c140ceb7d3/bin/python -c 'import io, os, sys, setuptools, tokenize; sys.argv[0] = '"'"'/tmp/tmp.AIBPzaVqS7/1password-client/setup.py'"'"'; __file__='"'"'/tmp/tmp.AIBPzaVqS7/1password-client/setup.py'"'"';f = getattr(tokenize, '"'"'open'"'"', open)(__file__) if os.path.exists(__file__) else io.StringIO('"'"'from setuptools import setup; setup()'"'"');code = f.read().replace('"'"'\r\n'"'"', '"'"'\n'"'"');f.close();exec(compile(code, __file__, '"'"'exec'"'"'))' develop --no-deps Check the logs for full command output.
dtpryce commented 3 years ago

It fails because of a syntax error here.

https://github.com/wandera/1password-client/blob/de42798f060fdbe0da5024ab8165f5277bf1a9ec/install_op.py#L37-L38

Full output below:

$ pip install -e .
Obtaining file:///tmp/tmp.AIBPzaVqS7/1password-client
Collecting wget>=3.2
  Using cached wget-3.2-py3-none-any.whl
Collecting pyyaml>=5.4
  Using cached PyYAML-5.4.1-cp38-cp38-manylinux1_x86_64.whl (662 kB)
Collecting pycryptodome>=3.9.7
  Using cached pycryptodome-3.10.1-cp35-abi3-manylinux2010_x86_64.whl (1.9 MB)
Collecting pexpect>=4.7.0
  Using cached pexpect-4.8.0-py2.py3-none-any.whl (59 kB)
Collecting ptyprocess>=0.5
  Using cached ptyprocess-0.7.0-py2.py3-none-any.whl (13 kB)
Installing collected packages: ptyprocess, wget, pyyaml, pycryptodome, pexpect, 1password
  Running setup.py develop for 1password
    ERROR: Command errored out with exit status 1:
     command: /home/isme/.local/share/virtualenvs/af4b7c140ceb7d3/bin/python -c 'import io, os, sys, setuptools, tokenize; sys.argv[0] = '"'"'/tmp/tmp.AIBPzaVqS7/1password-client/setup.py'"'"'; __file__='"'"'/tmp/tmp.AIBPzaVqS7/1password-client/setup.py'"'"';f = getattr(tokenize, '"'"'open'"'"', open)(__file__) if os.path.exists(__file__) else io.StringIO('"'"'from setuptools import setup; setup()'"'"');code = f.read().replace('"'"'\r\n'"'"', '"'"'\n'"'"');f.close();exec(compile(code, __file__, '"'"'exec'"'"'))' develop --no-deps
         cwd: /tmp/tmp.AIBPzaVqS7/1password-client/
    Complete output (35 lines):
    running develop
    running egg_info
    creating 1password.egg-info
    writing 1password.egg-info/PKG-INFO
    writing dependency_links to 1password.egg-info/dependency_links.txt
    writing requirements to 1password.egg-info/requires.txt
    writing top-level names to 1password.egg-info/top_level.txt
    writing manifest file '1password.egg-info/SOURCES.txt'
    reading manifest file '1password.egg-info/SOURCES.txt'
    reading manifest template 'MANIFEST.in'
    warning: no files found matching 'LICENSE.txt'
    adding license file 'LICENSE'
    writing manifest file '1password.egg-info/SOURCES.txt'
    running build_ext
    Creating /home/isme/.local/share/virtualenvs/af4b7c140ceb7d3/lib/python3.8/site-packages/1password.egg-link (link to .)
    Adding 1password 0.5.3 to easy-install.pth file

    Installed /tmp/tmp.AIBPzaVqS7/1password-client
    Traceback (most recent call last):
      File "<string>", line 1, in <module>
      File "/tmp/tmp.AIBPzaVqS7/1password-client/setup.py", line 33, in <module>
        setup(
      File "/home/isme/.local/share/virtualenvs/af4b7c140ceb7d3/lib/python3.8/site-packages/setuptools/__init__.py", line 153, in setup
        return distutils.core.setup(**attrs)
      File "/usr/lib/python3.8/distutils/core.py", line 148, in setup
        dist.run_commands()
      File "/usr/lib/python3.8/distutils/dist.py", line 966, in run_commands
        self.run_command(cmd)
      File "/usr/lib/python3.8/distutils/dist.py", line 985, in run_command
        cmd_obj.run()
      File "/tmp/tmp.AIBPzaVqS7/1password-client/setup.py", line 11, in run
        from install_op import install_op
      File "/tmp/tmp.AIBPzaVqS7/1password-client/install_op.py", line 37, in <module>
        "i686": "https://cache.agilebits.com/dist/1P/op/pkg/{}/op_linux_386_{].zip".format(version_string,
    ValueError: expected '}' before end of string
    ----------------------------------------
ERROR: Command errored out with exit status 1: /home/isme/.local/share/virtualenvs/af4b7c140ceb7d3/bin/python -c 'import io, os, sys, setuptools, tokenize; sys.argv[0] = '"'"'/tmp/tmp.AIBPzaVqS7/1password-client/setup.py'"'"'; __file__='"'"'/tmp/tmp.AIBPzaVqS7/1password-client/setup.py'"'"';f = getattr(tokenize, '"'"'open'"'"', open)(__file__) if os.path.exists(__file__) else io.StringIO('"'"'from setuptools import setup; setup()'"'"');code = f.read().replace('"'"'\r\n'"'"', '"'"'\n'"'"');f.close();exec(compile(code, __file__, '"'"'exec'"'"'))' develop --no-deps Check the logs for full command output.

Hilarious mistake ... fixed. Please do try again.

dtpryce commented 3 years ago

Version 0.6.0 released and should include you fix: pip install 1password==0.6.0

iainelder commented 3 years ago

Thanks! The installation process now works as expected.