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

Version detection is broken #468

Closed
saimn opened this issue May 7, 2021 · 3 comments
Closed

Version detection is broken #468

saimn opened this issue May 7, 2021 · 3 comments
Assignees
Milestone

Comments

@saimn
Copy link

saimn commented May 7, 2021

ALL software version info

$ pip show param       
Name: param
Version: 1.10.1

Description of expected behavior and the observed behavior

param.Version should not run git command on any random git repository it finds.

Complete, minimal, self-contained example code that reproduces the issue

param.Version.git_fetch runs git remote -v from the path of the file. If this is not a git repository, git will go up in the file tree until it finds a git repository, which can lead to some surprises and unexpected behavior.

The problem first appeared in GeminiDRSoftware/DRAGONS#244, where we run tox and import holoviews, which imports param. The path (self.fspath) in that case /.../dev/DRAGONS/.tox/py39-unit/lib/python3.9/site-packages/param/__init__.py and git returns the remote from our repository:

(Pdb) output
'origin\[email protected]:GeminiDRSoftware/DRAGONS.git (fetch)\norigin\[email protected]:GeminiDRSoftware/DRAGONS.git (push)'

(though Version is called here for param itself, from param/__init__.py).
Then repo_matches contains '', and '' in output is always True...
So it proceeds and parses the version but it crashes with DRAGONS' version. Which is probably not PEP440-compatible (v3.0.0-dev-1250-g9165f26bd-dirty), but that should not be param's problem.

param/param/version.py

Lines 177 to 185 in bbc1df8

output = run_cmd([cmd, 'remote', '-v'],
cwd=os.path.dirname(self.fpath))
repo_matches = ['', # No remote set
'/' + self.reponame + '.git' ,
# A remote 'server:reponame.git' can also be referred
# to (i.e. cloned) as `server:reponame`.
'/' + self.reponame + ' ']
if not any(m in output for m in repo_matches):
return self

Even better, when importing param in a fresh virtualenv, it's __version__ will not be correct in some case. I use pyenv so fspath is /home/me/.pyenv/versions/dragons/lib/python3.9/site-packages/param/__init__.py and thus param.__version__ will actually report pyenv's version ... :

❯ pyenv virtualenv param  
created virtual environment CPython3.9.4.final.0-64 in 249ms
  creator CPython3Posix(dest=/home/sconseil/.pyenv/versions/3.9.4/envs/param, clear=False, no_vcs_ignore=False, global=False)
  seeder FromAppData(download=False, pip=bundle, setuptools=bundle, wheel=bundle, via=copy, app_data_dir=/home/sconseil/.local/share/virtualenv)
    added seed packages: pip==21.0.1, setuptools==56.0.0, wheel==0.36.2
  activators BashActivator,CShellActivator,FishActivator,PowerShellActivator,PythonActivator,XonshActivator
pyenLooking in links: /tmp/sconseil/tmp_e9fnr3i
Requirement already satisfied: setuptools in /home/sconseil/.pyenv/versions/3.9.4/envs/param/lib/python3.9/site-packages (56.0.0)
Requirement already satisfied: pip in /home/sconseil/.pyenv/versions/3.9.4/envs/param/lib/python3.9/site-packages (21.0.1)

❯ pyenv activate param  

(param) param ❯ pip install param         
Collecting param
  Using cached param-1.10.1-py2.py3-none-any.whl (76 kB)
Installing collected packages: param
Successfully installed param-1.10.1
WARNING: You are using pip version 21.0.1; however, version 21.1.1 is available.
You should consider upgrading via the '/home/sconseil/.pyenv/versions/3.9.4/envs/param/bin/python -m pip install --upgrade pip' command.

(param) param ❯ python -c "import param; print(param.__version__)"
1.2.23.post97+gc010935a
@philippjfr
Copy link
Member

@jlstevens Wasn't this fixed?

@jlstevens
Copy link
Contributor

Yes it was fixed on autover here: holoviz-dev/autover#67 and needs to be propagated to param.

@jlstevens
Copy link
Contributor

I've ported the fix in #469

@jbednar jbednar added this to the v1.10.2 milestone May 10, 2021
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