fetch/git: Change to use clearer ssh url syntax for broken servers

PATCH In review Richard Purdie 4 participants 12 messages Jan 7, 2016 – Jan 15, 2016

Share & tools

Richard Purdie

author

2016-01-07 13:18 UTC #0 lore raw

PATCH 1/1 fetch/git: Change to use clearer ssh url syntax for broken servers

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 <[email protected]>

bitbake/lib/bb/fetch2/git.py +4 -0

index 5ffab22..10ba1d3 100644
@@ -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):

Andre McCurdy

2016-01-13 21:37 UTC #1 lore raw

Quoted 6 lines · Richard Purdie

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://[email protected]:/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://[email protected]:<username>/<repository>.git;protocol=ssh"

should be:

SRC_URI = "git://[email protected]/<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).

Quoted 30 lines

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 <[email protected]>
bitbake/lib/bb/fetch2/git.py +4 -1
index 5ffab22..10ba1d3 100644
@@ -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
[email protected]
http://lists.openembedded.org/mailman/listinfo/bitbake-devel

Richard Purdie

author

2016-01-13 22:19 UTC #2 lore raw

Quoted 12 lines · Andre McCurdy

Quoted 6 lines · Richard Purdie
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:

Quoted 7 lines

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
://[email protected]:/opt/git/myrepo.git;protocol=ssh"

Does:

SRC_URI = "git://[email protected]/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

Andre McCurdy

2016-01-13 23:20 UTC #3 lore raw

Quoted 18 lines · Richard Purdie

Quoted 17 lines · Andre McCurdy
Quoted 6 lines · Richard Purdie
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:

Quoted 8 lines

Quoted 2 lines
SRC_URI = "git
://[email protected]:/opt/git/myrepo.git;protocol=ssh"
Does:
SRC_URI = "git://[email protected]/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.

Quoted 3 lines

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...

Quoted 3 lines

Cheers,
Richard

Andre McCurdy

2016-01-13 23:59 UTC #4 lore raw

Quoted 39 lines · Andre McCurdy

Quoted 18 lines · Richard Purdie
Quoted 17 lines · Andre McCurdy
Quoted 6 lines · Richard Purdie
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:
Quoted 8 lines
Quoted 2 lines
SRC_URI = "git
://[email protected]:/opt/git/myrepo.git;protocol=ssh"
Does:
SRC_URI = "git://[email protected]/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.
Quoted 3 lines
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...

Quoted 8 lines

":/" 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://[email protected]: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).

Quoted 3 lines

Quoted 3 lines
Cheers,
Richard

Peter Kjellerstedt

2016-01-14 14:47 UTC #5 lore raw

In reply to Richard Purdie

Quoted 27 lines

-----Original Message-----
From: [email protected] [mailto:bitbake- [email protected]] 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
Quoted 17 lines · Andre McCurdy
Quoted 6 lines · Richard Purdie
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:

Quoted 8 lines

Quoted 2 lines
SRC_URI = "git
://[email protected]:/opt/git/myrepo.git;protocol=ssh"
Does:
SRC_URI = "git://[email protected]/opt/git/myrepo.git;protocol=ssh"
work though?

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

SRC_URI = "git://[email protected]: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.

Quoted 7 lines

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

Richard Purdie

author

2016-01-14 15:26 UTC #6 lore raw

Quoted 44 lines · Peter Kjellerstedt

Quoted 35 lines
-----Original Message-----
From: [email protected] [mailto:bitbake- [email protected]] 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
Quoted 15 lines · Andre McCurdy
Quoted 9 lines · Richard Purdie
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:
Quoted 8 lines
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
://[email protected]:/opt/git/myrepo.git;protocol=ssh"
Quoted 6 lines
Does:
SRC_URI = "git ://[email protected]/opt/git/myrepo.git;protocol=ssh"
work though?
The above may work, but this does not any more:

Quoted 8 lines

SRC_URI = "git ://[email protected]: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

Andre McCurdy

2016-01-14 16:51 UTC #7 lore raw

Quoted 48 lines · Richard Purdie

Quoted 47 lines · Peter Kjellerstedt
Quoted 35 lines
-----Original Message-----
From: [email protected] [mailto:bitbake- [email protected]] 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
Quoted 15 lines · Andre McCurdy
Quoted 9 lines · Richard Purdie
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:
Quoted 8 lines
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
://[email protected]:/opt/git/myrepo.git;protocol=ssh"
Quoted 6 lines
Does:
SRC_URI = "git ://[email protected]/opt/git/myrepo.git;protocol=ssh"
work though?
The above may work, but this does not any more:
SRC_URI = "git ://[email protected]:12345/opt/git/myrepo.git;protocol=ssh"

Quoted 8 lines

Quoted 5 lines
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.

Quoted 16 lines

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

Olof Johansson

2016-01-14 17:14 UTC #8 lore raw

Quoted 7 lines · Andre McCurdy

Quoted 2 lines
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.

Signature · 1 line

olofjn

Peter Kjellerstedt

2016-01-14 17:50 UTC #9 lore raw

Quoted 23 lines

-----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
Quoted 1 line · Andre McCurdy
Quoted 1 line
I'm not sure there is one other than special casing bitbucket urls
in
Quoted 5 lines
Quoted 1 line
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
Quoted 1 line
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

Quoted 7 lines

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:

[email protected]:Saur2000/test1.git
ssh://[email protected]/Saur2000/test1.git

The real error to the SRC_URI given in the ticket:

SRC_URI = "git://[email protected]:<username>/<repository>.git;rev=foo"

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

SRC_URI = "git://[email protected]/<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

Richard Purdie

author

2016-01-15 15:05 UTC #10 lore raw

Quoted 56 lines · Peter Kjellerstedt

Quoted 28 lines
-----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
Quoted 2 lines · Andre McCurdy
Quoted 2 lines
I'm not sure there is one other than special casing bitbucket urls
in
Quoted 8 lines
Quoted 1 line
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
Quoted 1 line
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
Quoted 7 lines
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:
[email protected]:Saur2000/test1.git
ssh://[email protected]/Saur2000/test1.git
The real error to the SRC_URI given in the ticket:
SRC_URI = "git ://[email protected]:<username>/<repository>.git;rev=foo"
is that it is missing the protocol argument. Changing it to:
SRC_URI = "git ://[email protected]/<username>/<repository>.git;protocol=ssh;rev=foo "

Quoted 7 lines

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

Olof Johansson

2016-01-15 16:18 UTC #11 lore raw

Quoted 5 lines · Richard Purdie

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!

Quoted 2 lines

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 :)

Signature · 1 line

olofjn