All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] fetch/git: Change to use clearer ssh url syntax for broken servers
@ 2016-01-07 13:18 Richard Purdie
  2016-01-13 21:37 ` Andre McCurdy
  0 siblings, 1 reply; 12+ messages in thread
From: Richard Purdie @ 2016-01-07 13:18 UTC (permalink / raw)
  To: bitbake-devel

Some servers, e.g. bitbucket.org can't cope with ssh:// as part of
the git url syntax. git itself is happy enough with this but you
get server side errors when using it.

This changes the git fetcher to use the more common ssh url format
which also means we need a : before the path.

Seems a shame to have to do this due to broken servers however
it should be safe enough since this other form is the one most people
use on the commandline so it should be safe enough.

[YOCTO #8864]

Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>

diff --git a/bitbake/lib/bb/fetch2/git.py b/bitbake/lib/bb/fetch2/git.py
index 5ffab22..10ba1d3 100644
--- a/bitbake/lib/bb/fetch2/git.py
+++ b/bitbake/lib/bb/fetch2/git.py
@@ -330,6 +330,10 @@ class Git(FetchMethod):
             username = ud.user + '@'
         else:
             username = ""
+        if ud.proto == "ssh":
+            # Some servers, e.g. bitbucket.org can't cope with ssh://
+            # and removing that means we need a : before path.
+            return "%s%s:%s" % (username, ud.host, ud.path)
         return "%s://%s%s%s" % (ud.proto, username, ud.host, ud.path)
 
     def _revision_key(self, ud, d, name):




^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH] fetch/git: Change to use clearer ssh url syntax for broken servers
  2016-01-07 13:18 [PATCH] fetch/git: Change to use clearer ssh url syntax for broken servers Richard Purdie
@ 2016-01-13 21:37 ` Andre McCurdy
  2016-01-13 22:19   ` Richard Purdie
  0 siblings, 1 reply; 12+ messages in thread
From: Andre McCurdy @ 2016-01-13 21:37 UTC (permalink / raw)
  To: Richard Purdie; +Cc: bitbake-devel

On Thu, Jan 7, 2016 at 5:18 AM, Richard Purdie
<richard.purdie@linuxfoundation.org> wrote:
> Some servers, e.g. bitbucket.org can't cope with ssh:// as part of
> the git url syntax. git itself is happy enough with this but you
> get server side errors when using it.
>
> This changes the git fetcher to use the more common ssh url format
> which also means we need a : before the path.

This seems to break SRC_URIs using ssh to access self hosted git
servers setup following the process described here:

  https://git-scm.com/book/en/v2/Git-on-the-Server-Setting-Up-the-Server

An example of such a SRC_URI, which now fails to work:

  SRC_URI = "git://git@mylocalserver.com:/opt/git/myrepo.git;protocol=ssh"

----

Looking at the original #8864 bug report, the problem seems to be that
the SRC_URI used to access BitBucket contains a ':' where it should
contain a '/'. ie

  SRC_URI = "git://git@bitbucket.org:<username>/<repository>.git;protocol=ssh"

should be:

  SRC_URI = "git://git@bitbucket.org/<username>/<repository>.git;protocol=ssh"

which is a format which works fine for both BitBucket and GitHub.

(Unfortunately the BitBucket website does provide copy and paste repo
URLs containing the ':' which work OK from the command line but need
to have the ':' manually replaced with a '/' for use in a SRC_URI).


> Seems a shame to have to do this due to broken servers however
> it should be safe enough since this other form is the one most people
> use on the commandline so it should be safe enough.
>
> [YOCTO #8864]
>
> Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>
>
> diff --git a/bitbake/lib/bb/fetch2/git.py b/bitbake/lib/bb/fetch2/git.py
> index 5ffab22..10ba1d3 100644
> --- a/bitbake/lib/bb/fetch2/git.py
> +++ b/bitbake/lib/bb/fetch2/git.py
> @@ -330,6 +330,10 @@ class Git(FetchMethod):
>              username = ud.user + '@'
>          else:
>              username = ""
> +        if ud.proto == "ssh":
> +            # Some servers, e.g. bitbucket.org can't cope with ssh://
> +            # and removing that means we need a : before path.
> +            return "%s%s:%s" % (username, ud.host, ud.path)
>          return "%s://%s%s%s" % (ud.proto, username, ud.host, ud.path)
>
>      def _revision_key(self, ud, d, name):
>
>
> --
> _______________________________________________
> bitbake-devel mailing list
> bitbake-devel@lists.openembedded.org
> http://lists.openembedded.org/mailman/listinfo/bitbake-devel


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] fetch/git: Change to use clearer ssh url syntax for broken servers
  2016-01-13 21:37 ` Andre McCurdy
