Repository navigation
fix(import,update): stop writing .dvc metadata that disagrees with dvc (0.24.0) - #185
Merged
Merged
Conversation
…c (0.24.0) Four defects from #182, all in the metadata dt records for repo imports. outs.size: 0 on multi-GB imports. get_file_size_from_cache built only the v3 object path, so a half-migrated remote -- v2 objects at <xx>/<rest> alongside v3 ones -- reported no size for every v2 resident, and the caller folded each miss into the total as zero. Checkout of the same objects worked throughout, because that path already falls back to v2, which is why the data was right and only the size was wrong. Sizing now goes through a shared cache_ops.object_size (both layouts, source remote then local cache); an object that cannot be found means no size field at all rather than a silent undercount. Verified against visium-raw: the 7 affected imports size to 4.18-10.60 GB, nothing unsized. outs.size left describing the previous hash. `dvc list --size` against a repo URL reports no sizes -- a .dir manifest holds only md5 and relpath, so the only place a size exists is the object -- and a None size was read as "leave it alone", retaining a figure 3x the real directory. Sizes are now totalled by stat'ing the objects, and a size belonging to the old hash is deleted rather than kept. deps.repo.rev left contradicting rev_lock, so the next plain `dvc update` resolved rev and rolled the import back. --rev now records the spec in rev and the resolved 40-char sha in rev_lock (it was writing branch names into rev_lock); without --rev, an import tracking a branch advances to that branch's tip; a pin that contradicts the new lock is dropped, loudly. Rebuilt .dir omitting git-tracked files. DVC's repo filesystem excludes only *.dvc, dvc.yaml, dvc.lock and .dvcignore, so importing a directory that is not itself an out hashes the git-tracked files there -- including the .gitignore DVC generated -- into the payload. dt cannot, so its manifest diverged, and was then published into a shared upstream remote. dt import and dt update now refuse such a path and name the files, pushing nothing. dt init seeds .dvcignore with .gitignore so a repo's own bookkeeping stops leaking into importers' data, and dt doctor flags repos that lack it. With that pattern in place dt import now matches dvc import byte-for-byte on such a directory. Two smaller bugs found in passing: the .dvc files inside a directory of outs were treated as payload candidates (DVC excludes them too), and a directory containing exactly one file was recorded as a single-file import, losing its .dir and nfiles. Co-Authored-By: Claude <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes the four defects in #182 — the three in the report plus the
size: 0one in the comment.What was wrong
outs.size: 0on fresh multi-GB imports.get_file_size_from_cachebuilt only the v3 object path./g/data/a56/dvc/registries/visium-rawis mixed-layout, and the affected imports' leaf objects live only at the legacy v2 path<xx>/<rest>— so every lookup missed andif file_size:folded each miss into the total as zero. Checkout of the same objects worked throughout, because that path already falls back to v2, which is why the data was correct and only the size was wrong.outs.sizeleft describing the previous hash.dvc list --sizeagainst a repo URL reports no sizes — a.dirmanifest holds only md5 and relpath, so the only place a size exists is the object itself — and aNonesize was read as "leave it alone", retaining a figure 3× the real directory.deps.repo.revleft contradictingrev_lock, so the next plaindvc updateresolvedrevand rolled the import backwards. Also found:--rev <branch>was writing the branch name intorev_lock, which is meant to hold the resolved commit.Rebuilt
.diromitting git-tracked files, then publishing that divergent manifest into a shared upstream remote. DVC's repo filesystem excludes only*.dvc,dvc.yaml,dvc.lockand.dvcignore, so importing a directory that is not itself an out hashes the git-tracked files there — including the.gitignoreDVC generated — into the payload. dt works from hashes alone and cannot.What changed
cache_ops.object_size: both layouts, source remote then local cache. An object that cannot be found means nosizefield rather than a silent undercount, and asizebelonging to a superseded hash is deleted rather than kept.--rev <spec>recordsrev: <spec>andrev_lock: <resolved 40-char sha>. Without--rev, an import tracking a branch advances to that branch's tip, not to whatever the tmp clone has checked out. A pin contradicting the new lock is dropped, loudly.dt importanddt updaterefuse a path that is not a single out, name the offending files, and push nothing.dt importpreviously died there with a bareAttributeError.dt initseeds.dvcignorewith.gitignoreso a repo's own bookkeeping stops leaking into importers' data;dt doctorflags repos that lack it.Two smaller bugs fixed in passing: the
.dvcfiles inside a directory of outs were treated as payload candidates (DVC excludes them too), and a directory containing exactly one file was recorded as a single-file import, losing its.dirandnfiles.Verification
Beyond unit tests (2352 passed, 1 skipped), each fix was driven end-to-end:
visium-rawregistry, the 7 affected imports now size to 4.18–10.60 GB with nothing unsized, where the released code wrote0.dt updateand a realdvc updatenow agree exactly:md5=0caa660ad774…dir size=101 nfiles=5 rev_lock=f8ad1237349bfrom both..dvcignoreseeded,dt importanddvc importof a non-out directory produce identical manifest hash,nfiles,sizeand entry list..dvcfile byte-identical and plants nothing in the source remote.SciLife_FFPE_lymph_nodes/3946_1F_LN/fastq, its parent,SciLife_FFPE_batch6): all fully hashed, so the new guard does not disturb them.The
.gitignorebehaviour was established by sandbox demo rather than inferred — DVC authors the file, importing the parent sweeps it in, hashing is raw md5 with no dos2unix, and upstream.dvcignoresuppresses it with no side effects. Also worth knowing:dvc pushcannot upload a repo import at all (stage_filterindvc/repo/fetch.pyexcludes repo-import stages), so an imported directory is a permanent live dependency on the upstream repo.🤖 Generated with Claude Code