Skip to content

katawa-shoujo-re-engineered: compile rpy files in the build phase#490290

Open
UlyssesZh wants to merge 1 commit intoNixOS:masterfrom
UlyssesZh:ksre-build
Open

katawa-shoujo-re-engineered: compile rpy files in the build phase#490290
UlyssesZh wants to merge 1 commit intoNixOS:masterfrom
UlyssesZh:ksre-build

Conversation

@UlyssesZh
Copy link
Member

This significantly speeds up launching.

Things done

  • Built on platform:
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • Tested, as applicable:
  • Ran nixpkgs-review on this PR. See nixpkgs-review usage.
  • Tested basic functionality of all binary files, usually in ./result/bin/.
  • Nixpkgs Release Notes
    • Package update: when the change is major or breaking.
  • NixOS Release Notes
    • Module addition: when adding a new NixOS module.
    • Module update: when the change is significant.
  • Fits CONTRIBUTING.md, pkgs/README.md, maintainers/README.md and other READMEs.

@nixpkgs-ci nixpkgs-ci bot added 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. 10.rebuild-darwin: 1-10 This PR causes between 1 and 10 packages to rebuild on Darwin. 10.rebuild-darwin: 1 This PR causes 1 package to rebuild on Darwin. 10.rebuild-linux: 1 This PR causes 1 package to rebuild on Linux. labels Feb 14, 2026
@UlyssesZh UlyssesZh closed this Feb 20, 2026
@UlyssesZh UlyssesZh reopened this Feb 20, 2026
@quantenzitrone
Copy link
Contributor

oh so renpy depends on python-ecdsa which is a purely educational crypto library that is absolutely not meant for usage anywhere
can't even play games without PermittedInsecurePackages

@quantenzitrone
Copy link
Contributor

ok it has been replaced in unreleased renpy

Copy link
Contributor

@quantenzitrone quantenzitrone left a comment

Choose a reason for hiding this comment

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

code looks good other than the nit
game builds, runs and plays fine

i seem to be having heavy memory leaks during cutscenes (~350MB/s), however this is independent of the game being pre-compiled or not, so not an issue with this pr but probably a renpy or ksre upstream bug

@nixpkgs-ci nixpkgs-ci bot added 12.approvals: 1 This PR was reviewed and approved by one person. 12.approved-by: package-maintainer This PR was reviewed and approved by a maintainer listed in any of the changed packages. labels Feb 21, 2026
Comment on lines +40 to +46
configurePhase = ''
runHook preConfigure

substituteInPlace game/config.rpy --replace-fail 0.0.0-localbuild ${finalAttrs.version}

runHook postConfigure
'';
Copy link
Contributor

Choose a reason for hiding this comment

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

if separate, that definitely belongs in the patchPhase

Suggested change
configurePhase = ''
runHook preConfigure
substituteInPlace game/config.rpy --replace-fail 0.0.0-localbuild ${finalAttrs.version}
runHook postConfigure
'';
postPatch = ''
substituteInPlace game/config.rpy --replace-fail 0.0.0-localbuild ${finalAttrs.version}
'';

Copy link
Member Author

Choose a reason for hiding this comment

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

Actually this definitely is the configure phase.

Copy link
Contributor

Choose a reason for hiding this comment

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

hm i don't know enough about what the configure phase is supposed to be other than autotools specific running the ./configure script.
so since you're so sure, i'll let it go

@quantenzitrone
Copy link
Contributor

bisecting the memory leak is actually impossible
half the time either renpy is broken or the version of ksre has been deleted

Copy link
Member

@Eveeifyeve Eveeifyeve left a comment

Choose a reason for hiding this comment

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

The worry that I have about this package is this comment here: #490290 (comment) about a memory leak, but if it's impossible to bisect then I wouldn't bother it in this pr. It would be good to create an issue for that upstream to the katawa-shoujo-re-engineered repo so that way people are aware of a memory leak. Everything else looks good.

@nixpkgs-ci nixpkgs-ci bot added 12.approvals: 2 This PR was reviewed and approved by two persons. and removed 12.approvals: 1 This PR was reviewed and approved by one person. labels Mar 1, 2026
@quantenzitrone
Copy link
Contributor

quantenzitrone commented Mar 1, 2026

renpy/renpy#6934
i actually managed to bisect by running ks:re from a local checkout and smartly choosing commits to try
d96d13a is the culprit (#393386)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

10.rebuild-darwin: 1-10 This PR causes between 1 and 10 packages to rebuild on Darwin. 10.rebuild-darwin: 1 This PR causes 1 package to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. 10.rebuild-linux: 1 This PR causes 1 package to rebuild on Linux. 12.approvals: 2 This PR was reviewed and approved by two persons. 12.approved-by: package-maintainer This PR was reviewed and approved by a maintainer listed in any of the changed packages.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants