Giter Site home page Giter Site logo

Comments (32)

dabrahams avatar dabrahams commented on August 18, 2024

I think of autoloads as being there for the purposes of interactive use, and the need to explicitly ask for the features needed by the :after hook doesn't seem overly burdensome, so I'm inclined to judge this "not a defect." On the other hand, if you proposed a patch to fix it I might be inclined to accept it :-)

from el-get.

dimitri avatar dimitri commented on August 18, 2024

Also note that we have a global hook that runs once all init are done. I guess the semantics would be very much like what you want. But maybe moving :after call to be after autoloads are handled would be better...

(defcustom el-get-post-init-hooks nil
"Hooks to run after a package init.
It will get called with the package as first argument."
:group 'el-get
:type 'hook)

from el-get.

dabrahams avatar dabrahams commented on August 18, 2024

But maybe moving :after call to be after autoloads are handled would be better...

For fast startup, the autoloads are all compiled into one big file and evaluated at idle time, we would have to queue the hooks to be run.

from el-get.

dimitri avatar dimitri commented on August 18, 2024

Sounds like the think to do yes. It would be good to have :after be called at the right time, whatever that is depending on autoloads implementation. Up for it?

from el-get.

dabrahams avatar dabrahams commented on August 18, 2024

Not really; this strikes me as super low-priority, arguably not a bug, and even if it is a bug there's an easy workaround, and I'm not even sure that "at idle time" is the right place for :after actions. In fact, I'm almost sure it is not the right place.

And if I can't talk you out of making this change, I advise you to wait for me to integrate the dependency-management stuff because it already includes some queued-idle-time-actions infrastructure.

from el-get.

dimitri avatar dimitri commented on August 18, 2024

IIUC the problem only occurs at install/update time, because that's when the after function is called and autoloads are maybe not yet available. At init time, the after is always called once the autoloads are available, right?

If that's the case, that's a bug. The recipe is not working the same way on install and after that. I'm not good at prioritizing even my own work, so I won't try having an opinion on that, but I'd like this bug fixed (or some more explaining how can it not be a bug).

from el-get.

dabrahams avatar dabrahams commented on August 18, 2024

At init time, the after is always called once the autoloads are available, right?

I'm not sure anymore. If so, and you really want them to be the same, I'd say the right thing to do is to postpone loading the autoload file until idle time in all cases.

If that's the case, that's a bug. The recipe is not working the same way on install and after that.

Meh. If the rule is "Recipes shouldn't rely on having autoloads already set up," then the worst effect of having autoloads already present when init'ing an already-installed package is that it could mask violations of the rule. However, anyone who tests that their recipe installs properly will already have eliminated said violations. So IMO having these two conditions be the same should not be a high priority and definitely isn't worth complicating the code for. IMO the right answer is to add this guideline for recipe writers:

Do not rely on autoloads; list any packages you may need in the :features field.

That's what :features is for, after all.

from el-get.

dimitri avatar dimitri commented on August 18, 2024

Yeah that's a valid answer. Now, what are the autoloads good for, already?

from el-get.

dabrahams avatar dabrahams commented on August 18, 2024

autoloads are there so that users don't pay prematurely for library load time and don't have to load libraries explicitly when they use keys bound to their functions (or invoke their functions from `M-x...')

If you want to ensure :after functions are called after init, maybe you should just set them with `eval-after-load'

from el-get.

dimitri avatar dimitri commented on August 18, 2024

I think I like the eval-after-load idea. Then we should first do that, then care about the :features property. Recipes that need immediate activation will then use :features and that will call the :after code, right?

And recipes that cope with lazy install and init will just get taken care of by autoloads and the :after will get lazily called at "the right time"...

Seems better this way. What do you think?

from el-get.

llvilanova avatar llvilanova commented on August 18, 2024

If I understand this correctly, this would still break my assumption that if a package provides an autoloaded function (like the one on the example), I should be able to use it in any :after hook and rely on it being provided on the ".loaddefs".

With what you propose, the :after code would never fail, but it's just because it would never be executed (as the package would never be loaded), and this kind of dependent :after activation could build up into a "large" chain of dependent code that is never executed just because noone explicitly loads the leaves of the depdency trees.

If dabrahams' dependency tracking code is going to be able to do just this kinds of tasks, then I'd say its easier to just wait for it to permanently fix the issue.

from el-get.

dabrahams avatar dabrahams commented on August 18, 2024

@dimitri:

I think I like the eval-after-load idea. Then we should first do that, then care about the :features property. Recipes that need immediate activation will then use :features and that will call the :after code, right?

:featires can cause libraries to be loaded and thus their after-load code to be called. The one fly in that ointment is that you need to decide which library/ies to attach the after-load code to. Normally it'll be the library with the same name as the package, of course.

And recipes that cope with lazy install and init will just get taken care of by autoloads

I don't think, in the scenario I'm proposing, autoloads do anything to take care of recipes.

and the :after will get lazily called at "the right time"...

Seems better this way. What do you think?

That's what I had in mind:

@xscript:

If I understand this correctly, this would still break my assumption that if a package provides an autoloaded function (like the one on the example), I should be able to use it in any :after hook and rely on it being provided on the ".loaddefs".

How so? That after-load code will always run after the library is loaded. The only way I can see this breaking down is for large libraries, e.g. org-mode, where (require 'org) doesn't necessarily force-load every piece of the library. But then to have any trouble you'd need to have :after actions for org.el that rely on a piece of org that isn't required by org.el itself, which is at best an extremely rare corner case.

With what you propose, the :after code would never fail, but it's just because it would never be executed (as the package would never be loaded),

And what's the problem with that? If the package was never loaded, it was never el-get-init'ed, so why should the :after code run?

and this kind of dependent :after activation could build up into a "large" chain of dependent code that is never executed just because noone explicitly loads the leaves of the depdency trees.

I could always be missing something (I've been wrong in the past, believe it or not!), but I think this is a bit alarmist and not particularly well thought-through. If you can come up with a concrete example of the kind of scenario that will cause problems, please spell it out. Then we can gauge its seriousness.

If dabrahams' dependency tracking code is going to be able to do just this kinds of tasks, then I'd say its easier to just wait for it to permanently fix the issue.

It will provide the infrastructure you'd need to implement some kind of complicated after-autoloads logic, but actually adding that logic it so that it didn't cause additional problems would still, as I've said, be difficult at best. I'm the first to admit when I have a bug, but this is Not A Bug™—it's simply a case of a programmer making an assumption that was not guaranteed—and the convenience you'd gain if we could get it right would not be worth the ongoing maintenance costs to el-get. I'm convinced that eval-after-load is a better way to address the convenience issue, if it needs to be addressed (you can always use eval-after-load yourself in your :after clause).

Just my opinion, probably stated too strongly, for which I should have apologized in advance :-)

from el-get.

dabrahams avatar dabrahams commented on August 18, 2024

With what you propose, the :after code would never fail, but it's just because it would never be executed (as the package would never be loaded),

And what's the problem with that? If the package was never loaded, it was never el-get-init'ed, so why should the :after code run?

Okay, I just realized I'm wrong here. el-get-init runs, need not load the library (after all, that's what autoloads are for), and currently, the :after code runs anyway, which is how you're seeing the problem. After the change, the :after load would not run until and unless the package was actually loaded.

Fine, but in that case el-get-init is not doing anything other than adding the package to the load-path, right? Is there really a use case for :after code that must run after the load path is set up but before the package is actually loaded, and yet will cause the package to be loaded via autoloads? It just seems like a terribly contrived set of circumstances.

from el-get.

dimitri avatar dimitri commented on August 18, 2024

Agreed. What we need I think is that the :after code is registered to eval-after-load on the first :features entry. We can add provision to funcall it as we do now when there's no :features, but it would mean that :after either means "after loading" or "after init" when it currently means only "after init".

I'd be fine both ways I think, but maybe that just confirms I need to sleep.

from el-get.

dabrahams avatar dabrahams commented on August 18, 2024

What we need I think is that the :after code is registered to eval-after-load on the first :features entry.

That seems a little useless if :features is still going to eagerly cause loading.

Before we had autoload support, recipe authors were probably far more zealous in their use of :features than they now need to be, though.

We can add provision to funcall it as we do now when there's no :features, but it would mean that :after either means "after loading" or "after init" when it currently means only "after init".

Just so I can be clear, what do you think it should mean? I vote for "after loading." However, we either need to also change the meaning of :features to "These are the libraries that the :after code applies to" (in which case its name should change because library != feature) or we need a way separate from features: to indicate those libraries. Whew!

Sleep sounds advisable :-)

from el-get.

llvilanova avatar llvilanova commented on August 18, 2024

Fine, but in that case el-get-init is not doing anything other than adding the package to the load-path, right? Is there really a use case for :after code that must run after the load path is set up but before the package is actually loaded, and yet will cause the package to be loaded via autoloads? It just seems like a terribly contrived set of circumstances.

From my point of view (see below) it's for consistency. For example, if I'm able to add some global keybindings to autoloaded functions from that package, or add calls to autoloaded functions into hooks (all without the need of using :features), then why shouldn't I also be able to call an autoloaded function from the same :after hook?

There is also the case of defcustom variables, which I don't like to have set up into a custom.el file but prefer to have them set up on my main config file. I'm still new to emacs, so my approach might be incorrect, but I set these variables before requiring the package (so here :after would not help, right?).

Just so I can be clear, what do you think it should mean? I vote for "after loading."

Well, I thought that :after was run after adding the package into the load-path and making all its autoloads available, which would then be consistent with the rest of packages bundled with emacs, where the equivalent to :after is code in my config file.

The only extra twist I saw in :after is that I can put all code related to a package in that specific el-get entry, instead of spanning lines somewhere else on my config file (which I think is nice for packages that need minimal setup from my part).

True, if that's its intended use, then why use :after instead of using some extra lines in my config file? Well, the only reason I see for the use of :after to be mandatory is for people using (el-get nil) (which I don't).

