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

fix(#130): Don't propagate semver metadata into deb metadata. #187

Merged
merged 2 commits into from
Jul 30, 2020

Conversation

erikgeiser
Copy link
Member

@erikgeiser erikgeiser commented Jul 29, 2020

This pull request closes #130 by not propagating the semver metadata info the Deb.Metadata field. As described in the corresponding issue, this is the expected behaviour for deb files.

EDIT: changed it to wip because I actually don't even know why Deb.Metadata exists if it shouldn't be included in the version.

@pull-request-size pull-request-size bot added the size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. label Jul 29, 2020
@vercel
Copy link

vercel bot commented Jul 29, 2020

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/goreleaser/nfpm/m1iekkfas
✅ Preview: https://nfpm-git-fork-erikgeiser-fix-deb-metadata.goreleaser.vercel.app

@vercel vercel bot temporarily deployed to Preview July 29, 2020 21:49 Inactive
@vercel vercel bot temporarily deployed to Preview July 29, 2020 22:01 Inactive
@erikgeiser erikgeiser changed the title fix(#130): Don't propagate semver metadata into deb metadata. wip(#130): Don't propagate semver metadata into deb metadata. Jul 29, 2020
@vercel vercel bot temporarily deployed to Preview July 29, 2020 22:35 Inactive
@erikgeiser
Copy link
Member Author

@caarlos0 is there still a point in keeping Deb.Metadata around at all if it shouldn't be included in the version?

@caarlos0
Copy link
Member

I think the user can still set something on nfpm.yaml 🤔

@erikgeiser
Copy link
Member Author

erikgeiser commented Jul 30, 2020

Yes but it won't end up anywhere as far as I can see. Until now in this PR I only removed the part here the metadata is populated automatically, but according to the arguments in the issue it should not end up in the packages version information at all, even if set explicitly in the config. In this case we could just remove the entire Deb.Metadata field altogether.

EDIT: It seems to me that while semver supports metadata, the deb format does not.

@caarlos0
Copy link
Member

Yes but it won't end up anywhere as far as I can see. Until now in this PR I only removed the part here the metadata is populated automatically, but according to the arguments in the issue it should not end up in the packages version information at all, even if set explicitly in the config. In this case we could just remove the entire Deb.Metadata field altogether.

I think removing the auto-metadata seems legit, but if the user wants to do add something manually I don't see a problem...

@erikgeiser
Copy link
Member Author

That's a valid argument. However, the counter argument would be that this enables users to create deb packages where the version comparison is broken.

@caarlos0
Copy link
Member

That's a valid argument. However, the counter argument would be that this enables users to create deb packages where the version comparison is broken.

It depends:

root@7f2aaf25ce50:/# dpkg --compare-versions 0.1.11~beta3+git eq 0.1.11~beta3+git && echo yes
yes
root@7f2aaf25ce50:/# dpkg --compare-versions 0.1.11~beta4+git gt 0.1.11~beta3+git && echo yes
yes
root@7f2aaf25ce50:/# dpkg --compare-versions 0.1.12~beta3+git gt 0.1.11~beta3+git && echo yes
yes

if the user wants to always set the metadata to git, for example, it works just fine.

The problem is that we were always changing it (setting the commit hash), and them the comparison was broken...

@caarlos0
Copy link
Member

Maybe we can just point that out in docs (to not abuse the metadata risking breaking dpkg version comparison)

@vercel vercel bot temporarily deployed to Preview July 30, 2020 13:52 Inactive
@erikgeiser erikgeiser changed the title wip(#130): Don't propagate semver metadata into deb metadata. fix(#130): Don't propagate semver metadata into deb metadata. Jul 30, 2020
@erikgeiser
Copy link
Member Author

Okay, let's do it this way. I changed the docs and removed the wip.

@caarlos0 caarlos0 merged commit 005a6f8 into goreleaser:master Jul 30, 2020
@caarlos0
Copy link
Member

Thanks!

@caarlos0
Copy link
Member

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@goreleaser goreleaser locked as resolved and limited conversation to collaborators Nov 17, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
size/XS Denotes a PR that changes 0-9 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

build metadata in .deb version breaks precedence order
2 participants