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

Add support for package scripts #19

Closed
chris-rock opened this issue Mar 18, 2018 · 22 comments · Fixed by #29
Closed

Add support for package scripts #19

chris-rock opened this issue Mar 18, 2018 · 22 comments · Fixed by #29
Labels
enhancement New feature or request

Comments

@chris-rock
Copy link
Contributor

chris-rock commented Mar 18, 2018

It would be nice to define scripts similar to fpm. The .goreleaser.yml could look like:

nfpm:
  scripts:
    "verify":         "scripts/verify.sh"
    "before_upgrade": "scripts/before_upgrade_script.sh"
    "after_upgrade":  "scripts/after_upgrade_script.sh"
    "before_install":  "scripts/before_install_script.sh"
    "after_install":  "scripts/after_install_script.sh"
    "before_remove":  "scripts/before_remove_script.sh"
    "after_remove":   "scripts/after_remove_script.sh"
    "rpm_pretrans":   "scripts/rpm_pretrans_script.sh"
    "rpm_posttrans":  "scripts/rpm_posttrans_script.sh"

Happy to help with a PR once we agreed on a way forward

@caarlos0 caarlos0 added the enhancement New feature or request label Mar 18, 2018
@caarlos0
Copy link
Member

LGTM

@caarlos0
Copy link
Member

Happy to help with a PR once we agreed on a way forward

Let me know if you need help :D

@chris-rock
Copy link
Contributor Author

@caarlos0 Unfortunately I am not able to start this work for at least the next week. Please go ahead, I look for any updated before I start working

@tympanix
Copy link
Contributor

tympanix commented Apr 8, 2018

Any progress? I may have some time on my hands to work on this. A few remarks; if we are to use the structure above, this would imply the same script is used across multiple packagers (rpm, deb ect.). This may not be an immediate problem, but power users may want to take advantage of some explicit features in a maintainer script (e.g. the arguments passed to debian's maintainer scripts). Therefore we may also, in addition to the above structure, allow custom maintainer scripts on a per packager basis.

In addition, for simple use cases, I would like to propose a module system. One may be able to draw inspiration from configuration managements tools (like modules in Ansible). This will enable the user to specify maintainer scripts with the use of higher level directives (e.g. creating users, declaring systemd units ect.). The maintainer scripts themselves will be generated by nfpm using packager specific best practices. This relates to solving #15 as well. However I expect this to be of the least priority among the above (and certainly something for the long run).

TLDR;

  • Packager independent maintainer scripts (like proposed by OP)
  • Maintainer scripts on a per packager basis (for power users)
  • A module abstraction level for maintainer scripts (in the long run)

N.B. Sorry for long post 😅

@caarlos0
Copy link
Member

caarlos0 commented Apr 8, 2018

Any progress?

Not really :)

The info about maintainer scripts on a per packager basis is good to know, I'm not a power user on these things so I usually only have the "common" parts... its good to know power users view on this as well :)

About the abstraction, IMHO, while it may be nice, it also may be overkill... maybe in a far future, definitely not soon :P

@caarlos0
Copy link
Member

caarlos0 commented Apr 8, 2018

seems like the fpm author also tried this init abstraction thing https://github.com/jordansissel/pleaserun

GitHub
pleaserun - An attempt to abstract this "init" script madness.

@chris-rock
Copy link
Contributor Author

I would prefer if we would stick to less abstraction. Including the scripts is straightforward.

I can follow the idea where we want some specific scripts for a specific os, but you can easily do this in your bash script, too. I also think package scripts are not necessarily service init scripts and I would like to avoid making any assumptions in goreleaser. Otherwise it does not only need to know about packaging but also os specific behavior. And what happens if some users run a custom init manager?

@caarlos0
Copy link
Member

caarlos0 commented Apr 8, 2018

And what happens if some users run a custom init manager?

that seems like a new and fresh kind of hell 😳

anyway, I agree with your points, less abstraction is definitely simpler!

@tympanix
Copy link
Contributor

tympanix commented Apr 8, 2018

Excellent! I agree, the abstraction is a wild and unwieldy idea at this stage. Let that be for another time. About the init files, for clarification, the user would still supply the systemd/upstart/sysvinit files (these are not generated), the maintainer scripts would only make the system aware of them (e.g. systemctl daemon-reload for systemd init systems).

I will see if I can find time to work on a fork 👍

@caarlos0
Copy link
Member

caarlos0 commented Apr 8, 2018

fwiw, I was curious and just found out some people do indeed write custom init systems.

😰

https://news.ycombinator.com/item?id=11064694

@tympanix
Copy link
Contributor

tympanix commented Apr 9, 2018

I am currently at a crossroads. Maybe we should follow the structure from OP, but I was thinking we can solve multiple issues at once (in partical #28) and per package scripts as mentioned above. Imagine a structure like this

type Config struct {
  nfpm.Info
  DebInfo nfpm.Info `yaml:"deb"` /* deb overrides */
  RPMInfo nfpm.Info `yaml:"rpm"`  /* rpm overrides */
}

This would allow per package overrides. One can also embed nfpm.Info in a new struct to allow packager explicit configurations. Scripts would look like this (and loose the rpm_ prefix).

scripts:
  preinstall: "./scripts/preinstall.sh" # both deb and rpm
rpm:
  scripts:
    pretrans: "./scripts/pretrans.sh" # rpm explicit script

I was thinking we can use imdario/mergo (to merge configurations) and mitchellh/mapstructure (to dynamically access overrides) avoiding to declare struct Config explicitly for a clean implementation (i.e. the packager interface would be the same, receiving the merged nfpm.Info struct only). We can go either way if you @caarlos0 could comment on this.

@caarlos0
Copy link
Member

caarlos0 commented Apr 9, 2018

Right now the root package is unaware of its implementations... I think it should stay that way.

Maybe we can have something like:

type Config struct {
  Info
  Overrides map[string]Info `yaml:"overrides,omitempty"`
}

And use it like:

scripts:
  preinstall: "./scripts/preinstall.sh" # both deb and rpm
overrides:
  rpm:
    scripts:
      pretrans: "./scripts/pretrans.sh" # rpm explicit script

What do you think?

@caarlos0
Copy link
Member

caarlos0 commented Apr 9, 2018

Or maybe even only allow overriding scripts and not all fields (as I don't know if it makes much sense to override other fields)

@tympanix
Copy link
Contributor

tympanix commented Apr 9, 2018

There was talk (somewhere) about the dependencies having different names across deb/rpm

@tympanix
Copy link
Contributor

tympanix commented Apr 9, 2018

I like the idea of having the overrides field. If you don't think there is a problem with the wording being misleading (overrides can contain package exclusive settings, not just overrides) I think we should go for it 👍

@caarlos0
Copy link
Member

caarlos0 commented Apr 9, 2018

overrides can contain package exclusive settings, not just overrides

yeah, I couldn't think of a better name, I'm open to suggestions if you have any...

There was talk (somewhere) about the dependencies having different names across deb/rpm

hmm, havent seen that, also makes sense, maybe we can add that later?

@tympanix
Copy link
Contributor

tympanix commented Apr 9, 2018

Yes I will add the hole override implementation in a new PR later if that is okay. Some naming suggestions: package, extra, exclusive. I like package the best:

package:
  deb: ...
  rpm: ...

The main gist is that I want the nfpm package not to reflect packager specific configuration and let the packager deal with that itself in it's own package. I'll let you decide on the final naming scheme 😉

@caarlos0
Copy link
Member

caarlos0 commented Apr 9, 2018

I think the name depends:

Will we enabled overriding "generic" configs? if yes, I'll go with overrides
If not, I like package as well :)

@tympanix
Copy link
Contributor

tympanix commented Apr 9, 2018

Yes I think the user should be able to override the entire config if one wishes so, even if overriding everything makes no sense. Most of the time users will only use few overrides (which is in fact the intended purpose), but they are indeed limitless to override whichever field they like.

@caarlos0
Copy link
Member

caarlos0 commented Apr 9, 2018

I think we should probably limit what can and what cannot be overridden... IMHO, only things than can be package specific (like scripts and dependencies)...

@tympanix
Copy link
Contributor

Surething. We can probably achieve this with some kind of map:

var allowedOverrides map[string]bool

I will have a look at this tomorrow

ecmrauh added a commit to ecmrauh/nfpm that referenced this issue Jun 4, 2018
I noticed that the new feature of pre- und postinstall scripts isn't
reflected in the output of the example config file. This commit adds the
missing parts to make it clearer that this feature is available.

See goreleaser#19
caarlos0 pushed a commit that referenced this issue Jun 4, 2018
I noticed that the new feature of pre- und postinstall scripts isn't
reflected in the output of the example config file. This commit adds the
missing parts to make it clearer that this feature is available.

See #19
@caarlos0
Copy link
Member

This issue 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
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants