[Buildroot] [PATCH 1/1] pkg-generic: fix no-check-hash for inferred site method

[Buildroot] [PATCH 1/1] pkg-generic: fix no-check-hash for inferred site method

[Ricardo Martincoski]

Currently, when the site method is explicitly set to a SCM other than
git, the main download is correctly excluded from being hash-checked.
But when the site method is inferred from the site uri, the download
from a SCM other than git is wrongly being hash-checked.

Fix this by moving the code that excludes SCM methods from hash-check
below the code that infers the site method.

Signed-off-by: Ricardo Martincoski <ricardo.martincoski@gmail.com>
Cc: Luca Ceresoli <luca@lucaceresoli.net>
Cc: Maxime Hadjinlian <maxime.hadjinlian@gmail.com>
Cc: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
Cc: Yann E. MORIN <yann.morin.1998@free.fr>
---
All packages in the tree that use those site methods currently
explicitly set the site method.
$ make -s printvars VARS=%_SITE_METHOD | grep '=\(bzr\|cvs\|hg\|svn\)'
CANFESTIVAL_SITE_METHOD=hg
DVB_APPS_SITE_METHOD=hg
FIS_SITE_METHOD=svn
OPEN2300_SITE_METHOD=svn
OPENTYRIAN_SITE_METHOD=hg
PYTHON_NFC_SITE_METHOD=bzr
PYTHON_PYGAME_SITE_METHOD=hg
---
 package/pkg-generic.mk | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
index 501dd3de39..91b61c6de0 100644
--- a/package/pkg-generic.mk
+++ b/package/pkg-generic.mk
@@ -489,10 +489,6 @@ ifndef $(2)_PATCH
  endif
 endif
 
-ifneq ($$(filter bzr cvs hg svn,$$($(2)_SITE_METHOD)),)
-BR_NO_CHECK_HASH_FOR += $$($(2)_SOURCE)
-endif
-
 $(2)_ALL_DOWNLOADS = \
 	$$(if $$($(2)_SOURCE),$$($(2)_SITE_METHOD)+$$($(2)_SITE)/$$($(2)_SOURCE)) \
 	$$(foreach p,$$($(2)_PATCH) $$($(2)_EXTRA_DOWNLOADS),\
@@ -514,6 +510,10 @@ ifndef $(2)_SITE_METHOD
  endif
 endif
 
+ifneq ($$(filter bzr cvs hg svn,$$($(2)_SITE_METHOD)),)
+BR_NO_CHECK_HASH_FOR += $$($(2)_SOURCE)
+endif
+
 # Do not accept to download git submodule if not using the git method
 ifneq ($$($(2)_GIT_SUBMODULES),)
  ifneq ($$($(2)_SITE_METHOD),git)
-- 
2.17.1

[Buildroot] [PATCH 1/1] pkg-generic: fix no-check-hash for inferred site method

[Luca Ceresoli]

Hi Ricardo,

On 25/06/2018 03:41, Ricardo Martincoski wrote:
> Currently, when the site method is explicitly set to a SCM other than
> git, the main download is correctly excluded from being hash-checked.
> But when the site method is inferred from the site uri, the download
> from a SCM other than git is wrongly being hash-checked.
> 
> Fix this by moving the code that excludes SCM methods from hash-check
> below the code that infers the site method.
> 
> Signed-off-by: Ricardo Martincoski <ricardo.martincoski@gmail.com>
> Cc: Luca Ceresoli <luca@lucaceresoli.net>
> Cc: Maxime Hadjinlian <maxime.hadjinlian@gmail.com>
> Cc: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
> Cc: Yann E. MORIN <yann.morin.1998@free.fr>
> ---
> All packages in the tree that use those site methods currently
> explicitly set the site method.
> $ make -s printvars VARS=%_SITE_METHOD | grep '=\(bzr\|cvs\|hg\|svn\)'
> CANFESTIVAL_SITE_METHOD=hg
> DVB_APPS_SITE_METHOD=hg
> FIS_SITE_METHOD=svn
> OPEN2300_SITE_METHOD=svn
> OPENTYRIAN_SITE_METHOD=hg
> PYTHON_NFC_SITE_METHOD=bzr
> PYTHON_PYGAME_SITE_METHOD=hg

Apologies, but I couldn't understand what this commit is supposed to
fix. From your description above I understand you're fixing packages
whose method is inferred (i.e. not explicitly set). But here you list
packages which explicitly set it.

Which one are you fixing exactly? Can you provide an example package
that your patch fixes?

BTW I couldn't find in Buildroot any package whose method is an SCM
other than git and is not explicitly set.

Thanks,
-- 
Luca

[Buildroot] [PATCH 1/1] pkg-generic: fix no-check-hash for inferred site method

[Ricardo Martincoski]

Hello,

On Mon, Jun 25, 2018 at 06:01 PM, Luca Ceresoli wrote:

> On 25/06/2018 03:41, Ricardo Martincoski wrote:
>> Currently, when the site method is explicitly set to a SCM other than
>> git, the main download is correctly excluded from being hash-checked.
>> But when the site method is inferred from the site uri, the download
>> from a SCM other than git is wrongly being hash-checked.
[snip]
>> ---
>> All packages in the tree that use those site methods currently
>> explicitly set the site method.
>> $ make -s printvars VARS=%_SITE_METHOD | grep '=\(bzr\|cvs\|hg\|svn\)'
[snip]
>> PYTHON_PYGAME_SITE_METHOD=hg
> 
> Apologies, but I couldn't understand what this commit is supposed to
> fix. From your description above I understand you're fixing packages
> whose method is inferred (i.e. not explicitly set). But here you list

I am fixing packages whose method is inferred.

> packages which explicitly set it.

Sorry. My message after the commit log was misleading.
Rethinking now, a better message would be:
"Currently there is no package in the tree that uses inferred site method, and
that is why the autobuilder didn't caught this. We had packages using inferred
site method in the past, the last one was 'expect', but since they didn't have a
hash file (for a license or extra-download for example) the build didn't error
out."

> 
> Which one are you fixing exactly? Can you provide an example package
> that your patch fixes?

There is no example in the tree right now, but they can either live in a
br2-external somewhere or they can be added to the tree in the future.
I don't have a real package with this problem.

I stumbled on this because I am creating automated tests for the download infra.
They setup a SCM remote in the docker image by calling git daemon, svnserver,
... depending on the download method and download a package. Here is one package
under test [1], the test failing before this patch [2] and the test passing
after this patch [3].

> 
> BTW I couldn't find in Buildroot any package whose method is an SCM
> other than git and is not explicitly set.

Correct. The last one was 'expect', it now uses a tarball instead of cvs.

You can reproduce the problem I a trying to fix by doing this:
$ git revert 23ab6cb162b7456925e2c3df4917cf9b45868d55
$ touch package/expect/expect.hash
$ make expect-source
ERROR: No hash found for expect-2014-05-02.tar.gz


[1] https://gitlab.com/RicardoMartincoski/buildroot/blob/910ad20b78db2b5f238daca18c5688e3ec3c157b/support/testing/tests/download/br2-external/svn-refs/package/svn-revision-number/svn-revision-number.mk
[2] https://gitlab.com/RicardoMartincoski/buildroot/-/jobs/77355189
[3] https://gitlab.com/RicardoMartincoski/buildroot/-/jobs/77355202


Regards,
Ricardo

[Buildroot] [PATCH 1/1] pkg-generic: fix no-check-hash for inferred site method

[Luca Ceresoli]

Hi Ricardo,

On 26/06/2018 04:01, Ricardo Martincoski wrote:
> Hello,
> 
> On Mon, Jun 25, 2018 at 06:01 PM, Luca Ceresoli wrote:
> 
>> On 25/06/2018 03:41, Ricardo Martincoski wrote:
>>> Currently, when the site method is explicitly set to a SCM other than
>>> git, the main download is correctly excluded from being hash-checked.
>>> But when the site method is inferred from the site uri, the download
>>> from a SCM other than git is wrongly being hash-checked.
> [snip]
>>> ---
>>> All packages in the tree that use those site methods currently
>>> explicitly set the site method.
>>> $ make -s printvars VARS=%_SITE_METHOD | grep '=\(bzr\|cvs\|hg\|svn\)'
> [snip]
>>> PYTHON_PYGAME_SITE_METHOD=hg
>>
>> Apologies, but I couldn't understand what this commit is supposed to
>> fix. From your description above I understand you're fixing packages
>> whose method is inferred (i.e. not explicitly set). But here you list
> 
> I am fixing packages whose method is inferred.
> 
>> packages which explicitly set it.
> 
> Sorry. My message after the commit log was misleading.
> Rethinking now, a better message would be:
> "Currently there is no package in the tree that uses inferred site method, and
> that is why the autobuilder didn't caught this. We had packages using inferred
> site method in the past, the last one was 'expect', but since they didn't have a
> hash file (for a license or extra-download for example) the build didn't error
> out."

Ok, thanks for the detailed explanation. I would append the above
paragraph to the existing message, just before the SoB line. If you
resubmit with this change, you can add my:

 Reviewed-by: Luca Ceresoli <luca@lucaceresoli.net>
 Tested-by: Luca Ceresoli <luca@lucaceresoli.net>

Bye,
-- 
Luca

[Buildroot] [PATCH v2 1/1] pkg-generic: fix no-check-hash for inferred site method

[Ricardo Martincoski]

Currently, when the site method is explicitly set to a SCM other than
git, the main download is correctly excluded from being hash-checked.
But when the site method is inferred from the site uri, the download
from a SCM other than git is wrongly being hash-checked.

Fix this by moving the code that excludes SCM methods from hash-check
below the code that infers the site method.

Currently there is no package in the tree that uses inferred site
method, and that is why the autobuilder didn't caught this. We had
packages using inferred site method in the past, the last one was
'expect', but since they didn't have a hash file (for a license or
extra-download for example) the build didn't error out.

Signed-off-by: Ricardo Martincoski <ricardo.martincoski@gmail.com>
Cc: Luca Ceresoli <luca@lucaceresoli.net>
Cc: Maxime Hadjinlian <maxime.hadjinlian@gmail.com>
Cc: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
Cc: Yann E. MORIN <yann.morin.1998@free.fr>
Reviewed-by: Luca Ceresoli <luca@lucaceresoli.net>
Tested-by: Luca Ceresoli <luca@lucaceresoli.net>
---
Changes v1 -> v2:
  - improve commit message (Luca Ceresoli);
  - collect Reviewed-by & Tested-by tags.
---
 package/pkg-generic.mk | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
index 501dd3de39..91b61c6de0 100644
--- a/package/pkg-generic.mk
+++ b/package/pkg-generic.mk
@@ -489,10 +489,6 @@ ifndef $(2)_PATCH
  endif
 endif
 
-ifneq ($$(filter bzr cvs hg svn,$$($(2)_SITE_METHOD)),)
-BR_NO_CHECK_HASH_FOR += $$($(2)_SOURCE)
-endif
-
 $(2)_ALL_DOWNLOADS = \
 	$$(if $$($(2)_SOURCE),$$($(2)_SITE_METHOD)+$$($(2)_SITE)/$$($(2)_SOURCE)) \
 	$$(foreach p,$$($(2)_PATCH) $$($(2)_EXTRA_DOWNLOADS),\
@@ -514,6 +510,10 @@ ifndef $(2)_SITE_METHOD
  endif
 endif
 
+ifneq ($$(filter bzr cvs hg svn,$$($(2)_SITE_METHOD)),)
+BR_NO_CHECK_HASH_FOR += $$($(2)_SOURCE)
+endif
+
 # Do not accept to download git submodule if not using the git method
 ifneq ($$($(2)_GIT_SUBMODULES),)
  ifneq ($$($(2)_SITE_METHOD),git)
-- 
2.17.1

[Buildroot] [PATCH v2 1/1] pkg-generic: fix no-check-hash for inferred site method

[Yann E. MORIN]

Ricardo, All,

On 2018-06-28 22:42 -0300, Ricardo Martincoski spake thusly:
> Currently, when the site method is explicitly set to a SCM other than
> git, the main download is correctly excluded from being hash-checked.
> But when the site method is inferred from the site uri, the download
> from a SCM other than git is wrongly being hash-checked.
> 
> Fix this by moving the code that excludes SCM methods from hash-check
> below the code that infers the site method.
> 
> Currently there is no package in the tree that uses inferred site
> method, and that is why the autobuilder didn't caught this. We had
> packages using inferred site method in the past, the last one was
> 'expect', but since they didn't have a hash file (for a license or
> extra-download for example) the build didn't error out.
> 
> Signed-off-by: Ricardo Martincoski <ricardo.martincoski@gmail.com>
> Cc: Luca Ceresoli <luca@lucaceresoli.net>
> Cc: Maxime Hadjinlian <maxime.hadjinlian@gmail.com>
> Cc: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
> Cc: Yann E. MORIN <yann.morin.1998@free.fr>
> Reviewed-by: Luca Ceresoli <luca@lucaceresoli.net>
> Tested-by: Luca Ceresoli <luca@lucaceresoli.net>

Reviewed-by: "Yann E. MORIN" <yann.morin.1998@free.fr>

Regards,
Yann E. MORIN.

