[PATCH] bitbake: fetch2: Make SRCREV_FORMAT name substitution safer

[PATCH] bitbake: fetch2: Make SRCREV_FORMAT name substitution safer

[Ulf Magnusson]

Given two names "foo" and "foobar" and SRCREV_FORMAT = "foo_foobar",
"foo" might currently get substituted twice. Work around the issue by
sorting the list of names by length and substituting longer names first.

Signed-off-by: Ulf Magnusson <ulfalizer@gmail.com>
---
 bitbake/lib/bb/fetch2/__init__.py | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/bitbake/lib/bb/fetch2/__init__.py b/bitbake/lib/bb/fetch2/__init__.py
index 11c75cc..4ca024e 100644
--- a/bitbake/lib/bb/fetch2/__init__.py
+++ b/bitbake/lib/bb/fetch2/__init__.py
@@ -764,7 +764,10 @@ def get_srcrev(d, method_name='sortable_revision'):
     seenautoinc = False
     for scm in scms:
         ud = urldata[scm]
-        for name in ud.names:
+        # Sort the list of names and replace the longest names first. Given two
+        # names "foo" and "foobar", this prevents "foo" being substituted twice
+        # in "foo_foobar" (and "foobar" from not being substituted at all).
+        for name in sorted(ud.names, key=len, reverse=True):
             autoinc, rev = getattr(ud.method, method_name)(ud, d, name)
             seenautoinc = seenautoinc or autoinc
             if len(rev) > 10:
-- 
2.5.0

[PATCH] bitbake: fetch2: Make SRCREV_FORMAT name substitution safer

[Ulf Magnusson]

Given two names "foo" and "foobar" and SRCREV_FORMAT = "foo_foobar",
"foo" might currently get substituted twice. Work around the issue by
sorting the list of names by length and substituting longer names first.

Signed-off-by: Ulf Magnusson <ulfalizer@gmail.com>
---
 bitbake/lib/bb/fetch2/__init__.py | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/bitbake/lib/bb/fetch2/__init__.py b/bitbake/lib/bb/fetch2/__init__.py
index 11c75cc..4ca024e 100644
--- a/bitbake/lib/bb/fetch2/__init__.py
+++ b/bitbake/lib/bb/fetch2/__init__.py
@@ -764,7 +764,10 @@ def get_srcrev(d, method_name='sortable_revision'):
     seenautoinc = False
     for scm in scms:
         ud = urldata[scm]
-        for name in ud.names:
+        # Sort the list of names and replace the longest names first. Given two
+        # names "foo" and "foobar", this prevents "foo" being substituted twice
+        # in "foo_foobar" (and "foobar" from not being substituted at all).
+        for name in sorted(ud.names, key=len, reverse=True):
             autoinc, rev = getattr(ud.method, method_name)(ud, d, name)
             seenautoinc = seenautoinc or autoinc
             if len(rev) > 10:
-- 
2.5.0

Re: [PATCH] bitbake: fetch2: Make SRCREV_FORMAT name substitution safer

[Ulf Magnusson]

On Mon, Sep 12, 2016 at 10:11 PM, Ulf Magnusson <ulfalizer@gmail.com> wrote:
> Given two names "foo" and "foobar" and SRCREV_FORMAT = "foo_foobar",
> "foo" might currently get substituted twice. Work around the issue by
> sorting the list of names by length and substituting longer names first.
>
> Signed-off-by: Ulf Magnusson <ulfalizer@gmail.com>
> ---
>  bitbake/lib/bb/fetch2/__init__.py | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/bitbake/lib/bb/fetch2/__init__.py b/bitbake/lib/bb/fetch2/__init__.py
> index 11c75cc..4ca024e 100644
> --- a/bitbake/lib/bb/fetch2/__init__.py
> +++ b/bitbake/lib/bb/fetch2/__init__.py
> @@ -764,7 +764,10 @@ def get_srcrev(d, method_name='sortable_revision'):
>      seenautoinc = False
>      for scm in scms:
>          ud = urldata[scm]
> -        for name in ud.names:
> +        # Sort the list of names and replace the longest names first. Given two
> +        # names "foo" and "foobar", this prevents "foo" being substituted twice
> +        # in "foo_foobar" (and "foobar" from not being substituted at all).
> +        for name in sorted(ud.names, key=len, reverse=True):
>              autoinc, rev = getattr(ud.method, method_name)(ud, d, name)
>              seenautoinc = seenautoinc or autoinc
>              if len(rev) > 10:
> --
> 2.5.0
>

Hrm... the bitbake-devel mailing list would be a better fit. Sent it there too.

Cheers,
Ulf

Re: [PATCH] bitbake: fetch2: Make SRCREV_FORMAT name substitution safer

[Ulf Magnusson]

On Mon, Sep 12, 2016 at 10:16 PM, Ulf Magnusson <ulfalizer@gmail.com> wrote:
> Given two names "foo" and "foobar" and SRCREV_FORMAT = "foo_foobar",
> "foo" might currently get substituted twice. Work around the issue by
> sorting the list of names by length and substituting longer names first.
>
> Signed-off-by: Ulf Magnusson <ulfalizer@gmail.com>
> ---
>  bitbake/lib/bb/fetch2/__init__.py | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/bitbake/lib/bb/fetch2/__init__.py b/bitbake/lib/bb/fetch2/__init__.py
> index 11c75cc..4ca024e 100644
> --- a/bitbake/lib/bb/fetch2/__init__.py
> +++ b/bitbake/lib/bb/fetch2/__init__.py
> @@ -764,7 +764,10 @@ def get_srcrev(d, method_name='sortable_revision'):
>      seenautoinc = False
>      for scm in scms:
>          ud = urldata[scm]
> -        for name in ud.names:
> +        # Sort the list of names and replace the longest names first. Given two
> +        # names "foo" and "foobar", this prevents "foo" being substituted twice
> +        # in "foo_foobar" (and "foobar" from not being substituted at all).
> +        for name in sorted(ud.names, key=len, reverse=True):
>              autoinc, rev = getattr(ud.method, method_name)(ud, d, name)
>              seenautoinc = seenautoinc or autoinc
>              if len(rev) > 10:
> --
> 2.5.0
>

A syntax like the following would have been safer, but it might be too
late for that now.

  SRCREV_FORMAT = "{foo}_{foobar}"

Cheers,
Ulf

Re: [PATCH] bitbake: fetch2: Make SRCREV_FORMAT name substitution safer

[Ulf Magnusson]

On Mon, Sep 12, 2016 at 10:20 PM, Ulf Magnusson <ulfalizer@gmail.com> wrote:
> On Mon, Sep 12, 2016 at 10:16 PM, Ulf Magnusson <ulfalizer@gmail.com> wrote:
>> Given two names "foo" and "foobar" and SRCREV_FORMAT = "foo_foobar",
>> "foo" might currently get substituted twice. Work around the issue by
>> sorting the list of names by length and substituting longer names first.
>>
>> Signed-off-by: Ulf Magnusson <ulfalizer@gmail.com>
>> ---
>>  bitbake/lib/bb/fetch2/__init__.py | 5 ++++-
>>  1 file changed, 4 insertions(+), 1 deletion(-)
>>
>> diff --git a/bitbake/lib/bb/fetch2/__init__.py b/bitbake/lib/bb/fetch2/__init__.py
>> index 11c75cc..4ca024e 100644
>> --- a/bitbake/lib/bb/fetch2/__init__.py
>> +++ b/bitbake/lib/bb/fetch2/__init__.py
>> @@ -764,7 +764,10 @@ def get_srcrev(d, method_name='sortable_revision'):
>>      seenautoinc = False
>>      for scm in scms:
>>          ud = urldata[scm]
>> -        for name in ud.names:
>> +        # Sort the list of names and replace the longest names first. Given two
>> +        # names "foo" and "foobar", this prevents "foo" being substituted twice
>> +        # in "foo_foobar" (and "foobar" from not being substituted at all).
>> +        for name in sorted(ud.names, key=len, reverse=True):
>>              autoinc, rev = getattr(ud.method, method_name)(ud, d, name)
>>              seenautoinc = seenautoinc or autoinc
>>              if len(rev) > 10:
>> --
>> 2.5.0
>>
>
> A syntax like the following would have been safer, but it might be too
> late for that now.
>
>   SRCREV_FORMAT = "{foo}_{foobar}"
>
> Cheers,
> Ulf

Seems I misread the code. This only works for the case where the same
repository URI is used multiple times in SRC_URI, with a different
name each time. That is probably a relatively rare case.

The basic idea should still work. Suggestions welcome.

Cheers,
Ulf