Skip to content

fix: handle packager, vendor and others#88

Merged
jarondl merged 1 commit intogoogle:masterfrom
caarlos0:optional-fields
Mar 29, 2024
Merged

fix: handle packager, vendor and others#88
jarondl merged 1 commit intogoogle:masterfrom
caarlos0:optional-fields

Conversation

@caarlos0
Copy link
Copy Markdown
Contributor

Packager, vendor, URL, and Epoch are being added to the headers even if they have no value and are not required.

This PR moves them to pointers, and add them if they are non-null values.

Could also not add them if they have their default value, which would not break anyone using the lib, but might be less clear, especially if someone wants to set epoch to 0, for example...

refs goreleaser/nfpm#619

@jarondl
Copy link
Copy Markdown
Contributor

jarondl commented Feb 6, 2024

Thank you!
So this is a breaking API change, right? I don't think we can simply push such a change without breaking anybody.
OTOH, using default values seems ok for strings, but I explicitly use epoch=0 in some of my code, to match other "repeatable builds" tooling.

Quite a conundrum.

@caarlos0
Copy link
Copy Markdown
Contributor Author

caarlos0 commented Feb 6, 2024

Yes, this will unfortunately be a breaking change...
I do think this is a change that should be made, though, even though the resulting rpm isn't exactly invalid, it is forbidden by rpmbuild, so probably there's a reason for that 🤔

@caarlos0
Copy link
Copy Markdown
Contributor Author

another idea would be to reserve values and use them instead e.g.

const NO_EPOCH = ^uint32(0)

if epoch != NO_EPOCH {
 // add field
}

wdyt?

@jarondl
Copy link
Copy Markdown
Contributor

jarondl commented Mar 28, 2024

Ohhhh using 4294967295 .. That's gross; I like it. OK I don't like it, but better it's than the alternative. Can you make the change? Be sure to include the literal 4294967295 in the code comment, and say why we went that route.

@caarlos0
Copy link
Copy Markdown
Contributor Author

pushed, let me know what you think!

Signed-off-by: Carlos Alexandro Becker <caarlos0@users.noreply.github.com>
@jarondl jarondl merged commit c2247cb into google:master Mar 29, 2024
@caarlos0 caarlos0 deleted the optional-fields branch April 1, 2024 19:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants