Fixes for ELPA support

[ruediger]

The biggest issue is that el-get-elpa-post-remove only deletes the package directory. But it won't remove the package from the package-alist which causes package.el to still consider the package installed and thus subsequent attempts to reinstall the package will fail. The patch changes the function to use package-delete instead.

Minor issues I had: el-get-elpa-package-directory should check if the elpa directory exists. On a fresh install ~/.emacs.d/elpa/ might not yet exist and this prevented package installation completely since the directory is queried before install is called. The patch also changes the function to consider packages in the global directories package-directory-list.

While debugging the major issue I also ran into trouble with el-get-elpa-symlink-package causing an error in file-relative-name. I changed it to provide a better error message.

[npostavs]

Yes, in emacs 24.3 and below we have: (defun package-delete (name version)...

[ruediger]

I attempted a fix for 24.3. I don't have Emacs 24.3 installed, so I have to wait for Travis to retest.

[ruediger]

Ok build passes. It should probably be manually tested for 24.3.

E.g.: test/test-recipe.sh recipes/adaptive-wrap.rcp

[npostavs]

24.3's package-delete wants strings. Anyway that's a minor thing.

More serious problems:

• 24.3's package-delete calls package--dir which assumes the package is in package-user-dir.
• Both versions of package-delete throw an error when given "system" packages.

(defun package-delete (pkg-desc)
  (let ((dir (package-desc-dir pkg-desc)))
    (if (not (string-prefix-p (file-name-as-directory
                               (expand-file-name package-user-dir))
                              (expand-file-name dir)))
        ;; Don't delete "system" packages.
    (error "Package `%s' is a system package, not deleting"
               (package-desc-full-name pkg-desc))

---

It looks like the only solution is to reimplement our own package-delete without these limitations, what do you think?

[ruediger]

If we implement our own package-delete then we have to keep fixing things if package.el changes again. Maybe for 24.3 the old behavior (simply delete) was correct.

Is the error really an issue? If package.el can't remove system packages we probably shouldn't try either and it's up to the user. If it causes problems in the chain of things then we could either use ignore-errors or turn the error into a message with

(condition-case err
    (package-delete ...)
  (error (message "err: %s" (cadr err))))

Here is a new suggestion

(defun el-get-elpa-post-remove (package)
  "Do remove the ELPA bits for package, now"
  (let* ((pkg (el-get-as-symbol package))
         (pkg-descs (cdr (assq pkg package-alist))))
    (dolist (pkg-desc pkg-descs)
      (if (version< emacs-version "24.4")
          (let ((p-elpa-dir (el-get-elpa-package-directory package)))
            (if p-elpa-dir
                (delete-directory p-elpa-dir 'recursive)
              (message "el-get: could not find ELPA dir for %s." package)))
        (with-no-warnings
          (package-delete pkg-desc))))))

This basically uses the old version (but delete-directory directly instead of dired) for Emacs <24.4 and else package-delete.

[npostavs]

Is the error really an issue? If package.el can't remove system packages we probably shouldn't try either and it's up to the user.

Oh, I mixed up your 1st commit which extends el-get-elpa-package-directory to the system packages, with the last one, so I was thinking you were extending package deletion to system packages.

However this could be an issue: el-get used to override package-user-dir to ~/.emacs.d/el-get/elpa/, #1641 removed that but put the old override value into package-directory-list. So there could potentially be some elpa packages installed by el-get that appear to be system packages even though they're really not.

This basically uses the old version (but delete-directory directly instead of dired) for Emacs <24.4 and else package-delete.

That's probably the right approach. I suspect the dired thing was there because the recursive argument to delete-directory was only introduced in 23.2, but I think it's okay to require at least 23.4 now (that's the minimum version we test with anyway).

(dolist (pkg-desc pkg-descs)

Is it correct to delete all package versions? Wouldn't the other versions be the system packages? I'm a bit confused as to what scenarios result in multiple installed package versions...

[ruediger]

However this could be an issue: el-get used to override package-user-dir to ~/.emacs.d/el-get/elpa/, #1641 removed that but put the old override value into package-directory-list. So there could potentially be some elpa packages installed by el-get that appear to be system packages even though they're really not.

This only happens when the user installs package.el on an old Emacs using el-get? We could try to dynamically bind package-user-dir in that specific case. But maybe it's enough of a corner case to tell the user to deal with it.

Is it correct to delete all package versions? Wouldn't the other versions be the system packages? I'm a bit confused as to what scenarios result in multiple installed package versions...

I'm not sure how to handle this case. Maybe we could read the symlink to figure out which version was installed via el-get. I'm not sure if there is a case which could cause el-get to try to install several versions (e.g., via dependencies).

[npostavs]

This only happens when the user installs package.el on an old Emacs using el-get?

No, it happens when a user installed elpa packages with an el-get from before April 16.

Maybe we could read the symlink to figure out which version was installed via el-get

Hmm, maybe we can just leave it as delete all, and see if someone complains. It's hard to envision a situation where it would matter...

[ruediger]

We could consider using EPL https://github.com/cask/epl/

[npostavs]

Here is what I have so far, test-recipe.sh is failing in Emacs 24.4. It looks like it's the same bug as cask/epl#18 (debbugs ref).

I'm not sure if using EPL will work: for instance I see their elp-package-delete answers the "should we delete all versions" question by taking a descriptor as argument. But from el-get's context if we have to find the package descriptor first it doesn't seem like EPL is helping us very much.

[npostavs]

test-recipe.sh is failing in Emacs 24.4.

Ah, that bug was fixed, I just had an outdated version. I made a new pull request at #1763.