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.

Sign up for GitHub

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

Jump to bottom

gitea: 1.19.4 -> 1.20.0 #243883

Merged
merged 4 commits into from
Jul 21, 2023
Merged

gitea: 1.19.4 -> 1.20.0 #243883

merged 4 commits into from
Jul 21, 2023

Conversation

techknowlogick
Copy link
Member

@techknowlogick techknowlogick commented Jul 17, 2023 •
edited by SuperSandro2000
Loading

Description of changes

https://blog.gitea.com/release-of-1.20.0/

Closes #241497

Things done
  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandbox = true set in nix.conf? (See Nix manual)
  • 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/)
  • 23.11 Release Notes (or backporting 23.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.

@ofborg ofborg bot requested review from Ma27, kolaente and disassembler July 17, 2023 01:51
Ma27 added 2 commits July 17, 2023 11:49
@Ma27
nixos/gitea: explicitly set CHUNKED_UPLOAD_PATH to writable location
42105b2 
Fix for Gitea 1.20.0.

Without this being set, e.g. a `git push` (or `ssh` to `git@` in general) fails like this:

    2023/07/17 09:27:05 ...s/setting/setting.go:109:LoadCommonSettings() [F] Unable to load settings from config: unable to create chunked upload directory: /nix/store/yna9nf66wl2n9hlnhxi2g7fdgawk2kxl-gitea-1.20.0/bin/data/tmp/package-upload (mkdir /nix/store/yna9nf66wl2n9hlnhxi2g7fdgawk2kxl-gitea-1.20.0/bin/data: read-only file system)
    Connection to git.mbosch.me closed.
@Ma27
nixos/tests/gitea: fix
b8a8e97 
* Since Gitea 1.20 the request to `/commits` requires at least one retry
  because it appears to take a moment until Gitea actually knows that
  this repo isn't empty anymore (previously on 1.20 this failed with
  HTTP 409 which occurs when the requested repo is empty).
* Remove `*.shutdown()`, for some reason they hang regularly for unknown
  reasons.
@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 Jul 17, 2023
@Ma27
Copy link
Member

Ma27 commented Jul 17, 2023

@techknowlogick pushed to more fixes that were necessary to fix the app itself and the tests. PTAL.

Otherwise, LGTM 👍

@ofborg ofborg bot requested a review from Ma27 July 17, 2023 10:20
@SuperSandro2000
Copy link
Member

SuperSandro2000 commented Jul 17, 2023 •
edited
Loading

Do we need https://github.com/NixOS/nixpkgs/pull/241497/files#diff-8fc27249c6db4db29555807acb61dd85c6118492c05b1696ff92183dd4266277R18 ?

or should we use https://blog.gitea.com/release-of-1.20.0/#-systemd-notify-support-21151 ?

@Ma27
Copy link
Member

Ma27 commented Jul 17, 2023

or should we use https://blog.gitea.com/release-of-1.20.0/#-systemd-notify-support-21151 ?

Sounds like a good idea! @techknowlogick would you mind adding that to this PR?

Do we need https://github.com/NixOS/nixpkgs/pull/241497/files#diff-8fc27249c6db4db29555807acb61dd85c6118492c05b1696ff92183dd4266277R18 ?

Not sure, my personal instance is now running with 1.20 and I haven't spotted any issues so far.
However, @bendlas sorry, I'm not sure if I fully understand the problem you had in #241497. Is it possible to provide a minimal reproducer?

In fact, I had to do a similar fix, though: For some reason, AppDataPath (that's used for CHUNKED_UPLOAD_PATH) pointed to the store-path even though it should've been overridden by the module. While manually declaring CHUNKED_UPLOAD_PATH solved the issue for me, I'm not sure if that's actually a bug somewhere. @techknowlogick since you seem to also work on the upstream code-base, would you mind taking a closer look? :)

@bendlas
Copy link
Contributor

bendlas commented Jul 17, 2023

However, @bendlas sorry, I'm not sure if I fully understand the problem you had in #241497. Is it possible to provide a minimal reproducer?

I think we already have that: If the instance comes up, we don't need the patch.

An earlier release candidate crashed for me on startup without it (because it tried to write to app.ini).

@bendlas bendlas mentioned this pull request Jul 17, 2023
Merged
12 tasks
@Kranzes
Copy link
Member

Kranzes commented Jul 20, 2023

What else needs to be done before merging this PR?

@Ma27
Copy link
Member

Ma27 commented Jul 20, 2023

I added Type = "notify" now to the module.

If everybody is fine with my hackery for the CHUNKED_UPLOAD_PATH we can merge IMHO.

@Ma27 Ma27 merged commit 38823d1 into NixOS:master Jul 21, 2023
7 of 9 checks passed
@emilylange
Copy link
Member

Type = "notify" from b61919e puts Forgejo in a crash loop (every ~90 seconds), as Forgejo isn't at v1.20 yet (scheduled for July 24).

And I would have preferred that packages.CHUNKED_UPLOAD_PATH to be a mkDefault instead.

This was referenced Jul 22, 2023
Closed
Merged
@SuperSandro2000 SuperSandro2000 added the 1.severity: security Issues which raise a security issue, or PRs that fix one label Jul 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@SuperSandro2000 SuperSandro2000 SuperSandro2000 approved these changes

@kolaente kolaente Awaiting requested review from kolaente

@disassembler disassembler Awaiting requested review from disassembler

@Ma27 Ma27 Awaiting requested review from Ma27

Assignees
No one assigned
Labels
1.severity: security Issues which raise a security issue, or PRs that fix one 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/` 10.rebuild-darwin: 1-10 10.rebuild-darwin: 1 10.rebuild-linux: 1-10 11.by: package-maintainer This PR was created by the maintainer of the package it changes
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@techknowlogick @Ma27 @SuperSandro2000 @bendlas @Kranzes @emilylange