[1/1] package/pkg-generic.mk: local and override support in legal-info

From: Nicolas Carrier <carrier.nicolas0@gmail.com>

Sources of packages which are using the 'local' _METHOD_SITE or
which declare an _OVERRIDE_SRC_DIR are not archived when generating
the legal-info target.
This is problematic since those packages are the most likely to
require being published, because it's highly probable that their
sources have been modified.

This patch generates a tar.gz archive on the fly, containing the
result of the rsync of the package's source directory, using the
same arguments as during the preparation of a build.
Note that is not possible to archive the builddir, since it may
contain artifacts of previous compilations.

It is not possible to consider rsync as a "normal" download method
either (which would have solved the initial problem), because if we
do that:
 * the DL dir's content would be altered improperly
 * the rsync + tar gz overhead would be paid on compilation and is
   significant, for example, for a linux kernel
 * a hash file would be necessary, which would be a burden to
   maintain, for sources frequently modified

Signed-off-by: Nicolas Carrier <carrier.nicolas0@gmail.com>
---
 package/pkg-generic.mk | 23 ++++++++++++++++-------
 1 file changed, 16 insertions(+), 7 deletions(-)

Nicolas, All,

Sorry for replying much later than I promised I would do...

On 2023-01-17 12:04 +0100, carrier.nicolas0@gmail.com spake thusly:
> From: Nicolas Carrier <carrier.nicolas0@gmail.com>
> 
> Sources of packages which are using the 'local' _METHOD_SITE or
> which declare an _OVERRIDE_SRC_DIR are not archived when generating
> the legal-info target.

That packages with an override-srcdir are not saved, is indeed
on-purpose. An override-srcdir is meant as a way to develop on a
package, rather than use its official download location.

During development, such packages are most often stored in a local
directory that is not tracked by the VCS (git, svn, etc...) and so
are not reproducible, and thus such a build can't be meaningful as
a production build, and thus we should not be able to produce a
legal-info content in such a case, quite the opposite in fact: we do
want to actually log that override packages are being used and that
they were not saved.

Some people do use that as a safeguard when generating the legal-info
and passing it for publication, to ensure that there are actually no
use of an override-srcdir package and that all the build is actually
reproducible without local hacks. Changing the behaviour as you suggest
would break for those users.

Now, I do understand that you are (ab)using override-srcdir to use
locally modified versions of packages that Buildroot would otherwise
fetch from their remote locations.

In that case, I would advise that you carry the necessary changes on
your Buildroot tree, that change such packages from whatever site and
site method Buildroot uses, to a path as the site, and "local" as the
site method. Yes, this means you need to patch Buildroot, and probably
carry that forever, but that is a very minor change [0].

As for getting in compliance for such local sources, my first comment
would be to point to the statement that the Buildroot developers made
a while back, and that was committed to the manual:

    https://buildroot.org/downloads/manual/manual.html#legal-info-buildroot

    Nevertheless, the general view of the Buildroot developers is that
    you should release the Buildroot source code along with the source
    code of other packages when releasing a product that contains
    GPL-licensed software. This is because the GNU GPL defines the
    "complete source code" for an executable work as "all the source
    code for all modules it contains, plus any associated interface
    definition files, plus the scripts used to control compilation
    and installation of the executable". Buildroot is part of the
    _scripts used to control compilation and installation of the
    executable_, and as such it is considered part of the material
    that must be redistributed.

Indeed, Buildroot contains the recipes that are used to configure,
build, and then install the packages, and in some cases, even hack
into the packages sources (with sed, awk...), and so Buildroot falls
under the _scripts used to control compilation and installation of
the executable_, and providing just the tarball and the patches would
not be enough for those packages.

Also, just providing the Buildroot source tree would not be enough
either, as you'd need to also provide the .config (or defconfig) that
was used to define the build, as that contains options that will
ultimately have an impact on the artifacts (like the -mcpu setting, or
the -O level...), and thus should also be considered as part of the
aforementioned scripts.

With that in mind, if you have packages that are "local" (either by
their _SITE_METHOD or by being _OVERRIDE_SRCDIR), then you hopefully
have them as part of the same VCS tree (or sub-tree, submodule...)
as your Buildroot tree, in which case you should just need to provide
the checkout of that tree that was used to do the build, and be in
compliance for those packages.

