Differential D4665

snapshot: add support for 'alias' target type
ClosedPublic
Actions

Authored by haltode on Dec 4 2020, 12:29 PM.

Details

Reviewers

olasd

Group Reviewers

Reviewers

Maniphest Tasks

T2840: FUSE: snapshot artifacts fail on "alias" target_type

Commits

rDFUSE77a5bcd6d73d: snapshot: add support for 'alias' target type

Summary

Fix T2840.

Diff Detail

Repository

rDFUSE FUSE virtual file system

Branch

feature/snapshot-support-alias

Lint

No Linters Available

Unit

No Unit Test Coverage

Build Status

Buildable 17744
Build 27429: Phabricator diff pipeline on jenkins	Jenkins console · Jenkins
Build 27428: arc lint + arc unit

Event Timeline

haltode created this revision.Dec 4 2020, 12:29 PM

Herald added a reviewer: Reviewers. · View Herald TranscriptDec 4 2020, 12:29 PM

Build is green

Patch application report for D4665 (id=16553)

Rebasing onto f3363d909f...

Current branch diff-target is up to date.

Changes applied before test

commit 1357dce791027fc9d8fefc1feae624a731702ff3
Author: Thibault Allançon <haltode@gmail.com>
Date:   Fri Dec 4 11:56:17 2020 +0100

    snapshot: add support for 'alias' target type
    
    Fix T2840.

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

Harbormaster completed remote builds in B17737: Diff 16553.Dec 4 2020, 12:32 PM

Hmm. Snapshot branch targets are either an alias, or a pointer to a swhid of any type.

This change will break, for instance, for branch targets that are Release objects (look, e.g., at most tags in a SWH git repository), or Content objects (people use that to store GPG keys), or Directory objects (such as some old tags on linux.git).

Might I suggest only special-casing alias branches to make them *relative symlinks* to their target (within the snapshot directory), and keeping the common case symlink to the archive/ directory for other types?

Rework implementation according to olasd comment.

Build is green

Patch application report for D4665 (id=16561)

Rebasing onto f3363d909f...

Current branch diff-target is up to date.

Changes applied before test

commit 9e8325f7396fcaac4d1213849bfbc846901820d6
Author: Thibault Allançon <haltode@gmail.com>
Date:   Fri Dec 4 11:56:17 2020 +0100

    snapshot: add support for 'alias' target type
    
    Fix T2840.

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

Harbormaster completed remote builds in B17744: Diff 16561.Dec 4 2020, 3:30 PM

This looks probably fine, with one caveat inline.

Maybe you should consider adding some more different object types to your snapshot in test fixtures.

swh/fuse/fs/artifact.py
488	I think this only works if you're not in a subdirectory. You should add a test for a branch "refs/heads/alias" pointing at "refs/heads/master" and see whether that really works.

This revision is now accepted and ready to land.Dec 4 2020, 4:32 PM

Rework prefix calculation using os.path.relpath()
Add custom snapshot data in test to try out multiple special cases

olasd accepted this revision.Dec 7 2020, 9:53 AM

Build is green

Patch application report for D4665 (id=16578)

Rebasing onto f3363d909f...

Current branch diff-target is up to date.

Changes applied before test

commit 77a5bcd6d73d168d05ae5baa5ce9291e36c74df5
Author: Thibault Allançon <haltode@gmail.com>
Date:   Fri Dec 4 11:56:17 2020 +0100

    snapshot: add support for 'alias' target type
    
    Fix T2840.

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

Harbormaster completed remote builds in B17760: Diff 16578.Dec 7 2020, 9:55 AM

Closed by commit rDFUSE77a5bcd6d73d: snapshot: add support for 'alias' target type (authored by haltode). · Explain WhyDec 7 2020, 9:58 AM

This revision was automatically updated to reflect the committed changes.

haltode added a commit: rDFUSE77a5bcd6d73d: snapshot: add support for 'alias' target type.