#4211 [Configuration] Remove pygit2.
Closed 3 years ago by pingou. Opened 5 years ago by hobbestigrou.
hobbestigrou/pagure remove_pygit_from_requirements  into  master

file modified
-1
@@ -18,7 +18,6 @@ 

  munch

  Pillow

  psutil

- pygit2 >= 0.26.0

  python-openid;python_version<="2.7"

  python3-openid;python_version>="3.0"

  python-openid-cla

Remove pygit2 from requirements. The version depends of the libgit2
version. So is useless to have this in requirements and that can create
problem if the user install the library before install the requirements.

Depends on the system dependencies If user install a specific version of libgit2 (by example 0.25.1) and pygit2 (related version 0.25.1) and after this user install requirements by using pip install -r requirements.txt like described in the documentation then requirements rules upgrade the package due to this rule pygit2 >= 0.26.0

In this situation user can facing an issue.

For further reading https://www.pygit2.org/install.html#version-numbers about related version between libgit2 and pygit2.

:thumbsup:

Also, pygit2 has wheels that include libgit2 in them on PyPI, too. So you can use that if you want.

Depends on the system dependencies If user install a specific version of libgit2 (by example 0.25.1) and pygit2 (related version 0.25.1) and after this user install requirements by using pip install -r requirements.txt like described in the documentation then requirements rules upgrade the package due to this rule pygit2 >= 0.26.0

There are a few reasons to keep it there:
- It's pretty much the standard location to declare all the dependencies
- 0.26.0 is the minimal required version, so if you have 0.25.1 and want to run pagure, pygit2 will try to upgrade but for good reasons, we do not know how pagure will behave with 0.25.0

I am a little inclined on leaving it there for these reasons

So I'm not sure to really understand why the documentation ask user to install manually pygit. Please can you give me more details?
The documentation is a little bit confusing about this point, I guess we need to keep the requirement in the requirements file and remove the documentation section where we ask user to install manually pygit.

Do you agree?

In this case it's better to remove the part about this manually install of pygit2 in the documentation because is confusing.

So I'm not sure to really understand why the documentation ask user to install manually pygit.

Because it really depends on the libgit2 version installed, and if there is a version installed (that satisfies the minimal version required) pip won't try to upgrade it.

The content of the requirements.txt file is also included in the setup.py which then reflects in the .egg-info files.

So do you think it's possible to remove the manual installation since the requirements would install pygit anyways?

So do you think it's possible to remove the manual installation since the requirements would install pygit anyways?

We could but then pip will try to install the latest pygit2 release which may not be the one that is compatible with the libgit2 installed on your machine :)

(Is it worth mentioning it's a little messy? :D)

Hey folks,

So what do we want to do about this PR? Should we close it, adjust it, merge it?

I've also just realized that the content of the requirements.txt is loaded in the setup.py which means it also appears in pagure.egg-info/requires.txt so removing it from requirements.txt means pagure will not longer fail to start on a system with no pygit2 installed.

I'm not sure personally...

By re-reading your documentation I guess you can merge this one due to the fact that you already request for a pygit install just before running pip install -r requirements.txt .

I guess this patch fix a misconfiguration error.

Example:

$ pip freeze # nothing is installed
$ pip install niet==1.3         
Collecting niet==1.3
  Using cached https://files.pythonhosted.org/packages/b2/1a/df9016023c926b5d522104c3efc5238a986ab6cd5e1c4fc737f9dc03f8a6/niet-1.3.0-py2.py3-none-any.whl
Collecting PyYAML (from niet==1.3)
Collecting future (from niet==1.3)
Installing collected packages: PyYAML, future, niet
Successfully installed PyYAML-5.1 future-0.17.1 niet-1.3.0
$ pip freeze # we can see that niet is in version 1.3.0
future==0.17.1
niet==1.3.0
PyYAML==5.1
$ echo "niet==1.4.2" > requirements.txt 
$ pip install -r requirements.txt
Collecting niet==1.4.2 (from -r requirements.txt (line 1))
  Using cached https://files.pythonhosted.org/packages/53/a6/689a583c63baf05ee1f53ca9829e84fb1f7b63e1332148287d4dd352579d/niet-1.4.2-py2.py3-none-any.whl
Requirement already satisfied: future in /home/hberaud/.local/share/virtualenvs/pyg-q9ZrAq7N/lib/python3.7/site-packages (from niet==1.4.2->-r requirements.txt (line 1)) (0.17.1)
Requirement already satisfied: PyYAML in /home/hberaud/.local/share/virtualenvs/pyg-q9ZrAq7N/lib/python3.7/site-packages (from niet==1.4.2->-r requirements.txt (line 1)) (5.1)
Installing collected packages: niet
  Found existing installation: niet 1.3.0
    Uninstalling niet-1.3.0:
      Successfully uninstalled niet-1.3.0
Successfully installed niet-1.4.2
$ pip freeze # the niet module was upgraded...               
future==0.17.1
niet==1.4.2
PyYAML==5.1

You are in the same use case that the previous example.

We should close this. We use requirements.txt to populate dependencies in setuptools and in packaging, too.

Personally I doesn't have the big picture of the pagure project so I can't identify possible side effects and I'm not the owner of these changes so in my point of view feel free to close it.

This doesn't seem to have made many people trip over the years, so I am inclined to keep pygit2 in the requirements.txt and as such I will close this PR.

Thank you for your contribution and the discussion around this point though, it was really interesting!

Pull-Request has been closed by pingou

3 years ago
Metadata