#359 ✓not-applicable
Ewout

conflicts with vlad deployer

Reported by Ewout | June 7th, 2008 @ 03:28 PM

This is not really a datamapper problem, but can be fixed anyway.

When setting variables in vlad the deployer, it will fail if there is a method defined with the same name. A crucial parameter in vlad is :repository.

Datamapper defines Kernel#repository, so vlad fails.

There is a workaround in vlad: setting the $TESTING flag to true. IMHO, datamapper should be friendly and not define methods in Kernel. This is just one example of bad things that could happen.

Comments and changes to this ticket

  • Sam Smoot

    Sam Smoot June 10th, 2008 @ 10:14 PM

    • Assigned user changed from “Sam Smoot” to “Adam French”

    Ewout: Right, generally I agree, but the learning curve gets pretty steep if you don't try to be a little nice atleast.

    Still, we can check if it's already declared and skip the definition if so to avoid conflicts.

    We'll do that tomorrow then...

  • Ewout

    Ewout June 11th, 2008 @ 07:05 AM

    Sam, I agree with you about the learning curve. Your solution will

    probably not work for the vlad conflict, however. Vlad does not define

    Kernel#repository, it just checks if repository is a "reserved" method

    that is present in one of their own classes. Kernel methods are present

    everywhere.

    Namespacing could be an option, like

    Kernel#dm_repository

    or

    Datamapper#repository

    When using the latter, the user may always choose to include the

    Datamapper module and use the repository function like it was defined in

    the kernel.

  • Sam Smoot

    Sam Smoot June 11th, 2008 @ 10:56 AM

    Well that's lame (reserving a method name but not actually defining it). I'll talk to them about it.

    There is a DataMapper::repository method. The Kernel method should just forward to it.

  • Adam French

    Adam French June 12th, 2008 @ 11:16 AM

    • State changed from “new” to “hold”

    putting this ticket on hold because the next step is a response from the Vlad guys.

    On a sidenote, maybe Kernel#repository (or Kernel#repo) would be better off in DM-Shorthand, and DataMapper.repository() be the core call.

  • Sam Smoot

    Sam Smoot June 12th, 2008 @ 11:25 AM

    Thanks Adam.

    Gotta disagree with the dm-shorthand tho'. Don't want to strip so much out of core that you need 5 other gems just to get something half-way useful. ;-)

    Generally yes, Kernel extensions are bad. Agree 100%. But there will be 1 or 2 exceptions in every project, and this one is ours. I don't think it's too much to ask that if you're going to require a reserved method, that you do it in a way that other libs can detect if they're going to try to be nice and not step on your toes.

    So Vlad should just stub-in a Kernel#repository method if they aren't going to scope their usage. Though, wild-guess, I'm assuming the situation is more like they're using instance_eval + method_missing to call methods that don't exist, and the Kernel#repository interferes.

    That's just fragile though and inherently fraught with peril, and you can't blame us or anyone else if it breaks when the alternative is to use actual methods for method definitions.

  • Ewout

    Ewout September 29th, 2008 @ 12:05 PM

    • Tag set to dm-core, bug, suggestion

    Until the vlad people decide to namespace their settings, add this hack to your rakefile to deploy a merb/datamapper application with vlad:

    Hack for top-level name clash between vlad and datamapper.

    if Rake.application.top_level_tasks.any? {|t| t == 'deploy' or t =~ /^vlad:/} begin

    $TESTING = true # Required to bypass check for reserved_name? in vlad. DataMapper 0.9.x defines Kernel#repository...
    require 'vlad'
    Vlad.load :scm => "git", :app => nil
    
    

    rescue LoadError

    # do nothing
    
    

    end end

  • Ewout

    Ewout September 29th, 2008 @ 12:06 PM

    Sorry for the formatting error. Here's the hack again:

    
    # Hack for top-level name clash between vlad and datamapper.
    if Rake.application.top_level_tasks.any? {|t| t == 'deploy' or t =~ /^vlad:/}
      begin
        $TESTING = true # Required to bypass check for reserved_name? in vlad. DataMapper 0.9.x defines Kernel#repository...
        require 'vlad'
        Vlad.load :scm => "git", :app => nil
      rescue LoadError
        # do nothing
      end
    end
    
  • Dan Kubb (dkubb)

    Dan Kubb (dkubb) December 4th, 2008 @ 04:40 AM

    • Tag changed from dm-core, bug, suggestion to bug, dm-core, suggestion
    • State changed from “hold” to “not-applicable”
    • Assigned user cleared.

    I'm not sure what the fix is for this. We certainly do need a "repository" method available to maintain a nice UI in the code, that's not really up for debate.

    However we should probably make sure we don't use Kernel::repository anywhere internally in DM so that if there is a conflict it does not affect DM code. This is something we generally do anyway.

    Ultimately though, we can't control what Vlad or any other library does, so I'm going to mark this as not-applicable given that there are work-arounds for Vlad mentioned in this ticket.

Please Sign in or create a free account to add a new ticket.

With your very own profile, you can contribute to projects, track your activity, watch tickets, receive and update tickets through your email and much more.

New-ticket Create new ticket

Create your profile

Help contribute to this project by taking a few moments to create your personal profile. Create your profile »

People watching this ticket

Pages