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

chef_gem and gem metadata don't play well #4710

Closed
thommay opened this issue Mar 15, 2016 · 10 comments · Fixed by #4809
Closed

chef_gem and gem metadata don't play well #4710

thommay opened this issue Mar 15, 2016 · 10 comments · Fixed by #4809
Assignees
Labels
Priority: Medium Type: Bug Does not work as expected.

Comments

@thommay
Copy link
Contributor

thommay commented Mar 15, 2016

Description

If one has a gem section in metadata and a chef_gem resource, the resource implodes. chef_gem needs to bundle bust.

Chef Version

12.8.x

Client Output

https://gist.github.com/thommay/1151fc72d256edd57c5e

@thommay thommay added this to the Accepted Minor milestone Mar 16, 2016
@lamont-granquist
Copy link
Contributor

So to be clear here (i think):

If any cookbook has a gem metadata section, then every cookbook has broken chef_gem resources.

Because this is a largely non-issue if its the same cookbook since if you're using the gem metadata you shouldn't be using the chef_gem resource.

This might be a workaround if you control both cookbooks:

Bundler.with_clean_env do
  chef_gem "aws-sdk" do
    compile_time true
  end
end

Obviously not an acceptable workaround since that means updating every community cookbook that does a chef_gem with that workaround which would be awful, so its not even a suggestion to do that. If that works, though, then that suggests the shape of the ultimate patch to chef_gem.

Don't we also have issues with gem_package as well? Or does the busting that gem_package does to install in the system ruby have the side effect of working around this as well?

@lamont-granquist
Copy link
Contributor

gem_package seems to work on pure-ruby gems but seems to be broken on native gem installs (e.g. nokogiri, ffi)

@lamont-granquist
Copy link
Contributor

So this is kind of the tip of the iceberg here.

chef_gem 'mysql'

require 'mysql'

Even if we beat on the gem provider so that the chef_gem succeeds the require line will blow up because mysql isn't in the bundle.

@lamont-granquist
Copy link
Contributor

Wrapping the bundle install we do with Bundler.with_clean_env still doesn't fix this. The problem is that we're doing a bundle install and then later need to break out of the bundle that we create. The Bundler.with_clean_env call is designed to wrap a subprocess so it cleans up the ENV -- but it expects you to fork and exec. There's no similar API call I can find to reset the internal state of Bundler so that a require for a gem that is outside of your bundle will succeed.

I'm thinking we need to fork off a subprocess to do the bundle install

@coderanger
Copy link
Contributor

@lamont-granquist Hmm, how would that work with the auto-Bundle.require bit? Maybe we just can't do that safely and the cookbook should have a stubby libraries/default.rb to require?

@lamont-granquist
Copy link
Contributor

Good catch, yes that'd break require in the pseudo-gemfile entirely

@coderanger
Copy link
Contributor

It wouldn't be the end of the world, but just something to note :)

@lamont-granquist
Copy link
Contributor

Yeah, another problem with fork() is just making sure to fix it on windows as well.

It would make the feature usable, though, right now I'd say we can't use this feature. It'd be nice to ship something in 12.9.x that was usable at some base level.

I'll probably poke around in the internals of Bundler some more and then just go with the fork() implementation as a quick fix if I can't find anything better.

@lamont-granquist
Copy link
Contributor

added Gem.clear_paths and went to these lengths and it still fails and i can't track down the global state that is trolling 'require':

        unless cookbook_gems.empty?
          begin
            # save the env ourselves just to be sure
            saved_env = ENV
            # use Bundler's API to reset the env
            Bundler.with_clean_env do
              inline_gemfile do
                source Chef::Config[:rubygems_url]
                cookbook_gems.each do |args|
                  gem(*args)
                end
              end
            end
            ENV.replace(saved_env)
          rescue Exception => e
            events.cookbook_gem_failed(e)
            raise
          end
        end

        # nuke all the state in Bundler from orbit
        Bundler.instance_variables.each do |ivar|
          Bundler.remove_instance_variable(ivar)
        end

        # reset rubygems
        Gem.clear_paths

@coderanger
Copy link
Contributor

We could possibly do the install in a subproc (either fork or spawn a dedicated helper) and then the Bundle.require in the main proc? I don't know if that would break things the same way.

@thommay thommay added Type: Bug Does not work as expected. Priority: Medium and removed Bug labels Jan 25, 2017
@chef chef locked and limited conversation to collaborators Nov 16, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Priority: Medium Type: Bug Does not work as expected.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants