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 Debian script header #231

Merged
merged 1 commit into from
Apr 21, 2014
Merged

Conversation

dax
Copy link

@dax dax commented Apr 17, 2014

Without the shell header, Debian package installation fails with Exec format error

@lightbend-cla-validator

Hi @dax,

Thank you for your contribution! We really value the time you've taken to put this together.

Before we proceed with reviewing this pull request, please sign the Typesafe Contributors License Agreement:

http://typesafe.com/contribute/cla

@muuki88
Copy link
Contributor

muuki88 commented Apr 17, 2014

Thanks :) For some of the scripts the header will be redundant, but that's okay has #!/bin/sh will be treated as a comment otherwise, right? On which system did you get the error and test the fix?

@dax
Copy link
Author

dax commented Apr 17, 2014

I got the error on a Debian squeeze.

@kardapoltsev
Copy link
Member

May be we should add something like postrm_header and put #!/bin/sh into it? Less code duplication and nobody will forget to add this to any new template.

@muuki88
Copy link
Contributor

muuki88 commented Apr 17, 2014

This would be even better

@dax
Copy link
Author

dax commented Apr 17, 2014

Ok, I will do it.

@kardapoltsev
Copy link
Member

@dax would you reimplement your PR? If not then I will fix. And thanks for your effort anyway!

@dax
Copy link
Author

dax commented Apr 17, 2014

I have updated my PR. Tell me if this is what you expected.

@kardapoltsev
Copy link
Member

LGTM except one small note.

val userGroupAdd = headerScript ++ Seq(
  TemplateWriter.generateScript(DebianPlugin.postinstGroupaddTemplateSource, replacements),
  TemplateWriter.generateScript(DebianPlugin.postinstUseraddTemplateSource, replacements))

May be it'll be better to handle postinst similar to other scripts like

prependAndFixPerms(postinst, headerScript, LinuxFileMetaData())

@lightbend-cla-validator

Hi @dax,

Thank you for your contribution! We really value the time you've taken to put this together.

Before we proceed with reviewing this pull request, please sign the Typesafe Contributors License Agreement:

http://typesafe.com/contribute/cla

@dax
Copy link
Author

dax commented Apr 18, 2014

Ok, I fixed it.

@muuki88
Copy link
Contributor

muuki88 commented Apr 18, 2014

LGTM. Works for Ubuntu 13.04. @kardapoltsev did you test it, too? If so, you can merge this.

kardapoltsev added a commit that referenced this pull request Apr 21, 2014
@kardapoltsev kardapoltsev merged commit ac499bd into sbt:master Apr 21, 2014
@kardapoltsev
Copy link
Member

@dax, thanks!

@kardapoltsev
Copy link
Member

And I've added tests for scripts header

@muuki88 muuki88 added the debian label Apr 21, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants