[PATCH v3 0/2] Handle git_path() with lock files correctly in worktrees
I stumbled over this during my recent work in Git GUI
[https://github.com/gitgitgadget/git/pull/361] that was originally really
only intended to use the correct hooks directory.
It turns out that my fears that index.lock was mishandled were unfounded,
hence this patch series has a lot lower priority for me than "OMG we must
push this into v2.24.0!".
Technically, the first patch is not needed (because I decided against adding
a test to t1400 in the end, in favor of t1500), but it shouldn't hurt,
either.
Changes since v2:
* Adjusted the commit message to really not talk about index.lock.
* Instead of modifying the code inside trie_find() to special-case .lock, I
now copy the string without the suffix .lock (if any) before handing it
off to trie_find().
Changes since v1:
* Clarified the commit message to state that index.lock is fine, it is
logs/HEAD.lock that isn't.
Johannes Schindelin (2):
t1400: wrap setup code in test case
git_path(): handle `.lock` files correctly
path.c| 8 +++-
t/t1400-update-ref.sh | 18 ++
t/t1500-rev-parse.sh | 15 +++
3 files changed, 32 insertions(+), 9 deletions(-)
base-commit: 108b97dc372828f0e72e56bbb40cae8e1e83ece6
Published-As:
https://github.com/gitgitgadget/git/releases/tag/pr-401%2Fdscho%2Flock-files-in-worktrees-v3
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git
pr-401/dscho/lock-files-in-worktrees-v3
Pull-Request: https://github.com/gitgitgadget/git/pull/401
Range-diff vs v2:
1: cf97c5182e = 1: cf97c5182e t1400: wrap setup code in test case
2: 93dba5a3a3 ! 2: 8b1f4f089a git_path(): handle `.lock` files correctly
@@ -4,12 +4,12 @@
Ever since worktrees were introduced, the `git_path()` function
_really_
needed to be called e.g. to get at the path to `logs/HEAD` (`HEAD` is
-specific to the worktree). However, the wrong path is returned for
-`logs/HEAD.lock`.
+specific to the worktree, and therefore so is its reflog). However,
the
+wrong path is returned for `logs/HEAD.lock`.
This does not matter as long as the Git executable is doing the
asking,
-as the path for that `index.lock` file is constructed from
-`git_path("index")` by appending the `.lock` suffix.
+as the path for that `logs/HEAD.lock` file is constructed from
+`git_path("logs/HEAD")` by appending the `.lock` suffix.
However, Git GUI just learned to use `--git-path` instead of appending
relative paths to what `git rev-parse --git-dir` returns (and as a
@@ -19,7 +19,8 @@
let's be safe rather than sorry.
Side note: Git GUI _does_ ask for `index.lock`, but that is already
-resolved correctly.
+resolved correctly, due to `update_common_dir()` preferring to leave
+unknown paths in the (worktree-specific) git directory.
Signed-off-by: Johannes Schindelin
@@ -27,23 +28,23 @@
--- a/path.c
+++ b/path.c
@@
- int result;
- struct trie *child;
-
-- if (!*key) {
-+ if (!*key || !strcmp(key, ".lock")) {
- /* we have reached the end of the key */
- if (root->value && !root->len)
- return fn(key, root->value, baton);
-@@
-
- /* Matched the entire compressed section */
- key += i;
-- if (!*key)
-+ if (!*key || !strcmp(key, ".lock"))
- /* End of key */
- return fn(key, root->value, baton);
+ static void update_common_dir(struct strbuf *buf, int git_dir_len,
+const char *common_dir)
+ {
+- char *base = buf->buf + git_dir_len;
++ char *base = buf->buf + git_dir_len, *base2 = NULL;
++
++ if (ends_with(base, ".lock"))
++ base = base2 = xstrndup(base, strlen(base) - 5);
++
+ init_common_trie();
+ if (trie_find(&common_trie, base, check_common, NULL) > 0)
+ replace_dir(buf, git_dir_len, common_dir);
++
++ free(base2);
+ }
+ void report_linked_checkout_garbage(void)
diff --git a/t/t1500-rev-parse.sh b/t/t1500-rev-parse.sh
--- a/t/t1500-rev-parse.sh
--
gitgitgadget
Re: [PATCH v3 0/2] Update: fixed typos in commit message
On Tue, Sep 24, 2019 at 03:40:28AM -0700, Alexandr Miloslavskiy via GitGitGadget wrote: > Commit 1/2: t0028: fix test for UTF-16-LE-BOM Commit 2/2: t0028: add more > tests Please refer to individual commit messages for more information. > > Alexandr Miloslavskiy (2): > t0028: fix test for UTF-16-LE-BOM > t0028: add more tests > Thanks for the update - Reviewed-by: Torsten Bögershausen
[PATCH v3 0/2] Update: fixed typos in commit message
Commit 1/2: t0028: fix test for UTF-16-LE-BOM Commit 2/2: t0028: add more
tests Please refer to individual commit messages for more information.
Alexandr Miloslavskiy (2):
t0028: fix test for UTF-16-LE-BOM
t0028: add more tests
t/t0028-working-tree-encoding.sh | 41 +++-
1 file changed, 40 insertions(+), 1 deletion(-)
base-commit: 4c86140027f4a0d2caaa3ab4bd8bfc5ce3c11c8a
Published-As:
https://github.com/gitgitgadget/git/releases/tag/pr-347%2FSyntevoAlex%2F%230189_t0028_fixes-v3
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git
pr-347/SyntevoAlex/#0189_t0028_fixes-v3
Pull-Request: https://github.com/gitgitgadget/git/pull/347
Range-diff vs v2:
1: d717a60932 ! 1: 438ac961a5 t0028: fix test for UTF-16-LE-BOM
@@ -3,15 +3,15 @@
t0028: fix test for UTF-16-LE-BOM
According to its name, the test is designed for UTF-16-LE-BOM.
-However, possibly due to copy&paste oversight, it was using UTF-32
file.
+However, possibly due to copy&paste oversight, it was using UTF-32.
-While the test succeeds (probably interprets extra \x00\x00 as
embedded
-zero), I myself had an unrelated problem which caused the test to
fail.
+While the test succeeds (extra \000\000 are interpreted as NUL),
+I myself had an unrelated problem which caused the test to fail.
When analyzing the failure I was quite puzzled by the fact that the
test is obviously bugged. And it seems that I'm not alone:
https://public-inbox.org/git/CAH8yC8kSakS807d4jc_BtcUJOrcVT4No37AXSz=jepxhw-o...@mail.gmail.com/T/#u
-This fix changes the test to follow its original intention.
+Fix the test to follow its original intention.
Signed-off-by: Alexandr Miloslavskiy
2: 40e54cf5ce ! 2: e4410274e6 t0028: add more tests
@@ -2,9 +2,9 @@
t0028: add more tests
-After I discovered that UTF-16-LE-BOM test was bugged and still
-succeeded, I decided that better tests are required. Possibly the best
-option here is to compare git results against hardcoded ground truth.
+After I discovered that UTF-16-LE-BOM test was bugged, I decided that
+better tests are required. Possibly the best option here is to compare
+git results against hardcoded ground truth.
The new tests also cover more interesting chars where (ANSI != UTF-8).
@@ -25,26 +25,26 @@
+ orig_string="$2"
+ expect_bytes="$3"
+
-+ test_expect_success "Commit utf-8, checkout ${encoding}" '
++ test_expect_success "Commit UTF-8, checkout $encoding" '
+ test_when_finished "git checkout HEAD -- .gitattributes" &&
+
-+ test_ext="commit_utf8_checkout_${encoding}" &&
-+ test_file="test.${test_ext}" &&
++ test_ext="commit_utf8_checkout_$encoding" &&
++ test_file="test.$test_ext" &&
+
-+ # Commit as utf-8
-+ echo "*.${test_ext} text working-tree-encoding=utf-8"
>.gitattributes &&
-+ printf "${orig_string}" >"${test_file}" &&
-+ git add "${test_file}" &&
++ # Commit as UTF-8
++ echo "*.$test_ext text working-tree-encoding=UTF-8"
>.gitattributes &&
++ printf "$orig_string" >$test_file &&
++ git add $test_file &&
+ git commit -m "Test data" &&
+
+ # Checkout in tested encoding
-+ rm "${test_file}" &&
-+ echo "*.${test_ext} text working-tree-encoding=${encoding}"
>.gitattributes &&
-+ git checkout HEAD -- "${test_file}" &&
++ rm $test_file &&
++ echo "*.$test_ext text working-tree-encoding=$encoding"
>.gitattributes &&
++ git checkout HEAD -- $test_file &&
+
+ # Test
-+ printf "${expect_bytes}" > "${test_file}.raw" &&
-+ test_cmp_bin "${test_file}.raw" "${test_file}"
++ printf $expect_bytes >$test_file.raw &&
++ test_cmp_bin $test_file.raw $test_file
+ '
+}
+
--
gitgitgadget
[PATCH v3 0/2] partial-clone: fix two issues with sparse filter handling
Included here are two fixes for partial cloning with sparse filters. These issues were uncovered in early testing internally at GitHub, where Taylor and Peff have provided early offlist review feedback. This third revision includes a fix for a test bug introduced in the second revision. Jon Simons (2): list-objects-filter: only parse sparse OID when 'have_git_dir' list-objects-filter: handle unresolved sparse filter OID list-objects-filter-options.c | 3 ++- list-objects-filter.c | 6 +- t/t5616-partial-clone.sh | 28 3 files changed, 35 insertions(+), 2 deletions(-) -- 2.23.0.37.g745f681289.dirty
[PATCH v3 0/2] Honor .gitattributes with rebase --am
This series makes rebase --am honor the .gitattributes file for
subsequent patches when a patch changes it.
Changes from v2:
* Rename has_path_suffix to ends_with_path_components.
Changes from v1:
* Add has_path_suffix in a separate commit.
brian m. carlson (2):
path: add a function to check for path suffix
apply: reload .gitattributes after patching it
apply.c | 7 +++
convert.c | 9 -
convert.h | 6 ++
path.c| 39 ++-
path.h| 3 +++
t/t3400-rebase.sh | 23 +++
6 files changed, 77 insertions(+), 10 deletions(-)
Range-diff against v2:
1: 125fda966c ! 1: 14c853420b path: add a function to check for path suffix
@@ Commit message
have one to check for a path suffix. For a plain filename, we can use
basename, but that requires an allocation, since POSIX allows it to
modify its argument. Refactor strip_path_suffix into a helper function
-and a new function, has_path_suffix to meet this need.
+and a new function, ends_with_path_components, to meet this need.
Signed-off-by: brian m. carlson
@@ path.c: static inline int chomp_trailing_dir_sep(const char *path, int
len)
+}
+
+/*
-+ * Returns true if the path ends with suffix, considering only complete
path
-+ * components and false otherwise.
++ * Returns true if the path ends with components, considering only
complete path
++ * components, and false otherwise.
+ */
-+int has_path_suffix(const char *path, const char *suffix)
++int ends_with_path_components(const char *path, const char *components)
+{
-+ return stripped_path_suffix_offset(path, suffix) != -1;
++ return stripped_path_suffix_offset(path, components) != -1;
+}
+
/*
@@ path.h: const char *git_path_merge_head(struct repository *r);
const char *git_path_shallow(struct repository *r);
+
-+int has_path_suffix(const char *path, const char *suffix);
++int ends_with_path_components(const char *path, const char *components);
+
#endif /* PATH_H */
2: f54af7e595 ! 2: 5f4402b38d apply: reload .gitattributes after patching it
@@ apply.c: static int apply_patch(struct apply_state *state,
listp = &patch->next;
+
+ if (!flush_attributes && patch->new_name &&
-+ has_path_suffix(patch->new_name,
GITATTRIBUTES_FILE))
++ ends_with_path_components(patch->new_name,
GITATTRIBUTES_FILE))
+ flush_attributes = 1;
}
else {
[PATCH v3 0/2] accessing the git(1) help page
This hopefully is a final version of the usage message to show how to access the git(1) manual page. The V3 change is Eric's suggestion to use 'See'. I have also taken the opportunity [Patch 2/2] to pick up peff's suggestion to include the git-scm doc link, and also correct the mis-quoting of a link to the html formatted github doc pages. Philip Oakley (2): git.c: show usage for accessing the git(1) help page Doc: git.txt: remove backticks from link and add git-scm.com/docs Documentation/git.txt | 3 ++- git.c | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) -- 2.21.0.windows.1.1517.gbad5f960a3.dirty
[PATCH v3 0/2] nd/merge-quit updates
v3 fixes the test breakage when GPG tests are skipped ('side' branch is
affected by these skipped tests)
Nguyễn Thái Ngọc Duy (2):
merge: remove drop_save() in favor of remove_merge_branch_state()
merge: add --quit
Documentation/git-merge.txt | 4
branch.c| 11 ---
branch.h| 6 ++
builtin/merge.c | 30 ++
t/t7600-merge.sh| 14 ++
5 files changed, 50 insertions(+), 15 deletions(-)
Range-diff dựa trên v2:
1: 324d237f0c ! 1: 86dd0fd99c merge: add --quit
@@ -13,7 +13,6 @@
different UI).
Signed-off-by: Nguyễn Thái Ngọc Duy
-Signed-off-by: Junio C Hamano
diff --git a/Documentation/git-merge.txt b/Documentation/git-merge.txt
--- a/Documentation/git-merge.txt
@@ -76,7 +75,7 @@
'
+test_expect_success 'merge --quit' '
-+ git reset --hard side &&
++ git reset --hard c2 &&
+ test_must_fail git -c rerere.enabled=true merge master &&
+ test_path_is_file .git/MERGE_HEAD &&
+ test_path_is_file .git/MERGE_MODE &&
--
2.21.0.1141.gd54ac2cb17
[PATCH v3 0/2] format-patch: teach format.notes config option
Hi Beat, thanks for catching the style errors. This version fixes those.
Changes since v2:
* Fixed if-else code style
* Fixed typoed errors in 2/2 log message
Changes since v1:
* Made format.notes accept a notes ref instead of a boolean
Denton Liu (2):
git-format-patch.txt: document --no-notes option
format-patch: teach format.notes config option
Documentation/config/format.txt| 13 ++
Documentation/git-format-patch.txt | 7 ++-
builtin/log.c | 18 +++-
t/t4014-format-patch.sh| 70 ++
4 files changed, 106 insertions(+), 2 deletions(-)
Range-diff against v2:
1: 4c3535f25b = 1: 4c3535f25b git-format-patch.txt: document --no-notes option
2: fe674bf63e ! 2: df864c4adf format-patch: teach format.notes config option
@@ -8,8 +8,8 @@
that they may forget to include it and generate a patch series without
notes.
-Teach git-format-patch the `format.notes` config option its value is a
-notes ref that will be automatically be appended. The special value of
+Teach git-format-patch the `format.notes` config option. Its value is a
+notes ref that will be automatically appended. The special value of
"standard" can be used to specify the standard notes. This option is
overridable with the `--no-notes` option in case a user wishes not to
append notes.
@@ -71,9 +71,9 @@
+ struct strbuf buf = STRBUF_INIT;
+
+ rev->show_notes = 1;
-+ if (!strcmp(value, "standard"))
++ if (!strcmp(value, "standard")) {
+ rev->notes_opt.use_default_notes = 1;
-+ else {
++ } else {
+ strbuf_addstr(&buf, value);
+ expand_notes_ref(&buf);
+ string_list_append(&rev->notes_opt.extra_notes_refs,
--
2.21.0.1049.geb646f7864
Re: [PATCH v3 0/2] Fix fsmonitor after discard_index()
Junio C Hamano writes: > Ævar Arnfjörð Bjarmason writes: > >> This v3 is all Johannes's patches. The outstanding review on v2 could >> be clarified with a commit message change, which I've addressed, and >> v2 conflicted with a cache.h change that's since landed in "master", >> which I've rebased this on. >> >> Junio: We're getting closer to the release so it would be great to >> have this. It's been broken for a long time, but having this finaly >> fixed in v2.22 would be great. The functional code changes are also >> isolated to the fsmonitor code path, which reduces the risk and makes >> this easier to review/reason about. > > Thanks. With this many topic backlog in flight (and not yet in > 'pu'), a resend like this, which is much easier to handle than mere > reference to the previous discussion thread, is really appreciated. Hmph, surprisingly, 1/2 alone did not reproduce any breakage. ... goes and looks ... Ah, that's false alarm. As I use prove, "make test" treats known breakage as a non-event and does not show, which made me lose a few minutes scratching my head wondering what I did wrong. It would have been nicer if it were easy to use 1/2 alone with the new test marked to expect success, but with a patch split into two like this, it requires going in to the test script and changing s/_failure/_success/, which is a bit suboptimial. Anyway, will queue this to 'pu' and hopefully we can merge it down soonish. Thanks.
Re: [PATCH v3 0/2] Fix fsmonitor after discard_index()
Ævar Arnfjörð Bjarmason writes: > This v3 is all Johannes's patches. The outstanding review on v2 could > be clarified with a commit message change, which I've addressed, and > v2 conflicted with a cache.h change that's since landed in "master", > which I've rebased this on. > > Junio: We're getting closer to the release so it would be great to > have this. It's been broken for a long time, but having this finaly > fixed in v2.22 would be great. The functional code changes are also > isolated to the fsmonitor code path, which reduces the risk and makes > this easier to review/reason about. Thanks. With this many topic backlog in flight (and not yet in 'pu'), a resend like this, which is much easier to handle than mere reference to the previous discussion thread, is really appreciated.
[PATCH v3 0/2] Fix fsmonitor after discard_index()
This v3 is all Johannes's patches. The outstanding review on v2 could be clarified with a commit message change, which I've addressed, and v2 conflicted with a cache.h change that's since landed in "master", which I've rebased this on. Junio: We're getting closer to the release so it would be great to have this. It's been broken for a long time, but having this finaly fixed in v2.22 would be great. The functional code changes are also isolated to the fsmonitor code path, which reduces the risk and makes this easier to review/reason about. Johannes Schindelin (2): fsmonitor: demonstrate that it is not refreshed after discard_index() fsmonitor: force a refresh after the index was discarded cache.h | 3 ++- fsmonitor.c | 5 ++--- read-cache.c| 1 + t/helper/test-read-cache.c | 24 +++- t/t7519-status-fsmonitor.sh | 8 5 files changed, 36 insertions(+), 5 deletions(-) Range-diff: 1: 51a7edf22a = 1: c31f834b07 fsmonitor: demonstrate that it is not refreshed after discard_index() 2: 79fdd0d586 ! 2: 7bf5f9f610 fsmonitor: force a refresh after the index was discarded @@ -6,8 +6,10 @@ flag that says whether the fsmonitor hook has been run, i.e. it is now per-index. -It also gets re-set when the index is discarded, fixing the bug where -fsmonitor-enabled Git would miss updates under certain circumstances. +It also gets re-set when the index is discarded, fixing the bug +demonstrated by the "test_expect_failure" test added in the preceding +commit. In that case fsmonitor-enabled Git would miss updates under +certain circumstances, see that preceding commit for details. Signed-off-by: Johannes Schindelin @@ -15,11 +17,11 @@ --- a/cache.h +++ b/cache.h @@ - struct cache_time timestamp; - unsigned name_hash_initialized : 1, initialized : 1, -- drop_cache_tree : 1; -+ drop_cache_tree : 1, +drop_cache_tree : 1, +updated_workdir : 1, +- updated_skipworktree : 1; ++ updated_skipworktree : 1, + fsmonitor_has_run_once : 1; struct hashmap name_hash; struct hashmap dir_hash; -- 2.21.0.593.g511ec345e18
[PATCH v3 0/2] allow checkout and branch to create branches on a merge base
Thanks again for the review, Junio. I've squashed 2/3 and 3/3 together and made that documentation change I was talking about earlier. --- Changes since v2: * Squashed 2/3 with 3/3 * Document merge base syntax for in git-checkout.txt Changes since v1: * Moved multiple `test_when_finished` calls that appeared in "reverse order" into one call that appears in the logical order * Made create_branch handle merge base revs instead of putting a hack into checkout Denton Liu (2): t2018: cleanup in current test branch: make create_branch accept a merge base rev Documentation/git-branch.txt | 6 +++- Documentation/git-checkout.txt | 4 +++ branch.c | 2 +- t/t2018-checkout-branch.sh | 56 ++ t/t3200-branch.sh | 14 ++--- 5 files changed, 50 insertions(+), 32 deletions(-) Range-diff against v2: 1: 9d04faf29d = 1: 9d04faf29d t2018: cleanup in current test 2: 5e8320cd80 < -: -- t2018: demonstrate checkout -b merge base bug 3: c91c7535a7 ! 2: bb25852740 branch: make create_branch accept a merge base rev @@ -41,6 +41,21 @@ Note that this will create the new branch, but it will not switch the working tree to it; use "git checkout " to switch to the + diff --git a/Documentation/git-checkout.txt b/Documentation/git-checkout.txt + --- a/Documentation/git-checkout.txt + +++ b/Documentation/git-checkout.txt +@@ + :: + The name of a commit at which to start the new branch; see + linkgit:git-branch[1] for details. Defaults to HEAD. +++ ++As a special case, you may use `"A...B"` as a shortcut for the ++merge base of `A` and `B` if there is exactly one merge base. You can ++leave out at most one of `A` and `B`, in which case it defaults to `HEAD`. + + :: + Tree to checkout from (when paths are given). If not specified, + diff --git a/branch.c b/branch.c --- a/branch.c +++ b/branch.c @@ -61,20 +76,29 @@ do_checkout branch2 ' --test_expect_failure 'checkout -b to a merge base' ' +test_expect_success 'checkout -b to a merge base' ' ++ test_when_finished " ++ git checkout branch1 && ++ test_might_fail git branch -D branch2" && ++ git checkout -b branch2 branch1... ++' ++ + test_expect_success 'checkout -b to a new branch, set to an explicit ref' ' test_when_finished " git checkout branch1 && - test_might_fail git branch -D branch2" && @@ do_checkout branch2 "" -B ' --test_expect_failure 'checkout -B to a merge base' ' +test_expect_success 'checkout -B to a merge base' ' - git checkout branch1 && ++ git checkout branch1 && ++ ++ git checkout -B branch2 branch1... ++' ++ + test_expect_success 'checkout -B to an existing branch from detached HEAD resets branch to HEAD' ' + git checkout $(git rev-parse --verify HEAD) && - git checkout -B branch2 branch1... diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh --- a/t/t3200-branch.sh -- 2.21.0.1000.g11cd861522
[PATCH v3 0/2] Server options when cloning
Thanks, Jonathan Nieder, for your review.
> Thanks for writing this. I'd be in favor of making this die().
> Compare what we already do in push:
>
> if (args->push_options && !push_options_supported)
> die(_("the receiving end does not support push options"));
Thanks for pointing out what "push" does, and done.
> What happens in the case of push with protocol v0, where server options
> are supported?
They are just sent to the pre-receive and post-receive hooks, according
to the "git push" documentation. I added a mention of the push option
behavior in the 2nd commit's message.
> For example:
>
> fatal: server options require protocol version 2 or later
> hint: see protocol.version in "git help config" for more details
Thanks - used your example (except I put the hint first, since the fatal
line is fatal).
> Should this use a static to also warn only once in the tag-catchup case
> you mentioned?
Since we're dying, this is no longer needed.
Jonathan Tan (2):
transport: warn if server options are unsupported
clone: send server options when using protocol v2
Documentation/fetch-options.txt | 3 ++-
Documentation/git-clone.txt | 8 +++
builtin/clone.c | 6 +
t/t5702-protocol-v2.sh | 41 +
transport.c | 10
5 files changed, 67 insertions(+), 1 deletion(-)
Range-diff against v2:
1: af3cc05324 ! 1: 63049081c9 transport: warn if server options are
unsupported
@@ -4,13 +4,13 @@
Server options were added in commit 5e3548ef16 ("fetch: send server
options when using protocol v2", 2018-04-24), supported only for
-protocol version 2. Add a warning if server options are specified for
-the user if a legacy protocol is used instead.
+protocol version 2. But if the user specifies server options, and the
+protocol version being used doesn't support them, the server options
are
+silently ignored.
-An effort is made to avoid printing the same warning more than once by
-clearing transport->server_options after the warning, but this does not
-fully avoid double-printing (for example, when backfulling tags using
-another fetch with a non-reusable transport).
+Teach any transport users to die instead in this situation, just like
+how "push" dies if push options are provided when the server doesn't
+support them.
Signed-off-by: Jonathan Tan
@@ -22,10 +22,11 @@
'
+test_expect_success 'warn if using server-option with ls-remote with
legacy protocol' '
-+ GIT_TEST_PROTOCOL_VERSION=0 git -c protocol.version=0 \
++ GIT_TEST_PROTOCOL_VERSION=0 test_must_fail git -c protocol.version=0 \
+ ls-remote -o hello -o world "file://$(pwd)/file_parent" master
2>err &&
+
-+ grep "Ignoring server options" err
++ grep "see protocol.version in" err &&
++ grep "server options require protocol version 2 or later" err
+'
test_expect_success 'clone with file:// using protocol v2' '
@@ -39,10 +40,11 @@
+
+ git init temp_child &&
+
-+ GIT_TEST_PROTOCOL_VERSION=0 git -C temp_child -c protocol.version=0 \
++ GIT_TEST_PROTOCOL_VERSION=0 test_must_fail git -C temp_child -c
protocol.version=0 \
+ fetch -o hello -o world "file://$(pwd)/file_parent" master
2>err &&
+
-+ grep "Ignoring server options" err
++ grep "see protocol.version in" err &&
++ grep "server options require protocol version 2 or later" err
+'
+
test_expect_success 'upload-pack respects config using protocol v2' '
@@ -56,10 +58,12 @@
return 0;
}
-+static void warn_server_options_unsupported(struct transport *transport)
++static void die_if_server_options(struct transport *transport)
+{
-+ warning(_("Ignoring server options because protocol version does not
support it"));
-+ transport->server_options = NULL;
++ if (!transport->server_options || !transport->server_options->nr)
++ return;
++ advise(_("see protocol.version in 'git help config' for more details"));
++ die(_("server options require protocol version 2 or later"));
+}
+
/*
@@ -69,7 +73,7 @@
break;
case protocol_v1:
case protocol_v0:
-+ warn_server_options_unsupported(transport);
++ die_if_server_options(transport);
get_remote_heads(&reader, &refs,
for_push ? REF_NORMAL : 0,
&data->extra_have,
@@ -77,7 +81,7 @@
break;
case protocol_v1:
case protocol_v0:
-+ warn_server_options_unsupported(transport);
++ die_if_server_options(transport);
refs = fetch_pack(&args, dat
[PATCH v3 0/2] tag: advise on recursive tagging
[ Sorry for the spam, I typoed the mailing list address the first time ] Hi all, I've been following the discussion and the tl;dr of it seems to be this: Junio wants to keep the long-standing behaviour of being able to git tag anything while Robert seems to believe that for most users, it is a mistake and even though Git is able unpeel tags, other tool authors may not be aware that this is even possible and not handle the case. I've tried to compromise between both sides. Now, we keep the old behaviour but instead of silently ignoring nested tags, we print out a hint that the user can act on. Hopefully, this should address everyone's concerns. The added benefit is that this won't break any existing scripts' unless they are parsing stderr for whatever reason, but no one would do that, right? ;) Denton Liu (2): tag: fix formatting tag: advise on nested tags Documentation/config/advice.txt | 2 ++ advice.c| 2 ++ advice.h| 1 + builtin/tag.c | 23 +-- t/t7004-tag.sh | 11 +++ 5 files changed, 33 insertions(+), 6 deletions(-) -- 2.21.0.843.gd0ae0373aa
[PATCH v3 0/2] Last big GIT_TEST_PROTOCOL_VERSION=2 fix, hopefully
Peff says in [1]:
> But isn't this line:
>
> > + if (version == protocol_v2) {
> > +- if (shallow && shallow->nr)
> > ++ if (shallow->nr)
> > BUG("Protocol V2 does not provide shallows at
> > this point in the fetch");
>
> added by patch 1? It's added with "shallow &&" in patch 1, and then
> modified in patch 2.
>
> I think the "it's never NULL" property is true even before this series,
> right?
Ah...yes you're right. I've updated it here.
Thanks for your review.
[1] https://public-inbox.org/git/20190326182047.gb24...@sigill.intra.peff.net/
Jonathan Tan (2):
fetch-pack: call prepare_shallow_info only if v0
fetch-pack: respect --no-update-shallow in v2
commit.h | 4
fetch-pack.c | 51 +--
2 files changed, 45 insertions(+), 10 deletions(-)
Range-diff against v2:
1: d2eb101709 ! 1: 64f44a18ad fetch-pack: call prepare_shallow_info only if v0
@@ -38,7 +38,7 @@
- prepare_shallow_info(&si, shallow);
- if (version == protocol_v2)
+ if (version == protocol_v2) {
-+ if (shallow && shallow->nr)
++ if (shallow->nr)
+ BUG("Protocol V2 does not provide shallows at this
point in the fetch");
+ memset(&si, 0, sizeof(si));
ref_cpy = do_fetch_pack_v2(args, fd, ref, sought, nr_sought,
2: 943b1cbc61 ! 2: 3f65698610 fetch-pack: respect --no-update-shallow in v2
@@ -120,11 +120,6 @@
fetch_pack_setup();
if (nr_sought)
@@
- die(_("no matching remote head"));
- }
- if (version == protocol_v2) {
-- if (shallow && shallow->nr)
-+ if (shallow->nr)
BUG("Protocol V2 does not provide shallows at this
point in the fetch");
memset(&si, 0, sizeof(si));
ref_cpy = do_fetch_pack_v2(args, fd, ref, sought, nr_sought,
--
2.21.0.155.ge902e9bcae.dirty
[PATCH v3 0/2] consolidate core.excludesFile docs
Robert reported that core.excludesFile was not mentioned in the git-clean docs[1]. This cleans that up by mentioning that in the docs. [1]: https://public-inbox.org/git/alpine.LFD.2.21.1902231328560.@localhost.localdomain/ Changes since v1: * Use Martin's suggestions of referencing the gitignore(5) instead of writing an exhaustive list. Also, sprinkle in some backticks ;) Changes since v2: * Use Junio's suggestion for rephrasing the text in git-clean.txt * Move core.excludesFile specific documentation out of git-add.txt and into gitignore.txt Denton Liu (2): git-clean.txt: clarify ignore pattern files Docs: move core.excludesFile from git-add to gitignore Documentation/git-add.txt | 9 - Documentation/git-clean.txt | 11 +-- Documentation/gitignore.txt | 8 3 files changed, 13 insertions(+), 15 deletions(-) -- 2.21.0.368.gbf248417d7
Re: [PATCH v3 0/2] Fix regression in checkout -b
Ben Peart writes: > From: Ben Peart > > Minor update to comment from V2. Also wrapped commit messages to be <80 > chars wide. Perfect. Thanks.
[PATCH v3 0/2] Fix regression in checkout -b
From: Ben Peart Minor update to comment from V2. Also wrapped commit messages to be <80 chars wide. Base Ref: master Web-Diff: https://github.com/benpeart/git/commit/fef76edbdc Checkout: git fetch https://github.com/benpeart/git initial-checkout-v3 && git checkout fef76edbdc ### Interdiff (v2..v3): diff --git a/builtin/checkout.c b/builtin/checkout.c index 9c6e94319e..9f8f3466f6 100644 --- a/builtin/checkout.c +++ b/builtin/checkout.c @@ -593,8 +593,9 @@ static int skip_merge_working_tree(const struct checkout_opts *opts, */ /* - * Do the merge if this is the initial checkout - * + * Do the merge if this is the initial checkout. We cannot use + * is_cache_unborn() here because the index hasn't been loaded yet + * so cache_nr and timestamp.sec are always zero. */ if (!file_exists(get_index_file())) return 0; ### Patches Ben Peart (2): checkout: add test demonstrating regression with checkout -b on initial commit checkout: fix regression in checkout -b on intitial checkout builtin/checkout.c | 8 t/t2018-checkout-branch.sh | 9 + 2 files changed, 17 insertions(+) base-commit: 77556354bb7ac50450e3b28999e3576969869068 -- 2.19.1.gvfs.1.16.g9d1374d
[PATCH v3 0/2] setup: fix memory leaks with `struct repository_format`
On Tue, 22 Jan 2019 at 14:34, Martin Ågren wrote: > > On Tue, 22 Jan 2019 at 08:07, Jeff King wrote: > > > For the record, I can live with it either way. There are so many funky > > little setup corner cases in the code already, and we don't even really > > have a real-world case to dissect at this point. So the right thing may > > also just be to finish this patch series as quickly as possible and move > > on to something more useful. :) > > I rebased the "something like this?" into this series yesterday and I > think the end result is better, but also that the way there is clearer, > mostly because this patch is then gone. I wanted to double-check it > tonight and submit it. I'll do that tonight. Here's that reroll. I now reset the entire struct in the error path of `clear_...()`. Thus, the user that is reading `repo_fmt.hash_algo` despite not being supposed to, can keep reading it, now knowing that the value has a default value. I also expanded on the documentation a little to point out that we'll reset to the default struct state if we don't find any "core.repositoryformatversion". Martin Martin Ågren (2): setup: free old value before setting `work_tree` setup: fix memory leaks with `struct repository_format` cache.h | 27 --- builtin/init-db.c | 3 ++- repository.c | 3 ++- setup.c | 33 + worktree.c| 4 +++- 5 files changed, 52 insertions(+), 18 deletions(-) -- 2.20.1.98.gecbdaf0899
Re: [PATCH v3 0/2] support for filtering trees and blobs based on depth
Matthew DeVore writes: > This seems like a easier problem to understand, but I'm not sure how > to avoid this issue in the future. Sorry---I was mostly venting and it was not productive. There isn't much individual contributors can do by themselves, other than choosing the right place to base their topics on and communicate it accurately when sending the patches to the list. I think I made sure that all the topics in master..pu that have tricky dependencies are at least buildable (which involved a few topics to be rebased on the right commit, sometimes rebuilding the base that is a merge of a few topics in flight), so hopefully we are in good shape now. Thanks.
Re: [PATCH v3 0/2] support for filtering trees and blobs based on depth
On 2019/01/15 15:41, Junio C Hamano wrote: Junio C Hamano writes: This is turning out to be messier than I like. The topic is tangled with too many things in flight and I think I reduced its dependencies down to nd/the-index and sb/more-repo-in-api plus then-current tip of master (and that is why it is based on a1411cecc7), but it seems that it wants a bit more than that; builtin/rebase.c at its tip does not even compile, so I'll need to wiggle the topic before it can go to 'next'. Half false alarm. I do need to wiggle the topic, but that was not because the choice of base was bad. It was that nd/the-index plus sb/more-repo-in-api had semantic merge conflicts with the then-current master. If I understand right, this is a product of the fact that you had to merge these branches together and base my change on top of them, and that is a result of that fact that I didn't work on top of master for the first iterations of the patch. Sorry about that. My last re-roll was based on master (commit 77556354) but I guess before I sent that version of the patch set I had already done some damage by working off of next for the earlier patches. I think my last version of the patch was fine since it was based off master. Let me know if I've misunderstood. And worse yet, it seems that filter-options-should-use-plain-int topic depends on this topic in turn as it wants to use LOFC_TREEE_DEPTH. This part is still true. The scaling-factor-over-the-wire topic does need to be rebuilt on top of this one. This seems like a easier problem to understand, but I'm not sure how to avoid this issue in the future. Thanks, Matt
Re: [PATCH v3 0/2] support for filtering trees and blobs based on depth
Junio C Hamano writes: > This is turning out to be messier than I like. > > The topic is tangled with too many things in flight and I think I > reduced its dependencies down to nd/the-index and > sb/more-repo-in-api plus then-current tip of master (and that is why > it is based on a1411cecc7), but it seems that it wants a bit more > than that; builtin/rebase.c at its tip does not even compile, so > I'll need to wiggle the topic before it can go to 'next'. Half false alarm. I do need to wiggle the topic, but that was not because the choice of base was bad. It was that nd/the-index plus sb/more-repo-in-api had semantic merge conflicts with the then-current master. > And worse yet, it seems that filter-options-should-use-plain-int > topic depends on this topic in turn as it wants to use > LOFC_TREEE_DEPTH. This part is still true. The scaling-factor-over-the-wire topic does need to be rebuilt on top of this one.
Re: [PATCH v3 0/2] support for filtering trees and blobs based on depth
Jonathan Tan writes: >> This applies suggestions from Jonathan Tan and Junio. These are mostly >> stylistic and readability changes, although there is also an added test case >> in t/t6112-rev-list-filters-objects.sh which checks for the scenario when >> filtering which would exclude a blob, but the blob is given on the command >> line. >> >> This has been rebased onto master, while the prior version was based on next. >> >> Thank you, > > Thanks, these 2 patches are Reviewed-by: me. > > Your approach in the 2nd patch makes more sense, and I checked that both > oidset_insert() and oidset_remove() return 1 when the element in > question was in the set (prior to invocation of the function), so that > works. This is turning out to be messier than I like. The topic is tangled with too many things in flight and I think I reduced its dependencies down to nd/the-index and sb/more-repo-in-api plus then-current tip of master (and that is why it is based on a1411cecc7), but it seems that it wants a bit more than that; builtin/rebase.c at its tip does not even compile, so I'll need to wiggle the topic before it can go to 'next'. And worse yet, it seems that filter-options-should-use-plain-int topic depends on this topic in turn as it wants to use LOFC_TREEE_DEPTH.
Re: [PATCH v3 0/2] support for filtering trees and blobs based on depth
> This applies suggestions from Jonathan Tan and Junio. These are mostly > stylistic and readability changes, although there is also an added test case > in t/t6112-rev-list-filters-objects.sh which checks for the scenario when > filtering which would exclude a blob, but the blob is given on the command > line. > > This has been rebased onto master, while the prior version was based on next. > > Thank you, Thanks, these 2 patches are Reviewed-by: me. Your approach in the 2nd patch makes more sense, and I checked that both oidset_insert() and oidset_remove() return 1 when the element in question was in the set (prior to invocation of the function), so that works.
[PATCH v3 0/2] support for filtering trees and blobs based on depth
This applies suggestions from Jonathan Tan and Junio. These are mostly stylistic and readability changes, although there is also an added test case in t/t6112-rev-list-filters-objects.sh which checks for the scenario when filtering which would exclude a blob, but the blob is given on the command line. This has been rebased onto master, while the prior version was based on next. Thank you, Matthew DeVore (2): list-objects-filter: teach tree:# how to handle >0 tree:: skip some trees even when collecting omits Documentation/rev-list-options.txt | 9 +- list-objects-filter-options.c | 7 +- list-objects-filter-options.h | 3 +- list-objects-filter.c | 122 +++- t/t6112-rev-list-filters-objects.sh | 122 +++- 5 files changed, 235 insertions(+), 28 deletions(-) -- 2.20.1.97.g81188d93c3-goog
[PATCH v3 0/2] completion: handle paths with spaces correctly
I found more issues with completion for git show ref:path, so here I add one more patch. Chayoung You (2): zsh: complete unquoted paths with spaces correctly completion: treat results of git ls-tree as file paths contrib/completion/git-completion.bash | 35 +++--- contrib/completion/git-completion.zsh | 4 +-- t/t9902-completion.sh | 10 3 files changed, 21 insertions(+), 28 deletions(-) -- 2.17.1
[PATCH v3 0/2] range-diff: doc + regression fix
As Johannes notes this --no-patch option I wanted to add is something we had already, but is it turns out it was broken. So this is an entirely rewritten v3 (not bothering with the range-diff for it) which a) documents the output stability of stuff like --stat and the like (there isn't any) b) fixes the regression & adds a test. I did try various other diff options and they all seem to work. Ævar Arnfjörð Bjarmason (2): range-diff doc: add a section about output stability range-diff: fix regression in passing along diff options Documentation/git-range-diff.txt | 17 + range-diff.c | 3 ++- t/t3206-range-diff.sh| 30 ++ 3 files changed, 49 insertions(+), 1 deletion(-) -- 2.19.1.930.g4563a0d9d0
Re: [PATCH v3 0/2] rebase -i: introduce the 'break' command
Hi Junio, On Fri, 12 Oct 2018, Junio C Hamano wrote: > "Johannes Schindelin via GitGitGadget" > writes: > > > This patch introduces that, based on ag/rebase-i-in-c. > > > > Changes since v2: > > > > * Fixed two typos. > > * When interrupting the rebase, break now outputs a message. > > I was about to merge the whole collection of topics to 'next', but > this came to stop me just in time. > > The typofixes are appreciated of course, and look trivially correct. > > I find that the if() condition that does too many side-effect-full > operations in stopped-at-head shows bad taste. It is still short > enough to understand what each step is trying to do, but would > encourage others who later may touch the code to make it even more > complex. > > But it is a short and isolated static helper function, so I won't > complain too loudly. > > Will replace and requeue. Thanks, Dscho > > Thanks. >
Re: [PATCH v3 0/2] rebase -i: introduce the 'break' command
"Johannes Schindelin via GitGitGadget" writes: > This patch introduces that, based on ag/rebase-i-in-c. > > Changes since v2: > > * Fixed two typos. > * When interrupting the rebase, break now outputs a message. I was about to merge the whole collection of topics to 'next', but this came to stop me just in time. The typofixes are appreciated of course, and look trivially correct. I find that the if() condition that does too many side-effect-full operations in stopped-at-head shows bad taste. It is still short enough to understand what each step is trying to do, but would encourage others who later may touch the code to make it even more complex. But it is a short and isolated static helper function, so I won't complain too loudly. Will replace and requeue. Thanks.
[PATCH v3 0/2] rebase -i: introduce the 'break' command
Stefan asked a while back
[https://public-inbox.org/git/20180118183618.39853-3-sbel...@google.com/]
for a todo command in interactive rebases that would essentially perform the
"stop" part of the edit command, but without the "pick" part: interrupt the
interactive rebase, with exit code 0, letting the user do things and stuff
and look around, with the idea of continuing the rebase later (using git
rebase --continue).
This patch introduces that, based on ag/rebase-i-in-c.
Changes since v2:
* Fixed two typos.
* When interrupting the rebase, break now outputs a message.
Changes since v1:
* Wrapped the commit message correctly.
* Added a preparatory patch to mention what happens in case an exec fails.
* Mentioned the break command in the git-rebase(1) documentation.
Cc: Stefan Beller sbel...@google.com [sbel...@google.com]Cc: Eric Sunshine
sunsh...@sunshineco.com [sunsh...@sunshineco.com]
Johannes Schindelin (2):
rebase -i: clarify what happens on a failed `exec`
rebase -i: introduce the 'break' command
Documentation/git-rebase.txt | 6 +-
rebase-interactive.c | 1 +
sequencer.c | 24 +++-
t/lib-rebase.sh | 2 +-
t/t3418-rebase-continue.sh | 9 +
5 files changed, 39 insertions(+), 3 deletions(-)
base-commit: 34b47315d9721a576b9536492cca0c11588113a2
Published-As:
https://github.com/gitgitgadget/git/releases/tags/pr-43%2Fdscho%2Frebase-i-break-v3
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git
pr-43/dscho/rebase-i-break-v3
Pull-Request: https://github.com/gitgitgadget/git/pull/43
Range-diff vs v2:
1: 2eefdb4874 ! 1: 92512a82d2 rebase -i: clarify what happens on a failed
`exec`
@@ -15,8 +15,8 @@
Append "exec " after each line creating a commit in the
final history. will be interpreted as one or more shell
- commands.
-+ commands. Anz command that fails will interrupt the rebase,
-+ withe exit code 1.
++ commands. Any command that fails will interrupt the rebase,
++ with exit code 1.
+
You may execute several commands by either using one instance of `--exec`
with several commands:
2: c74e02c4b6 ! 2: d44b425709 rebase -i: introduce the 'break' command
@@ -71,13 +71,37 @@
if (bol != eol)
return error(_("%s does not accept arguments: '%s'"),
command_to_string(item->command), bol);
+@@
+ return update_ref(NULL, "ORIG_HEAD", &oid, NULL, 0,
UPDATE_REFS_MSG_ON_ERR);
+ }
+
++static int stopped_at_head(void)
++{
++ struct object_id head;
++ struct commit *commit;
++ struct commit_message message;
++
++ if (get_oid("HEAD", &head) || !(commit = lookup_commit(&head)) ||
++ parse_commit(commit) || get_message(commit, &message))
++ fprintf(stderr, _("Stopped at HEAD\n"));
++ else {
++ fprintf(stderr, _("Stopped at %s\n"), message.label);
++ free_message(commit, &message);
++ }
++ return 0;
++
++}
++
+ static const char rescheduled_advice[] =
+ N_("Could not execute the todo command\n"
+ "\n"
@@
unlink(rebase_path_stopped_sha());
unlink(rebase_path_amend());
delete_ref(NULL, "REBASE_HEAD", NULL, REF_NO_DEREF);
+
+ if (item->command == TODO_BREAK)
-+ break;
++ return stopped_at_head();
}
if (item->command <= TODO_SQUASH) {
if (is_rebase_i(opts))
--
gitgitgadget
Re: [PATCH v3 0/2] EditorConfig file
On Mon, Oct 08, 2018 at 10:03:51PM +, brian m. carlson wrote: > This series introduces an EditorConfig file to help developers using any > editor set their editor's settings in conformance with the Git Project's > settings. This is helpful for developers who work on different projects > with different indentation standards to keep their work in sync. > > Changes since v2: > * Add .pl and .pm files. > > Changes since v1: > * Add notes to both .editorconfig and .clang-format that they should be > kept in sync. > * Add commit message line length. Thanks for both of these. I think that v3 is ready for queueing, if other folks find it OK to have an .editorconfig in the repository. Therefore: Reviewed-by: Taylor Blau Thanks, Taylor
[PATCH v3 0/2] EditorConfig file
This series introduces an EditorConfig file to help developers using any editor set their editor's settings in conformance with the Git Project's settings. This is helpful for developers who work on different projects with different indentation standards to keep their work in sync. Changes since v2: * Add .pl and .pm files. Changes since v1: * Add notes to both .editorconfig and .clang-format that they should be kept in sync. * Add commit message line length. brian m. carlson (2): editorconfig: provide editor settings for Git developers editorconfig: indicate settings should be kept in sync .clang-format | 2 ++ .editorconfig | 16 2 files changed, 18 insertions(+) create mode 100644 .editorconfig
[PATCH v3 0/2] Per-worktree config files
v3 changes are minor (besides test_cmp_config), mostly document
cleanup. Diff
diff --git a/Documentation/config.txt b/Documentation/config.txt
index 44407e69db..e036ff7b86 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -4,7 +4,7 @@ CONFIGURATION FILE
The Git configuration file contains a number of variables that affect
the Git commands' behavior. The files `.git/config` and optionally
`config.worktree` (see `extensions.worktreeConfig` below) in each
-repository is used to store the configuration for that repository, and
+repository are used to store the configuration for that repository, and
`$HOME/.gitconfig` is used to store a per-user configuration as
fallback values for the `.git/config` file. The file `/etc/gitconfig`
can be used to store a system-wide default configuration.
diff --git a/Documentation/git-worktree.txt b/Documentation/git-worktree.txt
index aa88278dde..408c87c9ef 100644
--- a/Documentation/git-worktree.txt
+++ b/Documentation/git-worktree.txt
@@ -221,7 +221,7 @@ $ git config extensions.worktreeConfig true
In this mode, specific configuration stays in the path pointed by `git
rev-parse --git-path config.worktree`. You can add or update
configuration in this file with `git config --worktree`. Older Git
-versions may will refuse to access repositories with this extension.
+versions will refuse to access repositories with this extension.
Note that in this file, the exception for `core.bare` and `core.worktree`
is gone. If you have them in $GIT_DIR/config before, you must move
@@ -283,6 +283,9 @@ to `/path/main/.git/worktrees/test-next` then a file named
`test-next` entry from being pruned. See
linkgit:gitrepository-layout[5] for details.
+When extensions.worktreeConfig is enabled, the config file
+`.git/worktrees//config.worktree` is read after `.git/config` is.
+
LIST OUTPUT FORMAT
--
The worktree list command has two output formats. The default format shows the
diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh
index 4cd7fb8fdf..2149b88392 100644
--- a/t/test-lib-functions.sh
+++ b/t/test-lib-functions.sh
@@ -747,28 +747,27 @@ test_cmp() {
$GIT_TEST_CMP "$@"
}
-# similar to test_cmp but $2 is a config key instead of actual value
-# it can also accept -C to read from a different repo, e.g.
+# Check that the given config key has the expected value.
#
-# test_cmp_config -C xyz foo core.bar
+#test_cmp_config [-C ]
+#[...]
#
-# is sort of equivalent of
+# for example to check that the value of core.bar is foo
+#
+#test_cmp_config foo core.bar
#
-# test "foo" = "$(git -C xyz core.bar)"
-
test_cmp_config() {
- if [ "$1" = "-C" ]
+ local GD
+ if test "$1" = "-C"
then
shift &&
GD="-C $1" &&
shift
- else
- GD=
fi &&
- echo "$1" >expected &&
+ printf "%s\n" "$1" >expect.config &&
shift &&
- git $GD config "$@" >actual &&
- test_cmp expected actual
+ git $GD config "$@" >actual.config &&
+ test_cmp expect.config actual.config
}
# test_cmp_bin - helper to compare binary files
Nguyễn Thái Ngọc Duy (2):
t1300: extract and use test_cmp_config()
worktree: add per-worktree config files
Documentation/config.txt | 12 +++-
Documentation/git-config.txt | 26 ++---
Documentation/git-worktree.txt | 33 +++
Documentation/gitrepository-layout.txt | 8 +++
builtin/config.c | 19 ++-
cache.h| 2 +
config.c | 11
environment.c | 1 +
setup.c| 40 ++---
t/t1300-config.sh | 79 +++---
t/t2029-worktree-config.sh | 79 ++
t/test-lib-functions.sh| 23
12 files changed, 255 insertions(+), 78 deletions(-)
create mode 100755 t/t2029-worktree-config.sh
--
2.19.0.342.gc057aaf40a.dirty
[PATCH v3 0/2] Properly peel tags in can_all_from_reach_with_flags()
As Peff reported [1], the refactored can_all_from_reach_with_flags() method
does not properly peel tags. Since the helper method can_all_from_reach()
and code in t/helper/test-reach.c all peel tags before getting to this
method, it is not super-simple to create a test that demonstrates this.
I modified t/helper/test-reach.c to allow calling
can_all_from_reach_with_flags() directly, and added a test in
t6600-test-reach.sh that demonstrates the segfault without the fix.
For V2, I compared the loop that inspects the 'from' commits in commit
ba3ca1edce "commit-reach: move can_all_from_reach_with_flags" to the version
here and got the following diff:
3c3
< if (from_one->flags & assign_flag)
---
> if (!from_one || from_one->flags & assign_flag)
5c5,7
< from_one = deref_tag(the_repository, from_one, "a from
object", 0);
---
>
> from_one = deref_tag(the_repository, from_one,
> "a from object", 0);
14a17,22
>
> list[nr_commits] = (struct commit *)from_one;
> if (parse_commit(list[nr_commits]) ||
> list[nr_commits]->generation < min_generation)
> return 0; /* is this a leak? */
> nr_commits++;
This diff includes the early termination we had before 'deref_tag' and the
comment for why we can ignore non-commit objects.
[1]
https://public-inbox.org/git/0bf9103c-9377-506b-7ad7-e5273d8e9...@gmail.com/T/#u
Derrick Stolee (2):
commit-reach: properly peel tags
commit-reach: fix memory and flag leaks
commit-reach.c| 41 ++---
t/helper/test-reach.c | 22 +-
t/t6600-test-reach.sh | 30 --
3 files changed, 79 insertions(+), 14 deletions(-)
base-commit: 6621c838743812aaba96e55cfec8524ea1144c2d
Published-As:
https://github.com/gitgitgadget/git/releases/tags/pr-39%2Fderrickstolee%2Ftag-fix-v3
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git
pr-39/derrickstolee/tag-fix-v3
Pull-Request: https://github.com/gitgitgadget/git/pull/39
Range-diff vs v2:
1: 4bf21204dd ! 1: 0a1e661271 commit-reach: properly peel tags
@@ -53,8 +53,11 @@
+
+ list[nr_commits] = (struct commit *)from_one;
+ if (parse_commit(list[nr_commits]) ||
-+ list[nr_commits]->generation < min_generation)
-+ return 0; /* is this a leak? */
++ list[nr_commits]->generation < min_generation) {
++ result = 0;
++ goto cleanup;
++ }
++
+ nr_commits++;
}
-: -- > 2: b2e0ee4978 commit-reach: fix memory and flag leaks
--
gitgitgadget
[PATCH v3 0/2] commit-graph: add progress output
Micro change since v2: Missing _() in 2/2 pointed out by Duy. Ævar Arnfjörð Bjarmason (2): commit-graph write: add progress output commit-graph verify: add progress output builtin/commit-graph.c | 5 ++-- builtin/gc.c | 3 +- commit-graph.c | 65 -- commit-graph.h | 5 ++-- 4 files changed, 65 insertions(+), 13 deletions(-) -- 2.19.0.rc2.392.g5ba43deb5a
[PATCH v3 0/2] Fixup for js/mingw-o-append
The recent change mingw O_APPEND change breaks writing to named pipes on
Windows. The first commit adds a new test to confirm the breakage and the
second commit fixes the problem. These could be squashed together or we can
just keep the fix and omit the test if that would be better.
d641097589 (js/mingw-o-append) mingw: enable atomic O_APPEND
The new mingw_open_append() routine successfully opens the client side of
the named pipe, but the first write() to it fails with EBADF. Adding the
FILE_WRITE_DATA corrects the problem.
Signed-off-by: Jeff Hostetler jeffh...@microsoft.com
[jeffh...@microsoft.com]
Cc: j6t@kdbg.orgCc: johannes.schinde...@gmx.decc: gitster@pobox.comCc:
p...@peff.net
Jeff Hostetler (2):
t0051: test GIT_TRACE to a windows named pipe
mingw: fix mingw_open_append to work with named pipes
Makefile | 1 +
compat/mingw.c | 36 +--
t/helper/test-tool.c | 3 ++
t/helper/test-tool.h | 3 ++
t/helper/test-windows-named-pipe.c | 72 ++
t/t0051-windows-named-pipe.sh | 17 +++
6 files changed, 129 insertions(+), 3 deletions(-)
create mode 100644 t/helper/test-windows-named-pipe.c
create mode 100755 t/t0051-windows-named-pipe.sh
base-commit: d641097589160eb795127d8dbcb14c877c217b60
Published-As:
https://github.com/gitgitgadget/git/releases/tags/pr-35%2Fjeffhostetler%2Ffixup-mingw-o-append-v3
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git
pr-35/jeffhostetler/fixup-mingw-o-append-v3
Pull-Request: https://github.com/gitgitgadget/git/pull/35
Range-diff vs v2:
1: ecb30eb47c = 1: ecb30eb47c t0051: test GIT_TRACE to a windows named pipe
2: f0361dd306 ! 2: 5592300ca5 mingw: fix mingw_open_append to work with
named pipes
@@ -46,22 +46,20 @@
return fd;
}
-+#define IS_SBS(ch) (((ch) == '/') || ((ch) == '\\'))
+/*
+ * Does the pathname map to the local named pipe filesystem?
+ * That is, does it have a "//./pipe/" prefix?
+ */
-+static int mingw_is_local_named_pipe_path(const char *filename)
++static int is_local_named_pipe_path(const char *filename)
+{
-+ return (IS_SBS(filename[0]) &&
-+ IS_SBS(filename[1]) &&
++ return (is_dir_sep(filename[0]) &&
++ is_dir_sep(filename[1]) &&
+ filename[2] == '.' &&
-+ IS_SBS(filename[3]) &&
++ is_dir_sep(filename[3]) &&
+ !strncasecmp(filename+4, "pipe", 4) &&
-+ IS_SBS(filename[8]) &&
++ is_dir_sep(filename[8]) &&
+ filename[9]);
+}
-+#undef IS_SBS
+
int mingw_open (const char *filename, int oflags, ...)
{
@@ -71,7 +69,7 @@
filename = "nul";
- if (oflags & O_APPEND)
-+ if ((oflags & O_APPEND) && !mingw_is_local_named_pipe_path(filename))
++ if ((oflags & O_APPEND) && !is_local_named_pipe_path(filename))
open_fn = mingw_open_append;
else
open_fn = _wopen;
--
gitgitgadget
[PATCH v3 0/2] Make git rebase work with --rebase-merges and --exec
It was reported via IRC that the exec lines are inserted in the wrong spots when using --rebase-merges. The reason is that we used a simple, incorrect implementation that happened to work as long as the generated todo list only contains pick, fixup and squash commands. Which is not the case with--rebase-merges. Fix this issue by using a correct implementation instead, that even takes into account merge commands in the --rebase-merges mode. Changes since v1: * Replaced the "look-ahead" design by a "keep looking" one: instead of having a nested loop that looks for the end of the fixup/squash chain, we continue the loop, delaying the insertion until we know where the fixup/squash chain ends, if any. Johannes Schindelin (2): t3430: demonstrate what -r, --autosquash & --exec should do rebase --exec: make it work with --rebase-merges sequencer.c | 42 +--- t/t3430-rebase-merges.sh | 17 2 files changed, 48 insertions(+), 11 deletions(-) base-commit: 1d89318c48d233d52f1db230cf622935ac3c69fa Published-As: https://github.com/gitgitgadget/git/releases/tags/pr-13%2Fdscho%2Frebase-merges-and-exec-commands-v3 Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-13/dscho/rebase-merges-and-exec-commands-v3 Pull-Request: https://github.com/gitgitgadget/git/pull/13 Range-diff vs v2: 1: 1d82eb450 = 1: 1d82eb450 t3430: demonstrate what -r, --autosquash & --exec should do 2: 7ca441a89 ! 2: b436f67ba rebase --exec: make it work with --rebase-merges @@ -22,6 +22,11 @@ `pick` lines, skip any fixup/squash chains, and then insert the `exec` line. Lather, rinse, repeat. +Note: we take pains to insert *before* comment lines whenever possible, +as empty commits are represented by commented-out pick lines (and we +want to insert a preceding pick's exec line *before* such a line, not +afterward). + While at it, also add `exec` lines after `merge` commands, because they are similar in spirit to `pick` commands: they add new commits. @@ -81,9 +86,13 @@ + insert = i + 1; } - /* append final */ +- /* append final */ - strbuf_add(buf, commands, commands_len); -+ if (insert >= 0 || !offset) ++ /* insert or append final */ ++ if (insert >= 0 && insert < todo_list.nr) ++ strbuf_insert(buf, todo_list.items[insert].offset_in_buf + ++ offset, commands, commands_len); ++ else if (insert >= 0 || !offset) + strbuf_add(buf, commands, commands_len); i = write_message(buf->buf, buf->len, todo_file, 0); -- gitgitgadget
[PATCH v3 0/2] Fix author script quoting
From: Phillip Wood Thanks to Eric for his comments on v2. The first patch hasn't changed much, the second one quite a bit. See the individual patches for a list of changes Phillip Wood (2): sequencer: handle errors in read_author_ident() sequencer: fix quoting in write_author_script git-rebase--interactive.sh| 2 +- sequencer.c | 137 ++ sequencer.h | 1 + t/t3404-rebase-interactive.sh | 20 - 4 files changed, 123 insertions(+), 37 deletions(-) -- 2.18.0
[PATCH v3 0/2] Address recovery failures with directory/file conflicts
This patch series fixes several "recovery" commands that outright fail or do not fully recover when directory-file conflicts are present. This includes: * git read-tree --reset HEAD * git am --skip * git am --abort * git merge --abort (or git reset --merge) * git reset --hard Changes since v2 (full range-diff below): - Backported to maint (there were some textual conflicts in t6042 due to the merging of en/merge-recursive-tests to master), because of this comment from Junio's what's cooking email: "This may have to be rebased on an older maintenance track before moving forward." Elijah Newren (2): t1015: demonstrate directory/file conflict recovery failures read-cache: fix directory/file conflict handling in read_index_unmerged() read-cache.c | 13 +-- t/t1015-read-index-unmerged.sh | 123 +++ t/t6020-merge-df.sh | 3 - t/t6042-merge-rename-corner-cases.sh | 2 - 4 files changed, 131 insertions(+), 10 deletions(-) create mode 100755 t/t1015-read-index-unmerged.sh 1: 4a1c9c3368 ! 1: 00f94a8b41 t1015: demonstrate directory/file conflict recovery failures @@ -14,7 +14,6 @@ Signed-off-by: Elijah Newren Signed-off-by: Junio C Hamano -Message-Id: <20180713163331.22446-2-new...@gmail.com> diff --git a/t/t1015-read-index-unmerged.sh b/t/t1015-read-index-unmerged.sh new file mode 100755 2: e105e8bfbd ! 2: d3b8d7edb6 read-cache: fix directory/file conflict handling in read_index_unmerged() @@ -59,7 +59,6 @@ Signed-off-by: Elijah Newren Signed-off-by: Junio C Hamano -Message-Id: <20180713163331.22446-3-new...@gmail.com> diff --git a/read-cache.c b/read-cache.c --- a/read-cache.c @@ -150,10 +149,18 @@ --- a/t/t6042-merge-rename-corner-cases.sh +++ b/t/t6042-merge-rename-corner-cases.sh @@ - ( - cd rename-directory-1 && + ' + + test_expect_success 'rename/directory conflict + clean content merge' ' +- git reset --hard && + git reset --hard && + git clean -fdqx && -- git reset --hard && - git reset --hard && - git clean -fdqx && +@@ + ' + + test_expect_success 'rename/directory conflict + content merge conflict' ' +- git reset --hard && + git reset --hard && + git clean -fdqx && -- 2.18.0.2.gf4c50c7885
Re: [PATCH v3 0/2] grep.c: teach --only-matching to 'git-grep(1)'
Jeff King writes: > FWIW, I like the GNU phrasing. I thought the "non-empty" part was not > all that interesting, but after hearing that BSD behaves differently, it > is probably worth mentioning. > > I think the actual behavior of your patch matches GNU grep, and does not > need changing. Great ;-)
Re: [PATCH v3 0/2] grep.c: teach --only-matching to 'git-grep(1)'
On Fri, Jul 06, 2018 at 03:15:22PM -0500, Taylor Blau wrote: > On Fri, Jul 06, 2018 at 11:21:06AM -0700, Junio C Hamano wrote: > > Taylor Blau writes: > > > > > I think that this might be clear enough on its own, especially since > > > this is the same as BSD grep on my machine. I think that part_s_ of a > > > line indicates that behavior, but perhaps not. On GNU grep, this is: > > > > > > Print only the matched (non-empty) parts of a matching line, with each > > > such part on a separate output line. > > > > Interesting. I wonder what "git grep -o '^'" would do ;-) > > That invocation prints nothing, but on BSD grep it prints quite a few > blank lines :-). > > I'm hesitant on sending a patch per the hunk of your reply below because > of this. Should we mirror BSD grep's behavior exactly here? I suppose > that we could somehow, but it seems like we might be doing too much to > support what appears to me to be an odd use-case. IMHO the GNU behavior (omitting non-empty matches) makes more sense. And it's also what your patch already does. ;) Although amusingly "git grep -o ^" will still print a ton of "Binary file ... matches". That _also_ matches what GNU grep does. I'm not sure if there's a saner behavior (it really has nothing to do with the funny empty match; any binary file with -o cannot show the normal text line). > > In any case, I find that the GNU phrasing is the most clear among > > the ones I've seen in this thread so far. > > OK. I'm happy to re-send that patch with the GNU phrasing depending on > what others think (and the above). I'll let this cook and collect some > thoughts over the weekend. FWIW, I like the GNU phrasing. I thought the "non-empty" part was not all that interesting, but after hearing that BSD behaves differently, it is probably worth mentioning. I think the actual behavior of your patch matches GNU grep, and does not need changing. -Peff
Re: [PATCH v3 0/2] grep.c: teach --only-matching to 'git-grep(1)'
On Fri, Jul 06, 2018 at 11:21:06AM -0700, Junio C Hamano wrote: > Taylor Blau writes: > > > I think that this might be clear enough on its own, especially since > > this is the same as BSD grep on my machine. I think that part_s_ of a > > line indicates that behavior, but perhaps not. On GNU grep, this is: > > > > Print only the matched (non-empty) parts of a matching line, with each > > such part on a separate output line. > > Interesting. I wonder what "git grep -o '^'" would do ;-) That invocation prints nothing, but on BSD grep it prints quite a few blank lines :-). I'm hesitant on sending a patch per the hunk of your reply below because of this. Should we mirror BSD grep's behavior exactly here? I suppose that we could somehow, but it seems like we might be doing too much to support what appears to me to be an odd use-case. > > I'm happy to pick either and re-send this patch (2/2) again, if it > > wouldn't be too much to juggle. Otherwise, I can re-roll to v4. > > Please do not re-send a different version of a patch with the same > v$n value. Either re-send, otherwise re-roll, will give us v4, not > v3. > > In any case, I find that the GNU phrasing is the most clear among > the ones I've seen in this thread so far. OK. I'm happy to re-send that patch with the GNU phrasing depending on what others think (and the above). I'll let this cook and collect some thoughts over the weekend. Thanks, Taylor
Re: [PATCH v3 0/2] grep.c: teach --only-matching to 'git-grep(1)'
Taylor Blau writes: > I think that this might be clear enough on its own, especially since > this is the same as BSD grep on my machine. I think that part_s_ of a > line indicates that behavior, but perhaps not. On GNU grep, this is: > > Print only the matched (non-empty) parts of a matching line, with each > such part on a separate output line. Interesting. I wonder what "git grep -o '^'" would do ;-) > I'm happy to pick either and re-send this patch (2/2) again, if it > wouldn't be too much to juggle. Otherwise, I can re-roll to v4. Please do not re-send a different version of a patch with the same v$n value. Either re-send, otherwise re-roll, will give us v4, not v3. In any case, I find that the GNU phrasing is the most clear among the ones I've seen in this thread so far.
Re: [PATCH v3 0/2] grep.c: teach --only-matching to 'git-grep(1)'
On Thu, Jul 05, 2018 at 10:21:11AM -0400, Jeff King wrote: > On Tue, Jul 03, 2018 at 04:51:52PM -0500, Taylor Blau wrote: > > > diff --git a/Documentation/git-grep.txt b/Documentation/git-grep.txt > > index 0de3493b80..be13fc3253 100644 > > --- a/Documentation/git-grep.txt > > +++ b/Documentation/git-grep.txt > > @@ -17,7 +17,7 @@ SYNOPSIS > >[-l | --files-with-matches] [-L | --files-without-match] > >[(-O | --open-files-in-pager) []] > >[-z | --null] > > - [-c | --count] [--all-match] [-q | --quiet] > > + [ -o | --only-matching ] [-c | --count] [--all-match] [-q | --quiet] > >[--max-depth ] > >[--color[=] | --no-color] > >[--break] [--heading] [-p | --show-function] > > @@ -201,6 +201,10 @@ providing this option will cause it to die. > > Output \0 instead of the character that normally follows a > > file name. > > > > +-o:: > > +--only-matching:: > > + Output only the matching part of the lines. > > + > > Putting myself into the shoes of a naive reader, I have to wonder what > happens when there are multiple matching parts on the same line. I know > the answer from your commit message, but maybe that should be covered > here? Maybe: > > Output only the matching part of the lines. If there are multiple > matching parts, each is output on a separate line. I think that this might be clear enough on its own, especially since this is the same as BSD grep on my machine. I think that part_s_ of a line indicates that behavior, but perhaps not. On GNU grep, this is: Print only the matched (non-empty) parts of a matching line, with each such part on a separate output line. I'm happy to pick either and re-send this patch (2/2) again, if it wouldn't be too much to juggle. Otherwise, I can re-roll to v4. Thanks, Taylor
Re: [PATCH v3 0/2] grep.c: teach --only-matching to 'git-grep(1)'
On Tue, Jul 03, 2018 at 04:51:52PM -0500, Taylor Blau wrote: > diff --git a/Documentation/git-grep.txt b/Documentation/git-grep.txt > index 0de3493b80..be13fc3253 100644 > --- a/Documentation/git-grep.txt > +++ b/Documentation/git-grep.txt > @@ -17,7 +17,7 @@ SYNOPSIS > [-l | --files-with-matches] [-L | --files-without-match] > [(-O | --open-files-in-pager) []] > [-z | --null] > -[-c | --count] [--all-match] [-q | --quiet] > +[ -o | --only-matching ] [-c | --count] [--all-match] [-q | --quiet] > [--max-depth ] > [--color[=] | --no-color] > [--break] [--heading] [-p | --show-function] > @@ -201,6 +201,10 @@ providing this option will cause it to die. > Output \0 instead of the character that normally follows a > file name. > > +-o:: > +--only-matching:: > + Output only the matching part of the lines. > + Putting myself into the shoes of a naive reader, I have to wonder what happens when there are multiple matching parts on the same line. I know the answer from your commit message, but maybe that should be covered here? Maybe: Output only the matching part of the lines. If there are multiple matching parts, each is output on a separate line. -Peff
[PATCH v3 0/2] grep.c: teach --only-matching to 'git-grep(1)'
Hi, Attached here is a third re-roll of my series to teach 'git grep --only-matching'. (I didn't mention it in the thread, but I _thought_ that twice would be enough, so I think Peff's advice about not counting on anything being the final re-roll of something applies to my thoughts, too ;-) ). Since last time: - The second patch has been amended to include the full invocation of 'git grep' (including `--column`, `--only-matching`, and `--line-number`) [1]. - The second patch has been also amended to add the new option (`--only-matching`) to Documentation/git-grep.txt per [2]. An inter-diff is available below, and thanks as always for your review :-). Thanks, Taylor [1]: https://public-inbox.org/git/20180703143820.gc23...@sigill.intra.peff.net/ [2]: https://public-inbox.org/git/xmqq1sckoxk8@gitster-ct.c.googlers.com/ Taylor Blau (2): grep.c: extract show_line_header() grep.c: teach 'git grep --only-matching' Documentation/git-grep.txt | 6 ++- builtin/grep.c | 6 +++ grep.c | 91 -- grep.h | 1 + t/t7810-grep.sh| 15 +++ 5 files changed, 85 insertions(+), 34 deletions(-) Inter-diff (since v2): diff --git a/Documentation/git-grep.txt b/Documentation/git-grep.txt index 0de3493b80..be13fc3253 100644 --- a/Documentation/git-grep.txt +++ b/Documentation/git-grep.txt @@ -17,7 +17,7 @@ SYNOPSIS [-l | --files-with-matches] [-L | --files-without-match] [(-O | --open-files-in-pager) []] [-z | --null] - [-c | --count] [--all-match] [-q | --quiet] + [ -o | --only-matching ] [-c | --count] [--all-match] [-q | --quiet] [--max-depth ] [--color[=] | --no-color] [--break] [--heading] [-p | --show-function] @@ -201,6 +201,10 @@ providing this option will cause it to die. Output \0 instead of the character that normally follows a file name. +-o:: +--only-matching:: + Output only the matching part of the lines. + -c:: --count:: Instead of showing every matched line, show the number of -- 2.18.0
[PATCH v3 0/2] Fix use of strategy options with interactive rebases
The interactive machinery for git rebase can accept special merge strategies or strategy options, but has a bug in its handling of strategy options. This short series patches that. Changes since v2: - Fix broken &&-chaining (Thanks to Eric for spotting) Elijah Newren (2): t3418: add testcase showing problems with rebase -i and strategy options Fix use of strategy options with interactive rebases git-rebase.sh | 2 +- sequencer.c| 7 ++- t/t3418-rebase-continue.sh | 32 3 files changed, 39 insertions(+), 2 deletions(-) 1: 43b9ac5a63 ! 1: f8a5df9ef1 t3418: add testcase showing problems with rebase -i and strategy options @@ -34,7 +34,7 @@ + EOF + chmod +x test-bin/git-merge-funny && + ( -+ PATH=./test-bin:$PATH ++ PATH=./test-bin:$PATH && + test_must_fail git rebase -i -s funny -Xopt -Xfoo master topic + ) && + test -f funny.was.run && @@ -42,7 +42,7 @@ + echo "Resolved" >F2 && + git add F2 && + ( -+ PATH=./test-bin:$PATH ++ PATH=./test-bin:$PATH && + git rebase --continue + ) && + test -f funny.was.run 2: d345eb96d5 = 2: b7e4971e66 Fix use of strategy options with interactive rebases -- 2.18.0.9.g431b2c36d5
Re: [GSoC][PATCH v3 0/2] rebase -i: rewrite the edit-todo functionality in C
Hi Alban, On Tue, 26 Jun 2018, Alban Gruin wrote: > This patch rewrites the edit-todo functionality from shell to C. This is > part of the effort to rewrite interactive rebase in C. > > This patch is based on the fourth iteration of my series rewriting > append_todo_help() in C. > > Changes since v2: > > - Moving edit_todo() from sequencer.c to interactive-rebase.c. Excellent. Thank you, Dscho
[GSoC][PATCH v3 0/2] rebase -i: rewrite the edit-todo functionality in C
This patch rewrites the edit-todo functionality from shell to C. This is part of the effort to rewrite interactive rebase in C. This patch is based on the fourth iteration of my series rewriting append_todo_help() in C. Changes since v2: - Moving edit_todo() from sequencer.c to interactive-rebase.c. Alban Gruin (2): editor: add a function to launch the sequence editor rebase-interactive: rewrite the edit-todo functionality in C builtin/rebase--helper.c | 13 - cache.h| 1 + editor.c | 27 +-- git-rebase--interactive.sh | 11 +-- rebase-interactive.c | 31 +++ rebase-interactive.h | 1 + strbuf.h | 2 ++ 7 files changed, 69 insertions(+), 17 deletions(-) -- 2.18.0
[PATCH v3 0/2] .gitmodules fsck cleanups
On Sat, Jun 09, 2018 at 11:46:13AM +0200, SZEDER Gábor wrote:
> I add few more lines of context here:
>
> tree=$(
> {
> printf "100644 blob $content\t$tricky\n" &&
>
> > printf "12 blob $target\t.gitmodules\n"
> > } | git mktree
> > ) &&
> > - commit=$(git commit-tree $tree) &&
>
> This was the only case where that $tree variable was used, so perhaps
> that can go away as well, in the name of even more simplicity?
Yep, that is simpler. Here's a v3 just dropping those $tree variables,
and adjusting the commit message as appropriate.
[1/2]: t7415: don't bother creating commit for symlink test
[2/2]: fsck: avoid looking at NULL blob->object
fsck.c | 3 ++-
t/t7415-submodule-names.sh | 29 ++---
2 files changed, 24 insertions(+), 8 deletions(-)
-Peff
Re: [PATCH v3 0/2] fsexcludes: Add programmatic way to exclude files
Ben Peart writes: > I found a bug with how this patch series deals with untracked > files. I'm going to retract this patch until I have time to create a > new test case to demonstrate the bug and come up with a good fix. > > Ben Thanks.
Re: [PATCH v3 0/2] fsexcludes: Add programmatic way to exclude files
I found a bug with how this patch series deals with untracked files. I'm going to retract this patch until I have time to create a new test case to demonstrate the bug and come up with a good fix. Ben
[PATCH v3 0/2] fsexcludes: Add programmatic way to exclude files
Only minor changes from V2: Switched to using get_dtype() instead of DTYPE() for platform independence. Cleaned up reverting of fsmonitor code in the untracked cache. Base Ref: master Web-Diff: https://github.com/benpeart/git/commit/709470f33f Checkout: git fetch https://github.com/benpeart/git fsexcludes-v3 && git checkout 709470f33f ### Patches Ben Peart (2): fsexcludes: add a programmatic way to exclude files from git's working directory traversal logic fsmonitor: switch to use new fsexcludes logic and remove unused untracked cache based logic Makefile| 1 + dir.c | 47 +--- dir.h | 2 - fsexcludes.c| 211 fsexcludes.h| 29 + fsmonitor.c | 21 +--- fsmonitor.h | 10 +- t/t7519-status-fsmonitor.sh | 14 +-- 8 files changed, 279 insertions(+), 56 deletions(-) create mode 100644 fsexcludes.c create mode 100644 fsexcludes.h base-commit: fe0a9eaf31dd0c349ae4308498c33a5c3794b293 -- 2.17.0.windows.1
[PATCH v3 0/2] builtin/config.c: prefer `--type=bool` over `--bool`, etc.
Hi, Here is a v3 of a series to (1) treat type specifiers in `git config` as singulars rather than a collection of bits, and (2) to prefer --type= over --. I have made the following changes since v2: - Fix some misspellings in Documentation/git-config.txt (cc: René). - Handle --no-type correctly (cc: Peff). - No longer prefer (1 << x) style for enumerating type specifiers (cc: Peff). - Fix awkward rebase (cc: Peff). Thanks as always for your review :-). Thanks, Taylor Taylor Blau (2): builtin/config.c: treat type specifiers singularly builtin/config.c: prefer `--type=bool` over `--bool`, etc. Documentation/git-config.txt | 66 --- builtin/config.c | 77 +++- t/t1300-repo-config.sh | 29 ++ 3 files changed, 113 insertions(+), 59 deletions(-) -- 2.16.2.440.gc6284da4f
Re: [PATCH v3 0/2] stash push -u -- fixes
On 03/19, Marc Strapetz wrote: > On 16.03.2018 21:43, Thomas Gummerer wrote: > >Thanks Marc for catching the regression I almost introduced and Junio > >for the review of the second patch. Here's a re-roll that should fix > >the issues of v2. > > Thanks, existing issues are fixed, but cleanup of the stashed files seems to > not work properly when stashing a mixture of tracked and untracked files: Thanks for the continued testing, and sorry I just can't seem to get this right :/ The problem here was that I misunderstood what 'git ls-files --error-unmatch' does without testing it myself. I'll send v5 in a bit, which will hopefully finally fix all the cases. > $ git init > $ touch file1 > $ touch file2 > $ git add file1 file2 > $ git commit -m "initial import" > $ echo "a" > file1 > $ echo "b" > file2 > $ touch file3 > $ git status --porcelain > M file1 > M file2 > ?? file3 > $ git stash push -u -- file1 file3 > Saved working directory and index state WIP on master: 48fb140 initial > import > $ git status --porcelain > M file1 > M file2 > > file1 and file3 are properly stashed, but file1 still remains modified in > the working tree. When instead stashing file1 and file2, results are fine: > > $ git stash push -u -- file1 file2 > Saved working directory and index state WIP on master: 48fb140 initial > import > $ git status > On branch master > nothing to commit, working tree clean > > -Marc > >
Re: [PATCH v3 0/2] stash push -u -- fixes
On 16.03.2018 21:43, Thomas Gummerer wrote: Thanks Marc for catching the regression I almost introduced and Junio for the review of the second patch. Here's a re-roll that should fix the issues of v2. Thanks, existing issues are fixed, but cleanup of the stashed files seems to not work properly when stashing a mixture of tracked and untracked files: $ git init $ touch file1 $ touch file2 $ git add file1 file2 $ git commit -m "initial import" $ echo "a" > file1 $ echo "b" > file2 $ touch file3 $ git status --porcelain M file1 M file2 ?? file3 $ git stash push -u -- file1 file3 Saved working directory and index state WIP on master: 48fb140 initial import $ git status --porcelain M file1 M file2 file1 and file3 are properly stashed, but file1 still remains modified in the working tree. When instead stashing file1 and file2, results are fine: $ git stash push -u -- file1 file2 Saved working directory and index state WIP on master: 48fb140 initial import $ git status On branch master nothing to commit, working tree clean -Marc
[PATCH v3 0/2] stash push -u -- fixes
Thanks Marc for catching the regression I almost introduced and Junio
for the review of the second patch. Here's a re-roll that should fix
the issues of v2.
Interdiff below:
diff --git a/git-stash.sh b/git-stash.sh
index 7a4ec98f6b..dbedc7fb9f 100755
--- a/git-stash.sh
+++ b/git-stash.sh
@@ -39,7 +39,7 @@ fi
no_changes () {
git diff-index --quiet --cached HEAD --ignore-submodules -- "$@" &&
git diff-files --quiet --ignore-submodules -- "$@" &&
- (test -z "$untracked" || test -z "$(untracked_files $@)")
+ (test -z "$untracked" || test -z "$(untracked_files "$@")")
}
untracked_files () {
@@ -320,11 +320,14 @@ push_stash () {
git clean --force --quiet -d $CLEAN_X_OPTION -- "$@"
fi
- if test $# != 0 && git ls-files --error-unmatch -- "$@"
>/dev/null 2>/dev/null
+ if test $# != 0
then
- git add -u -- "$@" |
- git checkout-index -z --force --stdin
- git diff-index -p --cached --binary HEAD -- "$@" | git
apply --index -R
+ if git ls-files --error-unmatch -- "$@" >/dev/null
2>/dev/null
+ then
+ git add -u -- "$@" |
+ git checkout-index -z --force --stdin
+ git diff-index -p --cached --binary HEAD --
"$@" | git apply --index -R
+ fi
else
git reset --hard -q
fi
diff --git a/t/t3903-stash.sh b/t/t3903-stash.sh
index 5e7078c083..7efc52fe11 100755
--- a/t/t3903-stash.sh
+++ b/t/t3903-stash.sh
@@ -1103,6 +1103,15 @@ test_expect_success 'stash -u -- doesnt
print error' '
test_line_count = 0 actual
'
+test_expect_success 'stash -u -- leaves rest of working tree in
place' '
+ >tracked &&
+ git add tracked &&
+ >untracked &&
+ git stash push -u -- untracked &&
+ test_path_is_missing untracked &&
+ test_path_is_file tracked
+'
+
test_expect_success 'stash -u -- shows no changes when there
are none' '
git stash push -u -- non-existant >actual &&
echo "No local changes to save" >expect &&
Thomas Gummerer (2):
stash push: avoid printing errors
stash push -u: don't create empty stash
git-stash.sh | 11 +++
t/t3903-stash.sh | 22 ++
2 files changed, 29 insertions(+), 4 deletions(-)
--
2.16.2.804.g6dcf76e11
Re: [PATCH v3 0/2] diff: add --stat-with-summary (was --compact-summary)
Duy Nguyen writes: >> Yup, this is about giving summary in a compact way, not about giving >> a compact stat information. I agree with all the above reasoning, >> and that is why I said that your "compact-summary" was a good way to >> refer to the feature. > > OK I'll wait for a few days. If there's no comment, I'll go with > --stat=compact-summary. Sorry, but that is not what I agreed with you on ;-) I meant to say that your earlier --compact-summary made sense. I do not think "compact summary" as a value of "--stat" makes any sense. It's not like this new one is one of the ways we offer to present "stat" differently and compared to other existing ways this is to give the stat in a compactly summarized fashion. If this were a value to "--summary", then "--summary=in-stat" might make sense, in that this is not about how we show the "stat" information but about how/where "summary" information is shown.
Re: [PATCH v3 0/2] diff: add --stat-with-summary (was --compact-summary)
On Thu, Feb 8, 2018 at 1:19 AM, Junio C Hamano wrote: > Duy Nguyen writes: > >> ... >> Then we still need to decide the new keyword for this feature, I feel >> compact is a bit too vague (I read --stat=compact as "it's compact >> stat", not "stat with compact summary"), so perhaps >> --stat=compact-summary, or just --stat=summary? > > Yup, this is about giving summary in a compact way, not about giving > a compact stat information. I agree with all the above reasoning, > and that is why I said that your "compact-summary" was a good way to > refer to the feature. OK I'll wait for a few days. If there's no comment, I'll go with --stat=compact-summary. -- Duy
Re: [PATCH v3 0/2] diff: add --stat-with-summary (was --compact-summary)
Duy Nguyen writes: > ... > Then we still need to decide the new keyword for this feature, I feel > compact is a bit too vague (I read --stat=compact as "it's compact > stat", not "stat with compact summary"), so perhaps > --stat=compact-summary, or just --stat=summary? Yup, this is about giving summary in a compact way, not about giving a compact stat information. I agree with all the above reasoning, and that is why I said that your "compact-summary" was a good way to refer to the feature.
Re: [PATCH v3 0/2] diff: add --stat-with-summary (was --compact-summary)
On Wed, Feb 7, 2018 at 4:52 PM, Eric Sunshine wrote: > On Tue, Feb 6, 2018 at 5:20 AM, Duy Nguyen wrote: >> On Tue, Feb 6, 2018 at 1:56 AM, Junio C Hamano wrote: >>> Duy Nguyen writes: >>> I actually think compact-summary was a good way to phrase it. >>> >>> Personally, I think it was a UI mistake that --summary can be given >>> independently with or without --stat (instead, there shouldn't have >>> been the --summary option, and instead when it was added, --stat >>> just should have gained an extra kind of output). A single option >>> that can give both kinds of info may be a good way forward, so >>> another possibility may be --summary-in-stat (meaning: the info >>> given by summary is included in stat output). I dunno. >> >> +Eric maybe he has some idea (sorry I forgot to include people from >> the last round). > > What about the earlier suggestion[1] (and minor follow-ups[2,3]) of > making this another option to --stat= (for instance, --stat=compact)? > Did that idea get shot down or am I misunderstanding the question > here. I thought that was something like --stat[=[,[,,[compact and turning on "compact" alone would get awkward because you need to specify all those widths and counts too. --stat=compact as a separate form, with no combination with any other stat params, does not feel justified. We could just do --stat-compact then. Perhaps we can allow compact to appear anywhere in --stat=, and not just the end? The --stat= syntax would be --stat=[[,[,...]]] where option could be keyword ones like compact or anything else in future, or = form. could also be a number, but in that case the three consecutive number options will present width, name-width and count in this order. Or we could simply add new --stat= syntax _without_ " as numbers". widths and counts must be specified keywords as well, e.g. --stat=width=40,name-width=20,count=10,compact and leave the old syntax "--stat=,," alone. Then we still need to decide the new keyword for this feature, I feel compact is a bit too vague (I read --stat=compact as "it's compact stat", not "stat with compact summary"), so perhaps --stat=compact-summary, or just --stat=summary? > [1]: > https://public-inbox.org/git/capig+cqlgs59jyxcmk30qy307arwqjx6pnoo95z39_jj9+d...@mail.gmail.com/ > [2]: > https://public-inbox.org/git/cacsjy8b5qrn8t1aai3y3nfec5baqn2xkk6vzozmp5lh-mpz...@mail.gmail.com/ > [3]: > https://public-inbox.org/git/CACsJy8CPHk+aXHr-mkHZi27s=c3+ny8d9csuzoso8pwevib...@mail.gmail.com/ -- Duy
Re: [PATCH v3 0/2] diff: add --stat-with-summary (was --compact-summary)
On Tue, Feb 6, 2018 at 5:20 AM, Duy Nguyen wrote: > On Tue, Feb 6, 2018 at 1:56 AM, Junio C Hamano wrote: >> Duy Nguyen writes: >> I actually think compact-summary was a good way to phrase it. >> >> Personally, I think it was a UI mistake that --summary can be given >> independently with or without --stat (instead, there shouldn't have >> been the --summary option, and instead when it was added, --stat >> just should have gained an extra kind of output). A single option >> that can give both kinds of info may be a good way forward, so >> another possibility may be --summary-in-stat (meaning: the info >> given by summary is included in stat output). I dunno. > > +Eric maybe he has some idea (sorry I forgot to include people from > the last round). What about the earlier suggestion[1] (and minor follow-ups[2,3]) of making this another option to --stat= (for instance, --stat=compact)? Did that idea get shot down or am I misunderstanding the question here. [1]: https://public-inbox.org/git/capig+cqlgs59jyxcmk30qy307arwqjx6pnoo95z39_jj9+d...@mail.gmail.com/ [2]: https://public-inbox.org/git/cacsjy8b5qrn8t1aai3y3nfec5baqn2xkk6vzozmp5lh-mpz...@mail.gmail.com/ [3]: https://public-inbox.org/git/CACsJy8CPHk+aXHr-mkHZi27s=c3+ny8d9csuzoso8pwevib...@mail.gmail.com/
Re: [PATCH v3 0/2] diff: add --stat-with-summary (was --compact-summary)
On Tue, Feb 6, 2018 at 1:56 AM, Junio C Hamano wrote: > Duy Nguyen writes: > >> On Sat, Feb 3, 2018 at 2:59 AM, Junio C Hamano wrote: >>> Nguyễn Thái Ngọc Duy writes: >>> Changes since v2 [1]: - goes back to my original version (yay!) where the extra info is appended after the path name. More is described in 2/2 - --compact-summary is now renamed --stat-with-summary and implies --stat - 1/2 is just a cleanup patch to make it easier to add 2/2 >>> >>> It may be just me and other old timers, but --X-with-Y naming means >>> quite different thing around these commands, and --stat-with-summary >>> would hint, at least to us, that it would behave as if the two >>> options "--stat --summary" are given at the same time. >>> >>> And from that point of view, the new name is a bit confusing one. >> >> I don't have any good alternative name to be honest. It's kinda hard >> to come up with another word that says "extended header information >> such as creations, renames and mode changes", except maybe the vague >> name --stat-extended? > > I actually think compact-summary was a good way to phrase it. > > Personally, I think it was a UI mistake that --summary can be given > independently with or without --stat (instead, there shouldn't have > been the --summary option, and instead when it was added, --stat > just should have gained an extra kind of output). A single option > that can give both kinds of info may be a good way forward, so > another possibility may be --summary-in-stat (meaning: the info > given by summary is included in stat output). I dunno. > +Eric maybe he has some idea (sorry I forgot to include people from the last round). -- Duy
Re: [PATCH v3 0/2] diff: add --stat-with-summary (was --compact-summary)
Duy Nguyen writes: > On Sat, Feb 3, 2018 at 2:59 AM, Junio C Hamano wrote: >> Nguyễn Thái Ngọc Duy writes: >> >>> Changes since v2 [1]: >>> >>> - goes back to my original version (yay!) where the extra info >>> is appended after the path name. More is described in 2/2 >>> - --compact-summary is now renamed --stat-with-summary and implies >>> --stat >>> - 1/2 is just a cleanup patch to make it easier to add 2/2 >> >> It may be just me and other old timers, but --X-with-Y naming means >> quite different thing around these commands, and --stat-with-summary >> would hint, at least to us, that it would behave as if the two >> options "--stat --summary" are given at the same time. >> >> And from that point of view, the new name is a bit confusing one. > > I don't have any good alternative name to be honest. It's kinda hard > to come up with another word that says "extended header information > such as creations, renames and mode changes", except maybe the vague > name --stat-extended? I actually think compact-summary was a good way to phrase it. Personally, I think it was a UI mistake that --summary can be given independently with or without --stat (instead, there shouldn't have been the --summary option, and instead when it was added, --stat just should have gained an extra kind of output). A single option that can give both kinds of info may be a good way forward, so another possibility may be --summary-in-stat (meaning: the info given by summary is included in stat output). I dunno.
Re: [PATCH v3 0/2] diff: add --stat-with-summary (was --compact-summary)
On Sat, Feb 3, 2018 at 2:59 AM, Junio C Hamano wrote: > Nguyễn Thái Ngọc Duy writes: > >> Changes since v2 [1]: >> >> - goes back to my original version (yay!) where the extra info >> is appended after the path name. More is described in 2/2 >> - --compact-summary is now renamed --stat-with-summary and implies >> --stat >> - 1/2 is just a cleanup patch to make it easier to add 2/2 > > It may be just me and other old timers, but --X-with-Y naming means > quite different thing around these commands, and --stat-with-summary > would hint, at least to us, that it would behave as if the two > options "--stat --summary" are given at the same time. > > And from that point of view, the new name is a bit confusing one. I don't have any good alternative name to be honest. It's kinda hard to come up with another word that says "extended header information such as creations, renames and mode changes", except maybe the vague name --stat-extended? -- Duy
Re: [PATCH v3 0/2] diff: add --stat-with-summary (was --compact-summary)
Nguyễn Thái Ngọc Duy writes: > Changes since v2 [1]: > > - goes back to my original version (yay!) where the extra info > is appended after the path name. More is described in 2/2 > - --compact-summary is now renamed --stat-with-summary and implies > --stat > - 1/2 is just a cleanup patch to make it easier to add 2/2 It may be just me and other old timers, but --X-with-Y naming means quite different thing around these commands, and --stat-with-summary would hint, at least to us, that it would behave as if the two options "--stat --summary" are given at the same time. And from that point of view, the new name is a bit confusing one. The patches themselves look excellent. Thanks.
Re: [PATCH v3 0/2] wrap format-patch diffstats around 72 columns
Nguyễn Thái Ngọc Duy writes: > v3 fixes tests in 2/2 that I overlooked (but Jeff didn't). Interdiff: > ... > Nguyễn Thái Ngọc Duy (2): > format-patch: keep cover-letter diffstat wrapped in 72 columns > format-patch: reduce patch diffstat width to 72 Thanks, will replace. I think we are pretty in good shape with this change now.
[PATCH v3 0/2] diff: add --stat-with-summary (was --compact-summary)
Changes since v2 [1]: - goes back to my original version (yay!) where the extra info is appended after the path name. More is described in 2/2 - --compact-summary is now renamed --stat-with-summary and implies --stat - 1/2 is just a cleanup patch to make it easier to add 2/2 [1] https://public-inbox.org/git/20180118100546.32251-1-pclo...@gmail.com/ Nguyễn Thái Ngọc Duy (2): diff.c: refactor pprint_rename() to use strbuf diff: add --stat-with-summary Documentation/diff-options.txt | 8 ++ diff.c | 101 ++--- diff.h | 1 + t/t4013-diff-various.sh| 5 + ...pretty_--root_--stat-with-summary_initial (new) | 12 +++ ...tty_-R_--root_--stat-with-summary_initial (new) | 12 +++ ...iff-tree_--stat-with-summary_initial_mode (new) | 4 + ...-tree_-R_--stat-with-summary_initial_mode (new) | 4 + 8 files changed, 113 insertions(+), 34 deletions(-) -- 2.16.1.75.ga05eb4
[PATCH v3 0/2] wrap format-patch diffstats around 72 columns
v3 fixes tests in 2/2 that I overlooked (but Jeff didn't). Interdiff: diff --git a/t/t4052-stat-output.sh b/t/t4052-stat-output.sh index 1e62333b46..6e2cf933f7 100755 --- a/t/t4052-stat-output.sh +++ b/t/t4052-stat-output.sh @@ -19,17 +19,33 @@ test_expect_success 'preparation' ' git commit -m message "$name" ' +cat >expect72 <<-'EOF' + ...a | 1 + +EOF +test_expect_success "format-patch: small change with long name gives more space to the name" ' + git format-patch -1 --stdout >output && + grep " | " output >actual && + test_cmp expect72 actual +' + while read cmd args do - cat >expect <<-'EOF' + cat >expect80 <<-'EOF' ...a | 1 + EOF test_expect_success "$cmd: small change with long name gives more space to the name" ' git $cmd $args >output && grep " | " output >actual && - test_cmp expect actual + test_cmp expect80 actual ' +done <<\EOF +diff HEAD^ HEAD --stat +show --stat +log -1 --stat +EOF +while read cmd args +do cat >expect <<-'EOF' ...a | 1 + EOF @@ -60,7 +76,7 @@ do test_cmp expect actual ' done <<\EOF -format-patch --stat=80 -1 --stdout +format-patch -1 --stdout diff HEAD^ HEAD --stat show --stat log -1 --stat Nguyễn Thái Ngọc Duy (2): format-patch: keep cover-letter diffstat wrapped in 72 columns format-patch: reduce patch diffstat width to 72 builtin/log.c | 7 ++- t/t4052-stat-output.sh | 46 -- 2 files changed, 37 insertions(+), 16 deletions(-) -- 2.16.1.205.g271f633410
Re: [PATCH v3 0/2] Doc/submodules: a few updates
On Wednesday 17 January 2018 01:32 AM, Junio C Hamano wrote: > I had a few "Huh?" > moments while reading the resulting text, but nothing show-stopping. > It always happens when there are people around who are trying to be over careful :) Anyways, it's only now that I remember that I've missed a change that I thought of doing. The change is about clarifying what a "de-initialized" submodule means and what an "inactive" submodule means and how they work together (IIUC, a submodule has not active flag when its deinitialized). I foresee people confusing 'init' and 'active'. So, I thought the documentation should be helpful enough in that aspect. Documenation/gitsubmodules doesn't seem to be talking much about 'init'. (It talks about 'active' a lot after these patches :) Now I think it's better to do that as separate change and move this forward as I don't want to make this clumsy anymore. Please let me know, if I'm over thinking things again. :) -- Kaartic signature.asc Description: OpenPGP digital signature
Re: [PATCH v3 0/2] Doc/submodules: a few updates
Kaartic Sivaraam writes: > These are just a few improvements that I thought would make the > documentation > related to submodules a little better in various way such as readability, > consistency etc., These were things I noticed while reading thise > documents. Overall they look like reasonable improvements. I had a few "Huh?" moments while reading the resulting text, but nothing show-stopping.
Re: [PATCH v3 0/2] Incremental rewrite of git-submodules
Prathamesh Chavan writes: > Changes in v3: > > * For the variables: super_config_url and sub_origin_url, xstrdup() was used > while assigning "" to them, before freeing. > > * In case of the function deinit_submodule, since the orignal code doesn't die > upon failure of the function mkdir(), printf was used instead of die_errno. > > As before you can find this series at: > https://github.com/pratham-pc/git/commits/patch-series-2 > > And its build report is available at: > https://travis-ci.org/pratham-pc/git/builds/ > Branch: patch-series-2 > Build #197 > > Prathamesh Chavan (2): > submodule: port submodule subcommand 'sync' from shell to C > submodule: port submodule subcommand 'deinit' from shell to C > > builtin/submodule--helper.c | 340 > > git-submodule.sh| 112 +-- > 2 files changed, 342 insertions(+), 110 deletions(-) Looks sensible. Thanks.
[PATCH v3 0/2] Doc/submodules: a few updates
Quoting from v1, These are just a few improvements that I thought would make the documentation related to submodules a little better in various way such as readability, consistency etc., These were things I noticed while reading thise documents. Changes since v2: - Made some changes suggested by Stefan. - A few more that caught my eyes. Inter diff between v2 and v3: diff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt index 801d291ca..71c5618e8 100644 --- a/Documentation/git-submodule.txt +++ b/Documentation/git-submodule.txt @@ -70,8 +70,8 @@ status [--cached] [--recursive] [--] [...]:: Show the status of the submodules. This will print the SHA-1 of the currently checked out commit for each submodule, along with the submodule path and the output of 'git describe' for the - SHA-1. Each SHA-1 will be prefixed with `-` if the submodule is not - initialized, `+` if the currently checked out submodule commit + SHA-1. Each SHA-1 will possibly be prefixed with `-` if the submodule is + not initialized, `+` if the currently checked out submodule commit does not match the SHA-1 found in the index of the containing repository and `U` if the submodule has merge conflicts. + diff --git a/Documentation/gitsubmodules.txt b/Documentation/gitsubmodules.txt index ce2369c2d..47bbc62e8 100644 --- a/Documentation/gitsubmodules.txt +++ b/Documentation/gitsubmodules.txt @@ -101,7 +101,7 @@ remotes are configured in the submodule as usual in the `$GIT_DIR/config` file. * The configuration file `$GIT_DIR/config` in the superproject. - Git only recurses into active submodules (see 'ACTIVE SUBMODULES' + Git only recurses into active submodules (see "ACTIVE SUBMODULES" section below). + If the submodule is not yet initialized, then the configuration @@ -164,52 +164,59 @@ from another repository. To completely remove a submodule, manually delete `$GIT_DIR/modules//`. -Active submodules +ACTIVE SUBMODULES - A submodule is considered active, - (a) if `submodule..active` is set + (a) if `submodule..active` is set to `true` or - (b) if the submodules path matches the pathspec in `submodule.active` + (b) if the submodule's path matches the pathspec in `submodule.active` or (c) if `submodule..url` is set. +and these are evaluated in this order. + For example: -[submodule "foo"] -active = false -url = https://example.org/foo -[submodule "bar"] -active = true -url = https://example.org/bar -[submodule "baz"] -url = https://example.org/baz + [submodule "foo"] +active = false +url = https://example.org/foo + [submodule "bar"] +active = true +url = https://example.org/bar + [submodule "baz"] +url = https://example.org/baz -In the above config only the submodule bar and baz are active, -bar due to (a) and baz due to (c). +In the above config only the submodule 'bar' and 'baz' are active, +'bar' due to (a) and 'baz' due to (c). 'foo' is inactive because +(a) takes precedence over (c). -Note that '(c)' is a historical artefact and will be ignored if the -pathspec set in (b) excludes the submodule. For example: +Note that (c) is a historical artefact and will be ignored if the +(a) and (b) specify that the submodule is not active. In other words, +if we have an `submodule..active` set to `false` or if the +submodule's path is excluded in the pathspec in `submodule.active`, the +url doesn't matter whether it is present or not. This is illustrated in +the example that follows. -[submodule "foo"] -active = true -url = https://example.org/foo -[submodule "bar"] -url = https://example.org/bar -[submodule "baz"] -url = https://example.org/baz -[submodule "bob"] -ignore = true -[submodule] -active = b* -active = (:exclude) baz + [submodule "foo"] +active = true +url = https://example.org/foo + [submodule "bar"] +url = https://example.org/bar + [submodule "baz"] +url = https://example.org/baz + [submodule "bob"] +ignore = true + [submodule] +active = b* +active = :(exclude) baz -In here all submodules except baz (foo, bar, bob) are active. +In here all submodules except 'baz' (foo, bar, bob) are active. 'foo' due to its own active flag and all the others due to the submodule active pathspec, which specifies that any submodule -starting with 'b' except 'baz' are also active, no matter if -the .url field is present. +starting with 'b' except 'baz' are also active, regardless of the +presence of the .url field. Workflow for a third party library -- Kaartic Sivaraam (2): Doc/gitsubmodules: make some changes to improve readability and syntax Doc/git-submodule: improve readability and grammar of a sentence Documentation/git-submodule.txt | 16 +++--
[PATCH v3 0/2] Incremental rewrite of git-submodules
Changes in v3: * For the variables: super_config_url and sub_origin_url, xstrdup() was used while assigning "" to them, before freeing. * In case of the function deinit_submodule, since the orignal code doesn't die upon failure of the function mkdir(), printf was used instead of die_errno. As before you can find this series at: https://github.com/pratham-pc/git/commits/patch-series-2 And its build report is available at: https://travis-ci.org/pratham-pc/git/builds/ Branch: patch-series-2 Build #197 Prathamesh Chavan (2): submodule: port submodule subcommand 'sync' from shell to C submodule: port submodule subcommand 'deinit' from shell to C builtin/submodule--helper.c | 340 git-submodule.sh| 112 +-- 2 files changed, 342 insertions(+), 110 deletions(-) -- 2.15.1
Re: [PATCH v3 0/2] Add a FREE_AND_NULL() wrapper macro
Ævar Arnfjörð Bjarmason writes: > On Thu, Jun 15 2017, Ævar Arnfjörð Bjarmason jotted: >> I'll change it to FREE_AND_NULL and submit my patch as-is, my reading >> of the rest of this thread is that making it a function instead of a >> macro would be interesting, but has its own caveats that are likely >> better considered as part of its own series, whereas this just changes >> existing code to its macro-expanded functional equivalent. > > Here's v3 with that change. Nothing but the macro name (and comments, > commit messages etc. referring to it) have changed. > > Ævar Arnfjörð Bjarmason (2): > git-compat-util: add a FREE_AND_NULL() wrapper around free(ptr); ptr = > NULL > *.[ch] refactoring: make use of the FREE_AND_NULL() macro Thanks. Perhaps somebody wants to do a follow-up patch on top of these two patches to add .cocci rule e.g. @@ type T; T *ptr; @@ - free(ptr); - ptr = NULL; + FREE_AND_NULL(ptr); so that we can periodically sweep new candidates, to which this macro can be applied?
[PATCH v3 0/2] Add a FREE_AND_NULL() wrapper macro
On Thu, Jun 15 2017, Ævar Arnfjörð Bjarmason jotted: > I'll change it to FREE_AND_NULL and submit my patch as-is, my reading > of the rest of this thread is that making it a function instead of a > macro would be interesting, but has its own caveats that are likely > better considered as part of its own series, whereas this just changes > existing code to its macro-expanded functional equivalent. Here's v3 with that change. Nothing but the macro name (and comments, commit messages etc. referring to it) have changed. Ævar Arnfjörð Bjarmason (2): git-compat-util: add a FREE_AND_NULL() wrapper around free(ptr); ptr = NULL *.[ch] refactoring: make use of the FREE_AND_NULL() macro alias.c | 6 ++ apply.c | 3 +-- attr.c | 6 ++ blame.c | 3 +-- branch.c | 3 +-- builtin/am.c | 18 +- builtin/clean.c | 6 ++ builtin/config.c | 6 ++ builtin/index-pack.c | 6 ++ builtin/pack-objects.c | 12 builtin/unpack-objects.c | 3 +-- builtin/worktree.c | 6 ++ commit-slab.h| 3 +-- commit.c | 3 +-- config.c | 3 +-- credential.c | 9 +++-- diff-lib.c | 3 +-- diff.c | 6 ++ diffcore-rename.c| 6 ++ dir.c| 9 +++-- fast-import.c| 6 ++ git-compat-util.h| 6 ++ gpg-interface.c | 15 +-- grep.c | 12 help.c | 3 +-- http-push.c | 24 http.c | 15 +-- imap-send.c | 3 +-- line-log.c | 6 ++ ll-merge.c | 3 +-- mailinfo.c | 3 +-- object.c | 3 +-- pathspec.c | 3 +-- prio-queue.c | 3 +-- read-cache.c | 6 ++ ref-filter.c | 3 +-- refs/files-backend.c | 3 +-- refs/ref-cache.c | 3 +-- remote-testsvn.c | 3 +-- rerere.c | 3 +-- sequencer.c | 3 +-- sha1-array.c | 3 +-- sha1_file.c | 3 +-- split-index.c| 3 +-- transport-helper.c | 27 +-- transport.c | 3 +-- tree-diff.c | 6 ++ tree-walk.c | 3 +-- tree.c | 3 +-- 49 files changed, 103 insertions(+), 197 deletions(-) -- 2.13.1.508.gb3defc5cc
[PATCH v3 0/2] perf: show that wildmatch() regressed for pathological cases in v2.0
Fixes the issues noted in v3, see <20170510225316.31680-1-ava...@gmail.com> (https://public-inbox.org/git/20170510225316.31680-1-ava...@gmail.com/). In addition I was wrong about for-each-ref not being subjected to this slowdown, I was just screwing up the testcase. Fix that. Now: $ GIT_PERF_REPEAT_COUNT=1 GIT_PERF_MAKE_OPTS="-j6 NO_OPENSSL=Y NO_WILDMATCH=YesPlease" ./run v1.9.5 v2.12.0 p0100-globbing.sh Test v1.9.5 v2.12.0 - 0100.2: refglob((a*)^nb) against tag (a^100).t; n = 1 0.00(0.00+0.00) 0.00(0.00+0.00) = 0100.3: refglob((a*)^nb) against tag (a^100).t; n = 2 0.00(0.00+0.00) 0.00(0.00+0.00) = 0100.4: refglob((a*)^nb) against tag (a^100).t; n = 3 0.00(0.00+0.00) 0.00(0.00+0.00) = 0100.5: refglob((a*)^nb) against tag (a^100).t; n = 4 0.00(0.00+0.00) 0.01(0.00+0.00) +inf 0100.6: refglob((a*)^nb) against tag (a^100).t; n = 5 0.00(0.00+0.00) 0.16(0.15+0.00) +inf 0100.7: refglob((a*)^nb) against tag (a^100).t; n = 6 0.00(0.00+0.00) 2.73(2.71+0.00) +inf 0100.8: fileglob((a*)^nb) against file (a^100).t; n = 10.00(0.00+0.00) 0.00(0.00+0.00) = 0100.9: fileglob((a*)^nb) against file (a^100).t; n = 20.00(0.00+0.00) 0.00(0.00+0.00) = 0100.10: fileglob((a*)^nb) against file (a^100).t; n = 3 0.00(0.00+0.00) 0.00(0.00+0.00) = 0100.11: fileglob((a*)^nb) against file (a^100).t; n = 4 0.00(0.00+0.00) 0.01(0.00+0.00) +inf 0100.12: fileglob((a*)^nb) against file (a^100).t; n = 5 0.00(0.00+0.00) 0.16(0.15+0.00) +inf 0100.13: fileglob((a*)^nb) against file (a^100).t; n = 6 0.00(0.00+0.00) 2.75(2.73+0.00) +inf Ævar Arnfjörð Bjarmason (2): perf: add function to setup a fresh test repo perf: add test showing exponential growth in path globbing t/perf/README| 1 + t/perf/p0100-globbing.sh | 43 +++ t/perf/perf-lib.sh | 17 + 3 files changed, 57 insertions(+), 4 deletions(-) create mode 100755 t/perf/p0100-globbing.sh -- 2.11.0
[PATCH v3 0/2] Clarify interaction between signed pushes and push options
Changes from v2: - incorporated Junio's suggestions - incorporated Ævar's suggestions Jonathan Tan (2): docs: correct receive.advertisePushOptions default receive-pack: verify push options in cert Documentation/config.txt | 5 ++- Documentation/technical/pack-protocol.txt | 32 +++ builtin/receive-pack.c| 51 +-- t/t5534-push-signed.sh| 37 ++ 4 files changed, 114 insertions(+), 11 deletions(-) -- 2.13.0.rc2.291.g57267f2277-goog
[PATCH v3 0/2] gethostbyname fixes
This version includes Junio's fixup to René's patch, and then my patch rebased on top of René's. I thought it was easier to just send both in one series, than to have Junio do a bunch of conflict resolution. I think this still needs Junio's signoff on the first patch, since I've added his code. David Turner (1): xgethostname: handle long hostnames René Scharfe (1): use HOST_NAME_MAX to size buffers for gethostname(2) builtin/gc.c | 12 builtin/receive-pack.c | 4 ++-- daemon.c | 4 fetch-pack.c | 4 ++-- git-compat-util.h | 6 ++ ident.c| 4 ++-- wrapper.c | 13 + 7 files changed, 33 insertions(+), 14 deletions(-) -- 2.11.GIT
[PATCH v3 0/2] http: few fixes for the proxy configuration handling
Hello, this is few patches, which fixes regressions in the proxy handling. Changes since v2: - fix grammar (thanks to Ævar) - add new patch which fixes the silent ignoring of proxy missconfiguration Sergey Ryazanov (2): http: honor empty http.proxy option to bypass proxy http: fix the silent ignoring of proxy misconfiguraion http.c | 13 +++-- 1 file changed, 11 insertions(+), 2 deletions(-) -- 2.10.2
[PATCH v3 0/2] string-list: use ALLOC_GROW macro when reallocing
From: Jeff Hostetler Version 3 eliminates unnecessary exports from p0005 perf test (and fixes the mode bits). Use ALLOC_GROW() macro when reallocating a string_list array rather than simply increasing it by 32. This helps performance of status on very large repos on Windows. Jeff Hostetler (2): string-list: use ALLOC_GROW macro when reallocing string_list p0005-status: time status on very large repo string-list.c | 5 + t/perf/p0005-status.sh | 61 ++ 2 files changed, 62 insertions(+), 4 deletions(-) create mode 100755 t/perf/p0005-status.sh -- 2.9.3
Re: [PATCH v3 0/2] read-cache: call verify_hdr() in a background thread
On 3/30/2017 4:44 PM, Junio C Hamano wrote: Jeff King writes: Still, I'm not sure the extra layer of cache is all that valuable. It should be a single hash lookup in the config cache (in an operation that otherwise reads the entire index). OK, let's drop that part, then. Yes, let's omit the caching there. That way perf tests can run it multiple times with it on or off as they see fit. And in the normal case it will only be executed once anyway.
Re: [PATCH v3 0/2] read-cache: call verify_hdr() in a background thread
On 3/30/2017 4:39 PM, Jeff King wrote: On Thu, Mar 30, 2017 at 09:06:48PM +0100, Thomas Gummerer wrote: Yeah, I think that would be fine. You _could_ write a t/perf test and then use your 400MB monstrosity as GIT_PERF_LARGE_REPO. But given that most people don't have such a thing, there's not much value over you just showing off the perf improvement in the commit message. Sorry if this was already discussed, but we already do have a perf test for the index (p0002), and a corresponding helper program which just does read_cache() and discard_cache(). Maybe we could re-use that and add a second test running the same using the new config? Oh, indeed. Yes, I would think the results of p0002 would probably show off Jeff's improvements. -Peff Let me re-roll it with Junio's cleanup, update fsck to force it on, and look at using p0002. Thanks, Jeff
Re: [PATCH v3 0/2] read-cache: call verify_hdr() in a background thread
Jeff King writes: > Still, I'm not sure the extra layer of cache is all that valuable. It > should be a single hash lookup in the config cache (in an operation that > otherwise reads the entire index). OK, let's drop that part, then.
Re: [PATCH v3 0/2] read-cache: call verify_hdr() in a background thread
On Thu, Mar 30, 2017 at 09:06:48PM +0100, Thomas Gummerer wrote: > > Yeah, I think that would be fine. You _could_ write a t/perf test and > > then use your 400MB monstrosity as GIT_PERF_LARGE_REPO. But given that > > most people don't have such a thing, there's not much value over you > > just showing off the perf improvement in the commit message. > > Sorry if this was already discussed, but we already do have a perf > test for the index (p0002), and a corresponding helper program which > just does read_cache() and discard_cache(). Maybe we could re-use > that and add a second test running the same using the new config? Oh, indeed. Yes, I would think the results of p0002 would probably show off Jeff's improvements. -Peff
Re: [PATCH v3 0/2] read-cache: call verify_hdr() in a background thread
On 03/28, Jeff King wrote: > On Tue, Mar 28, 2017 at 03:50:34PM -0400, Jeff Hostetler wrote: > > > It was a convenient way to isolate, average, and compare > > read_index() times, but I suppose we could do something > > like that. > > > > I did confirm that a ls-files does show a slight 0.008 > > second difference on the 58K file Linux tree when toggled > > on or off. > > Yeah, I agree it helps isolate the change. I'm just not sure we want to > carry a bunch of function-specific perf-testing code. And one of the > nice things about testing a real command is that it's...a real command. > So it's an actual improvement a user might see. > > > But I'm tempted to suggest that we just omit my helper exe > > and not worry about a test -- since we don't have any test > > repos large enough to really demonstrate the differences. > > My concern is that that 0.008 would be lost in the noise > > of the rest of the test and make for an unreliable result. > > Yeah, I think that would be fine. You _could_ write a t/perf test and > then use your 400MB monstrosity as GIT_PERF_LARGE_REPO. But given that > most people don't have such a thing, there's not much value over you > just showing off the perf improvement in the commit message. Sorry if this was already discussed, but we already do have a perf test for the index (p0002), and a corresponding helper program which just does read_cache() and discard_cache(). Maybe we could re-use that and add a second test running the same using the new config? > We could also have a t/perf test that generates a monstrous index and > shows that it's faster. But frankly, I don't think this is all that > interesting as a performance regression test. It's not like there's > something subtle about the performance improvement; we stopped computing > the SHA-1, and (gasp!) it takes exactly one SHA-1 computation's less > time. > > So just mentioning the test case and the improvement in the commit > message is sufficient, IMHO. > > -Peff
Re: [PATCH v3 0/2] read-cache: call verify_hdr() in a background thread
On Thu, Mar 30, 2017 at 12:49:15PM -0700, Junio C Hamano wrote:
> Notable suggested changes I have in this one are:
>
> * I stole the numbers from the cover letter of v2 and added them at
>the end of the log message.
Yeah, good.
> * As the checksum is not a useless relic, but is an integrity
>check, I dropped the "ancient relic" from the proposed log
>message. It is just that the modern disks are reliable enough to
>make it worthwhile to think about a trade-off this patch makes
>between performance and integrity.
Yeah, I'd agree this is more of a tradeoff than a cleanup.
> * As it is customary, the configuration variable starts as an opt
>in feature. In a few releases, perhaps we can flip the default,
>but we do not do so from day one.
I think this is so user-invisible that it doesn't really matter much,
but I'm OK with taking it slow.
> * Updated the code around the call to config-get-bool to avoid
>asking the same question twice.
A few comments on that below.
> * Added minimum documentation.
Yep, looks good.
> By the way, are we sure we have something that validates the
> checksum regardless of the configuration setting? If not, we may
> want to tweak this further so that we can force the validation from
> "git fsck" or something. I am not going to do that myself, but it
> may be necessary before this graduates to 'master'.
Yes, I'd agree this shouldn't graduate without the matching change to
teach fsck to flip the switch back.
> + static int do_checksum = -1;
> [...]
> + if (do_checksum < 0) {
> + /*
> + * Since we run very early in command startup, git_config()
> + * may not have been called yet and the various "core_*"
> + * global variables haven't been set. So look it up
> + * explicitly.
> + */
> + git_config_get_bool("core.checksumindex", &do_checksum);
> + if (do_checksum < 0)
> + do_checksum = 0; /* default to false */
> + }
> + if (!do_checksum)
> + return 0;
This is basically introducing a new caching layer, but there's no way to
invalidate it (if, say, we looked at the config once and then we knew
that the config changed). I think it's probably OK, because the main
reason for the config to change is reading it before/after repository
setup. But since this is index code, it shouldn't be called at all
before repository setup.
Still, I'm not sure the extra layer of cache is all that valuable. It
should be a single hash lookup in the config cache (in an operation that
otherwise reads the entire index).
-Peff
Re: [PATCH v3 0/2] read-cache: call verify_hdr() in a background thread
Jeff King writes:
> So just mentioning the test case and the improvement in the commit
> message is sufficient, IMHO.
So here is how I butchered [v3 1/2] to tentatively queue it on 'pu'.
Notable suggested changes I have in this one are:
* I stole the numbers from the cover letter of v2 and added them at
the end of the log message.
* As the checksum is not a useless relic, but is an integrity
check, I dropped the "ancient relic" from the proposed log
message. It is just that the modern disks are reliable enough to
make it worthwhile to think about a trade-off this patch makes
between performance and integrity.
* As it is customary, the configuration variable starts as an opt
in feature. In a few releases, perhaps we can flip the default,
but we do not do so from day one.
* Updated the code around the call to config-get-bool to avoid
asking the same question twice.
* Added minimum documentation.
By the way, are we sure we have something that validates the
checksum regardless of the configuration setting? If not, we may
want to tweak this further so that we can force the validation from
"git fsck" or something. I am not going to do that myself, but it
may be necessary before this graduates to 'master'.
Thanks.
-- >8 --
From: Jeff Hostetler
Date: Tue, 28 Mar 2017 19:07:31 +
Subject: [PATCH] read-cache: core.checksumindex
Teach git to skip verification of the SHA-1 checksum at the end of
the index file in verify_hdr() called from read_index() when the
core.checksumIndex configuration variable is set to false.
The checksum verification is for detecting disk corruption, and for
small projects, the time it takes to compute SHA-1 is not that
significant, but for gigantic repositories this calculation adds
significant time to every command.
On the Linux kernel repository, the effect is rather trivial.
The time to reading its index with 58k entries drops from 0.0284 sec
down to 0.0155 sec.
On my Windows source tree (450MB index), I'm seeing a savings of 0.6
seconds -- read_index() went from 1.2 to 0.6 seconds.
Signed-off-by: Jeff Hostetler
Signed-off-by: Junio C Hamano
---
Documentation/config.txt | 8
read-cache.c | 16
2 files changed, 24 insertions(+)
diff --git a/Documentation/config.txt b/Documentation/config.txt
index 1df1965457..bc7b216d43 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -329,6 +329,14 @@ advice.*::
show directions on how to proceed from the current state.
--
+core.checksumIndex::
+ Tell Git to validate the checksum at the end of the index
+ file to detect corruption. Defaults to `true`. Those who
+ work on a project with too many files may want to set this
+ variable to `false` to make it faster to load the index (in
+ exchange for reliability, but in general modern disks are
+ reliable enough for most people).
+
core.fileMode::
Tells Git if the executable bit of files in the working tree
is to be honored.
diff --git a/read-cache.c b/read-cache.c
index e447751823..3195702cf7 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -1376,12 +1376,28 @@ static int verify_hdr(struct cache_header *hdr,
unsigned long size)
git_SHA_CTX c;
unsigned char sha1[20];
int hdr_version;
+ static int do_checksum = -1;
if (hdr->hdr_signature != htonl(CACHE_SIGNATURE))
return error("bad signature");
hdr_version = ntohl(hdr->hdr_version);
if (hdr_version < INDEX_FORMAT_LB || INDEX_FORMAT_UB < hdr_version)
return error("bad index version %d", hdr_version);
+
+ if (do_checksum < 0) {
+ /*
+* Since we run very early in command startup, git_config()
+* may not have been called yet and the various "core_*"
+* global variables haven't been set. So look it up
+* explicitly.
+*/
+ git_config_get_bool("core.checksumindex", &do_checksum);
+ if (do_checksum < 0)
+ do_checksum = 0; /* default to false */
+ }
+ if (!do_checksum)
+ return 0;
+
git_SHA1_Init(&c);
git_SHA1_Update(&c, hdr, size - 20);
git_SHA1_Final(sha1, &c);
--
2.12.2-727-gf32eb5229d
Re: [PATCH v3 0/2] read-cache: call verify_hdr() in a background thread
On Tue, Mar 28, 2017 at 03:50:34PM -0400, Jeff Hostetler wrote: > It was a convenient way to isolate, average, and compare > read_index() times, but I suppose we could do something > like that. > > I did confirm that a ls-files does show a slight 0.008 > second difference on the 58K file Linux tree when toggled > on or off. Yeah, I agree it helps isolate the change. I'm just not sure we want to carry a bunch of function-specific perf-testing code. And one of the nice things about testing a real command is that it's...a real command. So it's an actual improvement a user might see. > But I'm tempted to suggest that we just omit my helper exe > and not worry about a test -- since we don't have any test > repos large enough to really demonstrate the differences. > My concern is that that 0.008 would be lost in the noise > of the rest of the test and make for an unreliable result. Yeah, I think that would be fine. You _could_ write a t/perf test and then use your 400MB monstrosity as GIT_PERF_LARGE_REPO. But given that most people don't have such a thing, there's not much value over you just showing off the perf improvement in the commit message. We could also have a t/perf test that generates a monstrous index and shows that it's faster. But frankly, I don't think this is all that interesting as a performance regression test. It's not like there's something subtle about the performance improvement; we stopped computing the SHA-1, and (gasp!) it takes exactly one SHA-1 computation's less time. So just mentioning the test case and the improvement in the commit message is sufficient, IMHO. -Peff
Re: [PATCH v3 0/2] read-cache: call verify_hdr() in a background thread
On 3/28/2017 3:16 PM, Jeff King wrote: On Tue, Mar 28, 2017 at 07:07:30PM +, g...@jeffhostetler.com wrote: From: Jeff Hostetler Version 3 of this patch series simplifies this effort to just turn on/off the hash verification using a "core.checksumindex" config variable. I've preserved the original checksum validation code so that we can force it on in fsck if desired. It eliminates the original threading model completely. Jeff Hostetler (2): read-cache: core.checksumindex test-core-checksum-index: core.checksumindex test helper Makefile| 1 + read-cache.c| 12 ++ t/helper/.gitignore | 1 + t/helper/test-core-checksum-index.c | 77 + Do we still need test-core-checksum-index? Can we just time ls-files or something in t/perf? It was a convenient way to isolate, average, and compare read_index() times, but I suppose we could do something like that. I did confirm that a ls-files does show a slight 0.008 second difference on the 58K file Linux tree when toggled on or off. But I'm tempted to suggest that we just omit my helper exe and not worry about a test -- since we don't have any test repos large enough to really demonstrate the differences. My concern is that that 0.008 would be lost in the noise of the rest of the test and make for an unreliable result. Jeff
Re: [PATCH v3 0/2] read-cache: call verify_hdr() in a background thread
On Tue, Mar 28, 2017 at 07:07:30PM +, g...@jeffhostetler.com wrote: > From: Jeff Hostetler > > Version 3 of this patch series simplifies this effort to just turn > on/off the hash verification using a "core.checksumindex" config variable. > > I've preserved the original checksum validation code so that we can > force it on in fsck if desired. > > It eliminates the original threading model completely. > > Jeff Hostetler (2): > read-cache: core.checksumindex > test-core-checksum-index: core.checksumindex test helper > > Makefile| 1 + > read-cache.c| 12 ++ > t/helper/.gitignore | 1 + > t/helper/test-core-checksum-index.c | 77 > + Do we still need test-core-checksum-index? Can we just time ls-files or something in t/perf? -Peff
[PATCH v3 0/2] read-cache: call verify_hdr() in a background thread
From: Jeff Hostetler Version 3 of this patch series simplifies this effort to just turn on/off the hash verification using a "core.checksumindex" config variable. I've preserved the original checksum validation code so that we can force it on in fsck if desired. It eliminates the original threading model completely. Jeff Hostetler (2): read-cache: core.checksumindex test-core-checksum-index: core.checksumindex test helper Makefile| 1 + read-cache.c| 12 ++ t/helper/.gitignore | 1 + t/helper/test-core-checksum-index.c | 77 + 4 files changed, 91 insertions(+) create mode 100644 t/helper/test-core-checksum-index.c -- 2.9.3
Re: [PATCH v3 0/2] [GSoC] remove_subtree(): reimplement using iterators
On Sat, Mar 25, 2017 at 11:12 AM, Daniel Ferreira wrote: > This is the third version of the GSoC microproject > of refactoring remove_subtree() from recursively using > readdir() to use dir_iterator. Below are the threads for > other versions: > > v1: > https://public-inbox.org/git/CAGZ79kZwT-9mHTiOJ5CEjk2wDFkn6+NcogjX0=vjhsah16a...@mail.gmail.com/T/#mae023e7a7d7626f00e0923833c4359f5af493730 > v2: > https://public-inbox.org/git/cacsjy8dxh-qpbblfafwpawusba9gvxa7x+mxljevykhk1zo...@mail.gmail.com/T/#t > > Duy suggested adding features to dir_iterator might go > beyond the intention of a microproject, but I figured I > might go for it to learn more about the project. > > The dir_iterator reimplementation has been tested in a > separate binary I created (and linked with libgit.a) to > reproduce remove_subtree()'s contents. As pointed out in the > last thread, git's tests for this function were unable to > catch a daunting bug I had introduced, and I still haven't > been able to come up with a way to reproduce remove_subtree() > being called. Any help? > I would think a test llike the following would work: test_expect_success 'remove nested subtrees' ' test_commit initial && mkdir -p dir/with/nested/dir && echo content >dir/with/nested/dir/file && echo content >dir/file && git add dir/with/nested/dir/file dir/file && git commit -a -m "commit directory structure" && git checkout initial && ! test dir '
[PATCH v3 0/2] [GSoC] remove_subtree(): reimplement using iterators
This is the third version of the GSoC microproject of refactoring remove_subtree() from recursively using readdir() to use dir_iterator. Below are the threads for other versions: v1: https://public-inbox.org/git/CAGZ79kZwT-9mHTiOJ5CEjk2wDFkn6+NcogjX0=vjhsah16a...@mail.gmail.com/T/#mae023e7a7d7626f00e0923833c4359f5af493730 v2: https://public-inbox.org/git/cacsjy8dxh-qpbblfafwpawusba9gvxa7x+mxljevykhk1zo...@mail.gmail.com/T/#t Duy suggested adding features to dir_iterator might go beyond the intention of a microproject, but I figured I might go for it to learn more about the project. The dir_iterator reimplementation has been tested in a separate binary I created (and linked with libgit.a) to reproduce remove_subtree()'s contents. As pointed out in the last thread, git's tests for this function were unable to catch a daunting bug I had introduced, and I still haven't been able to come up with a way to reproduce remove_subtree() being called. Any help? Thank you all again for all the reviews. Daniel Ferreira (2): dir_iterator: iterate over dir after its contents remove_subtree(): reimplement using iterators dir-iterator.c | 100 - dir-iterator.h | 7 entry.c| 32 +++--- 3 files changed, 95 insertions(+), 44 deletions(-) -- 2.7.4 (Apple Git-66)
Re: [PATCH v3 0/2] bringing attributes to pathspecs
On 03/21, Duy Nguyen wrote: > On Tue, Mar 14, 2017 at 1:23 AM, Brandon Williams wrote: > > v3 fixes some nits in style in the test script (using <<-\EOF instead of > > <<-EOF) > > as well as fixing a few other minor things reported by Junio and Jonathan. > > I'm slowly digging through the pile of mails in the past weeks... I > know this has landed on 'master' (thanks!). Just wanted to check > something. > > The series updated match_pathspec(), but that's only one of two > pathspec filtering functions. The other is tree_entry_interesting() > (e.g. for "git grep "). Do you have plans to support :(attr) > there too? "No" is a perfectly fine answer (and it will end up in my > forever growing backlog). > > The thing about tree_entry_interesting() is, we would want to stop > traversing subtrees as soon as possible. Naively implemented, we would > need to traverse all subtrees so we can call match_attrs(). That's not > great. Oii I'm rambling.. I don't know yet how to implement this thing > efficiently. I expect that this should be supported in tree_entry_interesting() at some point, though I didn't have any immediate plans to do that yet. One reason for that is I just haven't thought through all the cases to make it performant (as you've pointed out). So for now it'll probably be appended to the backlog (yours, mine, or both) and if there ends up being a larger demand for it then we can increase the priority for it. -- Brandon Williams
Re: [PATCH v3 0/2] bringing attributes to pathspecs
Duy Nguyen writes: > The series updated match_pathspec(), but that's only one of two > pathspec filtering functions. The other is tree_entry_interesting() > (e.g. for "git grep "). Do you have plans to support :(attr) > there too? "No" is a perfectly fine answer (and it will end up in my > forever growing backlog). > > The thing about tree_entry_interesting() is, we would want to stop > traversing subtrees as soon as possible. Naively implemented, we would > need to traverse all subtrees so we can call match_attrs(). That's not > great. Oii I'm rambling.. I don't know yet how to implement this thing > efficiently. Thanks for great insights. It indeed will become issue when an overly broad pathspec pattern is combined with an attribute requirement, e.g. ".:(attr=X)", and we may have to devise a way to tell that there won't be any paths with that satisfy the attribute requirement before descending into a tree as an optimization.
Re: [PATCH v3 0/2] bringing attributes to pathspecs
On Tue, Mar 14, 2017 at 1:23 AM, Brandon Williams wrote: > v3 fixes some nits in style in the test script (using <<-\EOF instead of > <<-EOF) > as well as fixing a few other minor things reported by Junio and Jonathan. I'm slowly digging through the pile of mails in the past weeks... I know this has landed on 'master' (thanks!). Just wanted to check something. The series updated match_pathspec(), but that's only one of two pathspec filtering functions. The other is tree_entry_interesting() (e.g. for "git grep "). Do you have plans to support :(attr) there too? "No" is a perfectly fine answer (and it will end up in my forever growing backlog). The thing about tree_entry_interesting() is, we would want to stop traversing subtrees as soon as possible. Naively implemented, we would need to traverse all subtrees so we can call match_attrs(). That's not great. Oii I'm rambling.. I don't know yet how to implement this thing efficiently. -- Duy
Re: [PATCH v3 0/2] diff --no-index: support symlinks and pipes
Dennis Kaarsemaker writes: > On Sun, 2017-03-19 at 15:08 -0700, Junio C Hamano wrote: > ... >> > - A --derefence option was added and the default is no longer to >> > dereference >> > symlinks. >> >> I do agree that it makes sense to have --[no-]dereference options, >> but I do not think it was my feedback and suggestion to make it >> optional (not default) to dereference, so please do not blame me for >> that choice. > > Then I misinterpreted your message at > http://public-inbox.org/git/xmqqk29yedkv@gitster.mtv.corp.google.com/ > No blame inteded, my apologies for coming across as blaming. s/blame/credit/ then. I do not too deeply care which one is the default, and if we were adding --no-index without any existing users today, I probably would suggest making it deref by default (i.e. to make "diff --no-index" match better what other peoples' diffs do), but that would be a behaviour change to existing users if done today, so I think what you did probably is a good thing. Thanks.
Re: [PATCH v3 0/2] diff --no-index: support symlinks and pipes
On Sun, 2017-03-19 at 15:08 -0700, Junio C Hamano wrote: > Dennis Kaarsemaker writes: > > > Normal diff provides arguably better output: the diff of the output of the > > commands. This series makes it possible for git diff --no-index to follow > > symlinks and read from pipes, mimicking the behaviour of normal diff. > > > > v1: > > http://public-inbox.org/git/2016201958.2175-1-den...@kaarsemaker.net/ > > v2: > > http://public-inbox.org/git/20170113102021.6054-1-den...@kaarsemaker.net/ > > > > Changes since v2, prompted by feedback from Junio: > > > > - A --derefence option was added and the default is no longer to dereference > > symlinks. > > I do agree that it makes sense to have --[no-]dereference options, > but I do not think it was my feedback and suggestion to make it > optional (not default) to dereference, so please do not blame me for > that choice. Then I misinterpreted your message at http://public-inbox.org/git/xmqqk29yedkv@gitster.mtv.corp.google.com/ No blame inteded, my apologies for coming across as blaming. -- Dennis Kaarsemaker http://www.kaarsemaker.net
Re: [PATCH v3 0/2] diff --no-index: support symlinks and pipes
Dennis Kaarsemaker writes: > Normal diff provides arguably better output: the diff of the output of the > commands. This series makes it possible for git diff --no-index to follow > symlinks and read from pipes, mimicking the behaviour of normal diff. > > v1: http://public-inbox.org/git/2016201958.2175-1-den...@kaarsemaker.net/ > v2: http://public-inbox.org/git/20170113102021.6054-1-den...@kaarsemaker.net/ > > Changes since v2, prompted by feedback from Junio: > > - A --derefence option was added and the default is no longer to dereference > symlinks. I do agree that it makes sense to have --[no-]dereference options, but I do not think it was my feedback and suggestion to make it optional (not default) to dereference, so please do not blame me for that choice.
