diff mbox series

[next,06/12] package/tinifier: new package

Message ID 20201119213658.1232531-7-thomas.petazzoni@bootlin.com
State Superseded
Headers show
Series Support for Cargo and Go vendoring | expand

Commit Message

Thomas Petazzoni Nov. 19, 2020, 9:36 p.m. UTC
This is a Go package that needs vendor modules to be downloaded at
build time.

Signed-off-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
---
 DEVELOPERS                     |  1 +
 package/Config.in              |  1 +
 package/tinifier/Config.in     | 10 ++++++++++
 package/tinifier/tinifier.hash |  3 +++
 package/tinifier/tinifier.mk   | 13 +++++++++++++
 5 files changed, 28 insertions(+)
 create mode 100644 package/tinifier/Config.in
 create mode 100644 package/tinifier/tinifier.hash
 create mode 100644 package/tinifier/tinifier.mk

Comments

Ryan Barnett Nov. 21, 2020, 4:37 p.m. UTC | #1
Thomas,

I've taken a preliminary look at your pkg-mgr patch series and will be
providing additional feedback over the next few days as I have a
chance to test it further.

On Thu, Nov 19, 2020 at 3:37 PM Thomas Petazzoni
<thomas.petazzoni@bootlin.com> wrote:
>
> This is a Go package that needs vendor modules to be downloaded at
> build time.
>
> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
> ---
>  DEVELOPERS                     |  1 +
>  package/Config.in              |  1 +
>  package/tinifier/Config.in     | 10 ++++++++++
>  package/tinifier/tinifier.hash |  3 +++
>  package/tinifier/tinifier.mk   | 13 +++++++++++++
>  5 files changed, 28 insertions(+)
>  create mode 100644 package/tinifier/Config.in
>  create mode 100644 package/tinifier/tinifier.hash
>  create mode 100644 package/tinifier/tinifier.mk

[...]

> diff --git a/package/tinifier/tinifier.mk b/package/tinifier/tinifier.mk
> new file mode 100644
> index 0000000000..b47d265a8e
> --- /dev/null
> +++ b/package/tinifier/tinifier.mk
> @@ -0,0 +1,13 @@
> +################################################################################
> +#
> +# tinifier
> +#
> +################################################################################
> +
> +TINIFIER_VERSION = 2.1.0
> +TINIFIER_SITE = $(call github,tarampampam,tinifier,v$(TINIFIER_VERSION))
> +TINIFIER_LICENSE = MIT
> +TINIFIER_LICENSE_FILES = LICENSE

I took a look at the legal-info side in regards to downloading
packages with the post-processing support. This has been discussed
previously on the patch "[v3,09/10] package/ripgrep: add legal-info
for dependencies":

https://patchwork.ozlabs.org/project/buildroot/patch/20200220160119.3407-9-patrick.havelange@essensium.com/

When I ran 'make legal-info' for the tinifier package all that is
mentioned in the 'manifest.csv' file for the package is:

   "tinifier","2.1.0","MIT","LICENSE","tinifier-2.1.0.tar.gz","https://github.com/tarampampam/tinifier/archive/v2.1.0","skeleton-init-common
[unknown] skeleton-init-none [unknown] toolchain-external-bootlin
[unknown]"

This doesn't give any indication or warnings that dependencies were
downloaded or that other open source license could be needed by
including this package. As user of buildroot who may not have any
knowledge in regards to 'go' or 'cargo'. When they see tinifier row in
the manifest.csv file, they could just think that the tinifier package
would only have/introduce the MIT license to their product. Which is
not the case because it downloads the following vendor packages:

  bou.ke/monkey v1.0.2
  github.com/dustin/go-humanize v1.0.0
  github.com/jessevdk/go-flags v1.4.1-0.20181221193153-c0795c8afcf4
  github.com/json-iterator/go v1.1.10
  github.com/kami-zh/go-capturer v0.0.0-20171211120116-e492ea43421d
  github.com/modern-go/concurrent v0.0.0-20180306012644-bacd9c7ef1dd // indirect
  github.com/modern-go/reflect2 v1.0.1 // indirect
  github.com/olekukonko/tablewriter v0.0.4
  github.com/schollz/progressbar/v3 v3.3.3
  github.com/stretchr/testify v1.6.1

I understand add/showing these license and how to exactly handle this
are future additions.  However, I re-read the section in regards to
the "legal-info" in the buildroot manual I came across this:

  Moreover, due to technical limitations, Buildroot does not produce some
  material that you will or may need, such as the toolchain source code for
  some of the external toolchains and the Buildroot source code itself.
  When you run +make legal-info+, Buildroot produces warnings in the +README+
  file to inform you of relevant material that could not be saved.

So would it be possible to put a warning into the 'legal-info/README'
file that not all the dependency licenses could be downloaded/added to
manifest.csv file?

Maybe a hint could be given to go take a look at the tinifier/go.mod
under the requires section to figure out the licenses. I think this
could be a good temporary solution under a more "dynamic legal-info"
infrastructure could be introduced. Would there be a way to detect if
there are any vendor packages downloaded and then add a warning in the
'legal-info/README' file easily?

Thanks,
-Ryan Barnett

> +TINIFIER_GOMOD = tinifier
> +
> +$(eval $(golang-package))
> --
> 2.28.0
Yann E. MORIN Nov. 21, 2020, 6:04 p.m. UTC | #2
Ryan, All,

On 2020-11-21 10:37 -0600, Ryan Barnett spake thusly:
> On Thu, Nov 19, 2020 at 3:37 PM Thomas Petazzoni
> <thomas.petazzoni@bootlin.com> wrote:
> > This is a Go package that needs vendor modules to be downloaded at
> > build time.
[--SNIP--]
> > diff --git a/package/tinifier/tinifier.mk b/package/tinifier/tinifier.mk
> > new file mode 100644
> > index 0000000000..b47d265a8e
> > --- /dev/null
> > +++ b/package/tinifier/tinifier.mk
> > @@ -0,0 +1,13 @@
> > +################################################################################
> > +#
> > +# tinifier
> > +#
> > +################################################################################
> > +
> > +TINIFIER_VERSION = 2.1.0
> > +TINIFIER_SITE = $(call github,tarampampam,tinifier,v$(TINIFIER_VERSION))
> > +TINIFIER_LICENSE = MIT
> > +TINIFIER_LICENSE_FILES = LICENSE
> 
> I took a look at the legal-info side in regards to downloading
> packages with the post-processing support. This has been discussed
> previously on the patch "[v3,09/10] package/ripgrep: add legal-info
> for dependencies":
> 
> https://patchwork.ozlabs.org/project/buildroot/patch/20200220160119.3407-9-patrick.havelange@essensium.com/

legal-info is also something Thomas and I discussed and IRC when he
posted his series.

We know it is not perfect, but this can be extended in a followup
series.

> When I ran 'make legal-info' for the tinifier package all that is
> mentioned in the 'manifest.csv' file for the package is:
> 
>    "tinifier","2.1.0","MIT","LICENSE","tinifier-2.1.0.tar.gz","https://github.com/tarampampam/tinifier/archive/v2.1.0","skeleton-init-common
> [unknown] skeleton-init-none [unknown] toolchain-external-bootlin
> [unknown]"
> 
> This doesn't give any indication or warnings that dependencies were
> downloaded or that other open source license could be needed by
> including this package.

To simplify the series, my position as a first step would be to extend
the FOO_LICENSE list in the infra, with just a very short notice,
something like:

    FOO_LICENSE += , vendored licenses not listed

> As user of buildroot who may not have any
> knowledge in regards to 'go' or 'cargo'. When they see tinifier row in
> the manifest.csv file, they could just think that the tinifier package
> would only have/introduce the MIT license to their product. Which is
> not the case because it downloads the following vendor packages:
> 
>   bou.ke/monkey v1.0.2
>   github.com/dustin/go-humanize v1.0.0
>   github.com/jessevdk/go-flags v1.4.1-0.20181221193153-c0795c8afcf4
>   github.com/json-iterator/go v1.1.10
>   github.com/kami-zh/go-capturer v0.0.0-20171211120116-e492ea43421d
>   github.com/modern-go/concurrent v0.0.0-20180306012644-bacd9c7ef1dd // indirect
>   github.com/modern-go/reflect2 v1.0.1 // indirect
>   github.com/olekukonko/tablewriter v0.0.4
>   github.com/schollz/progressbar/v3 v3.3.3
>   github.com/stretchr/testify v1.6.1
> 
> I understand add/showing these license and how to exactly handle this
> are future additions.

My idea is that the go/cargo/... infras would be responsible for
providing "some kind of" post-legal-info hooks, so they can extend the
licenses list and license files list as well.

But really, I would like to make that a next step, so that the technical
side of the support for package managers can get in sooner rather than
later.

If we can not in the end come up woth a satifying licensing report for
those (or for some of those) package managers, we would at least have
support for building them.

FTR, Thomas and I already adressed that issue quite a while ago, and we
concluded that it was not so obvious as one may initially think (I'd
have to dig my IRC logs to find the explanations...)

>  However, I re-read the section in regards to
> the "legal-info" in the buildroot manual I came across this:
> 
>   Moreover, due to technical limitations, Buildroot does not produce some
>   material that you will or may need, such as the toolchain source code for
>   some of the external toolchains and the Buildroot source code itself.
>   When you run +make legal-info+, Buildroot produces warnings in the +README+
>   file to inform you of relevant material that could not be saved.
> 
> So would it be possible to put a warning into the 'legal-info/README'
> file that not all the dependency licenses could be downloaded/added to
> manifest.csv file?

With the added blurb I suggest above, I think we would be pretty much
covered, no?

Regards,
Yann E. MORIN.

> Maybe a hint could be given to go take a look at the tinifier/go.mod
> under the requires section to figure out the licenses. I think this
> could be a good temporary solution under a more "dynamic legal-info"
> infrastructure could be introduced. Would there be a way to detect if
> there are any vendor packages downloaded and then add a warning in the
> 'legal-info/README' file easily?
> 
> Thanks,
> -Ryan Barnett
> 
> > +TINIFIER_GOMOD = tinifier
> > +
> > +$(eval $(golang-package))
> > --
> > 2.28.0
> _______________________________________________
> buildroot mailing list
> buildroot@busybox.net
> http://lists.busybox.net/mailman/listinfo/buildroot
Christian Stewart Nov. 22, 2020, 6:08 a.m. UTC | #3
Yann,


On Sat, Nov 21, 2020 at 10:04 AM Yann E. MORIN <yann.morin.1998@free.fr> wrote:
> > I took a look at the legal-info side in regards to downloading
> > packages with the post-processing support. This has been discussed
> > previously on the patch "[v3,09/10] package/ripgrep: add legal-info
> > for dependencies":
> >
> > https://patchwork.ozlabs.org/project/buildroot/patch/20200220160119.3407-9-patrick.havelange@essensium.com/
>
> legal-info is also something Thomas and I discussed and IRC when he
> posted his series.
>
> We know it is not perfect, but this can be extended in a followup
> series.

> > I understand add/showing these license and how to exactly handle this
> > are future additions.
>
> My idea is that the go/cargo/... infras would be responsible for
> providing "some kind of" post-legal-info hooks, so they can extend the
> licenses list and license files list as well.
>
> But really, I would like to make that a next step, so that the technical
> side of the support for package managers can get in sooner rather than
> later.
>
> If we can not in the end come up woth a satifying licensing report for
> those (or for some of those) package managers, we would at least have
> support for building them.
>
> FTR, Thomas and I already adressed that issue quite a while ago, and we
> concluded that it was not so obvious as one may initially think (I'd
> have to dig my IRC logs to find the explanations...)

If you're open to building + running host Go code as part of this
process, then it is straightforward to extract the module dependency
graph and the Licenses.

If we're doing this bash + makefile purism, then this is definitely not easy.

Best,
Christian
Sam Voss Nov. 22, 2020, 8:25 p.m. UTC | #4
Christian, Yann,

On Sun, Nov 22, 2020 at 12:09 AM Christian Stewart <christian@paral.in> wrote:
>
> Yann,
>
>
> On Sat, Nov 21, 2020 at 10:04 AM Yann E. MORIN <yann.morin.1998@free.fr> wrote:
> > > I took a look at the legal-info side in regards to downloading
> > > packages with the post-processing support. This has been discussed
> > > previously on the patch "[v3,09/10] package/ripgrep: add legal-info
> > > for dependencies":
> > >
> > > https://patchwork.ozlabs.org/project/buildroot/patch/20200220160119.3407-9-patrick.havelange@essensium.com/
> >
> > legal-info is also something Thomas and I discussed and IRC when he
> > posted his series.
> >
> > We know it is not perfect, but this can be extended in a followup
> > series.
>
> > > I understand add/showing these license and how to exactly handle this
> > > are future additions.
> >
> > My idea is that the go/cargo/... infras would be responsible for
> > providing "some kind of" post-legal-info hooks, so they can extend the
> > licenses list and license files list as well.
> >
> > But really, I would like to make that a next step, so that the technical
> > side of the support for package managers can get in sooner rather than
> > later.
> >
> > If we can not in the end come up woth a satifying licensing report for
> > those (or for some of those) package managers, we would at least have
> > support for building them.
> >
> > FTR, Thomas and I already adressed that issue quite a while ago, and we
> > concluded that it was not so obvious as one may initially think (I'd
> > have to dig my IRC logs to find the explanations...)
>
> If you're open to building + running host Go code as part of this
> process, then it is straightforward to extract the module dependency
> graph and the Licenses.
>
> If we're doing this bash + makefile purism, then this is definitely not easy.

Same with cargo - unfortunately though for cargo it isn't baked in and
you need to install a plugin to do it. Not the best...but possible.

Sam
Ryan Barnett Nov. 23, 2020, 2:48 p.m. UTC | #5
Yann, All,

On Sat, Nov 21, 2020 at 12:04 PM Yann E. MORIN <yann.morin.1998@free.fr> wrote:
>
> Ryan, All,
>
> On 2020-11-21 10:37 -0600, Ryan Barnett spake thusly:
> > On Thu, Nov 19, 2020 at 3:37 PM Thomas Petazzoni
> > <thomas.petazzoni@bootlin.com> wrote:
> > > This is a Go package that needs vendor modules to be downloaded at
> > > build time.
> [--SNIP--]
> > > diff --git a/package/tinifier/tinifier.mk b/package/tinifier/tinifier.mk
> > > new file mode 100644
> > > index 0000000000..b47d265a8e
> > > --- /dev/null
> > > +++ b/package/tinifier/tinifier.mk
> > > @@ -0,0 +1,13 @@
> > > +################################################################################
> > > +#
> > > +# tinifier
> > > +#
> > > +################################################################################
> > > +
> > > +TINIFIER_VERSION = 2.1.0
> > > +TINIFIER_SITE = $(call github,tarampampam,tinifier,v$(TINIFIER_VERSION))
> > > +TINIFIER_LICENSE = MIT
> > > +TINIFIER_LICENSE_FILES = LICENSE
> >
> > I took a look at the legal-info side in regards to downloading
> > packages with the post-processing support. This has been discussed
> > previously on the patch "[v3,09/10] package/ripgrep: add legal-info
> > for dependencies":
> >
> > https://patchwork.ozlabs.org/project/buildroot/patch/20200220160119.3407-9-patrick.havelange@essensium.com/
>
> legal-info is also something Thomas and I discussed and IRC when he
> posted his series.
>
> We know it is not perfect, but this can be extended in a followup
> series.
>
> > When I ran 'make legal-info' for the tinifier package all that is
> > mentioned in the 'manifest.csv' file for the package is:
> >
> >    "tinifier","2.1.0","MIT","LICENSE","tinifier-2.1.0.tar.gz","https://github.com/tarampampam/tinifier/archive/v2.1.0","skeleton-init-common
> > [unknown] skeleton-init-none [unknown] toolchain-external-bootlin
> > [unknown]"
> >
> > This doesn't give any indication or warnings that dependencies were
> > downloaded or that other open source license could be needed by
> > including this package.
>
> To simplify the series, my position as a first step would be to extend
> the FOO_LICENSE list in the infra, with just a very short notice,
> something like:
>
>     FOO_LICENSE += , vendored licenses not listed

Adding this within the go/cargo infrastructure works for since it
informs a user that the package has licenses not listed.

Would a note in the buildroot manual be useful under the legal-info
section mention the go/cargo license limitations be useful as well?

> > As user of buildroot who may not have any
> > knowledge in regards to 'go' or 'cargo'. When they see tinifier row in
> > the manifest.csv file, they could just think that the tinifier package
> > would only have/introduce the MIT license to their product. Which is
> > not the case because it downloads the following vendor packages:
> >
> >   bou.ke/monkey v1.0.2
> >   github.com/dustin/go-humanize v1.0.0
> >   github.com/jessevdk/go-flags v1.4.1-0.20181221193153-c0795c8afcf4
> >   github.com/json-iterator/go v1.1.10
> >   github.com/kami-zh/go-capturer v0.0.0-20171211120116-e492ea43421d
> >   github.com/modern-go/concurrent v0.0.0-20180306012644-bacd9c7ef1dd // indirect
> >   github.com/modern-go/reflect2 v1.0.1 // indirect
> >   github.com/olekukonko/tablewriter v0.0.4
> >   github.com/schollz/progressbar/v3 v3.3.3
> >   github.com/stretchr/testify v1.6.1
> >
> > I understand add/showing these license and how to exactly handle this
> > are future additions.
>
> My idea is that the go/cargo/... infras would be responsible for
> providing "some kind of" post-legal-info hooks, so they can extend the
> licenses list and license files list as well.
>
> But really, I would like to make that a next step, so that the technical
> side of the support for package managers can get in sooner rather than
> later.
>
> If we can not in the end come up woth a satifying licensing report for
> those (or for some of those) package managers, we would at least have
> support for building them.
>
> FTR, Thomas and I already adressed that issue quite a while ago, and we
> concluded that it was not so obvious as one may initially think (I'd
> have to dig my IRC logs to find the explanations...)

The patchworks link I provided in my original feedback email contains
the IRC convo between you and Thomas.

> >  However, I re-read the section in regards to
> > the "legal-info" in the buildroot manual I came across this:
> >
> >   Moreover, due to technical limitations, Buildroot does not produce some
> >   material that you will or may need, such as the toolchain source code for
> >   some of the external toolchains and the Buildroot source code itself.
> >   When you run +make legal-info+, Buildroot produces warnings in the +README+
> >   file to inform you of relevant material that could not be saved.
> >
> > So would it be possible to put a warning into the 'legal-info/README'
> > file that not all the dependency licenses could be downloaded/added to
> > manifest.csv file?
>
> With the added blurb I suggest above, I think we would be pretty much
> covered, no?

Yes the suggested blurb in the go/cargo infrastructure is good with me.

Thanks,
-Ryan
Thomas Petazzoni Dec. 10, 2020, 9:46 p.m. UTC | #6
Hello,

On Sat, 21 Nov 2020 19:04:18 +0100
"Yann E. MORIN" <yann.morin.1998@free.fr> wrote:

> legal-info is also something Thomas and I discussed and IRC when he
> posted his series.
> 
> We know it is not perfect, but this can be extended in a followup
> series.

Right.

> 
> > When I ran 'make legal-info' for the tinifier package all that is
> > mentioned in the 'manifest.csv' file for the package is:
> > 
> >    "tinifier","2.1.0","MIT","LICENSE","tinifier-2.1.0.tar.gz","https://github.com/tarampampam/tinifier/archive/v2.1.0","skeleton-init-common
> > [unknown] skeleton-init-none [unknown] toolchain-external-bootlin
> > [unknown]"
> > 
> > This doesn't give any indication or warnings that dependencies were
> > downloaded or that other open source license could be needed by
> > including this package.  
> 
> To simplify the series, my position as a first step would be to extend
> the FOO_LICENSE list in the infra, with just a very short notice,
> something like:
> 
>     FOO_LICENSE += , vendored licenses not listed

The problem is that we do not know if the vendoring was done by
Buildroot itself, or if vendored dependencies are provided directly in
the upstream repository, so it's difficult to add this only if
Buildroot has done the vendoring.

In addition, in both cases, the FOO_LICENSE of the package may very
well be completely accurate, taking into account all vendored
dependencies. Indeed, now that they are properly downloaded, nothing
prevents from having correct FOO_LICENSE and FOO_LICENSE_FILES values
for those packages.

Thomas
diff mbox series

Patch

diff --git a/DEVELOPERS b/DEVELOPERS
index d0c9c16423..eb97cfeac0 100644
--- a/DEVELOPERS
+++ b/DEVELOPERS
@@ -2590,6 +2590,7 @@  F:	package/rtc-tools/
 F:	package/sam-ba/
 F:	package/scons/
 F:	package/squashfs/
+F:	package/tinifier/
 F:	package/wayland/
 F:	package/weston/
 F:	support/testing/tests/boot/test_syslinux.py
diff --git a/package/Config.in b/package/Config.in
index d32a271113..a93959ada8 100644
--- a/package/Config.in
+++ b/package/Config.in
@@ -298,6 +298,7 @@  comment "Graphic applications"
 	source "package/rrdtool/Config.in"
 	source "package/stellarium/Config.in"
 	source "package/tesseract-ocr/Config.in"
+	source "package/tinifier/Config.in"
 
 comment "Graphic libraries"
 	source "package/cegui/Config.in"
diff --git a/package/tinifier/Config.in b/package/tinifier/Config.in
new file mode 100644
index 0000000000..fbadfe6bd9
--- /dev/null
+++ b/package/tinifier/Config.in
@@ -0,0 +1,10 @@ 
+config BR2_PACKAGE_TINIFIER
+	bool "tinifier"
+	depends on BR2_PACKAGE_HOST_GO_TARGET_ARCH_SUPPORTS
+	help
+	  CLI tool for images compressing
+
+	  This tool uses tinypng.com API endpoint for compressing your
+	  local jpg/png images (it supports parallel jobs).
+
+	  https://github.com/tarampampam/tinifier
diff --git a/package/tinifier/tinifier.hash b/package/tinifier/tinifier.hash
new file mode 100644
index 0000000000..146700817b
--- /dev/null
+++ b/package/tinifier/tinifier.hash
@@ -0,0 +1,3 @@ 
+# Locally calculated
+sha256  707a1d9e55aab8c83b65bb10f3ec8c3bc094d77cd576b266f821d9f39133ac3c  tinifier-2.1.0.tar.gz
+sha256  791d8fd993ace44d4d83e2f4820a64d5ad3e37412f029afad46d17a9259de2b6  LICENSE
diff --git a/package/tinifier/tinifier.mk b/package/tinifier/tinifier.mk
new file mode 100644
index 0000000000..b47d265a8e
--- /dev/null
+++ b/package/tinifier/tinifier.mk
@@ -0,0 +1,13 @@ 
+################################################################################
+#
+# tinifier
+#
+################################################################################
+
+TINIFIER_VERSION = 2.1.0
+TINIFIER_SITE = $(call github,tarampampam,tinifier,v$(TINIFIER_VERSION))
+TINIFIER_LICENSE = MIT
+TINIFIER_LICENSE_FILES = LICENSE
+TINIFIER_GOMOD = tinifier
+
+$(eval $(golang-package))