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

kavita: 0.7.1.4 -> 0.7.13 #281236

Merged
merged 6 commits into from
Mar 27, 2024
Merged

kavita: 0.7.1.4 -> 0.7.13 #281236

merged 6 commits into from
Mar 27, 2024

Conversation

melvyn2
Copy link
Contributor

@melvyn2 melvyn2 commented Jan 15, 2024

Description of changes

https://github.com/Kareadita/Kavita/releases/tag/v0.7.12
https://github.com/Kareadita/Kavita/releases/tag/v0.7.13

Also imported configuration changes from #263649, including switching to freeform settings.

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.05 Release Notes (or backporting 23.05 and 23.11 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.

@ofborg ofborg bot requested a review from Misterio77 January 16, 2024 07:52
@ofborg ofborg bot added 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 1-10 10.rebuild-linux: 1 labels Jan 16, 2024
Copy link
Member

@nevivurn nevivurn left a comment

Choose a reason for hiding this comment

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

Should supersede #242739 and #263649. You can probably refer to #263649 for test fixes.

@github-actions github-actions bot added 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 8.has: module (update) This PR changes an existing module in `nixos/` labels Jan 21, 2024
@melvyn2
Copy link
Contributor Author

melvyn2 commented Jan 21, 2024

I probably should have searched for these before I submitted another PR, oops. Yours seems to have a lot more fixes, but if you want it superseded I can cherry pick your changes.

@melvyn2
Copy link
Contributor Author

melvyn2 commented Jan 21, 2024

Seeing that you added yourself as a maintainer it might make more sense just to update the previous PR and get it merged, however.

@ofborg ofborg bot requested a review from nevivurn January 21, 2024 20:45
Copy link
Member

@nevivurn nevivurn left a comment

Choose a reason for hiding this comment

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

We can just move forward in this PR, IMO. Tested locally on my system.

Looks good to me, I think it just needs a mention in the release notes as the module changes are breaking.

A nit, doesn't need to be addressed in this PR, but I thought it would be nice to remove the backend->frontend dependency and add some glue to pass in the correct frontend path at runtime.

It could speed up builds, and it would also speed up running the update script as it currently needs to build the frontend just to build backend.fetch-deps. Also makes patching the frontend less burdensome.

@luzpaz
Copy link
Contributor

luzpaz commented Jan 24, 2024

Note: https://github.com/Kareadita/Kavita/releases/tag/v0.7.13 is latest

@ofborg ofborg bot requested a review from nevivurn January 26, 2024 01:15
@nevivurn
Copy link
Member

Changes seem OK, still needs release notes mention.

@melvyn2 melvyn2 changed the title kavita: 0.7.1.4 -> 0.7.12 kavita: 0.7.1.4 -> 0.7.13 Jan 27, 2024
@github-actions github-actions bot added 8.has: documentation This PR adds or changes documentation 8.has: changelog labels Jan 27, 2024
@ofborg ofborg bot added the 2.status: merge conflict This PR has merge conflicts with the target branch label Jan 27, 2024
@melvyn2
Copy link
Contributor Author

melvyn2 commented Jan 29, 2024

Rebased to solve the merge conflict

@ofborg ofborg bot removed the 2.status: merge conflict This PR has merge conflicts with the target branch label Jan 29, 2024
Copy link
Member

@Misterio77 Misterio77 left a comment

Choose a reason for hiding this comment

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

Thanks for this! Diff looks good, local testing looks nice as well.

@nevivurn nevivurn mentioned this pull request Feb 2, 2024
13 tasks
@Misterio77 Misterio77 added the 12.approvals: 2 This PR was reviewed and approved by two reputable people label Feb 16, 2024
@Misterio77 Misterio77 mentioned this pull request Feb 16, 2024
12 tasks
@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-already-reviewed/2617/1498

Comment on lines 28 to 35
(substituteAll {
src = ./change-webroot.diff;
web_root = "${finalAttrs.frontend}/lib/node_modules/kavita-webui/dist";
})
./change-webroot.diff
];
postPatch = ''
substituteInPlace API/Services/DirectoryService.cs --replace "@NixKavitaBackendLib@" "$out/lib/kavita-backend/"
substituteInPlace API/Startup.cs API/Services/LocalizationService.cs API/Controllers/FallbackController.cs \
--replace "@NixKavitaWebRoot@" "${finalAttrs.frontend}/lib/node_modules/kavita-webui/dist/browser"
'';
Copy link
Member

Choose a reason for hiding this comment

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

Please keep the subsituteAll, as it is more clean

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Do you have an idea for how to refer to the package output rather than the patch output, in that case? I first tried to preserve the substituteAll but couldn't figure that part out.

Copy link
Member

Choose a reason for hiding this comment

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

I don't think that is possible, as $out or placeholder $out refers to the patch

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In that case, I have to use the bash substituteAll - this seems like a bad choice, as the variables aren't explicitly defined, making it hard to trace what it's used for (further explained in #237216). I cleaned up the substituteInPlace usage a little, maybe this is more palatable.

@melvyn2 melvyn2 force-pushed the update-kavita branch 2 times, most recently from 36674ab to 5497589 Compare March 26, 2024 03:35
@ofborg ofborg bot requested review from nevivurn and Misterio77 March 26, 2024 04:05
Copy link
Member

@SuperSandro2000 SuperSandro2000 left a comment

Choose a reason for hiding this comment

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

Tests are failing:

kavita # [  979.738746] kavita[14893]: [Kavita] [2024-03-26 04:37:51.333 +00:00  9] [Fatal] API.Program Running MigrateLibrariesToHaveAllFileTypes migration - Completed. This is not an error
kavita # [  979.784446] kavita[14893]: [Kavita] [2024-03-26 04:37:51.378 +00:00  9] [Information] API.Program Running Migrations - complete
kavita # [  979.802062] kavita[14893]: [Kavita] [2024-03-26 04:37:51.385 +00:00  1] [Error]  There was an error setting base url
kavita # [  979.803021] kavita[14893]: System.IO.DirectoryNotFoundException: Could not find a part of the path '/var/lib/kavita/@version@/index.html'.
kavita # [  979.804095] kavita[14893]:    at Interop.ThrowExceptionForIoErrno(ErrorInfo errorInfo, String path, Boolean isDirError)
kavita # [  979.805046] kavita[14893]:    at Microsoft.Win32.SafeHandles.SafeFileHandle.Open(String path, OpenFlags flags, Int32 mode, Boolean failForSymlink, Boolean& wasSymlink, Func`4 createOpenException)
kavita # [  979.806432] kavita[14893]:    at Microsoft.Win32.SafeHandles.SafeFileHandle.Open(String fullPath, FileMode mode, FileAccess access, FileShare share, FileOptions options, Int64 preallocationSize, UnixFileMode openPermissions, Int64& fileLength, UnixFileMode& filePermissions, Boolean failForSymlink, Boolean& wasSymlink, Func`4 createOpenException)
kavita # [  979.808773] kavita[14893]:    at System.IO.Strategies.OSFileStreamStrategy..ctor(String path, FileMode mode, FileAccess access, FileShare share, FileOptions options, Int64 preallocationSize, Nullable`1 unixCreateMode)
kavita # [  979.810299] kavita[14893]:    at System.IO.StreamReader.ValidateArgsAndOpenPath(String path, Encoding encoding, Int32 bufferSize)
kavita # [  979.811350] kavita[14893]:    at System.IO.StreamReader..ctor(String path, Encoding encoding)
kavita # [  979.812097] kavita[14893]:    at HtmlAgilityPack.HtmlDocument.Load(String path)
kavita # [  979.812736] kavita[14893]:    at API.Startup.UpdateBaseUrlInIndex(String baseUrl) in /build/source/API/Startup.cs:line 409
kavita # [  979.820871] kavita[14893]: [Kavita] [2024-03-26 04:37:51.414 +00:00  1] [Fatal] Microsoft.AspNetCore.Hosting.Diagnostics Application startup exception
kavita # [  979.822081] kavita[14893]: System.ArgumentException: The path must be absolute. (Parameter 'root')
kavita # [  979.823054] kavita[14893]:    at Microsoft.Extensions.FileProviders.PhysicalFileProvider..ctor(String root, ExclusionFilters filters)
kavita # [  979.825072] kavita[14893]:    at API.Startup.Configure(IApplicationBuilder app, IBackgroundJobClient backgroundJobs, IWebHostEnvironment env, IHostApplicationLifetime applicationLifetime, IServiceProvider serviceProvider, ICacheService cacheService, IDirectoryService directoryService, IUnitOfWork unitOfWork, IBackupService backupService, IImageService imageService) in /build/source/API/Startup.cs:line 335
kavita # [  979.827836] kavita[14893]:    at System.RuntimeMethodHandle.InvokeMethod(Object target, Void** arguments, Signature sig, Boolean isConstructor)
kavita # [  979.828901] kavita[14893]:    at System.Reflection.MethodBaseInvoker.InvokeWithManyArgs(Object obj, BindingFlags invokeAttr, Binder binder, Object[] parameters, CultureInfo culture)
kavita # [  979.830210] kavita[14893]:    at Microsoft.AspNetCore.Hosting.ConfigureBuilder.Invoke(Object instance, IApplicationBuilder builder)
kavita # [  979.831199] kavita[14893]:    at Microsoft.AspNetCore.Hosting.GenericWebHostService.StartAsync(CancellationToken cancellationToken)
kavita # [  979.833059] kavita[14893]: [Kavita] [2024-03-26 04:37:51.419 +00:00  1] [Error] Microsoft.Extensions.Hosting.Internal.Host Hosting failed to start
kavita # [  979.834193] kavita[14893]: System.ArgumentException: The path must be absolute. (Parameter 'root')
kavita # [  979.834962] kavita[14893]:    at Microsoft.Extensions.FileProviders.PhysicalFileProvider..ctor(String root, ExclusionFilters filters)
kavita # [  979.835978] kavita[14893]:    at API.Startup.Configure(IApplicationBuilder app, IBackgroundJobClient backgroundJobs, IWebHostEnvironment env, IHostApplicationLifetime applicationLifetime, IServiceProvider serviceProvider, ICacheService cacheService, IDirectoryService directoryService, IUnitOfWork unitOfWork, IBackupService backupService, IImageService imageService) in /build/source/API/Startup.cs:line 335
kavita # [  979.838743] kavita[14893]:    at System.RuntimeMethodHandle.InvokeMethod(Object target, Void** arguments, Signature sig, Boolean isConstructor)
kavita # [  979.840124] kavita[14893]:    at System.Reflection.MethodBaseInvoker.InvokeWithManyArgs(Object obj, BindingFlags invokeAttr, Binder binder, Object[] parameters, CultureInfo culture)
kavita # [  979.841437] kavita[14893]:    at Microsoft.AspNetCore.Hosting.ConfigureBuilder.Invoke(Object instance, IApplicationBuilder builder)
kavita # [  979.842431] kavita[14893]:    at Microsoft.AspNetCore.Hosting.GenericWebHostService.StartAsync(CancellationToken cancellationToken)
kavita # [  979.844128] kavita[14893]:    at Microsoft.Extensions.Hosting.Internal.Host.<StartAsync>b__15_1(IHostedService service, CancellationToken token)
kavita # [  979.845243] kavita[14893]:    at Microsoft.Extensions.Hosting.Internal.Host.ForeachService[T](IEnumerable`1 services, CancellationToken token, Boolean concurrent, Boolean abortOnFirstException, List`1 exceptions, Func`3 operation)
kavita # [  979.850599] kavita[14893]: [Kavita] [2024-03-26 04:37:51.444 +00:00  1] [Fatal]  Host terminated unexpectedly
kavita # [  979.851495] kavita[14893]: System.ArgumentException: The path must be absolute. (Parameter 'root')
kavita # [  979.852343] kavita[14893]:    at Microsoft.Extensions.FileProviders.PhysicalFileProvider..ctor(String root, ExclusionFilters filters)
kavita # [  979.854060] kavita[14893]:    at API.Startup.Configure(IApplicationBuilder app, IBackgroundJobClient backgroundJobs, IWebHostEnvironment env, IHostApplicationLifetime applicationLifetime, IServiceProvider serviceProvider, ICacheService cacheService, IDirectoryService directoryService, IUnitOfWork unitOfWork, IBackupService backupService, IImageService imageService) in /build/source/API/Startup.cs:line 335
kavita # [  979.856825] kavita[14893]:    at System.RuntimeMethodHandle.InvokeMethod(Object target, Void** arguments, Signature sig, Boolean isConstructor)
kavita # [  979.857905] kavita[14893]:    at System.Reflection.MethodBaseInvoker.InvokeWithManyArgs(Object obj, BindingFlags invokeAttr, Binder binder, Object[] parameters, CultureInfo culture)
kavita # [  979.859218] kavita[14893]:    at Microsoft.AspNetCore.Hosting.ConfigureBuilder.Invoke(Object instance, IApplicationBuilder builder)
kavita # [  979.860215] kavita[14893]:    at Microsoft.AspNetCore.Hosting.GenericWebHostService.StartAsync(CancellationToken cancellationToken)
kavita # [  979.861214] kavita[14893]:    at Microsoft.Extensions.Hosting.Internal.Host.<StartAsync>b__15_1(IHostedService service, CancellationToken token)
kavita # [  979.862296] kavita[14893]:    at Microsoft.Extensions.Hosting.Internal.Host.ForeachService[T](IEnumerable`1 services, CancellationToken token, Boolean concurrent, Boolean abortOnFirstException, List`1 exceptions, Func`3 operation)
kavita # [  979.863916] kavita[14893]:    at Microsoft.Extensions.Hosting.Internal.Host.StartAsync(CancellationToken cancellationToken)
kavita # [  979.865123] kavita[14893]:    at Microsoft.Extensions.Hosting.HostingAbstractionsHostExtensions.RunAsync(IHost host, CancellationToken token)
kavita # [  979.866184] kavita[14893]:    at Microsoft.Extensions.Hosting.HostingAbstractionsHostExtensions.RunAsync(IHost host, CancellationToken token)
kavita # [  979.867241] kavita[14893]:    at API.Program.Main(String[] args) in /build/source/API/Program.cs:line 116
kavita # [  979.905592] systemd[1]: kavita.service: Deactivated successfully.
kavita # [  979.908392] systemd[1]: kavita.service: Consumed 2.373s CPU time, no IP traffic.
kavita # [  979.911450] systemd[1]: run-credentials-kavita.service.mount: Deactivated successfully.
kavita #   % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
kavita #                                  Dload  Upload   Total   Spent    Left  Speed
kavita #   0     0    0     0    0     0      0      0 --:--:-- --:--:-- --:--:--     0  0     0    0     0    0     0      0      0 --:--:-- --:--:-- --:--:--     0
kavita # curl: (7) Failed to connect to kavita port 5000 after 11 ms: Couldn't connect to server
kavita # [  980.162059] systemd[1]: kavita.service: Scheduled restart job, restart counter is at 322.
kavita # [  980.166589] systemd[1]: Starting Kavita...
kavita # [  980.256907] systemd[1]: Started Kavita.
kavita #   % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
kavita #                                  Dload  Upload   Total   Spent    Left  Speed
kavita #   0     0    0     0    0     0      0      0 --:--:-- --:--:-- --:--:--     0  0     0    0     0    0     0      0      0 --:--:-- --:--:-- --:--:--     0  0     0    0     0    0     0      0      0 --:--:-- --:--:-- --:--:--     0
kavita # curl: (7) Failed to connect to kavita port 5000 after 22 ms: Couldn't connect to server
kavita #   % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
kavita #                                  Dload  Upload   Total   Spent    Left  Speed
kavita #   0     0    0     0    0     0      0      0 --:--:-- --:--:-- --:--:--     0  0     0    0     0    0     0      0      0 --:--:-- --:--:-- --:--:--     0
kavita # curl: (7) Failed to connect to kavita port 5000 after 17 ms: Couldn't connect to server
cleanup
kill machine (pid 9)
qemu-kvm: terminating on signal 15 from pid 6 (/nix/store/7wz6hm9i8wljz0hgwz1wqmn2zlbgavrq-python3-3.11.8/bin/python3.11)

@ofborg ofborg bot requested a review from nevivurn March 26, 2024 19:12
@melvyn2
Copy link
Contributor Author

melvyn2 commented Mar 27, 2024

Tests should be passing now, after all these messy pushes.

@SuperSandro2000 SuperSandro2000 merged commit f87c956 into NixOS:master Mar 27, 2024
23 of 24 checks passed
@melvyn2 melvyn2 deleted the update-kavita branch March 27, 2024 18:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 8.has: changelog 8.has: documentation This PR adds or changes documentation 8.has: module (update) This PR changes an existing module in `nixos/` 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 1-10 12.approvals: 2 This PR was reviewed and approved by two reputable people
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants