| Message ID | 877f8udwha.fsf@gnu.org |
|---|---|
| State | New |
| Headers | show |
Roel Janssen <roel@gnu.org> writes: >> The cask file says: >> >> (development >> (depends-on "dash-functional") >> (depends-on "dash") > > @code{dash} provides @code{dash-functional}. > >> (depends-on "undercover") > > This one is needed to run the tests. > >> (depends-on "ert-runner") >> (depends-on "ert") >> (depends-on "ert-expectations") > > These should be built-in. See: > https://www.gnu.org/software/emacs/manual/html_node/ert/ > > (Or M-x ert) > >> (depends-on "el-mock") >> (depends-on "helm")) > > These should be added too, to run the tests. > > I added undercover, el-mock and helm to the inputs. […] > +(define-public emacs-org-trello > + (package > + (name "emacs-org-trello") > + (version "0.7.9") > + (source (origin > + (method url-fetch) > + (uri (string-append > + "https://github.com/org-trello/org-trello/archive/" > + version ".tar.gz")) > + (file-name (string-append name "-" version ".tar.gz")) > + (sha256 > + (base32 > + "074dka8g673bj1ck5vavbjaij5jyniygdlw51mdds005wd2br9wf")))) > + (build-system emacs-build-system) > + (propagated-inputs > + `(("emacs-deferred" ,emacs-deferred) > + ("emacs-request" ,emacs-request) > + ("emacs-dash" ,emacs-dash) > + ("emacs-s" ,emacs-s) > + ("emacs-undercover" ,emacs-undercover) > + ("emacs-el-mock" ,emacs-el-mock) > + ("emacs-helm" ,emacs-helm))) Hmm, now I wonder… I just looked at emacs.scm and only one package there has “emacs-undercover” as an input (and it looks like this was a mistake). We seem not to add test inputs to Emacs packages. @Alex: could you please confirm this? Should these inputs be mentioned at all? If so, is it sufficient to add them to native-inputs? ~~ Ricardo
Ricardo Wurmus (2016-10-27 16:09 +0200) wrote: > Roel Janssen <roel@gnu.org> writes: [...] >> +(define-public emacs-org-trello >> + (package >> + (name "emacs-org-trello") >> + (version "0.7.9") >> + (source (origin >> + (method url-fetch) >> + (uri (string-append >> + "https://github.com/org-trello/org-trello/archive/" >> + version ".tar.gz")) >> + (file-name (string-append name "-" version ".tar.gz")) >> + (sha256 >> + (base32 >> + "074dka8g673bj1ck5vavbjaij5jyniygdlw51mdds005wd2br9wf")))) >> + (build-system emacs-build-system) >> + (propagated-inputs >> + `(("emacs-deferred" ,emacs-deferred) >> + ("emacs-request" ,emacs-request) >> + ("emacs-dash" ,emacs-dash) >> + ("emacs-s" ,emacs-s) >> + ("emacs-undercover" ,emacs-undercover) >> + ("emacs-el-mock" ,emacs-el-mock) >> + ("emacs-helm" ,emacs-helm))) > > Hmm, now I wonder… I just looked at emacs.scm and only one package there > has “emacs-undercover” as an input (and it looks like this was a > mistake). We seem not to add test inputs to Emacs packages. > > @Alex: could you please confirm this? Should these inputs be mentioned > at all? If so, is it sufficient to add them to native-inputs? As for me, I don't see a reason to add these additional inputs, emacs-build-system will not run tests anyway. I think it is good to stick only to those dependencies that are listed in a header commentary (in "org-trello.el") or in a "-pkg.el" file if it presents ("org-trello-pkg.el") – these are the exact dependencies that are used by (M)ELPA. I wouldn't bother about the rest emacs packages that are listed in "Cask" file or are used in some other way by package developers; they don't add anything useful for the end users. So I think the original Roel's variant with 4 propagated-inputs is preferable. The rest dependencies ("undercover", "el-mock" and "helm") are used only for some developer tools which are not interesting for users, these tools are even excluded in the MELPA recipe: <https://github.com/melpa/melpa/blob/master/recipes/org-trello>
Alex Kost <alezost@gmail.com> writes: >> @Alex: could you please confirm this? Should these inputs be mentioned >> at all? If so, is it sufficient to add them to native-inputs? > > As for me, I don't see a reason to add these additional inputs, > emacs-build-system will not run tests anyway. Thank you for clarifying. Is there a reason why we don’t run the tests? We do this by default for all other packages. ~~ Ricardo
Ricardo Wurmus (2016-10-27 20:48 +0200) wrote: > Alex Kost <alezost@gmail.com> writes: > >>> @Alex: could you please confirm this? Should these inputs be mentioned >>> at all? If so, is it sufficient to add them to native-inputs? >> >> As for me, I don't see a reason to add these additional inputs, >> emacs-build-system will not run tests anyway. > > Thank you for clarifying. > > Is there a reason why we don’t run the tests? We do this by default for > all other packages. How can it be done? There is no common way for emacs packages to perform tests: only some packages have Makefile (with "check" phase), and it would be a really good luck if "make check" as is succeeded for them. Usually such projects use Cask to run tests or even some custom scripts. They may use just "ert" or "ert-running" package (or maybe something else). After all, I think that adding a 'check' phase to emacs-build-system is not worth an effort: there would be too much manual interventions in package recipes to make tests work for different emacs packages. But I better let Federico speak about «why we don’t run the tests» as he wrote emacs-build-system :-)
Alex Kost writes: > Ricardo Wurmus (2016-10-27 20:48 +0200) wrote: > >> Alex Kost <alezost@gmail.com> writes: >> >>>> @Alex: could you please confirm this? Should these inputs be mentioned >>>> at all? If so, is it sufficient to add them to native-inputs? >>> >>> As for me, I don't see a reason to add these additional inputs, >>> emacs-build-system will not run tests anyway. >> >> Thank you for clarifying. >> >> Is there a reason why we don’t run the tests? We do this by default for >> all other packages. > > How can it be done? There is no common way for emacs packages to > perform tests: only some packages have Makefile (with "check" phase), > and it would be a really good luck if "make check" as is succeeded for > them. Usually such projects use Cask to run tests or even some custom > scripts. They may use just "ert" or "ert-running" package (or maybe > something else). > > After all, I think that adding a 'check' phase to emacs-build-system is > not worth an effort: there would be too much manual interventions in > package recipes to make tests work for different emacs packages. > > But I better let Federico speak about «why we don’t run the tests» as he > wrote emacs-build-system :-) So, the only proper way to do it, is to manually add a test phase in each package definition. But then we still have to add a lot of dependencies that are only needed to run the tests, and even then the tests don't say much.. In the case of this package, the test functions can't run because they use functions that aren't loaded yet, which results in failures of the tests (while the package works just fine). Could I therefore, please, go ahead and push the patch without the additional inputs for running tests? Kind regards, Roel Janssen
On Fri, Oct 28, 2016 at 11:30 AM, Alex Kost <alezost@gmail.com> wrote: > Ricardo Wurmus (2016-10-27 20:48 +0200) wrote: > >> Alex Kost <alezost@gmail.com> writes: >> >>>> @Alex: could you please confirm this? Should these inputs be mentioned >>>> at all? If so, is it sufficient to add them to native-inputs? >>> >>> As for me, I don't see a reason to add these additional inputs, >>> emacs-build-system will not run tests anyway. >> >> Thank you for clarifying. >> >> Is there a reason why we don’t run the tests? We do this by default for >> all other packages. > > How can it be done? There is no common way for emacs packages to > perform tests: only some packages have Makefile (with "check" phase), > and it would be a really good luck if "make check" as is succeeded for > them. Usually such projects use Cask to run tests or even some custom > scripts. They may use just "ert" or "ert-running" package (or maybe > something else). > > After all, I think that adding a 'check' phase to emacs-build-system is > not worth an effort: there would be too much manual interventions in > package recipes to make tests work for different emacs packages. You said it all! Fede
Roel Janssen <roel@gnu.org> writes: > So, the only proper way to do it, is to manually add a test phase in > each package definition. But then we still have to add a lot of > dependencies that are only needed to run the tests, and even then the > tests don't say much.. In the case of this package, the test functions > can't run because they use functions that aren't loaded yet, which > results in failures of the tests (while the package works just fine). > > Could I therefore, please, go ahead and push the patch without the > additional inputs for running tests? Yes, please do! I was just curious. Sorry for delaying this! ~~ Ricardo
Ricardo Wurmus writes: > Roel Janssen <roel@gnu.org> writes: > >> So, the only proper way to do it, is to manually add a test phase in >> each package definition. But then we still have to add a lot of >> dependencies that are only needed to run the tests, and even then the >> tests don't say much.. In the case of this package, the test functions >> can't run because they use functions that aren't loaded yet, which >> results in failures of the tests (while the package works just fine). >> >> Could I therefore, please, go ahead and push the patch without the >> additional inputs for running tests? > > Yes, please do! I was just curious. Sorry for delaying this! > > ~~ Ricardo Pushed in 36e5d1038 and ed8bc028f. Thanks!
diff --git a/gnu/packages/emacs.scm b/gnu/packages/emacs.scm index 2a275d0..6af1325 100644 --- a/gnu/packages/emacs.scm +++ b/gnu/packages/emacs.scm @@ -2086,6 +2086,35 @@ number.") @code{org-mode} to be rendered as UTF-8 characters.") (license license:gpl3+))) +(define-public emacs-org-trello + (package + (name "emacs-org-trello") + (version "0.7.9") + (source (origin + (method url-fetch) + (uri (string-append + "https://github.com/org-trello/org-trello/archive/" + version ".tar.gz")) + (file-name (string-append name "-" version ".tar.gz")) + (sha256 + (base32 + "074dka8g673bj1ck5vavbjaij5jyniygdlw51mdds005wd2br9wf")))) + (build-system emacs-build-system) + (propagated-inputs + `(("emacs-deferred" ,emacs-deferred) + ("emacs-request" ,emacs-request) + ("emacs-dash" ,emacs-dash) + ("emacs-s" ,emacs-s) + ("emacs-undercover" ,emacs-undercover) + ("emacs-el-mock" ,emacs-el-mock) + ("emacs-helm" ,emacs-helm))) + (home-page "https://org-trello.github.io") + (synopsis "Emacs minor mode for interacting with Trello") + (description "This package provides an Emacs minor mode to extend +@code{org-mode} with Trello abilities. Trello is an online project +organizer.") + (license license:gpl3+))) + (define-public emacs-zenburn-theme (package (name "emacs-zenburn-theme")