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
diceware: 0.9.3 -> 0.9.4 #36114
diceware: 0.9.3 -> 0.9.4 #36114
Conversation
@GrahamcOfBorg build diceware |
Success on x86_64-linux (full log) Partial log (click to expand)
|
Success on x86_64-darwin (full log) Partial log (click to expand)
|
sha256 = "0ab4fc2pbl2hcxqw5rr6awbhlnmdna6igqjijywwr1byzb7ga4iq"; | ||
}) | ||
]; | ||
nativeBuildInputs = [ coverage pytest pytestrunner ]; |
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.
Those should be checkInputs
instead.
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 don't we think we could use these dependencies for cross compiling.
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.
However, pytestrunner
needs to be in nativeBuildInputs
or buildInputs
since it is needed at build time, not at check time.
@Mic92 Which one would be correct?
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.
Since pytestrunner
would execute code of the target platform, buildInputs
seems more appropriate. But I think nobody has yet looked into cross-compiling python packages.
I assume that for this package, pytestrunner
is just wrongly put into setup_requires
: https://github.com/ulif/diceware/blob/master/setup.py#L10
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.
That seems to be the standard way: https://github.com/pytest-dev/pytest-runner#example
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.
Ah, maybe the module is loaded in setup.py
to provide a test subcommand or something.
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.
Yeah, it provides setup.py pytest
.
@asymmetric I'd say just put pytestrunner
it in nativeBuildInputs
as it is done that way for other packages as well. We can change it later for all packages using pytestrunner
at once.
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.
And move the rest to checkInputs
?
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
59d9c86
to
909ebb2
Compare
@GrahamcOfBorg build diceware |
Success on x86_64-linux (full log) Partial log (click to expand)
|
Success on aarch64-linux (full log) Partial log (click to expand)
|
Motivation for this change
Update to latest release.
Things done
build-use-sandbox
innix.conf
on non-NixOS)nix-shell -p nox --run "nox-review wip"
./result/bin/
)