search

google / rpmpack

rpmpack (tar2rpm) - package rpms in pure golang or cli
Apache License 2.0
116 stars 32 forks source link

feat: add description, summary, build time and build host tags #33

Closed caarlos0 closed 4 years ago

caarlos0 commented 4 years ago

according to this, both are required tags and we are not setting them...

refs https://github.com/goreleaser/goreleaser/issues/1229

caarlos0 commented 4 years ago

Ideally we could add build date and build host as well, I can do it on another PR later. This one I think is more important, as those are required fields.

caarlos0 commented 4 years ago

cc/ @jarondl

caarlos0 commented 4 years ago

Ideally we could add build date and build host as well, I can do it on another PR later. This one I think is more important, as those are required fields.

scratch that, just added them too.

caarlos0 commented 4 years ago

gently ping @jarondl

jarondl commented 4 years ago

Sorry for taking so long. I have a very deep aversion to host and time. I don't think the host that packed the files has any meaning these days, especially when it can be some cloud runner running some go function. Both host and time make the build non reproducible (see https://reproducible-builds.org/ and https://wiki.debian.org/ReproducibleBuilds ), and also makes caching harder (for example for blaze).

The first two seem legit, and also required by the spec. Can we wait with host and time for #29 to be implemented, and then let clients roll it on their own if they want?

caarlos0 commented 4 years ago

Maybe just add the fields there but not set anything by default?

I added them because packagecloud breaks without them for some reason...

jarondl commented 4 years ago

Does packagecloud fail without all four, or just description and summary?

I'd be fine with an empty value in these fields.

caarlos0 commented 4 years ago

Does packagecloud fail without all four, or just description and summary?

it fails without only buildtime and buildhost as well.

On goreleaser/goreleaser#1229 we tried adding just desc and summary first, it still failed saying something about missing required fields (as far as I remember).

Adding build host and time fixed it... 🙁

caarlos0 commented 4 years ago

I'd be fine with an empty value in these fields.

just did that BTW :)

caarlos0 commented 4 years ago

thanks!