-
Notifications
You must be signed in to change notification settings - Fork 79
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
Use nix shell #16359
base: master
Are you sure you want to change the base?
Use nix shell #16359
Conversation
Jenkins BuildsClick to see older builds (142)
|
@igor-sirotin, @alexjba, @caybro, @micieslak, This PR is enforcing Nix environment for Linux builds. As a benefit you will have:
FYI @anastasiyaig |
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.
Not sure if I'm a fan of this flake-compat
stuff, but other than that this looks good.
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.
Glad to see this change! 🍻
@yakimant @siddarthkay we need to tell tests to use another artefact :) |
Need to sort out the locations of:
Possible places:
Links:
Env vars (5.15.8):
|
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.
Looking good.
@yakimant could you please give me some instructions on how to build and run the app using the nix shell? I have a new linux environment with just a bare minimum of packages installed: build-essential, git, nix, direnv. If I understand it correctly it's all I should have to get me started, right? |
One other note: would it be a good chance to update the |
@alexjba There are 2 options:
In regards of |
Thank you! I've followed the steps and this is my result:
|
@yakimant I think we'll need to find a way to share the nix packages. I've been naive enough to give it a spin in a linux VM and 15h later it's still compiling chromium. I'm really a nix noob, but I see lots of resources on sharing the pre-compiled packages. |
We have it setup already: So you need to run
before entering the nix shell. I will make sure it is set for all the cases, added to TODO. |
NVM! Had an issue with the display and all graphical apps would fail to find the display. @yakimant Works nicely! |
2c0f63e
to
c35a8a8
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.
Looks nice. Good work.
check-nix-shell: | ||
ifeq ($(detected_OS),Linux) | ||
ifndef IN_NIX_SHELL | ||
$(error Running outside of Nix shell is not supported) |
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.
Harsh :D.
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 didn't say it wasn't necessary.
}: | ||
|
||
let | ||
qtCustom = (with pkgs.qt515_8; |
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.
Another point in case, the latest released Qt version is 5.15.15, why do we have to use sth that's years old?
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.
This was originally requested by:
#9350
Switching to newer version will not be a problem. It's a next step.
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.
That would be great, and since this PR is still WIP and has many TODOs, how hard would be to make the switch now and not later? ;)
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.
If with switching to Qt 5.15.15 I will not need to implement and opt-out logic (to build and run with system libs) - I would certainly prefer this.
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.
Deal ;) I can test this locally on my machine next week, and provide patches/PR on top of this PR if needed
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.
haha, nice middle ground! 😆 Can't wait for the next iteration, on windows this time.😄
To summarize my concerns here; the usecase is pretty simple, I as a developer want to be able to:
The compatibility changes I'd like to propose are pretty simple, touching just the
For (a simple) example, the new Similarly, old |
I think that's a bad idea that will just introduce more complexity. We should support only one way of doing things and that's using Nix for the platforms that have been made to work with it, Linux first. If someone wants to not use it and have fun with dependency issues that's their problem and they can have fun with that. Mobile and status-go already are essentially built using only tools provided via Nix(aside from Xcode for iOS builds), and it has improved stability of both local and CI builds. We want to achieve this in desktop as well. I can understand finding a middle ground while Nix setup is being stabilized, but ultimately we should support only one way of building things. The only snag in this plan for Desktop is Windows platform. |
Do you think it's feasible to use NIX on Windows? Because if not, we'll need what I roughly described above anyway |
It's possible to use Nix on Windows using WSL. People do do it, it's just not something we've done before, so it will require more research and work. |
@anastasiyaig, did you (and CI e2e tests) were using X11 or Wayland? We need to make sure AppImage runs well on both. To check (this is Wayland): $ printnv XDG_SESSION_TYPE WAYLAND_DISPLAY
wayland
wayland-1 |
Testing @micieslak reported this issue (output is mine reproduced):
and related issue:
I tried to reproduced it, but faces a different issue:
That's a non-issue. Had is due to status-go leftowers. Interesting, looks like we don't support wayland and run in x11 compatibility (need to check this):
|
Tests don't work well: $ make run-storybook-tests
make[1]: Entering directory '/home/s/work/status-desktop'
Building: Storybook
gmake[2]: warning: jobserver unavailable: using -j1. Add '+' to parent make rule.
Running: Storybook Tests
UpdateCTestConfiguration from :/home/s/work/status-desktop/DartConfiguration.tcl
UpdateCTestConfiguration from :/home/s/work/status-desktop/DartConfiguration.tcl
Test project /home/s/work/status-desktop
Constructing a list of tests
Updating test list for fixtures
Added 0 tests to meet fixture requirements
Checking test dependency graph...
Checking test dependency graph end
No tests were found!!!
make[1]: Leaving directory '/home/s/work/status-desktop' $ make run-statusq-tests
make[1]: Entering directory '/home/s/work/status-desktop'
Configuring: StatusQ Unit Tests
Building: StatusQ Unit Tests
gmake[2]: warning: jobserver unavailable: using -j1. Add '+' to parent make rule.
Running: StatusQ Unit Tests
UpdateCTestConfiguration from :/home/s/work/status-desktop/DartConfiguration.tcl
UpdateCTestConfiguration from :/home/s/work/status-desktop/DartConfiguration.tcl
Test project /home/s/work/status-desktop
Constructing a list of tests
Updating test list for fixtures
Added 0 tests to meet fixture requirements
Checking test dependency graph...
Checking test dependency graph end
No tests were found!!!
make[1]: Leaving directory '/home/s/work/status-desktop' Our version of |
Failing test:
Fixed by using |
@micieslak reported another issue:
looks like non-nix and nix build were mixed. |
New issues from @micieslak:
From internet:
We have iris, but maybe it's missing this specific device:
|
After making a nix shell to build a Linux binary:
#9350
It was running as a part of CI:
https://ci.status.im/job/status-desktop/job/systems/job/linux/job/x86_64/job/package-nix/
Now it's time to switch developers to use it and eventually cleanup non-Nix shell related code.
What does the PR do
nix develop
)direnv
setup (`nix-direnv)check-nix-shell
Makefile target to block further run makeHow to test
direnv allow
nix develop
nix-shell
Useful Links
TODO
export FLAG_DAPPS_ENABLED=1
nix/README.md
ubuntu_build_setup.sh
(used in Ansible CI fleet)nix develop
issue:unsupported tarball input attribute 'lastModified'
. Maybe wrongnix
versionnix.conf
is loaded in each case (direnv, develop, shell)make run
fails to run in nix shellrun-storybook-tests
in Nix shellrun-statusq-tests
in Nix shell