brz branch <location> partially ignores the location path when using the lp: shorthand

Bug #2003950 reported by Paride Legovini
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Launchpad itself
Fix Released
High
Colin Watson
breezy (Ubuntu)
Fix Released
High
Paride Legovini

Bug Description

I'll get straight to a reproducer. In the following commands we assume that no prior authentication to Launchpad is in place

This works fine:

bzr export foo https://code.launchpad.net/ubuntu-test-cases/server/testsuites/default/

as it creates a "foo" directory with the contents of /testsuites/default/.
However if we replace https://code.launchpad.net/ with the lp: shorthand, like this:

bzr export foo lp:ubuntu-test-cases/server/testsuites/default/

the "/testsuites/default/: is ignored, as we get the same output of

bzr export foo lp:ubuntu-test-cases/server

which is wrong. This appears to be a regression in version 3.3.x.

Related branches

Paride Legovini (paride)
summary: - brz branch <location> partially ignores the location to branch
+ brz branch <location> partially ignores the location path when using the
+ lp: shorthand
Revision history for this message
Paride Legovini (paride) wrote :

I bumped the importance of the Ubuntu task as this bug blocks the automated ISO testing of the daily ISO images.

Changed in breezy (Ubuntu):
assignee: nobody → Paride Legovini (paride)
importance: Undecided → High
Revision history for this message
Jelmer Vernooij (jelmer) wrote (last edit ):

This looks like a bug in the Launchpad API (the new one, which we migrated to in 3.3):

% lp-shell production devel
>>> lp.branches.getByPath(path="ubuntu-test-cases/server/testsuites/default/")
<branch at https://api.launchpad.net/devel/~ubuntu-server-qa/ubuntu-test-cases/server-tests-raring>

This basically just uses the part of the path it can resolve, and it ignores the rest. This prevents proper resolution on the Breezy side, because we don't know how much of the path it has consumed.