> ---
> Changes v1 -> v2:
>   - improve commit message (Luca Ceresoli);
>   - collect Reviewed-by & Tested-by tags.
> ---
>  package/pkg-generic.mk | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
> index 501dd3de39..91b61c6de0 100644
> --- a/package/pkg-generic.mk
> +++ b/package/pkg-generic.mk
> @@ -489,10 +489,6 @@ ifndef $(2)_PATCH
>   endif
>  endif
>  
> -ifneq ($$(filter bzr cvs hg svn,$$($(2)_SITE_METHOD)),)
> -BR_NO_CHECK_HASH_FOR += $$($(2)_SOURCE)
> -endif
> -
>  $(2)_ALL_DOWNLOADS = \
>  	$$(if $$($(2)_SOURCE),$$($(2)_SITE_METHOD)+$$($(2)_SITE)/$$($(2)_SOURCE)) \
>  	$$(foreach p,$$($(2)_PATCH) $$($(2)_EXTRA_DOWNLOADS),\
> @@ -514,6 +510,10 @@ ifndef $(2)_SITE_METHOD
>   endif
>  endif
>  
> +ifneq ($$(filter bzr cvs hg svn,$$($(2)_SITE_METHOD)),)
> +BR_NO_CHECK_HASH_FOR += $$($(2)_SOURCE)
> +endif
> +
>  # Do not accept to download git submodule if not using the git method
>  ifneq ($$($(2)_GIT_SUBMODULES),)
>   ifneq ($$($(2)_SITE_METHOD),git)
> -- 
> 2.17.1
> 

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 223 225 172 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'

[Buildroot] [PATCH v2 1/1] pkg-generic: fix no-check-hash for inferred site method

[Thomas Petazzoni]

Hello,

On Thu, 28 Jun 2018 22:42:46 -0300, Ricardo Martincoski wrote:
> Currently, when the site method is explicitly set to a SCM other than
> git, the main download is correctly excluded from being hash-checked.
> But when the site method is inferred from the site uri, the download
> from a SCM other than git is wrongly being hash-checked.
> 
> Fix this by moving the code that excludes SCM methods from hash-check
> below the code that infers the site method.
> 
> Currently there is no package in the tree that uses inferred site
> method, and that is why the autobuilder didn't caught this. We had
> packages using inferred site method in the past, the last one was
> 'expect', but since they didn't have a hash file (for a license or
> extra-download for example) the build didn't error out.
> 
> Signed-off-by: Ricardo Martincoski <ricardo.martincoski@gmail.com>
> Cc: Luca Ceresoli <luca@lucaceresoli.net>
> Cc: Maxime Hadjinlian <maxime.hadjinlian@gmail.com>
> Cc: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
> Cc: Yann E. MORIN <yann.morin.1998@free.fr>
> Reviewed-by: Luca Ceresoli <luca@lucaceresoli.net>
> Tested-by: Luca Ceresoli <luca@lucaceresoli.net>
> ---
> Changes v1 -> v2:
>   - improve commit message (Luca Ceresoli);
>   - collect Reviewed-by & Tested-by tags.
> ---
>  package/pkg-generic.mk | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)

Applied to master, thanks.

Thomas
-- 
Thomas Petazzoni, CTO, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com

[Buildroot] [PATCH v2 1/1] pkg-generic: fix no-check-hash for inferred site method

[Peter Korsgaard]

>>>>> "Ricardo" == Ricardo Martincoski <ricardo.martincoski@gmail.com> writes:

 > Currently, when the site method is explicitly set to a SCM other than
 > git, the main download is correctly excluded from being hash-checked.
 > But when the site method is inferred from the site uri, the download
 > from a SCM other than git is wrongly being hash-checked.

 > Fix this by moving the code that excludes SCM methods from hash-check
 > below the code that infers the site method.

 > Currently there is no package in the tree that uses inferred site
 > method, and that is why the autobuilder didn't caught this. We had
 > packages using inferred site method in the past, the last one was
 > 'expect', but since they didn't have a hash file (for a license or
 > extra-download for example) the build didn't error out.

 > Signed-off-by: Ricardo Martincoski <ricardo.martincoski@gmail.com>
 > Cc: Luca Ceresoli <luca@lucaceresoli.net>
 > Cc: Maxime Hadjinlian <maxime.hadjinlian@gmail.com>
 > Cc: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
 > Cc: Yann E. MORIN <yann.morin.1998@free.fr>
 > Reviewed-by: Luca Ceresoli <luca@lucaceresoli.net>
 > Tested-by: Luca Ceresoli <luca@lucaceresoli.net>
 > ---
 > Changes v1 -> v2:
 >   - improve commit message (Luca Ceresoli);
 >   - collect Reviewed-by & Tested-by tags.
 > ---
 >  package/pkg-generic.mk | 8 ++++----
 >  1 file changed, 4 insertions(+), 4 deletions(-)

Committed to 2018.05.x, thanks.

-- 
Bye, Peter Korsgaard