* Re: [Emacs-diffs] master b689b90: Package archives now have priorities. [not found] ` <E1YC44Z-0002sG-4a@vcs.savannah.gnu.org> @ 2015-01-16 19:10 ` Glenn Morris 2015-01-16 19:33 ` Jorgen Schäfer 2015-01-16 23:48 ` Artur Malabarba 1 sibling, 1 reply; 15+ messages in thread From: Glenn Morris @ 2015-01-16 19:10 UTC (permalink / raw) To: emacs-devel; +Cc: Jorgen Schaefer Thanks for writing a test case. However, it fails: http://hydra.nixos.org/build/18817665 (wrong-type-argument arrayp ([cl-struct-package-desc ... )) Also, you forgot to make a test/ChangeLog entry. ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [Emacs-diffs] master b689b90: Package archives now have priorities. 2015-01-16 19:10 ` [Emacs-diffs] master b689b90: Package archives now have priorities Glenn Morris @ 2015-01-16 19:33 ` Jorgen Schäfer 2015-01-16 20:09 ` Glenn Morris 2015-01-19 21:40 ` Glenn Morris 0 siblings, 2 replies; 15+ messages in thread From: Jorgen Schäfer @ 2015-01-16 19:33 UTC (permalink / raw) To: Glenn Morris; +Cc: emacs-devel On Fri, Jan 16, 2015 at 8:10 PM, Glenn Morris <rgm@gnu.org> wrote: > > Thanks for writing a test case. However, it fails: > > http://hydra.nixos.org/build/18817665 > > (wrong-type-argument arrayp ([cl-struct-package-desc ... )) Thanks. I had serious trouble getting the test suite to run locally, so I tried to make it work with manual execution. Seems I failed at that. What's the correct way of invoking the test suite? "make check" results in 46 test failures here. Also, can I run individual tests from the command line easily? Especially the helper macros in package-test.el seem a bit fickly with interactive use. > Also, you forgot to make a test/ChangeLog entry. Oh, there's another ChangeLog file. I hope 78e6ccc fixes the test and adds the commit message. Regards, Jorgen ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [Emacs-diffs] master b689b90: Package archives now have priorities. 2015-01-16 19:33 ` Jorgen Schäfer @ 2015-01-16 20:09 ` Glenn Morris 2015-01-19 21:40 ` Glenn Morris 1 sibling, 0 replies; 15+ messages in thread From: Glenn Morris @ 2015-01-16 20:09 UTC (permalink / raw) To: Jorgen Schäfer; +Cc: emacs-devel Jorgen Schäfer wrote: > What's the correct way of invoking the test suite? "make check" > results in 46 test failures here. make check is correct. Sadly the test-suite often seems to be in a failing state. I don't have anything like 46 failures though. I can only suggest making bug reports. > Also, can I run individual tests from the command line easily? See the top of the Makefile. Eg make -C test/automated package-test ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [Emacs-diffs] master b689b90: Package archives now have priorities. 2015-01-16 19:33 ` Jorgen Schäfer 2015-01-16 20:09 ` Glenn Morris @ 2015-01-19 21:40 ` Glenn Morris 2015-01-20 9:03 ` Jorgen Schäfer 1 sibling, 1 reply; 15+ messages in thread From: Glenn Morris @ 2015-01-19 21:40 UTC (permalink / raw) To: Jorgen Schäfer; +Cc: emacs-devel Jorgen Schäfer wrote: > I hope 78e6ccc fixes the test No, it still fails. Can you not, err, test this? ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [Emacs-diffs] master b689b90: Package archives now have priorities. 2015-01-19 21:40 ` Glenn Morris @ 2015-01-20 9:03 ` Jorgen Schäfer 2015-01-20 15:26 ` Dmitry Gutov 2015-01-21 20:06 ` Glenn Morris 0 siblings, 2 replies; 15+ messages in thread From: Jorgen Schäfer @ 2015-01-20 9:03 UTC (permalink / raw) To: Glenn Morris; +Cc: emacs-devel On Mon, Jan 19, 2015 at 10:40 PM, Glenn Morris <rgm@gnu.org> wrote: > Jorgen Schäfer wrote: > >> I hope 78e6ccc fixes the test > > No, it still fails. Do you have a link to the hydra output so I can see what fails on hydra? As I mentioned, hydra seems to run tests in a different environment than I do on my own machine. > Can you not, err, test this? That is a tricky question. Yes, I can test this. But no, I can't make the test pass. All the package-test-install-* tests fail with an error "Autoloads file [...] lacks boilerplate". This seems to be a problem in the test suite code, and was the case before my commit. I was unable to figure out how to fix it, so I did not. As much as I could test my test code locally without the Emacs test suite, it seemed to pass. I was hoping the different test environment on hydra would fix the autoloads problem. I do not know what the error now was on hydra, so I can't say, but if it is the autoloads error, I do not know how to fix it. Still, I figured a test case that would most likely pass if the test suite was not broken was better than no test case. Would it be better if I removed my test case again? Regards, Jorgen ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [Emacs-diffs] master b689b90: Package archives now have priorities. 2015-01-20 9:03 ` Jorgen Schäfer @ 2015-01-20 15:26 ` Dmitry Gutov 2015-01-20 17:11 ` Jorgen Schäfer 2015-01-21 20:06 ` Glenn Morris 1 sibling, 1 reply; 15+ messages in thread From: Dmitry Gutov @ 2015-01-20 15:26 UTC (permalink / raw) To: Jorgen Schäfer, Glenn Morris; +Cc: emacs-devel On 01/20/2015 11:03 AM, Jorgen Schäfer wrote: > Yes, I can test this. But no, I can't make the test pass. Do you run 'make check'? I have only two test failures on my local machine: yours and vc-test-svn03-working-revision. > All the package-test-install-* tests fail with an error "Autoloads > file [...] lacks boilerplate". This seems to be a problem in the test > suite code, and was the case before my commit. I was unable to figure > out how to fix it, so I did not. Why don't you file a bug about it? ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [Emacs-diffs] master b689b90: Package archives now have priorities. 2015-01-20 15:26 ` Dmitry Gutov @ 2015-01-20 17:11 ` Jorgen Schäfer 2015-01-20 17:19 ` Dmitry Gutov 0 siblings, 1 reply; 15+ messages in thread From: Jorgen Schäfer @ 2015-01-20 17:11 UTC (permalink / raw) To: Dmitry Gutov; +Cc: emacs-devel On Tue, Jan 20, 2015 at 4:26 PM, Dmitry Gutov <dgutov@yandex.ru> wrote: > On 01/20/2015 11:03 AM, Jorgen Schäfer wrote: > >> Yes, I can test this. But no, I can't make the test pass. > > Do you run 'make check'? I have only two test failures on my local machine: > yours and vc-test-svn03-working-revision. Yes. As I mentioned, make -C test/automated gives me 46 test failures. >> All the package-test-install-* tests fail with an error "Autoloads >> file [...] lacks boilerplate". This seems to be a problem in the test >> suite code, and was the case before my commit. I was unable to figure >> out how to fix it, so I did not. > > Why don't you file a bug about it? I'm not even sure if it is a bug, or if I am doing something wrong. I do not have the time to investigate this in any detail. As this is indeed my test case and not this weird autoload problem I see, and I simply lack the time to work on this, I have removed my test case. Apologies to everyone for the noise. Regards, Jorgen ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [Emacs-diffs] master b689b90: Package archives now have priorities. 2015-01-20 17:11 ` Jorgen Schäfer @ 2015-01-20 17:19 ` Dmitry Gutov 0 siblings, 0 replies; 15+ messages in thread From: Dmitry Gutov @ 2015-01-20 17:19 UTC (permalink / raw) To: Jorgen Schäfer; +Cc: emacs-devel On 01/20/2015 07:11 PM, Jorgen Schäfer wrote: >> Why don't you file a bug about it? > > I'm not even sure if it is a bug, or if I am doing something wrong. So what? People file invalid bugs all the time. > As this is indeed my test case and not this weird autoload problem I > see, and I simply lack the time to work on this, I have removed my > test case. You've kept the now-untested feature, though. ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [Emacs-diffs] master b689b90: Package archives now have priorities. 2015-01-20 9:03 ` Jorgen Schäfer 2015-01-20 15:26 ` Dmitry Gutov @ 2015-01-21 20:06 ` Glenn Morris 1 sibling, 0 replies; 15+ messages in thread From: Glenn Morris @ 2015-01-21 20:06 UTC (permalink / raw) To: Jorgen Schäfer; +Cc: emacs-devel Jorgen Schäfer wrote: > All the package-test-install-* tests fail with an error "Autoloads > file [...] lacks boilerplate". It seems like your build might be broken. Personally I would do a `make boostrap'. > I was hoping the different test environment on hydra would fix the > autoloads problem. I do not know what the error now was on hydra, so I > can't say, but if it is the autoloads error, I do not know how to fix > it. http://hydra.nixos.org/build/18990023 http://hydra.nixos.org/build/18990023/log/raw Test package-test-install-prioritized condition: (ert-test-failed ((should (version-list-= '... (package-desc-version installed))) :form (version-list-= (1 3) (1 4)) :value nil)) FAILED 7/16 package-test-install-prioritized This is exactly the error I was getting on RHEL7, which is quite a different system to hydra. It seems unlikely that something like this would be platform specific. IIUC, it indicates the priority mechanism simply isn't working. > Still, I figured a test case that would most likely pass if the test > suite was not broken was better than no test case. AFAIK, the test suite is not broken. ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: master b689b90: Package archives now have priorities. [not found] ` <E1YC44Z-0002sG-4a@vcs.savannah.gnu.org> 2015-01-16 19:10 ` [Emacs-diffs] master b689b90: Package archives now have priorities Glenn Morris @ 2015-01-16 23:48 ` Artur Malabarba 2015-01-17 11:02 ` [Emacs-diffs] " Jorgen Schäfer 1 sibling, 1 reply; 15+ messages in thread From: Artur Malabarba @ 2015-01-16 23:48 UTC (permalink / raw) To: emacs-devel, Jorgen Schaefer; +Cc: emacs-diffs > +(defcustom package-archive-priorities nil > + "An alist of priorities for packages. > + > +Each element has the form (ARCHIVE-ID . PRIORITY). > + > +When installing packages, the package with the highest version > +number from the archive with the highest priority is > +selected. When higher versions are available from archives with > +lower priorities, the user has to select those manually. > + > +Archives not in this list have the priority 0." > + :type 'integer I think you meant 'alist? > +(defun package--add-to-alist (pkg-desc alist) > + "Add PKG-DESC to ALIST. > + > +Packages are grouped by name. The package descriptions are sorted > +by version number." > + (let* ((name (package-desc-name pkg-desc)) > + (priority-version (package-desc-priority-version pkg-desc)) > + (existing-packages (assq name alist))) > + (if (not existing-packages) > + (cons (list name pkg-desc) This list should be a cons, probably why the test is failing. ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [Emacs-diffs] master b689b90: Package archives now have priorities. 2015-01-16 23:48 ` Artur Malabarba @ 2015-01-17 11:02 ` Jorgen Schäfer 2015-01-17 12:12 ` Artur Malabarba 0 siblings, 1 reply; 15+ messages in thread From: Jorgen Schäfer @ 2015-01-17 11:02 UTC (permalink / raw) To: bruce.connor.am; +Cc: emacs-devel On Sat, Jan 17, 2015 at 12:48 AM, Artur Malabarba <bruce.connor.am@gmail.com> wrote: >> +(defcustom package-archive-priorities nil >> + "An alist of priorities for packages. >> + >> +Each element has the form (ARCHIVE-ID . PRIORITY). >> + >> +When installing packages, the package with the highest version >> +number from the archive with the highest priority is >> +selected. When higher versions are available from archives with >> +lower priorities, the user has to select those manually. >> + >> +Archives not in this list have the priority 0." >> + :type 'integer > I think you meant 'alist? Indeed, thanks for catching this. Fixed in d80fed. >> +(defun package--add-to-alist (pkg-desc alist) >> + "Add PKG-DESC to ALIST. >> + >> +Packages are grouped by name. The package descriptions are sorted >> +by version number." >> + (let* ((name (package-desc-name pkg-desc)) >> + (priority-version (package-desc-priority-version pkg-desc)) >> + (existing-packages (assq name alist))) >> + (if (not existing-packages) >> + (cons (list name pkg-desc) > This list should be a cons, probably why the test is failing. Why should this be a cons? The alist maps package names to ordered package descriptors – I guess (cons name (list pkg-desc)) would be clearer in intent. Regards, Jorgen ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [Emacs-diffs] master b689b90: Package archives now have priorities. 2015-01-17 11:02 ` [Emacs-diffs] " Jorgen Schäfer @ 2015-01-17 12:12 ` Artur Malabarba 2015-01-17 12:27 ` Jorgen Schäfer 2015-01-17 14:11 ` Stefan Monnier 0 siblings, 2 replies; 15+ messages in thread From: Artur Malabarba @ 2015-01-17 12:12 UTC (permalink / raw) To: Jorgen Schäfer; +Cc: emacs-devel >>> +(defun package--add-to-alist (pkg-desc alist) >>> + "Add PKG-DESC to ALIST. >>> + >>> +Packages are grouped by name. The package descriptions are sorted >>> +by version number." >>> + (let* ((name (package-desc-name pkg-desc)) >>> + (priority-version (package-desc-priority-version pkg-desc)) >>> + (existing-packages (assq name alist))) >>> + (if (not existing-packages) >>> + (cons (list name pkg-desc) >> This list should be a cons, probably why the test is failing. > > Why should this be a cons? The alist maps package names to ordered > package descriptors – I guess (cons name (list pkg-desc)) would be > clearer in intent. Reading your code again, I see there are places where this `list' is correct, but there's also one point where I think it's wrong. Note the following diff section. It used to push `(cons (package-desc-name pkg-desc) pkg-desc)', and now it uses `package--add-to-alist' which pushes a list instead of a cons. Do you see what I mean? (defun package--download-one-archive (archive file) "Retrieve an archive file FILE from ARCHIVE, and cache it. ARCHIVE should be a cons cell of the form (NAME . LOCATION), @@ -1991,18 +2035,18 @@ If optional arg BUTTON is non-nil, describe its associated package." ;; ENTRY is (PKG-DESC [NAME VERSION STATUS DOC]) (let ((pkg-desc (car entry)) (status (aref (cadr entry) 2))) - (cond ((member status '("installed" "unsigned")) - (push pkg-desc installed)) - ((member status '("available" "new")) - (push (cons (package-desc-name pkg-desc) pkg-desc) - available))))) + (cond ((member status '("installed" "unsigned")) + (push pkg-desc installed)) + ((member status '("available" "new")) + (setq available (package--add-to-alist pkg-desc available)))))) Maybe the solution is not to change `package--add-to-alist' but simply to not use it here. I also have a very tiny peeve here. Could we name this function `package--alist-append' or something like this? It's just that `add-to-alist' really reminds me of `add-to-list', which has a very different effect (the list variable is the first argument, it's referenced by name instead of value, and it's changed destructively). If you don't agree I won't push it. :-) ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [Emacs-diffs] master b689b90: Package archives now have priorities. 2015-01-17 12:12 ` Artur Malabarba @ 2015-01-17 12:27 ` Jorgen Schäfer 2015-01-17 14:11 ` Stefan Monnier 1 sibling, 0 replies; 15+ messages in thread From: Jorgen Schäfer @ 2015-01-17 12:27 UTC (permalink / raw) To: Artur Malabarba; +Cc: emacs-devel On Sat, Jan 17, 2015 at 1:12 PM, Artur Malabarba <bruce.connor.am@gmail.com> wrote: >>>> +(defun package--add-to-alist (pkg-desc alist) >>>> + "Add PKG-DESC to ALIST. >>>> + >>>> +Packages are grouped by name. The package descriptions are sorted >>>> +by version number." >>>> + (let* ((name (package-desc-name pkg-desc)) >>>> + (priority-version (package-desc-priority-version pkg-desc)) >>>> + (existing-packages (assq name alist))) >>>> + (if (not existing-packages) >>>> + (cons (list name pkg-desc) >>> This list should be a cons, probably why the test is failing. >> >> Why should this be a cons? The alist maps package names to ordered >> package descriptors – I guess (cons name (list pkg-desc)) would be >> clearer in intent. > > Reading your code again, I see there are places where this `list' is > correct, but there's also one point where I think it's wrong. > Note the following diff section. It used to push `(cons > (package-desc-name pkg-desc) pkg-desc)', and now it uses > `package--add-to-alist' which pushes a list instead of a cons. Do you > see what I mean? Yes – the code used to keep only one version around, which made finding the correct version by archive priority more tricky, so I changed it to keep a full list. That was also what another part of the code already did, so I could unify the two into a single function. All accesses to that list should be updated appropriately. (The whole logic in package.el is quite convoluted and has a lot of duplication with subtly different semantics, so I would not be too surprised if there are some edge cases I missed, though.) > I also have a very tiny peeve here. Could we name this function > `package--alist-append' or something like this? > > It's just that `add-to-alist' really reminds me of `add-to-list', > which has a very different effect (the list variable is the first > argument, it's referenced by name instead of value, and it's changed > destructively). > If you don't agree I won't push it. :-) Go ahead, I do not have any strong feelings about the names. Regards, Jorgen ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [Emacs-diffs] master b689b90: Package archives now have priorities. 2015-01-17 12:12 ` Artur Malabarba 2015-01-17 12:27 ` Jorgen Schäfer @ 2015-01-17 14:11 ` Stefan Monnier 2015-01-17 21:44 ` Stephen J. Turnbull 1 sibling, 1 reply; 15+ messages in thread From: Stefan Monnier @ 2015-01-17 14:11 UTC (permalink / raw) To: Artur Malabarba; +Cc: Jorgen Schäfer, emacs-devel > I also have a very tiny peeve here. Could we name this function > `package--alist-append' or something like this? > It's just that `add-to-alist' really reminds me of `add-to-list', > which has a very different effect (the list variable is the first > argument, it's referenced by name instead of value, and it's changed > destructively). FWIW, I had the same reaction (and since I really dislike add-to-list's use "call by symbol", I actully went to double check what package--add-to-alist did, fearing it made the same design mistake). Stefan ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [Emacs-diffs] master b689b90: Package archives now have priorities. 2015-01-17 14:11 ` Stefan Monnier @ 2015-01-17 21:44 ` Stephen J. Turnbull 0 siblings, 0 replies; 15+ messages in thread From: Stephen J. Turnbull @ 2015-01-17 21:44 UTC (permalink / raw) To: Stefan Monnier; +Cc: Jorgen Schäfer, Artur Malabarba, emacs-devel Stefan Monnier writes: > FWIW, I had the same reaction (and since I really dislike > add-to-list's use "call by symbol", So fix it and call it "push-unique" or something like that. (Yes, I know about the APPEND argument, but I wonder how often that is computed -- if never, then "push" is the default.) ^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2015-01-21 20:06 UTC | newest] Thread overview: 15+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- [not found] <20150116102411.11014.8945@vcs.savannah.gnu.org> [not found] ` <E1YC44Z-0002sG-4a@vcs.savannah.gnu.org> 2015-01-16 19:10 ` [Emacs-diffs] master b689b90: Package archives now have priorities Glenn Morris 2015-01-16 19:33 ` Jorgen Schäfer 2015-01-16 20:09 ` Glenn Morris 2015-01-19 21:40 ` Glenn Morris 2015-01-20 9:03 ` Jorgen Schäfer 2015-01-20 15:26 ` Dmitry Gutov 2015-01-20 17:11 ` Jorgen Schäfer 2015-01-20 17:19 ` Dmitry Gutov 2015-01-21 20:06 ` Glenn Morris 2015-01-16 23:48 ` Artur Malabarba 2015-01-17 11:02 ` [Emacs-diffs] " Jorgen Schäfer 2015-01-17 12:12 ` Artur Malabarba 2015-01-17 12:27 ` Jorgen Schäfer 2015-01-17 14:11 ` Stefan Monnier 2015-01-17 21:44 ` Stephen J. Turnbull
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).