(you could argue that this is intended behaviour, but the docs don't mention it and it seems undesirable)

I think we have a couple of options:

1. wait for a fix on the Launchpad side (unlikely to happen soon)

2. go back to the old API (don't really want to do that, it added a lot of Launchpad specific code and would be time consuming)

3. drop expansion with the API and simply translate lp: to ://code.launchpad.net/

Perhaps https://code.launchpad.net/ or bzr+ssh://bazaar.launchpad.net depending on whether the user has logged in. Will break certain things like use of "~" in lp: URLs, but would fix this bug. Would also be a performance win as we get to avoid a HTTPS connection setup and API call.

Changed in brz:
status: New → Triaged
importance: Undecided → Medium
Revision history for this message
Jelmer Vernooij (jelmer) wrote :

Hmm, we could probably expand "~" locally.

Revision history for this message
Paride Legovini (paride) wrote :

I added a Launchpad task, let's see if LP developer chimes in.

Revision history for this message
Jelmer Vernooij (jelmer) wrote :

(3) isn't a good option, as we don't get the right redirects for bzr+ssh (at least not on all commands)

Revision history for this message
Jelmer Vernooij (jelmer) wrote :

You could also argue that subpaths for lp: URLs are just fundamentally ambiguous. For example,
does:

lp:brz/3.3 mean "the file 3.3 in https://code.launchpad.net/brz" or "the root of branch https://code.launchpad.net/brz"

Revision history for this message
Colin Watson (cjwatson) wrote :

I don't think this can really be done as a fix to `lp.branches.getByPath`. That returns a branch entry, and there's no space in that for a suffix. It also seems difficult to design a webservice API method that returns a tuple of a branch entry and a suffix - I don't think lazr.restfulclient could deal with that sort of thing very well.

On the other hand, none of this is very difficult to put together a new API for if we can agree on what shape it should take.

What was it in the old API you were trying to get away from? Was it just the XML-RPC nature of it (reasonable) or was there something about the response? If it was just the XML-RPC bit, we could add something that looks pretty much like the old API but runs over the webservice instead.

Alternatively, I wonder about something like this (hypothetically):

  >>> lp.branches.getBranchURLByPath(path='ubuntu-test-cases/server/testsuites/default', scheme='https')
  'https://bazaar.launchpad.net/~ubuntu-server-qa/ubuntu-test-cases/server-tests-raring/testsuites/default'

Then you could just ask for the scheme you want. Would that be convenient? Or is there some other shape of API that would work better for Breezy?

Revision history for this message
Jelmer Vernooij (jelmer) wrote :

It's okay if Launchpad returns "No branch found" if the path specified doesn't match a branch exactly but contains more path elements - it's the fact that it silently ignores the suffix at the moment that's the problem.

Breezy already walks up the directory structure (trying lp:ubuntu-test-cases/testsuites/default, then lp:ubuntu-tests-cases/testsuites, then lp:ubuntu-test-cases) until it finds somewhere that has a branch.

Yeah, the XML/RPC nature was problematic - there was hacks in the HTTP code to make it work. The HTTP code in breezy is generally a large collection of workarounds on top of urllib, and everything that can be stripped away helps. Less baggage as we explore asyncio support is also helpful.

Colin Watson (cjwatson)
Changed in launchpad:
status: New → In Progress
importance: Undecided → High
assignee: nobody → Colin Watson (cjwatson)
Revision history for this message
Colin Watson (cjwatson) wrote :

The Launchpad change as discussed above is deployed now. I'm currently getting "brz: ERROR: Repository does not exist." when I try to export or branch lp:ubuntu-test-cases/server, though - can somebody more familiar with Breezy check whether this is a Breezy problem or a Launchpad problem?

Changed in launchpad:
status: In Progress → Fix Released
Revision history for this message
Jelmer Vernooij (jelmer) wrote :

What version of breezy?

mordred% bzr branch lp:ubuntu-test-cases/server
Branched 206 revisions.
mordred% bzr --version
Breezy (brz) 3.3.2

Jelmer

Jelmer Vernooij (jelmer)
no longer affects: brz
Revision history for this message
Colin Watson (cjwatson) wrote :

$ bzr branch lp:ubuntu-test-cases/server
brz: ERROR: Repository does not exist.
$ bzr --version
Breezy (brz) 3.3.2
  Python interpreter: /usr/bin/python3 3.11.1
  Python standard library: /usr/lib/python3.11
  Platform: Linux-5.19.0-26-generic-x86_64-with-glibc2.36
  breezy: /usr/lib/python3/dist-packages/breezy
  Breezy configuration: /home/cjwatson/.bazaar
  Breezy log file: /home/cjwatson/.cache/breezy/brz.log

Copyright 2005-2012 Canonical Ltd.
Copyright 2017-2022 Breezy developers
https://www.breezy-vcs.org/

brz comes with ABSOLUTELY NO WARRANTY. brz is free software, and
you may use, modify and redistribute it under the terms of the GNU
General Public License version 2 or later.

Revision history for this message
Colin Watson (cjwatson) wrote :
Download full text (4.3 KiB)

Traceback from brz.log:

Fri 2023-02-03 18:38:38 +0000
0.406 Using Bazaar configuration directory (/home/cjwatson/.bazaar)
0.568 encoding stdout as sys.stdout encoding 'utf-8'
0.569 Using Bazaar configuration directory (/home/cjwatson/.bazaar)
0.569 encoding stdout as sys.stdout encoding 'utf-8'
0.569 Using Bazaar configuration directory (/home/cjwatson/.bazaar)
0.569 encoding stdout as sys.stdout encoding 'utf-8'
0.569 Using Bazaar configuration directory (/home/cjwatson/.bazaar)
0.587 Using Bazaar configuration directory (/home/cjwatson/.bazaar)
0.672 breezy version: 3.3.2
0.672 brz arguments: ['branch', 'lp:ubuntu-test-cases/server']
0.672 Using Bazaar configuration directory (/home/cjwatson/.bazaar)
0.672 Using Bazaar configuration directory (/home/cjwatson/.bazaar)
0.679 Using Bazaar configuration directory (/home/cjwatson/.bazaar)
1.045 Using Bazaar configuration directory (/home/cjwatson/.bazaar)
1.047 Using Bazaar configuration directory (/home/cjwatson/.bazaar)
1.047 encoding stdout as sys.stdout encoding 'utf-8'
1.047 Using Bazaar configuration directory (/home/cjwatson/.bazaar)
1.499 Using Bazaar configuration directory (/home/cjwatson/.bazaar)
2.007 Transferred: 0kB (4.0kB/s r:0kB w:0kB)
2.008 Using Bazaar configuration directory (/home/cjwatson/.bazaar)
2.063 Traceback (most recent call last):
  File "/usr/lib/python3/dist-packages/breezy/controldir.py", line 545, in _get_tree_branch
    tree = self.open_workingtree()
           ^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3/dist-packages/breezy/git/remote.py", line 624, in open_workingtree
    raise NotLocalUrl(self.transport.base)
breezy.errors.NotLocalUrl: git+ssh://git.launchpad.net/ubuntu-test-cases/server is not a local path.

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/usr/lib/python3/dist-packages/breezy/git/remote.py", line 487, in fetch_pack
    result = self._client.fetch_pack(
             ^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3/dist-packages/dulwich/client.py", line 1152, in fetch_pack
    refs, server_capabilities = read_pkt_refs(proto.read_pkt_seq())
                                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3/dist-packages/dulwich/client.py", line 259, in read_pkt_refs
    raise GitProtocolError(ref.decode("utf-8", "replace"))
dulwich.errors.GitProtocolError: Repository does not exist.

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/usr/lib/python3/dist-packages/breezy/commands.py", line 1036, in exception_to_return_code
    return the_callable(*args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3/dist-packages/breezy/commands.py", line 1228, in run_bzr
    ret = run(*run_argv)
          ^^^^^^^^^^^^^^
  File "/usr/lib/python3/dist-packages/breezy/commands.py", line 779, in run_argv_aliases
    return self.run(**all_cmd_args)
           ^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3/dist-packages/breezy/commands.py", line 804, in run
    return class_run(*args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3/dis...

Read more...

Revision history for this message
Paride Legovini (paride) wrote :

I think the difference ("Repository does not exist" vs repository is found) may lie in how lp: is resolved (https or ssh).

Revision history for this message
Paride Legovini (paride) wrote :

Marking the breezy package task as Fix Released (actually fixed on the LP side).

Changed in breezy (Ubuntu):
status: New → Fix Released
Jelmer Vernooij (jelmer)
no longer affects: brz
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.