Skip to content

Cleanup nix build#245

Closed
bendi-github wants to merge 1 commit intovicinaehq:mainfrom
bendi-github:nix-improvements
Closed

Cleanup nix build#245
bendi-github wants to merge 1 commit intovicinaehq:mainfrom
bendi-github:nix-improvements

Conversation

@bendi-github
Copy link
Contributor

Motivation: Cleanup nix build and add fields required for inclusion in nixpkgs

Things done:

  • rename vicinae.nix to package.nix
  • Remove unnecessary dependencies
  • Remove unnecessary build steps
  • Add meta field @Vortriz
  • add version field
  • use new finalAttrs-style mkDerivation

This builds without issues. I have tested the resulting binary and it seems to work as well as one built from main. We also get proper build logs again, the hand-rolled build phase caused them to not be output to the log for some reason.

Why was I able to remove so many deps and build steps, am I missing something here?

When opening the launcher window WARN - Could not find any platform plugin is printed to the console, although this also happens on main. This does not seem to impede functionality however.

@aurelleb
Copy link
Contributor

aurelleb commented Sep 8, 2025

Not sure about
WARN - Could not find any platform plugin

Is the clipboard history working on your build?

@Vortriz
Copy link
Contributor

Vortriz commented Sep 8, 2025

it would be nice to not use flake-utils and instead use an eachSystem function

edit: thanks for that meta attribute!

@bendi-github
Copy link
Contributor Author

Not sure about WARN - Could not find any platform plugin

Is the clipboard history working on your build?

Yes, clipboard history works fine

@TomRomeo
Copy link
Contributor

TomRomeo commented Sep 11, 2025

Great work, thank you for cleaning up the nix build!

Why was I able to remove so many deps and build steps, am I missing something here?

I think many of these might have been leftovers from me trying things out that did not work / were unnecessary. (E.g.: I think I did not import cmake as a build input in the beginning because of which the native build steps did not work).
I also did not know about npmHooks, this is great!

One thing I am weary about though is the removal of the wayland binaries in the postFixup step. For me, the binary produced by the flake did originally not work without them. Maybe something changed since then? Did you build the flake on a system using a wayland compositor?

Btw, I think if we set pname and version, we can also remove the name attribute, right?

@bendi-github
Copy link
Contributor Author

One thing I am weary about though is the removal of the wayland binaries in the postFixup step. For me, the binary produced by the flake did originally not work without them. Maybe something changed since then? Did you build the flake on a system using a wayland compositor?

I am running Hyprland (a wayland-only compositor) and haven't had any issues with removing the wayland binaries. Maybe this only becomes a problem on non-NixOS? I have also checked echo $PATH | grep wayland, that doesn't output anything.

The reference I am using (https://github.com/NixOS/nixpkgs/blob/master/pkgs/README.md) doesn't mention anything about removing name, am I missing something?

@whispersofthedawn
Copy link

whispersofthedawn commented Sep 15, 2025

The reference I am using (https://github.com/NixOS/nixpkgs/blob/master/pkgs/README.md) doesn't mention anything about removing name, am I missing something?

This is a feature of stdenv.mkDerivation. See https://nixos.org/manual/nixpkgs/unstable/#sec-using-stdenv

For convenience, you can also use pname and version attributes and mkDerivation will automatically set name to "${pname}-${version}" by default. Since RFC 0035, this is preferred for packages in Nixpkgs, as it allows us to reuse the version easily:

@bendi-github bendi-github force-pushed the nix-improvements branch 2 times, most recently from eb46ca7 to 1c6415a Compare September 15, 2025 22:16
@bendi-github
Copy link
Contributor Author

@dawnofmidnight Hey, thanks for the info! I have removed the name attribute.

@aurelleb
Copy link
Contributor

the conflicts are from me merging your other pull request

This removes unnecessary build instructions from the nix build recipe
It also brings the package definition closer to what nixpkgs packages look like
@aurelleb
Copy link
Contributor

the existing nix checks are failing, is this normal @bendi-github ?

@bendi-github
Copy link
Contributor Author

bendi-github commented Sep 16, 2025

Seems like the error occurs because I renamed vicinae.nix to package.nix. The workflow uses both the newer and the older nix CLI with the latter making some implicit assumptions about filenames. I don't understand what running nix-shell --run "echo OK" is supposed to accomplish. @TomRomeo what was your intention with this workflow step? Should we make it work with the new CLI? Shouldn't the artifact we want to build be specified clearly instead of relying on nix build automatically choosing .#packages.<currentSystem>.default as the default target? You seem to have copied the github action from nix.dev almost 1:1, with the only change being the switch from nix-build to nix build. Looking at previous runs of this workflow, it seems like the invocation of nix-shell --run "echo OK" builds protoc-gen-ts_proto, which I removed in this PR. Is this intended?

Edit 2: Originally, there was a paragraph of me being confused about github actions here, which was woefully uninformed, therefore I removed it.

@idlip
Copy link

idlip commented Sep 27, 2025

Hi,

Since you are at it, can you please bump the nixpkgs revision (flake.lock)

I use unstable branch, and qutebrowser (from unstable) uses 6.9.2 QT library, but vicinae flake.lock gets 6.9.1 version
I think this issue occurs due to vicinae having overlay here?

I'm not very sure, please let me know if this can have some workaround.

14:39:50 CRITICAL: Cannot mix incompatible Qt library (6.9.2) with this library (6.9.1)

FYI: some old ref nixos discourse

@bendi-github
Copy link
Contributor Author

Hi,

Since you are at it, can you please bump the nixpkgs revision (flake.lock)

I use unstable branch, and qutebrowser (from unstable) uses 6.9.2 QT library, but vicinae flake.lock gets 6.9.1 version I think this issue occurs due to vicinae having overlay here?

I'm not very sure, please let me know if this can have some workaround.

14:39:50 CRITICAL: Cannot mix incompatible Qt library (6.9.2) with this library (6.9.1)

FYI: some old ref nixos discourse

I use

  vicinae = {
      url = "github:vicinaehq/vicinae";
      inputs.nixpkgs.follows = "nixpkgs";
    };

@idlip
Copy link

idlip commented Sep 28, 2025

I use

  vicinae = {
      url = "github:vicinaehq/vicinae";
      inputs.nixpkgs.follows = "nixpkgs";
    };

Yea, that should solve it. But I think I'll miss out cachix from vicinae, so I'd have to compile vicinae myself...
May I know how long it takes to compile this on avg?

Edit: Solved, I built it. It took ~ 8mins

@Leka1B
Copy link
Contributor

Leka1B commented Nov 1, 2025

I feel like this is still good to include into the flake, as vicinae is actively worked on and nixpkgs get updated much slower.

Is the aforementioned error possible to fix?

@aurelleb
Copy link
Contributor

closing as #858 got merged

@aurelleb aurelleb closed this Dec 19, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants