On Thu, Nov 21, 2019 at 08:04:35AM -0500, Derrick Stolee wrote: > On 11/19/2019 10:46 AM, SZEDER Gábor wrote: > > On Mon, Oct 21, 2019 at 01:56:13PM +0000, Derrick Stolee via GitGitGadget wrote: > >> From: Derrick Stolee <dstolee@xxxxxxxxxxxxx> > >> > >> The 'git sparse-checkout set' subcommand takes a list of patterns > >> as arguments and writes them to the sparse-checkout file. Then, it > >> updates the working directory using 'git read-tree -mu HEAD'. > >> > >> The 'set' subcommand will replace the entire contents of the > >> sparse-checkout file. The write_patterns_and_update() method is > >> extracted from cmd_sparse_checkout() to make it easier to implement > >> 'add' and/or 'remove' subcommands in the future. > > > > Yeah, an 'add' subcommand would be great, because 'set' throwing away > > all the existing patterns can lead to some frustration: > > I plan to extend this feature when this series lands. > > > Having said that, being forced to use 'git sparse-checkout set' and > > specify all patterns at once does have its own benefits: I needed like > > 6 subdirectories to build that subset of a big project that I was > > interested in, and now all the necessary patterns are saved in my Bash > > history, and I will be able to easily dig out the complete command in > > the future. That wouldn't work with using the 'add' subcommand to add > > one pattern at a time. > > In general, this "set" command is created first because tools can interact > with it more easily than "add" and "remove". Users would probably prefer the > "add" and "remove" way to interact. Makes sense. However, I'd like to add that in the meantime I got to like the 'set' subcommand more and more. I enabled-disabled sparse checkout a lot of times while testing and trying to poke holes, and to be able to set up everything with only one single command that I can easily recall from the shell's history was a great help. > >> diff --git a/t/t1091-sparse-checkout-builtin.sh b/t/t1091-sparse-checkout-builtin.sh > >> index cb74715ca6..bf2dc55bb1 100755 > >> --- a/t/t1091-sparse-checkout-builtin.sh > >> +++ b/t/t1091-sparse-checkout-builtin.sh > >> @@ -96,4 +96,36 @@ test_expect_success 'clone --sparse' ' > >> test_cmp expect dir > >> ' > >> > >> +test_expect_success 'set enables config' ' > >> + git init empty-config && > >> + ( > >> + cd empty-config && > >> + test_commit test file && > >> + test_path_is_missing .git/config.worktree && > >> + test_must_fail git sparse-checkout set nothing && > > > > This command prints the same error message twice: > > > > + test_must_fail git sparse-checkout set nothing > > error: Sparse checkout leaves no entry on working directory > > error: Sparse checkout leaves no entry on working directory > > At this commit, I do not see two identical lines, but instead the second > line says > > error: failed to update index with new sparse-checkout paths You're right, I must have looked at and copied the results from a test run of the full patch series or even from 'next'. The error message changes with commit efd9c53b6d (sparse-checkout: update working directory in-process, 2019-10-21) to what I shown above. Interestingly, at one point in my testing I managed to get a different error three times: $ ~/src/git/git sparse-checkout set foo error: Entry 'dir/bar' not uptodate. Cannot update sparse checkout. error: Entry 'dir/bar' not uptodate. Cannot update sparse checkout. error: Entry 'dir/bar' not uptodate. Cannot update sparse checkout. However, when I later re-run the same sequence of commands that lead to that three errors I got that error only twice, and couldn't reproduce it since. Style nit, seeing "error: Sparse..." and "error: Entry...": error messages should start with a lower-case letter. > (that "paths" should probably be "patterns") That would be better indeed, but that error message goes away by the end of the series anyway...
