1115188 - firefoxtree should recognised fxheads from shared repositories

Status: RESOLVED FIXED

(Developer Services :: Mercurial: firefoxtree, defect)

Description

[Dave Townsend [:mossop] (PTO until Aug 21st)]

If I have a unified repo "foo" where firefoxtree knows all the fxheads and I "hg share foo bar" then firefoxtree should also know the fxheads when in "bar". It does not though.

Comment 1

[Gregory Szorc [:gps]]

This is definitely a bug.

I could hack this into submission. But, the solution would be a bit fragile and may break other extensions.

Mercurial 3.3 is introducing a proper API for creating names attached to commits. It is technically superior to the localtags foo I'm building on top of now.

I'm inclined to ignore this bug until firefoxtree is ported to the Mercurial 3.3 API. Mercurial 3.3 should be released on Feb 1.

For now, hack around it by running `hg pull` in every shared clone.

Comment 2

[aleth [:aleth]]

(In reply to Gregory Szorc [:gps] from comment #1)
> I'm inclined to ignore this bug until firefoxtree is ported to the Mercurial
> 3.3 API. Mercurial 3.3 should be released on Feb 1.

I wonder if this is actionable now?

Comment 3

[Gregory Szorc [:gps]]

It is actionable. I have an incomplete patch series locally. 

Also, ~20% of our staff are still on 3.2 or older, according to the developer survey from a few weeks ago. While we try to maintain compatibility with the previous 4 released Mercurial versions, I'm fine with dropping 3.2 for firefoxtree now. (We'll drop 3.2 after 3.6 is released Nov 1 anyway.)

I almost certainly won't have time to look at this for the next few days - finishing up end of quarter foo.

Comment 5

[Gregory Szorc [:gps]]

Attachment: MozReview Request: firefoxtree: store firefox tree labels in a dedicated file; r?smacleod

firefoxtree: store firefox tree labels in a dedicated file; r?smacleod

Mercurial 3.3 has a "namespaces" API that allows namespaced labels to be
attached to Mercurial changesets. This API is a better version of the
"local tags" API, which firefoxtree is currently using. This patch
starts the series of converting firefoxtree to use the namespaces API.

We start the process by creating a standalone file that holds firefox
tree node mappings. The namespaces API doesn't do I/O for us, so we
have to roll some minimal code.

The .hg/firefoxtrees file is now written and read from when the
repository is loaded. It is currently a shadow copy of tags being
written to local tags. We will eventually transition off local tags
towards this new file.

Comment 6

[Gregory Szorc [:gps]]

Attachment: MozReview Request: firefoxtree: obtain firefox tree tags from the firefoxtrees file; r?smacleod

firefoxtree: obtain firefox tree tags from the firefoxtrees file; r?smacleod

get_firefoxtrees() is our internal API for iterating over Firefox tree
labels stored in the repository. It is currently getting information
from localtags. Since we are transitioning away from local tags, grab
the information from the local file instead.

This change will result in apparent data loss until the next `hg pull`.
This regression will be addressed in subsequent changes.

Comment 7

[Gregory Szorc [:gps]]

Attachment: MozReview Request: firefoxtree: define fxtrees namespace; r?smacleod

firefoxtree: define fxtrees namespace; r?smacleod

Comment 8

[Gregory Szorc [:gps]]

Attachment: MozReview Request: firefoxtree: update firefoxtrees file when processing wire protocol command; r?smacleod

firefoxtree: update firefoxtrees file when processing wire protocol command; r?smacleod

Previously, we didn't update the refs when the wire protocol command was
processed. Change that.

This does result in redundant output in `hg log`. This will be fixed in
a subsequent patch.

Comment 9

[Gregory Szorc [:gps]]

Attachment: MozReview Request: firefoxtree: read old tags using internal API; r?smacleod

firefoxtree: read old tags using internal API; r?smacleod

When processing tree nodes via the wire protocol command, use the values
from the firefoxtrees file instead of from the local tags file. This
reinforces the firefoxtrees file being the canonical location of this
data.

Comment 10

[Gregory Szorc [:gps]]

Attachment: MozReview Request: firefoxtree: move localtags sanitization into writefirefoxtrees; r?smacleod

firefoxtree: move localtags sanitization into writefirefoxtrees; r?smacleod

We have a function for writing firefoxtrees data. This code should live
there.

Comment 11

[Gregory Szorc [:gps]]

Attachment: MozReview Request: firefoxtree: implement `hg fxheads` using new APIs; r?smacleod

firefoxtree: implement `hg fxheads` using new APIs; r?smacleod

Previously, we were printing tags. This was a holdover from the days
before {fxheads} was available. It has the unfortunate side-effect of
including non-tree tags in the output.

Change the template to use {fxheads} so only Firefox tree labels are
shown.

As part of this, limit to one output per unique node. Otherwise we will
print the same changeset twice if it has multiple fxtree labels defined.

Comment 12

[Gregory Szorc [:gps]]

Attachment: MozReview Request: firefoxtree: don't test contents of localtags file; r?smacleod

firefoxtree: don't test contents of localtags file; r?smacleod

We're not testing localtags anywhere else. Remove the test.

This paves the way for easier removal of tag writing from the extension.

Comment 13

[Gregory Szorc [:gps]]

Attachment: MozReview Request: firefoxtree: don't write local tags; r?smacleod

firefoxtree: don't write local tags; r?smacleod

The namespaces API is the new preferred way to record topical labels.
Stop writing local tags for firefoxtree labels.

As the test changes demonstrate, our redundant labels in `hg log` are
now gone.

Comment 14

[Gregory Szorc [:gps]]

Attachment: MozReview Request: firefoxtree: prune entries from localtags file; r?smacleod

firefoxtree: prune entries from localtags file; r?smacleod

We no longer write to this file. We need to prune existing entries from
it or they may interfere with existing use.

Comment 15

[Gregory Szorc [:gps]]

Attachment: MozReview Request: firefoxtree: automatically share tree labels with `hg share` (bug 1115188); r?smacleod

firefoxtree: automatically share tree labels with `hg share` (bug 1115188); r?smacleod

Repositories created with `hg share` need to obtain certain repository
data from the shared source repo, otherwise each share has its own
state. For firefoxtree, it makes sense for the tree labels to be shared
across all shared repos. This commit implements that.

Comment 16

[Steven MacLeod [:smacleod]]

Comment on attachment 8697861 [details]
MozReview Request: firefoxtree: store firefox tree labels in a dedicated file; r?smacleod

https://reviewboard.mozilla.org/r/27729/#review25253

Comment 17

[Steven MacLeod [:smacleod]]

Comment on attachment 8697862 [details]
MozReview Request: firefoxtree: obtain firefox tree tags from the firefoxtrees file; r?smacleod

https://reviewboard.mozilla.org/r/27731/#review25255

Comment 18

[Steven MacLeod [:smacleod]]

Comment on attachment 8697863 [details]
MozReview Request: firefoxtree: define fxtrees namespace; r?smacleod

https://reviewboard.mozilla.org/r/27733/#review25257

::: hgext/firefoxtree/__init__.py:481
(Diff revision 1)
> +    def listnames(r):
> +        return r.firefoxtrees.keys()
> +    def namemap(r, name):
> +        node = r.firefoxtrees.get(name)
> +        if node:
> +            return [node]
> +        return []
> +    def nodemap(r, node):

blank between these please

::: hgext/firefoxtree/__init__.py:492
(Diff revision 1)
> +    n = namespaces.namespace('fxtrees', 'fxtree',

I'm not a fan of relying on kwarg position (`templatename`, in this case). Can we please call it out explicitly `templatename='fxtree'`?

Comment 19

[Steven MacLeod [:smacleod]]

Comment on attachment 8697864 [details]
MozReview Request: firefoxtree: update firefoxtrees file when processing wire protocol command; r?smacleod

https://reviewboard.mozilla.org/r/27735/#review25259

Comment 20

[Steven MacLeod [:smacleod]]

Comment on attachment 8697865 [details]
MozReview Request: firefoxtree: read old tags using internal API; r?smacleod

https://reviewboard.mozilla.org/r/27737/#review25261

Comment 21

[Steven MacLeod [:smacleod]]

Comment on attachment 8697866 [details]
MozReview Request: firefoxtree: move localtags sanitization into writefirefoxtrees; r?smacleod

https://reviewboard.mozilla.org/r/27739/#review25263

Comment 22

[Steven MacLeod [:smacleod]]

Comment on attachment 8697867 [details]
MozReview Request: firefoxtree: implement `hg fxheads` using new APIs; r?smacleod

https://reviewboard.mozilla.org/r/27741/#review25265

Comment 23

[Steven MacLeod [:smacleod]]

Comment on attachment 8697868 [details]
MozReview Request: firefoxtree: don't test contents of localtags file; r?smacleod

https://reviewboard.mozilla.org/r/27743/#review25267

Comment 24

[Steven MacLeod [:smacleod]]

Comment on attachment 8697869 [details]
MozReview Request: firefoxtree: don't write local tags; r?smacleod

https://reviewboard.mozilla.org/r/27745/#review25269

Comment 25

[Steven MacLeod [:smacleod]]

Comment on attachment 8697870 [details]
MozReview Request: firefoxtree: prune entries from localtags file; r?smacleod

https://reviewboard.mozilla.org/r/27747/#review25271

::: hgext/firefoxtree/__init__.py:196
(Diff revision 1)
> +        tree, uri = resolve_trees_to_uris([tag])[0]

Would it be worth migrating anything with a uri to the new namespace while clearing it? Feel free to drop.

Comment 26

[Gregory Szorc [:gps]]

https://reviewboard.mozilla.org/r/27747/#review25271

> Would it be worth migrating anything with a uri to the new namespace while clearing it? Feel free to drop.

I feel the most robust way to do this is to just delete the old data and have people re-pull to repopulate the data. I don't want to open the can of worms.

Comment 27

[Steven MacLeod [:smacleod]]

Comment on attachment 8697871 [details]
MozReview Request: firefoxtree: automatically share tree labels with `hg share` (bug 1115188); r?smacleod

https://reviewboard.mozilla.org/r/27749/#review25275

Would it be worth adding a simple command to mark an already shared repository clone as sharing firefoxtrees? or should users just manually update the share file?

::: hgext/firefoxtree/__init__.py:147
(Diff revision 1)
> +def share(orig, ui, source, *args, **kwargs):

it's a shame you have to copy so much boilerplate for this :/

Comment 28

[Gregory Szorc [:gps]]

https://reviewboard.mozilla.org/r/27749/#review25275

> it's a shame you have to copy so much boilerplate for this :/

There will be a hook point in 3.7 to make this much easier :D

Comment 29

[Gregory Szorc [:gps]]

https://reviewboard.mozilla.org/r/27749/#review25275

Follow-up if people ask for it.

Thanks for the reviews!

Comment 30

[Gregory Szorc [:gps]]

https://hg.mozilla.org/hgcustom/version-control-tools/rev/0ad688fdd138d2e19b09c2e653532b7bb5224d70
firefoxtree: automatically share tree labels with `hg share` (bug 1115188); r=smacleod

Comment 31

[Gregory Szorc [:gps]]

And this whole series landed!

The sharing will only be present on new shares. If you want to play with fire, add a "firefoxtrees" line to the .hg/shared file of each repo created from the share to retroactively add sharing.

This series also wipes out all old names/refs/labels: you'll need to `hg pull` to recreate the labels.