On Thu, Jul 12, 2012 at 09:52:55PM -0700, Andi McClure wrote: > Does this help? Attached are patches for the two changes discussed in this > thread: > > - send_pack should pass determine_wants a copy of the refs dict, not the > refs dict, so that the comparison shortly after can't get confused Thanks, applied.
> - Make usage of 'determine_wants' callbacks explicit in inline documentation > > Things to note: I claim in the documentation that determine_wants changes > remote heads even on fetch (because this appears to be the case from > reading the code?); and I do not improve the usage documentation around the > "generate_pack_contents" callbacks, because I do not understand how they > are used. That isn't right - determine_wants for fetch() returns a list of objects, not a dictionary. determine_wants probably isn't an entirely appropriate name for what that function does for send_pack(); it is also slightly confusing that both send and fetch take an argument with the same name that behaves slightly differently. Perhaps we should rename the determine_wants argument for send_pack to update_refs or something like that. > Also, for future reference, do you prefer proposed patches as patches on > the mailing list, or pull requests on github, or what? Preferably git-am style patches (with metadata) on the mailing list, rather than github. Cheers, Jelmer > On Wed, Jul 11, 2012 at 5:26 PM, Jelmer Vernooij <jel...@debian.org> wrote: > > > On Sun, Jul 08, 2012 at 01:41:18PM -0700, Andi McClure wrote: > > > Here is a standalone script which reproduces the problem, it's just a > > > connect, the push and some prints. My expected behavior is that on the > > > second run it will print "target is already deleted" because the first > > run > > > will have deleted it. What I see is that on every run it gets an accurate > > > list of heads from the server but then has no effect when it tries to > > > change them. Changing the ZERO_SHA to None has no effect either. > > > > > > Since this connects directly to one of my github repos you will probably > > > get a permissions error if you run it as written, maybe you could clone > > > https://github.com/mcclure/backup-Polycode and then change the "path" > > line > > > to point to your own github account? > > > > > > ######## Start > > > > > > import dulwich.client > > > from dulwich.protocol import ZERO_SHA > > > > > > client = dulwich.client.SSHGitClient("g...@github.com") > > > path = "/mcclure/backup-Polycode" > > > delete_this_head = "refs/heads/bullet279" > > > > > > print("client, path is: %s,%s\ntarget is %s\n" % > > > (client,repr(path),repr(delete_this_head))) > > > > > > def changed(refs): > > > print("determine_wants argument:\n%s\n" % (refs)) > > > if delete_this_head in refs: > > > refs[delete_this_head] = ZERO_SHA > > > print("determine_wants returning:\n%s\n" % (refs)) > > > else: > > > print("target is already deleted!\n") > > > return refs > > > > Unfortunately your mailer seems to have stripped the whitespace from > > these lines. It looks though like you're trying to modify the existing > > refs dictionary rather than returning your own. > > > > dulwich compares the dictionary it passes in and yours to see if it > > should make any changes to the remote server. A simple fix for this > > should be to add "refs = dict(refs)" as the first instruction in the > > changed() function. > > > > We should probably also improve dulwich by making it harder to shoot > > yourself in the foot like this, and having it take create a copy to > > pass to you. > > > > Cheers, > > > > Jelmer > > > > > > > > On Sun, Jul 8, 2012 at 6:27 AM, Jelmer Vernooij <jel...@samba.org> > > wrote: > > > > > > > On Sat, Jul 07, 2012 at 08:07:51PM -0700, Andi McClure wrote: > > > > > "What do you mean exactly when you say it doesn't work?" > > > > > > > > > When I say it doesn't work, what I mean is it appears to do nothing. > > The > > > > > remote repository is afterward unchanged, looking at github.com. > > > > > > > > > If it's okay to paste a big block, here's exactly what is happening. > > I'm > > > > > running the following code with the argument "key" = "bullet279" and > > the > > > > > referenced value "null_revision_hex" equal to "00"*20. > > > > Can you condense it down into just the bit that reproduces the issue? > > > > This should just do the connect and the call to .send_pack(). > > > > > > > > Actually, come to think of it - you might have to set the value to > > > > None rather than the ZERO_SHA1 - have you tried that? > > > > > > > > Cheers, > > > > > > > > Jelmer > > > > > > > > > client, path = > > > > > > self.git_handler.get_transport_and_path(self.path) > > > > > > print("client, path is:") > > > > > > print(client) > > > > > > print(path) > > > > > > def changed(refs): > > > > > > desired = gitrepo.head_prefix + key > > > > > > print("determine_wants argument:") > > > > > > print(refs) > > > > > > if desired in refs: > > > > > > refs[desired] = GitHandler.null_revision_hex > > > > > > print("determine_wants return:") > > > > > > print(refs) > > > > > > return refs > > > > > > def adebugger(*args): > > > > > > print("generate_pack_contents arguments:") > > > > > > print(args) > > > > > > import pdb; pdb.set_trace() > > > > > > return args > > > > > > genpack = adebugger > > > > > > try: > > > > > > self.git_handler.ui.status(_("creating and sending > > data > > > > > > for remote bookmark deletion\n")) > > > > > > changed_refs = client.send_pack(path, changed, > > genpack) > > > > > > print("sent_pack returned:") > > > > > > print(changed_refs) > > > > > > return True > > > > > > except (HangupException, GitProtocolError), e: > > > > > > raise hgutil.Abort(_("git remote error: ") + > > str(e)) > > > > > > > > > > > > > This is resulting in the following printed: > > > > > > > > > client, path is: > > > > > > <dulwich.client.SSHGitClient object at 0x118bed0> > > > > > > /mcclure/backup-Polycode > > > > > > creating and sending data for remote bookmark deletion > > > > > > ["git-receive-pack '/mcclure/backup-Polycode'"] > > > > > > determine_wants argument: > > > > > > {'refs/heads/screeninfo': > > '60660e76486583708684ae10aa5df4751f948570', > > > > > > 'refs/heads/createlua': '54995beed753f0af4d7f1c4f79b520e18fc6d22a', > > > > > > 'refs/heads/xibfix': 'd69b878d38632f3af8fd07127f05ce9c9e368e23', > > > > > > 'refs/heads/scene': '1a686b0d0ad3d9344c9ca463dac196be61dfd49e', > > > > > > 'refs/heads/zlibfix': '79762dc444f66c30710af2d8534956eab27aef7d', > > > > > > 'refs/heads/sound': 'b7e414ee212e3cc2fa93ed93053eeff71abfbd0a', > > > > > > 'refs/heads/dashg': '51030fa9ab337006ed2c9db05151b275cc4341f5', > > > > > > 'refs/heads/add_dependencies': > > > > '300bf6ac818221882505b9c8976830c2777a6964', > > > > > > 'refs/heads/bullet279': 'b13f974ce94790d53bdf166ecc68b499fbf0b629', > > > > > > 'refs/heads/cmake': '8a024fc8760e93f37229a8ef3f8c71961e1c4bf8', > > > > > > 'refs/heads/mingw': 'a573d046fe2cb9daad806a4a4c0853e4abf3b7c3', > > > > > > 'refs/heads/windowsquit': > > '94f728a23c8ddb93c6d1426eea25ada165af8bd9', > > > > > > 'refs/heads/zlib127': '67e474d6020f3ed3c20bde1d905f9fc53273416f', > > > > > > 'refs/heads/clearMesh': '6be76b1b21216485557d5cc1356be321ac761864', > > > > > > 'refs/heads/safestring': > > '9de8332077b1b13acb6c40c52479dde51f6d88df', > > > > > > 'refs/heads/master': '8355b00050696f4ddaaf13bf8ec721b3cf8b08e2', > > > > > > 'refs/heads/shaderfixes': > > 'f64acadc1c0ef4b6cfe6ff1e43c61b22bc216673'} > > > > > > determine_wants return: > > > > > > {'refs/heads/screeninfo': > > '60660e76486583708684ae10aa5df4751f948570', > > > > > > 'refs/heads/createlua': '54995beed753f0af4d7f1c4f79b520e18fc6d22a', > > > > > > 'refs/heads/xibfix': 'd69b878d38632f3af8fd07127f05ce9c9e368e23', > > > > > > 'refs/heads/scene': '1a686b0d0ad3d9344c9ca463dac196be61dfd49e', > > > > > > 'refs/heads/zlibfix': '79762dc444f66c30710af2d8534956eab27aef7d', > > > > > > 'refs/heads/sound': 'b7e414ee212e3cc2fa93ed93053eeff71abfbd0a', > > > > > > 'refs/heads/dashg': '51030fa9ab337006ed2c9db05151b275cc4341f5', > > > > > > 'refs/heads/add_dependencies': > > > > '300bf6ac818221882505b9c8976830c2777a6964', > > > > > > 'refs/heads/bullet279': '0000000000000000000000000000000000000000', > > > > > > 'refs/heads/cmake': '8a024fc8760e93f37229a8ef3f8c71961e1c4bf8', > > > > > > 'refs/heads/mingw': 'a573d046fe2cb9daad806a4a4c0853e4abf3b7c3', > > > > > > 'refs/heads/windowsquit': > > '94f728a23c8ddb93c6d1426eea25ada165af8bd9', > > > > > > 'refs/heads/zlib127': '67e474d6020f3ed3c20bde1d905f9fc53273416f', > > > > > > 'refs/heads/clearMesh': '6be76b1b21216485557d5cc1356be321ac761864', > > > > > > 'refs/heads/safestring': > > '9de8332077b1b13acb6c40c52479dde51f6d88df', > > > > > > 'refs/heads/master': '8355b00050696f4ddaaf13bf8ec721b3cf8b08e2', > > > > > > 'refs/heads/shaderfixes': > > 'f64acadc1c0ef4b6cfe6ff1e43c61b22bc216673'} > > > > > > sent_pack returned: > > > > > > {'refs/heads/screeninfo': > > '60660e76486583708684ae10aa5df4751f948570', > > > > > > 'refs/heads/createlua': '54995beed753f0af4d7f1c4f79b520e18fc6d22a', > > > > > > 'refs/heads/xibfix': 'd69b878d38632f3af8fd07127f05ce9c9e368e23', > > > > > > 'refs/heads/scene': '1a686b0d0ad3d9344c9ca463dac196be61dfd49e', > > > > > > 'refs/heads/zlibfix': '79762dc444f66c30710af2d8534956eab27aef7d', > > > > > > 'refs/heads/sound': 'b7e414ee212e3cc2fa93ed93053eeff71abfbd0a', > > > > > > 'refs/heads/dashg': '51030fa9ab337006ed2c9db05151b275cc4341f5', > > > > > > 'refs/heads/add_dependencies': > > > > '300bf6ac818221882505b9c8976830c2777a6964', > > > > > > 'refs/heads/bullet279': '0000000000000000000000000000000000000000', > > > > > > 'refs/heads/cmake': '8a024fc8760e93f37229a8ef3f8c71961e1c4bf8', > > > > > > 'refs/heads/mingw': 'a573d046fe2cb9daad806a4a4c0853e4abf3b7c3', > > > > > > 'refs/heads/windowsquit': > > '94f728a23c8ddb93c6d1426eea25ada165af8bd9', > > > > > > 'refs/heads/zlib127': '67e474d6020f3ed3c20bde1d905f9fc53273416f', > > > > > > 'refs/heads/clearMesh': '6be76b1b21216485557d5cc1356be321ac761864', > > > > > > 'refs/heads/safestring': > > '9de8332077b1b13acb6c40c52479dde51f6d88df', > > > > > > 'refs/heads/master': '8355b00050696f4ddaaf13bf8ec721b3cf8b08e2', > > > > > > 'refs/heads/shaderfixes': > > 'f64acadc1c0ef4b6cfe6ff1e43c61b22bc216673'} > > > > > > > > > > > > > So some things I notice: > > > > > - my generate_pack_contents function is never called. > > > > > - refs/heads/bullet279 is set to the value listed as ZERO_SHA1 in the > > > > > dulwich source in the return of my "determine_wants" function. > > > > > - The value returned by send_pack seems to be equal to the value > > returned > > > > > by my determine_wants function, indicating I succeeded. > > > > > > > > > However after running I check my test repository > > > > > https://github.com/mcclure/backup-Polycode in a web browser, and in > > the > > > > web > > > > > browser it says bullet279 is still present and pointing to b13f9. > > > > > > > > > Have I done anything wrong here? > > > > > > > > > "determine_wants() receives a dictionary of refs (refs as keys, the > > SHA1 > > > > > they references as value) and should return a list of heads that you > > > > would > > > > > like to see fetched." > > > > > > > > > In the case of send_pack, does this change to "a list of heads that > > you > > > > > would like to see written"? > > > > > > > > > Thanks. > > > > > > > > > (Jelmer, I'm sorry, I originally hit "reply" instead of "reply-all", > > so > > > > you > > > > > will get two copies of this message.) > > > > > > > > > On Sat, Jul 7, 2012 at 4:31 AM, Jelmer Vernooij <jel...@samba.org> > > > > wrote: > > > > > > > > > > On Thu, Jul 05, 2012 at 12:51:09AM -0700, Andi McClure wrote: > > > > > > > Thanks! What I'm personally trying to do is interface with > > Github. I > > > > > > don't > > > > > > > know what extensions they do or don't support. > > > > > > They support delete-refs. > > > > > > > > > > > So, I'm not sure I understand what to do here. ZERO_SHA1 is just > > 160 > > > > bits > > > > > > > of 0x00, right? I tried this and it didn't seem to work. What I'm > > > > doing > > > > > > is > > > > > > > again, the "changed" function which is my second argument to > > > > send_pack, > > > > > > > when this function is called it gets in a mapping of all git > > > > repository > > > > > > > heads to their corresponding git shas, I change the value > > associated > > > > with > > > > > > > (for example) "refs/heads/bullet279" in this dictionary to > > ("00"*20) > > > > and > > > > > > > return the modified dictionary. As far as I can tell (?) > > returning > > > > this > > > > > > > dictionary rewrites what all the ref names point to during the > > > > pack-- the > > > > > > > only point of the generate_pack_contents on the other hand seems > > (?) > > > > to > > > > > > be > > > > > > > to return object files given a sha (the shas returned by > > > > > > > determine_wants??). However when I set one of these to the > > > > > > 20-bytes-of-zero > > > > > > > hash and return it, it appears to have no effect on the Github > > side. > > > > I > > > > > > > don't get an error or anything, just... nothing happens. Any idea > > > > what I > > > > > > > might be doing wrong, or how to debug this further? I'm using > > Dulwich > > > > > > 0.8.2. > > > > > > What do you mean exactly when you say it doesn't work? The ref > > doesn't > > > > > > go away, or it simply gets its value set to the ZERO_SHA1? > > > > > > > > > > > By the way, there might be room for improvement in the > > documentation > > > > > > here. > > > > > > > If you look at the GitClient documentation: > > > > > > > > > > > > > > > > http://www.samba.org/~jelmer/dulwich/apidocs/dulwich.client.GitClient.html#send_pack > > > > > > > send_pack does not have its "determine_wants" argument > > documented at > > > > all. > > > > > > > The fetch() method documents an argument by that name with > > "Optional > > > > > > > function to determine what refs to fetch", but this doesn't seem > > to > > > > > > answer > > > > > > > questions like, what are the arguments to this optional function > > and > > > > what > > > > > > > does it return? The description of the generate_pack_contents > > > > argument of > > > > > > > send_pack is similarly vague in terms of what this function > > takes as > > > > an > > > > > > > argument and what it returns (I'm afraid I'm misunderstanding > > what it > > > > > > does > > > > > > > completely...). > > > > > > determine_wants() receives a dictionary of refs (refs as keys, the > > > > > > SHA1 they references as value) and should return a list of heads > > that > > > > > > you would like to see fetched. > > > > > > > > > > Patches to improve the documentation are very much welcome. > > > > > > > > > > Cheers, > > > > > > > > > > Jelmer > > > > > > > > > > > > > > > On Wed, Jul 4, 2012 at 3:34 PM, Jelmer Vernooij < > > jel...@samba.org> > > > > > > wrote: > > > > > > > > > > > > On Tue, Jul 03, 2012 at 11:04:30PM -0700, Andi McClure wrote: > > > > > > > > > Hello, I am trying to patch the "hg-git" project, which is > > based > > > > on > > > > > > > > > Dulwich. I do not understand very much about what I am doing > > and > > > > am > > > > > > > > mostly > > > > > > > > > working by studying the surrounding code. What I am trying > > to do > > > > is > > > > > > add a > > > > > > > > > feature where I delete a remote branch by name. > > > > > > > > > > > > > As far as I can tell, the way I'm meant to communicate with > > the > > > > > > server is > > > > > > > > > to call > > > > > > > > > client.send_pack(path, changed, genpack) > > > > > > > > > where client is a GitClient object, path is the appropriate > > git > > > > path, > > > > > > > > > "changed" is a method taking a dictionary of branch-name > > > > > > > > > ->branch-points-to-rev and returning a new dictionary > > describing > > > > > > same, > > > > > > > > and > > > > > > > > > "genpack" is > > > > > > dulwich.object_store.BaseObjectStore.generate_pack_contents. > > > > > > > > > The existing hg-git code uses this interface to upload > > objects > > > > and > > > > > > assign > > > > > > > > > refs. > > > > > > > > > > > > > How can I use the GitClient object to *delete* a ref, though? > > > > > > > > You should be able to do this by setting a particular ref to > > the > > > > > > > > ZERO_SHA1. > > > > > > > > > > > > Note that the server has to support the 'delete-refs' > > capability > > > > for > > > > > > > > this to work. > > > > > > > > > > > > Cheers, > > > > > > > > > > > > Jelmer > > > > > > > > > > > > > > > > -- > > > > > > _______________________________________________ Mailing list: https://launchpad.net/~dulwich-users Post to : dulwich-users@lists.launchpad.net Unsubscribe : https://launchpad.net/~dulwich-users More help : https://help.launchpad.net/ListHelp