For the sake of completeness, I'd like to also say that I see :features (foo) just as a very nice shorthand for :after '(lambda () (require 'foo)).

I just hope this explanation gives light to the reasons that led me to think I should be able to call autoloaded functions in :after without using :features :)

Oh! And infinite thanks to all of you for developing this package; it has simplified lots of semi-automatic messy initialization code on my config :)

from el-get.

dimitri avatar dimitri commented on August 18, 2024

Well I think it's getting easy: we want :after to bear the semantics of eval-after-load.

So, if we have :features, first require them then eval :after. If we don't, register the :after as an eval-after-load on the library name, the new entry :library, that defaults to (or :pkgname :package).

That should solve it, right?

from el-get.

dabrahams avatar dabrahams commented on August 18, 2024

Well I think it's getting easy: we want :after to bear the semantics of eval-after-load.

One alternative worth considering: you could say that :after is eager, and anyone who wants after-load semantics is free to write eval-after-load in his :after clause. That would be more flexible, if more verbose when you really want after-load semantics.

So, if we have :features, first require them then eval :after. If we don't, register the :after as an eval-after-load on the library name, the new entry :library, that defaults to (or :pkgname :package).

Seems perhaps needlessly complex. Why not use eval-after-load unconditionally?

from el-get.

dimitri avatar dimitri commented on August 18, 2024

Because I read that in the docs: Normally, well-designed Lisp programs should not use `eval-after-load'. That's the only reason :)

I'd prefer :after to have the "after load" semantics.

from el-get.

dabrahams avatar dabrahams commented on August 18, 2024

dimitri, I actually looked into that advice from the docs a few weeks ago, and it turns out that was not supposed to apply to situations like this one. It's more that lisp packages shouldn't be calling eval-after-load on other packages on which they depend. Anyway, since you're going to call it anyway in some circumstances, avoiding calling it to avoid falling afoul of that guideline on some code paths seems like a poor decision to me.

from el-get.

dimitri avatar dimitri commented on August 18, 2024

I think it's fixed now. I hope the fix is ok for all commenters here, I tried to pick the best alternative after considering all. Well, we can still revisit of course.

from el-get.

llvilanova avatar llvilanova commented on August 18, 2024

Sorry for the delay. I think the solution is overly complex. Bear in mind that my original complaint was on the case of autoloaded functions, so unless I'm missing something, this is not at all related to being able to add it 'eval-after-load', unless the loaded file is the newly created autoloads when a package is being installed.

from el-get.

dabrahams avatar dabrahams commented on August 18, 2024

Can you propose something less complex that addresses your concern?

Also, in your opinion is this concern serious enough to be worth addressing? If the only solution we could come up with is the one Dimitri implemented, should we keep that change or go back to the prior status quo?

from el-get.

dimitri avatar dimitri commented on August 18, 2024

After much discussion we didn't come to a way to ensure that autoloads are already loaded for a given package at init time. So if you want your recipe to just work, you need to use :features here.

You could use :post-init and when el-get-is-lazy this will only get called when the user explicitly requires the package, so you have the autoloads benefits.

Overall it's been argued that there's no point in autoloads support if all you want to do is an "immediate setup" so that your package is all ready when el-get is finished. Then you want :features.

from el-get.

llvilanova avatar llvilanova commented on August 18, 2024

Wow! Github just dropped my last comment during a service error. I'll try to summarize (it was pretty long).

As I see it, :after should be called always after the autoloads file is up-to-date and loaded wrt that package. That is, for an installed package, load the autoloads and call :after. For a package being installed, re-generate the autoloads, load that file and call :after.

This can be achived by performing the following steps:

  • install any pending packages
  • re-generate the autoloads file on the per-package post-install hook
  • syncronize (barrier)
  • load the autoloads file
  • initialize all packages (including calling :after when present)

This is much simpler than other approaches I was also describing on my deleted comment, and the only drawback is boot time iif there are packages to be installed.

I think this provides a much simpler UI, and behaves as expected from packages providing autoloads (or at least is how I expect it when comparing it to system-provided packages). And if the package writer was not kind enough to provide the autoload cookies, you still have the :features to immediately load it, or :after to manually declare what you want to autoload.

from el-get.

dabrahams avatar dabrahams commented on August 18, 2024

As I see it, :after should be called always after the autoloads file is up-to-date and loaded wrt that package.

Okay, but why? If you want to write an :after clause that requires the autoloads to be in place, you want to force the package to be loaded—if not as eagerly as possible, then at least as soon as the autoloads are ready. So you might as well use :features and get the package loaded right away, right? I'm trying to understand whether this idea that :after "should" be called after autoloads is a practical concern or just an aesthetic one. I agree that it would provide a slightly more forgiving environment for recipe authors if the autoloads were always available at :after time, but it doesn't seem to be crucial to me. Aesthetics count, of course, just not as much as practical concerns.

At the moment I'm not personally inclined take the effort to re-implement the way autoloads are handled; it was non-trivial to build and designed to impose the least latency on the user where he has to wait for the UI. However, if you can convince Dimitri or Julien to do it, or if you'd like to submit a patch that implements your idea so we can evaluate it, I'll be happy to weigh its benefits and costs with an open mind.

from el-get.

jd avatar jd commented on August 18, 2024

I agree with xscript's analysis. Note that :before is better to add autoloads if the package misses some.

Another solution would be to generate one autoload file per package and load it at the beginning of el-get-init. This would be simpler, but would force to use several "loaddefs" files.

With the async code, one needs to synchronize all package installation between their build and init, which does not sounds that easy to implement…

from el-get.

dabrahams avatar dabrahams commented on August 18, 2024

I agree with xscript's analysis.

If you mean that in some abstract sense it would be better if autoloads were in effect when :after was executed, then I agree too. I just can't see why it matters much in practice.

Note that :before is better to add autoloads if the package misses some.

Yes, I think we all came to that conclusion.

Another solution would be to generate one autoload file per package and load it at the beginning of el-get-init. This would be simpler, but would force to use several "loaddefs" files.

Worse, it would increase startup time. Usually, emacs won't byte-compile autoload files. I'm not sure why, but there's code specifically to prevent it. I arranged to dump all autoloads into a single file and then byte-compile it for maximum speed.

With the async code, one needs to synchronize all package installation between their build and init, which does not sounds that easy to implement…

I don't know what you mean here. Build reliably happens before init today; there's nothing new to implement AFAICT.

from el-get.

llvilanova avatar llvilanova commented on August 18, 2024

I agree with xscript's analysis.

If you mean that in some abstract sense it would be better if autoloads were in effect when :after was executed, then I agree too. I just can't see why it matters much in practice.

It doesn't. That why I said on the original report that this is just a small nuissance :)

Note that :before is better to add autoloads if the package misses some.

Yes, I think we all came to that conclusion.

Yes, sorry about that.

With the async code, one needs to synchronize all package installation between their build and init, which does not sounds that easy to implement…

I don't know what you mean here. Build reliably happens before init today; there's nothing new to implement AFAICT.

I think he means the process I described earlier:

  • parallel installation + generation of autoloads of non-installed packages
  • synchronize everything

None of the previous will be executed on the common case.

  • load the autoloads file
  • parallel initialization of all packages

Another option would be to update the contents of the autoloads file on each post-install hook, an re-load it before executing the package initialization. As long as we can ensure no one will load an autoloads file while it is being generated, this is probably simpler, and the performance does not matter that much as long as all overheads stem from the package installation code and not from package initialization.

from el-get.

dabrahams avatar dabrahams commented on August 18, 2024

Since we all agree this isn't more than a small nuisance, I'm not going to engage further with thoughts about how to solve it, if that's OK with you. I want to focus on dependency resolution.

from el-get.

llvilanova avatar llvilanova commented on August 18, 2024

Sure. My only concern now is with the new :library. If its only purpose is to tackle the case of this bug, then maybe it should be reverted.

Thanks for all,
Lluis

from el-get.

dimitri avatar dimitri commented on August 18, 2024

You eval-after-load against a library. I don't see any way to know this library name for sure if there's no user option for that.

from el-get.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.