Differential D8618

DirectoryLoader: Check nar hashes when provided
ClosedPublic
Actions

Authored by ardumont on Oct 4 2022, 7:33 PM.

Details

Reviewers

vlorentz

Group Reviewers

Reviewers

Maniphest Tasks

T3781: Replace the Nixguix loader with a lister

Commits

rDLDBASE4d51ad991d31: DirectoryLoader: Check nar hashes when provided

Summary

The lister now provides the "checksums_computation". This is either "standard" (for most
cases as in bare checksums on the object retrieved) or "nar" for some edge case. In that
case the computation is delegated to the "nix-store" command (which should be present in
the system running the loading).

This adapts the directory loader to deal with this case.

No work has been done for the ContentLoader yet besides failing the case if a call
happens with such case.

Related to T3781
Depends on D8627

Diff Detail

Repository

rDLDBASE Generic VCS/Package Loader

Branch

master

Lint

No Linters Available

Unit

No Unit Test Coverage

Build Status

Buildable 32089
Build 50241: Phabricator diff pipeline on jenkins	Jenkins console · Jenkins
Build 50240: arc lint + arc unit

Event Timeline

ardumont created this revision.Oct 4 2022, 7:33 PM

Herald added a reviewer: Reviewers. · View Herald TranscriptOct 4 2022, 7:33 PM

Build is green

Patch application report for D8618 (id=31120)

Rebasing onto b26b98810d...

Current branch diff-target is up to date.

Changes applied before test

commit f8c035cd8b48ca938a2f3fd4f9d53aeea2489abb
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Tue Oct 4 19:32:01 2022 +0200

    {Cnt|Dir}Loader: Check nar hashes when nix-store is available
    
    The lister nows provides the "checksums_computation" which is either "standard" (for
    most cases) and "nar" for some edge case.
    
    This adapts the content and directory loader to deal with those.
    
    Related to T3781

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

Harbormaster completed remote builds in B32089: Diff 31120.Oct 4 2022, 7:37 PM

ardumont requested review of this revision.Oct 4 2022, 7:37 PM

ardumont mentioned this in T3781: Replace the Nixguix loader with a lister.Oct 5 2022, 10:06 AM

vlorentz requested changes to this revision.Oct 5 2022, 10:39 AM

vlorentz added a subscriber: vlorentz.

vlorentz added inline comments.

swh/loader/core/loader.py
674	You need to add this to the docstring, along with the meaning of each value.
853–870	pathlib ftw :)
swh/loader/core/tests/test_loader.py
774	Please add it to Jenkins before landing this diff
swh/loader/core/utils.py
134	To make the above change work

This revision now requires changes to proceed.Oct 5 2022, 10:39 AM

ardumont added inline comments.Oct 5 2022, 10:54 AM

swh/loader/core/tests/test_loader.py
774	that's more complicated than this, i think jenkins runs buster?

ardumont edited the summary of this revision. (Show Details)Oct 5 2022, 12:04 PM

ardumont added a parent revision: D8627: base-buster/Dockerfile: Install nix binaries in buster image.

ardumont added a subscriber: anlambert.

ardumont added inline comments.

swh/loader/core/loader.py
750	I still need to check this happens for real for file...
swh/loader/core/tests/test_loader.py
774	@anlambert updated jenkins with D8627 \o/

ardumont marked an inline comment as done.Oct 5 2022, 12:19 PM

ardumont added inline comments.

swh/loader/core/loader.py
750	I'm gonna drop it for now and run with an assertion on checksum_computations == standard and see what happens in docker.

ardumont marked 3 inline comments as done.Oct 5 2022, 12:50 PM

ardumont added inline comments.

swh/loader/core/loader.py
853–870	\m/

ardumont retitled this revision from {Cnt|Dir}Loader: Check nar hashes when nix-store is available to DirectoryLoader: Check nar hashes when provided.Oct 5 2022, 12:57 PM

ardumont edited the summary of this revision. (Show Details)

Adapt according to review and add missing coverage

Build is green

Patch application report for D8618 (id=31146)

Rebasing onto b26b98810d...

Current branch diff-target is up to date.

Changes applied before test

commit 4d51ad991d3180da683d3f4a9076cd7bb7f54a97
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Tue Oct 4 19:32:01 2022 +0200

    DirectoryLoader: Check nar hashes when provided
    
    The lister now provides the "checksums_computation". This is either "standard" (for most
    cases as in bare checksums on the object retrieved) or "nar" for some edge case. In that
    case the computation is delegated to the "nix-store" command (which should be present in
    the system running the loading).
    
    This adapts the directory loader to deal with this case.
    
    No work has been done for the ContentLoader yet besides failing the case if a call
    happens with such case.
    
    Related to T3781

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

Harbormaster completed remote builds in B32112: Diff 31146.Oct 5 2022, 1:08 PM

vlorentz accepted this revision.Oct 5 2022, 1:08 PM

This revision is now accepted and ready to land.Oct 5 2022, 1:08 PM

ardumont added a child revision: D8630: {Cnt|Dir}Loader: Fix standard/nar hash mismatch behavior to fail loading.Oct 5 2022, 2:48 PM

Closed by commit rDLDBASE4d51ad991d31: DirectoryLoader: Check nar hashes when provided (authored by ardumont). · Explain WhyOct 5 2022, 4:35 PM

This revision was automatically updated to reflect the committed changes.

ardumont added a commit: rDLDBASE4d51ad991d31: DirectoryLoader: Check nar hashes when provided.