Arch packages implementation #31037

ExplodingDragon · 2024-05-21T07:23:36Z

This Commit was created by d1nch8g (#25396).

This PR adds a package registry for Arch Linux packages with support for package files, signatures, and automatic pacman-database management.

Features:

Push any tar.zst package and Gitea sign it.
Delete endpoint for specific package version and all related files
Supports trust levels with SigLevel = Required.
Package UI with instructions to connect to the new pacman database and visualised package metadata

You can follow this tutorial to build a *.pkg.tar.zst package for testing

docs pr: gitea/docs#47

Co-authored-by: d1nch8g@ion.lc
Co-authored-by: @KN4CK3R
Co-authored-by: @silverwind
Co-authored-by: @mahlzahn

ExplodingDragon · 2024-05-21T08:33:36Z

TODO:

Package signatures should be managed by Gitea, custom signatures will result in inconsistencies with index signatures.
Signatures should not be saved under properties
Pacman DB should not be generated on-the-fly, it needs to be added when uploaded
We should not support overwrite uploads, which breaks the structure, and should perform a delete operation before uploading.
Fix documentation errors .

KN4CK3R

Blocking until I have time to have a look.

wxiaoguang · 2024-11-05T03:15:05Z

Blocking until I have time to have a look.

Do you have a estimated time? 1.23 is releasing soon?

wxiaoguang · 2024-11-07T05:21:44Z

Made some changes:

Avoid using global variables (it slows down non-web commands).
Avoid bloating api.go, make the handlers process the requests.
Use unified PackageRegistryHost to avoid duplicate code.

Last call: if there is no other problem, I think this PR could be merged in a few days since it has enough tests.

…to pacman-packages

delvh · 2024-11-07T09:09:59Z

@KN4CK3R did you have time to take a look at this PR?

tests/integration/api_packages_arch_test.go

KN4CK3R

I still think this PR could be simpler just by following how the other package types are structured.

My implementation can still be found here: main...KN4CK3R:gitea:feature-arch

KN4CK3R · 2024-11-07T16:58:19Z

modules/packages/arch/metadata.go

+}
+
+// ValidatePackageSpec Arch package validation according to PKGBUILD specification.
+func ValidatePackageSpec(p *Package) error {


Why do we need this detailed validation? If the spec changes we must keep this in sync. It is the responsibility of the client to check these when installing a package but for the registry it's less relevant.

KN4CK3R · 2024-11-07T17:02:50Z

services/packages/arch/repository.go

+
+// GetPackageFile Get data related to provided filename and distribution, for package files
+// update download counter.
+func GetPackageFile(ctx context.Context, group, file string, ownerID int64) (io.ReadSeekCloser, *url.URL, *packages_model.PackageFile, error) {


Why not use one of the existing methods in code.gitea.io/gitea/services/packages?

KN4CK3R · 2024-11-07T17:04:26Z

modules/packages/arch/metadata.go

+}
+
+// Desc Create pacman package description file.
+func (p *Package) Desc() string {


I would move this code into the repository.go file to the other db related logic. Then it's not needed to have the MD5SUM and SHA256SUM fields because that info is already present.

KN4CK3R · 2024-11-07T17:05:22Z

services/packages/arch/repository.go

+	return packages_service.GetPackageFileStream(ctx, pkgFile)
+}
+
+func GetPackageDBFile(ctx context.Context, group, arch string, ownerID int64, signFile bool) (io.ReadSeekCloser, *url.URL, *packages_model.PackageFile, error) {


Same here. I think this should be handled by the router.

KN4CK3R · 2024-11-07T17:07:55Z

tests/integration/api_packages_arch_test.go

+	defer tests.PrepareTestEnv(t)()
+	user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2})
+	unpack := func(s string) []byte {
+		data, _ := base64.StdEncoding.DecodeString(strings.ReplaceAll(strings.ReplaceAll(strings.TrimSpace(s), "\n", ""), "\r", ""))


All these string replacements are not necessary for base64.StdEncoding.DecodeString.

https://github.com/go-gitea/gitea/blob/331e878e81d57235a53199383087c7649797a887/tests/integration/api_packages_alpine_test.go#L60C18-L60C50

KN4CK3R · 2024-11-07T17:09:18Z

tests/integration/api_packages_arch_test.go

+	}
+	rootURL := fmt.Sprintf("/api/packages/%s/arch", user.Name)
+
+	pkgs := map[string][]byte{


If we need so many test packages it may be better to generate them on the fly.

KN4CK3R · 2024-11-07T17:28:07Z

modules/packages/arch/metadata.go

+		"zst": {
+			magic: []byte{0x28, 0xB5, 0x2F, 0xFD},
+			archiver: func() archiver.Reader {
+				return archiver.NewTarZstd()
+			},
+		},
+		"xz": {
+			magic: []byte{0xFD, 0x37, 0x7A, 0x58, 0x5A},
+			archiver: func() archiver.Reader {
+				return archiver.NewTarXz()
+			},
+		},
+		"gz": {
+			magic: []byte{0x1F, 0x8B},
+			archiver: func() archiver.Reader {
+				return archiver.NewTarGz()
+			},
+		},


Are there any other containers than zst in the wild? All repositories I have tested just contain zst.

KN4CK3R · 2024-11-07T17:31:20Z

routers/api/packages/arch/arch.go

+	pathFields := strings.Split(strings.Trim(ctx.PathParam("*"), "/"), "/")
+	pathFieldsLen := len(pathFields)
+	if pathFieldsLen < 2 {
+		ctx.Status(http.StatusBadRequest)
+		return
+	}
+
+	group := strings.Join(pathFields[:pathFieldsLen-2], "/")
+	arch := pathFields[pathFieldsLen-2]
+	file := pathFields[pathFieldsLen-1]


Why not simply specify a clean route with parameters so you don't need to rebuild the logic here?

wxiaoguang · 2024-11-08T01:25:14Z

I still think this PR could be simpler just by following how the other package types are structured.

My implementation can still be found here: main...KN4CK3R:gitea:feature-arch

If we'd like to include it in 1.23, what's your suggestion to move on?

I think we have 3 choices:

Merge this as-is and refactor it later
You help to improve this PR and merge it
Use your implementation in a new PR and close this one (some tests seems good to be added to your implementation)

Personally I think 1 could work. If you have time recently, 2 & 3 could also work.

What's your opinion?

KN4CK3R · 2024-11-08T20:36:50Z

Problem with option 1 is that the refactoring would most likely use different routes and perhaps different metadata (stored as json) which may require migrations. So it's 2 or 3 for me. @ExplodingDragon are you willing to make/accept changes?

ExplodingDragon · 2024-11-09T05:18:53Z

@KN4CK3R

Are there any other containers than zst in the wild?

Yes, archlinuxarm uses xz.

use different routes and perhaps different metadata (stored as json)

The use of non-json storage is to avoid serialization when generating the db.

So it's 2 or 3 for me. @ExplodingDragon are you willing to make/accept changes?

I'm ok with both, 3 is better for me, I don't have much time to work on things, I'll close this pr later, other things are up to you to determine.

ExplodingDragon · 2024-11-09T05:20:54Z

Thanks @wxiaoguang and @yp05327 , sorry.

wxiaoguang · 2024-11-09T05:27:23Z

@KN4CK3R what's the next plan? Would you like to work on this PR, or propose a new one?

lunny · 2024-11-09T06:59:35Z

I think the main conflict is the router path, other codes could be refactored later. Maybe we can do a discussion about which change's routers are better?

wxiaoguang · 2024-11-15T15:05:04Z

@KN4CK3R what's the next plan? Would you like to work on this PR, or propose a new one?

If no plan, then still merge this as-is?

GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label May 21, 2024

pull-request-size bot added the size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. label May 21, 2024

ExplodingDragon marked this pull request as draft May 21, 2024 07:23

github-actions bot added modifies/translation modifies/api This PR adds API routes or modifies them modifies/go Pull requests that update Go code modifies/templates This PR modifies the template files modifies/docs labels May 21, 2024

ExplodingDragon force-pushed the pacman-packages branch from 9a3f45a to 1a49dd9 Compare May 21, 2024 07:28

github-actions bot added modifies/cli PR changes something on the CLI, i.e. gitea doctor or gitea admin modifies/migrations modifies/internal modifies/js modifies/dependencies labels May 21, 2024

ExplodingDragon closed this May 21, 2024

ExplodingDragon force-pushed the pacman-packages branch from 1a49dd9 to 1007ce7 Compare May 21, 2024 07:29

pull-request-size bot added size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. and removed size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels May 21, 2024

ExplodingDragon reopened this May 21, 2024

pull-request-size bot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels May 21, 2024

github-actions bot added modifies/dependencies and removed modifies/cli PR changes something on the CLI, i.e. gitea doctor or gitea admin modifies/migrations modifies/internal modifies/js modifies/dependencies labels May 21, 2024

github-actions bot removed the modifies/dependencies label Jun 28, 2024

ExplodingDragon marked this pull request as ready for review June 28, 2024 18:55

GiteaBot added lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. and removed lgtm/need 1 This PR needs approval from one additional maintainer to be merged. labels Oct 16, 2024

Merge branch 'main' into pacman-packages

aee7007

KN4CK3R requested changes Oct 16, 2024

View reviewed changes

GiteaBot added lgtm/blocked A maintainer has reservations with the PR and thus it cannot be merged and removed lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. labels Oct 16, 2024

ExplodingDragon and others added 3 commits November 6, 2024 09:02

Merge branch 'main' into pacman-packages

d679da8

Merge branch 'main' into pacman-packages

e07ddcb

fix

84ff353

use PackageRegistryHost

ce73234

wxiaoguang force-pushed the pacman-packages branch from 0d9547d to ce73234 Compare November 7, 2024 05:35

wxiaoguang and others added 4 commits November 7, 2024 16:06

fix test

76554fe

fix bugs from forgejo

f68456f

Merge branch 'pacman-packages' of github.com:ExplodingDragon/gitea in…

590e17b

…to pacman-packages

Merge branch 'main' into pacman-packages

5e959b0

wxiaoguang reviewed Nov 7, 2024

View reviewed changes

tests/integration/api_packages_arch_test.go Outdated Show resolved Hide resolved

improve test

15f7371

KN4CK3R reviewed Nov 7, 2024

View reviewed changes

ExplodingDragon closed this Nov 9, 2024

ExplodingDragon reopened this Nov 9, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Arch packages implementation #31037

Arch packages implementation #31037

ExplodingDragon commented May 21, 2024 •

edited

Loading

ExplodingDragon commented May 21, 2024 •

edited

Loading

KN4CK3R left a comment

wxiaoguang commented Nov 5, 2024

wxiaoguang commented Nov 7, 2024

delvh commented Nov 7, 2024

KN4CK3R left a comment

KN4CK3R Nov 7, 2024

KN4CK3R Nov 7, 2024

KN4CK3R Nov 7, 2024

KN4CK3R Nov 7, 2024

KN4CK3R Nov 7, 2024

KN4CK3R Nov 7, 2024

KN4CK3R Nov 7, 2024

KN4CK3R Nov 7, 2024

wxiaoguang commented Nov 8, 2024 •

edited

Loading

KN4CK3R commented Nov 8, 2024

ExplodingDragon commented Nov 9, 2024

ExplodingDragon commented Nov 9, 2024

wxiaoguang commented Nov 9, 2024

lunny commented Nov 9, 2024

wxiaoguang commented Nov 15, 2024

Arch packages implementation #31037

Are you sure you want to change the base?

Arch packages implementation #31037

Conversation

ExplodingDragon commented May 21, 2024 • edited Loading

ExplodingDragon commented May 21, 2024 • edited Loading

KN4CK3R left a comment

Choose a reason for hiding this comment

wxiaoguang commented Nov 5, 2024

wxiaoguang commented Nov 7, 2024

delvh commented Nov 7, 2024

KN4CK3R left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wxiaoguang commented Nov 8, 2024 • edited Loading

KN4CK3R commented Nov 8, 2024

ExplodingDragon commented Nov 9, 2024

ExplodingDragon commented Nov 9, 2024

wxiaoguang commented Nov 9, 2024

lunny commented Nov 9, 2024

wxiaoguang commented Nov 15, 2024

ExplodingDragon commented May 21, 2024 •

edited

Loading

ExplodingDragon commented May 21, 2024 •

edited

Loading

wxiaoguang commented Nov 8, 2024 •

edited

Loading