diff mbox series

[v2,1/2] iqvlinux: bump to version 1.2.0.3

Message ID 20171215153830.4864-1-casantos@datacom.ind.br
State Accepted
Headers show
Series [v2,1/2] iqvlinux: bump to version 1.2.0.3 | expand

Commit Message

Carlos Santos Dec. 15, 2017, 3:38 p.m. UTC
This package is really annoying since the archive name doesn't contain
the version. Use a small trick to save it with a better name. Also add
hashes for license files.

Signed-off-by: Carlos Santos <casantos@datacom.ind.br>
---
Changes v1->v2:
  - Move comment about the patch, in package/iqvlinux/Config.in, to the
    next commit.
  - Use a small trick to save the source file with a better name.
---
 package/iqvlinux/Config.in     |  2 +-
 package/iqvlinux/iqvlinux.hash |  9 +++++----
 package/iqvlinux/iqvlinux.mk   | 17 ++++++++++++++---
 3 files changed, 20 insertions(+), 8 deletions(-)

Comments

Thomas Petazzoni Dec. 16, 2017, 2:50 p.m. UTC | #1
Hello,

On Fri, 15 Dec 2017 13:38:29 -0200, Carlos Santos wrote:
> This package is really annoying since the archive name doesn't contain
> the version. Use a small trick to save it with a better name. Also add
> hashes for license files.

Have you tried contacting upstreaming to see if they would be willing
to use a more regular tarball naming scheme ?

Thanks!

Thomas
Carlos Santos Dec. 16, 2017, 7:03 p.m. UTC | #2
> From: "Thomas Petazzoni" <thomas.petazzoni@free-electrons.com>
> To: "Carlos Santos" <casantos@datacom.ind.br>
> Cc: buildroot@buildroot.org, "Romain Naour" <romain.naour@gmail.com>
> Sent: Saturday, December 16, 2017 12:50:53 PM
> Subject: Re: [Buildroot] [PATCH v2 1/2] iqvlinux: bump to version 1.2.0.3

> Hello,
> 
> On Fri, 15 Dec 2017 13:38:29 -0200, Carlos Santos wrote:
>> This package is really annoying since the archive name doesn't contain
>> the version. Use a small trick to save it with a better name. Also add
>> hashes for license files.
> 
> Have you tried contacting upstreaming to see if they would be willing
> to use a more regular tarball naming scheme ?

I will try it. Meanwhile we need a temporary solution.
Carlos Santos Dec. 16, 2017, 7:53 p.m. UTC | #3
> From: "Carlos Santos" <casantos@datacom.ind.br>
> To: "Thomas Petazzoni" <thomas.petazzoni@free-electrons.com>
> Cc: "Romain Naour" <romain.naour@gmail.com>, buildroot@buildroot.org
> Sent: Saturday, December 16, 2017 5:03:08 PM
> Subject: Re: [Buildroot] [PATCH v2 1/2] iqvlinux: bump to version 1.2.0.3

>> From: "Thomas Petazzoni" <thomas.petazzoni@free-electrons.com>
>> To: "Carlos Santos" <casantos@datacom.ind.br>
>> Cc: buildroot@buildroot.org, "Romain Naour" <romain.naour@gmail.com>
>> Sent: Saturday, December 16, 2017 12:50:53 PM
>> Subject: Re: [Buildroot] [PATCH v2 1/2] iqvlinux: bump to version 1.2.0.3
> 
>> Hello,
>> 
>> On Fri, 15 Dec 2017 13:38:29 -0200, Carlos Santos wrote:
>>> This package is really annoying since the archive name doesn't contain
>>> the version. Use a small trick to save it with a better name. Also add
>>> hashes for license files.
>> 
>> Have you tried contacting upstreaming to see if they would be willing
>> to use a more regular tarball naming scheme ?
> 
> I will try it. Meanwhile we need a temporary solution.

BTW, I their current naming scheme is already reasonable, since each
release use a separate path. Those who download the file can save it
with a different name. That's not rocket science.
Thomas Petazzoni Dec. 17, 2017, 4:05 a.m. UTC | #4
Hello,

On Sat, 16 Dec 2017 17:53:12 -0200 (BRST), Carlos Santos wrote:

> > I will try it. Meanwhile we need a temporary solution.  
> 
> BTW, I their current naming scheme is already reasonable, since each
> release use a separate path. Those who download the file can save it
> with a different name. That's not rocket science.

Except it's totally uncommon, and many people save tarballs in a single
folder. What they do is contrary to the practice of pretty much every
other open-source project.

Thomas
Thomas Petazzoni Jan. 9, 2018, 9:56 a.m. UTC | #5
Hello Carlos,

On Sun, 17 Dec 2017 05:05:54 +0100, Thomas Petazzoni wrote:
> On Sat, 16 Dec 2017 17:53:12 -0200 (BRST), Carlos Santos wrote:
> 
> > > I will try it. Meanwhile we need a temporary solution.    
> > 
> > BTW, I their current naming scheme is already reasonable, since each
> > release use a separate path. Those who download the file can save it
> > with a different name. That's not rocket science.  
> 
> Except it's totally uncommon, and many people save tarballs in a single
> folder. What they do is contrary to the practice of pretty much every
> other open-source project.

In the end, did you get the chance to raise the issue of the tarball
name with the upstream developers ?

Thanks!

Thomas
Carlos Santos Jan. 10, 2018, 1:56 a.m. UTC | #6
> From: "Thomas Petazzoni" <thomas.petazzoni@free-electrons.com>
> To: "Carlos Santos" <casantos@datacom.ind.br>
> Cc: "Romain Naour" <romain.naour@gmail.com>, buildroot@buildroot.org
> Sent: Tuesday, January 9, 2018 7:56:28 AM
> Subject: Re: [Buildroot] [PATCH v2 1/2] iqvlinux: bump to version 1.2.0.3

> Hello Carlos,
> 
> On Sun, 17 Dec 2017 05:05:54 +0100, Thomas Petazzoni wrote:
>> On Sat, 16 Dec 2017 17:53:12 -0200 (BRST), Carlos Santos wrote:
>> 
>> > > I will try it. Meanwhile we need a temporary solution.
>> > 
>> > BTW, I their current naming scheme is already reasonable, since each
>> > release use a separate path. Those who download the file can save it
>> > with a different name. That's not rocket science.
>> 
>> Except it's totally uncommon, and many people save tarballs in a single
>> folder. What they do is contrary to the practice of pretty much every
>> other open-source project.
> 
> In the end, did you get the chance to raise the issue of the tarball
> name with the upstream developers ?

Thanks for reminding me of this issue. I just submitted a ticket:

    https://sourceforge.net/p/e1000/bugs/589/
Carlos Santos Feb. 14, 2018, 11:38 a.m. UTC | #7
> From: "Carlos Santos" <casantos@datacom.ind.br>
> To: "Thomas Petazzoni" <thomas.petazzoni@free-electrons.com>
> Cc: "Romain Naour" <romain.naour@gmail.com>, "buildroot" <buildroot@buildroot.org>
> Sent: Tuesday, January 9, 2018 11:56:10 PM
> Subject: Re: [Buildroot] [PATCH v2 1/2] iqvlinux: bump to version 1.2.0.3

>> From: "Thomas Petazzoni" <thomas.petazzoni@free-electrons.com>
>> To: "Carlos Santos" <casantos@datacom.ind.br>
>> Cc: "Romain Naour" <romain.naour@gmail.com>, buildroot@buildroot.org
>> Sent: Tuesday, January 9, 2018 7:56:28 AM
>> Subject: Re: [Buildroot] [PATCH v2 1/2] iqvlinux: bump to version 1.2.0.3
> 
>> Hello Carlos,
>> 
>> On Sun, 17 Dec 2017 05:05:54 +0100, Thomas Petazzoni wrote:
>>> On Sat, 16 Dec 2017 17:53:12 -0200 (BRST), Carlos Santos wrote:
>>> 
>>> > > I will try it. Meanwhile we need a temporary solution.
>>> > 
>>> > BTW, I their current naming scheme is already reasonable, since each
>>> > release use a separate path. Those who download the file can save it
>>> > with a different name. That's not rocket science.
>>> 
>>> Except it's totally uncommon, and many people save tarballs in a single
>>> folder. What they do is contrary to the practice of pretty much every
>>> other open-source project.
>> 
>> In the end, did you get the chance to raise the issue of the tarball
>> name with the upstream developers ?
> 
> Thanks for reminding me of this issue. I just submitted a ticket:
> 
>    https://sourceforge.net/p/e1000/bugs/589/
> 

No love, so far. :-(
Thomas Petazzoni Feb. 14, 2018, 12:38 p.m. UTC | #8
Hello,

On Wed, 14 Feb 2018 09:38:06 -0200 (BRST), Carlos Santos wrote:

> >> In the end, did you get the chance to raise the issue of the tarball
> >> name with the upstream developers ?  
> > 
> > Thanks for reminding me of this issue. I just submitted a ticket:
> > 
> >    https://sourceforge.net/p/e1000/bugs/589/
> 
> No love, so far. :-(

I pinged on the bug report, just in case, but indeed, there isn't much
hope.

Thomas
Carlos Santos Feb. 14, 2018, 7:37 p.m. UTC | #9
> From: "Thomas Petazzoni" <thomas.petazzoni@bootlin.com>
> To: "Carlos Santos" <casantos@datacom.ind.br>
> Cc: "Thomas Petazzoni" <thomas.petazzoni@free-electrons.com>, "Romain Naour" <romain.naour@gmail.com>, "buildroot"
> <buildroot@buildroot.org>
> Sent: Wednesday, February 14, 2018 10:38:56 AM
> Subject: Re: [Buildroot] [PATCH v2 1/2] iqvlinux: bump to version 1.2.0.3

> Hello,
> 
> On Wed, 14 Feb 2018 09:38:06 -0200 (BRST), Carlos Santos wrote:
> 
>> >> In the end, did you get the chance to raise the issue of the tarball
>> >> name with the upstream developers ?
>> > 
>> > Thanks for reminding me of this issue. I just submitted a ticket:
>> > 
>> >    https://sourceforge.net/p/e1000/bugs/589/
>> 
>> No love, so far. :-(
> 
> I pinged on the bug report, just in case, but indeed, there isn't much
> hope.

So we cannot upgrade the package without some hack to like the one in

    https://patchwork.ozlabs.org/patch/849230/

It works, but uses $(DL_WRAPPER), which should be opaque to recipes.
Thomas Petazzoni Feb. 14, 2018, 7:46 p.m. UTC | #10
Hello,

+Yann in Cc.

On Wed, 14 Feb 2018 17:37:33 -0200 (BRST), Carlos Santos wrote:
> > From: "Thomas Petazzoni" <thomas.petazzoni@bootlin.com>
> > To: "Carlos Santos" <casantos@datacom.ind.br>
> > Cc: "Thomas Petazzoni" <thomas.petazzoni@free-electrons.com>, "Romain Naour" <romain.naour@gmail.com>, "buildroot"
> > <buildroot@buildroot.org>
> > Sent: Wednesday, February 14, 2018 10:38:56 AM
> > Subject: Re: [Buildroot] [PATCH v2 1/2] iqvlinux: bump to version 1.2.0.3  
> 
> > Hello,
> > 
> > On Wed, 14 Feb 2018 09:38:06 -0200 (BRST), Carlos Santos wrote:
> >   
> >> >> In the end, did you get the chance to raise the issue of the tarball
> >> >> name with the upstream developers ?  
> >> > 
> >> > Thanks for reminding me of this issue. I just submitted a ticket:
> >> > 
> >> >    https://sourceforge.net/p/e1000/bugs/589/  
> >> 
> >> No love, so far. :-(  
> > 
> > I pinged on the bug report, just in case, but indeed, there isn't much
> > hope.  
> 
> So we cannot upgrade the package without some hack to like the one in
> 
>     https://patchwork.ozlabs.org/patch/849230/
> 
> It works, but uses $(DL_WRAPPER), which should be opaque to recipes.

Either that, or we add explicit support for this in the package
infrastructure, but it's a bit annoying. Indeed, if you have several
files to download (in <pkg>_SOURCE, <pkg>_EXTRA_DOWNLOADS and
<pkg>_PATCH), specifying what destination file name should be used is a
bit tricky.

Yann, your thoughts ?

Thomas
Thomas Petazzoni April 1, 2018, 10:37 p.m. UTC | #11
Hello,

On Fri, 15 Dec 2017 13:38:29 -0200, Carlos Santos wrote:
> This package is really annoying since the archive name doesn't contain
> the version. Use a small trick to save it with a better name. Also add
> hashes for license files.
> 
> Signed-off-by: Carlos Santos <casantos@datacom.ind.br>

So back on this driver. We looked at it with Arnout and Peter, and
actually had a lot of fun reading the code. It is ugly as hell, uses
copy_from/to_user inside spinlocks, and does lots of horrible things.

But more importantly, this driver does nothing, except provide a weird
custom ioctl() interface, with no user-space program to use them.

How is this kernel driver meant to be used? Is it used with some
non-public, closed-source, Intel utilities ?

Also, in the bug tracker at https://sourceforge.net/p/e1000/bugs/589/,
the person from Intel said: "I'm not really sure why you're trying to
build this as it's only published here to conform with GPLv2. There's
really no use for it.". So why are you using it ? Why do we have it in
Buildroot ?

Romain, since you originally contributed this driver, perhaps you can
give some details about how you used it ?

Thanks!

Thomas
Thomas Petazzoni April 2, 2018, 9:26 a.m. UTC | #12
Hello,

On Fri, 15 Dec 2017 13:38:29 -0200, Carlos Santos wrote:

> +# This package is really annoying since the archive name doesn't contain the
> +# version. Use a small trick to save it with a better name.
> +define IQVLINUX_DOWNLOAD
> +	$(EXTRA_ENV) $(DL_WRAPPER) -b wget \
> +		-o $(DL_DIR)/$(IQVLINUX_SOURCE) \
> +		-H $(PKGDIR)/$($(PKG)_RAWNAME).hash \
> +		-- \
> +		$(IQVLINUX_SITE)/iqvlinux.tar.gz
> +endef
> +
> +IQVLINUX_PRE_DOWNLOAD_HOOKS = IQVLINUX_DOWNLOAD

So instead of this, I've added an iqvlinux-1.2.0.3.tar.gz tarball on
sources.buildroot.net, and just kept iqvlinux download first from
SourceForge (which fails with a 404), and then fall back to
sources.buildroot.net.

Accessing directly the download helper is not a good idea. The download
helpers are internal, and they are being changed by Maxime/Yann patch
series introducing Git caching, so we really don't want packages to
mess up with the download internals.

It would be good if you could reply on the bug report upstream, explain
why and how you use iqvlinux, and convince them to fix their tarball
name.

I've applied both patches to master, after doing the change explained
above. Thanks!

Thomas
Carlos Santos April 3, 2018, 3:29 a.m. UTC | #13
> From: "Thomas Petazzoni" <thomas.petazzoni@bootlin.com>
> To: "Carlos Santos" <casantos@datacom.ind.br>
> Cc: "buildroot" <buildroot@buildroot.org>, "Romain Naour" <romain.naour@gmail.com>
> Sent: Sunday, April 1, 2018 7:37:02 PM
> Subject: Re: [Buildroot] [PATCH v2 1/2] iqvlinux: bump to version 1.2.0.3

> Hello,
> 
> On Fri, 15 Dec 2017 13:38:29 -0200, Carlos Santos wrote:
>> This package is really annoying since the archive name doesn't contain
>> the version. Use a small trick to save it with a better name. Also add
>> hashes for license files.
>> 
>> Signed-off-by: Carlos Santos <casantos@datacom.ind.br>
> 
> So back on this driver. We looked at it with Arnout and Peter, and
> actually had a lot of fun reading the code. It is ugly as hell, uses
> copy_from/to_user inside spinlocks, and does lots of horrible things.
> 
> But more importantly, this driver does nothing, except provide a weird
> custom ioctl() interface, with no user-space program to use them.
> 
> How is this kernel driver meant to be used? Is it used with some
> non-public, closed-source, Intel utilities ?

It is used for testing/debug purposes by some closed-source binaries
that Intel distributes in its Quartzville Tools, under NDA.

> Also, in the bug tracker at https://sourceforge.net/p/e1000/bugs/589/,
> the person from Intel said: "I'm not really sure why you're trying to
> build this as it's only published here to conform with GPLv2. There's
> really no use for it.". So why are you using it ? Why do we have it in
> Buildroot ?

The code is distributed in a tarball along with Quartzville Tools so
anyone who signs the required NDA have access to it. I don't know why
the package exists in Buildroot but I'm glad it's there. I used it as
the base for the package we have in our br2_external to build the
driver and install the binaries provided by Intel. After doing that I
felt it would be fair to make a small contribution in return.

> Romain, since you originally contributed this driver, perhaps you can
> give some details about how you used it ?
Carlos Santos April 3, 2018, 3:29 a.m. UTC | #14
> From: "Thomas Petazzoni" <thomas.petazzoni@bootlin.com>
> To: "Carlos Santos" <casantos@datacom.ind.br>
> Cc: "buildroot" <buildroot@buildroot.org>, "Romain Naour" <romain.naour@gmail.com>
> Sent: Monday, April 2, 2018 6:26:50 AM
> Subject: Re: [Buildroot] [PATCH v2 1/2] iqvlinux: bump to version 1.2.0.3

> Hello,
> 
> On Fri, 15 Dec 2017 13:38:29 -0200, Carlos Santos wrote:
> 
>> +# This package is really annoying since the archive name doesn't contain the
>> +# version. Use a small trick to save it with a better name.
>> +define IQVLINUX_DOWNLOAD
>> +	$(EXTRA_ENV) $(DL_WRAPPER) -b wget \
>> +		-o $(DL_DIR)/$(IQVLINUX_SOURCE) \
>> +		-H $(PKGDIR)/$($(PKG)_RAWNAME).hash \
>> +		-- \
>> +		$(IQVLINUX_SITE)/iqvlinux.tar.gz
>> +endef
>> +
>> +IQVLINUX_PRE_DOWNLOAD_HOOKS = IQVLINUX_DOWNLOAD
> 
> So instead of this, I've added an iqvlinux-1.2.0.3.tar.gz tarball on
> sources.buildroot.net, and just kept iqvlinux download first from
> SourceForge (which fails with a 404), and then fall back to
> sources.buildroot.net.
> 
> Accessing directly the download helper is not a good idea. The download
> helpers are internal, and they are being changed by Maxime/Yann patch
> series introducing Git caching, so we really don't want packages to
> mess up with the download internals.
> 
> It would be good if you could reply on the bug report upstream, explain
> why and how you use iqvlinux, and convince them to fix their tarball
> name.

I posted a reply there. Meanwhile they released version 1.2.0.8 and
named it "iqvlinux.tar.gz". :-(

> I've applied both patches to master, after doing the change explained
> above. Thanks!

Thanks
diff mbox series

Patch

diff --git a/package/iqvlinux/Config.in b/package/iqvlinux/Config.in
index 8959955745..09502d141e 100644
--- a/package/iqvlinux/Config.in
+++ b/package/iqvlinux/Config.in
@@ -12,7 +12,7 @@  config BR2_PACKAGE_IQVLINUX
 	  Note: This driver requires PCI support to be enabled
 	  (i.e. CONFIG_PCI).
 
-	  http://sourceforge.net/projects/e1000/files/iqvlinux/
+	  https://sourceforge.net/projects/e1000/files/iqvlinux/
 
 comment "iqvlinux needs a Linux kernel to be built"
 	depends on !BR2_LINUX_KERNEL
diff --git a/package/iqvlinux/iqvlinux.hash b/package/iqvlinux/iqvlinux.hash
index ddf57b712c..20d612ddc5 100644
--- a/package/iqvlinux/iqvlinux.hash
+++ b/package/iqvlinux/iqvlinux.hash
@@ -1,5 +1,6 @@ 
-# From http://sourceforge.net/projects/e1000/files/iqvlinux/1.1.5.3/
-sha1	bd94416e4364015dbbd78a22e51080bf7ea81fac	iqvlinux.tar.gz
-md5	fb6a2a4dc122d39070fcb06985c97a05	iqvlinux.tar.gz
 # locally computed
-sha256	8cb19f3bfe040100a13bb2d05cb2b54f2b259e55cef23f8cc5aa6f2f31e98bec	iqvlinux.tar.gz
+sha256	4020a661940ba6d50f24654b90a41280eb8eccf00061fe6d105c654d3d97d551  iqvlinux-1.2.0.3.tar.gz
+sha256	386086e232db4708770f717595d4f83188f776aa6e7313cd12fb039e869f494b  COPYING
+sha256	beded926a8f18aad5d911daf581759e2b25697588034d2b29033ae1a929506ff  src/linux/driver/files.txt
+sha256	ecf93b9b739fc82f7b87b34f8c6411dfdd271234af2fe45212293d9df2b654b3  inc/linux/files.txt
+sha256	0a309350c1b32be72479e8fe08f7f6bbbad2e8c3d9a1cbcad3d482ea48a236ad  inc/files.txt
diff --git a/package/iqvlinux/iqvlinux.mk b/package/iqvlinux/iqvlinux.mk
index 165c7c4687..a00f85c0bc 100644
--- a/package/iqvlinux/iqvlinux.mk
+++ b/package/iqvlinux/iqvlinux.mk
@@ -4,9 +4,8 @@ 
 #
 ################################################################################
 
-IQVLINUX_VERSION = 1.1.5.3
-IQVLINUX_SITE = http://sourceforge.net/projects/e1000/files/iqvlinux/$(IQVLINUX_VERSION)
-IQVLINUX_SOURCE = iqvlinux.tar.gz
+IQVLINUX_VERSION = 1.2.0.3
+IQVLINUX_SITE = https://downloads.sourceforge.net/project/e1000/iqvlinux/$(IQVLINUX_VERSION)
 
 IQVLINUX_LICENSE = GPL-2.0, BSD-3-Clause
 IQVLINUX_LICENSE_FILES = \
@@ -17,5 +16,17 @@  IQVLINUX_MODULE_MAKE_OPTS = NALDIR=$(@D) KSRC=$(LINUX_DIR) CC=$(TARGET_CC)
 
 IQVLINUX_MODULE_SUBDIRS = src/linux/driver
 
+# This package is really annoying since the archive name doesn't contain the
+# version. Use a small trick to save it with a better name.
+define IQVLINUX_DOWNLOAD
+	$(EXTRA_ENV) $(DL_WRAPPER) -b wget \
+		-o $(DL_DIR)/$(IQVLINUX_SOURCE) \
+		-H $(PKGDIR)/$($(PKG)_RAWNAME).hash \
+		-- \
+		$(IQVLINUX_SITE)/iqvlinux.tar.gz
+endef
+
+IQVLINUX_PRE_DOWNLOAD_HOOKS = IQVLINUX_DOWNLOAD
+
 $(eval $(kernel-module))
 $(eval $(generic-package))