If you need to be able to separate FLOSS packages from proprietary ones,
then Buildroot already helps you in that respect, by allowing you to use
more than one br2-external trees at once, for example one with your
FLOSS packages, and one with your proprietary sources; just exclude from
the BR2_EXTERNAL variable, the br2-external tree(s) with your proprietary
stuff, when you prepare your legal-info, and exclude them from the
tarball you generate. If you have no proprietary source, then the point
is moot; you can redistribute everything (but can still use more than
one br2-external for other reasons, of course!).

Bottom line: 1. I am opposed to making it possible to generate local
archives from override-srcdir, and 2. I am not in favour of making it
possible for local packages either.

> This is problematic since those packages are the most likely to
> require being published, because it's highly probable that their
> sources have been modified.

This is totally irrelevant: whether the sources are changed or not does
not drive whether a package sources should be published. Believing so
would be a gross misconception on how licenses such as the GPL work.
Indeed, the GPL requires that the C&CS be published for any covered
work; it does not condition that publication to it having been locally
modified, or not, from wherever you got it from.

[0] The alternative is of course that you push your changes to the
corresponding upstreams, and carry those patches in a global-patch-dir;
for those that are not upstreamable, you'd have to carry them for ever
as well. And I see how it could be cumbersom for the kernel, which is
the reason why we already have a wide range of solution for how to get
the kernel source trees (arbitrary tarball URL, git, mercurial, or svn
trees...).

Regards,
Yann E. MORIN.

> This patch generates a tar.gz archive on the fly, containing the
> result of the rsync of the package's source directory, using the
> same arguments as during the preparation of a build.
> Note that is not possible to archive the builddir, since it may
> contain artifacts of previous compilations.
> 
> It is not possible to consider rsync as a "normal" download method
> either (which would have solved the initial problem), because if we
> do that:
>  * the DL dir's content would be altered improperly
>  * the rsync + tar gz overhead would be paid on compilation and is
>    significant, for example, for a linux kernel
>  * a hash file would be necessary, which would be a burden to
>    maintain, for sources frequently modified
> 
> Signed-off-by: Nicolas Carrier <carrier.nicolas0@gmail.com>
> ---
>  package/pkg-generic.mk | 23 ++++++++++++++++-------
>  1 file changed, 16 insertions(+), 7 deletions(-)
> 
> diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
> index 2f8ba39edf..a8760f40d4 100644
> --- a/package/pkg-generic.mk
> +++ b/package/pkg-generic.mk
> @@ -660,6 +660,12 @@ $$(error $(1) has local site method, but `$(2)_SITE` is not defined)
>  endif
>  endif
>  
> +# for legal-info sources archival
> +ifneq ($$($(2)_OVERRIDE_SRCDIR),)
> +$(2)_OVERRIDE_RSYNC_DIR = $$($(2)_BUILDDIR)/$$($(2)_BASENAME_RAW)
> +$(2)_OVERRIDE_RSYNC_ARCHIVE = $$($(2)_OVERRIDE_RSYNC_DIR).tar.gz
> +endif
> +
>  ifndef $(2)_LICENSE
>   ifdef $(3)_LICENSE
>    $(2)_LICENSE = $$($(3)_LICENSE)
> @@ -1146,13 +1152,16 @@ else
>  endif # license files
>  
>  ifeq ($$($(2)_REDISTRIBUTE),YES)
> -ifeq ($$($(2)_SITE_METHOD),local)
> -# Packages without a tarball: don't save and warn
> -	@$$(call legal-warning-nosource,$$($(2)_RAWNAME),local)
> -
> -else ifneq ($$($(2)_OVERRIDE_SRCDIR),)
> -	@$$(call legal-warning-nosource,$$($(2)_RAWNAME),override)
> -
> +ifneq ($$($(2)_OVERRIDE_SRCDIR),)
> +# Packages without a tarball: create one on the fly
> +	@echo "Package is of type local or override, archive sources"
> +	$$(Q)rm -rf $$($(2)_OVERRIDE_RSYNC_DIR) $$($(2)_OVERRIDE_RSYNC_ARCHIVE)
> +	$$(Q)rsync -au --chmod=u=rwX,go=rX $$(RSYNC_VCS_EXCLUSIONS) $(call qstrip,$$($(2)_OVERRIDE_SRCDIR))/ $$($(2)_OVERRIDE_RSYNC_DIR)/
> +	$$(Q)tar -zcf $$($(2)_OVERRIDE_RSYNC_ARCHIVE) -C $$($(2)_OVERRIDE_RSYNC_DIR) $$$$(ls -A $$($(2)_OVERRIDE_RSYNC_DIR)/)
> +	$$(Q)rm -rf $$($(2)_OVERRIDE_RSYNC_DIR)
> +	$$(Q)support/scripts/hardlink-or-copy \
> +			$$($(2)_OVERRIDE_RSYNC_ARCHIVE) \
> +			$$($(2)_REDIST_SOURCES_DIR)$$(sep)
>  else
>  # Other packages
>  
> -- 
> 2.30.2
> 
> _______________________________________________
> buildroot mailing list
> buildroot@buildroot.org
> https://lists.buildroot.org/mailman/listinfo/buildroot

