| Message ID | 1629908421-8543-2-git-send-email-pmorel@linux.ibm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Two fixes for KVM unit tests | expand |
On 25/08/2021 18.20, Pierre Morel wrote: > In Linux, cscope uses a wrong directory. > Simply search from the directory where the make is started. > > Signed-off-by: Pierre Morel <pmorel@linux.ibm.com> > --- > Makefile | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/Makefile b/Makefile > index f7b9f28c..c8b0d74f 100644 > --- a/Makefile > +++ b/Makefile > @@ -119,7 +119,7 @@ cscope: cscope_dirs = lib lib/libfdt lib/linux $(TEST_DIR) $(ARCH_LIBDIRS) lib/a > cscope: > $(RM) ./cscope.* > find -L $(cscope_dirs) -maxdepth 1 \ > - -name '*.[chsS]' -exec realpath --relative-base=$(PWD) {} \; | sort -u > ./cscope.files > + -name '*.[chsS]' -exec realpath --relative-base=. {} \; | sort -u > ./cscope.files Why is $PWD not pointing to the same location as "." ? Are you doing in-tree or out-of-tree builds? Thomas
On 8/26/21 7:07 AM, Thomas Huth wrote: > On 25/08/2021 18.20, Pierre Morel wrote: >> In Linux, cscope uses a wrong directory. >> Simply search from the directory where the make is started. >> >> Signed-off-by: Pierre Morel <pmorel@linux.ibm.com> >> --- >> Makefile | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/Makefile b/Makefile >> index f7b9f28c..c8b0d74f 100644 >> --- a/Makefile >> +++ b/Makefile >> @@ -119,7 +119,7 @@ cscope: cscope_dirs = lib lib/libfdt lib/linux >> $(TEST_DIR) $(ARCH_LIBDIRS) lib/a >> cscope: >> $(RM) ./cscope.* >> find -L $(cscope_dirs) -maxdepth 1 \ >> - -name '*.[chsS]' -exec realpath --relative-base=$(PWD) {} \; >> | sort -u > ./cscope.files >> + -name '*.[chsS]' -exec realpath --relative-base=. {} \; | >> sort -u > ./cscope.files > > Why is $PWD not pointing to the same location as "." ? Are you doing > in-tree or out-of-tree builds? > > Thomas > In tree. That is the point, why is PWD indicating void ? I use a bash on s390x. inside bash PWD shows current directory GNU Make is 4.2.1 on Ubuntu 18.04 This works on X with redhat and GNU make 3.82 This happens on s390x since: 51b8f0b1 2017-11-23 Andrew Jones Makefile: fix cscope target So I add Andrew as CC, I did forgot to do before. Regards, Pierre
On Fri, Aug 27, 2021 at 11:26:38AM +0200, Pierre Morel wrote: > > > On 8/26/21 7:07 AM, Thomas Huth wrote: > > On 25/08/2021 18.20, Pierre Morel wrote: > > > In Linux, cscope uses a wrong directory. > > > Simply search from the directory where the make is started. > > > > > > Signed-off-by: Pierre Morel <pmorel@linux.ibm.com> > > > --- > > > Makefile | 2 +- > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > diff --git a/Makefile b/Makefile > > > index f7b9f28c..c8b0d74f 100644 > > > --- a/Makefile > > > +++ b/Makefile > > > @@ -119,7 +119,7 @@ cscope: cscope_dirs = lib lib/libfdt lib/linux > > > $(TEST_DIR) $(ARCH_LIBDIRS) lib/a > > > cscope: > > > $(RM) ./cscope.* > > > find -L $(cscope_dirs) -maxdepth 1 \ > > > - -name '*.[chsS]' -exec realpath --relative-base=$(PWD) {} > > > \; | sort -u > ./cscope.files > > > + -name '*.[chsS]' -exec realpath --relative-base=. {} \; | > > > sort -u > ./cscope.files > > > > Why is $PWD not pointing to the same location as "." ? Are you doing > > in-tree or out-of-tree builds? > > > > Thomas > > > > In tree. > That is the point, why is PWD indicating void ? If you do 'env' I'll bet you'll see something like ... PWD= ... > I use a bash on s390x. > inside bash PWD shows current directory > GNU Make is 4.2.1 on Ubuntu 18.04 > > This works on X with redhat and GNU make 3.82 > > This happens on s390x since: > 51b8f0b1 2017-11-23 Andrew Jones Makefile: fix cscope target No surprise there, that's when the $(PWD) use was first introduced. > > So I add Andrew as CC, I did forgot to do before. > I'll send a patch changing $(PWD) to $(shell pwd) Thanks, drew
On 27/08/2021 11.26, Pierre Morel wrote: > > > On 8/26/21 7:07 AM, Thomas Huth wrote: >> On 25/08/2021 18.20, Pierre Morel wrote: >>> In Linux, cscope uses a wrong directory. >>> Simply search from the directory where the make is started. >>> >>> Signed-off-by: Pierre Morel <pmorel@linux.ibm.com> >>> --- >>> Makefile | 2 +- >>> 1 file changed, 1 insertion(+), 1 deletion(-) >>> >>> diff --git a/Makefile b/Makefile >>> index f7b9f28c..c8b0d74f 100644 >>> --- a/Makefile >>> +++ b/Makefile >>> @@ -119,7 +119,7 @@ cscope: cscope_dirs = lib lib/libfdt lib/linux >>> $(TEST_DIR) $(ARCH_LIBDIRS) lib/a >>> cscope: >>> $(RM) ./cscope.* >>> find -L $(cscope_dirs) -maxdepth 1 \ >>> - -name '*.[chsS]' -exec realpath --relative-base=$(PWD) {} \; | >>> sort -u > ./cscope.files >>> + -name '*.[chsS]' -exec realpath --relative-base=. {} \; | sort >>> -u > ./cscope.files >> >> Why is $PWD not pointing to the same location as "." ? Are you doing >> in-tree or out-of-tree builds? >> >> Thomas >> > > In tree. > That is the point, why is PWD indicating void ? > I use a bash on s390x. > inside bash PWD shows current directory > GNU Make is 4.2.1 on Ubuntu 18.04 Hmm, looking at the code twice, $(PWD) is used a a Make variable, not as a shell variable. For using a shell variable, it should use double $$ instead ... So yes, this is broken since $(PWD) does not seem to be an official Make variable. There is $(CURDIR) which could do the job, too, but I guess "." is just fine as well. Andrew, what do you think? Thomas
On 27/08/21 12:22, Andrew Jones wrote: >> 51b8f0b1 2017-11-23 Andrew Jones Makefile: fix cscope target > No surprise there, that's when the $(PWD) use was first introduced. > >> So I add Andrew as CC, I did forgot to do before. >> > I'll send a patch changing $(PWD) to $(shell pwd) I could not find the patch using $(CURDIR) in my mailbox, though I found it on spinics.net. I fudged the following From 164507376abae4be15b0f65aa14d56f179198a99 Mon Sep 17 00:00:00 2001 From: Andrew Jones <drjones@redhat.com> Date: Fri, 27 Aug 2021 12:31:15 +0200 Subject: [PATCH kvm-unit-tests] Makefile: Don't trust PWD It's possible that PWD is already set to something which isn't the full path of the current working directory. Let's use $(CURDIR) instead, which is always correct. Suggested-by: Thomas Huth <thuth@redhat.com> Signed-off-by: Andrew Jones <drjones@redhat.com> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> diff --git a/Makefile b/Makefile index f7b9f28..6792b93 100644 --- a/Makefile +++ b/Makefile @@ -119,7 +119,7 @@ cscope: cscope_dirs = lib lib/libfdt lib/linux $(TEST_DIR) $(ARCH_LIBDIRS) lib/a cscope: $(RM) ./cscope.* find -L $(cscope_dirs) -maxdepth 1 \ - -name '*.[chsS]' -exec realpath --relative-base=$(PWD) {} \; | sort -u > ./cscope.files + -name '*.[chsS]' -exec realpath --relative-base=$(CURDIR) {} \; | sort -u > ./cscope.files cscope -bk .PHONY: tags and queued it. Paolo
On Mon, Sep 20, 2021 at 03:27:11PM +0200, Paolo Bonzini wrote: > On 27/08/21 12:22, Andrew Jones wrote: > > > 51b8f0b1 2017-11-23 Andrew Jones Makefile: fix cscope target > > No surprise there, that's when the $(PWD) use was first introduced. > > > > > So I add Andrew as CC, I did forgot to do before. > > > > > I'll send a patch changing $(PWD) to $(shell pwd) > > I could not find the patch using $(CURDIR) in my mailbox, though I found > it on spinics.net. I fudged the following > > From 164507376abae4be15b0f65aa14d56f179198a99 Mon Sep 17 00:00:00 2001 > From: Andrew Jones <drjones@redhat.com> > Date: Fri, 27 Aug 2021 12:31:15 +0200 > Subject: [PATCH kvm-unit-tests] Makefile: Don't trust PWD > > It's possible that PWD is already set to something which isn't > the full path of the current working directory. Let's use $(CURDIR) > instead, which is always correct. > > Suggested-by: Thomas Huth <thuth@redhat.com> > Signed-off-by: Andrew Jones <drjones@redhat.com> > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> > > diff --git a/Makefile b/Makefile > index f7b9f28..6792b93 100644 > --- a/Makefile > +++ b/Makefile > @@ -119,7 +119,7 @@ cscope: cscope_dirs = lib lib/libfdt lib/linux $(TEST_DIR) $(ARCH_LIBDIRS) lib/a > cscope: > $(RM) ./cscope.* > find -L $(cscope_dirs) -maxdepth 1 \ > - -name '*.[chsS]' -exec realpath --relative-base=$(PWD) {} \; | sort -u > ./cscope.files > + -name '*.[chsS]' -exec realpath --relative-base=$(CURDIR) {} \; | sort -u > ./cscope.files > cscope -bk > .PHONY: tags > > and queued it. Hi Paolo, You'll get a conflict when you go to merge because I already did it :-) commit 3d4eb24cb5b4de6c26f79b849fe2818d5315a691 (origin/misc/queue, misc/queue) Author: Andrew Jones <drjones@redhat.com> Date: Fri Aug 27 12:25:27 2021 +0200 Makefile: Don't trust PWD PWD comes from the environment and it's possible that it's already set to something which isn't the full path of the current working directory. Use the make variable $(CURDIR) instead. Reviewed-by: Pierre Morel <pmorel@linux.ibm.com> Reviewed-by: Thomas Huth <thuth@redhat.com> Suggested-by: Thomas Huth <thuth@redhat.com> Signed-off-by: Andrew Jones <drjones@redhat.com> misc/queue is something I recently invented for stuff like this in order to help lighten your load a bit. Thanks, drew
On 20/09/21 16:10, Andrew Jones wrote: > Hi Paolo, > > You'll get a conflict when you go to merge because I already did it :-) > > commit 3d4eb24cb5b4de6c26f79b849fe2818d5315a691 (origin/misc/queue, misc/queue) > Author: Andrew Jones <drjones@redhat.com> > Date: Fri Aug 27 12:25:27 2021 +0200 > > Makefile: Don't trust PWD > > PWD comes from the environment and it's possible that it's already > set to something which isn't the full path of the current working > directory. Use the make variable $(CURDIR) instead. > > Reviewed-by: Pierre Morel <pmorel@linux.ibm.com> > Reviewed-by: Thomas Huth <thuth@redhat.com> > Suggested-by: Thomas Huth <thuth@redhat.com> > Signed-off-by: Andrew Jones <drjones@redhat.com> > > > > misc/queue is something I recently invented for stuff like this in order > to help lighten your load a bit. Ok, are you going to create a merge request?
On Mon, Sep 20, 2021 at 08:04:02PM +0200, Paolo Bonzini wrote: > On 20/09/21 16:10, Andrew Jones wrote: > > Hi Paolo, > > > > You'll get a conflict when you go to merge because I already did it :-) > > > > commit 3d4eb24cb5b4de6c26f79b849fe2818d5315a691 (origin/misc/queue, misc/queue) > > Author: Andrew Jones <drjones@redhat.com> > > Date: Fri Aug 27 12:25:27 2021 +0200 > > > > Makefile: Don't trust PWD > > PWD comes from the environment and it's possible that it's already > > set to something which isn't the full path of the current working > > directory. Use the make variable $(CURDIR) instead. > > Reviewed-by: Pierre Morel <pmorel@linux.ibm.com> > > Reviewed-by: Thomas Huth <thuth@redhat.com> > > Suggested-by: Thomas Huth <thuth@redhat.com> > > Signed-off-by: Andrew Jones <drjones@redhat.com> > > > > > > > > misc/queue is something I recently invented for stuff like this in order > > to help lighten your load a bit. > > Ok, are you going to create a merge request? > I did. And I merged it. https://gitlab.com/kvm-unit-tests/kvm-unit-tests/-/merge_requests/16 Thanks, drew
diff --git a/Makefile b/Makefile index f7b9f28c..c8b0d74f 100644 --- a/Makefile +++ b/Makefile @@ -119,7 +119,7 @@ cscope: cscope_dirs = lib lib/libfdt lib/linux $(TEST_DIR) $(ARCH_LIBDIRS) lib/a cscope: $(RM) ./cscope.* find -L $(cscope_dirs) -maxdepth 1 \ - -name '*.[chsS]' -exec realpath --relative-base=$(PWD) {} \; | sort -u > ./cscope.files + -name '*.[chsS]' -exec realpath --relative-base=. {} \; | sort -u > ./cscope.files cscope -bk .PHONY: tags
In Linux, cscope uses a wrong directory. Simply search from the directory where the make is started. Signed-off-by: Pierre Morel <pmorel@linux.ibm.com> --- Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)