[ANNOUNCE] tig-2.3.1
Hello, A new release is available fixing a few bugs and improving TTY management. What is Tig? Tig is an ncurses-based text-mode interface for git. It functions mainly as a Git repository browser, but can also assist in staging changes for commit at chunk level and act as a pager for output from various Git commands. - Homepage: https://jonas.github.io/tig/ - Manual: https://jonas.github.io/tig/doc/manual.html - Tarballs: https://github.com/jonas/tig/releases - Gitter: https://gitter.im/jonas/tig - Q: https://stackoverflow.com/questions/tagged/tig Release notes - Improvements: - Restore TTY attributes. (GH #725) - Handle `\n` like `\r`. (GH #758) Bug fixes: - Add workaround that detects busy loops when Tig loses the TTY. This may happen if Tig does not receive the HUP signal (e.g. when started with `nohup`). (GH #164) - Fix compatibility with ncurses-5.4 which caused copy-pasting to not work in the prompt. (GH #767) - tig(1): document correct environment variable. (GH #752) Change summary -- The diffstat and log summary for changes made in this release. .travis.yml | 50 ++--- INSTALL.adoc| 13 +++- Makefile| 2 +- NEWS.adoc | 17 + doc/tig.1.adoc | 2 +- include/tig/tig.h | 1 + src/display.c | 127 ++-- test/README.adoc| 17 + test/main/refresh-periodic-test | 2 + test/tools/libtest.sh | 76 +-- tools/aspell.dict | 12 ++- tools/travis.sh | 32 12 files changed, 293 insertions(+), 58 deletions(-) Christian Brabandt (1): Handle \n like \r (#758) David O'Trakoun (1): tig(1): Fix env var checked (#752) Jonas Fonseca (6): Fix formatting of the Windows install documentation Move loop updating views to separate method Fix #164: Add workaround to detect busy event loops Use initscr to ensure proper TTY setup for the prompt (#768) Update NEWS tig-2.3.1 Matt (1): Added another installation method (#753) -- Jonas Fonseca
Re: [PATCH] diffcore: add a filter to find a specific blob
Jonathan Niederwrites: >> Regarding finding a better name, I would want to hear from others, >> I am happy with --find-object, though I can see --pickaxe-object >> or --object--filter to be a good narrative as well. > > Drat, I was hoping for an opinion. I think it would make it a better companion to --pickaxe but we need to align its behaviour a little bit so that it plays better with the "--pickaxe-all" option, and also needs to hide mode and name only changes just like pickaxe. After all, the diffcore-blobfind code was written while looking at the diffcore-pickaxe's code in another window shown in the pager, and I tend to agree with your earlier message that this is an extreme case of -S where the contents happens to be the whole file.
Re: Need help migrating workflow from svn to git.
On 14/12/2017 23:27, Igor Djordjevic wrote: > > As you basically have a flow where two users (you and cron job) can > edit same files at the same time, desired outcome might be a bit > ambiguous, especially when scheduled execution of those files is > added to the mix. This said, and without having you to change your habits too much (nor use Git in possibly awkward ways), I`m thinking you may actually benefit of using `git worktree add `[1] to create a temporary working tree ("working copy", as you say), alongside a temporary branch, where you could hack and test as much as you want, unaffected by cron job updating and executing the original working copy/branch (also not stepping on cron job`s toes yourself). Once you`re satisfied and you commit/merge/push your changes from within the temporary working copy/branch, you can just delete it (both temporary working copy and its branch), and you`re good :) p.s. Even if you`re not familiar with Git branching and merging, it shouldn`t take too much effort to wrap your head around it, and it`s definitely worth it - and actually pretty easy, even more if you`re working alone. [1] https://git-scm.com/docs/git-worktree
Re: [PATCH 1/4] travis-ci: use 'set -x' in 'ci/*' scripts for extra tracing output
On Thu, Dec 14, 2017 at 12:10 AM, Lars Schneiderwrote: > >> On 12 Dec 2017, at 19:43, SZEDER Gábor wrote: >> >> On Tue, Dec 12, 2017 at 7:00 PM, Lars Schneider >> wrote: >>> On 12 Dec 2017, at 00:34, SZEDER Gábor wrote: While the build logic was embedded in our '.travis.yml', Travis CI used to produce a nice trace log including all commands executed in those embedded scriptlets. Since 657343a60 (travis-ci: move Travis CI code into dedicated scripts, 2017-09-10), however, we only see the name of the dedicated scripts, but not what those scripts are actually doing, resulting in a less useful trace log. A patch later in this series will move setting environment variables from '.travis.yml' to the 'ci/*' scripts, so not even those will be included in the trace log. Use 'set -x' in 'ci/lib-travisci.sh', which is sourced in most other 'ci/*' scripts, so we get trace log about the commands executed in all of those scripts. >>> >>> I kind of did that intentionally to avoid clutter in the logs. >>> However, I also agree with your reasoning. Therefore, the change >>> looks good to me! >> >> Great, 'cause I'm starting to have second thoughts about this change :) >> >> It sure helped a lot while I worked on this patch series and a couple of >> other Travis CI related patches (will submit them later)... OTOH it >> definitely creates clutter in the trace log. The worst offender might >> be 'ci/print-test-failures.sh', which iterates over all >> 't/test-results/*.exit' files to find which tests failed and to show >> their output, and 'set -x' makes every iteration visible. And we have >> about 800 tests, which means 800 iterations. Yuck. >> >> Perhaps we should use other means to show what's going on instead, e.g. >> use more 'echo's and '--verbose' options, or just avoid using '--quiet'. >> And if some brave souls really want to tweak '.travis.yml' or the 'ci/*' >> scripts, then they can set 'set -x' for themselves during development... >> as the patch below shows it's easy enough, just a single character :) > > Hm... in that case. Would it be an option to "set -x" only in the header > of "install-dependencies.sh"? > > In "lib-travisci.sh" we could keep your "set -x" and execute > "set +x" at the end of the file. Wouldn't that give us the > interesting traces without much clutter (e.g. what is $PATH etc)? Hmm, that's an idea worth considering. Scripts like 'run-build.sh', 'run-tests.sh' and 'run-static-analysis.sh' do basically nothing more than run make with different targets, so on one hand 'set -x' doesn't cause any clutter in the trace log, on the other hand there is no benefit from it either. 'run-linux32-docker.sh' runs docker (the command) twice, so it's basically in the same boat. I think both 'lib-travisci.sh' and 'install-dependencies.sh' benefit from 'set -x'. So does 'test-documentation.sh': it executes about 15 commands, among them a bunch of 'test -s ' which fail quietly. With 'set -x' we would see the last executed command and know that that's the one that failed. As mentioned above, 'print-test-failures.sh' definitely suffers from 'set -x'. There is a lot going on in 'run-windows-build.sh', so the output of 'set -x' might be useful or might be considered too much clutter, I don't know. I put Dscho on Cc, I think it's mainly his call. So it seems that there are more scripts that would benefit from tracing executed command using 'set -x' than scripts that would suffer because of it, and it doesn't matter for the rest. This means we could issue a 'set -x' in 'lib-travisci.sh' and disable it only in 'print-test-failures.sh'. There is one thing that triggers my OCD: whenever we echo something, it ends up being duplicated in the trace log, e.g.: + echo foo bar baz foo bar baz We could workaround it by writing 'echo "" >/dev/null', but it feels hackish and I'm not sure it's worth it. Gábor Signed-off-by: SZEDER Gábor --- ci/lib-travisci.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ci/lib-travisci.sh b/ci/lib-travisci.sh index ac05f1f46..a0c8ae03f 100755 --- a/ci/lib-travisci.sh +++ b/ci/lib-travisci.sh @@ -23,7 +23,7 @@ skip_branch_tip_with_tag () { # Set 'exit on error' for all CI scripts to let the caller know that # something went wrong -set -e +set -ex skip_branch_tip_with_tag -- 2.15.1.421.gc469ca1de >>> >
Re: [PATCH] git-svn: convert CRLF to LF in commit message to SVN
Hi Brian, Bennett, Brian wrote: > Thank you for your fast response, > > I haven't done a build of this type before (so I could > test the patch first) so I'm trying to do that and get > this far: ... > I don't want to drag out testing the patch, so if either > of you are able to quickly guide me on what I am doing > incorrectly I am willing to get the build done so I can > test it. If not, could one of you build with the patch and > somehow get that to me so I could test? I don't know about building git for windows, but since the git-svn command is a perl script, it might be easier to just patch that file. I think you can find the path where git-svn is installed using: git --exec-path For this one-liner, I'd just manually apply it. (If you want to use 'git apply' or the patch command, you'll have to edit the patch to adjust the name of the file, as it's git-svn.perl in the git tree. The .perl suffix is dropped in the installed version.) Hopefully that makes it easier for you to test Eric's patch. -- Todd ~~ I am in shape. Round is a shape.
[PATCH v2 2/2] version --build-options: report commit, too, if possible
In particular when local tags are used (or tags that are pushed to some fork) to build Git, it is very hard to figure out from which particular revision a particular Git executable was built. It gets worse when those tags are deleted, or even updated. Let's just report an exact, unabbreviated commit name in our build options. We need to be careful, though, to report when the current commit cannot be determined, e.g. when building from a tarball without any associated Git repository. This could be the case also when extracting Git's source code into an unrelated Git worktree. Signed-off-by: Johannes Schindelin--- Makefile | 4 +++- help.c| 5 + version.c | 1 + version.h | 1 + 4 files changed, 10 insertions(+), 1 deletion(-) diff --git a/Makefile b/Makefile index 5587bccc932..2ce70d205d9 100644 --- a/Makefile +++ b/Makefile @@ -1902,7 +1902,9 @@ builtin/help.sp builtin/help.s builtin/help.o: EXTRA_CPPFLAGS = \ version.sp version.s version.o: GIT-VERSION-FILE GIT-USER-AGENT version.sp version.s version.o: EXTRA_CPPFLAGS = \ '-DGIT_VERSION="$(GIT_VERSION)"' \ - '-DGIT_USER_AGENT=$(GIT_USER_AGENT_CQ_SQ)' + '-DGIT_USER_AGENT=$(GIT_USER_AGENT_CQ_SQ)' \ + '-DGIT_BUILT_FROM_COMMIT="$(shell GIT_CEILING_DIRECTORIES=\"$(CURDIR)/..\" \ + git rev-parse -q --verify HEAD || :)"' $(BUILT_INS): git$X $(QUIET_BUILT_IN)$(RM) $@ && \ diff --git a/help.c b/help.c index cbcb159f367..60071a9beaa 100644 --- a/help.c +++ b/help.c @@ -413,6 +413,11 @@ int cmd_version(int argc, const char **argv, const char *prefix) if (build_options) { printf("cpu: %s\n", GIT_HOST_CPU); + if (git_built_from_commit_string[0]) + printf("built from commit: %s\n", + git_built_from_commit_string); + else + printf("no commit associated with this build\n"); printf("sizeof-long: %d\n", (int)sizeof(long)); /* NEEDSWORK: also save and output GIT-BUILD_OPTIONS? */ } diff --git a/version.c b/version.c index 6106a8098c8..41b718c29e1 100644 --- a/version.c +++ b/version.c @@ -3,6 +3,7 @@ #include "strbuf.h" const char git_version_string[] = GIT_VERSION; +const char git_built_from_commit_string[] = GIT_BUILT_FROM_COMMIT; const char *git_user_agent(void) { diff --git a/version.h b/version.h index 6911a4f1558..7c62e805771 100644 --- a/version.h +++ b/version.h @@ -2,6 +2,7 @@ #define VERSION_H extern const char git_version_string[]; +extern const char git_built_from_commit_string[]; const char *git_user_agent(void); const char *git_user_agent_sanitized(void); -- 2.15.1.windows.2
[PATCH v2 1/2] version --build-options: also report host CPU
From: Eric SunshineIt can be helpful for bug reports to include information about the environment in which the bug occurs. "git version --build-options" can help to supplement this information. In addition to the size of 'long' already reported by --build-options, also report the host's CPU type. Example output: $ git version --build-options git version 2.9.3.windows.2.826.g06c0f2f cpu: x86_64 sizeof-long: 4 New Makefile variable HOST_CPU supports cross-compiling. Suggested-by: Adric Norris Signed-off-by: Eric Sunshine Signed-off-by: Johannes Schindelin --- Makefile | 9 + help.c | 1 + 2 files changed, 10 insertions(+) diff --git a/Makefile b/Makefile index fef9c8d2725..5587bccc932 100644 --- a/Makefile +++ b/Makefile @@ -425,6 +425,9 @@ all:: # # to say "export LESS=FRX (and LV=-c) if the environment variable # LESS (and LV) is not set, respectively". +# +# When cross-compiling, define HOST_CPU as the canonical name of the CPU on +# which the built Git will run (for instance "x86_64"). GIT-VERSION-FILE: FORCE @$(SHELL_PATH) ./GIT-VERSION-GEN @@ -1095,6 +1098,12 @@ else BROKEN_PATH_FIX = '/^\# @@BROKEN_PATH_FIX@@$$/d' endif +ifeq (,$(HOST_CPU)) + BASIC_CFLAGS += -DGIT_HOST_CPU="\"$(firstword $(subst -, ,$(uname_M)))\"" +else + BASIC_CFLAGS += -DGIT_HOST_CPU="\"$(HOST_CPU)\"" +endif + ifneq (,$(INLINE)) BASIC_CFLAGS += -Dinline=$(INLINE) endif diff --git a/help.c b/help.c index 88a3aeaeb9f..cbcb159f367 100644 --- a/help.c +++ b/help.c @@ -412,6 +412,7 @@ int cmd_version(int argc, const char **argv, const char *prefix) printf("git version %s\n", git_version_string); if (build_options) { + printf("cpu: %s\n", GIT_HOST_CPU); printf("sizeof-long: %d\n", (int)sizeof(long)); /* NEEDSWORK: also save and output GIT-BUILD_OPTIONS? */ } -- 2.15.1.windows.2
[PATCH v2 0/2] Offer more information in `git version --build-options`'s output
In Git for Windows, we ask users to paste the output of said command into their bug reports, with the idea that this frequently helps identify where the problems are coming from. There are some obvious missing bits of information in said output, though, and this patch series tries to fill the gaps at least a little. Changes since v1: - replaced 1/2 by Eric's proposed alternative patch - when no commit can be determined, it now says no commit associated with this build instead of built from commit: (unknown) - the code is now careful not to look further than Git's top-level directory for a Git repository from which to determine the current commit. As Junio pointed out, some developer may extract Git's source code from a tarball into a worktree of a completely different project. Eric Sunshine (1): version --build-options: also report host CPU Johannes Schindelin (1): version --build-options: report commit, too, if possible Makefile | 13 - help.c| 6 ++ version.c | 1 + version.h | 1 + 4 files changed, 20 insertions(+), 1 deletion(-) base-commit: 95ec6b1b3393eb6e26da40c565520a8db9796e9f Published-As: https://github.com/dscho/git/releases/tag/built-from-commit-v2 Fetch-It-Via: git fetch https://github.com/dscho/git built-from-commit-v2 Interdiff vs v1: diff --git a/Makefile b/Makefile index 92a0ae3d8e3..2ce70d205d9 100644 --- a/Makefile +++ b/Makefile @@ -425,6 +425,9 @@ all:: # # to say "export LESS=FRX (and LV=-c) if the environment variable # LESS (and LV) is not set, respectively". +# +# When cross-compiling, define HOST_CPU as the canonical name of the CPU on +# which the built Git will run (for instance "x86_64"). GIT-VERSION-FILE: FORCE @$(SHELL_PATH) ./GIT-VERSION-GEN @@ -1095,6 +1098,12 @@ else BROKEN_PATH_FIX = '/^\# @@BROKEN_PATH_FIX@@$$/d' endif +ifeq (,$(HOST_CPU)) + BASIC_CFLAGS += -DGIT_HOST_CPU="\"$(firstword $(subst -, ,$(uname_M)))\"" +else + BASIC_CFLAGS += -DGIT_HOST_CPU="\"$(HOST_CPU)\"" +endif + ifneq (,$(INLINE)) BASIC_CFLAGS += -Dinline=$(INLINE) endif @@ -1894,8 +1903,8 @@ version.sp version.s version.o: GIT-VERSION-FILE GIT-USER-AGENT version.sp version.s version.o: EXTRA_CPPFLAGS = \ '-DGIT_VERSION="$(GIT_VERSION)"' \ '-DGIT_USER_AGENT=$(GIT_USER_AGENT_CQ_SQ)' \ - '-DGIT_BUILT_FROM_COMMIT="$(shell git rev-parse -q --verify HEAD || \ - echo "(unknown)")"' + '-DGIT_BUILT_FROM_COMMIT="$(shell GIT_CEILING_DIRECTORIES=\"$(CURDIR)/..\" \ + git rev-parse -q --verify HEAD || :)"' $(BUILT_INS): git$X $(QUIET_BUILT_IN)$(RM) $@ && \ diff --git a/help.c b/help.c index 6ebea665780..60071a9beaa 100644 --- a/help.c +++ b/help.c @@ -390,7 +390,6 @@ const char *help_unknown_cmd(const char *cmd) int cmd_version(int argc, const char **argv, const char *prefix) { - static char build_platform[] = GIT_BUILD_PLATFORM; int build_options = 0; const char * const usage[] = { N_("git version []"), @@ -413,10 +412,13 @@ int cmd_version(int argc, const char **argv, const char *prefix) printf("git version %s\n", git_version_string); if (build_options) { - printf("built from commit: %s\n", - git_built_from_commit_string); + printf("cpu: %s\n", GIT_HOST_CPU); + if (git_built_from_commit_string[0]) + printf("built from commit: %s\n", + git_built_from_commit_string); + else + printf("no commit associated with this build\n"); printf("sizeof-long: %d\n", (int)sizeof(long)); - printf("machine: %s\n", build_platform); /* NEEDSWORK: also save and output GIT-BUILD_OPTIONS? */ } return 0; diff --git a/help.h b/help.h index 42dd9852194..b21d7c94e8c 100644 --- a/help.h +++ b/help.h @@ -33,16 +33,3 @@ extern void list_commands(unsigned int colopts, struct cmdnames *main_cmds, stru */ extern void help_unknown_ref(const char *ref, const char *cmd, const char *error); #endif /* HELP_H */ - -/* - * identify build platform - */ -#ifndef GIT_BUILD_PLATFORM - #if defined __x86__ || defined __i386__ || defined __i586__ || defined __i686__ - #define GIT_BUILD_PLATFORM "x86" - #elif defined __x86_64__ - #define GIT_BUILD_PLATFORM "x86_64" - #else - #define GIT_BUILD_PLATFORM "unknown" - #endif -#endif -- 2.15.1.windows.2
Re: [PATCH 2/2] version --build-options: report commit, too, if possible
Hi Junio, On Fri, 8 Dec 2017, Junio C Hamano wrote: > Jonathan Niederwrites: > > >> We need to be careful, though, to report when the current commit cannot be > >> determined, e.g. when building from a tarball without any associated Git > >> repository. > > > > This means that on Debian, it would always print > > > > built from commit: (unknown) > > > > Maybe I shouldn't care, but I wonder if there's a way to improve on > > that. E.g. should there be a makefile knob to allow Debian to specify > > what to put there? > > Another "interesting" possibility is to build from a tarball > extracted into a directory hierarchy that is controlled by an > unrelated Git repository. E.g. "my $HOME is under $HOME/.git > repository, and then I have a tarball extract in $HOME/src/git". > We shouldn't embed the HEAD commit of that $HOME directory project > in the resulting executable in such a case. > > We should be able to do this by being a bit more careful than the > presented patch. Make sure that the toplevel is at the same > directory as we assumed to be (i.e. where we found that Makefile) > and trust rev-parse output only when that is the case, or something > like that. Cute. I added specific handling for that. Ciao, Dscho
Re: [PATCH 2/2] version --build-options: report commit, too, if possible
Hi Jonathan, On Fri, 8 Dec 2017, Jonathan Nieder wrote: > Johannes Schindelin wrote: > > > In particular when local tags are used (or tags that are pushed to some > > fork) to build Git, it is very hard to figure out from which particular > > revision a particular Git executable was built. > > Hm, can you say more about how this comes up in practice? I recently saw a version string on this here list (in a generated patch) that looked something like "git v2.14.0.chrome". I sometimes build custom Git for Windows snapshots from commits that I keep in my own fork. I would expect other people to do the same. With this patch, at least I can verify very easily whether I have access to the corresponding commit or not. > I wonder if we should always augment the version string with the commit > hash. That would probably be more confusing than helpful to the end-users. > E.g. I am running > > git version 2.15.1.424.g9478a66081 which is currently good enough, but in the future may clash with another object, maybe even a commit. Unabbreviated commit names are what I am after. > > We need to be careful, though, to report when the current commit cannot be > > determined, e.g. when building from a tarball without any associated Git > > repository. > > This means that on Debian, it would always print > > built from commit: (unknown) > > Maybe I shouldn't care, but I wonder if there's a way to improve on > that. E.g. should there be a makefile knob to allow Debian to specify > what to put there? I changed the text to "no commit associated with this build". Does that suffice? If not, what "UI" would you suggest (most likely a new Makefile variable? What name would you prefer?). Ciao, Dscho
Re: [PATCH v1] convert: add support for 'encoding' attribute
Lars Schneiderwrites: > That way you could control the encoding for a text file specific > for each path similar to the "mode bits". That also means you could > change the encoding of a file while the blob content stays the same. That is exactly why I said that I doubt it makes sense. When you have the same blob object (that is in UTF-8) appear at two places in a tree (or two different subtrees inside a single tree) and the two tree entries that point at the blob tells contradicting story, you would have a checkout of the same contents in two different encodings. If you have the same blob object appear in two adjacent commits at the same path, with one commit's tree recording its encoding differently from the other's, you would switch encodings when you switch branches. I doubt these are "features", but sources of confusion. Be it a feature, or a misfeature, it is shared with the existing solution which is .gitattributes embedded in the tree, so you are not making anything better or worse by moving the information to tree entry. What I actually expected to hear when somebody talks about "ideal" (which may not even be achievable given existing user base) was to make blob object declare its desired external representation. That would remove the two possible confusion cited above, because once you have a blob, you have everything needed to externalize it. In any case, I do not think we'd go there anyway, so ...
Re: [PATCH] diffcore: add a filter to find a specific blob
Hi, Stefan Beller wrote: > On Thu, Dec 14, 2017 at 1:22 PM, Jonathan Niederwrote: >> - what about mode changes? If the file became executable but the >> blob content didn't change, does that commit match? > > ./git log --find-object=$(git rev-parse ba67504f:t/perf/p3400-rebase.sh) > > claims it does find the mode change (commit ba67504f is just a mode > change) Thanks. Reminder to self to add a test + docs about that (as a followup change; this isn't a complaint about the patch). >> - are copies and renames shown (if I am passing -M -C)? > > It restricts the commits shown, not the renamed files. But I assume > you mean it the same way as with mode changes. > I did not find a good commit in gits history to demonstrate, but as > it is orthogonal to the object id restrictions, I would think it works Ok, will add test + doc. >> Nit, not related to this change: it would be nice to have a long >> option to go along with the short name '-t' --- e.g. --include-trees. > > follow up patches welcome. :) Will think more and try to send a patch if it still seems like a good idea in a day or so. ;) >> Another nit: s/gitlink entry/submodule commit/, perhaps. The commit >> object is not a gitlink entry: it is pointed to by a gitlink entry. > > Well, what if the user doesn't have a submodule, but uses gitlinks > for other purposes? We do inspect the gitlink, so it is correct IMHO. It's a language nit. The argument to --find-object is a commit object name, not a gitlink entry. A gitlink entry looks like 16 >> Another documentation idea: it may be nice to point out that this >> is only about the preimage and postimage submodule commit and that >> it doesn't look at the history in between. > > That is sensible. One might be tempted to ask: "Which superproject > commit contains a submodule pointer, that has commit $X in the > submodule history?", but this new option is totally not answering this. Ok, will try to come up with wording. >>> The >>> reason why these commits both occur prior to v2.0.0 are evil >>> merges that are not found using this new mechanism. >> >> Would it be worth the doc mentioning that this doesn't look at merges >> unless you use one of the -m/-c/--cc options, or does that go without >> saying? > > I assumed it goes without saying, just like the lacking -t could mean > to ignore trees. ;) I suspect it's worth a mention, based on the discussion in this thread (i.e. without such docs it was non-obvious and took some time to diagnose). [...] >>> +--find-object=:: >>> + Restrict the output such that one side of the diff >>> + matches the given object id. The object can be a blob, >>> + gitlink entry or tree (when `-t` is given). >> >> I like this name --find-object more than --blobfind! I am not sure it >> quite matches what the user is looking for, though. We are not >> looking for all occurences of the object; we only care about when the >> object appears (was added or removed) in the diff. > > Thanks! Yes, but the 'edges' are so few commits that a further walk > will reveal all you need to know? Sorry for the lack of clarity: I actually like this behavior *more* than a "find trees pointing to object" behavior. I'm just saying the name sets an unclear expectation. [...] > Regarding finding a better name, I would want to hear from others, > I am happy with --find-object, though I can see --pickaxe-object > or --object--filter to be a good narrative as well. Drat, I was hoping for an opinion. Based on the answers above about mode changes and renames, at the moment --object-filter seems clearest to me. Thanks, Jonathan
Re: [PATCH] diffcore: add a filter to find a specific blob
Jonathan Niederwrites: > Putting it in context, we have: > > pickaxe options: > -S: detect changes in occurence count of a string > -G: grep lines in diff for a string > > --pickaxe-all: > do not filter the diff when the patch matches pickaxe > conditions. > > kind of like log --full-diff, but restricted to pickaxe > options. > --pickaxe-regex: treat -S argument as a regex, not a string > > Is this another kind of pickaxe option? It feels similar to -S, but > at an object level instead of a substring level, so in a way it would > be appealing to call it --pickaxe-object. Does --pickaxe-all affect > it like it affects -S and -G? It does feel quite close to -S, but is slightly different. Instead of spelling out the exact contents to pass as the argument to the -S option, you are giving a blob object name instead. However, if you rename path F to G without any modification, or if you flip only the mode bits, -S would reject such a filepair, as both sides have the same number of the sought-after string, I think. Unlike -S, this new thing should show such a filepair because it is designed to show any filepair with either (or both) side has the sought-after object. We could make this identical to -S, i.e. only when a filepair has sought-after object on either (or both) sides *and* its pre and post image sides do not talk about the same object. Note. I am phrasing the above awkwardly because we can ask for more than one object, and a filepair that changes a path from object A to object B, both of which are what we are looking for, want to be caught. If I phrased the above as "only when only one side of a filepair has an object we are looking for and the other side does not", such a filepair would not be shown. I have a suspicion that such a change may be worthwhile. > Another context to put it in is: > > --diff-filter: > limit paths (but not commits?) to those with a change > matching optarg IIUC, this is applied at the output phase after all of the rename, pickaxe etc. are done (e.g. "Now you have the diff to give me, show me only the additions in it"). It is a poor match for this new thing, as "Now you have the diff to give me, show me only the ones that involve these blobs" does not sound all that useful.
Re: [PATCH] diffcore: add a filter to find a specific blob
On Thu, Dec 14, 2017 at 1:22 PM, Jonathan Niederwrote: > - what about mode changes? If the file became executable but the > blob content didn't change, does that commit match? ./git log --find-object=$(git rev-parse ba67504f:t/perf/p3400-rebase.sh) claims it does find the mode change (commit ba67504f is just a mode change) > - are copies and renames shown (if I am passing -M -C)? It restricts the commits shown, not the renamed files. But I assume you mean it the same way as with mode changes. I did not find a good commit in gits history to demonstrate, but as it is orthogonal to the object id restrictions, I would think it works > Nit, not related to this change: it would be nice to have a long > option to go along with the short name '-t' --- e.g. --include-trees. follow up patches welcome. :) > > Another nit: s/gitlink entry/submodule commit/, perhaps. The commit > object is not a gitlink entry: it is pointed to by a gitlink entry. Well, what if the user doesn't have a submodule, but uses gitlinks for other purposes? We do inspect the gitlink, so it is correct IMHO. > Another documentation idea: it may be nice to point out that this > is only about the preimage and postimage submodule commit and that > it doesn't look at the history in between. That is sensible. One might be tempted to ask: "Which superproject commit contains a submodule pointer, that has commit $X in the submodule history?", but this new option is totally not answering this. >> The >> reason why these commits both occur prior to v2.0.0 are evil >> merges that are not found using this new mechanism. > > Would it be worth the doc mentioning that this doesn't look at merges > unless you use one of the -m/-c/--cc options, or does that go without > saying? I assumed it goes without saying, just like the lacking -t could mean to ignore trees. ;) > > [...] >> --- a/Documentation/diff-options.txt >> +++ b/Documentation/diff-options.txt >> @@ -500,6 +500,12 @@ information. >> --pickaxe-regex:: >> Treat the given to `-S` as an extended POSIX regular >> expression to match. >> + >> +--find-object=:: >> + Restrict the output such that one side of the diff >> + matches the given object id. The object can be a blob, >> + gitlink entry or tree (when `-t` is given). > > I like this name --find-object more than --blobfind! I am not sure it > quite matches what the user is looking for, though. We are not > looking for all occurences of the object; we only care about when the > object appears (was added or removed) in the diff. Thanks! Yes, but the 'edges' are so few commits that a further walk will reveal all you need to know? > > Putting it in context, we have: > > pickaxe options: > -S: detect changes in occurence count of a string > -G: grep lines in diff for a string > > --pickaxe-all: > do not filter the diff when the patch matches pickaxe > conditions. > > kind of like log --full-diff, but restricted to pickaxe > options. > --pickaxe-regex: treat -S argument as a regex, not a string > > Is this another kind of pickaxe option? It feels similar to -S, but > at an object level instead of a substring level, so in a way it would > be appealing to call it --pickaxe-object. Does --pickaxe-all affect > it like it affects -S and -G? > > Another context to put it in is: > > --diff-filter: > limit paths (but not commits?) to those with a change > matching optarg > > If I understand correctly, then --diff-filter does not interact with > --pickaxe-all, or in other words it is a different filtering > condition. Is this another kind of diff filter? In that context, it > may be appealing to call it something like --object-filter. > > --diff-filter is an example where it seems appealing to have a > --full-diff option to diff-tree that could apply to all filters and > not just pickaxe. > > [... implementation snipped ...] > > The implementation looks lovely and I'm especially happy about the > tests. Thanks for writing it. > > Thoughts? > Jonathan Regarding finding a better name, I would want to hear from others, I am happy with --find-object, though I can see --pickaxe-object or --object--filter to be a good narrative as well. Stefan
Re: Need help migrating workflow from svn to git.
Hi Josef, I`m not a Git expert, and I know less of Subversion, but following your explanation, I might try to help, at least until more experienced people join. On 14/12/2017 14:09, Josef Wolf wrote: > > Every machine has a working copy of the repository in a specific > directory. A cron job (running every 15 minutes) executes "svn > update" and executes the scripts which are contained in this working > copy. > ... > Sometimes, I need to fix a problem on some host or need to implement > a new feature. For this, I go to the working copy of a host where the > change needs to be done and start haking. With svn, I don't need to > stop the cron job. "svn update" will happily merge any in-coming > changes and leave alone the files which were not modified upstream. > Conflicts with my local modifications which I am currently hacking on > are extremely rare, because the scripts are pretty much independent. > So I'm pretty much happy with this mode of operation. Aside "update and merge" working copy while you`re hacking on it, what happens with "execute" part? It seems really strange that you don`t mind cron job running the same scripts which you are actively working on, thus being in an inconsistent state, if not broken, even. > With git, by contrast, this won't work. Git will refuse to pull > anything as long as there are ANY local modifications. Not sure what`s happening at your end, but "ANY" part shouldn`t be true - you can have local modifications and still execute `git pull` successfully. Only if you have local modifications in files that _also_ changed on the remote end, `git pull` aborts (fetch of the remote branch succeeds, actually, just merge with local branch is aborted). Now, having in mind you said conflicts are extremely rare in your flow anyway, would this be enough for you? Of course, provided that issue you`re having with being unable to `git pull` with ANY local modifications, as you wrote, is resolved first. > The cron job would need to > >git stash >git pull >git stash pop > > But this will temporarily remove my local modifications. If I happen > to do a test run at this time, the test run would NOT contain the > local modifications which I was about to test. Even worse: if I > happen to save one of the modified files while the modifications are > in the stash, the "git stash pop" will definitely cause a conflict, > although nothing really changed. Is `git stash pop` causing conflicts your only concern here? How about a situation where you save one of the modified files _after_ `git stash pop` was successful, effectively discarding any updates introduced by `git pull` from remote end...? As you basically have a flow where two users (you and cron job) can edit same files at the same time, desired outcome might be a bit ambiguous, especially when scheduled execution of those files is added to the mix. > So, how would I get this workflow with git? Is it possible to emulate > the behavior of "svn update"? > > Any ideas? I`m thinking of a workflow involving (scripted) creation of a temporary branch at fetched remote branch position, and using something like `git checkout --merge ` to merge your local modifications to latest changes fetched from remote (ending up with conflicts inside working tree, if any), which would seem to simulate `svn update` as desired (if I understand it correctly), but it might be good to address some of the concerns I raised above first. Regards, Buga
Re: [PATCH 2/2] transport: make transport vtable more private
Jonathan Tanwrites: > Move the definition of the transport-specific functions provided by > transports, whether declared in transport.c or transport-helper.c, into > an internal header. This means that transport-using code (as opposed to > transport-declaring code) can no longer access these functions (without > importing the internal header themselves), making it clear that they > should use the transport_*() functions instead, and also allowing the > interface between the transport mechanism and an individual transport to > independently evolve. Yay!
Re: [PATCH 1/2] clone, fetch: remove redundant transport check
Jonathan Tanwrites: > Prior to commit a2d725b7bdf7 ("Use an external program to implement > fetching with curl", 2009-08-05), if Git was compiled with NO_CURL, the > get_refs_list and fetch methods in struct transport might not be > populated, hence the checks in clone and fetch. After that commit, all > transports populate get_refs_list and fetch, making the checks in clone > and fetch redundant. Remove those checks. I do not agree with the above justification. We could view these checks as serving to future-proof the callers for (initially buggy) transports that we have not seen, so the current set of transports not triggering is not a good enough reason to remove them. But I do agree with the removal of these checks for another reason. A call into transport_get_remote_refs() or transport_fetch_refs() would segfault if these necessary fields are not filled in a buggy transport under development anyway. And more importantly, if it is desirable to have a check that is more friendly than merely segfaulting, such a check must be added to the implementation of transport API that knows the implementation details like "fetching would require get_refs_list and fetch fields"; the consumers of the API is a wrong place to do so. Thanks. > Signed-off-by: Jonathan Tan > --- > builtin/clone.c | 3 --- > builtin/fetch.c | 3 --- > 2 files changed, 6 deletions(-) > > diff --git a/builtin/clone.c b/builtin/clone.c > index dbddd98f8..979af0383 100644 > --- a/builtin/clone.c > +++ b/builtin/clone.c > @@ -1083,9 +1083,6 @@ int cmd_clone(int argc, const char **argv, const char > *prefix) > warning(_("--local is ignored")); > transport->cloning = 1; > > - if (!transport->get_refs_list || (!is_local && !transport->fetch)) > - die(_("Don't know how to clone %s"), transport->url); > - > transport_set_option(transport, TRANS_OPT_KEEP, "yes"); > > if (option_depth) > diff --git a/builtin/fetch.c b/builtin/fetch.c > index 225c73492..09eb1fc17 100644 > --- a/builtin/fetch.c > +++ b/builtin/fetch.c > @@ -1094,9 +1094,6 @@ static int do_fetch(struct transport *transport, > tags = TAGS_UNSET; > } > > - if (!transport->get_refs_list || !transport->fetch) > - die(_("Don't know how to fetch from %s"), transport->url); > - > /* if not appending, truncate FETCH_HEAD */ > if (!append && !dry_run) { > retcode = truncate_fetch_head();
Question about the ahead-behind computation and status
We working with the (enormous) Windows repo, we observed a performance problem in the ahead-behind computation and were considering a few options. We had a local repo with a local branch that was 150K commits behind the upstream branch[*]. There was a ~20 second different in the run times for these 2 commands: $ git status --porcelain=v2 $ git status --porcelain=v2 --branch Profiling showed the additional time was spent computing the ahead/behind values for the branch. (The problem is not specific to porcelain V2, that was just the command where we discovered it; for example, there is a similar perf problem in "git branch" vs "git branch -vv".) I don't want to jump into the graph algorithm at this time, but was wondering about adding a --no-ahead-behind flag (or something similar or a config setting) that would disable the a/b computation during status. For status V2 output, we could omit the "# branch.ab x y" line. For normal status output, change the prose a/b message to say something like "are [not] up to date". Thoughts or suggestions ??? Thanks, Jeff [*] Sadly, the local repo was only about 20 days out of date (including the Thanksgiving holidays)
[PATCH 0/2] More transport API improvements
Note that this is built on jt/transport-no-more-rsync. I have found the transport mechanism relatively complicated, so here is some more effort in the hope of making it more readily understood. Patch 1 is probably good to go in as-is. Patch 2 is a modification of the transport API by making certain variables in the transport interface struct more private, and might need more discussion. I also discuss the possible future work that this modification makes possible. Jonathan Tan (2): clone, fetch: remove redundant transport check transport: make transport vtable more private builtin/clone.c | 3 --- builtin/fetch.c | 3 --- transport-helper.c | 23 +++--- transport-internal.h | 61 ++ transport.c | 69 transport.h | 54 ++-- 6 files changed, 120 insertions(+), 93 deletions(-) create mode 100644 transport-internal.h -- 2.15.1.504.g5279b80103-goog
[PATCH 2/2] transport: make transport vtable more private
Move the definition of the transport-specific functions provided by transports, whether declared in transport.c or transport-helper.c, into an internal header. This means that transport-using code (as opposed to transport-declaring code) can no longer access these functions (without importing the internal header themselves), making it clear that they should use the transport_*() functions instead, and also allowing the interface between the transport mechanism and an individual transport to independently evolve. This is superficially a reversal of commit 824d5776c3f2 ("Refactor struct transport_ops inlined into struct transport", 2007-09-19). However, the scope of the involved variables was neither affected nor discussed in that commit, and I think that the advantages in making those functions more private outweigh the advantages described in that commit's commit message. A minor additional point is that the code has gotten more complicated since then, in that the function-pointer variables are potentially mutated twice (once initially and once if transport_take_over() is invoked), increasing the value of corralling them into their own struct. Signed-off-by: Jonathan Tan--- The main evolution I see is to move all traces of the "takeover" mechanism to transport.c, instead of having individual transports (the builtin smart transport in transport.c and the remote helper interface in transport-helper.c) do the connect-then-fetch etc. thing themselves. This means that transport_vtable->connect must be allowed to return "fallback", and we have the following vtables: - the one for bundles (unchanged) - one for builtin smart transports in which fetch and push_refs are NULL - one for remote helpers (unchanged) - one for the result of takeover (unchanged) (to be used when get_refs_list/fetch/pull is used with a connect-supporting transport) - one for the result of transport_connect (connect, fetch, and push_refs are NULL) (to be used when user code invokes transport_connect in order to control the stream directly) transport_get_remote_refs() and friends will always attempt to connect (and perform the takeover if it succeeds) before proceeding. In this way, functionality becomes clearer: - Transports that do not support connect leave connect as NULL and implement get_refs_list/fetch/pull. - Transports that support connect implement connect. If connect is do-or-die (e.g. builtin smart transports), leave get_refs_list, fetch, and pull as NULL. Otherwise (e.g. remote helpers) populate them, and they will be used as a fallback if connect fails. - When transport_connect() is invoked, the vtable is switched so that connect/get_refs_list/fetch/pull no longer work. The user code has full control. - When transport_get_remote_refs() (or friends) is invoked, if connect is successful, the vtable is switched so that connect no longer works. get_refs_list/fetch/pull use the established connection, and it is clear that the user can no longer call transport_connect(). This seems feasible to me, but I haven't tried to implement it yet. --- transport-helper.c | 23 +++--- transport-internal.h | 61 ++ transport.c | 69 transport.h | 54 ++-- 4 files changed, 120 insertions(+), 87 deletions(-) create mode 100644 transport-internal.h diff --git a/transport-helper.c b/transport-helper.c index c948d5215..1a4b43ff1 100644 --- a/transport-helper.c +++ b/transport-helper.c @@ -11,6 +11,7 @@ #include "sigchain.h" #include "argv-array.h" #include "refs.h" +#include "transport-internal.h" static int debug; @@ -650,7 +651,7 @@ static int fetch(struct transport *transport, if (process_connect(transport, 0)) { do_take_over(transport); - return transport->fetch(transport, nr_heads, to_fetch); + return transport->vtable->fetch(transport, nr_heads, to_fetch); } count = 0; @@ -987,7 +988,7 @@ static int push_refs(struct transport *transport, if (process_connect(transport, 1)) { do_take_over(transport); - return transport->push_refs(transport, remote_refs, flags); + return transport->vtable->push_refs(transport, remote_refs, flags); } if (!remote_refs) { @@ -1035,7 +1036,7 @@ static struct ref *get_refs_list(struct transport *transport, int for_push) if (process_connect(transport, for_push)) { do_take_over(transport); - return transport->get_refs_list(transport, for_push); + return transport->vtable->get_refs_list(transport, for_push); } if (data->push && for_push) @@ -1084,6 +1085,15 @@ static struct ref *get_refs_list(struct transport *transport, int for_push) return ret; }
[PATCH 1/2] clone, fetch: remove redundant transport check
Prior to commit a2d725b7bdf7 ("Use an external program to implement fetching with curl", 2009-08-05), if Git was compiled with NO_CURL, the get_refs_list and fetch methods in struct transport might not be populated, hence the checks in clone and fetch. After that commit, all transports populate get_refs_list and fetch, making the checks in clone and fetch redundant. Remove those checks. Signed-off-by: Jonathan Tan--- builtin/clone.c | 3 --- builtin/fetch.c | 3 --- 2 files changed, 6 deletions(-) diff --git a/builtin/clone.c b/builtin/clone.c index dbddd98f8..979af0383 100644 --- a/builtin/clone.c +++ b/builtin/clone.c @@ -1083,9 +1083,6 @@ int cmd_clone(int argc, const char **argv, const char *prefix) warning(_("--local is ignored")); transport->cloning = 1; - if (!transport->get_refs_list || (!is_local && !transport->fetch)) - die(_("Don't know how to clone %s"), transport->url); - transport_set_option(transport, TRANS_OPT_KEEP, "yes"); if (option_depth) diff --git a/builtin/fetch.c b/builtin/fetch.c index 225c73492..09eb1fc17 100644 --- a/builtin/fetch.c +++ b/builtin/fetch.c @@ -1094,9 +1094,6 @@ static int do_fetch(struct transport *transport, tags = TAGS_UNSET; } - if (!transport->get_refs_list || !transport->fetch) - die(_("Don't know how to fetch from %s"), transport->url); - /* if not appending, truncate FETCH_HEAD */ if (!append && !dry_run) { retcode = truncate_fetch_head(); -- 2.15.1.504.g5279b80103-goog
Re: [PATCH v7] fixup: rev-list-options text
On Thu, Dec 14, 2017 at 10:19 PM, Jeff Hostetlerwrote: > --- a/Documentation/rev-list-options.txt > +++ b/Documentation/rev-list-options.txt > @@ -714,9 +714,9 @@ ifdef::git-rev-list[] > + > The form '--filter=blob:none' omits all blobs. > + > -The form '--filter=blob:limit=[kmg]' omits blobs larger than n bytes > -or units. n may be zero. The suffixes k, m, and g can be used to name > -units in KiB, MiB, or GiB. For example, 'blob:limit=1k' is the same > +The form '--filter=blob:limit=[kmg]' omits blobs larger than n bytes or While at it maybe: s/than n bytes/than '[kmg]' bytes/ > +units. '' may be zero. The suffixes 'k', 'm', and 'g' can be used to > +name units in KiB, MiB, or GiB. For example, 'blob:limit=1k' is the same > as 'blob:limit=1024'.
Re: [PATCH] doc: Modify git-add doc to say "staging area"
On Thu, Dec 14, 2017 at 1:16 PM, David A. Wheelerwrote: > "David A. Wheeler" writes: >> > Why is "index" better? It is a confusing name, one that has many >> > other unrelated meanings. In particular, many projects managed by >> > git also have an index, but few have a staging area. > > On Thu, 14 Dec 2017 11:40:51 -0800, Junio C Hamano wrote: >> That's an absurd argument. A database product that wants to be used >> in library systems are forbidden to have "index" because that may be >> confused with library index cards? > > No, because most database systems aren't designed to be primarily used > in library systems. Even if they are, I haven't seen a "library index card" > in decades (many people will not know what they are), so > that is much less likely to be confusing. > > In contrast, git is widely used to manage source code (where "index" often > means "array index", "hash index", and so on) and/or HTML > (where "index.html" is pretty common). Using the *same* term for something > git often manages *is* confusing. > > Even if you don't buy that argument, I think most newer users find the term > "staging area" simpler... and we are *all* new to something at one time. > > A Google of git "staging area" returns 67,000 results, and "staging area" > is *much* newer terminology than "index" and has those hits in *spite* of > "index" and "cache" being the historical terms. > > Is there a term you'd prefer over "index" or "cache"? > I would personally prefer to drop 'cache', as the mechanism involved is not a cache from the users point of view. (A cache is not affecting behavior except for performance. In Git this "index" does affect more than just performance, it also allows a very specific workflow.) Personally I am indifferent to whether we call it index or staging area as long as it is consistent. Junio mentioned the 'X acts like Y' is different from 'X is Y'", so maybe we can use both words, as in "Use git-add to add files into the index, which is used as a staging area for the next commit". Note that this discussion seems to be quite old (way older than my contribution record): $ git log --grep "staging area" ... commit 11920d28da1ac1b65eb4041c1b7355924e5d1366 Author: Scott Chacon Date: 2008-12-01 22:14 Add a built-in alias for 'stage' to the 'add' command This comes from conversation at the GitTogether where we thought it would be helpful to be able to teach people to 'stage' files because it tends to cause confusion when told that they have to keep 'add'ing them. This continues the movement to start referring to the index as a staging area (eg: the --staged alias to 'git diff'). Also adds a doc file for 'git stage' that basically points to the docs for 'git add'. Signed-off-by: Scott Chacon Signed-off-by: Junio C Hamano
Re: What's cooking in git.git (Dec 2017, #03; Wed, 13)
Junio C Hamano wrote: > Jonathan Niederwrites: >>> * sb/diff-blobfind (2017-12-12) 1 commit >>> (merged to 'next' on 2017-12-13 at 9a27a20c5f) >>> + diffcore: add a filter to find a specific blob >>> >>> "git diff" family of commands learned --blobfind= that >>> allows you to limit the output only to a change that involves the >>> named blob object (either changing the contents from or to it). >>> >>> Will merge to 'master'. >> >> Sorry, I should have replied about this a long time ago: I love this >> option but I am not sure that --blobfind is the right name for it. > > Sorry. I should have updated the description when the option name > was updated in the latest round. Sure. My worry still applies: https://public-inbox.org/git/20171214212234.gc32...@aiede.mtv.corp.google.com/ But as I said, if nothing comes of it within a few days then I'm happy to conclude that we did our best. Thanks, Jonathan
Re: [PATCH] diffcore: add a filter to find a specific blob
Hi, Stefan Beller wrote: > Junio hinted at a different approach of solving this problem, which this > patch implements. Teach the diff machinery another flag for restricting > the information to what is shown. For example: > > $ ./git log --oneline --find-object=v2.0.0:Makefile > b2feb64309 Revert the whole "ask curl-config" topic for now > 47fbfded53 i18n: only extract comments marked with "TRANSLATORS:" > > we observe that the Makefile as shipped with 2.0 was appeared in > v1.9.2-471-g47fbfded53 and in v2.0.0-rc1-5-gb2feb6430b. Neat! Nit: s/was appeared/appeared/ (not a big deal though, since this is already in 'next'). >From the above it's not clear to me whether this is about commits where the object was added or where the object was removed. Fortunately, the docs are a bit clearer: ... one side of the diff matches the given object id. The object can be a blob, gitlink entry or tree (when `-t` is given). So this appears to be about both additions and removals. Followup questions: - are copies and renames shown (if I am passing -M -C)? - what about mode changes? If the file became executable but the blob content didn't change, does that commit match? Nit, not related to this change: it would be nice to have a long option to go along with the short name '-t' --- e.g. --include-trees. Another nit: s/gitlink entry/submodule commit/, perhaps. The commit object is not a gitlink entry: it is pointed to by a gitlink entry. Another documentation idea: it may be nice to point out that this is only about the preimage and postimage submodule commit and that it doesn't look at the history in between. > The > reason why these commits both occur prior to v2.0.0 are evil > merges that are not found using this new mechanism. Would it be worth the doc mentioning that this doesn't look at merges unless you use one of the -m/-c/--cc options, or does that go without saying? [...] > --- a/Documentation/diff-options.txt > +++ b/Documentation/diff-options.txt > @@ -500,6 +500,12 @@ information. > --pickaxe-regex:: > Treat the given to `-S` as an extended POSIX regular > expression to match. > + > +--find-object=:: > + Restrict the output such that one side of the diff > + matches the given object id. The object can be a blob, > + gitlink entry or tree (when `-t` is given). I like this name --find-object more than --blobfind! I am not sure it quite matches what the user is looking for, though. We are not looking for all occurences of the object; we only care about when the object appears (was added or removed) in the diff. Putting it in context, we have: pickaxe options: -S: detect changes in occurence count of a string -G: grep lines in diff for a string --pickaxe-all: do not filter the diff when the patch matches pickaxe conditions. kind of like log --full-diff, but restricted to pickaxe options. --pickaxe-regex: treat -S argument as a regex, not a string Is this another kind of pickaxe option? It feels similar to -S, but at an object level instead of a substring level, so in a way it would be appealing to call it --pickaxe-object. Does --pickaxe-all affect it like it affects -S and -G? Another context to put it in is: --diff-filter: limit paths (but not commits?) to those with a change matching optarg If I understand correctly, then --diff-filter does not interact with --pickaxe-all, or in other words it is a different filtering condition. Is this another kind of diff filter? In that context, it may be appealing to call it something like --object-filter. --diff-filter is an example where it seems appealing to have a --full-diff option to diff-tree that could apply to all filters and not just pickaxe. [... implementation snipped ...] The implementation looks lovely and I'm especially happy about the tests. Thanks for writing it. Thoughts? Jonathan
[PATCH v7] Partial clone part 1: object filtering
From: Jeff HostetlerHere is V7 of the list-object filtering, rev-list, and pack-objects. This is an incremental patch series to be applied on top of the V6 incremental patch which is already in 'next'. This version fixes some formatting of the help text as suggested by Christian. Jeff Hostetler (1): fixup: rev-list-options text Documentation/rev-list-options.txt | 8 1 file changed, 4 insertions(+), 4 deletions(-) -- 2.9.3
[PATCH v7] fixup: rev-list-options text
From: Jeff HostetlerSigned-off-by: Jeff Hostetler --- Documentation/rev-list-options.txt | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/Documentation/rev-list-options.txt b/Documentation/rev-list-options.txt index 8d8b7f4..b7237b9 100644 --- a/Documentation/rev-list-options.txt +++ b/Documentation/rev-list-options.txt @@ -714,9 +714,9 @@ ifdef::git-rev-list[] + The form '--filter=blob:none' omits all blobs. + -The form '--filter=blob:limit=[kmg]' omits blobs larger than n bytes -or units. n may be zero. The suffixes k, m, and g can be used to name -units in KiB, MiB, or GiB. For example, 'blob:limit=1k' is the same +The form '--filter=blob:limit=[kmg]' omits blobs larger than n bytes or +units. '' may be zero. The suffixes 'k', 'm', and 'g' can be used to +name units in KiB, MiB, or GiB. For example, 'blob:limit=1k' is the same as 'blob:limit=1024'. + The form '--filter=sparse:oid=' uses a sparse-checkout @@ -725,7 +725,7 @@ to omit blobs that would not be not required for a sparse checkout on the requested refs. + The form '--filter=sparse:path=' similarly uses a sparse-checkout -specification contained in . +specification contained in ''. --no-filter:: Turn off any previous `--filter=` argument. -- 2.9.3
[PATCH v3] partial-clone: design doc
From: Jeff HostetlerDesign document for partial clone feature. Signed-off-by: Jeff Hostetler --- Documentation/technical/partial-clone.txt | 324 ++ 1 file changed, 324 insertions(+) create mode 100644 Documentation/technical/partial-clone.txt diff --git a/Documentation/technical/partial-clone.txt b/Documentation/technical/partial-clone.txt new file mode 100644 index 000..f78fa32 --- /dev/null +++ b/Documentation/technical/partial-clone.txt @@ -0,0 +1,324 @@ +Partial Clone Design Notes +== + +The "Partial Clone" feature is a performance optimization for Git that +allows Git to function without having a complete copy of the repository. +The goal of this work is to allow Git better handle extremely large +repositories. + +During clone and fetch operations, Git downloads the complete contents +and history of the repository. This includes all commits, trees, and +blobs for the complete life of the repository. For extremely large +repositories, clones can take hours (or days) and consume 100+GiB of disk +space. + +Often in these repositories there are many blobs and trees that the user +does not need such as: + + 1. files outside of the user's work area in the tree. For example, in + a repository with 500K directories and 3.5M files in every commit, + we can avoid downloading many objects if the user only needs a + narrow "cone" of the source tree. + + 2. large binary assets. For example, in a repository where large build + artifacts are checked into the tree, we can avoid downloading all + previous versions of these non-mergeable binary assets and only + download versions that are actually referenced. + +Partial clone allows us to avoid downloading such unneeded objects *in +advance* during clone and fetch operations and thereby reduce download +times and disk usage. Missing objects can later be "demand fetched" +if/when needed. + +Use of partial clone requires that the user be online and the origin +remote be available for on-demand fetching of missing objects. This may +or may not be problematic for the user. For example, if the user can +stay within the pre-selected subset of the source tree, they may not +encounter any missing objects. Alternatively, the user could try to +pre-fetch various objects if they know that they are going offline. + + +Non-Goals +- + +Partial clone is a mechanism to limit the number of blobs and trees downloaded +*within* a given range of commits -- and is therefore independent of and not +intended to conflict with existing DAG-level mechanisms to limit the set of +requested commits (i.e. shallow clone, single branch, or fetch ''). + + +Design Overview +--- + +Partial clone logically consists of the following parts: + +- A mechanism for the client to describe unneeded or unwanted objects to + the server. + +- A mechanism for the server to omit such unwanted objects from packfiles + sent to the client. + +- A mechanism for the client to gracefully handle missing objects (that + were previously omitted by the server). + +- A mechanism for the client to backfill missing objects as needed. + + +Design Details +-- + +- A new pack-protocol capability "filter" is added to the fetch-pack and + upload-pack negotiation. + + This uses the existing capability discovery mechanism. + See "filter" in Documentation/technical/pack-protocol.txt. + +- Clients pass a "filter-spec" to clone and fetch which is passed to the + server to request filtering during packfile construction. + + There are various filters available to accommodate different situations. + See "--filter=" in Documentation/rev-list-options.txt. + +- On the server pack-objects applies the requested filter-spec as it + creates "filtered" packfiles for the client. + + These filtered packfiles are *incomplete* in the traditional sense because + they may contain objects that reference objects not contained in the + packfile and that the client doesn't already have. For example, the + filtered packfile may contain trees or tags that reference missing blobs + or commits that reference missing trees. + +- On the client these incomplete packfiles are marked as "promisor packfiles" + and treated differently by various commands. + +- On the client a repository extension is added to the local config to + prevent older versions of git from failing mid-operation because of + missing objects that they cannot handle. + See "extensions.partialClone" in Documentation/technical/repository-version.txt" + + +Handling Missing Objects + + +- An object may be missing due to a partial clone or fetch, or missing due + to repository corruption. To differentiate these cases, the local + repository specially indicates such filtered packfiles obtained from the + promisor remote as "promisor packfiles". + + These promisor packfiles
[PATCH v4 3/3] rebase: rebasing can also be done when HEAD is detached
From: Kaartic SivaraamDate: Mon, 27 Nov 2017 22:51:04 +0530 Attempting to rebase when the HEAD is detached and is already up to date with upstream (so there's nothing to do), the following message is shown Current branch HEAD is up to date. which is clearly wrong as HEAD is not a branch. Handle the special case of HEAD correctly to give a more precise error message. Signed-off-by: Kaartic Sivaraam Signed-off-by: Junio C Hamano --- git-rebase.sh | 16 ++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/git-rebase.sh b/git-rebase.sh index e5adb596a0..f3dd864437 100755 --- a/git-rebase.sh +++ b/git-rebase.sh @@ -601,11 +601,23 @@ then test -z "$switch_to" || GIT_REFLOG_ACTION="$GIT_REFLOG_ACTION: checkout $switch_to" \ git checkout -q "$switch_to" -- - say "$(eval_gettext "Current branch \$branch_name is up to date.")" + if test "$branch_name" = "HEAD" && +!(git symbolic-ref -q HEAD) + then + say "$(eval_gettext "HEAD is up to date.")" + else + say "$(eval_gettext "Current branch \$branch_name is up to date.")" + fi finish_rebase exit 0 else - say "$(eval_gettext "Current branch \$branch_name is up to date, rebase forced.")" + if test "$branch_name" = "HEAD" && +!(git symbolic-ref -q HEAD) + then + say "$(eval_gettext "HEAD is up to date, rebase forced.")" + else + say "$(eval_gettext "Current branch \$branch_name is up to date, rebase forced.")" + fi fi fi -- 2.15.1-554-g7ec1e7e2b9
[PATCH v3] Partial clone design document
From: Jeff HostetlerThis patch contains V3 os the partial clone design document. It incorporates feedback from Philip that I missed before sending V2 and today's comments from Junio. Jeff Hostetler (1): partial-clone: design doc Documentation/technical/partial-clone.txt | 324 ++ 1 file changed, 324 insertions(+) create mode 100644 Documentation/technical/partial-clone.txt -- 2.9.3
[PATCH v4 2/3] rebase: distinguish user input by quoting it
From: Kaartic SivaraamDate: Mon, 27 Nov 2017 22:51:03 +0530 Signed-off-by: Kaartic Sivaraam Signed-off-by: Junio C Hamano --- git-rebase.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/git-rebase.sh b/git-rebase.sh index 5526b17a36..e5adb596a0 100755 --- a/git-rebase.sh +++ b/git-rebase.sh @@ -477,7 +477,7 @@ then ;; esac upstream=$(peel_committish "${upstream_name}") || - die "$(eval_gettext "invalid upstream \$upstream_name")" + die "$(eval_gettext "invalid upstream '\$upstream_name'")" upstream_arg="$upstream_name" else if test -z "$onto" @@ -539,7 +539,7 @@ case "$#" in head_name="detached HEAD" else - die "$(eval_gettext "fatal: no such branch/commit: \$branch_name")" + die "$(eval_gettext "fatal: no such branch/commit '\$branch_name'")" fi ;; 0) -- 2.15.1-554-g7ec1e7e2b9
Re: What's cooking in git.git (Dec 2017, #03; Wed, 13)
Junio C Hamanowrites: > I think you only sent 3/3 in newer rounds, which made it not to > apply to my tree. If you meant to drop changes in 1/3 and 2/3, > perhaps because they were needless churn, then 3/3 needs to be > updated not to depend on the changes these two made. Here is what I reconstructed to suit my taste better ;-) -- >8 -- From: Kaartic Sivaraam Date: Mon, 27 Nov 2017 22:51:02 +0530 Subject: [PATCH v4 1/3] rebase: consistently use branch_name variable The variable "branch_name" holds the parameter in "git rebase ", but one codepath did not use it after assigning $1 to it (instead it kept using $1). Make it use the variable consistently. Also, update an error message to say there is no such branch or commit, as we are expecting either of them, and not limiting ourselves to a branch name. Signed-off-by: Kaartic Sivaraam --- git-rebase.sh | 15 +-- 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/git-rebase.sh b/git-rebase.sh index 60b70f3def..5526b17a36 100755 --- a/git-rebase.sh +++ b/git-rebase.sh @@ -528,15 +528,18 @@ case "$#" in branch_name="$1" switch_to="$1" - if git show-ref --verify --quiet -- "refs/heads/$1" && - orig_head=$(git rev-parse -q --verify "refs/heads/$1") + # Is it a local branch? + if git show-ref --verify --quiet -- "refs/heads/$branch_name" && + orig_head=$(git rev-parse -q --verify "refs/heads/$branch_name") then - head_name="refs/heads/$1" - elif orig_head=$(git rev-parse -q --verify "$1") + head_name="refs/heads/$branch_name" + # If not is it a valid ref (branch or commit)? + elif orig_head=$(git rev-parse -q --verify "$branch_name") then head_name="detached HEAD" + else - die "$(eval_gettext "fatal: no such branch: \$branch_name")" + die "$(eval_gettext "fatal: no such branch/commit: \$branch_name")" fi ;; 0) @@ -547,7 +550,7 @@ case "$#" in branch_name=$(expr "z$branch_name" : 'zrefs/heads/\(.*\)') else head_name="detached HEAD" - branch_name=HEAD ;# detached + branch_name=HEAD fi orig_head=$(git rev-parse --verify HEAD) || exit ;; -- 2.15.1-554-g7ec1e7e2b9
RE: Need help migrating workflow from svn to git.
> On December 14, 2017 8:10 AM, Josef Wolf wrote: > Subject: Need help migrating workflow from svn to git. > > Hello folks, > > I am wondering whether/how my mode of work for a specific project > (currently based on SVN) could be transferred to git. > > I have a repository for maintaining configuration of hosts. This repository > contains several hundered scripts. Most of those scripts are don't depend on > each other. > > Every machine has a working copy of the repository in a specific directory. A > cron job (running every 15 minutes) executes "svn update" and executes the > scripts which are contained in this working copy. > > This way, I can commit changes to the main repository and all the hosts will > "download" and adopt by executing the newest revision of those scripts. > (The sripts need to be idempotent, but this is a different topic). > > NORMALLY, there are no local modifications in the working copy. Thus, > conflicts can not happen. Everything works fine. > > Sometimes, I need to fix a problem on some host or need to implement a > new feature. For this, I go to the working copy of a host where the change > needs to be done and start haking. With svn, I don't need to stop the cron > job. "svn update" will happily merge any in-coming changes and leave alone > the files which were not modified upstream. Conflicts with my local > modifications which I am currently hacking on are extremely rare, because > the scripts are pretty much independent. So I'm pretty much happy with this > mode of operation. > > With git, by contrast, this won't work. Git will refuse to pull anything as long > as there are ANY local modifications. The cron job would need to > >git stash >git pull >git stash pop > > But this will temporarily remove my local modifications. If I happen to do a > test run at this time, the test run would NOT contain the local modifications > which I was about to test. Even worse: if I happen to save one of the > modified files while the modifications are in the stash, the "git stash pop" will > definitely cause a conflict, although nothing really changed. > > So, how would I get this workflow with git? Is it possible to emulate the > behavior of "svn update"? > > Any ideas? You might want to consider a slight modification to your approach as follows. Instead of using git pull, use git fetch. Have each system on its own branch (sys1 = my-sys1-branch, for example) so you can track who has what. In your scripts, consider: git fetch if nothing changed, done git status if no changes, git merge --ff master && git push origin my-sys1-branch && done if changes, send an email whining about the changes your script could then (depending on your environment) git commit -a && git merge && git push origin my-sys1-branch && done This would allow you to track the condition of each system at your single upstream repository. Just my $0.02 Cheers. Randall\ -- Brief whoami: NonStop developer since approximately UNIX(421664400)/NonStop(2112884442) -- In my real life, I talk too much.
Re: What's cooking in git.git (Dec 2017, #03; Wed, 13)
Jonathan Niederwrites: >> * sb/diff-blobfind (2017-12-12) 1 commit >> (merged to 'next' on 2017-12-13 at 9a27a20c5f) >> + diffcore: add a filter to find a specific blob >> >> "git diff" family of commands learned --blobfind= that >> allows you to limit the output only to a change that involves the >> named blob object (either changing the contents from or to it). >> >> Will merge to 'master'. > > Sorry, I should have replied about this a long time ago: I love this > option but I am not sure that --blobfind is the right name for it. Sorry. I should have updated the description when the option name was updated in the latest round. Jonathan Nieder writes:
Re: What's cooking in git.git (Dec 2017, #03; Wed, 13)
Hi, Junio C Hamano wrote: > * sb/diff-blobfind (2017-12-12) 1 commit > (merged to 'next' on 2017-12-13 at 9a27a20c5f) > + diffcore: add a filter to find a specific blob > > "git diff" family of commands learned --blobfind= that > allows you to limit the output only to a change that involves the > named blob object (either changing the contents from or to it). > > Will merge to 'master'. Sorry, I should have replied about this a long time ago: I love this option but I am not sure that --blobfind is the right name for it. If we can think of a better name quickly then it would be nice to change it before people start relying on it, so I'd rather hold off on merging this to 'master' for the moment. That said, if we don't have any better ideas for the option name within a few days then my objection goes away. I'll reply in the patch thread. Thanks, Jonathan
Re: [PATCH v2] partial-clone: design doc
On 12/14/2017 1:24 PM, Junio C Hamano wrote: Jeff Hostetlerwrites: +- On the client these incomplete packfiles are marked as "promisor pacfiles" s/pacfiles/packfiles/ + These "promisor packfiles" consist of a ".promisor" file with + arbitrary contents (like the ".keep" files), in addition to + their ".pack" and ".idx" files. + + In the future, this ability may be extended to loose objects in case + a promisor packfile is accidentally unpacked. Hmph. Because we cannot assume that such an "accidental" unpacking would do anything extra to help us tell the loose objects created out of a promisor pack from other loose objects, you would end up making any and all loose objects to serve as if they came from a promisor remote? I am not sure if that makes much sense. Do we really need to write this "in the future" down, before we have thought things through enough to specify the design at a bit more detailed level? good point. i'll move this to the bottom and elaborate on the problem rather than the solution. Jeff
Re: [PATCH v2] doc: add triangular workflow
Matthieu Moywrites: > I don't think you should talk about "Git contributors", which can be > read as "contributors to the git.git codebase". "Git users" would make > more sense, or perhaps you meant "contributors to Git projects". But > actually, I don't think this kind of documentation should focus only > on new users. I think many reasonably advanced Git users do not know > about remote.pushdefault for example. Thanks for a careful review. >> diff --git a/Documentation/Makefile b/Documentation/Makefile >> index 2ab6556..c3e98c7 100644 >> --- a/Documentation/Makefile >> +++ b/Documentation/Makefile >> @@ -10,6 +10,7 @@ OBSOLETE_HTML = >> MAN1_TXT += $(filter-out \ >> $(addsuffix .txt, $(ARTICLES) $(SP_ARTICLES)), \ >> $(wildcard git-*.txt)) >> +MAN1_TXT += git-triangular-workflow.txt > > git-*.txt is reserved for actual Git commands. Other tutorials use > git*.txt (e.g. we have gitworkflows.txt and not git-workflows.txt). Yeah, it probably is worth mentioning that MAN1 is for commands. Unless we have "git triangular-workflow" subcommand, this shouldn't be listed on MAN1_TXT list. Perhaps in MAN7 next to tutorial and workflow would be a better place. >> +This workflow is commonly used on different platforms like BitBucket, >> +GitHub or GitLab which provide a dedicated mechanism for requesting merges. > > As a user, I find it terribly frustrating when reading documentation > telling me that "there's a dedicated mechanism" for something without > telling me actually how to do it. Also I think the description is backwards from end-user's point of view. It's not that it is common to use the workflow on these hosting sites. It's more like people use the workflow and adopt use of these hosting sites as one building block of the workflow. >> +In a triangular workflow the rest of the community or the company >> +can review the code before it's in production. Everyone can read on >> +**PUBLISH** so everyone can review code before the maintainer merge >> +it to **UPSTREAM**. In a free software, anyone can >> +propose code. Reviewers accept the code when everyone agree >> +with it. The above hints that the workflow covers wide range of development circles from open source to proprietary, but the description in this paragraph does not seem to show how the workflow achieves that goal very well. Not all environment allow "everyone" to read "publish" (it is common to see siloed source repositories in commercial settings), and not all projects require unanimous agreement for inclusion. Also, "anyone can propose code" may be true for any project, not limited to "free" ones, as long as the code to base your changes on is available, but if the project would not take external contributions, being able to "propose" alone does not mean that much to the proposer. >> +If **PUBLISH** doesn't exist, a contributor can publish his own repository. >> +**PUBLISH** contains modifications before integration. I am not sure what this really means. Isn't it the norm to create a place to publish your work (and only your work) for your own use? IOW, the above two lines solicit questions like "So... what happens when publish does already exist??? and where does that publish repository come from???"
RE: [PATCH] git-svn: convert CRLF to LF in commit message to SVN
Thank you for your fast response, I haven't done a build of this type before (so I could test the patch first) so I'm trying to do that and get this far: 1. git clone https://github.com/msysgit/msysgit.git "c:\temp\git\guitbuild" 2. git clone https://github.com/git-for-windows/git.git "c:\temp\git\guitbuild\git" 3. c:\temp\git\guitbuild\msys.bat --- Building and Installing Git --- CC archive-tar.o In file included from run-command.h:5, from archive-tar.c:9: compat/win32/pthread.h:38: error: expected ')' before 'ConditionVariable' compat/win32/pthread.h:40: error: expected ')' before 'ConditionVariable' compat/win32/pthread.h:42: error: expected ')' before 'ConditionVariable' compat/win32/pthread.h:44: error: expected ')' before 'ConditionVariable' make: *** [archive-tar.o] Error 1 The applicable lines from compat/win32/pthread.h: 37: WINBASEAPI VOID WINAPI 38: InitializeConditionVariable(PCONDITION_VARIABLE ConditionVariable); 39: WINBASEAPI VOID WINAPI 40: WakeConditionVariable(PCONDITION_VARIABLE ConditionVariable); 41: WINBASEAPI VOID WINAPI 42: WakeAllConditionVariable(PCONDITION_VARIABLE ConditionVariable); 43: WINBASEAPI BOOL WINAPI 44: SleepConditionVariableCS(PCONDITION_VARIABLE ConditionVariable, 45: PCRITICAL_SECTION CriticalSection, 46: DWORD dwMilliseconds); I don't want to drag out testing the patch, so if either of you are able to quickly guide me on what I am doing incorrectly I am willing to get the build done so I can test it. If not, could one of you build with the patch and somehow get that to me so I could test? Brian Bennett | Supv System Admin & Support, TA TECH Change Mgmt/Production Support o: 319-355-7602 | c: 319-533-1094 e: brian.benn...@transamerica.com | w: www.transamerica.com Transamerica 6400 C St. SW, Cedar Rapids, IA 52404 MS-2410 Facebook | LinkedIn -Original Message- From: Eric Wong [mailto:e...@80x24.org] Sent: Wednesday, December 13, 2017 6:21 PM To: Bennett, Brian; Junio C Hamano Cc: git@vger.kernel.org Subject: [PATCH] git-svn: convert CRLF to LF in commit message to SVN "Bennett, Brian" wrote: > Environment: > > Desktop: Windows 7 Enterprise 64-bit > svn client (if applicable): 1.8.8 from Apache git > (https://urldefense.proofpoint.com/v2/url?u=https-3A__git-2Dfor-2Dwind > ows.github.io_=DwIBaQ=9g4MJkl2VjLjS6R4ei18BA=CorEYR_fG6hKwP1xRO7 > dkFFJM6UfxLGgypqJT0q3mO4=f1K2uzEyLbtIX-0te07VlclknjdUztTvbgDMA0thROs > =3AqxH_SEQG48PhnwuCD8udYta0mqXfgKKlmAWMfSlfE=): git version > 2.10.1.windows.1 GitTfs > (https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_git-2 > Dtfs_git-2Dtfs=DwIBaQ=9g4MJkl2VjLjS6R4ei18BA=CorEYR_fG6hKwP1xRO7 > dkFFJM6UfxLGgypqJT0q3mO4=f1K2uzEyLbtIX-0te07VlclknjdUztTvbgDMA0thROs > =C2gZ6zgihigH5eMpa5UVgj1mglbQbGN1HG0blMqjmsY=): git-tfs version > 0.27.0.0 (TFS client library 14.0.0.0 (MS)) (32-bit) Team Foundation > Server: 2010 Visual Studio installation: 2010 and 2015 Thanks for the report and research! > I've researched enough to believe that the commit message being used > by git svn contains a carriage return character > (x'0D') and that has not been allowed in Subversion since version 1.6 > (I can replicate this specific error message using an SVN dump file > that contains x'0D' characters in the log messages.). However, I > cannot find where I have any control over the log message that git svn > is trying to use nor can I observe it. Note that I've also used the > '-v' switch with the 'git svn dcommit', but do not receive anything > other than what I am showing above. Maybe git-for-windows isn't filtering CRLF into LF as "git commit" does on GNU/Linux when the original commit was made? I had to use "git commit-tree" to reproduce the error in testing (instead of "git commit)" Anyways, the one-line fix below should be enough for you. Care to give it a shot? Thanks again. Junio: please pull when Brian confirms, thanks. The following changes since commit 95ec6b1b3393eb6e26da40c565520a8db9796e9f: RelNotes: the eighth batch (2017-12-06 09:29:50 -0800) are available in the git repository at: git://bogomips.org/git-svn.git svn-crlf for you to fetch changes up to 95450bbbaaacaf2d603a4fbded25d55243dfb291: git-svn: convert CRLF to LF in commit message to SVN (2017-12-14 00:09:38 +) Eric Wong (1): git-svn: convert CRLF to LF in commit message to SVN git-svn.perl| 1 + t/t9169-git-svn-dcommit-crlf.sh | 27 +++ 2 files changed, 28 insertions(+) create mode 100755 t/t9169-git-svn-dcommit-crlf.sh --8< Subject: [PATCH] git-svn: convert CRLF to LF in commit message to SVN Subversion since 1.6 does not accept
Re: [PATCH] partial-clone: design doc
On 12/13/2017 8:17 AM, Philip Oakley wrote: From: "Junio C Hamano""Philip Oakley" writes: + These filtered packfiles are incomplete in the traditional sense because + they may contain trees that reference blobs that the client does not have. Is a comment needed here noting that currently, IIUC, the complete trees are fetched in the packfiles, it's just the un-necessary blobs that are omitted ? I probably am misreading what you meant to say, but the above statement with "currently" taken literally to mean the system without JeffH's changes, is false. I was meaning the current JeffH's V6 series, rather than the last Git release. In one of the previous discussions Jeff had noted that (at that time) his partial design would provide a full set of trees for the selected commits (excluding the trees already available locally), but only a few of the file blobs (based on the filter spec). So yes, I should have been clearer to avoid talking at cross purposes. Right, we build upon the existing thin-pack capabilities such that a fetch following a clone gets a packfile that assumes the client already has all of the objects in the "edge". So a fetch would not need to receive trees and blobs that are already present in the edge commits. What we are adding here is a way to filter/restrict even further the set of objects sent to the client. When the receiver says it has commit A and the sender wants to send a commit B (because the receiver said it does not have it, and it wants it), trees in A are not sent in the pack the sender sends to give objects sufficient to complete B, which the receiver wanted to have, even if B also has those trees. If you fetch from me twice and between that time Documentation/ directory did not change, the second fetch will not have the tree object that corresponds to that hierarchy (and of course no blobs and sub trees inside it). Though, after the fetch has completed (v2.15 Git), the receiver will have the 'full set of trees and blobs'. In Jeff's design (V6) the reciever would still have a full set of trees, but only a partial set of the blobs. So my viewpoint was not of the pack file but of the receiver's object store after the fetch. Currently (with our changes) the receiver will have all of the trees and only some of the blobs. If we later add another filter that can filter trees, the client will also have missing but referenced trees too. So "the complete trees are fetched" is not true. What is true (and what matters more in JeffH's document) is that fetching is done in such a way that objects resulting in the receiving repository are complete in the current system that does not allow promised objects. If some objects resulting in the receiving repository are incomplete, the current system considers that we corrupted the repository. The promise mechanism says that it is fine for the receiving end to lack blobs, trees or commits, as long as the promisor repository tells it that these "missing" objects can be obtained from it later. True. (though I'm not sure exactly how Jeff decides about commits - I thought theye were not part of this optimisation) I've not talked about commit filtering -- mainly because we already have such machinery in shallow-clone -- and I did not want to mess with the haves/wants computations. But it will work with missing commits, because of the way object lookup happens a missing commit will trigger the fetch-object code just like it does for missing blobs. The ODB layer doesn't really care what type of object it is -- just that it is missing and needs to be dynamically fetched. Thanks Jeff
Re: [PATCH 0/2] t/lib-git-svn.sh: improve svnserve tests with parallel make test
Junio C Hamano wrote: > Todd Zullingerwrites: >> I wanted to check if this minor patch series had slipped >> under your radar. > > Totally. Queued. > > As they come with Ack by the area maintainer, I'll fast-track them > down to 'master' (other topics typically cook at least for a week in > 'next'). > > Thanks for pinging. Thank you, as always. -- Todd ~~ There is considerable overlap between the intelligence of the smartest bears and the dumbest tourists. -- Park ranger yro.slashdot.org/comments.pl?sid=191810=15757347
Re: [PATCH] partial-clone: design doc
Sorry, I didn't see this message in my inbox when I posted V2 of the design doc. I'll address questions here and update the doc as necessary. On 12/12/2017 6:31 PM, Philip Oakley wrote: From: "Jeff Hostetler"From: Jeff Hostetler First draft of design document for partial clone feature. Signed-off-by: Jeff Hostetler Signed-off-by: Jonathan Tan --- Documentation/technical/partial-clone.txt | 240 ++ 1 file changed, 240 insertions(+) create mode 100644 Documentation/technical/partial-clone.txt diff --git a/Documentation/technical/partial-clone.txt b/Documentation/technical/partial-clone.txt new file mode 100644 index 000..7ab39d8 --- /dev/null +++ b/Documentation/technical/partial-clone.txt @@ -0,0 +1,240 @@ +Partial Clone Design Notes +== + +The "Partial Clone" feature is a performance optimization for git that +allows git to function without having a complete copy of the repository. + I think it would be worthwhile at least listing the issues that make the 'optimisation' necessary, and then the available factors that make the optimisation possible. This helps for future adjustments when those issues and factors change. I think the issues are: * the size of the repository that is being cloned, both in the width of a commit (you mentioned 100M trees) and the time (hours to days) / size to clone over the connection. While the supporting factor is: * the remote is always on-line and available for on-demand object fetching (seconds) The solution choice then should fall out fairly obviously, and we can separate out the other optimisations that are based on other views about the issues. E.g. my desire for a solution in the off-line case. In fact the current design, apart from some terminology, does look well matched, with only a couple of places that would be affected. The airplane-mode expectations of a partial clone should also be stated. Good points. I'll try to work these into V3. +During clone and fetch operations, git normally downloads the complete +contents and history of the repository. That is, during clone the client +receives all of the commits, trees, and blobs in the repository into a +local ODB. Subsequent fetches extend the local ODB with any new objects. +For large repositories, this can take significant time to download and +large amounts of diskspace to store. + +The goal of this work is to allow git better handle extremely large +repositories. Shouln't this goal be nearer the top? maybe. i'll see about reordering the paragraphs in the introduction. Often in these repositories there are many files that the +user does not need such as ancient versions of source files, files in +portions of the worktree outside of the user's work area, or large binary +assets. If we can avoid downloading such unneeded objects *in advance* +during clone and fetch operations, we can decrease download times and +reduce ODB disk usage. + Does this need to distinguish between the shallow clone mechanism for reducing the cloning of old history from the desire for a width wise partial clone of only the users narrow work area, and/or without large files/blobs? I tried to state in the next section that partial clone is independent of shallow clone. That is, our stuff works on filtering *within* the set of commits received. The existing shallow clone and have/wants commit limiting features still apply. I didn't go into detail on the specific filters, because they are documented elsewhere and I view them as an expandable set. The primary goal here is to describe how we handle missing objects without regard to why an object is missing. + +Non-Goals +- + +Partial clone is independent of and not intended to conflict with +shallow-clone, refspec, or limited-ref mechanisms since these all operate +at the DAG level whereas partial clone and fetch works *within* the set +of commits already chosen for download. + [...] +Design Details +-- [...] + These filtered packfiles are incomplete in the traditional sense because + they may contain trees that reference blobs that the client does not have. Is a comment needed here noting that currently, IIUC, the complete trees are fetched in the packfiles, it's just the un-necessary blobs that are omitted ? Currently, we have filters to omit unwanted blobs. Later, we hope to add other filters to omit trees too. My point was that the packfiles are incomplete (have missing objects). I'll reword the above statement a little. + + + How the local repository gracefully handles missing objects + +With partial clone, the fact that objects can be missing makes such +repositories incompatible with older versions of Git, necessitating a +repository extension (see the documentation of "extensions.partialClone" +for more information). + +An
Re: [PATCH] doc: Modify git-add doc to say "staging area"
On Thu, Dec 14 2017, Junio C. Hamano jotted: > Stefan Bellerwrites: > >> Anyway I think spending list band width on good documentation is >> not bandwidth wasted. > > I agree with that. I do not consider the proposed change "good". The case you're talking about upthread is something which we could describe in the docs as "the starting point of the staging area is that it's equivalent to the current commit, and is thus used as an index/cache by various commands", if that ever comes up. I think in the vast majority of other cases talking about it as the staging area would be an improvement, since that's the function that has the closest correspondence to what the UI is actually doing, that we're using it as a cache / index is usually (always?) an implementation detail. Even the merge case you mentioned is something where staging area makes more sense: "We tried to merge, but had a conflict, we've staged some of your changes leaving the rest for you to sort out".
Re: [PATCH] doc: Modify git-add doc to say "staging area"
"David A. Wheeler"writes: > On December 14, 2017 1:50:00 PM EST, Junio C Hamano wrote: >>I agree with that. I do not consider the proposed change "good". > > Why is "index" better? It is a confusing name, one that has many > other unrelated meanings. In particular, many projects managed by > git also have an index, but few have a staging area. That's an absurd argument. A database product that wants to be used in library systems are forbidden to have "index" because that may be confused with library index cards? > Also, the phrase "staging area" is already in use, so this is not > a new term (e.g., git-staging). That gets us back to the "'X acts like Y' is different from 'X is Y'". Besides, the phrase "staging area" is a near-sighted and narrow minded term. It focuses too much on working towards the next commit, and ignores there are other aspects that are equally important. When you check out historical revisions (without any intention of making new commits, just sightseeing), for example, the index does not act as "staging area" for creating a new commit. But it still serves Git users by keeping track of the list of paths that came from the HEAD, and recording their contents and the cached stat info for the working tree files (all using the pathnames as keys into these data items).
Re: [PATCH] doc: Modify git-add doc to say "staging area"
On December 14, 2017 1:50:00 PM EST, Junio C Hamanowrote: >I agree with that. I do not consider the proposed change "good". Why is "index" better? It is a confusing name, one that has many other unrelated meanings. In particular, many projects managed by git also have an index, but few have a staging area. Also, the phrase "staging area" is already in use, so this is not a new term (e.g., git-staging). --- David A.Wheeler
Re: [PATCH] doc: Modify git-add doc to say "staging area"
Stefan Bellerwrites: > Anyway I think spending list band width on good documentation is > not bandwidth wasted. I agree with that. I do not consider the proposed change "good".
Re: [PATCH] RelNotes: minor typo fixes in 2.16.0 draft
Thanks.
Re: [PATCH 0/2] t/lib-git-svn.sh: improve svnserve tests with parallel make test
Todd Zullingerwrites: > I wanted to check if this minor patch series had slipped > under your radar. Totally. Queued. As they come with Ack by the area maintainer, I'll fast-track them down to 'master' (other topics typically cook at least for a week in 'next'). Thanks for pinging.
Re: [PATCH v2] partial-clone: design doc
Jeff Hostetlerwrites: > + There are various filters available to accomodate different situations. s/accomodate/accommodate/ I'll squash in this and /pacfile/packfile/ typofix while queuing. Thanks.
Re: [PATCH v2] partial-clone: design doc
Jeff Hostetlerwrites: > +- On the client these incomplete packfiles are marked as "promisor pacfiles" s/pacfiles/packfiles/ > + These "promisor packfiles" consist of a ".promisor" file with > + arbitrary contents (like the ".keep" files), in addition to > + their ".pack" and ".idx" files. > + > + In the future, this ability may be extended to loose objects in case > + a promisor packfile is accidentally unpacked. Hmph. Because we cannot assume that such an "accidental" unpacking would do anything extra to help us tell the loose objects created out of a promisor pack from other loose objects, you would end up making any and all loose objects to serve as if they came from a promisor remote? I am not sure if that makes much sense. Do we really need to write this "in the future" down, before we have thought things through enough to specify the design at a bit more detailed level?
Re: [PATCH] doc: Modify git-add doc to say "staging area"
On Thu, Dec 14, 2017 at 10:08 AM, Junio C Hamanowrote: > Ævar Arnfjörð Bjarmason writes: > >> On Wed, Dec 13, 2017 at 6:46 AM, David A. Wheeler >> wrote: >>> On December 13, 2017 12:40:12 AM EST, Jacob Keller >>> wrote: I know we've used various terms for this concept across a lot of the documentation. However, I was under the impression that we most explicitly used "index" rather than "staging area". >>> >>> I think "staging area" is the better term. It focuses on its purpose, and >>> it is also less confusing ("index" and "cache" have other meanings in many >>> of the repos managed by git). >> >> After your patch the majority of the docs will still talk about >> "index", is this part of some larger series, perhaps it would be good >> to see it all at once... > > ... or none of it. I do not quite see a point of spending list > bandwidth on a change like this one. I think wording (as well as its consistency) in the documentation is rather important. Just the other day I was reading[1], yet another blog explaining why git sucks. TL;DR: (1) (a) The staging area is an advanced concept and should be disabled by default (b) and is documented super confusingly. (2) Branches and Remotes Management is Complex and Time-Consuming (3) its ecosystem (GitHub et al.) is not pushing for innovation, because "forks are not the right model". [1] https://gregoryszorc.com/blog/2017/12/11/high-level-problems-with-git-and-how-to-fix-them/ When I saw the original patch, I assumed it was a reaction to this blog and attempting to fix (1b), but maybe it is unrelated. Anyway I think spending list band width on good documentation is not bandwidth wasted. Stefan
[PATCH 0/2] t/lib-git-svn.sh: improve svnserve tests with parallel make test
Hi Junio, I wanted to check if this minor patch series had slipped under your radar. If it's waiting patiently in your queue for other topics to advance, that's fine, of course. :) Finished patches: <20171201155653.29553-1-...@pobox.com> and <20171201155653.29553-2-...@pobox.com>. Thanks, -- Todd ~~ Anyone who is capable of getting themselves made President should on no account be allowed to do the job. -- Douglas Adams, "The Hitchhiker's Guide to the Galaxy"
Re: [PATCH v4] git-gui: Prevent double UTF-8 conversion
On Thu, Dec 14, 2017 at 5:43 AM, Łukasz Stelmachwrote: > It was <2017-12-14 czw 10:42>, when Eric Sunshine wrote: >> No need to re-send. If you consult Junio's latest "What's cooking"[1], >> you'll find that your patch has already graduated from his 'pu' branch >> to 'next' and is slated to graduate to 'master' (at some point). >> >> [1]: >> https://public-inbox.org/git/xmqqzi6mutcc@gitster.mtv.corp.google.com/ > > I am sorry. I didn't get any notifiaction by mail and I haven't studied > Documentation/SubmittingPatches throughly enough. No need for an apology. My response was just a simple informational message to help you (and perhaps other newcomers reading this thread) get up to speed.
Re: [PATCH] doc: Modify git-add doc to say "staging area"
Ævar Arnfjörð Bjarmasonwrites: > On Wed, Dec 13, 2017 at 6:46 AM, David A. Wheeler > wrote: >> On December 13, 2017 12:40:12 AM EST, Jacob Keller >> wrote: >>>I know we've used various terms for this concept across a lot of the >>>documentation. However, I was under the impression that we most >>>explicitly used "index" rather than "staging area". >> >> I think "staging area" is the better term. It focuses on its purpose, and it >> is also less confusing ("index" and "cache" have other meanings in many of >> the repos managed by git). > > After your patch the majority of the docs will still talk about > "index", is this part of some larger series, perhaps it would be good > to see it all at once... ... or none of it. I do not quite see a point of spending list bandwidth on a change like this one.
Re: [PATCH v4 2/2] Doc/check-ref-format: clarify information about @{-N} syntax
Kaartic Sivaraamwrites: > With the `--branch` option, the command takes a name and checks if > -it can be used as a valid branch name (e.g. when creating a new > -branch). The rule `git check-ref-format --branch $name` implements > -may be stricter than what `git check-ref-format refs/heads/$name` > -says (e.g. a dash may appear at the beginning of a ref component, > -but it is explicitly forbidden at the beginning of a branch name). > -When run with `--branch` option in a repository, the input is first > -expanded for the ``previous branch syntax'' > -`@{-n}`. For example, `@{-1}` is a way to refer the last branch you > -were on. This option should be used by porcelains to accept this > -syntax anywhere a branch name is expected, so they can act as if you > -typed the branch name. > +it can be used as a valid branch name e.g. when creating a new branch > +(but be cautious when using the previous checkout syntax; it may refer > +to a detached HEAD state). The rule `git check-ref-format --branch > +$name` implements may be stricter than what `git check-ref-format > +refs/heads/$name` says (e.g. a dash may appear at the beginning of a > +ref component, but it is explicitly forbidden at the beginning of a > +branch name). When run with `--branch` option in a repository, the > +input is first expanded for the ``previous checkout syntax'' > +`@{-n}`. For example, `@{-1}` is a way to refer the last thing that > +was checked out using "git checkout" operation. This option should be > +used by porcelains to accept this syntax anywhere a branch name is > +expected, so they can act as if you typed the branch name. As an > +exception note that, the ``previous checkout operation'' might result > +in a commit object name when the N-th last thing checked out was not > +a branch. Looks alright. It was made unnecessarily harder to review because it was marked as 2/2, even though this no longer applies on top of the copy of 1/2 that was merged some time ago. I needed to find that it was rebased on top of 'master'; it wouldn't have been necessary if this was sent as a single patch (with comment saying that this used to be 2/2 whose first one already graduated to 'master' under the three-dash line). Also re-wrapping the lines only to squeeze in "but be cautious..." and replace s/branch/checkout/ in a few places did not help to make it easy to spot what's changed. This part looked a bit strange. > +it can be used as a valid branch name e.g. when creating a new branch > +(but be cautious when using the previous checkout syntax; it may refer > +to a detached HEAD state). The rule `git check-ref-format --branch I think "e.g. when creating a new branch" is a parenthetical remark, so it should be inside parenthesis. As the last three lines in the new text (quoted above) already warns that it may not be a branch name, I am not sure if the "but be cautious" adds much value, though.
Re: [PATCH] doc: Modify git-add doc to say "staging area"
On Thu, Dec 14 2017, David A. Wheeler jotted: > On December 13, 2017 7:54:04 AM EST, "Ævar Arnfjörð Bjarmason" >wrote: >>After your patch the majority of the docs will still talk about >>"index", is this part of some larger series, perhaps it would be good >>to see it all at once... > > Yes, this would be part of a larger series. > > I'm happy to do the work, but I don't want to do it if it's just going to be > rejected. > > The work is very straightforward, in almost all cases you simply replace the > word index with the phrase staging area. The change is similar for the word > cache. So I'm not sure what seeing it all at once would do for anybody. > > Are there one or two other files that you would like to see transformed to > see as an example? If you're just looking for a sense of it, that should be > enough. No I get the idea, I'm just wondering if you'll continue to work on this, because if not mentioning "staging area" in more places without continuing to eradicate "index" isn't going to improve things much, and possibly make it worse. I like the direction of this series.
Re: [PATCH] doc: Modify git-add doc to say "staging area"
On December 13, 2017 7:54:04 AM EST, "Ævar Arnfjörð Bjarmason"wrote: >After your patch the majority of the docs will still talk about >"index", is this part of some larger series, perhaps it would be good >to see it all at once... Yes, this would be part of a larger series. I'm happy to do the work, but I don't want to do it if it's just going to be rejected. The work is very straightforward, in almost all cases you simply replace the word index with the phrase staging area. The change is similar for the word cache. So I'm not sure what seeing it all at once would do for anybody. Are there one or two other files that you would like to see transformed to see as an example? If you're just looking for a sense of it, that should be enough. --- David A.Wheeler
Re: What's cooking in git.git (Dec 2017, #03; Wed, 13)
On Wed, Dec 13 2017, Junio C. Hamano jotted: > > * ab/simplify-perl-makefile (2017-12-11) 1 commit > (merged to 'next' on 2017-12-13 at 1b791d2503) > + Makefile: replace perl/Makefile.PL with simple make rules > > The build procedure for perl/ part has been greatly simplified by > weaning ourselves off of MakeMaker. > > Will merge to 'master'. I noticed this tiny grammar error in the commit message which you may want to amend (or maybe it's not worth it since it's in next already): As a side-effect of these general changes the perl documentation now only installed by install-{doc,man}, not a mere "install" as before That should be: As a side-effect of these general changes the perl documentation is now only installed by install-{doc,man}, not a mere "install" as before I.e. it's missing an "is" between "documentation" and "now".
Re: [PATCH 3/8] perf/aggregate: implement codespeed JSON output
Christian Couderwrites: >> Aside from portability of 'uname -o' Eric raised, I wonder if the >> platform information is still useful even when perf-subsection is >> specified. With the above code, we can identify that a single >> result is for (say) MacOS only when we are not limiting to a single >> subsection, but wouldn't it be equally a valid desire to be able to >> track performance figures for a single subsection over time and >> being able to say "On MacOS, subsection A's performance dropped >> between release X and X+1 quite a bit, but on Linux x86-64, there >> was no such change" or somesuch? > > Yeah, I agree that it would be useful. Unfortunately it looks like the > number of fields in Codespeed is limited and I am not sure what will > be more important for people in general. Is there a reason why such textual labels meant for human consumption need to be two (or more) separate fields? Would simply concatenating them into a single string defeat whatever you wanted to achieve by having this information in $executable in the first place?
Re: What's cooking in git.git (Dec 2017, #03; Wed, 13)
Kaartic Sivaraamwrites: > It seems my series that fixes an error message in 'git-rebase'[1] > (apart from a little cleanups) is missing. I guess I addressed the > issue that was raised in 3/3 as a v3 for that patch[2]. Are any more > changes needed? > > [1]: <20171127172104.5796-1-kaartic.sivar...@gmail.com> > [2]: <20171201060935.19749-1-kaartic.sivar...@gmail.com> I think you only sent 3/3 in newer rounds, which made it not to apply to my tree. If you meant to drop changes in 1/3 and 2/3, perhaps because they were needless churn, then 3/3 needs to be updated not to depend on the changes these two made. Thanks.
[PATCH v2] partial-clone: design doc
From: Jeff HostetlerFirst draft of design document for partial clone feature. Signed-off-by: Jeff Hostetler --- Documentation/technical/partial-clone.txt | 259 ++ 1 file changed, 259 insertions(+) create mode 100644 Documentation/technical/partial-clone.txt diff --git a/Documentation/technical/partial-clone.txt b/Documentation/technical/partial-clone.txt new file mode 100644 index 000..731bd8c --- /dev/null +++ b/Documentation/technical/partial-clone.txt @@ -0,0 +1,259 @@ +Partial Clone Design Notes +== + +The "Partial Clone" feature is a performance optimization for git that +allows git to function without having a complete copy of the repository. + +During clone and fetch operations, git normally downloads the complete +contents and history of the repository. That is, during clone the client +receives all of the commits, trees, and blobs in the repository into a +local ODB. Subsequent fetches extend the local ODB with any new objects. +For large repositories, this can take significant time to download and +large amounts of diskspace to store. + +The goal of this work is to allow git better handle extremely large +repositories. Often in these repositories there are many files that the +user does not need such as ancient versions of source files, files in +portions of the worktree outside of the user's work area, or large binary +assets. If we can avoid downloading such unneeded objects *in advance* +during clone and fetch operations, we can decrease download times and +reduce ODB disk usage. + + +Non-Goals +- + +Partial clone is a mechanism to limit the number of blobs and trees downloaded +*within* a given range of commits -- and is therefore independent of and not +intended to conflict with existing DAG-level mechanisms to limit the set of +requested commits (i.e. shallow clone, single branch, or fetch ''). + + +Design Overview +--- + +Partial clone logically consists of the following parts: + +- A mechanism for the client to describe unneeded or unwanted objects to + the server. + +- A mechanism for the server to omit such unwanted objects from packfiles + sent to the client. + +- A mechanism for the client to gracefully handle missing objects (that + were previously omitted by the server). + +- A mechanism for the client to backfill missing objects as needed. + + +Design Details +-- + +- A new pack-protocol capability "filter" is added to the fetch-pack and + upload-pack negotiation. + + This uses the existing capability discovery mechanism. + See "filter" in Documentation/technical/pack-protocol.txt. + +- Clients pass a "filter-spec" to clone and fetch which is passed to the + server to request filtering during packfile construction. + + There are various filters available to accomodate different situations. + See "--filter=" in Documentation/rev-list-options.txt. + +- On the server pack-objects applies the requested filter-spec as it + creates "filtered" packfiles for the client. + + These filtered packfiles are incomplete in the traditional sense because + they may contain trees that reference blobs that the client does not have. + +- On the client these incomplete packfiles are marked as "promisor pacfiles" + and treated differently by various commands. + +- On the client a repository extension is added to the local config to + prevent older versions of git from failing mid-operation because of + missing objects that they cannot handle. + See "extensions.partialClone" in Documentation/technical/repository-version.txt" + + +Handling Missing Objects + + +- An object may be missing due to a partial clone or fetch, or missing due + to repository corruption. To differentiate these cases, the local + repository specially indicates packfiles obtained from the promisor + remote. + + These "promisor packfiles" consist of a ".promisor" file with + arbitrary contents (like the ".keep" files), in addition to + their ".pack" and ".idx" files. + + In the future, this ability may be extended to loose objects in case + a promisor packfile is accidentally unpacked. + +- The local repository considers a "promisor object" to be an object that + it knows (to the best of its ability) that the promisor remote has, either + because the local repository has that object in one of its promisor + packfiles, or because another promisor object refers to it. + + When git encounters a missing object, Git can see if it a promisor object + and handle it appropriately. If not, Git can report a corruption. + + This means that there is no need for the client to explicitly maintain an + expensive-to-modify list of missing objects. + +- Since almost all Git code currently expects any referenced object to be + present locally and because we do not want to force every command to do + a dry-run first, a fallback mechanism is added
[PATCH v2] Partial clone design document
From: Jeff HostetlerThis patch contains V2 of the partial clone design document. It incorporates suggestions from the mailing list on V1 and elaborates on a few topics. Jeff Hostetler (1): partial-clone: design doc Documentation/technical/partial-clone.txt | 259 ++ 1 file changed, 259 insertions(+) create mode 100644 Documentation/technical/partial-clone.txt -- 2.9.3
Urgent reply is needed,
Dear Partner, Please consider this mail serious despite the fact that you did not expect it. Hope you are doing well. I am Mr, Nodian Omera, The Manager of head opérations department of our bank in Burkina Faso. I discovered a risk-free deal of US$9.9 million from my department which was left unclaimed as a result of non existing body.Provided you will put trust forward, let us share the deal if you are interested. Reply to this address;nodian.om...@gmail.com, Urgent reply is needed for more details. Regards, Mr, Nodian Omera,
Re: [PATCH v2] doc: add triangular workflow
"BENSOUSSAN--BOHM DANIEL p1507430"wrote: > Added a new file about triangular workflow for a clearer organization. "for a clearer organization" is an explanation of why you are doing things this way compared to your previous email. But this is the commit message, and its wording shoud make sense in this context, i.e. regardless of previous emails you sent which won't appear it the Git history. Now, read again this sentence: adding a file about triangular workflow does not make any "organization" "clearer". > With a more precise documentation, any new Git contributors > will spend less time on understanding this doc and the way Git works. I understand what you mean, but adding a new piece of documentation cannot make people spend less time on this particular documentation. Also "any new Git contributors will spend less time" sounds like marketing speach, not technical. Your goal is to make it easier for new users, but claiming that everybody is going to gain time by reading your documentation is a bit overselling your text. I don't think you should talk about "Git contributors", which can be read as "contributors to the git.git codebase". "Git users" would make more sense, or perhaps you meant "contributors to Git projects". But actually, I don't think this kind of documentation should focus only on new users. I think many reasonably advanced Git users do not know about remote.pushdefault for example. > diff --git a/Documentation/Makefile b/Documentation/Makefile > index 2ab6556..c3e98c7 100644 > --- a/Documentation/Makefile > +++ b/Documentation/Makefile > @@ -10,6 +10,7 @@ OBSOLETE_HTML = > MAN1_TXT += $(filter-out \ > $(addsuffix .txt, $(ARTICLES) $(SP_ARTICLES)), \ > $(wildcard git-*.txt)) > +MAN1_TXT += git-triangular-workflow.txt git-*.txt is reserved for actual Git commands. Other tutorials use git*.txt (e.g. we have gitworkflows.txt and not git-workflows.txt). > +- The project maintainer publishes a repository, called **UPSTREAM** in > +this document, which is a read-only for contributors. They can clone and s/a read-only/read-only/ Perhaps s/can/& only/ too. > +- Contributors publish their modifications by pushing to a repository, > +called **PUBLISH** in this document, and request a merge. > +- Opening a pull request Other items use full sentences, this one seems half-written. Actually, I think this item is included in the "request a merge" part of the previous one. > +This workflow is commonly used on different platforms like BitBucket, > +GitHub or GitLab which provide a dedicated mechanism for requesting merges. As a user, I find it terribly frustrating when reading documentation telling me that "there's a dedicated mechanism" for something without telling me actually how to do it. > + > + > +-- - > +| UPSTREAM | maintainer | PUBLISH | > +||- - - - - - - -| | > +-- <- - > + \ / > + \ / > +fetch | \ / ^ push > + v \ / | > + \ / > + - > + | LOCAL | > + - This kind of diagram deserves a bit of text to explain the situation. For example, LOCAL is local only for the contributor (the maintainer doesn't need to know about it for example). I'd add a sentence to explain that this gives the overall view on the flow, from the point of view of a contributor. > +With the triangular workflow, the contributors have the write > +access on **PUBLISH** and push their code there. Only the "have write access", no need for "the". > +* Code review is made before integration. > + > +In a triangular workflow the rest of the community or the company > +can review the code before it's in production. Everyone can read on > +**PUBLISH** so everyone can review code before the maintainer merge > +it to **UPSTREAM**. In a free software, anyone can > +propose code. Reviewers accept the code when everyone agree > +with it. > + > +* Encourages clean history by using `rebase -i` and `push --force` to > +the public fork before the code is merged. "Encourages" has no subject. It could be OK, but for consistency with other items I'd add one ("Triangular workflow encourages ..."?). > +If **PUBLISH** doesn't exist, a contributor can publish his own repository. > +**PUBLISH** contains modifications before integration. > + > + > +* `git clone ` > +* `git remote add **PUBLISH**` git remote add needs two arguments, you're giving only one. > +* `git push` This will push to UPSTREAM, right? > +Adding **UPSTREAM** remote: > + > +=== > +`git remote add upstream ` >
Re: [PATCH] send-email: extract email-parsing code into a subroutine
"PAYRE NATHAN p1508475"wrote: > - print $c2 $_; > } > + > close $c; Nit: this added newline does not seem necessary to me. Nothing serious, but this kind of thing tend to distract the reader when reviewing the patch. > + foreach my $key (keys %parsed_email) { > + next if $key == 'body'; > + print $c2 "$key: $parsed_email{$key}"; > + } I'd add a comment like # Preserve unknown headers at the top of the loop to make it clear what we're doing. On a side note: there's no comment in the code you're adding. This is not necessarily a bad thing (beautifully written code does not need comments to be readable), but you may re-read your code with the question "did I explain everything well-enough?" in mind. The loop above is a case where IMHO a short and sweet comment helps the reader. Two potential issues not mentionned in your message but that we discussed offlist is that 1) this doesn't preserve the order, and 2) this strips duplicate headers. I believe this is not a problem here, and trying to solve these points would make the code overkill, but this would really deserve being mentionned in the commit message. First, so that people reviewing your patch now can confirm (or not) that you are taking the right decision by doing this, and also for people in the future examining your patch (e.g. after a bisect). > +sub parse_header_line { > + my $lines = shift; > + my $parsed_line = shift; > + my $pattern = join "|", qw(To Cc Bcc); Nit: you may want to rename it to something more explicit, like $addr_headers_pat. None of my nit should block the patch inclusion, but I think the commit message should be expanded to include a mention of the "duplicate headers"/"header order" potential issue. -- Matthieu Moy https://matthieu-moy.fr/
Seasons greetings
I am Mr.Sheng Li Hung, from china I got your information while search for a reliable person, I have a very profitable business proposition for you and i can assure you that you will not regret been part of this mutual beneficial transaction after completion. Kindly get back to me for more details on this email id: shengl...@hotmail.com Thanks Sheng Li Hung
Need help migrating workflow from svn to git.
Hello folks, I am wondering whether/how my mode of work for a specific project (currently based on SVN) could be transferred to git. I have a repository for maintaining configuration of hosts. This repository contains several hundered scripts. Most of those scripts are don't depend on each other. Every machine has a working copy of the repository in a specific directory. A cron job (running every 15 minutes) executes "svn update" and executes the scripts which are contained in this working copy. This way, I can commit changes to the main repository and all the hosts will "download" and adopt by executing the newest revision of those scripts. (The scripts need to be idempotent, but this is a different topic). NORMALLY, there are no local modifications in the working copy. Thus, conflicts can not happen. Everything works fine. Sometimes, I need to fix a problem on some host or need to implement a new feature. For this, I go to the working copy of a host where the change needs to be done and start haking. With svn, I don't need to stop the cron job. "svn update" will happily merge any in-coming changes and leave alone the files which were not modified upstream. Conflicts with my local modifications which I am currently hacking on are extremely rare, because the scripts are pretty much independent. So I'm pretty much happy with this mode of operation. With git, by contrast, this won't work. Git will refuse to pull anything as long as there are ANY local modifications. The cron job would need to git stash git pull git stash pop But this will temporarily remove my local modifications. If I happen to do a test run at this time, the test run would NOT contain the local modifications which I was about to test. Even worse: if I happen to save one of the modified files while the modifications are in the stash, the "git stash pop" will definitely cause a conflict, although nothing really changed. So, how would I get this workflow with git? Is it possible to emulate the behavior of "svn update"? Any ideas? -- Josef Wolf j...@raven.inka.de
Re: What's cooking in git.git (Dec 2017, #03; Wed, 13)
It seems my series that fixes an error message in 'git-rebase'[1] (apart from a little cleanups) is missing. I guess I addressed the issue that was raised in 3/3 as a v3 for that patch[2]. Are any more changes needed? [1]: <20171127172104.5796-1-kaartic.sivar...@gmail.com> [2]: <20171201060935.19749-1-kaartic.sivar...@gmail.com> -- Kaartic
[PATCH v4 2/2] Doc/check-ref-format: clarify information about @{-N} syntax
When the N-th previous thing checked out syntax (@{-N}) is used with '--branch' option of check-ref-format the result may not be the name of a branch that currently exists or ever existed. This is because @{-N} is used to refer to the N-th last checked out "thing", which might be a commit object name if the previous check out was a detached HEAD state; or a branch name, otherwise. The documentation thus does a wrong thing by promoting it as the "previous branch syntax". State that @{-N} is the syntax for specifying "N-th last thing checked out" and also state that the result of using @{-N} might also result in an commit object name. Signed-off-by: Kaartic Sivaraam--- Changes in v4: - updated the commit message - made changes suggested by Junio Documentation/git-check-ref-format.txt | 28 1 file changed, 16 insertions(+), 12 deletions(-) diff --git a/Documentation/git-check-ref-format.txt b/Documentation/git-check-ref-format.txt index cf0a0b7df..8172a6b9a 100644 --- a/Documentation/git-check-ref-format.txt +++ b/Documentation/git-check-ref-format.txt @@ -78,17 +78,21 @@ reference name expressions (see linkgit:gitrevisions[7]): . at-open-brace `@{` is used as a notation to access a reflog entry. With the `--branch` option, the command takes a name and checks if -it can be used as a valid branch name (e.g. when creating a new -branch). The rule `git check-ref-format --branch $name` implements -may be stricter than what `git check-ref-format refs/heads/$name` -says (e.g. a dash may appear at the beginning of a ref component, -but it is explicitly forbidden at the beginning of a branch name). -When run with `--branch` option in a repository, the input is first -expanded for the ``previous branch syntax'' -`@{-n}`. For example, `@{-1}` is a way to refer the last branch you -were on. This option should be used by porcelains to accept this -syntax anywhere a branch name is expected, so they can act as if you -typed the branch name. +it can be used as a valid branch name e.g. when creating a new branch +(but be cautious when using the previous checkout syntax; it may refer +to a detached HEAD state). The rule `git check-ref-format --branch +$name` implements may be stricter than what `git check-ref-format +refs/heads/$name` says (e.g. a dash may appear at the beginning of a +ref component, but it is explicitly forbidden at the beginning of a +branch name). When run with `--branch` option in a repository, the +input is first expanded for the ``previous checkout syntax'' +`@{-n}`. For example, `@{-1}` is a way to refer the last thing that +was checked out using "git checkout" operation. This option should be +used by porcelains to accept this syntax anywhere a branch name is +expected, so they can act as if you typed the branch name. As an +exception note that, the ``previous checkout operation'' might result +in a commit object name when the N-th last thing checked out was not +a branch. OPTIONS --- @@ -116,7 +120,7 @@ OPTIONS EXAMPLES -* Print the name of the previous branch: +* Print the name of the previous thing checked out: + $ git check-ref-format --branch @{-1} -- 2.15.0.531.g2ccb3012c
[PATCH] send-email: extract email-parsing code into a subroutine
The existing code mixes parsing of email header with regular expression and actual code. Extract the parsing code into a new subroutine "parse_header_line()". This improves the code readability and make parse_header_line reusable in other place. "parsed_header_line()" and "filter_body()" could be used for refactoring the part of code which parses the header to prepare the email and send it. Signed-off-by: Nathan PayreSigned-off-by: Matthieu Moy Signed-off-by: Timothee Albertin Signed-off-by: Daniel Bensoussan Signed-off-by: Junio C Hamano Thanks-to: Ævar Arnfjörð Bjarmason --- >> "PAYRE NATHAN p1508475" wrote: >>> + my %parsed_email; >>> + $parsed_email{'body'} = ''; >>> + while (my $line = <$c>) { >>> + next if $line =~ m/^GIT:/; >>> + parse_header_line($line, \%parsed_email); >>> + if ($line =~ /^$/) { >>> + $parsed_email{'body'} = filter_body($c); >>> } >>> - print $c2 $_; >> >> I didn't notice this at first, but you're modifying the behavior here: >> the old code used to print to $c2 anything that didn't match any of >> the if/else if branches. >> >> To keep this behavior, you need to keep all these extra headers in >> $parsed_email (you do, in this version) and print them after taking >> care of all the known headers (AFAICT, you don't). > > This case is not that easy to correct because: > - It's could weigh the code. > - The refactoring may not be legitimate anymore. > > I've found two way to resolve this: > .1) After every if($parsed_email{'key'}) remove the corresponding key > and just before closing $c2 create a new loop which add all the > remaining parts. > > .2) Making a mix between the old and new code. Some parts of > my patch can improve the old code (like the removing of > $need_8bit_cte) then it will be kept and the while loop will be > similar the old code > > I think that the first version will look like better than the second > one, easy to read, but it will change the order of the email header. This is how I see the first choice of the two I've proposed in my last email. git-send-email.perl | 116 +++- 1 file changed, 78 insertions(+), 38 deletions(-) diff --git a/git-send-email.perl b/git-send-email.perl index 2208dcc21..f942fc2a5 100755 --- a/git-send-email.perl +++ b/git-send-email.perl @@ -703,57 +703,71 @@ EOT3 do_edit($compose_filename); } - open my $c2, ">", $compose_filename . ".final" - or die sprintf(__("Failed to open %s.final: %s"), $compose_filename, $!); - open $c, "<", $compose_filename or die sprintf(__("Failed to open %s: %s"), $compose_filename, $!); - my $need_8bit_cte = file_has_nonascii($compose_filename); - my $in_body = 0; - my $summary_empty = 1; if (!defined $compose_encoding) { $compose_encoding = "UTF-8"; } - while(<$c>) { - next if m/^GIT:/; - if ($in_body) { - $summary_empty = 0 unless (/^\n$/); - } elsif (/^\n$/) { - $in_body = 1; - if ($need_8bit_cte) { - print $c2 "MIME-Version: 1.0\n", -"Content-Type: text/plain; ", - "charset=$compose_encoding\n", -"Content-Transfer-Encoding: 8bit\n"; - } - } elsif (/^MIME-Version:/i) { - $need_8bit_cte = 0; - } elsif (/^Subject:\s*(.+)\s*$/i) { - $initial_subject = $1; - my $subject = $initial_subject; - $_ = "Subject: " . - quote_subject($subject, $compose_encoding) . - "\n"; - } elsif (/^In-Reply-To:\s*(.+)\s*$/i) { - $initial_reply_to = $1; - next; - } elsif (/^From:\s*(.+)\s*$/i) { - $sender = $1; - next; - } elsif (/^(?:To|Cc|Bcc):/i) { - print __("To/Cc/Bcc fields are not interpreted yet, they have been ignored\n"); - next; + + my %parsed_email; + while (my $line = <$c>) { + next if $line =~ m/^GIT:/; + parse_header_line($line, \%parsed_email); + if ($line =~ /^$/) { + $parsed_email{'body'} = filter_body($c); } - print $c2 $_; } + close $c; - close $c2; - if
[PATCH v2] doc: add triangular workflow
Added a new file about triangular workflow for a clearer organization. With a more precise documentation, any new Git contributors will spend less time on understanding this doc and the way Git works. Based-on-patch-by: Jordan DE GEASigned-off-by: Michael Haggerty Signed-off-by: Jordan DE GEA Signed-off-by: Matthieu Moy Signed-off-by: Timothee Albertin Signed-off-by: Nathan Payre Signed-off-by: Daniel Bensoussan --- Documentation/Makefile| 1 + Documentation/git-triangular-workflow.txt | 221 ++ Documentation/git.txt | 2 +- Documentation/gittutorial.txt | 1 + Documentation/gitworkflows.txt| 1 + 5 files changed, 225 insertions(+), 1 deletion(-) create mode 100644 Documentation/git-triangular-workflow.txt diff --git a/Documentation/Makefile b/Documentation/Makefile index 2ab6556..c3e98c7 100644 --- a/Documentation/Makefile +++ b/Documentation/Makefile @@ -10,6 +10,7 @@ OBSOLETE_HTML = MAN1_TXT += $(filter-out \ $(addsuffix .txt, $(ARTICLES) $(SP_ARTICLES)), \ $(wildcard git-*.txt)) +MAN1_TXT += git-triangular-workflow.txt MAN1_TXT += git.txt MAN1_TXT += gitk.txt MAN1_TXT += gitremote-helpers.txt diff --git a/Documentation/git-triangular-workflow.txt b/Documentation/git-triangular-workflow.txt new file mode 100644 index 000..ee599b7 --- /dev/null +++ b/Documentation/git-triangular-workflow.txt @@ -0,0 +1,221 @@ +git-triangular-workflow(1) +== + +NAME + +git-triangular-workflow - An overview of the triangular workflow with Git + + +SYNOPSIS + +[verse] +git * + + +DESCRIPTION +--- + +In some projects, contributors cannot push directly to the project but +have to suggest their commits to the maintainer (e.g. pull requests). +For these projects, it's common to use what's called a *triangular +workflow*: + +- The project maintainer publishes a repository, called **UPSTREAM** in +this document, which is a read-only for contributors. They can clone and +fetch from this repository. +- Contributors publish their modifications by pushing to a repository, +called **PUBLISH** in this document, and request a merge. +- Opening a pull request +- If the maintainers accepts the changes, they merge them into the + **UPSTREAM** repository. + +This workflow is commonly used on different platforms like BitBucket, +GitHub or GitLab which provide a dedicated mechanism for requesting merges. + + +-- - +| UPSTREAM | maintainer | PUBLISH | +||- - - - - - - -| | +-- <- - + \ / + \ / +fetch | \ / ^ push + v \ / | + \ / + - + | LOCAL | + - + + +Motivations +~~~ + +* Allows contributors to work with Git even if they don't have +write access to **UPSTREAM**. + +With the triangular workflow, the contributors have the write +access on **PUBLISH** and push their code there. Only the +maintainers merge from **PUBLISH** to **UPSTREAM**. + +* Code review is made before integration. + +In a triangular workflow the rest of the community or the company +can review the code before it's in production. Everyone can read on +**PUBLISH** so everyone can review code before the maintainer merge +it to **UPSTREAM**. In a free software, anyone can +propose code. Reviewers accept the code when everyone agree +with it. + +* Encourages clean history by using `rebase -i` and `push --force` to +the public fork before the code is merged. + +This is just a side-effect of the "review before merge" mentioned +above but this is still a good point. + + +Here are the configuration variables you will need to arrange your +workflow. + +Preparation as a contributor + + +Cloning from **UPSTREAM**. + +== +`git clone ` +== + +If **PUBLISH** doesn't exist, a contributor can publish his own repository. +**PUBLISH** contains modifications before integration. + + +* `git clone ` +* `git remote add **PUBLISH**` +* `git push` + + +Adding **UPSTREAM** remote: + +=== +`git remote add upstream ` +=== + +With the `remote add` above, using `git pull upstream` pulls there, +instead of saying its URL. In addition, the `git pull` command
Re: [PATCH v4] git-gui: Prevent double UTF-8 conversion
It was <2017-12-14 czw 10:42>, when Eric Sunshine wrote: > On Thu, Dec 14, 2017 at 4:32 AM, Łukasz Stelmach> wrote: >> Convert author's name and e-mail address from the UTF-8 (or any other) >> encoding in load_last_commit function the same way commit message is >> converted. >> >> Amending commits in git-gui without such conversion breaks UTF-8 >> strings. For example, "\305\201ukasz" (as written by git cat-file) becomes >> "\303\205\302\201ukasz" in an amended commit. >> >> Signed-off-by: Łukasz Stelmach >> Reviewed-by: Johannes Schindelin >> --- >> Changes since v3: >> >> - Added Reviewed-by footer. Thank you Johannes Schindelin, for review. > > No need to re-send. If you consult Junio's latest "What's cooking"[1], > you'll find that your patch has already graduated from his 'pu' branch > to 'next' and is slated to graduate to 'master' (at some point). > > [1]: > https://public-inbox.org/git/xmqqzi6mutcc@gitster.mtv.corp.google.com/ I am sorry. I didn't get any notifiaction by mail and I haven't studied Documentation/SubmittingPatches throughly enough. -- Łukasz Stelmach Samsung R Institute Poland Samsung Electronics signature.asc Description: PGP signature
Re: [PATCH] send-email: extract email-parsing code into a subroutine
2017-12-11 22:12 GMT+01:00 Matthieu Moy: > "PAYRE NATHAN p1508475" wrote: >> + my %parsed_email; >> + $parsed_email{'body'} = ''; >> + while (my $line = <$c>) { >> + next if $line =~ m/^GIT:/; >> + parse_header_line($line, \%parsed_email); >> + if ($line =~ /^$/) { >> + $parsed_email{'body'} = filter_body($c); >> } >> - print $c2 $_; > > I didn't notice this at first, but you're modifying the behavior here: > the old code used to print to $c2 anything that didn't match any of > the if/else if branches. > > To keep this behavior, you need to keep all these extra headers in > $parsed_email (you do, in this version) and print them after taking > care of all the known headers (AFAICT, you don't). This case is not that easy to correct because: - It's could weigh the code. - The refactoring may not be legitimate anymore. I've found two way to resolve this: .1) After every if($parsed_email{'key'}) remove the corresponding key and just before closing $c2 create a new loop which add all the remaining parts. .2) Making a mix between the old and new code. Some parts of my patch can improve the old code (like the removing of $need_8bit_cte) then it will be kept and the while loop will be similar the old code I think that the first version will look like better than the second one, easy to read, but it will change the order of the email header.
Re: [PATCH v4] git-gui: Prevent double UTF-8 conversion
On Thu, Dec 14, 2017 at 4:32 AM, Łukasz Stelmachwrote: > Convert author's name and e-mail address from the UTF-8 (or any other) > encoding in load_last_commit function the same way commit message is > converted. > > Amending commits in git-gui without such conversion breaks UTF-8 > strings. For example, "\305\201ukasz" (as written by git cat-file) becomes > "\303\205\302\201ukasz" in an amended commit. > > Signed-off-by: Łukasz Stelmach > Reviewed-by: Johannes Schindelin > --- > Changes since v3: > > - Added Reviewed-by footer. Thank you Johannes Schindelin, for review. No need to re-send. If you consult Junio's latest "What's cooking"[1], you'll find that your patch has already graduated from his 'pu' branch to 'next' and is slated to graduate to 'master' (at some point). [1]: https://public-inbox.org/git/xmqqzi6mutcc@gitster.mtv.corp.google.com/
[PATCH v4] git-gui: Prevent double UTF-8 conversion
Convert author's name and e-mail address from the UTF-8 (or any other) encoding in load_last_commit function the same way commit message is converted. Amending commits in git-gui without such conversion breaks UTF-8 strings. For example, "\305\201ukasz" (as written by git cat-file) becomes "\303\205\302\201ukasz" in an amended commit. Signed-off-by: Łukasz StelmachReviewed-by: Johannes Schindelin --- Changes since v3: - Added Reviewed-by footer. Thank you Johannes Schindelin, for review. git-gui/lib/commit.tcl | 12 +--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/git-gui/lib/commit.tcl b/git-gui/lib/commit.tcl index 83620b7cb..75ea965da 100644 --- a/git-gui/lib/commit.tcl +++ b/git-gui/lib/commit.tcl @@ -25,6 +25,8 @@ You are currently in the middle of a merge that has not been fully completed. Y set msg {} set parents [list] if {[catch { + set name "" + set email "" set fd [git_read cat-file commit $curHEAD] fconfigure $fd -encoding binary -translation lf # By default commits are assumed to be in utf-8 @@ -34,9 +36,7 @@ You are currently in the middle of a merge that has not been fully completed. Y lappend parents [string range $line 7 end] } elseif {[string match {encoding *} $line]} { set enc [string tolower [string range $line 9 end]] - } elseif {[regexp "author (.*)\\s<(.*)>\\s(\\d.*$)" $line all name email time]} { - set commit_author [list name $name email $email date $time] - } + } elseif {[regexp "author (.*)\\s<(.*)>\\s(\\d.*$)" $line all name email time]} { } } set msg [read $fd] close $fd @@ -44,7 +44,13 @@ You are currently in the middle of a merge that has not been fully completed. Y set enc [tcl_encoding $enc] if {$enc ne {}} { set msg [encoding convertfrom $enc $msg] + set name [encoding convertfrom $enc $name] + set email [encoding convertfrom $enc $email] } + if {$name ne {} && $email ne {}} { + set commit_author [list name $name email $email date $time] + } + set msg [string trim $msg] } err]} { error_popup [strcat [mc "Error loading commit data for amend:"] "\n\n$err"] -- 2.11.0
Re: [PATCH 4/8] perf/run: use $default_value instead of $4
On Wed, Dec 13, 2017 at 9:54 PM, Junio C Hamanowrote: > Junio C Hamano writes: > >> If you want to be able to use this helper to specify a default value >> of an empty string (which the orignal that used $4 did), then the >> previous hunk must be corrected so that it does not unconditionally >> set default_value to $4. Perhaps like >> >> if test -n "${4+x}" >> then >> default_value=$4 >> else >> unset default_value || : >> fi >> >> or something. > > And if you do not care about passing an empty string as the default > value, then the earlier hunk can stay the same > > default_value=$4 > > but the eval part should become something like > > test -n "$default_value" && eval ... > > Given that you are planning to add yet another optional argument to > the helper, and when you pass that variable, you'd probably need to > pass "" as the "default" that is not exported, perhaps this "give up > ability to pass an empty string as the default" approach may be the > only thing you could do. Yeah, thanks for the explanations and suggestions.
Re: [PATCH 4/8] perf/run: use $default_value instead of $4
On Wed, Dec 13, 2017 at 9:40 PM, Junio C Hamanowrote: > Christian Couder writes: > >> Signed-off-by: Christian Couder >> --- >> t/perf/run | 4 ++-- >> 1 file changed, 2 insertions(+), 2 deletions(-) >> >> diff --git a/t/perf/run b/t/perf/run >> index 43e4de49ef..bbd703dc4f 100755 >> --- a/t/perf/run >> +++ b/t/perf/run >> @@ -105,7 +105,7 @@ get_var_from_env_or_config () { >> env_var="$1" >> conf_sec="$2" >> conf_var="$3" >> - # $4 can be set to a default value >> + default_value="$4" # optional >> >> # Do nothing if the env variable is already set >> eval "test -z \"\${$env_var+x}\"" || return >> @@ -123,7 +123,7 @@ get_var_from_env_or_config () { >> conf_value=$(git config -f "$GIT_PERF_CONFIG_FILE" "$var") && >> eval "$env_var=\"$conf_value\"" && return >> >> - test -n "${4+x}" && eval "$env_var=\"$4\"" >> + test -n "${default_value+x}" && eval "$env_var=\"$default_value\"" > > This conversion changes the behaviour. Because default_value is > always set by your change in the previous hunk, we end up always > doing this eval. > > The original says "If $4 is set, then ${4+x} becomes x and if $4 is > not set, ${4+x} is empty, so let's check if ${4+x} is a non-empty > string to see if $4 is set. If ${4+x} is a non-empty string, that > means $4 was set so we do the eval. > > If you want to be able to use this helper to specify a default value > of an empty string (which the orignal that used $4 did), then the > previous hunk must be corrected so that it does not unconditionally > set default_value to $4. Perhaps like > > if test -n "${4+x}" > then > default_value=$4 > else > unset default_value || : > fi > > or something. Yeah, you are right I will fix this.
Re: [PATCH 3/8] perf/aggregate: implement codespeed JSON output
On Wed, Dec 13, 2017 at 9:33 PM, Junio C Hamanowrote: > Christian Couder writes: > >> my $resultsdir = "test-results"; >> +my $results_section = ""; >> if (exists $ENV{GIT_PERF_SUBSECTION} and $ENV{GIT_PERF_SUBSECTION} ne "") { >> $resultsdir .= "/" . $ENV{GIT_PERF_SUBSECTION}; >> + $results_section = $ENV{GIT_PERF_SUBSECTION}; >> } > > ... > >> + my $executable; >> + if ($results_section eq "") { >> + $executable = `uname -o -p`; >> + } else { >> + $executable = $results_section; >> + chomp $executable; >> + } > > Aside from portability of 'uname -o' Eric raised, I wonder if the > platform information is still useful even when perf-subsection is > specified. With the above code, we can identify that a single > result is for (say) MacOS only when we are not limiting to a single > subsection, but wouldn't it be equally a valid desire to be able to > track performance figures for a single subsection over time and > being able to say "On MacOS, subsection A's performance dropped > between release X and X+1 quite a bit, but on Linux x86-64, there > was no such change" or somesuch? Yeah, I agree that it would be useful. Unfortunately it looks like the number of fields in Codespeed is limited and I am not sure what will be more important for people in general. Another option would be to have "MacOS" in the "environment" field. Or maybe there is a need for a generic way to customize this. For now I just tried to come up with something sensible for what AEvar and me want to do. > IOW, shouldn't the "executable" label always contain the platform > information, plus an optional subsection info when (and only when) > the result is limited to a subsection? > > By the way, $results_section that is not an empty string at this > point must have come from $ENV{GIT_PERF_SUBSECTION}, and from the > way the environment variable is used in t/perf/run, e.g. > > ( > GIT_PERF_SUBSECTION="$subsec" > export GIT_PERF_SUBSECTION > echo " Run for subsection > '$GIT_PERF_SUBSECTION' " > run_subsection "$@" > ) > > I do not think we expect it to have a trailing LF. What's that > chomp doing there? It's a silly mistake I made when I reorganized the patches just before sending them. The chomp should be after "$executable = `uname -o -p`;" (so in the other branch of the "if ... else"). I will fix this in the next version.
Re: [PATCH 3/8] perf/aggregate: implement codespeed JSON output
On Wed, Dec 13, 2017 at 7:25 PM, Eric Sunshinewrote: > On Wed, Dec 13, 2017 at 10:13 AM, Christian Couder > wrote: >> Codespeed (https://github.com/tobami/codespeed/) is an open source >> project that can be used to track how some software performs over >> time. It stores performance test results in a database and can show >> nice graphs and charts on a web interface. >> >> As it can be interesting to Codespeed to see how Git performance >> evolves over time and releases, let's implement a Codespeed output >> in "perf/aggregate.perl". >> >> Signed-off-by: Christian Couder >> --- >> diff --git a/t/perf/aggregate.perl b/t/perf/aggregate.perl >> @@ -174,6 +181,63 @@ sub print_default_results { >> +sub print_codespeed_results { >> + my ($results_section) = @_; >> + >> + my $project = "Git"; >> + >> + my $executable; >> + if ($results_section eq "") { >> + $executable = `uname -o -p`; > > Option -o is not recognized on MacOS and, on at least a couple of my > Linux installations, -p returns only "unknown". > > A combination, on the other hand, which seems to work nicely on MacOS, > Linux, and BSD is: uname -s -m Ok I will take a look at this. Thanks!