Page MenuHomeSoftware Heritage

Add support for symbolic references
ClosedPublic

Authored by olasd on Oct 7 2019, 11:35 AM.

Details

Summary

This makes the symbolic references sent from upstream branch aliases in
snapshots, instead of pointing to the target.

Test Plan

tox

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

Looks good, but I'm puzzled by some testing results.

The logic of extracting the branches for snapshot computation has trickled down to swh identify. (BTW, if you're up for some refactoring, this diff might be a chance to avoid the duplication, but it's certainly not a blocker here.)

So I've tried to just reuse your change in swh.model.cli too, here's the minimal diff: D2081.

Then I've dumped the branch list just before it's hashed (with P542 in swh.model.identifiers) and what I get is this:

alias HEAD 726566732f68656164732f6d6173746572
revision refs/heads/master 9768d0b576dbaaecd80abedad6dfd0d72f1476da
revision refs/remotes/origin/master c3c588713233609f5bbbb2d9e7f3fb4a660f3f72

where HEAD is an alias, but it's pointing to an object ID (this is on the sample git repo used for tests on swh-load-git, but I obtain similar results on any other repo I've tried).

What am I missing here?

I think your code is fine, but we want compatibility with what swh.model.identifiers does, and I don't understand why it's not the case with my proposed trivial change.

In D2078#48207, @zack wrote:

Looks good, but I'm puzzled by some testing results.

The logic of extracting the branches for snapshot computation has trickled down to swh identify. (BTW, if you're up for some refactoring, this diff might be a chance to avoid the duplication, but it's certainly not a blocker here.)

So I've tried to just reuse your change in swh.model.cli too, here's the minimal diff: D2078.

Then I've dumped the branch list just before it's hashed (with P542 in swh.model.identifiers) and what I get is this:

alias HEAD 726566732f68656164732f6d6173746572
revision refs/heads/master 9768d0b576dbaaecd80abedad6dfd0d72f1476da
revision refs/remotes/origin/master c3c588713233609f5bbbb2d9e7f3fb4a660f3f72

where HEAD is an alias, but it's pointing to an object ID (this is on the sample git repo used for tests on swh-load-git, but I obtain similar results on any other repo I've tried).

What am I missing here?

That's not an object id, that's a branch name (refs/heads/master, in ASCII).

I think your code is fine, but we want compatibility with what swh.model.identifiers does, and I don't understand why it's not the case with my proposed trivial change.

This revision is now accepted and ready to land.Oct 15 2019, 4:57 PM

Rebase dance party (>'-')> <('-'<) ^('-')^ v('-')v(>'-')> (^-^)

This revision was automatically updated to reflect the committed changes.
In D2078#49823, @olasd wrote:
In D2078#48207, @zack wrote:

Then I've dumped the branch list just before it's hashed (with P542 in swh.model.identifiers) and what I get is this:

alias HEAD 726566732f68656164732f6d6173746572
revision refs/heads/master 9768d0b576dbaaecd80abedad6dfd0d72f1476da
revision refs/remotes/origin/master c3c588713233609f5bbbb2d9e7f3fb4a660f3f72

where HEAD is an alias, but it's pointing to an object ID (this is on the sample git repo used for tests on swh-load-git, but I obtain similar results on any other repo I've tried).

What am I missing here?

That's not an object id, that's a branch name (refs/heads/master, in ASCII).

(this was a hilarious bug, nice catch !)