Repository navigation
Pass over a staged patch that cannot be accepted - #15
Merged
Merged
Conversation
A staged patch names the source file its snapshot belongs to, which is a separate question from where it was staged. A queued snapshot is asked both: the owner holds only what a run sent it, and the scan drops what names a source outside the root. A staged one was asked neither, so a scan offered every patch under the root's obj, however old. Two kinds of those can never be accepted. One naming a source outside the root, which reviewing a directory must not reach into. And one naming a source that has gone, since the literal is located by searching that file, so there is nothing for an accept to rewrite. Both are now passed over, which leaves the staged half asked what the queued half already was. The integration harness is where this repo's own obj filled with the second kind: it copied a scenario's staged files under the harness directory rather than moving them, leaving the originals to name a temp directory that goes when the scenario ends. Maps had it worse, since most scenarios never publish them and so only ever left them behind. Publishing now moves, and the harness takes whatever else names its directory with it, so a run leaves obj as it found it.
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.
A staged patch names the source file its snapshot belongs to, which is a separate question from where it was staged. A queued snapshot is asked both: the owner holds only what a run sent it, and the scan drops what names a source outside the root. A staged one was asked neither, so a scan offered every patch under the root's
obj, however old.Two kinds of those can never be accepted. One naming a source outside the root, which reviewing a directory must not reach into. And one naming a source that has gone, since the literal is located by searching that file, so there is nothing for an accept to rewrite. Both are now passed over, which leaves the staged half asked what the queued half already was.
The integration harness is where this repo's own
objfilled with the second kind: it copied a scenario's staged files under the harness directory rather than moving them, leaving the originals to name a temp directory that goes when the scenario ends. Maps had it worse, since most scenarios never publish them and so only ever left them behind. Publishing now moves, and the harness takes whatever else names its directory with it, so a run leavesobjas it found it.Verify and DiffEngine references go to 33.1.3 and 20.5.0.
Tests
Two finder tests: a staged snapshot outside the root, and one whose source is gone. Each fails when its half of the check is removed.
Some manager tests built their staged list by running the finder over a real temp source, which the root rule now declines, so the staging helper hands the record back directly instead.
Locally this cleared 210 staged files, from 81 patches going back a month, and 808 maps, all naming temp directories that no longer exist. Two full integration runs afterwards left
objempty, which is what confirms the harness half.