[Buildroot] [PATCH] package/qt5/qt5webkit: fix generated artifacts

[Thomas Ballasi <thomas.ballasi@savoirfairelinux.com>]

Hello Giulio,

On Thu, 29 Sep 2022 12:43:22 +0200
Giulio Benetti <giulio.benetti@benettiengineering.com> wrote:
> > Generated artifacts of the installation process were wrongly
> > located, causing packages using qt5webkit (qt-webkit-kiosk and
> > python-pyqt5) to fail at build time. The changes aims at fixing
> > this issue.
> 
> "The changes aims at fixing this issue." should be:
> "Let's add a patch that:"

This isn't necessarily true. While the patch answers points 1 and 2,
the third one isn't really affected by it. I will try to rephrase the
message for more clarification as the way I phrased it is ambiguous,
thanks.

> What is the upstream status of this patch? Can you point here the URL
> of the pending patch?

This patch isn't pending for merge in the qt5webkit repository for the
reason that it may break other projects. At first, I made this patch to
counter the use of CMAKE_INSTALL_PREFIX_INITIALIZED_TO_DEFAULT, which
CMake's documentation seems to imply that it cannot be set by the user
but is set automatically by CMake, which leads to the unwanted
behavior. The patch makes it as if it was ON specifically for buildroot.

I reckon that it may be bad practice to create such patches, so I just
checked whether or not this variable can be set via CONF_OPTS and it
seems it can, leading to the same working behavior (it also doesn't seem
to be set in CMakeCache by CMake if not manually added).

I also made sure setting this variable doesn't have any potential side
effects as it is only used once in the case inside the patch.

Therefore, it seems to be a better fit to set INITIALIZED_TO_DEFAULT's
value in CONF_OPTS rather than bypassing its usage inside a patch. This
has been replaced.

> > More info @ https://bugs.buildroot.org/show_bug.cgi?id=14606
> 
> Here ^^^ it should be:
> Fixes:
> https://bugs.buildroot.org/show_bug.cgi?id=14606

Done!

> Also here please add:
> '[Upstream status: URL of this pending patch]

Patch is replaced, ignoring.

> The patch works correctly, so with commit log improve and the local 
> patch with Upstream status pointed:
> Reviewed-by: Giulio Benetti <giulio.benetti@benettiengineering.com>
> 
> Thanks for contributing!
> 
> Best regards

Thanks a lot for the constructive review! I will send a v2 asap.

Kind regards,
Thomas Ballasi
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot