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

python3Package.aionotify: init at 0.2.0 #98406

Merged
merged 1 commit into from Sep 29, 2020
Merged

python3Package.aionotify: init at 0.2.0 #98406

merged 1 commit into from Sep 29, 2020

Conversation

thiagokokada
Copy link
Contributor

@thiagokokada thiagokokada commented Sep 21, 2020

Motivation for this change

Add python-aionotify package: https://github.com/rbarrois/aionotify

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.

@thiagokokada
Copy link
Contributor Author

@jonringer Can you review it again please?

@thiagokokada
Copy link
Contributor Author

Ping @jonringer again (and @FRidh).

@jonringer
Copy link
Contributor

the template is there for a reason, running nixpkgs-review saves me from having to run it for you

cannot build derivation '/nix/store/73khy0q1jkh1vxw2vd2m3nnp1j2n5hy2-env.drv': 2 dependencies couldn't be built
[0 built (2 failed), 0.0 MiB DL]
error: build of '/nix/store/73khy0q1jkh1vxw2vd2m3nnp1j2n5hy2-env.drv' failed
https://github.com/NixOS/nixpkgs/pull/98406
2 packages failed to build:
python37Packages.aionotify python38Packages.aionotify

@thiagokokada
Copy link
Contributor Author

thiagokokada commented Sep 29, 2020

I did try to run nixpkgs-review. I just didn't mark in the template because I got this:

$ nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
these paths will be fetched (0.04 MiB download, 0.14 MiB unpacked):
  /nix/store/1f7ckaqwr5pgjc0rcvcylwjzv4i8pvkn-nixpkgs-review-2.4.0
copying path '/nix/store/1f7ckaqwr5pgjc0rcvcylwjzv4i8pvkn-nixpkgs-review-2.4.0' from 'https://cache.nixos.org'...
$ git -c fetch.prune=false fetch --force https://github.com/NixOS/nixpkgs master:refs/nixpkgs-review/0
remote: Enumerating objects: 2665, done.
remote: Counting objects: 100% (2665/2665), done.
remote: Compressing objects: 100% (168/168), done.
remote: Total 5889 (delta 2618), reused 2506 (delta 2496), pack-reused 3224
Receiving objects: 100% (5889/5889), 4.06 MiB | 8.83 MiB/s, done.
Resolving deltas: 100% (4299/4299), completed with 1062 local objects.
From https://github.com/NixOS/nixpkgs
   366a677dbba..f89469957dc  master     -> refs/nixpkgs-review/0
$ git worktree add /home/thiagoko/.cache/nixpkgs-review/rev-db05be008b877bb75dc0fe45906452cb79d7702a-dirty/nixpkgs f89469957dcd2ca1c897325228ff9023f99a5725
Preparing worktree (detached HEAD f89469957dc)
Updating files: 100% (22804/22804), done.
HEAD is now at f89469957dc azure-cli: 2.11.1 -> 2.12.1
$ nix-env -f /home/thiagoko/.cache/nixpkgs-review/rev-db05be008b877bb75dc0fe45906452cb79d7702a-dirty/nixpkgs -qaP --xml --out-path --show-trace
No diff detected, stopping review...
$ git worktree prune

Maybe I am running it wrong, but I did exactly what the PR template is showing.

@jonringer
Copy link
Contributor

the wip command is for running it when you have unstaged changes. since you already have a pr open, you need to do

nixpkgs-review pr 98406

@thiagokokada thiagokokada marked this pull request as draft September 29, 2020 13:45
@thiagokokada thiagokokada marked this pull request as ready for review September 29, 2020 14:01
@thiagokokada
Copy link
Contributor Author

thiagokokada commented Sep 29, 2020

@jonringer Done, sorry for the unnecessary work. But should work now:

$ nix-shell -p nixpkgs-review --run "nixpkgs-review pr 98406"       
these paths will be fetched (0.09 MiB download, 0.42 MiB unpacked):
  /nix/store/1f7ckaqwr5pgjc0rcvcylwjzv4i8pvkn-nixpkgs-review-2.4.0
  /nix/store/d43v6bx7r6fcaq3fbbfd5mwh1f5s7rmg-bash-interactive-4.4-p23-dev
copying path '/nix/store/d43v6bx7r6fcaq3fbbfd5mwh1f5s7rmg-bash-interactive-4.4-p23-dev' from 'https://cache.nixos.org'...
copying path '/nix/store/1f7ckaqwr5pgjc0rcvcylwjzv4i8pvkn-nixpkgs-review-2.4.0' from 'https://cache.nixos.org'...
$ git -c fetch.prune=false fetch --force https://github.com/NixOS/nixpkgs master:refs/nixpkgs-review/0 pull/98406/head:refs/nixpkgs-review/1
$ git worktree add /home/thiagoko/.cache/nixpkgs-review/pr-98406/nixpkgs b76670f139e140afe0f49016e0beb85b086add48
Preparing worktree (detached HEAD b76670f139e)
Updating files: 100% (22805/22805), done.
HEAD is now at b76670f139e duf: 0.3.0 -> 0.3.1 (#99092)
$ nix-env -f /home/thiagoko/.cache/nixpkgs-review/pr-98406/nixpkgs -qaP --xml --out-path --show-trace
$ git merge --no-commit ba0c83e44f6ff8a5c67137d481a16c93de641f75
Automatic merge went well; stopped before committing as requested
$ nix-env -f /home/thiagoko/.cache/nixpkgs-review/pr-98406/nixpkgs -qaP --xml --out-path --show-trace --meta
2 packages added:
python3.7-aionotify-v0.2.0 python3.8-aionotify-v0.2.0

$ nix build --no-link --keep-going --option build-use-sandbox relaxed -f /home/thiagoko/.cache/nixpkgs-review/pr-98406/build.nix
warning: ignoring the user-specified setting 'sandbox', because it is a restricted setting and you are not a trusted user
[4 built, 29 copied (103.7 MiB), 20.2 MiB DL]
https://github.com/NixOS/nixpkgs/pull/98406
2 packages built:
python37Packages.aionotify python38Packages.aionotify

$ nix-shell /home/thiagoko/.cache/nixpkgs-review/pr-98406/shell.nix
these paths will be fetched (0.51 MiB download, 2.47 MiB unpacked):
  /nix/store/3qhafmf9yj5nyyrnm9xnil6vl6gglsf5-bash-interactive-4.4-p23-info
  /nix/store/546gk8pnvdya2m2a6y791ppr71wj2imq-bash-interactive-4.4-p23-doc
  /nix/store/l2g6my7nykfshahijqnk7gy7s5lq2r3k-bash-interactive-4.4-p23-man
  /nix/store/rlknkw60sngvky1rkrdws0qik3scjcln-bash-interactive-4.4-p23-dev
copying path '/nix/store/546gk8pnvdya2m2a6y791ppr71wj2imq-bash-interactive-4.4-p23-doc' from 'https://cache.nixos.org'...
copying path '/nix/store/3qhafmf9yj5nyyrnm9xnil6vl6gglsf5-bash-interactive-4.4-p23-info' from 'https://cache.nixos.org'...
copying path '/nix/store/l2g6my7nykfshahijqnk7gy7s5lq2r3k-bash-interactive-4.4-p23-man' from 'https://cache.nixos.org'...
copying path '/nix/store/rlknkw60sngvky1rkrdws0qik3scjcln-bash-interactive-4.4-p23-dev' from 'https://cache.nixos.org'...

[nix-shell:~/.cache/nixpkgs-review/pr-98406]$ 

But I learned a lot from this PR, thanks for the inputs 👍 .

Copy link
Contributor

@jonringer jonringer left a comment

Choose a reason for hiding this comment

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

the v${version} paradigm is just help distinguish tags in version control. The version shouldn't have the prefix

also allows for the bot to update this package

pkgs/development/python-modules/aionotify/default.nix Outdated Show resolved Hide resolved
pkgs/development/python-modules/aionotify/default.nix Outdated Show resolved Hide resolved
@thiagokokada
Copy link
Contributor Author

@jonringer Sorry about the version thing. Anything else?

Copy link
Contributor

@jonringer jonringer left a comment

Choose a reason for hiding this comment

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

LGTM

Result of nixpkgs-review pr 98406 1

2 packages built:
  • python37Packages.aionotify
  • python38Packages.aionotify

@jonringer jonringer merged commit 9a43f55 into NixOS:master Sep 29, 2020
@thiagokokada thiagokokada deleted the python-aionotify-init branch September 29, 2020 23:45
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

2 participants