loader: don't ignore closed branches
ClosedPublic
Actions

Authored by Alphare on May 27 2021, 12:53 PM.

Details

Reviewers

ardumont

Group Reviewers

Reviewers

Commits

rDLDHG44fc0133da71: loader: don't ignore closed branches

Summary

The existing util for getting a repo's branches skips closed branches
but did not leave any explanation why, either in the code or in the
commit message. I cannot think of a good reason for ignoring closed
branches, so we're removing this exception, which in turn fixes the
incremental issue detailed in T3336.

This has affected existing tests of the two repositories that had closed
branches. A test for the incremental behavior was added as well.

Diff Detail

Repository

rDLDHG Mercurial loader

Lint

Automatic diff as part of commit; lint not applicable.

Unit

Automatic diff as part of commit; unit tests not applicable.

Event Timeline

Alphare created this revision.May 27 2021, 12:53 PM

Herald added a reviewer: Reviewers. · View Herald TranscriptMay 27 2021, 12:53 PM

Build is green

Patch application report for D5790 (id=20694)

Rebasing onto cce9ec940e...

Current branch diff-target is up to date.

Changes applied before test

commit 661a4a9193136d3f2f661d9289a6dc0b69bc7862
Author: Raphaël Gomès <rgomes@octobus.net>
Date:   Thu May 27 11:49:57 2021 +0200

    loader: don't ignore closed branches
    
    The existing util for getting a repo's branches skips closed branches
    but did not leave any explanation why, either in the code or in the
    commit message. I cannot think of a good reason for ignoring closed
    branches, so we're removing this exception, which in turn fixes the
    incremental issue detailed in T3336.
    
    This has affected existing tests of the two repositories that had closed
    branches. A test for the incremental behavior was added as well.

See https://jenkins.softwareheritage.org/job/DLDHG/job/tests-on-diff/241/ for more details.

Harbormaster completed remote builds in B21656: Diff 20694.May 27 2021, 12:58 PM

Alphare requested review of this revision.May 27 2021, 12:58 PM

lgtm.

Although, i don't really know what's a closed branch and the
commit page does not say much [1]

Branch, closed
A named branch whose branch heads have all been closed.

(oooh yeah, that's clearer, or not ;)

I guess it's some kind of read-only branch. Once marked
closed by the hg commit cli, we can't push any more
on it, right?

[1] https://www.selenic.com/mercurial/hg.1.html#commit

swh/loader/mercurial/tests/test_from_disk.py
393–424	So as to avoid a bit repetition and explicit the changes in between ;)

ardumont accepted this revision.May 27 2021, 1:55 PM

This revision is now accepted and ready to land.May 27 2021, 1:55 PM

In D5790#147332, @ardumont wrote:

I guess it's some kind of read-only branch. Once marked
closed by the hg commit cli, we can't push any more
on it, right?

That's correct, closing a branch is a means of signifying that it's not to be used again, Mercurial preventing you from doing anything to a closed branch.

EDIT: I was wrong, you can totally re-open closed branches. The underlying problem is actually fixed in D5793, see T3336 for more details.

Refactor expected stats in new test

Build is green

Patch application report for D5790 (id=20697)

Rebasing onto cce9ec940e...

Current branch diff-target is up to date.

Changes applied before test

commit 44fc0133da716d4690419dd91f2da7801f7b6b8a
Author: Raphaël Gomès <rgomes@octobus.net>
Date:   Thu May 27 11:49:57 2021 +0200

    loader: don't ignore closed branches
    
    The existing util for getting a repo's branches skips closed branches
    but did not leave any explanation why, either in the code or in the
    commit message. I cannot think of a good reason for ignoring closed
    branches, so we're removing this exception, which in turn fixes the
    incremental issue detailed in T3336.
    
    This has affected existing tests of the two repositories that had closed
    branches. A test for the incremental behavior was added as well.

See https://jenkins.softwareheritage.org/job/DLDHG/job/tests-on-diff/242/ for more details.

Harbormaster completed remote builds in B21659: Diff 20697.May 27 2021, 2:48 PM

Closed by commit rDLDHG44fc0133da71: loader: don't ignore closed branches (authored by Alphare). · Explain WhyMay 27 2021, 2:51 PM

This revision was automatically updated to reflect the committed changes.

Alphare added a commit: rDLDHG44fc0133da71: loader: don't ignore closed branches.

Alphare mentioned this in T3336: Deploy swh.loader.mercurial 2.1 in staging.May 27 2021, 10:58 PM