Conversation
There was a problem hiding this comment.
We can't use patchShebangs here?
There was a problem hiding this comment.
no test 04 is writing its own script for the tests so we would still need this additionnal sed, so no real interest in using patchShebangs
There was a problem hiding this comment.
| checkInputs = [ pkgs.glibcLocales ]; | |
| checkInputs = [ glibcLocales ]; |
There was a problem hiding this comment.
why do you want to remove the checkPhase ?
|
Result of 5 packages failed to build:
1 package built:
|
|
Result of 4 packages failed to build:
3 packages built:
|
bad862f to
6c56325
Compare
|
Result of 7 packages built:
|
|
Result of 3 packages failed to build:
3 packages built:
|
|
Result of 3 packages failed to build:
3 packages built:
|
|
Ok I can't run the build on a darwin machine, I'd be grateful if someone can try to fix the build on this platform |
|
This pull request has been mentioned on NixOS Discourse. There might be relevant details there: |
jonringer
left a comment
There was a problem hiding this comment.
do you mind squashing the darwin fixup commit into the pysvn commit?
|
cc @SuperSandro2000 for darwin review |
|
we can also do the darwin fix in another PR. Doesn't make sense to force people to support hardware they don't have (and Apple doesn't make it easy for me to emulate) |
Mathnerd314
left a comment
There was a problem hiding this comment.
Looks good to me. I'm not using rabbitvcs anymore so if you want to take over maintainership of the package feel free.
|
Result of 7 packages built:
|
|
Result of 3 packages failed to build:
3 packages built:
|
There was a problem hiding this comment.
It'd be nice to explain this if possible.
There was a problem hiding this comment.
Not needed anymore in fact, I remove this line
There was a problem hiding this comment.
Then please squash that last commit into the commit that touches this file.
There was a problem hiding this comment.
@freezeboy so do you intend to remove this prePatch ? Even if not, perhaps since it's tests related it'd be best to use postPatch, per https://github.com/jtojnar/nixpkgs-hammering/blob/master/explanations/patch-phase.md .
There was a problem hiding this comment.
Oh I thought your note was about the cflags, indeed I can move this sed call to postPatch
2a45c79 to
c7f5f93
Compare
|
cc @doronbehar |
c7f5f93 to
3acbf22
Compare
3acbf22 to
301b1da
Compare
jonringer
left a comment
There was a problem hiding this comment.
LGTM
https://github.com/NixOS/nixpkgs/pull/102378
7 packages built:
python37Packages.pycxx python37Packages.pysvn python38Packages.pycxx python38Packages.pysvn python39Packages.pycxx python39Packages.pysvn rabbitvcs
I think future improvements can be done later. But the PR is still a benefit
|
@freezeboy ping me if no one merges in a day or so |
Also remove dependency to python2
301b1da to
e54e408
Compare
|
nixpkgs-review fails for me with: Probably happens only on my machine. |
|
Strange, it works like a charm on my side :/ |
|
@doronbehar that's probably a transient issue with pypi's hosting. I was able to build it earlier |
Motivation for this change
Update the tool and support python3
part of #101964
Needed to update pysvn and add pycxx in the process
Things done
sandboxinnix.confon non-NixOS linux)nix-shell -p nixpkgs-review --run "nixpkgs-review wip"./result/bin/)nix path-info -Sbefore and after)