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

Issue with dynamic loading of module #6049

Open
p6rt opened this issue Feb 3, 2017 · 7 comments
Open

Issue with dynamic loading of module #6049

p6rt opened this issue Feb 3, 2017 · 7 comments

Comments

@p6rt
Copy link

p6rt commented Feb 3, 2017

Migrated from rt.perl.org#130709 (status was 'open')

Searchable as RT130709$

@p6rt
Copy link
Author

p6rt commented Feb 3, 2017

From @cjfields

I am running into a bug that seems to have appeared after lexical module loading was fixed​:

rakudo/rakudo@4b529c8

The stripped-down code (w. tests) is located here​:

https://github.com/cjfields/bp6-bug

It appears that attempting to load modules dynamically when they are located deeper in the library (in at least one nested directory) fails with a ‘No symbol found’ error. Any help appreciated

Thanks,

chris

@p6rt
Copy link
Author

p6rt commented Feb 3, 2017

From @niner

The issue can be worked around/fixed with the following patch​:

Inline Patch
diff --git a/lib/Bio/SeqIO.pm6 b/lib/Bio/SeqIO.pm6
index 9ee4980..710dec4 100644
--- a/lib/Bio/SeqIO.pm6
+++ b/lib/Bio/SeqIO.pm6
@@ -10,10 +10,10 @@ class Bio::SeqIO {
             require ::($format);
         };
 
-        if ::($format) ~~ Failure {
+        if MY::{$format} ~~ Failure {
             die "Can't load $format: $!";
         } else {
-            $!plugin = ::($format).new();
+            $!plugin = MY::{$format}.new();
         }
     }
 }

As the symbols are imported lexically, you will find them in the lexical (MY) scope. I'm not sure if this should be considered a bug or not. I'm not familiar enough with the thoughts that went into the design. It's certainly a pitty, that ::() which is quite short became less useful and users have to use the longer MY::() instead.

@p6rt
Copy link
Author

p6rt commented Feb 3, 2017

The RT System itself - Status changed from 'new' to 'open'

@p6rt
Copy link
Author

p6rt commented Feb 3, 2017

From @cjfields

Thanks nine. My suggestion is that it's at the very least a buglet, in that there isn't consistency with how modules are loaded (note the module in the base directory, 'fasta', worked).

If the syntax stays, I also think it's worth noting this behavior in the docs and pointing out why it's set up this way, both of the following imply the 'require :​:($foo)' should work, but maybe this wasn't worked out prior to lexical module fixes?

https://docs.perl6.org/syntax/require
https://docs.perl6.org/language/packages#index-entry-%3A%3A%28%29

On Fri, 03 Feb 2017 02​:34​:45 -0800, nine@​detonation.org wrote​:

The issue can be worked around/fixed with the following patch​:

diff --git a/lib/Bio/SeqIO.pm6 b/lib/Bio/SeqIO.pm6
index 9ee4980..710dec4 100644
--- a/lib/Bio/SeqIO.pm6
+++ b/lib/Bio/SeqIO.pm6
@​@​ -10,10 +10,10 @​@​ class Bio​::SeqIO {
require :​:($format);
};

- if :​:($format) ~~ Failure {
+ if MY​::{$format} ~~ Failure {
die "Can't load $format​: $!";
} else {
- $!plugin = :​:($format).new();
+ $!plugin = MY​::{$format}.new();
}
}
}

As the symbols are imported lexically, you will find them in the
lexical (MY) scope. I'm not sure if this should be considered a bug or
not. I'm not familiar enough with the thoughts that went into the
design. It's certainly a pitty, that :​:() which is quite short became
less useful and users have to use the longer MY​::() instead.

@p6rt
Copy link
Author

p6rt commented Feb 4, 2017

From @cjfields

Ah, okay, I missed that it isn’t in the ‘require’ but in the symbol lookup afterwards. Okay, I think this makes some sense (I’ll look through docs to see if it goes with what is mentioned there), though it is a bit inconsistent in behavior with the base ‘fasta' module working while the nested ones (‘Bio​::fasta’, ‘Bio​::SeqIO​::fasta’) don't.

On 2/3/17, 7​:38 AM, "Chris Fields via RT" <perl6-bugs-followup@​perl.org> wrote​:

Thanks nine. My suggestion is that it's at the very least a buglet, in that there isn't consistency with how modules are loaded (note the module in the base directory, 'fasta', worked).

If the syntax stays, I also think it's worth noting this behavior in the docs and pointing out why it's set up this way, both of the following imply the 'require :​:($foo)' should work, but maybe this wasn't worked out prior to lexical module fixes?

https://docs.perl6.org/syntax/require
https://docs.perl6.org/language/packages#index-entry-%3A%3A%28%29

On Fri, 03 Feb 2017 02​:34​:45 -0800, nine@​detonation.org wrote​:

The issue can be worked around/fixed with the following patch​:

diff --git a/lib/Bio/SeqIO.pm6 b/lib/Bio/SeqIO.pm6
index 9ee4980..710dec4 100644
--- a/lib/Bio/SeqIO.pm6
+++ b/lib/Bio/SeqIO.pm6
@​@​ -10,10 +10,10 @​@​ class Bio​::SeqIO {
require :​:($format);
};

- if :​:($format) ~~ Failure {
+ if MY​::{$format} ~~ Failure {
die "Can't load $format​: $!";
} else {
- $!plugin = :​:($format).new();
+ $!plugin = MY​::{$format}.new();
}
}
}

As the symbols are imported lexically, you will find them in the
lexical (MY) scope. I'm not sure if this should be considered a bug or
not. I'm not familiar enough with the thoughts that went into the
design. It's certainly a pitty, that :​:() which is quite short became
less useful and users have to use the longer MY​::() instead.

@p6rt
Copy link
Author

p6rt commented Feb 4, 2017

From @LLFourn

looks similar to​: https://rt.perl.org/Public/Bug/Display.html?id=130535

require seems to be pretty broken since the lexical module thing.
Unfortunately our tests weren't good enough to pick this up ( I think it's
because it's being required dynamically outside the mainline).

LL

On Sat, Feb 4, 2017 at 4​:32 PM Fields, Christopher J <cjfields@​illinois.edu>
wrote​:

Ah, okay, I missed that it isn’t in the ‘require’ but in the symbol lookup
afterwards. Okay, I think this makes some sense (I’ll look through docs to
see if it goes with what is mentioned there), though it is a bit
inconsistent in behavior with the base ‘fasta' module working while the
nested ones (‘Bio​::fasta’, ‘Bio​::SeqIO​::fasta’) don't.

On 2/3/17, 7​:38 AM, "Chris Fields via RT" <perl6-bugs-followup@​perl.org>
wrote​:

Thanks nine. My suggestion is that it's at the very least a buglet, in
that there isn't consistency with how modules are loaded (note the module
in the base directory, 'fasta', worked).

If the syntax stays, I also think it's worth noting this behavior in the
docs and pointing out why it's set up this way, both of the following imply
the 'require :​:($foo)' should work, but maybe this wasn't worked out prior
to lexical module fixes?

https://docs.perl6.org/syntax/require
https://docs.perl6.org/language/packages#index-entry-%3A%3A%28%29

On Fri, 03 Feb 2017 02​:34​:45 -0800, nine@​detonation.org wrote​:

The issue can be worked around/fixed with the following patch​:

diff --git a/lib/Bio/SeqIO.pm6 b/lib/Bio/SeqIO.pm6
index 9ee4980..710dec4 100644
--- a/lib/Bio/SeqIO.pm6
+++ b/lib/Bio/SeqIO.pm6
@​@​ -10,10 +10,10 @​@​ class Bio​::SeqIO {
require :​:($format);
};

- if :​:($format) ~~ Failure {
+ if MY​::{$format} ~~ Failure {
die "Can't load $format​: $!";
} else {
- $!plugin = :​:($format).new();
+ $!plugin = MY​::{$format}.new();
}
}
}

As the symbols are imported lexically, you will find them in the
lexical (MY) scope. I'm not sure if this should be considered a bug or
not. I'm not familiar enough with the thoughts that went into the
design. It's certainly a pitty, that :​:() which is quite short became
less useful and users have to use the longer MY​::() instead.

@p6rt
Copy link
Author

p6rt commented Feb 6, 2017

From @cjfields

nine, I did an update to show that the proposed fix doesn’t actually work. The tests pass but that’s b/c they aren’t testing for instances of that class; a simple check of the data indicates that the ‘MY​::{$plugin}’ is Nil and the produced object is an Any.

chris

From​: Lloyd Fournier <lloyd.fourn@​gmail.com<mailto​:lloyd.fourn@​gmail.com>>
Date​: Saturday, February 4, 2017 at 7​:40 AM
To​: Chris Fields <cjfields@​illinois.edu<mailto​:cjfields@​illinois.edu>>, "perl6-bugs-followup@​perl.org<mailto​:perl6-bugs-followup@​perl.org>" <perl6-bugs-followup@​perl.org<mailto​:perl6-bugs-followup@​perl.org>>
Subject​: Re​: [perl #​130709] Issue with dynamic loading of module

looks similar to​: https://rt.perl.org/Public/Bug/Display.html?id=130535https://urldefense.proofpoint.com/v2/url?u=https-3A__rt.perl.org_Public_Bug_Display.html-3Fid-3D130535&d=DwMFaQ&c=8hUWFZcy2Z-Za5rBPlktOQ&r=fbHa8Njtvh9VmSnzJxiEUTW9NWDwMMwQAzhgZDO41GQ&m=tNfZnbxhTQyMX-PjVfJyNu9PxenVYrHF-bu-bi0Ngyw&s=ayfJt8ohQ2RMSLmkUBKBROo-yuE64w1RoiE6fh0SyfY&e=

require seems to be pretty broken since the lexical module thing. Unfortunately our tests weren't good enough to pick this up ( I think it's because it's being required dynamically outside the mainline).

LL

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

1 participant