fog / fog-libvirt

libvirt provider for fog
MIT License
16 stars 44 forks source link

Fix use latest fog-core/fog-json with Gemfile-edge #50

Closed electrofelix closed 5 years ago

electrofelix commented 6 years ago

Newer releases of fog-json require newer fog-core, pin the release back to use a version that depends on the same releases of fog-core.

plribeiro3000 commented 6 years ago

Is there a problem with the latest releases of fog-core and fog-json?

electrofelix commented 6 years ago

I need to dig a bit more, but it appears that the latest fog-json requires fog-core >= 2.0.0. As I'm unsure as to whether that is ok with fog-libvirt, I figured I'd first help pin them together and then whether fog-libvirt can move forward to a newer fog-core can be looked at separately?

Need to look closer at what is happening with the Travis builds though as my initial naive attempt didn't work as expected, so trying to reproduce the build steps locally.

electrofelix commented 6 years ago

@plribeiro3000 so I misunderstood what was happening, seems that the gemspec file ends up always replacing the Gemfile-edge specifiers for fog-core/fog-json resulting in always pinning to fog-core releases rather than taking the latest git repo contents.

So far the only way I can see to solve this is from the gemspec file to skip setting them if using the Gemfile-edge, as I can't see any way to inspect whether the dependencies are already declared. Maybe there is a nicer way?

electrofelix commented 6 years ago

Only other alternative I can think of is to use 'env' in travis, drop the Gemfile-edge and specify use of released or git source for the fog-core/fog-json deps always from the gemspec, or similarly move the two dependencies to only be managed by the Gemfiles.

plribeiro3000 commented 6 years ago

I'm not sure i follow your issue here. This Gemfile should take care of this.

And then this .travis.yml:

https://github.com/fog/fog-libvirt/blob/a1046378bb1abc42bab10857520d683585117392/.travis.yml#L12-L26

should take care of the problem.

We should also include 2.3 and 2.4 in the build.

electrofelix commented 6 years ago

@plribeiro3000 unfortunately the line https://github.com/fog/fog-libvirt/blob/master/gemfiles/Gemfile-edge#L14 pulls in the gemspec which results in use of the release versions only along with the version pins as set https://github.com/fog/fog-libvirt/blob/master/fog-libvirt.gemspec#L28-L29.

Please take a look at the job results from https://travis-ci.org/fog/fog-libvirt/jobs/348628246 Both of those using the Gemfile-edge fail with the following message:

  In Gemfile-edge:
    fog-core
    fog-libvirt was resolved to 0.4.2, which depends on
      fog-json was resolved to 1.1.0, which depends on
        fog-core (~> 2.0)
    fog-libvirt was resolved to 0.4.2, which depends on
      fog-core (>= 1.27.4, ~> 1.27)

As there is no pin in Gemfile-edge, it appears it is still picking up the pin listed set at https://github.com/fog/fog-libvirt/blob/master/fog-libvirt.gemspec#L28-L29 .

What I think actually happens is that bundle gets two dependency entries for fog-core and fog-json, one based on the released package versions and one git source because they are in separate sources (Gemfile vs gemspec). The result is that with fog-json latest release of 1.1.0 requiring a newer version of fog-core, when parsing the deps from the gemspec file it hits the restriction of (>= 1.27.4, ~> 1.27) and exits. Setting a restriction in fog-libvirt.gemspec for the versions of fog-json doesn't work, so it appears the gemspec dependencies are additive rather replacing.

I've been able to reproduce this locally, and I can confirm that essentially CI for this project is now blocked due to the recent release of fog-json which now requires fog-core 2.0.

The only solution I've come across so far is to ensure the dependency only appears from one source depending on the need. So if using Gemfile-edge, then the dependencies for fog-core/fog-json in fog-libvirt.gemspec needs to be ignored.

By comparison the run for this latest patch, all jobs succeed: https://travis-ci.org/fog/fog-libvirt/builds/349231538?utm_source=github_status&utm_medium=notification

So: https://travis-ci.org/fog/fog-libvirt/builds/348628242 for #29 fails on just the Gemfile-edge https://travis-ci.org/fog/fog-libvirt/builds/348630400 initial naive attempt with this PR #50 failed with the same error https://travis-ci.org/fog/fog-libvirt/builds/349231538 with latest update of this patch works for Gemfile-edge runs

There are a few ways to solve, this way, drop the Gemfile-edge and use an env variable in the gemspec file to use fog-core/fog-json from either git or rubygems.org depending on value, or move fog-core/fog-json depencies out from gemspec file into the Gemfile so that they are only managed by Gemfile and gemfiles/Gemfile-edge.

Maybe there is a alternative nicer way as well?

electrofelix commented 6 years ago

@plribeiro3000 have you had a chance to look at this further? I believe the tests for this project using the Gemfile-edge will not pass until some fix is landed.

plribeiro3000 commented 6 years ago

@electrofelix Sorry for the delay. I did not had time to put my hands on code yet, just mainly review some stuff here and there. I do plan to check it out this weekend. I will let you known how it goes.

Again, sorry for the delay here.

plribeiro3000 commented 6 years ago

Hey @electrofelix . Sorry for the huge delay!

I believe you are right about the dependency. We might be able to solve this by switching the require of the gemspec to the top of the Gemfile.

So Gemfile.edge would be:

source "https://rubygems.org"

gemspec :path => "../"

