Re: [PATCH] doc: Modify git-add doc to say "staging area"

[David A. Wheeler]

On December 13, 2017 12:40:12 AM EST, Jacob Keller  
wrote:
>I know we've used various terms for this concept across a lot of the
>documentation. However, I was under the impression that we most
>explicitly used "index" rather than "staging area".

I think "staging area" is the better term. It focuses on its purpose, and it is 
also less confusing ("index" and "cache" have other meanings in many of the 
repos managed by git).


--- David A.Wheeler

Re: [PATCH] doc: Modify git-add doc to say "staging area"

[Jacob Keller]

On Tue, Dec 12, 2017 at 6:32 PM, David A. Wheeler  wrote:
> Change the documentation of git-add so that it consistently uses
> the phrase "staging area".  The current git documentation uses
> inconsistent terminology ("index", "cache", and "staging area").
> This commit switches git-add's documentation to consistently use
> the phrase "staging area", which is higher-level and should be less
> confusing for new users.
>

I know we've used various terms for this concept across a lot of the
documentation. However, I was under the impression that we most
explicitly used "index" rather than "staging area".

Additionally, I think there are many other locations which
consistently use "index" as the term already.

Thanks,
Jake

Re: [PATCH] doc: clarify usage of XDG_CONFIG_HOME config file

[Jacob Keller]

On Tue, Dec 12, 2017 at 11:47 AM, Junio C Hamano  wrote:
> Jacob Keller  writes:
>
>>  --global::
>> + For writing options: write to global user configuration file
>> + rather than the repository `.git/config`.
>>  +
>> +For reading options: read only from global user configuration file
>> +rather than from all available files.
>>  +
>>  See also <>.
>
> OK.
>
>> @@ -237,26 +235,30 @@ See also <>.
>>  FILES
>>  -
>>
>> +If not set explicitly with `--file`, there are three locations where
>>  'git config' will search for configuration options:
>>
>> +System-wide configuration::
>> + Located at `$(prefix)/etc/gitconfig`.
>>
>> +User-specific configuration::
>> + One and only one of the following files will be read
>
> We said "will search for" upfront, but this talks about "will be
> read", leaving the reader puzzled as to what should happen when
> writing.  Perhaps "s/read/used/"?
>

Ok, that makes sense. I'm definitely iffy on all this wording, as I
didn't really like the previous approach, but couldn't find anything
better than the approach shown here.

I'd be welcome to suggestions for another way to format this information.

>> ++
>> +- `~/.gitconfig`
>> +- `$XDG_CONFIG_HOME/git/config`
>> +- `$HOME/.config/git/config`
>> ++
>> +If `~/.gitconfig` exists, it will be used, and the other files will not be
>> +read. Otherwise, if `$XDG_CONFIG_HOME` is set, then 
>> `$XDG_CONFIG_HOME/git/config`
>> +will be used, otherwise `$HOME/.config/git/config` will be used.
>
> And then "and the other files will not be read" can be dropped from
> the first sentence of this paragraph?
>
> Yaroslav on the original thread mentioned that reading codepath
> without --file or --global does not limit to one of the three, and
> this section is about "If not set explicitly with `--file`", so we'd
> need to make sure if the above is what happens in reality (or update
> the proposed clarification to match the reality).

I'm pretty sure it does not read XDG_CONFIG_HOME unless ~/.gitconfig
is missing. I tried a few things, but it was 2am for me, so I may be
mis-remembering.

Either way, I'd prefer if we had explicit tests in the suite which
verified our assumptions.

Thanks,
Jake

>
> Thanks.

Re: [PATCH] doc: clarify usage of XDG_CONFIG_HOME config file

[Jacob Keller]

On Tue, Dec 12, 2017 at 7:20 AM, Todd Zullinger  wrote:
> Hi Jacob,
>
> Jacob Keller wrote:
>> The documentation for git config and how it reads the user specific
>> configuration file is misleading. In some places it implies that
>> $XDG_CONFIG_HOME/git/config will always be read. In others, it implies
>> that only one of ~/.gitconfig and $XDG_CONFIG_HOME/git/config will be
>> read.
>>
>> Improve the documentation explaining how the various configuration files
>> are read, and combined.
>>
>> Instead of referencing each file individually, reference each type of
>> location git will check. When discussing the user configuration, explain
>> how we switch between one of three choices. Ensure to note that only one
>> of the three choices is used.
>
> Perhaps it would read a little easier as "Make it clear ..."
> rather than "Ensure to note that ..." ?
>
>> +Note that git will only ever use one of these files as the global user
>> +configuration file at once. Additionally if you sometimes use an older 
>> version
>> +of git, it is best to only rely on `~/.gitconfig` as support for the others 
>> was
>> +added fairly recently.
>
> Is it really accurate to say these were added fairly
> recently?  It looks like XDG_CONFIG_HOME was added in
> 21cf322791 ("config: read (but not write) from
> $XDG_CONFIG_HOME/git/config file", 2012-06-22) and
> 0e8593dc5b ("config: write to $XDG_CONFIG_HOME/git/config
> file when appropriate", 2012-06-22) which are in 1.7.12.
>
> Would it be better to say something like "if you sometimes
> use a version of git prior to 1.7.12" here?
>

I copied this from the original and didn't look to see how accurate it
was. I'd be ok with dropping it now that we've had support for such a
long time.

Thanks,
Jake

> Or maybe we can drop "Additionally ..." altogether now?
> Someone using a 5 year old git version sometimes will
> hopefully know to check the documentation for that older
> version.
>
> --
> Todd
> ~~
> Now don't say you can't swear off drinking; it's easy. I've done it a
> thousand times.
> -- W.C. Fields
>

Re: Q: rational for $XDG_CONFIG_HOME/git/config to be "non global" or just a bug?

[Jacob Keller]

On Tue, Dec 12, 2017 at 6:13 AM, Yaroslav Halchenko  wrote:
>
> On Mon, 11 Dec 2017, Junio C Hamano wrote:
>
>> Jonathan Nieder  writes:
>
>> > I think the documentation
>
>> > ~/.gitconfig
>> > User-specific configuration file. Also called "global"
>> > configuration file.
>
>> > should be clarified --- e.g. it could say
>
>> > $XDG_CONFIG_HOME/git/config
>> > ~/.gitconfig
>> > User-specific configuration files. Because options in
>> > these files are not specific to any repository, thes
>> > are sometimes called global configuration files.
>
>> Yeah, I think that makes sense.
>
>> > As for "git config --global", I think the best thing would be to split
>> > it into two options: something like "git config --user" and "git
>> > config --xdg-user".  That way, it is unambiguous which configuration
>> > file the user intends to inspect or modify.  When a user calls "git
>> > config --global" and both files exist, it could warn that the command
>> > is ambiguous.
>
>> > Thoughts?
>
>> I actually thought that the plan was "you either have this, or the
>> other one, never both at the same time" (and I think those who
>> pushed the XDG thing in to the system made us favor it over the
>> traditional one).  So as long as --global updates the one that
>> exists, and updates XDG one when both or neither do, I think we
>> should be OK.  And from that viewpoint, we definitely do not want
>> two kinds of --global to pretend as if we support use of both at the
>> same time.
>
> note that atm $XDG_CONFIG_HOME/git/config is read as --global iff
> ~/.gitconfig is absent and read always without --global.  So it is
> flipping between "global" and "some kind of non-global but user-specific
> configuration file" (so sounds like  a global to me ;) )
>
> --
> Yaroslav O. Halchenko
> Center for Open Neuroscience http://centerforopenneuroscience.org
> Dartmouth College, 419 Moore Hall, Hinman Box 6207, Hanover, NH 03755
> Phone: +1 (603) 646-9834   Fax: +1 (603) 646-1419
> WWW:   http://www.linkedin.com/in/yarik

I didn't see it read, if ~/.gitconfig exists, it appears to never be
read on my system..

Thanks,
Jake

Re: Q: rational for $XDG_CONFIG_HOME/git/config to be "non global" or just a bug?

[Jacob Keller]

On Tue, Dec 12, 2017 at 11:36 AM, Junio C Hamano  wrote:
> Jacob Keller  writes:
>
>>> I actually thought that the plan was "you either have this, or the
>>> other one, never both at the same time" (and I think those who
>>> pushed the XDG thing in to the system made us favor it over the
>>> traditional one).  So as long as --global updates the one that
>>> exists, and updates XDG one when both or neither do, I think we
>>> should be OK.  And from that viewpoint, we definitely do not want
>>> two kinds of --global to pretend as if we support use of both at the
>>> same time.
>>
>> It appears that we actually prefer ~/.gitconfig rather than XDG_CONFIG_HOME..
>>
>> And at least based on current cursory testing on the command line, we
>> do both read and write to the proper location, assuming that
>> ~/.gitconfig is preferred over $XDG_CONFIG_HOME.
>
> OK, so I misremembered the details but it seems that the behaviour
> is consistent and there is no ambiguity?
>
> Am I reading you correctly?

As far as I could tell based on local testing. I could be wrong, and
haven't yet cooked up a test case for it yet.

Thanks,
Jake

[PATCH] doc: Modify git-add doc to say "staging area"

[David A. Wheeler]

Change the documentation of git-add so that it consistently uses
the phrase "staging area".  The current git documentation uses
inconsistent terminology ("index", "cache", and "staging area").
This commit switches git-add's documentation to consistently use
the phrase "staging area", which is higher-level and should be less
confusing for new users.

Signed-off-by: David A. Wheeler 
---
 Documentation/git-add.txt | 104 --
 1 file changed, 54 insertions(+), 50 deletions(-)

diff --git a/Documentation/git-add.txt b/Documentation/git-add.txt
index d50fa339d..927a152b0 100644
--- a/Documentation/git-add.txt
+++ b/Documentation/git-add.txt
@@ -3,7 +3,7 @@ git-add(1)
 
 NAME
 
-git-add - Add file contents to the index
+git-add - Add file contents to the staging area
 
 SYNOPSIS
 
@@ -15,23 +15,24 @@ SYNOPSIS
 
 DESCRIPTION
 ---
-This command updates the index using the current content found in
-the working tree, to prepare the content staged for the next commit.
-It typically adds the current content of existing paths as a whole,
+This command updates the staging area using the current content found
+in the working tree.
+This command typically adds the current content of existing paths as a whole,
 but with some options it can also be used to add content with
 only part of the changes made to the working tree files applied, or
 remove paths that do not exist in the working tree anymore.
 
-The "index" holds a snapshot of the content of the working tree, and it
-is this snapshot that is taken as the contents of the next commit.  Thus
-after making any changes to the working tree, and before running
-the commit command, you must use the `add` command to add any new or
-modified files to the index.
+The staging area (historically called the "index" or "cache")
+holds a snapshot of the content of the working tree, and it
+is this snapshot that is taken by default as the contents of the next commit.
+Thus after making any changes to the working tree, and before running
+the commit command, you can use the `add` command to add any new or
+modified files to the staging area.
 
 This command can be performed multiple times before a commit.  It only
 adds the content of the specified file(s) at the time the add command is
 run; if you want subsequent changes included in the next commit, then
-you must run `git add` again to add the new content to the index.
+you must run `git add` again to add the new content to the staging area.
 
 The `git status` command can be used to obtain a summary of which
 files have changes that are staged for the next commit.
@@ -45,7 +46,9 @@ be used to add ignored files with the `-f` (force) option.
 
 Please see linkgit:git-commit[1] for alternative ways to add content to a
 commit.
-
+For example, you can use the git commit `-a` option to first automatically
+add to the staging area all the files that have been have been
+modified or deleted in the working tree.
 
 OPTIONS
 ---
@@ -53,7 +56,7 @@ OPTIONS
Files to add content from.  Fileglobs (e.g. `*.c`) can
be given to add all matching files.  Also a
leading directory name (e.g. `dir` to add `dir/file1`
-   and `dir/file2`) can be given to update the index to
+   and `dir/file2`) can be given to update the staging area to
match the current state of the directory as a whole (e.g.
specifying `dir` will record not just a file `dir/file1`
modified in the working tree, a file `dir/file2` added to
@@ -81,16 +84,16 @@ in linkgit:gitglossary[7].
 -i::
 --interactive::
Add modified contents in the working tree interactively to
-   the index. Optional path arguments may be supplied to limit
+   the staging area. Optional path arguments may be supplied to limit
operation to a subset of the working tree. See ``Interactive
mode'' for details.
 
 -p::
 --patch::
-   Interactively choose hunks of patch between the index and the
-   work tree and add them to the index. This gives the user a chance
+   Interactively choose hunks of patch between the staging area and the
+   work tree and add them to the staging area. This gives the user a chance
to review the difference before adding modified contents to the
-   index.
+   staging area.
 +
 This effectively runs `add --interactive`, but bypasses the
 initial command menu and directly jumps to the `patch` subcommand.
@@ -98,20 +101,20 @@ See ``Interactive mode'' for details.
 
 -e::
 --edit::
-   Open the diff vs. the index in an editor and let the user
+   Open the diff vs. the staging area in an editor and let the user
edit it.  After the editor was closed, adjust the hunk headers
-   and apply the patch to the index.
+   and apply the patch to the staging area.
 +
 The intent of this option is to pick and choose lines of the patch to
 apply, or even to modify the contents 

Re: [PATCH v4 00/34] Add directory rename detection to git

[Junio C Hamano]

OK, it seems that I managed to make this test pass under poison build
(see https://travis-ci.org/git/git/jobs/315658242)

Please check 
https://github.com/git/git/commit/e5c5e24ad91a75b5a70c056fe6c6e3bfb55b56fc
and sprinkle its fix to whichever original commits in the series that
need fixing.

Thanks.

Re: [PATCH v4 00/34] Add directory rename detection to git

[Junio C Hamano]

Elijah Newren  writes:

> This patchset introduces directory rename detection to merge-recursive.

The use of negated form of test_i18ngrep in these patches are all
wrong.  Because the helper must say "even though the string does not
match (does match), the test expects it to match (does not match),
and we know that expectation won't hold simply because we are under
poison build", so negating the result of test_i18ngrep won't work.
Instead, you would tell test_i18ngrep that we do not expect it to
find matching lines.

Even with the attached, test #70 will still fail because you have a
construct that greps in output of test_i18ngrep.  That won't work
under poison build, because the output of test_i18ngrep won't have
the string you are looking for under poison build.

We may probably want to redirect the output of underlying grep to
/dev/null in test_i18ngrep to make this kind of misuse easier to
spot.

 t/t6043-merge-rename-directories.sh | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/t/t6043-merge-rename-directories.sh 
b/t/t6043-merge-rename-directories.sh
index f64c7d273b..8f58f08ed2 100755
--- a/t/t6043-merge-rename-directories.sh
+++ b/t/t6043-merge-rename-directories.sh
@@ -554,7 +554,7 @@ test_expect_success '2b-check: Directory split into two on 
one side, with equal
git rev-parse >expect \
O:z/b O:z/c B:x/d &&
test_cmp expect actual &&
-   ! test_i18ngrep "CONFLICT.*directory rename split" out
+   test_i18ngrep ! "CONFLICT.*directory rename split" out
)
 '
 
@@ -705,7 +705,7 @@ test_expect_success '3b-check: Avoid implicit rename if 
involved as source on cu
test_cmp expect actual &&
 
test_i18ngrep CONFLICT.*rename/rename.*z/d.*x/d.*w/d out &&
-   ! test_i18ngrep CONFLICT.*rename/rename.*y/d
+   test_i18ngrep ! CONFLICT.*rename/rename.*y/d
)
 '
 
@@ -3146,7 +3146,7 @@ test_expect_failure '10e-check: Does git complain about 
untracked file that is n
echo random >z/c &&
 
git merge -s recursive B^0 >out 2>err &&
-   ! test_i18ngrep "following untracked working tree files would be 
overwritten by merge" err &&
+   test_i18ngrep ! "following untracked working tree files would 
be overwritten by merge" err &&
 
test 3 -eq $(git ls-files -s | wc -l) &&
test 0 -eq $(git ls-files -u | wc -l) &&

Re: [PATCH] transport: remove unused "push" in vtable

[Jeff King]

On Tue, Dec 12, 2017 at 03:10:56PM -0800, Jonathan Tan wrote:

> After commit 0d0bac67ce3b ("transport: drop support for git-over-rsync",
> 2016-02-01), no transport in Git populates the "push" entry in the
> transport vtable. Remove this entry.

Yay. Thanks for cleaning this up.

-Peff

Re: [PATCH] partial-clone: design doc

[Junio C Hamano]

"Philip Oakley"  writes:

>> +  These filtered packfiles are incomplete in the traditional sense
>> because
>> +  they may contain trees that reference blobs that the client does
>> not have.
>
> Is a comment needed here noting that currently, IIUC, the complete
> trees are fetched in the packfiles, it's just the un-necessary blobs
> that are omitted ?

I probably am misreading what you meant to say, but the above
statement with "currently" taken literally to mean the system
without JeffH's changes, is false.

When the receiver says it has commit A and the sender wants to send
a commit B (because the receiver said it does not have it, and it
wants it), trees in A are not sent in the pack the sender sends to
give objects sufficient to complete B, which the receiver wanted to
have, even if B also has those trees.  If you fetch from me twice
and between that time Documentation/ directory did not change, the
second fetch will not have the tree object that corresponds to that
hierarchy (and of course no blobs and sub trees inside it).

So "the complete trees are fetched" is not true.  What is true (and
what matters more in JeffH's document) is that fetching is done in
such a way that objects resulting in the receiving repository are
complete in the current system that does not allow promised objects.
If some objects resulting in the receiving repository are incomplete,
the current system considers that we corrupted the repository.

The promise mechanism says that it is fine for the receiving end to
lack blobs, trees or commits, as long as the promisor repository
tells it that these "missing" objects can be obtained from it later.
The way the receiving end which notices that it does not have an
otherwise required blob, tree or commit is one promised by the
promisor repository is to see if it is referenced by a pack that
came from such a promisor repository.

RE: [Proposed] Externalize man/html ref for quick-install-man and quick-install-html

[Randall S. Becker]

On December 12, 2017 6:40 PM Junio C Hamano wrote to my own embarrassment:
"Randall S. Becker"  writes:

>> Yes, needed. The lines wrapped om Documentation/Makefile - each change 
>> in quick-install-man/html should be exactly one line:
>>
>> quick-install-man: require-manrepo
>> -'$(SHELL_PATH_SQ)' ./install-doc-quick.sh $(MAN_REPO) 
>> $(DESTDIR)$(mandir)
>> +'$(SHELL_PATH_SQ)' ./install-doc-quick.sh $(MAN_REPO) 
>> +$(DESTDIR)$(mandir) $(GIT_MAN_REF)
>>  
>> And here
>>
>>  quick-install-html: require-htmlrepo
>> -'$(SHELL_PATH_SQ)' ./install-doc-quick.sh $(HTML_REPO) 
>> $(DESTDIR)$(htmldir)
>> +'$(SHELL_PATH_SQ)' ./install-doc-quick.sh $(HTML_REPO) 
>> +$(DESTDIR)$(htmldir) $(GIT_MAN_REF)>

>To everybody else who did not complain that what I sent was line-wrapped, the 
>message should be showing like this:
>https://public-inbox.org/git/xmqqtvwvy1rh@gitster.mtv.corp.google.com/
It is correct at the above link. My mailer is Outlook 2016... so... yeah.

>Perhaps the mail program on your receiving end is mangling what you got from 
>the mailing list, giving you a line-wrapped version.
Yes it is. It loves mangling. Nice to see it mangled it again ☹. Porting 
sendmail was on my list of things to do, but pretty far down.

>It also unfortunately makes me suspect that you didn't actually have a chance 
>to apply the patch mechanically and make sure it works for you due to mail 
>mangling at your end X-<.
I have no such capability on the system where the changes were made, nor even 
with Outlook on my own local Windows dev box. I've tried my mac and linux 
machines but can't connect up to my (bleep) mailer from those without creating 
more (bleep). It's either that or I'm too close to the holidays.

>> And otherwise please consider it signed off.
>Will do, thanks.



Re: [Proposed] Externalize man/html ref for quick-install-man and quick-install-html

[Junio C Hamano]

"Randall S. Becker"  writes:

> Yes, needed. The lines wrapped om Documentation/Makefile - each
> change in quick-install-man/html should be exactly one line:
>
> quick-install-man: require-manrepo
> - '$(SHELL_PATH_SQ)' ./install-doc-quick.sh $(MAN_REPO) 
> $(DESTDIR)$(mandir)
> + '$(SHELL_PATH_SQ)' ./install-doc-quick.sh $(MAN_REPO) 
> $(DESTDIR)$(mandir) $(GIT_MAN_REF)
>  
> And here
>
>  quick-install-html: require-htmlrepo
> - '$(SHELL_PATH_SQ)' ./install-doc-quick.sh $(HTML_REPO) 
> $(DESTDIR)$(htmldir)
> + '$(SHELL_PATH_SQ)' ./install-doc-quick.sh $(HTML_REPO) 
> $(DESTDIR)$(htmldir) $(GIT_MAN_REF)

I somehow have a feeling that you are not even looking at the right
rendition.

To everybody else who did not complain that what I sent was
line-wrapped, the message should be showing like this:

https://public-inbox.org/git/xmqqtvwvy1rh@gitster.mtv.corp.google.com/

Perhaps the mail program on your receiving end is mangling what you
got from the mailing list, giving you a line-wrapped version.

It also unfortunately makes me suspect that you didn't actually have
a chance to apply the patch mechanically and make sure it works for
you due to mail mangling at your end X-<.

> And otherwise please consider it signed off.

Will do, thanks.

Re: [PATCH] partial-clone: design doc

[Philip Oakley]

From: "Jeff Hostetler" 

From: Jeff Hostetler 

First draft of design document for partial clone feature.

Signed-off-by: Jeff Hostetler 
Signed-off-by: Jonathan Tan 
---
Documentation/technical/partial-clone.txt | 240 
++

1 file changed, 240 insertions(+)
create mode 100644 Documentation/technical/partial-clone.txt

diff --git a/Documentation/technical/partial-clone.txt 
b/Documentation/technical/partial-clone.txt

new file mode 100644
index 000..7ab39d8
--- /dev/null
+++ b/Documentation/technical/partial-clone.txt
@@ -0,0 +1,240 @@
+Partial Clone Design Notes
+==
+
+The "Partial Clone" feature is a performance optimization for git that
+allows git to function without having a complete copy of the repository.
+


I think it would be worthwhile at least listing the issues that make the 
'optimisation' necessary, and then the available factors that make the 
optimisation possible. This helps for future adjustments when those issues 
and factors change.


I think the issues are:
* the size of the repository that is being cloned, both in the width of a 
commit (you mentioned 100M trees) and the time (hours to days) / size to 
clone over the connection.


While the supporting factor is:
* the remote is always on-line and available for on-demand object fetching 
(seconds)


The solution choice then should fall out fairly obviously, and we can 
separate out the other optimisations that are based on other views about the 
issues. E.g. my desire for a solution in the off-line case.


In fact the current design, apart from some terminology, does look well 
matched, with only a couple of places that would be affected.


The airplane-mode expectations of a partial clone should also be stated.



+During clone and fetch operations, git normally downloads the complete
+contents and history of the repository.  That is, during clone the client
+receives all of the commits, trees, and blobs in the repository into a
+local ODB.  Subsequent fetches extend the local ODB with any new objects.
+For large repositories, this can take significant time to download and
+large amounts of diskspace to store.
+
+The goal of this work is to allow git better handle extremely large
+repositories.


Shouln't this goal be nearer the top?


   Often in these repositories there are many files that the
+user does not need such as ancient versions of source files, files in
+portions of the worktree outside of the user's work area, or large binary
+assets.  If we can avoid downloading such unneeded objects *in advance*
+during clone and fetch operations, we can decrease download times and
+reduce ODB disk usage.
+


Does this need to distinguish between the shallow clone mechanism for 
reducing the cloning of old history from the desire for a width wise partial 
clone of only the users narrow work area, and/or without large files/blobs?



+
+Non-Goals
+-
+
+Partial clone is independent of and not intended to conflict with
+shallow-clone, refspec, or limited-ref mechanisms since these all operate
+at the DAG level whereas partial clone and fetch works *within* the set
+of commits already chosen for download.
+
+
+Design Overview
+---
+
+Partial clone logically consists of the following parts:
+
+- A mechanism for the client to describe unneeded or unwanted objects to
+  the server.
+
+- A mechanism for the server to omit such unwanted objects from packfiles
+  sent to the client.
+
+- A mechanism for the client to gracefully handle missing objects (that
+  were previously omitted by the server).
+
+- A mechanism for the client to backfill missing objects as needed.
+
+
+Design Details
+--
+
+- A new pack-protocol capability "filter" is added to the fetch-pack and
+  upload-pack negotiation.
+
+  This uses the existing capability discovery mechanism.
+  See "filter" in Documentation/technical/pack-protocol.txt.
+
+- Clients pass a "filter-spec" to clone and fetch which is passed to the
+  server to request filtering during packfile construction.
+
+  There are various filters available to accomodate different situations.
+  See "--filter=" in Documentation/rev-list-options.txt.
+
+- On the server pack-objects applies the requested filter-spec as it
+  creates "filtered" packfiles for the client.
+
+  These filtered packfiles are incomplete in the traditional sense 
because
+  they may contain trees that reference blobs that the client does not 
have.


Is a comment needed here noting that currently, IIUC, the complete trees are 
fetched in the packfiles, it's just the un-necessary blobs that are omitted 
?



+
+
+ How the local repository gracefully handles missing objects
+
+With partial clone, the fact that objects can be missing makes such
+repositories incompatible with older versions of Git, necessitating a
+repository extension (see the 

RE: [Proposed] Externalize man/html ref for quick-install-man and quick-install-html

[Randall S. Becker]

-Original Message-
On December 12, 2017 6:18 PM Junio C Hamano wrote:
Subject: Re: [Proposed] Externalize man/html ref for quick-install-man and 
quick-install-html
>"Randall S. Becker"  writes:
>> I can send you a pull request on github, if you want 
>I don't.  It's not that I can or cannot take a pull request.  I just do not 
>want to queue anything that is not reviwed on list.
No worries.

>I however could queue this (typed to mimic what I saw in your message), but 
>you'd need to say what you see here is OK (and preferably, you applied this 
>version and it tested fine); I may have made a typo or two, and I would really 
>prefer extra set of eyes.
Yes, needed. The lines wrapped om Documentation/Makefile - each change in 
quick-install-man/html should be exactly one line:

quick-install-man: require-manrepo
-   '$(SHELL_PATH_SQ)' ./install-doc-quick.sh $(MAN_REPO) 
$(DESTDIR)$(mandir)
+   '$(SHELL_PATH_SQ)' ./install-doc-quick.sh $(MAN_REPO) 
$(DESTDIR)$(mandir) $(GIT_MAN_REF)
 
And here

 quick-install-html: require-htmlrepo
-   '$(SHELL_PATH_SQ)' ./install-doc-quick.sh $(HTML_REPO) 
$(DESTDIR)$(htmldir)
+   '$(SHELL_PATH_SQ)' ./install-doc-quick.sh $(HTML_REPO) 
$(DESTDIR)$(htmldir) $(GIT_MAN_REF)

And otherwise please consider it signed off.

Signed-off-by: Randall S. Becker 

-- >8 --
From: "Randall S. Becker" 
Date: Sat, 9 Dec 2017 17:07:57 -0500
Subject: [PATCH] install-doc-quick: allow specifying what ref to install

We allow the builders, who want to install the preformatted manpages and html 
documents, to specify where in their filesystem these two repositories are 
stored.  Let them also specify which ref (or even a
revision) to grab the preformatted material from.
---
 Documentation/Makefile | 5 +++--
 Documentation/install-doc-quick.sh | 9 +
 2 files changed, 8 insertions(+), 6 deletions(-)

diff --git a/Documentation/Makefile b/Documentation/Makefile index 
2ab65561af..4ae9ba5c86 100644
--- a/Documentation/Makefile
+++ b/Documentation/Makefile
@@ -39,6 +39,7 @@ MAN7_TXT += gitworkflows.txt  MAN_TXT = $(MAN1_TXT) 
$(MAN5_TXT) $(MAN7_TXT)  MAN_XML = $(patsubst %.txt,%.xml,$(MAN_TXT))  MAN_HTML 
= $(patsubst %.txt,%.html,$(MAN_TXT))
+GIT_MAN_REF = master
 
 OBSOLETE_HTML += everyday.html
 OBSOLETE_HTML += git-remote-helpers.html @@ -437,14 +438,14 @@ 
require-manrepo::
then echo "git-manpages repository must exist at $(MAN_REPO)"; exit 1; 
fi
 
 quick-install-man: require-manrepo
-   '$(SHELL_PATH_SQ)' ./install-doc-quick.sh $(MAN_REPO) 
$(DESTDIR)$(mandir)
+   '$(SHELL_PATH_SQ)' ./install-doc-quick.sh $(MAN_REPO) 
+$(DESTDIR)$(mandir) $(GIT_MAN_REF)
 
 require-htmlrepo::
@if test ! -d $(HTML_REPO); \
then echo "git-htmldocs repository must exist at $(HTML_REPO)"; exit 1; 
fi
 
 quick-install-html: require-htmlrepo
-   '$(SHELL_PATH_SQ)' ./install-doc-quick.sh $(HTML_REPO) 
$(DESTDIR)$(htmldir)
+   '$(SHELL_PATH_SQ)' ./install-doc-quick.sh $(HTML_REPO) 
+$(DESTDIR)$(htmldir) $(GIT_MAN_REF)
 
 print-man1:
@for i in $(MAN1_TXT); do echo $$i; done diff --git 
a/Documentation/install-doc-quick.sh b/Documentation/install-doc-quick.sh
index 327f69bcf5..17231d8e59 100755
--- a/Documentation/install-doc-quick.sh
+++ b/Documentation/install-doc-quick.sh
@@ -3,11 +3,12 @@
 
 repository=${1?repository}
 destdir=${2?destination}
+GIT_MAN_REF=${3?master}
 
-head=master GIT_DIR=
+GIT_DIR=
 for d in "$repository/.git" "$repository"
 do
-   if GIT_DIR="$d" git rev-parse refs/heads/master >/dev/null 2>&1
+   if GIT_DIR="$d" git rev-parse "$GIT_MAN_REF" >/dev/null 2>&1
then
GIT_DIR="$d"
export GIT_DIR
@@ -27,12 +28,12 @@ export GIT_INDEX_FILE GIT_WORK_TREE  rm -f "$GIT_INDEX_FILE"
 trap 'rm -f "$GIT_INDEX_FILE"' 0
 
-git read-tree $head
+git read-tree "$GIT_MAN_REF"
 git checkout-index -a -f --prefix="$destdir"/
 
 if test -n "$GZ"
 then
-   git ls-tree -r --name-only $head |
+   git ls-tree -r --name-only "$GIT_MAN_REF" |
xargs printf "$destdir/%s\n" |
xargs gzip -f
 fi
--
2.15.1-525-g09180b8600

Re: [PATCH] Makefile: allow NO_DC_SHA1_SUBMODULE to be set in config.mak

[Ævar Arnfjörð Bjarmason]

On Tue, Dec 12 2017, Junio C. Hamano jotted:

> Junio C Hamano  writes:
>
>> Ævar Arnfjörð Bjarmason  writes:
>>
 I actually think that the block can go even further down, perhaps
 close to the run of choices "what variant are we building?" we make
 at around we have "ifdef NO_CURL".

 Ævar?
>>>
>>> Makes sense to me, do you want to squash this + your proposed edit &
>>> I'll pick it up if there's another version, or I can re-submit.
>>
>> OK.  I'll squash in the 'move it further down' to the original
>> commit that removed DC_SHA1_SUBMODULE and added NO_DC_SHA1_SUBMODULE
>> when rebuilding 'pu' branch.
>>
>> Thanks.
>
> Another minor thing I noticed (which I do not have any squashable
> fix for) is that "make distclean" does not even work without
> submodule or this environment, which feels a bit too excessive.  I
> haven't tried to figure out how involved a fix for that would be yet
> and I do not mind leaving it broken if it would be too much work.

Yes, that sucks. I can't think of a better fix for it than this massive
hack of outsourcing printing the error to the header itself, which'll
only be executed if we actually compile:

diff --git a/Makefile b/Makefile
index ba3e061edd..881cf55159 100644
--- a/Makefile
+++ b/Makefile
@@ -1022,9 +1022,7 @@ GIT_USER_AGENT = git/$(GIT_VERSION)

 ifndef DC_SHA1_EXTERNAL
ifneq ($(wildcard 
sha1collisiondetection/lib/sha1.h),sha1collisiondetection/lib/sha1.h)
-$(error The sha1collisiondetection submodule is not checked out. \
-Please make it available, either by cloning with --recurse-submodules, \
-or by running "git submodule update --init".)
+   BASIC_CFLAGS += -DERROR_SHA1COLLISIONDETECTION_SUBMODULE
endif
 else
ifdef NO_DC_SHA1_SUBMODULE
diff --git a/sha1dc_git.h b/sha1dc_git.h
index be1d48abbe..93a9be976a 100644
--- a/sha1dc_git.h
+++ b/sha1dc_git.h
@@ -1,5 +1,11 @@
 /* Plumbing with collition-detecting SHA1 code */

+#ifdef ERROR_SHA1COLLISIONDETECTION_SUBMODULE
+#error "The sha1collisiondetection submodule is not checked out." \
+   "Please make it available, either by cloning with 
--recurse-submodules," \
+   "or by running \"git submodule update --init\"."
+#endif
+
 #ifdef DC_SHA1_EXTERNAL
 #include 
 #else

But maybe I missed a way to make the $(error) path work only if certain
targets were about to be compiled.

Re: [Proposed] Externalize man/html ref for quick-install-man and quick-install-html

[Junio C Hamano]

"Randall S. Becker"  writes:

> Sorry about the response positioning...
>
> I can send you a pull request on github, if you want 

I don't.  It's not that I can or cannot take a pull request.  I just
do not want to queue anything that is not reviwed on list.

I however could queue this (typed to mimic what I saw in your
message), but you'd need to say what you see here is OK (and
preferably, you applied this version and it tested fine); I may have
made a typo or two, and I would really prefer extra set of eyes.

Also we need your sign-off, of course.

Thanks.

-- >8 --
From: "Randall S. Becker" 
Date: Sat, 9 Dec 2017 17:07:57 -0500
Subject: [PATCH] install-doc-quick: allow specifying what ref to install

We allow the builders, who want to install the preformatted manpages
and html documents, to specify where in their filesystem these two
repositories are stored.  Let them also specify which ref (or even a
revision) to grab the preformatted material from.
---
 Documentation/Makefile | 5 +++--
 Documentation/install-doc-quick.sh | 9 +
 2 files changed, 8 insertions(+), 6 deletions(-)

diff --git a/Documentation/Makefile b/Documentation/Makefile
index 2ab65561af..4ae9ba5c86 100644
--- a/Documentation/Makefile
+++ b/Documentation/Makefile
@@ -39,6 +39,7 @@ MAN7_TXT += gitworkflows.txt
 MAN_TXT = $(MAN1_TXT) $(MAN5_TXT) $(MAN7_TXT)
 MAN_XML = $(patsubst %.txt,%.xml,$(MAN_TXT))
 MAN_HTML = $(patsubst %.txt,%.html,$(MAN_TXT))
+GIT_MAN_REF = master
 
 OBSOLETE_HTML += everyday.html
 OBSOLETE_HTML += git-remote-helpers.html
@@ -437,14 +438,14 @@ require-manrepo::
then echo "git-manpages repository must exist at $(MAN_REPO)"; exit 1; 
fi
 
 quick-install-man: require-manrepo
-   '$(SHELL_PATH_SQ)' ./install-doc-quick.sh $(MAN_REPO) 
$(DESTDIR)$(mandir)
+   '$(SHELL_PATH_SQ)' ./install-doc-quick.sh $(MAN_REPO) 
$(DESTDIR)$(mandir) $(GIT_MAN_REF)
 
 require-htmlrepo::
@if test ! -d $(HTML_REPO); \
then echo "git-htmldocs repository must exist at $(HTML_REPO)"; exit 1; 
fi
 
 quick-install-html: require-htmlrepo
-   '$(SHELL_PATH_SQ)' ./install-doc-quick.sh $(HTML_REPO) 
$(DESTDIR)$(htmldir)
+   '$(SHELL_PATH_SQ)' ./install-doc-quick.sh $(HTML_REPO) 
$(DESTDIR)$(htmldir) $(GIT_MAN_REF)
 
 print-man1:
@for i in $(MAN1_TXT); do echo $$i; done
diff --git a/Documentation/install-doc-quick.sh 
b/Documentation/install-doc-quick.sh
index 327f69bcf5..17231d8e59 100755
--- a/Documentation/install-doc-quick.sh
+++ b/Documentation/install-doc-quick.sh
@@ -3,11 +3,12 @@
 
 repository=${1?repository}
 destdir=${2?destination}
+GIT_MAN_REF=${3?master}
 
-head=master GIT_DIR=
+GIT_DIR=
 for d in "$repository/.git" "$repository"
 do
-   if GIT_DIR="$d" git rev-parse refs/heads/master >/dev/null 2>&1
+   if GIT_DIR="$d" git rev-parse "$GIT_MAN_REF" >/dev/null 2>&1
then
GIT_DIR="$d"
export GIT_DIR
@@ -27,12 +28,12 @@ export GIT_INDEX_FILE GIT_WORK_TREE
 rm -f "$GIT_INDEX_FILE"
 trap 'rm -f "$GIT_INDEX_FILE"' 0
 
-git read-tree $head
+git read-tree "$GIT_MAN_REF"
 git checkout-index -a -f --prefix="$destdir"/
 
 if test -n "$GZ"
 then
-   git ls-tree -r --name-only $head |
+   git ls-tree -r --name-only "$GIT_MAN_REF" |
xargs printf "$destdir/%s\n" |
xargs gzip -f
 fi
-- 
2.15.1-525-g09180b8600

[PATCH] transport: remove unused "push" in vtable

[Jonathan Tan]

After commit 0d0bac67ce3b ("transport: drop support for git-over-rsync",
2016-02-01), no transport in Git populates the "push" entry in the
transport vtable. Remove this entry.

Signed-off-by: Jonathan Tan 
---
I was taking a look at the transport code and noticed that push is
unused (and push_refs is used instead). Here is a code cleanup.
---
 transport.c | 9 +
 transport.h | 1 -
 2 files changed, 1 insertion(+), 9 deletions(-)

diff --git a/transport.c b/transport.c
index 7231d1b1b..7cc39b7c0 100644
--- a/transport.c
+++ b/transport.c
@@ -627,7 +627,6 @@ void transport_take_over(struct transport *transport,
transport->set_option = NULL;
transport->get_refs_list = get_refs_via_connect;
transport->fetch = fetch_refs_via_pack;
-   transport->push = NULL;
transport->push_refs = git_transport_push;
transport->disconnect = disconnect_git;
transport->smart_options = &(data->options);
@@ -969,13 +968,7 @@ int transport_push(struct transport *transport,
*reject_reasons = 0;
transport_verify_remote_names(refspec_nr, refspec);
 
-   if (transport->push) {
-   /* Maybe FIXME. But no important transport uses this case. */
-   if (flags & TRANSPORT_PUSH_SET_UPSTREAM)
-   die("This transport does not support using 
--set-upstream");
-
-   return transport->push(transport, refspec_nr, refspec, flags);
-   } else if (transport->push_refs) {
+   if (transport->push_refs) {
struct ref *remote_refs;
struct ref *local_refs = get_local_heads();
int match_flags = MATCH_REFS_NONE;
diff --git a/transport.h b/transport.h
index bc5571574..ab4fe7f27 100644
--- a/transport.h
+++ b/transport.h
@@ -103,7 +103,6 @@ struct transport {
 * process involved generating new commits.
 **/
int (*push_refs)(struct transport *transport, struct ref *refs, int 
flags);
-   int (*push)(struct transport *connection, int refspec_nr, const char 
**refspec, int flags);
int (*connect)(struct transport *connection, const char *name,
   const char *executable, int fd[2]);
 
-- 
2.15.1.424.g9478a66081-goog

Re: [PATCH] Makefile: allow NO_DC_SHA1_SUBMODULE to be set in config.mak

[Junio C Hamano]

Junio C Hamano  writes:

> Ævar Arnfjörð Bjarmason  writes:
>
>>> I actually think that the block can go even further down, perhaps
>>> close to the run of choices "what variant are we building?" we make
>>> at around we have "ifdef NO_CURL".
>>>
>>> Ævar?
>>
>> Makes sense to me, do you want to squash this + your proposed edit &
>> I'll pick it up if there's another version, or I can re-submit.
>
> OK.  I'll squash in the 'move it further down' to the original
> commit that removed DC_SHA1_SUBMODULE and added NO_DC_SHA1_SUBMODULE
> when rebuilding 'pu' branch.
>
> Thanks.

Another minor thing I noticed (which I do not have any squashable
fix for) is that "make distclean" does not even work without
submodule or this environment, which feels a bit too excessive.  I
haven't tried to figure out how involved a fix for that would be yet
and I do not mind leaving it broken if it would be too much work.

Re: [PATCH v3] Makefile: replace perl/Makefile.PL with simple make rules

[Ævar Arnfjörð Bjarmason]

On Tue, Dec 12 2017, Randall S. Becker jotted:

> -Original Message-
> On December 10, 2017 4:14 PM, Ævar Arnfjörð Bjarmason wrote:
> Subject: [PATCH v3] Makefile: replace perl/Makefile.PL with simple make rules
>
>>Replace the perl/Makefile.PL and the fallback perl/Makefile used under 
>>NO_PERL_MAKEMAKER=NoThanks with a much simpler implementation heavily 
>>inspired by how the i18n infrastructure's build process works[1].
>>The reason for having the Makefile.PL in the first place is that it was 
>>initially[2] building a perl C binding to interface with libgit, this 
>>functionality, that was removed[3] before Git.pm ever made it to the master 
>>branch.
> 
>
> I would like to request that the we be careful that the git builds do not 
> introduce arbitrary dependencies to CPAN. Some platforms (I can think of one 
> off the top, being NonStop) does not provide for arbitrary additions to the 
> supplied perl implementation as of yet. The assumption about being able to 
> add CPAN modules may apply on some platforms but is not a general capability. 
> I am humbly requesting that caution be used when adding dependencies. Being 
> non-$DAYJOB responsible for the git port for NonStop, this scares me a bit, 
> but I and my group can help validate the available modules used for builds.
>
> Note: we do not yet have CPAN's SCM so can't and don't use perl for access to 
> git anyway - much that I've tried to change that.
>
> Please keep build dependencies to a minimum.
>
> Thanks for my and my whole team.

I think you should be happy with this patch then, and it doesn't add any
more CPAN dependency than before, and sets up a framework (as discussed
in [1]) where we can use more CPAN modules while not requiring packagers
such as yourself to package CPAN modules.

However, it doesn't sound believable to me that even on NonStop you
can't install any CPAN modules whatsoever.

That would also mean that this patch doesn't work for you, because it
means that you either don't have anything resembling a hierarchical
filesystem on which git can be installed in the first place (in which
case it wouldn't work), or perl doesn't have an @INC to search through
perl libs on on NonStop. What does:

perl -V

Return for you on that system?

If this patch works, and if at the bottom of `perl -V` you see some
directories which you could write a package to drop some static *.pm
files, then you can grab a *.tar.gz from CPAN such as the one for
Error.pm[2] and arrange for the *.pm files contained within its lib/
directory to be dropped into one of those @INC directories.

It may be that some aspect of the CPAN toolchain is broken for you, or
even ExtUtils::MakeMaker, but you typically don't need that to package
non-XS perl modules, certainly not any of the ones we've discussed
possibly bundling up in git.git on-list recently. As a (very occasional)
contributor to perl.git I'd be interested to know if that's what you
mean is broken, and if so see if it could be fixed for you.

1. 

Re: [PATCH] Makefile: allow NO_DC_SHA1_SUBMODULE to be set in config.mak

[Junio C Hamano]

Ævar Arnfjörð Bjarmason  writes:

>> I actually think that the block can go even further down, perhaps
>> close to the run of choices "what variant are we building?" we make
>> at around we have "ifdef NO_CURL".
>>
>> Ævar?
>
> Makes sense to me, do you want to squash this + your proposed edit &
> I'll pick it up if there's another version, or I can re-submit.

OK.  I'll squash in the 'move it further down' to the original
commit that removed DC_SHA1_SUBMODULE and added NO_DC_SHA1_SUBMODULE
when rebuilding 'pu' branch.

Thanks.

Re: [PATCH] git-send-email: fix get_maintainer.pl regression

[Junio C Hamano]

Ævar Arnfjörð Bjarmason  writes:

> Ripping out Error.pm for our few internal callers is one thing, trying
> to maintain bugwards compatibility with how it throws exceptions for
> users expecting Error.pm objects is another. I think at that point it's
> easier to just stay with Error.pm.
>
> Probably easier to stay with it either way, don't poke sleeping dragons
> and all that, it's working code, even if we wouldn't write it like that
> today the churn probably isn't worth it.

OK, I'm inclined to defer to your judgment.

THanks.

Re: [PATCH Outreachy v2 1/2] format: create pretty.h file

[Junio C Hamano]

Olga Telezhnaya  writes:

>  builtin/notes.c   |  2 +-
>  builtin/reset.c   |  2 +-
>  builtin/show-branch.c |  2 +-
>  commit.h  | 81 +--
>  pretty.h  | 87 
> +++
>  revision.h|  2 +-
>  6 files changed, 92 insertions(+), 84 deletions(-)
>  create mode 100644 pretty.h
>
> diff --git a/builtin/notes.c b/builtin/notes.c
> index 1a2c7d92ad7e7..7c8176164561b 100644
> --- a/builtin/notes.c
> +++ b/builtin/notes.c
> @@ -12,7 +12,7 @@
>  #include "builtin.h"
>  #include "notes.h"
>  #include "blob.h"
> -#include "commit.h"
> +#include "pretty.h"
>  #include "refs.h"
>  #include "exec_cmd.h"
>  #include "run-command.h"
> ...
> diff --git a/commit.h b/commit.h
> index 99a3fea68d3f6..8c68ca1a5a187 100644
> --- a/commit.h
> +++ b/commit.h
> @@ -7,6 +7,7 @@
>  #include "decorate.h"
>  #include "gpg-interface.h"
>  #include "string-list.h"
> +#include "pretty.h"

This is much nicer than what I imagined, which was to just add this
line here, move decls from commit.h to pretty.h, and do nothing
else, which would be the absolute safest thing from the point of
view of other topics in flight.  Separation of "pretty.h" would stay
to be an implementation detail of the "commit.h" file, where
everybody expects to find these decls.

Instead, this patch inspects each and every .c user of "commit.h"
and replaces its '#include' with the new one if it only uses things
declared in "pretty.h", which makes it very clear who have been
depending on what in the patch.  Those that include "commit.h"
because they need both the "what is a commit object" aspect and "how
to pretty print" aspect can keep using their original '#include' to
ease the transition.

Let's see how well this plays with other topics in flight---I had to
apply an evil merge to queue the previous one, if I recall right, as
a user of "commit.h" that did not use pretty-print (hence did not
get "pretty.h" with the previous round of this patch) gained use of
pretty-print function, or something like that.

Will queue.

Thanks.

 

RE: [PATCH v3] Makefile: replace perl/Makefile.PL with simple make rules

[Randall S. Becker]

-Original Message-
On December 10, 2017 4:14 PM, Ævar Arnfjörð Bjarmason wrote:
Subject: [PATCH v3] Makefile: replace perl/Makefile.PL with simple make rules

>Replace the perl/Makefile.PL and the fallback perl/Makefile used under 
>NO_PERL_MAKEMAKER=NoThanks with a much simpler implementation heavily inspired 
>by how the i18n infrastructure's build process works[1].
>The reason for having the Makefile.PL in the first place is that it was 
>initially[2] building a perl C binding to interface with libgit, this 
>functionality, that was removed[3] before Git.pm ever made it to the master 
>branch.


I would like to request that the we be careful that the git builds do not 
introduce arbitrary dependencies to CPAN. Some platforms (I can think of one 
off the top, being NonStop) does not provide for arbitrary additions to the 
supplied perl implementation as of yet. The assumption about being able to add 
CPAN modules may apply on some platforms but is not a general capability. I am 
humbly requesting that caution be used when adding dependencies. Being 
non-$DAYJOB responsible for the git port for NonStop, this scares me a bit, but 
I and my group can help validate the available modules used for builds.

Note: we do not yet have CPAN's SCM so can't and don't use perl for access to 
git anyway - much that I've tried to change that.

Please keep build dependencies to a minimum.

Thanks for my and my whole team.

Randall

-- Brief whoami: NonStop developer since approximately 
UNIX(421664400)/NonStop(2112884442) 
-- In my real life, I talk too much.

Re: [PATCH] git-send-email: fix get_maintainer.pl regression

[Ævar Arnfjörð Bjarmason]

On Tue, Dec 12 2017, Junio C. Hamano jotted:

> Ævar Arnfjörð Bjarmason  writes:
>
>> My "Makefile: replace perl/Makefile.PL with simple make rules" currently
>> cooking in pu changes that so that:
>>
>>  * We always at runtime test for the system CPAN module.
>>
>>  * In the case of Error.pm we happen to ship a fallback, in the case of
>>Mail::Address etc. we don't and have fallback code, but we could also
>>just ship a copy and remove the fallback code.
>>
>> This makes more sense, we always "dynamically link" as it were, we'll
>> just change the target to (a presumably newer) system module in the case
>> of Error.pm if it's found on the system, otherwise use our fallback.
>
> "When to fallback" aside, I think the above makes sense for the
> send-email simply because we would be replacing "our own" fallback
> we may need to maintain forever with something with an upstream that
> we do not have to worry too much about.

I'll see about submitting something that replaces the fallback with just
using the CPAN modules + bundling them once the Makefile patch has
cooked to master.

> A tangent; I thought I heard that use of Error.pm is strongly
> discouraged several years ago---am I mistaken, or if I am not,
> perhaps we should start looking into updating the users?

I'm not a fan of it, 41c01693ac ("git-svn: handle merge-base failures",
2010-01-06) shows how you can do that rather simply with just perl's
built-in exceptions.

My TODO list of "perl stuff in git" is now:

 - Get my Makefile.PL thing through
 - Make sure Dan Jacques's relocatable stuff is OK wrt perl on top of that
 - Upgrade the required version from 5.8 to 5.10
 - Update Error.pm itself, our copy is ancient
 - Add more stuff to Git::FromCPAN + remove fallbacks

I could add "rip out Error.pm" to that, it looks rather easy, however
given previous discussion about me needing to build a manpage from
Git.pm I understand that Git.pm is used by code outside of Git itself.

Ripping out Error.pm for our few internal callers is one thing, trying
to maintain bugwards compatibility with how it throws exceptions for
users expecting Error.pm objects is another. I think at that point it's
easier to just stay with Error.pm.

Probably easier to stay with it either way, don't poke sleeping dragons
and all that, it's working code, even if we wouldn't write it like that
today the churn probably isn't worth it.

Re: [PATCH] Makefile: allow NO_DC_SHA1_SUBMODULE to be set in config.mak

[Jonathan Tan]

On Tue, 12 Dec 2017 11:57:28 -0800
Junio C Hamano  wrote:

> Junio C Hamano  writes:
> 
> > Makes sense.  The patch looks scary by appearing to move the
> > includes far to the front of the Makefile, but it in fact is moving
> > the NO_DC_SHA1_SUBMODULE block slightly down and it is a sensible
> > and safe move.
> 
> A completely unrelated tangent.  This was a good change to try your
> anchored diff on.  Viewing this change with
> 
>$ git show --anchored='include config.mak'
> 
> looks a lot less scary than the way it is shown by default (which
> matches what was posted on the list).
> 
> Good job.

Thanks, glad that it helped.

Re: [PATCH] Makefile: allow NO_DC_SHA1_SUBMODULE to be set in config.mak

[Ævar Arnfjörð Bjarmason]

On Tue, Dec 12, 2017 at 8:53 PM, Junio C Hamano  wrote:
> Ramsay Jones  writes:
>
>> Signed-off-by: Ramsay Jones 
>> ---
>>
>> Hi Junio,
>>
>> Could you please add (or squash) this on top of the 'ab/sha1dc-build'
>> branch, so that I can build with NO_DC_SHA1_SUBMODULE=NoThanks in my
>> config.mak.
>
> Makes sense.  The patch looks scary by appearing to move the
> includes far to the front of the Makefile, but it in fact is moving
> the NO_DC_SHA1_SUBMODULE block slightly down and it is a sensible
> and safe move.
>
> I actually think that the block can go even further down, perhaps
> close to the run of choices "what variant are we building?" we make
> at around we have "ifdef NO_CURL".
>
> Ævar?

Makes sense to me, do you want to squash this + your proposed edit &
I'll pick it up if there's another version, or I can re-submit.

>> diff --git a/Makefile b/Makefile
>> index 929b49b04..91bbb0ed8 100644
>> --- a/Makefile
>> +++ b/Makefile
>> @@ -1042,6 +1042,10 @@ EXTLIBS =
>>
>>  GIT_USER_AGENT = git/$(GIT_VERSION)
>>
>> +include config.mak.uname
>> +-include config.mak.autogen
>> +-include config.mak
>> +
>>  ifndef NO_DC_SHA1_SUBMODULE
>>   ifndef DC_SHA1_EXTERNAL
>>   ifneq ($(wildcard 
>> sha1collisiondetection/lib/sha1.h),sha1collisiondetection/lib/sha1.h)
>> @@ -1053,10 +1057,6 @@ whatever reason define NO_DC_SHA1_SUBMODULE=NoThanks)
>>   endif
>>  endif
>>
>> -include config.mak.uname
>> --include config.mak.autogen
>> --include config.mak
>> -
>>  ifdef DEVELOPER
>>  CFLAGS += $(DEVELOPER_CFLAGS)
>>  endif

Re: [PATCH] t/helper: ignore everything but sources

[Todd Zullinger]

Hi Stefan,

Stefan Beller wrote:
>> If we ignore everything but resurrect *.[ch] with negative exclude
>> rules, can we do the same without moving things around?
> 
> Yes, there is also one lonely shell script in there, which also needs
> exclusion.

There aren't currently any .h files, but I suppose it doesn't hurt to
include that pattern to be safer for the future.

> +*
> +!.sh
> +!.[ch]

The ! patterns are missing a '*'.  I think it should be:

*
!*.[ch]
!*.sh

Does it make sense to also include !.gitignore as well?
It's already committed, so it's not ignored.  But perhaps
having it listed will save someone from getting their repo
into a state where local changes to .gitignore aren't picked
up (I know that's a bit of a stretch).

-- 
Todd
~~
How much does it cost to entice a dope-smoking UNIX system guru to
Dayton?
-- Brian Boyle, UNIX/WORLD's First Annual Salary Survey

Re: [PATCH] t/helper: ignore everything but sources

[Junio C Hamano]

Stefan Beller  writes:

> Yes, there is also one lonely shell script in there, which also needs
> exclusion.

Thanks for catching them.

> +*
> +!.sh
> +!.[ch]

I'd use this instead, though.

-- >8 --
*
!*.sh
!*.[ch]
!*.gitignore
-- 8< --

In a dirty repository full of crufts but without any local
modifications, if you do

$ git rm --cached -r t/helper
$ git add t/helper

you should be able to make your index identical to HEAD.  The
version that was posted did not resurrect .gitignore and none
of the source files, but the replaced one should.

[PATCH] t/helper: ignore everything but sources

[Stefan Beller]

Compiled test helpers in t/helper are out of sync with the .gitignore
files quite frequently. This can happen when new test helpers are added,
but the explicit .gitignore file is not updated in the same commit, or
when you forget to 'make clean' before checking out a different version
of git, as the different version may have a different explicit list of
test helpers to ignore.

Fix this by having an overly broad ignore pattern in that directory:
Anything, except C and shell source, will be ignored.

Signed-off-by: Stefan Beller 
---

> If we ignore everything but resurrect *.[ch] with negative exclude
> rules, can we do the same without moving things around?

Yes, there is also one lonely shell script in there, which also needs
exclusion.

 I guess we want to test this overly broad pattern in a sub directory,
 as I could imagine that at the top level directory we'd have more cases
 to think through (I used to put untracked files into the top level dir,
 but I do not do it anymore).
 
Thanks,
Stefan

 t/helper/.gitignore | 43 +++
 1 file changed, 3 insertions(+), 40 deletions(-)

diff --git a/t/helper/.gitignore b/t/helper/.gitignore
index d02f9b39ac..ee1e39fd08 100644
--- a/t/helper/.gitignore
+++ b/t/helper/.gitignore
@@ -1,40 +1,3 @@
-/test-chmtime
-/test-ctype
-/test-config
-/test-date
-/test-delta
-/test-drop-caches
-/test-dump-cache-tree
-/test-dump-fsmonitor
-/test-dump-split-index
-/test-dump-untracked-cache
-/test-fake-ssh
-/test-scrap-cache-tree
-/test-genrandom
-/test-hashmap
-/test-index-version
-/test-lazy-init-name-hash
-/test-line-buffer
-/test-match-trees
-/test-mergesort
-/test-mktemp
-/test-online-cpus
-/test-parse-options
-/test-path-utils
-/test-prio-queue
-/test-read-cache
-/test-ref-store
-/test-regex
-/test-revision-walking
-/test-run-command
-/test-sha1
-/test-sha1-array
-/test-sigchain
-/test-strcmp-offset
-/test-string-list
-/test-submodule-config
-/test-subprocess
-/test-svn-fe
-/test-urlmatch-normalization
-/test-wildmatch
-/test-write-cache
+*
+!.sh
+!.[ch]
-- 
2.15.1.504.g5279b80103-goog

Re: [PATCH 3/3] travis: run tests with GIT_TEST_SPLIT_INDEX

[Junio C Hamano]

Thomas Gummerer  writes:

>
> The breakages wen the split-index code fails tend to break things in
> much more obvious manners than a wrong message, usually git ends up
> dying if it gets broken.  Both of the bugs that were fixed here would
> have been caught with the change in my patch.
>
> But yeah I can see the argument that it doesn't give us a guarantee
> that it catches all things the test suite could catch.

I think you misunderstood me.  When split index is much easier to
break than poison tests, combining them together would hurt the test
coverage of poison tests.  If you value poison tests much more than
how well split index mode works, that is a worse outcome.

>> I wonder if it makes more sense to update ci/run-tests.sh so that
>> its final step is run twice with different settings, like so?
>
> I kind of wanted to avoid that, because it ends up running the test
> suite twice on travis for every test, which seems a bit overkill.  But
> I don't exactly how worried we are about cycles on travis (I don't
> check it very often personally, so I don't really know).  If we aren't
> worried about cycles what you have below would certainly make sense.

I think 64-bit gcc/clang builds tend to cost us about 10-20 minutes
each, and 32-bit linux builds about 10 minutes or so.  I wonder if
it makes sense to do the "run twice" for only one of 64-bit builds
(perhaps the clang one, as I suspect 32-bit linux one uses gcc) and
the 32-bit linux build, and nowhere else.

Re: [PATCH 0/3] convert submodule.c to not use the index compat macros

[Junio C Hamano]

Brandon Williams  writes:

> This series removes the remaining users of the index compatibility macros and
> ensures that future uses of the macros will result in compiler errors.

Nice.  Will queue.

Thanks.

Re: [PATCH 0/3] convert submodule.c to not use the index compat macros

[Junio C Hamano]

Stefan Beller  writes:

> ... would call out patch 2 to be a bugfix that could
> go independently, but the whole series is fine as-is with me.

Good eyes.  

I agree that it makes sense to treat 2/3 as a follow-up fix for an
already graduated topic, and make the other two depend on a result
of applying that first.  In practice it should not matter all that
much in this case, because the breakage is only in 'master' and is
not going to be merged down to 'maint', but it nevertheless was a
good point to raise.

Thanks for carefully reading the patches through.

Re: [PATCH 2/3] submodule: used correct index in is_staging_gitmodules_ok

[Eric Sunshine]

On Tue, Dec 12, 2017 at 2:53 PM, Brandon Williams  wrote:
> Commit 883e248b8 (fsmonitor: teach git to optionally utilize a file
> system monitor to speed up detecting new or changed files., 2017-09-22)
> introduced a call to 'ce_match_stat()' in 'is_staging_gitmodules_ok()'
> which implicitly relys on the the global 'the_index' instead of the

s/relys/relies/

> passed in 'struct index_state'.  Fix this by changing the call to
> 'ie_match_stat()' and using the passed in index_state struct.
>
> Signed-off-by: Brandon Williams 

Re: [PATCH 3/3] travis: run tests with GIT_TEST_SPLIT_INDEX

[Thomas Gummerer]

On 12/12, Junio C Hamano wrote:
> Lars Schneider  writes:
> 
> >> You're right, it's my first time using travis CI and I got confused
> >> about how the .travis.yml works, thanks for catching that.  Will
> >> re-phrase the commit message.
> >
> > Szeder is spot on. If you fix up the message, then this patch looks
> > perfect! Pragmatic (= very good!) idea to combine GITTEXT_POISON with 
> > GIT_TEST_SPLIT_INDEX :-)
> 
> I am failing to guess the real intent of the smiley here.
> 
> If split-index code is so easy to break, I do not think it is a good
> idea to combine it into the poison build.  In fact, the poison test
> is useless on a codebase where other/real breakages are expected to
> exist, because it is about seeing messages meant for non-humans are
> not passed to the _() mechanism by sloppy coding, and the way it
> does so is to corrupt all the messages that come through the _()
> mechanism.  If we do not even produce a message when a correct code
> _should_ produce one, poison test would catch nothing useful.

The breakages wen the split-index code fails tend to break things in
much more obvious manners than a wrong message, usually git ends up
dying if it gets broken.  Both of the bugs that were fixed here would
have been caught with the change in my patch.

But yeah I can see the argument that it doesn't give us a guarantee
that it catches all things the test suite could catch.

> I wonder if it makes more sense to update ci/run-tests.sh so that
> its final step is run twice with different settings, like so?

I kind of wanted to avoid that, because it ends up running the test
suite twice on travis for every test, which seems a bit overkill.  But
I don't exactly how worried we are about cycles on travis (I don't
check it very often personally, so I don't really know).  If we aren't
worried about cycles what you have below would certainly make sense.

>  ci/run-tests.sh | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/ci/run-tests.sh b/ci/run-tests.sh
> index f0c743de94..15a5f5a6cc 100755
> --- a/ci/run-tests.sh
> +++ b/ci/run-tests.sh
> @@ -8,3 +8,4 @@
>  mkdir -p $HOME/travis-cache
>  ln -s $HOME/travis-cache/.prove t/.prove
>  make --quiet test
> +GIT_TEST_SPLIT_INDEX=LetsTryIt make --quiet test

Re: [PATCH 0/3] convert submodule.c to not use the index compat macros

[Stefan Beller]

On Tue, Dec 12, 2017 at 11:53 AM, Brandon Williams  wrote:
> This series removes the remaining users of the index compatibility macros and
> ensures that future uses of the macros will result in compiler errors.

Thanks for converting the submodule code to avoid these old macros.
Specifically the submodule code will benefit once we have a repository
object available throughout the code base, so it is a great start.

I reviewed the series and would call out patch 2 to be a bugfix that could
go independently, but the whole series is fine as-is with me.

Thanks,
Stefan

Re: [RFC PATCH] t/helper: Move sources to t/helper-src; gitignore any files in t/helper

[Junio C Hamano]

Stefan Beller  writes:

> Compiled test helpers in t/helper are out of sync with the .gitignore
> files quite frequently. This can happen when new test helpers are added,
> but the explicit .gitignore file is not updated in the same commit, or
> when you forget to 'make clean' before checking out a different version
> of git, as the different version may have a different explicit list of
> test helpers to ignore.
>
> This can be fixed by using overly broad ignore patterns, such as ignoring
> the whole directory via '//t/helper/*' in .gitignore.

If we ignore everything but resurrect *.[ch] with negative exclude
rules, can we do the same without moving things around?

Re: [PATCH] Makefile: allow NO_DC_SHA1_SUBMODULE to be set in config.mak

[Junio C Hamano]

Junio C Hamano  writes:

> Makes sense.  The patch looks scary by appearing to move the
> includes far to the front of the Makefile, but it in fact is moving
> the NO_DC_SHA1_SUBMODULE block slightly down and it is a sensible
> and safe move.

A completely unrelated tangent.  This was a good change to try your
anchored diff on.  Viewing this change with

   $ git show --anchored='include config.mak'

looks a lot less scary than the way it is shown by default (which
matches what was posted on the list).

Good job.

[PATCH 3/3] submodule: convert get_next_submodule to not rely on the_index

[Brandon Williams]

Instead of implicitly relying on the global 'the_index', convert
'get_next_submodule()' to use the index of the repository stored in the
callback data 'struct submodule_parallel_fetch'.

Since this removes the last user of the index compatibility macros,
define 'NO_THE_INDEX_COMPATIBILITY_MACROS' to prevent future users of
these macros in submodule.c.

Signed-off-by: Brandon Williams 
---
 builtin/fetch.c |  4 +++-
 submodule.c | 23 +--
 submodule.h | 10 ++
 3 files changed, 22 insertions(+), 15 deletions(-)

diff --git a/builtin/fetch.c b/builtin/fetch.c
index e705237fa..e656746ab 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -3,6 +3,7 @@
  */
 #include "cache.h"
 #include "config.h"
+#include "repository.h"
 #include "refs.h"
 #include "commit.h"
 #include "builtin.h"
@@ -1397,7 +1398,8 @@ int cmd_fetch(int argc, const char **argv, const char 
*prefix)
struct argv_array options = ARGV_ARRAY_INIT;
 
add_options_to_argv();
-   result = fetch_populated_submodules(,
+   result = fetch_populated_submodules(the_repository,
+   ,
submodule_prefix,
recurse_submodules,
recurse_submodules_default,
diff --git a/submodule.c b/submodule.c
index a9670eaae..59372eada 100644
--- a/submodule.c
+++ b/submodule.c
@@ -1,3 +1,5 @@
+#define NO_THE_INDEX_COMPATIBILITY_MACROS
+
 #include "cache.h"
 #include "repository.h"
 #include "config.h"
@@ -1179,7 +1181,7 @@ int submodule_touches_in_range(struct object_id *excl_oid,
 struct submodule_parallel_fetch {
int count;
struct argv_array args;
-   const char *work_tree;
+   struct repository *r;
const char *prefix;
int command_line_option;
int default_option;
@@ -1200,7 +1202,7 @@ static int get_fetch_recurse_config(const struct 
submodule *submodule,
 
int fetch_recurse = submodule->fetch_recurse;
key = xstrfmt("submodule.%s.fetchRecurseSubmodules", 
submodule->name);
-   if (!repo_config_get_string_const(the_repository, key, )) 
{
+   if (!repo_config_get_string_const(spf->r, key, )) {
fetch_recurse = parse_fetch_recurse_submodules_arg(key, 
value);
}
free(key);
@@ -1219,11 +1221,11 @@ static int get_next_submodule(struct child_process *cp,
int ret = 0;
struct submodule_parallel_fetch *spf = data;
 
-   for (; spf->count < active_nr; spf->count++) {
+   for (; spf->count < spf->r->index->cache_nr; spf->count++) {
struct strbuf submodule_path = STRBUF_INIT;
struct strbuf submodule_git_dir = STRBUF_INIT;
struct strbuf submodule_prefix = STRBUF_INIT;
-   const struct cache_entry *ce = active_cache[spf->count];
+   const struct cache_entry *ce = spf->r->index->cache[spf->count];
const char *git_dir, *default_argv;
const struct submodule *submodule;
struct submodule default_submodule = SUBMODULE_INIT;
@@ -1231,7 +1233,7 @@ static int get_next_submodule(struct child_process *cp,
if (!S_ISGITLINK(ce->ce_mode))
continue;
 
-   submodule = submodule_from_path(_oid, ce->name);
+   submodule = submodule_from_cache(spf->r, _oid, ce->name);
if (!submodule) {
const char *name = default_name_or_path(ce->name);
if (name) {
@@ -1257,7 +1259,7 @@ static int get_next_submodule(struct child_process *cp,
continue;
}
 
-   strbuf_addf(_path, "%s/%s", spf->work_tree, ce->name);
+   strbuf_repo_worktree_path(_path, spf->r, "%s", 
ce->name);
strbuf_addf(_git_dir, "%s/.git", submodule_path.buf);
strbuf_addf(_prefix, "%s%s/", spf->prefix, ce->name);
git_dir = read_gitfile(submodule_git_dir.buf);
@@ -1310,7 +1312,8 @@ static int fetch_finish(int retvalue, struct strbuf *err,
return 0;
 }
 
-int fetch_populated_submodules(const struct argv_array *options,
+int fetch_populated_submodules(struct repository *r,
+  const struct argv_array *options,
   const char *prefix, int command_line_option,
   int default_option,
   int quiet, int max_parallel_jobs)
@@ -1318,16 +1321,16 @@ int fetch_populated_submodules(const struct argv_array 
*options,
int i;
struct submodule_parallel_fetch spf = SPF_INIT;
 
-   spf.work_tree = get_git_work_tree();
+   spf.r = r;
spf.command_line_option = command_line_option;

[PATCH 1/3] submodule: convert stage_updated_gitmodules to take a struct index_state

[Brandon Williams]

Signed-off-by: Brandon Williams 
---
 builtin/mv.c | 2 +-
 builtin/rm.c | 2 +-
 submodule.c  | 4 ++--
 submodule.h  | 2 +-
 4 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/builtin/mv.c b/builtin/mv.c
index ffdd5f01a..cf3684d90 100644
--- a/builtin/mv.c
+++ b/builtin/mv.c
@@ -291,7 +291,7 @@ int cmd_mv(int argc, const char **argv, const char *prefix)
}
 
if (gitmodules_modified)
-   stage_updated_gitmodules();
+   stage_updated_gitmodules(_index);
 
if (active_cache_changed &&
write_locked_index(_index, _file, COMMIT_LOCK))
diff --git a/builtin/rm.c b/builtin/rm.c
index d91451fea..4a2fcca27 100644
--- a/builtin/rm.c
+++ b/builtin/rm.c
@@ -382,7 +382,7 @@ int cmd_rm(int argc, const char **argv, const char *prefix)
}
strbuf_release();
if (gitmodules_modified)
-   stage_updated_gitmodules();
+   stage_updated_gitmodules(_index);
}
 
if (active_cache_changed) {
diff --git a/submodule.c b/submodule.c
index 95e6aff2b..7097be806 100644
--- a/submodule.c
+++ b/submodule.c
@@ -143,9 +143,9 @@ int remove_path_from_gitmodules(const char *path)
return 0;
 }
 
-void stage_updated_gitmodules(void)
+void stage_updated_gitmodules(struct index_state *istate)
 {
-   if (add_file_to_cache(GITMODULES_FILE, 0))
+   if (add_file_to_index(istate, GITMODULES_FILE, 0))
die(_("staging updated .gitmodules failed"));
 }
 
diff --git a/submodule.h b/submodule.h
index f0da0277a..cd984ecba 100644
--- a/submodule.h
+++ b/submodule.h
@@ -37,7 +37,7 @@ extern int is_gitmodules_unmerged(const struct index_state 
*istate);
 extern int is_staging_gitmodules_ok(const struct index_state *istate);
 extern int update_path_in_gitmodules(const char *oldpath, const char *newpath);
 extern int remove_path_from_gitmodules(const char *path);
-extern void stage_updated_gitmodules(void);
+extern void stage_updated_gitmodules(struct index_state *istate);
 extern void set_diffopt_flags_from_submodule_config(struct diff_options *,
const char *path);
 extern int git_default_submodule_config(const char *var, const char *value, 
void *cb);
-- 
2.15.1.504.g5279b80103-goog

[PATCH 0/3] convert submodule.c to not use the index compat macros

[Brandon Williams]

This series removes the remaining users of the index compatibility macros and
ensures that future uses of the macros will result in compiler errors.

Brandon Williams (3):
  submodule: convert stage_updated_gitmodules to take a struct
index_state
  submodule: used correct index in is_staging_gitmodules_ok
  submodule: convert get_next_submodule to not rely on the_index

 builtin/fetch.c |  4 +++-
 builtin/mv.c|  2 +-
 builtin/rm.c|  2 +-
 submodule.c | 32 ++--
 submodule.h | 14 --
 5 files changed, 31 insertions(+), 23 deletions(-)

-- 
2.15.1.504.g5279b80103-goog

[PATCH 2/3] submodule: used correct index in is_staging_gitmodules_ok

[Brandon Williams]

Commit 883e248b8 (fsmonitor: teach git to optionally utilize a file
system monitor to speed up detecting new or changed files., 2017-09-22)
introduced a call to 'ce_match_stat()' in 'is_staging_gitmodules_ok()'
which implicitly relys on the the global 'the_index' instead of the
passed in 'struct index_state'.  Fix this by changing the call to
'ie_match_stat()' and using the passed in index_state struct.

Signed-off-by: Brandon Williams 
---
 submodule.c | 5 +++--
 submodule.h | 2 +-
 2 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/submodule.c b/submodule.c
index 7097be806..a9670eaae 100644
--- a/submodule.c
+++ b/submodule.c
@@ -55,14 +55,15 @@ int is_gitmodules_unmerged(const struct index_state *istate)
  * future version when we learn to stage the changes we do ourselves without
  * staging any previous modifications.
  */
-int is_staging_gitmodules_ok(const struct index_state *istate)
+int is_staging_gitmodules_ok(struct index_state *istate)
 {
int pos = index_name_pos(istate, GITMODULES_FILE, 
strlen(GITMODULES_FILE));
 
if ((pos >= 0) && (pos < istate->cache_nr)) {
struct stat st;
if (lstat(GITMODULES_FILE, ) == 0 &&
-   ce_match_stat(istate->cache[pos], , 
CE_MATCH_IGNORE_FSMONITOR) & DATA_CHANGED)
+   ie_match_stat(istate, istate->cache[pos], ,
+ CE_MATCH_IGNORE_FSMONITOR) & DATA_CHANGED)
return 0;
}
 
diff --git a/submodule.h b/submodule.h
index cd984ecba..e2a5de3d8 100644
--- a/submodule.h
+++ b/submodule.h
@@ -34,7 +34,7 @@ struct submodule_update_strategy {
 #define SUBMODULE_UPDATE_STRATEGY_INIT {SM_UPDATE_UNSPECIFIED, NULL}
 
 extern int is_gitmodules_unmerged(const struct index_state *istate);
-extern int is_staging_gitmodules_ok(const struct index_state *istate);
+extern int is_staging_gitmodules_ok(struct index_state *istate);
 extern int update_path_in_gitmodules(const char *oldpath, const char *newpath);
 extern int remove_path_from_gitmodules(const char *path);
 extern void stage_updated_gitmodules(struct index_state *istate);
-- 
2.15.1.504.g5279b80103-goog

Re: [PATCH] Makefile: allow NO_DC_SHA1_SUBMODULE to be set in config.mak

[Junio C Hamano]

Ramsay Jones  writes:

> Signed-off-by: Ramsay Jones 
> ---
>
> Hi Junio,
>
> Could you please add (or squash) this on top of the 'ab/sha1dc-build'
> branch, so that I can build with NO_DC_SHA1_SUBMODULE=NoThanks in my
> config.mak.

Makes sense.  The patch looks scary by appearing to move the
includes far to the front of the Makefile, but it in fact is moving
the NO_DC_SHA1_SUBMODULE block slightly down and it is a sensible
and safe move.

I actually think that the block can go even further down, perhaps
close to the run of choices "what variant are we building?" we make
at around we have "ifdef NO_CURL".

Ævar?

> diff --git a/Makefile b/Makefile
> index 929b49b04..91bbb0ed8 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -1042,6 +1042,10 @@ EXTLIBS =
>  
>  GIT_USER_AGENT = git/$(GIT_VERSION)
>  
> +include config.mak.uname
> +-include config.mak.autogen
> +-include config.mak
> +
>  ifndef NO_DC_SHA1_SUBMODULE
>   ifndef DC_SHA1_EXTERNAL
>   ifneq ($(wildcard 
> sha1collisiondetection/lib/sha1.h),sha1collisiondetection/lib/sha1.h)
> @@ -1053,10 +1057,6 @@ whatever reason define NO_DC_SHA1_SUBMODULE=NoThanks)
>   endif
>  endif
>  
> -include config.mak.uname
> --include config.mak.autogen
> --include config.mak
> -
>  ifdef DEVELOPER
>  CFLAGS += $(DEVELOPER_CFLAGS)
>  endif

Re: [PATCH] doc: clarify usage of XDG_CONFIG_HOME config file

[Junio C Hamano]

Jacob Keller  writes:

>  --global::
> + For writing options: write to global user configuration file
> + rather than the repository `.git/config`.
>  +
> +For reading options: read only from global user configuration file
> +rather than from all available files.
>  +
>  See also <>.

OK.

> @@ -237,26 +235,30 @@ See also <>.
>  FILES
>  -
>  
> +If not set explicitly with `--file`, there are three locations where
>  'git config' will search for configuration options:
>  
> +System-wide configuration::
> + Located at `$(prefix)/etc/gitconfig`.
>  
> +User-specific configuration::
> + One and only one of the following files will be read

We said "will search for" upfront, but this talks about "will be
read", leaving the reader puzzled as to what should happen when
writing.  Perhaps "s/read/used/"?

> ++
> +- `~/.gitconfig`
> +- `$XDG_CONFIG_HOME/git/config`
> +- `$HOME/.config/git/config`
> ++
> +If `~/.gitconfig` exists, it will be used, and the other files will not be
> +read. Otherwise, if `$XDG_CONFIG_HOME` is set, then 
> `$XDG_CONFIG_HOME/git/config`
> +will be used, otherwise `$HOME/.config/git/config` will be used.

And then "and the other files will not be read" can be dropped from
the first sentence of this paragraph?

Yaroslav on the original thread mentioned that reading codepath
without --file or --global does not limit to one of the three, and
this section is about "If not set explicitly with `--file`", so we'd
need to make sure if the above is what happens in reality (or update
the proposed clarification to match the reality).

Thanks.

Re: Q: rational for $XDG_CONFIG_HOME/git/config to be "non global" or just a bug?

[Junio C Hamano]

Jacob Keller  writes:

>> I actually thought that the plan was "you either have this, or the
>> other one, never both at the same time" (and I think those who
>> pushed the XDG thing in to the system made us favor it over the
>> traditional one).  So as long as --global updates the one that
>> exists, and updates XDG one when both or neither do, I think we
>> should be OK.  And from that viewpoint, we definitely do not want
>> two kinds of --global to pretend as if we support use of both at the
>> same time.
>
> It appears that we actually prefer ~/.gitconfig rather than XDG_CONFIG_HOME..
>
> And at least based on current cursory testing on the command line, we
> do both read and write to the proper location, assuming that
> ~/.gitconfig is preferred over $XDG_CONFIG_HOME.

OK, so I misremembered the details but it seems that the behaviour
is consistent and there is no ambiguity?  

Am I reading you correctly?

Re: [PATCH] git-send-email: fix get_maintainer.pl regression

[Junio C Hamano]

Ævar Arnfjörð Bjarmason  writes:

> My "Makefile: replace perl/Makefile.PL with simple make rules" currently
> cooking in pu changes that so that:
>
>  * We always at runtime test for the system CPAN module.
>
>  * In the case of Error.pm we happen to ship a fallback, in the case of
>Mail::Address etc. we don't and have fallback code, but we could also
>just ship a copy and remove the fallback code.
>
> This makes more sense, we always "dynamically link" as it were, we'll
> just change the target to (a presumably newer) system module in the case
> of Error.pm if it's found on the system, otherwise use our fallback.

"When to fallback" aside, I think the above makes sense for the
send-email simply because we would be replacing "our own" fallback
we may need to maintain forever with something with an upstream that
we do not have to worry too much about.

A tangent; I thought I heard that use of Error.pm is strongly
discouraged several years ago---am I mistaken, or if I am not,
perhaps we should start looking into updating the users?

Thanks.

Re: [PATCH v1] convert: add support for 'encoding' attribute

[Junio C Hamano]

Lars Schneider  writes:

> Our favorite is "treat-encoding-as". Do you consider this better
> or worse than "checkout-encoding"?

I am afraid that "treat as" is not sufficiently specific and would
invite a misinterpretation, e.g. "You record the bytes I throw at
you as-is in the object store, but treat them appropriately as
contents that are encoded in cp1252 when presenting".

what is missing is at what stage in the overall user experience does
that "treating" happens.  That causes such a misinterpretation.

So from that point of view, "checkout-" or" working-tree-" would be
a better phrase to accompany "encoding" to clarify what this attr is
for than "treat-as".

Having said all that, this "feature" would need a moderate amount of
clear description in the documentation, and between the perfect and
a suboptimal name, the amount of explanation required would not be
all that different, I suspect.  We need to say that those who wish
to use this attribute are buying into recording their contents in
UTF-8 and when the contents are externalized to the working tree,
they are converted to the encoding the value of this attribute
specifies and vice versa.

Re: [PATCH 3/3] travis: run tests with GIT_TEST_SPLIT_INDEX

[Junio C Hamano]

Lars Schneider  writes:

>> You're right, it's my first time using travis CI and I got confused
>> about how the .travis.yml works, thanks for catching that.  Will
>> re-phrase the commit message.
>
> Szeder is spot on. If you fix up the message, then this patch looks
> perfect! Pragmatic (= very good!) idea to combine GITTEXT_POISON with 
> GIT_TEST_SPLIT_INDEX :-)

I am failing to guess the real intent of the smiley here.

If split-index code is so easy to break, I do not think it is a good
idea to combine it into the poison build.  In fact, the poison test
is useless on a codebase where other/real breakages are expected to
exist, because it is about seeing messages meant for non-humans are
not passed to the _() mechanism by sloppy coding, and the way it
does so is to corrupt all the messages that come through the _()
mechanism.  If we do not even produce a message when a correct code
_should_ produce one, poison test would catch nothing useful.

I wonder if it makes more sense to update ci/run-tests.sh so that
its final step is run twice with different settings, like so?

 ci/run-tests.sh | 1 +
 1 file changed, 1 insertion(+)

diff --git a/ci/run-tests.sh b/ci/run-tests.sh
index f0c743de94..15a5f5a6cc 100755
--- a/ci/run-tests.sh
+++ b/ci/run-tests.sh
@@ -8,3 +8,4 @@
 mkdir -p $HOME/travis-cache
 ln -s $HOME/travis-cache/.prove t/.prove
 make --quiet test
+GIT_TEST_SPLIT_INDEX=LetsTryIt make --quiet test

Re: [PATCH v2 2/2] t: add tests for pull --verify-signatures

[Junio C Hamano]

Hans Jerry Illikainen  writes:

> +test_expect_success GPG 'pull unsigned commit with --verify-signatures' '
> + test_must_fail git pull --ff-only --verify-signatures unsigned 
> 2>pullerror &&
> + test_i18ngrep "does not have a GPG signature" pullerror
> +'

Note that this is without "when-finished"; if 'git pull' got broken
and does not fail as expected, the next test will start from a state
that it is not expecting.  Same for the ones that run 'git pull'
under test_must_fail.

Interestingly, the tests that do expect 'git pull' to succeed
protect themselves with "when-finished" mechanism correctly [*1*],
like so:

> +test_expect_success GPG 'pull signed commit with --verify-signatures' '
> + test_when_finished "git checkout initial" &&
> + git pull --verify-signatures signed >pulloutput &&
> + test_i18ngrep "has a good GPG signature" pulloutput
> +'
> +

Other than that, looked nicely done.  Thanks.


[Footnote]

*1* I am guessing that the branches that are being pulled in tests
are designed in such a way to never produce merge conflicts, and
failures are possible only due to signature verification.  If
that were not the case, "when-finished" would want to do a hard
reset before checking out the initial to go back to a known
state.

[RFC PATCH] t/helper: Move sources to t/helper-src; gitignore any files in t/helper

[Stefan Beller]

Compiled test helpers in t/helper are out of sync with the .gitignore
files quite frequently. This can happen when new test helpers are added,
but the explicit .gitignore file is not updated in the same commit, or
when you forget to 'make clean' before checking out a different version
of git, as the different version may have a different explicit list of
test helpers to ignore.

This can be fixed by using overly broad ignore patterns, such as ignoring
the whole directory via '//t/helper/*' in .gitignore.

However we do not want to have an overlap of checked source files and
ignored files, hence we'll move the the source files currently residing
in t/helper to t/helper-src. To accommodate that we'll need to update
the Makefile as well to look at a different place for the source files.
(This patch takes the hacky approach in symlinking the sources back into
the t/helper, which we'd want to avoid long term)

Signed-off-by: Stefan Beller 
---
 Makefile   |  4 +++
 t/{helper => helper-src}/.gitignore|  0
 t/{helper => helper-src}/test-chmtime.c|  0
 t/{helper => helper-src}/test-config.c |  0
 t/{helper => helper-src}/test-ctype.c  |  0
 t/{helper => helper-src}/test-date.c   |  0
 t/{helper => helper-src}/test-delta.c  |  0
 t/{helper => helper-src}/test-dump-cache-tree.c|  0
 t/{helper => helper-src}/test-dump-split-index.c   |  0
 .../test-dump-untracked-cache.c|  0
 t/{helper => helper-src}/test-fake-ssh.c   |  0
 t/{helper => helper-src}/test-genrandom.c  |  0
 t/{helper => helper-src}/test-hashmap.c|  0
 t/{helper => helper-src}/test-index-version.c  |  0
 .../test-lazy-init-name-hash.c |  0
 t/{helper => helper-src}/test-line-buffer.c|  0
 t/{helper => helper-src}/test-match-trees.c|  0
 t/{helper => helper-src}/test-mergesort.c  |  0
 t/{helper => helper-src}/test-mktemp.c |  0
 t/{helper => helper-src}/test-online-cpus.c|  0
 t/{helper => helper-src}/test-parse-options.c  |  0
 t/{helper => helper-src}/test-path-utils.c |  0
 t/{helper => helper-src}/test-prio-queue.c |  0
 t/{helper => helper-src}/test-read-cache.c |  0
 t/{helper => helper-src}/test-ref-store.c  |  0
 t/{helper => helper-src}/test-regex.c  |  0
 t/{helper => helper-src}/test-revision-walking.c   |  0
 t/{helper => helper-src}/test-run-command.c|  0
 t/{helper => helper-src}/test-scrap-cache-tree.c   |  0
 t/{helper => helper-src}/test-sha1-array.c |  0
 t/{helper => helper-src}/test-sha1.c   |  0
 t/{helper => helper-src}/test-sha1.sh  |  0
 t/{helper => helper-src}/test-sigchain.c   |  0
 t/{helper => helper-src}/test-strcmp-offset.c  |  0
 t/{helper => helper-src}/test-string-list.c|  0
 t/{helper => helper-src}/test-submodule-config.c   |  0
 t/{helper => helper-src}/test-subprocess.c |  0
 t/{helper => helper-src}/test-svn-fe.c |  0
 .../test-urlmatch-normalization.c  |  0
 t/{helper => helper-src}/test-wildmatch.c  |  0
 t/{helper => helper-src}/test-write-cache.c|  0
 t/helper/.gitignore| 39 +-
 42 files changed, 5 insertions(+), 38 deletions(-)
 copy t/{helper => helper-src}/.gitignore (100%)
 rename t/{helper => helper-src}/test-chmtime.c (100%)
 rename t/{helper => helper-src}/test-config.c (100%)
 rename t/{helper => helper-src}/test-ctype.c (100%)
 rename t/{helper => helper-src}/test-date.c (100%)
 rename t/{helper => helper-src}/test-delta.c (100%)
 rename t/{helper => helper-src}/test-dump-cache-tree.c (100%)
 rename t/{helper => helper-src}/test-dump-split-index.c (100%)
 rename t/{helper => helper-src}/test-dump-untracked-cache.c (100%)
 rename t/{helper => helper-src}/test-fake-ssh.c (100%)
 rename t/{helper => helper-src}/test-genrandom.c (100%)
 rename t/{helper => helper-src}/test-hashmap.c (100%)
 rename t/{helper => helper-src}/test-index-version.c (100%)
 rename t/{helper => helper-src}/test-lazy-init-name-hash.c (100%)
 rename t/{helper => helper-src}/test-line-buffer.c (100%)
 rename t/{helper => helper-src}/test-match-trees.c (100%)
 rename t/{helper => helper-src}/test-mergesort.c (100%)
 rename t/{helper => helper-src}/test-mktemp.c (100%)
 rename t/{helper => helper-src}/test-online-cpus.c (100%)
 rename t/{helper => helper-src}/test-parse-options.c (100%)
 rename t/{helper => helper-src}/test-path-utils.c (100%)
 rename t/{helper => helper-src}/test-prio-queue.c (100%)
 rename t/{helper => helper-src}/test-read-cache.c (100%)
 rename t/{helper => helper-src}/test-ref-store.c (100%)
 rename t/{helper => helper-src}/test-regex.c (100%)
 rename t/{helper => helper-src}/test-revision-walking.c (100%)
 rename t/{helper => 

Re: [PATCH 1/3] merge: add config option for verifySignatures

[Junio C Hamano]

Kevin Daudt  writes:

> Hello Hans Jerry,
>
> Thank you for your contribution. I have soem remarks below.
> ...

Thanks for a detailed review.  I agree with all the things you
pointed out, and I see they are reflected in v2 of the series.

Very much appreciated; thanks, both.

Re: [PATCH 1/4] travis-ci: use 'set -x' in 'ci/*' scripts for extra tracing output

[SZEDER Gábor]

On Tue, Dec 12, 2017 at 7:00 PM, Lars Schneider
 wrote:
>
>> On 12 Dec 2017, at 00:34, SZEDER Gábor  wrote:
>>
>> While the build logic was embedded in our '.travis.yml', Travis CI
>> used to produce a nice trace log including all commands executed in
>> those embedded scriptlets.  Since 657343a60 (travis-ci: move Travis CI
>> code into dedicated scripts, 2017-09-10), however, we only see the
>> name of the dedicated scripts, but not what those scripts are actually
>> doing, resulting in a less useful trace log.  A patch later in this
>> series will move setting environment variables from '.travis.yml' to
>> the 'ci/*' scripts, so not even those will be included in the trace
>> log.
>>
>> Use 'set -x' in 'ci/lib-travisci.sh', which is sourced in most other
>> 'ci/*' scripts, so we get trace log about the commands executed in all
>> of those scripts.
>
> I kind of did that intentionally to avoid clutter in the logs.
> However, I also agree with your reasoning. Therefore, the change
> looks good to me!

Great, 'cause I'm starting to have second thoughts about this change :)

It sure helped a lot while I worked on this patch series and a couple of
other Travis CI related patches (will submit them later)...  OTOH it
definitely creates clutter in the trace log.  The worst offender might
be 'ci/print-test-failures.sh', which iterates over all
't/test-results/*.exit' files to find which tests failed and to show
their output, and 'set -x' makes every iteration visible.  And we have
about 800 tests, which means 800 iterations.  Yuck.

Perhaps we should use other means to show what's going on instead, e.g.
use more 'echo's and '--verbose' options, or just avoid using '--quiet'.
And if some brave souls really want to tweak '.travis.yml' or the 'ci/*'
scripts, then they can set 'set -x' for themselves during development...
as the patch below shows it's easy enough, just a single character :)


Gábor


>>
>> Signed-off-by: SZEDER Gábor 
>> ---
>> ci/lib-travisci.sh | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/ci/lib-travisci.sh b/ci/lib-travisci.sh
>> index ac05f1f46..a0c8ae03f 100755
>> --- a/ci/lib-travisci.sh
>> +++ b/ci/lib-travisci.sh
>> @@ -23,7 +23,7 @@ skip_branch_tip_with_tag () {
>>
>> # Set 'exit on error' for all CI scripts to let the caller know that
>> # something went wrong
>> -set -e
>> +set -ex
>>
>> skip_branch_tip_with_tag
>>
>> --
>> 2.15.1.421.gc469ca1de
>>
>

Re: [PATCH] diffcore: add a filter to find a specific blob

[Junio C Hamano]

Stefan Beller  writes:

> Sometimes users are given a hash of an object and they want to
> identify it further (ex.: Use verify-pack to find the largest blobs,
> but what are these? or [1])
> ...
>  Documentation/diff-options.txt |  6 
>  Makefile   |  1 +
>  builtin/log.c  |  2 +-
>  diff.c | 20 -
>  diff.h |  3 ++
>  diffcore-objfind.c | 42 ++
>  diffcore.h |  1 +
>  revision.c |  5 +++-
>  t/t4064-diff-oidfind.sh| 68 
> ++
>  9 files changed, 145 insertions(+), 3 deletions(-)
>  create mode 100644 diffcore-objfind.c
>  create mode 100755 t/t4064-diff-oidfind.sh

Thanks.  Will drop an asdf and queue.  

I think this is ready for 'next'; I'll wait for a day or two just to
give us a final chance to spot and save us from minor embarrassments,
but I expect anything we'd spot from here on would be so minor that
we can go incremental.

Re: [PATCH] git-send-email: fix get_maintainer.pl regression

[Ævar Arnfjörð Bjarmason]

On Tue, Dec 12 2017, Alex Bennée jotted:

> Thomas Adam  writes:
>
>> Hi,
>>
>> On Mon, Dec 11, 2017 at 08:46:46PM +0100, Ævar Arnfjörð Bjarmason wrote:
>>> I.e. we'd just ship a copy of Email::Valid and Mail::Address in
>>> perl/Git/FromCPAN/, use a wrapper to load them, and then we wouldn't
>>> need to if/else this at the code level, just always use the module,
>>> and it would work even on core perl.
>>
>> I disagree with the premise of this, Ævar.  As soon as you go down this 
>> route,
>> it increases maintenance to ensure we keep up to date with what's on CPAN for
>> a tiny edge-case which I don't believe exists.
>>
>> You may as well just use App::FatPacker.
>>
>> We're talking about package maintenance here -- and as I said before, there's
>> plenty of it around.  For those distributions which ship Git (and hence also
>> package git-send-email), the dependencies are already there, too.  I just
>> cannot see this being a problem in relying on non-core perl modules.  Every
>> perl program does this, and they don't go down this route of having copies of
>> various CPAN modules just in case.  So why should we?  We're not a special
>> snowflake.
>
> I less bothered my the potentially shipping a git specific copy than
> ensuring the packagers pick up the dependency when they do their builds.
> Do we already have a mechanism for testing for non-core perl modules
> during the "configure" phase of git?

Current git.git master does two things:

 * For Error.pm we test at build time. See `git grep Error --
   'perl/Make*'`. If you don't have Error.pm when you build we'll ship
   an old copy of it, and use that forever even if it's installed from
   CPAN afterwards.

 * For Mail::Address, Net::Domain etc. we don't ship the CPAN module,
   but some fallback code. We test at runtime, see `git grep
   eval.*require`. If you install the package from CPAN we'll start
   using it at your next invocation.

My "Makefile: replace perl/Makefile.PL with simple make rules" currently
cooking in pu changes that so that:

 * We always at runtime test for the system CPAN module.

 * In the case of Error.pm we happen to ship a fallback, in the case of
   Mail::Address etc. we don't and have fallback code, but we could also
   just ship a copy and remove the fallback code.

This makes more sense, we always "dynamically link" as it were, we'll
just change the target to (a presumably newer) system module in the case
of Error.pm if it's found on the system, otherwise use our fallback.

Re: [PATCH 1/4] travis-ci: use 'set -x' in 'ci/*' scripts for extra tracing output

[Lars Schneider]

> On 12 Dec 2017, at 00:34, SZEDER Gábor  wrote:
> 
> While the build logic was embedded in our '.travis.yml', Travis CI
> used to produce a nice trace log including all commands executed in
> those embedded scriptlets.  Since 657343a60 (travis-ci: move Travis CI
> code into dedicated scripts, 2017-09-10), however, we only see the
> name of the dedicated scripts, but not what those scripts are actually
> doing, resulting in a less useful trace log.  A patch later in this
> series will move setting environment variables from '.travis.yml' to
> the 'ci/*' scripts, so not even those will be included in the trace
> log.
> 
> Use 'set -x' in 'ci/lib-travisci.sh', which is sourced in most other
> 'ci/*' scripts, so we get trace log about the commands executed in all
> of those scripts.

I kind of did that intentionally to avoid clutter in the logs.
However, I also agree with your reasoning. Therefore, the change
looks good to me!

Thanks,
Lars

> 
> Signed-off-by: SZEDER Gábor 
> ---
> ci/lib-travisci.sh | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/ci/lib-travisci.sh b/ci/lib-travisci.sh
> index ac05f1f46..a0c8ae03f 100755
> --- a/ci/lib-travisci.sh
> +++ b/ci/lib-travisci.sh
> @@ -23,7 +23,7 @@ skip_branch_tip_with_tag () {
> 
> # Set 'exit on error' for all CI scripts to let the caller know that
> # something went wrong
> -set -e
> +set -ex
> 
> skip_branch_tip_with_tag
> 
> -- 
> 2.15.1.421.gc469ca1de
> 

Re: Re: Re: bug deleting "unmerged" branch (2.12.3)

[Philip Oakley]

From: "Ulrich Windl" 

Hi!

Sorry for the late response:
On a somewhat not-up-to date manual:

  -d, --delete
  Delete a branch. The branch must be fully merged in its upstream
  branch, or in HEAD if no upstream was set with --track or
  --set-upstream.


Maybe the topic of multiple branches pointing to the same commit could be 
mentioned (regarding the status of each such branch being considered to be 
merged or not). Also "fully merged" could be made a bit more precise, 
maybe.


Maybe gitglossary could have definitions for "merged" and "fully merged" 
with manual pages referring to it.


Thanks, I'll add your note to my list of clarifications.

Philip



Regards,
Ulrich


"Philip Oakley"  schrieb am 08.12.2017 um 21:26 
in

Nachricht <582105F8768F4DA6AF4EC82888F0BFBE@PhilipOakley>:

From: "Ulrich Windl" 

Hi Philip!

I'm unsure what you are asking for...

Ulrich


Hi Ulrich,

I was doing a retrospective follow up (of the second kind [1]).

In your initial email
https://public-inbox.org/git/5a1d70fd02a100029...@gwsmtp1.uni-regensburg.d
e/
you said

"I wanted to delete the temporary branch (which is of no use now), I got 
a

message that the branch is unmerged.
I think if more than one branches are pointing to the same commit, one
should be allowed to delete all but the last one without warning."

My retrospectives question was to find what what part of the 
documentation
could be improved to assist fellow coders and Git users in gaining a 
better

understanding here. I think it's an easy mistake [2] to make and that we
should try to make the man pages more assistive.

I suspect that the description for the `git branch -d` needs a few more
words to clarify the 'merged/unmerged' issue for those who recieve the
warning message. Or maybe the git-glossary, etc. I tend to believe that 
most
users will read some of the man pages, and would continue to do so if 
they

are useful.

I'd welcome any feedback or suggestions you could provide.
--
Philip


>>> "Philip Oakley"  04.12.17 0.30 Uhr >>>
From: "Junio C Hamano" 
> "Philip Oakley"  writes:
>
>> I think it was that currently you are on M, and neither A nor B are
>> ancestors (i.e. merged) of M.
>>
>> As Junio said:- "branch -d" protects branches that are yet to be
>> merged to the **current branch**.
>
> Actually, I think people loosened this over time and removal of
> branch X is not rejected even if the range HEAD..X is not empty, as
> long as X is marked to integrate with/build on something else with
> branch.X.{remote,merge} and the range X@{upstream}..X is empty.
>
> So the stress of "current branch" above you added is a bit of a
> white lie.

Ah, thanks. [I haven't had chance to check the code]

The man page does say:
.-d
.Delete a branch. The branch must be fully merged in its upstream
.branch, or in HEAD if no upstream was set with --track
.or --set-upstream.

It's whether or not Ulrich had joined the two aspects together, and if 
the

doc was sufficient to help recognise the 'unmerged' issue. Ulrich?
--
Philip




[1] Retrospective Second Directive, section 3.4.2 of (15th Ed) Agile
Processes in software engineering and extreme programming. ISBN 
1628251042

(for the perspective of the retrospective..)
[2] 'mistake' colloquial part of the error categories of slips lapses and
mistakes : Human Error, by Reason (James, prof) ISBN 0521314194 
(worthwhile)

Re: What's cooking in git.git (Dec 2017, #02; Thu, 7)

[Philip Oakley]

From: "Christian Couder" 

On Thu, Dec 7, 2017 at 7:04 PM, Junio C Hamano  wrote:


* jh/object-filtering (2017-12-05) 9 commits
  (merged to 'next' on 2017-12-05 at 3a56b51085)
 + rev-list: support --no-filter argument
 + list-objects-filter-options: support --no-filter
 + list-objects-filter-options: fix 'keword' typo in comment
  (merged to 'next' on 2017-11-27 at e5008c3b28)
 + pack-objects: add list-objects filtering
 + rev-list: add list-objects filtering support
 + list-objects: filter objects in traverse_commit_list
 + oidset: add iterator methods to oidset
 + oidmap: add oidmap iterator methods
 + dir: allow exclusions from blob in addition to file
 (this branch is used by jh/fsck-promisors and jh/partial-clone.)

 In preparation for implementing narrow/partial clone, the object
 walking machinery has been taught a way to tell it to "filter" some
 objects from enumeration.


* jh/fsck-promisors (2017-12-05) 12 commits
 - gc: do not repack promisor packfiles
 - rev-list: support termination at promisor objects
 - fixup: sha1_file: add TODO
 - fixup: sha1_file: convert gotos to break/continue
 - sha1_file: support lazily fetching missing objects
 - introduce fetch-object: fetch one promisor object
 - index-pack: refactor writing of .keep files
 - fsck: support promisor objects as CLI argument
 - fsck: support referenced promisor objects
 - fsck: support refs pointing to promisor objects
 - fsck: introduce partialclone extension
 - extension.partialclone: introduce partial clone extension
 (this branch is used by jh/partial-clone; uses jh/object-filtering.)

 In preparation for implementing narrow/partial clone, the machinery
 for checking object connectivity used by gc and fsck has been
 taught that a missing object is OK when it is referenced by a
 packfile specially marked as coming from trusted repository that
 promises to make them available on-demand and lazily.


I am currently working on integrating this series with my external odb
series 
(https://public-inbox.org/git/20170916080731.13925-1-chrisc...@tuxfamily.org/).


I too had seen that, as currently configured, the 'partialClone' could be 
seen as a method for using the remote as if it were an object database (odb) 
that was part of an 'always on-line' capability. However I'm cautious about 
locking out the original DVCS capability of being off-line relative to some, 
or all, remotes and still needing to work in 'airplane mode'.


It should be OK for the local narrowClone (my term) to be totally off-line 
for a while and still be able to work when back on line with other suitable 
remotes, even after the original remote has gone.




Instead of using an "extension.partialclone" config variable, an odb
will be configured like using an "odb..promisorRemote" (the
name might still change) config variable. Other odbs could still be
configured using "odb..scriptCommand" and
"odb..subprocessCommand".


The future work Jeff had indicated, IIRC, should be able to cope with 
multiple promisor remotes, which it's to be hope this could handle. I'm not 
sure how the odb code would handle a partial failure where a partition of 
the odb stops being available.




The current work is still very much WIP and some tests fail, but you
can take a look there:

https://github.com/chriscool/git/tree/gl-promisor-external-odb440

--
Philip 

Re: [PATCH] git-send-email: fix get_maintainer.pl regression

[Alex Bennée]

Thomas Adam  writes:

> Hi,
>
> On Mon, Dec 11, 2017 at 08:46:46PM +0100, Ævar Arnfjörð Bjarmason wrote:
>> I.e. we'd just ship a copy of Email::Valid and Mail::Address in
>> perl/Git/FromCPAN/, use a wrapper to load them, and then we wouldn't
>> need to if/else this at the code level, just always use the module,
>> and it would work even on core perl.
>
> I disagree with the premise of this, Ævar.  As soon as you go down this route,
> it increases maintenance to ensure we keep up to date with what's on CPAN for
> a tiny edge-case which I don't believe exists.
>
> You may as well just use App::FatPacker.
>
> We're talking about package maintenance here -- and as I said before, there's
> plenty of it around.  For those distributions which ship Git (and hence also
> package git-send-email), the dependencies are already there, too.  I just
> cannot see this being a problem in relying on non-core perl modules.  Every
> perl program does this, and they don't go down this route of having copies of
> various CPAN modules just in case.  So why should we?  We're not a special
> snowflake.

I less bothered my the potentially shipping a git specific copy than
ensuring the packagers pick up the dependency when they do their builds.
Do we already have a mechanism for testing for non-core perl modules
during the "configure" phase of git?

--
Alex Bennée

Need to add test artifacts to .gitignore

[Dan Jacques]

FYI, I've noticed when building from "pu" that neither the
"t/helper/test-print-values" or "t/helper/test-print-larger-than-ssize"
testing artifacts added to Makefile in this patch series are not added to
"t/helper/.gitignore" like other helpers, resulting in the testing artifact
being recognized as an untracked file.

HI THERE

[MS.ELLA GOLAN]

I am Ms.Ella Golan, I am the Executive Vice President Banking Division with 
FIRST INTERNATIONAL BANK OF ISRAEL LTD (FIBI). I am getting in touch with you 
regarding an extremely important and urgent matter. If you would oblige me the 
opportunity, I shall provide you with details upon your response.

Faithfully,
Ms.Ella Golan

Re: [PATCH 3/3] travis: run tests with GIT_TEST_SPLIT_INDEX

[Lars Schneider]

> On 11 Dec 2017, at 22:42, Thomas Gummerer  wrote:
> 
> On 12/11, SZEDER Gábor wrote:
>>> Make sure that split index doesn't get broken, by running it on travis
>>> CI.
>>> 
>>> Run the test suite with split index enabled in linux 64 bit mode, and
>>> leave split index turned off in 32-bit mode.
>> 
>> This doesn't accurately describe what the patch does.
>> Travis CI runs three 64 bit Linux build jobs for us: one compiled with
>> Clang, one with GCC, and one with GETTEXT_POISON enabled.  This patch
>> enables split index only in the latter build job, but not in the Clang
>> and GCC build jobs.
> 
> You're right, it's my first time using travis CI and I got confused
> about how the .travis.yml works, thanks for catching that.  Will
> re-phrase the commit message.

Szeder is spot on. If you fix up the message, then this patch looks
perfect! Pragmatic (= very good!) idea to combine GITTEXT_POISON with 
GIT_TEST_SPLIT_INDEX :-)

Thanks,
Lars


> 
>>> The laternative would be
>>> to add an extra target in the matrix, enabling split index mode, but
>>> that would only use additional cycles on travis and would not bring that
>>> much benefit, as we are still running the test suite in the "vanilla"
>>> version in the 32-bit mode.
>>> 
>>> Signed-off-by: Thomas Gummerer 
>>> ---
>>> .travis.yml | 2 +-
>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>> 
>>> diff --git a/.travis.yml b/.travis.yml
>>> index 281f101f31..c83c766dee 100644
>>> --- a/.travis.yml
>>> +++ b/.travis.yml
>>> @@ -39,7 +39,7 @@ env:
>>> 
>>> matrix:
>>>   include:
>>> -- env: GETTEXT_POISON=YesPlease
>>> +- env: GETTEXT_POISON=YesPlease GIT_TEST_SPLIT_INDEX=YesPlease
>>>   os: linux
>>>   compiler:
>>>   addons:
>>> -- 
>>> 2.15.1.504.g5279b80103

Re: [PATCH] doc: clarify usage of XDG_CONFIG_HOME config file

[Todd Zullinger]

Hi Jacob,

Jacob Keller wrote:
> The documentation for git config and how it reads the user specific
> configuration file is misleading. In some places it implies that
> $XDG_CONFIG_HOME/git/config will always be read. In others, it implies
> that only one of ~/.gitconfig and $XDG_CONFIG_HOME/git/config will be
> read.
> 
> Improve the documentation explaining how the various configuration files
> are read, and combined.
> 
> Instead of referencing each file individually, reference each type of
> location git will check. When discussing the user configuration, explain
> how we switch between one of three choices. Ensure to note that only one
> of the three choices is used.

Perhaps it would read a little easier as "Make it clear ..."
rather than "Ensure to note that ..." ?

> +Note that git will only ever use one of these files as the global user
> +configuration file at once. Additionally if you sometimes use an older 
> version
> +of git, it is best to only rely on `~/.gitconfig` as support for the others 
> was
> +added fairly recently.

Is it really accurate to say these were added fairly
recently?  It looks like XDG_CONFIG_HOME was added in
21cf322791 ("config: read (but not write) from
$XDG_CONFIG_HOME/git/config file", 2012-06-22) and
0e8593dc5b ("config: write to $XDG_CONFIG_HOME/git/config
file when appropriate", 2012-06-22) which are in 1.7.12.

Would it be better to say something like "if you sometimes
use a version of git prior to 1.7.12" here?

Or maybe we can drop "Additionally ..." altogether now?
Someone using a 5 year old git version sometimes will
hopefully know to check the documentation for that older
version.

-- 
Todd
~~
Now don't say you can't swear off drinking; it's easy. I've done it a
thousand times.
-- W.C. Fields

I wait for your prompt response.

[SAM AZADA]

Good day,

I am Mr. Sam Azada from Burkina Faso  a Minister confide on me to look
for foreign partner who will assist him to invest the sum of  Fifty
Million  Dollars  ($50,000,000) in your country.

He has investment interest in mining, exotic properties for commercial
resident, development properties, hotels and any other viable
investment opportunities in your country based on your recommendation
will be highly welcomed.

Hence your co -operation is highly needed to actualize this investment project

I wait for your prompt response.

Sincerely yours

Mr Sam Azada.

Re: Q: rational for $XDG_CONFIG_HOME/git/config to be "non global" or just a bug?

[Yaroslav Halchenko]

On Mon, 11 Dec 2017, Junio C Hamano wrote:

> Jonathan Nieder  writes:

> > I think the documentation

> > ~/.gitconfig
> > User-specific configuration file. Also called "global"
> > configuration file.

> > should be clarified --- e.g. it could say

> > $XDG_CONFIG_HOME/git/config
> > ~/.gitconfig
> > User-specific configuration files. Because options in
> > these files are not specific to any repository, thes
> > are sometimes called global configuration files.

> Yeah, I think that makes sense.

> > As for "git config --global", I think the best thing would be to split
> > it into two options: something like "git config --user" and "git
> > config --xdg-user".  That way, it is unambiguous which configuration
> > file the user intends to inspect or modify.  When a user calls "git
> > config --global" and both files exist, it could warn that the command
> > is ambiguous.

> > Thoughts?

> I actually thought that the plan was "you either have this, or the
> other one, never both at the same time" (and I think those who
> pushed the XDG thing in to the system made us favor it over the
> traditional one).  So as long as --global updates the one that
> exists, and updates XDG one when both or neither do, I think we
> should be OK.  And from that viewpoint, we definitely do not want
> two kinds of --global to pretend as if we support use of both at the
> same time.

note that atm $XDG_CONFIG_HOME/git/config is read as --global iff
~/.gitconfig is absent and read always without --global.  So it is
flipping between "global" and "some kind of non-global but user-specific
configuration file" (so sounds like  a global to me ;) )

-- 
Yaroslav O. Halchenko
Center for Open Neuroscience http://centerforopenneuroscience.org
Dartmouth College, 419 Moore Hall, Hinman Box 6207, Hanover, NH 03755
Phone: +1 (603) 646-9834   Fax: +1 (603) 646-1419
WWW:   http://www.linkedin.com/in/yarik

git svn dcommit error: Cannot accept non-LF line endings in 'svn:log' property

[Bennett, Brian]

Environment:

Desktop: Windows 7 Enterprise 64-bit
svn client (if applicable): 1.8.8 from Apache
git (https://git-for-windows.github.io/): git version 2.10.1.windows.1
GitTfs (https://github.com/git-tfs/git-tfs): git-tfs version 0.27.0.0 (TFS 
client library 14.0.0.0 (MS)) (32-bit)
Team Foundation Server: 2010
Visual Studio installation: 2010 and 2015

All processing is being done on my desktop described above. My goal is to 
migrate Team Foundation Server source into git and then from git into a local 
SVN repository. The specific source from Team Foundation Server has only 2 
changesets made against it and neither have a commit message associated with 
them.

Steps I'm taking:

1. Open a Windows (cmd.exe) command shell and put git-tfs into Path:
Set Path=C:\Users\brbennett\Downloads\GitTfs-0.27.0;%Path%

2. Create empty folder C:\TEMP\gitclone\Project_Elevation_Request and make this 
the working folder.

3. git-tfs clone -d "http://tfs:8080/tfs/collection; 
$/Folder1/Production/Elevation_Request
I can see the only 2 TFS changesets being cloned into the local git repository.
C423 = 6dfefb6160b53da7f580f24f2ce41af04f508b8a
C424 = e8d6573b5a6e29db78fd420f843ca7ad7480eda2

4. Move working folder to 
C:\TEMP\gitclone\Project_Elevation_Request\Elevation_Request

5. Create empty folder C:\TEMP\SVN\repos and create empty SVN repository:
svnadmin --compatible-version 1.8 create C:\TEMP\SVN\repos\Elevation_Request

6. Start up local SVN server in a different shell:
svnserve -d -r C:\TEMP\SVN\repos

7. Create trunk in local 
svn mkdir --parents svn:///C:/TEMP/SVN/repos/Elevation_Request/trunk -m 
"Importing git repo"
Committed revision 1.

8. git svn init svn:///C:/TEMP/SVN/repos/%PROJNAME% -s

9. git svn fetch
R1 for the trunk created earlier is retrieved
r1 = 5efc0da5f5af4cd62fde660a4402e3a751c2b003 (refs/remotes/origin/trunk)

10. git rebase origin/trunk
First, rewinding head to replay your work on top of it...
Applying:
Applying:

11. git svn dcommit
Committing to svn:///C:/TEMP/SVN/repos/Source_elevation_tool/trunk ...
A   Source_elevation_tool.sln
A   Source_elevation_tool/Form1.Designer.cs
A   Source_elevation_tool/Form1.cs
A   Source_elevation_tool/Form1.resx

ERROR from SVN:
Wrong or unexpected property value: Cannot accept non-LF line endings in 
'svn:log' property
W: 40ff09d157bcbbf1e6deefde38ed499ec8ac and refs/remotes/origin/trunk 
differ, using rebase -v:
:100644 00 8222efb4e5a055b3b0b41ab91972f07dd71e4b10 
 D  Source_elevation_tool.sln
:100644 00 794f014c920a6aee2f21ee348e30f38a458e9c7d 
 D  Source_elevation_tool.vssscc
:04 00 0acaa94fa910fe974019ae4c2dcbf9a620437758 
 D  Source_elevation_tool
Current branch master is up to date.
ERROR: Not all changes have been committed into SVN, however the committed
ones (if any) seem to be successfully integrated into the working tree.
Please see the above messages for details.

I've researched enough to believe that the commit message being used by git svn 
contains a carriage return character (x'0D') and that has not been allowed in 
Subversion since version 1.6 (I can replicate this specific error message using 
an SVN dump file that contains x'0D' characters in the log messages.). However, 
I cannot find where I have any control over the log message that git svn is 
trying to use nor can I observe it. Note that I've also used the '-v' switch 
with the 'git svn dcommit', but do not receive anything other than what I am 
showing above.

Brian Bennett | Supv System Admin & Support, TA TECH Change Mgmt/Production 
Support
o: 319-355-7602 | c: 319-533-1094
e: brian.benn...@transamerica.com | w: www.transamerica.com

Transamerica
6400 C St. SW, Cedar Rapids, IA 52404 MS-2410
Facebook | LinkedIn

Re: [PATCH] mru: Replace mru.[ch] with list.h implementation

[Оля Тележная]

Gargi,
If you have some difficulties - please feel free to ask questions
(here or you can write me directly). I will be happy to help you.
As I understand, you are super close to finish your first patch.

Olga

[PATCH] Makefile: allow NO_DC_SHA1_SUBMODULE to be set in config.mak

[Ramsay Jones]

Signed-off-by: Ramsay Jones 
---

Hi Junio,

Could you please add (or squash) this on top of the 'ab/sha1dc-build'
branch, so that I can build with NO_DC_SHA1_SUBMODULE=NoThanks in my
config.mak.

[If I were to get a vote, I would vote no to the submodule. ;-) ]

Thanks!

ATB,
Ramsay Jones

 Makefile | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/Makefile b/Makefile
index 929b49b04..91bbb0ed8 100644
--- a/Makefile
+++ b/Makefile
@@ -1042,6 +1042,10 @@ EXTLIBS =
 
 GIT_USER_AGENT = git/$(GIT_VERSION)
 
+include config.mak.uname
+-include config.mak.autogen
+-include config.mak
+
 ifndef NO_DC_SHA1_SUBMODULE
ifndef DC_SHA1_EXTERNAL
ifneq ($(wildcard 
sha1collisiondetection/lib/sha1.h),sha1collisiondetection/lib/sha1.h)
@@ -1053,10 +1057,6 @@ whatever reason define NO_DC_SHA1_SUBMODULE=NoThanks)
endif
 endif
 
-include config.mak.uname
--include config.mak.autogen
--include config.mak
-
 ifdef DEVELOPER
 CFLAGS += $(DEVELOPER_CFLAGS)
 endif
-- 
2.15.0

Re: [PATCH] git-send-email: fix get_maintainer.pl regression

[Ævar Arnfjörð Bjarmason]

On Tue, Dec 12 2017, Thomas Adam jotted:

> Hi,
>
> On Mon, Dec 11, 2017 at 08:46:46PM +0100, Ævar Arnfjörð Bjarmason wrote:
>> I.e. we'd just ship a copy of Email::Valid and Mail::Address in
>> perl/Git/FromCPAN/, use a wrapper to load them, and then we wouldn't
>> need to if/else this at the code level, just always use the module,
>> and it would work even on core perl.
>
> I disagree with the premise of this, Ævar.  As soon as you go down this route,
> it increases maintenance to ensure we keep up to date with what's on CPAN for
> a tiny edge-case which I don't believe exists.
>
> You may as well just use App::FatPacker.
>
> We're talking about package maintenance here -- and as I said before, there's
> plenty of it around.  For those distributions which ship Git (and hence also
> package git-send-email), the dependencies are already there, too.  I just
> cannot see this being a problem in relying on non-core perl modules.  Every
> perl program does this, and they don't go down this route of having copies of
> various CPAN modules just in case.  So why should we?  We're not a special
> snowflake.

Something like FatPacker wouldn't make sense in this case, we're not
packing stuff into an archive, but just dropping them during 'make
install', but yes, it's the same idea of shipping our dependencies with
us.

I wouldn't argue for doing this from first principles, in general I
think we're way too conservative about adding dependencies to git.git,
but the general consensus on-list is to do that carefully, that's why we
have all this stuff in contrib/, and why we're depending on perl core
only.

Users or packagers of git don't care what's normal for perl programs, to
them the fact that git-send-email is written in perl is an
implementation detail.

The maintenance burden of just shipping some CPAN module as a fallback
is trivial, for example we've shipped Error.pm since 2006-ish, and until
I sent a patch this month nobody had touched it since 2013.

It's certainly much easier than maintaining a bunch of if/else code
ourselves, or maintaining our own stuff purely because we don't want to
force people to package perl dependencies for git.

[PATCH] doc: clarify usage of XDG_CONFIG_HOME config file

[Jacob Keller]

The documentation for git config and how it reads the user specific
configuration file is misleading. In some places it implies that
$XDG_CONFIG_HOME/git/config will always be read. In others, it implies
that only one of ~/.gitconfig and $XDG_CONFIG_HOME/git/config will be
read.

Improve the documentation explaining how the various configuration files
are read, and combined.

Instead of referencing each file individually, reference each type of
location git will check. When discussing the user configuration, explain
how we switch between one of three choices. Ensure to note that only one
of the three choices is used.

Signed-off-by: Jacob Keller 
---
 Documentation/git-config.txt | 46 +++-
 1 file changed, 24 insertions(+), 22 deletions(-)

diff --git a/Documentation/git-config.txt b/Documentation/git-config.txt
index 14da5fc..4299fd6 100644
--- a/Documentation/git-config.txt
+++ b/Documentation/git-config.txt
@@ -104,13 +104,11 @@ OPTIONS
list them.  Returns error code 1 if no value is found.
 
 --global::
-   For writing options: write to global `~/.gitconfig` file
-   rather than the repository `.git/config`, write to
-   `$XDG_CONFIG_HOME/git/config` file if this file exists and the
-   `~/.gitconfig` file doesn't.
+   For writing options: write to global user configuration file
+   rather than the repository `.git/config`.
 +
-For reading options: read only from global `~/.gitconfig` and from
-`$XDG_CONFIG_HOME/git/config` rather than from all available files.
+For reading options: read only from global user configuration file
+rather than from all available files.
 +
 See also <>.
 
@@ -237,26 +235,30 @@ See also <>.
 FILES
 -
 
-If not set explicitly with `--file`, there are four files where
+If not set explicitly with `--file`, there are three locations where
 'git config' will search for configuration options:
 
-$(prefix)/etc/gitconfig::
-   System-wide configuration file.
-
-$XDG_CONFIG_HOME/git/config::
-   Second user-specific configuration file. If $XDG_CONFIG_HOME is not set
-   or empty, `$HOME/.config/git/config` will be used. Any single-valued
-   variable set in this file will be overwritten by whatever is in
-   `~/.gitconfig`.  It is a good idea not to create this file if
-   you sometimes use older versions of Git, as support for this
-   file was added fairly recently.
+System-wide configuration::
+   Located at `$(prefix)/etc/gitconfig`.
 
-~/.gitconfig::
-   User-specific configuration file. Also called "global"
-   configuration file.
+User-specific configuration::
+   One and only one of the following files will be read
++
+- `~/.gitconfig`
+- `$XDG_CONFIG_HOME/git/config`
+- `$HOME/.config/git/config`
++
+If `~/.gitconfig` exists, it will be used, and the other files will not be
+read. Otherwise, if `$XDG_CONFIG_HOME` is set, then 
`$XDG_CONFIG_HOME/git/config`
+will be used, otherwise `$HOME/.config/git/config` will be used.
++
+Note that git will only ever use one of these files as the global user
+configuration file at once. Additionally if you sometimes use an older version
+of git, it is best to only rely on `~/.gitconfig` as support for the others was
+added fairly recently.
 
-$GIT_DIR/config::
-   Repository specific configuration file.
+Repository-specific configuration::
+   Located at `$GIT_DIR/config`.
 
 If no further options are given, all reading options will read all of these
 files that are available. If the global or the system-wide configuration
-- 
2.7.4

Re: [PATCH v1] convert: add support for 'encoding' attribute

[Lars Schneider]

> On 12 Dec 2017, at 00:58, Eric Sunshine  wrote:
> 
> On Mon, Dec 11, 2017 at 6:47 PM, Lars Schneider
>  wrote:
>> On 11 Dec 2017, at 19:39, Eric Sunshine  wrote:
>>> On Mon, Dec 11, 2017 at 10:50 AM,   wrote:
 From: Lars Schneider 
 
 Git and its tools (e.g. git diff) expect all text files in UTF-8
 encoding. Git will happily accept content in all other encodings, too,
 but it might not be able to process the text (e.g. viewing diffs or
 changing line endings).
 
 Add an attribute to tell Git what encoding the user has defined for a
 given file. If the content is added to the index, then Git converts the
 content to a canonical UTF-8 representation. On checkout Git will
 reverse the conversion.
 
 Reviewed-by: Patrick Lühne 
 Signed-off-by: Lars Schneider 
 ---
 +static int encode_to_git(const char *path, const char *src, size_t 
 src_len,
 +struct strbuf *buf, struct encoding *enc)
 +{
 +   if (enc->to_git == invalid_conversion) {
 +   enc->to_git = iconv_open(default_encoding, encoding->name);
 +   if (enc->to_git == invalid_conversion)
 +   warning(_("unsupported encoding %s"), 
 encoding->name);
 +   }
 +
 +   if (enc->to_worktree == invalid_conversion)
 +   enc->to_worktree = iconv_open(encoding->name, 
 default_encoding);
>>> 
>>> Do you need to be calling iconv_close() somewhere on the result of the
>>> iconv_open() calls? [Answering myself after reading the rest of the
>>> patch: You're caching these opened 'iconv' descriptors, so you don't
>>> plan on closing them.]
>> 
>> Should this information go into the commit message to avoid confusing
>> future readers? I think, yes.
> 
> Maybe. However, the code which does the actual caching is so distant
> from these iconv_open() invocations that it might be more helpful to
> have an in-code comment here saying that the "missing" iconv_close()
> invocations is intentional.

Agreed. I will add that in v2.

Thanks,
Lars

Re: [PATCH v1] convert: add support for 'encoding' attribute

[Lars Schneider]

> On 12 Dec 2017, at 08:15, Johannes Sixt  wrote:
> 
> Am 12.12.2017 um 01:59 schrieb Junio C Hamano:
>> Stepping back a bit, what does this thing do you are introducing?
>> And what does the other thing do that J6t is using, that would get
>> confused with this new one?
>> What does the other one do?  "Declare that the contents of this path
>> is in this encoding"?  As opposed to the new one, which tells Git to
>> "run iconv from and to this encoding when checking out and checking
>> in"?
>> If so, any phrase that depends heavily on the word "encode" would
>> not help differenciating the two uses.  The phrase needs to be
>> something that contrasts the new one, which actively modifies things
>> (what is on the filesystem is not what is stored in the object
>> store), with the old one, which does not (passed as a declaration to
>> a viewer what encoding the contents already use and does not change
>> anything).
> 
> Well explained!
> 
>> ...  perhaps "smudge-encoding" would work (we declare that the
>> result of smudge operations are left in this encoding, so the
>> opposite operation "clean" will do the reverse---and we say this
>> without explicitly saying that the other end of the conversion is
>> always UTF-8)?  Or "checkout-encoding" (the same explanation; we do
>> not say the opposite operation "checkin/add" will do the reverse).
> 
> I would favor "checkout-encoding" over "smudge-encoding" only because 
> "checkout" is better known than "smudge", I would think. I do not have better 
> suggestions.

Thanks for your thoughts! "checkout-encoding" would work for me.
However, it might convey that Git "does change the files of the
user in some way" (which is true from Git's perspective but not
from the user perspective).

Patrick and I brainstormed a few more possible alternatives:

apply-encoding
blob-encoding
checkout-encoding
diff-encoding
encoding-hint
external-encoding
handle-as
internal-encoding
keep-encoding
present-as
preserve-encoding
source-encoding
text-conversion
text-encoding
treat-as 
treat-encoding-as

Our favorite is "treat-encoding-as". Do you consider this better
or worse than "checkout-encoding"?

Thanks,
Lars


PS: Naming things is hard ;-)

feature-request: git "cp" like there is git mv.

[Simon Doodkin]

please develop a new feature, git "cp" like there is git mv tomovefile1 tofile2
(to save space).

there is a solution in https://stackoverflow.com/a/44036771/466363
however, it is not single easy command.

Re: [Proposed] Externalize man/html ref for quick-install-man and quick-install-html

[Jacob Keller]

On Mon, Dec 11, 2017 at 4:26 PM, Randall S. Becker
 wrote:
> Sorry about the response positioning...
>
> I can send you a pull request on github, if you want 
>

You can use https://submitgit.herokuapp.com/ to submit to the mailing
list from a pull request.

Thanks,
Jake

Re: [PATCH v5 0/9] sequencer: don't fork git commit

[Phillip Wood]

On 11/12/17 23:44, Junio C Hamano wrote:
> Phillip Wood  writes:
> 
>> From: Phillip Wood 
>>
>> I've reworked the config handling since v4. It now stores the default
>> values in struct replay_opt rather than using global variables and
>> calls git_diff_basic_config(). Unfortunately I've not had time to
>> modify git_gpg_config() to indicate if it successfully handled the key
>> so git_diff_basic_config() is called unnecessarily in that case. Within
>> git_diff_basic_config() userdiff_config() also suffers from the same
>> problem of not indicating if it has handled the key.
> 
> Ouch.  I thought we agreed that we were ready to go incremental and
> the topic was merged to 'next' earlier last week.

Sorry, I thought we were waiting, not to worry.
> 
> After scanning the difference between the two rounds, it seems that
> the more important difference is the rework of the configuration,
> which looks better thought out than the previous round, and with
> associated change to use replay_opts fields instead of free variables
> to carry gpg-sign and cleanup-mode settings around, which also looks
> sensible to me.

Yes the configuration is the major change, I'm glad you think it is an
improvement

> Can you make these differences into incremental "that earlier one
> was suboptimal for this and that reasons, let's make it better by
> doing this" patches to queue them on top?

Will do, though if I don't get round to it today or tomorrow it may be
the New Year before I send them, I hope that's OK

Best Wishes

Phillip

> Thanks.
> 

Re: [PATCH] git-send-email: fix get_maintainer.pl regression

[Thomas Adam]

Hi,

On Mon, Dec 11, 2017 at 08:46:46PM +0100, Ævar Arnfjörð Bjarmason wrote:
> I.e. we'd just ship a copy of Email::Valid and Mail::Address in
> perl/Git/FromCPAN/, use a wrapper to load them, and then we wouldn't
> need to if/else this at the code level, just always use the module,
> and it would work even on core perl.

I disagree with the premise of this, Ævar.  As soon as you go down this route,
it increases maintenance to ensure we keep up to date with what's on CPAN for
a tiny edge-case which I don't believe exists.

You may as well just use App::FatPacker.

We're talking about package maintenance here -- and as I said before, there's
plenty of it around.  For those distributions which ship Git (and hence also
package git-send-email), the dependencies are already there, too.  I just
cannot see this being a problem in relying on non-core perl modules.  Every
perl program does this, and they don't go down this route of having copies of
various CPAN modules just in case.  So why should we?  We're not a special
snowflake.

-- Thomas Adam

Re: Q: rational for $XDG_CONFIG_HOME/git/config to be "non global" or just a bug?

[Jacob Keller]

On Mon, Dec 11, 2017 at 5:05 PM, Junio C Hamano  wrote:
> Jonathan Nieder  writes:
>
>> I think the documentation
>>
>>   ~/.gitconfig
>>   User-specific configuration file. Also called "global"
>>   configuration file.
>>
>> should be clarified --- e.g. it could say
>>
>>   $XDG_CONFIG_HOME/git/config
>>   ~/.gitconfig
>>   User-specific configuration files. Because options in
>>   these files are not specific to any repository, thes
>>   are sometimes called global configuration files.
>
> Yeah, I think that makes sense.
>
>> As for "git config --global", I think the best thing would be to split
>> it into two options: something like "git config --user" and "git
>> config --xdg-user".  That way, it is unambiguous which configuration
>> file the user intends to inspect or modify.  When a user calls "git
>> config --global" and both files exist, it could warn that the command
>> is ambiguous.
>>
>> Thoughts?
>
> I actually thought that the plan was "you either have this, or the
> other one, never both at the same time" (and I think those who
> pushed the XDG thing in to the system made us favor it over the
> traditional one).  So as long as --global updates the one that
> exists, and updates XDG one when both or neither do, I think we
> should be OK.  And from that viewpoint, we definitely do not want
> two kinds of --global to pretend as if we support use of both at the
> same time.
>

It appears that we actually prefer ~/.gitconfig rather than XDG_CONFIG_HOME..

And at least based on current cursory testing on the command line, we
do both read and write to the proper location, assuming that
~/.gitconfig is preferred over $XDG_CONFIG_HOME.

Thanks,
Jake

Re: What's cooking in git.git (Dec 2017, #02; Thu, 7)

[Christian Couder]

On Thu, Dec 7, 2017 at 7:04 PM, Junio C Hamano  wrote:
>
> * jh/object-filtering (2017-12-05) 9 commits
>   (merged to 'next' on 2017-12-05 at 3a56b51085)
>  + rev-list: support --no-filter argument
>  + list-objects-filter-options: support --no-filter
>  + list-objects-filter-options: fix 'keword' typo in comment
>   (merged to 'next' on 2017-11-27 at e5008c3b28)
>  + pack-objects: add list-objects filtering
>  + rev-list: add list-objects filtering support
>  + list-objects: filter objects in traverse_commit_list
>  + oidset: add iterator methods to oidset
>  + oidmap: add oidmap iterator methods
>  + dir: allow exclusions from blob in addition to file
>  (this branch is used by jh/fsck-promisors and jh/partial-clone.)
>
>  In preparation for implementing narrow/partial clone, the object
>  walking machinery has been taught a way to tell it to "filter" some
>  objects from enumeration.
>
>
> * jh/fsck-promisors (2017-12-05) 12 commits
>  - gc: do not repack promisor packfiles
>  - rev-list: support termination at promisor objects
>  - fixup: sha1_file: add TODO
>  - fixup: sha1_file: convert gotos to break/continue
>  - sha1_file: support lazily fetching missing objects
>  - introduce fetch-object: fetch one promisor object
>  - index-pack: refactor writing of .keep files
>  - fsck: support promisor objects as CLI argument
>  - fsck: support referenced promisor objects
>  - fsck: support refs pointing to promisor objects
>  - fsck: introduce partialclone extension
>  - extension.partialclone: introduce partial clone extension
>  (this branch is used by jh/partial-clone; uses jh/object-filtering.)
>
>  In preparation for implementing narrow/partial clone, the machinery
>  for checking object connectivity used by gc and fsck has been
>  taught that a missing object is OK when it is referenced by a
>  packfile specially marked as coming from trusted repository that
>  promises to make them available on-demand and lazily.

I am currently working on integrating this series with my external odb
series 
(https://public-inbox.org/git/20170916080731.13925-1-chrisc...@tuxfamily.org/).

Instead of using an "extension.partialclone" config variable, an odb
will be configured like using an "odb..promisorRemote" (the
name might still change) config variable. Other odbs could still be
configured using "odb..scriptCommand" and
"odb..subprocessCommand".

The current work is still very much WIP and some tests fail, but you
can take a look there:

https://github.com/chriscool/git/tree/gl-promisor-external-odb440

[PATCH Outreachy v2 2/2] format: create docs for pretty.h

[Olga Telezhnaya]

Write some docs for functions in pretty.h.
Take it as a first draft, they would be changed later.

Signed-off-by: Olga Telezhnaia 
Mentored-by: Christian Couder 
Mentored by: Jeff King 
---
 pretty.h | 44 
 1 file changed, 44 insertions(+)

diff --git a/pretty.h b/pretty.h
index ef5167484fb64..5c85d94e332d7 100644
--- a/pretty.h
+++ b/pretty.h
@@ -48,6 +48,7 @@ struct pretty_print_context {
int graph_width;
 };
 
+/* Check whether commit format is mail. */
 static inline int cmit_fmt_is_mail(enum cmit_fmt fmt)
 {
return (fmt == CMIT_FMT_EMAIL || fmt == CMIT_FMT_MBOXRD);
@@ -57,31 +58,74 @@ struct userformat_want {
unsigned notes:1;
 };
 
+/* Set the flag "w->notes" if there is placeholder %N in "fmt". */
 void userformat_find_requirements(const char *fmt, struct userformat_want *w);
+
+/*
+ * Shortcut for invoking pretty_print_commit if we do not have any context.
+ * Context would be set empty except "fmt".
+ */
 void pp_commit_easy(enum cmit_fmt fmt, const struct commit *commit,
struct strbuf *sb);
+
+/*
+ * Get information about user and date from "line", format it and
+ * put it into "sb".
+ * Format of "line" must be readable for split_ident_line function.
+ * The resulting format is "what: name  date".
+ */
 void pp_user_info(struct pretty_print_context *pp, const char *what,
struct strbuf *sb, const char *line,
const char *encoding);
+
+/*
+ * Format title line of commit message taken from "msg_p" and
+ * put it into "sb".
+ * First line of "msg_p" is also affected.
+ */
 void pp_title_line(struct pretty_print_context *pp, const char **msg_p,
struct strbuf *sb, const char *encoding,
int need_8bit_cte);
+
+/*
+ * Get current state of commit message from "msg_p" and continue formatting
+ * by adding indentation and '>' signs. Put result into "sb".
+ */
 void pp_remainder(struct pretty_print_context *pp, const char **msg_p,
struct strbuf *sb, int indent);
 
+/*
+ * Create a text message about commit using given "format" and "context".
+ * Put the result to "sb".
+ * Please use this function for custom formats.
+ */
 void format_commit_message(const struct commit *commit,
const char *format, struct strbuf *sb,
const struct pretty_print_context *context);
 
+/*
+ * Parse given arguments from "arg", check it for correctness and
+ * fill struct rev_info.
+ */
 void get_commit_format(const char *arg, struct rev_info *);
 
+/*
+ * Make a commit message with all rules from given "pp"
+ * and put it into "sb".
+ * Please use this function if you have a context (candidate for "pp").
+ */
 void pretty_print_commit(struct pretty_print_context *pp,
const struct commit *commit,
struct strbuf *sb);
 
+/*
+ * Change line breaks in "msg" to "line_separator" and put it into "sb".
+ * Return "msg" itself.
+ */
 const char *format_subject(struct strbuf *sb, const char *msg,
const char *line_separator);
 
+/* Check if "cmit_fmt" will produce an empty output. */
 int commit_format_is_empty(enum cmit_fmt);
 
 #endif /* PRETTY_H */

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

[PATCH Outreachy v2 1/2] format: create pretty.h file

[Olga Telezhnaya]

Create header for pretty.c to make formatting interface more structured.
This is a middle point, this file would be merged further with other
files which contain formatting stuff.

Signed-off-by: Olga Telezhnaia 
Mentored-by: Christian Couder 
Mentored by: Jeff King 
---
 builtin/notes.c   |  2 +-
 builtin/reset.c   |  2 +-
 builtin/show-branch.c |  2 +-
 commit.h  | 81 +--
 pretty.h  | 87 +++
 revision.h|  2 +-
 6 files changed, 92 insertions(+), 84 deletions(-)
 create mode 100644 pretty.h

diff --git a/builtin/notes.c b/builtin/notes.c
index 1a2c7d92ad7e7..7c8176164561b 100644
--- a/builtin/notes.c
+++ b/builtin/notes.c
@@ -12,7 +12,7 @@
 #include "builtin.h"
 #include "notes.h"
 #include "blob.h"
-#include "commit.h"
+#include "pretty.h"
 #include "refs.h"
 #include "exec_cmd.h"
 #include "run-command.h"
diff --git a/builtin/reset.c b/builtin/reset.c
index 906e541658230..e15f595799c40 100644
--- a/builtin/reset.c
+++ b/builtin/reset.c
@@ -12,7 +12,7 @@
 #include "lockfile.h"
 #include "tag.h"
 #include "object.h"
-#include "commit.h"
+#include "pretty.h"
 #include "run-command.h"
 #include "refs.h"
 #include "diff.h"
diff --git a/builtin/show-branch.c b/builtin/show-branch.c
index 2e24b5c330e8e..e8a4aa40cb4b6 100644
--- a/builtin/show-branch.c
+++ b/builtin/show-branch.c
@@ -1,6 +1,6 @@
 #include "cache.h"
 #include "config.h"
-#include "commit.h"
+#include "pretty.h"
 #include "refs.h"
 #include "builtin.h"
 #include "color.h"
diff --git a/commit.h b/commit.h
index 99a3fea68d3f6..8c68ca1a5a187 100644
--- a/commit.h
+++ b/commit.h
@@ -7,6 +7,7 @@
 #include "decorate.h"
 #include "gpg-interface.h"
 #include "string-list.h"
+#include "pretty.h"
 
 struct commit_list {
struct commit *item;
@@ -121,93 +122,13 @@ struct commit_list *copy_commit_list(struct commit_list 
*list);
 
 void free_commit_list(struct commit_list *list);
 
-/* Commit formats */
-enum cmit_fmt {
-   CMIT_FMT_RAW,
-   CMIT_FMT_MEDIUM,
-   CMIT_FMT_DEFAULT = CMIT_FMT_MEDIUM,
-   CMIT_FMT_SHORT,
-   CMIT_FMT_FULL,
-   CMIT_FMT_FULLER,
-   CMIT_FMT_ONELINE,
-   CMIT_FMT_EMAIL,
-   CMIT_FMT_MBOXRD,
-   CMIT_FMT_USERFORMAT,
-
-   CMIT_FMT_UNSPECIFIED
-};
-
-static inline int cmit_fmt_is_mail(enum cmit_fmt fmt)
-{
-   return (fmt == CMIT_FMT_EMAIL || fmt == CMIT_FMT_MBOXRD);
-}
-
 struct rev_info; /* in revision.h, it circularly uses enum cmit_fmt */
 
-struct pretty_print_context {
-   /*
-* Callers should tweak these to change the behavior of pp_* functions.
-*/
-   enum cmit_fmt fmt;
-   int abbrev;
-   const char *after_subject;
-   int preserve_subject;
-   struct date_mode date_mode;
-   unsigned date_mode_explicit:1;
-   int print_email_subject;
-   int expand_tabs_in_log;
-   int need_8bit_cte;
-   char *notes_message;
-   struct reflog_walk_info *reflog_info;
-   struct rev_info *rev;
-   const char *output_encoding;
-   struct string_list *mailmap;
-   int color;
-   struct ident_split *from_ident;
-
-   /*
-* Fields below here are manipulated internally by pp_* functions and
-* should not be counted on by callers.
-*/
-   struct string_list in_body_headers;
-   int graph_width;
-};
-
-struct userformat_want {
-   unsigned notes:1;
-};
-
 extern int has_non_ascii(const char *text);
 extern const char *logmsg_reencode(const struct commit *commit,
   char **commit_encoding,
   const char *output_encoding);
-extern void get_commit_format(const char *arg, struct rev_info *);
-extern const char *format_subject(struct strbuf *sb, const char *msg,
- const char *line_separator);
-extern void userformat_find_requirements(const char *fmt, struct 
userformat_want *w);
-extern int commit_format_is_empty(enum cmit_fmt);
 extern const char *skip_blank_lines(const char *msg);
-extern void format_commit_message(const struct commit *commit,
- const char *format, struct strbuf *sb,
- const struct pretty_print_context *context);
-extern void pretty_print_commit(struct pretty_print_context *pp,
-   const struct commit *commit,
-   struct strbuf *sb);
-extern void pp_commit_easy(enum cmit_fmt fmt, const struct commit *commit,
-  struct strbuf *sb);
-void pp_user_info(struct pretty_print_context *pp,
- const char *what, struct strbuf *sb,
- const char *line, const char *encoding);
-void pp_title_line(struct pretty_print_context *pp,
-  const char **msg_p,
-  struct strbuf *sb,