search

getpelican / pelican-plugins

Collection of plugins for the Pelican static site generator
Other
1.38k stars 847 forks source link

Display a warning for git submodules repos installable with pip #1189

Open Lucas-C opened 5 years ago

Lucas-C commented 5 years ago

The following repos from .gitmodules can be installed with pip:

We should find a way to display a deprecation warning to users of this repo to encourage them to switch to the pip install $plugin method of installation.

avaris commented 5 years ago

If the authors want those plugins to be pip-only, then they should remove submodules. We can extend readme with a list of "pip-installable-plugins". Otherwise, I don't see the point of a deprecation warning.

Lucas-C commented 5 years ago

@avaris: My end goal is to remove git submodules from this repo for plugins that can be installed with pip. In order to do that, I think we should warn our users. Otherwise removing submodules could break their static website with an obscure "plugin not found" error.

We could actually directly remove those gitmodules, and for each of them create a $plugin/__init__.py file containing:

import warnings

def register():
    warnings.warn("Plugin $plugin has been moved to Pypi. Install it with: pip install $plugin", DeprecationWarning)

But by doing so directly, pelican-plugins users will be forced to switch directly, with no period of time where the warning would be displayed and the plugin would continue to function.

Lucas-C commented 5 years ago

In fact, in the list above, only pelican-meetup-info & pelican-jinja2content have an __init__.py file and can be used through this repo and its git submodules.

I thinks we can simply remove all the other repos in this list from pelican-plugins git submodules. Would you agree with that @avaris ?

Also, for pelican-meetup-info, I tested having both pelican-plugins configured and doing pip install pelican-meetup-info: when using the plugin, the one in pelican-plugins is used instead of the one installed with pip :(

Lucas-C commented 5 years ago

I submitted this PR in order for pelican-plugins plugins to be overriden by the ones from Pypi: https://github.com/getpelican/pelican/pull/2617

avaris commented 5 years ago

I feel like the decision to "keep submodule or remove it in favor of pip" should be left to the author of the plugin.

Also, for pelican-meetup-info, I tested having both pelican-plugins configured and doing pip install pelican-meetup-info: when using the plugin, the one in pelican-plugins is used instead of the one installed with pip :(

That's normal. pelican prepends PLUGIN_PATHS to the front of the sys.path. They get priority when importing. If you want, you can import it in the settings and put the module in PLUGINS. That would avoid precedence issues since at the time settings is read PLUGIN_PATHS will not be importable:

import some_plugin

PLUGIN_PATHS = [...]
PLUGINS = [some_plugin, 'some_other_plugin_from_plugin_paths']
Lucas-C commented 5 years ago

Ah, interesting ! Thanks for your answer @avaris

I think the decision is not to "keep submodule or remove it in favor of pip". The plugins listed at the top of this issue, with the exception of pelican-meetup-info & pelican-jinja2content, cannot be used through this "proxy" repo now that they have been moved to Pypi, as they do not have a __init__.py file at their root. Hence, the question for those git submodules is: shouldn't we remove them as they are effectively useless, and can even bring confusion ?

Lucas-C commented 5 years ago

Your code snippet is very interesting. Don't you think it may be helpful to add a section explaining this in this repo README ? Something like "How to deal with conflicting plugins"

avaris commented 5 years ago

Well, you can still use them if you add the subfolder to PLUGIN_PATHS:

PLUGIN_PATHS = ['pelican-plugins', 'pelican-plugins/pelicanfly']
PLUGINS = ['pelicanfly']

Not ideal, but also not impossible :). But, I see your point and I am OK(-ish) with that. However, authors can also do this easily: add a __init__.py in their root folder and either act as normal plugin or defer to PyPi with a message.

About the implementation, it certainly should not be DeprecationWarning. It is used when you deprecate stuff but still keep it working for the time being. With your proposed method, it won't be working anymore. An error is more appropriate.

Your code snippet is very interesting. Don't you think it may be helpful to add a section explaining this in this repo README ? Something like "How to deal with conflicting plugins"

Importing plugins directly and using them in PLUGINS (instead of strings) is already in the README. If you think this would be common issue, sure, nothing wrong with more documentation :).

Lucas-C commented 5 years ago

Ok, thanks @avaris for your detailed answer.

As a reminder to myself, I plan to open the following PRs then: