Skip to content

Conversation

@derrickstolee
Copy link

This branch includes our VFS for Git specific commits and replays them on top of v2.20.0.windows.1.

Ben Peart and others added 8 commits December 10, 2018 14:01
This is just a bug fix to git so that the pager won't close stdin/out
before other atexit functions run. The easy way to repro the bug is to
turn on GIT_TRACE_PERFORMANCE and run a command that runs the pager. Then
notice you don't get your performance data at the end. With this fix, you
do actually get the performance trace data.

Signed-off-by: Ben Peart <[email protected]>
While using the reset --stdin feature on windows path added may have a
\r at the end of the path that wasn't getting removed so didn't match
the path in the index and wasn't reset.

Signed-off-by: Kevin Willford <[email protected]>
When using the sparse-checkout feature, the file might not be on disk
because the skip-worktree bit is on.

Signed-off-by: Kevin Willford <[email protected]>
When using the sparse-checkout feature git should not write to the working
directory for files with the skip-worktree bit on.  With the skip-worktree
bit on the file may or may not be in the working directory and if it is
not we don't want or need to create it by calling checkout_entry.

There are two callers of checkout_target.  Both of which check that the
file does not exist before calling checkout_target.  load_current which
make a call to lstat right before calling checkout_target and
check_preimage which will only run checkout_taret it stat_ret is less than
zero.  It sets stat_ret to zero and only if !stat->cached will it lstat
the file and set stat_ret to something other than zero.

This patch checks if skip-worktree bit is on in checkout_target and just
returns so that the entry doesn't not end up in the working directory.
This is so that apply will not create a file in the working directory,
then update the index but not keep the working directory up to date with
the changes that happened in the index.

Signed-off-by: Kevin Willford <[email protected]>
When using the sparse checkout feature the git reset command will add
entries to the index that will have the skip-worktree bit off but will
leave the working directory empty.  File data is lost because the index
version of the files has been changed but there is nothing that is in
the working directory.  This will cause the next status call to show
either deleted for files modified or deleting or nothing for files
added.  The added files should be shown as untracked and modified files
should be shown as modified.

To fix this when the reset is running if there is not a file in the
working directory and if it will be missing with the new index entry or
was not missing in the previous version, we create the previous index
version of the file in the working directory so that status will report
correctly and the files will be availble for the user to deal with.

Signed-off-by: Kevin Willford <[email protected]>
The multi-pack-index was added to the data verified by git-fsck in
ea5ae6c "fsck: verify multi-pack-index". This implementation was
based on the implementation for verifying the commit-graph, and a
copy-paste error kept the ERROR_COMMIT_GRAPH flag as the bit set
when an error appears in the multi-pack-index.

Add a new flag, ERROR_MULTI_PACK_INDEX, and use that instead.

Signed-off-by: Derrick Stolee <[email protected]>
dscho and others added 19 commits December 10, 2018 14:25
This header file will accumulate GVFS-specific definitions.

Signed-off-by: Kevin Willford <[email protected]>
This does not do anything yet. The next patches will add various values
for that config setting that correspond to the various features
offered/required by GVFS.

Signed-off-by: Kevin Willford <[email protected]>
The two existing members of the run_hook*() family, run_hook_ve() and
run_hook_le(), are good for callers that know the precise number of
parameters already. Let's introduce a new sibling that takes an argv
array for callers that want to pass a variable number of parameters.

Signed-off-by: Johannes Schindelin <[email protected]>
This takes a substantial amount of time, and if the user is reasonably
sure that the files' integrity is not compromised, that time can be saved.

Git no longer verifies the SHA-1 by default, anyway.

Signed-off-by: Kevin Willford <[email protected]>
The idea is to allow blob objects to be missing from the local repository,
and to load them lazily on demand.

After discussing this idea on the mailing list, we will rename the feature
to "lazy clone" and work more on this.

Signed-off-by: Ben Peart <[email protected]>
Hydrate missing loose objects in check_and_freshen() when running
virtualized. Add test cases to verify read-object hook works when
running virtualized.

This hook is called in check_and_freshen() rather than
check_and_freshen_local() to make the hook work also with alternates.

Helped-by: Kevin Willford <[email protected]>
Signed-off-by: Ben Peart <[email protected]>
Prevent the sparse checkout to delete files that were marked with
skip-worktree bit and are not in the sparse-checkout file.

This is because everything with the skip-worktree bit turned on is being
virtualized and will be removed with the change of HEAD.

There was only one failing test when running with these changes that was
checking to make sure the worktree narrows on checkout which was
expected since we would no longer be narrowing the worktree.

Signed-off-by: Kevin Willford <[email protected]>
…ng objects

This commit converts the existing read_object hook proc model for
downloading missing blobs to use a background process that is started
the first time git encounters a missing blob and stays running until git
exits.  Git and the read-object process communicate via stdin/stdout and
a versioned, capability negotiated interface as documented in
Documentation/technical/read-object-protocol.txt.  The advantage of this
over the previous hook proc is that it saves the overhead of spawning a
new hook process for every missing blob.

The model for the background process was refactored from the recent git
LFS work.  I refactored that code into a shared module (sub-process.c/h)
and then updated convert.c to consume the new library.  I then used the
same sub-process module when implementing the read-object background
process.

Signed-off-by: Ben Peart <[email protected]>
While performing a fetch with a virtual file system we know that there
will be missing objects and we don't want to download them just because
of the reachability of the commits.  We also don't want to download a
pack file with commits, trees, and blobs since these will be downloaded
on demand.

This flag will skip the first connectivity check and by returning zero
will skip the upload pack. It will also skip the second connectivity
check but continue to update the branches to the latest commit ids.

Signed-off-by: Kevin Willford <[email protected]>
This adds hard-coded call to GVFS.hooks.exe before and after each Git
command runs.

To make sure that this is only called on repositories cloned with GVFS, we
test for the tell-tale .gvfs.

Signed-off-by: Ben Peart <[email protected]>
If we are going to write an object there is no use in calling
the read object hook to get an object from a potentially remote
source.  We would rather just write out the object and avoid the
potential round trip for an object that doesn't exist.

This change adds a flag to the check_and_freshen() and
freshen_loose_object() functions' signatures so that the hook
is bypassed when the functions are called before writing loose
objects. The check for a local object is still performed so we
don't overwrite something that has already been written to one
of the objects directories.

Based on a patch by Kevin Willford.

Signed-off-by: Johannes Schindelin <[email protected]>
Ensure all filters and EOL conversions are blocked when running under
GVFS so that our projected file sizes will match the actual file size
when it is hydrated on the local machine.

Signed-off-by: Ben Peart <[email protected]>
The use case here is to allow usage statistics to be gathered by
running hooks before and after every hook, and to make that
configurable via hooks.

Signed-off-by: Johannes Schindelin <[email protected]>
GVFS Git introduced pre-command and post-command hooks, to gather usage
statistics and to be able to adjust the worktree if necessary.

As run_hooks() implicitly calls setup_git_directory(), and that
function does surprising things to the global state (sometimes even
changing the current working directory), it cannot be used here.

This commit introduces the pre-command/post-command hooks, based on
the previous patches that culminate in support for running hooks early,
i.e. before setup_git_directory() was called.

Signed-off-by: Ben Peart <[email protected]>
Suggested by Ben Peart.

Signed-off-by: Johannes Schindelin <[email protected]>
jeffhostetler and others added 15 commits December 10, 2018 14:25
Add trace2 events when reading and writing the index.

Signed-off-by: Jeff Hostetler <[email protected]>
The virtual file system code incorrectly treated symlinks as directories
instead of regular files.  This meant symlinks were not included even if
they are listed in the list of files returned by the core.virtualFilesystem
hook proc.  Fixes #25

Signed-off-by: Ben Peart <[email protected]>
virtualfilesystem: don't run the virtual file system hook if the index has been redirected
Add trace2 region and data events describing attempts to deserialize
status data using a status cache.

A category:status, label:deserialize region is pushed around the
deserialize code.

Deserialization results when reading from a file are:
    category:status, path   = <path>
    category:status, polled = <number_of_attempts>
    category:status, result = "ok" | "reject"

When reading from STDIN are:
    category:status, path   = "STDIN"
    category:status, result = "ok" | "reject"

Status will fallback and run a normal status scan when a "reject"
is reported (unless "--deserialize-wait=fail").  If "ok" is reported,
status was able to use the status cache and avoid scanning the workdir.

Signed-off-by: Jeff Hostetler <[email protected]>
Add check to see if a directory is included in the virtualfilesystem
before checking the directory hashmap.  This allows a directory entry
like foo/ to find all untracked files in subdirectories.
virtualfilesystem: fix bug with symlinks being ignored
…ncluded

Add check to see if a directory is included in the virtualfilesystem
before checking the directory hashmap. This allows a directory entry
like foo/ to find all untracked files in subdirectories.
(Experimental) Trace2 base plus GVFS extensions
When studying the performance of 'git push' we would like to know
how much time is spent at various parts of the command. One area
that could cause performance trouble is 'git pack-objects'.

Add trace2 regions around the three main actions taken in this
command:

1. Enumerate objects.
2. Prepare pack.
3. Write pack-file.

Signed-off-by: Derrick Stolee <[email protected]>
William Baker reported that the non-built-in rebase and stash fail to
run the post-command hook (which is important for VFS for Git, though).

The reason is that an `exec()` will replace the current process by the
newly-exec'ed one (our Windows-specific emulation cannot do that, and
does not even try, so this is only an issue on Linux/macOS). As a
consequence, not even the atexit() handlers are run, including the
one running the post-command hook.

To work around that, let's spawn the legacy rebase/stash and exit with
the reported exit code.
We want to make `git push` faster, but we need to know where the time is going!

There are likely four places where the time is going:

1. The info/refs call and force-update checking at the beginning.
2. The `git pack-objects` call that creates a pack-file to send to the server.
3. Sending the data to the server.
4. Waiting for the server to verify the pack-file.

This PR adds `trace2_region_` calls inside `git pack-objects` so we can track the time in item (2). The rest could be interpreted from the start and end time of the entire command after we know this region. The server-side verification is something we can track using server telemetry.
…t stash`

William Baker reported that the non-built-in rebase and stash fail to
run the post-command hook (which is important for VFS for Git, though).

The reason is that an `exec()` will replace the current process by the
newly-exec'ed one (our Windows-specific emulation cannot do that, and
does not even try, so this is only an issue on Linux/macOS). As a
consequence, not even the atexit() handlers are run, including the
one running the post-command hook.

To work around that, let's spawn the legacy rebase/stash and exit with
the reported exit code.
This includes commits that fixup!-revert all the midx-related commits from our GVFS branch and replaces them with the exact commits that are being merged upstream. This should automatically remove the commits during our next version rebase-and-merge action.

Changes upstream:
- The builtin is called 'git multi-pack-index'.
- The command-line takes a 'write' verb and an '--object-dir' parameter.
- We no longer have a 'midx-head' or '*.midx' files.
- Instead, we have a 'multi-pack-index' file in the pack-dir.
- It no longer makes sense to specify '--update-head'
@derrickstolee
Copy link
Author

Running VFS for Git tests in PR 600.

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.

Relative to #81, I see that these are missing:

60:  401c23d0067c <  -:  ------------ read-cache: add post-indexchanged hook
61:  4d7277d56597 <  -:  ------------ read-cache: post-indexchanged hook add skip-worktree bit changing support
62:  826c266f8f17 <  -:  ------------ read-cache: add test for post-indexchanged hook
63:  ea0d08735d02 <  -:  ------------ send-pack: do not check for sha1 file when GVFS_MISSING_OK set
64:  19564f520115 <  -:  ------------ Add documentation for the post-indexchanged hook
65:  d4b7d65588c7 <  -:  ------------ fixup! read-cache: add test for post-indexchanged hook
66:  eaed3e8b972a <  -:  ------------ fixup! read-cache: add test for post-indexchanged hook
67:  a0fb60f0161e <  -:  ------------ vfs: fix case where directories not handled correctly

(Compare via git range-diff v2.20.0-rc2.windows.1..Microsoft/tentative/vfs-2.20.0-rc2 v2.20. 0.windows.1..tentative/gvfs-2.20.0)

Any idea what happened to those?


GVF=GIT-VERSION-FILE
DEF_VER=v2.20.0.gvfs.1.1
DEF_VER=v2.20.0.vfs.1.1
Copy link
Member

Choose a reason for hiding this comment

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

Did you want to amend 0c8a4ac instead, perchance?

benpeart and others added 6 commits December 10, 2018 16:28
The vfs does not correctly handle the case when there is a file
that begins with the same prefix as a directory. For example, the
following setup would encounter this issue:

    A directory contains a file named `dir1.sln` and a directory
    named `dir1/`.

    The directory `dir1` contains other files.

    The directory `dir1` is in the virtual file system list

The contents of `dir1` should be in the virtual file system, but
it is not. The contents of this directory do not have the skip
worktree bit cleared as expected. The problem is in the
`apply_virtualfilesystem(...)` function where it does not include
the trailing slash of the directory name when looking up the
position in the index to start clearing the skip worktree bit.

This fix is it include the trailing slash when finding the first
index entry from `index_name_pos(...)`.
…tories

virtualfilesystem: fix case where directories not handled correctly
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.

8 participants