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.

Sign up for GitHub

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

Jump to bottom

Loading package loads some other libraries, which may mask missing requirements #47

Closed
alphapapa opened this issue Jun 17, 2024 · 6 comments
Closed

Loading package loads some other libraries, which may mask missing requirements #47

alphapapa opened this issue Jun 17, 2024 · 6 comments
Assignees
@alphapapa
Labels
bug Something isn't working
Milestone
0.8

Comments

@alphapapa
Copy link
Owner

See melpa/melpa#9062 (comment)
@alphapapa alphapapa self-assigned this Jun 17, 2024
@alphapapa alphapapa added bug Something isn't working help wanted Extra attention is needed labels Jun 17, 2024
@alphapapa alphapapa added this to the Future milestone Jun 17, 2024
alphapapa added a commit that referenced this issue Jun 17, 2024
@alphapapa
WIP: Avoid loading package.el
5688595 
The package.el library loads some other libraries, like url-parse.el,
which can give false negatives when byte-compiling.  With this change,
package.el is only loaded when installing dependencies or linters, not
when doing anything else.

TODO: Maybe load package.el when running interactively.  (An
interactive session does by default, but in the context of using
makem.sh, maybe we shouldn't...?)

See: <#47>

Reported-by: Joseph Turner <[email protected]>
@alphapapa
Copy link
Owner Author

alphapapa commented Jun 17, 2024 •
edited
Loading

@josephmturner This seems to work. With the changes on branch https://github.com/alphapapa/makem.sh/tree/wip/avoid-loading-package, compiling org-transclusion-http at ebbeba0 shows:

src/org-transclusion-http/makem.sh -v --emacs\=emacs-29.3 --sandbox\=.sandbox lint-compile
LOG (2024-06-17 03:30:43): Linting compilation...
LOG (2024-06-17 03:30:43): Compiling...

In toplevel form:
org-transclusion-http.el:69:4: Error: url is not a struct name
ERROR (2024-06-17 03:30:43): Linting compilation failed.
LOG (2024-06-17 03:30:43): Finished with 1 errors.

Please let me know what you think of the changes on the branch. Thanks.

@alphapapa alphapapa modified the milestones: Future, 0.8 Jun 17, 2024
@alphapapa alphapapa removed the help wanted Extra attention is needed label Jun 17, 2024
@josephmturner
Copy link
Contributor

josephmturner commented Jun 17, 2024 •
edited
Loading

Thanks for the fix! This seems to work except for a typo. There' s a missing backslash in the sandbox function, fixed in this commit:

2adfb42

@alphapapa
Copy link
Owner Author

Of course, that's what I get for copying and pasting. Thanks.

@alphapapa
Copy link
Owner Author

By the way, I think this change is probably fine to make, but it would be good to give it some basic testing first, e.g. test it with a few packages, intentionally cause some byte-compilation errors and ensure that they're caught, etc. If you have the time and interest to test it on a few like that, please do.

@josephmturner
Copy link
Contributor

I tested with org-transclusion-http, where this patch fixed this issue.

I also tested with hyperdrive.el by commenting out the (require 'plz) sexp in hyperdrive-lib.el, and makem correctly reported hyperdrive-lib.el:228:4: Error: plz-response is not a struct name.

In chordpro-mode.el, I commented out (require 'thingatpt), and makem correctly reported chordpro-mode.el: Error: the function thing-at-point-looking-at is not known to be defined.

@alphapapa
Copy link
Owner Author

Thank you!

alphapapa added a commit that referenced this issue Jun 27, 2024
@alphapapa
WIP: Avoid loading package.el
978e765 
The package.el library loads some other libraries, like url-parse.el,
which can give false negatives when byte-compiling.  With this change,
package.el is only loaded when installing dependencies or linters, not
when doing anything else.

TODO: Maybe load package.el when running interactively.  (An
interactive session does by default, but in the context of using
makem.sh, maybe we shouldn't...?)

See: <#47>

Reported-by: Joseph Turner <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@alphapapa alphapapa

Labels
bug Something isn't working
Projects
None yet
Milestone
0.8
Development

No branches or pull requests
2 participants
@alphapapa @josephmturner