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

Add autokey application #35392

Closed
wants to merge 1 commit into from
Closed

Add autokey application #35392

wants to merge 1 commit into from

Conversation

JohnJohnstone
Copy link

Motivation for this change

To add the text expander autokey

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option build-use-sandbox in nix.conf on non-NixOS)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nox --run "nox-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Fits CONTRIBUTING.md.

@@ -598,6 +598,8 @@ with pkgs;

autorevision = callPackage ../tools/misc/autorevision { };

autokey = callPackage ../tools/x11/autokey { };
Copy link
Contributor

Choose a reason for hiding this comment

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

Looks like it should be X11 not x11.

propagatedBuildInputs = with python3Packages; [ xlib pyinotify dbus-python ];

pythonPath = with python3Packages;
[ dbus-python ];
Copy link
Member

Choose a reason for hiding this comment

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

It's probably sufficient to only add dbus-python in propagatedBuildInputs.

python3Packages.buildPythonApplication rec {
pname = "autokey";
version = "0.94.0";
name = "${pname}-${version}";
Copy link
Member

Choose a reason for hiding this comment

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

When using buildPythonApplication and buildPythonPackage the name attribute defaults to `"${pname}-${version}"' so this live can be removed.

version = "0.94.0";
name = "${pname}-${version}";

src = fetchurl {
Copy link
Member

Choose a reason for hiding this comment

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

You should use fetchPypi or fetchFromGitHub.

pythonPath = with python3Packages;
[ dbus-python ];

meta = with stdenv.lib; {
Copy link
Member

Choose a reason for hiding this comment

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

Are you willing to maintain this package?
If so please add yourself to lib/maintainers.nix and reference yourself in meta.maintainers.

Copy link
Author

Choose a reason for hiding this comment

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

Once i get my head round nix expressions and packaging i would gladly maintain this package as i use it daily.

@@ -0,0 +1,24 @@
{ stdenv, fetchurl, python3Packages }:
Copy link
Member

Choose a reason for hiding this comment

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

Could you please replace callPackage in all-packages.nix with python3Packages.callPackage and explicitly declare all inputs to the derivation in the function arguments?

With that I mean replace the line { stdenv, fetchurl, python3Packages }: with { stdenv, fetchurl, xlib, pyinotify, dbus-python, buildPythonApplication }:.

Copy link
Author

Choose a reason for hiding this comment

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

Thank you the package starts building but fails due to dbus-python not being found, which is why i had dbus-python in buildinputs but no sucess

@tomberek
Copy link
Contributor

close due to inaction?

@JohnJohnstone
Copy link
Author

close due to inaction?

Yes im not using nixpkgs anymore.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants