Page MenuHomeSoftware Heritage

loader: don't ignore closed branches
ClosedPublic

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

Details

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

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.

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 ;)

This revision is now accepted and ready to land.May 27 2021, 1:55 PM
Alphare marked an inline comment as done.EditedMay 27 2021, 2:44 PM

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.

This revision was automatically updated to reflect the committed changes.