Re: [Buildroot] [PATCH 1/1] package: add agent-proxy target and host packages

From: Thomas Petazzoni via buildroot <buildroot@buildroot.org>
To: Rufus Segar <rhs@riseup.net>
Cc: buildroot@buildroot.org
Subject: Re: [Buildroot] [PATCH 1/1] package: add agent-proxy target and host packages
Date: Fri, 14 Oct 2022 23:11:47 +0200	[thread overview]
Message-ID: <20221014231147.05459ac1@windsurf> (raw)
In-Reply-To: <20220507161338.2125859-1-rhs@riseup.net>

Hello Rugus,

On Sat,  7 May 2022 17:13:37 +0100
Rufus Segar <rhs@riseup.net> wrote:

> Signed-off-by: Rufus Segar <rhs@riseup.net>

Thanks a lot for your contribution. Believe it or not, but after some
many months, I finally took the time to review and apply your patch.

However, I did quite a few changes:

 - I kept only the host variant of the package. Indeed, my
   understanding is that agent-proxy is meant to be executed on the
   developer workstation to de-multiplex the kgdb traffic from the
   console traffic. Therefore it was not clear why a target variant was
   needed. Of course, if additional justification/explanation is given,
   the target variant can be re-added via a follow-up patch.

 - Addition of an entry in the DEVELOPERS file

 - Addition of a .hash file with the hash of the tarball and hash of
   the COPYING file.

See below for some other comments.

> +AGENT_PROXY_VERSION = agent-proxy-1.97
> +AGENT_PROXY_SITE = git://git.kernel.org/pub/scm/utils/kernel/kgdb/agent-proxy.git
> +AGENT_PROXY_LICENSE = GPL-2.0+

This was not correct: the license is GPL-2.0 only.

> +AGENT_PROXY_LICENSE_FILES = COPYING
> +
> +define AGENT_PROXY_BUILD_CMDS
> +	$(MAKE) $(TARGET_CONFIGURE_OPTS) -C $(@D) all
> +endef
> +
> +define AGENT_PROXY_INSTALL_STAGING_CMDS
> +	$(INSTALL) -D -m 0755 $(@D)/agent-proxy $(STAGING_DIR)/usr/bin

This had not effect since AGENT_PROXY_INSTALL_STAGING is not set to
YES. But anyway, I dropped this since I dropped the target variant;

> +endef
> +
> +define AGENT_PROXY_INSTALL_TARGET_CMDS
> +	$(INSTALL) -D -m 0755 $(@D)/agent-proxy $(TARGET_DIR)/usr/bin
> +endef
> +
> +define HOST_AGENT_PROXY_BUILD_CMDS
> +	$(HOST_CONFIGURE_OPTS) $(MAKE) -C $(@D)
> +endef
> +
> +define HOST_AGENT_PROXY_INSTALL_CMDS
> +	$(HOST_CONFIGURE_OPTS) $(INSTALL) -D -m 0755 $(@D)/agent-proxy $(HOST_DIR)/bin

$(HOST_CONFIGURE_OPTS) not needed here, and we want the full
destination path as the last argument, i.e $(HOST_DIR)/bin/agent-proxy.

Once again, thanks a lot for this contribution!

Best regards,

Thomas
-- 
Thomas Petazzoni, co-owner and CEO, Bootlin
Embedded Linux and Kernel engineering and training
https://bootlin.com
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot