All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH XTF] build: introduce a dist target
@ 2016-07-20 11:55 Wei Liu
  2016-07-20 12:52 ` Andrew Cooper
                   ` (2 more replies)
  0 siblings, 3 replies; 14+ messages in thread
From: Wei Liu @ 2016-07-20 11:55 UTC (permalink / raw)
  To: Xen-devel; +Cc: andrew.cooper3, Wei Liu

The install target doesn't honour $PREFIX. Introduce a dist target to
make it easier to package the artifacts.

Signed-off-by: Wei Liu <wei.liu2@citrix.com>
---
 Makefile | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/Makefile b/Makefile
index 1a8099d..d625b90 100644
--- a/Makefile
+++ b/Makefile
@@ -1,6 +1,7 @@
 MAKEFLAGS += -r
 ROOT := $(abspath $(CURDIR))
 DESTDIR ?= $(ROOT)/dist
+DISTDIR ?= $(ROOT)/dist
 PREFIX ?= $(ROOT)
 
 .PHONY: all
@@ -10,6 +11,10 @@ all:
 		$(MAKE) -C $$D build; \
 	done
 
+.PHONY: dist
+dist: DESTDIR=$(DISTDIR)$(PREFIX)
+dist: install
+
 .PHONY: install
 install:
 	@mkdir -p $(DESTDIR)
-- 
2.1.4


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

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

* Re: [PATCH XTF] build: introduce a dist target
  2016-07-20 11:55 [PATCH XTF] build: introduce a dist target Wei Liu
@ 2016-07-20 12:52 ` Andrew Cooper
  2016-07-20 13:10   ` Wei Liu
  2016-07-20 18:21 ` [PATCH XTF] Correct the usage of $(DESTDIR) and $(prefix) Andrew Cooper
  2016-07-25 16:23 ` [XTF PATCH v3] Correct the usage of $(DESTDIR) and $(PREFIX) Andrew Cooper
  2 siblings, 1 reply; 14+ messages in thread
From: Andrew Cooper @ 2016-07-20 12:52 UTC (permalink / raw)
  To: Wei Liu, Xen-devel

On 20/07/16 12:55, Wei Liu wrote:
> The install target doesn't honour $PREFIX. Introduce a dist target to
> make it easier to package the artifacts.
>
> Signed-off-by: Wei Liu <wei.liu2@citrix.com>
> ---
>  Makefile | 5 +++++
>  1 file changed, 5 insertions(+)
>
> diff --git a/Makefile b/Makefile
> index 1a8099d..d625b90 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -1,6 +1,7 @@
>  MAKEFLAGS += -r
>  ROOT := $(abspath $(CURDIR))
>  DESTDIR ?= $(ROOT)/dist
> +DISTDIR ?= $(ROOT)/dist
>  PREFIX ?= $(ROOT)
>  
>  .PHONY: all
> @@ -10,6 +11,10 @@ all:
>  		$(MAKE) -C $$D build; \
>  	done
>  
> +.PHONY: dist
> +dist: DESTDIR=$(DISTDIR)$(PREFIX)
> +dist: install

Thusfar, I have been following GNU coding standards.

I don't see any references to DISTDIR, and this usage if `make dist` is
incompatible with the standard expectation.

FWIW, `make dist` seems incompatible with XTF at the moment, because of
the embedded absolute paths needed in the written xl configuration files.



For XenServer, my RPM configuration is:

%{__make} %{?_smp_mflags} install DESTDIR=%{buildroot}/opt/%{name}
PREFIX=/opt/%{name}

to get `make install` to put the content sensibly in the build root, and
to generate files with correct absolute paths.

How are you intending to make use of this new `make dist` target?

~Andrew

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

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

* Re: [PATCH XTF] build: introduce a dist target
  2016-07-20 12:52 ` Andrew Cooper
@ 2016-07-20 13:10   ` Wei Liu
  2016-07-20 13:15     ` Andrew Cooper
  0 siblings, 1 reply; 14+ messages in thread
From: Wei Liu @ 2016-07-20 13:10 UTC (permalink / raw)
  To: Andrew Cooper; +Cc: Xen-devel, Wei Liu

On Wed, Jul 20, 2016 at 01:52:43PM +0100, Andrew Cooper wrote:
> On 20/07/16 12:55, Wei Liu wrote:
> > The install target doesn't honour $PREFIX. Introduce a dist target to
> > make it easier to package the artifacts.
> >
> > Signed-off-by: Wei Liu <wei.liu2@citrix.com>
> > ---
> >  Makefile | 5 +++++
> >  1 file changed, 5 insertions(+)
> >
> > diff --git a/Makefile b/Makefile
> > index 1a8099d..d625b90 100644
> > --- a/Makefile
> > +++ b/Makefile
> > @@ -1,6 +1,7 @@
> >  MAKEFLAGS += -r
> >  ROOT := $(abspath $(CURDIR))
> >  DESTDIR ?= $(ROOT)/dist
> > +DISTDIR ?= $(ROOT)/dist
> >  PREFIX ?= $(ROOT)
> >  
> >  .PHONY: all
> > @@ -10,6 +11,10 @@ all:
> >  		$(MAKE) -C $$D build; \
> >  	done
> >  
> > +.PHONY: dist
> > +dist: DESTDIR=$(DISTDIR)$(PREFIX)
> > +dist: install
> 
> Thusfar, I have been following GNU coding standards.
> 
> I don't see any references to DISTDIR, and this usage if `make dist` is
> incompatible with the standard expectation.
> 

There is dist target in GNU standard. I regard DISTDIR an internal
variable. It's fine that the user doesn't set it.

> FWIW, `make dist` seems incompatible with XTF at the moment, because of
> the embedded absolute paths needed in the written xl configuration files.
> 

Exactly. There is hardcoded paths in guest config files.

> 
> 
> For XenServer, my RPM configuration is:
> 
> %{__make} %{?_smp_mflags} install DESTDIR=%{buildroot}/opt/%{name}
> PREFIX=/opt/%{name}

/opt/%{name} appeared twice here and the DESTDIR and PREFIX need to be
kept in sync.

I'm fine with this too. Don't feel I want to argue one way or another.

> 
> to get `make install` to put the content sensibly in the build root, and
> to generate files with correct absolute paths.
> 
> How are you intending to make use of this new `make dist` target?

More or less like what you do for XenServer -- I suppose you just
package buildroot and send that to the test host and install the
artifacts.

Wei.

> 
> ~Andrew

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

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

* Re: [PATCH XTF] build: introduce a dist target
  2016-07-20 13:10   ` Wei Liu
@ 2016-07-20 13:15     ` Andrew Cooper
  0 siblings, 0 replies; 14+ messages in thread
From: Andrew Cooper @ 2016-07-20 13:15 UTC (permalink / raw)
  To: Wei Liu; +Cc: Xen-devel

On 20/07/16 14:10, Wei Liu wrote:
> On Wed, Jul 20, 2016 at 01:52:43PM +0100, Andrew Cooper wrote:
>> On 20/07/16 12:55, Wei Liu wrote:
>>> The install target doesn't honour $PREFIX. Introduce a dist target to
>>> make it easier to package the artifacts.
>>>
>>> Signed-off-by: Wei Liu <wei.liu2@citrix.com>
>>> ---
>>>  Makefile | 5 +++++
>>>  1 file changed, 5 insertions(+)
>>>
>>> diff --git a/Makefile b/Makefile
>>> index 1a8099d..d625b90 100644
>>> --- a/Makefile
>>> +++ b/Makefile
>>> @@ -1,6 +1,7 @@
>>>  MAKEFLAGS += -r
>>>  ROOT := $(abspath $(CURDIR))
>>>  DESTDIR ?= $(ROOT)/dist
>>> +DISTDIR ?= $(ROOT)/dist
>>>  PREFIX ?= $(ROOT)
>>>  
>>>  .PHONY: all
>>> @@ -10,6 +11,10 @@ all:
>>>  		$(MAKE) -C $$D build; \
>>>  	done
>>>  
>>> +.PHONY: dist
>>> +dist: DESTDIR=$(DISTDIR)$(PREFIX)
>>> +dist: install
>> Thusfar, I have been following GNU coding standards.
>>
>> I don't see any references to DISTDIR, and this usage if `make dist` is
>> incompatible with the standard expectation.
>>
> There is dist target in GNU standard. I regard DISTDIR an internal
> variable. It's fine that the user doesn't set it.
>
>> FWIW, `make dist` seems incompatible with XTF at the moment, because of
>> the embedded absolute paths needed in the written xl configuration files.
>>
> Exactly. There is hardcoded paths in guest config files.
>
>>
>> For XenServer, my RPM configuration is:
>>
>> %{__make} %{?_smp_mflags} install DESTDIR=%{buildroot}/opt/%{name}
>> PREFIX=/opt/%{name}
> /opt/%{name} appeared twice here and the DESTDIR and PREFIX need to be
> kept in sync.
>
> I'm fine with this too. Don't feel I want to argue one way or another.

Hmm, re-reading
https://www.gnu.org/prep/standards/html_node/DESTDIR.html I may have got
the usage of DESTDIR wrong.

It looks like DESTDIR is supposed to be what you are using DISTDIR for.

~Andrew

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

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

* [PATCH XTF] Correct the usage of $(DESTDIR) and $(prefix)
  2016-07-20 11:55 [PATCH XTF] build: introduce a dist target Wei Liu
  2016-07-20 12:52 ` Andrew Cooper
@ 2016-07-20 18:21 ` Andrew Cooper
  2016-07-20 18:28   ` Doug Goldstein
                     ` (2 more replies)
  2016-07-25 16:23 ` [XTF PATCH v3] Correct the usage of $(DESTDIR) and $(PREFIX) Andrew Cooper
  2 siblings, 3 replies; 14+ messages in thread
From: Andrew Cooper @ 2016-07-20 18:21 UTC (permalink / raw)
  To: Xen-devel; +Cc: Andrew Cooper, Ian Jackson, Wei Liu

The GNU coding standards expect $(DESTDIR) to be the root of everything
installed, and for prefix to then be added to the path.  This is not how XTF
previously behaved.

Replace $(PREFIX) with its more common form $(prefix), and rearange $(DESTDIR)
and $(prefix) to match expectation.

Reported-by:Wei Liu <wei.liu2@citrix.com>
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Ian Jackson <Ian.Jackson@eu.citrix.com>
CC: Wei Liu <wei.liu2@citrix.com>
---
 Makefile     | 15 ++++++++++-----
 build/gen.mk | 22 +++++++++++++++-------
 2 files changed, 25 insertions(+), 12 deletions(-)

diff --git a/Makefile b/Makefile
index e21605d..c86dbb8 100644
--- a/Makefile
+++ b/Makefile
@@ -2,9 +2,14 @@ MAKEFLAGS += -r
 ROOT := $(abspath $(CURDIR))
 export ROOT
 
-DESTDIR ?= $(ROOT)/dist
-PREFIX ?= $(ROOT)
-export DESTDIR PREFIX
+# $(prefix) defaults to $(ROOT) so development and testing can be done
+# straight out of the working tree
+prefix  ?= $(ROOT)
+DESTDIR ?=
+
+# Provide $(DEST) as the install root
+DEST := $(DESTDIR)$(prefix)
+export DEST DESTDIR prefix
 
 # Programs used
 CC              ?= $(CROSS_COMPILE)gcc
@@ -27,8 +32,8 @@ all:
 
 .PHONY: install
 install:
-	@mkdir -p $(DESTDIR)
-	$(INSTALL_PROGRAM) -p xtf-runner $(DESTDIR)
+	@mkdir -p $(DEST)
+	$(INSTALL_PROGRAM) -p xtf-runner $(DEST)
 	@set -e; for D in $(wildcard tests/*); do \
 		[ ! -e $$D/Makefile ] && continue; \
 		$(MAKE) -C $$D install; \
diff --git a/build/gen.mk b/build/gen.mk
index 790212b..e20fd77 100644
--- a/build/gen.mk
+++ b/build/gen.mk
@@ -21,6 +21,14 @@ ifneq ($(filter-out $(ALL_CATEGORIES),$(CATEGORY)),)
 $(error Unrecognised category '$(filter-out $(ALL_CATEGORIES),$(CATEGORY))')
 endif
 
+ifeq ($(filter /%,$(prefix)),)
+$(error $$(prefix) must be absolute, not '$(prefix)')
+endif
+
+ifeq ($(filter /%,$(DEST)),)
+$(error $$(DEST) must be absolute, not '$(DEST)')
+endif
+
 .PHONY: build
 build: $(foreach env,$(TEST-ENVS),test-$(env)-$(NAME) test-$(env)-$(NAME).cfg)
 build: test-info.json
@@ -31,8 +39,8 @@ test-info.json: $(ROOT)/build/mkinfo.py FORCE
 
 .PHONY: install install-each-env
 install: install-each-env test-info.json
-	@mkdir -p $(DESTDIR)/tests/$(NAME)
-	$(INSTALL_DATA) -p test-info.json $(DESTDIR)/tests/$(NAME)
+	@mkdir -p $(DEST)/tests/$(NAME)
+	$(INSTALL_DATA) -p test-info.json $(DEST)/tests/$(NAME)
 
 define PERENV_build
 
@@ -54,7 +62,7 @@ test-$(1)-$(NAME).cfg: $$(cfg-$(1)) FORCE
 	@{ cat $$< $(TEST-EXTRA-CFG) ;} | \
 	sed -e "s/@@NAME@@/$$(NAME)/g" \
 		-e "s/@@ENV@@/$(1)/g" \
-		-e "s!@@PREFIX@@!$$(PREFIX)!g" \
+		-e "s!@@PREFIX@@!$$(prefix)!g" \
 		> $$@.tmp
 	@if ! cmp -s $$@ $$@.tmp; then mv -f $$@.tmp $$@; else rm -f $$@.tmp; fi
 
@@ -63,12 +71,12 @@ test-$(1)-$(NAME).cfg: $$(cfg-$(1)) FORCE
 
 .PHONY: install-$(1) install-$(1).cfg
 install-$(1): test-$(1)-$(NAME)
-	@mkdir -p $(DESTDIR)/tests/$(NAME)
-	$(INSTALL_PROGRAM) -p $$< $(DESTDIR)/tests/$(NAME)
+	@mkdir -p $(DEST)/tests/$(NAME)
+	$(INSTALL_PROGRAM) -p $$< $(DEST)/tests/$(NAME)
 
 install-$(1).cfg: test-$(1)-$(NAME).cfg
-	@mkdir -p $(DESTDIR)/tests/$(NAME)
-	$(INSTALL_DATA) -p $$< $(DESTDIR)/tests/$(NAME)
+	@mkdir -p $(DEST)/tests/$(NAME)
+	$(INSTALL_DATA) -p $$< $(DEST)/tests/$(NAME)
 
 install-each-env: install-$(1) install-$(1).cfg
 
-- 
2.1.4


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

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

* Re: [PATCH XTF] Correct the usage of $(DESTDIR) and $(prefix)
  2016-07-20 18:21 ` [PATCH XTF] Correct the usage of $(DESTDIR) and $(prefix) Andrew Cooper
@ 2016-07-20 18:28   ` Doug Goldstein
  2016-07-20 18:31   ` Wei Liu
  2016-07-21 10:43   ` Ian Jackson
  2 siblings, 0 replies; 14+ messages in thread
From: Doug Goldstein @ 2016-07-20 18:28 UTC (permalink / raw)
  To: Andrew Cooper, Xen-devel; +Cc: Wei Liu, Ian Jackson


[-- Attachment #1.1.1: Type: text/plain, Size: 620 bytes --]

On 7/20/16 1:21 PM, Andrew Cooper wrote:
> The GNU coding standards expect $(DESTDIR) to be the root of everything
> installed, and for prefix to then be added to the path.  This is not how XTF
> previously behaved.
> 
> Replace $(PREFIX) with its more common form $(prefix), and rearange $(DESTDIR)
> and $(prefix) to match expectation.
> 
> Reported-by:Wei Liu <wei.liu2@citrix.com>
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>

Reviewed-by: Doug Goldstein <cardoe@cardoe.com>

The conclusion in the commit message is correct. The supplied patch
conforms to that.

-- 
Doug Goldstein


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 959 bytes --]

[-- Attachment #2: Type: text/plain, Size: 127 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

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

* Re: [PATCH XTF] Correct the usage of $(DESTDIR) and $(prefix)
  2016-07-20 18:21 ` [PATCH XTF] Correct the usage of $(DESTDIR) and $(prefix) Andrew Cooper
  2016-07-20 18:28   ` Doug Goldstein
@ 2016-07-20 18:31   ` Wei Liu
  2016-07-21 10:43   ` Ian Jackson
  2 siblings, 0 replies; 14+ messages in thread
From: Wei Liu @ 2016-07-20 18:31 UTC (permalink / raw)
  To: Andrew Cooper; +Cc: Wei Liu, Ian Jackson, Xen-devel

On Wed, Jul 20, 2016 at 07:21:46PM +0100, Andrew Cooper wrote:
> The GNU coding standards expect $(DESTDIR) to be the root of everything
> installed, and for prefix to then be added to the path.  This is not how XTF
> previously behaved.
> 
> Replace $(PREFIX) with its more common form $(prefix), and rearange $(DESTDIR)

"rearrange"

> and $(prefix) to match expectation.
> 
> Reported-by:Wei Liu <wei.liu2@citrix.com>

Space after colon.

The changes look fine to me.

Wei.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

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

* Re: [PATCH XTF] Correct the usage of $(DESTDIR) and $(prefix)
  2016-07-20 18:21 ` [PATCH XTF] Correct the usage of $(DESTDIR) and $(prefix) Andrew Cooper
  2016-07-20 18:28   ` Doug Goldstein
  2016-07-20 18:31   ` Wei Liu
@ 2016-07-21 10:43   ` Ian Jackson
  2016-07-21 10:57     ` Andrew Cooper
  2 siblings, 1 reply; 14+ messages in thread
From: Ian Jackson @ 2016-07-21 10:43 UTC (permalink / raw)
  To: Andrew Cooper; +Cc: Wei Liu, Xen-devel

Andrew Cooper writes ("[PATCH XTF] Correct the usage of $(DESTDIR) and $(prefix)"):
> The GNU coding standards expect $(DESTDIR) to be the root of everything
> installed, and for prefix to then be added to the path.  This is not how XTF
> previously behaved.
> 
> Replace $(PREFIX) with its more common form $(prefix), and rearange $(DESTDIR)
> and $(prefix) to match expectation.
...
> -DESTDIR ?= $(ROOT)/dist
> -PREFIX ?= $(ROOT)
> -export DESTDIR PREFIX
> +# $(prefix) defaults to $(ROOT) so development and testing can be done
> +# straight out of the working tree
> +prefix  ?= $(ROOT)
...
> +DEST := $(DESTDIR)$(prefix)
...
> -	@mkdir -p $(DESTDIR)
> -	$(INSTALL_PROGRAM) -p xtf-runner $(DESTDIR)
> +	@mkdir -p $(DEST)
> +	$(INSTALL_PROGRAM) -p xtf-runner $(DEST)

The effect of this is that
   make prefix=/usr
will create
   /usr/xtf-runner
   /usr/tests/*/test-info.json
which is not how things would normally be expected work.

I think to make this work right, you have to do something like

  ifeq ($prefix,)
   bindir=$(ROOT)
   xtflibdir=$(ROOT)
  else
   bindir=$(prefix)/bin
   xtflibdir=$(prefix)/lib/xtf
  endif
  xtftestsdir=$(xtflibdir)/tests
  ...
        $(INSTALL_PROGRAM) -p xtf-runner $(DESTDIR)/$(bindir)
  ...
        $(INSTALL_DATA) -p test-info.json $(DESTDIR)/$(xtftestsdir)/$(NAME)

Also it would be more conventional to use $(INSTALL) -d or
$(INSTALL_DIR) rather than mkdir.

Ian.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

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

* Re: [PATCH XTF] Correct the usage of $(DESTDIR) and $(prefix)
  2016-07-21 10:43   ` Ian Jackson
@ 2016-07-21 10:57     ` Andrew Cooper
  2016-07-21 11:10       ` Ian Jackson
  0 siblings, 1 reply; 14+ messages in thread
From: Andrew Cooper @ 2016-07-21 10:57 UTC (permalink / raw)
  To: Ian Jackson; +Cc: Wei Liu, Xen-devel

On 21/07/16 11:43, Ian Jackson wrote:
> Andrew Cooper writes ("[PATCH XTF] Correct the usage of $(DESTDIR) and $(prefix)"):
>> The GNU coding standards expect $(DESTDIR) to be the root of everything
>> installed, and for prefix to then be added to the path.  This is not how XTF
>> previously behaved.
>>
>> Replace $(PREFIX) with its more common form $(prefix), and rearange $(DESTDIR)
>> and $(prefix) to match expectation.
> ...
>> -DESTDIR ?= $(ROOT)/dist
>> -PREFIX ?= $(ROOT)
>> -export DESTDIR PREFIX
>> +# $(prefix) defaults to $(ROOT) so development and testing can be done
>> +# straight out of the working tree
>> +prefix  ?= $(ROOT)
> ...
>> +DEST := $(DESTDIR)$(prefix)
> ...
>> -	@mkdir -p $(DESTDIR)
>> -	$(INSTALL_PROGRAM) -p xtf-runner $(DESTDIR)
>> +	@mkdir -p $(DEST)
>> +	$(INSTALL_PROGRAM) -p xtf-runner $(DEST)
> The effect of this is that
>    make prefix=/usr
> will create
>    /usr/xtf-runner
>    /usr/tests/*/test-info.json
> which is not how things would normally be expected work.

XTF doesn't match any standard installable package.  It is some
configuration files and a load of microkernels which are not system
executables.

I build it with prefix=/opt/xtf as that is the only plausible place for
the results to live, per the FHS.  It could certainly be argued that it
should insert its own xtf/ directory rather than relying on prefix to
pass it.

>
> I think to make this work right, you have to do something like
>
>   ifeq ($prefix,)
>    bindir=$(ROOT)
>    xtflibdir=$(ROOT)
>   else
>    bindir=$(prefix)/bin
>    xtflibdir=$(prefix)/lib/xtf
>   endif
>   xtftestsdir=$(xtflibdir)/tests
>   ...
>         $(INSTALL_PROGRAM) -p xtf-runner $(DESTDIR)/$(bindir)
>   ...
>         $(INSTALL_DATA) -p test-info.json $(DESTDIR)/$(xtftestsdir)/$(NAME)

xtf-runner expects to find tests/ in the same directory it resides in,
because otherwise there is no sensible way to find it.

>
> Also it would be more conventional to use $(INSTALL) -d or
> $(INSTALL_DIR) rather than mkdir.

I will make this change.

~Andrew

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

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

* Re: [PATCH XTF] Correct the usage of $(DESTDIR) and $(prefix)
  2016-07-21 10:57     ` Andrew Cooper
@ 2016-07-21 11:10       ` Ian Jackson
  0 siblings, 0 replies; 14+ messages in thread
From: Ian Jackson @ 2016-07-21 11:10 UTC (permalink / raw)
  To: Andrew Cooper; +Cc: Wei Liu, Ian Jackson, Xen-devel

Andrew Cooper writes ("Re: [PATCH XTF] Correct the usage of $(DESTDIR) and $(prefix)"):
> XTF doesn't match any standard installable package.  It is some
> configuration files and a load of microkernels which are not system
> executables.

The question is what
  really make install prefix=/usr
should do.  That's something that the standard Makefile targets
suggest ought to do something sane.

From what you write it probably ought to dump its stuff in
  /usr/lib/xtf/<whatever>
?

> I build it with prefix=/opt/xtf as that is the only plausible place for
> the results to live, per the FHS.  It could certainly be argued that it
> should insert its own xtf/ directory rather than relying on prefix to
> pass it.

It's conventional for /opt/foo to contain /opt/foo/bin, /opt/foo/man,
and so on.

Ian.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

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

* [XTF PATCH v3] Correct the usage of $(DESTDIR) and $(PREFIX)
  2016-07-20 11:55 [PATCH XTF] build: introduce a dist target Wei Liu
  2016-07-20 12:52 ` Andrew Cooper
  2016-07-20 18:21 ` [PATCH XTF] Correct the usage of $(DESTDIR) and $(prefix) Andrew Cooper
@ 2016-07-25 16:23 ` Andrew Cooper
  2016-07-26  9:09   ` Wei Liu
  2016-07-26 10:14   ` Ian Jackson
  2 siblings, 2 replies; 14+ messages in thread
From: Andrew Cooper @ 2016-07-25 16:23 UTC (permalink / raw)
  To: Xen-devel; +Cc: Andrew Cooper, Ian Jackson, Wei Liu

The GNU coding standards expect $(DESTDIR) to be the root of everything
installed, and for $(PREFIX) to then be added to the path.  This is not how
XTF previously behaved.

XTF is not a typical package, and doesn't meet the usual semantics; it expects
to arrange all files in a single directory.  Drop the use of $(PREFIX)
entirely (to avoid the expectation that it behaves as $(prefix) usually
behaves) and introduce $(xtfdir) instead.

$(DESTDIR) now works as intended for staged installes, and $(xtfdir) is the
single selected directy containing all installed content, typically expected
to be /opt/xtf or similar.

The intended way to install XTF now:

  $ make install DESTDIR=/path/to/staging/area xtfdir=/opt/xtf

Reported-by: Wei Liu <wei.liu2@citrix.com>
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Ian Jackson <Ian.Jackson@eu.citrix.com>
CC: Wei Liu <wei.liu2@citrix.com>

v3:
 * Drop $(PREFIX) entirely, as the semantics don't match.  Use $(xtfdir)
   instead to make it obvious that it is different.
v2:
 * Add an explicit subdirectory, to avoid prefix=/usr putting a number of
   files and directories straight in /usr
---
 Makefile                  | 23 +++++++++++++++++++----
 build/gen.mk              | 14 +++++++-------
 config/default-hvm.cfg.in |  2 +-
 config/default-pv.cfg.in  |  2 +-
 docs/introduction.dox     |  2 +-
 docs/mainpage.dox         |  4 ++--
 6 files changed, 31 insertions(+), 16 deletions(-)

diff --git a/Makefile b/Makefile
index 45f3846..8c36da0 100644
--- a/Makefile
+++ b/Makefile
@@ -2,9 +2,24 @@ MAKEFLAGS += -r
 ROOT := $(abspath $(CURDIR))
 export ROOT
 
+# $(xtfdir) defaults to $(ROOT) so development and testing can be done
+# straight out of the working tree.
+xtfdir  ?= $(ROOT)
 DESTDIR ?= $(ROOT)/dist
-PREFIX ?= $(ROOT)
-export DESTDIR PREFIX
+
+ifeq ($(filter /%,$(xtfdir)),)
+$(error $$(xtfdir) must be absolute, not '$(xtfdir)')
+endif
+
+ifneq ($(DESTDIR),)
+ifeq ($(filter /%,$(DESTDIR)),)
+$(error $$(DESTDIR) must be absolute, not '$(DESTDIR)')
+endif
+endif
+
+xtftestdir := $(xtfdir)/tests
+
+export DESTDIR xtfdir xtftestdir
 
 # Programs used
 CC              ?= $(CROSS_COMPILE)gcc
@@ -28,8 +43,8 @@ all:
 
 .PHONY: install
 install:
-	@$(INSTALL_DIR) $(DESTDIR)
-	$(INSTALL_PROGRAM) xtf-runner $(DESTDIR)
+	@$(INSTALL_DIR) $(DESTDIR)$(xtfdir)
+	$(INSTALL_PROGRAM) xtf-runner $(DESTDIR)$(xtfdir)
 	@set -e; for D in $(wildcard tests/*); do \
 		[ ! -e $$D/Makefile ] && continue; \
 		$(MAKE) -C $$D install; \
diff --git a/build/gen.mk b/build/gen.mk
index 0a172b3..aeac2a6 100644
--- a/build/gen.mk
+++ b/build/gen.mk
@@ -31,8 +31,8 @@ test-info.json: $(ROOT)/build/mkinfo.py FORCE
 
 .PHONY: install install-each-env
 install: install-each-env test-info.json
-	@$(INSTALL_DIR) $(DESTDIR)/tests/$(NAME)
-	$(INSTALL_DATA) test-info.json $(DESTDIR)/tests/$(NAME)
+	@$(INSTALL_DIR) $(DESTDIR)$(xtftestdir)/$(NAME)
+	$(INSTALL_DATA) test-info.json $(DESTDIR)$(xtftestdir)/$(NAME)
 
 define PERENV_build
 
@@ -54,7 +54,7 @@ test-$(1)-$(NAME).cfg: $$(cfg-$(1)) FORCE
 	@{ cat $$< $(TEST-EXTRA-CFG) ;} | \
 	sed -e "s/@@NAME@@/$$(NAME)/g" \
 		-e "s/@@ENV@@/$(1)/g" \
-		-e "s!@@PREFIX@@!$$(PREFIX)!g" \
+		-e "s!@@XTFDIR@@!$$(xtfdir)!g" \
 		> $$@.tmp
 	@if ! cmp -s $$@ $$@.tmp; then mv -f $$@.tmp $$@; else rm -f $$@.tmp; fi
 
@@ -63,12 +63,12 @@ test-$(1)-$(NAME).cfg: $$(cfg-$(1)) FORCE
 
 .PHONY: install-$(1) install-$(1).cfg
 install-$(1): test-$(1)-$(NAME)
-	@$(INSTALL_DIR) $(DESTDIR)/tests/$(NAME)
-	$(INSTALL_PROGRAM) $$< $(DESTDIR)/tests/$(NAME)
+	@$(INSTALL_DIR) $(DESTDIR)$(xtftestdir)/$(NAME)
+	$(INSTALL_PROGRAM) $$< $(DESTDIR)$(xtftestdir)/$(NAME)
 
 install-$(1).cfg: test-$(1)-$(NAME).cfg
-	@$(INSTALL_DIR) $(DESTDIR)/tests/$(NAME)
-	$(INSTALL_DATA) $$< $(DESTDIR)/tests/$(NAME)
+	@$(INSTALL_DIR) $(DESTDIR)$(xtftestdir)/$(NAME)
+	$(INSTALL_DATA) $$< $(DESTDIR)$(xtftestdir)/$(NAME)
 
 install-each-env: install-$(1) install-$(1).cfg
 
diff --git a/config/default-hvm.cfg.in b/config/default-hvm.cfg.in
index 2a395a5..7d29e0c 100644
--- a/config/default-hvm.cfg.in
+++ b/config/default-hvm.cfg.in
@@ -1,7 +1,7 @@
 name="test-@@ENV@@-@@NAME@@"
 builder="hvm"
 memory=128
-firmware_override="@@PREFIX@@/tests/@@NAME@@/test-@@ENV@@-@@NAME@@"
+firmware_override="@@XTFDIR@@/tests/@@NAME@@/test-@@ENV@@-@@NAME@@"
 
 # The framework doesn't reboot.  A reboot signal is almost certainly a triple
 # fault instead.  Prevent it turning into a runaway domain.
diff --git a/config/default-pv.cfg.in b/config/default-pv.cfg.in
index 044e50e..166e464 100644
--- a/config/default-pv.cfg.in
+++ b/config/default-pv.cfg.in
@@ -1,4 +1,4 @@
 name="test-@@ENV@@-@@NAME@@"
 loader="generic"
 memory=128
-kernel="@@PREFIX@@/tests/@@NAME@@/test-@@ENV@@-@@NAME@@"
+kernel="@@XTFDIR@@/tests/@@NAME@@/test-@@ENV@@-@@NAME@@"
diff --git a/docs/introduction.dox b/docs/introduction.dox
index e9a0777..3151fbf 100644
--- a/docs/introduction.dox
+++ b/docs/introduction.dox
@@ -131,7 +131,7 @@ If XTF in being built in dom0, all paths should be set up to run correctly.
 
 If XTF is built elsewhere, it should be installed:
 
-    $ make install PREFIX=/path DESTDIR=/path
+    $ make install xtfdir=/path DESTDIR=/path
 
 with paths appropriate for the system under test.
 
diff --git a/docs/mainpage.dox b/docs/mainpage.dox
index 2639423..942929e 100644
--- a/docs/mainpage.dox
+++ b/docs/mainpage.dox
@@ -62,9 +62,9 @@ To run tests on a Xen host: (see @ref errata first)
 
 - For the paths of binaries, `xl` accepts either an absolute path, or certain
   relative paths (`/etc/xen/` or `$CWD` for `kernel=`, `$libdir/xen/boot` for
-  `firmware_override=`).  The default `PREFIX=` is configured correctly for
+  `firmware_override=`).  The default `xtfdir=` is configured correctly for
   running the tests out of the build working tree.  If the tests are running
-  elsewhere, use `make install DESTDIR=$X PREFIX=$Y` to configure absolute
+  elsewhere, use `make install DESTDIR=$X xtfdir=$Y` to configure absolute
   paths appropriately for the test system.
 
 
-- 
2.1.4


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

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

* Re: [XTF PATCH v3] Correct the usage of $(DESTDIR) and $(PREFIX)
  2016-07-25 16:23 ` [XTF PATCH v3] Correct the usage of $(DESTDIR) and $(PREFIX) Andrew Cooper
@ 2016-07-26  9:09   ` Wei Liu
  2016-07-26 10:14   ` Ian Jackson
  1 sibling, 0 replies; 14+ messages in thread
From: Wei Liu @ 2016-07-26  9:09 UTC (permalink / raw)
  To: Andrew Cooper; +Cc: Wei Liu, Ian Jackson, Xen-devel

On Mon, Jul 25, 2016 at 05:23:18PM +0100, Andrew Cooper wrote:
> The GNU coding standards expect $(DESTDIR) to be the root of everything
> installed, and for $(PREFIX) to then be added to the path.  This is not how
> XTF previously behaved.
> 
> XTF is not a typical package, and doesn't meet the usual semantics; it expects
> to arrange all files in a single directory.  Drop the use of $(PREFIX)
> entirely (to avoid the expectation that it behaves as $(prefix) usually
> behaves) and introduce $(xtfdir) instead.
> 
> $(DESTDIR) now works as intended for staged installes, and $(xtfdir) is the
> single selected directy containing all installed content, typically expected
> to be /opt/xtf or similar.
> 
> The intended way to install XTF now:
> 
>   $ make install DESTDIR=/path/to/staging/area xtfdir=/opt/xtf
> 
> Reported-by: Wei Liu <wei.liu2@citrix.com>
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>

This code does what the commit message says:

Reviewed-by: Wei Liu <wei.liu2@citrix.com>

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

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

* Re: [XTF PATCH v3] Correct the usage of $(DESTDIR) and $(PREFIX)
  2016-07-25 16:23 ` [XTF PATCH v3] Correct the usage of $(DESTDIR) and $(PREFIX) Andrew Cooper
  2016-07-26  9:09   ` Wei Liu
@ 2016-07-26 10:14   ` Ian Jackson
  2016-07-26 10:17     ` Andrew Cooper
  1 sibling, 1 reply; 14+ messages in thread
From: Ian Jackson @ 2016-07-26 10:14 UTC (permalink / raw)
  To: Andrew Cooper; +Cc: Wei Liu, Xen-devel

Andrew Cooper writes ("[XTF PATCH v3] Correct the usage of $(DESTDIR) and $(PREFIX)"):
> The GNU coding standards expect $(DESTDIR) to be the root of everything
> installed, and for $(PREFIX) to then be added to the path.  This is not how
> XTF previously behaved.
> 
> XTF is not a typical package, and doesn't meet the usual semantics; it expects
> to arrange all files in a single directory.  Drop the use of $(PREFIX)
> entirely (to avoid the expectation that it behaves as $(prefix) usually
> behaves) and introduce $(xtfdir) instead.
> 
> $(DESTDIR) now works as intended for staged installes, and $(xtfdir) is the
> single selected directy containing all installed content, typically expected
> to be /opt/xtf or similar.

The semantics you describe are sensible but I want to quibble with
your example xtfdir value.  It is conventional for areas in /opt to
contain the usual subdirs.

http://www.pathname.com/fhs/pub/fhs-2.3.html#OPTADDONAPPLICATIONSOFTWAREPACKAGES

> The intended way to install XTF now:
>   $ make install DESTDIR=/path/to/staging/area xtfdir=/opt/xtf

So I think you should provide to a different example.  How about
/local/scratch/xtf ?

Ian.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

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

* Re: [XTF PATCH v3] Correct the usage of $(DESTDIR) and $(PREFIX)
  2016-07-26 10:14   ` Ian Jackson
@ 2016-07-26 10:17     ` Andrew Cooper
  0 siblings, 0 replies; 14+ messages in thread
From: Andrew Cooper @ 2016-07-26 10:17 UTC (permalink / raw)
  To: Ian Jackson; +Cc: Wei Liu, Xen-devel

On 26/07/16 11:14, Ian Jackson wrote:
> Andrew Cooper writes ("[XTF PATCH v3] Correct the usage of $(DESTDIR) and $(PREFIX)"):
>> The GNU coding standards expect $(DESTDIR) to be the root of everything
>> installed, and for $(PREFIX) to then be added to the path.  This is not how
>> XTF previously behaved.
>>
>> XTF is not a typical package, and doesn't meet the usual semantics; it expects
>> to arrange all files in a single directory.  Drop the use of $(PREFIX)
>> entirely (to avoid the expectation that it behaves as $(prefix) usually
>> behaves) and introduce $(xtfdir) instead.
>>
>> $(DESTDIR) now works as intended for staged installes, and $(xtfdir) is the
>> single selected directy containing all installed content, typically expected
>> to be /opt/xtf or similar.
> The semantics you describe are sensible but I want to quibble with
> your example xtfdir value.  It is conventional for areas in /opt to
> contain the usual subdirs.
>
> http://www.pathname.com/fhs/pub/fhs-2.3.html#OPTADDONAPPLICATIONSOFTWAREPACKAGES
>
>> The intended way to install XTF now:
>>   $ make install DESTDIR=/path/to/staging/area xtfdir=/opt/xtf
> So I think you should provide to a different example.  How about
> /local/scratch/xtf ?

Fine - I will adjust on commit.

~Andrew

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

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

end of thread, other threads:[~2016-07-26 10:17 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-07-20 11:55 [PATCH XTF] build: introduce a dist target Wei Liu
2016-07-20 12:52 ` Andrew Cooper
2016-07-20 13:10   ` Wei Liu
2016-07-20 13:15     ` Andrew Cooper
2016-07-20 18:21 ` [PATCH XTF] Correct the usage of $(DESTDIR) and $(prefix) Andrew Cooper
2016-07-20 18:28   ` Doug Goldstein
2016-07-20 18:31   ` Wei Liu
2016-07-21 10:43   ` Ian Jackson
2016-07-21 10:57     ` Andrew Cooper
2016-07-21 11:10       ` Ian Jackson
2016-07-25 16:23 ` [XTF PATCH v3] Correct the usage of $(DESTDIR) and $(PREFIX) Andrew Cooper
2016-07-26  9:09   ` Wei Liu
2016-07-26 10:14   ` Ian Jackson
2016-07-26 10:17     ` Andrew Cooper

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.