Re: [Emacs-diffs] trunk r115918: package.el: various fixes.

Re: [Emacs-diffs] trunk r115918: package.el: various fixes.

[Stefan Monnier]

> @@ -881,13 +881,14 @@
>    "Return true if PACKAGE, of MIN-VERSION or newer, is installed.
>  MIN-VERSION should be a version list."
>    (unless package--initialized (error "package.el is not yet initialized!"))
> +  (when (assq package package-alist)
>      (or
>       (let ((pkg-descs (cdr (assq package package-alist))))
>         (and pkg-descs
>              (version-list-<= min-version
>                               (package-desc-version (car pkg-descs)))))
>       ;; Also check built-in packages.
> -     (package-built-in-p package min-version)))
> +     (package-built-in-p package min-version))))
 
Could you explain in which circumstance you found this necessary?
This might point to a problem elsewhere,


        Stefan

Re: [Emacs-diffs] trunk r115918: package.el: various fixes.

[Bastien]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

>> @@ -881,13 +881,14 @@
>>    "Return true if PACKAGE, of MIN-VERSION or newer, is installed.
>>  MIN-VERSION should be a version list."
>>    (unless package--initialized (error "package.el is not yet initialized!"))
>> +  (when (assq package package-alist)
>>      (or
>>       (let ((pkg-descs (cdr (assq package package-alist))))
>>         (and pkg-descs
>>              (version-list-<= min-version
>>                               (package-desc-version (car pkg-descs)))))
>>       ;; Also check built-in packages.
>> -     (package-built-in-p package min-version)))
>> +     (package-built-in-p package min-version))))
>  
> Could you explain in which circumstance you found this necessary?

This is not necessary (as in "this does not fix a bug") but
since package-installed-p is more like to return nil than t*,
then it's quickier to first check (assq package package-alist)
instead of checking by the package descriptions.

* Assuming most people don't install the majority of packages,
  of course.

> This might point to a problem elsewhere,

In general, package.el goes directly at checking descriptions
instead of the names, but for checking the mere existence I
don't think it's necessary -- at least it's not anymore, now
that the structure of package-alist is that of what its docstring
describes.

-- 
 Bastien

Re: [Emacs-diffs] trunk r115918: package.el: various fixes.

[Glenn Morris]

>> +  (when (assq package package-alist)
>>      (or
>>       (let ((pkg-descs (cdr (assq package package-alist))))

This is redundant (accesses same thing twice).

>>         (and pkg-descs
>>              (version-list-<= min-version
>>                               (package-desc-version (car pkg-descs)))))
>>       ;; Also check built-in packages.
>> -     (package-built-in-p package min-version)))
>> +     (package-built-in-p package min-version))))

It also seems to make the "Also check built-in packages." branch unreachable.

Re: [Emacs-diffs] trunk r115918: package.el: various fixes.

[Bastien]

Glenn Morris <rgm@gnu.org> writes:

>>> +  (when (assq package package-alist)
>>>      (or
>>>       (let ((pkg-descs (cdr (assq package package-alist))))
>
> This is redundant (accesses same thing twice).
>
>>>         (and pkg-descs
>>>              (version-list-<= min-version
>>>                               (package-desc-version (car pkg-descs)))))
>>>       ;; Also check built-in packages.
>>> -     (package-built-in-p package min-version)))
>>> +     (package-built-in-p package min-version))))
>
> It also seems to make the "Also check built-in packages." branch
> unreachable.

Yes, fixed, thanks.

-- 
 Bastien