[PATCH] Make git log work for git CWD outside of work tree

[PATCH] Make git log work for git CWD outside of work tree

[Danny Sauer]

Make git log's `--use-mailmap` argument works if the GIT_DIR & GIT_WORK_TREE
env vars are set and git is run from outside of work tree.  Without the
NEED_WORK_TREE set on the log subcommand, .mailmap is silently not found.

Signed-off-by: Danny Sauer <danny@dannysauer.com>
---

Notes:
    I'm not entirely sure if this is the best way to fix it, as my git
    internals knowledge is pretty weak. But this /seems/ reasonable to me, and
    passes all of the current test cases.  If there's a more appropriate way to
    make `--use-mailmap` work properly when `git log` is run outside of the
    tree, I'd be excited about the opportunity to learn. :)

 git.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/git.c b/git.c
index 8ff44f0..e147f01 100644
--- a/git.c
+++ b/git.c
@@ -440,7 +440,7 @@ static struct cmd_struct commands[] = {
 	{ "init", cmd_init_db },
 	{ "init-db", cmd_init_db },
 	{ "interpret-trailers", cmd_interpret_trailers, RUN_SETUP_GENTLY },
-	{ "log", cmd_log, RUN_SETUP },
+	{ "log", cmd_log, RUN_SETUP | NEED_WORK_TREE },
 	{ "ls-files", cmd_ls_files, RUN_SETUP | SUPPORT_SUPER_PREFIX },
 	{ "ls-remote", cmd_ls_remote, RUN_SETUP_GENTLY },
 	{ "ls-tree", cmd_ls_tree, RUN_SETUP },
-- 
2.7.4

Re: [PATCH] Make git log work for git CWD outside of work tree

[Johannes Schindelin]

Hi Danny,

On Sat, 8 Apr 2017, Danny Sauer wrote:

> Make git log's `--use-mailmap` argument works if the GIT_DIR &
> GIT_WORK_TREE env vars are set and git is run from outside of work tree.
> Without the NEED_WORK_TREE set on the log subcommand, .mailmap is
> silently not found.

A laudable goal. How about adding a test case, say, to t/t4203-mailmap.sh,
to ensure that Git won't regress on your fix?

> diff --git a/git.c b/git.c
> index 8ff44f0..e147f01 100644
> --- a/git.c
> +++ b/git.c
> @@ -440,7 +440,7 @@ static struct cmd_struct commands[] = {
>  	{ "init", cmd_init_db },
>  	{ "init-db", cmd_init_db },
>  	{ "interpret-trailers", cmd_interpret_trailers, RUN_SETUP_GENTLY },
> -	{ "log", cmd_log, RUN_SETUP },
> +	{ "log", cmd_log, RUN_SETUP | NEED_WORK_TREE },
>  	{ "ls-files", cmd_ls_files, RUN_SETUP | SUPPORT_SUPER_PREFIX },
>  	{ "ls-remote", cmd_ls_remote, RUN_SETUP_GENTLY },
>  	{ "ls-tree", cmd_ls_tree, RUN_SETUP },

This may work for you, but it does not work for me, as I often call `git
log` in a bare repository. And that call works, and it should keep
working.

Instead, I think, you need to figure out why the .mailmap file is not read
correctly when you use the GIT_DIR & GIT_WORK_TREE approach. My vague
hunch is that you need to replace the ".mailmap" in read_mailmap()
(defined in mailmap.c):

        err |= read_mailmap_file(map, ".mailmap", repo_abbrev);

by something like mkpath("%s/%s", get_git_work_tree(), ".mailmap"), but
probably only after testing that we're not in a bare repository (which
would also fix a bug, I suspect, as `git log` in a bare repository
probably heeds a .mailmap file in the current directory, which is
incorrect). I.e. something like:

	if (!is_bare_repository()) {
		const char *path = mkpath("%s/%s",
					  get_git_work_tree(), ".mailmap")
		err |= read_mailmap_file(map, path, repo_abbrev);
	}

But you really want to add the test case first, with
`test_expect_failure`, to demonstrate what is currently broken, and then
triumphantly setting it to `test_expect_success` with your patch.

Ciao,
Johannes

Re: [PATCH] Make git log work for git CWD outside of work tree

[Danny Sauer]

On 4/9/2017 5:54 AM, Johannes Schindelin wrote:
> Hi Danny,
>
> On Sat, 8 Apr 2017, Danny Sauer wrote:
>> diff --git a/git.c b/git.c
>> index 8ff44f0..e147f01 100644
>> --- a/git.c
>> +++ b/git.c
>> @@ -440,7 +440,7 @@ static struct cmd_struct commands[] = {
>>  	{ "init", cmd_init_db },
>>  	{ "init-db", cmd_init_db },
>>  	{ "interpret-trailers", cmd_interpret_trailers, RUN_SETUP_GENTLY },
>> -	{ "log", cmd_log, RUN_SETUP },
>> +	{ "log", cmd_log, RUN_SETUP | NEED_WORK_TREE },
>>  	{ "ls-files", cmd_ls_files, RUN_SETUP | SUPPORT_SUPER_PREFIX },
>>  	{ "ls-remote", cmd_ls_remote, RUN_SETUP_GENTLY },
>>  	{ "ls-tree", cmd_ls_tree, RUN_SETUP },
> This may work for you, but it does not work for me, as I often call `git
> log` in a bare repository. And that call works, and it should keep
> working.

I was wondering about that situation, but honestly I wasn't positive how
to test it and sort of just went on the hope that the read_mailmap_blob
mechanism would continue handling it.  So, I guess there's another test
case which would be useful, as passing the test cases is what gave me
that undeserved confidence. :)  And I'm going to need to read a bit more
to figure out what a bare repository actually is...

> Instead, I think, you need to figure out why the .mailmap file is not read
> correctly when you use the GIT_DIR & GIT_WORK_TREE approach. My vague
> hunch is that you need to replace the ".mailmap" in read_mailmap()
> (defined in mailmap.c):
>
>         err |= read_mailmap_file(map, ".mailmap", repo_abbrev);
>
> by something like mkpath("%s/%s", get_git_work_tree(), ".mailmap"), but
> probably only after testing that we're not in a bare repository (which
> would also fix a bug, I suspect, as `git log` in a bare repository
> probably heeds a .mailmap file in the current directory, which is
> incorrect). I.e. something like:
>
> 	if (!is_bare_repository()) {
> 		const char *path = mkpath("%s/%s",
> 					  get_git_work_tree(), ".mailmap")
> 		err |= read_mailmap_file(map, path, repo_abbrev);
> 	}

That's feedback I was looking for - what's the "right" way to get the
path to the file.  The call as-is works in any subdirectory of a
repository, because there's some logic buried along the way which puts
you in the top level of the repo if you started underneath.  But if
you're outside, it does the same chdir to verify that the top level is a
real repo and then returns you to where you started (which is perfectly
reasonable).

In comparing how shortlog works v/s log, the main difference I could see
is the use of RUN_SETUP_GENTLY instead of RUN_SETUP as the flag, which
itself seems to only differ by passing in a true value for nongit_ok in
run_setup, ultimately leading to "some different directory checks" -
which I somewhat got lost in due partially to not fully knowing what a
bare repository is. :)  But changing that didn't make it work in my
(poorly-formalized) testing.

I'm on-board with a proper test case being the first situation which
needs rectifying.  It's off to learn stuff for me, then.  Thanks!

--Danny

Re: [PATCH] Make git log work for git CWD outside of work tree

[Junio C Hamano]

Danny Sauer <danny@dannysauer.com> writes:

> That's feedback I was looking for - what's the "right" way to get the
> path to the file.

My knee-jerk reaction is that you SHOULDN'T have to do anything
special to "get the path".

I do not have the code in front of me (this is my "down" week after
all), so let me give the general outline of how to reason about
this, with the knowledge of code evolution of the system.

In the beginning, Git worked only from the top level of the working
tree.  If you have your project in ~/myproject with a file "f" in a
directory "d", that file is ~/myproject/d/f", and you would do

    $ cd ~/myproject
    $ git add d/f

to add it.  If Git wanted to access the index, it just accessed
".git/index" as a relative path.  If Git wanted to access a file at
the top of the working tree, say ".gitignore", it just accessed
".gitignore" as a relatlvei path.  Because it only worked from the
top.

Then we added a support for running Git from a subdirectory, so that
you can say "cd ~/myproject/d && git add f".  In order to keep the
existing code that wants to access the index as ".git/index" and
wants to access the top-level ".gitignore" as ".gitignore" working,
the support to run Git from a subdirectory is designed this way:

    - Each main program of subcommand (e.g. cmd_log()) receives a
      parameter "prefix", which tells what subdirectory you were in
      when you started Git.

    - Before running the main program of subcommand, Git chdir(2)s
      up to the top level of the working tree.

    - The main program of subcommand receives the command line from
      the user intact.  It is responsible for prepending the prefix
      to the path the user gave it from the command line.

So if you did "cd ~/myproject/d && git add f", Git goes up to
"~/myproject", passes argv=("f", NULL) and prefix="d/" to cmd_add().

Adding to the index wants to read and update the index, which is
still done by opening ".git/index" (relative to the toplevel as
before), and inspecting the top-level .gitigore file is done by
opening ".gitignore" (relative to the toplevel as before).  And
the cmd_add() forms the path "d/f" by using the prefix "d/" and the
user-supplied pathname "f".

When we first added a support for having the (equivalent of) ".git/"
directory outside the working tree by setting GIT_DIR environment
variable, again, you can only use Git from the top-level of the
working tree.  Instead of "~/myproject/.git", you can keep your
repository metainformation in say "~/mypro.git/" and point at it
with GIT_DIR environment variable, and said

    $ export GIT_DIR=~/mypro.git
    $ cd ~/myproject
    $ git add d/f

Later we also added GIT_WORK_TREE environment variable to be used
together with GIT_DIR so that you can start from ~/myproject/d, very
similarly to how you worked from a subdirectory without these
environment variables, i.e.

    $ export GIT_DIR=~/mypro.git GIT_WORK_TREE=~/myproject
    $ cd ~/myproject/d
    $ git add f

The way this support was added was to go to the top-level of the
working tree (i.e. "~/myproject" in this example) and passing the
prefix (i.e. "d" in this example).

Notice that in all of the above configurations, if a Git command
knows a path to something that is relative to the top of the working
tree (e.g. ".git/index" in the ancient Git, ".gitignore" at the
top-level that governs the entire working tree, or ".mailmap"), it
should just be able to open that path without asking "where is the
top of the working tree?".

So if your directory arrangement is a variation of these basic
configurations supported, e.g. if your git directory is
~/myproject/.git and your working tree is ~/myproject, and you use
the GIT_DIR and GIT_WORK_TREE to point at them, regardless of which
subdirectory of $GIT_WORK_TREE you started with, Git should already
have done chdir(2) to ~/myproject/ before it starts cmd_log(), and
it should be able to just open and read ".mailmap" (of course, it
needs to limit itself to do so only when it is working with a
repository with a working tree).

If your arrangement is even more exotic, e.g. you have these two
variables set, and then are running from OUTSIDE the working tree,
my knee-jerk reaction is that you should get your head examined, as
it is totally unclear what "git add foo" would mean if you did this:

    $ export GIT_DIR=~/myproject/.git GIT_WORK_TREE=~/myproject
    $ cd ~/myproject/../somewhere/else
    $ git add foo

But it should still "work" in the sense that the above command
should notice that you are trying to add "../somewhere/else/foo" to
the index, which is obviously nonsense, and die with a message.

Similarly, if you replace "git add foo" with "git log", it still
should work in the above, i.e.

    $ export GIT_DIR=~/myproject/.git GIT_WORK_TREE=~/myproject
    $ cd ~/myproject/../somewhere/else
    $ git log

If Git is not chdir(2)ing to ~/myproject before calling cmd_log()
in the above (again, this is my down week so I didn't and will not
check with the code myself), we may want to call that a bug and fix
it, so that you do not have to do anything special to get to the
path of ".mailmap" that is at the top-level.

Re: [PATCH] Make git log work for git CWD outside of work tree

[Duy Nguyen]

