gitea icon indicating copy to clipboard operation
gitea copied to clipboard

Support nuspec manifest download for nuget packages

Open viceice opened this issue 1 year ago • 4 comments

Support downloading nuget nuspec manifest^1. This is useful for renovate because it uses this api to find the corresponding repository

viceice avatar Jan 25 '24 07:01 viceice

I think it would be better if we would store the nuspec file as additional file in the package version instead of generating it. The original file could/will have additional data we don't need for the package registry and don't store in the metadata. So we can't re-generate the full nuspec file afterwards.

KN4CK3R avatar Jan 25 '24 21:01 KN4CK3R

sure, but that's out of my go skills. sorry.

this implementation satisfies our needs for renovate bot.

viceice avatar Jan 26 '24 20:01 viceice

new idea:

read nuspec from package and add to cache. so later request can leverage the cache.

cache can also be filled when package is uploaded.

WDYT?

viceice avatar Jan 31 '24 19:01 viceice

we can also store it aside the package and serve directly if available. fallback to extract from package and put to cache in both cases. this way we don't need a migration.

viceice avatar Jan 31 '24 20:01 viceice

@KN4CK3R Can you please check this version?

viceice avatar Apr 04 '24 12:04 viceice

Converting to draft, doctor is not working as expected.

viceice avatar Apr 04 '24 13:04 viceice

sure, but that's out of my go skills. sorry.

this implementation satisfies our needs for renovate bot.

It's easier than what you have currently actually:

diff --git a/modules/packages/nuget/metadata.go b/modules/packages/nuget/metadata.go
index 6769c514cc..1e98ddffde 100644
--- a/modules/packages/nuget/metadata.go
+++ b/modules/packages/nuget/metadata.go
@@ -48,10 +48,11 @@ const maxNuspecFileSize = 3 * 1024 * 1024
 
 // Package represents a Nuget package
 type Package struct {
-	PackageType PackageType
-	ID          string
-	Version     string
-	Metadata    *Metadata
+	PackageType   PackageType
+	ID            string
+	Version       string
+	Metadata      *Metadata
+	NuspecContent *bytes.Buffer
 }
 
 // Metadata represents the metadata of a Nuget package
