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

nxdomain: init at 1.0.1 #101669

Merged
merged 1 commit into from Oct 28, 2020
Merged

nxdomain: init at 1.0.1 #101669

merged 1 commit into from Oct 28, 2020

Conversation

@zopieux
Copy link
Contributor

@zopieux zopieux commented Oct 25, 2020

Motivation for this change

nxdomain is a small tool to generate DNS block lists, available on PyPi and GitHub under GPLv3+.

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.
Copy link
Member

@SuperSandro2000 SuperSandro2000 left a comment

Successfully builds on darwin.

Copy link
Contributor

@r-burns r-burns left a comment

Works on linux too

Copy link
Member

@Ma27 Ma27 left a comment

As this is supposed to be an application, an executable should be in $out/bin.

@zopieux
Copy link
Contributor Author

@zopieux zopieux commented Oct 27, 2020

@Ma27 This is a module with a __main__ so you can do python -m foo. Do you want me to create a small bin/ bash wrapper that does <python> -m nxdomain "$@"?

@Ma27
Copy link
Member

@Ma27 Ma27 commented Oct 27, 2020

Actually I'm rather confused what's going on here: if upstream would configure executables properly[1], those should've been installed automatically into $out/bin.

[1] by setting console_scripts in setup() like this:

setup(
  entry_points={
    'console_scripts': [
      'executable = module.__main__:main'
    ]
  }
)

@zopieux
Copy link
Contributor Author

@zopieux zopieux commented Oct 27, 2020

Good point. I am "the upstream", so I did just that and verified it works:

$ nix-shell -I nixpkgs=$PWD -p nxdomain --command 'nxdomain --help'
usage: nxdomain [-h] --out OUT --format {bind,dnsmasq} [--simple SIMPLE] [--hosts HOSTS]

@zopieux zopieux requested a review from Ma27 Oct 27, 2020
@zopieux zopieux changed the title nxdomain: init at 1.0.0 nxdomain: init at 1.0.1 Oct 27, 2020
@@ -5902,6 +5902,8 @@ in

nwdiag = with python3Packages; toPythonApplication nwdiag;

nxdomain = python3.pkgs.callPackage ../tools/networking/nxdomain { };
Copy link
Contributor

@r-burns r-burns Oct 28, 2020

Ok, now that you have an executable, I think the typical pattern is to do nxdomain = with python3Packages; toPythonApplication nxdomain; and call nxdomain from pkgs/top-level/python-packages.nix

Copy link
Contributor Author

@zopieux zopieux Oct 28, 2020

The PR was merged before I could apply your feedback. I'll do that the next time I need to update the package. Thanks.

Ma27
Ma27 approved these changes Oct 28, 2020
@Ma27 Ma27 merged commit 3d04e9a into NixOS:master Oct 28, 2020
19 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

4 participants