Skip to content

Add --save and --save-to options to install #3873

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

Closed
wants to merge 2 commits into from
Closed

Add --save and --save-to options to install #3873

wants to merge 2 commits into from

Conversation

mkurnikov
Copy link
Contributor

@mkurnikov mkurnikov commented Jul 24, 2016

Resolves #1479.

--save is a boolean flag to save a name==version line to the requirements file, and --save-to specifies which requirements file to use.

Features:

  • updates package line in the requirements file if package already exists in the file
  • fails with FileNotFoundError if base directory for the requirements file does not exist
  • preserves file content (i.e. -r requirements.txt in the beginning of the file)

saved_packages[name] = line_number

for requirement in requirement_set.requirements.values():
if not requirement.comes_from: # not a dependency of smth
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is smth?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"Something". I'll remove a comment in a couple of hours, it's redundant there.

@mkurnikov
Copy link
Contributor Author

@xavfernandez Could it be possibly added to 8.2 milestone? Is there anything else I have to add before?

@xavfernandez
Copy link
Member

Hello @mkurnikov ,

Thanks for your contribution but I'm not sure we want to include this feature.

A limitation of the current implementation is if you have a requirement.txt only containing -r req2.txt and req2.txt containing pep8.
You will end up adding pep8 in req.txt even so it already there (via req2.txt).

Extras (requests[security], etc) are also ignored.

Maybe @pfmoore or @dstufft will feel differently though.

@@ -0,0 +1,56 @@
def test_save_installed_package_in_requirements(script):
freezed_requests = 'requests==1.0.0'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nitpick: "frozen" rather than "freezed".

@pfmoore
Copy link
Member

pfmoore commented Jul 28, 2016

I'm concerned that there are a lot of vague areas here. What if the requirements file contains a URL for the same project as we just installed? Will we end up with the project name plus the URL? As @xavfernandez points out, what about extras? And I don't think the code copes with a requirements line like foo>=1.0 - it seems to test specifically for ==.

When I've seen this feature requested, the motivation seems to typically be "like npm". For those of us who don't know npm, I think fuller documentation is needed - and that documentation should cover precisely how the feature interacts with an existing requirements file (covering the various complexities that could be in such a file - I have the feeling that the npm equivalent of a requirements file is much simpler than pip's version).

Maybe a more practical approach would be for --save to only work if the specified requirements file doesn't already exist. So it writes what got installed to a brand new file, and then it's up to the user to manually merge that with any project-level requirements file. That would be much simpler to specify and code. The downside is that it might not actually solve the users' problem :-(

There's obviously a need for something like this - the request keeps coming up. But I'd rather see a good spec of what's needed before we dive into implementation.

@mkurnikov
Copy link
Contributor Author

mkurnikov commented Jul 28, 2016

Can't we just move to a single json file for all the dependencies, like it's been done in the npm? Is there any purpose of having the requirements.txt file in the current form besides the possibility of comments?

@pfmoore
Copy link
Member

pfmoore commented Jul 28, 2016

That would be a major compatibility breaking change. You'd need a far better justification to even consider something like that. And if you removed any of the capabilities currently in requirements files (for example, the ability to specify options or extra indexes) you'd need to justify that and offer an alternative solution for the people using those capabilities.

tl; dr; No, we can't.

@mkurnikov
Copy link
Contributor Author

mkurnikov commented Jul 28, 2016

I can imagine something like

{
    "options": {
        "--index-url": "some_url",
        "--find-links": "/my/local/archives"
    }, 
    "dependencies": [
        {
            "name": "django",
            "version_specifier": "==1.9.8", 
            "source_url": "pypi"
        },
        {
            "name": "django-rest-framework",
            "version_specifier": "==3.4.0",
            "source_url": "[email protected]:tomchristie/django-rest-framework.git"
        }
    ]
}

and backward compatibility through generation of the requirements.txt file based on this json. I'm not sure about the order of the records in the generated file, though.

But yes, I completely understand, that is exactly what I thought the answer will be.

@mkurnikov
Copy link
Contributor Author

I'm closing the PR until better times.

@mkurnikov mkurnikov closed this Jul 30, 2016
@lock lock bot added the auto-locked Outdated issues that have been locked by automation label Jun 3, 2019
@lock lock bot locked as resolved and limited conversation to collaborators Jun 3, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
auto-locked Outdated issues that have been locked by automation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add a --save option to install ala npm's save option
4 participants