@@ -138,8 +139,9 @@ func ParsePackageMetaData(r io.ReaderAt, size int64) (*Package, error) {
 
 // ParseNuspecMetaData parses a Nuspec file to retrieve the metadata of a Nuget package
 func ParseNuspecMetaData(archive *zip.Reader, r io.Reader) (*Package, error) {
+	var nuspecBuf bytes.Buffer
 	var p nuspecPackage
-	if err := xml.NewDecoder(r).Decode(&p); err != nil {
+	if err := xml.NewDecoder(io.TeeReader(r, &nuspecBuf)).Decode(&p); err != nil {
 		return nil, err
 	}
 
@@ -212,10 +214,11 @@ func ParseNuspecMetaData(archive *zip.Reader, r io.Reader) (*Package, error) {
 		}
 	}
 	return &Package{
-		PackageType: packageType,
-		ID:          p.Metadata.ID,
-		Version:     toNormalizedVersion(v),
-		Metadata:    m,
+		PackageType:   packageType,
+		ID:            p.Metadata.ID,
+		Version:       toNormalizedVersion(v),
+		Metadata:      m,
+		NuspecContent: &nuspecBuf,
 	}, nil
 }

diff --git a/routers/api/packages/nuget/nuget.go b/routers/api/packages/nuget/nuget.go
index c28bc6c9d9..34dcda4e2d 100644
--- a/routers/api/packages/nuget/nuget.go
+++ b/routers/api/packages/nuget/nuget.go
@@ -431,7 +431,7 @@ func UploadPackage(ctx *context.Context) {
 		return
 	}
 
-	_, _, err := packages_service.CreatePackageAndAddFile(
+	pv, _, err := packages_service.CreatePackageAndAddFile(
 		ctx,
 		&packages_service.PackageCreationInfo{
 			PackageInfo: packages_service.PackageInfo{
@@ -465,6 +465,35 @@ func UploadPackage(ctx *context.Context) {
 		return
 	}
 
+	nuspecBuf, err := packages_module.CreateHashedBufferFromReaderWithSize(np.NuspecContent, np.NuspecContent.Len())
+	if err != nil {
+		apiError(ctx, http.StatusInternalServerError, err)
+		return
+	}
+	defer nuspecBuf.Close()
+
+	_, err = packages_service.AddFileToPackageVersionInternal(
+		ctx,
+		pv,
+		&packages_service.PackageFileCreationInfo{
+			PackageFileInfo: packages_service.PackageFileInfo{
+				Filename: strings.ToLower(fmt.Sprintf("%s.nuspec", np.ID)),
+			},
+			Creator: ctx.Doer,
+			Data:    nuspecBuf,
+			IsLead:  false,
+		},
+	)
+	if err != nil {
+		switch err {
+		case packages_service.ErrQuotaTotalCount, packages_service.ErrQuotaTypeSize, packages_service.ErrQuotaTotalSize:
+			apiError(ctx, http.StatusForbidden, err)
+		default:
+			apiError(ctx, http.StatusInternalServerError, err)
+		}
+		return
+	}
+
 	ctx.Status(http.StatusCreated)
 }

Nothing else is needed (+ tests). I have a branch currently with some NuGet changes for #30265. If you don't mind I could add this there too and add you as co-author. It doesn't add the nuspec file to existing packages with a migration but that may not be a big problem. I thought about a button in the package settings which would reinitialize (meta) data from the stored package (similar to what your doctor command would do) but that would be different PR.

KN4CK3R avatar Apr 04 '24 15:04 KN4CK3R

I've done the complex part, because I don't want to store the full manifest in the database a second time.

most other nuget API implementations are storing the nuspec aside the nupkg.

feel free to do your own implementation instead of this.

viceice avatar Apr 04 '24 20:04 viceice

It's not stored in the database but as file, same as your code.

I'm surprised the tests are passing because the hashed buffer is read twice (first nuspec parsing and when storing the file). The second read should already be at the end.

KN4CK3R avatar Apr 04 '24 21:04 KN4CK3R

It's not stored in the database but as file, same as your code.

🙈 I should have read your diff more closely

I'm surprised the tests are passing because the hashed buffer is read twice (first nuspec parsing and when storing the file). The second read should already be at the end.

I've a seek to begin somewhere 🙃

viceice avatar Apr 04 '24 21:04 viceice

I've a seek to begin somewhere 🙃

Searched and did not find it. But it's here in line 633 https://github.com/go-gitea/gitea/pull/28921/files#diff-efe687650466302ae2e35f382b2a66571e28065c6909e25893e515d5c9cc79f9R633

KN4CK3R avatar Apr 04 '24 21:04 KN4CK3R

Updated PR with your suggestions and added a doctor integration test

viceice avatar Apr 05 '24 07:04 viceice

Ready for another round of review 🤗

viceice avatar Apr 09 '24 06:04 viceice

As I said, I'm against the doctor command because it's to specific. #30280 added two enhancements which would need their own doctor command too. I'm in favor of some kind of action which would re-initialize the metadata of a given package.

KN4CK3R avatar Apr 13 '24 06:04 KN4CK3R

would it be mergable when I drop the doctor command? so the nuspec file are initially only available for new releases?

then we can later decide how to proceed with missing files. from a renovate perspective it would be enough because we only need this when there is a new release

viceice avatar Apr 15 '24 17:04 viceice

would it be mergable when I drop the doctor command? so the nuspec file are initially only available for new releases?

I agree

wxiaoguang avatar Apr 15 '24 17:04 wxiaoguang

Yes, I would prefer that.

KN4CK3R avatar Apr 15 '24 19:04 KN4CK3R

dropped the doctor command

viceice avatar Apr 16 '24 11:04 viceice

Will send some additions for the tests tomorrow. Then we can merge it.

KN4CK3R avatar Apr 16 '24 21:04 KN4CK3R

CI fails

wxiaoguang avatar Apr 17 '24 13:04 wxiaoguang

@KN4CK3R does this PR add support for reading repository data from metadata block, too? https://learn.microsoft.com/en-us/nuget/reference/nuspec#repository

<?xml version="1.0"?>
<package xmlns="http://schemas.microsoft.com/packaging/2010/07/nuspec.xsd">
    <metadata>
        ...
        <repository type="git" url="https://github.com/NuGet/NuGet.Client.git" branch="dev" commit="e1c65e4524cd70ee6e22abe33e6cb6ec73938cb3" />
        ...
    </metadata>
</package>

Or is repository data already available in 1.21 somehow?

PulsarFX avatar May 07 '24 16:05 PulsarFX

It's only displayed on the package page but I think you talk about auto-linking a package to repository? Then no, that's not part of this PR.

KN4CK3R avatar May 08 '24 06:05 KN4CK3R

It's only displayed on the package page but I think you talk about auto-linking a package to repository? Then no, that's not part of this PR.

Thanks. I am looking for a way to read the <repository> tag from the nuspec to get the commit data from it. Then I'll try reading the nuspec, as stated in the initial post for this PR. This should work, right?

PulsarFX avatar May 08 '24 07:05 PulsarFX

If you want to access the nuspec file from an external service, then yes, you can download that file from /api/packages/<user>/nuget/package/<name>/<version>/<name>.nuspec. But only for packages uploaded after this PR was merged if you use the nightly version of Gitea.

KN4CK3R avatar May 08 '24 07:05 KN4CK3R

Thanks! Works with 1.22.0-RC.1 for newly uploaded packages or also new versions of a package.

PulsarFX avatar May 08 '24 09:05 PulsarFX