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

error occurred when I modify source code (padrino is running) #1662

Open
ssut opened this Issue Apr 23, 2014 · 16 comments

Comments

Projects
None yet
6 participants
@ssut

ssut commented Apr 23, 2014

  ERROR -  ActiveSupport::Concern::MultipleIncludedBlocks - Cannot define multiple 'included' blocks for a Concern:
 /home/ssut/.rbenv/versions/2.1.1/lib/ruby/gems/2.1.0/gems/activesupport-4.1.0/lib/active_support/concern.rb:126:in `included'
  ERROR -  Failed to load /home/ssut/.rbenv/versions/2.1.1/lib/ruby/gems/2.1.0/gems/activesupport-4.1.0/lib/active_support/logger_silence.rb; removing partially defined constants
  ERROR -  ActiveSupport::Concern::MultipleIncludedBlocks - Cannot define multiple 'included' blocks for a Concern:
 /home/ssut/.rbenv/versions/2.1.1/lib/ruby/gems/2.1.0/gems/activesupport-4.1.0/lib/active_support/concern.rb:126:in `included'

An error occurred when I modify source code. *prerequisite: padrino is running and reload option is on.

@nesquena nesquena added this to the 0.12.2 milestone Apr 28, 2014

@namusyaka

This comment has been minimized.

Show comment
Hide comment
@namusyaka

namusyaka May 5, 2014

Member

@ssut Thanks for the report! However, I couldn't reproduce.
Please, provide a failing project.

Member

namusyaka commented May 5, 2014

@ssut Thanks for the report! However, I couldn't reproduce.
Please, provide a failing project.

@ssut

This comment has been minimized.

Show comment
Hide comment
@ssut

ssut May 6, 2014

(It's just a guess)
I need active_support/time module, and I was required to 'active_support/time' in controllers.
So, I modified controller and padrino will reload changes(file).
The problem is occured here. It seems the problem was in this case, active_support is necessary to require only once.

ssut commented May 6, 2014

(It's just a guess)
I need active_support/time module, and I was required to 'active_support/time' in controllers.
So, I modified controller and padrino will reload changes(file).
The problem is occured here. It seems the problem was in this case, active_support is necessary to require only once.

@namusyaka

This comment has been minimized.

Show comment
Hide comment
@namusyaka

namusyaka May 6, 2014

Member

Thanks, confirmed.
We will investigate the issue.

Member

namusyaka commented May 6, 2014

Thanks, confirmed.
We will investigate the issue.

@namusyaka namusyaka added the reloader label May 6, 2014

@namusyaka

This comment has been minimized.

Show comment
Hide comment
@namusyaka

namusyaka May 6, 2014

Member

I've digged in the issue.
@ssut quick fix:

require 'active_support/time' unless defined?(ActiveSupport::Duration)

@ujifgc @padrino/core-members
Padrino::Reloader.safe_load fails because ActiveSupport prohibits including the multiple block.
https://github.com/rails/rails/blob/v4.1.0/activesupport/lib/active_support/logger_silence.rb#L6-L9
https://github.com/rails/rails/blob/v4.1.0/activesupport/lib/active_support/concern.rb#L126

Perhaps, we should provide a method that can avoid loading specific external library.
What do you think?

Member

namusyaka commented May 6, 2014

I've digged in the issue.
@ssut quick fix:

require 'active_support/time' unless defined?(ActiveSupport::Duration)

@ujifgc @padrino/core-members
Padrino::Reloader.safe_load fails because ActiveSupport prohibits including the multiple block.
https://github.com/rails/rails/blob/v4.1.0/activesupport/lib/active_support/logger_silence.rb#L6-L9
https://github.com/rails/rails/blob/v4.1.0/activesupport/lib/active_support/concern.rb#L126

Perhaps, we should provide a method that can avoid loading specific external library.
What do you think?

@ujifgc

This comment has been minimized.

Show comment
Hide comment
@ujifgc

ujifgc May 6, 2014

Member

Does the app fail every time the source is changed or only when require 'active_support/time' gets added?

Member

ujifgc commented May 6, 2014

Does the app fail every time the source is changed or only when require 'active_support/time' gets added?

@namusyaka

This comment has been minimized.

Show comment
Hide comment
@namusyaka

namusyaka May 6, 2014

Member

Yes, this error occurs everytime the source is changed.

Member

namusyaka commented May 6, 2014

Yes, this error occurs everytime the source is changed.

@ujifgc ujifgc closed this in 8e23d81 May 7, 2014

@ujifgc

This comment has been minimized.

Show comment
Hide comment
@ujifgc

ujifgc May 7, 2014

Member

This commit 8e23d81 can possibly fix #1583 earlier marked as [wontfix].

Member

ujifgc commented May 7, 2014

This commit 8e23d81 can possibly fix #1583 earlier marked as [wontfix].

@namusyaka

This comment has been minimized.

Show comment
Hide comment
@namusyaka

namusyaka May 7, 2014

Member

Great!
I had assumed that external library was target of the reloader.

Member

namusyaka commented May 7, 2014

Great!
I had assumed that external library was target of the reloader.

@ssut

This comment has been minimized.

Show comment
Hide comment
@ssut

ssut May 8, 2014

Thanks for fix this issue! :)

ssut commented May 8, 2014

Thanks for fix this issue! :)

@ujifgc ujifgc added the bug label May 8, 2014

@senekis

