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

mididings package #50898

Closed
wants to merge 4 commits into from
Closed

mididings package #50898

wants to merge 4 commits into from

Conversation

@whohoho
Copy link

whohoho commented Nov 21, 2018

Motivation for this change

New package.

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option 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/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Fits CONTRIBUTING.md.

@whohoho whohoho requested a review from FRidh as a code owner Nov 21, 2018
+ for suffix in add_suffixes + ['', '-mt', '36']:
for libdir in lib_dirs():
for ext in ['so'] + ['dylib'] * (sys.platform == 'darwin'):
libname = 'lib%s%s.%s' % (name, suffix, ext)

This comment has been minimized.

Copy link
@Mic92

Mic92 Nov 21, 2018

Contributor

It is interesting that there is no symlink without the version suffix.

Copy link
Contributor

Mic92 left a comment

The project looks a bit unmaintained. Is this still actively developed? Please fix the indentation in the file. We use two spaces for indentation.

glib
libjack2
alsaLib
pkgconfig

This comment has been minimized.

Copy link
@Mic92

Mic92 Nov 21, 2018

Contributor

Not needed here.

pkgs/development/python-modules/mididings/default.nix Outdated Show resolved Hide resolved
pkgs/development/python-modules/mididings/default.nix Outdated Show resolved Hide resolved

patches = [ ./0001-add-boost-suffix.patch ];

LIBRARY_PATH="${(pkgs.boost.override {enablePython = true; inherit python; })}/lib";

This comment has been minimized.

Copy link
@Mic92

Mic92 Nov 21, 2018

Contributor

What is the purpose of this?

This comment has been minimized.

Copy link
@whohoho

whohoho Nov 22, 2018

Author

boost seems to be build without lboost_python by default

This comment has been minimized.

Copy link
@Mic92

Mic92 Nov 22, 2018

Contributor

I mean is LIBRARY_PATH an environment variable used by the project internally at build time?

This comment has been minimized.

Copy link
@whohoho

whohoho Nov 22, 2018

Author

Yes, without it it gives a linker error:
/nix/store/vcc4svb8gy29g4pam2zja6llkbcwsyiq-binutils-2.30/bin/ld: cannot find -lboost_python

meta = {
description = "midi router";
homepage = https://github.com/dsacre/mididings;
license = lib.licenses.gpl3;

This comment has been minimized.

Copy link
@Mic92

Mic92 Nov 21, 2018

Contributor

Are you going to maintain this package?

This comment has been minimized.

Copy link
@whohoho

whohoho Nov 22, 2018

Author

i put myself as the maintainer

, python36Packages
}:

buildPythonPackage rec {

This comment has been minimized.

Copy link
@Mic92

Mic92 Nov 21, 2018

Contributor

As this is an application please use buildPythonApplication and move it out of python-modules into a different directory.

This comment has been minimized.

Copy link
@whohoho

whohoho Nov 22, 2018

Author

If it is done as a python application, can you still import it as a library?, cause that is how it is mostly used.

This comment has been minimized.

Copy link
@FRidh

FRidh Nov 22, 2018

Member

No, in that case it needs to stay in python-modules. Note, however, that python36Packages can't be an argument.

, python36Packages
}:

buildPythonPackage rec {

This comment has been minimized.

Copy link
@FRidh

FRidh Nov 22, 2018

Member

No, in that case it needs to stay in python-modules. Note, however, that python36Packages can't be an argument.

@@ -22952,6 +22952,9 @@ with pkgs;

tsung = callPackage ../applications/networking/tsung {};

mididings = python3Packages.callPackage ../development/python-modules/mididings { };

This comment has been minimized.

Copy link
@FRidh

FRidh Nov 22, 2018

Member

needs to be moved to python-packages.nix. Note for use as an application you could put here

mididings = with python3Packages; toPythonApplication mididings;
@mmahut
Copy link
Member

mmahut commented Aug 11, 2019

Are there any updates on this pull request, please?

@gnidorah
Copy link
Member

gnidorah commented Feb 16, 2020

Closed by #63207

@gnidorah gnidorah mentioned this pull request Feb 16, 2020
3 of 10 tasks complete
@gnidorah gnidorah closed this May 18, 2020
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

7 participants
You can’t perform that action at this time.