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

feat: add support for more rpm fields #16

Closed

Conversation

djgilcrease
Copy link
Contributor

@djgilcrease djgilcrease commented Sep 11, 2019

Add support for

  • Description
  • OS (aka Platform)
  • Vendor
  • URL
  • Packager
  • Licence
  • Provides
  • Compressor
  • Require (aka Depends)
  • Obsolete (aka Replaces)
  • Recommends
  • Suggests
  • Conflicts
  • RPMFile.Type
    • ConfigFile
    • DocFile
    • IconFile
    • MissingFile
    • NoReplaceFile
    • SpecFile
    • GhostFile
    • ReadmeFile
    • ExcludeFile
    • UnpatchedFile
    • PubKeyFile
    • PolicyFile

tags.go Outdated Show resolved Hide resolved
tags.go Outdated Show resolved Hide resolved
tags.go Outdated Show resolved Hide resolved
tags.go Outdated Show resolved Hide resolved
@djgilcrease
Copy link
Contributor Author

No clue how to get the go deps to work with bazel

@jarondl
Copy link
Collaborator

jarondl commented Sep 13, 2019

Thank you for your contribution!

There are three separate efforts here, and I would like to see them in separate pull requests.

  1. Adding some more fields, like conflicts, replaces, etc. I generally like that, but I think that they should also be exported to the tar2rpm binary, and to bazel. Also, I don't see any reason to support a different os, unless you tell me there is an actual usecase of rpm on any other os. Even if so, I don't think the rpm tooling cares about this (the rpm code does not read this tag AFAICT), so the added complexity is not worth it in my opinion. Also the provides-itself mechanism must be improved to avoid surprising results: If the package itself is not in provides, it should be added.

  2. Adding different compressors, such as xz and lzma. I don't know.. on the one hand, there is not so much benefit. On the other hand, the code seems not too bad. If you have a real usecase (i.e. your dependent library needs it today) we can add this, but please separate it into a different pull request. ( I would also need to review the packages you pulled in for licensing and security, although I'll guess that would be fine).

  3. Adding filetypes. This seems only partially complete in this cl, and I am not sure about the interface, especially when concerning tar2rpm and bazel. Different tar files for each group? I don't know. This merits a nice discussion in an issue before the pull request.

Once again thank you, and I hope I am not discouraging you with my many comments :) I just want to have the most maintainable code we can have

@djgilcrease
Copy link
Contributor Author

Will split this up.

The os I only added because nfpm allows setting this field in it's config so figured I would set it.
I will also add flags to tar2rpm for all the new fields in the RPMMetaData struct

The compressors are also needed by nfpm since it supports using gzip, xz, and lzma

Will open a issue to talk about the file types. The only file type I really need at the moment is the ConfigFile, but do see a potential for needing docs, and readme types in the future.

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.

3 participants