This comment has been minimized.

Show comment
Hide comment
@senekis

senekis May 10, 2016

I'm getting the same error but I'm not using an external library.

I have a module that extend ActiveSupport::Concern and is included into a model. I created a dummy project here: https://github.com/senekis/dashboard to reproduce the error.

Steps to reproduce:

  1. Starts the padrino application
  2. Open / in a browser
  3. Modify the file: user_mixins/filters.rb
  4. Reload / in the browser

You'll see the error ActiveSupport::Concern::MultipleIncludedBlocks - Cannot define multiple 'included' blocks for a Concern:

I appreciate your help

senekis commented May 10, 2016

I'm getting the same error but I'm not using an external library.

I have a module that extend ActiveSupport::Concern and is included into a model. I created a dummy project here: https://github.com/senekis/dashboard to reproduce the error.

Steps to reproduce:

  1. Starts the padrino application
  2. Open / in a browser
  3. Modify the file: user_mixins/filters.rb
  4. Reload / in the browser

You'll see the error ActiveSupport::Concern::MultipleIncludedBlocks - Cannot define multiple 'included' blocks for a Concern:

I appreciate your help

@ujifgc ujifgc reopened this May 11, 2016

@ujifgc ujifgc self-assigned this May 11, 2016

@ujifgc ujifgc closed this in d25298c May 11, 2016

@ujifgc

This comment has been minimized.

Show comment
Hide comment
@ujifgc

ujifgc May 11, 2016

Member

Should be fixed on master by d25298c

Member

ujifgc commented May 11, 2016

Should be fixed on master by d25298c

@ujifgc ujifgc reopened this May 11, 2016

@ujifgc

This comment has been minimized.

Show comment
Hide comment
@ujifgc

ujifgc May 11, 2016

Member

The fix broke reloading apps. Reverted it by 579944b.

Temporary solution: don't use ActiveSupport::Concern and other techniques dependent on mutating module instance variables.

Member

ujifgc commented May 11, 2016

The fix broke reloading apps. Reverted it by 579944b.

Temporary solution: don't use ActiveSupport::Concern and other techniques dependent on mutating module instance variables.

@ujifgc ujifgc modified the milestones: 0.14.0, 0.12.2 Jul 6, 2016

@u007

This comment has been minimized.

Show comment
Hide comment
@u007

u007 Jul 22, 2016

👍 faced the same issue,
i guess since production no reload, it should work, but on development, we need to restart on concern change

u007 commented Jul 22, 2016

👍 faced the same issue,
i guess since production no reload, it should work, but on development, we need to restart on concern change

@u007

This comment has been minimized.

Show comment
Hide comment
@u007

u007 Jul 22, 2016

from my finding, adding this in my concern file

Object.send(:remove_const, :<ConcernClass>) if Object.constants.include?(:<ConcernClass>)

fixed the issue, but some how the model class that include this concern class will need to reload too...
otherwise the model class will be using the old concern (havent reload ConcernClass module)

u007 commented Jul 22, 2016

from my finding, adding this in my concern file

Object.send(:remove_const, :<ConcernClass>) if Object.constants.include?(:<ConcernClass>)

fixed the issue, but some how the model class that include this concern class will need to reload too...
otherwise the model class will be using the old concern (havent reload ConcernClass module)

@u007

This comment has been minimized.

Show comment
Hide comment
@u007

u007 Jul 24, 2016

i tried to remove activeconcern from my concerns/tokenable.rb
but i realize, when i modify this file, the actual file that include this file will not reload
Example:

# ruby way
module Tokenable
  def self.included(klass)
    klass.before_validation :yahoo
  end

  def yahoo
    puts "aaaaa"
    self.token = "aaaaa"
  end

# models/aaa.rb
class Aaa < ActiveRecord::Base
  include Tokenable

end

when i run in padrino console,
i modified file concerns/tokenable.rb and change my up and token assignment to "bbbb",
then i ran reload!
and Tried

row = Aaa.new
row.valid?
# output "aaaaa"
# expected "bbbb"

u007 commented Jul 24, 2016

i tried to remove activeconcern from my concerns/tokenable.rb
but i realize, when i modify this file, the actual file that include this file will not reload
Example:

# ruby way
module Tokenable
  def self.included(klass)
    klass.before_validation :yahoo
  end

  def yahoo
    puts "aaaaa"
    self.token = "aaaaa"
  end

# models/aaa.rb
class Aaa < ActiveRecord::Base
  include Tokenable

end

when i run in padrino console,
i modified file concerns/tokenable.rb and change my up and token assignment to "bbbb",
then i ran reload!
and Tried

row = Aaa.new
row.valid?
# output "aaaaa"
# expected "bbbb"
@u007

This comment has been minimized.

Show comment
Hide comment
@u007

u007 Jul 24, 2016

okay, ive a fix,
can someone verify that this works with few rspec? :)

it detects reloading of concern file, then find all models that include the file,
and try to locate the file for reload, and remove the constant from session,
and finally load it in the end

https://gist.github.com/u007/47b2f021670d54f862624ae70494a085

u007 commented Jul 24, 2016

okay, ive a fix,
can someone verify that this works with few rspec? :)

it detects reloading of concern file, then find all models that include the file,
and try to locate the file for reload, and remove the constant from session,
and finally load it in the end

https://gist.github.com/u007/47b2f021670d54f862624ae70494a085

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment