-
Notifications
You must be signed in to change notification settings - Fork 8
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
Refactor nix-foundryvtt flake and package derivation #15
Conversation
Thanks for your efforts - Looking over the code it seems fine. I will try at some point in the next day or so on my live server (with full backups). |
03b677b
to
118e820
Compare
I pushed a commit that allows build-based overrides. If you provide a build and don’t provide a version number, it uses the build to calculate the proper version. If you provide a version, that also works as expected. If you provide major and build, then it trusts you to know what you’re doing. |
118e820
to
98df076
Compare
The update script handles creating `packages-lock.json` for the new version as well as updating the default version and `versions.json`. The documentation has also been updated to reflect the new procedure.
98df076
to
0a7c92d
Compare
If you override a version then override the build, the build will determine the overall version number. If you overriden major and build, then the derivation assumes you know what you’re doing.
0a7c92d
to
896efae
Compare
I fetched and generated hashes for all the missing FoundryVTT versions. If someone really needs to run an old version, it should be possible to build the derivation with it at least. |
I added 11.309 to this PR. @JonBoyleCoding, were you able to test? |
I’ve currently got my server running 11.308 using this branch. I’m not actively running anything though, so I’ve just confirmed that it runs and I can enter games and use mods. Functionality seems okay, but I’d like to hear from users to make sure it’s also working for them as well. The current plan is to go ahead with merging on Wednesday barring any negative feedback. The new update scheme is considerably easier, and the new derivation should accommodate running a specific version much better. |
FoundryVTT checks that pixi.js has not been updated to a newer minor version and raises an error if it has. Respect that check by limiting pixi.js only to patch updates.
Thank you to @MatteoJoliveau for catching an issue with pixi.js. FoundryVTT checks that its minor version has not been increased, so I modified the update script to use a tilde constraint with pixi.js. I’d like to periodically update older package-lock.json files, so I also added a script to automate that and used it to regenerate package-lock.json for every version. |
This looks good to me. |
3349827
to
e7c459a
Compare
@reckenrode tested this branch (commit e7c459a) on my instance and nothing seems to have changed compared to my PR. Which is good! Pixi is fixed at 7.2.4, and all my mods and additions are still working. I'd say this branch is ready to go :) |
Thanks for testing! I’ve merged this PR into the main branch. Updates should be much easier now. I’ll also look at scripting regular refreshes since I’ve kept copies of every version available locally. |
Sorry for my late response here - everything's working as expected on the newer version. Good job with this update! |
@ciferkey @JonBoyleCoding @mfenniak @numinit Pinging everyone who’s contributed in case you’re still using this. If not, I apologize for the unnecessary ping. I also apologize for not getting around to this sooner. NixOS/nixpkgs#234710 has ended up taking up more of my time than I originally expected. 😅
This PR was prompted by #14. It seems something’s gone off with the node2nix stuff and Foundry’s package.json. I ran also into it when trying to use
npm shrinkwrap
instead ofnpm update
. I don’t know if this is a Foundry issue or an NPM issue. Doingnpm update
in the update script updates things in a way that allows them to build.This PR implements the code-related stuff mentioned in #6. I’ve built Foundry v9, v10, and v11 (builds 307 and 308). I’ve done some very light testing locally, but it would be good to make sure I haven’t broken anyone’s configs.