Re: [PATCH 1/2] t0051: test GIT_TRACE to a windows named pipe

[Sebastian Schuberth]

On 9/7/2018 8:19 PM, Jeff Hostetler via GitGitGadget wrote:


+test_expect_success MINGW 'o_append write to named pipe' '


Shouldn't this be "test_expect_failure" here, and then be changed to 
"test_expect_success" by your second patch?



--
Sebastian Schuberth

Re: [PATCH 0/9] Offer to run CI/PR builds in Visual Studio Team Services

[Sebastian Schuberth]

On 9/3/2018 11:10 PM, Johannes Schindelin via GitGitGadget wrote:


The one sad part about this is the Windows support. Travis lacks it, and we
work around that by using Visual Studio Team Services (VSTS) indirectly: one
phase in Travis would trigger a build, wait for its log, and then paste that
log.


I'm sorry if this has been discussed before, but as this recap doesn't 
mention it: Has AppVeyor been considered as an option? It seems to be 
the defacto standard for Windows CI for projects on GitHub.


--
Sebastian Schuberth

Re: [PATCH 2/9] ci/lib.sh: encapsulate Travis-specific things

[Sebastian Schuberth]

On 9/3/2018 11:10 PM, Johannes Schindelin via GitGitGadget wrote:


+if test -n "$TRAVIS_COMMIT"
+then
+   # We are running within Travis CI


Personally, I'd find a check like

if test "$TRAVIS" = "true"

more speaking (also see [1]).

[1] https://docs.travis-ci.com/user/environment-variables/

--
Sebastian Schuberth

Re: Colorize matches for git log --grep=pattern?

[Sebastian Schuberth]

On Wed, Nov 1, 2017 at 7:50 PM, Jeff King <p...@peff.net> wrote:

> The best workaround I could come up with is passing the output through a
> highlighting script:
>
>   git log --grep=foo --color |
>   perl -pe 's/foo/\x1b[1;31m$&\x1b[m/' |
>   less
>
> Pretty hacky.

Thanks anyway. I was also considering something like this, but
likewise found it to be too hacky.

-- 
Sebastian Schuberth

Colorize matches for git log --grep=pattern?

[Sebastian Schuberth]

Hi,

is there a way to colorize / highlight the pattern matched by

git log -E -i --grep=pattern

in the console output?

Regards,
Sebastian

[PATCH] docs: fix formatting of rev-parse's --show-superproject-working-tree

[Sebastian Schuberth]

Signed-off-by: Sebastian Schuberth <sschube...@gmail.com>
---
 Documentation/git-rev-parse.txt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Documentation/git-rev-parse.txt b/Documentation/git-rev-parse.txt
index 0917b8207b9d6..95326b85ff68e 100644
--- a/Documentation/git-rev-parse.txt
+++ b/Documentation/git-rev-parse.txt
@@ -264,7 +264,7 @@ print a message to stderr and exit with nonzero status.
 --show-toplevel::
Show the absolute path of the top-level directory.
 
---show-superproject-working-tree
+--show-superproject-working-tree::
Show the absolute path of the root of the superproject's
working tree (if exists) that uses the current repository as
its submodule.  Outputs nothing if the current repository is

--
https://github.com/git/git/pull/418

Re: Commit dropped when swapping commits with rebase -i -p

[Sebastian Schuberth]

On Sat, Sep 16, 2017 at 12:41 PM, Andreas Heiduk <ashei...@gmail.com> wrote:

> Therefore I would avoid "definitive wording" like "will drop" and use
> vague wording along "there are various dragons out there" like this:
>
> The todo list presented by `--preserve-merges --interactive` does
> not represent the topology of the revision graph.  Editing

I tried to avoid this introducing sentence from the original wording
as it reads like from a scientific research paper instead of from a
user's manual.

> commits and rewording their commit messages should work fine.
> But reordering, combining or dropping commits of a complex topology

There is no need for complex topology. If you reorder the two most
recent commits in a linear history, one gets dropped.

> can produce unexpected and useless results like missing commits,
> wrong merges, merges combining two unrelated histories and
> similar things.

"can produce" is much too soft, IMO. Reordering commits goes wrong,
period. Like wise "unexpected and useless results" is inappropriate.
The results are wrong in case of reordering, and wrong results are of
course unexpected and useless.

-- 
Sebastian Schuberth

Re: Commit dropped when swapping commits with rebase -i -p

[Sebastian Schuberth]

On 2017-09-02 02:04, Jonathan Nieder wrote:

>> Anyway, this should really more explicitly say *what* you need to know
>> about, that is, reordering commits does not work.
> 
> It tries to explain that, even with an example.  If you have ideas for
> improving the wording, that would be welcome.

As a first step, I indeed believe the wording must the stronger / clearer. How 
about this:

>From f69854ce7b9359603581317d152421ff6d89f345 Mon Sep 17 00:00:00 2001
From: Sebastian Schuberth <sschube...@gmail.com>
Date: Mon, 11 Sep 2017 10:41:27 +0200
Subject: [PATCH] docs: use a stronger wording when describing bugs with rebase 
-i -p

Signed-off-by: Sebastian Schuberth <sschube...@gmail.com>
---
 Documentation/git-rebase.txt | 9 +
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
index 6805a74aec..ccd0a04d54 100644
--- a/Documentation/git-rebase.txt
+++ b/Documentation/git-rebase.txt
@@ -782,10 +782,11 @@ case" recovery too!
 
 BUGS
 
-The todo list presented by `--preserve-merges --interactive` does not
-represent the topology of the revision graph.  Editing commits and
-rewording their commit messages should work fine, but attempts to
-reorder commits tend to produce counterintuitive results.
+Be careful when combining the `-i` / `--interactive` and `-p` /
+`--preserve-merges` options.  Reordering commits will drop commits from the
+main line. This is because the todo list does not represent the topology of the
+revision graph in this case.  However, editing commits and rewording their
+commit messages 'should' work fine.
 
 For example, an attempt to rearrange
 
-- 
2.14.1.windows.1

Re: Commit dropped when swapping commits with rebase -i -p

[Sebastian Schuberth]

On Wed, Aug 30, 2017 at 10:28 PM, Johannes Schindelin
<johannes.schinde...@gmx.de> wrote:

> Please see 'exchange two commits with -p' in t3404. This is a known

Thank for pointing out the test.

> breakage, and due to the fact that -p and -i are fundamentally
> incompatible with one another (even if -p's implementation was based on
> -i's). I never had in mind for -p to be allowed together with -i, and was
> against allowing it because of the design.

In any case, I wouldn't have expected *that* kind of side effect for
such a simple case (that does not involve any merge commits).

If these options are fundamentally incompatible as you say, would you
agree that it makes sense to disallow their usage together (instead of
just documenting that you should know what you're doing)?

-- 
Sebastian Schuberth

Re: Commit dropped when swapping commits with rebase -i -p

[Sebastian Schuberth]

On Wed, Aug 30, 2017 at 8:07 PM, Martin Ågren <martin.ag...@gmail.com> wrote:

> The man-page for git rebase says that combining -p with -i is "generally
> not a good idea unless you know what you are doing (see BUGS below)".

Thanks for pointing this out again. I remember to have read this some
time ago, but as I general consider myself to know what I'm doing, I
forgot about it :-)

Anyway, this should really more explicitly say *what* you need to know
about, that is, reordering commits does not work.

> So if you agree that a "dropped commit" is a "counterintuitive result",
> this is known and documented. Maybe the warning could be harsher, but it
> does say "unless you know what you are doing".

I'd say it's worse than counterintuitive, as counterintuitive might
still be correct, while in my case it clearly is not. So yes, the
warning must be harsher in my opinion. Maybe we should even abort
rebase -i-p if reordering of commits is detected.

-- 
Sebastian Schuberth

Commit dropped when swapping commits with rebase -i -p

[Sebastian Schuberth]

Hi,

I believe stumbled upon a nasty bug in Git: It looks like a commits gets 
dropped during interactive rebase when asking to preserve merges. Steps:

$ git clone -b git-bug --single-branch 
https://github.com/heremaps/scancode-toolkit.git
$ git rebase -i -p HEAD~2
# In the editor, swap the order of the two (non-merge) commits.
$ git diff origin/git-bug

The last command will show a non-empty diff which looks as if the "Do not 
shadow the os package with a variable name" commit has been dropped, and indeed 
"git log" confirms this. The commit should not get dropped, and it does not if 
just using "git rebase -i -p HEAD~2" (without "-p").

I'm observing this with Git 2.14.1 on Linux.

Regards,
Sebastian

Re: Best way to check whether working tree matches a commit's tree

[Sebastian Schuberth]

On Tue, Aug 22, 2017 at 9:34 PM, Junio C Hamano <gits...@pobox.com> wrote:

>> While this works, it feels sub-optimal. Is there a better / smarter way?
>
> I do not think so; you want three things to match and you have a way
> to compare two things at a time.

Right. I was just thinking if there's a lesser known command like "git
diff --no-index", but instead of taking two paths, take just one path
and a commit.

> By the way, I think your second check should compare
>
> rev-parse HEAD^{tree} $that_commit^{tree}
>
> as you are checking if the tree exactly matches.

In fact, I was considering to use "git diff HEAD $that_commit" as I
don't really care whether the SHA1s are equal, but just about the file
contents / tree.

-- 
Sebastian Schuberth

Best way to check whether working tree matches a commit's tree

[Sebastian Schuberth]

Hi,

I'd like to check whether my working tree exactly matches the tree of a given 
commit. That is, there should not be any untracked, staged or modified files 
(including ignored files).

Currently, I'm doing this in two steps:

- check for success and empty output of "git status --ignored --porcelain"
- check that the output of "git rev-parse HEAD" matches the given commit

While this works, it feels sub-optimal. Is there a better / smarter way?

-- 
Sebastian Schuberth

Re: [BUG] fast-export --anonymize does not maintain fixup! commits

[Sebastian Schuberth]

On Fri, May 12, 2017 at 11:22 AM, Jeff King <p...@peff.net> wrote:

>> these (or any other command prefixes in commit messages). Given that
>> the --anonymize option is explicitly designed to help reproducing
>> bugs, I consider this to be a bug in the --anonymize option itself.
>
> Yes, it probably should handle those prefixes.
>
> I don't know if I'd call it a bug. Maybe a missing feature. :)

I'd usually agree, but in this case, as I mentioned above, I consider
the missing feature to be so essential that the oversight to implement
it is actually a bug :-)

> So this seems like a good example of that. I think I'd prefer to see us
> add in known prefixes like "fixup!" and "squash!" then try to guess what
> other prefixes might be OK. I don't know of any other command prefixes
> besides those two, so maybe that's all you were suggesting.

Those were also the only two that came to my mind, but I wanted to
give some one who has a better overview the change to amend that list.

> It shouldn't be too hard to add. You'd probably need to make two
> adjustments to anonymize_commit_message():
>
>   1. Teach it to store the mapping of anonymized messages, using
>  anonymize_mem().
>
>   2. Parse "fixup! " and just anonymize_mem() the second half. I
>  think technically this wouldn't handle a fixup-of-fixup, but I
>  don't think rebase handles recursive ones anyway.

Thanks. I'll give it a try.

-- 
Sebastian Schuberth

Re: Possible bug in includeIf / conditional includes on non git initialised directories

[Sebastian Schuberth]

On 2017-05-11 20:53, Raphael Stolt wrote:

> I might have stumbled this time over a real bug in includeIf / conditional 
> includes or maybe it's just as intended.
> 1) Given I have a correct configured includeIf and I’m issuing `git config 
> --show-origin --get user.email` against an directory which hasn’t been `git 
> init`ed I get the user.email configured globally.

I don't think that's a bug surprise: The condition in the conditional include 
is "gitdir:". Before running "git init", it simply *is* no gitdir.

-- 
Sebastian Schuberth

[BUG] fast-export --anonymize does not maintain fixup! commits

[Sebastian Schuberth]

Hi,

I just tried to created an anonymized repo to allow the list to
reproduce a bug in "rebase -i" I discovered in Git 2.13 for Linux
(Windows is not affected). However, in order to reproduce the bug it's
important to keep the "fixup!" prefixes as part of the commit
messages. Unfortunately, "fast-export --anonymize" does not maintain
these (or any other command prefixes in commit messages). Given that
the --anonymize option is explicitly designed to help reproducing
bugs, I consider this to be a bug in the --anonymize option itself.

-- 
Sebastian Schuberth

Re: Possible bug in includeIf / conditional includes

[Sebastian Schuberth]

On 2017-05-10 19:00, raphael.st...@gmail.com wrote:


Current configuration which finds the conditional configuration.

~/.gitconfig
[includeIf "gitdir:~/Work/git-repos/oss/"]
   path = ~/Work/git-repos/oss/.oss-gitconfig

Expected configuration which doesn't find the conditional configuration:

~/.gitconfig
[includeIf "gitdir:~/Work/git-repos/oss/"]
   path = .oss-gitconfig


My guess is, because includeIf might contain other conditionals than 
"gitdir", the generic convention is to always use an absolute path for 
"path".


--
Sebastian Schuberth

Re: [PATCH] fetch-pack: always allow fetching of literal SHA1s

[Sebastian Schuberth]

On 2017-05-10 19:00, Jonathan Nieder wrote:


Right, makes sense.  I wondered if GitHub should be turning on
allowTipSHA1InWant, but it really doesn't make sense to. We _do_ hide
some internal refs[1], and they're things that users wouldn't want to
fetch. The problem for your case really is just on the client side, and
this patch fixes it.

[...]

[1] The reachability checks from upload-pack don't actually do much on
 GitHub, because you can generally access the objects via the API or
 the web site anyway. So I'm not really opposed to turning on
 allowTipSHA1InWant if it would be useful for users, but after
 Jonathan's patch I don't see how it would be.


Given that, what would make me really happy is if github enables
uploadpack.allowAnySHA1InWant.  That would be useful for me, at least.


Me too, BTW. If you're only interested in building / analyzing a 
specific SHA1 it's really a waste of network resources to fetch all of 
the history.


--
Sebastian Schuberth

Re: git-clone --config order & fetching extra refs during initial clone

[Sebastian Schuberth]

On 5/3/2017 22:22, Jeff King wrote:

>> My patch deals with 'remote..refspec', i.e. 'remote->fetch'.
>> Apparently some extra care is necessary for 'remote..tagOpt' and
>> 'remote->fetch_tags', too.  Perhaps there are more, I haven't checked
>> again, and maybe we'll add similar config variables in the future.  So
>> I don't think that dealing with such config variables one by one in
>> 'git clone', too, is the right long-term solution...  but perhaps it's
>> sufficient for the time being?
> 
> I think your patch is a strict improvement and we don't need to hold up
> waiting for a perfect fix (and because of the --single-branch thing you
> mentioned, this may be the best we can do anyway).

I agree. Let's fix one thing at a time, and not make this a overarching 
"account for all previously ignored config variables during clone" fix. Esp. as 
specifying the refspec is explicitly documented to work durig clone [1] I think 
we should get this in ASAP.

Thanks for your work so far!

[1] https://git-scm.com/docs/git-clone#git-clone---configltkeygtltvaluegt

Regards,
Sebastian

Cloning a custom refspec does not work as expected

[Sebastian Schuberth]

Hi,

I'm trying to clone a custom ref, avoiding the need to init && fetch
manually. From reading [1] which says:

"Set a configuration variable in the newly-created repository; this
takes effect immediately after the repository is initialized, but
before the remote history is fetched or any files checked out. [...]
This makes it safe, for example, to add additional fetch refspecs to
the origin remote."

I'd assume that this would work:

$ git clone -c remote.origin.fetch=+refs/changes/*:refs/remotes/origin/changes/*
https://gerrit.googlesource.com/git-repo

In fact, this *does* add the custom refs correct to .git/config, but
they are not fetched during the clone. I can manually fix that by
doing

$ cd git-repo
$ git fetch

afterwards, but the whole point is to avoid exactly that separate step.

Is this a code bug or documentation bug?

I'm using git version 2.12.2.windows.2.

[1] https://git-scm.com/docs/git-clone#git-clone---configltkeygtltvaluegt

-- 
Sebastian Schuberth

Re: [PATCH v2] git-gui--askpass: generalize the wording

[Sebastian Schuberth]

+ Pat

On 2017-04-27 08:38, Sebastian Schuberth wrote:


git-gui--askpass is not only used for SSH authentication, but also for
HTTPS. In that context it is confusing to only rfer to "OpenSSH", also
because another SSH client like PuTTY might be in use. So generalize
wording and also say which parent process, i.e. Git, requires
authentication.

Signed-off-by: Sebastian Schuberth <sschube...@gmail.com>
---
  git-gui/git-gui--askpass | 6 +++---
  1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/git-gui/git-gui--askpass b/git-gui/git-gui--askpass
index 4277f30..4e3f00d 100755
--- a/git-gui/git-gui--askpass
+++ b/git-gui/git-gui--askpass
@@ -2,7 +2,7 @@
  # Tcl ignores the next line -*- tcl -*- \
  exec wish "$0" -- "$@"
  
-# This is a trivial implementation of an SSH_ASKPASS handler.

+# This is a trivial implementation of an GIT_ASKPASS / SSH_ASKPASS handler.
  # Git-gui uses this script if none are already configured.
  
  package require Tk

@@ -12,7 +12,7 @@ set yesno  0
  set rc 255
  
  if {$argc < 1} {

-   set prompt "Enter your OpenSSH passphrase:"
+   set prompt "Enter your password / passphrase:"
  } else {
set prompt [join $argv " "]
if {[regexp -nocase {\(yes\/no\)\?\s*$} $prompt]} {
@@ -60,7 +60,7 @@ proc finish {} {
set ::rc 0
  }
  
-wm title . "OpenSSH"

+wm title . "Git Authentication"
  tk::PlaceWindow .
  vwait rc
  exit $rc




--
Sebastian Schuberth

[PATCH v2] git-gui--askpass: generalize the wording

[Sebastian Schuberth]

git-gui--askpass is not only used for SSH authentication, but also for
HTTPS. In that context it is confusing to only rfer to "OpenSSH", also
because another SSH client like PuTTY might be in use. So generalize
wording and also say which parent process, i.e. Git, requires
authentication.

Signed-off-by: Sebastian Schuberth <sschube...@gmail.com>
---
 git-gui/git-gui--askpass | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/git-gui/git-gui--askpass b/git-gui/git-gui--askpass
index 4277f30..4e3f00d 100755
--- a/git-gui/git-gui--askpass
+++ b/git-gui/git-gui--askpass
@@ -2,7 +2,7 @@
 # Tcl ignores the next line -*- tcl -*- \
 exec wish "$0" -- "$@"
 
-# This is a trivial implementation of an SSH_ASKPASS handler.
+# This is a trivial implementation of an GIT_ASKPASS / SSH_ASKPASS handler.
 # Git-gui uses this script if none are already configured.
 
 package require Tk
@@ -12,7 +12,7 @@ set yesno  0
 set rc 255
 
 if {$argc < 1} {
-   set prompt "Enter your OpenSSH passphrase:"
+   set prompt "Enter your password / passphrase:"
 } else {
set prompt [join $argv " "]
if {[regexp -nocase {\(yes\/no\)\?\s*$} $prompt]} {
@@ -60,7 +60,7 @@ proc finish {} {
set ::rc 0
 }
 
-wm title . "OpenSSH"
+wm title . "Git Authentication"
 tk::PlaceWindow .
 vwait rc
 exit $rc

--
https://github.com/git/git/pull/195

[PATCH] gitmodules: clarify the ignore option values

[Sebastian Schuberth]

Add more structure and describe each possible option in a self-contained
way, not referring to any of the previously described options.

Signed-off-by: Sebastian Schuberth <sschube...@gmail.com>
---
 Documentation/gitmodules.txt | 31 ---
 1 file changed, 20 insertions(+), 11 deletions(-)

diff --git a/Documentation/gitmodules.txt b/Documentation/gitmodules.txt
index 8f7c50f..4700624 100644
--- a/Documentation/gitmodules.txt
+++ b/Documentation/gitmodules.txt
@@ -66,17 +66,26 @@ submodule..fetchRecurseSubmodules::
 
 submodule..ignore::
Defines under what circumstances "git status" and the diff family show
-   a submodule as modified. When set to "all", it will never be considered
-   modified (but will nonetheless show up in the output of status and
-   commit when it has been staged), "dirty" will ignore all changes
-   to the submodules work tree and
-   takes only differences between the HEAD of the submodule and the commit
-   recorded in the superproject into account. "untracked" will additionally
-   let submodules with modified tracked files in their work tree show up.
-   Using "none" (the default when this option is not set) also shows
-   submodules that have untracked files in their work tree as changed.
-   If this option is also present in the submodules entry in .git/config of
-   the superproject, the setting there will override the one found in
+   a submodule as modified. The following values are supported:
+
+   all;; The submodule will never be considered modified (but will
+   nonetheless show up in the output of status and commit when it has
+   been staged).
+
+   dirty;; All changes to the submodule's work tree will be ignored, only
+   committed differences between the HEAD of the submodule and its
+   recorded state in the superproject are taken into account.
+
+   untracked;; Only untracked files in submodules will be ignored.
+   Committed differences and modifications to tracked files will show
+   up.
+
+   none;; No modifiations to submodules are ignored, all of committed
+   differences, and modifications to tracked and untracked files are
+   shown. This is the default option.
+
+   If this option is also present in the submodules entry in .git/config
+   of the superproject, the setting there will override the one found in
.gitmodules.
Both settings can be overridden on the command line by using the
"--ignore-submodule" option. The 'git submodule' commands are not

--
https://github.com/git/git/pull/348

[PATCH] gitmodules: clarify what history depth a shallow clone has

[Sebastian Schuberth]

Signed-off-by: Sebastian Schuberth <sschube...@gmail.com>
---
 Documentation/gitmodules.txt | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/Documentation/gitmodules.txt b/Documentation/gitmodules.txt
index 8f7c50f..6f39f24 100644
--- a/Documentation/gitmodules.txt
+++ b/Documentation/gitmodules.txt
@@ -84,8 +84,8 @@ submodule..ignore::
 
 submodule..shallow::
When set to true, a clone of this submodule will be performed as a
-   shallow clone unless the user explicitly asks for a non-shallow
-   clone.
+   shallow clone (with a history depth of 1) unless the user explicitly
+   asks for a non-shallow clone.
 
 
 EXAMPLES

--
https://github.com/git/git/pull/347

Re: Fwd: Failed shallow submodule clone for fixed SHA1 falls back to checking out HEAD

[Sebastian Schuberth]

On 2017-04-18 21:12, Stefan Beller wrote:


Wouldn't it make more sense to unshallow the submodule clone in this case and 
checkout the configured commit 2aa4cce7d7fd46164030f2b4d244c4b89e77f722 
afterwards?


If I remember correctly the conclusion of the discussion at the time
was that people are more interested in "less data" rather than
"correct data" because otherwise you would not have asked for shallow.


I do believe this is an awkward choice. What does it help to get less 
data if it's the wrong data?


--
Sebastian Schuberth

Failed shallow submodule clone for fixed SHA1 falls back to checking out HEAD

[Sebastian Schuberth]

Hi,

this is using "git version 2.12.2.windows.2" on Windows / "git version 
2.12.2-639-g584f897" on Linux.

I have configured my superproject with .gitmodules saying

---8<---
[submodule "src/funTest/resources/projects/external/jgnash"]
   path = src/funTest/resources/projects/external/jgnash
   url = https://github.com/ccavanaugh/jgnash.git
   shallow = true
---8<---

and configured the submodule to checkout commit 
2aa4cce7d7fd46164030f2b4d244c4b89e77f722 [1]. When doing a fresh clone of my 
superproject via "git clone --recursive" I get

---8<---
error: Server does not allow request for unadvertised object 
2aa4cce7d7fd46164030f2b4d244c4b89e77f722
Fetched in submodule path 'src/funTest/resources/projects/external/jgnash', but 
it did not contain 2aa4cce7d7fd464030f2b4d244c4b89e77f722. Direct fetching of 
that commit failed.
---8<---

So far so good, it simply seems that GitHub does not support 
allowReachableSHA1InWant [2]. The interesting bit is that my submodule checkout 
still ends up being shallow, but poiting to HEAD:

---8<---
$ cd src/funTest/resources/projects/external/jgnash
$ git log
commit 12036fffb6c620515edd96416363fd1749b5d003 (grafted, HEAD -> master, 
origin/master, origin/HEAD)
Author: Craig Cavanaugh 
Date:   Tue Apr 18 05:33:06 2017 -0400

Fix typos
---8<---

Wouldn't it make more sense to unshallow the submodule clone in this case and 
checkout the configured commit 2aa4cce7d7fd46164030f2b4d244c4b89e77f722 
afterwards? At least I'd be getting what I asked for in that case, even if at 
the cost of additional network traffic. Because if I read [3] correctly, the 
command line option belonging to "submodule..shallow" is called 
"--[no-]recommend-shallow", i.e. it's only a recommendation, to falling back to 
a full clone should be fine.

[1] 
https://github.com/ccavanaugh/jgnash/commit/2aa4cce7d7fd46164030f2b4d244c4b89e77f722
[2] 
https://git-scm.com/docs/git-config#git-config-uploadpackallowReachableSHA1InWant
[3] https://git-scm.com/docs/git-submodule

Regards,
Sebastian

Re: [PATCH] submodule: remove a superfluous second check for the "new" variable

[Sebastian Schuberth]

On 2017-04-17 20:02, Stefan Beller wrote:

>> diff --git a/submodule.c b/submodule.c
>> index c52d663..68623bd 100644
>> --- a/submodule.c
>> +++ b/submodule.c
>> @@ -1396,8 +1396,7 @@ int submodule_move_head(const char *path,
>>  cp1.no_stdin = 1;
>>  cp1.dir = path;
>>
>> -   argv_array_pushl(, "update-ref", "HEAD",
>> -new ? new : EMPTY_TREE_SHA1_HEX, 
>> NULL);
>> +   argv_array_pushl(, "update-ref", "HEAD", 
>> new, NULL);
> 
> EMPTY_TREE_SHA1_HEX != NULL?
> 
> Can you clarify the intent in the commit message?

Sure. A few lines up (3 lines out of the diff) we have "if (new) {" [1], thus 
there's no need to check "new != NULL" here again.

[1] 
https://github.com/git/git/pull/345/files#diff-471db3ea6697763218bb8335a95ece57R1392

-- 
Sebastian Schuberth

Re: Feature request: --format=json

[Sebastian Schuberth]

On 2017-04-17 14:44, Fred .Flintstone wrote:


However, if I want something more suitable for machine parsing, is
there any way to get that output?


Instead of machine parsing, why not directly get what you want via 
libgit2 (or one of its language bindings), or jgit?


[1] https://github.com/libgit2/libgit2
[2] https://github.com/eclipse/jgit

--
Sebastian Schuberth

[PATCH] submodule: remove a superfluous second check for the "new" variable

[Sebastian Schuberth]

Signed-off-by: Sebastian Schuberth <sschube...@gmail.com>
---
 submodule.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/submodule.c b/submodule.c
index c52d663..68623bd 100644
--- a/submodule.c
+++ b/submodule.c
@@ -1396,8 +1396,7 @@ int submodule_move_head(const char *path,
cp1.no_stdin = 1;
cp1.dir = path;
 
-   argv_array_pushl(, "update-ref", "HEAD",
-new ? new : EMPTY_TREE_SHA1_HEX, NULL);
+   argv_array_pushl(, "update-ref", "HEAD", new, 
NULL);
 
if (run_command()) {
ret = -1;

--
https://github.com/git/git/pull/345

[PATCH] sha1_file: remove an used fd variable

[Sebastian Schuberth]

Signed-off-by: Sebastian Schuberth <sschube...@gmail.com>
---
 sha1_file.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/sha1_file.c b/sha1_file.c
index 7106389..9ecf71f 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -3970,7 +3970,6 @@ int read_loose_object(const char *path,
  void **contents)
 {
int ret = -1;
-   int fd = -1;
void *map = NULL;
unsigned long mapsize;
git_zstream stream;
@@ -4020,7 +4019,5 @@ int read_loose_object(const char *path,
 out:
if (map)
munmap(map, mapsize);
-   if (fd >= 0)
-   close(fd);
return ret;
 }

--
https://github.com/git/git/pull/344

Re: [PATCH v1] travis-ci: add static analysis build job to run coccicheck

[Sebastian Schuberth]

On 2017-04-11 09:26, Lars Schneider wrote:


Add a dedicated build job for static analysis. As a starter we only run
coccicheck but in the future we could run Clang Static Analyzer or
similar tools, too.


Just FYI, some time ago someone (I don't recall who) signed up Git with 
Coverity's free scan service for OSS projects:


https://scan.coverity.com/projects/git?tab=overview

Maybe it makes sense to at least link to this page, too?

--
Sebastian Schuberth

Re: [PATCH v2] travis-ci: build and test Git on Windows

[Sebastian Schuberth]

On Fri, Mar 24, 2017 at 1:35 PM, Lars Schneider
<larsxschnei...@gmail.com> wrote:

>> 1. use appveyor.com, as that is a Travis-like service for Windows. We do our
>>   windows-builds in the curl project using that.
>
> The Git for Windows build and tests are *really* resources intensive and they
> take a lot of setup time. AFAIK we would run into timeouts with AppVeyor.
> Maybe Sebastian or Dscho know details?

At lot of setup time, in terms of installing the Git for Windows SDK,
could probably be saved by only installing the required packages on
top of MSYS2 that's already installed in AppVeyor nodes [1].

[1] https://www.appveyor.com/docs/build-environment/#mingw-msys-cygwin

-- 
Sebastian Schuberth

Re: [PATCH 1/3] t7504: document regression: reword no longer calls commit-msg

[Sebastian Schuberth]

On Thu, Mar 23, 2017 at 3:43 PM, Johannes Schindelin
<johannes.schinde...@gmx.de> wrote:

> I know Sebastian well, and I would hope that he lends his substantial
> competence to making sure that the changes that affect end users are
> correct and that I do not introduce another regression.

If you'd really know me, you would also know that I only say something
*if* I have to say something. I.e. me not making any remarks regarding
the C code means I did not find any issues.

> Besides, he is German, so I tried to spell it out clearly what I wish him
> to do in return for my addressing his problem.

I fail to see how this is "my" problem just because I happened to
notice it first. While I'm grateful that you've addressed it timely, I
believe this is a naturalness since you've introduced the regression.

-- 
Sebastian Schuberth

Re: [PATCH 1/3] t7504: document regression: reword no longer calls commit-msg

[Sebastian Schuberth]

On Wed, Mar 22, 2017 at 4:01 PM, Johannes Schindelin
<johannes.schinde...@gmx.de> wrote:

> Noticed by Sebastian Schuberth.

Thanks for working on the fix.

> +# set up fake editor to replace `pick` by `reword`
> +cat > reword-editor <<'EOF'
> +#!/bin/sh
> +mv "$1" "$1".bup &&
> +sed 's/^pick/reword/' <"$1".bup >"$1"
> +EOF

Maybe use

sed -i 's/^pick/reword/' "$1"

here to avoid renaming the input file? Not sure how portable -i for
sed is, though. Otherwise, maybe remove the file "$1".bup afterwards
to be clean?

-- 
Sebastian Schuberth

Re: Regression: Reword during rebase -i does not seem to trigger commit-msg hook anymore

[Sebastian Schuberth]

On 3/20/2017 18:43, Stefan Beller wrote:


With this version, when I do a "reword" during an inteactive rebase session,
the commit-msg hook doe snot seem to be triggered anymore. This was
particularly useful to let Gerrit's commit-msg hook add missing ChangeId
footers.

I can work-aorund the issue by doing an "edit" and "commit --amend".

Is anyone else seeing this?


In 2.12 RelNotes:

 * "git rebase -i" starts using the recently updated "sequencer" code.


Thanks, that looks like a possible culprit. I can *not* reproduce the 
issue on Linux with "git version 2.12.1-382-gc0f9c70", thought. So 
either it's Windows-specific, or it has been fixed in 2.12.1 already.


Regards,
Sebastian

Regression: Reword during rebase -i does not seem to trigger commit-msg hook anymore

[Sebastian Schuberth]

Hi,

I'm using

$ git --version
git version 2.12.0.windows.1

With this version, when I do a "reword" during an inteactive rebase 
session, the commit-msg hook doe snot seem to be triggered anymore. This 
was particularly useful to let Gerrit's commit-msg hook add missing 
ChangeId footers.


I can work-aorund the issue by doing an "edit" and "commit --amend".

Is anyone else seeing this?

Regards,
Sebastian

Re: diff.ignoreSubmoudles config setting broken?

[Sebastian Schuberth]

On Wed, Mar 8, 2017 at 9:54 PM, Junio C Hamano <gits...@pobox.com> wrote:

> Between these two:
>
> git -c diff.ignoresubmodules=all diff
> git diff --ignore-submodules=all
>
> one difference is that the latter disables reading extra config from
> .gitmodules (!) while the former makes the command honor it.
>
> This comes from aee9c7d6 ("Submodules: Add the new "ignore" config
> option for diff and status", 2010-08-06), which is ancient and
> predates even v2.0, so if you see problems with v2.12 or v2.11 but
> not with older ones, that would rule out this theory.

Thanks for reminding me of possible settings .gitmodules. Indeed I have

[submodule "scanners/scancode-toolkit"]
path = scanners/scancode-toolkit
url = https://github.com/sschuberth/scancode-toolkit.git
ignore = untracked

in .gitmodules, which explains why globally setting
"diff.ignoreSubmodules" to "all" had no effect.

-- 
Sebastian Schuberth

Re: diff.ignoreSubmoudles config setting broken?

[Sebastian Schuberth]

On Wed, Mar 8, 2017 at 2:33 PM, Jeff King <p...@peff.net> wrote:

>> I'm getting
>>
>> $ git config --global diff.ignoreSubmodules all
>> $ git diff
>> diff --git a/scanners/scancode-toolkit b/scanners/scancode-toolkit
>> index 65e5c9c..6b021a8 16
>> --- a/scanners/scancode-toolkit
>> +++ b/scanners/scancode-toolkit
>> @@ -1 +1 @@
>> -Subproject commit 65e5c9c9508441c5f62beff4749cf455c6eadc30
>> +Subproject commit 6b021a8addf6d3c5f2a6ef1af6245e095c21d8ec
>>
>> but with
>>
>> $ git diff --ignore-submodules=all
>
> Hrm. Isn't "all" the default? That's what git-diff(1) says (but I've
> never used the feature myself).
>
> That would imply to me that there's another config option set somewhere
> (perhaps in the repo-level config). What does:
>
>   git config --show-origin --get-all diff.ignoresubmodules
>
> say?

It says:

file:/home/seschube/.gitconfig  all

-- 
Sebastian Schuberth

Re: diff.ignoreSubmoudles config setting broken?

[Sebastian Schuberth]

On Wed, Mar 8, 2017 at 3:01 PM, Jeff King <p...@peff.net> wrote:

>> > Hrm. Isn't "all" the default? That's what git-diff(1) says (but I've
>> > never used the feature myself).
>> >
>> > That would imply to me that there's another config option set somewhere
>> > (perhaps in the repo-level config). What does:
>> >
>> >   git config --show-origin --get-all diff.ignoresubmodules
>> >
>> > say?
>>
>> It says:
>>
>> file:/home/seschube/.gitconfig  all
>
> OK, that looks right, so my guess is probably the wrong direction.
> Peeking at the code, it looks like there may be some per-submodule
> magic, but I don't know how it all works. So I'll stop looking and wait
> for somebody more clueful to respond.

+ Jens

-- 
Sebastian Schuberth

diff.ignoreSubmoudles config setting broken?

[Sebastian Schuberth]

Hi,

with

$ git --version
git version 2.12.0.windows.1

I'm getting

$ git config --global diff.ignoreSubmodules all
$ git diff
diff --git a/scanners/scancode-toolkit b/scanners/scancode-toolkit
index 65e5c9c..6b021a8 16
--- a/scanners/scancode-toolkit
+++ b/scanners/scancode-toolkit
@@ -1 +1 @@
-Subproject commit 65e5c9c9508441c5f62beff4749cf455c6eadc30
+Subproject commit 6b021a8addf6d3c5f2a6ef1af6245e095c21d8ec

but with

$ git diff --ignore-submodules=all

I'm getting the expected empty output.

I can reproduce the same on Linux with "git version 2.11.0". Am I missing 
something, or is this a bug?

Regards,
Sebastian

Re: [RESEND PATCH] git-gui--askpass: generalize the window title

[Sebastian Schuberth]

On Tue, Mar 7, 2017 at 7:30 PM, Stefan Beller <sbel...@google.com> wrote:

> Although the following are included in git.git repository, they have their
> own authoritative repository and maintainers:

Thanks. I continuously get confused by this fact.

-- 
Sebastian Schuberth

[RESEND PATCH] git-gui--askpass: generalize the window title

[Sebastian Schuberth]

git-gui--askpass is not only used for SSH authentication, but also for
HTTPS. In that context it is confusing to have a window title of
"OpenSSH". So generalize the title so that it also says which parent
process, i.e. Git, requires authentication.

Signed-off-by: Sebastian Schuberth <sschube...@gmail.com>
---
 git-gui/git-gui--askpass | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/git-gui/git-gui--askpass b/git-gui/git-gui--askpass
index 4277f30..1e5c3256 100755
--- a/git-gui/git-gui--askpass
+++ b/git-gui/git-gui--askpass
@@ -60,7 +60,7 @@ proc finish {} {
set ::rc 0
 }
 
-wm title . "OpenSSH"
+wm title . "Git Authentication"
 tk::PlaceWindow .
 vwait rc
 exit $rc

--
https://github.com/git/git/pull/195

Re: [PATCH] Do not require Python for the git-remote-{bzr,hg} placeholder scripts

[Sebastian Schuberth]

On Fri, Mar 3, 2017 at 8:10 PM, Junio C Hamano <gits...@pobox.com> wrote:

>> Just a niggle:  This change moves the warning message from stderr to stdout.
>
> Right.  Here is what I'll queue.

Indeed, thanks for the note, and also Junio for fixing while queuing.

-- 
Sebastian Schuberth

[PATCH] Do not require Python for the git-remote-{bzr,hg} placeholder scripts

[Sebastian Schuberth]

It does not make sense for these placeholder scripts to depend on Python
just because the real scripts do. At the example of Git for Windows, we
would not even be able to see those warnings as it does not ship with
Python. So just use plain shell scripts instead.

Signed-off-by: Sebastian Schuberth <sschube...@gmail.com>
---
 contrib/remote-helpers/git-remote-bzr | 16 +++-
 contrib/remote-helpers/git-remote-hg  | 16 +++-
 2 files changed, 14 insertions(+), 18 deletions(-)

diff --git a/contrib/remote-helpers/git-remote-bzr 
b/contrib/remote-helpers/git-remote-bzr
index 712a137..ccc4aea 100755
--- a/contrib/remote-helpers/git-remote-bzr
+++ b/contrib/remote-helpers/git-remote-bzr
@@ -1,13 +1,11 @@
-#!/usr/bin/env python
+#!/bin/sh
 
-import sys
-
-sys.stderr.write('WARNING: git-remote-bzr is now maintained independently.\n')
-sys.stderr.write('WARNING: For more information visit 
https://github.com/felipec/git-remote-bzr\n')
-
-sys.stderr.write('''WARNING:
+cat <<'EOT'
+WARNING: git-remote-bzr is now maintained independently.
+WARNING: For more information visit https://github.com/felipec/git-remote-bzr
+WARNING:
 WARNING: You can pick a directory on your $PATH and download it, e.g.:
-WARNING:   $ wget -O $HOME/bin/git-remote-bzr \\
+WARNING:   $ wget -O $HOME/bin/git-remote-bzr \
 WARNING: 
https://raw.github.com/felipec/git-remote-bzr/master/git-remote-bzr
 WARNING:   $ chmod +x $HOME/bin/git-remote-bzr
-''')
+EOT
diff --git a/contrib/remote-helpers/git-remote-hg 
b/contrib/remote-helpers/git-remote-hg
index 4255ad6..dfda44f 100755
--- a/contrib/remote-helpers/git-remote-hg
+++ b/contrib/remote-helpers/git-remote-hg
@@ -1,13 +1,11 @@
-#!/usr/bin/env python
+#!/bin/sh
 
-import sys
-
-sys.stderr.write('WARNING: git-remote-hg is now maintained independently.\n')
-sys.stderr.write('WARNING: For more information visit 
https://github.com/felipec/git-remote-hg\n')
-
-sys.stderr.write('''WARNING:
+cat <<'EOT'
+WARNING: git-remote-hg is now maintained independently.
+WARNING: For more information visit https://github.com/felipec/git-remote-hg
+WARNING:
 WARNING: You can pick a directory on your $PATH and download it, e.g.:
-WARNING:   $ wget -O $HOME/bin/git-remote-hg \\
+WARNING:   $ wget -O $HOME/bin/git-remote-hg \
 WARNING: https://raw.github.com/felipec/git-remote-hg/master/git-remote-hg
 WARNING:   $ chmod +x $HOME/bin/git-remote-hg
-''')
+EOT

--
https://github.com/git/git/pull/333

Re: [PATCH v4 0/3] git-p4: fix Git LFS pointer parsing

[Sebastian Schuberth]

On Thu, Apr 28, 2016 at 8:26 AM,   wrote:

> diff to v3:
> * fix missing assignment of pointerFile variable
>   ($gmane/292454, thanks Sebastian for making me aware)
> * fix s/brake/break/ in commit message
>   ($gmane/292451, thanks Eric)

The series looks good to me now.

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

Re: [PATCH v3 3/3] git-p4: fix Git LFS pointer parsing

[Sebastian Schuberth]

On Mon, Apr 25, 2016 at 9:33 AM, Lars Schneider
<larsxschnei...@gmail.com> wrote:

>> if you're omitting lines from the output. Also, the regex matches
>> against the whole multi-line string. That is, if the file for some
>> reason was ending in '\n\n' instead of just '\n', the '.*' would match
>> almost all content of the pointer file, not just the remains of the
>> preamble. One way to fix this would be to use
>>
>> re.sub(r'Git LFS pointer for [^\n]+\n\n', '', pointerFile)
>>
>> instead.
>
> In general you are right as "*" is greedy. However, in Python "." matches any
> character except a newline [1]. Therefore I think the regex is correct.

Ah, thanks for pointing that out. Looks ok to me then.

> Nevertheless... thanks for making me read the line again. I forgot to
> assign the pointerFile variable in the version I sent around :-(
>
> This is how it should be:
>
> pointerFile = re.sub(r'Git LFS pointer for.*\n\n', '', pointerFile)

Right. Good you've catched that!

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

Re: [PATCH v3 3/3] git-p4: fix Git LFS pointer parsing

[Sebastian Schuberth]

On Sun, Apr 24, 2016 at 8:58 PM,   wrote:

> --- a/git-p4.py
> +++ b/git-p4.py
> @@ -1064,8 +1064,15 @@ class GitLFS(LargeFileSystem):
>  if pointerProcess.wait():
>  os.remove(contentFile)
>  die('git-lfs pointer command failed. Did you install the 
> extension?')
> -pointerContents = [i+'\n' for i in pointerFile.split('\n')[2:][:-1]]
> -oid = pointerContents[1].split(' ')[1].split(':')[1][:-1]
> +
> +# Git LFS removed the preamble in the output of the 'pointer' command
> +# starting from version 1.2.0. Check for the preamble here to support
> +# earlier versions.
> +# c.f. 
> https://github.com/github/git-lfs/commit/da2935d9a739592bc775c98d8ef4df9c72ea3b43
> +if pointerFile.startswith('Git LFS pointer for'):
> +re.sub(r'Git LFS pointer for.*\n\n', '', pointerFile)

I liked the code from v2 better. I know Ben said "there could be
expansions or other modifications applied by git-lfs between input and
output", but I believe it's better to be too strict than too lenient
if you're omitting lines from the output. Also, the regex matches
against the whole multi-line string. That is, if the file for some
reason was ending in '\n\n' instead of just '\n', the '.*' would match
almost all content of the pointer file, not just the remains of the
preamble. One way to fix this would be to use

re.sub(r'Git LFS pointer for [^\n]+\n\n', '', pointerFile)

instead.

Regards,
Sebastian
--
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] git-p4: fix Git LFS pointer parsing

[Sebastian Schuberth]

On Fri, Apr 22, 2016 at 9:53 AM, Lars Schneider
<larsxschnei...@gmail.com> wrote:

> What would be the best way forward? A v3 with a better commit message
> mentioning the array -> string change?

I'd vote for that, yes. Also v3 could then properly incorporate my regexp.

-- 
Sebastian Schuberth
--
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 v1 2/2] git-p4: fix Git LFS pointer parsing

[Sebastian Schuberth]

On Wed, Apr 20, 2016 at 5:30 PM, Junio C Hamano <gits...@pobox.com> wrote:

>> If clients rely on output targeted at human consumption it's not
>> surprising that these clients need to be adjusted from time to time.
>> What's troubling is not the change to git-lfs, but the very un-generic
>> way git-p4 is implemented.
>
> Sounds like the subcommand they are using is not meant for
> scripting?  What is the kosher way to get at the information they
> can use that is a supported interface for scripters?

The "pointer" subcommand indeed is listed under "Low level commands"
by "git lfs" (without any arguments), and as such it probably can be
considered for scripting use. However, before my fix in [1] the
subcommand was printing both output targeted at humans and output
targeted at scripts to stdout. After my fix, only output targeted at
script goes to stdout, and output targeted at humans goes to stderr.

[1] https://github.com/github/git-lfs/pull/1105

-- 
Sebastian Schuberth
--
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] git-p4: fix Git LFS pointer parsing

[Sebastian Schuberth]

On Wed, Apr 20, 2016 at 10:10 AM,  <larsxschnei...@gmail.com> wrote:

> --- a/git-p4.py
> +++ b/git-p4.py
> @@ -1064,8 +1064,17 @@ class GitLFS(LargeFileSystem):
>  if pointerProcess.wait():
>  os.remove(contentFile)
>  die('git-lfs pointer command failed. Did you install the 
> extension?')
> -pointerContents = [i+'\n' for i in pointerFile.split('\n')[2:][:-1]]
> -oid = pointerContents[1].split(' ')[1].split(':')[1][:-1]
> +
> +# Git LFS removed the preamble in the output of the 'pointer' command

git-lfs did not remove the output. It simply goes to stderr instead of
stdout now. That said, could a fix simply be to capture both stdout
and sterr? If the output to the streams remain interleaved it should
look exactly like before.

> +# starting from version 1.2.0. Check for the preamble here to support
> +# earlier versions.
> +# c.f. 
> https://github.com/github/git-lfs/commit/da2935d9a739592bc775c98d8ef4df9c72ea3b43
> +preamble = 'Git LFS pointer for ' + contentFile + '\n\n'
> +if pointerFile.startswith(preamble):
> +pointerFile = pointerFile[len(preamble):]
> +
> +oidEntry = [i for i in pointerFile.split('\n') if 
> i.startswith('oid')]
> +oid = oidEntry[0].split(' ')[1].split(':')[1]

Why do we need to remove the preamble at all, if present? If all we
want is the oid, we should simply only look at the line that starts
with that keyword, which would skip any preamble. Which is what you
already do here. However, I'd probably use .splitlines() instead of
.split('\n') and .startswith('oid ') (note the trailing space) instead
of .startswith('oid') to ensure "oid" is a separate word.

But then again, I wonder why there's so much split() logic involved in
extracting the oid. Couldn't we replace all of that with a regexp like

oid = re.search(r"^oid \w+:(\w+)", pointerFile, re.MULTILINE).group(1)

-- 
Sebastian Schuberth
--
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 v1 2/2] git-p4: fix Git LFS pointer parsing

[Sebastian Schuberth]

On Tue, Apr 19, 2016 at 11:04 PM, Junio C Hamano <gits...@pobox.com> wrote:

>> I dropped the support for the older version to keep the code as
>> simple as possible (plus it would be cumbersome to test with an
>> outdated Git LFS version). Since it is probably a niche feature I
>> thought that might be acceptable.
>
> It is bad enough that clients need to be adjusted at all in the
> first place, but I would have found it very troubling if the
> problematic change to LFS thing were made in such a way that it
> makes backward compatible adjustment on the client code impossible.

If clients rely on output targeted at human consumption it's not
surprising that these clients need to be adjusted from time to time.
What's troubling is not the change to git-lfs, but the very un-generic
way git-p4 is implemented.

-- 
Sebastian Schuberth
--
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] doc: Clarify which objects notes can be attached to

[Sebastian Schuberth]

Explicitly name the supported object types, and ensure patches cannot be
misinterpreted as non-objects that can have notes attached.

Signed-off-by: Sebastian Schuberth <sschube...@gmail.com>
---
 Documentation/git-notes.txt | 9 +
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/Documentation/git-notes.txt b/Documentation/git-notes.txt
index 8de3499..101e6ba 100644
--- a/Documentation/git-notes.txt
+++ b/Documentation/git-notes.txt
@@ -25,7 +25,8 @@ SYNOPSIS
 DESCRIPTION
 ---
 Adds, removes, or reads notes attached to objects, without touching
-the objects themselves.
+the objects themselves.  Supported objects are commits, blobs, trees
+and annotated tags.
 
 By default, notes are saved to and read from `refs/notes/commits`, but
 this default can be overridden.  See the OPTIONS, CONFIGURATION, and
@@ -39,9 +40,9 @@ message stored in the commit object, the notes are indented 
like the
 message, after an unindented line saying "Notes ():" (or
 "Notes:" for `refs/notes/commits`).
 
-Notes can also be added to patches prepared with `git format-patch` by
-using the `--notes` option. Such notes are added as a patch commentary
-after a three dash separator line.
+Notes contents can also be included in patches prepared with
+`git format-patch` by using the `--notes` option. Such notes are added
+as a patch commentary after a three dash separator line.
 
 To change which notes are shown by 'git log', see the
 "notes.displayRef" configuration in linkgit:git-log[1].
-- 
2.8.0.windows.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] doc: clarify that notes can be attached to any type of stored object

[Sebastian Schuberth]

On Fri, Apr 1, 2016 at 6:47 PM, Junio C Hamano <gits...@pobox.com> wrote:

> DESCRIPTION
> ---
> Adds, removes, or reads notes attached to objects, without touching
> the objects themselves.
>
> This says "notes attached to objects" and "the objects themselves".
> They do not limit the target of attaching a note to "commits".
> So this may be the place to add "  Note that notes can be attached
> to any kind of objects, not limited to commits" or something, if
> we really wanted to.

Done, I'll send a patch shortly. But I wanted to list the supported
object types explicitly, in particular as many guide in the net are
unclear that only annotated tags are object, and lightweight ones are
not.

> Notes can also be added to patches prepared with `git format-patch` by
> using the `--notes` option. Such notes are added as a patch commentary
> after a three dash separator line.
>
> This paragraph _might_ be confusing to new readers.  The "added to"
> sounds as if you are attaching a note to a non-object which is a
> patch.  But this "add" is about "inserting the contents of the note
> attached to the commit being formatted" and corresponds to "can be
> shown by" in the previous paragraph.  We may want to avoid the verb
> "add" when talking about the use of data stored in an existing note
> to somewhere else like this.

Done.

> add::
> Add notes for a given object (defaults to HEAD). Abort if the
>
> And this "Add notes for " should probably be reworded to "Attach
> notes to" to match the first sentence in the description.

After a bit of thinking, I don't believe we should do this. All
subcommand docs start with the verb the subcommand is named after. In
that sense making the "add" docs start with "Attach" would be
inconsistent and probably raise the question why the subcommand is not
called "attach" after all. Also, in the description it says "Adds,
removes, or reads notes attached to objects", so it includes "[...]
removes [...] notes attached to objects", and if you read it like this
the word "attach" is not specific to the "add" subcommand. So I left
this as-is in my patch.

-- 
Sebastian Schuberth
--
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] doc: clarify that notes can be attached to any type of stored object

[Sebastian Schuberth]

On Fri, Apr 1, 2016 at 5:31 PM, Junio C Hamano <gits...@pobox.com> wrote:

> Sebastian Schuberth <sschube...@gmail.com> writes:
>
>> Signed-off-by: Sebastian Schuberth <sschube...@gmail.com>
>> ---
>>  Documentation/git-notes.txt | 5 +++--
>>  1 file changed, 3 insertions(+), 2 deletions(-)
>>
>> diff --git a/Documentation/git-notes.txt b/Documentation/git-notes.txt
>> index 8de3499..5375d98 100644
>> --- a/Documentation/git-notes.txt
>> +++ b/Documentation/git-notes.txt
>> @@ -234,8 +234,9 @@ which operation triggered the update, and the commit 
>> authorship is
>>  determined according to the usual rules (see linkgit:git-commit[1]).
>>  These details may change in the future.
>>
>> -It is also permitted for a notes ref to point directly to a tree
>> -object, in which case the history of the notes can be read with
>> +It is also permitted for a notes ref to point to any other object in
>> +the object store besides commit objects, that is annotated tags, blobs
>> +or trees. For the latter, the history of the notes can be read with
>>  `git log -p -g `.
>
> I do not think this is correct place to patch.  The original is not
> talking about what objects can have notes attached at all.  What it
> explains is this.

Thanks for the explanation, I was indeed misreading this. I'll try to
clarify this section then, too. In order to do so, I think we should
mention how to actually create a  that directly points to a
tree instead of a commit for the history of notes. Would you have an
example how to do that?

-- 
Sebastian Schuberth
--
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 is an efficient way to get all blobs / trees that have notes attached?

[Sebastian Schuberth]

On Fri, Apr 1, 2016 at 2:16 PM, Johan Herland <jo...@herland.net> wrote:

>> 3) Recursively list all blobs / trees (git-ls-tree) and look whether an
>> object's hash is conatined in our table to get its notes.
>>
>> In particular 3) could be expensive for repos with a lot of files as we're
>> looking at all of them just to see whether they have notes attached.
>
> In (3), why would you need to search through _all_ blobs/trees? Would
> it not be cheaper to simply query the object type of each annotated
> object from (2)? I.e. something like:
>
> for notes_ref in $(git for-each-ref refs/notes | cut -c 49-)
> do
> echo "--- $notes_ref ---"
> for annotated_obj in $(git notes --ref=$notes_ref list | cut -c 41-)
> do
> type=$(git cat-file -t "$annotated_obj")
> if test "$type" != "commit"
> then
> echo "$annotated_obj: $type"
> fi
> done
> done

Thanks for the idea. The problem is that I do want to list the notes
by path of the object they belong to. As a blob could potentially
belong to more than one path (copies of files in the repo), I do not
see another way of getting that information other than iterating over
all blobs and checking what path(s) they belong to.

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

What is an efficient way to get all blobs / trees that have notes attached?

[Sebastian Schuberth]

Hi,

I'm curious whether there's a more efficient way to get a list of blobs 
/ trees (and their names) that have notes attached than doing this:


1) Get all notes refs I'm interested in (git-for-each-ref).

2) For each notes ref, get the list of notes (git-notes list) and store 
them in a hash table that maps object hashes to notes.


3) Recursively list all blobs / trees (git-ls-tree) and look whether an 
object's hash is conatined in our table to get its notes.


In particular 3) could be expensive for repos with a lot of files as 
we're looking at all of them just to see whether they have notes attached.


Regards,
Sebastian



--
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] doc: clarify that notes can be attached to any type of stored object

[Sebastian Schuberth]

Signed-off-by: Sebastian Schuberth <sschube...@gmail.com>
---
 Documentation/git-notes.txt | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/Documentation/git-notes.txt b/Documentation/git-notes.txt
index 8de3499..5375d98 100644
--- a/Documentation/git-notes.txt
+++ b/Documentation/git-notes.txt
@@ -234,8 +234,9 @@ which operation triggered the update, and the commit 
authorship is
 determined according to the usual rules (see linkgit:git-commit[1]).
 These details may change in the future.
 
-It is also permitted for a notes ref to point directly to a tree
-object, in which case the history of the notes can be read with
+It is also permitted for a notes ref to point to any other object in
+the object store besides commit objects, that is annotated tags, blobs
+or trees. For the latter, the history of the notes can be read with
 `git log -p -g `.
 
 
-- 
2.8.0.windows.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: Trouble with cat-file on tags

[Sebastian Schuberth]

On 4/1/2016 11:32, Sebastian Schuberth wrote:


However, I still get information about the commit oject iintsead of the
tag object. Is this expected?


Solved this one, too: Yes it is. I was misreading the docs:

"If  is specified, the raw (though uncompressed) contents of the 
 will be returned."


This means

$ git cat-file tag refs/tags/v0.1.2

displays the *contents* of the tag, not the tag itself. Which leads me 
to the next question: For a given name of an annotated tag, how to get 
the hash of the tag object? The solution I found for now:


$ git show-ref --tags -- v0.1.2
92b67e2b0626519ef8cd4e9cacb2bdafba6d53f0 refs/tags/v0.1.2

Regards,
Sebastian



--
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: Trouble with cat-file on tags

[Sebastian Schuberth]

On 4/1/2016 11:26, Sebastian Schuberth wrote:


---8<---
$ git tag test-tag

$ git tag -l
test-tag
v0.0.3
v0.0.4
v0.1.0
v0.1.1
v0.1.2

$ git cat-file tag refs/tags/test-tag
fatal: git cat-file refs/tags/test-tag: bad file
---8<---


Alright, I just found out why that is: Lighweight tags are not stored as 
Git objects. As soon as I make it an annoted tag by specifying a 
message, "cat-file tag" do esnot display a fatal error anymore.


However, I still get information about the commit oject iintsead of the 
tag object. Is this expected?


Regards,
Sebastian


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

Trouble with cat-file on tags

[Sebastian Schuberth]

Hi,

I was trying to use cat-file to get the hash of a tag object (not the hash of 
the commit object the tag points to), and I'm running into some issues. At the 
example of a cloned gerry [1] repository:

---8<---
$ git tag test-tag

$ git tag -l
test-tag
v0.0.3
v0.0.4
v0.1.0
v0.1.1
v0.1.2

$ git cat-file tag refs/tags/test-tag
fatal: git cat-file refs/tags/test-tag: bad file
---8<---

So for a newly created local tag, cat-file does not seem to work. However:

---8<---
$ git cat-file tag refs/tags/v0.1.2
object 91b0d21eba039e5ba0a90104c9c485735576dcbf
type commit
tag v0.1.2
tagger Travis Truman  1452693317 -0500

Version 0.1.2
---8<---

For an existing tag, git-file suddenly *does* seem to work, although I'm 
puzzled why I'm getting info on the commit object here. I thought "cat-file 
tag" should explicitly make "cat-file" list information about the tag object 
itself, not about the commit object the tag points to.

Thoughts?

[1] https://github.com/trumant/gerry

Regards,
Sebastian



--
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 V3] MSVC: Use shipped headers instead of fallback definitions

[Sebastian Schuberth]

On 3/30/2016 13:37, Sven Strickroth wrote:


VS2010 comes with stdint.h [1]
VS2013 comes with inttypes.h [2]

[1] https://stackoverflow.com/a/2628014/3906760
[2] 
https://blogs.msdn.microsoft.com/vcblog/2013/07/19/c99-library-support-in-visual-studio-2013/

Signed-off-by: Sven Strickroth 


ACK.

Regards,
Sebastian


--
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 V2] MSVC: VS2013 comes with inttypes.h

[Sebastian Schuberth]

On 3/29/2016 19:23, Sven Strickroth wrote:

> --- a/compat/mingw.h
> +++ b/compat/mingw.h
> @@ -415,7 +415,7 @@ int mingw_offset_1st_component(const char *path);
>   extern void build_libgit_environment(void);
>   extern const char *program_data_config(void);
>   #define git_program_data_config program_data_config
> -#ifndef __MINGW64_VERSION_MAJOR
> +#if !defined(__MINGW64_VERSION_MAJOR) && (!defined(_MSC_VER) || _MSC_VER < 
> 1800)
>   #define PRIuMAX "I64u"
>   #define PRId64 "I64d"
>   #else

ACK for this part. For reference see [1].

> diff --git a/compat/vcbuild/include/unistd.h b/compat/vcbuild/include/unistd.h
> index c65c2cd..b7cc48c 100644
> --- a/compat/vcbuild/include/unistd.h
> +++ b/compat/vcbuild/include/unistd.h
> @@ -45,11 +45,15 @@ typedef unsigned long long uintmax_t;
>   
>   typedef int64_t off64_t;
>   
> +#if !defined(_MSC_VER) || _MSC_VER < 1800
>   #define INTMAX_MIN  _I64_MIN
>   #define INTMAX_MAX  _I64_MAX
>   #define UINTMAX_MAX _UI64_MAX
>   
>   #define UINT32_MAX 0x  /* 4294967295U */
> +#else
> +#include
> +#endif

If we would do "#include " here instead, we could lower the _MSC_VER 
requirement to at least 1700. According to the comment at [2] we could lower it 
even to 1600.

Also the original code is missing a single space after "#include".

[1] 
https://blogs.msdn.microsoft.com/vcblog/2013/07/19/c99-library-support-in-visual-studio-2013/
[2] 
https://stackoverflow.com/questions/126279/c99-stdint-h-header-and-ms-visual-studio#comment4620359_126279

Regards,
Sebastian



--
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] MSVC: vsnprintf in Visual Studio 2015 doesn't need SNPRINTF_SIZE_CORR any more

[Sebastian Schuberth]

On Wed, Mar 30, 2016 at 9:49 AM, Johannes Schindelin
<johannes.schinde...@gmx.de> wrote:

>> >  #ifndef SNPRINTF_SIZE_CORR
>> > -#if defined(WIN32) && (!defined(__GNUC__) || __GNUC__ < 4)
>> > +#if defined(WIN32) && (!defined(__GNUC__) || __GNUC__ < 4) && 
>> > (!defined(_MSC_VER) || _MSC_VER < 1900)
>> >  #define SNPRINTF_SIZE_CORR 1
>> >  #else
>> >  #define SNPRINTF_SIZE_CORR 0
>>
>> I wonder if the logic is (and was) sensible here. We assume that every
>> non-__GNUC__ and non-_MSC_VER compiler on Windows requires the
>> correction. Wouldn't it make sense to not assume requiring the
>> correction unless we know the compiler has this bug? That is,
>> shouldn't this better say
>>
>> #if defined(WIN32) && (defined(__GNUC__) && __GNUC__ < 4) ||
>> (defined(_MSC_VER) && _MSC_VER < 1900))
>> #define SNPRINTF_SIZE_CORR 1
>> #else
>> #define SNPRINTF_SIZE_CORR 0
>
> Since the standard on Windows always was MS Visual C, it should be assumed
> that compilers *other* than GCC followed Visual C's lead.
>
> Of course, evidence speaks louder than assumptions.
>
> Therefore I would prefer to keep the current version, at least until we
> encounter a case where it is incorrect.

Fine with me. It's probably better not to change the logic as we
wouldn't know whether this would break things for some exotic compiler
currently in use to compile Git.

Also ACK from my side on the path then.

-- 
Sebastian Schuberth
--
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] MSVC: vsnprintf in Visual Studio 2015 doesn't need SNPRINTF_SIZE_CORR any more

[Sebastian Schuberth]

On Tue, Mar 29, 2016 at 9:13 PM, Sven Strickroth <s...@cs-ware.de> wrote:

> diff --git a/compat/snprintf.c b/compat/snprintf.c
> index 42ea1ac..0b11688 100644
> --- a/compat/snprintf.c
> +++ b/compat/snprintf.c
> @@ -9,7 +9,7 @@
>   * always have room for a trailing NUL byte.
>   */
>  #ifndef SNPRINTF_SIZE_CORR
> -#if defined(WIN32) && (!defined(__GNUC__) || __GNUC__ < 4)
> +#if defined(WIN32) && (!defined(__GNUC__) || __GNUC__ < 4) && 
> (!defined(_MSC_VER) || _MSC_VER < 1900)
>  #define SNPRINTF_SIZE_CORR 1
>  #else
>  #define SNPRINTF_SIZE_CORR 0

I wonder if the logic is (and was) sensible here. We assume that every
non-__GNUC__ and non-_MSC_VER compiler on Windows requires the
correction. Wouldn't it make sense to not assume requiring the
correction unless we know the compiler has this bug? That is,
shouldn't this better say

#if defined(WIN32) && (defined(__GNUC__) && __GNUC__ < 4) ||
(defined(_MSC_VER) && _MSC_VER < 1900))
#define SNPRINTF_SIZE_CORR 1
#else
#define SNPRINTF_SIZE_CORR 0

-- 
Sebastian Schuberth
--
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] MSVC: vsnprintf in Visual Studio 2015 doesn't need SNPRINTF_SIZE_CORR any more

[Sebastian Schuberth]

On Tue, Mar 29, 2016 at 6:25 PM, Sven Strickroth <s...@cs-ware.de> wrote:

> In MSVC2015 the behavior of vsnprintf was changed.
> W/o this fix there is one character missing at the end.

How about adding a link to [1] in the commit message and quoting the
central "Beginning with the UCRT in Visual Studio 2015 and Windows 10,
vsnprintf is no longer identical to _vsnprintf. The vsnprintf function
complies with the C99 standard; _vnsprintf is retained for backward
compatibility" statement?

[1] https://msdn.microsoft.com/en-us/library/1kt27hek.aspx

-- 
Sebastian Schuberth
--
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] (exit 1) is silly

[Sebastian Schuberth]

On 3/22/2016 17:16, Junio C Hamano wrote:


IMO, this is such a minor thing that once it _is_ in the tree, it's
not really worth the patch noise to go and fix it up.


IMO, instead of writing this you could have just accepted the patch, 
reducing the patch noise ;-)


Regards,
Sebastian


--
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_PATCHv4 4/7] submodule init: redirect stdout to stderr

[Sebastian Schuberth]

On Tue, Mar 22, 2016 at 5:47 PM, Stefan Beller <sbel...@google.com> wrote:

> I think the stance of Git is to write only machine readable stuff to stdout,
> and essentially all _(translated) stuff (i.e. human readable) goes to stderr 
> as
> some sort of help or progress indication.

Thanks for the clarification.

-- 
Sebastian Schuberth
--
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_PATCHv4 4/7] submodule init: redirect stdout to stderr

[Sebastian Schuberth]

On Tue, Mar 22, 2016 at 3:06 AM, Stefan Beller <sbel...@google.com> wrote:

> Reroute the output of stdout to stderr as it is just informative
> messages, not to be consumed by machines.

Just wondering, what's Git's policy on this? This message is neither
an error nor a warning, but just purely informational. As such it
semantically does not belong to stderr, or? On the other hand I see
multiple places in Git's code where printing to stderr is (mis-)used
for informational messages, probably to separate output to be consumed
by humans from output to be consumed by machines, like you do here.

-- 
Sebastian Schuberth
--
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: [ANNOUNCE] Git v2.8.0-rc0

[Sebastian Schuberth]

On 2/27/2016 0:41, Junio C Hamano wrote:


  * Some calls to strcpy(3) triggers a false warning from static
analysers that are less intelligent than humans, and reducing the
number of these false hits helps us notice real issues.  A few
calls to strcpy(3) in test-path-utils that are already safe has
been rewritten to avoid false wanings.

  * Some calls to strcpy(3) triggers a false warning from static
analysers that are less intelligent than humans, and reducing the
number of these false hits helps us notice real issues.  A few
calls to strcpy(3) in "git rerere" that are already safe has been
rewritten to avoid false wanings.


This is a duplicate.

Regards,
Sebastian


--
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] git-completion.bash: always swallow error output of for-each-ref

[Sebastian Schuberth]

On Wed, Feb 24, 2016 at 12:30 AM, Junio C Hamano <gits...@pobox.com> wrote:

> So, have we decided to wait, or we'd rather apply the band-aid in
> the meantime?  I can go either way, just double checking as I
> noticed this thread while updating my leftover bits list.

Thanks for the follow-up, I was about to ask for a status update on
this. As my patch it ready now, and we don't know how long we'd have
to wait for the other solution, I'd vote for applying my patch.

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

Re: [PATCH v3 0/3] config: add '--sources' option to print the source of a config value

[Sebastian Schuberth]

On Sat, Feb 13, 2016 at 3:43 PM, Mike Rappazzo <rappa...@gmail.com> wrote:

>> I renamed the flag from "--source" to "--show-origin" as I got the impression
>> that Sebastian, Peff and Ramsay seem to be all OK with "--show-origin".
>
> I know that I am late to the party here, but why not make the option
> `--verbose` or `-v`?  `git config` doesn't have that now, and this
> seems like a logical thing to include as verbose data.  I would

`--verbose` would be fine with me.

-- 
Sebastian Schuberth
--
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] git-gui--askpass: generalize the window title

[Sebastian Schuberth]

On 01.02.2016 13:11, Sebastian Schuberth wrote:


git-gui--askpass is not only used for SSH authentication, but also for
HTTPS. In that context it is confusing to have a window title of
"OpenSSH". So generalize the title so that it also says which parent
process, i.e. Git, requires authentication.

Signed-off-by: Sebastian Schuberth <sschube...@gmail.com>


I haven't seen this being picked up so far. Any comments?

--
Sebastian Schuberth
--
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] git-completion.bash: always swallow error output of for-each-ref

[Sebastian Schuberth]

On 04.02.2016 11:34, Sebastian Schuberth wrote:


This avoids output like

 warning: ignoring broken ref refs/remotes/origin/HEAD

while completing branch names.

Signed-off-by: Sebastian Schuberth <sschube...@gmail.com>


The discussion got a bit off the point with the "short" vs. "strip=2" 
stuff, but I guess the patch itself if good to apply?


--
Sebastian Schuberth

--
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] config: add '--sources' option to print the source of a config value

[Sebastian Schuberth]

On Wed, Feb 10, 2016 at 1:47 PM, Ramsay Jones
<ram...@ramsayjones.plus.com> wrote:

>> Sebastian suggested "--show-origin" as a better option name over "--sources".
>> I still believe "--sources" might be slightly better as I fear that users 
>> could
>> somehow related "origin" to "remote" kind of configs. However, I am happy to
>> change that if a majority prefers "--show-origin".
>
> 
> As I have said before, I'm not very good at naming things, but ...
>
> Of the two, I *slightly* prefer --show-origin, since I don't think
> there will be any confusion. However, I think --source may be OK too
> (for some reason it sounds better than the plural). Another idea
> may be --show-source. ;-)
>
> 

I agree that using --source sounds better than --sources, as the
latter sounds even more like "source code".

Here's another idea: How about --declaration or --show-declaration?

-- 
Sebastian Schuberth
--
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] config: add '--sources' option to print the source of a config value

[Sebastian Schuberth]

On Wed, Feb 10, 2016 at 4:40 PM, Jeff King <p...@peff.net> wrote:

>> >   file:\t
>> >   blob:\t
>> >   stdin\t
>> >   cmd\t
>> >
>> > with a single delimited slot for the source, which can then be broken
>> > down further if desired.  I can't think of any reason to prefer one over
>> > the other rather than personal preference, though. They can both be
>> > parsed unambiguously.
>>
>> I also would have expected sopme like the latter, except that I'd also
>> expect a colon after "stdin" and "cmd" (or "cmdline", as said above).
>> I.e. the colon should be part of the prefix to mark it as such.
>
> Yeah, I waffled on that. Having a colon means you can definitely parse
> to the first ":" without looking at what the prefix is. But if you don't
> know what the prefix is, I don't know what good that does you. IOW, I'd

IMO that's asking the wrong question. The question should not be "what
good does it do if we add the colons also there", but "what
justification do we have to introduce an inconsistency by not adding
them".

> That's perl, but I think most languages make prefix-parsing like that
> easy. I dunno. I doubt it matters all that much, and we are deep into
> personal preference. There's already plenty to bikeshed on the option
> name :)

I agree the option wording mostly is personal preference. On the other
hand, I find discussions like these often prematurely waved aside as
bikeshedding, just because e.g. Perl can parse the one as good as the
other. But it's not about Perl, it's about humans. IMO Git has
historically made many mistakes by not caring enough about consistency
in docs, command and command line option naming, so I don't see it as
wasted time to discuss about things like this.

-- 
Sebastian Schuberth
--
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 v1] config: add '--sources' option to print the source of a config value

[Sebastian Schuberth]

On Sun, Feb 7, 2016 at 8:28 PM, Lars Schneider <larsxschnei...@gmail.com> wrote:

>>> However, the naming of the '--sources' option sounds a bit misleading to me.
>>> It has nothing to do with source code. So maybe better name it '--origin',
>>> or even more verbose '--show-origin' or '--show-filename'?
>>
>> I think he inherited the "--sources" name from me. :) I agree it could
>> be better. I think "--show-filename" is not right as there are
>> non-filename cases.  Just "--origin" sounds funny to me, perhaps because
>> of git's normal use of the word "origin".
>>
>> I like "--show-origin" the best of the ones suggested.
>
> I understand your reasoning and I agree that "--show-origin" is better than
> "--sources". However, I think just the word "origin" could be misleading in
> this context because people associate it with Git remotes. How about
> "--show-config-origin" then? Or would that be too verbose?

Well, "origin" just happens to be the name of the default remote.
AFAIK all options that deal with remotes have "remote" and not
"origin" in their name, so I think the risk of confusion is rather
low. But I'd be fine with "--show-config-origin", too. Although it's
verbose, it's probably not used very often, so personally I could live
with typing the extra character. Esp. if you add Bash completion
support for it :-)

-- 
Sebastian Schuberth
--
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 v1] config: add '--sources' option to print the source of a config value

[Sebastian Schuberth]

On 2/5/2016 14:58, Jeff King wrote:


Yeah, I agree it's unlikely. And the output is already ambiguous, as the
first field could be a blob (though I guess the caller knows if they
passed "--blob" or not). If we really wanted an unambiguous output, we
could have something like "file:...", "blob:...", etc. But that's a bit
less readable for humans, and I don't think solves any real-world
problems.

So I think it would be OK to use "" here, as long as the
token is documented.


Thinking about it again, I actually do like Peff's prefix solution 
better. It would solve the real-world problem that my proposed "line>" marker could in fact be a file name.


Regards,
Sebastian

--
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 v1] config: add '--sources' option to print the source of a config value

[Sebastian Schuberth]

On 2/5/2016 9:42, larsxschnei...@gmail.com wrote:


Teach 'git config' the '--sources' option to print the source
configuration file for every printed value.


Yay, not being able to see where a config setting originates from has 
bothered me in the past, too. So thanks for working on this.


However, the naming of the '--sources' option sounds a bit misleading to 
me. It has nothing to do with source code. So maybe better name it 
'--origin', or even more verbose '--show-origin' or '--show-filename'?


Regards,
Sebastian

--
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 v1] config: add '--sources' option to print the source of a config value

[Sebastian Schuberth]

On 2/5/2016 12:20, Jeff King wrote:


Hmm. I had originally envisioned this only being used with "--list", but
I guess it makes sense to say "--sources --get" to show where the value
for a particular option is coming from.


Being able to use "--sources --get" is a feature that I'd definitely 
like to see, too.



I'm not sure returning here is the best idea. We won't have a config
filename if we are reading from "-c", but if we return early from this
function, it parses differently than every other line. E.g., with your
patch, if I do:

   git config -c foo.bar=true config --sources --list

I'll get:

   /home/peff/.gitconfig  user.name=Jeff King
   /home/peff/.gitconfig  user.email=p...@peff.net
   ...etc...
   foo.bar=true

If somebody is parsing this as a tab-delimited list, then instead of the
source field for that line being empty, it is missing (and it looks like
"foo.bar=true" is the source file). I think it would be more friendly to
consumers of the output to have a blank (i.e., set "fn" to the empty
string and continue in the function).


Or to come up with a special string to denote config values specified on 
the command line. Maybe somehting like


  foo.bar=true

I acknowledge that "" would be a valid filename on some 
filesystems, but I think the risk is rather low that someone would 
actually be using that name for a Git config file.


Regards,
Sebastian

--
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] git-completion.bash: always swallow error output of for-each-ref

[Sebastian Schuberth]

This avoids output like

warning: ignoring broken ref refs/remotes/origin/HEAD

while completing branch names.

Signed-off-by: Sebastian Schuberth <sschube...@gmail.com>
---
 contrib/completion/git-completion.bash | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/contrib/completion/git-completion.bash 
b/contrib/completion/git-completion.bash
index 15ebba5..7c0549d 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -317,7 +317,7 @@ __git_heads ()
local dir="$(__gitdir)"
if [ -d "$dir" ]; then
git --git-dir="$dir" for-each-ref --format='%(refname:short)' \
-   refs/heads
+   refs/heads 2>/dev/null
return
fi
 }
@@ -327,7 +327,7 @@ __git_tags ()
local dir="$(__gitdir)"
if [ -d "$dir" ]; then
git --git-dir="$dir" for-each-ref --format='%(refname:short)' \
-   refs/tags
+   refs/tags 2>/dev/null
return
fi
 }
@@ -355,14 +355,14 @@ __git_refs ()
;;
esac
git --git-dir="$dir" for-each-ref --format="%($format)" \
-   $refs
+   $refs 2>/dev/null
if [ -n "$track" ]; then
# employ the heuristic used by git checkout
# Try to find a remote branch that matches the 
completion word
# but only output if the branch name is unique
local ref entry
git --git-dir="$dir" for-each-ref --shell 
--format="ref=%(refname:short)" \
-   "refs/remotes/" | \
+   "refs/remotes/" 2>/dev/null | \
while read -r entry; do
eval "$entry"
ref="${ref#*/}"
@@ -1835,7 +1835,7 @@ _git_config ()
remote="${remote%.push}"
__gitcomp_nl "$(git --git-dir="$(__gitdir)" \
for-each-ref --format='%(refname):%(refname)' \
-   refs/heads)"
+   refs/heads 2>/dev/null)"
return
;;
pull.twohead|pull.octopus)
-- 
2.7.0.windows.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] git-gui--askpass: generalize the window title

[Sebastian Schuberth]

From: Sebastian Schuberth <sschube...@gmail.com>

git-gui--askpass is not only used for SSH authentication, but also for
HTTPS. In that context it is confusing to have a window title of
"OpenSSH". So generalize the title so that it also says which parent
process, i.e. Git, requires authentication.

Signed-off-by: Sebastian Schuberth <sschube...@gmail.com>
---
 git-gui/git-gui--askpass | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/git-gui/git-gui--askpass b/git-gui/git-gui--askpass
index 4277f30..1e5c325 100755
--- a/git-gui/git-gui--askpass
+++ b/git-gui/git-gui--askpass
@@ -60,7 +60,7 @@ proc finish {} {
set ::rc 0
 }
 
-wm title . "OpenSSH"
+wm title . "Git Authentication"
 tk::PlaceWindow .
 vwait rc
 exit $rc

--
https://github.com/git/git/pull/195
--
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/5] submodule-config: keep labels around

[Sebastian Schuberth]

On Sat, Jan 23, 2016 at 1:31 AM, Stefan Beller <sbel...@google.com> wrote:

> We need the submodule groups in a later patch.

The commit message should now say "labels", too, I guess.

-- 
Sebastian Schuberth
--
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/4] Submodule Groups

[Sebastian Schuberth]

On Thu, Jan 21, 2016 at 10:56 PM, Stefan Beller <sbel...@google.com> wrote:

>>> [submodule "gcc"]
>>>  path = gcc
>>>  url = git://...
>>>  groups = default
>>>  groups = devel
>>
>> On the quick I was unable to find the rationale why entries are now stored 
>> as separated lines compared to v1. I liked the comma-separated approach 
>> better as it's more compact.
>
> IIUC the line oriented way is preferred as it is our standard. Do we
> have any other options stored as a comma separated list?

Out of my head I cannot think of any. But that shouldn't mean we
cannot introduce such comma separated list if it makes sense.

> Makes sense to use singular then. However per discussion with Junio in
> [PATCH 3/4] submodule update: Initialize all group-selected submodules
> by default, we want to not name it "group", as it's unclear what a group is
> supposed to mean. What does a group do? which operations are supported?

How about calling it "label" instead of "group"? IMO with the word
"label" it's more clean that a single submodule can have multiple
labels, as the concept of labels is familiar to the user already from
applications like Firefox (bookmarks), Google Mail, Mac OS X Finder
(files) etc.

> Instead of having a submodule -> set assignment, we could do it the
> other way round:
>
>  [submodule "gcc"]
>  ...
>
>  [submodule-set "default"]
> submodule = gcc
> submodule = foo
> submodule = by/path/*

In your example you're now introducing "set" as a new term. Shouldn't
this better be "submodule-group" then? I actually like this idea quite
a bit as it completely solves the problem about being clear that a
submodule can belong to mutiple groups.

> but I'd assume this is less useful for the user. How often does a user ask:
> "How many/Which submodules are in $GROUP" as opposed to "What about
> submodule foo,
> is that part of group $GROUP?"

True, but for answering that question a user would not look at
.gitmodules, but run a command, and the implementation of that command
would completely hide that complexity from the user.

> As asked above, how many comma separated things do we have in git configs?
> I'd really not want to add more mental complexity to Git. As far as I

I don't think it can get much worse anyway ;-)

> remember we have
> rather double configs than one long line separated somehow.
> (The only thing that comes to mind is multiple remote urls for pushing)

I believe so, too. But I'd see the introduction of comma-separated
values as an exit-strategy to that. More settings could make use of
that in the future, then.

-- 
Sebastian Schuberth
--
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/4] Submodule Groups

[Sebastian Schuberth]

On 20.01.2016 04:34, Stefan Beller wrote:

> So you could have a .gitmodules file such as:
> 
> [submodule "gcc"]
>  path = gcc
>  url = git://...
>  groups = default
>  groups = devel

On the quick I was unable to find the rationale why entries are now stored as 
separated lines compared to v1. I liked the comma-separated approach better as 
it's more compact.

Anyway, if it's only one group per line, I'd find it more fitting to call the 
entry "group" instead of "groups" as it will always refer to a single group 
only. Also that would better match the "--group" command line option naming for 
"submodule add".

However, if I'd read the single line "group = default" in a .gitmodules file, 
it wouldn't be immediately clear to me that "group" can appear multiple times 
per submodule. "groups = default" would me more hinting is this regard because 
the plural is used, but without reading the docs I'd assume multiple groups 
would be specified like "groups = default,devel".

Long story short, my personal favorite still would be 

[submodule "gcc"]
     groups = default,devel

followed by

[submodule "gcc"]
 group = default
 group = devel

-- 
Sebastian Schuberth
--
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: Questions on passing --depth to git-clone vs. git-fetch

[Sebastian Schuberth]

On 1/6/2016 13:41, Duy Nguyen wrote:


I think the culprit is the "git remote add" line. "git clone --depth"
by default will fetch only one branch (aka --single-branch option in
git-clone). But I suspect when you add a new remote, the default


Now that you mention it I see this being documented as part of 
--single-branch instead of --depth, which I think is confusing. I'll 
send a patch.


--
Sebastian Schuberth


--
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] docs: clarify that passing --depth to git-clone implies --single-branch

[Sebastian Schuberth]

It is confusing to document how --depth behaves as part of the
--single-branch docs. Better move that part to the --depth docs, saying
that it implies --single-branch by default.

Signed-off-by: Sebastian Schuberth <sschube...@gmail.com>
---
 Documentation/git-clone.txt | 9 -
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/Documentation/git-clone.txt b/Documentation/git-clone.txt
index 6bf000d..943de8b 100644
--- a/Documentation/git-clone.txt
+++ b/Documentation/git-clone.txt
@@ -190,15 +190,14 @@ objects from the source repository into a pack in the 
cloned repository.
 
 --depth ::
Create a 'shallow' clone with a history truncated to the
-   specified number of revisions.
+   specified number of revisions. Implies `--single-branch` unless
+   `--no-single-branch` is given to fetch the histories near the
+   tips of all branches.
 
 --[no-]single-branch::
Clone only the history leading to the tip of a single branch,
either specified by the `--branch` option or the primary
-   branch remote's `HEAD` points at. When creating a shallow
-   clone with the `--depth` option, this is the default, unless
-   `--no-single-branch` is given to fetch the histories near the
-   tips of all branches.
+   branch remote's `HEAD` points at.
Further fetches into the resulting repository will only update the
remote-tracking branch for the branch this option was used for the
initial cloning.  If the HEAD at the remote did not point at any
-- 
2.7.0.windows.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

Questions on passing --depth to git-clone vs. git-fetch

[Sebastian Schuberth]

Hi,

I recently compared the results of doing

$ git clone --depth=1 https://github.com/git/git.git git-clone-depth-1

versus

$ mkdir git-fetch-depth-1
$ cd git-fetch-depth-1
$ git init
$ git remote add origin https://github.com/git/git.git
$ git fetch --depth=1

and noticed a few things:

1. The docs of clone [1] say about --depth "Create a shallow clone with a 
history truncated to the specified number of revisions" while for fetch the 
docs [2] say "[...] to the specified number of commits [...]". As in this 
particular case revision are always commits, I think the clone docs should also 
say "commits".

2. In the fetch docs --depth is described to "Deepen or shorten the history of 
a shallow repository created by git clone". That sounds as if my example from 
above where I initialze a repo manually would not allow fetch to be called with 
--depth as I did not clone before. But in fact my example works fine. I guess 
we need some clarfication in the wording here.

3. When running "git log --all -oneline" in the two working trees I get 
different results, which is not what I'd expect:

$ cd git-clone-depth-1
$ git log --all --oneline
  7548842 Git 2.7

versus

$ cd git-fetch-depth-1
$ git log --all --oneline
  b819526 Merge branch 'jk/notes-merge-from-anywhere' into pu
  e2281f4 What's cooking (2016/01 #01)
  ef7b32d Sync with 2.7
  7548842 Git 2.7
  833e482 Git 2.6.5

So in the clone case only the specified number of commits from the tip of the 
default branch (master in this case) is fetched, not of each remote branch 
history. fetch in the other hand really gets the specified number of commits 
from the tip of each remote branch history. I don't know whether this behavior 
is inded or not as I cannot find any docs on it either way. But it seems 
inconsistent to me that clone with --depth only gets the history for the 
default branch, as clone without --depth would give me the history of all 
branches.

For completeness, I'm using Git for Windows 2.7.

Any comments?

[1] https://git-scm.com/docs/git-clone
[1] https://git-scm.com/docs/git-fetch

-- 
Sebastian Schuberth

--
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] contrib/git-candidate: Add README

[Sebastian Schuberth]

On 10.11.2015 13:56, Richard Ipsum wrote:


+Existing tools such as Github's pull-requests and Gerrit are already
+in wide use, why bother with something new?
+
+We are concerned that whilst git is a distributed version control
+system the systems used to store comments and reviews for content
+under version control are usually centralised,


I think it's a bit unjust to unconditionally mention Gerrit in this 
context as you seem to imply that Gerrit does not store *any* review 
data in Git.


Even without Dave's upcoming notedb, Gerrit already stores refs/changes 
in Git, and with the reviewnotes plugin [1] also the outcome of a review 
in refs/notes/review.


[1] 
https://gerrit.googlesource.com/plugins/reviewnotes/+/refs/heads/master/src/main/resources/Documentation/refs-notes-review.md


--
Sebastian Schuberth

--
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] contrib/subtree: Remove --annotate

[Sebastian Schuberth]

On Sat, Jan 2, 2016 at 9:36 PM, David Greene <gree...@obbligato.org> wrote:

> commit messages.  git has other tools more suited to rewriting
> commit messages and it's easy enough to use them after a subtree
> split.

For completeness, it probably would be a good idea to name examples
for such more suitable tools as part of the commit message.

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

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

[Sebastian Schuberth]

On Mon, Nov 23, 2015 at 6:05 PM, Torsten Bögershausen <tbo...@web.de> wrote:

> or, as another example:
> git ls-files -o --eol
> i/  w/binaryattr/  zlib.o

I see, somewhat convincing I have to agree.

On another note, how about making the prefix either all just one
letter (i.e. "attr/" becomes "a/"), or all multi-letter abbreviations
(i.e. "i/" becomes "idx/" and "w/" becomes "wt/" or "wtree/" or
"tree/")?

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

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

[Sebastian Schuberth]

On 21.11.2015 08:36, Torsten Bögershausen wrote:

> git ls-files --eol gives an output like this:
> 
> i/text-no-eol   w/text-no-eol   attr/text=auto t/t5100/empty

I'm sorry if this has been discussed before, but hav you considered to use a 
header line and omit the prefixed from the columns instead? Like

index working tree attributesfile

binarybinary   -text t/test-binary-2.png
text-lf   text-lf  eol=lft/t5100/rfc2047-info-0007
text-lf   text-crlfeol=crlf  doit.bat
text-crlf-lf  text-crlf-lf   locale/XX.po

I believe this would be both easier to read for humans, and easier to parse for 
scripts that e.g. want to compare line endings in the index and working tree.

> +stats_ascii () {
> + case "$1" in

[...]

> + *)
> + echo huh $1
> + ;;

Personally, I'm not a big fan of supposedly funny output like this. How about 
printing a proper message rather than "huh", even for cases that should not 
happen?

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

Re: [PATCH v4 4/4] Add Travis CI support

[Sebastian Schuberth]

On Fri, Nov 6, 2015 at 2:11 PM, Lars Schneider <larsxschnei...@gmail.com> wrote:

> Per platform/compiler (Linux/clang) we run two configurations. One
> normal configuration (see the lonely "-" right under "matrix:") and one
> configuration with all sorts of things are disabled ("NO...").
>
> You can see all 8 configurations ([linux, mac] * [clang, gcc] * [normal,
> NO_...]) here:
> https://travis-ci.org/larsxschneider/git/builds/89598194

Aren't these 8 configurations a bit too much? I see the total running
time is about 2 hours. For my taste, this is way to much to give
developers feedback about the status of their PR. It should be
something < 30 minutes.

IMO, the purpose of the Travis CI configuration mainly is to 1) save
developers work by not requiring to build manually, 2) build on other
platforms than the developer has access to. I doubt that the average
developer manually builds anything close to these 8 configurations
before we had this job, so I wouldn't make it a requirement for Travis
to do much more than a developer could / would to manually.

On the other hand, I see the point in letting a CI system test more
configurations than manually feasible. But that should be done as part
of a different job then. E.g. we could have a "fast" PR validation
job, and a "slow" nightly build job.

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

Re: [PATCH v4 4/4] Add Travis CI support

[Sebastian Schuberth]

On Fri, Nov 6, 2015 at 2:28 PM, Lars Schneider <larsxschnei...@gmail.com> wrote:

> Well, I partly agree. Right now the running time is ~20 min (that means less 
> than your 30min target!). After ~10min you even have all Linux results, Mac 
> takes a bit longer. Travis shows you 2h because that is the time that would 
> be required if all builds where run one after another (we run builds in 
> parallel).

Are you sure about than? I mean, what sense does it make to show how
long it *would* have taken *if* the builds were running serially? I
can see that the longest of the jobs took 21 minutes, which is ok. But
that does not mean that all jobs completed in within 21 minutes. It
could be that not all jobs started at (about) the same time due to a
lack of resources, and that the last job did not compete before the 2
hours were over because it only started to run 1 hours and 40 minutes
befor ethe first job was started.

> That being said, I see your point of to avoiding to burn Travis CI resources 
> meaningless. If I am not mistaken then you can configure Travis in a way that 
> it runs different configurations for different branches. E.g. I would like to 
> run all 8 configurations on maint, master, next and maybe pu. All other 
> branches on peoples own forks should be fine with the default Linux build 
> (~10min).
>
> What do you think?

I think running different configuration per branch makes sense, yes.

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

Re: [PATCH v4 4/4] Add Travis CI support

[Sebastian Schuberth]

On Fri, Nov 6, 2015 at 2:55 PM, Lars Schneider <larsxschnei...@gmail.com> wrote:

>> I think running different configuration per branch makes sense, yes.
>
> If the list decides to accept this patch. Do you think that would be a 
> necessary requirement for the first iteration?

No. I think this could be addressed later as an improvements. To me
it's more important to finally get *some* sane Travis CI configuration
in.

-- 
Sebastian Schuberth
--
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] ls-files: Add eol diagnostics

[Sebastian Schuberth]

On 31.10.2015 11:25, Matthieu Moy wrote:

>> ca:text-no-eol   wt:text-no-eol   t/t5100/empty
>> ca:binarywt:binaryt/test-binary-2.png
>> ca:text-lf   wt:text-lf   t/t5100/rfc2047-info-0007
>> ca:text-lf   wt:text-crlf doit.bat
>> ca:text-crlf-lf  wt:text-crlf-lf  locale/XX.po
> 
> I would spell the first "in" or "idx" (for "index"), not "ca" (for
> "cache"). I think we avoid talking about "the cache" these days even
> though the doc sometimes says "cached in the index" (i.e. use "cache" as
> a verb, not a noun).

Good point, I'd prefer "idx" over ca", too.

However, the commit message says "to check if text files are stored normalized 
in the *repository*", yet the output refers to the index / cache. Is there a 
(potential) difference between line endings in the index and repo? AFAIK there 
is not. Any I find it a bit confusing to refer to the index where, as e.g. for 
a freshly cloned repo the index should be empty, yet you do have specific line 
endings in the repo.

Long story short, how about consistently talking about line endings in the 
repo, and also using "repo" instead of "ca" here?

-- 
Sebastian Schuberth

--
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] ls-files: Add eol diagnostics

[Sebastian Schuberth]

On Sun, Nov 1, 2015 at 7:40 PM, Matthieu Moy
<matthieu@grenoble-inp.fr> wrote:

>> Any I find it a bit confusing to refer to the index where, as e.g. for
>> a freshly cloned repo the index should be empty,
>
> No it is not. The index is a complete snapshot of your working tree.
> When you have no uncommited staged changes, the index contains all files
> that are in HEAD. Most commands show you _changes_ in the index (wrt
> HEAD or wrt the working tree), but the index itself contain all files.

Thanks for the info.

> At stage 4), you really want to see the content of the index, because
> your HEAD is still broken.

Ok, I'm convinced. Thanks again!

-- 
Sebastian Schuberth
--
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/3] clone: Allow an explicit argument for parallel submodule clones

[Sebastian Schuberth]

On 23.10.2015 20:44, Stefan Beller wrote:


[...] which may pick reasonable
defaults if you don't specify an explicit number.


IMO the above should also be mentioned ini the docs:


+-j::
+--jobs::
+   The number of submodules fetched at the same time.


Otherwise, from reading the docs, my immediate question would be "What's 
the default for n if not specified?"


--
Sebastian Schuberth

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

Re: [PATCH v3 1/3] Add Travis CI support

[Sebastian Schuberth]

On 10/11/2015 19:55, larsxschnei...@gmail.com wrote:


+  sudo apt-get update -qq
+  sudo apt-get install -y apt-transport-https
+  sudo apt-get install perforce-server git-lfs


Why no "-y" also in this line, or append these to the previous line?

Or maybe even better, like [1] does, also use "--qq" (which implies 
"-y") for "apt-get install"?



+install: make configure && ./configure
+
+before_script: make
+
+script: make --quiet test


Semantically, it does not seem correct to me that configuarion goes to 
the install step. As "make test" will build git anyway, I'd instead 
propose to get rid of "install" and just say:


before_script: make configure && ./configure

script: make --quiet test

[1] https://github.com/git/git/pull/154/files

Regards,
Sebastian


--
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 v1] Add Travis CI support

[Sebastian Schuberth]

On 10/4/2015 19:46, Junio C Hamano wrote:


The very nice thing with Travis-CI is that it does not only test the
repository's branches, but also all pull-requests.


OK, that is the first real argument I heard for enabling it on
git/git that is worth listening to.


I was mentioning that very argument in the context of PRs filed for use 
with submitgit already back in July in the conversation at [1] in which 
you took part.


[1] https://github.com/rtyley/submitgit/issues/16

Regards,
Sebastian


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

Re: [PATCH v3 1/3] Add Travis CI support

[Sebastian Schuberth]

On Mon, Oct 12, 2015 at 6:02 PM, Junio C Hamano <gits...@pobox.com> wrote:

>> Semantically, it does not seem correct to me that configuarion goes to
>> the install step. As "make test" will build git anyway, I'd instead
>> propose to get rid of "install" and just say:
>>
>> before_script: make configure && ./configure
>>
>> script: make --quiet test
>
> Very good point.  Do we even need to do anything in the "install"
> target?  We aim to be able to testable without any installed Git,
> and not running "make install" at all, ever, would be one way to
> make sure that works.

Note that Travis' "install" step is about installing dependencies for
the application to build [1], not for installing the built application
(i.e. what "make install" does). In any case, I still think
configuring the application to built in this step is wrong.

> we are to start using automated tests, I wonder if we want to build
> (and optionally test) with various combinations of the customization
> options (e.g. NO_CURL, NO_OPENSSL, NO_MMAP, NO_IPV6, NO_PERL etc.)

I like that idea, but I think we should save that for a future
improvement to .travis.yml.

[1] http://docs.travis-ci.com/user/installing-dependencies/

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