Reading options for a branch from locations.conf fails with bzr+ssh

Bug #302593 reported by Jeroen Hoekx
16
This bug affects 3 people
Affects Status Importance Assigned to Milestone
Bazaar
Confirmed
Medium
Unassigned
Breezy
Triaged
Medium
Unassigned

Bug Description

I push my branch to a remote server using bzr+ssh:// in bzr 1.9. On the server, a feed is generated by a plugin using the 'post_change_branch_tip' hook. This plugin reads configuration options for the feed from locations.conf. This works fine when I'm working on the remote location and committing from there, but doing it over bzr+ssh:// does not work.

The branch location in that case is somthing like "chroot--1212656852:///users/prom2009/jhoekx/public_html/thesis/", which is not matched by the usual "/users/prom2009/jhoekx/public_html/thesis" line in locations.conf. bzr changes a file:// url into a normal filesystem path when reading the configuration file, but it does not change chroot urls. I fixed it with the attached patch, by detecting chroot urls and splitting on // in config.py, line 490.

        if location.startswith('file://'):
            location = urlutils.local_path_from_url(location)
        elif location.startswith('chroot'):
            location = urlutils.unescape(location.split("//",1)[1])
        self.location = location

Tags: config hpss
Revision history for this message
Jeroen Hoekx (jeroen-hoekx) wrote :
Revision history for this message
Jeroen Hoekx (jeroen-hoekx) wrote :

Previous patch did not unescape the url, sorry.

Revision history for this message
Marius Kruger (amanica) wrote :

I also came across this previously when trying to configure the shell-hooks plugin in locations.conf for server-side hooks.

Changed in bzr:
status: New → Confirmed
Revision history for this message
Andrew Bennetts (spiv) wrote :

Agreed, this is important for supporting server-side hooks well.

Perhaps the right API to get the real location is branch.bzrdir.root_transport.external_url(), but I'm not sure that ChrootTransport.external_url does the right thing yet.

Changed in bzr:
importance: Undecided → Medium
Revision history for this message
Marius Kruger (amanica) wrote : Re: [Bug 302593] Re: Reading options for a branch from locations.conf fails with bzr+ssh

2008/11/27 Andrew Bennetts <email address hidden>

> Agreed, this is important for supporting server-side hooks well.
>
> Perhaps the right API to get the real location is
> branch.bzrdir.root_transport.external_url(), but I'm not sure that
> ChrootTransport.external_url does the right thing yet.
>

The last time I checked, it did not.
See https://bugs.launchpad.net/bzr/+bug/270267

Revision history for this message
John Ferlito (johnf-inodes) wrote :

I was having a similar issue. I've fixed this a little bit further up in the chain in BranchConfig. Teaching it that it needs to unchroot the chroot URI.

Branch with fixes can be found at http://bzr.inodes.org/bzr/chroot_location_support/

Not quite sure if this is correct though. My use case is as follows.

* Running "bzr server"
* directory points at /bzr/srv which has multiple directorys which contain branches
* locations.conf has

[/bzr/srv]
public_branch = http://repo.com/
public_branch:policy = appendpath

Say I'm pushing to bzr://repo.com/project1/trunk

then I need file:///bzr/srv/project1/trunk to be passed into LocationConfig to reproduce that URL in bzr-email.

The patch I'm proposing does this.

It might make sense for this work to happen in bzrlib.transport.chroot maybe in external_url however I don't think there is acess to the branch in there so you can't access the /project1/trunk bit

Revision history for this message
James Teh (jteh) wrote :

I'm using bzr-email with locations.conf config on a bzr+ssh server and am encountering this as well. Strangely, this used to work fine in an earlier version of bzr, perhaps bzr 1.17, yet this bug indicates that this issue has existed since before then. Can anyone shed some light on this?

Revision history for this message
James Teh (jteh) wrote :

For bzr 2.1.0, John Ferlito's fix needs to be modified to include PathFilteringTransport as well as ChrootTransport. I'm guessing this is because of the new expansion of userdirs in the smart server.

I'm attaching a plugin which monkey patches BranchConfig._get_location() with John's fix (including my aforementioned change). I put this in a plugin because I want to be able to use the deb packages for bzr without modification. It's nasty, but perhaps it will be useful to someone else as well until this issue is fixed in the core.

Vincent Ladeuil (vila)
tags: added: config
Jelmer Vernooij (jelmer)
tags: added: check-for-breezy
Jelmer Vernooij (jelmer)
tags: removed: check-for-breezy
Changed in brz:
status: New → Triaged
importance: Undecided → Medium
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.