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

packages/generic: Do not restrict package versions to SemVer #20414

Merged
merged 10 commits into from
Jul 28, 2022
Merged

packages/generic: Do not restrict package versions to SemVer #20414

merged 10 commits into from
Jul 28, 2022

Conversation

algernon
Copy link
Contributor

There are existing packages out there whose version do not conform to SemVer, yet, one would like to have them available in a generic package repository. To this end, remove the SemVer restriction on package versions when using the Generic package registry, and replace it with a check that simply makes sure the version isn't empty.

This is similar in spirit to #20412, but for the Generic registry.

@6543 6543 added this to the 1.18.0 milestone Jul 19, 2022
@lunny
Copy link
Member

lunny commented Jul 20, 2022

This may become optional.

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Jul 20, 2022
@algernon
Copy link
Contributor Author

This may become optional.

I'd think that such version number enforcement should be the job of a CI, rather than the registrys.

With that said, I'm willing to update my PR to make the semver enforcement available, but optional, rather than taking it out completely. Should it be a gitea-wide (app.ini) or per-repo setting then?

@lafriks
Copy link
Member

lafriks commented Jul 20, 2022

I would say currently we could remove this restriction and in other PR implement it as per server or better per registry setting.

@lafriks
Copy link
Member

lafriks commented Jul 20, 2022

tests fail:

--- FAIL: TestPackageGeneric (1.25s)
    testlogger.go:78: 2022/07/19 19:18:07 modules/git/git.go:134:checkInit() [W] git module has been initialized already, duplicate init should be fixed
    --- FAIL: TestPackageGeneric/Upload (0.34s)
        testlogger.go:78: 2022/07/19 19:18:07 ...eb/routing/logger.go:99:func1() [I] [62d7036f-10] router: completed PUT /api/packages/user2/generic/te-st_pac.kage/1.0.3/fi-le_na.me for , 201 Created in 323.9ms @ generic/generic.go:66(generic.UploadPackage)
        api_packages_generic_test.go:45: 
            	Error Trace:	api_packages_generic_test.go:45
            	Error:      	Expected value not to be nil.
            	Test:       	TestPackageGeneric/Upload

@algernon
Copy link
Contributor Author

Looks like that test is expecting to get SemVer back, which is obviously wrong when that's the thing the PR removes. I'll update the test and force push in a bit.

There are existing packages out there whose version do not conform to SemVer,
yet, one would like to have them available in a generic package repository. To
this end, remove the SemVer restriction on package versions when using the
Generic package registry, and replace it with a check that simply makes sure the
version isn't empty.

Signed-off-by: Gergely Nagy <me@gergo.csillger.hu>
algernon and others added 2 commits July 20, 2022 18:22
Co-authored-by: KN4CK3R <admin@oldschoolhack.me>
Co-authored-by: KN4CK3R <admin@oldschoolhack.me>
algernon and others added 2 commits July 21, 2022 19:22
Instead of just checking against the space-trimmed version string, return the
same trimmed version too, so it gets used in that form down the line.

Also add `strings` to the list of imports, because of our use of TrimSpace.

Signed-off-by: Gergely Nagy <me@gergo.csillger.hu>
@GiteaBot GiteaBot added lgtm/need 1 This PR needs approval from one additional maintainer to be merged. and removed lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. labels Jul 24, 2022
algernon and others added 2 commits July 26, 2022 00:56
@GiteaBot 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 Jul 28, 2022
@6543
Copy link
Member

6543 commented Jul 28, 2022

🚀

@6543 6543 merged commit 99f2f82 into go-gitea:main Jul 28, 2022
6543 added a commit to 6543-forks/gitea that referenced this pull request Jul 28, 2022
…a#20414)

There are existing packages out there whose version do not conform to SemVer, yet, one would like to have them available in a generic package repository. To this end, remove the SemVer restriction on package versions when using the Generic package registry, and replace it with a check that simply makes sure the version isn't empty.

Signed-off-by: Gergely Nagy <me@gergo.csillger.hu>
Co-authored-by: KN4CK3R <admin@oldschoolhack.me>
Co-authored-by: 6543 <6543@obermui.de>
@6543 6543 added the backport/done All backports for this PR have been created label Jul 28, 2022
@6543
Copy link
Member

6543 commented Jul 28, 2022

-> #20531

6543 added a commit that referenced this pull request Jul 28, 2022
…#20531)

There are existing packages out there whose version do not conform to SemVer, yet, one would like to have them available in a generic package repository. To this end, remove the SemVer restriction on package versions when using the Generic package registry, and replace it with a check that simply makes sure the version isn't empty.

Signed-off-by: Gergely Nagy <me@gergo.csillger.hu>
Co-authored-by: KN4CK3R <admin@oldschoolhack.me>
Co-authored-by: 6543 <6543@obermui.de>
Co-authored-by: Gergely Nagy <algernon@users.noreply.github.com>
vsysoev pushed a commit to IntegraSDL/gitea that referenced this pull request Aug 10, 2022
…a#20414)

There are existing packages out there whose version do not conform to SemVer, yet, one would like to have them available in a generic package repository. To this end, remove the SemVer restriction on package versions when using the Generic package registry, and replace it with a check that simply makes sure the version isn't empty.

Signed-off-by: Gergely Nagy <me@gergo.csillger.hu>
Co-authored-by: KN4CK3R <admin@oldschoolhack.me>
Co-authored-by: 6543 <6543@obermui.de>
wxiaoguang pushed a commit that referenced this pull request Oct 7, 2022
Fixes #21250
Related #20414

Conan packages don't have to follow SemVer.
The migration fixes the setting for all existing Conan and Generic
(#20414) packages.
KN4CK3R added a commit to KN4CK3R/gitea that referenced this pull request Oct 7, 2022
Fixes go-gitea#21250
Related go-gitea#20414

Conan packages don't have to follow SemVer.
The migration fixes the setting for all existing Conan and Generic
(go-gitea#20414) packages.
@go-gitea go-gitea locked and limited conversation to collaborators May 3, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
backport/done All backports for this PR have been created lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. type/bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants