Skip to content

Conversation

@derrickstolee
Copy link

The OBJECT_INFO_FOR_PREFETCH bitflag was added to sha1-file.c in 0f4a4fb
(sha1-file: support OBJECT_INFO_FOR_PREFETCH, 2019-03-29) and is used to
prevent the fetch_objects() method when enabled.

However, there is a problem with the current use. The definition of
OBJECT_INFO_FOR_PREFETCH is given by adding 32 to OBJECT_INFO_QUICK. This is
clearly stated above the definition (in a comment) that this is so
OBJECT_INFO_FOR_PREFETCH implies OBJECT_INFO_QUICK. The problem is that using
"flag & OBJECT_INFO_FOR_PREFETCH" means that OBJECT_INFO_QUICK also implies
OBJECT_INFO_FOR_PREFETCH.

Split out the single bit from OBJECT_INFO_FOR_PREFETCH into a new
OBJECT_INFO_SKIP_FETCH_OBJECT as the single bit and keep
OBJECT_INFO_FOR_PREFETCH as the union of two flags. This allows a clearer use
of flag checking while also keeping the implication of OBJECT_INFO_QUICK.

@dscho: this might fix the problems with the VFS for Git functional tests.

@dscho dscho force-pushed the prepare-for-vfs-2.22.0 branch from acbc709 to 5b3b769 Compare May 28, 2019 19:51
Copy link
Member

@dscho dscho left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch!

Maybe you can run the functional tests without this (but with my backwards-compatibility hack to support post-index-changed, still), to see whether the functional tests fail, and then re-run with this PR merged?

@derrickstolee
Copy link
Author

@dscho I already updated microsoft/VFSForGit#1209 to use the new "post-index-change" name and got the tests to pass except the one that this should fix. I'll try creating an installer on this PR and update that PR accordingly.

@dscho
Copy link
Member

dscho commented May 28, 2019

I already updated microsoft/VFSForGit#1209 to use the new "post-index-change" name and got the tests to pass except the one that this should fix.

Never mind, then. Let's merge this here PR?

@derrickstolee
Copy link
Author

@dscho it looks like this PR didn't improve the situation, but the patch is being taken upstream rather quickly. No need to merge this PR.

derrickstolee added a commit to derrickstolee/VFSForGit that referenced this pull request Jun 10, 2019
Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants