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

pass-import: fix plugin usage #118143

Closed
wants to merge 2 commits into from
Closed

pass-import: fix plugin usage #118143

wants to merge 2 commits into from

Conversation

hennersz
Copy link
Contributor

@hennersz hennersz commented Mar 31, 2021

Motivation for this change

pass import needs magic to determine file types
the current python dependency used is
https://github.com/aliles/filemagic. This is very outdated and when
running the import plugin we get an error "AttributeError: module
'magic' has no attribute 'from_buffer'"
Pass import actually specifies this: https://github.com/file/file as the
required dependency. Using these python bindings fixes the error

pass-import relies on the PREFIX variable being set. Normally it is
available when the extension is called from pass because pass sources
the extension entry point script. Because of the way the extension gets
wrapped by nix we need to export PREFIX in the wrapper script.

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • 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 nixpkgs-review --run "nixpkgs-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.

pass import needs magic to determine file types
the current python dependency used is
https://github.com/aliles/filemagic. This is very outdated and when
running the import plugin we get an error "AttributeError: module
'magic' has no attribute 'from_buffer'"
Pass import actually specifies this: https://github.com/file/file as the
required dependency. Using these python bindings fixes the error
@SuperSandro2000
Copy link
Member

This is a semi-automatic executed nixpkgs-review with nixpkgs-review-checks extension. It is checked by a human on a best effort basis and does not build all packages (e.g. lumo, tensorflow or pytorch).
If you have any questions or problems please reach out to SuperSandro2000 on IRC.

Result of nixpkgs-review pr 118143 run on x86_64-linux 1

1 package built:
  • passExtensions.pass-import

@r-rmcgibbo
Copy link

Result of nixpkgs-review pr 118143 at 21980dd run on x86_64-linux 1

1 package built successfully:
  • passExtensions.pass-import
2 suggestions:
  • warning: missing-patch-comment

    Please add a comment on the line above, explaining the purpose of this patch.
    Near pkgs/tools/security/pass/extensions/import.nix:25:15:

       |
    25 |   patches = [ ./0001-Fix-installation-with-Nix.patch ];
       |               ^
    
  • warning: unused-argument

    Unused argument: fetchpatch.
    Near pkgs/tools/security/pass/extensions/import.nix:1:62:

      |
    1 | { lib, stdenv, fetchFromGitHub, pythonPackages, makeWrapper, fetchpatch }:
      |                                                              ^
    

pass-import relies on the PREFIX variable being set. Normally it is
available when the extension is called from pass because pass sources
the extension entry point script. Because of the way the extension gets
wrapped by nix we need to export PREFIX in the wrapper script.
@hennersz hennersz changed the title pass-import: use magic instead of filemagic pass-import: fix plugin usage Mar 31, 2021
@SuperSandro2000
Copy link
Member

@ofborg eval

@hennersz
Copy link
Contributor Author

closing due to changes made in #117165

@hennersz hennersz closed this Apr 15, 2021
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

3 participants