Skip Menu |
 

This queue is for tickets about the Import-Into CPAN distribution.

Report information
The Basics
Id: 97311
Status: open
Priority: 0/
Queue: Import-Into

People
Owner: ether [...] cpan.org
Requestors: kaoru [...] slackwise.net
Cc:
AdminCc:

Bug Information
Severity: Unimportant
Broken in:
  • 1.002003
  • 1.002004
Fixed in: (no value)



Subject: New automatic module loading broke Test::Kit 2.0
Download (untitled) / with headers
text/plain 777b
Hey, The new automatic module loading behaviour in import::into 1.002003 has broken the evil ways I'm using import::into in Test::Kit. http://www.cpantesters.org/cpan/report/2c4d1db0-0bfe-11e4-aeb4-166d0a370852 "Can't locate Test/Kit/Fake/MyTest/Basic/Test/More.pm in @INC (you may need to install the Test::Kit::Fake::MyTest::Basic::Test::More module)" I'm creating "fake" packages at runtime and importing things into them dynamically to make Test::Kit do its thing. The addition of the automatic loading in response to RT#96995 has broken this. I can probably fix things from the Test::Kit side by adding the "fake" packages to %INC manually... but I thought you might want to know about this and maybe make the automatic loading more optional somehow. Thanks, - Alex
Download (untitled) / with headers
text/plain 683b
On Thu Jul 17 03:17:08 2014, KAORU wrote: Show quoted text
> I can probably fix things from the Test::Kit side by adding the "fake" > packages to %INC manually... but I thought you might want to know > about this and maybe make the automatic loading more optional somehow.
Here's my fix for my code: https://github.com/kaoru/Test-Kit2/commit/3f6e3ea3b2d12e05b9e0795709845ff660a6af5b I don't have a patch for import::into because I don't know your preferred solution. I suppose my recommendation would be to add import::into::with_auto_module_load() or something, and revert import::into() to its older behaviour. Anyway, up to you. Test::Kit 2.02 is on its way through the PAUSE tunnels to CPAN.
Download (untitled) / with headers
text/plain 1.2k
On 2014-07-17 00:17:08, KAORU wrote: Show quoted text
> Hey, > > The new automatic module loading behaviour in import::into 1.002003 > has broken the evil ways I'm using import::into in Test::Kit. > > http://www.cpantesters.org/cpan/report/2c4d1db0-0bfe-11e4-aeb4- > 166d0a370852 > > "Can't locate Test/Kit/Fake/MyTest/Basic/Test/More.pm in @INC (you may > need to install the Test::Kit::Fake::MyTest::Basic::Test::More > module)" > > I'm creating "fake" packages at runtime and importing things into them > dynamically to make Test::Kit do its thing. The addition of the > automatic loading in response to RT#96995 has broken this. > > I can probably fix things from the Test::Kit side by adding the "fake" > packages to %INC manually... but I thought you might want to know > about this and maybe make the automatic loading more optional somehow. > > Thanks, > > - Alex
We could fix this by using an interface in Module::Runtime that doesn't exist yet but we've fantasized about a bit: "load the module, and only die if it does exist and it couldn't be loaded properly; if it simply doesn't exist at all, then simply return false." -- it's on my tuit list; I'll make a note of this distribution as being another place where we could make use of it.
Download (untitled) / with headers
text/plain 504b
On Thu Jul 17 12:06:19 2014, ETHER wrote: Show quoted text
> We could fix this by using an interface in Module::Runtime that > doesn't exist yet but we've fantasized about a bit: "load the module, > and only die if it does exist and it couldn't be loaded properly; if > it simply doesn't exist at all, then simply return false." -- it's on > my tuit list; I'll make a note of this distribution as being another > place where we could make use of it.
Ooh that does sound like a good idea, good thinking. Thanks, - Alex
Download (untitled) / with headers
text/plain 329b
On 2014-07-17 00:17:08, KAORU wrote: Show quoted text
> I can probably fix things from the Test::Kit side by adding the "fake" > packages to %INC manually... but I thought you might want to know > about this and maybe make the automatic loading more optional somehow.
Oh yes, you might find this of use: https://metacpan.org/pod/me::inlined :D
Download (untitled) / with headers
text/plain 489b
As the one who's at fault for this happening: I consider this behavior to be as intended. Creating packages by emulating use/require via poking at internals should be either done completely, including updating %INC, or not at all. As such i consider the correct fix indeed to update the breaking module to update %INC properly, instead of changing I::I to possibly hide such misbehavior. Others might disagree, but so far i have not seen compelling evidence or argument to the contrary.
Download (untitled) / with headers
text/plain 170b
On Thu Jul 17 12:20:33 2014, ETHER wrote: Show quoted text
> Oh yes, you might find this of use: https://metacpan.org/pod/me::inlined :D
Haha sneaky, you sure do write a lot of evil :-)
Download (untitled) / with headers
text/plain 907b
On Thu Jul 17 12:24:26 2014, MITHALDU wrote: Show quoted text
> As the one who's at fault for this happening: > > I consider this behavior to be as intended. Creating packages by > emulating use/require via poking at internals should be either done > completely, including updating %INC, or not at all.
Fair enough Christian, I see your point about remaining disciplined and keeping %INC in sync with other things going on. Also it meant I discovered Module::Runtime::module_notional_filename() which was a nice find. Show quoted text
> As such i consider the correct fix indeed to update the breaking > module to update %INC properly, instead of changing I::I to possibly > hide such misbehavior.
Already done, Test::Kit 2.02 is on CPAN :-) Show quoted text
> Others might disagree, but so far i have not seen compelling evidence > or argument to the contrary.
I don't have any strong feelings either way, I leave it up to Karen and co(-maintainers.)
Download (untitled) / with headers
text/plain 1.7k
On Thu Jul 17 03:17:08 2014, KAORU wrote: Show quoted text
> Hey, > > The new automatic module loading behaviour in import::into 1.002003 > has broken the evil ways I'm using import::into in Test::Kit. > > http://www.cpantesters.org/cpan/report/2c4d1db0-0bfe-11e4-aeb4- > 166d0a370852 > > "Can't locate Test/Kit/Fake/MyTest/Basic/Test/More.pm in @INC (you may > need to install the Test::Kit::Fake::MyTest::Basic::Test::More > module)" > > I'm creating "fake" packages at runtime and importing things into them > dynamically to make Test::Kit do its thing. The addition of the > automatic loading in response to RT#96995 has broken this. > > I can probably fix things from the Test::Kit side by adding the "fake" > packages to %INC manually... but I thought you might want to know > about this and maybe make the automatic loading more optional somehow.
No, sorry, this behaviour is absolutely as intended. But I do think we owe you a better explanation as to why. The basic problem here is that because import is a UNIVERSAL method, calling RandomThing->import is a no-op, rather than an error. So, while I absolutely support your right to create on-the-fly packages for various things (and even wrote Package::Variant to make it easier :) if we allow it without %INC being set, it'll turn any typo into a silently eaten error - hence why ether's solution, though excellent for some things, is useless here. I'm going to leave this ticket open for now because most of Import::Into's lines are already an explanation of what it does and why, and we should probably explain this more clearly so people who're inconvenienced by this change understand why it was a net win, and why the less inconvenient alternatives sadly don't quite do the right thing.
Download (untitled) / with headers
text/plain 1.1k
On Thu Jul 17 12:41:33 2014, MSTROUT wrote: Show quoted text
> No, sorry, this behaviour is absolutely as intended. But I do think we > owe you a better explanation as to why. > > The basic problem here is that because import is a UNIVERSAL method, > calling > > RandomThing->import > > is a no-op, rather than an error. > > So, while I absolutely support your right to create on-the-fly > packages for various things (and even wrote Package::Variant to make > it easier :) if we allow it without %INC being set, it'll turn any > typo into a silently eaten error - hence why ether's solution, though > excellent for some things, is useless here. > > I'm going to leave this ticket open for now because most of > Import::Into's lines are already an explanation of what it does and > why, and we should probably explain this more clearly so people who're > inconvenienced by this change understand why it was a net win, and why > the less inconvenient alternatives sadly don't quite do the right > thing.
Makes sense, thanks for the further explanation Matt. I'm convinced, keep the new behaviour for import::into and I'll keep my %INC fix in Test::Kit. - Alex
Download (untitled) / with headers
text/plain 374b
On 2014-07-17 09:41:33, MSTROUT wrote: Show quoted text
> ...and we should probably explain this more clearly so people who're > inconvenienced by this change understand why it was a net win, and why > the less inconvenient alternatives sadly don't quite do the right > thing.
Agreed, there's enough twisty passages here that a paragraph in documentation explaining it all would be helpful.


This service is sponsored and maintained by Best Practical Solutions and runs on Perl.org infrastructure.

Please report any issues with rt.cpan.org to rt-cpan-admin@bestpractical.com.