Conversation
AndersonTorres
left a comment
There was a problem hiding this comment.
Now it looks good to go.
Please squash everything and fix possible problems.
a8e2088 to
3fec0ac
Compare
AndersonTorres
left a comment
There was a problem hiding this comment.
Hum, let's reword the commit message:
brave: add aarch64-linux support
- Refactor update.sh to deal with both x86_64-linux and aarch64-linux packages.
Besides, LGTM.
|
This pull request has been mentioned on NixOS Discourse. There might be relevant details there: https://discourse.nixos.org/t/prs-already-reviewed/2617/1568 |
Done. |
|
Delete the gibberish in the commit message. |
Sorry didnt realize. We should be all set now. |
AndersonTorres
left a comment
There was a problem hiding this comment.
LGTM. Waiting an Armlinux user.
|
Result of |
|
Result of |
There was a problem hiding this comment.
This moves brave to brave.brave (so brave.meta doesn't exist, and the clean-up label is still there). Could something like let brave = something; in brave be done here?
There was a problem hiding this comment.
I'm going to need some help on this one.
There was a problem hiding this comment.
Try to find something like brave = callPackage <path> { }; in all-packages.nix and replace it with inherit (callPackages <path> { }) brave;.
There was a problem hiding this comment.
Try
| brave = if hostPlatform.isAarch64 | |
| if hostPlatform.isAarch64 |
There was a problem hiding this comment.
If I remove brave i get a the following linting error Expecting a binding like 'path = value;' or 'inherit attr;'. When building the package I get error: syntax error, unexpected IF.
There was a problem hiding this comment.
| { hostPlatform, callPackage }: { | |
| brave = if hostPlatform.isAarch64 | |
| { hostPlatform, callPackage }: | |
| if hostPlatform.isAarch64 |
I believe it should help, but I can't test.
There was a problem hiding this comment.
That doesn't work I get the same error as mentioned prior. Is there anything else we can do to get rid of clean up label. I have successfully built the package on both systems.
Refactor update.sh to deal with both x86_64-linux and aarch64-linux packages.
|
Result of 1 package built:
|
Description of changes
Accidentally closed original pr after rebasing to fix merge conflicts. This resolves issue #278278.
Things 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.