bug#23607: Please apply this patched function to package.el in Emacs 25.

classic Classic list List threaded Threaded
8 messages Options
Reply | Threaded
Open this post in threaded view
|

bug#23607: Please apply this patched function to package.el in Emacs 25.

Robert Weiner
Replacing this function in package.el allows for correct generation of the <package>-autoloads.el file for packages that have .el files in multiple subdirectories.  The only thing changed is the line with 'apply' in it.  It ignores symlinks to directories in case there are any.  It is a simple change.  I hope you like it.

This is important for larger packages such as my forthcoming Hyperbole package which includes a 2nd directory of code.


(defun package-generate-autoloads (name pkg-dir)
  (let* ((auto-name (format "%s-autoloads.el" name))
         ;;(ignore-name (concat name "-pkg.el"))
         (generated-autoload-file (expand-file-name auto-name pkg-dir))
         ;; Silence `autoload-generate-file-autoloads'.
         (noninteractive inhibit-message)
         (backup-inhibited t)
         (version-control 'never))
    (package-autoload-ensure-default-file generated-autoload-file)
    (apply #'update-directory-autoloads pkg-dir
  (delq nil (mapcar (lambda (f) (and (file-directory-p f)
     (not (file-symlink-p f)) f))
    (directory-files pkg-dir t "[a-zA-Z].*" nil))))
    (let ((buf (find-buffer-visiting generated-autoload-file)))
      (when buf (kill-buffer buf)))
    auto-name))
Reply | Threaded
Open this post in threaded view
|

bug#23607: Please apply this patched function to package.el in Emacs 25.

Stefan Monnier
> Replacing this function in package.el allows for correct generation of the
> <package>-autoloads.el file for packages that have .el files in multiple
> subdirectories.

FWIW, I think this will break the dash package which includes
a dev/ert.el file with autoload cookies in it and those shouldn't be
placed into the dash-autoloads.el file.


        Stefan


PS: Please send it as a patch so we can see what changed without having
to try and guess.



Reply | Threaded
Open this post in threaded view
|

bug#23607: Please apply this patched function to package.el in Emacs 25.

Clément Pit--Claudel-2
On 2016-07-20 22:05, Stefan Monnier wrote:
> FWIW, I think this will break the dash package which includes
> a dev/ert.el file with autoload cookies in it and those shouldn't be
> placed into the dash-autoloads.el file.

What kind of breakage do you have in mind?


signature.asc (836 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

bug#23607: Please apply this patched function to package.el in Emacs 25.

Stefan Monnier
>> FWIW, I think this will break the dash package which includes
>> a dev/ert.el file with autoload cookies in it and those shouldn't be
>> placed into the dash-autoloads.el file.
> What kind of breakage do you have in mind?

The autoloads for the real ert.el will be overridden by those from
dash-autoloads.el which will say that for example that ert-deftest
should be autoloaded from "dev/ert.el".


        Stefan



Reply | Threaded
Open this post in threaded view
|

bug#23607: Please apply this patched function to package.el in Emacs 25.

Clément Pit--Claudel-2
On 2016-07-21 00:40, Stefan Monnier wrote:
>>> FWIW, I think this will break the dash package which includes
>>> a dev/ert.el file with autoload cookies in it and those shouldn't be
>>> placed into the dash-autoloads.el file.
>> What kind of breakage do you have in mind?
>
> The autoloads for the real ert.el will be overridden by those from
> dash-autoloads.el which will say that for example that ert-deftest
> should be autoloaded from "dev/ert.el".

I see; I didn't realize dash had their own copy of ert... I wonder why they do.
But you're right, this is definitely an issue.

Clément.


signature.asc (836 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

bug#23607: Please apply this patched function to package.el in Emacs 25.

Bob Weiner
In reply to this post by Stefan Monnier
On Wed, Jul 20, 2016 at 10:05 PM, Stefan Monnier <[hidden email]> wrote:

>> Replacing this function in package.el allows for correct generation of the
>> <package>-autoloads.el file for packages that have .el files in multiple
>> subdirectories.
>
> FWIW, I think this will break the dash package which includes
> a dev/ert.el file with autoload cookies in it and those shouldn't be
> placed into the dash-autoloads.el file.
>
>
>         Stefan
>
>
> PS: Please send it as a patch so we can see what changed without having
> to try and guess.

Here is the patch.  -- Bob


*** package-orig.el 2016-05-17 12:16:33.000000000 -0400
--- package.el 2016-06-15 14:43:46.000000000 -0400
***************
*** 916,928 ****
           (backup-inhibited t)
           (version-control 'never))
      (package-autoload-ensure-default-file generated-autoload-file)
!     (update-directory-autoloads pkg-dir)
      (let ((buf (find-buffer-visiting generated-autoload-file)))
        (when buf (kill-buffer buf)))
      auto-name))
 
  (defun package--make-autoloads-and-stuff (pkg-desc pkg-dir)
   "Generate autoloads, description file, etc.. for PKG-DESC installed at PKG-DIR."
    (package-generate-autoloads (package-desc-name pkg-desc) pkg-dir)
    (let ((desc-file (expand-file-name (package--description-file pkg-dir)
                                       pkg-dir)))
--- 913,929 ----
           (backup-inhibited t)
           (version-control 'never))
      (package-autoload-ensure-default-file generated-autoload-file)
!     (apply #'update-directory-autoloads pkg-dir
!   (delq nil (mapcar (lambda (f) (and (file-directory-p f)
!      (not (file-symlink-p f))
!      f))
!     (directory-files pkg-dir t "[a-zA-Z].*" nil))))
      (let ((buf (find-buffer-visiting generated-autoload-file)))
        (when buf (kill-buffer buf)))
      auto-name))
 
  (defun package--make-autoloads-and-stuff (pkg-desc pkg-dir)
   "Generate autoloads, description file, etc.. for PKG-DESC installed at PKG-DIR."
    (package-generate-autoloads (package-desc-name pkg-desc) pkg-dir)
    (let ((desc-file (expand-file-name (package--description-file pkg-dir)
                                       pkg-dir)))


Reply | Threaded
Open this post in threaded view
|

bug#23607: Please apply this patched function to package.el in Emacs 25.

Phillip Lord-3
In reply to this post by Clément Pit--Claudel-2
Clément Pit--Claudel <[hidden email]> writes:

> On 2016-07-21 00:40, Stefan Monnier wrote:
>>>> FWIW, I think this will break the dash package which includes
>>>> a dev/ert.el file with autoload cookies in it and those shouldn't be
>>>> placed into the dash-autoloads.el file.
>>> What kind of breakage do you have in mind?
>>
>> The autoloads for the real ert.el will be overridden by those from
>> dash-autoloads.el which will say that for example that ert-deftest
>> should be autoloaded from "dev/ert.el".
>
> I see; I didn't realize dash had their own copy of ert... I wonder why they do.
> But you're right, this is definitely an issue.

Dash isn't the only package to do this. I've also seen package.el
packaged in other places. Mostly, I think, to support backward
compatability, I think.

Why are dev files being packaged, though? At the moment, I don't think
that they should be.

Phil



Reply | Threaded
Open this post in threaded view
|

bug#23607: Please apply this patched function to package.el in Emacs 25.

Lars Ingebrigtsen
In reply to this post by Stefan Monnier
Stefan Monnier <[hidden email]> writes:

>>> FWIW, I think this will break the dash package which includes
>>> a dev/ert.el file with autoload cookies in it and those shouldn't be
>>> placed into the dash-autoloads.el file.
>> What kind of breakage do you have in mind?
>
> The autoloads for the real ert.el will be overridden by those from
> dash-autoloads.el which will say that for example that ert-deftest
> should be autoloaded from "dev/ert.el".

It seems like the proposed patch would introduce breaking changes, and
the bug report stalled at that point.  So it seem unlikely that there'll
be further progress in this bug report, and I'm closing it.

--
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no