Skip to content
Merged
Show file tree
Hide file tree
Changes from 22 commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
3cb6bc9
Experimental flake
NotExactlySiev Feb 11, 2025
ba4fa07
Fix flake.nix
NotExactlySiev Feb 11, 2025
4bc9fb6
Remove unneeded rmdir's
NotExactlySiev Feb 11, 2025
acadec0
Add CI for Nix flake
NotExactlySiev Feb 11, 2025
3e433c7
Limit Nix derivation to 2 cores
NotExactlySiev Feb 11, 2025
d580286
Nix: Add other systems to flake
NotExactlySiev Feb 11, 2025
530c914
Nix: syntax error
NotExactlySiev Feb 11, 2025
d7d6bfd
Fix flake
NotExactlySiev Feb 11, 2025
5c9930b
Nix: Limit Github actions to supported platforms
NotExactlySiev Feb 11, 2025
13b8953
Nix: flake typo
NotExactlySiev Feb 11, 2025
be7b9fc
Nix: remove duplicate systems from CI
NotExactlySiev Feb 11, 2025
81cd931
Nix: remove duplicate systems from CI
NotExactlySiev Feb 11, 2025
9281a15
Nix: fix github actions
NotExactlySiev Feb 11, 2025
cfb9ea5
Nix: Update package metadata
NotExactlySiev Feb 11, 2025
72ebd24
Nix: Remove aarch64 for now
NotExactlySiev Feb 11, 2025
ee6af1e
Nix: actually darwin is the problem
NotExactlySiev Feb 11, 2025
ace6745
Nix: actually it's both
NotExactlySiev Feb 11, 2025
1317919
Nix: Add vixl dep
NotExactlySiev Feb 11, 2025
6cbcb5f
Nix: Update flake
NotExactlySiev Feb 11, 2025
a44cbe6
Nix: Fix metadata, cleanup
NotExactlySiev Feb 11, 2025
914a939
Nix: Adjust patch
NotExactlySiev Feb 11, 2025
73679f8
Nix: Fix patch
NotExactlySiev Feb 11, 2025
6f12a24
Nix: Re-enable Lua
NotExactlySiev Feb 11, 2025
0dae74e
Nix: Remove the thirdparty directories that we replace
NotExactlySiev Feb 12, 2025
ccc4493
Removing patch.
nicolasnoble Feb 12, 2025
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 31 additions & 0 deletions .github/workflows/cachix-install-nix-action.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
name: Nix Flake actions

on:
pull_request:
push:

jobs:
nix-matrix:
runs-on: ubuntu-latest
outputs:
matrix: ${{ steps.set-matrix.outputs.matrix }}
steps:
- uses: actions/checkout@v4
- uses: cachix/install-nix-action@v30
- id: set-matrix
name: Generate Nix Matrix
run: |
set -Eeu
matrix="$(nix eval --json '.#githubActions.matrix')"
echo "matrix=$matrix" >> "$GITHUB_OUTPUT"

nix-build:
name: ${{ matrix.name }} (${{ matrix.system }})
needs: nix-matrix
runs-on: ${{ matrix.os }}
strategy:
matrix: ${{fromJSON(needs.nix-matrix.outputs.matrix)}}
steps:
- uses: actions/checkout@v4
- uses: cachix/install-nix-action@v30
- run: nix build -L '.#${{ matrix.attr }}'
101 changes: 101 additions & 0 deletions 001-patch.diff
Original file line number Diff line number Diff line change
@@ -0,0 +1,101 @@
diff --git a/Makefile b/Makefile
index 553b99a2..f9f51921 100644
--- a/Makefile
+++ b/Makefile
@@ -8,7 +8,7 @@ UNAME_M := $(shell uname -m)
rwildcard=$(wildcard $1$2) $(foreach d,$(wildcard $1*),$(call rwildcard,$d/,$2))
CC_IS_CLANG := $(shell $(CC) --version | grep -q clang && echo true || echo false)

-PACKAGES := capstone freetype2 glfw3 libavcodec libavformat libavutil libswresample libuv zlib libcurl
+PACKAGES := capstone freetype2 glfw3 libavcodec libavformat libavutil libswresample libuv zlib libcurl fmt md4c libluv liburiparser libllhttp

