blocknotes / activeadmin_blaze_theme

A theme for Active Admin based on Blaze CSS 3.x
MIT License
46 stars 12 forks source link

WIP: Move dependencies gems into dev and prod group #12

Closed ngouy closed 3 years ago

ngouy commented 3 years ago

Adding gems directly in the gemfile instead of the gemspec have 2 impacts on a project that is using this gem:

blocknotes commented 3 years ago

Hey @ngouy. To put development and/or test gems in the Gemfile instead of the .gemspec is a common practice, as described in the Bundler's docs: https://bundler.io/rubygems.html

Many gems do the same, for example also devise: https://github.com/heartcombo/devise/blob/master/Gemfile

I suspect that you are including the gem in an uncommon way. You say "parent" project... are you using a copy of the gem source locally inside another project? 🤔

If you need to preserve the specific gems of a project I would go with bundle cache. While if you need a specific version of this gem you could just fork it and reference to it in the project's Gemfile.

ngouy commented 3 years ago

hey @blocknotes indeed I didn't knew

I still think there is an issue somewhere

I am including this current gem perfectly "normally" in my gemfile

# frozen_string_literal: true
source "https://rubygems.org"
git_source(:github) { |repo| "https://github.com/#{repo}.git" }

ruby "2.7.2"

group :default do
  #...
  gem "activeadmin"
  gem "activeadmin_blaze_theme"
  #...
end

Yet when I'm using it in my rails project I have all kind of issues

Here is another one : bin/rspec

The rspec executable in the rspec-core gem is being loaded, but it's also present in other gems (activeadmin_blaze_theme). If you meant to run the executable for another gem, make sure you use a project specific binstub (bundle binstub <gem_name>). If you plan to use multiple conflicting executables, generate binstubs for them and disambiguate their names.

ngouy commented 3 years ago

Maybe one solution would be to put those gems under a test/dev group

ngouy commented 3 years ago

Your gem shouldn't be based on pry rspec and such, we don't need that in production

All of the listed gems are used for specs

ngouy commented 3 years ago

if you prefer I can make a pr like

# frozen_string_literal: true

source 'https://rubygems.org'

gemspec

group :development do
  gem 'pry-rails'

  gem 'activestorage', '~> 6.0'
  gem 'capybara', '~> 3.33'
  gem 'puma', '~> 4.3'
  gem 'rspec-rails'
  gem 'rspec_junit_formatter', '~> 0.4'
  gem 'rubocop', '~> 1.0'
  gem 'sassc', '~> 2.4'
  gem 'selenium-webdriver', '~> 3.142'
  gem 'sprockets-rails', '~> 3.2'
  gem 'sqlite3', '~> 1.4'
end
blocknotes commented 3 years ago

Your gem shouldn't be based on pry rspec and such, we don't need that in production

All of the listed gems are used for specs

As said, the documentation is clear here: https://bundler.io/rubygems.html Those gems should not be installed with the standard installation workflow.

Also consider that:

If you can/want to share a sample project I could make a check.

blocknotes commented 3 years ago

if you prefer I can make a pr like ...

Sure, that's fine 👍 Better with group :development, :test

ngouy commented 3 years ago

@blocknotes TBH I am not sure how to test this properly on my machine, and if it will 💯 fix the problem but it can't do any harm anyway.

ngouy commented 3 years ago

@blocknotes jeez I'm sorry man I haven't read properly. Indeed there shouldn't be an issue even without the groups

Let me setup a fresh project to try reproduce it, maybe it's indeed a weird bug only on my side (even if I import the gem normally, maybe there is a aside effect somewhere else)

ngouy commented 3 years ago

@blocknotes here is a working example

I just added 3 gems, created one spec

https://github.com/ngouy/blaze-gem-exec-issue/blob/master/README.md

ngouy commented 3 years ago

fixed by https://github.com/blocknotes/activeadmin_blaze_theme/commit/9aa89b24e0400d372240464bbda6550743f442ee

git diff image

image

Thank @blocknotes

ngouy commented 3 years ago

@blocknotes sorry to bump it out, but there are still issue when using bundle exec <something> when the something is part of the things that are present under your /bin dir

It forced us to generate our own binaries for rails, rspec, rubocop and rake TBH I agree for some it make sense, but for rubocop for instance, I would like to run bundle exec rubocop without issues

I tried to look out for real-life example out here and I didn't find to manage any gems that expose their own binaries based on these common autogenerated bins

For example rubocop gem themselves have binaries, but none that is an autogenerated binstub rubocop.

image

image

What are your thoughts about it?

ngouy commented 3 years ago

Just found a counter exemple to it :

https://github.com/activeadmin/inherited_resources

Will continue to investigate

ngouy commented 3 years ago

find the issue

here is what I have in my file /Users/nathangouy/.rbenv/versions/2.7.2/lib/ruby/gems/2.7.0/bin/rubocop

#!/usr/bin/env ruby
#
# This file was generated by RubyGems.
#
# The application 'activeadmin_blaze_theme' is installed as part of a gem, and
# this file is here to facilitate running it.
#

require 'rubygems'

version = ">= 0.a"

str = ARGV.first
if str
  str = str.b[/\A_(.*)_\z/, 1]
  if str and Gem::Version.correct?(str)
    version = str
    ARGV.shift
  end
end

if Gem.respond_to?(:activate_bin_path)
load Gem.activate_bin_path('activeadmin_blaze_theme', 'rubocop', version)
else
gem "activeadmin_blaze_theme", version
load Gem.bin_path("activeadmin_blaze_theme", "rubocop", version)
end

Will try to figure out how to fix this. My guess is that the recent update on the gemspec file prevent this from happening

edit : Even my /usr/local/bin/rubocop is corrupted

I will just remove rbenv and bins all together and reinstall them ...

ngouy commented 3 years ago

works after full rbenv reset