@ 2016-01-13 22:19   ` Richard Purdie
  2016-01-13 23:20     ` Andre McCurdy
  2016-01-14 14:47     ` Peter Kjellerstedt
  0 siblings, 2 replies; 12+ messages in thread
From: Richard Purdie @ 2016-01-13 22:19 UTC (permalink / raw)
  To: Andre McCurdy; +Cc: bitbake-devel

On Wed, 2016-01-13 at 13:37 -0800, Andre McCurdy wrote:
> On Thu, Jan 7, 2016 at 5:18 AM, Richard Purdie
> <richard.purdie@linuxfoundation.org> wrote:
> > Some servers, e.g. bitbucket.org can't cope with ssh:// as part of
> > the git url syntax. git itself is happy enough with this but you
> > get server side errors when using it.
> > 
> > This changes the git fetcher to use the more common ssh url format
> > which also means we need a : before the path.
> 
> This seems to break SRC_URIs using ssh to access self hosted git
> servers setup following the process described here:
> 
>   https://git-scm.com/book/en/v2/Git-on-the-Server-Setting-Up-the-Ser
> ver
> 
> An example of such a SRC_URI, which now fails to work:
> 
>   SRC_URI = "git
> ://git@mylocalserver.com:/opt/git/myrepo.git;protocol=ssh"

Does:

SRC_URI = "git://git@mylocalserver.com/opt/git/myrepo.git;protocol=ssh"

work though?

I'm not sure we ever suggested people should use ":/" in SRC_URI since
that isn't a url format the system was ever designed to cope with. It
might have happened to work but that is different...

Cheers,

Richard


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] fetch/git: Change to use clearer ssh url syntax for broken servers
  2016-01-13 22:19   ` Richard Purdie
@ 2016-01-13 23:20     ` Andre McCurdy
  2016-01-13 23:59       ` Andre McCurdy
  2016-01-14 14:47     ` Peter Kjellerstedt
  1 sibling, 1 reply; 12+ messages in thread
From: Andre McCurdy @ 2016-01-13 23:20 UTC (permalink / raw)
  To: Richard Purdie; +Cc: bitbake-devel

On Wed, Jan 13, 2016 at 2:19 PM, Richard Purdie
<richard.purdie@linuxfoundation.org> wrote:
> On Wed, 2016-01-13 at 13:37 -0800, Andre McCurdy wrote:
>> On Thu, Jan 7, 2016 at 5:18 AM, Richard Purdie
>> <richard.purdie@linuxfoundation.org> wrote:
>> > Some servers, e.g. bitbucket.org can't cope with ssh:// as part of
>> > the git url syntax. git itself is happy enough with this but you
>> > get server side errors when using it.
>> >
>> > This changes the git fetcher to use the more common ssh url format
>> > which also means we need a : before the path.
>>
>> This seems to break SRC_URIs using ssh to access self hosted git
>> servers setup following the process described here:
>>
>>   https://git-scm.com/book/en/v2/Git-on-the-Server-Setting-Up-the-Ser
>> ver
>>
>> An example of such a SRC_URI, which now fails to work:
>>
>>   SRC_URI = "git
>> ://git@mylocalserver.com:/opt/git/myrepo.git;protocol=ssh"
>
> Does:
>
> SRC_URI = "git://git@mylocalserver.com/opt/git/myrepo.git;protocol=ssh"
>
> work though?

Removing the ':' breaks things from the command line, but it does work
OK in a SRC_URI (previously because of the ssh:// prefix and now
because the fetcher is effectively adding the ':' back again).

Updating my SRC_URIs looks like the best solution.

> I'm not sure we ever suggested people should use ":/" in SRC_URI since
> that isn't a url format the system was ever designed to cope with. It
> might have happened to work but that is different...

":/" comes from the examples in the git-scm.com documentation:

  https://git-scm.com/book/en/v2/Git-on-the-Server-Setting-Up-the-Server

It would be nice if those examples (continued to) work in SRC_URIs and
it's also nice if git URLs from SRC_URIs can be tested with git on the
command line. Maybe those goals aren't worth the extra complexity
though...


> Cheers,
>
> Richard


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] fetch/git: Change to use clearer ssh url syntax for broken servers
  2016-01-13 23:20     ` Andre McCurdy
