git blame --reverse doesn't find line in HEAD

2017-11-29 Thread Nick Snyder
I have a repo that reproduces a behavior with `git blame --reverse`
that surprises me. https://github.com/nicksnyder/git-blame-bug

The readme explains the observed behavior and what I expected to
happen. I will inline the readme at the bottom of this message for
convenience.

Am I misunderstanding --reverse or is this a bug?

Thanks!
Nick

$ git --version
git version 2.15.0

Blame of L465 in Tree.tsx at HEAD (ca0fb5) points to L463 at 199ee7

$ git blame -p -L465,465 Tree.tsx
199ee75d1240ae72cd965f62aceeb301ab64e1bd 463 465 1
filename Tree.tsx
public shouldComponentUpdate(nextProps: TileProps): boolean {

EXPECTED: Reverse blame of L463 at 199ee7 points to L465 at the
lastest commit in the repo (at least ca0fb5).
ACTUAL: Reverse blame of L463 at 199ee7 points to L463 at 199ee7.

$ git blame -p -L463,463 --reverse 199ee7.. Tree.tsx
199ee75d1240ae72cd965f62aceeb301ab64e1bd 463 463 1
boundary
previous ca0fb5a2d61cb16909bcb06f49dd5448a26f32b1 Tree.tsx
filename Tree.tsx
public shouldComponentUpdate(nextProps: TileProps): boolean {

The line in question is in the diff (git diff 199ee7..ca0fb5), but
that particular line is neither added nor deleted, so I don't know why
blame would think it is deleted.

Relevant hunk in diff:

@@ -452,28 +462,17 @@ export class LayerTile extends
React.Component {
 }
 }

-public validTokenRange(props: TileProps): boolean {
-if (props.selectedPath === '') {
-return true
-}
-const token = props.selectedPath.split('/').pop()!
-return token >= this.first && token <= this.last
-}
-
 public shouldComponentUpdate(nextProps: TileProps): boolean {
-const lastValid = this.validTokenRange(this.props)
-const nextValid = this.validTokenRange(nextProps)
-if (!lastValid && !nextValid) {
-// short circuit
-return false
+if (isEqualOrAncestor(this.props.selectedDir,
this.props.currSubpath)) {
+return true
 }
-if (isEqualOrAncestor(this.props.selectedDir,
this.props.currSubpath) && lastValid) {
+if (nextProps.selectedDir === nextProps.currSubpath) {
 return true
 }
-if (nextProps.selectedDir === nextProps.currSubpath &&
this.validTokenRange(nextProps)) {
+if (getParentDir(nextProps.selectedDir) === nextProps.currSubpath) {
 return true
 }
-if (getParentDir(nextProps.selectedDir) ===
nextProps.currSubpath && this.validTokenRange(nextProps)) {
+if (!isEqual(nextProps.pathSplits, this.props.pathSplits)) {
 return true
 }
 return false


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

2017-11-29 Thread Ulrich Windl




> Hi Ulrich,
> 
> On Tue, 28 Nov 2017, Ulrich Windl wrote:
> 
>> During a rebase that turned out to be heavier than expected 8-( I
>> decided to keep the old branch by creating a temporary branch name to
>> the commit of the branch to rebase (which was still the old commit ID at
>> that time).
>>
>> When done rebasing, I attached a new name to the new (rebased) branch,
>> deleted the old name (pointing at the same rebase commit), then
>> recreated the old branch from the temporary branch name (created to
>> remember the commit id).
>>
>> When I wanted to delete the temporary branch (which is of no use now), I
>> got a message that the branch is unmerged.
> 
> This is actually as designed, at least for performance reasons (it is not
> exactly cheap to figure out whether a given commit is contained in any
> other branch).
> 
>> 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. Do you
>> agree?
> 
> No, respectfully disagree, because I have found myself with branches
> pointing to the same commit, even if the branches served different
> purposes. I really like the current behavior where you can delete a
> branch with `git branch -d` as long as it is contained in its upstream
> branch.

Hi!

I'm not talking about the intention of a branch, but of the state of a branch: 
If multiple branches point (not "contain") the same commit, they are equivalent 
(besides the name) at that moment. As no program can predict the future or the 
intentions of the user, it should be safe to delete the branch, because it can 
easily be recreated (from the remaining branches pointing to the same commit).

It shouldn't need a lot of computational power to find out when multiple 
branches point to the same commit.

Regards,
Ulrich




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

2017-11-29 Thread Ulrich Windl

> "Ulrich Windl"  writes:
> 
>> 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. Do you agree?
> 
> That comes from a viewpoint that the only purpose "branch -d" exists
> in addition to "branch -D" is to protect objects from "gc".  Those
> who added the safety feature may have shared that view originally,
> but it turns out that it protects another important thing you are
> forgetting.
> 
> Imagine that two topics, 'topicA' and 'topicB', were independently
> forked from 'master', and then later we wanted to add a feature that
> depends on these two topics.  Since the 'feature' forked, there may
> have been other developments, and we ended up in this topology:
> 
> ---o---o---o---o---o---M
> \   \  
>  \   o---A---o---F
>   \ /  
>o---o---o---o---B
> 
> where A, B and F are the tips of 'topicA', 'topicB' and 'feature'
> branches right now [*1*].
> 
> Now imagine we are on 'master' and just made 'topicB' graduate.  We
> would have this topology.
> 
> ---o---o---o---o---o---o---M
> \   \ /
>  \   o---A---o---F   /
>   \ /   /
>o---o---o---o---B
> 
> While we do have 'topicA' and 'feature' branches still in flight,
> we are done with 'topicB'.  Even though the tip of 'topicA' is
> reachable from the tip of 'feature', the fact that the branch points
> at 'A' is still relevant.  If we lose that information right now,
> we'd have to go find it when we (1) want to further enhance the
> topic by checking out and building on 'topicA', and (2) want to
> finally get 'topicA' graduate to 'master'.
> 
> Because removal of a topic (in this case 'topicB') is often done
> after a merge of that topic is made into an integration branch,
> "branch -d" that protects branches that are yet to be merged to the
> current branch catches you if you said "branch -d topic{A,B}" (or
> other equivalent forms, most likely you'd have a script that spits
> out list of branches and feed it to "xargs branch -d").
> 
> So, no, I do not agree.

Hi!

I can follow your argumentation, but I fail to see that your branches A and B 
point to the same commit (which is what I was talking about). So my situation 
would be:

o---oA,B

I still think I could safely remove either A or B, even when the branch 
(identified by the commit, not by the name) is unmerged. What did I miss?

Regards,
Ulrich

> 
> 
> [Footnotes]
> 
> *1* Since the 'feature' started developing, there were a few commits
> added to 'topicB' but because the feature does not depend on
> these enhancements to that topic, B is ahead of the commit that
> was originally merged with the tip of 'topicA' to form the
> 'feature' branch.



Re: [PATCH] strbuf: Remove unused stripspace function alias

2017-11-29 Thread Tobias Klauser
On 2017-11-29 at 02:45:59 +0100, Elijah Newren  wrote:
> In commit 63af4a8446 ("strbuf: make stripspace() part of strbuf",
> 2015-10-16), stripspace() was moved to strbuf and renamed to
> strbuf_stripspace().  A "temporary" alias was added for the old name until
> all topic branches had time to switch over.  They have had time, so remove
> the old alias.
> 
> Signed-off-by: Elijah Newren 

Reviewed-by: Tobias Klauser 

Thanks!


Reply

2017-11-29 Thread Wong
I have something very confidential and private to discuss with you 




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

2017-11-29 Thread Johannes Schindelin
Hi Ulrich,

On Wed, 29 Nov 2017, Ulrich Windl wrote:

> > On Tue, 28 Nov 2017, Ulrich Windl wrote:
> > 
> >> During a rebase that turned out to be heavier than expected 8-( I
> >> decided to keep the old branch by creating a temporary branch name to
> >> the commit of the branch to rebase (which was still the old commit ID
> >> at that time).
> >>
> >> When done rebasing, I attached a new name to the new (rebased)
> >> branch, deleted the old name (pointing at the same rebase commit),
> >> then recreated the old branch from the temporary branch name (created
> >> to remember the commit id).
> >>
> >> When I wanted to delete the temporary branch (which is of no use
> >> now), I got a message that the branch is unmerged.
> > 
> > This is actually as designed, at least for performance reasons (it is
> > not exactly cheap to figure out whether a given commit is contained in
> > any other branch).
> > 
> >> 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.
> >> Do you agree?
> > 
> > No, respectfully disagree, because I have found myself with branches
> > pointing to the same commit, even if the branches served different
> > purposes. I really like the current behavior where you can delete a
> > branch with `git branch -d` as long as it is contained in its upstream
> > branch.
> 
> I'm not talking about the intention of a branch, but of the state of a
> branch: If multiple branches point (not "contain") the same commit, they
> are equivalent (besides the name) at that moment.

I did a poor job of explaining myself, please let me try again. I'll give
you one concrete example:

Recently, while working on some topic, I stumbled over a bug and committed
a bug fix, then committed that and branched off a new branch to remind
myself to rebase the bug fix and contribute it.

At that point, those branches were at the same revision, but distinctly
not equivalent (except in just one, very narrow sense of the word, which I
would argue is the wrong interpretation in this context).

Sadly, I was called away at that moment to take care of something
completely different. Even if I had not been, the worktree with the first
branch would still have been at that revision for a longer time, as I had
to try out a couple of changes before I could commit.

This is just one example where the idea backfires that you can safely
delete one of two branches that happen to point at the same commit at the
same time.

I am sure that you possess vivid enough of an imagination to come up with
plenty more examples where that is the case.

> As no program can predict the future or the intentions of the user, it
> should be safe to delete the branch, because it can easily be recreated
> (from the remaining branches pointing to the same commit).

Yes, no program can predict the future (at least *accurately*).

No, it is not safe to delete that branch. Especially if you take the
current paradigm of "it is safe to delete a branch if it is up-to-date
with, or at least fast-forwardable to, its upstream branch" into account.

And no, a branch cannot easily be recreated from the remaining branches in
the future, as branches can have different reflogs (and they are lost when
deleting the branch).

> It shouldn't need a lot of computational power to find out when multiple
> branches point to the same commit.

Sure, that test can even be scripted easily by using the `git for-each-ref
--points-at=` command.

By the way, if you are still convinced that my argument is flawed and that
it should be considered safe to delete a branch if any other branch points
to the same revision, I encourage you to work on a patch to make it so.

For maximum chance of getting included, you would want to guard this
behind a new config setting, say, branch.deleteRedundantIsSafe, parse it
here:

https://github.com/git/git/blob/v2.15.1/config.c#L1260-L1288

or here:

https://github.com/git/git/blob/v2.15.1/builtin/branch.c#L78-L97

document it here:

https://github.com/git/git/blob/v2.15.1/Documentation/git-branch.txt

and here:

https://github.com/git/git/blob/v2.15.1/Documentation/config.txt#L969

and handle it here:

https://github.com/git/git/blob/v2.15.1/builtin/branch.c#L185-L288

(look for the places where `force` is used, likely just before the call to
`check_branch_commit()`).

The way you'd want it to handle is most lilkely by imitating the
`--points-at` code here:
https://github.com/git/git/blob/v2.15.1/builtin/for-each-ref.c#L42

Ciao,
Johannes


[ANNOUNCE] Git for Windows 2.15.1

2017-11-29 Thread Johannes Schindelin
Dear Git users,

It is my pleasure to announce that Git for Windows 2.15.1 is available from:

https://git-for-windows.github.io/

Changes since Git for Windows v2.15.0 (October 30th 2017)

New Features

  * Comes with Git v2.15.1.
  * Operations in massively-sparse worktrees are now much faster if
core.fscache = true.
  * It is now possible to configure nano or Notepad++ as Git's default
editor instead of vim.
  * Comes with OpenSSL v1.0.2m.
  * Git for Windows' updater now uses non-intrusive toast notifications
on Windows 8, 8.1 and 10.
  * Running git fetch in a repository with lots of refs is now
considerably faster.
  * Comes with cURL v7.57.0.

Bug Fixes

  * The experimental --show-ignored-directory option of git status
which was removed in Git for Windows v2.15.0 without warning has
been reintroduced as a deprecated option.
  * The git update command (to auto-update Git for Windows) will now
work also work behind proxies.

Filename | SHA-256
 | ---
Git-2.15.1-64-bit.exe | 
a2ba53197db79b361502836eecf97f09881703148774f9b4e9e6749d41d4ff71
Git-2.15.1-32-bit.exe | 
73154bdfd1ad4ced8612d97b95603ff2b2383db9d46b4c308827fb5233d52592
PortableGit-2.15.1-64-bit.7z.exe | 
94d485454af33a32d08680950aaf38f0825a189ef8b617054b81b2c48d817699
PortableGit-2.15.1-32-bit.7z.exe | 
7d804748a7de735133d78c5420d9b338379123734509415035593e106b03515a
MinGit-2.15.1-64-bit.zip | 
5e38d13241b0742e6673bc5116ac82e851dd1fad01c660c943017f4549b6afea
MinGit-2.15.1-32-bit.zip | 
2fc85f67cabe3c13aceb6868b4bb6bda28ddfecd6f32d7e0da9ddce8cee9b940
MinGit-2.15.1-busybox-64-bit.zip | 
02611486e3c8c427f09d2c4639484cd604ea812471248ae928960f1e0dc59633
MinGit-2.15.1-busybox-32-bit.zip | 
a6dfb770f5cfa7b120ba49922d3434577b8601c5d322ad473dd2e2adc91e92b3
Git-2.15.1-64-bit.tar.bz2 | 
bb630e5f3d7b650db67134b0187cf0cb8f5ed75990838ee65fed85e21777f08a
Git-2.15.1-32-bit.tar.bz2 | 
ec3938e161ac1bbcf2b4f5d41fae1c05ea229fa0276b4db8530ec50b69131832

Ciao,
Johannes


Re: [PATCH] git-prompt: fix reading files with windows line endings

2017-11-29 Thread Johannes Schindelin
Hi Robert,

On Tue, 28 Nov 2017, Robert Abel wrote:

> If any of the files read by __git_eread have \r\n line endings, read
> will only strip \n, leaving \r. This results in an ugly prompt, where
> instead of
> 
> user@pc MINGW64 /path/to/repo (BARE:master)
> 
> the last parenthesis is printed over the beginning of the prompt like
> 
> )ser@pc MINGW64 /path/to/repo (BARE:master

Thats' unfortunate, and obviously something to fix.

> diff --git a/contrib/completion/git-prompt.sh 
> b/contrib/completion/git-prompt.sh
> index c6cbef38c2..71a64e7959 100644
> --- a/contrib/completion/git-prompt.sh
> +++ b/contrib/completion/git-prompt.sh
> @@ -282,7 +282,7 @@ __git_eread ()
>  {
>   local f="$1"
>   shift
> - test -r "$f" && read "$@" <"$f"
> + test -r "$f" && read "$@" <"$f" && export $@="${!@%$'\r'}"

As far as I understand, $'\r' is a Bash-only construct, and this file
(git-prompt.sh) is targeting other Unix shells, too.

So how about using `tr -d '\r' <"$f" | read "$@"` instead?

Or maybe keep with the Bash construct, but guarded behind a test that we
area actually running in Bash? Something like

test -z "$BASH" || IFS=$' \t\r\n'

Ciao,
Johannes


[PATCH v4 1/2] refactor "dumb" terminal determination

2017-11-29 Thread lars . schneider
From: Lars Schneider 

Move the code to detect "dumb" terminals into a single location. This
avoids duplicating the terminal detection code yet again in a subsequent
commit.

Signed-off-by: Lars Schneider 
---
 cache.h| 1 +
 color.c| 3 +--
 editor.c   | 9 +++--
 sideband.c | 5 ++---
 4 files changed, 11 insertions(+), 7 deletions(-)

diff --git a/cache.h b/cache.h
index 89f5d24579..3842fc097c 100644
--- a/cache.h
+++ b/cache.h
@@ -1469,6 +1469,7 @@ extern const char *ident_default_name(void);
 extern const char *ident_default_email(void);
 extern const char *git_editor(void);
 extern const char *git_pager(int stdout_is_tty);
+extern int is_terminal_dumb(void);
 extern int git_ident_config(const char *, const char *, void *);
 extern void reset_ident_date(void);
 
diff --git a/color.c b/color.c
index 9a9261ac16..d48dd947c9 100644
--- a/color.c
+++ b/color.c
@@ -329,8 +329,7 @@ static int check_auto_color(void)
if (color_stdout_is_tty < 0)
color_stdout_is_tty = isatty(1);
if (color_stdout_is_tty || (pager_in_use() && pager_use_color)) {
-   char *term = getenv("TERM");
-   if (term && strcmp(term, "dumb"))
+   if (!is_terminal_dumb())
return 1;
}
return 0;
diff --git a/editor.c b/editor.c
index 7519edecdc..c65ea698eb 100644
--- a/editor.c
+++ b/editor.c
@@ -7,11 +7,16 @@
 #define DEFAULT_EDITOR "vi"
 #endif
 
+int is_terminal_dumb(void)
+{
+   const char *terminal = getenv("TERM");
+   return !terminal || !strcmp(terminal, "dumb");
+}
+
 const char *git_editor(void)
 {
const char *editor = getenv("GIT_EDITOR");
-   const char *terminal = getenv("TERM");
-   int terminal_is_dumb = !terminal || !strcmp(terminal, "dumb");
+   int terminal_is_dumb = is_terminal_dumb();
 
if (!editor && editor_program)
editor = editor_program;
diff --git a/sideband.c b/sideband.c
index 1e4d684d6c..6d7f943e43 100644
--- a/sideband.c
+++ b/sideband.c
@@ -20,13 +20,12 @@
 
 int recv_sideband(const char *me, int in_stream, int out)
 {
-   const char *term, *suffix;
+   const char *suffix;
char buf[LARGE_PACKET_MAX + 1];
struct strbuf outbuf = STRBUF_INIT;
int retval = 0;
 
-   term = getenv("TERM");
-   if (isatty(2) && term && strcmp(term, "dumb"))
+   if (isatty(2) && !is_terminal_dumb())
suffix = ANSI_SUFFIX;
else
suffix = DUMB_SUFFIX;
-- 
2.15.0



[PATCH v4 2/2] launch_editor(): indicate that Git waits for user input

2017-11-29 Thread lars . schneider
From: Lars Schneider 

When a graphical GIT_EDITOR is spawned by a Git command that opens
and waits for user input (e.g. "git rebase -i"), then the editor window
might be obscured by other windows. The user may be left staring at the
original Git terminal window without even realizing that s/he needs to
interact with another window before Git can proceed. To this user Git
appears hanging.

Print a message that Git is waiting for editor input in the original
terminal and get rid of it when the editor returns.

No message is printed in a "dumb" terminal as it would not be possible
to remove the message after the editor returns. This should not be a
problem as this feature is targeted at novice Git users and they are
unlikely to work with a "dumb" terminal.

Power users might not want to see this message or their editor might
already print such a message (e.g. emacsclient). Allow these users to
suppress the message by disabling the "advice.waitingForEditor" config.

The standard advise() function is not used here as it would always add
a newline which would make deleting the message harder.

Helped-by: Junio C Hamano 
Signed-off-by: Lars Schneider 
---
 Documentation/config.txt |  3 +++
 advice.c |  2 ++
 advice.h |  1 +
 editor.c | 15 +++
 4 files changed, 21 insertions(+)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 671fcbaa0f..de64201e82 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -354,6 +354,9 @@ advice.*::
ignoredHook::
Advice shown if an hook is ignored because the hook is not
set as executable.
+   waitingForEditor::
+   Print a message to the terminal whenever Git is waiting for
+   editor input from the user.
 --
 
 core.fileMode::
diff --git a/advice.c b/advice.c
index c6169bcb52..406efc183b 100644
--- a/advice.c
+++ b/advice.c
@@ -18,6 +18,7 @@ int advice_object_name_warning = 1;
 int advice_rm_hints = 1;
 int advice_add_embedded_repo = 1;
 int advice_ignored_hook = 1;
+int advice_waiting_for_editor = 1;
 
 static struct {
const char *name;
@@ -40,6 +41,7 @@ static struct {
{ "rmhints", &advice_rm_hints },
{ "addembeddedrepo", &advice_add_embedded_repo },
{ "ignoredhook", &advice_ignored_hook },
+   { "waitingforeditor", &advice_waiting_for_editor },
 
/* make this an alias for backward compatibility */
{ "pushnonfastforward", &advice_push_update_rejected }
diff --git a/advice.h b/advice.h
index f525d6f89c..70568fa792 100644
--- a/advice.h
+++ b/advice.h
@@ -20,6 +20,7 @@ extern int advice_object_name_warning;
 extern int advice_rm_hints;
 extern int advice_add_embedded_repo;
 extern int advice_ignored_hook;
+extern int advice_waiting_for_editor;
 
 int git_default_advice_config(const char *var, const char *value);
 __attribute__((format (printf, 1, 2)))
diff --git a/editor.c b/editor.c
index c65ea698eb..cdad4f74ec 100644
--- a/editor.c
+++ b/editor.c
@@ -45,6 +45,13 @@ int launch_editor(const char *path, struct strbuf *buffer, 
const char *const *en
const char *args[] = { editor, real_path(path), NULL };
struct child_process p = CHILD_PROCESS_INIT;
int ret, sig;
+   int print_waiting_for_editor = advice_waiting_for_editor &&
+   isatty(2) && !is_terminal_dumb();
+
+   if (print_waiting_for_editor) {
+   fprintf(stderr, _("hint: Waiting for your editor 
input..."));
+   fflush(stderr);
+   }
 
p.argv = args;
p.env = env;
@@ -63,6 +70,14 @@ int launch_editor(const char *path, struct strbuf *buffer, 
const char *const *en
if (ret)
return error("There was a problem with the editor 
'%s'.",
editor);
+
+   if (print_waiting_for_editor)
+   /*
+* go back to the beginning and erase the
+* entire line to avoid wasting the vertical
+* space.
+*/
+   fputs("\r\033[K", stderr);
}
 
if (!buffer)
-- 
2.15.0



[PATCH v4 0/2] launch_editor(): indicate that Git waits for user input

2017-11-29 Thread lars . schneider
From: Lars Schneider 

Hi,

I simplified the code and enabled the "Git is waiting for editor input"
message only for "intelligent" terminals. The rational is, that today's
novice Git users are likely to have an "intelligent" terminal. People
working with "dumb" terminals have their reasons and are likely not the
target audience for this hint.

In addition, I added "advice.waitingForEditor" as an easy way to disable
the hint for experienced users.

I refactored the detection of dumb terminals in a preparation commit.

Thanks,
Lars


RFC: 
https://public-inbox.org/git/274b4850-2eb7-4bfa-a42c-25a573254...@gmail.com/
 v1: https://public-inbox.org/git/xmqqr2syvjxb@gitster.mtv.corp.google.com/
 v2: 
https://public-inbox.org/git/20171117135109.18071-1-lars.schnei...@autodesk.com/
 v3: 
https://public-inbox.org/git/20171127134716.69471-1-lars.schnei...@autodesk.com/

Base Ref: master
Web-Diff: https://github.com/larsxschneider/git/commit/9639e931bf
Checkout: git fetch https://github.com/larsxschneider/git editor-v4 && git 
checkout 9639e931bf


### Interdiff (v3..v4):

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 671fcbaa0f..de64201e82 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -354,6 +354,9 @@ advice.*::
ignoredHook::
Advice shown if an hook is ignored because the hook is not
set as executable.
+   waitingForEditor::
+   Print a message to the terminal whenever Git is waiting for
+   editor input from the user.
 --

 core.fileMode::
diff --git a/advice.c b/advice.c
index c6169bcb52..406efc183b 100644
--- a/advice.c
+++ b/advice.c
@@ -18,6 +18,7 @@ int advice_object_name_warning = 1;
 int advice_rm_hints = 1;
 int advice_add_embedded_repo = 1;
 int advice_ignored_hook = 1;
+int advice_waiting_for_editor = 1;

 static struct {
const char *name;
@@ -40,6 +41,7 @@ static struct {
{ "rmhints", &advice_rm_hints },
{ "addembeddedrepo", &advice_add_embedded_repo },
{ "ignoredhook", &advice_ignored_hook },
+   { "waitingforeditor", &advice_waiting_for_editor },

/* make this an alias for backward compatibility */
{ "pushnonfastforward", &advice_push_update_rejected }
diff --git a/advice.h b/advice.h
index f525d6f89c..70568fa792 100644
--- a/advice.h
+++ b/advice.h
@@ -20,6 +20,7 @@ extern int advice_object_name_warning;
 extern int advice_rm_hints;
 extern int advice_add_embedded_repo;
 extern int advice_ignored_hook;
+extern int advice_waiting_for_editor;

 int git_default_advice_config(const char *var, const char *value);
 __attribute__((format (printf, 1, 2)))
diff --git a/cache.h b/cache.h
index 89f5d24579..3842fc097c 100644
--- a/cache.h
+++ b/cache.h
@@ -1469,6 +1469,7 @@ extern const char *ident_default_name(void);
 extern const char *ident_default_email(void);
 extern const char *git_editor(void);
 extern const char *git_pager(int stdout_is_tty);
+extern int is_terminal_dumb(void);
 extern int git_ident_config(const char *, const char *, void *);
 extern void reset_ident_date(void);

diff --git a/color.c b/color.c
index 9a9261ac16..d48dd947c9 100644
--- a/color.c
+++ b/color.c
@@ -329,8 +329,7 @@ static int check_auto_color(void)
if (color_stdout_is_tty < 0)
color_stdout_is_tty = isatty(1);
if (color_stdout_is_tty || (pager_in_use() && pager_use_color)) {
-   char *term = getenv("TERM");
-   if (term && strcmp(term, "dumb"))
+   if (!is_terminal_dumb())
return 1;
}
return 0;
diff --git a/editor.c b/editor.c
index 4251ae9d7a..cdad4f74ec 100644
--- a/editor.c
+++ b/editor.c
@@ -7,11 +7,16 @@
 #define DEFAULT_EDITOR "vi"
 #endif

+int is_terminal_dumb(void)
+{
+   const char *terminal = getenv("TERM");
+   return !terminal || !strcmp(terminal, "dumb");
+}
+
 const char *git_editor(void)
 {
const char *editor = getenv("GIT_EDITOR");
-   const char *terminal = getenv("TERM");
-   int terminal_is_dumb = !terminal || !strcmp(terminal, "dumb");
+   int terminal_is_dumb = is_terminal_dumb();

if (!editor && editor_program)
editor = editor_program;
@@ -40,33 +45,11 @@ int launch_editor(const char *path, struct strbuf *buffer, 
const char *const *en
const char *args[] = { editor, real_path(path), NULL };
struct child_process p = CHILD_PROCESS_INIT;
int ret, sig;
-   static const char *close_notice = NULL;
-
-   if (isatty(2) && !close_notice) {
-   char *term = getenv("TERM");
-
-   if (term && strcmp(term, "dumb"))
-   /*
-* go back to the beginning and erase the
-* entire line if the terminal is capable
-* to do so, to avoid wasting the vertical
-

Re: [PATCH v5 5/6] rev-list: add list-objects filtering support

2017-11-29 Thread Jeff Hostetler



On 11/22/2017 3:08 PM, Jonathan Nieder wrote:

Hi,

Jeff Hostetler wrote:


Teach rev-list to use the filtering provided by the
traverse_commit_list_filtered() interface to omit
unwanted objects from the result.

Object filtering is only allowed when one of the "--objects*"
options are used.


micronit: the line widths seem to be uneven in these commit messages,
which is a bit distracting when reading.


ok




When the "--filter-print-omitted" option is used, the omitted
objects are printed at the end.  These are marked with a "~".
This option can be combined with "--quiet" to get a list of
just the omitted objects.


Neat.  Can you give a quick example?


I can put part of this in the V6 version, but here is a quick example.

$ git rev-list --objects HEAD
0629eb0173b5de0fb664e24bd4292988b2af1290
b2eaab8088df7ddad7f134e7b5144892b9646749
21b9d3ce3550213ac7de1391a518ab83f16912a2
3760f5a324c290c4f3518f087e2bdedd9d8f9ce4 large.1000
b3b5c5d37a767d73101e5a27393af2682bda4fd7 large.1
472cd9f4ed2a4804bcc2a2865e1a3ae00742eb01

$ git rev-list --objects HEAD --filter=blob:none --filter-print-omitted

0629eb0173b5de0fb664e24bd4292988b2af1290
b2eaab8088df7ddad7f134e7b5144892b9646749
21b9d3ce3550213ac7de1391a518ab83f16912a2
472cd9f4ed2a4804bcc2a2865e1a3ae00742eb01
~3760f5a324c290c4f3518f087e2bdedd9d8f9ce4
~b3b5c5d37a767d73101e5a27393af2682bda4fd7

$ git rev-list --objects HEAD --filter=blob:limit=5000 --filter-print-omitted

0629eb0173b5de0fb664e24bd4292988b2af1290
b2eaab8088df7ddad7f134e7b5144892b9646749
21b9d3ce3550213ac7de1391a518ab83f16912a2
3760f5a324c290c4f3518f087e2bdedd9d8f9ce4 large.1000
472cd9f4ed2a4804bcc2a2865e1a3ae00742eb01
~b3b5c5d37a767d73101e5a27393af2682bda4fd7

$ git rev-list --objects HEAD --filter=blob:limit=5000 --filter-print-omitted --quiet

~b3b5c5d37a767d73101e5a27393af2682bda4fd7




Using --quiet for this feels a bit odd, since it previously meant
to print nothing to stdout.  I wonder if there's another way ---
e.g.

--print-omitted=(yes|no|only)


Yeah, now that you say that it does seem a little odd.  I could
go either way here.  My expected usage was to be able to do a
commits-and-trees fetch and then do before doing a checkout, do
a narrow fetch of just the blobs in the tip that were omitted
when the commit and trees were received.



If I wanted to list all objects matching a filter, even objects
that are not reachable from any ref, is there a way to do that?
(Just curious, trying to think about this interface.)


I'm building on rev-list, so you can give it one or more refs or
revision ranges and it will traverse all of them and print any OIDs
it finds during the traversal.

I don't know of any way to visit unreachable objects, without using
something like GC.




Add t6112 test.


This part doesn't need to be in the commit message.  More generally,
anything I could more easily learn from the code or diffstat doesn't
need to be in the commit message: the commit message is about the
"why" more than the details of what in the code changed.



ok


In the future, we will introduce a "partial clone" mechanism
wherein an object in a repo, obtained from a remote, may
reference a missing object that can be dynamically fetched from
that remote once needed.  This "partial clone" mechanism will
have a way, sometimes slow, of determining if a missing link
is one of the links expected to be produced by this mechanism.


Does this mean the s will be part of the wire protocol?
I'll look more carefully at them below with that in mind.


yes.  a later commit will send the given  to the
server.  the idea is that clone and fetch will accept a filter
argument, pass that to the server, and have upload-pack/pack-objects
generate an incomplete packfile that omits unwanted objects.

so you could do something like:

$ git clone --no-checkout --filter=blob:none URL .

then either [1] explicitly request the omitted blobs/objects for a
tip commit or (in the case of a sparse-checkout) just the blobs
for a portion of a tip commit and then do a normal checkout.
or [2] rely on the (future) fetch-objects code to dynamically
fetch the required omitted blobs during a checkout.

Or if you know you (probably) never want gigantic blobs, do
something like:

$ git clone --filter=blob:limit=1m URL .

which would give you a mostly complete view repo, but just exclude
files > 1MB.  Those blobs could then be demand loaded as needed
later.





This patch introduces handling of missing objects to help
debugging and development of the "partial clone" mechanism,
and once the mechanism is implemented, for a power user to
perform operations that are missing-object aware without
incurring the cost of checking if a missing link is expected.


I had trouble understanding what this paragraph is about.  Can you
give an example?


The concept of filtering during a clone or fetch leads to
intentionally omitted object

[RFC/PATCH] Makefile: replace the overly complex perl build system with something simple

2017-11-29 Thread Ævar Arnfjörð Bjarmason
As the history of perl/Makefile.PL reveals the reason we have it in
the first place is because during its initial development the perl
bindings would link to libgit, building such a module with any sanity
requires ExtUtils::MakeMaker and Devel::PPPort.

However, since then this grew into a much simpler problem while
keeping all the complexity of the initial implementation. We just need
to take the files in perl/**.pm, copy them to a build directory while
search/replacing one string (@@LOCALEDIR@@), and have them available
for the likes of git-svn once Git is installed.

This patch is a WIP effort to do that. Trade-offs this makes & other
caveats:

 * This will not always install into perl's idea of its global
   "installsitelib". This only potentially matters for packagers that
   need to expose Git.pm for non-git use, and as explained in the
   INSTALL file there's a trivial workaround.

 * The scripts themselves will 'use lib' the target directory, but if
   INSTLIBDIR is set it overrides it. It doesn't have to be this way,
   but my reading of 0386dd37b1 ("Makefile: add PERLLIB_EXTRA variable
   that adds to default perl path", 2013-11-15) is that this does the
   right thing, but I may be wrong.

 * We don't build the Git(3) Git::I18N(3) etc. man pages from the
   embedded perldoc. I suspect nobody really cares, these are mostly
   internal APIs, and if someone's developing against them they likely
   know enough to issue a "perldoc" against the installed file to get
   the same result.

   But this is a change in how Git is installed now on e.g. CentOS &
   Debian which carry these manpages. They could be added (via
   pod2man) if anyone really cares.

 * Currently the Error.pm fallback doesn't work. This is a TODO item,
   but I didn't have enough time to hack this up.

   I think that whole thing should be changed, we're checking an
   ancient Error.pm version that was released in 2006, we should just
   stop that, then we check *at build time* whether the library is
   there, instead we should call it/load it via e.g. Git::Error which
   would be a small wrapper that would either load Error.pm from the
   system, or use our copy in Git::Error_CPAN.

Signed-off-by: Ævar Arnfjörð Bjarmason 
---

So *long* CC list but a lot of people have touched this stuff over the
years. This obviously conflicts with Dan Jacques's patches in a big
way, but I think it's worth it to rebase them on top of this if
there's interest in this, and depending on what people think about the
caveats I've noted about this above.

 INSTALL   | 18 ++-
 Makefile  | 51 +
 perl/.gitignore   |  9 +-
 perl/Git/I18N.pm  |  2 +-
 perl/Makefile | 90 ---
 perl/Makefile.PL  | 62 ---
 t/perf/aggregate.perl |  2 +-
 t/test-lib.sh |  2 +-
 wrap-for-bin.sh   |  2 +-
 9 files changed, 45 insertions(+), 193 deletions(-)
 delete mode 100644 perl/Makefile
 delete mode 100644 perl/Makefile.PL

diff --git a/INSTALL b/INSTALL
index ffb071e9f0..e6266a9127 100644
--- a/INSTALL
+++ b/INSTALL
@@ -84,9 +84,25 @@ Issues of note:
 
GIT_EXEC_PATH=`pwd`
PATH=`pwd`:$PATH
-   GITPERLLIB=`pwd`/perl/blib/lib
+   GITPERLLIB=`pwd`/perl/build
export GIT_EXEC_PATH PATH GITPERLLIB
 
+ - By default (unless NO_PERL is provided) Git will ship various perl
+   scripts & libraries it needs. However, for simplicity it doesn't
+   use the ExtUtils::MakeMaker toolchain to decide where to place the
+   perl libraries. Depending on the system this can result in the perl
+   libraries not being where you'd like them if they're expected to be
+   used by things other than Git itself.
+
+   Manually supplying a perllibdir prefix should fix this, if this is
+   a problem you care about, e.g.:
+
+   prefix=/usr perllibdir=/usr/$(/usr/bin/perl -MConfig -wE 'my ($relsite) 
= $Config{installsitelib} =~ m[^\Q$Config{siteprefixexp}\E/(.+)]s; say 
$relsite')
+
+   Will result in e.g. perllibdir=/usr/share/perl/5.26.1 on Debian,
+   perllibdir=/usr/share/perl5 (which we'd use by default) on CentOS
+   etc.
+
  - Git is reasonably self-sufficient, but does depend on a few external
programs and libraries.  Git can be used without most of them by adding
the approriate "NO_=YesPlease" to the make command line or
diff --git a/Makefile b/Makefile
index e53750ca01..6d69a7486b 100644
--- a/Makefile
+++ b/Makefile
@@ -295,9 +295,6 @@ all::
 #
 # Define PERL_PATH to the path of your Perl binary (usually /usr/bin/perl).
 #
-# Define NO_PERL_MAKEMAKER if you cannot use Makefiles generated by perl's
-# MakeMaker (e.g. using ActiveState under Cygwin).
-#
 # Define NO_PERL if you do not want Perl scripts or libraries at all.
 #
 # Define PYTHON_PATH to the path of your Python binary (often /usr/bin/python
@@ -473,6 +470,7 @@ gitexecdir = libexec/git-core
 mergetoolsdir = $(gitexecdir)/merg

[PATCH v4 4/4] exec_cmd: RUNTIME_PREFIX on some POSIX systems

2017-11-29 Thread Dan Jacques
Enable Git to resolve its own binary location using a variety of
OS-specific and generic methods, including:

- procfs via "/proc/self/exe" (Linux)
- _NSGetExecutablePath (Darwin)
- KERN_PROC_PATHNAME sysctl on BSDs.
- argv0, if absolute (all, including Windows).

This is used to enable RUNTIME_PREFIX support for non-Windows systems,
notably Linux and Darwin. When configured with RUNTIME_PREFIX, Git will
do a best-effort resolution of its executable path and automatically use
this as its "exec_path" for relative helper and data lookups, unless
explicitly overridden.

Small incidental formatting cleanup of "exec_cmd.c".

Signed-off-by: Dan Jacques 
Thanks-to: Robbie Iannucci 
Thanks-to: Junio C Hamano 
---
 Makefile |  35 +++-
 cache.h  |   1 +
 common-main.c|   4 +-
 config.mak.uname |   7 ++
 exec_cmd.c   | 239 +++
 exec_cmd.h   |   4 +-
 gettext.c|   8 +-
 git.c|   2 +-
 8 files changed, 260 insertions(+), 40 deletions(-)

diff --git a/Makefile b/Makefile
index 741d1583f..e663fda1f 100644
--- a/Makefile
+++ b/Makefile
@@ -416,6 +416,16 @@ all::
 #
 # Define HAVE_BSD_SYSCTL if your platform has a BSD-compatible sysctl function.
 #
+# Define HAVE_BSD_KERN_PROC_SYSCTL if your platform supports the KERN_PROC BSD
+# sysctl function.
+#
+# Define PROCFS_EXECUTABLE_PATH if your platform mounts a "procfs" filesystem
+# capable of resolving the path of the current executable. If defined, this
+# must be the canonical path for the "procfs" current executable path.
+#
+# Define HAVE_NS_GET_EXECUTABLE_PATH if your platform supports calling
+# _NSGetExecutablePath to retrieve the path of the running executable.
+#
 # Define HAVE_GETDELIM if your system has the getdelim() function.
 #
 # Define PAGER_ENV to a SP separated VAR=VAL pairs to define
@@ -426,6 +436,12 @@ all::
 # to say "export LESS=FRX (and LV=-c) if the environment variable
 # LESS (and LV) is not set, respectively".
 #
+# Define RUNTIME_PREFIX to configure Git to resolve its ancillary tooling and
+# support files relative to the location of the runtime binary, rather than
+# hard-coding them into the binary. Git installations built with RUNTIME_PREFIX
+# can be moved to arbitrary filesystem locations. Users may want to enable
+# RUNTIME_PREFIX_PERL as well (see below).
+#
 # Define RUNTIME_PREFIX_PERL to configure Git's PERL commands to locate Git
 # support libraries relative to their filesystem path instead of hard-coding
 # it.
@@ -466,6 +482,7 @@ ARFLAGS = rcs
 #   mandir
 #   infodir
 #   htmldir
+#   localedir
 #   perllibdir
 # This can help installing the suite in a relocatable way.
 
@@ -491,6 +508,7 @@ mandir_relative = $(patsubst $(prefix)/%,%,$(mandir))
 infodir_relative = $(patsubst $(prefix)/%,%,$(infodir))
 sharedir_relative = $(patsubst $(prefix)/%,%,$(sharedir))
 htmldir_relative = $(patsubst $(prefix)/%,%,$(htmldir))
+localedir_relative = $(patsubst $(prefix)/%,%,$(localedir))
 
 export prefix bindir sharedir sysconfdir gitwebdir localedir
 
@@ -1637,10 +1655,23 @@ ifdef HAVE_BSD_SYSCTL
BASIC_CFLAGS += -DHAVE_BSD_SYSCTL
 endif
 
+ifdef HAVE_BSD_KERN_PROC_SYSCTL
+   BASIC_CFLAGS += -DHAVE_BSD_KERN_PROC_SYSCTL
+endif
+
 ifdef HAVE_GETDELIM
BASIC_CFLAGS += -DHAVE_GETDELIM
 endif
 
+ifneq ($(PROCFS_EXECUTABLE_PATH),)
+   procfs_executable_path_SQ = $(subst ','\'',$(PROCFS_EXECUTABLE_PATH))
+   BASIC_CFLAGS += 
'-DPROCFS_EXECUTABLE_PATH="$(procfs_executable_path_SQ)"'
+endif
+
+ifdef HAVE_NS_GET_EXECUTABLE_PATH
+   BASIC_CFLAGS += -DHAVE_NS_GET_EXECUTABLE_PATH
+endif
+
 ifeq ($(TCLTK_PATH),)
 NO_TCLTK = NoThanks
 endif
@@ -1723,6 +1754,7 @@ bindir_relative_SQ = $(subst ','\'',$(bindir_relative))
 mandir_relative_SQ = $(subst ','\'',$(mandir_relative))
 infodir_relative_SQ = $(subst ','\'',$(infodir_relative))
 localedir_SQ = $(subst ','\'',$(localedir))
+localedir_relative_SQ = $(subst ','\'',$(localedir_relative))
 gitexecdir_SQ = $(subst ','\'',$(gitexecdir))
 template_dir_SQ = $(subst ','\'',$(template_dir))
 htmldir_relative_SQ = $(subst ','\'',$(htmldir_relative))
@@ -2185,6 +2217,7 @@ endif
 exec_cmd.sp exec_cmd.s exec_cmd.o: GIT-PREFIX
 exec_cmd.sp exec_cmd.s exec_cmd.o: EXTRA_CPPFLAGS = \
'-DGIT_EXEC_PATH="$(gitexecdir_SQ)"' \
+   '-DGIT_LOCALE_PATH="$(localedir_relative_SQ)"' \
'-DBINDIR="$(bindir_relative_SQ)"' \
'-DPREFIX="$(prefix_SQ)"'
 
@@ -2202,7 +2235,7 @@ attr.sp attr.s attr.o: EXTRA_CPPFLAGS = \
 
 gettext.sp gettext.s gettext.o: GIT-PREFIX
 gettext.sp gettext.s gettext.o: EXTRA_CPPFLAGS = \
-   -DGIT_LOCALE_PATH='"$(localedir_SQ)"'
+   -DGIT_LOCALE_PATH='"$(localedir_relative_SQ)"'
 
 http-push.sp http.sp http-walker.sp remote-curl.sp imap-send.sp: SPARSE_FLAGS 
+= \
-DCURL_DISABLE_TYPECHECK
diff --git a/cache.h b/cache.h
index 2e1434505..3d84aa0bf 100644
--- a/cache.h
+++ b/cache.h
@@ -449,6 +449,7 @@ static inline enum object_type object_type(

[PATCH v4 3/4] Makefile: add Perl runtime prefix support

2017-11-29 Thread Dan Jacques
Add a new Makefile flag, RUNTIME_PREFIX_PERL, which, when enabled,
configures Perl scripts to locate the Git installation's Perl support
libraries by resolving against the script's path, rather than
hard-coding that path at build-time.

Enabling RUNTIME_PREFIX_PERL overrides the system-specific Perl script
installation path generated by MakeMaker to force installation into a
platform-neutral location, "/share/perl5".

This change enables Git's Perl scripts to work when their Git installation
is relocated or moved to another system.

Signed-off-by: Dan Jacques 
Thanks-to: Ævar Arnfjörð Bjarmason 
Thanks-to: Johannes Schindelin 
---
 Makefile   | 40 +-
 perl/header_runtime_prefix.pl.template | 24 
 2 files changed, 63 insertions(+), 1 deletion(-)
 create mode 100644 perl/header_runtime_prefix.pl.template

diff --git a/Makefile b/Makefile
index 80904f8b0..741d1583f 100644
--- a/Makefile
+++ b/Makefile
@@ -425,6 +425,10 @@ all::
 #
 # to say "export LESS=FRX (and LV=-c) if the environment variable
 # LESS (and LV) is not set, respectively".
+#
+# Define RUNTIME_PREFIX_PERL to configure Git's PERL commands to locate Git
+# support libraries relative to their filesystem path instead of hard-coding
+# it.
 
 GIT-VERSION-FILE: FORCE
@$(SHELL_PATH) ./GIT-VERSION-GEN
@@ -485,6 +489,7 @@ pathsep = :
 
 mandir_relative = $(patsubst $(prefix)/%,%,$(mandir))
 infodir_relative = $(patsubst $(prefix)/%,%,$(infodir))
+sharedir_relative = $(patsubst $(prefix)/%,%,$(sharedir))
 htmldir_relative = $(patsubst $(prefix)/%,%,$(htmldir))
 
 export prefix bindir sharedir sysconfdir gitwebdir localedir
@@ -1967,7 +1972,38 @@ perl/PM.stamp: GIT-PERL-DEFINES FORCE
 PERL_HEADER_TEMPLATE = perl/header_fixed_prefix.pl.template
 
 PERL_DEFINES := $(PERL_PATH_SQ) $(PERLLIB_EXTRA_SQ)
-PERL_DEFINES += $(NO_PERL_MAKEMAKER)
+PERL_DEFINES += $(NO_PERL_MAKEMAKER) $(RUNTIME_PREFIX_PERL)
+
+# Support Perl runtime prefix. In this mode, a different header is installed
+# into Perl scripts. This header expects both the scripts and their support
+# library to be installed relative to $(prefix), and resolves the path to
+# the Perl libraries (perllibdir) from the executable's current path
+# (gitexecdir).
+#
+# This configuration requires both $(perllibdir) and $(gitexecdir) to be
+# relative paths, and will error if this is not the case.
+ifdef RUNTIME_PREFIX_PERL
+
+PERL_HEADER_TEMPLATE = perl/header_runtime_prefix.pl.template
+PERL_DEFINES += $(gitexecdir)
+
+# RUNTIME_PREFIX_PERL requires a $(perllibdir) value.
+ifeq ($(perllibdir),)
+perllibdir = $(sharedir_relative)/perl5
+endif
+
+ifneq ($(filter /%,$(firstword $(gitexecdir))),)
+$(error RUNTIME_PREFIX_PERL requires a relative gitexecdir, not: $(gitexecdir))
+endif
+gitexecdir_relative_SQ = $(gitexecdir_SQ)
+
+ifneq ($(filter /%,$(firstword $(perllibdir))),)
+$(error RUNTIME_PREFIX_PERL requires a relative perllibdir, not: $(perllibdir))
+endif
+perllibdir_relative_SQ = $(perllibdir_SQ)
+
+endif
+
 PERL_DEFINES += $(perllibdir)
 
 perl/perl.mak: GIT-CFLAGS GIT-PREFIX perl/Makefile perl/Makefile.PL
@@ -2001,6 +2037,8 @@ GIT-PERL-HEADER: $(PERL_HEADER_TEMPLATE) GIT-PERL-DEFINES 
perl/perl.mak Makefile
INSTLIBDIR="$$INSTLIBDIR$${INSTLIBDIR_EXTRA:+:$$INSTLIBDIR_EXTRA}" && \
sed -e 's=@@PATHSEP@@=$(pathsep)=g' \
-e 's=@@INSTLIBDIR@@='$$INSTLIBDIR'=g' \
+   -e 's=@@GITEXECDIR@@=$(gitexecdir_relative_SQ)=g' \
+   -e 's=@@PERLLIBDIR@@=$(perllibdir_relative_SQ)=g' \
$< >$@+ && \
mv $@+ $@
 
diff --git a/perl/header_runtime_prefix.pl.template 
b/perl/header_runtime_prefix.pl.template
new file mode 100644
index 0..fb9a9924d
--- /dev/null
+++ b/perl/header_runtime_prefix.pl.template
@@ -0,0 +1,24 @@
+# BEGIN RUNTIME_PREFIX_PERL generated code.
+#
+# This finds our Git::* libraries relative to the script's runtime path.
+BEGIN {
+   use lib split /@@PATHSEP@@/,
+   (
+   $ENV{GITPERLLIB}
+   ||
+   do {
+   require FindBin;
+   require File::Spec;
+   my $gitexecdir_relative = '@@GITEXECDIR@@';
+   my $perllibdir_relative = '@@PERLLIBDIR@@';
+
+   ($FindBin::Bin =~ m=${gitexecdir_relative}$=) ||
+   die('Unrecognized runtime path.');
+   my $prefix = substr($FindBin::Bin, 0, 
-length($gitexecdir_relative));
+   my $perllibdir = File::Spec->catdir($prefix, 
$perllibdir_relative);
+   (-e $perllibdir) || die("Invalid library path: 
$perllibdir");
+   $perllibdir;
+   }
+   );
+}
+# END RUNTIME_PREFIX_PERL generated code.
-- 
2.15.0.chromium12



[PATCH v4 0/4] RUNTIME_PREFIX relocatable Git

2017-11-29 Thread Dan Jacques
Previous threads:
v1: https://public-inbox.org/git/20171116170523.28696-1-...@google.com/
v2: https://public-inbox.org/git/20171119173141.4896-1-...@google.com/
v3: https://public-inbox.org/git/20171127164055.93283-1-...@google.com/

This is a small update to incorporate some Windows fixes from Johannes.
At this point, it passes the full test suite on Linux, Mac, and FreeBSD,
as well as the Travis.ci test suites, with and without
RUNTIME_PREFIX/RUNTIME_PREFIX_PERL flags.

I'm happy with the patch set, and feel that it is ready to move forward.
However, while it's been looked at by several people, and I have
incorporated reviewer feedback, the patch set hasn't received any formal
LGTM-style responses yet. I'm not sure what standard of review is required
to move forward with a patch on this project, so maybe this is totally
fine, but I wanted to make sure to point this out.

I also want to note Ævar Arnfjörð Bjarmason's RFC:
https://public-inbox.org/git/20171129153436.24471-1-ava...@gmail.com/T/

The proposed patch set conflicts with the Perl installation directory
changes in this patch set, as avarab@ notes. The proposed Perl installation
process would simplify patch 0002 in this patch set. I don't think the
landing order is terribly impactful - if this lands first, the patch in the
RFC would delete a few more lines, and if this lands later, patch 0002
would largely not be necessary.

Cheers,
-Dan

Built using this "config.mak" w/ autoconf:

=== BEGIN config.mak ===

RUNTIME_PREFIX = YesPlease
RUNTIME_PREFIX_PERL = YesPlease
gitexecdir = libexec/git-core
template_dir = share/git-core/templates
sysconfdir = etc

=== END config.mak ===

Changes in v4 from v3:

- Incorporated some quoting and Makefile dependency fixes, courtesy of
  .

Changes in v3 from v2:

- Broken into multiple patches now that Perl is isolated in its own
  RUNTIME_PREFIX_PERL flag.
- Working with avarab@, several changes to Perl script runtime prefix
  support:
  - Moved Perl header body content from Makefile into external template
file(s).
  - Added generic "perllibdir" variable to override Perl installation
path.
  - RUNTIME_PREFIX_PERL generated script header is more descriptive and
consistent with how the C version operates.
  - Fixed Generated Perl header Makefile dependency, should rebuild
when dependent files and flags change.
- Changed some of the new RUNTIME_PREFIX trace strings to use consistent
  formatting and terminology.

Changes in v2 from v1:

- Added comments and formatting to improve readability of
  platform-sepecific executable path resolution sleds in
  `git_get_exec_path`.
- Consolidated "cached_exec_path" and "argv_exec_path" globals
  into "exec_path_value".
- Use `strbuf_realpath` instead of `realpath` for procfs resolution.
- Removed new environment variable exports. Git with RUNTIME_PREFIX no
  longer exports or consumes any additional environment information.
- Updated Perl script resolution strategy: rather than having Git export
  the relative executable path to the Perl scripts, they now resolve
  it independently when RUNTIME_PREFIX_PERL is enabled.
- Updated resolution strategy for "gettext()": use system_path() instead
  of special environment variable.
- Added `sysctl` executable resolution support for BSDs that don't
  mount "procfs" by default (most of them).

Dan Jacques (4):
  Makefile: generate Perl header from template file
  Makefile: add support for "perllibdir"
  Makefile: add Perl runtime prefix support
  exec_cmd: RUNTIME_PREFIX on some POSIX systems

 .gitignore |   1 +
 Makefile   | 111 +--
 cache.h|   1 +
 common-main.c  |   4 +-
 config.mak.uname   |   7 +
 exec_cmd.c | 239 -
 exec_cmd.h |   4 +-
 gettext.c  |   8 +-
 git.c  |   2 +-
 perl/Makefile  |  52 ++-
 perl/header_fixed_prefix.pl.template   |   1 +
 perl/header_runtime_prefix.pl.template |  24 
 12 files changed, 396 insertions(+), 58 deletions(-)
 create mode 100644 perl/header_fixed_prefix.pl.template
 create mode 100644 perl/header_runtime_prefix.pl.template

-- 
2.15.0.chromium12



[PATCH v4 1/4] Makefile: generate Perl header from template file

2017-11-29 Thread Dan Jacques
Currently, the generated Perl script headers are emitted by commands in
the Makefile. This mechanism restricts options to introduce alternative
header content, needed by Perl runtime prefix support, and obscures the
origin of the Perl script header.

Change the Makefile to generate a header by processing a template file and
move the header content into the "perl/" subdirectory. The processed
generated will now be stored in the "GIT-PERL-HEADER" file. This allows
the content of the Perl header to be controlled by changing the path of
the template in the Makefile.

Signed-off-by: Dan Jacques 
Thanks-to: Ævar Arnfjörð Bjarmason 
Thanks-to: Johannes Schindelin 
---
 .gitignore   |  1 +
 Makefile | 24 +++-
 perl/header_fixed_prefix.pl.template |  1 +
 3 files changed, 17 insertions(+), 9 deletions(-)
 create mode 100644 perl/header_fixed_prefix.pl.template

diff --git a/.gitignore b/.gitignore
index 833ef3b0b..89bd7bd8a 100644
--- a/.gitignore
+++ b/.gitignore
@@ -3,6 +3,7 @@
 /GIT-LDFLAGS
 /GIT-PREFIX
 /GIT-PERL-DEFINES
+/GIT-PERL-HEADER
 /GIT-PYTHON-VARS
 /GIT-SCRIPT-DEFINES
 /GIT-USER-AGENT
diff --git a/Makefile b/Makefile
index e53750ca0..f7c4ac207 100644
--- a/Makefile
+++ b/Makefile
@@ -1964,18 +1964,15 @@ perl/PM.stamp: FORCE
 perl/perl.mak: GIT-CFLAGS GIT-PREFIX perl/Makefile perl/Makefile.PL
$(QUIET_SUBDIR0)perl $(QUIET_SUBDIR1) PERL_PATH='$(PERL_PATH_SQ)' 
prefix='$(prefix_SQ)' $(@F)
 
+PERL_HEADER_TEMPLATE = perl/header_fixed_prefix.pl.template
 PERL_DEFINES = $(PERL_PATH_SQ):$(PERLLIB_EXTRA_SQ)
-$(SCRIPT_PERL_GEN): % : %.perl perl/perl.mak GIT-PERL-DEFINES GIT-VERSION-FILE
+
+$(SCRIPT_PERL_GEN): % : %.perl perl/perl.mak GIT-PERL-DEFINES GIT-PERL-HEADER 
GIT-VERSION-FILE
$(QUIET_GEN)$(RM) $@ $@+ && \
-   INSTLIBDIR=`MAKEFLAGS= $(MAKE) -C perl -s --no-print-directory 
instlibdir` && \
-   INSTLIBDIR_EXTRA='$(PERLLIB_EXTRA_SQ)' && \
-   INSTLIBDIR="$$INSTLIBDIR$${INSTLIBDIR_EXTRA:+:$$INSTLIBDIR_EXTRA}" && \
sed -e '1{' \
-e 's|#!.*perl|#!$(PERL_PATH_SQ)|' \
-   -e 'h' \
-   -e 's=.*=use lib (split(/$(pathsep)/, $$ENV{GITPERLLIB} || 
"'"$$INSTLIBDIR"'"));=' \
-   -e 'H' \
-   -e 'x' \
+   -e 'rGIT-PERL-HEADER' \
+   -e 'G' \
-e '}' \
-e 's/@@GIT_VERSION@@/$(GIT_VERSION)/g' \
$< >$@+ && \
@@ -1989,6 +1986,15 @@ GIT-PERL-DEFINES: FORCE
echo "$$FLAGS" >$@; \
fi
 
+GIT-PERL-HEADER: $(PERL_HEADER_TEMPLATE) GIT-PERL-DEFINES perl/perl.mak 
Makefile
+   $(QUIET_GEN)$(RM) $@ && \
+   INSTLIBDIR=`MAKEFLAGS= $(MAKE) -C perl -s --no-print-directory 
instlibdir` && \
+   INSTLIBDIR_EXTRA='$(PERLLIB_EXTRA_SQ)' && \
+   INSTLIBDIR="$$INSTLIBDIR$${INSTLIBDIR_EXTRA:+:$$INSTLIBDIR_EXTRA}" && \
+   sed -e 's=@@PATHSEP@@=$(pathsep)=g' \
+   -e 's=@@INSTLIBDIR@@='$$INSTLIBDIR'=g' \
+   $< >$@+ && \
+   mv $@+ $@
 
 .PHONY: gitweb
 gitweb:
@@ -2707,7 +2713,7 @@ ifndef NO_TCLTK
 endif
$(RM) GIT-VERSION-FILE GIT-CFLAGS GIT-LDFLAGS GIT-BUILD-OPTIONS
$(RM) GIT-USER-AGENT GIT-PREFIX
-   $(RM) GIT-SCRIPT-DEFINES GIT-PERL-DEFINES GIT-PYTHON-VARS
+   $(RM) GIT-SCRIPT-DEFINES GIT-PERL-DEFINES GIT-PERL-HEADER 
GIT-PYTHON-VARS
 
 .PHONY: all install profile-clean clean strip
 .PHONY: shell_compatibility_test please_set_SHELL_PATH_to_a_more_modern_shell
diff --git a/perl/header_fixed_prefix.pl.template 
b/perl/header_fixed_prefix.pl.template
new file mode 100644
index 0..9a4bc4d30
--- /dev/null
+++ b/perl/header_fixed_prefix.pl.template
@@ -0,0 +1 @@
+use lib (split(/@@PATHSEP@@/, $ENV{GITPERLLIB} || "@@INSTLIBDIR@@"));
-- 
2.15.0.chromium12



[PATCH v4 2/4] Makefile: add support for "perllibdir"

2017-11-29 Thread Dan Jacques
Add the "perllibdir" Makefile variable, which allows the customization
of the Perl library installation path.

The Perl library installation path is currently left entirely to the
Perl Makefile implementation, either MakeMaker (default) or a fixed path
when NO_PERL_MAKEMAKER is enabled. This patch introduces "perllibdir", a
Makefile variable that can override that Perl module installation path.

As with some other Makefile variables, "perllibdir" may be either
absolute or relative. In the latter case, it is treated as relative to
"$(prefix)".

Add some incidental documentation to perl/Makefile.

Explicitly specifying an installation path is necessary for Perl runtime
prefix support, as runtime prefix resolution code must know in advance
where the Perl support modules are installed.

Signed-off-by: Dan Jacques 
---
 Makefile  | 18 +-
 perl/Makefile | 52 ++--
 2 files changed, 59 insertions(+), 11 deletions(-)

diff --git a/Makefile b/Makefile
index f7c4ac207..80904f8b0 100644
--- a/Makefile
+++ b/Makefile
@@ -462,6 +462,7 @@ ARFLAGS = rcs
 #   mandir
 #   infodir
 #   htmldir
+#   perllibdir
 # This can help installing the suite in a relocatable way.
 
 prefix = $(HOME)
@@ -1721,6 +1722,7 @@ gitexecdir_SQ = $(subst ','\'',$(gitexecdir))
 template_dir_SQ = $(subst ','\'',$(template_dir))
 htmldir_relative_SQ = $(subst ','\'',$(htmldir_relative))
 prefix_SQ = $(subst ','\'',$(prefix))
+perllibdir_SQ = $(subst ','\'',$(perllibdir))
 gitwebdir_SQ = $(subst ','\'',$(gitwebdir))
 
 SHELL_PATH_SQ = $(subst ','\'',$(SHELL_PATH))
@@ -1955,17 +1957,22 @@ $(SCRIPT_PERL_GEN): perl/perl.mak
 
 perl/perl.mak: perl/PM.stamp
 
-perl/PM.stamp: FORCE
+perl/PM.stamp: GIT-PERL-DEFINES FORCE
@$(FIND) perl -type f -name '*.pm' | sort >$@+ && \
+   cat GIT-PERL-DEFINES >>$@+ && \
$(PERL_PATH) -V >>$@+ && \
{ cmp $@+ $@ >/dev/null 2>/dev/null || mv $@+ $@; } && \
$(RM) $@+
 
-perl/perl.mak: GIT-CFLAGS GIT-PREFIX perl/Makefile perl/Makefile.PL
-   $(QUIET_SUBDIR0)perl $(QUIET_SUBDIR1) PERL_PATH='$(PERL_PATH_SQ)' 
prefix='$(prefix_SQ)' $(@F)
-
 PERL_HEADER_TEMPLATE = perl/header_fixed_prefix.pl.template
-PERL_DEFINES = $(PERL_PATH_SQ):$(PERLLIB_EXTRA_SQ)
+
+PERL_DEFINES := $(PERL_PATH_SQ) $(PERLLIB_EXTRA_SQ)
+PERL_DEFINES += $(NO_PERL_MAKEMAKER)
+PERL_DEFINES += $(perllibdir)
+
+perl/perl.mak: GIT-CFLAGS GIT-PREFIX perl/Makefile perl/Makefile.PL
+   $(QUIET_SUBDIR0)perl $(QUIET_SUBDIR1) PERL_PATH='$(PERL_PATH_SQ)' \
+ prefix='$(prefix_SQ)' perllibdir='$(perllibdir_SQ)' $(@F)
 
 $(SCRIPT_PERL_GEN): % : %.perl perl/perl.mak GIT-PERL-DEFINES GIT-PERL-HEADER 
GIT-VERSION-FILE
$(QUIET_GEN)$(RM) $@ $@+ && \
@@ -1979,6 +1986,7 @@ $(SCRIPT_PERL_GEN): % : %.perl perl/perl.mak 
GIT-PERL-DEFINES GIT-PERL-HEADER GI
chmod +x $@+ && \
mv $@+ $@
 
+PERL_DEFINES := $(subst $(space),:,$(PERL_DEFINES))
 GIT-PERL-DEFINES: FORCE
@FLAGS='$(PERL_DEFINES)'; \
if test x"$$FLAGS" != x"`cat $@ 2>/dev/null`" ; then \
diff --git a/perl/Makefile b/perl/Makefile
index f657de20e..b2aeeb0d8 100644
--- a/perl/Makefile
+++ b/perl/Makefile
@@ -1,6 +1,22 @@
 #
 # Makefile for perl support modules and routine
 #
+# This Makefile generates "perl.mak", which contains the actual build and
+# installation directions.
+#
+# PERL_PATH must be defined to be the path of the Perl interpreter to use.
+#
+# prefix must be defined as the Git installation prefix.
+#
+# localedir must be defined as the path to the locale data.
+#
+# perllibdir may be optionally defined to override the default Perl module
+# installation directory, which is relative to prefix. If perllibdir is not
+# absolute, it will be treated as relative to prefix.
+#
+# NO_PERL_MAKEMAKER may be defined to use a built-in Makefile generation method
+# instead of Perl MakeMaker.
+
 makfile:=perl.mak
 modules =
 
@@ -12,6 +28,16 @@ ifndef V
QUIET = @
 endif
 
+# If a library directory is provided, and it is not an absolute path, resolve
+# it relative to prefix.
+ifneq ($(perllibdir),)
+ifneq ($(filter /%,$(firstword $(perllibdir))),)
+perllib_instdir = $(perllibdir)
+else
+perllib_instdir = $(prefix)/$(perllibdir)
+endif
+endif
+
 all install instlibdir: $(makfile)
$(QUIET)$(MAKE) -f $(makfile) $@
 
@@ -25,7 +51,12 @@ clean:
 $(makfile): PM.stamp
 
 ifdef NO_PERL_MAKEMAKER
-instdir_SQ = $(subst ','\'',$(prefix)/lib)
+
+ifeq ($(perllib_instdir),)
+perllib_instdir = $(prefix)/lib
+endif
+
+instdir_SQ = $(subst ','\'',$(perllib_instdir))
 
 modules += Git
 modules += Git/I18N
@@ -42,7 +73,7 @@ modules += Git/SVN/Prompt
 modules += Git/SVN/Ra
 modules += Git/SVN/Utils
 
-$(makfile): ../GIT-CFLAGS Makefile
+$(makfile): ../GIT-CFLAGS ../GIT-PERL-DEFINES Makefile
echo all: private-Error.pm Git.pm Git/I18N.pm > $@
set -e; \
for i in $(modules); \
@@ -79,12 +110,21 @@ $(makfile): ../GIT-CFLAGS Makefile
echo '  cp pri

imap-send with gmail: curl_easy_perform() failed: URL using bad/illegal format or missing URL

2017-11-29 Thread Doron Behar
Hi,

I'm trying to send a patch with the command `git imap-send`, I used the
examples in the manual page as the main reference for my configuration:

```
[imap]
folder = "[Gmail]/Drafts"
host = imaps://imap.gmail.com
user = doron.be...@gmail.com
port = 993
sslverify = false
```

This is my `cat patch.out | git imap-send` output:

```
Password for 'imaps://doron.be...@gmail.com@imap.gmail.com':
sending 3 messages
curl_easy_perform() failed: URL using bad/illegal format or missing URL
```

The URI doesn't seem OK to me, I tried using `imap.user = doron.behar` and the
URI was `imaps://doron.be...@imap.gmail.com` but that ended up with the same
error as in the previous case.

I would love to get some help here, a Google Search didn't help as well.

Thanks.


Re: [PATCH] repository: fix a sparse 'using integer as NULL pointer' warning

2017-11-29 Thread Ramsay Jones


On 29/11/17 01:35, brian m. carlson wrote:
> On Tue, Nov 28, 2017 at 03:01:19AM +, Ramsay Jones wrote:
>>
>> Commit 78a6766802 ("Integrate hash algorithm support with repo setup",
>> 2017-11-12) added a 'const struct git_hash_algo *hash_algo' field to the
>> repository structure, without modifying the initializer of the 'the_repo'
>> variable. This does not actually introduce a bug, since the '0' initializer
>> for the 'ignore_env:1' bit-field is interpreted as a NULL pointer (hence
>> the warning), and the final field (now with no initializer) receives a
>> default '0'.
>>
>> Signed-off-by: Ramsay Jones 
>> ---
>>
>> Hi Junio,
>>
>> I don't recall Brian doing a re-roll of the 'bc/hash-algo' branch[1], but
>> now that it has been merged into the 'next' branch, sparse is barking on
>> that branch too. This patch (slightly different to the last one) applies
>> on top of 'next'.
> 
> Thanks for the patch; it's obviously correct.  I'll get sparse set up
> for future patch series.

Heh, at this point I usually remark that you would need to build
sparse from source, since all the packaged versions are so old
as to be virtually useless (to run over git).

That may still be the case, depending on your distro, since there
was a new release a couple of months ago. However, you know how
long it takes to get into some distros! ;-)

[I _think_ it may be in Debian Unstable]

Oh, the release (v0.5.1) was tagged on 17 Aug 2017.
(I'm currently running v0.5.1-30-gf1e4ba1, built from source).

ATB,
Ramsay Jones



Git filter clean not working when staging files

2017-11-29 Thread Rafał Treffler
Hello,

I've got a question or maybe bug about Git clean filter.
According to documentation[1] clean should be run when staging files.
When I'm staging file I see my script was run (I'm logging execution
into /tmp), but file content is unmodified.
My script is reading stdin and writing to stdout.
Nothing breaks in the script, from logs I see the source and modified
content are proper and I'm returning 0 return code.

Is this behavior intended? What should I do to modify the content?

Smudge works as intended.
But I've also noticed that git status runs clean filter but does not
modify contents.
Is that also intended?

Maybe I'm missing something?

If this is not going to work,
is there other way to modify file content on commit? (so the fixed
content goes to local repo)
Any other options ?

[1] https://git-scm.com/book/en/v2/Customizing-Git-Git-Attributes#filters_b

-- 
Thanks for help,
Rafal


[PATCH on en/rename-progress] diffcore-rename: make diff-tree -l0 mean -l

2017-11-29 Thread Jonathan Tan
In the documentation of diff-tree, it is stated that the -l option
"prevents rename/copy detection from running if the number of
rename/copy targets exceeds the specified number". The documentation
does not mention any special handling for the number 0, but the
implementation before commit b520abf ("sequencer: warn when internal
merge may be suboptimal due to renameLimit", 2017-11-14) treated 0 as a
special value indicating that the rename limit is to be a very large
number instead.

The commit b520abf changed that behavior, treating 0 as 0. Revert this
behavior to what it was previously. This allows existing scripts and
tools that use "-l0" to continue working. The alternative (to allow
"-l0") is probably much less useful, since users can just refrain from
specifying -M and/or -C to have the same effect.

Signed-off-by: Jonathan Tan 
---
Note that this patch is built on en/rename-progress.

We noticed this through an automated test for an internal tool - the
tool uses git diff-tree with -l0, and no longer produces the same
results as before.
---
 diffcore-rename.c  |  2 ++
 t/t4001-diff-rename.sh | 15 +++
 2 files changed, 17 insertions(+)

diff --git a/diffcore-rename.c b/diffcore-rename.c
index 9ca0eaec7..245e999fe 100644
--- a/diffcore-rename.c
+++ b/diffcore-rename.c
@@ -392,6 +392,8 @@ static int too_many_rename_candidates(int num_create,
 *
 *num_create * num_src > rename_limit * rename_limit
 */
+   if (rename_limit <= 0)
+   rename_limit = 32767;
if ((num_create <= rename_limit || num_src <= rename_limit) &&
((uint64_t)num_create * (uint64_t)num_src
 <= (uint64_t)rename_limit * (uint64_t)rename_limit))
diff --git a/t/t4001-diff-rename.sh b/t/t4001-diff-rename.sh
index 0d1fa45d2..eadf4f624 100755
--- a/t/t4001-diff-rename.sh
+++ b/t/t4001-diff-rename.sh
@@ -230,4 +230,19 @@ test_expect_success 'rename pretty print common prefix and 
suffix overlap' '
test_i18ngrep " d/f/{ => f}/e " output
 '
 
+test_expect_success 'diff-tree -l0 defaults to a big rename limit, not zero' '
+   test_write_lines line1 line2 line3 >myfile &&
+   git add myfile &&
+   git commit -m x &&
+
+   test_write_lines line1 line2 line4 >myotherfile &&
+   git rm myfile &&
+   git add myotherfile &&
+   git commit -m x &&
+
+   git diff-tree -M -l0 HEAD HEAD^ >actual &&
+   # Verify that a rename from myotherfile to myfile was detected
+   grep "myotherfile.*myfile" actual
+'
+
 test_done
-- 
2.15.0.173.g9268cf4a2.dirty



Re: [PATCH v4 0/2] launch_editor(): indicate that Git waits for user input

2017-11-29 Thread Thomas Adam
On Wed, Nov 29, 2017 at 03:37:50PM +0100, lars.schnei...@autodesk.com wrote:
> + if (print_waiting_for_editor) {
> + fprintf(stderr, _("hint: Waiting for your editor 
> input..."));
>   fflush(stderr);

Just FYI, stderr is typically unbuffered on most systems I've used, and
although the call to fflush() is harmless, I suspect it's not having any
effect.  That said, there's plenty of other places in Git which seems to think
fflush()ing stderr actually does something.

-- Thomas Adam


Re: [PATCH on en/rename-progress] diffcore-rename: make diff-tree -l0 mean -l

2017-11-29 Thread Elijah Newren
On Wed, Nov 29, 2017 at 10:32 AM, Jonathan Tan  wrote:
> In the documentation of diff-tree, it is stated that the -l option
> "prevents rename/copy detection from running if the number of
> rename/copy targets exceeds the specified number". The documentation
> does not mention any special handling for the number 0, but the
> implementation before commit b520abf ("sequencer: warn when internal
> merge may be suboptimal due to renameLimit", 2017-11-14) treated 0 as a
> special value indicating that the rename limit is to be a very large
> number instead.
>
> The commit b520abf changed that behavior, treating 0 as 0. Revert this
> behavior to what it was previously. This allows existing scripts and
> tools that use "-l0" to continue working. The alternative (to allow
> "-l0") is probably much less useful, since users can just refrain from
> specifying -M and/or -C to have the same effect.
>
> Signed-off-by: Jonathan Tan 
> ---
> Note that this patch is built on en/rename-progress.
>
> We noticed this through an automated test for an internal tool - the
> tool uses git diff-tree with -l0, and no longer produces the same
> results as before.

Thanks for testing that version and sending along the fix.

I suspect the commit referenced twice in the commit message should
have been 9f7e4bfa3b ("diff: remove silent clamp of renameLimit",
2017-11-13) rather than b520abf ("sequencer: warn when internal merge
may be suboptimal due to renameLimit", 2017-11-14).

Other than that minor issue, patch and test looks good to me.


Re: [PATCH on en/rename-progress] diffcore-rename: make diff-tree -l0 mean -l

2017-11-29 Thread Jonathan Tan
On Wed, 29 Nov 2017 10:51:20 -0800
Elijah Newren  wrote:

> Thanks for testing that version and sending along the fix.
> 
> I suspect the commit referenced twice in the commit message should
> have been 9f7e4bfa3b ("diff: remove silent clamp of renameLimit",
> 2017-11-13) rather than b520abf ("sequencer: warn when internal merge
> may be suboptimal due to renameLimit", 2017-11-14).

Ah...yes, you're right. I'll update the commit message if I need to send
out a new version or if Junio requests it (so that it's easier for him
to merge it in).

> Other than that minor issue, patch and test looks good to me.

Thanks.


Re: [SCRIPT/RFC 0/3] git-commit --onto-parent (three-way merge, no working tree file changes)

2017-11-29 Thread Johannes Sixt

Am 28.11.2017 um 02:15 schrieb Igor Djordjevic:

On 27/11/2017 22:54, Johannes Sixt wrote:


I my opinion, putting the focus on integration merge commits and the
desire to automate the re-merge step brings in a LOT of complexity in
the implementation for a very specific use-case that does not
necessarily help other cases.


It might seem more complex than it is, until you examine the guts to
see how it really works :)

Basic concept is pretty simple, as I actually don`t automate
anything, at least not in terms of what would manual user steps look
like - for example, there`s no real re-merge step in terms of
actually redoing the merge, I just reuse what was already there in a
very clean way, I would think (supported by my current, humble
knowledge, still).

The only merge that could possibly ever happen is upon committing
desired subset of changes onto parent, and that shouldn`t be too
complex by definition, otherwise that commit doesn`t really belong
there in the first place, if it can`t be meaningfully applied where
we want it (for now, at least).

That said, the whole operation of "posting on parent and re-merging
everything", the way it looks like from the outside, could end just
with a simple diff-apply-commit-commit internally, no merges at all.
Only if simple `git apply` fails, we try some trivial merging - and
all that inside separate (parent) index only, not touching original
HEAD index nor working tree, staying pristine for the whole process,
untouched.

Once done, you should be in the very same situation you started from,
nothing changed, just having your history tweaked a bit to tell a
different story on how you got there (now including a commit you
posted on your HEAD`s parent).


Ok, then please explain, how this process should work in my workflow and 
with the `commit --onto-parent` feature that you have in mind. I have an 
integration branch (which is a throw-away type, so you can mangle it in 
any way you want); it is a series of merges:


 ...A...C  <- topics A, C
 \   \
   ---o---o---o---o<- integration
 /   /
 ...B...D  <- topics B, D

Now I find a bug in topic B. Assume that the merges of C and D have 
textual conflicts with the integration branch (but not with B) and/or 
may be evil. What should I do?


With git-post, I make a fixup commit commit on the integration branch, 
then `git post B && git merge B`:


 ...A...C  <- topics A, C
 \   \
   ---o---o---o---o---f---F<- integration
 /   /   /
 ...B...D   /  <- topic D
 \ /
  f'--'<- topic B

The merge F does not introduce any changes on the integration branch, so 
I do not need it, but it helps keep topic B off radar when I ask `git 
branch --no-merged` later.



Don`t let "usual/preferred/recommended" Git workflow distract you too
much - one of the reasons I made this is because it also allows _kind
of_ "vanilla Git" patch queue, where you can quickly work on top of
the merge head, pushing commits onto parents below, being tips of
your "queues", putting you up to speed without a need to ever switch
a branch (hypothetically), until satisfied with what you have, where
you can slow down and polish each branch separately, as usual.

Like working on multiple branches at the same time, in the manner
similar to what `git add --patch` allows in regards to working on
multiple commits at the same time. This just takes it on yet another
level... hopefully :)


'kay, I'm not eagerly waiting for this particular next level (I prefer 
to keep things plain and simple), but I would never say this were a 
broken workflow. ;)



In your scenario above, it would certainly not be too bad if you
forgo the automatic merge and have the user issue a merge command
manually. The resulting history could look like this:

(3) o---o---A---X(topicA)
/ \   \
   /   M1--M2 (test, HEAD)
  /   /||
  ---o---o---M---' || (master)
  \   \   / |
   \   o-B /  (topicB)
\ /
 o---o---C(topicC)

I.e., commit --onto-parent A produced commit X, but M2 was then a
regular manual merge. (Of course, I am assuming that the merge
commits are dispensible, and only the resulting tree is of
interest.)


I see - and what you`re asking for is what I already envisioned and
hoped to get some more feedback about, here`s excerpt from
[SCRIPT/RFC 3/3] git-commit--onto-parent.sh[1] (I guess you didn`t
have time to checked that one yet?):


I did have a brief look, but I stopped when I saw

# Remove entry from HEAD reflog, not to pollute it with
# uninteresting in-between steps we take, leaking implementation
# details to end user.

It's a clear sign for me that's something wrong. It is not just reflogs 
that can become stale, but all operations that follow the `git commit` 
can fa

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

2017-11-29 Thread Ævar Arnfjörð Bjarmason
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.

We've since since started maintaining a fallback perl/Makefile, as
MakeMaker wouldn't work on some platforms[4]. That's just the tip of
the iceberg. We have the PM.stamp hack in the top-level Makefile[5] to
detect whether we need to regenerate the perl/perl.mak, which I fixed
just recently to deal with issues like the perl version changing from
under us[6].

There is absolutely no reason for why this needs to be so complex
anymore. All we're getting out of this elaborate Rube Goldberg machine
was copying perl/* to perl/blib/* as we do a string-replacement on
the *.pm files to hardcode @@LOCALEDIR@@ in the source.

So replace the whole thing with something that's pretty much a copy of
how we generate po/build/**.mo from po/*.po, just with a small sed(1)
command instead of msgfmt. As that's being done rename the files
from *.pm to *.pmc just to indicate that they're genreated (see
"perldoc -f require").

While I'm at it, change the fallback for Error.pm from being something
where we'll ship our own Error.pm if one doesn't exist at build time
to one where we just use a Git::Error wrapper that'll always prefer
the system-wide Error.pm, only falling back to our own copy if it
really doesn't exist at runtime. It's now shipped as
Git::FromCPAN::Error, making it easy to add other modules to
Git::FromCPAN::* in the future if that's needed.

Functional changes:

 * This will not always install into perl's idea of its global
   "installsitelib". This only potentially matters for packagers that
   need to expose Git.pm for non-git use, and as explained in the
   INSTALL file there's a trivial workaround.

 * The scripts themselves will 'use lib' the target directory, but if
   INSTLIBDIR is set it overrides it. It doesn't have to be this way,
   it could be set in addition to INSTLIBDIR, but my reading of [7] is
   that this is the desired behavior.

 * We don't build the Git(3) Git::I18N(3) etc. man pages from the
   embedded perldoc. I suspect nobody really cares, these are mostly
   internal APIs, and if someone's developing against them they likely
   know enough to issue a "perldoc" against the installed file to get
   the same result.

   But this is a change in how Git is installed now on e.g. CentOS &
   Debian which carry these manpages. They could be added (via
   pod2man) if anyone really cares.

   I doubt they will. The reason these were built in the first place
   was as a side-effect of how ExtUtils::MakeMaker works.

1. 5e9637c629 ("i18n: add infrastructure for translating Git with
   gettext", 2011-11-18)

2. b1edc53d06 ("Introduce Git.pm (v4)", 2006-06-24)

3. 18b0fc1ce1 ("Git.pm: Kill Git.xs for now", 2006-09-23)

4. f848718a69 ("Make perl/ build procedure ActiveState friendly.",
   2006-12-04)

5. ee9be06770 ("perl: detect new files in MakeMaker builds",
   2012-07-27)

6. c59c4939c2 ("perl: regenerate perl.mak if perl -V changes",
   2017-03-29)

7. 0386dd37b1 ("Makefile: add PERLLIB_EXTRA variable that adds to
   default perl path", 2013-11-15)

Signed-off-by: Ævar Arnfjörð Bjarmason 
---

Here's the non-RFC version. I think this is ready to be applied,
fixing that issue with Error.pm was easier than I thought.

 INSTALL  | 17 -
 Makefile | 53 +++---
 contrib/examples/git-difftool.perl   |  2 +-
 git-send-email.perl  |  2 +-
 perl/.gitignore  |  9 +--
 perl/Git.pm  |  2 +-
 perl/Git/Error.pm| 47 +
 perl/{private-Error.pm => Git/FromCPAN/Error.pm} |  0
 perl/Git/I18N.pm |  2 +-
 perl/Makefile| 90 
 perl/Makefile.PL | 62 
 t/perf/aggregate.perl|  2 +-
 t/test-lib.sh|  2 +-
 wrap-for-bin.sh  |  2 +-
 14 files changed, 95 insertions(+), 197 deletions(-)
 create mode 100644 perl/Git/Error.pm
 rename perl/{private-Error.pm => Git/FromCPAN/Error.pm} (100%)
 delete mode 100644 perl/Makefile
 delete mode 100644 perl/Makefile.PL

diff --git a/INSTALL b/INSTALL
index ffb071e9f0..822ed16095 100644
--- a/INSTALL
+++ b/INSTALL
@@ -84,9 +84,24 @@ Issues of note:
 
GIT_EXEC_PATH=`pwd`
PATH=`pwd`:$PATH
-   GITPERLLIB=`pwd`/perl/blib/lib
+   GITPERLLIB=`pwd`/perl/build
export GIT_EXEC_

[PATCH v6 1/6] checkout: factor out functions to new lib file

2017-11-29 Thread Thomas Gummerer
Factor the functions out, so they can be re-used from other places.  In
particular these functions will be re-used in builtin/worktree.c to make
git worktree add dwim more.

While there add some docs to the function.

Signed-off-by: Thomas Gummerer 
---
 Makefile   |  1 +
 builtin/checkout.c | 41 +
 checkout.c | 43 +++
 checkout.h | 13 +
 4 files changed, 58 insertions(+), 40 deletions(-)
 create mode 100644 checkout.c
 create mode 100644 checkout.h

diff --git a/Makefile b/Makefile
index e53750ca01..a80a8fcca9 100644
--- a/Makefile
+++ b/Makefile
@@ -759,6 +759,7 @@ LIB_OBJS += branch.o
 LIB_OBJS += bulk-checkin.o
 LIB_OBJS += bundle.o
 LIB_OBJS += cache-tree.o
+LIB_OBJS += checkout.o
 LIB_OBJS += color.o
 LIB_OBJS += column.o
 LIB_OBJS += combine-diff.o
diff --git a/builtin/checkout.c b/builtin/checkout.c
index 7d8bcc3833..ad8f94044c 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -1,5 +1,6 @@
 #include "builtin.h"
 #include "config.h"
+#include "checkout.h"
 #include "lockfile.h"
 #include "parse-options.h"
 #include "refs.h"
@@ -872,46 +873,6 @@ static int git_checkout_config(const char *var, const char 
*value, void *cb)
return git_xmerge_config(var, value, NULL);
 }
 
-struct tracking_name_data {
-   /* const */ char *src_ref;
-   char *dst_ref;
-   struct object_id *dst_oid;
-   int unique;
-};
-
-static int check_tracking_name(struct remote *remote, void *cb_data)
-{
-   struct tracking_name_data *cb = cb_data;
-   struct refspec query;
-   memset(&query, 0, sizeof(struct refspec));
-   query.src = cb->src_ref;
-   if (remote_find_tracking(remote, &query) ||
-   get_oid(query.dst, cb->dst_oid)) {
-   free(query.dst);
-   return 0;
-   }
-   if (cb->dst_ref) {
-   free(query.dst);
-   cb->unique = 0;
-   return 0;
-   }
-   cb->dst_ref = query.dst;
-   return 0;
-}
-
-static const char *unique_tracking_name(const char *name, struct object_id 
*oid)
-{
-   struct tracking_name_data cb_data = { NULL, NULL, NULL, 1 };
-   cb_data.src_ref = xstrfmt("refs/heads/%s", name);
-   cb_data.dst_oid = oid;
-   for_each_remote(check_tracking_name, &cb_data);
-   free(cb_data.src_ref);
-   if (cb_data.unique)
-   return cb_data.dst_ref;
-   free(cb_data.dst_ref);
-   return NULL;
-}
-
 static int parse_branchname_arg(int argc, const char **argv,
int dwim_new_local_branch_ok,
struct branch_info *new,
diff --git a/checkout.c b/checkout.c
new file mode 100644
index 00..ac42630f74
--- /dev/null
+++ b/checkout.c
@@ -0,0 +1,43 @@
+#include "cache.h"
+#include "remote.h"
+#include "checkout.h"
+
+struct tracking_name_data {
+   /* const */ char *src_ref;
+   char *dst_ref;
+   struct object_id *dst_oid;
+   int unique;
+};
+
+static int check_tracking_name(struct remote *remote, void *cb_data)
+{
+   struct tracking_name_data *cb = cb_data;
+   struct refspec query;
+   memset(&query, 0, sizeof(struct refspec));
+   query.src = cb->src_ref;
+   if (remote_find_tracking(remote, &query) ||
+   get_oid(query.dst, cb->dst_oid)) {
+   free(query.dst);
+   return 0;
+   }
+   if (cb->dst_ref) {
+   free(query.dst);
+   cb->unique = 0;
+   return 0;
+   }
+   cb->dst_ref = query.dst;
+   return 0;
+}
+
+const char *unique_tracking_name(const char *name, struct object_id *oid)
+{
+   struct tracking_name_data cb_data = { NULL, NULL, NULL, 1 };
+   cb_data.src_ref = xstrfmt("refs/heads/%s", name);
+   cb_data.dst_oid = oid;
+   for_each_remote(check_tracking_name, &cb_data);
+   free(cb_data.src_ref);
+   if (cb_data.unique)
+   return cb_data.dst_ref;
+   free(cb_data.dst_ref);
+   return NULL;
+}
diff --git a/checkout.h b/checkout.h
new file mode 100644
index 00..9980711179
--- /dev/null
+++ b/checkout.h
@@ -0,0 +1,13 @@
+#ifndef CHECKOUT_H
+#define CHECKOUT_H
+
+#include "cache.h"
+
+/*
+ * Check if the branch name uniquely matches a branch name on a remote
+ * tracking branch.  Return the name of the remote if such a branch
+ * exists, NULL otherwise.
+ */
+extern const char *unique_tracking_name(const char *name, struct object_id 
*oid);
+
+#endif /* CHECKOUT_H */
-- 
2.15.0.426.gb06021eeb



[PATCH v6 0/6] make git worktree add dwim more

2017-11-29 Thread Thomas Gummerer
The previous rounds were at
https://public-inbox.org/git/20171112134305.3949-1-t.gumme...@gmail.com/,
https://public-inbox.org/git/20171118181103.28354-1-t.gumme...@gmail.com/,
https://public-inbox.org/git/20171118224706.13810-1-t.gumme...@gmail.com/,
https://public-inbox.org/git/2017113020.2780-1-t.gumme...@gmail.com/ and
https://public-inbox.org/git/20171126194356.16187-1-t.gumme...@gmail.com.

Thanks Junio for the review of the last round!

Changes since the last round:

- rephrased documentation and commit messaegs a bit use the
- established pattern and call git_config only once, instead of
  calling it multiple times.

Interdiff below:
diff --git a/Documentation/git-worktree.txt b/Documentation/git-worktree.txt
index fd841886ef..89ad0faecf 100644
--- a/Documentation/git-worktree.txt
+++ b/Documentation/git-worktree.txt
@@ -116,11 +116,11 @@ OPTIONS
in linkgit:git-read-tree[1].
 
 --[no-]guess-remote::
-   With `add`, instead of creating a new branch from HEAD when
-   `` is not given, if there exists a tracking branch
-   in exactly one remote matching the basename of the path, base
-   the new branch on the remote-tracking branch, and mark the
-   remote-tracking branch as "upstream" from the new branch.
+   With `worktree add `, withouth ``, instead
+   of creating a new branch from HEAD, if there exists a tracking
+   branch in exactly one remote matching the basename of `,
+   base the new branch on the remote-tracking branch, and mark
+   the remote-tracking branch as "upstream" from the new branch.
 +
 This can also be set up as the default behaviour by using the
 `worktree.guessRemote` config option.
diff --git a/builtin/worktree.c b/builtin/worktree.c
index 426aea8761..002a569a11 100644
--- a/builtin/worktree.c
+++ b/builtin/worktree.c
@@ -43,7 +43,7 @@ static int git_worktree_config(const char *var, const char 
*value, void *cb)
return 0;
}
 
-   return 0;
+   return git_default_config(var, value, cb);
 }
 
 static int prune_worktree(const char *id, struct strbuf *reason)
@@ -371,8 +371,6 @@ static int add(int ac, const char **av, const char *prefix)
OPT_END()
};
 
-   git_config(git_worktree_config, NULL);
-
memset(&opts, 0, sizeof(opts));
opts.checkout = 1;
ac = parse_options(ac, av, prefix, options, worktree_usage, 0);
@@ -603,7 +601,7 @@ int cmd_worktree(int ac, const char **av, const char 
*prefix)
OPT_END()
};
 
-   git_config(git_default_config, NULL);
+   git_config(git_worktree_config, NULL);
 
if (ac < 2)
usage_with_options(worktree_usage, options);

Thomas Gummerer (6):
  checkout: factor out functions to new lib file
  worktree: add can be created from any commit-ish
  worktree: add --[no-]track option to the add subcommand
  worktree: make add   dwim
  worktree: add --guess-remote flag to add subcommand
  add worktree.guessRemote config option

 Documentation/config.txt   |  10 
 Documentation/git-worktree.txt |  44 ++
 Makefile   |   1 +
 builtin/checkout.c |  41 +
 builtin/worktree.c |  46 ++-
 checkout.c |  43 ++
 checkout.h |  13 +
 t/t2025-worktree-add.sh| 130 +
 8 files changed, 277 insertions(+), 51 deletions(-)
 create mode 100644 checkout.c
 create mode 100644 checkout.h

-- 
2.15.0.426.gb06021eeb



[PATCH v6 5/6] worktree: add --guess-remote flag to add subcommand

2017-11-29 Thread Thomas Gummerer
Currently 'git worktree add ' creates a new branch named after the
basename of the , that matches the HEAD of whichever worktree we
were on when calling "git worktree add ".

It's sometimes useful to have 'git worktree add  behave more like
the dwim machinery in 'git checkout ', i.e. check if the new
branch name, derived from the basename of the , uniquely matches
the branch name of a remote-tracking branch, and if so check out that
branch and set the upstream to the remote-tracking branch.

Add a new --guess-remote option that enables exactly that behaviour.

Signed-off-by: Thomas Gummerer 
---
 Documentation/git-worktree.txt |  7 +++
 builtin/worktree.c | 10 ++
 t/t2025-worktree-add.sh| 29 +
 3 files changed, 46 insertions(+)

diff --git a/Documentation/git-worktree.txt b/Documentation/git-worktree.txt
index 3044d305a6..a4ffee5e08 100644
--- a/Documentation/git-worktree.txt
+++ b/Documentation/git-worktree.txt
@@ -115,6 +115,13 @@ OPTIONS
such as configuring sparse-checkout. See "Sparse checkout"
in linkgit:git-read-tree[1].
 
+--[no-]guess-remote::
+   With `worktree add `, withouth ``, instead
+   of creating a new branch from HEAD, if there exists a tracking
+   branch in exactly one remote matching the basename of `,
+   base the new branch on the remote-tracking branch, and mark
+   the remote-tracking branch as "upstream" from the new branch.
+
 --[no-]track::
When creating a new branch, if `` is a branch,
mark it as "upstream" from the new branch.  This is the
diff --git a/builtin/worktree.c b/builtin/worktree.c
index 7021d02585..15cb1600ee 100644
--- a/builtin/worktree.c
+++ b/builtin/worktree.c
@@ -343,6 +343,7 @@ static int add(int ac, const char **av, const char *prefix)
char *path;
const char *branch;
const char *opt_track = NULL;
+   int guess_remote = 0;
struct option options[] = {
OPT__FORCE(&opts.force, N_("checkout  even if already 
checked out in other worktree")),
OPT_STRING('b', NULL, &opts.new_branch, N_("branch"),
@@ -355,6 +356,8 @@ static int add(int ac, const char **av, const char *prefix)
OPT_PASSTHRU(0, "track", &opt_track, NULL,
 N_("set up tracking mode (see git-branch(1))"),
 PARSE_OPT_NOARG | PARSE_OPT_OPTARG),
+   OPT_BOOL(0, "guess-remote", &guess_remote,
+N_("try to match the new branch name with a 
remote-tracking branch")),
OPT_END()
};
 
@@ -389,6 +392,13 @@ static int add(int ac, const char **av, const char *prefix)
int n;
const char *s = worktree_basename(path, &n);
opts.new_branch = xstrndup(s, n);
+   if (guess_remote) {
+   struct object_id oid;
+   const char *remote =
+   unique_tracking_name(opts.new_branch, &oid);
+   if (remote)
+   branch = remote;
+   }
}
 
if (ac == 2 && !opts.new_branch && !opts.detach) {
diff --git a/t/t2025-worktree-add.sh b/t/t2025-worktree-add.sh
index 96ebc63d04..d25c774cb7 100755
--- a/t/t2025-worktree-add.sh
+++ b/t/t2025-worktree-add.sh
@@ -384,4 +384,33 @@ test_expect_success '"add"   dwims' '
)
 '
 
+test_expect_success 'git worktree add does not match remote' '
+   test_when_finished rm -rf repo_a repo_b foo &&
+   setup_remote_repo repo_a repo_b &&
+   (
+   cd repo_b &&
+   git worktree add ../foo
+   ) &&
+   (
+   cd foo &&
+   test_must_fail git config "branch.foo.remote" &&
+   test_must_fail git config "branch.foo.merge" &&
+   ! test_cmp_rev refs/remotes/repo_a/foo refs/heads/foo
+   )
+'
+
+test_expect_success 'git worktree add --guess-remote sets up tracking' '
+   test_when_finished rm -rf repo_a repo_b foo &&
+   setup_remote_repo repo_a repo_b &&
+   (
+   cd repo_b &&
+   git worktree add --guess-remote ../foo
+   ) &&
+   (
+   cd foo &&
+   test_branch_upstream foo repo_a foo &&
+   test_cmp_rev refs/remotes/repo_a/foo refs/heads/foo
+   )
+'
+
 test_done
-- 
2.15.0.426.gb06021eeb



[PATCH v6 6/6] add worktree.guessRemote config option

2017-11-29 Thread Thomas Gummerer
Some users might want to have the --guess-remote option introduced in
the previous commit on by default, so they don't have to type it out
every time they create a new worktree.

Add a config option worktree.guessRemote that allows users to configure
the default behaviour for themselves.

Signed-off-by: Thomas Gummerer 
---
 Documentation/config.txt   | 10 ++
 Documentation/git-worktree.txt |  3 +++
 builtin/worktree.c | 14 --
 t/t2025-worktree-add.sh| 31 +++
 4 files changed, 56 insertions(+), 2 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 5f65fa9234..4966d90ebb 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -3425,3 +3425,13 @@ web.browser::
Specify a web browser that may be used by some commands.
Currently only linkgit:git-instaweb[1] and linkgit:git-help[1]
may use it.
+
+worktree.guessRemote::
+   With `add`, if no branch argument, and neither of `-b` nor
+   `-B` nor `--detach` are given, the command defaults to
+   creating a new branch from HEAD.  If `worktree.guessRemote` is
+   set to true, `worktree add` tries to find a remote-tracking
+   branch whose name uniquely matches the new branch name.  If
+   such a branch exists, it is checked out and set as "upstream"
+   for the new branch.  If no such match can be found, it falls
+   back to creating a new branch from the current HEAD.
diff --git a/Documentation/git-worktree.txt b/Documentation/git-worktree.txt
index a4ffee5e08..89ad0faecf 100644
--- a/Documentation/git-worktree.txt
+++ b/Documentation/git-worktree.txt
@@ -121,6 +121,9 @@ OPTIONS
branch in exactly one remote matching the basename of `,
base the new branch on the remote-tracking branch, and mark
the remote-tracking branch as "upstream" from the new branch.
++
+This can also be set up as the default behaviour by using the
+`worktree.guessRemote` config option.
 
 --[no-]track::
When creating a new branch, if `` is a branch,
diff --git a/builtin/worktree.c b/builtin/worktree.c
index 15cb1600ee..002a569a11 100644
--- a/builtin/worktree.c
+++ b/builtin/worktree.c
@@ -33,8 +33,19 @@ struct add_opts {
 
 static int show_only;
 static int verbose;
+static int guess_remote;
 static timestamp_t expire;
 
+static int git_worktree_config(const char *var, const char *value, void *cb)
+{
+   if (!strcmp(var, "worktree.guessremote")) {
+   guess_remote = git_config_bool(var, value);
+   return 0;
+   }
+
+   return git_default_config(var, value, cb);
+}
+
 static int prune_worktree(const char *id, struct strbuf *reason)
 {
struct stat st;
@@ -343,7 +354,6 @@ static int add(int ac, const char **av, const char *prefix)
char *path;
const char *branch;
const char *opt_track = NULL;
-   int guess_remote = 0;
struct option options[] = {
OPT__FORCE(&opts.force, N_("checkout  even if already 
checked out in other worktree")),
OPT_STRING('b', NULL, &opts.new_branch, N_("branch"),
@@ -591,7 +601,7 @@ int cmd_worktree(int ac, const char **av, const char 
*prefix)
OPT_END()
};
 
-   git_config(git_default_config, NULL);
+   git_config(git_worktree_config, NULL);
 
if (ac < 2)
usage_with_options(worktree_usage, options);
diff --git a/t/t2025-worktree-add.sh b/t/t2025-worktree-add.sh
index d25c774cb7..6ce9b9c070 100755
--- a/t/t2025-worktree-add.sh
+++ b/t/t2025-worktree-add.sh
@@ -413,4 +413,35 @@ test_expect_success 'git worktree add --guess-remote sets 
up tracking' '
)
 '
 
+test_expect_success 'git worktree add with worktree.guessRemote sets up 
tracking' '
+   test_when_finished rm -rf repo_a repo_b foo &&
+   setup_remote_repo repo_a repo_b &&
+   (
+   cd repo_b &&
+   git config worktree.guessRemote true &&
+   git worktree add ../foo
+   ) &&
+   (
+   cd foo &&
+   test_branch_upstream foo repo_a foo &&
+   test_cmp_rev refs/remotes/repo_a/foo refs/heads/foo
+   )
+'
+
+test_expect_success 'git worktree --no-guess-remote option overrides config' '
+   test_when_finished rm -rf repo_a repo_b foo &&
+   setup_remote_repo repo_a repo_b &&
+   (
+   cd repo_b &&
+   git config worktree.guessRemote true &&
+   git worktree add --no-guess-remote ../foo
+   ) &&
+   (
+   cd foo &&
+   test_must_fail git config "branch.foo.remote" &&
+   test_must_fail git config "branch.foo.merge" &&
+   ! test_cmp_rev refs/remotes/repo_a/foo refs/heads/foo
+   )
+'
+
 test_done
-- 
2.15.0.426.gb06021eeb



[PATCH v6 4/6] worktree: make add dwim

2017-11-29 Thread Thomas Gummerer
Currently 'git worktree add  ', errors out when 'branch'
is not a local branch.  It has no additional dwim'ing features that one
might expect.

Make it behave more like 'git checkout ' when the branch doesn't
exist locally, but a remote tracking branch uniquely matches the desired
branch name, i.e. create a new branch from the remote tracking branch
and set the upstream to the remote tracking branch.

As 'git worktree add' currently just dies in this situation, there are
no backwards compatibility worries when introducing this feature.

Signed-off-by: Thomas Gummerer 
---
 Documentation/git-worktree.txt |  8 
 builtin/worktree.c | 16 
 t/t2025-worktree-add.sh| 19 +++
 3 files changed, 43 insertions(+)

diff --git a/Documentation/git-worktree.txt b/Documentation/git-worktree.txt
index 15e58b18f7..3044d305a6 100644
--- a/Documentation/git-worktree.txt
+++ b/Documentation/git-worktree.txt
@@ -52,6 +52,14 @@ is linked to the current repository, sharing everything 
except working
 directory specific files such as HEAD, index, etc. `-` may also be
 specified as ``; it is synonymous with `@{-1}`.
 +
+If  is a branch name (call it `` and is not found,
+and neither `-b` nor `-B` nor `--detach` are used, but there does
+exist a tracking branch in exactly one remote (call it ``)
+with a matching name, treat as equivalent to
+
+$ git worktree add --track -b   /
+
++
 If `` is omitted and neither `-b` nor `-B` nor `--detach` used,
 then, as a convenience, a new branch based at HEAD is created automatically,
 as if `-b $(basename )` was specified.
diff --git a/builtin/worktree.c b/builtin/worktree.c
index ea9678cac8..7021d02585 100644
--- a/builtin/worktree.c
+++ b/builtin/worktree.c
@@ -1,4 +1,5 @@
 #include "cache.h"
+#include "checkout.h"
 #include "config.h"
 #include "builtin.h"
 #include "dir.h"
@@ -390,6 +391,21 @@ static int add(int ac, const char **av, const char *prefix)
opts.new_branch = xstrndup(s, n);
}
 
+   if (ac == 2 && !opts.new_branch && !opts.detach) {
+   struct object_id oid;
+   struct commit *commit;
+   const char *remote;
+
+   commit = lookup_commit_reference_by_name(branch);
+   if (!commit) {
+   remote = unique_tracking_name(branch, &oid);
+   if (remote) {
+   opts.new_branch = branch;
+   branch = remote;
+   }
+   }
+   }
+
if (opts.new_branch) {
struct child_process cp = CHILD_PROCESS_INIT;
cp.git_cmd = 1;
diff --git a/t/t2025-worktree-add.sh b/t/t2025-worktree-add.sh
index 72e8b62927..96ebc63d04 100755
--- a/t/t2025-worktree-add.sh
+++ b/t/t2025-worktree-add.sh
@@ -365,4 +365,23 @@ test_expect_success '--no-track avoids setting up 
tracking' '
)
 '
 
+test_expect_success '"add"   fails' '
+   test_must_fail git worktree add foo non-existent
+'
+
+test_expect_success '"add"   dwims' '
+   test_when_finished rm -rf repo_upstream repo_dwim foo &&
+   setup_remote_repo repo_upstream repo_dwim &&
+   git init repo_dwim &&
+   (
+   cd repo_dwim &&
+   git worktree add ../foo foo
+   ) &&
+   (
+   cd foo &&
+   test_branch_upstream foo repo_upstream foo &&
+   test_cmp_rev refs/remotes/repo_upstream/foo refs/heads/foo
+   )
+'
+
 test_done
-- 
2.15.0.426.gb06021eeb



[PATCH v6 3/6] worktree: add --[no-]track option to the add subcommand

2017-11-29 Thread Thomas Gummerer
Currently 'git worktree add' sets up tracking branches if '' is
a remote tracking branch, and doesn't set them up otherwise, as is the
default for 'git branch'.

This may or may not be what the user wants.  Allow overriding this
behaviour with a --[no-]track flag that gets passed through to 'git
branch'.

We already respect branch.autoSetupMerge, as 'git worktree' just calls
'git branch' internally.

Signed-off-by: Thomas Gummerer 
---
 Documentation/git-worktree.txt |  6 +
 builtin/worktree.c |  8 +++
 t/t2025-worktree-add.sh| 51 ++
 3 files changed, 65 insertions(+)

diff --git a/Documentation/git-worktree.txt b/Documentation/git-worktree.txt
index 121895209d..15e58b18f7 100644
--- a/Documentation/git-worktree.txt
+++ b/Documentation/git-worktree.txt
@@ -107,6 +107,12 @@ OPTIONS
such as configuring sparse-checkout. See "Sparse checkout"
in linkgit:git-read-tree[1].
 
+--[no-]track::
+   When creating a new branch, if `` is a branch,
+   mark it as "upstream" from the new branch.  This is the
+   default if `` is a remote-tracking branch.  See
+   "--track" in linkgit:git-branch[1] for details.
+
 --lock::
Keep the working tree locked after creation. This is the
equivalent of `git worktree lock` after `git worktree add`,
diff --git a/builtin/worktree.c b/builtin/worktree.c
index ed043d5f1c..ea9678cac8 100644
--- a/builtin/worktree.c
+++ b/builtin/worktree.c
@@ -341,6 +341,7 @@ static int add(int ac, const char **av, const char *prefix)
const char *new_branch_force = NULL;
char *path;
const char *branch;
+   const char *opt_track = NULL;
struct option options[] = {
OPT__FORCE(&opts.force, N_("checkout  even if already 
checked out in other worktree")),
OPT_STRING('b', NULL, &opts.new_branch, N_("branch"),
@@ -350,6 +351,9 @@ static int add(int ac, const char **av, const char *prefix)
OPT_BOOL(0, "detach", &opts.detach, N_("detach HEAD at named 
commit")),
OPT_BOOL(0, "checkout", &opts.checkout, N_("populate the new 
working tree")),
OPT_BOOL(0, "lock", &opts.keep_locked, N_("keep the new working 
tree locked")),
+   OPT_PASSTHRU(0, "track", &opt_track, NULL,
+N_("set up tracking mode (see git-branch(1))"),
+PARSE_OPT_NOARG | PARSE_OPT_OPTARG),
OPT_END()
};
 
@@ -394,9 +398,13 @@ static int add(int ac, const char **av, const char *prefix)
argv_array_push(&cp.args, "--force");
argv_array_push(&cp.args, opts.new_branch);
argv_array_push(&cp.args, branch);
+   if (opt_track)
+   argv_array_push(&cp.args, opt_track);
if (run_command(&cp))
return -1;
branch = opts.new_branch;
+   } else if (opt_track) {
+   die(_("--[no-]track can only be used if a new branch is 
created"));
}
 
UNLEAK(path);
diff --git a/t/t2025-worktree-add.sh b/t/t2025-worktree-add.sh
index b5c47ac602..72e8b62927 100755
--- a/t/t2025-worktree-add.sh
+++ b/t/t2025-worktree-add.sh
@@ -313,5 +313,56 @@ test_expect_success 'checkout a branch under bisect' '
 test_expect_success 'rename a branch under bisect not allowed' '
test_must_fail git branch -M under-bisect bisect-with-new-name
 '
+# Is branch "refs/heads/$1" set to pull from "$2/$3"?
+test_branch_upstream () {
+   printf "%s\n" "$2" "refs/heads/$3" >expect.upstream &&
+   {
+   git config "branch.$1.remote" &&
+   git config "branch.$1.merge"
+   } >actual.upstream &&
+   test_cmp expect.upstream actual.upstream
+}
+
+test_expect_success '--track sets up tracking' '
+   test_when_finished rm -rf track &&
+   git worktree add --track -b track track master &&
+   test_branch_upstream track . master
+'
+
+# setup remote repository $1 and repository $2 with $1 set up as
+# remote.  The remote has two branches, master and foo.
+setup_remote_repo () {
+   git init $1 &&
+   (
+   cd $1 &&
+   test_commit $1_master &&
+   git checkout -b foo &&
+   test_commit upstream_foo
+   ) &&
+   git init $2 &&
+   (
+   cd $2 &&
+   test_commit $2_master &&
+   git remote add $1 ../$1 &&
+   git config remote.$1.fetch \
+   "refs/heads/*:refs/remotes/$1/*" &&
+   git fetch --all
+   )
+}
+
+test_expect_success '--no-track avoids setting up tracking' '
+   test_when_finished rm -rf repo_upstream repo_local foo &&
+   setup_remote_repo repo_upstream repo_local &&
+   (
+   cd repo_local &&
+   git worktree add --no-track -b foo ../foo repo_upstream/foo
+   ) &&
+   (

[PATCH v6 2/6] worktree: add can be created from any commit-ish

2017-11-29 Thread Thomas Gummerer
Currently 'git worktree add' is documented to take an optional 
argument, which is checked out in the new worktree.  However it is more
generally possible to use a commit-ish as the optional argument, and
check that out into the new worktree.

Document that this is a possibility, as new users of git worktree add
might find it helpful.

Reported-by: Randall S. Becker 
Signed-off-by: Thomas Gummerer 
---
 Documentation/git-worktree.txt | 20 ++--
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/Documentation/git-worktree.txt b/Documentation/git-worktree.txt
index b472acc356..121895209d 100644
--- a/Documentation/git-worktree.txt
+++ b/Documentation/git-worktree.txt
@@ -9,7 +9,7 @@ git-worktree - Manage multiple working trees
 SYNOPSIS
 
 [verse]
-'git worktree add' [-f] [--detach] [--checkout] [--lock] [-b ] 
 []
+'git worktree add' [-f] [--detach] [--checkout] [--lock] [-b ] 
 []
 'git worktree list' [--porcelain]
 'git worktree lock' [--reason ] 
 'git worktree prune' [-n] [-v] [--expire ]
@@ -45,14 +45,14 @@ specifying `--reason` to explain why the working tree is 
locked.
 
 COMMANDS
 
-add  []::
+add  []::
 
-Create `` and checkout `` into it. The new working directory
+Create `` and checkout `` into it. The new working directory
 is linked to the current repository, sharing everything except working
 directory specific files such as HEAD, index, etc. `-` may also be
-specified as ``; it is synonymous with `@{-1}`.
+specified as ``; it is synonymous with `@{-1}`.
 +
-If `` is omitted and neither `-b` nor `-B` nor `--detach` used,
+If `` is omitted and neither `-b` nor `-B` nor `--detach` used,
 then, as a convenience, a new branch based at HEAD is created automatically,
 as if `-b $(basename )` was specified.
 
@@ -84,25 +84,25 @@ OPTIONS
 
 -f::
 --force::
-   By default, `add` refuses to create a new working tree when ``
+   By default, `add` refuses to create a new working tree when 
`` is a branch name and
is already checked out by another working tree. This option overrides
that safeguard.
 
 -b ::
 -B ::
With `add`, create a new branch named `` starting at
-   ``, and check out `` into the new working tree.
-   If `` is omitted, it defaults to HEAD.
+   ``, and check out `` into the new working tree.
+   If `` is omitted, it defaults to HEAD.
By default, `-b` refuses to create a new branch if it already
exists. `-B` overrides this safeguard, resetting `` to
-   ``.
+   ``.
 
 --detach::
With `add`, detach HEAD in the new working tree. See "DETACHED HEAD"
in linkgit:git-checkout[1].
 
 --[no-]checkout::
-   By default, `add` checks out ``, however, `--no-checkout` can
+   By default, `add` checks out ``, however, `--no-checkout` 
can
be used to suppress checkout in order to make customizations,
such as configuring sparse-checkout. See "Sparse checkout"
in linkgit:git-read-tree[1].
-- 
2.15.0.426.gb06021eeb



Re: [PATCH on en/rename-progress] diffcore-rename: make diff-tree -l0 mean -l

2017-11-29 Thread Jonathan Nieder
Elijah Newren wrote:
> On Wed, Nov 29, 2017 at 10:32 AM, Jonathan Tan  
> wrote:

>> In the documentation of diff-tree, it is stated that the -l option
>> "prevents rename/copy detection from running if the number of
>> rename/copy targets exceeds the specified number". The documentation
>> does not mention any special handling for the number 0, but the
>> implementation before commit b520abf ("sequencer: warn when internal
>> merge may be suboptimal due to renameLimit", 2017-11-14) treated 0 as a
>> special value indicating that the rename limit is to be a very large
>> number instead.
>>
>> The commit b520abf changed that behavior, treating 0 as 0. Revert this
>> behavior to what it was previously. This allows existing scripts and
>> tools that use "-l0" to continue working. The alternative (to allow
>> "-l0") is probably much less useful, since users can just refrain from

I think in the parenthesis you mean 'to allow "-l0" to suppress rename
detection', since this patch is all about allowing '-l0' already.

>> specifying -M and/or -C to have the same effect.
>>
>> Signed-off-by: Jonathan Tan 
>> ---
>> Note that this patch is built on en/rename-progress.
>>
>> We noticed this through an automated test for an internal tool - the
>> tool uses git diff-tree with -l0, and no longer produces the same
>> results as before.
>
> Thanks for testing that version and sending along the fix.
>
> I suspect the commit referenced twice in the commit message should
> have been 9f7e4bfa3b ("diff: remove silent clamp of renameLimit",
> 2017-11-13) rather than b520abf ("sequencer: warn when internal merge
> may be suboptimal due to renameLimit", 2017-11-14).
>
> Other than that minor issue, patch and test looks good to me.

Thanks, both.  Looking at that patch, the fix is obviously correct.

With Elijah's commit message tweak,
Reviewed-by: Jonathan Nieder 


Re: [PATCH v4 2/4] Makefile: add support for "perllibdir"

2017-11-29 Thread Ævar Arnfjörð Bjarmason

On Wed, Nov 29 2017, Dan Jacques jotted:

> Add the "perllibdir" Makefile variable, which allows the customization
> of the Perl library installation path.
>
> The Perl library installation path is currently left entirely to the
> Perl Makefile implementation, either MakeMaker (default) or a fixed path
> when NO_PERL_MAKEMAKER is enabled. This patch introduces "perllibdir", a
> Makefile variable that can override that Perl module installation path.
>
> As with some other Makefile variables, "perllibdir" may be either
> absolute or relative. In the latter case, it is treated as relative to
> "$(prefix)".
>
> Add some incidental documentation to perl/Makefile.
>
> Explicitly specifying an installation path is necessary for Perl runtime
> prefix support, as runtime prefix resolution code must know in advance
> where the Perl support modules are installed.
>
> Signed-off-by: Dan Jacques 
> ---
>  Makefile  | 18 +-
>  perl/Makefile | 52 ++--
>  2 files changed, 59 insertions(+), 11 deletions(-)
>
> diff --git a/Makefile b/Makefile
> index f7c4ac207..80904f8b0 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -462,6 +462,7 @@ ARFLAGS = rcs
>  #   mandir
>  #   infodir
>  #   htmldir
> +#   perllibdir
>  # This can help installing the suite in a relocatable way.
>
>  prefix = $(HOME)
> @@ -1721,6 +1722,7 @@ gitexecdir_SQ = $(subst ','\'',$(gitexecdir))
>  template_dir_SQ = $(subst ','\'',$(template_dir))
>  htmldir_relative_SQ = $(subst ','\'',$(htmldir_relative))
>  prefix_SQ = $(subst ','\'',$(prefix))
> +perllibdir_SQ = $(subst ','\'',$(perllibdir))
>  gitwebdir_SQ = $(subst ','\'',$(gitwebdir))
>
>  SHELL_PATH_SQ = $(subst ','\'',$(SHELL_PATH))
> @@ -1955,17 +1957,22 @@ $(SCRIPT_PERL_GEN): perl/perl.mak
>
>  perl/perl.mak: perl/PM.stamp
>
> -perl/PM.stamp: FORCE
> +perl/PM.stamp: GIT-PERL-DEFINES FORCE
>   @$(FIND) perl -type f -name '*.pm' | sort >$@+ && \
> + cat GIT-PERL-DEFINES >>$@+ && \
>   $(PERL_PATH) -V >>$@+ && \
>   { cmp $@+ $@ >/dev/null 2>/dev/null || mv $@+ $@; } && \
>   $(RM) $@+
>
> -perl/perl.mak: GIT-CFLAGS GIT-PREFIX perl/Makefile perl/Makefile.PL
> - $(QUIET_SUBDIR0)perl $(QUIET_SUBDIR1) PERL_PATH='$(PERL_PATH_SQ)' 
> prefix='$(prefix_SQ)' $(@F)
> -
>  PERL_HEADER_TEMPLATE = perl/header_fixed_prefix.pl.template
> -PERL_DEFINES = $(PERL_PATH_SQ):$(PERLLIB_EXTRA_SQ)
> +
> +PERL_DEFINES := $(PERL_PATH_SQ) $(PERLLIB_EXTRA_SQ)
> +PERL_DEFINES += $(NO_PERL_MAKEMAKER)
> +PERL_DEFINES += $(perllibdir)
> +
> +perl/perl.mak: GIT-CFLAGS GIT-PREFIX perl/Makefile perl/Makefile.PL
> + $(QUIET_SUBDIR0)perl $(QUIET_SUBDIR1) PERL_PATH='$(PERL_PATH_SQ)' \
> +   prefix='$(prefix_SQ)' perllibdir='$(perllibdir_SQ)' $(@F)
>
>  $(SCRIPT_PERL_GEN): % : %.perl perl/perl.mak GIT-PERL-DEFINES 
> GIT-PERL-HEADER GIT-VERSION-FILE
>   $(QUIET_GEN)$(RM) $@ $@+ && \
> @@ -1979,6 +1986,7 @@ $(SCRIPT_PERL_GEN): % : %.perl perl/perl.mak 
> GIT-PERL-DEFINES GIT-PERL-HEADER GI
>   chmod +x $@+ && \
>   mv $@+ $@
>
> +PERL_DEFINES := $(subst $(space),:,$(PERL_DEFINES))
>  GIT-PERL-DEFINES: FORCE
>   @FLAGS='$(PERL_DEFINES)'; \
>   if test x"$$FLAGS" != x"`cat $@ 2>/dev/null`" ; then \
> diff --git a/perl/Makefile b/perl/Makefile
> index f657de20e..b2aeeb0d8 100644
> --- a/perl/Makefile
> +++ b/perl/Makefile
> @@ -1,6 +1,22 @@
>  #
>  # Makefile for perl support modules and routine
>  #
> +# This Makefile generates "perl.mak", which contains the actual build and
> +# installation directions.
> +#
> +# PERL_PATH must be defined to be the path of the Perl interpreter to use.
> +#
> +# prefix must be defined as the Git installation prefix.
> +#
> +# localedir must be defined as the path to the locale data.
> +#
> +# perllibdir may be optionally defined to override the default Perl module
> +# installation directory, which is relative to prefix. If perllibdir is not
> +# absolute, it will be treated as relative to prefix.
> +#
> +# NO_PERL_MAKEMAKER may be defined to use a built-in Makefile generation 
> method
> +# instead of Perl MakeMaker.
> +
>  makfile:=perl.mak
>  modules =
>
> @@ -12,6 +28,16 @@ ifndef V
>   QUIET = @
>  endif
>
> +# If a library directory is provided, and it is not an absolute path, resolve
> +# it relative to prefix.
> +ifneq ($(perllibdir),)
> +ifneq ($(filter /%,$(firstword $(perllibdir))),)
> +perllib_instdir = $(perllibdir)
> +else
> +perllib_instdir = $(prefix)/$(perllibdir)
> +endif
> +endif

Maybe I'm missing something, but isn't this "perllibdir can be relative"
aim orthagonal to what this patch series is about? I.e. we could just
avoid this work by saying you must say "prefix=/usr
perllibdir=/usr/perl" instead of "prefix=/usr perllibdir=perl" as this
allows.

I started going down this route with my own patch and just threw it
away.

>  all install instlibdir: $(makfile)
>   $(QUIET)$(MAKE) -f $(makfile) $@
>
> @@ -25,7 +51,12 @@ clean:
>  $(makfile): PM.stamp
>
>  ifde

[PATCH on en/rename-progress v2] diffcore-rename: make diff-tree -l0 mean -l

2017-11-29 Thread Jonathan Tan
In the documentation of diff-tree, it is stated that the -l option
"prevents rename/copy detection from running if the number of
rename/copy targets exceeds the specified number". The documentation
does not mention any special handling for the number 0, but the
implementation before commit 9f7e4bfa3b ("diff: remove silent clamp of
renameLimit", 2017-11-13) treated 0 as a special value indicating that
the rename limit is to be a very large number instead.

The commit 9f7e4bfa3b changed that behavior, treating 0 as 0. Revert
this behavior to what it was previously. This allows existing scripts
and tools that use "-l0" to continue working. The alternative (to have
"-l0" suppress rename detection) is probably much less useful, since
users can just refrain from specifying -M and/or -C to have the same
effect.

Signed-off-by: Jonathan Tan 
---
There are multiple issues with the commit message, so I am sending this
new version out.

v2 is exactly the same as previously, except that the commit message is
changed following Elijah Newren's and Jonathan Nieder's comments.
---
 diffcore-rename.c  |  2 ++
 t/t4001-diff-rename.sh | 15 +++
 2 files changed, 17 insertions(+)

diff --git a/diffcore-rename.c b/diffcore-rename.c
index 9ca0eaec7..245e999fe 100644
--- a/diffcore-rename.c
+++ b/diffcore-rename.c
@@ -392,6 +392,8 @@ static int too_many_rename_candidates(int num_create,
 *
 *num_create * num_src > rename_limit * rename_limit
 */
+   if (rename_limit <= 0)
+   rename_limit = 32767;
if ((num_create <= rename_limit || num_src <= rename_limit) &&
((uint64_t)num_create * (uint64_t)num_src
 <= (uint64_t)rename_limit * (uint64_t)rename_limit))
diff --git a/t/t4001-diff-rename.sh b/t/t4001-diff-rename.sh
index 0d1fa45d2..eadf4f624 100755
--- a/t/t4001-diff-rename.sh
+++ b/t/t4001-diff-rename.sh
@@ -230,4 +230,19 @@ test_expect_success 'rename pretty print common prefix and 
suffix overlap' '
test_i18ngrep " d/f/{ => f}/e " output
 '
 
+test_expect_success 'diff-tree -l0 defaults to a big rename limit, not zero' '
+   test_write_lines line1 line2 line3 >myfile &&
+   git add myfile &&
+   git commit -m x &&
+
+   test_write_lines line1 line2 line4 >myotherfile &&
+   git rm myfile &&
+   git add myotherfile &&
+   git commit -m x &&
+
+   git diff-tree -M -l0 HEAD HEAD^ >actual &&
+   # Verify that a rename from myotherfile to myfile was detected
+   grep "myotherfile.*myfile" actual
+'
+
 test_done
-- 
2.15.0.173.g9268cf4a2.dirty



Re: [PATCH on en/rename-progress v2] diffcore-rename: make diff-tree -l0 mean -l

2017-11-29 Thread Jonathan Nieder
Jonathan Tan wrote:

> In the documentation of diff-tree, it is stated that the -l option
> "prevents rename/copy detection from running if the number of
> rename/copy targets exceeds the specified number". The documentation
> does not mention any special handling for the number 0, but the
> implementation before commit 9f7e4bfa3b ("diff: remove silent clamp of
> renameLimit", 2017-11-13) treated 0 as a special value indicating that
> the rename limit is to be a very large number instead.
>
> The commit 9f7e4bfa3b changed that behavior, treating 0 as 0. Revert
> this behavior to what it was previously. This allows existing scripts
> and tools that use "-l0" to continue working. The alternative (to have
> "-l0" suppress rename detection) is probably much less useful, since
> users can just refrain from specifying -M and/or -C to have the same
> effect.
>
> Signed-off-by: Jonathan Tan 
> ---
> v2 is exactly the same as previously, except that the commit message is
> changed following Elijah Newren's and Jonathan Nieder's comments.
>
>  diffcore-rename.c  |  2 ++
>  t/t4001-diff-rename.sh | 15 +++
>  2 files changed, 17 insertions(+)

Reviewed-by: Jonathan Nieder 

Thanks again.

> diff --git a/diffcore-rename.c b/diffcore-rename.c
> index 9ca0eaec7..245e999fe 100644
> --- a/diffcore-rename.c
> +++ b/diffcore-rename.c
> @@ -392,6 +392,8 @@ static int too_many_rename_candidates(int num_create,
>*
>*num_create * num_src > rename_limit * rename_limit
>*/
> + if (rename_limit <= 0)
> + rename_limit = 32767;
>   if ((num_create <= rename_limit || num_src <= rename_limit) &&
>   ((uint64_t)num_create * (uint64_t)num_src
><= (uint64_t)rename_limit * (uint64_t)rename_limit))
> diff --git a/t/t4001-diff-rename.sh b/t/t4001-diff-rename.sh
> index 0d1fa45d2..eadf4f624 100755
> --- a/t/t4001-diff-rename.sh
> +++ b/t/t4001-diff-rename.sh
> @@ -230,4 +230,19 @@ test_expect_success 'rename pretty print common prefix 
> and suffix overlap' '
>   test_i18ngrep " d/f/{ => f}/e " output
>  '
>  
> +test_expect_success 'diff-tree -l0 defaults to a big rename limit, not zero' 
> '
> + test_write_lines line1 line2 line3 >myfile &&
> + git add myfile &&
> + git commit -m x &&
> +
> + test_write_lines line1 line2 line4 >myotherfile &&
> + git rm myfile &&
> + git add myotherfile &&
> + git commit -m x &&
> +
> + git diff-tree -M -l0 HEAD HEAD^ >actual &&
> + # Verify that a rename from myotherfile to myfile was detected
> + grep "myotherfile.*myfile" actual
> +'
> +
>  test_done


Re: [PATCH v4 3/4] Makefile: add Perl runtime prefix support

2017-11-29 Thread Ævar Arnfjörð Bjarmason

On Wed, Nov 29 2017, Dan Jacques jotted:

> Add a new Makefile flag, RUNTIME_PREFIX_PERL, which, when enabled,
> configures Perl scripts to locate the Git installation's Perl support
> libraries by resolving against the script's path, rather than
> hard-coding that path at build-time.
> [...]
> diff --git a/perl/header_runtime_prefix.pl.template 
> b/perl/header_runtime_prefix.pl.template
> new file mode 100644
> index 0..fb9a9924d
> --- /dev/null
> +++ b/perl/header_runtime_prefix.pl.template
> @@ -0,0 +1,24 @@
> +# BEGIN RUNTIME_PREFIX_PERL generated code.
> +#
> +# This finds our Git::* libraries relative to the script's runtime path.
> +BEGIN {
> + use lib split /@@PATHSEP@@/,
> + (
> + $ENV{GITPERLLIB}
> + ||
> + do {
> + require FindBin;
> + require File::Spec;
> + my $gitexecdir_relative = '@@GITEXECDIR@@';
> + my $perllibdir_relative = '@@PERLLIBDIR@@';
> +
> + ($FindBin::Bin =~ m=${gitexecdir_relative}$=) ||
> + die('Unrecognized runtime path.');
> + my $prefix = substr($FindBin::Bin, 0, 
> -length($gitexecdir_relative));
> + my $perllibdir = File::Spec->catdir($prefix, 
> $perllibdir_relative);
> + (-e $perllibdir) || die("Invalid library path: 
> $perllibdir");
> + $perllibdir;
> + }
> + );
> +}
> +# END RUNTIME_PREFIX_PERL generated code.

Ah, I see. To answer my own question in
<87lgiovokg@evledraar.booking.com> you're making this stuff a
relative path so you can use it here later on. I.e. we $FindBin::Bin,
and then go from there. Makes sense.

We could use $ENV{GIT_EXEC_PATH} instead of FindBin here though, I
missed that the first time. But that's just a nano-optimization. I just
wondered whether git wasn't already passing us this info.

There is one remaining bug here. Git::I18N isn't doing the right thing,
I installed in /tmp/git and moved to /tmp/git2, and it has:

our $TEXTDOMAINDIR = $ENV{GIT_TEXTDOMAINDIR} || '/tmp/git/share/locale';

And GIT_TEXTDOMAINDIR is not passed by git (it's only used for the tests
IIRC). Would need a similar treatment as this. Easiest to just set the
path we find here in $Git::Whatever and pick it up in $Git::I18N later,
it's not like anyone uses it outside of git.git.

But that does raise a more general concern for me. Isn't there some way
we can run the test suite against an installed git (don't remember),
then build, install, move the dir, and run the tests from the moved dir.

That would have caught this bug, and anything else that may be lurking
still.


Re: [PATCH v4 3/4] Makefile: add Perl runtime prefix support

2017-11-29 Thread Ævar Arnfjörð Bjarmason

On Wed, Nov 29 2017, Dan Jacques jotted:


> + use lib split /@@PATHSEP@@/,
> + (
> + $ENV{GITPERLLIB}
> + ||
> + do {
> + require FindBin;
> + require File::Spec;
> + my $gitexecdir_relative = '@@GITEXECDIR@@';
> + my $perllibdir_relative = '@@PERLLIBDIR@@';
> +
> + ($FindBin::Bin =~ m=${gitexecdir_relative}$=) ||
> + die('Unrecognized runtime path.');
> + my $prefix = substr($FindBin::Bin, 0, 
> -length($gitexecdir_relative));
> + my $perllibdir = File::Spec->catdir($prefix, 
> $perllibdir_relative);
> + (-e $perllibdir) || die("Invalid library path: 
> $perllibdir");
> + $perllibdir;
> + }
> + );
> +}
> +# END RUNTIME_PREFIX_PERL generated code.

Noticed after I sent the last E-Mail, this is missing @@INSTLIBDIR@@
which per my reading of it being initially introduced should be here in
addition to this relative path.

My reading of the intial patch that added it, as indicated in my patch,
is that it's the dir we're going to be installing our libs to, so I
didn't fiddle with it, but I think with your patches we need that dir
*and* or own $perllibdir.


Re: [PATCH v4 0/4] RUNTIME_PREFIX relocatable Git

2017-11-29 Thread Ævar Arnfjörð Bjarmason

On Wed, Nov 29 2017, Dan Jacques jotted:

> This is a small update to incorporate some Windows fixes from Johannes.
> At this point, it passes the full test suite on Linux, Mac, and FreeBSD,
> as well as the Travis.ci test suites, with and without
> RUNTIME_PREFIX/RUNTIME_PREFIX_PERL flags.
>
> I'm happy with the patch set, and feel that it is ready to move forward.
> However, while it's been looked at by several people, and I have
> incorporated reviewer feedback, the patch set hasn't received any formal
> LGTM-style responses yet. I'm not sure what standard of review is required
> to move forward with a patch on this project, so maybe this is totally
> fine, but I wanted to make sure to point this out.
>
> I also want to note Ævar Arnfjörð Bjarmason's RFC:
> https://public-inbox.org/git/20171129153436.24471-1-ava...@gmail.com/T/
>
> The proposed patch set conflicts with the Perl installation directory
> changes in this patch set, as avarab@ notes. The proposed Perl installation
> process would simplify patch 0002 in this patch set. I don't think the
> landing order is terribly impactful - if this lands first, the patch in the
> RFC would delete a few more lines, and if this lands later, patch 0002
> would largely not be necessary.

In general this whole thing structurally looks good to me with the
caveats noted in other review E-Mails.

I haven't done anything but skim the details of the "where's my
executable" C code though, just looked at what it's doing structurally.

I think it makes sense for this to land first ahead of my patch. This is
an actual feature you need, whereas I just hate our use of MakeMaker,
but that can wait, unless you're keen to rebase this on my patch. Would
probably make your whole diff a bit shorter.

The whole converting our absolute to relative paths in the make code is
unavoidably ugly, but after having an initial knee-jerk reaction to it I
don't see how it can be avoided. I was hoping most of these paths
could/would just be a fixed path away from our libexec path, but alas
due to having had these configurable all along that simplicity seems out
of reach.

Maybe I asked this before, but I don't see any obvious reason for why
RUNTIME_PREFIX_PERL is a different thing than RUNTIME_PREFIX as opposed
to us just doing the right thing for the perl scripts.


Re: [PATCH] pathspec: only match across submodule boundaries when requested

2017-11-29 Thread Johannes Schindelin
Hi Brandon,

On Tue, 28 Nov 2017, Brandon Williams wrote:

> Commit 74ed43711fd (grep: enable recurse-submodules to work on 
> objects, 2016-12-16) taught 'tree_entry_interesting()' to be able to
> match across submodule boundaries in the presence of wildcards.  This is
> done by performing literal matching up to the first wildcard and then
> punting to the submodule itself to perform more accurate pattern
> matching.  Instead of introducing a new flag to request this behavior,
> commit 74ed43711fd overloaded the already existing 'recursive' flag in
> 'struct pathspec' to request this behavior.
> 
> This leads to a bug where whenever any other caller has the 'recursive'
> flag set as well as a pathspec with wildcards that all submodules will
> be indicated as matches.  One simple example of this is:
> 
>   git init repo
>   cd repo
> 
>   git init submodule
>   git -C submodule commit -m initial --allow-empty
> 
>   touch "[bracket]"
>   git add "[bracket]"
>   git commit -m bracket
>   git add submodule
>   git commit -m submodule
> 
>   git rev-list HEAD -- "[bracket]"
> 
> Fix this by introducing the new flag 'recurse_submodules' in 'struct
> pathspec' and using this flag to determine if matches should be allowed
> to cross submodule boundaries.
> 
> Signed-off-by: Brandon Williams 

Could you also add something like

This fixes https://github.com/git-for-windows/git/issues/1371

at the end of the commit message, to keep a reference to the original bug
report?

>  4 files changed, 22 insertions(+), 2 deletions(-)

Phew. That was much smaller than I expected.

> +test_expect_success 'tree_entry_interesting does not match past submodule 
> boundaries' '
> + test_when_finished "rm -rf repo submodule" &&
> + git init submodule &&
> + test_commit -C submodule initial &&
> + git init repo &&
> + >"repo/[bracket]" &&
> + git -C repo add "[bracket]" &&
> + git -C repo commit -m bracket &&
> + git -C repo rev-list HEAD -- "[bracket]" >expect &&
> +
> + git -C repo submodule add ../submodule &&
> + git -C repo commit -m submodule &&
> +
> + git -C repo rev-list HEAD -- "[bracket]" >actual &&
> + test_cmp expect actual
> +'

Nicely prepared for a new hash function, too (no explicit SHA-1).

I wonder, however, why we can't `git checkout -b bracket` and
`test_when_finished "git checkout master"` and void those many `-C repo`
options. But then, it is actually one of the shorter test cases, and
pretty easy to understand.

However, I would still like to see `test_tick`s before those `git commit`
calls, to make the commit names reproducible.

Thanks,
Dscho


From Mrs. Serita Miner

2017-11-29 Thread Mrs. Serita Miner
My name is Mrs. Serita Miner is 53years old, I wish to make a donation
of 3.5million dollars to you. As my doctor just confirmed to me that I
have limited days to live due to the cancer problems I am suffering
from.
I have asked the lord to forgive me all my sins and I believe he has,
because He is merciful. I will be going in for 3rd operation, and I
pray that I survive the operation this third time I have decided to
WILL/Donate this fund to charity through you for the good work of the
lord, and to help the motherless, less privileged and also for the
assistance of the widows genuinely.
At the moment I cannot take any telephone calls, I have been
restricted by my doctor, from taking telephone calls because I deserve
all the rest I can get. And I need is from you please
Full name: ___
Full Address: _
Nationality: 
Mobile Number: 
E-mail address: ___
Occupation: __
Country   : __
Any form of Identification either Drivers license or International
Passport: __
 If you can assure me of being able to handle my offer please get back
to me for more details by e-mailing me. ( seritaminer...@gmail.com )
miner...@yahoo.com
Thank you,
Expecting to hear from you soon,
Mrs. Serita Miner.


Re: [PATCH 3/5] rebase -i: add exec commands via the rebase--helper

2017-11-29 Thread Johannes Schindelin
Hi Liam,

On Tue, 28 Nov 2017, liam Beguin wrote:

> I just realized we could maybe add exec instructions only after pick
> commands if we do add-exec-commands before rearrange-squash.

That won't work, because the squash/fixup commands are pick commands
before rearrange-squash. So you'd add one unwanted exec per
squash/fixup...

Ciao,
Dscho


Re: [PATCH 4/5] rebase -i: learn to abbreviate command names

2017-11-29 Thread Johannes Schindelin
Hi Liam,

On Tue, 28 Nov 2017, liam Beguin wrote:

> On 27/11/17 06:04 PM, Johannes Schindelin wrote:
> > 
> > On Sun, 26 Nov 2017, Liam Beguin wrote:
> > 
> >> @@ -2483,7 +2491,9 @@ int sequencer_make_script(int keep_empty, FILE *out,
> >>strbuf_reset(&buf);
> >>if (!keep_empty && is_original_commit_empty(commit))
> >>strbuf_addf(&buf, "%c ", comment_line_char);
> >> -  strbuf_addf(&buf, "pick %s ", oid_to_hex(&commit->object.oid));
> >> +  strbuf_addf(&buf, "%s %s ",
> >> +  abbreviate_commands ? "p" : "pick",
> >> +  oid_to_hex(&commit->object.oid));
> > 
> > I guess the compiler will optimize this code so that the conditional
> > is evaluated only once. Not that this is performance critical ;-)
> 
> Is your guess enough? :-) If not, how could I make sure this is
> optimized?  Should I do that check before the while() loop?

I am a fan of not relying too heavily on compiler optimization and e.g.
extract code from loops when it does not need to be evaluated every single
iteration. In this case:

const char *pick = abbreviate_commands ? "p" : "pick";
...
strbuf_addf(&buf, "%s %s ", pick,
oid_to_hex(&commit->object.oid));

But given Junio's comment that the assignment of `first` was too far away
from the line where it is used for his taste, I guess he will argue (once
again) the exact opposite of me.

Ciao,
Dscho


Re: [PATCH] git-prompt: fix reading files with windows line endings

2017-11-29 Thread Robert Abel
Hi Johannes,

On 29 Nov 2017 15:27, Johannes Schindelin wrote:
>> diff --git a/contrib/completion/git-prompt.sh 
>> b/contrib/completion/git-prompt.sh
>> index c6cbef38c2..71a64e7959 100644
>> --- a/contrib/completion/git-prompt.sh
>> +++ b/contrib/completion/git-prompt.sh
>> @@ -282,7 +282,7 @@ __git_eread ()
>>  {
>>  local f="$1"
>>  shift
>> -test -r "$f" && read "$@" <"$f"
>> +test -r "$f" && read "$@" <"$f" && export $@="${!@%$'\r'}"
> 
> As far as I understand, $'\r' is a Bash-only construct, and this file
> (git-prompt.sh) is targeting other Unix shells, too.

Sorry, I wasn't really aware about this bash-ism. I agree that a generic
solution would be best.

> So how about using `tr -d '\r' <"$f" | read "$@"` instead?

That doesn't work for me. Apparently, the variable is always reset to ""
and hence the prompt will always display the shortened sha1.
Maybe it has something to do with variable scoping inside the backtick
evaluation?

> Or maybe keep with the Bash construct, but guarded behind a test that we
> area actually running in Bash? Something like
> 
>   test -z "$BASH" || IFS=$' \t\r\n'

Actually, this got me thinking and reading the POSIX.1-2008, specifically
http://pubs.opengroup.org/onlinepubs/9699919799/utilities/read.html.

It seems POSIX states that IFS should be supported by read. This means
that it should be okay to just do

> test -r "$f" && IFS=" \t\r\n" read "$@" < "$f"

This would also get rid of the export and avoid introducing backtick
evaluation.

Regards,

Robert


Re: [PATCH v4 0/4] RUNTIME_PREFIX relocatable Git

2017-11-29 Thread Dan Jacques
> In general this whole thing structurally looks good to me with the
> caveats noted in other review E-Mails.
>
> I haven't done anything but skim the details of the "where's my
> executable" C code though, just looked at what it's doing structurally.
>
> I think it makes sense for this to land first ahead of my patch. This is
> an actual feature you need, whereas I just hate our use of MakeMaker,
> but that can wait, unless you're keen to rebase this on my patch. Would
> probably make your whole diff a bit shorter.

I'm not strictly pressed for time here, so we can put this off if that's
strategically appropriate. Chromiuum, right now, just manually patches
upstream Git with a similar patch set, so it's working and function. This
is just an effort to upstream those changes for everyone else to enjoy!

I think that landing in either order is probably okay, so if your RFC
goes through pretty quickly I don't mind rebasing, but if this is otherwise
good to go in v5, I wouldn't mind landing it and then removing the
obsoleted parts during the Makefile update.

> The whole converting our absolute to relative paths in the make code is
> unavoidably ugly, but after having an initial knee-jerk reaction to it I
> don't see how it can be avoided. I was hoping most of these paths
> could/would just be a fixed path away from our libexec path, but alas
> due to having had these configurable all along that simplicity seems out
> of reach.

Yeah I spent no small amount of time massaging that code hoping some better
formulation would shake out, and this is what I ended up with.
UNTIME_PREFIX_PERL is a specialty build with a stronger set of assumptions
than the standard installation (RE prefix-relative paths).

> Maybe I asked this before, but I don't see any obvious reason for why
> RUNTIME_PREFIX_PERL is a different thing than RUNTIME_PREFIX as opposed
> to us just doing the right thing for the perl scripts.

I did this to isolate Windows builds from the Perl script changes. Git-for-
Windows uses (invented) RUNTIME_PREFIX, but runs its Perl scripts in a
MinGW sub-environment which, internally, has the Perl libraries installed
at a fixed path, so it doesn't need any special resolution logic.

I support that if Git-for-Windows wants to, a trivial future patch can
merge the two, so I opted to play it safe and keep these changes isolated.

===

> We could use $ENV{GIT_EXEC_PATH} instead of FindBin here though, I
> missed that the first time. But that's just a nano-optimization. I just
> wondered whether git wasn't already passing us this info.

Oh good idea - I'll go ahead and do this.

> There is one remaining bug here. Git::I18N isn't doing the right thing,
> I installed in /tmp/git and moved to /tmp/git2, and it has:
>
> our $TEXTDOMAINDIR = $ENV{GIT_TEXTDOMAINDIR} || '/tmp/git/share/locale';
>
> And GIT_TEXTDOMAINDIR is not passed by git (it's only used for the tests
> IIRC). Would need a similar treatment as this. Easiest to just set the
> path we find here in $Git::Whatever and pick it up in $Git::I18N later,
> it's not like anyone uses it outside of git.git.

Good catch! I'm going to see what I can do here.

> But that does raise a more general concern for me. Isn't there some way
> we can run the test suite against an installed git (don't remember),
> then build, install, move the dir, and run the tests from the moved dir.
>
> That would have caught this bug, and anything else that may be lurking
> still.

I am not aware of such an option, but I'll take a look again. This sort of
thing might just require a reformulation of the test suite in general to
make it run against a Git installation instead of the intermediate
artifacts. A positive outcome would be the ability to remove the Perl
path environment variable hacks ($ENV{...} || ...) and just use production
resolution logic, increasing the test surface! But I think that's a bit more
work than I have time for at the moment.

===

> Noticed after I sent the last E-Mail, this is missing @@INSTLIBDIR@@
> which per my reading of it being initially introduced should be here in
> addition to this relative path.
>
> My reading of the intial patch that added it, as indicated in my patch,
> is that it's the dir we're going to be installing our libs to, so I
> didn't fiddle with it, but I think with your patches we need that dir
> *and* or own $perllibdir.

INSTLIBDIR is used for the standard fixed-prefix header, but not in this
one. This one uses a combination of GITEXECDIR and PERLLIBDIR. PERLLIBDIR
is effectively the prefix-relative part of INSTLIBDIR, so it's here in
spirit!

Thanks for taking the time to review! I'll go ahead and see what I can do
RE your comments.

Cheers,
-Dan


Re: [SCRIPT/RFC 0/3] git-commit --onto-parent (three-way merge, no working tree file changes)

2017-11-29 Thread Igor Djordjevic
Hi Hannes,

On 29/11/2017 20:11, Johannes Sixt wrote:
> 
> Ok, then please explain, how this process should work in my workflow
> and with the `commit --onto-parent` feature that you have in mind. I
> have an integration branch (which is a throw-away type, so you can
> mangle it in any way you want); it is a series of merges:
> 
>  ...A...C<- topics A, C
>  \   \ E
>---o---o---o---o I<- integration
>  /   /
>  ...B...D<- topics B, D
> 
> Now I find a bug in topic B. Assume that the merges of C and D have
> textual conflicts with the integration branch (but not with B) and/or
> may be evil. What should I do?
> 
> With git-post, I make a fixup commit commit on the integration
> branch, then `git post B && git merge B`:
> 
>  ...A...C  <- topics A, C
>  \   \
>---o---o---o---o---f---F<- integration
>  /   /   /
>  ...B...D   /  <- topic D
>  \ /
>   f'--'<- topic B
> 
> The merge F does not introduce any changes on the integration branch,
> so I do not need it, but it helps keep topic B off radar when I ask
> `git branch --no-merged` later.

But you`re not committing (posting) on your HEAD`s (direct) parent in 
the first place (topic B), so `commit --onto-parent` isn`t right tool 
for the job... yet :)

To make it easier to explain, I marked your integration branch 
initial head with "I" in the quote above (commit merging-in branch 
D), and marked commit merging-in branch C with "E".

HEAD being currently on commit "I", you can only use `--onto-parent` 
option to commit onto "E" or "D", being parents of "I".

To work with `--onto-parent` and be able to commit on top of any of 
the topic branches, you would need a situation like this instead:

 (1)  ...C  <- topic C
 |
...A |  <- topic A
\|
  ...o I<- integration
/|
...B |  <- topic B
 |
  ...D  <- topic D

With `commit --onto-parent` you would skip `git post B && git merge 
B` steps, where "fixup commit" would be done with `--onto-parent B`, 
So you end up in situation like this:

 (2)  ...C  <- topic C
 |
...A |  <- topic A
\|
  ...o I'   <- integration
/|
...B---f |  <- topic B
 |
  ...D  <- topic D

State of index and working tree files in your F and my I' commit is 
exactly the same (I' = I + f), where in my case (2) history looks 
like "f" was part of topic B from the start, before integration 
merge happened.

BUT, all this said, I understand that your starting position, where 
not all topic branches are merged at the same time (possibly to keep 
conflict resolution sane), is probably more often to be encountered 
in real-life work... and as existing `--onto-parent` machinery should 
be able to support it already, I`m looking forward to make it happen :)

Once there, starting from your initial position:

>...A...C<- topics A, C
>\   \ E
>  ---o---o---o---o I<- integration <- HEAD
>/   /
>...B...D<- topics B, D

... and doing something like `git commit --onto B --merge` would yield:
 
 (3) ...A...C<- topics A, C
 \   \ E
   ---o---o---o---o I'   <- integration
 /   /|
 ...B...D |  <- topic D
 \|
  f---'  <- topic B

... where (I' = I + f) is still true. If that`s preferred in some 
cases, it could even look like this instead:

 (4) ...A...C <- topics A, C
 \   \ E  I
   ---o---o---o---o---F   <- integration
 /   /   /
 ...B...D   / <- topic D
 \ /
  f---'   <- topic B

... where F(4) = I'(3), so similar situation, just that we don`t 
discard I but post F on top of it.

Good thing is all necessary logic should already be in place, I just 
need to think a bit about the most sane user interface, and get back 
to you. Thanks for invaluable input so far :)

Of course, do feel free to drop any ideas you come up with as well, 
on how `git commit` user interface/options leading to (3) or (4) 
should look like (they could both be supported).

> > Like working on multiple branches at the same time, in the manner
> > similar to what `git add --patch` allows in regards to working on
> > multiple commits at the same time. This just takes it on yet another
> > level... hopefully :)
> 
> 'kay, I'm not eagerly waiting for this particular next level (I
> prefer to keep things plain and simple), but I would never say this
> were a broken workflow. ;)

Hehe, thanks, I guess :) Simplicity, but user-oriented, is the major 
point here, where you can work on unrelated patch series at the same 
time (patch queues?), without a need to switch branches, while you 
still ma

Re: [PATCH on en/rename-progress v2] diffcore-rename: make diff-tree -l0 mean -l

2017-11-29 Thread Elijah Newren
On Wed, Nov 29, 2017 at 12:11 PM, Jonathan Tan  wrote:
> In the documentation of diff-tree, it is stated that the -l option
> "prevents rename/copy detection from running if the number of
> rename/copy targets exceeds the specified number". The documentation
> does not mention any special handling for the number 0, but the
> implementation before commit 9f7e4bfa3b ("diff: remove silent clamp of
> renameLimit", 2017-11-13) treated 0 as a special value indicating that
> the rename limit is to be a very large number instead.
>
> The commit 9f7e4bfa3b changed that behavior, treating 0 as 0. Revert
> this behavior to what it was previously. This allows existing scripts
> and tools that use "-l0" to continue working. The alternative (to have
> "-l0" suppress rename detection) is probably much less useful, since
> users can just refrain from specifying -M and/or -C to have the same
> effect.
>
> Signed-off-by: Jonathan Tan 
> ---
> There are multiple issues with the commit message, so I am sending this
> new version out.
>
> v2 is exactly the same as previously, except that the commit message is
> changed following Elijah Newren's and Jonathan Nieder's comments.
> ---
>  diffcore-rename.c  |  2 ++
>  t/t4001-diff-rename.sh | 15 +++
>  2 files changed, 17 insertions(+)

Reviewed-by: Elijah Newren 

Thanks.


[PATCH] hashmap: adjust documentation to reflect reality

2017-11-29 Thread Johannes Schindelin
The hashmap API is just complicated enough that even at least one
long-time Git contributor has to look up how to use it every time he
finds a new use case. When that happens, it is really useful if the
provided example code is correct...

While at it, "fix a memory leak", avoid statements before variable
declarations, fix a const -> no-const cast, several %l specifiers (which
want to be %ld), avoid using an undefined constant, call scanf()
correctly, use FLEX_ALLOC_STR() where appropriate, and adjust the style
here and there.

Signed-off-by: Johannes Schindelin 
---
 hashmap.h | 60 +---
 1 file changed, 29 insertions(+), 31 deletions(-)

diff --git a/hashmap.h b/hashmap.h
index 7cb29a6aede..7ce79f3f72c 100644
--- a/hashmap.h
+++ b/hashmap.h
@@ -18,75 +18,71 @@
  *
  * #define COMPARE_VALUE 1
  *
- * static int long2string_cmp(const struct long2string *e1,
+ * static int long2string_cmp(const void *hashmap_cmp_fn_data,
+ *const struct long2string *e1,
  *const struct long2string *e2,
- *const void *keydata, const void *userdata)
+ *const void *keydata)
  * {
- * char *string = keydata;
- * unsigned *flags = (unsigned*)userdata;
+ * const char *string = keydata;
+ * unsigned flags = *(unsigned *)hashmap_cmp_fn_data;
  *
  * if (flags & COMPARE_VALUE)
- * return !(e1->key == e2->key) || (keydata ?
- *  strcmp(e1->value, keydata) : strcmp(e1->value, e2->value));
+ * return e1->key != e2->key ||
+ *  strcmp(e1->value, string ? string : e2->value);
  * else
- * return !(e1->key == e2->key);
+ * return e1->key != e2->key;
  * }
  *
  * int main(int argc, char **argv)
  * {
  * long key;
- * char *value, *action;
- *
- * unsigned flags = ALLOW_DUPLICATE_KEYS;
+ * char value[255], action[32];
+ * unsigned flags = 0;
  *
  * hashmap_init(&map, (hashmap_cmp_fn) long2string_cmp, &flags, 0);
  *
- * while (scanf("%s %l %s", action, key, value)) {
+ * while (scanf("%s %ld %s", action, &key, value)) {
  *
  * if (!strcmp("add", action)) {
  * struct long2string *e;
- * e = malloc(sizeof(struct long2string) + strlen(value));
+ * FLEX_ALLOC_STR(e, value, value);
  * hashmap_entry_init(e, memhash(&key, sizeof(long)));
  * e->key = key;
- * memcpy(e->value, value, strlen(value));
  * hashmap_add(&map, e);
  * }
  *
  * if (!strcmp("print_all_by_key", action)) {
- * flags &= ~COMPARE_VALUE;
- *
- * struct long2string k;
+ * struct long2string k, *e;
  * hashmap_entry_init(&k, memhash(&key, sizeof(long)));
  * k.key = key;
  *
- * struct long2string *e = hashmap_get(&map, &k, NULL);
+ * flags &= ~COMPARE_VALUE;
+ * e = hashmap_get(&map, &k, NULL);
  * if (e) {
- * printf("first: %l %s\n", e->key, e->value);
- * while (e = hashmap_get_next(&map, e))
- * printf("found more: %l %s\n", e->key, e->value);
+ * printf("first: %ld %s\n", e->key, e->value);
+ * while ((e = hashmap_get_next(&map, e)))
+ * printf("found more: %ld %s\n", e->key, e->value);
  * }
  * }
  *
  * if (!strcmp("has_exact_match", action)) {
- * flags |= COMPARE_VALUE;
- *
  * struct long2string *e;
- * e = malloc(sizeof(struct long2string) + strlen(value));
+ * FLEX_ALLOC_STR(e, value, value);
  * hashmap_entry_init(e, memhash(&key, sizeof(long)));
  * e->key = key;
- * memcpy(e->value, value, strlen(value));
  *
- * printf("%s found\n", hashmap_get(&map, e, NULL) ? "" : "not");
+ * flags |= COMPARE_VALUE;
+ * printf("%sfound\n", hashmap_get(&map, e, NULL) ? "" : "not ");
+ * free(e);
  * }
  *
  * if (!strcmp("has_exact_match_no_heap_alloc", action)) {
- * flags |= COMPARE_VALUE;
- *
- * struct long2string e;
- * hashmap_entry_init(e, memhash(&key, sizeof(long)));
- * e.key = key;
+ * struct long2string k;
+ * hashmap_entry_init(&k, memhash(&key, sizeof(long)));
+ * k.key = key;
  *
- * printf("%s found\n", hashmap_get(&map, e, value) ? "" : "not");
+ * flags |= COMPARE_VALUE;
+ * printf("%sfound\n", hashmap_get(&map, &k, value) ? "" : "not ");
  * }
  *
  * if (!strcmp("end", action)) {
@@ -94,6 +90,8 @@
  * break;
  * }
  * }
+ *
+ * return 0;
  * }
  */
 

base-commit: 1a4e40aa5dc16564af879142ba9dfbbb88d1e5ff
-- 
2.15.0.windows

Re: [ANNOUNCE] Git for Windows 2.15.1

2017-11-29 Thread Igor Djordjevic
Hi Johannes,

On 29/11/2017 14:57, Johannes Schindelin wrote:
> 
>   * It is now possible to configure nano or Notepad++ as Git's
> default editor instead of vim.

This seems as a really nice option, as it could\should greatly help 
Windows people in lowering friction in first encounter with Git (for 
Windows).

Being pretty unfamiliar with Linux and its tools at the time, I 
remember the initial frustration in trying to do what otherwise felt 
as a no-brain, simple and trivial task - write the damn commit 
message after `git commit`, lol. Even had to kill the bash window a 
few times, not knowing what to do, where it was clear it was 
expecting something from me :$

I later learned about vim, like getting started with Git wasn`t hard 
enough... :) As soon as I found it being a possibility, I`ve set 
Notepad++ as my default editor.

That said, what is the Notepad++ as default editor option doing, just 
setting:

[core]
editor = 'F:/Install/Notepad++/notepad++.exe' -multiInst -notabbar 
-nosession

... inside users` .gitconfig (`git config --global`)? Asking because 
I already had it there, and seems the option made no difference, so 
I`m not sure if it actually worked.

Otherwise, I guess I can dig the answer up from the installer code as 
well... :)

Thanks for yet another Git for Windows release.

Regards, Buga

p.s. Ok, It seems it actually went to `git config --system` instead, 
like:

[core]
editor = 'F:\\Install\\Notepad++\\notepad++.exe' -multiInst -notabbar 
-nosession -noPlugin

I removed my original (user) configuration, and it still works (minus 
plugins, due to that last parameter), so all good :)

Plugins do come in handy for me, though, like spell-check when 
writing commit messages, so I`ll just stick with my option for now.


Re: [PATCH] hashmap: adjust documentation to reflect reality

2017-11-29 Thread Jonathan Nieder
Johannes Schindelin wrote:

> The hashmap API is just complicated enough that even at least one
> long-time Git contributor has to look up how to use it every time he
> finds a new use case. When that happens, it is really useful if the
> provided example code is correct...
>
> While at it, "fix a memory leak", avoid statements before variable
> declarations, fix a const -> no-const cast, several %l specifiers (which
> want to be %ld), avoid using an undefined constant, call scanf()
> correctly, use FLEX_ALLOC_STR() where appropriate, and adjust the style
> here and there.
>
> Signed-off-by: Johannes Schindelin 
> ---
>  hashmap.h | 60 +---
>  1 file changed, 29 insertions(+), 31 deletions(-)

Yay, thanks for this.

Reviewed-by: Jonathan Nieder 

Follow-on idea for the interested: would making a test that extracts
this sample code from hashmap.h and confirms it still works be a crazy
idea?


Re: [PATCH] git-prompt: fix reading files with windows line endings

2017-11-29 Thread Johannes Schindelin
Hi Robert,

On Wed, 29 Nov 2017, Robert Abel wrote:

> On 29 Nov 2017 15:27, Johannes Schindelin wrote:
>
> > Or maybe keep with the Bash construct, but guarded behind a test that we
> > area actually running in Bash? Something like
> > 
> > test -z "$BASH" || IFS=$' \t\r\n'
> 
> Actually, this got me thinking and reading the POSIX.1-2008, specifically
> http://pubs.opengroup.org/onlinepubs/9699919799/utilities/read.html.
> 
> It seems POSIX states that IFS should be supported by read.

Yes, that's what I meant: you could use IFS.

> This means that it should be okay to just do
> 
> > test -r "$f" && IFS=" \t\r\n" read "$@" < "$f"

I am afraid that this won't work: when I call

printf '123\r\n' |
while IFS=" \t\r\n" read line
do
printf '%s' "$line" |
hexdump -C
done

it prints

  31 32 33 0d   |123.|
0004

If I replace the double-quoted IFS by the dollar-single-quoted one, it
works again. I think the reason is that \t, \r and \n are used literally
when double-quoted, not as ,  and .

Ciao,
Johannes


Re: [PATCH v3] diff: support anchoring line(s)

2017-11-29 Thread Johannes Schindelin
Hi Jonathan,

On Tue, 28 Nov 2017, Jonathan Tan wrote:

> @@ -4607,7 +4627,14 @@ int diff_opt_parse(struct diff_options *options,
>   DIFF_XDL_CLR(options, NEED_MINIMAL);
>   options->xdl_opts &= ~XDF_DIFF_ALGORITHM_MASK;
>   options->xdl_opts |= value;
> + if (value == XDF_PATIENCE_DIFF)
> + clear_patience_anchors(options);
>   return argcount;
> + } else if (skip_prefix(arg, "--anchored=", &arg)) {
> + options->xdl_opts = DIFF_WITH_ALG(options, PATIENCE_DIFF);
> + ALLOC_GROW(options->anchors, options->anchors_nr + 1,
> +options->anchors_alloc);
> + options->anchors[options->anchors_nr++] = xstrdup(arg);

I looked and failed to find the code that releases this array after the
diff is done... did I miss anything?

Ciao,
Dscho


Re: [PATCH] git-prompt: fix reading files with windows line endings

2017-11-29 Thread SZEDER Gábor
> > diff --git a/contrib/completion/git-prompt.sh 
> > b/contrib/completion/git-prompt.sh
> > index c6cbef38c2..71a64e7959 100644
> > --- a/contrib/completion/git-prompt.sh
> > +++ b/contrib/completion/git-prompt.sh
> > @@ -282,7 +282,7 @@ __git_eread ()
> >  {
> > local f="$1"
> > shift
> > -   test -r "$f" && read "$@" <"$f"
> > +   test -r "$f" && read "$@" <"$f" && export $@="${!@%$'\r'}"

I don't think that export is necessary here.

> As far as I understand, $'\r' is a Bash-only construct, and this file
> (git-prompt.sh) is targeting other Unix shells, too.

The only other shell the prompt (and completion) script is targeting
is ZSH, and ZSH understands this construct.  We already use this
construct to set IFS in several places in both scripts for a long
time, so it should be fine here, too.


Gábor



Re: [ANNOUNCE] Git for Windows 2.15.1

2017-11-29 Thread Johannes Schindelin
Hi Buga,

On Thu, 30 Nov 2017, Igor Djordjevic wrote:

> On 29/11/2017 14:57, Johannes Schindelin wrote:
> > 
> >   * It is now possible to configure nano or Notepad++ as Git's
> > default editor instead of vim.
> 
> This seems as a really nice option, as it could\should greatly help 
> Windows people in lowering friction in first encounter with Git (for 
> Windows).
> 
> Being pretty unfamiliar with Linux and its tools at the time, I 
> remember the initial frustration in trying to do what otherwise felt 
> as a no-brain, simple and trivial task - write the damn commit 
> message after `git commit`, lol. Even had to kill the bash window a 
> few times, not knowing what to do, where it was clear it was 
> expecting something from me :$
> 
> I later learned about vim, like getting started with Git wasn`t hard 
> enough... :) As soon as I found it being a possibility, I`ve set 
> Notepad++ as my default editor.

Thanks for this entertaining personal account!

And yes, you guessed it, I wanted this option for a long time, but never
got around to it (always hoping that somebody would beat me to it...). BTW
this installer page is probably far from done, there is a lot of room for
improvement, e.g. this up-for-grabs ticket:

https://github.com/git-for-windows/git/issues/1356

(hint, hint ;-))

> That said, what is the Notepad++ as default editor option doing, just 
> setting:
> 
> [core]
>   editor = 'F:/Install/Notepad++/notepad++.exe' -multiInst -notabbar 
> -nosession
> 
> ... inside users` .gitconfig (`git config --global`)?

As you found out, it is set in the system config. There are two reasons
for that:

- the installer runs as administrator, so it cannot know for which user you
  want to configure Notepad++

- in case the user does not like the setting (as in your case), they can
  still override it in their $HOME forever.

Ciao,
Johannes


[ANNOUNCE] Git for Windows 2.15.1(2), was Re:Git for Windows 2.15.1

2017-11-29 Thread Johannes Schindelin
Hi all,

unfortunately, a last-minute bug slipped in: MSYS2 updated vim (including
its defaults.vim, in a way that interacted badly with Git for Windows'
configuration). The result was that an ugly warning is shown every time a
user opens vim (which is still the default editor of Git for Windows, for
historical reasons).

Git for Windows v2.15.1(2) fixes just this one bug, and can be downloaded
here:

https://github.com/git-for-windows/git/releases/tag/v2.15.1.windows.2

Ciao,
Johannes

P.S.: Oh, yes, you're right, I forgot the SHA-256s:

Git-2.15.1.2-64-bit.exe
5eaffe3f364c89d2811f572678b3d4bbd092b887f2dda1f2d12b7b81eea4e6a7
Git-2.15.1.2-32-bit.exe
d227c0694fe1eef9a7299570167549aaf454bf28a5ac6d34ad3d288d9185f7e2
PortableGit-2.15.1.2-64-bit.7z.exe
36847f62418a5c62a7f50650f3662283f8d324602f4a611d592095ee296cd912
PortableGit-2.15.1.2-32-bit.7z.exe
8b1973bde82718684945c7373266976c70be55022ac554847a8a201c941952af
MinGit-2.15.1.2-64-bit.zip
59060c93425709d66db1756f884d8f235b012d58f6de0963087afbac46134c57
MinGit-2.15.1.2-32-bit.zip
6d2e782c9fd714feab86a6a6358353da32adb4975eaca8a6532bc51aa0ad4ed9
MinGit-2.15.1.2-busybox-64-bit.zip
2d2f3ac49caf7296dd4299236ca9425dc11954b3486f5a3301bf7b7a63286dbc
MinGit-2.15.1.2-busybox-32-bit.zip
2b25fcc2b27e8f1e75eae79912c59a35b066397195597600fd48e32140df0f24
Git-2.15.1.2-64-bit.tar.bz2
174f1a37f313a4ac6cb617693d10d28304d6b952121b41a934e5c484eb68444c
Git-2.15.1.2-32-bit.tar.bz2
709c15a33c0c25f4b293438eef5cf8c501e84032be36a5f4de8815df1e927eaf

On Wed, 29 Nov 2017, Johannes Schindelin wrote:

> Dear Git users,
> 
> It is my pleasure to announce that Git for Windows 2.15.1 is available from:
> 
>   https://git-for-windows.github.io/
> 
> Changes since Git for Windows v2.15.0 (October 30th 2017)
> 
> New Features
> 
>   * Comes with Git v2.15.1.
>   * Operations in massively-sparse worktrees are now much faster if
> core.fscache = true.
>   * It is now possible to configure nano or Notepad++ as Git's default
> editor instead of vim.
>   * Comes with OpenSSL v1.0.2m.
>   * Git for Windows' updater now uses non-intrusive toast notifications
> on Windows 8, 8.1 and 10.
>   * Running git fetch in a repository with lots of refs is now
> considerably faster.
>   * Comes with cURL v7.57.0.
> 
> Bug Fixes
> 
>   * The experimental --show-ignored-directory option of git status
> which was removed in Git for Windows v2.15.0 without warning has
> been reintroduced as a deprecated option.
>   * The git update command (to auto-update Git for Windows) will now
> work also work behind proxies.
> 
> Filename | SHA-256
>  | ---
> Git-2.15.1-64-bit.exe | 
> a2ba53197db79b361502836eecf97f09881703148774f9b4e9e6749d41d4ff71
> Git-2.15.1-32-bit.exe | 
> 73154bdfd1ad4ced8612d97b95603ff2b2383db9d46b4c308827fb5233d52592
> PortableGit-2.15.1-64-bit.7z.exe | 
> 94d485454af33a32d08680950aaf38f0825a189ef8b617054b81b2c48d817699
> PortableGit-2.15.1-32-bit.7z.exe | 
> 7d804748a7de735133d78c5420d9b338379123734509415035593e106b03515a
> MinGit-2.15.1-64-bit.zip | 
> 5e38d13241b0742e6673bc5116ac82e851dd1fad01c660c943017f4549b6afea
> MinGit-2.15.1-32-bit.zip | 
> 2fc85f67cabe3c13aceb6868b4bb6bda28ddfecd6f32d7e0da9ddce8cee9b940
> MinGit-2.15.1-busybox-64-bit.zip | 
> 02611486e3c8c427f09d2c4639484cd604ea812471248ae928960f1e0dc59633
> MinGit-2.15.1-busybox-32-bit.zip | 
> a6dfb770f5cfa7b120ba49922d3434577b8601c5d322ad473dd2e2adc91e92b3
> Git-2.15.1-64-bit.tar.bz2 | 
> bb630e5f3d7b650db67134b0187cf0cb8f5ed75990838ee65fed85e21777f08a
> Git-2.15.1-32-bit.tar.bz2 | 
> ec3938e161ac1bbcf2b4f5d41fae1c05ea229fa0276b4db8530ec50b69131832
> 
> Ciao,
> Johannes
> 


Re: [PATCH] git-prompt: fix reading files with windows line endings

2017-11-29 Thread Johannes Schindelin
Hi Gábor,

On Thu, 30 Nov 2017, SZEDER Gábor wrote:

> > > diff --git a/contrib/completion/git-prompt.sh 
> > > b/contrib/completion/git-prompt.sh
> > > index c6cbef38c2..71a64e7959 100644
> > > --- a/contrib/completion/git-prompt.sh
> > > +++ b/contrib/completion/git-prompt.sh
> > > @@ -282,7 +282,7 @@ __git_eread ()
> > >  {
> > >   local f="$1"
> > >   shift
> > > - test -r "$f" && read "$@" <"$f"
> > > + test -r "$f" && read "$@" <"$f" && export $@="${!@%$'\r'}"
> 
> I don't think that export is necessary here.
> 
> > As far as I understand, $'\r' is a Bash-only construct, and this file
> > (git-prompt.sh) is targeting other Unix shells, too.
> 
> The only other shell the prompt (and completion) script is targeting
> is ZSH, and ZSH understands this construct.  We already use this
> construct to set IFS in several places in both scripts for a long
> time, so it should be fine here, too.

That's good to know! I should have `git grep`ped...

Sorry for the noise,
Johannes

Re: imap-send with gmail: curl_easy_perform() failed: URL using bad/illegal format or missing URL

2017-11-29 Thread Jonathan Nieder
(+cc: Nicolas)
Hi,

Doron Behar wrote:

> I'm trying to send a patch with the command `git imap-send`, I used the
> examples in the manual page as the main reference for my configuration:
>
> ```
> [imap]
>   folder = "[Gmail]/Drafts"
>   host = imaps://imap.gmail.com
>   user = doron.be...@gmail.com
>   port = 993
>   sslverify = false
> ```
>
> This is my `cat patch.out | git imap-send` output:
>
> ```
> Password for 'imaps://doron.be...@gmail.com@imap.gmail.com':
> sending 3 messages
> curl_easy_perform() failed: URL using bad/illegal format or missing URL
> ```

Thanks for reporting this.  I suspect this is related to
v2.15.0-rc0~63^2 (imap-send: use curl by default when possible,
2017-09-14) --- e.g. perhaps our custom IMAP code was doing some
escaping on the username that libcurl does not do.

"man git imap-send" says this is a recommended configuration, so I
don't think it's a configuration error.

What platform are you on?  What version of libcurl are you using?

In libcurl::lib/easy.c I am also seeing

if(mcode)
  return CURLE_URL_MALFORMAT; /* TODO: return a proper error! */

which looks suspicious.

Nicolas, am I on the right track?

Thanks,
Jonathan

> The URI doesn't seem OK to me, I tried using `imap.user = doron.behar` and the
> URI was `imaps://doron.be...@imap.gmail.com` but that ended up with the same
> error as in the previous case.
>
> I would love to get some help here, a Google Search didn't help as well.
>
> Thanks.


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

2017-11-29 Thread Jonathan Nieder
Hi,

Ævar Arnfjörð Bjarmason wrote:

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

Yay!  This looks exciting.

One quick comment:

[...]
>  * We don't build the Git(3) Git::I18N(3) etc. man pages from the
>embedded perldoc. I suspect nobody really cares, these are mostly
>internal APIs, and if someone's developing against them they likely
>know enough to issue a "perldoc" against the installed file to get
>the same result.
>
>But this is a change in how Git is installed now on e.g. CentOS &
>Debian which carry these manpages. They could be added (via
>pod2man) if anyone really cares.
>
>I doubt they will. The reason these were built in the first place
>was as a side-effect of how ExtUtils::MakeMaker works.

Debian cares (see
https://www.debian.org/doc/packaging-manuals/perl-policy/ch-module_packages.html
for details).

I'll try applying this patch and seeing what happens some time this
week.

Thanks,
Jonathan


Att! Att!! Att!!! Att!!!!

2017-11-29 Thread Albert H Daniels
Good Day,
I'm Wong Shiu a staff of Wing Hang Bank
here in Hong Kong. Can i TRUST you in transferring-
$13,991,674 USD? If yes do get back to me with my private email: 
wong.shiu...@accountant.com
Best Regards


Re: [PATCH] hashmap: adjust documentation to reflect reality

2017-11-29 Thread Jeff King
On Thu, Nov 30, 2017 at 12:51:41AM +0100, Johannes Schindelin wrote:

> The hashmap API is just complicated enough that even at least one
> long-time Git contributor has to look up how to use it every time he
> finds a new use case. When that happens, it is really useful if the
> provided example code is correct...
> 
> While at it, "fix a memory leak", avoid statements before variable
> declarations, fix a const -> no-const cast, several %l specifiers (which
> want to be %ld), avoid using an undefined constant, call scanf()
> correctly, use FLEX_ALLOC_STR() where appropriate, and adjust the style
> here and there.

Heh, that's quite a list of faults for what's supposed to be simple
example code. ;)

Your improvements all look good to me, and I'd be happy to see this
applied as-is. But here are two possible suggestions:

> diff --git a/hashmap.h b/hashmap.h
> index 7cb29a6aede..7ce79f3f72c 100644
> --- a/hashmap.h
> +++ b/hashmap.h
> @@ -18,75 +18,71 @@
>   *
>   * #define COMPARE_VALUE 1
>   *
> - * static int long2string_cmp(const struct long2string *e1,
> + * static int long2string_cmp(const void *hashmap_cmp_fn_data,
> + *const struct long2string *e1,
>   *const struct long2string *e2,
> - *const void *keydata, const void *userdata)
> + *const void *keydata)

If these struct pointers became "const void *", then we would not need
to cast the function pointer here:

>   * hashmap_init(&map, (hashmap_cmp_fn) long2string_cmp, &flags, 0);

which would mean that the original problem you are fixing would have
been caught by the compiler, rather than probably segfaulting at
runtime.

My second suggestion (which I'm on the fence about) is: would it better
to just say "see t/helper/test-hashmap.c for a representative example?"

I'm all for code examples in documentation, but this one is quite
complex. The code in test-hashmap.c is not much more complex, and is at
least guaranteed to compile and run. It doesn't show off how to combine
a flex-array with a hashmap as well, but I'm not sure how important that
is. So I could go either way.

-Peff


How hard would it be to implement sparse fetching/pulling?

2017-11-29 Thread Vitaly Arbuzov
Hi guys,

I'm looking for ways to improve fetch/pull/clone time for large git
(mono)repositories with unrelated source trees (that span across
multiple services).
I've found sparse checkout approach appealing and helpful for most of
client-side operations (e.g. status, reset, commit, etc.)
The problem is that there is no feature like sparse fetch/pull in git,
this means that ALL objects in unrelated trees are always fetched.
It may take a lot of time for large repositories and results in some
practical scalability limits for git.
This forced some large companies like Facebook and Google to move to
Mercurial as they were unable to improve client-side experience with
git while Microsoft has developed GVFS, which seems to be a step back
to CVCS world.

I want to get a feedback (from more experienced git users than I am)
on what it would take to implement sparse fetching/pulling.
(Downloading only objects related to the sparse-checkout list)
Are there any issues with missing hashes?
Are there any fundamental problems why it can't be done?
Can we get away with only client-side changes or would it require
special features on the server side?

If we had such a feature then all we would need on top is a separate
tool that builds the right "sparse" scope for the workspace based on
paths that developer wants to work on.

In the world where more and more companies are moving towards large
monorepos this improvement would provide a good way of scaling git to
meet this demand.

PS. Please don't advice to split things up, as there are some good
reasons why many companies decide to keep their code in the monorepo,
which you can easily find online. So let's keep that part out the
scope.

-Vitaly


Re: imap-send with gmail: curl_easy_perform() failed: URL using bad/illegal format or missing URL

2017-11-29 Thread Jeff King
On Wed, Nov 29, 2017 at 06:04:45PM -0800, Jonathan Nieder wrote:

> > Password for 'imaps://doron.be...@gmail.com@imap.gmail.com':
> > sending 3 messages
> > curl_easy_perform() failed: URL using bad/illegal format or missing URL
> > ```
> 
> Thanks for reporting this.  I suspect this is related to
> v2.15.0-rc0~63^2 (imap-send: use curl by default when possible,
> 2017-09-14) --- e.g. perhaps our custom IMAP code was doing some
> escaping on the username that libcurl does not do.
> 
> "man git imap-send" says this is a recommended configuration, so I
> don't think it's a configuration error.
> 
> What platform are you on?  What version of libcurl are you using?

All good thoughts/questions. I have two suggestions to add:

 1. As an immediate work-around, running "imap-send --no-curl" may work. 
That will at least get this case working while we debug.

 2. Setting GIT_TRACE_CURL=1 may dump more verbose information. But one
caveat: if you get as far as authenticating, then the trace will
contain your password. We redact HTTP auth from the trace output,
but not imap ones.

-Peff


Re: imap-send with gmail: curl_easy_perform() failed: URL using bad/illegal format or missing URL

2017-11-29 Thread Jeff King
On Wed, Nov 29, 2017 at 10:28:32PM -0500, Jeff King wrote:

>  2. Setting GIT_TRACE_CURL=1 may dump more verbose information. But one
> caveat: if you get as far as authenticating, then the trace will
> contain your password. We redact HTTP auth from the trace output,
> but not imap ones.

I tried my hand at a patch, but it ended up a lot more complicated than
I would have liked. Thoughts?

diff --git a/http.c b/http.c
index 713525f38e..fcc9001af6 100644
--- a/http.c
+++ b/http.c
@@ -560,7 +560,7 @@ static void set_curl_keepalive(CURL *c)
 }
 #endif
 
-static void redact_sensitive_header(struct strbuf *header)
+static void redact_http_header(struct strbuf *header)
 {
const char *sensitive_header;
 
@@ -577,7 +577,60 @@ static void redact_sensitive_header(struct strbuf *header)
}
 }
 
-static void curl_dump_header(const char *text, unsigned char *ptr, size_t 
size, int hide_sensitive_header)
+static void redact_imap_header(struct strbuf *header)
+{
+   const char *p;
+
+   /* skip past the command tag */
+   p = strchr(header->buf, ' ');
+   if (!p)
+   return; /* no tag */
+   p++;
+
+   if (skip_prefix(p, "AUTHENTICATE ", &p)) {
+   /* the first token is the auth type, which is OK to log */
+   while (*p && !isspace(*p))
+   p++;
+   /* the rest is an opaque blob; fall through to redact */
+   } else if (skip_prefix(p, "LOGIN ", &p)) {
+   /* fall through to redact both login and password */
+   } else {
+   /* not a sensitive header */
+   return;
+   }
+
+   strbuf_setlen(header, p - header->buf);
+   strbuf_addstr(header, " ");
+}
+
+static void redact_sensitive_header(CURL *handle, struct strbuf *header)
+{
+   const char *url;
+   int ret;
+
+   ret = curl_easy_getinfo(handle, CURLINFO_EFFECTIVE_URL, &url);
+   if (!ret && url) {
+   if (starts_with(url, "http")) {
+   redact_http_header(header);
+   return;
+   }
+   if (starts_with(url, "imap")) {
+   redact_imap_header(header);
+   return;
+   }
+   }
+
+   /*
+* We weren't able to figure out the protocol. Err on the side of
+* redacting too much.
+*/
+   redact_http_header(header);
+   redact_imap_header(header);
+}
+
+static void curl_dump_header(CURL *handle, const char *text,
+unsigned char *ptr, size_t size,
+int hide_sensitive_header)
 {
struct strbuf out = STRBUF_INIT;
struct strbuf **headers, **header;
@@ -591,7 +644,7 @@ static void curl_dump_header(const char *text, unsigned 
char *ptr, size_t size,
 
for (header = headers; *header; header++) {
if (hide_sensitive_header)
-   redact_sensitive_header(*header);
+   redact_sensitive_header(handle, *header);
strbuf_insert((*header), 0, text, strlen(text));
strbuf_insert((*header), strlen(text), ": ", 2);
strbuf_rtrim((*header));
@@ -641,7 +694,7 @@ static int curl_trace(CURL *handle, curl_infotype type, 
char *data, size_t size,
break;
case CURLINFO_HEADER_OUT:
text = "=> Send header";
-   curl_dump_header(text, (unsigned char *)data, size, DO_FILTER);
+   curl_dump_header(handle, text, (unsigned char *)data, size, 
DO_FILTER);
break;
case CURLINFO_DATA_OUT:
text = "=> Send data";
@@ -653,7 +706,7 @@ static int curl_trace(CURL *handle, curl_infotype type, 
char *data, size_t size,
break;
case CURLINFO_HEADER_IN:
text = "<= Recv header";
-   curl_dump_header(text, (unsigned char *)data, size, NO_FILTER);
+   curl_dump_header(handle, text, (unsigned char *)data, size, 
NO_FILTER);
break;
case CURLINFO_DATA_IN:
text = "<= Recv data";


Re: [PATCH] git-prompt: fix reading files with windows line endings

2017-11-29 Thread Robert Abel
Hi Johannes,

On 30 Nov 2017 01:21, Johannes Schindelin wrote:
> On Wed, 29 Nov 2017, Robert Abel wrote:
>> This means that it should be okay to just do
>>
>>> test -r "$f" && IFS=" \t\r\n" read "$@" < "$f"
> 
> I am afraid that this won't work: when I call

I managed to trick myself with that one, yes...
Apparently I had already converted my HEAD back to Unix line endings.

However, I noticed that the behavior of read is apparently
ambiguous for the last (or a single) variable:

>From POSIX.1-2008:
> If there are fewer vars than fields, the last var shall be set to a
> value comprising the following elements:
> - The field that corresponds to the last var in the normal assignment
>   sequence described above
> - The delimiter(s) that follow the field corresponding to the last var
> - The remaining fields and their delimiters, with trailing IFS white
>   space ignored

I read that last "ignored" as "trailing IFS white space shall not be
appended". Apparently, people implementing read read it as "trailing
IFS while space shall not be processed further"

Thus, the behavior for trailing IFS white space is different in
case of one or two variables:

printf '123 456\r\n' | while IFS=$' \t\r\n' read foo bar
do
printf 'foo: %s' "$foo" | hexdump -C
printf 'bar: %s' "$bar" | hexdump -C
done

This works as expected trimming the trailing \r:
  66 6f 6f 3a 20 31 32 33   |foo: 123|
0008
  62 61 72 3a 20 34 35 36   |bar: 456|
0008

While doing the same just reading a single variable

printf '123 456\r\n' | while IFS=$' \t\r\n' read foo
do
printf 'foo: %s' "$foo" | hexdump -C
printf 'bar: %s' "$bar" | hexdump -C
done

prints

  66 6f 6f 3a 20 31 32 33  20 34 35 36 0d   |foo:
123 456.|
000d
  62 61 72 3a 20|bar: |
0005

Notice the 0d at the end of foo, which didn't get trimmed.

So reading a dummy variable along with the actual content variable
works for git-prompt:

__git_eread ()
{
local f="$1"
local dummy
shift
test -r "$f" && IFS=$'\r\n' read "$@" dummy < "$f"
}

I feel like this would be the most readable solution thus far.

Regards,

Robert


Git stash (or git rev-parse) doesn't inherit the '--work-tree' parameter

2017-11-29 Thread marc PINHEDE
Hello guys,

I don't know if this is a normal behavior, so I'll just explain it here.

* Behavior:

from a directory OUTSIDE my git repo, I can run this command successfully:
 $ git --git-dir=/my/repo/path/.git --work-tree=/my/repo/path pull origin master

But this one:
 $ git --git-dir=/my/repo/path/.git --work-tree=/my/repo/path stash
Will fails with :
 'fatal: $program_name cannot be used without a working tree.'
(Yes, the error message has been corrected since, but I use debian, so
I'm at least 6 month late).

* A bit of investigation:
I don't know many about git internal functioning, but with a bit of
googleing, I would point out git-sh-setup.sh:204, in function
require_work_tree, called by git-stash.
I could test (from outside my git repo, again):
 git --git-dir=/my/repo/path/.git --work-tree=/my/repo/path rev-parse
--is-inside-work-tree
return:
 false

What surprise me is that 'git-dir' seems to be passed ok to rev-parse:
 git --git-dir=/my/repo/path/.git --work-tree=/my/repo/path rev-parse --git-dir
returns:
 /my/repo/path/.git

* Question
Is this a normal behavior? Stash can only be applied in current
working directory?
If yes, I'll have to change my script to
 (cd && git command)
If not, should I push a bug report somewhere?

Marc