Skip to content
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

Problem installing HTTP::UserAgent after Bailador #178

Closed
nige123 opened this issue Aug 5, 2017 · 8 comments
Closed

Problem installing HTTP::UserAgent after Bailador #178

nige123 opened this issue Aug 5, 2017 · 8 comments

Comments

@nige123
Copy link

nige123 commented Aug 5, 2017

I recently upgraded to Rakudo 2017.07 - after installing Bailador with zef I installed HTTP::UserAgent.

Starting Bailador now gives the error:

===SORRY!===
P6M Merging GLOBAL symbols failed: duplicate definition of symbol Response

Is it possible there is a symbol clash with 'Response'?

@jonathanstowe
Copy link
Collaborator

jonathanstowe commented Aug 5, 2017

yes, there may well be if Bailador or one of its dependencies has an HTTP::Response or possibly even just a Response in some other namespace. I think it may be because Bailador doesn't specify an "auth" in its META6. HTTP::UserAgent itself goes out of its way to fully qualify its own HTTP::Response and HTTP::Request with the appropriate auth and ISTR I sent a patch for HTTP::Server to avoid the clash with the similarly named modules that it provides. I guess that you'd probably see the same problem if you installed HTTP::Server.

Unfortunately there's not much that can be done in HTTP::UserAgent itself to prevent this short of renaming the modules, which might be a good idea in the medium term but not without some deprecation cycle as the module is a dependency of other modules.

It might be good if you could provide the actual code that is failing so we can test however.

@nige123
Copy link
Author

nige123 commented Aug 7, 2017

Hi,

Thanks for the explanation. 👍

Here is some code that shows the problem ...

shell> rakudobrew build moar 2017.07
shell> rakudobrew build zef
shell> zef install Bailador
shell> zef install HTTP::UserAgent
shell> echo 'use Bailador; use HTTP::UserAgent' > boom.p6
shell> perl6 boom.p6
===SORRY!===
P6M Merging GLOBAL symbols failed: duplicate definition of symbol Response
shell> echo 'use HTTP::UserAgent; use Bailador' > works.p6
shell> perl6 works.p6
shell>

HTH

@ugexe
Copy link
Collaborator

ugexe commented Aug 11, 2017

The problem seems to be related to 3 things:

  1. HTTP::UserAgent has a X::HTTP::Response
  2. HTTP::Response has a X::HTTP::Response
  3. Bailador has put something in either the X:: or ::Response namespace

1 and 2 seems to be trying to avoid circular dependency re X::HTTP base class. But by removing X::HTTP::Response from HTTP::UserAgent it no longer has merge failures (although it fails its test, since X::HTTP::Response in HTTP::Response does not have all the members of the one in HTTP::UserAgent).

The fact this only happens when loading Bailador first points at a rakudo bug.

@jonathanstowe
Copy link
Collaborator

jonathanstowe commented Aug 11, 2017

Yeah I was hesitating on calling it as a rakudo bug without collecting some tangible evidence but yeah I was inclining toward that position. But this observation gives a starting point to make a standalone reproducer/

I'll take a look at the weekend to see if we can work around this.

@jonathanstowe
Copy link
Collaborator

jonathanstowe commented Aug 12, 2017

Having had a quick look I think the sensible thing to do would to move all the exception classes into a single module and rationalise the names somewhat as this is not ideal by any standard :) Even if there is a bug in the Stash merge somewhere it will be neater and prevent any future problems.

Unless anyone has any strong objections, I will do this today.

@jonathanstowe
Copy link
Collaborator

For what it's worth, doing what I described above and using the more complex version of X::HTTP::Response just works and fixes the problem with Bailador. The just working thing may however may be due to insufficient testing of the exceptions, I'll try and improve this as well.

@jonathanstowe
Copy link
Collaborator

I'll merge the above branch in a bit unless someone finds a problem with it.

@nige123
Copy link
Author

nige123 commented Aug 12, 2017

Thanks for sorting this ! 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants