* 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