Skip to content
This repository has been archived by the owner on Apr 14, 2021. It is now read-only.

bundle lock does not activate gems for current platform if it was activated under a different platform for a separate dependency #4896

Closed
mwrock opened this issue Aug 19, 2016 · 8 comments

Comments

@mwrock
Copy link
Contributor

mwrock commented Aug 19, 2016

As part of our build process we bundle lock against ruby and then again against x86-mingw32 and commit a unified lock file with aggregate gem requirements. This works well for us in most cases. However there is a specific edge case where this breaks.

Assume this minimal Gemfile:

source "https://rubygems.org"

gem "mixlib-shellout"
gem "gssapi"

Now if I bundle lock separately on linux and windows, each platform generates an acceptable lockfile. However, if I have a windows generated lock file that looks like this:

GEM
  remote: https://rubygems.org/
  specs:
    ffi (1.9.14-x86-mingw32)
    gssapi (1.2.0)
      ffi (>= 1.0.1)
    mixlib-shellout (2.2.6-universal-mingw32)
      win32-process (~> 0.8.2)
      wmi-lite (~> 1.0)
    win32-process (0.8.3)
      ffi (>= 1.0.0)
    wmi-lite (1.0.0)

PLATFORMS
  x86-mingw32

DEPENDENCIES
  gssapi
  mixlib-shellout

BUNDLED WITH
   1.12.5

Then I run bundle lock on linux, I get:

GEM
  remote: https://rubygems.org/
  specs:
    ffi (1.9.14-x86-mingw32)
    gssapi (1.2.0)
      ffi (>= 1.0.1)
    mixlib-shellout (2.2.6)
    mixlib-shellout (2.2.6-universal-mingw32)
      win32-process (~> 0.8.2)
      wmi-lite (~> 1.0)
    win32-process (0.8.3)
      ffi (>= 1.0.0)
    wmi-lite (1.0.0)

PLATFORMS
  ruby
  x86-mingw32

DEPENDENCIES
  gssapi
  mixlib-shellout

BUNDLED WITH
   1.12.5

The key error here is the ommision of ffi (1.9.14). I'm left with only the windows variant but gssapi should be activating one for the ruby platform.

First, I realize there is work going into 1.13 to improve platform related issues. I have tried this scenario with 1.13.0.RC.1 and this problem is still present. Second I have looked through bundler source and "think" I understand the issue and have a fix that remedies the problem. However as I'll discuss shortly, its probably not the right fix.

So this is definitely an edge case and with different requirements I get expected results. For example, if my Gemfile only had gssapi and did not include mixlib-shellout, I'd end up with both ffi platforms. Here is why the a Gemfile with mixlib-shellout (and I'm sure there are others) produces unexpected results. mixlib-shellout has two separate gemspecs - a second windows specific one. Only its windows gemspec takes a dependency on win32-process which in turn takes a dependency on ffi. So the resolver resolves mixlib-shellout and rightly so activates no ffi on linux but does activate it for windows given a lock file with a mingw dependency. Later when it resolves gssapi on linux, the resolver produces a SpecGroup for ffi with an activated ruby platform. However, when the Resolution attempts to activate the possibility:

      # Attempts to activate the current {#possibility}
      # @return [void]
      def attempt_to_activate
        debug(depth) { 'Attempting to activate ' + possibility.to_s }
        existing_node = activated.vertex_named(name)
          if existing_node.payload
          debug(depth) { "Found existing spec (#{existing_node.payload})" }
          attempt_to_activate_existing_spec(existing_node)
        else
          attempt_to_activate_new_spec
        end
      end

Here existing_node points to the previously activated windows dependency and the ruby possibility is thrown away.

Changing:

if existing_node.payload

to:

if existing_node.payload && (possibility.activated - existing_node.payload.activated).empty?

fixes this and ensures if the current possibility contains active platforms not included in the existing possibiity, it will be activated. This produces the ffi (>= 1.0.1) requirement previously missing and prevents corrupted Gemfile.lock errors on linux.

Now I think this is possibly not the "right" change since it is in the vendored Molinillo library. I'm honestly not familiar enough with these code bases to know if I should submit a PR and tests to that repo or if something should be changed in the way bundler consumes it. Like maybe keying off of something other than name (if thats even controlled on the bundler side).

Would really appretiate thoughts on this. Thanks!!

@segiddins
Copy link
Member

Have you tried this against #4836 ? I believe it should fix the issue. Also, you might want to look into using bundle lock --add-platform to add the platform instead of moving between different machines

@mwrock
Copy link
Contributor Author

mwrock commented Aug 19, 2016

I'll try #4836 I did try bundle lock --add-platform but got the same undesirable results.

@segiddins
Copy link
Member

That's expected, since I think #4836 fixes it by introducing the materialize_for_all_platforms method

@segiddins
Copy link
Member

Ugh just confirmed the bug still exists, I'll see if I can dig into why

@mwrock
Copy link
Contributor Author

mwrock commented Aug 19, 2016

Oh that does fix the issue. Very awesome! I didn't use --add-platform since just a vanilla `bundle lock worked. I can certainly wait but just curious if there is a ball park estimate when that might ship. Thanks!

@segiddins
Copy link
Member

I have a fix, will add it to the existing PR which ought to ship in 1.14

@mwrock
Copy link
Contributor Author

mwrock commented Aug 19, 2016

The --add-platform will be very nice by the way and help us avoid current tom foolery hacking with Gem.platforms in order to lock to a platform different from the current environment.

@segiddins
Copy link
Member

In that case, you might also find #4895 interesting!

homu added a commit that referenced this issue Aug 21, 2016
[Resolver] Ensure all platforms are activated for the activated spec group

Closes #4896
homu added a commit that referenced this issue Aug 23, 2016
[Resolver] Ensure all platforms are activated for the activated spec group

Closes #4896
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants