-
-
Notifications
You must be signed in to change notification settings - Fork 15.4k
Add behave #28628
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
Add behave #28628
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've added some comments to one expression. They apply to the other expressions as well. Please fix that.
nativeBuildInputs = [ mock nose pyhamcrest strace ]; | ||
propagatedBuildInputs = [ glibcLocales parse parse-type six ]; | ||
|
||
patchPhase = '' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
postPatch
propagatedBuildInputs = [ glibcLocales parse parse-type six ]; | ||
|
||
patchPhase = '' | ||
# Fix `bin/behave` shebang … |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
indentation
doCheck = true; | ||
|
||
checkPhase = '' | ||
export LANG="en_US.UTF-8" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
indentation
version = "1.2.5"; | ||
name = "${pname}-${version}"; | ||
|
||
src = fetchurl { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fetchPypi
}; | ||
|
||
nativeBuildInputs = [ mock nose pyhamcrest strace ]; | ||
propagatedBuildInputs = [ glibcLocales parse parse-type six ]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
glibcLocales
should not be propagated
sha256 = "81b731ac5187e31e4aad2594944fa914943683a9818320846d037c5ebd6d5d0b"; | ||
}; | ||
|
||
nativeBuildInputs = [ mock nose pyhamcrest strace ]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
test runner requirements can be added to checkInputs
@@ -0,0 +1,27 @@ | |||
{ stdenv, fetchurl |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we already have pytestrunner
Thanks! I didn't know if |
I've cleaned up an issue I found with the licenses but I'm not sure how to handle the license for parse (it only indicates the license through the pypi classifiers as BSD). I've currently set the parse license to all BSD licenses but this doesn't feel correct. Can you let me know what the best case would be for this? |
pkgs/top-level/python-packages.nix
Outdated
|
||
parse-type = callPackage ../development/python-modules/parse-type { }; | ||
|
||
pytest-runner = callPackage ../development/python-modules/pytest-runner { }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You forget to remove this line.
|
||
postPatch = '' | ||
# Fix `bin/behave` shebang … | ||
sed -e "1s@/usr/bin/env python@${python.interpreter}@" -i bin/behave |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
use patchShebangs
here
]; | ||
|
||
checkPhase = '' | ||
${python.interpreter} test_parse.py |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@FRidh is there actually a reason ${python.interpreter}
is used all over the place in python-modules
instead of just python
which should be in PATH
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, because we have different interpreters, like python3
(which nowadays have a symlink from python
) and pypy
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are there cases where both interpreters end up in buildPythonPackage
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I hope not because then we would have issues also with PYTHONPATH
.
In the end you need to be able to write python test_parse.py
or pypy test_parse.py
so I don't see any other way than using a variable like that here.
The license can be found in the footer of the Python files, e.g. |
}; | ||
|
||
patches = [ | ||
(fetchpatch { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what is this patch for? Can you include a comment.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, nevermind, the patch name clarifies it.
I think I got the latest batch of finds. Let me know if there is anything else that looks amiss and I'll keep my eye on the travis run. Thanks for all the suggestions so far. |
Does anyone have any suggestions on how to get the last test on travis to succeed? Also, are there any other suggestions blocking this merge? |
Thank you. Added it to #29009. |
Motivation for this change
Adding package for behave. Please, let me know if I missed anything. I think I understood the examples and documentation correctly but would love to know any tips as well.
Things done
(nix.useSandbox on NixOS,
or option
build-use-sandbox
innix.conf
on non-NixOS)
nix-shell -p nox --run "nox-review wip"
./result/bin/
)