* bug#16762: Installed packages are not considered part of the archive contents
@ 2014-02-15 16:06 Johan Andersson
2014-03-21 6:47 ` Dmitry Gutov
0 siblings, 1 reply; 14+ messages in thread
From: Johan Andersson @ 2014-02-15 16:06 UTC (permalink / raw)
To: 16762
[-- Attachment #1: Type: text/plain, Size: 780 bytes --]
Hi,
I have noticed a change of behavior in package.el between version 24.3.1
and current snapshot version.
In snapshot, if a package is installed with the most recent version, it is
not added to the variable package-archive-contents. I don't know the reason
for this, but I don't think it makes sense. Even if the package is
installed, it is still part of the package archives in my opinion.
You have to realize that other packages (such as https://github.com/cask/epl)
depend on the internals of package.el because it does not have an API (for
some reason). Considering Epl is a dependency to Cask and a lot of people
are using Cask, this change will affect many users.
With this change, how can I know if a package is available, no matter if
it's installed or not?
Thanks!
[-- Attachment #2: Type: text/html, Size: 995 bytes --]
^ permalink raw reply [flat|nested] 14+ messages in thread
* bug#16762: Installed packages are not considered part of the archive contents
2014-02-15 16:06 bug#16762: Installed packages are not considered part of the archive contents Johan Andersson
@ 2014-03-21 6:47 ` Dmitry Gutov
2014-03-21 15:33 ` Stefan Monnier
0 siblings, 1 reply; 14+ messages in thread
From: Dmitry Gutov @ 2014-03-21 6:47 UTC (permalink / raw)
To: Johan Andersson; +Cc: 16762
Johan Andersson <johan.rejeep@gmail.com> writes:
> In snapshot, if a package is installed with the most recent version, it is
> not added to the variable package-archive-contents. I don't know the reason
> for this, but I don't think it makes sense. Even if the package is
> installed, it is still part of the package archives in my opinion.
This was changed in revision 113000, not exactly sure why.
Stefan, could you explain that part of the change?
Seems like it could've fixed installed packages wrongly showing up in
the `list-packages' list, but I don't remember having that problem. Was
it made to improve the behavior of `package-install'?
> You have to realize that other packages (such as https://github.com/cask/epl)
> depend on the internals of package.el because it does not have an API (for
> some reason). Considering Epl is a dependency to Cask and a lot of people
> are using Cask, this change will affect many users.
If you're building a house on shaky foundation, and don't work on the
foundation, having to fix the house from time to time seems to be in the
job description.
(Unlike many other packages, there are no developer(s) specifically
maintaining package.el, looking after its API, etc).
> With this change, how can I know if a package is available, no matter if
> it's installed or not?
Look in both `package-archive-contents' and `package-alist', probably.
Although that depends on your definition of "available".
^ permalink raw reply [flat|nested] 14+ messages in thread
* bug#16762: Installed packages are not considered part of the archive contents
2014-03-21 6:47 ` Dmitry Gutov
@ 2014-03-21 15:33 ` Stefan Monnier
2014-03-21 23:07 ` Dmitry Gutov
0 siblings, 1 reply; 14+ messages in thread
From: Stefan Monnier @ 2014-03-21 15:33 UTC (permalink / raw)
To: Dmitry Gutov; +Cc: Johan Andersson, 16762
>> In snapshot, if a package is installed with the most recent version, it is
>> not added to the variable package-archive-contents. I don't know the reason
>> for this, but I don't think it makes sense. Even if the package is
>> installed, it is still part of the package archives in my opinion.
> This was changed in revision 113000, not exactly sure why.
> Stefan, could you explain that part of the change?
Hmm... can't quite remember, sorry.
> Seems like it could've fixed installed packages wrongly showing up in
> the `list-packages' list, but I don't remember having that problem.
> Was it made to improve the behavior of `package-install'?
Clearly, the intention of my changes was to move towards a situation
where package-archive-contents indeed holds all the packages in the
archives. But some didn't quite make it (e.g. the ones that are on
hold, IIRC, and maybe the ones that are obsolete).
If you can show an actual problem that this causes (from the user's
point of view), we can try to fix it.
And in trunk feel free to try and change things so that those packages
are included in package-archive-contents. Such a change would be
welcome, in general (as would be a change that gets rid of the other
exceptions such as the packages that are "on-hold").
Stefan
^ permalink raw reply [flat|nested] 14+ messages in thread
* bug#16762: Installed packages are not considered part of the archive contents
2014-03-21 15:33 ` Stefan Monnier
@ 2014-03-21 23:07 ` Dmitry Gutov
2014-03-22 2:46 ` Stefan Monnier
0 siblings, 1 reply; 14+ messages in thread
From: Dmitry Gutov @ 2014-03-21 23:07 UTC (permalink / raw)
To: Stefan Monnier; +Cc: Johan Andersson, 16762
On 21.03.2014 17:33, Stefan Monnier wrote:
> Clearly, the intention of my changes was to move towards a situation
> where package-archive-contents indeed holds all the packages in the
> archives.
AFAICS, the revision in question did the reverse: it made installed
packages excluded from package-archive-contents (as long as the
installed version is not obsolete). Here's the relevant part:
@@ -914,19 +889,25 @@
(entry (cons name pkg-desc))
(existing-package (assq name package-archive-contents))
(pinned-to-archive (assoc name package-pinned-packages)))
- (cond ((and pinned-to-archive
- ;; If pinned to another archive, skip entirely.
- (not (equal (cdr pinned-to-archive) archive)))
- nil)
- ((not existing-package)
- (push entry package-archive-contents))
- ((version-list-< (package-desc-version (cdr existing-package))
- (package-desc-version pkg-desc))
- ;; Replace the entry with this one.
- (setq package-archive-contents
- (cons entry
- (delq existing-package
- package-archive-contents)))))))
+ (cond
+ ;; Skip entirely if pinned to another archive or if no more recent
+ ;; than what we already have installed.
+ ((or (and pinned-to-archive
+ (not (equal (cdr pinned-to-archive) archive)))
+ (let ((bi (assq name package--builtin-versions)))
+ (and bi (version-list-<= version (cdr bi))))
+ (let ((ins (cdr (assq name package-alist))))
+ (and ins (version-list-<= version (package-desc-version
ins)))))
+ nil)
+ ((not existing-package)
+ (push entry package-archive-contents))
+ ((version-list-< (package-desc-version (cdr existing-package))
+ version)
+ ;; Replace the entry with this one.
+ (setq package-archive-contents
+ (cons entry
+ (delq existing-package
+ package-archive-contents)))))))
(defun package-download-transaction (package-list)
"Download and install all the packages in PACKAGE-LIST.
> If you can show an actual problem that this causes (from the user's
> point of view), we can try to fix it.
Johan?
> And in trunk feel free to try and change things so that those packages
> are included in package-archive-contents.
I think this change, might be more beneficial to apply in 24.4.
This way, the packages interfacing with package.el (like Epl, already
mentioned here) won't have to deal with the sudden jump in behavior
specific only to 24.4, but not versions before or after it.
> (as would be a change that gets rid of the other
> exceptions such as the packages that are "on-hold").
I agree that it would make sense, and that would be better left for trunk.
^ permalink raw reply [flat|nested] 14+ messages in thread
* bug#16762: Installed packages are not considered part of the archive contents
2014-03-21 23:07 ` Dmitry Gutov
@ 2014-03-22 2:46 ` Stefan Monnier
2014-03-23 8:16 ` Dmitry Gutov
0 siblings, 1 reply; 14+ messages in thread
From: Stefan Monnier @ 2014-03-22 2:46 UTC (permalink / raw)
To: Dmitry Gutov; +Cc: Johan Andersson, 16762
> AFAICS, the revision in question did the reverse: it made installed packages
> excluded from package-archive-contents (as long as the installed version is
> not obsolete). Here's the relevant part:
Indeed. Not sure why I ended up doing it that way, but clearly that was
not by accident.
>> And in trunk feel free to try and change things so that those packages
>> are included in package-archive-contents.
> I think this change, might be more beneficial to apply in 24.4.
Yes, but as long as we don't know why this was done, it's too risky to
change it. If/when we figure out what that was about and come up with
a way to fix the problem, we can judge whether that's appropriate for
24.4.
Stefan
^ permalink raw reply [flat|nested] 14+ messages in thread
* bug#16762: Installed packages are not considered part of the archive contents
2014-03-22 2:46 ` Stefan Monnier
@ 2014-03-23 8:16 ` Dmitry Gutov
2014-03-24 0:55 ` Stefan
0 siblings, 1 reply; 14+ messages in thread
From: Dmitry Gutov @ 2014-03-23 8:16 UTC (permalink / raw)
To: Stefan Monnier; +Cc: Johan Andersson, 16762
On 22.03.2014 04:46, Stefan Monnier wrote:
> Yes, but as long as we don't know why this was done, it's too risky to
> change it. If/when we figure out what that was about and come up with
> a way to fix the problem, we can judge whether that's appropriate for
> 24.4.
From what I can see, except `package-install', all places where
`package-archive-contents' is used, either append its contents to
`package-alist' elements, or use a predicate calling `package-installed-p'.
The following patch seems to offer the same functionality:
=== modified file 'lisp/emacs-lisp/package.el'
--- lisp/emacs-lisp/package.el 2014-03-22 08:43:30 +0000
+++ lisp/emacs-lisp/package.el 2014-03-23 08:03:37 +0000
@@ -1047,14 +1047,11 @@
(existing-packages (assq name package-archive-contents))
(pinned-to-archive (assoc name package-pinned-packages)))
(cond
- ;; Skip entirely if pinned to another archive or already installed.
+ ;; Skip entirely if pinned to another archive or built-in.
((or (and pinned-to-archive
(not (equal (cdr pinned-to-archive) archive)))
(let ((bi (assq name package--builtin-versions)))
- (and bi (version-list-= version (cdr bi))))
- (let ((ins (cdr (assq name package-alist))))
- (and ins (version-list-= version
- (package-desc-version (car ins))))))
+ (and bi (version-list-= version (cdr bi)))))
nil)
((not existing-packages)
(push (list name pkg-desc) package-archive-contents))
@@ -1090,8 +1087,11 @@
(package-refresh-contents))
(list (intern (completing-read
"Install package: "
- (mapcar (lambda (elt) (symbol-name (car elt)))
- package-archive-contents)
+ (delq nil
+ (mapcar (lambda (elt)
+ (unless (package-installed-p (car elt))
+ (symbol-name (car elt))))
+ package-archive-contents))
nil t)))))
(package-download-transaction
(if (package-desc-p pkg)
^ permalink raw reply [flat|nested] 14+ messages in thread
* bug#16762: Installed packages are not considered part of the archive contents
2014-03-23 8:16 ` Dmitry Gutov
@ 2014-03-24 0:55 ` Stefan
2014-03-24 6:20 ` Dmitry Gutov
0 siblings, 1 reply; 14+ messages in thread
From: Stefan @ 2014-03-24 0:55 UTC (permalink / raw)
To: Dmitry Gutov; +Cc: Johan Andersson, 16762
>> Yes, but as long as we don't know why this was done, it's too risky to
>> change it. If/when we figure out what that was about and come up with
>> a way to fix the problem, we can judge whether that's appropriate for
>> 24.4.
> From what I can see, except `package-install', all places where
> `package-archive-contents' is used, either append its contents to
> `package-alist' elements, or use a predicate calling `package-installed-p'.
> The following patch seems to offer the same functionality:
Does it affect the output of M-x package-list RET?
What about the package-menu-mark-upgrades?
> (cond
> - ;; Skip entirely if pinned to another archive or already installed.
> + ;; Skip entirely if pinned to another archive or built-in.
> ((or (and pinned-to-archive
> (not (equal (cdr pinned-to-archive) archive)))
> (let ((bi (assq name package--builtin-versions)))
> - (and bi (version-list-= version (cdr bi))))
> - (let ((ins (cdr (assq name package-alist))))
> - (and ins (version-list-= version
> - (package-desc-version (car ins))))))
> + (and bi (version-list-= version (cdr bi)))))
> nil)
If we can keep the already installed packages, couldn't we also keep the
already built-in packages?
Stefan
^ permalink raw reply [flat|nested] 14+ messages in thread
* bug#16762: Installed packages are not considered part of the archive contents
2014-03-24 0:55 ` Stefan
@ 2014-03-24 6:20 ` Dmitry Gutov
2014-03-24 8:10 ` Johan Andersson
2014-03-24 13:07 ` Stefan
0 siblings, 2 replies; 14+ messages in thread
From: Dmitry Gutov @ 2014-03-24 6:20 UTC (permalink / raw)
To: Stefan; +Cc: Johan Andersson, 16762
On 24.03.2014 02:55, Stefan wrote:
> Does it affect the output of M-x package-list RET?
It's identical (checked with diff).
> What about the package-menu-mark-upgrades?
It works. Don't see any problems.
> If we can keep the already installed packages, couldn't we also keep the
> already built-in packages?
Yes, seems so. I hadn't tried this before because the previous behavior
was to omit them (I think), but the patch below seems to work fine WRT
the questions above.
=== modified file 'lisp/emacs-lisp/package.el'
--- lisp/emacs-lisp/package.el 2014-03-23 08:35:56 +0000
+++ lisp/emacs-lisp/package.el 2014-03-24 06:11:52 +0000
@@ -1047,14 +1047,9 @@
(existing-packages (assq name package-archive-contents))
(pinned-to-archive (assoc name package-pinned-packages)))
(cond
- ;; Skip entirely if pinned to another archive or already installed.
- ((or (and pinned-to-archive
- (not (equal (cdr pinned-to-archive) archive)))
- (let ((bi (assq name package--builtin-versions)))
- (and bi (version-list-= version (cdr bi))))
- (let ((ins (cdr (assq name package-alist))))
- (and ins (version-list-= version
- (package-desc-version (car ins))))))
+ ;; Skip entirely if pinned to another archive.
+ ((and pinned-to-archive
+ (not (equal (cdr pinned-to-archive) archive)))
nil)
((not existing-packages)
(push (list name pkg-desc) package-archive-contents))
@@ -1090,8 +1085,11 @@
(package-refresh-contents))
(list (intern (completing-read
"Install package: "
- (mapcar (lambda (elt) (symbol-name (car elt)))
- package-archive-contents)
+ (delq nil
+ (mapcar (lambda (elt)
+ (unless (package-installed-p (car elt))
+ (symbol-name (car elt))))
+ package-archive-contents))
nil t)))))
(package-download-transaction
(if (package-desc-p pkg)
^ permalink raw reply [flat|nested] 14+ messages in thread
* bug#16762: Installed packages are not considered part of the archive contents
2014-03-24 6:20 ` Dmitry Gutov
@ 2014-03-24 8:10 ` Johan Andersson
2014-03-24 13:07 ` Stefan
1 sibling, 0 replies; 14+ messages in thread
From: Johan Andersson @ 2014-03-24 8:10 UTC (permalink / raw)
To: Dmitry Gutov; +Cc: 16762
[-- Attachment #1: Type: text/plain, Size: 2710 bytes --]
> Johan?
I actually found a way around the issue because I had no time waiting for
it to get fixed.
But I still think that this should be fixed for the reason I mention that
other packages, such as Epl depends on the (non) package API. This specific
issue will break for example this function:
https://github.com/cask/epl/blob/master/epl.el#L447-L452
On Mon, Mar 24, 2014 at 7:20 AM, Dmitry Gutov <dgutov@yandex.ru> wrote:
> On 24.03.2014 02:55, Stefan wrote:
>
> Does it affect the output of M-x package-list RET?
>>
>
> It's identical (checked with diff).
>
> What about the package-menu-mark-upgrades?
>>
>
> It works. Don't see any problems.
>
>
> If we can keep the already installed packages, couldn't we also keep the
>> already built-in packages?
>>
>
> Yes, seems so. I hadn't tried this before because the previous behavior
> was to omit them (I think), but the patch below seems to work fine WRT the
> questions above.
>
>
> === modified file 'lisp/emacs-lisp/package.el'
> --- lisp/emacs-lisp/package.el 2014-03-23 08:35:56 +0000
> +++ lisp/emacs-lisp/package.el 2014-03-24 06:11:52 +0000
> @@ -1047,14 +1047,9 @@
>
> (existing-packages (assq name package-archive-contents))
> (pinned-to-archive (assoc name package-pinned-packages)))
> (cond
> - ;; Skip entirely if pinned to another archive or already installed.
> - ((or (and pinned-to-archive
> - (not (equal (cdr pinned-to-archive) archive)))
> - (let ((bi (assq name package--builtin-versions)))
>
> - (and bi (version-list-= version (cdr bi))))
> - (let ((ins (cdr (assq name package-alist))))
> - (and ins (version-list-= version
> - (package-desc-version (car ins))))))
> + ;; Skip entirely if pinned to another archive.
> + ((and pinned-to-archive
> + (not (equal (cdr pinned-to-archive) archive)))
>
> nil)
> ((not existing-packages)
> (push (list name pkg-desc) package-archive-contents))
> @@ -1090,8 +1085,11 @@
>
> (package-refresh-contents))
> (list (intern (completing-read
> "Install package: "
> - (mapcar (lambda (elt) (symbol-name (car elt)))
> - package-archive-contents)
> + (delq nil
> + (mapcar (lambda (elt)
> + (unless (package-installed-p (car
> elt))
> + (symbol-name (car elt))))
> + package-archive-contents))
> nil t)))))
> (package-download-transaction
> (if (package-desc-p pkg)
>
>
>
[-- Attachment #2: Type: text/html, Size: 3917 bytes --]
^ permalink raw reply [flat|nested] 14+ messages in thread
* bug#16762: Installed packages are not considered part of the archive contents
2014-03-24 6:20 ` Dmitry Gutov
2014-03-24 8:10 ` Johan Andersson
@ 2014-03-24 13:07 ` Stefan
2014-03-24 14:32 ` Dmitry Gutov
1 sibling, 1 reply; 14+ messages in thread
From: Stefan @ 2014-03-24 13:07 UTC (permalink / raw)
To: Dmitry Gutov; +Cc: Johan Andersson, 16762
> === modified file 'lisp/emacs-lisp/package.el'
> --- lisp/emacs-lisp/package.el 2014-03-23 08:35:56 +0000
> +++ lisp/emacs-lisp/package.el 2014-03-24 06:11:52 +0000
> @@ -1047,14 +1047,9 @@
> (existing-packages (assq name package-archive-contents))
> (pinned-to-archive (assoc name package-pinned-packages)))
> (cond
> - ;; Skip entirely if pinned to another archive or already installed.
> - ((or (and pinned-to-archive
> - (not (equal (cdr pinned-to-archive) archive)))
> - (let ((bi (assq name package--builtin-versions)))
> - (and bi (version-list-= version (cdr bi))))
> - (let ((ins (cdr (assq name package-alist))))
> - (and ins (version-list-= version
> - (package-desc-version (car ins))))))
> + ;; Skip entirely if pinned to another archive.
> + ((and pinned-to-archive
> + (not (equal (cdr pinned-to-archive) archive)))
> nil)
> ((not existing-packages)
> (push (list name pkg-desc) package-archive-contents))
> @@ -1090,8 +1085,11 @@
> (package-refresh-contents))
> (list (intern (completing-read
> "Install package: "
> - (mapcar (lambda (elt) (symbol-name (car elt)))
> - package-archive-contents)
> + (delq nil
> + (mapcar (lambda (elt)
> + (unless (package-installed-p (car elt))
> + (symbol-name (car elt))))
> + package-archive-contents))
Doesn't the above package-installed-p mean that we can't complete the
name of an already installed package we want to upgrade?
Stefan
^ permalink raw reply [flat|nested] 14+ messages in thread
* bug#16762: Installed packages are not considered part of the archive contents
2014-03-24 13:07 ` Stefan
@ 2014-03-24 14:32 ` Dmitry Gutov
2014-03-24 18:17 ` Stefan
0 siblings, 1 reply; 14+ messages in thread
From: Dmitry Gutov @ 2014-03-24 14:32 UTC (permalink / raw)
To: Stefan; +Cc: Johan Andersson, 16762
On 24.03.2014 15:07, Stefan wrote:
> Doesn't the above package-installed-p mean that we can't complete the
> name of an already installed package we want to upgrade?
Before the patch, `package-install' would be a no-op for a currently
installed package, even if a new version is available (because when the
PKG argument is not a pkg-desc, it's passed to
`package-compute-transaction' in the list of requirements without
version, and it's immediately satisfied, requiring no package installation).
Considering that `package-install' wouldn't remove the currently
installed version, I don't think we'd want it to work for upgrades, anyway.
^ permalink raw reply [flat|nested] 14+ messages in thread
* bug#16762: Installed packages are not considered part of the archive contents
2014-03-24 14:32 ` Dmitry Gutov
@ 2014-03-24 18:17 ` Stefan
2014-03-24 22:39 ` Dmitry Gutov
0 siblings, 1 reply; 14+ messages in thread
From: Stefan @ 2014-03-24 18:17 UTC (permalink / raw)
To: Dmitry Gutov; +Cc: Johan Andersson, 16762
>> Doesn't the above package-installed-p mean that we can't complete the
>> name of an already installed package we want to upgrade?
> Before the patch, `package-install' would be a no-op for a currently
> installed package, even if a new version is available (because when the PKG
> argument is not a pkg-desc, it's passed to `package-compute-transaction' in
> the list of requirements without version, and it's immediately satisfied,
> requiring no package installation).
> Considering that `package-install' wouldn't remove the currently installed
> version, I don't think we'd want it to work for upgrades, anyway.
OK. Then let's try it on `emacs-24' and keep our fingers crossed,
Stefan
^ permalink raw reply [flat|nested] 14+ messages in thread
* bug#16762: Installed packages are not considered part of the archive contents
2014-03-24 18:17 ` Stefan
@ 2014-03-24 22:39 ` Dmitry Gutov
2014-03-28 7:22 ` Johan Andersson
0 siblings, 1 reply; 14+ messages in thread
From: Dmitry Gutov @ 2014-03-24 22:39 UTC (permalink / raw)
To: Stefan; +Cc: 16762-done, Johan Andersson
Version: 24.4
On 24.03.2014 20:17, Stefan wrote:
> OK. Then let's try it on `emacs-24' and keep our fingers crossed,
Fingers crossed now. emacs-24, r116854.
^ permalink raw reply [flat|nested] 14+ messages in thread
* bug#16762: Installed packages are not considered part of the archive contents
2014-03-24 22:39 ` Dmitry Gutov
@ 2014-03-28 7:22 ` Johan Andersson
0 siblings, 0 replies; 14+ messages in thread
From: Johan Andersson @ 2014-03-28 7:22 UTC (permalink / raw)
To: Dmitry Gutov; +Cc: 16762-done
[-- Attachment #1: Type: text/plain, Size: 283 bytes --]
Thanks for fixing it guys!
On Mon, Mar 24, 2014 at 11:39 PM, Dmitry Gutov <dgutov@yandex.ru> wrote:
> Version: 24.4
>
>
> On 24.03.2014 20:17, Stefan wrote:
>
> OK. Then let's try it on `emacs-24' and keep our fingers crossed,
>>
>
> Fingers crossed now. emacs-24, r116854.
>
>
[-- Attachment #2: Type: text/html, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2014-03-28 7:22 UTC | newest]
Thread overview: 14+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-02-15 16:06 bug#16762: Installed packages are not considered part of the archive contents Johan Andersson
2014-03-21 6:47 ` Dmitry Gutov
2014-03-21 15:33 ` Stefan Monnier
2014-03-21 23:07 ` Dmitry Gutov
2014-03-22 2:46 ` Stefan Monnier
2014-03-23 8:16 ` Dmitry Gutov
2014-03-24 0:55 ` Stefan
2014-03-24 6:20 ` Dmitry Gutov
2014-03-24 8:10 ` Johan Andersson
2014-03-24 13:07 ` Stefan
2014-03-24 14:32 ` Dmitry Gutov
2014-03-24 18:17 ` Stefan
2014-03-24 22:39 ` Dmitry Gutov
2014-03-28 7:22 ` Johan Andersson
Code repositories for project(s) associated with this public inbox
https://git.savannah.gnu.org/cgit/emacs.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).