Page MenuHomeSoftware Heritage

Add a default target for symbolic reference branches
ClosedPublic

Authored by olasd on May 28 2020, 11:02 AM.

Details

Summary

Some repositories are wrongly configured and they have symbolic references (e.g.
a HEAD) that points to a non-existent branch. In the general case, we still want
to load these origins; Create a dangling branch for these symbolic references to
point to.

Test Plan

new tox test added

Diff Detail

Repository
rDLDG Git loader
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

olasd created this revision.May 28 2020, 11:02 AM

Build is green

Patch application report for D3188 (id=11330)

Rebasing onto dfd5be7b14...

Current branch diff-target is up to date.
Changes applied before test
commit 0c31a06b94f6f7cccd1db85317afce2032229461
Author: Nicolas Dandrimont <nicolas@dandrimont.eu>
Date:   Thu May 28 10:31:14 2020 +0200

    Add a default target for symbolic reference branches
    
    Some repositories are wrongly configured and they have symbolic references (e.g.
    a HEAD) that points to a non-existent branch. In the general case, we still want
    to load these origins; Create a dangling branch for these symbolic references to
    point to.

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

vlorentz requested changes to this revision.May 28 2020, 11:27 AM
vlorentz added a subscriber: vlorentz.

LGTM, but I think it's missing some tests:

This revision now requires changes to proceed.May 28 2020, 11:27 AM

LGTM, but I think it's missing some tests:

Yeah, I know of the caplog fixture, thanks. I will not be adding pytest fixtures in the mess of unittest class inheritance that these tests currently are.

  • Checking there is no warning if we first get a symref from A to B, then a reference from B to C.

As we don't really control what comes "first" (dulwich is reading the symbolic refs, not us) I don't think this (very) edge case can really be tested for reliably. I don't even know if chains of symbolic references exist in practice.

ardumont added inline comments.
swh/loader/git/utils.py
1

I am "the pre-commit hook" for copyright headers (I missed those yesterday... ;)
Can you please update those (we do for other repos ;)?

olasd updated this revision to Diff 11335.May 28 2020, 12:00 PM

Update copyright years

Build is green

Patch application report for D3188 (id=11335)

Rebasing onto dfd5be7b14...

Current branch diff-target is up to date.
Changes applied before test
commit ee22325a193dbc2b52b37496f3374c6bf0c1d1e1
Author: Nicolas Dandrimont <nicolas@dandrimont.eu>
Date:   Thu May 28 10:31:14 2020 +0200

    Add a default target for symbolic reference branches
    
    Some repositories are wrongly configured and they have symbolic references (e.g.
    a HEAD) that points to a non-existent branch. In the general case, we still want
    to load these origins; Create a dangling branch for these symbolic references to
    point to.

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

ardumont added inline comments.May 28 2020, 12:43 PM
swh/loader/git/utils.py
1

I got retired... maybe? \m/ D3191

ardumont accepted this revision.May 28 2020, 1:06 PM
vlorentz accepted this revision.May 28 2020, 4:14 PM
This revision is now accepted and ready to land.May 28 2020, 4:14 PM
This revision was automatically updated to reflect the committed changes.