Re: [PATCH v6] tools/xenstore: move xenstored sources into dedicated directory

[Juergen Gross <jgross@xxxxxxxx>]

On 21.08.23 17:17, Anthony PERARD wrote:
> On Mon, Aug 21, 2023 at 10:14:22AM +0200, Juergen Gross wrote:
> > diff --git a/tools/xenstored/.gitignore b/tools/xenstored/.gitignore
> > new file mode 100644
> > index 0000000000..edbb5d79fe
> > --- /dev/null
> > +++ b/tools/xenstored/.gitignore
> > @@ -0,0 +1 @@
> > +xenstored
> Could you write that "/xenstored" ? The prefix "/" just makes sure that
> only the file in the current directory is ignored, and not any
> "xenstored" in subdirectory. Just in case.
Okay.

> > diff --git a/tools/xenstored/Makefile b/tools/xenstored/Makefile
> > new file mode 100644
> > index 0000000000..f3bd3d43c4
> > --- /dev/null
> > +++ b/tools/xenstored/Makefile
> > @@ -0,0 +1,48 @@
> > +XEN_ROOT=$(CURDIR)/../..
> > +include $(XEN_ROOT)/tools/Rules.mk
> > +
> > +include Makefile.common
> > +
> > +xenstored: LDLIBS += $(LDLIBS_libxenevtchn)
> > +xenstored: LDLIBS += $(LDLIBS_libxengnttab)
> > +xenstored: LDLIBS += $(LDLIBS_libxenctrl)
> > +xenstored: LDLIBS += -lrt
> > +xenstored: LDLIBS += $(SOCKET_LIBS)
> > +
> > +ifeq ($(CONFIG_SYSTEMD),y)
> > +$(XENSTORED_OBJS-y): CFLAGS += $(SYSTEMD_CFLAGS)
> > +xenstored: LDLIBS += $(SYSTEMD_LIBS)
> > +endif
> > +
> > +TARGETS += xenstored
> Could you change that to := instead of += ? TARGETS is currently
> introduced with a := (in tools/xenstore/Makefile).
Fine with me.

> > diff --git a/tools/xs-clients/.gitignore b/tools/xs-clients/.gitignore
> > new file mode 100644
> > index 0000000000..233fd8228a
> > --- /dev/null
> > +++ b/tools/xs-clients/.gitignore
> > @@ -0,0 +1,10 @@
> > +xenstore
> > +xenstore-chmod
> > +xenstore-control
> > +xenstore-exists
> > +xenstore-list
> > +xenstore-ls
> > +xenstore-read
> > +xenstore-rm
> > +xenstore-watch
> > +xenstore-write
> Same thing here, could you prefix all those entries with "/"?
Yes.

> > diff --git a/tools/xenstore/Makefile b/tools/xs-clients/Makefile
> > similarity index 74%
> > rename from tools/xenstore/Makefile
> > rename to tools/xs-clients/Makefile
> > index dc39b6cb31..1c5740450a 100644
> > --- a/tools/xenstore/Makefile
> > +++ b/tools/xs-clients/Makefile
> I'm tempted to ask for the targets "clients-install" and
> "clients-uninstall" to be removed from this makefile. Nothing is calling
> them in our build system and something outside the git tree that rely on
> that would need to be adjusted to the new directory. But maybe that can
> be done in a followup patch as it would help with reverting it if the
> targets are actually useful.
I'll remove those.

> In any case, the patch is already good:
> Acked-by: Anthony PERARD <anthony.perard@xxxxxxxxxx>
Thanks,

Juergen

Attachment: OpenPGP_0xB0DE9DD628BF132F.asc
Description: OpenPGP public key

Attachment: OpenPGP_signature.asc
Description: OpenPGP digital signature