Forgot to reply all. Sorry about that ^^'

---

Yann, All,

> Nicolas, All,
>
> Sorry for replying much later than I promised I would do...


No problem, thank you for taking the time to answer.

> > This is problematic since those packages are the most likely to
> > require being published, because it's highly probable that their
> > sources have been modified.
>
> This is totally irrelevant: whether the sources are changed or not does
> not drive whether a package sources should be published. Believing so
> would be a gross misconception on how licenses such as the GPL work.
> Indeed, the GPL requires that the C&CS be published for any covered
> work; it does not condition that publication to it having been locally
> modified, or not, from wherever you got it from.

My point was just to emphasize that, among all the packages we are
redistributing, the most important ones are those which are modified,
because it's them who provide the most added value to the FLOSS
ecosystem (if the patches are good, that is ^^). OTHO, non-modified
packages don't provide a lot of value, being already available
elsewhere.
But again, be assured that my aim is to be fully compliant and
redistribute all :)

> During development, such packages are most often stored in a local
> directory that is not tracked by the VCS (git, svn, etc...) and so
> are not reproducible, and thus such a build can't be meaningful as
> a production build [...]

That is not true in general and in our situation in particular, where
we're using repo.
This tool allows us to handle 2 situations in parallel:
* integrate the most recent changes using manifests tracking branches,
in theory not reproducible, (but see later)
* integrate only fixed revisions (sha1, tag) for production (and
reproducible) builds

And even in the first situation, since we keep track of the revisions
used for each build by exporting the repo manifest, those builds
become reproducible too, even if for development purpose only.

> [...] Changing the behaviour as you suggest
> would break for those users.

I don't think that's really an issue, I'll just make this behavior an
option, whose default will be the previous behavior.

> Now, I do understand that you are (ab)using override-srcdir to use
> locally modified versions of packages that Buildroot would otherwise
> fetch from their remote locations.

Again, I'm ok to stop abusing override-srcdir, but the patches to add
a config option to use a local kernel or uboot tree got rejected a
while ago (and it doesn't solve the override problem for all the
packages anyway) and the solution you're proposing me (which I'll
summarize after) doesn't work in our situation.

> In that case, I would advise that you carry the necessary changes on
> your Buildroot tree, [...] in which case you should just need to provide
> the checkout of that tree that was used to do the build, and be in
> compliance for those packages.

Here, I'll try to summarize your reasoning in this section:

* I should patch buildroot to have the sources in it and use local
instead of override
* I should provide everything (patches, buildroot, configs...) to
comply to FLOSS licenses
* once all methods are "local", providing the modified buildroot
source tree should suffice

I'm perfectly ok with the last 2 points, but the first is the blocking
one, for various reasons:
- packages' source codes our developers have to modify, would be split
into 2 locations, proprietary in the external and floss (even if in
house) in the buildroot. That is not a show stopper for sure, but more
of an extra annoyance.

- then, how to integrate the open source packages inside the buildroot tree?
I see two ways:
* copy the source tree into that of buildroot, not an option :)
* use a solution à la submodules / subtrees...
And in fact, we worked hard to transition from a mercurial-based
workspace with submodules, to a repo-based one and the benefits are
really important, so we're not going to make the switch backwards.

- we are working hard, (by ourselves, but also with the help of
Bootlin), to ultimately get rid of all of our buildroot patches. The
main aim for us is to be able to test the integration of buildroot's
master branch, so as to fix integration issues as soon as they happen,
instead of at each year's LTS. For that, reducing our amount of
patches to the minimum, ideally 0, is crucial.