Re: [PATCH] contrib/subtree: fix "subtree split" skipped-merge bug.

[Eric Sunshine]

On Mon, Dec 7, 2015 at 3:50 PM, Dave Ware  wrote:
> [PATCH] contrib/subtree: fix "subtree split" skipped-merge bug.

As an aid for reviewers, please indicate the version of this patch
submission. For instance, this is the second attempt, so the subject
would be decorated as [PATCH v2], and the next one (if submitted) will
be v3. The -v option of git-format-patch can help automate this.

Style: drop the full-stop (period) from the subject line

> A bug occurs in 'git-subtree split' where a merge is skipped even when
> both parents act on the subtree, provided the merge results in a tree
> identical to one of the parents. Fix by copying the merge if at least
> one parent is non-identical, and the non-identical parent is not an
> ancestor of the identical parent.
>
> Also adding a test case, this checks that a descendant can be pushed to

s/Also adding/Also, add/
s/, this/which/

> it's ancestor in this case.

s/it's/its/

> Signed-off-by: Dave Ware 
> ---

Right here below the "---" line is a good place to describe what
changed since the previous version. For instance, in v2, you made
minor improvements to the commit message, added your sign-off, folded
the new test into the existing t7900-subtree.sh, added a subshell
around 'cd', and assigned the output of git-rev-list to a shell
variable rather than dumping it to a file.

Including a link to the previous version, like this[1], is also
reviewer-friendly.

[1]: http://thread.gmane.org/gmane.comp.version-control.git/282065

As before, I'm not a git-subtree user, so this review is superficial.
More below...

>  contrib/subtree/git-subtree.sh | 12 +++--
>  contrib/subtree/t/t7900-subtree.sh | 52 
> ++
>  2 files changed, 62 insertions(+), 2 deletions(-)
>
> diff --git a/contrib/subtree/t/t7900-subtree.sh 
> b/contrib/subtree/t/t7900-subtree.sh
> index 9051982..ea991eb 100755
> --- a/contrib/subtree/t/t7900-subtree.sh
> +++ b/contrib/subtree/t/t7900-subtree.sh
> @@ -468,4 +468,56 @@ test_expect_success 'verify one file change per commit' '
> ))
>  '
>
> +test_expect_success 'subtree descendent check' '
> +  mkdir git_subtree_split_check &&
> +  (
> +cd git_subtree_split_check &&

Style: indent with tabs rather than spaces

> +git init &&
> +
> +mkdir folder &&
> +
> +echo a >folder/a &&
> +git add . &&
> +git commit -m "first commit" &&
> +
> +git branch branch &&
> +
> +echo 0 >folder/0 &&
> +git add . &&
> +git commit -m "adding 0 to folder" &&
> +
> +echo b >folder/b &&
> +git add . &&
> +git commit -m "adding b to folder" &&
> +cherry=$(git rev-list HEAD -1) &&

git-rev-parse would probably be more idiomatic:

cherry=$(git rev-parse HEAD)

> +git checkout branch &&
> +echo text >textBranch.txt &&
> +git add . &&
> +git commit -m "commit to fiddle with branch: branch" &&
> +
> +git cherry-pick $cherry &&
> +git checkout master &&
> +git merge -m "merge" branch &&
> +
> +git branch noop_branch &&
> +
> +echo d >folder/d &&
> +git add . &&
> +git commit -m "adding d to folder" &&
> +
> +git checkout noop_branch &&
> +echo moreText >anotherText.txt &&
> +git add . &&
> +git commit -m "irrelevant" &&
> +
> +git checkout master &&
> +git merge -m "second merge" noop_branch &&
> +
> +git subtree split --prefix folder/ --branch subtree_tip master &&
> +git subtree split --prefix folder/ --branch subtree_branch branch &&
> +git push . subtree_tip:subtree_branch
> +  )
> +  '
> +
>  test_done
> --
> 1.9.1
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 1/2] gitk: match ttk fonts to gitk fonts

[Giuseppe Bilotta]

The fonts set in setoptions aren't consistently picked up by ttk, who
uses its own predefined fonts. This is noticeable when switching
between using and not using ttk with custom fonts or in HiDPI settings
(where the default TTK fonts do _not_ respect tk sclaing).

Fix by mapping the ttk fontset to the one used by gitk internally.

Signed-off-by: Giuseppe Bilotta 
---
 gitk | 18 ++
 1 file changed, 18 insertions(+)

diff --git a/gitk b/gitk
index fcc606e..e04264b 100755
--- a/gitk
+++ b/gitk
@@ -1943,6 +1943,8 @@ proc confirm_popup {msg {owner .}} {
 }
 
 proc setoptions {} {
+global use_ttk
+
 if {[tk windowingsystem] ne "win32"} {
 option add *Panedwindow.showHandle 1 startupFile
 option add *Panedwindow.sashRelief raised startupFile
@@ -1965,6 +1967,18 @@ proc setoptions {} {
 option add *Listbox.font mainfont startupFile
 }
 
+proc setttkstyle {} {
+eval font configure TkDefaultFont [fontflags mainfont]
+eval font configure TkTextFont [fontflags textfont]
+eval font configure TkHeadingFont [fontflags mainfont]
+eval font configure TkCaptionFont [fontflags mainfont] -weight bold
+eval font configure TkTooltipFont [fontflags uifont]
+eval font configure TkFixedFont   [fontflags textfont]
+eval font configure TkIconFont[fontflags uifont]
+eval font configure TkMenuFont[fontflags uifont]
+eval font configure TkSmallCaptionFont [fontflags uifont]
+}
+
 # Make a menu and submenus.
 # m is the window name for the menu, items is the list of menu items to add.
 # Each item is a list {mc label type description options...}
@@ -12356,6 +12370,10 @@ if {![info exists have_ttk]} {
 set use_ttk [expr {$have_ttk && $want_ttk}]
 set NS [expr {$use_ttk ? "ttk" : ""}]
 
+if {$use_ttk} {
+setttkstyle
+}
+
 regexp {^git version ([\d.]*\d)} [exec git version] _ git_version
 
 set show_notes {}
-- 
2.6.3.659.gfdd8f28

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 2/2] gitk: let .bleft.mid widgets 'breathe'

[Giuseppe Bilotta]

The widgets on top of the diff window are very tightly packed. Make
them breathe a little by adding an 'i'-spaced padding between them.

Signed-off-by: Giuseppe Bilotta 
---
 gitk | 8 ++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/gitk b/gitk
index e04264b..b621762 100755
--- a/gitk
+++ b/gitk
@@ -2361,6 +2361,9 @@ proc makewindow {} {
 ${NS}::frame .bleft.mid
 ${NS}::frame .bleft.bottom
 
+# gap between sub-widgets
+set wgap [font measure uifont "i"]
+
 ${NS}::button .bleft.top.search -text [mc "Search"] -command dosearch
 pack .bleft.top.search -side left -padx 5
 set sstring .bleft.top.sstring
@@ -2375,8 +2378,9 @@ proc makewindow {} {
-command changediffdisp -variable diffelide -value {0 1}
 ${NS}::radiobutton .bleft.mid.new -text [mc "New version"] \
-command changediffdisp -variable diffelide -value {1 0}
+
 ${NS}::label .bleft.mid.labeldiffcontext -text "  [mc "Lines of 
context"]: "
-pack .bleft.mid.diff .bleft.mid.old .bleft.mid.new -side left
+pack .bleft.mid.diff .bleft.mid.old .bleft.mid.new -side left -ipadx $wgap
 spinbox .bleft.mid.diffcontext -width 5 \
-from 0 -increment 1 -to 1000 \
-validate all -validatecommand "diffcontextvalidate %P" \
@@ -2384,7 +2388,7 @@ proc makewindow {} {
 .bleft.mid.diffcontext set $diffcontext
 trace add variable diffcontextstring write diffcontextchange
 lappend entries .bleft.mid.diffcontext
-pack .bleft.mid.labeldiffcontext .bleft.mid.diffcontext -side left
+pack .bleft.mid.labeldiffcontext .bleft.mid.diffcontext -side left -ipadx 
$wgap
 ${NS}::checkbutton .bleft.mid.ignspace -text [mc "Ignore space change"] \
-command changeignorespace -variable ignorespace
 pack .bleft.mid.ignspace -side left -padx 5
-- 
2.6.3.659.gfdd8f28

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 0/2] gitk spacing/sizing tuning for HiDPI

[Giuseppe Bilotta]

On Mon, Dec 7, 2015 at 5:09 AM, Eric Sunshine  wrote:
>
> Both patches are missing your Signed-off-by:.

Doh sorry, resending now.


-- 
Giuseppe "Oblomov" Bilotta
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Post-receive hook for "git pull"

[Stefan Monnier]

I have a system here where it can be quite common to have thousands of
branches in the remote repository, and where I'd like to update some
local state according to the appearance of new branches (or updates of
pre-existing ones).

Currently, I use a "git for-each-ref" after pulling and then check (for
each one of those refs) if an update is warranted, but this can get slow
with that many branches.  Is there some way to get something like the
post-receive hook to be run for "git pull", so that the script gets told
directly which (remote tracking) branches have been modified/created?


Stefan

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: What's cooking in git.git (Dec 2015, #01; Tue, 1)

[Patrick Steinhardt]

On Wed, Dec 02, 2015 at 05:31:14PM -0500, Jeff King wrote:
> On Wed, Dec 02, 2015 at 02:11:32PM -0800, Junio C Hamano wrote:
[snip]
> > "--keep-empty" has always been about keeping an originally empty
> > commit, not a commit that becomes empty because of rebasing
> > (i.e. what has already been applied to the updated base).  The
> > documentation, if it leads to any other interpretation, needs to be
> > fixed.
> > 
> > Besides, if "--keep-empty" were to mean "keep redundant ones that
> > are already in the updated base", the patch must do a lot more,
> > e.g. stop filtering with git-cherry patch equivalence.
> > 
> > I'm inclined to eject this topic.
> 
> That was my thinking too (and I notice it didn't get any review from
> anybody else).
[snip]

Well, I kind of agree. The cherry-pick command has both
--allow-empty and --keep-redundant flags, where the second one is
the kind of behavior I want to achieve in my case. As an
alternative to the proposed change to `--keep-empty` I could
instead introduce a new flag `--keep-redundant-commits` to `git
rebase` which would then pass the flag through to the
cherry-pick.

Any opinions on this?

Patrick


signature.asc
Description: Digital signature

rebase has no --verify-signatures

[Alexander 'z33ky' Hirsch]

Hi,

The git merge command has a --verify-signatures flag, which, when set, checks 
that the commits to be merged have trusted GPG signatures. git pull also knows 
this flag and forwards it to the merge command.

However, doing a git pull --rebase --verify-signatures silently ignores it, 
since rebase has no --verify-signatures flag.

Is there any technical reason why rebase should not have a --verify-signatures 
flag? I have written a patch to git-rebase--am which enables it to do such a 
check. If there is no reason not to include it I'd add documentation and a test 
and submit it.

Otherwise I think git pull should warn, or even die with an error, if both 
--rebase and --verify-signatures are passed.

Regards,
Alexander Hirsch
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

git branch in Bash subshell "$(git branch -a)" including ls output as part of return?

[Alex Jones]

Hello Folks,

I am Running OSX 10.10.5 Yosemite along with git 2.6.3 installed via
homebrew package manager.

I recently stumbled across the following bug in some scripting I was
doing. "git branch -a --list" and "git branch -a" seem to include the
output of an "ls" command if executed as part of a subshell in a bash
script. I can't speculate to the reason.

Script goal: Delete all branches aside from master in the repository
Example outputs from several commands  repo:

ls output:

ajonespro:Deploy_Script ajones$ ls -l
total 0
drwxr-xr-x  11 ajones  wheel  374 Dec  7 10:50 AppDeploy
drwxr-xr-x  11 ajones  wheel  374 Dec  7 11:15 WebDeploy


git branch -a output:

ajonespro:Deploy_Script ajones$ git branch -a

* DWH_concurrent_api
  Email_No_Error_If_No_Old_Version
  IT/configs_in_app_support
  PHP_Build_Repo
  master
  remotes/origin/DWH_concurrent_api
  remotes/origin/Email_No_Error_If_No_Old_Version
  remotes/origin/IT/configs_in_app_support
  remotes/origin/PHP_Build_Repo
  remotes/origin/master

echo $(git branch -a) output:

ajonespro:Deploy_Script ajones$ echo $(git branch -a)
AppDeploy WebDeploy DWH_concurrent_api
Email_No_Error_If_No_Old_Version IT/configs_in_app_support
PHP_Build_Repo master remotes/origin/DWH_concurrent_api
remotes/origin/Email_No_Error_If_No_Old_Version
remotes/origin/IT/configs_in_app_support remotes/origin/PHP_Build_Repo
remotes/origin/master

While it might be hard to see from that output, The first two
"branches" in the subshell's output are actually the directories
contained within the repo. If I place a file at the root it includes
that in the branch list as well. Since my test file "test.txt" (not
shown in example) and "WebDeploy" are sorted before all the branches,
I suspect some output buffer is being accidentally being written to.

Has any experienced this before, and can anyone reproduce it on a
different configuration? I wouldn't be surprised if it was some weird
bug with Apple's included terminal, but since I've only observed it
with git branch, I thought I'd try contact git maintainers first.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: git branch in Bash subshell "$(git branch -a)" including ls output as part of return?

[Charles Bailey]

On Mon, Dec 07, 2015 at 11:52:28AM -0500, Alex Jones wrote:
> git branch -a output:
> 
> ajonespro:Deploy_Script ajones$ git branch -a
> 
> * DWH_concurrent_api
>   Email_No_Error_If_No_Old_Version
>   IT/configs_in_app_support
>   PHP_Build_Repo
>   master
>   remotes/origin/DWH_concurrent_api
>   remotes/origin/Email_No_Error_If_No_Old_Version
>   remotes/origin/IT/configs_in_app_support
>   remotes/origin/PHP_Build_Repo
>   remotes/origin/master
> 
> echo $(git branch -a) output:
> 
> ajonespro:Deploy_Script ajones$ echo $(git branch -a)
> AppDeploy WebDeploy DWH_concurrent_api
> Email_No_Error_If_No_Old_Version IT/configs_in_app_support
> PHP_Build_Repo master remotes/origin/DWH_concurrent_api
> remotes/origin/Email_No_Error_If_No_Old_Version
> remotes/origin/IT/configs_in_app_support remotes/origin/PHP_Build_Repo
> remotes/origin/master
> 
> While it might be hard to see from that output, The first two
> "branches" in the subshell's output are actually the directories
> contained within the repo.

Looking at the two outputs, you are seeing the shell's glob expansion of
the '*' current branch marker. You probably want to quote the command
expansion to prevent this:

echo "$(git branch -a)"
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v7] ls-files: Add eol diagnostics

[Torsten Bögershausen]

When working in a cross-platform environment, a user wants to
check if text files are stored normalized in the repository and if
.gitattributes are set appropriately.

Make it possible to let Git show the line endings in the index and
in the working tree and the effective text/eol attributes.

The end of line ("eolinfo") are shown like this:
"binary"   binary file
"text-no-eol"  text file without any EOL
"text-lf"  text file with LF
"text-crlf"text file with CRLF
"text-crlf-lf" text file with mixed line endings.

The effective text/eol attribute is one of these:
"", "-text", "text", "text=auto", "eol=lf", "eol=crlf"

git ls-files --eol gives an output like this:

i/text-no-eol   w/text-no-eol   attr/text=auto t/t5100/empty
i/binaryw/binaryattr/-text t/test-binary-2.png
i/text-lf   w/text-lf   attr/eol=lft/t5100/rfc2047-info-0007
i/text-lf   w/text-crlf attr/eol=crlf  doit.bat
i/text-crlf-lf  w/text-crlf-lf  attr/  locale/XX.po

Note that the output is meant to be human-readable and may change.

Helped-By: Eric Sunshine 
Signed-off-by: Torsten Bögershausen 
---
Changes since v6:
- Fixed potential memory leak in convert.c, when strbuf_read_file()
  fails.
- t0027:
  Cleanups (empty lines, egrep, un-needed quoting)
  test_when_finished 'rm e expect actual' 
  There doesn't seem to be 100% consistency when and how to remove files.
  (I think if we create files, we should be able to remove them:
  use "rm" rather than "rm -f")
  Add comment about the "last test case", which removes file to run
  'git ls-files -d"

 Documentation/git-ls-files.txt |  22 +
 builtin/ls-files.c |  19 
 convert.c  |  85 
 convert.h  |   3 ++
 t/t0027-auto-crlf.sh   | 108 -
 5 files changed, 226 insertions(+), 11 deletions(-)

diff --git a/Documentation/git-ls-files.txt b/Documentation/git-ls-files.txt
index e26f01f..8f29c99 100644
--- a/Documentation/git-ls-files.txt
+++ b/Documentation/git-ls-files.txt
@@ -12,6 +12,7 @@ SYNOPSIS
 'git ls-files' [-z] [-t] [-v]

(--[cached|deleted|others|ignored|stage|unmerged|killed|modified])*
(-[c|d|o|i|s|u|k|m])*
+   [--eol]
[-x |--exclude=]
[-X |--exclude-from=]
[--exclude-per-directory=]
@@ -147,6 +148,18 @@ a space) at the start of each line:
possible for manual inspection; the exact format may change at
any time.
 
+--eol::
+   Show line endings ("eolinfo") and the text/eol attributes 
("texteolattr") of files.
+   "eolinfo" is the file content identification used by Git when
+   the "text" attribute is "auto", or core.autocrlf != false.
+
+   "eolinfo" is either "" (when the the info is not available"), or one of 
"binary",
+   "text-no-eol", "text-lf", "text-crlf" or "text-crlf-lf".
+   The "texteolattr" can be "", "-text", "text", "text=auto", "eol=lf", 
"eol=crlf".
+
+   Both the content in the index ("i/") and the content in the working 
tree ("w/")
+   are shown for regular files, followed by the "texteolattr ("attr/").
+
 \--::
Do not interpret any more arguments as options.
 
@@ -161,6 +174,15 @@ which case it outputs:
 
 [ ]   
 
+'git ls-files --eol' will show
+   i/ w/ attr/ 
+
+'git ls-files --eol -o' will show
+   i/  w/ attr/ 
+
+'git ls-files --eol -s' will show
+[ ]   i/ w/ attr/ 
+
 'git ls-files --unmerged' and 'git ls-files --stage' can be used to examine
 detailed information on unmerged paths.
 
diff --git a/builtin/ls-files.c b/builtin/ls-files.c
index b6a7cb0..ef892bc 100644
--- a/builtin/ls-files.c
+++ b/builtin/ls-files.c
@@ -27,6 +27,7 @@ static int show_killed;
 static int show_valid_bit;
 static int line_terminator = '\n';
 static int debug_mode;
+static int show_eol;
 
 static const char *prefix;
 static int max_prefix_len;
@@ -47,6 +48,21 @@ static const char *tag_modified = "";
 static const char *tag_skip_worktree = "";
 static const char *tag_resolve_undo = "";
 
+static void write_eolinfo(const struct cache_entry *ce, const char *path)
+{
+   struct stat st;
+   const char *i_txt = "";
+   const char *w_txt = "";
+   if (!show_eol)
+   return;
+   if (ce && S_ISREG(ce->ce_mode))
+   i_txt = get_cached_convert_stats_ascii(ce->name);
+   if (!lstat(path, ) && (S_ISREG(st.st_mode)))
+   w_txt = get_wt_convert_stats_ascii(path);
+   printf("i/%-13s w/%-13s attr/%-9s ", i_txt, w_txt,
+get_convert_attr_ascii(path));
+}
+
 static void write_name(const char *name)
 {
/*
@@ -68,6 +84,7 @@ static void show_dir_entry(const char *tag, struct dir_entry 
*ent)
return;
 
fputs(tag, stdout);
+   write_eolinfo(NULL, ent->name);
   

Hello Friend,

[Nicholas Andrew]

Please confirm receipt of my previous mail? When and What time can I call you?
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH/RFC 07/10] ref-filter: introduce align_atom_parser()

[Karthik Nayak]

On Thu, Dec 3, 2015 at 2:53 AM, Eric Sunshine  wrote:
> On Wed, Nov 11, 2015 at 2:44 PM, Karthik Nayak  wrote:
>> Introduce align_atom_parser() which will parse 'align' atoms and store
>> the required width and position into the 'used_atom' structure. While
>> we're here, add support for the usage of 'width=' and 'position=' when
>> using the 'align' atom (e.g. %(align:position=middle,width=30)).
>
> This patch is doing too much by both moving code around and modifying
> that code (somewhat dramatically), thus it is difficult for reviewers
> to compare the old and new behaviors. It deserves to be split apart
> into at least two patches. First, the code movement patch which
> introduces align_atom_parser() (and possibly get_align_position())
> without any behavior or logical change; then the patch which changes
> behavior to recognize the spelled-out forms "width=" and "position=".
> You may even want to spilt it into more patches, for instance by doing
> the get_align_position() extraction in its own patch.
>

I split it into two separate patches:
1. add align_atom_parser()
2. introduce "width=" and "position=" prefixes.

I think now it seems easier to follow.

>> Add documentation and modify the existing tests in t6302 to reflect
>> the same.
>>
>> Signed-off-by: Karthik Nayak 
>> ---
>> diff --git a/Documentation/git-for-each-ref.txt 
>> b/Documentation/git-for-each-ref.txt
>> @@ -129,14 +129,16 @@ color::
>>  align::
>> Left-, middle-, or right-align the content between
>> -   %(align:...) and %(end). The "align:" is followed by ``
>> -   and `` in any order separated by a comma, where the
>> -   `` is either left, right or middle, default being
>> -   left and `` is the total length of the content with
>> -   alignment. If the contents length is more than the width then
>> -   no alignment is performed. If used with '--quote' everything
>> -   in between %(align:...) and %(end) is quoted, but if nested
>> -   then only the topmost level performs quoting.
>> +   %(align:...) and %(end). The "align:" is followed by
>> +   `width=` and `position=` in any order
>> +   separated by a comma, where the `` is either left,
>> +   right or middle, default being left and `` is the total
>> +   length of the content with alignment. The prefix for the
>> +   arguments is not mandatory. If the contents length is more
>
> This paragraph is so bulky that it's very easy to overlook the bit
> about the "prefix for the arguments" being optional, and it's not
> necessarily even clear to the casual reader what that means. It might,
> therefore, be a good idea to spell it out explicitly. For instance,
> you might say something like:
>
> For brevity, the "width=" and/or "position=" prefixes may be
> omitted, and bare  and  used instead.
> For instance, `%(align:,)`.
>

Added this in.

>> +   than the width then no alignment is performed. If used with
>> +   '--quote' everything in between %(align:...) and %(end) is
>> +   quoted, but if nested then only the topmost level performs
>> +   quoting.
>> diff --git a/ref-filter.c b/ref-filter.c
>> @@ -63,6 +69,61 @@ void color_atom_parser(struct used_atom *atom)
>> +static align_type get_align_position(const char *type)
>
> Taken in context of the callers, this isn't a great function name, as
> it implies that it is retrieving some value, when in fact it is
> parsing the input argument. A better name might be
> parse_align_position().
>
> Likewise, 'type' isn't necessarily a great argument name. You might
> instead call it 'pos' or even just short and sweet 's'.
>

Will change.

>> +{
>> +   if (!strcmp(type, "right"))
>> +   return ALIGN_RIGHT;
>> +   else if (!strcmp(type, "middle"))
>> +   return ALIGN_MIDDLE;
>> +   else if (!strcmp(type, "left"))
>> +   return ALIGN_LEFT;
>> +   return -1;
>> +}
>> +
>> +void align_atom_parser(struct used_atom *atom)
>> +{
>> +   struct align *align = >u.align;
>> +   const char *buf = NULL;
>> +   struct strbuf **s, **to_free;
>> +   int width = -1;
>> +
>> +   match_atom_name(atom->str, "align", );
>> +   if (!buf)
>> +   die(_("expected format: %%(align:,)"));
>
> Is this still the way you want this error message to appear, or should
> it show the long-form of the arguments? (I don't care strongly.)
>

I think it still holds good, not too keen on changing it either.

>> +   s = to_free = strbuf_split_str_without_term(buf, ',', 0);
>> +
>> +   /*  By default align to ALGIN_LEFT */
>
> What is ALGIN? Regardless of the answer, this comment is not
> particularly useful since it merely repeats what the code itself
> already states clearly.
>

will do.

>> +   align->position = ALIGN_LEFT;
>> +
>> +   while (*s) {
>> +   int position;
>> +   buf = s[0]->buf;

Re: git branch in Bash subshell "$(git branch -a)" including ls output as part of return?

[Charles Bailey]

On Mon, Dec 07, 2015 at 04:58:10PM +, Charles Bailey wrote:
> 
> Looking at the two outputs, you are seeing the shell's glob expansion of
> the '*' current branch marker. You probably want to quote the command
> expansion to prevent this:
> 
> echo "$(git branch -a)"

Pressing send has, of course, caused me to think further. You probably
don't want to parse the output of a "porcelain" command such as "git
branch" at all, but instead look at using something like "git
for-each-ref", perhaps with the --format=%(refname) option, grepping out
master and iterating through the rest.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC/PATCH 1/8] update-index: add untracked cache notifications

[Duy Nguyen]

On Mon, Dec 7, 2015 at 10:08 AM, Christian Couder
 wrote:
> On Wed, Dec 2, 2015 at 8:16 PM, Duy Nguyen  wrote:
>>> --- a/builtin/update-index.c
>>> +++ b/builtin/update-index.c
>>> @@ -121,7 +121,7 @@ static int test_if_untracked_cache_is_supported(void)
>>> if (!mkdtemp(mtime_dir.buf))
>>> die_errno("Could not make temporary directory");
>>>
>>> -   fprintf(stderr, _("Testing "));
>>> +   fprintf(stderr, _("Testing mtime in '%s' "), xgetcwd());
>>
>> We probably should respect --verbose. I know I violated it in the first 
>> place.
>
> The verbose help is:
>
> --verbose report actions to standard output
>
> so yeah, it is not respected first because the output is on by
> default, and second because the output is on stderr instead of stdout.
> Anyway it can be a separate patch or patch series to make it respect
> one or both of these points.
>
> I am not very much interested in doing it myself as I think it's
> interesting to have the output by default especially if the above
> patch is applied. But if people agree that it would be a good thing, I
> will do it.

Then we can leave it out. Not important (until people actually complain).
-- 
Duy
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 0/2] format-patch: introduce option to suppress commit hashes

[Junio C Hamano]

"brian m. carlson"  writes:

> The hash of the source file isn't generally as much of a problem,
> because the patch tends to change, even incidentally (line numbers and
> such), when the hash of the file changes.  It's also something that we
> have in our history, whereas the temporary branch we rebased in is not.

That is exactly the kind of workflow specific reasoning that tells
you "object name of the commit that the patch was taken from is the
only thing that is undesired" that makes me wonder if the feature is
too workflow specific.  You do something on a temporary branch without
worrying about producing unnecessary object name churn, and end up
wanting not to see object names.

But I can buy that a step in the workflow to rebuild the history on
a temporary branch before going to the next step is a common thing
to have, so let's decide to accept the goal as a good thing to have,
and see how well the patched code implements, documents and tests
the advertised new feature.

Thanks.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC/PATCH 1/8] update-index: add untracked cache notifications

[Christian Couder]

On Wed, Dec 2, 2015 at 8:16 PM, Duy Nguyen  wrote:
> On Tue, Dec 1, 2015 at 9:31 PM, Christian Couder
>  wrote:
>> Doing:
>>
>>   cd /tmp
>>   git --git-dir=/git/somewhere/else/.git update-index --untracked-cache
>>
>> doesn't work how one would expect. It hardcodes "/tmp" as the directory
>> that "works" into the index, so if you use the working tree, you'll
>> never use the untracked cache.
>>
>> With this patch "git update-index --untracked-cache" tells the user in
>> which directory tests are performed and in which working directory the
>> untracked cache is allowed.
>>
>> Signed-off-by: Christian Couder 
>> ---
>>  builtin/update-index.c | 4 +++-
>>  1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/builtin/update-index.c b/builtin/update-index.c
>> index 7431938..e568acc 100644
>> --- a/builtin/update-index.c
>> +++ b/builtin/update-index.c
>> @@ -121,7 +121,7 @@ static int test_if_untracked_cache_is_supported(void)
>> if (!mkdtemp(mtime_dir.buf))
>> die_errno("Could not make temporary directory");
>>
>> -   fprintf(stderr, _("Testing "));
>> +   fprintf(stderr, _("Testing mtime in '%s' "), xgetcwd());
>
> We probably should respect --verbose. I know I violated it in the first place.

The verbose help is:

--verbose report actions to standard output

so yeah, it is not respected first because the output is on by
default, and second because the output is on stderr instead of stdout.
Anyway it can be a separate patch or patch series to make it respect
one or both of these points.

I am not very much interested in doing it myself as I think it's
interesting to have the output by default especially if the above
patch is applied. But if people agree that it would be a good thing, I
will do it.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC/PATCH 8/8] update-index: make core.untrackedCache a bool

[Christian Couder]

(Sorry I already sent a private reply to Tosten by mistake.)

On Sat, Dec 5, 2015 at 1:44 PM, Torsten Bögershausen  wrote:
> On 01.12.15 21:31, Christian Couder wrote:
>> Most features in Git can be enabled or disabled using a simple
>> bool config variable and it would be nice if untracked cache
>> behaved the same way.
>>
>> This makes --[no-|force-]untracked-cache change the value of
>> core.untrackedCache in the repo config file, to avoid making
>> those options useless and because this avoids the untracked
>> cache being disabled by a kernel change or a directory
>> change. Of course this breaks some backward compatibility,
>> but the simplification and increased useability is worth it.
>
> Some loose thinking, how the core.untrackedcache and the
> command line can work together:
>
> core.untrackedcache = unset
>   everything is as today
>   if --test-untracked-cache is used on command line,
>   the test is run, additionally the result is stored
>   in the config variable core.untrackedcache

I don't like storing the result in core.untrackedcache as it means
that --test-untracked-cache would not just test.
And I agree with Aevar that it's a good thing to be able to completely
separate testing and configuring.

> core.untrackedcache = true
>   same as --force-untracked-cache
>   if --no-untracked-cache is given on command line,
>   this has higher priority

I guess you mean that --no-untracked-cache has priority over
"core.untrackedcache = true" without removing this config variable.
This means that --no-untracked-cache would only remove the untracked
cache (UC) from the index.

But what git should then do the next time "git status" is run?
Git sees that the index does not contain any UC, but it doesn't know
that "git update-index --no-untracked-cache" has just been run. It
might be "git config core.untrackedcache true" that has been run last.

If "git config core.untrackedcache true" has been run last, it would
be logical to add the UC to the index and use it.
If "git update-index --no-untracked-cache" has been run last, it would
be logical to not add the UC.

> core.untrackedcache = false
>   same as --no-untracked-cache
>   if --force-untracked-cache is given on command line,
>   this has higher priority

Then the same kind of problem happens as above. There is no clear
solution about what "git status" should do when the state of the index
conflicts with the value of core.untrackedcache.

> Does this support the workflow described by Ævar ?

I don't think so. I think that when we set "core.untrackedcache =
true" we want the UC to be added to the index and used as much as
possible, because we know that our OS/filesystem supports it.

This means that using "git update-index --no-untracked-cache" when
"core.untrackedcache = true" is set can only have two possible
outcomes. It could either just die saying that "core.untrackedcache"
is true, or it could just unset "core.untrackedcache" or set it to
false (which might mean the same thing).
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Problems installing git-for-windows

[Peter Toye]

Hi,

I've just downloaded git-for-windows. It uninstalled an earlier
version that I had (about 5 years old) and I got some problems.

1)  It wanted to install into my User directory instead of the
Program Files directory, which is the best place for all programs.
The installer told me that this had access right problems, so I
installed it into another directory. It seemed to go OK.

2)  However,  the  icons  on  the desktop pointed to the place that my
previous installation had been ( C:\Program Files (x86)\Git ) - that's
where I'd have hoped that the new installation would go!

3)  I  uninstalled it to see if a cleaner installatoin would work, and
the  GIT-GUI icon didn't disappear  from the desktop - minor bug I
suspect.
 
 
Regards,
 
Peter
 mailto:g...@ptoye.com
 www.ptoye.com 

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: What's cooking in git.git (Dec 2015, #01; Tue, 1)

[Junio C Hamano]

Patrick Steinhardt  writes:

> On Wed, Dec 02, 2015 at 05:31:14PM -0500, Jeff King wrote:
>> On Wed, Dec 02, 2015 at 02:11:32PM -0800, Junio C Hamano wrote:
> [snip]
>> > "--keep-empty" has always been about keeping an originally empty
>> > commit, not a commit that becomes empty because of rebasing
>> > (i.e. what has already been applied to the updated base).  The
>> > documentation, if it leads to any other interpretation, needs to be
>> > fixed.
>> > 
>> > Besides, if "--keep-empty" were to mean "keep redundant ones that
>> > are already in the updated base", the patch must do a lot more,
>> > e.g. stop filtering with git-cherry patch equivalence.
>> > 
>> > I'm inclined to eject this topic.
>> 
>> That was my thinking too (and I notice it didn't get any review from
>> anybody else).
> [snip]
>
> Well, I kind of agree. The cherry-pick command has both
> --allow-empty and --keep-redundant flags, where the second one is
> the kind of behavior I want to achieve in my case. As an
> alternative to the proposed change to `--keep-empty` I could
> instead introduce a new flag `--keep-redundant-commits` to `git
> rebase` which would then pass the flag through to the
> cherry-pick.
>
> Any opinions on this?

Honestly, I am not interested if that is only about passing that
option and doing nothing else.

Imagine that you have two changes from the branch being rebased
already in the updated base, one was accepted verbatim, while the
other one was accepted with a slight tweak.  Perhaps there were some
conflicts in the context, or something.

When you run your rebase, the former will not even be considered
because command will notice, via patch equivalence, that you do not
need it, even before it attempts to replay it.  But not the latter.
The command will attempt to replay it, and only in this step it may
notice, by seeing that the result becomes a no-op change, that the
change has already been included in the updated base.

I cannot quite see how adding "--keep-redundant-commits" to the
command would help anybody by allowing the latter in the history but
not the former.  That is primarily because you can imagine a future
in which the patch equivalence determination becomes smarter.  With
or without "--keep-redundant-commits", both of these two changes
would not appear in the resulting history when that happens.

The "--keep-redundant" option to "cherry-pick" makes sense, as the
user will be the one who is deciding which commit to replay on top
of a new base.  If the user fed a commit that is already in the new
base, and the command is run with the option, that is a deliberate
request to leave a no-op cruft.

We cannot say the same thing for "rebase", as it tries to avoid
redundant cruft, and it cannot do as good a job as humans in doing
so.  The "--keep-redundant-commits" option will become a way to make
a distinction between what got dropped by the command automatically
and what didn't get dropped because the command did not look deeply
enough.  That distinction is not at all useful, as that can change
as the tool improves.

A "--keep-redundant-commits" to "rebase" that also disables the
patch equivalence filtering (not just keeping empty crufts in the
resulting history) might make sense, but I do not see myself (or
anybody sane) using it.  "In what situation would such a feature be
useful?" is a question I cannot answer offhand.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/2] format-patch: add an option to suppress commit hash

[Junio C Hamano]

"brian m. carlson"  writes:

> Oftentimes, patches created by git format-patch will be stored in
> version control or compared with diff.  In these cases, two otherwise
> identical patches can have different commit hashes, leading to diff
> noise.  Teach git format-patch a --no-hash option that instead produces
> an all-zero hash to avoid this diff noise.
>
> Signed-off-by: brian m. carlson 
> ---
>  Documentation/git-format-patch.txt | 4 
>  builtin/log.c  | 5 +
>  log-tree.c | 3 ++-
>  revision.h | 1 +
>  t/t4014-format-patch.sh| 6 ++
>  5 files changed, 18 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/git-format-patch.txt 
> b/Documentation/git-format-patch.txt
> index 40356491..1266f135 100644
> --- a/Documentation/git-format-patch.txt
> +++ b/Documentation/git-format-patch.txt
> @@ -256,6 +256,10 @@ you can use `--suffix=-patch` to get 
> `0001-description-of-my-change-patch`.
>   using this option cannot be applied properly, but they are
>   still useful for code review.
>  
> +--no-hash::
> +  Output an all-zero hash in each patch's From header instead
> +  of the hash of the commit.
> +

Two (big) problems with the option name.

 - "--no-something" would mislead people to think you are removing
   something, not replacing it with something else.  This option
   does the latter (i.e. the first line of your output still has
   40-hex; it's just it no longer has a useful 40-hex).

 - There are many places we use hexadecimal strings in format-patch
   output and you are not removing or replacing all of them, only
   the commit object name on the fake "From " line.  Saying "hash"
   would mislead readers.

> +test_expect_success 'format-patch --no-hash' '
> + git format-patch --no-hash --stdout v2..v1 >patch2 &&
> + cnt=$(egrep "^From 0+ Mon Sep 17 00:00:00 2001" patch2 | wc -l) &&

Don't test "any number of '0'"; test 40 '0's.  This is because the
line format was designed to be usable by things like /etc/magic to
detect format-patch output, and we want to notice if/when we break
that aspect of our output format.

> + test $cnt = 3
> +'
> +
>  test_done

Thanks.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Problems installing git-for-windows

[Johannes Schindelin]

Hi Peter,

On Mon, 7 Dec 2015, Peter Toye wrote:

> Thanks - sorry I didn't report the version number - it was 2.6.3 as you
> suggested. I didn't realise that development was so active. Do yo know
> when the next version will be stable or should I go for an earlier
> version?

I am "side-tracked" with a couple of tickets but plan to do a 2.6.3(2)
soon unless Junio releases a 2.6.4 first.

If you need a fixed installer *real fast*, you can follow these easy
steps: https://github.com/git-for-windows/git/wiki/Making-an-installer

Ciao,
Johannes
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] clean: new option --exclude-from

[Jeff King]

On Sat, Dec 05, 2015 at 07:51:03PM -0800, Junio C Hamano wrote:

> In any case, what we've been discussing may be an interesting and
> potentially important tangent, but it still is a tangent while
> evaluating the patch in question.  I do not think I'd be using the
> new "--exclude-from=" option to "clean" (simply because I do
> not think I've ever used the existing "-e" option to it unless I am
> merely testing the command to make sure it works as advertised)
> myself, but I do not immediately see how it would hurt us in the
> future to add it now.  So...

I think that is OK. The reason I brought it up was "let's stop and make
sure we don't want to go this other route before we add more potentially
redundant options that we cannot get rid of later". But based on this
discussion it is not at all clear that "clean --exclude-from" would
become redundant, so let's cross that off as an objection.

-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/2] format-patch: add an option to suppress commit hash

[Junio C Hamano]

Junio C Hamano  writes:

>> +--no-hash::
>> +  Output an all-zero hash in each patch's From header instead
>> +  of the hash of the commit.
>> +
>
> Two (big) problems with the option name.
>
>  - "--no-something" would mislead people to think you are removing
>something, not replacing it with something else.  This option
>does the latter (i.e. the first line of your output still has
>40-hex; it's just it no longer has a useful 40-hex).
>
>  - There are many places we use hexadecimal strings in format-patch
>output and you are not removing or replacing all of them, only
>the commit object name on the fake "From " line.  Saying "hash"
>would mislead readers.

I am not good at bikeshedding, but you used 'zero_commit' elsewhere
in the code.  I think that would be a much better name--perhaps use
that consistently throughout, as the local variable in options[]
array and end-user facing option name?

>
>> +test_expect_success 'format-patch --no-hash' '
>> +git format-patch --no-hash --stdout v2..v1 >patch2 &&
>> +cnt=$(egrep "^From 0+ Mon Sep 17 00:00:00 2001" patch2 | wc -l) &&
>
> Don't test "any number of '0'"; test 40 '0's.  This is because the
> line format was designed to be usable by things like /etc/magic to
> detect format-patch output, and we want to notice if/when we break
> that aspect of our output format.
>
>> +test $cnt = 3
>> +'
>> +
>>  test_done
>
> Thanks.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Post-receive hook for "git pull"

[Junio C Hamano]

Stefan Monnier  writes:

> I have a system here where it can be quite common to have thousands of
> branches in the remote repository, and where I'd like to update some
> local state according to the appearance of new branches (or updates of
> pre-existing ones).
>
> Currently, I use a "git for-each-ref" after pulling and then check (for
> each one of those refs) if an update is warranted, but this can get slow
> with that many branches.  Is there some way to get something like the
> post-receive hook to be run for "git pull", so that the script gets told
> directly which (remote tracking) branches have been modified/created?

I do not think there is.  But you could easily script along the
lines of...

#!/bin/sh
git for-each-ref | sort >prestate
git pull "$@"
git for-each-ref | sort >poststate
comm -12 prestate poststate

... or something like that, no?

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Multiple fetches when unshallowing a shallow clone

[Jason Paller-Rzepka]

I should point out that I never saw a "deepen 0" line.  Rather, I saw
git send "option depth 0" to the remote helper.
Duy, you mentioned that depth=0 means "do not change depth".  I assume
that means the server should use exactly the shallows that the client
sent, and it does not need to traverse the tree or modify the shallow
or unshallow sets at all.  Right?
Duy, you also mentioned that "those lines should be rejected any way".
You just mean that a "deepen 0" line should be rejected, right? And
that's because the right way to tell git-upload-pack not to change the
depth is to omit the "deepen" line after the "shallow" lines, so
there's never a need to send "deepen 0"?

Thanks!

On Sun, Dec 6, 2015 at 5:46 AM, Duy Nguyen  wrote:
> On Sun, Dec 6, 2015 at 8:01 AM, Jeff King  wrote:
>> On Sun, Dec 06, 2015 at 01:37:18AM -0500, Jeff King wrote:
>>
>>> And indeed, replacing the logic with what I wrote does make the backfill
>>> go away in my test case. But it's so far from what is there that I feel
>>> like I must be missing something.
>>
>> I think one thing I was missing is that we need to just grab the
>> _object_, but we need to realize that the ref needs updating[1]. So we
>> cannot skip backfill of any tag that we do not already have, even if we
>> already have the tag object.
>
> It's probably worth adding a few comment lines about this. I did
> search back commit history but did not get this.
>
>> Which made me wonder why this:
>>
>>   git init parent &&
>>   git -C parent commit --allow-empty -m one &&
>>   git clone parent child &&
>>   git -C parent commit --allow-empty -m two &&
>>   git -C parent tag -m mytag foo &&
>>   git -C parent commit --allow-empty -m three &&
>>   git -C child fetch
>>
>> does not appear to need to backfill to pick up refs/tags/foo. But it
>> does. It's just that it hits the quickfetch() code path and does not
>> have to ask the other side for a pack. And that explains why it does hit
>> in the --shallow case: we explicitly disable quickfetch in such cases.
>>
>> For the unshallow case, of course we could use it (but only for the
>> second, backfill fetch). Something like this seems to work for me:
>>
>> diff --git a/builtin/fetch.c b/builtin/fetch.c
>> index ed84963..b33b90f 100644
>> --- a/builtin/fetch.c
>> +++ b/builtin/fetch.c
>> @@ -881,6 +881,8 @@ static void backfill_tags(struct transport *transport, 
>> struct ref *ref_map)
>>
>> transport_set_option(transport, TRANS_OPT_FOLLOWTAGS, NULL);
>> transport_set_option(transport, TRANS_OPT_DEPTH, "0");
>> +   if (unshallow)
>> +   depth = NULL;
>> fetch_refs(transport, ref_map);
>>
>> if (gsecondary) {
>>
>> But I admit I am not at all confident that it doesn't cause other
>> problems, or that it covers all cases. Even in a shallow repo, we should
>> be able to quickfetch individual tags, shouldn't we?
>
> Yes. depth is only non-NULL when you pass --depth (or --unshallow).
> quickfetch should happen when you fetch without those options.
>
>> I wonder if we could just always set "depth = NULL" here.
>
> --unshallow is essentially --depth=, so I don't see why
> --unshallow should be singled out here. We probably want to restore
> depth back (or pass a flag to explicitly ignore the "depth" exception
> in quickfetch). For multiple fetches, we spawn new commands so depth
> being NULL does not harm. Just in case somebody tries to fetch a
> couple more times in the same process in future.
> --
> Duy
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v7] ls-files: Add eol diagnostics

[Philip Oakley]

From: "Torsten Bögershausen" 

When working in a cross-platform environment, a user wants to
check if text files are stored normalized in the repository and if
.gitattributes are set appropriately.



The need for this came up again on the Git Users list 
(https://groups.google.com/d/msg/git-users/jC-mngwVYo4/Sr4JXgpGJVYJ). It 
will definitely be useful in mixed environments wher users can get rather 
confused.


I've added a couple of bikeshed comments on some of the wider issues this 
bumps into.



Make it possible to let Git show the line endings in the index and
in the working tree and the effective text/eol attributes.

The end of line ("eolinfo") are shown like this:
"binary"   binary file
"text-no-eol"  text file without any EOL
"text-lf"  text file with LF
"text-crlf"text file with CRLF
"text-crlf-lf" text file with mixed line endings.


 This "text-crlf-lf" can easily occur for eol=crlf files which 
have merge conflict markers, which are always eol=lf. It may be that the 
conflict markers should use the eol setting that is in place for the file 
being merged. e.g. 
http://stackoverflow.com/questions/17832616/make-git-use-crlf-on-its-head-merge-lines 
(which links to https://github.com/git/git/blob/master/xdiff/xmerge.c#L173, 
and 3 other places in fill_conflict_hunk). The issue of how to mark EOL 
conflicts in-text is tricky as it's a non-obvious white space issue - it 
doesn't fit well with the normal >>> === <<< conflict markers.




The effective text/eol attribute is one of these:
"", "-text", "text", "text=auto", "eol=lf", "eol=crlf"


 the "-text" attribute isn't explained in the gitattributes(5) 
page, except by implication  from one of the examples.


git ls-files --eol gives an output like this:

i/text-no-eol   w/text-no-eol   attr/text=auto t/t5100/empty
i/binaryw/binaryattr/-text t/test-binary-2.png
i/text-lf   w/text-lf   attr/eol=lft/t5100/rfc2047-info-0007
i/text-lf   w/text-crlf attr/eol=crlf  doit.bat
i/text-crlf-lf  w/text-crlf-lf  attr/  locale/XX.po

Note that the output is meant to be human-readable and may change.

Helped-By: Eric Sunshine 
Signed-off-by: Torsten Bögershausen 
---
Changes since v6:
- Fixed potential memory leak in convert.c, when strbuf_read_file()
 fails.
- t0027:
 Cleanups (empty lines, egrep, un-needed quoting)
 test_when_finished 'rm e expect actual'
 There doesn't seem to be 100% consistency when and how to remove files.
 (I think if we create files, we should be able to remove them:
 use "rm" rather than "rm -f")
 Add comment about the "last test case", which removes file to run
 'git ls-files -d"

Documentation/git-ls-files.txt |  22 +
builtin/ls-files.c |  19 
convert.c  |  85 
convert.h  |   3 ++
t/t0027-auto-crlf.sh   | 108 
-

5 files changed, 226 insertions(+), 11 deletions(-)

diff --git a/Documentation/git-ls-files.txt 
b/Documentation/git-ls-files.txt

index e26f01f..8f29c99 100644
--- a/Documentation/git-ls-files.txt
+++ b/Documentation/git-ls-files.txt
@@ -12,6 +12,7 @@ SYNOPSIS
'git ls-files' [-z] [-t] [-v]
 (--[cached|deleted|others|ignored|stage|unmerged|killed|modified])*
 (-[c|d|o|i|s|u|k|m])*
+ [--eol]
 [-x |--exclude=]
 [-X |--exclude-from=]
 [--exclude-per-directory=]
@@ -147,6 +148,18 @@ a space) at the start of each line:
 possible for manual inspection; the exact format may change at
 any time.

+--eol::
+ Show line endings ("eolinfo") and the text/eol attributes 
("texteolattr") of files.

+ "eolinfo" is the file content identification used by Git when
+ the "text" attribute is "auto", or core.autocrlf != false.
+
+ "eolinfo" is either "" (when the the info is not available"), or one of 
"binary",

+ "text-no-eol", "text-lf", "text-crlf" or "text-crlf-lf".
+ The "texteolattr" can be "", "-text", "text", "text=auto", "eol=lf", 
"eol=crlf".


perhaps add  'see also linkgit:gitattributes[5] .' though it may be too 
tangential.



+
+ Both the content in the index ("i/") and the content in the working tree 
("w/")

+ are shown for regular files, followed by the "texteolattr ("attr/").
+
\--::
 Do not interpret any more arguments as options.

@@ -161,6 +174,15 @@ which case it outputs:

[ ]   

+'git ls-files --eol' will show
+ i/ w/ attr/ 
+
+'git ls-files --eol -o' will show
+ i/  w/ attr/ 
+
+'git ls-files --eol -s' will show
+[ ]   i/ w/ attr/ 


+
'git ls-files --unmerged' and 'git ls-files --stage' can be used to 
examine

detailed information on unmerged paths.

diff --git a/builtin/ls-files.c b/builtin/ls-files.c
index b6a7cb0..ef892bc 100644
--- a/builtin/ls-files.c
+++ b/builtin/ls-files.c
@@ -27,6 +27,7 @@ static int show_killed;
static int show_valid_bit;
static int line_terminator = '\n';
static int debug_mode;
+static int show_eol;

static const 

Re: Git 2.3.7 hangs on fetch but not clone

[Jeff King]

On Sun, Dec 06, 2015 at 05:12:12PM -0500, Randall S. Becker wrote:

> I have some strange behaviour I am trying to diagnose on the NonStop port of
> git 2.3.7. The situation is we have a *LARGE* cloned repository with some
> local modifications of openssl, which we are trying to clone again for a
> Jenkins build. The command:
>   git clone /local/openssl openssl

Because the two repositories are on the same local filesystem, git will
skip the usual pack transfer and just copy or hardlink the object files.

You can add "--no-local" to make it more like the fetch that is failing
(I suspect it will then fail, too, but if it doesn't, that would be an
interesting data point).

>   remote: Counting objects: 113436, done.
>   remote: Compressing objects: 100% (23462/23462), done.
> then hangs forever without creating any files in the working directory.
> There are also no files or directories modified since the init operation.
> Processes left around, and without consuming resources, are:
> 1493172290 2030043151 - 15:58:29   00:15 git pack-objects --revs --thin
> --stdout --progress --delta-base-offset --include-tag
> 452984908  402653262 - 15:58:29   00:00 git -c core.askpass=true fetch
> --verbose --progress /local/git/openssl +refs/heads/*:refs/remotes/origin/*
> 402653262 1694498903 - 15:58:28   00:00 git -c core.askpass=true fetch
> --verbose --progress /local/git/openssl +refs/heads/*:refs/remotes/origin/*
> 2030043151  402653262 - 15:58:29   00:00 git-upload-pack
> /local/git/openssl

What are the processes doing?

Upload-pack on the "server" side spawns pack-objects, writes the set of
wanted objects to its stdin, and then waits for it to produce pack data
to stdout (which it then multiplexes back to the client). You clearly
got past the write (since pack-objects finished "Counting objects...").
I'd expect it to be waiting for input in poll() for input from the
stdout or stderr of pack-objects. It may also be blocking on write()
back to the client.

Pack-objects clearly completed delta compression, and so should be
writing out the final pack.  It doesn't look like it ever wrote the
"writing objects..." progress line (or perhaps upload-pack got stuck and
failed to relay it). It's probably blocking on write() to upload-pack.

You have two fetch processes. Presumably you build with NO_PTHREADS and
one of them is a fork()ed copy to demux the data from upload-pack. It
clearly runs at least a little while (it relays the "Counting" message).
It should either be blocking on read() from upload-pack, or perhaps
blocking on write() back to main fetch.

The main fetch hasn't yet spawned index-pack or unpack-objects, which
means it hasn't gotten enough of the pack header to do so. So it's
probably blocking on read() from the sideband demuxer process.

Do you have some equivalent of strace on your system? Or better yet, a
debugger?

> This does not happen for our smaller repositories. Any pointers on where to
> look would be appreciated.

My wild guess would be some pipe deadlock between the processes, but I
don't think we've had any of those lately. And v2.3.7 is not _too_ old
(there are some fixes for http deadlocks in v2.3.7..v2.6.3, but that
should not affect you here).

-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v7] ls-files: Add eol diagnostics

[Eric Sunshine]

On Monday, December 7, 2015, Torsten Bögershausen  wrote:
> When working in a cross-platform environment, a user wants to
> check if text files are stored normalized in the repository and if
> .gitattributes are set appropriately.[...]

A few style comments this time around...

> Helped-By: Eric Sunshine 
> Signed-off-by: Torsten Bögershausen 
> ---
> diff --git a/builtin/ls-files.c b/builtin/ls-files.c
> @@ -47,6 +48,21 @@ static const char *tag_modified = "";
> +static void write_eolinfo(const struct cache_entry *ce, const char *path)
> +{
> +   struct stat st;
> +   const char *i_txt = "";
> +   const char *w_txt = "";
> +   if (!show_eol)
> +   return;
> +   if (ce && S_ISREG(ce->ce_mode))
> +   i_txt = get_cached_convert_stats_ascii(ce->name);
> +   if (!lstat(path, ) && (S_ISREG(st.st_mode)))

Style: unnecessary parentheses around S_ISREG(), which is inconsistent
with S_ISREG() two lines above.

> +   w_txt = get_wt_convert_stats_ascii(path);
> +   printf("i/%-13s w/%-13s attr/%-9s ", i_txt, w_txt,
> +get_convert_attr_ascii(path));
> +}
> diff --git a/convert.c b/convert.c
> @@ -95,6 +100,62 @@ static int is_binary(unsigned long size, struct text_stat 
> *stats)
> +static unsigned int gather_convert_stats(const char *data, unsigned long 
> size)
> +{
> +   struct text_stat stats;
> +   if (!data || !size)
> +   return 0;
> +   gather_stats(data, size, );
> +   if (is_binary(size, ) || stats.cr != stats.crlf)
> +   return CONVERT_STAT_BITS_BIN;
> +   else if (stats.crlf && (stats.crlf == stats.lf))

Style: unnecessary parentheses around 'foo == bar'

> +   return CONVERT_STAT_BITS_TXT_CRLF;
> +   else if (stats.crlf && stats.lf)
> +   return CONVERT_STAT_BITS_TXT_CRLF | CONVERT_STAT_BITS_TXT_LF;
> +   else if (stats.lf)
> +   return CONVERT_STAT_BITS_TXT_LF;
> +   else
> +   return 0;
> +}
> +
> +static const char *gather_convert_stats_ascii(const char *data, unsigned 
> long size)
> +{
> +   unsigned int convert_stats = gather_convert_stats(data, size);
> +
> +   if (convert_stats & CONVERT_STAT_BITS_BIN)
> +   return "binary";
> +   switch (convert_stats) {
> +   case CONVERT_STAT_BITS_TXT_LF:
> +   return("text-lf");

Style: space after "return".

Also, can we lose the unnecessary noisy parentheses? (I recall
mentioning this in my first review.)

Same for "return" statements below.

> +   case CONVERT_STAT_BITS_TXT_CRLF:
> +   return("text-crlf");
> +   case CONVERT_STAT_BITS_TXT_LF | CONVERT_STAT_BITS_TXT_CRLF:
> +   return("text-crlf-lf");
> +   default:
> +   return ("text-no-eol");
> +   }
> +}
> diff --git a/t/t0027-auto-crlf.sh b/t/t0027-auto-crlf.sh
> @@ -214,6 +239,20 @@ checkout_files () {
> fi
> done
>
> +   test_expect_success "ls-files --eol $lfname ${pfx}LF.txt" "
> + test_when_finished 'rm e expect actual' &&

Style: test_when_finished is incorrectly indented with tab+spaces
rather than only tabs

> +   cat >e <<-EOF &&
> +   i/text-crlf w/$(stats_ascii $crlfname) ${src}CRLF.txt
> +   i/text-crlf-lf w/$(stats_ascii $lfmixcrlf) 
> ${src}CRLF_mix_LF.txt
> +   i/text-lf w/$(stats_ascii $lfname) ${src}LF.txt
> +   i/binary w/$(stats_ascii $lfmixcr) ${src}LF_mix_CR.txt
> +   i/binary w/$(stats_ascii $crlfnul) ${src}CRLF_nul.txt
> +   i/binary w/$(stats_ascii $crlfnul) ${src}LF_nul.txt
> +   EOF
> +   sort expect &&
> +   git ls-files --eol $src* | sed -e 's!attr/[=a-z-]*!!g' -e 's/ 
>  */ /g' | sort >actual &&
> +   test_cmp expect actual
> +   "
> @@ -480,4 +550,20 @@ checkout_filesnative  true  "lf"  LFCRLF  
> CRLF_mix_LF  LF_mix_CR
>  checkout_filesnative  false "crlf"CRLF  CRLF  CRLF 
> CRLF_mix_CR  CRLF_nul
>  checkout_filesnative  true  "crlf"CRLF  CRLF  CRLF 
> CRLF_mix_CR  CRLF_nul
>
> +

Style: unnecessary blank line

> +# Should be the last test case: remove some files from the worktree
> +# run 'git ls-files -d'

This seems kind of fragile. Might it be possible to either recreate
those files when this test finishes or instead provide a function
which creates them on-demand for tests which need them? My concern is
that there is a good chance that someone later adding tests will
overlook this comment, especially since most people just plop new
tests at the bottom of the file.

> +test_expect_success 'ls-files --eol -d' "
> +   rm  crlf_false_attr__CRLF.txt crlf_false_attr__CRLF_mix_LF.txt 
> crlf_false_attr__LF.txt .gitattributes &&

Style: too many spaces 

Re: [PATCH v2] revision.c: fix possible null pointer access

[Junio C Hamano]

Stefan Naewe  writes:

> mark_tree_uninteresting dereferences a tree pointer before checking
> if the pointer is valid. Fix that by doing the check first.
>
> Signed-off-by: Stefan Naewe 
> ---

I still have a problem with "dereferences", as "dereference" is
about computing an address and accessing memory based on the result,
and only the first half is happening here.  I can live with "The
function does a pointer arithmetic on 'tree' before it makes sure
that 'tree' is not NULL", but in any case, let's queue this as-is
for now and wait for a while to see if others can come up with a
more appropriate phrases.

Thanks.

>  revision.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/revision.c b/revision.c
> index 0fbb684..8c569cc 100644
> --- a/revision.c
> +++ b/revision.c
> @@ -135,10 +135,12 @@ static void mark_tree_contents_uninteresting(struct 
> tree *tree)
>  
>  void mark_tree_uninteresting(struct tree *tree)
>  {
> - struct object *obj = >object;
> + struct object *obj;
>  
>   if (!tree)
>   return;
> +
> + obj = >object;
>   if (obj->flags & UNINTERESTING)
>   return;
>   obj->flags |= UNINTERESTING;
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Can't get 'git archive' to work

[Jeff King]

On Sun, Dec 06, 2015 at 09:31:06PM -0800, Yuri wrote:

> Why "operation is not supported" through http and https? 'archive' is
> supposed to be the most efficient operation to get the specific snapshot of
> the repository, and it should be available without authentication.

Remote git-archive is not well-supported. It does work over git-daemon,
but the server side has to explicitly turn it on in their config. It was
never plumbed into the git-over-http protocol at all. ssh is generally
the only place it works out-of-the-box (but see below).

> In case of ssh with the key there is the strange error message about
> "core.gitProxy". What is this about? I am not using a proxy.
> [...]
> # through ssh with an active ssh key
> $ git archive --format=tar --remote=g...@github.com:yurivict/vm-to-tor.git
> Invalid command: 'git-upload-archive 'yurivict/vm-to-tor.git''
>   You appear to be using ssh to clone a git:// URL.
>   Make sure your core.gitProxy config option and the
>   GIT_PROXY_COMMAND environment variable are NOT set.
> fatal: The remote end hung up unexpectedly

This message isn't from git, but rather from GitHub's ssh server. You
can see it here without running git locally at all:

  $ ssh g...@github.com git-upload-archive
  Invalid command: 'git-upload-archive'
  You appear to be using ssh to clone a git:// URL.
  Make sure your core.gitProxy config option and the
  GIT_PROXY_COMMAND environment variable are NOT set.

The short of it is that GitHub does not support remote git-archive at
all, not even over ssh. Part of the reason is that it maintains a
separate caching layer for grabbing zips and tarballs, which is
available at an http endpoint:

  https://github.com/yurivict/vm-to-tor/archive/master.tar.gz

Clearly the advice above is not helpful in this case; I don't recall the
case that it was originally written to help. You may want to write to
GitHub support about it.

> # through ssh without an ssh key
> $  git archive --format=tar --remote=g...@github.com:yurivict/vm-to-tor.git
> Permission denied (publickey).
> fatal: The remote end hung up unexpectedly

GitHub's ssh server only accepts key authentication, so this is not
surprising.

> # through https
> $  git archive --format=tar
> --remote=https://github.com/yurivict/vm-to-tor.git
> fatal: Operation not supported by protocol.
> 
> # through http
> $  git archive --format=tar
> --remote=http://github.com/yurivict/vm-to-tor.git
> fatal: Operation not supported by protocol.

This comes from the local git client. There's no scheme defined for
hitting git-upload-archive (the server side of a remote git-archive)
over HTTP. You'd have to define it, implement the client side of it, and
then convince the server side to configure their HTTP servers to handle
it as a CGI. That might be a nice project to work on, but it will not
get you tarballs anytime soon. :)

-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] filter-branch: pass tag name via stdin without newline

[Jeff King]

On Sun, Dec 06, 2015 at 06:55:21PM -0800, Junio C Hamano wrote:

> "Eric N. Vander Weele"  writes:
> 
> > "git filter-branch --tag-name-filter" fails when the user-provided
> > command attempts to trivially append text to the originally tag name,
> > passed via stdin, due to an unexpected newline ('\n').  The newline is
> > introduced due to "echo" piping the original tag name to the
> > user-provided tag name filter command.
> 
> Is there any other place where we feed such an incomplete line
> (i.e. a line without the terminating LF) to the filters in this
> command?

I suspect we would break filters if we did. On some systems, tools like
`sed` and `tr` often behave funny when there is no trailing newline, but
I do not recall all of the specific instances (if one felt like engaging
in some light masochism, searching the mailing list archive will
probably turn up results).

-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: What's cooking in git.git (Dec 2015, #02; Fri, 4)

[Jeff King]

On Sun, Dec 06, 2015 at 10:12:18AM -0600, Edmundo Carmona Antoranz wrote:

> Hi, Junio, Jeff!
> 
> On Fri, Dec 4, 2015 at 5:15 PM, Junio C Hamano  wrote:
> > * ec/annotate-deleted (2015-11-20) 1 commit
> >  - annotate: skip checking working tree if a revision is provided
> >
> >  Usability fix for annotate-specific " " syntax with deleted
> >  files.
> >
> >  Waiting for review.
> 
> Is there something I have to do about it?

A gentle ping is sometimes helpful. :)

I did not see anything wrong with it, but I think Junio would be a good
person to give it a look, as he knows all of the sordid history of blame
versus annotate, and the intended allowed command-line orderings.

-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] contrib/subtree: fix "subtree split" skipped-merge bug.

[Dave Ware]

A bug occurs in 'git-subtree split' where a merge is skipped even when
both parents act on the subtree, provided the merge results in a tree
identical to one of the parents. Fix by copying the merge if at least
one parent is non-identical, and the non-identical parent is not an
ancestor of the identical parent.

Also adding a test case, this checks that a descendant can be pushed to
it's ancestor in this case.

Signed-off-by: Dave Ware 
---
 contrib/subtree/git-subtree.sh | 12 +++--
 contrib/subtree/t/t7900-subtree.sh | 52 ++
 2 files changed, 62 insertions(+), 2 deletions(-)

diff --git a/contrib/subtree/git-subtree.sh b/contrib/subtree/git-subtree.sh
index 9f06571..b837531 100755
--- a/contrib/subtree/git-subtree.sh
+++ b/contrib/subtree/git-subtree.sh
@@ -479,8 +479,16 @@ copy_or_skip()
p="$p -p $parent"
fi
done
-   
-   if [ -n "$identical" ]; then
+
+   copycommit=
+   if [ -n "$identical" ] && [ -n "$nonidentical" ]; then
+   extras=$(git rev-list --boundary $identical..$nonidentical)
+   if [ -n "$extras" ]; then
+   # we need to preserve history along the other branch
+   copycommit=1
+   fi
+   fi
+   if [ -n "$identical" ] && [ -z "$copycommit" ]; then
echo $identical
else
copy_commit $rev $tree "$p" || exit $?
diff --git a/contrib/subtree/t/t7900-subtree.sh 
b/contrib/subtree/t/t7900-subtree.sh
index 9051982..ea991eb 100755
--- a/contrib/subtree/t/t7900-subtree.sh
+++ b/contrib/subtree/t/t7900-subtree.sh
@@ -468,4 +468,56 @@ test_expect_success 'verify one file change per commit' '
))
 '
 
+test_expect_success 'subtree descendent check' '
+  mkdir git_subtree_split_check &&
+  (
+cd git_subtree_split_check &&
+git init &&
+
+mkdir folder &&
+
+echo a >folder/a &&
+git add . &&
+git commit -m "first commit" &&
+
+git branch branch &&
+
+echo 0 >folder/0 &&
+git add . &&
+git commit -m "adding 0 to folder" &&
+
+echo b >folder/b &&
+git add . &&
+git commit -m "adding b to folder" &&
+cherry=$(git rev-list HEAD -1) &&
+
+git checkout branch &&
+echo text >textBranch.txt &&
+git add . &&
+git commit -m "commit to fiddle with branch: branch" &&
+
+git cherry-pick $cherry &&
+git checkout master &&
+git merge -m "merge" branch &&
+
+git branch noop_branch &&
+
+echo d >folder/d &&
+git add . &&
+git commit -m "adding d to folder" &&
+
+git checkout noop_branch &&
+echo moreText >anotherText.txt &&
+git add . &&
+git commit -m "irrelevant" &&
+
+git checkout master &&
+git merge -m "second merge" noop_branch &&
+
+git subtree split --prefix folder/ --branch subtree_tip master &&
+git subtree split --prefix folder/ --branch subtree_branch branch &&
+git push . subtree_tip:subtree_branch
+  )
+  '
+
 test_done
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 2/2] clean: new option --exclude-from

[Jeff King]

On Sun, Dec 06, 2015 at 09:58:26AM -0500, James wrote:

> +test_expect_success 'git clean -e --exclude-from' '
> + rm -fr repo &&
> + mkdir repo &&
> + cd repo &&
> + git init &&
> + touch known 1 2 3 &&
> + git add known &&
> + echo 1 >> .git/clean-exclude &&
> + git clean -f -e 2 --exclude-from=.git/clean-exclude &&
> + test_path_is_file 1 &&
> + test_path_is_file 2 &&
> + test_path_is_missing 3 &&
> + test_path_is_file known
> +'

This checks that we exclude the union of the "-e" and "--exclude-from"
parameters. What happens if one excludes a path and the other negates
the exclusion? How is the precedence handled? Is it based on order on
the command-line, or something else?

I do not have much of a preference myself, but it probably makes sense
to document and test it.

> +test_expect_success 'git clean --exclude-from --exclude-from' '
> + rm -fr repo &&
> + mkdir repo &&
> + git init &&
> + touch known 1 2 3 &&
> + git add known &&
> + cat >.git/clean-exclude1 <<-\EOF &&
> + 1
> + EOF
> + cat >.git/clean-exclude2 <<-\EOF &&
> + 2
> + EOF
> + git clean -f --exclude-from=.git/clean-exclude1 
> --exclude-from=.git/clean-exclude2 &&
> + test_path_is_file 1 &&
> + test_path_is_file 2 &&
> + test_path_is_missing 3 &&
> + test_path_is_file known
> +'

Ditto here (as the same "type", the only precedence that would make any
sense is command-line order here).

> +test_expect_success 'git clean --exclude-from=BADFILE' '
> + rm -fr repo &&
> + mkdir repo &&
> + cd repo &&
> + git init &&
> + test_expect_code 128 git clean -f 
> --exclude-from=.git/clean-exclude-not-there
> +'

Do you actually care about the 128 here, or is just "it should exit with
failure"? If the latter, we usually use test_must_fail.

-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: git subtree bug produces divergent descendants

[David Ware]

Thanks for taking the time to look over it. I'm not familiar with the
process here.

On Mon, Dec 7, 2015 at 5:53 PM, Eric Sunshine  wrote:
> Tests don't automatically return to the directory prior to the 'cd',
> so when this test ends, the current directory will still be
> 'git_subtree_split_check'. If someone later adds a test following
> this one, that test will execute within 'git_subtree_split_check',
> which might not be expected by the test writer.
>
> To ensure that the prior working directory is restored at the end of
> the test (regardless of success or failure), tests typically employ a
> subshell using this idiom:
>
> mkdir foo &&
> (
> cd foo &&
> ... &&
> ...
> )
>

I'm not at all familiar with this test harness so I had a few problems
here (like this, and the bash variable). Thank you for the advice.

Cheers,
Dave Ware
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: best practices against long git rebase times?

[Jeff King]

On Sun, Dec 06, 2015 at 05:43:45PM +0100, Andreas Krey wrote:

> On Fri, 04 Dec 2015 15:31:03 +, John Keeping wrote:
> ...
> > I'm pretty sure that you're right and the cherry-pick analysis is where
> > the time is spent.
> 
> But I'm pretty surprised as to the amount of CPU time that goes there.
> 
> I'm now rebasing a single commit with a single blank line added,
> and for 3000 new commits to rebase over (with 64 MByte of git log -
> for them) it takes twelve minutes, or about for commits per second,
> and all user CPU, no I/O. It's pretty linear in number of commits, too.

You're computing the patch against the parent for each of those 3000
commits (to get a hash of it to compare against the single hash on the
other side). Twelve minutes sounds long, but if you have a really
gigantic tree, it might not be unreasonable.

You can also try compiling with "make XDL_FAST_HASH=" (i.e., setting
that option to the empty string). Last year I found there were some
pretty suboptimal corner cases, and you may be hitting one (we should
probably turn that option off by default; I got stuck on trying to find
a hash that would perform faster and never followed up[1].

I doubt that is your problem, but it's possible).

-Peff

[1] http://thread.gmane.org/gmane.comp.version-control.git/261638
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Why does send-pack call pack-objects for all remote refs?

[Daniel Koverman]

I have a repository which has ~2000 branches on the remote, and it takes ~8 
seconds to push a change to one ref. The majority of this time is spent in 
pack-object. I wrote a hack so that only the ref being updated would be packed 
(the normal behavior is to pack for every ref on the remote).  The push time 
dropped to <1 second with (seemingly) no consequences. This raised a couple of 
questions:

1) Are there consequences for not packing refs that are not being updated? Can 
all operations in send-pack which operate on other refs be skipped?
2) Why isn't git more selective about what it packs? Is it simply a performance 
optimization not worth the implementation complexity?
3) Is something about my repository strange? ex: 2000 is too many branches, 
packing for each ref takes me an unusually long time, etc.

Thanks in advance to anyone who takes the time to clarify this for me.

Daniel
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH/RFC 3/3] ls-files/dir: use is_git_repo to detect submodules

[Jeff King]

On Sun, Dec 06, 2015 at 05:59:26PM +0100, Andreas Krey wrote:

> On Sat, 05 Dec 2015 02:37:44 +, Jeff King wrote:
> ...
> > Hrm. Weird. You'd think it would break with the existing code if I do
> > this, then:
> > 
> ...
> > -   (cd a/b/c; git init) &&
> > +   (cd a/b/c; git init && git commit --allow-empty -m foo) &&
> > git config remote.origin.url ../foo/bar.git &&
> > git submodule add ../bar/a/b/c ./a/b/c &&
> 
> I tried a -f here instead; did not work either.
> 
> I guess I will first wade through the other failures my 'fix'
> causes to see the total damage.

The only other one I saw was in the completion tests. And it looked like
`git add` failing in a way similar to what I dug into here. So I think
it's "just" the one bug.

> > We know it is a git dir, but there is no sha1 for us to actually add as
> > the gitlink entry.
> > 
> > If that is the case, then there is either some very tricky refactoring
> > required,
> 
> Yes, it looks like the return code delivered need to be slightly different
> dependent on the user.

Maybe. From your patch it looks like the "git-add" code will return it
as "untracked". Which makes sense if we then want to add it. But if it
has no HEAD commit we _cannot_ add it, as we have no sha1 to stick in
the index. It should probably be ignored totally in that case.

But that means you have to actually find out if HEAD is valid or not.
Which is what the current code is doing. :-/

> > or what we are trying to do here is simply wrong. Maybe it
> > would be simpler to just speed up resolve_gitlink_ref with a better data
> > structure.
> 
> Which is what I did on square one, but now we already have a real fix
> for git clean, and now it's even less advantageous the fix the consequence
> (the submodule cache blowing up) instead of the cause (asking for it
> in the first place).

I think "clean" is a much simpler case. It only wants to know "can I
skip this entry as an untracked sub-repo?". And that is similar to what
"git status" or "git ls-files" wants to know. But "git add" wants to
know "can I _add_ this entry to the index", and that is a different
question (but I think answered by the same code that powers ls-files).

> PS: I seem to not quite have send-email under control, the envelope from
> seems to made the patches not reach the mailing list (nor me in the CC).

Hmm, yeah. Obviously they made it to me directly, but the list is a
little bit picky.

-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Multiple fetches when unshallowing a shallow clone

[Duy Nguyen]

On Mon, Dec 7, 2015 at 8:57 PM, Jason Paller-Rzepka  wrote:
> Duy, you mentioned that depth=0 means "do not change depth".  I assume that
> means the server should use exactly the shallows that the client sent, and
> it does not need to traverse the tree or modify the shallow or unshallow
> sets at all.  Right?

Correct. The server might send new shallow lines anyway though, if the
server repo is also shallow and the new fetched ref needs to be cut.
But I don't know if Dulwich supports that yet.

> Duy, you also mentioned that "those lines should be rejected any way".  You
> just mean that a "deepen 0" line should be rejected, right? And that's
> because the right way to tell git-upload-pack not to change the depth is to
> omit the "deepen" line after the "shallow" lines, so there's never a need to
> send "deepen 0"?

Also correct. I didn't check the code when I wrote that. But I have
checked and upload-pack does reject "deepen 0"

if (starts_with(line, "deepen ")) {
   char *end;
   depth = strtol(line + 7, , 0);
   if (end == line + 7 || depth <= 0)
  die("Invalid deepen: %s", line);
-- 
Duy
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Multiple fetches when unshallowing a shallow clone

[Junio C Hamano]

Jeff King  writes:

> I think one thing I was missing is that we need to just grab the
> _object_, but we need to realize that the ref needs updating[1]. So we
> cannot skip backfill of any tag that we do not already have, even if we
> already have the tag object.
> ...
> [1] I'm still puzzled why find_non_local_tags uses has_sha1_file() on
> the tag object at all, then.

The designed semantics of auto-following tags (not necessarily as
implemented or documented, i.e. there may be implementation or
documentation bugs), I think, is to arrive at the same state as
doing a fetch (or a push) without the auto-following and then doing
a separate fetch (or a push) of tags that point at the objects that
are reachable from the tips of refs after finishing the first
(i.e. without auto-follow) fetch (or a push).  In a scenario where
we already have a commit reachable from existing remote-tracking
branch and the current transfer (be it a fetch or a push, with or
without auto-follow) does not update any remote-tracking branch
(because the source side did not have any changes), if the source
side added a tag that refers to that commit that the receiving end
lacks, that tag needs to be transferred and then stored.

So has_sha1_file() is not the right test---if anything, it needs to
be checking if the object being checked is reachable from a tip of
some ref.

But of course, that test is rather expensive, so perhaps the
implementation cheated and uses has_sha1_file() instead?  The only
case it would misidentify would be after an aborted fetch (or push)
left unconnected island of objects and some of these objects that
are not reachable are pointed at by tags the receiving end does not
have.





 
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 1/2] modernize t7300

[Junio C Hamano]

James  writes:

> From: James Rouzier 
>
> ---

Missing sign-off.

>  t/t7300-clean.sh | 382 
> +++
>  1 file changed, 190 insertions(+), 192 deletions(-)
>
> diff --git a/t/t7300-clean.sh b/t/t7300-clean.sh
> index 86ceb38..d555bb6 100755
> --- a/t/t7300-clean.sh
> +++ b/t/t7300-clean.sh
> @@ -28,15 +28,15 @@ test_expect_success 'git clean with skip-worktree 
> .gitignore' '
>   mkdir -p build docs &&
>   touch a.out src/part3.c docs/manual.txt obj.o build/lib.so &&
>   git clean &&
> - test -f Makefile &&
> - test -f README &&
> - test -f src/part1.c &&
> - test -f src/part2.c &&
> - test ! -f a.out &&
> - test ! -f src/part3.c &&
> - test -f docs/manual.txt &&
> - test -f obj.o &&
> - test -f build/lib.so &&
> + test_path_is_file Makefile &&
> + test_path_is_file README &&
> + test_path_is_file src/part1.c &&
> + test_path_is_file src/part2.c &&

OK.

> + test_path_is_missing a.out &&
> + test_path_is_missing src/part3.c &&

This is better than the original, which may have said "OK" upon
seeing a directory called "a.out".

> + test_path_is_file docs/manual.txt &&
> + test_path_is_file obj.o &&
> + test_path_is_file build/lib.so &&

OK.

>   git update-index --no-skip-worktree .gitignore &&
>   git checkout .gitignore
>  '
> @@ -46,15 +46,15 @@ test_expect_success 'git clean' '
>   mkdir -p build docs &&
>   touch a.out src/part3.c docs/manual.txt obj.o build/lib.so &&
>   git clean &&
> - test -f Makefile &&
> - test -f README &&
> - test -f src/part1.c &&
> - test -f src/part2.c &&
> - test ! -f a.out &&
> - test ! -f src/part3.c &&
> - test -f docs/manual.txt &&
> - test -f obj.o &&
> - test -f build/lib.so
> + test_path_is_file Makefile &&
> + test_path_is_file README &&
> + test_path_is_file src/part1.c &&
> + test_path_is_file src/part2.c &&
> + test_path_is_missing a.out &&
> + test_path_is_missing src/part3.c &&
> + test_path_is_file docs/manual.txt &&
> + test_path_is_file obj.o &&
> + test_path_is_file build/lib.so

The verbosity of this conversion makes me wonder if we want to have
"test_paths_are_files" and "test_paths_are_missing".  For that
matter, this test does not really care about the distinction between
files and directories (e.g. some tests said "test ! -d docs" and
would have passed if there were a 'docs' regular file, but what we
really care about is the path 'docs' is _gone_), so what we want may
be test_paths_exist and test_paths_are_missing.  With that, the
above hunk would become

test_paths_exist Makefile README src/part1.c src/part2.c \
obj.o build/lib.so &&
test_paths_are_missing a.out src/part3.c

I dunno.

>  test_expect_success 'git clean -e' '
>   rm -fr repo &&
>   mkdir repo &&
> - (
> - cd repo &&
> - git init &&
> - touch known 1 2 3 &&
> - git add known &&
> - git clean -f -e 1 -e 2 &&
> - test -e 1 &&
> - test -e 2 &&
> - ! (test -e 3) &&
> - test -e known
> - )
> + cd repo &&
> + git init &&
> + touch known 1 2 3 &&
> + git add known &&
> + git clean -f -e 1 -e 2 &&
> + test_path_is_file 1 &&
> + test_path_is_file 2 &&
> + test_path_is_missing 3 &&
> + test_path_is_file known
>  '

I think this is wrong.  The next test piece will be run inside
"repo" directory with this patch applied.  Don't lose the subshell
here.

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Multiple fetches when unshallowing a shallow clone

[Jeff King]

On Mon, Dec 07, 2015 at 01:27:51PM -0800, Junio C Hamano wrote:

> Jeff King  writes:
> 
> > I think one thing I was missing is that we need to just grab the
> > _object_, but we need to realize that the ref needs updating[1]. So we
> > cannot skip backfill of any tag that we do not already have, even if we
> > already have the tag object.
> > ...
> > [1] I'm still puzzled why find_non_local_tags uses has_sha1_file() on
> > the tag object at all, then.
> 
> The designed semantics of auto-following tags (not necessarily as
> implemented or documented, i.e. there may be implementation or
> documentation bugs), I think, is to arrive at the same state as
> doing a fetch (or a push) without the auto-following and then doing
> a separate fetch (or a push) of tags that point at the objects that
> are reachable from the tips of refs after finishing the first
> (i.e. without auto-follow) fetch (or a push).  In a scenario where
> we already have a commit reachable from existing remote-tracking
> branch and the current transfer (be it a fetch or a push, with or
> without auto-follow) does not update any remote-tracking branch
> (because the source side did not have any changes), if the source
> side added a tag that refers to that commit that the receiving end
> lacks, that tag needs to be transferred and then stored.
> 
> So has_sha1_file() is not the right test---if anything, it needs to
> be checking if the object being checked is reachable from a tip of
> some ref.
> 
> But of course, that test is rather expensive, so perhaps the
> implementation cheated and uses has_sha1_file() instead?  The only
> case it would misidentify would be after an aborted fetch (or push)
> left unconnected island of objects and some of these objects that
> are not reachable are pointed at by tags the receiving end does not
> have.

I may have confused myself. There are actually two has_sha1_file() calls
in find_non_local_tags.

I agree it is the only sensible test for "do we have the commit this tag
peels to, and if so, we want to grab the tag".  Reachability is too
expensive to compute.

But for the other one ("do we have the tag object itself"), I initially
claimed "if we have the tag object already, we do not have to do the
backfill fetch". Which is not quite true. We have to update the ref even
if we have the tag object. But then, what if we have the tag object for
other reasons (e.g., because another tag points at it?).

E.g. in this sequence:

  git -C parent commit --allow-empty -m base
  git -C parent tag -m mytag foo
  git clone parent child
  git -C parent update-ref refs/tags/bar foo
  git -C child fetch

we must backfill refs/tags/bar during the fetch, even though we already
have the object. I don't see any point in checking has_sha1_file() for
the tagged object at all. If we don't have it, we obviously must fetch.
And if we do have it, we must fetch the ref, even if that results in no
objects transferred.

It's entirely possible I'm just confused, and AFAICT nobody has noticed
any breakage here, so please don't feel you need to spend a lot of time
humoring me. I'm just writing up my confusion for posterity. :)

-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 2/2] clean: new option --exclude-from

[Junio C Hamano]

James  writes:

> +static int exclude_from_cb(const struct option *opt,
> +  const char *arg, int unset)
> +{
> + struct dir_struct *dir = opt->value;
> + add_excludes_from_file(dir, arg);

I suspect this is wrong.  add_excludes_from_file() creates a
new excludes_list that is separate from the command line level and
pushes that down to the exclude stack.  You'd instead need to add
each line of the input at the same EXC_CMDL level like -e 
option does from the command line, I would think.

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2] revision.c: fix possible null pointer access

[Johannes Sixt]

Am 07.12.2015 um 21:31 schrieb Junio C Hamano:

Stefan Naewe  writes:


mark_tree_uninteresting dereferences a tree pointer before checking
if the pointer is valid. Fix that by doing the check first.

Signed-off-by: Stefan Naewe 
---


I still have a problem with "dereferences", as "dereference" is
about computing an address and accessing memory based on the result,
and only the first half is happening here.  I can live with "The
function does a pointer arithmetic on 'tree' before it makes sure
that 'tree' is not NULL", but in any case, let's queue this as-is
for now and wait for a while to see if others can come up with a
more appropriate phrases.


Don't shoo away language lawyers, because this is a pure C language rule 
patch. If this were only about pointer arithmetic, a change would not be 
necessary. But it isn't. The patch corrects a case where the compiler 
can remove a NULL pointer check that we actually want to remain. The 
language rule that gives sufficient room for interpretation to the 
compiler is about dereferencing a pointer. It is irrelevant that an 
address of an object is taken after the dereference and then only 
pointer arithmetic remains---the dereference has already taken place, 
and that cannot occur for a NULL pointer in a valid program. So, the 
phrase "dereference" is precise and correct here.


-- Hannes



Thanks.


  revision.c | 4 +++-
  1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/revision.c b/revision.c
index 0fbb684..8c569cc 100644
--- a/revision.c
+++ b/revision.c
@@ -135,10 +135,12 @@ static void mark_tree_contents_uninteresting(struct tree 
*tree)

  void mark_tree_uninteresting(struct tree *tree)
  {
-   struct object *obj = >object;
+   struct object *obj;

if (!tree)
return;
+
+   obj = >object;
if (obj->flags & UNINTERESTING)
return;
obj->flags |= UNINTERESTING;


--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Why does send-pack call pack-objects for all remote refs?

[Junio C Hamano]

Daniel Koverman  writes:

> I have a repository which has ~2000 branches on the remote, and it
> takes ~8 seconds to push a change to one ref. The majority of this
> time is spent in pack-object. I wrote a hack so that only the ref
> being updated would be packed (the normal behavior is to pack for
> every ref on the remote).

I am having a hard time understanding what you are trying to say, as
nobody's pack-objects "packs for a ref" or "packs a ref", so my
response has to be based on my best guess---I think you are talking
about feeding the object names of the tips of all remote refs as
the bottoms of the revision range to pack-objects.

When you are pushing your 'topic' branch to update the 'topic'
branch at the remote, it is true that we compute

git rev-list --objects $your_topic --not $all_of_the_remote_refs

to produce a packfile.  And by tweaking this to

git rev-list --objects $your_topic --not $their_topic

you will cut down the processing time of 'rev-list', especially if
you have insane number of refs at the remote end.

There is a price you would pay for doing so, though.  An obvious one
is what if the 'topic' branch does not exist yet at the remote.
Without the "--not ..." part, you would end up sending the entire
history behind $your_topic, and the way you prevent that from
happening is to give what are known to exist at the remote end.
Even when there already is 'topic' at the remote, the contents at
the paths that are different between your 'topic' and the 'topic' as
exists at the remote may already exist on some other branches that
are already at the remote (e.g. you may have merged some branches
that are common between your repository and the remote, and the only
object missing from the remote that your repository has to send may
be a merge commit and the top-level tree object), but limiting the
bottoms of the revision range only to "--not $their_topic" would rob
this obvious optimization opportunity from you.

There has to be some way to limit the list of remote-refs that are
used as bottoms of the revision range.  For example, if you know
that the remote has all the tags, and that everything in the v1.0
tag is contained in the v2.0 tag, then a single "--not v2.0" should
give the same result as "--not v1.0 v2.0" that lists both.  But the
computation that is needed to figure out which tags and branches are
not worth listing as bottoms would need to look at all of them at
least once anyway, so a naive implementation of such would end up
spending the same cycles, I would suspect.

Also it was unclear if you are working with a shallow repository.
The performance trade-off made between the packsize and the cycles
is somewhat different between a normal and a shallow repository,
e.g. 2dacf26d (pack-objects: use --objects-edge-aggressive for
shallow repos, 2014-12-24) might be a good starting point to think
about this issue.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 1/2] modernize t7300

[Eric Sunshine]

On Sun, Dec 6, 2015 at 9:58 AM, James  wrote:
> From: James Rouzier 

This would be a good place to explain how you are modernizing the test
script. Right now, you're just updating to take advantage of
test_path_is_foo(), but some other modernizations you could do
include:

* using '>' rather than 'touch' to create empty files when the
timestamp doesn't matter (which is all cases in this script)

* (optional) replace unnecessarily complex 'mkdir -p foo' with simpler
'mkdir foo' (but leave "mkdir -p foo/bar" as is)

* drop blank lines before and after test body; for instance:

test_expect_success 'foo' '

blah &&
bloo

'

becomes:

test_expect_success 'foo' '
blah &&
bloo
'

> ---
> diff --git a/t/t7300-clean.sh b/t/t7300-clean.sh
> @@ -609,32 +609,30 @@ test_expect_success 'force removal of nested git work 
> tree' '
>  test_expect_success 'git clean -e' '
> rm -fr repo &&
> mkdir repo &&
> -   (
> -   cd repo &&
> -   git init &&
> -   touch known 1 2 3 &&
> -   git add known &&
> -   git clean -f -e 1 -e 2 &&
> -   test -e 1 &&
> -   test -e 2 &&
> -   ! (test -e 3) &&
> -   test -e known
> -   )
> +   cd repo &&

The previous working directory is not automatically restored at the
end of the test, so this "cd" without corresponding "cd .." causes
following tests to execute at the incorrect location. Unfortunately,
you can't just place "cd .." at the end of a test since it will be
skipped if something before the "cd .." fails. The way the existing
code deals with this is by using a subshell:

mkdir repo &&
(
cd repo &&
...
)

The "cd repo" is inside the subshell, so it doesn't affect the parent
shell, and when the subshell exits, the parent shell remains at the
correct working directory (regardless of whether the test succeeded or
failed). Therefore, you don't want to drop the subshell.

> +   git init &&
> +   touch known 1 2 3 &&
> +   git add known &&
> +   git clean -f -e 1 -e 2 &&
> +   test_path_is_file 1 &&
> +   test_path_is_file 2 &&
> +   test_path_is_missing 3 &&
> +   test_path_is_file known
>  '
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 1/2] modernize t7300

[Eric Sunshine]

On Mon, Dec 7, 2015 at 4:40 PM, Junio C Hamano  wrote:
> James  writes:
>> @@ -46,15 +46,15 @@ test_expect_success 'git clean' '
>>   mkdir -p build docs &&
>>   touch a.out src/part3.c docs/manual.txt obj.o build/lib.so &&
>>   git clean &&
>> - test -f Makefile &&
>> - test -f README &&
>> - test -f src/part1.c &&
>> - test -f src/part2.c &&
>> - test ! -f a.out &&
>> - test ! -f src/part3.c &&
>> - test -f docs/manual.txt &&
>> - test -f obj.o &&
>> - test -f build/lib.so
>> + test_path_is_file Makefile &&
>> + test_path_is_file README &&
>> + test_path_is_file src/part1.c &&
>> + test_path_is_file src/part2.c &&
>> + test_path_is_missing a.out &&
>> + test_path_is_missing src/part3.c &&
>> + test_path_is_file docs/manual.txt &&
>> + test_path_is_file obj.o &&
>> + test_path_is_file build/lib.so
>
> The verbosity of this conversion makes me wonder if we want to have
> "test_paths_are_files" and "test_paths_are_missing".  For that
> matter, this test does not really care about the distinction between
> files and directories (e.g. some tests said "test ! -d docs" and
> would have passed if there were a 'docs' regular file, but what we
> really care about is the path 'docs' is _gone_), so what we want may
> be test_paths_exist and test_paths_are_missing.  With that, the
> above hunk would become
>
> test_paths_exist Makefile README src/part1.c src/part2.c \
> obj.o build/lib.so &&
> test_paths_are_missing a.out src/part3.c
>
> I dunno.

Alternately, update test_path_foo() functions to accept multiple
pathnames, or is that too ugly?
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 1/2] modernize t7300

[Junio C Hamano]

Eric Sunshine  writes:

> Alternately, update test_path_foo() functions to accept multiple
> pathnames, or is that too ugly?

That actually would have been my first choice, except (1) that
path_is_missing has a cruft whose usefulness is dubious, and (2)
that "path_exists A B C" and "path_is_missing D E F" would be
gramatically incorrect.

I think we should first see if we can remove that "customized
message" that appears only in path_is_missing and remove it if
we can.  Extending "path_is_missing A B C" and friends to work
would then become trivial.

Thanks.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 2/2] clean: new option --exclude-from

[Eric Sunshine]

In addition to Peff's and Junio's review comments...

On Sun, Dec 6, 2015 at 9:58 AM, James  wrote:
> From: James Rouzier 
>
> Specify a file to read for exclude patterns.

Missing Signed-off-by:.

> ---
> diff --git a/t/t7300-clean.sh b/t/t7300-clean.sh
> @@ -628,6 +628,66 @@ test_expect_success 'git clean -e' '
> +test_expect_success 'git clean --exclude-from' '
> +   rm -fr repo &&
> +   mkdir repo &&
> +   cd repo &&

See my review comments for patch 1/2 as to why you want to wrap 'cd'
and remaining statements in a subshell.

> +   git init &&
> +   touch known 1 2 3 &&

Likewise, use '>' rather than 'touch' to create empty files when the
timestamp isn't significant.

   >1 &&
   >2 &&
   >3 &&

> +   git add known &&
> +   cat >.git/clean-exclude <<-\EOF &&
> +   1
> +   2
> +   EOF
> +   git clean -f --exclude-from=.git/clean-exclude &&
> +   test_path_is_file 1 &&
> +   test_path_is_file 2 &&
> +   test_path_is_missing 3 &&
> +   test_path_is_file known
> +'
> +
> +test_expect_success 'git clean -e --exclude-from' '
> +   rm -fr repo &&
> +   mkdir repo &&
> +   cd repo &&
> +   git init &&
> +   touch known 1 2 3 &&
> +   git add known &&
> +   echo 1 >> .git/clean-exclude &&
> +   git clean -f -e 2 --exclude-from=.git/clean-exclude &&
> +   test_path_is_file 1 &&
> +   test_path_is_file 2 &&
> +   test_path_is_missing 3 &&
> +   test_path_is_file known
> +'
> +
> +test_expect_success 'git clean --exclude-from --exclude-from' '
> +   rm -fr repo &&
> +   mkdir repo &&
> +   git init &&
> +   touch known 1 2 3 &&
> +   git add known &&
> +   cat >.git/clean-exclude1 <<-\EOF &&
> +   1
> +   EOF
> +   cat >.git/clean-exclude2 <<-\EOF &&
> +   2
> +   EOF

Creation of these single-line files probably would be more readable
using 'echo', as you do in the test just above (for
.git/clean-exclude):

echo 1 >.git/clean-exclude1 &&
echo 2 >.git/clean-exclude2 &&

> +   git clean -f --exclude-from=.git/clean-exclude1 
> --exclude-from=.git/clean-exclude2 &&
> +   test_path_is_file 1 &&
> +   test_path_is_file 2 &&
> +   test_path_is_missing 3 &&
> +   test_path_is_file known
> +'
> +
> +test_expect_success 'git clean --exclude-from=BADFILE' '
> +   rm -fr repo &&
> +   mkdir repo &&
> +   cd repo &&
> +   git init &&
> +   test_expect_code 128 git clean -f 
> --exclude-from=.git/clean-exclude-not-there
> +'
> +
>  test_expect_success SANITY 'git clean -d with an unreadable empty directory' 
> '
> mkdir foo &&
> chmod a= foo &&
> --
> 2.3.6
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: best practices against long git rebase times?

[Junio C Hamano]

Jeff King  writes:

> You're computing the patch against the parent for each of those 3000
> commits (to get a hash of it to compare against the single hash on the
> other side). Twelve minutes sounds long, but if you have a really
> gigantic tree, it might not be unreasonable.
>
> You can also try compiling with "make XDL_FAST_HASH=" (i.e., setting
> that option to the empty string). Last year I found there were some
> pretty suboptimal corner cases, and you may be hitting one (we should
> probably turn that option off by default; I got stuck on trying to find
> a hash that would perform faster and never followed up[1].
>
> I doubt that is your problem, but it's possible).
>
> -Peff
>
> [1] http://thread.gmane.org/gmane.comp.version-control.git/261638

I vaguely recall having discussed caching the patch-ids somewhere so
that this does not have to be done every time.  Would such an
extension help here, I wonder?
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Why does send-pack call pack-objects for all remote refs?

[Jeff King]

On Mon, Dec 07, 2015 at 02:41:00PM -0800, Junio C Hamano wrote:

> Also it was unclear if you are working with a shallow repository.
> The performance trade-off made between the packsize and the cycles
> is somewhat different between a normal and a shallow repository,
> e.g. 2dacf26d (pack-objects: use --objects-edge-aggressive for
> shallow repos, 2014-12-24) might be a good starting point to think
> about this issue.

Also note that for a while the "aggressive" form was used everywhere. I
think that started in fbd4a70 (list-objects: mark more commits as edges
in mark_edges_uninteresting - 2013-08-16), and was fixed in 1684c1b
(rev-list: add an option to mark fewer edges as uninteresting,
2014-12-24).

So it was present from v1.8.4.2 up to v2.3.0.

-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: best practices against long git rebase times?

[Jeff King]

On Mon, Dec 07, 2015 at 02:56:33PM -0800, Junio C Hamano wrote:

> Jeff King  writes:
> 
> > You're computing the patch against the parent for each of those 3000
> > commits (to get a hash of it to compare against the single hash on the
> > other side). Twelve minutes sounds long, but if you have a really
> > gigantic tree, it might not be unreasonable.
> >
> > You can also try compiling with "make XDL_FAST_HASH=" (i.e., setting
> > that option to the empty string). Last year I found there were some
> > pretty suboptimal corner cases, and you may be hitting one (we should
> > probably turn that option off by default; I got stuck on trying to find
> > a hash that would perform faster and never followed up[1].
> >
> > I doubt that is your problem, but it's possible).
> >
> > -Peff
> >
> > [1] http://thread.gmane.org/gmane.comp.version-control.git/261638
> 
> I vaguely recall having discussed caching the patch-ids somewhere so
> that this does not have to be done every time.  Would such an
> extension help here, I wonder?

I think you missed John's earlier response which gave several pointers
to such caching schemes. :)

I used to run with patch-id-caching in my personal fork (I frequently
use "git log --cherry-mark" to see what has made it upstream), but I
haven't for a while. It did make a big difference in speed, but I never
resolved the corner cases around cache invalidation.

-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/2] setup.c: re-fix d95138e (setup: set env $GIT_WORK_TREE when ..

[Junio C Hamano]

Duy Nguyen  writes:

> On Thu, Dec 3, 2015 at 7:17 PM, Nguyễn Thái Ngọc Duy  
> wrote:
>> The solution in d95138e is reverted in this commit. Instead we reuse the
>> solution from c056261 [4]. c056261 fixes another setup-messed-up-by-alias
>> by saving and restoring env for git-clone and git-init. Now I conclude
>> that setup-messed-by-alias is always evil. So the env restoration is
>> done for _all_ commands  whenever aliases are involved. It fixes what
>> d95138e tries to fix. And it does not upset git-clone-inside-hooks.
>
> (Reviewer hat on) I wrote env restoration is done for all commands,
> but the patch is actually about all _builtin_ commands. External ones
> do not receive this treatment. FIx is in the next reroll.

Thanks for being careful.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Problems installing git-for-windows

[Stefan Beller]

+cc Johannes

IIUC Git-for-Windows specific issues are handled in
https://github.com/git-for-windows/git/issues
instead of this mailing list.

On Mon, Dec 7, 2015 at 4:44 AM, Peter Toye  wrote:
> Hi,
>
> I've just downloaded git-for-windows. It uninstalled an earlier
> version that I had (about 5 years old) and I got some problems.
>
> 1)  It wanted to install into my User directory instead of the
> Program Files directory, which is the best place for all programs.
> The installer told me that this had access right problems, so I
> installed it into another directory. It seemed to go OK.
>
> 2)  However,  the  icons  on  the desktop pointed to the place that my
> previous installation had been ( C:\Program Files (x86)\Git ) - that's
> where I'd have hoped that the new installation would go!
>
> 3)  I  uninstalled it to see if a cleaner installatoin would work, and
> the  GIT-GUI icon didn't disappear  from the desktop - minor bug I
> suspect.
>
>
> Regards,
>
> Peter
>  mailto:g...@ptoye.com
>  www.ptoye.com
>
> --
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/2] git-p4: support multiple depot paths in p4 submit

[Sam Hocevar]

On Sun, Dec 06, 2015, Lars Schneider wrote:
> Thanks for the patch! Do you see a way to demonstrate the bug in a test case 
> similar to t9821 [1]?

   Not yet, I'm afraid. It's proving trickier than expected because for
now I can only reproduce the bug when the view uses multiples depots
(solution #2 on http://answers.perforce.com/articles/KB/2437), and
unfortunately the test case system in Git was designed for a single
depot.

   Would a refactor of lib-git-p4.sh (and probably all git-p4 tests) to
support multiple depots be acceptable and/or welcome? I prefer to ask
before I dig into the task.

Regards,
-- 
Sam.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: best practices against long git rebase times?

[Junio C Hamano]

Jeff King  writes:

> I think you missed John's earlier response which gave several pointers
> to such caching schemes. :)

Yeah, you're right.

>
> I used to run with patch-id-caching in my personal fork (I frequently
> use "git log --cherry-mark" to see what has made it upstream), but I
> haven't for a while. It did make a big difference in speed, but I never
> resolved the corner cases around cache invalidation.
>
> -Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: What's cooking in git.git (Dec 2015, #02; Fri, 4)

[Junio C Hamano]

Jeff King  writes:

> On Sun, Dec 06, 2015 at 10:12:18AM -0600, Edmundo Carmona Antoranz wrote:
>
>> Hi, Junio, Jeff!
>> 
>> On Fri, Dec 4, 2015 at 5:15 PM, Junio C Hamano  wrote:
>> > * ec/annotate-deleted (2015-11-20) 1 commit
>> >  - annotate: skip checking working tree if a revision is provided
>> >
>> >  Usability fix for annotate-specific " " syntax with deleted
>> >  files.
>> >
>> >  Waiting for review.
>> 
>> Is there something I have to do about it?
>
> A gentle ping is sometimes helpful. :)
>
> I did not see anything wrong with it, but I think Junio would be a good
> person to give it a look, as he knows all of the sordid history of blame
> versus annotate, and the intended allowed command-line orderings.

If you bring "intended allowed command-line orderings" into the
picture, then I would have to say that any effort that encourages
the use of paths before revs like this patch does should be frowned
upon---if anything, we should actively discouraging people from
feeding paths before revs, and this change goes in a wrong direction
by adding a special case only to annotate, giving another excuse to
people who think "git log path rev" should work "because annotate
does".

If the change is just to 'annotate path rev', and does not change
'blame path rev', it would be less distasteful, though.

I'd have to think about it.

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Problems installing git-for-windows

[Johannes Schindelin]

Hi Peter,

On Mon, 7 Dec 2015, Peter Toye wrote:

> 1)  It wanted to install into my User directory instead of the
> Program Files directory, which is the best place for all programs.

There has been a regression in Git for Windows 2.6.3 (please *always*
state the version when reporting bugs) that requires you to run this
installer as administrator manually, else it will install into AppData by
default.

The other problems seem to be consequences.

The regression has been fixed and the next Git for Windows version will
install into Program Files again (if your account is equipped with the
appropriate permissions to elevate the installer enough).

Ciao,
Johannes
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: git branch in Bash subshell "$(git branch -a)" including ls output as part of return?

[Alex Jones]

That did the trick, thanks for the help and the suggestion.

On Mon, Dec 7, 2015 at 12:02 PM, Charles Bailey  wrote:
> On Mon, Dec 07, 2015 at 04:58:10PM +, Charles Bailey wrote:
>>
>> Looking at the two outputs, you are seeing the shell's glob expansion of
>> the '*' current branch marker. You probably want to quote the command
>> expansion to prevent this:
>>
>> echo "$(git branch -a)"
>
> Pressing send has, of course, caused me to think further. You probably
> don't want to parse the output of a "porcelain" command such as "git
> branch" at all, but instead look at using something like "git
> for-each-ref", perhaps with the --format=%(refname) option, grepping out
> master and iterating through the rest.



-- 

Alex Jones | Software Engineer
919-238-4404 direct
336-263-2099 mobile
netsertive.com
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 3/2] git.c: make sure we do not leak GIT_* to alias scripts

[Junio C Hamano]

Nguyễn Thái Ngọc Duy   writes:

>  Let's hope there will be no third report about this commit..

Hmm, why does this additional test fail only under prove but pass
without it?
> diff --git a/t/t0001-init.sh b/t/t0001-init.sh
> index f91bbcf..19539fc 100755
> --- a/t/t0001-init.sh
> +++ b/t/t0001-init.sh
> @@ -87,6 +87,33 @@ test_expect_success 'plain nested in bare through aliased 
> command' '
>   check_config bare-ancestor-aliased.git/plain-nested/.git false unset
>  '
>  
> +test_expect_success 'No extra GIT_* on alias scripts' '
> + cat <<-\EOF >expected &&
> + GIT_ATTR_NOSYSTEM
> + GIT_AUTHOR_EMAIL
> + GIT_AUTHOR_NAME
> + GIT_COMMITTER_EMAIL
> + GIT_COMMITTER_NAME
> + GIT_CONFIG_NOSYSTEM
> + GIT_EXEC_PATH
> + GIT_MERGE_AUTOEDIT
> + GIT_MERGE_VERBOSITY
> + GIT_PREFIX
> + GIT_TEMPLATE_DIR
> + GIT_TEXTDOMAINDIR
> + GIT_TRACE_BARE
> + EOF
> + cat <<-\EOF >script &&
> + #!/bin/sh
> + env | grep GIT_ | sed "s/=.*//" | sort >actual

This is more about coding discipline than style, but piping grep
output to sed is wasteful.  "sed -ne '/^GIT_/s/=.*//p'" or something
like that, perhaps?

I wondered what happens if the user has an unrelated stray variable
whose name happens to begin with GIT_ in her environment, but it
turns out that we cleanse them in test-lib.sh fairly early, so that
would be fine.  You need to tighten your "grep" pattern, though.

> + exit 0
> + EOF
> + chmod 755 script &&
> + git config alias.script \!./script &&
> + ( mkdir sub && cd sub && git script ) &&
> + test_cmp expected actual
> +'
> +
>  test_expect_success 'plain with GIT_WORK_TREE' '
>   mkdir plain-wt &&
>   test_must_fail env GIT_WORK_TREE="$(pwd)/plain-wt" git init plain-wt
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 13/16] init: allow alternate backends to be set for new repos

[David Turner]

On Sat, 2015-12-05 at 02:44 -0500, Jeff King wrote:
> On Sat, Dec 05, 2015 at 07:30:13AM +0100, Duy Nguyen wrote:
> 
> > On Thu, Dec 3, 2015 at 1:35 AM, David Turner  
> > wrote:
> > > git init learns a new argument --refs-backend-type.  Presently, only
> > > "files" is supported, but later we will add other backends.
> > >
> > > When this argument is used, the repository's core.refsBackendType
> > > configuration value is set, and the refs backend's initdb function is
> > > used to set up the ref database.
> > 
> > git-init can also be used on existing repos. What happens in that case
> > if we use --refs-backend-type. Will existing  refs be migrated to the
> > new backend or hidden away?

I'd like to migrate, but I don't want to write that code prematurely.  I
figure we can get everything else right first.  

> It would be neat if it migrated, but I suspect that may introduce
> complications. It's probably OK in the initial implementation to bail if
> the option is used in an existing repo.

Will fix.

> I think the config option needs to be extensions.refsBackendType, too,
> per the logic in 00a09d5 (introduce "extensions" form of
> core.repositoryformatversion, 2015-06-23). And I guess it needs to bump
> core.repositoryformatversion to "1".

I did that in a later patch, but now is a better time; will fix.


--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: rebase has no --verify-signatures

[brian m. carlson]

On Mon, Dec 07, 2015 at 03:00:15PM +0100, Alexander 'z33ky' Hirsch wrote:
> Is there any technical reason why rebase should not have a
> --verify-signatures flag? I have written a patch to git-rebase--am
> which enables it to do such a check. If there is no reason not to
> include it I'd add documentation and a test and submit it.

As far as I know, there is no technical reason that it shouldn't.  It's
probably that nobody has implemented it yet.  I'd certainly be
interested in such a patch.

For a thorough change, you'd probably want to make it work with
git-rebase--merge and git-rebase--interactive as well.  I'm sure I'm not
the only person who frequently uses rebase -m.
-- 
brian m. carlson / brian with sandals: Houston, Texas, US
+1 832 623 2791 | https://www.crustytoothpaste.net/~bmc | My opinion only
OpenPGP: RSA v4 4096b: 88AC E9B2 9196 305B A994 7552 F1BA 225C 0223 B187


signature.asc
Description: PGP signature

Re: [PATCH 2/2] format-patch: add an option to suppress commit hash

[brian m. carlson]

On Mon, Dec 07, 2015 at 11:34:59AM -0800, Junio C Hamano wrote:
> Two (big) problems with the option name.
> 
>  - "--no-something" would mislead people to think you are removing
>something, not replacing it with something else.  This option
>does the latter (i.e. the first line of your output still has
>40-hex; it's just it no longer has a useful 40-hex).

Right.  I originally considered removing that line altogether, but other
parts of Git rely on that header to process patches correctly.

>  - There are many places we use hexadecimal strings in format-patch
>output and you are not removing or replacing all of them, only
>the commit object name on the fake "From " line.  Saying "hash"
>would mislead readers.

I'll reroll with the name --zero-commit, unless somebody comes up with a
better suggestion.

> > +test_expect_success 'format-patch --no-hash' '
> > +   git format-patch --no-hash --stdout v2..v1 >patch2 &&
> > +   cnt=$(egrep "^From 0+ Mon Sep 17 00:00:00 2001" patch2 | wc -l) &&
> 
> Don't test "any number of '0'"; test 40 '0's.  This is because the
> line format was designed to be usable by things like /etc/magic to
> detect format-patch output, and we want to notice if/when we break
> that aspect of our output format.

My idea was to future-proof it against changes in the hash function,
although that's in the distant future.  I'll reroll with the change you
suggested.  I might drop in another patch to improve the existing tests
to cover the case without this option, as I think we just look for
"From " currently.
-- 
brian m. carlson / brian with sandals: Houston, Texas, US
+1 832 623 2791 | https://www.crustytoothpaste.net/~bmc | My opinion only
OpenPGP: RSA v4 4096b: 88AC E9B2 9196 305B A994 7552 F1BA 225C 0223 B187


signature.asc
Description: PGP signature