On Mon, Apr 10, 2017 at 7:21 AM, Junio C Hamano <gitster@pobox.com> wrote:
> If your arrangement is even more exotic, e.g. you have these two
> variables set, and then are running from OUTSIDE the working tree,
> my knee-jerk reaction is that you should get your head examined, as
> it is totally unclear what "git add foo" would mean if you did this:
>
>     $ export GIT_DIR=~/myproject/.git GIT_WORK_TREE=~/myproject
>     $ cd ~/myproject/../somewhere/else
>     $ git add foo
>
> But it should still "work" in the sense that the above command
> should notice that you are trying to add "../somewhere/else/foo" to
> the index, which is obviously nonsense, and die with a message.
>
> Similarly, if you replace "git add foo" with "git log", it still
> should work in the above, i.e.
>
>     $ export GIT_DIR=~/myproject/.git GIT_WORK_TREE=~/myproject
>     $ cd ~/myproject/../somewhere/else
>     $ git log
>
> If Git is not chdir(2)ing to ~/myproject before calling cmd_log()
> in the above (again, this is my down week so I didn't and will not
> check with the code myself), we may want to call that a bug and fix
> it, so that you do not have to do anything special to get to the
> path of ".mailmap" that is at the top-level.

The behavior is "documented" in t1510 since f3bb8b4b84 (Merge branch
'nd/setup' - 2010-12-28)

"11. When user's cwd is outside worktree, cwd remains unchanged,
    prefix is NULL."

This behavior probably started long before my topic though, mine was
more of documentation, making worktree detection more consistent. It's
the same case with defining GIT_DIR without GIT_WORK_TREE, I think:
scripts started to depend on a behavior that we did not clearly
define, by the time we knew what we wanted and we kept the old
behavior forever.

I think it's just safer to go with Johannes' suggestion.

An alternative is, when you have found out you need to read .mailmap,
you call setup_work_tree() then, which prepares the worktree for you
(including moving back to cwd) or dies if worktree does not exist, or
no-op if worktree has already been asked by somebody. Many commands do
lazy worktree initialization this way.
-- 
Duy

Re: [PATCH] Make git log work for git CWD outside of work tree

[Jeff King]

On Mon, Apr 10, 2017 at 07:01:00PM +0700, Duy Nguyen wrote:

> > Similarly, if you replace "git add foo" with "git log", it still
> > should work in the above, i.e.
> >
> >     $ export GIT_DIR=~/myproject/.git GIT_WORK_TREE=~/myproject
> >     $ cd ~/myproject/../somewhere/else
> >     $ git log
> >
> > If Git is not chdir(2)ing to ~/myproject before calling cmd_log()
> > in the above (again, this is my down week so I didn't and will not
> > check with the code myself), we may want to call that a bug and fix
> > it, so that you do not have to do anything special to get to the
> > path of ".mailmap" that is at the top-level.
> 
> The behavior is "documented" in t1510 since f3bb8b4b84 (Merge branch
> 'nd/setup' - 2010-12-28)
> 
> "11. When user's cwd is outside worktree, cwd remains unchanged,
>     prefix is NULL."
> 
> This behavior probably started long before my topic though, mine was
> more of documentation, making worktree detection more consistent. It's
> the same case with defining GIT_DIR without GIT_WORK_TREE, I think:
> scripts started to depend on a behavior that we did not clearly
> define, by the time we knew what we wanted and we kept the old
> behavior forever.
> 
> I think it's just safer to go with Johannes' suggestion.
> 
> An alternative is, when you have found out you need to read .mailmap,
> you call setup_work_tree() then, which prepares the worktree for you
> (including moving back to cwd) or dies if worktree does not exist, or
> no-op if worktree has already been asked by somebody. Many commands do
> lazy worktree initialization this way.

I think this is much more than just .mailmap, though. For instance, I
have noticed a similar problem with .gitattributes:

  # set up a repo with an attribute
  git init repo
  (
    cd repo &&
    echo content >file &&
    echo "file diff=sed" >.gitattributes &&
    git add . &&
    git commit -m foo &&
    git config diff.sed.textconv 'sed s/^/foo:/'
  )

If I run:

  (cd repo && git log -p)

the diff for "file" shows:

  +foo:content

as expected. But:

  GIT_WORK_TREE=$PWD/repo GIT_DIR=$PWD/repo/.git git log -p

doesn't (while writing this I also noticed that "git log -p file"
doesn't work because of the DWIM lookup. That one is more debatable, but
I still think it makes more sense to move to $GIT_WORK_TREE).

-Peff

Re: [PATCH] Make git log work for git CWD outside of work tree

[Duy Nguyen]

