Conversation
There was a problem hiding this comment.
without the patch plugins can't be managed
There was a problem hiding this comment.
with this change plover still works in a nix-shell
but fails to start in my environment
switch to fetchFromGitHub and correct license
This reverts commit 3a6ac8e6d351e966baaf40a5ce573b0d7fd79238. this change prevents starting in my user environment (still works in a pure shell)
without it: Error while finding module specification for 'plover_plugins_manager.pip_wrapper' (ModuleNotFoundError: No module named 'plover_plugins_manager')
| src = fetchFromGitHub { | ||
| owner = "openstenoproject"; | ||
| repo = "plover"; | ||
| rev = "54dbcf4ea73cc1ecc1d7c70dbe7bdb13f055d101"; |
There was a problem hiding this comment.
| rev = "54dbcf4ea73cc1ecc1d7c70dbe7bdb13f055d101"; | |
| rev = "v${version}"; |
There was a problem hiding this comment.
nope, they're doing a moving tag of "continuous" but still have a versioning scheme that's no longer integrated with git
|
|
||
| buildPythonPackage rec { | ||
| pname = "requests-futures"; | ||
| version = "0.9.9-27-gf126048"; |
There was a problem hiding this comment.
From where did you get the version number?
There was a problem hiding this comment.
git describe --tags minus the leading v
| }; | ||
| }; | ||
|
|
||
| dev-with-plugins = dev.overrideAttrs (old: { |
There was a problem hiding this comment.
I can only echo my thoughts on #110658 that including the plugin manager is expected functionality with the dev branch of plover. It's what you'll get if you're installing plover dev with any other package manager, or the appimage, or whatever else.
If anything, now that my experience with Nix has grown, I'd like to see an argument that can be overridden (to false) on plover dev that enables the package manager by default, rather than creating two separate derivations that are built, something like
plover.dev.override {
withPluginManager = false;
}There was a problem hiding this comment.
Actually, thinking further, I'm not exactly happy that the plugin-manager derivation is exposed as it is either.
While there is the potential usecase that one would want to override the plugin manager, the way it's currently exposed gives no particularly elegant ability to do so (other than simply repeating your plover-with-plugins derivation wholesale). It shouldn't be exposed in the plover tree's top level as it currently is. My best image for how it should be exposed while giving a way to override it probably looks like putting it under a subtree like plover.devPlugins.plugin-manager, then plover.dev could expose, perhaps, both a withPluginManager and plugins override argument. If you wanted to swap out the plugin manager with an overridden version, you would set withPluginManager to false, then add your overridden version to plugins.
I'm more than happy for more brainstorming on the subject, though, I admit this probably isn't the most elegant solution but I think we need to deal with a couple of concerns, including:
- the plugin-manager being included being an expected default
- allowing users to disable it if they wish
- making the
ploversubtree not bloated, and - considering the possibility of extending plover with a
withPlugins-like functionality in the future, which this is already great groundwork for
There was a problem hiding this comment.
the right solution™ is probably to accept a list of plugins
then the (default) setting of an empty list would not provide the plugin manager
as far as i know this would require packaging all the plugins in nixpkgs
which i'm not willing to do because plover doesn't work on wayland...
for now, setting plover.dev to include the plugin manager does seem like the right way forward
unfortunately i seem to recall the issue with this PR being,
plover not reliably finding the site packages (plugins) installed by the plugin manager
(between a nix-env install, a nix-shell, and a NixOS systemPackages install)
which is something i don't know how to fix, but would like to see fixed before seeing this PR merged
alternatively, we could merge something that works in 1 use-case and let users get annoyed enough to fix it themselves
There was a problem hiding this comment.
You could probably see if it's possible to pass the site packages in an environmental variable?
It would need to be the same place recognized by the plugin manager and writeable though, but the only two cases I see are:
- executed by a user with a $HOME directory, and
- executed by a user without a $HOME directory (which, frankly, given the use-case of plover may not be a problem in the first place)
There was a problem hiding this comment.
This can be done with makeWrapper using the --run flag and a '-quoted argument to it using $HOME, you can see how my emacs config using Nix does similar here
I imagine something like (assuming the variable needed is SITE_PACKAGES and both the plugin manager and plover would read from it correctly)
makeWrapper "PATH/TO/PLOVER/EXECUTABLE" "$out/bin/plover"
--run 'mkdir -p "$HOME/PATH/TO/YOUR/SITEPACKAGES"'
--run 'export SITE_PACKAGES="$HOME/PATH/TO/YOUR/SITEPACKAGES"'would work.
There was a problem hiding this comment.
Of course, you would also need the site packages that nixpkgs' buildPythonPackage creates, but I'm sure it's possible to do with a bit of work.
At worst you could probably do some patching of plover and the plugin manager here or there to enable it.
There was a problem hiding this comment.
Upon research you may be interested in the PYTHONUSERBASE environmental variable, which I don't believe that buildPythonPackage co-opts for its own usage.
|
I marked this as stale due to inactivity. → More info |
based on / closes #110658
closes #89341
Motivation for this change
help some community members
Things done
sandboxinnix.confon non-NixOS linux)nix-shell -p nixpkgs-review --run "nixpkgs-review wip"./result/bin/)nix path-info -Sbefore and after)remaining issues (help wanted)
plugins don't work in a nix-shell
upstream recommendations (don't use the patch, use dist_main entry point) break things