Patchwork makefile: run Rust tests if cargo is installed

login
register
mail settings
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

Raphaël Gomès - Aug. 21, 2019, 4:09 p.m.
# 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.
Raphaël Gomès - Aug. 21, 2019, 4:51 p.m.
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
Yuya Nishihara - Aug. 21, 2019, 11:31 p.m.
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
Yuya Nishihara - Aug. 21, 2019, 11:37 p.m.
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.
Anton Shestakov - Aug. 22, 2019, 4:44 a.m.
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.
Raphaël Gomès - Aug. 22, 2019, 5:47 a.m.
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
Raphaël Gomès - Aug. 22, 2019, 6:11 a.m.
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
Yuya Nishihara - Aug. 22, 2019, 12:41 p.m.
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.
Yuya Nishihara - Aug. 22, 2019, 12:50 p.m.
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
Raphaël Gomès - Aug. 22, 2019, 12:52 p.m.
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) $@