localsend: 1.11.1 -> 1.12.0, build from source #259901
Conversation
linsui
requested review from
adamcstephens and
sikmir
and removed request for
adamcstephens
No, currently it's not supported. |
linsui
force-pushed
the
localsend
branch
2 times, most recently
from
7111cd8
to
f205a69
Compare
|
@sikmir Do you still maintain localsend? |
Yes, LGTM. |
|
@sikmir Good to merge? :) |
|
1.12.0 is out meanwhile |
|
I don't know how but the buildFlutterApplication is broken currently. See #263723. |
ofborg
bot
added
10.rebuild-darwin: 1-10
10.rebuild-darwin: 1
and removed
10.rebuild-darwin: 0
10.rebuild-linux: 1
labels
|
This PR should not include #263723. |
| # $PUB_CACHE/hosted is a symlink to a store path. | ||
| mv $PUB_CACHE/hosted $PUB_CACHE/hosted_copy | ||
| cp -HR $PUB_CACHE/hosted_copy $PUB_CACHE/hosted | ||
| substituteInPlace $PUB_CACHE/hosted/pub.dev/system_tray-*/linux/tray.cc \ |
There was a problem hiding this comment.
Normally patching the code like this would be a good solution, but modifying the package cache is a bit hacky. Would something like patchelf --add-rpath or --add-needed not work? The other alternative is adding to LD_LIBRARY_PATH.
This will be obsoleted by #263345 anyway, so I'm not too hung up on which solution we use for now.
There was a problem hiding this comment.
I was told that such patch is preferred. :) Could you add something like cargoDepsCopy to buildFlutterApplication?
Localsend has a cli. It seems it's not ready yet but maybe we want the keep the localsend binary name for it. |
|
This MR is still blocked by #263939. |
linsui
changed the title
|
@ofborg build localsend |
|
This pull request has been mentioned on NixOS Discourse. There might be relevant details there: https://discourse.nixos.org/t/prs-already-reviewed/2617/1244 |
There was a problem hiding this comment.
Result of nixpkgs-review pr 259901 run on x86_64-linux 1
1 package built:
- localsend
Result of nixpkgs-review pr 259901 run on aarch64-linux 1
1 package built:
- localsend
Result of nixpkgs-review pr 259901 run on aarch64-darwin 1
1 package built:
- localsend
Result of nixpkgs-review pr 259901 run on x86_64-darwin 1
1 package built:
- localsend
| @@ -1,5 +1,5 @@ | |||
| #! /usr/bin/env nix-shell | |||
| #! nix-shell -I nixpkgs=./. -i bash -p curl gnused | |||
| #! nix-shell -I nixpkgs=./. -i bash -p curl gnused jq | |||
There was a problem hiding this comment.
Why did you add a jq dependency here?
There was a problem hiding this comment.
It's used at line 8.
| @@ -16,10 +16,6 @@ fi | |||
|
|
|||
| sed -i "s/version = \".*\"/version = \"${latestVersion}\"/" "$ROOT/default.nix" | |||
|
|
|||
| LINUX_x64_URL="https://github.com/localsend/localsend/releases/download/v${latestVersion}/LocalSend-${latestVersion}-linux-x86-64.AppImage" | |||
There was a problem hiding this comment.
Would you be willing to fix updating Linux instead of just removing it?
There was a problem hiding this comment.
If so, we should also add passthru.updateScript = ./update.sh;
There was a problem hiding this comment.
I don't know how to do that. I would like to have a try but not in this PR.
|
|
Well, that goes against |
|
Oh, sorry, the line was added by mistake when I rebase the MR. I thought we should use localsend_app and leave localsend to the cli. |
|
I opened #269253 instead. |
Description of changes
Because why not? And the closure size is smaller (1.7G -> 372.8M). The update script can't update the linux package though.
Things done
sandbox = trueset innix.conf? (See Nix manual)nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage./result/bin/)