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

borg-activate: Extend load-path before loading autoloads #159

Conversation

dennis-hamester
Copy link

@dennis-hamester dennis-hamester commented Feb 5, 2024

This changes borg-activate to extend load-path always and unconditionally before loading autoloads.

The current code only extends load-path after loading the autoloads, and it does that also in a bit of a redundant way. Once as part of the local activate function and then again outside as a fallback.

edit: Sorry, I just realize I didn't motivate this PR. I tried to load el-patch with borg. That package emits a (require 'el-patch-stub) into its autoloads file (see el-patch.el). This then fails when borg tries to load the autoloads file. While that may be unconventional (?), it still seems reasonable to me to assume that a package is in load-path before its autoload file is evaluated.

@tarsius tarsius closed this in 0a1c8ee Feb 5, 2024
tarsius added a commit that referenced this pull request Feb 5, 2024
Autoload files are supposed to add the containing directory to the
`load-path'.  This was lost when `autoload' was deprecated in favor
of `loaddefs-gen'.  This is a known issue (see bug#63625).  Use the
same workaround as used by `package-generate-autoloads'.  Instead of
relying on `loaddefs-generate' to do it on its own, pass appropriate
EXTRA-DATA to it.

Closes #159.
@dennis-hamester
Copy link
Author

Well, thanks for figuring out the correct fix or work-around. It takes a lot of experience to know all these intricacies in Emacs.

@dennis-hamester dennis-hamester deleted the fix/load-path-before-autoload branch February 5, 2024 18:46
@tarsius
Copy link
Member

tarsius commented Feb 5, 2024

You're welcome and thanks for the report!

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

Successfully merging this pull request may close these issues.

2 participants