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

gpt4all: 2.8.0 -> 3.2.1 #337747

Merged
merged 3 commits into from
Sep 5, 2024
Merged

gpt4all: 2.8.0 -> 3.2.1 #337747

merged 3 commits into from
Sep 5, 2024

Conversation

Titaniumtown
Copy link
Contributor

@Titaniumtown Titaniumtown commented Aug 27, 2024

Description of changes

Changelog
closes: #335993

Things done

Updated gpt4all and added a patch to disable compile-time fetching of an embeddings model, shifted that to be done directly in the nix package.

  • 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.

@Titaniumtown
Copy link
Contributor Author

I've tested functionality locally on nixOS w/o any issues!

@ofborg ofborg bot requested a review from polygon August 27, 2024 17:35
@ofborg ofborg bot added 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 1-10 labels Aug 27, 2024
Copy link
Member

@ryand56 ryand56 left a comment

Choose a reason for hiding this comment

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

PR title should be: gpt4all: 2.8.0 -> 3.2.1

@Titaniumtown Titaniumtown changed the title gpt4all 2.7.5 -> 3.2.1 gpt4all 2.8.0 -> 3.2.1 Aug 28, 2024
@Titaniumtown
Copy link
Contributor Author

@ryand56 fixed. lol. ty for pointing that out :)

@Titaniumtown
Copy link
Contributor Author

@ryand56 does it all look good to you now?

@ryand56
Copy link
Member

ryand56 commented Aug 28, 2024

Looks good :) I'll test it later

@reedriley
Copy link
Contributor

reedriley commented Aug 28, 2024

When I apply this patch and build on x86_64-linux, I end up with a file named 60c2zplz1cxvsy27xg7sr5nxirqpkmlr-nomic-embed-text-v1.5.f16.gguf (with a prepended Nix store hash) in $out/resources/ - which the resulting binary won't look for.

More specifically - no embeddings for local docs are being generated; and the console has the following error:

[Warning] (Wed Aug 28 12:39:01 2024): embllm WARNING: Local embedding model not found
[Warning] (Wed Aug 28 12:39:01 2024): WARNING: Could not load model for embeddings

@reedriley
Copy link
Contributor

Here's what ended up working for me, in case it helps: e56a0c7

@Titaniumtown
Copy link
Contributor Author

@reedriley thanks for the feedback! I'll apply your changes tomorrow. Good catch!

@Titaniumtown
Copy link
Contributor Author

@reedriley just pushed your changes, ty <3

@reedriley
Copy link
Contributor

LGTM!

@Titaniumtown
Copy link
Contributor Author

@polygon any thoughts? the bot added you to review this :p

@wegank wegank added the 12.approvals: 1 This PR was reviewed and approved by one reputable person label Aug 31, 2024
@polygon
Copy link
Contributor

polygon commented Sep 2, 2024

@polygon any thoughts? the bot added you to review this :p

Looks like @drupol pulled a nice bait and switch on me. I maintained this out-of-tree because I only sporadically find time for maintenance work. Then he pushed for adoption into Nixpkgs and removed himself as a maintainer shortly after.

I will look into this, but it can take some time. If anyone wants to co-maintain this package, feel free to take over.

@drupol
Copy link
Contributor

drupol commented Sep 2, 2024

@polygon Sorry mate, it's not intentional at all... I try to avoid maintaining stuff I don't use, or else it's too much work for me.

Regarding this GPT4All, I used to use it until I switched to open-webui that I maintain actively.

@Titaniumtown
Copy link
Contributor Author

@polygon any thoughts? the bot added you to review this :p

Looks like @drupol pulled a nice bait and switch on me. I maintained this out-of-tree because I only sporadically find time for maintenance work. Then he pushed for adoption into Nixpkgs and removed himself as a maintainer shortly after.

I will look into this, but it can take some time. If anyone wants to co-maintain this package, feel free to take over.

I can work on it more if needed :) I want some more nix experience.

Copy link
Member

@felschr felschr left a comment

Choose a reason for hiding this comment

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

Commit message should be:

-gpt4all 2.8.0 -> 3.2.1
+gpt4all: 2.8.0 -> 3.2.1

@Titaniumtown if you wanna help maintain this package, you can also add yourself to the maitnainer-list.nix & meta.maintainers of this package.

@github-actions github-actions bot added the 8.has: maintainer-list (update) This PR changes `maintainers/maintainer-list.nix` label Sep 5, 2024
@Titaniumtown
Copy link
Contributor Author

@felschr I've added myself now! Thank you :)

@felschr
Copy link
Member

felschr commented Sep 5, 2024

@Titaniumtown adding yourself to the maintainers list & to meta.maintainers should be in separate commits, i.e. there should be 3 commits in total. Some of this is explained in CONTRIBUTING.md & maintainers/README.md but you can also check the git history for how people did this before.

@Titaniumtown
Copy link
Contributor Author

oh so sorry. I'll fix that right now.

@felschr
Copy link
Member

felschr commented Sep 5, 2024

Also as you can see in the checks, maintainer-list.nix needs to be formatted.

@felschr felschr changed the title gpt4all 2.8.0 -> 3.2.1 gpt4all: 2.8.0 -> 3.2.1 Sep 5, 2024
@ToasterUwU
Copy link
Member

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

2 packages built:
  • gpt4all
  • gpt4all-cuda

@ofborg ofborg bot added the 11.by: package-maintainer This PR was created by the maintainer of the package it changes label Sep 5, 2024
@felschr felschr merged commit 85057cf into NixOS:master Sep 5, 2024
29 of 31 checks passed
@Titaniumtown Titaniumtown deleted the dev/gpt4all-3.2.1 branch September 5, 2024 16:10
@felschr
Copy link
Member

felschr commented Sep 5, 2024

@Titaniumtown thank you for the update and welcome as a package maintainer 🥳!

@Titaniumtown
Copy link
Contributor Author

Thank you so much @felschr! I'm excited!

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` 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 1-10 11.by: package-maintainer This PR was created by the maintainer of the package it changes 12.approvals: 1 This PR was reviewed and approved by one reputable person
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Update request: gpt4all 2.8.0 → 3.2.1
8 participants