[Buildroot] [PATCH] package/swupdate: install headers at the standard location

[Buildroot] [PATCH] package/swupdate: install headers at the standard location

[Thomas Petazzoni via buildroot]

In commit eb2f2886b2364e5b6a0867f55106bf83acb064e7 ("package/swupdate:
add staging install"), the swupdate package was changed to also be
installed in the staging directory, as it provides an API to external
programs, through libraries and headers.

Back then the commit log said:

    This patch installs headers and libs to STAGING_DIR, to prevent a
    header name collision a subdirectory /usr/include/swupdate creates.

And indeed a custom INCLUDEDIR was specified. However, installing
swupdate headers in /usr/include/swupdate/ doesn't work well, as
progress_ipc.h (installed by swupdate) does a #include
<swupdate_status.h>, which doesn't work as swupdate_status.h is also
in /usr/include/swupdate/ instead of /usr/include/.

To address this Daniel Lang submitted a patch [0], which was proposed
to upstream swupdate to fix this header inclusion. The patch was
rejected by upstream saying that the swupdate header files should be
installed in /usr/include.

Currently, swupdate installs only 3 headers files:

  /usr/include/network_ipc.h
  /usr/include/swupdate_status.h
  /usr/include/progress_ipc.h

To the best of our knowledge, this doesn't conflict with any other
header files installed by other packages. A good proof of that is that
even Debian (which has many more packages than Buildroot) also
installs swupdate headers directly in /usr/include.

In addition, the original submitter of
eb2f2886b2364e5b6a0867f55106bf83acb064e7 said "I'm not remember what
was a problem. Probably network_ipc.h but Im not sure now. I think
it's safe to install this headers to /usr/include w/o subdir if this
collision was met only in my environment. PS: currently I don't see
any collisions with this files."

So, we switch to installing swupdate into their standard location.

[0] https://patchwork.ozlabs.org/project/buildroot/patch/VI1P190MB0493D233ECC579510F8C0FC69FB29@VI1P190MB0493.EURP190.PROD.OUTLOOK.COM/

Signed-off-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
---
 package/swupdate/swupdate.mk | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/package/swupdate/swupdate.mk b/package/swupdate/swupdate.mk
index 11a3946753..84905d2e0a 100644
--- a/package/swupdate/swupdate.mk
+++ b/package/swupdate/swupdate.mk
@@ -230,7 +230,7 @@ endef
 define SWUPDATE_INSTALL_STAGING_CMDS
 	$(TARGET_MAKE_ENV) $(SWUPDATE_MAKE_ENV) $(MAKE) -C $(@D) \
 		$(SWUPDATE_MAKE_OPTS) DESTDIR=$(STAGING_DIR) \
-		INCLUDEDIR=/usr/include/swupdate install
+		install
 endef
 
 define SWUPDATE_INSTALL_TARGET_CMDS
-- 
2.41.0

_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] [PATCH] package/swupdate: install headers at the standard location

[Daniel Lang]

On 10.08.23 00:05, Thomas Petazzoni via buildroot wrote:
> In commit eb2f2886b2364e5b6a0867f55106bf83acb064e7 ("package/swupdate:
> add staging install"), the swupdate package was changed to also be
> installed in the staging directory, as it provides an API to external
> programs, through libraries and headers.
>
> Back then the commit log said:
>
>     This patch installs headers and libs to STAGING_DIR, to prevent a
>     header name collision a subdirectory /usr/include/swupdate creates.
>
> And indeed a custom INCLUDEDIR was specified. However, installing
> swupdate headers in /usr/include/swupdate/ doesn't work well, as
> progress_ipc.h (installed by swupdate) does a #include
> <swupdate_status.h>, which doesn't work as swupdate_status.h is also
> in /usr/include/swupdate/ instead of /usr/include/.
>
> To address this Daniel Lang submitted a patch [0], which was proposed
> to upstream swupdate to fix this header inclusion. The patch was
> rejected by upstream saying that the swupdate header files should be
> installed in /usr/include.
>
> Currently, swupdate installs only 3 headers files:
>
>   /usr/include/network_ipc.h
>   /usr/include/swupdate_status.h
>   /usr/include/progress_ipc.h
>
> To the best of our knowledge, this doesn't conflict with any other
> header files installed by other packages. A good proof of that is that
> even Debian (which has many more packages than Buildroot) also
> installs swupdate headers directly in /usr/include.
>
> In addition, the original submitter of
> eb2f2886b2364e5b6a0867f55106bf83acb064e7 said "I'm not remember what
> was a problem. Probably network_ipc.h but Im not sure now. I think
> it's safe to install this headers to /usr/include w/o subdir if this
> collision was met only in my environment. PS: currently I don't see
> any collisions with this files."
>
> So, we switch to installing swupdate into their standard location.
>
> [0] https://patchwork.ozlabs.org/project/buildroot/patch/VI1P190MB0493D233ECC579510F8C0FC69FB29@VI1P190MB0493.EURP190.PROD.OUTLOOK.COM/
>
> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com>

Thanks for picking this up and investigating it.
And sorry for not responding earlier.

Tested-by: Daniel Lang <dalang@gmx.at>

_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] [PATCH] package/swupdate: install headers at the standard location

[Thomas Petazzoni via buildroot]

On Thu, 10 Aug 2023 00:05:14 +0200
Thomas Petazzoni via buildroot <buildroot@buildroot.org> wrote:

> In commit eb2f2886b2364e5b6a0867f55106bf83acb064e7 ("package/swupdate:
> add staging install"), the swupdate package was changed to also be
> installed in the staging directory, as it provides an API to external
> programs, through libraries and headers.
> 
> Back then the commit log said:
> 
>     This patch installs headers and libs to STAGING_DIR, to prevent a
>     header name collision a subdirectory /usr/include/swupdate creates.
> 
> And indeed a custom INCLUDEDIR was specified. However, installing
> swupdate headers in /usr/include/swupdate/ doesn't work well, as
> progress_ipc.h (installed by swupdate) does a #include
> <swupdate_status.h>, which doesn't work as swupdate_status.h is also
> in /usr/include/swupdate/ instead of /usr/include/.
> 
> To address this Daniel Lang submitted a patch [0], which was proposed
> to upstream swupdate to fix this header inclusion. The patch was
> rejected by upstream saying that the swupdate header files should be
> installed in /usr/include.
> 
> Currently, swupdate installs only 3 headers files:
> 
>   /usr/include/network_ipc.h
>   /usr/include/swupdate_status.h
>   /usr/include/progress_ipc.h
> 
> To the best of our knowledge, this doesn't conflict with any other
> header files installed by other packages. A good proof of that is that
> even Debian (which has many more packages than Buildroot) also
> installs swupdate headers directly in /usr/include.
> 
> In addition, the original submitter of
> eb2f2886b2364e5b6a0867f55106bf83acb064e7 said "I'm not remember what
> was a problem. Probably network_ipc.h but Im not sure now. I think
> it's safe to install this headers to /usr/include w/o subdir if this
> collision was met only in my environment. PS: currently I don't see
> any collisions with this files."
> 
> So, we switch to installing swupdate into their standard location.
> 
> [0] https://patchwork.ozlabs.org/project/buildroot/patch/VI1P190MB0493D233ECC579510F8C0FC69FB29@VI1P190MB0493.EURP190.PROD.OUTLOOK.COM/
> 
> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
> ---
>  package/swupdate/swupdate.mk | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Applied to next, thanks.

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