This is an archive of the discontinued Mercurial Phabricator instance.

remotefilelog: update for core changegroup packing changes

Authored by durin42 on Sep 13 2018, 3:02 PM.



This allows

cd tests && python $CREW/tests/ -j7 --extra-config-opt devel.all-warnings=no --blacklist=<(ls test*lfs*.t test*fastanno*.t test*treeman*.t) test*remotefilelog*.t

to pass against core revision 623081f2abc2, which is fairly recent. I
think that's the entire subset of remotefilelog functionality that we
need to worry about for Google, but the disabling of the
test*treemanifest*.t tests worries me a little, so I might be back
soon with more fixes.

There's some probably-wrong hacks in here, but it seems to work.

Diff Detail

rFBHGX Facebook Mercurial Extensions
Automatic diff as part of commit; lint not applicable.
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

durin42 created this revision.Sep 13 2018, 3:02 PM
Herald added a reviewer: Restricted Project. · View Herald TranscriptSep 13 2018, 3:02 PM
quark requested changes to this revision.Sep 13 2018, 3:30 PM
quark added a subscriber: quark.
quark added inline comments.

Remotefilelog does not have rev all over the place. See:

def rev(self, node):
    # This is a hack to make TortoiseHG work.
    return node

Maybe raising NotImplementedError is safer, if it's not used.


It does not have a return. Does it actually get used somewhere? Maybe raising NotImplementedError is safer to capture surprises?


Maybe just make it return True for most cases:

return not repo.ui.configbool('treemanifest', 'treeonly')

Need removal.


I think most of the remaining code is not prepared for an empty linknode (including storage). Maybe special case it so it can be stored as nullid and change to ignore nullid linknodes.

This revision now requires changes to proceed.Sep 13 2018, 3:30 PM
durin42 marked an inline comment as done.Sep 13 2018, 4:15 PM
durin42 added inline comments.

It's needed, fortunately by setting _generaldelta=True we end up on a codepath where it's only used as a UUID and not as a sortable.


Someone was at least referencing and calling it. I'm a little surprised the missing return (meaning it returned None) didn't break anything, but I've corrected that issue.


My debugging secrets revealed!


I'm not following - this seems to work and pass tests. I'm not sure how I could fill in the linknodes for a given file log, because remotefilelog has no way of iterating file revisions. Does that impasse make sense?

(Happy to discuss on IRC if you like.)

durin42 marked an inline comment as done.Sep 13 2018, 4:15 PM
durin42 edited the summary of this revision. (Show Details)
durin42 updated this revision to Diff 11020.
quark accepted this revision.Sep 13 2018, 6:12 PM
This revision is now accepted and ready to land.Sep 13 2018, 6:12 PM
This revision was automatically updated to reflect the committed changes.