* Errors on module feature [auto]loading (Re: zsh 5.0.2-test-1 is available)
[not found] ` <20131109223250.706e20fb@pws-pc.ntlworld.com>
@ 2013-11-10 20:03 ` Bart Schaefer
2013-11-10 20:44 ` Peter Stephenson
2013-11-13 20:11 ` PATCH: autoloading already loaded features Peter Stephenson
1 sibling, 1 reply; 3+ messages in thread
From: Bart Schaefer @ 2013-11-10 20:03 UTC (permalink / raw)
To: zsh-workers
[> workers]
On Nov 9, 10:32pm, Peter Stephenson wrote:
}
} Suppose we load builtin bar from zsh/foo at some point, doesn't matter
} how, then run "zmodload -a bar zsh/foo". We find that a builtin bar
} exists. We can then check that zsh/foo is already loaded, and is
} already providing builtin bar.
And conversely we can tell that a builtin bar already exists that is not
provided by zsh/foo ? This is the part that I thought for some reason
was difficult.
Where the analogy with autoloaded functions breaks down is that autoload
for functions cannot be told which file the function must come from. It
searches $fpath and takes the first one it finds. The autoload builtin
makes the assumption (rightly or wrongly) that $fpath is stable and that
if a function has already been loaded, it is the same one that would be
found if $fpath were searched again.
Loading of modules *themselves* with "zmodload zsh/foo" can make a similar
assumption about $module_path so the analogy holds there.
Explicit builtin autoloads with "zmodload -a zsh/foo bar" have no basis
for making that same assumption. We've provided a way to say "bar must
come from zsh/foo, even if zsh/bar would be found first by a $module_path
search". Therefore, if we *can't* guarantee that bar came from zsh/foo,
we should not suppress the error message.
} furthermore, given that you can check by this means the effect really
} *is* redundant rather than a conflict, it seems to me there is no
} harmful side effect of any sort.
I'm OK with this given the precondition. Do all the rules about warning
on circular dependency, etc., apply? If I try to explicitly autoload
from zsh/foo a feature that would implicitly be provided by zsh/bar,
when if ever would an error occur?
Aside: Should we update the manual to say that the right way to test for
module existence is "zmodload -F module" (with no feature arguments)? This
checks that the module could be loaded without enabling anything, whereas
"zmodload [-i] module" enables all the default features.
--
Barton E. Schaefer
^ permalink raw reply [flat|nested] 3+ messages in thread
* PATCH: autoloading already loaded features
[not found] ` <20131109223250.706e20fb@pws-pc.ntlworld.com>
2013-11-10 20:03 ` Errors on module feature [auto]loading (Re: zsh 5.0.2-test-1 is available) Bart Schaefer
@ 2013-11-13 20:11 ` Peter Stephenson
1 sibling, 0 replies; 3+ messages in thread
From: Peter Stephenson @ 2013-11-13 20:11 UTC (permalink / raw)
To: Zsh Hackers' List
On Sat, 9 Nov 2013 22:32:50 +0000
Peter Stephenson <p.w.stephenson@ntlworld.com> wrote:
> For autoloads from modules, which may be builtins, conditions, math
> functions or parameters, we can actually do a bit better, with a little
> more work but nothing like to the same extent (no additional loading is
> required). Suppose we load builtin bar from zsh/foo at some point,
> doesn't matter how, then run "zmodload -a bar zsh/foo". We find that a
> builtin bar exists. We can then check that zsh/foo is already loaded,
> and is already providing builtin bar.
Gah. This turns out to be completely trivial. We already check if the
module is loaded, so we can ensure that it does in fact provide that
feature. We just need to check at that point if it *is* providing that
feature. Then we can be absolutely confident that simply telling the
user everything is OK is the right thing to do: they're already getting
the feature they've just asked for. If it's not yet providing the
feature, marking it for autoload works unless there's a clash (it's a
little pointless since we could just enable it immediately but that's a
bit more work).
diff --git a/Src/module.c b/Src/module.c
index 5cc595c..c567b3c 100644
--- a/Src/module.c
+++ b/Src/module.c
@@ -3419,12 +3419,15 @@ autofeatures(const char *cmdnam, const char *module, char **features,
int ret = 0, subret;
Module defm, m;
char **modfeatures = NULL;
+ int *modenables = NULL;
if (module) {
defm = (Module)find_module(module,
FINDMOD_ALIASP|FINDMOD_CREATE, NULL);
if ((defm->node.flags & MOD_LINKED) ? defm->u.linked :
- defm->u.handle)
+ defm->u.handle) {
(void)features_module(defm, &modfeatures);
+ (void)enables_module(defm, &modenables);
+ }
} else
defm = NULL;
@@ -3544,6 +3547,16 @@ autofeatures(const char *cmdnam, const char *module, char **features,
ret = 1;
continue;
}
+ /*
+ * If the feature is already provided by the module, there's
+ * nothing more to do.
+ */
+ if (modenables[ptr-modfeatures])
+ continue;
+ /*
+ * Otherwise, marking it for autoload will do the
+ * right thing when the feature is eventually usd.
+ */
}
if (!m->autoloads) {
m->autoloads = znewlinklist();
--
Peter Stephenson <p.w.stephenson@ntlworld.com>
Web page now at http://homepage.ntlworld.com/p.w.stephenson/
^ permalink raw reply [flat|nested] 3+ messages in thread