Add missing requires

[jun0]

This PR fixes the problem where you get Symbol’s value as variable is void: org-gtd-archive-location or similar such-and-such-is-undefined errors if you don't do an explicit (require 'org-gtd), making autoloads useless.

Steps to reproduce

Short version: do M-x org-gtd-engage when org-gtd is installed and activated but not loaded.

Long version: assuming Emacs 29.1, make a fresh directory containing an init.el file with the following contents and nothing else.

(require 'package)
(setq package-archives
      '(("gnu" . "http://elpa.gnu.org/packages/")
        ("melpa" . "http://melpa.org/packages/")))

(require 'use-package)
(use-package org-gtd
  :ensure t
  :defer t ; Note this is implied by :bind, which is almost always used.
  :init
  (setq org-gtd-update-ack "3.0.0")
  :config
  (org-gtd-mode 1))

1. Invoke emacs --init-directory=<that directory>, wait for it to download and install org-gtd.
2. Restart emacs with the same arguments. (Restart is necessary because byte-compilation of org-gtd apparently causes bits of it to be loaded.)
3. Do M-x org-gtd-engage.

Result:
org-gtd-engage: Symbol’s value as variable is void: org-gtd-archive-location

Cause

1. Emacs calls package-activate-all at startup, which loads org-gtd-autoloads.el, which sets org-gtd-engage to be loaded from org-gtd-agenda.el.
2. org-gtd-engage uses with-org-gtd-context, but org-gtd-agenda.el doesn't load all the packages this macro requires, in particular org-gtd-archive.el.

Similar problems abound for other autoloaded functions. The root of the problem is that with-org-gtd-context tightly couples org-gtd-agenda.el, org-gtd-projects.el, org-gtd-delegate, and org-gtd-core.el, but these modules are not (cannot be) set up to require each other for fear of recursive require.

Fix

I fixed it by adding require for org-gtd-agenda et al. from the code generated by with-org-gtd-context.

I also added a missing (require 'f) in org-gtd-core.el while I was at it. You can tickle this bug by removing org-gtd-core.elc before doing org-gtd-engage, which complains that -flatten is undefined.

[jun0]

Sorry, last-minute change: it's better to require org-gtd instead of individual modules like org-gtd-archive. See commit log for details.

[Trevoke]

That's interesting, I hadn't considered doing it that way, and I was going mad trying to put the right requires in all the right places.

Do you have any idea if/how we could write tests or otherwise ensure this works in a CI pipeline or during development, in an automated fashion?

[jun0]

That's interesting, I hadn't considered doing it that way, and I was going mad trying to put the right requires in all the right places.

Yeah, it's a bit tricky, but I think this really is the right place for this require. The reason with-org-gtd-context makes sense at all, despite the apparent cyclic dependency, is that the dependency is dynamic: the macro is only used inside defuns and not needed at load time. So it's only natural that the require is also dynamic.

Do you have any idea if/how we could write tests or otherwise ensure this works in a CI pipeline or during development, in an automated fashion?

We could do something like the "Steps to Reproduce" I wrote above: start a fresh Emacs instance with minimal config and invoke one of the autoloaded functions, restart Emacs and repeat. Perhaps write a test/autoload-test.el that repeatedly starts new Emacs instances as subprocesses, let the subprocess-Emacs write out test results to a file or stdout, and parse that in test/autoload-test.el?

I've never worked with Eldev, buttercup, or GitHub CI before, but if you can give me some quick pointers (how/when tests are triggered in your Eldev workflow or the CI framework; how it finds test files; and where I could put files meant only for the subprocess-Emacs) maybe I can put something together. Be warned I'm a bit busy over the next week or so, though.

[jun0]

So... it took a while to find the time to iron out the details, but this PR now includes automated testing for the autoload functionality. What do you think?

I skipped tests for several commands that need some setup (and I'm not knowledgeable enough to set them up myself), I hope that's fine:

org-gtd-delegate-agenda-item
org-gtd-delegate-item-at-point
org-gtd-review-area-of-focus
org-gtd-project-cancel

[Trevoke]

Oh wow, thank you! I was preparing to spend most of tomorrow reviewing this, but this new PR is fantastic.

And it's fine that you're leaving out tests for these commands; only review-area-of-focus has a reasonable chance of triggering an autoload situation, but I think this PR should cover that case anyway; I'll add that test in the future.

Thank you again for this, it's going to help a lot of people!