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 a self-extracting sh package type implementation #651

Merged
merged 4 commits into from Mar 26, 2014

Conversation

Projects
None yet
3 participants
@gerbercj
Contributor

gerbercj commented Feb 27, 2014

@jordansissel:
This PR contains an implementation of a sh package. We have been developing this format over the last 7 months and are using it internally. The files are all self contained, so I feel that it should be able to be merged back in at this point. Let me know if you have any questions.

cc: @jjrussell

@jjrussell

This comment has been minimized.

jjrussell commented Feb 28, 2014

Woohoo!

@gerbercj

This comment has been minimized.

Contributor

gerbercj commented Mar 5, 2014

@jordansissel, just wanted to check if you had any questions or concerns.

@jordansissel

This comment has been minimized.

Owner

jordansissel commented Mar 7, 2014

Been swamped with life/work lately havent' had a chance to review! Sorry about this :)

@jordansissel

This comment has been minimized.

Owner

jordansissel commented Mar 7, 2014

When I try this, I get the following error:

% bin/fpm -s dir -t sh -n fancy $HOME/.zshrc=/tmp/example
/home/jls/projects/fpm/templates/sh.erb:134:in `block in install_script': undefined local variable or method `release_id' for #<FPM::Package::Sh:0x000000016418f0> (NameError)
...

I can't see where release_id is defined?

@gerbercj

This comment has been minimized.

Contributor

gerbercj commented Mar 7, 2014

Thanks for taking some time to take a look at this. Took a second look. Our standard use case is to include --template-value release_id=#{File.basename(package_file_name, ".*")}. I'll tweak it up to handle the general case more gracefully.

@jordansissel

This comment has been minimized.

Owner

jordansissel commented Mar 7, 2014

ooh I see, this makes sense. We can have the package provide a reasonable default for 'release_id' Perhaps just the to_s value of the package?

@jordansissel

This comment has been minimized.

Owner

jordansissel commented Mar 7, 2014

Specifically, using 'packagename-version-iteration' as the release_id ?

@gerbercj

This comment has been minimized.

Contributor

gerbercj commented Mar 11, 2014

I have made a couple of changes:

  1. I now only set RELEASE_ID if it is passed in by Ruby. If it is not set, it defaults to the timestamp, like it was trying to do before. I'm still open to alternatives here, as we aren't using this code path for our purposes.
  2. I have fixed the variable handling for FORCE, and then made UNPACK_ONLY consistent with this new approach. This came out of an issue that we saw at work today.
  3. I have added the output of the of AFTER_INSTALL command to the log.
@jordansissel

This comment has been minimized.

Owner

jordansissel commented Mar 11, 2014

Woo! I can use this now. I'll play around with it and get you some feedback soon

@gerbercj

This comment has been minimized.

Contributor

gerbercj commented Mar 11, 2014

Awesome sauce. I'll be sorting out one more bug that we're seeing today and hope to add the commit soon.

@gerbercj

This comment has been minimized.

Contributor

gerbercj commented Mar 19, 2014

OK... I think that we're good now. We were creating PID files in the install directory. This last patch moves them up a level.

@jordansissel

This comment has been minimized.

Owner

jordansissel commented Mar 26, 2014

Playing with this now! :)

@jordansissel

This comment has been minimized.

Owner

jordansissel commented Mar 26, 2014

The default install path seems to be /, I think we can change this or require -i being passed as the flag?

% ./logstash.sh
mkdir: cannot create directory ‘/20140325173925’: Permission denied
Error: Unable to create install directory /20140325173925 : 

I also think that the "release" should be the package version+iteration value, not the time of installation

@gerbercj

This comment has been minimized.

Contributor

gerbercj commented Mar 26, 2014

I think that it makes sense to have a more sane default when -i is not provided. Version+iteration seems like a logical choice. Is there a logical base path? Maybe /opt/apps?

@jordansissel

This comment has been minimized.

Owner

jordansissel commented Mar 26, 2014

My intent is to merge this and make a few modifications, but I don't want to mess up how you are already using this, so let me know on the following things:

  • Make fpm's --prefix set the default for the shell script -i
  • without any --prefix the default will be /opt/<name>.
  • Make the default "release id" version-iteration (unless no iteration is set, then just version)
  • Add support to remove/uninstall. We can use 'tar -t' to list the files we should remove ;)

Thoughts?

jordansissel added a commit that referenced this pull request Mar 26, 2014

Merge pull request #651 from gerbercj/feature/sh_package
Add a self-extracting sh package type implementation

@jordansissel jordansissel merged commit 77ebc02 into jordansissel:master Mar 26, 2014

@jordansissel

This comment has been minimized.

Owner

jordansissel commented Mar 26, 2014

In your use, do you rely on DATESTAMP at install time, or do you use release_id?

@jordansissel

This comment has been minimized.

Owner

jordansissel commented Mar 26, 2014

Do you rely on the current setting of DEFAULT_DIR (basename $0) ?

@gerbercj

This comment has been minimized.

Contributor

gerbercj commented Mar 27, 2014

All of the proposed changes make sense to me. Our actual use case is below for reference.

      def fpm_command
        command = ['fpm']
        command << '--verbose'
        command << "--package #{package_file_name}"
        command << '--maintainer=dev@tapjoy.com'
        command << "-C #{project_root}"
        command << '-s dir'
        command << '-t sh'
        command << "-n #{project_name}"
        command << "-v #{date_stamp}"
        command << '--template-scripts'
        command << "--template-value release_id=#{release_id}"
        command << "--after-install #{post_install_script_path}"
        unless options[:'with-git']
          command << "--exclude '.git'"
          command << "--exclude '.git/**'"
        end
        command << "--exclude '*.slug'"
        command << "--exclude 'log/**'"
        command << "--exclude 'tmp/**'"
        command << "--exclude 'vendor/bundle/ruby/1.9.1/cache/*'"
        command << "." # package all the things
        command.join(' ')
      end

prof-milki pushed a commit to prof-milki/xpm that referenced this pull request Dec 18, 2014

jls
Merge pull request jordansissel#651 from gerbercj/feature/sh_package
Add a self-extracting sh package type implementation

prof-milki pushed a commit to prof-milki/xpm that referenced this pull request Dec 27, 2014

Merge pull request jordansissel#651 from gerbercj/feature/sh_package
Add a self-extracting sh package type implementation

jordansissel added a commit that referenced this pull request Apr 24, 2015

Merge pull request #651 from gerbercj/feature/sh_package
Add a self-extracting sh package type implementation

jordansissel added a commit that referenced this pull request Jun 20, 2016

Merge pull request #651 from gerbercj/feature/sh_package
Add a self-extracting sh package type implementation
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment