Skip to content

Conversation

@dscho
Copy link
Member

@dscho dscho commented Dec 2, 2018

This is the even more painless successor to #73. I only have to forward-port the merged PRs #64 and #68.

Oh, and I renamed the branch to vfs-<version> and the reported version to [...].vfs.1.1 instead of [...].gvfs.1.1.

Ben Peart and others added 30 commits December 2, 2018 22:06
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 <Ben.Peart@microsoft.com>
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 <kewillf@microsoft.com>
Signed-off-by: Saeed Noursalehi <sanoursa@microsoft.com>
Signed-off-by: Johannes Schindelin <johasc@microsoft.com>
This header file will accumulate GVFS-specific definitions.

Signed-off-by: Kevin Willford <kewillf@microsoft.com>
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 <kewillf@microsoft.com>
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 <kewillf@microsoft.com>
Signed-off-by: Kevin Willford <kewillf@microsoft.com>
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 <kewillf@microsoft.com>
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 <kewillf@microsoft.com>
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 <johannes.schindelin@gmx.de>
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 <Ben.Peart@microsoft.com>
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 <kewillf@microsoft.com>
Signed-off-by: Ben Peart <Ben.Peart@microsoft.com>
…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 <Ben.Peart@microsoft.com>
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 <johasc@microsoft.com>
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 <Ben.Peart@microsoft.com>
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 <Ben.Peart@microsoft.com>
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 <johannes.schindelin@gmx.de>
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 <Ben.Peart@microsoft.com>
Suggested by Ben Peart.

Signed-off-by: Johannes Schindelin <johasc@microsoft.com>
Signed-off-by: Alejandro Pauly <alpauly@microsoft.com>
We need to respect that config setting even if we already know that we
have a repository, but have not yet read the config.

The regression test was written by Alejandro Pauly.

Signed-off-by: Johannes Schindelin <johasc@microsoft.com>
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 <kewillf@microsoft.com>
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 <kewillf@microsoft.com>
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 <kewillf@microsoft.com>
Signed-off-by: Kevin Willford <kewillf@microsoft.com>
benpeart and others added 8 commits December 2, 2018 22:07
…tash`

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'
…_MISSING_OK set

send-pack: do not check for sha1 file when GVFS_MISSING_OK set
@dscho dscho changed the title (DO NOT MERGE) Tentative gvfs-2.20.0-rc1 (DO NOT MERGE) Tentative vfs-2.20.0-rc2 Dec 2, 2018
@dscho
Copy link
Member Author

dscho commented Dec 2, 2018

@kewillford FYI t7113-post-index-changed-hook.sh is failing here... probably some interaction between the changes introduced by #64 with the changes in v2.20.0-rc2...

@dscho
Copy link
Member Author

dscho commented Dec 3, 2018

FYI t7113-post-index-changed-hook.sh is failing here... probably some interaction between the changes introduced by #64 with the changes in v2.20.0-rc2...

So. The bisect run was pretty much useless.

Until the point, that is, when I realized that t7113 is flakey! It passes most of the time, but sometimes it does not.

@dscho
Copy link
Member Author

dscho commented Dec 3, 2018

t7113 is flakey!

And this fixes it:

diff --git a/t/t7113-post-index-changed-hook.sh b/t/t7113-post-index-changed-hook.sh
index 8c6b0972ca..c464751b9c 100755
--- a/t/t7113-post-index-changed-hook.sh
+++ b/t/t7113-post-index-changed-hook.sh
@@ -34,6 +34,8 @@ test_expect_success 'test status, add, commit, others trigger hook without flags
                echo "success" >testsuccess
        EOF
        mkdir -p dir2 &&
+       : force index to be dirty &&
+       test-tool chmtime +60 dir1/file1.txt &&
        touch dir2/file1.txt &&
        touch dir2/file2.txt &&
        git status &&

@dscho
Copy link
Member Author

dscho commented Dec 3, 2018

There you go: b38392e851. @benpeart this is probably as much for you to review as it is for @kewillford...

@dscho dscho force-pushed the tentative/vfs-2.20.0-rc2 branch from b38392e to 67016d9 Compare December 3, 2018 15:29
@dscho
Copy link
Member Author

dscho commented Dec 3, 2018

There you go: b38392e. @benpeart this is probably as much for you to review as it is for @kewillford...

Ooops, that was incomplete. See 67016d957b for a more complete fix.

@dscho dscho force-pushed the tentative/vfs-2.20.0-rc2 branch from 67016d9 to 73c16f7 Compare December 3, 2018 15:50
@dscho
Copy link
Member Author

dscho commented Dec 3, 2018

Still not complete... Let's see what the Azure Pipelines says.

dscho added 2 commits December 3, 2018 17:35
We need to force the index to be dirty; otherwise the index might not
even be written in the `git status`/`git checkout`/`git reset` calls,
meaning that the hook won't be called at all, either.
Ben implemented a short-cut for `git checkout -b` which avoids writing
the index. Therefore, no hook would be called in that case. Let's force
it do be called by using `-B` instead of `-b`.
@dscho dscho force-pushed the tentative/vfs-2.20.0-rc2 branch from 73c16f7 to eaed3e8 Compare December 3, 2018 16:36
@dscho
Copy link
Member Author

dscho commented Dec 3, 2018

There!

@dscho
Copy link
Member Author

dscho commented Dec 3, 2018

BTW @benpeart in that test case in t7113 where it essentially calls git checkout -b new-branch, your nice speed-up comes to play, and the index is not (re-)written. On Linux. But on macOS and Windows, it seems to be written, still, otherwise the Azure Pipeline would have failed the earlier iteration that did not change that git checkout -b to git checkout -B also on macOS on Windows, but it failed only on Linux (but there, consistently).

Maybe you want to look into that, as Windows and macOS are our primary target operating systems.

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(...)`.
virtualfilesystem: fix case where directories not handled correctly
@dscho dscho mentioned this pull request Dec 10, 2018
@dscho
Copy link
Member Author

dscho commented Dec 10, 2018

Superseded by #86

@dscho dscho closed this Dec 10, 2018
@dscho dscho deleted the tentative/vfs-2.20.0-rc2 branch December 11, 2018 15:52
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