[bitbake-devel] fetch2/svn: adds svn --ignore-externals option

Submitted by Krzysztof Zawadzki on Feb. 15, 2019, 3:25 a.m. | Patch ID: 158723

Details

Message ID 20190215032539.5297-1-mr.k.zawadzki@gmail.com
State New
Headers show

Commit Message

Krzysztof Zawadzki Feb. 15, 2019, 3:25 a.m.
With this option we can force users to provide all used sources in recipe.
Without this there is possibility that repositories which are using svn:externals
 can point to source code from HEAD or to nonexisting repository.
This can cause attempt to reproduce build after some time impossible.

Signed-off-by: Krzysztof Zawadzki <mr.k.zawadzki@gmail.com>
---
 lib/bb/fetch2/svn.py | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

Patch hide | download patch | download mbox

diff --git a/lib/bb/fetch2/svn.py b/lib/bb/fetch2/svn.py
index 9dcf3eb0..f4a07be3 100644
--- a/lib/bb/fetch2/svn.py
+++ b/lib/bb/fetch2/svn.py
@@ -97,13 +97,20 @@  class Svn(FetchMethod):
 
         if ud.pswd:
             options.append("--password %s" % ud.pswd)
-
+        
         if command == "info":
             svncmd = "%s info %s %s://%s/%s/" % (ud.basecmd, " ".join(options), proto, svnroot, ud.module)
         elif command == "log1":
             svncmd = "%s log --limit 1 %s %s://%s/%s/" % (ud.basecmd, " ".join(options), proto, svnroot, ud.module)
         else:
             suffix = ""
+
+            if "externals" in ud.parm:
+                if ud.parm["externals"] == "ignore":
+                    options.append("--ignore-externals")
+                else:
+                    raise FetchError("Invalid value for 'externals': expected 'ignore'")
+
             if ud.revision:
                 options.append("-r %s" % ud.revision)
                 suffix = "@%s" % (ud.revision)

Comments

Richard Purdie Feb. 15, 2019, 11 p.m.
On Fri, 2019-02-15 at 04:25 +0100, Krzysztof Zawadzki wrote:
> With this option we can force users to provide all used sources in
> recipe.
> Without this there is possibility that repositories which are using
> svn:externals
>  can point to source code from HEAD or to nonexisting repository.
> This can cause attempt to reproduce build after some time impossible.
> 
> Signed-off-by: Krzysztof Zawadzki <mr.k.zawadzki@gmail.com>
> ---
>  lib/bb/fetch2/svn.py | 9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)

I'm wondering if this should be the default and is something people
have to explicitly opt out of?

Cheers,

Richard
Krzysztof Zawadzki Feb. 16, 2019, 12:36 a.m.
Hi,

I think so also. Subversion client is poor in exit statues when there are
issues during svn:externals fetching. I have seen such issues previously.
And it took a while to find there root cause (e.g switching revisions of
svn:externals to non existing paths or revisions and some conflicts showed
up).

@Richard Purdie
It's better to create new patch which will add this option by default, with
additional Docs update? Or update this one?

Best Regards,
Krzysztof Z.

sob., 16 lut 2019 o 00:00 Richard Purdie <richard.purdie@linuxfoundation.org>
napisał(a):

> On Fri, 2019-02-15 at 04:25 +0100, Krzysztof Zawadzki wrote:
> > With this option we can force users to provide all used sources in
> > recipe.
> > Without this there is possibility that repositories which are using
> > svn:externals
> >  can point to source code from HEAD or to nonexisting repository.
> > This can cause attempt to reproduce build after some time impossible.
> >
> > Signed-off-by: Krzysztof Zawadzki <mr.k.zawadzki@gmail.com>
> > ---
> >  lib/bb/fetch2/svn.py | 9 ++++++++-
> >  1 file changed, 8 insertions(+), 1 deletion(-)
>
> I'm wondering if this should be the default and is something people
> have to explicitly opt out of?
>
> Cheers,
>
> Richard
>
>
Mark Hatle Feb. 19, 2019, 5:05 p.m.
I had a customer ask about this type of issue a few days ago as well.  I don't
know enough about SVN to comment directly on what should be default or not, just
that there is some type of an issue here, especially around svn:externals
pointing to bad external repositories and the need to pass --ignore-externals
for certain things.

On 2/16/19 1:36 AM, Krzysztof Zawadzki wrote:
> Hi,
> 
> I think so also. Subversion client is poor in exit statues when there are issues
> during svn:externals fetching. I have seen such issues previously. And it took a
> while to find there root cause (e.g switching revisions of svn:externals to non
> existing paths or revisions and some conflicts showed up).
> 
> @Richard Purdie
> It's better to create new patch which will add this option by default, with
> additional Docs update? Or update this one?

Do you have enough information to help create a reproducer case for this, so we
can add some additional fetcher test cases?

I know I needed to add a bunch of regression tests around the git submodule
fetcher to have any chance of preventing future regressions.

I suspect this may be a similar situation, we need more test cases.

--Mark

> Best Regards,
> Krzysztof Z.
> 
> sob., 16 lut 2019 o 00:00 Richard Purdie <richard.purdie@linuxfoundation.org
> <mailto:richard.purdie@linuxfoundation.org>> napisał(a):
> 
>     On Fri, 2019-02-15 at 04:25 +0100, Krzysztof Zawadzki wrote:
>     > With this option we can force users to provide all used sources in
>     > recipe.
>     > Without this there is possibility that repositories which are using
>     > svn:externals
>     >  can point to source code from HEAD or to nonexisting repository.
>     > This can cause attempt to reproduce build after some time impossible.
>     >
>     > Signed-off-by: Krzysztof Zawadzki <mr.k.zawadzki@gmail.com
>     <mailto:mr.k.zawadzki@gmail.com>>
>     > ---
>     >  lib/bb/fetch2/svn.py | 9 ++++++++-
>     >  1 file changed, 8 insertions(+), 1 deletion(-)
> 
>     I'm wondering if this should be the default and is something people
>     have to explicitly opt out of?
> 
>     Cheers,
> 
>     Richard
> 
>
Mark Hatle March 7, 2019, 5:26 p.m.
Ping, anything further happen with this?  (Accepted/rejected/etc)?

I didn't see any discussion after Feb 19th, but I'm still trying to resolve a
customer's issue that appears to be directly related to this.. but I don't have
enough SVN knowledge to say if this is good or bad.

--Mark

On 2/15/19 5:00 PM, Richard Purdie wrote:
> On Fri, 2019-02-15 at 04:25 +0100, Krzysztof Zawadzki wrote:
>> With this option we can force users to provide all used sources in
>> recipe.
>> Without this there is possibility that repositories which are using
>> svn:externals
>>  can point to source code from HEAD or to nonexisting repository.
>> This can cause attempt to reproduce build after some time impossible.
>>
>> Signed-off-by: Krzysztof Zawadzki <mr.k.zawadzki@gmail.com>
>> ---
>>  lib/bb/fetch2/svn.py | 9 ++++++++-
>>  1 file changed, 8 insertions(+), 1 deletion(-)
> 
> I'm wondering if this should be the default and is something people
> have to explicitly opt out of?
> 
> Cheers,
> 
> Richard
>
Richard Purdie March 7, 2019, 5:44 p.m.
On Thu, 2019-03-07 at 11:26 -0600, Mark Hatle wrote:
> Ping, anything further happen with this?  (Accepted/rejected/etc)?
> 
> I didn't see any discussion after Feb 19th, but I'm still trying to
> resolve a
> customer's issue that appears to be directly related to this.. but I
> don't have
> enough SVN knowledge to say if this is good or bad.

I'd take a patch to explicitly disable externals so we're
deterministic...

Cheers,

Richard