Skip to content
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

flake: add apps entry and fix runtime error #412

Merged
merged 3 commits into from
Jan 20, 2024

Conversation

TornaxO7
Copy link
Contributor

@TornaxO7 TornaxO7 commented Jan 13, 2024

Fixes the core dump when running rio with nix. @RaySlash you can take a look if you want.

@TornaxO7
Copy link
Contributor Author

Why is the windows build failing???

@TornaxO7 TornaxO7 marked this pull request as draft January 13, 2024 19:57
@TornaxO7 TornaxO7 marked this pull request as ready for review January 13, 2024 20:15
@TornaxO7 TornaxO7 marked this pull request as draft January 13, 2024 20:27
@TornaxO7 TornaxO7 marked this pull request as ready for review January 13, 2024 20:49
@TornaxO7 TornaxO7 force-pushed the fix-flake branch 2 times, most recently from 9ca7555 to 557f14b Compare January 13, 2024 20:49
flake: fix package creation
flake.nix Outdated

rustPackage = rust-toolchain:
pkgs.rustPlatform.buildRustPackage {
mkRio = {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems like an old snippet from old config? What was the motivation for this? However, I just tested this change and seems working on this end. Is this change necessary? I believe that we should not be adding additional functions on functions just for the dependencies. We could add these for PerSystem and see if that would work for us. I did this way of passing function carrying evaluations of dependencies to the function defined by rustPlatform as an experiment mostly. I am not certain if this could cause further breakage. Also, if this is for people to use flakes to generate a package for their config should not be really a concern as nixpkgs is being constantly updated by you and other maintainers as I can see now. I believe nix develop and nix build should be our focus. I would like some clarity regarding this.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, just adding

pkgs.rustPlatform.buildRustPackage {
   inherit (cargoToml.workspace.package) version;
   name = "rio";
   src = ./.;
   cargoLock.lockFile = ./Cargo.lock;
   
   cargoBuildFlags = "-p rioterm";
   
   buildInputs = runtimeDeps ++ buildDeps;
   nativeBuildInputs = buildDeps;
   runtimeDependencies = runtimeDeps;
   
   buildNoDefaultFeatures = true;
   buildFeatures = ["x11" "wayland"];
   
   meta = {
            description = "A hardware-accelerated GPU terminal emulator focusing to run in desktops and browsers";
            homepage = "https://raphamorim.io/rio";
            license = lib.licenses.mit;
            platforms = lib.platforms.unix;
            changelog = "https://github.com/raphamorim/rio/blob/master/CHANGELOG.md";
            mainProgram = "rio";
    };
};

was not enough to get rid of the issue where it core dumps with NoWaylandLib or NoXCBLib.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems like an old snippet from old config? What was the motivation for this? (...) Is this change necessary?

I'm not fully understanding this yet as well, but I assume that working with pkgs instead of an isolating function, as with mkRio here, potentially breaks some things, since callPackage does some "black magic" (that's what I got told of from the unofficial nixos-discord server).

However, I asked on the discourse what the preferred way is to declare a package and it seems to be either to move that into an extra file, as suggested in the nix pills (I only have skimmed through it at the time of writing this) or moving this into a variable, as we do here with mkRio.

I believe nix develop and nix build should be our focus.

I agree, but I'd say that nix run is more important than nix build because what's the point of having a succeeding build, if you can't use it? ;)

@raphamorim raphamorim force-pushed the main branch 2 times, most recently from ff5ce62 to 8bec894 Compare January 19, 2024 15:27
@TornaxO7 TornaxO7 requested a review from RaySlash January 19, 2024 20:46
Copy link
Contributor

@RaySlash RaySlash left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@raphamorim raphamorim merged commit eb296f6 into raphamorim:main Jan 20, 2024
4 checks passed
@TornaxO7 TornaxO7 deleted the fix-flake branch January 20, 2024 15:34
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.

3 participants