Refactor postPatch in handheld-daemon and hidapi #309530
Conversation
This reverts commit d9b487a.
ofborg
bot
added
11.by: package-maintainer
ofborg
bot
added
10.rebuild-linux: 11-100
and removed
10.rebuild-linux: 1
10.rebuild-linux: 1-10
labels
|
The file that should change in sync with hhd is |
|
oh |
toast003
force-pushed
the
hhd-hidapi-patch-refactor
branch
from
2a71c77 to
4813929
Compare
ofborg
bot
added
10.rebuild-darwin: 1-10
10.rebuild-linux: 1-10
and removed
10.rebuild-darwin: 0
| # handheld-daemon contains a fork of the python module `hid`, so this hook | ||
| # is borrowed from the `hid` derivation. | ||
| substituteInPlace src/hhd/controller/lib/hid.py \ | ||
| --replace-fail libhidapi ${hidapi}/lib/libhidapi | ||
|
|
||
| hidapi=${hidapi}/lib/ |
There was a problem hiding this comment.
Why isn't the substituteInPlace using this variable?
There was a problem hiding this comment.
Not sure since I did this a while ago, but iirc I just copied the substituteInPlace right on top and then changed it.
The check that's under will still work so I think it's fine if we leave it as is
There was a problem hiding this comment.
Except that it's duplicating the path now. If that path ever changed, you could change one and not the other - it would break and you might not realize it.
In some senses, this is just cargo-culted over from hid; however, as long as the variable is there, it should be used in both places. Moreover, since this does mirror the hid build rule, it should keep symmetry with the hid derivation (which does the test before calling subInPlace).
There was a problem hiding this comment.
Got it, just changed subInPlace so that it uses the variable
|
@Faupi pls check if this pr fixes #303846 (comment) |
👍 |
wegank
added
the
12.approvals: 1
wegank
added
the
12.approved-by: package-maintainer
wegank
added
the
2.status: merge conflict
|
This week is very fussy! Too much merge conflicts! |
|
@toast003 can you rebase this PR? |
ofborg
bot
removed
the
2.status: merge conflict
wegank
removed
the
12.approvals: 1
wegank
removed
the
12.approved-by: package-maintainer
wegank
added
the
2.status: merge conflict
Refactor discussed in #303846 (comment) and #303846 (comment)
Description of changes
Replace sed with substituteInPlace in
handheld-daemonandhidapi's postPatchThings done
nix.conf? (See Nix manual)sandbox = relaxedsandbox = truenix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage./result/bin/)Add a 👍 reaction to pull requests you find important.