-
-
Notifications
You must be signed in to change notification settings - Fork 14.8k
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
multipass: 1.14.0 -> 1.15.0 #363626
multipass: 1.14.0 -> 1.15.0 #363626
Conversation
81641cc
to
86a41a5
Compare
This comment was marked as outdated.
This comment was marked as outdated.
575022a
to
42f31ed
Compare
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.
Also is there a reason to be updating to a release candidate instead of waiting for a stable release?
Thanks for the feedback, I'll make those changes today - and nice tip on the desktop file, i'd not spotted that before. Indeed my intention was to wait until the 1.15.0 release, I meant to mark this as draft while i got feedback on the approach 🤦♂️ |
99e7f94
to
6a45cda
Compare
6a45cda
to
d39aa00
Compare
This comment was marked as outdated.
This comment was marked as outdated.
d39aa00
to
b4c6c08
Compare
b4c6c08
to
c1cbc37
Compare
|
@SuperSandro2000 @getchoo feedback addressed, and also now bumped to the actual release, not the RC. Hopefully good to go now! Thanks for the help. |
This commit breaks down multipass into two intermediate packages, `multipassd` and `multipass-gui`. These are then packaged up with `symlinkJoin` to work around some intricacies in the `buildFlutterApplication` process which would otherwise be difficult to patch. The multipass repo is setup upstream such that the flutter app would be built as part of the regular cmake process, but that can't work here due to the fetching of the flutter dependencies. The upstream multipass project builds `libdart_ffi.so`, which is needed in the LD_LIBRARY_PATH of the GUI. Building the two derivations seperately enables that to be done relatively simply.
c1cbc37
to
be677db
Compare
|
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.
Diff LGTM, but I'm not all that familiar with flutter
|
||
# Generate the Dart gRPC code for the Multipass GUI. | ||
protoc \ | ||
--plugin=protoc-gen-dart=${lib.getExe protoc-gen-dart} \ |
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 think as it is, this is broken for cross-compilation? And needs to use either:
$(which protoc-gen-dart)
in the shell to look up thenativeBuildInputs
.${lib.getExe buildPackages.protoc-gen-dart}
in Nix to correctly use a build package, not a host package.
This commit is a fairly big one:
pkgs/by-name
I've pushed this a little early while we're still in the late
rc
phase of the Multipass release to get feedback on the restructuring. I noted the following in the commit that introduces the GUI element:Hopefully that makes sense! Tested locally on my machine and all works as expected.
Perhaps @RossComputerGuy could give some feedback on this approach, seems they have lots of involvement with the Flutter tooling in nixpkgs.
Things done
nix.conf
? (See Nix manual)sandbox = relaxed
sandbox = true
nix-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.