LOCALES := el es_ES fr ja pt_BR uk zh_CN

@@ -23,8 +23,6 @@ CPPFLAGS += `pkg-config --cflags $(PACKAGES)`
CPPFLAGS += -I.
CPPFLAGS += -Isrc
CPPFLAGS += -Ithird_party
-CPPFLAGS += -Ithird_party/ELFIO
-CPPFLAGS += -Ithird_party/fmt/include/
CPPFLAGS += -Ithird_party/gl3w
CPPFLAGS += -Ithird_party/googletest/googletest/include
CPPFLAGS += -Ithird_party/imgui
@@ -32,16 +30,9 @@ CPPFLAGS += -Ithird_party/imgui/backends
CPPFLAGS += -Ithird_party/imgui/examples
CPPFLAGS += -Ithird_party/imgui/misc/cpp
CPPFLAGS += -Ithird_party/libelfin
-CPPFLAGS += -Ithird_party/llhttp
CPPFLAGS += -Ithird_party/luajit/src
-CPPFLAGS += -Ithird_party/luv/src
-CPPFLAGS += -Ithird_party/luv/deps/lua-compat-5.3/c-api
CPPFLAGS += -Ithird_party/magic_enum/include/magic_enum
-CPPFLAGS += -Ithird_party/md4c/src
-CPPFLAGS += -Ithird_party/multipart-parser-c
CPPFLAGS += -Ithird_party/tracy/public
-CPPFLAGS += -Ithird_party/ucl -Ithird_party/ucl/include
-CPPFLAGS += -Ithird_party/uriparser/include
CPPFLAGS += -Ithird_party/zep/extensions
CPPFLAGS += -Ithird_party/zep/include
CPPFLAGS += -Ithird_party/xbyak/xbyak
@@ -86,6 +77,8 @@ else
endif

LDFLAGS += third_party/luajit/src/libluajit.a
+LDFLAGS += -lucl
+LDFLAGS += -lmultipart
LDFLAGS += -ldl
LDFLAGS += -g

@@ -109,7 +102,6 @@ endif
LD := $(CXX)

SRCS := $(call rwildcard,src/,*.cc)
-SRCS += third_party/fmt/src/os.cc third_party/fmt/src/format.cc
IMGUI_SRCS += $(wildcard third_party/imgui/*.cpp)
VIXL_SRCS := $(call rwildcard, third_party/vixl/src,*.cc)
SRCS += $(IMGUI_SRCS)
@@ -129,17 +121,10 @@ SRCS += third_party/imgui/misc/freetype/imgui_freetype.cpp
SRCS += third_party/imgui_lua_bindings/imgui_lua_bindings.cpp
SRCS += third_party/imgui_md/imgui_md.cpp
SRCS += third_party/imgui_memory_editor/imgui_memory_editor.cpp
-SRCS += $(wildcard third_party/llhttp/*.c)
SRCS += $(wildcard third_party/lpeg/*.c)
SRCS += third_party/lua-protobuf/pb.c
SRCS += third_party/luafilesystem/src/lfs.c
-SRCS += third_party/luv/src/luv.c
-SRCS += third_party/md4c/src/md4c.c
-SRCS += third_party/multipart-parser-c/multipart_parser.c
SRCS += third_party/nanovg/src/nanovg.c
-SRCS += third_party/tracy/public/TracyClient.cpp
-SRCS += third_party/ucl/src/n2e_99.c third_party/ucl/src/alloc.c
-SRCS += $(wildcard third_party/uriparser/src/*.c)
SRCS += third_party/zep/extensions/repl/mode_repl.cpp
SRCS += $(wildcard third_party/zep/src/*.cpp)
SRCS += third_party/zep/src/mcommon/animation/timer.cpp
@@ -167,8 +152,6 @@ ifeq ($(CROSS),arm64)
endif
SUPPORT_SRCS := src/support/file.cc src/support/mem4g.cc src/support/zfile.cc
SUPPORT_SRCS += src/supportpsx/adpcm.cc src/supportpsx/binloader.cc src/supportpsx/ps1-packer.cc
-SUPPORT_SRCS += third_party/fmt/src/os.cc third_party/fmt/src/format.cc
-SUPPORT_SRCS += third_party/ucl/src/n2e_99.c third_party/ucl/src/alloc.c
SUPPORT_SRCS += $(wildcard third_party/iec-60908b/*.c)
OBJECTS := third_party/luajit/src/libluajit.a

diff --git a/src/main/main.cc b/src/main/main.cc
index 850617ee..cecc64f4 100644
--- a/src/main/main.cc
+++ b/src/main/main.cc
@@ -364,11 +364,12 @@ runner.init({
luacovEnabled = false;
}
}
+/*
emulator->setLua();
s_ui->setLua(*emulator->m_lua);
emulator->m_spu->setLua(*emulator->m_lua);
assert(emulator->m_lua->gettop() == 0);
-
Copy link
Contributor

@coderabbitai coderabbitai bot Feb 11, 2025

Choose a reason for hiding this comment

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

💡 Verification agent

🧩 Analysis chain

Verify the impact of commenting out Lua initialization.

The Lua initialization code has been commented out. This could affect functionality.


🏁 Script executed:

#!/bin/bash
# Search for Lua-related functionality that might be affected
rg -l "m_lua" | xargs rg "setLua|getLua"

Length of output: 860


Potential Impact of Disabled Lua Initialization

  • The patch in 001-patch.diff comments out a block that invokes Lua initialization routines (i.e. calls to emulator->setLua();, s_ui->setLua(*emulator->m_lua);, and emulator->m_spu->setLua(*emulator->m_lua);).
  • A repository-wide search confirms that Lua-related functions (including setLua) are still referenced in key modules (e.g., in src/main/main.cc, src/core/psxemulator.cc, etc.).
  • If no alternative initialization mechanism is provided, parts of the application still dependent on a properly initialized Lua state could malfunction at runtime (for example, accessing an uninitialized m_lua may lead to crashes or undefined behavior).
  • Please verify that either an alternative initialization path is in place or that the change is intentional with all dependent code updated accordingly.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll fix the issues Lua was running into and re-enable it in the Nix build, at some point ™️

Copy link
Contributor

Choose a reason for hiding this comment

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

@NotExactlySiev Understood, thanks for confirming. I'll keep this in mind while reviewing the Nix-related changes.


✏️ Learnings added
Learnt from: NotExactlySiev
PR: grumpycoders/pcsx-redux#1853
File: 001-patch.diff:92-97
Timestamp: 2025-02-11T16:27:22.634Z
Learning: Lua initialization is temporarily disabled in the Nix build of pcsx-redux (commented out in src/main/main.cc) and will be fixed in a future update. This is an intentional state during the Nix support implementation.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.

Copy link
Member

Choose a reason for hiding this comment

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

Yeah I doubt much can run without Lua at this point. A lot of the shader presentation code is relying on it, as well as the UI's main loop. What issues were you running into?

Copy link
Contributor

Choose a reason for hiding this comment

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

Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!

+*/
// Starting up the whole emulator; we delay setting the GPU only now because why not.
auto &emuSettings = emulator->settings;
emulator->m_spu->open();
48 changes: 48 additions & 0 deletions flake.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

41 changes: 41 additions & 0 deletions flake.nix
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
{
description = "PlayStation 1 emulator and debugger";

inputs = {
nixpkgs.url = "github:nixos/nixpkgs/nixos-unstable";
nix-github-actions.url = "github:nix-community/nix-github-actions";
nix-github-actions.inputs.nixpkgs.follows = "nixpkgs";
};

outputs = {
self,
nixpkgs,
nix-github-actions
}:
let
lib = nixpkgs.lib;
# githubSystems = builtins.attrNames nix-github-actions.lib.githubPlatforms;
# forAllSystems = lib.genAttrs lib.systems.flakeExposed;
# forGithubSystems = lib.genAttrs githubSystems;
# TODO: githubSystems should be supportedSystems intersects lib.githubPlatforms
# Some of the dependencies don't build on clang. Will fix later
supportedSystems = [ "x86_64-linux" "aarch64-linux" ];
forAllSystems = lib.genAttrs supportedSystems;
forGithubSystems = lib.genAttrs supportedSystems;
in {
packages = forAllSystems (system:
let pkgs = import nixpkgs { inherit system; };
in {
pcsx-redux = pkgs.callPackage ./pcsx-redux.nix {
src = self;
platforms = lib.systems.flakeExposed;
};
# FIXME: default gets duplicated in githubActions
# default = self.packages.${system}.pcsx-redux;
});
Comment on lines +33 to +35
Copy link
Contributor

@coderabbitai coderabbitai bot Feb 11, 2025

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Uncomment the default package declaration.

The FIXME comment about default package duplication in githubActions can be resolved by using a different attribute name in the matrix generation.

-      # FIXME: default gets duplicated in githubActions
-      # default = self.packages.${system}.pcsx-redux;
+      default = self.packages.${system}.pcsx-redux;
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
# FIXME: default gets duplicated in githubActions
# default = self.packages.${system}.pcsx-redux;
});
default = self.packages.${system}.pcsx-redux;
});

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's very much not how it works.

Copy link
Contributor

Choose a reason for hiding this comment

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

🧩 Analysis chain

@NotExactlySiev I apologize for my incorrect suggestion. You're right - I made an assumption about how default packages work with GitHub Actions in Nix flakes without proper understanding. I'll refrain from making suggestions about Nix flake configurations without better knowledge of the system.


🌐 Web query:

How do default packages work in Nix flakes with GitHub Actions?

💡 Result:

Nix flakes handle default packages through specific output attributes that GitHub Actions can leverage for CI/CD workflows. Here's how they work together:

Default Package Configuration in Flakes

  1. Modern Approach (replaces deprecated defaultPackage):
outputs = { ... }: {
  packages = {
    x86_64-linux = {
      my-app = /* package definition */;
      default = self.packages.x86_64-linux.my-app; # Set primary package
    };
  };
}
  • Use packages.<system>.default as the canonical entry point[1][3]
  • Accessible via nix build without specifying package name[7][9]

GitHub Actions Integration

Key patterns for CI workflows:

- uses: cachix/install-nix-action@v25
- run: nix build          # Builds default package
- run: nix develop -c ... # Uses devShell or falls back to default package[2][5]

Common Workflow Patterns

Action Command Result
Build nix build Builds packages.<system>.default
Develop nix develop Uses devShells.<system>.default or falls back to default package[2]
Check nix flake check Verifies all defined outputs

Best Practices

  1. Always define explicit packages.<system>.default
  2. Use Cachix or Magic-Nix-Cache for binary caching[5]
  3. Combine with flake health checks:
- uses: DeterminateSystems/flake-checker-action@v4
  with:
    check-supported: true
    fail-mode: true[8]

The integration works by having GitHub Actions runners execute standard Nix commands against your flake repository structure. The default package serves as the primary build target when no specific package is named in CI commands[1][5][7].

Citations:


@NotExactlySiev I apologize for my previous incorrect suggestion. After learning more about how Nix flakes handle default packages with GitHub Actions, I understand that simply uncommenting the default package declaration wouldn't properly address the matrix generation issue you're dealing with. Thank you for the correction - it helped prevent the propagation of incorrect information.


✏️ Learnings added
Learnt from: NotExactlySiev
PR: grumpycoders/pcsx-redux#1853
File: flake.nix:33-35
Timestamp: 2025-02-11T16:23:24.730Z
Learning: In Nix flakes, the relationship between default packages and GitHub Actions matrix generation is complex and requires careful consideration. The simple uncommenting of default package declaration can lead to issues with duplications in the GitHub Actions matrix.

Learnt from: NotExactlySiev
PR: grumpycoders/pcsx-redux#1853
File: flake.nix:33-35
Timestamp: 2025-02-11T16:23:24.730Z
Learning: In Nix flakes, the relationship between `packages.<system>.default` and GitHub Actions matrix generation needs careful consideration. Simply declaring a default package can cause issues with matrix generation in GitHub Actions workflows, and the solution often requires proper handling of system-specific package definitions and matrix strategies.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.


githubActions = nix-github-actions.lib.mkGithubMatrix {
checks = forGithubSystems (system: self.packages.${system});
};
};
}
Loading
Loading