(DO NOT MERGE) Tentative gvfs-2.20.0-rc0
#71
Closed
Conversation
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
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>
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>
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>
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>
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>
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 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>
During a 'git push' command, we run 'git send-pack' inside of our transport helper. This creates a 'git pack-objects' process and passes a list of object ids. If this list is large, then the pack-objects process can spend a lot of time checking the possible refs these strings could represent. Remove this extra check by setting core.warnAmbiguousRefs to false as we call 'git pack-objects'. I was able to see the thousands of calls to different refs using a PerfView trace with "File I/O" selected. After this change, seconds of reading thousands of files was removed.
dscho
requested review from
benpeart,
derrickstolee,
jamill,
jeffhostetler,
jrbriggs,
kewillford and
wilbaker
|
@dscho Sorry about the |
|
Build is failing on |
Since 73ba5d7 (roll wt_status_state into wt_status and populate in the collect phase, 2018-09-30), wt_longstatus_print() no longer sets the state, and hence does not realize when a merge is in progress. This triggered a test failure in t7524 that specifically tested for "You have unmerged paths" in the long status.
Member
Author
|
@kewillford could you have a look why https://gvfs.visualstudio.com/ci/_build/results?buildId=4066&view=logs passes in the Windows phase, despite there clearly being a problem in |
Member
Author
Actually, I think I got this. I simply added if (!$?) {
exit(1)
}after the call C:\git-sdk-64-ci\git-cmd.exe --command=usr\bin\sh.exe -lc @'
set -e
make -j10 DEVELOPER=1
make GIT_TEST_OPTS=--quiet -j10 -C t ||
make GIT_TEST_OPTS=\"-i -v -x\" -k -C t failed
'@ |
Member
Author
@derrickstolee I already had a look, and added 6657af1. Hopefully this fixes it the correct way. @jeffhostetler could you review that fixup! commit and tell me whether it is correct? And I think I addressed the Windows thing (incorrect pass, i.e. false negative) in #71 (comment) |
Member
Author
@derrickstolee no worries! I am glad that I guessed right. Actually, I guess I guessed half right? Which commits should we drop? |
Member
Author
@kewillford I also updated the build definition to use the Ubuntu1604 pool (Hosted Linux Preview is deprecated and will go away soon, and the only additional change that is required is to use |
Member
Author
@kewillford see https://aka.ms/linux-preview-deprecation for details; you may want to inspect your definitions before Dec 1st. The easiest way, I've been told, is to go to the admin page and look for the "Hosted Linux Preview" pool, then click on all of the agents in that pool to see which builds they ran lately. |
Member
Author
@kewillford I also updated the Windows phase to run on the "Hosted" pool instead of "Hosted VS2017". In my hands, the former performs some 20% faster than the latter. |
|
I'm getting started on a PR to point VFS for Git at this version. First step: build the installers. |
|
@dscho WRT "commitable" vs "committable": either way is fine. new deserialize clients will reject I'd say change it to "committable" to be consistent with the upstream change. |
|
@dscho WRT the trace2 in wt_status_collect(). that looks fine. thanks. |
Member
|
@dscho changing the pool to use Ubuntu failed. https://gvfs.visualstudio.com/ci/_build/results?buildId=4084&_a=summary&view=logs |
Member
Author
|
On Wed, 21 Nov 2018, Kevin Willford wrote:
@dscho changing the pool to use Ubuntu failed. https://gvfs.visualstudio.com/ci/_build/results?buildId=4084&_a=summary&view=logs
```
2018-11-21T16:41:37.4132217Z Reading package lists...
2018-11-21T16:41:37.4406105Z W: chmod 0700 of directory /var/lib/apt/lists/partial failed - SetupAPTPartialDirectory (1: Operation not permitted)
2018-11-21T16:41:37.4407293Z E: Could not open lock file /var/lib/apt/lists/lock - open (13: Permission denied)
2018-11-21T16:41:37.4407676Z E: Unable to lock directory /var/lib/apt/lists/
2018-11-21T16:41:37.4408580Z W: Problem unlinking the file /var/cache/apt/pkgcache.bin - RemoveCaches (13: Permission denied)
2018-11-21T16:41:37.4438740Z W: Problem unlinking the file /var/cache/apt/srcpkgcache.bin - RemoveCaches (13: Permission denied)
```
I see you fixed it already by prefixing the `apt-get` calls with `sudo`. That is a documented thing: Hosted Linux Preview was running inside Containers (where `sudo` is not necessary), but Ubuntu 16.04 runs on real VMs.
|
Member
Author
|
Closing, in favor of #73. |
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.
This is a tentative rebase of
gvfs-2.19.1ontov2.20.0-rc0.windows.1.The majority of the (quite painful) merge conflicts stemmed from the split of
Documentation/config.txtintoDocumentation/config/*.txt. I am glad that this is over.Other notable changes:
commitableinstruct wt_statuswas renamed tocommittablein git.git (6fa9019 (wt-status: rename commitable to committable, 2018-09-05)); I fixed the build by following said rename for that field, but kept the key name in the serialized status ascommitable, for backwards-compatibility. @jeffhostetler do you agree with this approach?wt_status_collect()had merge conflicts because that function was enhanced a little: it now also performs a check whether the current state is committable; I added another trace2 call pair (see ee1aef9, the pre-rebase version is here: 7a6b1cc). @jeffhostetler do you agree with this change?ref_newer(), but the post-rebase version (affa5d4) seems not to need that, asref_newer()learned to useis_descendant_of()in the meantime and no longer usespop_most_recent_commit()(andref_newer()also moved fromremote.ctocommit-reach.c). Do you concur with this reasoning, @derrickstolee?A ton of our patches made it upstream (thanks @benpeart, @derrickstolee and @jeffhostetler!!!). To make it easier to review this, I wrote this little shell script (which essentially uses the output of
range-diffto identify "dropped" commits and then looks for commits with the same oneline that are reachable from the upstream tag):The output:
ceab693 (multi-pack-index: add design document, 2018-07-12)
e0d1bcf (multi-pack-index: add format details, 2018-07-12)
6a257f0 (multi-pack-index: add builtin, 2018-07-12)
a340773 (multi-pack-index: add 'write' verb, 2018-07-12)
fc59e74 (midx: write header information to lockfile, 2018-07-12)
4d80560 (multi-pack-index: load into memory, 2018-07-12)
2c38133 (t5319: expand test data, 2018-07-12)
9208e31 (packfile: generalize pack directory list, 2018-07-12)
396f257 (multi-pack-index: read packfile list, 2018-07-12)
32f3c54 (multi-pack-index: write pack names in chunk, 2018-07-12)
3227565 (midx: read pack names into array, 2018-07-12)
fe1ed56 (midx: sort and deduplicate objects from packfiles, 2018-07-12)
0d5b3a5 (midx: write object ids in a chunk, 2018-07-12)
d7cacf2 (midx: write object id fanout chunk, 2018-07-12)
662148c (midx: write object offsets, 2018-07-12)
c4d2522 (config: create core.multiPackIndex setting, 2018-07-12)
3715a63 (midx: read objects from multi-pack-index, 2018-07-12)
8aac67a (midx: use midx in abbreviation calculations, 2018-07-12)
a40498a (midx: use existing midx when writing new one, 2018-07-12)
b8990fb (midx: use midx in approximate_object_count, 2018-07-12)
f3a002b (midx: prevent duplicate packfile loads, 2018-07-12)
17c35c8 (packfile: skip loading index if in multi-pack-index, 2018-07-12)
525e18c (midx: clear midx on repack, 2018-07-12)
6d68e6a (multi-pack-index: provide more helpful usage info, 2018-08-20)
2cf489a (multi-pack-index: store local property, 2018-08-20)
c39b02a (midx: mark bad packed objects, 2018-08-20)
fe86c3b (midx: stop reporting garbage, 2018-08-20)
29e2016 (midx: fix bug that skips midx with alternates, 2018-08-20)
0bff526 (packfile: add all_packs list, 2018-08-20)
454ea2e (treewide: use get_all_packs, 2018-08-20)
e9ab2ed (midx: test a few commands that use get_all_packs, 2018-08-20)
6a22d52 (pack-objects: consider packs in multi-pack-index, 2018-08-20)
56ee7ff (multi-pack-index: add 'verify' verb, 2018-09-13)
53ad040 (multi-pack-index: verify bad header, 2018-09-13)
d3f8e21 (multi-pack-index: verify corrupt chunk lookup table, 2018-09-13)
8e72a3c (multi-pack-index: verify packname order, 2018-09-13)
d4bf1d8 (multi-pack-index: verify missing pack, 2018-09-13)
2f23d3f (multi-pack-index: verify oid fanout order, 2018-09-13)
55c5648 (multi-pack-index: verify oid lookup order, 2018-09-13)
d8ac9ee (multi-pack-index: fix 32-bit vs 64-bit size check, 2018-09-13)
cc6af73 (multi-pack-index: verify object offsets, 2018-09-13)
144d703 (multi-pack-index: report progress during 'verify', 2018-09-13)
66ec039 (fsck: verify multi-pack-index, 2018-09-13)
0ce4ff9 (midx: fix broken free() in close_midx(), 2018-10-08)
1dcd9f2 (midx: close multi-pack-index on repack, 2018-10-12)
0465a50 (multi-pack-index: define GIT_TEST_MULTI_PACK_INDEX, 2018-10-12)
c46c406 (trace.h: support nested performance tracing, 2018-08-18)
0d1ed59 (unpack-trees: add performance tracing, 2018-08-18)
b4da373 (unpack-trees: optimize walking same trees with cache-tree, 2018-08-18)
f1e11c6 (unpack-trees: reduce malloc in cache-tree walk, 2018-08-18)
836ef2b (unpack-trees: reuse (still valid) cache-tree from src_index, 2018-08-18)
5697ca9 (unpack-trees: add missing cache invalidation, 2018-08-18)
4592e60 (cache-tree: verify valid cache-tree in the test suite, 2018-08-18)
5f4436a (Document update for nd/unpack-trees-with-cache-tree, 2018-08-25)
06ba9d0 (t0051: test GIT_TRACE to a windows named pipe, 2018-09-11)
eeaf7dd (mingw: fix mingw_open_append to work with named pipes, 2018-09-11)
The first of the two NOT upstream commits is of course a false positive: fa655d8 (checkout: optimize "git checkout -b <new_branch>", 2018-08-16) is upstream. @benpeart do we have to do anything to put that back under the
core.gvfsflag? If I read this correctly, then the speed-up is hidden behind a config setting that GVFS does not set, right?The second NOT upstream commit is obsolete, as Git for Windows renamed t5319 to t5321 (not t5320).