Ability to download a tarball for a revision

Bug #240580 reported by Martin Albisetti on 2008-06-17
136
This bug affects 23 people
Affects Status Importance Assigned to Milestone
Launchpad itself
High
Martin Pool
loggerhead
Low
Martin Pool
Declined for Trunk by Martin Albisetti

Bug Description

We should allow users to download a tarball for the revision they are currently looking at, probably from the log view, and optionally on the files one too.

Related branches

Martin Albisetti (beuno) on 2008-06-17
Changed in loggerhead:
status: New → Confirmed
Martin Albisetti (beuno) wrote :

Also see bug #246764

Changed in loggerhead:
importance: Undecided → Medium
Martin Albisetti (beuno) on 2009-01-23
Changed in loggerhead:
status: Confirmed → Triaged
danigm (danigm) wrote :

I make changes to loggerhead to support this, here is my branch:

http://repo.danigm.net:8000/loggerhead/files

Thanks danigm. Can you explain how this works?

On Fri, Jan 30, 2009 at 1:50 PM, danigm <email address hidden> wrote:
> I make changes to loggerhead to support this, here is my branch:
>
> http://repo.danigm.net:8000/loggerhead/files
>
> --
> Ability to download a tarball for a revision
> https://bugs.launchpad.net/bugs/240580
> You received this bug notification because you are a direct subscriber
> of the bug.
>
> Status in Loggerhead: Triaged
>
> Bug description:
> We should allow users to download a tarball for the revision they are currently looking at, probably from the log view, and optionally on the files one too.
>

danigm (danigm) wrote :

I attach a bzr diff modificated to show only the changes that are interesting.

I make some modifies to loggerhead to show like I want, and the code is merged with style and other own modification that make it difficult.

I will explain how it work:

First it's needed that loggerhead.history could export:

--- loggerhead/history.py 2008-12-28 05:10:10 +0000
+++ loggerhead/history.py 2009-01-08 23:54:47 +0000
@@ -51,6 +51,7 @@
 import bzrlib.textfile
 import bzrlib.tsort
 import bzrlib.ui
+import bzrlib.export

 # bzrlib's UIFactory is not thread-safe
 uihack = threading.local()
@@ -919,3 +920,8 @@
                 lineno += 1

         self.log.debug('annotate: %r secs' % (time.time() - z))
+
+ def export(self, revid, dest):
+ rev_tree = self._branch.repository.revision_tree(revid)
+ bzrlib.export.export(rev_tree, dest)
+ return dest

Then I add this to download_ui controller:
=== modified file 'loggerhead/controllers/download_ui.py'
--- loggerhead/controllers/download_ui.py 2008-12-05 18:52:44 +0000
+++ loggerhead/controllers/download_ui.py 2009-01-08 23:54:47 +0000
@@ -25,10 +25,26 @@
 from paste.request import path_info_pop

 from loggerhead.controllers import TemplatedBranchView
+import os

 log = logging.getLogger("loggerhead.controllers")

+class TGZ(TemplatedBranchView):
+ def get_values(self, path, kwargs, headers):
+ history = self._history
+ revid = self.get_revid()
+
+ revno = history.get_revno(revid)
+ filename = '%s_%s.tgz' % (history._branch_nick, revno)
+ rpath = os.getcwd()+ '/loggerhead/static/downloads/' + filename
+
+ if not os.path.exists(rpath):
+ history.export(revid, rpath)
+
+ return {'download': '/static/downloads/'+filename}
+
+
 class DownloadUI (TemplatedBranchView):

     def __call__(self, environ, start_response):

Then I modify The loggerhead.controllers.__init__.py to make a HTTPRedirect when is a download. This redirect the navigator to the real filename that is placed in os.getcwd()+ '/loggerhead/static/downloads/' + filename. It need to be changed because you need to run loggerhead inside the directory, instead of use os.getcwd you can use a config variable.

Then I modify loggerhead.apps.branch.py to add TGZ page and respond to redirection raising instead of returning.

And finally I modify the templates to show a download button.

Now I will try to create a launchpad branch of loggerhead applying that changes.

danigm (danigm) wrote :

I make the branch, and apply the changes. It's here:

https://code.launchpad.net/~danigm/loggerhead/loggerhead

I make a proposal to merge with trunk.

Alexander Belchenko (bialix) wrote :

zip archive as second option, please?

Eitan Isaacson (eeejay) wrote :

Any chance we could see this in trunk? I find this feature very useful. Github does this.

Martin Albisetti (beuno) wrote :

Yes, I'll try yo squeeze it in, I really want this too, and we already have in place the mechanism to switch the feature on/off.

Changed in loggerhead:
assignee: nobody → Martin Albisetti (beuno)
Martin Albisetti (beuno) wrote :

Never got to this :(

Changed in loggerhead:
assignee: Martin Albisetti (beuno) → nobody
Alexander Belchenko (bialix) wrote :

Martin Albisetti пишет:
> Never got to this :(

Bad :'-(

Martin Pool (mbp) on 2009-11-24
Changed in loggerhead:
status: Triaged → In Progress

bump? This is the most important feature I think loggerhead is missing - every similar program I'm aware of has this feature (all the git/hg web clients). It is really annoying to have to send tar balls to people who aren't going to contribute and don't have bazaar when its hosted anyway.

Please add this feature!

xaav (xaav) wrote :

This is really important to me. Please add this feature!

Robert Collins (lifeless) wrote :

Taking this out of 'in progress' because the merge proposal is stalled.

Changed in loggerhead:
status: In Progress → Triaged
importance: Medium → Low
Alexander Belchenko (bialix) wrote :

I'm sad to see you set the importance to Low.

lborgman (lennart-borgman) wrote :

I am surprised, the code for making and downloading a tarball is there, but the merge proposal has stalled. What is really happening?

Martin Albisetti (beuno) wrote :

I'll try and pick up the branch and re-propose it with the needed config change.

Changed in loggerhead:
assignee: nobody → Martin Albisetti (beuno)
status: Triaged → In Progress
Robert Collins (lifeless) wrote :

@Alexander I set it to low because we really don't get to bugs below 'high' unless they are retriaged to high : I was making it more accurately reflect reality.

@Martin - the key thing is to make it fit in with the LP deployment I think - doesn't necessarily need a config option, but it needs to be easy for launchpad_loggerhead to disable it *if* we find its a performance problem.

John A Meinel (jameinel) wrote :

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 03/09/2011 11:42 PM, Robert Collins wrote:
> @Alexander I set it to low because we really don't get to bugs below
> 'high' unless they are retriaged to high : I was making it more
> accurately reflect reality.
>
> @Martin - the key thing is to make it fit in with the LP deployment I
> think - doesn't necessarily need a config option, but it needs to be
> easy for launchpad_loggerhead to disable it *if* we find its a
> performance problem.
>

If this is true, then it should probably be a config option that the
LOSAs can set and restart (or SIGHUP?) the system. Having it be a
code-level option makes it quite a bit harder to change in-the-wild. (IMO).

John
=:->

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.10 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iEYEARECAAYFAk14mjgACgkQJdeBCYSNAAOo2wCeNouNpjBFpNKP5dz121hFymlQ
bFEAniZgRVTR+vJ2P9crD84kprbQT1lX
=Y3Fe
-----END PGP SIGNATURE-----

Robert Collins (lifeless) wrote :

On Thu, Mar 10, 2011 at 10:30 PM, John A Meinel <email address hidden> wrote:
> If this is true, then it should probably be a config option that the
> LOSAs can set and restart (or SIGHUP?) the system. Having it be a
> code-level option makes it quite a bit harder to change in-the-wild. (IMO).

Our config system in launchpad would set the flag in the loggerhead
side - its still not a /loggerhead config/ setting.

John A Meinel (jameinel) wrote :

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 03/10/2011 11:35 AM, Robert Collins wrote:
> On Thu, Mar 10, 2011 at 10:30 PM, John A Meinel <email address hidden> wrote:
>> If this is true, then it should probably be a config option that the
>> LOSAs can set and restart (or SIGHUP?) the system. Having it be a
>> code-level option makes it quite a bit harder to change in-the-wild. (IMO).
>
> Our config system in launchpad would set the flag in the loggerhead
> side - its still not a /loggerhead config/ setting.

Fair enough. Though the default being True might be something to think
about. Especially since the command line default for serve-branches (as
proposed) is False.

John
=:->

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.10 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iEYEARECAAYFAk14t4AACgkQJdeBCYSNAAOIGwCggI6ONQaGh809SefgrkNbEet3
n5YAoKjJwNzOFX7G9VS0LI0kg5C9vFOh
=jfbA
-----END PGP SIGNATURE-----

xaav (xaav) wrote :

Um... how is this coming along? Don't want this bug to get stale.

Martin Albisetti (beuno) wrote :

I've got a merge proposal up, and I'm working through the review comment: https://code.launchpad.net/~beuno/loggerhead/export-tarball/+merge/52797

Martin Albisetti (beuno) wrote :

At this stage, some changes were required in bzrlib to do this properly, and they have been done and landed by Jelmer.
I will pick this up again and use the new code, although this will mean it won't work with previous versions of bzr. It will require some thought.

xaav (xaav) wrote :

Great to hear!

xaav (xaav) wrote :

See http://wiki.bazaar.canonical.com/BzrLib, Create Local copy of branch.

xaav (xaav) on 2011-05-31
Changed in loggerhead:
assignee: Martin Albisetti (beuno) → Geoff (xaav)
Martin Pool (mbp) wrote :

I've merged an updated version of xaav's work to trunk from https://code.launchpad.net/~mbp/loggerhead/240580-tarball/+merge/83364

I will see about deploying that in lp

Launchpad QA Bot (lpqabot) wrote :
tags: added: qa-needstesting
Changed in loggerhead:
status: In Progress → Fix Committed
Martin Pool (mbp) wrote :

qa: http://bazaar.qastaging.launchpad.net/~rharding/bzr/rick2/files is just redirecting back to qastaging.launchpad.net. I don't know why. I don't think this needs to block deployment at least.

tags: added: qa-ok
removed: qa-needstesting
Robert Collins (lifeless) wrote :

On Sat, Nov 26, 2011 at 11:15 AM, Martin Pool <email address hidden> wrote:
> qa: http://bazaar.qastaging.launchpad.net/~rharding/bzr/rick2/files is
> just redirecting back to qastaging.launchpad.net.  I don't know why.  I

The content probably doesn't exist. Remember we only mirror a few
branches to qastaging.

Martin Pool (mbp) wrote :

qas had a broken deployment, which is now fixed.

You can now download a tarball from, for example, <http://bazaar.qastaging.launchpad.net/~mbp/+junk/buildd/revision/41>. (That link will break eventually.)

That should be live on launchpad.net in a couple of days.

I'm going to leave this open for the moment because the links through which you can download a tarball are not in all the places you would expect. It should also be on:

  * http://bazaar.qastaging.launchpad.net/~mbp/+junk/buildd/files
  * http://bazaar.qastaging.launchpad.net/~mbp/+junk/buildd/files/41
  * http://bazaar.qastaging.launchpad.net/~mbp/+junk/buildd/changes

Changed in launchpad:
status: New → In Progress
importance: Undecided → High
assignee: nobody → Martin Pool (mbp)
Changed in loggerhead:
assignee: xaav (xaav) → Martin Pool (mbp)
status: Fix Committed → In Progress
Martin Pool (mbp) wrote :

Actually I am going to mark this closed and follow on in bug 897535 and bug 897537.

Thanks to everyone who contributed and I'm sorry it took so long to get it over the line.

Changed in launchpad:
status: In Progress → Fix Released
Changed in loggerhead:
status: In Progress → Fix Released
lborgman (lennart-borgman) wrote :

Great! Thanks!

To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Duplicates of this bug

Other bug subscribers