kvm.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [kvm-unit-tests PATCH] Makefile: fix use of PWD in target "all"
@ 2020-12-11 19:43 Ricardo Koller
  2020-12-11 19:58 ` Paolo Bonzini
  0 siblings, 1 reply; 2+ messages in thread
From: Ricardo Koller @ 2020-12-11 19:43 UTC (permalink / raw)
  To: kvm
  Cc: Andrew Jones, Paolo Bonzini, Ricardo Koller, Jim Mattson, Oliver Upton

The "all" target creates the build-head file in the wrong location when
using "make -C" or "sudo make". The reason is that the PWD environment
variable gets the value of the current directory when calling "make -C"
(before the -C changes directories), or is unset in the case of "sudo
make".  Note that the PWD is not changed by the previous "cd $(SRCDIR)".

	/a/b/c $ make -C ../kvm-unit-tests
	=====> creates /a/b/c/build-head

	/a/b/kvm-unit-tests $ sudo make
	=====> creates /build-head
		(note the root)

The consequence of this is that the standalone script can't find the
build-head file:

	/a/b/c $ make -C kvm-unit-tests standalone
	cat: build-head: No such file or directory
	...

	/a/b/kvm-unit-tests $ sudo make standalone
	cat: build-head: No such file or directory
	...

The fix is to not use PWD. "cd $SRCDIR && git rev-parse" is run in a
subshell in order to not break out-of-tree builds, which expect
build-head in the current directory (/a/b/c/build-head below).

Tested:
	out-of-tree build:
	/a/b/c $ ../kvm-unit-tests/configure && make standalone
	
	sudo make:
	/a/b/kvm-unit-tests $ ./configure && sudo make standalone
	
	make -C:
	/a/b/c $ (cd ../kvm-unit-tests && ./configure) && \
				make -C ../kvm-unit-tests standalone

Signed-off-by: Ricardo Koller <ricarkol@google.com>
Reviewed-by: Jim Mattson <jmattson@google.com>
Reviewed-by: Oliver Upton <oupton@google.com>
---
 Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Makefile b/Makefile
index 0e21a49..e0828fe 100644
--- a/Makefile
+++ b/Makefile
@@ -101,7 +101,7 @@ directories:
 
 -include */.*.d */*/.*.d
 
-all: directories $(shell cd $(SRCDIR) && git rev-parse --verify --short=8 HEAD >$(PWD)/build-head 2>/dev/null)
+all: directories $(shell (cd $(SRCDIR) && git rev-parse --verify --short=8 HEAD) >build-head 2>/dev/null)
 
 standalone: all
 	@scripts/mkstandalone.sh
-- 
2.29.2.576.ga3fc446d84-goog


^ permalink raw reply related	[flat|nested] 2+ messages in thread

* Re: [kvm-unit-tests PATCH] Makefile: fix use of PWD in target "all"
  2020-12-11 19:43 [kvm-unit-tests PATCH] Makefile: fix use of PWD in target "all" Ricardo Koller
@ 2020-12-11 19:58 ` Paolo Bonzini
  0 siblings, 0 replies; 2+ messages in thread
From: Paolo Bonzini @ 2020-12-11 19:58 UTC (permalink / raw)
  To: Ricardo Koller, kvm; +Cc: Andrew Jones, Jim Mattson, Oliver Upton

On 11/12/20 20:43, Ricardo Koller wrote:
> The "all" target creates the build-head file in the wrong location when
> using "make -C" or "sudo make". The reason is that the PWD environment
> variable gets the value of the current directory when calling "make -C"
> (before the -C changes directories), or is unset in the case of "sudo
> make".  Note that the PWD is not changed by the previous "cd $(SRCDIR)".
> 
> 	/a/b/c $ make -C ../kvm-unit-tests
> 	=====> creates /a/b/c/build-head
> 
> 	/a/b/kvm-unit-tests $ sudo make
> 	=====> creates /build-head
> 		(note the root)
> 
> The consequence of this is that the standalone script can't find the
> build-head file:
> 
> 	/a/b/c $ make -C kvm-unit-tests standalone
> 	cat: build-head: No such file or directory
> 	...
> 
> 	/a/b/kvm-unit-tests $ sudo make standalone
> 	cat: build-head: No such file or directory
> 	...
> 
> The fix is to not use PWD. "cd $SRCDIR && git rev-parse" is run in a
> subshell in order to not break out-of-tree builds, which expect
> build-head in the current directory (/a/b/c/build-head below).
> 
> Tested:
> 	out-of-tree build:
> 	/a/b/c $ ../kvm-unit-tests/configure && make standalone
> 	
> 	sudo make:
> 	/a/b/kvm-unit-tests $ ./configure && sudo make standalone
> 	
> 	make -C:
> 	/a/b/c $ (cd ../kvm-unit-tests && ./configure) && \
> 				make -C ../kvm-unit-tests standalone
> 
> Signed-off-by: Ricardo Koller <ricarkol@google.com>
> Reviewed-by: Jim Mattson <jmattson@google.com>
> Reviewed-by: Oliver Upton <oupton@google.com>
> ---
>   Makefile | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/Makefile b/Makefile
> index 0e21a49..e0828fe 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -101,7 +101,7 @@ directories:
>   
>   -include */.*.d */*/.*.d
>   
> -all: directories $(shell cd $(SRCDIR) && git rev-parse --verify --short=8 HEAD >$(PWD)/build-head 2>/dev/null)
> +all: directories $(shell (cd $(SRCDIR) && git rev-parse --verify --short=8 HEAD) >build-head 2>/dev/null)
>   
>   standalone: all
>   	@scripts/mkstandalone.sh
> 

Queued, thanks.

Paolo


^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2020-12-11 21:20 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-12-11 19:43 [kvm-unit-tests PATCH] Makefile: fix use of PWD in target "all" Ricardo Koller
2020-12-11 19:58 ` Paolo Bonzini

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 NNTP newsgroup(s).