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

pyfa: init at 2.60.1 #314934

Merged
merged 2 commits into from
Oct 19, 2024
Merged

pyfa: init at 2.60.1 #314934

merged 2 commits into from
Oct 19, 2024

Conversation

Daholli
Copy link
Contributor

@Daholli Daholli commented May 26, 2024

Description of changes

add new package: /nix/store/bq2na219yk2mnafamqiwdira8wwk42m7-pyfa-v2.58.3

Things done

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
    • sandbox = relaxed
    • sandbox = true
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 24.11 Release Notes (or backporting 23.11 and 24.05 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md.

Add a 👍 reaction to pull requests you find important.

@NixOSInfra NixOSInfra added the 12. first-time contribution This PR is the author's first one; please be gentle! label May 26, 2024
@ofborg ofborg bot added 8.has: package (new) This PR adds a new package 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild 10.rebuild-linux: 1-10 10.rebuild-linux: 1 labels May 26, 2024
@Daholli Daholli mentioned this pull request Jun 3, 2024
13 tasks
@ocfox
Copy link
Member

ocfox commented Jul 10, 2024

v2.59.2 released. Could this build from source?

@Daholli
Copy link
Contributor Author

Daholli commented Jul 10, 2024

v2.59.2 released. Could this build from source?

I can update it when i get home, we might be able to build it from source, but i have not done this, and ive heard python is a pain on nix. Can definitely look into it tho

@Daholli
Copy link
Contributor Author

Daholli commented Aug 21, 2024

Spent some time looking into building this from source, since this is my first package and i havent really worked with python recently i couldnt get it to work. It seems to use some unconventional sourceode structure and when I got it to the point where all dependencies were installed and I thought i did everything right I couldnt find the main and all references tell me to change "my" code structure so for now I won't be doing that.

@Daholli Daholli changed the title pyfa: init at v2.58.3 pyfa: init at v2.59.2 Sep 2, 2024
@ToasterUwU
Copy link
Member

Result of nixpkgs-review pr 314934 run on x86_64-linux 1

1 package built:
  • pyfa

Copy link
Member

@ToasterUwU ToasterUwU left a comment

Choose a reason for hiding this comment

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

Builds fine, starts fine, but then crashes with the following error message after just a few seconds. Looks like a missing dependency at first glance.

** (pyfa.py:37703): ERROR **: 15:02:36.485: Unable to spawn a new child process: Failed to spawn child process “/usr/lib/x86_64-linux-gnu/webkit2gtk-4.0/WebKitNetworkProcess” (No such file or directory)

@ToasterUwU
Copy link
Member

@Daholli I checked, the missing runtime dependency is webkitgtk_4_1

Can you please add it and recommit? I would love to have pyfa as a package on NixOS.
If you dont feel like finishing this, or dont have the time anymore to take care of this, feel free to let me know and i can take over.

@Daholli
Copy link
Contributor Author

Daholli commented Sep 20, 2024

Lemme look into it, just got off work

@ToasterUwU
Copy link
Member

@Daholli I also just realized you didn't put yourself in there as a maintainer, which is fine if intentional, just wanna make sure.

Feel free to add me as a Maintainer if needed/wanted. I use Pyfa at least once a week, so I will keep up to date with it automatically. I'm already in the maintainer file, so you can add toasteruwu to the maintainer list if you want to. No pressure tho, its your PR after all.

@Daholli
Copy link
Contributor Author

Daholli commented Sep 20, 2024

I wasn't sure if you can be a maintainer if you are not part of the nixpkg official contributers, but yeah i can gladly add you if you want to look after it, haven't been playing much eve recently.

Interesting, it has already worked before, but now i am getting a different error compared to you if I run it in an isolated environment.

@Daholli
Copy link
Contributor Author

Daholli commented Sep 20, 2024

wth... i dunno what i just did, i built it, trying to reproduce your mistake, it worked perfectly, now I am trying to start it again and it doesnt work at all anymore, it crashes because things are missing in the graphics pipeline, probably related to me playing around with the nividia driver yesterday. I'll push it again, but is it possible for you @ToasterUwU to just finish it?

@Daholli Daholli force-pushed the add-pyfa branch 2 times, most recently from 12186c4 to 444f618 Compare September 20, 2024 16:15
@ToasterUwU
Copy link
Member

Ok it still says its missing that dependency, so thats weird. I can take over if you want me too. I will just take your commits and add onto them

@ofborg ofborg bot requested a review from ToasterUwU September 20, 2024 18:14
@ToasterUwU ToasterUwU self-assigned this Sep 20, 2024
@ToasterUwU
Copy link
Member

@Daholli So i have been trying all kinds of things, and while googling around i found this issue.

This seems vaguely connected, but i cant really point my finger at the right direction. Any ideas?

AppImageCrafters/appimage-builder#175

@Daholli
Copy link
Contributor Author

Daholli commented Sep 21, 2024

https://github.com/Daholli/Pyfa/tree/nixos-support

can you try if this works for you? I used to use this and archived it after creating the PR. Not sure why this would work, but i guess it doesn't hurt to try, and I did use this actively

nix run github:Daholli/Pyfa/nixos-support

@ToasterUwU
Copy link
Member

@Daholli That fails in the exact same way as the pr one.

@ToasterUwU
Copy link
Member

Result of nixpkgs-review pr 314934 run on x86_64-linux 1

1 package built:
  • pyfa

@ofborg ofborg bot requested a review from ToasterUwU October 11, 2024 02:29
@ToasterUwU
Copy link
Member

@SuperSandro2000 Unless you have anything else, this is ready for merge now

@Daholli Daholli mentioned this pull request Oct 15, 2024
13 tasks
Copy link
Contributor

@gepbird gepbird left a comment

Choose a reason for hiding this comment

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

Tested it on x86_64-linux, works well!

pkgs/by-name/py/pyfa/package.nix Outdated Show resolved Hide resolved
@gepbird gepbird added the 12.approvals: 3+ This PR was reviewed and approved by three or more reputable people label Oct 15, 2024
@Daholli
Copy link
Contributor Author

Daholli commented Oct 15, 2024

Result of nixpkgs-review pr 314934 run on x86_64-linux 1

1 package built:
  • pyfa

@ofborg ofborg bot requested a review from ToasterUwU October 15, 2024 23:16
@Daholli
Copy link
Contributor Author

Daholli commented Oct 17, 2024

fixing merge conflict...

@Daholli
Copy link
Contributor Author

Daholli commented Oct 17, 2024

I still don`t get how to resolve these properly.. rebase always ends up with me having merge conflicts

@ToasterUwU
Copy link
Member

Result of nixpkgs-review pr 314934 run on x86_64-linux 1

1 package built:
  • pyfa

@Daholli
Copy link
Contributor Author

Daholli commented Oct 17, 2024

can we please merge this soon, merge conflicts are surprisingly annoying in this, not sure if I am doing something wrong

@ToasterUwU
Copy link
Member

@SuperSandro2000 Can you review this again now that all things have been addressed? After that please merge it if nothing is left to change

@ofborg ofborg bot requested a review from ToasterUwU October 17, 2024 14:23
@nixos-discourse
Copy link

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/prs-ready-for-review/3032/4707

@SuperSandro2000 SuperSandro2000 merged commit 30cae82 into NixOS:master Oct 19, 2024
28 of 29 checks passed
@gepbird gepbird mentioned this pull request Oct 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
8.has: maintainer-list (update) This PR changes `maintainers/maintainer-list.nix` 8.has: package (new) This PR adds a new package 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild 10.rebuild-linux: 1-10 10.rebuild-linux: 1 11.by: package-maintainer This PR was created by the maintainer of the package it changes 12.approvals: 3+ This PR was reviewed and approved by three or more reputable people 12. first-time contribution This PR is the author's first one; please be gentle!
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants