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

Make kotl-autoloads without building hyperbole-autoloads.el #307

Merged
merged 3 commits into from
Feb 17, 2023

Conversation

matsl
Copy link
Collaborator

@matsl matsl commented Feb 15, 2023

What

Make kotl/kotl-autoloads without building hyperbole-autoloads.el

Why

The target kotl/kotl-autoloads.el has as a side effect to also build the hyperbole-autoloads.el. That is not correct and can lead to unexpected problems and even errors. Patch by Stefan Monnier. This PR fixed this so that only kotl-autoloads are produced as a result of that make target.

@matsl matsl requested a review from rswgnu February 15, 2023 09:28
Makefile Outdated
@@ -426,7 +427,7 @@ hyperbole-autoloads.el: $(EL_COMPILE)
$(EMACS_BATCH) --debug --eval "(progn (setq generated-autoload-file (expand-file-name \"hyperbole-autoloads.el\") backup-inhibited t) (let (find-file-hooks) (hload-path--make-directory-autoloads \".\" generated-autoload-file)))"

kotl/kotl-autoloads.el: $(EL_KOTL)
$(EMACS_BATCH) --debug --eval "(progn (setq generated-autoload-file (expand-file-name \"kotl/kotl-autoloads.el\") backup-inhibited t) (let (find-file-hooks) (hload-path--make-directory-autoloads \"kotl/\" generated-autoload-file)))"
$(EMACS_PLAIN_BATCH) --debug --eval "(progn (setq generated-autoload-file (expand-file-name \"kotl/kotl-autoloads.el\") backup-inhibited t) (let (find-file-hooks) (make-directory-autoloads \"kotl/\" generated-autoload-file)))"
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This breaks versions of Emacs that I have tested in the past that lack make-directory autoloads. I will have to see if it is true of 27.1. If not, then we can use it.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, you are right. That can be true. The interesting part is that the scraping for autoloads works for all our versions we use here but I don't think it uses that target. We don't build for 21.1 though. Only 27.2. I'll add a test to check.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The test proves it breaks on 27.2. I'll see if I can fix a workaround.

Copy link
Collaborator Author

@matsl matsl Feb 16, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rswgnu The autoload generation for kotl now fallbacks to Emacs 27 functionality if not the more modern functions are available.

The hyperbole-autoloads target also generates the kotl-autoloads which
is wrong. This is a test to see if the proposed change works on all
versions of Emacs. It flips the order and by that runs the kotl
autoloads generation first.
@matsl matsl force-pushed the make_kotl_autoloads_without_triggering_other_builds branch from 5fca7ea to 069cf2a Compare February 16, 2023 23:32
@matsl matsl force-pushed the make_kotl_autoloads_without_triggering_other_builds branch from 069cf2a to 510b171 Compare February 16, 2023 23:33
@matsl matsl requested a review from rswgnu February 16, 2023 23:34
@matsl matsl merged commit 0370145 into master Feb 17, 2023
@matsl matsl deleted the make_kotl_autoloads_without_triggering_other_builds branch February 17, 2023 07:43
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

Successfully merging this pull request may close these issues.

None yet

2 participants