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.

Sign up for GitHub

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

Jump to bottom

pytaglib: init at 1.4.3 #39498

Merged
merged 3 commits into from Apr 26, 2018
Merged

pytaglib: init at 1.4.3 #39498

merged 3 commits into from Apr 26, 2018

Conversation

mrkkrp
Copy link
Member

@mrkkrp mrkkrp commented Apr 25, 2018 •
edited

Motivation for this change

I need the binding.

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.

Problems:

  • Doesn't pass the tests because of some encoding issue:
======================================================================
ERROR: test_unicode_filename (tests.test_bytes_unicode.TestBytesUnicode)
Ensure file can be opened if filename is provided as bytes object.
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/tmp/nix-build-python2.7-pytaglib-1.4.3.drv-0/pytaglib-1.4.3/tests/test_bytes_unicode.py", line 25, in test_unicode_filename
    with copyTestFile('testöü.flac') as f:
  File "/nix/store/l9j2jsc9flrbmpf799nw9wdq59gpwms8-python-2.7.14/lib/python2.7/contextlib.py", line 17, in __enter__
    return self.gen.next()
  File "/tmp/nix-build-python2.7-pytaglib-1.4.3.drv-0/pytaglib-1.4.3/tests/__init__.py", line 39, in copyTestFile
    shutil.copyfile(orig_file, copy_file)
  File "/nix/store/l9j2jsc9flrbmpf799nw9wdq59gpwms8-python-2.7.14/lib/python2.7/shutil.py", line 82, in copyfile
    if _samefile(src, dst):
  File "/nix/store/l9j2jsc9flrbmpf799nw9wdq59gpwms8-python-2.7.14/lib/python2.7/shutil.py", line 72, in _samefile
    return os.path.samefile(src, dst)
  File "/nix/store/l9j2jsc9flrbmpf799nw9wdq59gpwms8-python-2.7.14/lib/python2.7/posixpath.py", line 155, in samefile
    s1 = os.stat(f1)
UnicodeEncodeError: 'ascii' codec can't encode characters in position 76-77: ordinal not in range(128)

----------------------------------------------------------------------
Ran 18 tests in 0.009s

FAILED (errors=7)
Test failed: <unittest.runner.TextTestResult run=18 errors=7 failures=0>
error: Test failed: <unittest.runner.TextTestResult run=18 errors=7 failures=0>
builder for '/nix/store/9m38142507d4d0p38cxx1kznfzdbjl1x-python2.7-pytaglib-1.4.3.drv' failed with exit code 1
error: build of '/nix/store/9m38142507d4d0p38cxx1kznfzdbjl1x-python2.7-pytaglib-1.4.3.drv' failed

I'm sure this must be solvable I just don't know how.

  • I'm not sure about "tested using sandboxing". Should I rebuild my NixOS with nix.useSandbox = true and then test it? This means that I should modify my system in order to test a package? Not nice, probably I'm missing something.
  • nix-shell -p nox --run "nox-review wip" says "No uncommit changes. Did you mean to use the "--against" option?". So I tried it like this: nix-shell -p nox --run "nox-review wip --against master", it failed because of the same encoding issue.
  • "meta.maintainers should be set", should I add myself there?

@mrkkrp
pytaglib: init at 1.4.3
cfebc9b 
Add the pytaglib package.
@mrkkrp mrkkrp requested a review from FRidh as a code owner April 25, 2018 18:17
Mic92
Mic92 reviewed Apr 25, 2018
View reviewed changes
pkgs/development/python-modules/pytaglib/default.nix
meta = {
homepage = https://github.com/supermihi/pytaglib;
description = "Python 2.x/3.x bindings for the Taglib audio metadata library";
license = lib.licenses.gpl3;
Copy link
Member

Choose a reason for hiding this comment

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

Do you want to maintain this package? https://nixos.wiki/wiki/Nixpkgs#Becoming_a_Nixpkgs_maintainer

Copy link
Member Author

@mrkkrp mrkkrp Apr 26, 2018 •
edited

Choose a reason for hiding this comment

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

Well I could, but I'm neither a Python nor Nix expert :)

Copy link
Member

@Mic92 Mic92 Apr 26, 2018 •
edited

Choose a reason for hiding this comment

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

The point is, if you can test if pytaglib works or not. Nix expression can be corrected in pull request.

Copy link
Member Author

Choose a reason for hiding this comment

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

OK, I can be a maintainer then. I’ll add myself in this PR later today.

@Mic92
Copy link
Member

Mic92 commented Apr 25, 2018

nox-review wip is supposed to be used against uncommited changes.
You can also use nix-review, which is faster anyway: https://github.com/Mic92/nix-review

$ cd nixpkgs
$ nix-review pr 39498

@Mic92
Copy link
Member

Mic92 commented Apr 25, 2018

Nix sandboxes are not enabled by default because they slow down some operations. It is not so significant, which is why many people have some just enabled by default by having nix.useSandbox = true; in configuration.nix.

@dotlambda
Copy link
Member

@GrahamcOfBorg build python2.pkgs.pytaglib python3.pkgs.pytaglib

@GrahamcOfBorg
Copy link

Failure on aarch64-linux (full log)

Attempted: python2.pkgs.pytaglib, python3.pkgs.pytaglib

Partial log (click to expand)

khavkine1.cpp: In function ‘PyObject* __pyx_f_9khavkine1___pyx_unpickle_T__set_state(__pyx_obj_9khavkine1_T*, PyObject*)’:
khavkine1.cpp:1916:7: warning: ‘__pyx_t_2’ may be used uninitialized in this function [-Wmaybe-uninitialized]
   int __pyx_t_2[1];
       ^~~~~~~~~
==============================
building of '/nix/store/g9ccyb85m5kxbfb3gy26840d90qxsf9k-python2.7-Cython-0.28.1.drv' timed out after 3600 seconds
building of '/nix/store/l60jan1mgs2yd0lnkmsprzjapx4rynv9-python3.6-Cython-0.28.1.drv' timed out after 3600 seconds
cannot build derivation '/nix/store/7dlrpzp5w8qii93i4n825djvmlxpxay7-python2.7-pytaglib-1.4.3.drv': 1 dependencies couldn't be built
cannot build derivation '/nix/store/5wgmvh51v19hr5zmkpjl4nj2pkahr8xd-python3.6-pytaglib-1.4.3.drv': 1 dependencies couldn't be built
�[31;1merror:�[0m build of '/nix/store/5wgmvh51v19hr5zmkpjl4nj2pkahr8xd-python3.6-pytaglib-1.4.3.drv', '/nix/store/7dlrpzp5w8qii93i4n825djvmlxpxay7-python2.7-pytaglib-1.4.3.drv' failed

@Mic92 Mic92 merged commit 2a4dc39 into NixOS:master Apr 26, 2018
@dotlambda
Copy link
Member

@mrkkrp Next time, please squash your commits and prepend pythonPackages. to the commit nessages.

@Mic92
Copy link
Member

Mic92 commented Apr 26, 2018 •
edited

I squashed it on merge, but did not prepend pythonPackages: 2a4dc39

@mrkkrp mrkkrp deleted the add-pytaglib branch April 26, 2018 17:14
@GrahamcOfBorg
Copy link

Failure on x86_64-linux (full log)

Partial log (click to expand)

Merge failed

Synthetica9 pushed a commit to Synthetica9/nixpkgs that referenced this pull request May 3, 2018
@mrkkrp @Synthetica9
pytaglib: init at 1.4.3 (NixOS#39498)
4ef96f4
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Mic92 Mic92 Mic92 left review comments

@FRidh FRidh Awaiting requested review from FRidh

Assignees
No one assigned
Labels
6.topic: python 8.has: package (new) 10.rebuild-darwin: 1-10 10.rebuild-linux: 1-10 12. first-time contribution
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@mrkkrp @Mic92 @dotlambda @GrahamcOfBorg @Janik-Haag