On Tue, Apr 11, 2017 at 12:13 AM, Jeff King <peff@peff.net> wrote:
> On Mon, Apr 10, 2017 at 07:01:00PM +0700, Duy Nguyen wrote:
>> An alternative is, when you have found out you need to read .mailmap,
>> you call setup_work_tree() then, which prepares the worktree for you
>> (including moving back to cwd) or dies if worktree does not exist, or
>> no-op if worktree has already been asked by somebody. Many commands do
>> lazy worktree initialization this way.
>
> I think this is much more than just .mailmap, though. For instance, I
> have noticed a similar problem with .gitattributes:

Urgh. assuming that we should not read .gitattributes if there's no
worktree to read from (similar to the "defaults to .git" situation),
how about

 - if mailmap stuff is requested, setup worktree, or die trying
 - if worktree is detected, but setup code does not jump to it, do it
 - if no worktree is detected, tell git-log to stop reading .gitattributes

We probablly want some "if no wotktree then die()" in .gitattributes
and .gitignore code, just in case it's incorrectly and accidentally
executed in exotic setup
-- 
Duy

Re: [PATCH] Make git log work for git CWD outside of work tree

[Junio C Hamano]

Duy Nguyen <pclouds@gmail.com> writes:

>> I think this is much more than just .mailmap, though. For instance, I
>> have noticed a similar problem with .gitattributes:
>
> Urgh. assuming that we should not read .gitattributes if there's no
> worktree to read from (similar to the "defaults to .git" situation),
> how about
>
>  - if mailmap stuff is requested, setup worktree, or die trying
>  - if worktree is detected, but setup code does not jump to it, do it
>  - if no worktree is detected, tell git-log to stop reading .gitattributes

My gut reaction is that we are doing something wrong once we start
saying "if mailmap stuff is requested".  This is not about .mailmap
but is about how sanely the paths relative to the root of the
working tree (which includes a path in the index, or comparing
$commit:$path with $path in the working tree) can be named by any
subcommand of Git.

Can't we model this after how setup_git_directory_gently() gives the
subcommands a choice?  While the majority of subcommands do not call
the _gently() variant and die when we are not in a repository, the
rest do use it and continue after learning that they are outside a
repository.  

Perhaps we want a new bit GOTO_WORK_TREE_ROOT that is orthogonal to
NEED_WORK_TREE to tell the codepath that calls cmd_foo() to always
move to the root of the working tree (if there is one), regaredless
of the behaviour f3bb8b4b84 documents.  I have a strong suspicion
that we didn't _care_ about a silly use case where GIT_WORK_TREE is
specified and the command is started from somewhere completely
unrelated to that location, and the users with such a silly use case
didn't care either what Git does to the files in the working tree,
i.e. what you quoted in your previous message

    "11. When user's cwd is outside worktree, cwd remains unchanged,
        prefix is NULL."

    This behavior probably started long before my topic though, mine was
    more of documentation, making worktree detection more consistent. It's

was not describing the design, but just describing whatever random
thing the code happened to be doing.

Re: [PATCH] Make git log work for git CWD outside of work tree

[Duy Nguyen]

On Wed, Apr 12, 2017 at 3:41 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Duy Nguyen <pclouds@gmail.com> writes:
>
>>> I think this is much more than just .mailmap, though. For instance, I
>>> have noticed a similar problem with .gitattributes:
>>
>> Urgh. assuming that we should not read .gitattributes if there's no
>> worktree to read from (similar to the "defaults to .git" situation),
>> how about
>>
>>  - if mailmap stuff is requested, setup worktree, or die trying
>>  - if worktree is detected, but setup code does not jump to it, do it
>>  - if no worktree is detected, tell git-log to stop reading .gitattributes
>
> My gut reaction is that we are doing something wrong once we start
> saying "if mailmap stuff is requested".  This is not about .mailmap
> but is about how sanely the paths relative to the root of the
> working tree (which includes a path in the index, or comparing
> $commit:$path with $path in the working tree) can be named by any
> subcommand of Git.

I probably should phrased that as "if any worktree stuff is
requested". I was under an impression that you need to pass a command
line option to use mailmap, but I dind't check the code

> Can't we model this after how setup_git_directory_gently() gives the
> subcommands a choice?  While the majority of subcommands do not call
> the _gently() variant and die when we are not in a repository, the
> rest do use it and continue after learning that they are outside a
> repository.

It may work, but we need to be careful because paths as command line
arguments will not be relative to cwd anymore. If some code assumes
cwd unchanged, they're in trouble. I guess they're in trouble either
way because of the "sometimes chdir, sometimes not" current behavior.

> Perhaps we want a new bit GOTO_WORK_TREE_ROOT that is orthogonal to
> NEED_WORK_TREE to tell the codepath that calls cmd_foo() to always
> move to the root of the working tree (if there is one), regaredless
> of the behaviour f3bb8b4b84 documents.  I have a strong suspicion
> that we didn't _care_ about a silly use case where GIT_WORK_TREE is
> specified and the command is started from somewhere completely
> unrelated to that location, and the users with such a silly use case
> didn't care either what Git does to the files in the working tree,
> i.e. what you quoted in your previous message
>
>     "11. When user's cwd is outside worktree, cwd remains unchanged,
>         prefix is NULL."
>
>     This behavior probably started long before my topic though, mine was
>     more of documentation, making worktree detection more consistent. It's
>
> was not describing the design, but just describing whatever random
> thing the code happened to be doing.

I never said otherwise :) The only thing I was worried was how long it
had behaved that way, long enough that scripts started to depend on
it? If we can get rid of special cases in setup code, I will be the
first one to be happier.
-- 
Duy

Re: [PATCH] Make git log work for git CWD outside of work tree

[Jeff King]

On Wed, Apr 12, 2017 at 01:30:31PM +0700, Duy Nguyen wrote:

> On Tue, Apr 11, 2017 at 12:13 AM, Jeff King <peff@peff.net> wrote:
> > On Mon, Apr 10, 2017 at 07:01:00PM +0700, Duy Nguyen wrote:
> >> An alternative is, when you have found out you need to read .mailmap,
> >> you call setup_work_tree() then, which prepares the worktree for you
> >> (including moving back to cwd) or dies if worktree does not exist, or
> >> no-op if worktree has already been asked by somebody. Many commands do
> >> lazy worktree initialization this way.
> >
> > I think this is much more than just .mailmap, though. For instance, I
> > have noticed a similar problem with .gitattributes:
> 
> Urgh. assuming that we should not read .gitattributes if there's no
> worktree to read from (similar to the "defaults to .git" situation),
> how about
> 
>  - if mailmap stuff is requested, setup worktree, or die trying
>  - if worktree is detected, but setup code does not jump to it, do it
>  - if no worktree is detected, tell git-log to stop reading .gitattributes
> 
> We probablly want some "if no wotktree then die()" in .gitattributes
> and .gitignore code, just in case it's incorrectly and accidentally
> executed in exotic setup

I didn't check what we do with attributes when there is no worktree. The
behavior you describe above sounds right. But note that in my test we
_do_ have a worktree, but just look in the wrong location. So doing a
chdir to $GIT_WORK_TREE would just work.

-Peff

Re: [PATCH] Make git log work for git CWD outside of work tree

[Jeff King]

On Wed, Apr 12, 2017 at 06:13:49PM +0700, Duy Nguyen wrote:

> > Can't we model this after how setup_git_directory_gently() gives the
> > subcommands a choice?  While the majority of subcommands do not call
> > the _gently() variant and die when we are not in a repository, the
> > rest do use it and continue after learning that they are outside a
> > repository.
> 
> It may work, but we need to be careful because paths as command line
> arguments will not be relative to cwd anymore. If some code assumes
> cwd unchanged, they're in trouble. I guess they're in trouble either
> way because of the "sometimes chdir, sometimes not" current behavior.

Exactly. The code itself must respect "prefix", and if it doesn't, it's
broken. So I don't think a code switch makes any sense here. It's not
the code which needs to care, it's the user who might be relying on the
externally visible behavior.

I can't think of a case where the existing behavior actually makes sense
for the user, though. They've provided a $GIT_WORK_TREE, but somehow
want their out-of-worktree relative paths to still work? What are they
using the paths for? Surely:

  cd /out-of-tree
  echo content >foo
  GIT_DIR=/repo/.git GIT_WORK_TREE=/repo git add foo

shouldn't work?

Likewise reading .mailmap and .gitattributes from /out-of-tree is simply
a bug.

I dunno. Maybe I am missing some subtle case, but it's not clear to me
what the user would be trying to do by having git stay in the original
directory.

-Peff

Re: [PATCH] Make git log work for git CWD outside of work tree

[Duy Nguyen]

On Wed, Apr 12, 2017 at 8:01 PM, Jeff King <peff@peff.net> wrote:
> I dunno. Maybe I am missing some subtle case, but it's not clear to me
> what the user would be trying to do by having git stay in the original
> directory.

Not sure if it really happens. But if we jump from worktree is inside
original cwd and we have to jump to worktree, we set empty prefix, not
"../../../" one. So we can't refer back to files relative to original
cwd with prefix. I was kinda hoping "super prefix" would solve it, but
that one seems designed specifically for submodules.
-- 
Duy

Re: [PATCH] Make git log work for git CWD outside of work tree

[Jeff King]

On Wed, Apr 12, 2017 at 08:11:22PM +0700, Duy Nguyen wrote:

> On Wed, Apr 12, 2017 at 8:01 PM, Jeff King <peff@peff.net> wrote:
> > I dunno. Maybe I am missing some subtle case, but it's not clear to me
> > what the user would be trying to do by having git stay in the original
> > directory.
> 
> Not sure if it really happens. But if we jump from worktree is inside
> original cwd and we have to jump to worktree, we set empty prefix, not
> "../../../" one. So we can't refer back to files relative to original
> cwd with prefix. I was kinda hoping "super prefix" would solve it, but
> that one seems designed specifically for submodules.

Ah, right. I think the issue is that "prefix" really serves two uses.
For things like command-line arguments, we use to find the original path
after we've moved. But we also use it for "where in the working tree
are we".

So with an out-of-tree cwd, we'd want to set the first one to the real
path ("../../whatever" or even just an absolute path), but the second
one would probably be empty (i.e., just pretend that we started from the
top-level).

But that would require first refactoring all of the cmd_* functions to
handle those prefixes separately.

-Peff

Re: [PATCH] Make git log work for git CWD outside of work tree

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

>> cwd with prefix. I was kinda hoping "super prefix" would solve it, but
>> that one seems designed specifically for submodules.
>
> Ah, right. I think the issue is that "prefix" really serves two uses.
> For things like command-line arguments, we use to find the original path
> after we've moved. But we also use it for "where in the working tree
> are we".
>
> So with an out-of-tree cwd, we'd want to set the first one to the real
> path ("../../whatever" or even just an absolute path), but the second
> one would probably be empty (i.e., just pretend that we started from the
> top-level).
>
> But that would require first refactoring all of the cmd_* functions to
> handle those prefixes separately.

Yes, I am still kinda hoping that the "super prefix" thing will
address the separation of the two uses, though.

Re: [PATCH] Make git log work for git CWD outside of work tree

[Duy Nguyen]

On Fri, Apr 14, 2017 at 4:29 AM, Jeff King <peff@peff.net> wrote:
> On Wed, Apr 12, 2017 at 08:11:22PM +0700, Duy Nguyen wrote:
>
>> On Wed, Apr 12, 2017 at 8:01 PM, Jeff King <peff@peff.net> wrote:
>> > I dunno. Maybe I am missing some subtle case, but it's not clear to me
>> > what the user would be trying to do by having git stay in the original
>> > directory.
>>
>> Not sure if it really happens. But if we jump from worktree is inside
>> original cwd and we have to jump to worktree, we set empty prefix, not
>> "../../../" one. So we can't refer back to files relative to original
>> cwd with prefix. I was kinda hoping "super prefix" would solve it, but
>> that one seems designed specifically for submodules.
>
> Ah, right. I think the issue is that "prefix" really serves two uses.
> For things like command-line arguments, we use to find the original path
> after we've moved. But we also use it for "where in the working tree
> are we".
>
> So with an out-of-tree cwd, we'd want to set the first one to the real
> path ("../../whatever" or even just an absolute path), but the second
> one would probably be empty (i.e., just pretend that we started from the
> top-level).
>
> But that would require first refactoring all of the cmd_* functions to
> handle those prefixes separately.

Yeah. I probably shouldn't start another series until all others of
mine settle. But if anyone is changing cmd_*, I suggest we take this
opportunity to pass "struct struct startup_info *" in the as the only
option. argv/argv could be stored there as well. And you can add
"cwd_prefix" to present that "../../whatever" (I would avoid the name
"super prefix" until things settle), or just keep the original "cwd"
in there and let people do whatever they want with it.
-- 
Duy