@ 2016-01-13 23:59       ` Andre McCurdy
  0 siblings, 0 replies; 12+ messages in thread
From: Andre McCurdy @ 2016-01-13 23:59 UTC (permalink / raw)
  To: Richard Purdie; +Cc: bitbake-devel

On Wed, Jan 13, 2016 at 3:20 PM, Andre McCurdy <armccurdy@gmail.com> wrote:
> On Wed, Jan 13, 2016 at 2:19 PM, Richard Purdie
> <richard.purdie@linuxfoundation.org> wrote:
>> On Wed, 2016-01-13 at 13:37 -0800, Andre McCurdy wrote:
>>> On Thu, Jan 7, 2016 at 5:18 AM, Richard Purdie
>>> <richard.purdie@linuxfoundation.org> wrote:
>>> > Some servers, e.g. bitbucket.org can't cope with ssh:// as part of
>>> > the git url syntax. git itself is happy enough with this but you
>>> > get server side errors when using it.
>>> >
>>> > This changes the git fetcher to use the more common ssh url format
>>> > which also means we need a : before the path.
>>>
>>> This seems to break SRC_URIs using ssh to access self hosted git
>>> servers setup following the process described here:
>>>
>>>   https://git-scm.com/book/en/v2/Git-on-the-Server-Setting-Up-the-Ser
>>> ver
>>>
>>> An example of such a SRC_URI, which now fails to work:
>>>
>>>   SRC_URI = "git
>>> ://git@mylocalserver.com:/opt/git/myrepo.git;protocol=ssh"
>>
>> Does:
>>
>> SRC_URI = "git://git@mylocalserver.com/opt/git/myrepo.git;protocol=ssh"
>>
>> work though?
>
> Removing the ':' breaks things from the command line, but it does work
> OK in a SRC_URI (previously because of the ssh:// prefix and now
> because the fetcher is effectively adding the ':' back again).
>
> Updating my SRC_URIs looks like the best solution.
>
>> I'm not sure we ever suggested people should use ":/" in SRC_URI since
>> that isn't a url format the system was ever designed to cope with. It
>> might have happened to work but that is different...
>
> ":/" comes from the examples in the git-scm.com documentation:
>
>   https://git-scm.com/book/en/v2/Git-on-the-Server-Setting-Up-the-Server
>
> It would be nice if those examples (continued to) work in SRC_URIs and
> it's also nice if git URLs from SRC_URIs can be tested with git on the
> command line. Maybe those goals aren't worth the extra complexity
> though...

One final comment is that the original problem in #8864 seems to be
due to git's handling of URLs which contain both ssh:// and ':' (and
not an issue with BitBucket).

Setting GIT_SSH to a wrapper script which calls 'ssh -v' shows:

  $ git ls-remote ssh://git@bitbucket.org:armcc/openembedded-core.git
  ...
  debug1: Sending command: git-upload-pack '/openembedded-core.git'

ie the BitBucket username (armcc in this case) has already been
dropped before the git-upload-pack command is set to the server.

Trying to use the ssh:// prefix together with a URL containing a ':'
fails in exactly the same way with GitHub as well.

(Tested with git 1.9.1).


>> Cheers,
>>
>> Richard


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] fetch/git: Change to use clearer ssh url syntax for broken servers
  2016-01-13 22:19   ` Richard Purdie
  2016-01-13 23:20     ` Andre McCurdy
@ 2016-01-14 14:47     ` Peter Kjellerstedt
  2016-01-14 15:25       ` Richard Purdie
  1 sibling, 1 reply; 12+ messages in thread
From: Peter Kjellerstedt @ 2016-01-14 14:47 UTC (permalink / raw)
  To: Richard Purdie, Andre McCurdy; +Cc: bitbake-devel

> -----Original Message-----
> From: bitbake-devel-bounces@lists.openembedded.org [mailto:bitbake-
> devel-bounces@lists.openembedded.org] On Behalf Of Richard Purdie
> Sent: den 13 januari 2016 23:20
> To: Andre McCurdy
> Cc: bitbake-devel
> Subject: Re: [bitbake-devel] [PATCH] fetch/git: Change to use clearer
> ssh url syntax for broken servers
> 
> On Wed, 2016-01-13 at 13:37 -0800, Andre McCurdy wrote:
> > On Thu, Jan 7, 2016 at 5:18 AM, Richard Purdie
> > <richard.purdie@linuxfoundation.org> wrote:
> > > Some servers, e.g. bitbucket.org can't cope with ssh:// as part of
> > > the git url syntax. git itself is happy enough with this but you
> > > get server side errors when using it.
> > >
> > > This changes the git fetcher to use the more common ssh url format
> > > which also means we need a : before the path.
> >
> > This seems to break SRC_URIs using ssh to access self hosted git
> > servers setup following the process described here:
> >
> >   https://git-scm.com/book/en/v2/Git-on-the-Server-Setting-Up-the-Ser
> > ver
> >
> > An example of such a SRC_URI, which now fails to work:
> >
> >   SRC_URI = "git
> > ://git@mylocalserver.com:/opt/git/myrepo.git;protocol=ssh"
> 
> Does:
> 
> SRC_URI = "git://git@mylocalserver.com/opt/git/myrepo.git;protocol=ssh"
> 
> work though?

The above may work, but this does not any more:

SRC_URI = "git://git@mylocalserver.com:12345/opt/git/myrepo.git;protocol=ssh"

I.e., when the Git server requires a port, like our Gerrit server does...
Please revert the commit and solve the actual problem with bitbucket.org 
some other way.

> I'm not sure we ever suggested people should use ":/" in SRC_URI since
> that isn't a url format the system was ever designed to cope with. It
> might have happened to work but that is different...
> 
> Cheers,
> 
> Richard

//Peter



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] fetch/git: Change to use clearer ssh url syntax for broken servers
  2016-01-14 14:47     ` Peter Kjellerstedt
@ 2016-01-14 15:25       ` Richard Purdie
  2016-01-14 16:51         ` Andre McCurdy
  0 siblings, 1 reply; 12+ messages in thread
From: Richard Purdie @ 2016-01-14 15:25 UTC (permalink / raw)
  To: Peter Kjellerstedt, Andre McCurdy; +Cc: bitbake-devel

On Thu, 2016-01-14 at 14:47 +0000, Peter Kjellerstedt wrote:
> > -----Original Message-----
> > From: bitbake-devel-bounces@lists.openembedded.org [mailto:bitbake-
> > devel-bounces@lists.openembedded.org] On Behalf Of Richard Purdie
> > Sent: den 13 januari 2016 23:20
> > To: Andre McCurdy
> > Cc: bitbake-devel
> > Subject: Re: [bitbake-devel] [PATCH] fetch/git: Change to use
> > clearer
> > ssh url syntax for broken servers
> > 
> > On Wed, 2016-01-13 at 13:37 -0800, Andre McCurdy wrote:
> > > On Thu, Jan 7, 2016 at 5:18 AM, Richard Purdie
> > > <richard.purdie@linuxfoundation.org> wrote:
> > > > Some servers, e.g. bitbucket.org can't cope with ssh:// as part
> > > > of
> > > > the git url syntax. git itself is happy enough with this but
> > > > you
> > > > get server side errors when using it.
> > > > 
> > > > This changes the git fetcher to use the more common ssh url
> > > > format
> > > > which also means we need a : before the path.
> > > 
> > > This seems to break SRC_URIs using ssh to access self hosted git
> > > servers setup following the process described here:
> > > 
> > >   https://git-scm.com/book/en/v2/Git-on-the-Server-Setting-Up-the
> > > -Ser
> > > ver
> > > 
> > > An example of such a SRC_URI, which now fails to work:
> > > 
> > >   SRC_URI = "git
> > > ://git@mylocalserver.com:/opt/git/myrepo.git;protocol=ssh"
> > 
> > Does:
> > 
> > SRC_URI = "git
> > ://git@mylocalserver.com/opt/git/myrepo.git;protocol=ssh"
> > 
> > work though?
> 
> The above may work, but this does not any more:
> 
> SRC_URI = "git
> ://git@mylocalserver.com:12345/opt/git/myrepo.git;protocol=ssh"
> 
> I.e., when the Git server requires a port, like our Gerrit server
> does...
> Please revert the commit and solve the actual problem with
> bitbucket.org 
> some other way.

I'm not sure there is one other than special casing bitbucket urls in
the fetcher.

With Andre's case, we clearly don't support cut and paste of urls into
SRC_URI so I'm less concerned about that but the above is more of an
issue.

The url parsing in the fetcher should really be aware of port numbers
and I suspect if we do that, we can fix the above. I'm not going to
instantly revert the other change over this.

I'd also state that if you expect particular syntaxes to work, I'd
strongly encourage they're in the selftests for the fetcher. The self
tests didn't show any regressions...

Cheers,

Richard



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] fetch/git: Change to use clearer ssh url syntax for broken servers
  2016-01-14 15:25       ` Richard Purdie
@ 2016-01-14 16:51         ` Andre McCurdy
  2016-01-14 17:07           ` Olof Johansson
  0 siblings, 1 reply; 12+ messages in thread
From: Andre McCurdy @ 2016-01-14 16:51 UTC (permalink / raw)
  To: Richard Purdie; +Cc: bitbake-devel

On Thu, Jan 14, 2016 at 7:25 AM, Richard Purdie
<richard.purdie@linuxfoundation.org> wrote:
> On Thu, 2016-01-14 at 14:47 +0000, Peter Kjellerstedt wrote:
>> > -----Original Message-----
>> > From: bitbake-devel-bounces@lists.openembedded.org [mailto:bitbake-
>> > devel-bounces@lists.openembedded.org] On Behalf Of Richard Purdie
>> > Sent: den 13 januari 2016 23:20
>> > To: Andre McCurdy
>> > Cc: bitbake-devel
>> > Subject: Re: [bitbake-devel] [PATCH] fetch/git: Change to use
>> > clearer
>> > ssh url syntax for broken servers
>> >
>> > On Wed, 2016-01-13 at 13:37 -0800, Andre McCurdy wrote:
>> > > On Thu, Jan 7, 2016 at 5:18 AM, Richard Purdie
>> > > <richard.purdie@linuxfoundation.org> wrote:
>> > > > Some servers, e.g. bitbucket.org can't cope with ssh:// as part
>> > > > of
>> > > > the git url syntax. git itself is happy enough with this but
>> > > > you
>> > > > get server side errors when using it.
>> > > >
>> > > > This changes the git fetcher to use the more common ssh url
>> > > > format
>> > > > which also means we need a : before the path.
>> > >
>> > > This seems to break SRC_URIs using ssh to access self hosted git
>> > > servers setup following the process described here:
>> > >
>> > >   https://git-scm.com/book/en/v2/Git-on-the-Server-Setting-Up-the
>> > > -Ser
>> > > ver
>> > >
>> > > An example of such a SRC_URI, which now fails to work:
>> > >
>> > >   SRC_URI = "git
>> > > ://git@mylocalserver.com:/opt/git/myrepo.git;protocol=ssh"
>> >
>> > Does:
>> >
>> > SRC_URI = "git
>> > ://git@mylocalserver.com/opt/git/myrepo.git;protocol=ssh"
>> >
>> > work though?
>>
>> The above may work, but this does not any more:
>>
>> SRC_URI = "git
>> ://git@mylocalserver.com:12345/opt/git/myrepo.git;protocol=ssh"
>>
>> I.e., when the Git server requires a port, like our Gerrit server
>> does...
>> Please revert the commit and solve the actual problem with
>> bitbucket.org
>> some other way.
>
> I'm not sure there is one other than special casing bitbucket urls in
> the fetcher.

But I don't think there is any problem with BitBucket. The original
bug report was caused by trying to use a URL with a ':' between the
host and the repo path. A URL like that does not work when prefixed by
ssh://, it's the same for GitHub URLs too.

> With Andre's case, we clearly don't support cut and paste of urls into
> SRC_URI so I'm less concerned about that but the above is more of an
> issue.
>
> The url parsing in the fetcher should really be aware of port numbers
> and I suspect if we do that, we can fix the above. I'm not going to
> instantly revert the other change over this.
>
> I'd also state that if you expect particular syntaxes to work, I'd
> strongly encourage they're in the selftests for the fetcher. The self
> tests didn't show any regressions...
>
> Cheers,
>
> Richard
>


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] fetch/git: Change to use clearer ssh url syntax for broken servers
  2016-01-14 16:51         ` Andre McCurdy
@ 2016-01-14 17:07           ` Olof Johansson
  2016-01-14 17:50             ` Peter Kjellerstedt
  0 siblings, 1 reply; 12+ messages in thread
From: Olof Johansson @ 2016-01-14 17:07 UTC (permalink / raw)
  To: Andre McCurdy, Richard Purdie, bitbake-devel; +Cc: pkj

On 16-01-14 08:51 -0800, Andre McCurdy wrote:
> > I'm not sure there is one other than special casing bitbucket urls in
> > the fetcher.
> 
> But I don't think there is any problem with BitBucket. The original
> bug report was caused by trying to use a URL with a ':' between the
> host and the repo path. A URL like that does not work when prefixed by
> ssh://, it's the same for GitHub URLs too.

Indeed, the reported URL is not an RFC 3986 comformant URI. While

  ssh://example.com:/username/project.git

would be a kind of valid URI (the RFC doesn't forbid having an
empty port, see section 3.2.3), not having a path delimiter at
all however is not allowed. And fixing this by breaking RFC
comformant use cases is very suprising to me.

-- 
olofjn


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] fetch/git: Change to use clearer ssh url syntax for broken servers
  2016-01-14 17:07           ` Olof Johansson
@ 2016-01-14 17:50             ` Peter Kjellerstedt
  2016-01-15 15:04               ` Richard Purdie
  0 siblings, 1 reply; 12+ messages in thread
From: Peter Kjellerstedt @ 2016-01-14 17:50 UTC (permalink / raw)
  To: Olof Johansson, Andre McCurdy, Richard Purdie, bitbake-devel

> -----Original Message-----
> From: Olof Johansson
> Sent: den 14 januari 2016 18:08
> To: Andre McCurdy; Richard Purdie; bitbake-devel
> Cc: Peter Kjellerstedt
> Subject: Re: [bitbake-devel] [PATCH] fetch/git: Change to use clearer
> ssh url syntax for broken servers
> 
> On 16-01-14 08:51 -0800, Andre McCurdy wrote:
> > > I'm not sure there is one other than special casing bitbucket urls
> in
> > > the fetcher.
> >
> > But I don't think there is any problem with BitBucket. The original
> > bug report was caused by trying to use a URL with a ':' between the
> > host and the repo path. A URL like that does not work when prefixed
> by
> > ssh://, it's the same for GitHub URLs too.
> 
> Indeed, the reported URL is not an RFC 3986 comformant URI. While
> 
>   ssh://example.com:/username/project.git
> 
> would be a kind of valid URI (the RFC doesn't forbid having an
> empty port, see section 3.2.3), not having a path delimiter at
> all however is not allowed. And fixing this by breaking RFC
> comformant use cases is very suprising to me.
> 
> --
> olofjn

I have now looked at the ticket [YOCTO #8864] and actually created an 
account on bitbucket.org to actually verify what is happening, and I 
can only come to the conclusion that the ticket is gibberish. I have 
tried the following two URL:s with bitbucket.org, and they both work 
as expected:

git@bitbucket.org:Saur2000/test1.git
ssh://git@bitbucket.org/Saur2000/test1.git

The real error to the SRC_URI given in the ticket:

SRC_URI = "git://git@bitbucket.org:<username>/<repository>.git;rev=foo"

is that it is missing the protocol argument. Changing it to:

SRC_URI = "git://git@bitbucket.org/<username>/<repository>.git;protocol=ssh;rev=foo"

should make everything work as expected.

The ticket author obviously did not realize that the git:// scheme 
used in the SRC_URI is for BitBake and not for Git, and that when 
the protocol=ssh argument is added, then the rest of the URI must 
be changed to match the syntax expected by an ssh:// URI even though 
the scheme in SRC_URI is git://.

//Peter



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] fetch/git: Change to use clearer ssh url syntax for broken servers
  2016-01-14 17:50             ` Peter Kjellerstedt
@ 2016-01-15 15:04               ` Richard Purdie
  2016-01-15 16:18                 ` Olof Johansson
  0 siblings, 1 reply; 12+ messages in thread
From: Richard Purdie @ 2016-01-15 15:04 UTC (permalink / raw)
  To: Peter Kjellerstedt, Olof Johansson, Andre McCurdy, bitbake-devel

On Thu, 2016-01-14 at 17:50 +0000, Peter Kjellerstedt wrote:
> > -----Original Message-----
> > From: Olof Johansson
> > Sent: den 14 januari 2016 18:08
> > To: Andre McCurdy; Richard Purdie; bitbake-devel
> > Cc: Peter Kjellerstedt
> > Subject: Re: [bitbake-devel] [PATCH] fetch/git: Change to use
> > clearer
> > ssh url syntax for broken servers
> > 
> > On 16-01-14 08:51 -0800, Andre McCurdy wrote:
> > > > I'm not sure there is one other than special casing bitbucket
> > > > urls
> > in
> > > > the fetcher.
> > > 
> > > But I don't think there is any problem with BitBucket. The
> > > original
> > > bug report was caused by trying to use a URL with a ':' between
> > > the
> > > host and the repo path. A URL like that does not work when
> > > prefixed
> > by
> > > ssh://, it's the same for GitHub URLs too.
> > 
> > Indeed, the reported URL is not an RFC 3986 comformant URI. While
> > 
> >   ssh://example.com:/username/project.git
> > 
> > would be a kind of valid URI (the RFC doesn't forbid having an
> > empty port, see section 3.2.3), not having a path delimiter at
> > all however is not allowed. And fixing this by breaking RFC
> > comformant use cases is very suprising to me.
> > 
> > --
> > olofjn
> 
> I have now looked at the ticket [YOCTO #8864] and actually created an
> account on bitbucket.org to actually verify what is happening, and I 
> can only come to the conclusion that the ticket is gibberish. I have 
> tried the following two URL:s with bitbucket.org, and they both work 
> as expected:
> 
> git@bitbucket.org:Saur2000/test1.git
> ssh://git@bitbucket.org/Saur2000/test1.git
> 
> The real error to the SRC_URI given in the ticket:
> 
> SRC_URI = "git
> ://git@bitbucket.org:<username>/<repository>.git;rev=foo"
> 
> is that it is missing the protocol argument. Changing it to:
> 
> SRC_URI = "git
> ://git@bitbucket.org/<username>/<repository>.git;protocol=ssh;rev=foo
> "
> 
> should make everything work as expected.
> 
> The ticket author obviously did not realize that the git:// scheme 
> used in the SRC_URI is for BitBake and not for Git, and that when 
> the protocol=ssh argument is added, then the rest of the URI must 
> be changed to match the syntax expected by an ssh:// URI even though 
> the scheme in SRC_URI is git://.

I have also double checked this and I agree, the bitbucket servers do
now seem to work as you'd expect.

I swear I had problems originally but I can't figure out what those
were so I will revert the patch.

My point about improving the selftests if there are url formats you
depend upon stands though.

Cheers,

Richard



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] fetch/git: Change to use clearer ssh url syntax for broken servers
  2016-01-15 15:04               ` Richard Purdie
@ 2016-01-15 16:18                 ` Olof Johansson
  0 siblings, 0 replies; 12+ messages in thread
From: Olof Johansson @ 2016-01-15 16:18 UTC (permalink / raw)
  To: Richard Purdie; +Cc: bitbake-devel

On 16-01-15 15:04 +0000, Richard Purdie wrote:
> I have also double checked this and I agree, the bitbucket servers do
> now seem to work as you'd expect.
> 
> I swear I had problems originally but I can't figure out what those
> were so I will revert the patch.

Thanks!

> My point about improving the selftests if there are url formats you
> depend upon stands though.

While I'm happy to contribute such a change, I'm not fond of the
structure of the bitbake tests. They are a lot of times system
tests that require access to remote servers etc.

And in this case, the manipulation happens in the git fetcher,
and it has no unit tests at all (not including indirect system
testing). E.g., the following trivial patch to the URLHandle
tests would not catch this issue:

  --- a/bitbake/lib/bb/tests/fetch.py
  +++ b/bitbake/lib/bb/tests/fetch.py
  @@ -644,7 +644,8 @@ class URLHandle(unittest.TestCase):
  ...
  -       "git://git.openembedded.org/bitbake;branch=@foo" : ('git', 'git.openembedded.org', '/bitbake', '', '', {'branch': '@foo'})
  +       "git://git.openembedded.org/bitbake;branch=@foo" : ('git', 'git.openembedded.org', '/bitbake', '', '', {'branch': '@foo'}),
  +       "git://git.example.org:1234/bitbake" : ('git', 'git.example.org:1234', '/bitbake', '', '', {}),
  ...

I did try to add unittests for the git fetcher at one point, but
the patches were rejected because I made changes to the git
fetcher itself, in order to simplify testing. In this case, the
behavior is isolated to _get_repo_url, which is side effect free,
so again, I'm happy to try adding tests for this particular case.

Could also mention that the bb.fetch2.URI class' unittest suite
does have tests for various variations, including port numbers,
and it was designed to be side-effect free in order to make unit
testing simple. Adapting this would give some testing for free.


Anyways, I'll hopefully have a patch ready for review early next
week. Have a nice weekend :)

-- 
olofjn


^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2016-01-15 16:18 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-01-07 13:18 [PATCH] fetch/git: Change to use clearer ssh url syntax for broken servers Richard Purdie
2016-01-13 21:37 ` Andre McCurdy
2016-01-13 22:19   ` Richard Purdie
2016-01-13 23:20     ` Andre McCurdy
2016-01-13 23:59       ` Andre McCurdy
2016-01-14 14:47     ` Peter Kjellerstedt
2016-01-14 15:25       ` Richard Purdie
2016-01-14 16:51         ` Andre McCurdy
2016-01-14 17:07           ` Olof Johansson
2016-01-14 17:50             ` Peter Kjellerstedt
2016-01-15 15:04               ` Richard Purdie
2016-01-15 16:18                 ` Olof Johansson

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.