Skip to content

Conversation

UlyssesZh
Copy link
Member

@UlyssesZh UlyssesZh commented Sep 19, 2025

Graphest: graphing calculator that can faithfully plot arbitrary mathematical relations.

On Linux, when closing the app without saving, it has the same GLib-GObject error as #354253 (comment). I am not sure how it can be fixed.

I do not know how to let Electron use the correct app icon on macOS. In the worst case, I will just patch the new BrowserWindow({...}) call.

I probably broke cross-compilation by passing build inputs through env var, but I do not know how to test it.

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.

Add a 👍 reaction to pull requests you find important.

@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. 9.needs: reviewer This PR currently has no reviewers requested and needs attention. labels Sep 19, 2025
@UlyssesZh

This comment was marked as outdated.

@RossSmyth
Copy link
Contributor

Have you submitted any of your patches upstream?

Copy link
Contributor

@RossSmyth RossSmyth left a comment

Choose a reason for hiding this comment

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

Really good start.

Comment on lines 22 to 29
static =
p:
p.overrideAttrs (oldAttrs: {
configureFlags = oldAttrs.configureFlags ++ [ "--enable-static" ];
});
# The dependencies of graphest-fftw-sys and graphest-arb-sys need to be static
fftwFloatStatic = static fftwFloat;
flintStatic = static flint;
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
static =
p:
p.overrideAttrs (oldAttrs: {
configureFlags = oldAttrs.configureFlags ++ [ "--enable-static" ];
});
# The dependencies of graphest-fftw-sys and graphest-arb-sys need to be static
fftwFloatStatic = static fftwFloat;
flintStatic = static flint;
fftwFloatStatic = pkgsStatic.fftwFloat;
flintStatic = pkgsStatic.flint;

Copy link
Member Author

@UlyssesZh UlyssesZh Sep 20, 2025

Choose a reason for hiding this comment

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

Unfortunately pkgsStatic.flint build fails.

Running phase: unpackPhase
@nix { "action": "setPhase", "phase": "unpackPhase" }
unpacking source archive /nix/store/5630mrni8ikjp91049yd0cs6mrhsf2fz-ntl-11.5.1.tar.gz
source root is ntl-11.5.1/src
setting SOURCE_DATE_EPOCH to timestamp 1624480787 of file "ntl-11.5.1/src/libtool-seed/configure.ac"
Running phase: patchPhase
@nix { "action": "setPhase", "phase": "patchPhase" }
Running phase: updateAutotoolsGnuConfigScriptsPhase
@nix { "action": "setPhase", "phase": "updateAutotoolsGnuConfigScriptsPhase" }
Updating Autotools / GNU config script to a newer upstream version: ./libtool-origin/config.sub
Updating Autotools / GNU config script to a newer upstream version: ./libtool-origin/config.guess
Running phase: updateAutotoolsGnuConfigScriptsPhase
@nix { "action": "setPhase", "phase": "updateAutotoolsGnuConfigScriptsPhase" }
Updating Autotools / GNU config script to a newer upstream version: ./libtool-origin/config.sub
Updating Autotools / GNU config script to a newer upstream version: ./libtool-origin/config.guess
Running phase: configurePhase
@nix { "action": "setPhase", "phase": "configurePhase" }
fixing libtool script ./libtool-origin/ltmain.sh
patching script interpreter paths in ./configure
./configure: interpreter directive changed from "#!/bin/sh" to "/nix/store/q7sqwn7i6w2b67adw0bmix29pxg85x3w-bash-5.3p3/bin/sh"
configure flags: DEF_PREFIX=\$\(out\) SHARED=on NATIVE=off TUNE=x86 CXX=x86_64-unknown-linux-musl-c++ NTL_GF2X_LIB=on GF2X_PREFIX=/nix/store/di428z71mww2j3yyryp8q9s3awpjifph-gf2x-static-x86_64-unknown-linux-musl-1.3.0 --enable-static --disable-shared --disable-shared
Error: unrecognized option: --enable-static
type "./configure -h" for help

It fails even with withNtl = fales;:

/nix/store/2lxkb5n0rjjkc0n3x6sqhd5qxjra6x3p-x86_64-unknown-linux-musl-binutils-2.44/bin/x86_64-unknown-linux-musl-ld: /build/flint-3.3.1/libflint.a(double_interval_merged.o): in function `d_randtest2':
/build/flint-3.3.1/src/double_interval.h:188: multiple definition of `d_randtest2'; /build/cc7cRRi3.o:/build/flint-3.3.1/src/mag/test/t-d_log_lower_bound.c:28: first defined here

"Science"
"Math"
];
comment = finalAttrs.meta.description;
Copy link
Contributor

Choose a reason for hiding this comment

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

Hardcode description

description = "Graphing calculator that can faithfully plot arbitrary mathematical relations";
homepage = "https://github.com/unageek/graphest";
downloadPage = "https://github.com/unageek/graphest/releases";
changelog = "https://github.com/unageek/graphest/releases/tag/${finalAttrs.src.tag}";
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
changelog = "https://github.com/unageek/graphest/releases/tag/${finalAttrs.src.tag}";
changelog = "https://github.com/unageek/graphest/releases/tag/v${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.

I thought using src.tag may be better because it should be the tag.

Copy link
Contributor

Choose a reason for hiding this comment

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

It is generally preferred not to do such things since to make it more consistent throughout nixpkgs

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 there are plenty of packages using src.tag here.

Copy link
Contributor

Choose a reason for hiding this comment

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

It is not preferred to add any more.

@nixpkgs-ci nixpkgs-ci bot removed the 9.needs: reviewer This PR currently has no reviewers requested and needs attention. label Sep 19, 2025
@UlyssesZh
Copy link
Member Author

Have you submitted any of your patches upstream?

No. I can probably open a PR for supporting Flint 3 and latest Rust.

@RossSmyth
Copy link
Contributor

Definitely want to submit as many patches upstream as you can. Then just use fetchpatch on the PRs

@UlyssesZh UlyssesZh force-pushed the graphest branch 2 times, most recently from 1346a00 to 0ded7d6 Compare September 20, 2025 03:55
@UlyssesZh
Copy link
Member Author

I opened 3 PRs to the upstream, among which 2 are relevant to the packaging and have been applied here by fetchpatch.

@UlyssesZh
Copy link
Member Author

nixpkgs-review result

Generated using nixpkgs-review-gha

Command: nixpkgs-review pr 444217
Commit: 0ded7d62107e6c00a5787ee1d092f7b9f409c08c (subsequent changes)
Merge: 7962893da2545227a988114db2e8f7a582b45de4

Logs: https://github.com/UlyssesZh/nixpkgs-review-gha/actions/runs/17875001630


x86_64-linux

✅ 1 package built:
  • graphest

aarch64-linux

✅ 1 package built:
  • graphest

x86_64-darwin (sandbox = true)

❌ 1 package failed to build:
  • graphest

aarch64-darwin (sandbox = true)

✅ 1 package built:
  • graphest

@UlyssesZh
Copy link
Member Author

Not sure why there is a failure in nixpkgs-review-gha. It works on my VM running macOS.

Copy link
Contributor

@RossSmyth RossSmyth left a comment

Choose a reason for hiding this comment

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

Looks good.

@RossSmyth
Copy link
Contributor

x86 Darwin is being removed from a "Tier 1" platform soon so uhhh I don't really care.

@nixpkgs-ci nixpkgs-ci bot added the 12.approvals: 1 This PR was reviewed and approved by one person. label Sep 20, 2025
@UlyssesZh UlyssesZh force-pushed the graphest branch 2 times, most recently from 38f95b6 to 5310673 Compare September 21, 2025 23:20
@UlyssesZh
Copy link
Member Author

UlyssesZh commented Sep 21, 2025

The app icon on macOS is now fixed. Tested by running open -n result/Applications/Graphest.app.

Not sure whether autoSignDarwinBinariesHook is actually necessary here, but I see other packages do that.

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: 1 This PR was reviewed and approved by one person.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants