Re: [PATCH v5] blame: add support for --[no-]progress option
On Tue, Nov 24, 2015 at 11:36 PM, Edmundo Carmona Antoranzwrote: > * created struct progress_info in builtin/blame.c > this struct holds the information used to display progress so > that only one additional parameter is passed to > found_guilty_entry(). Commit messages typically are composed of proper sentences and paragraphs rather than bullets points. Also, write in imperative mood: "Create progress_info..." In this case, though, the information in this bullet point isn't all that interesting for the commit message since it's a detail of implementation easily gleaned by reading the patch itself. There's nothing particularly tricky about it, thus it doesn't really deserve to be called out as special in the commit message. What might be more interesting and deserve mention in the commit message is how this new option interacts with porcelain and incremental modes and why the choice was made. More below... > * --[no-]progress option is also inherited by git-annotate. > > Signed-off-by: Edmundo Carmona Antoranz > --- > diff --git a/Documentation/blame-options.txt b/Documentation/blame-options.txt > @@ -69,6 +69,13 @@ include::line-range-format.txt[] > iso format is used. For supported values, see the discussion > of the --date option at linkgit:git-log[1]. > > +--[no-]progress:: > + Progress status is reported on the standard error stream > + by default when it is attached to a terminal. This flag > + enables progress reporting even if not attached to a > + terminal. Progress information won't be displayed if using > + `--porcelain` or `--incremental`. Is silently ignoring --progress a good idea when combined with --porcelain or --incremental, or would it be better to error out with a "mutually exclusive options" diagnostic? (Genuine question.) > diff --git a/Documentation/git-blame.txt b/Documentation/git-blame.txt > @@ -10,7 +10,8 @@ SYNOPSIS > [verse] > 'git blame' [-c] [-b] [-l] [--root] [-t] [-f] [-n] [-s] [-e] [-p] [-w] > [--incremental] > [-L ] [-S ] [-M] [-C] [-C] [-C] [--since=] > - [--abbrev=] [ | --contents | --reverse ] [--] > > + [--[no-]progress] [--abbrev=] [ | --contents | > --reverse ] Hmm, is [--[no-]progress] common in Git documentation? (Genuine question.) For the synopsis, I'd have expected to see only [--progress] and leave it to the more detailed description of the option to mention the possibility of negation (but I haven't double-checked other documentation, so my expectation may be skewed). > DESCRIPTION > --- > diff --git a/builtin/blame.c b/builtin/blame.c > @@ -127,6 +129,11 @@ struct origin { > +struct progress_info { > + struct progress *progress; > + int blamed_lines; > +}; > @@ -1746,7 +1753,8 @@ static int emit_one_suspect_detail(struct origin > *suspect, int repeat) > -static void found_guilty_entry(struct blame_entry *ent) > +static void found_guilty_entry(struct blame_entry *ent, > + struct progress_info *pi) > @@ -1758,6 +1766,8 @@ static void found_guilty_entry(struct blame_entry *ent) > + if (pi) > + display_progress(pi->progress, pi->blamed_lines += > ent->num_lines); This is updating of 'blamed_lines' is rather subtle and easily overlooked. Splitting it out as a separate statement could improve readability: pi->blamed_lines += ent->num_lines; display_progress(pi->progress, pi->blamed_lines); > } > @@ -1768,6 +1778,16 @@ static void assign_blame(struct scoreboard *sb, int > opt) > { > struct rev_info *revs = sb->revs; > struct commit *commit = prio_queue_get(>commits); > + struct progress_info *pi = NULL; > + > + if (show_progress) { > + pi = malloc(sizeof(*pi)); > + if (pi) { xmalloc(), unlike malloc(), will die() upon allocation failure which would allow you to avoid the "if (pi)" conditional. > + pi->progress = start_progress_delay(_("Blaming > lines"), > + sb->num_lines, > 50, 1); > + pi->blamed_lines = 0; > + } > + } > > while (commit) { > struct blame_entry *ent; > @@ -1809,7 +1829,7 @@ static void assign_blame(struct scoreboard *sb, int opt) > suspect->guilty = 1; > for (;;) { > struct blame_entry *next = ent->next; > - found_guilty_entry(ent); > + found_guilty_entry(ent, pi); > if (next) { > ent = next; > continue; > @@ -1825,6 +1845,11 @@ static void assign_blame(struct scoreboard *sb, int > opt) > if (DEBUG) /* sanity */ >
[PATCH v2] git-p4: Add option to ignore empty commits
From: Lars Schneiderdiff to v1: * change the default behavior and replace "ignore empty commits" option with "keep empty commits" (thanks Junio/Luke) --> I kept the original topic name in the subject letter to ease finding v1, ok? * add 'an' to fix grammar in commit message (thanks Luke) * remove quotes around 'true' in docs to avoid confusion (thanks Luke) * add deletion test (thanks Junio/Luke) * use print statement to avoid ugly \n (thanks Luke) * use positional argument specifiers in Python format string syntax to support Python 2.6 (thanks Pete Harlan) Thanks, Lars Lars Schneider (1): git-p4: Add option to keep empty commits Documentation/git-p4.txt | 4 ++ git-p4.py| 44 +++- t/t9826-git-p4-keep-empty-commits.sh | 134 +++ 3 files changed, 165 insertions(+), 17 deletions(-) create mode 100755 t/t9826-git-p4-keep-empty-commits.sh -- 2.5.1 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] git-p4: support multiple depot paths in p4 submit
On 07 Dec 2015, at 19:51, Sam Hocevarwrote: > On Sun, Dec 06, 2015, Lars Schneider wrote: >> Thanks for the patch! Do you see a way to demonstrate the bug in a test case >> similar to t9821 [1]? > > Not yet, I'm afraid. It's proving trickier than expected because for > now I can only reproduce the bug when the view uses multiples depots > (solution #2 on http://answers.perforce.com/articles/KB/2437), and > unfortunately the test case system in Git was designed for a single > depot. > > Would a refactor of lib-git-p4.sh (and probably all git-p4 tests) to > support multiple depots be acceptable and/or welcome? I prefer to ask > before I dig into the task. Can you outline your idea a bit? Are you aware of the following way to define client specs: [1] ? Would that help? I haven't used multiple depots, yet. Therefore please bare with me :-) Thanks, Lars [1] https://github.com/git/git/blob/362d2fc2f8ab9ee22072f76fb36ec16918511944/t/t9821-git-p4-path-variations.sh#L109-L111 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2] git-p4: Add option to keep empty commits
From: Lars SchneiderA changelist that contains only excluded files due to a client spec was imported as an empty commit. Fix that issue by ignoring these commits. Add option "git-p4.keepEmptyCommits" to make the previous behavior available. Signed-off-by: Lars Schneider Helped-by: Pete Harlan --- Documentation/git-p4.txt | 4 ++ git-p4.py| 44 +++- t/t9826-git-p4-keep-empty-commits.sh | 134 +++ 3 files changed, 165 insertions(+), 17 deletions(-) create mode 100755 t/t9826-git-p4-keep-empty-commits.sh diff --git a/Documentation/git-p4.txt b/Documentation/git-p4.txt index 82aa5d6..b3e768e 100644 --- a/Documentation/git-p4.txt +++ b/Documentation/git-p4.txt @@ -510,6 +510,10 @@ git-p4.useClientSpec:: option '--use-client-spec'. See the "CLIENT SPEC" section above. This variable is a boolean, not the name of a p4 client. +git-p4.keepEmptyCommits:: + A changelist that contains only excluded files will be imported + as an empty commit if this boolean option is set to true. + Submit variables git-p4.detectRenames:: diff --git a/git-p4.py b/git-p4.py index 0093fa3..62c26bc 100755 --- a/git-p4.py +++ b/git-p4.py @@ -2288,12 +2288,6 @@ class P4Sync(Command, P4UserMap): filesToDelete = [] for f in files: -# if using a client spec, only add the files that have -# a path in the client -if self.clientSpecDirs: -if self.clientSpecDirs.map_in_client(f['path']) == "": -continue - filesForCommit.append(f) if f['action'] in self.delete_actions: filesToDelete.append(f) @@ -2361,25 +2355,41 @@ class P4Sync(Command, P4UserMap): gitStream.write(description) gitStream.write("\n") +def inClientSpec(self, path): +if not self.clientSpecDirs: +return True +inClientSpec = self.clientSpecDirs.map_in_client(path) +if not inClientSpec and self.verbose: +print('Ignoring file outside of client spec: {0}'.format(path)) +return inClientSpec + +def hasBranchPrefix(self, path): +if not self.branchPrefixes: +return True +hasPrefix = [p for p in self.branchPrefixes +if p4PathStartsWith(path, p)] +if hasPrefix and self.verbose: +print('Ignoring file outside of prefix: {0}'.format(path)) +return hasPrefix + def commit(self, details, files, branch, parent = ""): epoch = details["time"] author = details["user"] if self.verbose: -print "commit into %s" % branch - -# start with reading files; if that fails, we should not -# create a commit. -new_files = [] -for f in files: -if [p for p in self.branchPrefixes if p4PathStartsWith(f['path'], p)]: -new_files.append (f) -else: -sys.stderr.write("Ignoring file outside of prefix: %s\n" % f['path']) +print('commit into {0}'.format(branch)) if self.clientSpecDirs: self.clientSpecDirs.update_client_spec_path_cache(files) +files = [f for f in files +if self.inClientSpec(f['path']) and self.hasBranchPrefix(f['path'])] + +if not files and not gitConfigBool('git-p4.keepEmptyCommits'): +print('Ignoring revision {0} as it would produce an empty commit.' +.format(details['change'])) +return + self.gitStream.write("commit %s\n" % branch) #gitStream.write("mark :%s\n" % details["change"]) self.committedChanges.add(int(details["change"])) @@ -2403,7 +2413,7 @@ class P4Sync(Command, P4UserMap): print "parent %s" % parent self.gitStream.write("from %s\n" % parent) -self.streamP4Files(new_files) +self.streamP4Files(files) self.gitStream.write("\n") change = int(details["change"]) diff --git a/t/t9826-git-p4-keep-empty-commits.sh b/t/t9826-git-p4-keep-empty-commits.sh new file mode 100755 index 000..be12960 --- /dev/null +++ b/t/t9826-git-p4-keep-empty-commits.sh @@ -0,0 +1,134 @@ +#!/bin/sh + +test_description='Clone repositories and keep empty commits' + +. ./lib-git-p4.sh + +test_expect_success 'start p4d' ' + start_p4d +' + +test_expect_success 'Create a repo' ' + client_view "//depot/... //client/..." && + ( + cd "$cli" && + + mkdir -p subdir && + + >subdir/file1.txt && + p4 add subdir/file1.txt && + p4 submit -d "Add file 1" && + + >file2.txt && + p4 add file2.txt && + p4 submit -d "Add file 2" && + + >subdir/file3.txt && + p4 add
Re: [PATCH 1/2] git-p4: support multiple depot paths in p4 submit
On 8 December 2015 at 11:41, Sam Hocevarwrote: > On Tue, Dec 08, 2015, Lars Schneider wrote: > >> > Would a refactor of lib-git-p4.sh (and probably all git-p4 tests) to >> > support multiple depots be acceptable and/or welcome? I prefer to ask >> > before I dig into the task. >> >> Can you outline your idea a bit? Are you aware of the following way to >> define client specs: [1] ? Would that help? > >That's the idea, but the bug occurs when the client view looks like this: > > //depot/... //client/dir1/... > //depot2/... //client/dir2/... > >And is then cloned with (it is not legal in Perforce to specify //... > directly to grab both depots at once): > > git p4 clone --use-client-spec //depot/... //depot2/... > >Then when a file is modified in dir2/, git p4 submit does not elect it > for the changelist. A file in dir1/ will work fine. > >Unfortunately the current test suite assumes everything is under > //depot/ so in order to write a test for this situation there are a few > things to change in lib-git-p4.sh. I think the existing structure ought to mostly work, but it might need a bit of tweaking. You would need to create a new depot, but you can do that in your test script. And you would need a client spec that pointed at this depot, but again you can do that in your script with the client_view shell function. I've not tried it myself though, so maybe it's harder than that. Luke > > Regards, > -- > Sam. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: What's cooking in git.git (Dec 2015, #01; Tue, 1)
On Mon, Dec 07, 2015 at 11:24:52AM -0800, Junio C Hamano wrote: > Patrick Steinhardtwrites: > > > On Wed, Dec 02, 2015 at 05:31:14PM -0500, Jeff King wrote: > >> On Wed, Dec 02, 2015 at 02:11:32PM -0800, Junio C Hamano wrote: > > [snip] > >> > "--keep-empty" has always been about keeping an originally empty > >> > commit, not a commit that becomes empty because of rebasing > >> > (i.e. what has already been applied to the updated base). The > >> > documentation, if it leads to any other interpretation, needs to be > >> > fixed. > >> > > >> > Besides, if "--keep-empty" were to mean "keep redundant ones that > >> > are already in the updated base", the patch must do a lot more, > >> > e.g. stop filtering with git-cherry patch equivalence. > >> > > >> > I'm inclined to eject this topic. > >> > >> That was my thinking too (and I notice it didn't get any review from > >> anybody else). > > [snip] > > > > Well, I kind of agree. The cherry-pick command has both > > --allow-empty and --keep-redundant flags, where the second one is > > the kind of behavior I want to achieve in my case. As an > > alternative to the proposed change to `--keep-empty` I could > > instead introduce a new flag `--keep-redundant-commits` to `git > > rebase` which would then pass the flag through to the > > cherry-pick. > > > > Any opinions on this? > > Honestly, I am not interested if that is only about passing that > option and doing nothing else. > > Imagine that you have two changes from the branch being rebased > already in the updated base, one was accepted verbatim, while the > other one was accepted with a slight tweak. Perhaps there were some > conflicts in the context, or something. > > When you run your rebase, the former will not even be considered > because command will notice, via patch equivalence, that you do not > need it, even before it attempts to replay it. But not the latter. > The command will attempt to replay it, and only in this step it may > notice, by seeing that the result becomes a no-op change, that the > change has already been included in the updated base. > > I cannot quite see how adding "--keep-redundant-commits" to the > command would help anybody by allowing the latter in the history but > not the former. That is primarily because you can imagine a future > in which the patch equivalence determination becomes smarter. With > or without "--keep-redundant-commits", both of these two changes > would not appear in the resulting history when that happens. > > The "--keep-redundant" option to "cherry-pick" makes sense, as the > user will be the one who is deciding which commit to replay on top > of a new base. If the user fed a commit that is already in the new > base, and the command is run with the option, that is a deliberate > request to leave a no-op cruft. > > We cannot say the same thing for "rebase", as it tries to avoid > redundant cruft, and it cannot do as good a job as humans in doing > so. The "--keep-redundant-commits" option will become a way to make > a distinction between what got dropped by the command automatically > and what didn't get dropped because the command did not look deeply > enough. That distinction is not at all useful, as that can change > as the tool improves. > > A "--keep-redundant-commits" to "rebase" that also disables the > patch equivalence filtering (not just keeping empty crufts in the > resulting history) might make sense, but I do not see myself (or > anybody sane) using it. "In what situation would such a feature be > useful?" is a question I cannot answer offhand. The scenario I require this feature for is somewhat more exotic, yes. We've got a CI where published branches can be rebased but should not be modified in their commit history. That is, the CI determines in a hook wether the branch that is being pushed drops or re-orders commits and if so, rejects the branch. So when a commit that is already included in origin/master gets rebased `git-rebase` currently simply drops the commit, which causes the CI to refuse the branch on a push. So obviously you are right in your conclusion that `--keep-redundant-commits` has to skip the patch equivalence determination in order to not drop any commits. Otherwise my change would only work for certain scenarios. That said, I do not care too deeply about this patch, especially as my scenario is rather uncommon. If you deem this to not have any greater benefit you may simply drop it. Patrick signature.asc Description: Digital signature
Re: [PATCH 1/2] git-p4: support multiple depot paths in p4 submit
On Tue, Dec 08, 2015, Lars Schneider wrote: > > Would a refactor of lib-git-p4.sh (and probably all git-p4 tests) to > > support multiple depots be acceptable and/or welcome? I prefer to ask > > before I dig into the task. > > Can you outline your idea a bit? Are you aware of the following way to define > client specs: [1] ? Would that help? That's the idea, but the bug occurs when the client view looks like this: //depot/... //client/dir1/... //depot2/... //client/dir2/... And is then cloned with (it is not legal in Perforce to specify //... directly to grab both depots at once): git p4 clone --use-client-spec //depot/... //depot2/... Then when a file is modified in dir2/, git p4 submit does not elect it for the changelist. A file in dir1/ will work fine. Unfortunately the current test suite assumes everything is under //depot/ so in order to write a test for this situation there are a few things to change in lib-git-p4.sh. Regards, -- Sam. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/2] git.c: make sure we do not leak GIT_* to alias scripts
On Mon, Dec 7, 2015 at 7:54 PM, Junio C Hamanowrote: > Nguyễn Thái Ngọc Duy writes: > >> Let's hope there will be no third report about this commit.. > > Hmm, why does this additional test fail only under prove but pass > without it? It passes with prove for me. Some mysterious variable leaks through somehow? >> + env | grep GIT_ | sed "s/=.*//" | sort >actual > > This is more about coding discipline than style, but piping grep > output to sed is wasteful. "sed -ne '/^GIT_/s/=.*//p'" or something > like that, perhaps? OK will fix. > I wondered what happens if the user has an unrelated stray variable > whose name happens to begin with GIT_ in her environment, but it > turns out that we cleanse them in test-lib.sh fairly early, so that > would be fine. You need to tighten your "grep" pattern, though. OK -- Duy -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/2] git.c: make sure we do not leak GIT_* to alias scripts
On Tue, Dec 08, 2015 at 05:55:20PM +0100, Duy Nguyen wrote: > On Mon, Dec 7, 2015 at 7:54 PM, Junio C Hamanowrote: > > Nguyễn Thái Ngọc Duy writes: > > > >> Let's hope there will be no third report about this commit.. > > > > Hmm, why does this additional test fail only under prove but pass > > without it? > > It passes with prove for me. Some mysterious variable leaks through somehow? It fails for me when run via "make" (with prove or without) but not as "./t0001-init.sh". Looks like extra variables from my config.mak leak through: $ make t0001-init.sh GIT_TEST_OPTS="-v -i" [...] --- expected2015-12-08 17:18:06.304699181 + +++ actual 2015-12-08 17:18:06.312699180 + @@ -9,5 +9,9 @@ GIT_MERGE_VERBOSITY GIT_PREFIX GIT_TEMPLATE_DIR +GIT_TEST_GIT_DAEMON +GIT_TEST_HTTPD +GIT_TEST_OPTS GIT_TEXTDOMAINDIR GIT_TRACE_BARE +MAKEFLAGS not ok 6 - No extra GIT_* on alias scripts Any GIT_TEST_* is allowed through by test-lib.sh. For the MAKEFLAGS one, I suspect it's hitting MAKEFLAGS=GIT_TEST_OPTS=... You should probably anchor your regex. :) -Peff -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: best practices against long git rebase times?
On Mon, Dec 7, 2015 at 11:59 PM, Jeff Kingwrote: > On Mon, Dec 07, 2015 at 02:56:33PM -0800, Junio C Hamano wrote: > >> Jeff King writes: >> >> > You're computing the patch against the parent for each of those 3000 >> > commits (to get a hash of it to compare against the single hash on the >> > other side). Twelve minutes sounds long, but if you have a really >> > gigantic tree, it might not be unreasonable. >> > >> > You can also try compiling with "make XDL_FAST_HASH=" (i.e., setting >> > that option to the empty string). Last year I found there were some >> > pretty suboptimal corner cases, and you may be hitting one (we should >> > probably turn that option off by default; I got stuck on trying to find >> > a hash that would perform faster and never followed up[1]. >> > >> > I doubt that is your problem, but it's possible). >> > >> > -Peff >> > >> > [1] http://thread.gmane.org/gmane.comp.version-control.git/261638 >> >> I vaguely recall having discussed caching the patch-ids somewhere so >> that this does not have to be done every time. Would such an >> extension help here, I wonder? > > I think you missed John's earlier response which gave several pointers > to such caching schemes. :) Yeah, he also gave very interesting performance numbers. Thanks John! > I used to run with patch-id-caching in my personal fork (I frequently > use "git log --cherry-mark" to see what has made it upstream), but I > haven't for a while. It did make a big difference in speed, but I never > resolved the corner cases around cache invalidation. I will see if I can work on that after I am done with untracked cache... -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] format-patch: add an option to suppress commit hash
"brian m. carlson"writes: >> Don't test "any number of '0'"; test 40 '0's. This is because the >> line format was designed to be usable by things like /etc/magic to >> detect format-patch output, and we want to notice if/when we break >> that aspect of our output format. > > My idea was to future-proof it against changes in the hash function, > although that's in the distant future. Making sure that the test fails when somebody accidentally lengthens (or fails to truncate to 40-hex, when you start using a longer hash) the displayed name there is a good future-proofing. Thanks. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 0/8] Untracked cache improvements
Following the previous RFC version of this patch series and the related discussions, here is a new version that tries to improve the untracked cache feature. This patch series implements core.untrackedCache as a bool config variable. When it's true git should always try to use the untracked cache, and when false git should never use it. Patchs 1/8 and 3/8 add some features that are missing. Patch 2/8, which is new, adds an enum as suggested by Duy. Patchs 4/8, 5/8 and 6/8 are some refactoring to prepare for patch 7/8 which implements core.untrackedCache. Patch 7/8 is the result of squashing the last 3 patches of the RFC series. As discussed this sacrifies backward compatibility for a simpler interface. Patch 8/8, which is new, add some tests. So the changes compared to the RFC version are just small bug fixes and patchs 2/8 and 8/8. The patch series is also available there: https://github.com/chriscool/git/tree/uc-notifs14 Christian Couder (8): update-index: add untracked cache notifications update-index: use enum for untracked cache options update-index: add --test-untracked-cache update-index: move 'uc' var declaration dir: add add_untracked_cache() dir: add remove_untracked_cache() config: add core.untrackedCache t7063: add tests for core.untrackedCache Documentation/config.txt | 7 + Documentation/git-update-index.txt | 31 ++-- builtin/update-index.c | 52 +++--- cache.h| 1 + config.c | 4 +++ contrib/completion/git-completion.bash | 1 + dir.c | 22 +- dir.h | 2 ++ environment.c | 1 + t/t7063-status-untracked-cache.sh | 52 ++ wt-status.c| 9 ++ 11 files changed, 145 insertions(+), 37 deletions(-) -- 2.6.3.478.g9f95483.dirty -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 5/8] dir: add add_untracked_cache()
This new function will be used in a later patch. Signed-off-by: Christian Couder--- builtin/update-index.c | 11 +-- dir.c | 14 ++ dir.h | 1 + 3 files changed, 16 insertions(+), 10 deletions(-) diff --git a/builtin/update-index.c b/builtin/update-index.c index 21f74b2..40530b0 100644 --- a/builtin/update-index.c +++ b/builtin/update-index.c @@ -1123,16 +1123,7 @@ int cmd_update_index(int argc, const char **argv, const char *prefix) if (untracked_cache == TEST_UC) return 0; } - if (!the_index.untracked) { - struct untracked_cache *uc = xcalloc(1, sizeof(*uc)); - strbuf_init(>ident, 100); - uc->exclude_per_dir = ".gitignore"; - /* should be the same flags used by git-status */ - uc->dir_flags = DIR_SHOW_OTHER_DIRECTORIES | DIR_HIDE_EMPTY_DIRECTORIES; - the_index.untracked = uc; - } - add_untracked_ident(the_index.untracked); - the_index.cache_changed |= UNTRACKED_CHANGED; + add_untracked_cache(); fprintf(stderr, _("Untracked cache enabled for '%s'\n"), get_git_work_tree()); } else if (untracked_cache == NO_UC && the_index.untracked) { the_index.untracked = NULL; diff --git a/dir.c b/dir.c index d2a8f06..0f7e293 100644 --- a/dir.c +++ b/dir.c @@ -1938,6 +1938,20 @@ void add_untracked_ident(struct untracked_cache *uc) strbuf_addch(>ident, 0); } +void add_untracked_cache(void) +{ + if (!the_index.untracked) { + struct untracked_cache *uc = xcalloc(1, sizeof(*uc)); + strbuf_init(>ident, 100); + uc->exclude_per_dir = ".gitignore"; + /* should be the same flags used by git-status */ + uc->dir_flags = DIR_SHOW_OTHER_DIRECTORIES | DIR_HIDE_EMPTY_DIRECTORIES; + the_index.untracked = uc; + } + add_untracked_ident(the_index.untracked); + the_index.cache_changed |= UNTRACKED_CHANGED; +} + static struct untracked_cache_dir *validate_untracked_cache(struct dir_struct *dir, int base_len, const struct pathspec *pathspec) diff --git a/dir.h b/dir.h index 7b5855d..ee94c76 100644 --- a/dir.h +++ b/dir.h @@ -308,4 +308,5 @@ void free_untracked_cache(struct untracked_cache *); struct untracked_cache *read_untracked_extension(const void *data, unsigned long sz); void write_untracked_extension(struct strbuf *out, struct untracked_cache *untracked); void add_untracked_ident(struct untracked_cache *); +void add_untracked_cache(void); #endif -- 2.6.3.478.g9f95483.dirty -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/8] update-index: add untracked cache notifications
Doing: cd /tmp git --git-dir=/git/somewhere/else/.git update-index --untracked-cache doesn't work how one would expect. It hardcodes "/tmp" as the directory that "works" into the index, so if you use the working tree, you'll never use the untracked cache. With this patch "git update-index --untracked-cache" tells the user in which directory tests are performed and in which working directory the untracked cache is allowed. Signed-off-by: Christian Couder--- builtin/update-index.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/builtin/update-index.c b/builtin/update-index.c index 7431938..6f6b289 100644 --- a/builtin/update-index.c +++ b/builtin/update-index.c @@ -121,7 +121,7 @@ static int test_if_untracked_cache_is_supported(void) if (!mkdtemp(mtime_dir.buf)) die_errno("Could not make temporary directory"); - fprintf(stderr, _("Testing ")); + fprintf(stderr, _("Testing mtime in '%s' "), xgetcwd()); atexit(remove_test_directory); xstat_mtime_dir(); fill_stat_data(, ); @@ -1122,9 +1122,11 @@ int cmd_update_index(int argc, const char **argv, const char *prefix) } add_untracked_ident(the_index.untracked); the_index.cache_changed |= UNTRACKED_CHANGED; + fprintf(stderr, _("Untracked cache enabled for '%s'\n"), get_git_work_tree()); } else if (!untracked_cache && the_index.untracked) { the_index.untracked = NULL; the_index.cache_changed |= UNTRACKED_CHANGED; + fprintf(stderr, _("Untracked cache disabled\n")); } if (active_cache_changed) { -- 2.6.3.478.g9f95483.dirty -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 7/8] config: add core.untrackedCache
When we know that mtime is fully supported by the environment, we might want the untracked cache to be always used by default without any mtime test or kernel version check being performed. Also when we know that mtime is not supported by the environment, for example because the repo is shared over a network file system, then we might want 'git update-index --untracked-cache' to fail immediately instead of preforming tests (because it might work on some systems using the repo over the network file system but not others). The normal way to achieve the above in Git is to use a config variable. That's why this patch introduces "core.untrackedCache". To keep things simple, this variable is a bool which default to false. When "git status" is run, it now adds or removes the untracked cache in the index to respect the value of this variable. This means that `git update-index --[no-|force-]untracked-cache`, to be compatible with the new config variable, have to set or unset it. This new behavior is backward incompatible change, but that is deliberate. Also `--untracked-cache` used to check that the underlying operating system and file system change `st_mtime` field of a directory if files are added or deleted in that directory. But those tests take a long time and there is now `--test-untracked-cache` to perform them. So to be more consistent with other git commands, this patch prevents `--untracked-cache` to perform tests. This means that after this patch there is no difference any more between `--untracked-cache` and `--force-untracked-cache`. Signed-off-by: Christian Couder--- Documentation/config.txt | 7 +++ Documentation/git-update-index.txt | 28 ++-- builtin/update-index.c | 23 +-- cache.h| 1 + config.c | 4 contrib/completion/git-completion.bash | 1 + dir.c | 2 +- environment.c | 1 + t/t7063-status-untracked-cache.sh | 4 +--- wt-status.c| 9 + 10 files changed, 56 insertions(+), 24 deletions(-) diff --git a/Documentation/config.txt b/Documentation/config.txt index 2d06b11..94820eb 100644 --- a/Documentation/config.txt +++ b/Documentation/config.txt @@ -308,6 +308,13 @@ core.trustctime:: crawlers and some backup systems). See linkgit:git-update-index[1]. True by default. +core.untrackedCache:: + Determines if untracked cache will be enabled. Using + 'git update-index --[no-|force-]untracked-cache' will set + this variable. Before setting it to true, you should check + that mtime is working properly on your system. + See linkgit:git-update-index[1]. False by default. + core.checkStat:: Determines which stat fields to match between the index and work tree. The user can set this to 'default' or diff --git a/Documentation/git-update-index.txt b/Documentation/git-update-index.txt index 0ff7396..0fb39db 100644 --- a/Documentation/git-update-index.txt +++ b/Documentation/git-update-index.txt @@ -175,22 +175,28 @@ may not support it yet. --no-untracked-cache:: Enable or disable untracked cache extension. This could speed up for commands that involve determining untracked files such - as `git status`. The underlying operating system and file - system must change `st_mtime` field of a directory if files - are added or deleted in that directory. + as `git status`. ++ +The underlying operating system and file system must change `st_mtime` +field of a directory if files are added or deleted in that +directory. You can test that using +`--test-untracked-cache`. `--untracked-cache` used to test that too +but it doesn't anymore. ++ +This sets the `core.untrackedCache` configuration variable to 'true' +or 'false' in the repo config file, (see linkgit:git-config[1]), so +that the untracked cache stays enabled or disabled. --test-untracked-cache:: Only perform tests on the working directory to make sure untracked cache can be used. You have to manually enable - untracked cache using `--force-untracked-cache` (or - `--untracked-cache` but this will run the tests again) - afterwards if you really want to use it. + untracked cache using `--untracked-cache` or + `--force-untracked-cache` or the `core.untrackedCache` + configuration variable afterwards if you really want to use + it. --force-untracked-cache:: - For safety, `--untracked-cache` performs tests on the working - directory to make sure untracked cache can be used. These - tests can take a few seconds. `--force-untracked-cache` can be - used to skip the tests. + Same as `--untracked-cache`. \--:: Do not interpret any more arguments as options. @@ -406,6 +412,8 @@ It
[PATCH 2/8] update-index: use enum for untracked cache options
Signed-off-by: Christian Couder--- builtin/update-index.c | 18 +- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/builtin/update-index.c b/builtin/update-index.c index 6f6b289..246b3d3 100644 --- a/builtin/update-index.c +++ b/builtin/update-index.c @@ -35,6 +35,14 @@ static int mark_skip_worktree_only; #define UNMARK_FLAG 2 static struct strbuf mtime_dir = STRBUF_INIT; +/* Untracked cache mode */ +enum uc_mode { + UNDEF_UC = -1, + NO_UC = 0, + UC, + FORCE_UC +}; + __attribute__((format (printf, 1, 2))) static void report(const char *fmt, ...) { @@ -902,7 +910,7 @@ static int reupdate_callback(struct parse_opt_ctx_t *ctx, int cmd_update_index(int argc, const char **argv, const char *prefix) { int newfd, entries, has_errors = 0, line_termination = '\n'; - int untracked_cache = -1; + enum uc_mode untracked_cache = UNDEF_UC; int read_from_stdin = 0; int prefix_length = prefix ? strlen(prefix) : 0; int preferred_index_format = 0; @@ -997,7 +1005,7 @@ int cmd_update_index(int argc, const char **argv, const char *prefix) OPT_BOOL(0, "untracked-cache", _cache, N_("enable/disable untracked cache")), OPT_SET_INT(0, "force-untracked-cache", _cache, - N_("enable untracked cache without testing the filesystem"), 2), + N_("enable untracked cache without testing the filesystem"), FORCE_UC), OPT_END() }; @@ -1104,10 +1112,10 @@ int cmd_update_index(int argc, const char **argv, const char *prefix) the_index.split_index = NULL; the_index.cache_changed |= SOMETHING_CHANGED; } - if (untracked_cache > 0) { + if (untracked_cache > NO_UC) { struct untracked_cache *uc; - if (untracked_cache < 2) { + if (untracked_cache < FORCE_UC) { setup_work_tree(); if (!test_if_untracked_cache_is_supported()) return 1; @@ -1123,7 +1131,7 @@ int cmd_update_index(int argc, const char **argv, const char *prefix) add_untracked_ident(the_index.untracked); the_index.cache_changed |= UNTRACKED_CHANGED; fprintf(stderr, _("Untracked cache enabled for '%s'\n"), get_git_work_tree()); - } else if (!untracked_cache && the_index.untracked) { + } else if (untracked_cache == NO_UC && the_index.untracked) { the_index.untracked = NULL; the_index.cache_changed |= UNTRACKED_CHANGED; fprintf(stderr, _("Untracked cache disabled\n")); -- 2.6.3.478.g9f95483.dirty -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 4/8] update-index: move 'uc' var declaration
Signed-off-by: Christian Couder--- builtin/update-index.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/builtin/update-index.c b/builtin/update-index.c index ecb685d..21f74b2 100644 --- a/builtin/update-index.c +++ b/builtin/update-index.c @@ -1116,8 +1116,6 @@ int cmd_update_index(int argc, const char **argv, const char *prefix) the_index.cache_changed |= SOMETHING_CHANGED; } if (untracked_cache > NO_UC) { - struct untracked_cache *uc; - if (untracked_cache < FORCE_UC) { setup_work_tree(); if (!test_if_untracked_cache_is_supported()) @@ -1126,7 +1124,7 @@ int cmd_update_index(int argc, const char **argv, const char *prefix) return 0; } if (!the_index.untracked) { - uc = xcalloc(1, sizeof(*uc)); + struct untracked_cache *uc = xcalloc(1, sizeof(*uc)); strbuf_init(>ident, 100); uc->exclude_per_dir = ".gitignore"; /* should be the same flags used by git-status */ -- 2.6.3.478.g9f95483.dirty -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 8/8] t7063: add tests for core.untrackedCache
Signed-off-by: Christian Couder--- t/t7063-status-untracked-cache.sh | 48 +-- 1 file changed, 46 insertions(+), 2 deletions(-) diff --git a/t/t7063-status-untracked-cache.sh b/t/t7063-status-untracked-cache.sh index 253160a..f0af41c 100755 --- a/t/t7063-status-untracked-cache.sh +++ b/t/t7063-status-untracked-cache.sh @@ -18,6 +18,10 @@ if ! test_have_prereq UNTRACKED_CACHE; then test_done fi +test_expect_success 'core.untrackedCache is unset' ' + test_must_fail git config --get core.untrackedCache +' + test_expect_success 'setup' ' git init worktree && cd worktree && @@ -28,6 +32,11 @@ test_expect_success 'setup' ' git update-index --untracked-cache ' +test_expect_success 'core.untrackedCache is true' ' + UC=$(git config core.untrackedCache) && + test "$UC" = "true" +' + test_expect_success 'untracked cache is empty' ' test-dump-untracked-cache >../actual && cat >../expect <../actual && - cat >../expect <../expect-from-test-dump <../actual && + echo "no untracked cache" >../expect && + test_cmp ../expect ../actual +' + +test_expect_success 'git status does not change anything' ' + git status && + test-dump-untracked-cache >../actual && + test_cmp ../expect ../actual && + UC=$(git config core.untrackedCache) && + test "$UC" = "false" +' + +test_expect_success 'setting core.untrackedCache and using git status creates the cache' ' + git config core.untrackedCache true && + test-dump-untracked-cache >../actual && + test_cmp ../expect ../actual && + git status && + test-dump-untracked-cache >../actual && + test_cmp ../expect-from-test-dump ../actual +' + +test_expect_success 'unsetting core.untrackedCache and using git status removes the cache' ' + git config --unset core.untrackedCache && + test-dump-untracked-cache >../actual && + test_cmp ../expect-from-test-dump ../actual && + git status && + test-dump-untracked-cache >../actual && + test_cmp ../expect ../actual +' + test_done -- 2.6.3.478.g9f95483.dirty -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 6/8] dir: add remove_untracked_cache()
This new function will be used in a later patch. Signed-off-by: Christian Couder--- builtin/update-index.c | 3 +-- dir.c | 6 ++ dir.h | 1 + 3 files changed, 8 insertions(+), 2 deletions(-) diff --git a/builtin/update-index.c b/builtin/update-index.c index 40530b0..e427657 100644 --- a/builtin/update-index.c +++ b/builtin/update-index.c @@ -1126,8 +1126,7 @@ int cmd_update_index(int argc, const char **argv, const char *prefix) add_untracked_cache(); fprintf(stderr, _("Untracked cache enabled for '%s'\n"), get_git_work_tree()); } else if (untracked_cache == NO_UC && the_index.untracked) { - the_index.untracked = NULL; - the_index.cache_changed |= UNTRACKED_CHANGED; + remove_untracked_cache(); fprintf(stderr, _("Untracked cache disabled\n")); } diff --git a/dir.c b/dir.c index 0f7e293..ffc0286 100644 --- a/dir.c +++ b/dir.c @@ -1952,6 +1952,12 @@ void add_untracked_cache(void) the_index.cache_changed |= UNTRACKED_CHANGED; } +void remove_untracked_cache(void) +{ + the_index.untracked = NULL; + the_index.cache_changed |= UNTRACKED_CHANGED; +} + static struct untracked_cache_dir *validate_untracked_cache(struct dir_struct *dir, int base_len, const struct pathspec *pathspec) diff --git a/dir.h b/dir.h index ee94c76..3e5114d 100644 --- a/dir.h +++ b/dir.h @@ -309,4 +309,5 @@ struct untracked_cache *read_untracked_extension(const void *data, unsigned long void write_untracked_extension(struct strbuf *out, struct untracked_cache *untracked); void add_untracked_ident(struct untracked_cache *); void add_untracked_cache(void); +void remove_untracked_cache(void); #endif -- 2.6.3.478.g9f95483.dirty -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: Why does send-pack call pack-objects for all remote refs?
Your interpretation of my email was correct. As you picked up on, I had a fundamental misunderstanding of what pack-objects was doing. Thanks for the explanation, I have a much better idea of what is going on now. Given my use pattern, it may be reasonable for me to patch in an option to compute git rev-list --objects $my_topic --not $subset_of_remote_refs capitalizing on my knowledge of this particular repository to come up with heuristics for picking a reasonable subset. This will come with the risk of sometimes producing an unnecessarily large (potentially an obscenely large) packfile. You have thoroughly convinced me that an option like that will not generalize and would be unsuitable for main line git. It is also good to know that 2000 remote refs is insane. The lower hanging fruit here sounds like trimming that to a reasonable number, so I'll try that approach first. Thanks again, Junio and Peff. Daniel -Original Message- From: Jeff King [mailto:p...@peff.net] Sent: Monday, December 07, 2015 5:57 PM To: Daniel Koverman Cc: Junio C Hamano; git@vger.kernel.org Subject: Re: Why does send-pack call pack-objects for all remote refs? On Mon, Dec 07, 2015 at 02:41:00PM -0800, Junio C Hamano wrote: > Also it was unclear if you are working with a shallow repository. > The performance trade-off made between the packsize and the cycles > is somewhat different between a normal and a shallow repository, > e.g. 2dacf26d (pack-objects: use --objects-edge-aggressive for > shallow repos, 2014-12-24) might be a good starting point to think > about this issue. Also note that for a while the "aggressive" form was used everywhere. I think that started in fbd4a70 (list-objects: mark more commits as edges in mark_edges_uninteresting - 2013-08-16), and was fixed in 1684c1b (rev-list: add an option to mark fewer edges as uninteresting, 2014-12-24). So it was present from v1.8.4.2 up to v2.3.0. -Peff N�r��yb�X��ǧv�^�){.n�+ا���ܨ}���Ơz�:+v���zZ+��+zf���h���~i���z��w���?�&�)ߢf
[PATCH 3/8] update-index: add --test-untracked-cache
It is nice to just be able to test if untracked cache is supported without enabling it. Signed-off-by: Christian Couder--- Documentation/git-update-index.txt | 9 - builtin/update-index.c | 5 + 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/Documentation/git-update-index.txt b/Documentation/git-update-index.txt index 3df9c26..0ff7396 100644 --- a/Documentation/git-update-index.txt +++ b/Documentation/git-update-index.txt @@ -17,7 +17,7 @@ SYNOPSIS [--[no-]assume-unchanged] [--[no-]skip-worktree] [--ignore-submodules] -[--[no-|force-]untracked-cache] +[--[no-|test-|force-]untracked-cache] [--really-refresh] [--unresolve] [--again | -g] [--info-only] [--index-info] [-z] [--stdin] [--index-version ] @@ -179,6 +179,13 @@ may not support it yet. system must change `st_mtime` field of a directory if files are added or deleted in that directory. +--test-untracked-cache:: + Only perform tests on the working directory to make sure + untracked cache can be used. You have to manually enable + untracked cache using `--force-untracked-cache` (or + `--untracked-cache` but this will run the tests again) + afterwards if you really want to use it. + --force-untracked-cache:: For safety, `--untracked-cache` performs tests on the working directory to make sure untracked cache can be used. These diff --git a/builtin/update-index.c b/builtin/update-index.c index 246b3d3..ecb685d 100644 --- a/builtin/update-index.c +++ b/builtin/update-index.c @@ -40,6 +40,7 @@ enum uc_mode { UNDEF_UC = -1, NO_UC = 0, UC, + TEST_UC, FORCE_UC }; @@ -1004,6 +1005,8 @@ int cmd_update_index(int argc, const char **argv, const char *prefix) N_("enable or disable split index")), OPT_BOOL(0, "untracked-cache", _cache, N_("enable/disable untracked cache")), + OPT_SET_INT(0, "test-untracked-cache", _cache, + N_("test if the filesystem supports untracked cache"), TEST_UC), OPT_SET_INT(0, "force-untracked-cache", _cache, N_("enable untracked cache without testing the filesystem"), FORCE_UC), OPT_END() @@ -1119,6 +1122,8 @@ int cmd_update_index(int argc, const char **argv, const char *prefix) setup_work_tree(); if (!test_if_untracked_cache_is_supported()) return 1; + if (untracked_cache == TEST_UC) + return 0; } if (!the_index.untracked) { uc = xcalloc(1, sizeof(*uc)); -- 2.6.3.478.g9f95483.dirty -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/8] update-index: use enum for untracked cache options
Christian Couderwrites: > Signed-off-by: Christian Couder > --- > builtin/update-index.c | 18 +- > 1 file changed, 13 insertions(+), 5 deletions(-) > > diff --git a/builtin/update-index.c b/builtin/update-index.c > index 6f6b289..246b3d3 100644 > --- a/builtin/update-index.c > +++ b/builtin/update-index.c > @@ -35,6 +35,14 @@ static int mark_skip_worktree_only; > #define UNMARK_FLAG 2 > static struct strbuf mtime_dir = STRBUF_INIT; > > +/* Untracked cache mode */ > +enum uc_mode { > + UNDEF_UC = -1, > + NO_UC = 0, > + UC, > + FORCE_UC > +}; > + With these, the code is much easier to read than with the mystery constants, but did you consider making UC_ a common prefix for further ease-of-reading? E.g. UC_UNSPECIFIED UC_DONTUSE UC_USE UC_FORCE or something? > __attribute__((format (printf, 1, 2))) > static void report(const char *fmt, ...) > { > @@ -902,7 +910,7 @@ static int reupdate_callback(struct parse_opt_ctx_t *ctx, > int cmd_update_index(int argc, const char **argv, const char *prefix) > { > int newfd, entries, has_errors = 0, line_termination = '\n'; > - int untracked_cache = -1; > + enum uc_mode untracked_cache = UNDEF_UC; > int read_from_stdin = 0; > int prefix_length = prefix ? strlen(prefix) : 0; > int preferred_index_format = 0; > @@ -997,7 +1005,7 @@ int cmd_update_index(int argc, const char **argv, const > char *prefix) > OPT_BOOL(0, "untracked-cache", _cache, > N_("enable/disable untracked cache")), > OPT_SET_INT(0, "force-untracked-cache", _cache, > - N_("enable untracked cache without testing the > filesystem"), 2), > + N_("enable untracked cache without testing the > filesystem"), FORCE_UC), > OPT_END() > }; > > @@ -1104,10 +1112,10 @@ int cmd_update_index(int argc, const char **argv, > const char *prefix) > the_index.split_index = NULL; > the_index.cache_changed |= SOMETHING_CHANGED; > } > - if (untracked_cache > 0) { > + if (untracked_cache > NO_UC) { > struct untracked_cache *uc; > > - if (untracked_cache < 2) { > + if (untracked_cache < FORCE_UC) { > setup_work_tree(); > if (!test_if_untracked_cache_is_supported()) > return 1; > @@ -1123,7 +1131,7 @@ int cmd_update_index(int argc, const char **argv, const > char *prefix) > add_untracked_ident(the_index.untracked); > the_index.cache_changed |= UNTRACKED_CHANGED; > fprintf(stderr, _("Untracked cache enabled for '%s'\n"), > get_git_work_tree()); > - } else if (!untracked_cache && the_index.untracked) { > + } else if (untracked_cache == NO_UC && the_index.untracked) { > the_index.untracked = NULL; > the_index.cache_changed |= UNTRACKED_CHANGED; > fprintf(stderr, _("Untracked cache disabled\n")); -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/8] update-index: add untracked cache notifications
Christian Couderwrites: > Doing: > > cd /tmp > git --git-dir=/git/somewhere/else/.git update-index --untracked-cache > > doesn't work how one would expect. It hardcodes "/tmp" as the directory > that "works" into the index, so if you use the working tree, you'll > never use the untracked cache. I think your "expectation" needs to be more explicitly spelled out. "git -C /tmp --git-dir=/git/somewhere/else/.git" is a valid way to use that repository you have in somewhere else to track things under /tmp/ (as you are only passing GIT_DIR but not GIT_WORK_TREE, the cwd, i.e. /tmp, is the root level of the working tree), and for such a usage, the above command works as expected. Perhaps Attempting to flip the untracked-cache feature on for a random index file with cd /random/unrelated/place git --git-dir=/somewhere/else/.git update-index --untracked-cache would not work as you might expect. Because flipping the feature on in the index also records the location of the corresponding working tree (/random/unrelated/place in the above example), when the index is subsequently used to keep track of files in the working tree in /somewhere/else, the feature is disabled. may be an improvement. The index already implicitly records where the working tree was and that is not limited to untracked-cache option. For example, if you have your repository and its working tree in /git/somewhere/else, which does not have a path X, then doing: cd /tmp && >tmp/X git --git-dir=/git/somewhere/else/.git update-index --add X would store X taken from /tmp in the index, so subsequent use of the index "knows" about X that was taken outside /git/somewhere/else/ after the above command finishes and the subsequent use is made without the --git-dir parameter, e.g. cd /git/somewhere/else/ && git diff-index --cached HEAD' would say that you added X, even though /git/somewhere/else/ may not have that X at all. And this is not limited to update-index, either. You can temporarily use --git-dir with "git add X" and the result would persist the same way in the index. I think the moral of the story is that you are not expected to randomly use git-dir and git-work-tree to point at different places without knowing what you are doing, and we may need a way to help people understand what is going on when it is done by a mistake. The patch to show result from xgetcwd() and get_git_work_tree() may be a step in the right direction, but if the user is not doing anything fancy, "Testing mtime in /long/path/to/the/directory" would be irritatingly verbose. I wonder if it is easy to tell when the user is doing such an unnatural thing. Off the top of my head, when the working tree is anywhere other than one level above $GIT_DIR, the user is doing something unusual; I do not know if there are cases where the user is doing something unnatural if $GIT_WORK_TREE is one level above $GIT_DIR, though. > With this patch "git update-index --untracked-cache" tells the user in > which directory tests are performed and in which working directory the > untracked cache is allowed. > > Signed-off-by: Christian Couder > --- > builtin/update-index.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/builtin/update-index.c b/builtin/update-index.c > index 7431938..6f6b289 100644 > --- a/builtin/update-index.c > +++ b/builtin/update-index.c > @@ -121,7 +121,7 @@ static int test_if_untracked_cache_is_supported(void) > if (!mkdtemp(mtime_dir.buf)) > die_errno("Could not make temporary directory"); > > - fprintf(stderr, _("Testing ")); > + fprintf(stderr, _("Testing mtime in '%s' "), xgetcwd()); > atexit(remove_test_directory); > xstat_mtime_dir(); > fill_stat_data(, ); > @@ -1122,9 +1122,11 @@ int cmd_update_index(int argc, const char **argv, > const char *prefix) > } > add_untracked_ident(the_index.untracked); > the_index.cache_changed |= UNTRACKED_CHANGED; > + fprintf(stderr, _("Untracked cache enabled for '%s'\n"), > get_git_work_tree()); > } else if (!untracked_cache && the_index.untracked) { > the_index.untracked = NULL; > the_index.cache_changed |= UNTRACKED_CHANGED; > + fprintf(stderr, _("Untracked cache disabled\n")); > } > > if (active_cache_changed) { -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 7/8] config: add core.untrackedCache
Christian Couderwrites: > When we know that mtime is fully supported by the environment, we > might want the untracked cache to be always used by default without > any mtime test or kernel version check being performed. > > Also when we know that mtime is not supported by the environment, > for example because the repo is shared over a network file system, > then we might want 'git update-index --untracked-cache' to fail > immediately instead of preforming tests (because it might work on > some systems using the repo over the network file system but not > others). > > The normal way to achieve the above in Git is to use a config > variable. That's why this patch introduces "core.untrackedCache". > > To keep things simple, this variable is a bool which default to > false. > > When "git status" is run, it now adds or removes the untracked > cache in the index to respect the value of this variable. > > This means that `git update-index --[no-|force-]untracked-cache`, > to be compatible with the new config variable, have to set or > unset it. This new behavior is backward incompatible change, but > that is deliberate. The logic in this paragraph is fuzzy to me. Shouldn't the config give a sensible default, that is overriden by command line options? I agree that it is insane to do a runtime check when the user says "update-index --untracked-cache" to enable it, as the user _knows_ that enabling it would help (or the user _knows_ that she wants to play with it). Similarly, shouldn't the config be ignored when the user says "update-index --no-untracked-cache" (hence removing the untracked cache from the resulting index no matter the config is set to)? Why is it a good idea to allow an explicit operation from the command line to muck with the configuration? If the user wants to change the configuration permanently, "git config" has been there for the purpose for all the configuration variables to do so for almost forever. Why is it a good idea to make this variable so special that the user does not have to use "git config" but disable or enable it in some other way? > Also `--untracked-cache` used to check that the underlying > operating system and file system change `st_mtime` field of a > directory if files are added or deleted in that directory. But > those tests take a long time and there is now > `--test-untracked-cache` to perform them. > > So to be more consistent with other git commands, this patch > prevents `--untracked-cache` to perform tests. Good change. > This means that > after this patch there is no difference any more between > `--untracked-cache` and `--force-untracked-cache`. A tip to write the explanation. Every time you say "This means that...", you are (perhaps unconsciously) admitting that what you wrote immedidately before that may not be understandable and what comes after it may be a better explanation. Discard the sentence before "This means that", and try to formulate your explanation around what you wrote after it, adding information in the discarded earlier sentence back to the later one. Doing this exercise often helps the result read much better. > > Signed-off-by: Christian Couder > --- > Documentation/config.txt | 7 +++ > Documentation/git-update-index.txt | 28 ++-- > builtin/update-index.c | 23 +-- > cache.h| 1 + > config.c | 4 > contrib/completion/git-completion.bash | 1 + > dir.c | 2 +- > environment.c | 1 + > t/t7063-status-untracked-cache.sh | 4 +--- > wt-status.c| 9 + > 10 files changed, 56 insertions(+), 24 deletions(-) > > diff --git a/Documentation/config.txt b/Documentation/config.txt > index 2d06b11..94820eb 100644 > --- a/Documentation/config.txt > +++ b/Documentation/config.txt > @@ -308,6 +308,13 @@ core.trustctime:: > crawlers and some backup systems). > See linkgit:git-update-index[1]. True by default. > > +core.untrackedCache:: > + Determines if untracked cache will be enabled. Using > + 'git update-index --[no-|force-]untracked-cache' will set > + this variable. Before setting it to true, you should check > + that mtime is working properly on your system. > + See linkgit:git-update-index[1]. False by default. > + > core.checkStat:: > Determines which stat fields to match between the index > and work tree. The user can set this to 'default' or > diff --git a/Documentation/git-update-index.txt > b/Documentation/git-update-index.txt > index 0ff7396..0fb39db 100644 > --- a/Documentation/git-update-index.txt > +++ b/Documentation/git-update-index.txt > @@ -175,22 +175,28 @@ may not support it yet. > --no-untracked-cache:: > Enable or disable untracked cache extension. This could
GPG public keys
Hello, Can you please point me to the public GPG keys used for source code signing? Thanks, Jamie -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 6/8] dir: add remove_untracked_cache()
Christian Couderwrites: > This new function will be used in a later patch. > > Signed-off-by: Christian Couder > --- Up to this step I looked at and they made sense (I am not saying the remainder of the series do not make sense). I however wonder where the memory used for untracked cache goes when this is called? > builtin/update-index.c | 3 +-- > dir.c | 6 ++ > dir.h | 1 + > 3 files changed, 8 insertions(+), 2 deletions(-) > > diff --git a/builtin/update-index.c b/builtin/update-index.c > index 40530b0..e427657 100644 > --- a/builtin/update-index.c > +++ b/builtin/update-index.c > @@ -1126,8 +1126,7 @@ int cmd_update_index(int argc, const char **argv, const > char *prefix) > add_untracked_cache(); > fprintf(stderr, _("Untracked cache enabled for '%s'\n"), > get_git_work_tree()); > } else if (untracked_cache == NO_UC && the_index.untracked) { > - the_index.untracked = NULL; > - the_index.cache_changed |= UNTRACKED_CHANGED; > + remove_untracked_cache(); > fprintf(stderr, _("Untracked cache disabled\n")); > } > > diff --git a/dir.c b/dir.c > index 0f7e293..ffc0286 100644 > --- a/dir.c > +++ b/dir.c > @@ -1952,6 +1952,12 @@ void add_untracked_cache(void) > the_index.cache_changed |= UNTRACKED_CHANGED; > } > > +void remove_untracked_cache(void) > +{ > + the_index.untracked = NULL; > + the_index.cache_changed |= UNTRACKED_CHANGED; > +} > + > static struct untracked_cache_dir *validate_untracked_cache(struct > dir_struct *dir, > int base_len, > const struct pathspec > *pathspec) > diff --git a/dir.h b/dir.h > index ee94c76..3e5114d 100644 > --- a/dir.h > +++ b/dir.h > @@ -309,4 +309,5 @@ struct untracked_cache *read_untracked_extension(const > void *data, unsigned long > void write_untracked_extension(struct strbuf *out, struct untracked_cache > *untracked); > void add_untracked_ident(struct untracked_cache *); > void add_untracked_cache(void); > +void remove_untracked_cache(void); > #endif -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3] contrib/subtree: fix "subtree split" skipped-merge bug
A bug occurs in 'git-subtree split' where a merge is skipped even when both parents act on the subtree, provided the merge results in a tree identical to one of the parents. Fix by copying the merge if at least one parent is non-identical, and the non-identical parent is not an ancestor of the identical parent. Also, add a test case which checks that a descendant can be pushed to its ancestor in this case. Signed-off-by: Dave Ware--- Notes: Many thanks to Eric Sunshine for his adivce on this patch Changes since v2: - Minor improvements to commit message - Changed space indentation to tab indentation in test case - Changed use of rev-list for obtaining commit id to use rev-parse instead Changes since v1: - Minor improvements to commit message - Added sign off - Moved test case from own file into t7900-subtree.sh - Added subshell to test around 'cd' - Moved record of commit for cherry-pick to variable instead of dumping into file [v2]: http://thread.gmane.org/gmane.comp.version-control.git/282065/focus=282121 [v1]: http://thread.gmane.org/gmane.comp.version-control.git/282065 contrib/subtree/git-subtree.sh | 12 +++-- contrib/subtree/t/t7900-subtree.sh | 52 ++ 2 files changed, 62 insertions(+), 2 deletions(-) diff --git a/contrib/subtree/git-subtree.sh b/contrib/subtree/git-subtree.sh index 9f06571..b837531 100755 --- a/contrib/subtree/git-subtree.sh +++ b/contrib/subtree/git-subtree.sh @@ -479,8 +479,16 @@ copy_or_skip() p="$p -p $parent" fi done - - if [ -n "$identical" ]; then + + copycommit= + if [ -n "$identical" ] && [ -n "$nonidentical" ]; then + extras=$(git rev-list --boundary $identical..$nonidentical) + if [ -n "$extras" ]; then + # we need to preserve history along the other branch + copycommit=1 + fi + fi + if [ -n "$identical" ] && [ -z "$copycommit" ]; then echo $identical else copy_commit $rev $tree "$p" || exit $? diff --git a/contrib/subtree/t/t7900-subtree.sh b/contrib/subtree/t/t7900-subtree.sh index 9051982..710278c 100755 --- a/contrib/subtree/t/t7900-subtree.sh +++ b/contrib/subtree/t/t7900-subtree.sh @@ -468,4 +468,56 @@ test_expect_success 'verify one file change per commit' ' )) ' +test_expect_success 'subtree descendent check' ' + mkdir git_subtree_split_check && + ( + cd git_subtree_split_check && + git init && + + mkdir folder && + + echo a >folder/a && + git add . && + git commit -m "first commit" && + + git branch branch && + + echo 0 >folder/0 && + git add . && + git commit -m "adding 0 to folder" && + + echo b >folder/b && + git add . && + git commit -m "adding b to folder" && + cherry=$(git rev-parse HEAD) && + + git checkout branch && + echo text >textBranch.txt && + git add . && + git commit -m "commit to fiddle with branch: branch" && + + git cherry-pick $cherry && + git checkout master && + git merge -m "merge" branch && + + git branch noop_branch && + + echo d >folder/d && + git add . && + git commit -m "adding d to folder" && + + git checkout noop_branch && + echo moreText >anotherText.txt && + git add . && + git commit -m "irrelevant" && + + git checkout master && + git merge -m "second merge" noop_branch && + + git subtree split --prefix folder/ --branch subtree_tip master && + git subtree split --prefix folder/ --branch subtree_branch branch && + git push . subtree_tip:subtree_branch + ) + ' + test_done -- 1.9.1 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4] contrib/subtree: fix "subtree split" skipped-merge bug
On Tue, Dec 8, 2015 at 7:19 PM, Dave Warewrote: > 'git subtree split' can incorrectly skip a merge even when both parents > act on the subtree, provided the merge results in a tree identical to > one of the parents. Fix by copying the merge if at least one parent is > non-identical, and the non-identical parent is not an ancestor of the > identical parent. > > Also, add a test case which checks that a descendant can be pushed to > its ancestor in this case. > > Signed-off-by: Dave Ware > --- > diff --git a/contrib/subtree/t/t7900-subtree.sh > b/contrib/subtree/t/t7900-subtree.sh > @@ -468,4 +468,56 @@ test_expect_success 'verify one file change per commit' ' > )) > ' > > +test_expect_success 'subtree descendent check' ' s/descendent/descendant/ > + mkdir git_subtree_split_check && > + ( > + cd git_subtree_split_check && > +[...] > + git push . subtree_tip:subtree_branch > + ) > + ' Style nit: don't indent closing quotation mark > test_done > -- > 1.9.1 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 6/8] dir: add remove_untracked_cache()
On 08.12.15 18:15, Christian Couder wrote: > This new function will be used in a later patch. Why not combine 5/8 with 6/8 into a single patch ? (And please consider to mark the series with v2) -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 5/8] dir: add add_untracked_cache()
On 08.12.15 18:15, Christian Couder wrote: > This new function will be used in a later patch. May be Factor out code into add_untracked_cache(), which will be used in the next commit. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/2] git.c: make sure we do not leak GIT_* to alias scripts
Jeff Kingwrites: > It fails for me when run via "make" (with prove or without) but not as > "./t0001-init.sh". Looks like extra variables from my config.mak leak > through: > > $ make t0001-init.sh GIT_TEST_OPTS="-v -i" > [...] > --- expected2015-12-08 17:18:06.304699181 + > +++ actual 2015-12-08 17:18:06.312699180 + > @@ -9,5 +9,9 @@ >GIT_MERGE_VERBOSITY >GIT_PREFIX >GIT_TEMPLATE_DIR > +GIT_TEST_GIT_DAEMON > +GIT_TEST_HTTPD > +GIT_TEST_OPTS >GIT_TEXTDOMAINDIR >GIT_TRACE_BARE > +MAKEFLAGS > not ok 6 - No extra GIT_* on alias scripts > > Any GIT_TEST_* is allowed through by test-lib.sh. Also GIT_PROVE_OPTS (which caused false positive for me). Perhaps grab "env" output outside the alias to create the expected (instead of a random handcrafted list that you have to maintain as the test suite evolves), and compare it from within the alias, or something? -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 7/8] config: add core.untrackedCache
Junio C Hamanowrites: > Christian Couder writes: > >> When we know that mtime is fully supported by the environment, we >> might want the untracked cache to be always used by default without >> any mtime test or kernel version check being performed. >> >> Also when we know that mtime is not supported by the environment, >> for example because the repo is shared over a network file system, >> then we might want 'git update-index --untracked-cache' to fail >> immediately instead of preforming tests (because it might work on >> some systems using the repo over the network file system but not >> others). >> ... > The logic in this paragraph is fuzzy to me. Shouldn't the config > give a sensible default, that is overriden by command line options? > I agree that it is insane to do a runtime check when the user says > "update-index --untracked-cache" to enable it, as the user _knows_ > that enabling it would help (or the user _knows_ that she wants to > play with it). Similarly, shouldn't the config be ignored when the > user says "update-index --no-untracked-cache" (hence removing the > untracked cache from the resulting index no matter the config is set > to)? ... As I think about this more, it really seems to me that we shouldn't need to make this configuration variable that special. Because I think it is a *BUG* in the current implementation to do the runtime check even when the user explicitly tells us to use untracked-cache, I'd imagine something along the lines of the following would be a lot simpler, easier to understand and a generally more useful bugfix: 1 Add one boolean configuration variable, core.untrackedCache, that defaults to 'false'. 2 When creating an index file in an empty repository, enable the untracked cache in the index (even without the user runninng "update-index --untracked-cache") iff the configuration is set to 'true'. No runtime check necessary. 3 When working on an existing index file, unless the operation is "update-index --[no-]untracked-cache", keep the current setting, regardless of this configuration variable. No runtime check necessary. 4 "update-index --[no-]untracked-cache" should enable or disable the untracked cache as the user tells us, regardless of the configuration variable. No runtime check necessary. It is OK to then add an "auto-detect" on top of the above, that would only affect the second bullet point, like so: 2a When creating an index file in an empty repository, if the configuration is set to 'auto', do the lengthy runtime check and enable the untracked cache in the index (even without the user runninng "update-index --untracked-cache"). without changing any other in the first 4-bullet list. Am I missing some other requirements? -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: GPG public keys
Jamie Evanswrites: > Can you please point me to the public GPG keys used for source code signing? I suspect that you are asking about our project, but instead of throwing you a fish, I'll show you how to catch one yourself. In a copy of linux kernel repository I have lying around from a random past, I did this: $ git log --show-signature and saw something like this: commit c6fa8e6de3dc420cba092bf155b2ed25bcd537f7 merged tag 'arm64-fixes' gpg: Signature made Wed 07 Oct 2015 03:10:34 AM PDT using RSA key ID 84C16334 gpg: Can't check signature: public key not found Merge: e82fa92 62c6c61 Author: Linus Torvalds Date: Wed Oct 7 18:17:46 2015 +0100 Merge tag 'arm64-fixes' of git://git.kernel.org/pub/scm/li... I do not have the public key with key ID 84C16334, but I can ask public keyservers. Put 0x84C16334 in "Search String" in pgp.mit.edu and click "Do the search!"--it would result in the key that was used to sign the merge request that resulted in this merge. I also can do this: $ git tag -v v3.0 and I would see something like: object 02f8c6aee8df3cdc935e9bdd4f2d020306035dbe type commit tag v3.0 tagger Linus Torvalds 1311301049 -0700 Linux 3.0 w00t! gpg: Signature made Thu 21 Jul 2011 07:17:44 PM PDT using DSA key ID 76E21CBB gpg: Good signature from "Linus Torvalds (tag signing key) " ... to find that Linus's tag signing key has ID 0x76E21CBB (I do have his key in my keyring, so this does not say "Can't check"). Perhaps you can do the same to whatever project you are interested in. For example, here is a starting point to do the same for our recent v2.6.4 tag: $ git tag -v v2.6.4 gpg: Signature made Tue 08 Dec 2015 02:12:50 PM PST using RSA key ID 96AFE6CB gpg: Can't check signature: public key not found error: could not verify the tag 'v2.6.4' -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[ANNOUNCE] Git v2.6.4
The latest maintenance release Git v2.6.4 is now available at the usual places, with a handful of fixes that are already in the 'master' front. The tarballs are found at: https://www.kernel.org/pub/software/scm/git/ The following public repositories all have a copy of the 'v2.6.4' tag and the 'maint' branch that the tag points at: url = https://kernel.googlesource.com/pub/scm/git/git url = git://repo.or.cz/alt-git.git url = git://git.sourceforge.jp/gitroot/git-core/git.git url = git://git-core.git.sourceforge.net/gitroot/git-core/git-core url = https://github.com/gitster/git Git v2.6.4 Release Notes Fixes since v2.6.3 -- * The "configure" script did not test for -lpthread correctly, which upset some linkers. * Add support for talking http/https over socks proxy. * Portability fix for Windows, which may rewrite $SHELL variable using non-POSIX paths. * We now consistently allow all hooks to ignore their standard input, rather than having git complain of SIGPIPE. * Fix shell quoting in contrib script. * Test portability fix for a topic in v2.6.1. * Allow tilde-expansion in some http config variables. * Give a useful special case "diff/show --word-diff-regex=." as an example in the documentation. * Fix for a corner case in filter-branch. * Make git-p4 work on a detached head. * Documentation clarification for "check-ignore" without "--verbose". * Just like the working tree is cleaned up when the user cancelled submission in P4Submit.applyCommit(), clean up the mess if "p4 submit" fails. * Having a leftover .idx file without corresponding .pack file in the repository hurts performance; "git gc" learned to prune them. * The code to prepare the working tree side of temporary directory for the "dir-diff" feature forgot that symbolic links need not be copied (or symlinked) to the temporary area, as the code already special cases and overwrites them. Besides, it was wrong to try computing the object name of the target of symbolic link, which may not even exist or may be a directory. * There was no way to defeat a configured rebase.autostash variable from the command line, as "git rebase --no-autostash" was missing. * Allow "git interpret-trailers" to run outside of a Git repository. * Produce correct "dirty" marker for shell prompts, even when we are on an orphan or an unborn branch. * Some corner cases have been fixed in string-matching done in "git status". * Apple's common crypto implementation of SHA1_Update() does not take more than 4GB at a time, and we now have a compile-time workaround for it. Also contains typofixes, documentation updates and trivial code clean-ups. Changes since v2.6.3 are as follows: Atousa Pahlevan Duprat (2): sha1: provide another level of indirection for the SHA-1 functions sha1: allow limiting the size of the data passed to SHA1_Update() Charles Bailey (1): http: treat config options sslCAPath and sslCAInfo as paths Christian Couder (1): Documentation/git-update-index: add missing opts to synopsys Clemens Buchacher (1): allow hooks to ignore their standard input stream Daniel Knittl-Frank (1): Escape Git's exec path in contrib/rerere-train.sh script David Aguilar (1): difftool: ignore symbolic links in use_wt_file Dennis Kaarsemaker (2): t5813: avoid creating urls that break on cygwin check-ignore: correct documentation about output Doug Kelly (2): t5304: test cleaning pack garbage gc: remove garbage .idx files from pack dir Fredrik Medley (1): rebase-i-exec: Allow space in SHELL_PATH GIRARD Etienne (1): git-p4: clean up after p4 submit failure John Keeping (3): interpret-trailers: allow running outside a repository rebase: support --no-autostash Documentation/git-rebase: fix --no-autostash formatting Junio C Hamano (3): prepare_packed_git(): refactor garbage reporting in pack directory Prepare for 2.6.4 Git 2.6.4 Luke Diamand (3): git-p4: add failing test for submit from detached head git-p4: add option to system() to return subshell status git-p4: work with a detached head Michael J Gruber (1): Documentation/diff: give --word-diff-regex=. example Pat Thoyts (1): remote-http(s): support SOCKS proxies Rainer M. Canavan (1): configure.ac: use $LIBS not $CFLAGS when testing -lpthread René Scharfe (1): wt-status: correct and simplify check for detached HEAD SZEDER Gábor (4): bash prompt: test dirty index and worktree while on an orphan branch bash prompt: remove a redundant 'git diff' option bash prompt: indicate dirty index even on orphan branches filter-branch: deal with object name vs. pathname ambiguity in
[PATCH v4] contrib/subtree: fix "subtree split" skipped-merge bug
'git subtree split' can incorrectly skip a merge even when both parents act on the subtree, provided the merge results in a tree identical to one of the parents. Fix by copying the merge if at least one parent is non-identical, and the non-identical parent is not an ancestor of the identical parent. Also, add a test case which checks that a descendant can be pushed to its ancestor in this case. Signed-off-by: Dave Ware--- Notes: Many thanks to Eric Sunshine and Junio Hamano for adivce on this patch Changes since v3: - Improvements to commit message - Removed incorrect use of --boundary on rev-list - Changed use of rev-list to use --count Changes since v2: - Minor improvements to commit message - Changed space indentation to tab indentation in test case - Changed use of rev-list for obtaining commit id to use rev-parse instead Changes since v1: - Minor improvements to commit message - Added sign off - Moved test case from own file into t7900-subtree.sh - Added subshell to test around 'cd' - Moved record of commit for cherry-pick to variable instead of dumping into file [v3]: http://thread.gmane.org/gmane.comp.version-control.git/282065/focus=282176 [v2]: http://thread.gmane.org/gmane.comp.version-control.git/282065/focus=282121 [v1]: http://thread.gmane.org/gmane.comp.version-control.git/282065 contrib/subtree/git-subtree.sh | 12 +++-- contrib/subtree/t/t7900-subtree.sh | 52 ++ 2 files changed, 62 insertions(+), 2 deletions(-) diff --git a/contrib/subtree/git-subtree.sh b/contrib/subtree/git-subtree.sh index 9f06571..ebf99d9 100755 --- a/contrib/subtree/git-subtree.sh +++ b/contrib/subtree/git-subtree.sh @@ -479,8 +479,16 @@ copy_or_skip() p="$p -p $parent" fi done - - if [ -n "$identical" ]; then + + copycommit= + if [ -n "$identical" ] && [ -n "$nonidentical" ]; then + extras=$(git rev-list --count $identical..$nonidentical) + if [ "$extras" -ne 0 ]; then + # we need to preserve history along the other branch + copycommit=1 + fi + fi + if [ -n "$identical" ] && [ -z "$copycommit" ]; then echo $identical else copy_commit $rev $tree "$p" || exit $? diff --git a/contrib/subtree/t/t7900-subtree.sh b/contrib/subtree/t/t7900-subtree.sh index 9051982..710278c 100755 --- a/contrib/subtree/t/t7900-subtree.sh +++ b/contrib/subtree/t/t7900-subtree.sh @@ -468,4 +468,56 @@ test_expect_success 'verify one file change per commit' ' )) ' +test_expect_success 'subtree descendent check' ' + mkdir git_subtree_split_check && + ( + cd git_subtree_split_check && + git init && + + mkdir folder && + + echo a >folder/a && + git add . && + git commit -m "first commit" && + + git branch branch && + + echo 0 >folder/0 && + git add . && + git commit -m "adding 0 to folder" && + + echo b >folder/b && + git add . && + git commit -m "adding b to folder" && + cherry=$(git rev-parse HEAD) && + + git checkout branch && + echo text >textBranch.txt && + git add . && + git commit -m "commit to fiddle with branch: branch" && + + git cherry-pick $cherry && + git checkout master && + git merge -m "merge" branch && + + git branch noop_branch && + + echo d >folder/d && + git add . && + git commit -m "adding d to folder" && + + git checkout noop_branch && + echo moreText >anotherText.txt && + git add . && + git commit -m "irrelevant" && + + git checkout master && + git merge -m "second merge" noop_branch && + + git subtree split --prefix folder/ --branch subtree_tip master && + git subtree split --prefix folder/ --branch subtree_branch branch && + git push . subtree_tip:subtree_branch + ) + ' + test_done -- 1.9.1 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3] contrib/subtree: fix "subtree split" skipped-merge bug
On Wed, Dec 9, 2015 at 10:23 AM, Junio C Hamanowrote: > Dave Ware writes: > >> A bug occurs in 'git-subtree split' where a merge is skipped even when >> both parents act on the subtree, provided the merge results in a tree >> identical to one of the parents. Fix by copying the merge if at least >> one parent is non-identical, and the non-identical parent is not an >> ancestor of the identical parent. >> >> Also, add a test case which checks that a descendant can be pushed to >> its ancestor in this case. >> >> Signed-off-by: Dave Ware >> --- > > The first sentence may be made clearer if you rephrased the early > part of the sentence this way: > > 'git subtree split' can incorrectly skip a merge even when > both parents ... > Noted. >> diff --git a/contrib/subtree/git-subtree.sh b/contrib/subtree/git-subtree.sh >> index 9f06571..b837531 100755 >> --- a/contrib/subtree/git-subtree.sh >> +++ b/contrib/subtree/git-subtree.sh >> @@ -479,8 +479,16 @@ copy_or_skip() >> p="$p -p $parent" >> fi >> done >> - >> - if [ -n "$identical" ]; then >> + >> + copycommit= >> + if [ -n "$identical" ] && [ -n "$nonidentical" ]; then >> + extras=$(git rev-list --boundary $identical..$nonidentical) >> + if [ -n "$extras" ]; then >> + # we need to preserve history along the other branch >> + copycommit=1 >> + fi > > What is the significance of "--boundary" here? I think for the > purpose of "is the identical one part of the nonidentical one?" you > do not need it, but there may be something subtle I missed. I am > asking this because use of "rev-list --boundary" in scripts is > almost always a bug. > The other way around actually I'm trying to determine if nonidentical contains any commits not in identical. I'll confess I don't actually know specifically what the --boundary option does, this probably came from a stack overflow example while we were looking up how to best do the check. Further experimentation with the option suggests that it does not do what I want, so I will remove it. Thank you. > Also, depending on how huge the output from the rev-list could be, > you might want to use "rev-list --count $i..$n" and compare it with > 0 instead--that way, you would not have to be worried about having > to carry around a huge string that you would otherwise not use, only > to see if that string is empty. Thanks, I didn't know about that option. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Update
Good day, hoping you read this email and respond to me in good time.I do not intend to solicit for funds but your time and energy in using my own resources to assist the less privileged becauseI am medically ill and confined at the moment hence I request your indulgence.I will give you a comprehensive brief once I hear from you. Thanks and reply. Robert Grondahl -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4] Documentation/git-update-index: add missing opts to synopsis
On Wed, Nov 25, 2015 at 10:30 AM, Christian Couderwrote: > Split index related options should appear in the 'SYNOPSIS' > section. > > These options are already documented in the 'OPTIONS' section. > > Signed-off-by: Christian Couder > --- > This v4 contains only the split-index options and applies on top > of the new master that already contains the untracked-cache options. It looks like this patch has not been applied. Maybe I should have given it a different title to avoid confusion with a previous patch that added [--[no-|force-]untracked-cache] in the SYNOPSIS. > Documentation/git-update-index.txt | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/Documentation/git-update-index.txt > b/Documentation/git-update-index.txt > index 3df9c26..f4e5a85 100644 > --- a/Documentation/git-update-index.txt > +++ b/Documentation/git-update-index.txt > @@ -17,6 +17,7 @@ SYNOPSIS > [--[no-]assume-unchanged] > [--[no-]skip-worktree] > [--ignore-submodules] > +[--[no-]split-index] > [--[no-|force-]untracked-cache] > [--really-refresh] [--unresolve] [--again | -g] > [--info-only] [--index-info] > -- > 2.6.3.380.g494b52d > -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3] contrib/subtree: fix "subtree split" skipped-merge bug
Dave Warewrites: > A bug occurs in 'git-subtree split' where a merge is skipped even when > both parents act on the subtree, provided the merge results in a tree > identical to one of the parents. Fix by copying the merge if at least > one parent is non-identical, and the non-identical parent is not an > ancestor of the identical parent. > > Also, add a test case which checks that a descendant can be pushed to > its ancestor in this case. > > Signed-off-by: Dave Ware > --- The first sentence may be made clearer if you rephrased the early part of the sentence this way: 'git subtree split' can incorrectly skip a merge even when both parents ... > diff --git a/contrib/subtree/git-subtree.sh b/contrib/subtree/git-subtree.sh > index 9f06571..b837531 100755 > --- a/contrib/subtree/git-subtree.sh > +++ b/contrib/subtree/git-subtree.sh > @@ -479,8 +479,16 @@ copy_or_skip() > p="$p -p $parent" > fi > done > - > - if [ -n "$identical" ]; then > + > + copycommit= > + if [ -n "$identical" ] && [ -n "$nonidentical" ]; then > + extras=$(git rev-list --boundary $identical..$nonidentical) > + if [ -n "$extras" ]; then > + # we need to preserve history along the other branch > + copycommit=1 > + fi What is the significance of "--boundary" here? I think for the purpose of "is the identical one part of the nonidentical one?" you do not need it, but there may be something subtle I missed. I am asking this because use of "rev-list --boundary" in scripts is almost always a bug. Also, depending on how huge the output from the rev-list could be, you might want to use "rev-list --count $i..$n" and compare it with 0 instead--that way, you would not have to be worried about having to carry around a huge string that you would otherwise not use, only to see if that string is empty. Thanks. > + fi > + if [ -n "$identical" ] && [ -z "$copycommit" ]; then > echo $identical > else > copy_commit $rev $tree "$p" || exit $? > diff --git a/contrib/subtree/t/t7900-subtree.sh > b/contrib/subtree/t/t7900-subtree.sh > index 9051982..710278c 100755 > --- a/contrib/subtree/t/t7900-subtree.sh > +++ b/contrib/subtree/t/t7900-subtree.sh > @@ -468,4 +468,56 @@ test_expect_success 'verify one file change per commit' ' > )) > ' > > +test_expect_success 'subtree descendent check' ' > + mkdir git_subtree_split_check && > + ( > + cd git_subtree_split_check && > + git init && > + > + mkdir folder && > + > + echo a >folder/a && > + git add . && > + git commit -m "first commit" && > + > + git branch branch && > + > + echo 0 >folder/0 && > + git add . && > + git commit -m "adding 0 to folder" && > + > + echo b >folder/b && > + git add . && > + git commit -m "adding b to folder" && > + cherry=$(git rev-parse HEAD) && > + > + git checkout branch && > + echo text >textBranch.txt && > + git add . && > + git commit -m "commit to fiddle with branch: branch" && > + > + git cherry-pick $cherry && > + git checkout master && > + git merge -m "merge" branch && > + > + git branch noop_branch && > + > + echo d >folder/d && > + git add . && > + git commit -m "adding d to folder" && > + > + git checkout noop_branch && > + echo moreText >anotherText.txt && > + git add . && > + git commit -m "irrelevant" && > + > + git checkout master && > + git merge -m "second merge" noop_branch && > + > + git subtree split --prefix folder/ --branch subtree_tip master > && > + git subtree split --prefix folder/ --branch subtree_branch > branch && > + git push . subtree_tip:subtree_branch > + ) > + ' > + > test_done -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html