| Submitter | Raphaël Gomès |
|---|---|
| Date | Aug. 21, 2019, 4:09 p.m. |
| Message ID | <5b9efc6098154fb5079f.1566403773@alphare-carbon.lan> |
| Download | mbox | patch |
| Permalink | /patch/41376/ |
| State | Superseded |
| Headers | show |
Comments
I figured patchbomb would add [PATCH STABLE], but I was wrong, it has to be specified. So this patch is for the stable branch. On 8/21/19 6:09 PM, Raphaël Gomès wrote: > # HG changeset patch > # User Raphaël Gomès <rgomes@octobus.net> > # Date 1566403010 -7200 > # Wed Aug 21 17:56:50 2019 +0200 > # Branch stable > # Node ID 5b9efc6098154fb5079f06b3c485b58363e0f43d > # Parent 302dbc9d52beeb2ef677aa18b3fa005bbce2134e > makefile: run Rust tests if cargo is installed > > While no particular minimum toolchain version is targeted as of yet, this > serves as a first step to make more people/machines run the Rust tests. > > diff -r 302dbc9d52be -r 5b9efc609815 Makefile > --- a/Makefile Fri Aug 16 15:41:53 2019 +0300 > +++ b/Makefile Wed Aug 21 17:56:50 2019 +0200 > @@ -110,8 +110,11 @@ > check: tests > > tests: > - cd tests && $(PYTHON) run-tests.py $(TESTFLAGS) > - > + if [ -x "$$(command -v cargo)" ]; then \ > + # Run Rust tests if cargo is installed > + cd $(HGROOT)/rust/hg-cpython && cargo test --quiet --all; \ > + fi; > + cd tests && $(PYTHON) run-tests.py $(TESTFLAGS) && \ > test-%: > cd tests && $(PYTHON) run-tests.py $(TESTFLAGS) $@ > > _______________________________________________ > Mercurial-devel mailing list > Mercurial-devel@mercurial-scm.org > https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
On Wed, 21 Aug 2019 18:09:33 +0200, Raphaël Gomès wrote: > # HG changeset patch > # User Raphaël Gomès <rgomes@octobus.net> > # Date 1566403010 -7200 > # Wed Aug 21 17:56:50 2019 +0200 > # Branch stable > # Node ID 5b9efc6098154fb5079f06b3c485b58363e0f43d > # Parent 302dbc9d52beeb2ef677aa18b3fa005bbce2134e > makefile: run Rust tests if cargo is installed > > While no particular minimum toolchain version is targeted as of yet, this > serves as a first step to make more people/machines run the Rust tests. > > diff -r 302dbc9d52be -r 5b9efc609815 Makefile > --- a/Makefile Fri Aug 16 15:41:53 2019 +0300 > +++ b/Makefile Wed Aug 21 17:56:50 2019 +0200 > @@ -110,8 +110,11 @@ > check: tests > > tests: > - cd tests && $(PYTHON) run-tests.py $(TESTFLAGS) > - > + if [ -x "$$(command -v cargo)" ]; then \ > + # Run Rust tests if cargo is installed > + cd $(HGROOT)/rust/hg-cpython && cargo test --quiet --all; \ > + fi; Perhaps, we'll want to specify $(CARGO) command to be used. $ make CARGO='rustup run nightly cargo' # in Makefile, provide the default CARGO = cargo
On Thu, 22 Aug 2019 08:31:50 +0900, Yuya Nishihara wrote: > On Wed, 21 Aug 2019 18:09:33 +0200, Raphaël Gomès wrote: > > # HG changeset patch > > # User Raphaël Gomès <rgomes@octobus.net> > > # Date 1566403010 -7200 > > # Wed Aug 21 17:56:50 2019 +0200 > > # Branch stable > > # Node ID 5b9efc6098154fb5079f06b3c485b58363e0f43d > > # Parent 302dbc9d52beeb2ef677aa18b3fa005bbce2134e > > makefile: run Rust tests if cargo is installed > > > > While no particular minimum toolchain version is targeted as of yet, this > > serves as a first step to make more people/machines run the Rust tests. > > > > diff -r 302dbc9d52be -r 5b9efc609815 Makefile > > --- a/Makefile Fri Aug 16 15:41:53 2019 +0300 > > +++ b/Makefile Wed Aug 21 17:56:50 2019 +0200 > > @@ -110,8 +110,11 @@ > > check: tests > > > > tests: > > - cd tests && $(PYTHON) run-tests.py $(TESTFLAGS) > > - > > + if [ -x "$$(command -v cargo)" ]; then \ And maybe -n ?. '-x ""' (empty string) seems wrong.
On Wed, 21 Aug 2019 18:09:33 +0200 Raphaël Gomès <raphael.gomes@octobus.net> wrote: > # HG changeset patch > # User Raphaël Gomès <rgomes@octobus.net> > # Date 1566403010 -7200 > # Wed Aug 21 17:56:50 2019 +0200 > # Branch stable > # Node ID 5b9efc6098154fb5079f06b3c485b58363e0f43d > # Parent 302dbc9d52beeb2ef677aa18b3fa005bbce2134e > makefile: run Rust tests if cargo is installed > > While no particular minimum toolchain version is targeted as of yet, this > serves as a first step to make more people/machines run the Rust tests. > > diff -r 302dbc9d52be -r 5b9efc609815 Makefile > --- a/Makefile Fri Aug 16 15:41:53 2019 +0300 > +++ b/Makefile Wed Aug 21 17:56:50 2019 +0200 > @@ -110,8 +110,11 @@ > check: tests > > tests: > - cd tests && $(PYTHON) run-tests.py $(TESTFLAGS) > - > + if [ -x "$$(command -v cargo)" ]; then \ > + # Run Rust tests if cargo is installed > + cd $(HGROOT)/rust/hg-cpython && cargo test --quiet --all; \ > + fi; > + cd tests && $(PYTHON) run-tests.py $(TESTFLAGS) && \ > test-%: I most likely forgot all the intricacies of make syntax, but this looks like a shell command is getting chained by && with a make target? Also an unrelated change removes an empty line between "tests" and "test-%" targets.
I'm not sure I agree, checking for execute this way seems very common and looks more semantic to me. On 8/22/19 1:37 AM, Yuya Nishihara wrote: > On Thu, 22 Aug 2019 08:31:50 +0900, Yuya Nishihara wrote: >> On Wed, 21 Aug 2019 18:09:33 +0200, Raphaël Gomès wrote: >>> # HG changeset patch >>> # User Raphaël Gomès <rgomes@octobus.net> >>> # Date 1566403010 -7200 >>> # Wed Aug 21 17:56:50 2019 +0200 >>> # Branch stable >>> # Node ID 5b9efc6098154fb5079f06b3c485b58363e0f43d >>> # Parent 302dbc9d52beeb2ef677aa18b3fa005bbce2134e >>> makefile: run Rust tests if cargo is installed >>> >>> While no particular minimum toolchain version is targeted as of yet, this >>> serves as a first step to make more people/machines run the Rust tests. >>> >>> diff -r 302dbc9d52be -r 5b9efc609815 Makefile >>> --- a/Makefile Fri Aug 16 15:41:53 2019 +0300 >>> +++ b/Makefile Wed Aug 21 17:56:50 2019 +0200 >>> @@ -110,8 +110,11 @@ >>> check: tests >>> >>> tests: >>> - cd tests && $(PYTHON) run-tests.py $(TESTFLAGS) >>> - >>> + if [ -x "$$(command -v cargo)" ]; then \ > And maybe -n ?. '-x ""' (empty string) seems wrong. > _______________________________________________ > Mercurial-devel mailing list > Mercurial-devel@mercurial-scm.org > https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Good catch. I somehow screwed up twice in the same patch with the comment and this bit. The original patch did not even apply locally, go figure. My follow up should address that. On 8/22/19 6:44 AM, Anton Shestakov wrote: > On Wed, 21 Aug 2019 18:09:33 +0200 > Raphaël Gomès <raphael.gomes@octobus.net> wrote: > >> # HG changeset patch >> # User Raphaël Gomès <rgomes@octobus.net> >> # Date 1566403010 -7200 >> # Wed Aug 21 17:56:50 2019 +0200 >> # Branch stable >> # Node ID 5b9efc6098154fb5079f06b3c485b58363e0f43d >> # Parent 302dbc9d52beeb2ef677aa18b3fa005bbce2134e >> makefile: run Rust tests if cargo is installed >> >> While no particular minimum toolchain version is targeted as of yet, this >> serves as a first step to make more people/machines run the Rust tests. >> >> diff -r 302dbc9d52be -r 5b9efc609815 Makefile >> --- a/Makefile Fri Aug 16 15:41:53 2019 +0300 >> +++ b/Makefile Wed Aug 21 17:56:50 2019 +0200 >> @@ -110,8 +110,11 @@ >> check: tests >> >> tests: >> - cd tests && $(PYTHON) run-tests.py $(TESTFLAGS) >> - >> + if [ -x "$$(command -v cargo)" ]; then \ >> + # Run Rust tests if cargo is installed >> + cd $(HGROOT)/rust/hg-cpython && cargo test --quiet --all; \ >> + fi; >> + cd tests && $(PYTHON) run-tests.py $(TESTFLAGS) && \ >> test-%: > I most likely forgot all the intricacies of make syntax, but this looks > like a shell command is getting chained by && with a make target? > > Also an unrelated change removes an empty line between "tests" and > "test-%" targets. > _______________________________________________ > Mercurial-devel mailing list > Mercurial-devel@mercurial-scm.org > https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
On Thu, 22 Aug 2019 07:47:30 +0200, Raphaël Gomès wrote: > I'm not sure I agree, checking for execute this way seems very common > and looks more semantic to me. [ -x path ] is valid, but why do you want to test the result of `command -v cmd` again? It maybe an empty string if the cmd doesn't exist.
On Thu, 22 Aug 2019 21:41:40 +0900, Yuya Nishihara wrote: > On Thu, 22 Aug 2019 07:47:30 +0200, Raphaël Gomès wrote: > > I'm not sure I agree, checking for execute this way seems very common > > and looks more semantic to me. > > [ -x path ] is valid, but why do you want to test the result of > `command -v cmd` again? It maybe an empty string if the cmd doesn't exist. According to the doc, we can simply test the exit code. if command -v cargo >/dev/null 2>&1; then https://pubs.opengroup.org/onlinepubs/9699919799/utilities/command.html
That makes it easier indeed, thanks. On 8/22/19 2:50 PM, Yuya Nishihara wrote: > On Thu, 22 Aug 2019 21:41:40 +0900, Yuya Nishihara wrote: >> On Thu, 22 Aug 2019 07:47:30 +0200, Raphaël Gomès wrote: >>> I'm not sure I agree, checking for execute this way seems very common >>> and looks more semantic to me. >> [ -x path ] is valid, but why do you want to test the result of >> `command -v cmd` again? It maybe an empty string if the cmd doesn't exist. > According to the doc, we can simply test the exit code. > > if command -v cargo >/dev/null 2>&1; then > > https://pubs.opengroup.org/onlinepubs/9699919799/utilities/command.html
Patch
diff -r 302dbc9d52be -r 5b9efc609815 Makefile --- a/Makefile Fri Aug 16 15:41:53 2019 +0300 +++ b/Makefile Wed Aug 21 17:56:50 2019 +0200 @@ -110,8 +110,11 @@ check: tests tests: - cd tests && $(PYTHON) run-tests.py $(TESTFLAGS) - + if [ -x "$$(command -v cargo)" ]; then \ + # Run Rust tests if cargo is installed + cd $(HGROOT)/rust/hg-cpython && cargo test --quiet --all; \ + fi; + cd tests && $(PYTHON) run-tests.py $(TESTFLAGS) && \ test-%: cd tests && $(PYTHON) run-tests.py $(TESTFLAGS) $@