lint-compile doesn't catch missing dependency on package "package"

[jscheid]

When a file fails to declare a dependency on package package, lint-compile doesn't flag it. This is because when running emacs for byte-compiling, $package_initialize_file gets loaded, which asks to (require 'package) leading to a false negative.

I've worked around it like so: https://github.com/jscheid/prettier.el/pull/60/commits/d1f0fc151a0d8b5fc4edf4fa81fc0791ed0dfedf

[alphapapa]

Thanks. It seems like one could just do (unload-feature 'package) after this line: https://github.com/alphapapa/makem.sh/blob/15ac490863892e10ae51eeb98d38009a216bb9d6/makem.sh#L235 What do you think?

BTW, note that you should probably upgrade your version of makem.sh, because there's a similar fix for byte-compilation issues in recent commits (files are now compiled separately, which reveals issues like this that could otherwise be hidden).

[jscheid]

I had tried your suggestion of adding just (unload-feature 'package) first, but it broke other things:

Run SANDBOX_DIR=$(mktemp -d) || exit 1
LOG (2020-12-13 10:57:54): Initializing sandbox...
LOG (2020-12-13 10:57:54): Installing packages into sandbox...
Importing package-keyring.gpg...
Importing package-keyring.gpg...done
Contacting host: elpa.gnu.org:443
Failed to download ‘gnu’ archive.
package.el is not yet initialized!
ERROR (2020-12-13 10:57:55): Unable to initialize sandbox.

I'll try upgrading to latest to see if that works without a patch.

[jscheid]

Oh, I see what you mean about compiling separately. That won't make a difference in this case because it's a single file, but yes that's a good change, I'll upgrade. Thanks for the heads up.

[alphapapa]

Not sure if this is still a problem, so deferring to 0.8. Please chime in if you can add more information.

[jscheid]

@alphapapa hmm, now I can't get it to flag any packages not listed as a dependency.

For example, the below passes lint-compile and lint-package (as long as async and package-lint packages are installed).

;;; main.el --- Summary

;; Package-Requires: ((emacs "26.1"))

;;; Commentary:

;; Blah

;;; Code:

(require 'async)
(require 'package)

(provide 'main)

;;; main.el ends here

Shouldn't it complain that Package-Require doesn't include async or package?

Admittedly it's been a while since I last used makem or package-lint so I might be missing something obvious...

[alphapapa]

Shouldn't it complain that Package-Require doesn't include async or package?

Well:

1. package is a built-in library.
2. If you're not using --sandbox, then it will find those packages in your local Emacs configuration and not give any warnings.
3. package-lint, which is used by the lint-package rule, is not part of makem.

[jscheid]

Right, got confused here -- this ticket wasn't even about lint-package originally but about lint-compile. Ignore the above, here is the correct repro:

;;; main.el --- Summary

;;; Commentary:

;; Blah

;;; Code:

(package-version-join '(1 2 3))

(provide 'main)

;;; main.el ends here

./makem.sh lint-compile doesn't produce any warnings for it, but emacs --batch --eval '(byte-compile-file "main.el")' does:

In end of data:
main.el:9:2: Warning: the function ‘package-version-join’ is not known to be defined.

Tested with 2c4eac6 so I'd say it's still a problem.

[alphapapa]

I think this is because makem.sh loads the package library, because it needs that to load dependency packages that are installed by the package system. If you test a function from another library that's built-in but not loaded, you should get a warning, e.g.

;;; main.el --- Summary

;;; Commentary:

;; Blah

;;; Code:

(package-version-join '(1 2 3))
(lm-section-start "Commentary")

(provide 'main)

;;; main.el ends here

-*- mode: compilation; default-directory: "/tmp/makem-test/" -*-
Compilation started at Sun Jan  7 21:28:57

/tmp/makem-test/makem.sh -vvv --emacs\=emacs-29.1 --sandbox\=.sandbox lint-compile
LOG (2024-01-07 21:28:57): Initializing sandbox...
LOG (2024-01-07 21:28:57): Sandbox initialized.
LOG (2024-01-07 21:28:57): Linting compilation...
LOG (2024-01-07 21:28:57): Compiling...
LOG (2024-01-07 21:28:57): Compiling file: test.el...

In end of data:
test.el:10:2: Warning: the function `lm-section-start' is not known to be defined.
LOG (2024-01-07 21:28:57): Compiling file failed: test.el
ERROR (2024-01-07 21:28:57): Linting compilation failed.
LOG (2024-01-07 21:28:57): Finished with 1 errors.

Compilation exited abnormally with code 1 at Sun Jan  7 21:28:57

I think it's rare to call a function from package directly like this, so I'm comfortable with this being a very minor, known issue, which we could document. What do you think? Thanks.

[jscheid]

@alphapapa I reported it because I did run into it in real life (https://github.com/jscheid/prettier.el/issues/58), so it's rare but not just theoretical. Also, in case you've missed it, I've suggested a fix that involves unloading the package package.

That being said, I do agree it's a minor problem so if you don't want to fix it that's totally fine.

[alphapapa]

AFAICT, package is loaded even when running emacs -Q, so I don't think it would make sense to manually unload it--it would cause Emacs's state to be more different from the default. As well, that would mean that using makem.sh interactive would be unable to call package commands without manually loading the library again.

There are a number of built-in libraries which are loaded by default, and those don't require a (require FEATURE) call to be present, and it wouldn't be appropriate to unload-feature on them in order to cause a compiler warning for missing require calls that aren't needed in practice.

It appears that emacs --batch doesn't load package by default, so using --batch to byte-compile-file doesn't produce the same environment, so it emits the warning--one which shouldn't matter in practice, unless the file is actually being used in batch mode. So this appears to be an artificial problem and a false warning.

[jscheid]

@alphapapa interesting, but according to the original bug report (https://github.com/jscheid/prettier.el/issues/58) it seems that it can happen in an interactive setting, outside emacs --batch. Maybe something that's changed in more recent Emacs versions? I'll see if I can repro that original report on latest Emacs.

[alphapapa]

@alphapapa interesting, but according to the original bug report (jscheid/prettier.el#58) it seems that it can happen in an interactive setting, outside emacs --batch. Maybe something that's changed in more recent Emacs versions? I'll see if I can repro that original report on latest Emacs.

If package-install is configured to install packages asynchronously, it does so in a separate process, likely using --batch, which would explain the warning when installing the package, but it should still work at runtime.

[jscheid]

Ahh... right, that would explain it. Good catch.

To summarize:

• some warnings printed by package-install aren't caught by lint-compile
• this likely only affects package package and only when package-install is configured to be async, and shouldn't have any ill effects at runtime.

I still think it's pretty easy to fix (by unloading package) but since it really is an edge case, feel free to close, up to you.

[alphapapa]

Well, the summary as I understand it something like this:

1. Emacs loads the package library by default, except when called with --batch.
2. package-install can be configured to install packages asynchronously, which uses --batch, which means that it byte-compiles files in a different environment than the one encountered at runtime. Usually that's not a problem, and sometimes it's even better to do it that way. makem.sh does it that way for the same reason.
3. So calling functions from packages should work without requireing the library. But --batch doesn't "know" that, so compiling packages in it will warn if packages isn't explicitly required.

My best judgment is that unloading package would be a very bad idea; unloading any packages/libraries/features is not really supported by Emacs in the first place, despite the unload-feature command's existing. Doing so would likely lead to mysterious problems in the future. And this unusual case of a false warning caused by a built-in package being loaded automatically except for some cases in a separate Emacs process can't justify that risk.

So I'm afraid this edge case will have to remain a known issue. At least the workaround is simple on your end. :) There are many warnings that become impossible to eliminate due to supporting multiple Emacs versions, so it becomes impossible to get a clean linting sometimes.

[jscheid]

@alphapapa OK no problem, makes sense to me. Thanks anyway for taking a look.