# Shared components
gem "fog-core", :github => "fog/fog-core"
gem "fog-json", :github => "fog/fog-json"

group :development, :test do
  # This is here because gemspec doesn"t support require: false
  gem "coveralls", :require => false
  gem "netrc", :require => false
  gem "octokit", :require => false
end
electrofelix commented 6 years ago

@plribeiro3000 unfortunately I tried that approach and it outputted the following:

The git source `git://github.com/fog/fog-core.git` uses the `git` protocol, which transmits data without encryption. Disable this warning with `bundle config git.allow_insecure true`, or switch to the `https` protocol to keep your data secure.
The git source `git://github.com/fog/fog-json.git` uses the `git` protocol, which transmits data without encryption. Disable this warning with `bundle config git.allow_insecure true`, or switch to the `https` protocol to keep your data secure.
Warning: the running version of Bundler (1.13.6) is older than the version that created the lockfile (1.16.1). We suggest you upgrade to the latest version of Bundler by running `gem install bundler`.
Fetching git://github.com/fog/fog-core.git
Starting gitproxy script
socat STDIO PROXY:web-proxy.emea.hpqcorp.net:github.com:9418,proxyport=8080
Fetching git://github.com/fog/fog-json.git
Starting gitproxy script
socat STDIO PROXY:web-proxy.emea.hpqcorp.net:github.com:9418,proxyport=8080
Fetching gem metadata from https://rubygems.org/........
Fetching version metadata from https://rubygems.org/..
Fetching dependency metadata from https://rubygems.org/.
Resolving dependencies...
Bundler could not find compatible versions for gem "fog-core":
  In Gemfile:
    fog-libvirt was resolved to 0.4.2, which depends on
      fog-core (>= 1.27.4, ~> 1.27)

    fog-libvirt was resolved to 0.4.2, which depends on
      fog-xml (~> 0.1.1) was resolved to 0.1.3, which depends on
        fog-core

    fog-libvirt was resolved to 0.4.2, which depends on
      fog-xml (~> 0.1.1) was resolved to 0.1.3, which depends on
        fog-core

Could not find gem 'fog-core (>= 1.27.4, ~> 1.27)', which is required by gem 'fog-libvirt', in any of the sources.

For some reason the two different declarations do not replace one another, but appear to act in a cumulative manner, which is super annoying.

electrofelix commented 6 years ago

Seems I was using an older bundler, updated that and the error is now back to the following:

The git source `git://github.com/fog/fog-core.git` uses the `git` protocol, which transmits data without encryption. Disable this warning with `bundle config git.allow_insecure true`, or switch to the `https` protocol to keep your data secure.
The git source `git://github.com/fog/fog-json.git` uses the `git` protocol, which transmits data without encryption. Disable this warning with `bundle config git.allow_insecure true`, or switch to the `https` protocol to keep your data secure.
Fetching git://github.com/fog/fog-core.git
Starting gitproxy script
socat STDIO PROXY:web-proxy.emea.hpqcorp.net:github.com:9418,proxyport=8080
Fetching git://github.com/fog/fog-json.git
Starting gitproxy script
socat STDIO PROXY:web-proxy.emea.hpqcorp.net:github.com:9418,proxyport=8080
Fetching gem metadata from https://rubygems.org/........
Resolving dependencies...
Bundler could not find compatible versions for gem "fog-core":
  In Gemfile-edge:
    fog-core

    fog-libvirt was resolved to 0.4.2, which depends on
      fog-core (>= 1.27.4, ~> 1.27)

Could not find gem 'fog-core (>= 1.27.4, ~> 1.27)', which is required by gem 'fog-libvirt', in any of the relevant sources:
  git://github.com/fog/fog-core.git (at master@d7ff1c1)
plribeiro3000 commented 6 years ago

@electrofelix Lets just drop the Gemfile-edge then.

I don't think it worth the trouble.

electrofelix commented 6 years ago

Before we do, I've created some alternatives just to see if you prefer any of the other approaches

https://github.com/fog/fog-libvirt/compare/master...electrofelix:pin-fog-json-with-fog-core-alternative1 maintains fog-core/fog-json dependencies in the two Gemfiles so that they are managed at the equivalent location and this results in bundler resolving them separately and correctly.

or:

https://github.com/fog/fog-libvirt/compare/master...electrofelix:pin-fog-json-with-fog-core-alternative2 moves management of both pinned and git releases of fog-core/fog-json into the same common Gemfile and places the logic there to determine which to use, dropping the Gemfile-edge and updating .travis.yml to set the desired variable when looking to test the latest.

If you think either of these aren't worth pursuing then I'll update this PR to simply delete testing against the latest fog-core/fog-json releases.

plribeiro3000 commented 6 years ago

Hmmm. Both work but i believe both have drawbacks that make them not worth.

In the first one the drawback is that someone that is not in this discussion might miss this change when looking for the actual dependencies. In the second we are safe guarded from the first drawback. But i still feel that we might not need it.

If we look at it, both fog-core and fog-json are stable enough that we almost don't do new releases. And if we find a bug, we certainly will release it right away. So both options seems unnecessary. What do you think?

strzibny commented 5 years ago

I agree with @plribeiro3000 that given the stability of fog-core and our direction of removing the upper dependency requirement this is indeed not necessary.

I took the liberty of removing Gemfile-edge in #64.

Thank you @electrofelix for the patch and your hard work on exploring possible options (which we might take in future if necessary).