[ANNOUNCE] tig-2.3.1

2017-12-14 Thread Jonas Fonseca
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

2017-12-14 Thread Junio C Hamano
Jonathan Nieder  writes:

>> 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.

2017-12-14 Thread Igor Djordjevic
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

2017-12-14 Thread SZEDER Gábor
On Thu, Dec 14, 2017 at 12:10 AM, Lars Schneider
 wrote:
>
>> 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

2017-12-14 Thread Todd Zullinger
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

2017-12-14 Thread Johannes Schindelin
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

2017-12-14 Thread Johannes Schindelin
From: Eric Sunshine 

It 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

2017-12-14 Thread Johannes Schindelin
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

2017-12-14 Thread Johannes Schindelin
Hi Junio,

On Fri, 8 Dec 2017, Junio C Hamano wrote:

> Jonathan Nieder  writes:
> 
> >> 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

2017-12-14 Thread Johannes Schindelin
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

2017-12-14 Thread Junio C Hamano
Lars Schneider  writes:

> 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

2017-12-14 Thread Jonathan Nieder
Hi,

Stefan Beller wrote:
> On Thu, Dec 14, 2017 at 1:22 PM, Jonathan Nieder  wrote:

>> - 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

2017-12-14 Thread Junio C Hamano
Jonathan Nieder  writes:

> 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

2017-12-14 Thread Stefan Beller
On Thu, Dec 14, 2017 at 1:22 PM, Jonathan Nieder  wrote:

> - 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.

2017-12-14 Thread Igor Djordjevic
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

2017-12-14 Thread Junio C Hamano
Jonathan Tan  writes:

> 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

2017-12-14 Thread Junio C Hamano
Jonathan Tan  writes:

> 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

2017-12-14 Thread Jeff Hostetler


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

2017-12-14 Thread Jonathan Tan
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

2017-12-14 Thread Jonathan Tan
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

2017-12-14 Thread Jonathan Tan
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

2017-12-14 Thread Christian Couder
On Thu, Dec 14, 2017 at 10:19 PM, Jeff Hostetler  wrote:

> --- 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"

2017-12-14 Thread Stefan Beller
On Thu, Dec 14, 2017 at 1:16 PM, David A. Wheeler  wrote:
> "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)

2017-12-14 Thread Jonathan Nieder
Junio C Hamano wrote:
> Jonathan Nieder  writes:

>>> * 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

2017-12-14 Thread Jonathan Nieder
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

2017-12-14 Thread Jeff Hostetler
From: Jeff Hostetler 

Here 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

2017-12-14 Thread Jeff Hostetler
From: Jeff Hostetler 

Signed-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

2017-12-14 Thread Jeff Hostetler
From: Jeff Hostetler 

Design 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

2017-12-14 Thread Junio C Hamano
From: Kaartic Sivaraam 
Date: 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

2017-12-14 Thread Jeff Hostetler
From: Jeff Hostetler 

This 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

2017-12-14 Thread Junio C Hamano
From: Kaartic Sivaraam 
Date: 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)

2017-12-14 Thread Junio C Hamano
Junio C Hamano  writes:

> 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.

2017-12-14 Thread Randall S. Becker
> 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)

2017-12-14 Thread Junio C Hamano
Jonathan Nieder  writes:

>> * 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)

2017-12-14 Thread Jonathan Nieder
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

2017-12-14 Thread Jeff Hostetler



On 12/14/2017 1:24 PM, Junio C Hamano wrote:

Jeff Hostetler  writes:


+- 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

2017-12-14 Thread Junio C Hamano
Matthieu Moy  writes:

> 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

2017-12-14 Thread Bennett, Brian
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

2017-12-14 Thread Jeff Hostetler



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

2017-12-14 Thread Todd Zullinger
Junio C Hamano wrote:
> Todd Zullinger  writes:
>> 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

2017-12-14 Thread Jeff Hostetler


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"

2017-12-14 Thread Ævar Arnfjörð Bjarmason

On Thu, Dec 14 2017, Junio C. Hamano jotted:

> Stefan Beller  writes:
>
>> 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"

2017-12-14 Thread Junio C Hamano
"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"

2017-12-14 Thread David A. Wheeler
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.

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"

2017-12-14 Thread Junio C Hamano
Stefan Beller  writes:

> 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

2017-12-14 Thread Junio C Hamano
Thanks.


Re: [PATCH 0/2] t/lib-git-svn.sh: improve svnserve tests with parallel make test

2017-12-14 Thread Junio C Hamano
Todd Zullinger  writes:

> 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

2017-12-14 Thread Junio C Hamano
Jeff Hostetler  writes:

> +  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

2017-12-14 Thread Junio C Hamano
Jeff Hostetler  writes:

> +- 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"

2017-12-14 Thread Stefan Beller
On Thu, Dec 14, 2017 at 10:08 AM, Junio C Hamano  wrote:
> Æ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

2017-12-14 Thread Todd Zullinger
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

2017-12-14 Thread Eric Sunshine
On Thu, Dec 14, 2017 at 5:43 AM, Łukasz Stelmach  wrote:
> 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"

2017-12-14 Thread Junio C Hamano
Æ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.


Re: [PATCH v4 2/2] Doc/check-ref-format: clarify information about @{-N} syntax

2017-12-14 Thread Junio C Hamano
Kaartic Sivaraam  writes:

>  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"

2017-12-14 Thread Ævar Arnfjörð Bjarmason

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"

2017-12-14 Thread David A. Wheeler
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)

2017-12-14 Thread Ævar Arnfjörð Bjarmason

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

2017-12-14 Thread Junio C Hamano
Christian Couder  writes:

>> 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)

2017-12-14 Thread Junio C Hamano
Kaartic Sivaraam  writes:

> 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

2017-12-14 Thread Jeff Hostetler
From: Jeff Hostetler 

First 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

2017-12-14 Thread Jeff Hostetler
From: Jeff Hostetler 

This 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,

2017-12-14 Thread Mr,Nodian Omera
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

2017-12-14 Thread Matthieu Moy
"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

2017-12-14 Thread Matthieu Moy
"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

2017-12-14 Thread Mr Sheng Li Hung
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.

2017-12-14 Thread Josef Wolf
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)

2017-12-14 Thread Kaartic Sivaraam
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

2017-12-14 Thread Kaartic Sivaraam
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

2017-12-14 Thread Nathan Payre
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 Payre 
Signed-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

2017-12-14 Thread Daniel Bensoussan
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 GEA 
Signed-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

2017-12-14 Thread Łukasz Stelmach
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-14 Thread Nathan PAYRE
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

2017-12-14 Thread Eric Sunshine
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/


[PATCH v4] git-gui: Prevent double UTF-8 conversion

2017-12-14 Thread Łukasz Stelmach
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.

 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

2017-12-14 Thread Christian Couder
On Wed, Dec 13, 2017 at 9:54 PM, Junio C Hamano  wrote:
> 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

2017-12-14 Thread Christian Couder
On Wed, Dec 13, 2017 at 9:40 PM, Junio C Hamano  wrote:
> 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

2017-12-14 Thread Christian Couder
On Wed, Dec 13, 2017 at 9:33 PM, Junio C Hamano  wrote:
> 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

2017-12-14 Thread Christian Couder
On Wed, Dec 13, 2017 at 7:25 PM, Eric Sunshine  wrote:
> 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!