Re: [PATCH 0/9] Avoid problem where git_dir is set after alias expansion

[Brandon Williams <bmwill@google.com>]

On 06/07, Johannes Schindelin wrote:
> When expanding an alias in a subdirectory, we setup the git_dir
> (gently), read the config, and then restore the "env" (e.g. the current
> working directory) so that the command specified by the alias can run
> correctly.
> 
> What we failed to reset was the git_dir, meaning that in the most common
> case, it was now pointing to a .git/ directory *in the subdirectory*.
> 
> This problem was identified in the GVFS fork, where a pre-command hook
> was introduced to allow pre-fetching missing blobs.
> 
> An early quick fix in the GVFS fork simply built on top of the
> save_env_before_alias() hack, introducing another hack that saves the
> git_dir and restores it after an alias is expanded:
> 
> 	https://github.com/Microsoft/git/commit/2d859ba3b
> 
> That is very hacky, though, and it is much better (although much more
> involved, too) to fix this "properly", i.e. by replacing the ugly
> save/restore logic by simply using the early config code path.
> 
> However, aliases are strange beasts.
> 
> When an alias refers to a single Git command (originally the sole
> intention of aliases), the current working directory is restored to what
> it had been before expanding the alias.
> 
> But when an alias starts with an exclamation point, i.e. referring to a
> command-line to be interpreted by the shell, the current working
> directory is no longer in the subdirectory but instead in the worktree's
> top-level directory.
> 
> This is even true for worktrees added by `git worktree add`.
> 
> But when we are inside the .git/ directory, the current working
> directory is *restored* to the subdirectory inside the .git/ directory.
> 
> In short, the logic is a bit complicated what is the expected current
> working directory after expanding an alias and before actually running
> it.
> 
> That is why this patch series had to expand the signature of the early
> config machinery to return the additional information for aliases'
> benefit.
> 

Looks good, I don't have any major issues with the series, just some
comments for clarity mostly.  And relevant to this series, you may be
interested in looking at patch 03/31 in my repository object series as
that may have an impact on the early config stuff.

> 
> Johannes Schindelin (9):
>   discover_git_directory(): avoid setting invalid git_dir
>   config: report correct line number upon error
>   help: use early config when autocorrecting aliases
>   read_early_config(): optionally return the worktree's top-level
>     directory
>   t1308: relax the test verifying that empty alias values are disallowed
>   t7006: demonstrate a problem with aliases in subdirectories
>   alias_lookup(): optionally return top-level directory
>   Use the early config machinery to expand aliases
>   TODO:
> 
>  alias.c                | 33 +++++++++++++++++++++-------
>  builtin/help.c         |  2 +-
>  cache.h                |  7 +++---
>  config.c               |  7 +++---
>  git.c                  | 59 ++++++--------------------------------------------
>  help.c                 |  2 +-
>  pager.c                |  4 ++--
>  setup.c                | 13 +++++++++--
>  t/helper/test-config.c |  2 +-
>  t/t1308-config-set.sh  |  4 +++-
>  t/t7006-pager.sh       | 11 ++++++++++
>  11 files changed, 70 insertions(+), 74 deletions(-)
> 
> 
> base-commit: 8d1b10321b20bd2a73a5b561cfc3cf2e8051b70b
> Published-As: https://github.com/dscho/git/releases/tag/alias-early-config-v1
> Fetch-It-Via: git fetch https://github.com/dscho/git alias-early-config-v1
> -- 
> 2.13.0.windows.1.460.g13f583bedb5
> 

-- 
Brandon Williams